[AAASM-6352] 🐛 (native): Deny redact verdicts under enforce instead of running the tool

docs/01-introduction/index.md

@@ -70,7 +70,7 @@ human, the call waits for an approval decision.
     own `gatewayClient`, routes checks to the runtime; `auto` / `sdk-only` /
     `grpc-sidecar` route through the allow-all no-op client, where a DENY does not block.
   - **Redaction** — applied by the runtime/proxy, not here. Under `enforce` this layer
-    treats a `redact` verdict as allow (see [Configuration](../05-configuration/index.md)).
+    denies on a `redact` verdict because it cannot apply the redaction itself (see [Configuration](../05-configuration/index.md)).
   - **An audit trail** — not on the default path: the no-op client `auto` resolves
     holds no event channel and drops hook-layer audit events. `napi-inprocess` hands
     them to the runtime instead, which is a handoff and not a retention guarantee

docs/05-configuration/index.md

@@ -56,7 +56,7 @@ Both `gatewayUrl` and `apiKey` are resolved with the same four-step precedence
 | Priority | Source                            | URL key                                                  | API-key key                                      |
 | -------- | --------------------------------- | -------------------------------------------------------- | ------------------------------------------------ |
 | 1        | Explicit field on `config`        | `gatewayUrl`                                             | `apiKey`                                         |
-| 2        | Environment variable              | `AA_GATEWAY_URL`                                      | `AA_API_KEY`                                  |
+| 2        | Environment variable              | `AA_GATEWAY_URL`                                         | `AA_API_KEY`                                     |
 | 3        | Config file `~/.aasm/config.yaml` | `agent.gateway_url`                                      | `agent.api_key`                                  |
 | 4        | Local default                     | `http://localhost:7391` (probed; auto-started if absent) | `""` (local mode accepts unauthenticated agents) |
 
@@ -125,15 +125,16 @@ preserving the pre-feature wire shape.
 
 | Value        | Behavior                                                                                                                  |
 | ------------ | ------------------------------------------------------------------------------------------------------------------------- |
-| `"enforce"`  | Default. Deny blocks the action; redact strips secrets.                                                                   |
+| `"enforce"`  | Default. Deny blocks the action; redact also blocks (see below).                                                          |
 | `"observe"`  | Dry-run. Every action proceeds; would-be violations are recorded as shadow audit events (`aa audit list --dry-run-only`). |
 | `"disabled"` | Policy evaluation skipped entirely. Hermetic tests only.                                                                  |
 
-> **Redaction is applied by the runtime/proxy, not the in-process SDK.** Under `"enforce"`
-> a `redact` verdict is treated as allow by this SDK layer — the event is emitted, but no
-> client-side redaction happens here. Secret-stripping is performed by the authoritative
-> `aa-runtime` / `aa-proxy` layers, which see the full tool I/O this advisory SDK seam does
-> not. (Parallels the Go SDK, AAASM-4788.)
+> **This SDK cannot apply redaction.** Under `"enforce"` a `redact` verdict is treated as a
+> deny: no redacted payload crosses the native ABI (AAASM-1920), so running the tool would
+> hand it the original, secret-bearing arguments. The denial reason is "gateway required
+> redaction the SDK cannot apply client-side" (AAASM-6352). Under `"observe"` / `"disabled"`
+> the tool still runs unmodified. Secret-stripping itself is performed by the authoritative
+> `aa-runtime` / `aa-proxy` layers. The Go SDK likewise errors on Redact.
 
 Unknown values throw a `RangeError` so a typo can never silently downgrade an agent into
 live enforcement when the operator meant `observe`.

src/gateway/client.ts

