Skip to content

Commit 92fa992

Browse files
committed
refactor: extract isDirectExecution + windows-shim helpers into @coder-studio/utils
The server package and root build scripts previously kept identical copies of isDirectExecution (with its path-normalizing helpers) and the WINDOWS_CMD_SHIMS / shouldUseShellForCommand pair. Both areas care about the same Windows / ESM entry-point gotchas, so any fix needed to be made twice and risked drifting. - Add packages/utils workspace package mirroring the @coder-studio/core layout. - Move helpers into utils with co-located vitest coverage. - packages/server now depends on @coder-studio/utils; server.ts and provider-runtime/command-runner.ts import from it and the duplicate direct-execution.ts is removed. - scripts/shared/process.ts re-exports the helpers from utils (call sites unchanged) and the obsolete process.test.ts is dropped in favor of the new utils tests. - Wire @coder-studio/utils into the root devDependencies, ci:typecheck, the esbuild alias map, and scripts/shared/paths so the CLI bundle resolves it correctly.
1 parent 091c50c commit 92fa992

15 files changed

Lines changed: 136 additions & 98 deletions

package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@
1313
"version-packages": "changeset version",
1414
"publish:cli": "tsx scripts/publish-cli.ts",
1515
"ci:lint": "pnpm exec biome check --diagnostic-level=error --max-diagnostics=none .",
16-
"ci:test:scripts": "pnpm exec vitest run scripts/publish-cli.test.ts scripts/build-cli.test.ts scripts/validate-changesets.test.ts scripts/husky-pre-commit.test.ts scripts/shared/process.test.ts --environment node",
16+
"ci:test:scripts": "pnpm exec vitest run scripts/publish-cli.test.ts scripts/build-cli.test.ts scripts/validate-changesets.test.ts scripts/husky-pre-commit.test.ts --environment node",
1717
"ci:test:workspace": "pnpm -r --filter './packages/**' --if-present run test",
1818
"ci:test": "pnpm ci:test:scripts && pnpm ci:test:workspace",
19-
"ci:typecheck": "pnpm --filter @spencer-kit/coder-studio exec tsc -p tsconfig.json --noEmit && pnpm --filter @coder-studio/core exec tsc -p tsconfig.json --noEmit && pnpm --filter @coder-studio/providers exec tsc -p tsconfig.json --noEmit && pnpm --filter @coder-studio/server exec tsc -p tsconfig.json --noEmit && pnpm --filter @coder-studio/web exec tsc -p tsconfig.json --noEmit",
19+
"ci:typecheck": "pnpm --filter @spencer-kit/coder-studio exec tsc -p tsconfig.json --noEmit && pnpm --filter @coder-studio/utils exec tsc -p tsconfig.json --noEmit && pnpm --filter @coder-studio/core exec tsc -p tsconfig.json --noEmit && pnpm --filter @coder-studio/providers exec tsc -p tsconfig.json --noEmit && pnpm --filter @coder-studio/server exec tsc -p tsconfig.json --noEmit && pnpm --filter @coder-studio/web exec tsc -p tsconfig.json --noEmit",
2020
"ci:build": "pnpm build",
2121
"ci:verify": "pnpm changeset:validate && pnpm ci:lint && pnpm ci:test && pnpm ci:build",
2222
"ci:release:validate": "pnpm ci:verify && pnpm publish:cli -- --no-build",
@@ -33,6 +33,7 @@
3333
"@biomejs/biome": "^2.4.14",
3434
"@changesets/changelog-github": "^0.7.0",
3535
"@changesets/cli": "^2.31.0",
36+
"@coder-studio/utils": "workspace:*",
3637
"@types/node": "^25.6.0",
3738
"esbuild": "^0.28.0",
3839
"husky": "^9.1.7",

