mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 16:02:32 +00:00
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:
@@ -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
|
||||
|
||||
@@ -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)) {
|
||||
|
||||
Reference in New Issue
Block a user