From b54562d764e59cbe33b7ff6a8743e1d2b391346c Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Mon, 12 Oct 2026 18:32:10 -0400 Subject: [PATCH 1/3] test: give native-compose CLI harnesses a private environment Tests that spawn the source CLI built the child env from `{ ...process.env, ... }`, so the contributor's HOME and inherited HACK_*, DOCKER_* and XDG_* variables reached the child CLI. Some had no HACK_HOME either and used the real ~/.hack. Add tests/helpers/private-cli-env.ts: privateCliEnv() gives the child a 0700 HOME inside the fixture, inherits only PATH and TMPDIR, and adds the isolated Docker client config. Migrate every source-CLI spawn under tests/ that spread process.env, plus the three harnesses fixed in #313. Add tests/cli-spawn-env-isolation.test.ts, which scans tests/ and fails when a file that spawns index.ts spreads process.env without a private HOME. Exceptions are listed with a reason and an exact count. Co-Authored-By: Claude Opus 5.5 --- tests/cli-spawn-env-isolation.test.ts | 218 ++++++++++++++++++ tests/doctor-command.test.ts | 29 ++- tests/doctor-domain-migration.test.ts | 15 +- tests/e2e/run-dependency-cache.test.ts | 19 +- tests/env-backend-command.test.ts | 49 ++-- tests/helpers/native-compose-command.ts | 34 +-- tests/helpers/native-compose-files.ts | 32 +-- tests/helpers/private-cli-env.ts | 81 +++++++ tests/mcp-lifetime.test.ts | 17 +- tests/mcp-prompts.test.ts | 8 +- tests/mcp.test.ts | 13 +- tests/native-ca-doctor-cli.test.ts | 32 ++- ...native-compose-adoption-generation.test.ts | 13 +- tests/native-compose-before-command.test.ts | 25 +- tests/native-compose-build-command.test.ts | 27 ++- tests/native-compose-network-command.test.ts | 31 +-- tests/native-compose-open.test.ts | 27 ++- .../native-compose-routed-run-command.test.ts | 29 +-- tests/native-compose-routing-command.test.ts | 27 ++- tests/native-compose-saved-command.test.ts | 65 +++--- ...compose-storage-activation-command.test.ts | 36 +-- tests/native-config-import-preview.test.ts | 17 +- tests/native-hack-local-route-cli.test.ts | 23 +- tests/native-project-down.test.ts | 47 ++-- tests/native-project-exec.test.ts | 19 +- tests/native-routing-project.test.ts | 11 +- tests/prerelease-plan.test.ts | 13 +- tests/project-env-local-inheritance.test.ts | 5 +- tests/project-env-worktree-key.test.ts | 18 +- 29 files changed, 686 insertions(+), 294 deletions(-) create mode 100644 tests/cli-spawn-env-isolation.test.ts create mode 100644 tests/helpers/private-cli-env.ts diff --git a/tests/cli-spawn-env-isolation.test.ts b/tests/cli-spawn-env-isolation.test.ts new file mode 100644 index 000000000..61965b475 --- /dev/null +++ b/tests/cli-spawn-env-isolation.test.ts @@ -0,0 +1,218 @@ +import { expect, test } from "bun:test"; +import { mkdtemp, readFile, rm, stat } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, relative, resolve } from "node:path"; +import { restoreEnv } from "./helpers/env.ts"; +import { + PRIVATE_CLI_INHERITED_ENV, + privateCliEnv, +} from "./helpers/private-cli-env.ts"; + +/** + * Guard: a test that spawns the source CLI must not hand it the test runner's + * environment with the contributor's HOME. Build the child env with + * `privateCliEnv` (tests/helpers/private-cli-env.ts), or at least override + * HOME with a path inside the fixture. + * + * This is a text scan, not a type check. A file "spawns the source CLI" when + * it names `index.ts` in a string literal or uses the E2E harness CLI + * entrypoint. In such a file, every object literal that spreads `process.env` + * must set its own `HOME`, and that `HOME` must not be the runner's home. + */ +const ALLOWED: Readonly< + Record +> = { + // `runCommand` runs git and docker for the real-engine E2E suite, not the + // CLI. CLI invocations in this harness use `buildCliEnv`. + "tests/e2e/harness.ts": { + reason: "runCommand spawns git/docker clients for real-engine E2E", + count: 1, + }, + // Release planning scripts (`scripts/prerelease-plan.ts`) read only + // GITHUB_* and release inputs; the source CLI spawn in this file is private. + "tests/prerelease-plan.test.ts": { + reason: "env objects for scripts/prerelease-plan.ts, not the CLI", + count: 2, + }, +}; + +const SOURCE_CLI = + /["'`](?:\.{1,2}\/)*index\.ts["'`]|\bCLI_ENTRYPOINT\b|\bresolveCliSpawnArgs\b/; +const SPREAD = /\.\.\.\s*process\.env(?![\w.[])/g; +const RUNNER_HOME = + /\bHOME\s*:\s*(?:process\.env\.HOME|process\.env\[["']HOME["']\]|homedir\(\))/; +const HOME_KEY = /(?:^|[\s,{])HOME\s*(?::|,|\})/; + +/** The text of the object literal that encloses `index`, or null. */ +function enclosingObject(source: string, index: number): string | null { + let depth = 0; + let start = -1; + for (let at = index - 1; at >= 0; at -= 1) { + const char = source[at]; + if (char === "}") { + depth += 1; + } else if (char === "{") { + if (depth === 0) { + start = at; + break; + } + depth -= 1; + } + } + if (start < 0) { + return null; + } + depth = 0; + for (let at = start; at < source.length; at += 1) { + const char = source[at]; + if (char === "{") { + depth += 1; + } else if (char === "}") { + depth -= 1; + if (depth === 0) { + return source.slice(start, at + 1); + } + } + } + return null; +} + +/** Line numbers of `...process.env` spreads that reach the source CLI with the runner's HOME. */ +function leakingCliEnvSpreads(source: string): number[] { + if (!SOURCE_CLI.test(source)) { + return []; + } + const lines: number[] = []; + for (const match of source.matchAll(SPREAD)) { + const object = enclosingObject(source, match.index); + const safe = + object !== null && HOME_KEY.test(object) && !RUNNER_HOME.test(object); + if (!safe) { + lines.push(source.slice(0, match.index).split("\n").length); + } + } + return lines; +} + +async function testSources(): Promise { + const root = resolve(import.meta.dir); + const files: string[] = []; + for await (const path of new Bun.Glob("**/*.{ts,tsx,mts}").scan({ + cwd: root, + absolute: true, + })) { + // This file names the patterns it scans for in its own examples. + if (!(path.includes("/node_modules/") || path === import.meta.path)) { + files.push(path); + } + } + return files.sort(); +} + +test("no test spawns the source CLI with the runner's environment and HOME", async () => { + const repo = resolve(import.meta.dir, ".."); + const violations: string[] = []; + const allowedSeen = new Map(); + for (const path of await testSources()) { + const name = relative(repo, path); + const lines = leakingCliEnvSpreads(await readFile(path, "utf8")); + if (lines.length === 0) { + continue; + } + const allowed = ALLOWED[name]; + if (allowed) { + allowedSeen.set(name, lines.length); + if (lines.length <= allowed.count) { + continue; + } + } + violations.push(`${name}:${lines.join(",")}`); + } + expect(violations).toEqual([]); + // Each exception still exists; remove stale entries rather than keep them. + for (const [name, allowed] of Object.entries(ALLOWED)) { + expect({ name, count: allowedSeen.get(name) ?? 0 }).toEqual({ + name, + count: allowed.count, + }); + } +}); + +test("the scan flags runner HOME and accepts a private HOME", () => { + const spawn = (env: string) => + `Bun.spawn([process.execPath, "../index.ts"], {\n env: ${env},\n});`; + expect( + leakingCliEnvSpreads(spawn("{ ...process.env, HACK_HOME: h }")) + ).toEqual([2]); + expect( + leakingCliEnvSpreads(spawn("{ ...process.env, HOME: process.env.HOME }")) + ).toEqual([2]); + expect( + leakingCliEnvSpreads(spawn("{ ...process.env, HOME: homedir() }")) + ).toEqual([2]); + // A nested object's HOME does not cover the outer spread. + expect( + leakingCliEnvSpreads(spawn("{ ...process.env, X: { HOME: h } }")) + ).toEqual([]); + expect( + leakingCliEnvSpreads(spawn('{ ...process.env, HOME: join(root, "home") }')) + ).toEqual([]); + expect(leakingCliEnvSpreads(spawn("{ ...process.env, HOME }"))).toEqual([]); + expect(leakingCliEnvSpreads(spawn("privateCliEnv({ root })"))).toEqual([]); + // Reading one variable is not a spread of the environment. + expect( + leakingCliEnvSpreads(spawn("{ PATH: `${process.env.PATH}` }")) + ).toEqual([]); + // Files that never spawn the source CLI are out of scope. + expect( + leakingCliEnvSpreads('Bun.spawn(["git"], { env: { ...process.env } });') + ).toEqual([]); +}); + +test("privateCliEnv keeps only the allow-list, test values and a private HOME", async () => { + const root = await mkdtemp(join(tmpdir(), "private-cli-env-")); + const saved = { + HOME: process.env.HOME, + HACK_HOME: process.env.HACK_HOME, + DOCKER_HOST: process.env.DOCKER_HOST, + DOCKER_CONTEXT: process.env.DOCKER_CONTEXT, + XDG_CONFIG_HOME: process.env.XDG_CONFIG_HOME, + }; + try { + process.env.HACK_HOME = "/runner/hack-home"; + process.env.DOCKER_HOST = "unix:///runner/docker.sock"; + process.env.DOCKER_CONTEXT = "runner-context"; + process.env.XDG_CONFIG_HOME = "/runner/xdg"; + const env = privateCliEnv({ + root, + env: { + HACK_HOME: join(root, "home"), + PATH: "/fixture/bin", + TMPDIR: undefined, + }, + }); + expect(env.HOME).toBe(join(root, "user-home")); + expect(env.HOME).not.toBe(saved.HOME); + expect((await stat(join(root, "user-home"))).mode & 0o777).toBe(0o700); + expect(env.HACK_HOME).toBe(join(root, "home")); + expect(env.PATH).toBe("/fixture/bin"); + expect(env.DOCKER_CONFIG).toBe(join(root, "docker-client")); + expect(Object.keys(env).sort()).toEqual( + ["DOCKER_CONFIG", "HACK_HOME", "HOME", "PATH"].sort() + ); + const inherited = privateCliEnv({ root, isolateDockerClient: false }); + for (const key of Object.keys(inherited)) { + expect([...PRIVATE_CLI_INHERITED_ENV, "HOME"]).toContain(key); + } + expect(() => privateCliEnv({ root, home: tmpdir() })).toThrow("inside"); + expect(() => privateCliEnv({ root, env: { HOME: root } })).toThrow( + "home option" + ); + expect(privateCliEnv({ root, home: root }).HOME).toBe(resolve(root)); + } finally { + for (const [key, value] of Object.entries(saved)) { + restoreEnv(key, value); + } + await rm(root, { recursive: true, force: true }); + } +}); diff --git a/tests/doctor-command.test.ts b/tests/doctor-command.test.ts index 6d7be786c..24986ae7e 100644 --- a/tests/doctor-command.test.ts +++ b/tests/doctor-command.test.ts @@ -22,6 +22,7 @@ import { buildRecoveryNextSteps, buildRecoveryWorkflowLines, } from "../src/commands/recovery-guidance.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; async function createDoctorTestProject(opts: { readonly modern?: boolean; @@ -74,18 +75,20 @@ test("doctor refuses native input before probing or repairing global runtime", a [process.execPath, cli, "doctor", "--path", project, ...flags], { cwd: project, - env: { - ...process.env, - HOME: root, - HACK_HOME: join(root, "state"), - HACK_GLOBAL_CONFIG_PATH: join(root, "state", "hack.config.json"), - HACK_DAEMON_DISABLE: "1", - HACK_NO_INTERACTIVE: "1", - HACK_LOGGER: "console", - XDG_CACHE_HOME: cache, - PATH: `${tools}:/usr/bin:/bin`, - RUNTIME_SENTINEL: sentinel, - }, + env: privateCliEnv({ + root, + home: root, + env: { + HACK_HOME: join(root, "state"), + HACK_GLOBAL_CONFIG_PATH: join(root, "state", "hack.config.json"), + HACK_DAEMON_DISABLE: "1", + HACK_NO_INTERACTIVE: "1", + HACK_LOGGER: "console", + XDG_CACHE_HOME: cache, + PATH: `${tools}:/usr/bin:/bin`, + RUNTIME_SENTINEL: sentinel, + }, + }), stdout: "pipe", stderr: "pipe", } @@ -104,8 +107,10 @@ test("doctor refuses native input before probing or repairing global runtime", a }); } expect(await Bun.file(sentinel).exists()).toBe(false); + // `docker-client` is the test's isolated Docker client config. expect((await readdir(root)).sort()).toEqual([ "cache", + "docker-client", "project", "tools", ]); diff --git a/tests/doctor-domain-migration.test.ts b/tests/doctor-domain-migration.test.ts index d35ed1bec..80c6dbeea 100644 --- a/tests/doctor-domain-migration.test.ts +++ b/tests/doctor-domain-migration.test.ts @@ -10,6 +10,7 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const directories: string[] = []; const sentinel = "PRIVATE_CONFIG_MUST_NOT_APPEAR"; @@ -60,12 +61,14 @@ async function invoke( ...extra, ], { - env: { - ...process.env, - HACK_HOME: f.home, - HACK_NO_INTERACTIVE: "1", - PATH: join(f.root, "no-tools"), - }, + env: privateCliEnv({ + root: f.root, + env: { + HACK_HOME: f.home, + HACK_NO_INTERACTIVE: "1", + PATH: join(f.root, "no-tools"), + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/e2e/run-dependency-cache.test.ts b/tests/e2e/run-dependency-cache.test.ts index ec1373909..c60e4be62 100644 --- a/tests/e2e/run-dependency-cache.test.ts +++ b/tests/e2e/run-dependency-cache.test.ts @@ -9,6 +9,7 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; +import { privateCliEnv } from "../helpers/private-cli-env.ts"; const roots: string[] = []; const cli = resolve(import.meta.dir, "../../index.ts"); @@ -129,14 +130,16 @@ else if(args.includes("run")) console.log("fixture-command-output"); { cwd: primary, timeout: 10_000, - env: { - ...process.env, - HOME: home, - HACK_HOME: join(home, ".hack"), - PATH: `${bin}:${process.env.PATH ?? ""}`, - HACK_NO_INTERACTIVE: "1", - HACK_LOGGER: "console", - }, + env: privateCliEnv({ + root, + home, + env: { + HACK_HOME: join(home, ".hack"), + PATH: `${bin}:${process.env.PATH ?? ""}`, + HACK_NO_INTERACTIVE: "1", + HACK_LOGGER: "console", + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/env-backend-command.test.ts b/tests/env-backend-command.test.ts index 796ef1107..676e74d5f 100644 --- a/tests/env-backend-command.test.ts +++ b/tests/env-backend-command.test.ts @@ -3,6 +3,7 @@ import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { restoreEnv } from "./helpers/env.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; type RunResult = { readonly exitCode: number; @@ -47,7 +48,6 @@ test( const result = await runHack({ args: ["env", "backend", "status", "--json"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", }, }); @@ -93,7 +93,7 @@ test( await writeFile(tempGlobalConfigPath, contents); const result = await runHack({ args: ["env", "backend", "use", "encrypted_file", "--json"], - env: { ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath }, + env: { HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath }, }); expect(result.exitCode).not.toBe(0); expect(`${result.stdout}${result.stderr}`).toContain( @@ -119,7 +119,6 @@ test( "--json", ], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", }, }); @@ -160,7 +159,6 @@ test( "--json", ], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath, }, }); @@ -206,7 +204,6 @@ test( "--json", ], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath, }, }); @@ -238,7 +235,6 @@ test( const invalid = await runHack({ args: ["env", "backend", "use", "cloud"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", }, }); @@ -262,7 +258,6 @@ test( "--json", ], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", }, }); @@ -336,7 +331,6 @@ test( const setResult = await runHack({ args: ["env", "set", "--secret", "API_KEY=super-secret"], env: { - ...process.env, HACK_AUTH_BROKER_URL: "http://127.0.0.1:9", HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-backend-command-key", @@ -356,7 +350,6 @@ test( const listResult = await runHack({ args: ["env", "list", "--show-secrets", "--json"], env: { - ...process.env, HACK_AUTH_BROKER_URL: "http://127.0.0.1:9", HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-backend-command-key", @@ -387,7 +380,6 @@ test( const redactedListResult = await runHack({ args: ["env", "list", "--json"], env: { - ...process.env, HACK_AUTH_BROKER_URL: "http://127.0.0.1:9", HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-backend-command-key", @@ -408,7 +400,6 @@ test( const listTextResult = await runHack({ args: ["env", "list", "--show-secrets"], env: { - ...process.env, HACK_AUTH_BROKER_URL: "http://127.0.0.1:9", HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-backend-command-key", @@ -474,7 +465,6 @@ test( const result = await runHack({ args: ["env", "set", "SECRET_TOKEN=super-secret"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", }, cwd: projectRoot, @@ -531,7 +521,6 @@ test( const result = await runHack({ args: ["env", "set", "SECRET_TOKEN=super-secret"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", }, cwd: projectRoot, @@ -602,7 +591,6 @@ test( const result = await runHack({ args: ["env", "set", "SECRET_TOKEN=super-secret"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", }, cwd: projectRoot, @@ -642,7 +630,6 @@ test( const result = await runHack({ args: ["env", "set", "SECRET_TOKEN=plain-value"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", }, cwd: projectRoot, @@ -710,7 +697,6 @@ test( const result = await runHack({ args: ["env", "list", "--json"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_DISABLE_KEYCHAIN_FALLBACK: "true", }, @@ -797,7 +783,6 @@ test( const result = await runHack({ args: ["env", "list", "--json"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-backend-command-key", }, @@ -962,7 +947,6 @@ test( )}\n` ); const hackEnv = { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", }; @@ -1072,7 +1056,6 @@ test( const setResult = await runHack({ args: ["env", "set", "PUBLIC_URL=https://example.test"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-backend-command-key", }, @@ -1091,7 +1074,6 @@ test( const listResult = await runHack({ args: ["env", "list", "--json", "--show-secrets"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-backend-command-key", }, @@ -1182,7 +1164,6 @@ test( const baseSet = await runHack({ args: ["env", "set", "--env=base", "PUBLIC_URL=https://base.test"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-overlay-command-key", }, @@ -1193,7 +1174,6 @@ test( const overlaySet = await runHack({ args: ["env", "set", "--env=qa", "PUBLIC_URL=https://qa.test"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-overlay-command-key", }, @@ -1212,7 +1192,6 @@ test( const defaultList = await runHack({ args: ["env", "list", "--json", "--show-secrets"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-overlay-command-key", }, @@ -1238,7 +1217,6 @@ test( const baseList = await runHack({ args: ["env", "list", "--json", "--show-secrets", "--env=base"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-overlay-command-key", }, @@ -1317,7 +1295,6 @@ test( const overlaySet = await runHack({ args: ["env", "set", "--env=qa", "PUBLIC_URL=https://qa-bundled.test"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-overlay-backend-key", }, @@ -1330,7 +1307,6 @@ test( const listResult = await runHack({ args: ["env", "list", "--json", "--show-secrets", "--env=qa"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-overlay-backend-key", }, @@ -1406,7 +1382,6 @@ test( const baseSet = await runHack({ args: ["env", "set", "--env=base", "PUBLIC_URL=https://base.test"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-overlay-manual-file-key", }, @@ -1422,7 +1397,6 @@ test( const listResult = await runHack({ args: ["env", "list", "--json", "--show-secrets", "--env=qa"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-overlay-manual-file-key", }, @@ -1502,7 +1476,6 @@ test( const listResult = await runHack({ args: ["env", "list", "--json", "--env=qa"], env: { - ...process.env, HACK_GLOBAL_CONFIG_PATH: tempGlobalConfigPath ?? "", HACK_SECRETS_FILE_KEY: "env-overlay-warning-key", }, @@ -1531,16 +1504,30 @@ test( { timeout: 40_000 } ); +/** + * Runs the source CLI with a private HOME under the test's temp directory. + * Only the file-store key a test sets in-process and the values in `env` reach + * the child; the runner's HOME, keychain and HACK_* settings do not. + */ async function runHack(input: { readonly args: readonly string[]; - readonly env: NodeJS.ProcessEnv; + readonly env: Readonly>; readonly cwd?: string; }): Promise { + if (!tempDir) { + throw new Error("Missing temp directory state"); + } const proc = Bun.spawn( ["bun", resolve(import.meta.dir, "../index.ts"), ...input.args], { cwd: input.cwd ?? resolve(import.meta.dir, ".."), - env: input.env, + env: privateCliEnv({ + root: tempDir, + env: { + HACK_SECRETS_FILE_KEY: process.env.HACK_SECRETS_FILE_KEY, + ...input.env, + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/helpers/native-compose-command.ts b/tests/helpers/native-compose-command.ts index 2d5c39329..2aebc693f 100644 --- a/tests/helpers/native-compose-command.ts +++ b/tests/helpers/native-compose-command.ts @@ -2,8 +2,8 @@ import { afterEach, expect } from "bun:test"; import { chmod, mkdir, mkdtemp, realpath, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; -import { isolatedDockerClientEnv } from "./docker-client.ts"; import { nativeComposeStorageDockerFixtureScript } from "./native-compose-storage-docker.ts"; +import { privateCliEnv } from "./private-cli-env.ts"; const roots: string[] = []; const cleanupGuards = new Map boolean>(); @@ -247,21 +247,23 @@ export async function invoke( ], { cwd: root, - env: { - HOME: join(root, "user-home"), - LANG: "C", - PATH: `${root}:/usr/bin:/bin`, - HACK_HOME: join(root, "home"), - HACK_GLOBAL_CONFIG_PATH: join(root, "global.json"), - HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), - HACK_RUNTIME_BACKEND: "compose", - HACK_LOGGER: "console", - HACK_COMPOSE_STARTUP_TIMEOUT_MS: String(startupTimeoutMs), - CI: "1", - HACK_EXECUTION_MODE: "non_interactive", - ...isolatedDockerClientEnv(root), - ...extraEnv, - }, + // Private HOME (`/user-home`) and isolated Docker client config. + env: privateCliEnv({ + root, + env: { + LANG: "C", + PATH: `${root}:/usr/bin:/bin`, + HACK_HOME: join(root, "home"), + HACK_GLOBAL_CONFIG_PATH: join(root, "global.json"), + HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), + HACK_RUNTIME_BACKEND: "compose", + HACK_LOGGER: "console", + HACK_COMPOSE_STARTUP_TIMEOUT_MS: String(startupTimeoutMs), + CI: "1", + HACK_EXECUTION_MODE: "non_interactive", + ...extraEnv, + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/helpers/native-compose-files.ts b/tests/helpers/native-compose-files.ts index 957b07662..c33c9e931 100644 --- a/tests/helpers/native-compose-files.ts +++ b/tests/helpers/native-compose-files.ts @@ -11,6 +11,7 @@ import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { isRecord } from "../../src/lib/guards.ts"; import { openNativeComposeGenerationStore } from "../../src/lib/native-compose-generation.ts"; +import { privateCliEnv } from "./private-cli-env.ts"; const roots: string[] = []; const children: Bun.Subprocess<"ignore", "pipe", "pipe">[] = []; @@ -168,20 +169,23 @@ export function spawnFiles( ], { cwd: fixture.root, - env: { - HOME: join(fixture.parent, "user-home"), - LANG: "C", - PATH: `${fixture.parent}:/usr/bin:/bin`, - HACK_HOME: join(fixture.parent, "home"), - HACK_GLOBAL_CONFIG_PATH: join(fixture.parent, "global.json"), - HACK_CONFIG_COMPILER_BINARY: compiler, - HACK_RUNTIME_BACKEND: "compose", - HACK_LOGGER: "console", - HACK_COMPOSE_STARTUP_TIMEOUT_MS: "3000", - CI: "1", - HACK_EXECUTION_MODE: "non_interactive", - ...extraEnv, - }, + // Private HOME (`/user-home`) and isolated Docker client config. + env: privateCliEnv({ + root: fixture.parent, + env: { + LANG: "C", + PATH: `${fixture.parent}:/usr/bin:/bin`, + HACK_HOME: join(fixture.parent, "home"), + HACK_GLOBAL_CONFIG_PATH: join(fixture.parent, "global.json"), + HACK_CONFIG_COMPILER_BINARY: compiler, + HACK_RUNTIME_BACKEND: "compose", + HACK_LOGGER: "console", + HACK_COMPOSE_STARTUP_TIMEOUT_MS: "3000", + CI: "1", + HACK_EXECUTION_MODE: "non_interactive", + ...extraEnv, + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/helpers/private-cli-env.ts b/tests/helpers/private-cli-env.ts new file mode 100644 index 000000000..1b5b77232 --- /dev/null +++ b/tests/helpers/private-cli-env.ts @@ -0,0 +1,81 @@ +import { mkdirSync } from "node:fs"; +import { isAbsolute, join, relative, resolve } from "node:path"; +import { isolatedDockerClientEnv } from "./docker-client.ts"; + +/** + * The only test-runner variables a spawned source CLI inherits. Everything else + * in the runner's environment is dropped: the contributor's `HOME`, any + * `HACK_*` setting, `DOCKER_HOST`, `DOCKER_CONTEXT`, `DOCKER_CONFIG`, `XDG_*`, + * `CI`, credentials and shell state. A harness passes what its test needs + * through `env` instead. + */ +export const PRIVATE_CLI_INHERITED_ENV = ["PATH", "TMPDIR"] as const; + +/** The default private HOME of a fixture root. */ +export function privateCliHome(root: string): string { + return join(root, "user-home"); +} + +export type PrivateCliEnvOptions = { + /** Fixture directory that owns the private HOME and Docker client config. */ + readonly root: string; + /** + * HOME for the child. It must be the fixture root or a path inside it. + * Defaults to `/user-home`. It is created with mode 0700 when absent. + */ + readonly home?: string; + /** + * Values the test needs, applied after the inherited allow-list. An + * `undefined` value removes an inherited key. `HOME` is not accepted here. + */ + readonly env?: Readonly>; + /** + * Default true: an isolated Docker client config whose current context does + * not exist, so no real engine is selected. A `DOCKER_HOST` or + * `DOCKER_CONFIG` passed in `env` still wins. Pass false only when the test + * asserts the absence of a Docker client config. + */ + readonly isolateDockerClient?: boolean; +}; + +/** + * Builds the complete environment for a child process that runs the source CLI + * (`process.execPath index.ts ...`). Use it instead of `{ ...process.env }`. + * + * Invariant: the child's HOME is always private to the fixture, and no + * variable from the test runner reaches the child unless it is on + * {@link PRIVATE_CLI_INHERITED_ENV} or the test passes it explicitly. + */ +export function privateCliEnv( + opts: PrivateCliEnvOptions +): Record { + const root = resolve(opts.root); + const home = resolve(opts.home ?? privateCliHome(root)); + const inside = relative(root, home); + if (inside.startsWith("..") || isAbsolute(inside)) { + throw new Error("privateCliEnv: HOME must be inside the fixture root."); + } + if (opts.env && Object.hasOwn(opts.env, "HOME")) { + throw new Error("privateCliEnv: pass HOME through the home option."); + } + mkdirSync(home, { recursive: true, mode: 0o700 }); + const env: Record = {}; + for (const key of PRIVATE_CLI_INHERITED_ENV) { + const value = process.env[key]; + if (typeof value === "string" && value.length > 0) { + env[key] = value; + } + } + if (opts.isolateDockerClient !== false) { + Object.assign(env, isolatedDockerClientEnv(root)); + } + for (const [key, value] of Object.entries(opts.env ?? {})) { + if (value === undefined) { + Reflect.deleteProperty(env, key); + } else { + env[key] = value; + } + } + env.HOME = home; + return env; +} diff --git a/tests/mcp-lifetime.test.ts b/tests/mcp-lifetime.test.ts index 4a7787f61..99945f81b 100644 --- a/tests/mcp-lifetime.test.ts +++ b/tests/mcp-lifetime.test.ts @@ -10,6 +10,7 @@ import { import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { readLinesFromStream } from "../src/ui/lines.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const fixtures: Array<{ root: string; @@ -86,13 +87,15 @@ async function startFixture( ], { cwd: root, - env: { - ...process.env, - HOME: root, - HACK_HOME: join(root, ".hack"), - HACK_GLOBAL_CONFIG_PATH: join(root, "global", "hack.config.json"), - HACK_MCP_COMMAND: stub, - }, + env: privateCliEnv({ + root, + home: root, + env: { + HACK_HOME: join(root, ".hack"), + HACK_GLOBAL_CONFIG_PATH: join(root, "global", "hack.config.json"), + HACK_MCP_COMMAND: stub, + }, + }), stdin: "pipe", stdout: "pipe", stderr: "ignore", diff --git a/tests/mcp-prompts.test.ts b/tests/mcp-prompts.test.ts index c78248bc0..39b54833b 100644 --- a/tests/mcp-prompts.test.ts +++ b/tests/mcp-prompts.test.ts @@ -2,11 +2,10 @@ import { afterEach, expect } from "bun:test"; import { mkdir, mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; - import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; - import { testIntegration } from "./helpers/ci.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; let tempDir: string | null = null; let client: Client | null = null; @@ -93,10 +92,7 @@ async function startMcpClient(): Promise { command: "bun", args: ["index.ts", "mcp", "serve"], cwd: repoRoot, - env: { - ...process.env, - HOME: homeDir, - }, + env: privateCliEnv({ root: tempDir, home: homeDir }), stderr: "pipe", }); diff --git a/tests/mcp.test.ts b/tests/mcp.test.ts index 1dfce8c04..e69b7fd40 100644 --- a/tests/mcp.test.ts +++ b/tests/mcp.test.ts @@ -9,11 +9,9 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; - import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; - import { parseCliArgv, resolveCommand } from "../src/cli/command.ts"; import { CLI_SPEC } from "../src/cli/spec.ts"; import { @@ -25,6 +23,7 @@ import { type McpRefusalReason, } from "../src/mcp/server.ts"; import { testIntegration } from "./helpers/ci.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; let tempDir: string | null = null; let client: Client | null = null; @@ -122,11 +121,11 @@ async function startMcpClient(): Promise { command: "bun", args: ["index.ts", "mcp", "serve"], cwd: repoRoot, - env: { - ...process.env, - HACK_MCP_COMMAND: stubPath, - HOME: homeDir, - }, + env: privateCliEnv({ + root: tempDir, + home: homeDir, + env: { HACK_MCP_COMMAND: stubPath }, + }), stderr: "pipe", }); diff --git a/tests/native-ca-doctor-cli.test.ts b/tests/native-ca-doctor-cli.test.ts index 8d43196fd..8cbbdc1f7 100644 --- a/tests/native-ca-doctor-cli.test.ts +++ b/tests/native-ca-doctor-cli.test.ts @@ -1,14 +1,30 @@ -import { expect, test } from "bun:test"; +import { afterEach, expect, test } from "bun:test"; +import { mkdtemp, realpath, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; + +const roots: string[] = []; +afterEach(async () => { + await Promise.all( + roots.splice(0).map((root) => rm(root, { recursive: true, force: true })) + ); +}); async function invoke(args: string[]) { + const root = await realpath(await mkdtemp(join(tmpdir(), "native-ca-cli-"))); + roots.push(root); const child = Bun.spawn([process.execPath, "index.ts", ...args], { - env: { - ...process.env, - HACK_RUNTIME_BACKEND: "native", - HACK_NATIVE_BINARY: "/nonexistent/hack-native", - HACK_NATIVE_HOME: "/nonexistent/native-home", - HACK_NO_INTERACTIVE: "1", - }, + env: privateCliEnv({ + root, + env: { + HACK_HOME: join(root, "home"), + HACK_RUNTIME_BACKEND: "native", + HACK_NATIVE_BINARY: "/nonexistent/hack-native", + HACK_NATIVE_HOME: "/nonexistent/native-home", + HACK_NO_INTERACTIVE: "1", + }, + }), stdout: "pipe", stderr: "pipe", timeout: 5000, diff --git a/tests/native-compose-adoption-generation.test.ts b/tests/native-compose-adoption-generation.test.ts index 02695e342..da07a4e76 100644 --- a/tests/native-compose-adoption-generation.test.ts +++ b/tests/native-compose-adoption-generation.test.ts @@ -48,6 +48,7 @@ import { completedJobFixtureSources } from "./e2e/scenarios/native-compose-adopt import { createCompletedJobFixtureSettlement } from "./e2e/scenarios/native-compose-adoption-job-worktrees.ts"; import { restoreEnv } from "./helpers/env.ts"; import { managedEnvCompilerFixture } from "./helpers/managed-env-compiler.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; // Each owner workflow performs multiple bounded child probes; allow their cumulative work on shared CI hosts. const test = (name: string, run: () => Promise) => @@ -4011,11 +4012,13 @@ test("CLI adoption refuses other backends before probes, preparation or engine e ...flags, ], { - env: { - ...process.env, - HACK_RUNTIME_BACKEND: "native", - HACK_HOME: join(root, "isolated-home"), - }, + env: privateCliEnv({ + root, + env: { + HACK_RUNTIME_BACKEND: "native", + HACK_HOME: join(root, "isolated-home"), + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/native-compose-before-command.test.ts b/tests/native-compose-before-command.test.ts index b6df38205..eb3ba325e 100644 --- a/tests/native-compose-before-command.test.ts +++ b/tests/native-compose-before-command.test.ts @@ -2,6 +2,7 @@ import { afterEach, expect, test } from "bun:test"; import { chmod, mkdir, mkdtemp, realpath, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const roots: string[] = []; afterEach(async () => { @@ -128,17 +129,19 @@ async function invoke(root: string) { ], { cwd: root, - env: { - ...process.env, - PATH: `${root}:/usr/bin:/bin`, - HACK_HOME: join(root, "home"), - HACK_GLOBAL_CONFIG_PATH: join(root, "global.json"), - HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), - HACK_RUNTIME_BACKEND: "compose", - HACK_LOGGER: "console", - CI: "1", - HACK_EXECUTION_MODE: "non_interactive", - }, + env: privateCliEnv({ + root, + env: { + PATH: `${root}:/usr/bin:/bin`, + HACK_HOME: join(root, "home"), + HACK_GLOBAL_CONFIG_PATH: join(root, "global.json"), + HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), + HACK_RUNTIME_BACKEND: "compose", + HACK_LOGGER: "console", + CI: "1", + HACK_EXECUTION_MODE: "non_interactive", + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/native-compose-build-command.test.ts b/tests/native-compose-build-command.test.ts index 97a546ec2..ed622833f 100644 --- a/tests/native-compose-build-command.test.ts +++ b/tests/native-compose-build-command.test.ts @@ -3,6 +3,7 @@ import { chmod, mkdir, mkdtemp, realpath, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { isRecord } from "../src/lib/guards.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const roots: string[] = []; afterEach(async () => { @@ -126,18 +127,20 @@ async function invoke(root: string, operation: "up" | "run") { ], { cwd: root, - env: { - ...process.env, - PATH: `${root}:/usr/bin:/bin`, - HACK_HOME: join(root, "home"), - HACK_GLOBAL_CONFIG_PATH: join(root, "global.json"), - HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), - HACK_RUNTIME_BACKEND: "compose", - HACK_LOGGER: "console", - AMBIENT: "must-not-expand", - CI: "1", - HACK_EXECUTION_MODE: "non_interactive", - }, + env: privateCliEnv({ + root, + env: { + PATH: `${root}:/usr/bin:/bin`, + HACK_HOME: join(root, "home"), + HACK_GLOBAL_CONFIG_PATH: join(root, "global.json"), + HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), + HACK_RUNTIME_BACKEND: "compose", + HACK_LOGGER: "console", + AMBIENT: "must-not-expand", + CI: "1", + HACK_EXECUTION_MODE: "non_interactive", + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/native-compose-network-command.test.ts b/tests/native-compose-network-command.test.ts index fc24cc9f0..e4f2ecb36 100644 --- a/tests/native-compose-network-command.test.ts +++ b/tests/native-compose-network-command.test.ts @@ -2,6 +2,7 @@ import { afterEach, expect, test } from "bun:test"; import { chmod, mkdir, mkdtemp, realpath, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const roots: string[] = []; const children: Bun.Subprocess<"ignore", "pipe", "pipe">[] = []; @@ -87,20 +88,22 @@ process.exit(23); ], { cwd: root, - env: { - ...process.env, - PATH: `${root}:/usr/bin:/bin`, - HACK_HOME: join(root, "home"), - HACK_GLOBAL_CONFIG_PATH: join(root, "global.json"), - HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), - HACK_RUNTIME_BACKEND: "compose", - HACK_LOGGER: "console", - CI: "1", - HACK_EXECUTION_MODE: "non_interactive", - GIT_DIR: "", - GIT_WORK_TREE: "", - GIT_COMMON_DIR: "", - }, + env: privateCliEnv({ + root, + env: { + PATH: `${root}:/usr/bin:/bin`, + HACK_HOME: join(root, "home"), + HACK_GLOBAL_CONFIG_PATH: join(root, "global.json"), + HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), + HACK_RUNTIME_BACKEND: "compose", + HACK_LOGGER: "console", + CI: "1", + HACK_EXECUTION_MODE: "non_interactive", + GIT_DIR: "", + GIT_WORK_TREE: "", + GIT_COMMON_DIR: "", + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/native-compose-open.test.ts b/tests/native-compose-open.test.ts index cae2e1f87..42db9315c 100644 --- a/tests/native-compose-open.test.ts +++ b/tests/native-compose-open.test.ts @@ -10,6 +10,7 @@ import { openNativeComposeRouteClaims } from "../src/lib/native-compose-route-cl import { prepareNativeComposeRouteOwner } from "../src/lib/native-compose-route-owner.ts"; import type { NativeRoutingResolution } from "../src/lib/native-routing-plan-protocol.ts"; import { composeFixture } from "./helpers/native-compose.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const PRIVATE_CANARY = "native-open-private-value-must-not-leak"; const roots: string[] = []; @@ -202,18 +203,20 @@ async function invoke(root: string, args: readonly string[]) { ], { cwd: root, - env: { - ...process.env, - PATH: `${root}:/usr/bin:/bin`, - HACK_HOME: join(root, "home"), - HACK_GLOBAL_CONFIG_PATH: join(root, "home", "hack.config.json"), - HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), - HACK_RUNTIME_BACKEND: "compose", - HACK_NO_INTERACTIVE: "1", - HACK_LOGGER: "console", - GIT_DIR: "", - GIT_WORK_TREE: "", - }, + env: privateCliEnv({ + root, + env: { + PATH: `${root}:/usr/bin:/bin`, + HACK_HOME: join(root, "home"), + HACK_GLOBAL_CONFIG_PATH: join(root, "home", "hack.config.json"), + HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), + HACK_RUNTIME_BACKEND: "compose", + HACK_NO_INTERACTIVE: "1", + HACK_LOGGER: "console", + GIT_DIR: "", + GIT_WORK_TREE: "", + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/native-compose-routed-run-command.test.ts b/tests/native-compose-routed-run-command.test.ts index 7521cc833..256045424 100644 --- a/tests/native-compose-routed-run-command.test.ts +++ b/tests/native-compose-routed-run-command.test.ts @@ -17,6 +17,7 @@ import { openNativeComposeRouteClaims } from "../src/lib/native-compose-route-cl import { prepareNativeComposeRouteOwner } from "../src/lib/native-compose-route-owner.ts"; import type { NativeRoutingResolution } from "../src/lib/native-routing-plan-protocol.ts"; import { composeFixture } from "./helpers/native-compose.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const roots: string[] = []; const children: Bun.Subprocess<"ignore", "pipe", "pipe">[] = []; @@ -280,19 +281,21 @@ async function invoke(root: string) { ], { cwd: root, - env: { - ...process.env, - PATH: `${root}:/usr/bin:/bin`, - HACK_HOME: join(root, "home"), - HACK_GLOBAL_CONFIG_PATH: join(root, "home/hack.config.json"), - HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), - HACK_RUNTIME_BACKEND: "compose", - HACK_LOGGER: "console", - HACK_COMPOSE_STARTUP_TIMEOUT_MS: "10000", - GIT_DIR: "", - GIT_WORK_TREE: "", - GIT_COMMON_DIR: "", - }, + env: privateCliEnv({ + root, + env: { + PATH: `${root}:/usr/bin:/bin`, + HACK_HOME: join(root, "home"), + HACK_GLOBAL_CONFIG_PATH: join(root, "home/hack.config.json"), + HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), + HACK_RUNTIME_BACKEND: "compose", + HACK_LOGGER: "console", + HACK_COMPOSE_STARTUP_TIMEOUT_MS: "10000", + GIT_DIR: "", + GIT_WORK_TREE: "", + GIT_COMMON_DIR: "", + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/native-compose-routing-command.test.ts b/tests/native-compose-routing-command.test.ts index 6925bbbed..36cc12d0e 100644 --- a/tests/native-compose-routing-command.test.ts +++ b/tests/native-compose-routing-command.test.ts @@ -11,6 +11,7 @@ import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { openNativeComposeRouteClaims } from "../src/lib/native-compose-route-claims.ts"; import { composeFixture } from "./helpers/native-compose.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const BINDING = { engineId: "fixture-engine:1", @@ -233,18 +234,20 @@ async function invoke(root: string, args: readonly string[]) { ], { cwd: root, - env: { - ...process.env, - PATH: `${root}:/usr/bin:/bin`, - HACK_HOME: join(root, "home"), - HACK_GLOBAL_CONFIG_PATH: join(root, "home", "hack.config.json"), - HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), - HACK_RUNTIME_BACKEND: "compose", - HACK_LOGGER: "console", - GIT_DIR: "", - GIT_WORK_TREE: "", - GIT_COMMON_DIR: "", - }, + env: privateCliEnv({ + root, + env: { + PATH: `${root}:/usr/bin:/bin`, + HACK_HOME: join(root, "home"), + HACK_GLOBAL_CONFIG_PATH: join(root, "home", "hack.config.json"), + HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), + HACK_RUNTIME_BACKEND: "compose", + HACK_LOGGER: "console", + GIT_DIR: "", + GIT_WORK_TREE: "", + GIT_COMMON_DIR: "", + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/native-compose-saved-command.test.ts b/tests/native-compose-saved-command.test.ts index 80427da02..7a1d427f9 100644 --- a/tests/native-compose-saved-command.test.ts +++ b/tests/native-compose-saved-command.test.ts @@ -20,6 +20,7 @@ import { } from "../src/lib/native-compose-route-owner.ts"; import type { NativeRoutingResolution } from "../src/lib/native-routing-plan-protocol.ts"; import { composeFixture } from "./helpers/native-compose.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const roots: string[] = []; const children: Bun.Subprocess<"ignore", "pipe", "pipe">[] = []; @@ -358,13 +359,15 @@ async function recoverSavedStop(root: string) { ], { cwd: root, - env: { - ...process.env, - PATH: `${root}:/usr/bin:/bin`, - HACK_HOME: join(root, "home"), - HACK_RUNTIME_BACKEND: "compose", - HACK_LOGGER: "console", - }, + env: privateCliEnv({ + root, + env: { + PATH: `${root}:/usr/bin:/bin`, + HACK_HOME: join(root, "home"), + HACK_RUNTIME_BACKEND: "compose", + HACK_LOGGER: "console", + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", @@ -385,13 +388,15 @@ test("saved ps reports pending state after an incomplete first startup", async ( [process.execPath, resolve(import.meta.dir, "../index.ts"), "ps", "--json"], { cwd: root, - env: { - ...process.env, - PATH: `${root}:/usr/bin:/bin`, - HACK_HOME: join(root, "home"), - HACK_RUNTIME_BACKEND: "compose", - HACK_LOGGER: "console", - }, + env: privateCliEnv({ + root, + env: { + PATH: `${root}:/usr/bin:/bin`, + HACK_HOME: join(root, "home"), + HACK_RUNTIME_BACKEND: "compose", + HACK_LOGGER: "console", + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", @@ -430,13 +435,15 @@ test.each([ ], { cwd: root, - env: { - ...process.env, - PATH: `${root}:/usr/bin:/bin`, - HACK_HOME: join(root, "home"), - HACK_RUNTIME_BACKEND: "compose", - HACK_LOGGER: "console", - }, + env: privateCliEnv({ + root, + env: { + PATH: `${root}:/usr/bin:/bin`, + HACK_HOME: join(root, "home"), + HACK_RUNTIME_BACKEND: "compose", + HACK_LOGGER: "console", + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", @@ -522,13 +529,15 @@ process.exit(0); ], { cwd: root, - env: { - ...process.env, - PATH: `${root}:/usr/bin:/bin`, - HACK_HOME: join(root, "home"), - HACK_RUNTIME_BACKEND: "compose", - HACK_LOGGER: "console", - }, + env: privateCliEnv({ + root, + env: { + PATH: `${root}:/usr/bin:/bin`, + HACK_HOME: join(root, "home"), + HACK_RUNTIME_BACKEND: "compose", + HACK_LOGGER: "console", + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/native-compose-storage-activation-command.test.ts b/tests/native-compose-storage-activation-command.test.ts index e44bc003c..73f398c67 100644 --- a/tests/native-compose-storage-activation-command.test.ts +++ b/tests/native-compose-storage-activation-command.test.ts @@ -2,8 +2,8 @@ import { afterEach, expect, test } from "bun:test"; import { join, resolve } from "node:path"; import { openNativeComposeGenerationStore } from "../src/lib/native-compose-generation.ts"; import { captureCompletedJobFixtureCommand } from "./e2e/scenarios/native-compose-adoption-job-worktrees.ts"; -import { isolatedDockerClientEnv } from "./helpers/docker-client.ts"; import { fixture, invoke } from "./helpers/native-compose-command.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const storage = { data: { kind: "persistent", scope: "worktree" } } as const; // Each synthetic carrier has the ordinary finite 30s proof budget. The wrapper @@ -70,22 +70,24 @@ async function ownedColdCase( ...args, ], cwd: root, - env: { - // A private HOME under the fixture, never the contributor's. - HOME: join(root, "user-home"), - LANG: "C", - PATH: `${root}:/usr/bin:/bin`, - HACK_HOME: join(root, "home"), - HACK_GLOBAL_CONFIG_PATH: join(root, "global.json"), - HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), - HACK_RUNTIME_BACKEND: "compose", - HACK_LOGGER: "console", - HACK_NATIVE_COMPOSE_PHASE_TRACE: "1", - HACK_COMPOSE_STARTUP_TIMEOUT_MS: "30000", - CI: "1", - HACK_EXECUTION_MODE: "non_interactive", - ...isolatedDockerClientEnv(root), - }, + // A private HOME under the fixture, never the contributor's, and + // an isolated Docker client config. + env: privateCliEnv({ + root, + env: { + LANG: "C", + PATH: `${root}:/usr/bin:/bin`, + HACK_HOME: join(root, "home"), + HACK_GLOBAL_CONFIG_PATH: join(root, "global.json"), + HACK_CONFIG_COMPILER_BINARY: join(root, "compiler"), + HACK_RUNTIME_BACKEND: "compose", + HACK_LOGGER: "console", + HACK_NATIVE_COMPOSE_PHASE_TRACE: "1", + HACK_COMPOSE_STARTUP_TIMEOUT_MS: "30000", + CI: "1", + HACK_EXECUTION_MODE: "non_interactive", + }, + }), captures: join(root, `storage-capture-${capture++}`), timeoutMs: 120_000, onUnconfirmed: retain, diff --git a/tests/native-config-import-preview.test.ts b/tests/native-config-import-preview.test.ts index c0aad3336..93bacdd97 100644 --- a/tests/native-config-import-preview.test.ts +++ b/tests/native-config-import-preview.test.ts @@ -15,6 +15,7 @@ import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { previewNativeConfigImport } from "../src/lib/native-config-import-preview.ts"; import { restoreEnv } from "./helpers/env.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const CANARY = "synthetic-private-import-owner"; const PROTOCOL = { transport_version: 1, authored_version: 1, plan_version: 1 }; @@ -311,11 +312,17 @@ test("CLI requires dry-run, reports values-free mappings and preserves files", a ], { cwd, - env: { - ...process.env, - HACK_CONFIG_COMPILER_BINARY: binary, - FORCE_COLOR: "0", - }, + env: privateCliEnv({ + root, + env: { + HACK_HOME: process.env.HACK_HOME, + HACK_GLOBAL_CONFIG_PATH: process.env.HACK_GLOBAL_CONFIG_PATH, + HACK_ENV_SECRET_KEY: process.env.HACK_ENV_SECRET_KEY, + + HACK_CONFIG_COMPILER_BINARY: binary, + FORCE_COLOR: "0", + }, + }), stdout: "pipe", stderr: "pipe", } diff --git a/tests/native-hack-local-route-cli.test.ts b/tests/native-hack-local-route-cli.test.ts index 7d3f8e22c..b9c10543b 100644 --- a/tests/native-hack-local-route-cli.test.ts +++ b/tests/native-hack-local-route-cli.test.ts @@ -2,6 +2,7 @@ import { afterEach, expect, test } from "bun:test"; import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const roots: string[] = []; @@ -16,16 +17,18 @@ async function invoke(args: string[]) { const home = await mkdtemp(join(tmpdir(), "hack-local-route-cli-")); roots.push(home); const child = Bun.spawn([process.execPath, "index.ts", ...args], { - env: { - ...process.env, - HOME: home, - HACK_HOME: join(home, ".hack"), - HACK_GLOBAL_CONFIG_PATH: join(home, ".hack", "hack.config.json"), - HACK_RUNTIME_BACKEND: "native", - HACK_NATIVE_BINARY: "/nonexistent/hack-native", - HACK_NATIVE_HOME: "/nonexistent/native-home", - HACK_NO_INTERACTIVE: "1", - }, + env: privateCliEnv({ + root: home, + home, + env: { + HACK_HOME: join(home, ".hack"), + HACK_GLOBAL_CONFIG_PATH: join(home, ".hack", "hack.config.json"), + HACK_RUNTIME_BACKEND: "native", + HACK_NATIVE_BINARY: "/nonexistent/hack-native", + HACK_NATIVE_HOME: "/nonexistent/native-home", + HACK_NO_INTERACTIVE: "1", + }, + }), stdout: "pipe", stderr: "pipe", timeout: 10_000, diff --git a/tests/native-project-down.test.ts b/tests/native-project-down.test.ts index 760d9f738..c76aa3cbc 100644 --- a/tests/native-project-down.test.ts +++ b/tests/native-project-down.test.ts @@ -17,6 +17,7 @@ import { removeNativeProjectRun, saveNativeProjectRun, } from "../src/backends/native-project-run.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const roots: string[] = []; afterEach(async () => { @@ -431,6 +432,10 @@ test("foreign owner refuses before hooks or cleanup", async () => { }); test("native down rejects unsupported flags before project or runtime access", async () => { + const root = await realpath( + await mkdtemp(join(tmpdir(), "native-down-flags-")) + ); + roots.push(root); for (const args of [ ["--env", "qa"], ["--profile", "web"], @@ -439,12 +444,14 @@ test("native down rejects unsupported flags before project or runtime access", a ["--target", "remote"], ]) { const child = Bun.spawn([process.execPath, "index.ts", "down", ...args], { - env: { - ...process.env, - HACK_RUNTIME_BACKEND: "native", - HACK_NATIVE_HOME: "/nonexistent-native-home", - HACK_NATIVE_BINARY: "/nonexistent-native-binary", - }, + env: privateCliEnv({ + root, + env: { + HACK_RUNTIME_BACKEND: "native", + HACK_NATIVE_HOME: "/nonexistent-native-home", + HACK_NATIVE_BINARY: "/nonexistent-native-binary", + }, + }), stdout: "pipe", stderr: "pipe", }); @@ -479,12 +486,14 @@ test("not-started native down does not run configured hooks", async () => { const child = Bun.spawn( [process.execPath, "index.ts", "down", "--path", opts.scope.projectRoot], { - env: { - ...process.env, - HACK_RUNTIME_BACKEND: "native", - HACK_NATIVE_HOME: opts.scope.nativeHome, - HACK_NATIVE_BINARY: "/nonexistent-native-binary", - }, + env: privateCliEnv({ + root: opts.scope.projectRoot, + env: { + HACK_RUNTIME_BACKEND: "native", + HACK_NATIVE_HOME: opts.scope.nativeHome, + HACK_NATIVE_BINARY: "/nonexistent-native-binary", + }, + }), stdout: "pipe", stderr: "pipe", } @@ -641,12 +650,14 @@ test("native down JSON isolates hook output and uses saved overlay through real opts.scope.projectRoot, ], { - env: { - ...process.env, - HACK_RUNTIME_BACKEND: "native", - HACK_NATIVE_HOME: opts.scope.nativeHome, - HACK_NATIVE_BINARY: binary, - }, + env: privateCliEnv({ + root: opts.scope.projectRoot, + env: { + HACK_RUNTIME_BACKEND: "native", + HACK_NATIVE_HOME: opts.scope.nativeHome, + HACK_NATIVE_BINARY: binary, + }, + }), stdout: "pipe", stderr: "pipe", } diff --git a/tests/native-project-exec.test.ts b/tests/native-project-exec.test.ts index 1f53b8407..17808bda3 100644 --- a/tests/native-project-exec.test.ts +++ b/tests/native-project-exec.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { nativeProjectExec } from "../src/backends/native-project-exec.ts"; import { saveNativeProjectRun } from "../src/backends/native-project-run.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const roots: string[] = []; afterEach(async () => { @@ -194,16 +195,22 @@ test("foreign receipt refuses before exec and changed container after exec is un }); test("native exec rejects env and profile before project lookup or runtime effects", async () => { + const root = await realpath( + await mkdtemp(join(tmpdir(), "native-exec-flags-")) + ); + roots.push(root); for (const flag of ["--env", "--profile"]) { const child = Bun.spawn( [process.execPath, "index.ts", "exec", flag, "qa", "web", "true"], { - env: { - ...process.env, - HACK_RUNTIME_BACKEND: "native", - HACK_NATIVE_HOME: "/nonexistent-native-home", - HACK_NATIVE_BINARY: "/nonexistent-native-binary", - }, + env: privateCliEnv({ + root, + env: { + HACK_RUNTIME_BACKEND: "native", + HACK_NATIVE_HOME: "/nonexistent-native-home", + HACK_NATIVE_BINARY: "/nonexistent-native-binary", + }, + }), stdout: "pipe", stderr: "pipe", } diff --git a/tests/native-routing-project.test.ts b/tests/native-routing-project.test.ts index e8abe1534..4876ccfc2 100644 --- a/tests/native-routing-project.test.ts +++ b/tests/native-routing-project.test.ts @@ -15,6 +15,7 @@ import { validateNativeProject, } from "../src/lib/native-project-validation.ts"; import { restoreEnv } from "./helpers/env.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const PROTOCOL = { transport_version: 1, @@ -224,7 +225,15 @@ async function cli(args: readonly string[]) { ], { cwd: projectRoot, - env: { ...process.env, HACK_LOGGER: "console" }, + // HACK_HOME and the compiler are this fixture's in-process settings. + env: privateCliEnv({ + root, + env: { + HACK_HOME: process.env.HACK_HOME, + HACK_CONFIG_COMPILER_BINARY: process.env.HACK_CONFIG_COMPILER_BINARY, + HACK_LOGGER: "console", + }, + }), stdin: "ignore", stdout: "pipe", stderr: "pipe", diff --git a/tests/prerelease-plan.test.ts b/tests/prerelease-plan.test.ts index 690f176dd..72527fc5c 100644 --- a/tests/prerelease-plan.test.ts +++ b/tests/prerelease-plan.test.ts @@ -37,6 +37,7 @@ import { } from "../src/backends/native-authored-storage-tool.ts"; import { packageMcpBundle } from "../src/mcp/bundle.ts"; import { nativeCandidateMcpPayload } from "../src/mcp/candidate-payload.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const sha = "a".repeat(40); const input = { @@ -1172,11 +1173,13 @@ test("compiled CLI reports the embedded candidate version and ordinary source re [process.execPath, "index.ts", "--version"], ]) { const child = Bun.spawn(args, { - env: { - ...process.env, - HACK_BUILD_VERSION: "wrong", - __HACK_BUILD_VERSION__: "wrong", - }, + env: privateCliEnv({ + root, + env: { + HACK_BUILD_VERSION: "wrong", + __HACK_BUILD_VERSION__: "wrong", + }, + }), stdout: "pipe", stderr: "pipe", }); diff --git a/tests/project-env-local-inheritance.test.ts b/tests/project-env-local-inheritance.test.ts index 5bb92c339..27bf70d18 100644 --- a/tests/project-env-local-inheritance.test.ts +++ b/tests/project-env-local-inheritance.test.ts @@ -17,6 +17,7 @@ import { unsetProjectEnvValue, } from "../src/lib/project-env-config.ts"; import { restoreEnv } from "./helpers/env.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const roots: string[] = []; const originalCI = process.env.CI; @@ -97,7 +98,7 @@ async function fixture() { envName: "qa", serviceNames: ["api"], }); - return { primary, primaryRoot, linkedRoot, linked, resolve }; + return { root, primary, primaryRoot, linkedRoot, linked, resolve }; } test("linked env layers observe primary updates without copying or inheriting primary tracked state", async () => { @@ -283,7 +284,7 @@ test("actual CLI masks inherited global secret overriding tracked service plaint ], { cwd: process.cwd(), - env: { ...process.env, HACK_LOGGER: "console" }, + env: privateCliEnv({ root: f.root, env: { HACK_LOGGER: "console" } }), stdout: "pipe", stderr: "pipe", } diff --git a/tests/project-env-worktree-key.test.ts b/tests/project-env-worktree-key.test.ts index 00cf7fa34..1fa6774af 100644 --- a/tests/project-env-worktree-key.test.ts +++ b/tests/project-env-worktree-key.test.ts @@ -12,7 +12,6 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; - import { PROJECT_COMPOSE_FILENAME, PROJECT_CONFIG_FILENAME, @@ -26,6 +25,7 @@ import { resolveProjectEnvSharedKeyLocation, setProjectEnvValue, } from "../src/lib/project-env-config.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const tempDirs = new Set(); const restorePermissions: Array<() => Promise> = []; @@ -269,13 +269,15 @@ async function runHackCli(opts: { }): Promise { const home = resolve(opts.sandbox, "home"); await mkdir(home, { recursive: true }); - const env: Record = { - ...process.env, - HOME: home, - HACK_GLOBAL_CONFIG_PATH: resolve(home, "hack.config.json"), - HACK_LOGGER: "console", - }; - Reflect.deleteProperty(env, "HACK_ENV_SECRET_KEY"); + // No runner variable such as HACK_ENV_SECRET_KEY reaches the child. + const env = privateCliEnv({ + root: opts.sandbox, + home, + env: { + HACK_GLOBAL_CONFIG_PATH: resolve(home, "hack.config.json"), + HACK_LOGGER: "console", + }, + }); const proc = Bun.spawn( ["bun", resolve(import.meta.dir, "../index.ts"), ...opts.args], { From 9b90167026bb8f0a07c0c31329c300137b02492e Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Mon, 12 Oct 2026 19:58:30 -0400 Subject: [PATCH 2/3] test: close remaining runner-env paths to the source CLI Migrate the CLI spawns the first pass missed: the project-name fixture env (used by project-name-command and the local-network-hint tests), dispatch-e2e, the MCP contract server env, and the spawns that passed no env at all (doctor-browser-network, log-pipe, mcp-entrypoint and the host-exec-tty Python probes). Rebuild the guard on the TypeScript syntax tree. It now follows a CLI file's test-local imports and flags any whole copy of the runner env (spread, direct assignment, Bun.env, Object.assign, rest destructuring), a HOME derived from the runner's home, a CLI spawn without env, and an env literal without HOME. Its limits are documented in the scan module. privateCliEnv now sets mode 0700 on a HOME that already exists, and native-project-down keeps the private HOME out of the project checkout. Co-Authored-By: Claude Opus 5.5 --- tests/cli-spawn-env-isolation.test.ts | 337 +++++++---- tests/dispatch-e2e.test.ts | 34 +- tests/doctor-browser-network.test.ts | 52 +- tests/helpers/cli-spawn-env-scan.ts | 634 +++++++++++++++++++++ tests/helpers/private-cli-env.ts | 7 +- tests/helpers/project-name-fixture.ts | 22 +- tests/host-exec-tty.test.ts | 58 +- tests/log-pipe.test.ts | 80 +-- tests/mcp-entrypoint.test.ts | 55 +- tests/mcp.test.ts | 19 +- tests/native-config-import-preview.test.ts | 1 - tests/native-project-down.test.ts | 13 +- tests/project-env-worktree-key.test.ts | 2 +- 13 files changed, 1077 insertions(+), 237 deletions(-) create mode 100644 tests/helpers/cli-spawn-env-scan.ts diff --git a/tests/cli-spawn-env-isolation.test.ts b/tests/cli-spawn-env-isolation.test.ts index 61965b475..314900ee6 100644 --- a/tests/cli-spawn-env-isolation.test.ts +++ b/tests/cli-spawn-env-isolation.test.ts @@ -1,7 +1,12 @@ import { expect, test } from "bun:test"; -import { mkdtemp, readFile, rm, stat } from "node:fs/promises"; +import { chmod, mkdir, mkdtemp, readFile, rm, stat } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, relative, resolve } from "node:path"; +import { + describeFindings, + scanSource, + scanSources, +} from "./helpers/cli-spawn-env-scan.ts"; import { restoreEnv } from "./helpers/env.ts"; import { PRIVATE_CLI_INHERITED_ENV, @@ -9,15 +14,11 @@ import { } from "./helpers/private-cli-env.ts"; /** - * Guard: a test that spawns the source CLI must not hand it the test runner's - * environment with the contributor's HOME. Build the child env with - * `privateCliEnv` (tests/helpers/private-cli-env.ts), or at least override - * HOME with a path inside the fixture. - * - * This is a text scan, not a type check. A file "spawns the source CLI" when - * it names `index.ts` in a string literal or uses the E2E harness CLI - * entrypoint. In such a file, every object literal that spreads `process.env` - * must set its own `HOME`, and that `HOME` must not be the runner's home. + * Guard: no test hands the source CLI the test runner's environment or HOME. + * Build the child env with `privateCliEnv` (tests/helpers/private-cli-env.ts). + * The scope, rules and limits of the scan are documented in + * tests/helpers/cli-spawn-env-scan.ts. Each exception below names its reason + * and its exact finding count. */ const ALLOWED: Readonly< Record @@ -28,105 +29,57 @@ const ALLOWED: Readonly< reason: "runCommand spawns git/docker clients for real-engine E2E", count: 1, }, - // Release planning scripts (`scripts/prerelease-plan.ts`) read only - // GITHUB_* and release inputs; the source CLI spawn in this file is private. + // Two env objects for scripts/prerelease-plan.ts, which reads only GITHUB_* + // and release inputs; one `tar` control env (rest destructuring); and one + // `bun build index.ts --compile`, which compiles and does not run the CLI. + // The compiled and source CLI spawns in this file use `privateCliEnv`. "tests/prerelease-plan.test.ts": { - reason: "env objects for scripts/prerelease-plan.ts, not the CLI", - count: 2, + reason: "release-plan script envs, a tar env and a compile, not the CLI", + count: 4, + }, + // The fixture TLS probe runs the real-engine `docker` client, which needs + // the runner's Docker selection. It does not run the CLI. + "tests/e2e/scenarios/native-routing-fixture-ingress.ts": { + reason: "real-engine docker TLS probe env, not the CLI", + count: 1, + }, + // `bun build index.ts --compile` compiles and does not run the CLI. + "tests/host-exec-tty.test.ts": { + reason: "bun build --compile of index.ts, which does not run the CLI", + count: 1, }, }; -const SOURCE_CLI = - /["'`](?:\.{1,2}\/)*index\.ts["'`]|\bCLI_ENTRYPOINT\b|\bresolveCliSpawnArgs\b/; -const SPREAD = /\.\.\.\s*process\.env(?![\w.[])/g; -const RUNNER_HOME = - /\bHOME\s*:\s*(?:process\.env\.HOME|process\.env\[["']HOME["']\]|homedir\(\))/; -const HOME_KEY = /(?:^|[\s,{])HOME\s*(?::|,|\})/; - -/** The text of the object literal that encloses `index`, or null. */ -function enclosingObject(source: string, index: number): string | null { - let depth = 0; - let start = -1; - for (let at = index - 1; at >= 0; at -= 1) { - const char = source[at]; - if (char === "}") { - depth += 1; - } else if (char === "{") { - if (depth === 0) { - start = at; - break; - } - depth -= 1; - } - } - if (start < 0) { - return null; - } - depth = 0; - for (let at = start; at < source.length; at += 1) { - const char = source[at]; - if (char === "{") { - depth += 1; - } else if (char === "}") { - depth -= 1; - if (depth === 0) { - return source.slice(start, at + 1); - } - } - } - return null; -} - -/** Line numbers of `...process.env` spreads that reach the source CLI with the runner's HOME. */ -function leakingCliEnvSpreads(source: string): number[] { - if (!SOURCE_CLI.test(source)) { - return []; - } - const lines: number[] = []; - for (const match of source.matchAll(SPREAD)) { - const object = enclosingObject(source, match.index); - const safe = - object !== null && HOME_KEY.test(object) && !RUNNER_HOME.test(object); - if (!safe) { - lines.push(source.slice(0, match.index).split("\n").length); - } - } - return lines; -} - -async function testSources(): Promise { +async function testSources(): Promise> { const root = resolve(import.meta.dir); - const files: string[] = []; + const sources = new Map(); for await (const path of new Bun.Glob("**/*.{ts,tsx,mts}").scan({ cwd: root, absolute: true, })) { // This file names the patterns it scans for in its own examples. if (!(path.includes("/node_modules/") || path === import.meta.path)) { - files.push(path); + sources.set(path, await readFile(path, "utf8")); } } - return files.sort(); + return sources; } -test("no test spawns the source CLI with the runner's environment and HOME", async () => { +test("no test spawns the source CLI with the runner's environment or HOME", async () => { const repo = resolve(import.meta.dir, ".."); const violations: string[] = []; const allowedSeen = new Map(); - for (const path of await testSources()) { + const results = scanSources(await testSources(), resolve(import.meta.dir)); + for (const [path, findings] of results) { const name = relative(repo, path); - const lines = leakingCliEnvSpreads(await readFile(path, "utf8")); - if (lines.length === 0) { - continue; - } const allowed = ALLOWED[name]; if (allowed) { - allowedSeen.set(name, lines.length); - if (lines.length <= allowed.count) { + allowedSeen.set(name, findings.length); + if (findings.length <= allowed.count) { continue; } } - violations.push(`${name}:${lines.join(",")}`); + violations.push(`${name}: ${describeFindings(findings)}`); } expect(violations).toEqual([]); // Each exception still exists; remove stale entries rather than keep them. @@ -138,35 +91,207 @@ test("no test spawns the source CLI with the runner's environment and HOME", asy } }); -test("the scan flags runner HOME and accepts a private HOME", () => { - const spawn = (env: string) => - `Bun.spawn([process.execPath, "../index.ts"], {\n env: ${env},\n});`; +/** Findings for one synthetic in-scope file, as `line rule` strings. */ +function scanExample(...lines: string[]): string[] { + return scanSource("/virtual/example.test.ts", lines.join("\n")).map( + ({ line, rule }) => `${line} ${rule}` + ); +} + +const spawnWith = (options: string) => + `Bun.spawn([process.execPath, "../index.ts"], ${options});`; + +test("the scan flags every way to hand the CLI the runner's environment", () => { + // Whole-environment copies, in any form. + expect(scanExample(spawnWith("{ env: process.env }"))).toEqual([ + "1 runner-env", + ]); + expect( + scanExample(spawnWith("{ env: { ...process.env, HOME: h } }")) + ).toEqual(["1 runner-env"]); + expect(scanExample(spawnWith("{ env: { ...Bun.env, HOME: h } }"))).toEqual([ + "1 runner-env", + ]); + expect( + scanExample( + spawnWith("{ env: Object.assign({}, process.env, { HOME: h }) }") + ) + ).toEqual(["1 runner-env"]); + expect( + scanExample( + 'import { env as e } from "node:process";', + spawnWith("{ env: { ...e, HOME: h } }") + ) + ).toEqual(["2 runner-env"]); + expect( + scanExample( + "const { A: _a, ...rest } = process.env;", + spawnWith("{ env: { ...rest } }") + ) + ).toEqual(["1 runner-env"]); + expect( + scanExample( + "const saved = process.env;", + spawnWith("{ env: { ...saved } }") + ) + ).toEqual(["1 runner-env"]); + // The spread is flagged whatever HOME is set to. + expect( + scanExample(spawnWith("{ env: { ...process.env, HOME: someVar } }")) + ).toEqual(["1 runner-env"]); + // The runner's HOME, directly or through a same-file binding. + expect( + scanExample(spawnWith("{ env: { ...process.env, HOME: os.homedir() } }")) + ).toEqual(["1 runner-env", "1 runner-home"]); + expect( + scanExample(spawnWith("{ env: { PATH: p, HOME: homedir() } }")) + ).toEqual(["1 runner-home"]); + expect( + scanExample(spawnWith("{ env: { PATH: p, HOME: process.env.HOME } }")) + ).toEqual(["1 runner-home"]); + expect( + scanExample(spawnWith('{ env: { PATH: p, HOME: Bun.env["HOME"] } }')) + ).toEqual(["1 runner-home"]); + expect( + scanExample( + "const someVar = join(homedir(), 'x');", + spawnWith("{ env: { PATH: p, HOME: someVar } }") + ) + ).toEqual(["2 runner-home"]); expect( - leakingCliEnvSpreads(spawn("{ ...process.env, HACK_HOME: h }")) - ).toEqual([2]); + scanExample( + "const HOME = userInfo().homedir;", + spawnWith("{ env: { PATH: p, HOME } }") + ) + ).toEqual(["2 runner-home"]); + // A CLI spawn without env inherits everything; one without HOME falls back + // to the account's real home. + expect(scanExample(spawnWith("{ stdout: 'pipe' }"))).toEqual([ + "1 cli-without-env", + ]); + expect(scanExample('Bun.spawn(["bun", "index.ts", "log-pipe"]);')).toEqual([ + "1 cli-without-env", + ]); expect( - leakingCliEnvSpreads(spawn("{ ...process.env, HOME: process.env.HOME }")) - ).toEqual([2]); + scanExample( + 'import { spawnSync as run } from "node:child_process";', + 'const cli = resolve(dir, "../index.ts");', + "const argv = [process.execPath, cli];", + "run(argv[0], argv.slice(1), { cwd });" + ) + ).toEqual(["4 cli-without-env"]); expect( - leakingCliEnvSpreads(spawn("{ ...process.env, HOME: homedir() }")) - ).toEqual([2]); - // A nested object's HOME does not cover the outer spread. + scanExample( + 'import * as cp from "child_process";', + 'cp.execFile(process.execPath, ["index.ts"], { cwd });' + ) + ).toEqual(["2 cli-without-env"]); expect( - leakingCliEnvSpreads(spawn("{ ...process.env, X: { HOME: h } }")) + scanExample( + 'Bun.spawn(["python3", "probe.py", process.execPath, "../index.ts"], { stdin: "ignore" });' + ) + ).toEqual(["1 cli-without-env"]); + expect(scanExample(spawnWith("{ env: { PATH: p } }"))).toEqual([ + "1 env-without-home", + ]); + expect( + scanExample( + "const env = { PATH: p };", + spawnWith("{ env, stdout: 'pipe' }") + ) + ).toEqual(["2 env-without-home"]); +}); + +test("the scan accepts a private env and one-key uses of process.env", () => { + expect(scanExample(spawnWith("{ env: privateCliEnv({ root }) }"))).toEqual( + [] + ); + expect( + scanExample(spawnWith('{ env: { PATH: p, HOME: join(root, "home") } }')) + ).toEqual([]); + // A fixture field named homedir is not the runner's home. + expect( + scanExample(spawnWith("{ env: { PATH: p, HOME: fixture.homedir } }")) ).toEqual([]); expect( - leakingCliEnvSpreads(spawn('{ ...process.env, HOME: join(root, "home") }')) + scanExample( + "const env = privateCliEnv({ root });", + spawnWith("{ env, cwd }") + ) ).toEqual([]); - expect(leakingCliEnvSpreads(spawn("{ ...process.env, HOME }"))).toEqual([]); - expect(leakingCliEnvSpreads(spawn("privateCliEnv({ root })"))).toEqual([]); - // Reading one variable is not a spread of the environment. + // Another module's env object is trusted here and checked where it is built. + expect(scanExample(spawnWith("{ env: { ...fixture.env, X: '1' } }"))).toEqual( + [] + ); + // An opaque options value is trusted; its builder is checked separately. + expect(scanExample(spawnWith("options"))).toEqual([]); expect( - leakingCliEnvSpreads(spawn("{ PATH: `${process.env.PATH}` }")) + scanExample( + "const home = process.env.HOME;", + 'Reflect.deleteProperty(process.env, "HACK_HOME");', + 'if ("CI" in process.env && Object.hasOwn(process.env, "X")) {}', + "process.env = { PATH: p };", + "const { PATH } = process.env;", + "restoreEnv('HOME', home);", + "const p = `${process.env.PATH}`;" + ) ).toEqual([]); - // Files that never spawn the source CLI are out of scope. + // A spawn that does not reach the CLI may inherit, including one that uses + // the result of a CLI spawn. + expect(scanExample('Bun.spawn(["git", "status"], { cwd });')).toEqual([]); expect( - leakingCliEnvSpreads('Bun.spawn(["git"], { env: { ...process.env } });') + scanExample( + 'const proc = Bun.spawn([process.execPath, "index.ts"], { env: privateCliEnv({ root }) });', + 'Bun.spawnSync(["ps", "-p", `${proc.pid}`]);', + 'const run = (args) => { const argv = [process.execPath, "index.ts", ...args]; return Bun.spawn(argv, { env }); };', + 'const result = await run(["up"]);', + 'Bun.spawnSync(["git", "-C", result.root]);' + ) ).toEqual([]); + // A local function named like a spawn export is not one. + expect( + scanExample( + "const exec = (a) => a;", + 'exec([process.execPath, "index.ts"], { cwd });' + ) + ).toEqual([]); +}); + +test("the scan follows a CLI file's local imports and skips other files", () => { + const root = "/virtual/tests"; + const sources = new Map([ + [ + `${root}/name.test.ts`, + [ + 'import { fixture } from "./helpers/fixture.ts";', + 'Bun.spawn([process.execPath, "../index.ts"], { env: fixture.env });', + ].join("\n"), + ], + [ + `${root}/helpers/fixture.ts`, + [ + 'import { other } from "@tests/helpers/other";', + "export const fixture = { env: { ...process.env, HOME: root } };", + ].join("\n"), + ], + [ + `${root}/helpers/other.ts`, + "export const other = Object.assign({}, process.env);", + ], + [`${root}/git.test.ts`, 'Bun.spawn(["git"], { env: { ...process.env } });'], + ]); + const results = scanSources(sources, root); + expect( + Object.fromEntries( + [...results].map(([path, findings]) => [ + relative(root, path), + describeFindings(findings), + ]) + ) + ).toEqual({ + "helpers/fixture.ts": "2 runner-env", + "helpers/other.ts": "1 runner-env", + }); }); test("privateCliEnv keeps only the allow-list, test values and a private HOME", async () => { @@ -204,6 +329,12 @@ test("privateCliEnv keeps only the allow-list, test values and a private HOME", for (const key of Object.keys(inherited)) { expect([...PRIVATE_CLI_INHERITED_ENV, "HOME"]).toContain(key); } + // A HOME the test created first is still made private to the owner. + const existing = join(root, "existing-home"); + await mkdir(existing); + await chmod(existing, 0o755); + expect(privateCliEnv({ root, home: existing }).HOME).toBe(existing); + expect((await stat(existing)).mode & 0o777).toBe(0o700); expect(() => privateCliEnv({ root, home: tmpdir() })).toThrow("inside"); expect(() => privateCliEnv({ root, env: { HOME: root } })).toThrow( "home option" diff --git a/tests/dispatch-e2e.test.ts b/tests/dispatch-e2e.test.ts index 05de7c77b..5a81e855c 100644 --- a/tests/dispatch-e2e.test.ts +++ b/tests/dispatch-e2e.test.ts @@ -9,6 +9,7 @@ import { import { findProjectContext } from "../src/lib/project.ts"; import { upsertProjectRegistration } from "../src/lib/projects-registry.ts"; import { restoreEnv } from "./helpers/env.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const shouldRun = process.env.HACK_DISPATCH_E2E === "1"; const runTest = shouldRun ? test : test.skip; @@ -138,13 +139,11 @@ runTest( const result = await runCommand({ cmd: command, cwd: resolve(import.meta.dir, ".."), - env: { - ...process.env, - HOME: tempHome, + env: childEnv({ HACK_GLOBAL_CONFIG_PATH: "", [nodeTokenEnv]: nodeToken, NO_COLOR: "1", - }, + }), }); if (result.exitCode !== 0) { throw new Error( @@ -301,15 +300,13 @@ runTest( const result = await runCommand({ cmd: command, cwd: resolve(import.meta.dir, ".."), - env: { - ...process.env, - HOME: tempHome, + env: childEnv({ HACK_GLOBAL_CONFIG_PATH: "", [nodeTokenEnv]: nodeToken, PATH: `${mutagenBinDir}:${process.env.PATH ?? ""}`, HACK_TEST_MUTAGEN_LOG_PATH: mutagenLogPath, NO_COLOR: "1", - }, + }), }); if (result.exitCode !== 0) { throw new Error( @@ -421,13 +418,11 @@ runTest( const result = await runCommand({ cmd: command, cwd: resolve(import.meta.dir, ".."), - env: { - ...process.env, - HOME: tempHome, + env: childEnv({ HACK_GLOBAL_CONFIG_PATH: "", [nodeTokenEnv]: nodeToken, NO_COLOR: "1", - }, + }), }); expect(result.exitCode).toBe(1); expect(result.stderr).toContain("Workspace ensure failed (500)"); @@ -506,6 +501,19 @@ async function initializeGitRepo(input: { }); } +/** + * A private env for a child of the current test: HOME is the test's temporary + * home, and no other runner variable is inherited beyond PATH and TMPDIR. + */ +function childEnv( + env: Readonly> +): Record { + if (tempHome === null) { + throw new Error("dispatch E2E child env requires a temporary HOME"); + } + return privateCliEnv({ root: tempHome, home: tempHome, env }); +} + async function runGit(input: { readonly cwd: string; readonly args: readonly string[]; @@ -513,7 +521,7 @@ async function runGit(input: { const result = await runCommand({ cmd: ["git", ...input.args], cwd: input.cwd, - env: process.env, + env: childEnv({}), }); if (result.exitCode !== 0) { throw new Error( diff --git a/tests/doctor-browser-network.test.ts b/tests/doctor-browser-network.test.ts index 0c1ce4c62..b15370abd 100644 --- a/tests/doctor-browser-network.test.ts +++ b/tests/doctor-browser-network.test.ts @@ -1,10 +1,14 @@ import { expect, test } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { buildDoctorJsonData } from "../src/commands/doctor.ts"; import { checkBrowserLocalNetwork, parseBrowserNetworkOptions, probeBrowserTarget, } from "../src/lib/doctor-browser-network.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const url = "https://app.hack/"; @@ -152,25 +156,35 @@ test("curl invocation validates TLS, disables config, and bounds transfer withou }); test("invalid browser arguments fail before Doctor invokes tools", async () => { - const child = Bun.spawn( - [ - process.execPath, - "index.ts", - "doctor", - "--browser-result", - "works", - "--json", - ], - { stdout: "pipe", stderr: "pipe", timeout: 5000 } - ); - const [stdout, stderr, code] = await Promise.all([ - new Response(child.stdout).text(), - new Response(child.stderr).text(), - child.exited, - ]); - expect(code).not.toBe(0); - expect(stdout).not.toContain("docker"); - expect(stdout + stderr).toContain("requires --browser-url"); + const root = await mkdtemp(join(tmpdir(), "hack-doctor-browser-")); + try { + const child = Bun.spawn( + [ + process.execPath, + "index.ts", + "doctor", + "--browser-result", + "works", + "--json", + ], + { + env: privateCliEnv({ root }), + stdout: "pipe", + stderr: "pipe", + timeout: 5000, + } + ); + const [stdout, stderr, code] = await Promise.all([ + new Response(child.stdout).text(), + new Response(child.stderr).text(), + child.exited, + ]); + expect(code).not.toBe(0); + expect(stdout).not.toContain("docker"); + expect(stdout + stderr).toContain("requires --browser-url"); + } finally { + await rm(root, { recursive: true, force: true }); + } }); test("JSON and summary retain the unknown browser warning separately", async () => { diff --git a/tests/helpers/cli-spawn-env-scan.ts b/tests/helpers/cli-spawn-env-scan.ts new file mode 100644 index 000000000..c18b70d6b --- /dev/null +++ b/tests/helpers/cli-spawn-env-scan.ts @@ -0,0 +1,634 @@ +import { dirname, join, resolve } from "node:path"; +import ts from "typescript"; + +/** + * Syntax scan behind tests/cli-spawn-env-isolation.test.ts: finds tests that + * hand the source CLI the test runner's environment or HOME. + * + * Scope. A file is in scope when it names the source CLI (a string that ends + * in `index.ts`, `CLI_ENTRYPOINT` or `resolveCliSpawnArgs`). Every test-local + * module that such a file imports, directly or transitively, is in scope too, + * so an env object that a fixture helper builds for the CLI is checked where + * it is built. + * + * Rules, checked on the TypeScript syntax tree of each in-scope file: + * - `runner-env`: no copy of the whole runner environment. A use of + * `process.env`, `Bun.env`, `import.meta.env` or a `node:process` `env` + * import is a copy unless it reads, deletes or tests one key, or replaces + * the whole object. Spreads, `env: process.env`, `Object.assign(x, + * process.env)`, `const saved = process.env` and rest destructuring are + * all copies. + * - `runner-home`: no `HOME` property whose value derives from the runner's + * home: `process.env.HOME`, `homedir()` in any form, `userInfo().homedir`, + * or a binding in the same file that holds one of these. + * - `cli-without-env`: a spawn call (`spawn`, `spawnSync`, `execFile`, + * `exec`, `fork` and their `Bun.`/`child_process.` forms) whose arguments + * reach the CLI must pass an `env` option. Without it the child inherits the + * whole runner environment, including HOME. A wrapper such as `python3 + * probe.py bun index.ts` counts, because its argv names the CLI. + * - `env-without-home`: when that `env` is an object literal, it must set + * HOME or spread another object. Without HOME the child resolves its home + * from the account database, which is the runner's real home. + * + * Limits. This is a syntax check within one file, not data flow. It trusts an + * env, HOME or argv value that comes from another module or a parameter: for + * example `HOME: opts.home`, `env: options.env` in a shared runner, or a local + * `run(argv)` wrapper whose spawn never names the CLI itself. A spawn of a CLI + * path built at runtime without naming `index.ts` (for example a compiled + * binary) is not seen either. Those spawns still need a private env by + * convention; prefer one shared runner that calls `privateCliEnv`. + */ + +export type Rule = + | "runner-env" + | "runner-home" + | "cli-without-env" + | "env-without-home"; + +export type Finding = { readonly line: number; readonly rule: Rule }; + +const SOURCE_CLI_PATH = /(?:^|\/)index\.ts$/; +const SOURCE_CLI_NAMES = new Set(["CLI_ENTRYPOINT", "resolveCliSpawnArgs"]); +/** Process-spawning exports, by module. `Bun.spawn`/`Bun.spawnSync` are global. */ +const SPAWN_MODULES: Readonly>> = { + child_process: new Set([ + "spawn", + "spawnSync", + "execFile", + "execFileSync", + "exec", + "execSync", + "fork", + ]), + execa: new Set(["execa", "execaSync"]), +}; +const ENV_OWNERS = new Set([ + "process", + "Bun", + "import.meta", + "globalThis.process", + "globalThis.Bun", +]); +const KEY_ACCESSORS = new Set([ + "Reflect.deleteProperty", + "Reflect.has", + "Object.hasOwn", +]); + +function parse(path: string, source: string): ts.SourceFile { + return ts.createSourceFile(path, source, ts.ScriptTarget.Latest, true); +} + +/** Visits `node` and its descendants; `visit` returning false skips a subtree. */ +function walk(node: ts.Node, visit: (node: ts.Node) => unknown): void { + if (visit(node) === false) { + return; + } + ts.forEachChild(node, (child) => walk(child, visit)); +} + +function moduleName(specifier: string): string { + return specifier.replace(/^node:/, ""); +} + +/** How this file can spawn a process: imported functions and namespaces. */ +type SpawnNames = { + readonly functions: ReadonlySet; + readonly namespaces: ReadonlyMap>; +}; + +/** Local names that one import declaration binds. */ +type ImportNames = { + /** Module specifier without a `node:` prefix. */ + readonly module: string; + /** Default and namespace import names. */ + readonly whole: readonly string[]; + /** Named imports: the exported name and its local name. */ + readonly named: readonly { + readonly imported: string; + readonly local: string; + }[]; +}; + +function importNames(file: ts.SourceFile): ImportNames[] { + const imports: ImportNames[] = []; + for (const statement of file.statements) { + if ( + !( + ts.isImportDeclaration(statement) && + ts.isStringLiteral(statement.moduleSpecifier) && + statement.importClause + ) + ) { + continue; + } + const clause = statement.importClause; + const whole = clause.name ? [clause.name.text] : []; + const named: { imported: string; local: string }[] = []; + const bindings = clause.namedBindings; + if (bindings && ts.isNamespaceImport(bindings)) { + whole.push(bindings.name.text); + } else if (bindings) { + for (const element of bindings.elements) { + named.push({ + imported: (element.propertyName ?? element.name).text, + local: element.name.text, + }); + } + } + imports.push({ + module: moduleName(statement.moduleSpecifier.text), + whole, + named, + }); + } + return imports; +} + +function spawnImports(file: ts.SourceFile): SpawnNames { + const functions = new Set(); + const namespaces = new Map>([ + ["Bun", new Set(["spawn", "spawnSync"])], + ]); + for (const entry of importNames(file)) { + const exports = SPAWN_MODULES[entry.module]; + if (!exports) { + continue; + } + for (const name of entry.whole) { + namespaces.set(name, exports); + } + for (const { imported, local } of entry.named) { + if (exports.has(imported)) { + functions.add(local); + } + } + } + return { functions, namespaces }; +} + +function isSpawnCall( + node: ts.Node, + spawn: SpawnNames +): node is ts.CallExpression { + if (!ts.isCallExpression(node)) { + return false; + } + const callee = unwrap(node.expression); + if (ts.isIdentifier(callee)) { + return spawn.functions.has(callee.text); + } + return ( + ts.isPropertyAccessExpression(callee) && + ts.isIdentifier(callee.expression) && + (spawn.namespaces.get(callee.expression.text)?.has(callee.name.text) ?? + false) + ); +} + +/** The called name: `f` for `f()` and `x.f()`. */ +function calleeName(call: ts.CallExpression): string | null { + const callee = unwrap(call.expression); + if (ts.isIdentifier(callee)) { + return callee.text; + } + return ts.isPropertyAccessExpression(callee) ? callee.name.text : null; +} + +function isFunctionLike(node: ts.Node): boolean { + return ( + ts.isArrowFunction(node) || + ts.isFunctionExpression(node) || + ts.isFunctionDeclaration(node) || + ts.isMethodDeclaration(node) + ); +} + +function unwrap(node: ts.Expression): ts.Expression { + let current = node; + while ( + ts.isParenthesizedExpression(current) || + ts.isAsExpression(current) || + ts.isNonNullExpression(current) || + ts.isSatisfiesExpression(current) || + ts.isTypeAssertionExpression(current) + ) { + current = current.expression; + } + return current; +} + +function propertyName(name: ts.PropertyName | undefined): string | null { + if (!name) { + return null; + } + if (ts.isIdentifier(name) || ts.isStringLiteralLike(name)) { + return name.text; + } + return null; +} + +/** Local names that `node:process` binds: the module itself and its `env`. */ +function processImports(file: ts.SourceFile): { + readonly owners: Set; + readonly envs: Set; +} { + const owners = new Set(ENV_OWNERS); + const envs = new Set(); + for (const entry of importNames(file)) { + if (entry.module !== "process") { + continue; + } + for (const name of entry.whole) { + owners.add(name); + } + for (const { imported, local } of entry.named) { + if (imported === "env") { + envs.add(local); + } + } + } + return { owners, envs }; +} + +/** Initializers of same-file `const`/`let`/`var` bindings. */ +function bindings(file: ts.SourceFile): Map { + const values = new Map(); + walk(file, (node) => { + if ( + ts.isVariableDeclaration(node) && + ts.isIdentifier(node.name) && + node.initializer + ) { + values.set(node.name.text, node.initializer); + } + }); + return values; +} + +/** Names whose same-file value satisfies `matches`, closed over aliases. */ +function taintedNames( + values: ReadonlyMap, + matches: (node: ts.Node, names: ReadonlySet) => boolean +): Set { + const names = new Set(); + let grew = true; + while (grew) { + grew = false; + for (const [name, value] of values) { + if (!names.has(name) && matches(value, names)) { + names.add(name); + grew = true; + } + } + } + return names; +} + +function isReference(node: ts.Identifier): boolean { + const parent = node.parent; + return !( + (ts.isPropertyAccessExpression(parent) && parent.name === node) || + (ts.isPropertyAssignment(parent) && parent.name === node) || + ts.isImportSpecifier(parent) || + (ts.isVariableDeclaration(parent) && parent.name === node) || + (ts.isBindingElement(parent) && parent.name === node) + ); +} + +/** + * Whether a subtree names the source CLI, directly or through `names`. + * `prune` skips subtrees, such as function bodies and spawn calls, whose CLI + * reference does not make the enclosing value a CLI path. + */ +function namesCli( + node: ts.Node, + names: ReadonlySet, + prune: (node: ts.Node) => boolean = () => false +): boolean { + let found = false; + walk(node, (child) => { + if (found || prune(child)) { + return false; + } + if ( + (ts.isStringLiteralLike(child) || + ts.isTemplateHead(child) || + ts.isTemplateMiddle(child) || + ts.isTemplateTail(child)) && + SOURCE_CLI_PATH.test(child.text) + ) { + found = true; + } else if ( + ts.isIdentifier(child) && + (SOURCE_CLI_NAMES.has(child.text) || names.has(child.text)) && + isReference(child) + ) { + found = true; + } + }); + return found; +} + +type EnvNames = ReturnType; + +/** `process.env` or an alias of it, as a whole object. */ +function isWholeEnv(node: ts.Node, env: EnvNames): boolean { + if (ts.isPropertyAccessExpression(node) && node.name.text === "env") { + return env.owners.has(node.expression.getText()); + } + return ts.isIdentifier(node) && env.envs.has(node.text) && isReference(node); +} + +/** Whether this use of the whole environment only touches one key or replaces it. */ +function isKeyUse(node: ts.Node): boolean { + const parent = node.parent; + if ( + (ts.isPropertyAccessExpression(parent) || + ts.isElementAccessExpression(parent)) && + parent.expression === node + ) { + return true; + } + if ( + ts.isCallExpression(parent) && + parent.arguments[0] === node && + KEY_ACCESSORS.has(parent.expression.getText()) + ) { + return true; + } + if (ts.isBinaryExpression(parent)) { + const operator = parent.operatorToken.kind; + return ( + (operator === ts.SyntaxKind.InKeyword && parent.right === node) || + (operator === ts.SyntaxKind.EqualsToken && parent.left === node) + ); + } + return ( + ts.isVariableDeclaration(parent) && + ts.isObjectBindingPattern(parent.name) && + parent.name.elements.every((element) => !element.dotDotDotToken) + ); +} + +/** Whether a subtree reads the runner's home, directly or through `names`. */ +function readsRunnerHome( + node: ts.Node, + env: EnvNames, + names: ReadonlySet +): boolean { + let found = false; + walk(node, (child) => { + if (found) { + return false; + } + if (ts.isPropertyAccessExpression(child)) { + // `process.env.HOME`, or `userInfo().homedir` in any import form. + found = + (child.name.text === "HOME" && isWholeEnv(child.expression, env)) || + (child.name.text === "homedir" && + ts.isCallExpression(child.expression) && + calleeName(child.expression) === "userInfo"); + } else if (ts.isElementAccessExpression(child)) { + found = + ts.isStringLiteralLike(child.argumentExpression) && + child.argumentExpression.text === "HOME" && + isWholeEnv(child.expression, env); + } else if (ts.isCallExpression(child)) { + // `homedir()`, `os.homedir()` or `node_os.homedir()`. + found = calleeName(child) === "homedir"; + } else if (ts.isIdentifier(child)) { + found = names.has(child.text) && isReference(child); + } + }); + return found; +} + +/** An expression, or the same-file value it is bound to. */ +function resolveLocal( + node: ts.Expression, + values: ReadonlyMap +): ts.Node { + let current: ts.Node = unwrap(node); + const seen = new Set(); + while (ts.isIdentifier(current) && !seen.has(current.text)) { + seen.add(current.text); + const value = values.get(current.text); + if (!value) { + break; + } + current = unwrap(value); + } + return current; +} + +type NamedProperty = ts.PropertyAssignment | ts.ShorthandPropertyAssignment; + +function isNamedProperty( + property: ts.ObjectLiteralElementLike, + name: string +): property is NamedProperty { + return ( + (ts.isPropertyAssignment(property) || + ts.isShorthandPropertyAssignment(property)) && + propertyName(property.name) === name + ); +} + +function envProperty(object: ts.ObjectLiteralExpression): NamedProperty | null { + return ( + object.properties.find((property) => isNamedProperty(property, "env")) ?? + null + ); +} + +function setsHome(object: ts.ObjectLiteralExpression): boolean { + return object.properties.some( + (property) => + ts.isSpreadAssignment(property) || isNamedProperty(property, "HOME") + ); +} + +/** Findings for one spawn call that reaches the source CLI. */ +function cliSpawnFindings( + call: ts.CallExpression, + values: ReadonlyMap, + cliNames: ReadonlySet +): Rule[] { + const options = call.arguments.map((argument) => + resolveLocal(argument, values) + ); + for (const option of options) { + if (!ts.isObjectLiteralExpression(option)) { + continue; + } + const property = envProperty(option); + if (!property) { + continue; + } + const value = resolveLocal( + ts.isPropertyAssignment(property) ? property.initializer : property.name, + values + ); + return ts.isObjectLiteralExpression(value) && !setsHome(value) + ? ["env-without-home"] + : []; + } + // An options value from a parameter, another module or a spread is opaque: + // it may carry an env, which is checked where it is built. + const opaque = options + .slice(1) + .some((option) => + ts.isObjectLiteralExpression(option) + ? option.properties.some(ts.isSpreadAssignment) + : (ts.isIdentifier(option) || ts.isPropertyAccessExpression(option)) && + !namesCli(option, cliNames) + ); + return opaque ? [] : ["cli-without-env"]; +} + +/** Rule findings for one in-scope source file. */ +export function scanSource(path: string, source: string): Finding[] { + const file = parse(path, source); + const env = processImports(file); + const values = bindings(file); + const spawn = spawnImports(file); + // A binding is a CLI path when its value names the CLI outside a function + // or a spawn call: `const proc = Bun.spawn([.., "index.ts"])` is not one. + const cliNames = taintedNames(values, (node, names) => + namesCli( + node, + names, + (child) => isFunctionLike(child) || isSpawnCall(child, spawn) + ) + ); + const homeNames = taintedNames(values, (node, names) => + readsRunnerHome(node, env, names) + ); + const findings: Finding[] = []; + const add = (node: ts.Node, rule: Rule) => { + findings.push({ + line: file.getLineAndCharacterOfPosition(node.getStart(file)).line + 1, + rule, + }); + }; + walk(file, (node) => { + if (isWholeEnv(node, env) && !isKeyUse(node)) { + add(node, "runner-env"); + } + if ( + (ts.isPropertyAssignment(node) && + propertyName(node.name) === "HOME" && + readsRunnerHome(node.initializer, env, homeNames)) || + (ts.isShorthandPropertyAssignment(node) && + node.name.text === "HOME" && + homeNames.has("HOME")) + ) { + add(node, "runner-home"); + } + if ( + isSpawnCall(node, spawn) && + node.arguments.some((argument) => namesCli(argument, cliNames)) + ) { + for (const rule of cliSpawnFindings(node, values, cliNames)) { + add(node, rule); + } + } + }); + return findings.sort((a, b) => a.line - b.line); +} + +/** Test-local modules that `path` imports. */ +function localImports( + path: string, + source: string, + known: ReadonlySet, + testsRoot: string +): string[] { + const file = parse(path, source); + const specifiers: string[] = []; + walk(file, (node) => { + if ( + (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) && + node.moduleSpecifier && + ts.isStringLiteral(node.moduleSpecifier) + ) { + specifiers.push(node.moduleSpecifier.text); + } else if ( + ts.isCallExpression(node) && + node.expression.kind === ts.SyntaxKind.ImportKeyword && + node.arguments[0] && + ts.isStringLiteralLike(node.arguments[0]) + ) { + specifiers.push(node.arguments[0].text); + } + }); + const found: string[] = []; + for (const specifier of specifiers) { + let base: string | null = null; + if (specifier.startsWith(".")) { + base = resolve(dirname(path), specifier); + } else if (specifier.startsWith("@tests/")) { + base = join(testsRoot, specifier.slice("@tests/".length)); + } + if (base === null) { + continue; + } + const target = [base, `${base}.ts`, join(base, "index.ts")].find((item) => + known.has(item) + ); + if (target) { + found.push(target); + } + } + return found; +} + +/** + * Files that name the source CLI, plus the test-local modules they import, + * transitively. `sources` maps absolute paths under `testsRoot` to text. + */ +function cliScope( + sources: ReadonlyMap, + testsRoot: string +): Set { + const known = new Set(sources.keys()); + const scope = new Set(); + const queue: string[] = []; + for (const [path, source] of sources) { + if (namesCli(parse(path, source), new Set())) { + queue.push(path); + } + } + while (queue.length > 0) { + const path = queue.pop() as string; + if (scope.has(path)) { + continue; + } + scope.add(path); + queue.push( + ...localImports(path, sources.get(path) ?? "", known, testsRoot) + ); + } + return scope; +} + +/** Findings for every in-scope file in `sources`, by absolute path. */ +export function scanSources( + sources: ReadonlyMap, + testsRoot: string +): Map { + const results = new Map(); + for (const path of [...cliScope(sources, testsRoot)].sort()) { + const findings = scanSource(path, sources.get(path) ?? ""); + if (findings.length > 0) { + results.set(path, findings); + } + } + return results; +} + +/** `line rule` pairs, comma separated. */ +export function describeFindings(findings: readonly Finding[]): string { + return findings.map(({ line, rule }) => `${line} ${rule}`).join(", "); +} diff --git a/tests/helpers/private-cli-env.ts b/tests/helpers/private-cli-env.ts index 1b5b77232..c2b69dac5 100644 --- a/tests/helpers/private-cli-env.ts +++ b/tests/helpers/private-cli-env.ts @@ -1,4 +1,4 @@ -import { mkdirSync } from "node:fs"; +import { chmodSync, mkdirSync } from "node:fs"; import { isAbsolute, join, relative, resolve } from "node:path"; import { isolatedDockerClientEnv } from "./docker-client.ts"; @@ -21,7 +21,8 @@ export type PrivateCliEnvOptions = { readonly root: string; /** * HOME for the child. It must be the fixture root or a path inside it. - * Defaults to `/user-home`. It is created with mode 0700 when absent. + * Defaults to `/user-home`. It is created when absent, and its mode + * is set to 0700 either way. */ readonly home?: string; /** @@ -59,6 +60,8 @@ export function privateCliEnv( throw new Error("privateCliEnv: pass HOME through the home option."); } mkdirSync(home, { recursive: true, mode: 0o700 }); + // mkdir leaves an existing directory's mode alone. + chmodSync(home, 0o700); const env: Record = {}; for (const key of PRIVATE_CLI_INHERITED_ENV) { const value = process.env[key]; diff --git a/tests/helpers/project-name-fixture.ts b/tests/helpers/project-name-fixture.ts index 24ee7ec22..0f43449c5 100644 --- a/tests/helpers/project-name-fixture.ts +++ b/tests/helpers/project-name-fixture.ts @@ -3,6 +3,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import type { ProjectContext } from "../../src/lib/project.ts"; import type { RegisteredProject } from "../../src/lib/projects-registry.ts"; +import { privateCliEnv } from "./private-cli-env.ts"; /** Synthetic checkout and registry; callers own cleanup of root. */ export async function projectNameFixture() { @@ -10,15 +11,18 @@ export async function projectNameFixture() { const state = join(root, "state"); await mkdir(state); const registryPath = join(state, "projects.json"); - const env = { - ...process.env, - HOME: root, - PATH: process.env.PATH ?? "", - HACK_HOME: state, - HACK_GLOBAL_CONFIG_PATH: join(state, "hack.config.json"), - HACK_DAEMON_DISABLE: "1", - NO_COLOR: "1", - }; + // The source CLI runs with this env: no runner variable such as DOCKER_HOST, + // XDG_* or HACK_* reaches it, and HOME is the fixture root. + const env = privateCliEnv({ + root, + home: root, + env: { + HACK_HOME: state, + HACK_GLOBAL_CONFIG_PATH: join(state, "hack.config.json"), + HACK_DAEMON_DISABLE: "1", + NO_COLOR: "1", + }, + }); async function createProject(directory: string, name: string) { const projectRoot = join(root, directory); const projectDir = join(projectRoot, ".hack"); diff --git a/tests/host-exec-tty.test.ts b/tests/host-exec-tty.test.ts index 80c3bc41f..838b6781f 100644 --- a/tests/host-exec-tty.test.ts +++ b/tests/host-exec-tty.test.ts @@ -2,6 +2,7 @@ import { expect, test } from "bun:test"; import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const cases = [ { @@ -73,24 +74,38 @@ for (const scenario of cases) { test.skipIf(!Bun.which("python3"))( `TTY ${scenario.name} preserves terminal ownership and cleans only owned processes`, async () => { - const proc = Bun.spawn( - [ - "python3", - resolve(import.meta.dir, "fixtures/tty-cancellation.py"), - "probe", - process.execPath, - resolve(import.meta.dir, "../index.ts"), - scenario.signal, - scenario.behavior, - scenario.mode, - ], - { stdin: "ignore", stdout: "pipe", stderr: "pipe" } - ); - const [stdout, stderr, exitCode] = await Promise.all([ - new Response(proc.stdout).text(), - new Response(proc.stderr).text(), - proc.exited, - ]); + // The probe passes its env, plus a private HACK_HOME, to the CLI. + const root = await mkdtemp(join(tmpdir(), "hack-tty-env-")); + let stdout: string; + let stderr: string; + let exitCode: number; + try { + const proc = Bun.spawn( + [ + "python3", + resolve(import.meta.dir, "fixtures/tty-cancellation.py"), + "probe", + process.execPath, + resolve(import.meta.dir, "../index.ts"), + scenario.signal, + scenario.behavior, + scenario.mode, + ], + { + env: privateCliEnv({ root }), + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + } + ); + [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + } finally { + await rm(root, { recursive: true, force: true }); + } expect(stderr).toBe(""); expect(exitCode).toBe(0); const outcome = JSON.parse(stdout); @@ -183,7 +198,12 @@ test.skipIf(!Bun.which("python3"))( "exit", "paused", ], - { stdin: "ignore", stdout: "pipe", stderr: "pipe" } + { + env: privateCliEnv({ root: directory }), + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + } ); const [stdout, stderr, exit] = await Promise.all([ new Response(probe.stdout).text(), diff --git a/tests/log-pipe.test.ts b/tests/log-pipe.test.ts index dd308eb7a..f48940855 100644 --- a/tests/log-pipe.test.ts +++ b/tests/log-pipe.test.ts @@ -1,20 +1,26 @@ import { expect } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { testIntegration } from "./helpers/ci.ts"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; function stripTrailingNewline(text: string): string { return text.endsWith("\n") ? text.slice(0, -1) : text; } -testIntegration( - "log-pipe formats docker-compose prefixed lines from stdin", - { timeout: 15_000 }, - async () => { - const input = `${[ - 'api | {"level":"warn","msg":"hello","foo":1}', - "api | plain text", - ].join("\n")}\n`; - +/** Runs source `hack log-pipe` on `input` with a private HOME and env. */ +async function runLogPipe(opts: { + readonly stream: "stdout" | "stderr"; + readonly input: string; +}): Promise<{ + readonly exitCode: number; + readonly stdoutText: string; + readonly stderrText: string; +}> { + const root = await mkdtemp(join(tmpdir(), "hack-log-pipe-")); + try { const proc = Bun.spawn( [ "bun", @@ -23,19 +29,40 @@ testIntegration( "--format", "docker-compose", "--stream", - "stdout", + opts.stream, ], { cwd: process.cwd(), - stdin: new Response(input).body, + env: privateCliEnv({ root }), + stdin: new Response(opts.input).body, stdout: "pipe", stderr: "pipe", } ); + const [exitCode, stdoutText, stderrText] = await Promise.all([ + proc.exited, + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + ]); + return { exitCode, stdoutText, stderrText }; + } finally { + await rm(root, { recursive: true, force: true }); + } +} + +testIntegration( + "log-pipe formats docker-compose prefixed lines from stdin", + { timeout: 15_000 }, + async () => { + const input = `${[ + 'api | {"level":"warn","msg":"hello","foo":1}', + "api | plain text", + ].join("\n")}\n`; - const exitCode = await proc.exited; - const stdoutText = await new Response(proc.stdout).text(); - const stderrText = await new Response(proc.stderr).text(); + const { exitCode, stdoutText, stderrText } = await runLogPipe({ + stream: "stdout", + input, + }); expect(exitCode).toBe(0); expect(stderrText).toBe(""); @@ -51,27 +78,10 @@ testIntegration( { timeout: 15_000 }, async () => { const input = 'api | {"level":"info","msg":"boom"}\n'; - const proc = Bun.spawn( - [ - "bun", - "index.ts", - "log-pipe", - "--format", - "docker-compose", - "--stream", - "stderr", - ], - { - cwd: process.cwd(), - stdin: new Response(input).body, - stdout: "pipe", - stderr: "pipe", - } - ); - - const exitCode = await proc.exited; - const stdoutText = await new Response(proc.stdout).text(); - const stderrText = await new Response(proc.stderr).text(); + const { exitCode, stdoutText, stderrText } = await runLogPipe({ + stream: "stderr", + input, + }); expect(exitCode).toBe(0); expect(stdoutText).toBe(""); diff --git a/tests/mcp-entrypoint.test.ts b/tests/mcp-entrypoint.test.ts index 605cc7f6d..2e92fce8d 100644 --- a/tests/mcp-entrypoint.test.ts +++ b/tests/mcp-entrypoint.test.ts @@ -1,5 +1,31 @@ -import { expect, test } from "bun:test"; -import { resolve } from "node:path"; +import { afterEach, expect, test } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; + +const roots: string[] = []; + +afterEach(async () => { + await Promise.all( + roots.splice(0).map((root) => rm(root, { recursive: true, force: true })) + ); +}); + +/** Spawns the source CLI with a private HOME and environment. */ +async function spawnCli(args: readonly string[]) { + const root = await mkdtemp(join(tmpdir(), "hack-mcp-entrypoint-")); + roots.push(root); + return Bun.spawn( + [process.execPath, resolve(import.meta.dir, "..", "index.ts"), ...args], + { + env: privateCliEnv({ root }), + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + } + ); +} for (const args of [ ["mcp", "serve", "--help"], @@ -7,14 +33,7 @@ for (const args of [ ["mcp", "serve", "--version"], ]) { test(`MCP entrypoint preserves parser behavior for ${args.join(" ")}`, async () => { - const proc = Bun.spawn( - [process.execPath, resolve(import.meta.dir, "..", "index.ts"), ...args], - { - stdin: "ignore", - stdout: "pipe", - stderr: "pipe", - } - ); + const proc = await spawnCli(args); const [stdout, stderr, code] = await Promise.all([ new Response(proc.stdout).text(), new Response(proc.stderr).text(), @@ -29,21 +48,7 @@ for (const args of [ } test("MCP entrypoint rejects unknown options through the normal JSON error path", async () => { - const proc = Bun.spawn( - [ - process.execPath, - resolve(import.meta.dir, "..", "index.ts"), - "mcp", - "serve", - "--json", - "--bogus-option", - ], - { - stdin: "ignore", - stdout: "pipe", - stderr: "pipe", - } - ); + const proc = await spawnCli(["mcp", "serve", "--json", "--bogus-option"]); const [stdout, stderr, code] = await Promise.all([ new Response(proc.stdout).text(), new Response(proc.stderr).text(), diff --git a/tests/mcp.test.ts b/tests/mcp.test.ts index e69b7fd40..29314f5b1 100644 --- a/tests/mcp.test.ts +++ b/tests/mcp.test.ts @@ -265,14 +265,17 @@ async function startContractFixture( ].join("\n") ); await chmod(stub, 0o755); - const env: Record = { - ...process.env, - HOME: root, - HACK_HOME: state, - HACK_GLOBAL_CONFIG_PATH: join(registryDir, "config.json"), - HACK_MCP_COMMAND: stub, - }; - Reflect.deleteProperty(env, "HACK_NO_INTERACTIVE"); + // The server runs HACK_MCP_COMMAND with this env. No runner variable, such + // as HACK_NO_INTERACTIVE, reaches the stub unless the server sets it. + const env: Record = privateCliEnv({ + root, + home: root, + env: { + HACK_HOME: state, + HACK_GLOBAL_CONFIG_PATH: join(registryDir, "config.json"), + HACK_MCP_COMMAND: stub, + }, + }); const server = createMcpServer({ cwd, env }); const client = new Client({ name: "hack-mcp-contract", version: "1" }); contractClients.push(client); diff --git a/tests/native-config-import-preview.test.ts b/tests/native-config-import-preview.test.ts index 93bacdd97..7d57945a0 100644 --- a/tests/native-config-import-preview.test.ts +++ b/tests/native-config-import-preview.test.ts @@ -318,7 +318,6 @@ test("CLI requires dry-run, reports values-free mappings and preserves files", a HACK_HOME: process.env.HACK_HOME, HACK_GLOBAL_CONFIG_PATH: process.env.HACK_GLOBAL_CONFIG_PATH, HACK_ENV_SECRET_KEY: process.env.HACK_ENV_SECRET_KEY, - HACK_CONFIG_COMPILER_BINARY: binary, FORCE_COLOR: "0", }, diff --git a/tests/native-project-down.test.ts b/tests/native-project-down.test.ts index c76aa3cbc..2ef474f2b 100644 --- a/tests/native-project-down.test.ts +++ b/tests/native-project-down.test.ts @@ -74,6 +74,15 @@ async function fixture(saved = true) { return { scope, runtime: { binary: "/not-invoked", home: nativeHome } }; } +/** A fixture root for the CLI's private HOME, beside the project checkout. */ +async function clientRoot(): Promise { + const root = await realpath( + await mkdtemp(join(tmpdir(), "native-down-client-")) + ); + roots.push(root); + return root; +} + function finalizationRoot(scope: NativeProjectRunScope): string { const hash = createHash("sha256") .update( @@ -487,7 +496,7 @@ test("not-started native down does not run configured hooks", async () => { [process.execPath, "index.ts", "down", "--path", opts.scope.projectRoot], { env: privateCliEnv({ - root: opts.scope.projectRoot, + root: await clientRoot(), env: { HACK_RUNTIME_BACKEND: "native", HACK_NATIVE_HOME: opts.scope.nativeHome, @@ -651,7 +660,7 @@ test("native down JSON isolates hook output and uses saved overlay through real ], { env: privateCliEnv({ - root: opts.scope.projectRoot, + root: await clientRoot(), env: { HACK_RUNTIME_BACKEND: "native", HACK_NATIVE_HOME: opts.scope.nativeHome, diff --git a/tests/project-env-worktree-key.test.ts b/tests/project-env-worktree-key.test.ts index 1fa6774af..4c6349f14 100644 --- a/tests/project-env-worktree-key.test.ts +++ b/tests/project-env-worktree-key.test.ts @@ -267,8 +267,8 @@ async function runHackCli(opts: { readonly sandbox: string; readonly args: readonly string[]; }): Promise { + // privateCliEnv creates this HOME with mode 0700. const home = resolve(opts.sandbox, "home"); - await mkdir(home, { recursive: true }); // No runner variable such as HACK_ENV_SECRET_KEY reaches the child. const env = privateCliEnv({ root: opts.sandbox, From 6aeec7e20ee0c3eb46ed398a8ef32da0cfe3566e Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Mon, 12 Oct 2026 20:54:55 -0400 Subject: [PATCH 3/3] test: isolate host-exec and lifecycle client CLI spawns host-exec-lifetime built its CLI env without HOME, so the CLI resolved the runner's real home. It now uses privateCliEnv with the same PATH, HACK_HOME and HACK_NO_INTERACTIVE values. lifecycle-process-delivery ran the client CLI through shell run, which lays env over the runner's whole process.env. New privateCliRunEnv returns the private env plus unsetEnvKeys for every other runner key, so the child sees only the private env. LEAKED_PARENT still reaches the client and must still be absent from the launched command. The guard now follows same-file function returns for env and argv, treats src/lib/shell.ts run/exec and same-file wrappers of them as spawns, and adds shell-runner-env for a shell CLI spawn without unsetEnvKeys. Object.keys of the env counts as a key-name use. Reverting either fix fails the guard. Co-Authored-By: Claude Opus 5.5 --- tests/cli-spawn-env-isolation.test.ts | 105 +++++- tests/helpers/cli-spawn-env-scan.ts | 445 ++++++++++++++++++++--- tests/helpers/private-cli-env.ts | 22 ++ tests/host-exec-lifetime.test.ts | 15 +- tests/lifecycle-process-delivery.test.ts | 17 +- 5 files changed, 537 insertions(+), 67 deletions(-) diff --git a/tests/cli-spawn-env-isolation.test.ts b/tests/cli-spawn-env-isolation.test.ts index 314900ee6..37ef5df61 100644 --- a/tests/cli-spawn-env-isolation.test.ts +++ b/tests/cli-spawn-env-isolation.test.ts @@ -129,6 +129,9 @@ test("the scan flags every way to hand the CLI the runner's environment", () => spawnWith("{ env: { ...rest } }") ) ).toEqual(["1 runner-env"]); + expect(scanExample("const all = Object.values(process.env);")).toEqual([ + "1 runner-env", + ]); expect( scanExample( "const saved = process.env;", @@ -233,7 +236,8 @@ test("the scan accepts a private env and one-key uses of process.env", () => { "process.env = { PATH: p };", "const { PATH } = process.env;", "restoreEnv('HOME', home);", - "const p = `${process.env.PATH}`;" + "const p = `${process.env.PATH}`;", + "const keys = Object.keys(process.env);" ) ).toEqual([]); // A spawn that does not reach the CLI may inherit, including one that uses @@ -257,6 +261,105 @@ test("the scan accepts a private env and one-key uses of process.env", () => { ).toEqual([]); }); +test("the scan follows same-file functions for env and argv", () => { + // An env returned by a same-file function, directly or through a binding. + expect( + scanExample( + "function fixtureEnv(root) { return { PATH: p, HACK_HOME: root }; }", + spawnWith("{ env: fixtureEnv(root) }") + ) + ).toEqual(["2 env-without-home"]); + expect( + scanExample( + "const envFor = (root) => ({ PATH: p });", + "const env = envFor(root);", + spawnWith("{ env }") + ) + ).toEqual(["3 env-without-home"]); + expect( + scanExample( + "function fixtureEnv() { const e = { PATH: p }; return e; }", + spawnWith("{ env: fixtureEnv() }") + ) + ).toEqual(["2 env-without-home"]); + expect( + scanExample( + "function fixtureEnv(root) { return privateCliEnv({ root, env: { X: '1' } }); }", + spawnWith("{ env: fixtureEnv(root) }") + ) + ).toEqual([]); + // An argv that a same-file function returns reaches the CLI. + expect( + scanExample( + 'const entry = resolve(dir, "../index.ts");', + "function command() { return [process.execPath, entry]; }", + 'Bun.spawn(command(), { stdout: "pipe" });', + "Bun.spawn(command(), { env: fixtureEnv(root) });", + "function fixtureEnv(root) { return { HACK_HOME: root }; }" + ) + ).toEqual(["3 cli-without-env", "4 env-without-home"]); +}); + +test("the scan treats src/lib/shell.ts runners and their wrappers as spawns", () => { + const cli = '[process.execPath, "../index.ts"]'; + const shell = 'import { run } from "../src/lib/shell.ts";'; + // `run` lays env over the runner's whole environment. + expect(scanExample(shell, `await run(${cli}, { stdin: "ignore" });`)).toEqual( + ["2 cli-without-env"] + ); + expect( + scanExample(shell, `await run(${cli}, { env: { LEAKED: "x" } });`) + ).toEqual(["2 env-without-home"]); + expect( + scanExample(shell, `await run(${cli}, { env: privateCliEnv({ root }) });`) + ).toEqual(["2 shell-runner-env"]); + expect( + scanExample( + shell, + `await run(${cli}, { ...privateCliRunEnv({ root }), stdin: "ignore" });`, + "const { env, unsetEnvKeys } = privateCliRunEnv({ root });", + `await run(${cli}, { env, unsetEnvKeys });`, + 'await run(["git", "status"], { cwd });' + ) + ).toEqual([]); + // Namespace, alias and renamed imports; shell `exec` cannot unset keys. + expect( + scanExample( + 'import * as sh from "@lib/shell";', + `sh.exec(${cli}, { env: privateCliEnv({ root }) });` + ) + ).toEqual(["2 shell-runner-env"]); + expect( + scanExample( + 'import { exec as realExec } from "../../src/lib/shell.ts";', + 'realExec(["bun", "index.ts"]);' + ) + ).toEqual(["2 cli-without-env"]); + // A wrapper that forwards its parameters is checked at the call site. + expect( + scanExample( + shell, + "function runIt(cmd, opts) { return run(cmd, opts); }", + `runIt(${cli}, { stdin: "ignore" });`, + `runIt(${cli}, { ...privateCliRunEnv({ root }) });` + ) + ).toEqual(["3 cli-without-env"]); + // A wrapper with its own options is checked by them. + expect( + scanExample( + shell, + "const runPrivate = (cmd) => run(cmd, { ...privateCliRunEnv({ root }) });", + 'const runLeaky = async (cmd) => { await run(cmd, { stdin: "ignore" }); };', + "const runTwice = (cmd) => runLeaky(cmd);", + "const runPrivateTwice = (cmd) => runPrivate(cmd);", + `runPrivate(${cli});`, + `runLeaky(${cli});`, + `runTwice(${cli});`, + `runPrivateTwice(${cli});` + ) + ).toEqual(["7 cli-without-env", "8 cli-without-env"]); +}); + test("the scan follows a CLI file's local imports and skips other files", () => { const root = "/virtual/tests"; const sources = new Map([ diff --git a/tests/helpers/cli-spawn-env-scan.ts b/tests/helpers/cli-spawn-env-scan.ts index c18b70d6b..a47b5f09d 100644 --- a/tests/helpers/cli-spawn-env-scan.ts +++ b/tests/helpers/cli-spawn-env-scan.ts @@ -11,39 +11,69 @@ import ts from "typescript"; * so an env object that a fixture helper builds for the CLI is checked where * it is built. * + * Spawn calls are `spawn`, `spawnSync`, `execFile`, `exec`, `fork` and their + * `Bun.`/`child_process.`/`execa` forms; `run` and `exec` imported from + * src/lib/shell.ts, named or through a namespace import; and calls to a + * same-file function (a declaration, or a `const` arrow or function + * expression) whose body calls one of those shell functions or another such + * wrapper. + * * Rules, checked on the TypeScript syntax tree of each in-scope file: * - `runner-env`: no copy of the whole runner environment. A use of * `process.env`, `Bun.env`, `import.meta.env` or a `node:process` `env` - * import is a copy unless it reads, deletes or tests one key, or replaces - * the whole object. Spreads, `env: process.env`, `Object.assign(x, - * process.env)`, `const saved = process.env` and rest destructuring are - * all copies. + * import is a copy unless it reads, deletes or tests one key, lists only + * the key names with `Object.keys`, or replaces the whole object. Spreads, + * `env: process.env`, `Object.assign(x, process.env)`, `const saved = + * process.env` and rest destructuring are all copies. * - `runner-home`: no `HOME` property whose value derives from the runner's * home: `process.env.HOME`, `homedir()` in any form, `userInfo().homedir`, * or a binding in the same file that holds one of these. - * - `cli-without-env`: a spawn call (`spawn`, `spawnSync`, `execFile`, - * `exec`, `fork` and their `Bun.`/`child_process.` forms) whose arguments - * reach the CLI must pass an `env` option. Without it the child inherits the - * whole runner environment, including HOME. A wrapper such as `python3 - * probe.py bun index.ts` counts, because its argv names the CLI. + * - `cli-without-env`: a spawn call whose arguments reach the CLI must pass an + * `env` option. Without it the child inherits the whole runner environment, + * including HOME. Arguments reach the CLI when they name it directly, + * through a same-file binding, or through a call to a same-file function + * that returns it. A wrapper such as `python3 probe.py bun index.ts` counts, + * because its argv names the CLI. * - `env-without-home`: when that `env` is an object literal, it must set * HOME or spread another object. Without HOME the child resolves its home - * from the account database, which is the runner's real home. + * from the account database, which is the runner's real home. An `env` + * that calls a same-file function, directly or through a binding, is + * checked through the values that function returns. + * - `shell-runner-env`: a CLI spawn through src/lib/shell.ts must also pass + * `unsetEnvKeys`, or spread an options object such as + * `privateCliRunEnv(...)`, because shell `run` and `exec` lay `env` over the + * runner's whole `process.env`. Shell `exec` cannot unset keys, so it cannot + * run the CLI privately. + * + * Shell wrappers. At a wrapper call that names the CLI, each shell call inside + * the wrapper is checked: against the wrapper call's own arguments when the + * inner options use a wrapper parameter, through the inner wrapper when it is + * one (at most four deep), and against the inner options otherwise. * * Limits. This is a syntax check within one file, not data flow. It trusts an * env, HOME or argv value that comes from another module or a parameter: for * example `HOME: opts.home`, `env: options.env` in a shared runner, or a local - * `run(argv)` wrapper whose spawn never names the CLI itself. A spawn of a CLI - * path built at runtime without naming `index.ts` (for example a compiled - * binary) is not seen either. Those spawns still need a private env by - * convention; prefer one shared runner that calls `privateCliEnv`. + * `run(argv)` wrapper around `Bun.spawn` whose spawn never names the CLI + * itself. A same-file function is followed only for an `env` value (at most + * four calls deep) and an argv, and only through its `return` expressions, + * not the branches of a conditional; a function that returns options is not + * followed, so such a call counts as options without `env`. Inner options + * that only partly use a wrapper parameter are checked as the call site's + * arguments. An options object that spreads another object is trusted, and + * so is the `unsetEnvKeys` value. A + * dynamic key read such as `process.env[key]` counts as a one-key use, so a + * loop that copies values key by key is not seen. A spawn of a CLI path built + * at runtime without naming `index.ts` (for example a compiled binary) is not + * seen either. Those spawns still need a private env by convention; prefer one + * shared runner that calls `privateCliEnv` or `privateCliRunEnv`. */ export type Rule = | "runner-env" | "runner-home" | "cli-without-env" - | "env-without-home"; + | "env-without-home" + | "shell-runner-env"; export type Finding = { readonly line: number; readonly rule: Rule }; @@ -69,6 +99,11 @@ const ENV_OWNERS = new Set([ "globalThis.process", "globalThis.Bun", ]); +/** src/lib/shell.ts, by relative path or through a `@lib`/`@` alias. */ +const SHELL_MODULE = /(?:^|[/@])lib\/shell(?:\.ts)?$/; +const SHELL_SPAWNS: ReadonlySet = new Set(["run", "exec"]); +/** How many same-file calls an `env` value is followed through. */ +const MAX_RETURN_DEPTH = 4; const KEY_ACCESSORS = new Set([ "Reflect.deleteProperty", "Reflect.has", @@ -91,12 +126,28 @@ function moduleName(specifier: string): string { return specifier.replace(/^node:/, ""); } -/** How this file can spawn a process: imported functions and namespaces. */ +type FunctionNode = + | ts.FunctionDeclaration + | ts.ArrowFunction + | ts.FunctionExpression; + +/** How this file can spawn a process. */ type SpawnNames = { + /** Imported process-spawning functions, by local name. */ readonly functions: ReadonlySet; + /** Namespace imports (and `Bun`), with the spawn exports they hold. */ readonly namespaces: ReadonlyMap>; + /** Local names of src/lib/shell.ts `run` and `exec`. */ + readonly shellFunctions: ReadonlySet; + /** Namespace imports of src/lib/shell.ts. */ + readonly shellNamespaces: ReadonlySet; + /** Same-file functions that call a shell spawn, by name. */ + readonly wrappers: ReadonlyMap; }; +/** A process API, src/lib/shell.ts, or a same-file wrapper of the shell. */ +type SpawnKind = "process" | "shell" | "wrapper"; + /** Local names that one import declaration binds. */ type ImportNames = { /** Module specifier without a `node:` prefix. */ @@ -145,45 +196,152 @@ function importNames(file: ts.SourceFile): ImportNames[] { return imports; } -function spawnImports(file: ts.SourceFile): SpawnNames { +/** Same-file named functions: declarations and `const` function values. */ +function localFunctions(file: ts.SourceFile): Map { + const functions = new Map(); + walk(file, (node) => { + if (ts.isFunctionDeclaration(node) && node.name && node.body) { + functions.set(node.name.text, node); + } else if ( + ts.isVariableDeclaration(node) && + ts.isIdentifier(node.name) && + node.initializer + ) { + const value = unwrap(node.initializer); + if (ts.isArrowFunction(value) || ts.isFunctionExpression(value)) { + functions.set(node.name.text, value); + } + } + }); + return functions; +} + +/** Calls in a function body, nested functions included, that `matches`. */ +function callsIn( + fn: FunctionNode, + matches: (call: ts.CallExpression) => boolean +): ts.CallExpression[] { + const calls: ts.CallExpression[] = []; + if (!fn.body) { + return calls; + } + walk(fn.body, (node) => { + if (ts.isCallExpression(node) && matches(node)) { + calls.push(node); + } + }); + return calls; +} + +function isShellCall(node: ts.CallExpression, spawn: SpawnNames): boolean { + const kind = spawnKind(node, spawn); + return kind === "shell" || kind === "wrapper"; +} + +/** Records the spawn exports that one import binds, by local name. */ +function addSpawnImport( + entry: ImportNames, + exports: ReadonlySet, + functions: Set, + namespaces: Map> +): void { + for (const name of entry.whole) { + namespaces.set(name, exports); + } + for (const { imported, local } of entry.named) { + if (exports.has(imported)) { + functions.add(local); + } + } +} + +function spawnImports( + file: ts.SourceFile, + localFunctionsByName: ReadonlyMap +): SpawnNames { const functions = new Set(); const namespaces = new Map>([ ["Bun", new Set(["spawn", "spawnSync"])], ]); + const shellFunctions = new Set(); + const shellNamespaces = new Set(); for (const entry of importNames(file)) { - const exports = SPAWN_MODULES[entry.module]; - if (!exports) { - continue; - } - for (const name of entry.whole) { - namespaces.set(name, exports); + if (SHELL_MODULE.test(entry.module)) { + addSpawnImport(entry, SHELL_SPAWNS, shellFunctions, namespaces); + for (const name of entry.whole) { + shellNamespaces.add(name); + } + } else { + const exports = SPAWN_MODULES[entry.module]; + if (exports) { + addSpawnImport(entry, exports, functions, namespaces); + } } - for (const { imported, local } of entry.named) { - if (exports.has(imported)) { - functions.add(local); + } + const wrappers = new Map(); + const names: SpawnNames = { + functions, + namespaces, + shellFunctions, + shellNamespaces, + wrappers, + }; + addShellWrappers(names, wrappers, localFunctionsByName); + return names; +} + +/** Adds same-file shell wrappers to `wrappers`, closed over wrappers of wrappers. */ +function addShellWrappers( + names: SpawnNames, + wrappers: Map, + localFunctionsByName: ReadonlyMap +): void { + let grew = true; + while (grew) { + grew = false; + for (const [name, fn] of localFunctionsByName) { + if ( + !wrappers.has(name) && + callsIn(fn, (call) => isShellCall(call, names)).length > 0 + ) { + wrappers.set(name, fn); + grew = true; } } } - return { functions, namespaces }; } -function isSpawnCall( - node: ts.Node, - spawn: SpawnNames -): node is ts.CallExpression { +function spawnKind(node: ts.Node, spawn: SpawnNames): SpawnKind | null { if (!ts.isCallExpression(node)) { - return false; + return null; } const callee = unwrap(node.expression); if (ts.isIdentifier(callee)) { - return spawn.functions.has(callee.text); + if (spawn.functions.has(callee.text)) { + return "process"; + } + if (spawn.shellFunctions.has(callee.text)) { + return "shell"; + } + return spawn.wrappers.has(callee.text) ? "wrapper" : null; } - return ( + if ( ts.isPropertyAccessExpression(callee) && ts.isIdentifier(callee.expression) && - (spawn.namespaces.get(callee.expression.text)?.has(callee.name.text) ?? - false) - ); + spawn.namespaces.get(callee.expression.text)?.has(callee.name.text) + ) { + return spawn.shellNamespaces.has(callee.expression.text) + ? "shell" + : "process"; + } + return null; +} + +function isSpawnCall( + node: ts.Node, + spawn: SpawnNames +): node is ts.CallExpression { + return spawnKind(node, spawn) !== null; } /** The called name: `f` for `f()` and `x.f()`. */ @@ -353,7 +511,9 @@ function isKeyUse(node: ts.Node): boolean { if ( ts.isCallExpression(parent) && parent.arguments[0] === node && - KEY_ACCESSORS.has(parent.expression.getText()) + (KEY_ACCESSORS.has(parent.expression.getText()) || + (parent.arguments.length === 1 && + parent.expression.getText() === "Object.keys")) ) { return true; } @@ -449,12 +609,73 @@ function setsHome(object: ts.ObjectLiteralExpression): boolean { ); } -/** Findings for one spawn call that reaches the source CLI. */ +/** The `return` expressions of a function, outside nested functions. */ +function returnedValues(fn: FunctionNode): ts.Expression[] { + if (!fn.body) { + return []; + } + if (!ts.isBlock(fn.body)) { + return [fn.body]; + } + const returned: ts.Expression[] = []; + walk(fn.body, (node) => { + if (isFunctionLike(node)) { + return false; + } + if (ts.isReturnStatement(node) && node.expression) { + returned.push(node.expression); + } + }); + return returned; +} + +/** Same-file facts that a spawn check reads. */ +type ScanContext = { + readonly values: ReadonlyMap; + readonly functions: ReadonlyMap; + readonly cliNames: ReadonlySet; + readonly spawn: SpawnNames; +}; + +/** + * Whether an `env` value is an object literal without HOME: directly, through + * a same-file binding, or as a value that a same-file function it calls + * returns. + */ +function envLacksHome( + node: ts.Expression, + ctx: ScanContext, + depth = 0 +): boolean { + const value = resolveLocal(node, ctx.values); + if (ts.isObjectLiteralExpression(value)) { + return !setsHome(value); + } + if (!ts.isCallExpression(value) || depth >= MAX_RETURN_DEPTH) { + return false; + } + const callee = unwrap(value.expression); + const fn = ts.isIdentifier(callee) + ? ctx.functions.get(callee.text) + : undefined; + return ( + fn !== undefined && + returnedValues(fn).some((returned) => + envLacksHome(returned, ctx, depth + 1) + ) + ); +} + +/** + * Findings for one spawn call that reaches the source CLI. `shell` marks a + * src/lib/shell.ts call, which lays `env` over the runner's environment. + */ function cliSpawnFindings( call: ts.CallExpression, - values: ReadonlyMap, - cliNames: ReadonlySet + ctx: ScanContext, + shell: boolean ): Rule[] { + const { values, cliNames } = ctx; const options = call.arguments.map((argument) => resolveLocal(argument, values) ); @@ -466,13 +687,21 @@ function cliSpawnFindings( if (!property) { continue; } - const value = resolveLocal( - ts.isPropertyAssignment(property) ? property.initializer : property.name, - values + if ( + envLacksHome( + ts.isPropertyAssignment(property) + ? property.initializer + : property.name, + ctx + ) + ) { + return ["env-without-home"]; + } + const unsets = option.properties.some( + (item) => + ts.isSpreadAssignment(item) || isNamedProperty(item, "unsetEnvKeys") ); - return ts.isObjectLiteralExpression(value) && !setsHome(value) - ? ["env-without-home"] - : []; + return shell && !unsets ? ["shell-runner-env"] : []; } // An options value from a parameter, another module or a spread is opaque: // it may carry an env, which is checked where it is built. @@ -487,24 +716,120 @@ function cliSpawnFindings( return opaque ? [] : ["cli-without-env"]; } +function parameterNames(fn: FunctionNode): Set { + const names = new Set(); + for (const parameter of fn.parameters) { + walk(parameter.name, (node) => { + if (ts.isIdentifier(node)) { + names.add(node.text); + } + }); + } + return names; +} + +/** Whether any of `nodes` references one of `names`. */ +function referencesAny( + nodes: readonly ts.Node[], + names: ReadonlySet +): boolean { + let found = false; + for (const node of nodes) { + walk(node, (child) => { + if (found) { + return false; + } + found = + ts.isIdentifier(child) && names.has(child.text) && isReference(child); + }); + } + return found; +} + +/** Findings for a CLI spawn call of any kind. */ +function spawnCallFindings( + call: ts.CallExpression, + kind: SpawnKind, + ctx: ScanContext, + depth = 0 +): Rule[] { + if (kind !== "wrapper") { + return cliSpawnFindings(call, ctx, kind === "shell"); + } + const callee = unwrap(call.expression); + const fn = ts.isIdentifier(callee) + ? ctx.spawn.wrappers.get(callee.text) + : undefined; + if (!fn || depth >= MAX_RETURN_DEPTH) { + return []; + } + const parameters = parameterNames(fn); + const rules = new Set(); + for (const inner of callsIn(fn, (node) => isShellCall(node, ctx.spawn))) { + let found: Rule[]; + if (referencesAny(inner.arguments.slice(1), parameters)) { + // Options built from the wrapper's parameters come from this call site. + found = cliSpawnFindings(call, ctx, true); + } else if (spawnKind(inner, ctx.spawn) === "wrapper") { + found = spawnCallFindings(inner, "wrapper", ctx, depth + 1); + } else { + found = cliSpawnFindings(inner, ctx, true); + } + for (const rule of found) { + rules.add(rule); + } + } + return [...rules]; +} + +/** + * Names that hold a CLI path. A binding is one when its value names the CLI + * outside a function or a spawn call: `const proc = Bun.spawn([.., + * "index.ts"])` is not one. A same-file function is one when a value it + * returns names the CLI the same way, so `hostCommand(...)` in an argv counts. + */ +function cliPathNames( + values: ReadonlyMap, + functions: ReadonlyMap, + spawn: SpawnNames +): Set { + const names = new Set(); + const notPath = (child: ts.Node) => + isFunctionLike(child) || isSpawnCall(child, spawn); + let grew = true; + while (grew) { + grew = false; + for (const [name, value] of values) { + if (!names.has(name) && namesCli(value, names, notPath)) { + names.add(name); + grew = true; + } + } + for (const [name, fn] of functions) { + if ( + !names.has(name) && + returnedValues(fn).some((value) => namesCli(value, names, notPath)) + ) { + names.add(name); + grew = true; + } + } + } + return names; +} + /** Rule findings for one in-scope source file. */ export function scanSource(path: string, source: string): Finding[] { const file = parse(path, source); const env = processImports(file); const values = bindings(file); - const spawn = spawnImports(file); - // A binding is a CLI path when its value names the CLI outside a function - // or a spawn call: `const proc = Bun.spawn([.., "index.ts"])` is not one. - const cliNames = taintedNames(values, (node, names) => - namesCli( - node, - names, - (child) => isFunctionLike(child) || isSpawnCall(child, spawn) - ) - ); + const functions = localFunctions(file); + const spawn = spawnImports(file, functions); + const cliNames = cliPathNames(values, functions, spawn); const homeNames = taintedNames(values, (node, names) => readsRunnerHome(node, env, names) ); + const ctx: ScanContext = { values, functions, cliNames, spawn }; const findings: Finding[] = []; const add = (node: ts.Node, rule: Rule) => { findings.push({ @@ -526,11 +851,13 @@ export function scanSource(path: string, source: string): Finding[] { ) { add(node, "runner-home"); } + const kind = spawnKind(node, spawn); if ( - isSpawnCall(node, spawn) && + kind !== null && + ts.isCallExpression(node) && node.arguments.some((argument) => namesCli(argument, cliNames)) ) { - for (const rule of cliSpawnFindings(node, values, cliNames)) { + for (const rule of spawnCallFindings(node, kind, ctx)) { add(node, rule); } } diff --git a/tests/helpers/private-cli-env.ts b/tests/helpers/private-cli-env.ts index c2b69dac5..2d6c83931 100644 --- a/tests/helpers/private-cli-env.ts +++ b/tests/helpers/private-cli-env.ts @@ -82,3 +82,25 @@ export function privateCliEnv( env.HOME = home; return env; } + +/** + * Options for `run` in src/lib/shell.ts, which builds a child env from the + * runner's whole `process.env` plus `env`. `unsetEnvKeys` names every runner + * key that {@link privateCliEnv} did not set, so the child gets exactly the + * private env. Spread the result into the `run` options. + * + * The key list is a snapshot: a runner key added between this call and the + * spawn would still reach the child. Call it right before `run`. + */ +export function privateCliRunEnv(opts: PrivateCliEnvOptions): { + readonly env: Record; + readonly unsetEnvKeys: readonly string[]; +} { + const env = privateCliEnv(opts); + return { + env, + unsetEnvKeys: Object.keys(process.env).filter( + (key) => !Object.hasOwn(env, key) + ), + }; +} diff --git a/tests/host-exec-lifetime.test.ts b/tests/host-exec-lifetime.test.ts index 48facbcac..dba007612 100644 --- a/tests/host-exec-lifetime.test.ts +++ b/tests/host-exec-lifetime.test.ts @@ -9,6 +9,7 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { resolve } from "node:path"; +import { privateCliEnv } from "./helpers/private-cli-env.ts"; const fixtures: string[] = []; const wrappers: ReturnType[] = []; @@ -224,12 +225,16 @@ function hostCommand({ ]; } +/** A private CLI env: fixture HOME and HACK_HOME, no runner `HACK_*`/Docker state. */ function fixtureEnv(root: string): Record { - return { - PATH: process.env.PATH ?? "/usr/bin:/bin", - HACK_HOME: resolve(root, "hack-home"), - HACK_NO_INTERACTIVE: "1", - }; + return privateCliEnv({ + root, + env: { + PATH: process.env.PATH ?? "/usr/bin:/bin", + HACK_HOME: resolve(root, "hack-home"), + HACK_NO_INTERACTIVE: "1", + }, + }); } async function createFixture(opts: { diff --git a/tests/lifecycle-process-delivery.test.ts b/tests/lifecycle-process-delivery.test.ts index 1c145afb7..bb9c1da7e 100644 --- a/tests/lifecycle-process-delivery.test.ts +++ b/tests/lifecycle-process-delivery.test.ts @@ -7,6 +7,7 @@ import { serveLifecycleProcessDelivery, } from "../src/lib/lifecycle-process-delivery.ts"; import { run } from "../src/lib/shell.ts"; +import { privateCliRunEnv } from "./helpers/private-cli-env.ts"; const roots = new Set(); let unknown = false; @@ -50,6 +51,12 @@ async function fixture() { await mkdtemp(resolve(tmpdir(), "hack-host-delivery-")) ); roots.add(root); + // The client CLI's private HOME and Docker config live outside `root`, + // which a test expects to be empty after the handoff. + const cliRoot = await realpath( + await mkdtemp(resolve(tmpdir(), "hack-host-delivery-cli-")) + ); + roots.add(cliRoot); const abort = new AbortController(); const deadline = performance.now() + 5000; const remaining = () => { @@ -59,7 +66,7 @@ async function fixture() { } return result; }; - return { root, abort, remaining }; + return { root, cliRoot, abort, remaining }; } test("private delivery keeps per-process values out of mux command arguments and preserves exact exit", async () => { const f = await fixture(); @@ -92,8 +99,13 @@ test("private delivery keeps per-process values out of mux command arguments and let exited = false; unknown = true; try { + // The client gets a private env that carries LEAKED_PARENT; the launched + // command must still not see it. const code = await run(args, { - env: { LEAKED_PARENT: "synthetic-inherited-value" }, + ...privateCliRunEnv({ + root: f.cliRoot, + env: { LEAKED_PARENT: "synthetic-inherited-value" }, + }), stdin: "ignore", stdout: "ignore", stderr: "ignore", @@ -171,6 +183,7 @@ test("source drift at delivery claim refuses before the client command", async ( delivery.path, ], { + ...privateCliRunEnv({ root: f.cliRoot }), stdin: "ignore", stdout: "ignore", stderr: "ignore",