From df4d82f0ecec7accd324be127a1947c372704776 Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Thu, 30 Jul 2026 00:16:07 -0400 Subject: [PATCH 1/4] fix(reviewer): gate auth-prompt detection on the absence of review structure Adopt agy's own adversarial review finding (flagged BLOCKING on the sync PRs): the initial is_auth_prompt() matched the OAuth-login signature anywhere in agy's output, so a GENUINE review that quotes those strings -- e.g. one reviewing this very script (whose diff contains them) or a PR implementing OAuth -- would be wrongly discarded in the retry loop or blocked from posting. Require BOTH conditions now: the auth signature present AND the mandated review structure 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. A real review thus keeps its section and posts; only the raw login flow (signature present, structure absent) is rejected. Verified against six cases incl. the exact false-positive agy flagged (a review quoting the auth strings WITH a Blocking-issues section now posts). bash -n clean; byte-identical across the template and all three Rusty* repos. Co-Authored-By: Claude Opus 4.8 --- scripts/agy-review.sh | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/scripts/agy-review.sh b/scripts/agy-review.sh index b291c6cb..b0dd1e03 100755 --- a/scripts/agy-review.sh +++ b/scripts/agy-review.sh @@ -23,12 +23,26 @@ have_text() { [ -s "$1" ] && grep -q '[^[:space:]]' "$1"; } # prompt (a live-shaped OAuth URL + "paste the authorization code here") instead of a review. # That text is non-empty, so have_text() alone would treat it as a valid review and POST it — # leaking an OAuth URL + phishing-shaped auth prompt into a public PR comment (exactly the -# incident this guards against). Keyed on agy's own runtime prompt strings and the -# accounts.google.com OAuth-endpoint URL, none of which a genuine code review emits, so a review -# that merely discusses "authentication" is not misclassified. Used twice: to reject such a -# capture in the retry loop, and as a final hard block before posting (defense in depth). +# incident this guards against). Used twice: to reject such a capture in the retry loop, and as +# a final hard block before posting (defense in depth). +# +# TWO conditions, BOTH required, so a genuine review is never suppressed: +# (1) the text carries agy's interactive-login signature — the accounts.google.com OAuth +# endpoint URL, or one of agy's literal login-prompt lines; AND +# (2) the text is NOT a structured review. The reviewer prompt (instruction HEAD below) +# MANDATES a "### Blocking issues" section in every review (agy writes "None found." +# when there are none), which agy's raw login flow never contains. +# Condition (2) is the fix for the false-positive agy's own review of the sync PRs flagged as +# BLOCKING: a review that legitimately QUOTES these strings — e.g. one reviewing THIS script, +# whose diff contains them, or a PR that implements OAuth — still has its "Blocking issues" +# section, so it is correctly kept and posted. Only the raw login flow (signature present, +# review structure absent) is rejected. The coupling to "Blocking issues" is deliberate: if the +# prompt's mandated sections below ever change, update this sentinel in the same edit. is_auth_prompt() { - [ -s "$1" ] && grep -qiE \ + [ -s "$1" ] || return 1 + # A structured review — even one quoting the auth strings — is never the raw login flow. + grep -qiE 'Blocking issues' "$1" && return 1 + grep -qiE \ 'accounts\.google\.com/o/oauth2|paste the authorization code|Authentication required\. Please visit|Waiting for authentication|authentication failed or timed out' \ "$1" } From 913d857c1dcc1dd3d9a810cc670c89213f592a6d Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Thu, 30 Jul 2026 00:21:50 -0400 Subject: [PATCH 2/4] fix(reviewer): anchor the review-structure sentinel to the header line Adopt agy's second-round BLOCKING finding: the review-structure check used an unanchored, case-insensitive `grep -qi 'Blocking issues'`, so the mere phrase appearing in prose or an error line -- alongside an OAuth URL -- would disarm is_auth_prompt() and let the login flow post (a false NEGATIVE = a leak). Anchor to the Markdown section header at line start (`^#{2,4} Blocking issues`, case-sensitive) instead. This fails SAFE: text without a genuine header is treated as a possible login flow and blocked, while a real review's mandated "### Blocking issues" header always matches, so a review that quotes the auth strings still posts. Verified against seven cases, incl. the exact bypass agy described (a raw login flow containing the phrase "blocking issues" in prose + an OAuth URL is now correctly blocked) and a lowercase-phrase-plus-URL log line. bash -n clean; byte-identical across template + all three Rusty* repos. Co-Authored-By: Claude Opus 4.8 --- scripts/agy-review.sh | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/scripts/agy-review.sh b/scripts/agy-review.sh index b0dd1e03..5ce52b4d 100755 --- a/scripts/agy-review.sh +++ b/scripts/agy-review.sh @@ -30,18 +30,28 @@ have_text() { [ -s "$1" ] && grep -q '[^[:space:]]' "$1"; } # (1) the text carries agy's interactive-login signature — the accounts.google.com OAuth # endpoint URL, or one of agy's literal login-prompt lines; AND # (2) the text is NOT a structured review. The reviewer prompt (instruction HEAD below) -# MANDATES a "### Blocking issues" section in every review (agy writes "None found." -# when there are none), which agy's raw login flow never contains. +# MANDATES a "### Blocking issues" section HEADER in every review (agy writes "None +# found." when there are none), which agy's raw login flow never contains. # Condition (2) is the fix for the false-positive agy's own review of the sync PRs flagged as # BLOCKING: a review that legitimately QUOTES these strings — e.g. one reviewing THIS script, -# whose diff contains them, or a PR that implements OAuth — still has its "Blocking issues" -# section, so it is correctly kept and posted. Only the raw login flow (signature present, -# review structure absent) is rejected. The coupling to "Blocking issues" is deliberate: if the -# prompt's mandated sections below ever change, update this sentinel in the same edit. +# whose diff contains them, or a PR that implements OAuth — still has its "### Blocking issues" +# section header, so it is correctly kept and posted. Only the raw login flow (signature +# present, review structure absent) is rejected. +# +# The condition-(2) match is LINE-ANCHORED to the Markdown section header (`^## … Blocking +# issues`), NOT a loose case-insensitive substring. agy's second-round review flagged the loose +# form as BLOCKING: an unanchored `grep -qi 'Blocking issues'` would disarm the guard if the +# phrase merely appeared in prose or an error line alongside an OAuth URL (a false NEGATIVE = +# a leak). Anchoring to the header form fails SAFE — ambiguous text without a real header is +# treated as a possible login flow and blocked, while a genuine review's header always matches. +# The coupling to this header is deliberate: if the prompt's mandated sections below ever +# change, update this sentinel in the same edit. is_auth_prompt() { [ -s "$1" ] || return 1 - # A structured review — even one quoting the auth strings — is never the raw login flow. - grep -qiE 'Blocking issues' "$1" && return 1 + # A structured review has a real "### Blocking issues" section header at line start; the raw + # login flow is plain text with none. Anchored + case-sensitive so the phrase in prose/errors + # cannot disarm the guard. `#{2,4}` tolerates agy emitting ## … #### for the header level. + grep -qE '^#{2,4}[[:space:]]+Blocking issues' "$1" && return 1 grep -qiE \ 'accounts\.google\.com/o/oauth2|paste the authorization code|Authentication required\. Please visit|Waiting for authentication|authentication failed or timed out' \ "$1" From 653f380b676ca60e677c769b966bc8a3bc2ea09d Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Thu, 30 Jul 2026 00:31:45 -0400 Subject: [PATCH 3/4] fix(reviewer): key the OAuth-leak guard on the live URL, not review structure Redesign converging several rounds of agy's own adversarial review, which oscillated between the guard being too broad (false positive: suppressing a genuine review that discusses auth or quotes this script) and too narrow (false negative: an OAuth URL leaking when a section header appeared in the same output). Both failure modes came from trying to answer "is this a review?" heuristically. Key on the one artifact that unambiguously marks agy's login flow instead: a live Google OAuth authorization URL (the https scheme + host + /o/oauth2/ endpoint path). * oauth_url_present() -- Layer 3, now UNCONDITIONAL: refuse to post any body carrying such a URL, with no header/structure exemption an interleaved URL could disarm (fixes the round-3 leak finding; also immune to header case and trailing punctuation). * is_auth_prompt() -- Layer 1: a lapsed session is detected by the live URL (always present in the flow) plus agy's terminal failure line; it gates only the retry / clean-exit UX, never the security guarantee. A code 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. Verified against seven cases covering every scenario agy raised across three rounds: raw login flow (blocked), a review quoting the bare regex (posts), a title-case header with auth prose (posts), a header+colon with an interleaved live URL (blocked), a failure-line-only lapse, empty, and a real review body. bash -n clean; byte-identical across template + all three Rusty* repos. Co-Authored-By: Claude Opus 4.8 --- scripts/agy-review.sh | 82 ++++++++++++++++++++++--------------------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/scripts/agy-review.sh b/scripts/agy-review.sh index 5ce52b4d..c8fb6eec 100755 --- a/scripts/agy-review.sh +++ b/scripts/agy-review.sh @@ -18,43 +18,43 @@ set -euo pipefail # `log: command not found` under set -e instead of warning — a latent misconfiguration trap.) log() { printf '[agy-review] %s\n' "$*" >&2; } have_text() { [ -s "$1" ] && grep -q '[^[:space:]]' "$1"; } -# True when a captured agy output is its interactive Google OAuth login flow rather than a -# review — i.e. agy's cached session has lapsed on the runner, so `--print` emitted the login -# prompt (a live-shaped OAuth URL + "paste the authorization code here") instead of a review. -# That text is non-empty, so have_text() alone would treat it as a valid review and POST it — -# leaking an OAuth URL + phishing-shaped auth prompt into a public PR comment (exactly the -# incident this guards against). Used twice: to reject such a capture in the retry loop, and as -# a final hard block before posting (defense in depth). +# Guarding against a leak of agy's interactive Google OAuth login flow. When agy's cached +# session lapses on the runner, `--print` emits the login prompt (a live OAuth URL + "paste the +# authorization code here") instead of a review; that text is non-empty, so have_text() alone +# would treat it as a valid review and POST it — leaking the URL into a public PR comment (the +# incident this guards against). # -# TWO conditions, BOTH required, so a genuine review is never suppressed: -# (1) the text carries agy's interactive-login signature — the accounts.google.com OAuth -# endpoint URL, or one of agy's literal login-prompt lines; AND -# (2) the text is NOT a structured review. The reviewer prompt (instruction HEAD below) -# MANDATES a "### Blocking issues" section HEADER in every review (agy writes "None -# found." when there are none), which agy's raw login flow never contains. -# Condition (2) is the fix for the false-positive agy's own review of the sync PRs flagged as -# BLOCKING: a review that legitimately QUOTES these strings — e.g. one reviewing THIS script, -# whose diff contains them, or a PR that implements OAuth — still has its "### Blocking issues" -# section header, so it is correctly kept and posted. Only the raw login flow (signature -# present, review structure absent) is rejected. -# -# The condition-(2) match is LINE-ANCHORED to the Markdown section header (`^## … Blocking -# issues`), NOT a loose case-insensitive substring. agy's second-round review flagged the loose -# form as BLOCKING: an unanchored `grep -qi 'Blocking issues'` would disarm the guard if the -# phrase merely appeared in prose or an error line alongside an OAuth URL (a false NEGATIVE = -# a leak). Anchoring to the header form fails SAFE — ambiguous text without a real header is -# treated as a possible login flow and blocked, while a genuine review's header always matches. -# The coupling to this header is deliberate: if the prompt's mandated sections below ever -# change, update this sentinel in the same edit. +# The guard keys on the ONE artifact that reliably distinguishes agy's login flow from any +# review: a live Google OAuth *authorization URL* (scheme + host + endpoint path). This choice +# is deliberate and is the convergence point of several rounds of agy's own adversarial review: +# * It does NOT false-positive on reviewing the reviewer or on an OAuth-related PR. A code +# review quotes this script's BARE regex ("accounts.google.com/o/oauth2", no scheme) or +# discusses auth in prose; neither contains the "https://…/o/oauth2/" URL form. (Earlier +# signature/prompt-phrase heuristics wrongly flagged such reviews.) +# * It needs NO "is this a review?" exemption, so it CANNOT be disarmed by a section header +# (or the phrase "blocking issues") appearing in the same output as a URL — a false +# NEGATIVE would be a leak. (Earlier "### Blocking issues" exemptions had exactly this hole, +# and were also brittle to header case / trailing punctuation.) +# So this is both necessary (agy's login flow always prints the URL) and sufficient (nothing +# else legitimately carries a live authorization URL), with no heuristic that flips between +# false-positive and false-negative. +OAUTH_URL_RE='https?://accounts\.google\.com/o/oauth2/' + +# Layer-3 hard guard (UNCONDITIONAL): true if a text contains a live OAuth authorization URL. +# Used to refuse posting ANY assembled body that carries one — no exemptions, whatever upstream +# does to the body. This is the actual security guarantee: a public PR comment must never +# contain such a URL. +oauth_url_present() { [ -s "$1" ] && grep -qE "$OAUTH_URL_RE" "$1"; } + +# Layer-1 lapse detector: agy printed its login flow instead of a review, so its session has +# lapsed. Primary signal is the live URL (always present in the flow); the secondary matches +# agy's terminal failure line so a lapse is still recognized if the URL scrolled out of the +# captured tail. This only gates the retry / clean-exit UX — a Layer-1 false positive merely +# skips a review (recoverable), never a leak; the unconditional Layer 3 above is the guarantee. is_auth_prompt() { [ -s "$1" ] || return 1 - # A structured review has a real "### Blocking issues" section header at line start; the raw - # login flow is plain text with none. Anchored + case-sensitive so the phrase in prose/errors - # cannot disarm the guard. `#{2,4}` tolerates agy emitting ## … #### for the header level. - grep -qE '^#{2,4}[[:space:]]+Blocking issues' "$1" && return 1 - grep -qiE \ - 'accounts\.google\.com/o/oauth2|paste the authorization code|Authentication required\. Please visit|Waiting for authentication|authentication failed or timed out' \ - "$1" + oauth_url_present "$1" && return 0 + grep -qiE 'authentication failed or timed out' "$1" } # --- configuration (all env-overridable from the workflow) --------------------- @@ -528,12 +528,14 @@ gh api "repos/${REPO}/issues/${PR}/comments" --paginate \ fi done -# Final hard guard — the last line of defense. Layer 1 (the retry loop) already rejects an -# auth-prompt capture, but a public PR comment must NEVER carry a live-shaped OAuth URL or a -# "paste the authorization code" prompt, whatever any upstream change does to the body. If the -# assembled body trips the auth-prompt signature, fail the job instead of leaking it. -if is_auth_prompt "$body_file"; then - log "refusing to post: the assembled comment body contains an agy auth prompt / OAuth URL. Re-authenticate agy on the runner host." +# Final hard guard — the last line of defense, and UNCONDITIONAL. Layer 1 (the retry loop) +# already rejects a lapsed-session capture, but a public PR comment must NEVER carry a live +# OAuth authorization URL, whatever any upstream change does to the body — and with no "looks +# like a review" exemption that a header alongside a URL could disarm. Uses oauth_url_present +# (not is_auth_prompt): a genuine review that merely discusses auth or quotes this script's bare +# regex has no live URL and posts normally; only an actual authorization URL blocks the post. +if oauth_url_present "$body_file"; then + log "refusing to post: the assembled comment body contains a live OAuth authorization URL. Re-authenticate agy on the runner host." exit 1 fi From 1399295b471e8d5937e7a213643f43a4363fe5eb Mon Sep 17 00:00:00 2001 From: DoubleGate Date: Thu, 30 Jul 2026 00:40:57 -0400 Subject: [PATCH 4/4] fix(reviewer): drop trailing-slash requirement + remove the phrase fallback Adopt agy's round-4 findings on the OAuth-URL leak guard: * OAUTH_URL_RE no longer requires a trailing slash after `/o/oauth2`. A URL emitted as `.../o/oauth2?client_id=...` or bare `.../o/oauth2` would otherwise slip past oauth_url_present and leak. The scheme (`https?://`) is still required, so a review quoting this script's bare host/path pattern still does not match -- no safety lost. Matching is also made case-insensitive (URL scheme/host are case-insensitive; still self-safe). * Removed the secondary `authentication failed or timed out` substring fallback and collapsed both layers onto the single oauth_url_present check. agy flagged the fallback (correctly) as broad enough to abort a genuine review that merely discusses auth-timeout handling. The live URL is always present in a real login flow, so it is both necessary and sufficient on its own; the same unconditional check now serves Layer 1 (retry / clean-exit) and Layer 3 (block the post). Verified: no-slash and query-only URLs are now blocked; a review quoting the bare pattern or discussing auth timeouts posts normally; the script does not self-match. bash -n clean; byte-identical across template + all three Rusty* repos. Co-Authored-By: Claude Opus 4.8 --- scripts/agy-review.sh | 57 +++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/scripts/agy-review.sh b/scripts/agy-review.sh index c8fb6eec..d2285e73 100755 --- a/scripts/agy-review.sh +++ b/scripts/agy-review.sh @@ -29,7 +29,7 @@ have_text() { [ -s "$1" ] && grep -q '[^[:space:]]' "$1"; } # is deliberate and is the convergence point of several rounds of agy's own adversarial review: # * It does NOT false-positive on reviewing the reviewer or on an OAuth-related PR. A code # review quotes this script's BARE regex ("accounts.google.com/o/oauth2", no scheme) or -# discusses auth in prose; neither contains the "https://…/o/oauth2/" URL form. (Earlier +# discusses auth in prose; neither contains the "https://…/o/oauth2" URL form. (Earlier # signature/prompt-phrase heuristics wrongly flagged such reviews.) # * It needs NO "is this a review?" exemption, so it CANNOT be disarmed by a section header # (or the phrase "blocking issues") appearing in the same output as a URL — a false @@ -38,24 +38,22 @@ have_text() { [ -s "$1" ] && grep -q '[^[:space:]]' "$1"; } # So this is both necessary (agy's login flow always prints the URL) and sufficient (nothing # else legitimately carries a live authorization URL), with no heuristic that flips between # false-positive and false-negative. -OAUTH_URL_RE='https?://accounts\.google\.com/o/oauth2/' - -# Layer-3 hard guard (UNCONDITIONAL): true if a text contains a live OAuth authorization URL. -# Used to refuse posting ANY assembled body that carries one — no exemptions, whatever upstream -# does to the body. This is the actual security guarantee: a public PR comment must never -# contain such a URL. -oauth_url_present() { [ -s "$1" ] && grep -qE "$OAUTH_URL_RE" "$1"; } - -# Layer-1 lapse detector: agy printed its login flow instead of a review, so its session has -# lapsed. Primary signal is the live URL (always present in the flow); the secondary matches -# agy's terminal failure line so a lapse is still recognized if the URL scrolled out of the -# captured tail. This only gates the retry / clean-exit UX — a Layer-1 false positive merely -# skips a review (recoverable), never a leak; the unconditional Layer 3 above is the guarantee. -is_auth_prompt() { - [ -s "$1" ] || return 1 - oauth_url_present "$1" && return 0 - grep -qiE 'authentication failed or timed out' "$1" -} +# +# The regex requires the scheme (`https?://`) and the host + `/o/oauth2` endpoint prefix, but +# deliberately NOT a trailing slash: agy round-4 flagged that a `.../o/oauth2/` requirement +# would miss a URL emitted as `.../o/oauth2?client_id=…` or bare `.../o/oauth2` (a leak). The +# scheme is what keeps a review's bare-pattern quote from matching, so dropping the trailing +# slash loses no safety. +OAUTH_URL_RE='https?://accounts\.google\.com/o/oauth2' + +# The single guard, used at BOTH the retry-loop capture (Layer 1) and the assembled body before +# posting (Layer 3): true iff a text contains a live OAuth authorization URL. There is no +# separate heuristic and no "is this a review?" exemption — earlier a secondary "authentication +# failed or timed out" substring fallback was tried, but agy round-4 flagged it (correctly) as +# broad enough to abort a genuine review that merely discusses auth-timeout handling. The live +# URL is always present in a real login flow, so it alone is both necessary and sufficient; a +# lapse is detected AND a leak is blocked by the same unconditional check. +oauth_url_present() { [ -s "$1" ] && grep -qiE "$OAUTH_URL_RE" "$1"; } # --- configuration (all env-overridable from the workflow) --------------------- AGY_BIN="${AGY_BIN:-agy}" @@ -450,13 +448,14 @@ for (( attempt=1; attempt<=AGY_RETRIES; attempt++ )); do # normalize CRs without sed -i (avoid in-place edit footguns) tr -d '\r' < "$out_file" > "$out_file.clean" && mv "$out_file.clean" "$out_file" - # If agy emitted its interactive OAuth login flow, its cached Google session has lapsed on this - # runner and there is NO review — just a live-shaped OAuth URL + "paste the authorization code" - # prompt. That must never reach a public PR comment (noise, phishing-shaped, and it advertises - # that the runner's auth dropped). Treat it as a hard, NON-retryable failure: re-auth is a human - # action on the runner host, so retrying only thrashes the backoff for a minute. Blank the - # capture so no later path (have_text / body assembly) can ever post it, flag it, and stop. - if is_auth_prompt "$out_file"; then + # If the capture carries a live OAuth authorization URL, agy's cached Google session has lapsed + # on this runner and there is NO review — just its interactive login flow (the OAuth URL + a + # "paste the authorization code" prompt). That must never reach a public PR comment (noise, + # phishing-shaped, and it advertises that the runner's auth dropped). Treat it as a hard, + # NON-retryable failure: re-auth is a human action on the runner host, so retrying only thrashes + # the backoff for a minute. Blank the capture so no later path (have_text / body assembly) can + # ever post it, flag it, and stop. + if oauth_url_present "$out_file"; then log "agy is NOT authenticated on this runner: it printed its interactive Google OAuth login flow instead of a review. Refusing to post it (it carries an OAuth URL). Re-authenticate agy on the runner host, then re-run with '/agy-review'." : > "$out_file" auth_failed=1 @@ -531,9 +530,9 @@ gh api "repos/${REPO}/issues/${PR}/comments" --paginate \ # Final hard guard — the last line of defense, and UNCONDITIONAL. Layer 1 (the retry loop) # already rejects a lapsed-session capture, but a public PR comment must NEVER carry a live # OAuth authorization URL, whatever any upstream change does to the body — and with no "looks -# like a review" exemption that a header alongside a URL could disarm. Uses oauth_url_present -# (not is_auth_prompt): a genuine review that merely discusses auth or quotes this script's bare -# regex has no live URL and posts normally; only an actual authorization URL blocks the post. +# like a review" exemption that a header alongside a URL could disarm. A genuine review that +# merely discusses auth or quotes this script's bare regex has no live URL and posts normally; +# only an actual authorization URL blocks the post. if oauth_url_present "$body_file"; then log "refusing to post: the assembled comment body contains a live OAuth authorization URL. Re-authenticate agy on the runner host." exit 1