feat(reborn): handle product auth oauth callbacks

[hanakannzashi]

Summary

• Added a Reborn composition-owned RebornProductAuthServices::handle_oauth_callback boundary for hosted OAuth callback routes.
• Route authorized callbacks through the Reborn AuthProviderClient and AuthFlowManager, then emit typed AuthContinuationEvents through an injected dispatcher.
• Return stable sanitized callback responses/errors and document route responsibilities so callback code does not activate extensions, resume turns, replay prompts, or dispatch runtime work directly.
• Added caller-level contract coverage for success, stale callback, malformed callback, provider denial, token exchange failure, cross-scope denial, and secret redaction.

Change Type

• Bug fix
• New feature
• Refactor
• Documentation
• CI/Infrastructure
• Security
• Dependencies

Linked Issue

Related #3812
Related #3289

Stacked on #3878, which is stacked on #3865. This PR implements the composition handler/continuation slice; actual HTTP route mounting remains deferred until the Reborn host route surface lands.

Validation

• cargo fmt --all -- --check
• cargo clippy -p ironclaw_reborn_composition --all-targets -- -D warnings
• cargo build
• Relevant tests pass: cargo test -p ironclaw_reborn_composition --test auth_callbacks --locked, cargo test -p ironclaw_reborn_composition --locked, cargo test -p ironclaw_architecture reborn_product_auth_contract_stays_reborn_native --locked
• cargo test --features integration if database-backed or integration behavior changed
• Manual testing: Not applicable; composition/facade behavior covered by contract tests.
• If a coding agent was used and supports it, review-pr or pr-shepherd --fix was run before requesting review

Additional validation:

• cargo check -p ironclaw_reborn_composition --features libsql --locked
• cargo check -p ironclaw_reborn_composition --features postgres --locked
• git diff --check

Security Impact

Yes. This touches the Reborn OAuth callback/auth boundary. It keeps provider exchange, flow completion, account updates, and continuation scheduling inside Reborn product-auth composition, returns sanitized error codes, and adds regression coverage that raw OAuth code/verifier/token material is not exposed in callback output. It does not add a listener, route mount, new network egress path, or durable secret encryption changes.

Database Impact

None. No migrations or schema changes.

Blast Radius

Limited to ironclaw_reborn_composition product-auth facade behavior, docs, and callback contract tests. The implementation depends on the auth contracts from #3865 and the composition seam from #3878.

Rollback Plan

Revert this PR to remove the callback handler seam, continuation dispatcher hook, docs, and tests. The lower-level auth contracts and composition seam remain intact in the parent PRs.

Review Follow-Through

Reviewer judgment requested on whether this should close #3812 now or stay as a stacked handler slice until actual Reborn HTTP route mounting lands. The PR intentionally does not use V1 OAuth routes or route-local pending auth state.

---

Review track: C

[gemini-code-assist]

Code Review

This pull request implements OAuth callback handling within the ironclaw_reborn_composition crate, introducing the RebornAuthContinuationDispatcher trait and the handle_oauth_callback method to manage provider exchanges and flow completion. Feedback suggests that failures during the secondary continuation dispatch should be logged as warnings instead of returning an error, ensuring that a successfully committed primary auth flow is not misrepresented to the user as a failure.

[serrrfirat]

Multi-agent review complete for head 7ccc52bbbc04d53d35836bf79151f27bdf2855ac.

Summary: 5 reviewer lenses completed. I found 2 blocking High issues and 2 non-blocking follow-ups. The blocking issues are both in the new OAuth callback boundary: provider exchange happens before flow/state validation, and continuation dispatch failure can be hidden behind a successful callback response.

[henrypark133]

Single-agent review.

User-overridden verdict policy: any Med/Low present → COMMENT (no approval for auth-sensitive code).

Verdict: COMMENT

0 Critical · 0 High · 3 Medium · 2 Low

The overall design is solid: two-phase claim-then-complete prevents double-exchange, scope/state/provider/PKCE hash checks are all correctly ordered before the terminal-status idempotency branch, Debug and Serialize correctly redact raw secrets, and the test suite covers the primary paths well. The findings below are about gaps and edge cases rather than fundamental problems.

---

Medium 1 — Flow leaks into non-terminal CallbackReceived on provider exchange failure (Reliability/Security, confidence 85%)

File: crates/ironclaw_reborn_composition/src/auth.rs — handle_oauth_callback, the Authorized arm

claim_oauth_callback atomically transitions the flow from AwaitingUser → CallbackReceived before the provider exchange. If exchange_callback then fails (network error, provider 5xx, token mismatch), handle_oauth_callback returns early via ? without ever calling complete_oauth_callback. The flow is now permanently stuck in CallbackReceived:

• CallbackReceived is not in is_terminal_status, so it passes prepare_callback_flow.
• But claim_oauth_callback checks record.status != AuthFlowStatus::AwaitingUser and returns FlowAlreadyTerminal for CallbackReceived.
• So a retry attempt hits FlowAlreadyTerminal — with no way to complete the flow — until the flow expires.

