fix(authz): recheck policy before sandbox reuse

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

backend/packages/harness/deerflow/sandbox/AGENTS.md

@@ -1,8 +1,8 @@
 ### Sandbox System (`packages/harness/deerflow/sandbox/`)
 
 **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 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.

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(),
                 )

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"(?<![:\w])(?<!:/)/(?:[^\s\"'`;&|<>()]+)")
 # 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
@@ -1395,6 +1408,15 @@ 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.
+    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
     # still-wrapped state before the context-based release branch, so this
@@ -1417,15 +1439,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 +1468,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.
+    await authorize_sandbox_execution_async(
+        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 +1493,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))
 
@@ -1510,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: