fix(sandbox): scrub abbreviated *_PASS vars and Postgres PGPASSFILE from skill env (#4026)

* 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.

backend/AGENTS.md

@@ -348,7 +348,7 @@ Proxied through nginx: `/api/langgraph/*` → Gateway LangGraph-compatible runti
 
 **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.
+**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*`/`*PASS*`/`*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`. Legacy global-custom mounts are gated by the same user-scoped skill discovery rule used for prompt/list visibility; providers must not infer visibility from raw directory presence alone.
 - `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. Legacy global-custom mounts follow the same shared visibility helper as local and remote providers.
@@ -458,7 +458,7 @@ Lets a caller pass per-request, short-lived end-user credentials (e.g. an ERP to
 - **Bind (point A+)**: `SkillActivationMiddleware._resolve_secret_bindings` recomputes the injection set (`runtime.context[__active_skill_secrets]`) on every model call from two unioned sources, then REPLACES the key. (1) *Slash*: the run's most recent `/skill` activation, persisted as a source on the run context (only the activated skill's **canonical container path**, never its declared secrets) so the whole tool loop after the activation call keeps the binding; a new activation replaces it. Slash reads the genuine user text via `get_original_user_content_text`; `InputSanitizationMiddleware` preserves it (`ORIGINAL_USER_CONTENT_KEY`), so activation fires even after sanitization. (2) *In-context* (autonomous invocation): skills the model actually loaded in this thread — `ThreadState.skill_context` entries. **Both sources resolve the live registry skill by normalized container path on every call** (`_resolve_registry_skill`) and bind only that skill's own declared secrets — enabled + allowlist checked for both; the `secrets-autonomous: false` opt-out (malformed values fail closed to `false`) additionally gates the in-context path but exempts explicit slash. Resolving by registry — not by trusting the source's stored data — is what makes a caller-forged `__slash_skill_secret_source` harmless (`runtime.context` is caller-mergeable; the gateway also strips caller `__`-keys in `build_run_config`), #3938. Authorization is three-gated regardless of activation style: skill **enabled** by the operator × values **supplied per-request** by the caller (`context.secrets`) × names **declared** in frontmatter (∩ semantics). Because the set is recomputed per call, a skill evicted from `skill_context` (capacity) or a caller that stops supplying a value loses injection on the next call. 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 once per binding change, not injected; binding changes are recorded as a `middleware:skill_secrets` journal event (skill and secret names only, never values).
 - **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.
 - **AIO image requirement**: on `AioSandbox` the env path uses the `bash.exec` API (`POST /v1/bash/exec`), which upstream all-in-one-sandbox only ships since `1.9.3` — older images (including a `latest` tag frozen on the `1.0.0.x` line) 404 the whole `/v1/bash/*` namespace. `AioSandbox` detects the 404, remembers the capability gap on the instance, and fails fast with an actionable upgrade error instead of letting the model retry raw 404s; there is deliberately **no** fallback through the legacy shell path because none keeps the secret values out of the command string (#3921). Regression tests: `tests/test_aio_sandbox.py::TestBashExecUnsupportedFailFast`.
-- **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.
+- **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*`/`*PASS*`/`*CREDENTIAL*`/`*DSN*` + a connection-string denylist like `DATABASE_URL`/`REDIS_URL`/`GH_PAT`, plus no-flag credential sources like `MYSQL_PWD`/`REDISCLI_AUTH`/`PGPASSFILE`/`PGSERVICEFILE`) 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**: no persistence/vaulting — values are request-scoped and never stored server-side, so long-lived use means the caller re-supplies `context.secrets` on each request while the skill stays in `skill_context`; subagents do not inherit the injection set; the MCP per-user-credential gap (#3322) is a sibling, not covered here. Tests: `tests/test_skill_request_scoped_secrets.py`.
 

backend/packages/harness/deerflow/sandbox/env_policy.py

@@ -25,8 +25,22 @@ _SECRET_NAME_PATTERNS: tuple[str, ...] = (
     "*KEY*",
     "*SECRET*",
     "*TOKEN*",
-    "*PASSWORD*",
-    "*PASSWD*",
+    # ``*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
 )
@@ -38,14 +52,17 @@ _SECRET_NAME_PATTERNS: tuple[str, ...] = (
 # 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.
+# 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.
-# 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``, ...)
+# 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",
@@ -66,6 +83,7 @@ _BLOCKED_EXACT_NAMES: frozenset[str] = frozenset(
         "MYSQL_PWD",
         "REDISCLI_AUTH",
         "REDIS_AUTH",
+        "PGSERVICEFILE",
     }
 )
 

backend/tests/test_skill_request_scoped_secrets.py

@@ -167,6 +167,28 @@ class TestEnvPolicy:
             "MYSQL_PWD",  # read directly by mysql / libmysqlclient
             "REDISCLI_AUTH",  # read directly by redis-cli
             "REDIS_AUTH",
+            # Abbreviated ``_PASS`` password vars: value-bearing plaintext passwords
+            # that the full-spelling ``*PASSWORD*`` / ``*PASSWD*`` patterns miss.
+            "DB_PASS",
+            "SMTP_PASS",
+            "MYSQL_PASS",
+            "REDIS_PASS",
+            "FTP_PASS",
+            "MAIL_PASS",
+            # Postgres file-based credential sources read by libpq/psql with no flag,
+            # the direct analog of MYSQL_PWD/REDISCLI_AUTH above. PGPASSFILE names a
+            # .pgpass (host:port:db:user:password); PGSERVICEFILE names a
+            # pg_service.conf that may carry a password field.
+            "PGPASSFILE",
+            "PGSERVICEFILE",
+            # Credential *helpers*: each names a program that dispenses a credential
+            # on demand. Inheriting the pointer is the same leak class as inheriting
+            # the value, so ``*PASS*`` scrubbing them is intended. Pinned here so the
+            # behaviour is a deliberate decision rather than a side effect of the
+            # pattern's shape.
+            "GIT_ASKPASS",
+            "SSH_ASKPASS",
+            "SUDO_ASKPASS",
         ],
     )
     def test_secret_like_names_are_blocked(self, name):
@@ -196,6 +218,16 @@ class TestEnvPolicy:
         ],
     )
     def test_benign_names_are_allowed(self, name):
+        """Names here must survive the scrub.
+
+        Note what this list does *not* contain: any name carrying a ``PASS``
+        substring. That is deliberate, not an oversight — ``*PASS*`` scrubs every
+        such name, including the ``*_ASKPASS`` credential helpers pinned in
+        ``test_secret_like_names_are_blocked`` above. Over-scrubbing is this
+        module's fail-safe direction; a skill that needs a scrubbed name declares
+        it via ``required-secrets``. ``PWD``/``OLDPWD`` are the boundary this list
+        does pin: they carry no ``PASS`` substring and must never be stripped.
+        """
         from deerflow.sandbox.env_policy import is_blocked_env_name
 
         assert is_blocked_env_name(name) is False