mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-15 00:19:14 +00:00
fix(sandbox): platform-aware Lark CLI runtime validation for Windows hosts (#5442)
* fix(sandbox): platform-aware Lark CLI runtime validation for Windows hosts The managed Lark CLI sandbox runtime validation and its tests assumed POSIX semantics that Windows hosts cannot satisfy, breaking the focused AIO/Lark CLI suites (5 failures on current main). - _validate_lark_cli_sandbox_runtime keeps the strict executable-bit contract on POSIX; on Windows it validates the Linux-only artifacts by content instead (ELF/PE/Mach-O image magic for linux-*/lark-cli, shebang for the bin/lark-cli launcher), since NTFS cannot represent the exec bit. - The AIO runtime-mounts test now asserts the explicit Windows credential contract: an owner-only inheritable DACL (via the existing PowerShell ACL resolvers) instead of exact 0o700 modes, which remain asserted on POSIX. - Accept-path runtime tests stage ELF-prefixed payloads so both platforms exercise realistic artifact content; the extractor's mode assertion is POSIX-only with a writability check on Windows. Focused suites: 5 failed / 162 passed -> 167 passed, 3 skipped on Windows 11; POSIX behavior unchanged (POSIX branches keep the original assertions). * refactor(tests): share Windows ACL resolvers via a helper module and cover the Windows shebang gate Review follow-ups on #5442: - Move the PowerShell ACL resolvers (_windows_acl_env/_windows_acl_sids/ _windows_acl_protected/_windows_acl_owner_sid) from tests/test_lark_cli_integration.py into tests/_windows_acl_helpers.py, following the existing shared-helper convention, so the aio suite no longer imports the full lark-cli integration module (which drags in app.gateway routers and the FastAPI TestClient at collection time). - Add test_managed_sandbox_runtime_rejects_launcher_without_shebang_on_windows: a launcher without a shebang plus ELF-magic binaries, with lark_cli.os monkeypatched via the existing Windows stub so the shebang-missing reject branch of _runtime_artifact_is_executable is covered on every platform. - Comment the rejects-non-executable prestaged-binary test to record that on Windows the rejection comes from the payload's non-magic content, since chmod() cannot clear the exec bit there. Focused suites: 168 passed / 3 skipped on Windows 11.
This commit is contained in:
xiaodu55
GitHub
parent
3dc895df4d
commit
2536f24f81
@ -1249,6 +1249,23 @@ def _write_lark_cli_sandbox_launcher(staging: Path) -> None:
|
||||
launcher.chmod(0o755)
|
||||
|
||||
|
||||
def _runtime_artifact_is_executable(relative: Path, candidate: Path) -> bool:
|
||||
"""Decide whether a managed runtime artifact is executable, platform-aware.
|
||||
|
||||
POSIX keeps the strict executable-bit contract. NTFS cannot represent the
|
||||
exec bit, so Windows validates these Linux-only artifacts by content
|
||||
instead: the per-arch binaries must carry an executable image magic and
|
||||
the ``bin/lark-cli`` launcher must be a script with a shebang.
|
||||
"""
|
||||
if os.name != "nt":
|
||||
return candidate.stat().st_mode & 0o111 != 0
|
||||
with candidate.open("rb") as handle:
|
||||
prefix = handle.read(4)
|
||||
if relative == Path("bin/lark-cli"):
|
||||
return prefix.startswith(b"#!")
|
||||
return is_executable_binary_prefix(prefix)
|
||||
|
||||
|
||||
def _validate_lark_cli_sandbox_runtime(root: Path) -> None:
|
||||
if root.is_symlink() or not root.is_dir():
|
||||
raise ValueError("Managed Lark CLI sandbox runtime root must be a regular directory, not a symlink.")
|
||||
@ -1261,7 +1278,7 @@ def _validate_lark_cli_sandbox_runtime(root: Path) -> None:
|
||||
candidate = root / relative
|
||||
if not candidate.is_file():
|
||||
raise ValueError(f"Managed Lark CLI sandbox runtime is missing a regular file: {relative}")
|
||||
if candidate.stat().st_mode & 0o111 == 0:
|
||||
if not _runtime_artifact_is_executable(relative, candidate):
|
||||
raise ValueError(f"Managed Lark CLI sandbox runtime file is not executable: {relative}")
|
||||
|
||||
|
||||
|
||||
70
backend/tests/_windows_acl_helpers.py
Normal file
70
backend/tests/_windows_acl_helpers.py
Normal file
@ -0,0 +1,70 @@
|
||||
"""Shared Windows ACL inspection helpers for lark-cli credential tests.
|
||||
|
||||
These resolvers shell out to Windows PowerShell ``Get-Acl`` and translate ACEs
|
||||
to raw SIDs so tests can assert the owner-only DACL contract that the
|
||||
credential-tree hardener establishes on NTFS (where POSIX modes are not
|
||||
representable). They are Windows-only; callers gate them on ``os.name == "nt"``.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def _windows_acl_env() -> dict[str, str]:
|
||||
"""Return a PowerShell environment with a clean, ordered ``PSModulePath``.
|
||||
|
||||
The Codex runtime prepends a bundled PowerShell module path that shadows the
|
||||
stock ``Microsoft.PowerShell.Security`` module, which makes ``Get-Acl`` fail
|
||||
to autoload under ``-NoProfile``. Use the stock Windows PowerShell module path
|
||||
so ACL inspection is reliable on any host.
|
||||
"""
|
||||
system_root = os.environ.get("SystemRoot", r"C:\Windows")
|
||||
program_files = os.environ.get("ProgramFiles", r"C:\Program Files")
|
||||
modules = f"{system_root}\\system32\\WindowsPowerShell\\v1.0\\Modules;{program_files}\\WindowsPowerShell\\Modules"
|
||||
return {**os.environ, "PSModulePath": modules}
|
||||
|
||||
|
||||
def _windows_acl_sids(path: Path) -> set[str]:
|
||||
"""Return the SIDs granted on *path* (Windows-only, PowerShell resolver).
|
||||
|
||||
``icacls`` displays localized account names rather than raw SIDs, so we
|
||||
translate each ACE IdentityReference back to a SID before asserting.
|
||||
"""
|
||||
cmd = "(Get-Acl -LiteralPath '" + str(path) + "').Access | ForEach-Object { $_.IdentityReference.Translate([System.Security.Principal.SecurityIdentifier]).Value }"
|
||||
out = subprocess.run(
|
||||
["powershell", "-NoProfile", "-Command", cmd],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
env=_windows_acl_env(),
|
||||
)
|
||||
return {line.strip() for line in out.stdout.splitlines() if line.strip()}
|
||||
|
||||
|
||||
def _windows_acl_protected(path: Path) -> bool:
|
||||
"""Return whether *path*'s DACL is protected from inheritance (Windows-only)."""
|
||||
cmd = "(Get-Acl -LiteralPath '" + str(path) + "').AreAccessRulesProtected"
|
||||
out = subprocess.run(
|
||||
["powershell", "-NoProfile", "-Command", cmd],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
env=_windows_acl_env(),
|
||||
)
|
||||
return out.stdout.strip() == "True"
|
||||
|
||||
|
||||
def _windows_acl_owner_sid(path: Path) -> str:
|
||||
"""Return *path*'s object owner as a raw SID (Windows-only)."""
|
||||
cmd = "$acl = Get-Acl -LiteralPath $env:DEER_FLOW_TEST_ACL_PATH; $acl.GetOwner([System.Security.Principal.SecurityIdentifier]).Value"
|
||||
out = subprocess.run(
|
||||
["powershell", "-NoProfile", "-Command", cmd],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
env={**_windows_acl_env(), "DEER_FLOW_TEST_ACL_PATH": str(path)},
|
||||
)
|
||||
return out.stdout.strip()
|
||||
@ -4,12 +4,14 @@ import asyncio
|
||||
import contextlib
|
||||
import hashlib
|
||||
import importlib
|
||||
import os
|
||||
import stat
|
||||
import threading
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from _windows_acl_helpers import _windows_acl_owner_sid, _windows_acl_sids
|
||||
|
||||
from deerflow.config.paths import Paths, join_host_path
|
||||
from deerflow.config.sandbox_config import SandboxConfig
|
||||
@ -218,9 +220,19 @@ def test_get_lark_cli_runtime_mounts_uses_user_auth_dirs(tmp_path, monkeypatch):
|
||||
str(tmp_path / "users" / "alice" / "integrations" / "lark-cli" / "data"),
|
||||
False,
|
||||
)
|
||||
assert stat.S_IMODE((tmp_path / "users" / "alice" / "integrations" / "lark-cli" / "config").stat().st_mode) == 0o700
|
||||
assert stat.S_IMODE((tmp_path / "users" / "alice" / "integrations" / "lark-cli" / "config" / "locks").stat().st_mode) == 0o700
|
||||
assert stat.S_IMODE((tmp_path / "users" / "alice" / "integrations" / "lark-cli" / "data").stat().st_mode) == 0o700
|
||||
config_dir = tmp_path / "users" / "alice" / "integrations" / "lark-cli" / "config"
|
||||
locks_dir = config_dir / "locks"
|
||||
data_dir = tmp_path / "users" / "alice" / "integrations" / "lark-cli" / "data"
|
||||
if os.name == "nt":
|
||||
# NTFS cannot represent POSIX modes; the contract the credential-tree
|
||||
# hardener establishes on Windows is an owner-only inheritable DACL.
|
||||
owner_sid = _windows_acl_owner_sid(config_dir)
|
||||
for hardened in (config_dir, locks_dir, data_dir):
|
||||
assert _windows_acl_sids(hardened) == {owner_sid}
|
||||
else:
|
||||
assert stat.S_IMODE(config_dir.stat().st_mode) == 0o700
|
||||
assert stat.S_IMODE(locks_dir.stat().st_mode) == 0o700
|
||||
assert stat.S_IMODE(data_dir.stat().st_mode) == 0o700
|
||||
assert container_paths["/mnt/integrations/lark-cli/runtime"] == (
|
||||
str(runtime_dir),
|
||||
True,
|
||||
|
||||
@ -21,6 +21,7 @@ from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from _router_auth_helpers import make_authed_test_app
|
||||
from _windows_acl_helpers import _windows_acl_owner_sid, _windows_acl_protected, _windows_acl_sids
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from app.gateway.auth.models import User
|
||||
@ -92,63 +93,6 @@ def _bootstrap_credential_dirs(monkeypatch, tmp_path, *, config: bool = True, da
|
||||
return config_dir, data_dir
|
||||
|
||||
|
||||
def _windows_acl_env() -> dict[str, str]:
|
||||
"""Return a PowerShell environment with a clean, ordered ``PSModulePath``.
|
||||
|
||||
The Codex runtime prepends a bundled PowerShell module path that shadows the
|
||||
stock ``Microsoft.PowerShell.Security`` module, which makes ``Get-Acl`` fail
|
||||
to autoload under ``-NoProfile``. Use the stock Windows PowerShell module path
|
||||
so ACL inspection is reliable on any host.
|
||||
"""
|
||||
system_root = os.environ.get("SystemRoot", r"C:\Windows")
|
||||
program_files = os.environ.get("ProgramFiles", r"C:\Program Files")
|
||||
modules = f"{system_root}\\system32\\WindowsPowerShell\\v1.0\\Modules;{program_files}\\WindowsPowerShell\\Modules"
|
||||
return {**os.environ, "PSModulePath": modules}
|
||||
|
||||
|
||||
def _windows_acl_sids(path: Path) -> set[str]:
|
||||
"""Return the SIDs granted on *path* (Windows-only, PowerShell resolver).
|
||||
|
||||
``icacls`` displays localized account names rather than raw SIDs, so we
|
||||
translate each ACE IdentityReference back to a SID before asserting.
|
||||
"""
|
||||
cmd = "(Get-Acl -LiteralPath '" + str(path) + "').Access | ForEach-Object { $_.IdentityReference.Translate([System.Security.Principal.SecurityIdentifier]).Value }"
|
||||
out = subprocess.run(
|
||||
["powershell", "-NoProfile", "-Command", cmd],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
env=_windows_acl_env(),
|
||||
)
|
||||
return {line.strip() for line in out.stdout.splitlines() if line.strip()}
|
||||
|
||||
|
||||
def _windows_acl_protected(path: Path) -> bool:
|
||||
"""Return whether *path*'s DACL is protected from inheritance (Windows-only)."""
|
||||
cmd = "(Get-Acl -LiteralPath '" + str(path) + "').AreAccessRulesProtected"
|
||||
out = subprocess.run(
|
||||
["powershell", "-NoProfile", "-Command", cmd],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
env=_windows_acl_env(),
|
||||
)
|
||||
return out.stdout.strip() == "True"
|
||||
|
||||
|
||||
def _windows_acl_owner_sid(path: Path) -> str:
|
||||
"""Return *path*'s object owner as a raw SID (Windows-only)."""
|
||||
cmd = "$acl = Get-Acl -LiteralPath $env:DEER_FLOW_TEST_ACL_PATH; $acl.GetOwner([System.Security.Principal.SecurityIdentifier]).Value"
|
||||
out = subprocess.run(
|
||||
["powershell", "-NoProfile", "-Command", cmd],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
env={**_windows_acl_env(), "DEER_FLOW_TEST_ACL_PATH": str(path)},
|
||||
)
|
||||
return out.stdout.strip()
|
||||
|
||||
|
||||
class _FakeWindowsHandle:
|
||||
"""Real-filesystem-backed stand-in for ``_WindowsTreeHandle`` used by mocks.
|
||||
|
||||
@ -295,8 +239,8 @@ def test_managed_sandbox_runtime_verifies_and_installs_linux_archives(monkeypatc
|
||||
assert hasattr(lark_cli, "_ensure_managed_sandbox_lark_cli"), "managed sandbox runtime installer is missing"
|
||||
_patch_paths(monkeypatch, tmp_path / "home")
|
||||
archives = {
|
||||
"lark-cli-1.0.65-linux-amd64.tar.gz": _make_lark_cli_binary_tar(b"amd64-binary"),
|
||||
"lark-cli-1.0.65-linux-arm64.tar.gz": _make_lark_cli_binary_tar(b"arm64-binary"),
|
||||
"lark-cli-1.0.65-linux-amd64.tar.gz": _make_lark_cli_binary_tar(b"\x7fELF-amd64-payload"),
|
||||
"lark-cli-1.0.65-linux-arm64.tar.gz": _make_lark_cli_binary_tar(b"\x7fELF-arm64-payload"),
|
||||
}
|
||||
checksums = "".join(f"{hashlib.sha256(payload).hexdigest()} {name}\n" for name, payload in archives.items()).encode()
|
||||
assets = {"checksums.txt": checksums, **archives}
|
||||
@ -305,9 +249,15 @@ def test_managed_sandbox_runtime_verifies_and_installs_linux_archives(monkeypatc
|
||||
|
||||
runtime = lark_cli._ensure_managed_sandbox_lark_cli("v1.0.65")
|
||||
|
||||
assert (runtime / "linux-amd64" / "lark-cli").read_bytes() == b"amd64-binary"
|
||||
assert (runtime / "linux-arm64" / "lark-cli").read_bytes() == b"arm64-binary"
|
||||
assert stat.S_IMODE((runtime / "linux-amd64" / "lark-cli").stat().st_mode) == 0o755
|
||||
assert (runtime / "linux-amd64" / "lark-cli").read_bytes() == b"\x7fELF-amd64-payload"
|
||||
assert (runtime / "linux-arm64" / "lark-cli").read_bytes() == b"\x7fELF-arm64-payload"
|
||||
installed_mode = stat.S_IMODE((runtime / "linux-amd64" / "lark-cli").stat().st_mode)
|
||||
if os.name == "nt":
|
||||
# NTFS cannot represent the exec bit; writability is the strongest
|
||||
# host-side contract the extractor can establish for the artifact.
|
||||
assert installed_mode & 0o222
|
||||
else:
|
||||
assert installed_mode == 0o755
|
||||
launcher = (runtime / "bin" / "lark-cli").read_text(encoding="utf-8")
|
||||
assert "uname -m" in launcher
|
||||
assert "x86_64" in launcher and "aarch64" in launcher
|
||||
@ -353,7 +303,7 @@ def test_managed_sandbox_runtime_accepts_prestaged_airgapped_tree(monkeypatch, t
|
||||
for arch in ("amd64", "arm64"):
|
||||
binary = source / f"linux-{arch}" / "lark-cli"
|
||||
binary.parent.mkdir(parents=True)
|
||||
binary.write_bytes(f"{arch}-binary".encode())
|
||||
binary.write_bytes(b"\x7fELF" + f"{arch}-binary".encode())
|
||||
binary.chmod(0o755)
|
||||
launcher = source / "bin" / "lark-cli"
|
||||
launcher.parent.mkdir(parents=True)
|
||||
@ -368,8 +318,8 @@ def test_managed_sandbox_runtime_accepts_prestaged_airgapped_tree(monkeypatch, t
|
||||
|
||||
runtime = lark_cli._ensure_managed_sandbox_lark_cli("v1.0.65")
|
||||
|
||||
assert (runtime / "linux-amd64" / "lark-cli").read_bytes() == b"amd64-binary"
|
||||
assert (runtime / "linux-arm64" / "lark-cli").read_bytes() == b"arm64-binary"
|
||||
assert (runtime / "linux-amd64" / "lark-cli").read_bytes() == b"\x7fELFamd64-binary"
|
||||
assert (runtime / "linux-arm64" / "lark-cli").read_bytes() == b"\x7fELFarm64-binary"
|
||||
|
||||
|
||||
def test_managed_sandbox_runtime_rejects_any_symlink_in_prestaged_tree(monkeypatch, tmp_path) -> None:
|
||||
@ -401,6 +351,8 @@ def test_managed_sandbox_runtime_rejects_any_symlink_in_prestaged_tree(monkeypat
|
||||
def test_managed_sandbox_runtime_rejects_non_executable_prestaged_binary(monkeypatch, tmp_path) -> None:
|
||||
_patch_paths(monkeypatch, tmp_path / "home")
|
||||
source = tmp_path / "pre-staged"
|
||||
# On Windows chmod() cannot clear the exec bit, so rejection there comes
|
||||
# from the non-magic payload content rather than the 0o644 mode below.
|
||||
for arch in ("amd64", "arm64"):
|
||||
binary = source / f"linux-{arch}" / "lark-cli"
|
||||
binary.parent.mkdir(parents=True)
|
||||
@ -419,13 +371,48 @@ def test_managed_sandbox_runtime_rejects_non_executable_prestaged_binary(monkeyp
|
||||
assert not lark_cli.lark_cli_managed_sandbox_dir().exists()
|
||||
|
||||
|
||||
def test_managed_sandbox_runtime_rejects_launcher_without_shebang_on_windows(monkeypatch, tmp_path) -> None:
|
||||
"""The Windows content gate must reject a launcher without a shebang.
|
||||
|
||||
``test_managed_sandbox_runtime_rejects_non_executable_prestaged_binary``
|
||||
stages a launcher WITH a shebang, so on Windows its rejection always comes
|
||||
from the per-arch binary content and this branch is only exercised
|
||||
positively. Monkeypatching ``lark_cli.os`` with the Windows stub runs the
|
||||
Windows branch on POSIX CI too; ``candidate.open()`` is a pathlib method,
|
||||
so the stub is safe here.
|
||||
"""
|
||||
_patch_paths(monkeypatch, tmp_path / "home")
|
||||
source = tmp_path / "pre-staged"
|
||||
for arch in ("amd64", "arm64"):
|
||||
binary = source / f"linux-{arch}" / "lark-cli"
|
||||
binary.parent.mkdir(parents=True)
|
||||
binary.write_bytes(b"\x7fELF" + f"{arch}-binary".encode())
|
||||
binary.chmod(0o755)
|
||||
launcher = source / "bin" / "lark-cli"
|
||||
launcher.parent.mkdir(parents=True)
|
||||
launcher.write_text("echo not-a-launcher\n", encoding="utf-8")
|
||||
launcher.chmod(0o755)
|
||||
monkeypatch.setenv(lark_cli.LARK_CLI_SANDBOX_RUNTIME_SOURCE_ENV, str(source))
|
||||
windows_os = _windows_os_stub()
|
||||
# The installer also touches a few host-neutral os helpers; only os.name
|
||||
# must read "nt" to drive the validation branch under test.
|
||||
windows_os.getenv = os.getenv
|
||||
windows_os.getpid = os.getpid
|
||||
monkeypatch.setattr(lark_cli, "os", windows_os)
|
||||
|
||||
with pytest.raises(ValueError, match="executable"):
|
||||
lark_cli._ensure_managed_sandbox_lark_cli("v1.0.65")
|
||||
|
||||
assert not lark_cli.lark_cli_managed_sandbox_dir().exists()
|
||||
|
||||
|
||||
def test_concurrent_managed_sandbox_runtime_installs_serialize_replacement(monkeypatch, tmp_path) -> None:
|
||||
_patch_paths(monkeypatch, tmp_path / "home")
|
||||
source = tmp_path / "pre-staged"
|
||||
for arch in ("amd64", "arm64"):
|
||||
binary = source / f"linux-{arch}" / "lark-cli"
|
||||
binary.parent.mkdir(parents=True)
|
||||
binary.write_bytes(f"{arch}-binary".encode())
|
||||
binary.write_bytes(b"\x7fELF" + f"{arch}-binary".encode())
|
||||
binary.chmod(0o755)
|
||||
launcher = source / "bin" / "lark-cli"
|
||||
launcher.parent.mkdir(parents=True)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user