Agent hardening#34
Merged
Merged
Conversation
The AI agent (Claude/Gemini CLI) is spawned as a subprocess of mergai and inherited mergai's full environment via os.environ.copy() -- which in CI carries the GitHub write token used by mergai's own push/PR steps. A misbehaving or prompt-injected agent could therefore push to origin or call GitHub write APIs. Add agent_subprocess_env() which removes GITHUB_TOKEN/GH_TOKEN/ GH_ENTERPRISE_TOKEN from the environment handed to the agent (substituting a read-only MERGAI_AGENT_GH_TOKEN if provided), and use it when spawning both adapters. The parent mergai process keeps its write-capable environment, so its deterministic pushes are unaffected -- only the agent is de-privileged. Local operations (file edits, local git reads/commits) need no credential and still work. This closes the environment path only; the checkout's persisted http.extraheader credential is closed separately with persist-credentials: false in the workflow.
There was a problem hiding this comment.
Pull request overview
This PR hardens the CLI agent adapters to reduce the risk of remote writes and credential misuse by (a) sanitizing the subprocess environment and (b) tightening/standardizing “yolo” behavior away from fully-bypassed permission modes.
Changes:
- Add
agent_subprocess_env()to strip GitHub credential environment variables from spawned agent subprocesses. - Harden
ClaudeCLIAgent.build_args()by always applying a--disallowedToolsdeny-list and mappingyolotoacceptEdits(not--dangerously-skip-permissions). - Harden
GeminiCLIAgentby forcing--approval-mode auto_editand using the sanitized subprocess environment; add tests covering these guardrails.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_claude_build_args.py | Adds tests for Claude CLI guardrail args (deny-list, permission mode behavior, prompt position). |
| tests/test_agent_env.py | Adds tests ensuring GitHub credential env vars are stripped while unrelated env is preserved. |
| src/mergai/agents/gemini_cli.py | Forces safer approval mode and uses sanitized env for the Gemini subprocess. |
| src/mergai/agents/env.py | Introduces environment sanitization helper for agent subprocesses. |
| src/mergai/agents/claude_cli.py | Adds deny-list guardrails, changes yolo semantics, and uses sanitized env for the Claude subprocess. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`mergai resolve -y` / describe previously ran the Claude agent with --dangerously-skip-permissions, which bypasses every guardrail and lets the agent run any command (including git push and gh writes). Instead: - map yolo (and any edit-capable run) to --permission-mode acceptEdits, which auto-approves file edits in the working dir but not real yolo; - pass --disallowedTools on *every* invocation to deny remote-mutating git/gh, network egress (WebFetch/WebSearch) and work-spawning/orchestration tools (Task, Skill, Workflow, Cron*, ...) that could bypass these limits. No --allowedTools list is used: testing showed it is not a strict allow-list (under acceptEdits a command in neither list still runs), so it would neither enable nor restrict anything. Enforcement is this always-on deny-list plus the authoritative server-side boundary (the agent runs with no GitHub credential; see env.py and the workflow's persist-credentials: false). Bash deny rules are prefix-matched and best-effort; the credential boundary is what actually guarantees no remote writes. Verified live: git log runs, git push is denied. Allow Bash broadly so the agent can run read commands under the deny-list CI (claude 2.1.198) proved that a deny-list-only config denies un-allowed Bash in --print mode: the agent's read commands (git ls-tree, git diff | head, for-loops) all hit "requires approval" and were blocked, leaving describe/ resolve blind to the repo. An earlier local test (older CLI) suggested unlisted Bash ran; that did not hold on CI. Re-introduce a broad --allowedTools (Bash + file tools) alongside the deny-list. Deny takes precedence, so remote-write / bypass commands stay blocked while all other Bash (the agent's read shell) is auto-approved.
Mirror the Claude adapter: map yolo to Gemini's auto_edit approval mode (which auto-approves file edits only) instead of "yolo" (which auto-approves every tool, including shell commands that could push or reach the remote). The agent subprocess already runs with no GitHub credential (see env.py), so it cannot write to the remote regardless; this removes the blanket tool auto-approval.
`mergai review fix` runs the AI agent, so it must stay read-only with respect
to GitHub. Previously `--ack` posted the acknowledgement comment directly
(create_issue_comment) from within the fix command, which required a write
token in the agent's step.
Record the ack on the cache note instead and publish it from `review post`
(the write-token step that already posts the recorded replies), mirroring the
existing "record then post" pattern used for review replies:
- MergaiNote gains a `review_ack` record ({message, pr_number, posted_at}) with
set_review_ack / pending_review_ack / mark_review_ack_posted, serialized in
to_dict/from_dict.
- `review fix --ack` now calls _record_ack (writes only the note); it performs
no GitHub write.
- `review post` publishes any pending ack after the replies and marks it posted
(idempotent across re-runs), and no longer no-ops when replies are empty but
an ack is pending.
This lets the review-handle workflow run `review fix` under a read-only token
and scope the write token to `review post`.
mergai ci status --state (read by the CI-fix gate) used a crude
conclusion rollup: all runs completed and at least one != success ->
failure. That disagreed with the fixer (classify_run / ci fix all), so
the gate reported failure for runs the fixer would then skip, spinning
up the privileged handler (and its deployment-approval request) for
nothing: a human/timeout-cancelled check, a rejected approval, an infra
failure with no failing step, or a run mergai already fixed.
Replace _aggregate_state with _actionable_state, which reduces the same
per-run classification ci list / ci fix all use (via _list_run_status).
Now failure means mergai has actionable work:
* plain cancellation / rejected approval / no failing step -> success
* already-handled run (solution or comment recorded) -> success
* fail-fast cancel masking a real failing step -> failure
* real failure -> failure
* passing run with Code Scanning findings -> failure
* any watched run still running -> in-progress
(wins over failure so the handler fires only once the whole watched
set for HEAD is done)
Thread check_failure_kind through _list_run_status (default False, so
ci list is unchanged; the gate passes True) so an approval-rejected /
infra failure is classified skip.
The gate stays read-only. Confirming Code Scanning findings needs a
token with security-events read; when the gate runs without it (or any
read side-call errors) the run degrades to non-actionable rather than
crashing the gate or firing the handler on an unconfirmed signal.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Hardens the mergai AI agent (Claude/Gemini CLI) so it cannot modify remote state
or be used to bypass its limits, and stops it from running in real "yolo" mode.
Companion to the psmdb workflow change (PSMDB-2142-workflows-hardening), which
keeps the write token out of the agent's environment; this PR is the mergai
half.
Changes
agents/env.py): theagent is spawned with
GITHUB_TOKEN/GH_TOKEN/GH_ENTERPRISE_TOKENremoved,so even a prompt-injected agent has no token to push or call GitHub write
APIs. Local file/git operations need no credential and are unaffected.
agents/claude_cli.py):yolomaps to
--permission-mode acceptEdits(auto-approve edits in the workingdir) instead of
--dangerously-skip-permissions.Bashis allowed broadly(the agent needs read shell to inspect the merge) with a deny-list for the
dangerous subset —
git push,gh,WebFetch/WebSearch, andwork-spawning tools (
Task/Skill/Workflow/…). Deny takes precedence overallow.
agents/gemini_cli.py):yolomaps to--approval-mode auto_editinstead of Gemini'syolo.--acktoreview post(commands/review.py,models.py):review fix --acknow records the acknowledgement on the note(read-only) instead of posting it;
review post(the write-token step)publishes it. This lets
review fixrun under a read-only token.Notes
guarantee is the credential boundary — the agent has no reachable write token
(this PR strips it from the env; the workflow PR removes the on-disk copy and
scopes the write token to non-agent steps).
Validation
Run green end-to-end on a fork with
MERGAI_REFpointed at this branch: theagent runs restricted (acceptEdits, allow-Bash + deny-list, read-only token)
with zero denied tool calls across trigger-merge, review-handle and
ci-handle (inline / semantic-relocate / no-op paths); the
--ackposts fromreview post(no 403); all writes succeed from the write-token steps.