diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index aded1f3..430485c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -24,63 +24,109 @@ 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(); -mountArtifacts(api, { db, contentStore, resolvePrincipal }); +const api = new Hono(); +// Host middleware has already placed `tenant` and `principal` on the context. +mountArtifacts(api, { db, contentStore, requireGrant }); app.route("/api", api); ``` 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 frozen `mountX(app, opts) => Hono` -seam untouched. +core a base path keeps the mount free of a configurable base path. -Everything else it needs arrives through `opts`. Nothing is reached for. +Everything else it needs arrives through `opts` or the host's request context. +Nothing is reached for. ## The mount seam -`mountArtifacts(app: Hono, opts): Hono` is generic over the -host's Hono `Env`, so it composes with an app that carries its own environment -rather than requiring a bare `Hono`. - -Three options have no sensible default — `db`, `contentStore`, -`resolvePrincipal` — and the rest degrade a *feature*, never safety, when -omitted. The README's table of what a minimal host passes is the reference; what -matters architecturally is that every default **fails closed**: no `isAdmin` -means nobody is an admin, no `identity` means no directory and no cross-tenant -reads, no `decorate` means no decoration. - -What the package does **not** require of a host: no auth middleware, no session -library, no UI. What it DOES require: Interchange's control plane — -`public.tenant` and `public.principal` must exist before the migrations run, -because the tables carry hard foreign keys into them. - -`resolvePrincipal`'s signature is identical across the Corbits cores, so a host -mounting more than one passes the same function to each. The resolved tenant is -authoritative — there is no caller-supplied tenant override anywhere in the -route surface. +`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. + +Three options have no sensible default — `db`, `contentStore`, `requireGrant` — +and the rest degrade a *feature*, never safety, when omitted. The README's table +of what a minimal host passes is the reference; what matters architecturally is +that optional seams **fail closed**: no `decorate` means no decoration. + +What the package does **not** require of a host: no session library, no UI, no +directory, no owner/admin policy callback. What it DOES require: Interchange's +control plane — `public.tenant` and `public.principal` must exist before the +migrations run, because the tables carry hard foreign keys into them — and a +host that puts the authenticated principal on `TenantEnv` and hands in its +`RequireGrant`. + +The principal's tenant is authoritative — there is no caller-supplied tenant +override anywhere in the route or tool surface. Tool reads always stay inside +`scope.tenantId`. + +**This is a decision, not an oversight.** The prior `Identity` port let +`readArtifact` / `readArtifactChunk` take a `tenantId` argument and cross into +it when `identity.ownerIsMemberOfTenant(scope, tenantId)` said the caller's +owner belonged there — a membership check this package invented and owned. +That is exactly the kind of policy this PR removes. It is not replaced by a +grant check, and won't be by a later one either: Interchange's `GrantStore` +resolves a principal's grants **within one tenant** +(`collectGrants(principalId, tenantId)`; `@intx/db`'s implementation filters +`grant` rows by `tenant_id`, and a principal is itself a row scoped to one +tenant). There is no platform primitive for "principal P, home tenant A, holds +a grant readable from tenant B" to check — inventing one here would mean this +package building a second, bespoke cross-tenant authorization concept on top +of the platform's, which is the precise failure mode "authorization is the +host's job" is meant to prevent. If a real product need for cross-tenant +artifact reads shows up, it belongs in Interchange's grant model, not +re-derived per package. + +## Three custom seams + +Beyond the host's native context and grants, this package exposes **three** +extension seams: the substrate (`ContentStore`), a display-only decorator +(`decorate` / provenance), and a grant-provisioning hook (`onArtifactCreated`). +Authorization is not a custom seam — it is the host's Interchange `RequireGrant`; +`onArtifactCreated` is not authorization either, it is the write side of the +same idea — the host deciding what makes its grant model true, this package +only handing it the row and the scope that made it. ## The options -`ContentStore` and `Identity` are types declared in `ports.ts`; the rest are -plain `mountArtifacts` options. `resolvePrincipal` takes the host's request -context as `unknown`. +`ContentStore` is declared in `ports.ts`; the rest are plain `mountArtifacts` +options. Who the request runs as is read from `TenantEnv`, not passed as a +callback. | Option | What it is for | Default | | --- | --- | --- | -| `resolvePrincipal` | Who the request runs as. Reads the host session; returns `null` when signed out. | none — required | +| `requireGrant` | Host-owned Interchange grant middleware factory. Single-artifact mutations (revise, archive/unarchive, …) run `requireGrant(idResource("artifact", "id"), )`. | none — required | | `contentStore` | Where an artifact's file bytes live (`ContentStore`). | none — required | -| `isAdmin` | Whether a principal is a tenant admin. Only archive/unarchive consults it. | nobody is an admin | -| `identity` | Owner display names, the agent→human ownership resolution, creator-kind principal sets, and cross-tenant membership (`Identity`). | `anonymousIdentity` | -| `decorate` | A **display-only** decorator over serialized rows. | no-op | +| `decorate` | A **display-only** decorator over serialized rows (provenance labels, host joins). | no-op | +| `onArtifactCreated` | Host hook run inside the same transaction as artifact creation — where a host mints grants for the row it just made. | no-op | `decorate`'s display-only status is a contract, not a convention: it may add fields to rows on their way out and must never affect *what* is returned or *who* may see it. Joining a host's workflow tables inside this package would couple it to a schema it must not know, so the host supplies the decorator. - -`Identity.ownerIsMemberOfTenant` gates cross-tenant reads and must fail closed; -the shipped `anonymousIdentity` does. +Clients that need an owner display name resolve `ownerPrincipalId` themselves; +this package never ships directory names on the wire. + +### Grant provisioning (`onArtifactCreated`) + +Checking a grant (`requireGrant`) and minting one (`onArtifactCreated`) are the +same host responsibility looked at from both ends: this package neither +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 — +`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 +platform's real `createRequireGrant` over that same table (via +`createGrantStore`), not a stub — a principal with no matching row is refused, +exactly as in production. See `grantOwnership` in +`examples/reference-host/src/index.ts` and the "ownership-derived grants" +scenarios in its acceptance suite for the end-to-end proof: the creator +succeeds, a co-tenant with no grant does not. ### ContentStore @@ -107,15 +153,15 @@ which store is installed. | File | Role | | --- | --- | -| `mount.ts` | HTTP surface: parsing, validation, status codes. | +| `mount.ts` | HTTP surface: parsing, validation, status codes; reads `TenantEnv` principal; wires host `requireGrant`. | | `artifacts.ts` | The core domain — create, revise, list, get, archive, serialize. | | `uploads.ts` | `createFileArtifact`, the MIME policies, and the size caps. | | `download.ts` | One download path over the three storage conventions. | | `content-store.ts` | The two shipped `ContentStore` implementations. | -| `tools.ts` | Agent-facing tool definitions and windowed artifact reads. | +| `tools.ts` | Agent-facing tool definitions and windowed artifact reads (caller tenant only). | | `web-site.ts` | The `web-site` kind's content encoding and validation. | | `mail-attachments.ts` | Artifact↔message associations. | -| `ports.ts` | The `ContentStore` and `Identity` types, and the fail-closed `anonymousIdentity` default. | +| `ports.ts` | The `ContentStore` type and the shared `ResolvedPrincipal` shape. | | `schema.ts` / `migrations.ts` | The four tables, and the DDL that creates them. | ## Data model @@ -140,10 +186,10 @@ single-column constraints applied by a ledgered migration — free at write time the control plane independently; it does **not** enforce that `principal_id` (or `owner_principal_id`) belongs to the same tenant as `tenant_id`. A multi-table trigger or composite FK into `public.principal` would couple every write to a -control-plane lookup and is deliberately out of scope. The host's -`resolvePrincipal` is the authority: it returns the `(tenantId, principalId)` -pair every route and tool write stamps, so a correctly mounted host never -plants a cross-tenant principal. Operators cleaning legacy rows before the +control-plane lookup and is deliberately out of scope. The host's middleware and +context are the authority: routes stamp the `(tenantId, principalId)` pair from +the Interchange `principal` already on `TenantEnv`, so a correctly mounted host +never plants a cross-tenant principal. Operators cleaning legacy rows before the `tenant_id NOT NULL` migration must assign a valid tenant or delete orphans — the migration fails with an explicit message if null `tenant_id` rows remain. @@ -234,9 +280,9 @@ upload **gate** (`createFileArtifact` takes `policy` as a required argument and refuses anything outside it before the `ContentStore` is touched); and the download path with its `nosniff`/`attachment` behaviour. -Supplied by the host: the Hono app and the database handle; who the caller is; -whether they are an admin; the directory, if there is one; provenance -decoration; and a `ContentStore`. +Supplied by the host: the `Hono` app and the database handle; the +authenticated `tenant`/`principal` on the request context; the host's +`RequireGrant`; display-only provenance decoration; and a `ContentStore`. ## Known limits diff --git a/CHANGELOG.md b/CHANGELOG.md index f765d38..3cb7048 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,47 @@ 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 + 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. + `examples/reference-host` now wires a real one (`grantOwnership`) against + Interchange's own `grant` table, and its default `requireGrant` is the + platform's real `createRequireGrant` over that table rather than a + default-allow stub — see ARCHITECTURE.md's "Grant provisioning" section. +- Single-artifact write routes (`POST .../versions`, `POST .../archive`, + `POST .../unarchive`) now resolve existence/tenant/skill-draft (the same + check `loadScoped` does) BEFORE running `requireGrant`, not after. A real, + resource-specific grant evaluator has no existence check of its own — it + denies a ghost id or another tenant's artifact with the same `403` it would + give for a real row the caller lacks permission on, which a default-allow + stub can never surface. This restores the documented "a caller who cannot + see the artifact gets 404" guarantee for write routes running a real grant + check, matching what already held for reads. + +### Breaking + +- `mountArtifacts` takes `Hono`, reads the host-provided tenant and + principal context natively, and requires 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`. + Artifact lists no longer accept `creatorKind`. +- **Cross-tenant tool reads are removed, intentionally, not just undocumented.** + `readArtifact` / `readArtifactChunk` no longer take a `tenantId` override; + tool reads are always confined to `scope.tenantId`. The prior override read + through `Identity.ownerIsMemberOfTenant`, a membership policy this package + invented and owned — exactly what this PR removes. It is not replaced by a + grant check because there is no platform primitive to replace it with: + Interchange's `GrantStore` resolves a principal's grants within one tenant + (a principal is itself a row scoped to one tenant), so "grant readable + across tenants" does not exist to check. Reintroducing cross-tenant reads + here would mean this package inventing a second, bespoke cross-tenant + authorization concept on top of the platform's — the failure mode this PR + exists to remove. If a real need for it surfaces, it belongs in + Interchange's grant model, not a per-package workaround. ### 0.1.0 — first release @@ -24,10 +65,10 @@ new; the list below is what the surface consists of rather than what changed. - `mountArtifacts(app, opts)` — mounts artifacts, versions and uploads on a host's existing Hono app: tenant-scoped list with keyset paging and - query/kind/owner/creator-kind/date filters, human import of a link or pasted - text, multipart upload, deep-link detail, version history and revision, - idempotent soft archive and unarchive, a single download path, and - artifact↔message attachment refs. Every route carries OpenAPI metadata. + query/kind/owner/date filters, human import of a link or pasted text, + multipart upload, deep-link detail, version history and revision, idempotent + soft archive and unarchive, a single download path, and artifact↔message + attachment refs. Every route carries OpenAPI metadata. - `runArtifactMigrations(db)` — idempotent, advisory-locked, checksum-guarded, with its own ledger table (`artifacts.migrations`) and silent re-runs. Safe to call on every boot of every replica. All tables live in the package-owned @@ -38,10 +79,10 @@ new; the list below is what the surface consists of rather than what changed. - `ContentStore` port with two shipped implementations, `InlineContentStore` (bytea side-table) and `DataUrlContentStore` (inline `data:` URL), both passing the same suite. -- Host options: `resolvePrincipal`, plus `isAdmin`, `identity` and - `decorate`, each with a fail-closed default. -- Agent-facing tool definitions with windowed artifact reads, and the `web_site` - artifact kind. +- Host options: required `db`, `contentStore`, and `requireGrant`, plus optional + display-only `decorate` and `uploadPolicy` behavior. +- Agent-facing tool definitions with tenant-confined windowed artifact reads, + and the `web_site` artifact kind. - Requires `@intx/*` 0.2.2 or newer, Node 22+ or Bun 1.1+, and Postgres 13+. (`@intx/*` 0.1.2 does not install — its deps pin the unpublished `@intx/*@0.0.0` — and ships raw TypeScript.) diff --git a/README.md b/README.md index 71d41ca..41e6dde 100644 --- a/README.md +++ b/README.md @@ -19,11 +19,12 @@ design rationale behind them. | Minimum `@intx/*` | **0.2.2** | Peer dependencies: `hono`, `hono-openapi`, `drizzle-orm`, `postgres`, `arktype`, -`@intx/types`. They are peers rather than pinned deps because each is shared runtime -state — a Hono app, a drizzle handle, a module-global registry — and a second copy in -the tree does not error, it silently misbehaves. +`@intx/types`, `@intx/hub-api`. They are peers rather than pinned deps because each is +shared runtime state — a Hono app, a drizzle handle, Interchange's `TenantEnv` / +`RequireGrant`, a module-global registry — and a second copy in the tree does not +error, it silently misbehaves. -Your **host** additionally needs `@intx/hub-api`, `@intx/db`, `@intx/hub-sessions` and +Your **host** additionally needs `@intx/db`, `@intx/hub-sessions` and `@intx/hub-common` for `createApp`. This module imports none of them, so they are not peers here and npm will not warn you they are missing. @@ -35,13 +36,15 @@ bun add github:corbitsdev/corbits-artifacts # Or with peers explicitly bun add github:corbitsdev/corbits-artifacts \ - hono hono-openapi drizzle-orm postgres arktype @intx/types@^0.2.2 + hono hono-openapi drizzle-orm postgres arktype \ + @intx/types@^0.2.2 @intx/hub-api@^0.2.2 ``` ```bash # npm / pack npm install @corbits/artifacts \ - hono hono-openapi drizzle-orm postgres arktype @intx/types@^0.2.2 + hono hono-openapi drizzle-orm postgres arktype \ + @intx/types@^0.2.2 @intx/hub-api@^0.2.2 ``` > **Not on npm yet.** Until the first release, consume it from git or an `npm pack` @@ -52,26 +55,24 @@ npm install @corbits/artifacts \ ```ts import { Hono } from "hono"; -import type { AppEnv } from "@intx/hub-api"; +import { createRequireGrant, type TenantEnv } from "@intx/hub-api"; import { InlineContentStore, mountArtifacts, runArtifactMigrations, - type ResolvedPrincipal, } from "@corbits/artifacts"; // Boot-time, once. Idempotent — safe on every boot of every replica. await runArtifactMigrations(hub.db); -const api = new Hono(); +// Host middleware places Interchange `tenant` and `principal` on the context +// before these routes run. Mount takes Hono and reads them natively. +const api = new Hono(); +const requireGrant = createRequireGrant({ grantStore, conditionRegistry }); mountArtifacts(api, { db: hub.db, contentStore: InlineContentStore, - resolvePrincipal(ctx): ResolvedPrincipal | null { - const user = (ctx as Context).get("user"); - if (!user) return null; - return { tenantId: user.tenantId, principalId: user.id }; - }, + requireGrant, }); app.route("/api", api); ``` @@ -81,7 +82,8 @@ serves its own routes under. No `/v1`, no vendor prefix. `examples/reference-host` in this repository is a complete `@intx/hub-api` host with this module mounted and the acceptance suite pointed at it. Start there if you need the -whole `createApp` wiring. +whole `createApp` wiring, including host middleware that sets `tenant`/`principal` and +a host-owned `RequireGrant`. ## The options @@ -92,12 +94,16 @@ never safety. | --- | --- | --- | | `db` | **yes** | The drizzle handle the host already has | | `contentStore` | **yes** | `InlineContentStore` for a minimal host | -| `resolvePrincipal` | **yes** | `(ctx: unknown) => { tenantId, principalId } \| null`. Identical to `@corbits/mailbox-core`'s, so a host mounting both passes one function to both. The resolved tenant is authoritative — no caller-supplied override. | -| `isAdmin` | no | Nobody is an admin — only the owner (and the member behind a producing agent) can archive. Nothing becomes more permissive. | -| `identity` | no | `anonymousIdentity` — `ownerName` is `null`, `?creatorKind=` matches nothing, cross-tenant reads refused. | -| `decorate` | no | No-op — rows carry no decoration. Display-only by contract, so it can never change what is returned or who sees it. | +| `requireGrant` | **yes** | The host's Interchange grant middleware factory. Archive/unarchive and other single-artifact mutations authorize through `requireGrant(idResource("artifact", "id"), …)`. This package implements no owner, agent-owner, membership, or admin policy. | +| `decorate` | no | No-op — rows carry no decoration. Display-only by contract, so it can never change what is returned or who sees it. Clients resolve display names from `ownerPrincipalId` when they need them. | +| `onArtifactCreated` | no | No-op. Runs inside the same transaction as artifact creation, once per row — the seam a host uses to provision grants (e.g. a `creator`-origin grant on `artifact:` for `write`/`archive`) for the row it just made. See `examples/reference-host`'s `grantOwnership` for a worked example against a real grant store. | | `uploadPolicy` | no | `ARTIFACT_UPLOAD_POLICY` — the standard document/image/spreadsheet allowlist. | +Who the request runs as is **not** an option: the host's auth/tenant middleware puts +`tenant` and `principal` on the `TenantEnv` context, and this package reads them. No +principal on the context is the signed-out case (empty collection reads, `403` +everywhere else). + ### Compatibility `MountArtifactsOpts` follows semver: breaking changes to option names or shapes only @@ -107,7 +113,7 @@ happen in major versions. | Surface | Behavior | | --- | --- | -| `GET /api/artifacts` | Tenant-scoped list; query/kind/owner/creatorKind/date filters, keyset cursor, archived toggle. **Discovery only:** each item omits `content` (fetch bodies via detail, download, or tools) | +| `GET /api/artifacts` | Tenant-scoped list; query/kind/owner/date filters, keyset cursor, archived toggle. **Discovery only:** each item omits `content` (fetch bodies via detail, download, or tools) | | `POST /api/artifacts` | Human import — link a URL or paste text | | `POST /api/artifacts/upload` | multipart import. An optional `generatedBy` form field is stored as `source.generatedBy`, a free-form display label nothing here reads back | | `GET /api/artifacts/:id` | Deep link (archived artifacts still load) | @@ -138,14 +144,19 @@ server, reverse proxy) — the package check is a best-effort edge guard, not a for a host-level cap. Upload byte caps remain on the multipart path. **Auth before body:** mutating JSON routes (`POST /api/artifacts`, -`POST /api/artifacts/:id/versions`, `POST …/mail-attachments`) resolve the principal -before parsing the body, so an unauthenticated caller gets 403 without learning whether -the JSON was well-formed. Upload already auth'd first. +`POST /api/artifacts/:id/versions`, `POST …/mail-attachments`) read the principal from +context before parsing the body, so an unauthenticated caller gets 403 without learning +whether the JSON was well-formed. Upload already auth'd first. Single-artifact write +routes run existence/tenant/skill-draft resolution (the same check `loadScoped` does) +before the host's `requireGrant`, and only run `requireGrant` once that has confirmed a +real, visible row — so `requireGrant` is answering "is this permitted," never "does this +exist," and a caller who cannot see the row gets `404` however a real grant evaluator +would answer for it. ### Two response contracts -**No resolvable principal** — the cross-core rule every `@corbits/*` package follows, so -a host mounting several hands its client one policy rather than three: +**No principal on the context** — the cross-core rule every `@corbits/*` package +follows, so a host mounting several hands its client one policy rather than three: | Route class | Response | In this core | | --- | --- | --- | @@ -157,14 +168,15 @@ specific resource, and whether it exists is not an unresolvable caller's to lear core exposes no stream today; the class is listed so a future one is classified by the rule rather than by guesswork. -**Once a principal is resolved**, four causes collapse into one `404 +**Once a principal is on the context**, four causes collapse into one `404 {"error":"Artifact not found"}` on all six single-artifact routes: the id was never minted, the id is not shaped like an id, the row is a `skill-draft`, or the row belongs to another tenant. A cross-tenant `403` would be an existence oracle — any account holder could walk ids and learn which name a real artifact somewhere in the deployment. -Archive/unarchive still answer `403` for a caller who can see the artifact but may not -administer it: an authorization decision about a row known to exist, not a disclosure. +Archive/unarchive still answer `403` when the host's `requireGrant` denies the +`archive` action on a row the caller can see: an authorization decision about a row +known to exist, not a disclosure. ## Uploads: three allowlists, and who owns each @@ -228,8 +240,9 @@ package-owned `artifacts` Postgres schema. `tenant_id` is required (`NOT NULL`) with the principal columns, is a hard foreign key into Interchange's `public.tenant` / `public.principal`, so the host's own migrations must run first. Version columns CHECK ≥ 1; size columns CHECK ≥ 0. Whether a principal belongs to -the stamped tenant is **host-owned** via `resolvePrincipal` — the package does not -install multi-table triggers for that alignment (see ARCHITECTURE.md). +the stamped tenant is **host-owned** via the middleware that places `tenant` and +`principal` on the request context — the package does not install multi-table triggers +for that alignment (see ARCHITECTURE.md). `runArtifactMigrations(db)` is idempotent, advisory-locked, creates and owns the `artifacts` Postgres schema, and keeps its own ledger diff --git a/bun.lock b/bun.lock index e4637af..820ec9e 100644 --- a/bun.lock +++ b/bun.lock @@ -8,6 +8,7 @@ "@hono/standard-validator": "^0.2.3", }, "devDependencies": { + "@intx/hub-api": "0.2.2", "@intx/types": "0.2.2", "@types/bun": "1.1.14", "@types/node": "22.10.5", @@ -19,6 +20,7 @@ "typescript": "5.7.2", }, "peerDependencies": { + "@intx/hub-api": "^0.2.2", "@intx/types": "^0.2.2", "arktype": "^2.1.29", "drizzle-orm": "^0.45.2", diff --git a/examples/reference-host/src/index.ts b/examples/reference-host/src/index.ts index 5770406..99ba762 100644 --- a/examples/reference-host/src/index.ts +++ b/examples/reference-host/src/index.ts @@ -2,21 +2,27 @@ // (`createApp` from the published `@intx/hub-api`) against a real Postgres. // // The host is the real thing: hub routes, the hub request logger and the hub -// session middleware are all live, and the artifact principal is resolved out -// of the hub's own request context (`c.var.user`) rather than a local variable. -// The identity/authz/decorate options are implemented against the host's OWN control plane -// (interchange `principal` / `user` rows), which is the point: the module knows -// nothing about them, the host supplies them. +// session middleware are all live. Artifact routes nest under `/api` on a +// `Hono` that places the seeded tenant/principal on the context +// from the hub session user, and authorizes through the platform's real +// `createRequireGrant` backed by the real `grant` table — a caller with no +// matching grant row is refused, same as production. `buildApp`'s optional +// `authorize` still lets a test force a specific answer without provisioning +// rows for it. // // This module only BUILDS the host. The acceptance scenarios live in // `test/acceptance.test.ts` and run under `bun test`, so they are collected by // CI like any other test instead of being a hand-rolled assert script nothing // executes. -import { and, eq, inArray, sql } from "drizzle-orm"; +import { sql } from "drizzle-orm"; import { Hono } from "hono"; -import type { Context } from "hono"; -import { createApp, type AppEnv } from "@intx/hub-api"; -import { createDB, runMigrations, schema as intxSchema } from "@intx/db"; +import { + createApp, + createRequireGrant, + type RequireGrant, + type TenantEnv, +} from "@intx/hub-api"; +import { createDB, createGrantStore, runMigrations, schema as intxSchema } from "@intx/db"; // Interchange owns its id scheme; the host mints its OWN control-plane rows // with it rather than inventing a second one. import { generateId } from "@intx/hub-common"; @@ -31,9 +37,10 @@ import { mountArtifacts, runArtifactMigrations, type ArtifactDb, + type ArtifactRow, + type ArtifactTx, type ResolvedPrincipal, type ContentStore, - type Identity, type SerializedArtifactBase, } from "@corbits/artifacts"; @@ -54,81 +61,6 @@ function parsePostgresUrl(raw: string) { }; } -/** Identity, implemented against the host's own directory tables. */ -function createIdentity(db: ArtifactDb): Identity { - return { - async ownerNames(tenantId, ownerPrincipalIds) { - const principals = await db - .select({ id: intxSchema.principal.id, refId: intxSchema.principal.refId }) - .from(intxSchema.principal) - .where( - and( - eq(intxSchema.principal.tenantId, tenantId), - inArray(intxSchema.principal.id, ownerPrincipalIds), - ), - ); - const refIds = [...new Set(principals.map((p) => p.refId))]; - const users = - refIds.length > 0 - ? await db - .select({ id: intxSchema.user.id, name: intxSchema.user.name }) - .from(intxSchema.user) - .where(inArray(intxSchema.user.id, refIds)) - : []; - const nameByRefId = new Map(users.map((u) => [u.id, u.name])); - return new Map(principals.map((p) => [p.id, nameByRefId.get(p.refId) ?? null])); - }, - - async ownerMemberPrincipalId(scope) { - // An agent principal's refId names the human who owns it in this host. - const [agent] = await db - .select({ refId: intxSchema.principal.refId }) - .from(intxSchema.principal) - .where( - and( - eq(intxSchema.principal.id, scope.principalId), - eq(intxSchema.principal.tenantId, scope.tenantId), - eq(intxSchema.principal.kind, "agent"), - ), - ) - .limit(1); - if (!agent) return null; - const [member] = await db - .select({ id: intxSchema.principal.id }) - .from(intxSchema.principal) - .where( - and( - eq(intxSchema.principal.tenantId, scope.tenantId), - eq(intxSchema.principal.kind, "user"), - eq(intxSchema.principal.refId, agent.refId), - eq(intxSchema.principal.status, "active"), - ), - ) - .limit(1); - return member?.id ?? null; - }, - - async principalIdsByKind(tenantId, kind) { - const rows = await db - .select({ id: intxSchema.principal.id }) - .from(intxSchema.principal) - .where( - and( - eq(intxSchema.principal.tenantId, tenantId), - eq(intxSchema.principal.kind, kind), - ), - ); - return rows.map((r) => r.id); - }, - - // This host has exactly one tenant, so a cross-tenant read is always - // refused. A multi-tenant host would check active membership there. - async ownerIsMemberOfTenant() { - return false; - }, - }; -} - /** Display-only decorator. Adds a label, never changes what is returned. */ async function decorate(_tenantId: string, rows: readonly SerializedArtifactBase[]) { for (const row of rows) { @@ -137,6 +69,36 @@ 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 + * the row it grants on, so a grant never outlives (or fails to accompany) the + * artifact it names. + * + * `origin: "creator"` is the platform's own vocabulary for exactly this case + * (see `@intx/types/authz`'s `GrantRule.origin`) — the host is not inventing + * a policy layer, it is recording, in the platform's own grant table, the + * one fact this module already decided: `scope.principalId` made this row. + */ +async function grantOwnership(tx: ArtifactTx, row: ArtifactRow, scope: ResolvedPrincipal) { + const resource = `artifact:${row.id}`; + await tx.insert(intxSchema.grant).values( + (["write", "archive"] as const).map((action) => ({ + id: generateId("grant"), + tenantId: scope.tenantId, + principalId: scope.principalId, + roleId: null, + resource, + action, + effect: "allow" as const, + origin: "creator" as const, + conditions: null, + })), + ); +} + export type Session = { userId: string } | null; export type ReferenceHost = { @@ -146,20 +108,20 @@ export type ReferenceHost = { /** Principal id of the agent Alice owns. */ agentPrincipal: string; /** - * The scope a host-owned surface runs as — the same `ResolvedPrincipal` the - * mounted routes resolve for Alice. A host that owns its own file-minting - * route (a chat attachment divert, a workflow's generated PDF) calls + * The scope a host-owned surface runs as — the same principal the mounted + * routes resolve for Alice. A host that owns its own file-minting route + * (a chat attachment divert, a workflow's generated PDF) calls * `createFileArtifact` with this. */ scope: () => ResolvedPrincipal; /** Who the hub's session middleware will report. `null` means signed out. */ setSession: (session: Session) => void; - /** Request the default host (InlineContentStore, nobody is admin). */ + /** Request the default host (InlineContentStore, real DB-backed grants). */ request: (path: string, init?: RequestInit) => Promise; - /** Build another host over a different ContentStore or authz answer. */ + /** Build another host over a different ContentStore or grant answer. */ buildApp: ( contentStore: ContentStore, - isAdmin: () => Promise, + authorize?: (resource: string, action: string) => boolean, ) => { request: (path: string, init?: RequestInit) => Promise }; close: () => Promise; }; @@ -218,9 +180,8 @@ export async function createReferenceHost(): Promise { await db.execute(sql`DELETE FROM "user" WHERE "id" IN ('user-alice', 'user-bob')`); // Seed the host's own control plane: a tenant, two humans, and one agent - // owned by Alice. Note the contrast — interchange's `principal` has a real FK - // to `tenant`, while @corbits/artifacts holds the tenant BY VALUE and so needs - // nothing to exist here at all. + // owned by Alice. Returning full rows so TenantEnv middleware can place them + // on the request context without a second lookup. const [tenantRow] = await db .insert(intxSchema.tenant) .values({ @@ -229,8 +190,8 @@ export async function createReferenceHost(): Promise { slug: "reference", domain: "reference.example", }) - .returning({ id: intxSchema.tenant.id }); - const tenant = tenantRow!.id; + .returning(); + const tenant = tenantRow!; await db.insert(intxSchema.user).values([ { @@ -246,31 +207,27 @@ export async function createReferenceHost(): Promise { .values([ { id: generateId("principal"), - tenantId: tenant, + tenantId: tenant.id, kind: "user", refId: "user-alice", status: "active", }, { id: generateId("principal"), - tenantId: tenant, + tenantId: tenant.id, kind: "user", refId: "user-bob", status: "active", }, { id: generateId("principal"), - tenantId: tenant, + tenantId: tenant.id, kind: "agent", refId: "user-alice", status: "active", }, ]) - .returning({ - id: intxSchema.principal.id, - kind: intxSchema.principal.kind, - refId: intxSchema.principal.refId, - }); + .returning(); const agentPrincipal = principals.find( (p) => p.kind === "agent" && p.refId === "user-alice", @@ -301,15 +258,6 @@ export async function createReferenceHost(): Promise { }; }; - // The exact signature @corbits/mailbox-core takes, so a host mounting both - // cores hands the same function to both. - function resolvePrincipal(ctx: unknown): ResolvedPrincipal | null { - const user = (ctx as Context).get("user"); - if (!user) return null; - const principal = principals.find((p) => p.kind === "user" && p.refId === user.id); - return principal ? { tenantId: tenant, principalId: principal.id } : null; - } - // A bare Interchange host: real sidecar router, real event-collector // registry. It runs no agent sessions, so its SessionService refuses every // launch verb rather than pretending to serve it. @@ -329,10 +277,11 @@ export async function createReferenceHost(): Promise { endSession: refuse("endSession"), }; - const identity = createIdentity(db); - /** Build a host app with one ContentStore backend mounted. */ - function buildApp(contentStore: ContentStore, isAdmin: () => Promise) { + function buildApp( + contentStore: ContentStore, + authorize?: (resource: string, action: string) => boolean, + ) { const app = createApp({ getSession, authHandler: () => new Response("", { status: 404 }), @@ -349,14 +298,56 @@ export async function createReferenceHost(): Promise { // 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. - const api = new Hono(); + 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 + // the no-principal contract. + api.use("*", async (c, next) => { + const user = c.get("user"); + if (user) { + const principal = principals.find( + (p) => p.kind === "user" && p.refId === user.id && p.status === "active", + ); + if (principal) { + c.set("tenant", tenant); + c.set("principal", principal); + } + } + await next(); + }); + // The real evaluator: the platform's own `createRequireGrant`, backed by + // the real `grant` table via `createGrantStore(db)`. No existence-blind, + // no default-allow — a caller with no matching row in `grant` is refused, + // same as production. `authorize` remains for tests that want to force a + // specific answer without provisioning rows for it (e.g. "deny always"). + const requireGrant: RequireGrant = + authorize === undefined + ? createRequireGrant({ grantStore: createGrantStore(hub.db), conditionRegistry: {} }) + : (resource, action) => async (c, next) => { + const resolved = + typeof resource === "function" + ? resource({ param: (name) => c.req.param(name) }) + : resource; + const allowed = authorize(resolved, action); + if (!allowed) { + return c.json( + { + error: { + code: "forbidden", + message: "forbidden", + }, + }, + 403, + ); + } + return next(); + }; mountArtifacts(api, { db, contentStore, - resolvePrincipal, - isAdmin, - identity, + requireGrant, decorate, + onArtifactCreated: grantOwnership, }); const mounted = app.route("/api", api); // `Hono#request` may answer synchronously; normalize to a promise so every @@ -367,14 +358,14 @@ export async function createReferenceHost(): Promise { }; } - const defaultApp = buildApp(InlineContentStore, async () => false); + const defaultApp = buildApp(InlineContentStore); return { db, - tenantId: tenant, + tenantId: tenant.id, agentPrincipal, scope: () => ({ - tenantId: tenant, + tenantId: tenant.id, principalId: principals.find((p) => p.kind === "user" && p.refId === "user-alice")! .id, }), diff --git a/examples/reference-host/test/acceptance.test.ts b/examples/reference-host/test/acceptance.test.ts index 41c347b..b38bf32 100644 --- a/examples/reference-host/test/acceptance.test.ts +++ b/examples/reference-host/test/acceptance.test.ts @@ -57,11 +57,10 @@ describe("import a URL, read it back, revise it, read the history", () => { artifactId = created.artifact.id; }); - test("the identity seam resolves the owner's name from the host directory", async () => { + test("the detail route serves back the imported source origin", async () => { const detail = await json<{ - artifact: { ownerName: string | null; source: Record }; + artifact: { source: Record }; }>(await host.request(`/api/artifacts/${artifactId}`)); - expect(detail.artifact.ownerName).toBe("Alice Ash"); expect(detail.artifact.source.origin).toBe("imported"); }); @@ -101,7 +100,7 @@ describe.each<[string, ContentStore]>([ let uploaded: Uploaded[]; beforeAll(async () => { - app = host.buildApp(store, async () => false); + app = host.buildApp(store); const form = new FormData(); form.append("files", new File([PNG], "chart.png", { type: "image/png" })); form.append("files", new File([PDF], "deck.pdf", { type: "application/pdf" })); @@ -173,7 +172,7 @@ describe("an unsupported upload is refused, leaving nothing behind", () => { }); }); -describe("list: keyset paging, creatorKind, and the archived toggle", () => { +describe("list: keyset paging and the archived toggle", () => { test("a keyset cursor is minted and the next page repeats nothing", async () => { const page1 = await json<{ artifacts: { id: string }[]; nextCursor: string | null }>( await host.request("/api/artifacts?limit=2"), @@ -191,27 +190,6 @@ describe("list: keyset paging, creatorKind, and the archived toggle", () => { ); expect(overlap).toEqual([]); }); - - test("creatorKind resolves through the identity seam", async () => { - // An agent-owned artifact, so creatorKind has something to separate. - await host.db.execute(sql` - INSERT INTO "artifacts"."artifact" ("tenant_id", "principal_id", "owner_principal_id", - "kind", "title", "content", "source", "version") - VALUES (${host.tenantId}, ${host.agentPrincipal}, ${host.agentPrincipal}, 'document', - 'Agent memo', 'written by an agent', '{"origin":"agent"}'::jsonb, 1) - `); - - const agentOnly = await json<{ artifacts: { title: string }[] }>( - await host.request("/api/artifacts?creatorKind=agent"), - ); - expect(agentOnly.artifacts.map((a) => a.title)).toEqual(["Agent memo"]); - - const humanOnly = await json<{ artifacts: { title: string }[] }>( - await host.request("/api/artifacts?creatorKind=user"), - ); - expect(humanOnly.artifacts.length).toBeGreaterThan(0); - expect(humanOnly.artifacts.some((a) => a.title === "Agent memo")).toBe(false); - }); }); describe("archive is a soft-hide, not a revocation", () => { @@ -243,38 +221,96 @@ describe("archive is a soft-hide, not a revocation", () => { }); }); -describe("a non-owner is refused unless the host's authz seam says admin", () => { - test("a non-owner, non-admin member is refused 403", async () => { - host.setSession({ userId: "user-bob" }); - const res = await host.request(`/api/artifacts/${artifactId}/archive`, { method: "POST" }); +describe("host grant authorization", () => { + test("a denied grant returns 403 and leaves archived_at null", async () => { + const app = host.buildApp(InlineContentStore, () => false); + const res = await app.request(`/api/artifacts/${artifactId}/archive`, { method: "POST" }); expect(res.status).toBe(403); + + const [row] = await host.db.execute<{ archived_at: string | null }>( + sql`SELECT "archived_at" FROM "artifacts"."artifact" WHERE "id" = ${artifactId}`, + ); + expect(row!.archived_at).toBeNull(); }); - test("the same member succeeds once the authz seam grants admin", async () => { - const adminApp = host.buildApp(InlineContentStore, async () => true); - const res = await adminApp.request(`/api/artifacts/${artifactId}/archive`, { - method: "POST", + test("an allowed grant receives the resource and action and archives", async () => { + const checks: { resource: string; action: string }[] = []; + const app = host.buildApp(InlineContentStore, (resource, action) => { + checks.push({ resource, action }); + return true; }); + + const res = await app.request(`/api/artifacts/${artifactId}/archive`, { method: "POST" }); expect(res.status).toBe(200); - await adminApp.request(`/api/artifacts/${artifactId}/unarchive`, { method: "POST" }); - host.setSession({ userId: "user-alice" }); - }); + expect(checks).toContainEqual({ + resource: `artifact:${artifactId}`, + action: "archive", + }); - test("the member who owns the producing agent may administer its artifact", async () => { - const [agentRow] = await host.db.execute<{ id: string }>( - sql`SELECT "id" FROM "artifacts"."artifact" WHERE "title" = 'Agent memo' LIMIT 1`, - ); - host.setSession({ userId: "user-alice" }); expect( - (await host.request(`/api/artifacts/${agentRow!.id}/archive`, { method: "POST" })).status, + (await app.request(`/api/artifacts/${artifactId}/unarchive`, { method: "POST" })).status, ).toBe(200); + }); +}); +/** + * The block above proves WIRING against a bare-predicate `authorize`. This + * one is the worked example: `host.request` (no `authorize` override) runs + * the real `createRequireGrant` backed by `@intx/db`'s `grant` table — the + * exact evaluator a production host runs. Nothing here stubs authorization. + * + * `artifactId`'s ownership grant was minted by `grantOwnership` (this host's + * `onArtifactCreated` hook) in the SAME transaction as its creation, back in + * the very first "a URL import creates the artifact" test — this block only + * spends it, and shows the row it spent. + */ +describe("ownership-derived grants: real provisioning, real refusal", () => { + test("the grant minted on create names the creator, the artifact, write and archive", async () => { + const rows = await host.db.execute<{ + principal_id: string; + resource: string; + action: string; + origin: string; + }>(sql` + SELECT "principal_id", "resource", "action", "origin" FROM "grant" + WHERE "resource" = ${`artifact:${artifactId}`} ORDER BY "action" + `); + expect(rows.map((r) => ({ action: r.action, origin: r.origin }))).toEqual([ + { action: "archive", origin: "creator" }, + { action: "write", origin: "creator" }, + ]); + expect(new Set(rows.map((r) => r.principal_id)).size).toBe(1); + }); + + test("Bob, a co-tenant with no grant on Alice's artifact, is refused", async () => { host.setSession({ userId: "user-bob" }); - expect( - (await host.request(`/api/artifacts/${agentRow!.id}/unarchive`, { method: "POST" })) - .status, - ).toBe(403); - host.setSession({ userId: "user-alice" }); + try { + const revise = await host.request( + `/api/artifacts/${artifactId}/versions`, + postJson({ content: "bob was here" }), + ); + expect(revise.status).toBe(403); + const archive = await host.request(`/api/artifacts/${artifactId}/archive`, { + method: "POST", + }); + expect(archive.status).toBe(403); + } finally { + host.setSession({ userId: "user-alice" }); + } + + const [row] = await host.db.execute<{ content: string; archived_at: string | null }>( + sql`SELECT "content", "archived_at" FROM "artifacts"."artifact" WHERE "id" = ${artifactId}`, + ); + expect(row!.content).not.toBe("bob was here"); + expect(row!.archived_at).toBeNull(); + }); + + test("Alice, the creator, still succeeds through the same real grant check", async () => { + const res = await host.request( + `/api/artifacts/${artifactId}/versions`, + postJson({ content: "alice, for real this time" }), + ); + expect(res.status).toBe(200); }); }); diff --git a/package.json b/package.json index 31752d9..8659700 100644 --- a/package.json +++ b/package.json @@ -64,6 +64,7 @@ "@hono/standard-validator": "^0.2.3" }, "peerDependencies": { + "@intx/hub-api": "^0.2.2", "@intx/types": "^0.2.2", "arktype": "^2.1.29", "drizzle-orm": "^0.45.2", @@ -72,6 +73,8 @@ "postgres": "^3.4.9" }, "devDependencies": { + "@intx/authz": "0.2.2", + "@intx/hub-api": "0.2.2", "@intx/types": "0.2.2", "@types/bun": "1.1.14", "@types/node": "22.10.5", diff --git a/src/artifacts.test.ts b/src/artifacts.test.ts index 74f5757..53501bb 100644 --- a/src/artifacts.test.ts +++ b/src/artifacts.test.ts @@ -351,7 +351,6 @@ describe("serialization", () => { expect(json.createdAt).toBe(row.createdAt.toISOString()); expect(json.archivedAt).toBeNull(); expect(json.ownerPrincipalId).toBeNull(); - expect(json.ownerName).toBeNull(); }); }); diff --git a/src/artifacts.ts b/src/artifacts.ts index 98b3233..b1c409e 100644 --- a/src/artifacts.ts +++ b/src/artifacts.ts @@ -8,7 +8,6 @@ import { getTableColumns, gte, ilike, - inArray, isNotNull, isNull, lt, @@ -20,7 +19,7 @@ import { } from "drizzle-orm"; import type { ArtifactDb, ArtifactTx } from "./db.js"; import { artifact, artifactVersion, type ArtifactRow } from "./schema.js"; -import type { ResolvedPrincipal, Identity } from "./ports.js"; +import type { ResolvedPrincipal } from "./ports.js"; import { parseWebSiteContentJson, serializeWebSiteContent, @@ -117,7 +116,6 @@ export type SerializedArtifactBase = { source: Record & { origin: string }; version: number; ownerPrincipalId: string | null; - ownerName: string | null; archivedAt: string | null; createdAt: string; updatedAt: string; @@ -147,7 +145,6 @@ function serializeArtifactBase( source: normalizeSource(row.source), version: row.version, ownerPrincipalId: row.ownerPrincipalId, - ownerName: null, archivedAt: row.archivedAt?.toISOString() ?? null, createdAt: row.createdAt.toISOString(), updatedAt: row.updatedAt.toISOString(), @@ -452,7 +449,6 @@ export type ListArtifactsFilters = { sort?: string; kind?: string; ownerPrincipalId?: string; - creatorKind?: "user" | "agent"; createdAfter?: Date; createdBefore?: Date; cursor?: { at: string; id: string }; @@ -499,7 +495,6 @@ export const ListArtifactsQuery = type({ "sort?": "'newest' | 'oldest'", "kind?": "string", "ownerPrincipalId?": "string", - "creatorKind?": "'user' | 'agent'", "createdAfter?": dateBound(false), "createdBefore?": dateBound(true), "cursor?": ListCursor, @@ -553,7 +548,6 @@ function cursorCondition( export async function listArtifacts( db: ArtifactDb, - identity: Identity, tenantId: string, filters: ListArtifactsFilters, ): Promise<{ rows: ArtifactListRow[]; nextCursor: string | null }> { @@ -580,15 +574,6 @@ export async function listArtifacts( if (filters.ownerPrincipalId) { conditions.push(eq(artifact.ownerPrincipalId, filters.ownerPrincipalId)); } - if (filters.creatorKind) { - // Creator kind is a facet of the owner principal, not a column here, so it - // folds in as an ownerPrincipalId membership test. NO matching principals - // must exclude everything, not fall through to unfiltered. - const ids = await identity.principalIdsByKind(tenantId, filters.creatorKind); - conditions.push( - ids.length > 0 ? inArray(artifact.ownerPrincipalId, ids) : sql`false`, - ); - } if (filters.createdAfter !== undefined) { conditions.push(gte(artifact.createdAt, filters.createdAfter)); } @@ -646,26 +631,15 @@ export async function findArtifactByTitle( } /** - * Attach owner display names and run the display-only provenance decorator. - * One call so no surface can serialize a row and forget half the enrichment. - * Accepts list items (no content) and detail rows alike. + * Run the display-only provenance decorator over serialized rows. One call so + * no surface can serialize a row and forget the decorator. Accepts list items + * (no content) and detail rows alike; display enrichment never affects what is + * returned or who may see it. */ export async function enrich( - identity: Identity, decorate: (tenantId: string, rows: readonly SerializedArtifactBase[]) => Promise, tenantId: string, rows: SerializedArtifactBase[], ): Promise { - const ownerIds = [ - ...new Set(rows.map((r) => r.ownerPrincipalId).filter((id) => id !== null)), - ]; - if (ownerIds.length > 0) { - const names = await identity.ownerNames(tenantId, ownerIds); - for (const row of rows) { - if (row.ownerPrincipalId !== null) { - row.ownerName = names.get(row.ownerPrincipalId) ?? null; - } - } - } await decorate(tenantId, rows); } diff --git a/src/index.ts b/src/index.ts index 5f13059..ab70ef4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,7 +5,6 @@ export type { MountArtifactsOpts } from "./mount.js"; export { runArtifactMigrations, MigrationChecksumError, MigrationAdoptError } from "./migrations.js"; export type { RunArtifactMigrationsOptions } from "./migrations.js"; - export { createArtifactDb } from "./db.js"; export type { ArtifactDb, ArtifactTx } from "./db.js"; @@ -23,12 +22,10 @@ export type { MailAttachmentRefRow, } from "./schema.js"; -export { anonymousIdentity } from "./ports.js"; export type { ResolvedPrincipal, ContentStore, FileBlob, - Identity, StoredFile, } from "./ports.js"; diff --git a/src/list.test.ts b/src/list.test.ts index 17307b6..7712df1 100644 --- a/src/list.test.ts +++ b/src/list.test.ts @@ -9,11 +9,9 @@ import { serializeArtifactListItem, setArtifactArchived, } from "./artifacts.js"; -import { fakeIdentity, seedArtifact, seedSkillDraft, testDb } from "./test-helpers.js"; +import { seedArtifact, seedSkillDraft, testDb } from "./test-helpers.js"; import type { ArtifactDb } from "./db.js"; -const identity = fakeIdentity(); - /** Parse a raw query string the way the route does, failing the test on error. */ function parseQuery(query: Record) { const parsed = ListArtifactsQuery(query); @@ -35,7 +33,7 @@ describe("list projection", () => { const large = "x".repeat(50_000); const seeded = await seedArtifact(db, { title: "Bulky", content: large }); - const page = await listArtifacts(db, identity, "acme", {}); + const page = await listArtifacts(db, "acme", {}); expect(page.rows.map((r) => r.id)).toEqual([seeded.id]); const row = page.rows[0] as { id: string; content?: string; title?: string }; // The list query must not select the body column at all. @@ -58,10 +56,10 @@ describe("list filters", () => { const hidden = await seedArtifact(db, { title: "Hidden" }); await setArtifactArchived(db, hidden, true); - const defaults = await listArtifacts(db, identity, "acme", {}); + const defaults = await listArtifacts(db, "acme", {}); expect(defaults.rows.map((r) => r.id)).toEqual([visible.id]); - const archived = await listArtifacts(db, identity, "acme", { archived: true }); + const archived = await listArtifacts(db, "acme", { archived: true }); expect(archived.rows.map((r) => r.id)).toEqual([hidden.id]); }); @@ -70,9 +68,9 @@ describe("list filters", () => { await seedSkillDraft(db, "Scratch"); await seedArtifact(db, { title: "Real" }); - expect((await listArtifacts(db, identity, "acme", {})).rows.length).toBe(1); + expect((await listArtifacts(db, "acme", {})).rows.length).toBe(1); expect( - (await listArtifacts(db, identity, "acme", { kind: "skill-draft" })).rows.length, + (await listArtifacts(db, "acme", { kind: "skill-draft" })).rows.length, ).toBe(0); }); @@ -81,7 +79,7 @@ describe("list filters", () => { await seedArtifact(db, { title: "Ours" }); await seedArtifact(db, { title: "Theirs", tenantId: "other" }); - const rows = (await listArtifacts(db, identity, "acme", {})).rows; + const rows = (await listArtifacts(db, "acme", {})).rows; expect(rows.map((r) => r.title)).toEqual(["Ours"]); }); @@ -91,10 +89,10 @@ describe("list filters", () => { await seedArtifact(db, { title: "Other", content: "mentions quarterly plans" }); await seedArtifact(db, { title: "100%", content: "literal" }); - expect((await listArtifacts(db, identity, "acme", { query: "quarter" })).rows.length).toBe( + expect((await listArtifacts(db, "acme", { query: "quarter" })).rows.length).toBe( 2, ); - const percent = await listArtifacts(db, identity, "acme", { query: "%" }); + const percent = await listArtifacts(db, "acme", { query: "%" }); expect(percent.rows.map((r) => r.title)).toEqual(["100%"]); }); @@ -105,7 +103,6 @@ describe("list filters", () => { const sameDay = await listArtifacts( db, - identity, "acme", parseQuery({ createdAfter: "2026-03-04", createdBefore: "2026-03-04" }), ); @@ -119,7 +116,6 @@ describe("list filters", () => { const before = await listArtifacts( db, - identity, "acme", parseQuery({ createdBefore: "2026-03-04T12:00:00Z" }), ); @@ -132,28 +128,6 @@ describe("list filters", () => { } }); - test("creatorKind with no matching principals excludes everything", async () => { - const db = await testDb(); - await seedArtifact(db, { title: "Mine" }); - - const none = await listArtifacts(db, identity, "acme", { creatorKind: "agent" }); - expect(none.rows.length).toBe(0); - }); - - test("creatorKind narrows to the matching owner principals", async () => { - const db = await testDb(); - const mine = await seedArtifact(db, { title: "Mine" }); - await seedArtifact(db, { title: "Bot", ownerPrincipalId: "agent-9" }); - - const users = await listArtifacts( - db, - fakeIdentity({ principalIdsByKind: async () => ["user-1"] }), - "acme", - { creatorKind: "user" }, - ); - expect(users.rows.map((r) => r.id)).toEqual([mine.id]); - }); - test("filters by kind and by owner", async () => { const db = await testDb(); const csv = await seedArtifact(db, { title: "Export", kind: "csv-export" }); @@ -161,11 +135,11 @@ describe("list filters", () => { const theirs = await seedArtifact(db, { title: "Bot", ownerPrincipalId: "agent-9" }); expect( - (await listArtifacts(db, identity, "acme", { kind: "csv-export" })).rows.map((r) => r.id), + (await listArtifacts(db, "acme", { kind: "csv-export" })).rows.map((r) => r.id), ).toEqual([csv.id]); expect( ( - await listArtifacts(db, identity, "acme", { ownerPrincipalId: "agent-9" }) + await listArtifacts(db, "acme", { ownerPrincipalId: "agent-9" }) ).rows.map((r) => r.id), ).toEqual([theirs.id]); }); @@ -186,7 +160,6 @@ describe("list paging", () => { for (let page = 0; page < 5; page += 1) { const result: Awaited> = await listArtifacts( db, - identity, "acme", parseQuery({ limit: "2", ...(cursor ? { cursor } : {}) }), ); @@ -209,11 +182,10 @@ describe("list paging", () => { ids.push(row.id); } - const first = await listArtifacts(db, identity, "acme", { sort: "oldest", limit: 2 }); + const first = await listArtifacts(db, "acme", { sort: "oldest", limit: 2 }); expect(first.rows.map((r) => r.id)).toEqual([ids[0], ids[1]]); const second = await listArtifacts( db, - identity, "acme", parseQuery({ sort: "oldest", limit: "2", cursor: first.nextCursor! }), ); @@ -236,15 +208,15 @@ describe("list paging", () => { FROM generate_series(1, ${MAX_LIST_LIMIT + 5}) AS i `); - const huge = await listArtifacts(db, identity, "acme", parseQuery({ limit: "10000" })); + const huge = await listArtifacts(db, "acme", parseQuery({ limit: "10000" })); expect(huge.rows.length).toBe(MAX_LIST_LIMIT); expect(huge.nextCursor).not.toBeNull(); - const zero = await listArtifacts(db, identity, "acme", parseQuery({ limit: "0" })); + const zero = await listArtifacts(db, "acme", parseQuery({ limit: "0" })); expect(zero.rows.length).toBe(1); // A non-numeric `?limit=` must take the default, not collapse to one row. - const garbage = await listArtifacts(db, identity, "acme", parseQuery({ limit: "abc" })); + const garbage = await listArtifacts(db, "acme", parseQuery({ limit: "abc" })); expect(garbage.rows.length).toBe(DEFAULT_LIST_LIMIT); // An absent limit defaults at the schema. @@ -268,7 +240,6 @@ describe("list paging", () => { for (let page = 0; page < 6; page += 1) { const result: Awaited> = await listArtifacts( db, - identity, "acme", parseQuery({ limit: "1", ...(cursor ? { cursor } : {}) }), ); @@ -287,7 +258,7 @@ describe("list paging", () => { await setTimes(db, row.id, "2026-01-01T00:00:00.123456Z"); await seedArtifact(db, { title: "Second" }); - const first = await listArtifacts(db, identity, "acme", { limit: 1, sort: "oldest" }); + const first = await listArtifacts(db, "acme", { limit: 1, sort: "oldest" }); expect(first.nextCursor).toBe(`2026-01-01T00:00:00.123456Z__${row.id}`); expect(new Date(first.nextCursor!.slice(0, 27)).toISOString()).not.toBe( "2026-01-01T00:00:00.123456Z", @@ -312,7 +283,7 @@ describe("list paging", () => { ids.push(row.id); } - const first = await listArtifacts(db, identity, "acme", { limit: 1 }); + const first = await listArtifacts(db, "acme", { limit: 1 }); // Newest first: 2026-01-03 12:00 UTC — not the LA wall clock 04:00 with a // lying Z suffix. expect(first.rows.map((r) => r.id)).toEqual([ids[2]]); @@ -320,7 +291,6 @@ describe("list paging", () => { const second = await listArtifacts( db, - identity, "acme", parseQuery({ limit: "1", cursor: first.nextCursor! }), ); @@ -329,7 +299,6 @@ describe("list paging", () => { const third = await listArtifacts( db, - identity, "acme", parseQuery({ limit: "1", cursor: second.nextCursor! }), ); @@ -339,7 +308,6 @@ describe("list paging", () => { // Date filters must also compare absolute instants, not session walls. const filtered = await listArtifacts( db, - identity, "acme", parseQuery({ createdAfter: "2026-01-02T00:00:00Z", diff --git a/src/migrations.test.ts b/src/migrations.test.ts index 4932822..076599d 100644 --- a/src/migrations.test.ts +++ b/src/migrations.test.ts @@ -479,8 +479,8 @@ describe("migrations", () => { /** * DB invariants: tenant_id is required on every artifact row; version and size - * stay non-negative. Principal↔tenant alignment is host-owned (resolvePrincipal) - * — no multi-table trigger here. + * stay non-negative. Principal↔tenant alignment is host middleware/context + * (TenantEnv) — no multi-table trigger here. */ test("null tenant_id on artifact is rejected after migrations", async () => { await runArtifactMigrations(db); diff --git a/src/migrations.ts b/src/migrations.ts index 1b7d440..cec7630 100644 --- a/src/migrations.ts +++ b/src/migrations.ts @@ -155,7 +155,7 @@ export const MIGRATIONS: Migration[] = [ // dump can still hold orphans; refuse to SET NOT NULL over them and tell the // operator to assign a tenant or delete the rows first. Version/size CHECKs // are single-column and free at write time. Principal↔tenant alignment is - // deliberately host-owned (resolvePrincipal) — a multi-table trigger into + // host middleware/context (TenantEnv) — a multi-table trigger into // public.principal is out of scope and would couple write path latency to // the control plane. id: "0003_schema_invariants", diff --git a/src/mount.test.ts b/src/mount.test.ts index f6d27b8..f5cf6cd 100644 --- a/src/mount.test.ts +++ b/src/mount.test.ts @@ -1,6 +1,9 @@ import { describe, expect, test } from "bun:test"; import { sql } from "drizzle-orm"; 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 { InlineContentStore } from "./content-store.js"; import { @@ -15,25 +18,112 @@ import { } from "./uploads.js"; import type { ArtifactDb } from "./db.js"; import type { MountArtifactsOpts } from "./mount.js"; -import type { ResolvedPrincipal, Identity } from "./ports.js"; -import { fakeIdentity, seedArtifact, seedSkillDraft, SCOPE, testDb } from "./test-helpers.js"; +import type { ResolvedPrincipal } from "./ports.js"; +import { seedArtifact, seedSkillDraft, SCOPE, testDb } from "./test-helpers.js"; + +/** Places tenant/principal on the context the way a real host's session + * middleware does, without pinning it to any one `requireGrant` wiring. */ +function withPrincipal(app: Hono, principal: ResolvedPrincipal | null) { + app.use("*", async (c, next) => { + if (principal !== null) { + const now = new Date(0); + c.set("tenant", { + id: principal.tenantId, + name: principal.tenantId, + slug: principal.tenantId, + domain: `${principal.tenantId}.example`, + parentId: null, + config: null, + createdAt: now, + updatedAt: now, + }); + c.set("principal", { + id: principal.principalId, + tenantId: principal.tenantId, + kind: "user", + refId: principal.principalId, + status: "active", + createdAt: now, + updatedAt: now, + }); + } + await next(); + }); + return app; +} + +/** A grant minted for exactly one resource/action/principal — deny is simply + * not minting the matching one, which is how the real store discriminates. */ +function grantRule(over: Partial & Pick): GrantRule { + return { + id: `grant-${over.resource}-${over.action}-${over.principalId}`, + effect: "allow", + origin: "creator", + conditions: null, + expiresAt: null, + roleId: null, + ...over, + }; +} type HostOpts = { principal?: ResolvedPrincipal | null; - identity?: Identity; - isAdmin?: MountArtifactsOpts["isAdmin"]; + authorize?: (resource: string, action: string) => boolean; decorate?: MountArtifactsOpts["decorate"]; + contentStore?: MountArtifactsOpts["contentStore"]; }; function host(db: ArtifactDb, opts: HostOpts = {}) { - const app = new Hono(); + const app = new Hono(); const principal = opts.principal === undefined ? SCOPE : opts.principal; + app.use("*", async (c, next) => { + if (principal !== null) { + const now = new Date(0); + c.set("tenant", { + id: principal.tenantId, + name: principal.tenantId, + slug: principal.tenantId, + domain: `${principal.tenantId}.example`, + parentId: null, + config: null, + createdAt: now, + updatedAt: now, + }); + c.set("principal", { + id: principal.principalId, + tenantId: principal.tenantId, + kind: "user", + refId: principal.principalId, + status: "active", + createdAt: now, + updatedAt: now, + }); + } + await next(); + }); + const requireGrant: RequireGrant = (resource, action) => async (c, next) => { + const resolved = + typeof resource === "function" + ? resource({ param: (name) => c.req.param(name) }) + : resource; + const allowed = (opts.authorize ?? (() => true))(resolved, action); + if (!allowed) { + return c.json( + { + error: { + code: "forbidden", + message: "forbidden", + }, + }, + 403, + ); + } + return next(); + }; return mountArtifacts(app, { db, - contentStore: InlineContentStore, - resolvePrincipal: () => principal, - identity: opts.identity ?? fakeIdentity(), - ...(opts.isAdmin ? { isAdmin: opts.isAdmin } : {}), + contentStore: opts.contentStore ?? InlineContentStore, + requireGrant, ...(opts.decorate ? { decorate: opts.decorate } : {}), }); } @@ -189,15 +279,12 @@ describe("GET /artifacts", () => { expect(detail.artifact.content).toBe(large); }); - test("attaches owner names and runs the display-only provenance decorator", async () => { + test("runs the display-only provenance decorator", async () => { const db = await testDb(); await seedArtifact(db, { title: "Doc" }); const decorated: string[] = []; const app = host(db, { - identity: fakeIdentity({ - ownerNames: async (_tenant, ids) => new Map(ids.map((id) => [id, `Name of ${id}`])), - }), decorate: async (tenantId, rows) => { decorated.push(tenantId); for (const row of rows) { @@ -210,7 +297,6 @@ describe("GET /artifacts", () => { artifacts: Record[]; }; expect(body.artifacts[0]).toMatchObject({ - ownerName: "Name of user-1", sessionName: "Weekly brief", }); expect(decorated).toEqual(["acme"]); @@ -252,10 +338,9 @@ describe("GET /artifacts", () => { expect("sessionName" in byTitle.get("Hand written")!).toBe(false); }); - test("rejects a bad creatorKind and a bad cursor with 400", async () => { + test("rejects a bad cursor and a bad date with 400", async () => { const db = await testDb(); const app = host(db); - expect((await app.request("/artifacts?creatorKind=robot")).status).toBe(400); expect((await app.request("/artifacts?cursor=garbage")).status).toBe(400); expect((await app.request("/artifacts?createdAfter=nonsense")).status).toBe(400); }); @@ -550,13 +635,19 @@ describe("versions", () => { }); describe("archive authorization", () => { - const post = (app: Hono, id: string, verb: string) => + const post = (app: Hono, id: string, verb: string) => app.request(`/artifacts/${id}/${verb}`, { method: "POST" }); - test("the exact owner may archive and unarchive", async () => { + test("allow records checks for archive and unarchive", async () => { const db = await testDb(); - const app = host(db); const row = await seedArtifact(db); + const checks: { resource: string; action: string }[] = []; + const app = host(db, { + authorize: (resource, action) => { + checks.push({ resource, action }); + return true; + }, + }); const archived = await post(app, row.id, "archive"); expect(archived.status).toBe(200); @@ -564,31 +655,24 @@ describe("archive authorization", () => { const restored = await post(app, row.id, "unarchive"); expect(((await restored.json()) as any).artifact.archivedAt).toBeNull(); - }); - test("a non-owner without admin is refused", async () => { - const db = await testDb(); - const row = await seedArtifact(db, { ownerPrincipalId: "someone-else" }); - expect((await post(host(db), row.id, "archive")).status).toBe(403); + expect(checks).toEqual([ + { resource: `artifact:${row.id}`, action: "archive" }, + { resource: `artifact:${row.id}`, action: "archive" }, + ]); }); - test("the member who owns the producing agent may archive", async () => { + test("deny authorize false expects 403 and leaves archived_at null", async () => { const db = await testDb(); - const row = await seedArtifact(db, { ownerPrincipalId: "agent-9" }); - const app = host(db, { - identity: fakeIdentity({ - ownerMemberPrincipalId: async (scope) => - scope.principalId === "agent-9" ? "user-1" : null, - }), - }); - expect((await post(app, row.id, "archive")).status).toBe(200); - }); + const row = await seedArtifact(db); + const app = host(db, { authorize: () => false }); - test("an admin may archive anyone's artifact via the authz seam", async () => { - const db = await testDb(); - const row = await seedArtifact(db, { ownerPrincipalId: "someone-else" }); - const app = host(db, { isAdmin: async () => true }); - expect((await post(app, row.id, "archive")).status).toBe(200); + expect((await post(app, row.id, "archive")).status).toBe(403); + + const rows = await db.execute<{ archived_at: Date | null }>( + sql`SELECT "archived_at" FROM "artifacts"."artifact" WHERE "id" = ${row.id}`, + ); + expect(rows[0]!.archived_at).toBeNull(); }); test("archiving is idempotent over the route", async () => { @@ -602,6 +686,193 @@ describe("archive authorization", () => { }); }); +/** + * `archive authorization` above proves WIRING: the package calls whatever + * `requireGrant` it is handed with the right resource/action and honors the + * answer. It says nothing about whether a real grant check would refuse + * anyone, because its stub `authorize` is a bare predicate the test itself + * chooses the answer for. + * + * This block runs the SAME routes through `createRequireGrant` + + * `createInMemoryGrantStore` from the platform's own `@intx/hub-api` / + * `@intx/authz` — the exact evaluator a real host runs in production, not a + * reimplementation of it. Grants are looked up by `principalId`, so a caller + * who was never minted one is refused on the merits, not because a test typed + * `() => false`. + */ +describe("authorization through the real platform grant evaluator", () => { + const OWNER: ResolvedPrincipal = SCOPE; + const NON_OWNER: ResolvedPrincipal = { tenantId: SCOPE.tenantId, principalId: "someone-else" }; + + function hostWithGrants( + db: ArtifactDb, + principal: ResolvedPrincipal | null, + grants: GrantRule[], + ) { + const requireGrant = createRequireGrant({ + grantStore: createInMemoryGrantStore(grants), + conditionRegistry: {}, + }); + return mountArtifacts(withPrincipal(new Hono(), principal), { + db, + contentStore: InlineContentStore, + requireGrant, + }); + } + + test("the owner's creator-origin grant allows write; a co-tenant with no grant is refused", async () => { + const db = await testDb(); + const row = await seedArtifact(db, { content: "v1" }); + const grants = [ + grantRule({ resource: `artifact:${row.id}`, action: "write", principalId: OWNER.principalId }), + ]; + + const ownerRes = await hostWithGrants(db, OWNER, grants).request( + `/artifacts/${row.id}/versions`, + json({ content: "v2" }), + ); + expect(ownerRes.status).toBe(200); + + // Same grant list, same tenant, no grant naming this principal: the real + // evaluator finds nothing to match and fails closed, not open. + const intruderRes = await hostWithGrants(db, NON_OWNER, grants).request( + `/artifacts/${row.id}/versions`, + json({ content: "hijack" }), + ); + expect(intruderRes.status).toBe(403); + + const [current] = await db.execute<{ content: string }>( + sql`SELECT "content" FROM "artifacts"."artifact" WHERE "id" = ${row.id}`, + ); + expect(current!.content).toBe("v2"); + }); + + test("a grant for the wrong action does not authorize a different one", async () => { + const db = await testDb(); + const row = await seedArtifact(db); + // The owner can write, but was never granted archive. + const grants = [ + grantRule({ resource: `artifact:${row.id}`, action: "write", principalId: OWNER.principalId }), + ]; + const app = hostWithGrants(db, OWNER, grants); + + expect( + (await app.request(`/artifacts/${row.id}/archive`, { method: "POST" })).status, + ).toBe(403); + const [current] = await db.execute<{ archived_at: Date | null }>( + sql`SELECT "archived_at" FROM "artifacts"."artifact" WHERE "id" = ${row.id}`, + ); + expect(current!.archived_at).toBeNull(); + }); + + test("no grants at all refuses everyone, including the artifact's own owner", async () => { + const db = await testDb(); + const row = await seedArtifact(db); + const app = hostWithGrants(db, OWNER, []); + expect( + (await app.request(`/artifacts/${row.id}/archive`, { method: "POST" })).status, + ).toBe(403); + }); + + // A real grant evaluator has no existence check of its own — it denies a + // ghost id or a cross-tenant artifact with the SAME 403 it gives a real row + // the caller lacks permission on. Without checking existence first, this + // would collapse "not permitted" and "not found" into one signal, and a + // caller could no longer tell (from the write routes) whether an id names + // anything at all — the existence oracle every 404 on this package guards + // against. `artifactExists` runs before `requireGrant` precisely so a + // caller with zero grants still gets 404, not 403, for ids that name + // nothing they could ever see. + test("a ghost id and a cross-tenant id are still 404 through a zero-grant evaluator, not 403", async () => { + const db = await testDb(); + const app = hostWithGrants(db, OWNER, []); + const foreign = await seedArtifact(db, { tenantId: "other" }); + + for (const [cause, id] of [ + ["ghost id", "00000000-0000-4000-8000-000000000000"], + ["cross-tenant", foreign.id], + ] as [string, string][]) { + const res = await app.request(`/artifacts/${id}/versions`, json({ content: "x" })); + expect({ cause, status: res.status, body: await res.json() }).toEqual({ + cause, + status: 404, + body: { error: "Artifact not found" }, + }); + } + }); +}); + +/** + * The other half of the wiring test above: `onArtifactCreated` is the seam a + * host uses to provision the grant that later authorizes the caller against + * the row it just made — see the reference host for a real one backed by + * `@intx/db`'s grant table. Here the point is narrower: the hook runs inside + * the SAME transaction as the insert, once per created row, with the row and + * the creating scope. + */ +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. + 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 res = await app.request( + "/artifacts", + json({ mode: "text", title: "Provisioned", content: "body" }), + ); + const body = (await res.json()) as { artifact: { id: string } }; + expect(seen).toEqual([{ row: { id: body.artifact.id }, scope: SCOPE }]); + }); + + 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"); + }, + }); + + await app.request("/artifacts", json({ mode: "text", title: "Orphan?", content: "body" })); + const rows = await listArtifacts(db, SCOPE.tenantId, {}); + expect(rows.rows.length).toBe(0); + }); + + 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 form = new FormData(); + form.append("files", new File(["a"], "a.txt", { type: "text/plain" })); + form.append("files", new File(["b"], "b.txt", { type: "text/plain" })); + const res = await app.request("/artifacts/upload", { method: "POST", body: form }); + const body = (await res.json()) as { artifacts: { id: string }[] }; + expect(ids.sort()).toEqual(body.artifacts.map((a) => a.id).sort()); + }); +}); + describe("POST /artifacts/upload", () => { const form = (files: File[], generatedBy?: string) => { const data = new FormData(); @@ -822,7 +1093,7 @@ describe("mail attachment references", () => { ).status, ).toBe(400); // The write is a mutation, so 403. The matching READ is a collection read - // and answers an empty 200 — see the no-identity route-class block below. + // and answers an empty 200 — see the no-principal route-class block below. expect( ( await host(db, { principal: null }).request( @@ -907,15 +1178,10 @@ describe("mail attachment references", () => { }); describe("post-commit side effects never turn a committed write into a 500", () => { - // Enrichment runs against HOST-supplied seams after the transaction commits. + // Enrichment runs against HOST-supplied decorator after the transaction commits. // A throwing host must not make a durable mutation report failure: the client // would retry a write that already succeeded, and keep retrying forever. const exploding = { - identity: fakeIdentity({ - ownerNames: async () => { - throw new Error("host directory is down"); - }, - }), decorate: async () => { throw new Error("host workflow lookup is down"); }, @@ -930,10 +1196,9 @@ describe("post-commit side effects never turn a committed write into a 500", () json({ mode: "text", title: "Survives", content: "body" }), ); expect(res.status).toBe(201); - const body = (await res.json()) as { artifact: { id: string; ownerName: null } }; - expect(body.artifact.ownerName).toBeNull(); + const body = (await res.json()) as { artifact: { id: string } }; - const rows = await listArtifacts(db, fakeIdentity(), SCOPE.tenantId, {}); + const rows = await listArtifacts(db, SCOPE.tenantId, {}); expect(rows.rows.map((r) => r.id)).toEqual([body.artifact.id]); }); @@ -966,7 +1231,7 @@ describe("post-commit side effects never turn a committed write into a 500", () // The cross-core no-member asymmetry, asserted per route CLASS rather // than per happenstance, so a route added later has an obvious bucket to fall // into and this file fails if it lands in the wrong one. -describe("no-identity response: every route matches the cross-core rule", () => { +describe("no-principal response: every route matches the cross-core rule", () => { // Collection reads answer the truth — "you have none" — because the answer // names no resource and so discloses nothing. test("list/collection reads return an empty 200", async () => { @@ -1044,24 +1309,32 @@ describe("no-identity response: every route matches the cross-core rule", () => // would be the worse bug of the two. test("a refused mutation leaves nothing behind", async () => { const db = await testDb(); - const before = await listArtifacts(db, fakeIdentity(), SCOPE.tenantId, {}); + const before = await listArtifacts(db, SCOPE.tenantId, {}); const app = host(db, { principal: null }); await app.request("/artifacts", json({ mode: "text", title: "ghost", content: "b" })); - const after = await listArtifacts(db, fakeIdentity(), SCOPE.tenantId, {}); + const after = await listArtifacts(db, SCOPE.tenantId, {}); expect(after.rows.length).toBe(before.rows.length); }); }); describe("hardening regressions", () => { - test("revising someone else's artifact is 403, admin may", async () => { + test("write grant deny is 403; allow records artifact:id/write and returns 200", async () => { const db = await testDb(); - const row = await seedArtifact(db, { ownerPrincipalId: "someone-else" }); - const revise = (app: Hono) => + const row = await seedArtifact(db); + const revise = (app: Hono) => app.request(`/artifacts/${row.id}/versions`, json({ content: "hijack" })); - expect((await revise(host(db))).status).toBe(403); - const admin = host(db, { isAdmin: async () => true }); - expect((await revise(admin)).status).toBe(200); + expect((await revise(host(db, { authorize: () => false }))).status).toBe(403); + + const checks: { resource: string; action: string }[] = []; + const allowed = host(db, { + authorize: (resource, action) => { + checks.push({ resource, action }); + return true; + }, + }); + expect((await revise(allowed)).status).toBe(200); + expect(checks).toEqual([{ resource: `artifact:${row.id}`, action: "write" }]); }); test("revising a web_site with invalid content is 400, not 404", async () => { @@ -1140,13 +1413,7 @@ describe("hardening regressions", () => { filename: "view.bin", }), }; - const app = new Hono(); - mountArtifacts(app, { - db, - contentStore: store, - resolvePrincipal: () => SCOPE, - identity: fakeIdentity(), - }); + const app = host(db, { contentStore: store }); const row = await seedArtifact(db, { kind: "file", source: { origin: "imported", upload: { id: "u-1", filename: "view.bin", mimeType: "application/octet-stream", size: 3 } }, diff --git a/src/mount.ts b/src/mount.ts index 390db00..bfe5e39 100644 --- a/src/mount.ts +++ b/src/mount.ts @@ -1,8 +1,10 @@ import "./arktype.js"; import { type } from "arktype"; -import type { Context, Env, Hono } from "hono"; +import type { Context, Hono } from "hono"; +import type { MiddlewareHandler } from "hono"; import { describeRoute } from "hono-openapi"; -import type { ArtifactDb } from "./db.js"; +import { idResource, type RequireGrant, type TenantEnv } from "@intx/hub-api"; +import type { ArtifactDb, ArtifactTx } from "./db.js"; import { ArtifactNotFoundError, ArtifactSizeError, @@ -32,12 +34,7 @@ import { SaveMailAttachmentRefsSchema, } from "./mail-attachments.js"; import type { ArtifactRow } from "./schema.js"; -import { - anonymousIdentity, - type ResolvedPrincipal, - type ContentStore, - type Identity, -} from "./ports.js"; +import type { ResolvedPrincipal, ContentStore } from "./ports.js"; import { ARTIFACT_UPLOAD_POLICY, contentDispositionHeader, @@ -55,21 +52,13 @@ export type MountArtifactsOpts = { db: ArtifactDb; contentStore: ContentStore; /** - * Who the request runs as. `ctx` is `unknown` on purpose — the seam never - * reaches into Hono's context typing — and the signature is identical to - * `@corbits/mailbox-core`'s, so a host mounting both passes the same function. - * - * The resolved tenant is authoritative: there is no caller-supplied tenant - * override, so a request can only ever reach the tenant its own session - * resolves to. + * 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 + * one artifact are guarded with `requireGrant(idResource("artifact", "id"), + * )`. */ - resolvePrincipal: ( - ctx: unknown, - ) => Promise | ResolvedPrincipal | null; - /** Whether this principal is a tenant admin. Defaults to nobody being an admin. */ - isAdmin?: (scope: ResolvedPrincipal) => Promise; - /** Defaults to no directory. */ - identity?: Identity; + requireGrant: RequireGrant; /** * A DISPLAY-ONLY decorator: it may add fields to the serialized rows and * must never affect what is returned or who may see it. Defaults to a no-op. @@ -80,6 +69,21 @@ export type MountArtifactsOpts = { tenantId: string, rows: readonly SerializedArtifactBase[], ) => Promise; + /** + * Host hook run INSIDE the same transaction as artifact creation, once per + * row (`POST /artifacts` once, `POST /artifacts/upload` once per file). + * This is the seam a host uses to provision whatever grants make its + * authorization model true — for example, a `creator`-origin grant on + * `artifact:` for `write` and `archive` so the caller who just made the + * row can revise and archive it. artifact-core mints no grants itself: + * provisioning, like checking, is the host's job. Defaults to a no-op, so a + * host with no grant model omitting this behaves exactly as before. + */ + onArtifactCreated?: ( + tx: ArtifactTx, + row: ArtifactRow, + scope: ResolvedPrincipal, + ) => Promise; /** Which files `POST /artifacts/upload` accepts. */ uploadPolicy?: UploadPolicy; }; @@ -150,37 +154,85 @@ const idParam = { /** * Mount the artifact routes onto a host Hono app. * - * Generic over `E extends Env` and returning `Hono` so it composes with a - * host app that carries its own environment (an Interchange `createApp` - * returns `Hono`, not a bare `Hono`). + * 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`. * - * With no resolvable principal, collection reads answer an empty 200 while + * 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 "The no-identity contract" in the README for why. + * package follows. See "No principal on the context" in the README for why. */ -export function mountArtifacts( - app: Hono, +export function mountArtifacts( + app: Hono, opts: MountArtifactsOpts, -): Hono { +): Hono { const { db, contentStore, - resolvePrincipal, - isAdmin = async () => false, - identity = anonymousIdentity, + requireGrant, decorate = async () => {}, + onArtifactCreated = async () => {}, uploadPolicy = ARTIFACT_UPLOAD_POLICY, } = opts; - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- the handler - // context is the host's, never this package's, so it stays untyped here and - // is handed to `resolvePrincipal` as `unknown`. - type Ctx = Context; + // The handler context is TenantEnv. `principal` (and `tenant`) is placed by + // Interchange's middleware; nothing here resolves it. + type Ctx = Context; - const scopeFor = (c: Ctx) => resolvePrincipal(c); + /** + * Read the authenticated principal the host placed on the context. Returns + * null when the host's auth/tenant middleware did not resolve one — the + * signed-out case each route class handles per the no-principal contract. + */ + const scopeFor = (c: Ctx): ResolvedPrincipal | null => { + const principal = c.get("principal"); + if (!principal) return null; + return { tenantId: principal.tenantId, principalId: principal.id }; + }; const readJson = (c: Ctx): Promise => c.req.json().catch(() => null); + /** + * Reject the request when the host has not put a principal on the context. + * Must run BEFORE `requireGrant`: Interchange's middleware reads + * `principal.id` without its own null guard, so on an unresolved context it + * throws and the host sees a 500 rather than the 403 this package answers + * everywhere else for a signed-out caller. + */ + const principalRequired: MiddlewareHandler = async (c, next) => { + if (!c.get("principal")) return c.json({ error: "Forbidden" }, 403); + await next(); + }; + + /** + * Confirm the id names a real, in-tenant, non-skill-draft artifact — or + * answer the same 404 `loadScoped` does — BEFORE `requireGrant` runs. + * + * Must run between `principalRequired` and `requireGrant`: a real + * `requireGrant` (Interchange's `authorize()`) has no existence check of its + * own — it just asks whether the caller holds a grant naming the resource + * string built from the URL param, real artifact or not. A ghost id, a + * skill-draft, and another tenant's artifact all name a resource the caller + * holds no grant for, so without this check they would deny with the SAME + * 403 a real artifact the caller merely lacks permission on gets — losing + * the one thing single-artifact routes guarantee: a caller who cannot see + * the artifact gets 404, not "403 because you don't own something (real or + * not)". A stub `requireGrant` that always allows never surfaced this, + * because it never got the chance to deny before `loadScoped` ran. + */ + const artifactExists: MiddlewareHandler = async (c, next) => { + const scope = await scopeFor(c); + // principalRequired already ran; a null scope here would mean it didn't. + if (!scope) return c.json({ error: "Forbidden" }, 403); + const row = await getArtifact(db, c.req.param("id")!); + if (!row || row.kind === SKILL_DRAFT_KIND || row.tenantId !== scope.tenantId) { + return c.json({ error: "Artifact not found" }, 404); + } + await next(); + }; + /** * Coarse HTTP body ceiling for JSON mutators, reusing the content-byte constant. * Only acts when Content-Length is present; missing length still streams into @@ -198,7 +250,7 @@ export function mountArtifacts( rows: ArtifactRow[], ): Promise { const serialized = rows.map(serializeArtifact); - await enrich(identity, decorate, scope.tenantId, serialized); + await enrich(decorate, scope.tenantId, serialized); return serialized; } @@ -208,7 +260,7 @@ export function mountArtifacts( rows: ArtifactListRow[], ): Promise { const serialized = rows.map(serializeArtifactListItem); - await enrich(identity, decorate, scope.tenantId, serialized); + await enrich(decorate, scope.tenantId, serialized); return serialized; } @@ -258,7 +310,7 @@ export function mountArtifacts( tags: ["Artifacts"], summary: "List artifacts in the caller's tenant", description: - "Newest-updated first by default. Supports query/kind/owner/creatorKind/date filters, an `updatedAt__id` keyset cursor, and an archived-only toggle. skill-draft artifacts are never listed. List is discovery only: each item omits `content` (fetch the body via GET /artifacts/:id, download, or tools).", + "Newest-updated first by default. Supports query/kind/owner/date filters, an `updatedAt__id` keyset cursor, and an archived-only toggle. skill-draft artifacts are never listed. List is discovery only: each item omits `content` (fetch the body via GET /artifacts/:id, download, or tools).", parameters: [ { name: "query", in: "query", required: false, schema: { type: "string" } }, { name: "sort", in: "query", required: false, schema: { type: "string" } }, @@ -269,12 +321,6 @@ export function mountArtifacts( required: false, schema: { type: "string" }, }, - { - name: "creatorKind", - in: "query", - required: false, - schema: { type: "string", enum: ["user", "agent"] }, - }, { name: "createdAfter", in: "query", @@ -313,7 +359,7 @@ export function mountArtifacts( const scope = await scopeFor(c); if (!scope) return c.json({ artifacts: [], nextCursor: null }); - const page = await listArtifacts(db, identity, scope.tenantId, filters); + const page = await listArtifacts(db, scope.tenantId, filters); return c.json({ artifacts: await serializeList(scope, page.rows), nextCursor: page.nextCursor, @@ -356,8 +402,8 @@ export function mountArtifacts( const isUrl = body.mode === "url"; try { - const row = await db.transaction((tx) => - createArtifact(tx, { + const row = await db.transaction(async (tx) => { + const created = await createArtifact(tx, { scope, ownerPrincipalId: scope.principalId, kind: body.kind ?? (isUrl ? "link" : "document"), @@ -366,8 +412,10 @@ export function mountArtifacts( source: isUrl ? { origin: "imported", url: body.content } : { origin: "manual" }, - }), - ); + }); + await onArtifactCreated(tx, created, scope); + return created; + }); const [artifactJson] = await serializeCommitted(scope, [row]); return c.json({ artifact: artifactJson }, 201); @@ -458,17 +506,17 @@ export function mountArtifacts( rows = await db.transaction(async (tx) => { const created: ArtifactRow[] = []; for (const file of files) { - created.push( - await createFileArtifact(tx, contentStore, { - scope, - ownerPrincipalId, - filename: file.name, - mimeType: effectiveUploadMime(file, uploadPolicy), - policy: uploadPolicy, - bytes: new Uint8Array(await file.arrayBuffer()), - ...(generatedBy !== undefined ? { generatedBy } : {}), - }), - ); + const row = await createFileArtifact(tx, contentStore, { + scope, + ownerPrincipalId, + filename: file.name, + mimeType: effectiveUploadMime(file, uploadPolicy), + policy: uploadPolicy, + bytes: new Uint8Array(await file.arrayBuffer()), + ...(generatedBy !== undefined ? { generatedBy } : {}), + }); + await onArtifactCreated(tx, row, scope); + created.push(row); } return created; }); @@ -545,18 +593,18 @@ export function mountArtifacts( responses: { 200: { description: "New version created" }, 400: { description: "Invalid request body or content" }, - 403: { description: "No resolvable principal, or not the owner" }, + 403: { description: "No resolvable principal, or not permitted" }, 404: { description: "Artifact not found" }, 413: { description: "Declared Content-Length over the content ceiling" }, }, }), + principalRequired, + artifactExists, + requireGrant(idResource("artifact", "id"), "write"), async (c) => { // loadScoped resolves the principal before any body parse. const loaded = await loadScoped(c); if ("response" in loaded) return loaded.response; - if (!(await canMutate(loaded.row, loaded.scope))) { - return c.json({ error: "Forbidden" }, 403); - } if (contentLengthOverCeiling(c)) { return c.json( { @@ -594,32 +642,10 @@ export function mountArtifacts( }, ); - /** - * The mutation authz rule, consulted by revise and archive alike. Allowed for - * the principal-exact owner; for the member who owns the agent that produced - * it (agent artifacts are owned by a synthetic principal, so resolve it - * back); or for a tenant admin. - */ - async function canMutate( - row: ArtifactRow, - scope: ResolvedPrincipal, - ): Promise { - if (row.ownerPrincipalId === scope.principalId) return true; - if (row.ownerPrincipalId !== null) { - const ownerMember = await identity.ownerMemberPrincipalId({ - tenantId: scope.tenantId, - principalId: row.ownerPrincipalId, - }); - if (ownerMember !== null && ownerMember === scope.principalId) return true; - } - return await isAdmin(scope); - } - async function setArchived(c: Ctx, archive: boolean) { const loaded = await loadScoped(c); if ("response" in loaded) return loaded.response; const { row, scope } = loaded; - if (!(await canMutate(row, scope))) return c.json({ error: "Forbidden" }, 403); const updated = await setArtifactArchived(db, row, archive); const [artifactJson] = await serializeCommitted(scope, [updated]); @@ -640,6 +666,9 @@ export function mountArtifacts( 404: { description: "Artifact not found" }, }, }), + principalRequired, + artifactExists, + requireGrant(idResource("artifact", "id"), "archive"), (c) => setArchived(c, true), ); @@ -656,6 +685,9 @@ export function mountArtifacts( 404: { description: "Artifact not found" }, }, }), + principalRequired, + artifactExists, + requireGrant(idResource("artifact", "id"), "archive"), (c) => setArchived(c, false), ); diff --git a/src/ports.ts b/src/ports.ts index bde9333..b93bfb3 100644 --- a/src/ports.ts +++ b/src/ports.ts @@ -47,36 +47,3 @@ export type ContentStore = { artifact: { tenantId: string | null; source: unknown }, ): Promise; }; - -/** - * Identity: owner display names, and the agent→human resolution that lets the - * member who owns a producing agent administer its artifacts. - */ -export type Identity = { - /** Display names for owner principal ids. Missing ids simply stay unnamed. */ - ownerNames( - tenantId: string, - ownerPrincipalIds: string[], - ): Promise>; - /** The human member principal behind an agent principal, or null. */ - ownerMemberPrincipalId(scope: ResolvedPrincipal): Promise; - /** Principal ids in a tenant whose creator kind matches. Drives `?creatorKind`. */ - principalIdsByKind(tenantId: string, kind: "user" | "agent"): Promise; - /** - * Whether the human behind this principal is an ACTIVE member of another - * tenant — the gate on a cross-tenant read. Must fail closed. - */ - ownerIsMemberOfTenant( - scope: ResolvedPrincipal, - targetTenantId: string, - ): Promise; -}; - -/** An identity for hosts with no directory: no names, no agent ownership, - * and no cross-tenant reads. */ -export const anonymousIdentity: Identity = { - ownerNames: async () => new Map(), - ownerMemberPrincipalId: async () => null, - principalIdsByKind: async () => [], - ownerIsMemberOfTenant: async () => false, -}; diff --git a/src/test-helpers.ts b/src/test-helpers.ts index e9e4a91..6406b4d 100644 --- a/src/test-helpers.ts +++ b/src/test-helpers.ts @@ -3,7 +3,6 @@ import { createArtifactDb, type ArtifactDb } from "../src/db.js"; import { runArtifactMigrations } from "../src/migrations.js"; import { createArtifact } from "../src/artifacts.js"; import type { ArtifactRow } from "../src/schema.js"; -import type { Identity } from "../src/ports.js"; export const DATABASE_URL = process.env.ARTIFACT_DATABASE_URL ?? @@ -188,17 +187,6 @@ export async function seedArtifact( ); } -/** A directory that answers exactly what a test wires into it, nothing more. */ -export function fakeIdentity(overrides: Partial = {}): Identity { - return { - ownerNames: async () => new Map(), - ownerMemberPrincipalId: async () => null, - principalIdsByKind: async () => [], - ownerIsMemberOfTenant: async () => false, - ...overrides, - }; -} - /** Bypasses `createArtifact` so a test can plant a kind the module refuses to mint. */ export async function seedSkillDraft(db: ArtifactDb, title: string): Promise { const rows = await db.execute<{ id: string }>(sql` diff --git a/src/tools.test.ts b/src/tools.test.ts index 431aafa..8b7f870 100644 --- a/src/tools.test.ts +++ b/src/tools.test.ts @@ -14,9 +14,8 @@ import { SAFE_ENCODED_BUDGET, windowContent, } from "./tools.js"; -import { fakeIdentity, seedArtifact, seedSkillDraft, SCOPE, testDb } from "./test-helpers.js"; +import { seedArtifact, seedSkillDraft, SCOPE, testDb } from "./test-helpers.js"; -const identity = fakeIdentity(); const base = { artifactId: "a1", title: "T", kind: "document", version: 1 }; const encoded = (value: unknown) => JSON.stringify(value, null, 2).length; @@ -72,7 +71,7 @@ describe("artifact_read", () => { const row = await seedArtifact(db, { title: "Doc", content: "v1" }); await writeArtifactVersion(db, { scope: SCOPE, artifactId: row.id, content: "v2" }); - const result = await readArtifact(db, identity, { scope: SCOPE, artifactId: row.id }); + const result = await readArtifact(db, { scope: SCOPE, artifactId: row.id }); expect(result).toMatchObject({ version: 2, content: "v2" }); }); @@ -86,7 +85,7 @@ describe("artifact_read", () => { content: "v2", }); - const result = await readArtifact(db, identity, { + const result = await readArtifact(db, { scope: SCOPE, artifactId: row.id, version: 1, @@ -98,7 +97,7 @@ describe("artifact_read", () => { const db = await testDb(); const row = await seedArtifact(db); await expect( - readArtifact(db, identity, { scope: SCOPE, artifactId: row.id, version: 7 }), + readArtifact(db, { scope: SCOPE, artifactId: row.id, version: 7 }), ).rejects.toThrow(/Version 7 not found/); }); @@ -106,38 +105,15 @@ describe("artifact_read", () => { const db = await testDb(); const id = await seedSkillDraft(db, "scratch"); await expect( - readArtifact(db, identity, { scope: SCOPE, artifactId: id }), + readArtifact(db, { scope: SCOPE, artifactId: id }), ).rejects.toBeInstanceOf(ArtifactNotFoundError); }); - test("a cross-tenant read fails closed when the owner is not a member there", async () => { + test("an artifact in another tenant is not found", async () => { const db = await testDb(); const row = await seedArtifact(db, { tenantId: "other" }); await expect( - readArtifact(db, identity, { - scope: SCOPE, - artifactId: row.id, - tenantId: "other", - }), - ).rejects.toBeInstanceOf(ArtifactNotFoundError); - }); - - test("a cross-tenant read succeeds when the owner IS a member there", async () => { - const db = await testDb(); - const row = await seedArtifact(db, { tenantId: "other", content: "shared" }); - const result = await readArtifact( - db, - fakeIdentity({ ownerIsMemberOfTenant: async () => true }), - { scope: SCOPE, artifactId: row.id, tenantId: "other" }, - ); - expect(result).toMatchObject({ content: "shared" }); - }); - - test("an artifact in another tenant is invisible without naming that tenant", async () => { - const db = await testDb(); - const row = await seedArtifact(db, { tenantId: "other" }); - await expect( - readArtifact(db, identity, { scope: SCOPE, artifactId: row.id }), + readArtifact(db, { scope: SCOPE, artifactId: row.id }), ).rejects.toBeInstanceOf(ArtifactNotFoundError); }); }); @@ -152,7 +128,7 @@ describe("web_site reads", () => { const db = await testDb(); const row = await seedArtifact(db, { kind: "web_site", content: site }); - const result = await readArtifact(db, identity, { scope: SCOPE, artifactId: row.id }); + const result = await readArtifact(db, { scope: SCOPE, artifactId: row.id }); expect(result).toMatchObject({ summary: { kind: "web_site", @@ -171,7 +147,7 @@ describe("web_site reads", () => { const db = await testDb(); const row = await seedArtifact(db, { kind: "web_site", content: site }); - const result = await readArtifact(db, identity, { + const result = await readArtifact(db, { scope: SCOPE, artifactId: row.id, path: "/style.css", @@ -184,10 +160,10 @@ describe("web_site reads", () => { const row = await seedArtifact(db, { kind: "web_site", content: site }); await expect( - readArtifact(db, identity, { scope: SCOPE, artifactId: row.id, path: "nope.js" }), + readArtifact(db, { scope: SCOPE, artifactId: row.id, path: "nope.js" }), ).rejects.toThrow(/File not found in web_site artifact/); await expect( - readArtifact(db, identity, { + readArtifact(db, { scope: SCOPE, artifactId: row.id, path: "../secret", @@ -199,7 +175,7 @@ describe("web_site reads", () => { const db = await testDb(); const row = await seedArtifact(db, { kind: "web_site", content: site }); await expect( - readArtifactChunk(db, identity, { scope: SCOPE, artifactId: row.id }), + readArtifactChunk(db, { scope: SCOPE, artifactId: row.id }), ).rejects.toThrow(/use artifact_read/); }); }); @@ -209,7 +185,7 @@ describe("artifact_read_chunk", () => { const db = await testDb(); const row = await seedArtifact(db, { content: "abcdefghij" }); - const result = await readArtifactChunk(db, identity, { + const result = await readArtifactChunk(db, { scope: SCOPE, artifactId: row.id, offset: 3, @@ -226,7 +202,7 @@ describe("artifact_read_chunk", () => { const row = await seedArtifact(db, { content: "original" }); await writeArtifactVersion(db, { scope: SCOPE, artifactId: row.id, content: "revised" }); - const result = await readArtifactChunk(db, identity, { + const result = await readArtifactChunk(db, { scope: SCOPE, artifactId: row.id, version: 1, @@ -335,7 +311,7 @@ describe("artifact_link_file", () => { test("a linked artifact is readable through artifact_read", async () => { const db = await testDb(); const row = await linkFileArtifact(db, linkArgs({ preview: "Slide 1: revenue" })); - const read = await readArtifact(db, identity, { scope: SCOPE, artifactId: row.id }); + const read = await readArtifact(db, { scope: SCOPE, artifactId: row.id }); expect(read).toMatchObject({ title: "Quarterly deck", version: 1, content: "Slide 1: revenue" }); }); }); diff --git a/src/tools.ts b/src/tools.ts index 373b16d..fa8a50d 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -7,7 +7,7 @@ import { SKILL_DRAFT_KIND, } from "./artifacts.js"; import { artifact, type ArtifactRow } from "./schema.js"; -import type { ResolvedPrincipal, Identity } from "./ports.js"; +import type { ResolvedPrincipal } from "./ports.js"; import { parseWebSiteContentJson, normalizeWebSitePath, @@ -99,31 +99,24 @@ export function windowContent( } /** - * Resolve an artifact for an agent read, honoring a version pin and an explicit - * cross-tenant target. skill-draft reads as NOT FOUND, not forbidden. + * Resolve an artifact for an agent read, honoring a version pin. Reads are + * always confined to the caller's tenant; there is no tenant override. A + * skill-draft reads as NOT FOUND, not forbidden. */ async function resolveForRead( db: ArtifactDb, - identity: Identity, args: { scope: ResolvedPrincipal; artifactId: string; version?: number; - tenantId?: string; }, ): Promise<{ base: ReadBase; content: string }> { - const tenantId = args.tenantId ?? args.scope.tenantId; - if ( - tenantId !== args.scope.tenantId && - !(await identity.ownerIsMemberOfTenant(args.scope, tenantId)) - ) { - throw new ArtifactNotFoundError(args.artifactId); - } - const [row] = await db .select() .from(artifact) - .where(and(eq(artifact.id, args.artifactId), eq(artifact.tenantId, tenantId))) + .where( + and(eq(artifact.id, args.artifactId), eq(artifact.tenantId, args.scope.tenantId)), + ) .limit(1); if (!row || row.kind === SKILL_DRAFT_KIND) { throw new ArtifactNotFoundError(args.artifactId); @@ -165,16 +158,14 @@ async function resolveForRead( */ export async function readArtifact( db: ArtifactDb, - identity: Identity, args: { scope: ResolvedPrincipal; artifactId: string; version?: number; - tenantId?: string; path?: string; }, ): Promise { - const { base, content } = await resolveForRead(db, identity, args); + const { base, content } = await resolveForRead(db, args); if (base.kind !== WEB_SITE_KIND) return windowContent(base, content); if (args.path === undefined) { @@ -191,17 +182,15 @@ export async function readArtifact( /** `artifact_read_chunk`: one bounded character range. Not for `web_site`. */ export async function readArtifactChunk( db: ArtifactDb, - identity: Identity, args: { scope: ResolvedPrincipal; artifactId: string; version?: number; - tenantId?: string; offset?: number; limit?: number; }, ): Promise { - const { base, content } = await resolveForRead(db, identity, args); + const { base, content } = await resolveForRead(db, args); if (base.kind === WEB_SITE_KIND) { throw new Error( "artifact_read_chunk does not support web_site artifacts; use artifact_read for a summary or pass path to read one file", @@ -339,11 +328,6 @@ export const ARTIFACT_TOOL_DEFINITIONS: readonly ArtifactToolDefinition[] = [ type: "number", description: "Optional version to read. Defaults to the latest.", }, - tenantId: { - type: "string", - description: - "Optional tenant the artifact lives in. Defaults to your own tenant.", - }, path: { type: "string", description: @@ -374,10 +358,6 @@ export const ARTIFACT_TOOL_DEFINITIONS: readonly ArtifactToolDefinition[] = [ type: "number", description: "Optional version to read. Defaults to the latest.", }, - tenantId: { - type: "string", - description: "Optional tenant the artifact lives in.", - }, }, required: ["artifactId"], }, diff --git a/src/web-site.test.ts b/src/web-site.test.ts index 9285d04..88b0a52 100644 --- a/src/web-site.test.ts +++ b/src/web-site.test.ts @@ -10,7 +10,6 @@ import { WEB_SITE_MAX_TOTAL_BYTES, WebSiteContentError, } from "./web-site.js"; -import { anonymousIdentity } from "./ports.js"; describe("path normalization", () => { test("strips leading slashes and converts backslashes", () => { @@ -113,19 +112,3 @@ describe("summary", () => { expect(summarizeWebSiteContent(raw).totalBytes).toBe(2); }); }); - -describe("default identity", () => { - test("the anonymous identity knows nobody and refuses cross-tenant reads", async () => { - expect(await anonymousIdentity.ownerNames("t", ["p"])).toEqual(new Map()); - expect( - await anonymousIdentity.ownerMemberPrincipalId({ tenantId: "t", principalId: "p" }), - ).toBeNull(); - expect(await anonymousIdentity.principalIdsByKind("t", "user")).toEqual([]); - expect( - await anonymousIdentity.ownerIsMemberOfTenant( - { tenantId: "t", principalId: "p" }, - "other", - ), - ).toBe(false); - }); -});