fix(security): set strictMcpConfig to block cloned-PR .mcp.json auto-load by chrisleekr · Pull Request #210 · chrisleekr/github-app

chrisleekr · 2026-06-03T08:26:03Z

What

Closes #196. Follow-up to #191 / PR #195. That PR pinned settingSources: [] to close the .claude/settings.json SessionStart-hook RCE in the cloned PR working tree. This closes the sibling vector: a hostile .mcp.json committed in the same tree.

Determination (verified, not assumed)

settingSources: [] does not gate project .mcp.json discovery:

The agent-sdk arg builder (node_modules/@anthropic-ai/claude-agent-sdk/sdk.mjs) maps strictMcpConfig → the CLI --strict-mcp-config flag, mcpServers → --mcp-config, and settingSources → --setting-sources.
The Claude Code CLI reference --bare flag ("skip auto-discovery of hooks, skills, plugins, MCP servers, auto memory, and CLAUDE.md") shows MCP servers are auto-discovered by default and are a distinct item from settings/memory (what settingSources controls).
--strict-mcp-config restricts MCP loading to the --mcp-config set, ignoring all other configuration including project .mcp.json.

So with cwd = the cloned PR tree + permissionMode: bypassPermissions, a hostile .mcp.json was auto-discovered and loaded — registering a stdio MCP server whose command runs on connect, the same RCE primitive class as #191.

Fix

src/core/executor.ts — add strictMcpConfig: true to queryOptions, next to the settingSources: [] pin, with an inline comment recording the determination (the TS docstring only mentions validation, but the flag it emits also suppresses discovery).
test/core/executor.test.ts — regression test: options.strictMcpConfig === true, the injected mcpServers are still forwarded (no-loss), and settingSources stays [].

No-loss: the bot supplies all its MCP servers explicitly via mcpServers; it never relies on a project .mcp.json, so this keeps every legitimate server and blocks only the attacker-controlled file.

Acceptance criteria

✅ Determined: project .mcp.json at cwd IS auto-loaded independent of settingSources: [].
✅ strictMcpConfig: true added with a regression test; injected mcpServers preserved.

Out of scope (residual, operator-controlled, not cloned-tree)

~/.claude.json, plugin-provided, and MCPB-bundled MCP servers live in operator-controlled locations, not the attacker-controllable PR tree. Both cloned-tree attacker config classes (settings.json, .mcp.json) are now closed.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved MCP server configuration controls to prevent unintended auto-registration from project files, ensuring only explicitly specified servers load.

