fix(auth): deliver the session cookie to the Owletto extension iframe (CHIPS)#1092
Merged
Conversation
… (CHIPS) owletto-web is embedded in the extension side panel as a CROSS-SITE iframe (top-level chrome-extension://). The deep-link session cookie was SameSite=Lax, which the browser withholds on cross-site iframe loads, so the embedded app rendered signed-out (sign-in page / Google 403 on the in-iframe login). - POST /api/exchange-token: new handler (token in the body) that mints the session cookie as SameSite=None; Secure; Partitioned (CHIPS). The extension's iframe bootstrap POSTs here from inside its OWN partition, so the cookie is keyed to the chrome-extension:// top-level — delivered on same-partition iframe loads, isolated from every other site's partition (no CSRF widening), and it survives third-party-cookie deprecation. - GET /api/exchange-token and /api/local-init keep the first-party SameSite=Lax cookie unchanged (CLI / menu-bar deep-link runs in a top-level tab). - GET /api/extension-bootstrap: serves the in-iframe bootstrap page that reads the PAT from the URL fragment, strips it from history, and POSTs it — so the PAT never lands in a request URL or history entry. - Test: POST -> None;Secure;Partitioned, GET -> Lax (never Partitioned/None), 400/401 rejects, and the bootstrap HTML. Adds a postForm test helper. Backward compatible: the new routes are additive and the existing GET/local-init cookie is unchanged. Pairs with the extension change in lobu-ai/owletto#233; the submodule pointer is bumped in a follow-up after that merges.
📝 WalkthroughWalkthroughImplements CHIPS-style partitioned session cookies and POST-based token exchange for extension iframes, adds a shared exchange handler, an extension bootstrap HTML flow, a form-post test helper, and tests validating cookie posture and error cases. ChangesPartitioned Session Cookie Exchange
Sequence DiagramsequenceDiagram
participant Client
participant ExtBootstrap
participant ExchangeToken
participant SessionStore
Note over Client,SessionStore: Extension iframe flow (POST)
Client->>ExtBootstrap: GET with deep-link token in fragment
ExtBootstrap->>Client: Serve HTML with fragment extraction and form script
Client->>Client: Extract token from fragment and strip it from URL
Client->>ExchangeToken: POST token in form body
ExchangeToken->>SessionStore: Validate token and mint partitioned cookie
ExchangeToken->>Client: 302 redirect with Set-Cookie SameSite=None Secure Partitioned
Note over Client,SessionStore: First-party flow (GET)
Client->>ExchangeToken: GET with token in query params
ExchangeToken->>SessionStore: Validate token and mint first-party cookie
ExchangeToken->>Client: 302 redirect with Set-Cookie SameSite=Lax
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Member
Author
|
bug_free 86, simplicity 82, slop 0, bugs 0, 0 blockers Typecheck/unit/integration logs all exit 0. Ran git diff --check and scanned the diff for dynamic imports/banned browser APIs; no findings. No live browser/server probe; CHIPS behavior is covered by header-level tests, not an actual browser run. Full verdict JSON{
"bug_free_confidence": 86,
"bugs": 0,
"slop": 0,
"simplicity": 82,
"blockers": [],
"change_type": "fix",
"behavior_change_risk": "medium",
"tests_adequate": true,
"suggested_fixes": [],
"notes": "Typecheck/unit/integration logs all exit 0. Ran git diff --check and scanned the diff for dynamic imports/banned browser APIs; no findings. No live browser/server probe; CHIPS behavior is covered by header-level tests, not an actual browser run.",
"categories": {
"src": 158,
"tests": 116,
"docs": 0,
"config": 0,
"deps": 0,
"migrations": 0,
"ci": 0,
"generated": 0
}
}Local review gate — branch protection can require the |
resolveDeepLinkToken only handled owl_pat_ PATs and Better Auth session tokens. The Owletto extension's CLOUD pairing (OAuth device-code) stores an oauth_tokens access token, which is neither — so POST /api/exchange-token 401'd and the cloud-paired side-panel iframe rendered signed-out (the original bug, for the OAuth path specifically; local-init/native use owl_pat_ and already worked). Resolve via OAuthProvider.verifyAccessToken, which already accepts both PATs and oauth_tokens access tokens, then fall back to the Better Auth session lookup. Test: a genuine OAuth access token (createTestAccessToken, asserted not owl_pat_) now exchanges to a 302 + partitioned cookie.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/server/src/auth/routes.ts`:
- Around line 366-369: The current flow returns authInfo.userId for any valid
OAuth token which lets arbitrary bearer tokens mint full 7-day web sessions;
modify the logic around OAuthProvider.verifyAccessToken to only allow session
creation for an approved exchange token by checking a unique identifier or scope
on authInfo (e.g. authInfo.clientId === ALLOWED_PAIRING_CLIENT_ID OR
authInfo.scopes includes 'session:exchange' or authInfo.singleUse === true)
before returning the userId; otherwise reject/throw and do not create a session.
Ensure the checks are applied where verifyAccessToken(...) is called and keep
the existing calls to createDbClientFromEnv and resolveBaseUrl untouched.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 76d31348-e196-4fa9-8ca0-4c9ca7d55ab0
📒 Files selected for processing (2)
packages/server/src/auth/__tests__/exchange-token-cookie.test.tspackages/server/src/auth/routes.ts
Comment on lines
+366
to
+369
| const sql = createDbClientFromEnv(c.env); | ||
| const baseUrl = resolveBaseUrl({ request: c.req.raw }); | ||
| const authInfo = await new OAuthProvider(sql, baseUrl).verifyAccessToken(token); | ||
| if (authInfo?.userId) return authInfo.userId; |
There was a problem hiding this comment.
Don't let arbitrary OAuth bearers mint full web sessions.
This path keeps only userId from verifyAccessToken() and then creates a 7-day Better Auth session, so a scoped OAuth access token can be upgraded into an unrestricted browser login. The new test coverage even proves a generic client token with only profile:read is accepted. Please restrict this exchange to the Owletto pairing client, a dedicated scope, or a single-use exchange credential instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/server/src/auth/routes.ts` around lines 366 - 369, The current flow
returns authInfo.userId for any valid OAuth token which lets arbitrary bearer
tokens mint full 7-day web sessions; modify the logic around
OAuthProvider.verifyAccessToken to only allow session creation for an approved
exchange token by checking a unique identifier or scope on authInfo (e.g.
authInfo.clientId === ALLOWED_PAIRING_CLIENT_ID OR authInfo.scopes includes
'session:exchange' or authInfo.singleUse === true) before returning the userId;
otherwise reject/throw and do not create a session. Ensure the checks are
applied where verifyAccessToken(...) is called and keep the existing calls to
createDbClientFromEnv and resolveBaseUrl untouched.
This was referenced May 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The Owletto Chrome side panel embeds owletto-web in an
<iframe>whose top-level ischrome-extension://…— a cross-site context. The deep-link session cookie was mintedSameSite=Lax, which the browser withholds on cross-site iframe loads, so the embedded app rendered signed-out (the sign-in page; the Google 403 when the user clicked the in-iframe login).Root cause (verified with browser experiments)
Cross-site-iframe cookie probes from the extension top-level on a real Chrome:
SameSite=Lax→ withheld (the live bug)SameSite=None; Secure(unpartitioned) → delivered, but widens CSRF + fragile to 3PC deprecationSameSite=None; Secure; Partitionedset inside the iframe's partition → delivered same-partition, absent from other partitionsFix (CHIPS)
POST /api/exchange-token(new): token in the request body; mints the cookieSameSite=None; Secure; Partitioned. The extension bootstrap POSTs here from inside the iframe, so the cookie is keyed to thechrome-extension://partition — isolated (no CSRF widening) and 3PC-proof.GET /api/exchange-tokenand/api/local-initkeep the first-partySameSite=Laxcookie unchanged (CLI / menu-bar deep-link in a top-level tab).GET /api/extension-bootstrap(new): the in-iframe page that reads the PAT from the URL fragment, strips it from history, and POSTs it — the PAT never lands in a request URL or history entry.Backward compatible: the new routes are additive and the existing first-party cookie is unchanged, so this is safe to merge/deploy before the extension change.
Tests
New
exchange-token-cookie.test.ts(passing locally against the integration DB):POST→None; Secure; Partitioned;GET→Lax(neverPartitioned/None); 400/401 on missing/invalid token;/extension-bootstrapserves the form-posting page. Adds apostFormtest helper.End-to-end (real stack)
Booted a real local gateway (embedded Postgres) with this change + the extension:
POST /api/exchange-tokenfires from the iframe → 302 → zero/sign-inbounces →get-session200 → the iframe renders the full signed-in app. (Fresh DB, so the session was minted by this flow, not a leftover cookie.)Related / follow-up
packages/owlettosubmodule pointer bump lands as a separate PR after feat(owletto): consolidate CLI profiles into lobu.toml #233 merges (per AGENTS.md).Summary by CodeRabbit
New Features
Tests