Skip to content

fix: source client access tokens from the session - #115

Merged
nicknisi merged 2 commits into
mainfrom
nickcollisson/sec-1348-remove-workos-access-token-cookie-channel
Sep 15, 2026
Merged

nicknisi merged 2 commits into
mainfrom
nickcollisson/sec-1348-remove-workos-access-token-cookie-channel

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

The client TokenStore (src/client/tokenStore.ts) read a JavaScript-readable workos-access-token cookie and adopted it as the session bearer token. This change removes that client-readable cookie channel and sources the token from the server-side session.

Please review before merging.

The client TokenStore read a plain, JavaScript-readable workos-access-token cookie and adopted its value as the session bearer access token, with no binding to the authenticated session. No server component in this SDK (or in @workos/authkit-session) ever writes this cookie, so it was a dead input channel: an attacker able to write a cookie on the app's registrable domain could plant their own valid token and cause the victim's client-side API calls to authenticate as the attacker. A Path mismatch in the consume-and-delete logic also let a Path=/ plant persist across page loads.

Remove the cookie read/consume/delete paths entirely. Tokens are now sourced only from the session-bound server RPCs, which are the only legitimate source. Add a regression test asserting a planted cookie is ignored.
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author
Original prompt from Linear User

Please work on ticket "AuthKit TanStack client token store adopts attacker-plantable workos-access-token cookie as the session access token (persistent token fixation via cookie injection)" (SEC-1348)

@playbook:playbook-b588614117c7477a9b9729928385384f

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@linear-code

linear-code Bot commented Jul 22, 2026

Copy link
Copy Markdown

SEC-1348

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge.

Summary

Removes the client-readable access-token cookie channel.

  • Initializes the client token store without adopting a browser cookie.
  • Sources access tokens through the session-bound server action.
  • Updates token refresh tests and adds coverage proving planted cookies are ignored.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Client[Client TokenStore] --> Action[getAccessTokenAction]
    Action --> Session[Server-side session context]
    Session --> Token[Session access token]
    Token --> Client
    Cookie[Client-readable workos-access-token cookie] -. ignored .-> Client
Loading

Reviews (2) · Last reviewed commit: "fix: recover session-bound access token ..."

@devin-ai-integration
devin-ai-integration Bot deleted the nickcollisson/sec-1348-remove-workos-access-token-cookie-channel branch July 27, 2026 16:15
@devin-ai-integration devin-ai-integration Bot changed the title Remove client-readable access-token cookie channel Consolidate access-token storage to the server-side session Jul 27, 2026
@nicknisi nicknisi reopened this Sep 14, 2026
@nicknisi nicknisi changed the title Consolidate access-token storage to the server-side session fix: source client access tokens from the session Sep 14, 2026
@nicknisi

Copy link
Copy Markdown
Member

Recovered this branch onto current main. The original cookie-channel removal remains unchanged.

Verification: 251 tests passed, including coverage. Lint, typecheck, formatting, library build, example build, and client-bundle leak check passed. A fresh independent code review found no blocker. Not merged or released.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

❌ Cannot revive Devin session - the session is too old. Please start a new session instead.

View session

@nicknisi
nicknisi merged commit 2bb84c2 into main Sep 15, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant