diff --git a/src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts b/src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts new file mode 100644 index 00000000000..4f2f359f54d --- /dev/null +++ b/src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts @@ -0,0 +1,290 @@ +import os from "node:os"; +import path from "node:path"; +import { describe, expect, test, vi } from "vitest"; +import { WebSocket } from "ws"; +import { + loadOrCreateDeviceIdentity, + publicKeyRawBase64UrlFromPem, + type DeviceIdentity, +} from "../infra/device-identity.js"; +import * as devicePairingModule from "../infra/device-pairing.js"; +import { + approveDevicePairing, + getPairedDevice, + requestDevicePairing, +} from "../infra/device-pairing.js"; +import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js"; +import { + connectOk, + connectReq, + installGatewayTestHooks, + onceMessage, + startServerWithClient, + trackConnectChallengeNonce, +} from "./test-helpers.js"; + +installGatewayTestHooks({ scope: "suite" }); + +function resolveDeviceIdentityPath(name: string): string { + const root = process.env.OPENCLAW_STATE_DIR ?? process.env.HOME ?? os.tmpdir(); + return path.join(root, "test-device-identities", `${name}.json`); +} + +function loadDeviceIdentity(name: string): { + identityPath: string; + identity: DeviceIdentity; + publicKey: string; +} { + const identityPath = resolveDeviceIdentityPath(name); + const identity = loadOrCreateDeviceIdentity(identityPath); + return { + identityPath, + identity, + publicKey: publicKeyRawBase64UrlFromPem(identity.publicKeyPem), + }; +} + +async function pairReadScopedOperator(name: string): Promise<{ + deviceId: string; + identityPath: string; + deviceToken: string; +}> { + const loaded = loadDeviceIdentity(name); + const request = await requestDevicePairing({ + deviceId: loaded.identity.deviceId, + publicKey: loaded.publicKey, + role: "operator", + scopes: ["operator.read"], + clientId: GATEWAY_CLIENT_NAMES.TEST, + clientMode: GATEWAY_CLIENT_MODES.TEST, + }); + await approveDevicePairing(request.request.requestId); + + const paired = await getPairedDevice(loaded.identity.deviceId); + const deviceToken = paired?.tokens?.operator?.token ?? ""; + expect(deviceToken).toBeTruthy(); + expect(paired?.approvedScopes).toEqual(["operator.read"]); + + return { + deviceId: loaded.identity.deviceId, + identityPath: loaded.identityPath, + deviceToken, + }; +} + +async function openTrackedWs(port: number): Promise { + const ws = new WebSocket(`ws://127.0.0.1:${port}`); + trackConnectChallengeNonce(ws); + await new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error("timeout waiting for ws open")), 5_000); + ws.once("open", () => { + clearTimeout(timer); + resolve(); + }); + ws.once("error", (error) => { + clearTimeout(timer); + reject(error); + }); + }); + return ws; +} + +describe("gateway silent scope-upgrade reconnect", () => { + test("does not silently widen a read-scoped paired device to admin on shared-auth reconnect", async () => { + const started = await startServerWithClient("secret"); + const paired = await pairReadScopedOperator("silent-scope-upgrade-reconnect-poc"); + + let watcherWs: WebSocket | undefined; + let sharedAuthReconnectWs: WebSocket | undefined; + let postAttemptDeviceTokenWs: WebSocket | undefined; + + try { + watcherWs = await openTrackedWs(started.port); + await connectOk(watcherWs, { scopes: ["operator.admin"] }); + const requestedEvent = onceMessage( + watcherWs, + (obj) => obj.type === "event" && obj.event === "device.pair.requested", + ); + sharedAuthReconnectWs = await openTrackedWs(started.port); + const sharedAuthUpgradeAttempt = await connectReq(sharedAuthReconnectWs, { + token: "secret", + deviceIdentityPath: paired.identityPath, + scopes: ["operator.admin"], + }); + expect(sharedAuthUpgradeAttempt.ok).toBe(false); + expect(sharedAuthUpgradeAttempt.error?.message).toBe("pairing required"); + + const pending = await devicePairingModule.listDevicePairing(); + expect(pending.pending).toHaveLength(1); + expect( + (sharedAuthUpgradeAttempt.error?.details as { requestId?: unknown; code?: string }) + ?.requestId, + ).toBe(pending.pending[0]?.requestId); + const requested = (await requestedEvent) as { + payload?: { requestId?: string; deviceId?: string; scopes?: string[] }; + }; + expect(requested.payload?.requestId).toBe(pending.pending[0]?.requestId); + expect(requested.payload?.deviceId).toBe(paired.deviceId); + expect(requested.payload?.scopes).toEqual(["operator.admin"]); + + const afterUpgradeAttempt = await getPairedDevice(paired.deviceId); + expect(afterUpgradeAttempt?.approvedScopes).toEqual(["operator.read"]); + expect(afterUpgradeAttempt?.tokens?.operator?.scopes).toEqual(["operator.read"]); + expect(afterUpgradeAttempt?.tokens?.operator?.token).toBe(paired.deviceToken); + + postAttemptDeviceTokenWs = await openTrackedWs(started.port); + const afterUpgrade = await connectReq(postAttemptDeviceTokenWs, { + skipDefaultAuth: true, + deviceToken: paired.deviceToken, + deviceIdentityPath: paired.identityPath, + scopes: ["operator.admin"], + }); + expect(afterUpgrade.ok).toBe(false); + } finally { + watcherWs?.close(); + sharedAuthReconnectWs?.close(); + postAttemptDeviceTokenWs?.close(); + started.ws.close(); + await started.server.close(); + started.envSnapshot.restore(); + } + }); + + test("accepts local silent reconnect when pairing was concurrently approved", async () => { + const started = await startServerWithClient("secret"); + const loaded = loadDeviceIdentity("silent-reconnect-race"); + let ws: WebSocket | undefined; + + const approveOriginal = devicePairingModule.approveDevicePairing; + let simulatedRace = false; + const forwardApprove = async (requestId: string, optionsOrBaseDir?: unknown) => { + if (optionsOrBaseDir && typeof optionsOrBaseDir === "object") { + return await approveOriginal( + requestId, + optionsOrBaseDir as { callerScopes?: readonly string[] }, + ); + } + return await approveOriginal(requestId); + }; + const approveSpy = vi + .spyOn(devicePairingModule, "approveDevicePairing") + .mockImplementation(async (requestId: string, optionsOrBaseDir?: unknown) => { + if (simulatedRace) { + return await forwardApprove(requestId, optionsOrBaseDir); + } + simulatedRace = true; + await forwardApprove(requestId, optionsOrBaseDir); + return null; + }); + + try { + ws = await openTrackedWs(started.port); + const res = await connectReq(ws, { + token: "secret", + deviceIdentityPath: loaded.identityPath, + }); + expect(res.ok).toBe(true); + + const paired = await getPairedDevice(loaded.identity.deviceId); + expect(paired?.publicKey).toBe(loaded.publicKey); + expect(paired?.tokens?.operator?.token).toBeTruthy(); + } finally { + approveSpy.mockRestore(); + ws?.close(); + started.ws.close(); + await started.server.close(); + started.envSnapshot.restore(); + } + }); + + test("does not rebroadcast a deleted silent pairing request after a concurrent rejection", async () => { + const started = await startServerWithClient("secret"); + const loaded = loadDeviceIdentity("silent-reconnect-reject-race"); + let ws: WebSocket | undefined; + + const approveSpy = vi + .spyOn(devicePairingModule, "approveDevicePairing") + .mockImplementation(async (requestId: string) => { + await devicePairingModule.rejectDevicePairing(requestId); + return null; + }); + + try { + await connectOk(started.ws, { scopes: ["operator.pairing"], device: null }); + const requestedEvent = onceMessage( + started.ws, + (obj) => obj.type === "event" && obj.event === "device.pair.requested", + 300, + ); + + ws = await openTrackedWs(started.port); + const res = await connectReq(ws, { + token: "secret", + deviceIdentityPath: loaded.identityPath, + }); + + expect(res.ok).toBe(false); + expect(res.error?.message).toBe("pairing required"); + expect( + (res.error?.details as { requestId?: unknown; code?: string } | undefined)?.requestId, + ).toBeUndefined(); + await expect(requestedEvent).rejects.toThrow("timeout"); + + const pending = await devicePairingModule.listDevicePairing(); + expect(pending.pending).toEqual([]); + } finally { + approveSpy.mockRestore(); + ws?.close(); + started.ws.close(); + await started.server.close(); + started.envSnapshot.restore(); + } + }); + + test("returns the replacement pending request id when a silent request is superseded", async () => { + const started = await startServerWithClient("secret"); + const loaded = loadDeviceIdentity("silent-reconnect-supersede-race"); + let ws: WebSocket | undefined; + let replacementRequestId = ""; + + const approveSpy = vi + .spyOn(devicePairingModule, "approveDevicePairing") + .mockImplementation(async (_requestId: string) => { + const replacement = await devicePairingModule.requestDevicePairing({ + deviceId: loaded.identity.deviceId, + publicKey: loaded.publicKey, + role: "operator", + scopes: ["operator.read"], + clientId: GATEWAY_CLIENT_NAMES.TEST, + clientMode: GATEWAY_CLIENT_MODES.TEST, + silent: false, + }); + replacementRequestId = replacement.request.requestId; + return null; + }); + + try { + ws = await openTrackedWs(started.port); + const res = await connectReq(ws, { + token: "secret", + deviceIdentityPath: loaded.identityPath, + }); + + expect(res.ok).toBe(false); + expect(res.error?.message).toBe("pairing required"); + expect(replacementRequestId).toBeTruthy(); + expect( + (res.error?.details as { requestId?: unknown; code?: string } | undefined)?.requestId, + ).toBe(replacementRequestId); + + const pending = await devicePairingModule.listDevicePairing(); + expect(pending.pending.map((entry) => entry.requestId)).toContain(replacementRequestId); + } finally { + approveSpy.mockRestore(); + ws?.close(); + started.ws.close(); + await started.server.close(); + started.envSnapshot.restore(); + } + }); +}); diff --git a/src/gateway/server/ws-connection/message-handler.ts b/src/gateway/server/ws-connection/message-handler.ts index f6c3847fece..faba448f26b 100644 --- a/src/gateway/server/ws-connection/message-handler.ts +++ b/src/gateway/server/ws-connection/message-handler.ts @@ -11,6 +11,7 @@ import { approveDevicePairing, ensureDeviceToken, getPairedDevice, + listDevicePairing, requestDevicePairing, updatePairedDeviceMetadata, verifyDeviceToken, @@ -746,6 +747,37 @@ export function attachGatewayWsMessageHandler(params: { const requirePairing = async ( reason: "not-paired" | "role-upgrade" | "scope-upgrade" | "metadata-upgrade", ) => { + const pairingStateAllowsRequestedAccess = ( + pairedCandidate: Awaited>, + ): boolean => { + if (!pairedCandidate || pairedCandidate.publicKey !== devicePublicKey) { + return false; + } + const pairedRoles = Array.isArray(pairedCandidate.roles) + ? pairedCandidate.roles + : pairedCandidate.role + ? [pairedCandidate.role] + : []; + if (pairedRoles.length > 0 && !pairedRoles.includes(role)) { + return false; + } + if (scopes.length === 0) { + return true; + } + const pairedScopes = Array.isArray(pairedCandidate.approvedScopes) + ? pairedCandidate.approvedScopes + : Array.isArray(pairedCandidate.scopes) + ? pairedCandidate.scopes + : []; + if (pairedScopes.length === 0) { + return false; + } + return roleScopesAllow({ + role, + requestedScopes: scopes, + allowedScopes: pairedScopes, + }); + }; const allowSilentLocalPairing = shouldAllowSilentLocalPairing({ isLocalClient, hasBrowserOriginHeader, @@ -757,11 +789,28 @@ export function attachGatewayWsMessageHandler(params: { deviceId: device.id, publicKey: devicePublicKey, ...clientPairingMetadata, - silent: allowSilentLocalPairing, + silent: reason === "scope-upgrade" ? false : allowSilentLocalPairing, }); const context = buildRequestContext(); + let approved: Awaited> | undefined; + let resolvedByConcurrentApproval = false; + let recoveryRequestId: string | undefined = pairing.request.requestId; + const resolveLivePendingRequestId = async (): Promise => { + const pendingList = await listDevicePairing(); + const exactPending = pendingList.pending.find( + (pending) => pending.requestId === pairing.request.requestId, + ); + if (exactPending) { + return exactPending.requestId; + } + const replacementPending = pendingList.pending.find( + (pending) => + pending.deviceId === device.id && pending.publicKey === devicePublicKey, + ); + return replacementPending?.requestId; + }; if (pairing.request.silent === true) { - const approved = await approveDevicePairing(pairing.request.requestId); + approved = await approveDevicePairing(pairing.request.requestId); if (approved) { logGateway.info( `device pairing auto-approved device=${approved.device.deviceId} role=${approved.device.role ?? "unknown"}`, @@ -776,15 +825,29 @@ export function attachGatewayWsMessageHandler(params: { }, { dropIfSlow: true }, ); + } else { + resolvedByConcurrentApproval = pairingStateAllowsRequestedAccess( + await getPairedDevice(device.id), + ); + let requestStillPending = false; + if (!resolvedByConcurrentApproval) { + recoveryRequestId = await resolveLivePendingRequestId(); + requestStillPending = recoveryRequestId === pairing.request.requestId; + } + if (requestStillPending) { + context.broadcast("device.pair.requested", pairing.request, { dropIfSlow: true }); + } } } else if (pairing.created) { context.broadcast("device.pair.requested", pairing.request, { dropIfSlow: true }); } - if (pairing.request.silent !== true) { + // Re-resolve: another connection may have superseded/approved the request since we created it + recoveryRequestId = await resolveLivePendingRequestId(); + if (!(pairing.request.silent === true && (approved || resolvedByConcurrentApproval))) { setHandshakeState("failed"); setCloseCause("pairing-required", { deviceId: device.id, - requestId: pairing.request.requestId, + ...(recoveryRequestId ? { requestId: recoveryRequestId } : {}), reason, }); send({ @@ -794,7 +857,7 @@ export function attachGatewayWsMessageHandler(params: { error: errorShape(ErrorCodes.NOT_PAIRED, "pairing required", { details: { code: ConnectErrorDetailCodes.PAIRING_REQUIRED, - requestId: pairing.request.requestId, + ...(recoveryRequestId ? { requestId: recoveryRequestId } : {}), reason, }, }),