fix(security): sanitize credential test logs

src/main/ipc-credentials-handlers.ts

@@ -36,6 +36,87 @@ function describeError(error: unknown): string {
   return error instanceof Error ? error.message : String(error);
 }
 
+function isSafeLogToken(value: unknown): value is string {
+  return typeof value === "string" && /^[A-Za-z0-9._:-]{1,160}$/.test(value);
+}
+
+function safeLogToken(value: unknown, fallback: string): string {
+  return isSafeLogToken(value) ? value : fallback;
+}
+
+function extractHeader(headers: unknown, name: string): unknown {
+  if (!headers) return undefined;
+  if (typeof (headers as { get?: unknown }).get === "function") {
+    return (headers as { get: (key: string) => unknown }).get(name);
+  }
+  if (typeof headers === "object") {
+    const record = headers as Record<string, unknown>;
+    return record[name] ?? record[name.toLowerCase()] ?? record[name.toUpperCase()];
+  }
+  return undefined;
+}
+
+function readObjectField(source: unknown, field: string): unknown {
+  if (!source || typeof source !== "object") return undefined;
+  return (source as Record<string, unknown>)[field];
+}
+
+function extractCredentialErrorCode(error: unknown, fallback: string): string {
+  const candidates = [
+    readObjectField(error, "code"),
+    readObjectField(error, "errorCode"),
+    readObjectField(error, "status"),
+    readObjectField(error, "statusCode"),
+    error instanceof Error ? error.name : undefined,
+  ];
+  for (const value of candidates) {
+    if (isSafeLogToken(value)) return value;
+    if (typeof value === "number" && Number.isInteger(value)) return String(value);
+  }
+  return fallback;
+}
+
+function extractCredentialRequestId(error: unknown): string | null {
+  const response = readObjectField(error, "response");
+  const metadata = readObjectField(error, "$metadata");
+  const candidates = [
+    readObjectField(error, "requestId"),
+    readObjectField(error, "request_id"),
+    readObjectField(error, "x-request-id"),
+    readObjectField(metadata, "requestId"),
+    extractHeader(readObjectField(error, "headers"), "x-request-id"),
+    extractHeader(readObjectField(response, "headers"), "x-request-id"),
+    extractHeader(readObjectField(response, "headers"), "x-amzn-requestid"),
+  ];
+  for (const value of candidates) {
+    if (isSafeLogToken(value)) return value;
+  }
+  return null;
+}
+
+function credentialLogLine(action: string, input: {
+  connectorId: string;
+  provider: string | undefined;
+  code: string;
+  requestId: string | null;
+}): string {
+  const fields = [
+    `connector=${safeLogToken(input.connectorId, "invalid")}`,
+    `provider=${safeLogToken(input.provider, "unknown")}`,
+    `code=${safeLogToken(input.code, "unknown")}`,
+  ];
+  if (input.requestId) fields.push(`requestId=${safeLogToken(input.requestId, "invalid")}`);
+  return `[credentials] ${action}: ${fields.join(" ")}`;
+}
+
+function getConnectorProviderForLog(connectorId: string): string | undefined {
+  try {
+    return getConnectorDefinition(connectorId).provider;
+  } catch {
+    return undefined;
+  }
+}
+
 interface CredentialHandlerDeps {
   store: CredentialStore;
   resolver: CredentialResolver;
@@ -95,7 +176,12 @@ export function registerCredentialIpcHandlers(deps: CredentialHandlerDeps): () =
         message: `save failed for ${parsed.data.connectorId}`,
         context: { connectorId: parsed.data.connectorId },
       });
-      deps.logger.error(`[${err.code}] ${err.message}: ${describeError(error)}`);
+      deps.logger.error(credentialLogLine("save failed", {
+        connectorId: parsed.data.connectorId,
+        provider: definition.provider,
+        code: err.code,
+        requestId: extractCredentialRequestId(error),
+      }));
       return deps.fail("SAVE_FAILED", "Could not save credential. Check application logs for details.");
     }
   });
@@ -115,7 +201,12 @@ export function registerCredentialIpcHandlers(deps: CredentialHandlerDeps): () =
         message: `remove failed for ${parsed.data.connectorId}`,
         context: { connectorId: parsed.data.connectorId },
       });
