feat(base-action): default enableAllProjectMcpServers to false, add opt-in input

[OctavianGuzu]

Three related changes to how the action and base-action read project-level configuration:

• enableAllProjectMcpServers defaults to false in base-action. The wrapper action auto-enables it only when project config has been restored from the base ref (i.e., when running on a PR and restoreConfigFromBase ran). Otherwise it's opt-in via the new enable_all_project_mcp_servers input.
• base-action's bun run step now pins --config/--tsconfig-override/--no-env-file to the action's own files, matching the main action's invocations. Adds base-action/bunfig.toml so the flag resolves when base-action is used standalone.
• bunfig.toml added to the base-ref restore list alongside the other config files in SENSITIVE_PATHS.

Adds a resolveEnableAllProjectMcpServers helper + tests. Handles the workflow_run event case in the restore-config gate.

[claude]

Implementation looks solid and all my earlier feedback has been addressed — deferring to a human only because this is an intentional breaking change to a security-relevant default for standalone base-action consumers, which warrants a maintainer sign-off.

Extended reasoning...

Overview

This PR changes setupClaudeCodeSettings() so that enableAllProjectMcpServers is no longer hardcoded to true; it is now driven by a new enable_all_project_mcp_servers input. The base-action defaults the input to false (a behavior change), the wrapper action defaults it to true (no behavior change, since .mcp.json is already restored from the PR base ref via restoreConfigFromBase). Touches both action.yml files, both entrypoints (base-action/src/index.ts, src/entrypoints/run.ts), the settings helper, its unit tests, the MCP integration workflow, and four doc surfaces.

Security risks

The change is a security hardening: it stops the standalone base-action from auto-approving every MCP server in a PR-controlled .mcp.json. The new input is a plain boolean string compared with === "true", plumbed via the standard composite-action env: mapping — no injection surface. The wrapper keeps its prior behavior, justified by the existing restoreConfigFromBase mitigation. I see no new exposure introduced.

Level of scrutiny

Medium-high. The code change is mechanically simple (one new optional parameter, one boolean plumbed through two action manifests), but it sits on the MCP trust boundary and is an intentional breaking change for anyone calling anthropics/claude-code-base-action directly with a project .mcp.json. The split default (false for base-action, true for wrapper) is a reasonable design but is exactly the kind of policy decision a maintainer should explicitly ratify rather than have auto-approved.

Other factors

Over four review rounds I flagged stale doc notes, a missing docs/usage.md row, and two CI regressions in test-mcp-servers.yml (including the latent mcp_config plumbing gap) — all were fixed in f3f6fb1, cf66f1d, 9d2286e, and 52fafff and the threads are resolved. Unit tests now cover both the new default-false and explicit opt-in paths, plus the override-precedence over settings JSON. The current bug-hunt pass found nothing further. The only residual item is the pre-existing mcp_config plumbing gap, which the author has explicitly scoped out with a TODO and a follow-up — that's reasonable.

[claude]

All my earlier feedback has been addressed and I found nothing new — but this is an acknowledged breaking change to a security-relevant default (with the open "uniform rule vs. special-case trusted non-PR triggers" design call you flagged), so a maintainer should sign off rather than me.

Extended reasoning...

Overview

This PR flips enableAllProjectMcpServers from a hardcoded true to an opt-in: false by default in the base-action, and gated on restoreConfigFromBase in the wrapper. It adds a new enable_all_project_mcp_servers input to both action.yml files, threads it through base-action/src/index.ts → setup-claude-code-settings.ts and src/entrypoints/run.ts, adds resolveEnableAllProjectMcpServers() to restore-config.ts, and updates tests, docs (README, configuration.md, usage.md), and the test-mcp-servers.yml CI workflow. 12 files touched.

Security risks

The change moves in the safer direction (stops auto-trusting checkout-controlled .mcp.json), so it does not introduce new attack surface. The remaining risk is policy/design: the wrapper now defaults to false on non-PR triggers (push/schedule/workflow_dispatch/issues), which are trusted-checkout contexts where the original threat model doesn't apply. The author intentionally chose a uniform rule over special-casing those and explicitly invited maintainer input on that tradeoff.

Level of scrutiny

High. This is (a) an acknowledged breaking change for existing non-PR wrapper users relying on in-repo .mcp.json, (b) a change to a security-relevant default, and (c) touches both public action input surfaces and the main run.ts entrypoint. Breaking changes to published action defaults warrant human maintainer sign-off, not bot approval.

Other factors

Across six prior review rounds I raised stale-doc notes, two CI regressions, a JSDoc-ordering nit, and the non-PR-trigger behavior question — all were addressed (commits f3f6fb1, cf66f1d, 9d2286e, 52fafff, 9452710, 7942438, 47f4332) and the PR description was updated with an accurate behavior matrix and breaking-change section. Unit tests cover both the new setupClaudeCodeSettings parameter and resolveEnableAllProjectMcpServers. The implementation looks correct to me; I'm deferring purely on scope/breaking-change grounds, not correctness.

