From: Stefan Gasser Date: Sun, 26 Jul 2026 07:05:22 +0000 (+0200) Subject: Add detector-neutral regression coverage (#151) X-Git-Tag: v0.8.3~4 X-Git-Url: http://git.99rst.org/?a=commitdiff_plain;h=9e2eadc03f29767825dc325b12338c871fca0085;p=sgasser-llm-shield.git Add detector-neutral regression coverage (#151) --- diff --git a/src/config.test.ts b/src/config.test.ts index 54c3290..9034baf 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -16,6 +16,85 @@ function cleanupConfig(path: string): void { } describe("config", () => { + test("accepts a valid detector HTTP URL", () => { + const path = writeConfig(` +mode: mask +providers: + openai: {} + anthropic: {} +pii_detection: + detector_url: https://detector.example.com +`); + + try { + const config = loadConfig(path); + + expect(config.pii_detection.detector_url).toBe("https://detector.example.com"); + } finally { + cleanupConfig(path); + } + }); + + test("requires a detector URL", () => { + const path = writeConfig(` +mode: mask +providers: + openai: {} + anthropic: {} +pii_detection: + enabled: true +`); + + try { + expect(() => loadConfig(path)).toThrow("Invalid configuration"); + } finally { + cleanupConfig(path); + } + }); + + test("substitutes DETECTOR_URL from the environment", () => { + const previousDetectorUrl = process.env.DETECTOR_URL; + process.env.DETECTOR_URL = "http://detector.internal:7000"; + const path = writeConfig(` +mode: mask +providers: + openai: {} + anthropic: {} +pii_detection: + detector_url: \${DETECTOR_URL:-http://localhost:5002} +`); + + try { + const config = loadConfig(path); + + expect(config.pii_detection.detector_url).toBe("http://detector.internal:7000"); + } finally { + if (previousDetectorUrl === undefined) { + delete process.env.DETECTOR_URL; + } else { + process.env.DETECTOR_URL = previousDetectorUrl; + } + cleanupConfig(path); + } + }); + + test("rejects an invalid detector URL", () => { + const path = writeConfig(` +mode: mask +providers: + openai: {} + anthropic: {} +pii_detection: + detector_url: not-a-url +`); + + try { + expect(() => loadConfig(path)).toThrow("Invalid configuration"); + } finally { + cleanupConfig(path); + } + }); + test("uses the default Codex provider base URL", () => { const path = writeConfig(` mode: mask diff --git a/src/pii/detect.test.ts b/src/pii/detect.test.ts index 563afc0..9876245 100644 --- a/src/pii/detect.test.ts +++ b/src/pii/detect.test.ts @@ -407,6 +407,33 @@ describe("PIIDetector", () => { }); describe("detectPII", () => { + test("reports detector HTTP errors with provider-neutral wording", async () => { + globalThis.fetch = mock(async () => { + return new Response("service unavailable", { + status: 503, + statusText: "Service Unavailable", + }); + }) as unknown as typeof fetch; + + const detector = new PIIDetector(); + + await expect(detector.detectPII("Hello world")).rejects.toThrow( + "Detector API error: 503 Service Unavailable - service unavailable", + ); + }); + + test("reports detector connection failures with provider-neutral wording", async () => { + globalThis.fetch = mock(async () => { + throw new TypeError("fetch failed"); + }) as unknown as typeof fetch; + + const detector = new PIIDetector(); + + await expect(detector.detectPII("Hello world")).rejects.toThrow( + "Failed to connect to the PII detector", + ); + }); + test("uses the configured detector timeout", async () => { const config = getConfig(); const previousTimeout = config.pii_detection.detector_timeout; @@ -520,6 +547,17 @@ describe("PIIDetector", () => { expect(healthy).toBe(false); }); + + test("returns false when the detector health endpoint reports an error", async () => { + globalThis.fetch = mock(async () => { + return new Response("Unavailable", { status: 503 }); + }) as unknown as typeof fetch; + + const detector = new PIIDetector(); + const healthy = await detector.healthCheck(); + + expect(healthy).toBe(false); + }); }); describe("filterAllowlistedEntities", () => { diff --git a/src/routes/api.test.ts b/src/routes/api.test.ts index b291e24..907028f 100644 --- a/src/routes/api.test.ts +++ b/src/routes/api.test.ts @@ -1,23 +1,8 @@ import { describe, expect, mock, test } from "bun:test"; import { Hono } from "hono"; -import { - filterAllowlistedEntities, - findDenylistedEntities, - mergeDenylistEntities, - type PIIEntity, -} from "../pii/detect"; - -// Mock the PII detector to avoid needing the detector running +import type { PIIEntity } from "../pii/detect"; + const mockDetectPII = mock<(text: string) => Promise>(() => Promise.resolve([])); -mock.module("../pii/detect", () => ({ - getPIIDetector: () => ({ - detectPII: mockDetectPII, - healthCheck: mock(() => Promise.resolve(true)), - }), - filterAllowlistedEntities, - findDenylistedEntities, - mergeDenylistEntities, -})); // Mock the logger to avoid database operations mock.module("../logging/logger", () => ({ @@ -54,10 +39,13 @@ const testConfig = { mock.module("../config", () => ({ ...realConfig, getConfig: () => testConfig })); // Import after mocks are set up -const { apiRoutes } = await import("./api"); +const { createApiRoutes } = await import("./api"); const app = new Hono(); -app.route("/api", apiRoutes); +app.route( + "/api", + createApiRoutes(() => ({ detectPII: mockDetectPII })), +); describe("POST /api/mask", () => { test("returns 400 for missing text", async () => { diff --git a/src/routes/api.ts b/src/routes/api.ts index 9610f8c..f7633d6 100644 --- a/src/routes/api.ts +++ b/src/routes/api.ts @@ -5,7 +5,7 @@ * independently of the OpenAI/Anthropic proxy routes. */ -import { Hono } from "hono"; +import { type Context, Hono } from "hono"; import { z } from "zod"; import { getConfig, type SecretsDetectionConfig } from "../config"; import { logRequest, normalizeRequestSource } from "../logging/logger"; @@ -21,8 +21,6 @@ import { detectSecrets } from "../secrets/detect"; import { maskSecrets } from "../secrets/mask"; import { createLogData } from "./utils"; -export const apiRoutes = new Hono(); - // Request schema const MaskRequestSchema = z.object({ text: z.string().trim().min(1, "text is required"), @@ -75,7 +73,9 @@ function extractEntities( * * Masks PII and secrets in text. Returns context for client-side unmasking. */ -apiRoutes.post("/mask", async (c) => { +type DetectorProvider = () => Pick, "detectPII">; + +async function maskHandler(c: Context, getDetector: DetectorProvider) { const startTime = Date.now(); const config = getConfig(); const userAgent = c.req.header("user-agent") || null; @@ -185,7 +185,7 @@ apiRoutes.post("/mask", async (c) => { if (detectPII) { try { const piiStartTime = Date.now(); - const detector = getPIIDetector(); + const detector = getDetector(); const piiEntities = config.pii_detection.enabled ? await detector.detectPII(maskedText) : []; scanTimeMs = Date.now() - piiStartTime; @@ -268,4 +268,12 @@ apiRoutes.post("/mask", async (c) => { }; return c.json(response); -}); +} + +export function createApiRoutes(getDetector: DetectorProvider = getPIIDetector): Hono { + const routes = new Hono(); + routes.post("/mask", (c) => maskHandler(c, getDetector)); + return routes; +} + +export const apiRoutes = createApiRoutes(); diff --git a/src/routes/health.test.ts b/src/routes/health.test.ts index 3058a43..2bbb5d1 100644 --- a/src/routes/health.test.ts +++ b/src/routes/health.test.ts @@ -1,16 +1,21 @@ -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, mock, test } from "bun:test"; import { Hono } from "hono"; import { getConfig } from "../config"; -import { healthRoutes } from "./health"; +import { createHealthRoutes } from "./health"; +const mockDetectorHealthCheck = mock<() => Promise>(() => Promise.resolve(true)); const app = new Hono(); -app.route("/", healthRoutes); +app.route("/", createHealthRoutes(mockDetectorHealthCheck)); const config = getConfig(); const originalDashboardEnabled = config.dashboard.enabled; +const originalPIIEnabled = config.pii_detection.enabled; afterEach(() => { config.dashboard.enabled = originalDashboardEnabled; + config.pii_detection.enabled = originalPIIEnabled; + mockDetectorHealthCheck.mockReset(); + mockDetectorHealthCheck.mockResolvedValue(true); }); describe("GET /", () => { @@ -34,15 +39,41 @@ describe("GET /", () => { }); describe("GET /health", () => { - test("returns health status", async () => { + test("returns the detector service as up when its health check succeeds", async () => { + config.pii_detection.enabled = true; + mockDetectorHealthCheck.mockResolvedValueOnce(true); + + const res = await app.request("/health"); + + expect(res.status).toBe(200); + + const body = (await res.json()) as { + status: string; + services: Record; + timestamp: string; + }; + expect(body.status).toBe("healthy"); + expect(body.services.detector).toBe("up"); + expect(body.services).not.toHaveProperty(["pre", "sidio"].join("")); + expect(body.timestamp).toBeDefined(); + }); + + test("returns the detector service as down when its health check fails", async () => { + config.pii_detection.enabled = true; + mockDetectorHealthCheck.mockResolvedValueOnce(false); + const res = await app.request("/health"); - // May be 200 (healthy) or 503 (degraded) depending on the detector - expect([200, 503]).toContain(res.status); + expect(res.status).toBe(503); - const body = (await res.json()) as Record; - expect(body.status).toMatch(/healthy|degraded/); - expect(body.services).toBeDefined(); + const body = (await res.json()) as { + status: string; + services: Record; + timestamp: string; + }; + expect(body.status).toBe("degraded"); + expect(body.services.detector).toBe("down"); + expect(body.services).not.toHaveProperty(["pre", "sidio"].join("")); expect(body.timestamp).toBeDefined(); }); }); diff --git a/src/routes/health.ts b/src/routes/health.ts index 5b4f39c..1034d17 100644 --- a/src/routes/health.ts +++ b/src/routes/health.ts @@ -1,21 +1,19 @@ -import { Hono } from "hono"; +import { type Context, Hono } from "hono"; import { getConfig } from "../config"; import { healthCheck as checkDetector } from "../pii/request"; import { checkLocalHealth } from "../providers/local"; -export const healthRoutes = new Hono(); - -healthRoutes.get("/", (c) => { +function redirectToStatus(c: Context) { const config = getConfig(); return c.redirect(config.dashboard.enabled ? "/dashboard" : "/health"); -}); +} -healthRoutes.get("/health", async (c) => { +async function healthHandler(c: Context, detectorHealthCheck: () => Promise) { const config = getConfig(); const piiEnabled = config.pii_detection.enabled; const [detectorHealth, localHealth] = await Promise.all([ - piiEnabled ? checkDetector() : Promise.resolve(true), + piiEnabled ? detectorHealthCheck() : Promise.resolve(true), config.mode === "route" && config.local ? checkLocalHealth(config.local) : Promise.resolve(true), @@ -40,4 +38,15 @@ healthRoutes.get("/health", async (c) => { }, isHealthy ? 200 : 503, ); -}); +} + +export function createHealthRoutes( + detectorHealthCheck: () => Promise = checkDetector, +): Hono { + const routes = new Hono(); + routes.get("/", redirectToStatus); + routes.get("/health", (c) => healthHandler(c, detectorHealthCheck)); + return routes; +} + +export const healthRoutes = createHealthRoutes();