From cbd2e8eea8c142bdf5a17555c5c8a50c599d678e Mon Sep 17 00:00:00 2001 From: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> Date: Mon, 2 Mar 2026 16:06:58 -0600 Subject: [PATCH] Config: consolidate raw redaction overlap and SecretRef safety --- src/config/redact-snapshot.test.ts | 51 +++++++++++++++++++++++-- src/config/redact-snapshot.ts | 61 +++++++++++++++++++++++++++--- 2 files changed, 103 insertions(+), 9 deletions(-) diff --git a/src/config/redact-snapshot.test.ts b/src/config/redact-snapshot.test.ts index cec50573a..3abaea37f 100644 --- a/src/config/redact-snapshot.test.ts +++ b/src/config/redact-snapshot.test.ts @@ -263,19 +263,64 @@ describe("redactConfigSnapshot", () => { }, }; const snapshot = makeSnapshot(config, JSON.stringify(config)); - - const result = redactConfigSnapshot(snapshot); + const result = redactConfigSnapshot(snapshot, mainSchemaHints); const parsed: { gateway?: { mode?: string; auth?: { password?: string } }; } = JSON5.parse(result.raw ?? "{}"); expect(parsed.gateway?.mode).toBe("local"); expect(parsed.gateway?.auth?.password).toBe(REDACTED_SENTINEL); - const restored = restoreRedactedValues(parsed, snapshot.config, mainSchemaHints); expect(restored.gateway.mode).toBe("local"); expect(restored.gateway.auth.password).toBe("local"); }); + it("preserves SecretRef structural fields while redacting SecretRef id", () => { + const config = { + models: { + providers: { + default: { + apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + baseUrl: "https://api.openai.com", + }, + }, + }, + }; + const snapshot = makeSnapshot(config, JSON.stringify(config, null, 2)); + const result = redactConfigSnapshot(snapshot, mainSchemaHints); + expect(result.raw).not.toContain("OPENAI_API_KEY"); + const parsed: { + models?: { providers?: { default?: { apiKey?: { source?: string; provider?: string } } } }; + } = JSON5.parse(result.raw ?? "{}"); + expect(parsed.models?.providers?.default?.apiKey?.source).toBe("env"); + expect(parsed.models?.providers?.default?.apiKey?.provider).toBe("default"); + const restored = restoreRedactedValues(parsed, snapshot.config, mainSchemaHints); + expect(restored).toEqual(snapshot.config); + }); + + it("handles overlap fallback and SecretRef in the same snapshot", () => { + const config = { + gateway: { mode: "default", auth: { password: "default" } }, + models: { + providers: { + default: { + apiKey: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, + baseUrl: "https://api.openai.com", + }, + }, + }, + }; + const snapshot = makeSnapshot(config, JSON.stringify(config, null, 2)); + const result = redactConfigSnapshot(snapshot, mainSchemaHints); + const parsed = JSON5.parse(result.raw ?? "{}"); + expect(parsed.gateway?.mode).toBe("default"); + expect(parsed.gateway?.auth?.password).toBe(REDACTED_SENTINEL); + expect(parsed.models?.providers?.default?.apiKey?.source).toBe("env"); + expect(parsed.models?.providers?.default?.apiKey?.provider).toBe("default"); + expect(result.raw).not.toContain("OPENAI_API_KEY"); + const restored = restoreRedactedValues(parsed, snapshot.config, mainSchemaHints); + expect(restored).toEqual(snapshot.config); + }); + it("redacts parsed and resolved objects", () => { const snapshot = makeSnapshot({ channels: { discord: { token: "MTIzNDU2Nzg5MDEyMzQ1Njc4.GaBcDe.FgH" } }, diff --git a/src/config/redact-snapshot.ts b/src/config/redact-snapshot.ts index eea34c20b..d600327cb 100644 --- a/src/config/redact-snapshot.ts +++ b/src/config/redact-snapshot.ts @@ -1,3 +1,4 @@ +import { isDeepStrictEqual } from "node:util"; import JSON5 from "json5"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { isSensitiveConfigPath, type ConfigUiHints } from "./schema.hints.js"; @@ -23,6 +24,24 @@ function isWholeObjectSensitivePath(path: string): boolean { return lowered.endsWith("serviceaccount") || lowered.endsWith("serviceaccountref"); } +function isSecretRefShape( + value: Record, +): value is Record & { source: string; id: string } { + return typeof value.source === "string" && typeof value.id === "string"; +} + +function redactSecretRef( + value: Record & { source: string; id: string }, + values: string[], +): Record { + const redacted: Record = { ...value }; + if (!isEnvVarPlaceholder(value.id)) { + values.push(value.id); + redacted.id = REDACTED_SENTINEL; + } + return redacted; +} + function collectSensitiveStrings(value: unknown, values: string[]): void { if (typeof value === "string") { if (!isEnvVarPlaceholder(value)) { @@ -37,7 +56,16 @@ function collectSensitiveStrings(value: unknown, values: string[]): void { return; } if (value && typeof value === "object") { - for (const item of Object.values(value as Record)) { + const obj = value as Record; + // SecretRef objects include structural fields like source/provider that are + // not secret material and may appear widely in config text. + if (isSecretRefShape(obj)) { + if (!isEnvVarPlaceholder(obj.id)) { + values.push(obj.id); + } + return; + } + for (const item of Object.values(obj)) { collectSensitiveStrings(item, values); } } @@ -176,8 +204,13 @@ function redactObjectWithLookup( values.push(value); } else if (typeof value === "object" && value !== null) { if (hints[candidate]?.sensitive === true && !Array.isArray(value)) { - collectSensitiveStrings(value, values); - result[key] = REDACTED_SENTINEL; + const objectValue = value as Record; + if (isSecretRefShape(objectValue)) { + result[key] = redactSecretRef(objectValue, values); + } else { + collectSensitiveStrings(objectValue, values); + result[key] = REDACTED_SENTINEL; + } } else { result[key] = redactObjectWithLookup(value, lookup, candidate, values, hints); } @@ -295,6 +328,18 @@ function redactRawText(raw: string, config: unknown, hints?: ConfigUiHints): str return result; } +let suppressRestoreWarnings = false; + +function withRestoreWarningsSuppressed(fn: () => T): T { + const prev = suppressRestoreWarnings; + suppressRestoreWarnings = true; + try { + return fn(); + } finally { + suppressRestoreWarnings = prev; + } +} + function shouldFallbackToStructuredRawRedaction(params: { redactedRaw: string; originalConfig: unknown; @@ -302,11 +347,13 @@ function shouldFallbackToStructuredRawRedaction(params: { }): boolean { try { const parsed = JSON5.parse(params.redactedRaw); - const restored = restoreRedactedValues(parsed, params.originalConfig, params.hints); + const restored = withRestoreWarningsSuppressed(() => + restoreRedactedValues(parsed, params.originalConfig, params.hints), + ); if (!restored.ok) { return true; } - return JSON.stringify(restored.result) !== JSON.stringify(params.originalConfig); + return !isDeepStrictEqual(restored.result, params.originalConfig); } catch { return true; } @@ -448,7 +495,9 @@ function restoreOriginalValueOrThrow(params: { if (params.key in params.original) { return params.original[params.key]; } - log.warn(`Cannot un-redact config key ${params.path} as it doesn't have any value`); + if (!suppressRestoreWarnings) { + log.warn(`Cannot un-redact config key ${params.path} as it doesn't have any value`); + } throw new RedactionError(params.path); }