mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-09 15:59:30 +00:00
fix: surface node approval guidance from devices CLI (#98115)
* fix: surface node approval guidance from devices CLI * fix: preserve node approval connection guidance * fix: handle admin retry unknown device approvals * fix: require stable node approval matches --------- Co-authored-by: welfo-beo <187608477+welfo-beo@users.noreply.github.com> Co-authored-by: welfo-beo <welfo-beo@users.noreply.github.com>
This commit is contained in:
welfo-beo
GitHub
parent
490a4c5d9b
commit
fc97e92ddd
2 changed files with 428 additions and 7 deletions
|
|
@ -21,7 +21,12 @@ import {
|
|||
callGateway,
|
||||
formatGatewayTransportErrorJson,
|
||||
} from "../gateway/call.js";
|
||||
import { ADMIN_SCOPE, PAIRING_SCOPE, type OperatorScope } from "../gateway/method-scopes.js";
|
||||
import {
|
||||
ADMIN_SCOPE,
|
||||
PAIRING_SCOPE,
|
||||
READ_SCOPE,
|
||||
type OperatorScope,
|
||||
} from "../gateway/method-scopes.js";
|
||||
import { isLoopbackHost } from "../gateway/net.js";
|
||||
import {
|
||||
approveDevicePairing,
|
||||
|
|
@ -38,6 +43,8 @@ import {
|
|||
type DevicePairingAccessSummary,
|
||||
type PendingDeviceApprovalKind,
|
||||
} from "../shared/device-pairing-access.js";
|
||||
import { parseNodeList } from "../shared/node-list-parse.js";
|
||||
import type { NodeListNode } from "../shared/node-list-types.js";
|
||||
import { formatCliCommand } from "./command-format.js";
|
||||
import { parseTimeoutMsWithFallback } from "./parse-timeout.js";
|
||||
import { withProgress } from "./progress.js";
|
||||
|
|
@ -82,6 +89,7 @@ type PairedDevice = {
|
|||
deviceId: string;
|
||||
publicKey?: string;
|
||||
displayName?: string;
|
||||
role?: string;
|
||||
roles?: string[];
|
||||
scopes?: string[];
|
||||
remoteIp?: string;
|
||||
|
|
@ -100,6 +108,12 @@ type ApprovePairingGatewayContext = {
|
|||
scopes?: OperatorScope[];
|
||||
};
|
||||
|
||||
type PendingNodeApprovalNotice = {
|
||||
node: NodeListNode;
|
||||
command: string;
|
||||
connectionReminder: string | null;
|
||||
};
|
||||
|
||||
const FALLBACK_NOTICE = "Direct scope access failed; using local fallback.";
|
||||
const DEFAULT_DEVICES_TIMEOUT_MS = 10_000;
|
||||
const FALLBACK_STATE_MISMATCH_MESSAGE =
|
||||
|
|
@ -140,6 +154,147 @@ const callGatewayCli = async (
|
|||
}),
|
||||
);
|
||||
|
||||
function isPendingNodeApprovalState(
|
||||
state: unknown,
|
||||
): state is "pending-approval" | "pending-reapproval" {
|
||||
return state === "pending-approval" || state === "pending-reapproval";
|
||||
}
|
||||
|
||||
function buildNodeApproveCommand(opts: DevicesRpcOpts, requestId: string): string {
|
||||
const args = ["openclaw", "nodes", "approve", requestId];
|
||||
const timeout = normalizeOptionalString(opts.timeout);
|
||||
if (timeout && timeout !== String(DEFAULT_DEVICES_TIMEOUT_MS)) {
|
||||
args.push("--timeout", timeout);
|
||||
}
|
||||
return formatCliCommand(args.map(quoteCliArg).join(" "));
|
||||
}
|
||||
|
||||
function formatNodeConnectionFlagReminder(opts: DevicesRpcOpts): string | null {
|
||||
const flags = [
|
||||
normalizeOptionalString(opts.url) ? "--url" : null,
|
||||
normalizeOptionalString(opts.token) ? "--token" : null,
|
||||
].filter((flag) => flag !== null);
|
||||
return flags.length > 0
|
||||
? `Reuse the same connection option${flags.length === 1 ? "" : "s"} when rerunning: ${flags.join(", ")}.`
|
||||
: null;
|
||||
}
|
||||
|
||||
async function tryReadPendingNodeApprovals(opts: DevicesRpcOpts): Promise<NodeListNode[]> {
|
||||
try {
|
||||
return parseNodeList(
|
||||
await callGatewayCli("node.list", opts, {}, { scopes: [READ_SCOPE, PAIRING_SCOPE] }),
|
||||
).filter(
|
||||
(node) =>
|
||||
isPendingNodeApprovalState(node.approvalState) &&
|
||||
Boolean(normalizeOptionalString(node.pendingRequestId)),
|
||||
);
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
function pairedDeviceCanBeNode(device: PairedDevice): boolean {
|
||||
return [device.role, ...(device.roles ?? [])].some(
|
||||
(role) => normalizeOptionalString(role) === "node",
|
||||
);
|
||||
}
|
||||
|
||||
function stringsMatch(left: unknown, right: unknown): boolean {
|
||||
const normalizedLeft = normalizeOptionalString(left);
|
||||
const normalizedRight = normalizeOptionalString(right);
|
||||
return Boolean(normalizedLeft && normalizedRight && normalizedLeft === normalizedRight);
|
||||
}
|
||||
|
||||
function nodeMatchesPairedDevice(node: NodeListNode, device: PairedDevice): boolean {
|
||||
return stringsMatch(node.nodeId, device.deviceId) || stringsMatch(node.remoteIp, device.remoteIp);
|
||||
}
|
||||
|
||||
function nodeMatchesQuery(node: NodeListNode, query: string): boolean {
|
||||
return (
|
||||
stringsMatch(node.nodeId, query) ||
|
||||
stringsMatch(node.remoteIp, query) ||
|
||||
stringsMatch(node.pendingRequestId, query)
|
||||
);
|
||||
}
|
||||
|
||||
function pairedDeviceMatchesQuery(device: PairedDevice, query: string): boolean {
|
||||
return stringsMatch(device.deviceId, query) || stringsMatch(device.remoteIp, query);
|
||||
}
|
||||
|
||||
async function tryReadGatewayPairingList(opts: DevicesRpcOpts): Promise<DevicePairingList | null> {
|
||||
try {
|
||||
return parseDevicePairingList(
|
||||
await callGatewayCli("device.pair.list", opts, {}, { scopes: [PAIRING_SCOPE] }),
|
||||
);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function buildPendingNodeApprovalNoticesForOpts(
|
||||
nodes: NodeListNode[],
|
||||
opts: DevicesRpcOpts,
|
||||
): PendingNodeApprovalNotice[] {
|
||||
return nodes.flatMap((node) => {
|
||||
const requestId = normalizeOptionalString(node.pendingRequestId);
|
||||
return requestId
|
||||
? [
|
||||
{
|
||||
node,
|
||||
command: buildNodeApproveCommand(opts, requestId),
|
||||
connectionReminder: formatNodeConnectionFlagReminder(opts),
|
||||
},
|
||||
]
|
||||
: [];
|
||||
});
|
||||
}
|
||||
|
||||
function formatNodeApprovalNotice(notice: PendingNodeApprovalNotice): string {
|
||||
const action = notice.node.approvalState === "pending-reapproval" ? "reapproval" : "approval";
|
||||
const label = sanitizeForLog(
|
||||
normalizeOptionalString(notice.node.displayName) ?? notice.node.nodeId,
|
||||
);
|
||||
const lines = [`Node ${action} pending for ${label}. Run ${sanitizeForLog(notice.command)}`];
|
||||
if (notice.connectionReminder) {
|
||||
lines.push(notice.connectionReminder);
|
||||
}
|
||||
return lines.join("\n");
|
||||
}
|
||||
|
||||
async function findPairedDevicePendingNodeApprovalNotices(
|
||||
opts: DevicesRpcOpts,
|
||||
paired: PairedDevice[] | undefined,
|
||||
): Promise<PendingNodeApprovalNotice[]> {
|
||||
const nodeDevices = (paired ?? []).filter(pairedDeviceCanBeNode);
|
||||
if (nodeDevices.length === 0) {
|
||||
return [];
|
||||
}
|
||||
const nodes = await tryReadPendingNodeApprovals(opts);
|
||||
return buildPendingNodeApprovalNoticesForOpts(
|
||||
nodes.filter((node) => nodeDevices.some((device) => nodeMatchesPairedDevice(node, device))),
|
||||
opts,
|
||||
);
|
||||
}
|
||||
|
||||
async function findQueryPendingNodeApprovalNotices(
|
||||
opts: DevicesRpcOpts,
|
||||
query: string,
|
||||
): Promise<PendingNodeApprovalNotice[]> {
|
||||
const nodes = await tryReadPendingNodeApprovals(opts);
|
||||
const directMatches = nodes.filter((node) => nodeMatchesQuery(node, query));
|
||||
if (directMatches.length > 0) {
|
||||
return buildPendingNodeApprovalNoticesForOpts(directMatches, opts);
|
||||
}
|
||||
const pairingList = await tryReadGatewayPairingList(opts);
|
||||
const pairedMatches = (pairingList?.paired ?? []).filter((device) =>
|
||||
pairedDeviceMatchesQuery(device, query),
|
||||
);
|
||||
return buildPendingNodeApprovalNoticesForOpts(
|
||||
nodes.filter((node) => pairedMatches.some((device) => nodeMatchesPairedDevice(node, device))),
|
||||
opts,
|
||||
);
|
||||
}
|
||||
|
||||
function normalizeErrorMessage(error: unknown): string {
|
||||
if (error instanceof Error) {
|
||||
return error.message;
|
||||
|
|
@ -153,6 +308,22 @@ function isDevicePairingApprovalDenied(error: unknown): boolean {
|
|||
);
|
||||
}
|
||||
|
||||
function isUnknownRequestIdError(error: unknown): boolean {
|
||||
const maybeGatewayError =
|
||||
typeof error === "object" && error !== null
|
||||
? (error as { gatewayCode?: unknown; message?: unknown })
|
||||
: undefined;
|
||||
const gatewayCode = maybeGatewayError?.gatewayCode;
|
||||
if (gatewayCode !== undefined && gatewayCode !== "INVALID_REQUEST") {
|
||||
return false;
|
||||
}
|
||||
const message =
|
||||
typeof maybeGatewayError?.message === "string"
|
||||
? maybeGatewayError.message
|
||||
: normalizeErrorMessage(error);
|
||||
return normalizeLowercaseStringOrEmpty(message).includes("unknown requestid");
|
||||
}
|
||||
|
||||
function resolveLocalPairingFallback(
|
||||
opts: DevicesRpcOpts,
|
||||
error: unknown,
|
||||
|
|
@ -250,15 +421,25 @@ async function approvePairingWithFallback(
|
|||
);
|
||||
} catch (error) {
|
||||
if (isDevicePairingApprovalDenied(error) && !scopes?.includes(ADMIN_SCOPE)) {
|
||||
return await callGatewayCli(
|
||||
"device.pair.approve",
|
||||
opts,
|
||||
{ requestId },
|
||||
{ scopes: [ADMIN_SCOPE] },
|
||||
);
|
||||
try {
|
||||
return await callGatewayCli(
|
||||
"device.pair.approve",
|
||||
opts,
|
||||
{ requestId },
|
||||
{ scopes: [ADMIN_SCOPE] },
|
||||
);
|
||||
} catch (adminError) {
|
||||
if (isUnknownRequestIdError(adminError)) {
|
||||
return null;
|
||||
}
|
||||
throw adminError;
|
||||
}
|
||||
}
|
||||
const fallback = resolveLocalPairingFallback(opts, error);
|
||||
if (!fallback) {
|
||||
if (isUnknownRequestIdError(error)) {
|
||||
return null;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
const gatewayRequestId = normalizeOptionalString(fallback.details.requestId);
|
||||
|
|
@ -762,6 +943,10 @@ export async function runDevicesListCommand(opts: DevicesRpcOpts): Promise<void>
|
|||
})),
|
||||
}).trimEnd(),
|
||||
);
|
||||
const nodeApprovalNotices = await findPairedDevicePendingNodeApprovalNotices(opts, list.paired);
|
||||
for (const notice of nodeApprovalNotices) {
|
||||
defaultRuntime.log(theme.warn(formatNodeApprovalNotice(notice)));
|
||||
}
|
||||
}
|
||||
if (!list.pending?.length && !list.paired?.length) {
|
||||
defaultRuntime.log(theme.muted("No device pairing entries."));
|
||||
|
|
@ -920,6 +1105,10 @@ export async function runDevicesApproveCommand(
|
|||
const result = await approvePairingWithFallback(opts, resolvedRequestId);
|
||||
if (!result) {
|
||||
defaultRuntime.error("unknown requestId");
|
||||
const nodeApprovalNotices = await findQueryPendingNodeApprovalNotices(opts, resolvedRequestId);
|
||||
for (const notice of nodeApprovalNotices) {
|
||||
defaultRuntime.error(formatNodeApprovalNotice(notice));
|
||||
}
|
||||
defaultRuntime.exit(1);
|
||||
return;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -485,6 +485,159 @@ describe("devices cli approve", () => {
|
|||
expect(runtime.exit).toHaveBeenCalledWith(1);
|
||||
expect(hasGatewayMethod("device.pair.approve")).toBe(false);
|
||||
});
|
||||
|
||||
it("suggests pending node approval when a device IP is approved at the wrong layer", async () => {
|
||||
callGateway
|
||||
.mockResolvedValueOnce({
|
||||
pending: [],
|
||||
paired: [
|
||||
pairedDevice({
|
||||
deviceId: "android-node",
|
||||
displayName: "Colin's S25",
|
||||
remoteIp: "192.168.0.202",
|
||||
roles: ["node"],
|
||||
}),
|
||||
],
|
||||
})
|
||||
.mockRejectedValueOnce(new Error("device pairing approval denied"))
|
||||
.mockRejectedValueOnce({ message: "unknown requestId", gatewayCode: "INVALID_REQUEST" })
|
||||
.mockResolvedValueOnce({
|
||||
nodes: [
|
||||
{
|
||||
nodeId: "android-node",
|
||||
displayName: "Colin's S25",
|
||||
approvalState: "pending-reapproval",
|
||||
pendingRequestId: "node-req-1",
|
||||
},
|
||||
],
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
pending: [],
|
||||
paired: [
|
||||
pairedDevice({
|
||||
deviceId: "android-node",
|
||||
displayName: "Colin's S25",
|
||||
remoteIp: "192.168.0.202",
|
||||
roles: ["node"],
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
await runDevicesApprove([
|
||||
"192.168.0.202",
|
||||
"--url",
|
||||
"ws://gateway-user:url-secret@gateway.example:18789/openclaw?cluster=qa",
|
||||
"--token",
|
||||
"secret-token",
|
||||
]);
|
||||
|
||||
expectGatewayCall(3, { method: "node.list" });
|
||||
expectGatewayCall(4, { method: "device.pair.list" });
|
||||
const errorOutput = readRuntimeErrorOutput();
|
||||
expect(errorOutput).toContain("unknown requestId");
|
||||
expect(errorOutput).toContain("Node reapproval pending for Colin's S25");
|
||||
expect(errorOutput).toContain("openclaw nodes approve node-req-1");
|
||||
expect(errorOutput).toContain(
|
||||
"Reuse the same connection options when rerunning: --url, --token.",
|
||||
);
|
||||
expect(errorOutput).not.toContain("gateway-user");
|
||||
expect(errorOutput).not.toContain("url-secret");
|
||||
expect(errorOutput).not.toContain("gateway.example");
|
||||
expect(errorOutput).not.toContain("secret-token");
|
||||
expect(runtime.exit).toHaveBeenCalledWith(1);
|
||||
});
|
||||
|
||||
it("does not suggest node approval for a wrong-layer device IP when only display names match", async () => {
|
||||
callGateway
|
||||
.mockResolvedValueOnce({
|
||||
pending: [],
|
||||
paired: [
|
||||
pairedDevice({
|
||||
deviceId: "android-node",
|
||||
displayName: "Shared Phone",
|
||||
remoteIp: "192.168.0.202",
|
||||
roles: ["node"],
|
||||
}),
|
||||
],
|
||||
})
|
||||
.mockRejectedValueOnce(new Error("device pairing approval denied"))
|
||||
.mockRejectedValueOnce({ message: "unknown requestId", gatewayCode: "INVALID_REQUEST" })
|
||||
.mockResolvedValueOnce({
|
||||
nodes: [
|
||||
{
|
||||
nodeId: "unrelated-node",
|
||||
displayName: "Shared Phone",
|
||||
remoteIp: "10.0.0.50",
|
||||
approvalState: "pending-reapproval",
|
||||
pendingRequestId: "node-req-unrelated",
|
||||
},
|
||||
],
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
pending: [],
|
||||
paired: [
|
||||
pairedDevice({
|
||||
deviceId: "android-node",
|
||||
displayName: "Shared Phone",
|
||||
remoteIp: "192.168.0.202",
|
||||
roles: ["node"],
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
await runDevicesApprove(["192.168.0.202"]);
|
||||
|
||||
expectGatewayCall(3, { method: "node.list" });
|
||||
expectGatewayCall(4, { method: "device.pair.list" });
|
||||
const errorOutput = readRuntimeErrorOutput();
|
||||
expect(errorOutput).toContain("unknown requestId");
|
||||
expect(errorOutput).not.toContain("node-req-unrelated");
|
||||
expect(errorOutput).not.toContain("openclaw nodes approve");
|
||||
});
|
||||
|
||||
it("does not suggest node approval when the query only matches a paired device display name", async () => {
|
||||
callGateway
|
||||
.mockResolvedValueOnce({
|
||||
pending: [],
|
||||
paired: [
|
||||
pairedDevice({
|
||||
deviceId: "paired-node",
|
||||
displayName: "Shared Phone",
|
||||
roles: ["node"],
|
||||
}),
|
||||
],
|
||||
})
|
||||
.mockRejectedValueOnce({ message: "unknown requestId", gatewayCode: "INVALID_REQUEST" })
|
||||
.mockResolvedValueOnce({
|
||||
nodes: [
|
||||
{
|
||||
nodeId: "paired-node",
|
||||
displayName: "Shared Phone",
|
||||
approvalState: "pending-approval",
|
||||
pendingRequestId: "node-req-display-name",
|
||||
},
|
||||
],
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
pending: [],
|
||||
paired: [
|
||||
pairedDevice({
|
||||
deviceId: "paired-node",
|
||||
displayName: "Shared Phone",
|
||||
roles: ["node"],
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
await runDevicesApprove(["Shared Phone"]);
|
||||
|
||||
expectGatewayCall(2, { method: "node.list" });
|
||||
expectGatewayCall(3, { method: "device.pair.list" });
|
||||
const errorOutput = readRuntimeErrorOutput();
|
||||
expect(errorOutput).toContain("unknown requestId");
|
||||
expect(errorOutput).not.toContain("node-req-display-name");
|
||||
expect(errorOutput).not.toContain("openclaw nodes approve");
|
||||
});
|
||||
});
|
||||
|
||||
describe("devices cli remove", () => {
|
||||
|
|
@ -1337,6 +1490,85 @@ describe("devices cli list", () => {
|
|||
expect(output).toContain("operator.read");
|
||||
});
|
||||
|
||||
it("shows pending node approval commands for paired node devices", async () => {
|
||||
callGateway
|
||||
.mockResolvedValueOnce({
|
||||
pending: [],
|
||||
paired: [
|
||||
pairedDevice({
|
||||
deviceId: "android-node",
|
||||
displayName: "Colin's S25",
|
||||
remoteIp: "192.168.0.202",
|
||||
role: "node",
|
||||
roles: [],
|
||||
}),
|
||||
],
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
nodes: [
|
||||
{
|
||||
nodeId: "android-node",
|
||||
displayName: "Colin's S25",
|
||||
remoteIp: "192.168.0.202",
|
||||
approvalState: "pending-reapproval",
|
||||
pendingRequestId: "node-req-1",
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await runDevicesCommand([
|
||||
"list",
|
||||
"--url",
|
||||
"ws://gateway-user:url-secret@gateway.example:18789/openclaw?cluster=qa",
|
||||
"--token",
|
||||
"secret-token",
|
||||
]);
|
||||
|
||||
expectGatewayCall(1, { method: "node.list" });
|
||||
const output = readRuntimeOutput();
|
||||
expect(output).toContain("Node reapproval pending for Colin's S25");
|
||||
expect(output).toContain("openclaw nodes approve node-req-1");
|
||||
expect(output).toContain("Reuse the same connection options when rerunning: --url, --token.");
|
||||
expect(output).not.toContain("gateway-user");
|
||||
expect(output).not.toContain("url-secret");
|
||||
expect(output).not.toContain("gateway.example");
|
||||
expect(output).not.toContain("secret-token");
|
||||
});
|
||||
|
||||
it("does not show node approval commands for paired node devices when only display names match", async () => {
|
||||
callGateway
|
||||
.mockResolvedValueOnce({
|
||||
pending: [],
|
||||
paired: [
|
||||
pairedDevice({
|
||||
deviceId: "android-node",
|
||||
displayName: "Shared Phone",
|
||||
remoteIp: "192.168.0.202",
|
||||
role: "node",
|
||||
roles: [],
|
||||
}),
|
||||
],
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
nodes: [
|
||||
{
|
||||
nodeId: "unrelated-node",
|
||||
displayName: "Shared Phone",
|
||||
remoteIp: "10.0.0.50",
|
||||
approvalState: "pending-reapproval",
|
||||
pendingRequestId: "node-req-unrelated",
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await runDevicesCommand(["list"]);
|
||||
|
||||
expectGatewayCall(1, { method: "node.list" });
|
||||
const output = readRuntimeOutput();
|
||||
expect(output).not.toContain("node-req-unrelated");
|
||||
expect(output).not.toContain("openclaw nodes approve");
|
||||
});
|
||||
|
||||
it("does not show upgrade context for key-mismatched pending requests", async () => {
|
||||
mockGatewayPairingList({ publicKey: "new-key" }, { publicKey: "old-key" });
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue