Skip to content

fix: warn when session cookie secret is not configured#935

Merged
stevenle merged 1 commit intomainfrom
codex/add-warning-log-for-missing-session-secret
Feb 25, 2026
Merged

fix: warn when session cookie secret is not configured#935
stevenle merged 1 commit intomainfrom
codex/add-warning-log-for-missing-session-secret

Conversation

@stevenle
Copy link
Member

Motivation

  • Warn developers when a session cookie is about to be written but server.sessionCookieSecret is not configured so they know sessions may be invalidated across server restarts.

Description

  • Added a one-time process-level flag hasWarnedMissingSessionCookieSecret and a check in res.saveSession() that logs a console.warn when req.rootConfig?.server?.sessionCookieSecret is not set; change is in packages/root/src/middleware/session.ts.

Testing

  • No automated tests were run for this change.

Codex Task

@stevenle
Copy link
Member Author

@jeremydw here's a PR for that issue you were debugging earlier today.

@jeremydw
Copy link
Member

LGTM but i think this will appear in the deployed app logs (and not locally in dev mode due to https://github.com/blinkk/rootjs/pull/895/changes) - is that right?

if so maybe we need to log something more prominently since, at least in cloud console's logs viewer, it might be hard to spot that

@stevenle
Copy link
Member Author

it checks the root.config.ts setting explicitly so it should appear in dev afaict

@jeremydw
Copy link
Member

ah right - of course. sounds good. this is a good temp fix - we may still want to show a "setup error" on the CMS login page though if it's not configured (to prevent the eng from having to check the logs on first setup)

@stevenle
Copy link
Member Author

agreed, let me work on a warning box for that in a follow-up.

@stevenle stevenle merged commit 2a5cdf8 into main Feb 25, 2026
1 check passed
@stevenle stevenle deleted the codex/add-warning-log-for-missing-session-secret branch February 25, 2026 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants