Track downstream migration for effective Git credential preflight #1598

Activity

1. fro-bot

   added

   agent: working

   on Sep 11, 2026
2. fro-bot commented on Sep 11, 2026

   

   fro-bot

   on Sep 11, 2026

   Owner

   More actions

   Triage: downstream migration tracking for the effective-credential preflight

   Classification: tracking / coordination issue, not a defect. No code change is required in this repo to close it. The one thing that is actionable here is that the checklist's stated compliance criterion is narrower than what #1597 actually enforces, and the completion gate as written is not reachable given how this action is released.

   ---

   What I verified

   I read origin/fix/effective-git-credential-preflight (the #1597 branch, 5 commits ahead of main) and ran its tests locally.

   The preflight itself is real and already on main in narrow form — src/harness/phases/bootstrap.ts:79-86 calls assertNoPersistedGitCredentials and core.setFaileds on a hit. The withheld event set is exactly pull_request, issue_comment, issues, per classifyEvent in packages/runtime/src/agent/response-delivery.ts:30-44. The persist-credentials expression in the issue body matches that set exactly — it is correct as written.

   #1597 widens the check substantially. git config --local --get-regexp '^http\..*\.extraheader$' becomes git config --includes --name-only --get-regexp '^http\.(.*\.)?extraheader$', plus a rev-parse --absolute-git-dir repo-context probe, and every ambiguous exit code now denies instead of passing.

   I measured the delta by running the branch's new integration suite against main's implementation:

   main:    13 failed | 6 passed  (19)
   #1597:   61 passed              (3 files, incl. unit + bootstrap)
   

   The 13 vectors main misses include global-scope headers, system-scope headers, includeIf.gitdir targets, worktree-scoped config, bare repositories, bare-key http.extraheader (no URL subsection), malformed config, and invalid GIT_DIR.

   ---

   Finding 1 — the checklist's compliance criterion no longer matches the check (highest impact)

   The "Required posture" section defines compliance as "a checkout that sets persist-credentials: false." After #1597 that is necessary but not sufficient. The check now reads the effective config across local, global, system, worktree, and includeIf scopes. A repo with a perfectly compliant actions/checkout still fails if anything in the job leaves a host-scope http.*.extraheader — a container image with a baked /etc/gitconfig, a self-hosted runner, a composite setup action, or any earlier step doing git config --global.

   This means the 16 entries under "Already compliant" were validated against the old criterion, and static workflow inspection cannot prove the new one. They are probably fine — hosted runners don't ship an extraheader — but the list is asserting more than the evidence supports. I'd relabel that section as "checkout is compliant" rather than "compliant", and treat the first post-release issues run in each repo as the actual verification.

   Finding 2 — the completion gate is unreachable as written

   The issue says migration must land "before consuming the breaking behavior from #1597." But:

   • docs/examples/fro-bot.yaml:407 tells consumers to use fro-bot/agent@v0 — a floating major tag.
   • The fix(setup)!: check effective Git credentials on withheld runs #1597 branch adds breaking: true → release: minor to .releaserc.yaml, keeping the project in 0.x.

   So #1597 ships as a routine minor, v0 moves, and every @v0 consumer picks up fail-closed behavior on their next withheld-event run with no opt-in window. There is no "before consuming" for them to migrate in.

   Only two orderings actually satisfy the stated gate:

   1. Land all 12 remediation PRs before merging/releasing fix(setup)!: check effective Git credentials on withheld runs #1597. Simple, but blocks fix(setup)!: check effective Git credentials on withheld runs #1597 on 12 external repos, two of which are archived.
   2. Decouple — see Finding 3.

   Finding 3 — local-scope self-heal would collapse most of the migration

   actions/checkout writes its extraheader into local config only. The action already runs before the model and already inspects that config. It could git config --local --unset-all the matched key (and rewrite a credentialed origin) instead of failing, while still failing closed on global/system/includeIf scopes it can't safely own.

   That would make ~all 12 pending repos a no-op and remove the release-ordering problem entirely. The tradeoffs are real — mutating a consumer's checkout is surprising, and failing loudly arguably produces better long-term hygiene — so this deserves an explicit decision on #1597 rather than an implicit one. Worth noting the security property is identical either way: an unset local extraheader is exactly as unreadable to the model as a refused run.

   Finding 4 — the event list will drift, 30 times over

   The ["pull_request", "issue_comment", "issues"] array duplicates classifyEvent into every downstream workflow. pull_request_review_comment and discussion_comment are documented in response-delivery.ts:24-28 as deferred surfaces. If either is later reclassified as affected, all 30 repos silently regress to persisting credentials on a withheld event — and the preflight would catch it as a hard failure across the fleet at once.

   #1597 already builds the right guard for this repo: the new scripts/fro-bot-workflow.test.ts block derives the expected persist decision from resolveResponseDelivery and checks it against the workflow's own declared on: triggers, so the YAML and the policy can't diverge. That pattern is the fix — it just needs to be reachable from downstream repos (a published snippet, a composite action, or an exported expression) rather than living only in this repo's test suite.

   Finding 5 — the error message dropped the matched key name

   main named the offending config key in the error (git-credential-check.ts:37-41). #1597's ERR_HEADER_FOUND does not, even though it runs with --name-only, so the matched key is already non-secret by construction. For an issue whose entire premise is 30 repos debugging this failure remotely, putting the key name back is high-value and zero-risk. Small suggestion for #1597.

   Finding 6 — guidance inconsistency for the remediation PRs

   The issue recommends the event-gated expression as the default posture, but docs/examples/fro-bot.yaml:401 — the canonical example downstream repos copy — uses flat persist-credentials: false, in a single job that handles all triggers including schedule and workflow_dispatch. Flat false is the simpler and safer default; the gated form is only needed when the same job also needs a persisted checkout credential for autonomous pushes.

   Worth stating which is the default before writing 12 PRs, otherwise they'll diverge. The per-repo notes already hint at this (some say "set false", others say "add event-gated"), so the distinction exists — it just isn't stated as a rule.

   ---

   Proposed next steps

   Blocking on #1597:

   1. Decide fail-closed vs. local-scope self-heal (Finding 3) — this determines whether the other 11 items matter at all.
   2. Restore the matched key name in ERR_HEADER_FOUND (Finding 5).

   On this issue:
   3. Rewrite "Required posture" to state the real criterion: no effective http.*.extraheader in any scope, and no credential in the origin URL. Note that persist-credentials: false covers the common case but not host-level config.
   4. State the default (flat false) and when to use the gated form (Finding 6).
   5. Resolve the two archived repos now — archived repos cannot dispatch workflows, so the live risk is zero. Record "accepted risk: archived, workflows cannot run" and strike them rather than carrying them as blockers.
   6. Re-label "Already compliant" as checkout-level compliance, and add a post-release verification pass (Finding 1).

   Follow-up, separate issue:
   7. Make the persist-credentials expression consumable from one place so it can't drift across 30 repos (Finding 4).

   What I could not verify

   gh is unavailable in this run, so I could not confirm the repository inventory, the 30/27/3 counts, the per-repo workflow contents, or the archived status of fro-bot/tokentoilet and marcusrbrown/copiloting. Everything above is derived from this repository's source, the #1597 branch, and local test execution.

   ---

   Run Summary

   Field	Value
   Event	issues
   Repository	fro-bot/agent
   Run ID	34563289464
   Cache	hit
   Session	ses_f7137fcccffeGSQli7cBrZYgd5
3. fro-bot

   removed

   agent: working

   on Sep 11, 2026
4. marcusrbrown

   added 3 commits that reference this issue on Sep 13, 2026

   fix(ci): make runtime verification observation-only

   ...

   fb37866

   feat(ci): sweep downstream runtime verification in the daily report

   ...

   6f420b1

   feat(ci): sweep downstream runtime verification in the daily report

   ...

   3cddaba