diff --git a/CHANGELOG.md b/CHANGELOG.md index dc8e31e..4842273 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## Unreleased + +- **`checkPolicy` allowlists now fail closed.** When a policy set `allowedCodes` + or `allowedCustomers`, a request that omitted `metadata.code` / + `metadata.customer` (or sent an empty string) skipped the allowlist and was + approved if the amount and budget checks passed. Such a request is now denied + with `CODE_NOT_ALLOWED` / `CUSTOMER_NOT_ALLOWED`. **Behavior change** for + callers that set an allowlist but do not send the field: send it, or drop the + allowlist. Policies without allowlists are unaffected. Reported in + [#8](https://github.com/goodmeta/agent-verifier/issues/8). + ## 0.6.1 - **`requiredConstraints` now reaches `verifyPaymentChain`.** 0.6.0 added it to diff --git a/README.md b/README.md index b907179..d186547 100644 --- a/README.md +++ b/README.md @@ -66,10 +66,19 @@ const policy: SpendingPolicy = { checkPolicy(policy, { agentId: "billing-agent", amount: 4_500, + metadata: { code: "api_calls" }, idempotencyKey: "tx-1", }) // → { approved: true, remaining: { budget: 15500, period: "monthly" } } +// ❌ no code, but the policy only allows listed codes (allowlists fail closed) +checkPolicy(policy, { + agentId: "billing-agent", + amount: 4_500, + idempotencyKey: "tx-3", +}) +// → { approved: false, reason: "CODE_NOT_ALLOWED", detail: "No code given; this policy only allows listed codes" } + // ❌ over per-event limit checkPolicy(policy, { agentId: "billing-agent", diff --git a/src/policy.ts b/src/policy.ts index a1ace7f..e6b3246 100644 --- a/src/policy.ts +++ b/src/policy.ts @@ -88,8 +88,16 @@ export function checkPolicy( }; } - // Code allowlist - if (metadata?.code && constraints.allowedCodes?.length) { + // Code allowlist. Fails closed: when the policy lists allowed codes, a request + // that omits `metadata.code` (or sends "") is denied, not waved through. + if (constraints.allowedCodes?.length) { + if (!metadata?.code) { + return { + approved: false, + reason: "CODE_NOT_ALLOWED", + detail: "No code given; this policy only allows listed codes", + }; + } if (!constraints.allowedCodes.includes(metadata.code)) { return { approved: false, @@ -108,8 +116,15 @@ export function checkPolicy( }; } - // Customer allowlist - if (metadata?.customer && constraints.allowedCustomers?.length) { + // Customer allowlist. Fails closed, as the code allowlist does. + if (constraints.allowedCustomers?.length) { + if (!metadata?.customer) { + return { + approved: false, + reason: "CUSTOMER_NOT_ALLOWED", + detail: "No customer given; this policy only allows listed customers", + }; + } if (!constraints.allowedCustomers.includes(metadata.customer)) { return { approved: false, diff --git a/test/policy.test.ts b/test/policy.test.ts index 0960caa..80ba4b5 100644 --- a/test/policy.test.ts +++ b/test/policy.test.ts @@ -12,19 +12,19 @@ const req = (amount: unknown, extra: Record = {}) => ({ agentId: "billing-agent", amount, idempotencyKey: "k", ...extra }) as never; test("approves a valid in-budget request and reports remaining", () => { - const r = checkPolicy(policy, req(4_500)); + const r = checkPolicy(policy, req(4_500, { metadata: { code: "api_calls" } })); assert.equal(r.approved, true); assert.deepEqual(r.remaining, { budget: 15_500, period: "monthly" }); }); test("denies over per-event max", () => { - const r = checkPolicy(policy, req(6_000)); + const r = checkPolicy(policy, req(6_000, { metadata: { code: "api_calls" } })); assert.equal(r.approved, false); assert.equal(r.reason, "AMOUNT_EXCEEDED"); }); test("denies when amount exceeds remaining budget (currentSpend applied)", () => { - const r = checkPolicy(policy, req(3_000), 18_000); + const r = checkPolicy(policy, req(3_000, { metadata: { code: "api_calls" } }), 18_000); assert.equal(r.approved, false); assert.equal(r.reason, "BUDGET_EXCEEDED"); }); @@ -36,7 +36,7 @@ test("denies a code not in the allowlist", () => { }); test("denies a blocked customer", () => { - const r = checkPolicy(policy, req(1_000, { metadata: { customer: "acme" } })); + const r = checkPolicy(policy, req(1_000, { metadata: { code: "api_calls", customer: "acme" } })); assert.equal(r.approved, false); assert.equal(r.reason, "CUSTOMER_BLOCKED"); }); @@ -59,3 +59,36 @@ test("rejects a malformed request shape (missing agentId)", () => { assert.equal(r.approved, false); assert.equal(r.reason, "INVALID_REQUEST"); }); + +// #8: an allowlist used to be skipped when the request omitted the field, so a +// request with no code (or no customer) cleared a policy that restricts it. +test("denies a request with no code when the policy has a code allowlist", () => { + for (const extra of [{}, { metadata: {} }, { metadata: { code: "" } }]) { + const r = checkPolicy(policy, req(1_000, extra)); + assert.equal(r.approved, false); + assert.equal(r.reason, "CODE_NOT_ALLOWED"); + } +}); + +const customerScoped: SpendingPolicy = { + ...policy, + constraints: { maxPerEvent: 5_000, allowedCustomers: ["globex"] }, +}; + +test("denies a request with no customer when the policy has a customer allowlist", () => { + for (const extra of [{}, { metadata: {} }, { metadata: { customer: "" } }]) { + const r = checkPolicy(customerScoped, req(1_000, extra)); + assert.equal(r.approved, false); + assert.equal(r.reason, "CUSTOMER_NOT_ALLOWED"); + } +}); + +test("still approves a listed customer, and an unlisted one is still denied", () => { + assert.equal(checkPolicy(customerScoped, req(1_000, { metadata: { customer: "globex" } })).approved, true); + assert.equal(checkPolicy(customerScoped, req(1_000, { metadata: { customer: "acme" } })).reason, "CUSTOMER_NOT_ALLOWED"); +}); + +test("a policy with no allowlists does not require code or customer", () => { + const open: SpendingPolicy = { ...policy, constraints: { maxPerEvent: 5_000 } }; + assert.equal(checkPolicy(open, req(1_000)).approved, true); +});