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

tests/security/git_workflow_read_only_probe.mjs

@@ -0,0 +1,34 @@
+import assert from 'node:assert/strict';
+import {spawn, execFileSync} from 'node:child_process';
+import {readFileSync,writeFileSync,mkdirSync,mkdtempSync,existsSync,chmodSync} from 'node:fs';
+import {tmpdir} from 'node:os';
+import path from 'node:path';
+import vm from 'node:vm';
+const source=readFileSync(process.argv[2],'utf8');
+const defs=source.slice(source.indexOf('function runGitCommand'),source.indexOf('class GitWorkflowMCPServer'));
+let failures=0;
+for (const kind of ['fsmonitor','fsmonitor-long','external','external-staged','textconv','textconv-staged','write-hook']) {
+ const root=mkdtempSync(path.join(tmpdir(),'git-readonly-'));
+ const env={PATH:process.env.PATH,HOME:root,GIT_CONFIG_NOSYSTEM:'1'};
+ const git=(...args)=>execFileSync('git',args,{cwd:root,env,stdio:'pipe'});
+ git('init','-q');git('config','user.name','Fixture');git('config','user.email','fixture@example.invalid');
+ writeFileSync(path.join(root,'file.txt'),'before\n');git('add','.');git('commit','-qm','baseline');
+ const marker=path.join(root,'marker');
+ const helper=path.join(root,'helper.sh');
+ writeFileSync(helper,'#!/bin/sh\nprintf fixture > "'+marker+'"\n');chmodSync(helper,0o755);
+ writeFileSync(path.join(root,'file.txt'),'after\n');
+ if(kind.endsWith('-staged'))git('add','file.txt');
+ if(kind.startsWith('fsmonitor'))git('config','core.fsmonitor',helper);
+ if(kind.startsWith('external'))git('config','diff.external',helper);
+ if(kind.startsWith('textconv')){git('config','diff.fixture.textconv',helper);writeFileSync(path.join(root,'.gitattributes'),'*.txt diff=fixture\n');}
+ if(kind==='write-hook'){const hook=path.join(root,'.git/hooks/pre-commit');writeFileSync(hook,readFileSync(helper));chmodSync(hook,0o755);}
+ const context=vm.createContext({spawn,path,process:{env},WORKSPACE_ROOT:root});
+ vm.runInContext(defs,context);
+ try {
+  const result=await vm.runInContext(kind.startsWith('fsmonitor')?'gitStatus({porcelain:'+(!kind.endsWith('-long'))+'})':kind==='write-hook'?'gitCommit({message:"fixture",allowEmpty:true})':'gitDiff({staged:'+kind.endsWith('-staged')+',pathspec:"file.txt"})',context);
+  if(kind==='write-hook')assert.ok(existsSync(marker),'explicit commit hook behavior preserved');
+  else {assert.ok(!existsSync(marker),kind+' must not execute repository helper');if(!kind.startsWith('fsmonitor'))assert.ok(result.stdout.includes('+after'),'ordinary diff preserved');}
+  console.log('PASS '+kind);
+ } catch(error){failures++;console.log('FAIL '+kind+': '+error.message);}
+}
+process.exitCode=failures?1:0;

tests/unit/test_git_read_only_security.py

@@ -0,0 +1,24 @@
+"""Real Git marker regressions for the exact read-only launcher function bodies.
+
+No MCP server or provider is launched. The Node VM excludes SDK imports and
+transport wiring; this test does not establish MCP end-to-end acceptance.
+"""
+import subprocess
+import unittest
+from pathlib import Path
+
+
+class GitReadOnlySecurityTests(unittest.TestCase):
+    def test_repository_helpers_do_not_run_from_read_tools(self):
+        root = Path(__file__).resolve().parents[2]
+        result = subprocess.run(
+            ["node", str(root / "tests/security/git_workflow_read_only_probe.mjs"),
+             str(root / "tools/mcp/git_workflow_server.mjs")],
+            capture_output=True, text=True, timeout=30, check=False,
+        )
+        self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+        self.assertEqual(result.stdout.count("PASS "), 7)
+
+
+if __name__ == "__main__":
+    unittest.main()

tools/mcp/git_workflow_server.mjs

@@ -28,9 +28,14 @@ if (!fs.existsSync(WORKSPACE_ROOT)) {
   process.exit(1);
 }
 
-function runGitCommand(args, { allowEmpty = false } = {}) {
+function runGitCommand(args, { allowEmpty = false, readOnly = false } = {}) {
   return new Promise((resolve, reject) => {
-    const child = spawn('git', args, {
+    // Read tools must not execute helpers supplied by repository configuration.
+    // Explicit write tools retain their existing Git/hook behavior.
+    const commandArgs = readOnly
+      ? ['--no-pager', '-c', 'core.fsmonitor=false', ...args]
+      : args;
+    const child = spawn('git', commandArgs, {
       cwd: WORKSPACE_ROOT,
       env: process.env,
     });
@@ -62,7 +67,8 @@ function runGitCommand(args, { allowEmpty = false } = {}) {
 
 async function gitStatus({ porcelain = true } = {}) {
   const args = porcelain ? ['status', '--short', '--branch'] : ['status'];
-  return runGitCommand(args);
+  args.push('--ignore-submodules=dirty');
+  return runGitCommand(args, { readOnly: true });
 }
 
 async function gitAdd({ files }) {
@@ -109,14 +115,14 @@ async function gitPull({ remote, branch, rebase = false } = {}) {
 }
 
 async function gitDiff({ pathspec, staged = false } = {}) {
-  const args = ['diff'];
+  const args = ['diff', '--no-ext-diff', '--no-textconv', '--ignore-submodules=dirty'];
   if (staged) {
     args.push('--staged');
   }
   if (pathspec && pathspec.length > 0) {
     args.push('--', pathspec);
   }
-  return runGitCommand(args);
+  return runGitCommand(args, { readOnly: true });
 }
 
 class GitWorkflowMCPServer {