From b00aaf1bbdf412fca3cc47486119821d633ac64b Mon Sep 17 00:00:00 2001 From: fi3ework Date: Tue, 11 Aug 2026 17:35:39 +0800 Subject: [PATCH 1/2] test(vscode): cover the rstack config bridge end to end - Add e2e/rstest/suite/bridge.test.ts: a folder whose only test signal is rstack.config.ts gets a bridged project - explorer tree (node-less layout + AST test cases) and a real run through rstack's shipped shim. - Bump the rstack fixture to rstack@0.5.0-alpha.1 (newest published); move its public-hoist settings from the fixture .npmrc (no longer read by pnpm 11) to shared install flags in setupFixtures.mjs. - Replace the enumerated test:e2e:* script chain with e2e/run.mjs: a SLICES table declares each slice's fixtures and entry, package.json scripts become thin forwards, and tsc compiles first to fail fast. - Record the tools-and-configs vocabulary (atomic tool, Rstack config, shim, bridged project, ownership) in CONTEXT.md. --- CONTEXT.md | 9 + packages/vscode/AGENTS.md | 2 +- packages/vscode/e2e/fixtures/rstack/.npmrc | 9 - packages/vscode/e2e/fixtures/rstack/.nvmrc | 1 + .../vscode/e2e/fixtures/rstack/package.json | 2 +- packages/vscode/e2e/rstest/runTest.ts | 20 ++- .../vscode/e2e/rstest/suite/bridge.test.ts | 157 ++++++++++++++++++ packages/vscode/e2e/rstest/suite/helpers.ts | 53 ++++++ packages/vscode/e2e/run.mjs | 102 ++++++++++++ packages/vscode/e2e/setupFixtures.mjs | 21 ++- packages/vscode/package.json | 12 +- 11 files changed, 362 insertions(+), 26 deletions(-) delete mode 100644 packages/vscode/e2e/fixtures/rstack/.npmrc create mode 100644 packages/vscode/e2e/fixtures/rstack/.nvmrc create mode 100644 packages/vscode/e2e/rstest/suite/bridge.test.ts create mode 100644 packages/vscode/e2e/run.mjs diff --git a/CONTEXT.md b/CONTEXT.md index a40b9ab..091ce87 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -17,6 +17,15 @@ Glossary of terms used across rstack-editor. Code, docs, commit messages and rev - **Preflight** — the check that picks a User Node runtime, run once per extension host before any worker is spawned. Its failure is a status, never a crash. - **Runtime floor** — the version range a User Node runtime must satisfy (`NODE_RUNTIME_RANGE` in `shared/versionCheck.ts`). A declared support contract, not a probed capability. +## Tools and configs + +- **Atomic tool** — a single Rstack tool used standalone (Rstest, Rslint). Each atomic tool's CLI reads only its own native config and has no knowledge of the Rstack config. _Avoid_: standalone tool, raw tool. +- **Native config** — the config file an atomic tool reads by itself (`rstest.config.*`, `rslint.config.*`). _Avoid_: tool config, own config. +- **Rstack config** — the unified `rstack.config.*` file consumed by rstack-cli (`rs`), holding per-tool sections. Tools never read it themselves; `rs` hands each tool its section through a shim. +- **Shim** — the module rstack-cli ships per tool that loads the Rstack config and exposes that tool's section through the tool's ordinary explicit-config channel. The extension points upstream machinery at the shim rather than re-implementing Rstack config semantics. +- **Bridged project** — a test project the extension synthesizes for a directory whose test signal is a Rstack config, wired to the shim. _Avoid_: virtual project, rstack project. +- **Ownership** — the editor-side rule assigning a directory to one tool when both a native config and a Rstack config are present there: the atomic tool's native config wins and the bridge yields. This rule exists only in the editor; upstream CLIs never face the choice, since each reads only its own config. + ## fmt - **Cold format** — a format request served by spawning a fresh `rs fmt` process at request time; the request pays the full process start-up cost. diff --git a/packages/vscode/AGENTS.md b/packages/vscode/AGENTS.md index 9b9b759..80a10da 100644 --- a/packages/vscode/AGENTS.md +++ b/packages/vscode/AGENTS.md @@ -43,4 +43,4 @@ One extension replacing the standalone `rstack.rslint` and `rstack.rstest` exten - E2E suites ported from upstream keep upstream's assertion semantics; every intentional deviation is documented in a comment in the test itself. A failing ported test is a regression, not a test to adjust. - E2E fixtures install published npm packages (not workspace links): the extension must work against what users actually install. Fixture `node_modules` are disposable and never committed. -- Prefer running the E2E slice that covers the change (`test:e2e:*` scripts; `RSTACK_LINT_E2E_SUITES=` filters lint suites) over the full chain. +- Prefer running the E2E slice that covers the change over the full chain: `pnpm test:e2e ` (or the `test:e2e:` aliases). Slices are declared in the `SLICES` table in `e2e/run.mjs` (name, fixtures, entry) — the package.json scripts are thin forwards and carry no slice knowledge. `RSTACK_LINT_E2E_SUITES=` filters lint suites. diff --git a/packages/vscode/e2e/fixtures/rstack/.npmrc b/packages/vscode/e2e/fixtures/rstack/.npmrc deleted file mode 100644 index d6cdbff..0000000 --- a/packages/vscode/e2e/fixtures/rstack/.npmrc +++ /dev/null @@ -1,9 +0,0 @@ -# `@rslint/core` and `@rstest/core` are transitive dependencies of `rstack`. -# npm and Yarn hoist those into the project's root `node_modules`, which is -# where this extension resolves them from; pnpm's isolated -# store does not, and a fixture that cannot resolve them would silently walk up -# into THIS REPO's own node_modules and test the extension's dev copies instead -# of the project's published ones. Hoisting them reproduces the layout the -# extension is designed against and keeps the fixture self-contained. -public-hoist-pattern[]=@rslint/core -public-hoist-pattern[]=@rstest/core diff --git a/packages/vscode/e2e/fixtures/rstack/.nvmrc b/packages/vscode/e2e/fixtures/rstack/.nvmrc new file mode 100644 index 0000000..6f4247a --- /dev/null +++ b/packages/vscode/e2e/fixtures/rstack/.nvmrc @@ -0,0 +1 @@ +26 diff --git a/packages/vscode/e2e/fixtures/rstack/package.json b/packages/vscode/e2e/fixtures/rstack/package.json index 4ab416e..6f1070e 100644 --- a/packages/vscode/e2e/fixtures/rstack/package.json +++ b/packages/vscode/e2e/fixtures/rstack/package.json @@ -5,7 +5,7 @@ "type": "module", "description": "E2E fixture: an rstack-cli project whose only config is `rstack.config.ts`, which lights the Rstest and rs fmt stacks.", "dependencies": { - "rstack": "^0.3.5" + "rstack": "0.5.0-alpha.1" }, "devDependencies": { "jiti": "^2.0.0" diff --git a/packages/vscode/e2e/rstest/runTest.ts b/packages/vscode/e2e/rstest/runTest.ts index 0bc2e2d..e3740dc 100644 --- a/packages/vscode/e2e/rstest/runTest.ts +++ b/packages/vscode/e2e/rstest/runTest.ts @@ -23,7 +23,17 @@ import { tmpdir } from 'node:os'; import path from 'node:path'; import { runTests } from '@vscode/test-electron'; -const FIXTURE_DIRS = ['workspace-1', 'workspace-2'] as const; +// Repo-relative fixture dirs whose installs this slice needs. workspace-2 has +// no node_modules of its own per project; the root install serves both nested +// projects, so the root is what the guard probes. `e2e/fixtures/rstack` is the +// shared fixture `suite/bridge.test.ts` adds as a second workspace folder (the +// same folder the `vscode` slice opens); the bridge resolves the rstack shim +// from its install. +const FIXTURE_DIRS = [ + 'e2e/rstest/fixtures/workspace-1', + 'e2e/rstest/fixtures/workspace-2', + 'e2e/fixtures/rstack', +] as const; async function main() { // `__dirname` is `/tests-dist/e2e/rstest` (see tsconfig.e2e.json). @@ -41,12 +51,10 @@ async function main() { 'dist/extension.js is missing — run `pnpm build` before `pnpm test:e2e:rstest`.', ); } - // workspace-2 has no node_modules of its own per project; the root install - // serves both nested projects, so the root is what the guard probes. - for (const name of FIXTURE_DIRS) { - if (!existsSync(path.join(fixturesRoot, name, 'node_modules'))) { + for (const dir of FIXTURE_DIRS) { + if (!existsSync(path.join(extensionDevelopmentPath, dir, 'node_modules'))) { throw new Error( - `the rstest/${name} E2E fixture is not installed — run \`pnpm test:e2e:fixtures\`.`, + `the ${dir} E2E fixture is not installed — run \`pnpm test:e2e:fixtures\`.`, ); } } diff --git a/packages/vscode/e2e/rstest/suite/bridge.test.ts b/packages/vscode/e2e/rstest/suite/bridge.test.ts new file mode 100644 index 0000000..f2189fc --- /dev/null +++ b/packages/vscode/e2e/rstest/suite/bridge.test.ts @@ -0,0 +1,157 @@ +// NOT ported from upstream — upstream's extension predates the rstack bridge. +// This suite covers the bridged-project path end to end: a folder whose only +// test signal is `rstack.config.ts` (the `e2e/fixtures/rstack` fixture, shared +// with the `vscode` slice) must get a synthesized project driven +// through rstack's shipped shim, show the same node-less tree a native root +// config gets, and actually run its tests through the worker. +// +// The fixture folder is added as a second workspace folder and removed again +// in teardown: `suite/index.ts` collects `*.test.js` sorted, so this suite runs +// *first*, and every suite after it (`index`, `progress`, ...) asserts on the +// unwrapped single-folder tree the run starts with. +// +// Adding a folder also flips the tree into its wrapped layout, so the probes +// below re-resolve the live controller through `currentRstestExports()` — a +// detection change can deregister and re-register the stack, which publishes a +// fresh `TestController` (same reason as `workspace.test.ts`). +import assert from 'node:assert'; +import path from 'node:path'; +import vscode from 'vscode'; +import { + createCollectingMockRun, + currentRstestExports, + FIXTURES_ROOT, + getRstestExports, + getTestItemByLabels, + toLabelTree, + waitFor, +} from './helpers'; + +/** `/e2e/fixtures/rstack` — the rstack-cli fixture, no tool-native config. */ +const RSTACK_FIXTURE = path.resolve(FIXTURES_ROOT, '../../fixtures/rstack'); +const RSTACK_FIXTURE_URI = vscode.Uri.file(RSTACK_FIXTURE); + +const WORKSPACE_1_FILES = [ + { label: 'each.test.ts' }, + { label: 'foo.test.ts' }, + { label: 'index.test.ts' }, + { label: 'jsFile.spec.js' }, + { label: 'jsxFile.test.jsx' }, + { label: 'progress.test.ts' }, + { label: 'tsxFile.test.tsx' }, +]; + +suite('Rstack bridge suite', () => { + suiteSetup(async () => { + await getRstestExports(); + const added = vscode.workspace.updateWorkspaceFolders( + vscode.workspace.workspaceFolders?.length || 0, + 0, + { uri: RSTACK_FIXTURE_URI }, + ); + assert.ok(added, 'adding the rstack fixture folder should be accepted'); + }); + + suiteTeardown(async () => { + // Compare `uri.toString()`, not `fsPath`: `fsPath` lower-cases the Windows + // drive letter while `path.resolve` keeps it as-is, so a raw string + // compare can miss on Windows — and a missed removal here would leak the + // folder into every later suite. + const index = vscode.workspace.workspaceFolders?.findIndex( + (folder) => folder.uri.toString() === RSTACK_FIXTURE_URI.toString(), + ); + assert.ok(index !== undefined && index >= 0); + const removed = vscode.workspace.updateWorkspaceFolders(index, 1); + assert.ok(removed, 'removing the rstack fixture folder should be accepted'); + // Later suites assert on the unwrapped single-folder tree; leave only + // after the controller has actually settled back into it. + await waitFor(() => { + const testController = currentRstestExports().testController; + assert.deepStrictEqual(toLabelTree(testController.items, true), [ + { label: 'test', children: WORKSPACE_1_FILES }, + ]); + }); + }); + + test('discovers a bridged project from rstack.config.ts alone', async () => { + // Two detected folders → both wrapped in workspace nodes. The rstack + // folder holds a single bridged project whose source config sits at the + // folder root under a default name, so it gets the node-less layout — + // structurally identical to workspace-1's native root config. This is the + // first suite this slice runs in a cold Electron, so the probe pays + // workspace-1's discovery AND the bridged project's first worker spawn + // (User Node, shim + `loadRstackConfig()`, Rstest/Rspack init) — hence + // the extended budget (the mocha timeout is 120s). + await waitFor( + () => { + const testController = currentRstestExports().testController; + assert.deepStrictEqual(toLabelTree(testController.items, true), [ + { + label: 'rstack', + children: [ + { + label: 'tests', + children: [{ label: 'basic.test.ts' }], + }, + ], + }, + { + label: 'workspace-1', + children: [{ label: 'test', children: WORKSPACE_1_FILES }], + }, + ]); + }, + { timeoutMs: 60_000 }, + ); + + // Test-case level (AST collection) inside the bridged project. + await waitFor(() => { + const testController = currentRstestExports().testController; + const file = getTestItemByLabels(testController.items, [ + 'rstack', + 'tests', + 'basic.test.ts', + ]); + assert.deepStrictEqual(toLabelTree(file.children), [ + { label: 'trims a string' }, + ]); + }); + }); + + test('runs bridged tests through the rstack config shim', async () => { + const collecting = createCollectingMockRun(); + + // Resolve the exports and the item together: holding an instance from + // before the poll would keep a controller a re-registration had replaced. + const { rstestInstance, item } = await waitFor(() => { + const rstestInstance = currentRstestExports(); + return { + rstestInstance, + item: getTestItemByLabels(rstestInstance.testController.items, [ + 'rstack', + 'tests', + 'basic.test.ts', + ]), + }; + }); + + rstestInstance.startTestRun( + new vscode.TestRunRequest([item], undefined, rstestInstance.runProfile), + new vscode.CancellationTokenSource().token, + false, + collecting.createMockRun, + ); + await collecting.ended; + + assert.equal(collecting.failedMessages.length, 0); + // A file requested as a whole reports twice: the case itself + // (`onTestCaseResult`) and the file item, which only goes green when the + // whole file passed (`onTestFileResult`). `progress.test.ts` never sees the + // second one — its file always has failures. + assert.deepStrictEqual( + collecting.passedItems.map((passed) => passed.label).sort(), + ['basic.test.ts', 'trims a string'], + ); + assert.match(collecting.output, /1 passed/); + }); +}); diff --git a/packages/vscode/e2e/rstest/suite/helpers.ts b/packages/vscode/e2e/rstest/suite/helpers.ts index f358c79..18cddbb 100644 --- a/packages/vscode/e2e/rstest/suite/helpers.ts +++ b/packages/vscode/e2e/rstest/suite/helpers.ts @@ -145,3 +145,56 @@ export function toLabelTree( nodes.sort((a, b) => (a.label < b.label ? -1 : a.label > b.label ? 1 : 0)); return nodes; } + +/** + * A collecting `vscode.TestRun` double for suites that only need "the run + * ended — what passed, what failed, what was printed". Repo-local, not + * ported: `progress.test.ts` keeps its own hand-rolled copy because it + * resets the captures per `createMockRun` call and counts invocations, + * which this deliberately does not do. + */ +export function createCollectingMockRun() { + const deferred = Promise.withResolvers(); + let output = ''; + const failedMessages: vscode.TestMessage[] = []; + const passedItems: vscode.TestItem[] = []; + const skippedItems: vscode.TestItem[] = []; + + const createMockRun = (): vscode.TestRun => ({ + isPersisted: true, + name: '', + token: new vscode.CancellationTokenSource().token, + onDidDispose: new vscode.EventEmitter().event, + addCoverage: () => {}, + appendOutput: (message) => { + output += message; + }, + end: () => { + deferred.resolve(null); + }, + enqueued: () => {}, + errored: () => {}, + failed: (_test, message = []) => { + failedMessages.push(...(message as vscode.TestMessage[])); + }, + passed: (test) => { + passedItems.push(test); + }, + skipped: (test) => { + skippedItems.push(test); + }, + started: () => {}, + }); + + return { + createMockRun, + /** Resolves when the run calls `end()`. */ + ended: deferred.promise, + get output() { + return output; + }, + failedMessages, + passedItems, + skippedItems, + }; +} diff --git a/packages/vscode/e2e/run.mjs b/packages/vscode/e2e/run.mjs new file mode 100644 index 0000000..53a7759 --- /dev/null +++ b/packages/vscode/e2e/run.mjs @@ -0,0 +1,102 @@ +// The single E2E entry point: `node ./e2e/run.mjs [slice ...]` (via +// `pnpm test:e2e [slice ...]`). No arguments runs every slice in table order. +// +// SLICES is the single source of truth for what E2E slices exist: which +// fixtures each needs installed and which script runs it. The package.json +// `test:e2e*` scripts are thin forwards to this file and carry none of that +// knowledge. The shared `tsc -p tsconfig.e2e.json` pass runs first (the +// cheap, likely-to-fail step), then the selected slices' fixtures install in +// one `setupFixtures.mjs` invocation (idempotent — pnpm no-ops on an +// up-to-date fixture, and unknown names throw there), then the entries run +// sequentially — each `compile`d entry launches its own VS Code via +// `@vscode/test-electron`. +import { spawnSync } from 'node:child_process'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +/** @type {{name: string, fixtures: string[], entry: string, compile?: boolean}[]} */ +const SLICES = [ + { + // A plain Node script — no VS Code, no TypeScript compile. + name: 'smoke', + fixtures: ['rslint'], + entry: 'e2e/smoke/rslintPluginHost.mjs', + }, + { + // The shell/detection/fmt suites (`e2e/suite/`) over the multi-root + // workspace of the three shared fixtures. + name: 'vscode', + fixtures: ['rslint', 'rstest', 'rstack'], + entry: 'tests-dist/e2e/runTest.js', + compile: true, + }, + { + // The ported Rstest suites, plus `suite/bridge.test.ts`, which adds the + // shared `rstack` fixture as a second workspace folder. + name: 'rstest', + fixtures: ['rstest-workspace-1', 'rstest-workspace-2', 'rstack'], + entry: 'tests-dist/e2e/rstest/runTest.js', + compile: true, + }, + { + // The ported Rslint suites; `RSTACK_LINT_E2E_SUITES=` filters + // which of them run. + name: 'lint', + fixtures: ['lint'], + entry: 'tests-dist/e2e/lint/runTest.js', + compile: true, + }, +]; + +const packageRoot = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + '..', +); + +/** + * @param {string} command + * @param {string[]} args + */ +const run = (command, args) => { + const result = spawnSync(command, args, { + cwd: packageRoot, + stdio: 'inherit', + env: process.env, + // On Windows, pnpm is a .cmd shim, and Node refuses to spawn batch files + // without a shell (CVE-2024-27980 hardening) — EINVAL otherwise. + shell: process.platform === 'win32', + }); + if (result.error) { + throw result.error; + } + if (result.status !== 0) { + process.exit(result.status ?? 1); + } +}; + +const known = SLICES.map((slice) => slice.name); +const requested = process.argv.slice(2); +for (const name of requested) { + if (!known.includes(name)) { + throw new Error(`unknown E2E slice: ${name} (known: ${known.join(', ')})`); + } +} +const selected = + requested.length > 0 + ? SLICES.filter((slice) => requested.includes(slice.name)) + : SLICES; + +if (selected.some((slice) => slice.compile)) { + console.log('[e2e] compiling slices (tsc -p tsconfig.e2e.json)'); + run('pnpm', ['exec', 'tsc', '-p', 'tsconfig.e2e.json']); +} + +run(process.execPath, [ + 'e2e/setupFixtures.mjs', + ...new Set(selected.flatMap((slice) => slice.fixtures)), +]); + +for (const slice of selected) { + console.log(`[e2e] running slice: ${slice.name}`); + run(process.execPath, [path.join(packageRoot, slice.entry)]); +} diff --git a/packages/vscode/e2e/setupFixtures.mjs b/packages/vscode/e2e/setupFixtures.mjs index 3639ed3..caef3a4 100644 --- a/packages/vscode/e2e/setupFixtures.mjs +++ b/packages/vscode/e2e/setupFixtures.mjs @@ -22,6 +22,8 @@ export const FIXTURES_DIR = path.join(here, 'fixtures'); * `lint` entry is the shared install root serving every ported Rslint suite * workspace (`e2e/lint/fixtures/*` — the workspaces themselves have no * package.json; @rslint/core resolves via Node's walk-up from one install). + * A fixture that a test-worker-spawning slice opens as a workspace folder + * carries a `.nvmrc` pin — rationale in the `e2e/rstest/runTest.ts` header. */ export const FIXTURES = { rslint: path.join(FIXTURES_DIR, 'rslint'), @@ -53,8 +55,9 @@ const install = (name) => { // the moment a patch release lands. '--no-frozen-lockfile', '--prefer-offline', - // Changing a fixture's `.npmrc` makes pnpm want to purge `node_modules`, - // which it refuses to do without a TTY. The directory is disposable. + // Changing a fixture's install config (its hoist patterns, say) makes + // pnpm want to purge `node_modules`, which it refuses to do without a + // TTY. The directory is disposable. '--config.confirmModulesPurge=false', // Fixtures deliberately install pinned published versions of the Rstack // toolchain, which are often hours old — disable pnpm's @@ -68,6 +71,20 @@ const install = (name) => { // published packages exactly like a user project would, so run their // build scripts as-is. '--config.dangerouslyAllowAllBuilds=true', + // `@rslint/core` / `@rstest/core` may reach a fixture only as transitive + // dependencies of `rstack` (the rstack fixture depends on `rstack` + // alone), yet the extension resolves them with a node_modules walk-up + // from the project dir — which pnpm's isolated store defeats: the + // walk-up would climb out of the fixture and silently find THIS REPO's + // dev copies instead of the published ones. Public-hoisting the two + // reproduces the npm/Yarn layout the extension is designed against, and + // is inert for fixtures that already depend on them directly. It must + // be a CLI flag: pnpm 11 no longer reads `public-hoist-pattern` from a + // fixture-local `.npmrc` (verified — it lands as an empty + // `publicHoistPattern` in `.modules.yaml`), and `--ignore-workspace` + // also ignores a local pnpm-workspace.yaml. + '--config.publicHoistPattern=@rslint/core', + '--config.publicHoistPattern=@rstest/core', ], { cwd, diff --git a/packages/vscode/package.json b/packages/vscode/package.json index 401b050..fa63eb3 100644 --- a/packages/vscode/package.json +++ b/packages/vscode/package.json @@ -29,14 +29,12 @@ "package": "pnpm run build && vsce package", "package:targets": "node scripts/packageTargets.mjs", "test": "pnpm run test:unit && pnpm run test:e2e", - "test:e2e": "pnpm run test:e2e:fixtures && pnpm run test:e2e:smoke && pnpm run test:e2e:vscode && pnpm run test:e2e:rstest:run && pnpm run test:e2e:lint:run", + "test:e2e": "node ./e2e/run.mjs", "test:e2e:fixtures": "node ./e2e/setupFixtures.mjs", - "test:e2e:lint": "pnpm run test:e2e:fixtures lint && pnpm run test:e2e:lint:run", - "test:e2e:lint:run": "tsc -p tsconfig.e2e.json && node ./tests-dist/e2e/lint/runTest.js", - "test:e2e:rstest": "pnpm run test:e2e:fixtures rstest-workspace-1 rstest-workspace-2 && pnpm run test:e2e:rstest:run", - "test:e2e:rstest:run": "tsc -p tsconfig.e2e.json && node ./tests-dist/e2e/rstest/runTest.js", - "test:e2e:smoke": "node ./e2e/smoke/rslintPluginHost.mjs", - "test:e2e:vscode": "tsc -p tsconfig.e2e.json && node ./tests-dist/e2e/runTest.js", + "test:e2e:lint": "node ./e2e/run.mjs lint", + "test:e2e:rstest": "node ./e2e/run.mjs rstest", + "test:e2e:smoke": "node ./e2e/run.mjs smoke", + "test:e2e:vscode": "node ./e2e/run.mjs vscode", "test:unit": "rstest", "watch": "rslib build --watch", "watch:local": "cross-env SOURCEMAP=true rslib build --watch" From d3d778ec20bcc7366d0623ca6d7c11b1b9020b2c Mon Sep 17 00:00:00 2001 From: fi3ework Date: Tue, 11 Aug 2026 17:42:08 +0800 Subject: [PATCH 2/2] fix(vscode): confine the E2E runner shell to the pnpm spawn With shell: true Node concatenates command and args unescaped, so spawning process.execPath or an entry path containing spaces through the shared option would break argument boundaries on Windows. Only the pnpm .cmd shim needs a shell (CVE-2024-27980 hardening); node spawns now run shell-less. Raised by PR review. --- packages/vscode/e2e/run.mjs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/vscode/e2e/run.mjs b/packages/vscode/e2e/run.mjs index 53a7759..f86d0d5 100644 --- a/packages/vscode/e2e/run.mjs +++ b/packages/vscode/e2e/run.mjs @@ -56,15 +56,18 @@ const packageRoot = path.resolve( /** * @param {string} command * @param {string[]} args + * @param {{shell?: boolean}} [opts] */ -const run = (command, args) => { +const run = (command, args, opts = {}) => { const result = spawnSync(command, args, { cwd: packageRoot, stdio: 'inherit', env: process.env, - // On Windows, pnpm is a .cmd shim, and Node refuses to spawn batch files - // without a shell (CVE-2024-27980 hardening) — EINVAL otherwise. - shell: process.platform === 'win32', + // With `shell: true` Node concatenates command and args UNESCAPED, so a + // path containing spaces (the checkout, `process.execPath`) would fall + // apart into several arguments — callers opt in only where the command + // cannot spawn without a shell and every argument is a fixed safe token. + shell: opts.shell ?? false, }); if (result.error) { throw result.error; @@ -88,7 +91,12 @@ const selected = if (selected.some((slice) => slice.compile)) { console.log('[e2e] compiling slices (tsc -p tsconfig.e2e.json)'); - run('pnpm', ['exec', 'tsc', '-p', 'tsconfig.e2e.json']); + // On Windows, pnpm is a .cmd shim, and Node refuses to spawn batch files + // without a shell (CVE-2024-27980 hardening) — EINVAL otherwise. The shell + // stays confined to this spawn: its arguments are fixed safe tokens. + run('pnpm', ['exec', 'tsc', '-p', 'tsconfig.e2e.json'], { + shell: process.platform === 'win32', + }); } run(process.execPath, [