mcpx testing: e2e inherits the caller's environment -- build it from an allowlist so tests never reach the user's daemon #1992

Relocated from dezren39/darwin-nix#138 on 2026-10-28 (placement audit): mcpx's single source lives in developing-today/code (pkgs/mcpx); re-home tracked at #309. Original body below; issue references inside it are historical (dezren39/nix numbering).

Parent: #97

Summary

The e2e harness starts every test's environment from the developer's own os.Environ(), then overrides four variables. Anything else set in the caller's shell reaches the test binary. That includes MCPX_DAEMON_ENDPOINT, which the opencode mcpx plugin injects into every shell it opens. So an agent running go test ./internal/e2e inside opencode can point the suite at the user's real daemon. Separately, several code paths read $HOME/XDG_* directly, and the harness does not isolate those.

The fix: build the child environment from an allowlist, not by inheriting and overriding, and add a guard that catches new MCPX_* leaks. (The short autostart idle-exit this issue also asked for landed in #182, which fixed #52; the inheritance itself is untouched and is what remains.)

This is the precondition for "very few shouldn't work if user has own real daemon running" (#134): isolation has to be strong enough that "very few" is true.

Original notes

• testing
  • a general goal is to ensure all features have one or more integration tests
    • for both binary && app entrypoint (consider binary a superset)
      • all tests should pass for nix flake & container
        • all local also but may need impractical setup
          • very few shouldn't work if user has own real daemon running

Current state (written on d0b7e01; re-checked at 05c78b2)

newEnv (internal/e2e/e2e_test.go:35-87 at 05c78b2) sets envVars: append(os.Environ(), MCPX_REGISTRY_URL=..., MCPX_STATE_DIR=..., MCPX_CACHE_DIR=..., MCPX_CONFIG=..., MCPX_AUTOSTART_IDLE_EXIT=60s) (:52-69). os/exec keeps the last value for a duplicated key, so these five win. Everything else is still inherited -- #182 added the fifth variable and changed the cleanup to stop --all with a logged error (:71-85), but did not touch the os.Environ() base. A developer's or agent's MCPX_DAEMON_ENDPOINT, MCPX_SOCKET or any other MCPX_* still reaches every test.

The table below comes from reading the code; the suite was not run for this issue. The first implementation step confirms these reads with a test.

Related, from #52 (closed, fixed in #182): newEnv now sets autostart.idleExit (internal/settings/wire.go:135) to 60 s through MCPX_AUTOSTART_IDLE_EXIT (e2e_test.go:68, testIdleExit at :92), and cleanup runs mcpx stop --all through the daemon-<key>.json info files, logging rather than discarding its error (:71-85). That bounds a leak; it does not stop a test from reaching the caller's daemon, which is this issue.

The in-process runner (#133) inherits all of this through os.Getenv in the same process. The fix below has to apply to both runners.

Approaches

A -- denylist: unset known-dangerous variables (least recommended)

Strip MCPX_DAEMON_ENDPOINT and MCPX_SOCKET after os.Environ(). It is the smallest diff, and it fixes today's concrete hole. It fails open: every future MCPX_* setting, and every new XDG_*/HOME read, leaks again until someone notices. That is the pattern the history shows (#65 registry, #52 leaks). Not recommended as the end state.

B -- strip every MCPX_* and override HOME/XDG_*

Drop every inherited variable that starts with MCPX_, and set HOME, XDG_CONFIG_HOME, XDG_DATA_HOME, XDG_STATE_HOME, XDG_CACHE_HOME and XDG_RUNTIME_DIR under e.dir (short names, for the 100-byte socket cap, paths.go:75). This is robust against new settings. It still inherits unrelated variables: DENO_DIR, BUN_INSTALL, NODE_OPTIONS, OPENCODE_*, proxies. Some of those are wanted (runtime caches), some are not (NODE_OPTIONS=--require ...).

C -- allowlist plus a guard (recommended)

• Child environment = an explicit allowlist from the parent: PATH, TMPDIR, LANG/LC_*, TERM, SYSTEMROOT (for Windows later, Update dependency happy-dom to v21 [SECURITY] #136), and runtime cache dirs (DENO_DIR, BUN_INSTALL_CACHE_DIR, npm cache) so runs stay fast. Add B's HOME/XDG_* under e.dir, then the five variables newEnv already sets (including Phase 5: tag event bodies must travel with the event #182's MCPX_AUTOSTART_IDLE_EXIT=60s).
• Opt-in pass-through for the rare test that needs the outer world: e.inherit("NAME"). The tests labelled "needs the user's default paths" (Update Rust crate rand to v0.9.3 [SECURITY] #134, RequireNoUserDaemon) are the ones that use it.
• Guard test, TestTheHarnessInheritsNoMCPXVariable. It sets MCPX_DAEMON_ENDPOINT=http://127.0.0.1:1, MCPX_SOCKET=/nonexistent, MCPX_SCRIPT_RUNTIME=bogus and HOME=/nonexistent in its own process (t.Setenv), builds an env, runs mcpx doctor -v, mcpx settings list and mcpx recipes, and asserts the output is local, default and home-free. This is the regression test that makes the fix stay fixed (Bump unsafe-libyaml from 0.2.8 to 0.2.10 in /sources/pavex/examples/realworld #99: a test per bug, a guard per bug class). Make its variable list derived, not typed (from Phase 2: authority judged at causal past or canonical prefix #162): enumerate the settings registry and require every EnvName() and EnvAliases entry (internal/settings/schema.go:220-224,284) to be either set by the harness or on an explicit safe-to-inherit list, so a new setting is covered the day it is added. Undeclared MCPX_* reads are not in the registry; take them from Phase 2: membership, delegation, groups, and server config #172's allowlist once it lands rather than keeping a second list here.
• Backstop: TestMain in internal/e2e fails fast, with a clear message, if the parent has MCPX_DAEMON_ENDPOINT or MCPX_SOCKET set and the allowlist is somehow bypassed. That turns a silent "tests passed against the user's daemon" into a loud failure.
• Machine-level check (from Phase 2: authority judged at causal past or canonical prefix #162): TestMain records mcpx daemons for the caller's real environment at start and end and fails on any difference. It is the one assertion of "the suite does not disturb your machine" that cannot rot, and it also catches a regression of Phase 5: tag event bodies must travel with the event #182's leak fix.

Recommended because it fails closed. A new setting, or a new read of $HOME, is isolated by default, and the guard catches the regression if someone widens the allowlist.

Implementation steps

1. Write the guard test first, expecting it to fail on main. That confirms the table above.
2. Replace append(os.Environ(), ...) with the allowlist builder in internal/testsupport (shared with the plugin harness in Update dependency @chenglou/pretext to ^0.0.9 [SECURITY] #132 and the in-process runner in Update Rust crate bytes to v1.11.1 [SECURITY] #133).
3. Add MCPX_AUTOSTART_IDLE_EXIT=30s, and record the stop error rather than discarding it (chore(deps): bump build/buck/preludes/buck2-prelude from d956689 to e7457f6 #52 fixes 1 and 2). Done in Phase 5: tag event bodies must travel with the event #182 (60 s; stop --all, error logged).
4. Add the TestMain backstop.

Open questions

• Should a caller's DENO_DIR/bun cache be shared with tests (fast) or isolated (hermetic)? Proposal: shared on laptops, isolated in the container entry.
• Does anything need SSH_AUTH_SOCK/git credentials? The repo/worktree scopes use git (package.nix comment), but only locally. (The GIT architect is replacing the git subprocess with native resolution in internal/config/scope.go; if that lands, git may drop out of the allowlist question entirely.)
• Should the plugin's shell.env injection skip go test commands? No: the harness must be robust against any caller, and special-casing a command name in the plugin is the wrong layer.

Ease · Impact · Related

Ease S-M · Impact high (a correctness hole in every e2e run started from opencode with the plugin, and the foundation for the "user's daemon" subset)
Depends on / related: #52 (closed, fixed in #182 -- step 3 is done), #139 (this is the remaining precondition for t.Parallel in e2e), #88 (owns MCPX_SOCKET precedence, #181 collision 8), #172 (the undeclared MCPX_* allowlist the guard should reuse), #134 (the labelled subset; the inline/spawn/remote run modes), #133 (the in-process runner needs the same env), #132 (the plugin harness), #99 (guard-per-bug-class), #65 (the registry precedent for "never the real thing").

Security/auth deferred; see tracker.

Acceptance criteria

• The e2e harness builds the child environment from an allowlist (no inherited MCPX_* or caller daemon endpoints), a guard test catches new MCPX_* leaks, and CI is green.
• The work described in the Summary/Scope above is delivered via a branch + PR (no direct push to main); verification runs in GitHub Actions CI, not on the sandbox.
• Evidence (CI links, timings, or probe output as applicable) is recorded on this issue before it is closed.