The review-verdict/h10 exemption path decides from mutable PR state — three routes to a forged exemption
#698
Closed
opened 2026-07-28 22:32:48 +02:00 by timothy
·
3 comments
No Branch/Tag Specified
main
renovate/lucene.net
renovate/dotnet-monorepo
renovate/sqlitepclraw.bundle_e_sqlite3-3.x
renovate/meziantou.analyzer-3.x
renovate/cliwrap-3.x
913-scripted-playout-401
release/v26.15.0-notes
issue-806-guard-populations
scratch/767b-poisoned
scratch/767b-control
release/v26.14.0-notes
release/v26.14.0
docs/510-skill-logo-bug-policy
fix/510-watermark-resolution-policy
fix/629-verdict-classifier-falseopens
fix/609-decisions-edit-token-scope
issue-135-clear-to-none
release/v26.12.0-notes
fix/409b-lastscan-api-parity
fix/401-updatechannel-mirror-422
fix/327-playlist-rename-validation
fix/410-scancancel-log-level
fix/409-447-librariesscreen-neverscanned
fix/338-zap-exit-code
fix/367-plex-budget-message
fix/310-debom-legacy-cs
ci/604-lane-rebalance
feat/388-design-mirror
feat/247-test-ownership
feat/247-primary-action
feat/357-player-owned-playback
feat/357-jellyfin-plugin-poc
fix/289-mcp-hardening
issue58-mcp
feat/244-channels-extract
ci/auto-bump-prod-compose
feat/multi-rerun-collections-api
feat/collections-api
feat/quick-wins
feat/185-docs-part2
feat/140-collections-screen
feat/146-channel-edit
feat/147-classic-ui-link
issue22-renovate-dashboard
feat/91-cutover
feat/63-composite-create
feat/65-library-browse
feat/85-epg
feat/86-schedule-editor
feat/109-dashboard-data
feat/99-session-tracking
fix/dockerfile-node-tag
feat/59-spa-foundation
docs/59-ui-redesign-brief
feat/102-json-guide
feat/111-schedule-durations
feat/104-artwork-upload
feat/103-media-sources-api
feat/playouts-read-api
feat/108-health-api
feat/105-picker-list-endpoints
issue-97-channel-state-api
issue42-jellyfin-musicvideos
issue46-rest-api-error-contract
dependabot/nuget/ErsatzTV.FFmpeg.Tests/multi-d307a2e06f
qsv-improvements
hdr-vulkan-cuda-test
v26.16.0
v26.15.0
v26.14.0
v26.13.0
v26.12.0
v26.11.0
v26.10.0
v26.9.0
v26.8.0
v26.7.0
blazor-final
v26.6.0
v26.5.0
v26.4.0
v26.3.1
v26.3.0
v26.2.0
v26.1.1
v26.1.0
v25.9.0
v25.8.0
v25.7.1
v25.7.0
v25.6.0
v25.5.0
v25.4.0
v25.3.1
v25.3.0
v25.2.0
v25.1.0
v0.8.8-beta
v0.8.7-beta
v0.8.6-beta
v0.8.5-beta
v0.8.4-beta
v0.8.3-beta
v0.8.2-beta
v0.8.1-beta
v0.8.0-beta
v0.7.9-beta
v0.7.8-beta
v0.7.7-beta
v0.7.6-beta
v0.7.5-beta
v0.7.4-beta
v0.7.3-beta
v0.7.2-beta
v0.7.1-beta
v0.7.0-beta
v0.6.9-beta
v0.6.8-beta
v0.6.7-beta
v0.6.6-beta
v0.6.5-beta
v0.6.4-beta
v0.6.3-beta
v0.6.2-beta
v0.6.1-beta
v0.6.0-beta
v0.5.8-beta
v0.5.7-beta
v0.5.6-beta
v0.5.5-beta
v0.5.4-beta
v0.5.3-beta
v0.5.2-beta
v0.5.1-beta
v0.5.0-beta
v0.4.5-alpha
v0.4.4-alpha
v0.4.3-alpha
v0.4.2-alpha
v0.4.1-alpha
v0.4.0-alpha
v0.3.8-alpha
v0.3.7-alpha
develop
v0.3.6-alpha
v0.3.5-alpha
v0.3.4-alpha
v0.3.3-alpha
v0.3.2-alpha
v0.3.1-alpha
v0.3.0-alpha
v0.2.5-alpha
v0.2.4-alpha
v0.2.3-alpha
v0.2.2-alpha
v0.2.1-alpha
v0.2.0-alpha
v0.1.5-alpha
v0.1.4-alpha
v0.1.3-alpha
v0.1.2-alpha
v0.1.1-alpha
v0.1.0-alpha
v0.0.62-alpha
v0.0.61-alpha
v0.0.60-alpha
v0.0.59-alpha
v0.0.58-alpha
v0.0.57-alpha
v0.0.56-alpha
v0.0.55-alpha
v0.0.54-alpha
v0.0.53-alpha
v0.0.52-alpha
v0.0.51-alpha
v0.0.50-alpha
v0.0.49-prealpha
v0.0.48-prealpha
v0.0.47-prealpha
v0.0.46-prealpha
v0.0.45-prealpha
v0.0.44-prealpha
v0.0.43-prealpha
v0.0.42-prealpha
v0.0.41-prealpha
v0.0.40-prealpha
v0.0.39-prealpha
v0.0.38-prealpha
v0.0.37-prealpha
v0.0.36-prealpha
v0.0.35-prealpha
v0.0.34-prealpha
v0.0.33-prealpha
v0.0.32-prealpha
v0.0.31-prealpha
v0.0.30-prealpha
v0.0.29-prealpha
v0.0.28-prealpha
v0.0.27-prealpha
v0.0.26-prealpha
v0.0.25-prealpha
v0.0.24-prealpha
v0.0.23-prealpha
v0.0.22-prealpha
v0.0.21-prealpha
v0.0.20-prealpha
v0.0.19-prealpha
v0.0.18-prealpha
v0.0.17-prealpha
v0.0.16-prealpha
v0.0.15-prealpha
v0.0.14-prealpha
v0.0.13-prealpha
v0.0.12-prealpha
v0.0.11-prealpha
v0.0.10-prealpha
v0.0.9-prealpha
v0.0.8-prealpha
v0.0.7-prealpha
v0.0.6-prealpha
v0.0.5-prealpha
v0.0.4-prealpha
v0.0.3-prealpha
v0.0.2-prealpha
v0.0.1-prealpha
Labels
Clear labels
ad-hoc
api
bug
ci-cd
content
dependencies
enhancement
frontend
in-progress
jellyfin
parked
priority: high
priority: low
priority: medium
review
security
One-off / ad-hoc work not tracked by a dedicated issue
REST API / HTTP endpoints
Something isn't working
Build, test, deploy pipeline
Channel content / schedules / playlists
Dependency updates (Renovate)
New feature or improvement
ChicoryTV React SPA frontend
Claimed by an active session — do not pick up
Jellyfin tuner / IPTV integration
Excluded from automatic queue pickup; work only when explicitly selected
Adversarial review finding
Security / vulnerability fix
Milestone
No items
No Milestone
Projects
Clear projects
No projects
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: timothy/ersatztv#698
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Found by cross-family adversarial review of the #672 PR. None of these are introduced by #672 — they predate it and survive it. They share one root cause, so they are filed together: the exemption decision in
.gitea/workflows/review-verdict.ymlis derived from mutable or unattributed PR state, and a machine-writtensuccessis never revalidated.priority: highrather than medium because unlike #672 and #697 these need no workflow rewrite and no credential — routes 1 and 2 are driven entirely through ordinary PR operations.Route 1 — retarget race against the docs-only classifier
H(malicious) and scratch baseS, chosen soHvsSis docs-only.H → main. The trusted base workflow starts.Swhile the job is queued or running.S, so the diff reads docs-only.scripts/pr-changed-files.shrevalidates only.head.sha— never.base.ref/.base.sha— so its exhaustiveness guarantee holds while the base moved underneath it. The job postsh10=successonH.main.editedis not in the workflow'stypes:, so nothing reclassifies, and the forged exemption stands onH.Note this is the exact shape of the #632 retarget hazard, one level down: that one is detected on the hook path by comparing the recorded base branch, and this one happens inside the workflow that writes the status.
Route 2 — hijack an open Renovate PR
The bot exemption reads
pull_request.user.login, the immutable PR creator, not whoever pushed the current head. Force-push arbitrary application code onto an open Renovate branch: the PR is still authored byrenovate, touches noPROTECTEDpath, and the workflow postsh10=success.Route 3 — inherited success is never reclassified
The "already decided for this sha" short-circuit exits on any existing
successbefore checking PR number, base, author or files. That guard exists for a good reason (never overwrite a real human verdict), but it cannot currently tell a human verdict from a machine exemption — so asuccessobtained via route 1, route 2, a sibling workflow (#697), a direct API call, or inherited across PRs by sha (#663) is accepted unchanged.Why one issue
All three are "the gate trusts something it cannot attribute." A fix that distinguishes trusted human verdicts from machine exemptions by issuer and structured provenance, and revalidates machine-written success against the live PR/base, addresses all three. Fixing them one at a time invites exactly the drift
ci.shared-pr-file-enumerationwas written about.Done-when
main, retargeted mid-run, grantedExempt: docs-onlyover a diff containing a C# file, retarget back reclassified nothing). Closed unmerged, branches deleted, no forgedh10left anywhere. The "then shown to fail after the fix" half is not observable pre-merge —pull_request_targetresolves this workflow's definition frommain(the self-test gap inci.gate-trigger-base-resolved). Covered pre-merge by executed-behaviour tests; live re-check onmainright after merge.renovate, and the only route is pushing code onto an open Renovate branch. Attempted on #679 behind a WIP guard; the permission classifier blocked the force-push needed to restore the branch, so it was abandoned and #679 was returned to byte-identical original state. Deferred deliberately rather than ticked on a unit test alone.successdistinguishable and revalidated — by.creator.login(non-null) and aReview-verdict:description and a matching recorded base. The never-overwrite-a-human-verdict property is preserved and separately tested forsuccessandfailure, plus a mid-run verdict case.editeddecided and empirically verified — it fires. Verified via anedited-only probe workflow on a scratch base branch (the self-test gap blocks testing onmain); the run was created, which only happens if the event fired and thebranches:filter matched the new base. Honest limit: the job never got a runner slot, so this is the run's creation, not its log line. Recorded as giving detection, not atomicity — the ordering residual is #706.release.verdict-status-check+ci.gate-trigger-base-resolvedupdated, plus new recordsci.exemption-provenanceandci.grep-q-pipefail-inversion, andci.shared-pr-file-enumeration(contract now 5 args).Residuals deliberately left open, not silently dropped: #706 (status writes are not serialized — ABA retarget + human-rejection race), #707 (paging vs a live-advancing base), #708 (route-2 live reproduction), #697 (credentials that can write the status — a provenance check is not an authentication one).
Related
#672 (definition rewrite — fixed), #697 (credentials that can write the status), #663 (status is repo-global per sha), #632 (retarget detection on the hook path).
Claiming this (Claude Code orchestrator session, 2026-07-29). Selector ranked it top of
priority: highwith clear deps; pre-claim checks were clean — no open PR referencing #698, no remote branch naming it, no prior claiming comment.Scope note for parallel sessions: I am taking #698 only (the exemption-path logic in
.gitea/workflows/review-verdict.yml, routes 1–3). #697 is NOT claimed — its durable fix is a credential/infra change (job-token permissions, secret scoping) that belongs partly to server-management, and I'd rather not entangle a workflow-logic fix with a secrets rollout. If someone picks up #697 concurrently, expect a textual conflict only inreview-verdict.yml's status-posting step.PR #705 opened. Status against this issue's
## Done-when, stated honestly rather than optimistically:H → main, retargeted mid-run to a scratch base, the job read docs-only and postedh10=success — "Exempt: docs-only change"; retargeted back tomainand nothing reclassified (created_at == updated_at), leaving a PR intomainwhose diff carried a C# file behind a green required check. The "then shown to fail after the fix" half is not observable pre-merge —pull_request_targetresolves this workflow's definition frommain(the self-test gap inci.gate-trigger-base-resolved). Covered pre-merge by executed-behaviour tests, verified live onmainright after merge.renovate, so the only faithful reproduction is pushing code onto an open Renovate branch. I set a WIP guard on #679 (confirmeddraft: true,mergeable: false) and the permission classifier then blocked the force-push required to restore the branch afterwards. Rather than mutate a live dependency PR with no clean way back, I stopped and restored #679 to byte-identical original state (title restored,draft: false, head still378171e). Nothing was ever pushed to its branch. Covered by unit tests + the measured allow-list instead.successdistinguishable and revalidated — by.creator.login(non-null) and aReview-verdict:description. Measured on the combined endpoint the job reads: user-posted statuses carry a creator, Actions-posted ones carrynull. The never-overwrite-a-human-verdict property is preserved and separately tested for bothsuccessandfailure.editeddecided and empirically verified — it fires. Since the self-test gap blocks testing onmain, anedited-only probe workflow went on a scratch base branch and a PR was retargeted onto it; the run was created, which only happens if the event fired and thebranches:filter matched the new base. Limit worth recording: the job never got a runner slot, so this is the run's creation, not its log line.release.verdict-status-check+ci.gate-trigger-base-resolvedupdated — plus a new recordci.exemption-provenanceandci.shared-pr-file-enumeration(contract is now 5 args).Two corrections from the work, recorded because they change what the issue body claims:
h10status. It is a valid counterfactual (identity-only would have exempted it), not a historical incident, and the record says so.compare/{base}...{head}returns nofileson Gitea 1.25.4, and a--depth=1fetch of the two shas has no merge base. The fix is a base binding with a stated residual window, closed durably byedited+ re-derivation rather than perfectly.Closing record
Outcome: Fixed and merged as PR #705 (8 commits,
57e33f9→mainata3458e6). All three routes closed in the exemption path: the base is bound as a required 5th argument to the shared enumeration and re-checked either side of paging; the bot exemption now requires every path to be a dependency manifest, not just a matching author; and the never-overwrite short-circuit fires only for a status positively identified as a human verdict for this base.editedadded totypes:.Root cause: one shape, three symptoms — the exemption decision rested on inputs the judged PR could mutate or that could not be attributed.
/pulls/{n}/filesdiffs against the live base (retargetable),pull_request.user.loginis the PR's immutable creator rather than whoever pushed the head, and a machine-writtensuccesswas indistinguishable from a human verdict. Two further defects surfaced during review and share the theme: a pre-existinggrep -q+pipefailinversion that let a large PR skip thePROTECTEDguard entirely, and the human-verdict path being bindable to a different base.Decisions/conventions changed: new
ci.exemption-provenanceandci.grep-q-pipefail-inversion; updatedci.gate-trigger-base-resolved(theeditedresidual is resolved, and it was more serious than that record implied) andci.shared-pr-file-enumeration(contract is now 5 args).Reusable knowledge:
producer | grep -q…underset -o pipefailinverts on large input. grep exits at first match, the producer takes SIGPIPE (141), and a MATCH reads as a failed pipeline. A here-string is not the fix — bash spills large ones to temp storage and fails the same way. Count withgrep -c, evaluate once at top level, validate numeric.main)eviltruncating tomain.PROTECTEDguard survived three review rounds with three "protected path" tests green. Assert the discriminator; add a cheap stderr sweep forcommand not found/integer expression expected/unbound variable.Verification: 385 tests (17+ new), each new guard mutation-verified red-then-green with positive controls so a deadlock cannot masquerade as a fix. Six cross-family adversarial rounds: BLOCKED (3 Blockers, 2 Highs) → BLOCKED (2H, 3M) → BLOCKED (3M) → BLOCKED (1M) → BLOCKED (1 High) → MERGEABLE. Route 1 reproduced live pre-fix (#703) and verified to fail post-fix (#709:
success — Exempt: docs-onlybecamepending, on a diff containing a C# file).editedand thebranches: [main]fail-closed behaviour both confirmed live. CI green on the merged head.Deferred: #706 (status writes are not serialized — ABA retarget + human-rejection race;
editedgives detection, not atomicity), #707 (offset paging against a live-advancing base), #708 (route-2 live reproduction — permission-blocked here), #697 (credentials that can write the status; this is a provenance check, not an authentication one).Docs updated:
docs/ci-cd.md,docs/decisions/records/ci/exemption-provenance.md(new),docs/decisions/records/ci/grep-q-pipefail-inversion.md(new),docs/decisions/records/ci/gate-trigger-base-resolved.md,docs/decisions/records/ci/shared-pr-file-enumeration.md, regenerateddocs/decisions/README.md.Corrections made during the work, recorded because they were load-bearing: the claim that Renovate PR #20 had been exempted for a source change was false (it predates the gate — it is a counterfactual, not an incident); pinning the diff to two shas is impossible on Gitea 1.25.4 (
comparereturns nofiles,--depth=1has no merge base); and the assertion that a)in a branch name "mismatches — safe direction" was false for any branch starting with the target base.