Bound OAuth state lifetime and replay

[AnthonyRonning]

Summary

• expire pending OAuth state after 10 minutes and prune expired entries during normal store/consume operations
• cap each provider store at 4,096 entries and return HTTP 429 for new initiations when still full after pruning, preserving existing valid callbacks
• atomically consume a matching state so only one concurrent callback can proceed while mismatches preserve the legitimate state

Session binding is intentionally out of scope: browser clients retain their encrypted session through redirects, but the official Rust SDK only retains it in memory and does not guarantee that the callback reuses the initiating session.

Tests

• deterministic TTL expiration and pruning
• fixed-capacity rejection that preserves existing states
• mismatch preservation and one-time consumption
• two-consumer race with exactly one winner
• cargo fmt --all -- --check
• cargo clippy --locked --all-targets --all-features -- -D warnings
• cargo test --locked --all-features (220 passed, 16 ignored database tests)

---

Summary by CodeRabbit

• Security Enhancements

  • Improved OAuth state protection with expiration and capacity limits.
  • OAuth states are now consumed only once after a successful verification.
  • Added safeguards for concurrent callback attempts and invalid state matches.

• Bug Fixes

  • Expired OAuth states are automatically removed, reducing stale authentication data.

[coderabbitai]

Walkthrough

OAuth CSRF state handling now uses a private, bounded store with TTL expiration, capacity eviction, pruning, and atomic one-time consumption. All providers implement the new consumption contract, and the OAuth callback uses it for state verification.

Changes

OAuth state consumption

Layer / File(s)	Summary
Bounded OAuth state store src/oauth.rs	OAuthStateStore adds TTL expiration, capacity limits, oldest-entry eviction, expired-entry pruning, atomic consumption, and coverage for concurrency and lifecycle behavior.
Provider state integration src/oauth.rs	Github, Google, and Apple providers use private OAuthStateStore fields, and OAuthProvider replaces validate_state with consume_state.
Callback state consumption src/web/oauth_routes.rs	oauth_callback consumes matching OAuth state values and leaves mismatches stored.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

Poem

A bunny checks the state at night,
One little token, one true bite.
Old crumbs fade and baskets sway,
The first good match hops away.
“Safe callbacks!” the rabbit sings,
While TTL trims tangled things.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: OAuth state is now bounded in lifetime and protected against replay.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex-opensecret-oauth-state-lifecycle-opensecret

---

_{Comment @coderabbitai help to get the list of available commands.}

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[AnthonyRonning]

On CodeRabbit’s OAuth-state churn finding: this is a valid residual availability concern, but intentionally outside this focused lifecycle/memory-safety PR. The deployment throttles these public routes at Cloudflare; an in-process per-IP limiter requires a separate trusted-proxy/client-IP policy, while per-session state binding currently breaks the Rust SDK callback flow because it reconstructs the client. The cap changes the prior unbounded-memory failure mode into a bounded, temporary login failure (maximum TTL 10 minutes). I’m leaving that tradeoff explicit rather than broadening this patch; application-layer throttling/session binding should be handled separately.