fix(data): 审计锚点归引擎所有，lookup 必须能解析 (#4447, #4441) - #4511
Merged
os-zhuang merged 8 commits intoAug 1, 2026
Conversation
…te (#4441) A `lookup` accepted an id that exists in no row of the object it declares, on both platform and application objects: POST /data/sys_position_permission_set {"position_id":"…","permission_set_id":"ps_does_not_exist_at_all"} → 200 POST /data/showcase_task {"title":"…","project":"proj_does_not_exist","status":"backlog"} → 200 The field metadata is unambiguous (`type: lookup`, `required: true`, `reference: 'sys_permission_set'`) and `deleteBehavior: 'set_null'` shows the platform already reasons about this edge on the DELETE side. Only the insert side never checked. On the RBAC link tables that is a security-surface record that resolves to nothing: an administrator auditing permissions sees a binding whose target cannot be inspected, and the audience-anchor gate must resolve that very set to evaluate the grant — so the row is an unevaluable gate input, not just untidy. Enforced in the engine, so every write path inherits it (REST, flows, actions), with the refusal carried in the `fields[]` envelope a `required` violation already uses: `{ field, code: 'reference_not_found', value, message }` → `400 VALIDATION_FAILED`. `reference_not_found` was already a catalogued `FieldErrorCode` with no emitter; it has one now, plus its message in the four platform locales. Scope kept deliberately narrow: - **Caller-supplied keys only.** `owner_id` / `organization_id` / `created_by` / `updated_by` are lookups too, written by hooks and middleware; re-validating them would turn a platform stamp into a caller-facing rejection. - **Non-system writes only**, like every other write-path guard here (`stripReadonlyFields`, `stripReadonlyForInsert`). Seed replay, package install and boot provisioning legitimately write in an order that resolves only once the batch completes; failing them closed turns an ordering detail into a boot failure. The residual — an `isSystem` caller can still write a dangling reference — is recorded rather than silently accepted. - **Empty is not a reference.** `null` / `undefined` / `''` mean "no link" — exactly what `deleteBehavior: 'set_null'` produces. - **Fails OPEN when the target cannot be checked** (unregistered object, no driver, probe throws). An integrity check that cannot run must not invent a rejection; the alternative converts a connectivity problem into data loss. - **The probe is unscoped.** Existence is a fact about the database, not about the caller's visibility — the same distinction #4435's probe turns on. A scoped probe would refuse a link to a permission set the caller cannot READ, which is ordinary under RLS and would break the platform's own admin flows. Whether the caller may create the binding is the RBAC/RLS layer's decision. Wired at BOTH update call sites, single-id and bulk — PD #10's own worked example (#3106) is a guard that reached only the single-id path. No new authorable spec key and no protocol change: the issue's "optional dangling references should be opt-in" would need one, so it is NOT implemented here and is left as a decision for the maintainers. Tests: `engine-lookup-referential-integrity.test.ts` (10) — the RBAC link table and an ordinary object, insert / update / bulk update, multi-value elements, clearing a lookup, the system-context exemption, server-stamped lookups, and the fail-open target. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
…at cannot loosen it (#4447) `created_at` accepted a client-supplied value on an ordinary REST write and PERSISTED it: PATCH /api/v1/data/showcase_task/<id> {"progress":42,"created_at":"1999-01-01T00:00:00Z"} → 200, and the row reads back 1999-01-01 from then on Any authenticated caller who could edit a record could forge the anchor of the audit timeline — the field historical import deliberately preserves, dashboards bucket on, and an auditor reads to establish when a record came into existence — silently, in the same request as a legitimate edit, with no diagnostic. Its two siblings only LOOKED protected. The audit hook force-advances `updated_at` / `updated_by` on every update, so a forged value there is overwritten rather than refused. `created_at` is insert-only, so nothing overwrote it: one field out of the trio was genuinely unguarded, which is why this read as "one field, not a posture". ## Root cause Not a missing definition — `AUDIT_FIELD_DEFS.created_at` has carried `readonly: true, system: true` all along. The hole is that it never applied: `applySystemFields` injects an audit field only when the object does not already have one, and the merge (`{...additions, ...schema.fields}`) lets a declared field WIN. Correct for an authored business field; wrong for a family whose whole point is that the engine owns it. `showcase_task` declares no `created_at` in source, yet the built artifact ships one: "created_at": {"label":"Created At","type":"datetime","readonly":false, …} — a materialized field carrying only FieldSchema DEFAULTS. It shadowed the engine-owned definition, so `stripReadonlyFields` had nothing to key off and the forged value went straight through. No `droppedFields` either, because from the platform's point of view nothing was dropped — which is also why the #3794 contract had no live producer on this axis to test against. ## Fix The audit family's GOVERNANCE is not authorable. A declared audit field now keeps everything presentational (label, description, hidden, group, ordering …) and has `type` / `readonly` / `system` / `reference` forced to the platform's values, derived from `AUDIT_FIELD_DEFS` rather than restated so the two cannot drift. Defensive by design: it closes the class for any producer of a bogus audit field — artifact, stored metadata, AI-authored object, hand-written YAML — not just the one that surfaced it. Deliberate back-dating is untouched: `preserveAudit` (#3479/#3493) and `isSystem` writes still reinstate the original timeline, because both are checked downstream of `readonly`, not by it. Forcing `readonly`/`system` also only ever RELAXES validation — `validateRecord` skips system/readonly fields — so no previously-accepted write starts failing. ## Verified on a real boot `pnpm dev -- --fresh -p 38106`, showcase, ordinary admin session, the issue's own curl: before {"created_at":"2026-06-22T00:00:00.000Z","progress":55} PATCH {"progress":42,"created_at":"1999-01-01T00:00:00Z"} → 200 droppedFields: [{"fields":["created_at"],"reason":"readonly"}] after {"created_at":"2026-06-22T00:00:00.000Z","progress":42} The legitimate half of the request still lands, the anchor does not move, and the drop is now REPORTED — the `droppedFields` contract's first live producer here. Tests: `engine-audit-anchor-write.test.ts` (10) — the update strip, the `droppedFields` report, the whole trio behaving alike, the bulk call site, the `preserveAudit` and `isSystem` escape hatches, and the shadowing root cause reproduced with the artifact's field verbatim. Verified failing on the three root-cause cases before the fix. Known follow-up, NOT fixed here: `/api/v1/meta/objects/showcase_task` still reports `readonly: false` for `created_at` — that surface reads the artifact rather than the registry, so it now disagrees with the enforcement. Filed separately; a machine-readable surface must not lie (AGENTS.md, Route & surface ownership #4). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
…the audit override (#4441, #4447) Two corrections found by running the full objectql suite (1554 tests). ## The reference check validated platform-derived ids (#4441) Filtering by supplied KEY was wrong. A form serializes an unpicked control as an explicit `null`, and `applyFieldDefaults` then fills it from `defaultValue` — including the `current_user` token (#2706). The key IS in the payload, but the id that lands is the platform's, so the check reported a server-derived value as the caller's bad reference and rejected an ordinary insert whenever the acting principal had no row in the target object. The value is still read from the post-normalization payload (multi-value strings are already split by then), but WHETHER to check is now decided by the caller's own RAW value being non-empty. `engine.test.ts`'s #2706 case was the one that caught it. ## The audit override forced more than the defect required (#4447) Forcing `type`/`reference` broke `registry.test.ts`'s "does NOT overwrite author-declared audit fields", and that test is right: an external/federated object legitimately maps its audit column to a differently-typed remote column, and #4447 is about WRITABILITY, not storage shape. The override now carries only `readonly` and `system` — the keys that decide who may write it — so the author keeps `type`, `label`, `description`, `hidden`, `group` and the rest. The pre-existing test passes unchanged. Suite green: objectql 95 files / 1554 tests. New case pinned in `engine-lookup-referential-integrity.test.ts` (11) for the derived-value direction, both for an explicit `null` and an omitted key, plus the proof that a value the caller DID name is still checked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 111 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Checked the three hand-written pages the docs-drift report flagged as actually describing behaviour these fixes change (the other ~107 are indirect `@objectstack/spec` references). **`api/error-catalog.mdx` — was wrong.** It documented `INVALID_REFERENCE` as what a `lookup`/`master_detail` pointing at a missing record answers with. That code has ZERO producers in source, and now that dangling references are actually refused, the real answer is `400 VALIDATION_FAILED` with `fields[].code === 'reference_not_found'`. The entry now says so, tells callers which code to branch on, and a callout carries the real envelope plus the three deliberate non-rejections (empty value, `isSystem` write, uncheckable target). `INVALID_REFERENCE` itself is left in place — it is a `StandardErrorCode` member, and removing it is a spec change, not a docs fix. **`data-modeling/fields.mdx` — was silent.** The lookup section documented `reference` and `deleteBehavior` without saying whether the target has to exist; it does now, so authors need it. Added a short "Referential integrity" note with the rejection shape, the bulk-update coverage, and the fact that clearing a relationship is not a dangling reference — plus the distinction from `deleteBehavior`, which governs the other half of the same contract. **`protocol/objectql/schema.mdx` — accurate as-is, unchanged.** It documents the `reference` PROPERTY in a property table, not write-time semantics, so it makes no claim this change falsifies. Duplicating the note there would be two places to keep in sync. Also checked `droppedFields`, since #4447 gives that contract its first live producer on the audit axis: the only non-generated mention is in `protocol/kernel/http-protocol.mdx`, about the CORS-exposed `x-objectstack-dropped-fields` header, and it is correct. `releases/v17.mdx` mentions it too but is deliberately untouched — release notes are written centrally at release time (CLAUDE.md), never as a rider on a code PR. `content/docs/references/` is auto-generated and was not touched. `pnpm check:doc-authoring` green (215 files). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
…wer for (#4441) The dogfood gate rejected ordinary metadata authoring — package create, publish and clone all failed with: ValidationError: Recorded By: no sys_user record has id "system" `sys_metadata_history.recorded_by` is `Field.lookup('sys_user', { readonly: true })`, and the metadata repository fills it with `actor ?? 'system'` — a SENTINEL STRING, not a user id — on a write that does not carry `isSystem`. So the PR body's claim that "non-system writes are caller writes" was simply not true of the running platform, and only the real gate could show that: the unit suite's fakes never write a sentinel into a lookup. The narrowing follows from the check's OWN stated scope rather than being an exemption bolted on to go green. `stripReadonlyFields` removes a non-system caller's value from a readonly field before the write, and the create ingress does the same (`stripReadonlyForInsert`, #3043). So a value still sitting in a readonly field at this point was written by the PLATFORM by construction — "the reference the caller named" was never going to include it. This does not weaken the fix. The two fields #4441 names — `sys_position_permission_set.permission_set_id` and `showcase_task.project` — are ordinary author-facing lookups with no `readonly`, and both stay enforced; a new case asserts exactly that, so a future widening of this skip cannot quietly swallow them. The sentinel-in-a-lookup is a genuine modelling wart — a lookup column holding a non-id — and is the same class #4441 is about, written by the platform rather than a caller. It is filed separately; rejecting the platform's own write is not the way to report it, and changing what `recorded_by` stores is not this change's call. objectql: 23 tests across the two new suites green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
…synthetic ids (#4441) `field-zoo-roundtrip` seeded its three relational fields with ids that exist in no row: { field: 'f_lookup', write: 'acc_synthetic_0001' } { field: 'f_master_detail', write: 'proj_synthetic_0001' } { field: 'f_tree', write: 'cat_synthetic_0001' } under a comment reading "FK enforcement is off in this harness". That comment described a HOLE — precisely the one #4441 closes — so the fixture depended on the defect the platform now prevents, and the create started failing with `reference_not_found` for all three fields at once. This is the fixture being wrong, not the check being strict, so the data is what changes. The suite now creates a real row in each target object and substitutes its id. What the file actually proves is unchanged — an id string must round-trip as the same id string, the #2004 type-fidelity guard — and it now proves it over references that resolve, which is strictly stronger. The matrix keeps a `REFERENCE_PLACEHOLDER` symbol rather than a magic string, so a future edit cannot typo its way back into writing a literal id, and the placeholder can never reach the wire: substitution is keyed on identity. `REFERENCE_TARGETS` is ORDERED with a body factory because the targets reference each other — `showcase_project` declares a REQUIRED lookup to `showcase_account`, so the account must exist first and the project must be given its real id. Seeding them in the wrong order now fails loudly instead of writing a project that points at nothing, which is a small proof of #4441 in its own right. `status: 'planned'` is the state machine's declared initial state; `active` is refused with `invalid_initial_state`. Verified: `field-zoo-roundtrip.dogfood.test.ts` 46/46 green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
… just one (#4441) Fixes the shard-1 regression 7bcd40d introduced. `field-zoo.matrix.ts` has TWO consumers, and I only checked one: - `field-zoo-roundtrip.dogfood.test.ts` — drives the vectors over real HTTP and (as of 7bcd40d) substitutes a seeded reference id; - `field-zoo-value-shape.test.ts` — the ADR-0104 contract⇔oracle interlock, which parses every `write` vector against `valueSchemaFor(type, 'stored')` WITHOUT booting a stack, and therefore never substitutes. The `Symbol` placeholder satisfied the first and broke the second with `expected string, received symbol`. Because the two live in different FILES and dogfood shards by file, that surfaced as "shard 2 fixed, shard 1 regressed" — one root cause wearing two masks, not two problems. The placeholder is now a STRING, which is what a reference's stored form actually is, so the contract test parses it like any other id vector. And substitution is keyed on the field appearing in `REFERENCE_TARGETS` — the authoritative list — rather than on comparing against the placeholder value, so the placeholder is documentation rather than a control signal and cannot leak to the wire even if someone edits it. Verified together this time, in one tree: - both matrix consumers in one run: 91/91 - dogfood shard 1/2: 38 files passed - dogfood shard 2/2: 37 passed, 1 skipped Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gEHJN2NFpS9VMeURvakgD
os-zhuang
marked this pull request as ready for review
August 1, 2026 16:07
os-zhuang
deleted the
claude/v17-verification-defects-gnf9e6-audit-anchor-v2
branch
August 1, 2026 16:17
This was referenced Aug 1, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v17 验证清单（#3909）里两个写入路径的契约漏洞。与 PR #4496 同属一张卡，因为 #4496 已进入合并队列无法再推送，这两条另起分支。
Fixes #4447
Fixes #4441
#4447 ——
created_at可被客户端在普通 PATCH 里写入并持久化任何能编辑该记录的已认证调用方都能伪造审计时间线的锚点——历史导入特意保留的字段、仪表盘分桶依据、审计人员用来确定记录何时存在的字段——而且是在一次合法编辑的同一个请求里静默完成，没有任何诊断信息。
为什么只有这一个字段中招
它的两个兄弟只是看起来被保护了：审计 hook 在每次 update 都强制推进
updated_at/updated_by，伪造值被覆盖而非被拒绝。created_at只在 insert 时写入，所以没有任何东西覆盖它。三件套里真正无防护的就这一个。根因（不是定义缺失）
AUDIT_FIELD_DEFS.created_at一直带着readonly: true, system: true。漏洞在于它从未生效：applySystemFields只在对象尚未拥有该字段时注入，而合并顺序{...additions, ...schema.fields}让「已声明的字段」获胜。对作者自定义的业务字段这是对的，对一个存在意义就是「引擎独占」的字段族则是错的。showcase_task源码里并没有声明created_at，但构建产物里有——一个只带 FieldSchema 默认值（readonly: false）的物化字段。它遮蔽了引擎自己的定义，于是stripReadonlyFields无从判断，伪造值直接落库。也因此没有droppedFields：从平台视角看确实什么都没被丢弃，这同时解释了为什么 #3794 的契约在这条轴上一直没有活的生产者可供测试。修法
审计字段族的治理权不可作者化：已声明的审计字段保留全部表现层设置（label、description、hidden、group、排序……），仅
readonly/system由平台强制。范围刻意收窄到只强制这两个键。第一版还强制了
type/reference，被registry.test.ts既有用例挡下来了，而那个用例是对的——外部/联邦对象完全可能把审计列映射到类型不同的远端列，而 #4447 讲的是可写性，不是存储形状。回填路径不受影响：
preserveAudit（#3479/#3493）和isSystem写入仍可重建原始时间线，因为二者是在readonly之后判定的。强制readonly/system也只会放宽校验（validateRecord跳过 system/readonly 字段），不存在原本能通过的写入开始失败。#4441 ——
lookup接受在被引用对象里根本不存在的 id字段元数据毫不含糊（
type: lookup、required: true、reference: 'sys_permission_set'），deleteBehavior: 'set_null'说明平台在 delete 一侧已经考虑过这条边界，只有 insert 一侧从未检查。在 RBAC 关联表上，一条悬空行是指向虚无的安全面记录：管理员审计权限时看到的绑定，其目标无法被查看；而受众锚点闸门恰恰要解析那个权限集才能评估授权——所以它是一个无法求值的闸门输入。
修法
在引擎层强制，所有写入路径（REST / flow / action）一并继承，拒绝信封复用
required违规已有的fields[]形状。reference_not_found本就是FieldErrorCode目录成员，此前没有任何生产者；现在有了，并补齐四个平台语种消息。两个 update 调用点都接上了，单条与批量。豁免边界（逐条经真实写入路径反证，非复述）
readonly字段跳过sys_metadata_history.recorded_by是lookup('sys_user', {readonly:true})，平台以actor ?? 'system'填入哨兵字符串，且该写入不带isSystem。「非 system 即调用方」这一前提被真实平台证伪。收窄依据来自检查自身的定义域：stripReadonlyFields/stripReadonlyForInsert保证非 system 调用方的值不可能存活在 readonly 字段里。grep -c reference_not_found→0，种子/包安装未受影响。null，applyFieldDefaults随后用defaultValue（含current_user，#2706）回填——key 在 payload 里但 id 是平台的。由engine.test.ts的 #2706 用例抓出。null/''/[]均跳过并有用例。对 dogfood 夹具的改动，以及为什么那些数据本来就是错的
field-zoo矩阵原先往lookup/master_detail/tree里写acc_synthetic_0001等在任何行里都不存在的 id，注释写着：这句注释记录的正是 #4441 要关掉的那个洞——夹具依赖着平台现在要禁止的缺陷。该文件真正要证的是 #2004 的类型保真：id 字符串必须原样往返；用真实 id 同样能证，而且更强。两个独立佐证说明是数据错而非检查严：
showcase_project自身声明了对showcase_account的必填 lookup（所以播种必须有序），且status: 'active'被invalid_initial_state拒绝——都是合成 id 一直在绕开的既有约束。夹具现在为每个目标对象创建真实行并使用其 id。占位符是字符串而非 Symbol：矩阵有两个消费者，
field-zoo-value-shape.test.ts（ADR-0104 契约⇔预言机互锁）不启动栈、也从不替换，直接把每个write向量喂给valueSchemaFor(type,'stored')。Symbol 占位符满足了 HTTP 那个、却打断了契约那个（expected string, received symbol）——而两者在不同文件、dogfood 按文件分片，于是表现为「分片 2 修好、分片 1 回退」。这是一个根因的两副面孔，已对称修复并在同一棵树里同时验证两个消费者与两个分片。真实启动验证（针对 HEAD
0250afd9的前一提交7bcd40dd重新采集）服务器确实监听：健康检查
HTTP 200，启动日志含➜ API: http://localhost:38107/。#4447（
showcase_task/4COiRR30W9Hl6M79）：合法的那一半照常落库，锚点纹丝不动，丢弃被上报了。
#4441：
用完已
kill，端口已确认释放。边界：没有做、需要拍板的事
isSystem调用方仍可写入悬空引用（种子例外的代价）。已记录，未静默接受。顺带发现（PD #10）
created_atreadsreadonly: falsewhile the write path enforces read-only #4513 ——/api/v1/meta/objects/showcase_task仍报created_at: readonly false：该面读构建产物而非注册表，因此在本 PR 之后它与实际强制行为公开矛盾。机器可读面不许撒谎（AGENTS.md，Route & surface ownership Add Changesets and GitHub Actions automation #4）。已开 unassigned issue。sys_metadata_history.recorded_by把哨兵字符串'system'存进lookup('sys_user')—— 与 data: a lookup accepts an id that does not exist in the referenced object — including the RBAC permission-set link tables #4441 同类（lookup 列存了非 id），只是由平台而非调用方写入。不在本 PR 范围，拒绝平台自身的写入不是报告它的正确方式。门禁
pnpm typecheckpnpm --filter @objectstack/objectql testpnpm --filter @objectstack/spec testpnpm --filter @objectstack/spec check:generatedpnpm check:doc-authoringengine-audit-anchor-write.test.tsengine-lookup-referential-integrity.test.ts-p 38107，HEAD 代码）.changeset/audit-anchor-and-lookup-integrity.md文档：
api/error-catalog.mdx（该错误码此前无生产者，且原文把悬空引用记成INVALID_REFERENCE——实际是VALIDATION_FAILED+fields[].reference_not_found）与data-modeling/fields.mdx（补引用完整性一节）已更正；protocol/objectql/schema.mdx核对后本就准确，未改；content/docs/references/为自动生成，未手改。