[Security] requirePreviewAccess grants access when the build session is missing

[SakethSumanBathini]

Summary

requirePreviewAccess skips its ownership check whenever creatorLogin is falsy, which includes the case where the session isn't in memory at all. Since the handler below it falls back to reading from disk, any authenticated user holding a session ID can read another user's rendered document after a server restart.

The code

backend/src/routes/preview.routes.ts:127

const owner = buildExecutor.getSession(sessionId)?.creatorLogin;

if (owner && owner !== login) {
  return res.status(403).send('Access denied');
}

return next();

The owner && guard means a falsy owner never reaches the comparison, and execution falls through to next().

owner=alice  login=alice  -> ALLOWED   (correct)
owner=alice  login=bob    -> 403       (correct)
owner=null   login=bob    -> ALLOWED   <- creatorLogin explicitly null
owner=undef  login=bob    -> ALLOWED   <- session not in memory
owner=""     login=bob    -> ALLOWED

Why the missing-session case is routine, not exotic

buildExecutor holds sessions in a plain in-memory Map (this.sessions = new Map()) behind a TTL cleanup timer. There's no persistence. So every process restart and every TTL expiry empties it, after which no session ID resolves and getSession returns undefined for all of them.

creatorLogin is also written as an explicit null in six construction paths in buildExecutor.ts (lines 1111, 1151, 1182, 1221, 1267 and 365), so the null case is reachable without a restart at all.

Why it leaks data rather than just failing open

The handler doesn't need the in-memory session to serve files. When it's absent it reads straight from disk:

const activeSession = buildExecutor.getSession(sessionId);
const outputPath = activeSession
  ? path.resolve(activeSession.outputPath)
  : path.resolve(getProofdeskDataPath(sessionId, 'output'));

So the documents are still there and still served — the ownership gate is the only thing that was standing between them and any other logged-in user.

Path traversal within the preview root is correctly blocked further down (line 161, with the path.sep boundary). The defect is the ownership check above it, not the file resolution.

This is the same bug that was already fixed next door

checkWorkspaceOwner in backend/src/middleware/auth.ts had exactly this flaw, and the fix carries a comment describing the mechanism:

An unresolvable session is refused rather than waved through. This previously fell through to next(), which meant the ownership check was skipped entirely whenever the session could not be found — and that is not an exotic state. buildExecutor holds sessions in an in-memory Map behind a TTL cleanup timer, so every expiry and every process restart empties it.

requirePreviewAccess implements the same check independently and was missed in that pass.

Suggested fix

Mirror the corrected pattern — refuse an unresolvable session rather than continuing:

const buildSession = buildExecutor.getSession(sessionId);

if (!buildSession) {
  return res.status(404).send('Session not found');
}

if (buildSession.creatorLogin && buildSession.creatorLogin !== login) {
  return res.status(403).send('Access denied');
}

404 rather than 403 for the missing case, matching checkWorkspaceOwner: it doesn't reveal whether the ID ever existed.

One thing worth deciding separately

Both this and the already-fixed checkWorkspaceOwner retain creatorLogin && ..., so a session whose creator is null still passes for any authenticated user. That's a narrower hole than the missing-session case and may be deliberate — sessions created outside an authenticated flow. Worth confirming rather than assuming; if it isn't intentional, both middlewares need the same follow-up.

I have the fix ready and will open a PR shortly.