Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .gitleaksignore
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
# Fingerprints of known false positives, one per line:
# <commit>:<file>:<rule>:<line>
# Fake Supabase-style secret fixture in a test that verifies secret *redaction*.
# The current tip no longer matches the rule, but this historical commit is
# still scanned on every PR run.
92cd8ac656800a0db504bdf9559bd9b30b52212c:tests/privacy.test.ts:generic-api-key:32
Original file line numberDiff line numberDiff line change
@@ -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.
4 changes: 3 additions & 1 deletion src/app/api/search/route.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ import {
queryPrivacyMetadata,
queryTextForStorage,
} from "@/lib/query-privacy";
import { safeErrorLogDetails } from "@/lib/privacy";
import type { ChunkImage, ClinicalSourceMetadata, SearchResult } from "@/lib/types";

export const runtime = "nodejs";
Expand DownExpand Up@@ -544,8 +545,9 @@ function logRetrievalDiagnostics(args: {
} catch (error) {
retrievalLogWriteMetrics.failures += 1;
retrievalLogWriteMetrics.lastFailureAt = new Date().toISOString();
const safe = safeErrorLogDetails(error);
retrievalLogWriteMetrics.lastFailureMessage =
error instanceof Error ? `${error.name}: ${error.message}` : "Unknown retrieval logging error";
typeof safe.message === "string" ? safe.message : "Unknown retrieval logging error";
if (retrievalLogWriteMetrics.failures <= 3 || retrievalLogWriteMetrics.failures % 25 === 0) {
console.warn("rag_retrieval_logs insert failed", {
...retrievalLogWriteMetrics,
Expand Down
8 changes: 1 addition & 7 deletions src/components/ClinicalDashboard.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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") => {
Expand Down
19 changes: 18 additions & 1 deletion src/lib/embedding-dimensions.ts
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,21 @@
export const EXPECTED_EMBED_DIM = 1536;
import * as envModule from "./env";

function expectedEmbeddingDimensions() {
const configured =
Object.prototype.hasOwnProperty.call(envModule, "env") &&
typeof (envModule as { env?: unknown }).env === "object" &&
(envModule as { env?: { EMBEDDING_DIMENSIONS?: unknown } }).env !== null
? (envModule as { env?: { EMBEDDING_DIMENSIONS?: unknown } }).env?.EMBEDDING_DIMENSIONS
: undefined;
if (typeof configured === "number" && Number.isInteger(configured) && configured > 0) {
return configured;
}

const fallback = Number(process.env.EMBEDDING_DIMENSIONS ?? 1536);
return Number.isInteger(fallback) && fallback > 0 ? fallback : 1536;
}

export const EXPECTED_EMBED_DIM = expectedEmbeddingDimensions();

export function assertEmbeddingDim(vec: unknown, context: string): number[] {
if (!Array.isArray(vec)) {
Expand Down
26 changes: 22 additions & 4 deletions src/lib/privacy.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,10 +19,11 @@ function redactLogValue(value: unknown): unknown {
const htmlTitle = value.match(/<title>\s*([^<]+?)\s*<\/title>/i)?.[1]?.trim();
const normalizedValue = htmlTitle ? `HTML response: ${htmlTitle}` : value;
return normalizedValue
.replace(/\b[A-Za-z]:\\[^\s'")]+/g, "[path]")
.replace(/\/(?:[^\s'")]+\/)+[^\s'")]+/g, "[path]")
.replace(/https?:\/\/[^\s'")]+/g, "[url]")
.replace(/\b(?:sk|pk|sbp|eyJ)[A-Za-z0-9._-]{12,}\b/g, "[secret]")
.replace(/\b[A-Za-z]:\\[^\s'\")]+/g, "[path]")
.replace(/\/(?:[^\s'\")]+\/)+[^\s'\")]+/g, "[path]")
.replace(/https?:\/\/[^\s'\")]+/g, "[url]")
// Redact common secret/token formats, including modern Supabase keys like sb_secret_ and sb_publishable_
.replace(/\b(?:sk|pk|sbp|sb_secret_|sb_publishable_|eyJ)[A-Za-z0-9._-]{8,}\b/g, "[secret]")
.replace(/\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}\b/g, "[email]")
.slice(0, 500);
}
Expand DownExpand Up@@ -52,3 +53,20 @@ export function safeErrorLogDetails(error: unknown) {
...(firstStackLine ? { stack: redactLogValue(firstStackLine) } : {}),
};
}

export function redactCaptionIdentifiers(value: string): string {
const clinicalRangePattern = /^\d+(?:\.\d+)?\s*-\s*\d+(?:\.\d+)?(?:\s*[A-Za-zµ/%][\w/%.-]*)?$/i;
return value
.replace(/\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}\b/g, "[email]")
.replace(/\b(?:mrn|nhs)\s*[:#-]?\s*([0-9]+(?:[ \-][0-9]+)+|[A-Za-z0-9-]{4,})\b/gi, (match, idPart: string) => {
const trimmed = idPart.replace(/\s+/g, " ").trim();
// Count digits only; require at least 4 digits to consider it an identifier (avoids short numeric ranges).
const digitCount = trimmed.replace(/\D/g, "").length;
if (digitCount > 0 && digitCount < 4) return match;
return "[id]";
})
.replace(/\b(?:\+?\d[\d\s().-]{6,}\d)\b/g, (match) => {
const digits = match.replace(/\D/g, "");
return digits.length >= 8 && !clinicalRangePattern.test(match.trim()) ? "[phone]" : match;
});
}
140 changes: 0 additions & 140 deletions src/lib/supabase/auth.ts
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,12 @@
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<typeof createAdminClient>;

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<AuthenticatedUser>;
} | null;
};

type GlobalWithLocalOwnerResolutionState = typeof globalThis & {
__clinicalKbLocalOwnerResolutionState?: LocalOwnerResolutionState;
};

const localOwnerResolutionState = ((
globalThis as GlobalWithLocalOwnerResolutionState
).__clinicalKbLocalOwnerResolutionState ??= {
cache: null,
inFlight: null,
});

function readCookies(cookieHeader: string | null): Map<string, string> {
if (!cookieHeader) return new Map<string, string>();

Expand DownExpand Up@@ -95,130 +68,17 @@ export function unauthorizedResponse(error?: AuthenticationError) {
}

export async function requireAuthenticatedUser(request: Request, supabase: AdminClient): Promise<AuthenticatedUser> {
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();
}

return { id: userId };
}

async function resolveLocalNoAuthUser(supabase: AdminClient): Promise<AuthenticatedUser> {
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<number>();

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;
}
Loading
Loading