From 2f50459c8029c85f64fee543975de4ef27487d9b Mon Sep 17 00:00:00 2001 From: "quality-runtime[bot]" <330432719+quality-runtime[bot]@users.noreply.github.com> Date: Sat, 19 Sep 2026 17:50:08 +0200 Subject: [PATCH] feat: import standards and map controls to requirements over the API Standards arrive whole in one import and their requirements are read in the order the standard states them, filterable by whether any control is mapped and by the reference people cite. A control's requirements are replaced as a set, guarded by an ETag computed from the set's contents, and the mapping is readable from both ends. Collection queries and the import refuse unknown fields rather than silently answering a different question. Evidence, attestation and file storage follow separately. Signed-off-by: quality-runtime[bot] <330432719+quality-runtime[bot]@users.noreply.github.com> --- ARCHITECTURE.md | 4 + apps/server/app.ts | 27 +- apps/server/audit.ts | 2 +- apps/server/bun.ts | 5 +- apps/server/concurrency.test.ts | 152 ++++- apps/server/control-requirement.test.ts | 501 +++++++++++++++ apps/server/controls.ts | 246 ++++++- apps/server/documented-setup.test.ts | 22 +- apps/server/openapi.test.ts | 131 ++++ apps/server/openapi.ts | 164 ++++- apps/server/organization.ts | 8 +- apps/server/pagination.ts | 61 +- apps/server/preconditions.test.ts | 33 +- apps/server/preconditions.ts | 20 +- apps/server/privileges.test.ts | 25 + apps/server/requirements.test.ts | 398 ++++++++++++ apps/server/requirements.ts | 117 ++++ apps/server/standards.test.ts | 599 ++++++++++++++++++ apps/server/standards.ts | 307 +++++++++ apps/server/validation.ts | 6 +- docs/adr/0006-cursor-paged-collections.md | 6 +- docs/adr/0008-standards-and-requirements.md | 61 ++ docs/adr/0009-importing-a-standard.md | 43 ++ .../0010-mapping-controls-to-requirements.md | 61 ++ .../0011-reading-a-mapping-from-both-ends.md | 56 ++ docs/adr/0017-discarding-a-draft-control.md | 16 +- docs/adr/0019-conditional-writes.md | 12 +- docs/adr/0020-testing-races.md | 10 +- docs/data-model.md | 42 +- docs/deployment.md | 2 +- docs/development.md | 4 +- docs/product.md | 6 +- packages/db/schema/migrations.test.ts | 6 +- packages/db/schema/standard.ts | 7 +- 34 files changed, 3055 insertions(+), 105 deletions(-) create mode 100644 apps/server/control-requirement.test.ts create mode 100644 apps/server/requirements.test.ts create mode 100644 apps/server/requirements.ts create mode 100644 apps/server/standards.test.ts create mode 100644 apps/server/standards.ts create mode 100644 docs/adr/0008-standards-and-requirements.md create mode 100644 docs/adr/0009-importing-a-standard.md create mode 100644 docs/adr/0010-mapping-controls-to-requirements.md create mode 100644 docs/adr/0011-reading-a-mapping-from-both-ends.md diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 8762488..7d5a88b 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -48,6 +48,8 @@ Quality Runtime is being designed as a TypeScript application with these layers: email, AI, etc. ``` +Domain mutation rules still live in route handlers, which use a Hono context to resolve the tenant and attribute changes. Extract them when a non-HTTP caller needs them, into functions taking explicit inputs and actor context; PostgreSQL tenant scoping and audit recording are already available independently of Hono. + Deployment environments sit outside the core application: ```text @@ -222,6 +224,8 @@ Applied migrations must not be rewritten. **AUDIT-01 — Important changes are auditable** Material quality and compliance state changes must leave durable audit history. +Changes made by a domain mutation request are audited. What a _foreign key_ does is not: a cascade is a referential action, invisible to the application that triggered it, so removing a standard takes its requirements and removing an organization takes everything with no event for any of it. That is a known hole in this invariant rather than a reading of it — `docs/data-model.md` and [ADR 0010](docs/adr/0010-mapping-controls-to-requirements.md) name each place it bites. + **VERSION-01 — Historical state is preserved where required** Controlled or finalized records must not silently lose historical state. diff --git a/apps/server/app.ts b/apps/server/app.ts index 76c3f1e..0effe74 100644 --- a/apps/server/app.ts +++ b/apps/server/app.ts @@ -13,6 +13,8 @@ import { failure } from "./responses.ts"; import { openApiDocument, openApiPath, referencePath } from "./openapi.ts"; import { organizationContext } from "./organization.ts"; import { history } from "./history.ts"; +import { requirements } from "./requirements.ts"; +import { standards } from "./standards.ts"; /** * The HTTP surface. @@ -42,24 +44,31 @@ export function createApp({ * Where the rendered reference loads its bundle from, when not the CDN. * * Passed in rather than read from `process.env` here: this is core, and core - * must not know how a deployment keeps its configuration (ARCH-01). A Workers - * entry has no `process.env` at all — its environment arrives per request. + * must not know how a deployment keeps its configuration (ARCH-01). */ apiReferenceBundleUrl?: string; }) { const tenant = "/api/v1/organizations/:organizationId"; /** - * How much of a body a route may read, decided before any of it is read. + * How much of a body each route may read, decided before any of it is read. * * A body is read and parsed in full before a validator sees it, so the field - * bounds in a route schema do not bound the work a request costs. Chosen - * here rather than on the route: a limiter with no `Content-Length` to go on - * buffers the stream before passing it down, so a route that needs a - * different allowance has to be told apart here, ahead of this one. + * bounds in a route schema do not bound the work a request costs. One figure + * cannot serve every route: 64 KiB refuses a legitimate standard, and a + * standard's allowance would let every other route accept one. + * + * It has to be chosen here rather than on the route. A limiter with no + * `Content-Length` to go on buffers the stream before passing it down, so a + * generous limit in front of a strict one is simply the generous one. */ const tooLarge = (c: Context) => c.json(failure("payload_too_large", "The request body is too large."), 413); + const ordinary = bodyLimit({ maxSize: 64 * 1024, onError: tooLarge }); + // A standard arrives whole, with the text of every clause it states (ADR 0009). + const standardImport = bodyLimit({ maxSize: 1024 * 1024, onError: tooLarge }); + const importsAStandard = (c: Context) => + c.req.method === "POST" && /^\/api\/v1\/organizations\/[^/]+\/standards$/.test(c.req.path); return ( new Hono() @@ -93,9 +102,11 @@ export function createApp({ ...(apiReferenceBundleUrl ? { cdn: apiReferenceBundleUrl } : {}), }), ) - .use("/api/v1/*", bodyLimit({ maxSize: 64 * 1024, onError: tooLarge })) + .use("/api/v1/*", (c, next) => (importsAStandard(c) ? standardImport : ordinary)(c, next)) .use(`${tenant}/*`, organizationContext({ auth, db })) .route(tenant, controls) .route(tenant, history) + .route(tenant, standards) + .route(tenant, requirements) ); } diff --git a/apps/server/audit.ts b/apps/server/audit.ts index a3cc2b9..4b5586d 100644 --- a/apps/server/audit.ts +++ b/apps/server/audit.ts @@ -20,7 +20,7 @@ type AuditFields = schema.AuditFields; * out which record an identifier names. One source, so a new entity cannot be * recordable and unreadable. */ -export const resourceTypes = ["control"] as const; +export const resourceTypes = ["control", "standard"] as const; export type ResourceType = (typeof resourceTypes)[number]; diff --git a/apps/server/bun.ts b/apps/server/bun.ts index 2411a12..ae1718e 100644 --- a/apps/server/bun.ts +++ b/apps/server/bun.ts @@ -4,9 +4,8 @@ /** * Bun entry point: the deployment adapter for a long-lived server process. * - * Deployment-specific by design — it reads the environment and owns a - * connection pool for the life of the process. A Workers entry would sit - * beside this file and build a per-request client behind Hyperdrive instead + * Reads deployment configuration and owns the connection pool for the life of + * the process; core code depends on neither this entry point nor its environment * (ARCH-01). */ diff --git a/apps/server/concurrency.test.ts b/apps/server/concurrency.test.ts index 7b2543f..32c35f6 100644 --- a/apps/server/concurrency.test.ts +++ b/apps/server/concurrency.test.ts @@ -19,7 +19,8 @@ */ import { fileURLToPath } from "node:url"; -import { createDatabase, schema } from "@qualityruntime/db"; +import { createDatabase, schema, withOrganization } from "@qualityruntime/db"; +import { eq } from "drizzle-orm"; import { drizzle } from "drizzle-orm/node-postgres"; import { migrate } from "drizzle-orm/node-postgres/migrator"; import { Pool, type PoolClient } from "pg"; @@ -168,6 +169,36 @@ const control = async (name: string) => { return (await json<{ data: { id: string } }>(response)).data.id; }; +/** + * Two of acme's requirements, importing a standard the first time. Each test + * asks for its own rather than relying on one that ran before it, so a test + * picked out with `-t` still has what it needs. + */ +const requirements = async (): Promise<[string, string]> => { + const held = () => + admin.query<{ id: string }>( + `select "id" from "requirement" where "organization_id" = $1 order by "id" limit 2`, + [acme.organizationId], + ); + let { rows } = await held(); + if (rows.length < 2) { + const imported = await request("/standards", { + method: "POST", + body: JSON.stringify({ + name: "ISO 9001", + edition: "2015", + requirements: [ + { reference: "7.5.1", title: "General" }, + { reference: "7.5.3", title: "Documented information" }, + ], + }), + }); + expect(imported.status).toBe(201); + ({ rows } = await held()); + } + return [rows[0]!.id, rows[1]!.id]; +}; + beforeAll(async () => { if (!usable) return; @@ -351,6 +382,125 @@ describe.skipIf(!usable)("what a lock actually prevents", () => { expect(replaced).toContain("Original"); expect(new Set(replaced).size).toBe(2); }); + + it("refuses a conditional remapping of a control remapped while it waited", async () => { + // The last mutation that was last-writer-wins. A set has no `xmin`, so its + // version is its contents — and the handler already holds `for update` on + // the control while it replaces them, which is what makes comparing the + // contents safe (ADR 0019). + const id = await control("Contested requirements"); + const [first, second] = await requirements(); + + const read = (await request(`/controls/${id}/requirements`)).headers.get("etag")!; + + const response = await holding( + `select * from "control" where "id" = $1 for update`, + [id], + async ({ session, blocked }) => { + const mapping = request(`/controls/${id}/requirements`, { + method: "PUT", + body: JSON.stringify({ requirementIds: [first] }), + headers: { "if-match": read }, + }); + await blocked(); + // Somebody else maps it to the other requirement, and commits. + await session.query( + `insert into "control_requirement" ("organization_id", "control_id", "requirement_id") + values ($1, $2, $3)`, + [acme.organizationId, id, second], + ); + await session.query("commit"); + return mapping; + }, + ); + + expect(response.status).toBe(412); + // And the set the other writer left is untouched. + const { data } = await json<{ data: { id: string }[] }>( + await request(`/controls/${id}/requirements`), + ); + expect(data.map((row) => row.id)).toEqual([second]); + }); + + it("refuses a mapping to a requirement deleted while it waited, rather than failing", async () => { + // What `for key share` on the named requirements is for. Deleting the + // standard takes its requirements by cascade and holds their rows until it + // commits; the replacement waits on that lock and then finds them gone. + // With a plain read it would see them, pass the check, and meet the + // foreign key on insert instead — a 500 for what is a bad request. + const id = await control("Answering to a doomed clause"); + const imported = await request("/standards", { + method: "POST", + body: JSON.stringify({ + name: "Doomed", + edition: "1", + requirements: [{ reference: "1", title: "Soon gone" }], + }), + }); + expect(imported.status).toBe(201); + const standardId = (await json<{ data: { id: string } }>(imported)).data.id; + const { rows } = await admin.query<{ id: string }>( + `select "id" from "requirement" where "standard_id" = $1`, + [standardId], + ); + const requirementId = rows[0]!.id; + + const response = await holding( + `delete from "standard" where "id" = $1`, + [standardId], + async ({ session, blocked }) => { + const mapping = request(`/controls/${id}/requirements`, { + method: "PUT", + body: JSON.stringify({ requirementIds: [requirementId] }), + }); + await blocked(); + await session.query("commit"); + return mapping; + }, + ); + + expect(response.status).toBe(400); + const { error } = await json<{ error: { details?: { message: string }[] } }>(response); + expect(error.details?.[0]?.message).toContain(requirementId); + }); + + it.each([ + ["one snapshot", true, "same"], + ["a snapshot per statement", false, "different"], + ])("reads a collection and its version from %s", async (_case, repeatableRead, expected) => { + // Two statements in one transaction see two committed states under `read + // committed`, which is right for a write deciding from what is stored now + // and wrong for a read whose answers must agree: a page of a collection and + // the version describing that collection (ADR 0019). + const id = await control(`Snapshot ${expected}`); + const [requirementId] = await requirements(); + + const mapped = (tx: Parameters[2]>[0]) => + tx + .select({ requirementId: schema.controlRequirement.requirementId }) + .from(schema.controlRequirement) + .where(eq(schema.controlRequirement.controlId, id)); + + const [before, after] = await withOrganization( + db, + acme.organizationId, + async (tx) => { + const first = await mapped(tx); + // Somebody else maps it, and commits, between the two reads. + await admin.query( + `insert into "control_requirement" ("organization_id", "control_id", "requirement_id") + values ($1, $2, $3)`, + [acme.organizationId, id, requirementId], + ); + return [first, await mapped(tx)] as const; + }, + { repeatableRead }, + ); + + expect(before).toEqual([]); + if (expected === "same") expect(after).toEqual(before); + else expect(after).toHaveLength(1); + }); }); describe.skipIf(!usable)("tenants sharing a connection pool", () => { diff --git a/apps/server/control-requirement.test.ts b/apps/server/control-requirement.test.ts new file mode 100644 index 0000000..985b74a --- /dev/null +++ b/apps/server/control-requirement.test.ts @@ -0,0 +1,501 @@ +// SPDX-FileCopyrightText: 2026 Quality Runtime contributors +// SPDX-License-Identifier: Apache-2.0 + +/** + * Mapping controls to the requirements they answer to. + * + * `PUT` replaces the whole set, so most of what is checked here is that saying + * the same thing twice changes nothing — in the rows, and in the history. + * Requests run as a non-superuser role that owns the tables, so the row-level + * security policies are in force as they are in a deployment. + */ + +import { fileURLToPath } from "node:url"; +import { PGlite } from "@electric-sql/pglite"; +import { schema, withOrganization } from "@qualityruntime/db"; +import { and, eq } from "drizzle-orm"; +import { drizzle } from "drizzle-orm/pglite"; +import { migrate } from "drizzle-orm/pglite/migrator"; +import { beforeAll, describe, expect, it } from "vite-plus/test"; +import { createApp } from "./app.ts"; +import { createAuth } from "./auth.ts"; + +const migrationsFolder = fileURLToPath(new URL("../../packages/db/migrations", import.meta.url)); + +const createTestDatabase = (client: PGlite) => drizzle({ client, schema, casing: "snake_case" }); + +let db: ReturnType; +let app: ReturnType; + +type Tenant = { cookie: string; organizationId: string }; +let acme: Tenant; +let globex: Tenant; + +/** Acme's requirements, in the order their standard states them. */ +let requirements: { id: string; reference: string }[]; +let theirRequirement: string; + +const json = async (response: Response): Promise => (await response.json()) as T; + +type Page = { data: T[]; nextCursor: string | null }; +type Failure = { error: { code: string; details?: { path: string; message: string }[] } }; + +const request = ( + tenant: Tenant, + path: string, + init: Omit & { headers?: Record } = {}, +) => + app.request(`/api/v1/organizations/${tenant.organizationId}${path}`, { + ...init, + // Merged, not replaced: a caller's own headers are the point of + // passing them, and dropping them silently makes a test pass for + // the wrong reason. + headers: { + cookie: tenant.cookie, + ...(init.body ? { "content-type": "application/json" } : {}), + ...init.headers, + }, + }); + +async function newControl(tenant: Tenant, name: string): Promise { + const response = await request(tenant, "/controls", { + method: "POST", + body: JSON.stringify({ name }), + }); + expect(response.status).toBe(201); + return (await json<{ data: { id: string } }>(response)).data.id; +} + +const setRequirements = (tenant: Tenant, controlId: string, requirementIds: string[]) => + request(tenant, `/controls/${controlId}/requirements`, { + method: "PUT", + body: JSON.stringify({ requirementIds }), + }); + +const listRequirements = async (tenant: Tenant, controlId: string, query = "") => + json>( + await request(tenant, `/controls/${controlId}/requirements${query}`), + ); + +/** Every audit event recorded against a control. */ +const historyOf = (tenant: Tenant, controlId: string) => + withOrganization(db, tenant.organizationId, (tx) => + tx + .select() + .from(schema.auditEvent) + .where( + and( + eq(schema.auditEvent.resourceType, "control"), + eq(schema.auditEvent.resourceId, controlId), + ), + ), + ); + +beforeAll(async () => { + const client = new PGlite(); + db = createTestDatabase(client); + await migrate(db, { migrationsFolder }); + app = createApp({ + db, + auth: createAuth(db, { + baseURL: "http://localhost", + secret: "test-secret-of-at-least-32-characters", + }), + }); + + const tenant = async (slug: string): Promise => { + const signedUp = await app.request("/api/auth/sign-up/email", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "Ada", + email: `${slug}@example.test`, + password: "correct horse", + }), + }); + expect(signedUp.status).toBe(200); + const cookie = signedUp.headers + .getSetCookie() + .map((value) => value.split(";", 1)[0]) + .join("; "); + const created = await app.request("/api/auth/organization/create", { + method: "POST", + headers: { "content-type": "application/json", cookie }, + body: JSON.stringify({ name: slug, slug }), + }); + expect(created.status).toBe(200); + return { cookie, organizationId: (await json<{ id: string }>(created)).id }; + }; + + acme = await tenant("acme"); + globex = await tenant("globex"); + + await client.exec(` + create role qualityruntime_app nosuperuser nobypassrls; + grant all on all tables in schema public to qualityruntime_app; + alter table "control" owner to qualityruntime_app; + alter table "audit_event" owner to qualityruntime_app; + alter table "standard" owner to qualityruntime_app; + alter table "requirement" owner to qualityruntime_app; + alter table "control_requirement" owner to qualityruntime_app; + set role qualityruntime_app; + `); + + const importStandard = async (tenant: Tenant, count: number) => { + const response = await request(tenant, "/standards", { + method: "POST", + body: JSON.stringify({ + name: "ISO 9001", + edition: "2015", + requirements: Array.from({ length: count }, (_, index) => ({ + reference: `7.${index + 1}`, + title: `Clause ${index + 1}`, + })), + }), + }); + expect(response.status).toBe(201); + const { id } = (await json<{ data: { id: string } }>(response)).data; + const { data } = await listStandardRequirements(tenant, id); + return data; + }; + const listStandardRequirements = async (tenant: Tenant, standardId: string) => + json>( + await request(tenant, `/standards/${standardId}/requirements?limit=100`), + ); + + requirements = await importStandard(acme, 5); + theirRequirement = (await importStandard(globex, 1))[0]!.id; +}, 60_000); + +describe("setting what a control answers to", () => { + it("returns the set it was given, sorted", async () => { + const control = await newControl(acme, "Access review"); + const ids = [requirements[2]!.id, requirements[0]!.id]; + + const response = await setRequirements(acme, control, ids); + + expect(response.status).toBe(200); + const { data } = await json<{ data: { requirementIds: string[] } }>(response); + expect(data.requirementIds).toEqual([...ids].sort()); + }); + + it("maps a retired control too, as it may be renamed", async () => { + // Deliberate: retiring freezes nothing else on a control, so freezing only + // its mappings would be a rule of its own (ADR 0010). + const control = await newControl(acme, "Retired"); + for (const status of ["active", "retired"]) { + const changed = await request(acme, `/controls/${control}`, { + method: "PATCH", + body: JSON.stringify({ status }), + }); + expect(changed.status).toBe(200); + } + + const response = await setRequirements(acme, control, [requirements[0]!.id]); + + expect(response.status).toBe(200); + }); + + it("replaces the set rather than adding to it", async () => { + const control = await newControl(acme, "Replaced"); + await setRequirements(acme, control, [requirements[0]!.id, requirements[1]!.id]); + + await setRequirements(acme, control, [requirements[1]!.id, requirements[2]!.id]); + + const { data } = await listRequirements(acme, control); + expect(data.map((row) => row.reference)).toEqual(["7.2", "7.3"]); + }); + + it("clears the set when given an empty array", async () => { + const control = await newControl(acme, "Cleared"); + await setRequirements(acme, control, [requirements[0]!.id]); + + expect((await setRequirements(acme, control, [])).status).toBe(200); + + expect((await listRequirements(acme, control)).data).toEqual([]); + }); + + it("treats a repeated identifier as one", async () => { + const control = await newControl(acme, "Repeated"); + + const response = await setRequirements(acme, control, [ + requirements[0]!.id, + requirements[0]!.id, + ]); + + expect(response.status).toBe(200); + expect( + (await json<{ data: { requirementIds: string[] } }>(response)).data.requirementIds, + ).toEqual([requirements[0]!.id]); + expect((await listRequirements(acme, control)).data).toHaveLength(1); + }); + + it("changes nothing when asked for what is already there", async () => { + const control = await newControl(acme, "Idempotent"); + const ids = [requirements[0]!.id, requirements[1]!.id]; + await setRequirements(acme, control, ids); + const before = await historyOf(acme, control); + + expect((await setRequirements(acme, control, [...ids].reverse())).status).toBe(200); + + // Same set, said differently: no rows moved, and nothing to record. + expect(await historyOf(acme, control)).toEqual(before); + }); + + it("records the change on the control, as one event", async () => { + const control = await newControl(acme, "Audited"); + + await setRequirements(acme, control, [requirements[0]!.id]); + await setRequirements(acme, control, [requirements[1]!.id]); + + const history = await historyOf(acme, control); + expect(history.map((event) => event.action)).toEqual(["created", "updated", "updated"]); + expect(history.at(-1)).toMatchObject({ + before: { requirementIds: [requirements[0]!.id] }, + after: { requirementIds: [requirements[1]!.id] }, + }); + }); +}); + +describe("requirements a control cannot answer to", () => { + it("names a requirement that does not exist", async () => { + const control = await newControl(acme, "Unknown"); + + const response = await setRequirements(acme, control, [ + requirements[0]!.id, + "req_0000000000000000", + ]); + + expect(response.status).toBe(400); + const { error } = await json(response); + expect(error.code).toBe("invalid_request"); + expect(error.details?.[0]?.message).toContain("req_0000000000000000"); + }); + + it("refuses another organization's requirement the same way", async () => { + // The foreign key would refuse it too, as a 500; and the answer must not + // distinguish a requirement that is elsewhere from one that is nowhere. + const control = await newControl(acme, "Cross tenant"); + + const response = await setRequirements(acme, control, [theirRequirement]); + const absent = await setRequirements(acme, control, ["req_0000000000000000"]); + + expect(response.status).toBe(400); + expect((await json(response)).error.code).toBe( + (await json(absent)).error.code, + ); + }); + + it("writes nothing when one requirement in the set is unknown", async () => { + const control = await newControl(acme, "All or nothing"); + await setRequirements(acme, control, [requirements[0]!.id]); + + await setRequirements(acme, control, [requirements[1]!.id, "req_0000000000000000"]); + + const { data } = await listRequirements(acme, control); + expect(data.map((row) => row.reference)).toEqual(["7.1"]); + }); + + it.each([ + ["a requirement id of the wrong shape", ["not-an-id"]], + ["an id carrying another table's prefix", ["ctl_v1stgxr8z5jdhi6b"]], + ])("refuses %s", async (_case, ids) => { + const control = await newControl(acme, `Bad ${ids[0]}`); + + expect((await setRequirements(acme, control, ids)).status).toBe(400); + }); + + it("answers 404 for a control that is not there", async () => { + const response = await setRequirements(acme, "ctl_0000000000000000", []); + + expect(response.status).toBe(404); + }); +}); + +describe("reading what a control answers to", () => { + it("lists them in the order their standard states", async () => { + const control = await newControl(acme, "Ordered"); + // Given out of order, and out of order lexically: 7.10 before 7.9. + await setRequirements(acme, control, [ + requirements[4]!.id, + requirements[0]!.id, + requirements[2]!.id, + ]); + + const { data } = await listRequirements(acme, control); + + expect(data.map((row) => row.reference)).toEqual(["7.1", "7.3", "7.5"]); + }); + + it("pages like every other collection", async () => { + const control = await newControl(acme, "Paged"); + await setRequirements( + acme, + control, + requirements.map((row) => row.id), + ); + + const first = await listRequirements(acme, control, "?limit=2"); + expect(first.nextCursor).not.toBeNull(); + const second = await listRequirements( + acme, + control, + `?limit=2&cursor=${encodeURIComponent(first.nextCursor!)}`, + ); + + expect(first.data.map((row) => row.reference)).toEqual(["7.1", "7.2"]); + expect(second.data.map((row) => row.reference)).toEqual(["7.3", "7.4"]); + }); + + it("carries the standard each requirement comes from", async () => { + const control = await newControl(acme, "Grouped"); + await setRequirements(acme, control, [requirements[0]!.id]); + + const { data } = await listRequirements(acme, control); + + expect(data[0]?.standardId).toMatch(/^std_[0-9a-z]{16}$/); + }); + + it("answers 404 for another organization's control", async () => { + const theirs = await newControl(globex, "Globex only"); + + expect((await request(acme, `/controls/${theirs}/requirements`)).status).toBe(404); + }); +}); + +describe("what deletion does to a mapping", () => { + it("forgets the link when the requirement's standard goes", async () => { + // Nothing deletes a standard over HTTP yet, so this is the database's + // cascade rather than a route — but a control must not be left answering + // to a requirement that is gone. + const control = await newControl(acme, "Orphaned"); + const [standard] = await withOrganization(db, acme.organizationId, (tx) => + tx + .insert(schema.standard) + .values({ organizationId: acme.organizationId, name: "Temporary", edition: "1" }) + .returning(), + ); + const [requirement] = await withOrganization(db, acme.organizationId, (tx) => + tx + .insert(schema.requirement) + .values({ + organizationId: acme.organizationId, + standardId: standard!.id, + reference: "1", + title: "Only clause", + position: 1, + }) + .returning(), + ); + await setRequirements(acme, control, [requirement!.id]); + expect((await listRequirements(acme, control)).data).toHaveLength(1); + + await withOrganization(db, acme.organizationId, (tx) => + tx.delete(schema.standard).where(eq(schema.standard.id, standard!.id)), + ); + + expect((await listRequirements(acme, control)).data).toEqual([]); + }); +}); + +describe("setting only the requirements you read", () => { + /** The current version of a control's mapping set, as a client obtains it. */ + const tagOf = async (controlId: string) => { + const response = await request(acme, `/controls/${controlId}/requirements`); + expect(response.status).toBe(200); + return response.headers.get("etag")!; + }; + + const setWith = (controlId: string, tag: string | undefined, requirementIds: string[]) => + request(acme, `/controls/${controlId}/requirements`, { + method: "PUT", + body: JSON.stringify({ requirementIds }), + ...(tag ? { headers: { "if-match": tag } } : {}), + }); + + it("serves a version that follows the set, not the order it is written in", async () => { + // A set is its members: the same two requirements are the same set however + // a client lists them, and a tag that disagreed would refuse a write that + // changed nothing (ADR 0019). + const control = await newControl(acme, "Versioned mapping"); + const pair = [requirements[0]!.id, requirements[1]!.id]; + + expect((await setWith(control, undefined, pair)).status).toBe(200); + const forward = await tagOf(control); + expect((await setWith(control, undefined, [...pair].reverse())).status).toBe(200); + + expect(await tagOf(control)).toBe(forward); + expect(forward).toMatch(/^"[0-9a-f]{64}"$/); + }); + + it("changes the version when the set changes, and not otherwise", async () => { + const control = await newControl(acme, "Changing mapping"); + expect((await setWith(control, undefined, [requirements[0]!.id])).status).toBe(200); + const one = await tagOf(control); + + expect((await setWith(control, undefined, [requirements[1]!.id])).status).toBe(200); + const other = await tagOf(control); + expect(other).not.toBe(one); + + // An empty set is a set, and has a version of its own. + expect((await setWith(control, undefined, [])).status).toBe(200); + expect(await tagOf(control)).not.toBe(other); + }); + + it("refuses a replacement against a set that has moved", async () => { + // What last-writer-wins looked like here: the second write removed a + // mapping the first had added, and neither client learned anything + // (ADR 0010). + const control = await newControl(acme, "Contested mapping"); + const read = await tagOf(control); + expect((await setWith(control, read, [requirements[0]!.id])).status).toBe(200); + + const late = await setWith(control, read, [requirements[1]!.id]); + + expect(late.status).toBe(412); + expect((await json(late)).error.code).toBe("precondition_failed"); + const { data } = await listRequirements(acme, control); + expect(data.map((row) => row.id)).toEqual([requirements[0]!.id]); + }); + + it("allows a replacement against the set just read, and answers with the new version", async () => { + const control = await newControl(acme, "Agreed mapping"); + + const response = await setWith(control, await tagOf(control), [requirements[0]!.id]); + + expect(response.status).toBe(200); + expect(response.headers.get("etag")).toBe(await tagOf(control)); + }); + + it("carries the whole set's version on every page of it", async () => { + // The page is a window on the set; the version is the set's. A client that + // paged through and then wrote must be writing against what it read. + const control = await newControl(acme, "Paged mapping"); + expect( + ( + await setWith( + control, + undefined, + requirements.slice(0, 2).map((row) => row.id), + ) + ).status, + ).toBe(200); + + const first = await request(acme, `/controls/${control}/requirements?limit=1`); + const tag = first.headers.get("etag"); + const { nextCursor } = await json>(first); + expect(nextCursor).not.toBeNull(); + const second = await request( + acme, + `/controls/${control}/requirements?limit=1&cursor=${encodeURIComponent(nextCursor!)}`, + ); + + expect(second.headers.get("etag")).toBe(tag); + }); + + it("goes on working for a client that asks for no guarantee", async () => { + const control = await newControl(acme, "Unconditional mapping"); + + expect((await setWith(control, undefined, [requirements[0]!.id])).status).toBe(200); + }); +}); diff --git a/apps/server/controls.ts b/apps/server/controls.ts index 2fdf516..8b74d1f 100644 --- a/apps/server/controls.ts +++ b/apps/server/controls.ts @@ -12,16 +12,17 @@ * ordinary 404 is the right answer for it. */ -import { idPattern, schema } from "@qualityruntime/db"; -import { eq, getTableColumns } from "drizzle-orm"; +import { idPattern, schema, type TenantTransaction } from "@qualityruntime/db"; +import { and, eq, getTableColumns, inArray } from "drizzle-orm"; import { type Context, Hono } from "hono"; import { createMiddleware } from "hono/factory"; import { z } from "zod"; import { diffFields, fieldsOf } from "./audit.ts"; -import { entityTag, ifMatch, rowVersion, withoutVersion } from "./preconditions.ts"; +import { entityTag, ifMatch, rowVersion, setVersion, withoutVersion } from "./preconditions.ts"; import { failure } from "./responses.ts"; import type { OrganizationEnv } from "./organization.ts"; import { + asStated, collectionQuery, cursorAt, newestFirst, @@ -29,7 +30,7 @@ import { page, rowsAfter, } from "./pagination.ts"; -import { jsonBody, prose, queryParams, words } from "./validation.ts"; +import { jsonBody, prose, queryParams, rejection, words } from "./validation.ts"; type ControlStatus = (typeof schema.controlStatuses)[number]; @@ -63,8 +64,8 @@ export const updateBody = z * The status changes the lifecycle in `docs/data-model.md` allows: one way. * * A control in effect is withdrawn deliberately rather than quietly unpublished, - * and a withdrawn one stays withdrawn — what replaces it is a new control, so - * the one evidence was recorded against keeps meaning what it meant. Setting + * and a withdrawn one stays withdrawn — what replaces it is a new control, and + * the retired one stays a distinct record of what was in effect. Setting * the status it already has is a no-op and always allowed. The database holds * the part that matters: nothing that took effect can be a draft again. */ @@ -79,6 +80,29 @@ const audited = ["name", "description", "status"] as const; const version = rowVersion(schema.control); +/** + * Every requirement a control is mapped to, in one stable order. + * + * Read whole rather than by the page: the page is a window on the set, and the + * version has to be the set's (ADR 0019). + */ +const mappedTo = async (tx: TenantTransaction, controlId: string): Promise => { + const rows = await tx + .select({ requirementId: schema.controlRequirement.requirementId }) + .from(schema.controlRequirement) + .where(eq(schema.controlRequirement.controlId, controlId)); + return rows.map((row) => row.requirementId).sort(); +}; + +/** The one answer for an `If-Match` that no longer names these requirements. */ +const staleRequirements = (c: Context) => + c.json( + failure("precondition_failed", "The control's requirements changed since they were read.", [ + { path: "", message: "Read them again, and decide against what they now are." }, + ]), + 412, + ); + /** The one answer for an `If-Match` that no longer names this control. */ const staleControl = (c: Context) => c.json( @@ -90,9 +114,57 @@ const staleControl = (c: Context) => const isControlId = new RegExp(idPattern("control")); +/** + * The requirements of `ids` that are here, held against deletion until commit. + * + * Named rather than left to the foreign key, which reports a requirement in + * another organization and one that does not exist the same way — as a 500. + * Reading them is not enough on its own: under `read committed` another + * transaction may delete one, or the standard stating it, between this and the + * insert, and the foreign key would then raise. `for key share` is the lock + * that says so — it blocks a delete without blocking anyone else reading the + * same rows. + */ +const lockRequirements = async (tx: TenantTransaction, ids: string[]) => + ids.length === 0 + ? [] + : tx + .select({ id: schema.requirement.id }) + .from(schema.requirement) + .where(inArray(schema.requirement.id, ids)) + .for("key share"); + +/** + * The requirements a control answers to, as a client sets them. + * + * A set, not a sequence: order carries no meaning, and a repeated identifier + * says nothing the first did not. + */ +export const controlRequirementsBody = z.object({ + requirementIds: z + .array(z.string().regex(new RegExp(idPattern("requirement")))) + .max(500) + .meta({ + description: + "Replaces the whole set. At most 500 entries; a repeated identifier is ignored when " + + "forming the set. An empty array clears it.", + }), +}); + +/** + * A control's requirements, in the order their standards state them. + * + * Requirements from different standards interleave, because a position is only + * meaningful within its own standard. Each carries its `standardId`, which is + * what a client groups by; ordering across standards is a decision to make when + * something needs one. + */ +export const controlRequirementsOrder = (controlId: string) => + asStated(`control-requirements/${controlId}`, schema.requirement.position, schema.requirement.id); + /** Controls and their history are both records of what has happened. */ export const controlOrder = newestFirst("controls", schema.control.createdAt, schema.control.id); -/** A control, as a client sees it. Strict, so a new column cannot slip out. */ +/** A control response contract; tests reject undeclared fields. */ export const controlResponse = z.strictObject({ id: z.string(), organizationId: z.string(), @@ -109,6 +181,11 @@ export const controlResponse = z.strictObject({ updatedAt: z.iso.datetime(), }); +/** The requirements a control is mapped to, as a client sees them. */ +export const controlRequirementsResponse = z.strictObject({ + requirementIds: z.array(z.string()), +}); + /** * Stops an id that could not name a control before it reaches PostgreSQL. * @@ -187,6 +264,149 @@ export const controls = new Hono() return c.json({ data: withoutVersion(row) }); }) + .get("/controls/:controlId/requirements", knownControlId, async (c) => { + const controlId = c.req.param("controlId"); + const ordering = controlRequirementsOrder(controlId); + const query = collectionQuery(ordering).safeParse(c.req.query()); + if (!query.success) return c.json(rejection("query", query.error), 400); + const { limit, cursor } = query.data; + + const found = await c.var.withOrganization( + async (tx) => { + const [control] = await tx + .select({ id: schema.control.id }) + .from(schema.control) + .where(eq(schema.control.id, controlId)); + if (!control) return undefined; + + // The whole set's version travels with every page of it, so a client that + // paged through and then wrote is writing against what it read. Read from + // the same snapshot as the page below — under `read committed` these are + // two statements and can see two different committed sets, which would + // hand a client a version for membership it was not shown. + const mapped = await mappedTo(tx, controlId); + return tx + .select({ ...getTableColumns(schema.requirement), cursorAt: cursorAt(ordering) }) + .from(schema.controlRequirement) + .innerJoin( + schema.requirement, + eq(schema.requirement.id, schema.controlRequirement.requirementId), + ) + .where( + and( + eq(schema.controlRequirement.controlId, controlId), + cursor ? rowsAfter(ordering, cursor) : undefined, + ), + ) + .orderBy(...orderedBy(ordering)) + .limit(limit + 1) + .then((rows) => ({ rows, mapped })); + }, + { repeatableRead: true }, + ); + if (!found) return c.json(failure("not_found", "No such control."), 404); + + const { rows, nextCursor } = page(found.rows, limit, ordering); + c.header("etag", entityTag({ version: setVersion(found.mapped) })); + return c.json({ data: rows, nextCursor }); + }) + + .put( + "/controls/:controlId/requirements", + knownControlId, + jsonBody(controlRequirementsBody), + async (c) => { + const controlId = c.req.param("controlId"); + // A set: the same requirement named twice is named once. + const wanted = [...new Set(c.req.valid("json").requirementIds)]; + + const result = await c.var.withOrganization(async (tx) => { + const [control] = await tx + .select({ id: schema.control.id }) + .from(schema.control) + .where(eq(schema.control.id, controlId)) + .for("update"); + if (!control) return { outcome: "missing" } as const; + + const known = await lockRequirements(tx, wanted); + const unknown = wanted.filter((id) => !known.some((row) => row.id === id)); + if (unknown.length > 0) return { outcome: "unknown", unknown } as const; + + const before = await mappedTo(tx, controlId); + + // Compared under the lock taken above, so the set cannot change between + // the test and the replacement. A set has no `xmin`, so its contents + // are its version (ADR 0019). + if ( + ifMatch(c.req.header("if-match"), entityTag({ version: setVersion(before) })) === "failed" + ) { + return { outcome: "stale" } as const; + } + + const after = [...wanted].sort(); + const removed = before.filter((id) => !wanted.includes(id)); + const added = wanted.filter((id) => !before.includes(id)); + + if (removed.length > 0) { + await tx + .delete(schema.controlRequirement) + .where( + and( + eq(schema.controlRequirement.controlId, controlId), + inArray(schema.controlRequirement.requirementId, removed), + ), + ); + } + if (added.length > 0) { + await tx.insert(schema.controlRequirement).values( + added.map((requirementId) => ({ + organizationId: c.var.member.organizationId, + controlId, + requirementId, + })), + ); + } + + // A mapping change is a change to the control, not an event about a link: + // the link has no life of its own (ADR 0010). Nothing changed, nothing + // recorded — the request is idempotent all the way down. + if (added.length > 0 || removed.length > 0) { + await c.var.audit(tx, { + action: "updated", + resourceType: "control", + resourceId: controlId, + before: { requirementIds: before }, + after: { requirementIds: after }, + }); + } + + return { outcome: "set", requirementIds: after } as const; + }); + + if (result.outcome === "missing") { + return c.json(failure("not_found", "No such control."), 404); + } + if (result.outcome === "stale") return staleRequirements(c); + if (result.outcome === "unknown") { + return c.json( + failure( + "invalid_request", + "Some requirements are not here.", + result.unknown.map((id) => ({ + path: "requirementIds", + message: `No such requirement: ${id}.`, + })), + ), + 400, + ); + } + // The new version, so a client can make a second change without reading + // the whole set again. + c.header("etag", entityTag({ version: setVersion(result.requirementIds) })); + return c.json({ data: { requirementIds: result.requirementIds } }); + }, + ) + .patch("/controls/:controlId", knownControlId, jsonBody(updateBody), async (c) => { const body = c.req.valid("json"); const controlId = c.req.param("controlId"); @@ -277,9 +497,11 @@ export const controls = new Hono() const controlId = c.req.param("controlId"); // Only a draft. A control that was in effect is part of the record and is - // retired rather than removed; a draft claims nothing and was never relied - // on, so there is nothing about it to keep (ADR 0017). The policy says the - // same thing, so this is the courteous answer rather than the enforcement. + // retired rather than removed; a draft claims nothing, and retiring it would + // record that it had been in effect (ADR 0017). What it carries is not + // lost: evidence refuses the delete, and the audit history stays. The policy + // says the same thing, so this is the courteous answer rather than the + // enforcement. const result = await c.var.withOrganization(async (tx) => { // Locked for the same reason `PATCH` locks: the decision is made from // what is stored, and a concurrent patch must not activate it in between. @@ -320,7 +542,7 @@ export const controls = new Hono() .returning({ id: schema.control.id }); // The lock above is what makes this impossible: a locked draft is one the // policy admits. A delete matching nothing must still not be answered - // 204, and there is no honest 4xx for it. Evidence checks the same thing. + // 204, and there is no honest 4xx for it. if (!removed) throw new Error(`Control ${controlId} was locked as a draft but not deleted.`); // Written after the row is gone, and it survives it: `resource_id` is a @@ -340,7 +562,7 @@ export const controls = new Hono() } if (result.outcome === "in_effect") { const advice = { - active: "Retire it instead; a control that was relied on is part of the record.", + active: "Retire it instead; a control that has been in effect is part of the record.", retired: "A retired control is part of the record and is kept.", }[result.status]; return c.json( diff --git a/apps/server/documented-setup.test.ts b/apps/server/documented-setup.test.ts index 066a3eb..ab81694 100644 --- a/apps/server/documented-setup.test.ts +++ b/apps/server/documented-setup.test.ts @@ -238,7 +238,7 @@ describe.each(documents)("the setup in $path", ({ path, heading }) => { const request = (suffix: string, init: Request = {}) => app.request(`${base}${suffix}`, { ...init, headers: { cookie, ...init.headers } }); - // Every route there is, on privileges the document alone produced. + // The loop so far, on privileges the document alone produced. const control = await request("/controls", asJson({ name: "Access review" })); expect(control.status).toBe(201); const controlId = (await json<{ data: { id: string } }>(control)).data.id; @@ -253,6 +253,26 @@ describe.each(documents)("the setup in $path", ({ path, heading }) => { const history = await request(`/history?resource=${controlId}`); expect(history.status).toBe(200); + const standard = await request( + "/standards", + asJson({ + name: "ISO 9001", + edition: "2015", + requirements: [{ reference: "7.5.3", title: "Documented information" }], + }), + ); + expect(standard.status).toBe(201); + const standardId = (await json<{ data: { id: string } }>(standard)).data.id; + const requirements = await request(`/standards/${standardId}/requirements`); + const requirementId = (await json<{ data: { id: string }[] }>(requirements)).data[0]!.id; + + const mapped = await request(`/controls/${controlId}/requirements`, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ requirementIds: [requirementId] }), + }); + expect(mapped.status).toBe(200); + // The refusals the revoke block exists for. const refused = async (statement: string) => { const failure = await client.exec(statement).then( diff --git a/apps/server/openapi.test.ts b/apps/server/openapi.test.ts index 3b60fa3..9a3e10f 100644 --- a/apps/server/openapi.test.ts +++ b/apps/server/openapi.test.ts @@ -198,6 +198,17 @@ describe("the document", () => { expect(cursor?.schema.type).toBe("string"); }); + + it("documents looking a requirement up by the reference people cite", () => { + const document = openApiDocument(sessionCookieName(auth)); + const list = document.paths[ + "/api/v1/organizations/{organizationId}/standards/{standardId}/requirements" + ]!.get as { parameters: { name: string; required?: boolean; schema: { type: string } }[] }; + const reference = list.parameters.find((parameter) => parameter.name === "reference"); + + expect(reference?.schema.type).toBe("string"); + expect(reference?.required).not.toBe(true); + }); }); describe("the requests it promises to accept", () => { @@ -257,6 +268,8 @@ describe("the requests it promises to accept", () => { it.each([ ["get", "/api/v1/organizations/{organizationId}/controls/{controlId}"], ["patch", "/api/v1/organizations/{organizationId}/controls/{controlId}"], + ["get", "/api/v1/organizations/{organizationId}/controls/{controlId}/requirements"], + ["put", "/api/v1/organizations/{organizationId}/controls/{controlId}/requirements"], ])("says that %s %s answers with an ETag", (method, path) => { // A document that asks for `If-Match` and never says where the tag comes // from describes half a contract (ADR 0019). @@ -405,3 +418,121 @@ describe("the responses it promises", () => { await conformsToDocument(document, "post", controls, response); }); }); + +describe("the responses it promises for standards and mappings", () => { + const tenant = "/api/v1/organizations/{organizationId}"; + let document: Document; + + const at = ( + path: string, + init: Omit & { headers?: Record } = {}, + ) => + app.request(`/api/v1/organizations/${acme.organizationId}${path}`, { + ...init, + headers: { + cookie: acme.cookie, + ...(init.body ? { "content-type": "application/json" } : {}), + ...init.headers, + }, + }); + + /** One of each shape: the standard, a requirement, and a mapped control. */ + let standardId: string; + let requirementId: string; + let controlId: string; + + beforeAll(async () => { + document = openApiDocument(sessionCookieName(auth)); + const imported = await at("/standards", { + method: "POST", + body: JSON.stringify({ + name: "ISO 9001", + edition: "2015", + requirements: [{ reference: "7.5.3", title: "Documented information" }], + }), + }); + standardId = (await json<{ data: { id: string } }>(imported)).data.id; + const listed = await at(`/standards/${standardId}/requirements`); + requirementId = (await json<{ data: { id: string }[] }>(listed)).data[0]!.id; + controlId = (await create("Mapped")).id; + const mapped = await at(`/controls/${controlId}/requirements`, { + method: "PUT", + body: JSON.stringify({ requirementIds: [requirementId] }), + }); + expect(mapped.status).toBe(200); + }); + + it.each([ + ["get", "/standards", `${tenant}/standards`], + ["get", "/standards/{standardId}", `${tenant}/standards/{standardId}`], + [ + "get", + "/standards/{standardId}/requirements", + `${tenant}/standards/{standardId}/requirements`, + ], + ["get", "/requirements/{requirementId}", `${tenant}/requirements/{requirementId}`], + [ + "get", + "/requirements/{requirementId}/controls", + `${tenant}/requirements/{requirementId}/controls`, + ], + ["get", "/controls/{controlId}/requirements", `${tenant}/controls/{controlId}/requirements`], + ])("describes %s %s", async (method, path, documented) => { + const response = await at( + path + .replace("{standardId}", standardId) + .replace("{requirementId}", requirementId) + .replace("{controlId}", controlId), + ); + + expect(response.status).toBe(200); + const body = (await response.clone().json()) as { data: unknown }; + // A collection must carry an item, or only the envelope is checked. + if (Array.isArray(body.data)) expect(body.data.length).toBeGreaterThan(0); + await conformsToDocument(document, method, documented, response); + }); + + it("describes an imported standard", async () => { + const response = await at("/standards", { + method: "POST", + body: JSON.stringify({ + name: "ISO 9001", + edition: "2026", + requirements: [{ reference: "7.5.3", title: "Documented information" }], + }), + }); + + expect(response.status).toBe(201); + await conformsToDocument(document, "post", `${tenant}/standards`, response); + }); + + it("tells a client that the mapping tag versions the whole set, not the page", () => { + const list = document.paths[`${tenant}/controls/{controlId}/requirements`]!.get as { + responses: { "200": { headers: { ETag: { description: string } } } }; + }; + + expect(list.responses["200"].headers.ETag.description).toMatch(/every page/); + + const put = document.paths[`${tenant}/controls/{controlId}/requirements`]!.put as { + parameters: { name: string; description: string }[]; + }; + const ifMatch = put.parameters.find((parameter) => parameter.name === "If-Match"); + expect(ifMatch?.description).toMatch(/every page/); + expect(ifMatch?.description).toContain("`*`"); + }); + + it("describes a replaced mapping", async () => { + const response = await at(`/controls/${controlId}/requirements`, { + method: "PUT", + body: JSON.stringify({ requirementIds: [requirementId] }), + }); + + expect(response.status).toBe(200); + await conformsToDocument( + document, + "put", + `${tenant}/controls/{controlId}/requirements`, + response, + ); + }); +}); diff --git a/apps/server/openapi.ts b/apps/server/openapi.ts index d890de1..ef1c178 100644 --- a/apps/server/openapi.ts +++ b/apps/server/openapi.ts @@ -14,8 +14,24 @@ import { auditEventResponse, historyQuery } from "./history.ts"; import { idPattern, type IdType } from "@qualityruntime/db"; import { z } from "zod"; -import { controlOrder, controlResponse, createBody, updateBody } from "./controls.ts"; +import { + controlOrder, + controlResponse, + controlRequirementsBody, + controlRequirementsOrder, + controlRequirementsResponse, + createBody, + updateBody, +} from "./controls.ts"; import { collectionQuery } from "./pagination.ts"; +import { requirementControlsOrder, requirementResponse } from "./requirements.ts"; +import { + importBody, + requirementOrder, + requirementsQuery, + standardOrder, + standardResponse, +} from "./standards.ts"; import { collection, failureResponse, single } from "./responses.ts"; /** OpenAPI 3.1 is JSON Schema draft 2020-12, which is what Zod emits. */ @@ -46,16 +62,24 @@ const fails = (description: string) => responds(description, failureResponse); * Declared where one is actually served: a document that asks for `If-Match` * and never says where the tag comes from describes half a contract (ADR 0019). */ -const versioned = (description: string, schema: z.ZodType) => ({ +const versioned = ( + description: string, + schema: z.ZodType, + tag = "The version of what was read. Quote it back in `If-Match` to change only that.", +) => ({ ...responds(description, schema), - headers: { - ETag: { - description: "The record's version. Quote it back in `If-Match` to change only this.", - schema: { type: "string" }, - }, - }, + headers: { ETag: { description: tag, schema: { type: "string" } } }, }); +/** + * A control's requirements are versioned as a whole set, not per page, and a + * client assembling the set from pages has to know it (ADR 0010). + */ +const setTag = + "The version of the whole set, not of this page. A client reading several pages to replace " + + "the set needs the same tag on every page, and reads again from the first if one differs; " + + "quote it in `If-Match` on the replacement."; + /** * Query parameters, one per property of the schema. * @@ -80,6 +104,8 @@ function queryParameters(schema: z.ZodObject) { const pathParameterTypes: Record = { organizationId: "organization", controlId: "control", + standardId: "standard", + requirementId: "requirement", }; /** Derived from the path itself, so a parameter cannot be left undescribed. */ @@ -102,8 +128,8 @@ const conditional = { in: "header", required: false, description: - "The ETag of the record as it was read. Supplied, the write is refused with 412 if the " + - "record has changed since; `*` means only if it still exists.", + "The ETag of what was read. Supplied, the write is refused with 412 if it has " + + "changed since; `*` means only if it still exists.", schema: { type: "string" }, }; @@ -113,13 +139,14 @@ type Operation = { summary: string; query?: z.ZodObject; request?: z.ZodType; - /** Headers an operation takes, which no schema here describes. */ + /** Additional request parameters, including optional and required headers. */ parameters?: unknown[]; responses: Record; }; const tenant = "/api/v1/organizations/{organizationId}"; const controls = `${tenant}/controls`; +const standards = `${tenant}/standards`; /** * Every operation `/api/v1` serves. @@ -192,6 +219,118 @@ const operations: Operation[] = [ "412": fails("The record changed since it was read."), }, }, + { + method: "get", + path: `${controls}/{controlId}/requirements`, + summary: "List the requirements a control answers to.", + // Any control: the published schema is the same whichever it is. + query: collectionQuery(controlRequirementsOrder("{controlId}")), + responses: { + // The tag is the whole set's, so it is the same on every page of it. + "200": versioned("A page of requirements.", collection(requirementResponse), setTag), + "400": fails("The query is not valid."), + "401": fails("The request is not authenticated."), + "404": fails("No such control."), + }, + }, + { + method: "put", + path: `${controls}/{controlId}/requirements`, + summary: "Set which requirements a control answers to, replacing the whole set.", + parameters: [ + { + ...conditional, + description: + "The set's ETag, as every page of it was read. Supplied, the replacement is refused " + + "with 412 if the set has changed since; `*` means only if the control still exists.", + }, + ], + request: controlRequirementsBody, + responses: { + "200": versioned( + "The set as it now stands.", + single(controlRequirementsResponse), + "The version of the set as it now stands. Quote it in `If-Match` to change it again.", + ), + "400": fails("The body is not valid, or names a requirement that is not here."), + "401": fails("The request is not authenticated."), + "404": fails("No such control."), + "412": fails("The requirements changed since they were read."), + "413": fails("The body is too large."), + }, + }, + { + method: "get", + path: standards, + summary: "List the standards the organization has imported, newest first.", + query: collectionQuery(standardOrder), + responses: { + "200": responds("A page of standards.", collection(standardResponse)), + "400": fails("The query is not valid."), + "401": fails("The request is not authenticated."), + "404": fails("No such organization, or the caller is not a member of it."), + }, + }, + { + method: "post", + path: standards, + summary: "Import a standard with the requirements it states, in one request.", + request: importBody, + responses: { + "201": responds("The standard that was imported.", single(standardResponse)), + "400": fails("The body is not valid."), + "401": fails("The request is not authenticated."), + "404": fails("No such organization, or the caller is not a member of it."), + "409": fails("That edition of that standard is already here."), + "413": fails("The body is too large."), + }, + }, + { + method: "get", + path: `${standards}/{standardId}`, + summary: "Retrieve one standard.", + responses: { + "200": responds("The standard.", single(standardResponse)), + "401": fails("The request is not authenticated."), + "404": fails("No such standard."), + }, + }, + { + method: "get", + path: `${standards}/{standardId}/requirements`, + summary: "List a standard's requirements, in the order the standard states them.", + // Any standard: the published schema is the same whichever it is. + query: requirementsQuery(requirementOrder("{standardId}")), + responses: { + "200": responds("A page of requirements.", collection(requirementResponse)), + "400": fails("The query is not valid."), + "401": fails("The request is not authenticated."), + "404": fails("No such standard."), + }, + }, + { + method: "get", + path: `${tenant}/requirements/{requirementId}`, + summary: "Retrieve one requirement, without going through its standard.", + responses: { + "200": responds("The requirement.", single(requirementResponse)), + "401": fails("The request is not authenticated."), + "404": fails("No such requirement."), + }, + }, + { + method: "get", + path: `${tenant}/requirements/{requirementId}/controls`, + summary: "List the controls that answer to a requirement, newest first.", + // Any requirement: the published schema is the same whichever it is. + query: collectionQuery(requirementControlsOrder("{requirementId}")), + responses: { + "200": responds("A page of controls.", collection(controlResponse)), + "400": fails("The query is not valid."), + "401": fails("The request is not authenticated."), + "404": fails("No such requirement."), + }, + }, { method: "get", path: `${tenant}/history`, @@ -243,7 +382,8 @@ export function openApiDocument(sessionCookie: string) { version: "0", description: "The open-source runtime for quality and compliance. Every tenant-owned resource " + - "lives under an organization, and a caller must be a member of it.", + "lives under an organization, and a caller must be a member of it. A collection " + + "refuses query parameters it does not know rather than ignoring them.", }, components: { securitySchemes: { diff --git a/apps/server/organization.ts b/apps/server/organization.ts index 1cf9415..b5ae8b5 100644 --- a/apps/server/organization.ts +++ b/apps/server/organization.ts @@ -34,7 +34,10 @@ export type OrganizationEnv = { * Runs tenant-owned work scoped to this request's organization. Bound, so a * handler cannot scope to an organization the caller was not authorized for. */ - withOrganization: (work: (tx: TenantTransaction) => Promise) => Promise; + withOrganization: ( + work: (tx: TenantTransaction) => Promise, + options?: { repeatableRead?: boolean }, + ) => Promise; /** * Records a change on the transaction that makes it. Bound to the caller * for the same reason: a handler says what happened, never who did it. @@ -138,11 +141,12 @@ export function organizationContext({ c.set("audit", (tx, change) => recordChange(tx, actor, organizationId, change)); // The driver is erased here so handlers need not be generic over it; every // transaction method a handler uses is identical across drivers. - c.set("withOrganization", ((work) => + c.set("withOrganization", ((work, options) => withOrganization( db, organizationId, work, + options, )) as OrganizationEnv["Variables"]["withOrganization"]); await next(); diff --git a/apps/server/pagination.ts b/apps/server/pagination.ts index 8e8e74b..e643cca 100644 --- a/apps/server/pagination.ts +++ b/apps/server/pagination.ts @@ -5,12 +5,12 @@ * How `/api/v1` collections are paged. * * A collection is ordered by a key and the identifier that breaks its ties, and - * paged by a cursor naming the last row of the page before. Collections that - * record what happened are newest first. Reasoning: - * `docs/adr/0006-cursor-paged-collections.md`. + * paged by a cursor naming the last row of the page before. Most collections + * are newest first; a standard's requirements are in the order the standard + * states them. Reasoning: `docs/adr/0006-cursor-paged-collections.md`. */ -import { desc, sql, type SQL } from "drizzle-orm"; +import { asc, desc, sql, type SQL } from "drizzle-orm"; import type { PgColumn } from "drizzle-orm/pg-core"; import { z } from "zod"; @@ -24,13 +24,14 @@ import { z } from "zod"; */ export type Ordering = { readonly name: string; - /** Newest first: every collection so far records what has happened. */ + readonly direction: "asc" | "desc"; readonly key: PgColumn; readonly id: PgColumn; /** The key rendered losslessly as text, for the cursor. */ readonly keyAsText: SQL; /** Whether text coming back is something the cast below will accept. */ readonly keyIsValid: (value: string) => boolean; + readonly keyType: "timestamptz" | "integer"; }; /** The shape a timestamp key takes. Says nothing about whether it exists. */ @@ -53,6 +54,10 @@ function isRealInstant(value: string): boolean { return !Number.isNaN(parsed.getTime()) && parsed.toISOString() === millisecond; } +/** Anything `integer` holds, and nothing that would overflow the cast. */ +const isInt32 = (value: string) => + /^-?\d{1,10}$/.test(value) && Number(value) >= -2_147_483_648 && Number(value) <= 2_147_483_647; + /** * Newest first — the ordering of a collection that is a record of what has * happened rather than a document with an order of its own. @@ -65,10 +70,28 @@ function isRealInstant(value: string): boolean { */ export const newestFirst = (collection: string, createdAt: PgColumn, id: PgColumn): Ordering => ({ name: `${collection}:recent`, + direction: "desc", key: createdAt, id, keyAsText: sql`to_char(${createdAt} at time zone 'UTC', 'YYYY-MM-DD"T"HH24:MI:SS.US"Z"')`, keyIsValid: isRealInstant, + keyType: "timestamptz", +}); + +/** + * The order a document states things in, lowest position first. + * + * Positions may tie — a standard's clauses are not renumbered to insert one — + * so the identifier is as much a part of the key here as anywhere. + */ +export const asStated = (collection: string, position: PgColumn, id: PgColumn): Ordering => ({ + name: `${collection}:stated`, + direction: "asc", + key: position, + id, + keyAsText: sql`${position}::text`, + keyIsValid: isInt32, + keyType: "integer", }); /** The position of a row in a collection's order. */ @@ -91,11 +114,13 @@ const encodeCursor = ({ ordering, key, id }: Cursor): string => * The query a collection ordered by `ordering` accepts. * * A cursor must name this ordering and carry a valid key and identifier shape. - * Values PostgreSQL refuses — a NUL, a year outside its range — would otherwise - * turn a bad request into a 500. + * Values PostgreSQL refuses — a NUL, a year outside its range, an overflowing + * integer — would otherwise turn a bad request into a 500. */ export const collectionQuery = (ordering: Ordering) => - z.object({ + // Strict: a misspelt filter dropped silently answers a different question + // with a page that looks right. + z.strictObject({ limit: z.coerce.number().int().min(1).max(100).default(25), cursor: z .string() @@ -130,7 +155,10 @@ export const collectionQuery = (ordering: Ordering) => export const cursorAt = (ordering: Ordering) => ordering.keyAsText; /** The collection's order, for the query that reads it. */ -export const orderedBy = (ordering: Ordering) => [desc(ordering.key), desc(ordering.id)] as const; +export const orderedBy = (ordering: Ordering) => + ordering.direction === "desc" + ? ([desc(ordering.key), desc(ordering.id)] as const) + : ([asc(ordering.key), asc(ordering.id)] as const); /** * Restricts a query to the rows after `cursor` in the collection's order. @@ -141,16 +169,21 @@ export const orderedBy = (ordering: Ordering) => [desc(ordering.key), desc(order * concurrent inserts and deletes, which can repeat or skip rows. */ export function rowsAfter(ordering: Ordering, cursor: Cursor): SQL { - return sql`(${ordering.key}, ${ordering.id}) < (${cursor.key}::timestamptz, ${cursor.id}::text)`; + const bound = + ordering.keyType === "timestamptz" + ? sql`${cursor.key}::timestamptz` + : sql`${cursor.key}::integer`; + return ordering.direction === "desc" + ? sql`(${ordering.key}, ${ordering.id}) < (${bound}, ${cursor.id}::text)` + : sql`(${ordering.key}, ${ordering.id}) > (${bound}, ${cursor.id}::text)`; } /** * Splits rows fetched with `limit + 1` into a page and the cursor after it. * - * Asking for one more row than the page holds is how the last page is known - * exactly, without a second query and without a count that would be wrong by - * the time it was read. `cursorAt` is dropped on the way out: it is how a page - * is found, not something a caller asked for. + * The extra row tells whether more matched this query, without a second query + * or a total count. Later pages may see changed data. `cursorAt` is dropped on + * the way out: it locates the page and is not part of the response. */ export function page( rows: T[], diff --git a/apps/server/preconditions.test.ts b/apps/server/preconditions.test.ts index 15c122b..85a361f 100644 --- a/apps/server/preconditions.test.ts +++ b/apps/server/preconditions.test.ts @@ -10,7 +10,7 @@ */ import { describe, expect, it } from "vite-plus/test"; -import { ifMatch } from "./preconditions.ts"; +import { ifMatch, setVersion } from "./preconditions.ts"; describe("reading If-Match", () => { const tag = '"424242"'; @@ -91,3 +91,34 @@ describe("reading If-Match", () => { expect(ifMatch(" ", tag)).toBe("failed"); }); }); + +describe("versioning a set", () => { + // Its callers happen to sort before handing it anything, so nothing about + // these guarantees is observable through a route. They are the function's + // own, and this is where they can be held to. + it("is the same for the same members, in any order", () => { + expect(setVersion(["b", "a", "c"])).toBe(setVersion(["a", "b", "c"])); + }); + + it("is different for different members", () => { + expect(setVersion(["a", "b"])).not.toBe(setVersion(["a", "c"])); + expect(setVersion(["a", "b"])).not.toBe(setVersion(["a"])); + }); + + it("gives the empty set a version of its own", () => { + // "Mapped to nothing" is a state a caller can hold and write against, not + // the absence of one. + expect(setVersion([])).toMatch(/^[0-9a-f]{64}$/); + expect(setVersion([])).not.toBe(setVersion([""])); + }); + + it("counts a repeated member once, as a set does", () => { + expect(setVersion(["a", "a"])).toBe(setVersion(["a"])); + }); + + it("does not confuse one member with two", () => { + // The serialisation must be unambiguous: a member containing a comma is + // still one member. + expect(setVersion(["a,b"])).not.toBe(setVersion(["a", "b"])); + }); +}); diff --git a/apps/server/preconditions.ts b/apps/server/preconditions.ts index 25309ce..e5ceccd 100644 --- a/apps/server/preconditions.ts +++ b/apps/server/preconditions.ts @@ -10,6 +10,7 @@ * ask gets the write regardless. */ +import { createHash } from "node:crypto"; import { type SQL, sql } from "drizzle-orm"; import type { PgTable } from "drizzle-orm/pg-core"; @@ -29,6 +30,22 @@ export const rowVersion = (table: PgTable): SQL => sql`${table}. /** The entity tag for a row read with `rowVersion`. */ export const entityTag = (row: { version: string }) => `"${row.version}"`; +/** + * A version for a *set* of rows, which has no `xmin` of its own. + * + * Replacing a control's requirements replaces mapping rows, so no single row + * version describes the set. Hash the member identifiers instead (ADR 0019): + * the same members give the same tag regardless of their input order. + * + * It says nothing about *when* — two sets that are equal are indistinguishable, + * which is what a caller asking "is it still what I read?" actually means. + */ +export const setVersion = (members: readonly string[]): string => + // JSON rather than a join, so no member can pass for two. + createHash("sha256") + .update(JSON.stringify([...new Set(members)].sort())) + .digest("hex"); + /** What `If-Match` said about the row as it now stands. */ export type Precondition = "absent" | "met" | "failed"; @@ -95,7 +112,8 @@ export function ifMatch(header: string | undefined, tag: string): Precondition { * The row as a client sees it: everything but the version. * * The version is read alongside the columns so that one query serves both the - * body and the tag, and a strict response schema would refuse it in the body. + * body and the tag. Response-schema tests reject the extra field; handlers + * do not validate outgoing responses at runtime. */ export const withoutVersion = (row: T): Omit => { const { version, ...rest } = row; diff --git a/apps/server/privileges.test.ts b/apps/server/privileges.test.ts index 7ebdba7..389e8aa 100644 --- a/apps/server/privileges.test.ts +++ b/apps/server/privileges.test.ts @@ -175,6 +175,31 @@ describe("the product runs on the privileges it documents", () => { expect(data.map((event) => event.action).sort()).toEqual(["created", "updated"]); }); + it("imports a standard and maps a control to it", async () => { + const imported = await request( + acme, + "/standards", + asJson({ + name: "ISO 9001", + edition: "2015", + requirements: [{ reference: "7.5.3", title: "Documented information" }], + }), + ); + expect(imported.status).toBe(201); + const standardId = (await json<{ data: { id: string } }>(imported)).data.id; + const { data } = await json<{ data: { id: string }[] }>( + await request(acme, `/standards/${standardId}/requirements`), + ); + + const mapped = await request(acme, `/controls/${control}/requirements`, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ requirementIds: [data[0]!.id] }), + }); + + expect(mapped.status).toBe(200); + }); + it("needs no privilege on a sequence, because nothing here has one", async () => { // Identifiers are generated by the application (ADR 0002), so a deployment // granting only table privileges is not missing something. diff --git a/apps/server/requirements.test.ts b/apps/server/requirements.test.ts new file mode 100644 index 0000000..6b49ac2 --- /dev/null +++ b/apps/server/requirements.test.ts @@ -0,0 +1,398 @@ +// SPDX-FileCopyrightText: 2026 Quality Runtime contributors +// SPDX-License-Identifier: Apache-2.0 + +/** + * Reading a requirement on its own, the controls that answer to it, which + * clauses of a standard nobody has taken up, and finding a clause by the + * reference people cite. + * + * The last is the first question this API answers rather than records, so most + * of what is checked here is that the filter says what it means. Requests run + * as a non-superuser role that owns the tables, so the row-level security + * policies are in force as they are in a deployment. + */ + +import { fileURLToPath } from "node:url"; +import { PGlite } from "@electric-sql/pglite"; +import { schema } from "@qualityruntime/db"; +import { drizzle } from "drizzle-orm/pglite"; +import { migrate } from "drizzle-orm/pglite/migrator"; +import { beforeAll, describe, expect, it } from "vite-plus/test"; +import { createApp } from "./app.ts"; +import { createAuth } from "./auth.ts"; + +const migrationsFolder = fileURLToPath(new URL("../../packages/db/migrations", import.meta.url)); + +const createTestDatabase = (client: PGlite) => drizzle({ client, schema, casing: "snake_case" }); + +let app: ReturnType; +type Tenant = { cookie: string; organizationId: string }; +let acme: Tenant; +let globex: Tenant; + +let standardId: string; +/** Acme's five requirements, in the order their standard states them. */ +let requirements: { id: string; reference: string }[]; +let theirRequirement: string; + +const json = async (response: Response): Promise => (await response.json()) as T; + +type Page = { data: T[]; nextCursor: string | null }; +type Named = { id: string; name: string }; +type Failure = { error: { code: string; details?: { path: string }[] } }; + +const request = ( + tenant: Tenant, + path: string, + init: Omit & { headers?: Record } = {}, +) => + app.request(`/api/v1/organizations/${tenant.organizationId}${path}`, { + ...init, + // Merged, not replaced: a caller's own headers are the point of + // passing them, and dropping them silently makes a test pass for + // the wrong reason. + headers: { + cookie: tenant.cookie, + ...(init.body ? { "content-type": "application/json" } : {}), + ...init.headers, + }, + }); + +async function newControl(tenant: Tenant, name: string): Promise { + const response = await request(tenant, "/controls", { + method: "POST", + body: JSON.stringify({ name }), + }); + expect(response.status).toBe(201); + return (await json<{ data: { id: string } }>(response)).data.id; +} + +const answerTo = (tenant: Tenant, controlId: string, requirementIds: string[]) => + request(tenant, `/controls/${controlId}/requirements`, { + method: "PUT", + body: JSON.stringify({ requirementIds }), + }); + +const listRequirements = async (tenant: Tenant, id: string, query = "") => + json>( + await request(tenant, `/standards/${id}/requirements${query}`), + ); + +beforeAll(async () => { + const client = new PGlite(); + const db = createTestDatabase(client); + await migrate(db, { migrationsFolder }); + app = createApp({ + db, + auth: createAuth(db, { + baseURL: "http://localhost", + secret: "test-secret-of-at-least-32-characters", + }), + }); + + const tenant = async (slug: string): Promise => { + const signedUp = await app.request("/api/auth/sign-up/email", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "Ada", + email: `${slug}@example.test`, + password: "correct horse", + }), + }); + expect(signedUp.status).toBe(200); + const cookie = signedUp.headers + .getSetCookie() + .map((value) => value.split(";", 1)[0]) + .join("; "); + const created = await app.request("/api/auth/organization/create", { + method: "POST", + headers: { "content-type": "application/json", cookie }, + body: JSON.stringify({ name: slug, slug }), + }); + expect(created.status).toBe(200); + return { cookie, organizationId: (await json<{ id: string }>(created)).id }; + }; + + acme = await tenant("acme"); + globex = await tenant("globex"); + + await client.exec(` + create role qualityruntime_app nosuperuser nobypassrls; + grant all on all tables in schema public to qualityruntime_app; + alter table "control" owner to qualityruntime_app; + alter table "audit_event" owner to qualityruntime_app; + alter table "standard" owner to qualityruntime_app; + alter table "requirement" owner to qualityruntime_app; + alter table "control_requirement" owner to qualityruntime_app; + set role qualityruntime_app; + `); + + const importStandard = async (owner: Tenant, count: number) => { + const response = await request(owner, "/standards", { + method: "POST", + body: JSON.stringify({ + name: "ISO 9001", + edition: "2015", + requirements: Array.from({ length: count }, (_, index) => ({ + reference: `7.${index + 1}`, + title: `Clause ${index + 1}`, + })), + }), + }); + expect(response.status).toBe(201); + const id = (await json<{ data: { id: string } }>(response)).data.id; + return { id, requirements: (await listRequirements(owner, id, "?limit=100")).data }; + }; + + const acmeStandard = await importStandard(acme, 5); + standardId = acmeStandard.id; + requirements = acmeStandard.requirements; + theirRequirement = (await importStandard(globex, 1)).requirements[0]!.id; + + // 7.1 is answered by two controls, 7.2 by one, and 7.3 to 7.5 by none. + const first = await newControl(acme, "Access review"); + const second = await newControl(acme, "Backup restore test"); + await answerTo(acme, first, [requirements[0]!.id, requirements[1]!.id]); + await answerTo(acme, second, [requirements[0]!.id]); +}, 60_000); + +describe("a requirement on its own", () => { + it("is reachable without naming its standard", async () => { + const response = await request(acme, `/requirements/${requirements[0]!.id}`); + + expect(response.status).toBe(200); + const { data } = await json<{ data: { reference: string; standardId: string } }>(response); + expect(data.reference).toBe("7.1"); + expect(data.standardId).toBe(standardId); + }); + + it.each([ + ["one that does not exist", "req_0000000000000000"], + ["an id of the wrong shape", "not-an-id"], + ["an id carrying another table's prefix", "ctl_v1stgxr8z5jdhi6b"], + ])("answers 404 for %s", async (_case, id) => { + const response = await request(acme, `/requirements/${id}`); + + expect(response.status).toBe(404); + expect((await json(response)).error.code).toBe("not_found"); + }); + + it("answers 404 for another organization's requirement", async () => { + const theirs = await request(acme, `/requirements/${theirRequirement}`); + const absent = await request(acme, "/requirements/req_0000000000000000"); + + expect(theirs.status).toBe(404); + expect(await json(theirs)).toEqual(await json(absent)); + }); +}); + +describe("the controls that answer to a requirement", () => { + it("lists them, newest first", async () => { + const response = await request(acme, `/requirements/${requirements[0]!.id}/controls`); + + expect(response.status).toBe(200); + const { data } = await json>(response); + expect(data.map((row) => row.name)).toEqual(["Backup restore test", "Access review"]); + }); + + it("is empty for a requirement nothing answers to", async () => { + const { data } = await json>( + await request(acme, `/requirements/${requirements[4]!.id}/controls`), + ); + + expect(data).toEqual([]); + }); + + it("pages like every other collection", async () => { + const first = await json>( + await request(acme, `/requirements/${requirements[0]!.id}/controls?limit=1`), + ); + expect(first.nextCursor).not.toBeNull(); + + const second = await json>( + await request( + acme, + `/requirements/${requirements[0]!.id}/controls?limit=1&cursor=${encodeURIComponent(first.nextCursor!)}`, + ), + ); + + expect(second.data.map((row) => row.name)).toEqual(["Access review"]); + expect(second.nextCursor).toBeNull(); + }); + + it("refuses a cursor from another requirement's controls", async () => { + const theirs = await json>( + await request(acme, `/requirements/${requirements[0]!.id}/controls?limit=1`), + ); + + const response = await request( + acme, + `/requirements/${requirements[1]!.id}/controls?cursor=${encodeURIComponent(theirs.nextCursor!)}`, + ); + + expect(response.status).toBe(400); + expect((await json(response)).error.details?.map((d) => d.path)).toContain("cursor"); + }); + + it("answers 404 for another organization's requirement", async () => { + expect((await request(acme, `/requirements/${theirRequirement}/controls`)).status).toBe(404); + }); +}); + +describe("which clauses nobody has taken up", () => { + it("keeps only the requirements with no control mapped", async () => { + const { data } = await listRequirements(acme, standardId, "?mapped=false&limit=100"); + + expect(data.map((row) => row.reference)).toEqual(["7.3", "7.4", "7.5"]); + }); + + it("keeps only the requirements that have one", async () => { + const { data } = await listRequirements(acme, standardId, "?mapped=true&limit=100"); + + expect(data.map((row) => row.reference)).toEqual(["7.1", "7.2"]); + }); + + it("returns everything when the filter is left out", async () => { + const { data } = await listRequirements(acme, standardId, "?limit=100"); + + expect(data).toHaveLength(5); + }); + + it("follows a clause as it is taken up and let go", async () => { + const control = await newControl(acme, "Temporary"); + const unmapped = () => + listRequirements(acme, standardId, "?mapped=false&limit=100").then(({ data }) => + data.map((row) => row.reference), + ); + + await answerTo(acme, control, [requirements[2]!.id]); + expect(await unmapped()).toEqual(["7.4", "7.5"]); + + await answerTo(acme, control, []); + expect(await unmapped()).toEqual(["7.3", "7.4", "7.5"]); + }); + + it("pages the filtered collection without losing the filter", async () => { + const first = await listRequirements(acme, standardId, "?mapped=false&limit=2"); + expect(first.nextCursor).not.toBeNull(); + + const second = await listRequirements( + acme, + standardId, + `?mapped=false&limit=2&cursor=${encodeURIComponent(first.nextCursor!)}`, + ); + + expect(first.data.map((row) => row.reference)).toEqual(["7.3", "7.4"]); + expect(second.data.map((row) => row.reference)).toEqual(["7.5"]); + }); + + it.each([ + ["neither true nor false", "?mapped=perhaps"], + ["an empty value", "?mapped="], + ])("refuses %s", async (_case, query) => { + const response = await request(acme, `/standards/${standardId}/requirements${query}`); + + expect(response.status).toBe(400); + expect((await json(response)).error.details?.map((d) => d.path)).toContain("mapped"); + }); +}); + +describe("a filter that is misspelt", () => { + it.each([ + ["mapped", "?maped=false"], + ["reference", "?refernece=7.3"], + ])("refuses it rather than answering unfiltered (%s)", async (_case, query) => { + const response = await request(acme, `/standards/${standardId}/requirements${query}`); + + expect(response.status).toBe(400); + expect((await json(response)).error.code).toBe("invalid_request"); + }); +}); + +describe("a clause looked up by the reference people cite", () => { + it("resolves a reference to the requirement it names", async () => { + const { data, nextCursor } = await listRequirements(acme, standardId, "?reference=7.3"); + + expect(data.map((row) => row.id)).toEqual([requirements[2]!.id]); + expect(nextCursor).toBeNull(); + }); + + it("finds nothing for a reference the standard does not state", async () => { + const { data, nextCursor } = await listRequirements(acme, standardId, "?reference=7.30"); + + expect(data).toEqual([]); + expect(nextCursor).toBeNull(); + }); + + it("matches exactly: not a prefix, not a pattern", async () => { + for (const query of ["?reference=7", "?reference=7.%25", "?reference=7._"]) { + expect((await listRequirements(acme, standardId, query)).data).toEqual([]); + } + }); + + it("removes surrounding whitespace, as the import did", async () => { + const { data } = await listRequirements(acme, standardId, "?reference=%207.3%20"); + + expect(data.map((row) => row.reference)).toEqual(["7.3"]); + }); + + it("keeps case, because the reference is its exact string", async () => { + const response = await request(acme, "/standards", { + method: "POST", + body: JSON.stringify({ + name: "ETSI EN 303 645", + edition: "V3.1.3", + requirements: [ + { reference: "Provision 5.1-1", title: "Unique passwords" }, + { reference: "provision 5.1-1", title: "The same clause, typed differently" }, + ], + }), + }); + expect(response.status).toBe(201); + const etsi = (await json<{ data: { id: string } }>(response)).data.id; + + const { data } = await listRequirements(acme, etsi, "?reference=Provision%205.1-1"); + + expect(data.map((row) => row.reference)).toEqual(["Provision 5.1-1"]); + }); + + it("looks only in the standard named, and only in this organization", async () => { + // Globex states 7.1 too, in its own copy of the same standard. + const { data } = await listRequirements(acme, standardId, "?reference=7.1"); + + expect(data.map((row) => row.id)).toEqual([requirements[0]!.id]); + }); + + it("composes with the mapped filter", async () => { + const mapped = await listRequirements(acme, standardId, "?reference=7.1&mapped=true"); + const unmapped = await listRequirements(acme, standardId, "?reference=7.1&mapped=false"); + + expect(mapped.data.map((row) => row.reference)).toEqual(["7.1"]); + expect(unmapped.data).toEqual([]); + }); + + it.each([ + ["an empty value", "?reference="], + ["only whitespace", "?reference=%20%20"], + ["a NUL", "?reference=7%00"], + ["more than a reference can hold", `?reference=${"7".repeat(101)}`], + ])("refuses %s", async (_case, query) => { + const response = await request(acme, `/standards/${standardId}/requirements${query}`); + + expect(response.status).toBe(400); + expect((await json(response)).error.details?.map((d) => d.path)).toContain( + "reference", + ); + }); + + it("answers 404 for another organization's standard, not an empty page", async () => { + const theirs = await request(globex, `/requirements/${theirRequirement}`); + const { standardId: theirStandard } = (await json<{ data: { standardId: string } }>(theirs)) + .data; + + const response = await request(acme, `/standards/${theirStandard}/requirements?reference=7.1`); + + expect(response.status).toBe(404); + }); +}); diff --git a/apps/server/requirements.ts b/apps/server/requirements.ts new file mode 100644 index 0000000..1d56def --- /dev/null +++ b/apps/server/requirements.ts @@ -0,0 +1,117 @@ +// SPDX-FileCopyrightText: 2026 Quality Runtime contributors +// SPDX-License-Identifier: Apache-2.0 + +/** + * Requirement routes. + * + * A requirement belongs to a standard, but is not only reachable through one: a + * control names requirements by identifier, and following that identifier + * should not mean knowing which standard stated it. Reasoning: + * `docs/adr/0011-reading-a-mapping-from-both-ends.md`. + * + * Mounted under `/api/v1/organizations/:organizationId` behind + * `organizationContext`; row-level security scopes every query (ADR 0003). + */ + +import { idPattern, schema } from "@qualityruntime/db"; +import { and, eq, getTableColumns } from "drizzle-orm"; +import { Hono } from "hono"; +import { createMiddleware } from "hono/factory"; +import { z } from "zod"; +import type { OrganizationEnv } from "./organization.ts"; +import { + collectionQuery, + cursorAt, + newestFirst, + orderedBy, + page, + rowsAfter, +} from "./pagination.ts"; +import { failure } from "./responses.ts"; +import { rejection } from "./validation.ts"; + +/** A requirement, as a client sees it. */ +export const requirementResponse = z.strictObject({ + id: z.string(), + organizationId: z.string(), + standardId: z.string(), + reference: z.string(), + title: z.string(), + text: z.string().nullable(), + position: z.int(), + createdAt: z.iso.datetime(), + updatedAt: z.iso.datetime(), +}); + +/** + * The controls answering to a requirement, newest first. + * + * Controls have no order of their own — nothing states them in a sequence the + * way a standard states its clauses — so the ordering is the one every record + * of what has happened gets. Scoped to the requirement, like every cursor. + */ +export const requirementControlsOrder = (requirementId: string) => + newestFirst(`requirement-controls/${requirementId}`, schema.control.createdAt, schema.control.id); + +const isRequirementId = new RegExp(idPattern("requirement")); + +/** Stops an id that could not name a requirement before it reaches PostgreSQL. */ +const knownRequirementId = createMiddleware(async (c, next) => { + if (!isRequirementId.test(c.req.param("requirementId") ?? "")) { + return c.json(failure("not_found", "No such requirement."), 404); + } + await next(); +}); + +export const requirements = new Hono() + .get("/requirements/:requirementId", knownRequirementId, async (c) => { + const [row] = await c.var.withOrganization((tx) => + tx + .select() + .from(schema.requirement) + .where(eq(schema.requirement.id, c.req.param("requirementId"))), + ); + if (!row) return c.json(failure("not_found", "No such requirement."), 404); + + return c.json({ data: row }); + }) + + .get("/requirements/:requirementId/controls", knownRequirementId, async (c) => { + const requirementId = c.req.param("requirementId"); + const ordering = requirementControlsOrder(requirementId); + const query = collectionQuery(ordering).safeParse(c.req.query()); + if (!query.success) return c.json(rejection("query", query.error), 400); + const { limit, cursor } = query.data; + + // One snapshot: under `read committed` the check and the page are two + // statements, and a parent deleted between them would read as an empty one. + const found = await c.var.withOrganization( + async (tx) => { + // The requirement has to be visible first, or its absence would read as a + // requirement nothing answers to. + const [requirement] = await tx + .select({ id: schema.requirement.id }) + .from(schema.requirement) + .where(eq(schema.requirement.id, requirementId)); + if (!requirement) return undefined; + + return tx + .select({ ...getTableColumns(schema.control), cursorAt: cursorAt(ordering) }) + .from(schema.controlRequirement) + .innerJoin(schema.control, eq(schema.control.id, schema.controlRequirement.controlId)) + .where( + and( + eq(schema.controlRequirement.requirementId, requirementId), + cursor ? rowsAfter(ordering, cursor) : undefined, + ), + ) + .orderBy(...orderedBy(ordering)) + .limit(limit + 1); + }, + { repeatableRead: true }, + ); + if (!found) return c.json(failure("not_found", "No such requirement."), 404); + + const { rows, nextCursor } = page(found, limit, ordering); + return c.json({ data: rows, nextCursor }); + }); diff --git a/apps/server/standards.test.ts b/apps/server/standards.test.ts new file mode 100644 index 0000000..fde4456 --- /dev/null +++ b/apps/server/standards.test.ts @@ -0,0 +1,599 @@ +// SPDX-FileCopyrightText: 2026 Quality Runtime contributors +// SPDX-License-Identifier: Apache-2.0 + +/** + * Importing a standard, and reading it back in the order it states. + * + * Requirements are the first collection ordered by something other than time, + * so most of what is checked here is that a second ordering behaves like the + * first without borrowing its cursors. Requests run as a non-superuser role + * that owns the tables, so the row-level security policies are in force as they + * are in a deployment. + */ + +import { fileURLToPath } from "node:url"; +import { PGlite } from "@electric-sql/pglite"; +import { schema, withOrganization } from "@qualityruntime/db"; +import { and, eq } from "drizzle-orm"; +import { drizzle } from "drizzle-orm/pglite"; +import { migrate } from "drizzle-orm/pglite/migrator"; +import { beforeAll, describe, expect, it } from "vite-plus/test"; +import { createApp } from "./app.ts"; +import { createAuth } from "./auth.ts"; + +const migrationsFolder = fileURLToPath(new URL("../../packages/db/migrations", import.meta.url)); + +const createTestDatabase = (client: PGlite) => drizzle({ client, schema, casing: "snake_case" }); + +let db: ReturnType; +let app: ReturnType; +type Tenant = { cookie: string; organizationId: string }; +let acme: Tenant; +let globex: Tenant; + +const json = async (response: Response): Promise => (await response.json()) as T; + +type Standard = { id: string; name: string; edition: string; requirementCount: number }; +type Requirement = { + id: string; + reference: string; + title: string; + text: string | null; + position: number; +}; +type Page = { data: T[]; nextCursor: string | null }; +type Failure = { error: { code: string; details?: { path: string }[] } }; + +const request = ( + tenant: Tenant, + path: string, + init: Omit & { headers?: Record } = {}, +) => + app.request(`/api/v1/organizations/${tenant.organizationId}${path}`, { + ...init, + // Merged, not replaced: a caller's own headers are the point of + // passing them, and dropping them silently makes a test pass for + // the wrong reason. + headers: { + cookie: tenant.cookie, + ...(init.body ? { "content-type": "application/json" } : {}), + ...init.headers, + }, + }); + +/** A standard of `count` clauses, numbered so that lexical order is wrong. */ +const clauses = (count: number) => + Array.from({ length: count }, (_, index) => ({ + reference: `7.${index + 1}`, + title: `Clause ${index + 1}`, + text: index === 0 ? null : `The organization shall do thing ${index + 1}.`, + })); + +async function importStandard(tenant: Tenant, body: unknown): Promise { + const response = await request(tenant, "/standards", { + method: "POST", + body: JSON.stringify(body), + }); + expect(response.status).toBe(201); + return (await json<{ data: Standard }>(response)).data; +} + +/** Walks every page, returning what a client would have collected. */ +async function walk(tenant: Tenant, path: string, query: string): Promise { + const collected: T[] = []; + let cursor: string | null = null; + for (let guard = 0; guard < 30; guard++) { + const suffix: string = cursor ? `&cursor=${encodeURIComponent(cursor)}` : ""; + const response = await request(tenant, `${path}?${query}${suffix}`); + expect(response.status).toBe(200); + const body: Page = await json>(response); + collected.push(...body.data); + if (!body.nextCursor) return collected; + cursor = body.nextCursor; + } + throw new Error("paging did not terminate"); +} + +beforeAll(async () => { + const client = new PGlite(); + db = createTestDatabase(client); + await migrate(db, { migrationsFolder }); + app = createApp({ + db, + auth: createAuth(db, { + baseURL: "http://localhost", + secret: "test-secret-of-at-least-32-characters", + }), + }); + + const tenant = async (slug: string): Promise => { + const signedUp = await app.request("/api/auth/sign-up/email", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "Ada", + email: `${slug}@example.test`, + password: "correct horse", + }), + }); + expect(signedUp.status).toBe(200); + const cookie = signedUp.headers + .getSetCookie() + .map((value) => value.split(";", 1)[0]) + .join("; "); + const created = await app.request("/api/auth/organization/create", { + method: "POST", + headers: { "content-type": "application/json", cookie }, + body: JSON.stringify({ name: slug, slug }), + }); + expect(created.status).toBe(200); + return { cookie, organizationId: (await json<{ id: string }>(created)).id }; + }; + + acme = await tenant("acme"); + globex = await tenant("globex"); + + await client.exec(` + create role qualityruntime_app nosuperuser nobypassrls; + grant all on all tables in schema public to qualityruntime_app; + alter table "control" owner to qualityruntime_app; + alter table "audit_event" owner to qualityruntime_app; + alter table "standard" owner to qualityruntime_app; + alter table "requirement" owner to qualityruntime_app; + set role qualityruntime_app; + `); +}, 60_000); + +describe("importing a standard", () => { + it("stores the standard and everything it states, in one request", async () => { + const imported = await importStandard(acme, { + name: "ISO 9001", + edition: "2015", + requirements: clauses(3), + }); + + expect(imported.id).toMatch(/^std_[0-9a-z]{16}$/); + expect(imported.requirementCount).toBe(3); + const { data } = await json>( + await request(acme, `/standards/${imported.id}/requirements`), + ); + expect(data.map((row) => row.reference)).toEqual(["7.1", "7.2", "7.3"]); + expect(data.map((row) => row.position)).toEqual([1, 2, 3]); + expect(data[0]?.text).toBeNull(); + }); + + it("refuses a second import of the same edition", async () => { + const body = { name: "SOC 2", edition: "2017", requirements: clauses(2) }; + await importStandard(acme, body); + + const again = await request(acme, "/standards", { method: "POST", body: JSON.stringify(body) }); + + expect(again.status).toBe(409); + expect((await json(again)).error.code).toBe("already_exists"); + }); + + it("accepts a different edition of the same standard", async () => { + await importStandard(acme, { name: "ISO 27001", edition: "2013", requirements: clauses(1) }); + + const later = await importStandard(acme, { + name: "ISO 27001", + edition: "2022", + requirements: clauses(2), + }); + + expect(later.edition).toBe("2022"); + }); + + it("lets another organization import the same standard", async () => { + const theirs = await importStandard(globex, { + name: "ISO 9001", + edition: "2015", + requirements: clauses(1), + }); + + expect(theirs.name).toBe("ISO 9001"); + }); + + it("refuses a clause repeated after trimming", async () => { + // The database says the same thing, and reaching it would be a 500 for + // what is plainly malformed input. + const response = await request(acme, "/standards", { + method: "POST", + body: JSON.stringify({ + name: "Repeated", + edition: "1", + requirements: [ + { reference: "1", title: "First" }, + { reference: " 1 ", title: "Also first" }, + ], + }), + }); + + expect(response.status).toBe(400); + expect((await json(response)).error.code).toBe("invalid_request"); + }); + + it("writes nothing at all when the body is refused", async () => { + const response = await request(acme, "/standards", { + method: "POST", + body: JSON.stringify({ + name: "Half a standard", + edition: "1", + requirements: [ + { reference: "1", title: "Fine" }, + { reference: "2", title: " " }, + ], + }), + }); + + expect(response.status).toBe(400); + const all = await walk(acme, "/standards", "limit=100"); + expect(all.map((row) => row.name)).not.toContain("Half a standard"); + }); + + it("writes nothing new when the standard is already here", async () => { + // The import returns before touching requirements or history, so a refused + // second import leaves the first exactly as it was. + const body = { name: "Once only", edition: "1", requirements: clauses(3) }; + const first = await importStandard(acme, body); + + const again = await request(acme, "/standards", { method: "POST", body: JSON.stringify(body) }); + expect(again.status).toBe(409); + + const events = await withOrganization(db, acme.organizationId, (tx) => + tx + .select() + .from(schema.auditEvent) + .where( + and( + eq(schema.auditEvent.resourceType, "standard"), + eq(schema.auditEvent.resourceId, first.id), + ), + ), + ); + expect(events).toHaveLength(1); + const { data } = await json>( + await request(acme, `/standards/${first.id}/requirements?limit=100`), + ); + expect(data).toHaveLength(3); + }); + + it("counts the requirements it stored, on the list and on the standard", async () => { + const imported = await importStandard(acme, { + name: "Counted", + edition: "1", + requirements: clauses(4), + }); + + const retrieved = await json<{ data: Standard }>( + await request(acme, `/standards/${imported.id}`), + ); + const listed = (await walk(acme, "/standards", "limit=100")).find( + (row) => row.id === imported.id, + ); + + expect(retrieved.data.requirementCount).toBe(4); + expect(listed?.requirementCount).toBe(4); + }); + + it("accepts an import larger than another route would take", async () => { + // Well past the 64 KiB a control is allowed, and well inside the ceiling. + const long = "x".repeat(40_000); + const imported = await importStandard(acme, { + name: "Wordy", + edition: "1", + requirements: [ + { reference: "1", title: "First", text: long }, + { reference: "2", title: "Second", text: long }, + ], + }); + + expect(imported.requirementCount).toBe(2); + }); + + it("refuses an import past its 1 MiB body limit", async () => { + // Every clause valid on its own, so only the body limit can refuse it. + const response = await request(acme, "/standards", { + method: "POST", + body: JSON.stringify({ + name: "Oversized", + edition: "1", + requirements: Array.from({ length: 22 }, (_, index) => ({ + reference: `${index + 1}`, + title: "Long", + text: "x".repeat(50_000), + })), + }), + }); + + expect(response.status).toBe(413); + }); + + it("refuses a field it does not know on the standard itself", async () => { + const response = await request(acme, "/standards", { + method: "POST", + body: JSON.stringify({ + name: "Misspelt", + edtion: "1", + edition: "1", + requirements: [{ reference: "1", title: "A" }], + }), + }); + + expect(response.status).toBe(400); + expect((await json(response)).error.code).toBe("invalid_request"); + }); + + it("refuses a field it does not know, rather than dropping what it holds", async () => { + const response = await request(acme, "/standards", { + method: "POST", + body: JSON.stringify({ + name: "Misspelt", + edition: "1", + requirements: [{ reference: "1", title: "A", tetx: "The wording, lost" }], + }), + }); + + expect(response.status).toBe(400); + expect((await json(response)).error.code).toBe("invalid_request"); + }); + + it("refuses more requirements than one import may carry", async () => { + const response = await request(acme, "/standards", { + method: "POST", + body: JSON.stringify({ name: "Too many", edition: "1", requirements: clauses(2_001) }), + }); + + expect(response.status).toBe(400); + expect((await json(response)).error.details?.map((d) => d.path)).toContain( + "requirements", + ); + }); + + it.each([ + ["no requirements at all", { name: "Empty", edition: "1", requirements: [] }], + ["no edition", { name: "Unversioned", requirements: [{ reference: "1", title: "A" }] }], + ["a clause with no reference", { name: "N", edition: "1", requirements: [{ title: "A" }] }], + ])("refuses %s", async (_case, body) => { + const response = await request(acme, "/standards", { + method: "POST", + body: JSON.stringify(body), + }); + + expect(response.status).toBe(400); + expect((await json(response)).error.code).toBe("invalid_request"); + }); + + it("records the import as one event, not one per clause", async () => { + const imported = await importStandard(acme, { + name: "Audited", + edition: "1", + requirements: clauses(5), + }); + + const response = await request(acme, `/history?resource=${imported.id}`); + expect(response.status).toBe(200); + const { data } = await json>(response); + + expect(data).toHaveLength(1); + expect(data[0]).toMatchObject({ + action: "created", + resourceType: "standard", + after: { name: "Audited", edition: "1", requirementCount: 5 }, + }); + }); +}); + +describe("reading requirements in the order they are stated", () => { + let standard: Standard; + + beforeAll(async () => { + // Twelve, so that clause 10 exists: `7.10` sorts before `7.9` lexically, + // and ordering by reference rather than position would show it. + standard = await importStandard(acme, { + name: "Ordered", + edition: "1", + requirements: clauses(12), + }); + }); + + it("pages through every clause once, in the standard's order", async () => { + const walked = await walk( + acme, + `/standards/${standard.id}/requirements`, + "limit=5", + ); + + expect(walked.map((row) => row.position)).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); + expect(walked.map((row) => row.reference)).toEqual( + Array.from({ length: 12 }, (_, index) => `7.${index + 1}`), + ); + }); + + it("pages across clauses that share a position, by identifier", async () => { + // Positions may tie, so the identifier is part of the order. An import + // cannot make a tie, so this one is made in the database. + const standard = await importStandard(acme, { + name: "Tied positions", + edition: "1", + requirements: clauses(3), + }); + await withOrganization(db, acme.organizationId, (tx) => + tx + .update(schema.requirement) + .set({ position: 2 }) + .where(eq(schema.requirement.standardId, standard.id)), + ); + const tied = await withOrganization(db, acme.organizationId, (tx) => + tx + .select({ id: schema.requirement.id }) + .from(schema.requirement) + .where(eq(schema.requirement.standardId, standard.id)), + ); + + const walked = await walk( + acme, + `/standards/${standard.id}/requirements`, + "limit=1", + ); + + expect(walked.map((row) => row.id)).toEqual(tied.map((row) => row.id).sort()); + }); + + it("refuses a cursor from another standard's requirements", async () => { + // Every standard numbers its clauses from one, so the key alone would be + // accepted here and would resume from a position that means something else. + const other = await importStandard(acme, { + name: "Elsewhere", + edition: "1", + requirements: clauses(3), + }); + const theirs = await json>( + await request(acme, `/standards/${other.id}/requirements?limit=1`), + ); + expect(theirs.nextCursor).not.toBeNull(); + + const response = await request( + acme, + `/standards/${standard.id}/requirements?cursor=${encodeURIComponent(theirs.nextCursor!)}`, + ); + + expect(response.status).toBe(400); + expect((await json(response)).error.details?.map((d) => d.path)).toContain("cursor"); + }); + + it("refuses a cursor from another control's history", async () => { + // Same defect as between standards, one level down: every control's history + // is newest first, so only the control in the cursor tells them apart. + const made = async (name: string) => { + const response = await request(acme, "/controls", { + method: "POST", + body: JSON.stringify({ name }), + }); + expect(response.status).toBe(201); + return (await json<{ data: { id: string } }>(response)).data.id; + }; + const first = await made("Has history"); + const second = await made("Also has history"); + await request(acme, `/controls/${first}`, { + method: "PATCH", + body: JSON.stringify({ name: "Renamed" }), + }); + const theirs = await json>( + await request(acme, `/history?resource=${first}&limit=1`), + ); + expect(theirs.nextCursor).not.toBeNull(); + + const response = await request( + acme, + `/history?resource=${second}&cursor=${encodeURIComponent(theirs.nextCursor!)}`, + ); + + expect(response.status).toBe(400); + expect((await json(response)).error.details?.map((d) => d.path)).toContain("cursor"); + }); + + it("refuses a cursor from a collection ordered the other way", async () => { + // A position in one ordering means nothing in another, and answering from + // the wrong place would be worse than refusing. + const standards = await json>(await request(acme, "/standards?limit=1")); + expect(standards.nextCursor).not.toBeNull(); + + const response = await request( + acme, + `/standards/${standard.id}/requirements?cursor=${encodeURIComponent(standards.nextCursor!)}`, + ); + + expect(response.status).toBe(400); + expect((await json(response)).error.details?.map((d) => d.path)).toContain("cursor"); + }); + + it.each([ + ["past what an integer holds", "2147483648"], + ["not a number", "seven"], + ["empty", ""], + ])("refuses a cursor whose position is %s", async (_case, position) => { + // A position the cast to integer would refuse must be a 400, not a 500. + const standard = await importStandard(acme, { + name: `Positions ${position || "empty"}`, + edition: "1", + requirements: clauses(1), + }); + const [only] = await walk(acme, `/standards/${standard.id}/requirements`, ""); + const cursor = Buffer.from( + `requirements/${standard.id}:stated|${position}|${only!.id}`, + "utf8", + ).toString("base64url"); + + const response = await request( + acme, + `/standards/${standard.id}/requirements?cursor=${encodeURIComponent(cursor)}`, + ); + + expect(response.status).toBe(400); + expect((await json(response)).error.details?.map((d) => d.path)).toContain("cursor"); + }); + + it("refuses a cursor from a different collection ordered the same way", async () => { + // Controls and standards are both newest first, so the key would be + // accepted on either. Only the collection in the cursor tells them apart, + // and answering a list of standards from a position in the controls is the + // kind of wrong that looks like it worked. + await request(acme, "/controls", { method: "POST", body: JSON.stringify({ name: "One" }) }); + await request(acme, "/controls", { method: "POST", body: JSON.stringify({ name: "Two" }) }); + const controls = await json>(await request(acme, "/controls?limit=1")); + expect(controls.nextCursor).not.toBeNull(); + + const response = await request( + acme, + `/standards?cursor=${encodeURIComponent(controls.nextCursor!)}`, + ); + + expect(response.status).toBe(400); + expect((await json(response)).error.details?.map((d) => d.path)).toContain("cursor"); + }); +}); + +describe("standards and tenants", () => { + it("shows an organization only its own standards", async () => { + const ours = await importStandard(acme, { + name: "Ours", + edition: "1", + requirements: clauses(1), + }); + const theirs = await importStandard(globex, { + name: "Theirs", + edition: "1", + requirements: clauses(1), + }); + + const mine = (await walk(acme, "/standards", "limit=100")).map((row) => row.id); + const yours = (await walk(globex, "/standards", "limit=100")).map((row) => row.id); + + expect(mine).toContain(ours.id); + expect(mine).not.toContain(theirs.id); + expect(yours).toContain(theirs.id); + expect(yours).not.toContain(ours.id); + }); + + it("answers 404 for another organization's standard", async () => { + const theirs = await importStandard(globex, { + name: "Globex only", + edition: "1", + requirements: clauses(1), + }); + + const retrieved = await request(acme, `/standards/${theirs.id}`); + const requirements = await request(acme, `/standards/${theirs.id}/requirements`); + + expect(retrieved.status).toBe(404); + expect(requirements.status).toBe(404); + }); + + it.each([ + ["an id of the wrong shape", "not-an-id"], + ["an id carrying another table's prefix", "ctl_v1stgxr8z5jdhi6b"], + ])("answers 404 for %s", async (_case, id) => { + expect((await request(acme, `/standards/${id}`)).status).toBe(404); + }); +}); diff --git a/apps/server/standards.ts b/apps/server/standards.ts new file mode 100644 index 0000000..a449341 --- /dev/null +++ b/apps/server/standards.ts @@ -0,0 +1,307 @@ +// SPDX-FileCopyrightText: 2026 Quality Runtime contributors +// SPDX-License-Identifier: Apache-2.0 + +/** + * Standard routes. + * + * A standard arrives whole. Nobody enters ISO 9001 one clause at a time, so the + * only way to create one here is to import it with its requirements in a single + * request and a single transaction — reasoning in + * `docs/adr/0009-importing-a-standard.md`. + * + * Mounted under `/api/v1/organizations/:organizationId` behind + * `organizationContext`, like controls; row-level security scopes every query + * (ADR 0003), so nothing here filters by organization itself. + */ + +import { idPattern, schema } from "@qualityruntime/db"; +import { and, eq, getTableColumns, sql } from "drizzle-orm"; +import { Hono } from "hono"; +import { createMiddleware } from "hono/factory"; +import { z } from "zod"; +import { fieldsOf } from "./audit.ts"; +import type { OrganizationEnv } from "./organization.ts"; +import { + asStated, + collectionQuery, + type Ordering, + cursorAt, + newestFirst, + orderedBy, + page, + rowsAfter, +} from "./pagination.ts"; +import { failure } from "./responses.ts"; +import { jsonBody, prose, queryParams, rejection, words } from "./validation.ts"; + +/** + * How many clauses one import may carry. + * + * Bounds the rows inserted in one transaction, independently of the request + * body limit in `app.ts`: a small body can contain many short clauses. + */ +const maxRequirements = 2_000; + +// Strict, so a misspelt `text` is refused rather than imported as no text. +export const importBody = z.strictObject({ + name: words(200), + edition: words(100), + /** + * In the order the standard states them. `position` is assigned from this + * order rather than sent, because a client that has the clauses in order + * already knows it, and one that does not would be guessing. + */ + requirements: z + .array( + z.strictObject({ + reference: words(100), + title: words(500), + text: prose(50_000).nullish(), + }), + ) + .min(1) + .max(maxRequirements) + // A standard does not state the same clause twice, and the database says so + // too — but reaching it would be a 500 for what is plainly malformed input. + // Checked after trimming, because `1` and ` 1 ` are the same reference. + .refine((rows) => new Set(rows.map((row) => row.reference)).size === rows.length, { + message: "Two requirements share a reference.", + }) + .meta({ description: "In the order the standard states them; references must be distinct." }), +}); + +export const standardResponse = z.strictObject({ + id: z.string(), + organizationId: z.string(), + name: z.string(), + edition: z.string(), + requirementCount: z.int().nonnegative(), + createdAt: z.iso.datetime(), + updatedAt: z.iso.datetime(), +}); + +/** + * Whether any control is mapped to the requirement. + * + * `mapped`, not `covered`: a link says a control is meant to address a + * requirement, and whether that amounts to coverage is a judgement nothing here + * makes (ADR 0010). What this answers is the narrower, checkable question — + * which clauses of a standard nobody has taken up at all. + */ +const hasAControl = (mapped: boolean) => { + const link = sql`select 1 from "control_requirement" + where "control_requirement"."requirement_id" = "requirement"."id"`; + return mapped ? sql`exists (${link})` : sql`not exists (${link})`; +}; + +/** The collection query, plus the filters a standard's requirements take. */ +export const requirementsQuery = (ordering: Ordering) => + collectionQuery(ordering).extend({ + /** + * Resolves a clause as people cite it — `7.5.3` in a commit message — to + * the requirement it names, without reading the whole standard. Exact, + * because a reference's identity is its exact string (ADR 0008). + */ + reference: words(100) + .meta({ + description: + "Keep only the requirement with exactly this reference, after surrounding whitespace " + + "is removed. References are unique within a standard, so a page holds one or none.", + }) + .optional(), + mapped: z + .enum(["true", "false"]) + .meta({ + description: + "Keep only requirements with a mapped control, or only those with none. Draft, active, " + + "and retired controls all count. A mapping is not by itself a claim of coverage.", + }) + .transform((value) => value === "true") + .optional(), + }); + +/** A standard is a record of what the organization imported, and when. */ +export const standardOrder = newestFirst( + "standards", + schema.standard.createdAt, + schema.standard.id, +); + +/** + * Its requirements are a document, read in the order the document states. + * + * Scoped to the standard, not just to the collection: every standard numbers its + * clauses from one, so a cursor from another standard would be accepted on this + * one and resume from a position that means something else entirely. + */ +export const requirementOrder = (standardId: string) => + asStated(`requirements/${standardId}`, schema.requirement.position, schema.requirement.id); + +const isStandardId = new RegExp(idPattern("standard")); + +/** Stops an id that could not name a standard before it reaches PostgreSQL. */ +const knownStandardId = createMiddleware(async (c, next) => { + if (!isStandardId.test(c.req.param("standardId") ?? "")) { + return c.json(failure("not_found", "No such standard."), 404); + } + await next(); +}); + +/** + * How many requirements the standard states. + * + * A subquery rather than a stored counter: it cannot disagree with the rows, + * and the index on `(organization_id, standard_id, position, id)` serves it. + * + * The column names are written out rather than interpolated. Drizzle renders a + * single-table selection unqualified, which would turn the correlation into + * `"standard_id" = "id"` — both resolving to the subquery's own table, and a + * count of zero every time. + */ +const requirementCount = sql`( + select count(*)::int from "requirement" + where "requirement"."standard_id" = "standard"."id" +)`; + +/** What audit history records about a standard. */ +const audited = ["name", "edition"] as const; + +export const standards = new Hono() + .get("/standards", queryParams(collectionQuery(standardOrder)), async (c) => { + const { limit, cursor } = c.req.valid("query"); + + const found = await c.var.withOrganization((tx) => + tx + .select({ + ...getTableColumns(schema.standard), + requirementCount, + cursorAt: cursorAt(standardOrder), + }) + .from(schema.standard) + .where(cursor ? rowsAfter(standardOrder, cursor) : undefined) + .orderBy(...orderedBy(standardOrder)) + .limit(limit + 1), + ); + + const { rows, nextCursor } = page(found, limit, standardOrder); + return c.json({ data: rows, nextCursor }); + }) + + .post("/standards", jsonBody(importBody), async (c) => { + const body = c.req.valid("json"); + + const result = await c.var.withOrganization(async (tx) => { + const [created] = await tx + .insert(schema.standard) + .values({ + organizationId: c.var.member.organizationId, + name: body.name, + edition: body.edition, + }) + .returning() + .onConflictDoNothing({ + target: [schema.standard.organizationId, schema.standard.name, schema.standard.edition], + }); + // The unique constraint on (organization, name, edition) decided this, + // not a look-up beforehand that two imports could both pass. + if (!created) return { outcome: "duplicate" } as const; + + await tx.insert(schema.requirement).values( + body.requirements.map((requirement, index) => ({ + organizationId: created.organizationId, + standardId: created.id, + reference: requirement.reference, + title: requirement.title, + text: requirement.text ?? null, + // The order they were sent in, one-based so it reads like a document. + position: index + 1, + })), + ); + + // One event for the import, not one per clause: what happened is that a + // standard was imported (ADR 0009). + await c.var.audit(tx, { + action: "created", + resourceType: "standard", + resourceId: created.id, + after: { ...fieldsOf(created, audited), requirementCount: body.requirements.length }, + }); + + return { + outcome: "imported", + row: { ...created, requirementCount: body.requirements.length }, + } as const; + }); + + if (result.outcome === "duplicate") { + return c.json( + failure("already_exists", "That edition of that standard is already here.", [ + { + path: "", + message: + "That name and edition are already imported, and cannot be imported again or merged.", + }, + ]), + 409, + ); + } + return c.json({ data: result.row }, 201); + }) + + .get("/standards/:standardId", knownStandardId, async (c) => { + const [row] = await c.var.withOrganization((tx) => + tx + .select({ + ...getTableColumns(schema.standard), + requirementCount, + }) + .from(schema.standard) + .where(eq(schema.standard.id, c.req.param("standardId"))), + ); + if (!row) return c.json(failure("not_found", "No such standard."), 404); + + return c.json({ data: row }); + }) + + .get("/standards/:standardId/requirements", knownStandardId, async (c) => { + const standardId = c.req.param("standardId"); + // Parsed here rather than by middleware: the schema depends on which + // standard is being read, which only this handler knows. + const ordering = requirementOrder(standardId); + const query = requirementsQuery(ordering).safeParse(c.req.query()); + if (!query.success) return c.json(rejection("query", query.error), 400); + const { limit, cursor, mapped, reference } = query.data; + + // One snapshot: under `read committed` the check and the page are two + // statements, and a parent deleted between them would read as an empty one. + const found = await c.var.withOrganization( + async (tx) => { + // The standard has to be visible first, or its absence would read as a + // standard with no requirements. + const [standard] = await tx + .select({ id: schema.standard.id }) + .from(schema.standard) + .where(eq(schema.standard.id, standardId)); + if (!standard) return undefined; + + return tx + .select({ ...getTableColumns(schema.requirement), cursorAt: cursorAt(ordering) }) + .from(schema.requirement) + .where( + and( + eq(schema.requirement.standardId, standardId), + cursor ? rowsAfter(ordering, cursor) : undefined, + mapped === undefined ? undefined : hasAControl(mapped), + reference === undefined ? undefined : eq(schema.requirement.reference, reference), + ), + ) + .orderBy(...orderedBy(ordering)) + .limit(limit + 1); + }, + { repeatableRead: true }, + ); + if (!found) return c.json(failure("not_found", "No such standard."), 404); + + const { rows, nextCursor } = page(found, limit, ordering); + return c.json({ data: rows, nextCursor }); + }); diff --git a/apps/server/validation.ts b/apps/server/validation.ts index c57396f..3c71214 100644 --- a/apps/server/validation.ts +++ b/apps/server/validation.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 /** - * Request body validation for `/api/v1`. + * Request body and query validation for `/api/v1`. * * Hono's own `validator` does the plumbing; this only decides what a rejection * looks like, which is part of the API contract rather than of any one route. @@ -66,8 +66,8 @@ export const rejection = (what: string, error: z.ZodError) => /** * Parses a JSON body against `schema`, answering 400 when it does not fit. * - * The supplied schema decides how to handle unknown properties. Current - * request objects use `z.object`, which strips them for client compatibility. + * The supplied schema decides how to handle unknown properties: control + * bodies strip them, while standard imports reject them. */ export const jsonBody = (schema: T) => validator("json", (value, c) => { diff --git a/docs/adr/0006-cursor-paged-collections.md b/docs/adr/0006-cursor-paged-collections.md index bf742ce..34f9c85 100644 --- a/docs/adr/0006-cursor-paged-collections.md +++ b/docs/adr/0006-cursor-paged-collections.md @@ -24,7 +24,7 @@ GET /api/v1/organizations/{organizationId}/controls?limit=25&cursor=Y29udHJvbHM6 { "data": [ … ], "nextCursor": "Y29udHJvbHM6cmVjZW50fDIwMjYtMDktMThUMTA6MDk6MDAuMDAwMDAwWnxjdGxfMDAwMDAwMDAwMDAwMDAwNA" } ``` -`nextCursor` is null on the last page, so a client pages until it is null and never has to ask whether an empty page is coming. +`nextCursor` is null when the query finds no further row. Clients continue until it is null; a later page can still be empty if rows are deleted or stop matching a filter between requests. Paging is a live view, not a snapshot ([ADR 0011](0011-reading-a-mapping-from-both-ends.md)). **A cursor, not an offset.** `LIMIT`/`OFFSET` is correct only against a collection that is not changing. These are ordered newest first and written to constantly: a row inserted ahead of the offset between requests shifts the next page back onto an already-read row; deleting one ahead of it can skip an unread row. For an audit log — where the whole point is that nothing is missed — that is disqualifying. A cursor names a position in an ordering rather than a count from the start, so concurrent inserts cannot move it. @@ -48,10 +48,10 @@ A client cannot jump to page five, and there is no total. Both are real costs an The order is not a client's to choose. Sorting by name, or oldest first, would each need their own cursor encoding, because the cursor _is_ the ordering key. When a collection needs that, the cursor gains the key it sorts by and clients that treated it as opaque keep working — which is why it is opaque. A standard's requirements were the first to need it, and [ADR 0009](0009-importing-a-standard.md) records how an ordering became a value rather than a constant. -Filtering was not part of the initial implementation. A filter composes with this cleanly: it narrows the rows, the ordering key is unchanged, and the cursor still names a position. `?status=active` can be added to controls without revisiting any of this. The first filter to arrive was `?resource=` on history ([ADR 0018](0018-one-history-rather-than-one-per-record.md)), and it bore that out with one wrinkle worth carrying forward: a filtered collection is a _different_ ordering, so its name has to carry the filter or a cursor will cross between them. +Filtering was not part of the initial implementation. A filter composes with this cleanly: it narrows the rows, the ordering key is unchanged, and the cursor still names a position. `?status=active` can be added to controls without revisiting any of this. History binds its cursor name to `?resource=` to prevent reusing a cursor across different records' histories ([ADR 0018](0018-one-history-rather-than-one-per-record.md)). This is a collection-specific choice: a standard's `mapped` and `reference` filters retain the same ordering and cursor name, so cursors can be reused across those filters ([ADR 0011](0011-reading-a-mapping-from-both-ends.md)). > **Superseded by [ADR 0018](0018-one-history-rather-than-one-per-record.md).** `GET /controls/{controlId}/history` checked the control was visible before reading its history, and answered 404 when it was not. That is now one organization-wide collection with no record lookup: audit rows outlive the records they describe, which is exactly why requiring the record to still exist was wrong. The empty list this paragraph worried about conceals nothing — an out-of-tenant control produced one either way. -The ordering key is in the index, not just in the query. `control` is indexed on `(organization_id, created_at, id)` and `audit_event` on `(organization_id, resource_type, resource_id, created_at, id)` (see [`0000_schema.sql`](../../packages/db/migrations/0000_schema.sql)), so a page is a backward index scan with the row comparison as an index condition — no sort, and no scan of everything that matched. Without that, paging a large collection costs a full scan per page, which is the quadratic version of the thing paging exists to avoid. A future filter or ordering needs the same consideration: a cursor that the index cannot follow is slower than the offset it replaced. +The ordering key is in the index, not just in the query. `control` is indexed on `(organization_id, created_at, id)`. For history, `(organization_id, resource_type, resource_id, created_at, id)` supports reads for one resource; [ADR 0018](0018-one-history-rather-than-one-per-record.md) added `(organization_id, created_at, id)` for organization-wide reads (see [`0000_schema.sql`](../../packages/db/migrations/0000_schema.sql)). These indexes allow backward scans in the requested order, though the query planner chooses the execution plan. Future filters and orderings need the same consideration: a small page can still require scanning or sorting many rows when an index cannot supply it. The limit is capped at 100 and defaults to 25. The cap is what stops a page being a denial of service; the default is a guess, and the kind that is easy to change once something reads these collections in anger. diff --git a/docs/adr/0008-standards-and-requirements.md b/docs/adr/0008-standards-and-requirements.md new file mode 100644 index 0000000..7f6c26d --- /dev/null +++ b/docs/adr/0008-standards-and-requirements.md @@ -0,0 +1,61 @@ +# 8. Standards and requirements + +Date: 2026-09-18 + +## Status + +Accepted + +## Context + +`control` exists and has nothing to be _for_. A control is a measure an organization operates to meet a requirement, and until requirements exist the product is a list of measures answering to nothing. `requirement → control → evidence` is the loop that makes this a quality runtime rather than a CRUD application, and this is its first half. + +Standards are where a naive model goes wrong, so the shape matters more than the size: + +- **They have editions.** ISO 9001:2015 and its successor are neither the same standard nor unrelated ones. A record of conformity that does not say to which issue says very little. +- **Their text is usually copyrighted.** A deployment may hold a licence to read ISO 9001 without any right to store or redistribute its wording. +- **Not every standard is published.** An organization's own policies state requirements too, and they are not second-class. +- **Clause references are not identifiers.** `7.5.3` means something inside one standard and nothing outside it, and `7.10` sorts before `7.9`. + +## Decision + +Two tables, `standard` and `requirement`, both tenant-owned like everything else. + +**One row per issue.** `standard` carries `name` and `edition` and is unique on `(organization, name, edition)`. ISO 9001:2015 and ISO 9001:2026 are two rows. That is not a workaround for lacking a version model — it is the model: a requirement belongs to an issue of a standard, and a clause that changed between issues is a different requirement that happens to share a reference. + +**Every organization holds its own copy.** A shared catalogue of standards visible to all tenants was considered and rejected. It would mean a domain table whose `organization_id` is sometimes null, which breaks the single policy shape every tenant-owned table has ([ADR 0003](0003-tenant-isolation-with-row-level-security.md)) and the invariant that a domain row belongs to exactly one organization. It also sits badly with licensing, where the question is what _this_ deployment may hold. Distributing a standard is then an import — content that arrives from a file or a catalogue and becomes that organization's rows — rather than a second class of row with a second set of rules. The duplication is a few thousand rows per organization, which is nothing, and it leaves room for an organization to annotate its own copy later without a second class of row to put the annotation on. + +**The requirement text is nullable.** A licence to read is not a licence to store. A requirement tracked by reference and title alone is still worth having: once controls can answer to requirements, none of that needs the copyrighted wording to work. This is the one place the schema is shaped by a legal constraint rather than a domain one, and it is worth being explicit that it was deliberate. + +**Order is a column, not a derivation.** `position` records where a requirement falls in its standard. Sorting by `reference` puts `7.10` before `7.9`, and any scheme clever enough to fix that is a scheme that breaks on `A.5.1` or `CC6.1`. + +It is not unique within a standard. Allowing ties lets a clause use an occupied position without shifting later clauses. The tradeoff is that tied clauses are ordered by identifier, not by an additional author-assigned rank: the order is `(position, id)` in the index and in every reader. + +**A requirement carries its own `organization_id`, and a composite foreign key keeps it honest:** + +```sql +FOREIGN KEY ("standard_id", "organization_id") + REFERENCES "standard" ("id", "organization_id") ON DELETE CASCADE +``` + +Denormalising the organization onto a child row is what lets one policy shape serve every tenant-owned table — no join in a policy, no table needing its own. The composite key is what stops that being a lie: referencing `(id, organization_id)` rather than `id` alone makes a requirement in one organization pointing at a standard in another _impossible to write_, rather than merely wrong. `ARCHITECTURE.md` asks that cross-tenant relationships be impossible unless explicitly modelled; this is how a tenant-owned child does that. + +The pattern is for **relationships within one tenant**, and only those. A grandchild references its immediate parent the same way — `(parent_id, organization_id)`, with that parent carrying the matching unique constraint — rather than accumulating every ancestor. A reference to something instance-wide, `user` above all, takes an ordinary foreign key and is authorized separately: there is no organization on the other side to agree with. Nor does `ON DELETE CASCADE` follow automatically; it suits a requirement, whose existence is its standard's, and would be wrong for a relationship that should refuse deletion instead. + +The unique constraint this references is a table constraint rather than a unique index, because a constraint is part of `CREATE TABLE` and is therefore already there when the referencing table's foreign key is added. + +## Consequences + +Mapping a requirement across editions has no home yet, and will need one — an organization that moves from ISO 9001:2015 to its successor will want to know which controls carry over. That is a table relating two requirements, and it can be added without disturbing anything here, which is the point of not inventing it now. + +Requirements were not yet related to controls when this was written; [ADR 0010](0010-mapping-controls-to-requirements.md) relates them. `control_requirement` was the next table and the reason both of these exist; it should be keyed by its two foreign keys and carries nothing else until a mapping genuinely needs metadata. + +Nothing imported a standard when this was written; [ADR 0009](0009-importing-a-standard.md) added the import. Creating requirements one at a time is not how anyone enters a standard with 300 clauses, so the API for this is a bulk import rather than the create-one-at-a-time shape `control` has — which is exactly why the API was left out of this decision instead of being assumed to match. The constraints here stop a duplicate standard and a duplicate clause, but they cannot make an import atomic or resumable; that is the importer's transaction to own, and it also owns assigning positions. + +**Requirements need a different collection order.** [ADR 0006](0006-cursor-paged-collections.md) initially used `(created_at, id)` descending; a standard's clauses need `(position, id)` ascending. [ADR 0009](0009-importing-a-standard.md) added named orderings and integer cursor keys to support this, retaining the timestamp ordering for controls and standards. + +A standard and its requirements are ordinary mutable rows. Adopting an edition does not freeze it, and nothing marks one final (VERSION-01) — a typo in a title is meant to be fixable. What changed is recorded in `audit_event`: an import is one event for the standard ([ADR 0009](0009-importing-a-standard.md)), not one per clause. Deleting a standard deletes its requirements, by the cascade above. + +Identity is the exact string once surrounding whitespace is removed. `ISO 9001` and `ISO9001` are two standards, and nothing canonicalises spelling, case or inner whitespace beyond that. An importer is where that belongs, because only it knows what the input was meant to say. + +`text` being nullable means "not stored", not "does not exist", and nothing yet records _why_. If a deployment needs to distinguish "we are not licensed to store this" from "nobody has typed it in", that is a column, and it should be added when something acts on the difference rather than in anticipation of it. diff --git a/docs/adr/0009-importing-a-standard.md b/docs/adr/0009-importing-a-standard.md new file mode 100644 index 0000000..7a65336 --- /dev/null +++ b/docs/adr/0009-importing-a-standard.md @@ -0,0 +1,43 @@ +# 9. A standard is imported, not authored + +Date: 2026-09-18 + +## Status + +Accepted + +## Context + +[ADR 0008](0008-standards-and-requirements.md) settled what a standard and a requirement are, and deliberately left the API out: entering ISO 9001 is not the same activity as creating a control, and assuming the same shape would have been the mistake. + +It also predicted the harder half. [ADR 0006](0006-cursor-paged-collections.md) fixed every collection to `(created_at, id)` descending, and a standard's clauses are a document with an order of its own. That ADR said the cursor would gain the key it sorts by when a collection needed it. This is the collection. + +## Decision + +**A standard arrives whole.** `POST /standards` takes the standard and the requirements it states in one request, in one transaction. There is no way to create an empty standard and fill it, because a standard with no requirements states nothing, and a half-imported one is worse than none. A requirement's `position` is assigned from the order the clauses were sent in rather than supplied: a client with the clauses in order already knows it, and one without them in order would be guessing. + +**Re-importing is a conflict, not a merge.** A second import of the same name and edition answers 409. Merging would have to decide what a clause missing from the second import means — removed from the standard, or absent from this file — and neither answer is safe to guess. The decision is the database's: the insert relies on the unique constraint rather than a look-up beforehand that two concurrent imports could both pass. + +**One audit event per import.** What happened is that an organization imported a standard, not that three hundred clauses appeared. The event records the name, the edition and how many requirements came with it. Auditing each clause would bury every other event in the organization's history, and the clauses are not independently interesting until something changes one. + +**How much a route may read is chosen before any of it is read.** One figure cannot serve every route: 64 KiB refuses a legitimate standard, and a standard's allowance would let every other route accept one. So `app.ts` picks the limit per request — 1 MiB for an import, 64 KiB for everything else. + +Picking it there rather than on the route is not a stylistic choice. A limit declared on the import route is never reached, because the shared one runs first; and a generous limit in front of a strict one is simply the generous one, since a limiter with no `Content-Length` to go on buffers the stream before passing it down. Either arrangement leaves the strict bound decorative. + +The number of requirements is capped separately at 2,000 — one bound on bytes, one on rows, because a small body can still carry a great many tiny clauses. + +**Collections name their ordering, and a cursor carries it.** `Ordering` is now a value: a key column, the identifier that breaks its ties, a direction, and how the key is rendered into a cursor and read back. Two exist — `newestFirst` for a record of what has happened, `asStated` for a document with an order of its own — and a collection names itself in its cursors. + +That name is the part worth explaining. A position in one ordering is meaningless in another, which is obvious; less obvious is that it is equally meaningless in a _different collection ordered the same way_. Controls and standards are both newest first, so a cursor from one would be accepted by the other and would silently answer a list of standards from a position in the controls. Naming the collection is what makes that a 400. + +## Consequences + +Requirements page in `position` order, which ties — clauses are not renumbered to insert one — so the ordering key is `(position, id)` and the index [ADR 0008](0008-standards-and-requirements.md) added carries both. + +There is no way to change a standard once imported: no `PATCH`, no `DELETE`, no way to add a clause. That is not a claim that standards are immutable — the rows are ordinary and mutable, and the schema says so — only that nothing yet has a reason to write them. A correction to a published standard is a new edition; a correction to a typo is a real gap, and the shape it should take (edit the requirement, or re-import) is a decision for whoever needs it first. + +A 1 MiB import of 2,000 clauses is parsed and validated in memory before a row is written. That is the cost of atomicity, and it is bounded by both limits above. A standard too large for one request would need a different mechanism — a staged import with its own identity — and nothing suggests one exists. + +A duplicate clause reference is a bad request, not a constraint violation. The unique index would catch it, but as a 500 for what is plainly malformed input — so the import schema checks it after trimming, because `"1"` and `" 1 "` are the same reference. + +`requirementCount` on a standard is a subquery, not a stored counter. It cannot disagree with the rows, and the index on `(organization_id, standard_id, position, id)` serves it. If listing standards ever becomes slow, that is the thing to measure first. diff --git a/docs/adr/0010-mapping-controls-to-requirements.md b/docs/adr/0010-mapping-controls-to-requirements.md new file mode 100644 index 0000000..9f7ad0d --- /dev/null +++ b/docs/adr/0010-mapping-controls-to-requirements.md @@ -0,0 +1,61 @@ +# 10. Mapping controls to requirements + +Date: 2026-09-18 + +## Status + +Accepted + +## Context + +Controls exist. Standards and their requirements exist. Neither means very much alone: a control answering to nothing is a measure with no reason, and a requirement no control answers to is an obligation nobody has taken up. The join between them is what `requirement → control → evidence` has been building towards, and it is the first thing in this repository that lets the database say which obligations somebody has taken up. + +## Decision + +`control_requirement`, keyed by the pair, carrying only when the link was made. + +**No surrogate identifier.** The pair _is_ the identity, and a second row for the same pair would say nothing the first does not. [ADR 0002](0002-prefixed-identifiers.md) already exempted a join table keyed by its foreign keys, and `schema/index.test.ts` enforces that exemption rather than assuming it. + +**No metadata beyond `created_at`.** No rationale, no coverage strength, no "partially satisfies". Each would be a real thing to model and none has a use yet; a column added when something reads it is cheaper than one that turned out to mean the wrong thing. This is the same restraint `control` was built with, and the same reason. + +**Both references are composite**, and both borrow one `organization_id` ([ADR 0008](0008-standards-and-requirements.md)). A link can only exist if the control and the requirement it names are in that same organization — not as a rule the application applies, but as a row PostgreSQL will not store. This is what "cross-tenant relationships must be impossible unless explicitly modelled" looks like when the relationship has two tenant-owned ends. + +**Linking is not creating, so the API is `PUT`.** `PUT /controls/{controlId}/requirements` replaces the whole set: + +```json +{ "requirementIds": ["req_…", "req_…"] } +``` + +This is a genuine trade rather than an obvious win. Link and unlink can each be idempotent too, and they are better for a client that knows about one requirement and nothing else: they leave the rest of the mapping alone by construction. What `PUT` buys is that "these are the requirements this control answers to" — which is what a person editing a control actually means — is one statement of intent, applied whole or not at all, rather than a diff the client computes and applies in some order. Sending it twice changes nothing, including the history. + +The cost is last-writer-wins. Two people editing the same control a second apart do not corrupt anything — the writes serialise — but the second replaces the set the first stored, and a requirement the second editor never saw is removed without anything saying so. Locking prevents an inconsistent write; it cannot preserve an intent the request did not carry. A conditional write is the answer when that matters, and there is one. [ADR 0019](0019-conditional-writes.md) first left this route out — `If-Match` names the version of a row, and this replaces a set — and then gave the set a version of its own, computed from its contents. Listing a control's requirements serves that as an `ETag`, the same on every page of it, and a replacement quoting it is refused if the set has moved. A client that reads the set page by page and writes it back needs the same tag on every page, and reads again if one differs: a mapping added behind its cursor changes the tag on later pages without appearing in them. A client that wants the last writer not to win silently now has a way to ask. + +It is a set, not a sequence. Order carries no meaning and a repeated identifier is one identifier. + +**An unknown requirement is a 400 that names it.** The foreign key would refuse it anyway, but as a 500 — and it would refuse a requirement belonging to another organization identically, which is right, because the answer must not distinguish a requirement that is elsewhere from one that is nowhere. The check runs inside the same transaction as the write, so nothing is stored when part of the set is unknown. + +Reading them inside the transaction is not enough. Under `read committed`, another transaction may delete a requirement — or the standard stating it — between the check and the insert, and the foreign key raises after all. So the check takes `for key share` on the rows it validated: it blocks a delete until this transaction commits, without blocking anyone else reading them. Locking the control with `for update` serialises competing writes to the same control, which is a different race and does nothing about this one. + +**A mapping change is recorded on the control.** The link has no life of its own and no history worth reading separately, so `audit_event` gets one `updated` event against the control with the whole set before and after. Recording the difference instead would be smaller, but `before` and `after` mean "how it was" and "how it is" everywhere else, and a set is a value like any other. A request accepts at most 500 entries before duplicates are removed, bounding the set written through this API and its audit payload. + +Changing one link in a set of four hundred therefore stores eight hundred identifiers to express it. Nothing is lost — the difference is derivable, and deriving it is the reader's job — but anything rendering this history should show what was added and removed rather than two long lists. + +**Reading it back is `GET /controls/{controlId}/requirements`**, paged like every collection, ordered by the requirement's position in its own standard. + +## Consequences + +Requirements from different standards interleave in that listing, because a position is only meaningful within the standard that assigned it. Each row carries its `standardId`, which is what a client groups by. A cross-standard ordering would have to invent a rank for standards themselves, and nothing yet needs one. + +The reverse question — _which controls answer to this requirement?_ — is the same table read the other way, and the index for it is already there. It had no route when this was written; [ADR 0011](0011-reading-a-mapping-from-both-ends.md) gave it one, along with the unmapped-requirements read described below. + +Which requirements nobody has taken up was answerable but not answered when this was written: nothing reported which requirements of a standard have no control, which is the question a quality manager actually asks. [ADR 0011](0011-reading-a-mapping-from-both-ends.md) answered it with `?mapped=false`. The filter narrows the existing standard-requirements collection by checking for mappings; it does not add a separate route. + +Deleting a standard deletes its requirements, which deletes the links to them — a control simply stops answering to what is gone, silently. That is the right database behaviour and an uncomfortable product one: a control can lose its reason without anything being recorded, because the cascade is not a change the application made. Audit history for cascaded deletion is a real gap, recorded here rather than solved. + +Neither lock could be exercised when this was written: PGlite has one connection, so no interleaving was reachable and the tests could only assert that a lock was _taken_. [ADR 0020](0020-testing-races.md) changed that. A conditional remapping is now run against a competing transaction holding the control's lock, and refused after release. The other direction is exercised too, although no route deletes a requirement or a standard: a replacement naming a requirement whose standard is being deleted waits for the deletion and answers 400, where a plain read would have passed the check and met the foreign key as a 500. + +Importing a new edition of a standard leaves the old mappings exactly as they were, and creates none to the new edition's requirements. That is right — clauses that share a reference across editions are not the same requirement, and guessing they are would put a control's name against an obligation nobody checked — but it means adopting an edition is only half the work, and the remapping is manual with nothing to help. A tool for it is a real piece of work, and [ADR 0008](0008-standards-and-requirements.md) already noted that relating requirements across editions needs a table of its own. + +A retired control can still be remapped. Retiring freezes nothing else on a control — its name and description stay editable, and history records each change — so freezing only its mappings would be a rule of its own. If retirement comes to freeze what a control meant, it should freeze the control and its mappings together. + +`PUT` with an empty array clears the set, and is how a control is unlinked from everything. There is no `DELETE` on the collection, because it would mean the same thing twice. diff --git a/docs/adr/0011-reading-a-mapping-from-both-ends.md b/docs/adr/0011-reading-a-mapping-from-both-ends.md new file mode 100644 index 0000000..6881fbb --- /dev/null +++ b/docs/adr/0011-reading-a-mapping-from-both-ends.md @@ -0,0 +1,56 @@ +# 11. Reading a mapping from both ends + +Date: 2026-09-18 + +## Status + +Accepted + +## Context + +[ADR 0010](0010-mapping-controls-to-requirements.md) linked controls to requirements and read the mapping one way: the requirements a control answers to. It left the other direction unbuilt, and said why — a requirement was reachable only under its standard, and `/standards/{id}/requirements/{id}/controls` is not a URL anyone should have to type. + +That URL is a symptom. The question behind it is whether a requirement is a thing in its own right or a part of a standard, and the mapping had already answered: a control names requirements by identifier, and following an identifier should not require knowing which standard stated it. + +## Decision + +**Requirements get a path of their own**, beside controls and standards rather than beneath them: + +```text +GET /api/v1/organizations/{organizationId}/requirements/{requirementId} +GET /api/v1/organizations/{organizationId}/requirements/{requirementId}/controls +``` + +A requirement still belongs to exactly one standard, and `GET /standards/{id}/requirements` is still how you read a standard. What changes is that belonging to a standard is no longer the only way to reach one. `/standards/{id}/requirements` lists a document; `/requirements/{id}` identifies a thing. + +The controls answering to a requirement are ordered newest first. Controls have no order of their own — nothing states them in a sequence the way a standard states its clauses — so they get the ordering every record of what has happened gets. + +**And the first filter:** `GET /standards/{id}/requirements?mapped=false`. + +This is the first request in the API that answers a question rather than reporting what is stored. _Which clauses of this standard has nobody taken up?_ is the thing a quality manager asks first, and until now it could only be computed by a client reading every requirement and every mapping. + +**It is `mapped`, not `covered`.** A link says a control is _meant to address_ a requirement ([ADR 0010](0010-mapping-controls-to-requirements.md)). The filter asks whether any control is linked, regardless of whether it is draft, active, or retired. It makes no claim that the requirement is met or that evidence supports the mapping. + +The filter is a `not exists` against the mapping, correlated on the requirement. [ADR 0006](0006-cursor-paged-collections.md) said a filter composes with a cursor without disturbing it — the filter narrows the rows, the ordering key is unchanged, and the cursor still names a position — and this is the case that shows it: a filtered page is walked with the same cursor as an unfiltered one. + +**And a second: `?reference=`.** Engineers cite clauses by reference — `7.5.3`, `5.1-1` — in commits, pull requests and checklists, and a script linking a control to a cited clause needs the requirement it names. Without a filter that means reading the whole standard, up to 2,000 clauses, to find one. + +`GET /standards/{id}/requirements?reference=7.5.3` narrows the collection to that requirement, or to nothing. It is a filter rather than a lookup route answering 404, for the same reasons `mapped` is one: it composes with the other filter and the cursor, keeps one shape for "requirements of this standard", and an empty page is an honest answer to "does this standard state 7.5.3?". The match is exact after trimming — the same normalisation the import applied — and case-sensitive, because [ADR 0008](0008-standards-and-requirements.md) makes the exact string the identity. `(standard_id, reference)` is unique, so the index enforcing that answers it too. + +It stays inside one standard. The same reference means different things in different standards and editions, so a lookup across them would return several answers to what was asked as one question; a client names the edition it works to. One reference per request: this route reads the first of a repeated `reference`, not a list of them. + +## Consequences + +The subquery is inside the tenant context like every other read, so it counts only this organization's mappings. It has no `organization_id` predicate of its own and does not need one: it is correlated on the requirement, and a mapping can only name a requirement in its own organization, so another organization's mappings cannot reach it. The policies apply as well, and the index on `(organization_id, requirement_id, control_id)` serves it: the existence check stops at the first mapping rather than reading them all. + +**`limit` bounds the rows returned, not the work done.** Finding a page of unmapped requirements may require examining all remaining requirements in the standard when few match. The work depends on where matches fall and the query plan; the page size alone does not bound it. Imports are capped at 2,000 clauses. + +**A filtered walk is a live view, not a snapshot.** This is true of every collection here, and only noticeable once there is a filter. A requirement that someone maps while a client is walking `?mapped=false` leaves the walk before it is reached, which is right — it is no longer unmapped. One that is _unmapped_ behind the cursor does not reappear; it will be there on the next walk. A later page can come back empty for the same reason, and an empty page ends the walk rather than signalling a problem. Anything that needs a consistent picture of a moment — an export, a report someone signs — needs a snapshot, and nothing here offers one. + +`mapped=true` keeps requirements with at least one mapping; omitting `mapped` includes requirements with and without mappings. + +`GET /requirements/{id}` returns the requirement and nothing about its standard beyond `standardId`. A client rendering "clause 7.3 of ISO 9001:2015" needs a second request. Embedding the standard, or a `?expand=` of any kind, is a decision about the whole API rather than this route, and nothing is blocked on it. + +Neither of the reverse reads is filtered. _Which requirements does this control answer to, among those of one standard?_ and _which controls answering to this requirement are active?_ are both reasonable and neither is asked yet. + +What is still missing is the question one level up: how much of a standard is taken up, as a number rather than a list. That is a count, not a collection, and it does not fit the shape every route here has — which is worth noticing before inventing a route that pretends otherwise. diff --git a/docs/adr/0017-discarding-a-draft-control.md b/docs/adr/0017-discarding-a-draft-control.md index 6526cea..c0250e6 100644 --- a/docs/adr/0017-discarding-a-draft-control.md +++ b/docs/adr/0017-discarding-a-draft-control.md @@ -13,7 +13,7 @@ Accepted. A control's lifecycle was `draft → active → retired → draft` when this was written, and nothing else ([ADR 0003](0003-tenant-isolation-with-row-level-security.md) enforces the tenancy; the moves themselves are a rule the API keeps). There is no `DELETE`. -That left an abandoned draft with no disposal at all. Somebody starts authoring a control, thinks better of it, and the only way to be rid of it is to put it **into effect** and then retire it — which writes two events into audit history saying a control was in effect when it never was, and leaves a `retired` row claiming to have once covered something. The alternative is to leave it in the drafts forever, where it clutters the one list that is supposed to show what is being worked on. +That left an abandoned draft with no disposal at all. Somebody starts authoring a control, thinks better of it, and the only way to be rid of it is to put it **into effect** and then retire it — which writes two events into audit history saying a control was in effect when it never was, and leaves a `retired` row claiming to have once been in effect. The alternative is to leave it in the drafts forever, where it clutters the one list that is supposed to show what is being worked on. So one of two things had to change: allow `draft → retired`, or allow a draft to be deleted. @@ -23,19 +23,19 @@ So one of two things had to change: allow `draft → retired`, or allow a draft `DELETE /api/v1/organizations/{organizationId}/controls/{controlId}` answers `204` when the control never took effect, `409` when it did, and `404` when it is not there or belongs to someone else. -**A draft has to mean "never took effect", and the database has to hold that.** The first version tested `status = 'draft'` and was wrong: `retired → draft` was then a legal move, so `active → retired → draft` turned a control that _was_ in effect into a plain draft in three ordinary requests, and a status test let it be erased. A review found it by doing exactly that. The subtler version is the same hole in SQL: a policy is re-evaluated per statement, so an `UPDATE … SET status = 'draft'` followed by a `DELETE` in one transaction defeats a status test — the delete really does see a draft. +**A draft has to mean "never took effect", and the database has to hold that.** The first version tested `status = 'draft'` and was wrong: `retired → draft` was then a legal move, so `active → retired → draft` turned a control that _was_ in effect into a plain draft in three ordinary requests, and a status test let it be erased. The subtler version is the same hole in SQL: a policy is re-evaluated per statement, so an `UPDATE … SET status = 'draft'` followed by a `DELETE` in one transaction defeats a status test — the delete really does see a draft. -`control` therefore carries `activated_at`, when it first became active. For a while the policy tested that column instead of the status, which kept `retired → draft` and moved the whole meaning of "was ever in effect" into a timestamp. Repeated reviews argued the transition was the problem rather than the test, and it was removed: **the lifecycle is `draft → active → retired`, one way.** A retired control stays retired, and what replaces it is a new control, so evidence recorded against the old one keeps meaning what it meant. Reactivation can be added when a workflow needs it; a status that can be revived is probably misnamed. +`control` therefore carries `activated_at`, when it first became active. For a while the policy tested that column instead of the status, which kept `retired → draft` and moved the whole meaning of "was ever in effect" into a timestamp. That still allowed a previously active control to be called a draft, so the transition was removed: **the lifecycle is `draft → active → retired`, one way.** A retired control stays retired, and what replaces it is a new control, and the retired one stays a distinct record of what was in effect. Reactivation can be added when a workflow needs it; a status that can be revived is probably misnamed. -So the `DELETE` policy tests `status = 'draft'` again, and it means what it says because of two rules below the API: +So the `DELETE` policy tests `status = 'draft'` again, and it means what it says because of the rules below the API: -- **A trigger owns `activated_at`.** It stamps a control the first time it becomes active, by whatever path, and refuses any other write — a caller can neither backdate it, forge it, nor clear it. A second review found the need: `UPDATE … SET status = 'draft', activated_at = NULL` in one transaction erased a control that had been in effect, leaving no `deleted` event. A policy cannot prevent that, because `WITH CHECK` sees only the new row; a `BEFORE INSERT OR UPDATE` trigger is the only thing in PostgreSQL that can compare the two. +- **A trigger owns `activated_at`.** It stamps a control the first time it becomes active, by whatever path, and refuses any other write — a caller can neither backdate it, forge it, nor clear it. Allowing `UPDATE … SET status = 'draft', activated_at = NULL` would make a previously active control deletable again. A policy cannot prevent that, because `WITH CHECK` sees only the new row; a `BEFORE INSERT OR UPDATE` trigger is the only thing in PostgreSQL that can compare the two. - **A CHECK ties a draft to a null stamp** (`control_took_effect_unless_draft`). With the stamp immovable, nothing that took effect can become a draft again, and nothing is created active-and-unstamped or retired. - **The same trigger keeps a retired control retired.** A retired row keeps its stamp, so the CHECK would admit `retired → active`; only a trigger can see that the row was retired before. It is `control_lifecycle_enforce`: the two lifecycle rules that need a row's past, and nothing else. The route does not set the column at all, so there is no second place for the rule to be forgotten. -**Why not `draft → retired`.** `docs/data-model.md` already says what `retired` is for: _a control that once covered a requirement is part of the record_. That is the whole reason retiring beats deleting — there is something worth keeping. A control that never took effect was never relied on, never evidence of coverage, and nothing points at it. There is no record to preserve, so preserving it is not conservatism, it is clutter. And `retired` means "no longer in effect", which misdescribes something that never was: the two would become indistinguishable in exactly the list where the distinction matters. +**Why not `draft → retired`.** `docs/data-model.md` already says what `retired` is for: _a control that was once in effect is part of the record_. That is the whole reason retiring beats deleting — there is something worth keeping. A draft was never put into effect here, so retiring it would record that it had been. It is not nothing — it may carry mappings, and even evidence — but what it carries is dealt with deliberately: it can be discarded only once it carries no evidence, its mappings go by cascade, and its audit history stays. Keeping the row itself would not be conservatism, it would be clutter. And `retired` means "no longer in effect", which misdescribes something that never was: the two would become indistinguishable in exactly the list where the distinction matters. **The rule is a policy, not a route.** `control` had one `FOR ALL` policy, which cannot say _delete only these rows_. It is now four per-command policies, and the `DELETE` one admits only a draft. The route answers 409 with something a person can act on; PostgreSQL is what makes the rule true, which is the same shape as evidence finality ([ADR 0012](0012-evidence-and-attestation.md)) and audit append-only ([ADR 0005](0005-audit-history.md)). @@ -43,9 +43,7 @@ Splitting the policy costs nothing elsewhere: `SELECT … FOR UPDATE` is charged **A control carrying evidence is refused.** `evidence`'s foreign key to `control` is `ON DELETE restrict` ([ADR 0014](0014-the-runtime-role-owns-nothing.md)), so removing a control that has evidence fails in the database. The route asks first and answers `409 has_evidence`, because a foreign key violation surfacing as a 500 tells a client nothing. -The route holds `SELECT … FOR UPDATE` on the control while it asks, and an evidence insert needs `FOR KEY SHARE` on that same row for its own foreign key check, so the two cannot interleave. - -That was reasoning when this was written, and [ADR 0020](0020-testing-races.md) turned it into a test — which promptly showed it half wrong. The locks do exclude each other, but the recording took its key share only at the _insert_, having read its control unlocked: a discard landing in between left it inserting against a parent that was gone, and a foreign key violation became a `500`. It now takes the lock on the read and holds it, so whichever request loses the race loses cleanly — `404` when the control went first, `409 has_evidence` when the evidence did. +The deletion route holds `SELECT … FOR UPDATE` on the control while checking for evidence. Evidence creation takes `FOR KEY SHARE` when reading the control and holds it through insertion. Waiting until the foreign key check to take that lock would allow a deletion between the read and insert, producing a `500`. With both reads protected, the losing request answers `404` when the control was deleted first, or `409 has_evidence` when evidence was recorded first. [ADR 0020](0020-testing-races.md) records the tests for both interleavings. **So unattested evidence gained a `DELETE` of its own.** Without it this refusal was a dead end: a control that ever had evidence recorded against it could not be discarded at all, and the only escape was to activate and retire it — the exact thing this ADR exists to avoid. The policy was already there. `evidence`'s `DELETE` policy has admitted only unattested rows since [ADR 0012](0012-evidence-and-attestation.md); no route had ever asked. Attested evidence is still refused, and then the control stays too, which is correct: attested evidence must go on naming what it was evidence of. Discarding evidence takes its `file` rows by cascade, and the bytes stay on the volume, so the audit event names the filenames — the only record left of them, and [ADR 0018](0018-one-history-rather-than-one-per-record.md) is what made that event readable. diff --git a/docs/adr/0019-conditional-writes.md b/docs/adr/0019-conditional-writes.md index 3d3c714..1e1d251 100644 --- a/docs/adr/0019-conditional-writes.md +++ b/docs/adr/0019-conditional-writes.md @@ -27,17 +27,17 @@ The machinery to prevent that already existed — a row version, an entity tag, **The version is `xmin`.** It identifies the transaction that wrote the row version. Separate transactions receive different values until transaction IDs wrap; repeated updates within one transaction share a value. `updated_at` would not do: it is set from JavaScript, so it carries milliseconds, and two writes inside one millisecond would share a value — a stale tag that still matched. `xmin` is not durable across a dump and restore, which makes outstanding tags stale; that is the safe direction, since a write is refused and the caller reads again. Freezing does _not_ do that, despite the folklore: PostgreSQL marks a frozen tuple with a flag and leaves the `xmin` it reports alone, which a probe confirms across a `VACUUM FREEZE`. -**The comparison happens after the row is locked.** Every conditional route takes `SELECT … FOR UPDATE` before comparing, so nothing can move between the test and the write and the version does not need repeating in the `WHERE`. +**For amendments and deletions, the comparison happens after the row is locked.** These handlers take `SELECT … FOR UPDATE` before comparing, so nothing can move between the test and the write and the version does not need repeating in the `WHERE`. Attestation instead constrains its `UPDATE` by the version, as described below. -The first draft of this got the evidence discard wrong: it compared against an unlocked read and then deleted, which leaves the row free to be amended in between — a review reproduced exactly that with two sessions, and the delete removed evidence the caller had not seen. A conditional write that compares something it does not hold is not a conditional write. +Comparing against an unlocked read and then deleting allows a concurrent amendment between the two statements, deleting evidence the caller had not seen. The lock must span the comparison and deletion. -Evidence reads unlocked _first_, then locks, because `SELECT … FOR UPDATE` is governed by the `UPDATE` policy: an attested row is not there to lock, and "cannot be locked" would come back as "does not exist" rather than "cannot be changed". The unlocked read is what tells 404 from 409; the lock is what decides. Attestation repeats the version in its `WHERE` instead, because it never locks at all, for the same reason. +Evidence reads unlocked _first_, then locks, because `SELECT … FOR UPDATE` is governed by the `UPDATE` policy: an attested row is not there to lock, and "cannot be locked" would come back as "does not exist" rather than "cannot be changed". The unlocked read is what tells 404 from 409; the lock is what decides. Attestation repeats the version in its `UPDATE` predicate instead of taking an explicit lock before the comparison. **`*` means only if it still exists**, and only as the whole field — never one item of a list, which RFC 9110 does not allow and which would otherwise turn a list into an unconditional write. **Attesting does not use this parser at all**, and that is deliberate. It compares the header to the tag exactly, so `*` and a list are both refused even when the list holds the right tag. Everywhere else `If-Match` asks "has this moved?", and a wildcard meaning "only if it still exists" is a reasonable thing to ask. A signature is of something in particular, and "whatever version is there" is not a thing to sign. -The field is parsed rather than split. An entity tag is opaque and quoted, so a comma or an asterisk between the quotes is part of it: `"old,*,other"` is one tag that matches nothing, and splitting on commas exposes an `*` that was never a wildcard. That was the first draft's other mistake, and it let any write through. Comparison is strong — a weak tag never matches, and nothing here issues one — and anything that is not a well-formed field fails, including a header that is present and empty. A client that sent the header meant something by it, and writing anyway is the wrong way to be wrong. +The field is parsed rather than split. An entity tag is opaque and quoted, so a comma or an asterisk between the quotes is part of it: `"old,*,other"` is one tag that matches nothing, and splitting on commas exposes an `*` that was never a wildcard. Comparison is strong — a weak tag never matches, and nothing here issues one — and anything that is not a well-formed field fails, including a header that is present and empty. A client that sent the header meant something by it, and writing anyway is the wrong way to be wrong. **A tag covers the whole representation, not just its row.** Evidence reads back with its files, so attaching one changes the record — and `file` is a separate table, whose insert does not move `evidence`'s version. Without something to say otherwise, a caller could discard evidence carrying an attachment it never saw, and the file would cascade away with it. Attaching therefore touches the evidence row, which the audit event already called an update to the evidence; now the row agrees. @@ -47,7 +47,7 @@ The field is parsed rather than split. An entity tag is opaque and quoted, so a ## Consequences -`PUT /controls/{controlId}/requirements` is conditional too, by the route this paragraph originally ruled out. It replaces a set of mapping rows rather than amending one record, so there is no `xmin` to quote — the **contents are the version**: the sorted, length-prefixed member identifiers, hashed. Members are length-prefixed so that no member can impersonate two, and an empty set has a version of its own rather than colliding with a set holding an empty member; neither is reachable with the identifiers used today, and a version that collides is worse than no version. +`PUT /controls/{controlId}/requirements` was initially excluded because it replaces a set of mapping rows rather than amending one record, so there is no single `xmin` to quote. It now supports conditional writes using the **contents as the version**: SHA-256 of the sorted, deduplicated member identifiers encoded as a JSON array. JSON keeps member boundaries unambiguous and distinguishes an empty set from a set containing an empty string. That tag says nothing about _when_. Two equal sets are indistinguishable, which is exactly what a caller asking "is it still what I read?" means. @@ -55,7 +55,7 @@ That tag says nothing about _when_. Two equal sets are indistinguishable, which **The listing reads its page and its version from one snapshot.** Under `read committed` those are two statements and can see two different committed sets, which would hand a client a version for membership it was never shown. `withOrganization` takes a `repeatableRead` option for reads whose answers have to agree with each other. Reading one piece of evidence and listing a control's evidence use it too, for the same reason: a row and its separately queried attachments are two statements, and the tag an attestation quotes has to describe the files shown beside it. No write uses it — a write deciding from what is stored _now_ wants the opposite. -The version is selected alongside the columns, so one query serves both the body and the tag, and `withoutVersion` strips it before the response. A strict response schema would refuse it in the body ([ADR 0007](0007-openapi-from-the-schemas.md)), which is the backstop if that ever slips. +For row tags, the version is selected alongside the columns, so one query serves both the body and the tag; `withoutVersion` strips it before the response. Tests check representative responses against the strict published schemas ([ADR 0007](0007-openapi-from-the-schemas.md)); handlers do not validate outgoing responses at runtime. Nothing obliges a client to use this, so nothing guarantees a careless one is safe. That is the cost of optional, taken knowingly: the product now offers the guarantee rather than enforcing it, and a client that wants to be careful can be. diff --git a/docs/adr/0020-testing-races.md b/docs/adr/0020-testing-races.md index 31fd229..98804b3 100644 --- a/docs/adr/0020-testing-races.md +++ b/docs/adr/0020-testing-races.md @@ -21,13 +21,11 @@ One of those claims was wrong. [ADR 0019](0019-conditional-writes.md) asserted t **One suite runs against a real PostgreSQL, and only it does.** `apps/server/concurrency.test.ts` is skipped unless `TEST_DATABASE_URL` names a database — so `bun run test` still needs nothing running, and the property that makes the rest of the suite pleasant is kept. -**Each test forces the interleaving rather than hoping for it.** A second session takes the row lock; the request is started and blocks on it; the second session makes its change and commits; the request is released into a world that moved under it. Timing is never relied on. +**Lock-race tests force the interleaving rather than hoping for it.** A second session takes the row lock; the request is started and blocks on it; the second session makes its change and commits; the request is released into a world that moved under it. Timing is never relied on. -**A request that never blocked fails the test.** This is the part that matters, and the first version got it wrong. Starting a request only schedules it: without waiting for the request to actually block, the other session can finish before the handler has touched the database, and the two never overlap. Every test passed, and removing the locks they were written to exercise changed nothing. The suite now asks PostgreSQL who is waiting — `pg_stat_activity` where `wait_event_type = 'Lock'` — and gives up with an explicit failure if nobody is. +**A request that never blocked fails the test.** Starting a request only schedules it: the other session could finish before the handler touches the database, letting a test pass without exercising the lock. The helper captures the lock holder's backend PID and recursively follows `pg_blocking_pids` through `pg_stat_activity` in the current database. It requires the requested number of waiters in that holder's queue, including sessions blocked behind another waiter; an unrelated lock wait cannot satisfy the check. Failure to observe those waiters fails the test. -That check cannot filter by role, incidentally: the application switches role after connecting, so `usename` remains the login user. It filters on the database instead, which is sound because the only session deliberately holding a lock is not itself waiting on one. - -**It runs as a role that owns nothing and bypasses nothing**, set per connection, so the policies are in force as they are in a deployment. A superuser connection would be exempt from row-level security, and several of these handlers depend on it — an attested row cannot be locked, which is why evidence reads unlisted before it locks. +**It runs as a role that owns nothing and bypasses nothing**, set per connection, so the policies are in force as they are in a deployment. A superuser connection would be exempt from row-level security, and several of these handlers depend on it — an attested row cannot be locked, which is why evidence reads unlocked before it locks. **The database is wiped every run**, so the file refuses one whose name does not end in `_test`. @@ -41,7 +39,7 @@ The locks are now load-bearing in a way that can be checked. Removing `FOR UPDAT **It kept finding things.** A second round added the same foreign-key race one level down — attaching a file read its evidence unlocked and took the key share only at the insert, so a discard landing in between made it a `500`. Fixing that introduced a regression of its own, caught by review rather than by the suite: a locked read is governed by the `UPDATE` policy, which sees only unattested rows, so evidence attested mid-upload came back as "does not exist" rather than "already attested". The same trap this file warns about two paragraphs above, walked into while fixing something else. -And forcing a transaction to fail — by taking away the privilege its audit write needs — showed that `insufficient_privilege` was being read as "the evidence was attested in between" wherever it came from. It now checks the table too, so a privilege error elsewhere in the transaction is no longer answered with a confident wrong diagnosis. +Revoking the audit insert privilege during an upload verifies transaction rollback and byte cleanup. The handler lets that failure reach the API's internal-error response rather than interpreting it as a concurrent attestation; attestation and deletion conflicts are resolved by reading evidence state under the lock, with an unlocked re-read if the locked read returns nothing. **It found a third defect, in the race it was written to prove.** [ADR 0013](0013-durable-storage.md) argued that recording evidence and discarding its control cannot interleave, because the foreign key check takes `FOR KEY SHARE` and the discard holds `FOR UPDATE`. True as far as it went — but the recording read its control _without_ a lock and took the key share only at the insert, so a discard landing in between turned it into a foreign key violation and a `500`. It now takes that lock on the read and holds it, which makes the loser lose cleanly: `404` if the control went, `409 has_evidence` if the evidence did. diff --git a/docs/data-model.md b/docs/data-model.md index c08ff75..e2e5aa8 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -48,10 +48,26 @@ Domain tables are tenant-owned: each carries an `organization_id` referencing th PostgreSQL enforces the boundary: every table here has row-level security enabled and forced, and is reached through `withOrganization`, which scopes a transaction to one organization. A query that forgets its tenant predicate returns that organization's rows rather than everyone's, and a transaction with no organization set sees nothing. [ADR 0003](adr/0003-tenant-isolation-with-row-level-security.md) records the design; adding a tenant-owned table means adding its policy, and `migrations.test.ts` fails until you do. -| Table | Holds | -| ------------- | -------------------------------------------------------- | -| `control` | A measure an organization operates to meet a requirement | -| `audit_event` | A change to a record: who, what, when, and from what | +### Standard and requirement + +A standard is something an organization works to: a published one such as ISO 9001, or a policy it wrote itself. A requirement is one thing that standard asks for. Both are defined in `schema/standard.ts`; [ADR 0008](adr/0008-standards-and-requirements.md) records the design. + +**An edition is part of a standard's identity.** `standard` carries `name` and `edition` and is unique on `(organization, name, edition)`, so ISO 9001:2015 and ISO 9001:2026 are two rows rather than one row with a version. A requirement belongs to an issue, and a clause that changed between issues is a different requirement that happens to share a reference. Relating requirements across editions is not modelled yet. + +**Every organization holds its own copy.** There is no shared catalogue: a standard is imported into an organization — whole, with its requirements, in one request and one transaction ([ADR 0009](adr/0009-importing-a-standard.md)) — and becomes that organization's rows, keeping every domain row owned by exactly one organization. + +| Column | Holds | +| ----------- | ---------------------------------------------------------------------- | +| `reference` | How the standard refers to the requirement — `7.5.3`, `A.5.1`, `CC6.1` | +| `title` | What the requirement is about | +| `text` | The requirement as stated, where the deployment may store it | +| `position` | Where it falls in the standard's own order | + +`reference` is unique within a standard and means nothing outside it. It is how people cite a clause — in a commit message, a pull request, a checklist — so `?reference=` on a standard's requirements resolves one to its requirement by exact match ([ADR 0011](adr/0011-reading-a-mapping-from-both-ends.md)). `position` is **not** unique, so a clause can use an occupied position without renumbering later clauses. Requirements are ordered by `(position, id)`; ties are broken by identifier. `text` is **nullable on purpose**: the wording of a published standard is usually copyrighted, and a licence to read one is not a licence to store it — a requirement tracked by reference and title alone can still be mapped to controls. `position` exists because clause references do not sort: `7.10` precedes `7.9` lexically. + +A requirement carries its own `organization_id` and references its standard by `(standard_id, organization_id)` together, so a requirement in one organization pointing at a standard in another cannot be written at all (TENANT-01). Every tenant-owned child should be related to its parent the same way; a reference to something instance-wide, such as `user`, takes an ordinary foreign key instead. + +Both are ordinary mutable rows: importing an edition does not freeze it, and identity is the exact string once surrounding whitespace is removed — `ISO 9001` and `ISO9001` are two standards. Deleting a standard deletes its requirements. ### Control @@ -64,12 +80,12 @@ gone ◀── draft ──▶ active ──▶ retired ``` - `draft` — being authored; claims nothing. -- `active` — in effect, and may be relied on as evidence of coverage. -- `retired` — no longer in effect, but kept: a control that once covered a requirement is part of the record. Retiring is what deletion should usually be. +- `active` — in effect, and may be relied on. The status does not say a requirement is met or that the control was operated: its mappings record what it is meant to address, and its evidence records its operation. +- `retired` — no longer in effect, but kept: a control that was once in effect is part of the record. Retiring is what deletion should usually be. -The schema admits exactly these three values and no more. The API answers for the moves between them — the arrows above are the only ones — and setting the status a control already has is a no-op. The lifecycle runs one way, and PostgreSQL holds it to that whatever writes the row ([ADR 0017](adr/0017-discarding-a-draft-control.md)). A control in effect is withdrawn deliberately rather than quietly returned to draft, and a withdrawn one stays withdrawn: what replaces it is a new control, so the one evidence was recorded against keeps meaning what it meant. There is no `active → draft`, and nothing leaves `retired`. A control is always created as a `draft`; the create request cannot choose otherwise. +The schema admits exactly these three values and no more. The API answers for the moves between them — the arrows above are the only ones — and setting the status a control already has is a no-op. The lifecycle runs one way, and PostgreSQL holds it to that whatever writes the row ([ADR 0017](adr/0017-discarding-a-draft-control.md)). A control in effect is withdrawn deliberately rather than quietly returned to draft, and a withdrawn one stays withdrawn: what replaces it is a new control, and the retired one stays a distinct record of what was in effect. There is no `active → draft`, and nothing leaves `retired`. A control is always created as a `draft`; the create request cannot choose otherwise. -**A control that was never in effect can be discarded** ([ADR 0017](adr/0017-discarding-a-draft-control.md)). `DELETE` removes it outright, and answers 409 otherwise. The reason is the one above: retiring preserves a control that was once relied on, and one that never took effect was not — there is nothing to preserve, and calling it `retired` would claim it had been in effect. +**A control that was never in effect can be discarded** ([ADR 0017](adr/0017-discarding-a-draft-control.md)). `DELETE` removes it outright, and answers 409 otherwise. The reason is the one above: retiring preserves a control that was once in effect, and one that never took effect was not — calling it `retired` would claim it had been. The `DELETE` policy admits only a `draft`, and PostgreSQL is what makes "draft" mean "never took effect" rather than the API. `activated_at` records when a control first became active; a trigger sets it and refuses any other write to it, and a CHECK allows a draft exactly when it is null. So nothing that was in effect can be a draft again — not through the API, and not through raw SQL, including an `UPDATE` and a `DELETE` in one transaction. The trigger has to be a trigger: a policy cannot compare a row to what it used to be, and anything able to clear the stamp could turn a control that was in effect back into a deletable draft. @@ -83,6 +99,12 @@ Fields a quality system eventually wants — category, framework, test method, r A control row is mutable: editing one overwrites it. What it was is recorded in `audit_event` rather than kept on the row, so the history of a control is a query rather than a column. Versioned prior states (VERSION-01) — a numbered revision a reader can cite and return to — are still not implemented, and are a separate thing from the change log below. +### Control and requirement + +`control_requirement` records which controls answer to which requirements. It is read from both ends — the requirements a control answers to, and the controls answering to a requirement — and it is what `?mapped=false` uses to say which clauses of a standard nobody has taken up ([ADR 0011](adr/0011-reading-a-mapping-from-both-ends.md)). _Mapped_ is not _covered_: a link says a control is meant to address a requirement and nothing about whether it does. It is keyed by the pair and carries only when the link was made — no rationale, no coverage strength — until something reads one ([ADR 0010](adr/0010-mapping-controls-to-requirements.md)). Both references are composite and share one `organization_id`, so a link between organizations cannot be stored. + +The set is replaced whole rather than added to one link at a time, and a mapping change is recorded against the control, not the link. Deleting a control, a requirement, or a requirement's standard removes the links to it — silently, because a cascade is not a change the application made. + ### Audit event Domain API mutations record changes in the same transaction as the change itself ([ADR 0005](adr/0005-audit-history.md)). A change PostgreSQL makes on its own — a foreign key's cascade removing rows — writes nothing, which is a known gap rather than a decision. It names the actor, the action, the record, and the fields that moved. @@ -100,7 +122,7 @@ An administrator impersonating a member is the actor, because they are accountab Neither `actor_id` nor `resource_id` is a foreign key. Both the actor and the record can be deleted, and history that vanishes with them is not history (AUDIT-01) — `actor_label` exists for the same reason, since an identifier alone means nothing to a reader once the row is gone. -`before` and `after` carry a record's own fields, and for an update only the ones that differ. Record identity is already a column, and bookkeeping timestamps (`created_at`, `updated_at`) describe the write rather than the change. A domain timestamp — when something happened, rather than when it was written — is a field like any other. An update that changes nothing writes no event at all. +`before` and `after` carry a record's own fields — for a field edit, only those that differ. Payloads can also describe related records: a standard import records its requirement count, and a mapping update records the full requirement-id sets before and after the change. Record identity is already a column, and bookkeeping timestamps (`created_at`, `updated_at`) describe the write rather than the change. A domain timestamp — when something happened, rather than when it was written — is a field like any other. An update that changes nothing writes no event at all. History is readable at `GET /api/v1/organizations/{organizationId}/history`, newest first and paged like every other collection ([ADR 0006](adr/0006-cursor-paged-collections.md)). `?resource={id}` narrows it to one record, named by its identifier alone since the identifier says what kind it is. There is no per-record route and no 404: history outlives what it describes, so there is nothing to look a resource up in, and what a caller may see is decided by the policies ([ADR 0018](adr/0018-one-history-rather-than-one-per-record.md)). @@ -112,6 +134,8 @@ Amending or discarding a control accepts `If-Match`, and answers `412` when the The header is optional: omitting it leaves writes last-writer-wins. +`PUT /controls/{controlId}/requirements` replaces a set of rows rather than amending a record, so there is no single row version to quote. Its version is the set's contents instead, served as an `ETag` when the requirements are listed — the same on every page of them — and honoured on the replacement. A control therefore carries two versions, its own and its mappings', and they are not interchangeable. A client that reads the set page by page and writes it back needs the same tag on every page, and reads again if one differs: a mapping added behind its cursor changes the tag on later pages without appearing in them. + ### Not implemented yet Documents, risks, audits, findings, incidents, CAPAs, training, suppliers, approvals, and workflows. They join the same package and the same migration history. diff --git a/docs/deployment.md b/docs/deployment.md index 524f419..bab6936 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -76,7 +76,7 @@ Then, once the tables exist, take back what no policy would ever allow anyway: ```sql REVOKE UPDATE, DELETE ON "audit_event" FROM qualityruntime; -- append-only (ADR 0005) REVOKE UPDATE, DELETE ON "file" FROM qualityruntime; -- attached for good -REVOKE UPDATE ON "control_requirement" FROM qualityruntime; -- a link is made or unmade +REVOKE UPDATE ON "control_requirement" FROM qualityruntime; -- a link is made or unmade (ADR 0010) REVOKE DELETE ON "organization" FROM qualityruntime; -- see below ``` diff --git a/docs/development.md b/docs/development.md index 04c12d9..ea64580 100644 --- a/docs/development.md +++ b/docs/development.md @@ -111,11 +111,11 @@ bun run dev # http://localhost:3000, restarting on change It runs from the repository root so Bun loads the root `.env`, and it refuses to start when `DATABASE_URL`, `BETTER_AUTH_URL`, or `BETTER_AUTH_SECRET` is missing rather than failing on the first request that needs one. -`apps/server` mounts [Better Auth](https://better-auth.com) at `/api/auth/*`, and this product's own API at `/api/v1`. Tenant-owned resources — controls, and the history of what happened to them — sit under `/api/v1/organizations/:organizationId` behind `organizationContext`, which resolves the caller's membership and binds `withOrganization` to that organization ([ADR 0004](adr/0004-organization-in-the-request-path.md)); a route mounted outside that prefix has no `withOrganization` on its context and fails rather than serving unscoped rows. `apps/server/organization.test.ts` and `controls.test.ts` drive the stack over HTTP as a non-superuser role, so the policies apply there too; request bodies and query strings are validated with [Zod](https://zod.dev) through `validation.ts`, which owns what a rejection looks like, and collections are paged by cursor through `pagination.ts`, each naming the ordering it is read in ([ADR 0006](adr/0006-cursor-paged-collections.md)). `responses.ts` holds the shapes a response takes, as both what routes build and the schemas that describe them; `openapi.ts` assembles those into the document served at `/api/v1/openapi.json` ([ADR 0007](adr/0007-openapi-from-the-schemas.md)). Adding a route means adding its operation there too — `openapi.test.ts` derives what the app serves and fails until the two agree. A mutating handler also records what changed through `c.var.audit`, on the same transaction as the change ([ADR 0005](adr/0005-audit-history.md)); `audit.test.ts` covers that, including that the history cannot be rewritten. `authOptions` in `apps/server/auth.ts` is the schema contract — it decides which tables exist, and `auth.test.ts` derives its expectations from that same object. Better Auth refuses to start when the Drizzle schema object disagrees with it; that check reads the schema in code, not the live database, so applying migrations is still on you. +`apps/server` mounts [Better Auth](https://better-auth.com) at `/api/auth/*`, and this product's own API at `/api/v1`. Tenant-owned resources — controls, standards, requirements, and the history of what happened to them — sit under `/api/v1/organizations/:organizationId` behind `organizationContext`, which resolves the caller's membership and binds `withOrganization` to that organization ([ADR 0004](adr/0004-organization-in-the-request-path.md)); a route mounted outside that prefix has no `withOrganization` on its context and fails rather than serving unscoped rows. `apps/server/organization.test.ts` and `controls.test.ts` drive the stack over HTTP as a non-superuser role, so the policies apply there too; request bodies and query strings are validated with [Zod](https://zod.dev) through `validation.ts`, which owns what a rejection looks like, and collections are paged by cursor through `pagination.ts`, each naming the ordering it is read in ([ADR 0006](adr/0006-cursor-paged-collections.md), [ADR 0009](adr/0009-importing-a-standard.md)). `responses.ts` defines shared response envelopes and builds errors; resource modules define their response schemas, and handlers build successful responses. `openapi.ts` combines those schemas with operation metadata into the document served at `/api/v1/openapi.json` ([ADR 0007](adr/0007-openapi-from-the-schemas.md)). Adding a route means adding its operation there too — `openapi.test.ts` derives what the app serves and fails until the two agree. A mutating handler also records what changed through `c.var.audit`, on the same transaction as the change ([ADR 0005](adr/0005-audit-history.md)); `audit.test.ts` covers that, including that the history cannot be rewritten. `authOptions` in `apps/server/auth.ts` is the schema contract — it decides which tables exist, and `auth.test.ts` derives its expectations from that same object. Better Auth refuses to start when the Drizzle schema object disagrees with it; that check reads the schema in code, not the live database, so applying migrations is still on you. ## Testing races -Most of the suite runs on PGlite, which is a single connection: two things cannot happen at once, so nothing that depends on a lock has ever been exercised there. `apps/server/concurrency.test.ts` is the exception. It needs a real server, and it is skipped unless `TEST_DATABASE_URL` names one: +Most of the suite runs on PGlite, which uses a single connection and cannot exercise lock contention between transactions. `apps/server/concurrency.test.ts` is the exception. It needs a real server, and it is skipped unless `TEST_DATABASE_URL` names one: ```sh docker exec qualityruntime-postgres createdb -U postgres qualityruntime_test diff --git a/docs/product.md b/docs/product.md index 1df6644..1ce1d81 100644 --- a/docs/product.md +++ b/docs/product.md @@ -20,7 +20,7 @@ A requirement is what a standard asks for. A control is what the organization do ## Who it is for -**The person accountable for conformity** — a quality manager, a compliance lead, whoever has to say "yes, we do that, and here is why you should believe me". They need to see what is covered and what is not, and to produce a defensible record without assembling it by hand. +**The person accountable for conformity** — a quality manager, a compliance lead, whoever has to say "yes, we do that, and here is why you should believe me". They need to see what has been taken up and what has not, and to produce a defensible record without assembling it by hand. **The people who actually operate the controls** — engineers, administrators, anyone who performs the review or runs the restore test. For them the product must be quick and out of the way, or the evidence stops arriving. @@ -36,7 +36,7 @@ These are not aspirations. Each one is already a decision somewhere in `docs/adr **Record what happened rather than overwrite it.** History is not a feature. A control that was in effect and is now retired, and an attestation made by a person who has since left, are both part of the record — so retiring is what deletion usually means, and attribution survives the actor. -**Say what you know, and not more.** A control mapped to a requirement means somebody _intends_ it to address that requirement. It does not mean the requirement is met, and the product does not let that word creep in. A compliance score computed from mappings would be a number that means nothing, arrived at confidently. +**Say what you know, and not more.** A control mapped to a requirement means somebody _intends_ it to address that requirement. It does not mean the requirement is met, and the product does not let that word creep in — the filter is called `mapped`, not `covered`. A compliance score computed from mappings would be a number that means nothing, arrived at confidently. **Model small, and add when something needs it.** Every entity here is narrower than a quality system eventually wants: no owner on a control, no rationale on a mapping, no validity period on evidence. A field added when a workflow needs it is cheaper than one that turned out to mean the wrong thing. The absences are deliberate and written down. @@ -74,7 +74,7 @@ A managed service is planned, and what belongs to it is the operation rather tha ## Status -The schema for the whole loop is in place, and PostgreSQL enforces its tenancy and finality: standards, requirements, controls, mappings, evidence, attestation and files. The API serves the first part of it: controls can be created, changed, moved through their lifecycle and — while they never took effect — discarded, and every change is audited and readable as history. Standards, mappings, evidence and files are not yet reachable through the API. +The schema for the whole loop is in place, and PostgreSQL enforces its tenancy and finality: standards, requirements, controls, mappings, evidence, attestation and files. The API serves the first part of it: standards can be imported, and controls created, changed, moved through their lifecycle, mapped to the requirements they answer and — while they never took effect — discarded, and every change is audited and readable as history. Evidence and files are not yet reachable through the API. There is no user interface, no deployment artifact, and none of the entities beyond that loop. diff --git a/packages/db/schema/migrations.test.ts b/packages/db/schema/migrations.test.ts index d0f24ba..31ae059 100644 --- a/packages/db/schema/migrations.test.ts +++ b/packages/db/schema/migrations.test.ts @@ -315,9 +315,9 @@ describe("standards and requirements", () => { }); it("allows two requirements to share a position", async () => { - // Uniqueness here would mean renumbering every later clause to insert one, - // and a swap would need a spare value to pass through. The cost is that - // `position` alone is not an order, which is why the index carries the id. + // Ties let a clause use an occupied position without renumbering later + // clauses. The cost is that `position` alone is not an order, which is why + // the index carries the id. const [org] = await db.insert(organization).values(newOrganization()).returning(); const parent = await newStandard(org!.id); diff --git a/packages/db/schema/standard.ts b/packages/db/schema/standard.ts index 21dddb1..c3352b3 100644 --- a/packages/db/schema/standard.ts +++ b/packages/db/schema/standard.ts @@ -102,10 +102,9 @@ export const requirement = pgTable( * — `7.10` precedes `7.9` lexically — and rendering a standard out of order * is rendering a different document. * - * Not unique within a standard, deliberately: inserting a clause between - * two others, or swapping a pair, would otherwise need every row after it - * renumbered in the same statement. Ties are therefore possible, so the - * order is `(position, id)` and never `position` alone. + * Not unique within a standard: a clause can use an occupied position + * without renumbering later clauses. Ties are broken by identifier, so + * every read orders by `(position, id)`. */ position: integer("position").notNull(), createdAt: createdAt(),