Repository navigation
Conversation
…in backstop (#1992) Replace the inherit-minus-MCPX_* harness environment with a true allowlist builder in internal/testsupport (ChildEnv): the parent contributes PATH, TMPDIR, LANG/LC_*, TERM, SYSTEMROOT and the runtime-cache passthroughs only; XDG_* resolve under the test dir and everything else -- MCPX_*, XDG_*, OPENCODE_*, proxies, NODE_OPTIONS -- is dropped. newEnv builds on it. Add the registry-derived guard: every settings EnvName()/EnvAliases entry is classified as harness-set or safe-under-inherit-nothing, with a behaviour half proving the drop for each one, plus the hostile-parent doctor/settings/recipes regression test. Add the TestMain backstop: fail fast if MCPX_DAEMON_ENDPOINT or MCPX_SOCKET would reach a child, and fail if the caller's real daemon set differs at suite end.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 39 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. CodeRabbit Verify
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Follow-up for #1992 (task 3284).
What changed
pkgs/mcpx/internal/testsupport/childenv.go(new):ChildEnvallowlist builder -- parent contributes PATH, TMPDIR, LANG/LC_, TERM, SYSTEMROOT and explicit runtime-cache passthroughs (DENO_DIR, BUN_INSTALL_CACHE_DIR, NPM_CONFIG_CACHE) only; XDG_CONFIG_HOME/XDG_DATA_HOME/XDG_STATE_HOME/XDG_CACHE_HOME/XDG_RUNTIME_DIR resolve under the test dir. Everything else (all MCPX_, XDG_, OPENCODE_, proxies, NODE_OPTIONS, credentials) is dropped. Unit tests are inchildenv_test.go.pkgs/mcpx/internal/e2e/e2e_test.go:newEnvbuilds onChildEnv(replaceswithoutMCPXVars); newTestMainbackstop fails fast if MCPX_DAEMON_ENDPOINT/MCPX_SOCKET would reach a child and fails if the caller's real daemon set (info records in the parent-resolved state dir) differs at suite end.pkgs/mcpx/internal/e2e/envguard_test.go(new): registry-derived guard -- every settingsEnvName()/EnvAliasesentry (231 names) is classified harness-set (52) or safe-under-inherit-nothing (179); the behavior test plants canaries for every name plus XDG/OPENCODE/proxy/NODE_OPTIONS/HOME and checks isolation; hostile-parent doctor/settings/recipes regression test.Verification status
gofmtandgit diff --check: pass. No Go build/test was run locally (task requires CI verification only).basic: production line limit (2500)(pass); it does not test Go.mcpx testandmcpxworkflows aredisabled_manually. Manual dispatch ofmcpx-test.ymlon this branch was rejected by GitHub (HTTP 422, workflow disabled), so the e2e andgo test ./...CI evidence, and empirical guard fail-on-main/pass-on-fix evidence, are still blocked. Do not close mcpx testing: e2e inherits the caller's environment -- build it from an allowlist so tests never reach the user's daemon #1992 until an enabled Actions path runs and its result is recorded here.