-      deps.logger.error(`[${err.code}] ${err.message}: ${describeError(error)}`);
+      deps.logger.error(credentialLogLine("remove failed", {
+        connectorId: parsed.data.connectorId,
+        provider: getConnectorProviderForLog(parsed.data.connectorId),
+        code: err.code,
+        requestId: extractCredentialRequestId(error),
+      }));
       return deps.fail("REMOVE_FAILED", "Could not remove credential. Check application logs for details.");
     }
   });
@@ -133,7 +224,12 @@ export function registerCredentialIpcHandlers(deps: CredentialHandlerDeps): () =
         message: `test failed lookup for ${parsed.data.connectorId}`,
         context: { connectorId: parsed.data.connectorId },
       });
-      deps.logger.error(`[${err.code}] ${err.message}: ${describeError(error)}`);
+      deps.logger.error(credentialLogLine("test failed", {
+        connectorId: parsed.data.connectorId,
+        provider: undefined,
+        code: err.code,
+        requestId: extractCredentialRequestId(error),
+      }));
       return deps.fail("UNKNOWN_CONNECTOR", "Unknown connector.");
     }
     let resolved;
@@ -147,7 +243,12 @@ export function registerCredentialIpcHandlers(deps: CredentialHandlerDeps): () =
         context: { connectorId: parsed.data.connectorId },
       });
       const message = describeError(error);
-      deps.logger.error(`[${err.code}] ${err.message}: ${message}`);
+      deps.logger.error(credentialLogLine("test failed", {
+        connectorId: parsed.data.connectorId,
+        provider: definition.provider,
+        code: extractCredentialErrorCode(error, err.code),
+        requestId: extractCredentialRequestId(error),
+      }));
       return ok({ ok: false, message: redactSecrets(message) });
     }
 
@@ -181,13 +282,23 @@ export function registerCredentialIpcHandlers(deps: CredentialHandlerDeps): () =
         const final = await stream.result();
         if (final.stopReason === "error" || final.stopReason === "aborted") {
           const message = final.errorMessage ?? `Provider returned ${final.stopReason}.`;
-          deps.logger.warn(`[credentials] test failed for ${parsed.data.connectorId}: ${message}`);
+          deps.logger.warn(credentialLogLine("test failed", {
+            connectorId: parsed.data.connectorId,
+            provider,
+            code: final.stopReason === "error" ? "stream_error" : "stream_aborted",
+            requestId: null,
+          }));
           return ok({ ok: false, message: redactSecrets(message) });
         }
         return ok({ ok: true, message: `OK (${model.id})` });
       } catch (error) {
         const message = describeError(error);
-        deps.logger.error(`[credentials] test threw for ${parsed.data.connectorId}: ${message}`);
+        deps.logger.error(credentialLogLine("test failed", {
+          connectorId: parsed.data.connectorId,
+          provider: definition.provider,
+          code: extractCredentialErrorCode(error, "stream_exception"),
+          requestId: extractCredentialRequestId(error),
+        }));
         return ok({ ok: false, message: redactSecrets(message) });
       }
     };

tests/unit/ipc-credentials-handlers.test.ts

