Skip to content

Commit c593683

Browse files
committed
fix(server): harden activation command gating
1 parent 760ab1a commit c593683

3 files changed

Lines changed: 147 additions & 6 deletions

File tree

packages/server/src/__tests__/activation-commands.test.ts

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,134 @@ describe("activation commands", () => {
104104
});
105105
});
106106

107+
it("rejects non-activation websocket commands when metadata lookup returns undefined", async () => {
108+
const broadcaster = {
109+
broadcast: vi.fn(),
110+
sendToClient: vi.fn(() => true),
111+
sendBinaryToClient: vi.fn(() => true),
112+
getRequestMetadata: vi.fn(() => undefined),
113+
} satisfies Broadcaster;
114+
const ctx = createBaseContext({ broadcaster });
115+
116+
const result = await dispatch(
117+
{
118+
kind: "command",
119+
id: "00000000-0000-4000-8000-000000000004",
120+
op: "workspace.list",
121+
args: {},
122+
},
123+
ctx,
124+
"ws-a"
125+
);
126+
127+
expect(result.ok).toBe(false);
128+
expect(result.error).toEqual({
129+
code: "activation_required",
130+
message: "This tab is no longer the active session",
131+
});
132+
});
133+
134+
it("does not allow a stale websocket to heartbeat after same-client rebind", async () => {
135+
const request = createMockRequest();
136+
const broadcaster = {
137+
broadcast: vi.fn(),
138+
sendToClient: vi.fn(() => true),
139+
sendBinaryToClient: vi.fn(() => true),
140+
getRequestMetadata: vi.fn(() => request),
141+
} satisfies Broadcaster;
142+
const ctx = createBaseContext({ broadcaster });
143+
144+
await dispatch(
145+
{
146+
kind: "command",
147+
id: "claim-1",
148+
op: "activation.claim",
149+
args: { clientInstanceId: "client-a" },
150+
},
151+
ctx,
152+
"ws-a"
153+
);
154+
155+
const rebound = await dispatch(
156+
{
157+
kind: "command",
158+
id: "claim-2",
159+
op: "activation.claim",
160+
args: { clientInstanceId: "client-a" },
161+
},
162+
ctx,
163+
"ws-b"
164+
);
165+
166+
expect(rebound.ok).toBe(true);
167+
168+
const heartbeat = await dispatch(
169+
{
170+
kind: "command",
171+
id: "heartbeat-stale",
172+
op: "activation.heartbeat",
173+
args: { clientInstanceId: "client-a", generation: 1 },
174+
},
175+
ctx,
176+
"ws-a"
177+
);
178+
179+
expect(heartbeat.ok).toBe(true);
180+
expect(heartbeat.data).toEqual({ ok: false });
181+
});
182+
183+
it("does not allow a stale websocket to release after same-client rebind", async () => {
184+
const request = createMockRequest();
185+
const broadcaster = {
186+
broadcast: vi.fn(),
187+
sendToClient: vi.fn(() => true),
188+
sendBinaryToClient: vi.fn(() => true),
189+
getRequestMetadata: vi.fn(() => request),
190+
} satisfies Broadcaster;
191+
const ctx = createBaseContext({ broadcaster });
192+
193+
await dispatch(
194+
{
195+
kind: "command",
196+
id: "claim-3",
197+
op: "activation.claim",
198+
args: { clientInstanceId: "client-a" },
199+
},
200+
ctx,
201+
"ws-a"
202+
);
203+
204+
await dispatch(
205+
{
206+
kind: "command",
207+
id: "claim-4",
208+
op: "activation.claim",
209+
args: { clientInstanceId: "client-a" },
210+
},
211+
ctx,
212+
"ws-b"
213+
);
214+
215+
const release = await dispatch(
216+
{
217+
kind: "command",
218+
id: "release-stale",
219+
op: "activation.release",
220+
args: { clientInstanceId: "client-a", generation: 1 },
221+
},
222+
ctx,
223+
"ws-a"
224+
);
225+
226+
expect(release.ok).toBe(true);
227+
expect(release.data).toEqual({ ok: false });
228+
expect(ctx.activationMgr.getLease()).toMatchObject({
229+
clientInstanceId: "client-a",
230+
wsClientId: "ws-b",
231+
generation: 1,
232+
});
233+
});
234+
107235
it("does not block direct internal dispatches without websocket request metadata", async () => {
108236
const ctx = createBaseContext();
109237

packages/server/src/commands/activation.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,27 @@ registerCommand(
2727
registerCommand(
2828
"activation.heartbeat",
2929
z.object({ clientInstanceId: z.string(), generation: z.number().int().positive() }),
30-
async (args, ctx) => ({
31-
ok: ctx.activationMgr.heartbeat(args.clientInstanceId, args.generation),
32-
})
30+
async (args, ctx, clientId) => {
31+
const lease = ctx.activationMgr.getLease();
32+
if (!clientId || !lease || lease.wsClientId !== clientId) {
33+
return { ok: false };
34+
}
35+
36+
return {
37+
ok: ctx.activationMgr.heartbeat(args.clientInstanceId, args.generation),
38+
};
39+
}
3340
);
3441

3542
registerCommand(
3643
"activation.release",
3744
z.object({ clientInstanceId: z.string(), generation: z.number().int().positive() }),
38-
async (args, ctx) => {
45+
async (args, ctx, clientId) => {
46+
const lease = ctx.activationMgr.getLease();
47+
if (!clientId || !lease || lease.wsClientId !== clientId) {
48+
return { ok: false };
49+
}
50+
3951
ctx.activationMgr.release(args.clientInstanceId, args.generation);
4052
return { ok: true };
4153
}

packages/server/src/ws/dispatch.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,10 @@ export async function dispatch(
8585
ctx: CommandContext,
8686
clientId?: string
8787
): Promise<Result> {
88-
const requestMetadata = clientId ? ctx.broadcaster.getRequestMetadata?.(clientId) : undefined;
88+
const isWsDispatch =
89+
clientId !== undefined && typeof ctx.broadcaster.getRequestMetadata === "function";
8990

90-
if (requestMetadata && !ACTIVATION_ALLOWLIST.has(msg.op)) {
91+
if (isWsDispatch && !ACTIVATION_ALLOWLIST.has(msg.op)) {
9192
const active = ctx.activationMgr.getLease();
9293
if (!active || active.wsClientId !== clientId) {
9394
return {

0 commit comments

Comments
 (0)