mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-24 15:41:40 +00:00
refactor(exec): dedupe durable approval checks
This commit is contained in:
@@ -80,6 +80,19 @@ export type ProcessGatewayAllowlistResult = {
|
||||
pendingResult?: AgentToolResult<ExecToolDetails>;
|
||||
};
|
||||
|
||||
function hasGatewayAllowlistMiss(params: {
|
||||
hostSecurity: ExecSecurity;
|
||||
analysisOk: boolean;
|
||||
allowlistSatisfied: boolean;
|
||||
durableApprovalSatisfied: boolean;
|
||||
}): boolean {
|
||||
return (
|
||||
params.hostSecurity === "allowlist" &&
|
||||
(!params.analysisOk || !params.allowlistSatisfied) &&
|
||||
!params.durableApprovalSatisfied
|
||||
);
|
||||
}
|
||||
|
||||
export async function processGatewayAllowlist(
|
||||
params: ProcessGatewayAllowlistParams,
|
||||
): Promise<ProcessGatewayAllowlistResult> {
|
||||
@@ -330,10 +343,13 @@ export async function processGatewayAllowlist(
|
||||
}
|
||||
|
||||
if (
|
||||
hostSecurity === "allowlist" &&
|
||||
(!analysisOk || !allowlistSatisfied) &&
|
||||
!approvedByAsk &&
|
||||
!durableApprovalSatisfied
|
||||
hasGatewayAllowlistMiss({
|
||||
hostSecurity,
|
||||
analysisOk,
|
||||
allowlistSatisfied,
|
||||
durableApprovalSatisfied,
|
||||
})
|
||||
) {
|
||||
deniedReason = deniedReason ?? "allowlist-miss";
|
||||
}
|
||||
@@ -406,9 +422,12 @@ export async function processGatewayAllowlist(
|
||||
}
|
||||
|
||||
if (
|
||||
hostSecurity === "allowlist" &&
|
||||
(!analysisOk || !allowlistSatisfied) &&
|
||||
!durableApprovalSatisfied
|
||||
hasGatewayAllowlistMiss({
|
||||
hostSecurity,
|
||||
analysisOk,
|
||||
allowlistSatisfied,
|
||||
durableApprovalSatisfied,
|
||||
})
|
||||
) {
|
||||
throw new Error("exec denied: allowlist miss");
|
||||
}
|
||||
|
||||
@@ -165,6 +165,32 @@ async function expectGatewayExecWithoutApproval(options: {
|
||||
expect(calls).not.toContain("exec.approval.waitDecision");
|
||||
}
|
||||
|
||||
async function expectGatewayAskAlwaysPrompt(options: {
|
||||
turnId: string;
|
||||
command?: string;
|
||||
allowlist?: Array<{ pattern: string; source?: "allow-always" }>;
|
||||
}) {
|
||||
await writeExecApprovalsConfig({
|
||||
version: 1,
|
||||
defaults: { security: "full", ask: "always", askFallback: "full" },
|
||||
agents: {
|
||||
main: options.allowlist ? { allowlist: options.allowlist } : {},
|
||||
},
|
||||
});
|
||||
mockPendingApprovalRegistration();
|
||||
|
||||
const tool = createExecTool({
|
||||
host: "gateway",
|
||||
ask: "always",
|
||||
security: "full",
|
||||
approvalRunningNoticeMs: 0,
|
||||
});
|
||||
|
||||
return await tool.execute(options.turnId, {
|
||||
command: options.command ?? `${JSON.stringify(process.execPath)} --version`,
|
||||
});
|
||||
}
|
||||
|
||||
function mockAcceptedApprovalFlow(options: {
|
||||
onAgent?: (params: Record<string, unknown>) => void;
|
||||
onNodeInvoke?: (params: unknown) => unknown;
|
||||
@@ -484,26 +510,9 @@ describe("exec approvals", () => {
|
||||
});
|
||||
|
||||
it("keeps ask=always prompts even when durable allow-always trust matches", async () => {
|
||||
await writeExecApprovalsConfig({
|
||||
version: 1,
|
||||
defaults: { security: "full", ask: "always", askFallback: "full" },
|
||||
agents: {
|
||||
main: {
|
||||
allowlist: [{ pattern: process.execPath, source: "allow-always" }],
|
||||
},
|
||||
},
|
||||
});
|
||||
mockPendingApprovalRegistration();
|
||||
|
||||
const tool = createExecTool({
|
||||
host: "gateway",
|
||||
ask: "always",
|
||||
security: "full",
|
||||
approvalRunningNoticeMs: 0,
|
||||
});
|
||||
|
||||
const result = await tool.execute("call-gateway-durable-still-prompts", {
|
||||
command: `${JSON.stringify(process.execPath)} --version`,
|
||||
const result = await expectGatewayAskAlwaysPrompt({
|
||||
turnId: "call-gateway-durable-still-prompts",
|
||||
allowlist: [{ pattern: process.execPath, source: "allow-always" }],
|
||||
});
|
||||
|
||||
expect(result.details.status).toBe("approval-pending");
|
||||
@@ -517,26 +526,9 @@ describe("exec approvals", () => {
|
||||
});
|
||||
|
||||
it("keeps ask=always prompts for static allowlist entries without allow-always trust", async () => {
|
||||
await writeExecApprovalsConfig({
|
||||
version: 1,
|
||||
defaults: { security: "full", ask: "always", askFallback: "full" },
|
||||
agents: {
|
||||
main: {
|
||||
allowlist: [{ pattern: process.execPath }],
|
||||
},
|
||||
},
|
||||
});
|
||||
mockPendingApprovalRegistration();
|
||||
|
||||
const tool = createExecTool({
|
||||
host: "gateway",
|
||||
ask: "always",
|
||||
security: "full",
|
||||
approvalRunningNoticeMs: 0,
|
||||
});
|
||||
|
||||
const result = await tool.execute("call-static-allowlist-still-prompts", {
|
||||
command: `${JSON.stringify(process.execPath)} --version`,
|
||||
const result = await expectGatewayAskAlwaysPrompt({
|
||||
turnId: "call-static-allowlist-still-prompts",
|
||||
allowlist: [{ pattern: process.execPath }],
|
||||
});
|
||||
|
||||
expect(result.details.status).toBe("approval-pending");
|
||||
|
||||
@@ -146,6 +146,19 @@ describe("exec approvals policy helpers", () => {
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("treats fully allow-always-matched segments as durable trust", () => {
|
||||
expect(
|
||||
hasDurableExecApproval({
|
||||
analysisOk: true,
|
||||
segmentAllowlistEntries: [
|
||||
{ pattern: "/usr/bin/echo", source: "allow-always" },
|
||||
{ pattern: "/usr/bin/printf", source: "allow-always" },
|
||||
],
|
||||
allowlist: [],
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("marks policy-blocked segments as non-durable allowlist entries", () => {
|
||||
const executable = makeMockExecutableResolution({
|
||||
rawExecutable: "/usr/bin/echo",
|
||||
|
||||
@@ -727,24 +727,16 @@ export function hasDurableExecApproval(params: {
|
||||
allowlist?: readonly ExecAllowlistEntry[];
|
||||
commandText?: string | null;
|
||||
}): boolean {
|
||||
const normalizedCommand = params.commandText?.trim();
|
||||
const commandPattern = normalizedCommand
|
||||
? buildDurableCommandApprovalPattern(normalizedCommand)
|
||||
: null;
|
||||
const exactCommandMatch = normalizedCommand
|
||||
? (params.allowlist ?? []).some(
|
||||
(entry) =>
|
||||
entry.source === "allow-always" &&
|
||||
(entry.pattern === commandPattern ||
|
||||
(typeof entry.commandText === "string" &&
|
||||
entry.commandText.trim() === normalizedCommand)),
|
||||
)
|
||||
: false;
|
||||
const allowlistMatch =
|
||||
params.analysisOk &&
|
||||
params.segmentAllowlistEntries.length > 0 &&
|
||||
params.segmentAllowlistEntries.every((entry) => entry?.source === "allow-always");
|
||||
return exactCommandMatch || allowlistMatch;
|
||||
return (
|
||||
hasExactCommandDurableExecApproval({
|
||||
allowlist: params.allowlist,
|
||||
commandText: params.commandText,
|
||||
}) ||
|
||||
hasSegmentDurableExecApproval({
|
||||
analysisOk: params.analysisOk,
|
||||
segmentAllowlistEntries: params.segmentAllowlistEntries,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
function buildDurableCommandApprovalPattern(commandText: string): string {
|
||||
@@ -752,6 +744,34 @@ function buildDurableCommandApprovalPattern(commandText: string): string {
|
||||
return `=command:${digest}`;
|
||||
}
|
||||
|
||||
function hasExactCommandDurableExecApproval(params: {
|
||||
allowlist?: readonly ExecAllowlistEntry[];
|
||||
commandText?: string | null;
|
||||
}): boolean {
|
||||
const normalizedCommand = params.commandText?.trim();
|
||||
if (!normalizedCommand) {
|
||||
return false;
|
||||
}
|
||||
const commandPattern = buildDurableCommandApprovalPattern(normalizedCommand);
|
||||
return (params.allowlist ?? []).some(
|
||||
(entry) =>
|
||||
entry.source === "allow-always" &&
|
||||
(entry.pattern === commandPattern ||
|
||||
(typeof entry.commandText === "string" && entry.commandText.trim() === normalizedCommand)),
|
||||
);
|
||||
}
|
||||
|
||||
function hasSegmentDurableExecApproval(params: {
|
||||
analysisOk: boolean;
|
||||
segmentAllowlistEntries: Array<ExecAllowlistEntry | null>;
|
||||
}): boolean {
|
||||
return (
|
||||
params.analysisOk &&
|
||||
params.segmentAllowlistEntries.length > 0 &&
|
||||
params.segmentAllowlistEntries.every((entry) => entry?.source === "allow-always")
|
||||
);
|
||||
}
|
||||
|
||||
export function recordAllowlistUse(
|
||||
approvals: ExecApprovalsFile,
|
||||
agentId: string | undefined,
|
||||
|
||||
Reference in New Issue
Block a user