From 4e01916a7e1bdc83e5d8fda44f6ed08a0053ac59 Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Mon, 20 Apr 2026 13:05:06 +0530 Subject: [PATCH] fix(gateway): report pairing upgrade details --- src/gateway/protocol/connect-error-details.ts | 48 +++++ src/gateway/server.auth.control-ui.suite.ts | 166 ++++++++++++++++++ ...silent-scope-upgrade-reconnect.poc.test.ts | 63 ++++++- .../server/ws-connection/message-handler.ts | 17 ++ 4 files changed, 290 insertions(+), 4 deletions(-) diff --git a/src/gateway/protocol/connect-error-details.ts b/src/gateway/protocol/connect-error-details.ts index a47d284cfcf..0f1724d88cd 100644 --- a/src/gateway/protocol/connect-error-details.ts +++ b/src/gateway/protocol/connect-error-details.ts @@ -59,6 +59,11 @@ export type PairingConnectErrorDetails = { reason?: ConnectPairingRequiredReason; requestId?: string; remediationHint?: string; + deviceId?: string; + requestedRole?: string; + requestedScopes?: string[]; + approvedRoles?: string[]; + approvedScopes?: string[]; }; const CONNECT_RECOVERY_NEXT_STEP_VALUES: ReadonlySet = new Set([ @@ -209,6 +214,16 @@ export function normalizePairingConnectRequestId(value: unknown): string | undef return normalized && PAIRING_CONNECT_REQUEST_ID_PATTERN.test(normalized) ? normalized : undefined; } +function normalizeStringArray(value: unknown): string[] | undefined { + if (!Array.isArray(value)) { + return undefined; + } + const normalized = value + .map((item) => normalizeOptionalString(item)) + .filter((item): item is string => Boolean(item)); + return normalized.length > 0 ? normalized : []; +} + export function describePairingConnectRequirement( reason: ConnectPairingRequiredReason | undefined, ): string { @@ -245,16 +260,31 @@ export function buildPairingConnectErrorDetails(params: { reason: ConnectPairingRequiredReason | undefined; requestId?: string; remediationHint?: string; + deviceId?: string; + requestedRole?: string; + requestedScopes?: string[]; + approvedRoles?: string[]; + approvedScopes?: string[]; }): PairingConnectErrorDetails { const requestId = normalizePairingConnectRequestId(params.requestId); const remediationHint = normalizeOptionalString(params.remediationHint) ?? buildPairingConnectRemediationHint(params.reason); + const deviceId = normalizeOptionalString(params.deviceId); + const requestedRole = normalizeOptionalString(params.requestedRole); + const requestedScopes = normalizeStringArray(params.requestedScopes); + const approvedRoles = normalizeStringArray(params.approvedRoles); + const approvedScopes = normalizeStringArray(params.approvedScopes); return { code: ConnectErrorDetailCodes.PAIRING_REQUIRED, ...(params.reason ? { reason: params.reason } : {}), ...(requestId ? { requestId } : {}), ...(remediationHint ? { remediationHint } : {}), + ...(deviceId ? { deviceId } : {}), + ...(requestedRole ? { requestedRole } : {}), + ...(requestedScopes ? { requestedScopes } : {}), + ...(approvedRoles ? { approvedRoles } : {}), + ...(approvedScopes ? { approvedScopes } : {}), }; } @@ -273,19 +303,37 @@ export function readPairingConnectErrorDetails( if (readConnectErrorDetailCode(details) !== ConnectErrorDetailCodes.PAIRING_REQUIRED) { return null; } + if (!details || typeof details !== "object" || Array.isArray(details)) { + return null; + } const raw = details as { reason?: unknown; requestId?: unknown; remediationHint?: unknown; + deviceId?: unknown; + requestedRole?: unknown; + requestedScopes?: unknown; + approvedRoles?: unknown; + approvedScopes?: unknown; }; const reason = normalizePairingConnectReason(raw.reason); const requestId = normalizePairingConnectRequestId(raw.requestId); const remediationHint = normalizeOptionalString(raw.remediationHint) ?? buildPairingConnectRemediationHint(reason); + const deviceId = normalizeOptionalString(raw.deviceId); + const requestedRole = normalizeOptionalString(raw.requestedRole); + const requestedScopes = normalizeStringArray(raw.requestedScopes); + const approvedRoles = normalizeStringArray(raw.approvedRoles); + const approvedScopes = normalizeStringArray(raw.approvedScopes); return { code: ConnectErrorDetailCodes.PAIRING_REQUIRED, ...(reason ? { reason } : {}), ...(requestId ? { requestId } : {}), ...(remediationHint ? { remediationHint } : {}), + ...(deviceId ? { deviceId } : {}), + ...(requestedRole ? { requestedRole } : {}), + ...(requestedScopes ? { requestedScopes } : {}), + ...(approvedRoles ? { approvedRoles } : {}), + ...(approvedScopes ? { approvedScopes } : {}), }; } diff --git a/src/gateway/server.auth.control-ui.suite.ts b/src/gateway/server.auth.control-ui.suite.ts index 9eec1d8aa6b..5480a15211a 100644 --- a/src/gateway/server.auth.control-ui.suite.ts +++ b/src/gateway/server.auth.control-ui.suite.ts @@ -14,6 +14,7 @@ import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES, onceMessage, + openTailscaleWs, openWs, originForPort, readConnectChallengeNonce, @@ -202,6 +203,16 @@ export function registerControlUiAndPairingSuite(): void { await writeJsonAtomic(pairedPath, paired); }; + const overwritePairedPublicKey = async (deviceId: string, publicKey: string) => { + const { resolvePairingPaths, readJsonFile } = await import("../infra/pairing-files.js"); + const { writeJsonAtomic } = await import("../infra/json-files.js"); + const { pairedPath } = resolvePairingPaths(undefined, "devices"); + const paired = (await readJsonFile>>(pairedPath)) ?? {}; + const metadata = getRequiredPairedMetadata(paired, deviceId); + metadata.publicKey = publicKey; + await writeJsonAtomic(pairedPath, paired); + }; + const seedApprovedOperatorReadPairing = async (params: { identityPrefix: string; clientId: string; @@ -740,6 +751,93 @@ export function registerControlUiAndPairingSuite(): void { restoreGatewayToken(prevToken); }); + test("does not expose approved access when a paired device id reconnects with a different key", async () => { + const { identity, identityPath } = await seedApprovedOperatorReadPairing({ + identityPrefix: "openclaw-device-key-mismatch-", + clientId: TEST_OPERATOR_CLIENT.id, + clientMode: TEST_OPERATOR_CLIENT.mode, + displayName: "remote-key-mismatch", + platform: TEST_OPERATOR_CLIENT.platform, + }); + await overwritePairedPublicKey(identity.deviceId, "mismatched-public-key"); + + const { server, port, prevToken } = await startControlUiServer("secret"); + const ws2 = await openTailscaleWs(port); + try { + const nonce2 = await readConnectChallengeNonce(ws2); + const mismatched = await connectReq(ws2, { + token: "secret", + scopes: ["operator.admin"], + client: { ...TEST_OPERATOR_CLIENT }, + device: await buildSignedDeviceForIdentity({ + identityPath, + client: TEST_OPERATOR_CLIENT, + scopes: ["operator.admin"], + nonce: nonce2, + }), + }); + expect(mismatched.ok).toBe(false); + expect(mismatched.error?.message ?? "").toContain("pairing required"); + expect( + ( + mismatched.error?.details as + | { + reason?: string; + requestedRole?: string; + requestedScopes?: string[]; + approvedRoles?: string[]; + approvedScopes?: string[]; + } + | undefined + )?.reason, + ).toBe("not-paired"); + expect( + ( + mismatched.error?.details as + | { + requestedRole?: string; + requestedScopes?: string[]; + } + | undefined + )?.requestedRole, + ).toBe("operator"); + expect( + ( + mismatched.error?.details as + | { + requestedRole?: string; + requestedScopes?: string[]; + } + | undefined + )?.requestedScopes, + ).toEqual(["operator.admin"]); + expect( + ( + mismatched.error?.details as + | { + approvedRoles?: string[]; + approvedScopes?: string[]; + } + | undefined + )?.approvedRoles, + ).toBeUndefined(); + expect( + ( + mismatched.error?.details as + | { + approvedRoles?: string[]; + approvedScopes?: string[]; + } + | undefined + )?.approvedScopes, + ).toBeUndefined(); + } finally { + ws2.close(); + await server.close(); + restoreGatewayToken(prevToken); + } + }); + test("auto-approves fresh node bootstrap pairing from qr setup code", async () => { const { issueDeviceBootstrapToken, verifyDeviceBootstrapToken } = await import("../infra/device-bootstrap.js"); @@ -1025,6 +1123,26 @@ export function registerControlUiAndPairingSuite(): void { expect( (upgrade.error?.details as { code?: string; reason?: string } | undefined)?.reason, ).toBe("role-upgrade"); + expect( + ( + upgrade.error?.details as + | { + requestedRole?: string; + approvedRoles?: string[]; + } + | undefined + )?.requestedRole, + ).toBe("node"); + expect( + ( + upgrade.error?.details as + | { + requestedRole?: string; + approvedRoles?: string[]; + } + | undefined + )?.approvedRoles, + ).toEqual(["operator"]); const pending = (await listDevicePairing()).pending.filter( (entry) => entry.deviceId === identity.deviceId, @@ -1285,6 +1403,54 @@ export function registerControlUiAndPairingSuite(): void { }); expect(upgraded.ok).toBe(false); expect(upgraded.error?.message ?? "").toContain("pairing required"); + expect( + ( + upgraded.error?.details as + | { + reason?: string; + requestedRole?: string; + requestedScopes?: string[]; + approvedScopes?: string[]; + } + | undefined + )?.reason, + ).toBe("scope-upgrade"); + expect( + ( + upgraded.error?.details as + | { + reason?: string; + requestedRole?: string; + requestedScopes?: string[]; + approvedScopes?: string[]; + } + | undefined + )?.requestedRole, + ).toBe("operator"); + expect( + ( + upgraded.error?.details as + | { + reason?: string; + requestedRole?: string; + requestedScopes?: string[]; + approvedScopes?: string[]; + } + | undefined + )?.requestedScopes, + ).toEqual(["operator.admin"]); + expect( + ( + upgraded.error?.details as + | { + reason?: string; + requestedRole?: string; + requestedScopes?: string[]; + approvedScopes?: string[]; + } + | undefined + )?.approvedScopes, + ).toEqual(["operator.read"]); wsUpgrade.close(); const pendingUpgrade = (await listDevicePairing()).pending.find( diff --git a/src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts b/src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts index 425fe761934..d01e7ef92af 100644 --- a/src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts +++ b/src/gateway/server.silent-scope-upgrade-reconnect.poc.test.ts @@ -37,13 +37,68 @@ async function expectRejectedScopeUpgradeAttempt({ requestId?: unknown; reason?: unknown; remediationHint?: unknown; + requestedRole?: unknown; + requestedScopes?: unknown; + approvedScopes?: unknown; } ).requestId, ).toBe(pending.pending[0]?.requestId); - expect(((attempt.error?.details ?? {}) as { reason?: unknown }).reason).toBe("scope-upgrade"); - expect(((attempt.error?.details ?? {}) as { remediationHint?: unknown }).remediationHint).toBe( - "Review the requested scopes, then approve the pending upgrade.", - ); + expect( + ( + (attempt.error?.details ?? {}) as { + requestId?: unknown; + reason?: unknown; + requestedRole?: unknown; + requestedScopes?: unknown; + approvedScopes?: unknown; + } + ).reason, + ).toBe("scope-upgrade"); + expect( + ( + (attempt.error?.details ?? {}) as { + requestId?: unknown; + reason?: unknown; + requestedRole?: unknown; + requestedScopes?: unknown; + approvedScopes?: unknown; + } + ).requestedRole, + ).toBe("operator"); + expect( + ( + (attempt.error?.details ?? {}) as { + requestId?: unknown; + reason?: unknown; + requestedRole?: unknown; + requestedScopes?: unknown; + approvedScopes?: unknown; + } + ).requestedScopes, + ).toEqual(["operator.admin"]); + expect( + ( + (attempt.error?.details ?? {}) as { + requestId?: unknown; + reason?: unknown; + requestedRole?: unknown; + requestedScopes?: unknown; + approvedScopes?: unknown; + } + ).approvedScopes, + ).toEqual(["operator.read"]); + expect( + ( + (attempt.error?.details ?? {}) as { + requestId?: unknown; + reason?: unknown; + remediationHint?: unknown; + requestedRole?: unknown; + requestedScopes?: unknown; + approvedScopes?: unknown; + } + ).remediationHint, + ).toBe("Review the requested scopes, then approve the pending upgrade."); const requested = (await requestedEvent) as { payload?: { requestId?: string; deviceId?: string; scopes?: string[] }; diff --git a/src/gateway/server/ws-connection/message-handler.ts b/src/gateway/server/ws-connection/message-handler.ts index 6c16fd095b4..edddc8082d5 100644 --- a/src/gateway/server/ws-connection/message-handler.ts +++ b/src/gateway/server/ws-connection/message-handler.ts @@ -19,6 +19,7 @@ import { ensureDeviceToken, getPairedDevice, hasEffectivePairedDeviceRole, + listApprovedPairedDeviceRoles, listDevicePairing, listEffectivePairedDeviceRoles, requestDevicePairing, @@ -1006,9 +1007,25 @@ export function attachGatewayWsMessageHandler(params: { (approved?.status === "approved" || resolvedByConcurrentApproval) ) ) { + const exposeApprovedAccess = existingPairedDevice?.publicKey === devicePublicKey; + const approvedRoles = exposeApprovedAccess + ? listApprovedPairedDeviceRoles(existingPairedDevice) + : []; + const approvedScopes = exposeApprovedAccess + ? Array.isArray(existingPairedDevice.approvedScopes) + ? existingPairedDevice.approvedScopes + : Array.isArray(existingPairedDevice.scopes) + ? existingPairedDevice.scopes + : [] + : []; const pairingErrorDetails = buildPairingConnectErrorDetails({ reason, requestId: recoveryRequestId, + deviceId: device.id, + requestedRole: role, + requestedScopes: scopes, + ...(approvedRoles.length > 0 ? { approvedRoles } : {}), + ...(approvedScopes.length > 0 ? { approvedScopes } : {}), }); const pairingErrorMessage = buildPairingConnectErrorMessage(reason); setHandshakeState("failed");