mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-27 08:28:00 +00:00
04a85b307a
* fix(sandbox): scrub abbreviated *_PASS vars and Postgres PGPASSFILE from skill env env_policy blocks the full PASSWORD/PASSWD spellings but not the ubiquitous abbreviated _PASS form (DB_PASS, SMTP_PASS, MYSQL_PASS, ...), whose value is the plaintext password itself, so every skill subprocess inherited it. Postgres's file-based credential sources PGPASSFILE (.pgpass locator) and PGSERVICEFILE (pg_service.conf, may carry a password) were likewise missed -- the psql analog of the MYSQL_PWD/REDISCLI_AUTH no-flag sources #4018 added. Collapse *PASSWORD*/*PASSWD* into a single *PASS* pattern (which subsumes both, covers the abbreviated family, and catches PGPASSFILE) and add PGSERVICEFILE to the exact-name denylist. *PASS* deliberately does not touch PWD/OLDPWD, which carry no PASS substring. Capability-handle vars (KUBECONFIG, DOCKER_CONFIG, GIT_ASKPASS, ...) are a broader, non-value-bearing class and are left out of this fix. test_secret_like_names_are_blocked already pins the contract; the new names are added to its parametrize list (red before, green after) and the benign-names test guards against over-scrubbing PWD/OLDPWD. * test(sandbox): pin the *_ASKPASS over-scrub that *PASS* introduces *PASS* also matches the credential-helper vars GIT_ASKPASS, SSH_ASKPASS and SUDO_ASKPASS, which main inherits. Each names a program whose purpose is to hand the caller a credential, so inheriting the pointer is the same leak class as inheriting the value -- a skill can simply invoke it. Scrubbing them is intended; pin all three in test_secret_like_names_are_blocked (red on main) so the behaviour is a decision rather than a side effect of the pattern's shape. test_benign_names_are_allowed could not have caught this: none of its 14 names carries a PASS substring, so it passes identically before and after the broadening and asserts nothing about it. Record that in its docstring -- the absence of a benign PASS-bearing name is a decision, and PWD/OLDPWD are the boundary the list does pin. Also note the incidental COMPASS_*/BYPASS_* over-scrub in the module comment (over-scrubbing is this module's fail-safe direction; required-secrets is the escape hatch), and list PGPASSFILE alongside PGSERVICEFILE in AGENTS.md.
113 lines
5.0 KiB
Python
113 lines
5.0 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*",
|
|
# ``*PASS*`` subsumes the full ``PASSWORD``/``PASSWD`` spellings *and* the
|
|
# ubiquitous abbreviated form (``DB_PASS``, ``SMTP_PASS``, ``MYSQL_PASS``, ...),
|
|
# whose plaintext value is the password itself. It also covers ``PGPASSFILE``
|
|
# (libpq's ``.pgpass`` locator).
|
|
#
|
|
# It deliberately also catches the ``*_ASKPASS`` credential helpers
|
|
# (``GIT_ASKPASS``, ``SSH_ASKPASS``, ``SUDO_ASKPASS``). Those name a *program*
|
|
# rather than a secret, but that program exists to hand the caller a
|
|
# credential — inheriting the pointer is the same leak class this module
|
|
# closes, so scrubbing them is intended, not incidental.
|
|
#
|
|
# Incidental names that merely contain ``PASS`` (``COMPASS_*``, ``BYPASS_*``)
|
|
# are scrubbed too. That is the fail-safe direction for this module: a skill
|
|
# that genuinely needs any scrubbed name declares it via required-secrets.
|
|
# Benign ``PWD``/``OLDPWD`` carry no ``PASS`` substring and are unaffected.
|
|
"*PASS*",
|
|
"*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).
|
|
#
|
|
# The same reasoning covers the credential sources those clients read directly.
|
|
# ``MYSQL_PWD`` and ``REDISCLI_AUTH`` are the documented no-flag credential
|
|
# sources for ``mysql`` and ``redis-cli``. ``REDIS_AUTH`` is *not* canonical for
|
|
# any standard Redis client — it is blocked defensively because client libraries
|
|
# and deployment charts commonly set it. ``PGSERVICEFILE`` is the Postgres analog:
|
|
# libpq reads the ``pg_service.conf`` it points at (which may carry a password
|
|
# field) with no flag; its sibling ``PGPASSFILE`` is already caught by ``*PASS*``.
|
|
# These need exact entries: ``PWD``/``AUTH``/``SERVICEFILE`` cannot be wildcarded,
|
|
# since ``*PWD*`` would strip ``PWD``/``OLDPWD`` and no shared token is unique to
|
|
# them. (``*PASS*`` already covers ``PGPASSWORD``, ``MYSQL_PASSWORD``, ``DB_PASS``,
|
|
# ``PGPASSFILE``, ...)
|
|
_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",
|
|
"MYSQL_PWD",
|
|
"REDISCLI_AUTH",
|
|
"REDIS_AUTH",
|
|
"PGSERVICEFILE",
|
|
}
|
|
)
|
|
|
|
|
|
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
|