🍪 feat: Add Session Cookie Secure Override#13189
Conversation
danny-avila
force-pushed
the
danny-avila/fix-session-cookie-secure-override
branch
from
8c06c54 to
d3bc904
Compare
There was a problem hiding this comment.
Pull request overview
Adds an explicit SESSION_COOKIE_SECURE environment override to shouldUseSecureCookie() so operators can force secure/non-secure cookie behavior regardless of the existing NODE_ENV + DOMAIN_SERVER heuristic (addressing HTTP-only deployments where browsers drop Secure cookies).
Changes:
- Added
SESSION_COOKIE_SECURE=true|falseoverride handling inshouldUseSecureCookie()before the current heuristic. - Added Jest coverage for explicit true/false, normalization, and invalid-value fallback behavior.
- Documented
SESSION_COOKIE_SECUREin.env.examplewith guidance for HTTP-only deployments.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/api/src/oauth/csrf.ts | Adds SESSION_COOKIE_SECURE override parsing to the secure-cookie decision helper. |
| packages/api/src/oauth/csrf.spec.ts | Adds unit tests covering the new override behavior and normalization/fallback. |
| .env.example | Documents the new SESSION_COOKIE_SECURE override and intended usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Returns `true` in production unless the server is running on localhost (HTTP). | ||
| * This allows cookies to work on `http://localhost` during local development | ||
| * even when `NODE_ENV=production` (common in Docker Compose setups). |
| it('should return true when SESSION_COOKIE_SECURE=true', () => { | ||
| process.env.NODE_ENV = 'development'; | ||
| process.env.DOMAIN_SERVER = 'http://localhost:3080'; | ||
| process.env.SESSION_COOKIE_SECURE = 'true'; | ||
| expect(shouldUseSecureCookie()).toBe(true); | ||
| }); |
danny-avila
force-pushed
the
danny-avila/fix-session-cookie-secure-override
branch
from
d3bc904 to
1ad1522
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
GitNexus: 🚀 deployedThe |
danny-avila
changed the title
danny-avila
force-pushed
the
danny-avila/fix-session-cookie-secure-override
branch
from
1ad1522 to
fa930ee
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
GitNexus: 🚀 deployedThe |
* fix: add session cookie secure override * chore: remove empty whitespace
* fix: add session cookie secure override * chore: remove empty whitespace
Closes #13188.
Summary
I added an explicit
SESSION_COOKIE_SECUREoverride for LibreChat session and auth cookies so HTTP-only deployments can opt out of theSecurecookie flag when the production hostname heuristic would otherwise enable it.SESSION_COOKIE_SECURE=true|falsehandling insideshouldUseSecureCookie()before the existingNODE_ENVandDOMAIN_SERVERheuristic runs..env.example, including guidance to set it tofalseonly for HTTP-only deployments where browsers dropSecurecookies.true, explicitfalse, normalized values, and invalid-value fallback.The behavior that broke HTTP-only non-localhost OIDC deployments came from secure-cookie hardening in #11407, followed by the localhost-only bypass in #11518. That fixed local Docker over HTTP but still classified non-localhost HTTP endpoints as secure-cookie deployments, so browsers dropped the OIDC
connect.sidcookie and later auth cookies.Change Type
Testing
npm run smart-reinstallto install dependencies and build packages.cd packages/api && npx jest src/oauth/csrf.spec.ts --coverage=false --runInBand.Test Configuration:
packages/api/src/oauth/csrf.spec.tspasses with 19 tests.Checklist