Skip to content

fix(objectql): stamp created_at from the system clock on an ordinary create, so a caller cannot forge the audit anchor through a plain POST - #16313

Merged
zhuangjianguo merged 5 commits into
mainfrom
claude/issue-15964-created-at-unconditional-stamp
Sep 6, 2026
Merged

fix(objectql): stamp created_at from the system clock on an ordinary create, so a caller cannot forge the audit anchor through a plain POST#16313
zhuangjianguo merged 5 commits into
mainfrom
claude/issue-15964-created-at-unconditional-stamp

Conversation

@claude

@claude claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #15964

Maintainer ruling of 2026-09-06 (director seat, decision batch #54), verbatim 「同意」 — recommendation A adopted: the audit binder's beforeInsert stamp for created_at takes the same shape as updated_at, so the system clock wins on an ordinary create and a caller-supplied value is stripped-by-overwrite. This PR is that one-line shape change, its pin, and the enumeration the ruling asked for.

The change

packages/objectql/src/plugin.ts, the audit binder's beforeInsert stamp:

-        record.created_at = record.created_at ?? now;
+        record.created_at = preserveAudit ? (record.created_at ?? now) : now;

which is now byte-symmetric with the line under it, record.updated_at = preserveAudit ? (record.updated_at ?? now) : now;.

Why the preserveAudit ternary and not a bare = now. The ruling asks for two things at once — the ordinary create closed, and the historical-import channel kept — and only this shape gives both. treatAsHistorical reaches created_at through this very hook: it sets preserveAudit: true on the write context (packages/rest/src/import-runner.ts:448) and has no separate path to the audit stamp. A bare = now would have closed the hole and broken the ruled channel in the same line.

Reproduced FIRST on origin/main, then again after — same test file, same head, one line apart

Both tables below are printed by the same pin test (packages/objectql/src/plugin-audit-created-at-create-side.test.ts) run at head fe4d6cec0; the only difference is whether plugin.ts is at origin/main (7beaaa32c) or at this branch. (The branch has since merged origin/main at 3e270d4e2; the final head is 2bb64a7bf and everything under Verification was re-run there.) The rig is the real ingress — an ObjectKernel with ObjectQLPlugin, so the shipped sys_stamp_audit_insert hook is bound through bindHooksToEngine and engine.insert runs the static-readonly strip after it — over a capturing driver, so what reaches driver.create is the stored row.

BEFORE (plugin.ts at origin/main; the pin fails on exactly one assertion):

  id         sent=conv_REST_FORGED         stored=undefined                  -> stripped
  run_at     sent=1999-01-01T00:00:00.000Z stored=undefined                  -> stripped
  updated_at sent=1999-01-01T00:00:00.000Z stored=2026-09-06T13:42:02.408Z   -> overwritten
  created_at sent=1999-01-01T00:00:00.000Z stored=1999-01-01T00:00:00.000Z   -> KEPT (the hole)

AFTER (this branch):

  id         sent=conv_REST_FORGED         stored=undefined                  -> stripped
  run_at     sent=1999-01-01T00:00:00.000Z stored=undefined                  -> stripped
  updated_at sent=1999-01-01T00:00:00.000Z stored=2026-09-06T13:40:18.796Z   -> overwritten
  created_at sent=1999-01-01T00:00:00.000Z stored=2026-09-06T13:40:18.796Z   -> overwritten

The three stripped rows are the card's own in-experiment controls and they are what makes this a reading rather than an anecdote: they prove the create-side strip IS running on this path and DOES take other author-declared readonly datetimes, so created_at surviving was "the strip ran and spared exactly this one". All three still strip after the change — a fix that closed created_at and opened one of them would be a regression on a security card.

The ruled historical-import control, same file, both before and after:

[preserveAudit control]  updated_at  KEPT (1999-01-01T00:00:00.000Z)
                         created_at  KEPT (1999-01-01T00:00:00.000Z)
                         run_at      stripped   <- the create-side strip still ignores preserveAudit

run_at in that row is the second control: the preservation is the audit binder's, not the strip's. The 2026-08-08 ruling that made preserveAudit UPDATE-only for stripReadonlyFields is untouched here — an ordinary readonly business column is still taken on the create side even under the flag.

Ablation hygiene. The before-leg was taken with plugin.ts byte-identical to origin/main (git hash-object == git rev-parse BASE:packages/objectql/src/plugin.ts, blob b261d3cf7), the mutation was proved on disk by whole-line anchored counts in both directions (new-line 0 / old-line 1) before anything was measured, and the restore was proved after (blob back to e3b3bdda7 == the HEAD blob, git diff HEAD empty). No rebuild is involved on either leg: the test imports ./plugin.js relatively, so vitest resolves the mutated file from source, never through the package's exports and dist/.

Other creators that relied on the create-side ?? — the enumeration the ruling asked for

Any other creator that relied on the create-side ?? is enumerated in the PR body (it is a finding if one exists, not a reason to keep ??).

One exists, and it is filed as #16312: packages/metadata/src/migrations/migrate-sys-notification-to-event.ts:185,197 inserts the materialized inbox row and its receipt with the LEGACY notification's created_at, through a real IDataEngine, passing no options bag — so no preserveAudit and no isSystem. After this change those rows carry the migration instant instead of the original one. Its own suite stays green (23 passed) because it drives the migration through a fake engine double, where no before-phase hook runs — so nothing in CI would have said this out loud. Per the ruling that is a finding, not a reason to keep the ??; the remedy there is one context key, { context: { preserveAudit: true } }, the same channel treatAsHistorical uses.

Method, and the rest of the sweep: every non-generated source under packages/, apps/ and examples/ carrying created_at as an object-literal key was classified by whether the value is the current instant (nothing to preserve) or an external/back-dated one (a real reliance).

site value it supplies verdict
metadata/src/migrations/migrate-sys-notification-to-event.ts:185,197 the legacy row's created_at RELIANT — filed as #16312
metadata/src/loaders/database-loader.ts:1357 its own now same instant, nothing preserved
rest/src/rest-server.ts:8826 new Date().toISOString() same instant
services/service-messaging/src/sql-outbox.ts:117, sql-http-outbox.ts:189 its own now same instant
services/service-messaging/src/inbox-channel.ts:120,207, messaging-service.ts:783,1017 the delivery / read instant of the same call same instant
services/service-automation/src/flow-dispatch-store.ts:71, suspended-run-store.ts:333,691 its own now same instant
objectql/src/engine.ts:6741 new Date().toISOString() — and it calls secretDriver.create directly never reaches this hook
runtime/src/domains/share-links.ts:238 a response body field not an insert

A second consumer exists and CI found it, not my sweep — and it is the interesting one. packages/qa/dogfood/test/analytics-timezone.dogfood.test.ts seeds three crm_lead rows at a deliberate timezone-boundary instant (2024-03-01T03:00:00.000Z, which is still 2024-02-29 in America/Los_Angeles) to drive analytics date bucketing, and its own loud sanity check then reads them back. At fe4d6cec0 the Dogfood Regression Gate (3/3) went red on exactly that check: expected '2026-09-06T14:00:40.005Z' to be '2024-03-01T03:00:00.000Z', 1 failed of 44 files. Reproduced locally before anything was written, as the firing control that my rig really does see what CI saw: reverting ONLY the seeding context (assertion untouched, proved by anchored counts in both directions) gives expected '2026-09-06T14:21:25.288Z' to be '2024-03-01T03:00:00.000Z' — the same assertion, the same shape. Re-applying the one-key adaptation turns it green, 2 passed.

That is the first measured evidence that the old ?? was load-bearing for a LEGITIMATE use — seeding historical analytics rows — rather than only for a forgery. It strengthens the card rather than weakening it: the create-side channel really was doing work, it was just doing it for everyone, unconditionally and with no flag, which is precisely why an unauthenticated-looking POST could reach it too.

The decisive question was measured before anything was written: does the ruled explicit channel reach this seeding path? It does, both by source and by experiment. ObjectQL.buildSession (packages/objectql/src/engine.ts:3722) propagates preserveAudit from the ExecutionContext on its own branch, unrelated to isSystem, so { context: { isSystem: true, preserveAudit: true } } reaches this hook the same way runImport({ treatAsHistorical: true }) does. So the fixture moves to the explicit channel and its assertion is untouched — still toBe(BOUNDARY), still exact, still loud.

⚠️ And the same measurement corrects something the fixture's own comment implied: isSystem never was the reason its created_at survived. isSystem exempts the engine's readonly STRIP; the audit binder's stamp is not gated on it at all. The seed was riding the ??, not its own elevation. Both halves are now pinned at unit level in plugin-audit-created-at-create-side.test.ts — under isSystem alone, id and run_at DO survive (the control proving the elevation really took effect) while created_at and updated_at are stamped; add preserveAudit and the back-dated created_at lands.

How my sweep missed it, stated rather than smoothed over. The table above covers non-test sources only, and the second pass I ran over test files keyed on ObjectQLPlugin|defineStack|createStack|startStack|bootKernel — which omits bootStack, the dogfood harness's boot verb. The fixture was outside my population twice. Re-run with bootStack included and over test files, the sweep now finds three more candidates and clears all three by measurement: storage-growth.dogfood.test.ts:152,158,231 back-dates through driver.create directly, so no engine hook runs; attachments-permission-matrix.dogfood.test.ts:452 is likewise a direct driver insert and :479 back-dates through ql.update, which this change does not touch; oidc-authorization-code-flow and oidc-authorize-env-gate seed nowIso. Consistent with CI, where the dogfood shard reports exactly one failure across 44 files.

A third finding came out of the same drive and is filed as #16311: created_by has the identical laundering hole one field over — record.created_by = record.created_by ?? session.userId is ?? while its sibling updated_by is the preserveAudit ternary. Measured on the same rig: an authenticated caller sending created_by: 'forged_user' had it stored while updated_by in the same payload was correctly overwritten with the session user; with no session.userId the hook assigns nothing and the strip then takes the forgery correctly. The triage comment on #15964 raised exactly this and recorded it as untested; it is now measured. It is not changed here: the ruling adopted option A for created_at and named only that field, and created_by's shape has a second question of its own (it is hasField- and session-guarded, unlike created_at). #16311 is not addressed by this PR.

Clause ②: no — re-derived from the diff, not inherited

The ruling's stated expectation was no; declared at claim and then measured rather than carried over. Method: build at head, swap only packages/objectql/src/plugin.ts back to origin/main, rebuild, and compare every declaration file the package publishes — dist/{index,core,util-zFBRz_yg}.d.ts and their .d.mts twins — by both exported-name set and bytes.

file names base -> head bytes
index.d.ts / index.d.mts 252 -> 252, ADDED [] REMOVED [] identical blob
core.d.ts / core.d.mts 79 -> 79, ADDED [] REMOVED [] identical blob
util-zFBRz_yg.d.ts / .d.mts 133 -> 133, ADDED [] REMOVED [] identical blob, and the content-hashed chunk name did not move

The rebuild is proved to have re-run rather than been skipped: dist/index.js mtime advanced 1788702294 -> 1788702314 -> 1788702335 across the three legs, and the declarations' own mtimes advanced with it while their hashes did not.

The instrument is proved live, because an identical manifest is otherwise indistinguishable from a blind spot. A public member injected onto the exported ObjectQLPlugin class moved dist/index.d.ts from blob 5f44a2a8d to 54e2b4ba4 and appeared in the published declaration; restoring returned it to 5f44a2a8d with the member gone. (A first control aimed at a NEW top-level export const in plugin.ts did not fire, and that is a fact about the barrel rather than a dead instrument: packages/objectql/src/index.ts:463 re-exports ObjectQLPlugin by name, so a new top-level export in that module reaches no published entry point. The control was re-aimed inside the published set.)

So: no exported symbol moves, and the accept set only narrows, to the readonly contract the field already documents. A caller may still send created_at; it is now ignored on an ordinary create rather than honoured. patch, per the repo rule that a bug fix in a released package takes a patch changeset.

Verification

Everything here was re-run at the final head 2bb64a7bf, after merging origin/main at 3e270d4e2 (the gate-family deriver had flagged the pre-merge tree as stale: 12 commits behind with 34 of the gate scripts themselves changed in that range, so no local gate reading taken on it would have been about a tree anyone is on). Exit codes were captured by redirect-then-read, never through a pipe.

  • New pin packages/objectql/src/plugin-audit-created-at-create-side.test.ts — 4 cases: the ordinary create with its three controls; a create sending no created_at; isSystem alone versus isSystem + preserveAudit; and the preserveAudit historical control.
  • Targeted objectql set — 12 files, 306 tests, all passing: the new pin plus plugin.integration.test.ts, engine-audit-anchor-write, engine-insert-static-readonly-strip, engine-readonly-strip-caller-values, engine-readonly-strip-signal, engine-strict-readonly-warning-truthful, engine-hook-provenance-sibling-seams, engine-post-hook-undeclared-field, stamped-system-fields-spec-conformance, seed-loader-org-stamp, protocol-data.
  • The dogfood consumer — analytics-timezone.dogfood.test.ts, 2 tests, passing, driven end to end on a real booted CRM stack.
  • @objectstack/rest import path — 3 files, 20 tests, all passing, including import-runner-historical-readonly-insert.test.ts, the pin that drives treatAsHistorical through the real insert ingress. (These two files first read as failures against an unbuilt @objectstack/objectql; that was Failed to resolve entry for package, not an assertion.)
  • pnpm --filter @objectstack/objectql typecheck and pnpm --filter @objectstack/dogfood typecheck — both clean, the first including check:test-typecheck (44 files / 242 errors / 69 pinned signatures held; the new test file is inside the checked zone and owes no ledger entry).
  • The 60 gate families derived for this diff by node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (the deriver takes the change set from the merge base itself, so no hand-built diff can narrow it): 60 run, 60 green, nothing declared unmeasured.
    • One real failure was found along the way and fixed rather than argued with: check:slot-lookup flagged the new test's getService('objectql') as any as a NEW service-lookup erasure. It now takes the slot's contract type, and the ratchet is back at baseline (106 unswept sites in 25 files, none new).
    • check:dual-build-cjs-loads and check:type-check-debt had both been refusing with exit 3 (PREREQUISITE NOT MET) because they read built output for the whole workspace. A full turbo run build --filter='./packages/*' --filter='./packages/*/*' (71/71 tasks, 68 packages emitting dist/) turned both into real measurements: dual-build sweeps 103 entries / 66 packages / 619 CJS files / 1 probe against floors of 90 / 58 / 520 / 1; check:type-check-debt --re-measure re-measures 5 ledger entries in 103.6s, 55 raw tsc errors total, none above its recorded number, surplus none. The re-measure needs more heap than the 4096 MB this seat runs commands under — it OOM'd and correctly refused to record a 0 rather than silently reporting one — so it was re-run at 10240 MB, where the gate pins tsc itself to its own CI-shaped 6144 MB ceiling.
    • check:dts-closure's green is about this package and not someone else's: it reports 15 built package(s) swept without naming them, so the workspace was enumerated — at that point exactly 15 packages had a dist/ and @objectstack/objectql was one of them.

Ruled 2026-09-06 by the maintainer, decision batch #54, option A. Implemented by the domain:engine execution seat in session 01ARYe3yQTQCUFm5qPYNgKaJ (https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ), branch claude/issue-15964-created-at-unconditional-stamp.


Generated by Claude Code

The audit binder's beforeInsert stamp used `record.created_at ?? now`, which
since #15395 launders a caller-supplied value past the engine-side static
readonly strip (#14259 reads a key a hook ASSIGNED as the hook's write). It now
takes the same shape as `updated_at`: the system clock wins unless
`preserveAudit` is set, which is the historical-import channel and stays.

Ruled by the maintainer 2026-09-06, decision batch #54, option A.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…rveAudit control

Reproduces the card's four-field table through the real ingress (kernel +
ObjectQLPlugin + engine.insert), with `id` / `run_at` / `updated_at` as the
in-experiment controls, and pins that `preserveAudit` still reinstates an
original created_at.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…lot-lookup)

