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
- 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
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