security: move session secrets to HttpOnly cookies

frontend/src/App.js

@@ -43,14 +43,13 @@ function MainApp() {
   const [knowledgeGapResults, setKnowledgeGapResults] = useState({});
 
   // ── Credential storage key ────────────────────────────────────────────────
-  // Session credentials (session_id + session_secret) are stored in
-  // sessionStorage, NOT localStorage. sessionStorage is:
+  // Session identifiers are stored in sessionStorage, NOT localStorage.
+  // New uploads rely on the backend's HttpOnly cookie for the secret instead
+  // of persisting it in browser-accessible storage. sessionStorage is:
   //   - Scoped to the browser tab — cleared automatically when the tab closes.
   //   - Never persisted to disk between browser sessions.
   //   - Inaccessible to other tabs and origins.
-  // This eliminates the long-lived credential theft risk: even if an attacker
-  // achieves XSS, the credentials become invalid the moment the tab closes
-  // (or immediately if the session TTL on the server expires first).
+  // This reduces the amount of secret material exposed to frontend JavaScript.
   //
   // pdfqa_preferred_mode is a non-sensitive UI preference and intentionally
   // stays in localStorage so the user's chosen reading mode is remembered
@@ -122,27 +121,33 @@ function MainApp() {
           (s) =>
             s &&
             typeof s.session_id === "string" &&
-            s.session_id.trim() !== "" &&
-            typeof s.session_secret === "string" &&
-            s.session_secret.trim() !== "",
+            s.session_id.trim() !== "",
         )
         .map((s) => ({
           session_id: s.session_id.trim(),
-          session_secret: s.session_secret.trim(),
+          session_secret:
+            typeof s.session_secret === "string" && s.session_secret.trim() !== ""
+              ? s.session_secret.trim()
+              : null,
         }));
     } catch (_) {
       return [];
     }
   }, []); // eslint-disable-line react-hooks/exhaustive-deps
 
   const upsertKnownSession = React.useCallback(
-    (sessionId, sessionSecret) => {
-      if (!sessionId || !sessionSecret) return;
-      if (typeof sessionId !== "string" || typeof sessionSecret !== "string") return;
+    (sessionId, sessionSecret = null) => {
+      if (!sessionId || typeof sessionId !== "string") return;
+      if (sessionSecret !== null && typeof sessionSecret !== "string") return;
       const existing = loadKnownSessions();
+      const normalizedSessionId = sessionId.trim();
+      const normalizedSessionSecret =
+        typeof sessionSecret === "string" && sessionSecret.trim() !== ""
+          ? sessionSecret.trim()
+          : null;
       const next = [
-        { session_id: sessionId.trim(), session_secret: sessionSecret.trim() },
-        ...existing.filter((s) => s.session_id !== sessionId.trim()),
+        { session_id: normalizedSessionId, session_secret: normalizedSessionSecret },
+        ...existing.filter((s) => s.session_id !== normalizedSessionId),
       ];
       try {
         localStorage.setItem(SESSION_STORAGE_KEY, encodePayload(next.slice(0, 50)));
@@ -157,6 +162,51 @@ function MainApp() {
   );
 
 
+  // One-time migration: if credentials were previously stored in localStorage
+  // under the same key (pre-fix behaviour), move them to sessionStorage and
+  // then delete them from localStorage so they are no longer readable by
+  // JavaScript after the next reload.
+  React.useEffect(() => {
+    try {
+      const legacy = localStorage.getItem(SESSION_STORAGE_KEY);
+      if (!legacy) return;
+      // Legacy format was plain JSON; try both plain and base64.
+      let parsed;
+      try { parsed = JSON.parse(legacy); } catch (_) { parsed = decodePayload(legacy); }
+      if (!Array.isArray(parsed) || parsed.length === 0) {
+        localStorage.removeItem(SESSION_STORAGE_KEY);
+        return;
+      }
+      const valid = parsed.filter(
+        (s) =>
+          s &&
+          typeof s.session_id === "string" &&
+          s.session_id.trim() !== "",
+      );
+      if (valid.length > 0) {
+        const existing = loadKnownSessions();
+        const existingIds = new Set(existing.map((s) => s.session_id));
+        const merged = [
+          ...existing,
+          ...valid
+            .filter((s) => !existingIds.has(s.session_id.trim()))
+            .map((s) => ({
+              session_id: s.session_id.trim(),
+              session_secret:
+                typeof s.session_secret === "string" && s.session_secret.trim() !== ""
+                  ? s.session_secret.trim()
+                  : null,
+            })),
+        ].slice(0, 50);
+        sessionStorage.setItem(SESSION_STORAGE_KEY, encodePayload(merged));
+      }
+      localStorage.removeItem(SESSION_STORAGE_KEY);
+    } catch (_) {
+      try { localStorage.removeItem(SESSION_STORAGE_KEY); } catch (_) {}
+    }
+  // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, []);
+
   React.useEffect(() => {
     // Load historical sessions on initial mount
     const fetchHistory = async () => {
@@ -240,8 +290,11 @@ function MainApp() {
       const url = URL.createObjectURL(file);
       const pdfId = data.document?.document_id || data.session_id;
 
-      if (data.session_id && data.session_secret) {
-        upsertKnownSession(data.session_id, data.session_secret);
+      if (data.session_id) {
+        upsertKnownSession(
+          data.session_id,
+          data.session_secret || currentPdfForUpload?.session_secret || null,
+        );
       }
 
     setPdfs((prev) => {
@@ -254,7 +307,7 @@ function MainApp() {
       url,
       chat: [],
       session_id: data.session_id,
-      session_secret: data.session_secret || null,
+      session_secret: data.session_secret || currentPdfForUpload?.session_secret || null,
     },
   ];
 

frontend/src/App.test.js

@@ -1,16 +1,14 @@
 /**
  * App.test.js — regression tests for session credential storage.
  *
- * Issue #234: session_secret was stored in localStorage, which is readable by
- * any JavaScript executing on the page (XSS risk). These tests enforce that:
+ * Issue #234: session_secret should no longer be required for browser storage.
+ * These tests enforce that:
  *
- *   1. Credentials are written to sessionStorage, not localStorage.
- *   2. The pdfqa_sessions key is absent from localStorage after a successful
- *      upload cycle (one-time migration removes any pre-existing value).
- *   3. The migration from localStorage → sessionStorage works correctly and
- *      always removes the value from localStorage regardless of outcome.
- *   4. Non-credential UI preferences (pdfqa_preferred_mode) are unaffected and
- *      continue to use localStorage as before.
+ *   1. Session identifiers are written to sessionStorage, not localStorage.
+ *   2. Session entries can be retained without persisting a secret.
+ *   3. The migration from localStorage → sessionStorage still removes the
+ *      legacy localStorage key.
+ *   4. Non-credential UI preferences (pdfqa_preferred_mode) are unaffected.
  */
 
 // ── Storage isolation helpers ────────────────────────────────────────────────
@@ -71,13 +69,18 @@ function loadKnownSessions() {
   }
 }
 
-function upsertKnownSession(sessionId, sessionSecret) {
-  if (!sessionId || !sessionSecret) return;
-  if (typeof sessionId !== "string" || typeof sessionSecret !== "string") return;
+function upsertKnownSession(sessionId, sessionSecret = null) {
+  if (!sessionId || typeof sessionId !== "string") return;
+  if (sessionSecret !== null && typeof sessionSecret !== "string") return;
   const existing = loadKnownSessions();
+  const normalizedSessionId = sessionId.trim();
+  const normalizedSessionSecret =
+    typeof sessionSecret === "string" && sessionSecret.trim() !== ""
+      ? sessionSecret.trim()
+      : null;
   const next = [
-    { session_id: sessionId.trim(), session_secret: sessionSecret.trim() },
-    ...existing.filter((s) => s.session_id !== sessionId.trim()),
+    { session_id: normalizedSessionId, session_secret: normalizedSessionSecret },
+    ...existing.filter((s) => s.session_id !== normalizedSessionId),
   ];
   sessionStorage.setItem(SESSION_KEY, encodePayload(next.slice(0, 50)));
 }
@@ -97,16 +100,22 @@ function migrateCredentialsFromLocalStorage() {
       (s) =>
         s &&
         typeof s.session_id === "string" &&
-        s.session_id.trim() !== "" &&
-        typeof s.session_secret === "string" &&
-        s.session_secret.trim() !== "",
+        s.session_id.trim() !== "",
     );
     if (valid.length > 0) {
       const existing = loadKnownSessions();
       const existingIds = new Set(existing.map((s) => s.session_id));
       const merged = [
         ...existing,
-        ...valid.filter((s) => !existingIds.has(s.session_id.trim())),
+        ...valid
+          .filter((s) => !existingIds.has(s.session_id.trim()))
+          .map((s) => ({
+            session_id: s.session_id.trim(),
+            session_secret:
+              typeof s.session_secret === "string" && s.session_secret.trim() !== ""
+                ? s.session_secret.trim()
+                : null,
+          })),
       ].slice(0, 50);
       sessionStorage.setItem(SESSION_KEY, encodePayload(merged));
     }
@@ -163,10 +172,13 @@ test("upsertKnownSession is a no-op when session_id is falsy", () => {
   expect(sessionStorage.getItem(SESSION_KEY)).toBeNull();
 });
 
-test("upsertKnownSession is a no-op when session_secret is falsy", () => {
+test("upsertKnownSession keeps session identifiers even without a secret", () => {
   upsertKnownSession("550e8400-e29b-41d4-a716-446655440000", null);
-  upsertKnownSession("550e8400-e29b-41d4-a716-446655440000", "");
-  expect(sessionStorage.getItem(SESSION_KEY)).toBeNull();
+  upsertKnownSession("550e8400-e29b-41d4-a716-446655440001", "");
+
+  const sessions = loadKnownSessions();
+  expect(sessions).toHaveLength(2);
+  expect(sessions[0].session_secret).toBeNull();
 });
 
 test("loadKnownSessions filters entries with missing or blank session_id", () => {
@@ -195,8 +207,9 @@ test("loadKnownSessions filters entries with missing or blank session_secret", (
   );
 
   const sessions = loadKnownSessions();
-  expect(sessions).toHaveLength(1);
-  expect(sessions[0].session_id).toBe("550e8400-e29b-41d4-a716-446655440002");
+  expect(sessions).toHaveLength(3);
+  expect(sessions[0].session_secret).toBeNull();
+  expect(sessions[2].session_secret).toBe("good-secret");
 });
 
 test("loadKnownSessions handles corrupt data without throwing", () => {

frontend/src/services/api.js

@@ -2,6 +2,8 @@ import axios from "axios";
 
 const API_BASE = process.env.REACT_APP_API_URL || "";
 
+axios.defaults.withCredentials = true;
+
 export const extractApiErrorMessage = (error, fallbackMessage) => {
   return (
     error?.response?.data?.detail ||
@@ -110,6 +112,7 @@ export const askQuestionStreamApi = async (question, sessionId, sessionSecret, m
   const response = await fetch(`${API_BASE}/ask/stream`, {
     method: "POST",
     headers: { "Content-Type": "application/json" },
+    credentials: "include",
     body: JSON.stringify({ question, session_id: sessionId, session_secret: sessionSecret, mode }),
     signal,
   });