* fix(#371): invoke installed bin via shell for Windows .cmd shims in release-tarball-smoke `node <gsd-tools.cmd>` cannot execute a Windows batch shim as a JS script, so runSmoke returned bin_not_callable for every check on Windows. Route .cmd/.bat shims through shell:true (required by Node >=18.20/20.12) and keep the POSIX node-invocation path unchanged. Add an exported binInvocation seam plus a platform-agnostic regression test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(#371): surface bin-invocation failure details in release-tarball-smoke The smoke harness captured stderr/stdout in `details` but never printed them, so Windows bin_not_callable failures gave no actionable cause in CI. Log the resolved bin, invocation descriptor, exit status/signal/error, and captured stderr/stdout on spawn-derived failures so the real error is visible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(#371): include smoke result details in install-test assertion messages The node --test TAP runner swallows in-test console.error, so Windows bin_not_callable failures gave no cause. Embed code + details (incl. captured stderr/stdout) into the assertion messages, which DO reach the CI log, so the real Windows failure is diagnosable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#371): resolve installed bin from prefix root on Windows npm install -g --prefix X writes bin shims to X\ (the prefix root) on Windows, not X\node_modules\.bin\. The smoke harness only searched node_modules\.bin on win32, so the installed gsd-tools/installer bin was never found and runSmoke returned bin_not_callable before invoking anything. Search the prefix root first (then node_modules/.bin as fallback), report the searched candidates on miss, and drop the TAP-swallowed console.error probes. The .cmd-via-shell binInvocation fix remains for actually running the shim. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -65,6 +65,44 @@ const SMOKE = Object.freeze({
|
||||
WORKFLOW_BODY_COLON_LEAK: 'workflow_body_colon_leak',
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Exported helper: binInvocation
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Build the { command, args, shell } descriptor needed to spawn an installed
|
||||
* npm bin correctly on both Windows and POSIX.
|
||||
*
|
||||
* On Windows, npm installs a `.cmd` (or `.bat`) shim in .bin/. Node ≥18.20.2
|
||||
* / ≥20.12.2 throws EINVAL when you try to spawnSync a .cmd/.bat without
|
||||
* shell:true (CVE-2024-27980 mitigation). With shell:true, Node does NOT
|
||||
* auto-quote argv, so a bin path that contains spaces must be wrapped in
|
||||
* double-quotes to arrive at the shell as one token.
|
||||
*
|
||||
* On POSIX the bin is a regular shebang JS file; we invoke it directly via
|
||||
* process.execPath (the same Node binary) without a shell.
|
||||
*
|
||||
* @param {string} binPath - Absolute path to the resolved bin file.
|
||||
* @param {string[]} [args] - Additional arguments (e.g. ['--help']).
|
||||
* @returns {{ command: string, args: string[], shell: boolean }}
|
||||
*/
|
||||
function binInvocation(binPath, args = []) {
|
||||
const lower = binPath.toLowerCase();
|
||||
// Note: .ps1 shims are intentionally NOT handled here. The bin-resolution
|
||||
// helpers (findGsdToolsBin / findInstallerBin) only ever surface a .cmd path
|
||||
// on Windows — npm does not write .ps1 shims into .bin/ by default — so a
|
||||
// .ps1 path never reaches this function in practice.
|
||||
if (lower.endsWith('.cmd') || lower.endsWith('.bat')) {
|
||||
// Quote the path if it contains a space so the Windows shell treats it as
|
||||
// a single token. Simple double-quote wrap is sufficient because npm-
|
||||
// generated shim paths don't contain embedded double-quotes.
|
||||
const command = binPath.includes(' ') ? `"${binPath}"` : binPath;
|
||||
return { command, args: [...args], shell: true };
|
||||
}
|
||||
// POSIX: invoke via node, no shell needed.
|
||||
return { command: process.execPath, args: [binPath, ...args], shell: false };
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Internal helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -81,19 +119,36 @@ function pkgRoot(installPrefix) {
|
||||
return fs.existsSync(posix) ? posix : win;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the ordered list of candidate paths to check when locating an npm
|
||||
* global bin named `name` under `installPrefix`.
|
||||
*
|
||||
* On Windows, `npm install -g --prefix X` writes shims (*.cmd, *.ps1, bare)
|
||||
* to the PREFIX ROOT (X\), NOT to X\node_modules\.bin\. We therefore probe
|
||||
* the prefix root first, then fall back to node_modules\.bin in case a
|
||||
* non-standard layout puts them there.
|
||||
*
|
||||
* On POSIX the shim lands in <prefix>/bin/ as a symlink; only one candidate.
|
||||
*/
|
||||
function binCandidates(installPrefix, name) {
|
||||
if (process.platform === 'win32') {
|
||||
return [
|
||||
// npm global --prefix on Windows writes shims to the prefix ROOT
|
||||
path.join(installPrefix, `${name}.cmd`),
|
||||
path.join(installPrefix, name),
|
||||
// fallback: some layouts use node_modules/.bin
|
||||
path.join(installPrefix, 'node_modules', '.bin', `${name}.cmd`),
|
||||
path.join(installPrefix, 'node_modules', '.bin', name),
|
||||
];
|
||||
}
|
||||
return [path.join(installPrefix, 'bin', name)];
|
||||
}
|
||||
|
||||
/**
|
||||
* Locate the installed gsd-tools binary (symlink in <prefix>/bin/).
|
||||
*/
|
||||
function findGsdToolsBin(installPrefix) {
|
||||
const binDir = process.platform === 'win32'
|
||||
? path.join(installPrefix, 'node_modules', '.bin')
|
||||
: path.join(installPrefix, 'bin');
|
||||
|
||||
const candidates = process.platform === 'win32'
|
||||
? [path.join(binDir, 'gsd-tools.cmd'), path.join(binDir, 'gsd-tools')]
|
||||
: [path.join(binDir, 'gsd-tools')];
|
||||
|
||||
for (const c of candidates) {
|
||||
for (const c of binCandidates(installPrefix, 'gsd-tools')) {
|
||||
if (fs.existsSync(c)) return c;
|
||||
}
|
||||
return null;
|
||||
@@ -103,15 +158,7 @@ function findGsdToolsBin(installPrefix) {
|
||||
* Locate the get-shit-done-redux installer binary (the symlink in <prefix>/bin/).
|
||||
*/
|
||||
function findInstallerBin(installPrefix) {
|
||||
const binDir = process.platform === 'win32'
|
||||
? path.join(installPrefix, 'node_modules', '.bin')
|
||||
: path.join(installPrefix, 'bin');
|
||||
|
||||
const candidates = process.platform === 'win32'
|
||||
? [path.join(binDir, 'get-shit-done-redux.cmd'), path.join(binDir, 'get-shit-done-redux')]
|
||||
: [path.join(binDir, 'get-shit-done-redux')];
|
||||
|
||||
for (const c of candidates) {
|
||||
for (const c of binCandidates(installPrefix, 'get-shit-done-redux')) {
|
||||
if (fs.existsSync(c)) return c;
|
||||
}
|
||||
return null;
|
||||
@@ -266,22 +313,21 @@ function runSmoke({
|
||||
const actualBin = findGsdToolsBin(installPrefix);
|
||||
|
||||
if (!actualBin) {
|
||||
const binDir = process.platform === 'win32'
|
||||
? path.join(installPrefix, 'node_modules', '.bin')
|
||||
: path.join(installPrefix, 'bin');
|
||||
const searched = binCandidates(installPrefix, 'gsd-tools');
|
||||
return {
|
||||
code: SMOKE.BIN_NOT_CALLABLE,
|
||||
details: { ...details, binDir, searched: [] },
|
||||
details: { ...details, searched },
|
||||
};
|
||||
}
|
||||
|
||||
// --- Invoke `gsd-tools --help` to assert the shipped binary is callable ---
|
||||
// Use effectiveNpmEnv so the installed binary sees an isolated HOME on Docker
|
||||
// hosts where HOME may be unwritable (same isolation as the npm install). (#131)
|
||||
const versionInvocation = binInvocation(actualBin, ['--help']);
|
||||
const versionResult = spawnSync(
|
||||
process.execPath,
|
||||
[actualBin, '--help'],
|
||||
{ encoding: 'utf-8', timeout: CHILD_TIMEOUT_MS, env: effectiveNpmEnv },
|
||||
versionInvocation.command,
|
||||
versionInvocation.args,
|
||||
{ encoding: 'utf-8', timeout: CHILD_TIMEOUT_MS, env: effectiveNpmEnv, shell: versionInvocation.shell },
|
||||
);
|
||||
|
||||
if (versionResult.status !== 0) {
|
||||
@@ -341,9 +387,10 @@ function runSmoke({
|
||||
const initEnv = { ...process.env };
|
||||
delete initEnv.GSD_TEST_MODE;
|
||||
|
||||
const initInvocation = binInvocation(installerBin, ['--local', '--claude']);
|
||||
const initResult = spawnSync(
|
||||
process.execPath,
|
||||
[installerBin, '--local', '--claude'],
|
||||
initInvocation.command,
|
||||
initInvocation.args,
|
||||
{
|
||||
encoding: 'utf-8',
|
||||
cwd: fixtureDir,
|
||||
@@ -351,6 +398,7 @@ function runSmoke({
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
env: initEnv,
|
||||
timeout: CHILD_TIMEOUT_MS,
|
||||
shell: initInvocation.shell,
|
||||
},
|
||||
);
|
||||
|
||||
@@ -556,7 +604,7 @@ function cleanup(...dirs) {
|
||||
// Exports
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
module.exports = { SMOKE, runSmoke };
|
||||
module.exports = { SMOKE, runSmoke, binInvocation };
|
||||
|
||||
if (require.main === module) {
|
||||
cliMain();
|
||||
|
||||
57
tests/release-tarball-smoke-bin-invocation.test.cjs
Normal file
57
tests/release-tarball-smoke-bin-invocation.test.cjs
Normal file
@@ -0,0 +1,57 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Unit tests for the binInvocation helper in scripts/release-tarball-smoke.cjs.
|
||||
*
|
||||
* Asserts ONLY on the returned descriptor — no process spawning.
|
||||
*/
|
||||
|
||||
const { describe, it } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
|
||||
const { binInvocation } = require('../scripts/release-tarball-smoke.cjs');
|
||||
|
||||
describe('binInvocation', () => {
|
||||
it('returns shell:true and command=binPath (not execPath) for a .cmd path', () => {
|
||||
const bin = 'C:\\prefix\\node_modules\\.bin\\gsd-tools.cmd';
|
||||
const result = binInvocation(bin, ['--help']);
|
||||
|
||||
assert.strictEqual(result.command, bin,
|
||||
'command must be the .cmd path itself, not process.execPath');
|
||||
assert.strictEqual(result.shell, true,
|
||||
'shell must be true for .cmd shims (Node CVE-2024-27980 mitigation)');
|
||||
// The bin path must NOT appear in args as if it were a node script argument
|
||||
assert.ok(
|
||||
!result.args.includes(bin),
|
||||
'the .cmd path must not be pushed into args as a node-script positional',
|
||||
);
|
||||
});
|
||||
|
||||
it('returns shell:true and command=binPath for a .bat path', () => {
|
||||
const bin = 'C:\\prefix\\node_modules\\.bin\\gsd-tools.bat';
|
||||
const result = binInvocation(bin, ['--help']);
|
||||
|
||||
assert.strictEqual(result.shell, true, 'shell must be true for .bat shims');
|
||||
assert.strictEqual(result.command, bin, 'command must be the .bat path');
|
||||
});
|
||||
|
||||
it('returns command===process.execPath and args[0]===binPath and shell falsy for a POSIX path', () => {
|
||||
const bin = '/tmp/prefix/bin/gsd-tools';
|
||||
const result = binInvocation(bin, ['--help']);
|
||||
|
||||
assert.strictEqual(result.command, process.execPath,
|
||||
'POSIX bin must be invoked via node (process.execPath)');
|
||||
assert.strictEqual(result.args[0], bin,
|
||||
'POSIX bin path must be args[0] (the node script argument)');
|
||||
assert.ok(!result.shell,
|
||||
'shell must be falsy for POSIX shebang bins');
|
||||
});
|
||||
|
||||
it('quotes a .cmd path containing a space so the shell receives one token', () => {
|
||||
const bin = 'C:\\Users\\a b\\node_modules\\.bin\\gsd-tools.cmd';
|
||||
const result = binInvocation(bin, ['--help']);
|
||||
|
||||
assert.strictEqual(result.shell, true, 'shell must be true');
|
||||
assert.strictEqual(result.command, `"${bin}"`, 'spaced .cmd path must be wrapped in double-quotes as a single shell token');
|
||||
});
|
||||
});
|
||||
@@ -15,6 +15,9 @@ const path = require('node:path');
|
||||
const { cleanup, createTempDir, runNpm, isolatedNpmEnv } = require('./helpers.cjs');
|
||||
const { SMOKE, runSmoke } = require('../scripts/release-tarball-smoke.cjs');
|
||||
|
||||
const smokeMsg = (label, result) =>
|
||||
`${label}: code=${result.code} details=${JSON.stringify(result.details)}`;
|
||||
|
||||
const PKG_PATH = path.join(__dirname, '..', 'package.json');
|
||||
const pkg = JSON.parse(fs.readFileSync(PKG_PATH, 'utf-8'));
|
||||
|
||||
@@ -74,8 +77,8 @@ describe('release-tarball-smoke', () => {
|
||||
npmEnv: isolatedNpmEnv(),
|
||||
});
|
||||
|
||||
assert.equal(result.code, SMOKE.OK);
|
||||
assert.equal(result.details.version, pkg.version);
|
||||
assert.equal(result.code, SMOKE.OK, smokeMsg('A', result));
|
||||
assert.equal(result.details.version, pkg.version, smokeMsg('A', result));
|
||||
});
|
||||
|
||||
// ── Test B — version mismatch detected ────────────────────────────────────
|
||||
@@ -88,7 +91,7 @@ describe('release-tarball-smoke', () => {
|
||||
npmEnv: isolatedNpmEnv(),
|
||||
});
|
||||
|
||||
assert.equal(result.code, SMOKE.VERSION_MISMATCH);
|
||||
assert.equal(result.code, SMOKE.VERSION_MISMATCH, smokeMsg('B', result));
|
||||
});
|
||||
|
||||
// ── Test C — happy lifecycle ───────────────────────────────────────────────
|
||||
@@ -106,7 +109,7 @@ describe('release-tarball-smoke', () => {
|
||||
npmEnv: isolatedNpmEnv(),
|
||||
});
|
||||
|
||||
assert.equal(result.code, SMOKE.OK);
|
||||
assert.equal(result.code, SMOKE.OK, smokeMsg('C', result));
|
||||
|
||||
// Each non-init command must be in lifecycleResolved with both paths populated
|
||||
const resolved = result.details.lifecycleResolved;
|
||||
@@ -145,9 +148,9 @@ describe('release-tarball-smoke', () => {
|
||||
npmEnv: isolatedNpmEnv(),
|
||||
});
|
||||
|
||||
assert.equal(result.code, SMOKE.COMMAND_FILE_MISSING);
|
||||
assert.equal(result.details.command, 'nonexistent-phase-xyz');
|
||||
assert.ok(typeof result.details.path === 'string' && result.details.path.length > 0);
|
||||
assert.equal(result.code, SMOKE.COMMAND_FILE_MISSING, smokeMsg('D', result));
|
||||
assert.equal(result.details.command, 'nonexistent-phase-xyz', smokeMsg('D', result));
|
||||
assert.ok(typeof result.details.path === 'string' && result.details.path.length > 0, smokeMsg('D', result));
|
||||
});
|
||||
|
||||
// ── Test E — workflow-body checks run (informational) ─────────────────────
|
||||
@@ -167,11 +170,11 @@ describe('release-tarball-smoke', () => {
|
||||
// Structural: the scan ran and populated the counters
|
||||
assert.ok(
|
||||
Number.isInteger(result.details.workflowsScanned) && result.details.workflowsScanned >= 1,
|
||||
`expected workflowsScanned >= 1, got ${result.details.workflowsScanned}`,
|
||||
smokeMsg('E', result),
|
||||
);
|
||||
assert.ok(
|
||||
Number.isInteger(result.details.colonLeakCount),
|
||||
`expected colonLeakCount to be an integer, got ${result.details.colonLeakCount}`,
|
||||
smokeMsg('E', result),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user