test(wsl): close the two ratchet gaps that let planted spawns pass

- variable-indirected wsl.exe (`const b = 'wsl.exe'; spawnProcess(b)`) is now
  tracked, so the 5 files recorded only in a comment become real allowlist
  entries. Three actually spawn that way; the other two never spawned wsl.exe
  at all, so the prose record was wrong by three in the hiding direction.
- promisify(renamedAlias) is now resolved, so `const run = promisify(execFile)`
  behind an `execFile as x` import can no longer skip windowsHide.

Each verified by planting the violation, watching it fail, restoring, watching
it pass. Credit: GPT-Sol.
This commit is contained in:
Neil
2026-08-22 16:33:19 -07:00
parent 1161af4191
commit 2bbbd99c0d
3 changed files with 78 additions and 25 deletions
@@ -1,11 +1,11 @@
# Files that spawn wsl.exe directly instead of through src/main/wsl/wsl-runner.ts.
# Enforced by ../wsl-invocation-boundary.test.ts. This list only shrinks -- its
# length is the W3 goalpost.
#
# Enforced by ../wsl-invocation-boundary.test.ts. This list only shrinks -- its
# express because it always prepends --exec.
# Files that spawn wsl.exe directly instead of through src/main/wsl/wsl-runner.ts.
# host-level flag like --status / --list that the guest-command API cannot
# length is the W3 goalpost.
# Remaining entries need a runner mode that does not exist: a long-lived
# streaming child (OAuth logins, the hook relay), a synchronous caller, or a
# host-level flag like --status / --list that the guest-command API cannot
# express because it always prepends --exec.
main/agent-hooks/wsl-hook-relay-launch.ts
main/claude-accounts/service.ts
main/codex-accounts/runtime-home-service.ts
@@ -13,10 +13,13 @@ main/codex-accounts/service.ts
main/codex/codex-state-db-backfill-recovery.ts
main/codex/codex-trust-grant-host.ts
main/codex/codex-wsl-hook-install-plan.ts
main/daemon/pty-subprocess.ts
main/git/runner.ts
main/git/wsl-git-read-environment.ts
main/ipc/filesystem-watcher-wsl.ts
main/local-worktree-filesystem.ts
main/providers/local-pty-provider.ts
main/rate-limits/claude-pty.ts
main/rate-limits/codex-fetcher.ts
main/wsl-availability.ts
main/wsl-unc-delete.ts
+51 -17
View File
@@ -45,28 +45,19 @@ const IGNORED = new Set(['node_modules', 'dist', 'out', 'build', '.git', '__fixt
*
* Any identifier followed by `(` counts as an opener, and the assignment-style
* fields are matched by name. Indirection through a variable
* (`const f = cond ? 'wsl.exe' : x`) is still invisible; those files are listed
* explicitly below so the gap is recorded rather than implied.
* (`const f = cond ? 'wsl.exe' : x`) is caught separately, by
* `bindsWslBinaryToASpawnedIdentifier`.
*/
const SPAWN_OPENER = /\b[A-Za-z_$][\w$]*\s*\(\s*$|(?:program|binary|command|file|shellPath):\s*$/
/*
* Known blind spot, recorded rather than implied: these files assign
* `'wsl.exe'` to a variable and spawn it elsewhere, which no regex over the
* literal's neighbourhood can see. They are NOT in the allowlist, because the
* stale-entry check would reject an entry the scanner cannot find -- so the
* guard's count under-reports by these five, and this comment is the record.
*
* main/rate-limits/claude-pty.ts
* main/providers/local-pty-provider.ts
* main/daemon/pty-subprocess.ts
* relay/pty-shell-launch.ts
* relay/pty-handler.ts
*
* All five are PTY-pane launches, which are out of W3's scope anyway: they go
* through node-pty, not a child-process wrapper.
* The five files this comment used to list as an unscannable blind spot are
* now handled: three bind `wsl.exe` to a variable and spawn it, and are real
* allowlist entries; the other two never spawned it at all -- one compares a
* basename, one lists it among accepted shells. Recording a gap in prose was
* worse than it looked, because the count is the goalpost and it was wrong by
* three in the direction that hides offenders.
*/
function isTestFile(path: string): boolean {
return (
/\.(?:test|spec)\.tsx?$/.test(path) ||
@@ -92,6 +83,46 @@ function collectSourceFiles(root: string): string[] {
return found
}
/**
* `const binary = 'wsl.exe'` ... `spawn(binary)`, which no test over the
* literal's neighbourhood can see.
*
* Why it earns its place: the neighbourhood test was the whole guard, and a
* planted `const p = 'wsl.exe'; spawnProcess(p)` passed it. Five files were
* already known to spawn this way and were recorded in a comment instead of
* the allowlist, which means the count -- the actual goalpost -- was wrong by
* five and any NEW indirect spawner would have been invisible.
*/
function bindsWslBinaryToASpawnedIdentifier(source: string): boolean {
const bound = new Set<string>()
// Covers `const x = 'wsl.exe'`, a ternary picking it, and `binary: 'wsl.exe'`.
for (const match of source.matchAll(
/(?:const|let|var)\s+([A-Za-z_$][\w$]*)[^=;\n]*=[^;\n]*['"]wsl\.exe['"]/g
)) {
bound.add(match[1]!)
}
for (const match of source.matchAll(
/\b([A-Za-z_$][\w$]*)\s*:\s*[^,;\n]*['"]wsl\.exe['"]/g
)) {
bound.add(match[1]!)
}
for (const name of bound) {
const identifier = name.replace(/[$]/g, '\\$&')
// The identifier reaching a call opener, a spawn-style field, or the first
// argument of a spawn-style call.
if (
new RegExp(`\\b${identifier}\\s*\\(`).test(source) ||
new RegExp(`(?:program|binary|command|file|shellPath)\\s*:\\s*${identifier}\\b`).test(
source
) ||
new RegExp(`\\b\\w*(?:spawn|exec|run)\\w*\\s*\\(\\s*${identifier}\\b`, 'i').test(source)
) {
return true
}
}
return false
}
function findSpawnSites(): string[] {
const offenders = new Set<string>()
for (const path of collectSourceFiles(SOURCE_ROOT)) {
@@ -108,6 +139,9 @@ function findSpawnSites(): string[] {
offenders.add(relativePath)
}
}
if (bindsWslBinaryToASpawnedIdentifier(source)) {
offenders.add(relativePath)
}
}
return [...offenders].sort()
}
@@ -74,9 +74,25 @@ function findOffenders(): string[] {
const decommented = stripComments(file.source)
// Resolve `import { spawn as sp }` so a renamed binding is still a spawn.
// The previous comment claimed this; only three names were hardcoded.
const aliases = [...decommented.matchAll(/\b(?:spawn|spawnSync|execFile|execFileSync|exec|execSync|fork)\s+as\s+(\w+)/g)].map(
(match) => match[1]
)
const aliases = [
...decommented.matchAll(
/\b(?:spawn|spawnSync|execFile|execFileSync|exec|execSync|fork)\s+as\s+(\w+)/g
)
].map((match) => match[1]!)
// `const run = promisify(execFile)` mints a third name, and it can wrap a
// renamed binding, so this has to run after the aliases are known. A
// planted `promisify(renamedExecFile)` spawn passed the guard without it.
for (const match of decommented.matchAll(
/(?:const|let|var)\s+(\w+)\s*=\s*promisify\s*\(\s*(\w+)\s*\)/g
)) {
const wrapped = match[2]!
if (
aliases.includes(wrapped) ||
/^(?:spawn|spawnSync|execFile|execFileSync|exec|execSync|fork)$/.test(wrapped)
) {
aliases.push(match[1]!)
}
}
// The import test needs the module name, which blanking would erase; the
// call scan needs parens inside strings neutralised. Two views, one file.
if (!CHILD_PROCESS_IMPORT.test(decommented)) {