mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-09 10:10:44 +00:00
fix: preserve node exec approvals for control ui
Signed-off-by: sallyom <somalley@redhat.com>
This commit is contained in:
@@ -124,6 +124,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Exec approvals/node: let trusted backend node invokes complete no-device Control UI approvals after the original request connection changes, while keeping node, command, cwd, env, and allow-once replay bindings enforced. Fixes #78569. Thanks @naturedogdog.
|
||||
- CLI/completion: guard the shell-profile source line written by `openclaw completion --install` with a file existence check (`[ -f ... ] && source ...` for bash/zsh, `test -f ...; and source ...` for fish) so uninstalling OpenClaw no longer makes new login shells error on a missing completion cache. (#78659) Thanks @sjf.
|
||||
- Cron/doctor: repair persisted cron jobs whose `payload.model` was stored as `"default"`, `"null"`, blank, or JSON `null` by removing the bad override during `openclaw doctor --fix` while keeping cron runtime model validation strict. Fixes #78549. Thanks @bizzle12368239.
|
||||
- Telegram: honor `accessGroup:*` sender allowlists for DMs, groups, native commands, and callback authorization before applying Telegram's numeric sender-ID checks. Fixes #78660. Thanks @manugc.
|
||||
|
||||
@@ -297,6 +297,7 @@ describe("executeNodeHostCommand", () => {
|
||||
timeoutMs: 30_000,
|
||||
}),
|
||||
}),
|
||||
{ scopes: ["operator.write", "operator.approvals"] },
|
||||
);
|
||||
});
|
||||
|
||||
@@ -355,6 +356,7 @@ describe("executeNodeHostCommand", () => {
|
||||
systemRunPlan: expectedPlan,
|
||||
}),
|
||||
}),
|
||||
{ scopes: ["operator.write", "operator.approvals"] },
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
|
||||
import { APPROVALS_SCOPE, WRITE_SCOPE } from "../gateway/operator-scopes.js";
|
||||
import {
|
||||
requiresExecApproval,
|
||||
resolveExecApprovalAllowedDecisions,
|
||||
@@ -29,6 +30,8 @@ import { callGatewayTool } from "./tools/gateway.js";
|
||||
|
||||
export type { ExecuteNodeHostCommandParams } from "./bash-tools.exec-host-node.types.js";
|
||||
|
||||
const APPROVED_NODE_INVOKE_SCOPES = [WRITE_SCOPE, APPROVALS_SCOPE];
|
||||
|
||||
export async function executeNodeHostCommand(
|
||||
params: ExecuteNodeHostCommandParams,
|
||||
): Promise<AgentToolResult<ExecToolDetails>> {
|
||||
@@ -225,6 +228,7 @@ export async function executeNodeHostCommand(
|
||||
notifyOnExit: params.notifyOnExit,
|
||||
systemRunPlan: prepared.plan,
|
||||
}),
|
||||
{ scopes: APPROVED_NODE_INVOKE_SCOPES },
|
||||
);
|
||||
const payload =
|
||||
raw?.payload && typeof raw.payload === "object"
|
||||
@@ -271,22 +275,24 @@ export async function executeNodeHostCommand(
|
||||
}
|
||||
|
||||
const startedAt = Date.now();
|
||||
const raw = await callGatewayTool(
|
||||
"node.invoke",
|
||||
{ timeoutMs: target.invokeTimeoutMs },
|
||||
buildNodeSystemRunInvoke({
|
||||
target,
|
||||
command: prepared.argv,
|
||||
rawCommand: prepared.rawCommand,
|
||||
cwd: prepared.cwd,
|
||||
agentId: prepared.agentId,
|
||||
sessionKey: prepared.sessionKey,
|
||||
approved: inlineApprovedByAsk,
|
||||
approvalDecision: inlineApprovalDecision,
|
||||
runId: inlineApprovalId,
|
||||
notifyOnExit: params.notifyOnExit,
|
||||
systemRunPlan: prepared.plan,
|
||||
}),
|
||||
);
|
||||
const invoke = buildNodeSystemRunInvoke({
|
||||
target,
|
||||
command: prepared.argv,
|
||||
rawCommand: prepared.rawCommand,
|
||||
cwd: prepared.cwd,
|
||||
agentId: prepared.agentId,
|
||||
sessionKey: prepared.sessionKey,
|
||||
approved: inlineApprovedByAsk,
|
||||
approvalDecision: inlineApprovalDecision,
|
||||
runId: inlineApprovalId,
|
||||
notifyOnExit: params.notifyOnExit,
|
||||
systemRunPlan: prepared.plan,
|
||||
});
|
||||
const raw =
|
||||
inlineApprovedByAsk && inlineApprovalId
|
||||
? await callGatewayTool("node.invoke", { timeoutMs: target.invokeTimeoutMs }, invoke, {
|
||||
scopes: APPROVED_NODE_INVOKE_SCOPES,
|
||||
})
|
||||
: await callGatewayTool("node.invoke", { timeoutMs: target.invokeTimeoutMs }, invoke);
|
||||
return formatNodeRunToolResult({ raw, startedAt, cwd: params.workdir });
|
||||
}
|
||||
|
||||
@@ -31,6 +31,7 @@ export type ExecApprovalRecord<TPayload = ExecApprovalRequestPayload> = {
|
||||
requestedByConnId?: string | null;
|
||||
requestedByDeviceId?: string | null;
|
||||
requestedByClientId?: string | null;
|
||||
requestedByDeviceTokenAuth?: boolean;
|
||||
resolvedAtMs?: number;
|
||||
decision?: ExecApprovalDecision;
|
||||
consumedDecision?: ExecApprovalDecision;
|
||||
|
||||
@@ -12,8 +12,16 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
|
||||
connId: "conn-1",
|
||||
connect: {
|
||||
scopes: ["operator.write", "operator.approvals"],
|
||||
client: { id: "cli-1", mode: "cli" },
|
||||
device: { id: "dev-1" },
|
||||
client: { id: "cli-1" },
|
||||
},
|
||||
};
|
||||
const trustedBackendClient = {
|
||||
connId: "backend-conn",
|
||||
connect: {
|
||||
scopes: ["operator.write", "operator.approvals"],
|
||||
client: { id: "gateway-client", mode: "backend" },
|
||||
device: null,
|
||||
},
|
||||
};
|
||||
|
||||
@@ -45,6 +53,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
|
||||
requestedByConnId: "conn-1",
|
||||
requestedByDeviceId: "dev-1",
|
||||
requestedByClientId: "cli-1",
|
||||
requestedByDeviceTokenAuth: false,
|
||||
resolvedAtMs: now - 500,
|
||||
decision: "allow-once",
|
||||
resolvedBy: "operator",
|
||||
@@ -361,6 +370,7 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
|
||||
record.requestedByConnId = "conn-1";
|
||||
record.requestedByDeviceId = "dev-1";
|
||||
record.requestedByClientId = "cli-1";
|
||||
record.requestedByDeviceTokenAuth = false;
|
||||
|
||||
const decisionPromise = approvalManager.register(record, 60_000);
|
||||
approvalManager.resolve(runId, "allow-once", "operator");
|
||||
@@ -426,4 +436,113 @@ describe("sanitizeSystemRunParamsForForwarding", () => {
|
||||
});
|
||||
expectRejectedForwardingResult(result, "APPROVAL_NODE_MISMATCH", "not valid for this node");
|
||||
});
|
||||
|
||||
test("accepts trusted backend replay for no-device approval after the request connection changes", () => {
|
||||
const record = makeRecord("echo SAFE", ["echo", "SAFE"]);
|
||||
record.requestedByConnId = "control-ui-conn";
|
||||
record.requestedByDeviceId = null;
|
||||
record.requestedByClientId = "openclaw-control-ui";
|
||||
record.requestedByDeviceTokenAuth = false;
|
||||
|
||||
const result = sanitizeSystemRunParamsForForwarding({
|
||||
rawParams: {
|
||||
command: ["echo", "SAFE"],
|
||||
rawCommand: "echo SAFE",
|
||||
runId: "approval-1",
|
||||
approved: true,
|
||||
approvalDecision: "allow-once",
|
||||
},
|
||||
nodeId: "node-1",
|
||||
client: trustedBackendClient,
|
||||
execApprovalManager: manager(record),
|
||||
nowMs: now,
|
||||
});
|
||||
|
||||
expectAllowOnceForwardingResult(result);
|
||||
});
|
||||
|
||||
test("rejects no-device approval replay from a backend client without approval scope", () => {
|
||||
const record = makeRecord("echo SAFE", ["echo", "SAFE"]);
|
||||
record.requestedByConnId = "control-ui-conn";
|
||||
record.requestedByDeviceId = null;
|
||||
record.requestedByClientId = "openclaw-control-ui";
|
||||
record.requestedByDeviceTokenAuth = false;
|
||||
|
||||
const result = sanitizeSystemRunParamsForForwarding({
|
||||
rawParams: {
|
||||
command: ["echo", "SAFE"],
|
||||
rawCommand: "echo SAFE",
|
||||
runId: "approval-1",
|
||||
approved: true,
|
||||
approvalDecision: "allow-once",
|
||||
},
|
||||
nodeId: "node-1",
|
||||
client: {
|
||||
...trustedBackendClient,
|
||||
connect: {
|
||||
...trustedBackendClient.connect,
|
||||
scopes: ["operator.write"],
|
||||
},
|
||||
},
|
||||
execApprovalManager: manager(record),
|
||||
nowMs: now,
|
||||
});
|
||||
|
||||
expectRejectedForwardingResult(result, "APPROVAL_CLIENT_MISMATCH", "not valid for this client");
|
||||
});
|
||||
|
||||
test("rejects no-device approval replay from a non-backend client on a different connection", () => {
|
||||
const record = makeRecord("echo SAFE", ["echo", "SAFE"]);
|
||||
record.requestedByConnId = "control-ui-conn";
|
||||
record.requestedByDeviceId = null;
|
||||
record.requestedByClientId = "openclaw-control-ui";
|
||||
record.requestedByDeviceTokenAuth = false;
|
||||
|
||||
const result = sanitizeSystemRunParamsForForwarding({
|
||||
rawParams: {
|
||||
command: ["echo", "SAFE"],
|
||||
rawCommand: "echo SAFE",
|
||||
runId: "approval-1",
|
||||
approved: true,
|
||||
approvalDecision: "allow-once",
|
||||
},
|
||||
nodeId: "node-1",
|
||||
client: {
|
||||
connId: "other-control-ui-conn",
|
||||
connect: {
|
||||
scopes: ["operator.write", "operator.approvals"],
|
||||
client: { id: "openclaw-control-ui", mode: "ui" },
|
||||
device: null,
|
||||
},
|
||||
},
|
||||
execApprovalManager: manager(record),
|
||||
nowMs: now,
|
||||
});
|
||||
|
||||
expectRejectedForwardingResult(result, "APPROVAL_CLIENT_MISMATCH", "not valid for this client");
|
||||
});
|
||||
|
||||
test("rejects no-device approval replay when the original request used device-token auth", () => {
|
||||
const record = makeRecord("echo SAFE", ["echo", "SAFE"]);
|
||||
record.requestedByConnId = "control-ui-conn";
|
||||
record.requestedByDeviceId = null;
|
||||
record.requestedByClientId = "openclaw-control-ui";
|
||||
record.requestedByDeviceTokenAuth = true;
|
||||
|
||||
const result = sanitizeSystemRunParamsForForwarding({
|
||||
rawParams: {
|
||||
command: ["echo", "SAFE"],
|
||||
rawCommand: "echo SAFE",
|
||||
runId: "approval-1",
|
||||
approved: true,
|
||||
approvalDecision: "allow-once",
|
||||
},
|
||||
nodeId: "node-1",
|
||||
client: trustedBackendClient,
|
||||
execApprovalManager: manager(record),
|
||||
nowMs: now,
|
||||
});
|
||||
|
||||
expectRejectedForwardingResult(result, "APPROVAL_CLIENT_MISMATCH", "not valid for this client");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
evaluateSystemRunApprovalMatch,
|
||||
toSystemRunApprovalMismatchError,
|
||||
} from "./node-invoke-system-run-approval-match.js";
|
||||
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "./protocol/client-info.js";
|
||||
|
||||
type SystemRunParamsLike = {
|
||||
command?: unknown;
|
||||
@@ -35,12 +36,21 @@ type ApprovalLookup = {
|
||||
|
||||
type ApprovalClient = {
|
||||
connId?: string | null;
|
||||
isDeviceTokenAuth?: boolean;
|
||||
connect?: {
|
||||
scopes?: unknown;
|
||||
client?: { id?: string | null; mode?: string | null } | null;
|
||||
device?: { id?: string | null } | null;
|
||||
} | null;
|
||||
};
|
||||
|
||||
const BACKEND_BRIDGEABLE_NO_DEVICE_REQUEST_CLIENT_IDS = new Set<string>([
|
||||
GATEWAY_CLIENT_NAMES.CONTROL_UI,
|
||||
GATEWAY_CLIENT_NAMES.WEBCHAT_UI,
|
||||
GATEWAY_CLIENT_NAMES.WEBCHAT,
|
||||
GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT,
|
||||
]);
|
||||
|
||||
function normalizeApprovalDecision(value: unknown): "allow-once" | "allow-always" | null {
|
||||
const s = normalizeNullableString(value);
|
||||
return s === "allow-once" || s === "allow-always" ? s : null;
|
||||
@@ -51,6 +61,29 @@ function clientHasApprovals(client: ApprovalClient | null): boolean {
|
||||
return scopes.includes("operator.admin") || scopes.includes("operator.approvals");
|
||||
}
|
||||
|
||||
function isTrustedBackendApprovalClient(client: ApprovalClient | null): boolean {
|
||||
return (
|
||||
clientHasApprovals(client) &&
|
||||
client?.connect?.client?.id === GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT &&
|
||||
client.connect.client.mode === GATEWAY_CLIENT_MODES.BACKEND &&
|
||||
client.isDeviceTokenAuth !== true
|
||||
);
|
||||
}
|
||||
|
||||
function canBridgeNoDeviceApprovalFromBackend(params: {
|
||||
snapshot: ExecApprovalRecord;
|
||||
client: ApprovalClient | null;
|
||||
}): boolean {
|
||||
const requestedByClientId = normalizeNullableString(params.snapshot.requestedByClientId);
|
||||
return (
|
||||
params.snapshot.requestedByDeviceId == null &&
|
||||
params.snapshot.requestedByDeviceTokenAuth !== true &&
|
||||
requestedByClientId !== null &&
|
||||
BACKEND_BRIDGEABLE_NO_DEVICE_REQUEST_CLIENT_IDS.has(requestedByClientId) &&
|
||||
isTrustedBackendApprovalClient(params.client)
|
||||
);
|
||||
}
|
||||
|
||||
function pickSystemRunParams(raw: Record<string, unknown>): Record<string, unknown> {
|
||||
// Defensive allowlist: only forward fields that the node-host `system.run` handler understands.
|
||||
// This prevents future internal control fields from being smuggled through the gateway.
|
||||
@@ -190,7 +223,8 @@ export function sanitizeSystemRunParamsForForwarding(opts: {
|
||||
}
|
||||
} else if (
|
||||
snapshot.requestedByConnId &&
|
||||
snapshot.requestedByConnId !== (opts.client?.connId ?? null)
|
||||
snapshot.requestedByConnId !== (opts.client?.connId ?? null) &&
|
||||
!canBridgeNoDeviceApprovalFromBackend({ snapshot, client: opts.client })
|
||||
) {
|
||||
return systemRunApprovalGuardError({
|
||||
code: "APPROVAL_CLIENT_MISMATCH",
|
||||
|
||||
@@ -263,6 +263,7 @@ export function createExecApprovalHandlers(
|
||||
record.requestedByConnId = client?.connId ?? null;
|
||||
record.requestedByDeviceId = client?.connect?.device?.id ?? null;
|
||||
record.requestedByClientId = client?.connect?.client?.id ?? null;
|
||||
record.requestedByDeviceTokenAuth = client?.isDeviceTokenAuth === true;
|
||||
// Use register() to synchronously add to pending map before sending any response.
|
||||
// This ensures the approval ID is valid immediately after the "accepted" response.
|
||||
let decisionPromise: Promise<
|
||||
|
||||
Reference in New Issue
Block a user