diff --git a/.changeset/import-job-dto-canonical-timestamps.md b/.changeset/import-job-dto-canonical-timestamps.md new file mode 100644 index 0000000000..8286df4903 --- /dev/null +++ b/.changeset/import-job-dto-canonical-timestamps.md @@ -0,0 +1,41 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): serve canonical ISO-8601 for the import-job DTO's four timestamps on Postgres/MySQL (#13994) + +`importJobToProgress` — the mapper behind `GET /data/import/jobs/:jobId`, +`/results` and the history list — rendered `created_at`, `started_at`, +`completed_at` and `reverted_at` through `String(value)`. On Postgres and +MySQL, the production default driver materialises those columns as JS `Date`s, +so `String` ran `Date.prototype.toString` and the REST contract served + +``` +Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard Time) <- what the API served +2026-08-30T10:19:25.947Z <- what it promises +``` + +Milliseconds were dropped, the **server's** timezone was baked into the value, +there was no `Z`, and the result is not `Date.parse`-safe for a client doing +strict ISO parsing. `ImportJobProgressSchema` / `ImportJobSummarySchema` +declare all four as `z.string()` documented "(ISO 8601)", and the client SDK +and objectui's `ImportJobProgressInfo` both restate that as `string` — the +declaration was right, the emitted value was wrong. + +Nothing upstream repaired it: `formatOutput`'s two timestamp repairs — the +`AUDIT_TIMESTAMP_COLUMNS` pass and the `normalizeSqliteDatetimeOutput` pass +over `datetimeFields` — both sit inside its `if (this.isSqlite)` arm, so a +declared `Field.datetime` is **not** protected on Postgres/MySQL. SQLite +returns canonical ISO text, where `String()` was an identity — which is why +every SQLite-backed test stayed green for the whole life of the defect. + +The four sites now go through the same three-branch normaliser this repo +already landed in `@objectstack/metadata-protocol` (string passthrough → +`instanceof Date` → `toISOString()` → last-resort `String(v ?? '')`): one +spelling repo-wide, no tolerant `??` fallback, and no change to the presence +semantics — a job that has not started still omits `startedAt` entirely. + +Values that were already canonical (every SQLite deployment) are returned +byte-identical, so this changes nothing for them; on Postgres and MySQL a +client that parsed the old string leniently now receives the same instant +spelled correctly, with the milliseconds it previously lost. diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index a514a33271..1c5d83c513 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -64,7 +64,7 @@ not on any flag. ## How the flag is set `isSystem` is **server-constructed and never client-supplied**. Inbound HTTP -cannot set it (`packages/rest/src/rest-server.ts:1389`, `:1418`), and neither +cannot set it (`packages/rest/src/rest-server.ts:1445`, `:1474`), and neither can an action body (`packages/runtime/src/domains/actions.ts:404`). It is written by internal callers only, as an option on the engine call: @@ -103,7 +103,7 @@ that silently does not happen. | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:246` | | 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` | | 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` | -| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1421` | +| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1477` | ### 2. Write pipeline and data integrity @@ -158,7 +158,7 @@ The largest single consumer — **20 of the 109 sites**. |:--|:---|:---|:---|:---| | 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:136` | | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:399` | -| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4573`, `:5936`, `:6184`, `:6615`, `:6808` | +| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4629`, `:5992`, `:6240`, `:6671`, `:6864` | | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` | | 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:411`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:246`, `external-datasource-routes.ts:302`, `package-routes.ts:97` | | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | @@ -199,7 +199,7 @@ assuming `isSystem` covers it is a documented source of bugs. | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1516` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:286` | -| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1389`, `:1418`; `domains/actions.ts:404` | +| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1445`, `:1474`; `domains/actions.ts:404` | --- diff --git a/packages/rest/src/import-job-dto-timestamp-canonical.test.ts b/packages/rest/src/import-job-dto-timestamp-canonical.test.ts new file mode 100644 index 0000000000..95b99af12a --- /dev/null +++ b/packages/rest/src/import-job-dto-timestamp-canonical.test.ts @@ -0,0 +1,261 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13994] The import-job DTO serves CANONICAL ISO-8601 for its four timestamp + * fields, on every dialect and under every process timezone. + * + * ## The defect + * + * `importJobToProgress` rendered all four stamps through `String(v)`. On + * Postgres and MySQL — the production default driver — those columns arrive as + * JS `Date`s, so `String` ran `Date.prototype.toString` and the REST contract + * served + * + * "Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard Time)" + * + * where `ImportJobProgressSchema` promises `"2026-08-30T10:19:25.947Z"`: + * milliseconds dropped, the SERVER's timezone baked in, no `Z`, and not + * `Date.parse`-safe for a client doing strict ISO parsing. + * + * Why all four, and why nothing upstream repaired them: `formatOutput`'s two + * timestamp repairs — the `AUDIT_TIMESTAMP_COLUMNS` pass (`created_at`) and the + * `normalizeSqliteDatetimeOutput` pass over `datetimeFields` + * (`started_at` / `completed_at` / `reverted_at`, all declared `Field.datetime` + * on `sys_import_job`) — both sit INSIDE `formatOutput`'s `if (this.isSqlite)` + * arm. ⚠️ A declared `Field.datetime` is NOT protected on Postgres/MySQL. + * + * ## Why the obvious pin would have proved nothing + * + * SQLite stores and returns canonical ISO text, so `String()` was an IDENTITY + * there and every SQLite-backed test — including this package's real-engine + * `import-job-integration.test.ts` — stayed green through the whole life of the + * defect. A fixture of ISO strings cannot fail. **So these cases drive real + * `Date`s through the real routes**, which is the shape only a non-SQLite + * driver produces, and they do it under a forced non-UTC process zone. + * + * ## What is pinned — the property, not the spelling + * + * Not "the mapper calls `toISOString()`". The invariants are: + * + * 1. **A `Date` from the read door is served as canonical ISO-Z**, on all + * four fields, through both mappers (progress and summary), with the four + * stamps DISTINCT so no field can pass by echoing another's value. + * 2. **The answer does not depend on `process.env.TZ`** — swept over three + * zones, with a non-vacuity control proving those zones really do move the + * broken spelling (three green rows under three identical spellings would + * prove nothing about timezone independence). + * 3. **An already-canonical string is a fixed point** (the SQLite shape is + * returned byte-identical). This is what shows the pin DISCRIMINATES + * rather than being globally sensitive to any change at the seam. + * 4. **The response satisfies the declared contract**, asserted by a full + * `safeParse` against the spec's own `ImportJobProgressSchema` / + * `ImportJobSummarySchema` — the judgement here is about a VALUE, so a + * green parse is the assertion, not merely the absence of unknown keys. + * This limb is also what refuses the tempting "just delete the `String()` + * and let `JSON.stringify` do it" route: that emits the right text but + * widens the declared `z.string()` to `string | Date`. + * + * ## What is deliberately NOT claimed here + * + * That `driver-sql` hands this seam a `Date` on Postgres. That is a fact about + * `driver-sql`, measured beside the fix (`formatOutput`'s `isSqlite` bracketing) + * and pinned in that package; `@objectstack/rest` must not grow a Postgres + * dependency to restate it. What these tests own is the mapper's behaviour + * GIVEN each input shape a driver can produce. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +// The contract itself, not a local restatement of it: the same schemas +// `ImportJobApiContracts` names as the `output` of these very routes. +import { ImportJobProgressSchema, ImportJobSummarySchema } from '@objectstack/spec/api'; +import { RestServer } from './rest-server'; + +/** Canonical ISO-8601 UTC with milliseconds — what the contract promises. */ +const CANONICAL_ISO = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + +/** + * Four DISTINCT instants, each with a distinct NON-ZERO millisecond component. + * Distinct so no field can pass by echoing another's value; non-zero + * milliseconds so the millisecond-dropping spelling cannot pass by accident. + */ +const CREATED = '2026-08-30T10:19:25.947Z'; +const STARTED = '2026-08-30T10:20:31.001Z'; +const COMPLETED = '2026-08-30T10:21:44.512Z'; +const REVERTED = '2026-08-30T10:22:59.083Z'; + +const ALL_FOUR = { createdAt: CREATED, startedAt: STARTED, completedAt: COMPLETED, revertedAt: REVERTED }; + +/** The card's zone, a zone on the other side of UTC, and UTC itself. */ +const ZONES = ['Asia/Shanghai', 'America/New_York', 'UTC'] as const; + +/** + * One `sys_import_job` row as a driver materialises it. `stamp` decides the + * shape of the four timestamp columns: `Date` (Postgres / MySQL / MongoDB) or + * canonical ISO text (SQLite and friends). + */ +function makeRow(stamp: (iso: string) => unknown) { + return { + id: 'imp_13994', + object_name: 'task', + status: 'succeeded', + dry_run: false, + write_mode: 'insert', + total_rows: 3, + processed_rows: 3, + created_count: 2, + updated_count: 0, + skipped_count: 0, + error_count: 1, + created_at: stamp(CREATED), + started_at: stamp(STARTED), + completed_at: stamp(COMPLETED), + reverted_at: stamp(REVERTED), + }; +} + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { + write: () => true, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +/** + * The REAL routes, over a protocol whose read door returns exactly `row`. + * + * A stub read door rather than a real engine ON PURPOSE: the shape under test + * is the one a SQLite-backed engine cannot produce, and it is precisely the + * unreachability of that shape from SQLite that hid this defect. + */ +function boot(row: unknown) { + const protocol = { findData: async () => ({ records: [row] }) }; + const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const routes = rest.getRoutes(); + const find = (method: string, path: string) => routes.find((r: any) => r.method === method && r.path === path); + return { + progress: find('GET', '/api/v1/data/import/jobs/:jobId'), + results: find('GET', '/api/v1/data/import/jobs/:jobId/results'), + list: find('GET', '/api/v1/data/import/jobs'), + }; +} + +async function call(route: any, req: any = {}) { + const res = makeRes(); + await route.handler({ params: { jobId: 'imp_13994' }, query: {}, ...req } as any, res); + return res._json; +} + +const ORIGINAL_TZ = process.env.TZ; +afterEach(() => { + if (ORIGINAL_TZ === undefined) delete process.env.TZ; + else process.env.TZ = ORIGINAL_TZ; +}); + +describe('[#13994] the import-job DTO serves canonical ISO-8601 for a `Date` from the read door', () => { + it('renders all four stamps canonically under a forced non-UTC process zone', async () => { + process.env.TZ = 'Asia/Shanghai'; + + // NON-VACUITY CONTROL. The defect's spelling, evaluated right here under + // the same zone: if `String(Date)` already produced canonical ISO, the + // assertions below would be green against the broken code too. + const broken = String(new Date(CREATED)); + expect(broken, 'the broken spelling did not move — this pin would be vacuous').not.toBe(CREATED); + expect(broken).not.toMatch(CANONICAL_ISO); + + const body = await call(boot(makeRow((iso) => new Date(iso))).progress); + + // All four, each against ITS OWN instant — distinct values, so a mapper + // that echoed one stamp into all four fields fails here. + expect(body).toMatchObject(ALL_FOUR); + for (const [field, value] of Object.entries(ALL_FOUR)) { + expect(body[field], `${field} is not canonical ISO-Z`).toMatch(CANONICAL_ISO); + // Strict-ISO round-trip: what a client doing `Date.parse` receives. + expect(new Date(body[field]).toISOString()).toBe(value); + } + + // Limb 4: the declared contract, parsed by the spec's own schema. A bare + // `Date` here (the "just delete the String()" route) fails this. + const parsed = ImportJobProgressSchema.safeParse(body); + expect(parsed.success, JSON.stringify((parsed as any).error?.issues)).toBe(true); + }); + + it('gives the same answer under every process timezone, and the zones really do move the broken spelling', async () => { + const served = new Set(); + const brokenSpellings = new Set(); + + for (const zone of ZONES) { + process.env.TZ = zone; + brokenSpellings.add(String(new Date(CREATED))); + const body = await call(boot(makeRow((iso) => new Date(iso))).progress); + served.add(JSON.stringify([body.createdAt, body.startedAt, body.completedAt, body.revertedAt])); + } + + // The control: three zones, three DIFFERENT broken spellings. Without + // this, three green rows would say nothing about timezone independence. + expect( + brokenSpellings.size, + 'the process zone did not move `String(Date)` — the sweep is vacuous', + ).toBe(ZONES.length); + + // The property: one answer, whatever the server's zone. + expect(served).toEqual(new Set([JSON.stringify([CREATED, STARTED, COMPLETED, REVERTED])])); + }); + + it('serves the summary (list) DTO canonically too', async () => { + process.env.TZ = 'Asia/Shanghai'; + const body = await call(boot(makeRow((iso) => new Date(iso))).list); + const [job] = body.jobs; + + // `importJobToSummary` re-reads `importJobToProgress`'s output, so this + // is the second mapper's face on the same repair. + expect(job).toMatchObject({ createdAt: CREATED, completedAt: COMPLETED, revertedAt: REVERTED }); + for (const field of ['createdAt', 'completedAt', 'revertedAt'] as const) { + expect(job[field], `${field} is not canonical ISO-Z`).toMatch(CANONICAL_ISO); + } + const parsed = ImportJobSummarySchema.safeParse(job); + expect(parsed.success, JSON.stringify((parsed as any).error?.issues)).toBe(true); + }); + + it('serves the results DTO canonically too', async () => { + process.env.TZ = 'Asia/Shanghai'; + const body = await call(boot(makeRow((iso) => new Date(iso))).results); + expect(body).toMatchObject(ALL_FOUR); + }); +}); + +describe('[#13994] an already-canonical string is a fixed point — the pin discriminates', () => { + it('returns the SQLite shape byte-identical, under a non-UTC zone', async () => { + process.env.TZ = 'Asia/Shanghai'; + const body = await call(boot(makeRow((iso) => iso)).progress); + + // Idempotence: the dialect that was already correct must not move. A + // repair that re-derived every value (`new Date(v).toISOString()`) would + // pass the `Date` cases above and still be a change in behaviour here. + expect(body).toMatchObject(ALL_FOUR); + expect(ImportJobProgressSchema.safeParse(body).success).toBe(true); + }); + + it('leaves a missing optional stamp absent, and an absent `created_at` an empty string', async () => { + process.env.TZ = 'Asia/Shanghai'; + // The presence-guards are semantics this repair does NOT touch: a job + // that has not started yet omits the three optional stamps entirely. + const row: any = makeRow((iso) => new Date(iso)); + delete row.started_at; delete row.completed_at; delete row.reverted_at; delete row.created_at; + + const body = await call(boot(row).progress); + expect('startedAt' in body).toBe(false); + expect('completedAt' in body).toBe(false); + expect('revertedAt' in body).toBe(false); + expect(body.createdAt).toBe(''); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 902319cdf0..2057e27731 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -515,13 +515,69 @@ function importJobUndoable(row: any): boolean { return !!log && (log.created.length > 0 || log.updated.length > 0); } +/** + * The canonical ISO-8601 spelling of a timestamp column read back through the + * engine's record read door, for a DTO field whose contract declares a string. + * + * [#13994] The input domain is what a DRIVER materialises into such a column, + * and it is dialect-dependent — measured, not guessed: + * + * - **JS `Date`** — `driver-sql` on Postgres and MySQL. `timestamptz` / + * `DATETIME(3)` are instants and the driver materialises them as `Date` on + * purpose (`SqlDriver.withPostgresCalendarDayAsText` says so in as many + * words); `driver-mongodb` stamps `new Date()` and BSON round-trips it. + * `formatOutput`'s two timestamp repairs — the `AUDIT_TIMESTAMP_COLUMNS` + * pass and the `normalizeSqliteDatetimeOutput` pass over `datetimeFields` — + * both sit INSIDE its `if (this.isSqlite)` arm, so neither runs here. ⚠️ A + * declared `Field.datetime` is therefore NOT protected on Postgres/MySQL. + * - **`string`, already canonical ISO-8601 UTC** — `driver-sql` on SQLite and + * its `driver-turso` / `driver-sqlite-wasm` siblings, and `driver-memory`. + * Passed through unchanged, so a canonical row is a fixed point. + * - **anything else** a host stamps into the column — rendered as before. + * + * Why this is not `String(v)`: on a `Date`, `String` runs + * `Date.prototype.toString`, which drops milliseconds and bakes in the PROCESS + * timezone with no `Z` — `"Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard + * Time)"` where the contract promises `"2026-08-30T10:19:25.947Z"`. That value + * is not `Date.parse`-safe for a client doing strict ISO parsing, and it moves + * with the server's zone. SQLite hands back canonical text, so `String()` was + * an identity there and every development environment stayed green — the same + * camouflage that made the OCC seam a production bug (#13382). + * + * Why not simply DELETE the `String()` and let `JSON.stringify` serialise the + * bare `Date` through `toJSON()`: that emits the right text but changes the + * value's static type from `string` to `string | Date`, widening a declared + * contract that three independent declarations spell as `string` — + * `ImportJobProgressSchema` / `ImportJobSummarySchema` + * (`@objectstack/spec`, `z.string()`, "ISO 8601"), the `ImportJobProgress` + * the client SDK returns, and objectui's `ImportJobProgressInfo`. The + * declaration is right; the emitted value was wrong. This makes the value what + * the declaration already says. + * + * Same three branches as the two landed normalisers in + * `@objectstack/metadata-protocol` — `auditMetaItem`'s `occurredAt` in + * `protocol.ts` and `canonicalIsoInstant` in `sys-metadata-repository.ts` + * (#13997) — ONE spelling repo-wide for this repair, deliberately not a new + * variant. The only difference from `canonicalIsoInstant` is its nullish arm, + * and that difference is forced by the call sites: it returns `undefined` so + * each caller's own `?? ` chain keeps its meaning, whereas the four + * sites here are the DTO's last step and the required `createdAt` field's + * absent-value spelling — `''` — is folded in, exactly as the `String(row?. + * created_at ?? '')` it replaces produced. + */ +function canonicalIsoStamp(value: unknown): string { + if (typeof value === 'string') return value; + if (value instanceof Date) return value.toISOString(); + return String(value ?? ''); +} + /** Map a persisted `sys_import_job` row to the ImportJobProgress DTO. */ function importJobToProgress(row: any): Record { const total = Number(row?.total_rows ?? 0); const processed = Number(row?.processed_rows ?? 0); return { undoable: importJobUndoable(row), - ...(row?.reverted_at ? { revertedAt: String(row.reverted_at) } : {}), + ...(row?.reverted_at ? { revertedAt: canonicalIsoStamp(row.reverted_at) } : {}), jobId: String(row?.id ?? ''), object: String(row?.object_name ?? ''), status: String(row?.status ?? 'pending'), @@ -535,9 +591,9 @@ function importJobToProgress(row: any): Record { errors: Number(row?.error_count ?? 0), percentComplete: total > 0 ? Math.min(100, Math.round((processed / total) * 100)) : (processed > 0 ? 100 : 0), ...(row?.error ? { error: String(row.error) } : {}), - ...(row?.started_at ? { startedAt: String(row.started_at) } : {}), - ...(row?.completed_at ? { completedAt: String(row.completed_at) } : {}), - createdAt: String(row?.created_at ?? ''), + ...(row?.started_at ? { startedAt: canonicalIsoStamp(row.started_at) } : {}), + ...(row?.completed_at ? { completedAt: canonicalIsoStamp(row.completed_at) } : {}), + createdAt: canonicalIsoStamp(row?.created_at), }; }