…load settingSources:[] (issue #191) gates settings.json/CLAUDE.md but not a project .mcp.json, which the CLI auto-discovers from cwd by default. With cwd = the cloned PR tree under bypassPermissions, a hostile .mcp.json could register a stdio MCP server whose command runs on connect, the same RCE primitive class as #191. strictMcpConfig:true emits --strict-mcp-config, restricting MCP loading to the explicitly-injected mcpServers. Closes #196 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

coderabbitai · 2026-06-03T08:26:11Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 44f3c61a-5362-457c-aea9-2560bbbd412f

📥 Commits

Reviewing files that changed from the base of the PR and between 5407dcd and 3d625f6.

📒 Files selected for processing (2)

src/core/executor.ts
test/core/executor.test.ts

📝 Walkthrough

Walkthrough

This PR hardens MCP (Model Context Protocol) server discovery in executeAgent by setting strictMcpConfig: true, which blocks auto-registration of .mcp.json files committed to the project and restricts MCP loading to explicitly provided mcpServers. The implementation adds inline documentation clarifying the interaction with settings discovery, and corresponding test coverage verifies the configuration is set correctly.

Changes

MCP Configuration Hardening

Layer / File(s)	Summary
Executor strictMcpConfig implementation `src/core/executor.ts`	`executeAgent` sets `strictMcpConfig: true` in SDK query options with expanded documentation explaining that `.mcp.json` discovery under `workDir` is blocked and MCP loading is restricted to executor-provided `mcpServers`.
Test infrastructure and MCP hardening assertions `test/core/executor.test.ts`	`QueryCall` interface is extended with `strictMcpConfig`, `settingSources`, and `mcpServers` fields, and a new test case verifies `executeAgent` sets `strictMcpConfig` to `true`, preserves injected `mcpServers`, and keeps `settingSources` empty.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

chrisleekr/github-app-playground#196: This PR directly implements the MCP config hardening control by setting strictMcpConfig: true in executeAgent and adding corresponding test assertions for this setting and the related mcpServers/settingSources behavior.

Possibly related PRs

chrisleekr/github-app-playground#195: Both PRs modify src/core/executor.ts's executeAgent SDK query options around settingSources field and settings/MCP discovery interaction, with this PR building directly on the earlier filesystem-settings isolation change.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding strictMcpConfig to block .mcp.json auto-loading in cloned PRs, which is the primary security fix across both modified files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR hardens the Claude Agent SDK invocation against hostile MCP auto-discovery from an attacker-controlled cloned PR working directory by forcing MCP loading to only the explicitly injected mcpServers set.

Changes:

Set strictMcpConfig: true in the Agent SDK queryOptions to suppress project .mcp.json auto-discovery.
Add a regression test asserting strictMcpConfig is enabled while preserving mcpServers forwarding and the existing settingSources: [] pin.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`src/core/executor.ts`	Pins `strictMcpConfig: true` alongside `settingSources: []` to prevent cloned-tree `.mcp.json` auto-load.
`test/core/executor.test.ts`	Adds a regression test ensuring `strictMcpConfig` is set and `mcpServers`/`settingSources` are preserved.

Make the #196/#191 assertions type-visible instead of relying on the captured object's runtime shape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

# [1.14.0](v1.13.0...v1.14.0) (2026-06-30) ### Bug Fixes * **agent-sdk:** pass settingSources [] so SDK ignores cloned PR .claude/settings.json ([#195](#195)) ([153fef3](153fef3)) * **check:** derive scoped-executor scan set from filesystem (dead guard) ([#208](#208)) ([8b0e8ef](8b0e8ef)) * **daemon:** sweep full workspace triple on startup and crash exit ([#239](#239)) ([56fa714](56fa714)) * **idempotency:** gate side-effecting handlers with Valkey claim to prevent redelivery duplicates ([#212](#212)) ([68dacdb](68dacdb)) * **infrastructure:** patch krb5 HIGH CVE-2026-40356 in shared Docker base ([#180](#180)) ([1d1bc3b](1d1bc3b)) * **mcp:** redact Octokit error tool-results and widen GitHub token regex ([#238](#238)) ([675d610](675d610)) * **mcp:** wrap GitHub-touching MCP servers + state-fetchers in retryWithBackoff ([#205](#205)) ([319beb9](319beb9)) * **observability:** canonicalise child-logger entity id under entityNumber ([#178](#178)) ([808ca46](808ca46)) * **security:** gate LLM scanner redacted_body to deletion-only ([#206](#206)) ([d52cf78](d52cf78)) * **security:** set strictMcpConfig to block cloned-PR .mcp.json auto-load ([#210](#210)) ([2c58ec1](2c58ec1)) * **testing:** run colocated src/**/*.test.ts in CI + add drift guard ([#204](#204)) ([5990e0d](5990e0d)) ### Features * **agent-sdk:** block destructive Bash at runtime via PreToolUse hook ([#241](#241)) ([f3132f2](f3132f2)) * **observability:** add 12 structured Pino event families with Zod-strict schemas ([#251](#251)) ([eaad36b](eaad36b)) * **observability:** add queue_wait_ms to dispatcher offer/no-daemon logs ([#207](#207)) ([7a5cfb0](7a5cfb0)) * **observability:** add structured retry.* events ([#225](#225)) ([6713cbf](6713cbf)) * **observability:** emit failed_stage and failed_stage_delta_ms on pipeline.failed ([#244](#244)) ([4f2483c](4f2483c)) * **observability:** emit structured idempotency events on all 4 claimDelivery outcomes ([#242](#242)) ([e1e7f9e](e1e7f9e)) * **observability:** installationId on loggers + config-free MCP retry ([#189](#189)) ([ba09f76](ba09f76)), closes [#177](#177) [#184](#184) * **observability:** log + persist SDK token usage on executions ([#209](#209)) ([5407dcd](5407dcd)) * **observability:** log octokit rate-limit headers via hook.after ([#183](#183)) ([30e1715](30e1715)) * **observability:** periodic fleet-state gauge snapshot ([#186](#186)) ([7429460](7429460)) * **observability:** redact crash logs via uncaughtException/unhandledRejection handlers ([#181](#181)) ([d4248f4](d4248f4)) * **observability:** structured dispatcher + heartbeat log events ([#188](#188)) ([c615bc4](c615bc4)) * **observability:** structured pino logger for stdio MCP servers ([#185](#185)) ([5244e8a](5244e8a)) * **observability:** structured pipeline.stage timing events with delta_ms ([#182](#182)) ([4125971](4125971))

chrisleekr · 2026-06-30T12:41:45Z

🎉 This PR is included in version 1.14.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Copilot AI review requested due to automatic review settings June 3, 2026 08:26

github-actions Bot added the type: fix 🐞 label Jun 3, 2026

Copilot started reviewing on behalf of chrisleekr June 3, 2026 08:26 View session

Copilot AI reviewed Jun 3, 2026

View reviewed changes

Comment thread test/core/executor.test.ts

test: declare security-pin fields on the QueryCall options interface

3d625f6

Make the #196/#191 assertions type-visible instead of relying on the captured object's runtime shape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

chrisleekr merged commit 2c58ec1 into main Jun 3, 2026
22 checks passed

chrisleekr deleted the fix/issue-196 branch June 3, 2026 08:57

chrisleekr added the released label Jun 30, 2026

Navigation Menu

fix(security): set strictMcpConfig to block cloned-PR .mcp.json auto-load - #210

fix(security): set strictMcpConfig to block cloned-PR .mcp.json auto-load#210
chrisleekr merged 2 commits into
mainfrom
fix/issue-196

chrisleekr commented Jun 3, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 3, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

chrisleekr commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

chrisleekr commented Jun 3, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Determination (verified, not assumed)

Fix

Acceptance criteria

Out of scope (residual, operator-controlled, not cloned-tree)

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

chrisleekr commented Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chrisleekr commented Jun 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 3, 2026 •

edited

Loading