diff --git a/.changeset/23857-comment-parent-pair.md b/.changeset/23857-comment-parent-pair.md new file mode 100644 index 00000000000..648c724ad5d --- /dev/null +++ b/.changeset/23857-comment-parent-pair.md @@ -0,0 +1,25 @@ +--- +'@objectstack/plugin-audit': minor +'@objectstack/cli': minor +--- + +feat(plugin-audit): `sys_comment` rows carry a writer-stamped parent pair, `object_name` + `record_id`, and the comment-thread read gate narrows by it with one `ReadableIds` leaf per parent object, with no pre-scan (ADR-0139 E7) + +**Run `os migrate comment-parent-pair --apply` at once after upgrading.** After the upgrade, and until it runs, the `sys_comment` rows written before the upgrade carry no pair, and the read gate answers them as it answered every comment before: by a SYSTEM pre-scan of at most 2,000 candidate rows per read, fail closed. So a broad comment list — one not scoped to a single record's thread — can omit comments the reader may see, and its `total` agrees with the short list; a record's own comment thread is unaffected. The door is `os migrate comment-parent-pair`, dry run by default, `--apply` to write, idempotent, and it stamps both columns in one run, keeping each row's `updated_at`. Its flags are `os migrate activity-changed-fields`'s, one for one (`--database-url`, `--apply`, `--yes`, `--force`, `--json`). The columns arrive with the upgraded deployment's schema sync, which an `--apply` run performs itself; a dry run against a database no upgraded process has opened cannot read them and exits `1`. + +Clause-②: yes + +`sys_comment` gains two `readonly` text columns and an index over them. They are derived from `thread_id` (`{object}:{record_id}`, split on the first colon) by one planner, `planCommentParentPair`, and `thread_id` stays the key authors write: nothing changes for a writer. A `sys_comment` before-insert hook stamps every inserted row, whoever writes it, over whatever value the writer sent; an update that names `thread_id` (a re-point) is re-stamped from it; and an update that names either column without a thread has the pair re-derived from the row's stored thread, for every caller — a historical import (`preserveAudit`) included, where the engine's update-side `readonly` strip keeps a value — so no writer's value for either column lands. A multi-row update that names either column without a thread is refused whole (`VALIDATION_ERROR` / 400, nothing written) when its rows' threads name different records, since one write cannot store each row's own pair; over rows sharing one thread it stores that thread's pair. A thread that names no record stamps `null` / `null`, and such a comment stays hidden from every non-system reader, as before. + +The comment-thread read gate (`installCommentReadVisibility`) is now the parent-record gate the activity stream and the compliance ledger use, over the comment's own pair: one branch `{ object_name: P, record_id: { $in: ReadableIds.of(P) } }` per registered object other than `sys_comment`, ORed and ANDed into the read's `where`, on `find`, `findOne`, `count` and `aggregate`. A broad comment read is served every comment the reader may read, on any table size, with a true `total`. What keeps the bounded 2,000-row fail-closed probe, chosen up front by name, never by catching a refusal: + +- ADR-0139 D5's three classes: a delegated (on-behalf-of) caller (the whole read); a parent narrowed by an object-scoped read middleware (`sys_attachment`, `sys_comment_reaction`, `sys_approval_request`, …); a parent bound to another datasource; +- a security service that answers no `canReadObject`, or none at all (no leaf is written without it), and an engine that cannot classify a parent — each sends the whole read to the probe and is said once; +- a row with no stamped pair (until the backfill reaches it): probed by its `thread_id`, restricted to rows with no pair, never leafed; +- a composition whose registered `sys_comment` declares no pair: the thread probe over every row, said once. + +**The cost:** a deployment whose driver compiles no `ReadableIds` leaf — `driver-mongodb`, until ADR-0139's O3 — now fails a non-system comment read with `INTERNAL_ERROR` / 500 naming the driver in the log, as the activity, ledger and attachment reads already do (ADR-0139 D3); the gate does not catch that refusal into an empty list. + +Comment reactions are unchanged: their only parent is `sys_comment`, which this gate narrows by an object-scoped middleware, so the reaction gate keeps the bounded probe (ADR-0139 D5.2). + +`installCommentReadVisibility` takes an optional third argument, the security source whose `canReadObject` admits a leaf (`ParentObjectReadAdmissionSource`); without it every comment read is the bounded probe, as before. Its first parameter widens from `CommentAccessEngine` to `ParentRecordGateEngine` (`CommentAccessEngine` plus three optional members: a registry view, `hasObjectMiddleware` and `resolveEffectiveDatasource`); every existing caller's engine still satisfies it. Both types reach the published declaration through that signature. `CommentAccessEngine` gains an optional `getSchema`. `@objectstack/plugin-audit` exports the backfill, `backfillCommentParentPair` (with `COMMENT_PARENT_PAIR_BACKFILL_PAGE_SIZE` and its report and engine types), the planner `planCommentParentPair` (with `COMMENT_PARENT_OBJECT_COLUMN`, `COMMENT_PARENT_RECORD_COLUMN` and the `CommentParentPair` type). diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index b2abb579384..7be92ac01ff 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -1194,6 +1194,7 @@ where the data lives. | `os migrate comment-reactions` | Carry the emoji reactions a 17.x deployment stored in the retired `sys_comment.reactions` column into `sys_comment_reaction` records, list every stored value that does not read, and record the verified run that lets `os migrate apply --allow-destructive` drop the column | | `os migrate organization-ownership` | The v18 ownership ceremony (ADR-0131 D10): write a plan of what each table's rows will become, then `--apply` that reviewed plan. A v18 server refuses to start until it has run. See [below](#os-migrate-organization-ownership) | | `os migrate activity-changed-fields` | Stamp the two read-rule columns (`changed_fields`, `milestone_key`) on the `sys_activity` rows written before they existed. Until it runs, those rows are withheld from every reader but the system, so run it at once after upgrading. No flag; idempotent | +| `os migrate comment-parent-pair` | Stamp the parent pair (`object_name`, `record_id`) on the `sys_comment` rows written before it existed. Until it runs, a broad comment read past 2,000 candidate rows omits comments the reader may see, and its total agrees with the short list, so run it at once after upgrading. No flag; idempotent | | `os migrate meta --stored` | Replay the metadata conversion chain over this deployment's `sys_metadata` rows and rewrite the ones still carrying a pre-protocol shape. Hygiene, not a gate — nothing depends on it having run | | `os migrate security-catalog-overlays` | List the environment-wide permission-set and position rows (the legacy plurals `permissions` / `positions` included) stored over a name a configured package holds — the rows a v18 cold boot refuses — each with the package that holds its name; `--apply` deletes them, one history tombstone per row. Run it before the first v18 boot, with the flags and environment the deployment boots with: the platform security plugin's sets are held only where `os serve` composes it (an auth secret set, and the `auth` tier on), and the step takes `serve`'s `--preset` and `--dev` with `serve`'s meaning. Adopts nothing; exits 1 while it lists a row | | `os migrate duplicates` | Report business identifiers already minted twice across the organization partitions, and the rows blocking the boot-time NULL-safe index tightenings — a read-only inventory as JSON on stdout. Renumbers nothing and writes no row and no schema; run it before the boot-time tenancy repair, which overwrites part of the evidence | @@ -1499,6 +1500,36 @@ os migrate organization-ownership --dev … # boot as `os ser the security and audit plugins), the development `.env*` files, and the `telemetry` sibling. Pass it for a database that `os dev` serves. +#### `os migrate comment-parent-pair` + +A comment is served to a reader only when the reader can see the record its +thread names. Since this version that check reads two columns the platform +stamps on every new comment from its `thread_id`, `object_name` and +`record_id`, and narrows a read in one statement, on any number of comments. +Rows written before the upgrade carry no pair, and the platform answers them +the way it answered every comment before: it examines at most **2,000 +candidate rows per read** and hides the rest (it never shows a comment the +reader may not see). So until this command runs, a broad comment list — one +not scoped to a single record — can omit comments the reader may see, and its +total agrees with the short list. A record's own comment thread is unaffected. + +```bash +os migrate comment-parent-pair # Dry run: counts, writes no rows +os migrate comment-parent-pair --apply # Stamp the rows (prompts) +os migrate comment-parent-pair --apply --yes --json # CI / scripts +``` + +Each row is stamped from its own `thread_id`, by the rule the platform stamps +new comments with; the row's `updated_at` is kept. **A comment whose thread +names no record stays unstamped**, and stays hidden from every reader but the +system, as before. The report counts those rows. + +**Idempotent**: only unstamped rows are read, so a second run stamps nothing, +and re-running is how an interrupted run is finished. The columns arrive with +the upgraded deployment's schema sync, which an `--apply` run performs itself: +a dry run against a database no upgraded process has opened cannot read them, +reports the failed read and exits `1`. A failed read or write exits `1`. + #### A database created by this version needs no migration A deployment whose database the platform **creates from empty** records these diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 4b14b834cd0..188515925b6 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -122,7 +122,7 @@ are reported as `undecidable` rather than assumed either way. The same holds twice over for the context. An options argument spelled as a literal can be read; one spelled `options`, `{ ...opts }`, or handed through a -forwarding shim cannot, and **53 of the 226 sites are spelled that way**. A +forwarding shim cannot, and **53 of the 227 sites are spelled that way**. A context resolved from an inline literal or a local `const` can be tested for `isSystem`; one arriving from a helper call cannot. @@ -187,10 +187,10 @@ reproduce them. Where it disagrees, it disagrees on the page: | carried figure | where it survives | this census | | :--- | :--- | ---: | -| 175 write call sites | quoted in the merged changeset | **226** | +| 175 write call sites | quoted in the merged changeset | **227** | | 24 carrying no tenant context | quoted in the merged changeset | **2** provable and tenancy-enabled; **32** more whose options argument is unreadable | -| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **155 of 226** decidable, **71** undecidable | -| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 122 decidably elevated, 0 decidably not, 96 undecidable | +| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **156 of 227** decidable, **71** undecidable | +| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 123 decidably elevated, 0 decidably not, 96 undecidable | | 141 and 132, two independent re-derivations | the card that filed this work | — | **The differences are not reconciled, and deliberately so.** The old census's @@ -201,17 +201,17 @@ at any commit. Two structural facts do plausibly widen this reading against any hand or regex one, and both are counted in the generated tables below: the 38 sites reached -through an erased (`any`) receiver, and the 51 that name their object through a +through an erased (`any`) receiver, and the 52 that name their object through a `const` rather than inline. An instrument that read either the way a person does would report a smaller number and would not say so. The fourth row is the one worth flagging to anyone citing it. **The 135 / 77% figure has no surviving corroboration anywhere in the tree.** This census reads -122 of 226 (54%) as decidably elevated, with 96 more whose elevation is a +123 of 227 (54%) as decidably elevated, with 96 more whose elevation is a run-time fact — so the claim is neither confirmed nor refuted, and the honest answer is that a static reading cannot settle it. -⇒ **Cite `2 / 226`, and say what it is**: the sites whose options argument was +⇒ **Cite `2 / 227`, and say what it is**: the sites whose options argument was READ and holds no tenant context, against a decidably tenancy-enabled object. That is the control's provable yield surface. ⛔ Do not cite it as "the sites without tenant context" — **32 further sites** have an options argument this @@ -223,29 +223,29 @@ cannot read, and they are neither in nor out. | what | count | | :--- | ---: | -| write call sites on the application surface | **226** | -| …whose object name is statically decidable | 155 | +| write call sites on the application surface | **227** | +| …whose object name is statically decidable | 156 | | …whose object name is chosen at run time | 71 | -| …against an object with tenancy ENABLED | 154 | +| …against an object with tenancy ENABLED | 155 | | …against an object that declares tenancy off | 1 | -| threading a tenant context | 165 | +| threading a tenant context | 166 | | PROVABLY carrying none (options read, no context key) | **8** | | …of those, against a decidably tenancy-enabled object | **2** | | options argument UNREADABLE — may or may not carry one | 53 | | …of those, against a decidably tenancy-enabled object | 32 | -| threading a decidably ELEVATED (`isSystem`) context | 122 | +| threading a decidably ELEVATED (`isSystem`) context | 123 | | threading a context that is decidably NOT elevated | 0 | | threading a context whose elevation is a run-time fact | 96 | | how the instrument reached the site | count | | :--- | ---: | -| receiver carried a readable engine type | 188 | +| receiver carried a readable engine type | 189 | | receiver erased, placed by the object NAME | 28 | | receiver erased, placed by an `object: string` PARAMETER | 5 | | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 5 | | object name spelled inline | 104 | -| object name spelled through a `const` | 51 | +| object name spelled through a `const` | 52 | | object name is an `object: string` parameter | 7 | | object name is some other run-time expression | 64 | @@ -297,12 +297,12 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-16 at `c53c594e7`. +Measured on 2026-10-16 at `caebc6a7b`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 654 | -| engine-shaped types recognised | 77 | +| tracked non-test sources scanned | 655 | +| engine-shaped types recognised | 78 | | declared objects in the registry | 116 | | same-named calls subtracted as non-engine | 165 | diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index 997257c9558..986c02c7c76 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -33,17 +33,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution. | Measure | Value | |---|---:| -| Write call sites | 226 | -| Object name statically decidable | 155 | +| Write call sites | 227 | +| Object name statically decidable | 156 | | Object name chosen at run time | 71 | -| Against a tenancy-enabled object | 154 | +| Against a tenancy-enabled object | 155 | | Against an object declaring tenancy off | 1 | -| Threading a tenant context | 165 | +| Threading a tenant context | 166 | | Provably carrying none | 8 | | …and decidably tenancy-enabled | 2 | | Options argument unreadable | 53 | | …and decidably tenancy-enabled | 32 | -| Threading a decidably elevated context | 122 | +| Threading a decidably elevated context | 123 | | Threading a decidably non-elevated context | 0 | | Threading a context of undecidable elevation | 96 | @@ -90,12 +90,12 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-16 at `c53c594e7`. +Measured on 2026-10-16 at `caebc6a7b`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 654 | -| engine-shaped types recognised | 77 | +| tracked non-test sources scanned | 655 | +| engine-shaped types recognised | 78 | | declared objects in the registry | 116 | | same-named calls subtracted as non-engine | 165 | @@ -117,6 +117,7 @@ Measured on 2026-10-16 at `c53c594e7`. | `packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts` | `update` | `objectPlan.object` | undecidable | context, elevation undecidable | 1 | | `packages/plugins/plugin-audit/src/activity-changed-fields-backfill.ts` | `update` | `sys_activity` | enabled | elevated | 1 | | `packages/plugins/plugin-audit/src/auth-event-audit.ts` | `insert` | `sys_audit_log` | enabled | options unreadable | 1 | +| `packages/plugins/plugin-audit/src/comment-parent-pair-backfill.ts` | `update` | `sys_comment` | enabled | elevated | 1 | | `packages/plugins/plugin-audit/src/read-audit.ts` | `insert` | `sys_audit_log` | enabled | options unreadable | 1 | | `packages/plugins/plugin-audit/src/stored-metadata-body-migration.ts` | `update` | `object` | undecidable | elevated | 1 | | `packages/plugins/plugin-auth/src/admin-import-users.ts` | `insert` | `sys_audit_log` | enabled | options unreadable | 1 | diff --git a/docs/qa/platform-checklist/areas/attachments-storage.json b/docs/qa/platform-checklist/areas/attachments-storage.json index aad9ea7c983..9cdabe7a6bc 100644 --- a/docs/qa/platform-checklist/areas/attachments-storage.json +++ b/docs/qa/platform-checklist/areas/attachments-storage.json @@ -267,7 +267,7 @@ "title": "sys_attachment reads inherit parent visibility: a restricted member sees neither rows nor counts for invisible parents", "since": "v15.1", "status": "active", - "revision": 4, + "revision": 5, "priority": "P1", "surface": "api", "personas": ["member who can read the parent record", "member who cannot read the parent record"], @@ -283,7 +283,8 @@ "knownGaps": [ "MARKER (QA run #9401 finding 5, carried here so runs stop re-deriving it): the CROSS-TENANT clauses of this area are structurally unprovable in the OSS workspace. `@objectstack/organizations` is open core since ADR-0132 (Apache-2.0, published on npm) — ⛔ it being closed-source is NOT the reason and has not been since #16215. The reason is ADR-0132's entitlement boundary: no framework package may declare `@objectstack/organizations` (pinned by `packages/plugins/organizations/src/no-framework-dependents.pin.test.ts` — apps declare it, packages do not), so `packages/qa/dogfood` cannot depend on it and the dogfood matrix's cross-tenant block is gated behind `describe.skipIf(!organizationsAvailable)` and skips BY DESIGN here. That is blocked(dependency), never a pass and never a defect. A run that is supposed to ship the package sets OS_TEST_MULTI_ORG_ENABLED=1, which turns the skip into a hard failure instead of a silent one.", "CLOSED by the qa-scratch-authz recipe (#7670): stock showcase's files-enabled parent (showcase_project) is public_read_write — every member sees every project, so the invisible-parent case is not demonstrable on stock seeds, and the pinned dogfood matrix proves it only on its own private fixture (att_secret). The recipe provisions the private files-enabled parent at runtime; landing one in the showcase seeds proper would retire this gap entirely, and remains the better long-term fix.", - "a broad read over an ordinary parent no longer meets the 2000-candidate cap (ADR-0139 E7, #23767): the gate writes one ReadableIds leaf per parent object and pre-scans nothing, so the scratch parents' broad read is served every visible row with a true total and no '[storage] attachment read visibility' warning. The bounded 2000-row fail-closed probe (READ_SCAN_LIMIT, clause 4's warning) remains for ADR-0139 D5's three classes only: a delegated (on-behalf-of) caller, a parent narrowed by an object-scoped read middleware (sys_comment, sys_comment_reaction, sys_activity and sys_audit_log among them), and a parent bound to another datasource than sys_attachment. None of the three is provisioned by the qa-scratch-authz recipe, so clause 4's probe half is not reachable from a run here: it is pinned in packages/services/service-storage/src/attachment-read-leaf.integration.test.ts and attachment-read-leaf.test.ts, and a run cites them. On driver-mongodb, which compiles no ReadableIds leaf until ADR-0139's O3 is measured, a member's sys_attachment read fails INTERNAL_ERROR / 500 naming the driver instead of serving a short list: a ruled limit (ADR-0139 D3, Consequences), never a pass and never a leak." + "a broad read over an ordinary parent no longer meets the 2000-candidate cap (ADR-0139 E7, #23767): the gate writes one ReadableIds leaf per parent object and pre-scans nothing, so the scratch parents' broad read is served every visible row with a true total and no '[storage] attachment read visibility' warning. The bounded 2000-row fail-closed probe (READ_SCAN_LIMIT, clause 4's warning) remains for ADR-0139 D5's three classes only: a delegated (on-behalf-of) caller, a parent narrowed by an object-scoped read middleware (sys_comment, sys_comment_reaction, sys_activity and sys_audit_log among them), and a parent bound to another datasource than sys_attachment. None of the three is provisioned by the qa-scratch-authz recipe, so clause 4's probe half is not reachable from a run here: it is pinned in packages/services/service-storage/src/attachment-read-leaf.integration.test.ts and attachment-read-leaf.test.ts, and a run cites them. On driver-mongodb, which compiles no ReadableIds leaf until ADR-0139's O3 is measured, a member's sys_attachment read fails INTERNAL_ERROR / 500 naming the driver instead of serving a short list: a ruled limit (ADR-0139 D3, Consequences), never a pass and never a leak.", + "two more states send the WHOLE read to the bounded 2000-row probe, up front, and the gate warns once per engine for each: a security service that answers no canReadObject (or no security service at all), because no ReadableIds leaf is written without that admission; and an engine that cannot classify a parent (no registry view, no hasObjectMiddleware or no resolveEffectiveDatasource). A stock boot composes both, so neither is reachable from this lane; a run that sees the once-per-engine warning has met one of them and scores the broad-read clause against the 2000-row bound, not as a pass. On the API surface the driver-mongodb refusal names its driver in the server LOG only: the response body is the ADR-0112 envelope (INTERNAL_ERROR / 500) and does not carry the driver name, so a run reads the driver from the log, never from the body." ] }, "steps": [ @@ -345,6 +346,12 @@ "date": "2026-10-16", "change": "clause 3's broad-read half re-pinned for ADR-0139 E7: the attachment gate writes one ReadableIds leaf per parent object and pre-scans nothing, so a broad read over an ordinary parent past 2000 candidates is served whole with a true total, and the scan-cap pin is rewritten for that side of the bound (its @proof tag kept). knownGaps and automated.ref now name the limits that remain: ADR-0139 D5's three classes keep the bounded 2000-row probe (pinned in service-storage), and driver-mongodb answers INTERNAL_ERROR / 500 until O3. The unknown-parent-object and outer-catch arms are restated for the new gate.", "ref": "#23767" + }, + { + "revision": 5, + "date": "2026-10-16", + "change": "knownGaps names the gate's two whole-read probe states (no canReadObject or no security service; an engine that cannot classify) and says that the driver-mongodb 500 names its driver in the server log, not in the response body — the two precision notes carried from the attachment landing's contract review. No clause, step or pin changes.", + "ref": "#23857" } ] }, diff --git a/docs/qa/platform-checklist/areas/cli.json b/docs/qa/platform-checklist/areas/cli.json index 20c882e9501..7c98539cb40 100644 --- a/docs/qa/platform-checklist/areas/cli.json +++ b/docs/qa/platform-checklist/areas/cli.json @@ -289,14 +289,15 @@ "organization-ownership", "security-catalog-overlays", "comment-reactions", - "activity-changed-fields" + "activity-changed-fields", + "comment-parent-pair" ], "traps": ["stale-dist"], "automated": { "kind": "unit", "ref": "packages/cli/src/utils/format.exit-code.test.ts" }, "source": [ "packages/cli/src/commands/migrate/index.ts + plan.ts (#2186 bare-command-is-plan; #3917 enforced never-mutates; occupancy warning) + apply.ts (--allow-destructive / --force / --yes / in_sync)", "packages/cli/src/utils/format.ts#CliExitCode (CliExitCode narrows the emitJson exit slot — the #4873 fix, commit 83df2fd)", - "packages/cli/src/commands/migrate/ (the 17 registered subcommands under `os migrate`, besides the bare command, enumerated as variants — oclif pattern strategy, one default-exported Command per non-test file)", + "packages/cli/src/commands/migrate/ (the 18 registered subcommands under `os migrate`, besides the bare command, enumerated as variants — oclif pattern strategy, one default-exported Command per non-test file)", "sibling item cli.migrate-duplicates-inventory (the duplicates subcommand's own contract — cross-referenced, not duplicated)" ], "history": [ diff --git a/packages/cli/src/commands/migrate/comment-parent-pair.integration.test.ts b/packages/cli/src/commands/migrate/comment-parent-pair.integration.test.ts new file mode 100644 index 00000000000..d4939ff11e6 --- /dev/null +++ b/packages/cli/src/commands/migrate/comment-parent-pair.integration.test.ts @@ -0,0 +1,250 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi } from 'vitest'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import Database from 'better-sqlite3'; +import { isExitSignal } from '../../utils/format.js'; +import MigrateApply from './apply.js'; +import MigrateCommentParentPair from './comment-parent-pair.js'; + +// Pay the first transform of the dist-resolved workspace deps the commands +// reach through dynamic `import()`s at MODULE LOAD, not inside a case. +import '@objectstack/runtime'; +import '@objectstack/objectql'; +import '@objectstack/plugin-auth'; +import '@objectstack/plugin-security'; +import '@objectstack/plugin-audit'; + +/** + * [ADR-0139 E7] The upgrade stamps the `sys_comment` rows written before the + * parent pair existed: `os migrate comment-parent-pair`. + * + * ## The database + * + * The schema this version creates (`os migrate apply --yes` over an empty + * file, the composition `os serve` mounts with an auth secret, so plugin-audit's + * objects are among it), then turned back into the shape a deployment upgraded + * from the previous version holds: the pair's index and both columns DROPPED + * from `sys_comment`, and comment rows planted the way the previous writer left + * them — a `thread_id` and no pair. A real SQLite file under the SQL driver the + * commands boot, because the raw column read is the point. + * + * ## The cases run in order over ONE database, as an operator runs the steps + * + * 1. a dry run on the database no upgraded process has opened: the columns + * are not there yet, so it reads nothing it could stamp and says so; + * 2. `--apply` boots plain, so the schema sync adds both columns, and stamps + * every row whose thread names a record, keeping each row's `updated_at`; + * 3. a re-run stamps nothing (idempotent) and still names the thread that + * names no record, which stays unstamped. + */ + +const HERE = dirname(fileURLToPath(import.meta.url)); +const CLI_ROOT = resolve(HERE, '..', '..', '..'); +const CASE_TIMEOUT_MS = 240_000; +const SECRET = 'os23857-integration-secret-at-least-32-chars'; +const UPDATED_AT = '2026-01-02T03:04:05.000Z'; + +/** The rows the previous writer left, and the pair each must get. */ +const PLANTED: Array<{ id: string; thread_id: string; pair: { object_name: string | null; record_id: string | null } }> = [ + { id: 'cmt_1', thread_id: 'os23857_thing:t1', pair: { object_name: 'os23857_thing', record_id: 't1' } }, + { id: 'cmt_2', thread_id: 'os23857_thing:t:2', pair: { object_name: 'os23857_thing', record_id: 't:2' } }, + { id: 'cmt_3', thread_id: 'sys_user:u1', pair: { object_name: 'sys_user', record_id: 'u1' } }, + { id: 'cmt_x', thread_id: 'watercooler', pair: { object_name: null, record_id: null } }, +]; + +type Invoke = (argv: string[]) => Promise; +const invoke = (command: { run: (argv: string[], opts: { root: string }) => Promise }): Invoke => + (argv) => command.run(argv, { root: CLI_ROOT }); + +/** Env that would point a command's boot somewhere other than the fixture. */ +const OVERRIDING_ENV = [ + 'OS_DATABASE_URL', 'DATABASE_URL', 'TURSO_DATABASE_URL', 'OS_DATABASE_DRIVER', 'OS_HOME', + 'OS_TELEMETRY_DB', 'OS_AUTH_SECRET', 'AUTH_SECRET', 'BETTER_AUTH_SECRET', 'OS_ARTIFACT_PATH', +] as const; + +/** Run one command and capture its JSON document and exit status. */ +async function runJson(command: Invoke, argv: string[]): Promise<{ payload: any; exitCode: number }> { + const savedExit = process.exitCode; + const swallow = ((_chunk: unknown, ...rest: unknown[]) => { + const cb = rest.find((a) => typeof a === 'function') as (() => void) | undefined; + if (cb) cb(); + return true; + }) as typeof process.stdout.write; + const stdout = vi.spyOn(process.stdout, 'write').mockImplementation(swallow); + vi.spyOn(process.stderr, 'write').mockImplementation(swallow); + vi.spyOn(console, 'log').mockImplementation(() => {}); + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); + let processExit: number | undefined; + vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + processExit = code ?? 0; + throw new Error(`__PROCESS_EXIT__:${processExit}`); + }) as never); + let thrownExit: number | undefined; + try { + try { + await command(argv); + } catch (error) { + const trapped = error instanceof Error && error.message.startsWith('__PROCESS_EXIT__'); + if (!trapped && !isExitSignal(error)) throw error; + if (!trapped) thrownExit = (error as { oclif?: { exit?: number } }).oclif?.exit; + } + const out = stdout.mock.calls.map((c) => String(c[0])).join('').trim(); + let payload: any; + try { + payload = JSON.parse(out); + } catch { + payload = { unparsed: out.slice(-400) }; + } + return { payload, exitCode: processExit ?? thrownExit ?? (process.exitCode as number | undefined) ?? 0 }; + } finally { + process.exitCode = savedExit; + vi.restoreAllMocks(); + } +} + +let dir: string; +let dbFile: string; +const savedEnv: Record = {}; +const savedCwd = process.cwd(); + +function withDb(fn: (db: Database.Database) => T, readonly = true): T { + const db = new Database(dbFile, { readonly }); + try { + return fn(db); + } finally { + db.close(); + } +} + +/** Insert a row, filling every other NOT NULL column without a default with a placeholder of its type. */ +function plant(db: Database.Database, table: string, values: Record): void { + const columns = db.prepare(`PRAGMA table_info('${table}')`).all() as Array< + { name: string; type: string; notnull: number; dflt_value: unknown; pk: number } + >; + const row: Record = { ...values }; + for (const c of columns) { + if (c.name in row || !c.notnull || c.dflt_value !== null) continue; + row[c.name] = /INT|BOOL|REAL|NUM/i.test(c.type) ? 0 : `${c.name}-placeholder`; + } + const names = Object.keys(row); + db.prepare(`INSERT INTO "${table}" (${names.map((n) => `"${n}"`).join(', ')}) VALUES (${names.map(() => '?').join(', ')})`) + .run(...names.map((n) => row[n] as never)); +} + +const columnsOf = (table: string): string[] => + withDb((db) => (db.prepare(`PRAGMA table_info('${table}')`).all() as Array<{ name: string }>).map((c) => c.name)); + +const pairs = (): Record => + withDb((db) => { + const out: Record = {}; + const rows = db.prepare("SELECT id, object_name, record_id FROM sys_comment WHERE id LIKE 'cmt_%' ORDER BY id").all() as + Array<{ id: string; object_name: string | null; record_id: string | null }>; + for (const r of rows) out[r.id] = { object_name: r.object_name, record_id: r.record_id }; + return out; + }); + +const updatedAts = (): string[] => + withDb((db) => (db.prepare("SELECT updated_at FROM sys_comment WHERE id LIKE 'cmt_%' ORDER BY id").all() as + Array<{ updated_at: unknown }>).map((r) => String(r.updated_at))); + +beforeAll(async () => { + for (const key of [...OVERRIDING_ENV, 'NODE_ENV'] as const) savedEnv[key] = process.env[key]; + for (const key of OVERRIDING_ENV) delete process.env[key]; + dir = mkdtempSync(join(tmpdir(), 'os-23857-comment-')); + mkdirSync(join(dir, 'dist'), { recursive: true }); + writeFileSync( + join(dir, 'objectstack.config.ts'), + [ + 'export default {', + " manifest: { id: 'com.example.os23857comment', name: 'requires auth', version: '0.0.0', type: 'app' },", + " requires: ['auth'],", + " objects: [{ name: 'os23857_thing', fields: { title: { type: 'text' } } }],", + '};', + '', + ].join('\n'), + ); + process.env.OS_ARTIFACT_PATH = join(dir, 'dist', 'objectstack.json'); + // `os serve` composes the auth family, and with it plugin-audit, only when an + // auth secret resolves; the deployment this pins serves with one. + process.env.OS_AUTH_SECRET = SECRET; + process.env.NODE_ENV = 'production'; + process.env.OS_TELEMETRY_DB = '0'; + dbFile = join(dir, 'upgraded.db'); + process.chdir(dir); + try { + const created = await runJson(invoke(MigrateApply), ['--database-url', `file:${dbFile}`, '--yes', '--json']); + expect(created.exitCode, JSON.stringify(created.payload).slice(0, 400)).toBe(0); + } finally { + process.chdir(savedCwd); + } + + // The previous version's shape: no pair, and its rows. + withDb((db) => { + const indexes = db.prepare("SELECT name, sql FROM sqlite_master WHERE type = 'index' AND tbl_name = 'sys_comment'").all() as + Array<{ name: string; sql: string | null }>; + for (const index of indexes) { + if (index.sql && /object_name|record_id/.test(index.sql)) db.exec(`DROP INDEX "${index.name}"`); + } + db.exec('ALTER TABLE sys_comment DROP COLUMN object_name'); + db.exec('ALTER TABLE sys_comment DROP COLUMN record_id'); + for (const row of PLANTED) { + plant(db, 'sys_comment', { + id: row.id, thread_id: row.thread_id, body: row.id, author_id: 'u1', + created_at: UPDATED_AT, updated_at: UPDATED_AT, + }); + } + }, false); + expect(columnsOf('sys_comment'), 'fixture: the previous version has no pair').not.toContain('object_name'); +}, CASE_TIMEOUT_MS); + +afterAll(() => { + process.chdir(savedCwd); + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + try { rmSync(dir, { recursive: true, force: true }); } catch { /* best-effort */ } +}); + +beforeEach(() => { process.chdir(dir); }); +afterEach(() => { process.chdir(savedCwd); }); + +describe('os migrate comment-parent-pair stamps the comment rows the previous version left (ADR-0139 E7)', () => { + const db = () => `file:${dbFile}`; + const run = (argv: string[]) => runJson(invoke(MigrateCommentParentPair), ['--database-url', db(), ...argv, '--json']); + + it('1. a dry run on the not-yet-synced database writes nothing, adds no column, and does not report a clean table', async () => { + const { payload, exitCode } = await run([]); + expect(columnsOf('sys_comment')).not.toContain('object_name'); + expect(exitCode).toBe(1); + expect(payload).toMatchObject({ apply: false, report: { stamped: 0, failures: 1 } }); + }, CASE_TIMEOUT_MS); + + it('2. --apply adds the pair and stamps every row whose thread names a record, keeping updated_at', async () => { + const { payload, exitCode } = await run(['--apply', '--yes']); + expect(exitCode).toBe(0); + expect(columnsOf('sys_comment')).toEqual(expect.arrayContaining(['object_name', 'record_id'])); + expect(payload.report).toEqual({ apply: true, scanned: PLANTED.length, stamped: PLANTED.length - 1, unparented: 1, failures: 0 }); + expect(pairs()).toEqual(Object.fromEntries(PLANTED.map((r) => [r.id, r.pair]))); + expect(new Set(updatedAts())).toEqual(new Set([UPDATED_AT])); + }, CASE_TIMEOUT_MS); + + it('3. a re-run stamps nothing and still names the thread that names no record', async () => { + const before = pairs(); + const { payload, exitCode } = await run(['--apply', '--yes']); + expect(exitCode).toBe(0); + expect(payload.report).toEqual({ apply: true, scanned: 1, stamped: 0, unparented: 1, failures: 0 }); + expect(pairs()).toEqual(before); + }, CASE_TIMEOUT_MS); + + it('4. a dry run after the apply reports the same clean table', async () => { + const { payload, exitCode } = await run([]); + expect(exitCode).toBe(0); + expect(payload.report).toEqual({ apply: false, scanned: 1, stamped: 0, unparented: 1, failures: 0 }); + }, CASE_TIMEOUT_MS); +}); diff --git a/packages/cli/src/commands/migrate/comment-parent-pair.ts b/packages/cli/src/commands/migrate/comment-parent-pair.ts new file mode 100644 index 00000000000..bca098b7a32 --- /dev/null +++ b/packages/cli/src/commands/migrate/comment-parent-pair.ts @@ -0,0 +1,240 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { Command, Flags } from '@oclif/core'; +import chalk from 'chalk'; +import { createInterface } from 'node:readline'; +import type { IObjectQLEngine } from '@objectstack/spec/contracts'; +import { + printHeader, + printSuccess, + printWarning, + printError, + printInfo, + printStep, + createTimer, + emitJson, + isExitSignal, + errorCodeFields, +} from '../../utils/format.js'; +import { bootSchemaStack } from '../../utils/schema-migrate.js'; +import { OCCUPANCY_HINT, probeMigrationTarget } from '../../utils/migrate-occupancy-gate.js'; +import { describeOccupancy, JOURNAL_MODE_SWITCH_NOTE } from '../../utils/sqlite-occupancy.js'; +import { buildDataMigrationPlugins } from '../../utils/data-migration-plugins.js'; +import { absentTableReads } from '../../utils/absent-table-reads.js'; + +async function confirm(question: string): Promise { + if (!process.stdin.isTTY) return false; // non-interactive → require --yes + const rl = createInterface({ input: process.stdin, output: process.stdout }); + try { + const answer: string = await new Promise((resolve) => rl.question(question, resolve)); + return /^y(es)?$/i.test(answer.trim()); + } finally { + rl.close(); + } +} + +/** + * `os migrate comment-parent-pair` — the stored-row backfill of `sys_comment`'s + * writer-stamped parent pair, `object_name` + `record_id` (ADR-0139 E7). + * + * The comment-thread read gate narrows a comment read with one `ReadableIds` + * leaf per parent object over the pair. New rows are stamped at write time; + * rows written before the upgrade carry no pair, and the gate answers them with + * the bounded thread probe instead — at most 2,000 candidate rows per read, + * fail closed. So after the upgrade, and until this runs, a broad comment read + * past that window omits comments the caller may read, and its total agrees + * with the short answer. Operators run it at once after upgrading, dry run + * first. + * + * Each row is stamped from its `thread_id` by the planner the writer stamps + * new rows with (`@objectstack/plugin-audit`'s `backfillCommentParentPair`). A + * row whose thread names no record stays unstamped (hidden from every + * non-system reader, as before) and is counted. + * + * Dry run by default (writes no row), `--apply` to stamp. Idempotent: only + * unstamped rows are read, so a second run stamps nothing — re-running and + * reading a clean report is the verification. No `sys_migration` flag is + * recorded: nothing gates irreversible behaviour on it. The flags are + * `os migrate activity-changed-fields`'s, one for one. + */ +export default class MigrateCommentParentPair extends Command { + static override description = + 'Stamp the parent pair (object_name, record_id) on the sys_comment rows written before it existed: until it ' + + 'runs, a broad comment read past 2,000 candidate rows omits comments the caller may read. Dry run by default; ' + + '--apply writes. Run it at once after upgrading.'; + + static override examples = [ + '$ os migrate comment-parent-pair', + '$ os migrate comment-parent-pair --apply', + '$ os migrate comment-parent-pair --apply --yes --json', + ]; + + static override flags = { + 'database-url': Flags.string({ + description: 'Database URL to migrate (defaults to $OS_DATABASE_URL / the project DB)', + env: 'OS_DATABASE_URL', + }), + apply: Flags.boolean({ + description: 'Stamp the unstamped rows (default is a read-only dry run)', + default: false, + }), + yes: Flags.boolean({ char: 'y', description: 'Skip the --apply confirmation prompt', default: false }), + force: Flags.boolean({ + description: 'Apply even when another process is using the database (SQLite occupancy check)', + default: false, + }), + json: Flags.boolean({ description: 'Output as JSON (implies non-interactive; requires --yes to apply)' }), + }; + + async run(): Promise { + const { flags } = await this.parse(MigrateCommentParentPair); + const timer = createTimer(); + const apply = flags.apply; + + if (!flags.json) printHeader('Migrate · comment-parent-pair'); + + const occupancy = await probeMigrationTarget(flags['database-url']); + if (occupancy.status === 'busy' && apply && !flags.force) { + if (flags.json) { + await emitJson( + { error: 'database_busy', database: occupancy.filename, signal: occupancy.signal, detail: occupancy.detail, hint: OCCUPANCY_HINT }, + 0, + { compact: true }, + ); + this.exit(1); + return; + } + printError(describeOccupancy(occupancy)); + printWarning(OCCUPANCY_HINT); + this.exit(1); + return; + } + if (occupancy.status === 'busy' && !flags.json) { + printWarning(apply + ? `--force: ${describeOccupancy(occupancy)} Stamping anyway — the live process may write rows mid-walk.` + : `${describeOccupancy(occupancy)} The dry run below writes no row or table, but its counts may shift while that process is running. ${JOURNAL_MODE_SWITCH_NOTE}`); + } + if (occupancy.status === 'unknown' && !flags.json) { + printWarning(`Could not check whether the database is in use — ${occupancy.detail}`); + } + + if (apply && !flags.yes) { + if (flags.json || !process.stdin.isTTY) { + if (flags.json) { + await emitJson({ error: 'confirmation_required', hint: 'pass --yes' }, 0, { compact: true }); + this.exit(1); + return; + } + printWarning('Apply mode stamps sys_comment rows. Re-run with --yes to confirm, or run without --apply to preview.'); + this.exit(1); + return; + } + const ok = await confirm( + chalk.bold('\nStamp every unstamped sys_comment row on this database? [y/N] '), + ); + if (!ok) { + printInfo('Aborted — no changes made.'); + return; + } + } + + if (!flags.json) { + printStep(apply ? 'Booting data stack (APPLY mode)…' : 'Booting data stack (dry run)…'); + } + + let stack; + try { + // [#21349] The dry run boots READ-ONLY — the same boot `os migrate plan` + // takes: `deferSchemaDdl` holds schema DDL back and suppresses the + // artifact's inline seed loader (whose upserts rewrite every seeded row + // of the app's tables), and `readOnlyProbe` keeps a missing sqlite file + // from being created. `--apply` keeps the plain boot, unchanged. + stack = await bootSchemaStack({ + jsonOutput: flags.json, + databaseUrl: flags['database-url'], + extraPlugins: await buildDataMigrationPlugins({ audit: true }), + ...(apply ? {} : { deferSchemaDdl: true, readOnlyProbe: true }), + }); + } catch (error: any) { + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } + printError(error.message || String(error)); + this.exit(1); + return; + } + + try { + // `SchemaStack.kernel` is untyped, so the slot's contract is stated on the + // RESULT — the `objectql` slot serves `IObjectQLEngine`, whose `find` / + // `update` are exactly what the backfill calls. + const engine = stack.kernel.getService('objectql') as IObjectQLEngine | undefined; + if (typeof engine?.find !== 'function' || typeof engine?.update !== 'function') { + throw new Error('No ObjectQL engine on this stack — cannot stamp comment rows.'); + } + + const { backfillCommentParentPair } = await import('@objectstack/plugin-audit'); + + const logger = flags.json + ? { info: (m: string) => console.error(m), warn: (m: string) => console.error(m) } + : { info: (m: string) => printInfo(m), warn: (m: string) => printWarning(m) }; + + // [#21552] The dry run boots read-only, so the boot's own measurement of + // which tables exist is not believed: the table is read, and only the + // refusal of a table that is not there (a database no process has opened) + // reads as no rows (`absentTableReads`). ⛔ Any other refused read — a + // `sys_comment` the upgraded schema sync has not yet given its pair + // columns, included — is still counted in `failures` and still exits + // non-zero. `--apply` booted plain; its writes go to the engine itself. + const reads = absentTableReads(stack, (object) => engine.getObject(object)); + const readView: Pick = { + find: (object, query, options) => reads.rows(object, () => engine.find(object, query, options)), + update: (object, data, options) => engine.update(object, data, options), + }; + + const report = await backfillCommentParentPair(readView, logger, { apply }); + + if (flags.json) { + reads.notice(true); + await emitJson({ database: stack.dbLabel, apply, report, duration: timer.elapsed() }); + if (report.failures > 0) this.exit(1); + return; + } + + printInfo(`Database: ${chalk.white(stack.dbLabel)}`); + reads.notice(false); + console.log(''); + console.log( + ` ${chalk.white('sys_comment')}: ${report.scanned} unstamped, ${report.stamped} ${apply ? 'stamped' : 'to stamp'}` + + `, ${report.unparented} name no record`, + ); + console.log(''); + + if (report.failures > 0) { + printError(`${report.failures} read(s) or row(s) failed — re-run to finish them.`); + } else if (apply && report.stamped > 0) { + printSuccess( + `Stamped ${report.stamped} comment row(s). Re-run any time — it only revisits rows still unstamped.`, + ); + } else if (!apply && report.stamped > 0) { + printInfo(`Dry run only — ${report.stamped} row(s) would be stamped. Re-run with --apply.`); + } else { + printSuccess('Nothing to stamp — every sys_comment row whose thread names a record is stamped.'); + } + if (report.unparented > 0) { + printWarning( + `${report.unparented} row(s) have a thread_id that names no record: they stay unstamped and, as before, ` + + 'are hidden from every non-system reader.', + ); + } + console.log(chalk.dim(` ${timer.display()}`)); + console.log(''); + if (report.failures > 0) this.exit(1); + } catch (error: any) { + if (isExitSignal(error)) throw error; + if (flags.json) { await emitJson({ error: error.message, ...errorCodeFields(error) }, 0, { compact: true }); this.exit(1); } + printError(error.message || String(error)); + this.exit(1); + } finally { + await stack.shutdown(); + } + } +} diff --git a/packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts b/packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts index ecfd1864809..4fb87952a03 100644 --- a/packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts +++ b/packages/cli/src/commands/migrate/data-commands.absent-database.integration.test.ts @@ -64,6 +64,10 @@ * control row is the seeded activity row, planted below the writer, so it * carries no stamp. * + * [ADR-0139 E7] `os migrate comment-parent-pair` joined it the same way: it + * reads `sys_comment` alone, through `absentTableReads`. Its control row is the + * seeded comment, planted below the writer, so it carries no parent pair. + * * [#21573] `os migrate unmapped-columns` joined the roster later, born with * the same answer: it asks `tableAbsent` before the differ. Its control row is * a retired field's column on `os21529_contact`, holding a value. @@ -429,6 +433,10 @@ await k(shard.name).insert({ }, }), }); +await k('sys_comment').insert({ + id: 'cmt_23857', thread_id: 'sys_metadata:m_21552', body: 'planted below the writer', author_id: 'u1', + created_at: new Date().toISOString(), +}); await k('sys_secret').insert({ id: 'sec_21552', namespace: 'os21552', key: 'orphan', kms_key_id: 'local', alg: 'aes-256-gcm', version: 1, ciphertext: 'not-a-real-ciphertext', }); @@ -501,6 +509,18 @@ const DOORS: readonly Door[] = [ expect(doc.report).toMatchObject({ scanned: 1, stamped: 1, unjudged: 0, failures: 0 }); }, }, + { + name: 'migrate comment-parent-pair', + argv: ['migrate', 'comment-parent-pair'], + empty: (doc) => expect(doc).toMatchObject({ apply: false, report: { scanned: 0, stamped: 0, failures: 0 } }), + humanEmpty: /Nothing to stamp/, + tables: ['sys_comment'], + work: (doc) => { + // The seeded comment was planted below the writer, so it carries no pair: + // the one row to stamp. + expect(doc.report).toMatchObject({ scanned: 1, stamped: 1, unparented: 0, failures: 0 }); + }, + }, { name: 'migrate meta --stored', argv: ['migrate', 'meta', '--stored'], diff --git a/packages/cli/src/utils/schema-migrate.one-shot-family.integration.test.ts b/packages/cli/src/utils/schema-migrate.one-shot-family.integration.test.ts index 68a6c42324f..defe0dace82 100644 --- a/packages/cli/src/utils/schema-migrate.one-shot-family.integration.test.ts +++ b/packages/cli/src/utils/schema-migrate.one-shot-family.integration.test.ts @@ -56,6 +56,7 @@ import { bootSchemaStack } from './schema-migrate.js'; import { buildDataMigrationPlugins } from './data-migration-plugins.js'; import MigrateAccountIssuer from '../commands/migrate/account-issuer.js'; import MigrateActivityChangedFields from '../commands/migrate/activity-changed-fields.js'; +import MigrateCommentParentPair from '../commands/migrate/comment-parent-pair.js'; import MigrateApply from '../commands/migrate/apply.js'; import MigrateAuditMetadataBodies from '../commands/migrate/audit-metadata-bodies.js'; import MigrateCommentReactions from '../commands/migrate/comment-reactions.js'; @@ -164,6 +165,11 @@ const CALLERS: Record = { noWrite: [{ label: 'migrate activity-changed-fields', argv: ['--database-url', '@DB@', '--json'] }], write: [{ label: 'migrate activity-changed-fields --apply', argv: ['--apply', '--yes', '--database-url', '@DB@', '--json'] }], }, + 'commands/migrate/comment-parent-pair.ts': { + run: invoke(MigrateCommentParentPair), + noWrite: [{ label: 'migrate comment-parent-pair', argv: ['--database-url', '@DB@', '--json'] }], + write: [{ label: 'migrate comment-parent-pair --apply', argv: ['--apply', '--yes', '--database-url', '@DB@', '--json'] }], + }, 'commands/migrate/apply.ts': { run: invoke(MigrateApply), noWrite: [], diff --git a/packages/cli/test/json-stdout-purity.e2e.test.ts b/packages/cli/test/json-stdout-purity.e2e.test.ts index 0c1b9851f03..4ee761b6936 100644 --- a/packages/cli/test/json-stdout-purity.e2e.test.ts +++ b/packages/cli/test/json-stdout-purity.e2e.test.ts @@ -94,6 +94,10 @@ const FAMILY: Record = { // `sys_activity` and writes nothing. Its backfill logs through a sink of its // own, which `--json` points at stderr, as `audit-metadata-bodies` does. 'migrate activity-changed-fields': [], + // [ADR-0139 E7] The same shape: a dry run by default, reading `sys_comment` + // and writing nothing, its backfill logging through a sink `--json` points at + // stderr. + 'migrate comment-parent-pair': [], 'migrate apply': [], // A dry run is its DEFAULT and the only form driven here: without `--apply` // it boots (plugin-audit's objects registered), reads `sys_audit_log` / diff --git a/packages/plugins/plugin-audit/src/audit-plugin.ts b/packages/plugins/plugin-audit/src/audit-plugin.ts index 21e162f15a3..39a5f970334 100644 --- a/packages/plugins/plugin-audit/src/audit-plugin.ts +++ b/packages/plugins/plugin-audit/src/audit-plugin.ts @@ -458,7 +458,16 @@ export class AuditPlugin implements Plugin { }, ); if (typeof (engine as any).registerMiddleware === 'function') { - installCommentReadVisibility(engine as any, ctx.logger); + // [ADR-0139 E7] The thread gate writes one ReadableIds leaf per parent + // object, and only where the security service answers `canReadObject` + // (`parent-record-read-gate.ts`). Resolved on every read. + installCommentReadVisibility(engine as any, ctx.logger, () => { + try { + return ctx.getService('security'); + } catch { + return undefined; + } + }); } else { ctx.logger.warn( 'AuditPlugin: engine has no middleware seam — sys_comment and sys_comment_reaction READ visibility NOT ' + diff --git a/packages/plugins/plugin-audit/src/comment-access-hooks.ts b/packages/plugins/plugin-audit/src/comment-access-hooks.ts index 1618e3e54de..b57f9b25707 100644 --- a/packages/plugins/plugin-audit/src/comment-access-hooks.ts +++ b/packages/plugins/plugin-audit/src/comment-access-hooks.ts @@ -116,6 +116,13 @@ import type { StandardErrorCode } from '@objectstack/spec/api'; import type { ISecurityService, ISharingService } from '@objectstack/spec/contracts'; import type { ExecutionContext } from '@objectstack/spec/kernel'; import { renderOperationMessage, type ValidationMessageTranslator } from '@objectstack/spec/system'; +import { AUDIT_EXCLUDED_OBJECTS } from './audit-writers.js'; +import { + computeParentRecordFilter, + type ParentObjectReadAdmissionSource, + type ParentRecordGate, + type ParentRecordGateEngine, +} from './parent-record-read-gate.js'; /** Minimal engine surface these installers need — duck-typed (like * service-storage's attachment seams) so tests can fake it and so plugin-audit @@ -146,6 +153,10 @@ export interface CommentAccessEngine { ): void; find(object: string, options: Record): Promise>>; findOne(object: string, options: Record): Promise | null>; + /** [ADR-0139 E7] The registered schema, read to learn whether `sys_comment` + * declares its parent pair ({@link declaresCommentParentPair}). Optional: an + * engine without it is treated as one whose `sys_comment` declares none. */ + getSchema?(object: string): unknown; } /** Minimal shape of the engine `OperationContext` the read middleware reads. */ @@ -254,6 +265,175 @@ export function parseCommentThreadId(threadId: unknown): CommentThreadTarget | n return { object, recordId }; } +/** [ADR-0139 E7, ADR-0052 §5] The column a comment's parent OBJECT is stamped in. */ +export const COMMENT_PARENT_OBJECT_COLUMN = 'object_name'; + +/** [ADR-0139 E7, ADR-0052 §5] The column a comment's parent RECORD id is stamped in. */ +export const COMMENT_PARENT_RECORD_COLUMN = 'record_id'; + +/** The pair, in one list. */ +export const COMMENT_PARENT_PAIR_COLUMNS = [COMMENT_PARENT_OBJECT_COLUMN, COMMENT_PARENT_RECORD_COLUMN] as const; + +/** What a comment row's parent pair is stamped with. Both `null`: the thread names no record. */ +export interface CommentParentPair { + object_name: string | null; + record_id: string | null; +} + +/** + * [ADR-0139 E7] The ONE derivation of a comment's parent pair: the record its + * `thread_id` names, by {@link parseCommentThreadId} — the parse every gate in + * this module authorizes by — or `null` / `null` for a thread that names none. + * Called by the stamp on create and on a re-point, and by the stored-row + * backfill (`comment-parent-pair-backfill.ts`), so a stored row and a new row + * with the same thread carry the same pair. Pure. + * + * `thread_id` stays the authored key: the pair is never authored, only derived. + */ +export function planCommentParentPair(threadId: unknown): CommentParentPair { + const target = parseCommentThreadId(threadId); + return target ? { object_name: target.object, record_id: target.recordId } : { object_name: null, record_id: null }; +} + +/** + * Whether the REGISTERED `sys_comment` declares both pair columns. Read on + * every call, never cached: a composition that registers its own earlier + * definition of the object (a test harness, a host override) has no column to + * stamp — an insert naming one would be refused as an unknown field — and no + * column a read could be narrowed by, so the read gate keeps the thread probe + * for it (`installCommentReadVisibility`). The E6 precedent + * (`declaresActivityStampColumns`). + */ +export function declaresCommentParentPair(engine: { getSchema?(object: string): unknown }): boolean { + let fields: unknown; + try { + const schema = typeof engine.getSchema === 'function' ? engine.getSchema('sys_comment') : undefined; + fields = schema && typeof schema === 'object' ? (schema as { fields?: unknown }).fields : undefined; + } catch { + return false; + } + return !!fields && typeof fields === 'object' && COMMENT_PARENT_PAIR_COLUMNS.every((column) => column in (fields as object)); +} + +/** Stamp one row in place: the planner's answer, over whatever the writer sent. */ +function stampCommentParentPair(row: Record): void { + const pair = planCommentParentPair(row.thread_id); + row[COMMENT_PARENT_OBJECT_COLUMN] = pair.object_name; + row[COMMENT_PARENT_RECORD_COLUMN] = pair.record_id; +} + +/** + * [ADR-0139 E7] The pair's two writers, both registered for EVERY caller — + * system, seed and context-less included — because the pair is derived, not + * authorized: the authorization handlers below exempt system callers, this + * stamp exempts nobody. + * + * - create (`beforeInsert`): every inserted row is stamped from its + * `thread_id`, over whatever value the writer sent. The engine's create-side + * `readonly` strip leaves `sys_` objects to their own guards + * (`staticReadonlyInsertSubject`), so this assignment is the guard, as the + * create hook is for `is_edited`. + * - update (`beforeUpdate`), for every caller: an update that names + * `thread_id` — the re-point the update gate authorizes as an insert into + * the new thread — is stamped from it. The payload is shared by every row a + * predicate update matches, so every row gets the same pair, as it gets the + * same `thread_id`. An update that names either pair column WITHOUT a + * thread is re-derived from the row's own stored thread (`previous`), so no + * writer's value lands, the backfill's included. ⛔ The engine's update-side + * `readonly` strip is not the guard here: under `preserveAudit` (a REST + * historical import puts it on an ordinary caller's write) it keeps a + * readonly value. A predicate update naming the pair without a thread + * re-derives a pair per matched row over one shared payload, and one SET + * clause carries a single pair to every row, so when the matched rows' + * threads name different records the write is refused whole, before + * anything is written ({@link refusePairDivergence}, `VALIDATION_ERROR` / + * 400): the engine's own key-set divergence check cannot see it, since + * every row assigns the same two keys. With no stored row to derive from, + * the pair is dropped from the write. An update naming none of the three + * columns stamps nothing. + */ +/** The catalog member for a request whose shape is refused (ADR-0112). */ +const PAIR_DIVERGENCE_CODE: StandardErrorCode = 'VALIDATION_ERROR'; + +/** The pair the first row of a predicate update derived, per write (the engine's shared `dispatch.scope`). */ +const pairByWrite = new WeakMap(); + +/** + * Refuse a predicate (`per-row`) update whose matched rows derive different + * parent pairs: one `SET` clause cannot carry two pairs, and carrying one row's + * pair to another row would authorize that comment by a record its thread does + * not name. Refused before anything is written (every before-hook runs ahead + * of the one write). A by-id update, and a predicate update whose rows share + * one thread, pass. `VALIDATION_ERROR` / 400: the request's shape is what is + * refused, and the remedy is in the caller's hands. + */ +function refusePairDivergence(dispatch: unknown, pair: CommentParentPair): void { + const d = dispatch as { mode?: unknown; scope?: unknown } | undefined; + if (d?.mode !== 'per-row' || !d.scope || typeof d.scope !== 'object') return; + const first = pairByWrite.get(d.scope); + if (!first) { + pairByWrite.set(d.scope, pair); + return; + } + if (first.object_name === pair.object_name && first.record_id === pair.record_id) return; + const err: any = new Error( + 'Refusing a multi-row update of sys_comment that names object_name / record_id without thread_id: the matched ' + + "comments' threads name different records, and one write cannot store each comment's own pair. The pair is " + + "derived from thread_id — update the comments one by one, or name thread_id.", + ); + err.code = PAIR_DIVERGENCE_CODE; + err.status = 400; + err.statusCode = 400; + throw err; +} + +function installCommentParentPairStamp(engine: CommentAccessEngine): void { + engine.registerHook( + 'beforeInsert', + (ctx: any) => { + if (!declaresCommentParentPair(engine)) return; + const data = ctx?.input?.data; + for (const row of Array.isArray(data) ? data : [data]) { + if (row && typeof row === 'object' && !Array.isArray(row)) stampCommentParentPair(row as Record); + } + }, + { object: COMMENT_OBJECT, packageId: PACKAGE_ID }, + ); + engine.registerHook( + 'beforeUpdate', + (ctx: any) => { + const data: unknown = ctx?.input?.data; + if (!data || typeof data !== 'object' || Array.isArray(data)) return; + const payload = data as Record; + const names = (key: string) => Object.prototype.hasOwnProperty.call(payload, key); + if (!names('thread_id') && !COMMENT_PARENT_PAIR_COLUMNS.some(names)) return; + if (!declaresCommentParentPair(engine)) return; + if (names('thread_id')) { + stampCommentParentPair(payload); + return; + } + // The pair named without a thread: derive it from the row's own thread. + const previous: unknown = ctx?.previous; + const storedThread = + previous && typeof previous === 'object' ? (previous as Record).thread_id : undefined; + if (typeof storedThread !== 'string') { + for (const column of COMMENT_PARENT_PAIR_COLUMNS) delete payload[column]; + return; + } + const pair = planCommentParentPair(storedThread); + // A predicate update dispatches this handler once per matched row over + // ONE shared payload, and one SET clause carries the last row's values to + // every row. The engine's key-set divergence check cannot see this (every + // row assigns the same two keys), so the pair is compared here, across + // the call's rows, on the scope object the engine shares between them. + refusePairDivergence(ctx?.dispatch, pair); + payload[COMMENT_PARENT_OBJECT_COLUMN] = pair.object_name; + payload[COMMENT_PARENT_RECORD_COLUMN] = pair.record_id; + }, + { object: COMMENT_OBJECT, packageId: PACKAGE_ID }, + ); +} + function forbid(message: string, object?: string): never { const err: any = new Error(message); err.code = DENY_CODE; @@ -801,6 +981,11 @@ export function installCommentAccessHooks( // {@link stampCommentBodyEdit}. engine.registerHook('beforeUpdate', stampCommentBodyEdit, { object: COMMENT_OBJECT, packageId: PACKAGE_ID }); + // ── Create and re-point: the platform stamps the parent pair (ADR-0139 E7) ─ + // Its own registrations, for the stamp's reason above: every caller is + // stamped — see {@link installCommentParentPairStamp}. + installCommentParentPairStamp(engine); + // ── Delete: author or parent editor ───────────────────────────────── engine.registerHook( 'beforeDelete', @@ -836,22 +1021,80 @@ export function installCommentAccessHooks( * the returned rows — a beforeFind/afterFind hook would leave `count()` * unfiltered and leak the true row count via `total`. * - * Mechanism: for each read, pre-scan the candidate `thread_id`s the query - * would touch (system context), resolve which of their parent records the - * caller can actually read through the caller-scoped engine (the parent's own - * RLS/OWD/sharing apply), and AND `{ thread_id: { $in: } }` - * into `ctx.ast.where`. Threads whose parent is invisible — and threads that - * name no parent at all — simply never enter that list. + * ## Comment threads: one `ReadableIds` leaf per parent object (ADR-0139 D4, E7) + * + * A comment's parent is stamped in its pair, `object_name` + `record_id` + * ({@link planCommentParentPair}), so the thread gate is the parent-record gate + * the activity stream and the ledger use (`parent-record-read-gate.ts`, + * {@link computeParentRecordFilter}), over the comment's own pair: one branch + * `{ object_name: P, record_id: { $in: ReadableIds.of(P) } }` per registered + * parent object, ORed and ANDed into the read's `where`. No pre-scan reads the + * comments, so a list, its `total` and a grouped count are each one statement + * over every comment the caller may read, on any table size. The parent + * objects are every registered object but `sys_comment` itself: a comment can + * hang off any record (the console writes `thread_id` from every record + * detail page), so every object the audit writer excludes is a named parent of + * this gate ({@link commentThreadGate}). + * + * That module decides up front, by name, what keeps the bounded probe (over the + * pair): ADR-0139 D5's three classes — a delegated caller (the whole read), a + * parent an object-scoped read middleware narrows (`sys_attachment`, + * `sys_comment_reaction`, `sys_approval_request`, …), a federated parent — and + * the two whole-read states it reports once: an engine that cannot classify, + * and a security service that answers no `canReadObject` (no leaf is written + * without it). ⛔ Never reached by catching a refusal. + * + * ## A row the backfill has not reached keeps the probe — never a leaf + * + * A row with no stamped pair (`object_name` NULL) — written before the pair + * existed, until `os migrate comment-parent-pair --apply` stamps it, or a + * thread that names no record — is answered by the bounded thread probe this + * gate always had, restricted to those rows ({@link probeUnstampedThreads}): + * pre-scan their `thread_id`s under SYSTEM context (at most + * {@link READ_SCAN_LIMIT}, in the caller's order), resolve which parent records + * the caller can read through the caller-scoped engine, and OR in + * `{ object_name: NULL, thread_id: { $in: VISIBLE_THREADS } }`. Beyond the + * bound it fails CLOSED, and says so. A thread naming no record never enters + * that list. Once the backfill has run, the probe finds no candidate. + * + * A composition whose registered `sys_comment` declares no pair + * ({@link declaresCommentParentPair}) has no column to leaf over: every read is + * the thread probe over every row, and that is said once. + * + * ## What the read's own refusal does (ADR-0139 D3) + * + * A leaf is composed by the engine after this middleware hands the read on + * (`next()`), so a composition or driver refusal (on `driver-mongodb`, a 500 + * until O3) is the read's own failure and reaches the caller saying why. + * ⛔ Nothing here catches it into the deny-all sentinel: the `catch` below + * wraps only this gate's own filter computation, which runs before `next()`. + * + * ## Comment reactions keep the probe (ADR-0139 D5.2) + * + * A reaction's only parent is `sys_comment`, which this very middleware + * narrows — D5.2's class by name — so the reaction gate stays the bounded + * probe ({@link computeReactionVisibilityFilter}). A leaf over the comment's + * parent record instead would skip the caller's grant on `sys_comment` and + * widen an access boundary. + * + * `getAdmission` resolves, on every read, the security service whose + * `canReadObject` admits a leaf's parent object (absent: every read is the + * probe, said once). */ export function installCommentReadVisibility( - engine: CommentAccessEngine, + engine: ParentRecordGateEngine, logger: CommentAccessLogger, + getAdmission?: ParentObjectReadAdmissionSource, ): void { if (typeof engine.registerMiddleware !== 'function') return; // engine lacks the seam const andIn = (ctx: CommentReadMiddlewareCtx, filter: unknown) => { if (!ctx.ast) return; ctx.ast.where = ctx.ast.where ? { $and: [ctx.ast.where, filter] } : filter; }; + // Built at install time, never at module load: `audit-writers.ts` is read + // through an import this module shares with its consumers. + const threadGate = commentThreadGate(); + const admission: ParentObjectReadAdmissionSource = getAdmission ?? (() => undefined); // One handler for both objects (see "A comment's reactions" in the module // header): the system / context-less exemption is decided once. @@ -865,7 +1108,7 @@ export function installCommentReadVisibility( try { const filter = reaction ? await computeReactionVisibilityFilter(engine, ctx, logger) - : await computeThreadVisibilityFilter(engine, ctx, logger); + : await computeThreadVisibilityFilter(engine, ctx, logger, threadGate, admission); if (filter) andIn(ctx, filter); } catch (err) { // A filter-compute failure must never fall open into a leak. @@ -875,12 +1118,30 @@ export function installCommentReadVisibility( ); andIn(ctx, reaction ? REACTION_READ_DENY_ALL : READ_DENY_ALL); } + // [ADR-0139 D3] Outside the `try`: a leaf's composition refusal is the read's. return next(); }; engine.registerMiddleware(gate, { object: COMMENT_OBJECT }); engine.registerMiddleware(gate, { object: COMMENT_REACTION_OBJECT }); } +/** + * [ADR-0139 E7] The comment-thread gate, as the parent-record gate reads it: + * `sys_comment` over its pair, every row naming no parent excluded, no caller + * exempt. Every object the audit writer excludes is a named parent: comment + * writers name any record, so the comment gate enumerates every registered + * object but `sys_comment`, exactly the set {@link parseCommentThreadId} + * accepts. + */ +export function commentThreadGate(): ParentRecordGate { + return { + object: COMMENT_OBJECT, + seam: 'comment read visibility', + denyAll: READ_DENY_ALL, + namedParents: [...AUDIT_EXCLUDED_OBJECTS], + }; +} + /** * [#22566] Resolve the WHERE predicate for one `sys_comment_reaction` read: a * reaction is readable exactly when its comment is. @@ -950,34 +1211,102 @@ async function computeReactionVisibilityFilter( return { comment_id: { $in: kept } }; } +/** The thread probe's whole-read state, reported once per engine. */ +const undeclaredPairReported = new WeakSet(); + +/** The rows no pair is stamped on: the unbackfilled ones, and threads naming no record. */ +const UNSTAMPED_ROWS = { [COMMENT_PARENT_OBJECT_COLUMN]: { $null: true } } as const; + /** - * Resolve the thread-visibility WHERE predicate for one sys_comment read. - * Returns `null` when the query matches no rows (nothing to narrow), a - * `thread_id` `$in` of the visible threads, or the deny-all sentinel. + * Resolve the thread-visibility WHERE predicate for one sys_comment read + * (installer docblock above): the parent-record gate's branches over the + * stamped pair, ORed with the bounded thread probe's branch for the rows no + * pair is stamped on — or, on a composition whose `sys_comment` declares no + * pair, the thread probe over every row. Returns `null` only on that last + * path when the query matches no row (nothing to narrow); otherwise the + * predicate, or the deny-all sentinel when no branch remains. */ async function computeThreadVisibilityFilter( + engine: ParentRecordGateEngine, + ctx: CommentReadMiddlewareCtx, + logger: CommentAccessLogger, + gate: ParentRecordGate, + getAdmission: ParentObjectReadAdmissionSource, +): Promise { + if (!declaresCommentParentPair(engine)) { + if (!undeclaredPairReported.has(engine)) { + undeclaredPairReported.add(engine); + logger.warn( + `[audit] comment read visibility: the registered sys_comment declares no ${COMMENT_PARENT_OBJECT_COLUMN} / ` + + `${COMMENT_PARENT_RECORD_COLUMN} pair, so every read is answered by the bounded thread probe (at most ` + + `${READ_SCAN_LIMIT} candidate rows, fail-closed). Reported once for this engine.`, + ); + } + return probeUnstampedThreads(engine, ctx, logger, false); + } + + const branches: unknown[] = []; + // 1. The stamped rows: one ReadableIds leaf per composable parent object, or + // the bounded probe over the pair for D5's classes (ADR-0139 D4, D5). + const stamped = await computeParentRecordFilter(engine, ctx, logger, gate, getAdmission); + if (stamped && stamped !== gate.denyAll) branches.push(stamped); + // 2. The rows no pair is stamped on: the bounded thread probe, never a leaf. + const unstamped = await probeUnstampedThreads(engine, ctx, logger, true); + if (unstamped && unstamped !== READ_DENY_ALL) branches.push(unstamped); + + if (branches.length === 0) return READ_DENY_ALL; + return branches.length === 1 ? branches[0] : { $or: branches }; +} + +/** + * The bounded thread probe (the gate's mechanism before ADR-0139 E7): pre-scan + * the candidate `thread_id`s the query would touch under SYSTEM context, + * resolve which of their parent records the caller can read through the + * caller-scoped engine (the parent's own RLS/OWD/sharing apply), and keep the + * visible threads verbatim. + * + * `onlyUnstamped`: restricted to the rows no pair is stamped on, and the + * emitted branch says so, so it can never select a stamped row — those are the + * leaf's. Returns `null` when the query matches no candidate, the deny-all + * sentinel when none is visible. A throw is the caller's to fail closed on. + */ +async function probeUnstampedThreads( engine: CommentAccessEngine, ctx: CommentReadMiddlewareCtx, logger: CommentAccessLogger, + onlyUnstamped: boolean, ): Promise { // 1. Candidate thread ids the query would touch — read under SYSTEM context // (the caller may not see the rows yet; that is exactly what we are - // deciding). Bypasses this middleware (isSystem). + // deciding). Bypasses this middleware (isSystem). The caller's own order + // rides along, so on a table larger than the scan bound the window + // scanned is the window the caller pages through. + const callerWhere = ctx.ast?.where as Record | undefined; + const where = onlyUnstamped + ? callerWhere + ? { $and: [callerWhere, { ...UNSTAMPED_ROWS }] } + : { ...UNSTAMPED_ROWS } + : (callerWhere ?? {}); + const orderBy = (ctx.ast as { orderBy?: unknown } | undefined)?.orderBy; const candidates = await engine.find('sys_comment', { - where: (ctx.ast?.where as Record) ?? {}, + where, fields: ['thread_id'], + ...(Array.isArray(orderBy) && orderBy.length > 0 ? { orderBy } : {}), limit: READ_SCAN_LIMIT, context: { ...SYSTEM_CTX }, }); if (!candidates.length) return null; if (candidates.length >= READ_SCAN_LIMIT) { // Not silent (fail-closed truncation): rows beyond the scan window are - // excluded from the visibility filter, so a very broad unscoped list may - // omit rows the caller could see. The comment panel scopes to one thread - // so never hits this; a global list should paginate by thread_id. + // excluded from the visibility filter, so a very broad read may omit rows + // the caller could see. Stamping the pair lifts the bound for every row. logger.warn( - `[audit] comment read visibility: candidate pre-scan hit the ${READ_SCAN_LIMIT}-row cap; ` + - 'the visibility filter for this broad read is fail-closed and may omit visible rows — scope the query by thread_id', + `[audit] comment read visibility: the thread probe's candidate pre-scan ` + + `(${onlyUnstamped ? 'rows with no stamped parent pair' : 'the whole read'}) hit the ${READ_SCAN_LIMIT}-row ` + + 'cap; the visibility filter for these rows is fail-closed and may omit visible rows — ' + + (onlyUnstamped + ? 'run `os migrate comment-parent-pair --apply` to stamp them' + : 'scope the query by thread_id'), ); } @@ -1008,6 +1337,6 @@ async function computeThreadVisibilityFilter( if (visibleByObject.get(target.object)?.has(target.recordId)) visibleThreads.push(threadId); } if (visibleThreads.length === 0) return READ_DENY_ALL; - return { thread_id: { $in: visibleThreads } }; + const visible = { thread_id: { $in: visibleThreads } }; + return onlyUnstamped ? { ...UNSTAMPED_ROWS, ...visible } : visible; } - diff --git a/packages/plugins/plugin-audit/src/comment-parent-pair-backfill.ts b/packages/plugins/plugin-audit/src/comment-parent-pair-backfill.ts new file mode 100644 index 00000000000..6a5fe4ef706 --- /dev/null +++ b/packages/plugins/plugin-audit/src/comment-parent-pair-backfill.ts @@ -0,0 +1,182 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0139 E7] The stored-row backfill of `sys_comment`'s writer-stamped + * parent pair, `object_name` + `record_id`. + * + * Rows written before the pair existed carry none, and the comment-thread read + * gate (`comment-access-hooks.ts`, `installCommentReadVisibility`) answers + * such a row with the bounded thread probe — at most 2,000 candidate rows per + * read, fail closed — never a `ReadableIds` leaf. So until this runs, a broad + * comment read past that window omits comments the caller may read, and its + * `total` agrees with the short answer. `os migrate comment-parent-pair` is + * its door. + * + * ## One derivation + * + * Each row is stamped by {@link planCommentParentPair}, the planner the create + * and re-point stamps use, so a stored row and a new row with the same + * `thread_id` carry the same pair. + * + * ## As the platform, through the migration door + * + * Rows are read and updated as the platform (a SYSTEM context), the shape of + * `stored-metadata-body-migration.ts` and E6's activity backfill. The pair the + * update names is re-derived by the update stamp from the row's thread as it + * is stored at write time (`comment-access-hooks.ts`). That narrows the + * read-then-write race to the one engine call: a re-point landing between this + * page's read and the row's write no longer leaves the pair naming the thread + * the comment left; one committing between the engine's own prior-row read and + * its driver write is the window every `previous`-derived hook has. ⛔ No + * conditional `where`: that would put the write on the predicate path. The + * update names the row's stored + * `updated_at` under `preserveAudit`, so + * the backfill changes no comment's `updated_at` and names no `thread_id` or + * `body` (no re-point, no edit stamp). A row whose thread names no record is + * not stamped (its pair stays `null`, and it stays hidden from every + * non-system reader); the report counts it. + * + * ## Idempotent, its own verification + * + * Only unstamped rows (`object_name` NULL) are read, so a second run finds + * nothing to stamp but the threads that name no record, and re-running and + * reading a clean report is the check. No `sys_migration` flag is recorded: + * nothing gates irreversible behaviour on it. The walk pages by `id`. + */ + +import type { IDataEngine } from '@objectstack/spec/contracts'; +import { + COMMENT_PARENT_OBJECT_COLUMN, + COMMENT_PARENT_RECORD_COLUMN, + planCommentParentPair, +} from './comment-access-hooks.js'; +import type { MigrationLogger } from './stored-metadata-body-migration.js'; + +const COMMENT_OBJECT = 'sys_comment'; + +/** The backfill reads as the platform, and writes as the platform keeping each row's audit stamp. */ +const SYSTEM_CTX = { isSystem: true } as const; +const SYSTEM_PRESERVING_CTX = { isSystem: true, preserveAudit: true } as const; + +/** Rows read per page. */ +export const COMMENT_PARENT_PAIR_BACKFILL_PAGE_SIZE = 500; + +/** The engine surface the runner needs — the real data-engine contract's members. */ +export type CommentParentPairBackfillEngine = Pick; + +/** What one backfill run did (or would do in dry-run). */ +export interface CommentParentPairBackfillReport { + apply: boolean; + /** Unstamped rows read. */ + scanned: number; + /** Rows the planner stamped (or would stamp). */ + stamped: number; + /** Unstamped rows whose thread names no record: they stay unstamped (hidden). */ + unparented: number; + /** A failed read, or a row an `update` threw on (apply mode) — re-run to finish. */ + failures: number; +} + +function asArray(result: unknown): Record[] { + if (Array.isArray(result)) return result as Record[]; + if (result && typeof result === 'object' && Array.isArray((result as { records?: unknown }).records)) { + return (result as { records: Record[] }).records; + } + return []; +} + +/** + * Stamp `object_name` and `record_id` on every unstamped `sys_comment` row. + * Dry-run by default; `apply` writes. Idempotent. + */ +export async function backfillCommentParentPair( + engine: CommentParentPairBackfillEngine, + logger: MigrationLogger, + options: { apply?: boolean; pageSize?: number } = {}, +): Promise { + const apply = options.apply === true; + const pageSize = options.pageSize && options.pageSize > 0 ? options.pageSize : COMMENT_PARENT_PAIR_BACKFILL_PAGE_SIZE; + const report: CommentParentPairBackfillReport = { apply, scanned: 0, stamped: 0, unparented: 0, failures: 0 }; + + // Keyset paging on `id`: a stamped row leaves the unstamped set, an + // unparented one stays in it, and neither is read twice in one run. + let after: string | undefined; + for (;;) { + const unstamped: Record = { [COMMENT_PARENT_OBJECT_COLUMN]: { $null: true } }; + const where = after === undefined ? unstamped : { $and: [unstamped, { id: { $gt: after } }] }; + let rows: Record[]; + try { + rows = asArray( + await engine.find( + COMMENT_OBJECT, + { + where, + fields: ['id', 'thread_id', 'updated_at'], + orderBy: [{ field: 'id', order: 'asc' }], + limit: pageSize, + }, + { context: SYSTEM_CTX }, + ), + ); + } catch (e) { + // A table this run could not read is NOT a clean table: counted, so the + // run says so (and the door exits non-zero). + report.failures += 1; + logger.warn( + `[comment-parent-pair-backfill] could not read ${COMMENT_OBJECT} — its remaining rows were NOT ` + + `examined: ${(e as Error)?.message ?? String(e)}`, + ); + break; + } + if (rows.length === 0) break; + + for (const row of rows) { + report.scanned += 1; + const pair = planCommentParentPair(row.thread_id); + if (pair.object_name === null) { + report.unparented += 1; + continue; + } + report.stamped += 1; + if (!apply) continue; + const id = row.id; + if (typeof id !== 'string' || id === '') { + report.failures += 1; + logger.warn(`[comment-parent-pair-backfill] ${COMMENT_OBJECT} row has no id — cannot stamp it`); + continue; + } + try { + // A single-id write, as the platform. `preserveAudit` with the stored + // `updated_at` keeps the row's own timestamp: stamping a derived + // pointer is not an edit of the comment. + await engine.update( + COMMENT_OBJECT, + { + id, + [COMMENT_PARENT_OBJECT_COLUMN]: pair.object_name, + [COMMENT_PARENT_RECORD_COLUMN]: pair.record_id, + ...(row.updated_at !== undefined && row.updated_at !== null ? { updated_at: row.updated_at } : {}), + }, + { context: SYSTEM_PRESERVING_CTX }, + ); + } catch (e) { + report.failures += 1; + logger.warn( + `[comment-parent-pair-backfill] failed to stamp ${COMMENT_OBJECT}/${id}: ${(e as Error)?.message ?? String(e)}`, + ); + } + } + + const last = rows[rows.length - 1]?.id; + if (rows.length < pageSize || typeof last !== 'string' || last === '') break; + after = last; + } + + logger.info?.( + `[comment-parent-pair-backfill] ${apply ? 'stamped' : 'would stamp'} ${report.stamped} of ${report.scanned} ` + + `unstamped ${COMMENT_OBJECT} row(s)` + + (report.unparented > 0 ? `; ${report.unparented} name no record and stay unstamped (hidden)` : '') + + (report.failures > 0 ? ` (${report.failures} failed — re-run to finish)` : ''), + ); + return report; +} diff --git a/packages/plugins/plugin-audit/src/comment-parent-pair.integration.test.ts b/packages/plugins/plugin-audit/src/comment-parent-pair.integration.test.ts new file mode 100644 index 00000000000..f556480e1e5 --- /dev/null +++ b/packages/plugins/plugin-audit/src/comment-parent-pair.integration.test.ts @@ -0,0 +1,550 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0139 E7] The comment-thread read gate on the LEAF path, over + * `sys_comment`'s writer-stamped parent pair, through a real engine and a real + * SQL driver — and the pair's writers and its stored-row backfill. + * + * What is measured here and nowhere else: the leaf is composed by the engine + * at its executor boundary (ADR-0139 D2) and evaluated by the driver as one + * subquery against the parent's table (D3), so a broad comment read is served + * every comment the caller may read, with a true total, past the 2,000-row + * window the thread probe had; D5's classes still read through the bounded + * probe, by name; and a row the backfill has not reached is probed by its + * thread, never leafed. + * + * The security stand-in is E5's (`parent-record-read-leaf.integration.test.ts`, + * whose header says why it is the smallest honest one): one rule per parent + * object, served as `getReadFilter` and by one GLOBAL engine middleware. + * + * - `e7_doc`: a member reads the records it owns; the admin reads them all; + * - `e7_vault`: no member may read it; the admin may; + * - `e7_gated`: narrowed by an object-scoped read middleware (D5.2); + * - `e7_far`: bound to another datasource (D5.3). + */ + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { ObjectKernel, findMisplacedReadableIds, refuseReadableIdsOnce } from '@objectstack/core'; +import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; + +import { AuditPlugin } from './audit-plugin.js'; +import { parseCommentThreadId } from './comment-access-hooks.js'; +import { backfillCommentParentPair } from './comment-parent-pair-backfill.js'; + +const COMMENT = 'sys_comment'; +const REACTION = 'sys_comment_reaction'; +const DOC = 'e7_doc'; +const VAULT = 'e7_vault'; +const GATED = 'e7_gated'; +const FAR = 'e7_far'; +const HARNESS_PACKAGE = 'com.objectstack.audit.test.comment-parent-pair'; + +/** The thread probe's window this card lifts. */ +const WINDOW = 2_000; + +const SYS = { isSystem: true } as const; +const MEMBER_A = { userId: 'u_a', tenantId: 'org_1', positions: ['org_member'] }; +const MEMBER_B = { userId: 'u_b', tenantId: 'org_1', positions: ['org_member'] }; +const ADMIN = { userId: 'u_admin', tenantId: 'org_1', positions: ['org_admin'] }; +const DELEGATED_A = { ...MEMBER_A, onBehalfOf: { userId: 'u_delegator' } }; + +/** Past the 2,000-row candidate window the leaf replaces. */ +const DOCS_OF_A = 2_300; +const DOCS_OF_B = 300; +const BATCH = 200; + +const READ_OPS = new Set(['find', 'findOne', 'count', 'aggregate']); +const isAdmin = (context: any) => context?.userId === ADMIN.userId; + +type Row = Record; + +function readScopeOf(object: string, context: any): Record | undefined { + if (context?.isSystem || isAdmin(context)) return undefined; + if (object === DOC) return { owner: context?.userId ?? '__nobody__' }; + return undefined; +} +const mayReadObject = (object: string, context: any) => context?.isSystem || object !== VAULT || isAdmin(context); + +const security = { + async canReadObject(object: string, context?: any): Promise { + return mayReadObject(object, context); + }, + async getReadFilter(object: string, context?: any): Promise | undefined> { + return readScopeOf(object, context); + }, +}; + +class WarehouseDriver extends SqliteWasmDriver { + public override readonly name: string = 'warehouse'; +} + +const objectOf = (name: string, extra: Record = {}) => ({ + name, + label: name, + fields: { + name: { name: 'name', label: 'Name', type: 'text' as const }, + owner: { name: 'owner', label: 'Owner', type: 'text' as const }, + }, + ...extra, +}); + +async function bootKernel(driver: SqliteWasmDriver) { + const kernel = new ObjectKernel({ logger: { level: 'silent' } }); + await kernel.use(new ObjectQLPlugin()); + await kernel.use(new AuditPlugin()); + await kernel.use({ + name: 'test.security-stand-in', + version: '0.0.0', + init: async (ctx: any) => ctx.registerService('security', security), + start: async () => {}, + } as any); + await kernel.bootstrap(); + const engine = kernel.getService('objectql'); + await driver.connect(); + engine.registerDriver(driver, true); + return { kernel, engine }; +} + +function installSecurityMiddleware(engine: ObjectQL): void { + engine.registerMiddleware(async (op: any, next: () => Promise) => { + if (READ_OPS.has(op.operation) && op.context && !op.context.isSystem) { + if (!mayReadObject(op.object, op.context)) { + throw Object.assign(new Error(`[Security] Access denied: operation '${op.operation}' on object '${op.object}'`), { + code: 'PERMISSION_DENIED', + statusCode: 403, + }); + } + const scope = readScopeOf(op.object, op.context); + if (scope && op.ast) op.ast.where = op.ast.where ? { $and: [op.ast.where, scope] } : scope; + } + return next(); + }); +} + +async function insertInBatches(engine: ObjectQL, object: string, rows: Row[]): Promise { + const ids: string[] = []; + for (let i = 0; i < rows.length; i += BATCH) { + const out = await engine.insert(object, rows.slice(i, i + BATCH) as any, { context: SYS }); + for (const r of (Array.isArray(out) ? out : [out]) as Row[]) ids.push(String(r.id)); + } + return ids; +} + +const commentOn = (object: string, id: string, body: string): Row => ({ + thread_id: `${object}:${id}`, + body, + author_id: 'u_fixture', +}); + +describe('[ADR-0139 E7] the comment-thread read gate over the stamped parent pair', () => { + let kernel: ObjectKernel; + let engine: ObjectQL; + let driver: SqliteWasmDriver; + const seenWhere: unknown[] = []; + const ids: Record = {}; + + /** Ground truth from each row's THREAD (never its pair), judged by the stand-in's rules. */ + const truth = async (reader: any): Promise => { + const atRest: Row[] = await engine.find(COMMENT, { limit: 100_000, context: SYS }); + const live = new Map>(); + for (const parent of [DOC, VAULT, GATED, FAR]) { + const records: Row[] = await engine.find(parent, { limit: 100_000, context: SYS }); + live.set(parent, new Map(records.map((r) => [String(r.id), r]))); + } + return atRest + .filter((row) => { + const target = parseCommentThreadId(row.thread_id); + const parent = target && live.get(target.object)?.get(target.recordId); + if (!target || !parent) return false; + if (isAdmin(reader)) return true; + if (target.object === VAULT) return false; + if (target.object === DOC || target.object === GATED) return parent.owner === reader.userId; + return true; + }) + .map((r) => String(r.id)) + .sort(); + }; + const served = async (context: any, extra: Record = {}) => + ((await engine.find(COMMENT, { limit: 100_000, ...extra, context })) as Row[]).map((r) => String(r.id)).sort(); + const atRest = async (id: string): Promise => (await engine.findOne(COMMENT, { where: { id }, context: SYS })) as Row; + + beforeAll(async () => { + ({ kernel, engine } = await bootKernel((driver = new SqliteWasmDriver({ filename: ':memory:' })))); + const warehouse = new WarehouseDriver({ filename: ':memory:' }); + await warehouse.connect(); + engine.registerDriver(warehouse); + engine.registry.registerObject(objectOf(DOC) as any, HARNESS_PACKAGE); + engine.registry.registerObject(objectOf(VAULT) as any, HARNESS_PACKAGE); + engine.registry.registerObject(objectOf(GATED) as any, HARNESS_PACKAGE); + engine.registry.registerObject(objectOf(FAR, { datasource: 'warehouse' }) as any, HARNESS_PACKAGE); + await engine.syncSchemas(); + installSecurityMiddleware(engine); + engine.registerMiddleware(async (op: any, next: () => Promise) => { + if (READ_OPS.has(op.operation) && op.context && !op.context.isSystem && !isAdmin(op.context) && op.ast) { + const mine = { owner: op.context.userId ?? '__nobody__' }; + op.ast.where = op.ast.where ? { $and: [op.ast.where, mine] } : mine; + } + return next(); + }, { object: GATED }); + // Registered last, so it sees each comment read's WHERE as the gate left it. + engine.registerMiddleware(async (op: any, next: () => Promise) => { + if (READ_OPS.has(op.operation) && op.context && !op.context.isSystem) seenWhere.push(op.ast?.where); + return next(); + }, { object: COMMENT }); + + const docs: Row[] = []; + for (let i = 0; i < DOCS_OF_A; i++) docs.push({ name: `a${i}`, owner: MEMBER_A.userId }); + for (let i = 0; i < DOCS_OF_B; i++) docs.push({ name: `b${i}`, owner: MEMBER_B.userId }); + const docIds = await insertInBatches(engine, DOC, docs); + await insertInBatches(engine, COMMENT, docIds.map((id, i) => commentOn(DOC, id, `c${i}`))); + + ids.gone = String((await engine.insert(DOC, { name: 'gone', owner: MEMBER_A.userId }, { context: SYS })).id); + ids.vault = String((await engine.insert(VAULT, { name: 'vault' }, { context: SYS })).id); + ids.gatedA = String((await engine.insert(GATED, { name: 'gated a', owner: MEMBER_A.userId }, { context: SYS })).id); + ids.gatedB = String((await engine.insert(GATED, { name: 'gated b', owner: MEMBER_B.userId }, { context: SYS })).id); + ids.far = String((await engine.insert(FAR, { name: 'far' }, { context: SYS })).id); + for (const [key, object, id] of [ + ['onGone', DOC, ids.gone], + ['onVault', VAULT, ids.vault], + ['onGatedA', GATED, ids.gatedA], + ['onGatedB', GATED, ids.gatedB], + ['onFar', FAR, ids.far], + ] as const) { + ids[key] = String((await engine.insert(COMMENT, commentOn(object, id, key), { context: SYS })).id); + } + await engine.delete(DOC, { where: { id: ids.gone }, context: SYS }); + // A writer sending its own pair, and a thread that names no record. + ids.forged = String((await engine.insert(COMMENT, { + ...commentOn(DOC, docIds[0], 'forged'), object_name: VAULT, record_id: ids.vault, + }, { context: SYS })).id); + ids.dangling = String((await engine.insert(COMMENT, { thread_id: 'watercooler', body: 'x', author_id: 'u_fixture' }, { context: SYS })).id); + ids.firstDoc = docIds[0]; + ids.lastDocOfB = docIds[docIds.length - 1]; + }, 180_000); + + afterAll(async () => { + if (kernel) await Promise.race([kernel.shutdown(), new Promise((r) => setTimeout(r, 10_000))]); + }, 30_000); + + it('control: the fixture is past the window, and every writer stamped the pair from the thread', async () => { + expect((await truth(MEMBER_A)).length).toBeGreaterThan(WINDOW); + const rows: Row[] = await engine.find(COMMENT, { limit: 100_000, context: SYS }); + for (const row of rows) { + const target = parseCommentThreadId(row.thread_id); + expect({ object_name: row.object_name ?? null, record_id: row.record_id ?? null }, String(row.id)).toEqual( + target ? { object_name: target.object, record_id: target.recordId } : { object_name: null, record_id: null }, + ); + } + // The writer's own pair never lands: the stamp is assigned over it. + expect(await atRest(ids.forged)).toMatchObject({ object_name: DOC, record_id: ids.firstDoc }); + expect(await atRest(ids.dangling)).toMatchObject({ object_name: null, record_id: null }); + }); + + it('a member is served every comment it may read past the old window, and count equals the rows served', async () => { + for (const member of [MEMBER_A, MEMBER_B]) { + const expected = await truth(member); + const rows = await served(member); + expect(rows).toEqual(expected); + expect(await engine.count(COMMENT, {}, { context: member })).toBe(rows.length); + } + }); + + it('[O2] the bound case, recorded: rows at rest, the member served and counted, the administrator total', async () => { + const reading = { + rowsAtRest: (await engine.find(COMMENT, { limit: 100_000, context: SYS })).length, + memberTruth: (await truth(MEMBER_A)).length, + memberServed: (await served(MEMBER_A)).length, + memberTotal: await engine.count(COMMENT, {}, { context: MEMBER_A }), + adminTruth: (await truth(ADMIN)).length, + adminTotal: await engine.count(COMMENT, {}, { context: ADMIN }), + }; + // A reading, never a verdict on a duration (AGENTS.md). + console.log(`E7-COMMENT-READING ${JSON.stringify(reading)}`); + expect(reading.memberServed).toBe(reading.memberTruth); + expect(reading.memberTotal).toBe(reading.memberServed); + expect(reading.adminTotal).toBe(reading.adminTruth); + }); + + it('an administrator’s newest-20 read is served 20 rows and a true total, not the window', async () => { + const page: Row[] = await engine.find(COMMENT, { orderBy: [{ field: 'created_at', order: 'desc' }], limit: 20, context: ADMIN }); + expect(page).toHaveLength(20); + const total = await engine.count(COMMENT, {}, { context: ADMIN }); + expect(total).toBe((await truth(ADMIN)).length); + expect(total).toBeGreaterThan(WINDOW); + }); + + it('an unreadable parent’s comments stay hidden: the vault, a deleted record, another member’s, and a thread naming no record', async () => { + const rows = new Set(await served(MEMBER_A)); + for (const key of ['onVault', 'onGone', 'onGatedB', 'dangling']) expect(rows.has(ids[key]), key).toBe(false); + expect(rows.has(ids.onGatedA)).toBe(true); + expect(rows.has(ids.onFar)).toBe(true); + const admin = new Set(await served(ADMIN)); + expect(admin.has(ids.onVault)).toBe(true); + expect(admin.has(ids.onGone)).toBe(false); + expect(admin.has(ids.dangling)).toBe(false); + // Scoped to one unreadable thread, a member is served nothing and counts nothing. + const where = { thread_id: `${VAULT}:${ids.vault}` }; + expect(await engine.find(COMMENT, { where, context: MEMBER_A })).toEqual([]); + expect(await engine.count(COMMENT, { where }, { context: MEMBER_A })).toBe(0); + }); + + it('one leaf per enumerated parent; D5’s middleware-gated and federated parents are probed beside them', async () => { + seenWhere.length = 0; + await served(MEMBER_A); + const where = seenWhere[0] as any; + const branches: any[] = JSON.stringify(where).includes('"$or"') ? findOr(where) : [where]; + const stampedLeaves = branches.flatMap((b) => (b?.$or ? b.$or : [b])); + const leaves = stampedLeaves.filter((b) => b?.record_id?.$in && !Array.isArray(b.record_id.$in)).map((b) => b.object_name); + expect(leaves).toEqual(expect.arrayContaining([DOC, VAULT])); + for (const probed of [GATED, FAR, COMMENT, REACTION]) expect(leaves, probed).not.toContain(probed); + expect(stampedLeaves).toEqual(expect.arrayContaining([ + { object_name: GATED, record_id: { $in: [ids.gatedA] } }, + { object_name: FAR, record_id: { $in: [ids.far] } }, + ])); + }); + + it('D5: a delegated caller reads through the bounded probe — no leaf — never wider than the truth', async () => { + seenWhere.length = 0; + const rows = await served(DELEGATED_A); + expect(JSON.stringify(seenWhere[0])).not.toMatch(/ReadableIds|__readable/i); + expect(rows.length).toBeLessThanOrEqual(WINDOW); + const expected = new Set(await truth(MEMBER_A)); + for (const id of rows) expect(expected.has(id)).toBe(true); + expect(await engine.count(COMMENT, {}, { context: DELEGATED_A })).toBe(rows.length); + }); + + it('a row the backfill has not reached is probed by its thread, never leafed — then the backfill stamps it', async () => { + // The state after the upgrade, for three rows: no pair at rest. + const firstComment = ((await engine.find(COMMENT, { where: { thread_id: `${DOC}:${ids.firstDoc}` }, limit: 1, context: SYS })) as Row[])[0]; + const rowIds = [String(firstComment.id), ids.onVault, ids.onGatedA]; + // Planted below the engine: every engine write of the pair is re-derived. + for (const id of rowIds) await driver.update(COMMENT, id, { object_name: null, record_id: null }); + for (const id of rowIds) expect((await atRest(id)).object_name ?? null).toBeNull(); + + seenWhere.length = 0; + const rows = await served(MEMBER_A); + expect(rows).toEqual(await truth(MEMBER_A)); + expect(rows).toContain(String(firstComment.id)); + expect(rows).not.toContain(ids.onVault); + // The unstamped rows' branch: a literal thread list, only over rows with no pair. + const probe = findOr(seenWhere[0]).find((b) => b?.object_name?.$null === true); + expect(probe).toEqual({ object_name: { $null: true }, thread_id: { $in: expect.any(Array) } }); + expect([...probe.thread_id.$in].sort()).toEqual([`${DOC}:${ids.firstDoc}`, `${GATED}:${ids.gatedA}`].sort()); + expect(await engine.count(COMMENT, {}, { context: MEMBER_A })).toBe(rows.length); + + const report = await backfillCommentParentPair(engine as any, { info: vi.fn(), warn: vi.fn() }, { apply: true }); + expect(report).toEqual({ apply: true, scanned: 4, stamped: 3, unparented: 1, failures: 0 }); + seenWhere.length = 0; + expect(await served(MEMBER_A)).toEqual(rows); + expect(JSON.stringify(seenWhere[0])).not.toContain('"thread_id":{"$in"'); + }); + + it('the re-point rule re-stamps the pair; a member’s own pair is never written', async () => { + const [own, other] = ((await engine.find(DOC, { where: { owner: MEMBER_A.userId }, limit: 2, context: SYS })) as Row[]).map((r) => String(r.id)); + const created = await engine.insert(COMMENT, { + thread_id: `${DOC}:${own}`, body: 'mine', author_id: 'spoofed', object_name: VAULT, record_id: ids.vault, + }, { context: MEMBER_A }); + const id = String(created.id); + expect(await atRest(id)).toMatchObject({ object_name: DOC, record_id: own, author_id: MEMBER_A.userId }); + + await engine.update(COMMENT, { thread_id: `${DOC}:${other}` }, { where: { id }, context: MEMBER_A }); + expect(await atRest(id)).toMatchObject({ thread_id: `${DOC}:${other}`, object_name: DOC, record_id: other }); + + // Naming the pair without a thread: the engine strips a member's readonly value. + await engine.update(COMMENT, { object_name: VAULT, record_id: ids.vault, body: 'still mine' }, { where: { id }, context: MEMBER_A }); + expect(await atRest(id)).toMatchObject({ body: 'still mine', object_name: DOC, record_id: other }); + + // ...and under `preserveAudit` (a REST historical import puts it on an + // ordinary caller's write), where the engine's strip keeps a readonly + // value, the stamp re-derives the pair from the row's own thread. + await engine.update(COMMENT, { object_name: VAULT, record_id: ids.vault }, { where: { id }, context: { ...MEMBER_A, preserveAudit: true } }); + expect(await atRest(id)).toMatchObject({ thread_id: `${DOC}:${other}`, object_name: DOC, record_id: other }); + await engine.update(COMMENT, { record_id: ids.vault }, { where: { id }, context: { ...MEMBER_A, preserveAudit: true } }); + expect(await atRest(id)).toMatchObject({ object_name: DOC, record_id: other }); + }); + + it('a SYSTEM multi-row update naming the pair over comments on different records is refused whole; over one thread it stores that thread\'s pair', async () => { + const [d1, d2] = ((await engine.find(DOC, { limit: 2, context: SYS })) as Row[]).map((r) => String(r.id)); + const a = String((await engine.insert(COMMENT, commentOn(DOC, d1, 'multi a'), { context: SYS })).id); + const b = String((await engine.insert(COMMENT, commentOn(DOC, d2, 'multi b'), { context: SYS })).id); + const c = String((await engine.insert(COMMENT, commentOn(DOC, d1, 'multi c'), { context: SYS })).id); + const outcome = await engine + .update(COMMENT, { object_name: VAULT, record_id: ids.vault }, { multi: true, where: { id: { $in: [a, b] } }, context: SYS } as any) + .then((value) => ({ value }), (error) => ({ error })); + expect(outcome).toHaveProperty('error'); + const error = (outcome as { error: any }).error; + expect({ code: error.code, status: error.status }).toEqual({ code: 'VALIDATION_ERROR', status: 400 }); + expect(await atRest(a)).toMatchObject({ object_name: DOC, record_id: d1 }); + expect(await atRest(b)).toMatchObject({ object_name: DOC, record_id: d2 }); + // Rows sharing one thread derive one pair: the write lands, and stores that pair. + await engine.update(COMMENT, { object_name: VAULT, record_id: ids.vault }, { multi: true, where: { id: { $in: [a, c] } }, context: SYS } as any); + expect(await atRest(a)).toMatchObject({ object_name: DOC, record_id: d1 }); + expect(await atRest(c)).toMatchObject({ object_name: DOC, record_id: d1 }); + }); + + it('D5.2: the reaction gate keeps the bounded probe — a literal comment-id list, no leaf', async () => { + const reactionWhere: unknown[] = []; + engine.registerMiddleware(async (op: any, next: () => Promise) => { + if (READ_OPS.has(op.operation) && op.context && !op.context.isSystem) reactionWhere.push(op.ast?.where); + return next(); + }, { object: REACTION }); + const own = ((await engine.find(COMMENT, { where: { thread_id: `${GATED}:${ids.gatedA}` }, context: SYS })) as Row[])[0]; + await engine.insert(REACTION, { comment_id: own.id, emoji: '👍', user_id: MEMBER_A.userId }, { context: SYS }); + await engine.insert(REACTION, { comment_id: ids.onVault, emoji: '👍', user_id: MEMBER_A.userId }, { context: SYS }); + const rows: Row[] = await engine.find(REACTION, { context: MEMBER_A }); + expect(rows.map((r) => String(r.comment_id))).toEqual([String(own.id)]); + expect(reactionWhere[0]).toEqual({ comment_id: { $in: [own.id] } }); + }); +}); + +/** The `$or` of the gate's predicate, wherever the caller's own WHERE put it. */ +function findOr(where: any): any[] { + if (where?.$or) return where.$or; + for (const part of where?.$and ?? []) { + const found = findOr(part); + if (found.length) return found; + } + return []; +} + +describe('[ADR-0139 E7] backfillCommentParentPair', () => { + let kernel: ObjectKernel; + let engine: ObjectQL; + let driver: SqliteWasmDriver; + const rowIds: string[] = []; + const stampedAt = new Map(); + const logger = () => ({ info: vi.fn(), warn: vi.fn() }); + + beforeAll(async () => { + driver = new SqliteWasmDriver({ filename: ':memory:' }); + ({ kernel, engine } = await bootKernel(driver)); + engine.registry.registerObject(objectOf(DOC) as any, HARNESS_PACKAGE); + await engine.syncSchemas(); + for (let i = 0; i < 5; i++) { + const doc = String((await engine.insert(DOC, { name: `d${i}` }, { context: SYS })).id); + rowIds.push(String((await engine.insert(COMMENT, commentOn(DOC, doc, `c${i}`), { context: SYS })).id)); + } + rowIds.push(String((await engine.insert(COMMENT, { thread_id: 'free-form', body: 'x', author_id: 'u' }, { context: SYS })).id)); + // The state after the upgrade: every row at rest, unstamped, with an + // updated_at of its own — planted below the engine, which re-derives any + // pair an engine write names. + for (const id of rowIds) { + await driver.update(COMMENT, id, { object_name: null, record_id: null, updated_at: '2020-01-01T00:00:00.000Z' }); + stampedAt.set(id, (await engine.findOne(COMMENT, { where: { id }, context: SYS }) as Row).updated_at); + } + }, 120_000); + + afterAll(async () => { + if (kernel) await Promise.race([kernel.shutdown(), new Promise((r) => setTimeout(r, 10_000))]); + }, 30_000); + + const all = async () => (await engine.find(COMMENT, { orderBy: [{ field: 'id', order: 'asc' }], context: SYS })) as Row[]; + + it('control: every row is unstamped', async () => { + for (const row of await all()) expect(row.object_name ?? null, String(row.id)).toBeNull(); + }); + + it('a dry run counts every unstamped row and writes none', async () => { + const report = await backfillCommentParentPair(engine as any, logger(), { pageSize: 2 }); + expect(report).toEqual({ apply: false, scanned: 6, stamped: 5, unparented: 1, failures: 0 }); + for (const row of await all()) expect(row.object_name ?? null).toBeNull(); + }); + + it('apply stamps each row from its thread, a page at a time, and keeps each row’s updated_at', async () => { + const report = await backfillCommentParentPair(engine as any, logger(), { apply: true, pageSize: 2 }); + expect(report).toEqual({ apply: true, scanned: 6, stamped: 5, unparented: 1, failures: 0 }); + for (const row of await all()) { + const target = parseCommentThreadId(row.thread_id); + expect({ object_name: row.object_name ?? null, record_id: row.record_id ?? null }).toEqual( + target ? { object_name: target.object, record_id: target.recordId } : { object_name: null, record_id: null }, + ); + expect(row.updated_at, String(row.id)).toEqual(stampedAt.get(String(row.id))); + expect(row.is_edited ?? false).toBe(false); + } + }); + + it('idempotent: a second run stamps nothing, and finds only the thread naming no record', async () => { + const before = await all(); + const report = await backfillCommentParentPair(engine as any, logger(), { apply: true, pageSize: 2 }); + expect(report).toEqual({ apply: true, scanned: 1, stamped: 0, unparented: 1, failures: 0 }); + expect(await all()).toEqual(before); + }); + + it('a row re-pointed between the page read and the write keeps the re-pointed pair', async () => { + const doc = String((await engine.insert(DOC, { name: 'race' }, { context: SYS })).id); + const moved = String((await engine.insert(DOC, { name: 'race target' }, { context: SYS })).id); + const id = String((await engine.insert(COMMENT, commentOn(DOC, doc, 'race'), { context: SYS })).id); + await driver.update(COMMENT, id, { object_name: null, record_id: null }); + // The engine the backfill drives re-points the row right after reading the page. + const racing = { + find: async (...args: any[]) => { + const page = await (engine.find as any)(...args); + await engine.update(COMMENT, { id, thread_id: `${DOC}:${moved}` }, { context: SYS }); + return page; + }, + update: (...args: any[]) => (engine.update as any)(...args), + }; + await backfillCommentParentPair(racing as any, logger(), { apply: true, pageSize: 500 }); + expect(await engine.findOne(COMMENT, { where: { id }, context: SYS })).toMatchObject({ + thread_id: `${DOC}:${moved}`, object_name: DOC, record_id: moved, + }); + }); + + it('a read the table refuses is a failure, never a clean report', async () => { + const log = logger(); + const failing = { find: vi.fn(async () => { throw new Error('no such table'); }), update: vi.fn() }; + const report = await backfillCommentParentPair(failing as any, log, { apply: true }); + expect(report.failures).toBe(1); + expect(failing.update).not.toHaveBeenCalled(); + expect(log.warn).toHaveBeenCalledTimes(1); + }); +}); + +/** A driver that does not compile the composed leaf, refusing it with D3's envelope, as `driver-mongodb` does. */ +class RefusingDriver extends SqliteWasmDriver { + private readonly refusalState = { logged: false }; + private refuse(query: any): void { + const site = findMisplacedReadableIds(query?.where, false); + if (site) throw refuseReadableIdsOnce('driver-e7-refusing', site, undefined, this.refusalState, 'this test driver compiles no subquery'); + } + override async find(object: string, query: any, options?: any) { + this.refuse(query); + return super.find(object, query, options); + } + override async count(object: string, query?: any, options?: any) { + this.refuse(query); + return super.count(object, query, options); + } +} + +describe('[ADR-0139 D3] on a driver that refuses the leaf, a comment read fails and says why', () => { + let kernel: ObjectKernel; + let engine: ObjectQL; + + beforeAll(async () => { + ({ kernel, engine } = await bootKernel(new RefusingDriver({ filename: ':memory:' }))); + engine.registry.registerObject(objectOf(DOC) as any, HARNESS_PACKAGE); + await engine.syncSchemas(); + const doc = String((await engine.insert(DOC, { name: 'one', owner: MEMBER_A.userId }, { context: SYS })).id); + await engine.insert(COMMENT, commentOn(DOC, doc, 'one'), { context: SYS }); + }, 120_000); + + afterAll(async () => { + if (kernel) await Promise.race([kernel.shutdown(), new Promise((r) => setTimeout(r, 10_000))]); + }, 30_000); + + it('control: a system read is served the comment', async () => { + expect((await engine.find(COMMENT, { context: SYS })).length).toBe(1); + }); + + it('a member’s list and total are refused with D3’s envelope — never served as the deny-all sentinel', async () => { + for (const read of [() => engine.find(COMMENT, { context: MEMBER_A }), () => engine.count(COMMENT, {}, { context: MEMBER_A })]) { + const outcome = await read().then((value) => ({ value }), (error) => ({ error })); + expect(outcome).toHaveProperty('error'); + const error = (outcome as { error: any }).error; + expect({ code: error.code, status: error.status }).toEqual({ code: 'INTERNAL_ERROR', status: 500 }); + expect(String(error.message)).toContain('[driver-e7-refusing]'); + } + }); +}); diff --git a/packages/plugins/plugin-audit/src/comment-thread-leaf.test.ts b/packages/plugins/plugin-audit/src/comment-thread-leaf.test.ts new file mode 100644 index 00000000000..dfae9e62603 --- /dev/null +++ b/packages/plugins/plugin-audit/src/comment-thread-leaf.test.ts @@ -0,0 +1,304 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0139 E7] The comment-thread gate's choice between a `ReadableIds` leaf + * and the bounded probe, per parent object and up front, and the parent pair's + * stamp — against a stand-in engine that records every read, so "no pre-scan" + * and "no leaf" are assertions about calls, not about shapes alone. The real + * engine composing and evaluating the leaf is + * `comment-parent-pair.integration.test.ts`. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ReadableIds } from '@objectstack/spec/data'; +import { + installCommentAccessHooks, + installCommentReadVisibility, + planCommentParentPair, + type CommentReadMiddlewareCtx, +} from './comment-access-hooks.js'; + +const silentLogger = () => ({ info: vi.fn(), warn: vi.fn(), debug: vi.fn() }); + +const PAIR_FIELDS = { thread_id: {}, body: {}, object_name: {}, record_id: {} }; + +/** + * What the stand-in answers: the comments with no stamped pair (the only rows + * this gate pre-scans), or the requested parent ids the caller may read. + */ +function standInRows( + opts: { comments?: Array>; readable?: Record }, + object: string, + requested: unknown, +): Array> { + if (object === 'sys_comment') return (opts.comments ?? []).filter((r) => r.object_name == null); + const ids = Array.isArray(requested) ? requested.map(String) : []; + return ids.filter((id) => (opts.readable?.[object] ?? []).includes(id)).map((id) => ({ id })); +} + +type Mw = (ctx: CommentReadMiddlewareCtx, next: () => Promise) => Promise; + +/** A stand-in engine: one registry, the D5 questions answered by name, every find recorded. */ +function standIn(opts: { + objects?: string[]; + middleware?: string[]; + federated?: string[]; + declaresPair?: boolean; + classify?: boolean; + /** comments as stored (system pre-scan) */ + comments?: Array>; + /** parent object → readable ids for the caller */ + readable?: Record; + findThrows?: boolean; +}) { + const finds: Array<{ object: string; options: any }> = []; + let mw!: Mw; + const engine: any = { + registerHook: () => {}, + registerMiddleware: (fn: Mw) => { + mw = fn; + }, + getSchema: (object: string) => + object === 'sys_comment' + ? { name: 'sys_comment', fields: opts.declaresPair === false ? { thread_id: {}, body: {} } : PAIR_FIELDS } + : undefined, + find: async (object: string, options: any) => { + finds.push({ object, options }); + const rows = standInRows(opts, object, options?.where?.id?.$in); + return typeof options?.limit === 'number' ? rows.slice(0, options.limit) : rows; + }, + }; + if (opts.findThrows) { + engine.find = async (object: string, options: any) => { + finds.push({ object, options }); + throw new Error('pre-scan exploded'); + }; + } + if (opts.classify !== false) { + engine.registry = { getAllObjects: () => (opts.objects ?? ['crm_case', 'sys_comment']).map((name) => ({ name })) }; + engine.hasObjectMiddleware = (object: string) => (opts.middleware ?? []).includes(object); + engine.resolveEffectiveDatasource = (object: string) => ((opts.federated ?? []).includes(object) ? 'warehouse' : undefined); + } + return { engine, finds, mw: () => mw }; +} + +async function read(mw: Mw, context: Record = { userId: 'u1' }, where?: unknown) { + const ctx: CommentReadMiddlewareCtx = { object: 'sys_comment', operation: 'find', ast: { object: 'sys_comment', where }, context: context as any }; + let ran = false; + await mw(ctx, async () => { + ran = true; + }); + return { where: ctx.ast?.where as any, ran }; +} + +const admission = { canReadObject: async () => true }; +const leavesOf = (where: any): string[] => + (where?.$or ?? [where]).filter((b: any) => b?.record_id?.$in && !Array.isArray(b.record_id.$in)).map((b: any) => b.object_name); + +describe('[ADR-0139 E7] the comment-thread gate: a leaf per parent object, the probe by name', () => { + it('writes one ReadableIds leaf per parent object and pre-scans no stamped row', async () => { + const s = standIn({ objects: ['crm_case', 'crm_deal', 'sys_comment', 'sys_session'] }); + installCommentReadVisibility(s.engine, silentLogger(), () => admission); + const { where, ran } = await read(s.mw()); + expect(ran).toBe(true); + // Every registered object but sys_comment, the audit writer's exclusions included. + expect(where).toEqual({ + $or: ['crm_case', 'crm_deal', 'sys_session'].map((p) => ({ object_name: p, record_id: { $in: ReadableIds.of(p) } })), + }); + // The only read is the probe of rows with no stamped pair. + expect(s.finds).toHaveLength(1); + expect(s.finds[0]).toMatchObject({ object: 'sys_comment', options: { where: { object_name: { $null: true } }, context: { isSystem: true } } }); + }); + + it('ORs the bounded thread probe for rows with no stamped pair — a literal list, never a leaf', async () => { + const s = standIn({ + comments: [ + { id: 'k1', thread_id: 'crm_case:c1', object_name: null }, + { id: 'k2', thread_id: 'crm_case:c2', object_name: null }, + { id: 'k3', thread_id: 'watercooler', object_name: null }, + ], + readable: { crm_case: ['c1'] }, + }); + installCommentReadVisibility(s.engine, silentLogger(), () => admission); + const { where } = await read(s.mw(), { userId: 'u1' }, { body: 'x' }); + expect(where).toEqual({ + $and: [ + { body: 'x' }, + { + $or: [ + { object_name: 'crm_case', record_id: { $in: ReadableIds.of('crm_case') } }, + { object_name: { $null: true }, thread_id: { $in: ['crm_case:c1'] } }, + ], + }, + ], + }); + expect(s.finds[0].options.where).toEqual({ $and: [{ body: 'x' }, { object_name: { $null: true } }] }); + }); + + it('D5.2 and D5.3: a middleware-gated parent and a federated parent are probed over the pair, beside the leaves', async () => { + const s = standIn({ objects: ['crm_case', 'sys_attachment', 'far_obj'], middleware: ['sys_attachment'], federated: ['far_obj'] }); + installCommentReadVisibility(s.engine, silentLogger(), () => admission); + const { where } = await read(s.mw()); + expect(leavesOf(where)).toEqual(['crm_case']); + const pairProbe = s.finds.find((f) => JSON.stringify(f.options.where).includes('"object_name":{"$in"')); + expect(pairProbe?.options.where).toEqual({ object_name: { $in: ['far_obj', 'sys_attachment'] } }); + expect(pairProbe?.options.fields).toEqual(['object_name', 'record_id']); + }); + + it('D5.1: a delegated caller writes no leaf — the whole read is the probe', async () => { + const s = standIn({ objects: ['crm_case'] }); + installCommentReadVisibility(s.engine, silentLogger(), () => admission); + const { where } = await read(s.mw(), { userId: 'u1', onBehalfOf: { userId: 'u0' } }); + expect(leavesOf(where)).toEqual([]); + expect(where).toEqual({ id: '__comment_thread_denied__' }); + }); + + it('no leaf without canReadObject: the whole read is the probe, said once', async () => { + for (const source of [undefined, () => ({}) as any, () => { throw new Error('no slot'); }]) { + const logger = silentLogger(); + const s = standIn({ objects: ['crm_case'] }); + installCommentReadVisibility(s.engine, logger, source as any); + expect(leavesOf((await read(s.mw())).where)).toEqual([]); + await read(s.mw()); + expect(logger.warn.mock.calls.filter(([m]) => String(m).includes('canReadObject'))).toHaveLength(1); + } + }); + + it('an engine that cannot classify writes no leaf — the whole read is the probe, said once', async () => { + const logger = silentLogger(); + const s = standIn({ classify: false }); + installCommentReadVisibility(s.engine, logger, () => admission); + expect(leavesOf((await read(s.mw())).where)).toEqual([]); + await read(s.mw()); + expect(logger.warn.mock.calls.filter(([m]) => String(m).includes('hasObjectMiddleware'))).toHaveLength(1); + }); + + it('a sys_comment that declares no pair is read by the thread probe over every row, said once', async () => { + const logger = silentLogger(); + const s = standIn({ declaresPair: false, comments: [{ id: 'k1', thread_id: 'crm_case:c1' }], readable: { crm_case: ['c1'] } }); + installCommentReadVisibility(s.engine, logger, () => admission); + expect((await read(s.mw())).where).toEqual({ thread_id: { $in: ['crm_case:c1'] } }); + await read(s.mw()); + expect(s.finds[0].options.where).toEqual({}); + expect(logger.warn.mock.calls.filter(([m]) => String(m).includes('declares no object_name'))).toHaveLength(1); + }); + + it('a probe that throws denies the whole read (fail closed) and says so', async () => { + const logger = silentLogger(); + const s = standIn({ findThrows: true }); + installCommentReadVisibility(s.engine, logger, () => admission); + expect((await read(s.mw())).where).toEqual({ id: '__comment_thread_denied__' }); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('denying all')); + }); + + it('[ADR-0139 D3] the consumer catches nothing: a refusal raised by the read itself reaches the caller', async () => { + const s = standIn({}); + installCommentReadVisibility(s.engine, silentLogger(), () => admission); + const refusal = Object.assign(new Error('[driver-x] cannot compile ReadableIds'), { code: 'INTERNAL_ERROR', status: 500 }); + const ctx: CommentReadMiddlewareCtx = { object: 'sys_comment', operation: 'count', ast: { object: 'sys_comment' }, context: { userId: 'u1' } }; + await expect(s.mw()(ctx, async () => { throw refusal; })).rejects.toMatchObject({ code: 'INTERNAL_ERROR', status: 500 }); + expect(ctx.ast?.where).not.toEqual({ id: '__comment_thread_denied__' }); + }); +}); + +describe('[ADR-0139 E7] the parent pair: one planner, stamped on create and on the re-point rule', () => { + it('plans the pair from the thread, by the parse every gate authorizes by', () => { + expect(planCommentParentPair('crm_case:a:b')).toEqual({ object_name: 'crm_case', record_id: 'a:b' }); + for (const none of ['crm_case:', 'watercooler', 'sys_comment:c1', 'Bad:x', undefined, 42]) { + expect(planCommentParentPair(none), String(none)).toEqual({ object_name: null, record_id: null }); + } + }); + + function hooks(declaresPair = true) { + const registered: Array<{ event: string; handler: (ctx: any) => any }> = []; + const engine: any = { + registerHook: (event: string, handler: (ctx: any) => any, options: any) => { + if (options?.object === 'sys_comment') registered.push({ event, handler }); + }, + getSchema: () => ({ fields: declaresPair ? PAIR_FIELDS : { thread_id: {} } }), + }; + installCommentAccessHooks(engine, () => null, silentLogger()); + // The stamp is the one handler that exempts no caller: drive every handler as the system. + const run = async (event: string, data: unknown, previous: unknown = { body: 'b' }) => { + for (const h of registered.filter((r) => r.event === event)) { + await h.handler({ object: 'sys_comment', event, input: { data, options: {} }, session: { isSystem: true }, previous }); + } + return data; + }; + return run; + } + + it('create: every row is stamped over whatever the writer sent, a single row and a batch', async () => { + const run = hooks(); + expect(await run('beforeInsert', { thread_id: 'crm_case:c1', object_name: 'vault', record_id: 'v1' })).toMatchObject({ + object_name: 'crm_case', + record_id: 'c1', + }); + expect(await run('beforeInsert', [{ thread_id: 'crm_case:c2' }, { thread_id: 'watercooler', object_name: 'x' }])).toEqual([ + { thread_id: 'crm_case:c2', object_name: 'crm_case', record_id: 'c2' }, + { thread_id: 'watercooler', object_name: null, record_id: null }, + ]); + }); + + it('update: a re-point re-stamps the pair; an update naming no thread stamps nothing', async () => { + const run = hooks(); + expect(await run('beforeUpdate', { thread_id: 'crm_deal:d1', object_name: 'vault' })).toMatchObject({ + object_name: 'crm_deal', + record_id: 'd1', + }); + expect(await run('beforeUpdate', { body: 'b' })).toEqual({ body: 'b' }); + }); + + it('update: a pair named without a thread is re-derived from the stored thread, or dropped when there is none', async () => { + const run = hooks(); + const stored = { body: 'b', thread_id: 'crm_case:c9' }; + expect(await run('beforeUpdate', { object_name: 'vault', record_id: 'v1' }, stored)).toEqual({ + object_name: 'crm_case', + record_id: 'c9', + }); + expect(await run('beforeUpdate', { record_id: 'v1', body: 'x' }, stored)).toMatchObject({ + body: 'x', + object_name: 'crm_case', + record_id: 'c9', + }); + expect(await run('beforeUpdate', { object_name: null, record_id: null }, { thread_id: 'watercooler' })).toEqual({ + object_name: null, + record_id: null, + }); + const dropped = (await run('beforeUpdate', { object_name: 'vault', body: 'x' }, undefined)) as Record; + expect(dropped).toMatchObject({ body: 'x' }); + expect(dropped).not.toHaveProperty('object_name'); + }); + + it('a multi-row update naming the pair: refused when the rows derive different pairs, passed when they share one', async () => { + const registered: Array<{ event: string; handler: (ctx: any) => any }> = []; + const engine: any = { + registerHook: (event: string, handler: (ctx: any) => any, options: any) => { + if (options?.object === 'sys_comment' && event === 'beforeUpdate') registered.push({ event, handler }); + }, + getSchema: () => ({ fields: PAIR_FIELDS }), + }; + installCommentAccessHooks(engine, () => null, silentLogger()); + const dispatchRows = async (threads: string[]) => { + const scope = {}; + const payload: Record = { object_name: 'vault' }; + for (const [index, thread] of threads.entries()) { + for (const h of registered) { + await h.handler({ + object: 'sys_comment', input: { data: payload, options: {} }, session: { isSystem: true }, + previous: { thread_id: thread }, dispatch: { mode: 'per-row', index, scope }, + }); + } + } + return payload; + }; + await expect(dispatchRows(['crm_case:c1', 'crm_case:c2'])).rejects.toMatchObject({ code: 'VALIDATION_ERROR', status: 400 }); + expect(await dispatchRows(['crm_case:c1', 'crm_case:c1'])).toEqual({ object_name: 'crm_case', record_id: 'c1' }); + }); + + it('a sys_comment that declares no pair is never stamped (no unknown column is written)', async () => { + const run = hooks(false); + expect(await run('beforeInsert', { thread_id: 'crm_case:c1' })).toEqual({ thread_id: 'crm_case:c1' }); + expect(await run('beforeUpdate', { thread_id: 'crm_case:c1' })).toEqual({ thread_id: 'crm_case:c1' }); + }); +}); diff --git a/packages/plugins/plugin-audit/src/index.ts b/packages/plugins/plugin-audit/src/index.ts index 7456ae3be0a..859376bf4b3 100644 --- a/packages/plugins/plugin-audit/src/index.ts +++ b/packages/plugins/plugin-audit/src/index.ts @@ -36,7 +36,21 @@ export { installCommentAccessHooks, installCommentReadVisibility, parseCommentThreadId, + planCommentParentPair, + COMMENT_PARENT_OBJECT_COLUMN, + COMMENT_PARENT_RECORD_COLUMN, } from './comment-access-hooks.js'; +// [ADR-0139 E7] The stored-row backfill of `sys_comment`'s writer-stamped +// parent pair, which the comment-thread read gate leafs over. The CLI command +// (`os migrate comment-parent-pair`) drives it. +export { + backfillCommentParentPair, + COMMENT_PARENT_PAIR_BACKFILL_PAGE_SIZE, +} from './comment-parent-pair-backfill.js'; +export type { + CommentParentPairBackfillEngine, + CommentParentPairBackfillReport, +} from './comment-parent-pair-backfill.js'; // [#21120] The one-off rewrite of at-rest cleartext metadata-body copies this // writer left in `sys_audit_log` / `sys_activity`, and the pure planners the // CLI command (`os migrate audit-metadata-bodies`) drives. @@ -70,6 +84,7 @@ export type { ActivityStamp } from './activity-changed-fields.js'; export type { CommentAccessEngine, CommentAccessLogger, + CommentParentPair, CommentReadMiddlewareCtx, CommentSecurityLike, CommentSharingLike, diff --git a/packages/plugins/plugin-audit/src/objects/sys-comment.object.ts b/packages/plugins/plugin-audit/src/objects/sys-comment.object.ts index 0bd44fb5cfd..62bbb7b149b 100644 --- a/packages/plugins/plugin-audit/src/objects/sys-comment.object.ts +++ b/packages/plugins/plugin-audit/src/objects/sys-comment.object.ts @@ -96,6 +96,32 @@ import { ObjectSchema, Field } from '@objectstack/spec/data'; * (historical import) update keeps a supplied value, as it does for * `updated_at`. * + * ## Platform-written: the parent pair `object_name` / `record_id` (ADR-0139 E7) + * + * The record `thread_id` names, stamped as ADR-0052 §5's pointer pair so the + * read gate can narrow comments with one `ReadableIds` leaf per parent object + * instead of a bounded pre-scan (`comment-access-hooks.ts`, + * `installCommentReadVisibility`). `thread_id` stays the authored key; the + * pair is derived from it by one planner (`planCommentParentPair`): + * + * - **The stamp.** On create, every row is stamped from its `thread_id`, for + * every caller and over any value sent. On an update that names `thread_id` + * (the re-point the update gate authorizes as an insert into the new + * thread), the pair is re-stamped from it. A thread naming no record stamps + * `null` / `null`. + * - **Read-only to authors.** Both declare `readonly: true`, and no writer's + * value lands: on create the stamp is assigned over it, and an update that + * names either column without a thread has the pair re-derived from the + * row's stored thread — for every caller, because under `preserveAudit` (a + * historical import) the engine's update-side strip keeps a readonly value. + * A multi-row update naming the pair without a thread is refused whole + * (`VALIDATION_ERROR` / 400) when its rows' threads name different records: + * one write cannot store each row's own pair. + * - **Stored rows.** Rows written before the pair existed carry `null` until + * `os migrate comment-parent-pair --apply` stamps them; until then the read + * gate answers them with the bounded thread probe (fail closed), never a + * leaf. + * * @namespace sys */ export const SysComment = ObjectSchema.create({ @@ -130,6 +156,28 @@ export const SysComment = ObjectSchema.create({ group: 'Thread', }), + // [ADR-0139 E7, ADR-0052 §5] The record the thread names, as the pointer + // pair every other parent-derived stream keys on. Platform-written from + // `thread_id` (see "Platform-written: the parent pair" above); never + // authored. + object_name: Field.text({ + label: 'Parent Object', + required: false, + readonly: true, + maxLength: 255, + description: 'The object of the record `thread_id` names — stamped by the platform, never written by authors', + group: 'Thread', + }), + + record_id: Field.text({ + label: 'Parent Record', + required: false, + readonly: true, + maxLength: 255, + description: 'The id of the record `thread_id` names — stamped by the platform, never written by authors', + group: 'Thread', + }), + // The reply relationship is `parent_id` and nothing else — a reply count is // an aggregate over the children, computed at read time (#4756). parent_id: Field.lookup('sys_comment', { @@ -217,6 +265,9 @@ export const SysComment = ObjectSchema.create({ indexes: [ { fields: ['thread_id', 'created_at'] }, + // [ADR-0139 E7] The thread gate's leaf is one `record_id IN (…)` per + // `object_name`, and its probe of unbackfilled rows is `object_name IS NULL`. + { fields: ['object_name', 'record_id'] }, { fields: ['parent_id'] }, { fields: ['author_id'] }, ], diff --git a/packages/plugins/plugin-audit/src/translations/en.objects.generated.ts b/packages/plugins/plugin-audit/src/translations/en.objects.generated.ts index 80b2c0d4498..d8f38e3c76e 100644 --- a/packages/plugins/plugin-audit/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-audit/src/translations/en.objects.generated.ts @@ -204,6 +204,14 @@ export const enObjects: NonNullable = { label: "Thread", help: "Thread identifier — conventionally `{object}:{record_id}` (e.g. `account:9001`)" }, + object_name: { + label: "Parent Object", + help: "The object of the record `thread_id` names — stamped by the platform, never written by authors" + }, + record_id: { + label: "Parent Record", + help: "The id of the record `thread_id` names — stamped by the platform, never written by authors" + }, parent_id: { label: "Parent Comment", help: "Optional parent comment for nested replies" diff --git a/packages/plugins/plugin-audit/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-audit/src/translations/es-ES.objects.generated.ts index 5e35559b5f1..5e2e4ad6224 100644 --- a/packages/plugins/plugin-audit/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-audit/src/translations/es-ES.objects.generated.ts @@ -204,6 +204,14 @@ export const esESObjects: NonNullable = { label: "Hilo", help: "Identificador del hilo; por convención `{object}:{record_id}` (p. ej. `account:9001`)." }, + object_name: { + label: "Objeto principal", + help: "El objeto del registro que nombra `thread_id`; lo registra la plataforma y los autores no lo escriben." + }, + record_id: { + label: "Registro principal", + help: "El id del registro que nombra `thread_id`; lo registra la plataforma y los autores no lo escriben." + }, parent_id: { label: "Comentario principal", help: "Comentario principal opcional para respuestas anidadas." diff --git a/packages/plugins/plugin-audit/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-audit/src/translations/ja-JP.objects.generated.ts index 59ede934ec5..a768237a380 100644 --- a/packages/plugins/plugin-audit/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-audit/src/translations/ja-JP.objects.generated.ts @@ -204,6 +204,14 @@ export const jaJPObjects: NonNullable = { label: "スレッド", help: "スレッド識別子 — 通常は `{object}:{record_id}`(例: `account:9001`)" }, + object_name: { + label: "親オブジェクト", + help: "`thread_id` が指すレコードのオブジェクト — プラットフォームが記録し、作成者は書き込めません" + }, + record_id: { + label: "親レコード", + help: "`thread_id` が指すレコードの ID — プラットフォームが記録し、作成者は書き込めません" + }, parent_id: { label: "親コメント", help: "ネストした返信用のオプションの親コメント" diff --git a/packages/plugins/plugin-audit/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-audit/src/translations/zh-CN.objects.generated.ts index 1df7465a740..bf5f0be8b88 100644 --- a/packages/plugins/plugin-audit/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-audit/src/translations/zh-CN.objects.generated.ts @@ -204,6 +204,14 @@ export const zhCNObjects: NonNullable = { label: "线程", help: "线程标识——约定格式为 `{object}:{record_id}`(例如 `account:9001`)" }, + object_name: { + label: "父对象", + help: "`thread_id` 指向的记录所属的对象——由平台写入,作者不可填写" + }, + record_id: { + label: "父记录", + help: "`thread_id` 指向的记录的 ID——由平台写入,作者不可填写" + }, parent_id: { label: "父评论", help: "可选的父评论,用于嵌套回复" diff --git a/packages/qa/dogfood/test/comment-thread-parent-pair.dogfood.test.ts b/packages/qa/dogfood/test/comment-thread-parent-pair.dogfood.test.ts new file mode 100644 index 00000000000..106f14b1305 --- /dev/null +++ b/packages/qa/dogfood/test/comment-thread-parent-pair.dogfood.test.ts @@ -0,0 +1,324 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [ADR-0139 E7] The comment-thread read gate writes one ReadableIds leaf per +// parent object over `sys_comment`'s writer-stamped parent pair, with no +// pre-scan and no window, on a real boot at the HTTP door. +// +// ## What changed +// +// A comment is readable when the record its `thread_id` names is. The gate +// used to decide it from a SYSTEM pre-scan of at most 2,000 candidate rows, +// which capped every broad comment read: a member was served 1,765 of the +// 2,300 comments it may read, and an administrator's total read 2,000 of +// 2,601. Every comment now carries `object_name` + `record_id`, stamped from +// its thread, and the gate narrows by them. A row with no stamped pair (one +// written before the upgrade, until `os migrate comment-parent-pair` stamps +// it) keeps the bounded thread probe — never a leaf. +// +// ## The composition +// +// `bootStack` with the real `SecurityPlugin`, `ObjectQL`, SQL driver, REST and +// auth layers, plus `AuditPlugin`. A member owns some private records; a +// writer owns others the member cannot open. Each record carries one comment, +// so every comment is its own thread: the old probe's window bounded threads, +// and only a fixture of more than twice the window of THREADS measures it. The +// comments are inserted as the system (the before-insert hook stamps each from +// its thread). +// +// `@objectstack/plugin-audit` resolves through its BUILT output here. +// Fixtures are synthetic. ⚠️ No test title states a value. + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { AuditPlugin } from '@objectstack/plugin-audit'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field, isReadableIds } from '@objectstack/spec/data'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { assertArmed, armedWhen } from './armed.js'; +import { catalogPermissionSet } from './catalog-permission-set.js'; + +const COMMENT = 'sys_comment'; +const OBJ = 'ctp_item'; +const SYS = { isSystem: true } as const; +/** The old pre-scan window. */ +const OLD_WINDOW = 2_000; +/** Records the member owns, one comment each: served to it whole. */ +const MINE_BULK = 2_050; +/** Records the writer owns, one comment each, which the member cannot open. */ +const OTHER_BULK = 2_101; +/** Every fixture comment's author: the filter a fixture read is scoped by. */ +const AUTHOR = 'usr_ctp_fixture'; +/** Rows whose pair is cleared below: the state of a row the backfill has not reached. */ +const UNSTAMPED = 5; +const BATCH = 250; +const PASSWORD = 'Ctp-Fixture-Pass-23857'; + +const Item = ObjectSchema.create({ + name: OBJ, + label: 'CTP Item', + pluralLabel: 'CTP Items', + // sharingModel omitted: a custom object defaults to private (ADR-0090). + fields: { name: Field.text({ label: 'Name', required: true }) }, +}); + +const fixtureStack = defineStack({ + manifest: { + id: 'com.dogfood.comment-thread-parent-pair', + namespace: 'ctp', + version: '0.0.0', + type: 'app', + name: 'Comment Thread Parent Pair Fixture', + description: 'One private object whose comments a member and an administrator read past the old pre-scan window.', + }, + objects: [Item], +}); + +const write = { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }; +const baselineSet = PermissionSetSchema.parse({ + name: 'ctp_baseline', + label: 'CTP baseline', + objects: { [OBJ]: write, [COMMENT]: write }, +}); + +type Row = Record; +const rowsOf = (body: any): Row[] => body?.records ?? body?.data ?? (Array.isArray(body) ? body : []); +const filterOf = (where: Record) => encodeURIComponent(JSON.stringify(where)); + +describe('[ADR-0139 E7] the comment-thread gate leafs over the parent pair: every comment a reader may read, past the old window', () => { + let stack: VerifyStack; + let ql: any; + const token = { admin: '', member: '', writer: '' }; + const id = { mine: '', other: '' }; + const users = { member: '', writer: '' }; + const bulk = { mine: [] as string[], other: [] as string[] }; + /** Every read of sys_comment, with whether it ran as the system. */ + const commentReads: Array<{ method: string; system: boolean; where: string; fields: string; leaf: boolean }> = []; + /** The WHERE each non-system comment read carried past the gate. */ + const gatedWheres: unknown[] = []; + const hasLeaf = (node: unknown): boolean => + isReadableIds(node) || + (Array.isArray(node) ? node.some(hasLeaf) : !!node && typeof node === 'object' && Object.values(node).some(hasLeaf)); + + const list = async (who: keyof typeof token, query: string) => { + const res = await stack.apiAs(token[who], 'GET', `/data/${COMMENT}?${query}`); + expect(res.status).toBe(200); + const body = (await res.json()) as any; + return { rows: rowsOf(body), total: Number(body.total) }; + }; + const walk = async (who: keyof typeof token, where: Record) => { + const ids: string[] = []; + const totals = new Set(); + for (let skip = 0; skip < 10_000; skip += 500) { + const page = await list(who, `$filter=${filterOf(where)}&$orderby=id&$top=500&$skip=${skip}`); + totals.add(page.total); + ids.push(...page.rows.map((r) => String(r.id))); + if (page.rows.length < 500) break; + } + return { ids, totals: [...totals] }; + }; + /** Every fixture comment: a read over more than twice the old window of candidate threads. */ + const bothThreads = () => ({ author_id: AUTHOR }); + + beforeAll(async () => { + stack = await bootStack(fixtureStack as unknown as Parameters[0], { + security: new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, baselineSet], + fallbackPermissionSet: baselineSet.name, + }), + extraPlugins: [new AuditPlugin()], + }); + ql = await stack.kernel.getServiceAsync('objectql'); + token.admin = await stack.signIn(); + const userIdOf = async (email: string) => + String((await ql.findOne('sys_user', { where: { email }, context: { ...SYS } }))?.id ?? ''); + for (const who of ['member', 'writer'] as const) { + const email = `ctp-${who}@verify.test`; + token[who] = await stack.signUp(email, PASSWORD); + users[who] = await userIdOf(email); + const set = await catalogPermissionSet(ql, baselineSet.name); + expect(set, 'fixture permission set served by the security catalog').toBeTruthy(); + await ql.insert('sys_user_permission_set', { user_id: await userIdOf(email), permission_set: set.name }, { context: { ...SYS } }); + } + const idOf = async (res: Response) => { + expect(res.status, 'fixture write').toBeLessThan(300); + const j = (await res.json()) as any; + return String(j.id ?? j.record?.id ?? j.data?.id); + }; + id.mine = await idOf(await stack.apiAs(token.member, 'POST', `/data/${OBJ}`, { name: 'mine' })); + id.other = await idOf(await stack.apiAs(token.writer, 'POST', `/data/${OBJ}`, { name: 'other' })); + + // The bulk, inserted as the system: one record per comment, each owned by + // its member, and the before-insert hook stamps each comment's pair from + // its thread. + let n = 0; + const plant = async (owner: string, count: number, into: string[]) => { + for (let i = 0; i < count; i += BATCH) { + const size = Math.min(BATCH, count - i); + const records = await ql.insert( + OBJ, + Array.from({ length: size }, (_, k) => ({ name: `ctp record ${i + k}`, owner_id: owner })), + { context: { ...SYS } }, + ); + const rows = ((Array.isArray(records) ? records : [records]) as Row[]).map((record) => { + n += 1; + return { thread_id: `${OBJ}:${record.id}`, body: `ctp ${n}`, author_id: AUTHOR }; + }); + const out = await ql.insert(COMMENT, rows, { context: { ...SYS } }); + for (const r of (Array.isArray(out) ? out : [out]) as Row[]) into.push(String(r.id)); + } + }; + await plant(users.member, MINE_BULK, bulk.mine); + await plant(users.writer, OTHER_BULK, bulk.other); + + // Registered after AuditPlugin's gate, so it sees each read's WHERE as the gate left it. + ql.registerMiddleware(async (op: any, next: () => Promise) => { + if (op.context && !op.context.isSystem && ['find', 'count'].includes(op.operation)) gatedWheres.push(op.ast?.where); + return next(); + }, { object: COMMENT }); + + for (const method of ['find', 'findOne', 'count', 'aggregate'] as const) { + const original = ql[method].bind(ql); + vi.spyOn(ql, method).mockImplementation(async (...args: any[]) => { + if (args[0] === COMMENT) { + const ctx = args[1]?.context ?? args[2]?.context; + commentReads.push({ + method, + system: ctx?.isSystem === true, + where: JSON.stringify(args[1]?.where ?? null), + fields: JSON.stringify(args[1]?.fields ?? null), + leaf: hasLeaf(args[1]?.where), + }); + } + return original(...args); + }); + } + + await assertArmed([ + armedWhen({ + control: 'the store holds more than twice the old window, and each bulk comment carries the pair of its thread', + disarmedBy: 'a store inside the window, or rows the hook did not stamp, would let every case below pass on a capped or probed read', + observe: async () => { + const atRest: Row[] = await ql.find(COMMENT, { where: bothThreads(), limit: 10_000, context: { ...SYS } }); + return { + atRest: atRest.length, + stamped: atRest.filter((r) => r.object_name === OBJ && r.thread_id === `${OBJ}:${r.record_id}`).length, + }; + }, + armed: (o) => o.atRest > 2 * OLD_WINDOW && o.stamped === o.atRest, + describe: (o) => JSON.stringify(o), + }), + armedWhen({ + control: 'the member opens its own record and not the writer’s', + disarmedBy: 'a member who could open the writer’s record would make the hidden cases below measure nothing', + observe: async () => { + const recordOf = async (commentId: string) => + String((await ql.findOne(COMMENT, { where: { id: commentId }, context: { ...SYS } })).record_id); + return { + mine: (await stack.apiAs(token.member, 'GET', `/data/${OBJ}/${id.mine}`)).status, + other: (await stack.apiAs(token.member, 'GET', `/data/${OBJ}/${id.other}`)).status, + bulkMine: (await stack.apiAs(token.member, 'GET', `/data/${OBJ}/${await recordOf(bulk.mine[0])}`)).status, + bulkOther: (await stack.apiAs(token.member, 'GET', `/data/${OBJ}/${await recordOf(bulk.other[0])}`)).status, + }; + }, + armed: (o) => + o.mine === 200 && o.bulkMine === 200 && (o.other === 404 || o.other === 403) && (o.bulkOther === 404 || o.bulkOther === 403), + describe: (o) => JSON.stringify(o), + }), + ]); + }, 600_000); + + afterAll(async () => { + vi.restoreAllMocks(); + await stack?.stop(); + }); + + it('a member is served every comment it may read, past the old window, and every page reports that total', async () => { + const expected = [...bulk.mine].sort(); + expect(expected.length).toBeGreaterThan(OLD_WINDOW); + const { ids, totals } = await walk('member', bothThreads()); + // A reading, recorded beside the verdict. + console.log(`E7-DOGFOOD-READING ${JSON.stringify({ visible: expected.length, served: ids.length, totals })}`); + expect([...ids].sort()).toEqual(expected); + expect(totals).toEqual([expected.length]); + }); + + it('the member is served no comment on a record it cannot open, by list, total or thread', async () => { + const { ids } = await walk('member', bothThreads()); + const served = new Set(ids); + expect(bulk.other.filter((rowId) => served.has(rowId))).toEqual([]); + const otherThread = (await ql.findOne(COMMENT, { where: { id: bulk.other[0] }, context: { ...SYS } })).thread_id; + const scoped = await list('member', `$filter=${filterOf({ thread_id: otherThread })}&$top=5`); + expect({ rows: scoped.rows.length, total: scoped.total }).toEqual({ rows: 0, total: 0 }); + }); + + it('an administrator’s newest-20 read is served 20 rows and a total past the old window', async () => { + const { rows, total } = await list('admin', '$orderby=created_at%20desc&$top=20'); + expect(rows).toHaveLength(20); + const fixture = (await list('admin', `$filter=${filterOf(bothThreads())}&$top=1`)).total; + expect(fixture).toBe(MINE_BULK + OTHER_BULK); + expect(total).toBeGreaterThanOrEqual(fixture); + expect(total).toBeGreaterThan(2 * OLD_WINDOW); + }); + + it('no SYSTEM read of the comments carries a leaf; the only SYSTEM reads are the bounded probes of unstamped rows and of D5’s parents', async () => { + commentReads.length = 0; + gatedWheres.length = 0; + await list('member', `$filter=${filterOf(bothThreads())}&$top=20`); + await list('admin', '$orderby=created_at%20desc&$top=20'); + const system = commentReads.filter((r) => r.system); + expect(commentReads.length).toBeGreaterThan(system.length); + expect(system.filter((r) => r.leaf)).toEqual([]); + const probeShapes = [JSON.stringify(['thread_id']), JSON.stringify(['object_name', 'record_id'])]; + expect(system.filter((r) => !probeShapes.includes(r.fields))).toEqual([]); + // The thread probe reads only rows with no stamped pair. + for (const r of system.filter((s) => s.fields === probeShapes[0])) expect(r.where).toContain('"object_name":{"$null":true}'); + // The member's own read reaches the driver carrying the leaf the gate wrote. + expect(gatedWheres.some(hasLeaf)).toBe(true); + }); + + it('a member’s historical import (treatAsHistorical) naming a foreign pair stores its thread’s pair', async () => { + const posted = await stack.apiAs(token.member, 'POST', `/data/${COMMENT}`, { thread_id: `${OBJ}:${id.mine}`, body: 'ctp import probe' }); + expect(posted.status, 'fixture comment').toBeLessThan(300); + const j = (await posted.json()) as any; + const commentId = String(j.id ?? j.record?.id ?? j.data?.id); + const res = await stack.apiAs(token.member, 'POST', `/data/${COMMENT}/import`, { + format: 'json', + writeMode: 'update', + matchFields: ['id'], + treatAsHistorical: true, + rows: [{ id: commentId, object_name: OBJ, record_id: id.other }], + }); + expect(res.status).toBe(200); + const report: any = await res.json(); + const atRest = await ql.findOne(COMMENT, { where: { id: commentId }, context: { ...SYS } }); + // The import updated the row, and the stored pair is still the thread's. + expect({ updated: report.updated, ok: report.results?.[0]?.ok }).toEqual({ updated: 1, ok: true }); + expect({ object_name: atRest.object_name, record_id: atRest.record_id }).toEqual({ object_name: OBJ, record_id: id.mine }); + // ...so the writer, who reads `id.other` and not `id.mine`, is not served it. + const writerScoped = await list('writer', `$filter=${filterOf({ id: commentId })}&$top=1`); + expect(writerScoped.rows).toEqual([]); + }); + + it('a row the backfill has not reached is probed by its thread, not leafed: still served when readable, still hidden when not', async () => { + const cleared = [...bulk.mine.slice(0, UNSTAMPED), ...bulk.other.slice(0, UNSTAMPED)]; + // Planted below the engine, as a row from before the upgrade is: every + // engine write of the pair is re-derived from the row's thread. + const driver = ql.getDriver(COMMENT); + for (const rowId of cleared) await driver.update(COMMENT, rowId, { object_name: null, record_id: null }); + try { + commentReads.length = 0; + const { ids, totals } = await walk('member', bothThreads()); + expect([...ids].sort()).toEqual([...bulk.mine].sort()); + expect(totals).toEqual([bulk.mine.length]); + const probes = commentReads.filter((r) => r.system && r.fields === JSON.stringify(['thread_id'])); + expect(probes.length).toBeGreaterThan(0); + expect(probes.every((r) => r.where.includes('"object_name":{"$null":true}'))).toBe(true); + } finally { + // The engine write re-derives each row's pair from its thread: the backfill's own write path. + for (const rowId of cleared) await ql.update(COMMENT, { id: rowId, object_name: null, record_id: null }, { context: { ...SYS } }); + const restored: Row[] = await ql.find(COMMENT, { where: { id: { $in: cleared } }, context: { ...SYS } }); + expect(restored.every((r) => r.thread_id === `${r.object_name}:${r.record_id}`)).toBe(true); + } + }); +});