diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 9c437bbdf..fa5b5c040 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -400,6 +400,7 @@ Lets a caller pass per-request, short-lived end-user credentials (e.g. an ERP to - **Carry**: the caller sends values out-of-band in the run request's `context.secrets` mapping (never a message). `runtime/secret_context.py` owns the contract (`SECRETS_CONTEXT_KEY`, `extract_request_secrets`). The existing `context` passthrough carries it to `runtime.context` without mirroring into `configurable`. `build_run_config` still sets `configurable.thread_id` on the context path — the checkpointer requires it. - **Bind (point A)**: on slash-activation, `SkillActivationMiddleware._apply_skill_secrets` resolves the activated skill's declared secrets against `context.secrets` and writes the per-run injection set to `runtime.context[__active_skill_secrets]`. Slash activation reads the genuine user text via `get_original_user_content_text`; `InputSanitizationMiddleware` preserves it (`ORIGINAL_USER_CONTENT_KEY`) when it wraps input in BEGIN/END markers, so activation fires even after sanitization. The injected value always comes from the caller's request, never the host environment (scrubbed first — see below), so a declared name that also exists in the host env is safe: the caller's value wins and the host value is dropped (the #3861 per-user-key-overrides-shared-key case). Missing required secrets are logged, not injected. - **Inject**: `bash_tool` reads the injection set and passes it as `execute_command(env=...)`. Scope is the activation turn/run only — a run without `/skill` activation injects nothing. +- **AIO image requirement**: on `AioSandbox` the env path uses the `bash.exec` API (`POST /v1/bash/exec`), which upstream all-in-one-sandbox only ships since `1.9.3` — older images (including a `latest` tag frozen on the `1.0.0.x` line) 404 the whole `/v1/bash/*` namespace. `AioSandbox` detects the 404, remembers the capability gap on the instance, and fails fast with an actionable upgrade error instead of letting the model retry raw 404s; there is deliberately **no** fallback through the legacy shell path because none keeps the secret values out of the command string (#3921). Regression tests: `tests/test_aio_sandbox.py::TestBashExecUnsupportedFailFast`. - **Inherited-env scrub**: `execute_command` no longer leaks the Gateway's `os.environ` to skill subprocesses — `env_policy.build_sandbox_env` drops secret-looking names (`*KEY*`/`*SECRET*`/`*TOKEN*`/`*PASSWORD*`/`*CREDENTIAL*`/`*DSN*` + a connection-string denylist like `DATABASE_URL`/`REDIS_URL`/`GH_PAT`) so platform credentials never reach a skill; a skill that needs one must declare it. - **Leak surfaces sealed** (verified by a real-gateway e2e run — secret reaches the sandbox but none of these): prompt (value never in a message), trace (`tracing/metadata.py` never copies `context`), checkpoint (secrets live on `runtime.context`, not graph state), audit (journal records names only), stdout (`tools.py::mask_secret_values` redacts injected values from bash output), and **run-record persistence + run API** (`services.py::start_run` stores `redact_config_secrets(body.config)` so `runs.kwargs_json` and `RunResponse.kwargs` never carry the secret). - **Scope / non-goals**: only `/slash`-activated skills receive secrets (autonomously invoked enabled skills do not); no persistence/vaulting; the MCP per-user-credential gap (#3322) is a sibling, not covered here. Tests: `tests/test_skill_request_scoped_secrets.py`. diff --git a/backend/docs/CONFIGURATION.md b/backend/docs/CONFIGURATION.md index c9ded6a71..b89ea6920 100644 --- a/backend/docs/CONFIGURATION.md +++ b/backend/docs/CONFIGURATION.md @@ -455,6 +455,7 @@ If you rebuild the runtime from scratch instead of extending the published image - `sandbox.get_context()`, including `home_dir` - `shell.exec_command(...)` +- `bash.exec(...)` — only exercised for per-command environment injection (skills that declare `required-secrets`). The `/v1/bash/*` routes exist since upstream all-in-one-sandbox `1.9.3`; on older images (including a `latest` tag still frozen on the `1.0.0.x` line) DeerFlow fails fast with an actionable error instead of surfacing the raw 404. Pin `sandbox.image` to `1.9.3` or newer (e.g. `1.11.0`) and recreate the sandbox container to use `required-secrets` with the AIO sandbox. - `file.read_file(...)` - `file.write_file(...)`, including base64 writes for binary content - streamed `file.download_file(...)` diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py index d7ce406b4..43737752e 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox.py @@ -6,6 +6,7 @@ import threading import uuid from agent_sandbox import Sandbox as AioSandboxClient +from agent_sandbox.core.api_error import ApiError from deerflow.config.paths import VIRTUAL_PATH_PREFIX from deerflow.sandbox.sandbox import Sandbox @@ -17,6 +18,19 @@ _MAX_DOWNLOAD_SIZE = 100 * 1024 * 1024 # 100 MB _ERROR_OBSERVATION_SIGNATURE = "'ErrorObservation' object has no attribute 'exit_code'" +# Env-bearing commands require the bash.exec API (POST /v1/bash/exec), which the +# all-in-one-sandbox image only ships since 1.9.x. Older images (including any +# ``latest`` tag frozen on the 1.0.0.x line) answer 404 for the whole /v1/bash/* +# namespace. That raw 404 is useless to the model (it just retries), so the +# sandbox fails fast with this operator-facing message instead (#3921). +_BASH_EXEC_UNSUPPORTED_ERROR = ( + "Error: this sandbox image does not support per-command environment injection " + "(POST /v1/bash/exec returned 404), which is required to run skills that declare " + "required-secrets. This is a deployment issue that retrying cannot fix: upgrade the " + "sandbox image to all-in-one-sandbox >= 1.9.3 (set `sandbox.image` in config.yaml, " + "e.g. pin the tag `1.11.0`) and recreate the sandbox container, then try again." +) + class AioSandbox(Sandbox): """Sandbox implementation using the agent-infra/sandbox Docker container. @@ -40,6 +54,9 @@ class AioSandbox(Sandbox): self._home_dir = home_dir self._lock = threading.Lock() self._closed = False + # Set to True after bash.exec answers 404 (image predates /v1/bash/*), + # so later env-bearing calls fail fast instead of re-hitting HTTP (#3921). + self._bash_exec_unsupported = False @property def base_url(self) -> str: @@ -204,7 +221,14 @@ class AioSandbox(Sandbox): legacy persistent-shell path: if the (unlikely, since each call is a fresh session) corruption marker shows up, the call is retried on another fresh session rather than returned verbatim. + + Images older than all-in-one-sandbox 1.9.x have no ``/v1/bash/*`` routes; + there is no fallback on the legacy shell path that would keep the secret + values out of the command string, so the only safe behaviour is to fail + fast with an actionable error (#3921). """ + if self._bash_exec_unsupported: + return _BASH_EXEC_UNSUPPORTED_ERROR output = self._run_bash_exec(command, env) if output and _ERROR_OBSERVATION_SIGNATURE in output: logger.warning("ErrorObservation detected in bash.exec output, retrying on a fresh session") @@ -229,6 +253,13 @@ class AioSandbox(Sandbox): if stderr: output += f"\nStd Error:\n{stderr}" if output else stderr return output if output else "(no output)" + except ApiError as e: + if e.status_code == 404: + self._bash_exec_unsupported = True + logger.error("Sandbox %s does not support bash.exec (/v1/bash/exec returned 404); env-bearing commands are unavailable until the sandbox image is upgraded to all-in-one-sandbox >= 1.9.3", self.id) + return _BASH_EXEC_UNSUPPORTED_ERROR + logger.error(f"Failed to execute command with injected env in sandbox: {e}") + return f"Error: {e}" except Exception as e: logger.error(f"Failed to execute command with injected env in sandbox: {e}") return f"Error: {e}" diff --git a/backend/tests/test_aio_sandbox.py b/backend/tests/test_aio_sandbox.py index 37b0ade06..9102c23b8 100644 --- a/backend/tests/test_aio_sandbox.py +++ b/backend/tests/test_aio_sandbox.py @@ -161,6 +161,90 @@ class TestErrorObservationRetry: assert call_count == 1 +class TestBashExecUnsupportedFailFast: + """Regression tests for #3921: sandbox images older than all-in-one-sandbox + 1.9.x have no ``/v1/bash/exec`` route, so every env-bearing command (skills + declaring ``required-secrets``) hit a bare nginx 404 that the model kept + retrying. The sandbox must fail fast with an actionable, operator-facing + error instead.""" + + def _api_error_404(self): + from agent_sandbox.core.api_error import ApiError + + return ApiError( + headers={"server": "nginx/1.18.0 (Ubuntu)"}, + status_code=404, + body={"success": False, "message": "Not Found", "data": None}, + ) + + def test_bash_exec_404_returns_actionable_error(self, sandbox): + """A 404 from bash.exec must explain the image capability gap and the + remediation (upgrade image), not surface the raw nginx error.""" + sandbox._client.bash.exec = MagicMock(side_effect=self._api_error_404()) + + out = sandbox.execute_command("echo $TOK", env={"TOK": "secret-v"}) + + assert out.startswith("Error:") + # Actionable: names the missing capability and the minimum image version. + assert "/v1/bash/exec" in out + assert "1.9.3" in out + assert "required-secrets" in out + # Not the raw upstream 404 body the model can't act on. + assert "nginx" not in out + + def test_bash_exec_404_is_cached_and_stops_retry_storm(self, sandbox): + """After one 404 the capability gap is remembered on the instance: + follow-up env-bearing calls return the same actionable error without + another HTTP round-trip (the original bug produced 4 consecutive 404s + as the model retried variants of the command).""" + sandbox._client.bash.exec = MagicMock(side_effect=self._api_error_404()) + + first = sandbox.execute_command("cmd-1", env={"TOK": "v"}) + second = sandbox.execute_command("cmd-2", env={"TOK": "v"}) + + assert sandbox._client.bash.exec.call_count == 1 + assert first == second + assert "1.9.3" in second + + def test_bash_exec_non_404_error_is_not_cached(self, sandbox): + """Transient failures (e.g. 500) must not permanently disable the env + path — the next env-bearing call should try bash.exec again.""" + from agent_sandbox.core.api_error import ApiError + + sandbox._client.bash.exec = MagicMock(side_effect=ApiError(status_code=500, body="boom")) + + first = sandbox.execute_command("cmd-1", env={"TOK": "v"}) + second = sandbox.execute_command("cmd-2", env={"TOK": "v"}) + + assert sandbox._client.bash.exec.call_count == 2 + assert first.startswith("Error:") + assert "1.9.3" not in first + assert second.startswith("Error:") + + def test_env_less_path_unaffected_after_404(self, sandbox): + """The legacy persistent-shell path must keep working on an image + without bash.exec — only env injection is unavailable there.""" + sandbox._client.bash.exec = MagicMock(side_effect=self._api_error_404()) + sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="plain ok"))) + + sandbox.execute_command("cmd", env={"TOK": "v"}) + out = sandbox.execute_command("echo plain") + + assert out == "plain ok" + sandbox._client.shell.exec_command.assert_called_once() + + def test_bash_exec_success_does_not_mark_unsupported(self, sandbox): + """A healthy bash.exec keeps the env path fully enabled.""" + sandbox._client.bash.exec = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(stdout="ok", stderr=None))) + + first = sandbox.execute_command("cmd-1", env={"TOK": "v"}) + second = sandbox.execute_command("cmd-2", env={"TOK": "v"}) + + assert first == "ok" + assert second == "ok" + assert sandbox._client.bash.exec.call_count == 2 + + class TestListDirSerialization: """Verify that list_dir also acquires the lock."""