chrisleekr / chrisleekr/github-app

security(agent-sdk): destructive-action gate only covers the Bash tool — adjacent vectors (.git/ writes, future MCP merge tools) bypass it

Open
#240 0 comments 0 reactions 0 assignees View on GitHub
research
Dominant language
TypeScript
Stars
1
Forks
0
Avg merge
6h 39m
Merged PRs (30d)
27

Description

Follow-up from #222 (PR adding the runtime PreToolUse `forbidden-bash` hook).

The hook added in #222 gates **only** the `Bash` tool (`matcher: "Bash"` in `src/core/executor.ts`). The same destructive effects remain reachable through tools the hook never inspects:

1. **Write/Edit into the cloned `.git/`.** The agent has filesystem write under `cwd` (the clone). Writing `.git/config` (e.g. changing the push URL or adding a mirror refspec), overwriting `.git/refs/heads/` / packed-refs, or planting `.git/hooks/pre-push` rewrites repo state with no `git push --force` string ever passing through Bash; a later plain `git push` (allowed) then publishes the rewritten ref. This is the more realistic path since the agent already has `.git/` write access under `bypassPermissions`.
2. **MCP server tools.** The hook does not inspect MCP tool calls. Today the wired GitHub MCP servers are scoped to comments/reviews (no merge), but a future server exposing a merge/branch-mutation tool would bypass the Bash gate.

Also note (documented in #222): the regex denylist on free-form shell is inherently incomplete (variable indirection, command substitution, base64|sh, write-script-then-run). The hook is defense-in-depth for the naive/literal path, not a sandbox.

### Suggested directions
- Consider denying Write/Edit tool calls whose resolved target path is inside `.git/`.
- Re-audit the matcher set if any MCP server gains merge/branch-mutation tools.
- The durable, vendor-independent backstop is **server-side branch protection** on the remote (force-push + direct-merge rejected regardless of the local command) plus a least-privilege installation token. Evaluate enforcing branch protection as the primary control, with the runtime hook as one layer.

Surfaced by the senior-review panel during #222.

Contributor guide

Open the contributing guide

Research direction

Start in src/core/executor.ts and review the runtime PreToolUse hook added by #222, then trace how Write/Edit and MCP tools are wired. Compare the suggested .git/ path restrictions, MCP matcher coverage, and remote branch-protection controls; done means the project has a decided, tested defense strategy rather than only the Bash denylist.

Written by the indexing model from the issue text.

Assessment

Tech stack
github, typescript
Domain
security, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.