Skip to content

fix(reviewer): never post agy's OAuth login prompt to a PR #332

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
doublegate merged 1 commit into from
Jul 30, 2026
Merged

fix(reviewer): never post agy's OAuth login prompt to a PR #332

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions scripts/agy-review.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,20 @@ 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). 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).
is_auth_prompt() {
[ -s "$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"
}

# --- configuration (all env-overridable from the workflow) ---------------------
AGY_BIN="${AGY_BIN:-agy}"
Expand Down Expand Up @@ -100,6 +114,9 @@ log "reviewing ${REPO}#${PR}"
# Remove every temp file on exit. Pre-declared so the trap is safe under `set -u` even if the
# script exits before a given file is created.
diff_file= diff_err= meta_file= prompt_file= out_file= raw= body_file= agy_diff_file= agy_work_dir=
# Set to 1 if agy printed its interactive OAuth login flow instead of a review (lapsed session);
# gates the no-post abort below. Pre-declared so `${auth_failed:-0}` is set-u-safe on every path.
auth_failed=0
# Set when the large-diff fallback below creates refs/agy/* so the trap can remove them.
agy_refs_created=
cleanup() {
Expand Down Expand Up @@ -409,6 +426,19 @@ 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
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
break
fi

# NOTE: there is deliberately no SQLite-conversation-store fallback here. agy's
# store is keyed by mtime, not session, and on a shared/multi-user runner the
# most-recent `.db` can belong to an UNRELATED concurrent local `agy` session --
Expand All @@ -424,6 +454,14 @@ for (( attempt=1; attempt<=AGY_RETRIES; attempt++ )); do
done
exec 9>&- 2>/dev/null || true # release the agy lock so the next queued job proceeds

# Lapsed-auth abort takes precedence over the generic empty-output path: it is a specific,
# actionable cause (re-auth agy on the runner), not a transient backend blip, and we already
# blanked $out_file above so nothing is postable. Exit non-zero WITHOUT posting anything.
if [ "${auth_failed:-0}" = "1" ]; then
log "aborting without posting: agy requires re-authentication on the runner host (no review was produced)."
exit 1
fi

if ! have_text "$out_file"; then
log "no review output after ${AGY_RETRIES} attempt(s). Check $LOG and confirm 'agy -p \"hi\"' works for this user."
# Surface agy's stderr into the job log. RUNNER_TEMP is wiped between jobs, so a bare
Expand Down Expand Up @@ -466,5 +504,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."
exit 1
fi

gh pr comment "$PR" --repo "$REPO" --body-file "$body_file"
log "posted review to ${REPO}#${PR}"