Skip to content

Commit e06dab0

Browse files
authored
Redact secrets from websocket errors (#384)
- Add shared text/value redaction helpers - Apply redaction to server, transport, and UI error paths - Cover redaction behavior with unit tests
1 parent 6390836 commit e06dab0

9 files changed

Lines changed: 173 additions & 8 deletions

File tree

apps/server/src/wsServer.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ import { expandHomePath } from "./os-jank.ts";
9090
import { makeServerPushBus } from "./wsServer/pushBus.ts";
9191
import { makeServerReadiness } from "./wsServer/readiness.ts";
9292
import { decodeJsonResult, formatSchemaError } from "@okcode/shared/schemaJson";
93+
import { redactSensitiveText, redactSensitiveValue } from "@okcode/shared/redaction";
9394
import { PrReview } from "./prReview/Services/PrReview.ts";
9495
import { GitHub } from "./github/Services/GitHub.ts";
9596
import { GitActionExecutionError } from "./git/Errors.ts";
@@ -1685,17 +1686,17 @@ export const createServer = Effect.fn(function* (): Effect.fn.Return<
16851686
squashed instanceof GitActionExecutionError
16861687
) {
16871688
return {
1688-
message: squashed.failure.summary,
1689+
message: redactSensitiveText(squashed.failure.summary),
16891690
code: "git_action_failed",
1690-
data: squashed.failure,
1691+
data: redactSensitiveValue(squashed.failure),
16911692
};
16921693
}
16931694

16941695
if (squashed instanceof Error) {
1695-
return { message: squashed.message };
1696+
return { message: redactSensitiveText(squashed.message) };
16961697
}
16971698

1698-
return { message: Cause.pretty(cause) };
1699+
return { message: redactSensitiveText(Cause.pretty(cause)) };
16991700
};
17001701

