-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix: Safari magic link redirect on MacOS #3799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ export const DEFAULT_SESSION_DURATION_SECONDS = 31_556_952; | |
| export const sessionStorage = createCookieSessionStorage({ | ||
| cookie: { | ||
| name: "__session", | ||
| sameSite: "lax", | ||
| sameSite: env.NODE_ENV === "production" ? "none" : "lax", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚩 All other cookies in the webapp remain sameSite: "lax" — inconsistent security posture Every other cookie-based session storage in the webapp hardcodes Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| path: "/", | ||
| httpOnly: true, | ||
| secrets: [env.SESSION_SECRET], | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Changing sameSite to "none" in production removes CSRF protection from the session cookie
Setting
sameSite: "none"means the__sessioncookie will be sent on all cross-site requests in production. The application has no alternative CSRF protection mechanism — no CSRF tokens, no double-submit cookies — as confirmed by searching the codebase for "csrf"/"CSRF". The only references to CSRF in the entire webapp are comments noting thatsameSite: "lax"itself is the CSRF defense (e.g.apps/webapp/app/services/impersonation.server.ts:12). Every other cookie in the app (impersonation,onboardingSession,referralSource,redirectTo,gitHubSession,lastAuthMethod,uiPreferences, OAuth state cookies) all remainsameSite: "lax". An attacker could host a page that issuesPOSTrequests (or other state-changing methods) to the Trigger.dev webapp, and the victim's session cookie would be included automatically — allowing task triggers, settings changes, or other authenticated actions without consent.Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.