From 20ad68744f4068cd6e84969e8fd64b40a7c3ffc2 Mon Sep 17 00:00:00 2001 From: Rolando Bosch Date: Mon, 7 Sep 2026 07:36:59 -0400 Subject: [PATCH] fix(permissions): merge an explicit policy over the secure baseline `AgentOs.create` only applied the baseline policy when `permissions` was absent entirely, so any explicit object replaced it wholesale. The sidecar denies every scope the wire policy omits, so `{ network: "allow" }` denied fs/childProcess/process/env, and registering bindings alongside any explicit policy lost the `binding` auto-grant. Overlay the caller's scopes on the baseline instead, which is what the permissions docs promise and what the Rust client already does. --- packages/core/package.json | 2 +- packages/core/src/agent-os.ts | 24 ++++- .../core/tests/binding-permissions.test.ts | 67 ++++++++++++- .../core/tests/host-permission-merge.test.ts | 96 +++++++++++++++++++ 4 files changed, 183 insertions(+), 6 deletions(-) create mode 100644 packages/core/tests/host-permission-merge.test.ts diff --git a/packages/core/package.json b/packages/core/package.json index f000012970..8b65c7af96 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -56,7 +56,7 @@ "build:agentos-protocol": "node ./scripts/compile-agentos-protocol.mjs", "build:protocols": "pnpm run build:agentos-protocol", "test": "vitest run --exclude '**/*.nightly.test.ts' --reporter=verbose", - "test:unit": "vitest run tests/agent-exit-event.test.ts tests/agentos-package.test.ts tests/agentos-protocol.test.ts tests/allowed-node-builtins.test.ts tests/bindings-zod.test.ts tests/bindings.test.ts tests/cron-manager.test.ts tests/cron-timer-driver.test.ts tests/execution-abort.test.ts tests/generated-protocol.test.ts tests/leak-agent-os-processes.test.ts tests/leak-rpc-client.test.ts tests/mount-descriptors.test.ts tests/mount-reconfigure.test.ts tests/options-schema.test.ts tests/public-api-exports.test.ts tests/root-filesystem-descriptors.test.ts tests/runtime-compat-mount.test.ts tests/session-event-ordering.test.ts tests/session-permission-surface.test.ts tests/sidecar-client.test.ts tests/sidecar-permission-descriptors.test.ts tests/wasm-permission-tiers.test.ts --fileParallelism=false", + "test:unit": "vitest run tests/agent-exit-event.test.ts tests/agentos-package.test.ts tests/agentos-protocol.test.ts tests/allowed-node-builtins.test.ts tests/bindings-zod.test.ts tests/bindings.test.ts tests/cron-manager.test.ts tests/cron-timer-driver.test.ts tests/execution-abort.test.ts tests/generated-protocol.test.ts tests/host-permission-merge.test.ts tests/leak-agent-os-processes.test.ts tests/leak-rpc-client.test.ts tests/mount-descriptors.test.ts tests/mount-reconfigure.test.ts tests/options-schema.test.ts tests/public-api-exports.test.ts tests/root-filesystem-descriptors.test.ts tests/runtime-compat-mount.test.ts tests/session-event-ordering.test.ts tests/session-permission-surface.test.ts tests/sidecar-client.test.ts tests/sidecar-permission-descriptors.test.ts tests/wasm-permission-tiers.test.ts --fileParallelism=false", "test:pr": "pnpm test:unit && vitest run tests/migration-parity.test.ts tests/acp-reactor-regression.test.ts --fileParallelism=false --reporter=verbose", "test:nightly": "vitest run tests/*.nightly.test.ts --reporter=verbose --passWithNoTests" }, diff --git a/packages/core/src/agent-os.ts b/packages/core/src/agent-os.ts index 6e786ac748..5154c19d09 100644 --- a/packages/core/src/agent-os.ts +++ b/packages/core/src/agent-os.ts @@ -2549,6 +2549,25 @@ function describeBindingPayload( }; } +/** + * Overlay an explicit permission policy on the secure baseline, scope by scope. + * + * The sidecar denies every scope the wire policy omits, so a partial policy + * must not replace the baseline wholesale: `{ network: "allow" }` has to keep + * the execution essentials and the `binding` auto-grant that a VM with no + * policy gets. This matches the documented merge semantics in + * `docs/content/docs/permissions.mdx` and the Rust client, which already fills + * each omitted scope from its own baseline (`permissions_policy` in + * `crates/client/src/agent_os.rs`). + */ +export function resolveHostPermissions(permissions?: Permissions): Permissions { + return { + ...allowAll, + binding: "allow", + ...permissions, + }; +} + function bindingPermissionMode( permissions: Permissions, callbackKey: string, @@ -3273,10 +3292,7 @@ export class AgentOs { client = shared.client; const session = shared.session; nativeSession = session; - const hostPermissions = options?.permissions ?? { - ...allowAll, - binding: "allow", - }; + const hostPermissions = resolveHostPermissions(options?.permissions); const sidecarPermissions = serializePermissionsForSidecar(hostPermissions); const createVmConfig: CreateVmConfig = { diff --git a/packages/core/tests/binding-permissions.test.ts b/packages/core/tests/binding-permissions.test.ts index 617e33fe90..599dfa9e07 100644 --- a/packages/core/tests/binding-permissions.test.ts +++ b/packages/core/tests/binding-permissions.test.ts @@ -178,7 +178,9 @@ describe("binding collection permissions", () => { }); }); - test("denies binding collection invocation by default until binding permissions are granted", async () => { + // The binding auto-grant is part of the baseline an explicit policy is + // merged over, so passing an unrelated scope must not revoke it. + test("keeps the binding auto-grant when an explicit policy sets no binding scope", async () => { vm = await AgentOs.create({ software: [common], bindings: [mathBindings], @@ -188,6 +190,31 @@ describe("binding collection permissions", () => { }, }); + const result = await runCommand(vm, "agentos-math", [ + "add", + "--a", + "5", + "--b", + "7", + ]); + expect(result.exitCode).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + ok: true, + result: { sum: 12 }, + }); + }); + + test("denies binding collection invocation when the policy denies binding", async () => { + vm = await AgentOs.create({ + software: [common], + bindings: [mathBindings], + permissions: { + fs: "allow", + childProcess: "allow", + binding: "deny", + }, + }); + const result = await runCommand(vm, "agentos-math", [ "add", "--a", @@ -244,6 +271,44 @@ describe("binding collection permissions — raw host_callback RPC path", () => vm = null; }); + // Same auto-grant, over the guest-controlled RPC path. + test("host_callback RPC keeps the binding auto-grant when an explicit policy sets no binding scope", async () => { + const executed: unknown[] = []; + const spyBindings = bindings({ + name: "math", + description: "Math utilities", + bindings: { + add: binding({ + description: "Add two numbers", + inputSchema: z.object({ a: z.number(), b: z.number() }), + execute: ({ a, b }) => { + executed.push({ a, b }); + return { sum: a + b }; + }, + }), + }, + }); + + const created = await createVmCapturingHandler({ + bindings: [spyBindings], + // Names only fs/childProcess: `binding` keeps its baseline grant. + permissions: { + fs: "allow", + childProcess: "allow", + }, + }); + vm = created.vm; + + const response = await created.handler( + hostCallbackFrame("math:add", { a: 2, b: 3 }), + ); + + expect(executed).toEqual([{ a: 2, b: 3 }]); + expect(response.type).toBe("host_callback_result"); + expect(response.error).toBeUndefined(); + expect(response.result).toEqual({ sum: 5 }); + }); + // N-001 (J.1/J.2): host_callback RPC must honor binding.invoke deny. test("denies host_callback RPC binding invocation when binding.invoke policy is deny (not just the CLI path)", async () => { const executed: unknown[] = []; diff --git a/packages/core/tests/host-permission-merge.test.ts b/packages/core/tests/host-permission-merge.test.ts new file mode 100644 index 0000000000..bd2b0ee4d0 --- /dev/null +++ b/packages/core/tests/host-permission-merge.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, test } from "vitest"; +import { resolveHostPermissions } from "../src/agent-os.js"; +import type { Permissions } from "../src/runtime-compat.js"; +import { serializePermissionsForSidecar } from "../src/sidecar/permissions.js"; + +// The sidecar denies every scope the wire policy omits, so anything +// `resolveHostPermissions` leaves out is a silent over-denial rather than the +// documented baseline. See docs/content/docs/permissions.mdx. +describe("resolveHostPermissions", () => { + test("keeps the baseline for a VM created without a policy", () => { + expect(resolveHostPermissions(undefined)).toEqual({ + fs: "allow", + network: "allow", + childProcess: "allow", + process: "allow", + env: "allow", + binding: "allow", + }); + }); + + test("merges a partial policy over the baseline instead of replacing it", () => { + expect(resolveHostPermissions({ network: "allow" })).toEqual({ + fs: "allow", + network: "allow", + childProcess: "allow", + process: "allow", + env: "allow", + binding: "allow", + }); + }); + + test("keeps the execution essentials when only a network rule set is given", () => { + const network: Permissions["network"] = { + default: "deny", + rules: [ + { mode: "allow", operations: ["fetch"], patterns: ["example.com"] }, + ], + }; + + expect(resolveHostPermissions({ network })).toEqual({ + fs: "allow", + network, + childProcess: "allow", + process: "allow", + env: "allow", + binding: "allow", + }); + }); + + test("keeps the binding auto-grant alongside an explicit policy", () => { + expect( + resolveHostPermissions({ fs: "allow", childProcess: "allow" }).binding, + ).toBe("allow"); + }); + + test("lets an explicit scope win over the baseline", () => { + expect(resolveHostPermissions({ fs: "deny", network: "deny" })).toEqual({ + fs: "deny", + network: "deny", + childProcess: "allow", + process: "allow", + env: "allow", + binding: "allow", + }); + }); + + test("preserves an explicit binding deny", () => { + expect(resolveHostPermissions({ binding: "deny" }).binding).toBe("deny"); + }); + + test("preserves an explicit binding rule set", () => { + const binding: Permissions["binding"] = { + default: "deny", + rules: [ + { mode: "allow", operations: ["invoke"], patterns: ["math:add"] }, + ], + }; + + expect(resolveHostPermissions({ binding }).binding).toEqual(binding); + }); + + test("serializes every scope so the sidecar never sees an absent scope", () => { + const serialized = serializePermissionsForSidecar( + resolveHostPermissions({ network: "allow" }), + ); + + expect(serialized).toEqual({ + fs: "allow", + network: "allow", + childProcess: "allow", + process: "allow", + env: "allow", + binding: "allow", + }); + }); +});