Skip to content

fix(security): block Git helpers in read-only MCP tools - #2669

Draft
groupthinking wants to merge 1 commit into
mainfrom
codex/git-mcp-read-helper-guard-20261113
Draft

groupthinking wants to merge 1 commit into
mainfrom
codex/git-mcp-read-helper-guard-20261113

Conversation

@groupthinking

@groupthinking groupthinking commented Nov 13, 2026 •

Copy link
Copy Markdown
Owner

Closes #2668
Refs #2349 (parent remains open).

Verified problem and bounded repair

At base 512a255 the active repository config points at tools/mcp/git_workflow_server.mjs. Its status/diff functions spawn Git with the repository configuration intact. A harmless offline reproduction executed marker helpers through core.fsmonitor, diff.external and textconv. This applies to repositories arriving with a hostile .git/config or altered local configuration; a plain clone does not copy the remote .git/config.

Related primary evidence: GHSA-6hqx-gqrx-c5r2, published November 11, 2026, describes the same class in the separate reference Python server. It does not assign a vulnerability identifier to this custom implementation.

The patch passes --no-pager and core.fsmonitor=false for the two read tools, disables external diffs/textconv explicitly, and ignores dirty submodule inspection. Submodule working-tree detail is intentionally reduced; commit-pointer differences remain visible. Explicit add/commit/push/pull behavior is preserved. This is not a complete Git sandbox, write authorization layer, credential-containment fix or the full #2349 scope.

Executed verification

Node 24.22.1, Git 2.52.0, Python 3.12.14. Seven scenarios execute the exact source function bodies in a Node VM against actual temporary Git repositories. The seven-case regression evaluates functions without transport. A separate actual SDK 1.32.2 stdio test negotiated protocol 2025-11-25: baseline tools/call executed fsmonitor, external-diff and textconv marker helpers; patched tools/call executed none, with successful responses and exit 0. Allowlisted child environment contains only PATH, HOME and GIT_CONFIG_NOSYSTEM; helpers only write a local marker.

  • Original: six read variants fail denial assertions; explicit commit-hook preservation passes (exit1).
  • Patched: all seven scenarios pass through the unittest wrapper (exit0); node --check passes.
  • Covered: condensed/long status, staged/unstaged external-diff and textconv, ordinary patch output, and explicit commit hook behavior.
  • Dependency comparison/record/upload skipped because no dependency declarations changed; this does not advance the separate live multi-page gate.
  • Hosted Python 3.11/3.12 each passed 3,089 tests, including this regression. Frontend: 1,593 passed, one skipped. Coverage: 3,260 passed, 87.77%. Current-head CI (including production-image build), security, CodeQL, dependency review and coverage workflows completed successfully. The image scan reports 316 findings, including the pre-existing libxml2 CVE-2026-6653; success is not a clean-image claim. Current reviews/threads empty. No production acceptance claim.

Complete runnable recipe

Prerequisites: Git, Node 22+ and Python 3.11/3.12; no SDK install, keys, provider calls or paid service. ESTIMATE: 5–10 minutes excluding CI.

  1. From an existing authorized checkout: git fetch origin codex/git-mcp-read-helper-guard-20261113; git worktree add --detach /tmp/eventrelay-read-guard 87eda70c7cc557ad9d967c83ac81cca6ec90162c; cd /tmp/eventrelay-read-guard.
  2. Run python tests/unit/test_git_read_only_security.py; expected one unittest OK with all seven scenarios passing.
  3. Run node tests/security/git_workflow_read_only_probe.mjs tools/mcp/git_workflow_server.mjs; expected seven PASS lines and exit0.
  4. Run node --check tools/mcp/git_workflow_server.mjs and git diff --check.
  5. Negative control: save the base source with git show 512a2558f73d6281b6a880548bedf140b7e30292:tools/mcp/git_workflow_server.mjs > /tmp/git-workflow-baseline.mjs, then run the same probe against that file. Expected six FAIL lines and one PASS, exit1. Only temporary fixture repositories are modified.
  6. Inspect current-head CI, reviews and exact published source. The separate stdio test above is local client integration; no real user repository or production acceptance is inferred.
  7. Rollback: close the unmerged draft or prepare an exact reviewed revert after an authorized merge. Reverting restores the known read-helper execution behavior.

Codex owns terminal CI/review follow-through and safe repairs. Exact PR/head approval is required before merge. No permission expansion, paid call, production deployment, remote Git operation or secrets access. Workflow value: read-oriented agent inspection cannot trigger these repository-configured helpers. A supervised local MCP audit is a potential service deliverable; revenue/savings are unmeasured.

@vercel

vercel Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
v0-uvai Ready Ready Preview, v0 Nov 13, 2026 8:33am UTC

@coderabbitai

coderabbitai Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

groupthinking commented Nov 13, 2026 •

Copy link
Copy Markdown
Owner Author

TEST — verified read-tool repair receipt, 2026-11-13

Notification decision: YES. A new custom-server security failure was independently reproduced and a tested fix is now reviewable. This is not a routine release alert.

Objective, impact and exact inputs

