fix: stabilize invalid-connect handshake response
This commit is contained in:
@@ -23,6 +23,7 @@
|
|||||||
|
|
||||||
### Fixes
|
### Fixes
|
||||||
- Gateway/WebChat: include handshake validation details in the WebSocket close reason for easier debugging.
|
- Gateway/WebChat: include handshake validation details in the WebSocket close reason for easier debugging.
|
||||||
|
- Gateway/Auth: send invalid connect responses before closing the handshake; stabilize invalid-connect auth test.
|
||||||
- Doctor: surface plugin diagnostics in the report.
|
- Doctor: surface plugin diagnostics in the report.
|
||||||
- CLI/Update: preserve base environment when passing overrides to update subprocesses. (#713) — thanks @danielz1z.
|
- CLI/Update: preserve base environment when passing overrides to update subprocesses. (#713) — thanks @danielz1z.
|
||||||
- Agents: treat message tool errors as failures so fallback replies still send; require `to` + `message` for `action=send`. (#717) — thanks @theglove44.
|
- Agents: treat message tool errors as failures so fallback replies still send; require `to` + `message` for `action=send`. (#717) — thanks @theglove44.
|
||||||
|
|||||||
@@ -126,20 +126,11 @@ describe("gateway server auth/connect", () => {
|
|||||||
await server.close();
|
await server.close();
|
||||||
});
|
});
|
||||||
|
|
||||||
test.skip(
|
test(
|
||||||
"invalid connect params surface in response and close reason",
|
"invalid connect params surface in response and close reason",
|
||||||
{ timeout: 15000 },
|
{ timeout: 15000 },
|
||||||
async () => {
|
async () => {
|
||||||
const { server, ws } = await startServerWithClient();
|
const { server, ws } = await startServerWithClient();
|
||||||
await new Promise<void>((resolve) => ws.once("open", resolve));
|
|
||||||
|
|
||||||
const closePromise = new Promise<{ code: number; reason: string }>(
|
|
||||||
(resolve) => {
|
|
||||||
ws.once("close", (code, reason) =>
|
|
||||||
resolve({ code, reason: reason.toString() }),
|
|
||||||
);
|
|
||||||
},
|
|
||||||
);
|
|
||||||
|
|
||||||
ws.send(
|
ws.send(
|
||||||
JSON.stringify({
|
JSON.stringify({
|
||||||
@@ -159,8 +150,7 @@ describe("gateway server auth/connect", () => {
|
|||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
|
|
||||||
const raceResult = await Promise.race([
|
const res = await onceMessage<{
|
||||||
onceMessage<{
|
|
||||||
ok: boolean;
|
ok: boolean;
|
||||||
error?: { message?: string };
|
error?: { message?: string };
|
||||||
}>(
|
}>(
|
||||||
@@ -168,28 +158,26 @@ describe("gateway server auth/connect", () => {
|
|||||||
(o) =>
|
(o) =>
|
||||||
(o as { type?: string }).type === "res" &&
|
(o as { type?: string }).type === "res" &&
|
||||||
(o as { id?: string }).id === "h-bad",
|
(o as { id?: string }).id === "h-bad",
|
||||||
),
|
);
|
||||||
closePromise,
|
expect(res.ok).toBe(false);
|
||||||
]);
|
expect(String(res.error?.message ?? "")).toContain(
|
||||||
|
|
||||||
if ("ok" in raceResult) {
|
|
||||||
expect(raceResult.ok).toBe(false);
|
|
||||||
expect(String(raceResult.error?.message ?? "")).toContain(
|
|
||||||
"invalid connect params",
|
"invalid connect params",
|
||||||
);
|
);
|
||||||
|
|
||||||
const closeInfo = await new Promise<{ code: number; reason: string }>(
|
const closeInfo = await new Promise<{ code: number; reason: string }>(
|
||||||
(resolve) => {
|
(resolve, reject) => {
|
||||||
ws.once("close", (code, reason) =>
|
const timer = setTimeout(
|
||||||
resolve({ code, reason: reason.toString() }),
|
() => reject(new Error("close timeout")),
|
||||||
|
3000,
|
||||||
);
|
);
|
||||||
|
ws.once("close", (code, reason) => {
|
||||||
|
clearTimeout(timer);
|
||||||
|
resolve({ code, reason: reason.toString() });
|
||||||
|
});
|
||||||
},
|
},
|
||||||
);
|
);
|
||||||
expect(closeInfo.code).toBe(1008);
|
expect(closeInfo.code).toBe(1008);
|
||||||
expect(closeInfo.reason).toContain("invalid connect params");
|
expect(closeInfo.reason).toContain("invalid connect params");
|
||||||
} else {
|
|
||||||
// handshake timed out/closed before response; still ensure closure happened
|
|
||||||
expect(raceResult.code === 1008 || raceResult.code === 1000).toBe(true);
|
|
||||||
}
|
|
||||||
|
|
||||||
await server.close();
|
await server.close();
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -1469,17 +1469,18 @@ export async function startGatewayServer(
|
|||||||
if (!client) {
|
if (!client) {
|
||||||
// Handshake must be a normal request:
|
// Handshake must be a normal request:
|
||||||
// { type:"req", method:"connect", params: ConnectParams }.
|
// { type:"req", method:"connect", params: ConnectParams }.
|
||||||
|
const isRequestFrame = validateRequestFrame(parsed);
|
||||||
if (
|
if (
|
||||||
!validateRequestFrame(parsed) ||
|
!isRequestFrame ||
|
||||||
(parsed as RequestFrame).method !== "connect" ||
|
(parsed as RequestFrame).method !== "connect" ||
|
||||||
!validateConnectParams((parsed as RequestFrame).params)
|
!validateConnectParams((parsed as RequestFrame).params)
|
||||||
) {
|
) {
|
||||||
const handshakeError = validateRequestFrame(parsed)
|
const handshakeError = isRequestFrame
|
||||||
? (parsed as RequestFrame).method === "connect"
|
? (parsed as RequestFrame).method === "connect"
|
||||||
? `invalid connect params: ${formatValidationErrors(validateConnectParams.errors)}`
|
? `invalid connect params: ${formatValidationErrors(validateConnectParams.errors)}`
|
||||||
: "invalid handshake: first request must be connect"
|
: "invalid handshake: first request must be connect"
|
||||||
: "invalid request frame";
|
: "invalid request frame";
|
||||||
if (validateRequestFrame(parsed)) {
|
if (isRequestFrame) {
|
||||||
const req = parsed as RequestFrame;
|
const req = parsed as RequestFrame;
|
||||||
send({
|
send({
|
||||||
type: "res",
|
type: "res",
|
||||||
@@ -1502,7 +1503,11 @@ export async function startGatewayServer(
|
|||||||
const closeReason = truncateCloseReason(
|
const closeReason = truncateCloseReason(
|
||||||
handshakeError || "invalid handshake",
|
handshakeError || "invalid handshake",
|
||||||
);
|
);
|
||||||
|
if (isRequestFrame) {
|
||||||
|
queueMicrotask(() => close(1008, closeReason));
|
||||||
|
} else {
|
||||||
close(1008, closeReason);
|
close(1008, closeReason);
|
||||||
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user