Skip to content

Screenshot-table gate's presence-mode check has no correlation to the PR's current head SHA, so a stale/superseded screenshot table satisfies a close-on-violation gate indefinitely #6417

Description

@JSONbored

Severity: HIGH
Location: src/review/screenshot-table-gate.ts:368

Found via a full-system adversarial audit of the ORB review/scoring engine (2026-07-15) — 2 independent skeptic passes, both had to vote "confirmed" for this to survive.

Failure scenario

evaluateScreenshotTableGate (src/review/screenshot-table-gate.ts:343-373) has two evidence paths: (1) botCaptureSatisfied, computed in src/queue/processors.ts:2748 as Boolean(pr.headSha) && pr.visualCaptureSatisfiedSha === pr.headSha — correctly tied to the live head SHA; and (2) the PRESENCE-mode fallback at lines 368-372, hasImageBearingMarkdownTable(input.prBody) && !hasImageOutsideTable(...) && !hasCommittedImageFile(...) — pure regex/string checks over pr.body text with NO SHA, timestamp, or diff-content input at all. Per the doc comment at src/queue/processors.ts:2280-2287, the whole maintenance planner (including this gate call at :2749-2759) reruns 'after the gate runs on a PR webhook' — i.e. on every relevant webhook, including synchronize (new pushes), re-reading pr.body fresh from the DB but NOT requiring it to have changed. Concrete trigger: a contributor opens a visual PR and pastes a genuine before/after screenshot table on push #1 (gate evaluates hasTable=true → NO_VIOLATION, and with action: 'close' this clears the gate). They then push commits #2-#5 that substantially change the actual UI, without editing the PR body or re-uploading screenshots. Every subsequent webhook re-evaluation still finds the SAME unchanged table+image markup in pr.body → still NO_VIOLATION. The image URLs themselves are only checked with a general SSRF/protocol guard (isSafeHttpUrl in src/queue/processors.ts:7341), not tied to GitHub's upload CDN or this PR/commit in any way, so even a screenshot copy-pasted from an unrelated PR would satisfy the check.

Impact

A gate specifically built to guarantee 'unreviewable at a glance without before/after evidence' (per the file's own top comment) can be permanently satisfied by evidence that no longer reflects the merged code. A maintainer relying on this deterministic, zero-hallucination check as proof the final diff was visually verified is wrong: a visual regression introduced after the screenshots were taken ships with the gate showing green, defeating the check's entire purpose. This is a real gap in a mechanism explicitly designed to prevent exactly this kind of gaming (contrast with the bot-capture path in the same function, which correctly ties evidence to the live head SHA).

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions