diff --git a/scripts/release-tarball-smoke.cjs b/scripts/release-tarball-smoke.cjs index 47cc26309..07032d5dd 100644 --- a/scripts/release-tarball-smoke.cjs +++ b/scripts/release-tarball-smoke.cjs @@ -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) { diff --git a/tests/bug-131-release-tarball-smoke-explicit-home.test.cjs b/tests/bug-131-release-tarball-smoke-explicit-home.test.cjs new file mode 100644 index 000000000..fa531f98e --- /dev/null +++ b/tests/bug-131-release-tarball-smoke-explicit-home.test.cjs @@ -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}`, + ); + }); +}); diff --git a/tests/helpers.cjs b/tests/helpers.cjs index fba603e3f..109c9838a 100644 --- a/tests/helpers.cjs +++ b/tests/helpers.cjs @@ -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 }; diff --git a/tests/release-tarball-smoke.install.test.cjs b/tests/release-tarball-smoke.install.test.cjs index baee6124f..1e6b05e98 100644 --- a/tests/release-tarball-smoke.install.test.cjs +++ b/tests/release-tarball-smoke.install.test.cjs @@ -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