fix(auth): add PKCE to installed-app OAuth

[TurboTheTurtle]

Summary

• Add S256 PKCE to local browser installed-app OAuth authorization and token exchange.
• Persist the short-lived PKCE verifier for manual two-step auth and clear it with manual state.
• Add PKCE verifier handling to account-manager auth start, upgrade, and callback flows.
• Update focused auth tests.

Fixes #693.

Compatibility note

Manual authorization-code exchange now requires a current gog-generated state/verifier pair. Stale pre-PKCE manual state and bare raw-code exchanges without a matching verifier fail instead of falling back to a non-PKCE exchange; that is intentional because PKCE binds the code exchange to the original authorization request.

Validation

• make ci

Proof

Local tests verify auth URLs include code_challenge/code_challenge_method=S256, do not expose code_verifier, and token exchange sends the verifier. I did not run a live Google OAuth browser flow in this environment.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed June 10, 2026, 3:18 AM ET / 07:18 UTC.

Summary
The PR adds S256 PKCE verifier generation, auth URL challenge parameters, verifier-backed token exchange, manual verifier persistence/clearing, account-manager callback handling, and focused auth tests.

Reproducibility: yes. from source inspection: current main calls AuthCodeURL and Exchange without PKCE options in the installed-app OAuth paths. I did not establish a live failing Google-provider reproduction in this review.

Review metrics: 2 noteworthy metrics.

• Changed auth surface: 3 OAuth entry points changed. Browser login, manual/remote login, and account-manager auth all need provider-level confidence before merge.
• Diff size: 8 files, +389/-75. The patch is focused on auth but large enough that compatibility and proof should be checked explicitly.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P1] Add redacted terminal output, logs, or a short recording from a live Google installed-app OAuth flow; redact private details before posting.
• [P1] Get maintainer acceptance for the manual-code fail-closed behavior or revise/document the compatibility path.

Proof guidance:

• [P1] Needs real behavior proof before merge: The PR has focused tests and sanitized local loopback output, but no redacted live Google OAuth provider run showing the after-fix auth URL, hidden verifier, callback, and token exchange. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

• [P1] Manual two-step authorization and raw --auth-code exchange now fail unless the code was created with a current gog manual state and stored verifier; that is security-motivated but can break existing scripts or stale manual workflows on upgrade.
• [P1] The proof is still tests plus sanitized local loopback output, not a live Google OAuth authorization and token exchange against the real provider.

Maintainer options:

1. Accept and document fail-closed manual auth
   Maintainers can intentionally accept the stricter PKCE-bound manual-code behavior, but the PR should document the upgrade impact and include live Google proof before merge.
2. Preserve or migrate the manual-code path
   Revise the branch so existing supported manual workflows either keep working by default or fail with a documented migration path and focused upgrade coverage.
3. Pause until auth policy is clear
   If raw-code exchange without a gog-generated verifier should no longer be supported, pause the PR until that policy decision is explicit.

Next step before merge

• [P1] Human review is needed because automation cannot provide contributor-environment live Google proof or decide the manual-code compatibility policy.

Security
Cleared: No new dependency, supply-chain, or credential-exposure concern was found in the diff; the remaining security question is the intended PKCE compatibility policy.

Review findings

• [P1] Gate the manual-code compatibility break — internal/googleauth/oauth_flow_manual.go:109-110

Review details

Best possible solution:

Land PKCE for all installed-app OAuth entry points after maintainers either explicitly accept and document the manual fail-closed upgrade behavior or revise it to preserve a supported compatibility path, with redacted live Google OAuth proof attached.

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

Yes from source inspection: current main calls AuthCodeURL and Exchange without PKCE options in the installed-app OAuth paths. I did not establish a live failing Google-provider reproduction in this review.

Is this the best way to solve the issue?

Unclear: PKCE is the right security direction, but this PR's unconditional manual-code fail-closed behavior needs maintainer policy acceptance or a narrower compatibility plan before it is the best merge path.

Full review comments:

• [P1] Gate the manual-code compatibility break — internal/googleauth/oauth_flow_manual.go:109-110
  At this new failure path, any manual code exchange without a current stored PKCE verifier now returns errManualStateMissing, which also covers the existing hidden --auth-code flow and scripts that exchange a code with --redirect-uri without first running gog's current step 1. That may be the right security policy, but it changes upgrade behavior and should be explicitly accepted/documented or revised before merge.
  Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

