Child-process env scrub misses PAT/CRED/PASSPHRASE token names (secrets leak into every spawned child)

[ClaraOswald076]

Summary

SENSITIVE_ENV_PATTERN = /KEY|PASSWORD|SECRET|TOKEN/i (packages/subprocess/subprocess/src/index.ts) scrubs credential-shaped names from every ambient-inheritance child — bash, PTYs, MCP servers, subagent CLIs, browser launches. Credential names built from other tokens — GITHUB_PAT, *_CRED, *_PASSPHRASE — match none of the substrings and pass through to every spawned child.

Reproduction

1. Export GITHUB_PAT=... in the parent environment.
2. Run any sandboxed bash command that prints its environment (for example env | grep PAT).
3. The value is present in the child: GITHUB_PAT contains no KEY/PASSWORD/SECRET/TOKEN substring and survives scrubbedParentEnv().

Current behavior

The denylist is substring-only, so token-shaped credential names outside the four substrings leak implicitly. (A second, larger hole sits in plugin-manager's cli execution path, which passes the fully unscrubbed process.env regardless of this pattern — worth its own follow-up.)

Expected behavior

Separator-anchored token matching covers PAT/CRED/PASSPHRASE without false positives: PATH, PATHEXT, COMPAT, and PATTERNS keep flowing; deliberately forwarded credentials (explicit spec.env) are unaffected because explicit layers merge after the scrub.

Environment

• Repository: deepseek-ai/deepseek-harness, master @ 5badb1500 (v0.2.1-alpha.1)
• Platform: Windows 11 (26200), Node 24, pnpm 11.7.0; findings re-verified from source on a fresh checkout
• Related packages: see each section
• packages/subprocess/subprocess and every spawner importing scrubbedParentEnv

Proposed fix

Verified on Windows: npx vitest run packages/subprocess/subprocess/tests/service.spec.ts → 5/5 (two new cases pin the widened scrub). This package's win32 test exclusion also narrows from the whole directory to egress.spec.ts, so the pattern gains a Windows lane.

Patch (applies cleanly to master @ 5badb15)

diff --git a/packages/mcp/mcp-client/README.md b/packages/mcp/mcp-client/README.md
index 950b9b8c8..9239e36ee 100644
--- a/packages/mcp/mcp-client/README.md
+++ b/packages/mcp/mcp-client/README.md
@@ -134,7 +134,7 @@ A tool call uses the SDK with the raw name, complete tool definition, JSON argum
 
 ### Environment scrubbing (stdio)
 
-The child environment starts from the subprocess seam's `scrubbedParentEnv()` — ambient names matching `/KEY|PASSWORD|SECRET|TOKEN/i` and ambient `DSH_*` names are dropped — and the configured `env` merges on top, so explicit overrides survive. The MCP SDK owns the actual spawn; this package shares the scrub definition, not the spawn path.
+The child environment starts from the subprocess seam's `scrubbedParentEnv()` — ambient names matching `/KEY|PASSWORD|SECRET|TOKEN|(^|_)(PAT|CRED|PASSPHRASE)(_|$)/i` and ambient `DSH_*` names are dropped — and the configured `env` merges on top, so explicit overrides survive. The MCP SDK owns the actual spawn; this package shares the scrub definition, not the spawn path.
 
 </details>
 
diff --git a/packages/mcp/mcp-client/README.zh.md b/packages/mcp/mcp-client/README.zh.md
index 3ba203841..0c3bb500b 100644
--- a/packages/mcp/mcp-client/README.zh.md
+++ b/packages/mcp/mcp-client/README.zh.md
@@ -134,7 +134,7 @@ SDK 通过旧版通知或现代协议订阅接收工具列表变化。监督器
 
 ### 环境清洗（stdio）
 
-子进程环境以子进程 seam 的 `scrubbedParentEnv()` 为基座——删除匹配 `/KEY|PASSWORD|SECRET|TOKEN/i` 的环境名称与所有 `DSH_*` 名称——再在其上合并配置的 `env`，因此显式覆盖得以保留。实际 spawn 由 MCP SDK 负责；本包共享清洗定义，而非 spawn 路径。
+子进程环境以子进程 seam 的 `scrubbedParentEnv()` 为基座——删除匹配 `/KEY|PASSWORD|SECRET|TOKEN|(^|_)(PAT|CRED|PASSPHRASE)(_|$)/i` 的环境名称与所有 `DSH_*` 名称——再在其上合并配置的 `env`，因此显式覆盖得以保留。实际 spawn 由 MCP SDK 负责；本包共享清洗定义，而非 spawn 路径。
 
 </details>
 
diff --git a/packages/subprocess/subprocess/src/index.ts b/packages/subprocess/subprocess/src/index.ts
index 6dbbc6271..9aa5faf90 100644
--- a/packages/subprocess/subprocess/src/index.ts
+++ b/packages/subprocess/subprocess/src/index.ts
@@ -42,9 +42,12 @@ export type {
  * harness's own `DEEPSEEK_API_KEY`/secrets must not leak into a spawned
  * process implicitly). One heuristic for every in-repo spawner; a
  * deliberately supplied entry survives because explicit env layers merge
- * after the scrub.
+ * after the scrub. Separator-anchored alternatives cover token names that
+ * contain none of the bare substrings (`GITHUB_PAT`, `*_CRED`,
+ * `*_PASSPHRASE`) without matching neighbors like `PATH` or `COMPAT`: the
+ * `(^|_)…(_|$)` anchors accept `PAT` only as a whole `_`-separated token.
  */
-export const SENSITIVE_ENV_PATTERN = /KEY|PASSWORD|SECRET|TOKEN/i
+export const SENSITIVE_ENV_PATTERN = /KEY|PASSWORD|SECRET|TOKEN|(^|_)(PAT|CRED|PASSPHRASE)(_|$)/i
 
 /**
  * The ambient parent environment minus credential-shaped names and minus all
diff --git a/packages/subprocess/subprocess/tests/service.spec.ts b/packages/subprocess/subprocess/tests/service.spec.ts
index b9b9c707c..55baeaca5 100644
--- a/packages/subprocess/subprocess/tests/service.spec.ts
+++ b/packages/subprocess/subprocess/tests/service.spec.ts
@@ -100,4 +100,42 @@ describe('SubprocessRuntime seam', () => {
       delete process.env.SCRUB_PROBE_PLAIN
     }
   })
+
+  it('drops PAT/CRED/PASSPHRASE token names that contain no KEY/TOKEN substring', () => {
+    process.env.GITHUB_PAT = 'secret'
+    process.env.github_pat_lower = 'secret'
+    process.env.NPM_CRED = 'secret'
+    process.env.SSH_PASSPHRASE = 'secret'
+    try {
+      const env = scrubbedParentEnv()
+      expect(env.GITHUB_PAT).toBeUndefined()
+      expect(env.github_pat_lower).toBeUndefined()
+      expect(env.NPM_CRED).toBeUndefined()
+      expect(env.SSH_PASSPHRASE).toBeUndefined()
+    } finally {
+      delete process.env.GITHUB_PAT
+      delete process.env.github_pat_lower
+      delete process.env.NPM_CRED
+      delete process.env.SSH_PASSPHRASE
+    }
+  })
+
+  it('keeps names where PAT or CRED is only part of a longer token', () => {
+    process.env.SCRUB_PATH_LIKE = 'visible'
+    process.env.PATHEXT = 'visible'
+    process.env.COMPAT = 'visible'
+    process.env.GIT_PATTERNS = 'visible'
+    try {
+      const env = scrubbedParentEnv()
+      expect(env.SCRUB_PATH_LIKE).toBe('visible')
+      expect(env.PATHEXT).toBe('visible')
+      expect(env.COMPAT).toBe('visible')
+      expect(env.GIT_PATTERNS).toBe('visible')
+    } finally {
+      delete process.env.SCRUB_PATH_LIKE
+      delete process.env.PATHEXT
+      delete process.env.COMPAT
+      delete process.env.GIT_PATTERNS
+    }
+  })
 })
diff --git a/vitest.config.ts b/vitest.config.ts
index da9a95551..9e3b6ac62 100644
--- a/vitest.config.ts
+++ b/vitest.config.ts
@@ -41,7 +41,9 @@ const windowsUnsupportedPackages = process.platform === 'win32'
 const windowsUnsupportedTests = process.platform === 'win32'
   ? [
       ...windowsUnsupportedPackages.map(path => `${path}/tests/**/*.spec.ts`),
-      'packages/subprocess/subprocess/tests/**/*.spec.ts',
+      // egress binds sockets and spawns children against POSIX assumptions;
+      // service.spec is spawn-free and platform-neutral, so it stays on Windows.
+      'packages/subprocess/subprocess/tests/egress.spec.ts',
       'packages/subprocess/subprocess-local/tests/local.spec.ts',
       'packages/subprocess/subprocess-local/tests/process-inspector.spec.ts',
       'packages/subprocess/subprocess-local/tests/spawn.spec.ts',

[argszero]

Three additions from a source pass over this: a coverage inventory of the spawn sites, one correction to the follow-up you flagged, and one residual gap the proposed pattern leaves open.

Which children the widened pattern actually reaches

scrubbedParentEnv() is the single definition every in-repo spawner shares, but through three different mechanisms — which is what decides where a widened pattern has to live:

• By construction, through the subprocess service. packages/subprocess/subprocess-local/src/spawn.ts:44 childEnv() builds every service-routed child from the scrub and merges explicit entries after (index.ts:147); sandbox runner launches use the same builder (runner-launch.ts:73, :308); persistent terminals inherit it and add only their own overrides (packages/terminal/terminal-bash/src/index.ts:65).
• By direct import, owning their own spawn. mcp-client/src/transport.ts:22, subagent-dsh-sdk/src/run.ts:243, subagent-claude-code/src/run.ts:329 (+ process.ts:34), host/open-in-app/src/resolver.ts:80, experimental/browser-use-stagehand-native/src/launch.ts:48, bundle/web-app/src/index.ts:189, boot/plugin-manager/src/github-connection.ts:47, boot/plugin-manager/src/operations.ts:598 / :630. The MCP SDK's and node-pty's spawns are outside the service, which is exactly why the policy is a shared function rather than a service hook.
• Caller-owned, so covered only when the caller opts in. @deepseek-ai/dsh-sdk/client treats env: undefined as "inherit the parent environment entirely" (sdk/client/src/types.ts:39); its no-env default is not scrubbed, and the two subagent SDK launches above are the callers that make it scrub by passing the base explicitly.

Consequence for the fix's placement: widening the exported regex reaches all three groups at once. A plugin that filtered ctx.subprocess.spawn / spawnTerminal would see only the first group and leave the MCP-owned and SDK-client spawns uncovered while reading as if they were covered — so the core function is the right home, and the SDK client's inherit-by-default is the one spot worth its own note.

The cli execution path is intended, not a second hole

operations.ts:315 does pass the unscrubbed process.env, but the branch is documented one line above its own declaration: "CLI inherits authentication and terminal descriptors; service scrubs secrets and captures output" (operations.ts:39), paired with leadsOwnGroup and inherited descriptors (:356–:362). The cli rung is the user's own interactive package-manager run, where the child is meant to see the user's registry and git authentication; the service rung (index.ts:717, the agent-driven install) is the one that scrubs. I would keep your follow-up aimed at scrub coverage rather than at that branch, otherwise the fix would break private-registry installs from the terminal.

One residual gap in the proposed pattern

Separator anchoring does keep PATH / PATHEXT / COMPAT / GIT_PATTERNS, and catches GITHUB_PAT / NPM_CRED / SSH_PASSPHRASE (measured against your regex, not asserted). Making CRED a whole _-separated token also excludes its longer spellings:

name	current	proposed
AZURE_CREDENTIALS	no	no
GOOGLE_APPLICATION_CREDENTIALS	no	no
AWS_SHARED_CREDENTIALS_FILE	no	no
NPM_CREDS	no	no
DOCKER_AUTH_CONFIG	no	no

AZURE_CREDENTIALS is the one I would not leave out: in the CI convention it carries a JSON blob with the service-principal secret, so it is the same class as the names you name. Widening the middle alternative to CRED[A-Z]* — or spelling out CRED|CREDS|CREDENTIALS — keeps the anchors and the false-positive set unchanged. DOCKER_AUTH_CONFIG would need an AUTH token, and there I would want your judgement rather than mine, since *_AUTH_METHOD-style neighbours are plausible.

The patch and the Windows-lane narrowing both look right; the only thing I would add is one case pinning a *_CREDENTIALS name, so the pattern's boundary is stated by a test rather than by the comment above it.