@@ -0,0 +1,256 @@
+import type { IpcMainInvokeEvent } from "electron";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import type { DesktopResult } from "../../src/shared/contracts";
+import { IPC_CHANNELS } from "../../src/shared/contracts";
+import type { CredentialResolver } from "../../src/main/credentials/credential-resolver";
+import type { CredentialStore } from "../../src/main/credentials/credential-store";
+
+const streamMocks = vi.hoisted(() => ({
+  safeStreamSimpleWithEnv: vi.fn(),
+}));
+
+vi.mock("electron", () => ({
+  ipcMain: {
+    removeHandler: vi.fn(),
+  },
+}));
+
+vi.mock("../../src/main/worker/runtime/pi-safe-stream", () => ({
+  safeStreamSimpleWithEnv: streamMocks.safeStreamSimpleWithEnv,
+}));
+
+import { registerCredentialIpcHandlers } from "../../src/main/ipc-credentials-handlers";
+
+type IpcHandler = (
+  event: IpcMainInvokeEvent,
+  ...args: unknown[]
+) => Promise<DesktopResult<unknown>> | DesktopResult<unknown>;
+
+function createDeps(overrides: { resolver?: Partial<CredentialResolver>; store?: Partial<CredentialStore> } = {}) {
+  const handlers = new Map<string, IpcHandler>();
+  const logger = {
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn(),
+  };
+  const resolver = {
+    resolve: vi.fn(async () => ({
+      apiKey: "sk-proj-testcredentialsecret0123456789",
+    })),
+    ...(overrides.resolver ?? {}),
+  } as unknown as CredentialResolver;
+
+  registerCredentialIpcHandlers({
+    store: {
+      list: vi.fn(() => []),
+      save: vi.fn(),
+      remove: vi.fn(),
+      ...(overrides.store ?? {}),
+    } as unknown as CredentialStore,
+    resolver,
+    emitEvent: vi.fn(),
+    refreshConnectorHealth: vi.fn(async () => {}),
+    registerHandle: (channel, handler) => {
+      handlers.set(channel, handler);
+    },
+    fail: (code, message) => ({ ok: false, error: { code, message } }),
+    logger,
+  });
+
+  const handler = handlers.get(IPC_CHANNELS.credentialsTest);
+  if (!handler) {
+    throw new Error("credentialsTest handler was not registered.");
+  }
+  return { handler, handlers, logger };
+}
+
+async function runCredentialTest(handler: IpcHandler) {
+  return handler(null as unknown as IpcMainInvokeEvent, { connectorId: "fireworks" }) as Promise<DesktopResult<{
+    ok: boolean;
+    message: string;
+  }>>;
+}
+
+function requireHandler(handlers: Map<string, IpcHandler>, channel: string) {
+  const handler = handlers.get(channel);
+  if (!handler) {
+    throw new Error(`${channel} handler was not registered.`);
+  }
+  return handler;
+}
+
+function joinLogCalls(mock: ReturnType<typeof vi.fn>) {
+  return mock.mock.calls.flat().map(String).join(" ");
+}
+
+describe("credential IPC handler logging", () => {
+  beforeEach(() => {
+    streamMocks.safeStreamSimpleWithEnv.mockReset();
+  });
+
+  it("redacts resolver failures for the UI and logs only safe credential test metadata", async () => {
+    const secret = "sk-proj-resolversecret012345678901234";
+    const error = Object.assign(
+      new Error(`failed for ${secret} at https://api.fireworks.ai/inference/v1`),
+      { code: "CREDENTIAL_INVALID", $metadata: { requestId: "req_resolver_123" } },
+    );
+    const { handler, logger } = createDeps({
+      resolver: {
+        resolve: vi.fn(async () => {
+          throw error;
+        }),
+      },
+    });
+
+    const result = await runCredentialTest(handler);
+    const log = joinLogCalls(logger.error);
+
+    expect(result).toEqual({
+      ok: true,
+      data: {
+        ok: false,
+        message: "failed for sk-proj-[REDACTED] at https://api.fireworks.ai/inference/v1",
+      },
+    });
+    expect(log).toBe("[credentials] test failed: connector=fireworks provider=fireworks code=CREDENTIAL_INVALID requestId=req_resolver_123");
+    expect(log).not.toContain(secret);
+    expect(log).not.toContain("api.fireworks.ai");
+  });
+
+  it("keeps stream error messages out of credential test logs", async () => {
+    const secret = "Bearer eyJhbGciOiJIUzI1NiJ9.secret.payload";
+    streamMocks.safeStreamSimpleWithEnv.mockReturnValueOnce({
+      result: vi.fn(async () => ({
+        stopReason: "error",
+        errorMessage: `provider rejected ${secret} for https://api.fireworks.ai/inference/v1`,
+      })),
+    });
+    const { handler, logger } = createDeps();
+
+    const result = await runCredentialTest(handler);
+    const log = joinLogCalls(logger.warn);
+
+    expect(result).toEqual({
+      ok: true,
+      data: {
+        ok: false,
+        message: "provider rejected Bearer [REDACTED] for https://api.fireworks.ai/inference/v1",
+      },
+    });
+    expect(log).toBe("[credentials] test failed: connector=fireworks provider=fireworks code=stream_error");
+    expect(log).not.toContain(secret);
+    expect(log).not.toContain("api.fireworks.ai");
+  });
+
+  it("logs safe code and request id when credential test streaming throws", async () => {
+    const secret = "AKIA1234567890ABCDEF";
+    const error = Object.assign(
+      new Error(`upstream threw ${secret} at https://bedrock-runtime.us-east-1.amazonaws.com`),
+      {
+        code: "AccessDeniedException",
+        response: { headers: new Map([["x-request-id", "req_stream_456"]]) },
+      },
+    );
+    streamMocks.safeStreamSimpleWithEnv.mockReturnValueOnce({
+      result: vi.fn(async () => {
+        throw error;
+      }),
+    });
+    const { handler, logger } = createDeps();
+
+    const result = await runCredentialTest(handler);
+    const log = joinLogCalls(logger.error);
+
+    expect(result).toEqual({
+      ok: true,
+      data: {
+        ok: false,
+        message: "upstream threw AKIA[REDACTED] at https://bedrock-runtime.us-east-1.amazonaws.com",
+      },
+    });
+    expect(log).toBe("[credentials] test failed: connector=fireworks provider=fireworks code=AccessDeniedException requestId=req_stream_456");
+    expect(log).not.toContain(secret);
+    expect(log).not.toContain("bedrock-runtime.us-east-1.amazonaws.com");
+  });
+
+  it("keeps raw save failure messages out of credential logs", async () => {
+    const secret = "sk-proj-savesecret012345678901234";
+    const error = Object.assign(
+      new Error(`save failed for ${secret} at https://api.fireworks.ai/account/acme`),
+      { requestId: "req_save_789" },
+    );
+    const { handlers, logger } = createDeps({
+      store: {
+        save: vi.fn(() => {
+          throw error;
+        }),
+      },
+    });
+    const saveHandler = requireHandler(handlers, IPC_CHANNELS.credentialsSave);
+
+    const result = await saveHandler(null as unknown as IpcMainInvokeEvent, {
+      connectorId: "fireworks",
+      payload: {
+        kind: "api_key",
+        value: { source: "literal", value: secret },
+      },
+    });
+    const log = joinLogCalls(logger.error);
+
+    expect(result).toEqual({
+      ok: false,
+      error: {
+        code: "SAVE_FAILED",
+        message: "Could not save credential. Check application logs for details.",
+      },
+    });
+    expect(log).toBe("[credentials] save failed: connector=fireworks provider=fireworks code=credentials.save.failed requestId=req_save_789");
+    expect(log).not.toContain(secret);
+    expect(log).not.toContain("api.fireworks.ai");
+  });
+
+  it("sanitizes connector ids on remove failure logs", async () => {
+    const connectorId = "unknown\nhttps://api.fireworks.ai/account/sk-proj-remove012345678901234";
+    const { handlers, logger } = createDeps({
+      store: {
+        remove: vi.fn(() => {
+          throw new Error("remove failed for https://api.fireworks.ai/account/sk-proj-remove012345678901234");
+        }),
+      },
+    });
+    const removeHandler = requireHandler(handlers, IPC_CHANNELS.credentialsRemove);
+
+    const result = await removeHandler(null as unknown as IpcMainInvokeEvent, { connectorId });
+    const log = joinLogCalls(logger.error);
+
+    expect(result).toEqual({
+      ok: false,
+      error: {
+        code: "REMOVE_FAILED",
+        message: "Could not remove credential. Check application logs for details.",
+      },
+    });
+    expect(log).toBe("[credentials] remove failed: connector=invalid provider=unknown code=credentials.remove.failed");
+    expect(log).not.toContain("api.fireworks.ai");
+    expect(log).not.toContain("sk-proj-remove012345678901234");
+  });
+
+  it("sanitizes connector ids on unknown connector test logs", async () => {
+    const connectorId = "missing\nhttps://api.fireworks.ai/account/sk-proj-unknown012345678901234";
+    const { handler, logger } = createDeps();
+
+    const result = await handler(null as unknown as IpcMainInvokeEvent, { connectorId });
+    const log = joinLogCalls(logger.error);
+
+    expect(result).toEqual({
+      ok: false,
+      error: {
+        code: "UNKNOWN_CONNECTOR",
+        message: "Unknown connector.",
+      },
+    });
+    expect(log).toBe("[credentials] test failed: connector=invalid provider=unknown code=credentials.test.unknown_connector");
+    expect(log).not.toContain("api.fireworks.ai");
+    expect(log).not.toContain("sk-proj-unknown012345678901234");
+  });
+});