From 9811610d55d8be3d8ac88710fca2bae80607fdde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20M=C3=BCller?= Date: Sun, 6 Sep 2026 14:44:27 +0200 Subject: [PATCH] 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 --- DEFERRED_SECURITY.md | 14 +++++++++++++- test/admin/auth-binding.ts | 5 +++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/DEFERRED_SECURITY.md b/DEFERRED_SECURITY.md index 0321d1a..a933937 100644 --- a/DEFERRED_SECURITY.md +++ b/DEFERRED_SECURITY.md @@ -11,7 +11,19 @@ These items were identified during a security review on 2026-05-02 and conscious `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:** Move to request headers (`X-Session-Id` / `X-Session-Key`) or the request body. Requires a corresponding frontend update. +**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. diff --git a/test/admin/auth-binding.ts b/test/admin/auth-binding.ts index 21b00b2..d50a0c9 100644 --- a/test/admin/auth-binding.ts +++ b/test/admin/auth-binding.ts @@ -102,6 +102,11 @@ export const describeAdminBinding = ( expect(res.locals.admin).toMatchObject({id: 'u1', email: 'a@nachklang.art', displayName: 'Anna Admin'}); }); + // Weaker than it looks and kept deliberately: neither module imports + // checkSession any more, so this cannot fail today. It is a tripwire for + // the change that would matter - someone reintroducing a header-session + // fallback "just for the calendar users who have not been invited yet", + // which is exactly the shortcut the cutover exists to close. it('never falls back to a calendar header session', async () => { m.getSession.mockResolvedValue(null);