diff --git a/docs/01-introduction/index.md b/docs/01-introduction/index.md index 7cd9d6483..d6af38d2c 100644 --- a/docs/01-introduction/index.md +++ b/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 diff --git a/docs/05-configuration/index.md b/docs/05-configuration/index.md index b62025ce2..f6202d697 100644 --- a/docs/05-configuration/index.md +++ b/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`. diff --git a/src/gateway/client.ts b/src/gateway/client.ts index 70502746c..e86745fc1 100644 --- a/src/gateway/client.ts +++ b/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 diff --git a/src/native/client.ts b/src/native/client.ts index cab2b0288..f58e1492e 100644 --- a/src/native/client.ts +++ b/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 diff --git a/tests/native-client.test.ts b/tests/native-client.test.ts index f4b1ea318..8c1a8978f 100644 --- a/tests/native-client.test.ts +++ b/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 diff --git a/tests/native-napi-integration.test.ts b/tests/native-napi-integration.test.ts index 7e2b76e77..1e4a0d094 100644 --- a/tests/native-napi-integration.test.ts +++ b/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 { +function startMockRuntime( + socketPath: string, + policyResponseFrame: Buffer = POLICY_RESPONSE_ALLOW_FRAME +): Promise { let events = 0; const server = net.createServer((sock) => { let buf = Buffer.alloc(0); @@ -138,7 +149,7 @@ function startMockRuntime(socketPath: string): Promise { 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(); + }); }); diff --git a/tests/redact-enforce-tool-wrapper.test.ts b/tests/redact-enforce-tool-wrapper.test.ts new file mode 100644 index 000000000..ae00fd669 --- /dev/null +++ b/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>(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); + }); +});