The new test file is not grandfathered in the slot-lookup baseline, so the
kernel service lookup takes the slot's contract type instead of `as any`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions github-actions Bot added the size/m label Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 16 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 3e270d4e296368f6600d71fcec9902f3a14c1698packageMentionDocs.

Which tree this was computed on

This run read content/docs from d424f88d7bc67ab3d8ad87422a398db1277992da — the merge of head 2bb64a7bf4d68d90d346933197e7644a67d6c470 into base 3e270d4e296368f6600d71fcec9902f3a14c1698, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d424f88d7bc67ab3d8ad87422a398db1277992da && git checkout d424f88d7bc67ab3d8ad87422a398db1277992da
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3e270d4e296368f6600d71fcec9902f3a14c1698 2bb64a7bf4d68d90d346933197e7644a67d6c470 && git checkout -B drift-repro 3e270d4e296368f6600d71fcec9902f3a14c1698 && git merge --no-ff 2bb64a7bf4d68d90d346933197e7644a67d6c470

node scripts/docs-audit/affected-docs.mjs --json 3e270d4e296368f6600d71fcec9902f3a14c1698

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@claude

claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

CI is red on Dogfood Regression Gate, and it is the case the ruling anticipated — recording the diagnosis and the boundaries on the PR itself

PM (domain:engine dispatching seat), 2026-09-06T14:1xZ. The implementing round has this already; it is written here so the record does not depend on that round surviving.

