From: Stefan Gasser Date: Wed, 10 Jun 2026 12:03:49 +0000 (+0200) Subject: Fix masked content not logged when secrets are detected X-Git-Tag: v0.4.2~1^2~2 X-Git-Url: http://git.99rst.org/?a=commitdiff_plain;h=c982e62e461b88139f4a7d0cb53d8920c1cc5171;p=sgasser-llm-shield.git Fix masked content not logged when secrets are detected Masked content was never stored whenever secrets were detected, ignoring the log_masked_content setting. The dashboard then showed a misleading "Masked content not logged (log_masked_content: false)" message. With secrets_detection.action "mask" (the default), maskedContent already has both PII and secrets replaced by placeholders by the time it reaches the logger, so it is safe to store. Gate on log_masked_content plus whether detected secrets were actually masked, via a pure shouldLogMaskedContent helper. The secrets-masked condition keeps route mode with action "route_local" safe: there secrets are intentionally left raw for the trusted local provider and must never be persisted. This also enforces log_masked_content centrally for the openai/anthropic/ codex routes, which previously passed masked content unconditionally. The helper lives in its own module so the unit test can import it without tripping over the wholesale logger mock used by other route tests. Fixes #91 --- diff --git a/src/routes/utils.ts b/src/routes/utils.ts index a68494e..11ae6de 100644 --- a/src/routes/utils.ts +++ b/src/routes/utils.ts @@ -239,6 +239,7 @@ export function createLogData(options: CreateLogDataOptions): RequestLogData { detectedLanguage: pii?.detectedLanguage, maskedContent, secretsDetected: secrets?.detected, + secretsMasked: secrets?.masked, secretsTypes: secrets?.types, statusCode, errorMessage, diff --git a/src/services/log-content.test.ts b/src/services/log-content.test.ts new file mode 100644 index 0000000..782f713 --- /dev/null +++ b/src/services/log-content.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, test } from "bun:test"; +import { shouldLogMaskedContent } from "./log-content"; + +describe("shouldLogMaskedContent", () => { + // With action "mask", maskedContent has both PII and secrets replaced by + // placeholders, e.g. "My key is [API_KEY_SK_1] and email [[EMAIL_ADDRESS_1]]". + // Storing it is safe even when secrets were detected (issue #91). + const maskedWithSecret = "My key is [API_KEY_SK_1] and email [[EMAIL_ADDRESS_1]]"; + const maskedPiiOnly = "Email [[EMAIL_ADDRESS_1]]"; + + test("logs masked content when secrets were detected and masked", () => { + expect( + shouldLogMaskedContent({ + maskedContent: maskedWithSecret, + logMaskedContent: true, + secretsDetected: true, + secretsMasked: true, + }), + ).toBe(true); + }); + + test("logs masked content when only PII was detected", () => { + expect( + shouldLogMaskedContent({ + maskedContent: maskedPiiOnly, + logMaskedContent: true, + secretsDetected: false, + }), + ).toBe(true); + }); + + test("does not log when log_masked_content is false", () => { + expect( + shouldLogMaskedContent({ + maskedContent: maskedWithSecret, + logMaskedContent: false, + secretsDetected: true, + secretsMasked: true, + }), + ).toBe(false); + expect( + shouldLogMaskedContent({ + maskedContent: maskedPiiOnly, + logMaskedContent: false, + }), + ).toBe(false); + }); + + test("does not log when secrets were detected but not masked (route_local)", () => { + // Route mode with action "route_local" leaves secrets raw for the trusted + // local provider, so the content may contain actual secret material. + expect( + shouldLogMaskedContent({ + maskedContent: "My key is sk-live-actual-secret and email [[EMAIL_ADDRESS_1]]", + logMaskedContent: true, + secretsDetected: true, + secretsMasked: false, + }), + ).toBe(false); + }); + + test("does not log when there is no masked content", () => { + expect( + shouldLogMaskedContent({ + maskedContent: undefined, + logMaskedContent: true, + }), + ).toBe(false); + }); +}); diff --git a/src/services/log-content.ts b/src/services/log-content.ts new file mode 100644 index 0000000..10233f8 --- /dev/null +++ b/src/services/log-content.ts @@ -0,0 +1,26 @@ +export interface LogContentDecision { + maskedContent?: string; + logMaskedContent: boolean; + secretsDetected?: boolean; + secretsMasked?: boolean; +} + +/** + * Decide whether masked content should be persisted to the request log. + * + * When secrets_detection.action is "mask" (the default), maskedContent has both + * PII and secrets replaced by placeholders (e.g. "[API_KEY_SK_1]", + * "[[EMAIL_ADDRESS_1]]") by the time it reaches the logger, so it is safe to + * store even when secrets were detected — gating follows log_masked_content. + * + * The exception is route mode with action "route_local": secrets are detected + * but intentionally left unmasked for the trusted local provider, so the + * content may contain raw secret material and must never be persisted. + */ +export function shouldLogMaskedContent(decision: LogContentDecision): boolean { + const { maskedContent, logMaskedContent, secretsDetected, secretsMasked } = decision; + if (!maskedContent || !logMaskedContent) return false; + // Detected but unmasked secrets (action: route_local) are still raw in the content + if (secretsDetected && !secretsMasked) return false; + return true; +} diff --git a/src/services/logger.ts b/src/services/logger.ts index c3c7ccc..0a41130 100644 --- a/src/services/logger.ts +++ b/src/services/logger.ts @@ -1,6 +1,7 @@ import { Database } from "bun:sqlite"; import { mkdirSync } from "node:fs"; import { getConfig } from "../config"; +import { shouldLogMaskedContent } from "./log-content"; export interface RequestLog { id?: number; @@ -302,6 +303,7 @@ export interface RequestLogData { detectedLanguage?: string; maskedContent?: string; secretsDetected?: boolean; + secretsMasked?: boolean; secretsTypes?: string[]; statusCode?: number; errorMessage?: string; @@ -312,9 +314,12 @@ export function logRequest(data: RequestLogData, userAgent: string | null): void const config = getConfig(); const logger = getLogger(); - // Safety: Never log content if secrets were detected - // Even if log_content is true, secrets are never logged - const shouldLogContent = data.maskedContent && !data.secretsDetected; + const shouldLogContent = shouldLogMaskedContent({ + maskedContent: data.maskedContent, + logMaskedContent: config.logging.log_masked_content, + secretsDetected: data.secretsDetected, + secretsMasked: data.secretsMasked, + }); // Only log secret types if configured to do so const shouldLogSecretTypes =