diff --git a/src/gateway/server.auth.trusted-proxy-device-autoapprove.test.ts b/src/gateway/server.auth.trusted-proxy-device-autoapprove.test.ts index 274c1355bcb6..857784e24afb 100644 --- a/src/gateway/server.auth.trusted-proxy-device-autoapprove.test.ts +++ b/src/gateway/server.auth.trusted-proxy-device-autoapprove.test.ts @@ -12,7 +12,7 @@ import { } from "../infra/device-identity.js"; import { getPairedDevice, listDevicePairing } from "../infra/device-pairing.js"; import { buildDeviceAuthPayloadV3 } from "./device-auth.js"; -import { CONTROL_UI_CLIENT } from "./server.auth.test-helpers.js"; +import { CONTROL_UI_CLIENT, NODE_CLIENT } from "./server.auth.test-helpers.js"; import { connectReq, installGatewayTestHooks, @@ -230,6 +230,43 @@ describe("trusted-proxy browser device auto-approval", () => { ]); }); + test("leaves mixed node and operator requests pending for manual approval", async () => { + await writeGatewayAuthConfig({ + mode: "trusted-proxy", + deviceAutoApprove: { enabled: true, scopes: ["operator.read"] }, + }); + const identityPath = deviceIdentityPath("trusted-proxy-mixed-role"); + const identity = loadOrCreateDeviceIdentity({ path: identityPath }); + + await withGatewayServer(async ({ port }) => { + const nodeWs = await openBrowserWs(port, trustedProxyHeaders()); + try { + const nodeRes = await connectReq(nodeWs, { + skipDefaultAuth: true, + client: NODE_CLIENT, + role: "node", + scopes: [], + deviceIdentityPath: identityPath, + }); + expect(nodeRes.ok).toBe(false); + } finally { + nodeWs.close(); + } + + const browserRes = await connectBrowser({ + port, + identityPath, + scopes: ["operator.read"], + }); + expect(browserRes.ok).toBe(false); + }); + + await expect(getPairedDevice(identity.deviceId)).resolves.toBeNull(); + expect( + (await listDevicePairing()).pending.find((entry) => entry.deviceId === identity.deviceId), + ).toMatchObject({ roles: ["node", "operator"] }); + }); + test("caps omitted scopes to the declared proxy scopes", async () => { await writeGatewayAuthConfig({ mode: "trusted-proxy", diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts index b37bc00bbb6d..430fa88a3f0d 100644 --- a/src/infra/device-pairing.test.ts +++ b/src/infra/device-pairing.test.ts @@ -552,6 +552,49 @@ describe("device pairing tokens", () => { ]); }); + test("refuses trusted-proxy auto-approval for a merged node and operator request", async () => { + const baseDir = await makeDevicePairingDir(); + await requestDevicePairing( + { + deviceId: "mixed-role-device-1", + publicKey: "public-key-mixed-role-1", + role: "node", + scopes: [], + }, + baseDir, + ); + const browser = await requestDevicePairing( + { + deviceId: "mixed-role-device-1", + publicKey: "public-key-mixed-role-1", + role: "operator", + scopes: ["operator.read"], + }, + baseDir, + ); + expect(browser.request.roles).toEqual(["node", "operator"]); + + await expect( + approveDevicePairing( + browser.request.requestId, + { + callerScopes: ["operator.read"], + approvedVia: "trusted-proxy", + autoApproveNewDeviceScopes: ["operator.read"], + }, + baseDir, + ), + ).resolves.toBeNull(); + + await expect(getPairedDevice("mixed-role-device-1", baseDir)).resolves.toBeNull(); + expect((await listDevicePairing(baseDir)).pending).toContainEqual( + expect.objectContaining({ + requestId: browser.request.requestId, + roles: ["node", "operator"], + }), + ); + }); + test.each([ { name: "node custom scope", diff --git a/src/infra/device-pairing.ts b/src/infra/device-pairing.ts index 31dfd3a93675..7f394999b6fd 100644 --- a/src/infra/device-pairing.ts +++ b/src/infra/device-pairing.ts @@ -792,9 +792,9 @@ export async function approveDevicePairing( "owner" | "silent" | "trusted-cidr" | "trusted-proxy" | "ssh-verified" >; /** - * Replace the pending scopes only if this is still a brand-new device. - * Used by trusted-proxy auto-approval to cap grants without ever approving - * an existing-device repair or upgrade request. + * Replace the pending scopes only if this is still a brand-new operator device. + * The live role set is rechecked under the pairing lock so a merged request + * cannot inherit non-operator access through browser auto-approval. */ autoApproveNewDeviceScopes?: readonly string[]; }, @@ -826,16 +826,19 @@ export async function approveDevicePairing( if (!pendingRecord) { return null; } + const autoApproveScopes = options?.autoApproveNewDeviceScopes; + const requestedRoles = resolveRequestedRoles(pendingRecord); if ( - options?.autoApproveNewDeviceScopes && - (pendingRecord.isRepair || state.pairedByDeviceId[pendingRecord.deviceId]) + autoApproveScopes && + (pendingRecord.isRepair || + state.pairedByDeviceId[pendingRecord.deviceId] || + !sameStringSet(requestedRoles, [OPERATOR_ROLE])) ) { return null; } - const pending = options?.autoApproveNewDeviceScopes - ? { ...pendingRecord, scopes: [...options.autoApproveNewDeviceScopes] } + const pending = autoApproveScopes + ? { ...pendingRecord, scopes: [...autoApproveScopes] } : pendingRecord; - const requestedRoles = mergeRoles(pending.roles, pending.role) ?? []; const requestedScopes = normalizeDeviceAuthScopes(pending.scopes); const roleMismatchScope = resolveScopeOutsideRequestedRoles({ requestedRoles,