17011702
const handleMessage = Effect.fnUntraced(function* (ws: WebSocket, raw: unknown) {

apps/web/src/components/chat/providerStatusPresentation.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { type ServerProviderStatus } from "@okcode/contracts";
2+
import { redactSensitiveText } from "@okcode/shared/redaction";
23

34
export type ProviderSetupPhase = "install" | "authenticate" | "verify" | "ready";
45

@@ -43,7 +44,7 @@ export function getProviderStatusHeading(status: ServerProviderStatus): string {
4344

4445
export function getProviderStatusDescription(status: ServerProviderStatus): string {
4546
if (status.message) {
46-
return status.message;
47+
return redactSensitiveText(status.message);
4748
}
4849

4950
const label = getProviderLabel(status.provider);

apps/web/src/components/chat/threadError.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,19 @@ describe("humanizeThreadError", () => {
2323
});
2424
});
2525

26+
it("redacts secret-like values before presenting thread errors", () => {
27+
expect(
28+
humanizeThreadError(
29+
"Git command failed in GitCore.createWorktree: OPENAI_API_KEY=sk-proj-secret (/repo) - token=abc123",
30+
),
31+
).toEqual({
32+
title: "Worktree thread could not start",
33+
description: "token=[REDACTED]",
34+
technicalDetails:
35+
"Git command failed in GitCore.createWorktree: OPENAI_API_KEY=[REDACTED] (/repo) - token=[REDACTED]",
36+
});
37+
});
38+
2639
it("detects provider authentication failures", () => {
2740
expect(
2841
isAuthenticationThreadError(

apps/web/src/components/chat/threadError.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { redactSensitiveText } from "@okcode/shared/redaction";
2+
13
export interface ThreadErrorPresentation {
24
title: string | null;
35
description: string;
@@ -36,7 +38,7 @@ export function isAuthenticationThreadError(error: string | null | undefined): b
3638
}
3739

3840
export function humanizeThreadError(error: string): ThreadErrorPresentation {
39-
const trimmed = error.trim();
41+
const trimmed = redactSensitiveText(error).trim();
4042
const worktreeDetail = extractWorktreeDetail(trimmed);
4143
if (worktreeDetail) {
4244
return {

apps/web/src/wsTransport.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,49 @@ describe("WsTransport", () => {
180180
transport.dispose();
181181
});
182182

183+
it("redacts secret-like values from rejected websocket errors", async () => {
184+
const transport = new WsTransport("ws://localhost:3020");
185+
const socket = getSocket();
186+
socket.open();
187+
188+
const requestPromise = transport.request("git.runStackedAction", { cwd: "/repo" });
189+
const sent = socket.sent.at(-1);
190+
if (!sent) {
191+
throw new Error("Expected request envelope to be sent");
192+
}
193+
194+
const requestEnvelope = JSON.parse(sent) as { id: string };
195+
socket.serverMessage(
196+
JSON.stringify({
197+
id: requestEnvelope.id,
198+
error: {
199+
message: "Push failed for sk-proj-secret",
200+
code: "git_action_failed",
201+
data: {
202+
code: "unknown",
203+
phase: "push",
204+
title: "Push failed",
205+
summary: "Push failed for sk-proj-secret",
206+
detail: "token=abc123 OPENAI_API_KEY=sk-proj-secret",
207+
nextSteps: ["Unset OPENAI_API_KEY=sk-proj-secret"],
208+
},
209+
},
210+
}),
211+
);
212+
213+
await expect(requestPromise).rejects.toMatchObject({
214+
message: "Push failed for [REDACTED]",
215+
code: "git_action_failed",
216+
data: {
217+
summary: "Push failed for [REDACTED]",
218+
detail: "token=[REDACTED] OPENAI_API_KEY=[REDACTED]",
219+
nextSteps: ["Unset OPENAI_API_KEY=[REDACTED]"],
220+
},
221+
});
222+
223+
transport.dispose();
224+
});
225+
183226
it("drops malformed envelopes without crashing transport", () => {
184227
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
185228
const transport = new WsTransport("ws://localhost:3020");

apps/web/src/wsTransport.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
type WsResponse as WsResponseMessage,
88
WsResponse as WsResponseSchema,
99
} from "@okcode/contracts";
10+
import { redactSensitiveText, redactSensitiveValue } from "@okcode/shared/redaction";
1011
import { decodeUnknownJsonResult, formatSchemaError } from "@okcode/shared/schemaJson";
1112
import { Result, Schema } from "effect";
1213
import { resolveRuntimeWsUrl } from "./lib/runtimeBridge";
@@ -72,10 +73,10 @@ export class WsRequestError<T = unknown> extends Error {
7273
readonly data: T | undefined;
7374

7475
constructor(input: WebSocketErrorPayload) {
75-
super(input.message);
76+
super(redactSensitiveText(input.message));
7677
this.name = "WsRequestError";
7778
this.code = input.code;
78-
this.data = input.data as T | undefined;
79+
this.data = redactSensitiveValue(input.data) as T | undefined;
7980
}
8081
}
8182

packages/shared/package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@
5151
"./brand": {
5252
"types": "./src/brand.ts",
5353
"import": "./src/brand.ts"
54+
},
55+
"./redaction": {
56+
"types": "./src/redaction.ts",
57+
"import": "./src/redaction.ts"
5458
}
5559
},
5660
"scripts": {
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { describe, expect, it } from "vitest";
2+
3+
import { redactSensitiveText, redactSensitiveValue } from "./redaction";
4+
5+
describe("redactSensitiveText", () => {
6+
it("redacts OpenAI-style secret keys", () => {
7+
expect(redactSensitiveText("OpenAI failed with sk-proj-abc123_secret-token"))
8+
.toBe("OpenAI failed with [REDACTED]");
9+
});
10+
11+
it("redacts environment variable assignments", () => {
12+
expect(
13+
redactSensitiveText(
14+
"Command failed with OPENAI_API_KEY=sk-proj-abc123 SECRET_TOKEN=hunter2 PATH=/tmp/bin",
15+
),
16+
).toBe(
17+
"Command failed with OPENAI_API_KEY=[REDACTED] SECRET_TOKEN=[REDACTED] PATH=[REDACTED]",
18+
);
19+
});
20+
21+
it("redacts sensitive JSON-like fields and query params", () => {
22+
expect(
23+
redactSensitiveText(
24+
'Request failed: {"token":"abc123","password":"hunter2"} https://x.test?token=abc123&ok=1',
25+
),
26+
).toBe(
27+
'Request failed: {"token":"[REDACTED]","password":"[REDACTED]"} https://x.test?token=[REDACTED]&ok=1',
28+
);
29+
});
30+
});
31+
32+
describe("redactSensitiveValue", () => {
33+
it("redacts nested structured payloads", () => {
34+
expect(
35+
redactSensitiveValue({
36+
summary: "Push failed for sk-proj-abc123",
37+
nextSteps: ["Unset OPENAI_API_KEY=sk-proj-abc123"],
38+
nested: {
39+
detail: "Authorization: Bearer topsecret",
40+
},
41+
}),
42+
).toEqual({
43+
summary: "Push failed for [REDACTED]",
44+
nextSteps: ["Unset OPENAI_API_KEY=[REDACTED]"],
45+
nested: {
46+
detail: "Authorization: Bearer [REDACTED]",
47+
},
48+
});
49+
});
50+
});

packages/shared/src/redaction.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
const REDACTED = "[REDACTED]";
2+
3+
const SECRET_PREFIX_PATTERN = /\bsk-[A-Za-z0-9][A-Za-z0-9_-]*\b/g;
4+
const BEARER_TOKEN_PATTERN = /\b(Bearer\s+)([^\s,;]+)/gi;
5+
const SENSITIVE_QUERY_PARAM_PATTERN =
6+
/([?&](?:access[_-]?token|api[_-]?key|auth(?:orization)?|client[_-]?secret|password|refresh[_-]?token|secret|session[_-]?token|token)=)([^&#\s]+)/gi;
7+
const SENSITIVE_FIELD_PATTERN =
8+
/((?:"|')?(?:access[_-]?token|api[_-]?key|auth(?:orization)?|client[_-]?secret|password|refresh[_-]?token|secret|session[_-]?token|token)(?:"|')?\s*[:=]\s*)(["'`]?)([^"'`\s,}]+)(\2)/gi;
9+
const PROCESS_ENV_PATTERN =
10+
/\b((?:process\.)?env\.[A-Za-z_][A-Za-z0-9_]*\s*(?:=|:)\s*)(["'`]?)([^"'`\s,}]+)(\2)/g;
11+
const ENV_ASSIGNMENT_PATTERN =
12+
/\b([A-Z][A-Z0-9_]{1,63}\s*=\s*)(["'`]?)([^"'`\s]+)(\2)/g;
13+
14+
function isPlainObject(value: unknown): value is Record<string, unknown> {
15+
if (typeof value !== "object" || value === null) {
16+
return false;
17+
}
18+
const prototype = Object.getPrototypeOf(value);
19+
return prototype === Object.prototype || prototype === null;
20+
}
21+
22+
export function redactSensitiveText(text: string): string {
23+
return text
24+
.replace(SECRET_PREFIX_PATTERN, REDACTED)
25+
.replace(BEARER_TOKEN_PATTERN, `$1${REDACTED}`)
26+
.replace(SENSITIVE_QUERY_PARAM_PATTERN, `$1${REDACTED}`)
27+
.replace(SENSITIVE_FIELD_PATTERN, `$1$2${REDACTED}$4`)
28+
.replace(PROCESS_ENV_PATTERN, `$1$2${REDACTED}$4`)
29+
.replace(ENV_ASSIGNMENT_PATTERN, `$1$2${REDACTED}$4`);
30+
}
31+
32+
export function redactSensitiveValue<T>(value: T): T {
33+
if (typeof value === "string") {
34+
return redactSensitiveText(value) as T;
35+
}
36+
37+
if (Array.isArray(value)) {
38+
return value.map((item) => redactSensitiveValue(item)) as T;
39+
}
40+
41+
if (isPlainObject(value)) {
42+
const entries = Object.entries(value).map(([key, entryValue]) => [
43+
key,
44+
redactSensitiveValue(entryValue),
45+
]);
46+
return Object.fromEntries(entries) as T;
47+
}
48+
49+
return value;
50+
}

0 commit comments

Comments
 (0)