OAuth authorization codes are single-use, so re-exchange with the same code would fail at the provider anyway. However: (a) the flow is left in a misleadingly non-terminal state until expiry (no Failed status is set), and (b) the retryable: false on TokenExchangeFailed is semantically correct for the exchange itself but doesn't convey that the flow cannot recover without a fresh OAuth start. The contract doc notes "Terminal flows cannot be completed or canceled again" — exchange failure should produce a terminal Failed flow, not a CallbackReceived limbo.

Suggested fix: After exchange_callback fails, call a fail_flow method or complete_oauth_callback with a failure outcome so the flow reaches a terminal Failed state. If deferred to a follow-up, add a comment with a TODO linking the issue.

---

Medium 2 — Missing test: wrong PKCE hash rejected before provider exchange (Tests, confidence 80%)

File: crates/ironclaw_reborn_composition/tests/auth_callbacks.rs

oauth_callback_handler_rejects_wrong_state_without_provider_exchange_or_dispatch covers a wrong opaque_state_hash and correctly asserts provider_client.calls() == 0. There is no equivalent test for a mismatched pkce_verifier_hash in the OAuthCallbackClaimRequest. Per the project testing rules ("Test Through the Caller, Not Just the Helper"), the caller-level composition test should verify that a wrong PKCE hash is also rejected before the provider exchange is attempted. A future refactor could accidentally reorder the PKCE check after the provider call without this regression guard.

---

Medium 3 — Non-timing-safe state/PKCE hash comparison (Security, confidence 75%)

File: crates/ironclaw_auth/src/fakes.rs (lines 120, 126) and prepare_callback_flow (line 553)

The OpaqueStateHash and PkceVerifierHash comparisons use derived PartialEq, which short-circuits on the first mismatched byte. While these are hashes of the original secrets (not the secrets themselves), timing-oracle attacks on hash comparison can leak prefix information about the stored hash value. In a web-exposed callback endpoint where the attacker supplies the state parameter (which is hashed and compared), measurable response-time differences could enumerate valid hash prefixes. A constant-time comparison (subtle::ConstantTimeEq or a fixed-time bytes-equal helper) is appropriate here. This should be addressed before the HTTP route mounting that this PR explicitly defers.

---

Low 1 — Completing status is orphaned (Design, confidence 70%)

File: crates/ironclaw_auth/src/flow.rs line 31

AuthFlowStatus::Completing exists in the enum but is never set by InMemoryAuthProductServices and is absent from is_terminal_status. It has no role in the claim-then-complete two-phase protocol implemented here. If it is reserved for a future async-completion pattern, a comment should say so. If not, it should be removed to avoid confusing production implementors of AuthFlowManager.

---

Low 2 — No Deserialize on wire response types (Design, confidence 65%)

File: crates/ironclaw_reborn_composition/src/auth.rs

RebornOAuthCallbackResponse and RebornOAuthCallbackError derive Serialize but not Deserialize. The HTTP route test harness (when the route is mounted in a follow-up) will need to parse the response body. Adding Deserialize now avoids churn later. This is minor since the HTTP route is explicitly out of scope.

---

Checklist confirmations

• State parameter single-use: claim_oauth_callback in the fake transitions AwaitingUser → CallbackReceived atomically under mutex, preventing double-claim. ✓
• PKCE checked before exchange: OAuthCallbackClaimRequest carries pkce_verifier_hash and the fake validates it at claim time, before exchange_callback is invoked. ✓
• Cross-scope rejection: scope, state hash, provider, and PKCE are all validated before any terminal idempotency bypass. ✓
• Secret redaction: OAuthProviderCallbackRequest has a hand-rolled Debug impl redacting raw code/verifier; RebornOAuthCallbackRequest is not Serialize; tests at lines 205-207 and 233-237 confirm neither Debug nor JSON output leaks raw secrets. ✓
• Opaque error codes: RebornOAuthCallbackError surfaces only AuthErrorCode variants, never provider response bodies. ✓
• Composition boundary respected: No src/bridge/, V1 pending maps, or V1 extension manager is touched. Only ironclaw_auth trait-shaped ports are used. ✓
• No .unwrap()/.expect() in production code: None found. ✓
• Logging: Only tracing::debug! at the dispatch-failure site, with flow_id and error_code only — no secret values. ✓
• AuthFlowStatus wire enum: Uses #[serde(rename_all = "snake_case")]. ✓

[henrypark133]

Addressed #3879 (review) in e92da28.

What changed:

• Provider-exchange failures now terminally mark the claimed flow as Failed with the sanitized auth error code, avoiding CallbackReceived limbo.
• Added caller-level PKCE mismatch coverage proving no provider exchange or continuation dispatch happens on a bad verifier hash.
• Digest hash comparisons for state/PKCE now use constant-time equality via subtle.
• Documented AuthFlowStatus::Completing as reserved for future async completion workers.
• RebornOAuthCallbackResponse and RebornOAuthCallbackError now derive Deserialize.

Verification:

• cargo test -p ironclaw_reborn_composition --test auth_callbacks
• cargo test -p ironclaw_auth --test auth_product_contract
• cargo clippy -p ironclaw_auth -p ironclaw_reborn_composition --tests -- -D warnings
• git diff --check