Skip to content

Commit 81ebc7e

Browse files
drobison00greptile-apps[bot]
drobison00
and
greptile-apps[bot]
authored
fix(gateway): block silent reconnect scope-upgrade escalation (#54694)
* fix(gateway): block silent reconnect scope-upgrade escalation * formatting updateas * Resolve feedback * formatting fixes * Update src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Feedback updates * fix unit test * Feedback update * Review feedback update * More Greptile nit fixes --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
1 parent 19d91aa commit 81ebc7e

2 files changed

Lines changed: 358 additions & 5 deletions

File tree

‎src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts‎

Lines changed: 290 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,290 @@
1+
import os from "node:os";
2+
import path from "node:path";
3+
import { describe, expect, test, vi } from "vitest";
4+
import { WebSocket } from "ws";
5+
import {
6+
loadOrCreateDeviceIdentity,
7+
publicKeyRawBase64UrlFromPem,
8+
type DeviceIdentity,
9+
} from "../infra/device-identity.js";
10+
import * as devicePairingModule from "../infra/device-pairing.js";
11+
import {
12+
approveDevicePairing,
13+
getPairedDevice,
14+
requestDevicePairing,
15+
} from "../infra/device-pairing.js";
16+
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js";
17+
import {
18+
connectOk,
19+
connectReq,
20+
installGatewayTestHooks,
21+
onceMessage,
22+
startServerWithClient,
23+
trackConnectChallengeNonce,
24+
} from "./test-helpers.js";
25+
26+
installGatewayTestHooks({ scope: "suite" });
27+
28+
function resolveDeviceIdentityPath(name: string): string {
29+
const root = process.env.OPENCLAW_STATE_DIR ?? process.env.HOME ?? os.tmpdir();
30+
return path.join(root, "test-device-identities", `${name}.json`);
31+
}
32+
33+
function loadDeviceIdentity(name: string): {
34+
identityPath: string;
35+
identity: DeviceIdentity;
36+
publicKey: string;
37+
} {
38+
const identityPath = resolveDeviceIdentityPath(name);
39+
const identity = loadOrCreateDeviceIdentity(identityPath);
40+
return {
41+
identityPath,
42+
identity,
43+
publicKey: publicKeyRawBase64UrlFromPem(identity.publicKeyPem),
44+
};
45+
}
46+
47+
async function pairReadScopedOperator(name: string): Promise<{
48+
deviceId: string;
49+
identityPath: string;
50+
deviceToken: string;
51+
}> {
52+
const loaded = loadDeviceIdentity(name);
53+
const request = await requestDevicePairing({
54+
deviceId: loaded.identity.deviceId,
55+
publicKey: loaded.publicKey,
56+
role: "operator",
57+
scopes: ["operator.read"],
58+
clientId: GATEWAY_CLIENT_NAMES.TEST,
59+
clientMode: GATEWAY_CLIENT_MODES.TEST,
60+
});
61+
await approveDevicePairing(request.request.requestId);
62+
63+
const paired = await getPairedDevice(loaded.identity.deviceId);
64+
const deviceToken = paired?.tokens?.operator?.token ?? "";
65+
expect(deviceToken).toBeTruthy();
66+
expect(paired?.approvedScopes).toEqual(["operator.read"]);
67+
68+
return {
69+
deviceId: loaded.identity.deviceId,
70+
identityPath: loaded.identityPath,
71+
deviceToken,
72+
};
73+
}
74+
75+
async function openTrackedWs(port: number): Promise<WebSocket> {
76+
const ws = new WebSocket(`ws://127.0.0.1:${port}`);
77+
trackConnectChallengeNonce(ws);
78+
await new Promise<void>((resolve, reject) => {
79+
const timer = setTimeout(() => reject(new Error("timeout waiting for ws open")), 5_000);
80+
ws.once("open", () => {
81+
clearTimeout(timer);
82+
resolve();
83+
});
84+
ws.once("error", (error) => {
85+
clearTimeout(timer);
86+
reject(error);
87+
});
88+
});
89+
return ws;
90+
}
91+
92+
describe("gateway silent scope-upgrade reconnect", () => {
93+
test("does not silently widen a read-scoped paired device to admin on shared-auth reconnect", async () => {
94+
const started = await startServerWithClient("secret");
95+
const paired = await pairReadScopedOperator("silent-scope-upgrade-reconnect-poc");
96+
97+
let watcherWs: WebSocket | undefined;
98+
let sharedAuthReconnectWs: WebSocket | undefined;
99+
let postAttemptDeviceTokenWs: WebSocket | undefined;
100+
101+
try {
102+
watcherWs = await openTrackedWs(started.port);
103+
await connectOk(watcherWs, { scopes: ["operator.admin"] });
104+
const requestedEvent = onceMessage(
105+
watcherWs,
106+
(obj) => obj.type === "event" && obj.event === "device.pair.requested",
107+
);
108+
sharedAuthReconnectWs = await openTrackedWs(started.port);
109+
const sharedAuthUpgradeAttempt = await connectReq(sharedAuthReconnectWs, {
110+
token: "secret",
111+
deviceIdentityPath: paired.identityPath,
112+
scopes: ["operator.admin"],
113+
});
114+
expect(sharedAuthUpgradeAttempt.ok).toBe(false);
115+
expect(sharedAuthUpgradeAttempt.error?.message).toBe("pairing required");
116+
117+
const pending = await devicePairingModule.listDevicePairing();
118+
expect(pending.pending).toHaveLength(1);
119+
expect(
120+
(sharedAuthUpgradeAttempt.error?.details as { requestId?: unknown; code?: string })
121+
?.requestId,
122+
).toBe(pending.pending[0]?.requestId);
123+
const requested = (await requestedEvent) as {
124+
payload?: { requestId?: string; deviceId?: string; scopes?: string[] };
125+
};
126+
expect(requested.payload?.requestId).toBe(pending.pending[0]?.requestId);
127+
expect(requested.payload?.deviceId).toBe(paired.deviceId);
128+
expect(requested.payload?.scopes).toEqual(["operator.admin"]);
129+
130+
const afterUpgradeAttempt = await getPairedDevice(paired.deviceId);
131+
expect(afterUpgradeAttempt?.approvedScopes).toEqual(["operator.read"]);
132+
expect(afterUpgradeAttempt?.tokens?.operator?.scopes).toEqual(["operator.read"]);
133+
expect(afterUpgradeAttempt?.tokens?.operator?.token).toBe(paired.deviceToken);
134+
135+
postAttemptDeviceTokenWs = await openTrackedWs(started.port);
136+
const afterUpgrade = await connectReq(postAttemptDeviceTokenWs, {
137+
skipDefaultAuth: true,
138+
deviceToken: paired.deviceToken,
139+
deviceIdentityPath: paired.identityPath,
140+
scopes: ["operator.admin"],
141+
});
142+
expect(afterUpgrade.ok).toBe(false);
143+
} finally {
144+
watcherWs?.close();
145+
sharedAuthReconnectWs?.close();
146+
postAttemptDeviceTokenWs?.close();
147+
started.ws.close();
148+
await started.server.close();
149+
started.envSnapshot.restore();
150+
}
151+
});
152+
153+
test("accepts local silent reconnect when pairing was concurrently approved", async () => {
154+
const started = await startServerWithClient("secret");
155+
const loaded = loadDeviceIdentity("silent-reconnect-race");
156+
let ws: WebSocket | undefined;
157+
158+
const approveOriginal = devicePairingModule.approveDevicePairing;
159+
let simulatedRace = false;
160+
const forwardApprove = async (requestId: string, optionsOrBaseDir?: unknown) => {
161+
if (optionsOrBaseDir && typeof optionsOrBaseDir === "object") {
162+
return await approveOriginal(
163+
requestId,
164+
optionsOrBaseDir as { callerScopes?: readonly string[] },
165+
);
166+
}
167+
return await approveOriginal(requestId);
168+
};
169+
const approveSpy = vi
170+
.spyOn(devicePairingModule, "approveDevicePairing")
171+
.mockImplementation(async (requestId: string, optionsOrBaseDir?: unknown) => {
172+
if (simulatedRace) {
173+
return await forwardApprove(requestId, optionsOrBaseDir);
174+
}
175+
simulatedRace = true;
176+
await forwardApprove(requestId, optionsOrBaseDir);
177+
return null;
178+
});
179+
180+
try {
181+
ws = await openTrackedWs(started.port);
182+
const res = await connectReq(ws, {
183+
token: "secret",
184+
deviceIdentityPath: loaded.identityPath,
185+
});
186+
expect(res.ok).toBe(true);
187+
188+
const paired = await getPairedDevice(loaded.identity.deviceId);
189+
expect(paired?.publicKey).toBe(loaded.publicKey);
190+
expect(paired?.tokens?.operator?.token).toBeTruthy();
191+
} finally {
192+
approveSpy.mockRestore();
193+
ws?.close();
194+
started.ws.close();
195+
await started.server.close();
196+
started.envSnapshot.restore();
197+
}
198+
});
199+
200+
test("does not rebroadcast a deleted silent pairing request after a concurrent rejection", async () => {
201+
const started = await startServerWithClient("secret");
202+
const loaded = loadDeviceIdentity("silent-reconnect-reject-race");
203+
let ws: WebSocket | undefined;
204+
205+
const approveSpy = vi
206+
.spyOn(devicePairingModule, "approveDevicePairing")
207+
.mockImplementation(async (requestId: string) => {
208+
await devicePairingModule.rejectDevicePairing(requestId);
209+
return null;
210+
});
211+
212+
try {
213+
await connectOk(started.ws, { scopes: ["operator.pairing"], device: null });
214+
const requestedEvent = onceMessage(
215+
started.ws,
216+
(obj) => obj.type === "event" && obj.event === "device.pair.requested",
217+
300,
218+
);
219+
220+
ws = await openTrackedWs(started.port);
221+
const res = await connectReq(ws, {
222+
token: "secret",
223+
deviceIdentityPath: loaded.identityPath,
224+
});
225+
226+
expect(res.ok).toBe(false);
227+
expect(res.error?.message).toBe("pairing required");
228+
expect(
229+
(res.error?.details as { requestId?: unknown; code?: string } | undefined)?.requestId,
230+
).toBeUndefined();
231+
await expect(requestedEvent).rejects.toThrow("timeout");
232+
233+
const pending = await devicePairingModule.listDevicePairing();
234+
expect(pending.pending).toEqual([]);
235+
} finally {
236+
approveSpy.mockRestore();
237+
ws?.close();
238+
started.ws.close();
239+
await started.server.close();
240+
started.envSnapshot.restore();
241+
}
242+
});
243+
244+
test("returns the replacement pending request id when a silent request is superseded", async () => {
245+
const started = await startServerWithClient("secret");
246+
const loaded = loadDeviceIdentity("silent-reconnect-supersede-race");
247+
let ws: WebSocket | undefined;
248+
let replacementRequestId = "";
249+
250+
const approveSpy = vi
251+
.spyOn(devicePairingModule, "approveDevicePairing")
252+
.mockImplementation(async (_requestId: string) => {
253+
const replacement = await devicePairingModule.requestDevicePairing({
254+
deviceId: loaded.identity.deviceId,
255+
publicKey: loaded.publicKey,
256+
role: "operator",
257+
scopes: ["operator.read"],
258+
clientId: GATEWAY_CLIENT_NAMES.TEST,
259+
clientMode: GATEWAY_CLIENT_MODES.TEST,
260+
silent: false,
261+
});
262+
replacementRequestId = replacement.request.requestId;
263+
return null;
264+
});
265+
266+
try {
267+
ws = await openTrackedWs(started.port);
268+
const res = await connectReq(ws, {
269+
token: "secret",
270+
deviceIdentityPath: loaded.identityPath,
271+
});
272+
273+
expect(res.ok).toBe(false);
274+
expect(res.error?.message).toBe("pairing required");
275+
expect(replacementRequestId).toBeTruthy();
276+
expect(
277+
(res.error?.details as { requestId?: unknown; code?: string } | undefined)?.requestId,
278+
).toBe(replacementRequestId);
279+
280+
const pending = await devicePairingModule.listDevicePairing();
281+
expect(pending.pending.map((entry) => entry.requestId)).toContain(replacementRequestId);
282+
} finally {
283+
approveSpy.mockRestore();
284+
ws?.close();
285+
started.ws.close();
286+
await started.server.close();
287+
started.envSnapshot.restore();
288+
}
289+
});
290+
});

0 commit comments

Comments
 (0)