Skip to content

feat(auth): recover revoked OAuth tokens safely - #974

Merged
steipete merged 4 commits into
mainfrom
triage/t5-gogcli-pr973
Aug 10, 2026
Merged

feat(auth): recover revoked OAuth tokens safely#974
steipete merged 4 commits into
mainfrom
triage/t5-gogcli-pr973

Conversation

@steipete

Copy link
Copy Markdown
Collaborator

Summary

Maintainer landing branch for #973. This preserves @inamiy's authored feature commit, then hardens the released auth path before landing:

  • require explicit confirmation before opening a browser;
  • recognize typed OAuth invalid_grant failures only;
  • serialize recovery across all service clients in one command;
  • make waiting clients adopt the newly stored token instead of opening duplicate flows;
  • persist the replacement token before swapping the in-memory source, and retain it when later refresh responses omit RefreshToken;
  • reject missing or mismatched authorized identities;
  • keep --no-input, non-TTY, ADC, service-account, direct-token, non-replayable request, and single-retry boundaries intact.

Also includes the required dependency pass: Kong 1.16.1, Cloudflare Workers types 5.20260809.1, and pnpm 11.21.0.

Proof

  • make ci
  • go test -race ./internal/googleapi/... -run Reauth -count=1
  • worker lint, typecheck, 15 tests, and wrangler deploy --dry-run
  • structured autoreview: clean, no accepted/actionable findings
  • source-blind live Google token-endpoint check with an isolated fake refresh token:
    • --no-input: exit 1, no prompt, invalid_grant, actionable gog auth add hint
    • pseudo-TTY + n: explicit account-scoped confirmation, exit 1, no OAuth URL or credential output

The contributor's live Google proof covers successful browser consent, token persistence, request replay, and non-interactive suppression: #973 (comment)

No release is included.

inamiy and others added 3 commits August 9, 2026 18:33
…ant)

When the stored OAuth refresh token is expired or revoked (invalid_grant),
gog currently fails with a hard error and requires the user to manually
re-run 'gog auth add'. This is in contrast to other CLI tools (e.g. Rust's
yup-oauth2 InstalledFlowAuthenticator) which automatically fall back to a
browser-based re-authorization flow when the refresh token is invalid.

This PR adds auto-reauth support:

- In interactive sessions (TTY stdin, --no-input not set), gog detects
  invalid_grant during token refresh, launches a browser-based OAuth flow
  (with --force-consent to ensure a new refresh token), persists it to the
  keyring, resets the in-memory token source, and retries the original
  API request.
- In non-interactive sessions (--no-input or non-TTY stdin), gog surfaces
  a clear error message with the manual 'gog auth add' command instead.
- Excluded for ADC, service accounts, and direct access tokens.
- The reauth preserves the stored token's full scope/service set,
  preventing silent grant narrowing.
- The authorized email is verified to match the expected account before
  persisting.
- The in-memory token source (resettableOAuthTokenSource) is rebuilt with
  the new refresh token so the retried request doesn't reuse the revoked
  token.

Design inspired by yup-oauth2's InstalledFlowAuthenticator.find_token_info()
fallback pattern: https://github.com/dermesser/yup-oauth2/blob/master/src/authenticator.rs

Co-authored-by: Yasuhiro Inami <inamiy@gmail.com>
@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 10, 2026
@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 9, 2026, 10:08 PM ET / August 10, 2026, 02:08 UTC.

ClawSweeper review

What this changes

Adds an explicitly confirmed browser re-authorization fallback for revoked stored OAuth refresh tokens, plus small dependency and CI pin updates.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

Keep open. This is a clean, maintainer-authored landing branch for the revoked-token recovery feature; current main does not implement that recovery path, and the patch preserves explicit-consent and non-interactive safeguards.

Priority: P2
Reviewed head: 3a23e66aae2cccfdf122cfdbafcbb50f6aeb78a4

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The patch has focused safety boundaries and extensive regression coverage, with no discrete correctness finding from the review.
Proof confidence 🌊 off-meta tidepool Not applicable: This collaborator-authored landing PR is not subject to the external-contributor proof gate; its body nevertheless documents redacted live token-endpoint checks and links prior live consent evidence.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This collaborator-authored landing PR is not subject to the external-contributor proof gate; its body nevertheless documents redacted live token-endpoint checks and links prior live consent evidence.
Evidence reviewed 4 items Current main lacks this recovery path: Current main creates the retry transport with ordinary refresh support and returns it without a re-authorization hook; the feature symbols are absent from the central auth path.
Recovery is serialized and adopts a stored replacement token: The branch serializes reauthorization across service clients, checks for a newly stored token before opening another browser flow, persists the replacement before swapping in-memory state, and rejects a mismatched identity.
Focused coverage covers retry boundaries: New tests cover typed invalid_grant detection, one retry, non-replayable and no-input suppression, token persistence, scope preservation, account validation, and concurrent recovery coalescing.
Findings None None.
Security None None.

