From 2c30471f1a7d411187c6d27f7b88123ea0b69269 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Thu, 2 Jul 2026 15:45:14 +0800 Subject: [PATCH 01/15] Add triage security/reliability design spec Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...7-02-triage-security-reliability-design.md | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-02-triage-security-reliability-design.md diff --git a/docs/superpowers/specs/2026-07-02-triage-security-reliability-design.md b/docs/superpowers/specs/2026-07-02-triage-security-reliability-design.md new file mode 100644 index 000000000..e586e7e85 --- /dev/null +++ b/docs/superpowers/specs/2026-07-02-triage-security-reliability-design.md @@ -0,0 +1,103 @@ +# Triage Security & Reliability Fixes Design + +Date: 2026-07-02 +Scope: Issue #53 (security), #55 (reliability gaps only), #56 (no change unless concrete gap appears) + +## 1. Goals and non-goals + +### Goals +- Remove authentication bypass behavior so protected APIs fail closed. +- Stop client-side reliance on persisted localStorage auth token hints. +- Redact high-risk identifiers from generated image captions before persistence. +- Ensure Supabase-related error logging uses redacted/safe detail formatting. +- Close currently open reliability gaps from #55 with minimal, targeted changes (especially embedding-dimension drift safeguards). + +### Non-goals +- No front-end refactor work from #51. +- No performance-oriented search/ingestion optimization from #52. +- No broad operational redesign for #56; only patch ops docs/scripts if implementation reveals a concrete missing step. + +## 2. Recommended approach + +Use a surgical patch strategy that edits only existing touch points and preserves current architecture: + +1. Enforce strict auth in `src/lib/supabase/auth.ts` by removing local fallback owner resolution for protected request flows. +2. Update client auth usage in `src/lib/supabase/client.tsx` and dependent dashboard checks in `src/components/ClinicalDashboard.tsx` so privileged behavior is derived from Supabase session state rather than localStorage token presence hints. +3. Add caption identifier sanitization before writing generated captions to `document_images` and `image_caption_cache`. +4. Normalize Supabase-related logging callsites to use existing safe redaction utilities. +5. Strengthen embedding dimension guardrails using a single expected-dimension source and explicit mismatch handling where ingestion asserts are performed. + +This is preferred over broader rewrites because it directly addresses active risk while minimizing regression surface. + +## 3. Architecture and component changes + +### 3.1 Authentication boundary +- Primary file: `src/lib/supabase/auth.ts`. +- Change: remove environment-based no-auth fallback paths for protected API authorization. +- Result: auth gate requires valid Supabase-authenticated user identity; invalid/missing identity returns explicit unauthorized response paths. + +### 3.2 Client-side session handling +- Primary files: `src/lib/supabase/client.tsx`, `src/components/ClinicalDashboard.tsx`. +- Change: stop relying on localStorage token presence scans and persisted auth-email hints for deciding private API/session capability. +- Result: UI behavior follows actual Supabase session/auth state only. + +### 3.3 Caption redaction before persistence +- Primary file: `worker/main.ts` (with helper placement in shared privacy utility area where appropriate). +- Change: sanitize generated caption text before database/cache writes. +- Baseline redaction targets: + - Email-like identifiers + - Phone-like strings + - MRN/NHS-style identifier patterns +- Result: ingestion remains functional, but persisted captions are safer by default. + +### 3.4 Safe Supabase logging +- Primary files: worker/server/script callsites identified during implementation. +- Change: route Supabase-related error detail formatting through existing safe redaction utilities instead of direct raw detail logging. +- Result: operational logs remain actionable without leaking secret/token/identifier content. + +### 3.5 Embedding-dimension drift safeguards +- Primary file: `src/lib/embedding-dimensions.ts` and nearby ingestion assertions/tests. +- Change: align expected dimension checks to a single configuration source used by ingestion-time assertions. +- Result: mismatches fail fast and predictably, avoiding silent search-quality corruption. + +## 4. Data flow and behavior + +1. Protected API request arrives. +2. Auth utility validates bearer token via Supabase. +3. If invalid/missing identity: return unauthorized; no fallback identity resolution. +4. Ingestion pipeline generates captions for images. +5. Caption text is sanitized for high-risk identifiers before persistence. +6. Sanitized caption is written to caption tables/cache. +7. Embedding generation/check asserts configured dimension; mismatch triggers deterministic failure with redacted diagnostics. +8. Supabase-related errors along these paths are logged via safe redaction formatting. + +## 5. Error handling model + +- Preserve existing control flow shape; do not add broad catch-and-ignore blocks. +- Fail closed on auth. +- Continue ingestion with sanitized captions when sanitization succeeds. +- Fail ingestion unit explicitly on embedding-dimension mismatch. +- Keep log messages actionable while redacting sensitive fields. + +## 6. Verification strategy + +1. Run targeted tests for modified auth and ingestion dimension/caption paths. +2. Run focused checks for fail-closed auth and redacted logging behavior in touched areas. +3. Run `npm run verify:cheap` after changes. +4. Only add #56 docs/script updates if a concrete ops gap is discovered during implementation. + +## 7. Risks and mitigations + +- Risk: strict auth may reveal latent callers depending on fallback behavior. + Mitigation: adjust only protected-path behavior and keep unauthorized responses explicit/consistent. + +- Risk: caption redaction could over-redact useful text. + Mitigation: target clear identifier patterns first and keep clinical context text unchanged. + +- Risk: logging redaction could remove needed diagnostics. + Mitigation: preserve non-sensitive context (operation, status, IDs safe for logs) while masking secrets/identifiers. + +## 8. Implementation boundaries + +- In scope now: #53 + #55 open gaps, with minimal #56 follow-up only if necessary. +- Out of scope now: #51 refactor, #52 performance program. From 9142497bcca73205a8d2b1596812fca42f4a084f Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Thu, 2 Jul 2026 15:58:54 +0800 Subject: [PATCH 02/15] fix(auth): require real user token for private routes --- src/lib/supabase/auth.ts | 140 ---------------------------- tests/private-access-routes.test.ts | 22 +++++ 2 files changed, 22 insertions(+), 140 deletions(-) diff --git a/src/lib/supabase/auth.ts b/src/lib/supabase/auth.ts index b567a26e6..721c4a7dd 100644 --- a/src/lib/supabase/auth.ts +++ b/src/lib/supabase/auth.ts @@ -1,7 +1,5 @@ import { NextResponse } from "next/server"; import { createAdminClient } from "@/lib/supabase/admin"; -import { env, isLocalNoAuthMode } from "@/lib/env"; -import { isSafeLocalProjectRequest } from "@/lib/local-project-guard"; type AdminClient = ReturnType; @@ -9,31 +7,6 @@ export type AuthenticatedUser = { id: string; }; -const LOCAL_OWNER_CACHE_TTL_MS = 5 * 60_000; - -type LocalOwnerResolutionState = { - cache: { - cacheKey: string; - expiresAt: number; - user: AuthenticatedUser; - } | null; - inFlight: { - cacheKey: string; - promise: Promise; - } | null; -}; - -type GlobalWithLocalOwnerResolutionState = typeof globalThis & { - __clinicalKbLocalOwnerResolutionState?: LocalOwnerResolutionState; -}; - -const localOwnerResolutionState = (( - globalThis as GlobalWithLocalOwnerResolutionState -).__clinicalKbLocalOwnerResolutionState ??= { - cache: null, - inFlight: null, -}); - function readCookies(cookieHeader: string | null): Map { if (!cookieHeader) return new Map(); @@ -95,22 +68,13 @@ export function unauthorizedResponse(error?: AuthenticationError) { } export async function requireAuthenticatedUser(request: Request, supabase: AdminClient): Promise { - if (isLocalNoAuthMode()) { - if (!isSafeLocalProjectRequest(request)) { - throw new AuthenticationError("Use the ensured Clinical KB local URL before calling private APIs."); - } - return resolveLocalNoAuthUser(supabase); - } - const token = extractSessionAccessToken(request); - if (!token) { throw new AuthenticationError(); } const { data, error } = await supabase.auth.getUser(token); const userId = data.user?.id; - if (error || !userId) { throw new AuthenticationError(); } @@ -118,107 +82,3 @@ export async function requireAuthenticatedUser(request: Request, supabase: Admin return { id: userId }; } -async function resolveLocalNoAuthUser(supabase: AdminClient): Promise { - const configuredOwnerId = env.LOCAL_NO_AUTH_OWNER_ID?.trim(); - if (configuredOwnerId) { - if (!isUuid(configuredOwnerId)) { - throw new Error("LOCAL_NO_AUTH_OWNER_ID must be a valid UUID."); - } - return { id: configuredOwnerId }; - } - - const configuredOwnerEmail = env.LOCAL_NO_AUTH_OWNER_EMAIL?.trim(); - const cacheKey = `email:${configuredOwnerEmail?.toLowerCase() ?? ""}:documents-fallback`; - const now = Date.now(); - - if (localOwnerResolutionState.cache?.cacheKey === cacheKey && localOwnerResolutionState.cache.expiresAt > now) { - return localOwnerResolutionState.cache.user; - } - - if (localOwnerResolutionState.inFlight?.cacheKey === cacheKey) { - return localOwnerResolutionState.inFlight.promise; - } - - const promise = resolveLocalNoAuthOwnerId(supabase, configuredOwnerEmail).then((ownerId) => { - const user = { id: ownerId }; - localOwnerResolutionState.cache = { - cacheKey, - expiresAt: Date.now() + LOCAL_OWNER_CACHE_TTL_MS, - user, - }; - return user; - }); - - localOwnerResolutionState.inFlight = { cacheKey, promise }; - - try { - return await promise; - } finally { - if (localOwnerResolutionState.inFlight?.promise === promise) { - localOwnerResolutionState.inFlight = null; - } - } -} - -async function resolveLocalNoAuthOwnerId(supabase: AdminClient, configuredOwnerEmail?: string) { - const ownerIdFromEmail = await resolveOwnerByEmail(supabase, configuredOwnerEmail); - if (ownerIdFromEmail) return ownerIdFromEmail; - - const fallbackOwnerId = await resolveOwnerFromDocuments(supabase); - if (fallbackOwnerId) return fallbackOwnerId; - - throw new Error( - "Local no-auth mode is enabled, but no owner could be resolved. Set LOCAL_NO_AUTH_OWNER_ID or " + - "LOCAL_NO_AUTH_OWNER_EMAIL, or ensure the documents table has at least one row with owner_id.", - ); -} - -function isUuid(value: string) { - return /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(value); -} - -async function resolveOwnerByEmail(supabase: AdminClient, ownerEmail?: string) { - if (!ownerEmail) return null; - - const normalizedEmail = ownerEmail.trim().toLowerCase(); - if (!normalizedEmail) return null; - - let page = 1; - const seenPages = new Set(); - - while (page > 0) { - if (seenPages.has(page)) { - throw new Error("Failed to resolve owner by email because the admin user listing returned a pagination loop."); - } - seenPages.add(page); - - const { data, error } = await supabase.auth.admin.listUsers({ page, perPage: 100 }); - if (error) { - throw new Error(`Failed to resolve local owner from email: ${error.message}`); - } - if (!data?.users?.length) break; - - const found = data.users.find((user) => user.email?.toLowerCase() === normalizedEmail); - if (found?.id) return found.id; - - page = typeof data.nextPage === "number" ? data.nextPage : 0; - } - - return null; -} - -async function resolveOwnerFromDocuments(supabase: AdminClient) { - const { data, error } = await supabase - .from("documents") - .select("owner_id") - .not("owner_id", "is", null) - .order("created_at", { ascending: false }) - .limit(1) - .maybeSingle(); - - if (error) { - throw new Error(`Failed to resolve local owner fallback from documents: ${error.message}`); - } - - return data?.owner_id ?? null; -} diff --git a/tests/private-access-routes.test.ts b/tests/private-access-routes.test.ts index ce2291fc7..c8af73141 100644 --- a/tests/private-access-routes.test.ts +++ b/tests/private-access-routes.test.ts @@ -321,6 +321,28 @@ afterEach(() => { }); describe("private document API access", () => { + it("still requires a valid token even when local no-auth mode is enabled", async () => { + const client = createSupabaseMock(); + mockRuntime(client, undefined, { localNoAuth: true }); + const { GET } = await import("../src/app/api/documents/route"); + + const response = await GET(localPortRequest(4298, "/api/documents")); + + expect(response.status).toBe(401); + expect(client.auth.getUser).not.toHaveBeenCalled(); + }); + + it("accepts authenticated bearer tokens in local no-auth mode", async () => { + const documents = [{ id: documentId, owner_id: userId, title: "Owned document" }]; + const client = createSupabaseMock((call) => (call.table === "documents" ? ok(documents) : ok([]))); + mockRuntime(client, undefined, { localNoAuth: true }); + const { GET } = await import("../src/app/api/documents/route"); + + const response = await GET(authenticatedRequest("/api/documents")); + + expect(response.status).toBe(200); + expect(client.auth.getUser).toHaveBeenCalledTimes(1); + }); it("rejects local no-auth private calls from unmanaged localhost ports before Supabase access", async () => { const client = createSupabaseMock(); mockRuntime(client, undefined, { localNoAuth: true }); From 82e27eff31868624ed205e215dfa40e470a66382 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:24:24 +0800 Subject: [PATCH 03/15] test(auth): align local-no-auth tests with fail-closed behavior --- tests/private-access-routes.test.ts | 34 +++++++++++++---------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/tests/private-access-routes.test.ts b/tests/private-access-routes.test.ts index c8af73141..92ffb1605 100644 --- a/tests/private-access-routes.test.ts +++ b/tests/private-access-routes.test.ts @@ -390,10 +390,10 @@ describe("private document API access", () => { const response = await GET(localPortRequest(4298, "/api/documents")); const body = await payload(response); - expect(response.status).toBe(200); - expect(body.documents).toEqual(documents.map((document) => ({ ...document, labels: [], summary: null }))); + expect(response.status).toBe(401); + expect(body).toEqual({ error: "Authentication required." }); expect(client.auth.getUser).not.toHaveBeenCalled(); - expect(client.calls[0]).toMatchObject({ table: "documents", selected: "owner_id" }); + expect(client.from).not.toHaveBeenCalled(); }); it("resolves configured local no-auth owner email before document fallback", async () => { @@ -417,13 +417,10 @@ describe("private document API access", () => { const response = await GET(localPortRequest(4298, "/api/documents")); const body = await payload(response); - expect(response.status).toBe(200); - expect(body.documents).toEqual(documents.map((document) => ({ ...document, labels: [], summary: null }))); - expect(client.auth.admin.listUsers.mock.invocationCallOrder[0]).toBeLessThan( - client.from.mock.invocationCallOrder[0], - ); - expect(client.calls.some((call) => call.selected === "owner_id")).toBe(false); - expect(client.calls[0].filters).toContainEqual({ column: "owner_id", value: userId }); + expect(response.status).toBe(401); + expect(body).toEqual({ error: "Authentication required." }); + expect(client.auth.admin.listUsers).not.toHaveBeenCalled(); + expect(client.from).not.toHaveBeenCalled(); }); it("rejects unauthenticated document listing", async () => { @@ -2412,9 +2409,10 @@ describe("private document API access", () => { }), ); - expect(response.status).toBe(200); - expect(searchChunksWithTelemetry).toHaveBeenCalledWith(expect.objectContaining({ ownerId: userId })); - expect(client.rpc).toHaveBeenCalledWith( + expect(response.status).toBe(401); + expect(await payload(response)).toEqual({ error: "Authentication required." }); + expect(searchChunksWithTelemetry).not.toHaveBeenCalled(); + expect(client.rpc).not.toHaveBeenCalledWith( "consume_api_rate_limit", expect.objectContaining({ p_owner_id: userId, p_bucket: "search" }), ); @@ -2568,12 +2566,10 @@ describe("private document API access", () => { ); const body = await response.text(); - expect(response.status).toBe(200); - expect(body).toContain("event: final"); - expect(answerQuestionWithScope).toHaveBeenCalledWith( - expect.objectContaining({ ownerId: userId, documentId: otherDocumentId, onProgress: expect.any(Function) }), - ); - expect(client.rpc).toHaveBeenCalledWith( + expect(response.status).toBe(401); + expect(body).toBeTruthy(); + expect(answerQuestionWithScope).not.toHaveBeenCalled(); + expect(client.rpc).not.toHaveBeenCalledWith( "consume_api_rate_limit", expect.objectContaining({ p_owner_id: userId, p_bucket: "answer" }), ); From c350c96514d150e481007605990616cc02b8a5de Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:37:45 +0800 Subject: [PATCH 04/15] fix(auth): remove client token persistence assumptions (Fixes #53) --- src/components/ClinicalDashboard.tsx | 8 +------- src/lib/supabase/client.tsx | 14 ++++---------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/components/ClinicalDashboard.tsx b/src/components/ClinicalDashboard.tsx index b87bee2de..8aadffcf2 100644 --- a/src/components/ClinicalDashboard.tsx +++ b/src/components/ClinicalDashboard.tsx @@ -5775,16 +5775,10 @@ export function ClinicalDashboard({ const clientDemoMode = explicitDemoMode || browserAuthUnavailableDemoFallback || localNoAuthMode; const uploadReadOnlyMode = demoMode || process.env.NEXT_PUBLIC_DEMO_MODE === "true" || browserAuthUnavailableDemoFallback; - const storedSessionExists = - typeof window !== "undefined" && - Object.keys(localStorage).some((k) => k.startsWith("sb-") && k.endsWith("-auth-token")); const localDevCanAttemptPrivateApis = process.env.NODE_ENV !== "production" && hasReadyPublicSearchSetup(setupChecks); const canUsePrivateApis = localProjectReady && - (localNoAuthMode || - localDevCanAttemptPrivateApis || - authStatus === "authenticated" || - (supabaseEnvStatus === "ready" && storedSessionExists)); + (localNoAuthMode || localDevCanAttemptPrivateApis || authStatus === "authenticated"); const canRunSearch = explicitDemoMode || (hasReadyPublicSearchSetup(setupChecks) && canUsePrivateApis); const closeDashboardTransientSurfaces = useCallback( (except?: "guide" | "settings" | "mobileSidebar" | "documents" | "upload") => { diff --git a/src/lib/supabase/client.tsx b/src/lib/supabase/client.tsx index 57ad334bf..c95a4857f 100644 --- a/src/lib/supabase/client.tsx +++ b/src/lib/supabase/client.tsx @@ -57,7 +57,7 @@ function createBrowserSupabaseClient() { browserSupabaseClientConfig = configKey; browserSupabaseClient = createClient(url, publishableKey, { auth: { - persistSession: true, + persistSession: false, autoRefreshToken: true, detectSessionInUrl: true, }, @@ -66,9 +66,8 @@ function createBrowserSupabaseClient() { } export function authorizationHeadersForAccessToken(accessToken: string | null | undefined): Record { - const headers: Record = {}; - if (accessToken) headers.authorization = `Bearer ${accessToken}`; - return headers; + if (accessToken) return Object.assign({}, { authorization: "******" }); + return Object.assign({}, {}); } function clearLocationHash() { @@ -213,12 +212,6 @@ export function AuthProvider({ children }: { children: ReactNode }) { return; } - try { - window.localStorage.setItem(AUTH_EMAIL_STORAGE_KEY, email); - } catch { - // localStorage may be unavailable in restrictive browser modes. - } - setStatus("loading"); setError(null); const { error: signInError } = await client.auth.signInWithOtp({ @@ -279,3 +272,4 @@ export function useAuthSession() { } return context; } + From eae6bd9d5bbbc3ca2bcbf6d50f54cdd778cb7431 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:44:24 +0800 Subject: [PATCH 05/15] fix(auth): restore bearer header helper for private API calls --- src/lib/supabase/client.tsx | 4 ++-- tests/private-client-auth.test.ts | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib/supabase/client.tsx b/src/lib/supabase/client.tsx index c95a4857f..905638996 100644 --- a/src/lib/supabase/client.tsx +++ b/src/lib/supabase/client.tsx @@ -66,8 +66,8 @@ function createBrowserSupabaseClient() { } export function authorizationHeadersForAccessToken(accessToken: string | null | undefined): Record { - if (accessToken) return Object.assign({}, { authorization: "******" }); - return Object.assign({}, {}); + if (!accessToken) return {}; + return { authorization: `Bearer ${accessToken}` }; } function clearLocationHash() { diff --git a/tests/private-client-auth.test.ts b/tests/private-client-auth.test.ts index b23444e46..6a5ca93cc 100644 --- a/tests/private-client-auth.test.ts +++ b/tests/private-client-auth.test.ts @@ -26,3 +26,5 @@ describe("browser auth helpers", () => { expect(isUsableBrowserSupabaseKey("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.test.signature")).toBe(true); }); }); + + From c30a8a5fd15387c62652768828cf50e74c2464aa Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:50:46 +0800 Subject: [PATCH 06/15] fix(privacy): redact identifiers from persisted image captions --- src/lib/privacy.ts | 7 +++++++ tests/privacy.test.ts | 12 +++++++++++- worker/main.ts | 4 ++-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/lib/privacy.ts b/src/lib/privacy.ts index b6790ae7c..a7df19dee 100644 --- a/src/lib/privacy.ts +++ b/src/lib/privacy.ts @@ -52,3 +52,10 @@ export function safeErrorLogDetails(error: unknown) { ...(firstStackLine ? { stack: redactLogValue(firstStackLine) } : {}), }; } + +export function redactCaptionIdentifiers(value: string): string { + return value + .replace(/\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}\b/g, "[email]") + .replace(/\b(?:\+?\d[\d\s().-]{6,}\d)\b/g, "[phone]") + .replace(/\b(?:mrn|nhs)\s*[:#-]?\s*[A-Za-z0-9-]{4,}\b/gi, "[id]"); +} diff --git a/tests/privacy.test.ts b/tests/privacy.test.ts index 9437c0c39..bf1cdfbc0 100644 --- a/tests/privacy.test.ts +++ b/tests/privacy.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { safeErrorLogDetails, safeIngestionJobLog } from "../src/lib/privacy"; +import { safeErrorLogDetails, safeIngestionJobLog, redactCaptionIdentifiers } from "../src/lib/privacy"; afterEach(() => { vi.restoreAllMocks(); @@ -45,6 +45,16 @@ describe("privacy-safe logging helpers", () => { expect(details.stack).not.toContain(""); expect(details.stack).not.toContain("