fix(security): require an authenticated session for preview routes (closes #51)

[SakethSumanBathini]

Summary

Going with cookie-based auth, as you picked in the issue thread.

Two things about the suggested remediation that I want to flag, because I didn't
implement it as written and I think both are worth knowing.

app.use('/preview', requireAccessToken, checkWorkspaceOwner, ...) would not have
worked. checkWorkspaceOwner reads req.params.sessionId, and at mount level
req.params is empty — :sessionId belongs to this router's route, not the mount
path. It would hit if (!login || !sessionId) return next() and allow everything
through. The authentication half would have worked and the authorization half would
have been a no-op that looked like a fix. So the check lives on the route instead.

I also didn't reuse checkWorkspaceOwner. It's fail-open in three other branches:
an unknown session, a session with no creatorLogin, and any bearer-token caller
(whose req.authSession is null, so login is undefined). That's reasonable as a
supplementary check on routes already guarded elsewhere, but not as the only guard on
this one.

Related Issue

Closes #51

Type of Change

• Bug fix
• New feature
• Documentation update
• Refactor
• Chore / dependency update

Changes Made

requirePreviewAccess runs on GET /:sessionId/*:

1. Malformed session id → 400 (unchanged, just moved ahead of the auth check so a bad
   id doesn't produce a misleading 401)
2. No authenticated token → 401
3. A build session on record naming a different creator → 403

No cookie changes were needed. extractAccessToken already reads the session cookie
before falling back to a bearer token, which is exactly what makes this viable — a
browser can't attach an Authorization header to an iframe navigation, and
DemoPage.tsx loads the preview in an iframe. The cookie is SameSite=Lax, and since
cookies are scoped by host rather than origin, it's still sent when the app and API
differ only by port (dev) or subdomain (prod). It would only fail if the backend moved
to a genuinely different registrable domain.

One deliberate compromise, please overrule if you disagree

Build sessions live in an in-memory map, so after any restart every preview on disk
is an orphan with no ownership record. I've allowed those through for any
authenticated user rather than failing closed, because failing closed would break every
preview on every deploy.

The reported hole — anyone on the internet reading a preview without logging in — is
closed either way, and reaching an orphan still needs a correct 16-hex id. But it is
weaker than strict owner-only, so if you'd rather have 403 there, say the word and I'll
change it; it's a one-line difference plus reworking the preview tests, which currently
use a session id with no build record.

AI Usage

• I did not use AI for any part of the code in this PR
• I used AI for coding (specify which tool below) and I fully understand every change I made, including which functions I changed, why I changed them, and what side effects they could create

Used Claude for coding.

Testing

Three new cases in server.routes.test.js, all passing:

• a preview request with no credentials returns 401 — the hole this closes
• a malformed session id still returns 400, before credentials are considered
• a preview request carrying the session cookie returns 200

That last one is the one that matters, since the cookie is the only mechanism available
to an iframe. The four existing preview assertions now authenticate rather than
expecting anonymous 200s.

server.routes.test.js: 19 tests, 17 pass, 2 fail. The two are
rewrites nested preview asset paths for knowls and shared CSS and
serves the Prometheus metrics data (401 ≠ 200), both of which fail identically on a
clean main and neither of which touches this change.

npx tsc --noEmit clean.

Checklist

• I was assigned to the issue before opening this PR
• My branch is up to date with main
• Code works locally and I have tested it
• PR title follows Conventional Commits format
• This PR description is written in my own words

Summary by CodeRabbit

• New Features

  • Preview content now requires authentication.
  • Authorized preview sessions can be accessed using a session cookie or bearer token.

• Bug Fixes

  • Improved preview asset routing for nested HTML and shared CSS files.
  • Added cache-busting for live preview JavaScript and CSS resources.
  • Preview demos now include improved MathBox loading cleanup.
  • Invalid or unauthorized preview requests return appropriate errors.

[coderabbitai]

Warning

Review limit reached

@SakethSumanBathini, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5ec9926a-8e0a-4ba3-9328-b3a74063676a

📥 Commits

Reviewing files that changed from the base of the PR and between dc04273 and 25a0efa.

📒 Files selected for processing (2)

• backend/src/routes/preview.routes.ts
• backend/tests/server.routes.test.js

📝 Walkthrough

Walkthrough

Preview routes now validate session IDs, require authentication, and enforce session ownership before serving files. Route tests cover asset rewriting, injected preview markup, cache busting, malformed requests, unauthenticated denial, and authenticated access.

Changes

Preview access control

Layer / File(s)	Summary
Preview authentication and ownership checks backend/src/routes/preview.routes.ts	Preview requests extract cookie or bearer credentials, reject unauthenticated requests, validate session IDs, and enforce session ownership before serving preview content.
Preview route and content coverage backend/tests/server.routes.test.js	Tests verify rewritten assets, MathBox cleanup markup, cache-busting URLs, malformed-session rejection, unauthenticated denial, and authenticated access.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: ELUSOC, VETERAN

Suggested reviewers: harsharajkumar-273

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant PreviewRoute
  participant AuthMiddleware
  participant SessionStore
  participant PreviewFiles
  Client->>PreviewRoute: Request /preview/:sessionId/*
  PreviewRoute->>AuthMiddleware: Validate session and extract credentials
  AuthMiddleware->>SessionStore: Check preview session owner
  AuthMiddleware->>PreviewFiles: Authorize matching user
  PreviewFiles-->>Client: Return preview content

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main security change to authenticated preview routes.
Linked Issues check	✅ Passed	The route now enforces authentication and session validation for preview access, addressing the unauthenticated access issue in #51.
Out of Scope Changes check	✅ Passed	The changes stay within preview-route security and related test coverage, with no clear unrelated code introduced.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/src/routes/preview.routes.ts`:
- Around line 112-124: Update the authentication guard around extractAccessToken
in the preview route so a bearer token is accepted only after credential
validation/introspection resolves a principal, or when req.authSession is
already authenticated; do not authorize based on token presence alone. Preserve
the owner comparison and ownerless-preview behavior only for authenticated
requests, and add a regression test confirming a random Bearer value returns
401.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 78f4c2dd-6974-474d-a761-f8ae513bb0d7

📥 Commits

Reviewing files that changed from the base of the PR and between 2f06e6e and dc04273.

📒 Files selected for processing (2)

• backend/src/routes/preview.routes.ts
• backend/tests/server.routes.test.js

📜 Review details

⏰ Context from checks skipped due to timeout. (1)

• GitHub Check: test