diff --git a/backend/packages/harness/deerflow/sandbox/env_policy.py b/backend/packages/harness/deerflow/sandbox/env_policy.py index e86ba704d..d0603041f 100644 --- a/backend/packages/harness/deerflow/sandbox/env_policy.py +++ b/backend/packages/harness/deerflow/sandbox/env_policy.py @@ -37,6 +37,15 @@ _SECRET_NAME_PATTERNS: tuple[str, ...] = ( # 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 password variables 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. +# All three need exact entries: ``PWD``/``AUTH`` cannot be wildcarded, since +# ``*PWD*`` would strip ``PWD`` and ``OLDPWD``. (``*PASSWORD*``/``*PASSWD*`` +# already cover ``PGPASSWORD``, ``MYSQL_PASSWORD``, ``REDIS_PASSWORD``, ...) _BLOCKED_EXACT_NAMES: frozenset[str] = frozenset( { "DATABASE_URL", @@ -54,6 +63,9 @@ _BLOCKED_EXACT_NAMES: frozenset[str] = frozenset( "CONN_STR", "GH_PAT", "GITHUB_PAT", + "MYSQL_PWD", + "REDISCLI_AUTH", + "REDIS_AUTH", } ) diff --git a/backend/tests/test_skill_request_scoped_secrets.py b/backend/tests/test_skill_request_scoped_secrets.py index 56f6ff0e4..f741c1997 100644 --- a/backend/tests/test_skill_request_scoped_secrets.py +++ b/backend/tests/test_skill_request_scoped_secrets.py @@ -160,6 +160,13 @@ class TestEnvPolicy: "POSTGRES_DSN", "CONN_STR", "GH_PAT", + # Password vars for services whose connection strings are already blocked + # above. These carry no KEY/SECRET/TOKEN/PASSWORD/PASSWD substring, and a + # blanket ``*PWD*`` / ``*AUTH*`` pattern would strip benign vars (``PWD``, + # ``OLDPWD``), so they need exact entries. + "MYSQL_PWD", # read directly by mysql / libmysqlclient + "REDISCLI_AUTH", # read directly by redis-cli + "REDIS_AUTH", ], ) def test_secret_like_names_are_blocked(self, name): @@ -177,6 +184,7 @@ class TestEnvPolicy: "LANG", "LC_ALL", "PWD", + "OLDPWD", "TMPDIR", "VIRTUAL_ENV", "PYTHONPATH", @@ -192,6 +200,36 @@ class TestEnvPolicy: assert is_blocked_env_name(name) is False + def test_db_password_vars_do_not_reach_the_subprocess_env(self, monkeypatch): + """The URL forms are scrubbed; the password vars for the same services must be too. + + ``mysql`` reads ``MYSQL_PWD`` and ``redis-cli`` reads ``REDISCLI_AUTH`` as the + password with no further configuration, so inheriting them hands a skill + subprocess the credential the connection-string block already withholds. + """ + from deerflow.sandbox.env_policy import build_sandbox_env + + monkeypatch.setenv("MYSQL_URL", "mysql://user:pw@host/db") + monkeypatch.setenv("MYSQL_PWD", "prod-db-password") + monkeypatch.setenv("REDISCLI_AUTH", "prod-redis-auth") + env = build_sandbox_env() + assert "MYSQL_URL" not in env + assert "MYSQL_PWD" not in env + assert "REDISCLI_AUTH" not in env + assert env.get("PWD") # the working directory must survive the added entries + + def test_injection_still_wins_for_the_newly_blocked_names(self, monkeypatch): + """``required-secrets`` stays the escape hatch for the names added here. + + The request-scoped value must also override the host's, which is the + per-user-key-overrides-shared-key case from #3861. + """ + from deerflow.sandbox.env_policy import build_sandbox_env + + monkeypatch.setenv("MYSQL_PWD", "host-value-must-not-leak") + env = build_sandbox_env(injected={"MYSQL_PWD": "request-scoped-value"}) + assert env["MYSQL_PWD"] == "request-scoped-value" + def test_build_sandbox_env_scrubs_inherited_and_layers_injected(self, monkeypatch): from deerflow.sandbox.env_policy import build_sandbox_env