mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-08-01 02:46:02 +00:00
* feat(skills): bind request-scoped secrets for in-context (autonomously invoked) skills Extends the #3861 binding point A (slash-activation only) to A+: the injection set is recomputed on every model call from two unioned sources — the run's most recent slash activation (persisted on the run context so the tool loop keeps the binding) and skills the model actually loaded in this thread (ThreadState.skill_context), re-validated against the live registry each call. Authorization stays three-gated regardless of activation style: skill enabled by the operator, values supplied per-request by the caller in context.secrets (never persisted server-side, never from the host env), names declared in the skill's required-secrets frontmatter. Because the set is replaced per call, eviction from skill_context or a caller that stops supplying a value revokes injection on the next call. New frontmatter field secrets-autonomous (default true) lets a skill restrict binding to explicit slash activation; malformed values fail closed to false. Binding changes are recorded as a middleware:skill_secrets journal event carrying names only. Design informed by a survey of peer systems (Claude Code, Codex CLI, opencode, pi, deepagents, hermes-agent, QwenPaw) and specs (agentskills.io, MCP 2025-11-25): the industry trust boundary is enable-time consent plus caller-scoped credentials, not per-invocation ceremony; no surveyed system scopes secrets to an activation turn. Part of #3914 * refactor(skills): centralize secret context keys, document intentional per-call reload Review follow-ups (no behavior change): move the two private binding keys (__slash_skill_secret_source, __skill_secrets_binding_audit) into secret_context.py and add them to REDACTED_CONTEXT_KEYS so the redaction allowlist stays a complete guard even though both keys hold names only. Document why _in_context_secret_sources reloads skills every call rather than caching: load_skills re-reads enabled state so an operator disabling a skill revokes its binding on the next model call — an mtime cache would miss enable/disable toggles and keep injecting after a disable. * fix(skills): match in-context secret bindings by path only, never by name Review finding (confused deputy): _in_context_secret_sources fell back to name matching when a skill_context path did not resolve. DeerFlow lets a custom skill shadow a same-named public/legacy one (load_skills de-dupes by name, custom wins), so a thread that read public/foo could bind the custom foo's declared secrets although the custom skill was never loaded in the thread. The recent user-isolation path changes make by-path misses (and thus the dangerous fallback) more likely. Drop the by-name fallback: match strictly by the exact container file path the model read; an unresolved path simply does not bind (the safe direction). Regression tests cover the shadowing case and a stale path. Part of #3914 * fix(skills): resolve secret-binding sources via registry; strip caller __-keys Security review (willem-bd, #3938): 1. Forged `__slash_skill_secret_source` bypassed the enabled/allowlist/ secrets-autonomous gates. runtime.context is caller-mergeable, and the slash source was trusted as authoritative (its stored requirements were injected directly). Now the slash source records only the activated skill's canonical container path, and BOTH the slash and in-context sources resolve the live registry skill by normalized path each call (_resolve_registry_skill) — binding only that real, enabled, allowlisted skill's own declared secrets. A forged path resolves to nothing. As defense in depth, build_run_config strips caller-supplied __-prefixed context keys at the gateway boundary. 2. Malformed caller requirements crashed the run (unguarded tuple unpack / DoS). The middleware no longer unpacks caller-provided requirement data at all — declarations come from the registry — so a malformed source fails closed instead of raising. 3. Path-normalization asymmetry silently disabled in-context binding on a trailing-slash container_path config. Both the registry keys and the lookup path are now posixpath.normpath'd. Regression tests: forged source rejected, forged-but-real path ignores caller requirements + allowlist, malformed source fails closed, trailing- slash config binds, gateway strips __-keys. Part of #3914 * docs(skills): correct _SLASH_SECRET_SOURCE_KEY comment and note fail-closed trade-off Post-review cleanup: the key now stores only the canonical container path (the comment still described the pre-fix skill-name+requirements shape), and document that a transient registry-load failure fails closed (drops the binding for that call) rather than trusting stale data. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
104 lines
4.2 KiB
Python
104 lines
4.2 KiB
Python
"""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))
|
|
|
|
|
|
# Private run-context keys the skill-activation middleware uses to carry secret
|
|
# bindings across a run. Only ``secrets`` / ``__active_skill_secrets`` hold
|
|
# values; the binding-source and audit keys hold names only. All are listed so
|
|
# the redaction allowlist stays a complete guard even if a future edit starts
|
|
# storing a value under one of the name-only keys.
|
|
_SLASH_SECRET_SOURCE_KEY = "__slash_skill_secret_source"
|
|
_SECRETS_BINDING_AUDIT_KEY = "__skill_secrets_binding_audit"
|
|
|
|
# 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,
|
|
_SLASH_SECRET_SOURCE_KEY,
|
|
_SECRETS_BINDING_AUDIT_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
|