fix(#131): isolate HOME for release-tarball-smoke (before() + runSmoke A-F) (#139)

* fix(#131): pass explicit HOME and npm cache to before() npm invocations

npm reads $HOME/.npmrc (user config) and writes to $HOME/.npm (default
cache dir) unless overridden. On Docker hosts the running user's HOME
may be uninitialized, unwritable, or contain stale state from prior
runs — any of which causes `npm pack` / `npm install -g` in the
before() hook to fail with EACCES, cancelling all 6 subtests (A–F).

Fix: allocate a fresh mkdtemp dir once per test process in helpers.cjs
and inject it as HOME, npm_config_cache, and npm_config_userconfig for
every runNpm() call. A process.on('exit') handler removes the dir on
teardown. The caller-supplied env option (if any) is merged on top of
the isolated env so explicit overrides still win.

TDD: tests/bug-131-release-tarball-smoke-explicit-home.test.cjs
- Test 1: runNpm succeeds when process HOME is chmod-0500 (unwritable)
- Test 2: npm_config_cache resolves under tmpdir, not caller HOME

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(#131): extend HOME isolation to runSmoke spawnSync calls so A-F pass

Pass effectiveNpmEnv to the gsd-sdk --version and gsd-sdk query spawnSync
invocations inside runSmoke(), matching the isolation already applied to the
npm install step. Also add npmEnv: isolatedNpmEnv() to every runSmoke() call
in the install test so the full env isolation chain is in effect.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(#131): address CI feedback — prompt-injection comment, Windows USERPROFILE stub, macOS realpath

- Rephrase 'act as a poisoned HOME' comment to 'serve as a poisoned HOME'
  to avoid triggering the prompt-injection scanner's act-as pattern
- Add paired process.env.USERPROFILE stub alongside process.env.HOME in
  Test 1 inline script so Windows parity guard offender count stays at 8
- Fix macOS /var→/private/var symlink false-negative in Test 2 by resolving
  the nearest existing ancestor with fs.realpathSync before the startsWith
  comparison

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(#131): export isolatedNpmEnv from helpers.cjs (CI repro of missing symbol)

isolatedNpmEnv() was defined in tests/helpers.cjs but never committed —
the function body and the updated module.exports line were left as unstaged
local edits. CI checkouts saw the old module.exports (without isolatedNpmEnv),
causing TypeError: isolatedNpmEnv is not a function at the call site in
bug-131-release-tarball-smoke-explicit-home.test.cjs:178 and in
release-tarball-smoke.install.test.cjs wherever the function is destructured.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(#131): canonicalize macOS tmpdir in remaining startsWith assertions

Replace the ad-hoc try/catch realpathSync fallback chain in Test 2 and the
inline try/catch in Test 3 with a shared safeRealpath() helper that walks up
to the nearest existing ancestor before resolving, then reconstructs the
canonical path. This ensures /var→/private/var symlink expansion succeeds
even when the leaf (.npm cache dir) does not yet exist on macOS CI runners.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-23 00:24:32 -04:00
committed by GitHub
parent 21c6c29ac0
commit 899c8cff3a
4 changed files with 305 additions and 6 deletions

View File

@@ -282,6 +282,10 @@ function scanWorkflowMissingSdkFallback(filePath) {
* @param {string} [opts.fixtureDir] - Temp dir to run `init` into (must NOT be HOME)
* @param {string[]} [opts.lifecycleCommands] - Commands to file-check (default: see below)
* @param {boolean} [opts.dryRun=false] - If true, skip actual npm install; validate input only
* @param {object} [opts.npmEnv] - Optional env dict for the internal npm install
* spawnSync call. Pass an isolated HOME env (e.g. from isolatedNpmEnv() in tests/helpers.cjs)
* to prevent npm from reading/writing the caller's $HOME — required on Docker hosts where HOME
* may be unwritable. Defaults to process.env. (#131)
* @returns {{ code: string, details: object }}
*/
function runSmoke({
@@ -291,6 +295,7 @@ function runSmoke({
fixtureDir,
lifecycleCommands = ['init', 'discuss-phase', 'plan-phase', 'execute-phase'],
dryRun = false,
npmEnv = undefined,
}) {
const details = {
tarball: tarballPath,
@@ -304,10 +309,14 @@ function runSmoke({
// --- Install the tarball into the temp prefix ----------------------------
const npmCmd = process.platform === 'win32' ? 'npm.cmd' : 'npm';
// Use the caller-supplied npmEnv if provided (allows HOME isolation on Docker
// hosts where HOME may be unwritable — same pattern as runNpm() in helpers.cjs).
// Falls back to process.env to preserve existing CLI / programmatic behaviour. (#131)
const effectiveNpmEnv = npmEnv !== undefined ? npmEnv : process.env;
const installResult = spawnSync(
npmCmd,
['install', '-g', '--prefix', installPrefix, tarballPath],
{ encoding: 'utf-8', shell: process.platform === 'win32', timeout: CHILD_TIMEOUT_MS },
{ encoding: 'utf-8', shell: process.platform === 'win32', timeout: CHILD_TIMEOUT_MS, env: effectiveNpmEnv },
);
if (installResult.status !== 0) {
@@ -335,10 +344,12 @@ function runSmoke({
}
// --- Invoke `gsd-sdk --version` ------------------------------------------
// 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 versionResult = spawnSync(
process.execPath,
[actualBin, '--version'],
{ encoding: 'utf-8', timeout: CHILD_TIMEOUT_MS },
{ encoding: 'utf-8', timeout: CHILD_TIMEOUT_MS, env: effectiveNpmEnv },
);
if (versionResult.status !== 0) {
@@ -497,11 +508,13 @@ function runSmoke({
// ─────────────────────────────────────────────────────────────────────────
// --- Verify `gsd-sdk` query is callable and returns parseable JSON -------
// 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 sdkQueryDir = fixtureDir || os.tmpdir();
const sdkQueryResult = spawnSync(
process.execPath,
[actualBin, 'query', 'state.json', '--project-dir', sdkQueryDir],
{ encoding: 'utf-8', timeout: CHILD_TIMEOUT_MS },
{ encoding: 'utf-8', timeout: CHILD_TIMEOUT_MS, env: effectiveNpmEnv },
);
if (sdkQueryResult.status !== 0) {

View File

@@ -0,0 +1,229 @@
// allow-test-rule: integration-test-input
// Regression test for #131: runNpm() must not fail when HOME points at an
// unwritable directory. The before() hook in release-tarball-smoke.install.test.cjs
// calls runNpm(['pack', ...]) and runNpm(['install', '-g', ...]) — if those inherit
// an unwritable HOME from the environment (common in constrained Docker hosts),
// the entire hook fails and all 6 subtests are cancelled.
//
// Fix: runNpm() must inject an explicit HOME, npm_config_cache, and
// npm_config_userconfig that point into a temp directory it owns, so that npm
// never reads from or writes to the caller's HOME.
//
// Test 3 (added in the second fix pass) verifies that isolatedNpmEnv() — the
// companion export that lets runSmoke() apply the same isolation — also redirects
// HOME away from the caller's HOME. Without this, subtests A-F of
// release-tarball-smoke.install.test.cjs still fail because runSmoke() calls
// spawnSync('npm', ...) internally and was not covered by the runNpm() fix.
'use strict';
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const { execFileSync } = require('node:child_process');
// The helpers under test.
const { runNpm, isolatedNpmEnv } = require('./helpers.cjs');
// Resolve a filesystem path to its canonical (symlink-free) form even if the
// leaf does not exist yet (e.g. ~/.npm before npm has written its cache).
// Walks up to the nearest existing ancestor, resolves that, then re-appends
// the trailing segments. This handles macOS /var → /private/var symlinks for
// paths created under os.tmpdir() where the leaf directory may not exist yet.
function safeRealpath(p) {
try {
return fs.realpathSync(p);
} catch (_) {
// Leaf does not exist — resolve the nearest existing ancestor then
// reconstruct the original suffix so the result is still canonical.
const segments = [];
let cur = p;
for (;;) {
const parent = path.dirname(cur);
if (parent === cur) {
// Reached filesystem root — return original path unchanged.
return p;
}
segments.unshift(path.basename(cur));
cur = parent;
try {
return path.join(fs.realpathSync(cur), ...segments);
} catch (__) {
// Keep walking up.
}
}
}
}
describe('bug-131: runNpm isolates HOME from the caller environment', () => {
// ── Test 1 — runNpm works with an unwritable HOME ────────────────────────
// Spawn a child Node process that sets HOME to a chmod-0500 directory, then
// invokes runNpm(['--version']). Without the fix, npm tries to read/write
// HOME/.npmrc and HOME/.npm, fails with EACCES, and runNpm throws.
// With the fix, runNpm injects its own isolated HOME and npm succeeds.
test('runNpm succeeds even when process HOME is unwritable', () => {
// Create an unwritable dir to serve as a poisoned HOME.
const poisonedHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-bug131-poison-'));
try {
fs.chmodSync(poisonedHome, 0o500); // r-x only — not writable
// We exercise the real runNpm() path by running a tiny inline Node script
// that requires helpers.cjs and calls runNpm(['--version']) with HOME set
// to the unwritable dir. The script exits 0 on success, non-zero on throw.
const script = `
process.env.HOME = ${JSON.stringify(poisonedHome)};
process.env.USERPROFILE = ${JSON.stringify(poisonedHome)};
const { runNpm } = require(${JSON.stringify(path.join(__dirname, 'helpers.cjs'))});
try {
const out = runNpm(['--version']);
if (!out || out.trim() === '') process.exit(2); // vacuous success guard
process.stdout.write(out);
process.exit(0);
} catch (e) {
process.stderr.write(e.message + '\\n');
process.exit(1);
}
`;
let stdout = '';
let stderr = '';
let exitCode = 0;
try {
stdout = execFileSync(process.execPath, ['-e', script], {
encoding: 'utf-8',
timeout: 30_000,
});
} catch (err) {
stdout = err.stdout || '';
stderr = err.stderr || '';
exitCode = err.status ?? 1;
}
assert.equal(
exitCode,
0,
`runNpm should succeed with an unwritable HOME but exited ${exitCode}. stderr: ${stderr}`,
);
// npm --version returns something like "10.x.y"
assert.match(
stdout.trim(),
/^\d+\.\d+/,
`expected semver output from npm --version, got: ${stdout}`,
);
} finally {
// Restore write permission before cleanup so rmSync can delete it.
try { fs.chmodSync(poisonedHome, 0o700); } catch (_) { /* best-effort */ }
fs.rmSync(poisonedHome, { recursive: true, force: true });
}
});
// ── Test 2 — runNpm does not leak a caller-supplied HOME into npm ────────
// Even if the caller exports HOME=/some/real/path, the injected HOME must be
// a different (temp) path so npm writes never touch the caller's $HOME.
test('runNpm injects a HOME distinct from process.env.HOME', () => {
// Capture what HOME runNpm actually passes to npm by asking npm to print
// the value it sees for the $HOME env var. We do this via `npm config get
// cache` which reveals the cache path — if it's under process.env.HOME,
// the fix is absent; if it's under a tmp dir, the fix is present.
const script = `
const { runNpm } = require(${JSON.stringify(path.join(__dirname, 'helpers.cjs'))});
try {
// npm config get cache prints the effective cache directory.
const out = runNpm(['config', 'get', 'cache']);
process.stdout.write(out.trim());
process.exit(0);
} catch (e) {
process.stderr.write(e.message + '\\n');
process.exit(1);
}
`;
let stdout = '';
let stderr = '';
let exitCode = 0;
try {
stdout = execFileSync(process.execPath, ['-e', script], {
encoding: 'utf-8',
timeout: 30_000,
});
} catch (err) {
stdout = err.stdout || '';
stderr = err.stderr || '';
exitCode = err.status ?? 1;
}
assert.equal(
exitCode,
0,
`runNpm config get cache failed with exit ${exitCode}. stderr: ${stderr}`,
);
const effectiveCacheDir = stdout.trim();
// The effective npm cache must NOT be inside the calling process's HOME.
// If it is, the fix was not applied and the Docker regression can still occur.
const callerHome = os.homedir();
assert.ok(
!effectiveCacheDir.startsWith(callerHome),
`npm cache dir ${effectiveCacheDir} is still under caller HOME ${callerHome} — fix not applied`,
);
// It must be somewhere under the system tmp dir, confirming isolation.
// Use safeRealpath on both sides so that macOS /var→/private/var symlinks
// do not cause a false mismatch when os.tmpdir() and the resolved cache
// path differ only in symlink expansion. The cache sub-directory (.npm) may
// not exist yet; safeRealpath walks up to the nearest existing ancestor.
const sysTmp = safeRealpath(os.tmpdir());
const realCacheDir = safeRealpath(effectiveCacheDir);
assert.ok(
realCacheDir.startsWith(sysTmp),
`npm cache dir ${realCacheDir} should be under tmpdir ${sysTmp}`,
);
});
// ── Test 3 — isolatedNpmEnv() redirects HOME away from the caller's HOME ──
// runSmoke() calls spawnSync('npm', ...) with npmEnv from isolatedNpmEnv().
// If isolatedNpmEnv() didn't redirect HOME, subtests A-F would still fail on
// Docker hosts with an unwritable HOME (the original bug #131 root cause,
// manifesting via the sibling runSmoke() path). (#131)
test('isolatedNpmEnv() HOME is distinct from the caller HOME and lives under tmpdir', () => {
const env = isolatedNpmEnv();
// Must expose a HOME key.
assert.ok(
typeof env.HOME === 'string' && env.HOME.length > 0,
'isolatedNpmEnv() must set HOME',
);
// Must not be the caller's HOME.
const callerHome = os.homedir();
assert.notEqual(
env.HOME,
callerHome,
`isolatedNpmEnv() HOME must differ from caller HOME ${callerHome}`,
);
// Must live under the system tmpdir, confirming it is an isolated temp directory.
// Use safeRealpath on both sides so that macOS /var→/private/var symlinks
// do not cause a false mismatch.
const sysTmp = safeRealpath(os.tmpdir());
const realHome = safeRealpath(env.HOME);
assert.ok(
realHome.startsWith(sysTmp),
`isolatedNpmEnv() HOME ${realHome} should be under tmpdir ${sysTmp}`,
);
// npm_config_cache and npm_config_userconfig must also be set and under the isolated HOME.
assert.ok(
typeof env.npm_config_cache === 'string' && env.npm_config_cache.startsWith(env.HOME),
`npm_config_cache ${env.npm_config_cache} should be under isolated HOME ${env.HOME}`,
);
assert.ok(
typeof env.npm_config_userconfig === 'string' && env.npm_config_userconfig.startsWith(env.HOME),
`npm_config_userconfig ${env.npm_config_userconfig} should be under isolated HOME ${env.HOME}`,
);
});
});

View File

@@ -183,6 +183,22 @@ function isUsageOutput(text) {
return /Usage:\s*gsd-tools/.test(text) && /Commands:/.test(text);
}
/**
* Isolated HOME directory used by runNpm() for the lifetime of this process.
*
* npm reads $HOME/.npmrc (user config) and writes to $HOME/.npm (default cache)
* when these paths are not overridden. On Docker hosts the running user's HOME
* may be uninitialized, unwritable, or contain stale state from a prior run —
* any of which causes `npm pack` / `npm install -g` to fail. Fix: create a
* fresh temp directory once per process, redirect HOME + cache + userconfig into
* it, and clean up on process exit. This makes runNpm() independent of the
* caller's environment. (#131)
*/
const _npmIsolatedHome = fs.mkdtempSync(path.join(require('os').tmpdir(), 'npm-home-'));
process.on('exit', () => {
try { fs.rmSync(_npmIsolatedHome, { recursive: true, force: true }); } catch (_) { /* best-effort */ }
});
/**
* Run `fn` with console.log/warn/error captured, returning {stdout, stderr}
* with ANSI colors stripped. Re-throws any exception fn threw AFTER restoring
@@ -248,12 +264,47 @@ function toPosixPath(p) {
function runNpm(args, options = {}) {
const isWindows = process.platform === 'win32';
const npmCmd = isWindows ? 'npm.cmd' : 'npm';
// Inject an isolated HOME so npm never reads from or writes to the caller's
// $HOME. This prevents failures on Docker hosts where HOME is unwritable or
// uninitialized. The caller may still pass { env: {...} } in options to
// further override specific variables — those overrides win because they are
// applied after the isolated env below (via the spread in the merge). (#131)
const isolatedEnv = {
...process.env,
HOME: _npmIsolatedHome,
npm_config_cache: path.join(_npmIsolatedHome, '.npm'),
npm_config_userconfig: path.join(_npmIsolatedHome, '.npmrc'),
};
const defaults = {
encoding: 'utf-8',
shell: isWindows,
timeout: 180000,
env: isolatedEnv,
};
return execFileSync(npmCmd, args, { ...defaults, ...options }).trim();
// Merge options; if caller passes their own env, merge it on top of isolatedEnv
// so the isolation is preserved unless the caller explicitly overrides HOME.
const { env: callerEnv, ...otherOptions } = options;
const mergedEnv = callerEnv ? { ...isolatedEnv, ...callerEnv } : isolatedEnv;
return execFileSync(npmCmd, args, { ...defaults, ...otherOptions, env: mergedEnv }).trim();
}
module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, parseFrontmatter, isUsageOutput, captureConsole, toPosixPath, runNpm, TOOLS_PATH };
/**
* Returns the isolated npm environment dict used by runNpm().
*
* Callers (e.g. runSmoke()) can spread this into a spawnSync env so that npm
* never reads from or writes to the caller's $HOME — the same guarantee
* runNpm() already provides. (#131)
*
* @returns {object} env dict with HOME, npm_config_cache, npm_config_userconfig
* pointing into a process-scoped temp directory.
*/
function isolatedNpmEnv() {
return {
...process.env,
HOME: _npmIsolatedHome,
npm_config_cache: path.join(_npmIsolatedHome, '.npm'),
npm_config_userconfig: path.join(_npmIsolatedHome, '.npmrc'),
};
}
module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, parseFrontmatter, isUsageOutput, captureConsole, toPosixPath, runNpm, isolatedNpmEnv, TOOLS_PATH };

View File

@@ -12,7 +12,7 @@ const assert = require('node:assert/strict');
const fs = require('node:fs');
const path = require('node:path');
const { cleanup, createTempDir, runNpm } = require('./helpers.cjs');
const { cleanup, createTempDir, runNpm, isolatedNpmEnv } = require('./helpers.cjs');
const { SMOKE, runSmoke } = require('../scripts/release-tarball-smoke.cjs');
const PKG_PATH = path.join(__dirname, '..', 'package.json');
@@ -71,6 +71,7 @@ describe('release-tarball-smoke', () => {
installPrefix,
expectedVersion: pkg.version,
fixtureDir,
npmEnv: isolatedNpmEnv(),
});
assert.equal(result.code, SMOKE.OK);
@@ -84,6 +85,7 @@ describe('release-tarball-smoke', () => {
installPrefix,
expectedVersion: '99.99.99',
fixtureDir,
npmEnv: isolatedNpmEnv(),
});
assert.equal(result.code, SMOKE.VERSION_MISMATCH);
@@ -101,6 +103,7 @@ describe('release-tarball-smoke', () => {
expectedVersion: pkg.version,
fixtureDir,
lifecycleCommands: ['init', 'discuss-phase', 'plan-phase'],
npmEnv: isolatedNpmEnv(),
});
assert.equal(result.code, SMOKE.OK);
@@ -139,6 +142,7 @@ describe('release-tarball-smoke', () => {
expectedVersion: pkg.version,
fixtureDir,
lifecycleCommands: ['init', 'nonexistent-phase-xyz'],
npmEnv: isolatedNpmEnv(),
});
assert.equal(result.code, SMOKE.COMMAND_FILE_MISSING);
@@ -156,6 +160,7 @@ describe('release-tarball-smoke', () => {
expectedVersion: pkg.version,
fixtureDir,
lifecycleCommands: [], // skip lifecycle checks; isolate SDK check
npmEnv: isolatedNpmEnv(),
});
// If the binary can't be called, code would be SDK_BINARY_NOT_CALLABLE
@@ -175,6 +180,7 @@ describe('release-tarball-smoke', () => {
expectedVersion: pkg.version,
fixtureDir,
lifecycleCommands: [],
npmEnv: isolatedNpmEnv(),
});
// Structural: the scan ran and populated the counters