diff --git a/.changeset/metadata-protocol-list-commits-created-at-iso.md b/.changeset/metadata-protocol-list-commits-created-at-iso.md new file mode 100644 index 0000000000..9a95040020 --- /dev/null +++ b/.changeset/metadata-protocol-list-commits-created-at-iso.md @@ -0,0 +1,21 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +`listCommits` now emits the ISO-8601 string its declared return type +(`createdAt?: string`) promises, instead of asserting the raw driver value + +`created_at` on `sys_metadata_commit` is an engine-injected audit column; +`SqlDriver#formatOutput` repairs it only inside its `if (this.isSqlite)` +arm, so Postgres and MySQL hand it out of the record read door as a JS +`Date` — a value every in-process consumer of `listCommits` received in a +field the type said was a `string`. The REST door (`GET +/packages/:id/commits`) was unaffected: `JSON.stringify` already renders a +`Date` as canonical ISO-Z text, so only in-process callers saw the mismatch. + +The repair is a narrow per-site conversion at the producer: an already- +canonical SQLite string and an absent column both pass through unchanged, +and — deliberately — so does an Invalid `Date`, rather than adopting the +shared `canonicalIsoInstant` spelling, which raises `RangeError` on that one +shape (measured reachable on both live dialects; the open subject of a +separate, unresolved card this change does not decide). diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index a9ec7d7e0a..be7b12f001 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -112,7 +112,7 @@ that silently does not happen. | 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11290` | | 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11473` | | 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:10025` | -| 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1747` | +| 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1795` | | 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:10073`, `readonly-strict-errors.ts:66` | | 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5892` | | 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3736`, `:3746`, `:3773` | diff --git a/packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts b/packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts new file mode 100644 index 0000000000..77b79df842 --- /dev/null +++ b/packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts @@ -0,0 +1,185 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14038] `listCommits`'s declared return type says `createdAt?: string`, + * but the mapping assigned the RAW driver value straight through: + * `...(r.created_at ? { createdAt: r.created_at } : {})`. `rows` is `any[]`, + * so tsc saw a `string` field and never checked it against what a driver + * actually hands back. + * + * ## The defect + * + * `created_at` is an engine-injected audit column: it is not in + * `datetimeFields`, and `SqlDriver#formatOutput` repairs it (both the + * builtin-audit-column repair and the `datetimeFields` fold) only inside its + * `if (this.isSqlite)` arm (`sql-driver.ts`, `formatOutput`). Postgres and + * MySQL therefore hand this column out of the record read door as a JS + * `Date`, while the SQLite family hands out canonical ISO-Z text — pinned + * live in + * `packages/drivers/driver-sql/src/sql-driver-13567-audit-stamp-materialisation.test.ts`. + * So on the production default driver, `listCommits` handed every + * in-process consumer a `Date` in a field the type says is a `string`. + * + * ## Why the fixture drives a hand-made `Date` + * + * `@objectstack/metadata-protocol` has no driver dependency and must not + * grow one — the layering runs the other way, the same split + * `sql-driver-13567-audit-stamp-materialisation.test.ts` documents. The + * `Date` here is hand-made rather than read off a live driver, matching the + * sibling `protocol.commit-timeline-instant-order.test.ts` (#13995) and the + * #14037 family's own fixtures. + * + * ## Route: a narrow per-site conversion, NOT the shared `canonicalIsoInstant` + * + * #14037 took this exact route for its five sibling sites and deliberately + * did NOT adopt `canonicalIsoInstant` (`sys-metadata-repository.ts` / + * `database-loader.ts`), because #14078 measured an Invalid `Date` reachable + * on BOTH live dialects (a MySQL zero datetime; any Postgres year in + * 275760..294276) where that spelling's `value.toISOString()` raises + * `RangeError`, and #13973 is `pm:blocked` on that ruling. This card follows + * #14037's precedent: `isoFromValidDate` in `protocol.ts` converts the ONE + * measured shape (a valid `Date`) and returns every other shape — including + * an Invalid `Date` — UNCHANGED. §D below is the pin that this card does not + * decide #14078: it goes red the moment anyone swaps the contested spelling + * into this site. + * + * ## Reverse verification, direction predicted BEFORE running + * + * Restoring the raw assignment (`createdAt: r.created_at`) turns §A red (the + * emitted value is a `Date` instance, `typeof` is `'object'`, and + * `JSON.stringify` — not `Date` equality — is what the old REST door hid + * behind) while §B, §C and §D stay green: an already-canonical SQLite string + * is unaffected by either spelling, and neither spelling converts an Invalid + * `Date`. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackProtocolImplementation } from './protocol.js'; +import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core'; + +/** Canonical instant text — exactly what `Date.prototype.toISOString` emits. */ +const ISO_Z = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + +/** + * The instant the live `Date`-materialising dialects hand back. Non-zero + * milliseconds on purpose: `String(date)` / `date.toString()` both drop + * them, so a truncating regression would stay observable rather than + * coincide with the canonical text. + */ +const PG_INSTANT = new Date('2026-03-04T05:06:07.089Z'); + +/** What SQLite hands out for the same instant — already the declared shape. */ +const SQLITE_TEXT = '2026-03-04T05:06:07.089Z'; + +/** A registry with nothing in it — the commit store is the only source here. */ +function emptyRegistry() { + return { + getObject: () => undefined, + getItem: () => undefined, + listItems: () => [], + applyNavContributions: (x: any) => x, + isPackageDisabled: () => false, + getObjectOwner: () => undefined, + }; +} + +/** One `sys_metadata_commit` row, in the driver's snake_case wire shape. */ +function commitRow(createdAt: unknown) { + return { + id: 'cmt_1', + package_id: 'pkg_crm', + organization_id: null, + operation: 'apply', + message: 'commit 1', + actor: 'alice', + item_count: 1, + items: JSON.stringify([{ type: 'object', name: 'acct', existedBefore: true, prevVersion: 3 }]), + created_at: createdAt, + }; +} + +/** An engine that answers the commit-store read out of `rows`. Read-only: `listCommits` never writes. */ +function engineWithCommits(rows: any[]) { + return { + registry: emptyRegistry(), + find: vi.fn(async () => rows), + findOne: vi.fn(async (object: string, query?: EngineFindOneQueryInput) => { + assertEngineFindOnePredicate(object, query); + const id = (query as any)?.where?.id; + return rows.find((r) => r.id === id) ?? null; + }), + } as any; +} + +describe('[#14038] listCommits emits the ISO-8601 string createdAt is declared as', () => { + describe('§A the `Date`-materialising dialects (Postgres, MySQL)', () => { + it('canonicalises a JS `Date` to a canonical ISO-Z string', async () => { + const p = new ObjectStackProtocolImplementation(engineWithCommits([commitRow(PG_INSTANT)])); + + // Non-vacuity guard: a fixture that silently degraded to a string + // would keep this file green while measuring nothing. + expect(PG_INSTANT).toBeInstanceOf(Date); + + const commits = await p.listCommits({ packageId: 'pkg_crm' }); + + expect(commits).toHaveLength(1); + expect(typeof commits[0]!.createdAt).toBe('string'); + expect(commits[0]!.createdAt).toMatch(ISO_Z); + expect(commits[0]!.createdAt).toBe(PG_INSTANT.toISOString()); + }); + }); + + describe('§B the ISO-text dialects (SQLite family, memory) are unaffected', () => { + it('passes an already-canonical string through byte-identically', async () => { + const p = new ObjectStackProtocolImplementation(engineWithCommits([commitRow(SQLITE_TEXT)])); + + const commits = await p.listCommits({ packageId: 'pkg_crm' }); + + // Idempotent: the dialect that was already correct must not be reshaped. + expect(commits[0]!.createdAt).toBe(SQLITE_TEXT); + }); + }); + + describe('§C an absent column stays absent', () => { + it('omits `createdAt` rather than inventing a value', async () => { + const row = commitRow(undefined); + delete (row as any).created_at; + const p = new ObjectStackProtocolImplementation(engineWithCommits([row])); + + const commits = await p.listCommits({ packageId: 'pkg_crm' }); + + expect(commits[0]!.createdAt).toBeUndefined(); + expect('createdAt' in commits[0]!).toBe(false); + }); + }); + + describe('§D #14078 neutrality — an Invalid `Date` is NOT converted here', () => { + /** + * ⛔ This card does not decide #14078. An Invalid `Date` is measured + * reachable on both live dialects (a MySQL zero datetime; any + * Postgres year in 275760..294276), and whether the shared + * canonical-ISO spelling (`canonicalIsoInstant`) should throw on it + * (option A) or fall back to a rendering (option B) is a maintainer + * call across four packages. Until it is ruled, this site hands that + * one shape through exactly as it does today — no new throw, no + * invented rendering. This case is what makes that a PIN rather than + * a claim: it goes red the moment `canonicalIsoInstant` (or any + * spelling that reaches `.toISOString()` unconditionally) is swapped + * into `listCommits`. + */ + it('hands the value through unchanged instead of raising RangeError', async () => { + const invalid = new Date(NaN); + expect(Number.isNaN(invalid.getTime())).toBe(true); + // The contested spelling's `Date` arm, on this input, for contrast. + expect(() => invalid.toISOString()).toThrow(RangeError); + + const p = new ObjectStackProtocolImplementation(engineWithCommits([commitRow(invalid)])); + + const commits = await p.listCommits({ packageId: 'pkg_crm' }); + + // Unchanged — and specifically NOT converted, which would mean + // this card had quietly chosen a rendering for the contested shape. + expect(commits[0]!.createdAt).toBe(invalid as unknown as string); + }); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 50125f1800..d3716a0267 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -1681,6 +1681,54 @@ function compareAuditInstants(a: unknown, b: unknown): number { return aToken < bToken ? -1 : aToken > bToken ? 1 : 0; } +/** + * Canonicalise the ONE driver materialisation {@link listCommits} was + * measured to produce for `sys_metadata_commit.created_at` — a valid JS + * `Date` — into the ISO-8601 string the return type declares (`createdAt?: + * string`). Every other shape, INCLUDING an Invalid `Date`, is returned + * UNTOUCHED. + * + * [#14038] `created_at` is an engine-injected audit column: it is not in + * `datetimeFields`, and `SqlDriver#formatOutput` repairs it only inside its + * `if (this.isSqlite)` arm (pinned live in driver-sql's + * `sql-driver-13567-audit-stamp-materialisation.test.ts`), so Postgres and + * MySQL hand it out of the record read door as a JS `Date` while the + * mapping below assigned it straight through as `r.created_at` — an + * unchecked value from an `any[]` row, never a measurement against the + * declared `string` return type. + * + * ⚠️ Deliberately NOT the `canonicalIsoInstant` spelling next door in + * `sys-metadata-repository.ts` / `database-loader.ts` (#14037's sibling + * sites): that spelling reaches `value.toISOString()` for ANY `Date`, which + * raises `RangeError: Invalid time value` on an Invalid `Date` — measured + * reachable on BOTH live dialects (a MySQL zero datetime; any Postgres year + * in 275760..294276) and the open subject of #14078, which #13973 is + * blocked on. Whether the shared spelling should throw there (option A) or + * fall back to a rendering (option B) is a maintainer call across four + * packages, so this repair imports NEITHER answer into a new call site: an + * Invalid `Date` is returned unchanged, exactly as the raw assignment + * passed it through today. When #14078 rules, this helper collapses into + * the shared spelling. + * + * ⛔ NOT a tolerant fallback (#13973's standing prohibition): it teaches no + * consumer to accept an off-spec shape; it converts the one measured + * producer materialisation at the producer. The + * `!Number.isNaN(value.getTime())` guard is the same one #14037 used for + * its two sibling sites, not a new spelling. + * + * ⛔ Not exported and not merged into {@link canonicalVersionInstant} above: + * that helper answers a different question (does this token denote AN + * instant at all, returning `null` when it does not) and callers of + * `listCommits` are promised the RAW value back untouched when it is not a + * valid `Date` — an absent/opaque column must still reach `sort`'s fallback + * branch and any in-process reader exactly as before. Consolidating the + * family's near-identical copies is #14078's call, not this card's. + */ +function isoFromValidDate(value: unknown): unknown { + if (value instanceof Date && !Number.isNaN(value.getTime())) return value.toISOString(); + return value; +} + // Lifecycle columns the engine always owns; the clone path drops them by NAME // so the insert re-stamps fresh values instead of copying the source's. Mirrors // record-validator's SKIP_FIELDS (system-injected, never author-supplied). @@ -19069,7 +19117,12 @@ export class ObjectStackProtocolImplementation implements ...(r.parent_commit_id ? { parentCommitId: r.parent_commit_id } : {}), itemCount: typeof r.item_count === 'number' ? r.item_count : 0, items: this.parseCommitItems(r.items), - ...(r.created_at ? { createdAt: r.created_at } : {}), + // [#14038] Canonicalise the ONE driver materialisation this + // column is measured to produce (a valid JS `Date`, on + // Postgres/MySQL); every other shape — including an + // Invalid `Date` — passes through unchanged. See {@link + // isoFromValidDate}. + ...(r.created_at ? { createdAt: isoFromValidDate(r.created_at) as string } : {}), })); // Newest-first; tolerate drivers that don't order by returning // insertion order, then sort by the audit instant. diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 7bdbcf0dc2..ae302177dd 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -111,6 +111,11 @@ "verb": "update", "pinned": 3 }, + { + "file": "packages/metadata-protocol/src/protocol-14038-list-commits-created-at-iso.test.ts", + "verb": "findOne", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/protocol-publish-drafts-advisories.test.ts", "verb": "delete",