| author | |
| committer | |
| log | c08093c2b136353bc755459febf3e8afd46bf50e |
| tree | caf38ab6b1a6bc72110baa52e250a80eb2de62a6 |
| parent | f5b5f9dd7be29c4f55f4687dda15e4105cf966fc |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>8 files changed, 89 insertions(+), 75 deletions(-)
apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts+5-2| ... | @@ -693,8 +693,11 @@ describe("CodexAdapterV2 process spawning", () => { | ... | @@ -693,8 +693,11 @@ describe("CodexAdapterV2 process spawning", () => { |
| 693 | if (!ChildProcess.isStandardCommand(command)) { | 693 | if (!ChildProcess.isStandardCommand(command)) { |
| 694 | return; | 694 | return; |
| 695 | } | 695 | } |
| 696 | assert.equal(command.command, '^"C:\\npm\\codex.cmd^"'); | 696 | assert.equal( |
| 697 | assert.deepEqual(command.args, ['^"app-server^"', '^"argument^ with^ spaces^"']); | 697 | command.command, |
| 698 | '^"C:\\npm\\codex.cmd^" ^"app-server^" ^"argument^ with^ spaces^"', | ||
| 699 | ); | ||
| 700 | assert.deepEqual(command.args, []); | ||
| 698 | assert.equal(command.options.shell, true); | 701 | assert.equal(command.options.shell, true); |
| 699 | assert.equal(command.options.cwd, "C:\\workspace"); | 702 | assert.equal(command.options.cwd, "C:\\workspace"); |
| 700 | assert.deepEqual(command.options.env, { CUSTOM: "1" }); | 703 | assert.deepEqual(command.options.env, { CUSTOM: "1" }); |
apps/server/src/process/externalLauncher.test.ts+29-18| ... | @@ -153,11 +153,15 @@ it.effect("launches an installed editor with platform-safe arguments", () => | ... | @@ -153,11 +153,15 @@ it.effect("launches an installed editor with platform-safe arguments", () => |
| 153 | ); | 153 | ); |
| 154 | 154 | ||
| 155 | assert.ok(spawned); | 155 | assert.ok(spawned); |
| 156 | assert.equal(spawned.command, '^"C:\\Program^ Files\\Microsoft^ VS^ Code\\bin\\code.CMD^"'); | 156 | assert.equal( |
| 157 | assert.deepEqual(spawned.args, [ | 157 | spawned.command, |
| 158 | '^"--goto^"', | 158 | [ |
| 159 | '^"C:\\workspace^ with^ spaces\\src\\index.ts:12:4^"', | 159 | '^"C:\\Program^ Files\\Microsoft^ VS^ Code\\bin\\code.CMD^"', |
| 160 | ]); | 160 | '^"--goto^"', |
| 161 | '^"C:\\workspace^ with^ spaces\\src\\index.ts:12:4^"', | ||
| 162 | ].join(" "), | ||
| 163 | ); | ||
| 164 | assert.deepEqual(spawned.args, []); | ||
| 161 | assert.equal(spawned.options.shell, true); | 165 | assert.equal(spawned.options.shell, true); |
| 162 | }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), | 166 | }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), |
| 163 | ); | 167 | ); |
| ... | @@ -241,12 +245,16 @@ it.effect("launches Cursor in classic IDE mode through the Windows command shim" | ... | @@ -241,12 +245,16 @@ it.effect("launches Cursor in classic IDE mode through the Windows command shim" |
| 241 | ); | 245 | ); |
| 242 | 246 | ||
| 243 | assert.ok(spawned); | 247 | assert.ok(spawned); |
| 244 | assert.equal(spawned.command, '^"C:\\Program^ Files\\Cursor\\bin\\cursor.CMD^"'); | 248 | assert.equal( |
| 245 | assert.deepEqual(spawned.args, [ | 249 | spawned.command, |
| 246 | '^"--classic^"', | 250 | [ |
| 247 | '^"--goto^"', | 251 | '^"C:\\Program^ Files\\Cursor\\bin\\cursor.CMD^"', |
| 248 | '^"C:\\workspace^ with^ spaces\\src\\index.ts:12:4^"', | 252 | '^"--classic^"', |
| 249 | ]); | 253 | '^"--goto^"', |
| 254 | '^"C:\\workspace^ with^ spaces\\src\\index.ts:12:4^"', | ||
| 255 | ].join(" "), | ||
| 256 | ); | ||
| 257 | assert.deepEqual(spawned.args, []); | ||
| 250 | assert.equal(spawned.options.shell, true); | 258 | assert.equal(spawned.options.shell, true); |
| 251 | }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), | 259 | }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), |
| 252 | ); | 260 | ); |
| ... | @@ -1033,12 +1041,13 @@ for (const { platform, installPath, editor, args } of [ | ... | @@ -1033,12 +1041,13 @@ for (const { platform, installPath, editor, args } of [ |
| 1033 | ), | 1041 | ), |
| 1034 | ); | 1042 | ); |
| 1035 | assert.ok(spawned); | 1043 | assert.ok(spawned); |
| 1044 | const shim = executable.endsWith(".cmd"); | ||
| 1036 | assert.equal( | 1045 | assert.equal( |
| 1037 | spawned.command, | 1046 | spawned.command, |
| 1038 | executable.endsWith(".cmd") ? `^"${executable.replaceAll(" ", "^ ")}^"` : executable, | 1047 | shim ? [`^"${executable.replaceAll(" ", "^ ")}^"`, ...args].join(" ") : executable, |
| 1039 | ); | 1048 | ); |
| 1040 | assert.deepEqual(spawned.args, args); | 1049 | assert.deepEqual(spawned.args, shim ? [] : args); |
| 1041 | assert.equal(spawned.options.shell, executable.endsWith(".cmd")); | 1050 | assert.equal(spawned.options.shell, shim); |
| 1042 | }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), | 1051 | }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), |
| 1043 | ); | 1052 | ); |
| 1044 | } | 1053 | } |
| ... | @@ -1476,10 +1485,12 @@ it.effect("starts a Windows command shim for unicode paths with cmd metacharacte | ... | @@ -1476,10 +1485,12 @@ it.effect("starts a Windows command shim for unicode paths with cmd metacharacte |
| 1476 | assert.isTrue(Exit.isSuccess(exit)); | 1485 | assert.isTrue(Exit.isSuccess(exit)); |
| 1477 | assert.ok(spawned); | 1486 | assert.ok(spawned); |
| 1478 | assert.equal(spawned.options.shell, true); | 1487 | assert.equal(spawned.options.shell, true); |
| 1479 | assert.deepEqual(spawned.args, [ | 1488 | assert.deepEqual(spawned.args, []); |
| 1480 | '^"--goto^"', | 1489 | assert.isTrue( |
| 1481 | '^"C:\\Users\\jö\\R^&D^ ^(100^%^)\\naïve^ ^^file^!.ts:3:7^"', | 1490 | spawned.command.endsWith( |
| 1482 | ]); | 1491 | ' ^"--goto^" ^"C:\\Users\\jö\\R^&D^ ^(100^%^)\\naïve^ ^^file^!.ts:3:7^"', |
| 1492 | ), | ||
| 1493 | ); | ||
| 1483 | }), | 1494 | }), |
| 1484 | ); | 1495 | ); |
| 1485 | 1496 |
apps/server/src/processRunner.test.ts+11-8| ... | @@ -127,14 +127,17 @@ describe("runProcess", () => { | ... | @@ -127,14 +127,17 @@ describe("runProcess", () => { |
| 127 | it.effect("resolves and escapes Windows command shims before spawning", () => { | 127 | it.effect("resolves and escapes Windows command shims before spawning", () => { |
| 128 | const spawner = makeSpawner((command) => | 128 | const spawner = makeSpawner((command) => |
| 129 | Effect.sync(() => { | 129 | Effect.sync(() => { |
| 130 | expect(command.command).toBe('^"C:\\Users\\tester\\AppData\\Roaming\\npm\\az.cmd^"'); | 130 | expect(command.command).toBe( |
| 131 | expect(command.args).toEqual([ | 131 | [ |
| 132 | '^"repos^"', | 132 | '^"C:\\Users\\tester\\AppData\\Roaming\\npm\\az.cmd^"', |
| 133 | '^"pr^"', | 133 | '^"repos^"', |
| 134 | '^"list^"', | 134 | '^"pr^"', |
| 135 | '^"--source-branch^"', | 135 | '^"list^"', |
| 136 | '^"feature^ ^&^ release^"', | 136 | '^"--source-branch^"', |
| 137 | ]); | 137 | '^"feature^ ^&^ release^"', |
| 138 | ].join(" "), | ||
| 139 | ); | ||
| 140 | expect(command.args).toEqual([]); | ||
| 138 | expect(command.options.shell).toBe(true); | 141 | expect(command.options.shell).toBe(true); |
| 139 | return makeHandle({ stdout: "[]" }); | 142 | return makeHandle({ stdout: "[]" }); |
| 140 | }), | 143 | }), |
apps/server/src/provider/providerMaintenanceRunner.test.ts+6-11| ... | @@ -943,21 +943,16 @@ describe("providerMaintenanceRunner", () => { | ... | @@ -943,21 +943,16 @@ describe("providerMaintenanceRunner", () => { |
| 943 | const result = yield* runner.updateProvider(CODEX_DRIVER); | 943 | const result = yield* runner.updateProvider(CODEX_DRIVER); |
| 944 | 944 | ||
| 945 | // On win32, resolveSpawnCommand resolves `npm` to the `.cmd` shim and | 945 | // On win32, resolveSpawnCommand resolves `npm` to the `.cmd` shim and |
| 946 | // routes the spawn through cmd.exe (shell: true), escaping every arg. | 946 | // routes the spawn through cmd.exe (shell: true) as one escaped line. |
| 947 | assert.strictEqual(captured.length, 1); | 947 | assert.strictEqual(captured.length, 1); |
| 948 | const call = captured[0]; | 948 | const call = captured[0]; |
| 949 | assert.ok(call, "expected the spawner to be invoked once"); | 949 | assert.ok(call, "expected the spawner to be invoked once"); |
| 950 | // The resolved command is the escaped `.cmd` path. Asserting the precise | 950 | assert.strictEqual( |
| 951 | // escaped string is brittle, so verify it carries the resolved shim and | 951 | call.command, |
| 952 | // that shell mode was used. | 952 | '^"C:\\fake\\npm\\npm.cmd^" ^"install^" ^"-g^" ^"@openai/codex@latest^"', |
| 953 | assert.match(call.command, /npm\.cmd/i); | 953 | ); |
| 954 | assert.deepEqual(call.args, []); | ||
| 954 | assert.strictEqual(call.shell, true); | 955 | assert.strictEqual(call.shell, true); |
| 955 | // Args are escaped for cmd.exe shell mode (each quoted) but still carry | ||
| 956 | // the original install command (`install -g @openai/codex@latest`) in order. | ||
| 957 | assert.strictEqual(call.args.length, 3); | ||
| 958 | assert.match(call.args[0] ?? "", /install/); | ||
| 959 | assert.match(call.args[1] ?? "", /-g/); | ||
| 960 | assert.match(call.args[2] ?? "", /@openai\/codex@latest/); | ||
| 961 | assert.strictEqual(result.providers[0]?.updateState?.status, "succeeded"); | 956 | assert.strictEqual(result.providers[0]?.updateState?.status, "succeeded"); |
| 962 | }).pipe( | 957 | }).pipe( |
| 963 | Effect.provide( | 958 | Effect.provide( |
packages/shared/src/shell.test.ts+25-11| ... | @@ -681,11 +681,19 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { | ... | @@ -681,11 +681,19 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { |
| 681 | }), | 681 | }), |
| 682 | ); | 682 | ); |
| 683 | 683 | ||
| 684 | it.effect("escapes the executable and arguments for Windows command shims", () => | 684 | it.effect("builds one escaped cmd.exe line for Windows command shims", () => |
| 685 | Effect.gen(function* () { | 685 | Effect.gen(function* () { |
| 686 | const command = yield* resolveSpawnCommand( | 686 | const command = yield* resolveSpawnCommand( |
| 687 | "vp", | 687 | "vp", |
| 688 | ["run", "value & calc", "%PATH%", 'quote"value'], | 688 | [ |
| 689 | "run", | ||
| 690 | "value & calc", | ||
| 691 | "%PATH%", | ||
| 692 | 'quote"value', | ||
| 693 | "C:\\dir with space\\", | ||
| 694 | 'slash\\"quote', | ||
| 695 | "x|y<z>^", | ||
| 696 | ], | ||
| 689 | { env: { PATH: "", PATHEXT: ".COM;.EXE;.BAT;.CMD" } }, | 697 | { env: { PATH: "", PATHEXT: ".COM;.EXE;.BAT;.CMD" } }, |
| 690 | ).pipe( | 698 | ).pipe( |
| 691 | Effect.provideService(HostProcess.Platform, "win32"), | 699 | Effect.provideService(HostProcess.Platform, "win32"), |
| ... | @@ -696,15 +704,21 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { | ... | @@ -696,15 +704,21 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { |
| 696 | ), | 704 | ), |
| 697 | ); | 705 | ); |
| 698 | 706 | ||
| 699 | expect(command.shell).toBe(true); | 707 | // No args alongside `shell: true`: Node prints DEP0190 for that pairing. |
| 700 | expect(command.command).not.toContain(" & "); | 708 | expect(command).toEqual({ |
| 701 | expect(command.command).toContain("^&"); | 709 | command: [ |
| 702 | expect(command.args).toEqual([ | 710 | String.raw`^"C:\Program^ Files\npm^ ^&^ tools\vp.cmd^"`, |
| 703 | '^"run^"', | 711 | String.raw`^"run^"`, |
| 704 | '^"value^ ^&^ calc^"', | 712 | String.raw`^"value^ ^&^ calc^"`, |
| 705 | '^"^%PATH^%^"', | 713 | String.raw`^"^%PATH^%^"`, |
| 706 | '^"quote\\^"value^"', | 714 | String.raw`^"quote\^"value^"`, |
| 707 | ]); | 715 | String.raw`^"C:\dir^ with^ space\\^"`, |
| 716 | String.raw`^"slash\\\^"quote^"`, | ||
| 717 | String.raw`^"x^|y^<z^>^^^"`, | ||
| 718 | ].join(" "), | ||
| 719 | args: [], | ||
| 720 | shell: true, | ||
| 721 | }); | ||
| 708 | }), | 722 | }), |
| 709 | ); | 723 | ); |
| 710 | 724 |
packages/shared/src/shell.ts+9-18| ... | @@ -46,10 +46,10 @@ const WINDOWS_SHELL_META_CHARS = /([()\][%!^"`<>&|;, *?])/g; | ... | @@ -46,10 +46,10 @@ const WINDOWS_SHELL_META_CHARS = /([()\][%!^"`<>&|;, *?])/g; |
| 46 | 46 | ||
| 47 | /** | 47 | /** |
| 48 | * Escapes a single argument for `cmd.exe` shell mode (`spawn(..., { shell: true })` | 48 | * Escapes a single argument for `cmd.exe` shell mode (`spawn(..., { shell: true })` |
| 49 | * on Windows). Node joins the command and arguments with spaces and hands the | 49 | * on Windows). Node hands the command string to `cmd.exe /d /s /c "<command>"` |
| 50 | * resulting string to `cmd.exe` without any quoting, so every dynamic argument | 50 | * without any quoting, so every dynamic argument must be escaped to survive both |
| 51 | * must be escaped to survive both cmd.exe parsing and the target program's | 51 | * cmd.exe parsing and the target program's `CommandLineToArgvW` parsing. |
| 52 | * `CommandLineToArgvW` parsing. Mirrors cross-spawn's argument escaping. | 52 | * Mirrors cross-spawn's argument escaping. |
| 53 | */ | 53 | */ |
| 54 | function escapeWindowsShellArg(arg: string): string { | 54 | function escapeWindowsShellArg(arg: string): string { |
| 55 | // Double up backslashes that precede a double quote, then escape the quote | 55 | // Double up backslashes that precede a double quote, then escape the quote |
| ... | @@ -63,18 +63,6 @@ function escapeWindowsShellArg(arg: string): string { | ... | @@ -63,18 +63,6 @@ function escapeWindowsShellArg(arg: string): string { |
| 63 | return escaped.replace(WINDOWS_SHELL_META_CHARS, "^$1"); | 63 | return escaped.replace(WINDOWS_SHELL_META_CHARS, "^$1"); |
| 64 | } | 64 | } |
| 65 | 65 | ||
| 66 | /** | ||
| 67 | * Escapes arguments for shell-mode spawns: applies {@link escapeWindowsShellArg} | ||
| 68 | * when the platform is `win32` (where `shell: true` routes through `cmd.exe`) | ||
| 69 | * and returns the arguments untouched everywhere else. | ||
| 70 | */ | ||
| 71 | function sanitizeShellModeArgsForPlatform( | ||
| 72 | args: ReadonlyArray<string>, | ||
| 73 | platform: NodeJS.Platform, | ||
| 74 | ): Array<string> { | ||
| 75 | return platform === "win32" ? args.map(escapeWindowsShellArg) : [...args]; | ||
| 76 | } | ||
| 77 | |||
| 78 | export interface ResolvedSpawnCommand { | 66 | export interface ResolvedSpawnCommand { |
| 79 | readonly command: string; | 67 | readonly command: string; |
| 80 | readonly args: ReadonlyArray<string>; | 68 | readonly args: ReadonlyArray<string>; |
| ... | @@ -773,9 +761,12 @@ export const resolveSpawnCommand = Effect.fnUntraced(function* ( | ... | @@ -773,9 +761,12 @@ export const resolveSpawnCommand = Effect.fnUntraced(function* ( |
| 773 | return { command: resolvedCommand, args: [...args], shell: false }; | 761 | return { command: resolvedCommand, args: [...args], shell: false }; |
| 774 | } | 762 | } |
| 775 | 763 | ||
| 764 | // Node runs a shell spawn as `cmd.exe /d /s /c "<command> <args...>"`, but | ||
| 765 | // passing args with `shell: true` prints DEP0190. Build the same escaped line | ||
| 766 | // here and pass no args. | ||
| 776 | return { | 767 | return { |
| 777 | command: escapeWindowsShellArg(resolvedCommand), | 768 | command: [resolvedCommand, ...args].map(escapeWindowsShellArg).join(" "), |
| 778 | args: sanitizeShellModeArgsForPlatform(args, platform), | 769 | args: [], |
| 779 | shell: true, | 770 | shell: true, |
| 780 | }; | 771 | }; |
| 781 | }); | 772 | }); |
scripts/dev-runner.ts+3-6| ... | @@ -791,15 +791,12 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { | ... | @@ -791,15 +791,12 @@ export function runDevRunnerWithInput(input: DevRunnerCliInput) { |
| 791 | } | 791 | } |
| 792 | } | 792 | } |
| 793 | 793 | ||
| 794 | const spawnCommand = yield* resolveSpawnCommand( | 794 | const args = [...MODE_ARGS[input.mode], ...input.runArgs]; |
| 795 | "vp", | 795 | const spawnCommand = yield* resolveSpawnCommand("vp", args, { env }); |
| 796 | [...MODE_ARGS[input.mode], ...input.runArgs], | ||
| 797 | { env }, | ||
| 798 | ); | ||
| 799 | const processContext = { | 796 | const processContext = { |
| 800 | mode: input.mode, | 797 | mode: input.mode, |
| 801 | executable: "vp" as const, | 798 | executable: "vp" as const, |
| 802 | argumentCount: spawnCommand.args.length, | 799 | argumentCount: args.length, |
| 803 | shell: spawnCommand.shell, | 800 | shell: spawnCommand.shell, |
| 804 | } as const; | 801 | } as const; |
| 805 | const child = yield* ChildProcess.make(spawnCommand.command, spawnCommand.args, { | 802 | const child = yield* ChildProcess.make(spawnCommand.command, spawnCommand.args, { |
scripts/mobile-native-static-check.ts+1-1| ... | @@ -127,7 +127,7 @@ export const runCommand = Effect.fn("runCommand")(function* ( | ... | @@ -127,7 +127,7 @@ export const runCommand = Effect.fn("runCommand")(function* ( |
| 127 | const spawnCommand = yield* resolveSpawnCommand(command, args); | 127 | const spawnCommand = yield* resolveSpawnCommand(command, args); |
| 128 | const processContext = { | 128 | const processContext = { |
| 129 | command, | 129 | command, |
| 130 | argumentCount: spawnCommand.args.length, | 130 | argumentCount: args.length, |
| 131 | cwd, | 131 | cwd, |
| 132 | shell: spawnCommand.shell, | 132 | shell: spawnCommand.shell, |
| 133 | } as const; | 133 | } as const; |