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

backend/src/routes/preview.routes.ts

@@ -5,6 +5,7 @@ import buildExecutor from '../services/buildExecutor.js';
 import { ensurePreviewBundle } from '../services/previewBundleService.js';
 import { injectLatestPreTeXtLayoutFix } from '../services/previewTransformService.js';
 import { getProofdeskDataPath } from '../utils/dataPaths.js';
+import authSessionStore from '../services/authSessionStore.js';
 
 const getPreviewMimeType = (ext: string): string => {
   const mimeTypes: Record<string, string> = {
@@ -76,14 +77,66 @@ const versionLivePreviewAssets = (html: string, version?: string): string => {
 export const createPreviewRouter = (): Router => {
   const router = Router();
 
-  router.get('/:sessionId/*', async (req: Request<any>, res: Response): Promise<any> => {
+  /**
+   * Gate preview access on an authenticated session (issue #51).
+   *
+   * Applied on the route rather than at the `app.use('/preview', ...)` mount, which matters:
+   * `req.params` is empty at mount level because `:sessionId` belongs to this route, so a check
+   * placed there reads `sessionId` as undefined and waves everything through.
+   *
+   * Authentication comes from the session cookie, which is also the only mechanism available to
+   * the caller that matters: a browser cannot attach an Authorization header to an iframe
+   * navigation, and the preview is loaded in an iframe. The
+   * cookie is `SameSite=Lax`, and since cookies are scoped by host rather than origin, it is still
+   * sent when the app and the API differ only by port or subdomain.
+   *
+   * On ownership: when a build session is on record and names a creator, only that user may read
+   * its preview. When no record exists the request is allowed through for any authenticated user,
+   * and that is a deliberate compromise rather than an oversight. Build sessions live in memory, so
+   * every preview on disk becomes an unowned orphan after a restart; failing closed there would
+   * break previews on every deploy. The reported hole — anyone on the internet reading a preview
+   * without logging in — is closed either way, and an orphan still requires guessing a 16-hex
+   * identifier.
+   */
+  const requirePreviewAccess = async (
+    req: Request<any>,
+    res: Response,
+    next: () => void
+  ): Promise<any> => {
     const { sessionId } = req.params;
-    const filePath = req.params[0] || 'overview.html';
 
     if (!/^[0-9a-f]{16}$/.test(sessionId)) {
       return res.status(400).send('Invalid session ID');
     }
 
+    // Only a real session cookie counts here, and a bearer token deliberately does not.
+    //
+    // `extractAccessToken` returns any bearer value it finds without checking it, setting
+    // `req.authSession` to null. On routes like /user that is harmless, because the token is
+    // forwarded to GitHub and a forged one fails there. This route never uses the token for
+    // anything, so presence would be the entire gate and `Authorization: Bearer anything` would
+    // walk straight through. Resolving the cookie directly means a request either carries a session
+    // this server issued or it does not.
+    const session = await authSessionStore.getSessionFromRequest(req);
+    const login = session?.user?.login;
+
+    if (!login) {
+      return res.status(401).send('Authentication required');
+    }
+
+    const owner = buildExecutor.getSession(sessionId)?.creatorLogin;
+
+    if (owner && owner !== login) {
+      return res.status(403).send('Access denied');
+    }
+
+    return next();
+  };
+
+  router.get('/:sessionId/*', requirePreviewAccess, async (req: Request<any>, res: Response): Promise<any> => {
+    const { sessionId } = req.params;
+    const filePath = req.params[0] || 'overview.html';
+
     const activeSession = buildExecutor.getSession(sessionId);
     const outputPath = activeSession
       ? path.resolve(activeSession.outputPath)

backend/tests/server.routes.test.js

@@ -425,34 +425,81 @@ describe('active backend routes', () => {
   });
 
   it('rewrites nested preview asset paths for knowls and shared CSS', async () => {
-    const knowlResponse = await request(app).get(`/preview/${previewSessionId}/knowl/sample.html`);
+    const knowlResponse = await request(app)
+      .get(`/preview/${previewSessionId}/knowl/sample.html`)
+      .set('Cookie', await previewCookie());
     assert.equal(knowlResponse.status, 200);
     assert.match(knowlResponse.text, /src="\/preview\/aaaaaaaaaaaaaaaa\/images\/important\.svg"/);
     assert.match(knowlResponse.text, /href="\/preview\/aaaaaaaaaaaaaaaa\/figure-images\/sample\.png"/);
     assert.match(knowlResponse.text, /href="\/preview\/aaaaaaaaaaaaaaaa\/dimension\.html#dimension-defn-basis"/);
     assert.ok(knowlResponse.text.includes("replaceDelimitedCommands(tex, ['syseq','spalignsys'], convertSpAlign)"));
     assert.match(knowlResponse.text, /inlineMath:\[\[/);
 
-    const cssResponse = await request(app).get(`/preview/${previewSessionId}/css/ila.css`);
+    const cssResponse = await request(app)
+      .get(`/preview/${previewSessionId}/css/ila.css`)
+      .set('Cookie', await previewCookie());
     assert.equal(cssResponse.status, 200);
     assert.match(cssResponse.text, /url\("\/preview\/aaaaaaaaaaaaaaaa\/fonts\/CharterBT-Roman\.woff"\)/);
   });
 
   it('injects the MathBox loader cleanup into preview HTML', async () => {
-    const response = await request(app).get(`/preview/${previewSessionId}/demo.html`);
+    const response = await request(app)
+      .get(`/preview/${previewSessionId}/demo.html`)
+      .set('Cookie', await previewCookie());
     assert.equal(response.status, 200);
     assert.match(response.text, /id="mathbox-loader-preview-fix"/);
     assert.match(response.text, /proofdesk-loader-hidden/);
     assert.match(response.text, /hideMathBoxLoaders/);
   });
 
   it('cache-busts local live preview JavaScript and CSS references', async () => {
-    const response = await request(app).get(`/preview/${previewSessionId}/demo.html?t=live-123`);
+    const response = await request(app)
+      .get(`/preview/${previewSessionId}/demo.html?t=live-123`)
+      .set('Cookie', await previewCookie());
     assert.equal(response.status, 200);
     assert.match(response.text, /href="styles\.css\?proofdeskLive=live-123"/);
     assert.match(response.text, /src="js\/demo\.js\?proofdeskLive=live-123"/);
   });
 
+  /** A session cookie from the local demo login — the only credential previews accept. */
+  const previewCookie = async () => {
+    const auth = await request(app).get('/auth/local-test');
+    return auth.headers['set-cookie'];
+  };
+
+  it('refuses a preview request carrying an unverified bearer token', async () => {
+    // A bearer value is never validated on this path, so accepting one would make the header
+    // itself the credential. Only a session this server issued may pass.
+    const response = await request(app)
+      .get(`/preview/${previewSessionId}/demo.html`)
+      .set('Authorization', 'Bearer not-a-real-token');
+    assert.equal(response.status, 401);
+  });
+
+  it('refuses a preview request with no credentials (issue #51)', async () => {
+    // The reported hole: anyone who knew a session id could read the compiled output.
+    const response = await request(app).get(`/preview/${previewSessionId}/demo.html`);
+    assert.equal(response.status, 401);
+  });
+
+  it('still rejects a malformed session id before asking for credentials', async () => {
+    const response = await request(app).get('/preview/not-a-session/demo.html');
+    assert.equal(response.status, 400);
+  });
+
+  it('accepts a preview request carrying the session cookie', async () => {
+    // A browser cannot put an Authorization header on an iframe navigation, so the cookie path is
+    // the one that actually matters in the product.
+    const auth = await request(app).get('/auth/local-test');
+    const cookie = auth.headers['set-cookie'];
+    assert.ok(cookie, 'the local demo login did not set a session cookie');
+
+    const response = await request(app)
+      .get(`/preview/${previewSessionId}/demo.html`)
+      .set('Cookie', cookie);
+    assert.equal(response.status, 200);
+  });
+
   it('serves the Prometheus metrics data', async () => {
     const response = await request(app).get('/metrics');
     assert.equal(response.status, 200);