Skip to content

test: give source CLI spawns a private environment - #319

Merged
roodboi merged 3 commits into
nextfrom
wf/d1-test-env-isolation-b12
Oct 13, 2026
Merged

test: give source CLI spawns a private environment#319
roodboi merged 3 commits into
nextfrom
wf/d1-test-env-isolation-b12

Conversation

@roodboi

@roodboi roodboi commented Oct 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Draft. Test-only change. No CLI surface, source or docs change.

Problem

Many tests ran the source CLI (process.execPath index.ts ...) with env: { ...process.env, ... }, or with no env at all. The child CLI then saw the contributor's real HOME and every inherited variable: HACK_*, DOCKER_HOST, DOCKER_CONTEXT, DOCKER_CONFIG, XDG_*, CI and so on. A few of these tests also had no HACK_HOME, so the child used the real ~/.hack (for example native-ca-doctor-cli, native-project-down, native-project-exec and env-backend-command).

What changes

  • New tests/helpers/private-cli-env.ts with privateCliEnv({ root, home?, env?, isolateDockerClient? }). It builds the whole child environment:
    • HOME is <root>/user-home or a given path inside the fixture root. Any other path throws, and env.HOME is refused. The directory is created if needed, and its mode is set to 0700 even when the test created it first.
    • Only PATH and TMPDIR are inherited from the runner. Everything else must be passed by the test.
    • By default it adds the existing isolated Docker client config (isolatedDockerClientEnv), so no real engine or context is selected. A DOCKER_HOST/DOCKER_CONFIG passed by the test still wins.
  • Migrated every source-CLI spawn that copied the runner environment or passed no env:
    • The seven named native-compose harnesses: native-compose-open, -routing-command, -routed-run-command, -saved-command (4 sites), -before-command, -adoption-generation, plus -build-command and -network-command.
    • Other spawns without a HOME override: doctor-domain-migration, env-backend-command (29 call sites, through runHack), native-ca-doctor-cli, native-config-import-preview, native-project-down (3), native-project-exec, native-routing-project, prerelease-plan (the index.ts --version spawn), project-env-local-inheritance.
    • Spawns that already set HOME but still inherited everything else: doctor-command, mcp-lifetime, mcp-prompts, mcp, native-hack-local-route-cli, project-env-worktree-key, e2e/run-dependency-cache.
    • helpers/project-name-fixture.ts: its env (HOME = fixture root) feeds the CLI spawns in project-name-command, local-network-hint and local-network-hint-call-sites, and the git spawns in projects-registry-identity. It now comes from privateCliEnv.
    • Spawns with no env, which inherited the whole runner environment and real HOME: doctor-browser-network, log-pipe (2), mcp-entrypoint (2), and host-exec-tty (the Python probe that runs bun index.ts with the env it was given, and the probe of the compiled binary).
    • mcp.test.ts: the env of the in-process MCP server, which spawns HACK_MCP_COMMAND with it.
    • dispatch-e2e.test.ts (opt-in, HACK_DISPATCH_E2E=1): its three CLI runs and its git runs now use privateCliEnv with the test's temporary HOME.
    • The three harnesses perf(storage): reuse unchanged storage witness proofs across warm commands #313 already fixed (helpers/native-compose-command.ts, helpers/native-compose-files.ts, native-compose-storage-activation-command) now use the helper too. Their env is the same as before, except that TMPDIR is inherited and the files helper now also gets the isolated Docker client config.
    • Tests that set HACK_HOME, the compiler path or a secret key in-process now pass those values explicitly.
    • native-project-down puts the CLI's private HOME and Docker config in a separate temporary directory, not inside the project checkout under test.
  • New guard tests/cli-spawn-env-isolation.test.ts, with the scan in tests/helpers/cli-spawn-env-scan.ts. The scan reads each file's TypeScript syntax tree (the typescript package that typecheck already installs).
    • Scope: files that name the source CLI (a string ending in index.ts, CLI_ENTRYPOINT or resolveCliSpawnArgs), plus every test-local module they import, transitively. So an env that a fixture helper builds, such as project-name-fixture.ts, is checked where it is built.
    • runner-env: no copy of the whole runner environment. Any 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 object. This covers ...process.env whatever HOME is set to, env: process.env, ...Bun.env, Object.assign({}, process.env), const saved = process.env and rest destructuring.
    • runner-home: no HOME property whose value comes from process.env.HOME, homedir() in any import form (os.homedir() included), userInfo().homedir, or a same-file binding of one of these.
    • cli-without-env: a Bun.spawn/spawnSync or node:child_process spawn whose arguments reach the CLI must pass env. A wrapper whose argv names the CLI, such as the Python probe, counts.
    • env-without-home: when that env is an object literal, it must set HOME or spread another object. Without HOME the child falls back to the account's real home.
    • Documented exceptions, each with an exact finding count so a new finding still fails, and a check that each exception still exists:
      • tests/e2e/harness.ts (1): runCommand runs git/docker for the real-engine E2E suite; its CLI calls use buildCliEnv.
      • tests/prerelease-plan.test.ts (4): two env objects for scripts/prerelease-plan.ts, one tar env, and one bun build index.ts --compile. None of them runs the CLI.
      • tests/e2e/scenarios/native-routing-fixture-ingress.ts (1): the real-engine docker TLS probe env, not the CLI.
      • tests/host-exec-tty.test.ts (1): bun build index.ts --compile, which does not run the CLI.
    • Limits, documented in the scan module: it 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, or a local run(argv) wrapper whose own spawn never names the CLI). It does not see a CLI path built at runtime without naming index.ts, such as a compiled binary.
  • doctor-command lists the fixture root after the run. That list now includes the test-created docker-client config directory.

