Skip to content

Commit 744c2c9

Browse files
committed
fix(ctl): harden proxy and WebSocket handler
1 parent a279a92 commit 744c2c9

5 files changed

Lines changed: 77 additions & 2 deletions

File tree

packages/control/src/bridge/handler.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
HEARTBEAT_TIMEOUT_MS,
2222
IDLE_TIMEOUT_MS,
2323
MAX_MESSAGE_BYTES,
24+
MAX_PENDING_MESSAGES,
2425
type ProxyConnection,
2526
RATE_LIMIT_MAX,
2627
RATE_LIMIT_WINDOW_MS,
@@ -366,7 +367,12 @@ export function createProxyHandler(
366367
onMessage(evt, ws) {
367368
conn.lastActivity = Date.now();
368369

369-
const raw = typeof evt.data === "string" ? evt.data : String(evt.data);
370+
if (typeof evt.data !== "string") {
371+
closeWith(ws, 1003, "Binary frames not supported");
372+
cleanup();
373+
return;
374+
}
375+
const raw = evt.data;
370376

371377
if (raw.length > MAX_MESSAGE_BYTES) {
372378
closeWith(ws, 1009, "Message too large");
@@ -381,6 +387,11 @@ export function createProxyHandler(
381387
}
382388

383389
if (conn.state === "connecting") {
390+
if (conn.pendingMessages.length >= MAX_PENDING_MESSAGES) {
391+
closeWith(ws, CloseCodes.RATE_LIMITED, "Too many pending messages");
392+
cleanup();
393+
return;
394+
}
384395
conn.pendingMessages.push(raw);
385396
return;
386397
}

packages/control/src/bridge/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ export const RATE_LIMIT_WINDOW_MS = 1_000;
5656
export const RATE_LIMIT_MAX = 10;
5757
/** Maximum inbound message size in bytes. */
5858
export const MAX_MESSAGE_BYTES = 1024 * 1024;
59+
/** Maximum messages buffered while backend is connecting. */
60+
export const MAX_PENDING_MESSAGES = 20;
5961

6062
/** Interval (ms) between heartbeat pings with session re-validation. */
6163
export const HEARTBEAT_INTERVAL_MS = 25_000;

packages/control/src/proxy.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,16 @@ export async function proxyToGateway(
2121
target.port = gateway.port;
2222
target.pathname = target.pathname.replace("/ops/control", "");
2323

24-
const res = await fetch(target.toString(), ctx.req.raw);
24+
const headers = new Headers(ctx.req.raw.headers);
25+
headers.delete("cookie");
26+
headers.delete("authorization");
27+
28+
const res = await fetch(target.toString(), {
29+
method: ctx.req.method,
30+
headers,
31+
body: ctx.req.raw.body,
32+
redirect: "manual",
33+
});
2534

2635
if (!res.ok) {
2736
return ctx.json({ error: "Bad Gateway" }, 502);

packages/control/test/handler.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,21 @@ describe("createProxyHandler", () => {
108108
expect(mock.closeCode).toBe(CloseCodes.BACKEND_UNAVAILABLE);
109109
});
110110

111+
it("rejects binary frames", () => {
112+
const handler = createProxyHandler(makePrincipal(), makeDeps());
113+
const mock = createMockWs();
114+
115+
const msgEvent = new MessageEvent("message", {
116+
data: new ArrayBuffer(8),
117+
});
118+
119+
// biome-ignore lint/suspicious/noExplicitAny: test mock
120+
handler.onMessage?.(msgEvent, mock.ws as any);
121+
122+
expect(mock.closed).toBe(true);
123+
expect(mock.closeCode).toBe(1003);
124+
});
125+
111126
it("handles oversized messages", () => {
112127
const handler = createProxyHandler(makePrincipal(), makeDeps());
113128
const mock = createMockWs();
@@ -181,6 +196,25 @@ describe("concurrent connection limit", () => {
181196

182197
// --- Rate limiting ---
183198

199+
describe("pending message buffer", () => {
200+
it("closes connection when pending buffer overflows", () => {
201+
const handler = createProxyHandler(makePrincipal(), makeDeps());
202+
const mock = createMockWs();
203+
204+
// Don't call onOpen — connection stays in "connecting" state
205+
for (let i = 0; i <= 20; i++) {
206+
const msgEvent = new MessageEvent("message", {
207+
data: JSON.stringify({ type: "req", method: "test", i }),
208+
});
209+
// biome-ignore lint/suspicious/noExplicitAny: test mock
210+
handler.onMessage?.(msgEvent, mock.ws as any);
211+
}
212+
213+
expect(mock.closed).toBe(true);
214+
expect(mock.closeCode).toBe(CloseCodes.RATE_LIMITED);
215+
});
216+
});
217+
184218
describe("message rate limiting", () => {
185219
it("closes connection after exceeding rate limit", () => {
186220
const handler = createProxyHandler(makePrincipal(), makeDeps());

packages/control/test/proxy.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,25 @@ describe("proxyToGateway", () => {
6868
expect(body).toEqual({ error: "Bad Gateway" });
6969
});
7070

71+
it("strips Cookie and Authorization headers before forwarding", async () => {
72+
fetchSpy.mockResolvedValueOnce(new Response("ok", { status: 200 }));
73+
74+
const app = createApp("http://gateway:18789");
75+
await app.request("/ops/control/dashboard", {
76+
headers: {
77+
Cookie: "access_token=secret; refresh_token=secret",
78+
Authorization: "Bearer leaked",
79+
Accept: "text/html",
80+
},
81+
});
82+
83+
const calledInit = fetchSpy.mock.calls[0]?.[1] as RequestInit;
84+
const fwdHeaders = new Headers(calledInit.headers);
85+
expect(fwdHeaders.get("cookie")).toBeNull();
86+
expect(fwdHeaders.get("authorization")).toBeNull();
87+
expect(fwdHeaders.get("accept")).toBe("text/html");
88+
});
89+
7190
it("does not leak gateway error details", async () => {
7291
fetchSpy.mockResolvedValueOnce(
7392
new Response(

0 commit comments

Comments
 (0)