• P2: This is a normal-priority auth security hardening PR with limited scope but real merge policy questions.
• merge-risk: 🚨 compatibility: The PR intentionally stops stale pre-PKCE manual state and bare raw-code exchanges that current users may rely on.
• merge-risk: 🚨 auth-provider: The change alters Google OAuth authorization and token exchange parameters across installed-app auth flows.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR has focused tests and sanitized local loopback output, but no redacted live Google OAuth provider run showing the after-fix auth URL, hidden verifier, callback, and token exchange. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

What I checked:

• Repository policy read: AGENTS.md was read fully; it instructs PR review via gh pr view/diff without switching branches and highlights OAuth credential safety. (AGENTS.md:37, b34e3033913a)
• Vision fit for auth hardening and proof bar: VISION.md lists security and reliability improvements for auth as fitting work, while behavior changes that could break scripts and live-provider behavior require discussion/proof. (VISION.md:16, b34e3033913a)
• Current main browser flow lacks PKCE: Current main builds the browser auth URL with authURLParams and exchanges the code without oauth2.S256ChallengeOption or oauth2.VerifierOption. (internal/googleauth/oauth_flow.go:225, b34e3033913a)
• Current main manual/account flows lack PKCE: Manual and account-manager flows exchange OAuth codes without a verifier, so the central requested PKCE work is not already implemented on main. (internal/googleauth/oauth_flow_manual.go:101, b34e3033913a)
• Existing manual-code surface: The CLI still has a hidden --auth-code path that skips state checks, which is the compatibility surface affected by the PR's verifier requirement. (internal/cmd/auth_add.go:28, b34e3033913a)
• PR adds PKCE to exchanges: The PR diff adds GenerateVerifier, S256 challenge auth URL parameters, and VerifierOption on token exchange across browser, manual, and account-manager flows. (internal/googleauth/oauth_flow.go:223, e4aef4d84f5a)

Likely related people:

• Peter Steinberger: Recent blame and log history for the current OAuth flow, account manager auth, and release state point to repeated work in the affected files. (role: recent area contributor; confidence: high; commits: 71a8504d0add, 1531013e50b6, 366f7acbbbb2; files: internal/googleauth/oauth_flow.go, internal/googleauth/oauth_flow_manual.go, internal/googleauth/manual_state.go)
• salmonumbrella: History shows prior work introducing and enforcing remote manual auth state and related auth behavior that this PR now changes for PKCE. (role: manual auth state contributor; confidence: medium; commits: 2df8ece2f629, 5ea6d607fbcf, 50dac265e173; files: internal/googleauth/manual_state.go, internal/googleauth/oauth_flow_manual.go, internal/cmd/auth_add.go)
• spookyuser: Prior manual OAuth redirect work is adjacent to the remote/manual flow that now has to persist and reuse a PKCE verifier. (role: adjacent manual redirect contributor; confidence: low; commits: 0e1f77dac0fa; files: internal/googleauth/oauth_flow_manual_redirect.go, internal/googleauth/oauth_flow_manual.go)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

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.

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.

[TurboTheTurtle]

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/27259683700
• Updated: 2026-06-10T07:19:05.779Z

[TurboTheTurtle]

Sanitized proof for this PR:

• PR head tested: e4aef4d
• GitHub checks are green: test, image, worker, windows, and darwin-cgo-build
• Focused local PKCE tests passed:

go test -v ./internal/googleauth -run 'TestAuthorize_ServerFlow_Success|TestManualAuthURL_UsesPKCEAndPersistsVerifier|TestAuthorize_Manual_AuthURL_UsesStoredPKCEVerifier' -count=1

• Sanitized installed-app/browser-loopback evidence from the local test flow:

Opening browser for authorization...
Auth URL included: code_challenge=<redacted>&code_challenge_method=S256
Auth URL did not include code_verifier
Authorization received. Finishing...
PASS

This verifies the installed-app OAuth URL includes an S256 PKCE challenge, keeps the verifier out of the browser URL, and completes the callback/exchange path with the verifier.

[TurboTheTurtle]

@clawsweeper re-review

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/27259345841
• Updated: 2026-06-10T07:12:39.888Z