fix: this is a real approval boundary bypass that tur (#323) (#62078)

This commit is contained in:
Devin Robison
2026-04-06 13:13:35 -06:00
committed by GitHub
parent 153d06f890
commit 681931345b
7 changed files with 285 additions and 19 deletions

View File

@@ -61,6 +61,7 @@ Docs: https://docs.openclaw.ai
- Agents/session keys: backfill `sessionKey` from `sessionId` in the embedded PI runner when callers omit it, so hooks, LCM, and compaction receive a valid key; also normalize whitespace-only session keys to `undefined` before downstream consumers see them. (#60555) Thanks @100yenadmin.
- Plugins/provider hooks: stop recursive provider snapshot loads from overflowing the stack during plugin initialization, while still preserving cached nested provider-hook results. (#61922, #61938, #61946, #61951)
- Discord/voice: re-arm DAVE receive passthrough without suppressing decrypt-failure rejoin recovery, and clear capture state before finalize teardown so rapid speaker restarts keep their next utterance. (#41536) Thanks @wit-oc.
- Agents/exec: keep `strictInlineEval` commands blocked after approval timeouts on both gateway and node exec hosts, so timeout fallback no longer turns timed-out inline interpreter prompts into automatic execution.
## 2026.4.5

View File

@@ -3,6 +3,13 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
const createAndRegisterDefaultExecApprovalRequestMock = vi.hoisted(() => vi.fn());
const buildExecApprovalPendingToolResultMock = vi.hoisted(() => vi.fn());
const buildExecApprovalFollowupTargetMock = vi.hoisted(() => vi.fn(() => null));
const createExecApprovalDecisionStateMock = vi.hoisted(() =>
vi.fn(() => ({
baseDecision: { timedOut: false },
approvedByAsk: false,
deniedReason: "approval-required",
})),
);
const evaluateShellAllowlistMock = vi.hoisted(() =>
vi.fn(() => ({
allowlistMatches: [],
@@ -20,6 +27,7 @@ const buildEnforcedShellCommandMock = vi.hoisted(() =>
})),
);
const recordAllowlistMatchesUseMock = vi.hoisted(() => vi.fn());
const resolveApprovalDecisionOrUndefinedMock = vi.hoisted(() => vi.fn(async () => undefined));
const resolveExecHostApprovalContextMock = vi.hoisted(() =>
vi.fn(() => ({
approvals: { allowlist: [], file: { version: 1, agents: {} } },
@@ -28,6 +36,12 @@ const resolveExecHostApprovalContextMock = vi.hoisted(() =>
askFallback: "deny",
})),
);
const runExecProcessMock = vi.hoisted(() => vi.fn());
const sendExecApprovalFollowupResultMock = vi.hoisted(() => vi.fn(async () => undefined));
const enforceStrictInlineEvalApprovalBoundaryMock = vi.hoisted(() =>
vi.fn((value: { approvedByAsk: boolean; deniedReason: string | null }) => value),
);
const detectInterpreterInlineEvalArgvMock = vi.hoisted(() => vi.fn(() => null));
vi.mock("../infra/exec-approvals.js", () => ({
evaluateShellAllowlist: evaluateShellAllowlistMock,
@@ -55,14 +69,11 @@ vi.mock("./bash-tools.exec-host-shared.js", () => ({
buildHeadlessExecApprovalDeniedMessage: vi.fn(() => "denied"),
buildExecApprovalFollowupTarget: buildExecApprovalFollowupTargetMock,
buildExecApprovalPendingToolResult: buildExecApprovalPendingToolResultMock,
createExecApprovalDecisionState: vi.fn(() => ({
baseDecision: { timedOut: false },
approvedByAsk: false,
deniedReason: "approval-required",
})),
createExecApprovalDecisionState: createExecApprovalDecisionStateMock,
createAndRegisterDefaultExecApprovalRequest: createAndRegisterDefaultExecApprovalRequestMock,
resolveApprovalDecisionOrUndefined: vi.fn(async () => undefined),
sendExecApprovalFollowupResult: vi.fn(async () => undefined),
enforceStrictInlineEvalApprovalBoundary: enforceStrictInlineEvalApprovalBoundaryMock,
resolveApprovalDecisionOrUndefined: resolveApprovalDecisionOrUndefinedMock,
sendExecApprovalFollowupResult: sendExecApprovalFollowupResultMock,
shouldResolveExecApprovalUnavailableInline: vi.fn(() => false),
}));
@@ -70,7 +81,7 @@ vi.mock("./bash-tools.exec-runtime.js", () => ({
DEFAULT_NOTIFY_TAIL_CHARS: 1000,
createApprovalSlug: vi.fn(() => "slug"),
normalizeNotifyOutput: vi.fn((value) => value),
runExecProcess: vi.fn(),
runExecProcess: runExecProcessMock,
}));
vi.mock("./bash-process-registry.js", () => ({
@@ -80,7 +91,7 @@ vi.mock("./bash-process-registry.js", () => ({
vi.mock("../infra/exec-inline-eval.js", () => ({
describeInterpreterInlineEval: vi.fn(() => "python -c"),
detectInterpreterInlineEvalArgv: vi.fn(() => null),
detectInterpreterInlineEvalArgv: detectInterpreterInlineEvalArgvMock,
}));
let processGatewayAllowlist: typeof import("./bash-tools.exec-host-gateway.js").processGatewayAllowlist;
@@ -94,6 +105,12 @@ describe("processGatewayAllowlist", () => {
buildExecApprovalPendingToolResultMock.mockReset();
buildExecApprovalFollowupTargetMock.mockReset();
buildExecApprovalFollowupTargetMock.mockReturnValue(null);
createExecApprovalDecisionStateMock.mockReset();
createExecApprovalDecisionStateMock.mockReturnValue({
baseDecision: { timedOut: false },
approvedByAsk: false,
deniedReason: "approval-required",
});
evaluateShellAllowlistMock.mockReset();
evaluateShellAllowlistMock.mockReturnValue({
allowlistMatches: [],
@@ -110,6 +127,8 @@ describe("processGatewayAllowlist", () => {
reason: "segment execution plan unavailable",
});
recordAllowlistMatchesUseMock.mockReset();
resolveApprovalDecisionOrUndefinedMock.mockReset();
resolveApprovalDecisionOrUndefinedMock.mockResolvedValue(undefined);
resolveExecHostApprovalContextMock.mockReset();
resolveExecHostApprovalContextMock.mockReturnValue({
approvals: { allowlist: [], file: { version: 1, agents: {} } },
@@ -117,6 +136,14 @@ describe("processGatewayAllowlist", () => {
hostAsk: "off",
askFallback: "deny",
});
runExecProcessMock.mockReset();
sendExecApprovalFollowupResultMock.mockReset();
enforceStrictInlineEvalApprovalBoundaryMock.mockReset();
enforceStrictInlineEvalApprovalBoundaryMock.mockImplementation(
(value: { approvedByAsk: boolean; deniedReason: string | null }) => value,
);
detectInterpreterInlineEvalArgvMock.mockReset();
detectInterpreterInlineEvalArgvMock.mockReturnValue(null);
buildExecApprovalPendingToolResultMock.mockReturnValue({
details: { status: "approval-pending" },
content: [],
@@ -242,4 +269,96 @@ describe("processGatewayAllowlist", () => {
}),
);
});
it("denies timed-out inline-eval requests instead of auto-running them", async () => {
resolveExecHostApprovalContextMock.mockReturnValue({
approvals: { allowlist: [], file: { version: 1, agents: {} } },
hostSecurity: "full",
hostAsk: "always",
askFallback: "full",
});
detectInterpreterInlineEvalArgvMock.mockReturnValue({ kind: "python-c" });
resolveApprovalDecisionOrUndefinedMock.mockResolvedValue(null);
createExecApprovalDecisionStateMock.mockReturnValue({
baseDecision: { timedOut: true },
approvedByAsk: true,
deniedReason: null,
});
enforceStrictInlineEvalApprovalBoundaryMock.mockReturnValue({
approvedByAsk: false,
deniedReason: "approval-timeout",
});
const result = await processGatewayAllowlist({
command: "python3 -c 'print(1)'",
workdir: process.cwd(),
env: process.env as Record<string, string>,
pty: false,
defaultTimeoutSec: 30,
security: "full",
ask: "always",
safeBins: new Set(),
safeBinProfiles: {},
strictInlineEval: true,
warnings: [],
approvalRunningNoticeMs: 0,
maxOutput: 1000,
pendingMaxOutput: 1000,
});
expect(result.pendingResult?.details.status).toBe("approval-pending");
await vi.waitFor(() => {
expect(sendExecApprovalFollowupResultMock).toHaveBeenCalledWith(
null,
"Exec denied (gateway id=req-1, approval-timeout): python3 -c 'print(1)'",
);
});
expect(runExecProcessMock).not.toHaveBeenCalled();
});
it("denies allowlist timeout fallback for strict inline-eval commands", async () => {
resolveExecHostApprovalContextMock.mockReturnValue({
approvals: { allowlist: [], file: { version: 1, agents: {} } },
hostSecurity: "allowlist",
hostAsk: "always",
askFallback: "allowlist",
});
detectInterpreterInlineEvalArgvMock.mockReturnValue({ kind: "python-c" });
resolveApprovalDecisionOrUndefinedMock.mockResolvedValue(null);
createExecApprovalDecisionStateMock.mockReturnValue({
baseDecision: { timedOut: true },
approvedByAsk: false,
deniedReason: null,
});
enforceStrictInlineEvalApprovalBoundaryMock.mockReturnValue({
approvedByAsk: false,
deniedReason: "approval-timeout",
});
const result = await processGatewayAllowlist({
command: "python3 -c 'print(1)'",
workdir: process.cwd(),
env: process.env as Record<string, string>,
pty: false,
defaultTimeoutSec: 30,
security: "allowlist",
ask: "always",
safeBins: new Set(),
safeBinProfiles: {},
strictInlineEval: true,
warnings: [],
approvalRunningNoticeMs: 0,
maxOutput: 1000,
pendingMaxOutput: 1000,
});
expect(result.pendingResult?.details.status).toBe("approval-pending");
await vi.waitFor(() => {
expect(sendExecApprovalFollowupResultMock).toHaveBeenCalledWith(
null,
"Exec denied (gateway id=req-1, approval-timeout): python3 -c 'print(1)'",
);
});
expect(runExecProcessMock).not.toHaveBeenCalled();
});
});

View File

@@ -30,6 +30,7 @@ import {
buildExecApprovalPendingToolResult,
createExecApprovalDecisionState,
createAndRegisterDefaultExecApprovalRequest,
enforceStrictInlineEvalApprovalBoundary,
resolveApprovalDecisionOrUndefined,
resolveExecHostApprovalContext,
sendExecApprovalFollowupResult,
@@ -238,12 +239,18 @@ export async function processGatewayAllowlist(
preResolvedDecision,
})
) {
const { approvedByAsk, deniedReason } = createExecApprovalDecisionState({
const { baseDecision, approvedByAsk, deniedReason } = createExecApprovalDecisionState({
decision: preResolvedDecision,
askFallback,
});
const strictInlineEvalDecision = enforceStrictInlineEvalApprovalBoundary({
baseDecision,
approvedByAsk,
deniedReason,
requiresInlineEvalApproval,
});
if (deniedReason || !approvedByAsk) {
if (strictInlineEvalDecision.deniedReason || !strictInlineEvalDecision.approvedByAsk) {
throw new Error(
buildHeadlessExecApprovalDeniedMessage({
trigger: params.trigger,
@@ -332,6 +339,13 @@ export async function processGatewayAllowlist(
}
}
({ approvedByAsk, deniedReason } = enforceStrictInlineEvalApprovalBoundary({
baseDecision,
approvedByAsk,
deniedReason,
requiresInlineEvalApproval,
}));
if (
!approvedByAsk &&
hasGatewayAllowlistMiss({

View File

@@ -36,9 +36,14 @@ const createExecApprovalDecisionStateMock = vi.hoisted(() =>
})),
);
const buildExecApprovalPendingToolResultMock = vi.hoisted(() => vi.fn());
const sendExecApprovalFollowupResultMock = vi.hoisted(() => vi.fn(async () => undefined));
const enforceStrictInlineEvalApprovalBoundaryMock = vi.hoisted(() =>
vi.fn((value: { approvedByAsk: boolean; deniedReason: string | null }) => value),
);
const registerExecApprovalRequestForHostOrThrowMock = vi.hoisted(() =>
vi.fn(async () => undefined),
);
const detectInterpreterInlineEvalArgvMock = vi.hoisted(() => vi.fn(() => null));
vi.mock("../infra/exec-approvals.js", () => ({
evaluateShellAllowlist: vi.fn(() => ({
@@ -59,7 +64,7 @@ vi.mock("../infra/exec-approvals.js", () => ({
vi.mock("../infra/exec-inline-eval.js", () => ({
describeInterpreterInlineEval: vi.fn(() => "inline-eval"),
detectInterpreterInlineEvalArgv: vi.fn(() => null),
detectInterpreterInlineEvalArgv: detectInterpreterInlineEvalArgvMock,
}));
vi.mock("../infra/node-shell.js", () => ({
@@ -84,7 +89,8 @@ vi.mock("./bash-tools.exec-host-shared.js", () => ({
buildExecApprovalFollowupTarget: vi.fn(() => ({ approvalId: "approval-1" })),
resolveApprovalDecisionOrUndefined: resolveApprovalDecisionOrUndefinedMock,
createExecApprovalDecisionState: createExecApprovalDecisionStateMock,
sendExecApprovalFollowupResult: vi.fn(async () => undefined),
enforceStrictInlineEvalApprovalBoundary: enforceStrictInlineEvalApprovalBoundaryMock,
sendExecApprovalFollowupResult: sendExecApprovalFollowupResultMock,
buildExecApprovalPendingToolResult: buildExecApprovalPendingToolResultMock,
buildHeadlessExecApprovalDeniedMessage: vi.fn(() => "denied"),
}));
@@ -189,6 +195,13 @@ describe("executeNodeHostCommand", () => {
content: [],
details: { status: "approval-pending" },
});
sendExecApprovalFollowupResultMock.mockReset();
enforceStrictInlineEvalApprovalBoundaryMock.mockReset();
enforceStrictInlineEvalApprovalBoundaryMock.mockImplementation(
(value: { approvedByAsk: boolean; deniedReason: string | null }) => value,
);
detectInterpreterInlineEvalArgvMock.mockReset();
detectInterpreterInlineEvalArgvMock.mockReturnValue(null);
registerExecApprovalRequestForHostOrThrowMock.mockReset();
});
@@ -231,4 +244,47 @@ describe("executeNodeHostCommand", () => {
}),
);
});
it("denies timed-out inline-eval requests instead of invoking the node", async () => {
detectInterpreterInlineEvalArgvMock.mockReturnValue({ kind: "python-c" });
resolveApprovalDecisionOrUndefinedMock.mockResolvedValue(null);
createExecApprovalDecisionStateMock.mockReturnValue({
baseDecision: { timedOut: true },
approvedByAsk: true,
deniedReason: null,
});
enforceStrictInlineEvalApprovalBoundaryMock.mockReturnValue({
approvedByAsk: false,
deniedReason: "approval-timeout",
});
resolveExecHostApprovalContextMock.mockReturnValue({
approvals: { allowlist: [], file: { version: 1, agents: {} } },
hostSecurity: "full",
hostAsk: "off",
askFallback: "full",
});
const result = await executeNodeHostCommand({
command: "python3 -c 'print(1)'",
workdir: "/tmp/work",
env: {},
security: "full",
ask: "off",
strictInlineEval: true,
defaultTimeoutSec: 30,
approvalRunningNoticeMs: 0,
warnings: [],
agentId: "requested-agent",
sessionKey: "requested-session",
});
expect(result.details?.status).toBe("approval-pending");
await vi.waitFor(() => {
expect(sendExecApprovalFollowupResultMock).toHaveBeenCalledWith(
{ approvalId: "approval-1" },
"Exec denied (node=node-1 id=approval-1, approval-timeout): python3 -c 'print(1)'",
);
});
expect(callGatewayToolMock).toHaveBeenCalledTimes(1);
});
});

View File

@@ -280,11 +280,18 @@ export async function executeNodeHostCommand(
preResolvedDecision,
})
) {
const { approvedByAsk, deniedReason } = execHostShared.createExecApprovalDecisionState({
decision: preResolvedDecision,
askFallback,
const { baseDecision, approvedByAsk, deniedReason } =
execHostShared.createExecApprovalDecisionState({
decision: preResolvedDecision,
askFallback,
});
const strictInlineEvalDecision = execHostShared.enforceStrictInlineEvalApprovalBoundary({
baseDecision,
approvedByAsk,
deniedReason,
requiresInlineEvalApproval: inlineEvalHit !== null,
});
if (deniedReason || !approvedByAsk) {
if (strictInlineEvalDecision.deniedReason || !strictInlineEvalDecision.approvedByAsk) {
throw new Error(
execHostShared.buildHeadlessExecApprovalDeniedMessage({
trigger: params.trigger,
@@ -295,8 +302,8 @@ export async function executeNodeHostCommand(
}),
);
}
inlineApprovedByAsk = approvedByAsk;
inlineApprovalDecision = approvedByAsk ? "allow-once" : null;
inlineApprovedByAsk = strictInlineEvalDecision.approvedByAsk;
inlineApprovalDecision = strictInlineEvalDecision.approvedByAsk ? "allow-once" : null;
inlineApprovalId = approvalId;
} else {
const followupTarget = execHostShared.buildExecApprovalFollowupTarget({
@@ -344,6 +351,16 @@ export async function executeNodeHostCommand(
approvalDecision = "allow-always";
}
({ approvedByAsk, deniedReason } = execHostShared.enforceStrictInlineEvalApprovalBoundary({
baseDecision,
approvedByAsk,
deniedReason,
requiresInlineEvalApproval: inlineEvalHit !== null,
}));
if (deniedReason) {
approvalDecision = null;
}
if (deniedReason) {
await execHostShared.sendExecApprovalFollowupResult(
followupTarget,

View File

@@ -39,6 +39,7 @@ vi.mock("../infra/exec-approvals.js", async (importOriginal) => {
let sendExecApprovalFollowupResult: typeof import("./bash-tools.exec-host-shared.js").sendExecApprovalFollowupResult;
let maxExecApprovalFollowupFailureLogKeys: typeof import("./bash-tools.exec-host-shared.js").MAX_EXEC_APPROVAL_FOLLOWUP_FAILURE_LOG_KEYS;
let enforceStrictInlineEvalApprovalBoundary: typeof import("./bash-tools.exec-host-shared.js").enforceStrictInlineEvalApprovalBoundary;
let resolveExecHostApprovalContext: typeof import("./bash-tools.exec-host-shared.js").resolveExecHostApprovalContext;
let sendExecApprovalFollowup: typeof import("./bash-tools.exec-approval-followup.js").sendExecApprovalFollowup;
let logWarn: typeof import("../logger.js").logWarn;
@@ -47,6 +48,7 @@ beforeAll(async () => {
({
sendExecApprovalFollowupResult,
MAX_EXEC_APPROVAL_FOLLOWUP_FAILURE_LOG_KEYS: maxExecApprovalFollowupFailureLogKeys,
enforceStrictInlineEvalApprovalBoundary,
resolveExecHostApprovalContext,
} = await import("./bash-tools.exec-host-shared.js"));
({ sendExecApprovalFollowup } = await import("./bash-tools.exec-approval-followup.js"));
@@ -148,3 +150,33 @@ describe("resolveExecHostApprovalContext", () => {
expect(result.hostSecurity).toBe("full");
});
});
describe("enforceStrictInlineEvalApprovalBoundary", () => {
it("denies timeout-based fallback when strict inline-eval approval is required", () => {
expect(
enforceStrictInlineEvalApprovalBoundary({
baseDecision: { timedOut: true },
approvedByAsk: true,
deniedReason: null,
requiresInlineEvalApproval: true,
}),
).toEqual({
approvedByAsk: false,
deniedReason: "approval-timeout",
});
});
it("keeps explicit approvals intact for strict inline-eval commands", () => {
expect(
enforceStrictInlineEvalApprovalBoundary({
baseDecision: { timedOut: false },
approvedByAsk: true,
deniedReason: null,
requiresInlineEvalApproval: true,
}),
).toEqual({
approvedByAsk: true,
deniedReason: null,
});
});
});

View File

@@ -341,6 +341,33 @@ export function createExecApprovalDecisionState(params: {
};
}
export function enforceStrictInlineEvalApprovalBoundary(params: {
baseDecision: {
timedOut: boolean;
};
approvedByAsk: boolean;
deniedReason: string | null;
requiresInlineEvalApproval: boolean;
}): {
approvedByAsk: boolean;
deniedReason: string | null;
} {
if (
!params.baseDecision.timedOut ||
!params.requiresInlineEvalApproval ||
!params.approvedByAsk
) {
return {
approvedByAsk: params.approvedByAsk,
deniedReason: params.deniedReason,
};
}
return {
approvedByAsk: false,
deniedReason: params.deniedReason ?? "approval-timeout",
};
}
export function shouldResolveExecApprovalUnavailableInline(params: {
trigger?: string;
unavailableReason: ExecApprovalUnavailableReason | null;