Files
API/DEFERRED_SECURITY.md
Paddy 9811610d55 Update the deferred-security advice for the cutover
DEFERRED_SECURITY.md item 1 still told the calendar to move its session
credentials from query parameters into X-Session-Id/X-Session-Key. That
was the right advice when two other modules read those headers; both
stopped in the cutover, so following it now would build a second
mechanism just as the first is being retired. The fix is the shared
admin identity, which closes the item outright rather than moving the
credential somewhere safer.

Also annotates the one assertion in auth-binding.ts that cannot
currently fail. It is kept deliberately - it is a tripwire against
someone reintroducing a header-session fallback for calendar users who
have not been invited yet, which is the shortcut this whole step exists
to close - but that was worth saying out loud rather than leaving it to
look like an oversight.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-06 14:44:27 +02:00

3.7 KiB

Deferred Security Issues

These items were identified during a security review on 2026-05-02 and consciously deferred. Must be addressed before opening the application to a larger or public userbase.


1. Session credentials in URL query parameters (logged-in users)

Files: src/models/calendar/events/events.router.ts — all GET/PUT/DELETE handlers

sessionId and sessionKey are currently read from query parameters, which means they appear in server access logs, browser history, proxy logs, and Referer headers.

Fix (updated 2026-09-06): Move the calendar onto the shared admin identity - requireAppAccess('calendar') against the better-auth session cookie, per docs/calendar-auth-migration.md. That closes this item outright rather than moving the credential to a safer place, and it is now the cheaper of the two: the feedback and tickets modules made the same move on 2026-09-06 for one line each.

Move to request headers (X-Session-Id / X-Session-Key) or the request body. No longer the recommendation. Nothing on the server reads those two headers any more - the calendar's query parameters are the last legacy credential path in the API - so this would build a second mechanism just as the first is being retired. They survive only in the CORS allowedHeaders list, and only until both frontends are redeployed.

Either fix requires a corresponding frontend update.

Note: the shared calendar password parameter in query params is intentional (iCal clients don't support headers) and is acceptable for the current setup.


2. No event ownership check

Files: src/models/calendar/events/events.router.ts

  • PUT /:eventId (update)
  • PUT /move/:eventId (move)
  • DELETE /:eventId (delete)

Currently any active user can edit, move, or delete any event regardless of who created it. This is acceptable while all users are trusted admins.

Fix: When non-admin users are introduced, fetch the event first and verify event.createdById === user.userId before allowing the mutation. Add an isAdmin flag to the user model to let admins bypass the check.


3. Activation token has no expiry

Superseded for new accounts (2026-09-05). The admin module (src/models/admin/) replaced account creation for the feedback, tickets and admin apps: accounts now come from invitations, whose tokens expire after 7 days and are stored only as a SHA-256 hash. The item below still stands for the legacy calendar users table, which the admin module deliberately left alone - see docs/calendar-auth-migration.md.

File: src/models/calendar/users/users.service.tscreateUser / activateUser

The email activation link is valid indefinitely. Acceptable for a small, trusted userbase.

Fix:

  1. Add an activation_expires column to the users table (e.g. DATETIME).
  2. Set it to NOW() + INTERVAL 24 HOUR in createUser.
  3. Check activation_expires > NOW() in activateUser before accepting the token.

4. Password reset token has no expiry

Superseded for new accounts (2026-09-05). Password resets for admin-module accounts go through better-auth, whose reset tokens expire after one hour. As with item 3, the text below still applies to the legacy calendar users table.

File: src/models/calendar/users/users.service.tsinitiatePasswordReset / finalizePasswordReset

The reset token stored in pw_reset_token_hash never expires. Acceptable for a small, trusted userbase.

Fix:

  1. Add a pw_reset_expires column to the users table (e.g. DATETIME).
  2. Set it to NOW() + INTERVAL 15 MINUTE in initiatePasswordReset.
  3. Check pw_reset_expires > NOW() in finalizePasswordReset before accepting the token.