The failure, at head fe4d6cec (job 101497734856)

FAIL packages/qa/dogfood/test/analytics-timezone.dogfood.test.ts
     > dogfood: org timezone drives analytics date bucketing (#1982/#2018)
AssertionError: expected '2026-09-06T14:00:40.005Z' to be '2024-03-01T03:00:00.000Z'
line 63:  expect(new Date(r.created_at as string).toISOString()).toBe(BOUN…)
Test Files 1 failed | 42 passed | 1 skipped (44)

That test seeds a deliberate timezone-boundary created_at (2024-03-01T03:00:00Z) to drive analytics date bucketing and reads it back. This PR makes the system clock win on an ordinary create, so it reads "now".

This is a real consumer of the create-side ?? — precisely what the maintainer's ruling named: "Any other creator that relied on the create-side ?? is enumerated in the PR body … it is a finding if one exists, not a reason to keep ??."

⭐ Worth stating plainly rather than treating as noise: this is the first evidence that the laundering hole was load-bearing for a legitimate use (seeding historical analytics rows). That strengthens #15964's case — the hole is reachable and used — and it is exactly why the ruling kept an explicit historical channel instead of only closing the door.

⛔ Three things are off the table on this PR

  1. ⛔ Reinstating or special-casing the ??. The ruling forecloses it.
  2. ⛔ Skipping, disabling, quarantining, or loosening that test's assertion to "any date". A failing test is never an infra flake, and this one asserts something real.
  3. ⛔ Editing content/docs/releases/**.

The question that decides the fix

Does treatAsHistorical — the explicit channel the ruling preserved — reach the path this dogfood test seeds through?

  • Yes ⇒ move the test's seeding onto that channel, keep its assertion exactly as strict, and enumerate this consumer in the PR body as the ruling requires. Adapting a consumer to a ruled behaviour change is not widening this PR.
  • No ⇒ ⛔ stop. That would falsify the ruling's own premise ("historical import keeps its explicit channel") for a real consumer, and it goes back to the maintainer — ⛔ not resolved by forcing the test green.

Generated by Claude Code

…eAudit

The analytics timezone fixture back-dates `created_at` to a deliberate DST
boundary and was relying on the create-side `??` that #15964 removes — the
first measured LEGITIMATE consumer of that hole. It now uses the explicit
historical channel the same ruling preserved (`preserveAudit`, what REST's
`treatAsHistorical` sets), and every assertion in the file is byte-unchanged.

`isSystem` alone never preserved it: that flag exempts the engine's readonly
strip, not the audit binder's stamp. Both halves are pinned at unit level in
`plugin-audit-created-at-create-side.test.ts`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 6, 2026 15:28
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 6, 2026
Merged via the queue into main with commit 7079694 Sep 6, 2026
39 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-15964-created-at-unconditional-stamp branch September 6, 2026 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

#15395's engine-side readonly strip runs AFTER the audit binder, so a plain REST caller's created_at survives on an object that declares it readonly

2 participants