feat(mcp,gitcheck): block server-side commits that bypass signing

[frits-v]

Summary

• MCP layer: push_files and create_or_update_file moved from Allow → Deny in GitHub MCP routing — these use the GitHub Contents API to create server-side commits that bypass local GPG/SSH signing
• Bash layer: New FR-GS-9 pattern blocks gh api calls to commit-creating endpoints (contents/, git/commits, git/trees, git/refs, git/blobs)
• Updated doc comments, test counts, and CLAUDE.md to reflect 9 git safety patterns and 241 total tests

Test plan

• New test_github_server_side_commit_deny — verifies both MCP tools return Deny
• New test_gh_api_commit_endpoints_blocked — verifies all 5 gh api patterns blocked
• New test_gh_api_read_endpoints_not_blocked — verifies read-only gh api calls still pass
• All 241 tests pass (cargo test)
• Existing MCP and gitcheck tests unchanged and passing

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR adds FR-GS-9, a new git-safety rule that blocks two surfaces through which the GitHub API can create server-side commits that bypass local GPG/SSH signing: the MCP layer (push_files, create_or_update_file, and delete_file moved to Deny) and the Bash layer (new RE_GH_API_COMMIT_PATH + RE_GH_API_WRITE_METHOD + RE_GH_API_WRITE_BODY regexes). The PR iteratively closed several bypass vectors identified in review, but one analogous gap remains in RE_GH_API_WRITE_BODY.

Key changes:

• hooks/src/mcp.rs: create_or_update_file, push_files, and delete_file explicitly denied with a commit-signing message; new test_github_server_side_commit_deny test verifies all three.
• hooks/src/gitcheck.rs: Three new regexes detect commit-creating gh api paths combined with an explicit write method flag or implicit-POST body-field flags; read-only calls are not blocked. The path+method split correctly handles flag ordering and =-delimited method flags.
• hooks/tests/integration.rs: stdin.write_all error is now silently ignored to avoid a spurious BrokenPipe panic when the child process exits early.
• Docs (CLAUDE.md, README.md) and test-count comments updated to reflect 9 patterns and 241 tests.

Confidence Score: 3/5

• Safe to merge for most usage, but one no-separator short-flag bypass remains open in RE_GH_API_WRITE_BODY — worth patching before merging given the iterative fix history on this PR.
• MCP changes and the majority of the Bash regex work are correct and well-tested. However, RE_GH_API_WRITE_BODY uses [=\s] (one-required) while the analogous RE_GH_API_WRITE_METHOD uses [=\s]* (zero-or-more) — the -fvalue no-separator form bypasses write-body detection, creating a concrete gap that mirrors every previous bypass fixed during this review cycle.
• hooks/src/gitcheck.rs — specifically RE_GH_API_WRITE_BODY at line 62

Important Files Changed

Filename	Overview
hooks/src/gitcheck.rs	Adds FR-GS-9 with three regexes (path, write-method, write-body). Write-method regex correctly handles -XPOST no-separator form via [=\s]*, but write-body regex uses [=\s] (one required) and misses the analogous -fvalue short-flag concatenation bypass.
hooks/src/mcp.rs	Moves create_or_update_file, push_files, and delete_file to an explicit Deny block before the safe-writes match arm. Clean and correct; new test_github_server_side_commit_deny test covers all three tools.
hooks/src/bin/permissions.rs	Single comment update from FR-GS-8 to FR-GS-9; no logic changes.
hooks/tests/integration.rs	Swaps unwrap() for let _ = on stdin.write_all to silently absorb BrokenPipe errors when the subprocess exits before reading all input — a correct and minor robustness improvement.
CLAUDE.md	Documentation updated to reflect 9 git safety patterns and revised test count breakdown (241 tests).
README.md	One-line documentation update matching CLAUDE.md: 8 → 9 git safety patterns.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Bash command] --> B{RE_GH_API_COMMIT_PATH\nmatches?}
    B -- No --> Z[GitResult::Ok]
    B -- Yes --> C{RE_GH_API_WRITE_METHOD\nmatches?\n-X/-Xpost/--method=POST etc.}
    C -- Yes --> BLOCK[GitResult::Block\nBLOCKED: bypasses commit signing]
    C -- No --> D{RE_GH_API_WRITE_BODY\nmatches?\n-f/--field/-F/--data/--input + sep}
    D -- Yes --> BLOCK
    D -- No --> Z

    subgraph GAP [Known gap]
        E["'-fvalue' no-separator form\nnot matched by RE_GH_API_WRITE_BODY"]
    end
    D -. false negative .-> GAP

Loading

_{Last reviewed commit: "fix(gitcheck): handl..."}

[frits-v]

@greptile review

[frits-v]

@greptile review

[frits-v]

@greptile review

[frits-v]

@greptile review

[frits-v]

@greptile review

[frits-v]

@greptile review

[greptile-apps]

Short-flag no-separator form bypasses RE_GH_API_WRITE_BODY

RE_GH_API_WRITE_BODY requires at least one = or whitespace character after the flag name ([=\s]). However, cobra/pflag — which gh uses — also accepts values concatenated directly against a short flag with no separator. The pattern -fvalue is equivalent to -f value for string flags.

This means a command like gh api repos/owner/repo/contents/file.txt -fmessage=create passes the guard:

• RE_GH_API_COMMIT_PATH matches (contents/)
• RE_GH_API_WRITE_METHOD does not match (no -X/--method)
• RE_GH_API_WRITE_BODY: \s(-f)[=\s] — the character immediately after -f is m, not = or whitespace → no match

Result: GitResult::Ok — bypassed.

The same applies to -F and -d short flags. This is the exact same class of bug that was fixed in RE_GH_API_WRITE_METHOD via [=\s]* (zero-or-more). A parallel fix would make the trailing separator optional for the short-flag forms:

Suggested change

	static RE_GH_API_WRITE_BODY: LazyLock<Regex> = LazyLock::new(|| {
	Regex::new(r"\s(-f|--field|-F|--raw-field|-d|--data|--input)[=\s]").unwrap()
	});
	static RE_GH_API_WRITE_BODY: LazyLock<Regex> = LazyLock::new(|| {
	Regex::new(r"\s(-f|-F|-d)[=\s]?|\s(--field|--raw-field|--data|--input)[=\s]").unwrap()
	});

The test suite should also add coverage for the no-separator form:

// Short-flag concat (no separator) — implicit POST
"gh api repos/owner/repo/contents/file.txt -fmessage=create",
"gh api repos/owner/repo/git/commits -Fmessage=bypass",