diff --git a/apps/server/src/wsServer.ts b/apps/server/src/wsServer.ts index 47474d55..8697220d 100644 --- a/apps/server/src/wsServer.ts +++ b/apps/server/src/wsServer.ts @@ -90,6 +90,7 @@ import { expandHomePath } from "./os-jank.ts"; import { makeServerPushBus } from "./wsServer/pushBus.ts"; import { makeServerReadiness } from "./wsServer/readiness.ts"; import { decodeJsonResult, formatSchemaError } from "@okcode/shared/schemaJson"; +import { redactSensitiveText, redactSensitiveValue } from "@okcode/shared/redaction"; import { PrReview } from "./prReview/Services/PrReview.ts"; import { GitHub } from "./github/Services/GitHub.ts"; import { GitActionExecutionError } from "./git/Errors.ts"; @@ -1685,17 +1686,17 @@ export const createServer = Effect.fn(function* (): Effect.fn.Return< squashed instanceof GitActionExecutionError ) { return { - message: squashed.failure.summary, + message: redactSensitiveText(squashed.failure.summary), code: "git_action_failed", - data: squashed.failure, + data: redactSensitiveValue(squashed.failure), }; } if (squashed instanceof Error) { - return { message: squashed.message }; + return { message: redactSensitiveText(squashed.message) }; } - return { message: Cause.pretty(cause) }; + return { message: redactSensitiveText(Cause.pretty(cause)) }; }; const handleMessage = Effect.fnUntraced(function* (ws: WebSocket, raw: unknown) { diff --git a/apps/web/src/components/chat/providerStatusPresentation.ts b/apps/web/src/components/chat/providerStatusPresentation.ts index 48fb0330..a35e04b4 100644 --- a/apps/web/src/components/chat/providerStatusPresentation.ts +++ b/apps/web/src/components/chat/providerStatusPresentation.ts @@ -1,4 +1,5 @@ import { type ServerProviderStatus } from "@okcode/contracts"; +import { redactSensitiveText } from "@okcode/shared/redaction"; export type ProviderSetupPhase = "install" | "authenticate" | "verify" | "ready"; @@ -43,7 +44,7 @@ export function getProviderStatusHeading(status: ServerProviderStatus): string { export function getProviderStatusDescription(status: ServerProviderStatus): string { if (status.message) { - return status.message; + return redactSensitiveText(status.message); } const label = getProviderLabel(status.provider); diff --git a/apps/web/src/components/chat/threadError.test.ts b/apps/web/src/components/chat/threadError.test.ts index bfb3e4d6..17c8903e 100644 --- a/apps/web/src/components/chat/threadError.test.ts +++ b/apps/web/src/components/chat/threadError.test.ts @@ -23,6 +23,19 @@ describe("humanizeThreadError", () => { }); }); + it("redacts secret-like values before presenting thread errors", () => { + expect( + humanizeThreadError( + "Git command failed in GitCore.createWorktree: OPENAI_API_KEY=sk-proj-secret (/repo) - token=abc123", + ), + ).toEqual({ + title: "Worktree thread could not start", + description: "token=[REDACTED]", + technicalDetails: + "Git command failed in GitCore.createWorktree: OPENAI_API_KEY=[REDACTED] (/repo) - token=[REDACTED]", + }); + }); + it("detects provider authentication failures", () => { expect( isAuthenticationThreadError( diff --git a/apps/web/src/components/chat/threadError.ts b/apps/web/src/components/chat/threadError.ts index 5b4ab998..f001b731 100644 --- a/apps/web/src/components/chat/threadError.ts +++ b/apps/web/src/components/chat/threadError.ts @@ -1,3 +1,5 @@ +import { redactSensitiveText } from "@okcode/shared/redaction"; + export interface ThreadErrorPresentation { title: string | null; description: string; @@ -36,7 +38,7 @@ export function isAuthenticationThreadError(error: string | null | undefined): b } export function humanizeThreadError(error: string): ThreadErrorPresentation { - const trimmed = error.trim(); + const trimmed = redactSensitiveText(error).trim(); const worktreeDetail = extractWorktreeDetail(trimmed); if (worktreeDetail) { return { diff --git a/apps/web/src/wsTransport.test.ts b/apps/web/src/wsTransport.test.ts index 1561b712..999cc1c3 100644 --- a/apps/web/src/wsTransport.test.ts +++ b/apps/web/src/wsTransport.test.ts @@ -180,6 +180,49 @@ describe("WsTransport", () => { transport.dispose(); }); + it("redacts secret-like values from rejected websocket errors", async () => { + const transport = new WsTransport("ws://localhost:3020"); + const socket = getSocket(); + socket.open(); + + const requestPromise = transport.request("git.runStackedAction", { cwd: "/repo" }); + const sent = socket.sent.at(-1); + if (!sent) { + throw new Error("Expected request envelope to be sent"); + } + + const requestEnvelope = JSON.parse(sent) as { id: string }; + socket.serverMessage( + JSON.stringify({ + id: requestEnvelope.id, + error: { + message: "Push failed for sk-proj-secret", + code: "git_action_failed", + data: { + code: "unknown", + phase: "push", + title: "Push failed", + summary: "Push failed for sk-proj-secret", + detail: "token=abc123 OPENAI_API_KEY=sk-proj-secret", + nextSteps: ["Unset OPENAI_API_KEY=sk-proj-secret"], + }, + }, + }), + ); + + await expect(requestPromise).rejects.toMatchObject({ + message: "Push failed for [REDACTED]", + code: "git_action_failed", + data: { + summary: "Push failed for [REDACTED]", + detail: "token=[REDACTED] OPENAI_API_KEY=[REDACTED]", + nextSteps: ["Unset OPENAI_API_KEY=[REDACTED]"], + }, + }); + + transport.dispose(); + }); + it("drops malformed envelopes without crashing transport", () => { const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); const transport = new WsTransport("ws://localhost:3020"); diff --git a/apps/web/src/wsTransport.ts b/apps/web/src/wsTransport.ts index 92d0f667..74498fec 100644 --- a/apps/web/src/wsTransport.ts +++ b/apps/web/src/wsTransport.ts @@ -7,6 +7,7 @@ import { type WsResponse as WsResponseMessage, WsResponse as WsResponseSchema, } from "@okcode/contracts"; +import { redactSensitiveText, redactSensitiveValue } from "@okcode/shared/redaction"; import { decodeUnknownJsonResult, formatSchemaError } from "@okcode/shared/schemaJson"; import { Result, Schema } from "effect"; import { resolveRuntimeWsUrl } from "./lib/runtimeBridge"; @@ -72,10 +73,10 @@ export class WsRequestError extends Error { readonly data: T | undefined; constructor(input: WebSocketErrorPayload) { - super(input.message); + super(redactSensitiveText(input.message)); this.name = "WsRequestError"; this.code = input.code; - this.data = input.data as T | undefined; + this.data = redactSensitiveValue(input.data) as T | undefined; } } diff --git a/packages/shared/package.json b/packages/shared/package.json index 01f71adc..243cf208 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -51,6 +51,10 @@ "./brand": { "types": "./src/brand.ts", "import": "./src/brand.ts" + }, + "./redaction": { + "types": "./src/redaction.ts", + "import": "./src/redaction.ts" } }, "scripts": { diff --git a/packages/shared/src/redaction.test.ts b/packages/shared/src/redaction.test.ts new file mode 100644 index 00000000..aa12372d --- /dev/null +++ b/packages/shared/src/redaction.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "vitest"; + +import { redactSensitiveText, redactSensitiveValue } from "./redaction"; + +describe("redactSensitiveText", () => { + it("redacts OpenAI-style secret keys", () => { + expect(redactSensitiveText("OpenAI failed with sk-proj-abc123_secret-token")) + .toBe("OpenAI failed with [REDACTED]"); + }); + + it("redacts environment variable assignments", () => { + expect( + redactSensitiveText( + "Command failed with OPENAI_API_KEY=sk-proj-abc123 SECRET_TOKEN=hunter2 PATH=/tmp/bin", + ), + ).toBe( + "Command failed with OPENAI_API_KEY=[REDACTED] SECRET_TOKEN=[REDACTED] PATH=[REDACTED]", + ); + }); + + it("redacts sensitive JSON-like fields and query params", () => { + expect( + redactSensitiveText( + 'Request failed: {"token":"abc123","password":"hunter2"} https://x.test?token=abc123&ok=1', + ), + ).toBe( + 'Request failed: {"token":"[REDACTED]","password":"[REDACTED]"} https://x.test?token=[REDACTED]&ok=1', + ); + }); +}); + +describe("redactSensitiveValue", () => { + it("redacts nested structured payloads", () => { + expect( + redactSensitiveValue({ + summary: "Push failed for sk-proj-abc123", + nextSteps: ["Unset OPENAI_API_KEY=sk-proj-abc123"], + nested: { + detail: "Authorization: Bearer topsecret", + }, + }), + ).toEqual({ + summary: "Push failed for [REDACTED]", + nextSteps: ["Unset OPENAI_API_KEY=[REDACTED]"], + nested: { + detail: "Authorization: Bearer [REDACTED]", + }, + }); + }); +}); diff --git a/packages/shared/src/redaction.ts b/packages/shared/src/redaction.ts new file mode 100644 index 00000000..2ea30dfd --- /dev/null +++ b/packages/shared/src/redaction.ts @@ -0,0 +1,50 @@ +const REDACTED = "[REDACTED]"; + +const SECRET_PREFIX_PATTERN = /\bsk-[A-Za-z0-9][A-Za-z0-9_-]*\b/g; +const BEARER_TOKEN_PATTERN = /\b(Bearer\s+)([^\s,;]+)/gi; +const SENSITIVE_QUERY_PARAM_PATTERN = + /([?&](?:access[_-]?token|api[_-]?key|auth(?:orization)?|client[_-]?secret|password|refresh[_-]?token|secret|session[_-]?token|token)=)([^&#\s]+)/gi; +const SENSITIVE_FIELD_PATTERN = + /((?:"|')?(?:access[_-]?token|api[_-]?key|auth(?:orization)?|client[_-]?secret|password|refresh[_-]?token|secret|session[_-]?token|token)(?:"|')?\s*[:=]\s*)(["'`]?)([^"'`\s,}]+)(\2)/gi; +const PROCESS_ENV_PATTERN = + /\b((?:process\.)?env\.[A-Za-z_][A-Za-z0-9_]*\s*(?:=|:)\s*)(["'`]?)([^"'`\s,}]+)(\2)/g; +const ENV_ASSIGNMENT_PATTERN = + /\b([A-Z][A-Z0-9_]{1,63}\s*=\s*)(["'`]?)([^"'`\s]+)(\2)/g; + +function isPlainObject(value: unknown): value is Record { + if (typeof value !== "object" || value === null) { + return false; + } + const prototype = Object.getPrototypeOf(value); + return prototype === Object.prototype || prototype === null; +} + +export function redactSensitiveText(text: string): string { + return text + .replace(SECRET_PREFIX_PATTERN, REDACTED) + .replace(BEARER_TOKEN_PATTERN, `$1${REDACTED}`) + .replace(SENSITIVE_QUERY_PARAM_PATTERN, `$1${REDACTED}`) + .replace(SENSITIVE_FIELD_PATTERN, `$1$2${REDACTED}$4`) + .replace(PROCESS_ENV_PATTERN, `$1$2${REDACTED}$4`) + .replace(ENV_ASSIGNMENT_PATTERN, `$1$2${REDACTED}$4`); +} + +export function redactSensitiveValue(value: T): T { + if (typeof value === "string") { + return redactSensitiveText(value) as T; + } + + if (Array.isArray(value)) { + return value.map((item) => redactSensitiveValue(item)) as T; + } + + if (isPlainObject(value)) { + const entries = Object.entries(value).map(([key, entryValue]) => [ + key, + redactSensitiveValue(entryValue), + ]); + return Object.fromEntries(entries) as T; + } + + return value; +}