fix(reviewer): gate auth-prompt detection on absence of review structure

[doublegate]

Summary

Follow-up to #332 (the OAuth-login-prompt guard). Adopts a blocking finding from the agy reviewer's own adversarial review of the sibling sync PRs (doublegate/RustySNES#259, doublegate/RustyN64#207): the initial is_auth_prompt() matched the auth signature anywhere in agy's output, so a genuine review that quotes those strings — one reviewing this very script (whose diff contains them), or a PR implementing OAuth — would be wrongly discarded in the retry loop (Layer 1) or blocked from posting (Layer 3).

Fix

is_auth_prompt() now requires both:

1. the auth signature is present (OAuth endpoint URL / agy's literal login-prompt lines), and
2. the mandated review structure is absent — the reviewer prompt requires a ### Blocking issues section in every review (agy writes "None found." when clean), which agy's raw interactive login flow never contains.

So a real review keeps its Blocking issues section and posts; only the raw login flow (signature present, structure absent) is rejected. On a healthy runner, behavior is unchanged.

Verification

Six cases, all pass:

Case	Expected
Raw login flow (no review structure)	blocked ✓
Review quoting the auth strings, with ### Blocking issues	posts ✓ (the false positive agy flagged)
Benign auth-discussing review	posts ✓
Empty output	posts nothing (via have_text) ✓
Real review body containing the URL	posts ✓
Login flow slipped into the wrapped body	blocked ✓ (Layer 3 backstop)

bash -n clean. Byte-identical (same sha256) with the template and the RustySNES/RustyN64 sync PRs.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

@doublegate, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 05ad9f55-9e05-48a6-82f3-ccb57c5e9ed6

📥 Commits

Reviewing files that changed from the base of the PR and between 90c74c6 and 1399295.

📒 Files selected for processing (1)

• scripts/agy-review.sh

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Antigravity review (Gemini via Ultra)

This PR updates is_auth_prompt in scripts/agy-review.sh to skip authentication prompt detection whenever a Markdown section header for blocking issues (^#{2,4}[[:space:]]+Blocking issues) is detected in the input file.

Blocking issues

• Security / Sensitive Data Leak: Short-circuiting is_auth_prompt when ^#{2,4}[[:space:]]+Blocking issues is present disarms the guard. If an output contains both a section header and a live OAuth URL or interactive login prompt (e.g., from interleaved process output, stderr captured alongside stdout, or a credential leak during review execution), is_auth_prompt returns 1 (false) and allows sensitive authentication URLs or prompts to be posted publicly to PR comments.
• Missing Tests: The project style guide mandates that tests accompany behavior changes. This change alters security-critical detection logic in scripts/agy-review.sh without adding test cases.

Suggestions

• agy-review.sh:50: Instead of completely skipping auth-prompt detection when a header exists, sanitize or redact detected OAuth URLs and authentication prompt signatures from the text prior to posting, or use a structured output format rather than line-based heuristics.

Nitpicks

• agy-review.sh:50: The pattern ^#{2,4}[[:space:]]+Blocking issues requires an exact trailing string match and will fail to recognize valid section headers containing trailing colons or punctuation (e.g., ### Blocking issues:).

_{Automated first-pass review by agy on a self-hosted runner -- not a human review.}

[doublegate]

Review adjudication (all rounds) — resolved via redesign

Thanks to agy for the multi-round adversarial pass. The findings across rounds were genuinely in tension: round 1 flagged the signature match as too broad (would suppress a real review that discusses auth), while rounds 2–3 flagged the ### Blocking issues exemption as too loose (a header appearing alongside a URL, or the phrase in prose, could disarm the guard → leak), and also as too strict (case-sensitive header → false positive on title-case). Chasing a "does this look like a review?" heuristic can't satisfy all of these at once.

Redesign — key the guard on the live OAuth URL itself, not on review structure:

• oauth_url_present() — Layer 3, now unconditional: refuse to post any body containing a live Google OAuth authorization URL (https://accounts.google.com/o/oauth2/…). No header/structure exemption, so an interleaved URL can't disarm it (fixes the round-3 leak finding), and it's immune to header case and trailing punctuation (fixes the case-sensitivity / :-suffix findings).
• is_auth_prompt() — Layer 1: detects a lapsed session via the live URL (always present in agy's login flow) plus agy's terminal failure line; gates only the retry / clean-exit UX, never the security guarantee.

A genuine review quotes this script's bare regex (host + path, no scheme) or discusses auth in prose — neither is a live URL — so reviewing the reviewer or an OAuth-related PR no longer false-positives (fixes round 1). Confirmed the script does not self-match.

Verification — 7 cases, each mapping to a scenario raised across the three rounds:

case	Layer 3	Layer 1
raw login flow (URL)	block	lapse
review quoting the bare regex	post	review
title-case header + auth prose	post	review
header + : + interleaved live URL	block	lapse
failure-line-only lapse	post	lapse
empty	post	review
real review body (bare regex)	post	review

bash -n clean; byte-identical (same sha256) across the template and all three Rusty* repos.

Not adopted, with rationale:

• Missing unit tests (project style guide): verification is documented above (7 cases); there is no shell-test harness wired in any of these repos, so a committed test would be net-new infra. Reasonable as a dedicated follow-up rather than folded into this focused security fix.
• $GITHUB_STEP_SUMMARY on auth failure (nice-to-have): the lapse already logs an actionable message to the job log; a Step Summary line is a good future QoL addition.
• Layer 3 "redundant" (round-1 nitpick): intentional defense-in-depth — Layer 1 guards agy's raw capture, Layer 3 guards the fully-assembled body; they cover different artifacts.

[doublegate]

Round-4 findings adopted

Both round-4 blocking items on the OAuth-URL guard were valid and are fixed:

• Trailing slash — OAUTH_URL_RE no longer requires /o/oauth2/ with a trailing slash, so a URL like .../o/oauth2?client_id=… or bare .../o/oauth2 is now caught. The https?:// scheme is still required (matching is also now case-insensitive), so a review quoting this script's bare host/path pattern still does not match — no safety lost.
• Overly broad fallback — removed the secondary authentication failed or timed out substring match and collapsed both layers onto the single oauth_url_present check. The live URL is always present in a real login flow, so it is necessary and sufficient on its own; a review that merely discusses auth-timeout handling no longer gets aborted.

The guard is now one case-insensitive grep for a live OAuth authorization URL, applied unconditionally at both the retry-loop capture and the assembled body. Re-verified (no-slash / query-only URLs blocked; bare-pattern quote and auth discussion post; no self-match). The ### Blocking issues / header / colon and "missing tests" points from earlier rounds are addressed or adjudicated in the prior comment.

[doublegate]

Convergence note (RustyNES) — why this PR's own agy run fails, and why that's expected

The reviewer track has converged: the sibling sync PRs (doublegate/RustySNES#259, doublegate/RustyN64#207) both now report Blocking: None found on the final URL-only guard, with zero live-session (code_challenge=) markers anywhere — i.e. no real leak at any point.

This PR's own agy run fails, and that is a bootstrap artifact, not a defect. #333's reviewer executes the script on RustyNES main — which still carries the initial (signature-only) guard from #332. When agy reviews a change to this very script, its review text quotes accounts.google.com/o/oauth2, and that initial guard's is_auth_prompt "$body_file" matches the quote and refuses to post (exit 1) — so the run fails and the comment above is stale (its ^### Blocking issues finding refers to code this PR has already removed). Merging this PR replaces that guard with the URL-only oauth_url_present (scheme required → a quoted bare pattern no longer matches), which ends the self-suppression. The review check is not a required status check, so this does not block the merge.

Stale/adjudicated items in the comment above:

• ^#{2,4} Blocking issues header exemption / trailing-colon — obsolete: the header heuristic was removed entirely; the guard is now a single scheme-anchored OAuth-URL match with no structure exemption.
• Missing tests — adjudicated (declined with rationale): the logic is a single documented grep verified against a documented case matrix; there is no shell-test harness wired in these repos, so a committed test is net-new infra better handled as a dedicated follow-up.

Recommended merge order: #333 first (fixes RustyNES main's reviewer), then the two sync PRs.