From 8163d4f332e3566b5ddef32e0e805002369c79c3 Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Tue, 3 Nov 2026 18:55:17 -0700 Subject: [PATCH 1/7] Tag the grant checks that requireGrant builds --- packages/hub-api/src/middleware/grant.test.ts | 44 ++++++++++++++++++- packages/hub-api/src/middleware/grant.ts | 16 ++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/packages/hub-api/src/middleware/grant.test.ts b/packages/hub-api/src/middleware/grant.test.ts index bd8fe6295..0fe3fe7ca 100644 --- a/packages/hub-api/src/middleware/grant.test.ts +++ b/packages/hub-api/src/middleware/grant.test.ts @@ -7,13 +7,20 @@ import { test, } from "bun:test"; import { Hono } from "hono"; +import { every } from "hono/combine"; +import { createMiddleware } from "hono/factory"; import { createInMemoryGrantStore, toolConsumerEvaluator } from "@intx/authz"; import { configureSync, getConfig } from "@intx/log"; import type { GrantRule, GrantStore } from "@intx/types/authz"; import type { PrincipalRow, TenantEnv, TenantRow } from "../context"; -import { createAuthorizeGrant, createRequireGrant } from "./grant"; +import { + createAuthorizeGrant, + createRequireGrant, + idResource, + isGrantCheck, +} from "./grant"; const PRINCIPAL_ID = "prn_caller"; const TENANT_ID = "tnt_a"; @@ -263,3 +270,38 @@ describe("createRequireGrant with a consumer", () => { expect((await app.request("/")).status).toBe(200); }); }); + +describe("isGrantCheck", () => { + const requireGrant = createRequireGrant({ + grantStore: createInMemoryGrantStore([]), + conditionRegistry: {}, + }); + + test("recognizes a check for a fixed resource", () => { + expect(isGrantCheck(requireGrant("widget:*", "read"))).toBe(true); + }); + + test("recognizes a check for a resource read from the URL", () => { + expect(isGrantCheck(requireGrant(idResource("widget", "id"), "read"))).toBe( + true, + ); + }); + + test("recognizes a check from any requireGrant factory", () => { + const other = createRequireGrant({ + grantStore: createInMemoryGrantStore([]), + conditionRegistry: {}, + }); + expect(isGrantCheck(other("widget:*", "read"))).toBe(true); + }); + + test("does not recognize other middleware", () => { + expect(isGrantCheck(createMiddleware(async (_c, next) => next()))).toBe( + false, + ); + }); + + test("does not recognize a wrapper around a grant check", () => { + expect(isGrantCheck(every(requireGrant("widget:*", "read")))).toBe(false); + }); +}); diff --git a/packages/hub-api/src/middleware/grant.ts b/packages/hub-api/src/middleware/grant.ts index 6e422db20..b357cf156 100644 --- a/packages/hub-api/src/middleware/grant.ts +++ b/packages/hub-api/src/middleware/grant.ts @@ -90,6 +90,8 @@ export function createAuthorizeGrant({ }; } +const grantChecks = new WeakSet(); + /** * Builds a `requireGrant` middleware factory bound to the application's * grant store and condition registry. Usage: @@ -100,7 +102,7 @@ export function createAuthorizeGrant({ export function createRequireGrant(deps: CreateRequireGrantDeps): RequireGrant { const authorizeGrant = createAuthorizeGrant(deps); return function requireGrant(resource, action) { - return createMiddleware(async (c, next) => { + const middleware = createMiddleware(async (c, next) => { const resolvedResource = typeof resource === "function" ? resource({ param: (name) => c.req.param(name) }) @@ -124,9 +126,21 @@ export function createRequireGrant(deps: CreateRequireGrantDeps): RequireGrant { "You do not have permission to perform this action", ); }); + grantChecks.add(middleware); + return middleware; }; } +/** + * Whether `handler` is a grant check that `requireGrant` built. Only the + * middleware itself carries the tag: a wrapper around it, such as one that + * `hono/combine` builds, does not, so a check over a route's handler chain + * finds only grant checks mounted directly. + */ +export function isGrantCheck(handler: object): boolean { + return grantChecks.has(handler); +} + /** * Helper that builds a resource string from a URL parameter. * From 524a64570484e8c1fca07f3096253a85598c40fc Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Tue, 3 Nov 2026 19:06:00 -0700 Subject: [PATCH 2/7] Require a read grant to view tenant details Tenant details include the tenant config. The default member role holds read on every resource, so members keep access. Only principals whose roles lack that read lose it, and for them the admin UI tenant pages fail to load. --- docs/API.md | 4 +- packages/hub-api/src/routes/tenants.test.ts | 50 +++++++++++++++---- packages/hub-api/src/routes/tenants.ts | 7 ++- .../workflow-lifecycle-policy-routes.test.ts | 6 +-- 4 files changed, 53 insertions(+), 14 deletions(-) diff --git a/docs/API.md b/docs/API.md index deb67061c..727b516d8 100644 --- a/docs/API.md +++ b/docs/API.md @@ -251,8 +251,10 @@ Body: CreateTenant ### GET /api/tenants/:tenantId Get tenant details +Requires read on the tenant (`tenant:` / `read`). + 200: TenantResponse -- Tenant details -403: ErrorResponse -- Not an active member of this tenant +403: ErrorResponse -- Not an active member of this tenant, or insufficient grants 404: ErrorResponse -- Tenant not found 409: ErrorResponse -- Stored tenant config is invalid diff --git a/packages/hub-api/src/routes/tenants.test.ts b/packages/hub-api/src/routes/tenants.test.ts index 72307b434..d3d3194ac 100644 --- a/packages/hub-api/src/routes/tenants.test.ts +++ b/packages/hub-api/src/routes/tenants.test.ts @@ -42,11 +42,11 @@ function principalWith(status: PrincipalRow["status"]): PrincipalRow { }; } -function manageGrant(): GrantRule { +function allowGrant(resource: string, action: string): GrantRule { return { - id: "grant-tenant-manage", - resource: `tenant:${TENANT_ID}`, - action: "manage", + id: `grant-${resource}-${action}`, + resource, + action, effect: "allow", origin: "system", conditions: null, @@ -56,6 +56,14 @@ function manageGrant(): GrantRule { }; } +function manageGrant(): GrantRule { + return allowGrant(`tenant:${TENANT_ID}`, "manage"); +} + +function readGrant(): GrantRule { + return allowGrant(`tenant:${TENANT_ID}`, "read"); +} + function createMockDB(principal: PrincipalRow | undefined) { const notImplemented = (path: string) => () => { throw new Error(`mock: ${path} not implemented`); @@ -117,23 +125,47 @@ function patchName(app: Hono) { } describe("tenant detail and update gates", () => { - test("an active member reads the tenant", async () => { - const app = createTestApp({ principal: principalWith("active") }); + test("an active member holding read reads the tenant", async () => { + const app = createTestApp({ + principal: principalWith("active"), + grants: [readGrant()], + }); + const res = await app.request(`/api/tenants/${TENANT_ID}`); + expect(res.status).toBe(200); + }); + + test("an active member holding the default member grant reads the tenant", async () => { + const app = createTestApp({ + principal: principalWith("active"), + grants: [allowGrant("*", "read")], + }); const res = await app.request(`/api/tenants/${TENANT_ID}`); expect(res.status).toBe(200); }); + test("an active member without read cannot read the tenant", async () => { + const app = createTestApp({ principal: principalWith("active") }); + const res = await app.request(`/api/tenants/${TENANT_ID}`); + expect(res.status).toBe(403); + }); + test.each(["suspended", "invited", "deactivated"] as const)( - "a principal with status %s cannot read the tenant", + "a principal with status %s holding read cannot read the tenant", async (status) => { - const app = createTestApp({ principal: principalWith(status) }); + const app = createTestApp({ + principal: principalWith(status), + grants: [readGrant()], + }); const res = await app.request(`/api/tenants/${TENANT_ID}`); expect(res.status).toBe(403); }, ); test("a user outside the tenant cannot read it", async () => { - const app = createTestApp({ principal: undefined }); + const app = createTestApp({ + principal: undefined, + grants: [readGrant()], + }); const res = await app.request(`/api/tenants/${TENANT_ID}`); expect(res.status).toBe(403); }); diff --git a/packages/hub-api/src/routes/tenants.ts b/packages/hub-api/src/routes/tenants.ts index ca2c18603..28ea47f25 100644 --- a/packages/hub-api/src/routes/tenants.ts +++ b/packages/hub-api/src/routes/tenants.ts @@ -303,14 +303,19 @@ export function createTenantRoutes({ describeRoute({ tags: ["Tenants"], summary: "Get tenant details", + description: "Requires read on the tenant (`tenant:` / `read`).", responses: { 200: jsonResponse("Tenant details", TenantResponse), - 403: jsonResponse("Not an active member of this tenant", ErrorResponse), + 403: jsonResponse( + "Not an active member of this tenant, or insufficient grants", + ErrorResponse, + ), 404: jsonResponse("Tenant not found", ErrorResponse), 409: jsonResponse("Stored tenant config is invalid", ErrorResponse), }, }), resolveTenant, + requireGrant(idResource("tenant", "tenantId"), "read"), async (c) => { const tenantRow = c.get("tenant"); try { diff --git a/tests/hub-api/workflow-lifecycle-policy-routes.test.ts b/tests/hub-api/workflow-lifecycle-policy-routes.test.ts index d6a8b49e7..5f1441b6a 100644 --- a/tests/hub-api/workflow-lifecycle-policy-routes.test.ts +++ b/tests/hub-api/workflow-lifecycle-policy-routes.test.ts @@ -368,9 +368,9 @@ describe.skipIf(!harnessDbEnvAvailable())( test("reading a tenant reports its invalid stored config", async () => { await setTenantConfig(INVALID_LIFECYCLE); - const res = await makeApp(createInMemoryGrantStore([])).request( - `/api/tenants/${TENANT_ID}`, - ); + const res = await makeApp( + createInMemoryGrantStore([allow(`tenant:${TENANT_ID}`, "read")]), + ).request(`/api/tenants/${TENANT_ID}`); expect(res.status).toBe(409); expect(await res.json()).toMatchObject({ error: { From 92fed8b8428ab2322beffd2341c25e1738be849d Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Tue, 3 Nov 2026 19:15:25 -0700 Subject: [PATCH 3/7] Require read on grants to evaluate a principal's grants The request names any principal in the tenant and any consumer, and the response lists the grants that match. Without a grant check, any member could probe the access of every principal, run-token consumers included. The default member role holds read on every resource, so members keep access. Only roles without read on grants lose it. --- docs/API.md | 3 +- docs/ROUTES.md | 2 +- packages/hub-api/src/app.ts | 2 +- packages/hub-api/src/routes/grants.ts | 6 ++- tests/hub-api/grant-evaluate-consumer.test.ts | 47 +++++++++++++++++-- 5 files changed, 51 insertions(+), 9 deletions(-) diff --git a/docs/API.md b/docs/API.md index 727b516d8..86c4655cd 100644 --- a/docs/API.md +++ b/docs/API.md @@ -454,11 +454,12 @@ Revoke a grant ### POST /api/tenants/:tenantId/principals/:principalId/evaluate Evaluate grants for a principal -Evaluates what would happen if a principal attempted an operation. Returns the resolved effect and all matching grants. Useful for debugging authorization. Pass `consumer` to evaluate as a workflow run token's consumer, so grants scoped to that consumer match. +Evaluates what would happen if a principal attempted an operation. Returns the resolved effect and all matching grants. Useful for debugging authorization. Pass `consumer` to evaluate as a workflow run token's consumer, so grants scoped to that consumer match. Requires read on grants (`grant:*` / `read`). Body: EvaluateRequest 200: EvaluateResult -- Evaluation result +403: ErrorResponse -- Insufficient grants 404: ErrorResponse -- Principal not found ## Runs diff --git a/docs/ROUTES.md b/docs/ROUTES.md index f34a4f12a..1fef6913e 100644 --- a/docs/ROUTES.md +++ b/docs/ROUTES.md @@ -102,7 +102,7 @@ Response types are documented in the OpenAPI spec via the same ArkType types but There are no dedicated "binding" endpoints for connecting resources to principals. No `POST /wallets/:walletId/agents/:agentId` to give an agent wallet access. No `POST /credentials/:credentialId/agents/:agentId` to bind a credential. -Instead, all authorization flows through capability grants. The grant endpoints (`/api/tenants/:tenantId/grants`) manage tenant-level policies and role-based grants. These are the materialized grants that live on principals. The evaluate endpoint (`/api/tenants/:tenantId/principals/:principalId/evaluate`) lets operators debug authorization by asking "what would happen if this principal tried to do X?" In the same discovery vein, `GET /api/tenants/:tenantId/credentials/resolve/:name` resolves a named credential through the tenant hierarchy and returns its metadata (no secret) — which credential an agent would get. +Instead, all authorization flows through capability grants. The grant endpoints (`/api/tenants/:tenantId/grants`) manage tenant-level policies and role-based grants. These are the materialized grants that live on principals. The evaluate endpoint (`/api/tenants/:tenantId/principals/:principalId/evaluate`) lets operators debug authorization by asking "what would happen if this principal tried to do X?" It requires read on grants (`grant:*` / `read`), because its response lists the principal's matching grants. In the same discovery vein, `GET /api/tenants/:tenantId/credentials/resolve/:name` resolves a named credential through the tenant hierarchy and returns its metadata (no secret) — which credential an agent would get. Agent definitions declare grant requirements with source annotations (tenant, creator, invoker). These requirements are resolved at launch time by the control plane, which materializes grants on the agent's new principal. Creator-sourced and invoker-sourced grants are resolved against the respective principal's authority at launch time. Tenant-sourced grants are resolved from tenant role and system policies. The grant API endpoints manage the tenant-level policy layer; the launch flow handles per-agent materialization. diff --git a/packages/hub-api/src/app.ts b/packages/hub-api/src/app.ts index 57631e7aa..a71791a0a 100644 --- a/packages/hub-api/src/app.ts +++ b/packages/hub-api/src/app.ts @@ -362,7 +362,7 @@ export function mountHubRoutes( ); app.route( "/api/tenants/:tenantId/principals/:principalId/evaluate", - createEvaluateRoutes({ db, grantStore, conditionRegistry }), + createEvaluateRoutes({ db, requireGrant, grantStore, conditionRegistry }), ); // The run management surface -- list, observe, stop, mail a single run. // Mounted as `/workflows/runs` (tenant-wide runs) before the `/workflows` diff --git a/packages/hub-api/src/routes/grants.ts b/packages/hub-api/src/routes/grants.ts index c90b363b8..e5d76c1e0 100644 --- a/packages/hub-api/src/routes/grants.ts +++ b/packages/hub-api/src/routes/grants.ts @@ -483,12 +483,14 @@ export function createGrantRoutes({ // Evaluate endpoint is mounted under principals export type CreateEvaluateRoutesDeps = { db: DB["db"]; + requireGrant: RequireGrant; grantStore: GrantStore; conditionRegistry: ConditionRegistry; }; export function createEvaluateRoutes({ db, + requireGrant, grantStore, conditionRegistry, }: CreateEvaluateRoutesDeps): Hono { @@ -496,13 +498,15 @@ export function createEvaluateRoutes({ evaluateApp.post( "/", + requireGrant("grant:*", "read"), describeRoute({ tags: ["Grants"], summary: "Evaluate grants for a principal", description: - "Evaluates what would happen if a principal attempted an operation. Returns the resolved effect and all matching grants. Useful for debugging authorization. Pass `consumer` to evaluate as a workflow run token's consumer, so grants scoped to that consumer match.", + "Evaluates what would happen if a principal attempted an operation. Returns the resolved effect and all matching grants. Useful for debugging authorization. Pass `consumer` to evaluate as a workflow run token's consumer, so grants scoped to that consumer match. Requires read on grants (`grant:*` / `read`).", responses: { 200: jsonResponse("Evaluation result", EvaluateResult), + 403: jsonResponse("Insufficient grants", ErrorResponse), 404: jsonResponse("Principal not found", ErrorResponse), }, }), diff --git a/tests/hub-api/grant-evaluate-consumer.test.ts b/tests/hub-api/grant-evaluate-consumer.test.ts index 298c67e67..d4090eac3 100644 --- a/tests/hub-api/grant-evaluate-consumer.test.ts +++ b/tests/hub-api/grant-evaluate-consumer.test.ts @@ -26,7 +26,8 @@ import { seedPrincipal, seedTenants } from "@intx/test-harness/seed"; // The evaluate endpoint explains an authorization decision. A run token's // requests carry a workflow consumer, so an operator evaluating a run // principal must be able to name that consumer to see the decision the token -// gets. +// gets. The response lists the principal's matching grants, so the caller +// needs read on grants. const TENANT_ID = "tnt_evaluate"; const OPERATOR_PRINCIPAL_ID = "prn_operator"; @@ -53,8 +54,7 @@ function grantRule( }; } -const GRANTS: GrantRule[] = [ - grantRule("grt_operator", OPERATOR_PRINCIPAL_ID, "*", "*", null), +const RUN_GRANTS: GrantRule[] = [ grantRule("grt_run", RUN_PRINCIPAL_ID, "widget:*", "read", { tool: "action:a", }), @@ -67,6 +67,16 @@ const GRANTS: GrantRule[] = [ }, ]; +function operatorGrant(resource: string, action: string): GrantRule { + return grantRule( + `grt_operator_${resource}_${action}`, + OPERATOR_PRINCIPAL_ID, + resource, + action, + null, + ); +} + function mockGetSession(userId: string): GetSession { const now = new Date("2026-01-01"); return async () => ({ @@ -102,7 +112,7 @@ function mockSessionService(): SessionService { const EvaluateBody = type({ effect: "string" }); describe.skipIf(!harnessDbEnvAvailable())( - "POST /api/tenants/:tenantId/principals/:principalId/evaluate with a consumer", + "POST /api/tenants/:tenantId/principals/:principalId/evaluate", () => { let h: TestDb; @@ -132,12 +142,16 @@ describe.skipIf(!harnessDbEnvAvailable())( async function evaluateResponse( body: Record, + operatorGrants: GrantRule[] = [operatorGrant("*", "*")], ): Promise { const app = createApp({ getSession: mockGetSession(OPERATOR_USER_ID), authHandler: () => new Response("", { status: 404 }), db: h.db, - grantStore: createInMemoryGrantStore(GRANTS), + grantStore: createInMemoryGrantStore([ + ...operatorGrants, + ...RUN_GRANTS, + ]), sidecarRouter: createSidecarRouter({ withExecutableWorkflowRun: async (_target, send) => send(), authenticateSidecar: async ({ sidecarId }) => ({ sidecarId }), @@ -217,5 +231,28 @@ describe.skipIf(!harnessDbEnvAvailable())( }); expect(res.status).toBe(400); }); + + const WIDGET_READ = { resource: "widget:w1", action: "read" }; + + test("refuses a caller without read on grants", async () => { + const res = await evaluateResponse(WIDGET_READ, [ + operatorGrant("widget:*", "read"), + ]); + expect(res.status).toBe(403); + }); + + test("answers a caller holding read on grants", async () => { + const res = await evaluateResponse(WIDGET_READ, [ + operatorGrant("grant:*", "read"), + ]); + expect(res.status).toBe(200); + }); + + test("answers a caller holding read on every resource", async () => { + const res = await evaluateResponse(WIDGET_READ, [ + operatorGrant("*", "read"), + ]); + expect(res.status).toBe(200); + }); }, ); From 88e79b304eb5efbae33be0249d2e148be910a227 Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Tue, 3 Nov 2026 19:24:30 -0700 Subject: [PATCH 4/7] Require the consumer in the smart-HTTP git grant verdict No consumer reaches these routes today, because the git token middleware refuses run tokens. The required argument makes each caller state the consumer, so a later route cannot leave it out by omission, the same rule that the other grant checks follow. --- .../hub-api/src/routes/agent-state-git.ts | 3 + packages/hub-api/src/routes/assets.ts | 3 + .../src/routes/git-user-principal.test.ts | 62 +++++++++++++++++++ .../hub-api/src/routes/git-user-principal.ts | 11 +++- 4 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 packages/hub-api/src/routes/git-user-principal.test.ts diff --git a/packages/hub-api/src/routes/agent-state-git.ts b/packages/hub-api/src/routes/agent-state-git.ts index f5b93436b..026b5320f 100644 --- a/packages/hub-api/src/routes/agent-state-git.ts +++ b/packages/hub-api/src/routes/agent-state-git.ts @@ -345,6 +345,9 @@ async function resolveSmartHttp( tenantId, resource: `agent-state:${resolved.id}`, action, + // Only a run token carries a consumer, and the git token middleware + // refuses run tokens. + consumer: undefined, }); if (authz.effect !== "allow") { log.info( diff --git a/packages/hub-api/src/routes/assets.ts b/packages/hub-api/src/routes/assets.ts index 58bf45d55..782c98ea3 100644 --- a/packages/hub-api/src/routes/assets.ts +++ b/packages/hub-api/src/routes/assets.ts @@ -941,6 +941,9 @@ export function createAssetRoutes({ tenantId, resource: `asset:${resolvedAsset.asset.id}`, action, + // Only a run token carries a consumer, and the git token middleware + // refuses run tokens. + consumer: undefined, }); if (authz.effect !== "allow") { log.info("smart-HTTP authz denied {tenantId} asset={assetId}", { diff --git a/packages/hub-api/src/routes/git-user-principal.test.ts b/packages/hub-api/src/routes/git-user-principal.test.ts new file mode 100644 index 000000000..f3c6d2b70 --- /dev/null +++ b/packages/hub-api/src/routes/git-user-principal.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, test } from "bun:test"; + +import { createInMemoryGrantStore, HUB_CONDITION_REGISTRY } from "@intx/authz"; +import type { GrantRule } from "@intx/types/authz"; + +import { resolveAuthzVerdict } from "./git-user-principal"; + +const PRINCIPAL_ID = "prn_caller"; +const TENANT_ID = "tnt_a"; +const RESOURCE = "asset:a1"; + +function readGrant( + effect: GrantRule["effect"], + tool: string | null, +): GrantRule { + return { + id: `grant-${effect}-${tool ?? "any"}`, + resource: RESOURCE, + action: "read", + effect, + origin: "system", + conditions: tool === null ? null : { tool }, + expiresAt: null, + roleId: null, + principalId: PRINCIPAL_ID, + }; +} + +async function verdictFor(grants: GrantRule[], consumer: string | undefined) { + const authz = await resolveAuthzVerdict({ + grantStore: createInMemoryGrantStore(grants), + conditionRegistry: HUB_CONDITION_REGISTRY, + principalId: PRINCIPAL_ID, + tenantId: TENANT_ID, + resource: RESOURCE, + action: "createPack", + consumer, + }); + return authz.effect; +} + +describe("resolveAuthzVerdict with a consumer", () => { + const scopedDeny = [readGrant("allow", null), readGrant("deny", "action:a")]; + + test("a deny scoped to the consumer overrides an unscoped allow", async () => { + expect(await verdictFor(scopedDeny, "action:a")).toBe("deny"); + }); + + test("a deny scoped to another consumer does not apply", async () => { + expect(await verdictFor(scopedDeny, "action:b")).toBe("allow"); + }); + + test("a deny scoped to a consumer does not apply without one", async () => { + expect(await verdictFor(scopedDeny, undefined)).toBe("allow"); + }); + + test("an unscoped deny applies to every consumer", async () => { + const grants = [readGrant("allow", null), readGrant("deny", null)]; + expect(await verdictFor(grants, "action:a")).toBe("deny"); + expect(await verdictFor(grants, undefined)).toBe("deny"); + }); +}); diff --git a/packages/hub-api/src/routes/git-user-principal.ts b/packages/hub-api/src/routes/git-user-principal.ts index 07604f544..d1b5798d6 100644 --- a/packages/hub-api/src/routes/git-user-principal.ts +++ b/packages/hub-api/src/routes/git-user-principal.ts @@ -48,6 +48,12 @@ export async function resolveAuthzVerdict(args: { tenantId: string; resource: string; action: RepoAction; + /** + * The run-token consumer, or `undefined` for a caller that is not one. + * Required so that no request-driven check can forget it: a check without + * the consumer would skip a `{tool}`-scoped deny grant. + */ + consumer: string | undefined; }): Promise { const grantVerb = repoActionToGrantVerb(args.action); const verdict = await authorize( @@ -56,7 +62,10 @@ export async function resolveAuthzVerdict(args: { args.tenantId, args.resource, grantVerb, - { conditions: args.conditionRegistry }, + { + conditions: args.conditionRegistry, + ...(args.consumer !== undefined ? { consumer: args.consumer } : {}), + }, ); return { effect: verdict.effect === "allow" ? "allow" : "deny", From b6abef6c7d893bb0776c9f93e053cb5d4cb36f60 Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Tue, 3 Nov 2026 19:36:47 -0700 Subject: [PATCH 5/7] Check the run-token allow-list without a database Route registration never touches the database, so the check runs in the unit suite instead of only where Postgres is available. --- .../hub-api/src/route-registration.test.ts | 104 ++++++++++++++++++ tests/hub-api/run-token-auth.test.ts | 76 +------------ 2 files changed, 105 insertions(+), 75 deletions(-) create mode 100644 packages/hub-api/src/route-registration.test.ts diff --git a/packages/hub-api/src/route-registration.test.ts b/packages/hub-api/src/route-registration.test.ts new file mode 100644 index 000000000..9cdc97199 --- /dev/null +++ b/packages/hub-api/src/route-registration.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, test } from "bun:test"; + +import type { DB } from "@intx/db"; +import { + createEventCollectorRegistry, + createSidecarRouter, + type AssetService, + type RepoStore, + type SessionService, +} from "@intx/hub-sessions"; + +import { createApp } from "./app"; +import { RUN_TOKEN_ROUTES } from "./middleware/run-token-auth"; + +// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- drizzle PgDatabase type cannot be structurally satisfied in tests +const mockDb = {} as unknown as DB["db"]; + +const sessionService: SessionService = { + stageWorkflowStep(_params) { + throw new Error("mock: sessionService.stageWorkflowStep not implemented"); + }, + endSession(_addr, _reason) { + throw new Error("mock: sessionService.endSession not implemented"); + }, +}; + +function createRegistrationApp() { + // The deploy, signal and asset route groups mount only with a repo + // store and an asset service. Registration never calls either, so + // placeholders are enough to list the routes. + return createApp({ + getSession: async () => null, + authHandler: () => new Response("", { status: 404 }), + db: mockDb, + sidecarRouter: createSidecarRouter({ + withExecutableWorkflowRun: async (_target, send) => send(), + authenticateSidecar: async ({ sidecarId }) => ({ sidecarId }), + resolveSidecarBindings: async () => [], + validateSidecarIdentity: async () => true, + }), + sessionService, + eventCollectors: createEventCollectorRegistry({ db: mockDb }), + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- registration-only placeholder + assetService: {} as AssetService, + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- registration-only placeholder + repoStore: {} as RepoStore, + maxTarballBytes: 10_000_000, + }); +} + +describe("run-token allow-list", () => { + test("every allow-listed route is a registered tenant route", () => { + const app = createRegistrationApp(); + const shape = (path: string) => path.replace(/:[^/]+/g, ":"); + const registered = new Set( + app.routes.map((r) => `${r.method} ${shape(r.path)}`), + ); + const unregistered = RUN_TOKEN_ROUTES.filter( + (route) => !registered.has(`${route.method} ${shape(route.path)}`), + ); + expect(unregistered).toEqual([]); + + // The guard admits paths, not handlers. Where a listed pattern and an + // off-list route can match the same path, the router must send that + // path to the listed route, so the listed route must be registered + // first. + const handlers = app.routes.filter((r) => r.method !== "ALL"); + const isListed = (r: { method: string; path: string }) => + RUN_TOKEN_ROUTES.some( + (l) => l.method === r.method && shape(l.path) === shape(r.path), + ); + const overlap = (a: string, b: string) => { + const x = a.split("/"); + const y = b.split("/"); + return ( + x.length === y.length && + x.every((segment, i) => { + const other = y[i] ?? ""; + return ( + segment.startsWith(":") || + other.startsWith(":") || + segment === other + ); + }) + ); + }; + const shadowing: string[] = []; + handlers.forEach((offList, offIndex) => { + if (isListed(offList)) return; + for (const listed of RUN_TOKEN_ROUTES) { + if (listed.method !== offList.method) continue; + if (!overlap(listed.path, offList.path)) continue; + const listedIndex = handlers.findIndex( + (r) => + r.method === listed.method && shape(r.path) === shape(listed.path), + ); + if (listedIndex > offIndex) { + shadowing.push(`${offList.method} ${offList.path}`); + } + } + }); + expect(shadowing).toEqual([]); + }); +}); diff --git a/tests/hub-api/run-token-auth.test.ts b/tests/hub-api/run-token-auth.test.ts index 45f42359d..111e265e0 100644 --- a/tests/hub-api/run-token-auth.test.ts +++ b/tests/hub-api/run-token-auth.test.ts @@ -11,7 +11,7 @@ import { eq } from "drizzle-orm"; import { createInMemoryGrantStore } from "@intx/authz"; import { createRunTokenStore } from "@intx/db"; import { principal, workflowDefinition, workflowRun } from "@intx/db/schema"; -import { createApp, RUN_TOKEN_ROUTES, type GetSession } from "@intx/hub-api"; +import { createApp, type GetSession } from "@intx/hub-api"; import { createEventCollectorRegistry, createSidecarRouter, @@ -373,79 +373,5 @@ describe.skipIf(!harnessDbEnvAvailable())( }); expect(res.status).toBe(200); }); - - test("every allow-listed route is a registered tenant route", () => { - // The deploy, signal and asset route groups mount only with a repo - // store and an asset service. Registration never calls either, so - // placeholders are enough to list the routes. - const app = createApp({ - getSession: mockGetSession(null), - authHandler: () => new Response("", { status: 404 }), - db: h.db, - sidecarRouter: createSidecarRouter({ - withExecutableWorkflowRun: async (_target, send) => send(), - authenticateSidecar: async ({ sidecarId }) => ({ sidecarId }), - resolveSidecarBindings: async () => [], - validateSidecarIdentity: async () => true, - }), - sessionService: mockSessionService(), - eventCollectors: createEventCollectorRegistry({ db: h.db }), - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- registration-only placeholder - assetService: {} as AssetService, - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- registration-only placeholder - repoStore: {} as RepoStore, - maxTarballBytes: 10_000_000, - }); - const shape = (path: string) => path.replace(/:[^/]+/g, ":"); - const registered = new Set( - app.routes.map((r) => `${r.method} ${shape(r.path)}`), - ); - const unregistered = RUN_TOKEN_ROUTES.filter( - (route) => !registered.has(`${route.method} ${shape(route.path)}`), - ); - expect(unregistered).toEqual([]); - - // The guard admits paths, not handlers. Where a listed pattern and an - // off-list route can match the same path, the router must send that - // path to the listed route, so the listed route must be registered - // first. - const handlers = app.routes.filter((r) => r.method !== "ALL"); - const isListed = (r: { method: string; path: string }) => - RUN_TOKEN_ROUTES.some( - (l) => l.method === r.method && shape(l.path) === shape(r.path), - ); - const overlap = (a: string, b: string) => { - const x = a.split("/"); - const y = b.split("/"); - return ( - x.length === y.length && - x.every((segment, i) => { - const other = y[i] ?? ""; - return ( - segment.startsWith(":") || - other.startsWith(":") || - segment === other - ); - }) - ); - }; - const shadowing: string[] = []; - handlers.forEach((offList, offIndex) => { - if (isListed(offList)) return; - for (const listed of RUN_TOKEN_ROUTES) { - if (listed.method !== offList.method) continue; - if (!overlap(listed.path, offList.path)) continue; - const listedIndex = handlers.findIndex( - (r) => - r.method === listed.method && - shape(r.path) === shape(listed.path), - ); - if (listedIndex > offIndex) { - shadowing.push(`${offList.method} ${offList.path}`); - } - } - }); - expect(shadowing).toEqual([]); - }); }, ); From 90faa79acebecedfbbc13f461c17aceffa046447 Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Tue, 3 Nov 2026 19:50:32 -0700 Subject: [PATCH 6/7] Check the approval list grant in middleware The list route checks a fixed resource, so it needs nothing from the handler. As middleware, the check is visible on the route like every other fixed-resource grant check. A refusal now carries the shared grant-check message. Only the routes take requireGrant. resolveApproval keeps the narrower ResolveApprovalDeps, exported beside the route deps, so that a caller that resolves an approval directly does not have to supply it. --- packages/hub-api/src/app.ts | 1 + packages/hub-api/src/index.ts | 1 + packages/hub-api/src/routes/approvals.ts | 39 +++++++++--------------- 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/packages/hub-api/src/app.ts b/packages/hub-api/src/app.ts index a71791a0a..ec83c0907 100644 --- a/packages/hub-api/src/app.ts +++ b/packages/hub-api/src/app.ts @@ -463,6 +463,7 @@ export function mountHubRoutes( }, } : {}), + requireGrant, grantStore, grantStoreFor, conditionRegistry, diff --git a/packages/hub-api/src/index.ts b/packages/hub-api/src/index.ts index 98ab36008..ce43b4c82 100644 --- a/packages/hub-api/src/index.ts +++ b/packages/hub-api/src/index.ts @@ -51,6 +51,7 @@ export { type ResolveApprovalRequest, type ResolveApprovalOutcome, type CreateApprovalRoutesDeps, + type ResolveApprovalDeps, type ReadRunLifecycles, } from "./routes/approvals"; export { RUN_TOKEN_ROUTES } from "./middleware/run-token-auth"; diff --git a/packages/hub-api/src/routes/approvals.ts b/packages/hub-api/src/routes/approvals.ts index cbf2830e2..ef3900d60 100644 --- a/packages/hub-api/src/routes/approvals.ts +++ b/packages/hub-api/src/routes/approvals.ts @@ -32,7 +32,7 @@ import { import type { TenantEnv } from "../context"; import { errorResponse } from "../error-response"; -import { createAuthorizeGrant } from "../middleware/grant"; +import { createAuthorizeGrant, type RequireGrant } from "../middleware/grant"; import { ts } from "../format"; import { approvalToolName, @@ -70,7 +70,7 @@ export function formatApproval(row: ParsedApproval) { }; } -export type CreateApprovalRoutesDeps = { +export type ResolveApprovalDeps = { db: DB["db"]; sidecarRouter: SidecarRouter; workflowDispatchService?: Pick< @@ -85,6 +85,10 @@ export type CreateApprovalRoutesDeps = { signalCorrelationStore: SignalCorrelationStore; }; +export type CreateApprovalRoutesDeps = ResolveApprovalDeps & { + requireGrant: RequireGrant; +}; + export type ReadRunLifecycles = ( agentAddress: string, topLevelRunId: string, @@ -136,7 +140,7 @@ type PendingFailureKind = Extract< * allocation retain their direct sidecar delivery behavior. */ export async function resolveApproval( - deps: CreateApprovalRoutesDeps, + deps: ResolveApprovalDeps, args: ResolveApprovalRequest, ): Promise { const { @@ -385,6 +389,13 @@ export function createApprovalRoutes( app.get( "/", + // Listing spans the whole tenant, so it needs a tenant-wide grant + // (`approval:*`), not a per-deployment one: a per-deployment approver + // holds only `approval:` and reads individual + // approvals by id via the detail route. The action is `resolve`, the + // same capability the approve/reject routes gate on -- reading an + // approval is part of being able to resolve it. + deps.requireGrant("approval:*", "resolve"), describeRoute({ tags: ["Approvals"], summary: "List pending approvals in the tenant", @@ -404,28 +415,6 @@ export function createApprovalRoutes( }), async (c) => { const tenant = c.get("tenant"); - const principal = c.get("principal"); - - // Listing spans the whole tenant, so it needs a tenant-wide grant - // (`approval:*`), not a per-deployment one: a per-deployment approver - // holds only `approval:` and reads individual - // approvals by id via the detail route. The action is `resolve`, the - // same capability the approve/reject routes gate on -- reading an - // approval is part of being able to resolve it. - const allowed = await authorizeGrant({ - principalId: principal.id, - tenantId: tenant.id, - resource: "approval:*", - action: "resolve", - consumer: c.get("consumer"), - }); - if (!allowed) { - return errorResponse( - c, - "forbidden", - "You do not have permission to list approvals", - ); - } const { limit, cursor } = parsePageParams({ cursor: c.req.query("cursor"), From c280ee7b5254fae9da1e64560ad78686ee6f5638 Mon Sep 17 00:00:00 2001 From: Alexander Guy Date: Tue, 3 Nov 2026 20:01:09 -0700 Subject: [PATCH 7/7] Fail when a tenant route mounts without a grant check Grants are the only control on the hub API only if every tenant route checks one. A new tenant route without a grant check now fails the unit suite unless it is on the exemption list with a reason. --- docs/ROUTES.md | 2 + .../hub-api/src/route-registration.test.ts | 422 ++++++++++++++++++ .../hub-api/src/routes/agent-state-git.ts | 2 + packages/hub-api/src/routes/approvals.ts | 5 + packages/hub-api/src/routes/assets.ts | 2 + packages/hub-api/src/routes/runs.ts | 2 + .../hub-api/src/routes/tenant-federation.ts | 2 + 7 files changed, 437 insertions(+) diff --git a/docs/ROUTES.md b/docs/ROUTES.md index 1fef6913e..c630f9de1 100644 --- a/docs/ROUTES.md +++ b/docs/ROUTES.md @@ -104,6 +104,8 @@ There are no dedicated "binding" endpoints for connecting resources to principal Instead, all authorization flows through capability grants. The grant endpoints (`/api/tenants/:tenantId/grants`) manage tenant-level policies and role-based grants. These are the materialized grants that live on principals. The evaluate endpoint (`/api/tenants/:tenantId/principals/:principalId/evaluate`) lets operators debug authorization by asking "what would happen if this principal tried to do X?" It requires read on grants (`grant:*` / `read`), because its response lists the principal's matching grants. In the same discovery vein, `GET /api/tenants/:tenantId/credentials/resolve/:name` resolves a named credential through the tenant hierarchy and returns its metadata (no secret) — which credential an agent would get. +Every route under `/api/tenants/:tenantId` checks a grant, so grants are the only control on the tenant API. A route mounts `requireGrant` ahead of its handler, on the route itself or on a `/*` prefix above it. A route whose resource comes from data that the handler reads (a request body, a database row, a git token) checks its grant in the handler instead, and it has an entry with a reason in `GRANT_EXEMPTIONS` in `packages/hub-api/src/route-registration.test.ts`. That unit test fails when a tenant route has neither. It proves that a check is present, not that its resource and action are right. + Agent definitions declare grant requirements with source annotations (tenant, creator, invoker). These requirements are resolved at launch time by the control plane, which materializes grants on the agent's new principal. Creator-sourced and invoker-sourced grants are resolved against the respective principal's authority at launch time. Tenant-sourced grants are resolved from tenant role and system policies. The grant API endpoints manage the tenant-level policy layer; the launch flow handles per-agent materialization. ## Smart-HTTP Git Endpoints diff --git a/packages/hub-api/src/route-registration.test.ts b/packages/hub-api/src/route-registration.test.ts index 9cdc97199..2bf45dc74 100644 --- a/packages/hub-api/src/route-registration.test.ts +++ b/packages/hub-api/src/route-registration.test.ts @@ -1,5 +1,8 @@ import { describe, expect, test } from "bun:test"; +import { Hono, type Env } from "hono"; +import type { RouterRoute } from "hono/types"; +import { createInMemoryGrantStore } from "@intx/authz"; import type { DB } from "@intx/db"; import { createEventCollectorRegistry, @@ -10,6 +13,8 @@ import { } from "@intx/hub-sessions"; import { createApp } from "./app"; +import type { TenantEnv } from "./context"; +import { createRequireGrant, isGrantCheck } from "./middleware/grant"; import { RUN_TOKEN_ROUTES } from "./middleware/run-token-auth"; // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- drizzle PgDatabase type cannot be structurally satisfied in tests @@ -102,3 +107,420 @@ describe("run-token allow-list", () => { expect(shadowing).toEqual([]); }); }); + +const TENANT_PREFIX = "/api/tenants/:tenantId"; + +// Any name for the tenant parameter, so a route that spells it differently +// is still checked. +function isTenantPath(path: string): boolean { + return /^\/api\/tenants\/:[^/]+(\/|$)/.test(path); +} + +/** + * A tenant route that checks its grant in the handler instead of in + * `requireGrant`, because the resource comes from data that the handler + * reads. `path` is relative to `TENANT_PREFIX`. `reason` names the function + * that checks the grant, the resource and action, and where the resource + * comes from. + */ +type GrantExemption = { method: string; path: string; reason: string }; + +type SampleVerdict = "checked" | "unchecked" | { fault: string }; + +// The value for each parameter and wildcard in a sample request. It must not +// match a static path segment, or a guard on that segment could seem to +// cover a route with a parameter in the same place. +const SAMPLE_SEGMENT = "__sample__"; + +const GRANT_EXEMPTIONS: GrantExemption[] = [ + { + method: "GET", + path: "/workflows/runs/blobs/:blobId", + reason: + "blob handler: workflow-run: / read; run from the session mail row", + }, + { + method: "GET", + path: "/approvals/:approvalId", + reason: + "approval detail handler: approval: / resolve; run from the approval row", + }, + { + method: "POST", + path: "/approvals/:approvalId/approve", + reason: + "resolveApproval: approval: / resolve; run from the approval row", + }, + { + method: "POST", + path: "/approvals/:approvalId/reject", + reason: + "resolveApproval: approval: / resolve; run from the approval row", + }, + { + method: "POST", + path: "/federation", + reason: + "denyUnlessBodyGrant: federation: / create; target from the body", + }, + { + method: "GET", + path: "/assets/:kind/:nameDotGit/info/refs", + reason: + "resolveSmartHttp (assets): git token, asset: verdict after the asset lookup", + }, + { + method: "POST", + path: "/assets/:kind/:nameDotGit/git-upload-pack", + reason: + "resolveSmartHttp (assets): git token, asset: verdict after the asset lookup", + }, + { + method: "POST", + path: "/assets/:kind/:nameDotGit/git-receive-pack", + reason: + "resolveSmartHttp (assets): git token, asset: verdict after the asset lookup", + }, + { + method: "GET", + path: "/workflows/runs/:runId/state.git/info/refs", + reason: + "resolveSmartHttp (agent-state-git): git token, agent-state: verdict after the run lookup", + }, + { + method: "POST", + path: "/workflows/runs/:runId/state.git/git-upload-pack", + reason: + "resolveSmartHttp (agent-state-git): git token, agent-state: verdict after the run lookup", + }, +]; + +/** + * Lists every tenant route that runs without a grant check and is not in + * `exemptions`, plus every exemption that is stale, repeated or not needed. + * + * The check asks the app's own router for the handler chain of a sample + * request to each route, and requires a `requireGrant` middleware before the + * route's handler. It proves that a grant check is present, not that its + * resource and action are right. A middleware that wraps `requireGrant` + * carries no tag, so a route behind one reports as unchecked. That includes + * every route of a sub-app with its own `onError`, because `app.route()` + * wraps each handler of such a sub-app. + * + * The route table does not say which entries are middleware. An `ALL` entry + * on a path that ends in `*` counts as middleware, so the check does not see + * a handler mounted with `app.all(".../*", ...)`, and middleware mounted with + * `app.use()` on an exact path reads as a route; mount tenant middleware on a + * `/*` path. A route registered twice in a row, the second time with a grant + * check and a handler, is also out of reach; a route registered twice with + * other routes between reports as registered more than once. A grant check + * counts if it comes before the route's last handler, so the check does not + * see a handler earlier in the same registration that answers first; put + * `requireGrant` ahead of every handler that can answer. + * + * A route for every method is sent as GET, so the check does not see a route + * that answers another method on the path first. Reject such a tenant route + * outright if one is ever needed; a list of methods cannot cover custom ones. + */ +function findGrantViolations( + app: Hono, + exemptions: GrantExemption[], +): string[] { + const violations: string[] = []; + // Each handler of a route has its own entry, and the router hands back + // the same entry objects, so entries identify a route even when routes + // share a handler function. One registration adds its entries in a row, + // and its last entry is the one that answers. + const entriesOf = new Map(); + app.routes.forEach((r, i) => { + if (r.method === "ALL" && r.path.endsWith("*")) return; + const key = `${r.method} ${r.path}`; + const seen = entriesOf.get(key); + if (seen !== undefined && seen.last !== i - 1 && isTenantPath(r.path)) { + violations.push(`${key}: registered more than once`); + } + entriesOf.set(key, { entries: [...(seen?.entries ?? []), r], last: i }); + }); + // Routes outside the tenant prefix stay in the map, because one of them can + // answer a tenant path first. + const routes = new Map(); + for (const [key, { entries }] of entriesOf) { + const answering = entries.at(-1); + if (answering === undefined) continue; + // Grant checks with no handler on a wildcard path guard a prefix, such as + // one mounted with `app.get(".../*", requireGrant(...))`. + const guard = entries.every((e) => isGrantCheck(e.handler)); + if (answering.path.endsWith("*") && guard) continue; + routes.set(key, answering); + } + const routeOfEntry = new Map(); + for (const [key, r] of routes) routeOfEntry.set(r, key); + + const exempt = new Set(); + for (const e of exemptions) { + const key = `${e.method} ${TENANT_PREFIX}${e.path}`; + if (exempt.has(key)) violations.push(`${key}: exempted twice`); + exempt.add(key); + if (!routes.has(key)) violations.push(`${key}: exemption matches no route`); + } + + for (const [key, r] of routes) { + if (!isTenantPath(r.path)) continue; + if (r.path.includes("{")) { + violations.push(`${key}: no sample value for a regex parameter`); + continue; + } + // A `/*` route also answers its bare prefix, so it gets a sample of each, + // unless the bare prefix has its own route, which the check covers. + const stem = r.path.replace(/:\w+/g, SAMPLE_SEGMENT); + const bare = `${r.method} ${r.path.slice(0, -2)}`; + const filled = stem.replace(/\*$/, SAMPLE_SEGMENT); + const samples = + stem.endsWith("/*") && !routes.has(bare) + ? [filled, stem.slice(0, -2)] + : [filled]; + const results = samples.map((sample) => ({ + sample, + verdict: verdictAt(r, sample), + })); + const uniform = results.every( + ({ verdict }) => + JSON.stringify(verdict) === JSON.stringify(results[0]?.verdict), + ); + for (const [i, { sample, verdict }] of results.entries()) { + if (uniform && i > 0) break; + const at = uniform ? key : `${key} at ${sample}`; + if (typeof verdict === "object") { + violations.push(`${at}: ${verdict.fault}`); + } else if (verdict === "unchecked" && !exempt.has(key)) { + violations.push(`${at}: no grant check`); + } + } + const checked = results.every(({ verdict }) => verdict === "checked"); + if (checked && exempt.has(key)) { + violations.push(`${key}: has a grant check, so needs no exemption`); + } + } + return violations.sort(); + + function verdictAt(r: RouterRoute, sample: string): SampleVerdict { + const [matches] = app.router.match( + r.method === "ALL" ? "GET" : r.method, + sample, + ); + const chain = matches.map(([[handler, entry]]) => ({ handler, entry })); + const own = chain.findIndex((link) => link.entry === r); + if (own < 0) return { fault: `${sample} does not reach the route` }; + const before = chain.slice(0, own); + const shadow = before.find((link) => routeOfEntry.has(link.entry)); + if (shadow !== undefined) { + return { fault: `shadowed by ${routeOfEntry.get(shadow.entry)}` }; + } + // A route for every method needs a guard for every method; a guard for + // some methods leaves the others open. + const checked = before.some( + (link) => + isGrantCheck(link.handler) && + (r.method !== "ALL" || link.entry.method === "ALL"), + ); + return checked ? "checked" : "unchecked"; + } +} + +describe("tenant route grant checks", () => { + test("every tenant route checks a grant or is exempt", () => { + expect( + findGrantViolations(createRegistrationApp(), GRANT_EXEMPTIONS), + ).toEqual([]); + }); + + test("the check sees the route groups that mount only with a repo store", () => { + const registered = new Set( + createRegistrationApp().routes.map((r) => `${r.method} ${r.path}`), + ); + const expected = [ + "POST /workflows/deployments", + "GET /git-tokens", + "GET /assets", + "GET /workflows/runs/:runId/state.git/info/refs", + ].map((route) => { + const [method, path] = route.split(" "); + return `${method} ${TENANT_PREFIX}${path}`; + }); + expect(expected.filter((route) => !registered.has(route))).toEqual([]); + }); + + const requireGrant = createRequireGrant({ + grantStore: createInMemoryGrantStore([]), + conditionRegistry: {}, + }); + const route = `${TENANT_PREFIX}/x`; + const answer = () => new Response("ok"); + + test("accepts a route that mounts a grant check", () => { + const app = new Hono(); + app.get(route, requireGrant("widget:*", "read"), answer); + expect(findGrantViolations(app, [])).toEqual([]); + }); + + test("accepts a route under a prefix that mounts a grant check", () => { + const app = new Hono(); + app.use(`${route}/*`, requireGrant("widget:*", "read")); + app.get(`${route}/y`, answer); + expect(findGrantViolations(app, [])).toEqual([]); + }); + + test("reports a route without a grant check", () => { + const app = new Hono(); + app.get(route, answer); + expect(findGrantViolations(app, [])).toEqual([ + `GET ${route}: no grant check`, + ]); + }); + + test("accepts an exempt route without a grant check", () => { + const app = new Hono(); + app.get(route, answer); + expect( + findGrantViolations(app, [{ method: "GET", path: "/x", reason: "r" }]), + ).toEqual([]); + }); + + test("reports stale, repeated and unneeded exemptions", () => { + const app = new Hono(); + app.get(route, requireGrant("widget:*", "read"), answer); + expect( + findGrantViolations(app, [ + { method: "GET", path: "/x", reason: "r" }, + { method: "GET", path: "/x", reason: "r" }, + { method: "GET", path: "/gone", reason: "r" }, + ]), + ).toEqual([ + `GET ${TENANT_PREFIX}/gone: exemption matches no route`, + `GET ${route}: exempted twice`, + `GET ${route}: has a grant check, so needs no exemption`, + ]); + }); + + test("reports a route that an earlier route answers first", () => { + const app = new Hono(); + app.get(`${route}/:id`, requireGrant("widget:*", "read"), answer); + app.get(`${route}/special`, answer); + expect(findGrantViolations(app, [])).toEqual([ + `GET ${route}/special: shadowed by GET ${route}/:id`, + ]); + }); + + test("reports a route without a grant check on the real app", () => { + const app = createRegistrationApp(); + app.get(route, answer); + expect(findGrantViolations(app, GRANT_EXEMPTIONS)).toEqual([ + `GET ${route}: no grant check`, + ]); + }); + + test("reports a route for every method without a grant check", () => { + const app = new Hono(); + app.all(route, answer); + expect(findGrantViolations(app, [])).toEqual([ + `ALL ${route}: no grant check`, + ]); + }); + + test("reports a route for every method behind a guard for one method", () => { + const app = new Hono(); + app.get(`${route}/*`, requireGrant("widget:*", "read")); + app.all(`${route}/y`, answer); + expect(findGrantViolations(app, [])).toEqual([ + `ALL ${route}/y: no grant check`, + ]); + }); + + test("reports a route whose grant check comes after its handler", () => { + const app = new Hono(); + app.get(route, answer, requireGrant("widget:*", "read")); + expect(findGrantViolations(app, [])).toEqual([ + `GET ${route}: no grant check`, + ]); + }); + + test("reports a route for every method behind a guard for some methods", () => { + const app = new Hono(); + app.on( + ["GET", "POST", "PUT", "PATCH", "DELETE"], + `${route}/*`, + requireGrant("widget:*", "read"), + ); + app.all(`${route}/y`, answer); + expect(findGrantViolations(app, [])).toEqual([ + `ALL ${route}/y: no grant check`, + ]); + }); + + test("reports a wildcard route whose grant check comes after its handler", () => { + const app = new Hono(); + app.get(`${route}/*`, answer, requireGrant("widget:*", "read")); + expect(findGrantViolations(app, [])).toEqual([ + `GET ${route}/*: no grant check`, + ]); + }); + + test("reports a tenant route that a route outside the prefix answers first", () => { + const app = new Hono(); + app.get("/api/tenants/*", answer); + app.get(route, requireGrant("widget:*", "read"), answer); + expect(findGrantViolations(app, [])).toEqual([ + `GET ${route}: shadowed by GET /api/tenants/*`, + ]); + }); + + test("reports a wildcard route whose bare prefix skips the guard", () => { + const app = new Hono(); + app.use(`${route}/:seg/*`, requireGrant("widget:*", "read")); + app.get(`${route}/*`, answer); + expect(findGrantViolations(app, [])).toEqual([ + `GET ${route}/* at /api/tenants/${SAMPLE_SEGMENT}/x: no grant check`, + ]); + }); + + test("reports a route that only a guard on a static segment seems to cover", () => { + const app = new Hono(); + app.use(`${route}/*`, requireGrant("widget:*", "read")); + app.get(`${TENANT_PREFIX}/:thing/y`, answer); + expect(findGrantViolations(app, [])).toEqual([ + `GET ${TENANT_PREFIX}/:thing/y: no grant check`, + ]); + }); + + test("leaves the bare prefix of a wildcard route to its own route", () => { + const app = new Hono(); + app.get(route, requireGrant("widget:*", "read"), answer); + app.get(`${route}/*`, requireGrant("widget:*", "read"), answer); + expect(findGrantViolations(app, [])).toEqual([]); + }); + + test("reports a tenant route with another name for the tenant parameter", () => { + const app = new Hono(); + app.get("/api/tenants/:tid/y", answer); + expect(findGrantViolations(app, [])).toEqual([ + "GET /api/tenants/:tid/y: no grant check", + ]); + }); + + test("accepts a route under a method prefix that mounts a grant check", () => { + const app = new Hono(); + app.get(`${route}/*`, requireGrant("widget:*", "read")); + app.get(`${route}/y`, answer); + expect(findGrantViolations(app, [])).toEqual([]); + }); + + test("reports a route registered again by a later route group", () => { + const app = new Hono(); + app.get(route, answer); + app.get(`${route}/other`, requireGrant("widget:*", "read"), answer); + app.get(route, requireGrant("widget:*", "read"), answer); + expect(findGrantViolations(app, [])).toEqual([ + `GET ${route}: registered more than once`, + ]); + }); +}); diff --git a/packages/hub-api/src/routes/agent-state-git.ts b/packages/hub-api/src/routes/agent-state-git.ts index 026b5320f..3dd0eb4f0 100644 --- a/packages/hub-api/src/routes/agent-state-git.ts +++ b/packages/hub-api/src/routes/agent-state-git.ts @@ -293,6 +293,8 @@ type ResolveSmartHttpDeps = { conditionRegistry: ConditionRegistry; }; +// The smart-HTTP routes check their grant here, after the URL names the +// run; see GRANT_EXEMPTIONS in route-registration.test.ts. async function resolveSmartHttp( deps: ResolveSmartHttpDeps, c: Context, diff --git a/packages/hub-api/src/routes/approvals.ts b/packages/hub-api/src/routes/approvals.ts index ef3900d60..c471bb170 100644 --- a/packages/hub-api/src/routes/approvals.ts +++ b/packages/hub-api/src/routes/approvals.ts @@ -138,6 +138,9 @@ type PendingFailureKind = Extract< * allocation's durable, generation-fenced dispatch path and remains replayable * until workflow Git records the signal as received. Deployments without an * allocation retain their direct sidecar delivery behavior. + * + * The approve and reject routes check their grant here, after the approval + * row names the run; see GRANT_EXEMPTIONS in route-registration.test.ts. */ export async function resolveApproval( deps: ResolveApprovalDeps, @@ -444,6 +447,8 @@ export function createApprovalRoutes( }, ); + // Checks its grant in the handler, after the approval row names the run; + // see GRANT_EXEMPTIONS in route-registration.test.ts. app.get( "/:approvalId", describeRoute({ diff --git a/packages/hub-api/src/routes/assets.ts b/packages/hub-api/src/routes/assets.ts index 782c98ea3..0f4a519a7 100644 --- a/packages/hub-api/src/routes/assets.ts +++ b/packages/hub-api/src/routes/assets.ts @@ -898,6 +898,8 @@ export function createAssetRoutes({ repoId: RepoId; }; + // The smart-HTTP routes check their grant here, after the URL names the + // asset; see GRANT_EXEMPTIONS in route-registration.test.ts. async function resolveSmartHttp( c: Context, action: RepoAction, diff --git a/packages/hub-api/src/routes/runs.ts b/packages/hub-api/src/routes/runs.ts index 7e40a5363..ac658974f 100644 --- a/packages/hub-api/src/routes/runs.ts +++ b/packages/hub-api/src/routes/runs.ts @@ -353,6 +353,8 @@ export function createRunRoutes({ }, ); + // Checks its grant in the handler, after the session mail row names the + // run; see GRANT_EXEMPTIONS in route-registration.test.ts. app.get( "/blobs/:blobId", describeRoute({ diff --git a/packages/hub-api/src/routes/tenant-federation.ts b/packages/hub-api/src/routes/tenant-federation.ts index da1e866b2..987c31995 100644 --- a/packages/hub-api/src/routes/tenant-federation.ts +++ b/packages/hub-api/src/routes/tenant-federation.ts @@ -106,6 +106,8 @@ export function createTenantFederationRoutes({ }, ); + // Checks its grant in the handler, after the body names the target tenant; + // see GRANT_EXEMPTIONS in route-registration.test.ts. app.post( "/", describeRoute({