[claude]

All prior feedback is addressed and I found nothing new, but this changes a security default, adds a public input to both action manifests, and ships a documented breaking change for non-PR triggers (with an open design question on whether to special-case merged-code contexts) — that combination should get a maintainer's eyes before merge.

Extended reasoning...

Overview

This PR flips the enableAllProjectMcpServers default from hardcoded true to opt-in, threading a new enable_all_project_mcp_servers input through both base-action/action.yml and the wrapper action.yml. The wrapper auto-enables it only when restoreConfigFromBase ran (PR-entity contexts); all other triggers now default to false. It touches 13 files: both action manifests, src/entrypoints/run.ts, src/github/operations/restore-config.ts (new resolveEnableAllProjectMcpServers helper + bunfig.toml added to SENSITIVE_PATHS), base-action/src/{index,setup-claude-code-settings}.ts, three doc files, two test files, the MCP CI workflow, and a new base-action/bunfig.toml. The latest commit (219a2de) also pins bun's --config/--tsconfig-override in base-action/action.yml to the action directory.

Security risks

The change is itself a security hardening — it stops auto-trusting a checkout-controlled .mcp.json. The direction is strictly safer than before. The new bun-config pinning and adding bunfig.toml to the base-ref restore list close a related workspace-config surface. I see no new injection, auth, or data-exposure risk introduced; the input is a plain boolean string compared with === "true"/=== "false".

Level of scrutiny

High. This is (a) a change to a security-relevant default, (b) a new public input on both published actions, and (c) an acknowledged breaking change for wrapper users on push/schedule/workflow_dispatch/issues triggers. The author explicitly left a design choice open ("happy to revisit if we'd rather special-case the merged-code triggers") — whether to keep the uniform rule or default trusted non-PR checkouts back to true is a product/security tradeoff a maintainer should ratify, not a bot.

Other factors

Seven rounds of bot review surfaced two CI regressions, an orphaned JSDoc, and four stale-doc spots — all fixed and resolved. Unit tests cover both setupClaudeCodeSettings and the new resolver; the MCP integration workflow was opted in. The diff is internally consistent and well-documented now, but the breaking-change scope plus the open design question put this outside what I'll approve without a human.

[claude]

No issues found in the final state — all earlier review threads are resolved. Deferring to a maintainer because this changes a security-relevant default (auto-enabling in-repo .mcp.json servers) with breaking-change implications for workflow_run and base-action-standalone users, and the trust-gating design iterated several times during review; worth a human sign-off on the final semantics.

Extended reasoning...

Overview

This PR changes how enableAllProjectMcpServers is resolved across both the base-action and the wrapper action, adds an enable_all_project_mcp_servers opt-in input, pins bun's --config/--tsconfig-override/--no-env-file in base-action, adds bunfig.toml to SENSITIVE_PATHS, and pins --setting-sources user for plugin install. It touches 16 files: both action.yml definitions, run.ts, restore-config.ts, setup-claude-code-settings.ts, install-plugins.ts, four docs files, two test files, a CI workflow, and a new bunfig.toml + test/restore-config.test.ts.

Security risks

The whole PR is about a security-relevant default — whether to auto-approve every MCP server in the checkout's .mcp.json. The final design is strictly tighter than main (where it was hardcoded true): the base-action now defaults to false under pull_request_target/workflow_run/issue_comment, and the wrapper only forces true when it has positively established the project config is merged code (PR-restored, or non-PR entity event). Nothing here widens an attack surface; the risk is in getting the trust gating right, and the two-layer resolver (wrapper → true/undefined, base-action ??= !isPrivilegedExternalEvent()) is subtle enough that a maintainer should confirm the final event-by-event matrix matches intent.

Level of scrutiny

High. This is a security-hardening change to a published GitHub Action's default behavior, with a documented breaking change for some triggers. The design flipped several times during review (wrapper default true → gated on configRestoredFromBase → bare true → gated on projectConfigTrusted deferring to base-action), so the docs/PR-body/behavior alignment deserves a final human read. Not a candidate for bot auto-approval.

Other factors

All twelve of my earlier inline threads are resolved and the bug hunter found nothing on the current head (fa0de7b). Unit tests cover both resolvers and the event-gated defaults. The one residual nit — action.yml/docs/usage.md/docs/configuration.md still phrase the wrapper default as a flat "Defaults to true" — is now mostly accurate after the final design (only workflow_run deviates), so I'm not re-raising it. The pre-existing mcp_config plumbing gap is correctly noted as a TODO and out of scope.