diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a67f59..99b4cda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,183 +7,68 @@ package follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Until 1.0, a minor bump may contain a breaking change; breaking changes are always called out under their own heading. -## [Unreleased] +## [0.2.0] — 2026-09-25 ### Added -- Run-scoped `POST /artifacts`, `POST /artifacts/binary`, and - `PATCH /artifacts/:id` (`mountWorkflowArtifacts`) accept an optional - `metadata` field, matching the tenant routes' semantics exactly: omitted on - a revise carries the prior version's metadata forward, an explicit `null` - clears it, and any other value must be a JSON object or the request is - `400`. `artifact_create` and `artifact_write` in `ARTIFACT_TOOL_DEFINITIONS` - gain a matching optional `metadata` object parameter, and the sidecar - bundle forwards it to the route unchanged. `source` (`{ origin: "workflow", - runId }`) and `generatedBy` stay server-stamped from the resolved run - scope — never read from `metadata` or any other body field. -- `artifact_version.content_sha256` (text, nullable), added by the new - `0005_version_content_digest` migration and mirrored onto - `artifact.content_sha256` the same way `metadata` already mirrors. Computed - at write time — sha256 (hex) over the UTF-8 bytes of `content` for text and - URL artifacts, or over the uploaded bytes for a blob-backed file artifact's - version 1, since its `content` column is a store-specific pointer, not the - bytes. A metadata/title-only revise (content omitted, so it carries - forward) carries the digest forward unchanged. Returned as `contentSha256` - on `GET /api/artifacts/:id`, `GET /api/artifacts/:id/versions/:version`, - `GET /api/artifacts/:id/versions`, and the `POST /api/artifacts/:id/versions` - response. Existing rows serialize `contentSha256: null` — written before - digests existed, and never backfilled. -- `expectedVersion` (positive integer, optional) on - `POST /api/artifacts/:id/versions`. Checked under the same `SELECT ... FOR - UPDATE` that guards the version bump: a mismatch answers `409 - {"error":"Version conflict","currentVersion":N}` and writes nothing. - Omitting it is today's unconditional-write behavior. Lets a caller bind a - revise to the exact version it last read — for example, a human approval on - specific content — instead of silently overwriting a change it never saw. -- `ARTIFACT_UPLOAD_POLICY` accepts packaged archives — `application/gzip` / - `application/x-gzip` (`.tar.gz`, `.tgz`, `.gz`) and `application/x-tar` - (`.tar`) — so consumers storing packaged builds are no longer refused with - 415. An archive mints kind `file`, is never inline-previewable (the - download path's inline allow-list is `application/pdf` only), and is still - subject to the existing `MAX_UPLOAD_BYTES` per-file ceiling. -- `GET /api/artifacts/:id/versions/:version` — one version including its - content, reusing `getArtifactVersion`, the same read authorization as - `GET /api/artifacts/:id`, and the same response shape. A malformed or - sub-1 version is `400`; an unknown version collapses into the same `404 - Artifact not found` every other single-artifact failure mode does. -- `GET /api/artifacts/:id/download?version=N` — pins the download to that - version's content for the data-URL and downloadable-text conventions, - where content really is per-version; omitting `version` is unchanged. Same - `400`/`404` rules as the new versions route, plus one more: a blob-backed - upload's `ContentStore` reference lives on the artifact row's own - `source`, never per-version, so `?version=N` for one is `400 "Uploaded - file content is not versioned"` unless `N` names the current version — - rather than silently answering with today's blob under an older version's - name. -- `artifact_version.metadata` (jsonb, nullable) and - `artifact_version.parent_version_ids` (text[], nullable), added by the new - `0004_version_metadata` migration. `metadata` is opaque to the package — - stored and returned as-is on every version read — and `artifact.metadata` - mirrors the current version's value the same way `title`/`content`/ - `version` already do. `parentVersionIds` is explicit lineage set by the - writer and is never inferred from version order or carried forward between - versions. `createArtifact`, `writeArtifactVersion`, and - `findOrVersionArtifact` all accept optional `metadata` and - `parentVersionIds`; `getArtifactVersion` and `listArtifactVersions` return - both fields alongside each version. -- `findOrVersionArtifact(db, args)` — the atomic primitive behind "find an - artifact by title, create it if absent, add a version if present." The - schema's only uniqueness is `(artifactId, version)`; nothing constrains - `(tenantId, title, kind)`, so that common pattern raced between the read - and the write when hand-rolled outside the package. This closes the race - with a transaction-scoped advisory lock keyed on `(tenantId, kind, title)`: - concurrent callers for the same triple always converge on one artifact — - the first to acquire the lock creates it, every other caller revises the - row the first one just committed. A uniqueness constraint on - `(tenant_id, title, kind)` was considered instead but rejected: - `createArtifact` is a public, unconditional insert used directly by the - import route, uploads, and `artifact_link_file`, and a shared title across - independent creates on those paths is normal, not a bug a schema - constraint should forbid. See the find-or-version notes in - CONTRIBUTING.md. +- `POST /api/artifacts/:id/versions` revises an uploaded file with new bytes + when sent as `multipart/form-data` with one `file` field and an optional + `expectedVersion`. The bytes go through the configured `ContentStore` and + upload policy (`415` for a refused type, `413` over `MAX_UPLOAD_BYTES`), + and are stored only after the version check passes. The title carries + forward; the version downloads under the new file's name. + `reviseFileArtifact` is the underlying function. +- Upload bytes are versioned. Each version records its own content + reference in `artifact_version.source` (added by `0004_version_source`, + which backfills existing versions), so `GET .../download?version=N` and + `GET .../versions/:version` return that version's file after a revision. ### Changed - `@intx/agent` is an optional peer. Only `@corbits/artifacts/sidecar-bundle` - imports it, so a host that mounts the routes alone need not install it. -- `arktype` is a regular dependency (`^2.2.3`) instead of a peer, so hosts no - longer install it themselves. The exported query schemas are still arktype - types; a host on another arktype version gets its own copy alongside. -- Minimum `@intx/*` is now **0.3.0**. (`@intx/*` lines before 0.3.0 do not - install — older lines pin the unpublished `@intx/*@0.0.0` or ship raw - TypeScript.) -- The repository root **is** the `@corbits/artifacts` package. The previous - `packages/artifacts` workspace nesting is gone so - `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. -- `createArtifactRoutes` takes an optional - `onArtifactCreated(tx, row, scope)` hook, run inside the same transaction - as artifact creation (once per row, so once on `POST /artifacts` and once - per file on `POST /artifacts/upload`). This is - the seam a host uses to provision grants for the row it just made — for - example, a `creator`-origin grant on `artifact:` for `write` and - `archive`. Defaults to a no-op, so existing hosts are unaffected. - `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 CONTRIBUTING.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. + imports it. +- `arktype` is a regular dependency instead of a peer. +- `@intx/db` is a new peer. The minimum `@intx/*` is **0.4.0**. ### Breaking - `mountArtifacts(app, opts)` is replaced by `createArtifactRoutes(deps)`, which returns a `Hono` sub-app the host mounts with - `app.route(...)` instead of mutating the host app. `MountArtifactsOpts` is - renamed `CreateArtifactRoutesDeps`; the options are unchanged. -- `POST /artifacts` and `POST /artifacts/upload` now require - `requireGrant("artifact:*", "create")`, as hub-api's `createGrantRoutes` - requires `create` on `grant:*`. A host must grant its principals `create` - on `artifact:*` for them to keep creating artifacts. An unauthenticated - caller of these two routes now gets `{ "error": "Forbidden" }` instead of - `{ "error": "Tenant not accessible" }`. + `app.route(...)`. `MountArtifactsOpts` is renamed `CreateArtifactRoutesDeps`. +- `mountWorkflowArtifacts(app, opts)` is replaced by + `createWorkflowArtifactRoutes(deps)`, a sub-app the host mounts at + `/api/workflow-artifacts`. `MountWorkflowArtifactsOpts` is renamed + `CreateWorkflowArtifactRoutesDeps`. +- `POST /artifacts` and `POST /artifacts/upload` require + `requireGrant("artifact:*", "create")`. On upgrade from 0.1.0, the + migrations grant `create` on `artifact:*` to every principal that has + already created an artifact in its tenant, in the host's `grant` table. + New principals need the grant from the host. An unauthenticated caller now gets + `{ "error": "Forbidden" }` instead of `{ "error": "Tenant not accessible" }`. - `runArtifactMigrations(config, { schema })` takes the same arguments as Interchange's `runMigrations`: a `DBConfig` and the host schema holding - `tenant` and `principal`. It applies the SQL files shipped under - `migrations/`, all idempotent, with no ledger. The `adopt` option, + `tenant` and `principal`. It applies the idempotent SQL files shipped under + `migrations/` with no ledger. The `adopt` option, `RunArtifactMigrationsOptions`, `MigrationChecksumError` and - `MigrationAdoptError` are removed, and the `artifacts.migrations` ledger - table is dropped on the next boot. + `MigrationAdoptError` are removed. +- The first 0.2.0 boot drops the `artifacts.migrations` ledger, so a + database cannot go back to 0.1.0. Do not run 0.1.0 and 0.2.0 replicas + against the same database. - The drizzle tables (`artifact`, `artifactVersion`, `upload`, - `mailAttachmentRef`) are no longer exported from the package entry. Hosts - reach artifacts through the routes and functions; `ARTIFACTS_SCHEMA` and the + `mailAttachmentRef`) are no longer exported. `ARTIFACTS_SCHEMA` and the `*Row` types stay public. -- The tenant routes take `Hono`, read the host-provided tenant and - principal context natively, and require the host's Interchange `RequireGrant` - middleware. The `resolvePrincipal`, `isAdmin`, and `identity` options and the - `Identity` / `anonymousIdentity` exports are not part of the package surface. -- Serialized artifact rows expose `ownerPrincipalId` without an `ownerName`. - 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. -- `SKILL_DRAFT_KIND` is removed. `skill-draft` is no longer a reserved kind: - create, list, find-by-title and every read treat it like any other `kind`. -- `web_site` handling is removed: `web_site` content is no longer normalized - on write, `readArtifact` no longer takes `path` or returns a site summary, - `artifact_read_chunk` no longer refuses it, and the sidecar's - `artifact_read` no longer forwards `path`. `WEB_SITE_KIND`, - `WEB_SITE_MAX_FILES`, `WEB_SITE_MAX_PATH_LENGTH`, `WEB_SITE_MAX_TOTAL_BYTES`, - `WebSiteContentError`, `normalizeWebSiteContent`, `normalizeWebSitePath`, - `parseWebSiteContentJson`, `serializeWebSiteContent`, - `summarizeWebSiteContent`, `WebSiteContent` and `WebSiteReadSummary` are no - longer exported. -- Mail attachment references are removed: `POST` and `GET - /instances/:instanceId/mail-attachments`, `saveMailAttachmentRefs`, +- `SKILL_DRAFT_KIND` is removed. `skill-draft` is an ordinary `kind`. +- `web_site` handling is removed: content is stored as given, `readArtifact` + no longer takes `path`, and every `WEB_SITE_*` constant and web-site + helper and type is no longer exported. +- Mail attachment references are removed: the + `/instances/:instanceId/mail-attachments` routes, `saveMailAttachmentRefs`, `listMailAttachmentRefs`, `MAIL_ATTACHABLE_KINDS`, `MailAttachmentKindError`, `MAX_MAIL_ATTACHMENT_BYTES`, - `MAX_MAIL_ATTACHMENTS_PER_MAIL` and `MailAttachmentRefRow`. The new - `0003_drop_mail_attachment_ref` migration drops the `mail_attachment_ref` - table and its rows. -- `windowContent` is no longer exported; it is internal to the tool reads. + `MAX_MAIL_ATTACHMENTS_PER_MAIL` and `MailAttachmentRefRow`. + `0003_drop_mail_attachment_ref` drops the table and its rows. +- `windowContent` is no longer exported. +- Node 24 or newer is required (0.1.0 accepted Node 22). ## [0.1.0] — first release @@ -214,5 +99,5 @@ new; the list below is what the surface consists of rather than what changed. (`@intx/*` 0.1.2 does not install — its deps pin the unpublished `@intx/*@0.0.0` — and ships raw TypeScript.) -[Unreleased]: https://github.com/corbitsdev/corbits-artifacts +[0.2.0]: https://github.com/corbitsdev/corbits-artifacts [0.1.0]: https://github.com/corbitsdev/corbits-artifacts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 44b5a39..54bf22d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,7 +14,8 @@ docker run -d --name corbits-artifact-pg -p 5457:5432 \ export ALLOW_DESTRUCTIVE_ARTIFACT_TESTS=1 bun run typecheck -bun run test # unit, integration and reference-host acceptance +bun run test # unit +bun run test:e2e # real-Postgres and reference-host acceptance bun run build # dist/ (JS + .d.ts) ``` @@ -37,18 +38,18 @@ couple of migration cases. Those paths are fail-closed: set ephemeral database (`artifact_core`, or any name ending in `_test`). Without both, the suite throws before mutating. -End-to-end suites live in `tests/`. `tests/lib/db-harness.ts` creates a fresh +End-to-end suites live in `e2e/`. `e2e/helpers.ts` creates a fresh `artifact__test` database per suite on the `ARTIFACT_DATABASE_URL` server, applies Interchange's `runMigrations` and `runArtifactMigrations`, and drops it afterwards. `artifactApp` mounts `createArtifactRoutes` for a seeded tenant principal, authorized by the platform's real `createRequireGrant` over the -database's `grant` table. `bun run test` runs `src/` and `tests/`. +database's `grant` table. `bun run test` runs `src/`; `bun run test:e2e` runs `e2e/`. ## The reference host is the acceptance suite, not a demo `examples/reference-host` mounts the package on a real `@intx/hub-api` app against a -live Postgres. `tests/reference-host.test.ts` asserts the end-to-end scenarios against -it as part of `bun run test`; the example imports `@corbits/artifacts`, which the +live Postgres. `e2e/reference-host.test.ts` asserts the end-to-end scenarios against +it as part of `bun run test:e2e`; the example imports `@corbits/artifacts`, which the root `tsconfig.json` maps to `src/`. CI's Node consumer smoke test covers the built `dist/` a consumer installs. @@ -80,6 +81,9 @@ boot, so every statement must be idempotent (`IF NOT EXISTS`, `IF EXISTS`). Ther is no ledger: a schema change is a new file whose statements are safe to re-run, never an edit that assumes it runs once. +Because every file re-runs on every boot, a data backfill must be cheap once it has +run: guard it so an already-migrated database does no work beyond a quick check. + `schema.ts` and `migrations/` must agree — every query goes through the drizzle table objects, and the route suites fail when a column they write is missing. Change one, change the other, in the same commit. @@ -393,3 +397,10 @@ authenticated `tenant`/`principal` on the request context; the host's - **One 404 covers three causes** for a resolved caller — never minted, malformed, or another tenant's. Distinguishing them would be an existence oracle. Expect no more detail than that from the API. + +## Commit messages + +Commit subjects and PR titles follow [Conventional Commits](https://www.conventionalcommits.org): `feat`, `fix`, `refactor`, `test`, `docs`, `build`, `ci`, `perf`, and `chore(release): x.y.z` for releases. +Add `!` only for public API breaks: removed or renamed exports, changed signatures, newly required params. Peer and dependency range changes are `build(deps):` with no `!`. +Keep subjects imperative, lowercase after the colon, 72 characters or less, and free of ticket IDs. +Every PR links its issue with a `Closes ` line in the PR body. diff --git a/README.md b/README.md index 975d7ce..c389b67 100644 --- a/README.md +++ b/README.md @@ -1,105 +1,158 @@ # @corbits/artifacts -An artifact is a versioned document or file, stored in Postgres and served over HTTP to a tenant's users and to its workflow runs. This package adds those routes to an Interchange host's Hono app; the host owns the app, the database pool, the session, and the grant store. Backend only — it ships no UI. +[![npm](https://img.shields.io/npm/v/@corbits/artifacts.svg)](https://www.npmjs.com/package/@corbits/artifacts) [![License: LGPL-2.1](https://img.shields.io/badge/license-LGPL--2.1-green.svg)](https://github.com/corbitsdev/corbits-artifacts/blob/main/LICENSE) -## Quickstart +Versioned documents and files for Interchange agents and their users: a Corbits hub module that mounts grant-gated Hono routes on `@intx/hub-api`, keeps versions in Postgres and bytes in a pluggable `ContentStore`, and ships agent tools for the sidecar. + +## Why @corbits/artifacts? + +1. **Every version is kept.** Each revision of a document or uploaded file is a new version with its own content, bytes and SHA-256 digest. `expectedVersion` turns a revise into a compare-and-set. +2. **Mounts like any hub route.** `createArtifactRoutes` returns a `Hono` sub-app. Reads are confined to the caller's tenant, and writes run through the hub's `requireGrant`. +3. **Agents write through the same store.** A run-scoped route set and a sidecar tool pack let a workflow run create, read and revise the artifacts its users see. +4. **Migrations that replay safely.** Every SQL file is idempotent and runs on every boot under an advisory lock, so several replicas can start at once. + +It ships no UI and no object store: the host renders artifacts and brings its own `ContentStore` for large files. + +## Install ```bash -npm add @corbits/artifacts +bun add @corbits/artifacts \ + @intx/agent @intx/authz @intx/db @intx/hub-api @intx/types \ + drizzle-orm hono hono-openapi postgres ``` -Requires Node 24 or newer and `@intx/*` 0.4.0 or newer. +Add `@intx/agent` only if an agent uses the sidecar tools. Runs on Node >= 24. -At boot, right after Interchange's `runMigrations`, apply this package's migrations with the same `config` and `schema`. The tables go in their own `artifacts` Postgres schema, with tenant and principal foreign keys pointing into `schema`. Then open a database handle for the routes; a host that already has a drizzle handle passes that instead of calling `createArtifactDb`. +## Quickstart -```ts -import { runMigrations } from "@intx/db"; -import { createArtifactDb, runArtifactMigrations } from "@corbits/artifacts"; +With `DATABASE_URL` pointing at a hub database that has run `runArtifactMigrations` (see [Using with Interchange](#using-with-interchange)): -// `config` is the host's `DBConfig` from `@intx/db`. -await runMigrations(config, { schema: "public" }); -await runArtifactMigrations(config, { schema: "public" }); +```ts +import { + createArtifact, + createArtifactDb, + getArtifact, +} from "@corbits/artifacts"; const { db, close } = createArtifactDb(process.env.DATABASE_URL!); -// on shutdown +const created = await db.transaction((tx) => + createArtifact(tx, { + scope: { tenantId: "acme", principalId: "alice" }, + ownerPrincipalId: "alice", + kind: "document", + title: "Notes", + content: "Hello", + source: { origin: "manual" }, + }), +); +console.log(await getArtifact(db, created.id)); await close(); ``` -### 1. Hub-side, tenant-scoped: `createArtifactRoutes` +`acme` and `alice` must be a tenant and principal in the hub. It prints the artifact at version 1. -Returns a `Hono` sub-app the host mounts with `app.route`, alongside its other session-authenticated routes. It reads `principal`/`tenant` off the Hono context (placed there by the host's own auth + tenant middleware) and authorizes mutations through the host's `requireGrant` — built from Interchange's `createRequireGrant` over the host's own `GrantStore` and `ConditionRegistry`. +## Where it fits -| `deps` | Type | What the host provides | -| --- | --- | --- | -| `db` | `ArtifactDb` | Artifacts are stored there. `createArtifactDb` opens a handle for a host with none; a hub that already has one passes it through. | -| `contentStore` | `ContentStore` | Blob storage for file bytes. `InlineContentStore` (exported by this package) fits a minimal host; bring your own store for object storage. | -| `requireGrant` | `RequireGrant` | The host's grant middleware factory. This package implements no ownership or membership policy of its own. Creating an artifact requires `create` on `artifact:*`; revising or archiving one requires `write` or `archive` on `artifact:`. | -| `countSegments` | `ArtifactCountSegments` (optional) | Named predicates over `ArtifactListRow` for `GET /artifacts/counts` (e.g. bucket by `kind`). The taxonomy is entirely host-owned; omitted, the route still answers with the tenant-wide `all` total. | -| `onArtifactCreated` | `(tx, row, scope) => Promise` (optional) | Runs inside the transaction that creates each artifact. This is where the host mints grants for the new row, e.g. `write` and `archive` on `artifact:` for its creator. The package mints none itself. | -| `decorate` | `(tenantId, rows) => Promise` (optional) | Adds display-only fields to serialized rows on the way out (provenance labels, host joins). It must never change which rows are returned or who may see them. | -| `uploadPolicy` | `UploadPolicy` (optional) | Which MIME types `POST /artifacts/upload` accepts. Defaults to `ARTIFACT_UPLOAD_POLICY`. | +[Interchange](https://github.com/faremeter/interchange) runs AI agents as principals: accounts with their own identity, permissions and credentials. Its hub is the multi-tenant control plane that holds tenants, principals and grants (permissions a principal holds on a resource); its sidecar is the agent runtime. -```ts -import { createRequireGrant } from "@intx/hub-api"; -import { createArtifactRoutes, InlineContentStore } from "@corbits/artifacts"; - -// `app` is the host's Hono; its auth + tenant middleware has -// already placed `tenant` and `principal` on the context. -app.route( - "/api", - createArtifactRoutes({ - db, - contentStore: InlineContentStore, - requireGrant: createRequireGrant({ grantStore, conditionRegistry }), - }), -); -``` +- **Runs in:** the hub, as routes on its Hono app and tables in its Postgres (`artifacts` schema). +- **Plugs into:** [`@intx/hub-api`](https://github.com/faremeter/interchange/tree/main/packages/hub-api) routes and grants, [`@intx/db`](https://github.com/faremeter/interchange/tree/main/packages/db) (its `DBConfig`, and its `tenant` and `principal` tables as FK targets), and [`@intx/agent`](https://github.com/faremeter/interchange/tree/main/packages/agent) tools on the sidecar. +- **Pairs with:** [`@corbits/mailbox`](https://github.com/corbitsdev/corbits-mailbox) and [`@corbits/memory`](https://github.com/corbitsdev/corbits-memory), the other Corbits hub modules, and [`@corbits/agent-token`](https://github.com/corbitsdev/corbits-agent-token) for agent bearer tokens. -### 2. Hub-side, run-scoped: `mountWorkflowArtifacts` +## Reference -A parallel mount for a workflow run, which has no browser session — only a bearer token and an `x-workflow-run-address` header. Mount it at `/api/workflow-artifacts`, the path the sidecar bundle below calls, rather than under the tenant prefix. `resolveRunScope` is the host's existing sidecar-token → run lookup; `agentToken` is a second, optional auth path so a deployed agent can present the bearer the hub minted for its own definition instead of the sidecar's token. +### `createArtifactRoutes(deps)` -| `opts` | Type | What the host provides | -| --- | --- | --- | -| `db`, `contentStore` | `ArtifactDb`, `ContentStore` | Same as above. | -| `resolveRunScope` | `WorkflowRunResolver` | `(bearerToken, runAddress) => ResolvedWorkflowRunScope \| null`. Returning `null` answers 401 — this package makes no assumption about how a host issues or verifies its sidecar tokens. | -| `agentToken` | `AgentTokenAuth` (optional) | `{ verify(ctx), resolveRun(runAddress) }`. `verify` returns `undefined` for "not an agent token" (the sidecar path is tried instead) and refuses a bearer whose tenant doesn't match the resolved run's. | -| `uploadPolicy` | `UploadPolicy` (optional) | Which MIME types `POST /artifacts/binary` accepts. Defaults to `ARTIFACT_UPLOAD_POLICY`. | -| `maxBinaryBytes` | `number` (optional) | Byte ceiling for `POST /artifacts/binary`. Defaults to `MAX_UPLOAD_BYTES`. | -| `maxContentChars` | `number` (optional) | Character ceiling for `content` on `POST /artifacts`. Defaults to 64,000. | +| `deps` | Type | What the host provides | +| ------------------- | ----------------------------------- | -------------------------------------------------------------------------------------------------------------------- | +| `db` | `ArtifactDb` | The hub's drizzle handle, for example from `createDB`. | +| `contentStore` | `ContentStore` | Where file bytes go. `InlineContentStore` keeps them in Postgres; implement the port for object storage. | +| `requireGrant` | `RequireGrant` | From `@intx/hub-api`'s `createRequireGrant`. | +| `onArtifactCreated` | `(tx, row, scope) => Promise` | Optional. Runs in the creating transaction; mint the creator's `write` and `archive` grants on `artifact:` here. | +| `decorate` | `(tenantId, rows) => Promise` | Optional. Adds display-only fields to serialized rows. It must not change which rows are returned. | +| `uploadPolicy` | `UploadPolicy` | Optional. MIME types `POST /artifacts/upload` accepts. Defaults to `ARTIFACT_UPLOAD_POLICY`. | +| `countSegments` | `ArtifactCountSegments` | Optional. Named predicates for `GET /artifacts/counts`. | + +| Route | Grant | Does | +| ------------------------------------------ | ---------------------------- | --------------------------------------------------------------------- | +| `GET /artifacts` | none (tenant-scoped) | Lists artifacts, paginated. Empty `200` with no principal. | +| `GET /artifacts/counts` | none (tenant-scoped) | Counts per `countSegments` bucket, plus `all`. | +| `POST /artifacts` | `create` on `artifact:*` | Creates a text or URL artifact at version 1. | +| `POST /artifacts/upload` | `create` on `artifact:*` | Uploads one or more files (`multipart/form-data`), one artifact each. | +| `GET /artifacts/:id` | none (tenant-scoped) | The latest version. | +| `GET /artifacts/:id/versions` | none (tenant-scoped) | Version history. | +| `GET /artifacts/:id/versions/:version` | none (tenant-scoped) | One version, with its content. | +| `POST /artifacts/:id/versions` | `write` on `artifact:` | Adds a version from JSON, or from a new `file` for an uploaded file. | +| `POST /artifacts/:id/archive`, `unarchive` | `archive` on `artifact:` | Archives or restores. | +| `GET /artifacts/:id/download?version=N` | none (tenant-scoped) | The bytes of a version, latest by default. | +| `GET /artifacts/:id/preview` | none (tenant-scoped) | A `text/html` artifact under a sandboxing CSP; `415` otherwise. | + +With no principal, every route except the list and counts answers `403`. Another tenant's artifact, or an unknown id, answers `404`. Set a request-body limit on the host for the upload and file-revise routes; they buffer the body before checking `MAX_UPLOAD_BYTES`. + +### `createWorkflowArtifactRoutes(deps)` + +Run-scoped routes for a workflow run, which authenticates with a bearer token and an `x-workflow-run-address` header instead of a session. + +| `deps` | Type | What the host provides | +| ----------------- | --------------------- | -------------------------------------------------------------------------------------------- | +| `db` | `ArtifactDb` | Same as above. | +| `contentStore` | `ContentStore` | Same as above. | +| `resolveRunScope` | `WorkflowRunResolver` | `(bearerToken, runAddress) => ResolvedWorkflowRunScope \| null`. `null` answers `401`. | +| `agentToken` | `AgentTokenAuth` | Optional. Accepts an agent's own hub token as a second way in. | +| `uploadPolicy` | `UploadPolicy` | Optional. MIME types `POST /artifacts/binary` accepts. Defaults to `ARTIFACT_UPLOAD_POLICY`. | +| `maxBinaryBytes` | `number` | Optional. Byte ceiling for `POST /artifacts/binary`. Defaults to `MAX_UPLOAD_BYTES`. | +| `maxContentChars` | `number` | Optional. Character ceiling for `content` on `POST /artifacts`. Defaults to 64,000. | + +### `runArtifactMigrations(config, { schema })` + +Takes the same `DBConfig` and `schema` as Interchange's `runMigrations`. `schema` holds the host's `tenant` and `principal` tables; this package's tables always go in `artifacts`. + +### `@corbits/artifacts/sidecar-bundle` + +`artifacts` is an `@intx/agent` tool pack: `artifact_create`, `artifact_write`, `artifact_read`, `artifact_read_chunk`, `artifact_list`, `artifact_find_by_title` and `artifact_link_file`. They call the run-scoped routes through the agent's `hub` credential. + +## Using with Interchange + +Run the migrations after Interchange's, mount both route sets, grant `create` on `artifact:*` to principals that create artifacts, and give agents the tool pack. ```ts -import type { Hono } from "hono"; +import { timeWindowEvaluator } from "@intx/authz"; +import { createDB, createGrantStore, runMigrations } from "@intx/db"; +import { createRequireGrant } from "@intx/hub-api"; import { + createArtifactRoutes, InlineContentStore, - mountWorkflowArtifacts, - type ArtifactDb, - type WorkflowArtifactEnv, - type WorkflowRunResolver, - type AgentTokenAuth, + runArtifactMigrations, } from "@corbits/artifacts"; -export function mountWorkflowArtifactRoutes( - app: Hono, - deps: { - db: ArtifactDb; - resolveRunScope: WorkflowRunResolver; - agentToken?: AgentTokenAuth; - }, -): void { - mountWorkflowArtifacts(app, { - db: deps.db, - contentStore: InlineContentStore, - resolveRunScope: deps.resolveRunScope, - ...(deps.agentToken !== undefined ? { agentToken: deps.agentToken } : {}), - }); -} +const dbConfig = { + host: "localhost", + port: 5432, + user: "postgres", + password: "postgres", + database: "interchange", +}; + +await runMigrations(dbConfig, { schema: "public" }); +await runArtifactMigrations(dbConfig, { schema: "public" }); + +const { db } = createDB(dbConfig); +const requireGrant = createRequireGrant({ + grantStore: createGrantStore(db), + conditionRegistry: { time_window: timeWindowEvaluator }, +}); + +export const artifactRoutes = createArtifactRoutes({ + db, + contentStore: InlineContentStore, + requireGrant, +}); ``` -### 3. Agent/tool side: `@corbits/artifacts/sidecar-bundle` +Mount `artifactRoutes` on the hub app at `/api`, behind the hub's auth and tenant middleware. -A deployed agent doesn't write its own artifact client — it imports the tool factory this package ships and adds it to its tool list. The factory resolves a `hub` credential from the runtime capabilities the host injects and calls the run-scoped routes above through it; it holds no database handle and no secret of its own. +For agents, mount `createWorkflowArtifactRoutes({ db, contentStore: InlineContentStore, resolveRunScope })` at `/api/workflow-artifacts`. `resolveRunScope` is the host's `(bearerToken, runAddress)` lookup that returns the run's tenant and principal from the hub's workflow runs, or `null`. The sidecar tools call `/api/workflow-artifacts`. Add them to an agent and bind its `hub` credential to the agent's hub token when you deploy it: ```ts import { defineAgent, type InferencePreference } from "@intx/agent"; @@ -116,21 +169,17 @@ export function buildAssistant(sources: readonly InferencePreference[]) { } ``` -When the host deploys this agent definition, it must bind the agent's `hub` credential to the agent's hub token. The tools send every request through that credential, so without the binding they cannot reach the hub. - -## Upgrading from 0.1.0 - -### Breaking - -- The `skill-draft` kind is no longer reserved. It is an ordinary `kind` string, created, listed and read like any other. -- The `web_site` kind has no special handling: its content is stored as given, and `artifact_read` no longer takes `path` or returns a site summary. The `web-site` exports (`WEB_SITE_KIND`, `WebSiteContentError`, `normalizeWebSiteContent` and the rest) are removed. -- `/instances/:instanceId/mail-attachments`, `saveMailAttachmentRefs`, `listMailAttachmentRefs` and the other mail-attachment exports are removed, and `runArtifactMigrations` drops the `mail_attachment_ref` table. -- `windowContent` is no longer exported. +## Upgrading from 0.1 -## Contributing +- `mountArtifacts(app, opts)` is now `createArtifactRoutes(deps)`, and `mountWorkflowArtifacts(app, opts)` is now `createWorkflowArtifactRoutes(deps)`. Mount both with `app.route`. +- `POST /artifacts` and `POST /artifacts/upload` need `create` on `artifact:*`. The upgrade grants it to every principal that has already created an artifact in its tenant; grant it to new principals yourself. +- `runArtifactMigrations(db)` is now `runArtifactMigrations(dbConfig, { schema })`. Existing 0.1.0 data upgrades in place on the first boot. +- That boot drops the 0.1.0 migration ledger. You cannot roll back to 0.1.0, and 0.1.0 and 0.2.0 replicas must not share a database. +- The drizzle tables, the `web_site` helpers, `SKILL_DRAFT_KIND`, `windowContent` and the mail-attachment routes and helpers are removed. The `mail_attachment_ref` table is dropped. +- Node 24 or newer is required. -See [CONTRIBUTING.md](./CONTRIBUTING.md). +See the [changelog](https://github.com/corbitsdev/corbits-artifacts/blob/main/CHANGELOG.md) for the full list. ## License -LGPL-2.1-only. See [LICENSE](./LICENSE). +[LGPL-2.1-only](https://github.com/corbitsdev/corbits-artifacts/blob/main/LICENSE) diff --git a/bunfig.toml b/bunfig.toml index 350b164..a63b96e 100644 --- a/bunfig.toml +++ b/bunfig.toml @@ -16,4 +16,4 @@ coverageThreshold = 0.8 # schema.ts is pure declarations — its "functions" are drizzle index/column # callbacks the coverage instrumentation cannot attribute, not logic. -coveragePathIgnorePatterns = ["src/schema.ts", "src/test-helpers.ts", "tests/lib"] +coveragePathIgnorePatterns = ["src/schema.ts", "e2e/helpers.ts", "e2e/fixtures.ts"] diff --git a/src/agent-token-mount.test.ts b/e2e/agent-token-mount.test.ts similarity index 93% rename from src/agent-token-mount.test.ts rename to e2e/agent-token-mount.test.ts index 4c2b2a4..2ea6160 100644 --- a/src/agent-token-mount.test.ts +++ b/e2e/agent-token-mount.test.ts @@ -1,14 +1,13 @@ import { describe, expect, test } from "bun:test"; -import { Hono } from "hono"; import { - mountWorkflowArtifacts, + createWorkflowArtifactRoutes, type AgentTokenAuth, type ResolvedWorkflowRunScope, - type WorkflowArtifactEnv, -} from "./workflow-mount.js"; -import { InlineContentStore } from "./content-store.js"; -import { seedArtifact, testDb } from "./test-helpers.js"; -import type { ArtifactDb } from "./db.js"; +} from "../src/workflow-mount.js"; +import { InlineContentStore } from "../src/content-store.js"; +import { seedArtifact } from "./fixtures.js"; +import { testDb } from "./helpers.js"; +import type { ArtifactDb } from "../src/db.js"; const RUN_SCOPE: ResolvedWorkflowRunScope = { tenantId: "acme", @@ -33,8 +32,7 @@ function agentTokenAuth(overrides: Partial = {}): AgentTokenAuth } function host(db: ArtifactDb, agentToken: AgentTokenAuth = agentTokenAuth()) { - const app = new Hono(); - return mountWorkflowArtifacts(app, { + return createWorkflowArtifactRoutes({ db, contentStore: InlineContentStore, // The sidecar path stays wired: an agent token is a second way in, not a diff --git a/src/artifacts.test.ts b/e2e/artifacts.test.ts similarity index 97% rename from src/artifacts.test.ts rename to e2e/artifacts.test.ts index bf66d77..72fca62 100644 --- a/src/artifacts.test.ts +++ b/e2e/artifacts.test.ts @@ -16,9 +16,10 @@ import { setArtifactArchived, sha256Hex, writeArtifactVersion, -} from "./artifacts.js"; -import { artifact, artifactVersion } from "./schema.js"; -import { seedArtifact, SCOPE, testDb } from "./test-helpers.js"; +} from "../src/artifacts.js"; +import { artifact, artifactVersion } from "../src/schema.js"; +import { seedArtifact, SCOPE } from "./fixtures.js"; +import { testDb } from "./helpers.js"; describe("create", () => { test("writes version 1 eagerly, so a pinned read of v1 resolves immediately", async () => { @@ -34,6 +35,7 @@ describe("create", () => { metadata: null, parentVersionIds: null, contentSha256: sha256Hex("first"), + source: { origin: "manual" }, }); }); @@ -172,6 +174,9 @@ describe("find by title", () => { const db = await testDb(); const older = await seedArtifact(db, { title: "Report" }); await seedArtifact(db, { title: "Report" }); + // Both seeds land within the same millisecond as the touch below often + // enough to tie; move them into the past so the touch is strictly newer. + await db.update(artifact).set({ updatedAt: new Date(Date.now() - 60_000) }); await writeArtifactVersion(db, { scope: SCOPE, artifactId: older.id, diff --git a/src/content-store.test.ts b/e2e/content-store.test.ts similarity index 95% rename from src/content-store.test.ts rename to e2e/content-store.test.ts index 2ca42b0..10d612c 100644 --- a/src/content-store.test.ts +++ b/e2e/content-store.test.ts @@ -4,18 +4,19 @@ import { InlineContentStore, decodeDataUrl, uploadRefFromSource, -} from "./content-store.js"; -import { normalizeSource } from "./artifacts.js"; -import { resolveDownload } from "./download.js"; +} from "../src/content-store.js"; +import { normalizeSource } from "../src/artifacts.js"; +import { resolveDownload } from "../src/download.js"; import { ARTIFACT_UPLOAD_POLICY, createFileArtifact, uploadArtifactKind, -} from "./uploads.js"; -import { upload } from "./schema.js"; -import type { ContentStore } from "./ports.js"; -import { seedArtifact, SCOPE, testDb } from "./test-helpers.js"; -import type { ArtifactDb } from "./db.js"; +} from "../src/uploads.js"; +import { upload } from "../src/schema.js"; +import type { ContentStore } from "../src/ports.js"; +import { seedArtifact, SCOPE } from "./fixtures.js"; +import { testDb } from "./helpers.js"; +import type { ArtifactDb } from "../src/db.js"; const PNG = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 1, 2, 3]); const PDF = new Uint8Array(Buffer.from("%PDF-1.4 body")); diff --git a/e2e/file-round-trip.test.ts b/e2e/file-round-trip.test.ts new file mode 100644 index 0000000..2f1584b --- /dev/null +++ b/e2e/file-round-trip.test.ts @@ -0,0 +1,96 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { mkdtemp, readdir, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { Hono } from "hono"; +import type { TenantEnv } from "@intx/hub-api"; +import { grant, seedActor } from "./fixtures.js"; +import { + artifactApp, + createTestDb, + type TestDb, + createFsContentStore, +} from "./helpers.js"; + +let testDb: TestDb; +let dir: string; +let app: Hono; + +beforeAll(async () => { + testDb = await createTestDb(); + dir = await mkdtemp(join(tmpdir(), "artifact-store-")); + const actor = await seedActor(testDb.db, "acme"); + await grant(testDb.db, actor, "artifact:*", "write"); + app = artifactApp(testDb.db, actor, createFsContentStore(dir)); +}); + +afterAll(async () => { + await testDb?.close(); + await rm(dir, { recursive: true, force: true }); +}); + +async function download(id: string, version?: number): Promise { + const query = version === undefined ? "" : `?version=${version}`; + return await app.request(`/api/artifacts/${id}/download${query}`); +} + +describe("upload, version, download on a filesystem ContentStore", () => { + test("each version downloads its own bytes", async () => { + const v1Bytes = new Uint8Array([0x25, 0x50, 0x44, 0x46, 0x00, 0xff, 0x10, 0x80]); + const v3Bytes = new Uint8Array([0x25, 0x50, 0x44, 0x46, 0x01, 0x02]); + const form = new FormData(); + form.append("files", new File([v1Bytes], "report.pdf", { type: "application/pdf" })); + const uploaded = await app.request("/api/artifacts/upload", { method: "POST", body: form }); + expect(uploaded.status).toBe(201); + const { artifacts } = (await uploaded.json()) as { artifacts: { id: string }[] }; + const id = artifacts[0]!.id; + + const renamed = await app.request(`/api/artifacts/${id}/versions`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ title: "report (final).pdf" }), + }); + expect(renamed.status).toBe(200); + + const revise = new FormData(); + revise.append("file", new File([v3Bytes], "report.pdf", { type: "application/pdf" })); + const revised = await app.request(`/api/artifacts/${id}/versions`, { + method: "POST", + body: revise, + }); + expect(revised.status).toBe(200); + expect(await revised.json()).toMatchObject({ version: 3 }); + + for (const [version, bytes] of [ + [1, v1Bytes], + [2, v1Bytes], + [3, v3Bytes], + [undefined, v3Bytes], + ] as const) { + const res = await download(id, version); + expect(res.status).toBe(200); + expect(new Uint8Array(await res.arrayBuffer())).toEqual(bytes); + } + + const sources = []; + for (const version of [1, 3]) { + const res = await app.request(`/api/artifacts/${id}/versions/${version}`); + expect(res.status).toBe(200); + sources.push(((await res.json()) as { artifact: { source: unknown } }).artifact.source); + } + expect(sources[0]).toMatchObject({ upload: { size: v1Bytes.length } }); + expect(sources[1]).toMatchObject({ upload: { size: v3Bytes.length } }); + + const storedFiles = async () => (await readdir(dir, { recursive: true })).length; + const before = await storedFiles(); + const stale = new FormData(); + stale.append("file", new File([v3Bytes], "report.pdf", { type: "application/pdf" })); + stale.append("expectedVersion", "1"); + const conflict = await app.request(`/api/artifacts/${id}/versions`, { + method: "POST", + body: stale, + }); + expect(conflict.status).toBe(409); + expect(await storedFiles()).toBe(before); + }); +}); diff --git a/e2e/fixtures.ts b/e2e/fixtures.ts new file mode 100644 index 0000000..d17ed78 --- /dev/null +++ b/e2e/fixtures.ts @@ -0,0 +1,92 @@ +import { schema as intx } from "@intx/db"; +import { generateId } from "@intx/hub-common"; +import { createArtifact } from "../src/artifacts.js"; +import type { ArtifactDb } from "../src/db.js"; +import type { ArtifactRow } from "../src/schema.js"; +import type { HostDb } from "./helpers.js"; + +type Tenant = typeof intx.tenant.$inferSelect; +type Principal = typeof intx.principal.$inferSelect; + +export type Actor = { tenant: Tenant; principal: Principal }; + +/** A tenant with one active user principal, allowed to create artifacts. */ +export async function seedActor(db: HostDb, slug: string): Promise { + const [tenant] = await db + .insert(intx.tenant) + .values({ + id: generateId("tenant"), + name: slug, + slug, + domain: `${slug}.example`, + }) + .returning(); + const [principal] = await db + .insert(intx.principal) + .values({ + id: generateId("principal"), + tenantId: tenant!.id, + kind: "user", + refId: `user-${slug}`, + status: "active", + }) + .returning(); + await grant( + db, + { tenant: tenant!, principal: principal! }, + "artifact:*", + "create", + ); + return { tenant: tenant!, principal: principal! }; +} + +export async function grant( + db: HostDb, + actor: Actor, + resource: string, + action: string, +): Promise { + await db.insert(intx.grant).values({ + id: generateId("grant"), + tenantId: actor.tenant.id, + principalId: actor.principal.id, + roleId: null, + resource, + action, + effect: "allow", + origin: "system", + conditions: null, + }); +} + +export const SCOPE = { tenantId: "acme", principalId: "user-1" }; + +export async function seedArtifact( + db: ArtifactDb, + overrides: Partial<{ + kind: string; + title: string; + content: string; + source: Record; + ownerPrincipalId: string | null; + tenantId: string; + }> = {}, +): Promise { + const scope = { + tenantId: overrides.tenantId ?? SCOPE.tenantId, + principalId: SCOPE.principalId, + }; + return await db.transaction((tx) => + createArtifact(tx, { + scope, + ownerPrincipalId: + overrides.ownerPrincipalId === undefined + ? scope.principalId + : overrides.ownerPrincipalId, + kind: overrides.kind ?? "document", + title: overrides.title ?? "Untitled", + content: overrides.content ?? "body", + source: overrides.source ?? { origin: "manual" }, + }), + ); +} diff --git a/src/test-helpers.ts b/e2e/helpers.ts similarity index 52% rename from src/test-helpers.ts rename to e2e/helpers.ts index 849cdfe..81b69d7 100644 --- a/src/test-helpers.ts +++ b/e2e/helpers.ts @@ -1,9 +1,28 @@ +// Real-Postgres harness for the e2e suites: the shared artifact database, a +// fresh database per suite with Interchange's control plane and this package's +// migrations applied, a mounted artifact app, and an on-disk ContentStore. +import { randomUUID } from "node:crypto"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { + createDB, + createGrantStore, + runMigrations, + type DBConfig, +} from "@intx/db"; +import { createRequireGrant, type TenantEnv } from "@intx/hub-api"; import { sql } from "drizzle-orm"; -import type { DBConfig } from "@intx/db"; +import { Hono } from "hono"; +import postgres from "postgres"; +import { uploadRefFromSource } from "../src/content-store.js"; 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 { + createArtifactRoutes, + InlineContentStore, + runArtifactMigrations, + type ContentStore, +} from "../src/index.js"; +import type { Actor } from "./fixtures.js"; export const DATABASE_URL = process.env.ARTIFACT_DATABASE_URL ?? @@ -25,7 +44,7 @@ export function databaseConfig(connectionString: string): DBConfig { * Explicit opt-in required before the harness runs TRUNCATE or DROP SCHEMA. * Must be the string `"1"` — any other value (including `"true"`) is refused. */ -export const ALLOW_DESTRUCTIVE_ARTIFACT_TESTS = "ALLOW_DESTRUCTIVE_ARTIFACT_TESTS"; +const ALLOW_DESTRUCTIVE_ARTIFACT_TESTS = "ALLOW_DESTRUCTIVE_ARTIFACT_TESTS"; type EnvMap = { readonly [key: string]: string | undefined }; @@ -33,7 +52,7 @@ type EnvMap = { readonly [key: string]: string | undefined }; * Database name segment of a Postgres connection string. * Pure URL parsing so the refuse path is unit-testable without a live server. */ -export function databaseNameFromConnectionString(connectionString: string): string { +function databaseNameFromConnectionString(connectionString: string): string { let parsed: URL; try { parsed = new URL(connectionString); @@ -42,7 +61,9 @@ export function databaseNameFromConnectionString(connectionString: string): stri `Invalid ARTIFACT_DATABASE_URL (not a URL): ${JSON.stringify(connectionString)}`, ); } - const name = decodeURIComponent(parsed.pathname.replace(/^\//, "").split("/")[0] ?? ""); + const name = decodeURIComponent( + parsed.pathname.replace(/^\//, "").split("/")[0] ?? "", + ); if (!name) { throw new Error( `ARTIFACT_DATABASE_URL has no database name (path is empty): ${JSON.stringify(connectionString)}`, @@ -58,7 +79,7 @@ export function databaseNameFromConnectionString(connectionString: string): stri * * Everything else — production-looking names included — is refused. */ -export function isAllowlistedArtifactTestDatabase(name: string): boolean { +function isAllowlistedArtifactTestDatabase(name: string): boolean { if (name === "artifact_core") return true; if (name.endsWith("_test")) return true; return false; @@ -69,7 +90,7 @@ export function isAllowlistedArtifactTestDatabase(name: string): boolean { * and an allowlisted database name. Pure (URL + env only) so CI without PG * can still prove the refuse path. */ -export function assertDestructiveArtifactTestsAllowed( +function assertDestructiveArtifactTestsAllowed( connectionString: string, env: EnvMap = process.env, ): void { @@ -117,7 +138,7 @@ let shared: ArtifactDb | undefined; * stand-ins (a real host brings the full Interchange tables) and seeds every * tenant and principal id the tests mint rows for. */ -export async function ensureControlPlane(db: ArtifactDb): Promise { +async function ensureControlPlane(db: ArtifactDb): Promise { await db.execute( sql`CREATE TABLE IF NOT EXISTS "public"."tenant" ("id" text PRIMARY KEY)`, ); @@ -158,44 +179,140 @@ export async function ensureControlPlane(db: ArtifactDb): Promise { export async function testDb(): Promise { // Gate before any pool open or TRUNCATE — a mispointed URL must never wipe. assertDestructiveArtifactTestsAllowed(DATABASE_URL); - let db = shared; - if (!db) { - db = createArtifactDb(DATABASE_URL).db; - shared = db; - await ensureControlPlane(db); - await runArtifactMigrations(databaseConfig(DATABASE_URL), { schema: "public" }); - } + const fresh = !shared; + const db = shared ?? createArtifactDb(DATABASE_URL).db; + shared = db; + // Re-seeded per call: the reference-host suite truncates the control plane on boot. + await ensureControlPlane(db); + if (fresh) + await runArtifactMigrations(databaseConfig(DATABASE_URL), { + schema: "public", + }); await db.execute( sql`TRUNCATE TABLE "artifacts"."artifact", "artifacts"."artifact_version", "artifacts"."upload" CASCADE`, ); return db; } -export const SCOPE = { tenantId: "acme", principalId: "user-1" }; - -export async function seedArtifact( - db: ArtifactDb, - overrides: Partial<{ - kind: string; - title: string; - content: string; - source: Record; - ownerPrincipalId: string | null; - tenantId: string; - }> = {}, -): Promise { - const scope = { ...SCOPE, ...(overrides.tenantId ? { tenantId: overrides.tenantId } : {}) }; - return await db.transaction((tx) => - createArtifact(tx, { - scope, - ownerPrincipalId: - overrides.ownerPrincipalId === undefined - ? scope.principalId - : overrides.ownerPrincipalId, - kind: overrides.kind ?? "document", - title: overrides.title ?? "Untitled", - content: overrides.content ?? "body", - source: overrides.source ?? { origin: "manual" }, +export type HostDb = ReturnType["db"]; + +export type TestDb = { + db: HostDb; + config: DBConfig; + close: () => Promise; +}; + +export function connectionString(config: DBConfig): string { + const user = encodeURIComponent(config.user); + const password = encodeURIComponent(config.password ?? ""); + return `postgres://${user}:${password}@${config.host}:${config.port}/${config.database}`; +} + +async function admin(run: (sql: postgres.Sql) => Promise): Promise { + const sql = postgres(DATABASE_URL, { max: 1, onnotice: () => undefined }); + try { + return await run(sql); + } finally { + await sql.end(); + } +} + +/** + * Creates `artifact__test`, applies Interchange's migrations and then + * `migrateArtifacts` (this package's by default), and drops it on `close`. + */ +export async function createTestDb( + migrateArtifacts: (config: DBConfig) => Promise = (config) => + runArtifactMigrations(config, { schema: "public" }), +): Promise { + assertDestructiveArtifactTestsAllowed(DATABASE_URL); + const name = `artifact_${randomUUID().replaceAll("-", "").slice(0, 12)}_test`; + await admin((sql) => sql.unsafe(`CREATE DATABASE "${name}"`)); + const server = databaseConfig(DATABASE_URL); + const config: DBConfig = { + host: server.host, + port: server.port, + user: server.user, + password: server.password, + database: name, + }; + await runMigrations(config, { schema: "public" }); + await migrateArtifacts(config); + const handle = createDB(config); + return { + db: handle.db, + config, + close: async () => { + await handle.close(); + await admin((sql) => + sql.unsafe(`DROP DATABASE IF EXISTS "${name}" WITH (FORCE)`), + ); + }, + }; +} + +/** + * `createArtifactRoutes` mounted at `/api` for `actor`, authorized by the + * platform's real `createRequireGrant` over the database's `grant` table. + */ +export function artifactApp( + db: HostDb, + actor: Actor, + contentStore: ContentStore = InlineContentStore, +): Hono { + const app = new Hono(); + app.use("*", async (c, next) => { + c.set("tenant", actor.tenant); + c.set("principal", actor.principal); + await next(); + }); + return app.route( + "/api", + createArtifactRoutes({ + db, + contentStore, + requireGrant: createRequireGrant({ + grantStore: createGrantStore(db), + conditionRegistry: {}, + }), }), ); } + +// A ContentStore that keeps file bytes on disk, one file per upload, referenced +// from the artifact's `source.upload.id` the way InlineContentStore references +// its bytea row. +export function createFsContentStore(dir: string): ContentStore { + const pathFor = (tenantId: string, id: string) => join(dir, tenantId, id); + return { + async put(_tx, scope, blob) { + const id = randomUUID(); + await mkdir(join(dir, scope.tenantId), { recursive: true }); + await writeFile(pathFor(scope.tenantId, id), blob.bytes); + return { + content: "", + source: { + upload: { + id, + filename: blob.filename, + mimeType: blob.mimeType, + size: blob.bytes.byteLength, + }, + }, + }; + }, + async get(_db, artifact) { + const ref = uploadRefFromSource(artifact.source); + if (ref?.id === undefined || artifact.tenantId === null) return null; + const bytes = await readFile(pathFor(artifact.tenantId, ref.id)).catch( + () => null, + ); + if (bytes === null) return null; + return { + filename: ref.filename, + mimeType: ref.mimeType, + bytes: new Uint8Array(bytes), + }; + }, + }; +} diff --git a/src/list.test.ts b/e2e/list.test.ts similarity index 98% rename from src/list.test.ts rename to e2e/list.test.ts index 2476927..6719d7b 100644 --- a/src/list.test.ts +++ b/e2e/list.test.ts @@ -8,9 +8,10 @@ import { MAX_LIST_LIMIT, serializeArtifactListItem, setArtifactArchived, -} from "./artifacts.js"; -import { seedArtifact, testDb } from "./test-helpers.js"; -import type { ArtifactDb } from "./db.js"; +} from "../src/artifacts.js"; +import { seedArtifact } from "./fixtures.js"; +import { testDb } from "./helpers.js"; +import type { ArtifactDb } from "../src/db.js"; /** Parse a raw query string the way the route does, failing the test on error. */ function parseQuery(query: Record) { diff --git a/tests/migrations.test.ts b/e2e/migrations.test.ts similarity index 96% rename from tests/migrations.test.ts rename to e2e/migrations.test.ts index 123bf9e..1606d58 100644 --- a/tests/migrations.test.ts +++ b/e2e/migrations.test.ts @@ -1,7 +1,8 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test"; import { sql } from "drizzle-orm"; import { createArtifactDb, runArtifactMigrations } from "../src/index.js"; -import { connectionString, createTestDb, seedActor, type TestDb } from "./lib/db-harness.js"; +import { seedActor } from "./fixtures.js"; +import { connectionString, createTestDb, type TestDb } from "./helpers.js"; const TABLES = ["artifact", "artifact_version", "upload"]; diff --git a/src/mount.test.ts b/e2e/mount.test.ts similarity index 94% rename from src/mount.test.ts rename to e2e/mount.test.ts index bf7aeb0..da1e64b 100644 --- a/src/mount.test.ts +++ b/e2e/mount.test.ts @@ -4,23 +4,25 @@ 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 { createArtifactRoutes } from "./mount.js"; -import { InlineContentStore } from "./content-store.js"; +import { createArtifactRoutes } from "../src/mount.js"; +import { InlineContentStore } from "../src/content-store.js"; import { + getArtifact, listArtifacts, MAX_ARTIFACT_CONTENT_BYTES, setArtifactArchived, sha256Hex, -} from "./artifacts.js"; +} from "../src/artifacts.js"; import { MAX_UPLOAD_BYTES, MAX_UPLOAD_FILE_COUNT, MAX_UPLOAD_TOTAL_BYTES, -} from "./uploads.js"; -import type { ArtifactDb } from "./db.js"; -import type { CreateArtifactRoutesDeps } from "./mount.js"; -import type { ResolvedPrincipal } from "./ports.js"; -import { seedArtifact, SCOPE, testDb } from "./test-helpers.js"; +} from "../src/uploads.js"; +import type { ArtifactDb } from "../src/db.js"; +import type { CreateArtifactRoutesDeps } from "../src/mount.js"; +import type { ResolvedPrincipal } from "../src/ports.js"; +import { seedArtifact, SCOPE } from "./fixtures.js"; +import { testDb } from "./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. */ @@ -1245,27 +1247,82 @@ describe("download over HTTP", () => { expect(res.status).toBe(400); }); - // A blob's bytes live in the ContentStore, referenced from the artifact - // row's own `source` — never per-version. Silently serving the current - // blob for an older `?version=N` would misrepresent it as that version's - // content, so a non-current version is refused instead of lying. - test("?version naming a non-current version of a blob-backed upload is 400", async () => { + test("?version on a revised upload serves each version's own bytes", async () => { const db = await testDb(); const app = host(db); - const png = new File([new Uint8Array([1, 2, 3])], "chart.png", { type: "image/png" }); + const first = new Uint8Array([1, 2, 3]); + const second = new Uint8Array([4, 5, 6, 7]); const data = new FormData(); - data.append("files", png); + data.append("files", new File([first], "chart.png", { type: "image/png" })); const created = (await ( await app.request("/artifacts/upload", { method: "POST", body: data }) ).json()) as { artifacts: { id: string }[] }; const id = created.artifacts[0]!.id; - await app.request(`/artifacts/${id}/versions`, json({ title: "Renamed" })); + const revise = new FormData(); + revise.append("file", new File([second], "chart-v2.png", { type: "image/png" })); + const revised = await app.request(`/artifacts/${id}/versions`, { + method: "POST", + body: revise, + }); + expect(revised.status).toBe(200); + expect(await revised.json()).toMatchObject({ version: 2, title: "chart.png" }); - const stale = await app.request(`/artifacts/${id}/download?version=1`); - expect(stale.status).toBe(400); - expect(await stale.json()).toEqual({ - error: "Uploaded file content is not versioned", + const bytesOf = async (query: string) => + new Uint8Array(await (await app.request(`/artifacts/${id}/download${query}`)).arrayBuffer()); + expect(await bytesOf("?version=1")).toEqual(first); + expect(await bytesOf("?version=2")).toEqual(second); + expect(await bytesOf("")).toEqual(second); + }); + + test("revising with a file matches the multipart content type case-insensitively", async () => { + const db = await testDb(); + const app = host(db); + const data = new FormData(); + data.append("files", new File([new Uint8Array([1])], "a.png", { type: "image/png" })); + const created = (await ( + await app.request("/artifacts/upload", { method: "POST", body: data }) + ).json()) as { artifacts: { id: string }[] }; + const id = created.artifacts[0]!.id; + const revise = new FormData(); + revise.append("file", new File([new Uint8Array([2])], "b.png", { type: "image/png" })); + const body = new Request("http://x", { method: "POST", body: revise }); + const contentType = body.headers.get("content-type")!.replace("multipart/form-data", "Multipart/Form-Data"); + const revised = await app.request(`/artifacts/${id}/versions`, { + method: "POST", + headers: { "content-type": contentType }, + body: await body.arrayBuffer(), }); + expect(revised.status).toBe(200); + expect(await revised.json()).toMatchObject({ version: 2 }); + }); + + test("revising with a file is refused for a non-upload, a kind change, or a stale expectedVersion", async () => { + const db = await testDb(); + const app = host(db); + const reviseWith = (id: string, file: File, expectedVersion?: string) => { + const form = new FormData(); + form.append("file", file); + if (expectedVersion !== undefined) form.append("expectedVersion", expectedVersion); + return app.request(`/artifacts/${id}/versions`, { method: "POST", body: form }); + }; + const png = new File([new Uint8Array([1])], "a.png", { type: "image/png" }); + + const doc = await seedArtifact(db); + expect((await reviseWith(doc.id, png)).status).toBe(400); + + const data = new FormData(); + data.append("files", png); + const created = (await ( + await app.request("/artifacts/upload", { method: "POST", body: data }) + ).json()) as { artifacts: { id: string }[] }; + const id = created.artifacts[0]!.id; + const pdf = new File([new Uint8Array([2])], "a.pdf", { type: "application/pdf" }); + expect((await reviseWith(id, pdf)).status).toBe(400); + expect((await reviseWith(id, png, "7")).status).toBe(409); + expect((await reviseWith(id, png, "zero")).status).toBe(400); + const svg = new File([""], "a.svg", { type: "image/svg+xml" }); + expect((await reviseWith(id, svg)).status).toBe(415); + expect((await getArtifact(db, id))!.version).toBe(1); }); test("?version naming the CURRENT version of a blob-backed upload still serves it", async () => { diff --git a/tests/reference-host.test.ts b/e2e/reference-host.test.ts similarity index 94% rename from tests/reference-host.test.ts rename to e2e/reference-host.test.ts index d1eb1fb..5315da1 100644 --- a/tests/reference-host.test.ts +++ b/e2e/reference-host.test.ts @@ -174,11 +174,6 @@ describe.each<[string, ContentStore]>([ }); }); -// Only DataUrlContentStore keeps its bytes IN `content`, so it is the store -// where ?version=N really changes what downloads. InlineContentStore's blob -// lives out-of-band, referenced from the artifact row's own `source` (never -// versioned) — see the next describe block for how that case is refused -// rather than silently serving the current blob under an older version's name. describe("download an older version's content over HTTP (DataUrlContentStore)", () => { let app: { request: (path: string, init?: RequestInit) => Promise }; let id: string; @@ -213,35 +208,36 @@ describe("download an older version's content over HTTP (DataUrlContentStore)", }); }); -describe("?version on a blob-backed upload (InlineContentStore) is refused unless current", () => { +describe("?version on a blob-backed upload (InlineContentStore) serves that version's bytes", () => { let app: { request: (path: string, init?: RequestInit) => Promise }; let id: string; - let currentVersion: number; - const BYTES = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 3, 3, 3]); + const ORIGINAL = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 3, 3, 3]); + const REVISED = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 4, 4, 4, 4]); beforeAll(async () => { app = host.buildApp(InlineContentStore); const form = new FormData(); - form.append("files", new File([BYTES], "chart.png", { type: "image/png" })); - const uploaded = await json<{ artifacts: { id: string; version: number }[] }>( + form.append("files", new File([ORIGINAL], "chart.png", { type: "image/png" })); + const uploaded = await json<{ artifacts: { id: string }[] }>( await app.request("/api/artifacts/upload", { method: "POST", body: form }), ); - ({ id, version: currentVersion } = uploaded.artifacts[0]!); - // Bumps the artifact's version without touching the out-of-band blob — - // `source` (and so the ContentStore reference) is never revised. - await app.request(`/api/artifacts/${id}/versions`, postJson({ title: "Renamed" })); + id = uploaded.artifacts[0]!.id; + const revise = new FormData(); + revise.append("file", new File([REVISED], "chart.png", { type: "image/png" })); + await app.request(`/api/artifacts/${id}/versions`, { method: "POST", body: revise }); }); - test("a non-current ?version is 400, not a silent lie about which bytes came back", async () => { - const res = await app.request(`/api/artifacts/${id}/download?version=${currentVersion}`); - expect(res.status).toBe(400); - expect(await res.json()).toEqual({ error: "Uploaded file content is not versioned" }); + test("?version=1 downloads the original bytes", async () => { + const res = await app.request(`/api/artifacts/${id}/download?version=1`); + expect(res.status).toBe(200); + expect(new Uint8Array(await res.arrayBuffer())).toEqual(ORIGINAL); }); - test("?version naming the CURRENT version still downloads the blob", async () => { - const res = await app.request(`/api/artifacts/${id}/download?version=${currentVersion + 1}`); - expect(res.status).toBe(200); - expect(new Uint8Array(await res.arrayBuffer())).toEqual(BYTES); + test("?version=2 and no version download the revised bytes", async () => { + for (const query of ["?version=2", ""]) { + const res = await app.request(`/api/artifacts/${id}/download${query}`); + expect(new Uint8Array(await res.arrayBuffer())).toEqual(REVISED); + } }); }); diff --git a/src/tools.test.ts b/e2e/tools.test.ts similarity index 98% rename from src/tools.test.ts rename to e2e/tools.test.ts index 97edf49..facb137 100644 --- a/src/tools.test.ts +++ b/e2e/tools.test.ts @@ -4,7 +4,7 @@ import { ArtifactNotFoundError, listArtifactVersions, writeArtifactVersion, -} from "./artifacts.js"; +} from "../src/artifacts.js"; import { ARTIFACT_TOOL_DEFINITIONS, DEFAULT_READ_LIMIT, @@ -12,8 +12,9 @@ import { readArtifact, readArtifactChunk, SAFE_ENCODED_BUDGET, -} from "./tools.js"; -import { seedArtifact, SCOPE, testDb } from "./test-helpers.js"; +} from "../src/tools.js"; +import { seedArtifact, SCOPE } from "./fixtures.js"; +import { testDb } from "./helpers.js"; async function read(content: string, offset?: number, limit?: number) { const db = await testDb(); diff --git a/e2e/upgrade-from-0.1.0.test.ts b/e2e/upgrade-from-0.1.0.test.ts new file mode 100644 index 0000000..88f0c98 --- /dev/null +++ b/e2e/upgrade-from-0.1.0.test.ts @@ -0,0 +1,148 @@ +// A database migrated and written by the published 0.1.0 package upgrades in +// place under this version's runArtifactMigrations. +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { schema as intx } from "@intx/db"; +import { generateId } from "@intx/hub-common"; +import { sql } from "drizzle-orm"; +import * as v010 from "@corbits/artifacts-0.1.0"; +import { runArtifactMigrations } from "../src/index.js"; +import { grant, seedActor, type Actor } from "./fixtures.js"; +import { artifactApp, connectionString, createTestDb, type TestDb } from "./helpers.js"; + +const PDF = new Uint8Array([0x25, 0x50, 0x44, 0x46, 0x2d, 0x31, 0x2e, 0x37, 0x00, 0xff]); + +let testDb: TestDb; +let actor: Actor; +let fileId: string; +let newcomer: Actor; + +beforeAll(async () => { + testDb = await createTestDb(async (config) => { + const legacy = v010.createArtifactDb(connectionString(config)); + try { + await v010.runArtifactMigrations(legacy.db); + } finally { + await legacy.close(); + } + }); + actor = await seedActor(testDb.db, "acme"); + // 0.1.0 needed no create grant; the upgrade has to supply it. + await testDb.db.execute(sql`DELETE FROM "grant" WHERE action = 'create'`); + await grant(testDb.db, actor, "artifact:*", "write"); + const scope = { tenantId: actor.tenant.id, principalId: actor.principal.id }; + + const legacy = v010.createArtifactDb(connectionString(testDb.config)); + try { + const file = await legacy.db.transaction((tx) => + v010.createFileArtifact(tx, v010.InlineContentStore, { + scope, + ownerPrincipalId: scope.principalId, + filename: "deck.pdf", + mimeType: "application/pdf", + bytes: PDF, + policy: v010.ARTIFACT_UPLOAD_POLICY, + }), + ); + fileId = file.id; + await v010.writeArtifactVersion(legacy.db, { scope, artifactId: file.id, title: "deck v2.pdf" }); + await v010.saveMailAttachmentRefs(legacy.db, { + scope, + instanceId: "inst-1", + body: { + mailId: "mail-1", + attachments: [ + { artifactId: file.id, name: "deck.pdf", type: "application/pdf", size: PDF.length }, + ], + }, + }); + } finally { + await legacy.close(); + } + + await runArtifactMigrations(testDb.config, { schema: "public" }); + const [principal] = await testDb.db + .insert(intx.principal) + .values({ + id: generateId("principal"), + tenantId: actor.tenant.id, + kind: "user", + refId: "user-newcomer", + status: "active", + }) + .returning(); + newcomer = { tenant: actor.tenant, principal: principal! }; +}); + +afterAll(async () => { + await testDb?.close(); +}); + +const createDoc = (as: Actor) => + artifactApp(testDb.db, as).request("/api/artifacts", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ mode: "text", title: "After upgrade", content: "hi" }), + }); + +describe("upgrading a 0.1.0 database", () => { + test("a 0.1.0 creator keeps creating; a new principal needs the grant", async () => { + expect((await createDoc(actor)).status).toBe(201); + expect((await createDoc(newcomer)).status).toBe(403); + }); + + test("rerunning the migrations grants nothing new", async () => { + const count = async () => + ( + await testDb.db.execute<{ n: number }>(sql` + SELECT count(*)::int AS n FROM "grant" + WHERE resource = 'artifact:*' AND action = 'create' + `) + )[0]!.n; + const before = await count(); + await runArtifactMigrations(testDb.config, { schema: "public" }); + expect(await count()).toBe(before); + }); + + test("drops mail_attachment_ref and the 0.1.0 migration ledger", async () => { + const rows = await testDb.db.execute<{ table_name: string }>(sql` + SELECT table_name FROM information_schema.tables + WHERE table_schema = 'artifacts' + ORDER BY table_name + `); + expect(rows.map((row) => row.table_name)).toEqual([ + "artifact", + "artifact_version", + "upload", + ]); + }); + + test("every version of a 0.1.0 upload downloads its bytes", async () => { + const app = artifactApp(testDb.db, actor); + for (const query of ["?version=1", "?version=2", ""]) { + const res = await app.request(`/api/artifacts/${fileId}/download${query}`); + expect(res.status).toBe(200); + expect(new Uint8Array(await res.arrayBuffer())).toEqual(PDF); + } + }); + + test("revising a 0.1.0 upload with new bytes keeps version 1's bytes", async () => { + const app = artifactApp(testDb.db, actor); + const revised = new Uint8Array([0x25, 0x50, 0x44, 0x46, 0x2d, 0x32]); + const form = new FormData(); + form.append("file", new File([revised], "deck.pdf", { type: "application/pdf" })); + const res = await app.request(`/api/artifacts/${fileId}/versions`, { + method: "POST", + body: form, + }); + expect(res.status).toBe(200); + expect(await res.json()).toMatchObject({ version: 3 }); + + const bytesOf = async (query: string) => + new Uint8Array( + await (await app.request(`/api/artifacts/${fileId}/download${query}`)).arrayBuffer(), + ); + expect(await bytesOf("?version=1")).toEqual(PDF); + expect(await bytesOf("?version=3")).toEqual(revised); + expect(await bytesOf("")).toEqual(revised); + }); +}); diff --git a/tests/upload-policy.test.ts b/e2e/upload-policy.test.ts similarity index 92% rename from tests/upload-policy.test.ts rename to e2e/upload-policy.test.ts index 9f7c34c..9117a65 100644 --- a/tests/upload-policy.test.ts +++ b/e2e/upload-policy.test.ts @@ -2,7 +2,8 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test"; import type { Hono } from "hono"; import type { TenantEnv } from "@intx/hub-api"; import { MAX_UPLOAD_BYTES } from "../src/index.js"; -import { artifactApp, createTestDb, seedActor, type TestDb } from "./lib/db-harness.js"; +import { seedActor } from "./fixtures.js"; +import { artifactApp, createTestDb, type TestDb } from "./helpers.js"; let testDb: TestDb; let app: Hono; diff --git a/src/uploads.test.ts b/e2e/uploads.test.ts similarity index 97% rename from src/uploads.test.ts rename to e2e/uploads.test.ts index 3364659..3a7149f 100644 --- a/src/uploads.test.ts +++ b/e2e/uploads.test.ts @@ -13,11 +13,12 @@ import { UnsupportedUploadTypeError, uploadArtifactKind, type UploadPolicy, -} from "./uploads.js"; -import { InlineContentStore } from "./content-store.js"; -import { disposition } from "./download.js"; -import { artifact, upload } from "./schema.js"; -import { SCOPE, testDb } from "./test-helpers.js"; +} from "../src/uploads.js"; +import { InlineContentStore } from "../src/content-store.js"; +import { disposition } from "../src/download.js"; +import { artifact, upload } from "../src/schema.js"; +import { SCOPE } from "./fixtures.js"; +import { testDb } from "./helpers.js"; const XLSX = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"; diff --git a/src/workflow-mount.test.ts b/e2e/workflow-mount.test.ts similarity index 95% rename from src/workflow-mount.test.ts rename to e2e/workflow-mount.test.ts index f9a596c..6f6c4c5 100644 --- a/src/workflow-mount.test.ts +++ b/e2e/workflow-mount.test.ts @@ -1,14 +1,13 @@ import { describe, expect, test } from "bun:test"; -import { Hono } from "hono"; import { - mountWorkflowArtifacts, + createWorkflowArtifactRoutes, type ResolvedWorkflowRunScope, - type WorkflowArtifactEnv, -} from "./workflow-mount.js"; -import { InlineContentStore } from "./content-store.js"; -import { getArtifact } from "./artifacts.js"; -import { seedArtifact, testDb } from "./test-helpers.js"; -import type { ArtifactDb } from "./db.js"; +} from "../src/workflow-mount.js"; +import { InlineContentStore } from "../src/content-store.js"; +import { getArtifact } from "../src/artifacts.js"; +import { seedArtifact } from "./fixtures.js"; +import { testDb } from "./helpers.js"; +import type { ArtifactDb } from "../src/db.js"; const RUN_SCOPE: ResolvedWorkflowRunScope = { tenantId: "acme", @@ -20,8 +19,7 @@ const VALID_TOKEN = "sidecar-token"; const VALID_ADDRESS = "run-1@acme"; function host(db: ArtifactDb, opts: { resolves?: ResolvedWorkflowRunScope | null } = {}) { - const app = new Hono(); - return mountWorkflowArtifacts(app, { + return createWorkflowArtifactRoutes({ db, contentStore: InlineContentStore, resolveRunScope: (token, address) => { @@ -151,7 +149,7 @@ describe("POST /artifacts", () => { test("413s content over the configured character ceiling", async () => { const db = await testDb(); - const app = mountWorkflowArtifacts(new Hono(), { + const app = createWorkflowArtifactRoutes({ db, contentStore: InlineContentStore, resolveRunScope: () => RUN_SCOPE, @@ -266,7 +264,7 @@ describe("POST /artifacts/binary", () => { test("413s bytes over the configured byte ceiling", async () => { const db = await testDb(); - const app = mountWorkflowArtifacts(new Hono(), { + const app = createWorkflowArtifactRoutes({ db, contentStore: InlineContentStore, resolveRunScope: () => RUN_SCOPE, diff --git a/examples/reference-host/src/index.ts b/examples/reference-host/src/index.ts index ba8fe77..9095910 100644 --- a/examples/reference-host/src/index.ts +++ b/examples/reference-host/src/index.ts @@ -11,7 +11,7 @@ // rows for it. // // This module only BUILDS the host. The acceptance scenarios live in -// `tests/reference-host.test.ts` and run under `bun run test`, so they are collected by +// `e2e/reference-host.test.ts` and run under `bun run test:e2e`, so they are collected by // CI like any other test instead of being a hand-rolled assert script nothing // executes. import { sql } from "drizzle-orm"; @@ -49,7 +49,7 @@ import { type ContentStore, type SerializedArtifactBase, } from "@corbits/artifacts"; -import { databaseConfig, DATABASE_URL } from "../../../src/test-helpers.js"; +import { databaseConfig, DATABASE_URL } from "../../../e2e/helpers.js"; const EPOCH = new Date(0); diff --git a/migrations/0002_drop_migration_ledger.sql b/migrations/0002_drop_migration_ledger.sql index d2efaed..37b0cf3 100644 --- a/migrations/0002_drop_migration_ledger.sql +++ b/migrations/0002_drop_migration_ledger.sql @@ -1 +1,25 @@ +-- Upgrading from 0.1.0, where creating needed no grant: before the ledger goes, +-- give `create` on `artifact:*` to every principal that has created an artifact +-- in its tenant. Guarded on the ledger, so it runs once and never re-grants a +-- revoked create. +DO $$ +BEGIN + IF to_regclass('"artifacts"."migrations"') IS NOT NULL THEN + INSERT INTO "public"."grant" + ("id", "tenant_id", "principal_id", "resource", "action", "effect", "origin") + SELECT DISTINCT ON ("a"."tenant_id", "a"."principal_id") + 'grt_' || replace(gen_random_uuid()::text, '-', ''), + "a"."tenant_id", "a"."principal_id", 'artifact:*', 'create', 'allow', 'system' + FROM "artifacts"."artifact" AS "a" + JOIN "public"."principal" AS "p" ON "p"."id" = "a"."principal_id" + WHERE NOT EXISTS ( + SELECT 1 FROM "public"."grant" AS "g" + WHERE "g"."tenant_id" = "a"."tenant_id" + AND "g"."principal_id" = "a"."principal_id" + AND "g"."resource" = 'artifact:*' + AND "g"."action" = 'create' + ); + END IF; +END $$; +--> statement-breakpoint DROP TABLE IF EXISTS "artifacts"."migrations"; diff --git a/migrations/0004_version_source.sql b/migrations/0004_version_source.sql new file mode 100644 index 0000000..7d9b808 --- /dev/null +++ b/migrations/0004_version_source.sql @@ -0,0 +1,6 @@ +ALTER TABLE "artifacts"."artifact_version" ADD COLUMN IF NOT EXISTS "source" jsonb; +--> statement-breakpoint +UPDATE "artifacts"."artifact_version" AS "v" + SET "source" = "a"."source" + FROM "artifacts"."artifact" AS "a" + WHERE "v"."artifact_id" = "a"."id" AND "v"."source" IS NULL AND "a"."source" IS NOT NULL; diff --git a/package.json b/package.json index 3fd89f2..a75a13a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@corbits/artifacts", - "version": "0.1.0", + "version": "0.2.0", "type": "module", "license": "LGPL-2.1-only", "description": "Artifacts, versions and uploads with a pluggable ContentStore, mountable onto any Interchange host. Requires @intx 0.4.0 or newer.", @@ -56,8 +56,9 @@ "typecheck": "tsc --noEmit", "build": "rm -rf dist && tsc -p tsconfig.build.json", "prepack": "bun run build", - "test": "bun test src tests", - "test:coverage": "bun test --coverage src tests" + "test": "bun test src", + "test:e2e": "bun test e2e", + "test:coverage": "bun test --coverage src e2e" }, "dependencies": { "@hono/standard-validator": "^0.2.3", diff --git a/src/artifacts.ts b/src/artifacts.ts index d9c86f8..57ef88d 100644 --- a/src/artifacts.ts +++ b/src/artifacts.ts @@ -281,6 +281,7 @@ export async function createArtifact( version: 1, title: args.title, content: args.content, + source: args.source, authorId: args.scope.principalId, metadata, parentVersionIds, @@ -326,7 +327,7 @@ export class VersionConflictError extends Error { * Archived artifacts present as NOT FOUND — an agent holding a * stale id must not silently revise something the user put away. */ -async function reviseArtifactVersion( +export async function reviseArtifactVersion( tx: ArtifactTx, args: { scope: ResolvedPrincipal; @@ -337,6 +338,16 @@ async function reviseArtifactVersion( metadata?: Record | null; /** Explicit lineage for this version — never inferred, never carried forward. */ parentVersionIds?: string[] | null; + /** + * Stores new file content once the row is locked and `expectedVersion` + * holds, so a refused revise never writes bytes. Omitted carries the + * prior `source` and digest forward. + */ + storeFile?: (locked: ArtifactRow) => Promise<{ + content: string; + source: Record; + contentSha256: string; + }>; /** * Precondition checked under the `FOR UPDATE` lock below: when set and it * does not match the current version, {@link VersionConflictError} is @@ -373,9 +384,10 @@ async function reviseArtifactVersion( throw new VersionConflictError(args.artifactId, existing.version); } + const file = args.storeFile ? await args.storeFile(existing) : undefined; const version = existing.version + 1; const title = args.title ?? existing.title; - const content = args.content ?? existing.content; + const content = file?.content ?? args.content ?? existing.content; if (args.content !== undefined) { assertArtifactFieldSizes({ content }); } @@ -384,14 +396,16 @@ async function reviseArtifactVersion( ? (existing.metadata as Record | null) : args.metadata; const parentVersionIds = args.parentVersionIds ?? null; + const source = file?.source ?? existing.source; // Content omitted: the previous content carries forward, so its digest // carries forward unchanged rather than being recomputed. const contentSha256 = - args.content === undefined ? existing.contentSha256 : sha256Hex(content); + file?.contentSha256 ?? + (args.content === undefined ? existing.contentSha256 : sha256Hex(content)); const [updated] = await tx .update(artifact) - .set({ title, content, version, metadata, contentSha256, updatedAt: now }) + .set({ title, content, source, version, metadata, contentSha256, updatedAt: now }) .where(eq(artifact.id, args.artifactId)) .returning(); if (!updated) throw new ArtifactNotFoundError(args.artifactId); @@ -401,6 +415,7 @@ async function reviseArtifactVersion( version, title, content, + source, authorId: args.scope.principalId, metadata, parentVersionIds, @@ -484,11 +499,13 @@ export async function getArtifactVersion( metadata: Record | null; parentVersionIds: string[] | null; contentSha256: string | null; + source: unknown; } | null> { const [row] = await db .select({ title: artifactVersion.title, content: artifactVersion.content, + source: artifactVersion.source, version: artifactVersion.version, metadata: artifactVersion.metadata, parentVersionIds: artifactVersion.parentVersionIds, diff --git a/src/index.ts b/src/index.ts index d42f93e..b6f4293 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2,12 +2,12 @@ export { createArtifactRoutes } from "./mount.js"; export type { CreateArtifactRoutesDeps } from "./mount.js"; -export { mountWorkflowArtifacts } from "./workflow-mount.js"; +export { createWorkflowArtifactRoutes } from "./workflow-mount.js"; export type { AgentTokenAuth, AgentTokenIdentity, CreatedWorkflowArtifact, - MountWorkflowArtifactsOpts, + CreateWorkflowArtifactRoutesDeps, ResolvedWorkflowRunScope, WorkflowArtifactEnv, WorkflowRunResolver, @@ -85,6 +85,7 @@ export { MAX_UPLOAD_FILE_COUNT, MAX_UPLOAD_TOTAL_BYTES, PARSED_DOCUMENT_POLICY, + reviseFileArtifact, SPREADSHEET_UPLOAD_POLICY, UnsupportedUploadTypeError, } from "./uploads.js"; diff --git a/src/mount.ts b/src/mount.ts index bcebf2f..1996581 100644 --- a/src/mount.ts +++ b/src/mount.ts @@ -46,7 +46,9 @@ import { MAX_UPLOAD_BYTES, MAX_UPLOAD_FILE_COUNT, MAX_UPLOAD_TOTAL_BYTES, + reviseFileArtifact, UnsupportedUploadTypeError, + uploadArtifactKind, type UploadPolicy, } from "./uploads.js"; @@ -192,6 +194,29 @@ const VersionRef = type("string").pipe((raw, ctx) => { return n; }); +// The artifact as it stood at one version, for the version detail and download routes. +function rowAtVersion( + row: ArtifactRow, + at: NonNullable>>, +): ArtifactRow { + return { + id: row.id, + tenantId: row.tenantId, + principalId: row.principalId, + ownerPrincipalId: row.ownerPrincipalId, + kind: row.kind, + title: at.title, + content: at.content, + source: at.source, + version: at.version, + metadata: at.metadata, + contentSha256: at.contentSha256, + archivedAt: row.archivedAt, + createdAt: row.createdAt, + updatedAt: row.updatedAt, + }; +} + /** * Build the artifact routes as a sub-app the host mounts with `app.route`. * @@ -720,14 +745,7 @@ export function createArtifactRoutes({ if (!versionRow) return c.json({ error: "Artifact not found" }, 404); const [artifactJson] = await serialize(loaded.scope, [ - { - ...loaded.row, - title: versionRow.title, - content: versionRow.content, - version: versionRow.version, - metadata: versionRow.metadata, - contentSha256: versionRow.contentSha256, - }, + rowAtVersion(loaded.row, versionRow), ]); return c.json({ artifact: artifactJson }); }, @@ -739,7 +757,7 @@ export function createArtifactRoutes({ tags: ["Artifacts"], summary: "Revise an artifact, creating a new version", description: - "Locks the row FOR UPDATE and bumps version by one; a unique (artifactId, version) index backstops a racing writer. Archived artifacts present as not found. `metadata` is optional and opaque; when omitted, the prior version's metadata carries forward, and an explicit `null` clears it. An optional `expectedVersion` is checked under the same lock: a mismatch answers 409 and writes nothing.", + "Locks the row FOR UPDATE and bumps version by one; a unique (artifactId, version) index backstops a racing writer. Archived artifacts present as not found. `metadata` is optional and opaque; when omitted, the prior version's metadata carries forward, and an explicit `null` clears it. An optional `expectedVersion` is checked under the same lock: a mismatch answers 409 and writes nothing. An uploaded file is revised with new bytes by sending multipart/form-data with one `file` field (and optionally `expectedVersion`); the new version keeps its own bytes, and every earlier version still downloads its own.", parameters: [idParam], responses: { 200: { description: "New version created" }, @@ -747,7 +765,8 @@ export function createArtifactRoutes({ 403: { description: "No resolvable principal, or not permitted" }, 404: { description: "Artifact not found" }, 409: { description: "expectedVersion did not match the current version" }, - 413: { description: "Declared Content-Length over the content ceiling" }, + 413: { description: "Declared Content-Length over the content ceiling, or a file over the upload limit" }, + 415: { description: "The file has an unsupported type" }, }, }), principalRequired, @@ -757,6 +776,9 @@ export function createArtifactRoutes({ // loadScoped resolves the principal before any body parse. const loaded = await loadScoped(c); if ("response" in loaded) return loaded.response; + if (c.req.header("content-type")?.toLowerCase().startsWith("multipart/form-data")) { + return await reviseWithFile(c, loaded.row, loaded.scope); + } if (contentLengthOverCeiling(c)) { return c.json( { @@ -801,6 +823,70 @@ export function createArtifactRoutes({ }, ); + async function reviseWithFile(c: Ctx, row: ArtifactRow, scope: ResolvedPrincipal) { + if (uploadRefFromSource(row.source) === null) { + return c.json({ error: "Only an uploaded file can be revised with a file" }, 400); + } + const parsed = await c.req.parseBody({ all: true }); + const file = parsed["file"]; + if (!(file instanceof File)) { + return c.json({ error: "Expected one file field named file" }, 400); + } + if (file.size > MAX_UPLOAD_BYTES) { + return c.json( + { error: `File "${file.name}" exceeds the ${MAX_UPLOAD_BYTES} byte limit` }, + 413, + ); + } + const rawExpected = parsed["expectedVersion"]; + const expectedVersion = + rawExpected === undefined ? undefined : ExpectedVersion(Number(rawExpected)); + if (expectedVersion instanceof type.errors) { + return c.json({ error: `expectedVersion ${expectedVersion.summary}` }, 400); + } + const mimeType = effectiveUploadMime(file, uploadPolicy); + if (mimeType.length > 0 && uploadArtifactKind(mimeType) !== row.kind) { + return c.json( + { error: `File "${file.name}" would change the artifact's kind from ${row.kind}` }, + 400, + ); + } + try { + const revised = await db.transaction(async (tx) => + reviseFileArtifact(tx, contentStore, { + scope, + artifact: row, + filename: file.name, + mimeType, + bytes: new Uint8Array(await file.arrayBuffer()), + policy: uploadPolicy, + ...(expectedVersion !== undefined ? { expectedVersion } : {}), + }), + ); + return c.json({ + artifactId: revised.id, + version: revised.version, + title: revised.title, + metadata: (revised.metadata as Record | null) ?? null, + contentSha256: revised.contentSha256, + }); + } catch (error) { + if (error instanceof UnsupportedUploadTypeError) { + return c.json({ error: error.message }, 415); + } + if (error instanceof ArtifactNotFoundError) { + return c.json({ error: "Artifact not found" }, 404); + } + if (error instanceof VersionConflictError) { + return c.json( + { error: "Version conflict", currentVersion: error.currentVersion }, + 409, + ); + } + throw error; + } + } + async function setArchived(c: Ctx, archive: boolean) { const loaded = await loadScoped(c); if ("response" in loaded) return loaded.response; @@ -856,7 +942,7 @@ export function createArtifactRoutes({ tags: ["Artifacts"], summary: "Download an artifact's content", description: - "One path over three storage conventions, in precedence order: out-of-band ContentStore blob, inline data: URL (file/image kinds), then downloadable text (csv-export). Served as an attachment except a PDF with ?inline=1; X-Content-Type-Options: nosniff always. An optional ?version=N pins the download to that version's content (getArtifactVersion) for the data-URL and downloadable-text conventions, where content really is per-version. A blob-backed upload has no per-version bytes — source.upload.id lives on the artifact row only — so ?version=N for one is 400 unless it names the artifact's current version.", + "One path over three storage conventions, in precedence order: out-of-band ContentStore blob, inline data: URL (file/image kinds), then downloadable text (csv-export). Served as an attachment except a PDF with ?inline=1; X-Content-Type-Options: nosniff always. An optional ?version=N serves that version's exact content, including an upload's bytes as of that version; without it, the latest.", parameters: [ idParam, { name: "inline", in: "query", required: false, schema: { type: "string" } }, @@ -866,7 +952,7 @@ export function createArtifactRoutes({ 200: { description: "The file body" }, 400: { description: - "Artifact kind is not downloadable, version is not a positive integer, or version names a non-current version of a blob-backed upload", + "Artifact kind is not downloadable, or version is not a positive integer", }, 403: { description: "No resolvable principal" }, 404: { description: "Artifact not found — also the answer for an unknown version" }, @@ -885,26 +971,7 @@ export function createArtifactRoutes({ } const versionRow = await getArtifactVersion(db, loaded.row.id, version); if (!versionRow) return c.json({ error: "Artifact not found" }, 404); - // A blob's bytes live in the ContentStore, referenced from the - // artifact row's own `source` — never from `artifact_version`, so - // there is no per-version blob to serve. Silently falling through to - // today's blob would answer version N's request with the current - // bytes, misrepresenting them as pinned. - if ( - version !== loaded.row.version && - uploadRefFromSource(loaded.row.source)?.id !== undefined - ) { - return c.json( - { error: "Uploaded file content is not versioned" }, - 400, - ); - } - row = { - ...loaded.row, - title: versionRow.title, - content: versionRow.content, - version: versionRow.version, - }; + row = rowAtVersion(loaded.row, versionRow); } const result = await resolveDownload( diff --git a/src/schema.ts b/src/schema.ts index 4ff44b7..f7e3623 100644 --- a/src/schema.ts +++ b/src/schema.ts @@ -125,6 +125,13 @@ export const artifactVersion = artifactsSchema.table( metadata: jsonb("metadata"), /** Explicit lineage set by the writer — never inferred from version order. */ parentVersionIds: text("parent_version_ids").array(), + /** + * This version's `source`, mirrored onto `artifact.source` for the current + * version. For a file artifact it carries the version's own content + * reference and size (`source.upload`), so `?version=N` downloads that + * version's bytes. + */ + source: jsonb("source"), /** * sha256 (hex) over the UTF-8 bytes of `content` for text and URL * artifacts, or over the uploaded bytes for a blob-backed file artifact's diff --git a/src/sidecar-bundle.ts b/src/sidecar-bundle.ts index 5936533..15fa15c 100644 --- a/src/sidecar-bundle.ts +++ b/src/sidecar-bundle.ts @@ -14,7 +14,7 @@ import type { RuntimeCapabilities } from "@intx/types/runtime-capabilities"; import { ARTIFACT_TOOL_DEFINITIONS } from "./tools.js"; -/** Where a host mounts `mountWorkflowArtifacts`. The bundle has no options of +/** Where a host mounts `createWorkflowArtifactRoutes`. The bundle has no options of * its own — the loader constructs it — so the path is a shared constant * rather than per-deploy configuration. */ export const WORKFLOW_ARTIFACTS_BASE_PATH = "/api/workflow-artifacts"; diff --git a/src/uploads.ts b/src/uploads.ts index 10b2ade..7315452 100644 --- a/src/uploads.ts +++ b/src/uploads.ts @@ -1,7 +1,7 @@ import { createHash } from "node:crypto"; import { isAllowedMimeType } from "@intx/types"; import type { ArtifactTx } from "./db.js"; -import { createArtifact } from "./artifacts.js"; +import { createArtifact, reviseArtifactVersion } from "./artifacts.js"; import type { ArtifactRow } from "./schema.js"; import type { ResolvedPrincipal, ContentStore } from "./ports.js"; @@ -194,6 +194,10 @@ export function contentDispositionHeader( return `${disposition}; filename="${ascii}"; filename*=UTF-8''${encoded}`; } +function bytesSha256(bytes: Uint8Array): string { + return createHash("sha256").update(bytes).digest("hex"); +} + /** * The ONE way a file becomes an artifact. Every entry point — the * multipart import route, a chat attachment divert, a workflow's generated @@ -237,7 +241,7 @@ export async function createFileArtifact( // Digest the uploaded bytes, not `stored.content` — for a blob-backed store // that column is a pointer (empty, or a data: URL), not the bytes // themselves, and this is version 1's authoritative digest either way. - const contentSha256 = createHash("sha256").update(args.bytes).digest("hex"); + const contentSha256 = bytesSha256(args.bytes); return await createArtifact(tx, { scope: args.scope, ownerPrincipalId: args.ownerPrincipalId, @@ -253,3 +257,49 @@ export async function createFileArtifact( ...(args.metadata !== undefined ? { metadata: args.metadata } : {}), }); } + +/** + * Revise a file artifact with new bytes. The bytes go to the ContentStore and + * the new version records its own content reference, size and digest, so every + * earlier version keeps serving its own bytes. The artifact's other `source` + * fields (origin, provenance) and its title carry forward; the new filename + * is what the version downloads as. + */ +export async function reviseFileArtifact( + tx: ArtifactTx, + contentStore: ContentStore, + args: { + scope: ResolvedPrincipal; + artifact: ArtifactRow; + filename: string; + mimeType: string; + bytes: Uint8Array; + policy: UploadPolicy; + expectedVersion?: number; + }, +): Promise { + if (!args.policy.accepts(args.mimeType)) { + throw new UnsupportedUploadTypeError(args.filename, args.mimeType); + } + return await reviseArtifactVersion( + tx, + { + scope: args.scope, + artifactId: args.artifact.id, + storeFile: async (locked) => { + const stored = await contentStore.put(tx, args.scope, { + filename: args.filename, + mimeType: args.mimeType, + bytes: args.bytes, + }); + return { + content: stored.content, + source: { ...(locked.source as Record), ...stored.source }, + contentSha256: bytesSha256(args.bytes), + }; + }, + ...(args.expectedVersion !== undefined ? { expectedVersion: args.expectedVersion } : {}), + }, + new Date(), + ); +} diff --git a/src/workflow-mount.ts b/src/workflow-mount.ts index 13f7a32..1e3fdd5 100644 --- a/src/workflow-mount.ts +++ b/src/workflow-mount.ts @@ -17,7 +17,7 @@ * however it likes) — this module only draws the auth + CRUD seam. */ import { type } from "arktype"; -import type { Context, Hono, MiddlewareHandler } from "hono"; +import { Hono, type Context, type MiddlewareHandler } from "hono"; import { ArtifactNotFoundError, createArtifact, @@ -99,7 +99,7 @@ export type AgentTokenAuth = { ) => Promise | ResolvedWorkflowRunScope | null; }; -export type MountWorkflowArtifactsOpts = { +export type CreateWorkflowArtifactRoutesDeps = { db: ArtifactDb; contentStore: ContentStore; resolveRunScope: WorkflowRunResolver; @@ -193,24 +193,22 @@ function parseRecentLimit(raw: string | undefined): number { } /** - * Mount the run-scoped artifact routes onto a host Hono app. Unlike + * Build the run-scoped artifact routes as a sub-app the host mounts with + * `app.route`, at `WORKFLOW_ARTIFACTS_BASE_PATH`. Unlike * `createArtifactRoutes`, every route here is behind its own bearer-token * middleware — there is no unauthenticated collection-read case, since a * workflow run always presents credentials. */ -export function mountWorkflowArtifacts( - app: Hono, - opts: MountWorkflowArtifactsOpts, -): Hono { - const { - db, - contentStore, - resolveRunScope, - agentToken, - uploadPolicy = ARTIFACT_UPLOAD_POLICY, - maxBinaryBytes = MAX_UPLOAD_BYTES, - maxContentChars = DEFAULT_MAX_WORKFLOW_CONTENT_CHARS, - } = opts; +export function createWorkflowArtifactRoutes({ + db, + contentStore, + resolveRunScope, + agentToken, + uploadPolicy = ARTIFACT_UPLOAD_POLICY, + maxBinaryBytes = MAX_UPLOAD_BYTES, + maxContentChars = DEFAULT_MAX_WORKFLOW_CONTENT_CHARS, +}: CreateWorkflowArtifactRoutesDeps): Hono { + const app = new Hono(); const authenticate: MiddlewareHandler = async (c, next) => { const authHeader = c.req.header("authorization") ?? ""; diff --git a/tests/file-round-trip.test.ts b/tests/file-round-trip.test.ts deleted file mode 100644 index 6d22cd2..0000000 --- a/tests/file-round-trip.test.ts +++ /dev/null @@ -1,61 +0,0 @@ -import { afterAll, beforeAll, describe, expect, test } from "bun:test"; -import { mkdtemp, rm } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import type { Hono } from "hono"; -import type { TenantEnv } from "@intx/hub-api"; -import { artifactApp, createTestDb, grant, seedActor, type TestDb } from "./lib/db-harness.js"; -import { createFsContentStore } from "./lib/fs-content-store.js"; - -let testDb: TestDb; -let dir: string; -let app: Hono; - -beforeAll(async () => { - testDb = await createTestDb(); - dir = await mkdtemp(join(tmpdir(), "artifact-store-")); - const actor = await seedActor(testDb.db, "acme"); - await grant(testDb.db, actor, "artifact:*", "write"); - app = artifactApp(testDb.db, actor, createFsContentStore(dir)); -}); - -afterAll(async () => { - await testDb?.close(); - await rm(dir, { recursive: true, force: true }); -}); - -async function download(id: string, version?: number): Promise { - const query = version === undefined ? "" : `?version=${version}`; - return await app.request(`/api/artifacts/${id}/download${query}`); -} - -describe("upload, version, download on a filesystem ContentStore", () => { - test("the uploaded bytes come back unchanged across a new version", async () => { - const bytes = new Uint8Array([0x25, 0x50, 0x44, 0x46, 0x00, 0xff, 0x10, 0x80]); - const form = new FormData(); - form.append("files", new File([bytes], "report.pdf", { type: "application/pdf" })); - const uploaded = await app.request("/api/artifacts/upload", { method: "POST", body: form }); - expect(uploaded.status).toBe(201); - const { artifacts } = (await uploaded.json()) as { artifacts: { id: string }[] }; - const id = artifacts[0]!.id; - - const v1 = await download(id, 1); - expect(v1.status).toBe(200); - expect(new Uint8Array(await v1.arrayBuffer())).toEqual(bytes); - - const revised = await app.request(`/api/artifacts/${id}/versions`, { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ title: "report (final).pdf" }), - }); - expect(revised.status).toBe(200); - - const v2 = await download(id, 2); - expect(v2.status).toBe(200); - expect(new Uint8Array(await v2.arrayBuffer())).toEqual(bytes); - - // Uploaded bytes live in the store, not in version history, so an older - // version of a stored file is refused rather than served as current bytes. - expect((await download(id, 1)).status).toBe(400); - }); -}); diff --git a/tests/lib/db-harness.ts b/tests/lib/db-harness.ts deleted file mode 100644 index 86b5d9d..0000000 --- a/tests/lib/db-harness.ts +++ /dev/null @@ -1,152 +0,0 @@ -// Real-Postgres harness for the tests/ suites: a fresh database per suite with -// Interchange's control plane and this package's migrations applied, and a -// mounted artifact app for a seeded tenant principal. -import { randomUUID } from "node:crypto"; -import { - createDB, - createGrantStore, - runMigrations, - schema as intx, - type DBConfig, -} from "@intx/db"; -import { createRequireGrant, type TenantEnv } from "@intx/hub-api"; -import { generateId } from "@intx/hub-common"; -import { Hono } from "hono"; -import postgres from "postgres"; -import { - createArtifactRoutes, - InlineContentStore, - runArtifactMigrations, - type ContentStore, -} from "../../src/index.js"; -import { - assertDestructiveArtifactTestsAllowed, - databaseConfig, - DATABASE_URL, -} from "../../src/test-helpers.js"; - -type Tenant = typeof intx.tenant.$inferSelect; -type Principal = typeof intx.principal.$inferSelect; -type HostDb = ReturnType["db"]; - -export type Actor = { tenant: Tenant; principal: Principal }; - -export type TestDb = { - db: HostDb; - config: DBConfig; - close: () => Promise; -}; - -export function connectionString(config: DBConfig): string { - const user = encodeURIComponent(config.user); - const password = encodeURIComponent(config.password ?? ""); - return `postgres://${user}:${password}@${config.host}:${config.port}/${config.database}`; -} - -async function admin(run: (sql: postgres.Sql) => Promise): Promise { - const sql = postgres(DATABASE_URL, { max: 1, onnotice: () => undefined }); - try { - return await run(sql); - } finally { - await sql.end(); - } -} - -/** - * Creates `artifact__test`, applies Interchange's migrations and then - * `migrateArtifacts` (this package's by default), and drops it on `close`. - */ -export async function createTestDb( - migrateArtifacts: (config: DBConfig) => Promise = (config) => - runArtifactMigrations(config, { schema: "public" }), -): Promise { - assertDestructiveArtifactTestsAllowed(DATABASE_URL); - const name = `artifact_${randomUUID().replaceAll("-", "").slice(0, 12)}_test`; - await admin((sql) => sql.unsafe(`CREATE DATABASE "${name}"`)); - const server = databaseConfig(DATABASE_URL); - const config: DBConfig = { - host: server.host, - port: server.port, - user: server.user, - password: server.password, - database: name, - }; - await runMigrations(config, { schema: "public" }); - await migrateArtifacts(config); - const handle = createDB(config); - return { - db: handle.db, - config, - close: async () => { - await handle.close(); - await admin((sql) => sql.unsafe(`DROP DATABASE IF EXISTS "${name}" WITH (FORCE)`)); - }, - }; -} - -/** A tenant with one active user principal, allowed to create artifacts. */ -export async function seedActor(db: HostDb, slug: string): Promise { - const [tenant] = await db - .insert(intx.tenant) - .values({ id: generateId("tenant"), name: slug, slug, domain: `${slug}.example` }) - .returning(); - const [principal] = await db - .insert(intx.principal) - .values({ - id: generateId("principal"), - tenantId: tenant!.id, - kind: "user", - refId: `user-${slug}`, - status: "active", - }) - .returning(); - await grant(db, { tenant: tenant!, principal: principal! }, "artifact:*", "create"); - return { tenant: tenant!, principal: principal! }; -} - -export async function grant( - db: HostDb, - actor: Actor, - resource: string, - action: string, -): Promise { - await db.insert(intx.grant).values({ - id: generateId("grant"), - tenantId: actor.tenant.id, - principalId: actor.principal.id, - roleId: null, - resource, - action, - effect: "allow", - origin: "system", - conditions: null, - }); -} - -/** - * `createArtifactRoutes` mounted at `/api` for `actor`, authorized by the - * platform's real `createRequireGrant` over the database's `grant` table. - */ -export function artifactApp( - db: HostDb, - actor: Actor, - contentStore: ContentStore = InlineContentStore, -): Hono { - const app = new Hono(); - app.use("*", async (c, next) => { - c.set("tenant", actor.tenant); - c.set("principal", actor.principal); - await next(); - }); - return app.route( - "/api", - createArtifactRoutes({ - db, - contentStore, - requireGrant: createRequireGrant({ - grantStore: createGrantStore(db), - conditionRegistry: {}, - }), - }), - ); -} diff --git a/tests/lib/fs-content-store.ts b/tests/lib/fs-content-store.ts deleted file mode 100644 index 9b077c8..0000000 --- a/tests/lib/fs-content-store.ts +++ /dev/null @@ -1,37 +0,0 @@ -// A ContentStore that keeps file bytes on disk, one file per upload, referenced -// from the artifact's `source.upload.id` the way InlineContentStore references -// its bytea row. -import { randomUUID } from "node:crypto"; -import { mkdir, readFile, writeFile } from "node:fs/promises"; -import { join } from "node:path"; -import { uploadRefFromSource } from "../../src/content-store.js"; -import type { ContentStore } from "../../src/index.js"; - -export function createFsContentStore(dir: string): ContentStore { - const pathFor = (tenantId: string, id: string) => join(dir, tenantId, id); - return { - async put(_tx, scope, blob) { - const id = randomUUID(); - await mkdir(join(dir, scope.tenantId), { recursive: true }); - await writeFile(pathFor(scope.tenantId, id), blob.bytes); - return { - content: "", - source: { - upload: { - id, - filename: blob.filename, - mimeType: blob.mimeType, - size: blob.bytes.byteLength, - }, - }, - }; - }, - async get(_db, artifact) { - const ref = uploadRefFromSource(artifact.source); - if (ref?.id === undefined || artifact.tenantId === null) return null; - const bytes = await readFile(pathFor(artifact.tenantId, ref.id)).catch(() => null); - if (bytes === null) return null; - return { filename: ref.filename, mimeType: ref.mimeType, bytes: new Uint8Array(bytes) }; - }, - }; -} diff --git a/tests/upgrade-from-0.1.0.test.ts b/tests/upgrade-from-0.1.0.test.ts deleted file mode 100644 index dd52376..0000000 --- a/tests/upgrade-from-0.1.0.test.ts +++ /dev/null @@ -1,89 +0,0 @@ -// A database migrated and written by the published 0.1.0 package upgrades in -// place under this version's runArtifactMigrations. -import { afterAll, beforeAll, describe, expect, test } from "bun:test"; -import { sql } from "drizzle-orm"; -import * as v010 from "@corbits/artifacts-0.1.0"; -import { runArtifactMigrations } from "../src/index.js"; -import { - artifactApp, - connectionString, - createTestDb, - grant, - seedActor, - type Actor, - type TestDb, -} from "./lib/db-harness.js"; - -const PDF = new Uint8Array([0x25, 0x50, 0x44, 0x46, 0x2d, 0x31, 0x2e, 0x37, 0x00, 0xff]); - -let testDb: TestDb; -let actor: Actor; -let fileId: string; - -beforeAll(async () => { - testDb = await createTestDb(async (config) => { - const legacy = v010.createArtifactDb(connectionString(config)); - try { - await v010.runArtifactMigrations(legacy.db); - } finally { - await legacy.close(); - } - }); - actor = await seedActor(testDb.db, "acme"); - await grant(testDb.db, actor, "artifact:*", "write"); - const scope = { tenantId: actor.tenant.id, principalId: actor.principal.id }; - - const legacy = v010.createArtifactDb(connectionString(testDb.config)); - try { - const file = await legacy.db.transaction((tx) => - v010.createFileArtifact(tx, v010.InlineContentStore, { - scope, - ownerPrincipalId: scope.principalId, - filename: "deck.pdf", - mimeType: "application/pdf", - bytes: PDF, - policy: v010.ARTIFACT_UPLOAD_POLICY, - }), - ); - fileId = file.id; - await v010.saveMailAttachmentRefs(legacy.db, { - scope, - instanceId: "inst-1", - body: { - mailId: "mail-1", - attachments: [ - { artifactId: file.id, name: "deck.pdf", type: "application/pdf", size: PDF.length }, - ], - }, - }); - } finally { - await legacy.close(); - } - - await runArtifactMigrations(testDb.config, { schema: "public" }); -}); - -afterAll(async () => { - await testDb?.close(); -}); - -describe("upgrading a 0.1.0 database", () => { - test("drops mail_attachment_ref and the 0.1.0 migration ledger", async () => { - const rows = await testDb.db.execute<{ table_name: string }>(sql` - SELECT table_name FROM information_schema.tables - WHERE table_schema = 'artifacts' - ORDER BY table_name - `); - expect(rows.map((row) => row.table_name)).toEqual([ - "artifact", - "artifact_version", - "upload", - ]); - }); - - test("a 0.1.0 upload still downloads its bytes", async () => { - const res = await artifactApp(testDb.db, actor).request(`/api/artifacts/${fileId}/download`); - expect(res.status).toBe(200); - expect(new Uint8Array(await res.arrayBuffer())).toEqual(PDF); - }); -}); diff --git a/tsconfig.build.json b/tsconfig.build.json index a8a6531..9998a21 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -14,5 +14,5 @@ "rootDir": "src" }, "include": ["src"], - "exclude": ["src/**/*.test.ts", "src/test-helpers.ts"] + "exclude": ["src/**/*.test.ts"] } diff --git a/tsconfig.json b/tsconfig.json index c5d604d..aeb030c 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -11,5 +11,5 @@ "types": ["node", "@types/bun"], "paths": { "@corbits/artifacts": ["./src/index.ts"] } }, - "include": ["src", "tests"] + "include": ["src", "e2e"] }