mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-05 02:11:37 +00:00
* fix(exec): preserve approved exec continuation output Approved async exec continuations reused the compact background notification formatter, so the agent resumed from the last 400 characters of output with all whitespace collapsed and the head silently dropped. Both hosts now render the continuation through a shared whitespace-preserving formatter bounded at 16,000 UTF-16 units. Compact `notifyOnExit` notifications, poll/retained output, and the gateway diagnostics branch are unchanged. The truncation marker deliberately reports no exact omission count: output can already be capped at capture time without leaving a marker, so an exact number would describe only this cut while reading as though nothing else was lost. Closes #41152 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d9762e2-f967-4cdd-9895-299512a20114 * fix(exec): secure approved continuation handoff * test(exec): cover authenticated approval handoff * fix(exec): retain approved followup delivery ownership * fix(exec): retain compact continuation fallback * fix(exec): bound accepted followup observation * fix(exec): keep observer diagnostics internal --------- Co-authored-by: Omar Shahine <10343873+omarshahine@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d9762e2-f967-4cdd-9895-299512a20114
253 lines
8.5 KiB
TypeScript
253 lines
8.5 KiB
TypeScript
import { beforeEach, describe, expect, it, vi } from "vitest";
|
|
import type { ExecAsk, ExecSecurity } from "../infra/exec-approvals.js";
|
|
import type { ExecAutoReviewer } from "../infra/exec-auto-review.js";
|
|
import { createDeferred } from "../test-utils/deferred.js";
|
|
import type { ExecuteNodeHostCommandParams } from "./bash-tools.exec-host-node.types.js";
|
|
|
|
type NodeApprovalPolicy = Awaited<
|
|
ReturnType<typeof import("./bash-tools.exec-host-shared.js").resolveExecHostApprovalContext>
|
|
>;
|
|
|
|
const mocks = vi.hoisted(() => ({
|
|
analyzeNodeApprovalRequirement: vi.fn(),
|
|
callGatewayTool: vi.fn(
|
|
async (
|
|
_method: string,
|
|
_options: unknown,
|
|
_params?: { command?: string },
|
|
_extra?: { scopes?: string[]; signal?: AbortSignal },
|
|
) => ({ payload: { success: true, stdout: "ok", exitCode: 0 } }),
|
|
),
|
|
invokeNodeSystemRunDirect: vi.fn(),
|
|
registerNodeApproval: vi.fn(async () => undefined),
|
|
requiresExecApproval: vi.fn(),
|
|
resolveExecHostApprovalContext: vi.fn(),
|
|
}));
|
|
|
|
vi.mock("../infra/exec-approvals.js", () => ({
|
|
maxAsk: (left: ExecAsk, right: ExecAsk) => {
|
|
const priority: Record<ExecAsk, number> = { off: 0, "on-miss": 1, always: 2 };
|
|
return priority[left] >= priority[right] ? left : right;
|
|
},
|
|
minSecurity: (left: ExecSecurity, right: ExecSecurity) => {
|
|
const priority: Record<ExecSecurity, number> = { deny: 0, allowlist: 1, full: 2 };
|
|
return priority[left] <= priority[right] ? left : right;
|
|
},
|
|
requiresExecApproval: mocks.requiresExecApproval,
|
|
resolveExecApprovalAllowedDecisions: vi.fn(() => ["allow-once", "allow-always", "deny"]),
|
|
resolveExecApprovalUnavailableDecisions: vi.fn(() => []),
|
|
}));
|
|
|
|
vi.mock("../infra/exec-auto-review.js", () => ({
|
|
defaultExecAutoReviewer: vi.fn(),
|
|
resolveExecAutoReviewDecision: vi.fn(async (reviewer: ExecAutoReviewer, input) =>
|
|
reviewer(input),
|
|
),
|
|
}));
|
|
|
|
vi.mock("./bash-tools.exec-approval-request.js", () => ({
|
|
buildExecApprovalRequesterContext: vi.fn(() => ({})),
|
|
buildExecApprovalTurnSourceContext: vi.fn(() => ({})),
|
|
isExecApprovalRunAbortedError: vi.fn(() => false),
|
|
registerExecApprovalRequestForHostOrThrow: mocks.registerNodeApproval,
|
|
}));
|
|
|
|
vi.mock("./bash-tools.exec-host-node-phases.js", () => ({
|
|
analyzeNodeApprovalRequirement: mocks.analyzeNodeApprovalRequirement,
|
|
buildNodeSystemRunInvoke: vi.fn(() => ({ command: "system.run" })),
|
|
formatNodeRunToolResult: vi.fn(() => ({
|
|
content: [],
|
|
details: { status: "completed" },
|
|
})),
|
|
invokeNodeSystemRunDirect: mocks.invokeNodeSystemRunDirect,
|
|
prepareNodeSystemRun: vi.fn(async () => ({
|
|
plan: {},
|
|
argv: ["tool", "--version"],
|
|
rawCommand: "tool --version",
|
|
transportRawCommand: "tool --version",
|
|
cwd: "/tmp/work",
|
|
agentId: "agent",
|
|
sessionKey: "agent:main:main",
|
|
})),
|
|
resolveNodeExecutionTarget: vi.fn(async () => ({
|
|
nodeId: "node-1",
|
|
argv: ["tool", "--version"],
|
|
invokeTimeoutMs: 30_000,
|
|
supportsSystemRunPrepare: true,
|
|
})),
|
|
shouldSkipNodeApprovalPrepare: vi.fn(
|
|
(policy: { hostSecurity: ExecSecurity; hostAsk: ExecAsk }) =>
|
|
policy.hostSecurity === "full" && policy.hostAsk === "off",
|
|
),
|
|
}));
|
|
|
|
vi.mock("./bash-tools.exec-host-shared.js", () => ({
|
|
resolveExecHostApprovalContext: mocks.resolveExecHostApprovalContext,
|
|
}));
|
|
|
|
vi.mock("./bash-process-registry.js", () => ({ tail: vi.fn((text: string) => text) }));
|
|
|
|
vi.mock("./bash-tools.exec-runtime.js", () => ({
|
|
createApprovalSlug: vi.fn(() => "approval"),
|
|
}));
|
|
|
|
vi.mock("./embedded-agent-runner/run/abortable.js", () => ({
|
|
abortable: vi.fn(async (_signal: AbortSignal, pending: Promise<unknown>) => pending),
|
|
}));
|
|
|
|
vi.mock("./tools/gateway.js", () => ({ callGatewayTool: mocks.callGatewayTool }));
|
|
|
|
import { executeNodeHostCommand } from "./bash-tools.exec-host-node.js";
|
|
|
|
function createPolicy(security: ExecSecurity, ask: ExecAsk): NodeApprovalPolicy {
|
|
return {
|
|
approvals: {
|
|
path: "",
|
|
socketPath: "",
|
|
token: "",
|
|
defaults: { security, ask, askFallback: "deny", autoAllowSkills: false },
|
|
agent: { security, ask, askFallback: "deny", autoAllowSkills: false },
|
|
agentSources: { security: null, ask: null, askFallback: null },
|
|
allowlist: [],
|
|
file: { version: 1, agents: {} },
|
|
},
|
|
hostSecurity: security,
|
|
hostAsk: ask,
|
|
askFallback: "deny",
|
|
};
|
|
}
|
|
|
|
function createRequest(overrides: Partial<ExecuteNodeHostCommandParams>) {
|
|
return {
|
|
command: "tool --version",
|
|
workdir: "/tmp/work",
|
|
env: {},
|
|
security: "full",
|
|
ask: "off",
|
|
defaultTimeoutSec: 30,
|
|
approvalRunningNoticeMs: 0,
|
|
warnings: [],
|
|
agentId: "agent",
|
|
sessionKey: "agent:main:main",
|
|
...overrides,
|
|
} satisfies ExecuteNodeHostCommandParams;
|
|
}
|
|
|
|
describe("node-host dispatch cancellation", () => {
|
|
beforeEach(() => {
|
|
vi.clearAllMocks();
|
|
mocks.resolveExecHostApprovalContext.mockReset();
|
|
mocks.requiresExecApproval.mockImplementation(({ ask }: { ask: ExecAsk }) => ask !== "off");
|
|
mocks.analyzeNodeApprovalRequirement.mockResolvedValue({
|
|
analysisOk: true,
|
|
allowlistSatisfied: false,
|
|
durableApprovalSatisfied: false,
|
|
nodeApprovalPolicyKnown: true,
|
|
nodeSecurity: "allowlist",
|
|
nodeAsk: "on-miss",
|
|
inlineEvalHit: null,
|
|
requiresSecurityAuditSuppressionApproval: false,
|
|
autoReviewArgv: ["tool", "--version"],
|
|
allowAlwaysPersistence: { kind: "patterns", patterns: [] },
|
|
});
|
|
mocks.invokeNodeSystemRunDirect.mockResolvedValue({
|
|
content: [],
|
|
details: { status: "completed" },
|
|
});
|
|
});
|
|
|
|
it.each([
|
|
{ name: "direct full/off", security: "full", ask: "off", autoReview: false },
|
|
{
|
|
name: "auto-reviewed allowlist",
|
|
security: "allowlist",
|
|
ask: "on-miss",
|
|
autoReview: true,
|
|
},
|
|
] as const)("never dispatches $name after cancellation during final policy", async (scenario) => {
|
|
const controller = new AbortController();
|
|
const reason = new Error("cancelled during final node dispatch policy");
|
|
const policy = createPolicy(scenario.security, scenario.ask);
|
|
const checkpoint = createDeferred<NodeApprovalPolicy>();
|
|
mocks.resolveExecHostApprovalContext
|
|
.mockResolvedValueOnce(policy)
|
|
.mockReturnValueOnce(checkpoint.promise);
|
|
const reviewer: ExecAutoReviewer = async () => ({
|
|
decision: "allow-once",
|
|
risk: "low",
|
|
rationale: "safe command",
|
|
});
|
|
|
|
const result = executeNodeHostCommand(
|
|
createRequest({
|
|
security: scenario.security,
|
|
ask: scenario.ask,
|
|
autoReview: scenario.autoReview,
|
|
autoReviewer: reviewer,
|
|
signal: controller.signal,
|
|
}),
|
|
);
|
|
|
|
await vi.waitFor(() => expect(mocks.resolveExecHostApprovalContext).toHaveBeenCalledTimes(2));
|
|
controller.abort(reason);
|
|
checkpoint.resolve(policy);
|
|
|
|
await expect(result).rejects.toBe(reason);
|
|
expect(mocks.invokeNodeSystemRunDirect).not.toHaveBeenCalled();
|
|
expect(
|
|
mocks.callGatewayTool.mock.calls.some(
|
|
([method, , params]) => method === "node.invoke" && params?.command === "system.run",
|
|
),
|
|
).toBe(false);
|
|
expect(mocks.registerNodeApproval).toHaveBeenCalledTimes(scenario.autoReview ? 1 : 0);
|
|
});
|
|
|
|
it("forwards cancellation without removing auto-reviewed execution scopes", async () => {
|
|
const controller = new AbortController();
|
|
mocks.resolveExecHostApprovalContext.mockResolvedValue(createPolicy("allowlist", "on-miss"));
|
|
const reviewer: ExecAutoReviewer = async () => ({
|
|
decision: "allow-once",
|
|
risk: "low",
|
|
rationale: "safe command",
|
|
});
|
|
|
|
await executeNodeHostCommand(
|
|
createRequest({
|
|
security: "allowlist",
|
|
ask: "on-miss",
|
|
autoReview: true,
|
|
autoReviewer: reviewer,
|
|
signal: controller.signal,
|
|
}),
|
|
);
|
|
|
|
expect(mocks.callGatewayTool).toHaveBeenCalledWith(
|
|
"node.invoke",
|
|
{ timeoutMs: 30_000 },
|
|
expect.objectContaining({ command: "system.run" }),
|
|
{ scopes: ["operator.write", "operator.approvals"], signal: controller.signal },
|
|
);
|
|
});
|
|
|
|
it("forwards cancellation for prepared commands that need no approval", async () => {
|
|
const controller = new AbortController();
|
|
mocks.resolveExecHostApprovalContext.mockResolvedValue(createPolicy("allowlist", "off"));
|
|
|
|
await executeNodeHostCommand(
|
|
createRequest({
|
|
security: "allowlist",
|
|
ask: "off",
|
|
signal: controller.signal,
|
|
}),
|
|
);
|
|
|
|
expect(mocks.callGatewayTool).toHaveBeenCalledWith(
|
|
"node.invoke",
|
|
{ timeoutMs: 30_000 },
|
|
expect.objectContaining({ command: "system.run" }),
|
|
{ signal: controller.signal },
|
|
);
|
|
expect(mocks.registerNodeApproval).not.toHaveBeenCalled();
|
|
});
|
|
});
|