mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 02:06:43 +00:00
fix(gateway): auto-approve trusted-proxy same-key device scope upgrades (#111916)
This commit is contained in:
@@ -411,7 +411,7 @@ describe("trusted-proxy browser device auto-approval", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
test("keeps scope upgrades on existing devices pending for manual approval", async () => {
|
||||
test("auto-approves same-key scope upgrades on existing devices", async () => {
|
||||
await writeGatewayAuthConfig({
|
||||
mode: "trusted-proxy",
|
||||
deviceAutoApprove: {
|
||||
@@ -431,18 +431,98 @@ describe("trusted-proxy browser device auto-approval", () => {
|
||||
identityPath,
|
||||
scopes: ["operator.read", "operator.write"],
|
||||
});
|
||||
expect(upgrade.ok).toBe(false);
|
||||
expect(upgrade.error?.message ?? "").toContain("pairing required");
|
||||
expect(upgrade.ok).toBe(true);
|
||||
});
|
||||
|
||||
const pairing = await listDevicePairing();
|
||||
const pending = pairing.pending.filter((entry) => entry.deviceId === identity.deviceId);
|
||||
expect(pending).toHaveLength(1);
|
||||
expect(pending[0]).toMatchObject({
|
||||
isRepair: true,
|
||||
scopes: ["operator.read", "operator.write"],
|
||||
expect(pairing.pending.filter((entry) => entry.deviceId === identity.deviceId)).toHaveLength(0);
|
||||
expect((await getPairedDevice(identity.deviceId))?.approvedScopes).toEqual([
|
||||
"operator.read",
|
||||
"operator.write",
|
||||
]);
|
||||
});
|
||||
|
||||
test("rejects a foreign-key connect for an already-paired deviceId", async () => {
|
||||
await writeGatewayAuthConfig({
|
||||
mode: "trusted-proxy",
|
||||
deviceAutoApprove: {
|
||||
enabled: true,
|
||||
scopes: ["operator.read", "operator.write"],
|
||||
},
|
||||
});
|
||||
expect((await getPairedDevice(identity.deviceId))?.approvedScopes).toEqual(["operator.read"]);
|
||||
const identityPath = deviceIdentityPath("trusted-proxy-key-mismatch-victim");
|
||||
const identity = loadOrCreateDeviceIdentity({ path: identityPath });
|
||||
const attackerIdentity = loadOrCreateDeviceIdentity({
|
||||
path: deviceIdentityPath("trusted-proxy-key-mismatch-attacker"),
|
||||
});
|
||||
|
||||
await withGatewayServer(async ({ port }) => {
|
||||
const initial = await connectBrowser({ port, identityPath, scopes: ["operator.read"] });
|
||||
expect(initial.ok).toBe(true);
|
||||
|
||||
// Same deviceId, different key pair: a valid signature over the attacker
|
||||
// key must not ride the trusted-proxy upgrade lane.
|
||||
const ws = await openBrowserWs(port, trustedProxyHeaders());
|
||||
try {
|
||||
const nonce = await readConnectChallengeNonce(ws);
|
||||
if (!nonce) {
|
||||
throw new Error("missing connect.challenge nonce");
|
||||
}
|
||||
const signedAt = Date.now();
|
||||
const scopes = ["operator.read", "operator.write"];
|
||||
const payload = buildDeviceAuthPayloadV3({
|
||||
deviceId: identity.deviceId,
|
||||
clientId: CONTROL_UI_CLIENT.id,
|
||||
clientMode: CONTROL_UI_CLIENT.mode,
|
||||
role: "operator",
|
||||
scopes,
|
||||
signedAtMs: signedAt,
|
||||
token: null,
|
||||
nonce,
|
||||
platform: CONTROL_UI_CLIENT.platform,
|
||||
});
|
||||
const id = randomUUID();
|
||||
const response = onceMessage<{ type: "res"; id: string; ok: boolean }>(
|
||||
ws,
|
||||
(message) => message.type === "res" && message.id === id,
|
||||
);
|
||||
ws.send(
|
||||
JSON.stringify({
|
||||
type: "req",
|
||||
id,
|
||||
method: "connect",
|
||||
params: {
|
||||
minProtocol: PROTOCOL_VERSION,
|
||||
maxProtocol: PROTOCOL_VERSION,
|
||||
client: CONTROL_UI_CLIENT,
|
||||
caps: [],
|
||||
commands: [],
|
||||
role: "operator",
|
||||
scopes,
|
||||
device: {
|
||||
id: identity.deviceId,
|
||||
publicKey: publicKeyRawBase64UrlFromPem(attackerIdentity.publicKeyPem),
|
||||
signature: signDevicePayload(attackerIdentity.privateKeyPem, payload),
|
||||
signedAt,
|
||||
nonce,
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
expect((await response).ok).toBe(false);
|
||||
} finally {
|
||||
ws.close();
|
||||
}
|
||||
});
|
||||
|
||||
// The connect is rejected before any pending request exists: deviceId is
|
||||
// bound to the key fingerprint, so a foreign key cannot even enqueue a
|
||||
// repair, let alone ride the same-key upgrade lane.
|
||||
const pairing = await listDevicePairing();
|
||||
expect(pairing.pending.filter((entry) => entry.deviceId === identity.deviceId)).toEqual([]);
|
||||
const paired = await getPairedDevice(identity.deviceId);
|
||||
expect(paired?.approvedScopes).toEqual(["operator.read"]);
|
||||
expect(paired?.publicKey).toBe(publicKeyRawBase64UrlFromPem(identity.publicKeyPem));
|
||||
});
|
||||
|
||||
test("leaves trusted-proxy pairing unchanged when auto-approval is disabled", async () => {
|
||||
|
||||
@@ -202,9 +202,14 @@ export async function authorizeGatewayConnectDevice(
|
||||
const trustedProxyAutoApproveConfig =
|
||||
configSnapshot.gateway?.auth?.trustedProxy?.deviceAutoApprove;
|
||||
const trustedProxyUser = authResult.user?.trim();
|
||||
// A scope upgrade from a device whose paired public key matches the one
|
||||
// this connect just proved by signature is the same physical browser
|
||||
// behind the SSO proxy — auto-approvable like a first pairing. A key
|
||||
// mismatch stays a manual owner decision (possible deviceId squat).
|
||||
const isTrustedProxySameKeyUpgrade =
|
||||
reason === "scope-upgrade" && existingPairedDevice?.publicKey === devicePublicKey;
|
||||
const allowTrustedProxyDeviceAutoApproval =
|
||||
reason === "not-paired" &&
|
||||
!existingPairedDevice &&
|
||||
((reason === "not-paired" && !existingPairedDevice) || isTrustedProxySameKeyUpgrade) &&
|
||||
role === "operator" &&
|
||||
(isBrowserOperatorUi || isWebchat) &&
|
||||
authMethod === "trusted-proxy" &&
|
||||
@@ -294,7 +299,8 @@ export async function authorizeGatewayConnectDevice(
|
||||
allowControlUiOperatorBootstrapPairing,
|
||||
});
|
||||
const trustedProxyAutoApproveScopes =
|
||||
allowTrustedProxyDeviceAutoApproval && pairing.request.isRepair !== true
|
||||
allowTrustedProxyDeviceAutoApproval &&
|
||||
(pairing.request.isRepair !== true || isTrustedProxySameKeyUpgrade)
|
||||
? resolveTrustedProxyControlUiScopes({
|
||||
requestedScopes: resolveTrustedProxyDeviceAutoApproveScopes({
|
||||
requestedScopes: scopes,
|
||||
|
||||
@@ -494,7 +494,7 @@ describe("device pairing tokens", () => {
|
||||
).resolves.toEqual({ ok: true });
|
||||
});
|
||||
|
||||
test("caps trusted-proxy auto-approval for new devices and refuses repairs", async () => {
|
||||
test("caps trusted-proxy auto-approval for new devices and upgrades same-key re-requests", async () => {
|
||||
const baseDir = await makeDevicePairingDir();
|
||||
const initial = await requestDevicePairing(
|
||||
{
|
||||
@@ -523,7 +523,7 @@ describe("device pairing tokens", () => {
|
||||
approvedVia: "trusted-proxy",
|
||||
});
|
||||
|
||||
const repair = await requestDevicePairing(
|
||||
const upgrade = await requestDevicePairing(
|
||||
{
|
||||
deviceId: "browser-device-1",
|
||||
publicKey: "public-key-browser-1",
|
||||
@@ -532,6 +532,56 @@ describe("device pairing tokens", () => {
|
||||
},
|
||||
baseDir,
|
||||
);
|
||||
const upgraded = await approveDevicePairing(
|
||||
upgrade.request.requestId,
|
||||
{
|
||||
callerScopes: ["operator.read", "operator.write"],
|
||||
approvedVia: "trusted-proxy",
|
||||
autoApproveNewDeviceScopes: ["operator.read", "operator.write"],
|
||||
},
|
||||
baseDir,
|
||||
);
|
||||
expectRecordFields(upgraded, "trusted-proxy upgrade result", {
|
||||
status: "approved",
|
||||
requestId: upgrade.request.requestId,
|
||||
});
|
||||
expect((await listDevicePairing(baseDir)).pending).toEqual([]);
|
||||
expect((await getPairedDevice("browser-device-1", baseDir))?.approvedScopes).toEqual([
|
||||
"operator.read",
|
||||
"operator.write",
|
||||
]);
|
||||
});
|
||||
|
||||
test("refuses trusted-proxy auto-approval when the pending key mismatches the paired device", async () => {
|
||||
const baseDir = await makeDevicePairingDir();
|
||||
const initial = await requestDevicePairing(
|
||||
{
|
||||
deviceId: "browser-device-2",
|
||||
publicKey: "public-key-browser-2",
|
||||
role: "operator",
|
||||
scopes: ["operator.read"],
|
||||
},
|
||||
baseDir,
|
||||
);
|
||||
await approveDevicePairing(
|
||||
initial.request.requestId,
|
||||
{
|
||||
callerScopes: ["operator.read"],
|
||||
approvedVia: "trusted-proxy",
|
||||
autoApproveNewDeviceScopes: ["operator.read"],
|
||||
},
|
||||
baseDir,
|
||||
);
|
||||
|
||||
const repair = await requestDevicePairing(
|
||||
{
|
||||
deviceId: "browser-device-2",
|
||||
publicKey: "public-key-browser-2-rotated",
|
||||
role: "operator",
|
||||
scopes: ["operator.read", "operator.write"],
|
||||
},
|
||||
baseDir,
|
||||
);
|
||||
await expect(
|
||||
approveDevicePairing(
|
||||
repair.request.requestId,
|
||||
@@ -543,11 +593,56 @@ describe("device pairing tokens", () => {
|
||||
baseDir,
|
||||
),
|
||||
).resolves.toBeNull();
|
||||
|
||||
expect((await listDevicePairing(baseDir)).pending).toContainEqual(
|
||||
expect.objectContaining({ requestId: repair.request.requestId, isRepair: true }),
|
||||
);
|
||||
expect((await getPairedDevice("browser-device-1", baseDir))?.approvedScopes).toEqual([
|
||||
expect((await getPairedDevice("browser-device-2", baseDir))?.approvedScopes).toEqual([
|
||||
"operator.read",
|
||||
]);
|
||||
});
|
||||
|
||||
test("refuses non-trusted-proxy auto-approval for a known device even with a matching key", async () => {
|
||||
const baseDir = await makeDevicePairingDir();
|
||||
const initial = await requestDevicePairing(
|
||||
{
|
||||
deviceId: "browser-device-3",
|
||||
publicKey: "public-key-browser-3",
|
||||
role: "operator",
|
||||
scopes: ["operator.read"],
|
||||
},
|
||||
baseDir,
|
||||
);
|
||||
await approveDevicePairing(
|
||||
initial.request.requestId,
|
||||
{
|
||||
callerScopes: ["operator.read"],
|
||||
approvedVia: "trusted-proxy",
|
||||
autoApproveNewDeviceScopes: ["operator.read"],
|
||||
},
|
||||
baseDir,
|
||||
);
|
||||
|
||||
const upgrade = await requestDevicePairing(
|
||||
{
|
||||
deviceId: "browser-device-3",
|
||||
publicKey: "public-key-browser-3",
|
||||
role: "operator",
|
||||
scopes: ["operator.read", "operator.write"],
|
||||
},
|
||||
baseDir,
|
||||
);
|
||||
await expect(
|
||||
approveDevicePairing(
|
||||
upgrade.request.requestId,
|
||||
{
|
||||
callerScopes: ["operator.read", "operator.write"],
|
||||
approvedVia: "silent",
|
||||
autoApproveNewDeviceScopes: ["operator.read", "operator.write"],
|
||||
},
|
||||
baseDir,
|
||||
),
|
||||
).resolves.toBeNull();
|
||||
expect((await getPairedDevice("browser-device-3", baseDir))?.approvedScopes).toEqual([
|
||||
"operator.read",
|
||||
]);
|
||||
});
|
||||
|
||||
@@ -792,9 +792,11 @@ export async function approveDevicePairing(
|
||||
"owner" | "silent" | "trusted-cidr" | "trusted-proxy" | "ssh-verified"
|
||||
>;
|
||||
/**
|
||||
* 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.
|
||||
* Replace the pending scopes only for a brand-new operator device, or — under
|
||||
* trusted-proxy approval — for a known operator device re-requesting with its
|
||||
* already-paired public key. 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[];
|
||||
},
|
||||
@@ -828,10 +830,20 @@ export async function approveDevicePairing(
|
||||
}
|
||||
const autoApproveScopes = options?.autoApproveNewDeviceScopes;
|
||||
const requestedRoles = resolveRequestedRoles(pendingRecord);
|
||||
const knownDevice = state.pairedByDeviceId[pendingRecord.deviceId];
|
||||
// Trusted-proxy connects carry an SSO-authenticated user, and the connect
|
||||
// handshake has already proven possession of the pending public key. A
|
||||
// matching key on the paired record is therefore the same physical device
|
||||
// re-requesting (typically a scope upgrade) and may auto-approve; a key
|
||||
// mismatch is a real repair — possibly a deviceId squat — and stays a
|
||||
// manual owner decision.
|
||||
const trustedProxySameKeyDevice =
|
||||
options?.approvedVia === "trusted-proxy" &&
|
||||
knownDevice !== undefined &&
|
||||
knownDevice.publicKey === pendingRecord.publicKey;
|
||||
if (
|
||||
autoApproveScopes &&
|
||||
(pendingRecord.isRepair ||
|
||||
state.pairedByDeviceId[pendingRecord.deviceId] ||
|
||||
(((pendingRecord.isRepair || knownDevice) && !trustedProxySameKeyDevice) ||
|
||||
!sameStringSet(requestedRoles, [OPERATOR_ROLE]))
|
||||
) {
|
||||
return null;
|
||||
|
||||
Reference in New Issue
Block a user