diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 352a090..1758669 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -76,7 +76,7 @@ jobs: npm install "$TARBALL" node -e ' import("@corbits/artifacts").then((m) => { - for (const name of ["mountArtifacts", "runArtifactMigrations"]) { + for (const name of ["createArtifactRoutes", "runArtifactMigrations"]) { if (typeof m[name] !== "function") throw new Error(`missing export: ${name}`); } console.log("node consumer ok"); diff --git a/CHANGELOG.md b/CHANGELOG.md index ee2661a..929ca19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ always called out under their own heading. - Run-scoped `POST /artifacts`, `POST /artifacts/binary`, and `PATCH /artifacts/:id` (`mountWorkflowArtifacts`) accept an optional - `metadata` field, matching `mountArtifacts`' semantics exactly: omitted on + `metadata` field, matching the tenant routes' semantics exactly: omitted on a revise carries the prior version's metadata forward, an explicit `null` clears it, and any other value must be a JSON object or the request is `400`. `artifact_create` and `artifact_write` in `ARTIFACT_TOOL_DEFINITIONS` @@ -102,9 +102,10 @@ always called out under their own heading. `bun add github:corbitsdev/corbits-artifacts` installs cleanly. Bun consumers resolve TypeScript sources via the `bun` export condition; Node consumers continue to use the built `dist/` from `npm pack` / a published release. -- `mountArtifacts` takes an optional `onArtifactCreated(tx, row, scope)` hook, - run inside the same transaction as artifact creation (once per row, so once - on `POST /artifacts` and once per file on `POST /artifacts/upload`). This is +- `createArtifactRoutes` takes an optional + `onArtifactCreated(tx, row, scope)` hook, run inside the same transaction + as artifact creation (once per row, so once on `POST /artifacts` and once + per file on `POST /artifacts/upload`). This is the seam a host uses to provision grants for the row it just made — for example, a `creator`-origin grant on `artifact:` for `write` and `archive`. Defaults to a no-op, so existing hosts are unaffected. @@ -124,6 +125,16 @@ always called out under their own heading. ### Breaking +- `mountArtifacts(app, opts)` is replaced by `createArtifactRoutes(deps)`, + which returns a `Hono` sub-app the host mounts with + `app.route(...)` instead of mutating the host app. `MountArtifactsOpts` is + renamed `CreateArtifactRoutesDeps`; the options are unchanged. +- `POST /artifacts` and `POST /artifacts/upload` now require + `requireGrant("artifact:*", "create")`, as hub-api's `createGrantRoutes` + requires `create` on `grant:*`. A host must grant its principals `create` + on `artifact:*` for them to keep creating artifacts. An unauthenticated + caller of these two routes now gets `{ "error": "Forbidden" }` instead of + `{ "error": "Tenant not accessible" }`. - `runArtifactMigrations(config, { schema })` takes the same arguments as Interchange's `runMigrations`: a `DBConfig` and the host schema holding `tenant` and `principal`. It applies the SQL files shipped under @@ -135,8 +146,8 @@ always called out under their own heading. `mailAttachmentRef`) are no longer exported from the package entry. Hosts reach artifacts through the routes and functions; `ARTIFACTS_SCHEMA` and the `*Row` types stay public. -- `mountArtifacts` takes `Hono`, reads the host-provided tenant and - principal context natively, and requires the host's Interchange `RequireGrant` +- The tenant routes take `Hono`, read the host-provided tenant and + principal context natively, and require the host's Interchange `RequireGrant` middleware. The `resolvePrincipal`, `isAdmin`, and `identity` options and the `Identity` / `anonymousIdentity` exports are not part of the package surface. - Serialized artifact rows expose `ownerPrincipalId` without an `ownerName`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 557e745..9d4eb1f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -47,7 +47,7 @@ built `dist/` — the same artifact a consumer installs. That is why `test:accep builds first: running it against stale output is how a green acceptance run stops meaning anything. -If you change the mount seam, a port, or anything about how a host wires this up, the +If you change the route factory, a port, or anything about how a host wires this up, the reference host is where that change has to be shown working. ## Dependency rule @@ -103,27 +103,25 @@ Interchange serves its own routes under (`app.route("/api/me", …)`, `app.route("/api/tenants", …)`). No `/v1` segment, no vendor prefix. ```ts -const api = new Hono(); // Host middleware has already placed `tenant` and `principal` on the context. -mountArtifacts(api, { db, contentStore, requireGrant }); -app.route("/api", api); +app.route("/api", createArtifactRoutes({ db, contentStore, requireGrant })); ``` which serves `/api/artifacts`, `/api/artifacts/:id`, `/api/artifacts/:id/versions`, `/api/artifacts/:id/download`, and -`/api/instances/:instanceId/mail-attachments`. Nesting rather than teaching the -core a base path keeps the mount free of a configurable base path. +`/api/instances/:instanceId/mail-attachments`. Returning a sub-app rather than +taking a base path keeps the factory free of a configurable base path. -Everything else it needs arrives through `opts` or the host's request context. +Everything else it needs arrives through `deps` or the host's request context. Nothing is reached for. -### The mount seam +### The route factory -`mountArtifacts(app: Hono, opts): Hono` takes Interchange's -`TenantEnv` so it composes with a host app mounted beneath Interchange auth + -tenant middleware. The host places full `tenant` and `principal` rows on the -context; this package reads them natively and never invents a second principal -resolution path. +`createArtifactRoutes(deps): Hono` returns a sub-app typed with +Interchange's `TenantEnv`, built the way hub-api's `createGrantRoutes` is, so it +composes beneath Interchange auth + tenant middleware. The host places full +`tenant` and `principal` rows on the context; this package reads them natively +and never invents a second principal resolution path. Three options have no sensible default — `db`, `contentStore`, `requireGrant` — and the rest degrade a *feature*, never safety, when omitted. The README's option @@ -185,7 +183,9 @@ invents authorization policy nor decides who a newly created row belongs to for grant purposes — it hands the host the row, inside the transaction that made it durable, and the host decides. -`examples/reference-host` provisions a real `creator`-origin grant on create — +Creating needs its own grant, `create` on `artifact:*`, checked before the +body is read; the reference host seeds it for every principal in its tenant. +`examples/reference-host` then provisions a real `creator`-origin grant on create — `write` and `archive` on `artifact:` for the creating principal, inserted into Interchange's own `grant` table via `@intx/db`'s schema, in the same transaction as the artifact row. Its `buildApp`'s default `requireGrant` is the diff --git a/README.md b/README.md index 0c3fadf..ca98989 100644 --- a/README.md +++ b/README.md @@ -26,51 +26,34 @@ const { db, close } = createArtifactDb(process.env.DATABASE_URL!); await close(); ``` -### 1. Hub-side, tenant-scoped: `mountArtifacts` +### 1. Hub-side, tenant-scoped: `createArtifactRoutes` -Mounted under the hub's tenant prefix, alongside a host's other session-authenticated routes. It reads `principal`/`tenant` off the Hono context (placed there by the host's own auth + tenant middleware) and authorizes mutations through the host's `requireGrant` — built from Interchange's `createRequireGrant` over the host's own `GrantStore` and `ConditionRegistry`. +Returns a `Hono` sub-app the host mounts with `app.route`, alongside its other session-authenticated routes. It reads `principal`/`tenant` off the Hono context (placed there by the host's own auth + tenant middleware) and authorizes mutations through the host's `requireGrant` — built from Interchange's `createRequireGrant` over the host's own `GrantStore` and `ConditionRegistry`. -| `opts` | Type | What the host provides | +| `deps` | Type | What the host provides | | --- | --- | --- | | `db` | `ArtifactDb` | Artifacts are stored there. `createArtifactDb` opens a handle for a host with none; a hub that already has one passes it through. | | `contentStore` | `ContentStore` | Blob storage for file bytes. `InlineContentStore` (exported by this package) fits a minimal host; bring your own store for object storage. | -| `requireGrant` | `RequireGrant` | The host's grant middleware factory. This package implements no ownership or membership policy of its own — every mutating single-artifact route is gated through it. | +| `requireGrant` | `RequireGrant` | The host's grant middleware factory. This package implements no ownership or membership policy of its own. Creating an artifact requires `create` on `artifact:*`; revising or archiving one requires `write` or `archive` on `artifact:`. Recording mail-attachment references needs only a principal. | | `countSegments` | `ArtifactCountSegments` (optional) | Named predicates over `ArtifactListRow` for `GET /artifacts/counts` (e.g. bucket by `kind`). The taxonomy is entirely host-owned; omitted, the route still answers with the tenant-wide `all` total. | | `onArtifactCreated` | `(tx, row, scope) => Promise` (optional) | Runs inside the transaction that creates each artifact. This is where the host mints grants for the new row, e.g. `write` and `archive` on `artifact:` for its creator. The package mints none itself. | | `decorate` | `(tenantId, rows) => Promise` (optional) | Adds display-only fields to serialized rows on the way out (provenance labels, host joins). It must never change which rows are returned or who may see them. | | `uploadPolicy` | `UploadPolicy` (optional) | Which MIME types `POST /artifacts/upload` accepts. Defaults to `ARTIFACT_UPLOAD_POLICY`. | ```ts -import type { Hono } from "hono"; -import type { TenantEnv } from "@intx/hub-api"; import { createRequireGrant } from "@intx/hub-api"; -import type { ConditionRegistry, GrantStore } from "@intx/types/authz"; -import { - InlineContentStore, - mountArtifacts, - type ArtifactDb, - type ArtifactCountSegments, -} from "@corbits/artifacts"; - -export function mountArtifactRoutes( - app: Hono, - deps: { - db: ArtifactDb; - grantStore: GrantStore; - conditionRegistry: ConditionRegistry; - countSegments?: ArtifactCountSegments; - }, -): void { - mountArtifacts(app, { - db: deps.db, +import { createArtifactRoutes, InlineContentStore } from "@corbits/artifacts"; + +// `app` is the host's Hono; its auth + tenant middleware has +// already placed `tenant` and `principal` on the context. +app.route( + "/api", + createArtifactRoutes({ + db, contentStore: InlineContentStore, - requireGrant: createRequireGrant({ - grantStore: deps.grantStore, - conditionRegistry: deps.conditionRegistry, - }), - ...(deps.countSegments !== undefined ? { countSegments: deps.countSegments } : {}), - }); -} + requireGrant: createRequireGrant({ grantStore, conditionRegistry }), + }), +); ``` ### 2. Hub-side, run-scoped: `mountWorkflowArtifacts` diff --git a/examples/reference-host/src/index.ts b/examples/reference-host/src/index.ts index ec869cb..cec62eb 100644 --- a/examples/reference-host/src/index.ts +++ b/examples/reference-host/src/index.ts @@ -40,7 +40,7 @@ import { } from "@intx/hub-sessions"; import { InlineContentStore, - mountArtifacts, + createArtifactRoutes, runArtifactMigrations, type ArtifactDb, type ArtifactRow, @@ -79,7 +79,7 @@ async function decorate(_tenantId: string, rows: readonly SerializedArtifactBase * The worked example this host owes the next `@corbits/*-core` package: what * a "the artifact's owner may write to it" grant actually IS, and who mints * it. `@corbits/artifacts` provisions nothing itself — this runs through - * `mountArtifacts`'s `onArtifactCreated` hook, inside the same transaction as + * `createArtifactRoutes`' `onArtifactCreated` hook, inside the same transaction as * the row it grants on, so a grant never outlives (or fails to accompany) the * artifact it names. * @@ -242,6 +242,23 @@ export async function createReferenceHost(): Promise { (p) => p.kind === "agent" && p.refId === "user-alice", )!.id; + // The routes check `artifact:*` / `create` before any create, the same way + // hub-api checks `grant:*` / `create` before minting a grant. This demo + // grants it to the principals it seeds here; a real host decides who may. + await db.insert(intxSchema.grant).values( + principals.map((principal) => ({ + id: generateId("grant"), + tenantId: tenant.id, + principalId: principal.id, + roleId: null, + resource: "artifact:*", + action: "create", + effect: "allow" as const, + origin: "system" as const, + conditions: null, + })), + ); + let currentSession: Session = { userId: "user-alice" }; const getSession = async (_headers: Headers) => { @@ -304,12 +321,13 @@ export async function createReferenceHost(): Promise { }); // Mounted @corbits/* modules serve under `/api`, matching Interchange's // own convention (`app.route("/api/me", …)`). The core registers its - // routes root-relative (`/artifacts*`, `/instances/:id/mail-attachments`), - // so the host nests them in a sub-app and routes that at `/api`. Served - // paths: `/api/artifacts*` — no `/v1` segment, no vendor prefix. + // routes root-relative (`/artifacts*`, `/instances/:id/mail-attachments`). + // The host wraps them in its own `api` sub-app so the principal middleware + // below stays scoped to `/api`. Served paths: `/api/artifacts*` — no `/v1` + // segment, no vendor prefix. const api = new Hono(); // Place full tenant/principal rows from the hub session user. Signed-out - // (or unknown) callers leave the context empty so mountArtifacts applies + // (or unknown) callers leave the context empty so the artifact routes apply // the no-principal contract. api.use("*", async (c, next) => { const user = c.get("user"); @@ -351,13 +369,16 @@ export async function createReferenceHost(): Promise { } return next(); }; - mountArtifacts(api, { - db, - contentStore, - requireGrant, - decorate, - onArtifactCreated: grantOwnership, - }); + api.route( + "/", + createArtifactRoutes({ + db, + contentStore, + requireGrant, + decorate, + onArtifactCreated: grantOwnership, + }), + ); const mounted = app.route("/api", api); // `Hono#request` may answer synchronously; normalize to a promise so every // caller can simply await it. diff --git a/src/index.ts b/src/index.ts index bcc64c9..6569673 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,6 @@ // @corbits/artifacts — a backend-only, mountable artifact + upload store. -export { mountArtifacts } from "./mount.js"; -export type { MountArtifactsOpts } from "./mount.js"; +export { createArtifactRoutes } from "./mount.js"; +export type { CreateArtifactRoutesDeps } from "./mount.js"; export { mountWorkflowArtifacts } from "./workflow-mount.js"; export type { diff --git a/src/mount.test.ts b/src/mount.test.ts index b751896..3e5e02c 100644 --- a/src/mount.test.ts +++ b/src/mount.test.ts @@ -4,7 +4,7 @@ import { Hono } from "hono"; import { createRequireGrant, type RequireGrant, type TenantEnv } from "@intx/hub-api"; import { createInMemoryGrantStore } from "@intx/authz"; import type { GrantRule } from "@intx/types/authz"; -import { mountArtifacts } from "./mount.js"; +import { createArtifactRoutes } from "./mount.js"; import { InlineContentStore } from "./content-store.js"; import { listArtifacts, @@ -18,7 +18,7 @@ import { MAX_UPLOAD_TOTAL_BYTES, } from "./uploads.js"; import type { ArtifactDb } from "./db.js"; -import type { MountArtifactsOpts } from "./mount.js"; +import type { CreateArtifactRoutesDeps } from "./mount.js"; import type { ResolvedPrincipal } from "./ports.js"; import { seedArtifact, seedSkillDraft, SCOPE, testDb } from "./test-helpers.js"; @@ -70,9 +70,9 @@ function grantRule(over: Partial & Pick boolean; - decorate?: MountArtifactsOpts["decorate"]; - contentStore?: MountArtifactsOpts["contentStore"]; - countSegments?: MountArtifactsOpts["countSegments"]; + decorate?: CreateArtifactRoutesDeps["decorate"]; + contentStore?: CreateArtifactRoutesDeps["contentStore"]; + countSegments?: CreateArtifactRoutesDeps["countSegments"]; }; function host(db: ArtifactDb, opts: HostOpts = {}) { @@ -122,13 +122,16 @@ function host(db: ArtifactDb, opts: HostOpts = {}) { } return next(); }; - return mountArtifacts(app, { - db, - contentStore: opts.contentStore ?? InlineContentStore, - requireGrant, - ...(opts.decorate ? { decorate: opts.decorate } : {}), - ...(opts.countSegments ? { countSegments: opts.countSegments } : {}), - }); + return app.route( + "/", + createArtifactRoutes({ + db, + contentStore: opts.contentStore ?? InlineContentStore, + requireGrant, + decorate: opts.decorate ?? (async () => {}), + countSegments: opts.countSegments ?? {}, + }), + ); } const json = (body: unknown) => ({ @@ -249,7 +252,7 @@ describe("POST /artifacts", () => { expect({ body, status: res.status, json: await res.json() }).toEqual({ body, status: 403, - json: { error: "Tenant not accessible" }, + json: { error: "Forbidden" }, }); } }); @@ -915,11 +918,14 @@ describe("authorization through the real platform grant evaluator", () => { grantStore: createInMemoryGrantStore(grants), conditionRegistry: {}, }); - return mountArtifacts(withPrincipal(new Hono(), principal), { - db, - contentStore: InlineContentStore, - requireGrant, - }); + return withPrincipal(new Hono(), principal).route( + "/", + createArtifactRoutes({ + db, + contentStore: InlineContentStore, + requireGrant, + }), + ); } test("the owner's creator-origin grant allows write; a co-tenant with no grant is refused", async () => { @@ -949,6 +955,30 @@ describe("authorization through the real platform grant evaluator", () => { expect(current!.content).toBe("v2"); }); + test("creating needs a create grant on artifact:*; without one, nothing is written", async () => { + const db = await testDb(); + const grants = [ + grantRule({ resource: "artifact:*", action: "create", principalId: OWNER.principalId }), + ]; + const body = { mode: "text", title: "Gated", content: "body" }; + const form = () => { + const f = new FormData(); + f.append("files", new File(["a"], "a.txt", { type: "text/plain" })); + return { method: "POST", body: f }; + }; + + const granted = hostWithGrants(db, OWNER, grants); + const ungranted = hostWithGrants(db, NON_OWNER, grants); + + expect((await granted.request("/artifacts", json(body))).status).toBe(201); + expect((await granted.request("/artifacts/upload", form())).status).toBe(201); + expect((await ungranted.request("/artifacts", json(body))).status).toBe(403); + expect((await ungranted.request("/artifacts/upload", form())).status).toBe(403); + + const rows = await listArtifacts(db, SCOPE.tenantId, {}); + expect(rows.rows.length).toBe(2); + }); + test("a grant for the wrong action does not authorize a different one", async () => { const db = await testDb(); const row = await seedArtifact(db); @@ -1014,21 +1044,23 @@ describe("authorization through the real platform grant evaluator", () => { */ describe("onArtifactCreated: the host's grant-provisioning seam", () => { // These tests are about the hook, not authorization, so the grant check - // itself is a trivial always-allow — `requireGrant` isn't even reached by - // POST /artifacts or /artifacts/upload, which authorize nothing on create. + // itself is a trivial always-allow. const allowAll: RequireGrant = () => async (_c, next) => next(); test("runs once with the created row and the creating scope", async () => { const db = await testDb(); const seen: { row: { id: string }; scope: ResolvedPrincipal }[] = []; - const app = mountArtifacts(withPrincipal(new Hono(), SCOPE), { - db, - contentStore: InlineContentStore, - requireGrant: allowAll, - onArtifactCreated: async (_tx, row, scope) => { - seen.push({ row: { id: row.id }, scope }); - }, - }); + const app = withPrincipal(new Hono(), SCOPE).route( + "/", + createArtifactRoutes({ + db, + contentStore: InlineContentStore, + requireGrant: allowAll, + onArtifactCreated: async (_tx, row, scope) => { + seen.push({ row: { id: row.id }, scope }); + }, + }), + ); const res = await app.request( "/artifacts", @@ -1040,14 +1072,17 @@ describe("onArtifactCreated: the host's grant-provisioning seam", () => { test("a throw inside the hook rolls back the artifact insert — no orphan row", async () => { const db = await testDb(); - const app = mountArtifacts(withPrincipal(new Hono(), SCOPE), { - db, - contentStore: InlineContentStore, - requireGrant: allowAll, - onArtifactCreated: async () => { - throw new Error("grant store is down"); - }, - }); + const app = withPrincipal(new Hono(), SCOPE).route( + "/", + createArtifactRoutes({ + db, + contentStore: InlineContentStore, + requireGrant: allowAll, + onArtifactCreated: async () => { + throw new Error("grant store is down"); + }, + }), + ); await app.request("/artifacts", json({ mode: "text", title: "Orphan?", content: "body" })); const rows = await listArtifacts(db, SCOPE.tenantId, {}); @@ -1057,14 +1092,17 @@ describe("onArtifactCreated: the host's grant-provisioning seam", () => { test("runs once per file on the upload route", async () => { const db = await testDb(); const ids: string[] = []; - const app = mountArtifacts(withPrincipal(new Hono(), SCOPE), { - db, - contentStore: InlineContentStore, - requireGrant: allowAll, - onArtifactCreated: async (_tx, row) => { - ids.push(row.id); - }, - }); + const app = withPrincipal(new Hono(), SCOPE).route( + "/", + createArtifactRoutes({ + db, + contentStore: InlineContentStore, + requireGrant: allowAll, + onArtifactCreated: async (_tx, row) => { + ids.push(row.id); + }, + }), + ); const form = new FormData(); form.append("files", new File(["a"], "a.txt", { type: "text/plain" })); diff --git a/src/mount.ts b/src/mount.ts index 23374ea..1ab28c2 100644 --- a/src/mount.ts +++ b/src/mount.ts @@ -1,6 +1,6 @@ import "./arktype.js"; import { type } from "arktype"; -import type { Context, Hono } from "hono"; +import { Hono, type Context } from "hono"; import type { MiddlewareHandler } from "hono"; import { describeRoute } from "hono-openapi"; import { idResource, type RequireGrant, type TenantEnv } from "@intx/hub-api"; @@ -58,13 +58,15 @@ import { } from "./uploads.js"; import { WebSiteContentError } from "./web-site.js"; -export type MountArtifactsOpts = { +export type CreateArtifactRoutesDeps = { db: ArtifactDb; contentStore: ContentStore; /** * The host's grant middleware factory (Interchange `createRequireGrant`). * Authorize is the host's responsibility: artifact-core implements no owner, - * agent-owner, membership, or admin policy. The mutating routes that act on + * agent-owner, membership, or admin policy. Creating an artifact + * (`POST /artifacts`, `POST /artifacts/upload`) requires + * `requireGrant("artifact:*", "create")`; the mutating routes that act on * one artifact are guarded with `requireGrant(idResource("artifact", "id"), * )`. */ @@ -177,6 +179,10 @@ const ReviseArtifactRequest = type({ ctx.mustBe("a body with content, title, and/or metadata"), ); +/** The resource creating an artifact is authorized against, as hub-api's + * `createGrantRoutes` authorizes creating a grant against `grant:*`. */ +const ARTIFACT_COLLECTION = "artifact:*"; + const idParam = { name: "id", in: "path", @@ -195,31 +201,28 @@ const VersionRef = type("string").pipe((raw, ctx) => { }); /** - * Mount the artifact routes onto a host Hono app. + * Build the artifact routes as a sub-app the host mounts with `app.route`. * - * Takes `Hono` so it composes with a host app mounted beneath - * Interchange's auth + tenant middleware, which puts the resolved `tenant` and - * `principal` on the context. The host owns principal resolution and grants; - * this package reads the principal from context and authorizes the mutating - * routes through the host's `requireGrant`. + * Typed `Hono` so it composes beneath Interchange's auth + tenant + * middleware, which puts the resolved `tenant` and `principal` on the context. + * The host owns principal resolution and grants; this package reads the + * principal from context and authorizes the mutating routes through the + * host's `requireGrant`. * * With no principal on the context, collection reads answer an empty 200 while * detail reads and mutations answer 403 — the same rule every `@corbits/*-core` * package follows. See "No principal on the context" in the README for why. */ -export function mountArtifacts( - app: Hono, - opts: MountArtifactsOpts, -): Hono { - const { - db, - contentStore, - requireGrant, - decorate = async () => {}, - onArtifactCreated = async () => {}, - uploadPolicy = ARTIFACT_UPLOAD_POLICY, - countSegments = {}, - } = opts; +export function createArtifactRoutes({ + db, + contentStore, + requireGrant, + decorate = async () => {}, + onArtifactCreated = async () => {}, + uploadPolicy = ARTIFACT_UPLOAD_POLICY, + countSegments = {}, +}: CreateArtifactRoutesDeps): Hono { + const app = new Hono(); // The handler context is TenantEnv. `principal` (and `tenant`) is placed by // Interchange's middleware; nothing here resolves it. @@ -421,15 +424,18 @@ export function mountArtifacts( responses: { 201: { description: "Artifact created" }, 400: { description: "Invalid request body" }, - 403: { description: "Tenant not accessible" }, + 403: { description: "No resolvable principal, or not permitted" }, 413: { description: "Declared Content-Length over the content ceiling" }, }, }), + // Principal and grant before body: an unauthenticated or unpermitted + // caller gets 403 without learning whether the JSON was well-formed. + principalRequired, + requireGrant(ARTIFACT_COLLECTION, "create"), async (c) => { - // Principal before body: unauthenticated callers get 403 without learning - // whether the JSON was well-formed. const scope = await scopeFor(c); - if (!scope) return c.json({ error: "Tenant not accessible" }, 403); + // principalRequired already ran; a null scope here would mean it didn't. + if (!scope) return c.json({ error: "Forbidden" }, 403); if (contentLengthOverCeiling(c)) { return c.json( { @@ -486,14 +492,17 @@ export function mountArtifacts( responses: { 201: { description: "Artifacts created" }, 400: { description: "No files supplied" }, - 403: { description: "Tenant not accessible" }, + 403: { description: "No resolvable principal, or not permitted" }, 413: { description: "Too many files, or a file/aggregate over the limit" }, 415: { description: "A file has an unsupported type" }, }, }), + principalRequired, + requireGrant(ARTIFACT_COLLECTION, "create"), async (c) => { const scope = await scopeFor(c); - if (!scope) return c.json({ error: "Tenant not accessible" }, 403); + // principalRequired already ran; a null scope here would mean it didn't. + if (!scope) return c.json({ error: "Forbidden" }, 403); const parsed = await c.req.parseBody({ all: true }); const files: File[] = []; diff --git a/src/workflow-mount.ts b/src/workflow-mount.ts index 5880b59..2ebb13c 100644 --- a/src/workflow-mount.ts +++ b/src/workflow-mount.ts @@ -1,11 +1,11 @@ /** - * Run-scoped variant of `mountArtifacts` for a host whose workflow-run - * callers have no browser session and thus no `TenantEnv`/`principal` on the - * context — a sidecar bearer token + run address instead. `mountArtifacts` - * itself has no bearer-token auth surface at all and never will: mixing two - * unrelated auth conventions into one mount would make each harder to reason - * about, so this is a second, parallel mount a host wires up only when it - * actually runs workflows. + * Run-scoped counterpart to the tenant routes in `createArtifactRoutes`, for a + * host whose workflow-run callers have no browser session and thus no + * `TenantEnv`/`principal` on the context — a sidecar bearer token + run address + * instead. The tenant routes themselves have no bearer-token auth surface at + * all and never will: mixing two unrelated auth conventions into one mount + * would make each harder to reason about, so this is a second, parallel mount + * a host wires up only when it actually runs workflows. * * The host supplies `resolveRunScope`, a function from `(bearerToken, * runAddress)` to a resolved run scope or `null` — exactly how it already @@ -107,7 +107,7 @@ export type MountWorkflowArtifactsOpts = { /** Optional second authentication path, tried before the sidecar token. */ agentToken?: AgentTokenAuth; /** Which files `POST /artifacts/binary` accepts. Defaults to the same - * policy `mountArtifacts`' `POST /artifacts/upload` uses. */ + * policy `createArtifactRoutes`' `POST /artifacts/upload` uses. */ uploadPolicy?: UploadPolicy; /** Byte ceiling for `POST /artifacts/binary`. Defaults to `MAX_UPLOAD_BYTES` * — the same per-file ceiling the tenant upload route enforces, so there is @@ -195,7 +195,7 @@ function parseRecentLimit(raw: string | undefined): number { /** * Mount the run-scoped artifact routes onto a host Hono app. Unlike - * `mountArtifacts`, every route here is behind its own bearer-token + * `createArtifactRoutes`, every route here is behind its own bearer-token * middleware — there is no unauthenticated collection-read case, since a * workflow run always presents credentials. */ @@ -484,8 +484,8 @@ export function mountWorkflowArtifacts( const scope = c.get("workflowRunScope"); const artifactId = c.req.param("id"); const row = await getArtifact(db, artifactId); - // Fetch-then-check, exactly mirroring `mountArtifacts`' own single-artifact - // routes: an id from another tenant, or a skill-draft, reads back + // Fetch-then-check, exactly mirroring the tenant routes' single-artifact + // handlers: an id from another tenant, or a skill-draft, reads back // identically to an id that never existed — never a distinguishable 403. if (row === null || row.tenantId !== scope.tenantId || row.kind === SKILL_DRAFT_KIND) { return c.json({ error: "Artifact not found" }, 404);