diff --git a/AGENTS.md b/AGENTS.md index 9a01dcd..0b05bd8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,7 +22,8 @@ CI runs `typecheck` + `test` — both must pass before any push. - `src/index.ts` — public surface: `createMemory` (optional `app` registers HTTP), `registerMemoryRoutes` - `src/mount-config.ts` / `src/config.ts` — mount config + engine config -- `src/routes/` — Hono routes (`add`, `search`, `list`) +- `src/routes/` — Hono routes (`add`, `search`, `list`, `feed`, retention + `forget`/`purge`/`retention-class`) - `src/tools/` — Interchange `defineTool` factories (`@corbits/memory/tools`); HTTP clients for mounted routes (env credentials; no in-process plane) - `src/services/` — capture / search / transform internals (not public verbs) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 896f330..0eba61f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -56,6 +56,20 @@ helpers are optional multi-writer / backfill — not the primary path. anything from the request body, is what `grantGuard` authorizes. - **Grants delegate to the host.** Pass `grantStore` + `conditionRegistry`; routes use `createRequireGrant("memory", action)`. +- **Two authorization mechanisms, not one — know which is source of truth + for what.** (1) Grant tags decide *capability* (may this principal call + `add`/`search`/`forget`/`purge` at all — `requireGrant`) and *visibility* + (which documents a principal may see — `accessTags` + `canAccessDocument` + in `grant-tags.ts`, where a share grant legitimately widens who can find a + document). (2) A separate, imperative **ownership** check — the creator + lookup in `services/retention-ownership.ts`, called from `memory.ts` — + decides who may *forget or purge* a specific document, and is the sole + source of truth for "whose document is this": it is never derived from + grant tags and a share grant never satisfies it. `MemoryGrantRequirement. + installHint` (`grant-requirements.ts`) looks adjacent to this but is not: + it is advisory metadata for install tooling sizing a capability grant, + read by nothing at request time. Do not extend mechanism (1) expecting it + to cover ownership — extend `retention-ownership.ts` instead. - **Dependencies**: `@intx/hub-api`, `@intx/authz`, `@intx/log`, Hono, Drizzle, arktype, `postgres`, `hono-openapi`. LGPL-2.1 — see `LICENSE`. @@ -85,9 +99,23 @@ exposes the same three verbs. authority/recency → MMR); optional live `SourceProvider` merge (fail-soft). - `GET /api/tenants/:tenantId/memory/list` — recent documents, same grant-tag filter as local search. - -Returns an in-process `Memory` (`add`, `search`, `list`, `close`) for host -workers and ingestion modules that already resolved identity. +- `POST /api/tenants/:tenantId/memory/documents/:documentId/forget` — tombstone + (grant `memory:forget`; creator-only, see below). +- `POST /api/tenants/:tenantId/memory/documents/:documentId/purge` — hard + delete (grant `memory:purge`; creator-only; irreversible). +- `POST /api/tenants/:tenantId/memory/versions/:versionId/retention-class` — + set retention class (grant `memory:forget`; creator-only). + +Forget and purge are deliberately separate routes and separate grant actions +(never one route with a boolean flag) — a host wiring a "forget this" button +cannot accidentally wire up permanent deletion. `sweepEphemeral` (TTL +auto-deprecation) is **not** HTTP-routed: it is a maintenance sweep a host +schedules on its own cron, not a user action; call it in-process against the +returned `Memory`. See docs/RETENTION.md. + +Returns an in-process `Memory` (`add`, `search`, `list`, `close`, plus the +optional retention writes) for host workers and ingestion modules that +already resolved identity. **Agent tools live in this package** as thin HTTP clients (`@corbits/memory/tools` / `interchange.tools`): `defineTool` factories that diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d6c247..facf1f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Retention HTTP routes (CL-6288): `POST …/memory/documents/:documentId/forget` + (tombstone, grant `memory:forget`), `POST …/memory/documents/:documentId/purge` + (hard delete, grant `memory:purge`), and + `POST …/memory/versions/:versionId/retention-class` (grant `memory:forget`). + Forget and purge are separate routes with separate grant actions — never one + route with a boolean flag — and both are refused with 403 unless the caller + is the document/version's creator, independent of any share grant that lets + them merely see it. `sweepEphemeral` stays off the HTTP surface (maintenance + sweep, not a user action); a host schedules it on its own cron against the + in-process `Memory`. New `memory:forget` / `memory:purge` grant requirements + (`source: "creator"`) and `capabilityIdsForSurface()` so distiller/tools + installs no longer pick up routes-only capabilities by accident. - `RouteDeps.callerResolver` / `createMemory({ callerResolver })` — an optional host-supplied resolver from a request to a `{ tenantId, principalId }` scope, for a caller that never goes through the host's diff --git a/IMPLEMENTATION.md b/IMPLEMENTATION.md index 01f860d..e55bdd5 100644 --- a/IMPLEMENTATION.md +++ b/IMPLEMENTATION.md @@ -633,8 +633,13 @@ surface, or a migrating host silently loses them. | `POST /api/tenants/:tenantId/memory/add` | `add` | `{ title, text, access_tags?, share? }` | `200 { documentId, versionId }`; `400` on validation | | `POST /api/tenants/:tenantId/memory/search` | `search` | `{ query, limit?, kinds?, entity_ids?, sources?, includeEvidence? }` (limit 1–50; `kinds`/`entity_ids`/`sources` narrow retrieval before fusion; unset or `[]` = unfiltered; `includeEvidence` adds a short evidence string when true) | `200 { items[], evidence?, degraded? }`; `400` on bad input | | `GET /api/tenants/:tenantId/memory/list` | `search` | query `?limit=` (1–100, string on the wire) | `200 { events: [{ at, title, source, tenantId, principalId }] }` — durable recent documents for the caller's scope, filtered with grant-tag access (`canAccessDocument`). One event per document (active live version). | +| `GET /api/tenants/:tenantId/memory/feed` | `search` | query `?after=&limit=&exclude_generator=` | `200 { entries[], nextCursor }` — cursor pull of new live versions. See `docs/FEED.md`. | +| `POST /api/tenants/:tenantId/memory/documents/:documentId/forget` | `forget` | `{ reason? }` | `200 { documentId, versions }`; `403` unless caller is the document's creator; `404` unknown document. Tombstones — content is redacted, not archived; see docs/RETENTION.md. | +| `POST /api/tenants/:tenantId/memory/documents/:documentId/purge` | `purge` | none | `200 { documentId, deleted, reason? }`; `403` unless caller is the document's creator; `404` unknown document. Hard-deletes the row — irreversible; refused while a `durable` version is untombstoned. | +| `POST /api/tenants/:tenantId/memory/versions/:versionId/retention-class` | `forget` | `{ retention_class }` | `200 { versionId, documentId, status }`; `400` invalid class; `403` unless caller is the version's creator; `404` unknown version. | -`registerMemoryRoutes` and `createMemory({ app })` register the three HTTP routes. +`registerMemoryRoutes` and `createMemory({ app })` register these seven HTTP +routes (add, search, list, feed, forget, purge, retention-class). Agent tools ship in this package as Interchange `defineTool` factories (`@corbits/memory/tools` / `interchange.tools`): thin HTTP clients that call the mounted routes with install env (`memoryBaseUrl`, `memoryTenantId`, @@ -645,7 +650,10 @@ protection — the client has no default timeout. OpenAPI→MCP remains an optio host bridge. The plane surface is `add` / `search` / `list` / `close`, plus optional transform methods when backed by the engine DocumentStore (`createTransformConfig`, `listTransformConfigs`, `runTransform`, -`promoteGeneration`, `demoteGeneration`). Inference stays on the host. +`promoteGeneration`, `demoteGeneration`) and optional retention methods +(`tombstoneDocument`, `hardDeleteDocument`, `setRetentionClass`, +`sweepEphemeral`, `deprecateVersion`) — see docs/RETENTION.md. Inference stays +on the host. ### Share materialization (CL-5873) diff --git a/docs/RETENTION.md b/docs/RETENTION.md index fe230cd..9963e1d 100644 --- a/docs/RETENTION.md +++ b/docs/RETENTION.md @@ -7,7 +7,7 @@ Versions carry a **retention class** orthogonal to temporal ranking class | --- | --- | | `durable` | Long-lived claims; hard-delete blocked until tombstoned | | `standard` | Default working memory | -| `ephemeral` | Short TTL; sweeper hard-deletes past `valid_until` (or 7d from `ingested_at`) | +| `ephemeral` | Short TTL; sweeper deprecates past `valid_until` (or 7d from `ingested_at`) — hard delete is a separate explicit step | | `source_only` | Keep raw capture; derived versions may be dropped by host policy | Schema: `memory.version.retention_class` (migration `0007_retention.sql`). @@ -21,7 +21,7 @@ CHECK constraint `version_retention_class_check` stays lockstep with | Deprecate | `memory.deprecateVersion` | `status=deprecated`, `deprecated_at` / reason | | Tombstone | `memory.tombstoneDocument` | All active/deprecated/superseded versions → `tombstoned`; chunk text redacted to `[redacted]` | | Hard delete | `memory.hardDeleteDocument` | Deletes document row (cascade); **refuses** if any non-tombstoned version is `durable` | -| Sweep | `memory.sweepEphemeral` | Auto-deprecates ephemeral versions past `valid_until` (or 7d from `ingested_at`); host schedules, core is cron-free | +| Sweep | `memory.sweepEphemeral` | Auto-**deprecates** (never deletes) ephemeral versions past `valid_until` (or 7d from `ingested_at`); host schedules, core is cron-free | | Set class | `memory.setRetentionClass` | Update `retention_class` on a version | Search and feed exclude non-active (and non-superseded for feed) rows by @@ -30,3 +30,46 @@ versions intentionally (ops / audit). Hard-delete is a separate explicit verb — TTL never hard-deletes. Service module: `src/services/retention.ts`. + +## HTTP surface (CL-6288) + +| Route | Grant action | Plane verb | +| --- | --- | --- | +| `POST …/memory/documents/:documentId/forget` | `memory:forget` | `tombstoneDocument` | +| `POST …/memory/documents/:documentId/purge` | `memory:purge` | `hardDeleteDocument` | +| `POST …/memory/versions/:versionId/retention-class` | `memory:forget` | `setRetentionClass` | + +`deprecateVersion` and `sweepEphemeral` have no route (see below). + +**Tombstone vs. hard delete stay distinct verbs, distinct grants.** A UI +offering "forget this" must never be one flag away from "shred this" by +accident. `forget` (tombstone) is **not** an undo-able action: the document +stops appearing in search/feed and its chunk text is overwritten with +`[redacted]` — the original content does not survive, there is no +un-tombstone/restore verb, and only version metadata (status, timestamps, +retention class) remains for audit. `purge` (hard delete) goes further and +removes the document row itself; it has its own grant action and is refused +outright while a `durable`-class version on the document is untombstoned. The +distinction that matters is *what's still queryable*: after `forget` a +document row and its metadata still exist (for audit) but its content is +gone; after `purge` nothing does. A host can grant `forget` broadly (every +user gets a "forget this" button) while keeping `purge` to an operator role — +but should not describe `forget` to end users as reversible. + +**Ownership, not just visibility.** `memory:search`/a document's `accessTags` +say who can *see* a document — never who may forget or purge it. Every +retention route additionally checks that the caller is the document's +creator (`created_by_principal_id` — the document's first version for +`forget`/`purge`, the specific version's own creator for `retention-class`), +independent of any share grant. A peer who can search a shared document gets +403 on `forget`/`purge`/`retention-class` for it. See +`src/services/retention-ownership.ts` and the ownership tests in +`src/memory.test.ts` / `src/routes/routes.test.ts`. + +**`sweepEphemeral` stays off the HTTP surface.** It is a maintenance sweep — +"deprecate every ephemeral version past its TTL for this tenant" — not +something a single user requests about their own data, and it has no natural +per-caller grant (it does not take a `principalId` and touches every +matching row tenant-wide). A host that wants it schedules a cron job calling +`memory.sweepEphemeral({ tenantId })` in-process (the returned `Memory` +already exposes it); the engine stays cron-free per `ARCHITECTURE.md`. diff --git a/package.json b/package.json index 963fe78..30d947e 100644 --- a/package.json +++ b/package.json @@ -15,14 +15,26 @@ { "resource": "memory", "action": "add", - "source": "tenant", + "installHint": "tenant", "surfaces": ["tools", "distiller", "routes"] }, { "resource": "memory", "action": "search", - "source": "tenant", + "installHint": "tenant", "surfaces": ["tools", "distiller", "routes"] + }, + { + "resource": "memory", + "action": "forget", + "installHint": "creator", + "surfaces": ["routes"] + }, + { + "resource": "memory", + "action": "purge", + "installHint": "creator", + "surfaces": ["routes"] } ] }, diff --git a/src/core/schemas/non-blank-id.ts b/src/core/schemas/non-blank-id.ts new file mode 100644 index 0000000..0381b88 --- /dev/null +++ b/src/core/schemas/non-blank-id.ts @@ -0,0 +1,15 @@ +import { type } from "arktype"; + +/** + * `"string >= 1"` is a LENGTH constraint, not a content one — `" "` has + * length 1 and would pass it, seating/accepting a whitespace-only id exactly + * like the empty-string case such a schema exists to reject. Require at + * least one non-whitespace character instead. + * + * Shared by the resolved-caller trust boundary (`routes/deps.ts`, CL-6286) + * and the retention path-param schemas (`http-bodies.ts`, CL-6288) so the + * fix lives in one schema instead of a comment repeated at each call site. + */ +export const NonBlankId = type("string").narrow( + (s, ctx) => s.trim().length > 0 || ctx.mustBe("non-blank (not just whitespace)"), +); diff --git a/src/distiller/workflow.ts b/src/distiller/workflow.ts index b59656b..34d96b1 100644 --- a/src/distiller/workflow.ts +++ b/src/distiller/workflow.ts @@ -23,7 +23,7 @@ import { defineWorkflow, type WorkflowDefinition } from "@intx/workflow"; import { memoryAdd } from "../tools/add.ts"; import { memoryFeed } from "../tools/feed.ts"; import { memorySearch } from "../tools/search.ts"; -import { MEMORY_CAPABILITY_IDS } from "../grant-requirements.ts"; +import { capabilityIdsForSurface } from "../grant-requirements.ts"; import { RESIDENT_DISTILLER_AGENT_ID, RESIDENT_DISTILLER_CRON_DEFAULT, @@ -97,7 +97,7 @@ export function createResidentDistiller( "Resident memory distiller — feed → classify → claim write", systemPrompt: opts.systemPrompt ?? DEFAULT_SYSTEM_PROMPT(generatorAgentId), tools, - capabilities: [...MEMORY_CAPABILITY_IDS], + capabilities: capabilityIdsForSurface("distiller"), inference: opts.inference, tags: { diff --git a/src/grant-requirements.test.ts b/src/grant-requirements.test.ts index 62aa248..0860846 100644 --- a/src/grant-requirements.test.ts +++ b/src/grant-requirements.test.ts @@ -3,27 +3,79 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; import { + capabilityIdsForSurface, MEMORY_CAPABILITY_IDS, MEMORY_GRANT_REQUIREMENTS, } from "./grant-requirements.ts"; describe("MEMORY_GRANT_REQUIREMENTS", () => { - test("covers add + search on memory resource", () => { + test("covers add, search, forget, and purge on memory resource", () => { expect(MEMORY_GRANT_REQUIREMENTS.map((r) => r.action).sort()).toEqual([ "add", + "forget", + "purge", "search", ]); for (const r of MEMORY_GRANT_REQUIREMENTS) { expect(r.resource).toBe("memory"); - expect(r.source).toBe("tenant"); + } + }); + + test("add/search hint tenant-wide and reach tools + distiller + routes", () => { + for (const action of ["add", "search"]) { + const r = MEMORY_GRANT_REQUIREMENTS.find((x) => x.action === action)!; + expect(r.installHint).toBe("tenant"); expect(r.surfaces).toContain("tools"); expect(r.surfaces).toContain("distiller"); + expect(r.surfaces).toContain("routes"); + } + }); + + test("forget/purge hint creator-scoped and are routes-only", () => { + for (const action of ["forget", "purge"]) { + const r = MEMORY_GRANT_REQUIREMENTS.find((x) => x.action === action)!; + expect(r.installHint).toBe("creator"); + expect(r.surfaces).toEqual(["routes"]); + } + }); + + test("installHint is advisory only — the requirement shape carries no enforcement field", () => { + // The actual ownership check lives in services/retention-ownership.ts, + // wired imperatively into memory.ts, entirely independent of this hint. + // This test exists so a future reader who tightens grant-requirements.ts + // notices this comment rather than assuming installHint is load-bearing. + for (const r of MEMORY_GRANT_REQUIREMENTS) { + expect(Object.keys(r).sort()).toEqual([ + "action", + "installHint", + "resource", + "surfaces", + ]); } }); test("capability ids are resource:action", () => { expect([...MEMORY_CAPABILITY_IDS].sort()).toEqual([ "memory:add", + "memory:forget", + "memory:purge", + "memory:search", + ]); + }); + + test("capabilityIdsForSurface excludes routes-only actions from distiller/tools", () => { + expect(capabilityIdsForSurface("distiller").sort()).toEqual([ + "memory:add", + "memory:search", + ]); + expect(capabilityIdsForSurface("tools").sort()).toEqual([ + "memory:add", + "memory:search", + ]); + expect(capabilityIdsForSurface("routes").sort()).toEqual([ + "memory:add", + "memory:forget", + "memory:purge", "memory:search", ]); }); @@ -36,7 +88,7 @@ describe("MEMORY_GRANT_REQUIREMENTS", () => { grantRequirements?: Array<{ resource: string; action: string; - source: string; + installHint: string; surfaces: string[]; }>; }; @@ -46,7 +98,7 @@ describe("MEMORY_GRANT_REQUIREMENTS", () => { MEMORY_GRANT_REQUIREMENTS.map((r) => ({ resource: r.resource, action: r.action, - source: r.source, + installHint: r.installHint, surfaces: [...r.surfaces], })), ); diff --git a/src/grant-requirements.ts b/src/grant-requirements.ts index e52d530..e1f880c 100644 --- a/src/grant-requirements.ts +++ b/src/grant-requirements.ts @@ -6,11 +6,23 @@ * export is the in-repo SSOT; keep package.json in lockstep. * * Shape matches Interchange definition grant requirements - * (`resource` + `action` + `source`). Control plane materializes grants + * (`resource` + `action` + `installHint`). Control plane materializes grants * onto the workflow principal at deploy/launch. */ -export type MemoryGrantSource = "tenant" | "creator" | "invoker"; +/** + * Advisory-only sizing hint for install tooling deciding how broadly to mint + * the underlying `resource`/`action` capability grant (e.g. "give every + * tenant member `memory:search`" vs "give this principal `memory:forget` + * scoped to what it creates"). **Nothing in this package reads or enforces + * this value** — it is not a `requireGrant`/`canAccessDocument` mode switch, + * and it does not gate anything at request time. Whether a specific caller + * may actually forget/purge a specific document is decided entirely by the + * imperative creator check in `services/retention-ownership.ts` (wired into + * `memory.ts`), independent of grant tags and of this field. See + * ARCHITECTURE.md § Boundaries for the two-mechanism split. + */ +export type MemoryGrantInstallHint = "tenant" | "creator" | "invoker"; /** Package surfaces that need the requirement when installed. */ export type MemoryGrantSurface = "tools" | "distiller" | "routes"; @@ -18,8 +30,8 @@ export type MemoryGrantSurface = "tools" | "distiller" | "routes"; export type MemoryGrantRequirement = { readonly resource: string; readonly action: string; - /** Recommended authority source; installer/deploy may override. */ - readonly source: MemoryGrantSource; + /** Install-sizing hint only — see `MemoryGrantInstallHint`. Not enforced. */ + readonly installHint: MemoryGrantInstallHint; readonly surfaces: readonly MemoryGrantSurface[]; }; @@ -32,18 +44,53 @@ export const MEMORY_GRANT_REQUIREMENTS = [ { resource: "memory", action: "add", - source: "tenant", + installHint: "tenant", surfaces: ["tools", "distiller", "routes"], }, { resource: "memory", action: "search", - source: "tenant", + installHint: "tenant", surfaces: ["tools", "distiller", "routes"], }, + /** + * Retention writes (CL-6288). Tombstone and retention-class changes share + * `forget`; hard delete gets its own `purge` so a host can hand out "let + * this user forget their own notes" without also handing out irreversible + * deletion. `installHint: "creator"` is a sizing suggestion for install + * tooling ONLY — the real per-document ownership check that stops a + * caller from forgetting/purging someone else's document runs in + * `services/retention-ownership.ts` regardless of how broadly this grant + * was minted. + */ + { + resource: "memory", + action: "forget", + installHint: "creator", + surfaces: ["routes"], + }, + { + resource: "memory", + action: "purge", + installHint: "creator", + surfaces: ["routes"], + }, ] as const satisfies readonly MemoryGrantRequirement[]; /** Compact `resource:action` form used on agent `capabilities` arrays. */ export const MEMORY_CAPABILITY_IDS = MEMORY_GRANT_REQUIREMENTS.map( (r) => `${r.resource}:${r.action}` as const, ); + +/** + * Capability ids scoped to one install surface — a distiller/tools install + * must not inherit a routes-only capability (like `forget`/`purge`) just + * because it appears somewhere in the full requirement list. + */ +export function capabilityIdsForSurface( + surface: MemoryGrantSurface, +): string[] { + return MEMORY_GRANT_REQUIREMENTS.filter((r) => + (r.surfaces as readonly MemoryGrantSurface[]).includes(surface), + ).map((r) => `${r.resource}:${r.action}`); +} diff --git a/src/http-bodies.ts b/src/http-bodies.ts index d4ea242..4b86b57 100644 --- a/src/http-bodies.ts +++ b/src/http-bodies.ts @@ -13,6 +13,7 @@ import { SEARCH_LIMIT_MAX, SEARCH_LIMIT_MIN, } from "./limits.ts"; +import { NonBlankId } from "./core/schemas/non-blank-id.ts"; export const ShareBody = type({ "tenant?": "boolean", @@ -152,3 +153,31 @@ export function parseWithArk( } return parsed; } + +/** Path param for the two document-scoped retention routes (forget/purge). */ +export const DocumentIdParam = type({ + documentId: NonBlankId, +}); + +export type DocumentIdParam = typeof DocumentIdParam.infer; + +/** Path param for the version-scoped retention-class route. */ +export const VersionIdParam = type({ + versionId: NonBlankId, +}); + +export type VersionIdParam = typeof VersionIdParam.infer; + +/** POST body for `.../forget` (tombstone) — reason is audit-only, never required. */ +export const ForgetRequest = type({ + "reason?": "string", +}); + +export type ForgetRequest = typeof ForgetRequest.infer; + +/** POST body for `.../retention-class`. Kept in lockstep with RETENTION_CLASSES (core/enums.ts). */ +export const SetRetentionClassRequest = type({ + retention_class: "'durable'|'standard'|'ephemeral'|'source_only'", +}); + +export type SetRetentionClassRequest = typeof SetRetentionClassRequest.infer; diff --git a/src/index.ts b/src/index.ts index 59230df..1b0a848 100644 --- a/src/index.ts +++ b/src/index.ts @@ -66,10 +66,11 @@ export { // Installer discovery — grant *requirements* (not live grants) export { + capabilityIdsForSurface, MEMORY_CAPABILITY_IDS, MEMORY_GRANT_REQUIREMENTS, type MemoryGrantRequirement, - type MemoryGrantSource, + type MemoryGrantInstallHint, type MemoryGrantSurface, } from "./grant-requirements.ts"; diff --git a/src/memory.test.ts b/src/memory.test.ts index 980d5e1..2200283 100644 --- a/src/memory.test.ts +++ b/src/memory.test.ts @@ -30,6 +30,7 @@ import type { MemoryConfig } from "./mount-config.ts"; import * as realDb from "./db/client.ts"; import * as realSearch from "./services/search.ts"; import * as realCapture from "./services/capture.ts"; +import * as realRetention from "./services/retention.ts"; import type { HybridSearchResult } from "./services/search.ts"; const PRINCIPAL = "p1"; @@ -768,3 +769,203 @@ async function freshPlane(opts?: { await plane.close(); }); }); + +describe("retention writes — ownership gate (CL-6288)", () => { + const OWNER = "alice"; + const OTHER = "mallory"; + + const tombstoneDocument = mock(() => Promise.resolve({ versions: 1 })); + const hardDeleteDocument = mock(() => Promise.resolve({ deleted: true })); + const setRetentionClass = mock(() => + Promise.resolve({ + versionId: "ver-1", + documentId: "doc-1", + status: "active", + }), + ); + + /** + * Only "doc-1" / "ver-1" exist, created by OWNER — everything else is a + * miss. resolveDocumentOwner and resolveVersionOwner both just need "a + * creator row" here (their distinct WHERE clauses are unit-tested with + * real scoping in retention-ownership.test.ts) so this fake does not + * branch on query text — a branch whose arms return the same row proves + * nothing and only invites the reader to assume a distinction that isn't + * there. + */ + const sql = Object.assign( + mock((_strings: TemplateStringsArray, ...values: unknown[]) => { + const isMissing = + values.includes("doc-missing") || values.includes("ver-missing"); + if (isMissing) return Promise.resolve([]); + return Promise.resolve([{ created_by_principal_id: OWNER }]); + }), + { + end: mock(() => Promise.resolve()), + unsafe: mock((sqlText: string) => ftsUnsafe(sqlText)), + }, + ); + + beforeAll(() => { + mock.module("./db/client.ts", () => ({ + ...realDb, + createDb: () => ({ db: {}, sql }), + })); + mock.module("./services/retention.ts", () => ({ + ...realRetention, + tombstoneDocument, + hardDeleteDocument, + setRetentionClass, + })); + }); + + afterAll(() => { + mock.module("./db/client.ts", () => realDb); + mock.module("./services/retention.ts", () => realRetention); + }); + + async function freshPlane() { + const { createMemory: makePlane } = await import( + `./memory.ts?retention-${Date.now()}-${Math.random()}` + ); + return makePlane({ config: wiringConfig }); + } + + function expectMemoryError(err: unknown, status: number, messagePart: string) { + expect(err).toBeInstanceOf(Error); + expect((err as Error).name).toBe("MemoryError"); + expect((err as { status: number }).status).toBe(status); + expect((err as Error).message).toContain(messagePart); + } + + it("tombstoneDocument succeeds for the creator", async () => { + tombstoneDocument.mockClear(); + const plane = await freshPlane(); + const result = await plane.tombstoneDocument({ + tenantId: TENANT, + principalId: OWNER, + documentId: "doc-1", + }); + expect(result).toEqual({ versions: 1 }); + expect(tombstoneDocument).toHaveBeenCalled(); + await plane.close(); + }); + + it("tombstoneDocument is refused for a non-creator even with document visibility", async () => { + tombstoneDocument.mockClear(); + const plane = await freshPlane(); + try { + await plane.tombstoneDocument({ + tenantId: TENANT, + principalId: OTHER, + documentId: "doc-1", + }); + throw new Error("expected tombstoneDocument to reject"); + } catch (err) { + expectMemoryError(err, 403, "creator"); + } + expect(tombstoneDocument).not.toHaveBeenCalled(); + await plane.close(); + }); + + it("tombstoneDocument 404s for an unknown document", async () => { + tombstoneDocument.mockClear(); + const plane = await freshPlane(); + try { + await plane.tombstoneDocument({ + tenantId: TENANT, + principalId: OWNER, + documentId: "doc-missing", + }); + throw new Error("expected tombstoneDocument to reject"); + } catch (err) { + expectMemoryError(err, 404, "not found"); + } + expect(tombstoneDocument).not.toHaveBeenCalled(); + await plane.close(); + }); + + it("hardDeleteDocument succeeds for the creator", async () => { + hardDeleteDocument.mockClear(); + const plane = await freshPlane(); + const result = await plane.hardDeleteDocument({ + tenantId: TENANT, + principalId: OWNER, + documentId: "doc-1", + }); + expect(result).toEqual({ deleted: true }); + expect(hardDeleteDocument).toHaveBeenCalled(); + await plane.close(); + }); + + it("hardDeleteDocument is refused for a non-creator even with document visibility", async () => { + hardDeleteDocument.mockClear(); + const plane = await freshPlane(); + try { + await plane.hardDeleteDocument({ + tenantId: TENANT, + principalId: OTHER, + documentId: "doc-1", + }); + throw new Error("expected hardDeleteDocument to reject"); + } catch (err) { + expectMemoryError(err, 403, "creator"); + } + expect(hardDeleteDocument).not.toHaveBeenCalled(); + await plane.close(); + }); + + it("setRetentionClass succeeds for the version's creator", async () => { + setRetentionClass.mockClear(); + const plane = await freshPlane(); + const result = await plane.setRetentionClass({ + tenantId: TENANT, + principalId: OWNER, + versionId: "ver-1", + retentionClass: "durable", + }); + expect(result).toEqual({ + versionId: "ver-1", + documentId: "doc-1", + status: "active", + }); + expect(setRetentionClass).toHaveBeenCalled(); + await plane.close(); + }); + + it("setRetentionClass is refused for a non-creator", async () => { + setRetentionClass.mockClear(); + const plane = await freshPlane(); + try { + await plane.setRetentionClass({ + tenantId: TENANT, + principalId: OTHER, + versionId: "ver-1", + retentionClass: "durable", + }); + throw new Error("expected setRetentionClass to reject"); + } catch (err) { + expectMemoryError(err, 403, "creator"); + } + expect(setRetentionClass).not.toHaveBeenCalled(); + await plane.close(); + }); + + it("setRetentionClass 404s for an unknown version", async () => { + setRetentionClass.mockClear(); + const plane = await freshPlane(); + try { + await plane.setRetentionClass({ + tenantId: TENANT, + principalId: OWNER, + versionId: "ver-missing", + retentionClass: "durable", + }); + throw new Error("expected setRetentionClass to reject"); + } catch (err) { + expectMemoryError(err, 404, "not found"); + } + expect(setRetentionClass).not.toHaveBeenCalled(); + await plane.close(); + }); +}); diff --git a/src/memory.ts b/src/memory.ts index f965d81..f1e3af8 100644 --- a/src/memory.ts +++ b/src/memory.ts @@ -48,6 +48,11 @@ import { sweepEphemeral, tombstoneDocument, } from "./services/retention.ts"; +import { + isOwner, + resolveDocumentOwner, + resolveVersionOwner, +} from "./services/retention-ownership.ts"; import { documentTag, materializeShareGrants, @@ -334,7 +339,13 @@ export type Memory = { generation: string; }): Promise; /** - * Retention write paths (engine store only). See docs/RETENTION.md (CL-5871). + * Retention write paths (engine store only). See docs/RETENTION.md + * (CL-5871, ownership CL-6288). `tombstoneDocument`, `hardDeleteDocument`, + * and `setRetentionClass` take the caller's `principalId` and are refused + * (`MemoryError` 403) unless it matches the document/version creator — a + * share grant that lets a peer *see* a document never lets them forget or + * purge it. `deprecateVersion` and `sweepEphemeral` are not HTTP-routed + * and keep the CL-5871 tenant-only signature. */ deprecateVersion?(input: { tenantId: string; @@ -343,11 +354,13 @@ export type Memory = { }): Promise<{ versionId: string; documentId: string; status: string } | null>; tombstoneDocument?(input: { tenantId: string; + principalId: string; documentId: string; reason?: string; }): Promise<{ versions: number }>; hardDeleteDocument?(input: { tenantId: string; + principalId: string; documentId: string; }): Promise<{ deleted: boolean; reason?: string }>; sweepEphemeral?(input: { @@ -356,6 +369,7 @@ export type Memory = { }): Promise<{ versionsDeprecated: number }>; setRetentionClass?(input: { tenantId: string; + principalId: string; versionId: string; retentionClass: "durable" | "standard" | "ephemeral" | "source_only"; }): Promise<{ versionId: string; documentId: string; status: string } | null>; @@ -1058,6 +1072,16 @@ function createPlaneFromStore( "retention APIs require the engine DocumentStore", ); } + const owner = await resolveDocumentOwner(transformDeps.sql, input); + if (!owner.exists) { + throw new MemoryError(404, "document not found"); + } + if (!isOwner(owner, input.principalId)) { + throw new MemoryError( + 403, + "only the document's creator may forget it", + ); + } return tombstoneDocument(transformDeps.db, input); }, @@ -1068,6 +1092,16 @@ function createPlaneFromStore( "retention APIs require the engine DocumentStore", ); } + const owner = await resolveDocumentOwner(transformDeps.sql, input); + if (!owner.exists) { + throw new MemoryError(404, "document not found"); + } + if (!isOwner(owner, input.principalId)) { + throw new MemoryError( + 403, + "only the document's creator may purge it", + ); + } return hardDeleteDocument(transformDeps.db, input); }, @@ -1088,6 +1122,16 @@ function createPlaneFromStore( "retention APIs require the engine DocumentStore", ); } + const owner = await resolveVersionOwner(transformDeps.sql, input); + if (!owner.exists) { + throw new MemoryError(404, "version not found"); + } + if (!isOwner(owner, input.principalId)) { + throw new MemoryError( + 403, + "only the version's creator may change its retention class", + ); + } return setRetentionClass(transformDeps.db, input); }, }; diff --git a/src/routes/deps.ts b/src/routes/deps.ts index c5fdbd4..22d5684 100644 --- a/src/routes/deps.ts +++ b/src/routes/deps.ts @@ -10,6 +10,7 @@ import { type } from "arktype"; import { log } from "../log.ts"; import type { Memory } from "../memory.ts"; +import { NonBlankId } from "../core/schemas/non-blank-id.ts"; /** * The host's grant store + condition registry — the same pair it feeds @@ -41,16 +42,6 @@ export type CallerResolver = ( c: Context, ) => ResolvedCaller | null | Promise; -/** - * `"string >= 1"` is a LENGTH constraint, not a content one — `" "` has - * length 1 and would pass it, seating a whitespace-only scope exactly like - * the empty-string case this schema exists to reject. Require at least one - * non-whitespace character instead. - */ -const NonBlankId = type("string").narrow( - (s, ctx) => s.trim().length > 0 || ctx.mustBe("non-blank (not just whitespace)"), -); - /** * The one boundary where a host hands this package an identity, so it is * parsed like any other trust boundary (AGENTS.md invariant 4) rather than diff --git a/src/routes/mount.ts b/src/routes/mount.ts index 48ff69e..fd605d6 100644 --- a/src/routes/mount.ts +++ b/src/routes/mount.ts @@ -18,6 +18,11 @@ import { mountAddRoute } from "./add.ts"; import { mountSearchRoute } from "./search.ts"; import { mountListRoute } from "./list.ts"; import { mountFeedRoute } from "./feed.ts"; +import { + mountForgetRoute, + mountPurgeRoute, + mountSetRetentionClassRoute, +} from "./retention.ts"; export type { CallerResolver, @@ -26,7 +31,7 @@ export type { RouteDeps, } from "./deps.ts"; -/** HTTP JSON routes: add, search, list, feed. */ +/** HTTP JSON routes: add, search, list, feed, forget, purge, retention-class. */ export function registerMemoryRoutes( app: Hono, deps: RouteDeps, @@ -35,4 +40,7 @@ export function registerMemoryRoutes( mountSearchRoute(app, deps); mountListRoute(app, deps); mountFeedRoute(app, deps); + mountForgetRoute(app, deps); + mountPurgeRoute(app, deps); + mountSetRetentionClassRoute(app, deps); } diff --git a/src/routes/retention.ts b/src/routes/retention.ts new file mode 100644 index 0000000..179443c --- /dev/null +++ b/src/routes/retention.ts @@ -0,0 +1,215 @@ +/** + * Retention HTTP routes (CL-6288): forget (tombstone), purge (hard delete), + * and set-retention-class. See docs/RETENTION.md. + * + * Tombstone and hard delete are deliberately separate routes with separate + * grant actions (`forget` vs `purge`) — never one route with a boolean flag + * a client could flip by accident. `purge` is the one that actually removes + * data; its path and grant name say so. + * + * `sweepEphemeral` has no route here — it is a maintenance sweep a host + * schedules on its own cron, not a user action (see docs/RETENTION.md). + */ +import type { Hono } from "hono"; +import type { TenantEnv } from "@intx/hub-api"; +import { describeRoute, resolver, validator } from "hono-openapi"; +import { type } from "arktype"; + +import { formatCaughtError, log } from "../log.ts"; +import { + DocumentIdParam, + ForgetRequest, + SetRetentionClassRequest, + VersionIdParam, +} from "../http-bodies.ts"; +import { MemoryError } from "../memory.ts"; +import type { RouteDeps } from "./deps.ts"; +import { + caller, + grantGuard, + requirePrincipal, + resolveCaller, +} from "./deps.ts"; + +function respondRetentionError(err: unknown, action: string) { + const errMessage = formatCaughtError(err); + log.error(`memory ${action} failed: ${errMessage}`, { error: errMessage }); + if (err instanceof MemoryError) { + return { body: { error: err.message }, status: err.status as 403 | 404 | 501 }; + } + return { body: { error: `${action} failed` }, status: 502 as const }; +} + +const ForgetResponse = type({ + documentId: "string", + versions: "number", +}); + +const PurgeResponse = type({ + documentId: "string", + deleted: "boolean", + "reason?": "string", +}); + +const RetentionClassResponse = type({ + versionId: "string", + documentId: "string", + status: "string", +}); + +export function mountForgetRoute(app: Hono, deps: RouteDeps): void { + app.post( + "/api/tenants/:tenantId/memory/documents/:documentId/forget", + + describeRoute({ + tags: ["memory"], + summary: "Tombstone a document — stops appearing in search, chunk text is redacted (not archived), version rows stay for audit", + responses: { + 200: { + description: "Tombstoned", + content: { "application/json": { schema: resolver(ForgetResponse) } }, + }, + 401: { description: "No principal on the request context" }, + 403: { + description: + "Missing the memory:forget grant, or caller is not the document's creator", + }, + 404: { description: "Document not found" }, + 502: { description: "forget failed" }, + }, + }), + resolveCaller(deps), + requirePrincipal(), + grantGuard(deps, "forget"), + validator("param", DocumentIdParam), + validator("json", ForgetRequest), + async (c) => { + const { documentId } = c.req.valid("param"); + const { reason } = c.req.valid("json"); + const { scopeId, subjectId } = caller(c); + if (!deps.memory.tombstoneDocument) { + return c.json({ error: "retention APIs require the engine DocumentStore" }, 501); + } + try { + const result = await deps.memory.tombstoneDocument({ + tenantId: scopeId, + principalId: subjectId, + documentId, + ...(reason !== undefined ? { reason } : {}), + }); + return c.json({ documentId, versions: result.versions }); + } catch (err) { + const { body, status } = respondRetentionError(err, "forget"); + return c.json(body, status); + } + }, + ); +} + +export function mountPurgeRoute(app: Hono, deps: RouteDeps): void { + app.post( + "/api/tenants/:tenantId/memory/documents/:documentId/purge", + + describeRoute({ + tags: ["memory"], + summary: "Hard-delete a document — irreversible; refused while a durable version is untombstoned", + responses: { + 200: { + description: "Deletion result (deleted may be false with a reason)", + content: { "application/json": { schema: resolver(PurgeResponse) } }, + }, + 401: { description: "No principal on the request context" }, + 403: { + description: + "Missing the memory:purge grant, or caller is not the document's creator", + }, + 404: { description: "Document not found" }, + 502: { description: "purge failed" }, + }, + }), + resolveCaller(deps), + requirePrincipal(), + grantGuard(deps, "purge"), + validator("param", DocumentIdParam), + async (c) => { + const { documentId } = c.req.valid("param"); + const { scopeId, subjectId } = caller(c); + if (!deps.memory.hardDeleteDocument) { + return c.json({ error: "retention APIs require the engine DocumentStore" }, 501); + } + try { + const result = await deps.memory.hardDeleteDocument({ + tenantId: scopeId, + principalId: subjectId, + documentId, + }); + return c.json({ + documentId, + deleted: result.deleted, + ...(result.reason !== undefined ? { reason: result.reason } : {}), + }); + } catch (err) { + const { body, status } = respondRetentionError(err, "purge"); + return c.json(body, status); + } + }, + ); +} + +export function mountSetRetentionClassRoute( + app: Hono, + deps: RouteDeps, +): void { + app.post( + "/api/tenants/:tenantId/memory/versions/:versionId/retention-class", + + describeRoute({ + tags: ["memory"], + summary: "Set a version's retention class (durable/standard/ephemeral/source_only)", + responses: { + 200: { + description: "Updated", + content: { + "application/json": { schema: resolver(RetentionClassResponse) }, + }, + }, + 400: { description: "Invalid retention_class" }, + 401: { description: "No principal on the request context" }, + 403: { + description: + "Missing the memory:forget grant, or caller is not the version's creator", + }, + 404: { description: "Version not found" }, + 502: { description: "retention-class update failed" }, + }, + }), + resolveCaller(deps), + requirePrincipal(), + grantGuard(deps, "forget"), + validator("param", VersionIdParam), + validator("json", SetRetentionClassRequest), + async (c) => { + const { versionId } = c.req.valid("param"); + const { retention_class } = c.req.valid("json"); + const { scopeId, subjectId } = caller(c); + if (!deps.memory.setRetentionClass) { + return c.json({ error: "retention APIs require the engine DocumentStore" }, 501); + } + try { + const result = await deps.memory.setRetentionClass({ + tenantId: scopeId, + principalId: subjectId, + versionId, + retentionClass: retention_class, + }); + if (!result) { + return c.json({ error: "version not found" }, 404); + } + return c.json(result); + } catch (err) { + const { body, status } = respondRetentionError(err, "retention-class"); + return c.json(body, status); + } + }, + ); +} diff --git a/src/routes/routes.test.ts b/src/routes/routes.test.ts index b2185ac..dfa079d 100644 --- a/src/routes/routes.test.ts +++ b/src/routes/routes.test.ts @@ -5,6 +5,7 @@ import type { GrantRule } from "@intx/authz"; import { createRequireGrant, type TenantEnv } from "@intx/hub-api"; import type { Memory, TimelineEvent } from "../memory.ts"; +import { MemoryError } from "../memory.ts"; import { registerMemoryRoutes } from "./mount.ts"; import type { RouteDeps } from "./deps.ts"; @@ -27,6 +28,23 @@ const TENANT = "t1"; const SECRET_TITLE = "Q3 layoffs — draft list"; const PUBLIC_TITLE = "team standup notes"; +// "run-principal" mirrors RUN_PRINCIPAL in the callerResolver describe +// blocks below — a document/version created by a resolved machine caller. +const RETENTION_DOCS: Record = { + "doc-mine": { ownerId: PRINCIPAL }, + "doc-alice": { ownerId: "alice" }, + "doc-run-owned": { ownerId: "run-principal" }, +}; + +const RETENTION_VERSIONS: Record< + string, + { ownerId: string; documentId: string } +> = { + "ver-mine": { ownerId: PRINCIPAL, documentId: "doc-mine" }, + "ver-alice": { ownerId: "alice", documentId: "doc-alice" }, + "ver-run-owned": { ownerId: "run-principal", documentId: "doc-run-owned" }, +}; + function stubPlane(opts?: { timelineCatalog?: Array< TimelineEvent & { visibleTo: readonly string[] | "tenant" } @@ -38,6 +56,9 @@ function stubPlane(opts?: { entityIds: string[] | undefined; limit: number | undefined; }> = []; + const tombstoned: string[] = []; + const purged: string[] = []; + const retentionClassChanges: Array<{ versionId: string; retentionClass: string }> = []; const catalog = opts?.timelineCatalog ?? []; const plane: Memory = { capabilities: { embeddingsConfigured: true }, @@ -62,9 +83,48 @@ function stubPlane(opts?: { ) .map(({ visibleTo: _v, ...event }) => event); }, + // Mirrors memory.ts's real ownership gate — a caller who can only see a + // document via a share grant is not its creator and gets refused. + tombstoneDocument: async ({ documentId, principalId }) => { + const doc = RETENTION_DOCS[documentId]; + if (!doc) throw new MemoryError(404, "document not found"); + if (doc.ownerId !== principalId) { + throw new MemoryError(403, "only the document's creator may forget it"); + } + tombstoned.push(documentId); + return { versions: 1 }; + }, + hardDeleteDocument: async ({ documentId, principalId }) => { + const doc = RETENTION_DOCS[documentId]; + if (!doc) throw new MemoryError(404, "document not found"); + if (doc.ownerId !== principalId) { + throw new MemoryError(403, "only the document's creator may purge it"); + } + purged.push(documentId); + return { deleted: true }; + }, + setRetentionClass: async ({ versionId, principalId, retentionClass }) => { + const version = RETENTION_VERSIONS[versionId]; + if (!version) throw new MemoryError(404, "version not found"); + if (version.ownerId !== principalId) { + throw new MemoryError( + 403, + "only the version's creator may change its retention class", + ); + } + retentionClassChanges.push({ versionId, retentionClass }); + return { versionId, documentId: version.documentId, status: "active" }; + }, close: async () => {}, }; - return { plane, added, searched }; + return { + plane, + added, + searched, + tombstoned, + purged, + retentionClassChanges, + }; } function buildApp( @@ -76,7 +136,8 @@ function buildApp( principalId?: string; }, ) { - const { plane, added, searched } = stubPlane(opts); + const { plane, added, searched, tombstoned, purged, retentionClassChanges } = + stubPlane(opts); const grantConfig = { grantStore: createInMemoryGrantStore(grants), conditionRegistry: {}, @@ -112,7 +173,7 @@ function buildApp( await next(); }); registerMemoryRoutes(app, deps); - return { app, added, searched }; + return { app, added, searched, tombstoned, purged, retentionClassChanges }; } /** @@ -131,6 +192,9 @@ function stubMachinePlane(opts?: { const searched: { tenantId: string; principalId: string; query: string }[] = []; const fed: { tenantId: string; principalId: string }[] = []; + const tombstoned: string[] = []; + const purged: string[] = []; + const retentionClassChanges: Array<{ versionId: string; retentionClass: string }> = []; const catalog = opts?.timelineCatalog ?? []; const plane: Memory = { capabilities: { embeddingsConfigured: true }, @@ -163,9 +227,41 @@ function stubMachinePlane(opts?: { fed.push({ tenantId: p.tenantId, principalId: p.principalId }); return { entries: [], nextCursor: null }; }, + // Same creator-check semantics as stubPlane, for a resolved machine + // caller retiring the memory it created itself (CL-6288 review). + tombstoneDocument: async ({ documentId, principalId }) => { + const doc = RETENTION_DOCS[documentId]; + if (!doc) throw new MemoryError(404, "document not found"); + if (doc.ownerId !== principalId) { + throw new MemoryError(403, "only the document's creator may forget it"); + } + tombstoned.push(documentId); + return { versions: 1 }; + }, + hardDeleteDocument: async ({ documentId, principalId }) => { + const doc = RETENTION_DOCS[documentId]; + if (!doc) throw new MemoryError(404, "document not found"); + if (doc.ownerId !== principalId) { + throw new MemoryError(403, "only the document's creator may purge it"); + } + purged.push(documentId); + return { deleted: true }; + }, + setRetentionClass: async ({ versionId, principalId, retentionClass }) => { + const version = RETENTION_VERSIONS[versionId]; + if (!version) throw new MemoryError(404, "version not found"); + if (version.ownerId !== principalId) { + throw new MemoryError( + 403, + "only the version's creator may change its retention class", + ); + } + retentionClassChanges.push({ versionId, retentionClass }); + return { versionId, documentId: version.documentId, status: "active" }; + }, close: async () => {}, }; - return { plane, added, searched, fed }; + return { plane, added, searched, fed, tombstoned, purged, retentionClassChanges }; } function buildAppWithCallerResolver( @@ -177,7 +273,8 @@ function buildAppWithCallerResolver( >; }, ) { - const { plane, added, searched, fed } = stubMachinePlane(opts); + const { plane, added, searched, fed, tombstoned, purged, retentionClassChanges } = + stubMachinePlane(opts); const grantConfig = { grantStore: createInMemoryGrantStore(grants), conditionRegistry: {}, @@ -192,7 +289,7 @@ function buildAppWithCallerResolver( // browser session; `callerResolver` is the only source of identity here. const app = new Hono(); registerMemoryRoutes(app, deps); - return { app, added, searched, fed }; + return { app, added, searched, fed, tombstoned, purged, retentionClassChanges }; } function buildAppWithoutPrincipal() { @@ -420,6 +517,181 @@ describe("memory HTTP routes", () => { }); }); +describe("memory HTTP routes — retention (CL-6288)", () => { + test("forget tombstones the caller's own document", async () => { + const { app, tombstoned } = buildApp([grant(PRINCIPAL, "forget")]); + const res = await app.request( + "/api/tenants/t1/memory/documents/doc-mine/forget", + jsonPost({}), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { documentId: string; versions: number }; + expect(body).toEqual({ documentId: "doc-mine", versions: 1 }); + expect(tombstoned).toEqual(["doc-mine"]); + }); + + test("forget without the memory:forget grant is 403 (search does not authorize it)", async () => { + const { app, tombstoned } = buildApp([ + grant(PRINCIPAL, "search"), + grant(PRINCIPAL, "add"), + ]); + const res = await app.request( + "/api/tenants/t1/memory/documents/doc-mine/forget", + jsonPost({}), + ); + expect(res.status).toBe(403); + expect(tombstoned).toHaveLength(0); + }); + + test("forget is refused for a document owned by another principal, even with the forget grant", async () => { + // The mirror of CL-6286's cross-tenant test: PRINCIPAL can call forget + // (has the grant) but doc-alice belongs to "alice" — visibility via a + // share is not the same as ownership. + const { app, tombstoned } = buildApp([grant(PRINCIPAL, "forget")]); + const res = await app.request( + "/api/tenants/t1/memory/documents/doc-alice/forget", + jsonPost({}), + ); + expect(res.status).toBe(403); + const body = (await res.json()) as { error: string }; + expect(body.error).toContain("creator"); + expect(tombstoned).toHaveLength(0); + }); + + test("forget 404s for an unknown document", async () => { + const { app } = buildApp([grant(PRINCIPAL, "forget")]); + const res = await app.request( + "/api/tenants/t1/memory/documents/doc-unknown/forget", + jsonPost({}), + ); + expect(res.status).toBe(404); + }); + + test("purge hard-deletes the caller's own document", async () => { + const { app, purged } = buildApp([grant(PRINCIPAL, "purge")]); + const res = await app.request( + "/api/tenants/t1/memory/documents/doc-mine/purge", + jsonPost({}), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { documentId: string; deleted: boolean }; + expect(body).toEqual({ documentId: "doc-mine", deleted: true }); + expect(purged).toEqual(["doc-mine"]); + }); + + test("purge without the memory:purge grant is 403 — the forget grant does not authorize purge", async () => { + const { app, purged } = buildApp([grant(PRINCIPAL, "forget")]); + const res = await app.request( + "/api/tenants/t1/memory/documents/doc-mine/purge", + jsonPost({}), + ); + expect(res.status).toBe(403); + expect(purged).toHaveLength(0); + }); + + test("purge is refused for a document owned by another principal", async () => { + const { app, purged } = buildApp([grant(PRINCIPAL, "purge")]); + const res = await app.request( + "/api/tenants/t1/memory/documents/doc-alice/purge", + jsonPost({}), + ); + expect(res.status).toBe(403); + expect(purged).toHaveLength(0); + }); + + test("forget and purge are distinct routes — calling forget never hard-deletes", async () => { + const { app, tombstoned, purged } = buildApp([ + grant(PRINCIPAL, "forget"), + grant(PRINCIPAL, "purge"), + ]); + const res = await app.request( + "/api/tenants/t1/memory/documents/doc-mine/forget", + jsonPost({}), + ); + expect(res.status).toBe(200); + expect(tombstoned).toEqual(["doc-mine"]); + expect(purged).toHaveLength(0); + }); + + test("retention-class updates the version for its creator", async () => { + const { app, retentionClassChanges } = buildApp([ + grant(PRINCIPAL, "forget"), + ]); + const res = await app.request( + "/api/tenants/t1/memory/versions/ver-mine/retention-class", + jsonPost({ retention_class: "durable" }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { versionId: string; status: string }; + expect(body.versionId).toBe("ver-mine"); + expect(retentionClassChanges).toEqual([ + { versionId: "ver-mine", retentionClass: "durable" }, + ]); + }); + + test("retention-class rejects an invalid retention_class value (400)", async () => { + const { app } = buildApp([grant(PRINCIPAL, "forget")]); + const res = await app.request( + "/api/tenants/t1/memory/versions/ver-mine/retention-class", + jsonPost({ retention_class: "nonsense" }), + ); + expect(res.status).toBe(400); + }); + + test("retention-class is refused for a version owned by another principal", async () => { + const { app, retentionClassChanges } = buildApp([ + grant(PRINCIPAL, "forget"), + ]); + const res = await app.request( + "/api/tenants/t1/memory/versions/ver-alice/retention-class", + jsonPost({ retention_class: "ephemeral" }), + ); + expect(res.status).toBe(403); + expect(retentionClassChanges).toHaveLength(0); + }); + + test("missing principal on forget is 401", async () => { + const app = buildAppWithoutPrincipal(); + const res = await app.request( + "/api/tenants/t1/memory/documents/doc-mine/forget", + jsonPost({}), + ); + expect(res.status).toBe(401); + }); + + test("forget rejects a whitespace-only documentId (400, never reaching the plane)", async () => { + const { app, tombstoned } = buildApp([grant(PRINCIPAL, "forget")]); + const res = await app.request( + "/api/tenants/t1/memory/documents/%20/forget", + jsonPost({}), + ); + expect(res.status).toBe(400); + expect(tombstoned).toHaveLength(0); + }); + + test("purge rejects a whitespace-only documentId (400, never reaching the plane)", async () => { + const { app, purged } = buildApp([grant(PRINCIPAL, "purge")]); + const res = await app.request( + "/api/tenants/t1/memory/documents/%20/purge", + jsonPost({}), + ); + expect(res.status).toBe(400); + expect(purged).toHaveLength(0); + }); + + test("retention-class rejects a whitespace-only versionId (400, never reaching the plane)", async () => { + const { app, retentionClassChanges } = buildApp([ + grant(PRINCIPAL, "forget"), + ]); + const res = await app.request( + "/api/tenants/t1/memory/versions/%20/retention-class", + jsonPost({ retention_class: "durable" }), + ); + expect(res.status).toBe(400); + expect(retentionClassChanges).toHaveLength(0); + }); +}); + describe("memory HTTP routes — machine caller (callerResolver)", () => { const RUN_TENANT = "tenant-run"; const RUN_PRINCIPAL = "run-principal"; @@ -594,6 +866,66 @@ describe("memory HTTP routes — machine caller (callerResolver)", () => { expect(res.status).toBe(403); expect(fed).toHaveLength(0); }); + + // The most likely real-world caller of forget/purge: a workflow-run child + // (resolved via callerResolver, CL-6286) retiring memory it wrote itself. + // If the resolver's principalId ever drifted from created_by_principal_id + // (different derivation, casing, run-address vs principal-address), this + // is exactly what would start 403ing in production instead of a test. + + test("forget tombstones a document the resolved run's own principal created", async () => { + const { app, tombstoned } = buildAppWithCallerResolver( + [grant(RUN_PRINCIPAL, "forget")], + () => ({ tenantId: RUN_TENANT, principalId: RUN_PRINCIPAL }), + ); + const res = await app.request( + "/api/tenants/t1/memory/documents/doc-run-owned/forget", + jsonPost({}), + ); + expect(res.status).toBe(200); + expect(tombstoned).toEqual(["doc-run-owned"]); + }); + + test("purge hard-deletes a document the resolved run's own principal created", async () => { + const { app, purged } = buildAppWithCallerResolver( + [grant(RUN_PRINCIPAL, "purge")], + () => ({ tenantId: RUN_TENANT, principalId: RUN_PRINCIPAL }), + ); + const res = await app.request( + "/api/tenants/t1/memory/documents/doc-run-owned/purge", + jsonPost({}), + ); + expect(res.status).toBe(200); + expect(purged).toEqual(["doc-run-owned"]); + }); + + test("retention-class updates a version the resolved run's own principal created", async () => { + const { app, retentionClassChanges } = buildAppWithCallerResolver( + [grant(RUN_PRINCIPAL, "forget")], + () => ({ tenantId: RUN_TENANT, principalId: RUN_PRINCIPAL }), + ); + const res = await app.request( + "/api/tenants/t1/memory/versions/ver-run-owned/retention-class", + jsonPost({ retention_class: "durable" }), + ); + expect(res.status).toBe(200); + expect(retentionClassChanges).toEqual([ + { versionId: "ver-run-owned", retentionClass: "durable" }, + ]); + }); + + test("forget is still refused for a resolved run caller that is not the creator, even with the grant", async () => { + const { app, tombstoned } = buildAppWithCallerResolver( + [grant(RUN_PRINCIPAL, "forget")], + () => ({ tenantId: RUN_TENANT, principalId: RUN_PRINCIPAL }), + ); + const res = await app.request( + "/api/tenants/t1/memory/documents/doc-alice/forget", + jsonPost({}), + ); + expect(res.status).toBe(403); + expect(tombstoned).toHaveLength(0); + }); }); describe("memory HTTP routes — resolver trust-boundary and row-fabrication contract", () => { diff --git a/src/services/retention-ownership.test.ts b/src/services/retention-ownership.test.ts new file mode 100644 index 0000000..52a8bbd --- /dev/null +++ b/src/services/retention-ownership.test.ts @@ -0,0 +1,139 @@ +import { describe, expect, it, mock } from "bun:test"; + +import type { RawSql } from "../db/client.ts"; +import { + isOwner, + resolveDocumentOwner, + resolveVersionOwner, +} from "./retention-ownership.ts"; + +const TENANT = "t1"; +const OTHER_TENANT = "t2"; + +function fakeSql(rows: Array<{ created_by_principal_id: string | null }>) { + const tag = mock(() => Promise.resolve(rows)); + return tag as unknown as RawSql; +} + +/** + * A fake sql tag that actually filters by the bound values, the way the real + * `WHERE tenant_id = ... AND document_id/id = ...` query does — so a test + * can prove tenant scoping is enforced (defense in depth, mirroring the + * destructive queries in retention.ts) rather than assume it from a + * single-tenant fixture that would pass either way. + */ +function fakeScopedSql( + rows: Array<{ + tenantId: string; + id: string; + created_by_principal_id: string | null; + }>, +) { + const tag = mock((_strings: TemplateStringsArray, ...values: unknown[]) => { + // Fails loudly (not silently) if the query stops binding exactly two + // values — e.g. if a future edit drops the tenant_id predicate, the + // query would interpolate only the id and this mock would no longer be + // exercising real tenant scoping. + if (values.length !== 2) { + throw new Error( + `expected exactly 2 bound values (tenantId, id); got ${values.length}`, + ); + } + const [tenantId, id] = values as [string, string]; + return Promise.resolve( + rows + .filter((r) => r.tenantId === tenantId && r.id === id) + .map((r) => ({ created_by_principal_id: r.created_by_principal_id })), + ); + }); + return tag as unknown as RawSql; +} + +describe("resolveDocumentOwner", () => { + it("returns exists:false for a document with no versions", async () => { + const sql = fakeSql([]); + const result = await resolveDocumentOwner(sql, { + tenantId: TENANT, + documentId: "doc-missing", + }); + expect(result).toEqual({ exists: false, ownerId: null }); + }); + + it("returns the creator of the document's first version", async () => { + const sql = fakeSql([{ created_by_principal_id: "alice" }]); + const result = await resolveDocumentOwner(sql, { + tenantId: TENANT, + documentId: "doc-1", + }); + expect(result).toEqual({ exists: true, ownerId: "alice" }); + }); + + it("treats a null creator (legacy row) as exists with no owner", async () => { + const sql = fakeSql([{ created_by_principal_id: null }]); + const result = await resolveDocumentOwner(sql, { + tenantId: TENANT, + documentId: "doc-1", + }); + expect(result).toEqual({ exists: true, ownerId: null }); + }); + + it("does not find tenant A's document under tenant B's tenantId — cross-tenant lookup is exists:false", async () => { + const sql = fakeScopedSql([ + { tenantId: TENANT, id: "doc-1", created_by_principal_id: "alice" }, + ]); + const result = await resolveDocumentOwner(sql, { + tenantId: OTHER_TENANT, + documentId: "doc-1", + }); + expect(result).toEqual({ exists: false, ownerId: null }); + }); +}); + +describe("resolveVersionOwner", () => { + it("returns exists:false for an unknown version", async () => { + const sql = fakeSql([]); + const result = await resolveVersionOwner(sql, { + tenantId: TENANT, + versionId: "ver-missing", + }); + expect(result).toEqual({ exists: false, ownerId: null }); + }); + + it("returns the version's own creator", async () => { + const sql = fakeSql([{ created_by_principal_id: "bob" }]); + const result = await resolveVersionOwner(sql, { + tenantId: TENANT, + versionId: "ver-1", + }); + expect(result).toEqual({ exists: true, ownerId: "bob" }); + }); + + it("does not find tenant A's version under tenant B's tenantId — cross-tenant lookup is exists:false", async () => { + // The security property this proves: tenant B cannot forget/purge/change + // retention on tenant A's version merely by guessing its id — the tenant + // filter in the ownership query is independent, defense-in-depth + // scoping alongside the tenant filter the destructive queries also carry. + const sql = fakeScopedSql([ + { tenantId: TENANT, id: "ver-1", created_by_principal_id: "bob" }, + ]); + const result = await resolveVersionOwner(sql, { + tenantId: OTHER_TENANT, + versionId: "ver-1", + }); + expect(result).toEqual({ exists: false, ownerId: null }); + }); +}); + +describe("isOwner", () => { + it("is false when the row does not exist, even if ownerId happens to match", () => { + expect(isOwner({ exists: false, ownerId: "alice" }, "alice")).toBe(false); + }); + + it("is false when the row exists but a different principal created it", () => { + expect(isOwner({ exists: true, ownerId: "alice" }, "mallory")).toBe(false); + }); + + it("is true only for the exact creator", () => { + expect(isOwner({ exists: true, ownerId: "alice" }, "alice")).toBe(true); + }); +}); diff --git a/src/services/retention-ownership.ts b/src/services/retention-ownership.ts new file mode 100644 index 0000000..8717413 --- /dev/null +++ b/src/services/retention-ownership.ts @@ -0,0 +1,60 @@ +/** + * Ownership gate for retention writes (CL-6288). + * + * A share grant lets a peer *see* a document via `canAccessDocument` + * (grant-tags.ts) — it must never let them forget or purge it. Retention + * routes check creator identity here, independent of the document's + * access tags, before ever calling into services/retention.ts. + * + * Raw `sql` (not the Drizzle `Db`) so unit tests can mock the query + * directly, matching the ACL-load pattern in memory.ts's search path. + */ +import type { RawSql } from "../db/client.ts"; + +export type OwnerLookup = { + /** False when no matching document/version row exists. */ + exists: boolean; + ownerId: string | null; +}; + +/** + * A document's creator is the `created_by_principal_id` of its first + * version — stable regardless of which versions are later deprecated, + * tombstoned, or added, and independent of `accessTags`. + */ +export async function resolveDocumentOwner( + sql: RawSql, + input: { tenantId: string; documentId: string }, +): Promise { + const rows = await sql<{ created_by_principal_id: string | null }[]>` + SELECT created_by_principal_id + FROM "memory"."version" + WHERE tenant_id = ${input.tenantId} + AND document_id = ${input.documentId} + ORDER BY version ASC + LIMIT 1 + `; + if (rows.length === 0) return { exists: false, ownerId: null }; + return { exists: true, ownerId: rows[0]!.created_by_principal_id }; +} + +/** A version's own creator (retention class is set per version). */ +export async function resolveVersionOwner( + sql: RawSql, + input: { tenantId: string; versionId: string }, +): Promise { + const rows = await sql<{ created_by_principal_id: string | null }[]>` + SELECT created_by_principal_id + FROM "memory"."version" + WHERE tenant_id = ${input.tenantId} + AND id = ${input.versionId} + LIMIT 1 + `; + if (rows.length === 0) return { exists: false, ownerId: null }; + return { exists: true, ownerId: rows[0]!.created_by_principal_id }; +} + +/** True only when the row exists AND its owner matches the caller. */ +export function isOwner(lookup: OwnerLookup, principalId: string): boolean { + return lookup.exists && lookup.ownerId === principalId; +} diff --git a/src/tools/index.ts b/src/tools/index.ts index 95d31fc..686a0a5 100644 --- a/src/tools/index.ts +++ b/src/tools/index.ts @@ -20,10 +20,11 @@ export { /** Re-export installer grant requirements (same as package root). */ export { + capabilityIdsForSurface, MEMORY_CAPABILITY_IDS, MEMORY_GRANT_REQUIREMENTS, type MemoryGrantRequirement, - type MemoryGrantSource, + type MemoryGrantInstallHint, type MemoryGrantSurface, } from "../grant-requirements.ts";