From cff202c16a79bbcd3d24cb7ab62abf2898449a23 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sat, 5 Sep 2026 21:12:40 -0700 Subject: [PATCH] fix(windows): drop EDR-flagged -ExecutionPolicy Bypass from encoded PowerShell (#17880) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(windows): drop EDR-flagged -ExecutionPolicy Bypass from encoded PowerShell MDE flags `-ExecutionPolicy Bypass` paired with base64 `-EncodedCommand` as a behavioural signal. Measured on Windows 11: neither `-Command` nor `-EncodedCommand` is execution-policy gated (both run under an explicit `-ExecutionPolicy Restricted` and `AllSigned`; only `-File` fails), so the switch was a pure no-op on every one of these command lines. Removes the switch from all four sites that spelled it, and de-encodes the one site whose payload never passes through a re-parsing shell: - ssh-remote-powershell: one chokepoint for ~40 remote-Windows call sites. Base64 kept — the remote sshd DefaultShell re-parses this string. - setup-agent-sequencing / windows-cmd-runner-delayed-launch: base64 kept — these strings are typed into a terminal pane. - windows-interactive-login-spawn: base64 kept — `cmd.exe /c start` re-parses, and the cmd-safe-token guard rejects the `&` and `"` in the raw relay script. - windows-mobile-firewall local runner: `-EncodedCommand` -> `-Command`, since execFile reaches CreateProcess with no shell in between. The setup startup gate keeps execution-policy relief in-payload (process scope), because it evals a user-authored startup command that may invoke a `.ps1`, and a `.ps1` IS gated. Caught by the real-process suite; mirrors the agent-hooks launcher's trade. The elevated firewall child deliberately stays encoded: `Start-Process -ArgumentList` joins its array into one ShellExecuteEx string without quoting and PowerShell re-splits on whitespace, measured to collapse `C:\My App\...` to `C:\My App\...` — a firewall rule for the wrong program. * test(ssh): enforce the no-script-file invariant remote payloads rely on Dropping `-ExecutionPolicy Bypass` from `powerShellCommand` is a no-op only while no remote payload loads a PowerShell script file — execution policy has never gated anything else. That invariant held by inspection and was guarded by nothing, so a future payload that dot-sourced, used `-File`, or imported a `.psm1` would break only on a remote host with a Restricted/AllSigned LocalMachine policy and no GPO: a failure on someone else's machine. States the invariant at the wrapper, and adds a ratchet that scans every module importing it for `.ps1`/`.psm1`, `Import-Module`, `-File`, and dot-sourcing. The scan discovers importers itself (13 today) so new ones are covered, and asserts it found some, so an emptied list cannot pass vacuously. Mutation-checked: injecting each construct into a real importer fails the matching case and names the file. The first dot-source pattern passed a `;`-prefixed sample but missed `powerShellCommand(". '$x'")` — the likelier shape — so the pattern now accepts a string-literal start and the self-test samples carry their surrounding quotes. * test(ssh): close two blind spots in the remote-payload ratchet Both found by independent mutation testing of the ratchet itself, and both let a real violation pass while the guard reported green. `-File` was matched case-sensitively, so `-file $scriptVar` slipped through — PowerShell switches are case-insensitive, and with a variable path the `.ps1` pattern does not cover for it, so that shape escaped both nets. The naive fix is wrong: bare /-File\b/i matches `--credential-file`, `--log-file` and `--body-file`, which occur in three of these importers. Anchoring to a token boundary catches the lowercase, odd-spacing and argv-element forms with zero offenders across all 14. Comment stripping paired a `/*` appearing inside a string (a glob such as 'src/*.ts') with any later comment close and deleted everything between, hiding violations in the gap. Anchoring the block strip to line start, as the `//` strip already was, fixes it — verified by injecting an `Import-Module` after a glob string: the unanchored form misses it, the anchored form catches it. Extends the same case-insensitivity to `.ps1`/`.psm1` and `Import-Module`, which had the identical flaw (`import-module`, `DEPLOY.PS1` are legitimate spellings); measured to add no false positive. Each construct now carries the fixtures it must catch AND the near-misses it must not, so a future tightening cannot quietly trade one for the other — the negative fixtures are what would have caught the naive `-File` fix. Non-vacuity bound tightened to >10 against 14 importers. * docs(ssh): state what the remote-payload ratchet cannot see The scan matches source text, so a script file reached only through a variable (`& $scriptPath`) never appears in source and no pattern can catch it. The ratchet narrows the hole; the invariant note on `powerShellCommand` covers the remainder. Recorded because a guard that reads as complete coverage when it is not is worse than one that states its edge: the next author trusts it further than it deserves, and should learn this limit from the test rather than an incident. * test(ssh): scan remote payloads with the shared source walk The ratchet had its own tree walk and comment stripper. The walk skipped neither node_modules/dist/.git nor dot-directories and excluded tests by `.test.ts` alone, so its importer count -- the guard's own goalpost -- could be wrong about what it scanned. The stripper was anchored to line start to dodge a `/*` inside a glob string, which silently skipped trailing comments; `stripComments` tracks quote state and handles both. Importer set re-derived against the shared walk: 15, floor unchanged at 10. * fix(setup): report a failed execution-policy relief instead of swallowing it The in-payload Set-ExecutionPolicy carried -ErrorAction SilentlyContinue and an empty catch, so any failure vanished. A Windows PowerShell 5.1 install with duplicate extended type data fails every cmdlet in Microsoft.PowerShell.Security -- autoload, not policy -- and the user then saw only their own .ps1 being refused, with no trace that the relief had been attempted or why. -ErrorAction Stop is what routes a non-terminating failure into the catch at all; the catch reports the FullyQualifiedErrorId to stderr and deliberately does not rethrow, so a broken policy cmdlet cannot take down the startup this gate exists to run. Success path is unchanged and stays stderr-clean. Verified by execution on a clean child environment: success -> policy=Bypass, stderr empty; shadowed failing cmdlet -> diagnostic on stderr and the gate still continues; the old empty catch -> silent. --------- Co-authored-by: Orca Worker --- .../runtime/windows-mobile-firewall.test.ts | 54 ++++++ src/main/runtime/windows-mobile-firewall.ts | 9 +- src/main/ssh/ssh-remote-powershell.test.ts | 164 ++++++++++++++++++ src/main/ssh/ssh-remote-powershell.ts | 23 ++- src/shared/setup-agent-sequencing.test.ts | 31 +++- src/shared/setup-agent-sequencing.ts | 26 ++- src/shared/setup-runner-command.test.ts | 4 +- .../windows-cmd-runner-delayed-launch.test.ts | 36 ++++ .../windows-cmd-runner-delayed-launch.ts | 5 +- .../windows-interactive-login-spawn.test.ts | 26 +-- src/shared/windows-interactive-login-spawn.ts | 6 +- 11 files changed, 363 insertions(+), 21 deletions(-) create mode 100644 src/main/ssh/ssh-remote-powershell.test.ts create mode 100644 src/shared/windows-cmd-runner-delayed-launch.test.ts diff --git a/src/main/runtime/windows-mobile-firewall.test.ts b/src/main/runtime/windows-mobile-firewall.test.ts index 19109a444b8..d561878a538 100644 --- a/src/main/runtime/windows-mobile-firewall.test.ts +++ b/src/main/runtime/windows-mobile-firewall.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from 'vitest' +import { execFile } from 'node:child_process' import { getWebSocketPort, inspectWindowsMobileFirewall, @@ -6,6 +7,9 @@ import { type WindowsMobileFirewallEnvironment } from './windows-mobile-firewall' +// Why: every other case injects `runPowerShell`, so only the argv case below reaches execFile. +vi.mock('node:child_process', () => ({ execFile: vi.fn() })) + function environment( runPowerShell: WindowsMobileFirewallEnvironment['runPowerShell'], overrides: Partial = {} @@ -179,6 +183,56 @@ describe('windows mobile firewall', () => { expect(repairScript).toContain('-EdgeTraversalPolicy Block') }) + it('keeps the elevated child encoded because Start-Process re-splits its ArgumentList', async () => { + // Why: `Start-Process -ArgumentList` joins the array into one ShellExecuteEx parameter + // string without quoting and PowerShell re-splits it on whitespace, which collapses runs + // of spaces. Measured on Windows 11: a `-Command` payload turned `C:\My App\Orca.exe` + // into `C:\My App\Orca.exe`, i.e. a firewall rule for the wrong program. Base64 is the + // only form that survives that hop, so this site must not follow the local runner. + const runPowerShell = vi.fn().mockResolvedValue('{"launched":true,"exitCode":0}') + await repairWindowsMobileFirewall( + 6769, + environment(runPowerShell, { executablePath: 'C:\\My App\\Orca.exe' }) + ) + + const outerScript = runPowerShell.mock.calls[0]![0] as string + expect(outerScript).toContain("'-EncodedCommand'") + expect(outerScript).not.toContain("'-Command'") + expect(outerScript).toContain('-Verb RunAs') + + const encoded = outerScript.match(/'-EncodedCommand', '([^']+)'/)?.[1] + const repairScript = Buffer.from(encoded!, 'base64').toString('utf16le') + expect(repairScript).toContain("-Program 'C:\\My App\\Orca.exe'") + }) + + it('runs the local PowerShell over argv with a plain -Command script', async () => { + // Why: execFile reaches CreateProcess with no shell in between, so the script needs no + // base64 armouring, and argv preserves runs of spaces that the elevated hop cannot. + // `-EncodedCommand` here was pure EDR signal. + const execFileMock = vi.mocked(execFile) + execFileMock.mockImplementation(((_file, _args, _options, callback) => { + callback(null, '{"privateFirewallEnabled":true,"networkCategory":"Private"}', '') + return {} + }) as unknown as typeof execFile) + + await inspectWindowsMobileFirewall(6768, undefined, { + platform: 'win32', + isPackaged: true, + executablePath: 'C:\\My App\\Orca.exe', + systemRoot: 'C:\\Windows' + }) + + const [file, args] = execFileMock.mock.calls[0]! + expect(file).toMatch(/WindowsPowerShell\\v1\.0\\powershell\.exe$/i) + expect(args!.slice(0, 3)).toEqual(['-NoProfile', '-NonInteractive', '-Command']) + expect(args).not.toContain('-EncodedCommand') + expect(args).not.toContain('-ExecutionPolicy') + // The script travels as ONE argv element, so its spaces and newlines survive verbatim. + expect(args).toHaveLength(4) + expect(args![3]).toContain("-Program 'C:\\My App\\Orca.exe'") + expect(args![3]).toContain('\n') + }) + it('distinguishes a cancelled UAC prompt from repair failure', async () => { await expect( repairWindowsMobileFirewall( diff --git a/src/main/runtime/windows-mobile-firewall.ts b/src/main/runtime/windows-mobile-firewall.ts index 88b885ab5f0..c90a025cb14 100644 --- a/src/main/runtime/windows-mobile-firewall.ts +++ b/src/main/runtime/windows-mobile-firewall.ts @@ -229,6 +229,11 @@ Get-NetFirewallRule -Name ${quotePowerShell(FIREWALL_RULE_NAME)} -ErrorAction Si New-NetFirewallRule -Name ${quotePowerShell(FIREWALL_RULE_NAME)} -DisplayName ${quotePowerShell(FIREWALL_RULE_DISPLAY_NAME)} -Description 'Allows Orca Mobile to connect to this Orca desktop on private networks.' -Direction Inbound -Action Allow -Enabled True -Profile Private -Protocol TCP -LocalPort ${port} -Program ${quotePowerShell(executablePath)} -EdgeTraversalPolicy Block | Out-Null` } +// Why the elevated child keeps `-EncodedCommand` while the local runner does not: `Start-Process +// -ArgumentList` joins its array into one ShellExecuteEx parameter string without quoting, and +// PowerShell then re-splits it on whitespace — measured to collapse `C:\My App\...` to +// `C:\My App\...`, which would silently write the firewall rule for the wrong program. Node's +// argv path (createPowerShellRunner) preserves runs of spaces, so only this hop needs base64. function buildElevationScript(powershellPath: string, encodedRepairScript: string): string { return `$ErrorActionPreference = 'Stop' try { @@ -257,7 +262,9 @@ function createPowerShellRunner(systemRoot?: string): PowerShellRunner { new Promise((resolve, reject) => { execFile( powershellPath, - ['-NoProfile', '-NonInteractive', '-EncodedCommand', encodePowerShell(script)], + // Why: argv reaches CreateProcess with no shell in between, so the script needs no base64 + // armouring — and plain `-Command` keeps this off EDR's encoded-PowerShell heuristics. + ['-NoProfile', '-NonInteractive', '-Command', script], { encoding: 'utf8', timeout: timeoutMs, windowsHide: true, maxBuffer: 1024 * 1024 }, (error, stdout) => { if (error) { diff --git a/src/main/ssh/ssh-remote-powershell.test.ts b/src/main/ssh/ssh-remote-powershell.test.ts new file mode 100644 index 00000000000..f6ba6136378 --- /dev/null +++ b/src/main/ssh/ssh-remote-powershell.test.ts @@ -0,0 +1,164 @@ +import { describe, expect, it } from 'vitest' +import { join } from 'node:path' +import { scanSourceTree, stripComments } from '../../shared/source-scan/source-tree-scan' +import { powerShellCommand } from './ssh-remote-powershell' + +function decodePayload(command: string): string { + const encoded = command.match(/ -EncodedCommand (\S+)$/)?.[1] + if (!encoded) { + throw new Error(`no -EncodedCommand payload in: ${command}`) + } + return Buffer.from(encoded, 'base64').toString('utf16le') +} + +/** + * This one helper builds the command line for every remote-Windows SSH call site + * (relay deploy, install locks, upload staging, GC claim, browse, CLI launch), so + * its switches are worth pinning. + */ +describe('powerShellCommand', () => { + it('spells no -ExecutionPolicy switch', () => { + const command = powerShellCommand('exit 0') + const switches = command.replace(/ -EncodedCommand \S+$/, '') + + // Why: `-EncodedCommand` is not execution-policy gated — only `-File` is — so the switch + // was a no-op, and `-ExecutionPolicy Bypass` beside base64 is among the most heavily + // EDR-flagged PowerShell command lines there is. + expect(switches).not.toMatch(/-ExecutionPolicy/i) + expect(switches).not.toMatch(/Bypass/i) + expect(switches).toBe('powershell.exe -NoProfile -NonInteractive') + }) + + it('keeps the base64 payload the remote shell cannot rewrite', () => { + // Why: this string is re-parsed by the remote host's sshd DefaultShell, which is + // cmd.exe on a stock Windows OpenSSH install. Base64 is load-bearing here. + const command = powerShellCommand("Write-Output 'a & b' | Out-String") + + expect(command).toMatch( + /^powershell\.exe -NoProfile -NonInteractive -EncodedCommand [A-Za-z0-9+/=]+$/ + ) + expect(decodePayload(command)).toBe("Write-Output 'a & b' | Out-String") + }) +}) + +const MAIN_DIR = join(import.meta.dirname, '..') + +// Why: execution policy gates loading script FILES and nothing else, so dropping +// `-ExecutionPolicy Bypass` is a no-op exactly while no remote payload loads one. That +// invariant is what makes the switch safe to omit, and it was previously guarded by nothing: +// a future payload that dot-sourced or used `-File` would fail only on a remote host whose +// LocalMachine policy is Restricted/AllSigned. See the invariant note on `powerShellCommand`. +// Every pattern is case-insensitive: PowerShell switches and cmdlet names are, and Windows +// paths are, so `-file`, `import-module` and `DEPLOY.PS1` are all legitimate spellings that a +// case-sensitive pattern would wave through. Verified to add no false positive across the real +// importers. Each entry carries the fixtures it must catch AND the near-misses it must not, so +// a future tightening cannot quietly trade one for the other. +// +// Limit: this is a source-text scan, so a script file reached only through a variable +// (`& $scriptPath`) never appears in source and no pattern here can catch it — this narrows +// the hole rather than sealing it. The invariant note on `powerShellCommand` covers the rest. +// +// Matched against `stripComments`, the shared quote-tracking stripper, so a construct named in +// prose is not counted as code. A line-anchored regex pair cannot do this job: it either eats +// live code by pairing a `/*` inside a glob string with a later comment close, or — anchoring +// to avoid that — skips every trailing comment. Quote state is the only fix. +const POLICY_GATED_CONSTRUCTS = [ + { + label: 'a PowerShell script file (.ps1/.psm1)', + pattern: /\.psm?1\b/i, + catches: [ + `powerShellCommand("$script = 'C:\\tools\\deploy.ps1'")`, + `powerShellCommand("Import-Module '$dir\\orca.psm1'")`, + `powerShellCommand("& '$root\\DEPLOY.PS1'")` + ], + ignores: [`const build = 'artifact.ps10'`] + }, + { + label: 'Import-Module', + pattern: /\bImport-Module\b/i, + catches: [ + `powerShellCommand("Import-Module 'NetSecurity'")`, + `powerShellCommand("import-module $modulePath")` + ], + ignores: [`const name = 'Import-ModuleList'`] + }, + { + // Anchored to a token boundary: a bare /-File\b/i also matches `--credential-file`, + // `--log-file` and `--body-file`, which are real arguments in three of these importers. + label: 'the -File switch', + pattern: /(^|[\s'"`([{,])-File\b/i, + catches: [ + `runRemote("powershell.exe -NoProfile -File 'C:\\x.ps1'")`, + `runRemote("powershell.exe -file $scriptVar")`, + `runRemote(["-NoProfile", "-File", scriptVar])` + ], + ignores: [`fetchWith("--credential-file", path)`, `run("--log-file $p --body-file $b")`] + }, + { + // The quote/backtick prefixes matter: a dot-source in a generated payload usually sits at + // the very start of a TS string literal — `powerShellCommand(". '$x'")` — not after a `;`. + label: 'dot-sourcing', + pattern: /(^|[;{'"`]|\n)[ \t]*\.[ \t]+['"$]/, + catches: [ + `powerShellCommand(". '$profileScript'")`, + `powerShellCommand("$ErrorActionPreference = 'Stop'; . '$profile'")`, + `powerShellCommand(". $profileScript")` + ], + ignores: [ + `cp -a $sourcePath/. $destinationPath/`, + `Host key verification failed for $displayHost. $detail` + ] + } +] as const + +describe('remote PowerShell payload invariant', () => { + // `scanSourceTree` is the shared walk: it skips node_modules/dist/out/build/.git, + // dot-directories and `__fixtures__`, and excludes tests by the shared `isTestFile` (which + // also covers `.spec.ts`, `__tests__/` and `-test-harness.ts`). A hand-rolled walk that got + // any of those wrong would move the floor below, which is this guard's own goalpost. + const importers = scanSourceTree(MAIN_DIR).filter((file) => + file.source.includes('ssh-remote-powershell') + ) + + it('finds the modules that build remote payloads', () => { + // Guards the scan itself: a resolution change that emptied this list would make every + // assertion below vacuously pass. 15 importers today, re-derived against the shared walk. + expect(importers.length).toBeGreaterThan(10) + }) + + it.each(POLICY_GATED_CONSTRUCTS)('loads no remote payload through $label', ({ + label, + pattern + }) => { + const offenders = importers + .filter((file) => pattern.test(stripComments(file.source))) + .map((file) => file.relativePath) + + expect( + offenders, + `${offenders.join(', ')} uses ${label}, which IS execution-policy gated on the remote ` + + 'host. Do not restore `-ExecutionPolicy Bypass` to the command line (a GPO scope ' + + 'beats it). Set the policy in-payload at process scope instead — see the note on ' + + 'powerShellCommand.' + ).toEqual([]) + }) + + // Why: these patterns only earn trust if they fire on a real violation spelled the way a + // generated payload spells it — inside a TS string literal — and stay quiet on the near + // misses. Both halves are load-bearing: an earlier dot-source pattern passed a `;`-prefixed + // sample but missed `powerShellCommand(". '$x'")`, and the obvious case-insensitive fix for + // `-File` matches `--credential-file` in three real importers. A fixture written from the + // pattern confirms the pattern; these are written from the requirement. + it.each(POLICY_GATED_CONSTRUCTS)('detects $label wherever it is spelled', ({ + pattern, + catches, + ignores + }) => { + for (const sample of catches) { + expect(pattern.test(sample), `should catch: ${sample}`).toBe(true) + } + for (const sample of ignores) { + expect(pattern.test(sample), `should ignore: ${sample}`).toBe(false) + } + }) +}) diff --git a/src/main/ssh/ssh-remote-powershell.ts b/src/main/ssh/ssh-remote-powershell.ts index 420223ced29..31587bcc668 100644 --- a/src/main/ssh/ssh-remote-powershell.ts +++ b/src/main/ssh/ssh-remote-powershell.ts @@ -18,6 +18,27 @@ const WINDOWS_REMOTE_COMMAND_LINE_BUDGET_CHARS = 8_000 */ export type WindowsPowerShellExecutable = 'powershell.exe' | 'pwsh.exe' +// Why: `-EncodedCommand` is not execution-policy gated (only `-File` is), so `-ExecutionPolicy +// Bypass` was a no-op here — and it is one of the most heavily EDR-flagged PowerShell tokens. +// The base64 stays: this string is re-parsed by the remote host's default SSH shell, which may +// be cmd.exe, PowerShell, or bash. +// +// INVARIANT — no remote payload may load a PowerShell *script file*. +// +// Execution policy has only ever gated loading script files (2.0 through 7.x). Inline +// statements, `& some.exe` and `Add-Type -TypeDefinition` are never gated, which is what makes +// dropping the switch a no-op for every payload we send today — the compressed path below stays +// inline too, since `Invoke-Expression` on a decompressed string loads no file. Loading a script +// file is the one thing the dropped switch actually covered, so a payload that dot-sources, runs +// `& '.ps1'`, calls `Import-Module '.psm1'`, or passes `-File` would silently fail on a +// remote host whose LocalMachine policy is Restricted/AllSigned with no GPO — a break that +// surfaces on someone else's machine, not ours. +// +// If you ever need one, do NOT restore the command-line switch (it loses to a GPO scope anyway, +// so it never covered the locked-down case): set the policy in-payload at process scope, the way +// `buildWindowsStartupCommand` in src/shared/setup-agent-sequencing.ts does. +// +// Enforced by the ratchet in ssh-remote-powershell.test.ts, which scans every importer. export function powerShellCommand( script: string, executable: WindowsPowerShellExecutable = 'powershell.exe' @@ -38,7 +59,7 @@ export function powerShellCommand( } function encodedPowerShellCommand(script: string, executable: WindowsPowerShellExecutable): string { - return `${executable} -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand ${encodePowerShellCommand(script)}` + return `${executable} -NoProfile -NonInteractive -EncodedCommand ${encodePowerShellCommand(script)}` } /** Orca-prefixed names so the payload can never shadow the bootstrap's own state. */ diff --git a/src/shared/setup-agent-sequencing.test.ts b/src/shared/setup-agent-sequencing.test.ts index fd567c145b0..1b8d69f0aa5 100644 --- a/src/shared/setup-agent-sequencing.test.ts +++ b/src/shared/setup-agent-sequencing.test.ts @@ -271,13 +271,13 @@ describe('createSequencedSetupAgentCommands', () => { const startupPowerShell = decodePowerShellScript(result.startupCommand) expect(result.setupCommand).toContain( - 'powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand' + 'powershell.exe -NoProfile -NonInteractive -EncodedCommand' ) expect(setupPowerShell).toContain("$runner = 'C:\\repo\\.git\\orca\\setup-runner.cmd'") expect(setupPowerShell).toContain('$nonce + ":" + $setupStatus') expect(result.startupCommand.match(/powershell\.exe/g)).toHaveLength(1) expect(result.startupCommand).toContain( - 'powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand' + 'powershell.exe -NoProfile -NonInteractive -EncodedCommand' ) expect(startupPowerShell).toContain('AddSeconds(3)') expect(startupPowerShell).toContain('Missing setup marker path.') @@ -292,6 +292,31 @@ describe('createSequencedSetupAgentCommands', () => { expect(result.startupEnv).toEqual({ [SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV]: "codex --model gpt-5 'fix !PATH! & test'" }) + // Why: `-EncodedCommand` is not execution-policy gated — only `-File` is — so the switch + // was a no-op, and base64 beside `-ExecutionPolicy Bypass` is a heavily EDR-flagged shape. + // The base64 itself must stay: these strings are typed into a terminal pane. + expect(result.setupCommand).not.toMatch(/-ExecutionPolicy/i) + expect(result.startupCommand).not.toMatch(/-ExecutionPolicy/i) + // Why: dropping the switch alone would break a user startup command that invokes a + // `.ps1` — a `.ps1` IS policy gated even though `-EncodedCommand` is not. The relief + // moves into the payload, where it is not part of the flagged command-line shape. + expect(startupPowerShell).toContain( + 'Set-ExecutionPolicy -Scope Process -ExecutionPolicy Bypass -Force -ErrorAction Stop' + ) + // Why `-ErrorAction Stop` and a reporting catch: autoload can fail for reasons that are + // not about policy at all (a 5.1 install with duplicate extended type data fails every + // cmdlet in Microsoft.PowerShell.Security), and the old SilentlyContinue plus `catch {}` + // hid that -- the user saw only their own script being refused. The catch must report and + // must NOT rethrow, or a broken policy cmdlet would take the whole startup with it. + expect(startupPowerShell).not.toContain('catch {}') + expect(startupPowerShell).toMatch(/catch \{ \[Console\]::Error\.WriteLine\(/) + expect(startupPowerShell).toContain('$_.FullyQualifiedErrorId') + expect(startupPowerShell).not.toMatch(/catch \{[^}]*throw/) + // Why: the autoloaded module's progress record would otherwise corrupt this gate's stderr. + expect(startupPowerShell).toContain("$ProgressPreference = 'SilentlyContinue'") + expect(startupPowerShell).toContain('$ProgressPreference = $orcaProgress') + // The setup gate only ever launches a .cmd/.bat runner, so it needs no relief. + expect(setupPowerShell).not.toMatch(/Set-ExecutionPolicy/i) }) it('launches a batch runner through the cmd launcher inside a Git Bash gate', () => { @@ -307,7 +332,7 @@ describe('createSequencedSetupAgentCommands', () => { }) expect(result.setupCommand).toContain( - 'powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand' + 'powershell.exe -NoProfile -NonInteractive -EncodedCommand' ) expect(result.setupCommand).not.toMatch(/bash\s+\S*setup-runner/) expect(decodePowerShellScript(result.setupCommand)).toContain( diff --git a/src/shared/setup-agent-sequencing.ts b/src/shared/setup-agent-sequencing.ts index 7108360f645..367be99c212 100644 --- a/src/shared/setup-agent-sequencing.ts +++ b/src/shared/setup-agent-sequencing.ts @@ -227,6 +227,27 @@ function buildWindowsStartupCommand( // Why: native Windows setup runners launch through cmd.exe, but PowerShell // gives us safe bounded file polling/parsing without a fragile batch label loop. const script = [ + // Why: the startup command is user-authored and may invoke a `.ps1`, which IS + // execution-policy gated even though `-EncodedCommand` is not. This is the in-payload + // stand-in for the `-ExecutionPolicy Bypass` switch dropped from the command line + // (same trade as the agent-hooks launcher). Progress must be silenced first and + // restored after: Set-ExecutionPolicy autoloads a module whose "Preparing modules for + // first use." record would otherwise land on the stderr this gate writes to. + // + // The failure is reported rather than swallowed. Autoload can fail for reasons that + // have nothing to do with policy -- a 5.1 install with duplicate extended type data + // fails every cmdlet in Microsoft.PowerShell.Security -- and the old + // `-ErrorAction SilentlyContinue` plus empty `catch` hid that completely, leaving the + // user with an execution-policy refusal from their own script and no trace that the + // relief had been attempted. `-ErrorAction Stop` is what routes a non-terminating + // failure into the catch at all. Still never throws: a diagnostic is worth a line of + // stderr, but not the startup this gate exists to run. + "$orcaProgress = $ProgressPreference; $ProgressPreference = 'SilentlyContinue'", + 'try { Set-ExecutionPolicy -Scope Process -ExecutionPolicy Bypass -Force -ErrorAction Stop } ' + + 'catch { [Console]::Error.WriteLine("Orca: could not relax the execution policy for this " + ' + + '"session (" + $_.FullyQualifiedErrorId + "). A startup command that runs a .ps1 " + ' + + '"may be blocked.") }', + '$ProgressPreference = $orcaProgress', `$marker = ${quotePowerShellString(markerPath)}`, 'if ([string]::IsNullOrWhiteSpace($marker)) {', ' [Console]::Error.WriteLine("Missing setup marker path.")', @@ -269,8 +290,11 @@ function buildWindowsStartupCommand( return encodePowerShellInvocation(script) } +// Why: `-EncodedCommand` is not execution-policy gated (only `-File` is), so `-ExecutionPolicy +// Bypass` was a no-op — and it is one of the most heavily EDR-flagged PowerShell tokens. The +// base64 stays: these strings are typed into a terminal pane and re-parsed by its shell. function encodePowerShellInvocation(script: string): string { - return `powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand ${encodePowerShellCommand(script)}` + return `powershell.exe -NoProfile -NonInteractive -EncodedCommand ${encodePowerShellCommand(script)}` } function quotePosixArg(value: string): string { diff --git a/src/shared/setup-runner-command.test.ts b/src/shared/setup-runner-command.test.ts index 069130b1313..4280ed4f6e0 100644 --- a/src/shared/setup-runner-command.test.ts +++ b/src/shared/setup-runner-command.test.ts @@ -82,7 +82,7 @@ describe('buildSetupRunnerCommand', () => { expect(command).not.toContain('cmd.exe /c') expect(command).toMatch( - /^powershell\.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand [A-Za-z0-9+/=]+$/ + /^powershell\.exe -NoProfile -NonInteractive -EncodedCommand [A-Za-z0-9+/=]+$/ ) }) @@ -129,7 +129,7 @@ describe('buildSetupRunnerCommand cmd metacharacter guard', () => { }) expect(command).toMatch( - /^powershell\.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand [A-Za-z0-9+/=]+$/ + /^powershell\.exe -NoProfile -NonInteractive -EncodedCommand [A-Za-z0-9+/=]+$/ ) } ) diff --git a/src/shared/windows-cmd-runner-delayed-launch.test.ts b/src/shared/windows-cmd-runner-delayed-launch.test.ts new file mode 100644 index 00000000000..21c5cc8befe --- /dev/null +++ b/src/shared/windows-cmd-runner-delayed-launch.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from 'vitest' +import { buildWindowsCmdRunnerDelayedLaunchCommand } from './windows-cmd-runner-delayed-launch' + +function decodePayload(command: string): string { + const encoded = command.match(/ -EncodedCommand (\S+)$/)?.[1] + if (!encoded) { + throw new Error(`no -EncodedCommand payload in: ${command}`) + } + return Buffer.from(encoded, 'base64').toString('utf16le') +} + +describe('buildWindowsCmdRunnerDelayedLaunchCommand', () => { + it('spells no -ExecutionPolicy switch', () => { + const command = buildWindowsCmdRunnerDelayedLaunchCommand('C:\\work\\setup.cmd') + const switches = command.replace(/ -EncodedCommand \S+$/, '') + + // Why: `-EncodedCommand` is not execution-policy gated — only `-File` is — so the switch + // was a no-op next to a heavily EDR-flagged base64 command line. + expect(switches).not.toMatch(/-ExecutionPolicy/i) + expect(switches).not.toMatch(/Bypass/i) + expect(switches).toBe('powershell.exe -NoProfile -NonInteractive') + }) + + it('keeps the base64 that shields the runner path from the pane shell', () => { + // Why: this whole module exists because the path carries cmd metacharacters; the + // command is typed into a terminal pane, so the base64 must stay. + const command = buildWindowsCmdRunnerDelayedLaunchCommand('C:\\work (x86)\\se&tup.cmd') + + expect(command).toMatch( + /^powershell\.exe -NoProfile -NonInteractive -EncodedCommand [A-Za-z0-9+/=]+$/ + ) + const script = decodePayload(command) + expect(script).toContain("$runner = 'C:\\work (x86)\\se&tup.cmd'") + expect(script).toContain('/d /s /v:on /c ""!ORCA_SETUP_RUNNER!""') + }) +}) diff --git a/src/shared/windows-cmd-runner-delayed-launch.ts b/src/shared/windows-cmd-runner-delayed-launch.ts index 50ed131dc99..cdc0af3a48a 100644 --- a/src/shared/windows-cmd-runner-delayed-launch.ts +++ b/src/shared/windows-cmd-runner-delayed-launch.ts @@ -34,7 +34,10 @@ export function buildWindowsCmdRunnerDelayedLaunchCommand(runnerScriptPath: stri 'exit $process.ExitCode' ].join('; ') - return `powershell.exe -NoProfile -NonInteractive -ExecutionPolicy Bypass -EncodedCommand ${encodePowerShellCommand(script)}` + // Why: `-EncodedCommand` is not execution-policy gated (only `-File` is), so `-ExecutionPolicy + // Bypass` was a no-op — and it is one of the most heavily EDR-flagged PowerShell tokens. The + // base64 stays: this string is typed into a shell, which is the whole point of the guard above. + return `powershell.exe -NoProfile -NonInteractive -EncodedCommand ${encodePowerShellCommand(script)}` } function quotePowerShellString(value: string): string { diff --git a/src/shared/windows-interactive-login-spawn.test.ts b/src/shared/windows-interactive-login-spawn.test.ts index 07cde964095..ef99f8edabe 100644 --- a/src/shared/windows-interactive-login-spawn.test.ts +++ b/src/shared/windows-interactive-login-spawn.test.ts @@ -17,8 +17,14 @@ function encodedValue(value: string): string { return `Read-OrcaValue '${Buffer.from(value).toString('base64')}'` } +/** Positional-independent so the argv shape can change without silently reading the wrong slot. */ +function decodedScript(args: string[]): string { + const payload = args[args.indexOf('-EncodedCommand') + 1] ?? '' + return Buffer.from(payload, 'base64').toString('utf16le') +} + function pidFilePathFromSpawnArgs(args: string[]): string { - const script = Buffer.from(args[11] ?? '', 'base64').toString('utf16le') + const script = decodedScript(args) const encodedPath = script.match( /WriteAllText\(\(Read-OrcaValue '([^']+)'\), \[string\]\$PID\)/ )?.[1] @@ -40,15 +46,15 @@ describe('buildWindowsHostInteractiveLoginSpawn', () => { expect(spawn.command).toBe(getCmdExePath()) expect(spawn.args.slice(0, 5)).toEqual(['/d', '/c', 'start', '', '/wait']) expect(spawn.args[5]).toMatch(/WindowsPowerShell\\v1\.0\\powershell\.exe$/i) - expect(spawn.args.slice(6, 11)).toEqual([ - '-NoLogo', - '-NoProfile', - '-ExecutionPolicy', - 'Bypass', - '-EncodedCommand' - ]) + // Why: `-ExecutionPolicy Bypass` is a no-op next to `-EncodedCommand` (only `-File` is + // policy gated) and is a heavily EDR-flagged token, so it must not come back. The base64 + // must stay — `start` re-parses this through cmd.exe, whose safe-token guard rejects the + // `&` and `"` in the raw relay script. + expect(spawn.args.slice(6, 9)).toEqual(['-NoLogo', '-NoProfile', '-EncodedCommand']) + expect(spawn.args).not.toContain('-ExecutionPolicy') + expect(spawn.args).not.toContain('Bypass') - const script = Buffer.from(spawn.args[11] ?? '', 'base64').toString('utf16le') + const script = decodedScript(spawn.args) expect(script).toContain('[string]$PID') expect(script).toContain(encodedValue(getCmdExePath())) expect(script).toContain(encodedValue('C:\\Tools\\claude.cmd')) @@ -68,7 +74,7 @@ describe('buildWindowsHostInteractiveLoginSpawn', () => { const spawn = withWindows(() => buildWindowsHostInteractiveLoginSpawn('C:\\Tools\\codex.exe', ['login']) ) - const script = Buffer.from(spawn.args[11] ?? '', 'base64').toString('utf16le') + const script = decodedScript(spawn.args) expect(script).toContain(encodedValue('C:\\Tools\\codex.exe')) expect(script).toContain(encodedValue('login')) spawn.cleanup() diff --git a/src/shared/windows-interactive-login-spawn.ts b/src/shared/windows-interactive-login-spawn.ts index 1068607e38f..a38ba1ae7a8 100644 --- a/src/shared/windows-interactive-login-spawn.ts +++ b/src/shared/windows-interactive-login-spawn.ts @@ -78,11 +78,13 @@ export function buildWindowsHostInteractiveLoginSpawn( 'powershell.exe' ) const script = buildPidRelayScript(spawnCmd, spawnArgs, pidFilePath) + // Why: `-EncodedCommand` is not execution-policy gated (only `-File` is), so `-ExecutionPolicy + // Bypass` was a no-op — and it is one of the most heavily EDR-flagged PowerShell tokens. The + // base64 stays: `wrapWindowsStartWait` sends this through `cmd.exe /c start`, whose + // `assertWindowsCmdSafeTokens` guard rejects the `&` and `"` the raw relay script contains. const wrapped = wrapWindowsStartWait(powershell, [ '-NoLogo', '-NoProfile', - '-ExecutionPolicy', - 'Bypass', '-EncodedCommand', Buffer.from(script, 'utf16le').toString('base64') ])