How this fits together

gog uses stored OAuth credentials to obtain access tokens before Google API requests reach each service client. On a typed revoked-token failure, this path can now ask an interactive user to re-authorize, store the replacement token, and retry the original request.

flowchart LR
A[CLI command] --> B[Stored OAuth token]
B --> C[Google token refresh]
C --> D{Revoked token?}
D -->|no| E[Google API request]
D -->|interactive| F[Confirm re-authorization]
F --> G[Persist replacement token]
G --> E
D -->|non-interactive| H[Manual recovery guidance]
Loading

Before merge

  • Resolve merge risk (P1) - Merging changes revoked-token recovery from a terminal error into an explicitly confirmed browser OAuth flow that writes a replacement credential; compatibility depends on preserving the scoped, identity-validated path for existing accounts.
  • Complete next step (P2) - This clean maintainer landing branch needs normal review rather than an automated repair worker.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Implementation and test delta production +540/-15, tests +1,072/-0 The substantial regression coverage is proportionate to a cross-service OAuth recovery change; the production growth is explained by the new recovery flow and pinned dependency updates.

Root-cause cluster

Relationship: canonical
Canonical: #974
Summary: This is the clean maintainer landing branch for the same revoked-token recovery feature, with additional safety hardening.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Accept the consented recovery behavior (recommended)
    Land the new browser fallback only with its existing explicit confirmation, non-interactive suppression, account check, and single-retry boundaries intact.

Technical review

Best possible solution:

Retain the typed-error-only, explicitly confirmed recovery design with stored-token scope preservation and identity validation, then land it through the normal maintainer review path.

Do we have a high-confidence way to reproduce the issue?

Not applicable as an issue report. The PR supplies a concrete disposable-token validation path and source-backed tests for the recovery and suppression boundaries.

Is this the best way to solve the issue?

Yes. It extends the existing OAuth authorization flow only after a typed revoked-token error and explicit confirmation, while keeping ADC, service-account, direct-token, non-interactive, and non-replayable paths out of the fallback.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e4e71c68224a.

Labels

Label changes:

  • add P2: This is a bounded OAuth recovery improvement for accounts with revoked refresh tokens, without evidence of an emergency outage.
  • add merge-risk: 🚨 auth-provider: The patch changes OAuth refresh failure handling, browser authorization, credential persistence, and request replay for existing accounts.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This collaborator-authored landing PR is not subject to the external-contributor proof gate; its body nevertheless documents redacted live token-endpoint checks and links prior live consent evidence.

Label justifications:

  • P2: This is a bounded OAuth recovery improvement for accounts with revoked refresh tokens, without evidence of an emergency outage.
  • merge-risk: 🚨 auth-provider: The patch changes OAuth refresh failure handling, browser authorization, credential persistence, and request replay for existing accounts.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This collaborator-authored landing PR is not subject to the external-contributor proof gate; its body nevertheless documents redacted live token-endpoint checks and links prior live consent evidence.

Evidence

What I checked:

  • Current main lacks this recovery path: Current main creates the retry transport with ordinary refresh support and returns it without a re-authorization hook; the feature symbols are absent from the central auth path. (internal/googleapi/client.go:160, e4e71c68224a)
  • Recovery is serialized and adopts a stored replacement token: The branch serializes reauthorization across service clients, checks for a newly stored token before opening another browser flow, persists the replacement before swapping in-memory state, and rejects a mismatched identity. (internal/googleapi/client_auth.go:179, 3a23e66aae2c)
  • Focused coverage covers retry boundaries: New tests cover typed invalid_grant detection, one retry, non-replayable and no-input suppression, token persistence, scope preservation, account validation, and concurrent recovery coalescing. (internal/googleapi/auto_reauth_test.go:70, 3a23e66aae2c)
  • Feature provenance: The feature was introduced by inamiy and hardened in the landing branch by steipete before the dependency and CI alignment commits. (internal/googleapi/client.go:320, e45ddcbf73dc)

Likely related people:

  • steipete: Auth hardening and the maintainer landing branch are attributed to this contributor. (role: recent area contributor; confidence: high; commits: e45ddcbf73dc, 3a23e66aae2c; files: internal/googleapi/client.go, internal/googleapi/client_auth.go, internal/cmd/root.go)
  • inamiy: The preserved feature commit introduced the revoked-token reauthorization behavior and its initial test surface. (role: feature introducer; confidence: high; commits: 9679c0612dee; files: internal/googleapi/transport.go, internal/googleauth/reauth.go)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit 229da12 into main Aug 10, 2026
14 checks passed
@steipete
steipete deleted the triage/t5-gogcli-pr973 branch August 10, 2026 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants