mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-08-01 02:46:02 +00:00
09988caf95
* 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.
83 lines
3.1 KiB
Python
83 lines
3.1 KiB
Python
"""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
|