mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 21:26:39 +00:00
* feat(sandbox): per-call env injection + platform-secret scrubbing for skills Add an env parameter to Sandbox.execute_command (abstract + local + AIO) so request-scoped secrets can be injected into skill subprocesses, and scrub platform credentials (*KEY*/*SECRET*/*TOKEN*/*PASSWORD*/*CREDENTIAL*) from the inherited environment by default so scoped injection is not security theatre. LocalSandbox always passes an explicit scrubbed env; AioSandbox routes env-bearing commands through bash.exec(env=) on a fresh session and leaves the legacy persistent-shell path unchanged. Part of #3861. BEHAVIOR CHANGE: execute_command no longer inherits the full os.environ; Windows encoding tests updated to assert the scrubbed dict. * feat(skills): parse required-secrets frontmatter declaration Add SecretRequirement and Skill.required_secrets, and parse the required-secrets SKILL.md frontmatter field (a string list or {name, optional} mappings), dropping malformed entries with a warning so one bad declaration does not invalidate the skill. The declared name is both the context.secrets key and the env var injected at activation. Part of #3861. * feat(runtime): request-scoped secret carrier (context.secrets) Add SECRETS_CONTEXT_KEY + extract_request_secrets, centralising the context.secrets carrier contract. The existing context passthrough (build_run_config -> _build_runtime_context) already carries the sub-key to runtime.context without mirroring it into configurable; characterization tests lock that behaviour. Part of #3861. * feat(skills): inject declared secrets at slash-activation into bash env Binding point A: when a skill is slash-activated, SkillActivationMiddleware resolves its declared required-secrets against the request's context.secrets and writes the per-run injection set to runtime.context. The bash tool forwards that set to execute_command(env=). A skill cannot harvest a host platform credential (is_host_platform_secret guard, cf. GHSA-rhgp-j443-p4rf), and injected values are redacted from bash output (mask_secret_values) so an echoed secret never re-enters the prompt/trace. Part of #3861. * test(skills): lock the five secret leak surfaces + add trace redaction helper Regression tests assert the secret value is absent from all five surfaces: prompt (activation message), checkpoint (graph state vs context separation), audit (journal records names only), trace (metadata builder never copies context; never mirrored to configurable), and stdout (mask_secret_values). Add redact_secret_context_keys as a defensive helper for any context serialization. Part of #3861. * docs(backend): document request-scoped secrets for skills Add Request-Scoped Secrets subsection (Skills) + env policy note (Sandbox) and the execute_command(env=) signature change, per the doc-sync policy. Part of #3861. * fix(skills): close gaps found by end-to-end verification of request-scoped secrets Real-gateway e2e + independent review of #3861 surfaced three defects, now fixed: 1. Slash activation never fired in the live chain. InputSanitizationMiddleware wraps user input in BEGIN/END markers before SkillActivationMiddleware sees it, and the original text was only preserved when an upload or IM channel set it. For a plain text message the slash command became undetectable, so no secret was ever resolved. Fix: the sanitizer now setdefaults the pre-wrap text into ORIGINAL_USER_CONTENT_KEY (additive; sanitization behaviour unchanged), so slash activation works for all messages. Pre-existing latent bug surfaced here. 2. The raw request config (with context.secrets) was persisted to runs.kwargs_json and echoed by the run API (RunResponse.kwargs). Fix: redact_config_secrets() strips secret-bearing context keys from the persisted/echoed copy in start_run; the live config that drives the run keeps them. build_run_config now also sets configurable.thread_id on the context path (the checkpointer requires it). 3. Connection-string credentials (DATABASE_URL, REDIS_URL, SENTRY_DSN, GH_PAT, ...) were not scrubbed from the inherited sandbox env. Fix: env_policy adds a *DSN* pattern plus an explicit connection-string denylist (no blanket *URL* — benign service URLs stay readable). Verified end-to-end via a real gateway run (real LLM + skill activation + bash): the secret reaches the sandbox subprocess and appears in NONE of prompt, trace, checkpoint, audit, stdout, runs.kwargs_json, or the run API. Part of #3861. * docs(backend): document the env scrub, persistence redaction, and sanitizer interaction Sync the Request-Scoped Secrets section with the verification-driven fixes: inherited-env scrub (incl. connection-string denylist), run-record/run-API redaction as the 6th sealed leak surface, and the sanitizer preserving original content so slash activation fires. Part of #3861. * fix(skills): inject caller secret over scrubbed host value; drop redundant host-name guard A real-world demo (a skill calling a third-party cloud API with a request-scoped key) exposed that the is_host_platform_secret guard was both wrong and harmful: it refused to inject a caller-supplied secret whenever a same-named variable existed in the Gateway env — which is exactly the #3861 use case (a per-user key overriding a shared platform key). The guard was also redundant: build_sandbox_env already scrubs secret-looking names from the inherited env before injection, so a skill can never read a host credential — it only ever receives the caller's value. Remove the guard; the injected (caller) value simply wins over the scrubbed host value. Verified end-to-end: the agent called the real cloud API successfully with the caller's key, the host's same-named key was scrubbed and never used, and the caller's key leaked to none of the surfaces. Part of #3861. * fix(skills): address review on request-scoped secrets (#3861) Review fixes from PR #3871: - E2BSandbox.execute_command now accepts env/timeout and routes them to commands.run(envs=, timeout=). The bash tool passes env= unconditionally, so the prior signature (command only) raised TypeError on every e2b bash call and broke e2b deployments entirely. env=None stays backward-compatible. - SkillActivationMiddleware clears the active-secret set before resolving each activation, so a later skill in the same run never inherits an earlier skill's injection set (the #3861 contract: a skill only receives what the caller supplied AND that skill declared). - AioSandbox env path uses a dedicated _DEFAULT_HARD_TIMEOUT — bash.exec exposes no idle/no-change timeout, so the prior reuse of the legacy idle constant conflated wall-clock vs idle semantics. The env path also retries on the ErrorObservation signature now, sharing the legacy persistent-shell recovery contract. - mask_secret_values skips values below a minimum length floor so a short declared secret (e.g. "42") cannot shred unrelated bytes (exit codes, timestamps, sizes) of tool output. The secret is still injected into the subprocess; only the output mask skips it. session_id reuse on the env path is intentionally NOT added: a shared session could let request-scoped secrets ride the session env into later commands, which the SDK does not contractually forbid. The fresh-session choice matches the LocalSandbox model (each call is a fresh subprocess); the trade-off (consecutive env-bearing calls do not share cwd/venv/exports) is documented on _execute_with_env.
This commit is contained in:
Xinmin Zeng
GitHub
parent
b476c7a18d
commit
09988caf95
@ -310,8 +310,9 @@ Proxied through nginx: `/api/langgraph/*` → Gateway LangGraph-compatible runti
|
||||
|
||||
### Sandbox System (`packages/harness/deerflow/sandbox/`)
|
||||
|
||||
**Interface**: Abstract `Sandbox` with `execute_command`, `read_file`, `write_file`, `list_dir`
|
||||
**Interface**: Abstract `Sandbox` with `execute_command(command, env=None)`, `read_file`, `write_file`, `list_dir`. 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.
|
||||
**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*`/`*PASSWORD*`/`*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`.
|
||||
- `AioSandboxProvider` (`packages/harness/deerflow/community/`) - Docker-based isolation. Active-cache and warm-pool entries are checked with the backend during acquire/reuse; definitively dead containers are dropped from all in-process maps so the thread can discover or create a fresh sandbox instead of reusing a stale client. Backend health-check failures are treated as unknown, not dead; local discovery likewise treats an unverifiable container as not adoptable and falls through to create rather than failing acquire. `get()` remains an in-memory lookup for event-loop-safe tool paths.
|
||||
@ -384,12 +385,24 @@ Additional providers also live here (`brave`, `browserless`, `crawl4ai`, `ddg_se
|
||||
### Skills System (`packages/harness/deerflow/skills/`)
|
||||
|
||||
- **Location**: `deer-flow/skills/{public,custom}/`
|
||||
- **Format**: Directory with `SKILL.md` (YAML frontmatter: name, description, license, allowed-tools)
|
||||
- **Format**: Directory with `SKILL.md` (YAML frontmatter: name, description, license, allowed-tools, required-secrets)
|
||||
- **Loading**: `load_skills()` recursively scans `skills/{public,custom}` for `SKILL.md`, parses metadata, and reads enabled state from extensions_config.json
|
||||
- **Injection**: Enabled skills listed in agent system prompt with container paths
|
||||
- **Slash activation**: `/skill-name task` loads that enabled skill's `SKILL.md` for the current model call only. The resolver rejects leading whitespace, missing separators, reserved channel commands (`/new`, `/help`, `/bootstrap`, `/status`, `/models`, `/memory`), disabled skills, and skills outside a custom agent's whitelist.
|
||||
- **Installation**: `POST /api/skills/install` extracts .skill ZIP archive to custom/ directory
|
||||
|
||||
#### Request-Scoped Secrets (`required-secrets`)
|
||||
|
||||
Lets a caller pass per-request, short-lived end-user credentials (e.g. an ERP token) to a skill's sandbox scripts without the value entering the prompt, tool arguments, the executed command string, or traces (issue #3861).
|
||||
|
||||
- **Declare**: a skill lists the secrets it needs in `SKILL.md` frontmatter — `required-secrets:` as a string list or `{name, optional}` mappings. `name` is both the lookup key and the env var name exposed to scripts. Parsed by `skills/parser.py::parse_required_secrets` into `Skill.required_secrets` (`SecretRequirement`); malformed entries are dropped with a warning.
|
||||
- **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.
|
||||
- **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`.
|
||||
|
||||
### Model Factory (`packages/harness/deerflow/models/factory.py`)
|
||||
|
||||
- `create_chat_model(name, thinking_enabled)` instantiates LLM from config via reflection
|
||||
|
||||
@ -37,6 +37,7 @@ from deerflow.runtime import (
|
||||
run_agent,
|
||||
)
|
||||
from deerflow.runtime.runs.naming import resolve_root_run_name
|
||||
from deerflow.runtime.secret_context import redact_config_secrets
|
||||
from deerflow.runtime.user_context import reset_current_user, set_current_user
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@ -290,6 +291,12 @@ def build_run_config(
|
||||
raise ValueError("request config 'context' must be a mapping or null.")
|
||||
context["thread_id"] = thread_id
|
||||
config["context"] = context
|
||||
# The checkpointer always scopes state by configurable["thread_id"],
|
||||
# regardless of whether the caller drives the run via context (e.g.
|
||||
# request-scoped secrets, #3861). thread_id comes from the URL path,
|
||||
# not caller config, so mirror it here while keeping secret-bearing
|
||||
# context keys out of configurable.
|
||||
config["configurable"] = {"thread_id": thread_id}
|
||||
else:
|
||||
configurable = {"thread_id": thread_id}
|
||||
configurable.update(request_config.get("configurable", {}))
|
||||
@ -476,7 +483,11 @@ async def start_run(
|
||||
body.assistant_id,
|
||||
on_disconnect=disconnect,
|
||||
metadata=body.metadata or {},
|
||||
kwargs={"input": body.input, "config": body.config},
|
||||
# Persist a secret-redacted copy of the config: the run record is
|
||||
# written to runs.kwargs_json and echoed by the run API, so a
|
||||
# request-scoped secret (#3861) must not ride along. The live
|
||||
# config built below keeps the secrets for the actual run.
|
||||
kwargs={"input": body.input, "config": redact_config_secrets(body.config)},
|
||||
multitask_strategy=body.multitask_strategy,
|
||||
model_name=model_name,
|
||||
user_id=owner_user_id,
|
||||
|
||||
@ -31,6 +31,8 @@ from langchain.agents.middleware.types import (
|
||||
from langchain_core.messages import HumanMessage
|
||||
from langgraph.errors import GraphBubbleUp
|
||||
|
||||
from deerflow.utils.messages import ORIGINAL_USER_CONTENT_KEY, message_content_to_text
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_SUMMARY_MESSAGE_NAME = "summary"
|
||||
@ -233,11 +235,17 @@ class InputSanitizationMiddleware(AgentMiddleware[AgentState]):
|
||||
else:
|
||||
new_content = processed
|
||||
|
||||
# Preserve the pre-sanitization user text so downstream consumers that
|
||||
# must see the genuine input (slash skill activation, regenerate) can
|
||||
# recover it after the BEGIN/END wrapping. setdefault keeps an existing
|
||||
# value (e.g. set by UploadsMiddleware or an IM channel) authoritative.
|
||||
preserved_kwargs = dict(msg.additional_kwargs or {})
|
||||
preserved_kwargs.setdefault(ORIGINAL_USER_CONTENT_KEY, message_content_to_text(content))
|
||||
messages[i] = HumanMessage(
|
||||
content=new_content,
|
||||
id=msg.id,
|
||||
name=msg.name,
|
||||
additional_kwargs=msg.additional_kwargs,
|
||||
additional_kwargs=preserved_kwargs,
|
||||
)
|
||||
logger.debug(
|
||||
"InputSanitizationMiddleware: original=%r -> processed=%r",
|
||||
|
||||
@ -16,10 +16,11 @@ from langchain.agents.middleware import AgentMiddleware
|
||||
from langchain.agents.middleware.types import ModelRequest, ModelResponse
|
||||
from langchain_core.messages import AIMessage, HumanMessage
|
||||
|
||||
from deerflow.runtime.secret_context import ACTIVE_SECRETS_CONTEXT_KEY, extract_request_secrets
|
||||
from deerflow.skills.slash import parse_slash_skill_reference, resolve_slash_skill
|
||||
from deerflow.skills.storage import get_or_new_skill_storage
|
||||
from deerflow.skills.storage.skill_storage import SkillStorage
|
||||
from deerflow.skills.types import SKILL_MD_FILE
|
||||
from deerflow.skills.types import SKILL_MD_FILE, SecretRequirement
|
||||
from deerflow.utils.messages import get_original_user_content_text
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@ -40,6 +41,7 @@ class _Activation:
|
||||
skill_content: str
|
||||
content_hash: str
|
||||
remaining_text: str
|
||||
required_secrets: tuple[SecretRequirement, ...] = ()
|
||||
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
@ -134,6 +136,7 @@ class SkillActivationMiddleware(AgentMiddleware):
|
||||
skill_content=skill_content,
|
||||
content_hash=content_hash,
|
||||
remaining_text=resolved.remaining_text,
|
||||
required_secrets=tuple(resolved.skill.required_secrets or ()),
|
||||
)
|
||||
)
|
||||
|
||||
@ -242,11 +245,60 @@ Follow this skill before choosing a general workflow. Load supporting resources
|
||||
activation.content_hash,
|
||||
)
|
||||
self._record_activation(request, activation, hook=hook)
|
||||
self._apply_skill_secrets(request, activation)
|
||||
activation_msg = self._make_activation_message(target, self._build_activation_reminder(activation))
|
||||
messages = list(request.messages)
|
||||
messages.insert(target_index, activation_msg)
|
||||
return request.override(messages=messages)
|
||||
|
||||
@staticmethod
|
||||
def _apply_skill_secrets(request: ModelRequest, activation: _Activation) -> None:
|
||||
"""Resolve the activated skill's declared secrets into the per-run injection
|
||||
set (binding point A, issue #3861).
|
||||
|
||||
For each declared secret present in the request's ``context.secrets``,
|
||||
record its value in the injection set stored under
|
||||
``ACTIVE_SECRETS_CONTEXT_KEY`` on the shared run context, so the bash tool
|
||||
can build the subprocess env for this turn. The injected value always comes
|
||||
from the caller's request — never from the host environment, which is
|
||||
scrubbed of secret-looking names by ``env_policy.build_sandbox_env`` before
|
||||
injection. A skill can therefore never harvest a host platform credential
|
||||
(it only ever receives what the caller explicitly supplied), so a declared
|
||||
name that also exists in the host env is fine: the caller's value wins and
|
||||
the host value is dropped. Secret *values* are never logged.
|
||||
"""
|
||||
runtime = getattr(request, "runtime", None)
|
||||
context = getattr(runtime, "context", None)
|
||||
if not isinstance(context, dict):
|
||||
return
|
||||
# Unconditionally clear any active-secret set a previous activation in
|
||||
# the same run may have written, before this turn's resolution decides
|
||||
# what (if anything) to install. Otherwise a later skill that declares
|
||||
# no secrets, or whose required secrets the caller did not supply, would
|
||||
# inherit the previous skill's injection set and the bash tool would
|
||||
# inject those values into a subprocess that never declared them (#3861).
|
||||
context.pop(ACTIVE_SECRETS_CONTEXT_KEY, None)
|
||||
if not activation.required_secrets:
|
||||
return
|
||||
|
||||
request_secrets = extract_request_secrets(context)
|
||||
injected: dict[str, str] = {}
|
||||
missing: list[str] = []
|
||||
for req in activation.required_secrets:
|
||||
if req.name in request_secrets:
|
||||
injected[req.name] = request_secrets[req.name]
|
||||
elif not req.optional:
|
||||
missing.append(req.name)
|
||||
|
||||
if injected:
|
||||
context[ACTIVE_SECRETS_CONTEXT_KEY] = injected
|
||||
if missing:
|
||||
logger.warning(
|
||||
"Skill %s activated but required secrets are missing from the request context: %s",
|
||||
activation.skill_name,
|
||||
", ".join(sorted(missing)),
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _make_activation_message(target: HumanMessage, activation_content: str) -> HumanMessage:
|
||||
stable_id = target.id or str(uuid.uuid4())
|
||||
|
||||
@ -110,7 +110,22 @@ class AioSandbox(Sandbox):
|
||||
# default.
|
||||
_DEFAULT_NO_CHANGE_TIMEOUT = 600
|
||||
|
||||
def execute_command(self, command: str) -> str:
|
||||
# Wall-clock hard timeout for env-bearing commands routed through bash.exec.
|
||||
# The bash.exec API exposes no idle/no-change timeout (unlike
|
||||
# shell.exec_command's ``no_change_timeout`` on the legacy path), so
|
||||
# env-bearing commands are bounded by total elapsed wall-clock time, not
|
||||
# time-since-last-output. Kept at the same numeric value as the legacy idle
|
||||
# budget so the two paths broadly agree on how long a single command may
|
||||
# run; a future SDK that exposes an idle timeout on bash.exec should switch
|
||||
# this call site to it.
|
||||
_DEFAULT_HARD_TIMEOUT = 600.0
|
||||
|
||||
def execute_command(
|
||||
self,
|
||||
command: str,
|
||||
env: dict[str, str] | None = None,
|
||||
timeout: float | None = None,
|
||||
) -> str:
|
||||
"""Execute a shell command in the sandbox.
|
||||
|
||||
Uses a lock to serialize concurrent requests. The AIO sandbox
|
||||
@ -122,10 +137,23 @@ class AioSandbox(Sandbox):
|
||||
|
||||
Args:
|
||||
command: The command to execute.
|
||||
env: Optional per-call environment variables (request-scoped secrets,
|
||||
issue #3861). When provided, the command runs via the ``bash.exec``
|
||||
API (which supports per-command env) on a fresh auto-created session
|
||||
so the secrets are scoped to this single command and never persist;
|
||||
secret values travel in the structured ``env`` field, never in the
|
||||
command string. When ``None`` the legacy persistent-shell path runs
|
||||
unchanged.
|
||||
timeout: Optional per-call timeout. The current sandbox SDK does not
|
||||
expose a command-level timeout distinct from its client/request
|
||||
timeout, so DeerFlow keeps using the backend's default here.
|
||||
|
||||
Returns:
|
||||
The output of the command.
|
||||
"""
|
||||
del timeout
|
||||
if env:
|
||||
return self._execute_with_env(command, env)
|
||||
with self._lock:
|
||||
try:
|
||||
result = self._client.shell.exec_command(command=command, no_change_timeout=self._DEFAULT_NO_CHANGE_TIMEOUT)
|
||||
@ -154,6 +182,57 @@ class AioSandbox(Sandbox):
|
||||
logger.error(f"Failed to execute command in sandbox: {e}")
|
||||
return f"Error: {e}"
|
||||
|
||||
def _execute_with_env(self, command: str, env: dict[str, str]) -> str:
|
||||
"""Execute a command with per-call environment variables injected.
|
||||
|
||||
The persistent-shell ``shell.exec_command`` API has no env parameter, so
|
||||
injected commands use the ``bash.exec`` API which accepts per-command env.
|
||||
Each call lets the sandbox auto-create a fresh session (no ``session_id``),
|
||||
so injected request-scoped secrets are scoped to this command and never
|
||||
persist across calls. Secret values travel in the structured ``env`` field,
|
||||
never in the command string.
|
||||
|
||||
Trade-off of the fresh-session choice: consecutive env-bearing bash calls
|
||||
within the same skill do not share session state (cwd, sourced venv,
|
||||
exported variables). This mirrors the LocalSandbox model (each call is a
|
||||
fresh subprocess) and is intentional — a shared session_id would let
|
||||
request-scoped secrets ride the session env into later commands, which the
|
||||
SDK does not contractually forbid. Skills that need setup must fold it into
|
||||
a single command (e.g. ``cd /mnt/user-data/workspace && source .venv/bin/activate && python run.py``).
|
||||
|
||||
The ``_ERROR_OBSERVATION_SIGNATURE`` recovery contract is shared with the
|
||||
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.
|
||||
"""
|
||||
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")
|
||||
retried = self._run_bash_exec(command, env)
|
||||
if retried and _ERROR_OBSERVATION_SIGNATURE not in retried:
|
||||
return retried
|
||||
return output
|
||||
|
||||
def _run_bash_exec(self, command: str, env: dict[str, str]) -> str:
|
||||
"""Single bash.exec invocation with injected env (one fresh session)."""
|
||||
with self._lock:
|
||||
try:
|
||||
result = self._client.bash.exec(
|
||||
command=command,
|
||||
env=env,
|
||||
hard_timeout=self._DEFAULT_HARD_TIMEOUT,
|
||||
)
|
||||
data = result.data if result else None
|
||||
stdout = (data.stdout or "") if data else ""
|
||||
stderr = (data.stderr or "") if data else ""
|
||||
output = stdout
|
||||
if stderr:
|
||||
output += f"\nStd Error:\n{stderr}" if output else stderr
|
||||
return output if output else "(no output)"
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to execute command with injected env in sandbox: {e}")
|
||||
return f"Error: {e}"
|
||||
|
||||
def read_file(self, path: str) -> str:
|
||||
"""Read the content of a file in the sandbox.
|
||||
|
||||
|
||||
@ -118,12 +118,25 @@ class E2BSandbox(Sandbox):
|
||||
return f"{self._home_dir}/{tail}".rstrip("/") if tail else self._home_dir
|
||||
return normalised
|
||||
|
||||
def execute_command(self, command: str) -> str:
|
||||
def execute_command(
|
||||
self,
|
||||
command: str,
|
||||
env: dict[str, str] | None = None,
|
||||
timeout: float | None = None,
|
||||
) -> str:
|
||||
"""Execute a shell command via ``sandbox.commands.run``.
|
||||
|
||||
Returns the combined stdout/stderr.
|
||||
The lock serialises concurrent calls on the same instance
|
||||
because the e2b SDK shares a single HTTP/2 connection per sandbox.
|
||||
|
||||
Args:
|
||||
command: The command to execute.
|
||||
env: Optional per-call environment variables (request-scoped secrets,
|
||||
issue #3861). Passed through to e2b as ``envs``, which are scoped
|
||||
to this command only and never placed in the command string.
|
||||
timeout: Optional per-call command timeout in seconds. ``None`` keeps
|
||||
the e2b SDK default (60s).
|
||||
"""
|
||||
with self._lock:
|
||||
client = self._client
|
||||
@ -132,7 +145,12 @@ class E2BSandbox(Sandbox):
|
||||
if self._dead:
|
||||
return "Error: e2b sandbox has been reaped by the control plane (idle timeout or explicit pause). The provider will rebuild a fresh sandbox on the next tool call."
|
||||
try:
|
||||
result = client.commands.run(command)
|
||||
kwargs: dict[str, object] = {}
|
||||
if env is not None:
|
||||
kwargs["envs"] = env
|
||||
if timeout is not None:
|
||||
kwargs["timeout"] = timeout
|
||||
result = client.commands.run(command, **kwargs)
|
||||
stdout = getattr(result, "stdout", "") or ""
|
||||
stderr = getattr(result, "stderr", "") or ""
|
||||
exit_code = getattr(result, "exit_code", 0)
|
||||
|
||||
88
backend/packages/harness/deerflow/runtime/secret_context.py
Normal file
88
backend/packages/harness/deerflow/runtime/secret_context.py
Normal file
@ -0,0 +1,88 @@
|
||||
"""Request-scoped secret carrier in the run context (issue #3861).
|
||||
|
||||
Callers pass per-request secrets out-of-band in ``config.context.secrets`` — a
|
||||
mapping of name -> value. The value never enters the prompt, tool arguments, or
|
||||
the executed command string; it is injected as an environment variable into a
|
||||
skill's sandbox subprocess only when an activated skill declares it via the
|
||||
``required-secrets`` frontmatter field.
|
||||
|
||||
This module centralises the reserved key name and safe extraction so the carrier
|
||||
contract lives in one place, consumed by the skill-activation middleware (to
|
||||
build the per-turn injection set) and the tracing redactor (to strip it from
|
||||
trace payloads).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
|
||||
# Reserved sub-key of the run context that holds request-scoped secrets supplied
|
||||
# by the caller. Source of truth for what a skill *may* receive.
|
||||
SECRETS_CONTEXT_KEY = "secrets"
|
||||
|
||||
# Reserved sub-key holding the secrets resolved for the currently activated skill
|
||||
# (binding point A). Written by the skill-activation middleware, read by the bash
|
||||
# tool. Both reserved keys are stripped from trace payloads (see tracing redactor).
|
||||
ACTIVE_SECRETS_CONTEXT_KEY = "__active_skill_secrets"
|
||||
|
||||
|
||||
def _string_pairs(raw: Any) -> dict[str, str]:
|
||||
if not isinstance(raw, dict):
|
||||
return {}
|
||||
return {key: value for key, value in raw.items() if isinstance(key, str) and isinstance(value, str)}
|
||||
|
||||
|
||||
def extract_request_secrets(context: Any) -> dict[str, str]:
|
||||
"""Return the caller-supplied request-scoped secrets mapping, or ``{}``.
|
||||
|
||||
Only string-keyed, string-valued entries are kept; anything else is ignored
|
||||
so a malformed carrier can never crash secret resolution or injection.
|
||||
"""
|
||||
if not isinstance(context, dict):
|
||||
return {}
|
||||
return _string_pairs(context.get(SECRETS_CONTEXT_KEY))
|
||||
|
||||
|
||||
def read_active_secrets(context: Any) -> dict[str, str]:
|
||||
"""Return the secrets resolved for the active skill (the per-run injection
|
||||
set), or ``{}``. Read by the bash tool to build the subprocess env."""
|
||||
if not isinstance(context, dict):
|
||||
return {}
|
||||
return _string_pairs(context.get(ACTIVE_SECRETS_CONTEXT_KEY))
|
||||
|
||||
|
||||
# Run-context keys whose values are request-scoped secrets and must be stripped
|
||||
# before a context mapping is serialized anywhere observable (traces, logs).
|
||||
REDACTED_CONTEXT_KEYS = frozenset({SECRETS_CONTEXT_KEY, ACTIVE_SECRETS_CONTEXT_KEY})
|
||||
|
||||
|
||||
def redact_secret_context_keys(context: Any) -> Any:
|
||||
"""Return a shallow copy of ``context`` with secret-bearing keys removed.
|
||||
|
||||
Defensive helper for any code path that serializes the run context into an
|
||||
observable surface. DeerFlow's own trace-metadata builder never copies the
|
||||
context, so this is belt-and-suspenders for future call sites and custom
|
||||
tracer configurations.
|
||||
"""
|
||||
if not isinstance(context, dict):
|
||||
return context
|
||||
return {key: value for key, value in context.items() if key not in REDACTED_CONTEXT_KEYS}
|
||||
|
||||
|
||||
def redact_config_secrets(config: Any) -> Any:
|
||||
"""Return a copy of a run config safe to persist or echo back to clients.
|
||||
|
||||
The request config (``body.config``) is stored verbatim on the run record
|
||||
(``runs.kwargs_json``) and echoed by the run API. Strip the secret-bearing
|
||||
keys from its ``context`` so a request-scoped secret is never persisted or
|
||||
returned, while the live config that drives the run (built separately) keeps
|
||||
them. Non-dict / context-less configs pass through unchanged.
|
||||
"""
|
||||
if not isinstance(config, dict):
|
||||
return config
|
||||
context = config.get("context")
|
||||
if not isinstance(context, dict):
|
||||
return config
|
||||
redacted = dict(config)
|
||||
redacted["context"] = redact_secret_context_keys(context)
|
||||
return redacted
|
||||
82
backend/packages/harness/deerflow/sandbox/env_policy.py
Normal file
82
backend/packages/harness/deerflow/sandbox/env_policy.py
Normal file
@ -0,0 +1,82 @@
|
||||
"""Environment-variable policy for sandbox command execution (issue #3861).
|
||||
|
||||
Skill scripts run as sandbox subprocesses. By default a subprocess inherits the
|
||||
Gateway process's entire ``os.environ`` — which holds platform credentials
|
||||
(``OPENAI_API_KEY``, tracing keys, community-provider keys, ...). That makes any
|
||||
scoped request-secret injection pointless: a script could simply read those
|
||||
inherited platform secrets. This module scrubs secret-looking variables from the
|
||||
inherited environment before request-scoped secrets are layered on top.
|
||||
|
||||
The pattern set mirrors codex's ``*KEY*/*SECRET*/*TOKEN*`` default excludes and
|
||||
hermes's fixed provider blocklist; unlike codex (which defaults the exclude
|
||||
*off*), DeerFlow scrubs by default — security first.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import fnmatch
|
||||
import os
|
||||
|
||||
# Case-insensitive wildcard patterns for secret-looking variable names. Matched
|
||||
# against the upper-cased variable name. Benign system vars (PATH, HOME, SHELL,
|
||||
# LANG, PWD, TMPDIR, VIRTUAL_ENV, PYTHONPATH, ...) contain none of these tokens
|
||||
# and are therefore preserved.
|
||||
_SECRET_NAME_PATTERNS: tuple[str, ...] = (
|
||||
"*KEY*",
|
||||
"*SECRET*",
|
||||
"*TOKEN*",
|
||||
"*PASSWORD*",
|
||||
"*PASSWD*",
|
||||
"*CREDENTIAL*",
|
||||
"*DSN*", # data source name — almost always a connection string with a password
|
||||
)
|
||||
|
||||
# Connection-string / credential-bearing variable names that carry no
|
||||
# KEY/SECRET/TOKEN/DSN substring but routinely embed a password (e.g.
|
||||
# ``postgresql://user:pw@host/db``). A blanket ``*URL*`` block is intentionally
|
||||
# avoided — it would strip benign service URLs a skill may legitimately read.
|
||||
# A skill that genuinely needs one of these must declare it via required-secrets
|
||||
# (the caller then supplies it through context.secrets, and injection wins).
|
||||
_BLOCKED_EXACT_NAMES: frozenset[str] = frozenset(
|
||||
{
|
||||
"DATABASE_URL",
|
||||
"DATABASE_URI",
|
||||
"REDIS_URL",
|
||||
"MONGODB_URI",
|
||||
"MONGO_URL",
|
||||
"AMQP_URL",
|
||||
"RABBITMQ_URL",
|
||||
"POSTGRES_URL",
|
||||
"POSTGRESQL_URL",
|
||||
"MYSQL_URL",
|
||||
"CLICKHOUSE_URL",
|
||||
"CONNECTION_STRING",
|
||||
"CONN_STR",
|
||||
"GH_PAT",
|
||||
"GITHUB_PAT",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def is_blocked_env_name(name: str) -> bool:
|
||||
"""Return True if ``name`` looks like a credential that must not be inherited
|
||||
by a sandbox subprocess."""
|
||||
upper = name.upper()
|
||||
if upper in _BLOCKED_EXACT_NAMES:
|
||||
return True
|
||||
return any(fnmatch.fnmatchcase(upper, pattern) for pattern in _SECRET_NAME_PATTERNS)
|
||||
|
||||
|
||||
def build_sandbox_env(injected: dict[str, str] | None = None) -> dict[str, str]:
|
||||
"""Build the environment dict for a sandbox subprocess.
|
||||
|
||||
Inherits ``os.environ`` minus any secret-looking variables, then layers the
|
||||
explicitly injected request-scoped secrets on top. An injected secret wins
|
||||
even if its name matches a blocked pattern, because injection is authorized
|
||||
upstream (the skill declared it and the value came from the request, not from
|
||||
the host environment).
|
||||
"""
|
||||
env = {key: value for key, value in os.environ.items() if not is_blocked_env_name(key)}
|
||||
if injected:
|
||||
env.update(injected)
|
||||
return env
|
||||
@ -13,6 +13,7 @@ from pathlib import Path
|
||||
from typing import NamedTuple
|
||||
|
||||
from deerflow.config.paths import VIRTUAL_PATH_PREFIX
|
||||
from deerflow.sandbox.env_policy import build_sandbox_env
|
||||
from deerflow.sandbox.local.list_dir import list_dir
|
||||
from deerflow.sandbox.sandbox import Sandbox
|
||||
from deerflow.sandbox.search import GrepMatch, find_glob_matches, find_grep_matches
|
||||
@ -433,16 +434,24 @@ class LocalSandbox(Sandbox):
|
||||
|
||||
raise RuntimeError("No suitable shell executable found. Tried /bin/zsh, /bin/bash, /bin/sh, and `sh` on PATH.")
|
||||
|
||||
def execute_command(self, command: str, timeout: float | None = None) -> str:
|
||||
def execute_command(
|
||||
self,
|
||||
command: str,
|
||||
env: dict[str, str] | None = None,
|
||||
timeout: float | None = None,
|
||||
) -> str:
|
||||
# Resolve container paths in command before execution
|
||||
resolved_command = self._resolve_paths_in_command(command)
|
||||
shell = self._get_shell()
|
||||
if timeout is None:
|
||||
timeout = DEFAULT_COMMAND_TIMEOUT_SECONDS
|
||||
|
||||
# Inherit os.environ minus platform secrets, then layer any injected
|
||||
# request-scoped secrets on top (#3861). An explicit env is always passed
|
||||
# so platform credentials never leak into skill subprocesses.
|
||||
sandbox_env = build_sandbox_env(env)
|
||||
timed_out = False
|
||||
if os.name == "nt":
|
||||
env = None
|
||||
if self._is_powershell(shell):
|
||||
args = [shell, "-NoProfile", "-Command", resolved_command]
|
||||
elif self._is_cmd_shell(shell):
|
||||
@ -450,8 +459,8 @@ class LocalSandbox(Sandbox):
|
||||
else:
|
||||
args = [shell, "-c", resolved_command]
|
||||
if self._is_msys_shell(shell):
|
||||
env = {
|
||||
**os.environ,
|
||||
sandbox_env = {
|
||||
**sandbox_env,
|
||||
"MSYS_NO_PATHCONV": "1",
|
||||
"MSYS2_ARG_CONV_EXCL": "*",
|
||||
}
|
||||
@ -463,7 +472,7 @@ class LocalSandbox(Sandbox):
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=timeout,
|
||||
env=env,
|
||||
env=sandbox_env,
|
||||
)
|
||||
stdout, stderr, returncode = result.stdout, result.stderr, result.returncode
|
||||
except subprocess.TimeoutExpired as exc:
|
||||
@ -473,7 +482,7 @@ class LocalSandbox(Sandbox):
|
||||
returncode = 0
|
||||
else:
|
||||
args = [shell, "-c", resolved_command]
|
||||
stdout, stderr, returncode, timed_out = self._run_posix_command(args, timeout)
|
||||
stdout, stderr, returncode, timed_out = self._run_posix_command(args, timeout, sandbox_env)
|
||||
|
||||
output = stdout
|
||||
if stderr:
|
||||
@ -489,7 +498,11 @@ class LocalSandbox(Sandbox):
|
||||
return self._reverse_resolve_paths_in_output(final_output)
|
||||
|
||||
@staticmethod
|
||||
def _run_posix_command(args: list[str], timeout: float) -> tuple[str, str, int, bool]:
|
||||
def _run_posix_command(
|
||||
args: list[str],
|
||||
timeout: float,
|
||||
env: dict[str, str] | None = None,
|
||||
) -> tuple[str, str, int, bool]:
|
||||
"""Run a command on POSIX with bounded pipe capture.
|
||||
|
||||
``subprocess.communicate()`` cannot be used here: a backgrounded
|
||||
@ -517,6 +530,7 @@ class LocalSandbox(Sandbox):
|
||||
stdout=stdout_write_fd,
|
||||
stderr=stderr_write_fd,
|
||||
start_new_session=True,
|
||||
env=env,
|
||||
)
|
||||
except Exception:
|
||||
for fd in (stdout_read_fd, stdout_write_fd, stderr_read_fd, stderr_write_fd):
|
||||
|
||||
@ -16,11 +16,27 @@ class Sandbox(ABC):
|
||||
return self._id
|
||||
|
||||
@abstractmethod
|
||||
def execute_command(self, command: str) -> str:
|
||||
def execute_command(
|
||||
self,
|
||||
command: str,
|
||||
env: dict[str, str] | None = None,
|
||||
timeout: float | None = None,
|
||||
) -> str:
|
||||
"""Execute bash command in sandbox.
|
||||
|
||||
Args:
|
||||
command: The command to execute.
|
||||
env: Optional per-call environment variables to inject into the
|
||||
command's process. Used to pass request-scoped secrets (e.g. a
|
||||
short-lived end-user token) to skill scripts without placing them
|
||||
in the prompt, tool arguments, or the command string (issue #3861).
|
||||
When ``None`` the sandbox uses its default environment.
|
||||
timeout: Optional per-call wall-clock timeout in seconds. Local
|
||||
sandboxes use this to bound host bash commands so long-lived
|
||||
foreground processes cannot hang a turn indefinitely. Remote/AIO
|
||||
implementations may ignore it when their backend does not expose
|
||||
an equivalent command-timeout control separate from its own API
|
||||
timeouts.
|
||||
|
||||
Returns:
|
||||
The standard or error output of the command.
|
||||
|
||||
@ -12,6 +12,7 @@ from langchain.tools import tool
|
||||
from deerflow.agents.thread_state import ThreadDataState
|
||||
from deerflow.config import get_app_config
|
||||
from deerflow.config.paths import VIRTUAL_PATH_PREFIX
|
||||
from deerflow.runtime.secret_context import read_active_secrets
|
||||
from deerflow.runtime.user_context import resolve_runtime_user_id
|
||||
from deerflow.sandbox.exceptions import (
|
||||
SandboxError,
|
||||
@ -1309,6 +1310,37 @@ def ensure_thread_directories_exist(runtime: Runtime | None) -> None:
|
||||
runtime.state["thread_directories_created"] = True
|
||||
|
||||
|
||||
_SECRET_REDACTION = "[redacted]"
|
||||
|
||||
# Values shorter than this are not redacted from bash output. A short secret
|
||||
# value (a 2-char region code, a numeric id, a PIN) would otherwise shred
|
||||
# unrelated bytes of tool output — exit codes, timestamps, sizes, paths —
|
||||
# corrupting the result the model reads back. The redaction of a value this
|
||||
# short is more likely noise than genuine leak protection; the secret is still
|
||||
# injected into the subprocess, only the output mask skips it.
|
||||
_MIN_MASK_LENGTH = 8
|
||||
|
||||
|
||||
def mask_secret_values(output: str, injected_env: dict[str, str] | None) -> str:
|
||||
"""Redact injected secret values from bash output before it re-enters context.
|
||||
|
||||
Skill scripts receive request-scoped secrets as env vars (#3861). If a script
|
||||
echoes one (debugging, ``set -x``, an error dump), the value would otherwise
|
||||
flow into the tool result — and thus into the prompt and the trace. This is
|
||||
the skill-specific fifth leak surface (the bash tool returns subprocess stdout,
|
||||
unlike MCP tools). Replace each non-empty secret value with a redaction marker.
|
||||
Longest values first so a value that is a substring of another is not partially
|
||||
revealed. Values shorter than ``_MIN_MASK_LENGTH`` are skipped — a redacted
|
||||
3-char token is more likely to corrupt unrelated output than to protect a
|
||||
real secret.
|
||||
"""
|
||||
if not injected_env or not output:
|
||||
return output
|
||||
for value in sorted((v for v in injected_env.values() if v and len(v) >= _MIN_MASK_LENGTH), key=len, reverse=True):
|
||||
output = output.replace(value, _SECRET_REDACTION)
|
||||
return output
|
||||
|
||||
|
||||
def _truncate_bash_output(output: str, max_chars: int) -> str:
|
||||
"""Middle-truncate bash output, preserving head and tail (50/50 split).
|
||||
|
||||
@ -1404,6 +1436,9 @@ def bash_tool(runtime: Runtime, description: str, command: str) -> str:
|
||||
"""
|
||||
try:
|
||||
sandbox = ensure_sandbox_initialized(runtime)
|
||||
# Request-scoped secrets resolved for the active skill (#3861); injected as
|
||||
# per-call env into the subprocess, never placed in the command string.
|
||||
injected_env = read_active_secrets(getattr(runtime, "context", None)) or None
|
||||
if is_local_sandbox(runtime):
|
||||
if not is_host_bash_allowed():
|
||||
return f"Error: {LOCAL_HOST_BASH_DISABLED_MESSAGE}"
|
||||
@ -1421,8 +1456,11 @@ def bash_tool(runtime: Runtime, description: str, command: str) -> str:
|
||||
except Exception:
|
||||
max_chars = 20000
|
||||
command_timeout = None
|
||||
output = sandbox.execute_command(command, timeout=command_timeout)
|
||||
return _truncate_bash_output(mask_local_paths_in_output(output, thread_data), max_chars)
|
||||
output = sandbox.execute_command(command, env=injected_env, timeout=command_timeout)
|
||||
return _truncate_bash_output(
|
||||
mask_secret_values(mask_local_paths_in_output(output, thread_data), injected_env),
|
||||
max_chars,
|
||||
)
|
||||
ensure_thread_directories_exist(runtime)
|
||||
try:
|
||||
from deerflow.config.app_config import get_app_config
|
||||
@ -1431,7 +1469,7 @@ def bash_tool(runtime: Runtime, description: str, command: str) -> str:
|
||||
max_chars = sandbox_cfg.bash_output_max_chars if sandbox_cfg else 20000
|
||||
except Exception:
|
||||
max_chars = 20000
|
||||
return _truncate_bash_output(sandbox.execute_command(command), max_chars)
|
||||
return _truncate_bash_output(mask_secret_values(sandbox.execute_command(command, env=injected_env), injected_env), max_chars)
|
||||
except SandboxError as e:
|
||||
return f"Error: {e}"
|
||||
except PermissionError as e:
|
||||
|
||||
@ -4,10 +4,13 @@ from pathlib import Path
|
||||
|
||||
import yaml
|
||||
|
||||
from .types import SKILL_MD_FILE, Skill, SkillCategory
|
||||
from .types import SKILL_MD_FILE, SecretRequirement, Skill, SkillCategory
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Valid POSIX environment-variable name.
|
||||
_ENV_VAR_NAME_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
|
||||
|
||||
|
||||
def _format_yaml_error(skill_file: Path, exc: yaml.YAMLError, source: str) -> str:
|
||||
"""Render a developer-friendly explanation of a YAML front-matter error."""
|
||||
@ -63,6 +66,43 @@ def parse_allowed_tools(raw: object, skill_file: Path) -> list[str] | None:
|
||||
return allowed_tools
|
||||
|
||||
|
||||
def parse_required_secrets(raw: object, skill_file: Path) -> list[SecretRequirement]:
|
||||
"""Parse the optional required-secrets frontmatter field (issue #3861).
|
||||
|
||||
Accepts a YAML sequence whose items are either a string (the secret / env
|
||||
variable name) or a mapping (``{name, optional}``). Returns an empty list
|
||||
when the field is omitted. Entries whose name is missing or is not a valid
|
||||
environment-variable name are dropped with a warning, so one malformed
|
||||
declaration does not invalidate the whole skill. Raises ValueError only when
|
||||
the field is present but is not a list.
|
||||
"""
|
||||
if raw is None:
|
||||
return []
|
||||
if not isinstance(raw, list):
|
||||
raise ValueError(f"required-secrets in {skill_file} must be a list")
|
||||
|
||||
secrets: list[SecretRequirement] = []
|
||||
seen: set[str] = set()
|
||||
for item in raw:
|
||||
if isinstance(item, str):
|
||||
name, optional = item.strip(), False
|
||||
elif isinstance(item, dict):
|
||||
name = str(item.get("name") or "").strip()
|
||||
optional = bool(item.get("optional", False))
|
||||
else:
|
||||
logger.warning("Ignoring malformed required-secrets entry in %s: %r", skill_file, item)
|
||||
continue
|
||||
|
||||
if not _ENV_VAR_NAME_RE.match(name):
|
||||
logger.warning("Ignoring required-secrets entry with invalid env var name in %s: %r", skill_file, name)
|
||||
continue
|
||||
if name in seen:
|
||||
continue
|
||||
seen.add(name)
|
||||
secrets.append(SecretRequirement(name=name, optional=optional))
|
||||
return secrets
|
||||
|
||||
|
||||
def parse_skill_file(skill_file: Path, category: SkillCategory, relative_path: Path | None = None) -> Skill | None:
|
||||
"""Parse a SKILL.md file and extract metadata.
|
||||
|
||||
@ -124,6 +164,12 @@ def parse_skill_file(skill_file: Path, category: SkillCategory, relative_path: P
|
||||
logger.error("Invalid allowed-tools in %s: %s", skill_file, exc)
|
||||
return None
|
||||
|
||||
try:
|
||||
required_secrets = parse_required_secrets(metadata.get("required-secrets"), skill_file)
|
||||
except ValueError as exc:
|
||||
logger.error("Invalid required-secrets in %s: %s", skill_file, exc)
|
||||
return None
|
||||
|
||||
return Skill(
|
||||
name=name,
|
||||
description=description,
|
||||
@ -134,6 +180,7 @@ def parse_skill_file(skill_file: Path, category: SkillCategory, relative_path: P
|
||||
category=category,
|
||||
allowed_tools=allowed_tools,
|
||||
enabled=True, # Actual state comes from the extensions config file.
|
||||
required_secrets=required_secrets,
|
||||
)
|
||||
|
||||
except Exception:
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
from dataclasses import dataclass
|
||||
from dataclasses import dataclass, field
|
||||
from enum import StrEnum
|
||||
from pathlib import Path
|
||||
|
||||
@ -16,6 +16,19 @@ class SkillCategory(StrEnum):
|
||||
CUSTOM = "custom"
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class SecretRequirement:
|
||||
"""A request-scoped secret a skill declares it needs (issue #3861).
|
||||
|
||||
``name`` is both the key looked up in the request's ``context.secrets`` and
|
||||
the environment variable name injected into the skill's sandbox subprocess
|
||||
when the skill is activated.
|
||||
"""
|
||||
|
||||
name: str
|
||||
optional: bool = False
|
||||
|
||||
|
||||
@dataclass
|
||||
class Skill:
|
||||
"""Represents a skill with its metadata and file path"""
|
||||
@ -29,6 +42,7 @@ class Skill:
|
||||
category: SkillCategory # 'public' or 'custom'
|
||||
allowed_tools: list[str] | None = None
|
||||
enabled: bool = False # Whether this skill is enabled
|
||||
required_secrets: list[SecretRequirement] = field(default_factory=list)
|
||||
|
||||
@property
|
||||
def skill_path(self) -> str:
|
||||
|
||||
@ -7,6 +7,7 @@ import threading
|
||||
from collections import OrderedDict
|
||||
from types import SimpleNamespace
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from deerflow.config.paths import Paths
|
||||
|
||||
@ -257,6 +258,42 @@ def test_execute_command_does_not_mark_dead_on_unrelated_error():
|
||||
assert sb.is_dead is False
|
||||
|
||||
|
||||
def test_execute_command_forwards_env_and_timeout_to_commands_run():
|
||||
"""execute_command(env=..., timeout=...) routes env as ``envs`` and the
|
||||
timeout through to ``commands.run`` so request-scoped secrets (#3861) reach
|
||||
the e2b subprocess without entering the command string. Regression for the
|
||||
signature mismatch that broke bash for every e2b user."""
|
||||
commands = MagicMock()
|
||||
commands.run.return_value = SimpleNamespace(stdout="ok\n", stderr="", exit_code=0)
|
||||
client = FakeClient(commands=commands)
|
||||
sb = _make_sandbox(client)
|
||||
|
||||
out = sb.execute_command("echo $TOK", env={"TOK": "secret-v"}, timeout=120)
|
||||
|
||||
assert out.rstrip() == "ok"
|
||||
args, kwargs = commands.run.call_args
|
||||
assert args == ("echo $TOK",)
|
||||
assert kwargs["envs"] == {"TOK": "secret-v"}
|
||||
assert kwargs["timeout"] == 120
|
||||
# The secret must not be smuggled into the command string.
|
||||
assert "secret-v" not in args[0]
|
||||
|
||||
|
||||
def test_execute_command_env_none_passes_no_envs_kwarg():
|
||||
"""env=None is fully backward-compatible — ``commands.run`` is called with no
|
||||
``envs``/``timeout`` kwargs, so existing (non-secret) callers are unaffected."""
|
||||
commands = MagicMock()
|
||||
commands.run.return_value = SimpleNamespace(stdout="ok\n", stderr="", exit_code=0)
|
||||
client = FakeClient(commands=commands)
|
||||
sb = _make_sandbox(client)
|
||||
|
||||
sb.execute_command("echo hi")
|
||||
|
||||
_, kwargs = commands.run.call_args
|
||||
assert "envs" not in kwargs
|
||||
assert "timeout" not in kwargs
|
||||
|
||||
|
||||
def test_ping_returns_false_when_sandbox_gone():
|
||||
client = FakeClient(commands=FakeCommandsAPI([FakeCommandsAPI.GONE]))
|
||||
sb = _make_sandbox(client)
|
||||
|
||||
@ -208,6 +208,21 @@ def test_build_run_config_with_overrides():
|
||||
assert config["metadata"]["user"] == "alice"
|
||||
|
||||
|
||||
def test_build_run_config_context_path_still_sets_configurable_thread_id(_stub_app_config):
|
||||
"""A caller-supplied context (e.g. request-scoped secrets, #3861) must not
|
||||
deprive the checkpointer of configurable.thread_id, which it always needs to
|
||||
scope checkpoints. Secrets stay in context; thread_id is mirrored into
|
||||
configurable for the checkpointer."""
|
||||
from app.gateway.services import build_run_config
|
||||
|
||||
config = build_run_config("thread-1", {"context": {"secrets": {"ERP_TOKEN": "v"}}}, None)
|
||||
assert config["context"]["secrets"] == {"ERP_TOKEN": "v"}
|
||||
assert config["context"]["thread_id"] == "thread-1"
|
||||
assert config["configurable"]["thread_id"] == "thread-1"
|
||||
# Secrets must NOT be mirrored into configurable.
|
||||
assert "secrets" not in config["configurable"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# recursion_limit clamping: the Gateway must not trust a client-supplied
|
||||
# recursion_limit verbatim (runaway LLM cost / DoS). See build_run_config.
|
||||
@ -853,7 +868,8 @@ def test_build_run_config_with_context():
|
||||
assert "context" in config
|
||||
assert config["context"]["user_id"] == "u-42"
|
||||
assert config["context"]["thread_id"] == "thread-1"
|
||||
assert "configurable" not in config
|
||||
# configurable carries thread_id for the checkpointer; user context stays in context.
|
||||
assert config["configurable"] == {"thread_id": "thread-1"}
|
||||
assert config["recursion_limit"] == 100
|
||||
|
||||
|
||||
@ -869,7 +885,7 @@ def test_build_run_config_context_injects_thread_id():
|
||||
assert config["context"]["user_id"] == "u-1"
|
||||
assert config["context"]["thinking_enabled"] is True
|
||||
assert config["context"]["thread_id"] == "T-deadbeef-42"
|
||||
assert "configurable" not in config
|
||||
assert config["configurable"] == {"thread_id": "T-deadbeef-42"}
|
||||
|
||||
|
||||
def test_build_run_config_null_context_becomes_empty_context():
|
||||
@ -879,7 +895,7 @@ def test_build_run_config_null_context_becomes_empty_context():
|
||||
config = build_run_config("thread-1", {"context": None}, None)
|
||||
|
||||
assert config["context"] == {"thread_id": "thread-1"}
|
||||
assert "configurable" not in config
|
||||
assert config["configurable"] == {"thread_id": "thread-1"}
|
||||
|
||||
|
||||
def test_build_run_config_rejects_non_mapping_context():
|
||||
@ -921,7 +937,10 @@ def test_build_run_config_context_plus_configurable_warns(caplog):
|
||||
)
|
||||
assert "context" in config
|
||||
assert config["context"]["user_id"] == "u-42"
|
||||
assert "configurable" not in config
|
||||
# context wins: caller's configurable (model_name) is dropped, but thread_id is
|
||||
# still set for the checkpointer.
|
||||
assert config["configurable"] == {"thread_id": "thread-1"}
|
||||
assert "model_name" not in config["configurable"]
|
||||
assert any("both 'context' and 'configurable'" in r.message for r in caplog.records)
|
||||
|
||||
|
||||
@ -935,7 +954,7 @@ def test_build_run_config_context_passthrough_other_keys():
|
||||
None,
|
||||
)
|
||||
assert config["context"]["thread_id"] == "thread-1"
|
||||
assert "configurable" not in config
|
||||
assert config["configurable"] == {"thread_id": "thread-1"}
|
||||
assert config["tags"] == ["prod"]
|
||||
|
||||
|
||||
|
||||
@ -77,7 +77,7 @@ def test_foreground_blocking_command_times_out_with_notice():
|
||||
|
||||
def test_timeout_notice_formats_fractional_and_singular_timeouts(monkeypatch):
|
||||
monkeypatch.setattr(LocalSandbox, "_get_shell", lambda self: "/bin/sh")
|
||||
monkeypatch.setattr(LocalSandbox, "_run_posix_command", staticmethod(lambda args, timeout: ("", "", 0, True)))
|
||||
monkeypatch.setattr(LocalSandbox, "_run_posix_command", staticmethod(lambda args, timeout, env=None: ("", "", 0, True)))
|
||||
|
||||
assert "after 1.5 seconds" in LocalSandbox("t").execute_command("wait", timeout=1.5)
|
||||
assert "after 1 second" in LocalSandbox("t").execute_command("wait", timeout=1)
|
||||
|
||||
@ -86,12 +86,16 @@ def test_execute_command_uses_powershell_command_mode_on_windows(monkeypatch):
|
||||
return SimpleNamespace(stdout="ok", stderr="", returncode=0)
|
||||
|
||||
monkeypatch.setattr(local_sandbox.os, "name", "nt")
|
||||
monkeypatch.setattr(local_sandbox.os, "environ", {"PATH": r"C:\Windows", "OPENAI_API_KEY": "should-not-leak"})
|
||||
monkeypatch.setattr(LocalSandbox, "_get_shell", staticmethod(lambda: r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe"))
|
||||
monkeypatch.setattr(local_sandbox.subprocess, "run", fake_run)
|
||||
|
||||
output = LocalSandbox("t").execute_command("Write-Output hello")
|
||||
|
||||
assert output == "ok"
|
||||
# Platform secrets are scrubbed from the inherited environment even on the
|
||||
# Windows PowerShell path (#3861); benign PATH is preserved and the env is an
|
||||
# explicit scrubbed dict, no longer None.
|
||||
assert calls == [
|
||||
(
|
||||
[
|
||||
@ -105,7 +109,7 @@ def test_execute_command_uses_powershell_command_mode_on_windows(monkeypatch):
|
||||
"capture_output": True,
|
||||
"text": True,
|
||||
"timeout": 600,
|
||||
"env": None,
|
||||
"env": {"PATH": r"C:\Windows"},
|
||||
},
|
||||
)
|
||||
]
|
||||
@ -152,13 +156,17 @@ def test_execute_command_does_not_set_msys_env_for_non_msys_posix_shell_on_windo
|
||||
return SimpleNamespace(stdout="ok", stderr="", returncode=0)
|
||||
|
||||
monkeypatch.setattr(local_sandbox.os, "name", "nt")
|
||||
monkeypatch.setattr(local_sandbox.os, "environ", {"PATH": r"C:\tools"})
|
||||
monkeypatch.setattr(LocalSandbox, "_get_shell", staticmethod(lambda: r"C:\tools\busybox\sh.exe"))
|
||||
monkeypatch.setattr(local_sandbox.subprocess, "run", fake_run)
|
||||
|
||||
output = LocalSandbox("t").execute_command("echo /mnt/skills/demo")
|
||||
|
||||
assert output == "ok"
|
||||
assert calls[0][1]["env"] is None
|
||||
# Non-MSYS posix shell adds no MSYS_* vars; the env is the scrubbed inherited
|
||||
# environment, not None (#3861).
|
||||
assert calls[0][1]["env"] == {"PATH": r"C:\tools"}
|
||||
assert "MSYS_NO_PATHCONV" not in calls[0][1]["env"]
|
||||
|
||||
|
||||
def test_execute_command_uses_cmd_command_mode_on_windows(monkeypatch):
|
||||
@ -169,12 +177,15 @@ def test_execute_command_uses_cmd_command_mode_on_windows(monkeypatch):
|
||||
return SimpleNamespace(stdout="ok", stderr="", returncode=0)
|
||||
|
||||
monkeypatch.setattr(local_sandbox.os, "name", "nt")
|
||||
monkeypatch.setattr(local_sandbox.os, "environ", {"PATH": r"C:\Windows", "GITHUB_TOKEN": "should-not-leak"})
|
||||
monkeypatch.setattr(LocalSandbox, "_get_shell", staticmethod(lambda: r"C:\Windows\System32\cmd.exe"))
|
||||
monkeypatch.setattr(local_sandbox.subprocess, "run", fake_run)
|
||||
|
||||
output = LocalSandbox("t").execute_command("echo hello")
|
||||
|
||||
assert output == "ok"
|
||||
# Platform secrets are scrubbed even on the Windows cmd path (#3861); the env
|
||||
# is an explicit scrubbed dict, no longer None.
|
||||
assert calls == [
|
||||
(
|
||||
[r"C:\Windows\System32\cmd.exe", "/c", "echo hello"],
|
||||
@ -183,7 +194,7 @@ def test_execute_command_uses_cmd_command_mode_on_windows(monkeypatch):
|
||||
"capture_output": True,
|
||||
"text": True,
|
||||
"timeout": 600,
|
||||
"env": None,
|
||||
"env": {"PATH": r"C:\Windows"},
|
||||
},
|
||||
)
|
||||
]
|
||||
|
||||
@ -37,7 +37,13 @@ class _SyncProvider(SandboxProvider):
|
||||
|
||||
|
||||
class _SandboxStub(Sandbox):
|
||||
def execute_command(self, command: str) -> str:
|
||||
def execute_command(
|
||||
self,
|
||||
command: str,
|
||||
env: dict[str, str] | None = None,
|
||||
timeout: float | None = None,
|
||||
) -> str:
|
||||
del env, timeout
|
||||
return "OK"
|
||||
|
||||
def read_file(self, path: str) -> str:
|
||||
|
||||
769
backend/tests/test_skill_request_scoped_secrets.py
Normal file
769
backend/tests/test_skill_request_scoped_secrets.py
Normal file
@ -0,0 +1,769 @@
|
||||
"""Tests for request-scoped secret injection into skills (issue #3861).
|
||||
|
||||
Covers the full feature surface:
|
||||
- Slice 1: ``Sandbox.execute_command(command, env=...)`` per-call env injection
|
||||
on both the local and AIO backends.
|
||||
- Slice 2: ``SKILL.md`` ``requires-secrets`` frontmatter parsing.
|
||||
- Slice 3: gateway carrier (``context.secrets``) and runtime-context passthrough.
|
||||
- Slice 4: activation-turn binding + ``bash`` tool injection.
|
||||
- Slice 5: the five leak surfaces (prompt / trace / checkpoint / audit / stdout).
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from langchain.agents.middleware.types import ModelRequest
|
||||
from langchain_core.messages import AIMessage, HumanMessage
|
||||
|
||||
from deerflow.sandbox.local.local_sandbox import LocalSandbox
|
||||
from deerflow.skills.types import SecretRequirement, Skill, SkillCategory
|
||||
|
||||
|
||||
class TestLocalSandboxEnvInjection:
|
||||
"""LocalSandbox.execute_command(env=...) injects per-call env into the subprocess."""
|
||||
|
||||
def test_injected_env_visible_to_command(self):
|
||||
sandbox = LocalSandbox(id="local")
|
||||
out = sandbox.execute_command(
|
||||
"echo $DEERFLOW_TEST_SECRET",
|
||||
env={"DEERFLOW_TEST_SECRET": "s3cret-value"},
|
||||
)
|
||||
assert "s3cret-value" in out
|
||||
|
||||
def test_env_none_keeps_inherited_environment(self, monkeypatch):
|
||||
"""env=None preserves the legacy inherited-os.environ behaviour."""
|
||||
monkeypatch.setenv("DEERFLOW_INHERITED_VAR", "inherited-value")
|
||||
sandbox = LocalSandbox(id="local")
|
||||
out = sandbox.execute_command("echo $DEERFLOW_INHERITED_VAR")
|
||||
assert "inherited-value" in out
|
||||
|
||||
def test_injected_env_is_per_call_only(self):
|
||||
"""Injected env must not leak into a subsequent call that does not pass it."""
|
||||
sandbox = LocalSandbox(id="local")
|
||||
sandbox.execute_command("true", env={"DEERFLOW_EPHEMERAL": "leaky"})
|
||||
out = sandbox.execute_command("echo [$DEERFLOW_EPHEMERAL]")
|
||||
assert "leaky" not in out
|
||||
|
||||
def test_platform_secret_scrubbed_from_inherited_env(self, monkeypatch):
|
||||
"""A platform credential present in os.environ must NOT reach the sandbox
|
||||
subprocess (the baseline-env leak surface). Without this, scoped injection
|
||||
is security theatre — a skill script could simply read $OPENAI_API_KEY."""
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "sk-platform-should-not-leak")
|
||||
sandbox = LocalSandbox(id="local")
|
||||
out = sandbox.execute_command("echo [$OPENAI_API_KEY]")
|
||||
assert "sk-platform-should-not-leak" not in out
|
||||
|
||||
def test_benign_env_still_inherited_after_scrub(self, monkeypatch):
|
||||
"""Scrubbing platform secrets must not strip harmless vars that skills rely on."""
|
||||
monkeypatch.setenv("DEERFLOW_PLAIN_VAR", "harmless-value")
|
||||
sandbox = LocalSandbox(id="local")
|
||||
out = sandbox.execute_command("echo [$DEERFLOW_PLAIN_VAR]")
|
||||
assert "harmless-value" in out
|
||||
|
||||
def test_injected_secret_survives_scrub(self, monkeypatch):
|
||||
"""An explicitly injected secret must win even if its name matches a blocked
|
||||
pattern — injection happens after scrubbing the inherited environment."""
|
||||
sandbox = LocalSandbox(id="local")
|
||||
out = sandbox.execute_command(
|
||||
"echo [$INJECTED_API_KEY]",
|
||||
env={"INJECTED_API_KEY": "scoped-value"},
|
||||
)
|
||||
assert "scoped-value" in out
|
||||
|
||||
|
||||
class TestAioSandboxEnvInjection:
|
||||
@pytest.fixture
|
||||
def sandbox(self):
|
||||
with patch("deerflow.community.aio_sandbox.aio_sandbox.AioSandboxClient"):
|
||||
from deerflow.community.aio_sandbox.aio_sandbox import AioSandbox
|
||||
|
||||
return AioSandbox(id="test-sandbox", base_url="http://localhost:8080")
|
||||
|
||||
def test_env_none_uses_legacy_shell_path(self, sandbox):
|
||||
"""No injected env → unchanged shell.exec_command path (backward compat)."""
|
||||
sandbox._client.shell.exec_command = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(output="hello")))
|
||||
sandbox._client.bash.exec = MagicMock()
|
||||
out = sandbox.execute_command("echo hello")
|
||||
sandbox._client.shell.exec_command.assert_called_once()
|
||||
sandbox._client.bash.exec.assert_not_called()
|
||||
assert "hello" in out
|
||||
|
||||
def test_injected_env_uses_bash_exec_with_env_dict(self, sandbox):
|
||||
"""Injected env → bash.exec(env=...) carries the dict; secret stays out of the command string."""
|
||||
sandbox._client.bash.exec = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(stdout="hello", stderr=None)))
|
||||
sandbox._client.shell.exec_command = MagicMock()
|
||||
out = sandbox.execute_command("echo $TOK", env={"TOK": "secret-v"})
|
||||
sandbox._client.bash.exec.assert_called_once()
|
||||
_, kwargs = sandbox._client.bash.exec.call_args
|
||||
assert kwargs["env"] == {"TOK": "secret-v"}
|
||||
# Secret must NOT be smuggled into the command string (audit / ps safety).
|
||||
assert "secret-v" not in kwargs["command"]
|
||||
sandbox._client.shell.exec_command.assert_not_called()
|
||||
assert "hello" in out
|
||||
|
||||
def test_env_path_uses_hard_timeout_not_no_change_timeout(self, sandbox):
|
||||
"""The env path routes through bash.exec which exposes no idle/no-change
|
||||
timeout; it must use the dedicated wall-clock ``_DEFAULT_HARD_TIMEOUT``,
|
||||
not the legacy idle constant (same numeric value today, but distinct
|
||||
semantics so a future change to one does not silently alter the other)."""
|
||||
from deerflow.community.aio_sandbox.aio_sandbox import AioSandbox
|
||||
|
||||
sandbox._client.bash.exec = MagicMock(return_value=SimpleNamespace(data=SimpleNamespace(stdout="ok", stderr=None)))
|
||||
sandbox.execute_command("echo hi", env={"X": "1"})
|
||||
_, kwargs = sandbox._client.bash.exec.call_args
|
||||
assert kwargs["hard_timeout"] == AioSandbox._DEFAULT_HARD_TIMEOUT
|
||||
assert AioSandbox._DEFAULT_HARD_TIMEOUT != AioSandbox._DEFAULT_NO_CHANGE_TIMEOUT or (
|
||||
# Same numeric value is fine today; the contract is that they are
|
||||
# named independently so the two call sites evolve independently.
|
||||
AioSandbox._DEFAULT_HARD_TIMEOUT == AioSandbox._DEFAULT_NO_CHANGE_TIMEOUT
|
||||
)
|
||||
|
||||
def test_env_path_retries_on_error_observation_signature(self, sandbox):
|
||||
"""The env path shares the legacy persistent-shell recovery contract: if
|
||||
the (unlikely, fresh-session) corruption marker appears, the call is
|
||||
retried rather than returned verbatim."""
|
||||
from deerflow.community.aio_sandbox.aio_sandbox import _ERROR_OBSERVATION_SIGNATURE
|
||||
|
||||
corrupted = SimpleNamespace(data=SimpleNamespace(stdout=_ERROR_OBSERVATION_SIGNATURE, stderr=None))
|
||||
clean = SimpleNamespace(data=SimpleNamespace(stdout="recovered", stderr=None))
|
||||
sandbox._client.bash.exec = MagicMock(side_effect=[corrupted, clean])
|
||||
out = sandbox.execute_command("script", env={"TOK": "v"})
|
||||
assert sandbox._client.bash.exec.call_count == 2
|
||||
assert "recovered" in out
|
||||
assert _ERROR_OBSERVATION_SIGNATURE not in out
|
||||
|
||||
|
||||
class TestEnvPolicy:
|
||||
"""Platform-secret scrubbing policy for sandbox subprocesses (delta 1)."""
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"name",
|
||||
[
|
||||
"OPENAI_API_KEY",
|
||||
"ANTHROPIC_API_KEY",
|
||||
"LANGFUSE_SECRET_KEY",
|
||||
"GITHUB_TOKEN",
|
||||
"AWS_SECRET_ACCESS_KEY",
|
||||
"DB_PASSWORD",
|
||||
"MY_SERVICE_CREDENTIAL",
|
||||
"api_key",
|
||||
"Some_Token_Here",
|
||||
# Connection-string credentials (no KEY/SECRET/TOKEN substring) — these
|
||||
# routinely embed a password, e.g. postgresql://user:pw@host/db.
|
||||
"DATABASE_URL",
|
||||
"REDIS_URL",
|
||||
"MONGODB_URI",
|
||||
"AMQP_URL",
|
||||
"SENTRY_DSN",
|
||||
"POSTGRES_DSN",
|
||||
"CONN_STR",
|
||||
"GH_PAT",
|
||||
],
|
||||
)
|
||||
def test_secret_like_names_are_blocked(self, name):
|
||||
from deerflow.sandbox.env_policy import is_blocked_env_name
|
||||
|
||||
assert is_blocked_env_name(name) is True
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"name",
|
||||
[
|
||||
"PATH",
|
||||
"HOME",
|
||||
"SHELL",
|
||||
"USER",
|
||||
"LANG",
|
||||
"LC_ALL",
|
||||
"PWD",
|
||||
"TMPDIR",
|
||||
"VIRTUAL_ENV",
|
||||
"PYTHONPATH",
|
||||
"DEERFLOW_PLAIN_VAR",
|
||||
# Not a blanket *URL* block: a benign service URL a skill may legitimately
|
||||
# read is not treated as a credential.
|
||||
"NEXT_PUBLIC_BASE_URL",
|
||||
"SERVICE_ENDPOINT",
|
||||
],
|
||||
)
|
||||
def test_benign_names_are_allowed(self, name):
|
||||
from deerflow.sandbox.env_policy import is_blocked_env_name
|
||||
|
||||
assert is_blocked_env_name(name) is False
|
||||
|
||||
def test_build_sandbox_env_scrubs_inherited_and_layers_injected(self, monkeypatch):
|
||||
from deerflow.sandbox.env_policy import build_sandbox_env
|
||||
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "platform-key-should-vanish")
|
||||
monkeypatch.setenv("HARMLESS_PLAIN", "ok")
|
||||
env = build_sandbox_env(injected={"SCOPED_TOKEN": "v"})
|
||||
assert "OPENAI_API_KEY" not in env # platform secret scrubbed
|
||||
assert env.get("HARMLESS_PLAIN") == "ok" # benign preserved
|
||||
assert env.get("SCOPED_TOKEN") == "v" # injected layered on top
|
||||
assert env.get("PATH") # core var preserved
|
||||
|
||||
def test_build_sandbox_env_none_injection_still_scrubs(self, monkeypatch):
|
||||
from deerflow.sandbox.env_policy import build_sandbox_env
|
||||
|
||||
monkeypatch.setenv("ANTHROPIC_API_KEY", "leak")
|
||||
env = build_sandbox_env()
|
||||
assert "ANTHROPIC_API_KEY" not in env
|
||||
|
||||
|
||||
class TestRequiredSecretsParsing:
|
||||
"""SKILL.md ``required-secrets`` frontmatter parsing (Slice 2)."""
|
||||
|
||||
def _write_skill(self, tmp_path, frontmatter_body: str):
|
||||
skill_dir = tmp_path / "erp-report"
|
||||
skill_dir.mkdir()
|
||||
skill_file = skill_dir / "SKILL.md"
|
||||
skill_file.write_text(f"---\n{frontmatter_body}\n---\n# body\n", encoding="utf-8")
|
||||
return skill_file
|
||||
|
||||
def test_absent_field_defaults_to_empty(self, tmp_path):
|
||||
from deerflow.skills.parser import parse_skill_file
|
||||
from deerflow.skills.types import SkillCategory
|
||||
|
||||
skill_file = self._write_skill(tmp_path, "name: erp-report\ndescription: Pull an ERP report")
|
||||
skill = parse_skill_file(skill_file, SkillCategory.CUSTOM)
|
||||
assert skill is not None
|
||||
assert skill.required_secrets == []
|
||||
|
||||
def test_string_list_form(self, tmp_path):
|
||||
from deerflow.skills.parser import parse_skill_file
|
||||
from deerflow.skills.types import SkillCategory
|
||||
|
||||
skill_file = self._write_skill(
|
||||
tmp_path,
|
||||
"name: erp-report\ndescription: d\nrequired-secrets:\n - ERP_TOKEN\n - OTHER_TOKEN",
|
||||
)
|
||||
skill = parse_skill_file(skill_file, SkillCategory.CUSTOM)
|
||||
assert [s.name for s in skill.required_secrets] == ["ERP_TOKEN", "OTHER_TOKEN"]
|
||||
assert all(s.optional is False for s in skill.required_secrets)
|
||||
|
||||
def test_object_list_with_optional(self, tmp_path):
|
||||
from deerflow.skills.parser import parse_skill_file
|
||||
from deerflow.skills.types import SkillCategory
|
||||
|
||||
skill_file = self._write_skill(
|
||||
tmp_path,
|
||||
"name: erp-report\ndescription: d\nrequired-secrets:\n - name: ERP_TOKEN\n optional: true\n - name: REQUIRED_ONE",
|
||||
)
|
||||
skill = parse_skill_file(skill_file, SkillCategory.CUSTOM)
|
||||
by_name = {s.name: s for s in skill.required_secrets}
|
||||
assert by_name["ERP_TOKEN"].optional is True
|
||||
assert by_name["REQUIRED_ONE"].optional is False
|
||||
|
||||
def test_invalid_env_name_entry_is_dropped(self, tmp_path):
|
||||
from deerflow.skills.parser import parse_skill_file
|
||||
from deerflow.skills.types import SkillCategory
|
||||
|
||||
skill_file = self._write_skill(
|
||||
tmp_path,
|
||||
'name: erp-report\ndescription: d\nrequired-secrets:\n - "bad name!"\n - GOOD_TOKEN',
|
||||
)
|
||||
skill = parse_skill_file(skill_file, SkillCategory.CUSTOM)
|
||||
# The malformed entry is dropped; the valid one survives — one bad
|
||||
# declaration must not nuke the whole skill.
|
||||
assert [s.name for s in skill.required_secrets] == ["GOOD_TOKEN"]
|
||||
|
||||
|
||||
class TestSecretCarrier:
|
||||
"""Request-scoped secret carrier: context.secrets → runtime.context (Slice 3)."""
|
||||
|
||||
def test_build_run_config_keeps_secrets_in_context_not_configurable(self):
|
||||
from app.gateway.services import build_run_config
|
||||
|
||||
config = build_run_config("thread-1", {"context": {"secrets": {"ERP_TOKEN": "v"}}}, None)
|
||||
assert config["context"]["secrets"] == {"ERP_TOKEN": "v"}
|
||||
# Secrets must never be mirrored into configurable (which legacy readers
|
||||
# and some trace backends surface).
|
||||
assert "secrets" not in config.get("configurable", {})
|
||||
|
||||
def test_runtime_context_carries_secrets(self):
|
||||
from deerflow.runtime.runs.worker import _build_runtime_context
|
||||
|
||||
ctx = _build_runtime_context("t", "r", {"secrets": {"ERP_TOKEN": "v"}})
|
||||
assert ctx["secrets"] == {"ERP_TOKEN": "v"}
|
||||
|
||||
def test_extract_request_secrets_filters_non_string_pairs(self):
|
||||
from deerflow.runtime.secret_context import extract_request_secrets
|
||||
|
||||
assert extract_request_secrets({"secrets": {"A": "x", "B": 123, 4: "y"}}) == {"A": "x"}
|
||||
|
||||
def test_extract_request_secrets_missing_or_malformed(self):
|
||||
from deerflow.runtime.secret_context import extract_request_secrets
|
||||
|
||||
assert extract_request_secrets({}) == {}
|
||||
assert extract_request_secrets({"secrets": "not-a-dict"}) == {}
|
||||
assert extract_request_secrets(None) == {}
|
||||
|
||||
|
||||
def _make_secret_skill(tmp_path: Path, name: str, required_secrets):
|
||||
skill_dir = tmp_path / name
|
||||
skill_dir.mkdir()
|
||||
skill_file = skill_dir / "SKILL.md"
|
||||
skill_file.write_text(f"# {name}\n", encoding="utf-8")
|
||||
return Skill(
|
||||
name=name,
|
||||
description=f"Description for {name}",
|
||||
license="MIT",
|
||||
skill_dir=skill_dir,
|
||||
skill_file=skill_file,
|
||||
relative_path=Path(name),
|
||||
category=SkillCategory.CUSTOM,
|
||||
enabled=True,
|
||||
required_secrets=required_secrets,
|
||||
)
|
||||
|
||||
|
||||
class TestActivationBindsSecrets:
|
||||
"""Binding point A: activation turn resolves declared secrets into the per-run injection set."""
|
||||
|
||||
def _activate(self, tmp_path, monkeypatch, skill, context):
|
||||
from deerflow.agents.middlewares import skill_activation_middleware as mw
|
||||
from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware
|
||||
|
||||
storage = SimpleNamespace(
|
||||
load_skills=lambda *, enabled_only: [skill],
|
||||
get_container_root=lambda: "/mnt/skills",
|
||||
get_skills_root_path=lambda: tmp_path,
|
||||
)
|
||||
monkeypatch.setattr(mw, "get_or_new_skill_storage", lambda **kwargs: storage)
|
||||
middleware = SkillActivationMiddleware()
|
||||
request = ModelRequest(
|
||||
model=object(),
|
||||
messages=[HumanMessage(content=f"/{skill.name} do it", id="m1")],
|
||||
state={"messages": []},
|
||||
runtime=SimpleNamespace(context=context),
|
||||
)
|
||||
middleware.wrap_model_call(request, lambda r: AIMessage(content="ok"))
|
||||
|
||||
def test_declared_secret_resolved_into_active_set(self, tmp_path, monkeypatch):
|
||||
from deerflow.runtime.secret_context import read_active_secrets
|
||||
|
||||
skill = _make_secret_skill(tmp_path, "erp-report", [SecretRequirement("ERP_TOKEN")])
|
||||
context = {"secrets": {"ERP_TOKEN": "tok-123", "UNUSED": "x"}}
|
||||
self._activate(tmp_path, monkeypatch, skill, context)
|
||||
# Only the declared secret is injected — not the whole secrets bag.
|
||||
assert read_active_secrets(context) == {"ERP_TOKEN": "tok-123"}
|
||||
|
||||
def test_skill_without_declaration_gets_no_injection(self, tmp_path, monkeypatch):
|
||||
from deerflow.runtime.secret_context import read_active_secrets
|
||||
|
||||
skill = _make_secret_skill(tmp_path, "plain", [])
|
||||
context = {"secrets": {"ERP_TOKEN": "tok-123"}}
|
||||
self._activate(tmp_path, monkeypatch, skill, context)
|
||||
assert read_active_secrets(context) == {}
|
||||
|
||||
def test_missing_required_secret_not_injected(self, tmp_path, monkeypatch):
|
||||
from deerflow.runtime.secret_context import read_active_secrets
|
||||
|
||||
skill = _make_secret_skill(tmp_path, "erp-report", [SecretRequirement("ERP_TOKEN")])
|
||||
context = {"secrets": {}} # caller provided none
|
||||
self._activate(tmp_path, monkeypatch, skill, context)
|
||||
assert read_active_secrets(context) == {}
|
||||
|
||||
def test_caller_secret_wins_over_host_value_of_same_name(self, tmp_path, monkeypatch):
|
||||
"""A skill may declare a name that also exists in the host env (e.g. a
|
||||
per-user key overriding a shared platform key — the #3861 use case). The
|
||||
skill receives the CALLER's value (from context.secrets), never the host's:
|
||||
the inherited host value is scrubbed and the caller's value is injected on
|
||||
top. There is therefore no host-credential harvest to guard against."""
|
||||
from deerflow.runtime.secret_context import read_active_secrets
|
||||
from deerflow.sandbox.env_policy import build_sandbox_env
|
||||
|
||||
monkeypatch.setenv("MEMOS_API_KEY", "host-shared-key-MUST-NOT-LEAK")
|
||||
skill = _make_secret_skill(tmp_path, "memos", [SecretRequirement("MEMOS_API_KEY")])
|
||||
context = {"secrets": {"MEMOS_API_KEY": "caller-per-user-key"}}
|
||||
self._activate(tmp_path, monkeypatch, skill, context)
|
||||
|
||||
injected = read_active_secrets(context)
|
||||
assert injected == {"MEMOS_API_KEY": "caller-per-user-key"} # caller's value injected
|
||||
|
||||
# The subprocess env gets the caller's value; the host's value is scrubbed.
|
||||
env = build_sandbox_env(injected)
|
||||
assert env["MEMOS_API_KEY"] == "caller-per-user-key"
|
||||
assert "host-shared-key-MUST-NOT-LEAK" not in str(env.values())
|
||||
|
||||
def test_undeclared_host_secret_is_scrubbed_not_harvested(self, tmp_path, monkeypatch):
|
||||
"""If a skill does NOT declare a host credential, the inherited value is
|
||||
scrubbed — a skill can never read a platform credential it wasn't given."""
|
||||
from deerflow.sandbox.env_policy import build_sandbox_env
|
||||
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "host-key-do-not-harvest")
|
||||
env = build_sandbox_env(None)
|
||||
assert "OPENAI_API_KEY" not in env
|
||||
|
||||
def test_activation_fires_after_input_sanitization_wrapping(self, tmp_path, monkeypatch):
|
||||
"""Integration: in the real chain InputSanitizationMiddleware wraps the user
|
||||
message in ``--- BEGIN USER INPUT ---`` markers before SkillActivationMiddleware
|
||||
sees it. Slash activation (and therefore secret resolution) must still fire — it
|
||||
relies on the original content being recoverable. Regression for the gateway
|
||||
path where no upload preserved it."""
|
||||
from deerflow.agents.middlewares import skill_activation_middleware as mw
|
||||
from deerflow.agents.middlewares.input_sanitization_middleware import InputSanitizationMiddleware
|
||||
from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware
|
||||
from deerflow.config.app_config import AppConfig, reset_app_config, set_app_config
|
||||
from deerflow.runtime.secret_context import read_active_secrets
|
||||
|
||||
skill = _make_secret_skill(tmp_path, "erp-report", [SecretRequirement("ERP_TOKEN")])
|
||||
storage = SimpleNamespace(
|
||||
load_skills=lambda *, enabled_only: [skill],
|
||||
get_container_root=lambda: "/mnt/skills",
|
||||
get_skills_root_path=lambda: tmp_path,
|
||||
)
|
||||
monkeypatch.setattr(mw, "get_or_new_skill_storage", lambda **kwargs: storage)
|
||||
|
||||
context = {"secrets": {"ERP_TOKEN": "tok-xyz"}}
|
||||
request = ModelRequest(
|
||||
model=object(),
|
||||
messages=[HumanMessage(content="/erp-report pull it", id="m1")],
|
||||
state={"messages": []},
|
||||
runtime=SimpleNamespace(context=context),
|
||||
)
|
||||
# The sanitizer loads enabled skills during wrap, so keep a stub app config
|
||||
# in place for the whole composed call.
|
||||
set_app_config(AppConfig.model_validate({"sandbox": {"use": "deerflow.sandbox.local:LocalSandboxProvider"}}))
|
||||
try:
|
||||
sanitizer = InputSanitizationMiddleware()
|
||||
skill_mw = SkillActivationMiddleware()
|
||||
|
||||
# Compose in real order: sanitizer (outer) -> skill activation (inner) -> model.
|
||||
def skill_layer(req):
|
||||
return skill_mw.wrap_model_call(req, lambda r: AIMessage(content="ok"))
|
||||
|
||||
sanitizer.wrap_model_call(request, skill_layer)
|
||||
finally:
|
||||
reset_app_config()
|
||||
|
||||
assert read_active_secrets(context) == {"ERP_TOKEN": "tok-xyz"}
|
||||
|
||||
def test_prior_activation_secrets_cleared_when_next_skill_declares_none(self, tmp_path, monkeypatch):
|
||||
"""A later skill in the same run never inherits an earlier skill's secrets.
|
||||
Turn 1 activates /skill-a (declares A_TOKEN, caller supplies it) → injected.
|
||||
Turn 2 activates /skill-b (declares nothing) → A_TOKEN must be cleared so
|
||||
bash in skill-b's turn cannot receive a value it never declared."""
|
||||
from deerflow.agents.middlewares import skill_activation_middleware as mw
|
||||
from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware
|
||||
from deerflow.runtime.secret_context import read_active_secrets
|
||||
|
||||
skill_a = _make_secret_skill(tmp_path, "skill-a", [SecretRequirement("A_TOKEN")])
|
||||
skill_b = _make_secret_skill(tmp_path, "skill-b", [])
|
||||
|
||||
def _storage(skills):
|
||||
return SimpleNamespace(
|
||||
load_skills=lambda *, enabled_only: skills,
|
||||
get_container_root=lambda: "/mnt/skills",
|
||||
get_skills_root_path=lambda: tmp_path,
|
||||
)
|
||||
|
||||
context = {"secrets": {"A_TOKEN": "v-a"}}
|
||||
|
||||
monkeypatch.setattr(mw, "get_or_new_skill_storage", lambda **kwargs: _storage([skill_a]))
|
||||
SkillActivationMiddleware().wrap_model_call(
|
||||
ModelRequest(
|
||||
model=object(),
|
||||
messages=[HumanMessage(content="/skill-a go", id="m1")],
|
||||
state={"messages": []},
|
||||
runtime=SimpleNamespace(context=context),
|
||||
),
|
||||
lambda r: AIMessage(content="ok"),
|
||||
)
|
||||
assert read_active_secrets(context) == {"A_TOKEN": "v-a"}
|
||||
|
||||
monkeypatch.setattr(mw, "get_or_new_skill_storage", lambda **kwargs: _storage([skill_b]))
|
||||
SkillActivationMiddleware().wrap_model_call(
|
||||
ModelRequest(
|
||||
model=object(),
|
||||
messages=[HumanMessage(content="/skill-b go", id="m2")],
|
||||
state={"messages": []},
|
||||
runtime=SimpleNamespace(context=context),
|
||||
),
|
||||
lambda r: AIMessage(content="ok"),
|
||||
)
|
||||
assert read_active_secrets(context) == {}
|
||||
|
||||
def test_prior_activation_secrets_cleared_when_caller_omits_required(self, tmp_path, monkeypatch):
|
||||
"""Even when the next skill DOES declare a required secret, if the caller
|
||||
omits it the prior skill's value must not linger — the injection set ends
|
||||
up empty, not stale."""
|
||||
from deerflow.agents.middlewares import skill_activation_middleware as mw
|
||||
from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware
|
||||
from deerflow.runtime.secret_context import read_active_secrets
|
||||
|
||||
skill = _make_secret_skill(tmp_path, "erp", [SecretRequirement("ERP_TOKEN")])
|
||||
storage = SimpleNamespace(
|
||||
load_skills=lambda *, enabled_only: [skill],
|
||||
get_container_root=lambda: "/mnt/skills",
|
||||
get_skills_root_path=lambda: tmp_path,
|
||||
)
|
||||
monkeypatch.setattr(mw, "get_or_new_skill_storage", lambda **kwargs: storage)
|
||||
|
||||
# Turn 1: caller supplies ERP_TOKEN → injected.
|
||||
context = {"secrets": {"ERP_TOKEN": "tok-1"}}
|
||||
mw_inst = SkillActivationMiddleware()
|
||||
mw_inst.wrap_model_call(
|
||||
ModelRequest(
|
||||
model=object(),
|
||||
messages=[HumanMessage(content="/erp go", id="m1")],
|
||||
state={"messages": []},
|
||||
runtime=SimpleNamespace(context=context),
|
||||
),
|
||||
lambda r: AIMessage(content="ok"),
|
||||
)
|
||||
assert read_active_secrets(context) == {"ERP_TOKEN": "tok-1"}
|
||||
|
||||
# Turn 2: caller omits ERP_TOKEN → prior value cleared, set empty (not stale).
|
||||
context2 = {"secrets": {}}
|
||||
mw_inst.wrap_model_call(
|
||||
ModelRequest(
|
||||
model=object(),
|
||||
messages=[HumanMessage(content="/erp again", id="m2")],
|
||||
state={"messages": []},
|
||||
runtime=SimpleNamespace(context=context2),
|
||||
),
|
||||
lambda r: AIMessage(content="ok"),
|
||||
)
|
||||
assert read_active_secrets(context2) == {}
|
||||
|
||||
|
||||
class TestBashToolInjectsActiveSecrets:
|
||||
"""The bash tool forwards the per-run injection set to execute_command(env=...)."""
|
||||
|
||||
def _run_bash(self, context):
|
||||
from deerflow.sandbox import tools as tools_mod
|
||||
|
||||
captured = {}
|
||||
|
||||
class FakeSandbox:
|
||||
def execute_command(self, command, env=None, timeout=None):
|
||||
captured["env"] = env
|
||||
captured["timeout"] = timeout
|
||||
return "done"
|
||||
|
||||
runtime = SimpleNamespace(context=context, state={"sandbox": {"sandbox_id": "aio:1"}})
|
||||
with (
|
||||
patch.object(tools_mod, "ensure_sandbox_initialized", return_value=FakeSandbox()),
|
||||
patch.object(tools_mod, "is_local_sandbox", return_value=False),
|
||||
patch.object(tools_mod, "ensure_thread_directories_exist", return_value=None),
|
||||
):
|
||||
out = tools_mod.bash_tool.func(runtime, "run skill", "echo hi")
|
||||
return out, captured
|
||||
|
||||
def test_active_secret_forwarded_as_env(self):
|
||||
out, captured = self._run_bash({"__active_skill_secrets": {"ERP_TOKEN": "tok-123"}})
|
||||
assert captured["env"] == {"ERP_TOKEN": "tok-123"}
|
||||
assert "done" in out
|
||||
|
||||
def test_no_active_secret_forwards_no_env(self):
|
||||
out, captured = self._run_bash({})
|
||||
assert captured["env"] in (None, {})
|
||||
|
||||
def test_local_bash_forwards_env_and_timeout(self, monkeypatch):
|
||||
from deerflow.sandbox import tools as tools_mod
|
||||
|
||||
captured = {}
|
||||
|
||||
class FakeSandbox:
|
||||
def execute_command(self, command, env=None, timeout=None):
|
||||
captured["command"] = command
|
||||
captured["env"] = env
|
||||
captured["timeout"] = timeout
|
||||
return "done"
|
||||
|
||||
runtime = SimpleNamespace(
|
||||
context={"__active_skill_secrets": {"ERP_TOKEN": "tok-456"}},
|
||||
state={"sandbox": {"sandbox_id": "local:1"}},
|
||||
)
|
||||
thread_data = {"workspace_path": "/tmp/ws", "cwd": "/mnt/user-data/workspace"}
|
||||
fake_cfg = SimpleNamespace(sandbox=SimpleNamespace(bash_output_max_chars=321, bash_command_timeout=42))
|
||||
with (
|
||||
patch.object(tools_mod, "ensure_sandbox_initialized", return_value=FakeSandbox()),
|
||||
patch.object(tools_mod, "is_local_sandbox", return_value=True),
|
||||
patch.object(tools_mod, "is_host_bash_allowed", return_value=True),
|
||||
patch.object(tools_mod, "ensure_thread_directories_exist", return_value=None),
|
||||
patch.object(tools_mod, "get_thread_data", return_value=thread_data),
|
||||
patch.object(tools_mod, "validate_local_bash_command_paths", return_value=None),
|
||||
patch.object(tools_mod, "replace_virtual_paths_in_command", side_effect=lambda command, td: command),
|
||||
patch.object(tools_mod, "_apply_cwd_prefix", side_effect=lambda command, td: command),
|
||||
patch("deerflow.config.app_config.get_app_config", return_value=fake_cfg),
|
||||
):
|
||||
out = tools_mod.bash_tool.func(runtime, "run local skill", "echo hi")
|
||||
|
||||
assert out == "done"
|
||||
assert captured["command"] == "echo hi"
|
||||
assert captured["env"] == {"ERP_TOKEN": "tok-456"}
|
||||
assert captured["timeout"] == 42
|
||||
|
||||
|
||||
_SECRET = "sk-erp-9f3c-DO-NOT-LEAK"
|
||||
|
||||
|
||||
class TestLeakSurfaces:
|
||||
"""Assert the secret value is absent from all five leak surfaces (#3861)."""
|
||||
|
||||
def _activate_with_secret(self, tmp_path, monkeypatch):
|
||||
from deerflow.agents.middlewares import skill_activation_middleware as mw
|
||||
from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware
|
||||
|
||||
skill = _make_secret_skill(tmp_path, "erp-report", [SecretRequirement("ERP_TOKEN")])
|
||||
storage = SimpleNamespace(
|
||||
load_skills=lambda *, enabled_only: [skill],
|
||||
get_container_root=lambda: "/mnt/skills",
|
||||
get_skills_root_path=lambda: tmp_path,
|
||||
)
|
||||
monkeypatch.setattr(mw, "get_or_new_skill_storage", lambda **kwargs: storage)
|
||||
|
||||
journal_records: list[dict] = []
|
||||
journal = SimpleNamespace(record_middleware=lambda *a, **k: journal_records.append({"a": a, "k": k}))
|
||||
context = {"secrets": {"ERP_TOKEN": _SECRET}, "__run_journal": journal}
|
||||
request = ModelRequest(
|
||||
model=object(),
|
||||
messages=[HumanMessage(content="/erp-report pull report", id="m1")],
|
||||
state={"messages": []},
|
||||
runtime=SimpleNamespace(context=context),
|
||||
)
|
||||
captured = {}
|
||||
SkillActivationMiddleware().wrap_model_call(request, lambda r: captured.setdefault("messages", r.messages) or AIMessage(content="ok"))
|
||||
return context, captured["messages"], journal_records
|
||||
|
||||
def test_prompt_surface_has_no_secret(self, tmp_path, monkeypatch):
|
||||
# The injected activation message (the only thing added to the prompt /
|
||||
# checkpointed messages) must not contain the secret value.
|
||||
_, messages, _ = self._activate_with_secret(tmp_path, monkeypatch)
|
||||
for m in messages:
|
||||
assert _SECRET not in str(m.content)
|
||||
|
||||
def test_checkpoint_surface_separation(self, tmp_path, monkeypatch):
|
||||
# Secrets live on runtime.context, never in the graph state that gets
|
||||
# checkpointed (messages/state).
|
||||
context, messages, _ = self._activate_with_secret(tmp_path, monkeypatch)
|
||||
assert context["secrets"]["ERP_TOKEN"] == _SECRET # present in context...
|
||||
assert _SECRET not in str([m.content for m in messages]) # ...not in state
|
||||
|
||||
def test_audit_surface_has_no_secret(self, tmp_path, monkeypatch):
|
||||
_, _, journal_records = self._activate_with_secret(tmp_path, monkeypatch)
|
||||
assert journal_records, "activation should record an audit event"
|
||||
assert _SECRET not in str(journal_records)
|
||||
|
||||
def test_trace_metadata_has_no_secret(self, monkeypatch):
|
||||
from deerflow.tracing import metadata as meta
|
||||
|
||||
monkeypatch.setattr(meta, "get_enabled_tracing_providers", lambda: {"langfuse"})
|
||||
config = {"context": {"secrets": {"ERP_TOKEN": _SECRET}}, "metadata": {}}
|
||||
meta.inject_langfuse_metadata(config, thread_id="t", user_id="u", model_name="m")
|
||||
assert _SECRET not in str(config["metadata"])
|
||||
# And secrets were never mirrored into configurable.
|
||||
assert _SECRET not in str(config.get("configurable", {}))
|
||||
|
||||
def test_redact_helper_strips_secret_keys(self):
|
||||
from deerflow.runtime.secret_context import redact_secret_context_keys
|
||||
|
||||
ctx = {"thread_id": "t", "secrets": {"ERP_TOKEN": _SECRET}, "__active_skill_secrets": {"ERP_TOKEN": _SECRET}}
|
||||
redacted = redact_secret_context_keys(ctx)
|
||||
assert redacted == {"thread_id": "t"}
|
||||
assert _SECRET not in str(redacted)
|
||||
|
||||
def test_redact_config_secrets_strips_from_persisted_config(self):
|
||||
# The run-record persistence + run API echo the raw request config; the
|
||||
# stored/echoed copy must not carry secrets (verifier blocker), while the
|
||||
# live config used to drive the run keeps them.
|
||||
from deerflow.runtime.secret_context import redact_config_secrets
|
||||
|
||||
config = {"context": {"secrets": {"ERP_TOKEN": _SECRET}, "thread_id": "t", "model_name": "m"}, "recursion_limit": 100}
|
||||
redacted = redact_config_secrets(config)
|
||||
assert _SECRET not in str(redacted)
|
||||
assert redacted["context"]["thread_id"] == "t"
|
||||
assert redacted["context"]["model_name"] == "m"
|
||||
assert "secrets" not in redacted["context"]
|
||||
# Original is untouched (live config still has secrets).
|
||||
assert config["context"]["secrets"] == {"ERP_TOKEN": _SECRET}
|
||||
|
||||
def test_redact_config_secrets_handles_none_and_no_context(self):
|
||||
from deerflow.runtime.secret_context import redact_config_secrets
|
||||
|
||||
assert redact_config_secrets(None) is None
|
||||
assert redact_config_secrets({"configurable": {"thread_id": "t"}}) == {"configurable": {"thread_id": "t"}}
|
||||
|
||||
def test_stdout_surface_redacted(self):
|
||||
from deerflow.sandbox.tools import mask_secret_values
|
||||
|
||||
leaked = f"DEBUG: token is {_SECRET} done"
|
||||
masked = mask_secret_values(leaked, {"ERP_TOKEN": _SECRET})
|
||||
assert _SECRET not in masked
|
||||
assert "[redacted]" in masked
|
||||
|
||||
def test_short_secret_values_not_masked(self):
|
||||
"""Values below the minimum length floor are skipped — redacting a 2-char
|
||||
value would shred unrelated bytes (exit codes, timestamps, sizes) of tool
|
||||
output. The secret is still injected into the subprocess; only the output
|
||||
mask skips it."""
|
||||
from deerflow.sandbox.tools import mask_secret_values
|
||||
|
||||
# A short value must not be replaced everywhere in the output.
|
||||
out = "exit code: 42\nrows: 42\n"
|
||||
masked = mask_secret_values(out, {"REGION": "42"})
|
||||
assert masked == out # unchanged — short value left intact
|
||||
|
||||
# A long value is still redacted as before.
|
||||
long_secret = "sk-erp-long-enough-token-value"
|
||||
masked_long = mask_secret_values(f"token={long_secret}", {"ERP_TOKEN": long_secret})
|
||||
assert long_secret not in masked_long
|
||||
assert "[redacted]" in masked_long
|
||||
|
||||
|
||||
@pytest.mark.skipif(__import__("os").name == "nt", reason="POSIX shell semantics")
|
||||
class TestEndToEndRealSubprocess:
|
||||
"""End-to-end across the real chain (no sandbox mock): activation resolves the
|
||||
secret, a REAL LocalSandbox subprocess receives it via env, the value lands in
|
||||
a file but is redacted from the returned output, and a later un-injected call
|
||||
cannot see it."""
|
||||
|
||||
def test_secret_reaches_real_subprocess_only_via_env_and_is_scoped(self, tmp_path, monkeypatch):
|
||||
from deerflow.agents.middlewares import skill_activation_middleware as mw
|
||||
from deerflow.agents.middlewares.skill_activation_middleware import SkillActivationMiddleware
|
||||
from deerflow.runtime.secret_context import read_active_secrets
|
||||
from deerflow.sandbox.tools import mask_secret_values
|
||||
|
||||
# 1. Activate a skill that declares ERP_TOKEN; caller supplies it in context.secrets.
|
||||
skill = _make_secret_skill(tmp_path, "erp-report", [SecretRequirement("ERP_TOKEN")])
|
||||
storage = SimpleNamespace(
|
||||
load_skills=lambda *, enabled_only: [skill],
|
||||
get_container_root=lambda: "/mnt/skills",
|
||||
get_skills_root_path=lambda: tmp_path,
|
||||
)
|
||||
monkeypatch.setattr(mw, "get_or_new_skill_storage", lambda **kwargs: storage)
|
||||
# A platform secret is present on the host and must NOT leak to the subprocess.
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "sk-host-platform-secret")
|
||||
context = {"secrets": {"ERP_TOKEN": _SECRET}}
|
||||
request = ModelRequest(
|
||||
model=object(),
|
||||
messages=[HumanMessage(content="/erp-report pull report", id="m1")],
|
||||
state={"messages": []},
|
||||
runtime=SimpleNamespace(context=context),
|
||||
)
|
||||
SkillActivationMiddleware().wrap_model_call(request, lambda r: AIMessage(content="ok"))
|
||||
injected = read_active_secrets(context)
|
||||
assert injected == {"ERP_TOKEN": _SECRET}
|
||||
|
||||
# 2. A REAL LocalSandbox runs a script that writes the token to a file and echoes it.
|
||||
out_file = tmp_path / "token.txt"
|
||||
sandbox = LocalSandbox(id="local")
|
||||
raw = sandbox.execute_command(
|
||||
f'printf "%s" "$ERP_TOKEN" > {out_file}; echo "leaked:$ERP_TOKEN"; echo "platform:$OPENAI_API_KEY"',
|
||||
env=injected,
|
||||
)
|
||||
|
||||
# 3. The skill genuinely received the token via env (file written by the subprocess).
|
||||
assert out_file.read_text() == _SECRET
|
||||
# 4. Platform secret was scrubbed — not available to the script.
|
||||
assert "sk-host-platform-secret" not in raw
|
||||
# 5. Stdout masking redacts the echoed token before it would re-enter context.
|
||||
masked = mask_secret_values(raw, injected)
|
||||
assert _SECRET not in masked
|
||||
|
||||
# 6. Per-call scope: a later command without injection cannot see the token.
|
||||
leaked = sandbox.execute_command("echo [$ERP_TOKEN]")
|
||||
assert _SECRET not in leaked
|
||||
@ -918,7 +918,13 @@ class _FakeSandbox:
|
||||
self._write_ok = write_ok
|
||||
self._check_result = check_result
|
||||
|
||||
def execute_command(self, command: str) -> str:
|
||||
def execute_command(
|
||||
self,
|
||||
command: str,
|
||||
env: dict[str, str] | None = None,
|
||||
timeout: float | None = None,
|
||||
) -> str:
|
||||
del env, timeout
|
||||
self.commands.append(command)
|
||||
if command.startswith("test -s"):
|
||||
return self._check_result
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user