Prevent repository-configured helper execution from the active custom MCP git_status/git_diff tools. Parent #2349 stays open; child #2668 covers this repair.
Base: 512a255.
Verified head: 87eda70, branch codex/git-mcp-read-helper-guard-20261113, draft/unmerged.
Three published blobs were fetched and matched the tested files:

  • tools/mcp/git_workflow_server.mjs: 33bb6ada4c70ccda45c1da42e6aca1a58a8aad30
  • tests/security/git_workflow_read_only_probe.mjs: fdae48d4beac37e23475e45d3974393885e36176
  • tests/unit/test_git_read_only_security.py: 5947a7aea0f0b97b480b3aaff10efbac10ec6c89

Primary signal: GHSA-6hqx-gqrx-c5r2, published November 11. It concerns a separate reference implementation; no shared product identity or assigned CVE is claimed here. Confidence in this custom reproduction: 0.99. Risk requires hostile/preserved/altered local .git/config; a plain clone does not copy the remote config.

Exact mutation and verification

Read paths disable fsmonitor, pager, external diff and textconv; dirty submodule inspection is reduced. Explicit write paths are preserved. No manifest, credential, permission or deployment change.

VERIFIED locally: Node 24.22.1 / Git 2.52.0 / Python 3.12.14.
python tests/unit/test_git_read_only_security.py: one unittest, exit 0, seven real-Git scenarios. Original source failed all six read-helper denial cases; patched source passed them. Explicit pre-commit hook behavior passed on both. Direct Node probe, syntax check and whitespace check passed. Helpers write only a temporary local marker.

VERIFIED actual MCP stdio integration: exact baseline/patched source files, isolated SDK 1.32.2 installed with npm install --prefix /tmp/watch-git-sdk --ignore-scripts --no-audit --no-fund @modelcontextprotocol/sdk@1.32.2. JSON-RPC initialize negotiated 2025-11-25; tools/call exercised git_status/fsmonitor and git_diff/external/textconv. Baseline marker_executed=true in all three; patched=false in all three; all calls returned without isError; harness exit 0. Child environment was allowlisted and no network/provider request was made by the test. The runnable seven-scenario regression and full recipe remain in this PR.

VERIFIED hosted exact-head:

  • CI 43394026595: all eight jobs success, including image build.
  • Python 3.11/3.12 each 3,089 passed, nine deselected, five XPASS, 121 subtests; the new regression appears PASSED in both job logs (131780496587 / 131780496498).
  • Frontend: 1,593 passed, one skipped.
  • Coverage 43394026676: 3,260 passed, five XPASS, 141 subtests, 87.77%. Qlty publication separately reports missing oidc/token; no Qlty-delivery claim.
  • Security 43394026470: all four jobs success. Trivy enforcement is deferred and reports 316 findings, including existing libxml2 CVE-2026-6653. Artifact 14022585298 exists; this receipt does not claim independent archive validation or a clean image.
  • CodeQL 43394026530, secret scan 43394026535 and E2E 43394026621 succeeded.
  • Dependency workflow 43394026364 succeeded, but comparison/record/upload skipped with no dependency changes. NOT live multi-page proof.
  • Reviews and unresolved inline threads empty at readback. No exact-head merge approval.

Remaining responsibility and closure

Codex owns safe follow-through and any new current-head CI/review failure. Merge is BLOCKED solely on Hayden's explicit approval of PR #2669/head 87eda70. On approval, Codex must freshly verify head, checks, reviews/threads and mergeability, use expected-head protection, then fetch the merge commit and verify applicable post-merge checks. No merge or production deployment occurred.

Independent open gates remain separately owned by Codex:

Audit and continuation

Neon observation 364; action 181 retains initial attempts and this follow-through. Failed helper initialization (btoa unavailable) made no filesystem write; replaced safely. A readback query used id instead of action_id and failed 42703; schema inspection corrected it, no database mutation from the failed query. Correction to the initial receipt wording: one connector read returned no parseable content and the local JSON parse failed; its cause is UNKNOWN. The next call used only documented arguments and exact readback succeeded. Do not infer connector rejection from a missing field without an explicit provider error. The first atomic ledger update then failed 23502 because a linked incorrect_claim_id was required; it rolled back. The correction was recorded with linked claims after schema inspection. No false completion claim resulted.
Template lookup: personal accountable validator absent from the complete executor catalog; manual receipt consistency check only, not an executable validator pass.
Existing Watch task 6a97c965b86c81919b63fd58795f4407 is enabled. Initial peek returned a past time, so it was treated as UNKNOWN. A subsequent post-update readback verified the same task enabled with actual next_run_time 2026-11-14T02:00:49-06:00 (08:00:49 UTC). Existing prompt checkpoint matched exactly; no new timer was created.
ESTIMATE: 5–10 minutes to reproduce the recipe, excluding hosted CI. Revenue/savings unmeasured. Potential supervised MCP audit deliverable is a hypothesis.
Rollback: close the unmerged draft; a later revert requires review and would restore the known helper behavior. No production acceptance is claimed.

This branch was successfully deployed

1 active deployment
Preview — 87eda70c Deployed Nov 13, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security: stop repository helpers executing through Git MCP read tools

1 participant