mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 21:26:39 +00:00
fix(lark): publish the CLI runtime files atomically (#6618)
* fix(lark): publish the CLI runtime files atomically install_shim wrote the shim, the launcher and the runtime marker with a truncating open. The sandbox chmods the launcher 0o755 and execs it, so a crash mid-write left an executable fragment, and the marker is parsed as JSON. Publish through a temporary file plus os.replace, as the storage backends already do. * style: drop the unused import ruff flags * fix(lark): publish runtime permissions atomically Set file permissions on temporary files before os.replace() to ensure the launcher, shim, and runtime marker are atomically published with correct permissions (0o755 for executables, 0o644 for the marker). This addresses reviewer feedback from PR #6591: - Permissions are now set before the atomic replace operation - Prevents brief window where files have incorrect permissions - The runtime marker now has mode 0o644 instead of 0600 (mkstemp default) Also updates test_lark_shim_atomic.py: - Fixes test_atomic_helper to match new function signature - Adds test_install_shim_publishes_correct_permissions to verify launcher/shim (0o755) and marker (0o644) permissions Co-authored-by: IronMurphy <hepushuo@gmail.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
This commit is contained in:
parent
38c55c67f6
commit
05297813ae
@ -24,6 +24,7 @@ import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
import threading
|
||||
import time
|
||||
from dataclasses import dataclass
|
||||
@ -461,6 +462,31 @@ def serve(config: BrokerConfig) -> ThreadingHTTPServer:
|
||||
return server
|
||||
|
||||
|
||||
def _write_text_atomically(path: str, content: str, *, mode: int) -> None:
|
||||
"""Publish complete *content* and its final *mode* together at *path*.
|
||||
|
||||
Set permissions on the temporary file before fsync and ``os.replace`` so
|
||||
an interrupted install cannot publish a non-executable launcher. A write,
|
||||
permission, or publish failure leaves the previous file untouched.
|
||||
"""
|
||||
directory = os.path.dirname(os.path.abspath(path))
|
||||
os.makedirs(directory, exist_ok=True)
|
||||
fd, temporary = tempfile.mkstemp(dir=directory, prefix=f".{os.path.basename(path)}.", suffix=".tmp")
|
||||
try:
|
||||
with os.fdopen(fd, "w", encoding="utf-8") as handle:
|
||||
handle.write(content)
|
||||
handle.flush()
|
||||
os.chmod(temporary, mode)
|
||||
os.fsync(handle.fileno())
|
||||
os.replace(temporary, path)
|
||||
except BaseException:
|
||||
try:
|
||||
os.unlink(temporary)
|
||||
except OSError:
|
||||
pass
|
||||
raise
|
||||
|
||||
|
||||
def install_shim(dest_dir: str, *, version: str | None = None) -> str:
|
||||
"""Write the launcher + shim + runtime marker into the sandbox runtime dir.
|
||||
|
||||
@ -481,16 +507,11 @@ def install_shim(dest_dir: str, *, version: str | None = None) -> str:
|
||||
bin_dir = os.path.join(dest, "bin")
|
||||
os.makedirs(bin_dir, exist_ok=True)
|
||||
shim_body = os.path.join(bin_dir, LARK_CLI_BROKER_SHIM_FILENAME)
|
||||
with open(shim_body, "w", encoding="utf-8") as handle:
|
||||
handle.write(LARK_CLI_BROKER_SHIM_SCRIPT)
|
||||
os.chmod(shim_body, 0o755)
|
||||
_write_text_atomically(shim_body, LARK_CLI_BROKER_SHIM_SCRIPT, mode=0o755)
|
||||
launcher = os.path.join(bin_dir, "lark-cli")
|
||||
with open(launcher, "w", encoding="utf-8") as handle:
|
||||
handle.write(render_launcher_script(shim_body))
|
||||
os.chmod(launcher, 0o755)
|
||||
_write_text_atomically(launcher, render_launcher_script(shim_body), mode=0o755)
|
||||
marker = os.path.join(dest, ".deerflow-lark-cli-runtime.json")
|
||||
with open(marker, "w", encoding="utf-8") as handle:
|
||||
json.dump({"version": version or "unknown", "kind": "shim"}, handle)
|
||||
_write_text_atomically(marker, json.dumps({"version": version or "unknown", "kind": "shim"}), mode=0o644)
|
||||
return launcher
|
||||
|
||||
|
||||
|
||||
87
backend/tests/test_lark_shim_atomic.py
Normal file
87
backend/tests/test_lark_shim_atomic.py
Normal file
@ -0,0 +1,87 @@
|
||||
# SPDX-License-Identifier: MIT
|
||||
"""The Lark CLI runtime files must never be observable half written."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
|
||||
import pytest
|
||||
|
||||
from deerflow.integrations.lark_broker import install_shim
|
||||
|
||||
|
||||
def test_install_shim_publishes_executable_files(tmp_path):
|
||||
launcher = install_shim(str(tmp_path), version="1.2.3")
|
||||
|
||||
assert os.access(launcher, os.X_OK)
|
||||
marker = tmp_path / ".deerflow-lark-cli-runtime.json"
|
||||
assert json.loads(marker.read_text(encoding="utf-8"))["version"] == "1.2.3"
|
||||
assert [p.name for p in (tmp_path / "bin").glob("*.tmp")] == []
|
||||
|
||||
|
||||
def test_install_shim_publishes_correct_permissions(tmp_path):
|
||||
"""Launcher and shim must be executable (0o755), marker readable (0o644)."""
|
||||
launcher = install_shim(str(tmp_path), version="1.2.3")
|
||||
|
||||
launcher_stat = os.stat(launcher)
|
||||
assert launcher_stat.st_mode & 0o777 == 0o755
|
||||
|
||||
shim_body = tmp_path / "bin" / "lark-cli-shim.py"
|
||||
shim_stat = os.stat(shim_body)
|
||||
assert shim_stat.st_mode & 0o777 == 0o755
|
||||
|
||||
marker = tmp_path / ".deerflow-lark-cli-runtime.json"
|
||||
marker_stat = os.stat(marker)
|
||||
assert marker_stat.st_mode & 0o777 == 0o644
|
||||
|
||||
|
||||
def test_a_failed_publish_leaves_the_previous_runtime_intact(tmp_path, monkeypatch):
|
||||
install_shim(str(tmp_path), version="1.0.0")
|
||||
marker = tmp_path / ".deerflow-lark-cli-runtime.json"
|
||||
before = marker.read_text(encoding="utf-8")
|
||||
|
||||
def _boom(*args, **kwargs):
|
||||
raise OSError("read-only filesystem")
|
||||
|
||||
monkeypatch.setattr(os, "replace", _boom)
|
||||
|
||||
with pytest.raises(OSError):
|
||||
install_shim(str(tmp_path), version="2.0.0")
|
||||
|
||||
# A truncating open would have emptied the marker before failing, leaving the
|
||||
# gateway with unparseable JSON; publishing through a temporary file cannot.
|
||||
assert marker.read_text(encoding="utf-8") == before
|
||||
|
||||
|
||||
def test_atomic_helper_keeps_the_previous_file_when_publish_fails(tmp_path, monkeypatch):
|
||||
from deerflow.integrations.lark_broker import _write_text_atomically
|
||||
|
||||
target = tmp_path / "runtime.json"
|
||||
target.write_text('{"version": "1.0.0"}', encoding="utf-8")
|
||||
|
||||
def _boom(*args, **kwargs):
|
||||
raise OSError("read-only filesystem")
|
||||
|
||||
monkeypatch.setattr(os, "replace", _boom)
|
||||
|
||||
with pytest.raises(OSError):
|
||||
_write_text_atomically(str(target), '{"version": "2.0.0"}', mode=0o644)
|
||||
|
||||
assert target.read_text(encoding="utf-8") == '{"version": "1.0.0"}'
|
||||
assert list(tmp_path.glob("*.tmp")) == []
|
||||
|
||||
|
||||
def test_truncating_open_would_have_destroyed_it(tmp_path):
|
||||
"""Control: the previous implementation loses the content in the same failure."""
|
||||
target = tmp_path / "runtime.json"
|
||||
target.write_text('{"version": "1.0.0"}', encoding="utf-8")
|
||||
|
||||
try:
|
||||
with open(target, "w", encoding="utf-8") as handle:
|
||||
handle.write('{"version": ')
|
||||
raise OSError("read-only filesystem")
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
assert target.read_text(encoding="utf-8") != '{"version": "1.0.0"}'
|
||||
Loading…
x
Reference in New Issue
Block a user