packages/server/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
"dependencies": {
3232
"@coder-studio/core": "workspace:*",
3333
"@coder-studio/providers": "workspace:*",
34+
"@coder-studio/utils": "workspace:*",
3435
"@fastify/compress": "^8.3.1",
3536
"@fastify/cors": "^11.2.0",
3637
"@fastify/multipart": "^10.0.0",

packages/server/src/provider-runtime/command-runner.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { spawn } from "node:child_process";
2+
import { shouldUseShellForCommand } from "@coder-studio/utils";
23

34
export type CommandRunnerOptions = { windowsHide?: boolean };
45

@@ -13,15 +14,6 @@ export type CommandRunner = (
1314
options?: CommandRunnerOptions
1415
) => Promise<CommandRunnerResult>;
1516

16-
// Windows ships these as .cmd shims that Node refuses to spawn directly post
17-
// CVE-2024-27980. Routing them through cmd.exe via shell:true is the only
18-
// approach that survives both ENOENT (bare name) and EINVAL (full .cmd path).
19-
const WINDOWS_CMD_SHIMS = new Set(["pnpm", "npm", "npx"]);
20-
21-
function shouldUseShellForCommand(file: string, platform: NodeJS.Platform): boolean {
22-
return platform === "win32" && WINDOWS_CMD_SHIMS.has(file.toLowerCase());
23-
}
24-
2517
export async function runCommandAsString(
2618
file: string,
2719
args: string[],

packages/server/src/server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
writeRuntimeConfig,
1111
} from "@coder-studio/core/runtime";
1212
import { providerRegistry } from "@coder-studio/providers";
13+
import { isDirectExecution } from "@coder-studio/utils";
1314
import type { FastifyInstance } from "fastify";
1415
import { buildFastifyApp } from "./app.js";
1516
import { EventBus } from "./bus/event-bus.js";
@@ -21,7 +22,6 @@ import {
2122
cleanupCodexConfigToml,
2223
} from "./config/codex-config-audit.js";
2324
import { ensureDataDir, parseServerConfig, type ServerConfig } from "./config.js";
24-
import { isDirectExecution } from "./direct-execution.js";
2525
import { runCommandAsString } from "./provider-runtime/command-runner.js";
2626
import { ProviderInstallManager } from "./provider-runtime/install-manager.js";
2727
import type { RuntimeStatusDeps } from "./provider-runtime/runtime-status.js";

packages/utils/package.json

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
{
2+
"name": "@coder-studio/utils",
3+
"version": "0.0.1",
4+
"private": true,
5+
"type": "module",
6+
"main": "./src/index.ts",
7+
"exports": {
8+
".": {
9+
"types": "./src/index.ts",
10+
"default": "./src/index.ts"
11+
}
12+
},
13+
"publishConfig": {
14+
"main": "./dist/index.js",
15+
"types": "./dist/index.d.ts",
16+
"exports": {
17+
".": {
18+
"types": "./dist/index.d.ts",
19+
"import": "./dist/index.js"
20+
}
21+
}
22+
},
23+
"scripts": {
24+
"build": "tsc -p tsconfig.json",
25+
"test": "vitest run",
26+
"test:watch": "vitest"
27+
},
28+
"devDependencies": {
29+
"@types/node": "^25.6.0",
30+
"typescript": "^6.0.3",
31+
"vitest": "^4.1.5"
32+
}
33+
}
Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from "vitest";
2-
import { isDirectExecution, shouldUseShellForCommand } from "./process.js";
2+
import { isDirectExecution } from "./direct-execution.js";
33

44
describe("isDirectExecution", () => {
55
it("matches direct execution for POSIX script paths", () => {
@@ -15,18 +15,12 @@ describe("isDirectExecution", () => {
1515
it("returns false when the current module differs from argv[1]", () => {
1616
expect(isDirectExecution("file:///repo/scripts/build.ts", "/repo/scripts/dev.ts")).toBe(false);
1717
});
18-
});
19-
20-
describe("shouldUseShellForCommand", () => {
21-
it("uses a shell for pnpm on Windows because pnpm.cmd is not directly executable", () => {
22-
expect(shouldUseShellForCommand("pnpm", "win32")).toBe(true);
23-
});
2418

25-
it("does not use a shell for native executables like git on Windows", () => {
26-
expect(shouldUseShellForCommand("git", "win32")).toBe(false);
19+
it("returns false when argv[1] is undefined", () => {
20+
expect(isDirectExecution("file:///repo/scripts/dev.ts", undefined)).toBe(false);
2721
});
2822

29-
it("does not use a shell on POSIX platforms", () => {
30-
expect(shouldUseShellForCommand("pnpm", "linux")).toBe(false);
23+
it("returns false when moduleUrl is not a file: URL", () => {
24+
expect(isDirectExecution("https://example.com/dev.ts", "/repo/scripts/dev.ts")).toBe(false);
3125
});
3226
});
Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ function normalizeModuleUrlPath(moduleUrl: string): string | null {
4141
return normalizeComparablePath(path);
4242
}
4343

44+
function normalizeArgvPath(argv1: string): string {
45+
const isAbsoluteWindowsPath = /^[A-Za-z]:[\\/]/.test(argv1) || /^\\\\/.test(argv1);
46+
return normalizeComparablePath(isAbsoluteWindowsPath ? argv1 : resolve(argv1));
47+
}
48+
4449
export function isDirectExecution(
4550
moduleUrl: string,
4651
argv1: string | undefined = process.argv[1]
@@ -55,8 +60,5 @@ export function isDirectExecution(
5560
return false;
5661
}
5762

58-
const isAbsoluteWindowsPath = /^[A-Za-z]:[\\/]/.test(argv1) || /^\\\\/.test(argv1);
59-
const argvPath = normalizeComparablePath(isAbsoluteWindowsPath ? argv1 : resolve(argv1));
60-
61-
return modulePath === argvPath;
63+
return modulePath === normalizeArgvPath(argv1);
6264
}

packages/utils/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export { isDirectExecution } from "./direct-execution.js";
2+
export { shouldUseShellForCommand } from "./windows-shim.js";
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { describe, expect, it } from "vitest";
2+
import { shouldUseShellForCommand } from "./windows-shim.js";
3+
4+
describe("shouldUseShellForCommand", () => {
5+
it("uses a shell for pnpm on Windows because pnpm.cmd is not directly executable", () => {
6+
expect(shouldUseShellForCommand("pnpm", "win32")).toBe(true);
7+
});
8+
9+
it("uses a shell for npm and npx on Windows", () => {
10+
expect(shouldUseShellForCommand("npm", "win32")).toBe(true);
11+
expect(shouldUseShellForCommand("npx", "win32")).toBe(true);
12+
});
13+
14+
it("does not use a shell for native executables like git on Windows", () => {
15+
expect(shouldUseShellForCommand("git", "win32")).toBe(false);
16+
});
17+
18+
it("does not use a shell on POSIX platforms", () => {
19+
expect(shouldUseShellForCommand("pnpm", "linux")).toBe(false);
20+
expect(shouldUseShellForCommand("pnpm", "darwin")).toBe(false);
21+
});
22+
23+
it("matches shim names case-insensitively", () => {
24+
expect(shouldUseShellForCommand("NPM", "win32")).toBe(true);
25+
expect(shouldUseShellForCommand("Pnpm", "win32")).toBe(true);
26+
});
27+
});

packages/utils/src/windows-shim.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* Helpers for spawning child processes that may resolve to Windows .cmd / .bat
3+
* shims.
4+
*
5+
* Why: Node 18.20.2 / 20.12.2 / 21.7.2 (CVE-2024-27980) refuses to spawn
6+
* .cmd or .bat files unless `shell: true` is set. The shims below ship as
7+
* .cmd on Windows, so they need shell:true; native executables (git, etc.)
8+
* must keep shell:false to avoid breaking argument escaping.
9+
*/
10+
11+
const WINDOWS_CMD_SHIMS = new Set(["pnpm", "npm", "npx"]);
12+
13+
export function shouldUseShellForCommand(
14+
command: string,
15+
platform: NodeJS.Platform = process.platform
16+
): boolean {
17+
return platform === "win32" && WINDOWS_CMD_SHIMS.has(command.toLowerCase());
18+
}

0 commit comments

Comments
 (0)