fix(agents): never write host git config from repo-action templates#250
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTemplates now compute token-embedded clone URLs at runtime when a GitHub token is present and perform ChangesConstrain git config & clone auth to run/repo-local
sequenceDiagram
participant Template
participant Runner
participant GitHub
participant LocalFS
Template->>Runner: render clone step (compute CLONE_URL / REPO_URL)
Runner->>GitHub: git clone <CLONE_URL or REPO_URL>
GitHub-->>Runner: repository data (.git)
Runner->>LocalFS: test -d repo/.git
Runner->>LocalFS: cd repo/ and git config --local user.email/name
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/tests/test_repo_action_templates.py`:
- Around line 34-43: The test currently searches for any occurrence of
"GIT_CONFIG_GLOBAL" which can match comments; change the first search to look
specifically for the export assignment (e.g. search for "export
GIT_CONFIG_GLOBAL" or "export GIT_CONFIG_GLOBAL=" using the existing export_idx
variable and content.find) so export_idx only matches the actual export line,
keep the assert export_idx != -1, and keep the following check that export_idx
comes before the first "git config --global" occurrence (first_global) to ensure
the export assignment is present and precedes any global writes.
- Around line 24-43: Add render tests that call the existing
_assert_global_config_is_scoped helper for the evidence_collector.md.j2 and
remediation_executor.md.j2 templates so scoping regressions fail the suite;
locate where SECURITY.md and dependabot templates are already rendered in the
tests and mirror that pattern to render evidence_collector and
remediation_executor (referencing the template names evidence_collector.md.j2
and remediation_executor.md.j2 and the helper function
_assert_global_config_is_scoped) and assert the helper is executed against their
rendered content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3d3d6e67-e406-487f-8d34-7e0f69d40611
📒 Files selected for processing (5)
backend/cliff/agents/templates/dependabot_config_generator.md.j2backend/cliff/agents/templates/evidence_collector.md.j2backend/cliff/agents/templates/remediation_executor.md.j2backend/cliff/agents/templates/security_md_generator.md.j2backend/tests/test_repo_action_templates.py
The repo-action agent templates ran `git config --global` to set an
`insteadOf` credential rewrite (embedding a GitHub App token) and the
"Cliff Posture Bot" identity before cloning. When an agent flow runs
outside an isolated sandbox (e.g. on a maintainer's or customer's host),
those `--global` writes land in the operator's real `~/.gitconfig`:
- the injected `insteadOf` rule shadows the host's own credential
helper, and the short-lived token then expires, breaking the
operator's `git push` with "Invalid username or token";
- the operator's global `user.name`/`user.email` get overwritten.
`--global` always succeeds against the host and cannot fail closed, so a
dropped env var or a reordered/retried step is enough to corrupt a user's
machine — unacceptable for a tool we ship to customers.
Fix: remove every host-scoped git-config write. Clone auth is carried by
a token-embedded clone URL built at runtime from `${GH_TOKEN}` (the
literal token never enters the rendered prompt; it only ever lands in the
throwaway `repo/.git/config`). Commit identity is set with
`git config --local`, which writes the repo's config and errors out when
not inside a repo — it can never reach `~/.gitconfig`. This also fixes a
latent bug where `remediation_executor` had no identity of its own and
only committed by relying on the leaked host identity.
The worst case is now a contained failure of the agent's own PR, never
damage to the operator's git configuration.
Regression guard (`_assert_never_writes_host_git_config`) asserts the
templates contain no `git config --global` or `--system`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
galanko
force-pushed
the
fix/agent-templates-scope-git-config
branch
from
4b3fa0b to
86af911
Compare
galanko
changed the title
git config --global to the workspace, not the host
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/cliff/agents/templates/dependabot_config_generator.md.j2`:
- Around line 49-54: The template builds CLONE_URL using the shell variable
${GH_TOKEN} but never exports GH_TOKEN into the shell; update the Jinja template
so when gh_token is present it emits an export GH_TOKEN="{{ gh_token }}" line
before the CLONE_URL block (i.e., inside the same {% if gh_token %} branch),
ensuring the exported environment variable name matches ${GH_TOKEN} used by
CLONE_URL and enabling authenticated git clone for private repos.
In `@backend/cliff/agents/templates/security_md_generator.md.j2`:
- Around line 49-55: The template references the shell variable GH_TOKEN when
setting CLONE_URL (symbols: GH_TOKEN, CLONE_URL, REPO_URL) but doesn't export it
first; update the template block that runs when gh_token is truthy to export the
token into the environment (e.g., add an export GH_TOKEN=... before computing
CLONE_URL) so subsequent uses like
CLONE_URL="https://x-access-token:${GH_TOKEN}@${REPO_URL#https://}" and the git
clone command see the variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a7856665-e00b-4ee1-89ed-e839ef021f0d
📒 Files selected for processing (5)
backend/cliff/agents/templates/dependabot_config_generator.md.j2backend/cliff/agents/templates/evidence_collector.md.j2backend/cliff/agents/templates/remediation_executor.md.j2backend/cliff/agents/templates/security_md_generator.md.j2backend/tests/test_repo_action_templates.py
… host git config Extend the no-host-gitconfig regression guard to the two cloning templates that go through `render_agent` rather than `render_repo_action`. The executor pushes commits, so it is the higher-risk path; both are now asserted to contain no `git config --global`/`--system` and to carry clone auth via a token-embedded URL. Addresses CodeRabbit review feedback on PR #250 (coverage gap). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks @coderabbitai — triaged all four comments against the current commit: 1 & 2 — "Missing 3 — Guard only exercised on 2 of 4 templates: fixed in c68f9b7. Added 4 — "Anchor |
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
|
`remediation_executor` and `evidence_collector` rendered
`export GH_TOKEN="{{ gh_token }}"`, baking the literal token into the
prompt sent to the LLM. The export was redundant: GH_TOKEN is already
injected into the workspace process env by the executor / repo-workspace
runner (the same env the repo-action templates rely on via `${GH_TOKEN}`
without exporting). Drop the export so the token is never echoed into the
prompt; the `${GH_TOKEN}` shell reference still resolves from the env.
Extends the template guard to assert the literal token never appears in
the rendered prompt for these two agents.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Folded the token-hygiene follow-up into this PR (316c298): dropped the redundant |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/cliff/agents/templates/evidence_collector.md.j2 (1)
63-65:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a clone-success guard before
cd repo/.Unlike
remediation_executor.md.j2(which runstest -d repo/.git || { …; exit 1; }), this templatecds intorepo/with no verification. There is noset -e, so if the clone fails thecd repo/also fails but the subsequentgrepsteps still run from the workspace root. Since the evidence collector's output feeds the remediation planner/executor, silently searching the wrong directory can produce misleading "affected files" that cascade into a bad fix.🛡️ Mirror the executor's clone verification
git clone --depth 1 "$CLONE_URL" repo/ + # Verify the clone succeeded — if repo/.git is missing, STOP here. + test -d repo/.git || { echo "clone failed: repo/.git missing"; exit 1; } cd repo/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/cliff/agents/templates/evidence_collector.md.j2` around lines 63 - 65, The template evidence_collector.md.j2 currently runs "git clone --depth 1 \"$CLONE_URL\" repo/" then unconditionally "cd repo/" which can proceed incorrectly if clone failed; add a clone-success guard like the one used in remediation_executor.md.j2 (e.g. test -d repo/.git || { echo "clone failed"; exit 1; }) immediately after the clone and before cd repo/ so the script aborts on clone failure and does not run grep from the wrong directory.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@backend/cliff/agents/templates/evidence_collector.md.j2`:
- Around line 63-65: The template evidence_collector.md.j2 currently runs "git
clone --depth 1 \"$CLONE_URL\" repo/" then unconditionally "cd repo/" which can
proceed incorrectly if clone failed; add a clone-success guard like the one used
in remediation_executor.md.j2 (e.g. test -d repo/.git || { echo "clone failed";
exit 1; }) immediately after the clone and before cd repo/ so the script aborts
on clone failure and does not run grep from the wrong directory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8369f7f8-b39b-4f7c-b837-4c96deae8709
📒 Files selected for processing (3)
backend/cliff/agents/templates/evidence_collector.md.j2backend/cliff/agents/templates/remediation_executor.md.j2backend/tests/test_repo_action_templates.py
The evidence collector cd'd into repo/ straight after clone with no verification. With no `set -e`, a failed clone left `cd repo/` failing while the subsequent search steps ran from the workspace root — reporting misleading "affected files" that cascade into the planner/executor. Mirror the remediation_executor guard (`test -d repo/.git || exit 1`) so the run aborts on clone failure. The template guard now asserts both cloning agents verify the clone. Addresses CodeRabbit review feedback on PR #250. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed the latest review (and rebased onto the Clone-success guard in The two "missing Re-verified against the merged code: no |
1 participant
Problem
The repo-action agent templates run
git config --globalbefore cloning to (a) set aninsteadOfrewrite embedding the GitHub App token for clone/push auth, and (b) set theCliff Posture Botcommit identity.When an agent flow runs outside an isolated sandbox (a maintainer's or customer's host), those
--globalwrites land in the operator's real~/.gitconfig:insteadOfrule shadows the host's own credential helper; the embeddedghu_…token is short-lived, so once it expires everygit pushfails withInvalid username or token;user.name/user.emailget overwritten with the bot identity.This was hit in practice — a maintainer's pushes to an unrelated repo broke, with the global identity silently flipped to
Cliff Posture Bot.git config --globalalways succeeds against the host and cannot fail closed. A dropped env var or a reordered/retried step is enough to corrupt a user's machine. That's an unacceptable failure mode for a tool we ship to customers.Fix: remove every host-scoped git-config write
git config --global …insteadOf${GH_TOKEN}repo/.git/config;origincarries it so push worksgit config --global user.*git config --local user.*aftercd repo/--localwrites the repo's.git/configand errors out when not in a repo — never reaches~/.gitconfigoriginalready authenticatedThe worst case is now a contained failure of the agent's own PR, never damage to the operator's git configuration.
Also fixes a latent bug:
remediation_executorhad no commit identity of its own and only committed by relying on the leaked host identity. It now sets--localidentity.Token hygiene (folded in)
remediation_executorandevidence_collectorpreviously didexport GH_TOKEN="{{ gh_token }}", baking the literal token into the LLM prompt. The export was redundant —GH_TOKENis already injected into the workspace process env (the same env the repo-action templates use via${GH_TOKEN}without exporting). Dropped the export so the token is never echoed into the prompt;${GH_TOKEN}still resolves at runtime.Files
backend/cliff/agents/templates/{security_md_generator,dependabot_config_generator,remediation_executor,evidence_collector}.md.j2backend/tests/test_repo_action_templates.py— regression guardsTest plan
pytest -m 'not e2e'→ 1522 passed, 1 skipped (33 e2e deselected). One unrelated LLM-eval test (test_plain_description_eval … [snyk-python-setuptools-rce]) is flaky — passes in isolation and on reruns; it touches none of the changed files.ruff check cliff/ tests/→ cleangit config --global/--system; clone auth via token-embedded URL only when a token is present; literal token never appears in any rendered prompt._assert_never_writes_host_git_configruns against all four templates (incl.remediation_executor, which pushes), plus a literal-token-absence assertion on the two tool agents.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation
Tests