diff --git a/adapters/adapter-core/package.json b/adapters/adapter-core/package.json index d9a0598..3554794 100644 --- a/adapters/adapter-core/package.json +++ b/adapters/adapter-core/package.json @@ -27,6 +27,6 @@ "scripts": { "build": "tsc -p tsconfig.json", "typecheck": "tsc -p tsconfig.json --noEmit", - "test": "tsc -p tsconfig.json && node test/static.test.mjs && node test/host.test.mjs && node test/whoami.test.mjs" + "test": "tsc -p tsconfig.json && node test/static.test.mjs && node test/host.test.mjs && node test/whoami.test.mjs && node test/delete-guard.test.mjs" } } diff --git a/adapters/adapter-core/src/server.ts b/adapters/adapter-core/src/server.ts index 49cd790..40ce3df 100644 --- a/adapters/adapter-core/src/server.ts +++ b/adapters/adapter-core/src/server.ts @@ -11,6 +11,7 @@ import { describeViolation, describeIncomplete, labelFieldOf, + onDeleteOf, schemaFingerprint, ERROR_CODES, type EntityPrint, @@ -59,6 +60,13 @@ const PROTOCOL_VERSION = "0.1"; * on. */ const DEFAULT_CREDENTIAL_HINT = "Read the app's .agent-token file (mode 0600) in the project directory."; +/** Page size for the referential scan a delete runs before it commits. */ +const REFERENCE_SCAN_PAGE = 500; +/** How many blocking record ids are reported per field. The answer is "yes, and + * here are examples" — a caller does not need ten thousand of them. */ +const REFERENCE_SCAN_LIMIT = 10; +/** Hard stop on paging, so a binding that ignores `page` cannot spin forever. */ +const REFERENCE_SCAN_MAX_PAGES = 200; const TASK_TIMEOUT_MS = 60_000; const TASK_MAX_DELIVERIES = 5; const DESCRIBE_PREFIX = "/api/_a2app/describe/"; @@ -584,6 +592,59 @@ export function createA2App(binding: Binding, config: A2AppConfig): A2App { /* ------------------------------------------------------------- records */ + /** + * Who still points at this record. + * + * Every `ref` and `list` in the model names the entity it targets, so the + * app has ALREADY told the framework where its references live — this reads + * that declaration rather than asking for a second one. + * + * Scans referencing entities page by page instead of filtering server-side: + * `filter` is interpreted by the binding, and a policy that silently did + * nothing against a binding whose filter grammar differed would be worse than + * no policy at all. Only entities that actually declare a `restrict` ref to + * this one are read, so an entity nothing points at costs nothing. + * + * Stops at {@link REFERENCE_SCAN_LIMIT} matches: the answer is "yes, and here + * are examples", and listing ten thousand blocking rows helps nobody. + */ + async function referencesTo(entity: string, id: string): Promise<{ entity: string; field: string; ids: string[] }[]> { + const model = binding.entities(); + const blockers: { entity: string; field: string; ids: string[] }[] = []; + + for (const [other, def] of Object.entries(model)) { + const pointing = def.fields.filter( + (f) => (f.type === "ref" || f.type === "list") && f.entity === entity && onDeleteOf(f) === "restrict", + ); + if (pointing.length === 0) continue; + + const found = new Map(); + let page = 1; + // Bounded: stop once every pointing field has its examples, or the pages + // run out. A page that comes back short is the last one. + for (;;) { + const result = await binding.listRecords(other, { page, perPage: REFERENCE_SCAN_PAGE }); + const items = result.items ?? []; + for (const row of items) { + for (const field of pointing) { + const value = (row as Record)[field.name]; + const hit = Array.isArray(value) ? value.includes(id) : value === id; + if (!hit) continue; + const ids = found.get(field.name) ?? []; + if (ids.length < REFERENCE_SCAN_LIMIT) ids.push(String(row["id"] ?? "")); + found.set(field.name, ids); + } + } + if (items.length < REFERENCE_SCAN_PAGE) break; + page++; + if (page > REFERENCE_SCAN_MAX_PAGES) break; + } + + for (const [field, ids] of found) blockers.push({ entity: other, field, ids }); + } + return blockers; + } + async function handleRecords( req: A2AppRequest, entity: string, @@ -635,6 +696,37 @@ export function createA2App(binding: Binding, config: A2AppConfig): A2App { if (req.method === "DELETE") { if (!recordId) return err(400, "usage", "DELETE requires a record id."); + + // The app's referential rules bind THIS door too. + // + // An app that guards deletion inside an operation has guarded exactly one + // way in: its own UI. This generic record route is the other, and it used + // to go straight to the store — so the rule held until an agent took the + // path the rule did not cover, and the orphan it left looked like a + // successful delete. The check belongs here, in the code an app author + // cannot edit, because that is the only place BOTH doors pass through. + const blockers = await referencesTo(entity, recordId); + if (blockers.length > 0) { + const total = blockers.reduce((n, b) => n + b.ids.length, 0); + const where = blockers.map((b) => `${b.entity}.${b.field}`).join(", "); + writeAudit(ctx, `data:${entity}`, recordId, "rejected", ERROR_CODES.RECORD_REFERENCED); + return err( + 409, + ERROR_CODES.RECORD_REFERENCED, + `Cannot delete ${entity} "${recordId}": ${ + total === 1 ? "a record still references" : `${total} records still reference` + } it (${where}).`, + { + referencedBy: blockers, + // Say what would make the delete legal, so the caller has a next + // move instead of a wall. + resolution: + `Remove or repoint the referencing records first, or run an operation the app provides for this. ` + + `An app that intends references to outlive the record declares \`onDelete: "ignore"\` on the ref.`, + }, + ); + } + const okDel = await binding.deleteRecord(entity, recordId); writeAudit(ctx, `data:${entity}`, recordId, okDel ? "ok" : "rejected"); if (!okDel) return err(404, "record_not_found", `No ${entity} record "${recordId}".`); diff --git a/adapters/adapter-core/test/delete-guard.test.mjs b/adapters/adapter-core/test/delete-guard.test.mjs new file mode 100644 index 0000000..0728734 --- /dev/null +++ b/adapters/adapter-core/test/delete-guard.test.mjs @@ -0,0 +1,175 @@ +/** + * Deleting a record that other records still point at. + * + * An app that guards deletion inside an operation has guarded one way in: its + * own UI. The generic record route — `DELETE /api/collections/{entity}/records/{id}`, + * reached by `a2app data delete ` — is the other, and it used + * to go straight to the store. So the rule held right up until an agent took the + * path the rule did not cover, and the orphan it left behind was reported as a + * successful delete. + * + * The check cannot live in the app: record deletion is adapter code, which + * toolkit-sync overwrites. So the app declares the relationship it already has + * to declare — a `ref` names the entity it targets — and the adapter enforces it + * on the one path both doors pass through. + * + * Standard library only, run directly. + */ +import { createA2App, MemoryBinding } from "../dist/index.js"; + +const failures = []; +const check = (label, got, want) => { + const g = JSON.stringify(got); + const w = JSON.stringify(want); + if (g !== w) failures.push(`${label}\n expected: ${w}\n actual: ${g}`); +}; +const ok = (label, cond) => { + if (!cond) failures.push(label); +}; + +/** An invoicing app: invoices point at a client, and may cite several projects. */ +const build = (onDelete) => + createA2App( + new MemoryBinding({ + appId: "billing", + appName: "Billing", + entities: { + clients: { + module: "sales", + fields: [{ name: "name", type: "string", required: true }], + seed: [ + { id: "c_acme", name: "Acme" }, + { id: "c_unused", name: "Nobody" }, + ], + }, + projects: { + module: "sales", + fields: [{ name: "title", type: "string", required: true }], + seed: [{ id: "p_one", title: "One" }], + }, + invoices: { + module: "sales", + fields: [ + { name: "total", type: "number" }, + // The relationship the app already declares. Nothing extra is asked + // of the author for the default to apply. + { name: "client", type: "ref", entity: "clients", ...(onDelete ? { onDelete } : {}) }, + { name: "projects", type: "list", entity: "projects" }, + ], + seed: [{ id: "inv_1", total: 100, client: "c_acme", projects: ["p_one"] }], + }, + }, + }), + { + modules: [{ name: "sales", summary: "clients and what they owe" }], + credentials: [ + { token: "t", credentialId: "cred", agentName: "test", principal: "owner", scopes: ["*"] }, + ], + }, + ); + +const call = (app, method, path, body) => + app.handle({ method, path, headers: { "x-a2app-token": "t" }, query: {}, body }); + +const del = (app, entity, id) => call(app, "DELETE", `/api/collections/${entity}/records/${id}`); + +/* ------------------------------------------- the reported hole, closed */ + +{ + const app = build(); + const res = await del(app, "clients", "c_acme"); + check("a referenced client is not deleted", res.status, 409); + check("and says why, by code", res.json.code, "record_referenced"); + ok("the message names the record", /c_acme/.test(res.json.message)); + ok("and where the references are", /invoices\.client/.test(res.json.message)); + + // A caller that is refused needs to know WHICH records are in the way; "no" + // with no location is a wall, not an answer. + // Read through a blank rather than indexing directly: when this regresses the + // delete simply succeeds, and a test that throws on the first missing field + // reports one TypeError instead of the list of things that are now wrong. + const by = Array.isArray(res.json.referencedBy) ? res.json.referencedBy : []; + const first = by[0] ?? {}; + ok("the blocking records are reported", by.length === 1); + check("naming the entity", first.entity, "invoices"); + check("and the field", first.field, "client"); + check("and the record ids", first.ids, ["inv_1"]); + ok("with a stated way forward", typeof res.json.resolution === "string" && res.json.resolution.length > 0); + + // The point of the guard: nothing was half-done. + const client = await call(app, "GET", "/api/collections/clients/records/c_acme"); + check("the client is still there", client.status, 200); + const invoice = await call(app, "GET", "/api/collections/invoices/records/inv_1"); + check("the invoice is untouched", invoice.status, 200); + check("still pointing at its client", invoice.json.client, "c_acme"); +} + +/* --------------------------------------------- what must still work */ + +{ + const app = build(); + const res = await del(app, "clients", "c_unused"); + check("a client nobody references still deletes", res.status, 200); + check("and reports what it deleted", res.json.deleted, "c_unused"); +} + +{ + // Removing the reference removes the block — the guard is about the current + // state, not a permanent mark on the record. + const app = build(); + const first = await del(app, "clients", "c_acme"); + check("blocked while the invoice exists", first.status, 409); + const dropped = await del(app, "invoices", "inv_1"); + check("the invoice itself deletes", dropped.status, 200); + const second = await del(app, "clients", "c_acme"); + check("and now the client does too", second.status, 200); +} + +/* ------------------------------------------------ list counts too */ + +{ + // A reference held in a list is still a reference. Checking only scalar refs + // would leave exactly the same hole, one type over. + const app = build(); + const res = await del(app, "projects", "p_one"); + check("a project cited in a list is not deleted", res.status, 409); + check("with the same code", res.json.code, "record_referenced"); + const cited = (Array.isArray(res.json.referencedBy) ? res.json.referencedBy : [])[0] ?? {}; + check("naming the list field", cited.field, "projects"); + check("and the record holding it", cited.ids, ["inv_1"]); +} + +/* --------------------------------------------- the deliberate opt-out */ + +{ + // An app that means for references to outlive the record says so, and then it + // is a decision recorded in the schema rather than an accident. + const app = build("ignore"); + const res = await del(app, "clients", "c_acme"); + check('onDelete "ignore" allows the delete', res.status, 200); + const invoice = await call(app, "GET", "/api/collections/invoices/records/inv_1"); + check("the invoice survives", invoice.status, 200); + check("with its reference left as declared", invoice.json.client, "c_acme"); + + // The opt-out is per field: the list never asked for it. + const stillGuarded = await del(app, "projects", "p_one"); + check("the other ref is still guarded", stillGuarded.status, 409); +} + +/* ------------------------------------------------------ absent record */ + +{ + const app = build(); + const res = await del(app, "clients", "c_missing"); + check("a record that does not exist is still 404, not 409", res.status, 404); + check("with its own code", res.json.code, "record_not_found"); +} + +/* ---------------------------------------------------------------- report */ + +if (failures.length > 0) { + console.error(`delete-guard: ${failures.length} failure(s)\n`); + for (const f of failures) console.error(" " + f); + process.exit(1); +} +console.log("delete-guard: all checks passed"); diff --git a/adapters/rules/src/index.ts b/adapters/rules/src/index.ts index 463b82c..f4282fc 100644 --- a/adapters/rules/src/index.ts +++ b/adapters/rules/src/index.ts @@ -78,6 +78,24 @@ export const PROTOCOL_TYPES = [ export type ProtocolType = (typeof PROTOCOL_TYPES)[number]; +/** Referential policies a `ref` may declare. See {@link NormalizedField.onDelete}. */ +export const ON_DELETE = ["restrict", "ignore"] as const; +export type OnDelete = (typeof ON_DELETE)[number]; + +/** + * What a `ref` does when nothing says otherwise: refuse the delete. + * + * Silently orphaning is the worse default of the two. It is invisible at the + * moment it happens, and the app that has to cope with it is the one reading the + * record weeks later. + */ +export const DEFAULT_ON_DELETE: OnDelete = "restrict"; + +/** The policy in force for a field — the declared one, or the default. */ +export function onDeleteOf(field: Pick): OnDelete { + return field.onDelete ?? DEFAULT_ON_DELETE; +} + /** The A2App error code registry. Clients branch on `code`, never on message * prose. Unknown codes are treated as rejection, never success. */ export const ERROR_CODES = { @@ -102,6 +120,8 @@ export const ERROR_CODES = { TASK_CANCELED: "task_canceled", AGENT_TOKEN_REQUIRED: "agent_token_required", RATE_LIMITED: "rate_limited", + /** The record is still referenced, and a `ref` pointing at it says `restrict`. */ + RECORD_REFERENCED: "record_referenced", } as const; /** @@ -120,6 +140,25 @@ export interface NormalizedField { values?: string[]; /** target entity for `ref` / `list` */ entity?: string; + /** + * What deleting the REFERENCED record does to this reference — `ref` and + * `list` only. Default {@link DEFAULT_ON_DELETE}. + * + * - `restrict` — refuse the delete while any record still points here. + * - `ignore` — allow it, and leave this reference dangling. + * + * It defaults to refusing because the alternative fails silently: the delete + * reports success and the damage surfaces later, somewhere else, as a record + * pointing at nothing. An app that genuinely wants dangling references says so + * in one word, and then it is a decision on the record rather than an accident. + * + * There is deliberately no `cascade` or `detach`. Both would have one delete + * quietly write to records the caller never named, which an agent cannot + * approve in advance and an audit log cannot explain afterwards. An app that + * wants that declares an operation, where it is named, described and approved + * like any other write. + */ + onDelete?: OnDelete; /** true for a `string` field carrying a `YYYY-MM-DD` day key */ dayKey?: boolean; } diff --git a/conformance/suites/class-a-core.yaml b/conformance/suites/class-a-core.yaml index bf0675f..22ad7fa 100644 --- a/conformance/suites/class-a-core.yaml +++ b/conformance/suites/class-a-core.yaml @@ -376,3 +376,69 @@ checks: expect: status: 404 match: { a2app: true, ok: false, code: unknown_operation } + + # --- Deletion: the app's referential rules bind the generic route too ------- + # Self-contained (creates what it deletes) so it does not depend on seed data + # or on where in the suite it runs. + # + # This route is the second door. An app that guards deletion inside an + # operation guards its own UI; `a2app data delete ` arrives + # here, and before this it went straight to the store — so the rule held until + # an agent took the path the rule did not cover, and the orphan it left was + # reported as a successful delete. + + - name: a list and a card that references it + request: + method: POST + path: /api/collections/lists/records + token: full + body: { title: "Doomed", position: 9 } + expect: + status: 200 + save: { doomedList: id } + + - name: the card that points at it + request: + method: POST + path: /api/collections/cards/records + token: full + body: { title: "Holds a reference", list: "${doomedList}" } + expect: + status: 200 + save: { holdingCard: id } + + - name: deleting the referenced list is refused, naming what blocks it + request: { method: DELETE, path: "/api/collections/lists/records/${doomedList}", token: full } + expect: + status: 409 + match: + a2app: true + ok: false + code: record_referenced + referencedBy.0.entity: cards + referencedBy.0.field: list + has: [resolution] + + - name: and the list is still there — the refusal left nothing half-done + request: { method: GET, path: "/api/collections/lists/records/${doomedList}", token: full } + expect: + status: 200 + match: { id: "${doomedList}" } + + - name: removing the reference removes the block + request: { method: DELETE, path: "/api/collections/cards/records/${holdingCard}", token: full } + expect: + status: 200 + match: { ok: true } + + - name: and now the list deletes + request: { method: DELETE, path: "/api/collections/lists/records/${doomedList}", token: full } + expect: + status: 200 + match: { ok: true, deleted: "${doomedList}" } + + - name: deleting a record that does not exist is 404, not 409 + request: { method: DELETE, path: /api/collections/lists/records/list_nope, token: full } + expect: + status: 404 + match: { code: record_not_found } diff --git a/toolkits/blueprint-pocketbase-react/template/pb/pb_hooks/_a2app.pb.js b/toolkits/blueprint-pocketbase-react/template/pb/pb_hooks/_a2app.pb.js index f534133..a56b82a 100644 --- a/toolkits/blueprint-pocketbase-react/template/pb/pb_hooks/_a2app.pb.js +++ b/toolkits/blueprint-pocketbase-react/template/pb/pb_hooks/_a2app.pb.js @@ -52,3 +52,9 @@ routerAdd("GET", "/api/_a2app/describe/{p1}/{p2}/{p3}/{p4}", (e) => // its `guard`, which either throws an ApiError (rejection) or calls `e.next()`. onRecordCreateRequest((e) => require(`${__hooks}/_a2app_impl.js`).guard(e)); onRecordUpdateRequest((e) => require(`${__hooks}/_a2app_impl.js`).guard(e)); + +// Delete was the unhooked one. An app that guards deletion inside an operation +// guards its own UI; the native records route is the other way in, and it went +// straight to the store — so the rule held until an agent took the path it did +// not cover, and the orphan that left was reported as a successful delete. +onRecordDeleteRequest((e) => require(`${__hooks}/_a2app_impl.js`).deleteGuard(e)); diff --git a/toolkits/blueprint-pocketbase-react/template/pb/pb_hooks/_a2app_impl.js b/toolkits/blueprint-pocketbase-react/template/pb/pb_hooks/_a2app_impl.js index 9b14023..b14663b 100644 --- a/toolkits/blueprint-pocketbase-react/template/pb/pb_hooks/_a2app_impl.js +++ b/toolkits/blueprint-pocketbase-react/template/pb/pb_hooks/_a2app_impl.js @@ -96,6 +96,20 @@ function identity(e) { const nf = { name: f.name, type: protocolType(f) }; if (f.required) nf.required = true; if (f.type() === "select" && f.values) nf.values = f.values; + // A relation's TARGET is part of what describe publishes, so it has to be + // part of what schemaVersion covers. `describe`'s own field mapping carries + // it and this one did not — and this is the copy the fingerprint is built + // from, so retargeting a ref at a different collection changed what describe + // published without moving the hash. A client caching against it kept a + // describe pointing the ref at the old collection, which the fingerprint's + // contract names as exactly the case it must not allow. + if (f.type() === "relation" && f.collectionId) { + try { + nf.entity = $app.findCollectionByNameOrId(f.collectionId).name; + } catch (_unresolved) { + /* a target that cannot be resolved is left absent, as before */ + } + } if (isDayKeyField(f)) nf.dayKey = true; return nf; } @@ -572,4 +586,99 @@ function guard(e) { e.next(); } -module.exports = { identity: identity, describe: describe, guard: guard }; +/** + * Refuse to delete a record that other records still point at. + * + * PocketBase serves records natively, so writes are guarded by hooking the + * request — and delete was not hooked at all. An app that refuses to delete a + * client with invoices inside its own operation guards one way in; + * `DELETE /api/collections/clients/records/{id}` is the other, and it went + * straight through, leaving invoices pointing at nothing and reporting success. + * + * The relationships are read from PocketBase itself: a `relation` field already + * names the collection it targets, so nothing extra is asked of the app. + * + * FAILS OPEN, deliberately. If the scan cannot run — an API shape this adapter + * did not expect, a collection it cannot read — the delete proceeds rather than + * being blocked. A guard that cannot run must not be able to brick every delete + * in an app: the worst case here is the behaviour that existed before this hook, + * never an app that can no longer delete anything. + */ +function deleteGuard(e) { + const rules = require(`${__hooks}/_a2app_rules.js`); + const target = e.record.collection().name; + const recordId = e.record.id; + // Examples, not an inventory: a caller does not need every blocking row. + const LIMIT = 10; + + const blockers = []; + try { + // Relation targets, read live. Built here rather than shared because this + // module is required fresh inside each pooled runtime. + const entities = {}; + const collections = $app.findAllCollections("base"); + for (let i = 0; i < collections.length; i++) { + const col = collections[i]; + if (col.name.indexOf("_") === 0) continue; // skip system collections + const names = col.fields.fieldNames(); + const byName = col.fields.asMap(); + const mapped = []; + for (let j = 0; j < names.length; j++) { + const f = byName[names[j]]; + if (f.system || f.type() !== "relation" || !f.collectionId) continue; + let targetName; + try { + targetName = $app.findCollectionByNameOrId(f.collectionId).name; + } catch (_unresolved) { + continue; // a relation whose target cannot be resolved blocks nothing + } + mapped.push({ name: f.name, type: f.maxSelect > 1 ? "list" : "ref", entity: targetName }); + } + entities[col.name] = { fields: mapped }; + } + + const pointing = rules.referencingFields(entities, target); + for (let k = 0; k < pointing.length; k++) { + const p = pointing[k]; + // `~` matches inside a multi-relation; `=` is exact for a single one. The + // id travels as a bound parameter, never spliced into the filter string. + const expr = p.list ? `${p.field} ~ {:id}` : `${p.field} = {:id}`; + const rows = $app.findRecordsByFilter(p.entity, expr, "", LIMIT, 0, { id: recordId }); + if (rows && rows.length) { + const ids = []; + for (let r = 0; r < rows.length; r++) ids.push(rows[r].id); + blockers.push({ entity: p.entity, field: p.field, ids: ids }); + } + } + } catch (err) { + try { + console.log(`a2app: referential delete check could not run for ${target} — allowing the delete: ${err}`); + } catch (_ignored) { + /* a logger that is not there must not become the failure */ + } + return e.next(); + } + + if (blockers.length) { + let total = 0; + const where = []; + for (let i = 0; i < blockers.length; i++) { + total += blockers[i].ids.length; + where.push(`${blockers[i].entity}.${blockers[i].field}`); + } + const said = total === 1 ? "a record still references" : `${total} records still reference`; + throw new ApiError(409, `Cannot delete ${target} "${recordId}": ${said} it (${where.join(", ")}).`, { + a2app: true, + ok: false, + code: "record_referenced", + referencedBy: blockers, + resolution: + "Remove or repoint the referencing records first, or run an operation the app provides for this. " + + 'An app that intends references to outlive the record sets the relation\'s onDelete to "ignore".', + }); + } + + e.next(); +} + +module.exports = { identity: identity, describe: describe, guard: guard, deleteGuard: deleteGuard }; diff --git a/toolkits/blueprint-pocketbase-react/template/pb/pb_hooks/_a2app_rules.js b/toolkits/blueprint-pocketbase-react/template/pb/pb_hooks/_a2app_rules.js index 1933ef5..6492849 100644 --- a/toolkits/blueprint-pocketbase-react/template/pb/pb_hooks/_a2app_rules.js +++ b/toolkits/blueprint-pocketbase-react/template/pb/pb_hooks/_a2app_rules.js @@ -262,7 +262,32 @@ function describeIncomplete(lost) { return "Rejected by a2app (not_stored): the database did not store " + names + ". Do NOT report this as done."; } -module.exports = { RULES_VERSION: RULES_VERSION, validate: validate, looksLikeDate: looksLikeDate, isDayKey: isDayKey, labelField: labelField, schemaVersion: schemaVersion, describeViolation: describeViolation, describeIncomplete: describeIncomplete, evaluatePredicate: evaluatePredicate, explainPredicate: explainPredicate }; +/** + * Which declared fields point at `target`, and would therefore be left dangling + * if a record of it were deleted. + * + * Pure so it can be self-tested: the hook that calls it cannot be, because it + * only runs inside PocketBase. `entities` is { name: { fields: [...] } } as the + * adapter maps it; a field opts out with onDelete "ignore". + */ +function referencingFields(entities, target) { + var out = []; + var names = Object.keys(entities || {}); + for (var i = 0; i < names.length; i++) { + var def = entities[names[i]]; + var fields = (def && def.fields) || []; + for (var j = 0; j < fields.length; j++) { + var f = fields[j]; + if (f.type !== "ref" && f.type !== "list") continue; + if (f.entity !== target) continue; + if ((f.onDelete || "restrict") !== "restrict") continue; + out.push({ entity: names[i], field: f.name, list: f.type === "list" }); + } + } + return out; +} + +module.exports = { RULES_VERSION: RULES_VERSION, validate: validate, looksLikeDate: looksLikeDate, isDayKey: isDayKey, labelField: labelField, schemaVersion: schemaVersion, describeViolation: describeViolation, describeIncomplete: describeIncomplete, evaluatePredicate: evaluatePredicate, explainPredicate: explainPredicate, referencingFields: referencingFields }; /* -------- gate self-test: `node _a2app_rules.js --selftest` -------- */ if (typeof process !== "undefined" && process.argv && process.argv.indexOf("--selftest") !== -1) { @@ -302,7 +327,31 @@ if (typeof process !== "undefined" && process.argv && process.argv.indexOf("--se failures.push("fingerprint ignores operation params"); } + // Referential deletes: which fields would be left dangling. The hook that + // uses this cannot run outside PocketBase; the decision it makes can. + var model = { + clients: { fields: [{ name: "name", type: "string" }] }, + invoices: { + fields: [ + { name: "client", type: "ref", entity: "clients" }, + { name: "projects", type: "list", entity: "projects" }, + ], + }, + }; + var blocking = referencingFields(model, "clients"); + if (blocking.length !== 1 || blocking[0].entity !== "invoices" || blocking[0].field !== "client") { + failures.push("referencingFields misses a ref pointing at the target"); + } + if (blocking.length && blocking[0].list !== false) failures.push("a scalar ref reported as a list"); + var listed = referencingFields(model, "projects"); + if (listed.length !== 1 || listed[0].list !== true) failures.push("referencingFields misses a list"); + if (referencingFields(model, "nobody").length !== 0) failures.push("referencingFields invents a blocker"); + var opted = { + invoices: { fields: [{ name: "client", type: "ref", entity: "clients", onDelete: "ignore" }] }, + }; + if (referencingFields(opted, "clients").length !== 0) failures.push('onDelete "ignore" must not block'); + if (failures.length) console.log("a2app_rules selftest FAILED:\n - " + failures.join("\n - ")); - else console.log("a2app_rules selftest: all rules pass (guard, predicates, fingerprint)"); + else console.log("a2app_rules selftest: all rules pass (guard, predicates, fingerprint, referential deletes)"); process.exit(failures.length ? 1 : 0); } diff --git a/toolkits/blueprint-python-fastapi/template/a2app_adapter.py b/toolkits/blueprint-python-fastapi/template/a2app_adapter.py index e783741..f48b86f 100644 --- a/toolkits/blueprint-python-fastapi/template/a2app_adapter.py +++ b/toolkits/blueprint-python-fastapi/template/a2app_adapter.py @@ -45,8 +45,28 @@ "TASK_CANCELED": "task_canceled", "AGENT_TOKEN_REQUIRED": "agent_token_required", "RATE_LIMITED": "rate_limited", + # The record is still referenced, and a `ref` pointing at it says `restrict`. + "RECORD_REFERENCED": "record_referenced", } +# What a `ref` does when nothing says otherwise: refuse the delete. Silently +# orphaning is the worse default -- it is invisible at the moment it happens, and +# the app that has to cope with it is the one reading the record weeks later. A +# field opts out with `onDelete: "ignore"`. +# +# There is deliberately no `cascade` or `detach`: both would let one delete write +# to records the caller never named, which an agent cannot approve in advance and +# an audit log cannot explain afterwards. That belongs in a declared operation. +DEFAULT_ON_DELETE = "restrict" + +# Page size for the referential scan a delete runs before it commits. +REFERENCE_SCAN_PAGE = 500 +# How many blocking record ids are reported per field: the answer is "yes, and +# here are examples", not a dump of every row in the way. +REFERENCE_SCAN_LIMIT = 10 +# Hard stop on paging, so a store that ignores `page` cannot spin forever. +REFERENCE_SCAN_MAX_PAGES = 200 + # --------------------------------------------------------------- pure rules _MONTH = [31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31] @@ -1130,6 +1150,55 @@ def dispatch(self, method: str, path: str, headers: dict, body: Optional[dict], return self._err(404, "not_found", "No such route.") # -- records ------------------------------------------------------------ + def _references_to(self, entity: str, rec_id: str) -> list[dict]: + """Who still points at this record. + + Every `ref` and `list` names the entity it targets, so the app has + ALREADY declared where its references live -- this reads that rather than + asking for a second declaration. + + Pages through referencing entities instead of filtering in the store: a + filter grammar differs per backend, and a policy that silently did + nothing against one of them would be worse than no policy. Only entities + that actually declare a `restrict` ref to this one are read, so an entity + nothing points at costs nothing. + """ + blockers: list[dict] = [] + for other, d in self.entity_defs.items(): + pointing = [ + f + for f in d["fields"] + if f.get("type") in ("ref", "list") + and f.get("entity") == entity + and f.get("onDelete", DEFAULT_ON_DELETE) == "restrict" + ] + if not pointing: + continue + + found: dict[str, list[str]] = {} + page = 1 + while True: + result = self.store.list_records(other, {"page": page, "perPage": REFERENCE_SCAN_PAGE}) + items = result.get("items") or [] + for row in items: + for f in pointing: + value = row.get(f["name"]) + hit = (rec_id in value) if isinstance(value, list) else (value == rec_id) + if not hit: + continue + ids = found.setdefault(f["name"], []) + if len(ids) < REFERENCE_SCAN_LIMIT: + ids.append(str(row.get("id", ""))) + if len(items) < REFERENCE_SCAN_PAGE: + break + page += 1 + if page > REFERENCE_SCAN_MAX_PAGES: + break + + for field, ids in found.items(): + blockers.append({"entity": other, "field": field, "ids": ids}) + return blockers + def _handle_records(self, method, headers, entity, rec_id, body, query): limited = self._rate_gate(headers, "data") if limited: @@ -1159,6 +1228,33 @@ def _handle_records(self, method, headers, entity, rec_id, body, query): if method == "DELETE": if not rec_id: return self._err(400, "usage", "DELETE requires a record id.") + + # The app's referential rules bind THIS door too. + # + # An app that guards deletion inside an operation has guarded one way + # in: its own UI. This generic record route is the other, and it used + # to go straight to the store -- so the rule held right up until an + # agent took the path the rule did not cover, and the orphan it left + # was reported as a successful delete. The check belongs here, in + # adapter code an app author cannot edit, because this is the only + # place both doors pass through. + blockers = self._references_to(entity, rec_id) + if blockers: + total = sum(len(b["ids"]) for b in blockers) + where = ", ".join(f"{b['entity']}.{b['field']}" for b in blockers) + said = "a record still references" if total == 1 else f"{total} records still reference" + return self._err( + 409, + ERROR_CODES["RECORD_REFERENCED"], + f'Cannot delete {entity} "{rec_id}": {said} it ({where}).', + referencedBy=blockers, + resolution=( + "Remove or repoint the referencing records first, or run an operation the app " + 'provides for this. An app that intends references to outlive the record ' + 'declares `onDelete: "ignore"` on the ref.' + ), + ) + ok = self.store.delete_record(entity, rec_id) if not ok: return self._err(404, "record_not_found", f'No {entity} record "{rec_id}".') @@ -1426,10 +1522,75 @@ def check(label: str, actual: Any, expected: Any) -> None: ): failures.append("fingerprint ignores operation params") + # 4. Referential deletes: the rule an app declares with a `ref` binds the + # generic record route, not just whatever operation the app wrote. This + # checks the scan itself -- the thing a delete consults before it commits. + class _FakeStore: + def __init__(self, rows): + self.rows = rows + + def list_records(self, entity, query): + items = list(self.rows.get(entity, {}).values()) + per_page = int(query.get("perPage") or len(items) or 1) + page = int(query.get("page") or 1) + start = (page - 1) * per_page + return {"items": items[start:start + per_page], "totalItems": len(items)} + + class _FakeAdapter: + _references_to = Adapter._references_to + + def __init__(self, entity_defs, store): + self.entity_defs = entity_defs + self.store = store + + invoice_fields = [ + {"name": "client", "type": "ref", "entity": "clients"}, + {"name": "projects", "type": "list", "entity": "projects"}, + ] + defs = { + "clients": {"fields": [{"name": "name", "type": "string"}], "module": "sales"}, + "projects": {"fields": [{"name": "title", "type": "string"}], "module": "sales"}, + "invoices": {"fields": invoice_fields, "module": "sales"}, + } + store = _FakeStore({ + "clients": {"c_acme": {"id": "c_acme"}, "c_unused": {"id": "c_unused"}}, + "projects": {"p_one": {"id": "p_one"}}, + "invoices": {"inv_1": {"id": "inv_1", "client": "c_acme", "projects": ["p_one"]}}, + }) + ad = _FakeAdapter(defs, store) + + check( + "a referenced record reports who blocks it", + ad._references_to("clients", "c_acme"), + [{"entity": "invoices", "field": "client", "ids": ["inv_1"]}], + ) + check("an unreferenced record blocks nothing", ad._references_to("clients", "c_unused"), []) + check( + "a reference held in a list counts too", + ad._references_to("projects", "p_one"), + [{"entity": "invoices", "field": "projects", "ids": ["inv_1"]}], + ) + + # The opt-out is per field, and it is the only way to allow orphaning. + ignoring = {**defs, "invoices": {"fields": [ + {"name": "client", "type": "ref", "entity": "clients", "onDelete": "ignore"}, + invoice_fields[1], + ], "module": "sales"}} + check( + 'onDelete "ignore" removes the block', + _FakeAdapter(ignoring, store)._references_to("clients", "c_acme"), + [], + ) + check( + "and leaves the other ref guarded", + _FakeAdapter(ignoring, store)._references_to("projects", "p_one"), + [{"entity": "invoices", "field": "projects", "ids": ["inv_1"]}], + ) + if failures: print("a2app_adapter selftest FAILED:\n - " + "\n - ".join(failures)) return 1 - print("a2app_adapter selftest ok (guard, predicates, fingerprint)") + print("a2app_adapter selftest ok (guard, predicates, fingerprint, referential deletes)") return 0 diff --git a/toolkits/blueprint-react-node/template/a2app.schema.mjs b/toolkits/blueprint-react-node/template/a2app.schema.mjs index 571201e..583d472 100644 --- a/toolkits/blueprint-react-node/template/a2app.schema.mjs +++ b/toolkits/blueprint-react-node/template/a2app.schema.mjs @@ -6,6 +6,23 @@ * `describe` and `schemaVersion` are DERIVED from this, so an agent always sees * the live model — you never hand-write describe. * + * REFERENCES GUARD DELETION. A `ref` (or `list`) names the entity it points + * at, and that declaration is enforced: deleting a record something still points + * at is refused with `record_referenced`, naming the records in the way. You do + * not write that check, and you cannot be routed around it — it runs inside the + * adapter, so it holds for your own UI and for `a2app data delete` + * alike. Guarding deletion only inside an operation guards one of those two. + * + * If an app genuinely wants references to outlive the record, say so on the + * field and the delete is allowed: + * + * { name: "client", type: "ref", entity: "clients", onDelete: "ignore" } + * + * There is no `cascade` or `detach` on purpose: both would let one delete write + * to records the caller never named, which an agent cannot approve up front and + * an audit log cannot explain later. Declare an operation for that instead, + * where it is named, described and approved like any other write. + * * MODULES COME FIRST. Every entity names the module it lives in, and every * operation names the module it appears under; the modules themselves are * declared in `manifest.json`. That is what gives describe a root screen to