feat(gateway): fail-closed plugin and tool approval gates

Squash-rebased #103932 segment onto the ancestor-propagation tip on current main. Plugin node.invoke approvals claim a one-shot allow-once decision before handing execution authority to the policy, so observation or retry cannot replay a consumed approval; sibling tool gates bind approval ids to their originating reviewer identity. (cherry picked from commit 122df0d75281f572c012b6484ed5daf085d1d577) (cherry picked from commit f23d8ac240a8dcf2a42c4daaae962263975a0ae8) (cherry picked from commit 8632fb6436a224dac7a9a9ef0216884530de2c24) (cherry picked from commit d9fe2dd5c53665e5732f5f3da5ed1907a686ade8) (cherry picked from commit bb139f2c8aa36ddad70413e1ef79ff491a6f2ecd) (cherry picked from commit 9b3e056b68ea515c7d4baef269289b6670570480) (cherry picked from commit b11e66b59a7af1f3b08712de06864aed64d349e4) (cherry picked from commit a12916ee592d03dfa35e86a2aaede4476a5d4e5d) (cherry picked from commit d699de840fc8346892b9ae244fe3c3e8c4413032) (cherry picked from commit 9b7e5a9608f94732226776b801b3a2ac65329e05) (cherry picked from commit 4b507593f1b2f10b4496984c84a2803b853e5c5e) (cherry picked from commit 56408a186733c4eeb5aa76bd5907cde7b0cd3d5b) (cherry picked from commit a4051d6d8353d39d6fa0b5d69c36e06c3cd3e796) (cherry picked from commit 76829805a7135ed3a7984ae372edd0d872cb0dfc) (cherry picked from commit a8b493f934b644ef8688a8044c2bdcdcc08d686d) (cherry picked from commit aceb990597affb8388c341a10cb386ef96bed3e4) (cherry picked from commit a29b0e75482470c53a9fb4fe3f204e14cf71868f)
2026-08-12 21:53:00 -06:00 · 2026-07-11 18:10:40 -04:00
parent 93ba8c4b09
commit 2e3be08653
16 changed files with 554 additions and 94 deletions
@@ -1,2 +1,2 @@
-634a2cda48d53c9a234d59200b4666c105615da4d64612ea30facb42df9c4d93  plugin-sdk-api-baseline.json
-f5bc2ec06dbdc26736f8d810d75f72f378ffd2111c2a143ed153a284b6d89d25  plugin-sdk-api-baseline.jsonl
+d6747bc65237cf327f9f307e405347969292aee5f711ad41a1c8f6c5bc29e7ce  plugin-sdk-api-baseline.json
+678d8dddf40f8a0830ae2fe678a40af17a4dfbd550cfa41cc0a5e25f12ecfcb3  plugin-sdk-api-baseline.jsonl
@@ -40,7 +40,6 @@ export default definePluginEntry({
            description: `Allow search query: ${String(event.params.query ?? "")}`,
            severity: "info",
            timeoutMs: 60_000,
-            timeoutBehavior: "deny",
          },
        };
      },
