Files
openclaw/src/gateway/server-methods/exec-approval.test.ts
Ramin Shirali Hossein Zade 1af0edf7ff fix: ensure exec approval is registered before returning (#2402) (#3357)
* feat(gateway): add register and awaitDecision methods to ExecApprovalManager

Separates registration (synchronous) from waiting (async) to allow callers
to confirm registration before the decision is made. Adds grace period for
resolved entries to prevent race conditions.

* feat(gateway): add two-phase response and waitDecision handler for exec approvals

Send immediate 'accepted' response after registration so callers can confirm
the approval ID is valid. Add exec.approval.waitDecision endpoint to wait for
decision on already-registered approvals.

* fix(exec): await approval registration before returning approval-pending

Ensures the approval ID is registered in the gateway before the tool returns.
Uses exec.approval.request with expectFinal:false for registration, then
fire-and-forget exec.approval.waitDecision for the decision phase.

Fixes #2402

* test(gateway): update exec-approval test for two-phase response

Add assertion for immediate 'accepted' response before final decision.

* test(exec): update approval-id test mocks for new two-phase flow

Mock both exec.approval.request (registration) and exec.approval.waitDecision
(decision) calls to match the new internal implementation.

* fix(lint): add cause to errors, use generics instead of type assertions

* fix(exec-approval): guard register() against duplicate IDs

* fix: remove unused timeoutMs param, guard register() against duplicates

* fix(exec-approval): throw on duplicate ID, capture entry in closure

* fix: return error on timeout, remove stale test mock branch

* fix: wrap register() in try/catch, make timeout handling consistent

* fix: update snapshot on timeout, make two-phase response opt-in

* fix: extend grace period to 15s, return 'expired' status

* fix: prevent double-resolve after timeout

* fix: make register() idempotent, capture snapshot before await

* fix(gateway): complete two-phase exec approval wiring

* fix: finalize exec approval race fix (openclaw#3357) thanks @ramin-shirali

* fix(protocol): regenerate exec approval request models (openclaw#3357) thanks @ramin-shirali

* fix(test): remove unused callCount in discord threading test

---------

Co-authored-by: rshirali <rshirali@rshirali-haga.local>
Co-authored-by: rshirali <rshirali@rshirali-haga-1.home>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
2026-02-13 19:57:02 +01:00

286 lines
9.0 KiB
TypeScript

import { describe, expect, it, vi } from "vitest";
import { ExecApprovalManager } from "../exec-approval-manager.js";
import { validateExecApprovalRequestParams } from "../protocol/index.js";
import { createExecApprovalHandlers } from "./exec-approval.js";
const noop = () => {};
describe("exec approval handlers", () => {
describe("ExecApprovalRequestParams validation", () => {
it("accepts request with resolvedPath omitted", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
host: "node",
};
expect(validateExecApprovalRequestParams(params)).toBe(true);
});
it("accepts request with resolvedPath as string", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
host: "node",
resolvedPath: "/usr/bin/echo",
};
expect(validateExecApprovalRequestParams(params)).toBe(true);
});
it("accepts request with resolvedPath as undefined", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
host: "node",
resolvedPath: undefined,
};
expect(validateExecApprovalRequestParams(params)).toBe(true);
});
// Fixed: null is now accepted (Type.Union([Type.String(), Type.Null()]))
// This matches the calling code in bash-tools.exec.ts which passes null.
it("accepts request with resolvedPath as null", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
host: "node",
resolvedPath: null,
};
expect(validateExecApprovalRequestParams(params)).toBe(true);
});
});
it("broadcasts request + resolve", async () => {
const manager = new ExecApprovalManager();
const handlers = createExecApprovalHandlers(manager);
const broadcasts: Array<{ event: string; payload: unknown }> = [];
const respond = vi.fn();
const context = {
broadcast: (event: string, payload: unknown) => {
broadcasts.push({ event, payload });
},
};
const requestPromise = handlers["exec.approval.request"]({
params: {
command: "echo ok",
cwd: "/tmp",
host: "node",
timeoutMs: 2000,
twoPhase: true,
},
respond,
context: context as unknown as Parameters<
(typeof handlers)["exec.approval.request"]
>[0]["context"],
client: null,
req: { id: "req-1", type: "req", method: "exec.approval.request" },
isWebchatConnect: noop,
});
const requested = broadcasts.find((entry) => entry.event === "exec.approval.requested");
expect(requested).toBeTruthy();
const id = (requested?.payload as { id?: string })?.id ?? "";
expect(id).not.toBe("");
// First response should be "accepted" (registration confirmation)
expect(respond).toHaveBeenCalledWith(
true,
expect.objectContaining({ status: "accepted", id }),
undefined,
);
const resolveRespond = vi.fn();
await handlers["exec.approval.resolve"]({
params: { id, decision: "allow-once" },
respond: resolveRespond,
context: context as unknown as Parameters<
(typeof handlers)["exec.approval.resolve"]
>[0]["context"],
client: { connect: { client: { id: "cli", displayName: "CLI" } } },
req: { id: "req-2", type: "req", method: "exec.approval.resolve" },
isWebchatConnect: noop,
});
await requestPromise;
expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined);
// Second response should contain the decision
expect(respond).toHaveBeenCalledWith(
true,
expect.objectContaining({ id, decision: "allow-once" }),
undefined,
);
expect(broadcasts.some((entry) => entry.event === "exec.approval.resolved")).toBe(true);
});
it("accepts resolve during broadcast", async () => {
const manager = new ExecApprovalManager();
const handlers = createExecApprovalHandlers(manager);
const respond = vi.fn();
const resolveRespond = vi.fn();
const resolveContext = {
broadcast: () => {},
};
const context = {
broadcast: (event: string, payload: unknown) => {
if (event !== "exec.approval.requested") {
return;
}
const id = (payload as { id?: string })?.id ?? "";
void handlers["exec.approval.resolve"]({
params: { id, decision: "allow-once" },
respond: resolveRespond,
context: resolveContext as unknown as Parameters<
(typeof handlers)["exec.approval.resolve"]
>[0]["context"],
client: { connect: { client: { id: "cli", displayName: "CLI" } } },
req: { id: "req-2", type: "req", method: "exec.approval.resolve" },
isWebchatConnect: noop,
});
},
};
await handlers["exec.approval.request"]({
params: {
command: "echo ok",
cwd: "/tmp",
host: "node",
timeoutMs: 2000,
},
respond,
context: context as unknown as Parameters<
(typeof handlers)["exec.approval.request"]
>[0]["context"],
client: null,
req: { id: "req-1", type: "req", method: "exec.approval.request" },
isWebchatConnect: noop,
});
expect(resolveRespond).toHaveBeenCalledWith(true, { ok: true }, undefined);
expect(respond).toHaveBeenCalledWith(
true,
expect.objectContaining({ decision: "allow-once" }),
undefined,
);
});
it("accepts explicit approval ids", async () => {
const manager = new ExecApprovalManager();
const handlers = createExecApprovalHandlers(manager);
const broadcasts: Array<{ event: string; payload: unknown }> = [];
const respond = vi.fn();
const context = {
broadcast: (event: string, payload: unknown) => {
broadcasts.push({ event, payload });
},
};
const requestPromise = handlers["exec.approval.request"]({
params: {
id: "approval-123",
command: "echo ok",
cwd: "/tmp",
host: "gateway",
timeoutMs: 2000,
},
respond,
context: context as unknown as Parameters<
(typeof handlers)["exec.approval.request"]
>[0]["context"],
client: null,
req: { id: "req-1", type: "req", method: "exec.approval.request" },
isWebchatConnect: noop,
});
const requested = broadcasts.find((entry) => entry.event === "exec.approval.requested");
const id = (requested?.payload as { id?: string })?.id ?? "";
expect(id).toBe("approval-123");
const resolveRespond = vi.fn();
await handlers["exec.approval.resolve"]({
params: { id, decision: "allow-once" },
respond: resolveRespond,
context: context as unknown as Parameters<
(typeof handlers)["exec.approval.resolve"]
>[0]["context"],
client: { connect: { client: { id: "cli", displayName: "CLI" } } },
req: { id: "req-2", type: "req", method: "exec.approval.resolve" },
isWebchatConnect: noop,
});
await requestPromise;
expect(respond).toHaveBeenCalledWith(
true,
expect.objectContaining({ id: "approval-123", decision: "allow-once" }),
undefined,
);
});
it("rejects duplicate approval ids", async () => {
const manager = new ExecApprovalManager();
const handlers = createExecApprovalHandlers(manager);
const respondA = vi.fn();
const respondB = vi.fn();
const broadcasts: Array<{ event: string; payload: unknown }> = [];
const context = {
broadcast: (event: string, payload: unknown) => {
broadcasts.push({ event, payload });
},
};
const requestPromise = handlers["exec.approval.request"]({
params: {
id: "dup-1",
command: "echo ok",
},
respond: respondA,
context: context as unknown as Parameters<
(typeof handlers)["exec.approval.request"]
>[0]["context"],
client: null,
req: { id: "req-1", type: "req", method: "exec.approval.request" },
isWebchatConnect: noop,
});
await handlers["exec.approval.request"]({
params: {
id: "dup-1",
command: "echo again",
},
respond: respondB,
context: context as unknown as Parameters<
(typeof handlers)["exec.approval.request"]
>[0]["context"],
client: null,
req: { id: "req-2", type: "req", method: "exec.approval.request" },
isWebchatConnect: noop,
});
expect(respondB).toHaveBeenCalledWith(
false,
undefined,
expect.objectContaining({ message: "approval id already pending" }),
);
const requested = broadcasts.find((entry) => entry.event === "exec.approval.requested");
const id = (requested?.payload as { id?: string })?.id ?? "";
const resolveRespond = vi.fn();
await handlers["exec.approval.resolve"]({
params: { id, decision: "deny" },
respond: resolveRespond,
context: context as unknown as Parameters<
(typeof handlers)["exec.approval.resolve"]
>[0]["context"],
client: { connect: { client: { id: "cli", displayName: "CLI" } } },
req: { id: "req-3", type: "req", method: "exec.approval.resolve" },
isWebchatConnect: noop,
});
await requestPromise;
});
});