feat: add PKCE (S256) to installed-app auth flow

[RedSlowpoke]

Why

RFC 7636 / RFC 8252 recommend PKCE for native/installed apps. Without it, a local malicious process that wins the race to bind the loopback port before gog can receive the authorization code and exchange it for a token — PKCE binds the exchange to the original authorization request, making the stolen code useless.

Google's OAuth 2.0 endpoint supports S256 for Desktop app clients.

Current state

The current OAuth flow in internal/googleauth/oauth_flow.go uses cfg.AuthCodeURL(state, ...) and cfg.Exchange(ctx, code) without PKCE. The project already depends on golang.org/x/oauth2 which ships PKCE support since v0.17.0 (pkce.go: GenerateVerifier, S256ChallengeOption, VerifierOption).

What's missing

// authorizeServer — before building authURL
verifier := oauth2.GenerateVerifier()

// AuthCodeURL call
authURL := cfg.AuthCodeURL(state,
    append(authURLParams(opts.ForceConsent, !opts.DisableIncludeGrantedScopes),
        oauth2.S256ChallengeOption(verifier))...)

// Exchange call
tok, exchangeErr := cfg.Exchange(ctx, code, oauth2.VerifierOption(verifier))

Same change applies to the manual redirect flow in oauth_flow_manual_redirect.go — the verifier needs to survive between step 1 (generate auth URL) and step 2 (exchange code).

Scope

~10–15 lines in oauth_flow.go, similar in oauth_flow_manual_redirect.go. No new dependencies.

[clawsweeper]

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve. Reviewed June 4, 2026, 9:44 AM ET / 13:44 UTC.

Summary
Keep this open: current main still builds the installed-app OAuth authorization URL and exchanges the code without PKCE, and the request is a narrow auth security hardening item that fits the project vision.

Reproducibility: yes. source inspection is enough to reproduce the missing behavior: current main's OAuth auth URL and exchange call sites do not include PKCE parameters, and a runtime test could confirm missing code_challenge/code_verifier fields.

Next step
Not queued for automated repair because this touches the OAuth security boundary and should get maintainer review plus live Google OAuth validation.

Security
Needs attention: This issue is security-sensitive because current installed-app OAuth exchanges are not bound with PKCE.

Review details

Best possible solution:

Add S256 PKCE to every installed-app OAuth code flow, persist only the short-lived verifier needed for remote step two, and cover auth URL plus token-exchange parameters in focused tests.

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

Yes, source inspection is enough to reproduce the missing behavior: current main's OAuth auth URL and exchange call sites do not include PKCE parameters, and a runtime test could confirm missing code_challenge/code_verifier fields.

Is this the best way to solve the issue?

Yes, the suggested oauth2.GenerateVerifier with S256ChallengeOption and VerifierOption is the narrowest maintainable direction, with the extra requirement that remote/manual state stores the verifier safely between steps.

AGENTS.md: found and applied where relevant.

Remaining risk / open question:

• The remote/manual flow needs a careful verifier lifecycle: the verifier must survive step one to step two, remain short-lived, and be cleared with the existing manual state.
• A complete fix may need to cover the account manager OAuth flow as well as the auth-add flows, otherwise gog would keep one installed-app OAuth path without PKCE.
• Live Google OAuth proof is still needed because unit tests can verify request parameters, but they cannot fully prove Google's Desktop client behavior.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 3d5c9cef65fa.

Label changes

Label changes:

• add P2: This is a normal-priority auth security improvement with a clear, bounded implementation path and no evidence of an active emergency.
• add impact:security: The request directly concerns OAuth code interception resistance and token exchange binding for installed-app auth.
• add issue-rating: 🦞 diamond lobster: Current issue advisory state selects this label.
• add clawsweeper:source-repro: Current issue advisory state selects this label.
• add clawsweeper:no-new-fix-pr: Current issue advisory state selects this label.
• add clawsweeper:needs-security-review: Current issue advisory state selects this label.

Label justifications:

• P2: This is a normal-priority auth security improvement with a clear, bounded implementation path and no evidence of an active emergency.
• impact:security: The request directly concerns OAuth code interception resistance and token exchange binding for installed-app auth.

Evidence reviewed

Security concerns:

• [medium] Installed-app OAuth flow lacks PKCE binding — internal/googleauth/oauth_flow.go:225
  Current main sends authorization-code requests and token exchanges without code_challenge/code_verifier, leaving the flow without the PKCE protection requested for local code interception resistance.
  Confidence: 0.9

What I checked:

• Repository policy read: Read the full target AGENTS.md and applied its auth/testing guidance while reviewing this issue. (AGENTS.md:1, 3d5c9cef65fa)
• Local browser OAuth lacks PKCE: authorizeServer builds AuthCodeURL with only authURLParams and exchanges with cfg.Exchange(ctx, code), with no S256ChallengeOption or VerifierOption. (internal/googleauth/oauth_flow.go:225, 3d5c9cef65fa)
• Manual/remote OAuth lacks verifier persistence: ManualAuthURL and manual exchange paths use the same non-PKCE AuthCodeURL/Exchange calls, while manualState stores state, client, scopes, force_consent, redirect_uri, and created_at only. (internal/googleauth/oauth_flow_manual.go:101, 3d5c9cef65fa)
• Temporary manual state has no verifier field: The manual auth state schema currently has no code verifier field, so a remote step-one verifier could not survive to step two on current main. (internal/googleauth/manual_state.go:27, 3d5c9cef65fa)
• Account manager OAuth also uses plain code exchange: The account manager auth start, auth upgrade, and callback paths also call AuthCodeURL/Exchange without PKCE options, which is useful scope context for a complete auth hardening pass. (internal/googleauth/accounts_server.go:309, 3d5c9cef65fa)
• No existing PKCE implementation found: A repository-wide search found no PKCE, S256, code_challenge, or code_verifier implementation on current main. (3d5c9cef65fa)

Likely related people:

• Peter Steinberger: Current blame for the OAuth flow files is on the v0.21.0 release commit, and nearby history shows Peter split the manual OAuth flow, protected account manager redirects, and made recent auth hardening changes. (role: recent auth area contributor; confidence: high; commits: 2c84d8981dd5, 366f7acbbbb2, 51dfdc2466c9; files: internal/googleauth/oauth_flow.go, internal/googleauth/oauth_flow_manual.go, internal/googleauth/accounts_server.go)
• salmonumbrella: Git history shows work on remote manual auth state enforcement and redirect URI support, both central to preserving a PKCE verifier across remote step one and step two. (role: remote/manual auth flow contributor; confidence: medium; commits: 2df8ece2f629, 5ea6d607fbcf; files: internal/googleauth/manual_state.go, internal/googleauth/oauth_flow.go, internal/googleauth/oauth_flow_manual.go)

How this review workflow works

• 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.