Not changed: tests/native-compose-renderer-config.test.ts passes HOME to a real docker compose config client, not the CLI, and does not name the CLI, so it is out of the scan's scope.

Validation

  • Guard mutations on real files: each makes the guard fail, and it passes again after the revert (16 of 16):
    • In native-compose-open, replacing privateCliEnv with env: process.env; no env; { ...Bun.env, HOME }; Object.assign({}, process.env); { ...process.env, HOME: os.homedir() }; { ...process.env, HOME: someVar }; { PATH, HOME: homedir() }; { PATH, HOME: someVar } with const someVar = homedir(); and { PATH } with no HOME.
    • Restoring the previous PR-head version of project-name-fixture, dispatch-e2e, doctor-browser-network, log-pipe, mcp-entrypoint, host-exec-tty or mcp.test.
  • Scan and helper mutations: each fails a guard-file test (14 of 14). They include disabling each rule, the transitive-import scope, child_process import tracking, node:process env aliases, Bun.env, the rest-destructuring check, the CLI-path taint pruning, and the helper's chmod of an existing HOME.
  • Focused runs (local macOS), with the runner's HACK_HOME, DOCKER_CONFIG, HACK_GLOBAL_CONFIG_PATH, DOCKER_HOST and DOCKER_CONTEXT pointed at canary paths: one bun test run of 44 files: all 31 changed test files, the project-name fixture users (project-name-command, local-network-hint, local-network-hint-call-sites, projects-registry-identity, project-name-registry), the perf(storage): reuse unchanged storage witness proofs across warm commands #313 harness users except file-command (after, down, phase-trace, retained-storage, storage-fence-cost, storage-witness, warm-up), and integration-ownership. 595 pass, 4 skip (3 opt-in dispatch-e2e, 1 existing in adoption-generation), 0 fail. The canary directories stayed empty.
  • bun run --cwd packages/cli typecheck: exit 0. bun run --cwd packages/cli check: exit 0, 0 errors (34 existing warnings in src). ultracite check on every changed test file: clean.

Gaps

  • Local macOS runs only. Hosted CI on this PR is the cross-platform check.
  • The local canary run did not also point HOME at a canary. The guard and the helper keep the runner's HOME out of the child.
  • dispatch-e2e.test.ts is opt-in and was only typechecked, scanned and run in its default skipped mode. Its in-process setup can reach the node-token keychain, so it was not run with HACK_DISPATCH_E2E=1.
  • native-compose-file-command needs dist/hack-config-compiler, which this worktree cannot build without the pinned Rust toolchain. It does not call privateCliEnv directly; it uses the perf(storage): reuse unchanged storage witness proofs across warm commands #313 harness, whose env is unchanged apart from the HOME chmod.

🤖 Generated with Claude Code

roodboi and others added 2 commits October 12, 2026 18:32
@roodboi @claude
test: give native-compose CLI harnesses a private environment
b54562d 
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 <noreply@anthropic.com>
@roodboi @claude
test: close remaining runner-env paths to the source CLI
9b90167 
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 <noreply@anthropic.com>
@roodboi roodboi changed the title test: give native-compose CLI harnesses a private environment test: give source CLI spawns a private environment Oct 12, 2026
@roodboi @claude
test: isolate host-exec and lifecycle client CLI spawns
6aeec7e 
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 <noreply@anthropic.com>
@roodboi
roodboi marked this pull request as ready for review October 13, 2026 02:16
@roodboi
roodboi merged commit 519bf90 into next Oct 13, 2026
16 checks passed
@roodboi
roodboi deleted the wf/d1-test-env-isolation-b12 branch October 13, 2026 02:16
@roodboi roodboi mentioned this pull request Oct 13, 2026
Merged
roodboi added a commit that referenced this pull request Oct 14, 2026
@roodboi
test(dns): cover native DNS activate/deactivate admin authorization; …
c29370f 
…add env secrets threat model (#321)

## Summary

HACK-1127 (unit E4). This PR has two parts.

1. It closes the test gap that #320 left on the `hack global dns activate` and `hack global dns
deactivate` administrator prompt. 2. It adds the env and secrets threat model.

## Final behavior

**DNS authorizer seam (`src/lib/native-domain-dns-authorization.ts`, new)**

- The two `authorize` closures in `handleGlobalDnsActivate` and `handleGlobalDnsDeactivate` are
  now one function, `createNativeDnsAuthorizer({ action, domain, dependencies? })`. Its
  injectable dependencies are `isInteractive`, `confirm` and `requestAdministrator`. The
  production defaults are the same calls as before: `process.stdin.isTTY`, `confirmSafe` with
  `nonInteractive: "decline"`, and `requestAdministratorAuthorization`. The confirmation text is
  the same for both actions.
- `requireNativeAdministratorAuthorization` moved out of `src/commands/global.ts` into the new
  module, and it now accepts an injected `requestAdministrator`. When the prompt fails, it
  throws `NativeDnsAdministratorAuthorizationError`. The error carries `outcome: "authorization-
  failed"` and the closed `failure` class, never sudo output. The message text is unchanged.
- `src/commands/global.ts` loses 48 lines. User-visible behavior is unchanged.

**Handler wiring guard (`tests/global-dns-authorizer-wiring.test.ts`, new)**

The handlers need a real macOS host, Homebrew dnsmasq and sudo, so no test runs them. This guard
reads the TypeScript syntax tree of `src/commands/global.ts` and checks each of
`handleGlobalDnsActivate` and `handleGlobalDnsDeactivate`:

- It calls `createNativeDnsAuthorizer` exactly once, with its own `action` string literal and
  `domain`, and stores the result in a `const`.
- That binding is the `authorize` passed to `createNativeDnsHostDependencies`.
- Its flow (`activateNativeDomainDns` or `deactivateNativeDomainDns`) runs with exactly those
  dependencies.
- The only other authorizer allowed is the decline-only `async () => false` on the deactivate
  verify-only path.

It is a syntax check of one file, not data flow. No production code changed in this rework.

**Threat model (`docs/security/env-secrets-threat-model.md`, new; linked from `docs/env.md`)**

The document covers:

- **Assets:** the project env key, encrypted `project_key` YAML, the `encrypted_file` store and
  its key, the `keychain` store, values from 1Password or another external manager, and
  decrypted values in flight.
- **Trust boundaries:** the host user, containers, agents and MCP, CI, and repository inputs.
- **Threats and current mitigations:** argv, log and process-list leakage; symlinks, TOCTOU and
  file modes; hostile ciphertext; public-artifact and git leakage; agent and MCP exfiltration; a
  compromised dependency. Each mitigation cites exact files and symbols from #288, #302, #308,
  #309, #318 and #319.
- **Ten open gaps:**
  1. crash-safe key rotation
  2. the crypto decision record, which is pending independent review (WU14)
  3. live keychain and 1Password custody receipts
  4. the plaintext `.hack/.internal/compose.env.override.yml`, which is written with `Bun.write`
  (umask mode, follows symlinks)
  5. YAML env writes that follow symlinks
  6. `encrypted_file` key-file hygiene
  7. directory TOCTOU
  8. MCP run, exec and log output
  9. container env visibility
  10. node-registry `security -w <value>` argv in the unsupported surface

The doc makes no code change.

## Validation (local, macOS, Bun 1.4.2)

- New `tests/native-domain-dns-authorization.test.ts`: 8 tests. Each one wires the authorizer
  into the real `activateNativeDomainDns` or `deactivateNativeDomainDns` and into
  `createNativeDnsHostDependencies`, with a temp directory, fake privileged and command runners,
  and one ordered effect log. No real sudo, HOME, keychain, Docker or CLI spawn is used.
  - **Activate and deactivate:** the log starts with `confirm`, then a single `sudo -v`. Every
  `sudo -n` step, receipt write and file effect comes after it. The test checks that those
  effects actually ran, so the assertion is not vacuous. The confirmation texts are asserted
  exactly.
  - **Activate and deactivate, failed prompt** (`rejected` exit 1, `timed-out` exit 124):
    - The flow fails in phase `authorization` with `NativeDnsAdministratorAuthorizationError`,
  `outcome: "authorization-failed"`. The message never says "denied".
    - The log is exactly `confirm`, `sudo -v`.
    - No receipt or dnsmasq file is created. For deactivation, the active receipt and both files
  are unchanged.
  - **Activate and deactivate, decline:** the flow fails with "was denied", and the log is
  exactly `confirm`. It never reaches sudo.
  - **Non-interactive:** nothing is prompted and nothing reaches sudo.
  - **Every failure class** (`not-foreground`, `timed-out`, `interrupted`, `rejected`) throws.
  None returns false.
- The same file has 2 more tests for the production defaults. The administrator prompt stays a
  fake in both, so no test reaches real sudo.
  - **Default interactivity check:** with `process.stdin.isTTY` set to undefined or false, the
  authorizer returns false and never confirms. With it set to true, it confirms. The original
  descriptor is restored afterwards.
  - **Default confirmation:** with `HACK_NO_INTERACTIVE=1`, the real `confirmSafe` declines, and
  the authorizer returns false before sudo. The variable is restored afterwards.
- New `tests/global-dns-authorizer-wiring.test.ts`: 2 tests, one per handler.
- Focused suites, 14 files, 153 pass, 1 skip, 0 fail: `global-dns-authorizer-wiring`, `native-
  domain-dns-authorization`, `native-admin-authorization`, `native-domain-dns-host`, `native-
  domain-dns-activate`, `native-domain-dns-deactivate`, `native-hack-local-route-cli`, `doctor-
  env-key`, `cli-reference-docs`, `tests/integration-ownership.test.ts`, `tests/cli-spawn-env-
  isolation.test.ts`, and the other `native-domain-dns-*` and `native-hack-local-route*` suites.
- `bun run --cwd packages/cli typecheck` and root `bun run typecheck`: exit 0.
- `bun run --cwd packages/cli check`: 0 errors. There are 34 existing warnings, none in changed
  files. Biome is clean on the changed TypeScript files.
- The CLI surface and the rendered guidance are unchanged, so `docs:cli-reference`,
  `generate:agent-plugins` and the fingerprint step were not needed.
- I checked that every file path cited in the threat model exists on this head.

## Negative controls

Each mutation was run against the wiring, authorization, activate, deactivate and admin-
authorization suites, then reverted. All 16 are caught.

| Mutation | Result |
| --- | --- |
| `global.ts` activate: `const authorize = async () => true;` | 1 fail |
| `global.ts` deactivate: authorizer built with `action: "activate"` | 1 fail |
| `global.ts` activate: host dependencies get `authorize: async () => true` | 1 fail |
| `global.ts` deactivate: flow gets `{ ...deps, authorize: async () => true }` | 1 fail |
| `global.ts` activate: authorizer built with a different `domain` | 1 fail |
| Default `isInteractive: () => true` | 1 fail |
| Default `isInteractive: () => false` | 1 fail |
| Default confirm `nonInteractive: "accept"` | 1 fail |
| Default confirm `initialValue: true, nonInteractive: "accept-default"` | 1 fail |
| Default confirm always returns true | 1 fail |
| `requireNativeAdministratorAuthorization` returns false instead of throwing | 3 fail |
| Remove the administrator call from the authorizer | 4 fail |
| Ignore a decline | 2 fail |
| Skip the interactive-terminal check | 1 fail |
| Run sudo before the confirmation | 6 fail |
| Throw a plain "denied" `Error` instead of the typed failure | 3 fail |

## Remaining gaps

- **The handlers are guarded by syntax, not run.** The wiring guard proves which authorizer each
  handler installs, but no test executes the handlers themselves. The default
  `requestAdministrator` (real `sudo -v`) is not exercised by any test. `nonInteractive:
  "accept-default"` with `initialValue: false` behaves the same as `"decline"`, so no test can
  tell them apart.
- **No real macOS run.** I did not run `hack global dns activate` or `deactivate` on a real
  host. A real `sudo -v` prompt, a real dnsmasq restart and a real `/etc/resolver` write are
  still unproven for this change.
- **Hosted CI** has not been checked on head `62a9d124`.
- The threat model's open gaps are documented but not fixed. Gaps 4 to 6 are concrete code
  findings that need follow-up changes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@roodboi