Config: consolidate raw redaction overlap and SecretRef safety

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" } },

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<string, unknown>,
+): value is Record<string, unknown> & { source: string; id: string } {
+  return typeof value.source === "string" && typeof value.id === "string";
+}
+
+function redactSecretRef(
+  value: Record<string, unknown> & { source: string; id: string },
+  values: string[],
+): Record<string, unknown> {
+  const redacted: Record<string, unknown> = { ...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<string, unknown>)) {
+    const obj = value as Record<string, unknown>;
+    // 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<string, unknown>;
+              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<T>(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);
 }