@@ -251,6 +250,7 @@ type BeforeToolCallResult = {
    description: string;
    severity?: "info" | "warning" | "critical";
    timeoutMs?: number;
+    /** @deprecated Unresolved approvals always deny. */
    timeoutBehavior?: "allow" | "deny";
    allowedDecisions?: Array<"allow-once" | "allow-always" | "deny">;
    pluginId?: string;
@@ -63,7 +63,6 @@ export default definePluginEntry({
              ? ["allow-once", "deny"]
              : ["allow-once", "allow-always", "deny"],
          timeoutMs: 120_000,
-          timeoutBehavior: "deny",
          onResolution(decision) {
            console.log(`deploy approval resolved: ${decision}`);
          },
@@ -99,10 +98,15 @@ available approval surfaces, and waits for a decision.
 | `allow-once`      | The current call continues.                                               |
 | `allow-always`    | The current call continues and the decision is passed to the plugin.      |
 | `deny`            | The call is blocked with a denied tool result.                            |
-| Timeout           | The call is blocked unless `timeoutBehavior` is `"allow"`.                |
+| Timeout           | The call is blocked.                                                      |
 | Cancellation      | The call is blocked when the run is aborted.                              |
 | No approval route | The call is blocked because no connected approval surface can resolve it. |

+Only the exact `allow-once` and `allow-always` decisions permitted by the
+request allow execution. Unknown, malformed, mismatched, missing, and timed-out
+decisions fail closed. The legacy `timeoutBehavior` field remains accepted for
+plugin compatibility but is deprecated and ignored; do not set it in new hooks.
+
 `allow-always` is only durable when the requesting plugin or runtime implements
 that persistence. For ordinary `before_tool_call.requireApproval` hooks,
 OpenClaw treats `allow-once` and `allow-always` as approval decisions for the
@@ -335,13 +335,17 @@ Final strict behavior:
 - malformed trusted verdict -> `denied`, deny;
 - only an allowed explicit allow decision -> `allowed`.

-Current shipped behavior conflicts with this contract:
+Current shipped exec behavior still conflicts with this contract:

 - `src/agents/bash-tools.exec-host-shared.ts` may apply `askFallback`.
- `src/agents/agent-tools.before-tool-call.ts` may honor `timeoutBehavior: "allow"`.
- `docs/tools/exec-approvals.md`, `docs/cli/approvals.md`, and `docs/plugins/plugin-permission-requests.md` document those surfaces.
+- `docs/tools/exec-approvals.md` and `docs/cli/approvals.md` document that surface.

-Do not silently change them in the storage PR. The strict-semantics PR must update code, types, docs, tests, and changelog together, with explicit owner/security review. `askFallback` may continue to describe pre-gate policy selection during migration, but it must not turn a created pending record's timeout into approval.
+Plugin approvals now fail closed on timeout and malformed verdicts; the legacy
+`timeoutBehavior` field remains accepted but ignored. The exec strict-semantics
+follow-up must update code, types, docs, tests, and changelog together, with
+explicit owner/security review. `askFallback` may continue to describe
+pre-gate policy selection during migration, but it must not turn a created
+pending record's timeout into approval.

 ## Compatibility plan

@@ -460,5 +464,5 @@ A committed transition is success even if later event delivery fails. Lifecycle
 ## Open decisions

 1. **Externally reachable Control UI origin.** Every snapshot carries the stable relative `urlPath`. An absolute URL may be advertised only from a cached Tailscale Serve/Funnel location after Gateway exposure succeeds; `allowedOrigins`, request Host headers, `gateway.remote.url`, and display-only loopback/LAN candidates are not canonical origins. Telegram can use its authenticated Mini App wrapper to retain the approval path through bootstrap. Arbitrary reverse proxies remain relative-only until a separately reviewed explicit public-URL contract exists. Never let a channel guess the origin.
-2. **Strict timeout compatibility cutover.** The target is fail-closed, but `askFallback` and plugin `timeoutBehavior: "allow"` are shipped contracts. Recommended: make the behavior change in PR 6 with explicit owner/security approval, changelog, docs, and a migration/deprecation decision rather than hiding it in PR 1.
+2. **Exec strict timeout compatibility cutover.** Plugin approval timeouts now fail closed and `timeoutBehavior` is deprecated. The remaining shipped `askFallback` contract needs explicit owner/security review, changelog, docs, and a migration/deprecation decision before it stops authorizing execution after a pending ask times out.
 3. **Gatewayless embedded mode.** Recommended: keep it local-only initially, then make it a client of the canonical service when a Gateway exists. Do not advertise a deep link that no server can resolve.
@@ -2248,6 +2248,36 @@ describe("Codex app-server approval bridge", () => {
    });
  });

+  it("ignores waitDecision replies bound to a different approval id", async () => {
+    const params = createParams();
+    const onNativeToolFailureDisposition = vi.fn();
+    mockCallGatewayTool
+      .mockResolvedValueOnce({ id: "plugin:approval-mismatch", status: "accepted" })
+      .mockResolvedValueOnce({ id: "plugin:approval-other", decision: "allow-once" });
+
+    const result = await handleCodexAppServerApprovalRequest({
+      method: "item/commandExecution/requestApproval",
+      requestParams: {
+        threadId: "thread-1",
+        turnId: "turn-1",
+        itemId: "cmd-mismatch",
+        command: "pnpm test",
+      },
+      paramsForRun: params,
+      threadId: "thread-1",
+      turnId: "turn-1",
+      onNativeToolFailureDisposition,
+    });
+
+    // A misrouted allow for another approval must not release this gate.
+    expect(result).toEqual({ decision: "decline" });
+    expect(onNativeToolFailureDisposition).toHaveBeenCalledWith("cmd-mismatch", "failed");
+    findApprovalEvent(params, {
+      status: "unavailable",
+      approvalId: "plugin:approval-mismatch",
+    });
+  });
+
  it("sanitizes reason previews before forwarding approval text and events", async () => {
    const params = createParams();
    mockCallGatewayTool.mockResolvedValueOnce({
@@ -93,8 +93,14 @@ export async function waitForPluginApprovalDecision(params: {
    { timeoutMs: resolveCodexGatewayTimeoutWithGraceMs(timeoutMs) },
    { id: params.approvalId },
  );
+  // Bind the verdict to the approval that parked this prompt. A stale or
+  // misrouted reply maps to "unavailable" instead of releasing another gate.
+  const bindDecision = (
+    result: ApprovalWaitResult | undefined,
+  ): ExecApprovalDecision | null | undefined =>
+    result?.id === params.approvalId ? result.decision : undefined;
  if (!params.signal) {
-    return (await waitPromise)?.decision;
+    return bindDecision(await waitPromise);
  }
  let onAbort: (() => void) | undefined;
  const abortPromise = new Promise<never>((_, reject) => {
@@ -106,7 +112,7 @@ export async function waitForPluginApprovalDecision(params: {
    params.signal!.addEventListener("abort", onAbort, { once: true });
  });
  try {
-    return (await Promise.race([waitPromise, abortPromise]))?.decision;
+    return bindDecision(await Promise.race([waitPromise, abortPromise]));
  } finally {
    if (onAbort) {
      params.signal.removeEventListener("abort", onAbort);
@@ -270,11 +270,95 @@ describe("file-transfer node invoke policy", () => {
    expect(invokeNode).not.toHaveBeenCalled();
  });

-  it("uses plugin approvals for ask-on-miss before invoking the node", async () => {
+  it.each(["allow-once", "allow-always"] as const)(
+    "uses exact %s plugin approval once across preflight and final invoke",
+    async (decision) => {
+      const policy = createFileTransferNodeInvokePolicy();
+      const approvals = {
+        request: vi.fn(async () => ({ id: "approval-1", decision })),
+      };
+      const { ctx, invokeNode } = createCtx({
+        params: { path: "/tmp/new.txt" },
+        pluginConfig: {
+          nodes: {
+            "node-1": {
+              ask: "on-miss",
+              allowReadPaths: ["/allowed/**"],
+              maxBytes: 256,
+            },
+          },
+        },
+        approvals,
+      });
+
+      const result = await policy.handle(ctx);
+
+      expect(result.ok).toBe(true);
+      expect(approvals.request).toHaveBeenCalledTimes(1);
+      expect(invokeNode).toHaveBeenCalledTimes(2);
+      const approvalCalls = approvals.request.mock.calls as unknown[][];
+      const approvalRequest = requireRecord(approvalCalls[0]?.[0], "approval request");
+      expectRecordFields(approvalRequest, {
+        title: "Read file: /tmp/new.txt",
+        severity: "info",
+        toolName: "file.fetch",
+      });
+      expect(invokeNode).toHaveBeenNthCalledWith(1, {
+        params: {
+          path: "/tmp/new.txt",
+          followSymlinks: false,
+          maxBytes: 256,
+          preflightOnly: true,
+        },
+      });
+      expect(invokeNode).toHaveBeenNthCalledWith(2, {
+        params: {
+          path: "/tmp/new.txt",
+          followSymlinks: false,
+          maxBytes: 256,
+        },
+      });
+    },
+  );
+
+  it.each([
+    {
+      label: "explicit deny",
+      decision: "deny",
+      code: "APPROVAL_DENIED",
+      message: "file.fetch APPROVAL_DENIED: operator denied the prompt",
+    },
+    {
+      label: "null decision",
+      decision: null,
+      code: "APPROVAL_UNAVAILABLE",
+      message:
+        "file.fetch APPROVAL_UNAVAILABLE: no operator client connected to approve the request",
+    },
+    {
+      label: "undefined decision",
+      decision: undefined,
+      code: "APPROVAL_UNAVAILABLE",
+      message:
+        "file.fetch APPROVAL_UNAVAILABLE: no operator client connected to approve the request",
+    },
+    {
+      label: "arbitrary truthy string",
+      decision: "accept",
+      code: "APPROVAL_DENIED",
+      message: "file.fetch APPROVAL_DENIED: invalid approval decision",
+    },
+    {
+      label: "arbitrary truthy object",
+      decision: { action: "accept" },
+      code: "APPROVAL_DENIED",
+      message: "file.fetch APPROVAL_DENIED: invalid approval decision",
+    },
+  ])("fails closed for $label", async ({ decision, code, message }) => {
    const policy = createFileTransferNodeInvokePolicy();
    const approvals = {
-      request: vi.fn(async () => ({ id: "approval-1", decision: "allow-once" as const })),
-    };
+      request: vi.fn(async () => ({ id: "approval-1", decision })),
+    } as unknown as NonNullable<OpenClawPluginNodeInvokePolicyContext["approvals"]>;
    const { ctx, invokeNode } = createCtx({
      params: { path: "/tmp/new.txt" },
      pluginConfig: {
@@ -282,7 +366,6 @@ describe("file-transfer node invoke policy", () => {
          "node-1": {
            ask: "on-miss",
            allowReadPaths: ["/allowed/**"],
-            maxBytes: 256,
          },
        },
      },
@@ -291,29 +374,9 @@ describe("file-transfer node invoke policy", () => {

    const result = await policy.handle(ctx);

-    expect(result.ok).toBe(true);
-    const approvalCalls = approvals.request.mock.calls as unknown[][];
-    const approvalRequest = requireRecord(approvalCalls[0]?.[0], "approval request");
-    expectRecordFields(approvalRequest, {
-      title: "Read file: /tmp/new.txt",
-      severity: "info",
-      toolName: "file.fetch",
-    });
-    expect(invokeNode).toHaveBeenNthCalledWith(1, {
-      params: {
-        path: "/tmp/new.txt",
-        followSymlinks: false,
-        maxBytes: 256,
-        preflightOnly: true,
-      },
-    });
-    expect(invokeNode).toHaveBeenNthCalledWith(2, {
-      params: {
-        path: "/tmp/new.txt",
-        followSymlinks: false,
-        maxBytes: 256,
-      },
-    });
+    expectResultFields(result, { ok: false, code, message });
+    expect(approvals.request).toHaveBeenCalledTimes(1);
+    expect(invokeNode).not.toHaveBeenCalled();
  });

  it("marks node transport failures as unavailable", async () => {
@@ -154,28 +154,37 @@ async function requestApproval(input: {
    severity: input.kind === "write" ? "warning" : "info",
    toolName: input.op,
  });
+  const approvalDecision: unknown = approval.decision;

-  if (approval.decision === "deny" || approval.decision === null || !approval.decision) {
+  if (approvalDecision !== "allow-once" && approvalDecision !== "allow-always") {
+    const unavailable = approvalDecision === null || approvalDecision === undefined;
+    const deniedByOperator = approvalDecision === "deny";
+    const reason = deniedByOperator
+      ? "operator denied"
+      : unavailable
+        ? "no operator available"
+        : "invalid approval decision";
    await appendFileTransferAudit({
      op: input.op,
      nodeId: input.ctx.nodeId,
      nodeDisplayName,
      requestedPath: input.path,
      decision: "denied:approval",
-      reason: approval.decision === "deny" ? "operator denied" : "no operator available",
+      reason,
      durationMs: Date.now() - input.startedAt,
    });
    return {
      ok: false,
-      code: approval.decision === "deny" ? "APPROVAL_DENIED" : "APPROVAL_UNAVAILABLE",
-      message:
-        approval.decision === "deny"
+      code: unavailable ? "APPROVAL_UNAVAILABLE" : "APPROVAL_DENIED",
+      message: unavailable
+        ? `${input.op} APPROVAL_UNAVAILABLE: no operator client connected to approve the request`
+        : deniedByOperator
          ? `${input.op} APPROVAL_DENIED: operator denied the prompt`
-          : `${input.op} APPROVAL_UNAVAILABLE: no operator client connected to approve the request`,
+          : `${input.op} APPROVAL_DENIED: invalid approval decision`,
    };
  }

-  if (approval.decision === "allow-always") {
+  if (approvalDecision === "allow-always") {
    try {
      await persistAllowAlways({
        nodeId: input.ctx.nodeId,
@@ -228,7 +237,7 @@ async function requestApproval(input: {
    nodeId: input.ctx.nodeId,
    nodeDisplayName,
    requestedPath: input.path,
-    decision: approval.decision === "allow-always" ? "allowed:always" : "allowed:once",
+    decision: approvalDecision === "allow-always" ? "allowed:always" : "allowed:once",
    durationMs: Date.now() - input.startedAt,
  });
  return {
@@ -19,6 +19,7 @@ import {
 } from "../infra/diagnostic-events.js";
 import { MAX_PLUGIN_APPROVAL_TIMEOUT_MS } from "../infra/plugin-approvals.js";
 import { resetDiagnosticSessionStateForTest } from "../logging/diagnostic-session-state.js";
+import { PluginApprovalResolutions } from "../plugins/hook-before-tool-call-result.js";
 import { getGlobalHookRunner } from "../plugins/hook-runner-global.js";
 import { createEmptyPluginRegistry } from "../plugins/registry-empty.js";
 import { setActivePluginRegistry } from "../plugins/runtime.js";
@@ -2037,18 +2038,25 @@ describe("before_tool_call requireApproval handling", () => {
    );
  });

-  it("allows on timeout when timeoutBehavior is allow and preserves hook params", async () => {
+  it.each([
+    ["a timeout", null],
+    ["an explicit timeout decision", PluginApprovalResolutions.TIMEOUT],
+    ["an unknown decision", "approved"],
+    ["a malformed truthy decision", true as unknown as string],
+  ])("blocks on %s even when deprecated timeoutBehavior is allow", async (_label, decision) => {
+    const onResolution = vi.fn();
    hookRunner.runBeforeToolCall.mockResolvedValue({
      params: { command: "safe-command" },
      requireApproval: {
        title: "Lenient timeout",
-        description: "Should allow on timeout",
+        description: "Must fail closed",
        timeoutBehavior: "allow",
+        onResolution,
      },
    });

    mockCallGateway.mockResolvedValueOnce({ id: "server-id-4", status: "accepted" });
-    mockCallGateway.mockResolvedValueOnce({ id: "server-id-4", decision: null });
+    mockCallGateway.mockResolvedValueOnce({ id: "server-id-4", decision });

    const result = await runBeforeToolCallHook({
      toolName: "bash",
@@ -2056,10 +2064,78 @@ describe("before_tool_call requireApproval handling", () => {
      ctx: { agentId: "main", sessionKey: "main" },
    });

-    expect(result.blocked).toBe(false);
-    if (!result.blocked) {
-      expect(result.params).toEqual({ command: "safe-command" });
-    }
+    expect(result).toMatchObject({
+      blocked: true,
+      kind: "failure",
+      disposition: "timed_out",
+      deniedReason: "plugin-approval",
+      reason: "Approval timed out",
+      params: { command: "rm -rf /" },
+    });
+    expect(onResolution).toHaveBeenCalledWith(PluginApprovalResolutions.TIMEOUT);
+  });
+
+  it("blocks exact allow decisions excluded by the request", async () => {
+    const onResolution = vi.fn();
+    hookRunner.runBeforeToolCall.mockResolvedValue({
+      params: { command: "safe-command" },
+      requireApproval: {
+        title: "Restricted approval",
+        description: "Allow once only",
+        allowedDecisions: ["allow-once", "deny"],
+        onResolution,
+      },
+    });
+    mockCallGateway.mockResolvedValueOnce({ id: "server-id-restricted", status: "accepted" });
+    mockCallGateway.mockResolvedValueOnce({
+      id: "server-id-restricted",
+      decision: "allow-always",
+    });
+
+    const result = await runBeforeToolCallHook({
+      toolName: "bash",
+      params: { command: "unsafe-command" },
+      ctx: { agentId: "main", sessionKey: "main" },
+    });
+
+    expect(result).toMatchObject({
+      blocked: true,
+      disposition: "timed_out",
+      reason: "Approval timed out",
+      params: { command: "unsafe-command" },
+    });
+    expect(onResolution).toHaveBeenCalledWith(PluginApprovalResolutions.TIMEOUT);
+  });
+
+  it("blocks a wait decision bound to another approval id", async () => {
+    const onResolution = vi.fn();
+    hookRunner.runBeforeToolCall.mockResolvedValue({
+      params: { command: "safe-command" },
+      requireApproval: {
+        title: "Bound approval",
+        description: "Must match the request id",
+        onResolution,
+      },
+    });
+    mockCallGateway.mockResolvedValueOnce({ id: "server-id-bound", status: "accepted" });
+    mockCallGateway.mockResolvedValueOnce({
+      id: "server-id-other",
+      decision: "allow-once",
+    });
+
+    const result = await runBeforeToolCallHook({
+      toolName: "bash",
+      params: { command: "unsafe-command" },
+      ctx: { agentId: "main", sessionKey: "main" },
+    });
+
+    expect(result).toMatchObject({
+      blocked: true,
+      disposition: "timed_out",
+      reason: "Approval timed out",
+      params: { command: "unsafe-command" },
+    });
+    expect(onResolution).toHaveBeenCalledWith(PluginApprovalResolutions.TIMEOUT);
  });

  it("falls back to block on gateway error", async () => {
@@ -39,6 +39,19 @@ vi.mock("./tools/gateway.js", () => ({
  callGatewayTool: vi.fn(),
 }));

+const agentToolsWarnSpy = vi.hoisted(() => vi.fn());
+vi.mock("../logging/subsystem.js", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("../logging/subsystem.js")>();
+  return {
+    ...actual,
+    createSubsystemLogger: (subsystem: string) => {
+      const logger = actual.createSubsystemLogger(subsystem);
+      // Capture agents/tools warnings so the deprecation signal is assertable.
+      return subsystem === "agents/tools" ? { ...logger, warn: agentToolsWarnSpy } : logger;
+    },
+  };
+});
+
 const mockGetGlobalHookRunner = vi.mocked(getGlobalHookRunner);
 const mockCallGatewayTool = vi.mocked(callGatewayTool);

@@ -220,6 +233,120 @@ describe("runBeforeToolCallHook — embedded mode approvals", () => {
    expect(onResolution).toHaveBeenCalledWith(PluginApprovalResolutions.CANCELLED);
  });

+  it("blocks embedded approvals on timeout even when deprecated timeoutBehavior is allow", async () => {
+    setEmbeddedMode(true);
+    const broker = new EmbeddedPluginApprovalBroker();
+    setEmbeddedPluginApprovalBroker(broker);
+    const onResolution = vi.fn();
+    runBeforeToolCallMock.mockResolvedValue({
+      requireApproval: {
+        pluginId: "test-plugin",
+        title: "Needs approval",
+        description: "Test approval request",
+        timeoutMs: 1,
+        timeoutBehavior: "allow",
+        onResolution,
+      },
+      params: { adjusted: true },
+    });
+
+    const result = await runBeforeToolCallHook({
+      toolName: "exec",
+      params: { command: "ls" },
+      toolCallId: "call-skill-timeout",
+      ctx: { agentId: "main", sessionKey: "agent:main:main" },
+    });
+
+    expect(result).toEqual({
+      blocked: true,
+      kind: "failure",
+      disposition: "timed_out",
+      deniedReason: "plugin-approval",
+      reason: "Approval timed out",
+      params: { command: "ls" },
+    });
+    expect(onResolution).toHaveBeenCalledWith(PluginApprovalResolutions.TIMEOUT);
+    expect(mockCallGatewayTool).not.toHaveBeenCalled();
+  });
+
+  it("warns once per plugin when deprecated timeoutBehavior allow arrives, still failing closed", async () => {
+    agentToolsWarnSpy.mockClear();
+    setEmbeddedMode(true);
+    const broker = new EmbeddedPluginApprovalBroker();
+    setEmbeddedPluginApprovalBroker(broker);
+    runBeforeToolCallMock.mockResolvedValue({
+      requireApproval: {
+        pluginId: "deprecated-timeout-plugin",
+        title: "Needs approval",
+        description: "Test approval request",
+        timeoutMs: 1,
+        timeoutBehavior: "allow",
+      },
+    });
+
+    const first = await runBeforeToolCallHook({
+      toolName: "exec",
+      params: { command: "ls" },
+      toolCallId: "call-deprecated-warn-1",
+      ctx: { agentId: "main", sessionKey: "agent:main:main" },
+    });
+    const second = await runBeforeToolCallHook({
+      toolName: "exec",
+      params: { command: "ls" },
+      toolCallId: "call-deprecated-warn-2",
+      ctx: { agentId: "main", sessionKey: "agent:main:main" },
+    });
+
+    expect(first).toMatchObject({ blocked: true, disposition: "timed_out" });
+    expect(second).toMatchObject({ blocked: true, disposition: "timed_out" });
+    const deprecationWarnings = agentToolsWarnSpy.mock.calls.filter(
+      ([message]) =>
+        typeof message === "string" &&
+        message.includes("deprecated-timeout-plugin") &&
+        message.includes("timeoutBehavior"),
+    );
+    expect(deprecationWarnings).toHaveLength(1);
+  });
+
+  it("blocks embedded allow decisions excluded by the request", async () => {
+    setEmbeddedMode(true);
+    const broker = new EmbeddedPluginApprovalBroker();
+    setEmbeddedPluginApprovalBroker(broker);
+    vi.spyOn(broker, "request").mockResolvedValue({
+      id: "plugin:unexpected-decision",
+      decision: PluginApprovalResolutions.ALLOW_ALWAYS,
+    });
+    const onResolution = vi.fn();
+    runBeforeToolCallMock.mockResolvedValue({
+      requireApproval: {
+        pluginId: "test-plugin",
+        title: "Restricted approval",
+        description: "Allow once only",
+        allowedDecisions: ["allow-once", "deny"],
+        onResolution,
+      },
+      params: { adjusted: true },
+    });
+
+    const result = await runBeforeToolCallHook({
+      toolName: "exec",
+      params: { command: "unsafe-command" },
+      toolCallId: "call-restricted-approval",
+      ctx: { agentId: "main", sessionKey: "agent:main:main" },
+    });
+
+    expect(result).toEqual({
+      blocked: true,
+      kind: "failure",
+      disposition: "timed_out",
+      deniedReason: "plugin-approval",
+      reason: "Approval timed out",
+      params: { command: "unsafe-command" },
+    });
+    expect(onResolution).toHaveBeenCalledWith(PluginApprovalResolutions.TIMEOUT);
+    expect(mockCallGatewayTool).not.toHaveBeenCalled();
+  });
+
  it("reports approval-required tools without opening an approval request", async () => {
    runBeforeToolCallMock.mockResolvedValue({
      requireApproval: {
@@ -40,6 +40,7 @@ import {
  describeNativePluginApprovalClientSetup,
  resolveApprovalInitiatingSurfaceState,
 } from "../infra/exec-approval-surface.js";
+import { resolveCanonicalPluginApprovalRequestAllowedDecisions } from "../infra/plugin-approval-canonical-decisions.js";
 import {
  DEFAULT_PLUGIN_APPROVAL_TIMEOUT_MS,
  MAX_PLUGIN_APPROVAL_TIMEOUT_MS,
@@ -828,6 +829,24 @@ function emitToolBlockedSecurityEvent(params: {
  });
 }

+// Once-per-plugin-per-process deprecation signal; the field is ignored at
+// runtime because unresolved approvals always fail closed on timeout.
+const warnedDeprecatedTimeoutBehaviorPluginIds = new Set<string>();
+
+function warnDeprecatedApprovalTimeoutBehavior(approval: PluginApprovalRequest): void {
+  if (approval.timeoutBehavior !== "allow") {
+    return;
+  }
+  const pluginId = approval.pluginId ?? "unknown-plugin";
+  if (warnedDeprecatedTimeoutBehaviorPluginIds.has(pluginId)) {
+    return;
+  }
+  warnedDeprecatedTimeoutBehaviorPluginIds.add(pluginId);
+  log.warn(
+    `plugin '${pluginId}' sets deprecated requireApproval.timeoutBehavior:"allow"; the field is ignored and approvals fail closed on timeout (see docs/plugins/plugin-permission-requests.md)`,
+  );
+}
+
 function notifyPluginApprovalResolution(
  approval: PluginApprovalRequest,
  resolution: PluginApprovalResolution,
@@ -845,6 +864,21 @@ function notifyPluginApprovalResolution(
  }
 }

+function resolvePermittedPluginApprovalResolution(
+  decision: unknown,
+  allowedDecisions: readonly string[],
+): PluginApprovalResolution {
+  if (
+    (decision === PluginApprovalResolutions.ALLOW_ONCE ||
+      decision === PluginApprovalResolutions.ALLOW_ALWAYS ||
+      decision === PluginApprovalResolutions.DENY) &&
+    allowedDecisions.includes(decision)
+  ) {
+    return decision;
+  }
+  return PluginApprovalResolutions.TIMEOUT;
+}
+
 function buildPluginApprovalFailureReason(params: {
  fallbackReason: string;
  ctx?: HookContext;
@@ -894,6 +928,7 @@ async function requestPluginToolApproval(params: {
  const approval = params.approval;
  const timeoutMs = resolvePluginToolApprovalTimeoutMs(approval);
  const gatewayTimeoutMs = resolvePluginToolApprovalGatewayTimeoutMs(timeoutMs);
+  const allowedDecisions = resolveCanonicalPluginApprovalRequestAllowedDecisions(approval);
  let gatewayApprovalPhase: "none" | "request" | "wait" = "none";
  try {
    const embeddedApprovalBroker = isEmbeddedMode() ? getEmbeddedPluginApprovalBroker() : null;
@@ -918,16 +953,11 @@ async function requestPluginToolApproval(params: {
        signal: params.signal,
      });
      const decision = result.decision;
-      const resolution: PluginApprovalResolution =
-        decision === PluginApprovalResolutions.ALLOW_ONCE ||
-        decision === PluginApprovalResolutions.ALLOW_ALWAYS ||
-        decision === PluginApprovalResolutions.DENY
-          ? decision
-          : PluginApprovalResolutions.TIMEOUT;
+      const resolution = resolvePermittedPluginApprovalResolution(decision, allowedDecisions);
      notifyPluginApprovalResolution(approval, resolution);
      if (
-        decision === PluginApprovalResolutions.ALLOW_ONCE ||
-        decision === PluginApprovalResolutions.ALLOW_ALWAYS
+        resolution === PluginApprovalResolutions.ALLOW_ONCE ||
+        resolution === PluginApprovalResolutions.ALLOW_ALWAYS
      ) {
        return {
          blocked: false,
@@ -935,7 +965,7 @@ async function requestPluginToolApproval(params: {
          approvalResolution: resolution,
        };
      }
-      if (decision === PluginApprovalResolutions.DENY) {
+      if (resolution === PluginApprovalResolutions.DENY) {
        return {
          blocked: true,
          kind: "failure",
@@ -945,13 +975,6 @@ async function requestPluginToolApproval(params: {
          params: params.baseParams,
        };
      }
-      if (approval.timeoutBehavior === "allow") {
-        return {
-          blocked: false,
-          params: mergeParamsWithApprovalOverrides(params.baseParams, params.overrideParams),
-          approvalResolution: resolution,
-        };
-      }
      // Veto carries the plugin-supplied reason; plain timeouts record a
      // timed_out failure disposition for the audit ledger.
      return approval.timeoutReason
@@ -976,7 +999,7 @@ async function requestPluginToolApproval(params: {
    const requestResult: {
      id?: string;
      status?: string;
-      decision?: string | null;
+      decision?: unknown;
      deliveryRoute?: string;
    } = await callGatewayTool(
      "plugin.approval.request",
@@ -1019,7 +1042,7 @@ async function requestPluginToolApproval(params: {
      };
    }
    const hasImmediateDecision = Object.hasOwn(requestResult ?? {}, "decision");
-    let decision: string | null | undefined;
+    let decision: unknown;
    if (hasImmediateDecision) {
      decision = requestResult?.decision;
      if (decision === null) {
@@ -1042,7 +1065,7 @@ async function requestPluginToolApproval(params: {
      gatewayApprovalPhase = "wait";
      const waitPromise: Promise<{
        id?: string;
-        decision?: string | null;
+        decision?: unknown;
      }> = callGatewayTool(
        "plugin.approval.waitDecision",
        // Buffer beyond the approval timeout so the gateway can clean up
@@ -1050,7 +1073,7 @@ async function requestPluginToolApproval(params: {
        { timeoutMs: gatewayTimeoutMs },
        { id },
      );
-      let waitResult: { id?: string; decision?: string | null } | undefined;
+      let waitResult: { id?: string; decision?: unknown } | undefined;
      if (params.signal) {
        let onAbort: (() => void) | undefined;
        const abortPromise = new Promise<never>((_, reject) => {
@@ -1071,18 +1094,15 @@ async function requestPluginToolApproval(params: {
      } else {
        waitResult = await waitPromise;
      }
-      decision = waitResult?.decision;
+      // Bind the verdict to the request that parked this call. A stale or
+      // misrouted reply must never release a different tool gate.
+      decision = waitResult?.id === id ? waitResult.decision : undefined;
    }
-    const resolution: PluginApprovalResolution =
-      decision === PluginApprovalResolutions.ALLOW_ONCE ||
-      decision === PluginApprovalResolutions.ALLOW_ALWAYS ||
-      decision === PluginApprovalResolutions.DENY
-        ? decision
-        : PluginApprovalResolutions.TIMEOUT;
+    const resolution = resolvePermittedPluginApprovalResolution(decision, allowedDecisions);
    notifyPluginApprovalResolution(approval, resolution);
    if (
-      decision === PluginApprovalResolutions.ALLOW_ONCE ||
-      decision === PluginApprovalResolutions.ALLOW_ALWAYS
+      resolution === PluginApprovalResolutions.ALLOW_ONCE ||
+      resolution === PluginApprovalResolutions.ALLOW_ALWAYS
    ) {
      return {
        blocked: false,
@@ -1090,7 +1110,7 @@ async function requestPluginToolApproval(params: {
        approvalResolution: resolution,
      };
    }
-    if (decision === PluginApprovalResolutions.DENY) {
+    if (resolution === PluginApprovalResolutions.DENY) {
      return {
        blocked: true,
        kind: "failure",
@@ -1100,14 +1120,6 @@ async function requestPluginToolApproval(params: {
        params: params.baseParams,
      };
    }
-    const timeoutBehavior = approval.timeoutBehavior ?? "deny";
-    if (timeoutBehavior === "allow") {
-      return {
-        blocked: false,
-        params: mergeParamsWithApprovalOverrides(params.baseParams, params.overrideParams),
-        approvalResolution: resolution,
-      };
-    }
    const fallbackTimeoutReason = approval.timeoutReason ?? "Approval timed out";
    const timeoutReason =
      requestResult?.deliveryRoute === "turn-source"
@@ -1201,6 +1213,7 @@ async function resolveBeforeToolCallApprovalOutcome(params: {
  if (!approval) {
    return undefined;
  }
+  warnDeprecatedApprovalTimeoutBehavior(approval);
  if (params.approvalMode === "defer") {
    return {
      blocked: false,
@@ -0,0 +1,86 @@
+// Covers gateway waitDecision id binding for native hook relay permission approvals.
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { callGatewayTool } from "../tools/gateway.js";
+import { invokeNativeHookRelay, registerNativeHookRelay, testing } from "./native-hook-relay.js";
+
+vi.mock("../tools/gateway.js", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("../tools/gateway.js")>()),
+  callGatewayTool: vi.fn(),
+}));
+
+const mockCallGatewayTool = vi.mocked(callGatewayTool);
+
+afterEach(() => {
+  // restoreAllMocks does not clear call history on module-mock vi.fn()s.
+  mockCallGatewayTool.mockReset();
+  vi.restoreAllMocks();
+  testing.clearNativeHookRelaysForTests();
+});
+
+function mockGatewayApproval(waitResult: { id?: string; decision?: string | null }) {
+  mockCallGatewayTool.mockImplementation(async (method: string) => {
+    if (method === "plugin.approval.request") {
+      return { id: "approval-1", status: "accepted" };
+    }
+    if (method === "plugin.approval.waitDecision") {
+      return waitResult;
+    }
+    throw new Error(`unexpected gateway method: ${method}`);
+  });
+}
+
+async function invokePermissionRequest(relayId: string) {
+  return invokeNativeHookRelay({
+    provider: "codex",
+    relayId,
+    event: "permission_request",
+    rawPayload: {
+      hook_event_name: "PermissionRequest",
+      cwd: "/repo",
+      tool_name: "Bash",
+      tool_use_id: "native-binding-call-1",
+      tool_input: { command: "printf binding" },
+    },
+  });
+}
+
+describe("native hook relay approval id binding", () => {
+  it("accepts a waitDecision reply bound to the requested approval id", async () => {
+    mockGatewayApproval({ id: "approval-1", decision: "allow-once" });
+    const relay = registerNativeHookRelay({
+      provider: "codex",
+      relayId: "codex-approval-binding-match",
+      sessionId: "session-1",
+      runId: "run-1",
+    });
+
+    const response = await invokePermissionRequest(relay.relayId);
+
+    expect(JSON.parse(response.stdout)).toEqual({
+      hookSpecificOutput: {
+        hookEventName: "PermissionRequest",
+        decision: { behavior: "allow" },
+      },
+    });
+  });
+
+  it("defers when a waitDecision reply carries a different approval id", async () => {
+    mockGatewayApproval({ id: "approval-other", decision: "allow-once" });
+    const relay = registerNativeHookRelay({
+      provider: "codex",
+      relayId: "codex-approval-binding-mismatch",
+      sessionId: "session-1",
+      runId: "run-1",
+    });
+
+    const response = await invokePermissionRequest(relay.relayId);
+
+    // A misrouted reply must never release the gate; the relay falls back to
+    // the provider's own approval path via the noop response.
+    expect(response).toEqual({ stdout: "", stderr: "", exitCode: 0 });
+    expect(mockCallGatewayTool.mock.calls.map(([method]) => method)).toEqual([
+      "plugin.approval.request",
+      "plugin.approval.waitDecision",
+    ]);
+  });
+});
@@ -2132,7 +2132,9 @@ async function requestNativeHookRelayPermissionApproval(
      signal: request.signal,
      timeoutMs,
    });
-    decision = waitResult?.decision;
+    // Bind the verdict to the request that parked this call. A stale or
+    // misrouted reply must never release a different tool gate.
+    decision = waitResult?.id === approvalId ? waitResult.decision : undefined;
  }
  if (decision === PluginApprovalResolutions.ALLOW_ONCE) {
    return "allow";
@@ -217,6 +217,8 @@ async function expectApprovalResolution(
    ok: true,
    payload: { id: record.id, decision: "allow-once" },
  });
+  expect(manager.getSnapshot(record.id)?.consumedDecision).toBe("allow-once");
+  expect(manager.consumeAllowOnce(record.id)).toBe(false);
 }

 describe("applyPluginNodeInvokePolicy", () => {
@@ -526,6 +528,31 @@ describe("applyPluginNodeInvokePolicy", () => {
    await expectApprovalResolution(resultPromise, manager, record);
  });

+  it("fails closed when the allow-once claim cannot be consumed", async () => {
+    const manager = new ExecApprovalManager<PluginApprovalRequestPayload>();
+    vi.spyOn(manager, "consumeAllowOnce").mockReturnValue(false);
+    setDangerousDemoCommandRegistry([createApprovalRequestPolicy()]);
+    const { context } = createContext({
+      pluginApprovalManager: manager,
+      getApprovalClientConnIds: createApprovalClientLookup([
+        createApprovalClient({
+          connId: "conn-owner-approval",
+          clientId: "client-owner",
+          deviceId: "device-owner",
+        }),
+      ]),
+    });
+    const resultPromise = invokeDemoPolicy(context, createOperatorClient());
+
+    const record = await expectSinglePendingApproval(manager);
+    expect(manager.resolve(record.id, "allow-once")).toBe(true);
+
+    await expect(resultPromise).resolves.toStrictEqual({
+      ok: true,
+      payload: { id: record.id, decision: null },
+    });
+  });
+
  it("fails closed before routing an unrenderable persistent policy approval", async () => {
    const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-node-policy-approval-"));
    tempDirs.push(stateDir);
@@ -145,7 +145,16 @@ function createApprovalRuntime(params: {
          });
        },
      });
-      return { id: record.id, decision: await decisionPromise };
+      const decision = await decisionPromise;
+      // This return hands execution authority to the plugin policy. Claim a
+      // one-shot decision here so observation or retry cannot replay it.
+      if (
+        decision === "allow-once" &&
+        !manager.consumeAllowOnce(record.id, `plugin.node.invoke:${record.id}`)
+      ) {
+        return { id: record.id, decision: null };
+      }
+      return { id: record.id, decision };
    },
  };
 }
@@ -18,6 +18,10 @@ export type PluginHookBeforeToolCallResult = {
    description: string;
    severity?: "info" | "warning" | "critical";
    timeoutMs?: number;
+    /**
+     * @deprecated Unresolved approvals always deny; retained for plugin API
+     * compatibility. The field will be removed after one deprecation release train.
+     */
    timeoutBehavior?: "allow" | "deny";
    /** Override timeout text and return the timeout as a blocked tool result. */
    timeoutReason?: string;