mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-24 03:15:46 -06:00
fix(agents): canonicalize node binding selectors (#66985)
This commit is contained in:
@@ -255,16 +255,44 @@ export function formatNodeRunToolResult(params: {
|
||||
export async function resolveNodeExecutionTarget(
|
||||
params: ExecuteNodeHostCommandParams,
|
||||
): Promise<NodeExecutionTarget> {
|
||||
if (params.boundNode && params.requestedNode && params.boundNode !== params.requestedNode) {
|
||||
throw new Error(`exec node not allowed (bound to ${params.boundNode})`);
|
||||
}
|
||||
const nodeQuery = params.boundNode || params.requestedNode;
|
||||
const nodes = await listNodes({});
|
||||
if (nodes.length === 0) {
|
||||
throw new Error(
|
||||
"exec host=node requires a paired node (none available). This requires a companion app or node host.",
|
||||
);
|
||||
}
|
||||
// Canonicalize boundNode and requestedNode (which may be display names, IPs,
|
||||
// or partial ID prefixes) to full device IDs before comparing.
|
||||
let resolvedBoundNodeId: string | undefined;
|
||||
if (params.boundNode) {
|
||||
try {
|
||||
resolvedBoundNodeId = resolveNodeIdFromList(nodes, params.boundNode);
|
||||
} catch {
|
||||
// boundNode comes from config; if it cannot be resolved, fall through
|
||||
// to the existing nodeQuery resolution which produces a clearer error.
|
||||
}
|
||||
}
|
||||
let resolvedRequestedNodeId: string | undefined;
|
||||
if (params.requestedNode) {
|
||||
try {
|
||||
resolvedRequestedNodeId = resolveNodeIdFromList(nodes, params.requestedNode);
|
||||
} catch (err) {
|
||||
throw new Error(
|
||||
`requested node not found: ${params.requestedNode} (${err instanceof Error ? err.message : String(err)})`,
|
||||
{ cause: err },
|
||||
);
|
||||
}
|
||||
}
|
||||
const canonicalBound = resolvedBoundNodeId ?? params.boundNode;
|
||||
if (canonicalBound && resolvedRequestedNodeId && canonicalBound !== resolvedRequestedNodeId) {
|
||||
throw new Error(
|
||||
`exec node not allowed (bound to ${canonicalBound}, requested resolved to ${resolvedRequestedNodeId})`,
|
||||
);
|
||||
}
|
||||
// Prefer resolved IDs; fall back to raw boundNode so stale/unresolvable
|
||||
// values still reach resolveNodeIdFromList (which produces a clear
|
||||
// "unknown node" error) instead of silently picking a default node.
|
||||
const nodeQuery = resolvedBoundNodeId || resolvedRequestedNodeId || params.boundNode;
|
||||
let nodeId: string;
|
||||
try {
|
||||
nodeId = resolveNodeIdFromList(nodes, nodeQuery, !nodeQuery);
|
||||
|
||||
@@ -223,9 +223,35 @@ vi.mock("./tools/gateway.js", () => ({
|
||||
callGatewayTool: callGatewayToolMock,
|
||||
}));
|
||||
|
||||
const resolveNodeIdFromListMock = vi.hoisted(() =>
|
||||
vi.fn((nodes: Array<{ nodeId: string; displayName?: string }>, query?: string) => {
|
||||
if (!query) {
|
||||
if (nodes.length === 1) {
|
||||
return nodes[0].nodeId;
|
||||
}
|
||||
throw new Error("node required");
|
||||
}
|
||||
const byId = nodes.find((n) => n.nodeId === query);
|
||||
if (byId) {
|
||||
return byId.nodeId;
|
||||
}
|
||||
const byName = nodes.find((n) => n.displayName === query);
|
||||
if (byName) {
|
||||
return byName.nodeId;
|
||||
}
|
||||
if (query.length >= 6) {
|
||||
const byPrefix = nodes.find((n) => n.nodeId.startsWith(query));
|
||||
if (byPrefix) {
|
||||
return byPrefix.nodeId;
|
||||
}
|
||||
}
|
||||
throw new Error(`unknown node: ${query}`);
|
||||
}),
|
||||
);
|
||||
|
||||
vi.mock("./tools/nodes-utils.js", () => ({
|
||||
listNodes: listNodesMock,
|
||||
resolveNodeIdFromList: vi.fn(() => "node-1"),
|
||||
resolveNodeIdFromList: resolveNodeIdFromListMock,
|
||||
}));
|
||||
|
||||
vi.mock("../logger.js", () => ({
|
||||
@@ -2659,6 +2685,145 @@ describe("executeNodeHostCommand", () => {
|
||||
expectSystemRunInvoke({ invokeTimeoutMs: 35_000, runTimeoutMs: 0 });
|
||||
});
|
||||
|
||||
it("allows exec when requestedNode is display name matching boundNode's device", async () => {
|
||||
listNodesMock.mockResolvedValue([
|
||||
{
|
||||
nodeId: "f2396b588d391d30a79d300e196a17cf197f34969b5e2485d2734c953567f44e",
|
||||
displayName: "home-wsl-debian",
|
||||
commands: ["system.run"],
|
||||
platform: process.platform,
|
||||
},
|
||||
]);
|
||||
const result = await executeNodeHostCommand({
|
||||
command: "echo hello",
|
||||
workdir: "/tmp/work",
|
||||
env: {},
|
||||
security: "full",
|
||||
ask: "off",
|
||||
defaultTimeoutSec: 30,
|
||||
approvalRunningNoticeMs: 0,
|
||||
warnings: [],
|
||||
agentId: "test-agent",
|
||||
sessionKey: "test-session",
|
||||
boundNode: "f2396b588d391d30a79d300e196a17cf197f34969b5e2485d2734c953567f44e",
|
||||
requestedNode: "home-wsl-debian",
|
||||
});
|
||||
expect(result.details?.status).toBeDefined();
|
||||
});
|
||||
|
||||
it("allows exec when requestedNode is partial ID matching boundNode's device", async () => {
|
||||
listNodesMock.mockResolvedValue([
|
||||
{
|
||||
nodeId: "f2396b588d391d30a79d300e196a17cf197f34969b5e2485d2734c953567f44e",
|
||||
displayName: "home-wsl-debian",
|
||||
commands: ["system.run"],
|
||||
platform: process.platform,
|
||||
},
|
||||
]);
|
||||
const result = await executeNodeHostCommand({
|
||||
command: "echo hello",
|
||||
workdir: "/tmp/work",
|
||||
env: {},
|
||||
security: "full",
|
||||
ask: "off",
|
||||
defaultTimeoutSec: 30,
|
||||
approvalRunningNoticeMs: 0,
|
||||
warnings: [],
|
||||
agentId: "test-agent",
|
||||
sessionKey: "test-session",
|
||||
boundNode: "f2396b588d391d30a79d300e196a17cf197f34969b5e2485d2734c953567f44e",
|
||||
requestedNode: "f2396b588d391d",
|
||||
});
|
||||
expect(result.details?.status).toBeDefined();
|
||||
});
|
||||
|
||||
it("rejects exec when requestedNode resolves to a different node than boundNode", async () => {
|
||||
listNodesMock.mockResolvedValue([
|
||||
{
|
||||
nodeId: "f2396b588d391d30a79d300e196a17cf197f34969b5e2485d2734c953567f44e",
|
||||
displayName: "home-wsl-debian",
|
||||
commands: ["system.run"],
|
||||
platform: process.platform,
|
||||
},
|
||||
{
|
||||
nodeId: "aaaa1111bbbb2222cccc3333dddd4444eeee5555ffff6666aaa7777bbb88889999",
|
||||
displayName: "other-node",
|
||||
commands: ["system.run"],
|
||||
platform: process.platform,
|
||||
},
|
||||
]);
|
||||
await expect(
|
||||
executeNodeHostCommand({
|
||||
command: "echo hello",
|
||||
workdir: "/tmp/work",
|
||||
env: {},
|
||||
security: "full",
|
||||
ask: "off",
|
||||
defaultTimeoutSec: 30,
|
||||
approvalRunningNoticeMs: 0,
|
||||
warnings: [],
|
||||
agentId: "test-agent",
|
||||
sessionKey: "test-session",
|
||||
boundNode: "f2396b588d391d30a79d300e196a17cf197f34969b5e2485d2734c953567f44e",
|
||||
requestedNode: "other-node",
|
||||
}),
|
||||
).rejects.toThrow("exec node not allowed (bound to f2396b588d391d30");
|
||||
});
|
||||
|
||||
it("preserves original error when requestedNode matches no known node", async () => {
|
||||
listNodesMock.mockResolvedValue([
|
||||
{
|
||||
nodeId: "f2396b588d391d30a79d300e196a17cf197f34969b5e2485d2734c953567f44e",
|
||||
displayName: "home-wsl-debian",
|
||||
commands: ["system.run"],
|
||||
platform: process.platform,
|
||||
},
|
||||
]);
|
||||
await expect(
|
||||
executeNodeHostCommand({
|
||||
command: "echo hello",
|
||||
workdir: "/tmp/work",
|
||||
env: {},
|
||||
security: "full",
|
||||
ask: "off",
|
||||
defaultTimeoutSec: 30,
|
||||
approvalRunningNoticeMs: 0,
|
||||
warnings: [],
|
||||
agentId: "test-agent",
|
||||
sessionKey: "test-session",
|
||||
requestedNode: "nonexistent-node",
|
||||
}),
|
||||
).rejects.toThrow(
|
||||
"requested node not found: nonexistent-node (unknown node: nonexistent-node)",
|
||||
);
|
||||
});
|
||||
|
||||
it("allows exec when boundNode is a display name matching the same device as requestedNode", async () => {
|
||||
listNodesMock.mockResolvedValue([
|
||||
{
|
||||
nodeId: "f2396b588d391d30a79d300e196a17cf197f34969b5e2485d2734c953567f44e",
|
||||
displayName: "home-wsl-debian",
|
||||
commands: ["system.run"],
|
||||
platform: process.platform,
|
||||
},
|
||||
]);
|
||||
const result = await executeNodeHostCommand({
|
||||
command: "echo hello",
|
||||
workdir: "/tmp/work",
|
||||
env: {},
|
||||
security: "full",
|
||||
ask: "off",
|
||||
defaultTimeoutSec: 30,
|
||||
approvalRunningNoticeMs: 0,
|
||||
warnings: [],
|
||||
agentId: "test-agent",
|
||||
sessionKey: "test-session",
|
||||
boundNode: "home-wsl-debian",
|
||||
requestedNode: "home-wsl-debian",
|
||||
});
|
||||
expect(result.details?.status).toBeDefined();
|
||||
});
|
||||
|
||||
it("auto-reviews strict inline-eval commands with full/off host policy when node policy is available", async () => {
|
||||
const inlinePlan = {
|
||||
argv: ["python3", "-c", "print(1)"],
|
||||
|
||||
Reference in New Issue
Block a user