fix(exec): add approval race changelog and regressions

This commit is contained in:
Peter Steinberger
2026-02-24 03:22:05 +00:00
parent 6f0dd61795
commit 7b2b86c60a
3 changed files with 112 additions and 0 deletions

View File

@@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai
- Security/Shell env fallback: remove trusted-prefix shell-path fallback and only trust login shells explicitly registered in `/etc/shells`, defaulting to `/bin/sh` when `SHELL` is not registered. This ships in the next npm release. Thanks @tdjackey for reporting.
- Security/Exec approvals: bind `host=node` approvals to explicit `nodeId`, reject cross-node replay of approved `system.run` requests, and include the target node in approval prompts. This ships in the next npm release. Thanks @tdjackey for reporting.
- Security/Exec approvals: restore two-phase approval registration + wait-decision handling for gateway/node exec paths, requiring approval IDs to be registered before returning `approval-pending` and honoring server-assigned approval IDs during wait resolution to prevent orphaned `/approve` flows and immediate-return races (`ask:on-miss`). This ships in the next npm release. Thanks @vitalyis for reporting.
- Security/Voice Call: harden Twilio webhook replay handling by preserving provider event IDs through normalization, adding bounded replay dedupe, and enforcing per-call turn-token matching for call-state transitions. This ships in the next npm release. Thanks @jiseoung for reporting.
- Security/Export session HTML: escape raw HTML markdown tokens in the exported session viewer, harden tree/header metadata rendering against HTML injection, and sanitize image data-URL MIME types in export output to prevent stored XSS when opening exported HTML files. This ships in the next npm release. Thanks @allsmog for reporting.
- Security/iOS deep links: require local confirmation (or trusted key) before forwarding `openclaw://agent` requests from iOS to gateway `agent.request`, and strip unkeyed delivery-routing fields to reduce exfiltration risk. This ships in the next npm release. Thanks @GCXWLP for reporting.

View File

@@ -102,6 +102,55 @@ describe("requestExecApprovalDecision", () => {
).resolves.toBeNull();
});
it("uses registration response id when waiting for decision", async () => {
vi.mocked(callGatewayTool)
.mockResolvedValueOnce({
status: "accepted",
id: "server-assigned-id",
expiresAtMs: DEFAULT_APPROVAL_TIMEOUT_MS,
})
.mockResolvedValueOnce({ decision: "allow-once" });
await expect(
requestExecApprovalDecision({
id: "client-id",
command: "echo hi",
cwd: "/tmp",
host: "gateway",
security: "allowlist",
ask: "on-miss",
}),
).resolves.toBe("allow-once");
expect(callGatewayTool).toHaveBeenNthCalledWith(
2,
"exec.approval.waitDecision",
{ timeoutMs: DEFAULT_APPROVAL_REQUEST_TIMEOUT_MS },
{ id: "server-assigned-id" },
);
});
it("treats expired-or-missing waitDecision as null decision", async () => {
vi.mocked(callGatewayTool)
.mockResolvedValueOnce({
status: "accepted",
id: "approval-id",
expiresAtMs: DEFAULT_APPROVAL_TIMEOUT_MS,
})
.mockRejectedValueOnce(new Error("approval expired or not found"));
await expect(
requestExecApprovalDecision({
id: "approval-id",
command: "echo hi",
cwd: "/tmp",
host: "gateway",
security: "allowlist",
ask: "on-miss",
}),
).resolves.toBeNull();
});
it("returns final decision directly when gateway already replies with decision", async () => {
vi.mocked(callGatewayTool).mockResolvedValue({ decision: "deny", id: "approval-id" });

View File

@@ -196,6 +196,68 @@ describe("exec approvals", () => {
expect(calls).toContain("exec.approval.waitDecision");
});
it("waits for approval registration before returning approval-pending", async () => {
const calls: string[] = [];
let resolveRegistration: ((value: unknown) => void) | undefined;
const registrationPromise = new Promise<unknown>((resolve) => {
resolveRegistration = resolve;
});
vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => {
calls.push(method);
if (method === "exec.approval.request") {
return await registrationPromise;
}
if (method === "exec.approval.waitDecision") {
return { decision: "deny" };
}
return { ok: true, id: (params as { id?: string })?.id };
});
const tool = createExecTool({
host: "gateway",
ask: "on-miss",
security: "allowlist",
approvalRunningNoticeMs: 0,
});
let settled = false;
const executePromise = tool.execute("call-registration-gate", { command: "echo register" });
void executePromise.finally(() => {
settled = true;
});
await Promise.resolve();
await Promise.resolve();
expect(settled).toBe(false);
resolveRegistration?.({ status: "accepted", id: "approval-id" });
const result = await executePromise;
expect(result.details.status).toBe("approval-pending");
expect(calls[0]).toBe("exec.approval.request");
expect(calls).toContain("exec.approval.waitDecision");
});
it("fails fast when approval registration fails", async () => {
vi.mocked(callGatewayTool).mockImplementation(async (method) => {
if (method === "exec.approval.request") {
throw new Error("gateway offline");
}
return { ok: true };
});
const tool = createExecTool({
host: "gateway",
ask: "on-miss",
security: "allowlist",
approvalRunningNoticeMs: 0,
});
await expect(tool.execute("call-registration-fail", { command: "echo fail" })).rejects.toThrow(
"Exec approval registration failed",
);
});
it("denies node obfuscated command when approval request times out", async () => {
vi.mocked(detectCommandObfuscation).mockReturnValue({
detected: true,