From 000a86bd81e3a39b0383a6339426fdbae39a93b9 Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Tue, 3 Nov 2026 03:48:50 -0700 Subject: [PATCH 1/9] Look up condition evaluators as own registry properties A condition key that names an inherited object member, such as "constructor", resolved to a built-in function that returned a truthy value, so a grant carrying it applied as if it had no condition. The key is now an unknown condition and the evaluation throws. --- packages/authz/src/conditions.test.ts | 18 ++++++++++++++++++ packages/authz/src/conditions.ts | 18 ++++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/packages/authz/src/conditions.test.ts b/packages/authz/src/conditions.test.ts index b07005faa..48c2e8316 100644 --- a/packages/authz/src/conditions.test.ts +++ b/packages/authz/src/conditions.test.ts @@ -3,6 +3,16 @@ import { describe, test, expect } from "bun:test"; import { evaluateConditions } from "./conditions"; import type { ConditionContext, ConditionRegistry } from "./types"; +const PROTOTYPE_KEYS = ["constructor", "toString", "__proto__"]; + +// An own enumerable key, as a JSON request body carries it. An object +// literal cannot make "__proto__" an own key. +function ownKey(key: string): Record { + const conditions: Record = {}; + Object.defineProperty(conditions, key, { value: 1, enumerable: true }); + return conditions; +} + function ctx(overrides?: Partial): ConditionContext { return { now: new Date("2025-06-15T14:30:00Z"), @@ -150,6 +160,14 @@ describe("evaluateConditions", () => { expect(secondCalled).toBe(false); }); + test("a key that names an Object prototype property is unknown", async () => { + for (const key of PROTOTYPE_KEYS) { + expect(evaluateConditions(ownKey(key), ctx(), {})).rejects.toThrow( + `Unknown condition: "${key}"`, + ); + } + }); + test("no registry provided with non-null conditions throws", async () => { // No registry argument at all -- defaults to empty registry expect(evaluateConditions({ some_condition: true }, ctx())).rejects.toThrow( diff --git a/packages/authz/src/conditions.ts b/packages/authz/src/conditions.ts index c641dbc8e..f85df54f5 100644 --- a/packages/authz/src/conditions.ts +++ b/packages/authz/src/conditions.ts @@ -1,4 +1,18 @@ -import type { ConditionContext, ConditionRegistry } from "./types"; +import type { + ConditionContext, + ConditionEvaluator, + ConditionRegistry, +} from "./types"; + +// Own properties only: a registry is a plain object, so a key such as +// "constructor" or "__proto__" would otherwise resolve to an inherited value +// that is not an evaluator. +function lookupEvaluator( + registry: ConditionRegistry, + key: string, +): ConditionEvaluator | undefined { + return Object.hasOwn(registry, key) ? registry[key] : undefined; +} /** * Evaluate whether a grant's conditions are satisfied. @@ -21,7 +35,7 @@ export async function evaluateConditions( if (keys.length === 0) return true; for (const key of keys) { - const evaluator = registry[key]; + const evaluator = lookupEvaluator(registry, key); if (!evaluator) { throw new Error( `Unknown condition: "${key}". Register an evaluator in the condition registry.`, From c6622cca23e8887f0c6ffa739b9ae589a0b0ceae Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Mon, 2 Nov 2026 22:05:43 -0700 Subject: [PATCH 2/9] Give each condition evaluator a schema-backed validate entrypoint A boundary that stores grant conditions needs to reject a body that the evaluator would later refuse, and it can only do that without drift if both read one set of shape rules. The new schemas are also stricter than the old hand-rolled checks: time_window rejects undeclared keys, and tool rejects an empty condition value. A stored grant carrying one of these now throws at evaluation instead of being ignored or silently withheld. --- bun.lock | 1 + packages/authz/README.md | 9 + packages/authz/package.json | 3 +- .../authz/src/condition-evaluator.test.ts | 56 +++++ packages/authz/src/condition-evaluator.ts | 40 +++ packages/authz/src/conditions.test.ts | 48 ++-- packages/authz/src/conditions.ts | 2 +- packages/authz/src/evaluate.test.ts | 35 ++- packages/authz/src/index.ts | 1 + packages/authz/src/mail-admission.test.ts | 14 +- packages/authz/src/time-window.test.ts | 228 +++++++++++------- packages/authz/src/time-window.ts | 152 +++++------- packages/authz/src/tool-consumer.test.ts | 40 ++- packages/authz/src/tool-consumer.ts | 33 ++- packages/types/src/authz.ts | 15 +- 15 files changed, 434 insertions(+), 243 deletions(-) create mode 100644 packages/authz/src/condition-evaluator.test.ts create mode 100644 packages/authz/src/condition-evaluator.ts diff --git a/bun.lock b/bun.lock index 5b79704c0..8a7208c0e 100644 --- a/bun.lock +++ b/bun.lock @@ -283,6 +283,7 @@ "version": "0.5.0", "dependencies": { "@intx/types": "workspace:*", + "arktype": "catalog:", }, }, "packages/crypto": { diff --git a/packages/authz/README.md b/packages/authz/README.md index 77bca2f48..1de8b0fa0 100644 --- a/packages/authz/README.md +++ b/packages/authz/README.md @@ -64,10 +64,19 @@ evaluation is fail-closed. `ConditionRegistry` evaluates each condition, and `"pessimistic"` applies a conditioned `deny` or `ask` as if it held and skips a conditioned `allow`, for a check that has no request. +- `createConditionEvaluator` — build a `ConditionEvaluator` from a + condition name, an arktype schema for the condition body, and an + evaluation callback. The evaluator's `validate` reports what the + schema rejects, and its `evaluate` parses with the same schema and + throws on the same input, so a boundary check and evaluation share + one set of shape rules. - `timeWindowEvaluator` — a `ConditionEvaluator` for time-of-day windows. Register it under the `time_window` key in a `ConditionRegistry` to gate grants on `{ after, before, timezone }`; cross-midnight windows are supported. +- `toolConsumerEvaluator` — a `ConditionEvaluator` for credential + consumers. Register it under the `tool` key to narrow a grant to one + consumer identity. - `evaluateConditions` — run a grant's `conditions` object against a `ConditionRegistry`. - `createInMemoryGrantStore` — a `GrantStore` backed by an in-memory diff --git a/packages/authz/package.json b/packages/authz/package.json index ac4df6ae0..a5b62de3a 100644 --- a/packages/authz/package.json +++ b/packages/authz/package.json @@ -12,7 +12,8 @@ } }, "dependencies": { - "@intx/types": "workspace:*" + "@intx/types": "workspace:*", + "arktype": "catalog:" }, "files": [ "dist", diff --git a/packages/authz/src/condition-evaluator.test.ts b/packages/authz/src/condition-evaluator.test.ts new file mode 100644 index 000000000..25ec6d5c0 --- /dev/null +++ b/packages/authz/src/condition-evaluator.test.ts @@ -0,0 +1,56 @@ +import { describe, test, expect } from "bun:test"; +import { type } from "arktype"; + +import { createConditionEvaluator } from "./condition-evaluator"; +import type { ConditionContext } from "./types"; + +const ctx: ConditionContext = { + now: new Date("2026-06-15T14:30:00Z"), + resource: "agent:agt_abc", + action: "read", + principalId: "prn_1", + tenantId: "tnt_1", + consumer: "", +}; + +describe("createConditionEvaluator", () => { + const Limit = type({ max: "number" }).pipe(({ max }) => max * 2); + + test("validate returns null for a value the schema accepts", () => { + const evaluator = createConditionEvaluator("limit", Limit, () => true); + expect(evaluator.validate({ max: 1 })).toBeNull(); + }); + + test("validate names the condition and joins problems on one line", () => { + const evaluator = createConditionEvaluator( + "limit", + type({ "+": "reject", max: "number" }), + () => true, + ); + expect(evaluator.validate({ cap: 1 })).toBe( + "limit: max must be a number (was missing); cap must be removed", + ); + }); + + test("evaluate passes the parsed value to the callback", async () => { + let received: unknown; + const evaluator = createConditionEvaluator("limit", Limit, (value) => { + received = value; + return true; + }); + expect(await evaluator.evaluate({ max: 3 }, ctx)).toBe(true); + expect(received).toBe(6); + }); + + test("evaluate throws the validate message without calling back", () => { + let called = false; + const evaluator = createConditionEvaluator("limit", Limit, () => { + called = true; + return true; + }); + expect(() => evaluator.evaluate({ max: "x" }, ctx)).toThrow( + "limit: max must be a number (was a string)", + ); + expect(called).toBe(false); + }); +}); diff --git a/packages/authz/src/condition-evaluator.ts b/packages/authz/src/condition-evaluator.ts new file mode 100644 index 000000000..ada8bfafb --- /dev/null +++ b/packages/authz/src/condition-evaluator.ts @@ -0,0 +1,40 @@ +import { type, type Type } from "arktype"; + +import type { ConditionContext, ConditionEvaluator } from "./types"; + +/** + * Build a condition evaluator whose shape check and evaluation share one + * schema. The returned `validate` reports what the schema rejects. The + * returned `evaluate` parses with the same schema, throws on the same input, + * and passes the parsed body to the `evaluate` callback. A stored body that + * bypassed `validate` therefore still fails loud at evaluation time. + * + * Every message starts with `name`, which is the condition's registry key. + */ +export function createConditionEvaluator( + name: string, + schema: S, + evaluate: ( + value: S["infer"], + ctx: ConditionContext, + ) => boolean | Promise, +): ConditionEvaluator { + // One line per body, so a caller can join several problems into one + // message. + const describe = (errors: type.errors) => + `${name}: ${errors.map((e) => e.message).join("; ")}`; + + return { + validate(value) { + const parsed = schema(value); + return parsed instanceof type.errors ? describe(parsed) : null; + }, + evaluate(value, ctx) { + const parsed = schema(value); + if (parsed instanceof type.errors) { + throw new Error(describe(parsed)); + } + return evaluate(parsed, ctx); + }, + }; +} diff --git a/packages/authz/src/conditions.test.ts b/packages/authz/src/conditions.test.ts index 48c2e8316..8aac1dcf1 100644 --- a/packages/authz/src/conditions.test.ts +++ b/packages/authz/src/conditions.test.ts @@ -1,7 +1,17 @@ import { describe, test, expect } from "bun:test"; import { evaluateConditions } from "./conditions"; -import type { ConditionContext, ConditionRegistry } from "./types"; +import type { + ConditionContext, + ConditionEvaluator, + ConditionRegistry, +} from "./types"; + +function evaluator( + evaluate: ConditionEvaluator["evaluate"], +): ConditionEvaluator { + return { validate: () => null, evaluate }; +} const PROTOTYPE_KEYS = ["constructor", "toString", "__proto__"]; @@ -42,7 +52,7 @@ describe("evaluateConditions", () => { test("single condition evaluated against registry", async () => { const registry: ConditionRegistry = { - is_tuesday: () => true, + is_tuesday: evaluator(() => true), }; expect( @@ -52,7 +62,7 @@ describe("evaluateConditions", () => { test("condition returning false rejects the grant", async () => { const registry: ConditionRegistry = { - always_fail: () => false, + always_fail: evaluator(() => false), }; expect( @@ -62,9 +72,9 @@ describe("evaluateConditions", () => { test("all conditions must pass (AND semantics)", async () => { const registry: ConditionRegistry = { - cond_a: () => true, - cond_b: () => true, - cond_c: () => false, + cond_a: evaluator(() => true), + cond_b: evaluator(() => true), + cond_c: evaluator(() => false), }; expect( @@ -78,8 +88,8 @@ describe("evaluateConditions", () => { test("all conditions passing returns true", async () => { const registry: ConditionRegistry = { - cond_a: () => true, - cond_b: () => true, + cond_a: evaluator(() => true), + cond_b: evaluator(() => true), }; expect( @@ -90,10 +100,10 @@ describe("evaluateConditions", () => { test("evaluator receives the condition value", async () => { let receivedValue: unknown; const registry: ConditionRegistry = { - threshold: (value) => { + threshold: evaluator((value) => { receivedValue = value; return true; - }, + }), }; await evaluateConditions({ threshold: 42 }, ctx(), registry); @@ -104,10 +114,10 @@ describe("evaluateConditions", () => { test("evaluator receives the full context", async () => { let receivedCtx: ConditionContext | undefined; const registry: ConditionRegistry = { - spy: (_value, c) => { + spy: evaluator((_value, c) => { receivedCtx = c; return true; - }, + }), }; const c = ctx({ principalId: "prn_spy", tenantId: "tnt_spy" }); @@ -121,10 +131,10 @@ describe("evaluateConditions", () => { test("async evaluators are supported", async () => { const registry: ConditionRegistry = { - async_check: async () => { + async_check: evaluator(async () => { await new Promise((r) => setTimeout(r, 1)); return true; - }, + }), }; expect( @@ -134,10 +144,10 @@ describe("evaluateConditions", () => { test("async evaluator returning false rejects", async () => { const registry: ConditionRegistry = { - async_fail: async () => { + async_fail: evaluator(async () => { await new Promise((r) => setTimeout(r, 1)); return false; - }, + }), }; expect( @@ -148,11 +158,11 @@ describe("evaluateConditions", () => { test("short-circuits on first failing condition", async () => { let secondCalled = false; const registry: ConditionRegistry = { - first: () => false, - second: () => { + first: evaluator(() => false), + second: evaluator(() => { secondCalled = true; return true; - }, + }), }; await evaluateConditions({ first: null, second: null }, ctx(), registry); diff --git a/packages/authz/src/conditions.ts b/packages/authz/src/conditions.ts index f85df54f5..f1f12cd9a 100644 --- a/packages/authz/src/conditions.ts +++ b/packages/authz/src/conditions.ts @@ -42,7 +42,7 @@ export async function evaluateConditions( ); } - const result = await evaluator(conditions[key], ctx); + const result = await evaluator.evaluate(conditions[key], ctx); if (!result) return false; } diff --git a/packages/authz/src/evaluate.test.ts b/packages/authz/src/evaluate.test.ts index fa905265b..c8bac66b8 100644 --- a/packages/authz/src/evaluate.test.ts +++ b/packages/authz/src/evaluate.test.ts @@ -1,7 +1,18 @@ import { describe, test, expect } from "bun:test"; import { authorize, evaluateGrants } from "./evaluate"; -import type { ConditionRegistry, GrantRule, GrantStore } from "./types"; +import type { + ConditionEvaluator, + ConditionRegistry, + GrantRule, + GrantStore, +} from "./types"; + +function evaluator( + evaluate: ConditionEvaluator["evaluate"], +): ConditionEvaluator { + return { validate: () => null, evaluate }; +} // The mode for a check with no request. On grants with no conditions it // gives the same result as a registry. @@ -473,7 +484,7 @@ describe("evaluateGrants conditions", () => { test("grant with conditions evaluated to true is included", async () => { const registry: ConditionRegistry = { - max_spend_per_day: () => true, + max_spend_per_day: evaluator(() => true), }; const grants = [ grant({ @@ -493,7 +504,7 @@ describe("evaluateGrants conditions", () => { test("grant with conditions evaluated to false is excluded", async () => { const registry: ConditionRegistry = { - max_spend_per_day: () => false, + max_spend_per_day: evaluator(() => false), }; const grants = [ grant({ @@ -527,7 +538,7 @@ describe("evaluateGrants conditions", () => { test("conditional and unconditional grants coexist", async () => { const registry: ConditionRegistry = { - time_window: () => false, + time_window: evaluator(() => false), }; const grants = [ grant({ @@ -557,7 +568,7 @@ describe("evaluateGrants conditions", () => { test("more specific conditional grant beats less specific unconditional", async () => { const registry: ConditionRegistry = { - time_window: () => true, + time_window: evaluator(() => true), }; const grants = [ grant({ @@ -587,11 +598,11 @@ describe("evaluateGrants conditions", () => { let capturedPrincipal = ""; let capturedTenant = ""; const registry: ConditionRegistry = { - spy: (_value, ctx) => { + spy: evaluator((_value, ctx) => { capturedPrincipal = ctx.principalId; capturedTenant = ctx.tenantId; return true; - }, + }), }; const grants = [ grant({ @@ -704,7 +715,9 @@ describe("evaluateGrants action verbs from spec", () => { test("spend action with conditions via registry", async () => { const registry: ConditionRegistry = { - max_spend_per_day: (value) => typeof value === "number" && value > 50, + max_spend_per_day: evaluator( + (value) => typeof value === "number" && value > 50, + ), }; const grants = [ grant({ @@ -1023,7 +1036,7 @@ describe("authorize with in-memory store", () => { test("passes registry through to condition evaluation", async () => { const registry: ConditionRegistry = { - time_window: () => true, + time_window: evaluator(() => true), }; const store = memoryStore([ grant({ @@ -1072,11 +1085,11 @@ describe("authorize with in-memory store", () => { let capturedPrincipal = ""; let capturedTenant = ""; const registry: ConditionRegistry = { - spy: (_v, ctx) => { + spy: evaluator((_v, ctx) => { capturedPrincipal = ctx.principalId; capturedTenant = ctx.tenantId; return true; - }, + }), }; const store = memoryStore([ grant({ diff --git a/packages/authz/src/index.ts b/packages/authz/src/index.ts index d39115c17..09fe62d8b 100644 --- a/packages/authz/src/index.ts +++ b/packages/authz/src/index.ts @@ -20,6 +20,7 @@ export { } from "./mail-accept-relation"; export { evaluateMailAdmission } from "./mail-admission"; export type { MailAdmissionResult } from "./mail-admission"; +export { createConditionEvaluator } from "./condition-evaluator"; export { evaluateConditions } from "./conditions"; export { timeWindowEvaluator } from "./time-window"; export { diff --git a/packages/authz/src/mail-admission.test.ts b/packages/authz/src/mail-admission.test.ts index b96fb6673..31f9ff1ac 100644 --- a/packages/authz/src/mail-admission.test.ts +++ b/packages/authz/src/mail-admission.test.ts @@ -3,7 +3,13 @@ import { describe, test, expect } from "bun:test"; import { evaluateMailAdmission } from "./mail-admission"; import { mailAcceptResource, MAIL_ACCEPT_ACTION } from "./coord"; import type { MailAcceptCoordinate } from "./coord"; -import type { ConditionRegistry, GrantRule } from "./types"; +import type { ConditionEvaluator, ConditionRegistry, GrantRule } from "./types"; + +function evaluator( + evaluate: ConditionEvaluator["evaluate"], +): ConditionEvaluator { + return { validate: () => null, evaluate }; +} function grant( overrides: Partial & @@ -80,7 +86,7 @@ describe("evaluateMailAdmission", () => { }); test("a conditioned grant with a registry supplied does not throw", async () => { - const registry: ConditionRegistry = { time_window: () => true }; + const registry: ConditionRegistry = { time_window: evaluator(() => true) }; const result = await evaluateMailAdmission({ senderCoordinates: [PRINCIPAL], recipientGrants: [ @@ -431,7 +437,7 @@ describe("evaluateMailAdmission", () => { test("a conditioned deny with a registry that applies it blocks admission", async () => { const registry: ConditionRegistry = { - always: () => true, + always: evaluator(() => true), }; const result = await evaluateMailAdmission({ senderCoordinates: [PRINCIPAL, TENANT], @@ -448,7 +454,7 @@ describe("evaluateMailAdmission", () => { test("a conditioned deny whose condition does not hold does not block admission", async () => { const registry: ConditionRegistry = { - never: () => false, + never: evaluator(() => false), }; const result = await evaluateMailAdmission({ senderCoordinates: [PRINCIPAL, TENANT], diff --git a/packages/authz/src/time-window.test.ts b/packages/authz/src/time-window.test.ts index 9519fa2ce..40970537c 100644 --- a/packages/authz/src/time-window.test.ts +++ b/packages/authz/src/time-window.test.ts @@ -24,23 +24,33 @@ describe("timeWindowEvaluator", () => { const window = { after: "09:00", before: "17:00", timezone: "UTC" }; test("inside window returns true", () => { - expect(timeWindowEvaluator(window, makeCtx(utc(12, 0)))).toBe(true); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(12, 0)))).toBe( + true, + ); }); test("at start boundary returns true", () => { - expect(timeWindowEvaluator(window, makeCtx(utc(9, 0)))).toBe(true); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(9, 0)))).toBe( + true, + ); }); test("at end boundary returns false (exclusive)", () => { - expect(timeWindowEvaluator(window, makeCtx(utc(17, 0)))).toBe(false); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(17, 0)))).toBe( + false, + ); }); test("before window returns false", () => { - expect(timeWindowEvaluator(window, makeCtx(utc(8, 59)))).toBe(false); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(8, 59)))).toBe( + false, + ); }); test("after window returns false", () => { - expect(timeWindowEvaluator(window, makeCtx(utc(17, 1)))).toBe(false); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(17, 1)))).toBe( + false, + ); }); }); @@ -48,44 +58,68 @@ describe("timeWindowEvaluator", () => { const window = { after: "22:00", before: "06:00", timezone: "UTC" }; test("late night returns true", () => { - expect(timeWindowEvaluator(window, makeCtx(utc(23, 0)))).toBe(true); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(23, 0)))).toBe( + true, + ); }); test("early morning returns true", () => { - expect(timeWindowEvaluator(window, makeCtx(utc(3, 0)))).toBe(true); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(3, 0)))).toBe( + true, + ); }); test("at start boundary returns true", () => { - expect(timeWindowEvaluator(window, makeCtx(utc(22, 0)))).toBe(true); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(22, 0)))).toBe( + true, + ); }); test("at end boundary returns false (exclusive)", () => { - expect(timeWindowEvaluator(window, makeCtx(utc(6, 0)))).toBe(false); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(6, 0)))).toBe( + false, + ); }); test("daytime returns false", () => { - expect(timeWindowEvaluator(window, makeCtx(utc(12, 0)))).toBe(false); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(12, 0)))).toBe( + false, + ); }); }); describe("midnight edge cases", () => { test("midnight in a cross-midnight window returns true", () => { const window = { after: "22:00", before: "06:00", timezone: "UTC" }; - expect(timeWindowEvaluator(window, makeCtx(utc(0, 0)))).toBe(true); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(0, 0)))).toBe( + true, + ); }); test("00:01 in a cross-midnight window returns true", () => { const window = { after: "22:00", before: "06:00", timezone: "UTC" }; - expect(timeWindowEvaluator(window, makeCtx(utc(0, 1)))).toBe(true); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(0, 1)))).toBe( + true, + ); }); test("midnight in a normal window starting at 00:00", () => { const window = { after: "00:00", before: "08:00", timezone: "UTC" }; - expect(timeWindowEvaluator(window, makeCtx(utc(0, 0)))).toBe(true); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(0, 0)))).toBe( + true, + ); }); }); describe("timezone conversion", () => { + test("evaluates against a UTC offset", () => { + // 00:30 UTC = 05:30 at +05:00 + const window = { after: "05:00", before: "06:00", timezone: "+05:00" }; + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(0, 30)))).toBe( + true, + ); + }); + test("converts to specified timezone", () => { // 20:00 UTC = 13:00 America/Los_Angeles (PDT, UTC-7) const window = { @@ -93,7 +127,9 @@ describe("timeWindowEvaluator", () => { before: "14:00", timezone: "America/Los_Angeles", }; - expect(timeWindowEvaluator(window, makeCtx(utc(20, 0)))).toBe(true); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(20, 0)))).toBe( + true, + ); }); test("outside window in specified timezone", () => { @@ -103,86 +139,100 @@ describe("timeWindowEvaluator", () => { before: "17:00", timezone: "America/Los_Angeles", }; - expect(timeWindowEvaluator(window, makeCtx(utc(10, 0)))).toBe(false); + expect(timeWindowEvaluator.evaluate(window, makeCtx(utc(10, 0)))).toBe( + false, + ); }); }); describe("validation", () => { const ctx = makeCtx(utc(12, 0)); - - test("throws on non-object value", () => { - expect(() => timeWindowEvaluator("not an object", ctx)).toThrow( - "expected an object", - ); - }); - - test("throws on null value", () => { - expect(() => timeWindowEvaluator(null, ctx)).toThrow( - "expected an object", - ); - }); - - test("throws on missing after", () => { - expect(() => - timeWindowEvaluator({ before: "17:00", timezone: "UTC" }, ctx), - ).toThrow('"after" must be a string'); - }); - - test("throws on missing before", () => { - expect(() => - timeWindowEvaluator({ after: "09:00", timezone: "UTC" }, ctx), - ).toThrow('"before" must be a string'); - }); - - test("throws on missing timezone", () => { - expect(() => - timeWindowEvaluator({ after: "09:00", before: "17:00" }, ctx), - ).toThrow('"timezone" is required'); - }); - - test("throws on malformed time string", () => { - expect(() => - timeWindowEvaluator( - { after: "9:00", before: "17:00", timezone: "UTC" }, - ctx, - ), - ).toThrow("must be HH:MM"); - }); - - test("throws on out-of-range hour", () => { - expect(() => - timeWindowEvaluator( - { after: "25:00", before: "17:00", timezone: "UTC" }, - ctx, - ), - ).toThrow("out of range"); - }); - - test("throws on out-of-range minute", () => { - expect(() => - timeWindowEvaluator( - { after: "09:60", before: "17:00", timezone: "UTC" }, - ctx, - ), - ).toThrow("out of range"); - }); - - test("throws when after equals before", () => { - expect(() => - timeWindowEvaluator( - { after: "12:00", before: "12:00", timezone: "UTC" }, - ctx, - ), - ).toThrow("zero-duration window"); - }); - - test("throws on invalid timezone", () => { - expect(() => - timeWindowEvaluator( - { after: "09:00", before: "17:00", timezone: "Mars/Olympus" }, - ctx, - ), - ).toThrow(); + const valid = { after: "09:00", before: "17:00", timezone: "UTC" }; + + const invalid: [string, unknown, string][] = [ + ["a non-object value", "not an object", "must be an object"], + ["a null value", null, "must be an object"], + [ + "a missing after", + { before: "17:00", timezone: "UTC" }, + "after must be a string", + ], + [ + "a missing before", + { after: "09:00", timezone: "UTC" }, + "before must be a string", + ], + [ + "a missing timezone", + { after: "09:00", before: "17:00" }, + "timezone must be a string", + ], + [ + "the wrong key names", + { start: "09:00", end: "17:00", timezone: "UTC" }, + "start must be removed", + ], + ["an undeclared key", { ...valid, extra: 1 }, "extra must be removed"], + [ + "a malformed time string", + { ...valid, after: "9:00" }, + 'after must be an HH:MM time from 00:00 to 23:59 (was "9:00")', + ], + [ + "an out-of-range hour", + { ...valid, after: "25:00" }, + "after must be an HH:MM time", + ], + [ + "an out-of-range minute", + { ...valid, before: "09:60" }, + "before must be an HH:MM time", + ], + [ + "equal bounds", + { ...valid, after: "12:00", before: "12:00" }, + 'must be a window whose "after" and "before" differ', + ], + [ + "an unknown timezone", + { ...valid, timezone: "Mars/Olympus" }, + "timezone must be a time zone name or UTC offset", + ], + ]; + + test("accepts a valid window", () => { + expect(timeWindowEvaluator.validate(valid)).toBeNull(); + }); + + test("accepts a UTC offset and a zone name in any letter case", () => { + for (const timezone of [ + "+05:30", + "-0800", + "utc", + "america/los_angeles", + ]) { + expect(timeWindowEvaluator.validate({ ...valid, timezone })).toBeNull(); + } + }); + + for (const [label, value, message] of invalid) { + test(`validate rejects ${label}`, () => { + const problem = timeWindowEvaluator.validate(value); + expect(problem).toStartWith("time_window: "); + expect(problem).toContain(message); + }); + + test(`evaluate throws on ${label}`, () => { + expect(() => timeWindowEvaluator.evaluate(value, ctx)).toThrow(message); + }); + } + + test("reports every problem on one line", () => { + const problem = timeWindowEvaluator.validate({ start: "09:00" }); + expect(problem).not.toContain("\n"); + expect(problem).toContain("after must be a string"); + expect(problem).toContain("timezone must be a string"); + expect(problem).toContain("start must be removed"); }); }); }); diff --git a/packages/authz/src/time-window.ts b/packages/authz/src/time-window.ts index e6d77dfa8..a08ddc135 100644 --- a/packages/authz/src/time-window.ts +++ b/packages/authz/src/time-window.ts @@ -6,87 +6,72 @@ // { time_window: { after: "09:00", before: "17:00", timezone: "America/Los_Angeles" } } // // The evaluator returns true if ctx.now falls within [after, before). -// Cross-midnight windows (after >= before) are supported: +// Cross-midnight windows (after > before) are supported: // { after: "22:00", before: "06:00" } → active from 10pm to 6am. // -// Timezone is required — omitting it is an error. Time strings must -// be HH:MM in 24-hour format. +// Timezone is required and must be a zone `Intl` accepts: an IANA name +// such as "America/Los_Angeles", or a UTC offset. Time strings must be +// HH:MM in 24-hour format, "after" and "before" must differ, and no other +// key is allowed. Each of these is an error. -import type { ConditionEvaluator } from "./types"; +import { type } from "arktype"; -type TimeWindowValue = { - after: string; - before: string; - timezone: string; -}; +import { createConditionEvaluator } from "./condition-evaluator"; -function parseHHMM(s: string, field: string): { hour: number; minute: number } { - const match = /^(\d{2}):(\d{2})$/.exec(s); - if (match === null || match[1] === undefined || match[2] === undefined) { - throw new Error( - `time_window: "${field}" must be HH:MM in 24-hour format, got "${s}"`, - ); - } - const hour = parseInt(match[1], 10); - const minute = parseInt(match[2], 10); - if (hour > 23 || minute > 59) { - throw new Error(`time_window: "${field}" is out of range, got "${s}"`); - } - return { hour, minute }; +// Throws a RangeError for a zone that `Intl` does not know. Built per call +// rather than cached: `Intl` accepts any letter case and UTC offsets, so a +// cache keyed by the authored string would grow with every distinct input. +function formatterFor(timezone: string): Intl.DateTimeFormat { + return new Intl.DateTimeFormat("en-US", { + timeZone: timezone, + hour: "2-digit", + minute: "2-digit", + hour12: false, + }); } -function validateShape(value: unknown): TimeWindowValue { - if (typeof value !== "object" || value === null) { - throw new Error( - `time_window: expected an object with { after, before, timezone }, got ${typeof value}`, - ); - } +const CLOCK_TIME = /^([01]\d|2[0-3]):([0-5]\d)$/; - const after = "after" in value ? value.after : undefined; - const before = "before" in value ? value.before : undefined; - const timezone = "timezone" in value ? value.timezone : undefined; +const ClockTime = type("string") + .narrow( + (s, ctx) => + CLOCK_TIME.test(s) || ctx.mustBe("an HH:MM time from 00:00 to 23:59"), + ) + .pipe((s) => parseInt(s.slice(0, 2), 10) * 60 + parseInt(s.slice(3), 10)); - if (typeof after !== "string") { - throw new Error( - `time_window: "after" must be a string, got ${typeof after}`, - ); - } - if (typeof before !== "string") { - throw new Error( - `time_window: "before" must be a string, got ${typeof before}`, - ); - } - if (typeof timezone !== "string") { - throw new Error( - `time_window: "timezone" is required and must be a string, got ${typeof timezone}`, - ); +const TimeZone = type("string").narrow((timezone, ctx) => { + try { + formatterFor(timezone); + return true; + } catch { + return ctx.mustBe("a time zone name or UTC offset"); } +}); - return { after, before, timezone }; -} - -function toMinutes(h: number, m: number): number { - return h * 60 + m; -} - -function getCurrentTimeInZone( - now: Date, - timezone: string, -): { hour: number; minute: number } { - const parts = new Intl.DateTimeFormat("en-US", { - timeZone: timezone, - hour: "2-digit", - minute: "2-digit", - hour12: false, - }).formatToParts(now); +const TimeWindow = type({ + "+": "reject", + after: ClockTime, + before: ClockTime, + timezone: TimeZone, +}).narrow( + (window, ctx) => + window.after !== window.before || + // The narrowed value holds minutes, not the authored HH:MM strings, so + // an "actual" here would mislead. + ctx.reject({ + expected: 'a window whose "after" and "before" differ', + actual: "", + }), +); +function minutesInZone(now: Date, timezone: string): number { let hour = 0; let minute = 0; - for (const part of parts) { + for (const part of formatterFor(timezone).formatToParts(now)) { if (part.type === "hour") hour = parseInt(part.value, 10); if (part.type === "minute") minute = parseInt(part.value, 10); } - return { hour, minute }; + return hour * 60 + minute; } /** @@ -95,29 +80,16 @@ function getCurrentTimeInZone( * Register as `time_window` in the condition registry: * `{ time_window: timeWindowEvaluator }` */ -export const timeWindowEvaluator: ConditionEvaluator = ( - value: unknown, - ctx, -): boolean => { - const tw = validateShape(value); - const after = parseHHMM(tw.after, "after"); - const before = parseHHMM(tw.before, "before"); - const current = getCurrentTimeInZone(ctx.now, tw.timezone); - - const afterMins = toMinutes(after.hour, after.minute); - const beforeMins = toMinutes(before.hour, before.minute); - const currentMins = toMinutes(current.hour, current.minute); - - if (afterMins === beforeMins) { - throw new Error( - `time_window: "after" and "before" are equal ("${tw.after}"), which produces a zero-duration window`, - ); - } - - if (afterMins < beforeMins) { - // Normal window: [after, before) - return currentMins >= afterMins && currentMins < beforeMins; - } - // Cross-midnight window: >= after OR < before - return currentMins >= afterMins || currentMins < beforeMins; -}; +export const timeWindowEvaluator = createConditionEvaluator( + "time_window", + TimeWindow, + ({ after, before, timezone }, ctx) => { + const current = minutesInZone(ctx.now, timezone); + if (after < before) { + // Normal window: [after, before) + return current >= after && current < before; + } + // Cross-midnight window: >= after OR < before + return current >= after || current < before; + }, +); diff --git a/packages/authz/src/tool-consumer.test.ts b/packages/authz/src/tool-consumer.test.ts index 6a06c2d6c..53773ee8f 100644 --- a/packages/authz/src/tool-consumer.test.ts +++ b/packages/authz/src/tool-consumer.test.ts @@ -42,21 +42,47 @@ function ctx(consumer: string): ConditionContext { describe("toolConsumerEvaluator", () => { test("matches a consumer identity that equals the value", () => { - expect(toolConsumerEvaluator(REACTOR, ctx(REACTOR))).toBe(true); + expect(toolConsumerEvaluator.evaluate(REACTOR, ctx(REACTOR))).toBe(true); }); test("withholds from a mismatched consumer", () => { - expect(toolConsumerEvaluator(REACTOR, ctx("tool:other"))).toBe(false); + expect(toolConsumerEvaluator.evaluate(REACTOR, ctx("tool:other"))).toBe( + false, + ); }); test("withholds from an empty consumer before comparing", () => { - // Even when the grant value is also empty, an empty consumer never matches. - expect(toolConsumerEvaluator("", ctx(""))).toBe(false); - expect(toolConsumerEvaluator(REACTOR, ctx(""))).toBe(false); + expect(toolConsumerEvaluator.evaluate(REACTOR, ctx(""))).toBe(false); + }); + + test("validate accepts a consumer identity", () => { + expect(toolConsumerEvaluator.validate(REACTOR)).toBeNull(); + }); + + test("validate rejects a non-string condition value", () => { + expect(toolConsumerEvaluator.validate(123)).toBe( + "tool: must be a non-empty consumer identity (was a number)", + ); + }); + + test("validate rejects an empty condition value", () => { + expect(toolConsumerEvaluator.validate("")).toBe( + "tool: must be a non-empty consumer identity", + ); }); test("throws on a non-string condition value", () => { - expect(() => toolConsumerEvaluator(123, ctx(REACTOR))).toThrow(); + expect(() => toolConsumerEvaluator.evaluate(123, ctx(REACTOR))).toThrow( + "tool: must be a non-empty consumer identity", + ); + }); + + test("throws on an empty condition value", () => { + // An empty grant value never reaches the equality check, so it cannot + // match an empty consumer. + expect(() => toolConsumerEvaluator.evaluate("", ctx(""))).toThrow( + "tool: must be a non-empty consumer identity", + ); }); }); @@ -81,7 +107,7 @@ describe("consumer identities", () => { test("an action consumer's grant does not authorize the same-named tool", () => { expect( - toolConsumerEvaluator( + toolConsumerEvaluator.evaluate( actionConsumer("deploy"), ctx(toolConsumer("deploy")), ), diff --git a/packages/authz/src/tool-consumer.ts b/packages/authz/src/tool-consumer.ts index 753cdf74b..13f0f2b59 100644 --- a/packages/authz/src/tool-consumer.ts +++ b/packages/authz/src/tool-consumer.ts @@ -13,14 +13,17 @@ // The evaluator returns true only when ctx.consumer is a non-empty identity // that exactly matches the configured value. An absent (empty) consumer fails // closed: a consumer-scoped grant never authorizes a caller that supplied no -// consumer identity, and it must be rejected before the equality check so a -// grant value of "" cannot match an empty consumer. A non-string condition -// value is a misconfiguration and throws, mirroring the fail-loud shape of the -// other evaluators rather than silently withholding. +// consumer identity. A condition value that is not a non-empty string is a +// misconfiguration and throws, mirroring the fail-loud shape of the other +// evaluators rather than silently withholding; because "" never reaches the +// comparison, an empty grant value cannot match an empty consumer. + +import { type } from "arktype"; import type { CredentialBinding } from "@intx/types"; -import type { ConditionEvaluator, ConditionRegistry } from "./types"; +import { createConditionEvaluator } from "./condition-evaluator"; +import type { ConditionRegistry } from "./types"; /** * Build the consumer identity for a tool package. This is the value stamped @@ -61,18 +64,14 @@ export function credentialBindingConsumer(binding: CredentialBinding): string { * Register as `tool` in the condition registry: * `{ tool: toolConsumerEvaluator }` */ -export const toolConsumerEvaluator: ConditionEvaluator = ( - value: unknown, - ctx, -): boolean => { - if (typeof value !== "string") { - throw new Error( - `tool: condition value must be a string consumer identity, got ${typeof value}`, - ); - } - if (ctx.consumer === "") return false; - return ctx.consumer === value; -}; +export const toolConsumerEvaluator = createConditionEvaluator( + "tool", + type("string > 0").describe("a non-empty consumer identity"), + (consumer, ctx) => { + if (ctx.consumer === "") return false; + return ctx.consumer === consumer; + }, +); /** * The condition registry for credential-use authorization. It carries only the diff --git a/packages/types/src/authz.ts b/packages/types/src/authz.ts index 9f0d6bd71..e1d472a8d 100644 --- a/packages/types/src/authz.ts +++ b/packages/types/src/authz.ts @@ -41,9 +41,16 @@ export type ConditionContext = { consumer: string; }; -export type ConditionEvaluator = ( - value: unknown, - ctx: ConditionContext, -) => boolean | Promise; +/** + * A condition's shape check and its evaluation. Both read the same rules, so + * a boundary that accepts a condition body through `validate` stores only + * bodies that `evaluate` can parse. + */ +export type ConditionEvaluator = { + /** Returns null for a valid condition body, otherwise the problem. */ + validate(value: unknown): string | null; + /** Throws when `validate` would reject the value. */ + evaluate(value: unknown, ctx: ConditionContext): boolean | Promise; +}; export type ConditionRegistry = Record; From 47c66b643f7449e521979de6ce68a8782143c331 Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Mon, 2 Nov 2026 22:51:42 -0700 Subject: [PATCH 3/9] Add validateConditions to check conditions against a registry --- packages/authz/README.md | 4 ++ packages/authz/src/conditions.test.ts | 91 +++++++++++++++++++++++- packages/authz/src/conditions.ts | 34 ++++++++- packages/authz/src/index.ts | 2 +- packages/authz/src/tool-consumer.test.ts | 10 +++ 5 files changed, 138 insertions(+), 3 deletions(-) diff --git a/packages/authz/README.md b/packages/authz/README.md index 1de8b0fa0..88bef6f02 100644 --- a/packages/authz/README.md +++ b/packages/authz/README.md @@ -79,6 +79,10 @@ evaluation is fail-closed. consumer identity. - `evaluateConditions` — run a grant's `conditions` object against a `ConditionRegistry`. +- `validateConditions` — check a grant's `conditions` object against a + `ConditionRegistry` without evaluating it, returning one problem per + unregistered key or rejected body. A boundary that stores conditions + calls it to refuse what `evaluateConditions` would later throw on. - `createInMemoryGrantStore` — a `GrantStore` backed by an in-memory grant list for tests and single-process deployments. - `matchPattern`, `patternSpecificity`, `grantSpecificity` — the diff --git a/packages/authz/src/conditions.test.ts b/packages/authz/src/conditions.test.ts index 8aac1dcf1..9f4b73e64 100644 --- a/packages/authz/src/conditions.test.ts +++ b/packages/authz/src/conditions.test.ts @@ -1,6 +1,7 @@ import { describe, test, expect } from "bun:test"; -import { evaluateConditions } from "./conditions"; +import { evaluateConditions, validateConditions } from "./conditions"; +import { timeWindowEvaluator } from "./time-window"; import type { ConditionContext, ConditionEvaluator, @@ -185,3 +186,91 @@ describe("evaluateConditions", () => { ); }); }); + +describe("validateConditions", () => { + function rejecting(problem: string): ConditionEvaluator { + return { validate: () => problem, evaluate: () => true }; + } + + test("null and empty conditions have no problems", () => { + expect(validateConditions(null, {})).toEqual([]); + expect(validateConditions({}, {})).toEqual([]); + }); + + test("entries every evaluator accepts have no problems", () => { + const registry: ConditionRegistry = { + cond_a: evaluator(() => true), + cond_b: evaluator(() => false), + }; + expect(validateConditions({ cond_a: 1, cond_b: 2 }, registry)).toEqual([]); + }); + + test("an unregistered key is named", () => { + expect(validateConditions({ bogus: true }, {})).toEqual([ + 'Unknown condition: "bogus"', + ]); + }); + + test("a key that names an Object prototype property is unknown", () => { + for (const key of PROTOTYPE_KEYS) { + expect(validateConditions(ownKey(key), {})).toEqual([ + `Unknown condition: "${key}"`, + ]); + } + }); + + test("an evaluator's problem passes through unchanged", () => { + const registry: ConditionRegistry = { + limit: rejecting("limit: max must be a number"), + }; + expect(validateConditions({ limit: {} }, registry)).toEqual([ + "limit: max must be a number", + ]); + }); + + test("the evaluator receives the condition value", () => { + let received: unknown; + const registry: ConditionRegistry = { + limit: { + validate: (value) => { + received = value; + return null; + }, + evaluate: () => true, + }, + }; + validateConditions({ limit: 42 }, registry); + expect(received).toBe(42); + }); + + test("every bad entry is reported in key order", () => { + const registry: ConditionRegistry = { + ok: evaluator(() => true), + bad: rejecting("bad: rejected"), + }; + expect(validateConditions({ unknown: 1, ok: 2, bad: 3 }, registry)).toEqual( + ['Unknown condition: "unknown"', "bad: rejected"], + ); + }); + + test("validation does not evaluate", () => { + let evaluated = false; + const registry: ConditionRegistry = { + spy: evaluator(() => { + evaluated = true; + return true; + }), + }; + validateConditions({ spy: null }, registry); + expect(evaluated).toBe(false); + }); + + test("reports a time_window body that is missing its timezone", () => { + expect( + validateConditions( + { time_window: { after: "09:00", before: "17:00" } }, + { time_window: timeWindowEvaluator }, + ), + ).toEqual(["time_window: timezone must be a string (was missing)"]); + }); +}); diff --git a/packages/authz/src/conditions.ts b/packages/authz/src/conditions.ts index f1f12cd9a..e0ab0ce77 100644 --- a/packages/authz/src/conditions.ts +++ b/packages/authz/src/conditions.ts @@ -4,6 +4,10 @@ import type { ConditionRegistry, } from "./types"; +function unknownConditionMessage(key: string): string { + return `Unknown condition: "${key}"`; +} + // Own properties only: a registry is a plain object, so a key such as // "constructor" or "__proto__" would otherwise resolve to an inherited value // that is not an evaluator. @@ -38,7 +42,7 @@ export async function evaluateConditions( const evaluator = lookupEvaluator(registry, key); if (!evaluator) { throw new Error( - `Unknown condition: "${key}". Register an evaluator in the condition registry.`, + `${unknownConditionMessage(key)}. Register an evaluator in the condition registry.`, ); } @@ -48,3 +52,31 @@ export async function evaluateConditions( return true; } + +/** + * Check a grant's conditions object against a registry without evaluating + * it. Returns one problem per bad entry, in key order: an unregistered key, + * or a body the key's evaluator rejects. An empty result means + * `evaluateConditions` can evaluate every entry against the same registry. + * + * Each evaluator's message already names its condition, so it passes + * through unchanged. + */ +export function validateConditions( + conditions: Record | null, + registry: ConditionRegistry, +): string[] { + if (!conditions) return []; + + const problems: string[] = []; + for (const [key, value] of Object.entries(conditions)) { + const evaluator = lookupEvaluator(registry, key); + if (!evaluator) { + problems.push(unknownConditionMessage(key)); + continue; + } + const problem = evaluator.validate(value); + if (problem !== null) problems.push(problem); + } + return problems; +} diff --git a/packages/authz/src/index.ts b/packages/authz/src/index.ts index 09fe62d8b..9b54f228b 100644 --- a/packages/authz/src/index.ts +++ b/packages/authz/src/index.ts @@ -21,7 +21,7 @@ export { export { evaluateMailAdmission } from "./mail-admission"; export type { MailAdmissionResult } from "./mail-admission"; export { createConditionEvaluator } from "./condition-evaluator"; -export { evaluateConditions } from "./conditions"; +export { evaluateConditions, validateConditions } from "./conditions"; export { timeWindowEvaluator } from "./time-window"; export { actionConsumer, diff --git a/packages/authz/src/tool-consumer.test.ts b/packages/authz/src/tool-consumer.test.ts index 53773ee8f..57aa11926 100644 --- a/packages/authz/src/tool-consumer.test.ts +++ b/packages/authz/src/tool-consumer.test.ts @@ -115,6 +115,16 @@ describe("consumer identities", () => { }); }); +describe("CREDENTIAL_USE_CONDITIONS", () => { + // validateConditions passes an evaluator's message through without adding + // the key, so each evaluator must name itself by its registry key. + test("each evaluator names itself by its registry key", () => { + for (const [key, evaluator] of Object.entries(CREDENTIAL_USE_CONDITIONS)) { + expect(evaluator.validate(undefined)).toStartWith(`${key}: `); + } + }); +}); + describe("CREDENTIAL_USE_CONDITIONS through evaluateGrants", () => { const opts = (consumer?: string) => ({ conditions: CREDENTIAL_USE_CONDITIONS, From 8630a3a069f35eec629925ce89349255204b6430 Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Mon, 2 Nov 2026 23:29:57 -0700 Subject: [PATCH 4/9] Require a registry when evaluating conditions An empty default registry makes every condition key throw, so the default did no work except hide a caller that forgot its registry. --- packages/authz/src/conditions.test.ts | 11 ++--------- packages/authz/src/conditions.ts | 2 +- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/authz/src/conditions.test.ts b/packages/authz/src/conditions.test.ts index 9f4b73e64..7e0d567b0 100644 --- a/packages/authz/src/conditions.test.ts +++ b/packages/authz/src/conditions.test.ts @@ -38,11 +38,11 @@ function ctx(overrides?: Partial): ConditionContext { describe("evaluateConditions", () => { test("null conditions are always met", async () => { - expect(await evaluateConditions(null, ctx())).toBe(true); + expect(await evaluateConditions(null, ctx(), {})).toBe(true); }); test("empty object conditions are always met", async () => { - expect(await evaluateConditions({}, ctx())).toBe(true); + expect(await evaluateConditions({}, ctx(), {})).toBe(true); }); test("unknown condition key throws an error", async () => { @@ -178,13 +178,6 @@ describe("evaluateConditions", () => { ); } }); - - test("no registry provided with non-null conditions throws", async () => { - // No registry argument at all -- defaults to empty registry - expect(evaluateConditions({ some_condition: true }, ctx())).rejects.toThrow( - 'Unknown condition: "some_condition"', - ); - }); }); describe("validateConditions", () => { diff --git a/packages/authz/src/conditions.ts b/packages/authz/src/conditions.ts index e0ab0ce77..dba24970e 100644 --- a/packages/authz/src/conditions.ts +++ b/packages/authz/src/conditions.ts @@ -31,7 +31,7 @@ function lookupEvaluator( export async function evaluateConditions( conditions: Record | null, ctx: ConditionContext, - registry: ConditionRegistry = {}, + registry: ConditionRegistry, ): Promise { if (!conditions) return true; From 802fd5c227edd1bcf8b25057e352864054ea34ef Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Mon, 2 Nov 2026 23:41:41 -0700 Subject: [PATCH 5/9] Move the hub condition registry into the authorization package The packages that the hub API depends on cannot import from it, so a registry kept there was out of reach for every hub layer below the routes that must check conditions against the same evaluators. --- apps/hub/package.json | 1 + apps/hub/src/server.ts | 2 +- bun.lock | 1 + packages/authz/README.md | 2 ++ .../src/condition-registry-invariants.test.ts | 22 +++++++++++++++++++ .../src/hub-condition-registry.ts} | 5 +++-- packages/authz/src/index.ts | 1 + packages/authz/src/tool-consumer.test.ts | 10 --------- packages/hub-api/src/app.ts | 2 +- packages/hub-api/src/index.ts | 1 - 10 files changed, 32 insertions(+), 15 deletions(-) create mode 100644 packages/authz/src/condition-registry-invariants.test.ts rename packages/{hub-api/src/condition-registry.ts => authz/src/hub-condition-registry.ts} (70%) diff --git a/apps/hub/package.json b/apps/hub/package.json index af3752921..380e4e5bf 100644 --- a/apps/hub/package.json +++ b/apps/hub/package.json @@ -11,6 +11,7 @@ } }, "dependencies": { + "@intx/authz": "workspace:*", "@intx/crypto": "workspace:*", "@intx/db": "workspace:*", "@intx/hub-api": "workspace:*", diff --git a/apps/hub/src/server.ts b/apps/hub/src/server.ts index 1be6bd7c1..6a7504785 100644 --- a/apps/hub/src/server.ts +++ b/apps/hub/src/server.ts @@ -1,3 +1,4 @@ +import { HUB_CONDITION_REGISTRY } from "@intx/authz"; import { createDB, createGrantStore, @@ -22,7 +23,6 @@ import { type SidecarCapabilityRule, } from "@intx/types"; import { - HUB_CONDITION_REGISTRY, createApp, createAuth, createCorrespondentGrantMinter, diff --git a/bun.lock b/bun.lock index 8a7208c0e..5213c0794 100644 --- a/bun.lock +++ b/bun.lock @@ -61,6 +61,7 @@ "name": "@intx/hub-app", "version": "0.5.0", "dependencies": { + "@intx/authz": "workspace:*", "@intx/crypto": "workspace:*", "@intx/db": "workspace:*", "@intx/hub-api": "workspace:*", diff --git a/packages/authz/README.md b/packages/authz/README.md index 88bef6f02..465862283 100644 --- a/packages/authz/README.md +++ b/packages/authz/README.md @@ -77,6 +77,8 @@ evaluation is fail-closed. - `toolConsumerEvaluator` — a `ConditionEvaluator` for credential consumers. Register it under the `tool` key to narrow a grant to one consumer identity. +- `HUB_CONDITION_REGISTRY` — the hub's standard `ConditionRegistry`, + registering `time_window` and `tool`. - `evaluateConditions` — run a grant's `conditions` object against a `ConditionRegistry`. - `validateConditions` — check a grant's `conditions` object against a diff --git a/packages/authz/src/condition-registry-invariants.test.ts b/packages/authz/src/condition-registry-invariants.test.ts new file mode 100644 index 000000000..4c167806d --- /dev/null +++ b/packages/authz/src/condition-registry-invariants.test.ts @@ -0,0 +1,22 @@ +import { describe, test, expect } from "bun:test"; + +import { HUB_CONDITION_REGISTRY } from "./hub-condition-registry"; +import { CREDENTIAL_USE_CONDITIONS } from "./tool-consumer"; +import type { ConditionRegistry } from "./types"; + +const REGISTRIES: [string, Readonly][] = [ + ["HUB_CONDITION_REGISTRY", HUB_CONDITION_REGISTRY], + ["CREDENTIAL_USE_CONDITIONS", CREDENTIAL_USE_CONDITIONS], +]; + +describe("condition registries", () => { + // validateConditions passes an evaluator's message through without adding + // the key, so each evaluator must name itself by its registry key. + for (const [name, registry] of REGISTRIES) { + test(`${name}: each evaluator names itself by its registry key`, () => { + for (const [key, evaluator] of Object.entries(registry)) { + expect(evaluator.validate(undefined)).toStartWith(`${key}: `); + } + }); + } +}); diff --git a/packages/hub-api/src/condition-registry.ts b/packages/authz/src/hub-condition-registry.ts similarity index 70% rename from packages/hub-api/src/condition-registry.ts rename to packages/authz/src/hub-condition-registry.ts index b1dc2360f..fd7a2f0b9 100644 --- a/packages/hub-api/src/condition-registry.ts +++ b/packages/authz/src/hub-condition-registry.ts @@ -1,5 +1,6 @@ -import { timeWindowEvaluator, toolConsumerEvaluator } from "@intx/authz"; -import type { ConditionRegistry } from "@intx/types/authz"; +import { timeWindowEvaluator } from "./time-window"; +import { toolConsumerEvaluator } from "./tool-consumer"; +import type { ConditionRegistry } from "./types"; /** * The hub's standard condition registry: every condition evaluator a hub grant diff --git a/packages/authz/src/index.ts b/packages/authz/src/index.ts index 9b54f228b..5aba48e8f 100644 --- a/packages/authz/src/index.ts +++ b/packages/authz/src/index.ts @@ -22,6 +22,7 @@ export { evaluateMailAdmission } from "./mail-admission"; export type { MailAdmissionResult } from "./mail-admission"; export { createConditionEvaluator } from "./condition-evaluator"; export { evaluateConditions, validateConditions } from "./conditions"; +export { HUB_CONDITION_REGISTRY } from "./hub-condition-registry"; export { timeWindowEvaluator } from "./time-window"; export { actionConsumer, diff --git a/packages/authz/src/tool-consumer.test.ts b/packages/authz/src/tool-consumer.test.ts index 57aa11926..53773ee8f 100644 --- a/packages/authz/src/tool-consumer.test.ts +++ b/packages/authz/src/tool-consumer.test.ts @@ -115,16 +115,6 @@ describe("consumer identities", () => { }); }); -describe("CREDENTIAL_USE_CONDITIONS", () => { - // validateConditions passes an evaluator's message through without adding - // the key, so each evaluator must name itself by its registry key. - test("each evaluator names itself by its registry key", () => { - for (const [key, evaluator] of Object.entries(CREDENTIAL_USE_CONDITIONS)) { - expect(evaluator.validate(undefined)).toStartWith(`${key}: `); - } - }); -}); - describe("CREDENTIAL_USE_CONDITIONS through evaluateGrants", () => { const opts = (consumer?: string) => ({ conditions: CREDENTIAL_USE_CONDITIONS, diff --git a/packages/hub-api/src/app.ts b/packages/hub-api/src/app.ts index 3c04563ba..c6d103c03 100644 --- a/packages/hub-api/src/app.ts +++ b/packages/hub-api/src/app.ts @@ -2,6 +2,7 @@ import type { Handler, MiddlewareHandler } from "hono"; import { Hono } from "hono"; import { openAPIRouteHandler } from "hono-openapi"; +import { HUB_CONDITION_REGISTRY } from "@intx/authz"; import { honoLogger, type HonoContext } from "@intx/log/hono"; import { createNoopCredentialCipher } from "@intx/crypto"; import { getLogger } from "@intx/log"; @@ -21,7 +22,6 @@ import { import type { ConditionRegistry, GrantStore } from "@intx/types/authz"; import type { AppEnv } from "./context"; -import { HUB_CONDITION_REGISTRY } from "./condition-registry"; import { createSessionMiddleware } from "./middleware/session"; import { createAuthorizeGrant, diff --git a/packages/hub-api/src/index.ts b/packages/hub-api/src/index.ts index 0c994594f..98ab36008 100644 --- a/packages/hub-api/src/index.ts +++ b/packages/hub-api/src/index.ts @@ -25,7 +25,6 @@ export { resolveWorkflowPrincipalNames, resolveWorkflowPrincipalLabels, } from "./routes/workflow-principal-name"; -export { HUB_CONDITION_REGISTRY } from "./condition-registry"; export { collectDeployerInvokerGrants, createMailTriggeredRunGrantsMaterializer, From fd26daf903aeccf39ad6d3691e5b968b70e2a9f7 Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Tue, 3 Nov 2026 01:12:13 -0700 Subject: [PATCH 6/9] Move the run child condition registry into the authorization package A run's declared grant requirements are evaluated in the run child as well as on the hub, so the hub's probe gate must check them against the child's registry, and it cannot reach one kept in the child. --- packages/authz/README.md | 3 +++ .../authz/src/condition-registry-invariants.test.ts | 12 ++++++++++++ packages/authz/src/index.ts | 1 + packages/authz/src/run-child-condition-registry.ts | 13 +++++++++++++ .../src/child/credentials-backed-authorize.ts | 10 ++-------- 5 files changed, 31 insertions(+), 8 deletions(-) create mode 100644 packages/authz/src/run-child-condition-registry.ts diff --git a/packages/authz/README.md b/packages/authz/README.md index 465862283..5143a16ea 100644 --- a/packages/authz/README.md +++ b/packages/authz/README.md @@ -79,6 +79,9 @@ evaluation is fail-closed. consumer identity. - `HUB_CONDITION_REGISTRY` — the hub's standard `ConditionRegistry`, registering `time_window` and `tool`. +- `RUN_CHILD_CONDITION_REGISTRY` — the `ConditionRegistry` a workflow + run's child process evaluates step grants with: `time_window` only, + a subset of the hub registry. - `evaluateConditions` — run a grant's `conditions` object against a `ConditionRegistry`. - `validateConditions` — check a grant's `conditions` object against a diff --git a/packages/authz/src/condition-registry-invariants.test.ts b/packages/authz/src/condition-registry-invariants.test.ts index 4c167806d..ce113cfe6 100644 --- a/packages/authz/src/condition-registry-invariants.test.ts +++ b/packages/authz/src/condition-registry-invariants.test.ts @@ -1,12 +1,14 @@ import { describe, test, expect } from "bun:test"; import { HUB_CONDITION_REGISTRY } from "./hub-condition-registry"; +import { RUN_CHILD_CONDITION_REGISTRY } from "./run-child-condition-registry"; import { CREDENTIAL_USE_CONDITIONS } from "./tool-consumer"; import type { ConditionRegistry } from "./types"; const REGISTRIES: [string, Readonly][] = [ ["HUB_CONDITION_REGISTRY", HUB_CONDITION_REGISTRY], ["CREDENTIAL_USE_CONDITIONS", CREDENTIAL_USE_CONDITIONS], + ["RUN_CHILD_CONDITION_REGISTRY", RUN_CHILD_CONDITION_REGISTRY], ]; describe("condition registries", () => { @@ -19,4 +21,14 @@ describe("condition registries", () => { } }); } + + // A run's grant rows are evaluated both on the hub and in the run child, so + // a condition the child accepts must mean the same thing on the hub. + test("the run child registry is a subset of the hub registry", () => { + for (const [key, evaluator] of Object.entries( + RUN_CHILD_CONDITION_REGISTRY, + )) { + expect(HUB_CONDITION_REGISTRY[key]).toBe(evaluator); + } + }); }); diff --git a/packages/authz/src/index.ts b/packages/authz/src/index.ts index 5aba48e8f..8b9f4c815 100644 --- a/packages/authz/src/index.ts +++ b/packages/authz/src/index.ts @@ -23,6 +23,7 @@ export type { MailAdmissionResult } from "./mail-admission"; export { createConditionEvaluator } from "./condition-evaluator"; export { evaluateConditions, validateConditions } from "./conditions"; export { HUB_CONDITION_REGISTRY } from "./hub-condition-registry"; +export { RUN_CHILD_CONDITION_REGISTRY } from "./run-child-condition-registry"; export { timeWindowEvaluator } from "./time-window"; export { actionConsumer, diff --git a/packages/authz/src/run-child-condition-registry.ts b/packages/authz/src/run-child-condition-registry.ts new file mode 100644 index 000000000..70289539e --- /dev/null +++ b/packages/authz/src/run-child-condition-registry.ts @@ -0,0 +1,13 @@ +import { timeWindowEvaluator } from "./time-window"; +import type { ConditionRegistry } from "./types"; + +/** + * The conditions a workflow run's child process can evaluate without a + * request: only the clock. The child evaluates its step grants with this + * registry, and any other condition key throws there, so the authorization + * fails closed. + */ +export const RUN_CHILD_CONDITION_REGISTRY: Readonly = + Object.freeze({ + time_window: timeWindowEvaluator, + }); diff --git a/packages/workflow-host/src/child/credentials-backed-authorize.ts b/packages/workflow-host/src/child/credentials-backed-authorize.ts index 83aa35cf7..036720983 100644 --- a/packages/workflow-host/src/child/credentials-backed-authorize.ts +++ b/packages/workflow-host/src/child/credentials-backed-authorize.ts @@ -8,8 +8,7 @@ import { evaluateGrants, - timeWindowEvaluator, - type ConditionRegistry, + RUN_CHILD_CONDITION_REGISTRY, type GrantRule, } from "@intx/authz"; import type { AuthzCallResult } from "@intx/inference"; @@ -215,11 +214,6 @@ export function createCredentialsBackedAuthorize( }; } -// The conditions the child can evaluate without a request: only the clock. -const CHILD_CONDITIONS: ConditionRegistry = { - time_window: timeWindowEvaluator, -}; - /** * The grant evaluator a run child's authorize closure delegates to. It ranks * the step's snapshot grants together with the step's pinned-tool floor @@ -252,7 +246,7 @@ export function createGrantEvaluator(opts: { [...grants, ...floor], resource, action, - { conditions: CHILD_CONDITIONS }, + { conditions: RUN_CHILD_CONDITION_REGISTRY }, ); return { effect: result.effect, From 5deeacfeaa6488cedba98f139c8e25c2c4606ef3 Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Tue, 3 Nov 2026 02:13:59 -0700 Subject: [PATCH 7/9] Add a check of grant conditions against every evaluating registry A grant on a workflow run's principal is evaluated by the run child and the credential gate as well as the hub, each with a narrower registry, so a check against the hub registry alone accepts rows that throw wherever the run evaluates them. --- packages/authz/README.md | 8 +- packages/authz/src/grant-conditions.test.ts | 90 +++++++++++++++++++++ packages/authz/src/grant-conditions.ts | 80 ++++++++++++++++++ packages/authz/src/index.ts | 2 + 4 files changed, 178 insertions(+), 2 deletions(-) create mode 100644 packages/authz/src/grant-conditions.test.ts create mode 100644 packages/authz/src/grant-conditions.ts diff --git a/packages/authz/README.md b/packages/authz/README.md index 5143a16ea..96e8d61ed 100644 --- a/packages/authz/README.md +++ b/packages/authz/README.md @@ -88,11 +88,15 @@ evaluation is fail-closed. `ConditionRegistry` without evaluating it, returning one problem per unregistered key or rejected body. A boundary that stores conditions calls it to refuse what `evaluateConditions` would later throw on. +- `validateGrantConditions` — check a grant's conditions against every + registry that will evaluate it: the hub registry, and for a row on a + workflow run's principal also the run child registry and the + credential-use registry, where the row's patterns reach them. - `createInMemoryGrantStore` — a `GrantStore` backed by an in-memory grant list for tests and single-process deployments. - `matchPattern`, `patternSpecificity`, `grantSpecificity` — the colon-segmented wildcard matcher and the specificity scoring used to pick the winning grant. - Types: `AuthzResult`, `ConditionContext`, `ConditionEvaluator`, - `ConditionMode`, `ConditionRegistry`, `Effect`, `GrantRule`, `GrantStore`, - `MatchedGrant`. + `ConditionMode`, `ConditionRegistry`, `Effect`, `GrantConditionTarget`, + `GrantRule`, `GrantStore`, `MatchedGrant`. diff --git a/packages/authz/src/grant-conditions.test.ts b/packages/authz/src/grant-conditions.test.ts new file mode 100644 index 000000000..66cc25492 --- /dev/null +++ b/packages/authz/src/grant-conditions.test.ts @@ -0,0 +1,90 @@ +import { describe, test, expect } from "bun:test"; + +import { validateGrantConditions } from "./grant-conditions"; +import { HUB_CONDITION_REGISTRY } from "./hub-condition-registry"; + +const WINDOW = { + time_window: { after: "09:00", before: "17:00", timezone: "UTC" }, +}; +const TOOL = { tool: "tool:pkg" }; + +function problems( + resource: string, + action: string, + conditions: Record | null, + onRunPrincipal: boolean, +) { + return validateGrantConditions( + { resource, action, conditions, onRunPrincipal }, + HUB_CONDITION_REGISTRY, + ); +} + +describe("validateGrantConditions", () => { + describe("a grant not on a run principal", () => { + test("accepts every hub condition on any resource", () => { + expect(problems("tool:search", "invoke", TOOL, false)).toEqual([]); + expect(problems("credential:crd_1", "use", WINDOW, false)).toEqual([]); + expect(problems("*", "*", { ...WINDOW, ...TOOL }, false)).toEqual([]); + }); + + test("reports what the hub registry rejects", () => { + expect(problems("tool:search", "invoke", { bogus: 1 }, false)).toEqual([ + 'Unknown condition: "bogus"', + ]); + }); + }); + + describe("a grant on a run principal", () => { + test("accepts no conditions anywhere", () => { + expect(problems("*", "*", null, true)).toEqual([]); + }); + + test("accepts a time window on a non-credential resource", () => { + expect(problems("tool:search", "invoke", WINDOW, true)).toEqual([]); + }); + + test("refuses a tool condition on a non-credential resource", () => { + expect(problems("tool:search", "invoke", TOOL, true)).toEqual([ + 'a workflow run cannot evaluate this grant: Unknown condition: "tool"', + ]); + }); + + test("accepts a tool condition on credential use", () => { + expect(problems("credential:crd_1", "use", TOOL, true)).toEqual([]); + expect(problems("credential:*", "use", TOOL, true)).toEqual([]); + }); + + test("refuses a time window on credential use", () => { + expect(problems("credential:crd_1", "use", WINDOW, true)).toEqual([ + 'credential use cannot evaluate this grant: Unknown condition: "time_window"', + ]); + }); + + test("a credential row with another action meets neither narrower check", () => { + expect(problems("credential:crd_1", "read", WINDOW, true)).toEqual([]); + expect(problems("credential:crd_1", "read", TOOL, true)).toEqual([]); + }); + + test("a wildcard resource meets both narrower checks", () => { + expect(problems("*", "*", WINDOW, true)).toEqual([ + 'credential use cannot evaluate this grant: Unknown condition: "time_window"', + ]); + expect(problems("*", "use", TOOL, true)).toEqual([ + 'a workflow run cannot evaluate this grant: Unknown condition: "tool"', + ]); + }); + + test("a prefix of credential: can match both kinds of resource", () => { + expect(problems("cred*", "*", TOOL, true)).toEqual([ + 'a workflow run cannot evaluate this grant: Unknown condition: "tool"', + ]); + }); + + test("reports a hub problem once, without a narrower copy", () => { + expect(problems("*", "*", { bogus: 1 }, true)).toEqual([ + 'Unknown condition: "bogus"', + ]); + }); + }); +}); diff --git a/packages/authz/src/grant-conditions.ts b/packages/authz/src/grant-conditions.ts new file mode 100644 index 000000000..5cb46a372 --- /dev/null +++ b/packages/authz/src/grant-conditions.ts @@ -0,0 +1,80 @@ +import { validateConditions } from "./conditions"; +import { matchPattern } from "./patterns"; +import { RUN_CHILD_CONDITION_REGISTRY } from "./run-child-condition-registry"; +import { CREDENTIAL_USE_CONDITIONS } from "./tool-consumer"; +import type { ConditionRegistry } from "./types"; + +const CREDENTIAL_PREFIX = "credential:"; + +export type GrantConditionTarget = { + readonly resource: string; + readonly action: string; + readonly conditions: Record | null; + /** + * Whether the grant sits directly on a workflow run's principal. Such a row + * ships to the run as a step grant, so the run child and the credential + * gate evaluate it as well as the hub. A role grant never ships to a run, + * because the run loads only the rows on its own principal; if that + * changes, a role grant needs this check too. + */ + readonly onRunPrincipal: boolean; +}; + +// The literal text a pattern's matches must start with: everything before +// its first `*`, or the whole pattern when it has none. +function literalPrefix(pattern: string): string { + const star = pattern.indexOf("*"); + return star === -1 ? pattern : pattern.slice(0, star); +} + +function canMatchCredential(resource: string): boolean { + const prefix = literalPrefix(resource); + if (!resource.includes("*")) return prefix.startsWith(CREDENTIAL_PREFIX); + return ( + prefix.startsWith(CREDENTIAL_PREFIX) || CREDENTIAL_PREFIX.startsWith(prefix) + ); +} + +function canMatchNonCredential(resource: string): boolean { + return !literalPrefix(resource).startsWith(CREDENTIAL_PREFIX); +} + +/** + * Check a grant's conditions against every registry that will evaluate the + * grant, not only the hub's. The hub evaluates every grant, with + * `hubRegistry`. A row on a + * workflow run's principal is also evaluated by the run child, for any + * resource its pattern can match outside `credential:`, and by the + * credential gate, for `credential:` / `use`. Each of those has a + * narrower registry, and a condition outside it throws there. + * + * Returns one problem per bad entry. A problem only a narrower registry + * reports names the place that rejects it. + */ +export function validateGrantConditions( + target: GrantConditionTarget, + hubRegistry: Readonly, +): string[] { + const problems = validateConditions(target.conditions, hubRegistry); + if (!target.onRunPrincipal) return problems; + + const narrower: [string, Readonly][] = []; + if (canMatchNonCredential(target.resource)) { + narrower.push(["a workflow run", RUN_CHILD_CONDITION_REGISTRY]); + } + if ( + canMatchCredential(target.resource) && + matchPattern(target.action, "use") + ) { + narrower.push(["credential use", CREDENTIAL_USE_CONDITIONS]); + } + + for (const [place, registry] of narrower) { + for (const problem of validateConditions(target.conditions, registry)) { + if (!problems.includes(problem)) { + problems.push(`${place} cannot evaluate this grant: ${problem}`); + } + } + } + return problems; +} diff --git a/packages/authz/src/index.ts b/packages/authz/src/index.ts index 8b9f4c815..33d892369 100644 --- a/packages/authz/src/index.ts +++ b/packages/authz/src/index.ts @@ -22,6 +22,8 @@ export { evaluateMailAdmission } from "./mail-admission"; export type { MailAdmissionResult } from "./mail-admission"; export { createConditionEvaluator } from "./condition-evaluator"; export { evaluateConditions, validateConditions } from "./conditions"; +export { validateGrantConditions } from "./grant-conditions"; +export type { GrantConditionTarget } from "./grant-conditions"; export { HUB_CONDITION_REGISTRY } from "./hub-condition-registry"; export { RUN_CHILD_CONDITION_REGISTRY } from "./run-child-condition-registry"; export { timeWindowEvaluator } from "./time-window"; From e0a60cf96f7318e8d7746b7778531985e5e21aa0 Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Tue, 3 Nov 2026 02:54:21 -0700 Subject: [PATCH 8/9] Reject grant conditions that cannot be evaluated on create and update A grant whose conditions an evaluating registry cannot evaluate was stored with a 2xx and only failed when authorization first reached it, far from the operator who wrote it. --- docs/API.md | 9 +- docs/AUTH.md | 20 +- packages/hub-api/src/app.ts | 8 +- packages/hub-api/src/routes/grants.ts | 80 +++++- packages/types/src/grants.ts | 2 +- tests/hub-api/grant-routes.test.ts | 346 +++++++++++++++++++++++++- 6 files changed, 445 insertions(+), 20 deletions(-) diff --git a/docs/API.md b/docs/API.md index 3778776bc..3abdb5c92 100644 --- a/docs/API.md +++ b/docs/API.md @@ -423,7 +423,7 @@ Creates a grant targeting either a role or a principal directly. Exactly one of Body: CreateGrant 201: GrantResponse -- Grant created -400: ErrorResponse -- Validation error +400: ErrorResponse -- Validation error. A body that fails its schema gets the body validator's error envelope. A body whose `conditions` names an unregistered condition, holds a malformed condition body, or, on a workflow run's principal, holds a condition the run cannot evaluate gets an `ErrorResponse` whose message lists each problem as a JSON array. ### GET /api/tenants/:tenantId/grants/:grantId Get grant details @@ -439,6 +439,7 @@ Update effect, conditions, or expiry on an existing grant. Body: UpdateGrant 200: GrantResponse -- Grant updated +400: ErrorResponse -- Validation error. A body that fails its schema gets the body validator's error envelope. A body whose `conditions` names an unregistered condition, holds a malformed condition body, or, on a workflow run's principal, holds a condition the run cannot evaluate gets an `ErrorResponse` whose message lists each problem as a JSON array. 404: ErrorResponse -- Grant not found ### DELETE /api/tenants/:tenantId/grants/:grantId @@ -1363,7 +1364,7 @@ Source: packages/types/src/grants.ts **effect**: Outcome when this grant is the one resolved for a request: `allow` permits the action, `deny` blocks it, `ask` requires interactive approval before proceeding. When several grants match, the most specific wins, and at equal specificity the strongest effect wins (`deny` over `ask` over `allow`). **origin**: Records where the grant came from: `system` (built-in), `role` (granted via a role), `creator` (from the workflow definition author), or `invoker` (delegated by the workflow deployment's deployer, its invoker of record). Origin is provenance only; it does not affect evaluation precedence. -**conditions**: Optional map of named conditions that must all pass for the grant to apply, evaluated against a condition registry at authorization time. A check with no request to evaluate conditions against, such as delegation at launch, treats a conditioned `deny` or `ask` as applying and a conditioned `allow` as not applying. +**conditions**: Optional map of named conditions that must all pass for the grant to apply, evaluated against a condition registry at authorization time. Grant create and update reject, with a 400, a condition that any registry evaluating the grant cannot evaluate: an unregistered name, a malformed body, or, on a workflow run's principal, a condition the run cannot evaluate. A check with no request to evaluate conditions against, such as delegation at launch, treats a conditioned `deny` or `ask` as applying and a conditioned `allow` as not applying. ### CreateModel `{ canonicalName: string, description?: string | null, displayName?: string | null }` @@ -1482,7 +1483,7 @@ Source: packages/types/src/grants.ts **effect**: Outcome when this grant is the one resolved for a request: `allow` permits the action, `deny` blocks it, `ask` requires interactive approval before proceeding. When several grants match, the most specific wins, and at equal specificity the strongest effect wins (`deny` over `ask` over `allow`). **origin**: Records where the grant came from: `system` (built-in), `role` (granted via a role), `creator` (from the workflow definition author), or `invoker` (delegated by the workflow deployment's deployer, its invoker of record). Origin is provenance only; it does not affect evaluation precedence. -**conditions**: Optional map of named conditions that must all pass for the grant to apply, evaluated against a condition registry at authorization time. A check with no request to evaluate conditions against, such as delegation at launch, treats a conditioned `deny` or `ask` as applying and a conditioned `allow` as not applying. +**conditions**: Optional map of named conditions that must all pass for the grant to apply, evaluated against a condition registry at authorization time. Grant create and update reject, with a 400, a condition that any registry evaluating the grant cannot evaluate: an unregistered name, a malformed body, or, on a workflow run's principal, a condition the run cannot evaluate. A check with no request to evaluate conditions against, such as delegation at launch, treats a conditioned `deny` or `ask` as applying and a conditioned `allow` as not applying. ### HistoryEntry `{ author: string, message: string, ref: string, timestamp: string, filesChanged?: number }` @@ -1625,7 +1626,7 @@ Source: packages/types/src/credentials.ts `{ conditions?: { [string]: unknown } | null, effect?: "allow" | "ask" | "deny", expiresAt?: string | null }` Source: packages/types/src/grants.ts -**conditions**: Optional map of named conditions that must all pass for the grant to apply, evaluated against a condition registry at authorization time. A check with no request to evaluate conditions against, such as delegation at launch, treats a conditioned `deny` or `ask` as applying and a conditioned `allow` as not applying. +**conditions**: Optional map of named conditions that must all pass for the grant to apply, evaluated against a condition registry at authorization time. Grant create and update reject, with a 400, a condition that any registry evaluating the grant cannot evaluate: an unregistered name, a malformed body, or, on a workflow run's principal, a condition the run cannot evaluate. A check with no request to evaluate conditions against, such as delegation at launch, treats a conditioned `deny` or `ask` as applying and a conditioned `allow` as not applying. **effect**: Outcome when this grant is the one resolved for a request: `allow` permits the action, `deny` blocks it, `ask` requires interactive approval before proceeding. When several grants match, the most specific wins, and at equal specificity the strongest effect wins (`deny` over `ask` over `allow`). ### UpdateModel diff --git a/docs/AUTH.md b/docs/AUTH.md index 076ab7ca0..ae67a0147 100644 --- a/docs/AUTH.md +++ b/docs/AUTH.md @@ -331,16 +331,30 @@ A standing decision changes the effect of an existing grant whose resource is th ### Conditions -The `conditions` JSONB field constrains when a grant applies. A condition key is evaluated only if an evaluator is registered for it; today the sole registered evaluator is `time_window`: +The `conditions` JSONB field constrains when a grant applies. Each key names a condition evaluator in the hub's condition registry. Two evaluators are registered: -- `{ "time_window": { "after": "09:00", "before": "17:00", "timezone": "America/Los_Angeles" } }` -- time-based access (implemented). `timezone` is required; `after` and `before` are `HH:MM` in 24-hour format. +- `{ "time_window": { "after": "09:00", "before": "17:00", "timezone": "America/Los_Angeles" } }` -- time-based access. `timezone` is required and is an IANA zone name or a UTC offset. `after` and `before` are `HH:MM` in 24-hour format and must differ; `after` later than `before` gives a window that crosses midnight. No other key is allowed. +- `{ "tool": "tool:" }` -- a consumer-scoped grant, which applies only when the named credential consumer makes the request. The value is a non-empty consumer identity. Spending limits and threshold-based escalation are planned conditions with no evaluator yet: - `{ "max_spend_per_day": 100, "currency": "USD" }` -- spending limits (not yet available). - `{ "require_approval_above": 50 }` -- threshold-based escalation (not yet available). -Conditions are evaluated at runtime by the authorization engine: a grant whose registered condition is unmet is skipped, but a grant carrying a condition key with no registered evaluator throws and fails the authorization to surface the misconfiguration. Do not author grants with the planned conditions above until their evaluators ship — a grant referencing an unregistered condition will fail authorization with an error rather than silently passing. +Every grant is evaluated on the hub with this registry. A grant that sits directly on a workflow run's principal is also evaluated where the run is, with narrower registries: + +| Where | Evaluates | Registry | +| ------------------- | ----------------------------------------------------------------------------------------------- | ------------------ | +| The run child | a row whose resource pattern can match a resource outside `credential:` | `time_window` only | +| The credential gate | a row whose resource pattern can match `credential:` and whose action pattern matches `use` | `tool` only | + +A role grant never reaches a run, because a run loads only the rows on its own principal. So a run-principal row may carry `time_window` on a non-credential resource and `tool` on credential use. A row whose patterns can reach both, such as resource `*` with an action pattern that matches `use`, may carry no condition, because neither key is evaluable in both places. + +The hub checks conditions against every registry that will evaluate the row before it stores the row: + +- Grant create (`POST`) and update (`PATCH`) check `conditions` for the grant's target, resource, and action. An unregistered key, such as the planned conditions above, a body that does not match its evaluator's shape, or a key that a run cannot evaluate on a run-principal row, gives a 400 that names each problem, and the grant is not stored. A `PATCH` that omits `conditions` does not check or change the stored value. + +The authorization engine evaluates conditions at runtime: a grant whose registered condition is unmet is skipped. A stored grant that carries an unregistered key or a malformed body still throws and fails the authorization, to surface the misconfiguration. Such a row comes from a path that does not check conditions: a grant materialized from a workflow's declared grant requirement (at launch, or when a change to the delegator's grants re-derives the run), a direct database write, or a row stored before the check existed. Every check names how it treats a conditioned grant. A check on a request evaluates each condition with the hub's condition registry. A check with no request to evaluate a condition against is **pessimistic**: a conditioned `deny` or `ask` applies as if its condition held, and a conditioned `allow` does not apply. Delegation is the pessimistic check: diff --git a/packages/hub-api/src/app.ts b/packages/hub-api/src/app.ts index c6d103c03..57631e7aa 100644 --- a/packages/hub-api/src/app.ts +++ b/packages/hub-api/src/app.ts @@ -352,7 +352,13 @@ export function mountHubRoutes( ); app.route( "/api/tenants/:tenantId/grants", - createGrantRoutes({ db, requireGrant, grantStoreFor, sidecarRouter }), + createGrantRoutes({ + db, + requireGrant, + grantStoreFor, + sidecarRouter, + conditionRegistry, + }), ); app.route( "/api/tenants/:tenantId/principals/:principalId/evaluate", diff --git a/packages/hub-api/src/routes/grants.ts b/packages/hub-api/src/routes/grants.ts index bbbd9e7de..c90b363b8 100644 --- a/packages/hub-api/src/routes/grants.ts +++ b/packages/hub-api/src/routes/grants.ts @@ -2,8 +2,8 @@ import { eq, and } from "drizzle-orm"; import { Hono } from "hono"; import { describeRoute, validator } from "hono-openapi"; -import { authorize } from "@intx/authz"; -import { grant, principal } from "@intx/db/schema"; +import { authorize, validateGrantConditions } from "@intx/authz"; +import { grant, principal, workflowRun } from "@intx/db/schema"; import { parseGrantRow } from "@intx/db"; import type { DB, DBExecutor, GrantStoreFactory } from "@intx/db"; import type { SidecarRouter } from "@intx/hub-sessions"; @@ -132,11 +132,41 @@ async function resolveGrantNames( return { roleNames, principalNames }; } +// One evaluator's problem can itself join several field errors, and problem +// text can quote author values, so the list is JSON-encoded to keep each +// problem's bounds unambiguous. +function conditionsMessage(problems: readonly string[]): string { + return `invalid conditions: ${JSON.stringify(problems)}`; +} + +const CONDITIONS_400_DESCRIPTION = + "Validation error. A body that fails its schema gets the body validator's error envelope. A body whose `conditions` names an unregistered condition, holds a malformed condition body, or, on a workflow run's principal, holds a condition the run cannot evaluate gets an `ErrorResponse` whose message lists each problem as a JSON array."; + +// A grant on a workflow run's principal ships to the run, which evaluates it +// with narrower registries than the hub's. +async function isRunPrincipal( + db: DB["db"], + tenantId: string, + principalId: string, +): Promise { + const run = await db.query.workflowRun.findFirst({ + columns: { id: true }, + where: and( + eq(workflowRun.tenantId, tenantId), + eq(workflowRun.principalId, principalId), + ), + }); + return run !== undefined; +} + export type CreateGrantRoutesDeps = { db: DB["db"]; requireGrant: RequireGrant; grantStoreFor: GrantStoreFactory; sidecarRouter: Pick; + // The registry a stored grant's conditions are evaluated against. Create + // and update refuse conditions it cannot evaluate. + conditionRegistry: ConditionRegistry; }; // The principals whose effective grants a grant row is part of: its own @@ -157,6 +187,7 @@ export function createGrantRoutes({ requireGrant, grantStoreFor, sidecarRouter, + conditionRegistry, }: CreateGrantRoutesDeps): Hono { const app = new Hono(); const delegatorDeps = { db, grantStoreFor, sidecarRouter }; @@ -234,7 +265,7 @@ export function createGrantRoutes({ "Creates a grant targeting either a role or a principal directly. Exactly one of roleId or principalId must be provided.", responses: { 201: jsonResponse("Grant created", GrantResponse), - 400: jsonResponse("Validation error", ErrorResponse), + 400: jsonResponse(CONDITIONS_400_DESCRIPTION, ErrorResponse), }, }), validator("json", CreateGrant), @@ -245,6 +276,21 @@ export function createGrantRoutes({ // request is a 400 before this handler runs. const body = c.req.valid("json"); + const problems = validateGrantConditions( + { + resource: body.resource, + action: body.action, + conditions: body.conditions ?? null, + onRunPrincipal: + body.principalId != null && + (await isRunPrincipal(db, tenantCtx.id, body.principalId)), + }, + conditionRegistry, + ); + if (problems.length > 0) { + return errorResponse(c, "bad_request", conditionsMessage(problems)); + } + const now = new Date(); const row = await changeDelegatorGrants( delegatorDeps, @@ -316,6 +362,7 @@ export function createGrantRoutes({ description: "Update effect, conditions, or expiry on an existing grant.", responses: { 200: jsonResponse("Grant updated", GrantResponse), + 400: jsonResponse(CONDITIONS_400_DESCRIPTION, ErrorResponse), 404: jsonResponse("Grant not found", ErrorResponse), }, }), @@ -325,6 +372,33 @@ export function createGrantRoutes({ const grantId = c.req.param("grantId"); const body = c.req.valid("json"); + // An omitted `conditions` leaves the stored value unchecked and + // unchanged; only a body that sets it is checked. + if (body.conditions !== undefined) { + // The update cannot change a grant's resource, action, or target, so + // the stored row supplies them. + const existing = await db.query.grant.findFirst({ + where: and(eq(grant.id, grantId), eq(grant.tenantId, tenantCtx.id)), + }); + if (!existing) { + return errorResponse(c, "not_found", "Grant not found"); + } + const problems = validateGrantConditions( + { + resource: existing.resource, + action: existing.action, + conditions: body.conditions, + onRunPrincipal: + existing.principalId !== null && + (await isRunPrincipal(db, tenantCtx.id, existing.principalId)), + }, + conditionRegistry, + ); + if (problems.length > 0) { + return errorResponse(c, "bad_request", conditionsMessage(problems)); + } + } + const updates: Record = { updatedAt: new Date() }; if (body.effect !== undefined) updates["effect"] = body.effect; if (body.conditions !== undefined) diff --git a/packages/types/src/grants.ts b/packages/types/src/grants.ts index 8dd2efce5..64d12fdc1 100644 --- a/packages/types/src/grants.ts +++ b/packages/types/src/grants.ts @@ -20,7 +20,7 @@ const originDescription = "Records where the grant came from: `system` (built-in), `role` (granted via a role), `creator` (from the workflow definition author), or `invoker` (delegated by the workflow deployment's deployer, its invoker of record). Origin is provenance only; it does not affect evaluation precedence."; const conditionsDescription = - "Optional map of named conditions that must all pass for the grant to apply, evaluated against a condition registry at authorization time. A check with no request to evaluate conditions against, such as delegation at launch, treats a conditioned `deny` or `ask` as applying and a conditioned `allow` as not applying."; + "Optional map of named conditions that must all pass for the grant to apply, evaluated against a condition registry at authorization time. Grant create and update reject, with a 400, a condition that any registry evaluating the grant cannot evaluate: an unregistered name, a malformed body, or, on a workflow run's principal, a condition the run cannot evaluate. A check with no request to evaluate conditions against, such as delegation at launch, treats a conditioned `deny` or `ask` as applying and a conditioned `allow` as not applying."; const specificityDescription = "Computed match-strength score used to rank grants: the count of non-wildcard characters in the resource and action patterns, with exact (wildcard-free) patterns scored far above prefix globs. Higher wins; ties are broken by effect priority."; diff --git a/tests/hub-api/grant-routes.test.ts b/tests/hub-api/grant-routes.test.ts index 7a4c97565..2305612f9 100644 --- a/tests/hub-api/grant-routes.test.ts +++ b/tests/hub-api/grant-routes.test.ts @@ -20,14 +20,19 @@ import { harnessDbEnvAvailable, type TestDb, } from "@intx/test-harness/db-harness"; +import { seedPrincipal, seedWorkflowRun } from "@intx/test-harness/seed"; -// Pins the exactly-one-target invariant at the POST /api/grants route layer. -// CreateGrant carries a `.narrow` (unit-tested in packages/types) requiring a -// grant to target exactly one of a role or a principal, mirroring the -// grant_target_exactly_one DB CHECK. This proves the validator is actually -// MOUNTED on the route: a both/neither body is a 400 here, not a database 500 -// from tripping the CHECK past an unguarded insert. Nothing else in the suite -// pins that the validator converts a malformed body into a 400 on this route. +// Pins request-body validation at the grant route layer, against a real DB. +// +// Exactly one target: CreateGrant carries a `.narrow` (unit-tested in +// packages/types) requiring a grant to target exactly one of a role or a +// principal, mirroring the grant_target_exactly_one DB CHECK. This proves the +// validator is actually MOUNTED on the route: a both/neither body is a 400 +// here, not a database 500 from tripping the CHECK past an unguarded insert. +// +// Conditions: create and update check `conditions` against the hub condition +// registry (unit-tested in packages/authz), so a grant that authorization +// could not evaluate is a 400 naming the problem, and is never stored. function mockGetSession(userId: string): GetSession { const now = new Date("2026-01-01"); @@ -74,7 +79,7 @@ function mockSessionService(): SessionService { } describe.skipIf(!harnessDbEnvAvailable())( - "POST /api/grants enforces exactly one target (real DB)", + "grant routes validate the request body (real DB)", () => { let h: TestDb; @@ -156,6 +161,31 @@ describe.skipIf(!harnessDbEnvAvailable())( }); } + function patchGrant( + app: ReturnType, + tenantId: string, + grantId: string, + body: Record, + ) { + return app.request(`/api/tenants/${tenantId}/grants/${grantId}`, { + method: "PATCH", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); + } + + const ErrorBody = type({ error: { code: "string", message: "string" } }); + + async function storedConditions(grantId: string) { + const row = await h.db.query.grant.findFirst({ + where: (g, { eq }) => eq(g.id, grantId), + }); + if (row === undefined) throw new Error(`grant ${grantId} not stored`); + return row.conditions; + } + + const WINDOW = { after: "09:00", before: "17:00", timezone: "UTC" }; + test("rejects a body with both targets as a 400", async () => { const app = makeApp(); const { tenantId, roleId } = await bootstrapTenant(app, "grant-both"); @@ -226,5 +256,305 @@ describe.skipIf(!harnessDbEnvAvailable())( action: "read", }); }); + describe("conditions", () => { + async function setup(slug: string) { + const app = makeApp(); + const { tenantId, roleId } = await bootstrapTenant(app, slug); + const grantBody = (conditions: unknown) => ({ + roleId, + resource: "agent:*", + action: "read", + effect: "allow", + origin: "creator", + conditions, + }); + return { app, tenantId, grantBody }; + } + + async function createWindowGrant(slug: string) { + const ctx = await setup(slug); + const res = await postGrant( + ctx.app, + ctx.tenantId, + ctx.grantBody({ time_window: WINDOW }), + ); + expect(res.status).toBe(201); + const { id } = type({ id: "string" }).assert(await res.json()); + return { ...ctx, grantId: id }; + } + + async function grantCount() { + return (await h.db.query.grant.findMany()).length; + } + + test("POST rejects a time_window missing its timezone", async () => { + const { app, tenantId, grantBody } = await setup("cond-no-tz"); + const before = await grantCount(); + + const res = await postGrant( + app, + tenantId, + grantBody({ time_window: { after: "09:00", before: "17:00" } }), + ); + + expect(res.status).toBe(400); + expect(ErrorBody.assert(await res.json()).error).toEqual({ + code: "bad_request", + message: + 'invalid conditions: ["time_window: timezone must be a string (was missing)"]', + }); + expect(await grantCount()).toBe(before); + }); + + test("POST rejects a time_window with the wrong keys", async () => { + const { app, tenantId, grantBody } = await setup("cond-keys"); + + const res = await postGrant( + app, + tenantId, + grantBody({ time_window: { start: "09:00", end: "17:00" } }), + ); + + expect(res.status).toBe(400); + const { message } = ErrorBody.assert(await res.json()).error; + expect(message).toContain("after must be a string (was missing)"); + expect(message).toContain("start must be removed"); + }); + + test("POST rejects an unregistered condition", async () => { + const { app, tenantId, grantBody } = await setup("cond-unknown"); + + const res = await postGrant( + app, + tenantId, + grantBody({ max_spend_per_day: 100 }), + ); + + expect(res.status).toBe(400); + expect(ErrorBody.assert(await res.json()).error.message).toBe( + 'invalid conditions: ["Unknown condition: \\"max_spend_per_day\\""]', + ); + }); + + test("POST stores a valid time_window", async () => { + const { grantId } = await createWindowGrant("cond-valid"); + expect(await storedConditions(grantId)).toEqual({ + time_window: WINDOW, + }); + }); + + test("PATCH rejects malformed conditions and keeps the stored ones", async () => { + const { app, tenantId, grantId } = + await createWindowGrant("cond-patch-bad"); + + const res = await patchGrant(app, tenantId, grantId, { + conditions: { time_window: { ...WINDOW, timezone: "Mars/Olympus" } }, + }); + + expect(res.status).toBe(400); + expect(ErrorBody.assert(await res.json()).error.message).toContain( + "time_window: timezone must be a time zone name or UTC offset", + ); + expect(await storedConditions(grantId)).toEqual({ + time_window: WINDOW, + }); + }); + + test("PATCH stores valid conditions", async () => { + const { app, tenantId, grantId } = + await createWindowGrant("cond-patch-ok"); + const next = { ...WINDOW, after: "08:00" }; + + const res = await patchGrant(app, tenantId, grantId, { + conditions: { time_window: next }, + }); + + expect(res.status).toBe(200); + expect(await storedConditions(grantId)).toEqual({ time_window: next }); + }); + + test("PATCH with null conditions clears them", async () => { + const { app, tenantId, grantId } = + await createWindowGrant("cond-patch-null"); + + const res = await patchGrant(app, tenantId, grantId, { + conditions: null, + }); + + expect(res.status).toBe(200); + expect(await storedConditions(grantId)).toBeNull(); + }); + + describe("on a principal without a run", () => { + // A `tool` condition on a non-credential resource is evaluable on the + // hub, and only a run's own principal also faces the run's narrower + // registries. These targets take the hub check alone. + async function postToolGrant(slug: string, kind: "user" | "workflow") { + const app = makeApp(); + const { tenantId } = await bootstrapTenant(app, slug); + const principalId = `prn_${slug}`; + await seedPrincipal(h.db, { + id: principalId, + tenantId, + kind, + refId: `ref_${slug}`, + }); + return postGrant(app, tenantId, { + principalId, + resource: "tool:search", + action: "invoke", + effect: "allow", + origin: "creator", + conditions: { tool: "tool:pkg" }, + }); + } + + test("POST accepts a tool condition on a user principal", async () => { + const res = await postToolGrant("user-tool", "user"); + expect(res.status).toBe(201); + }); + + test("POST accepts a tool condition on a workflow principal with no run", async () => { + // A workflow principal can name a definition rather than a run. + // The run check keys on a run that names the principal, not on + // the principal's kind. + const res = await postToolGrant("definition-tool", "workflow"); + expect(res.status).toBe(201); + }); + }); + + describe("on a workflow run's principal", () => { + const TOOL = { tool: "tool:pkg" }; + + async function setupRun(slug: string) { + const app = makeApp(); + const { tenantId, roleId } = await bootstrapTenant(app, slug); + const principalId = `prn_run_${slug}`; + await seedPrincipal(h.db, { + id: principalId, + tenantId, + kind: "workflow", + refId: `run_${slug}`, + }); + await seedWorkflowRun(h.db, { + id: `run_${slug}`, + tenantId, + principalId, + }); + const runGrant = ( + resource: string, + action: string, + conditions: unknown, + ) => ({ + principalId, + resource, + action, + effect: "allow", + origin: "creator", + conditions, + }); + return { app, tenantId, roleId, runGrant }; + } + + test("POST refuses a tool condition on a non-credential resource", async () => { + const { app, tenantId, runGrant } = await setupRun("run-tool"); + + const res = await postGrant( + app, + tenantId, + runGrant("tool:search", "invoke", TOOL), + ); + + expect(res.status).toBe(400); + expect(ErrorBody.assert(await res.json()).error.message).toContain( + "a workflow run cannot evaluate this grant", + ); + }); + + test("POST accepts a tool condition on credential use", async () => { + const { app, tenantId, runGrant } = await setupRun("run-cred"); + + const res = await postGrant( + app, + tenantId, + runGrant("credential:crd_1", "use", TOOL), + ); + + expect(res.status).toBe(201); + }); + + test("POST refuses a time window on credential use", async () => { + const { app, tenantId, runGrant } = await setupRun("run-cred-tw"); + + const res = await postGrant( + app, + tenantId, + runGrant("credential:crd_1", "use", { time_window: WINDOW }), + ); + + expect(res.status).toBe(400); + expect(ErrorBody.assert(await res.json()).error.message).toContain( + "credential use cannot evaluate this grant", + ); + }); + + test("POST accepts a tool condition on a role grant", async () => { + const { app, tenantId, roleId } = await setupRun("run-role"); + + const res = await postGrant(app, tenantId, { + roleId, + resource: "tool:search", + action: "invoke", + effect: "allow", + origin: "creator", + conditions: TOOL, + }); + + expect(res.status).toBe(201); + }); + + test("PATCH refuses a tool condition and keeps the stored one", async () => { + const { app, tenantId, runGrant } = await setupRun("run-patch"); + const created = await postGrant( + app, + tenantId, + runGrant("tool:search", "invoke", { time_window: WINDOW }), + ); + expect(created.status).toBe(201); + const { id } = type({ id: "string" }).assert(await created.json()); + + const res = await patchGrant(app, tenantId, id, { + conditions: TOOL, + }); + + expect(res.status).toBe(400); + expect(await storedConditions(id)).toEqual({ time_window: WINDOW }); + }); + + test("PATCH of a missing grant's conditions is a 404", async () => { + const { app, tenantId } = await setupRun("run-missing"); + + const res = await patchGrant(app, tenantId, "grt_missing", { + conditions: TOOL, + }); + + expect(res.status).toBe(404); + }); + }); + + test("PATCH without conditions leaves them unchanged", async () => { + const { app, tenantId, grantId } = + await createWindowGrant("cond-patch-omit"); + + const res = await patchGrant(app, tenantId, grantId, { + effect: "deny", + }); + + expect(res.status).toBe(200); + expect(await storedConditions(grantId)).toEqual({ + time_window: WINDOW, + }); + }); + }); }, ); From 522d949bede25b74822c6452bc79a6a4e8f45997 Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Tue, 3 Nov 2026 03:01:41 -0700 Subject: [PATCH 9/9] Refuse declared grant requirements whose conditions cannot be evaluated A requirement's conditions are copied verbatim into the grant rows a run mints, so a condition one of the run's evaluators cannot evaluate froze cleanly and then failed every authorization that reached the row, far from the author who declared it. --- bun.lock | 1 + docs/AUTH.md | 5 +- packages/hub-sessions/package.json | 1 + .../src/workflow-probe-gate.grants.test.ts | 136 ++++++++++++++++++ .../src/workflow-probe-gate.refusal.test.ts | 30 ++++ .../hub-sessions/src/workflow-probe-gate.ts | 68 ++++++++- tests/db/run-delegated-grants.test.ts | 3 + 7 files changed, 236 insertions(+), 8 deletions(-) diff --git a/bun.lock b/bun.lock index 5213c0794..874040e93 100644 --- a/bun.lock +++ b/bun.lock @@ -398,6 +398,7 @@ "version": "0.5.0", "dependencies": { "@intx/agent": "workspace:*", + "@intx/authz": "workspace:*", "@intx/crypto": "workspace:*", "@intx/db": "workspace:*", "@intx/hub-common": "workspace:*", diff --git a/docs/AUTH.md b/docs/AUTH.md index ae67a0147..083ad6beb 100644 --- a/docs/AUTH.md +++ b/docs/AUTH.md @@ -144,7 +144,7 @@ Each requirement specifies: action: string -- "invoke", "spend", "read", etc. source: "creator" | "invoker" effect?: "allow" | "ask" | "deny" -- default: "allow" - conditions?: object -- optional constraints + conditions?: object -- optional constraints; only conditions a run can evaluate (see Conditions) } ``` @@ -352,9 +352,10 @@ A role grant never reaches a run, because a run loads only the rows on its own p The hub checks conditions against every registry that will evaluate the row before it stores the row: +- The probe gate (`gateAndFreezeProbeResult`) checks a workflow definition's declared grant requirements, which become rows on the run principal, at install and approve. It refuses the definition with the reason `grant_requirement_conditions_invalid`, before the operator approval checks and before anything is frozen. - Grant create (`POST`) and update (`PATCH`) check `conditions` for the grant's target, resource, and action. An unregistered key, such as the planned conditions above, a body that does not match its evaluator's shape, or a key that a run cannot evaluate on a run-principal row, gives a 400 that names each problem, and the grant is not stored. A `PATCH` that omits `conditions` does not check or change the stored value. -The authorization engine evaluates conditions at runtime: a grant whose registered condition is unmet is skipped. A stored grant that carries an unregistered key or a malformed body still throws and fails the authorization, to surface the misconfiguration. Such a row comes from a path that does not check conditions: a grant materialized from a workflow's declared grant requirement (at launch, or when a change to the delegator's grants re-derives the run), a direct database write, or a row stored before the check existed. +The authorization engine evaluates conditions at runtime: a grant whose registered condition is unmet is skipped. A stored grant that carries an unregistered key or a malformed body still throws and fails the authorization, to surface the misconfiguration. Such a row comes from a path that does not check conditions: a grant materialized (at launch, or when a change to the delegator's grants re-derives the run) from a declared grant requirement that was frozen before the probe gate checked conditions, a direct database write, or a row stored before the check existed. Every check names how it treats a conditioned grant. A check on a request evaluates each condition with the hub's condition registry. A check with no request to evaluate a condition against is **pessimistic**: a conditioned `deny` or `ask` applies as if its condition held, and a conditioned `allow` does not apply. Delegation is the pessimistic check: diff --git a/packages/hub-sessions/package.json b/packages/hub-sessions/package.json index 42b873b0e..d8130b03e 100644 --- a/packages/hub-sessions/package.json +++ b/packages/hub-sessions/package.json @@ -18,6 +18,7 @@ }, "dependencies": { "@intx/agent": "workspace:*", + "@intx/authz": "workspace:*", "@intx/crypto": "workspace:*", "@intx/db": "workspace:*", "@intx/hub-common": "workspace:*", diff --git a/packages/hub-sessions/src/workflow-probe-gate.grants.test.ts b/packages/hub-sessions/src/workflow-probe-gate.grants.test.ts index 746a6928e..1777fbad9 100644 --- a/packages/hub-sessions/src/workflow-probe-gate.grants.test.ts +++ b/packages/hub-sessions/src/workflow-probe-gate.grants.test.ts @@ -420,3 +420,139 @@ describe("gateAndFreezeProbeResult declared grant requirements", () => { expect(calls).toEqual([]); }); }); + +describe("gateAndFreezeProbeResult declared requirement conditions", () => { + const WINDOW = { after: "09:00", before: "17:00", timezone: "UTC" }; + + function conditioned(conditions: Record): GrantRequirement { + return { ...FORGED_NAMED, conditions }; + } + + async function gate( + grantRequirements: GrantRequirement[], + approvals: ApprovalSet | { kind: "approve-probed" }, + ) { + const probeResult = await makeProbeResult({ + grants: [], + grantRequirements, + }); + const { persist, calls } = recordingPersist("def-conditions"); + const result = await gateAndFreezeProbeResult({ + assetId: "asset-1", + source: DEFINITION_SOURCE, + probeResult, + approvals, + persist, + }); + return { result, calls }; + } + + test("refuses a time_window missing its timezone and freezes nothing", async () => { + const bad = conditioned({ + time_window: { after: "09:00", before: "17:00" }, + }); + const { result, calls } = await gate([bad], { kind: "approve-probed" }); + + expect(result).toEqual({ + ok: false, + reason: "grant_requirement_conditions_invalid", + invalidGrantRequirements: [ + { + requirement: bad, + problems: ["time_window: timezone must be a string (was missing)"], + }, + ], + }); + expect(calls).toEqual([]); + }); + + test("refuses an unregistered condition", async () => { + const bad = conditioned({ max_spend_per_day: 100 }); + const { result, calls } = await gate([bad], { kind: "approve-probed" }); + + if (result.ok || result.reason !== "grant_requirement_conditions_invalid") { + throw new Error(`unexpected gate result: ${JSON.stringify(result)}`); + } + expect(result.invalidGrantRequirements[0]?.problems).toEqual([ + 'Unknown condition: "max_spend_per_day"', + ]); + expect(calls).toEqual([]); + }); + + test("refuses a tool condition, which the run child cannot evaluate", async () => { + const bad = conditioned({ tool: "tool:pkg" }); + const { result } = await gate([bad], { kind: "approve-probed" }); + + if (result.ok || result.reason !== "grant_requirement_conditions_invalid") { + throw new Error(`unexpected gate result: ${JSON.stringify(result)}`); + } + expect(result.invalidGrantRequirements[0]?.problems).toEqual([ + 'a workflow run cannot evaluate this grant: Unknown condition: "tool"', + ]); + }); + + test("refuses a time window on credential use, which the credential gate cannot evaluate", async () => { + const bad: GrantRequirement = { + resource: "credential:crd_1", + action: "use", + source: "creator", + conditions: { time_window: WINDOW }, + }; + const { result } = await gate([bad], { kind: "approve-probed" }); + + if (result.ok || result.reason !== "grant_requirement_conditions_invalid") { + throw new Error(`unexpected gate result: ${JSON.stringify(result)}`); + } + expect(result.invalidGrantRequirements[0]?.problems).toEqual([ + 'credential use cannot evaluate this grant: Unknown condition: "time_window"', + ]); + }); + + test("refuses any condition on a wildcard requirement", async () => { + const bad: GrantRequirement = { + resource: "*", + action: "*", + source: "creator", + conditions: { time_window: WINDOW }, + }; + const { result } = await gate([bad], { kind: "approve-probed" }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe("grant_requirement_conditions_invalid"); + } + }); + + test("names only the requirements whose conditions fail", async () => { + const good = conditioned({ time_window: WINDOW }); + const bad = { ...conditioned({ bogus: 1 }), resource: "wallet:wal_other" }; + const { result } = await gate([good, bad], { kind: "approve-probed" }); + + if (result.ok || result.reason !== "grant_requirement_conditions_invalid") { + throw new Error(`unexpected gate result: ${JSON.stringify(result)}`); + } + expect(result.invalidGrantRequirements.map((r) => r.requirement)).toEqual([ + bad, + ]); + }); + + test("refuses malformed conditions before the approval check", async () => { + // With no approval covering it, the requirement would otherwise be + // reported as unapproved, which an operator would act on instead. + const bad = conditioned({ bogus: 1 }); + const { result } = await gate([bad], createApprovalSet([], [])); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.reason).toBe("grant_requirement_conditions_invalid"); + } + }); + + test("freezes a requirement with a valid time_window", async () => { + const good = conditioned({ time_window: WINDOW }); + const { result, calls } = await gate([good], { kind: "approve-probed" }); + + expect(result.ok).toBe(true); + expect(calls).toHaveLength(1); + }); +}); diff --git a/packages/hub-sessions/src/workflow-probe-gate.refusal.test.ts b/packages/hub-sessions/src/workflow-probe-gate.refusal.test.ts index c7b04eb25..3dda2e254 100644 --- a/packages/hub-sessions/src/workflow-probe-gate.refusal.test.ts +++ b/packages/hub-sessions/src/workflow-probe-gate.refusal.test.ts @@ -100,6 +100,36 @@ describe("describeProbeGateRefusal", () => { }, 'credential_bindings_invalid: two bindings name action push and handle "github"', ], + [ + { + ok: false, + reason: "grant_requirement_conditions_invalid", + invalidGrantRequirements: [ + { + requirement: { + resource: "wallet:wal_1", + action: "spend", + source: "creator", + conditions: { time_window: {}, tool: "x" }, + }, + problems: [ + "time_window: after must be a string (was missing); before must be a string (was missing)", + 'Unknown condition: "tool"', + ], + }, + { + requirement: { + resource: "mail:*", + action: "send", + source: "invoker", + conditions: { bogus: 1 }, + }, + problems: ['Unknown condition: "bogus"'], + }, + ], + }, + 'grant_requirement_conditions_invalid: a requirement may carry only conditions a run can evaluate; declared: spend on wallet:wal_1 (creator) ["time_window: after must be a string (was missing); before must be a string (was missing)","Unknown condition: \\"tool\\""]; send on mail:* (invoker) ["Unknown condition: \\"bogus\\""]', + ], ]; test.each(cases)("names the refusal and what it refused", (refusal, text) => { diff --git a/packages/hub-sessions/src/workflow-probe-gate.ts b/packages/hub-sessions/src/workflow-probe-gate.ts index c6082cf31..798d79fb4 100644 --- a/packages/hub-sessions/src/workflow-probe-gate.ts +++ b/packages/hub-sessions/src/workflow-probe-gate.ts @@ -18,14 +18,17 @@ // 5. Reject a grant requirement inside the `mail.accept` namespace, and a // `mail.accept` walk marker the projection's `mailAccept` does not // produce, so the accept surface comes only from the hashed `mailAccept`. -// 6. Reject a projection carrying a step the grant walk left no record for, +// 6. Reject a declared grant requirement whose conditions a run cannot +// evaluate, so the frozen requirements only mint grant rows that +// authorization can evaluate. +// 7. Reject a projection carrying a step the grant walk left no record for, // so no deployment can schedule a step whose grants nobody approved. -// 7. Gate the advisory grant set AND the definition's declared grant +// 8. Gate the advisory grant set AND the definition's declared grant // requirements against the approval policy: an operator `ApprovalSet` // requires every grant the probe surfaced and every requirement it // declared to be approved or the gate fails, while `approve-probed` // approves exactly what the probe surfaced. -// 8. Freeze the approved wire hash onto the definition version row, keyed by +// 9. Freeze the approved wire hash onto the definition version row, keyed by // the definition's selector, and return the frozen approved grant set. // // The frozen approved set is the single source of truth for the definition's @@ -37,6 +40,7 @@ import { type } from "arktype"; import { and, eq } from "drizzle-orm"; +import { HUB_CONDITION_REGISTRY, validateGrantConditions } from "@intx/authz"; import type { DBExecutor } from "@intx/db"; import { workflowDefinitionVersion } from "@intx/db/schema"; import { @@ -135,8 +139,9 @@ export type PersistFrozenApprovalFn = ( * grant requirements the operator did not approve, a trigger type the runtime * does not implement, a grant requirement inside the `mail.accept` namespace, a * `mail.accept` walk marker the projection's `mailAccept` does not produce, - * credential bindings that cannot be delivered, and an executable step the - * grant walk left no record for. + * credential bindings that cannot be delivered, a declared grant requirement + * whose conditions a run cannot evaluate, and an executable step the grant + * walk left no record for. */ export type ProbeGateResult = | { @@ -213,6 +218,12 @@ export type ProbeGateResult = /** Declared requirements that use `self` outside `SELF_RUN_RESOURCE`. */ readonly grantRequirements: readonly GrantRequirement[]; } + | { + readonly ok: false; + readonly reason: "grant_requirement_conditions_invalid"; + /** Declared requirements whose conditions fail the check, in declared order. */ + readonly invalidGrantRequirements: readonly InvalidGrantRequirement[]; + } | { readonly ok: false; readonly reason: "steps_without_grant_record"; @@ -227,6 +238,15 @@ export type ProbeGateResult = readonly message: string; }; +/** + * A declared grant requirement whose conditions a run cannot evaluate, with + * one problem per bad condition entry. + */ +export type InvalidGrantRequirement = { + readonly requirement: GrantRequirement; + readonly problems: readonly string[]; +}; + /** * One step the deployment can execute that the probe's grant-walk snapshot * carries no record for. @@ -256,6 +276,9 @@ export type ProbeGateRefusal = Extract; // A requirement is named by what it asks for and whose authority it draws on. // Its conditions are left out: they are free-form and can be large, and the // action, resource, and source are what tell the author which one to change. +// A refusal about the conditions themselves adds the validator's problem +// text instead. That text can quote author values, so it is JSON-encoded to +// keep each problem's bounds unambiguous. function describeGrantRequirement(requirement: GrantRequirement): string { return `${requirement.action} on ${requirement.resource} (${requirement.source})`; } @@ -282,6 +305,8 @@ export function describeProbeGateRefusal(refusal: ProbeGateRefusal): string { return `mail_accept_markers_undeclared: the projection's mailAccept does not produce ${refusal.undeclaredMailAcceptMarkers.join(", ")}`; case "credential_bindings_invalid": return `credential_bindings_invalid: ${refusal.message}`; + case "grant_requirement_conditions_invalid": + return `grant_requirement_conditions_invalid: a requirement may carry only conditions a run can evaluate; declared: ${refusal.invalidGrantRequirements.map(({ requirement, problems }) => `${describeGrantRequirement(requirement)} ${JSON.stringify(problems)}`).join("; ")}`; case "self_run_resource_misplaced": return `self_run_resource_misplaced: a requirement names its own run only as "${SELF_RUN_RESOURCE}"; declared: ${refusal.grantRequirements.map(describeGrantRequirement).join("; ")}`; case "steps_without_grant_record": @@ -537,7 +562,8 @@ export type GateAndFreezeArgs = { * a projection whose triggers include a reserved-but-unimplemented type -- not * a security check, but the layer a pinned closure cannot carry a stale copy * of, so it is where a workflow that could only sit inert is caught -- one - * whose credential bindings cannot be delivered to a consumer it runs, and one + * whose credential bindings cannot be delivered to a consumer it runs, one + * declaring a grant requirement whose conditions a run cannot evaluate, and one * whose executable closure reaches a step the grant walk left no record for -- * the layer holding both halves of the probe answer at once, and a presence * check on those records rather than a check that any record's contents are @@ -671,6 +697,36 @@ export async function gateAndFreezeProbeResult( }; } + // A requirement's conditions are copied into the grant rows a run mints on + // its own principal, which the hub, the run child, and the credential gate + // each evaluate with their own registry. A condition one of them cannot + // evaluate would throw at every authorization the row reaches there, so it + // is an author error, refused before the operator checks: an approval match + // compares conditions, so a malformed one would otherwise read as + // unapproved. + const invalidGrantRequirements: InvalidGrantRequirement[] = []; + for (const requirement of probeResult.grantWalkSnapshot.grantRequirements) { + const problems = validateGrantConditions( + { + resource: requirement.resource, + action: requirement.action, + conditions: requirement.conditions ?? null, + onRunPrincipal: true, + }, + HUB_CONDITION_REGISTRY, + ); + if (problems.length > 0) { + invalidGrantRequirements.push({ requirement, problems }); + } + } + if (invalidGrantRequirements.length > 0) { + return { + ok: false, + reason: "grant_requirement_conditions_invalid", + invalidGrantRequirements, + }; + } + // Totality: every step the deployment can execute must have an approved-grant // record behind it. The two halves of a probe answer are produced // independently -- the projection by the hub's live->inert projector, the diff --git a/tests/db/run-delegated-grants.test.ts b/tests/db/run-delegated-grants.test.ts index 008f81788..363cab6d5 100644 --- a/tests/db/run-delegated-grants.test.ts +++ b/tests/db/run-delegated-grants.test.ts @@ -488,6 +488,9 @@ describe.skipIf(!harnessDbEnvAvailable())( }); describe("floors and standing decisions", () => { + // Re-derivation keys a row by its conditions without evaluating them, + // so any conditions body serves here, even this `tool` one that the + // probe gate would refuse on a declared requirement. const ASK_TOOL: GrantRequirement = { resource: "tool:search", action: "invoke",