fix(ci): stop the canonical-evidence gate failing Dependabot by construction - #1423
Conversation
StructuredFormatter only prepended an "Exception Details:" header and never stripped line separators, so any exc_info=True / logger.exception(...) sink — and structured `extra` fields the format references — could forge, corrupt, or split downstream log lines with attacker-controlled separators, even where the message itself was sanitized inline. This central fix escapes every str.splitlines() boundary (LF, CR, VT, FF, FS, GS, RS, NEL, LS, PS) plus ESC in the FINAL rendered record, covering every sink at once without touching call sites. Escapes are emitted as JSON-valid \uXXXX sequences so JSON logging (enable_json_logging=True) stays parseable, and backslash is escaped first so the transform is unambiguous and reversible. Adds tests/unit/test_logging_config_crlf.py asserting against rendered handler output (message, exc_info traceback, logger.exception, extra fields), JSON-log parseability, reversibility, and full splitlines-boundary coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFJ9na5UBaskoD4eAe1fhq
…ruction `Canonical issue and evidence` requires five prose sections and exactly one `Closes #<issue>` reference in the PR body. Dependabot composes its body from a fixed template and can emit none of them, so the gate named an action the author could not take and failed permanently. PR #1171 is the live proof: it is the only non-draft Dependabot PR open, and its governance run fails with exactly the five-section error. The other three (#1000, #1173, #1176) are drafts and take the existing draft escape; each goes permanently red the moment it is marked ready for review. The sibling truth gate in pr-checks.yml already carried `login !== 'dependabot[bot]'`, and justified deferring by asserting that the canonical requirement is one "an author can actually meet". That was false for Dependabot, so the exemption relocated the constraint instead of removing it. Both halves are fixed here: pr-governance.yml gains the matching escape, and the stale rationale comment is corrected. The escape reports `neutral`, not `success` — the contract is not applicable, not satisfied. Reporting it satisfied would be the same false signal this check exists to catch. Dependency PRs stay gated by dependency-review, npm-audit, trivy, build and test; only the PR-body prose contract is waived. Tests execute the real script under Node against synthetic payloads rather than matching strings in its source. Verified non-vacuous: the Dependabot case fails against the pre-fix workflow and passes after, with the other 13 unchanged. Human authors and other bots posting an identical template-less body still fail, so the exemption is keyed on author rather than body shape. Closes #1419 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGBZnk7MiKZR83NFb3QoWQ
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 18 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Agent Completion Truth Gate: BLOCKEDReasons: Machine-readable verdict{
"details": {
"collection_errors": [
"incomplete_linked_issue_contract",
"missing_intent_snapshot",
"missing_agent_run_id",
"missing_agent_login"
],
"invalid_fields": [
"policy.agent_login",
"policy.run_id"
]
},
"reasons": [
"invalid_payload"
],
"verdict": "blocked"
} |
groupthinking
added
ci-cd
tests
github_actions
Claude
|
| PR | truth-gate | outcome |
|---|---|---|
| #1408 | failure |
merged to main at 17:06 today |
| #1410 | failure (×2) |
open, same invalid_payload |
| #1423 (this) | failure |
— |
#1408 merging red is the decisive datum: the check does not gate merges, and MERGE_POLICY.md gate 2 does not list it among the required contexts.
I have not filled in an agent-lock-manifest. It declares a run_id and agent_login the gate treats as corroborated evidence; there is no dispatch behind this change, and fabricating them would inject false evidence into exactly the class of mechanism this PR repairs.
Everything else is green — build, test, guards, lint-python, lint-frontend, all six security scans, gitleaks, dependency-review, and Canonical issue and evidence reports neutral (draft escape). Terminal state: HALTED(awaiting_merge_approval) once CI settles.
@coderabbitai full review
Generated by Claude Code
|
I will treat ✏️ Learnings added
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
|
@vercel — confirmed, and it is a real bug. Tracked as #1429. Not fixing it here, for scope reasons below. The finding is correct. attacker = 'benign", "level": "DEBUG", "forged": "yes'Duplicate keys resolve last-wins in most parsers, so the attacker's One correction to the framing. The comment claiming the record "stays valid JSON" is not the defect — the forged record is valid JSON, and that is precisely what makes it dangerous. It parses cleanly into attacker-chosen fields. The comment overstates by implying faithful, not by being wrong about validity. The obvious fix does not work, which is why this needs its own PR rather than a line here. Adding
Why not in this PR. Full repro, exposure analysis ( Generated by Claude Code |
Resolves the pr-checks.yml conflict in favour of main: #1431 retired the agent-completion truth gate outright, so this branch's edits to that job (comment corrections, twice) have no surviving target. The substance of this PR -- the pr-governance.yml author escape and its five behavioural tests -- is untouched by the retirement. Updates the escape's own comment, which cited the retired gate as the sibling precedent for deferring. Left as-is it would assert a cross-check that no longer exists, which is the same class of stale claim this PR was opened to remove. Verified after the merge: both workflows parse, and tests/unit/test_pr_governance_workflow.py is 14 passed, including test_gate_is_not_applicable_to_dependabot and the two narrowness guards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MsrR4ngeBsCT9qiBftEWbB
The last red check is gone — not fixed here, retired upstream
The merge conflicted, and the resolution is worth stating
The substance of this PR was never in that file. The author escape lives in One thing the merge broke, now fixedThe escape's own comment cited the retired gate as its precedent:
That cross-reference is now false — Verification after the merge
The five behavioural tests still execute the real gate under Node — CI on Why this is being marked ready for reviewThe defect is live and currently costing two PRs.
While this PR sits in draft it is skipped by review, so the gate it repairs keeps failing PRs by construction and the dependency backports cannot land. No merge is being performed here — that decision is still human. Generated by Claude Code |
|
@coderabbitai full review Requesting explicitly because auto-review did not run: the CodeRabbit status on Worth a careful look at two things in particular, since this change modifies a gate rather than product code:
Generated by Claude Code |
|
I will verify that
|
|
@coderabbitai full review Requesting an explicit pass — auto-review is reporting Points worth independent scrutiny, since this changes a governance gate:
Verified live on head
Every other check on #1433 is green — Note for scope: this does not fully unblock #1171, which additionally fails Generated by Claude Code |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR metadata =='
gh pr view 1423 --repo groupthinking/EventRelay \
--json number,title,state,isDraft,author,headRefOid,baseRefName,files,statusCheckRollup \
--jq '{number,title,state,isDraft,author,headRefOid,baseRefName,files:[.files[].path],checks:[.statusCheckRollup[]|{name,conclusion,status}]}'
echo
echo '== PR diff =='
gh pr diff 1423 --repo groupthinking/EventRelay -- \
.github/workflows/pr-governance.yml tests/unit/test_pr_governance_workflow.py
echo
echo '== Workflow outline =='
ast-grep outline .github/workflows/pr-governance.yml --items all || true
echo
echo '== Relevant workflow lines =='
rg -n -C 12 'AUTOMATED_DEPENDENCY_AUTHORS|dependabot|pr\.user|conclusion|neutral|draft|Canonical issue' \
.github/workflows/pr-governance.yml
echo
echo '== Relevant test lines =='
rg -n -C 8 'test_gate_|dependabot|google-labs-jules|neutral|failure|node' \
tests/unit/test_pr_governance_workflow.pyLength of output: 4025 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Changed-file diff =='
gh pr diff 1423 --repo groupthinking/EventRelay
echo
echo '== Workflow file structure =='
ast-grep outline .github/workflows/pr-governance.yml --items all || true
echo
echo '== Workflow: policy script and surrounding YAML =='
nl -ba .github/workflows/pr-governance.yml | sed -n '1,320p'
echo
echo '== Unit-test file structure =='
ast-grep outline tests/unit/test_pr_governance_workflow.py --items all || true
echo
echo '== Unit tests: changed and supporting helpers =='
nl -ba tests/unit/test_pr_governance_workflow.py | sed -n '1,460p'
echo
echo '== Commits in PR =='
gh pr view 1423 --repo groupthinking/EventRelay --json commits \
--jq '.commits[] | {oid,messageHeadline,authoredDate}'Length of output: 9795 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='groupthinking/EventRelay'
ref='fb6d072b4697030407c8af3b5b2bddc4cf3c1c0b'
echo '== pr-governance.yml at PR head =='
gh api "repos/${repo}/contents/.github/workflows/pr-governance.yml?ref=${ref}" \
-H 'Accept: application/vnd.github.raw+json' | sed -n '1,360p'
echo
echo '== test_pr_governance_workflow.py at PR head =='
gh api "repos/${repo}/contents/tests/unit/test_pr_governance_workflow.py?ref=${ref}" \
-H 'Accept: application/vnd.github.raw+json' | sed -n '1,420p'
echo
echo '== Dependabot GitHub account =='
gh api users/dependabot%5Bbot%5D \
--jq '{login,id,type,site_admin,html_url,created_at}'
echo
echo '== PR `#1433` author and check conclusions =='
gh pr view 1433 --repo "$repo" --json author,state,isDraft,statusCheckRollup \
--jq '{author,state,isDraft,checks:[.statusCheckRollup[]|select(.name == "PR Governance" or .name == "Canonical issue and evidence")|{name,conclusion,status}]}'Length of output: 18813 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='groupthinking/EventRelay'
for number in 1433 1171; do
echo "== REST pull request #${number}: author identity =="
gh api "repos/${repo}/pulls/${number}" --jq \
'{number,user:{login:.user.login,id:.user.id,type:.user.type,html_url:.user.html_url},head:{label:.head.label,sha:.head.sha},draft}'
done
echo
echo '== Recent PR Governance workflow runs for `#1433` =='
gh api "repos/${repo}/actions/workflows/pr-governance.yml/runs?event=pull_request_target&per_page=100" \
--jq '.workflow_runs[] | select(.pull_requests[]?.number == 1433) | {id,event,actor:{login:.actor.login,type:.actor.type},head_sha,conclusion,status,created_at}'
echo
echo '== GitHub webhook schema reference fields in repository (if documented) =='
rg -n -C 3 'pull_request.*user|user\.login|dependabot\[bot\]|app/dependabot' \
.github tests README.md 2>/dev/null || trueLength of output: 4644
Approved. You are interacting with an AI system. |
|
@coderabbitai full review Re-requesting: the previous request did not produce a review. The CodeRabbit commit status on The review targets are unchanged from that request — whether the author exemption is too wide, and whether the gate can now fail open. Both are restated there. Check status: all complete,
|
| Check | Result |
|---|---|
test, build, guards, lint-python, lint-frontend |
success |
Canonical issue and evidence |
success |
PR Governance |
success |
CodeQL, Security Scan (python + javascript) |
success |
bandit, python-safety, npm-audit, trivy |
success |
gitleaks (working tree), dependency-review, validate |
success |
Generate and Upload Coverage |
success |
E2E Pipeline Tests |
skipped |
Trivy (capital-T duplicate) |
neutral |
That last row is the casing collision #1410 documents: trivy and Trivy are two distinct check runs, success and neutral respectively. It does not block, and it is not introduced by this change.
agent-completion/truth-gate does not appear at all — retired by #1431.
Terminal state: HALTED(awaiting_merge_approval)
gh pr merge 1423 --squash --repo groupthinking/EventRelay
Worth restating what this unblocks, since it is the reason this PR is worth a look ahead of the others: #1433 (js-yaml 4.3.0 → 4.3.1, a security backport) fails Canonical issue and evidence and PR Governance and nothing else. Merging this clears it outright. #1171 clears the same two but stays correctly red on validate-gh-aw, which is a genuine failure owned by #1398 / #1403.
Generated by Claude Code
New evidence: a second instance appeared while this PR was in review@coderabbitai's review surfaced #1433, which did not exist when this PR was opened. It is worth recording, because it moves the argument from "one stuck PR" to "a recurring failure with a measurable rate". #1433 — Non-draft from the moment it opened, so it took no draft escape and went red immediately — the same five-section failure as #1171, for the same unmeetable reason. That is two non-draft Dependabot PRs red on this gate within roughly an hour, plus three drafts (#1000, #1173, #1176) that will go red the instant anyone marks them ready. Dependabot opens PRs continuously, so this is not a backlog to drain — it is an ongoing failure that recurs on every dependency bump until the gate stops asking Dependabot for prose it cannot write. Correction to the PR body. It described #1171 as "the only non-draft Dependabot PR open". That was true when written and is now false; I have updated it to name both. Flagging the change rather than editing quietly, since the count is load-bearing for the argument. Both #1171 and #1433 clear this gate the moment this merges. #1171 stays red on Generated by Claude Code |
|
Tip For best results, initiate chat on the files or code changes.
The corrected PR body now accurately identifies 🐇 ✏️ Learnings added
You are interacting with an AI system. |
Independent red-team pass — CodeRabbit is still rate-limitedTwo 1. Is the exemption spoofable? — No
GitHub usernames cannot contain 2. Can it fail open? — No, it fails closed
Ordering is also correct: draft check → dependabot escape → body parsing. No path reaches a passing conclusion without the contract having been evaluated for a non-exempt author. Worth noting explicitly since this is a 3. Is
|
| Check run | Origin | On the escape path |
|---|---|---|
PR Governance |
created by checks.create() in publish() |
neutral ✅ |
Canonical issue and evidence |
auto-created from the job's name: |
success |
publish() only calls core.setFailed() when conclusion === "failure". On the neutral path the job exits cleanly, so the job-level check — the one named Canonical issue and evidence, which is the name this PR's title and body use throughout and the name that appears in MERGE_POLICY.md's required list — reports success, not neutral.
Verified, not inferred. Draft PR #1425 takes the pre-existing draft escape and shows exactly this split: PR Governance = neutral, Canonical issue and evidence = success. Failures do propagate to both — #1433 and #1171 are red on both right now.
So the stated guarantee —
neutral, notsuccess: the contract is not applicable here, and reporting it as satisfied would be the same false signal this check exists to catch.
— holds for PR Governance but not for Canonical issue and evidence, which will report the contract satisfied for Dependabot.
This is inherited, not introduced. The draft escape has had the identical shape since before this diff, and correcting it means either dropping the duplicate check or restructuring how the job concludes — a change to the workflow's check topology, not a line in this escape. It does not block the PR's purpose: #1433 goes green either way, which is the point.
I'd suggest only that the neutral, not success paragraph name which check it describes, so the next reader doesn't take it as a property of the required context. Happy to file that as a follow-up issue rather than expand this diff.
Verification I ran
Checked out fb6d072 and ran the suite directly:
$ python -m pytest tests/unit/test_pr_governance_workflow.py -q
14 passed in 0.43s
Including the two narrowness guards — test_gate_still_fails_a_human_with_the_same_body and test_gate_still_fails_other_bots both assert failure. A fix that merely dropped the section requirement would pass the Dependabot test and fail these two, so they are doing real mutation-detection work.
CI on this head: every check success except Generate and Upload Coverage, still running, and Trivy neutral as it is on every PR. Nothing failing.
Terminal state: HALTED(awaiting_merge_approval) — no automerge label and main is protected, so the merge decision stays human. This remains the keystone: merging it takes #1433 (a js-yaml security backport) to fully green on its own.
Generated by Claude Code
|
Tip For best results, initiate chat on the files or code changes.
On the exemption path:
Therefore, This behavior is inherited from the draft escape. This PR does not introduce the check-topology mismatch. It also does not widen the exemption or create a fail-open path for non-exempt authors. The PR body should revise the “ 🐇 ✏️ Learnings added
You are interacting with an AI system. |
|
Confirmed, and this one lands on me — the PR body overstated the guarantee. Corrected, and split out as #1436. Verified independently before accepting it, on two PRs on the draft escape path: The mechanism is as you describe: Why this correction matters more than usual here. My body said the escape reports Scope call. I agree it is inherited from the Worth flagging for whoever configures branch protection: if The body's " Generated by Claude Code |
2 participants
Canonical issue
Closes #1419
Outcome
Canonical issue and evidenceno longer fails PRs authored bydependabot[bot].The gate requires five prose sections —
## Canonical issue,## Outcome,## Risk,## Verification,## Production evidence— plus exactly oneCloses #<issue>reference. Dependabot composes its body from a fixed template (release notes, changelog, commit list) and can emit none of them. The gate named an action the author could not take, and failed permanently.Two live instances, both non-draft and both red on this gate for exactly that reason:
Canonical issue and evidencebuild(deps): bump github/gh-aw-actions/setupbfb1bb7failurebuild(deps-dev): bump js-yaml from 4.3.0 to 4.3.1dff2ba6failure#1433 opened while this PR was in review and went red immediately. The failure text on #1171:
Three further Dependabot PRs (#1000, #1173, #1176) are drafts and take the existing
if (pr.draft)escape. Each turns red the moment it is marked ready for review.Dependabot opens PRs continuously, so this is not a backlog to drain — it recurs on every dependency bump until the gate stops asking Dependabot for prose it cannot write.
What the exemption reports — precisely
The escape calls
publish("neutral", …). That sets the conclusion on the customPR Governancecheck, whichchecks.createpublishes. It does not make the job-levelCanonical issue and evidencecheck reportneutral:publishskipscore.setFailedon this path, so the job exits 0 and GitHub records that check assuccess.PR Governancechecks.createneutralCanonical issue and evidencesuccessneutralis the right conclusion forPR Governance— the contract is not applicable, not satisfied, and reporting it satisfied would be the same false signal this check exists to catch.The job-level check does still report
success, and that is a real gap. It is inherited from theif (pr.draft)escape and predates this change — verified on two PRs on the draft path (#1423 @ e08ac36and Dependabot#1000), both showingPR Governance: neutral/Canonical issue and evidence: success. This PR does not introduce, widen, or fail-open it. Tracked with evidence and three remedies in #1436; fixing it means moving the required context toPR Governanceor restructuring the job to skip, both branch-protection decisions with their own blast radius.Raised by CodeRabbit during review. An earlier revision of this section claimed the
neutralguarantee without that distinction, which overstated what the code delivers.Relationship to #1431
This PR originally also corrected a stale comment in
pr-checks.yml. That comment justified the truth gate's owndependabot[bot]escape by asserting the canonical gate "states a requirement an author can actually meet" — false for Dependabot, which relocated the constraint instead of removing it.#1431 retired the truth gate entirely, deleting that code path, so the comment fix is moot and has dropped out of this diff. The underlying defect is unaffected: it lived in
pr-governance.yml, which #1431 did not touch. With the sibling gate gone, this escape is now the only place the exemption lives — the in-code comment says so.Scope
pr-governance.yml(author escape) andtests/unit/test_pr_governance_workflow.py(+5 behavioural tests). Two files.validate-gh-awfailure on build(deps): bump github/gh-aw-actions/setup from 0.82.14 to 0.84.2 #1171 — a genuine failure (generated lock files patched without recompiling), owned by ci: gh-aw toolchain pin is hard-coded to v0.82.14 in 4 places, making Dependabot bumps (#1171) structurally unmergeable #1398 and ci: Dependabot should not manage compiler-generated gh-aw lock files #1403. build(deps): bump github/gh-aw-actions/setup from 0.82.14 to 0.84.2 #1171 stays red on that check after this change, correctly. build(deps-dev): bump js-yaml from 4.3.0 to 4.3.1 #1433 has no such second failure and should go fully green.successtopology — ci: governance escape paths reportsuccesson the job-level check, not "not applicable" #1436, pre-existing.dependency-review,npm-audit,trivy,build,testall still apply. Only the PR-body prose contract is waived."survives the #1270 log sanitizer #1429 — a CWE-117 JSON field-forgery bypass inlogging_config.pyfound while this PR was open. Not in this diff; needs its own change.Risk
dependabot[bot]). The realistic risk is over-broad exemption;test_gate_still_fails_other_botsandtest_gate_still_fails_a_human_with_the_same_bodypin it closed. Apull_request_targetworkflow that fails to parse would fail closed on every PR, so YAML and JS syntax were both checked explicitly.pull_request.user.login, not from anything the submitter writes. Independently confirmed in review against the live REST payloads for build(deps): bump github/gh-aw-actions/setup from 0.82.14 to 0.84.2 #1171 and build(deps-dev): bump js-yaml from 4.3.0 to 4.3.1 #1433 (type: Bot, Dependabot App URL) and the recordedpull_request_targetactor.git revert. No migration, config, or schema change.Verification
Head
fb6d072. Measured, not inferred. Re-verified after #1431 landed andmainwas merged in.YAML parses and the embedded governance script passes
node --check.The new tests execute the real gate, rather than matching strings in its source. The 9 pre-existing tests in this file are source-string assertions that stay green even if the logic inverts; the 5 added ones run the script under Node against synthetic payloads and assert on the published conclusion.
Non-vacuous against current
main. Re-run after ci: retire the agent-completion truth gate #1431, withpr-governance.ymlreverted toorigin/main(8cd4a10):With the fix applied:
14 passed. Exactly one behaviour changed.The exemption is narrow. A human (
groupthinking) and a different bot (google-labs-jules[bot]) posting the identical template-less body both still getfailure. A fix that merely stopped requiring the sections would pass the Dependabot test and fail these two.No collateral damage — 26 passed across
test_pr_governance_workflow.py,test_gh_aw_workflow_governance.py,test_dependabot_automation_workflow.py. (An earlier revision cited 138 across four files;test_agent_completion_gate.pywas deleted by ci: retire the agent-completion truth gate #1431, so that count no longer applies.)PR Governance)dependabot[bot]failureneutralgroupthinkingfailurefailuregoogle-labs-jules[bot]failurefailuregroupthinkingsuccesssuccessdependabot[bot]neutralneutraluserobjectsuccesssuccessfb6d072: no failing checks.agent-completion/truth-gate, red on earlier heads, no longer runs at all — retired by ci: retire the agent-completion truth gate #1431.successon the job-level check, not "not applicable" #1436.Production evidence
Not applicable — CI workflow and test changes only, no runtime surface. Under
MERGE_POLICY.mdgate 4, previews are scoped toapps/web/**; this diff touches neither. The Vercel preview built green regardless.Agent handoff
Canonical issue and evidenceis permanently unsatisfiable for Dependabot PRs #1419Canonical issue and evidenceis permanently unsatisfiable for Dependabot PRs #1419, with thepr-checks.ymlitem resolved by ci: retire the agent-completion truth gate #1431 rather than heremain, merges cleanlyAgent provenance
Agent-authored. No
agent-lock-manifestis filled in — there is no dispatch record behind this change, and the gate that consumed manifests has since been retired by #1431.