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 5b79704c0..874040e93 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:*", @@ -283,6 +284,7 @@ "version": "0.5.0", "dependencies": { "@intx/types": "workspace:*", + "arktype": "catalog:", }, }, "packages/crypto": { @@ -396,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/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..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) } ``` @@ -331,16 +331,31 @@ 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: + +- 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 (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/authz/README.md b/packages/authz/README.md index 77bca2f48..96e8d61ed 100644 --- a/packages/authz/README.md +++ b/packages/authz/README.md @@ -64,17 +64,39 @@ 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. +- `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 + `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/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/condition-registry-invariants.test.ts b/packages/authz/src/condition-registry-invariants.test.ts new file mode 100644 index 000000000..ce113cfe6 --- /dev/null +++ b/packages/authz/src/condition-registry-invariants.test.ts @@ -0,0 +1,34 @@ +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", () => { + // 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}: `); + } + }); + } + + // 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/conditions.test.ts b/packages/authz/src/conditions.test.ts index b07005faa..7e0d567b0 100644 --- a/packages/authz/src/conditions.test.ts +++ b/packages/authz/src/conditions.test.ts @@ -1,7 +1,28 @@ import { describe, test, expect } from "bun:test"; -import { evaluateConditions } from "./conditions"; -import type { ConditionContext, ConditionRegistry } from "./types"; +import { evaluateConditions, validateConditions } from "./conditions"; +import { timeWindowEvaluator } from "./time-window"; +import type { + ConditionContext, + ConditionEvaluator, + ConditionRegistry, +} from "./types"; + +function evaluator( + evaluate: ConditionEvaluator["evaluate"], +): ConditionEvaluator { + return { validate: () => null, evaluate }; +} + +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 { @@ -17,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 () => { @@ -32,7 +53,7 @@ describe("evaluateConditions", () => { test("single condition evaluated against registry", async () => { const registry: ConditionRegistry = { - is_tuesday: () => true, + is_tuesday: evaluator(() => true), }; expect( @@ -42,7 +63,7 @@ describe("evaluateConditions", () => { test("condition returning false rejects the grant", async () => { const registry: ConditionRegistry = { - always_fail: () => false, + always_fail: evaluator(() => false), }; expect( @@ -52,9 +73,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( @@ -68,8 +89,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( @@ -80,10 +101,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); @@ -94,10 +115,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" }); @@ -111,10 +132,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( @@ -124,10 +145,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( @@ -138,11 +159,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); @@ -150,10 +171,99 @@ describe("evaluateConditions", () => { expect(secondCalled).toBe(false); }); - 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"', + 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}"`, + ); + } + }); +}); + +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 c641dbc8e..dba24970e 100644 --- a/packages/authz/src/conditions.ts +++ b/packages/authz/src/conditions.ts @@ -1,4 +1,22 @@ -import type { ConditionContext, ConditionRegistry } from "./types"; +import type { + ConditionContext, + ConditionEvaluator, + 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. +function lookupEvaluator( + registry: ConditionRegistry, + key: string, +): ConditionEvaluator | undefined { + return Object.hasOwn(registry, key) ? registry[key] : undefined; +} /** * Evaluate whether a grant's conditions are satisfied. @@ -13,7 +31,7 @@ import type { ConditionContext, ConditionRegistry } from "./types"; export async function evaluateConditions( conditions: Record | null, ctx: ConditionContext, - registry: ConditionRegistry = {}, + registry: ConditionRegistry, ): Promise { if (!conditions) return true; @@ -21,16 +39,44 @@ 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.`, + `${unknownConditionMessage(key)}. Register an evaluator in the condition registry.`, ); } - const result = await evaluator(conditions[key], ctx); + const result = await evaluator.evaluate(conditions[key], ctx); if (!result) return false; } 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/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/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/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 d39115c17..33d892369 100644 --- a/packages/authz/src/index.ts +++ b/packages/authz/src/index.ts @@ -20,7 +20,12 @@ export { } from "./mail-accept-relation"; export { evaluateMailAdmission } from "./mail-admission"; export type { MailAdmissionResult } from "./mail-admission"; -export { evaluateConditions } from "./conditions"; +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"; export { actionConsumer, 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/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/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/hub-api/src/app.ts b/packages/hub-api/src/app.ts index 3c04563ba..57631e7aa 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, @@ -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/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, 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/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/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; 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/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, 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", 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, + }); + }); + }); }, );