@@ -152,7 +152,10 @@ export function resolveVerdict(
  * native `queryPolicy` primitive and maps it onto a `GatewayDecision`:
  *   - `deny`    → `{ denied: true }`  (the wrapper throws `PolicyViolationError`)
  *   - `pending` → `{ pending: true }` (routes to the approval path)
- *   - allow / redact / unspecified → `{ denied: false }`
+ *   - allow → `{ denied: false }`
+ *   - redact → `{ denied: true }` under enforce (the SDK cannot apply the
+ *     redaction client-side, AAASM-6352); `{ denied: false }` otherwise
+ *   - unspecified → `{ denied: true }` under enforce, else `{ denied: false }`
  *
  * **Enforcement posture (AAASM-3996 — py/go parity, AAASM-3920):** under
  * `enforce` the SDK is a fail-closed control: a caught local fault or an

src/native/client.ts

@@ -142,11 +142,19 @@ export interface NativeClient {
  */
 export const FAIL_OPEN_REASON = "aa-runtime unreachable or slow; failing open (advisory SDK)";
 
+/**
+ * Deny reason used under enforce when the gateway answers `redact`: the SDK has
+ * no way to apply a redaction to the tool args (AAASM-1920), so it must not run
+ * the tool with the unredacted payload.
+ */
+export const REDACT_UNHONOURABLE_REASON =
+  "gateway required redaction the SDK cannot apply client-side; tracked in AAASM-1920/AAASM-6352";
+
 /**
  * Translate the native `{decision, reason}` verdict into the SDK's
- * `PolicyResult`. Only `"deny"` blocks; `"pending"` routes to the approval
- * path; `"allow"` / `"redact"` proceed. This mirrors the shared enforcement
- * contract across the Python / Go / Node SDKs.
+ * `PolicyResult`. `"deny"` blocks; `"pending"` routes to the approval path;
+ * `"allow"` proceeds; `"redact"` denies under `failClosed` (enforce) because the
+ * SDK cannot apply redaction client-side (AAASM-6352) and proceeds otherwise.
  *
  * **Fail-closed (AAASM-4013):** the native shim folds an unreachable / slow /
  * closed runtime onto `"allow"` + {@link FAIL_OPEN_REASON}, and an
@@ -173,14 +181,15 @@ function mapDecisionToPolicyResult(
       }
       return { denied: false, pending: false };
     case "redact":
-      // Deliberately folded onto allow, NOT silently dropped. The in-process
-      // SDK does not honor `redact` client-side: redaction is a response-body
-      // transform owned by the authoritative runtime / proxy layer (aa-runtime,
-      // aa-proxy), which sees the full tool I/O this advisory SDK seam does not.
-      // No client-side wrapper acts on a redact verdict, so surfacing it as a
-      // distinct SDK state would be a no-op that misleads callers into thinking
-      // in-process redaction happened. This mirrors the Go SDK's redact handling
-      // (AAASM-4788) and keeps the SDK layer's non-authoritative posture honest.
+      // The gateway required the payload to be redacted before the tool runs,
+      // but no redacted payload crosses the native ABI (AAASM-1920), so this SDK
+      // cannot apply the redaction client-side. Running the tool with the
+      // ORIGINAL args would leak exactly what the verdict asked to strip, so
+      // under enforce this denies (AAASM-6352), as the Go SDK does by erroring
+      // on Redact. Observe / disabled stay advisory and proceed.
+      if (failClosed) {
+        return { denied: true, pending: false, reason: REDACT_UNHONOURABLE_REASON };
+      }
       return { denied: false, pending: false };
     default:
       // Empty / unrecognized decision: the runtime produced no authoritative

tests/native-client.test.ts

@@ -131,9 +131,7 @@ describe("createNativeClient", () => {
 
     expect(client.mode).toBe("grpc-sidecar");
     expect(client.canRegister).toBe(false);
-    await expect(client.register({ agentId: "a", name: "a", framework: "none" })).resolves.toBe(
-      ""
-    );
+    await expect(client.register({ agentId: "a", name: "a", framework: "none" })).resolves.toBe("");
     await expect(client.queryPolicy({ action: "check" })).resolves.toEqual({
       denied: false,
       pending: false
@@ -319,11 +317,11 @@ describe("createNativeClient", () => {
       await client.close();
     });
 
-    it("napi-inprocess + failClosed: a REDACT verdict is authoritative and proceeds", async () => {
-      // `redact` is a real runtime verdict (the tool runs, with output
-      // redaction applied downstream), NOT a "no verdict" sentinel — so it must
-      // proceed even under enforce. Only the fail-open sentinel / unknown
-      // decision deny under failClosed.
+    it("napi-inprocess + failClosed: a REDACT verdict denies (SDK cannot apply redaction)", async () => {
+      // AAASM-6352: previously pinned as redact -> allow, which let the tool run
+      // with the ORIGINAL (secret-bearing) args. No redacted payload crosses the
+      // native ABI (AAASM-1920), so under enforce the verdict cannot be honoured
+      // and must deny.
       const mod = await loadNativeClientWithBinding(() => ({
         connect: vi.fn(async () => ({ id: "handle-redact" })),
         sendEvent: vi.fn(() => undefined),
@@ -338,6 +336,28 @@ describe("createNativeClient", () => {
         failClosed: true
       });
 
+      await expect(client.queryPolicy({ action_type: "tool_call" })).resolves.toEqual({
+        denied: true,
+        pending: false,
+        reason: mod.REDACT_UNHONOURABLE_REASON
+      });
+      await client.close();
+    });
+
+    it("napi-inprocess without failClosed: a REDACT verdict proceeds (observe/disabled stay advisory)", async () => {
+      const mod = await loadNativeClientWithBinding(() => ({
+        connect: vi.fn(async () => ({ id: "handle-redact-observe" })),
+        sendEvent: vi.fn(() => undefined),
+        queryPolicy: vi.fn(() => ({ decision: "redact", reason: "secrets stripped" })),
+        disconnect: vi.fn(async () => undefined)
+      }));
+
+      const client = mod.createNativeClient({
+        gateway: "/tmp/aa.sock",
+        apiKey: "test-key",
+        mode: "napi-inprocess"
+      });
+
       await expect(client.queryPolicy({ action_type: "tool_call" })).resolves.toEqual({
         denied: false,
         pending: false

tests/native-napi-integration.test.ts

@@ -4,7 +4,11 @@ import { tmpdir } from "node:os";
 import { join, resolve } from "node:path";
 import blockedAt from "blocked-at";
 import { afterEach, beforeEach, describe, expect, it } from "vitest";
-import { NativeConnectError, createNativeClient } from "../src/native/client.js";
+import {
+  NativeConnectError,
+  REDACT_UNHONOURABLE_REASON,
+  createNativeClient
+} from "../src/native/client.js";
 
 const NATIVE_ADDON_DIR = resolve(process.cwd(), "native/aa-ffi-node");
 const RUN_NATIVE_TESTS =
@@ -60,6 +64,10 @@ const POLICY_RESPONSE_ALLOW_FRAME = Buffer.from([
   0x01
 ]);
 
+// A `CheckActionResponse { decision: REDACT }` — proto field 1 (varint) = 4
+// (common.proto `Decision.REDACT`). Same framing as the allow frame above.
+const POLICY_RESPONSE_REDACT_FRAME = Buffer.from([TAG_POLICY_RESPONSE, 0x02, 0x08, 0x04]);
+
 /**
  * Read a prost varint starting at `start`. Returns the decoded value and the
  * number of bytes consumed, or null if the buffer does not yet hold a complete
@@ -99,7 +107,10 @@ interface MockRuntime {
  * it receives. It performs no scanning, redaction, or proof verification; that
  * is the authoritative runtime's job, not the SDK's.
  */
-function startMockRuntime(socketPath: string): Promise<MockRuntime> {
+function startMockRuntime(
+  socketPath: string,
+  policyResponseFrame: Buffer = POLICY_RESPONSE_ALLOW_FRAME
+): Promise<MockRuntime> {
   let events = 0;
   const server = net.createServer((sock) => {
     let buf = Buffer.alloc(0);
@@ -138,7 +149,7 @@ function startMockRuntime(socketPath: string): Promise<MockRuntime> {
             sock.write(ACK_FRAME);
           } else {
             // A policy-query is answered with a PolicyResponse, not an Ack.
-            sock.write(POLICY_RESPONSE_ALLOW_FRAME);
+            sock.write(policyResponseFrame);
           }
           continue;
         }
@@ -231,4 +242,62 @@ describeNative("native napi integration", () => {
 
     await client.close();
   });
+  // AAASM-6352: the unit tests inject a fake binding that returns
+  // `decision: "redact"`. These drive the REAL napi addon, so they prove the
+  // codec actually surfaces a runtime REDACT verdict as "redact" and that the
+  // SDK then refuses to run the tool under enforce. The ALLOW case is the paired
+  // control: same socket, same call, only the runtime's verdict differs.
+  it("real addon: a runtime REDACT verdict denies the tool under failClosed", async () => {
+    const socketPath = join(socketDir, "aa-runtime-redact.sock");
+    runtime = await startMockRuntime(socketPath, POLICY_RESPONSE_REDACT_FRAME);
+
+    const client = createNativeClient({
+      gateway: socketPath,
+      apiKey: "test-key",
+      mode: "napi-inprocess",
+      failClosed: true
+    });
+
+    await expect(client.queryPolicy({ action_type: "tool_call", tool: "search" })).resolves.toEqual({
+      denied: true,
+      pending: false,
+      reason: REDACT_UNHONOURABLE_REASON
+    });
+    await client.close();
+  });
+
+  it("real addon: the same query is allowed when the runtime answers ALLOW (control)", async () => {
+    const socketPath = join(socketDir, "aa-runtime-allow.sock");
+    runtime = await startMockRuntime(socketPath);
+
+    const client = createNativeClient({
+      gateway: socketPath,
+      apiKey: "test-key",
+      mode: "napi-inprocess",
+      failClosed: true
+    });
+
+    await expect(client.queryPolicy({ action_type: "tool_call", tool: "search" })).resolves.toEqual({
+      denied: false,
+      pending: false
+    });
+    await client.close();
+  });
+
+  it("real addon: a runtime REDACT verdict stays advisory without failClosed", async () => {
+    const socketPath = join(socketDir, "aa-runtime-redact-observe.sock");
+    runtime = await startMockRuntime(socketPath, POLICY_RESPONSE_REDACT_FRAME);
+
+    const client = createNativeClient({
+      gateway: socketPath,
+      apiKey: "test-key",
+      mode: "napi-inprocess"
+    });
+
+    await expect(client.queryPolicy({ action_type: "tool_call", tool: "search" })).resolves.toEqual({
+      denied: false,
+      pending: false
+    });
+    await client.close();
+  });
 });

tests/redact-enforce-tool-wrapper.test.ts

@@ -0,0 +1,72 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import type { LangChainToolLike } from "../src/types/langchain-adapter.js";
+
+// AAASM-6352: end-to-end through the real native-client verdict mapping, the
+// native gateway client and wrapToolWithAssembly, with only the napi binding
+// faked. Asserts on whether the tool function ran and with which args — not on
+// the mapped verdict shape.
+async function wrapWithVerdict(decision: string, failClosed: boolean) {
+  vi.resetModules();
+  vi.doMock("node:module", () => ({
+    createRequire: () => () => ({
+      connect: vi.fn(async () => ({ id: "h" })),
+      sendEvent: vi.fn(() => undefined),
+      queryPolicy: vi.fn(() => ({ decision, reason: "policy" })),
+      disconnect: vi.fn(async () => undefined)
+    })
+  }));
+  const { createNativeClient } = await import("../src/native/client.js");
+  const { createNativeGatewayClient } = await import("../src/gateway/client.js");
+  // Re-imported after resetModules so the class identity matches the wrapper's.
+  const { PolicyViolationError } = await import("../src/errors/index.js");
+  const { wrapToolWithAssembly } = await import("../src/adapters/langchain/index.js");
+
+  const nativeClient = createNativeClient({
+    gateway: "/tmp/aa.sock",
+    apiKey: "k",
+    mode: "napi-inprocess",
+    failClosed
+  });
+  const gateway = createNativeGatewayClient(
+    "napi-inprocess",
+    nativeClient,
+    "agent-1",
+    undefined,
+    failClosed ? "enforce" : "observe"
+  );
+  const invoke = vi.fn<(args: unknown) => Promise<string>>(async () => "tool-result");
+  const tool: LangChainToolLike = { name: "send_email", invoke };
+  wrapToolWithAssembly(tool, gateway, { generateRunId: () => "run-1" });
+  return { tool, invoke, PolicyViolationError };
+}
+
+const SECRET_ARGS = { body: "password=hunter2" };
+
+afterEach(() => {
+  vi.doUnmock("node:module");
+});
+
+describe("wrapped tool under a gateway REDACT verdict (AAASM-6352)", () => {
+  it("enforce: does NOT run the tool and denies with the redaction reason", async () => {
+    const { tool, invoke, PolicyViolationError } = await wrapWithVerdict("redact", true);
+
+    await expect(tool.invoke(SECRET_ARGS)).rejects.toBeInstanceOf(PolicyViolationError);
+    await expect(tool.invoke(SECRET_ARGS)).rejects.toThrow(/cannot apply client-side/);
+    expect(invoke).not.toHaveBeenCalled();
+  });
+
+  it("enforce control: an ALLOW verdict runs the tool with its original args", async () => {
+    const { tool, invoke } = await wrapWithVerdict("allow", true);
+
+    await expect(tool.invoke(SECRET_ARGS)).resolves.toBe("tool-result");
+    expect(invoke).toHaveBeenCalledTimes(1);
+    expect(invoke.mock.calls[0]?.[0]).toEqual(SECRET_ARGS);
+  });
+
+  it("observe: REDACT keeps advisory semantics and runs the tool", async () => {
+    const { tool, invoke } = await wrapWithVerdict("redact", false);
+
+    await expect(tool.invoke(SECRET_ARGS)).resolves.toBe("tool-result");
+    expect(invoke.mock.calls[0]?.[0]).toEqual(SECRET_ARGS);
+  });
+});