From 869fce019dc90eff8f2ab11acb9cf1dccc281333 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=97=9C=E9=B5=BC?= Date: Tue, 25 Aug 2026 10:04:02 +0800 Subject: [PATCH 1/5] fix(authz): recheck policy before sandbox reuse --- .../harness/deerflow/sandbox/AGENTS.md | 2 +- .../harness/deerflow/sandbox/tools.py | 31 +++++----- backend/tests/test_sandbox_authorization.py | 62 ++++++++++++++++++- 3 files changed, 76 insertions(+), 19 deletions(-) diff --git a/backend/packages/harness/deerflow/sandbox/AGENTS.md b/backend/packages/harness/deerflow/sandbox/AGENTS.md index 98eca0775bb..34fd602113d 100644 --- a/backend/packages/harness/deerflow/sandbox/AGENTS.md +++ b/backend/packages/harness/deerflow/sandbox/AGENTS.md @@ -2,7 +2,7 @@ **Interface**: Abstract `Sandbox` with `execute_command(command, env=None)`, `read_file`, `write_file`, `list_dir`, `glob`, and `grep`. `grep` accepts either one text file or a directory tree. The optional `env` injects per-call environment variables (request-scoped secrets — see Request-Scoped Secrets below); `LocalSandbox` merges it via `subprocess.run(env=...)` and `AioSandbox` routes env-bearing commands through the `bash.exec(env=...)` API on a fresh session. **Provider Pattern**: `SandboxProvider` with `acquire`, `acquire_async`, `get`, `release` lifecycle. Async agent/tool paths call async sandbox lifecycle hooks so Docker sandbox creation, discovery, cross-process locking, readiness polling, and release stay off the event loop. -**Authorization gate** (`sandbox:execute`, RFC #4063 Phase 3): every sandbox acquisition passes through `authorize_sandbox_execution` (`deerflow/authz/sandbox_authz.py`) - a binary `authorize(principal, "sandbox", "execute", target="*")` check before `provider.acquire`. The gate lives at the single acquisition entry point (`ensure_sandbox_initialized` / `_acquire_sandbox_async` in `tools.py`, and `SandboxMiddleware.before_agent` / `abefore_agent`), so it cannot be bypassed regardless of which sandbox-dependent tool triggers it; the reuse path (sandbox already in state) skips the re-check. Deny raises `SandboxAuthorizationError` (`sandbox/exceptions.py`), which propagates out of tool execution as a friendly error `ToolMessage` ("sandbox execution is not permitted for your role") - the eager path catches it and skips acquisition instead, deferring the deny to the first sandbox-touching tool call so both paths share the same semantics. Provider errors (both `authorize()` and provider resolution) follow `authorization.fail_closed` / `fail_open`; no readable `config.yaml` or `authorization.enabled: false` makes the gate a no-op (`safe_app_config` tolerates missing config). Gateway auxiliary sync paths (uploads/artifacts routers) call `try_acquire_sandbox_for_request` (`app/gateway/authz.py`), which gates via `authorize_sandbox_for_request` and skips the sync on deny - the upload/artifact edit itself still succeeds. Tests: `tests/test_sandbox_authorization.py`. +**Authorization gate** (`sandbox:execute`, RFC #4063 Phase 3): every sandbox-backed tool call passes through `authorize_sandbox_execution` (`deerflow/authz/sandbox_authz.py`) - a binary `authorize(principal, "sandbox", "execute", target="*")` check before either reusing a persisted sandbox id or calling `provider.acquire`. Rechecking reuse is required because authorization config and user roles can change while the sandbox remains cached. The gate lives at the single tool initialization entry point (`ensure_sandbox_initialized` / `ensure_sandbox_initialized_async` in `tools.py`), while `SandboxMiddleware.before_agent` / `abefore_agent` apply the same check to eager acquisition. Deny raises `SandboxAuthorizationError` (`sandbox/exceptions.py`), which propagates out of tool execution as a friendly error `ToolMessage` ("sandbox execution is not permitted for your role") - the eager path catches it and skips acquisition instead, deferring the deny to the first sandbox-touching tool call so both paths share the same semantics. Provider errors (both `authorize()` and provider resolution) follow `authorization.fail_closed` / `fail_open`; no readable `config.yaml` or `authorization.enabled: false` makes the gate a no-op (`safe_app_config` tolerates missing config). Gateway auxiliary sync paths (uploads/artifacts routers) call `try_acquire_sandbox_for_request` (`app/gateway/authz.py`), which gates via `authorize_sandbox_for_request` and skips the sync on deny - the upload/artifact edit itself still succeeds. Tests: `tests/test_sandbox_authorization.py`. **Environment policy** (`sandbox/env_policy.py`): `execute_command` no longer inherits the full `os.environ`. `build_sandbox_env()` scrubs secret-looking names (`*KEY*`/`*SECRET*`/`*TOKEN*`/`*PASS*`/`*CREDENTIAL*`) from the inherited environment before layering injected request secrets on top, so platform credentials (e.g. `OPENAI_API_KEY`) never leak into skill subprocesses. Benign vars (`PATH`, `HOME`, `LANG`, `VIRTUAL_ENV`, ...) are preserved. **Implementations**: - `LocalSandboxProvider` - Local filesystem execution. `acquire(thread_id)` returns a per-thread `LocalSandbox` (id `local:{thread_id}`) whose `path_mappings` resolve `/mnt/user-data/{workspace,uploads,outputs}` and `/mnt/acp-workspace` to that thread's host directories, so the public `Sandbox` API honours the `/mnt/user-data` contract uniformly with AIO. `acquire()` / `acquire(None)` keeps the legacy generic singleton (id `local`) for callers without a thread context. Per-thread sandboxes are held in an LRU cache (default 256 entries) guarded by a `threading.Lock`. Public, custom, legacy, and managed integration skill mappings point at stable enabled-only projection roots rather than raw skill directories. On Windows, Git Bash/MSYS argument-conversion exclusions are limited to safe non-root virtual path prefixes; do not restore a blanket conversion disable, because host-native CLI launchers need normal MSYS path conversion for their own installation paths. diff --git a/backend/packages/harness/deerflow/sandbox/tools.py b/backend/packages/harness/deerflow/sandbox/tools.py index f927b36bcd0..b1a96aca6cb 100644 --- a/backend/packages/harness/deerflow/sandbox/tools.py +++ b/backend/packages/harness/deerflow/sandbox/tools.py @@ -1395,6 +1395,14 @@ def ensure_sandbox_initialized(runtime: Runtime | None = None) -> Sandbox: if runtime.state is None: raise SandboxRuntimeError("Tool runtime state not available") + # Authorization is a live execution policy, not a lifetime property of a + # sandbox id. Re-check before both reuse and acquisition so a role or policy + # change takes effect on the next sandbox-backed tool call. + authorize_sandbox_execution( + context=runtime.context or {}, + app_config=safe_app_config(), + ) + # Check if sandbox already exists in state # Discarding fork_restored is safe: after_agent short-circuits on the # still-wrapped state before the context-based release branch, so this @@ -1417,15 +1425,6 @@ def ensure_sandbox_initialized(runtime: Runtime | None = None) -> Sandbox: if thread_id is None: raise SandboxRuntimeError("Thread ID not available in runtime context") - # Phase 3: enforce sandbox:execute authorization before acquiring. On deny - # a SandboxAuthorizationError propagates up through the tool so the agent's - # tool-error handling returns a friendly message (RFC §9). Skipped on the - # reuse path above (already authorized when first acquired). - authorize_sandbox_execution( - context=runtime.context or {}, - app_config=safe_app_config(), - ) - provider = get_sandbox_provider() sandbox_id = provider.acquire(thread_id, user_id=resolve_runtime_user_id(runtime)) @@ -1455,6 +1454,13 @@ async def ensure_sandbox_initialized_async(runtime: Runtime | None = None) -> Sa if runtime.state is None: raise SandboxRuntimeError("Tool runtime state not available") + # Keep the async path aligned with the sync path: persisted sandbox state + # must not bypass a newly-revoked sandbox:execute grant. + authorize_sandbox_execution( + context=runtime.context or {}, + app_config=safe_app_config(), + ) + # Same discard as the sync path above: the reuse path never releases, # because after_agent short-circuits on the still-wrapped state first. sandbox_state, _ = unwrap_sandbox(runtime.state.get("sandbox")) @@ -1473,13 +1479,6 @@ async def ensure_sandbox_initialized_async(runtime: Runtime | None = None) -> Sa if thread_id is None: raise SandboxRuntimeError("Thread ID not available in runtime context") - # Phase 3: enforce sandbox:execute authorization before acquiring (async - # counterpart of the sync gate in ``ensure_sandbox_initialized``). - authorize_sandbox_execution( - context=runtime.context or {}, - app_config=safe_app_config(), - ) - provider = get_sandbox_provider() sandbox_id = await provider.acquire_async(thread_id, user_id=resolve_runtime_user_id(runtime)) diff --git a/backend/tests/test_sandbox_authorization.py b/backend/tests/test_sandbox_authorization.py index 1a43e290c03..251d2c4cd10 100644 --- a/backend/tests/test_sandbox_authorization.py +++ b/backend/tests/test_sandbox_authorization.py @@ -1,7 +1,7 @@ """Phase 3 sandbox-level authorization tests. -Covers the ``authorize("sandbox", "execute")`` gate at the sandbox-acquisition -entry point. When denied, a :class:`SandboxAuthorizationError` propagates up +Covers the ``authorize("sandbox", "execute")`` gate at the sandbox-use entry +point. When denied, a :class:`SandboxAuthorizationError` propagates up through the tool so the agent's tool-error handling returns a friendly message (RFC §9), rather than crashing the run. @@ -238,6 +238,34 @@ def test_ensure_sandbox_initialized_denies_on_authz_reject(monkeypatch): sandbox_provider.acquire.assert_not_called() +def test_ensure_sandbox_initialized_rechecks_authz_for_reused_sandbox(monkeypatch): + """A persisted sandbox id must not outlive a revoked execute grant.""" + from deerflow.sandbox import tools as sandbox_tools + + provider = RbacAuthorizationProvider(roles={"user": {"sandbox": {"allow": []}}}) + app_config = _make_app_config() + _enable_authz(app_config) + monkeypatch.setattr( + "deerflow.authz.sandbox_authz.resolve_authorization_provider", + lambda config: provider, + ) + monkeypatch.setattr("deerflow.config.get_app_config", lambda: app_config) + + sandbox_provider = MagicMock() + sandbox_provider.get.return_value = MagicMock() + monkeypatch.setattr(sandbox_tools, "get_sandbox_provider", lambda: sandbox_provider) + runtime = SimpleNamespace( + state={"sandbox": {"sandbox_id": "sbx-existing"}}, + context={"thread_id": "t1", "user_id": "u1", "user_role": "user"}, + config=None, + ) + + with pytest.raises(SandboxAuthorizationError): + sandbox_tools.ensure_sandbox_initialized(runtime) + sandbox_provider.get.assert_not_called() + sandbox_provider.acquire.assert_not_called() + + def test_ensure_sandbox_initialized_allows_on_authz_permit(monkeypatch): """ensure_sandbox_initialized proceeds to acquire on allow.""" from deerflow.sandbox import tools as sandbox_tools @@ -662,6 +690,36 @@ def test_ensure_sandbox_initialized_async_denies_on_authz_reject(monkeypatch): sandbox_provider.acquire_async.assert_not_called() +def test_ensure_sandbox_initialized_async_rechecks_authz_for_reused_sandbox(monkeypatch): + """Async tool calls also re-check a revoked grant before sandbox reuse.""" + from deerflow.sandbox import tools as sandbox_tools + + provider = RbacAuthorizationProvider(roles={"user": {"sandbox": {"allow": []}}}) + app_config = _make_app_config() + _enable_authz(app_config) + monkeypatch.setattr( + "deerflow.authz.sandbox_authz.resolve_authorization_provider", + lambda config: provider, + ) + monkeypatch.setattr("deerflow.config.get_app_config", lambda: app_config) + + sandbox_provider = MagicMock() + sandbox_provider.get.return_value = MagicMock() + sandbox_provider.acquire_async = AsyncMock(return_value="sbx-new") + monkeypatch.setattr(sandbox_tools, "get_sandbox_provider", lambda: sandbox_provider) + runtime = SimpleNamespace( + state={"sandbox": {"sandbox_id": "sbx-existing"}}, + context={"thread_id": "t1", "user_id": "u1", "user_role": "user"}, + config=None, + ) + import asyncio + + with pytest.raises(SandboxAuthorizationError): + asyncio.run(sandbox_tools.ensure_sandbox_initialized_async(runtime)) + sandbox_provider.get.assert_not_called() + sandbox_provider.acquire_async.assert_not_called() + + def test_abefore_agent_deny_skips_acquisition(monkeypatch): """Async eager path deny: acquisition skipped, no run-level error. From dfbded77a4fbbfe0b2eb53998f4829f53b557300 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=97=9C=E9=B5=BC?= Date: Tue, 25 Aug 2026 15:42:21 +0800 Subject: [PATCH 2/5] fix(authz): avoid duplicate async sandbox checks --- .../harness/deerflow/authz/sandbox_authz.py | 86 ++++++++++++++----- .../harness/deerflow/sandbox/AGENTS.md | 2 +- .../harness/deerflow/sandbox/middleware.py | 8 +- .../harness/deerflow/sandbox/tools.py | 32 +++++-- backend/tests/test_sandbox_authorization.py | 42 +++++++++ 5 files changed, 137 insertions(+), 33 deletions(-) diff --git a/backend/packages/harness/deerflow/authz/sandbox_authz.py b/backend/packages/harness/deerflow/authz/sandbox_authz.py index 1e4063caa5a..052879b1636 100644 --- a/backend/packages/harness/deerflow/authz/sandbox_authz.py +++ b/backend/packages/harness/deerflow/authz/sandbox_authz.py @@ -1,7 +1,7 @@ """Sandbox execution authorization gate. -Checks ``authorize("sandbox", "execute")`` before sandbox acquisition so a -role-scoped policy can deny sandbox execution entirely. On deny, a +Checks ``authorize("sandbox", "execute")`` before sandbox use so a role-scoped +policy can deny sandbox execution entirely. On deny, a :class:`~deerflow.sandbox.exceptions.SandboxAuthorizationError` propagates up through the tool's execution; the agent's tool-error handling converts it to a friendly ``ToolMessage`` ("sandbox not permitted for your role") rather than @@ -19,7 +19,7 @@ from typing import Any from deerflow.authz.principal import build_principal_from_context -from deerflow.authz.provider import AuthzDecision, AuthzRequest +from deerflow.authz.provider import AuthorizationProvider, AuthzDecision, AuthzRequest, Principal from deerflow.authz.runtime import resolve_authorization_provider from deerflow.config.app_config import AppConfig from deerflow.sandbox.exceptions import SandboxAuthorizationError @@ -52,21 +52,19 @@ def safe_app_config() -> AppConfig | None: _SANDBOX_TARGET = "*" -def authorize_sandbox_execution(*, context: Mapping[str, Any], app_config: AppConfig | None) -> None: - """Check ``authorize("sandbox", "execute")`` before sandbox acquisition. - - ``app_config=None`` (unreadable config) is treated the same as - ``authorization.enabled: false`` — a no-op. On deny (or provider error - with ``fail_closed``), raises :class:`SandboxAuthorizationError`; on - provider error with fail-open, returns silently (legacy allow behavior). - """ +def _resolve_authorization_inputs( + *, + context: Mapping[str, Any], + app_config: AppConfig | None, +) -> tuple[AuthorizationProvider, Any, Principal] | None: + """Resolve the enabled provider and principal shared by both call paths.""" # Guard against Mock/SimpleNamespace app_config objects in tests that # don't carry a real AuthorizationConfig. getattr avoids AttributeError # and the ``is not True`` identity check avoids truthy Mock attributes # (mirrors filter_available_skills_by_authorization in skill_filter.py). authz_config = getattr(app_config, "authorization", None) if authz_config is None or getattr(authz_config, "enabled", None) is not True: - return + return None # Provider *resolution* failures follow the same fail_closed/fail_open # decision as authorize() errors — a raw ValueError here would otherwise @@ -77,25 +75,67 @@ def authorize_sandbox_execution(*, context: Mapping[str, Any], app_config: AppCo logger.warning("Failed to resolve authorization provider for sandbox:execute", exc_info=True) if authz_config.fail_closed: raise SandboxAuthorizationError() from None - # fail-open: allow sandbox acquisition despite the resolution error. - return + # fail-open: allow sandbox use despite the resolution error. + return None if provider is None: - return + return None principal = build_principal_from_context(context, default_role=authz_config.default_role) - try: - decision = provider.authorize(AuthzRequest(principal=principal, resource="sandbox", action="execute", target=_SANDBOX_TARGET)) - if not isinstance(decision, AuthzDecision): - raise TypeError("AuthorizationProvider.authorize must return AuthzDecision") - if decision.allow: - return - # Explicit deny → block sandbox acquisition with a friendly error. + return provider, authz_config, principal + + +def _authorization_request(principal: Principal) -> AuthzRequest: + return AuthzRequest(principal=principal, resource="sandbox", action="execute", target=_SANDBOX_TARGET) + + +def _enforce_decision(decision: AuthzDecision, *, principal: Principal, method_name: str) -> None: + if not isinstance(decision, AuthzDecision): + raise TypeError(f"AuthorizationProvider.{method_name} must return AuthzDecision") + if not decision.allow: raise SandboxAuthorizationError(role=principal.role) + + +def authorize_sandbox_execution(*, context: Mapping[str, Any], app_config: AppConfig | None) -> None: + """Synchronously check ``authorize("sandbox", "execute")`` before use. + + ``app_config=None`` (unreadable config) is treated the same as + ``authorization.enabled: false`` — a no-op. On deny (or provider error + with ``fail_closed``), raises :class:`SandboxAuthorizationError`; on + provider error with fail-open, returns silently (legacy allow behavior). + """ + inputs = _resolve_authorization_inputs(context=context, app_config=app_config) + if inputs is None: + return + provider, authz_config, principal = inputs + + try: + decision = provider.authorize(_authorization_request(principal)) + _enforce_decision(decision, principal=principal, method_name="authorize") + except SandboxAuthorizationError: + raise + except Exception: + logger.warning("Authorization provider failed while checking sandbox:execute", exc_info=True) + if authz_config.fail_closed: + raise SandboxAuthorizationError(role=principal.role) + # fail-open: allow sandbox use despite the provider error. + return + + +async def authorize_sandbox_execution_async(*, context: Mapping[str, Any], app_config: AppConfig | None) -> None: + """Asynchronously check ``authorize("sandbox", "execute")`` before use.""" + inputs = _resolve_authorization_inputs(context=context, app_config=app_config) + if inputs is None: + return + provider, authz_config, principal = inputs + + try: + decision = await provider.aauthorize(_authorization_request(principal)) + _enforce_decision(decision, principal=principal, method_name="aauthorize") except SandboxAuthorizationError: raise except Exception: logger.warning("Authorization provider failed while checking sandbox:execute", exc_info=True) if authz_config.fail_closed: raise SandboxAuthorizationError(role=principal.role) - # fail-open: allow sandbox acquisition despite the provider error. + # fail-open: allow sandbox use despite the provider error. return diff --git a/backend/packages/harness/deerflow/sandbox/AGENTS.md b/backend/packages/harness/deerflow/sandbox/AGENTS.md index 34fd602113d..ad2609e42a6 100644 --- a/backend/packages/harness/deerflow/sandbox/AGENTS.md +++ b/backend/packages/harness/deerflow/sandbox/AGENTS.md @@ -2,7 +2,7 @@ **Interface**: Abstract `Sandbox` with `execute_command(command, env=None)`, `read_file`, `write_file`, `list_dir`, `glob`, and `grep`. `grep` accepts either one text file or a directory tree. The optional `env` injects per-call environment variables (request-scoped secrets — see Request-Scoped Secrets below); `LocalSandbox` merges it via `subprocess.run(env=...)` and `AioSandbox` routes env-bearing commands through the `bash.exec(env=...)` API on a fresh session. **Provider Pattern**: `SandboxProvider` with `acquire`, `acquire_async`, `get`, `release` lifecycle. Async agent/tool paths call async sandbox lifecycle hooks so Docker sandbox creation, discovery, cross-process locking, readiness polling, and release stay off the event loop. -**Authorization gate** (`sandbox:execute`, RFC #4063 Phase 3): every sandbox-backed tool call passes through `authorize_sandbox_execution` (`deerflow/authz/sandbox_authz.py`) - a binary `authorize(principal, "sandbox", "execute", target="*")` check before either reusing a persisted sandbox id or calling `provider.acquire`. Rechecking reuse is required because authorization config and user roles can change while the sandbox remains cached. The gate lives at the single tool initialization entry point (`ensure_sandbox_initialized` / `ensure_sandbox_initialized_async` in `tools.py`), while `SandboxMiddleware.before_agent` / `abefore_agent` apply the same check to eager acquisition. Deny raises `SandboxAuthorizationError` (`sandbox/exceptions.py`), which propagates out of tool execution as a friendly error `ToolMessage` ("sandbox execution is not permitted for your role") - the eager path catches it and skips acquisition instead, deferring the deny to the first sandbox-touching tool call so both paths share the same semantics. Provider errors (both `authorize()` and provider resolution) follow `authorization.fail_closed` / `fail_open`; no readable `config.yaml` or `authorization.enabled: false` makes the gate a no-op (`safe_app_config` tolerates missing config). Gateway auxiliary sync paths (uploads/artifacts routers) call `try_acquire_sandbox_for_request` (`app/gateway/authz.py`), which gates via `authorize_sandbox_for_request` and skips the sync on deny - the upload/artifact edit itself still succeeds. Tests: `tests/test_sandbox_authorization.py`. +**Authorization gate** (`sandbox:execute`, RFC #4063 Phase 3): every sandbox-backed tool call passes through the gate in `deerflow/authz/sandbox_authz.py` - a binary `authorize(principal, "sandbox", "execute", target="*")` check before either reusing a persisted sandbox id or calling `provider.acquire`. Rechecking reuse is required because authorization config and user roles can change while the sandbox remains cached. Sync tool invocations call `authorize_sandbox_execution`; async tool invocations await `authorize_sandbox_execution_async` exactly once, then use a task-local `ContextVar` handoff so the synchronous body dispatched through `asyncio.to_thread` does not authorize a second time. The gate lives at the single tool initialization entry point (`ensure_sandbox_initialized` / `ensure_sandbox_initialized_async` in `tools.py`), while `SandboxMiddleware.before_agent` / `abefore_agent` apply the matching sync/async check to eager acquisition. Deny raises `SandboxAuthorizationError` (`sandbox/exceptions.py`), which propagates out of tool execution as a friendly error `ToolMessage` ("sandbox execution is not permitted for your role") - the eager path catches it and skips acquisition instead, deferring the deny to the first sandbox-touching tool call so both paths share the same semantics. Provider errors (authorization calls and provider resolution) follow `authorization.fail_closed` / `fail_open`; no readable `config.yaml` or `authorization.enabled: false` makes the gate a no-op (`safe_app_config` tolerates missing config). Gateway auxiliary sync paths (uploads/artifacts routers) call `try_acquire_sandbox_for_request` (`app/gateway/authz.py`), which gates via `authorize_sandbox_for_request` and skips the sync on deny - the upload/artifact edit itself still succeeds. Tests: `tests/test_sandbox_authorization.py`. **Environment policy** (`sandbox/env_policy.py`): `execute_command` no longer inherits the full `os.environ`. `build_sandbox_env()` scrubs secret-looking names (`*KEY*`/`*SECRET*`/`*TOKEN*`/`*PASS*`/`*CREDENTIAL*`) from the inherited environment before layering injected request secrets on top, so platform credentials (e.g. `OPENAI_API_KEY`) never leak into skill subprocesses. Benign vars (`PATH`, `HOME`, `LANG`, `VIRTUAL_ENV`, ...) are preserved. **Implementations**: - `LocalSandboxProvider` - Local filesystem execution. `acquire(thread_id)` returns a per-thread `LocalSandbox` (id `local:{thread_id}`) whose `path_mappings` resolve `/mnt/user-data/{workspace,uploads,outputs}` and `/mnt/acp-workspace` to that thread's host directories, so the public `Sandbox` API honours the `/mnt/user-data` contract uniformly with AIO. `acquire()` / `acquire(None)` keeps the legacy generic singleton (id `local`) for callers without a thread context. Per-thread sandboxes are held in an LRU cache (default 256 entries) guarded by a `threading.Lock`. Public, custom, legacy, and managed integration skill mappings point at stable enabled-only projection roots rather than raw skill directories. On Windows, Git Bash/MSYS argument-conversion exclusions are limited to safe non-root virtual path prefixes; do not restore a blanket conversion disable, because host-native CLI launchers need normal MSYS path conversion for their own installation paths. diff --git a/backend/packages/harness/deerflow/sandbox/middleware.py b/backend/packages/harness/deerflow/sandbox/middleware.py index c0d98f560f4..3b9ed82569d 100644 --- a/backend/packages/harness/deerflow/sandbox/middleware.py +++ b/backend/packages/harness/deerflow/sandbox/middleware.py @@ -12,7 +12,11 @@ from langgraph.types import Command from deerflow.agents.thread_state import SandboxStateField, ThreadDataState -from deerflow.authz.sandbox_authz import authorize_sandbox_execution, safe_app_config +from deerflow.authz.sandbox_authz import ( + authorize_sandbox_execution, + authorize_sandbox_execution_async, + safe_app_config, +) from deerflow.runtime.user_context import resolve_runtime_user_id from deerflow.sandbox import get_sandbox_provider from deerflow.sandbox.exceptions import SandboxAuthorizationError @@ -116,7 +120,7 @@ async def abefore_agent(self, state: SandboxMiddlewareState, runtime: Runtime) - # ``ensure_sandbox_initialized`` denies per-tool with the RFC §9 # friendly message on the first sandbox-touching tool call. try: - authorize_sandbox_execution( + await authorize_sandbox_execution_async( context=runtime.context or {}, app_config=safe_app_config(), ) diff --git a/backend/packages/harness/deerflow/sandbox/tools.py b/backend/packages/harness/deerflow/sandbox/tools.py index b1a96aca6cb..fde8ec67400 100644 --- a/backend/packages/harness/deerflow/sandbox/tools.py +++ b/backend/packages/harness/deerflow/sandbox/tools.py @@ -6,13 +6,18 @@ import re import shlex from collections.abc import Callable +from contextvars import ContextVar from functools import lru_cache from pathlib import Path from langchain.tools import tool from deerflow.agents.thread_state import ThreadDataState -from deerflow.authz.sandbox_authz import authorize_sandbox_execution, safe_app_config +from deerflow.authz.sandbox_authz import ( + authorize_sandbox_execution, + authorize_sandbox_execution_async, + safe_app_config, +) from deerflow.config import get_app_config from deerflow.config.paths import VIRTUAL_PATH_PREFIX from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH @@ -34,6 +39,14 @@ logger = logging.getLogger(__name__) +# Async tool wrappers authorize before dispatching their synchronous body via +# ``asyncio.to_thread``. Context variables are copied into that worker, making +# this a task-local, one-invocation handoff rather than shared runtime state. +_ASYNC_SANDBOX_AUTHORIZATION_CHECKED: ContextVar[bool] = ContextVar( + "deerflow_async_sandbox_authorization_checked", + default=False, +) + _ABSOLUTE_PATH_PATTERN = re.compile(r"(?()]+)") # A ``{...}`` block holding a single identifier-like placeholder (e.g. ``{id}`` # in a REST template or ``{port}`` in an f-string). Bash brace expansion such as @@ -1398,10 +1411,11 @@ def ensure_sandbox_initialized(runtime: Runtime | None = None) -> Sandbox: # Authorization is a live execution policy, not a lifetime property of a # sandbox id. Re-check before both reuse and acquisition so a role or policy # change takes effect on the next sandbox-backed tool call. - authorize_sandbox_execution( - context=runtime.context or {}, - app_config=safe_app_config(), - ) + if not _ASYNC_SANDBOX_AUTHORIZATION_CHECKED.get(): + authorize_sandbox_execution( + context=runtime.context or {}, + app_config=safe_app_config(), + ) # Check if sandbox already exists in state # Discarding fork_restored is safe: after_agent short-circuits on the @@ -1456,7 +1470,7 @@ async def ensure_sandbox_initialized_async(runtime: Runtime | None = None) -> Sa # Keep the async path aligned with the sync path: persisted sandbox state # must not bypass a newly-revoked sandbox:execute grant. - authorize_sandbox_execution( + await authorize_sandbox_execution_async( context=runtime.context or {}, app_config=safe_app_config(), ) @@ -1509,7 +1523,11 @@ async def _run_sync_tool_after_async_sandbox_init( if func is None: return "Error: Tool implementation not available" - return await asyncio.to_thread(func, runtime, *args) + token = _ASYNC_SANDBOX_AUTHORIZATION_CHECKED.set(True) + try: + return await asyncio.to_thread(func, runtime, *args) + finally: + _ASYNC_SANDBOX_AUTHORIZATION_CHECKED.reset(token) def ensure_thread_directories_exist(runtime: Runtime | None) -> None: diff --git a/backend/tests/test_sandbox_authorization.py b/backend/tests/test_sandbox_authorization.py index 251d2c4cd10..b9f8269d729 100644 --- a/backend/tests/test_sandbox_authorization.py +++ b/backend/tests/test_sandbox_authorization.py @@ -720,6 +720,48 @@ def test_ensure_sandbox_initialized_async_rechecks_authz_for_reused_sandbox(monk sandbox_provider.acquire_async.assert_not_called() +def test_async_sandbox_tool_authorizes_once_via_async_provider(monkeypatch): + """One async tool invocation must make one async authorization decision.""" + from deerflow.sandbox import tools as sandbox_tools + + provider = MagicMock() + provider.authorize.return_value = AuthzDecision(allow=True) + provider.aauthorize = AsyncMock(return_value=AuthzDecision(allow=True)) + app_config = _make_app_config() + _enable_authz(app_config) + monkeypatch.setattr( + "deerflow.authz.sandbox_authz.resolve_authorization_provider", + lambda config: provider, + ) + monkeypatch.setattr("deerflow.config.get_app_config", lambda: app_config) + + sandbox = MagicMock() + sandbox.list_dir.return_value = [] + sandbox_provider = MagicMock() + sandbox_provider.get.return_value = sandbox + monkeypatch.setattr(sandbox_tools, "get_sandbox_provider", lambda: sandbox_provider) + monkeypatch.setattr(sandbox_tools, "is_local_sandbox", lambda runtime: False) + runtime = SimpleNamespace( + state={"sandbox": {"sandbox_id": "sbx-existing"}}, + context={"thread_id": "t1", "user_id": "u1", "user_role": "user"}, + config=None, + ) + + import asyncio + + result = asyncio.run( + sandbox_tools.ls_tool.coroutine( + runtime=runtime, + description="list workspace", + path="/mnt/user-data/workspace", + ) + ) + + assert result == "(empty)" + provider.aauthorize.assert_awaited_once() + provider.authorize.assert_not_called() + + def test_abefore_agent_deny_skips_acquisition(monkeypatch): """Async eager path deny: acquisition skipped, no run-level error. From d7687559d04b575005ec3f13f915041e1bd33719 Mon Sep 17 00:00:00 2001 From: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:09:28 +0800 Subject: [PATCH 3/5] fix(authz): scope sandbox decision across middleware --- .../deerflow/agents/middlewares/AGENTS.md | 2 +- .../read_before_write_middleware.py | 101 +++++++--- .../harness/deerflow/authz/sandbox_authz.py | 20 +- .../harness/deerflow/sandbox/AGENTS.md | 2 +- .../harness/deerflow/sandbox/middleware.py | 3 +- .../harness/deerflow/sandbox/tools.py | 79 +++++--- .../blocking_io/test_sandbox_authorization.py | 59 ++++++ backend/tests/test_sandbox_authorization.py | 178 ++++++++++++++++++ ...able-authorization-implementation-notes.md | 23 +++ 9 files changed, 410 insertions(+), 57 deletions(-) create mode 100644 backend/tests/blocking_io/test_sandbox_authorization.py diff --git a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md index 6870deef3f9..751e16683dc 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md +++ b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md @@ -59,7 +59,7 @@ it to that middleware's declaration in the same change. forgeries). Consumers pop it; the publisher and the consumer share only that contract module. 10. **SandboxAuditMiddleware** - Audits sandboxed shell/file operations for security logging before tool execution. Command classification is **defense-in-depth and audit, not a security boundary** — the sandbox itself is the isolation boundary. Command substitution is judged by *position*, not by the presence of `$(`: a substitution in **command position** (`$(curl url)`, `` `curl url` ``, the word after a `|`/`&&`/`;`, or any `eval`/`source` argument) executes fetched or interpreted content and is blocked, while **value position** (`x=$(curl url)`, `echo $(curl url)`, an argument, a `for` word list) only captures output and passes (#4611). `_HIGH_RISK_COMMAND_POSITION_PATTERNS` is therefore matched anchored against each split sub-command, never against the whole compound string, and `_split_compound_command(split_pipes=True)` supplies those sub-commands; rules that span a pipe (`| sh`, `base64 -d | ...`) still rely on `_classify_command`'s whole-command Pass 1. `_COMMAND_POSITION_PREFIX` extends the anchor over leading variable assignments and exec wrappers (`FOO=1 $(curl url)`, `env`/`command`/`builtin`/`exec`/`nohup`/`time`/`sudo`/`doas`), which are still command position; its assignment branch requires whitespace before the substitution, which is exactly what keeps `x=$(curl url)` in value position. Two execution contexts are deliberately **position-blind** and matched against the whole command in Pass 1, because they execute what they receive wherever they appear (including as an argument to something else, e.g. `xargs sh -c "$(curl url)"`): an `eval`/`source` argument, and an interpreter's **code-string flag** — `-c` (shells, `python`), `-e` (`perl`/`ruby`/`node`), `-p` (`perl`/`node`), `-r` (`php`) — plus the here-string (`<<<`) that reaches the same place through stdin. All three substitution spellings (`$(cmd`, `<(cmd`, `` `cmd ``) share one `_RISKY_SUBSTITUTION` opener so a rule cannot cover one spelling and miss another. An unquoted newline splits like `;`, because it separates statements the same way: leaving it joined let `echo hi\n$(curl url)` evade the anchored rules that its `;` spelling triggers. A heredoc body is data rather than statements, so `_split_compound_command` records headers (`<