Repository navigation
A stripped master-detail repoint unlocks a parent-scoped readonlyWhen: a by-id update judges the lock against the parent it names, not the parent it stores #19853
Description
Class: a (pre-existing on main; found by the #18682 round-14 dev while fixing the same shape on the traversal seam, and not in PR #19728's diff).
What happens
A by-id update() resolves the parent header that readonlyWhen judges from the PRE-strip patch, payload FK first:
packages/objectql/src/engine.ts:12673onorigin/mainafc3b64928:await this.resolveMasterDetailParent(updateSchema, preRoWhen, priorRecord).
When the master-detail FK is declared static readonly: true, the strip that removes it runs AFTER the readonlyWhen strip has already judged against the parent the caller NAMED. The row keeps its stored parent, but the lock was judged against a different one.
Measured (the round-14 dev, real engine at ada09f0953, memory driver)
inv_line.amounthasreadonlyWhen: "parent.status == 'paid'";inv_line.invoiceismaster_detailwithreadonly: true. Linel1sits under paid invoiceinv_a;inv_bis open.update(l1, { amount: 999, invoice: 'inv_b' })→ committed, stored{ amount: 999, invoice: 'inv_a' }. The paid-invoice lock was bypassed.- Control:
update(l1, { amount: 555 })alone →amountdropped by the lock (it stayed 999). - Without
readonlyoninvoicethe repoint lands, and the result is correct.
The seat has read the cited line on main; the run itself is the dev's.
Property wanted
A by-id update judges readonlyWhen against the parent the write STORES, i.e. after every strip that can change the FK. PR #19728 fixed the same order on its own seam by resolving from the post-strip merged view ({ ...priorRecord, ...post-strip data }); that is a reference, not a prescription.
Dedupe words: readonlyWhen parent repoint strip; resolveMasterDetailParent preRoWhen; master_detail readonly repoint lock bypass; payload-FK-first static readonly.
Generated by Claude Code
Activity
objectstack-fleet commentedon Sep 23, 2026
定级
pm:queue·priority:p1(摘finding)·security·bug·domain:engine保留Path: access-security
Triage: lands in
packages/objectql/src/engine.ts(by-idupdate,readonlyWhenparent resolution) ⇒domain:engine; rationale: a record-level lock (readonlyWhen: "parent.status == 'paid'") is bypassed by naming a different parent in the same update when the master-detail FK is statically readonly — any caller with edit rights can change a locked row; measured on a real engine; p1 on data integrity (not p0: within one organization, and needs a static-readonly master-detail FK plus a parent-scoped lock).分诊席(
session_01Tw7jnJinGHvoGSi8aFkhPJ),2026-09-23T12:22Z。本席读完了卡面(本卡尚无评论)。前提复核(
origin/main8cbc3c0084)packages/objectql/src/engine.ts:12693:await this.resolveMasterDetailParent(updateSchema, preRoWhen, priorRecord)—— 用的是剥离前的补丁(payload 里的外键优先)。卡面引用的行号:12673是afc3b64928上的,文件后来长了 20 行,调用本身未变。判定
update(l1, { amount: 999, invoice: 'inv_b' })提交成功,存为{ amount: 999, invoice: 'inv_a' };对照update(l1, { amount: 555 })被锁拦下。p1:北极星「优先级」第 1 条(数据完整性);任何有编辑权的调用方都能用。⛔ 不是 p0:发生在同一组织内部,且需要「静态只读的主从外键 + 按父记录判断的只读锁」这一组合。readonlyWhen要按这次写入真正存下的父记录判断,也就是在所有可能改动外键的剥离之后再解析父记录。PR feat(formula,objectql): read one hop through a lookup in a validation predicate #19728 在它自己那条路径上用剥离后的合并视图做了同样的修正,可作参考。engine.ts是热点文件,认领前读一遍在飞 PR 的文件清单;有碰这一段的就排在其后,⛔ 不并入。Generated by Claude Code
objectstack-fleet commentedon Sep 23, 2026
Claim: PM loop round 21
Session:
session_01TEhopqrWQYBycZzyJHpAZrBranch:
claude/issue-19853-readonlywhen-post-strip-parentWorktree:
objectstack-issue-19853Domain:
domain:engineSeat:
domain:engine#1File surface:
packages/objectql/src/engine.ts(the by-idupdatepath'sreadonlyWhenparent resolution, and the bulk path only if measured to share the defect), new or existing tests underpackages/objectql/src/, one.changeset/*.md(stop on breach; explain in the report)Container & model:
M,mode:subagent,model: opus(no path-derived mandate; default judgment tier — order of strips versus the lock evaluation on a hot file)Clause-②: no
Thread-read: 5794720834
Serial constraints cleared:
PR #19854 (#19837), which edited this same call (resolveMasterDetailParent(updateSchema, preRoWhen, priorRecord, opCtx.context)), landed as 2bbb462335. Open PR #19728 (#18682, spec lane) edits engine.ts about 60 lines below, at the same update path's evaluateValidationRules call: this card's diff must stay textually disjoint from #19728's head (merge-tree clean, recorded in the report), and it does not merge into or wait on it. No other in-flight claim touches packages/objectql/src/engine.ts.Written 2026-09-23T13:25Z. A record-level lock is bypassed: a by-id update judges
readonlyWhenagainst the parent the payload NAMES while a staticreadonlymaster-detail FK is stripped afterwards, so the row keeps its stored parent but was judged against another (measured by the #18682 round-14 dev; the cited call is still pre-strip onorigin/main).Why
Clause-②: no: a restored lock. No accept set widens and no public surface moves; a runtime security behaviour change is not clause ②.Generated by Claude Code
objectstack-fleet commentedon Sep 23, 2026
os-dev-report
{
"issue": 19853,
"status": "done",
"branch": "claude/issue-19853-readonlywhen-post-strip-parent",
"pr": "#19877",
"session": "session_01TEhopqrWQYBycZzyJHpAZr — the dispatching PM's session (this run is its subagent; identity is the branch)",
"premise_still_valid": true,
"summary": "A1 REPRODUCED on origin/main 2bbb462 with engine.ts untouched (real ObjectQL engine, the suite's in-memory driver, the card's shape): update(l1, {amount: 999, invoice: 'inv_b'}) with invoice master_detail readonly:true and l1 under paid inv_a committed and stored {invoice: 'inv_a', amount: 999}; the control update(l1, {amount: 555}) stayed locked (100). A2 strip order on both update branches, from the code: primary-key strip (id only) -> readonlyWhen strip -> static readonly strip (skipped for isSystem; preserveAudit and hook-written keys exempt) -> assertNoStrictDrops -> evaluateValidationRules -> reference check -> driver. Two strips can take the FK after parent was resolved from it: the FK's OWN readonlyWhen lock (same step) and the static readonly strip (later). Field-level security refuses a forbidden FK outright (plugin-security step 2.5 throws), the #16344 hide pass is net zero, and hooks/normalize/validateRecord run before the resolution, so none of those is a stored-vs-named split. Fix in packages/objectql/src/engine.ts: settleMasterDetailLanding (used by the by-id AND the bulk branch, which measured the same defect) judges the FK's own lock alone against the header it names (verdict unchanged, then held out of the main strip), asks the static strip's verdict of the same stripReadonlyFields silently, and resolves parent from the view the write stores, so masterIdOf falls through to the prior FK when the repoint does not land; the requiredWhen repoint pre-check reads the same view; the #19837 tenant scope on the header read is untouched. A4: with the FK writable, paid->open commits both fields (judged against inv_b) and open->paid drops the amount while the move lands (judged against inv_a) — in both the judged header is the stored one because no strip takes the FK. What moves (A3): onFieldsDropped/strict now report amount (readonly_when) beside invoice (readonly); the reverse over-lock is gone; requiredWhen moves with the shared binding in both directions (flagged in open_questions against the claim's no-widening declaration); a parent-scoped lock on the FK itself that refuses the landing costs 2 header reads (was 1); FK-lock warn line prints first. One sibling of the same class on the record root measured and left for the seat to file (out_of_scope_findings).",
"tests": "All on HEAD 0a538af unless stated. pnpm --filter @objectstack/objectql exec vitest run --project local --maxWorkers=2 -> 'Test Files 305 passed (305) / Tests 5105 passed (5105)', exit 0. pnpm --filter @objectstack/objectql test:repo -> 1 file / 5 tests passed. pnpm --filter @objectstack/objectql typecheck -> exit 0 ('check:test-typecheck: OK — 40 file(s) / 234 error(s) / 65 pinned signature(s) held'). New block in src/engine-readonly-when-parent.test.ts, 16 cases (card, control, onFieldsDropped report, strict envelope code ERR_READONLY_FIELD_REJECTED + name + fields ['amount','invoice'] + drops — the engine error has no status field, HTTP mapping not measured, reverse over-lock, one-header read ['inv_a'], isSystem, preserveAudit, legit repoint both directions, FK own record-scoped and parent-scoped locks, requiredWhen both directions with VALIDATION_FAILED + fields, bulk both directions). Pre-fix run on the untouched engine: 10 failed | 27 passed (37) before the 16th case was added. Ablation (fix committed first; script with EXIT/INT/TERM trap, absolute paths): engine.ts restored to base blob ffcda81540 via git restore --source=2bbb462335 (tree only), on-disk hash == base blob, grep -c settleMasterDetailLanding = 0 -> 'Tests 11 failed | 27 passed (38)', exit 1, all 11 failures are the new pins, the 5 unchanged-behaviour pins stay green; restore via git checkout HEAD -- ABS path, on-disk hash 624fc1e38a == HEAD blob, git diff HEAD empty, porcelain empty, grep count 5 -> 'Tests 38 passed (38)', exit 0. Run once on d449675 and again on the final 0a538af with the same readings. No dist in the path: the suite imports ./engine.js from src. Targeted eslint (pnpm lint is CI's): population from eslint's own config (calculateConfigForFile/isPathIgnored: engine.ts and the test file matched, the changeset ignored); eslint --no-inline-config --format json over the 3 paths -> 3 results, 0 errors/0 warnings on the 2 linted files, exit 0; invariance: the config enables no type-aware linting and no cross-file rule, its only file reads are two baselines this diff does not touch, so no untouched file's verdict can move. HEAD 0a538af.",
"gates": {
"head": "0a538af9d7",
"derivation": "node scripts/pm/dispatch-gates.mjs --commands (no paths) -> 64 commands, identical list on d449675 and 0a538af",
"ran_verdict": "node scripts/pm/dispatch-gates.mjs --ran ran.list -> exit 0: '64 derived famil(ies) accounted for — 64 run, 0 NOT-MEASURED'",
"ran": [
"node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0",
"node scripts/check-adr-0087-registration.mjs --self-test :: exit 0",
"node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0",
"node scripts/check-changeset-no-major.mjs --self-test :: exit 0",
"node scripts/check-ci-filter-parity.mjs :: exit 0",
"node scripts/check-closing-keyword-parity.mjs :: exit 0",
"node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0",
"node scripts/check-comment-mask-adoption.mjs :: exit 0",
"node scripts/check-comment-mask-adoption.mjs --self-test :: exit 0",
"node scripts/check-comment-mask-corpus.mjs :: exit 0",
"node scripts/check-empty-changeset.mjs --base origin/main :: exit 0",
"node scripts/check-empty-changeset.mjs --self-test :: exit 0",
"node scripts/check-engine-split-ratio.mjs --days 90 :: exit 0",
"node scripts/check-engine-split-ratio.mjs --self-test :: exit 0",
"node scripts/check-keyed-text-bounds.mjs :: exit 0",
"node scripts/check-keyed-text-bounds.mjs --self-test :: exit 0",
"node scripts/check-platform-object-tenancy-census.mjs :: exit 0",
"node scripts/check-platform-object-tenancy-census.mjs --self-test :: exit 0",
"node scripts/check-plugin-teardown-shape.mjs :: exit 0",
"node scripts/check-plugin-teardown-shape.mjs --self-test :: exit 0",
"node scripts/check-registry-log-declared.mjs :: exit 0",
"node scripts/check-registry-log-declared.mjs --self-test :: exit 0",
"node scripts/check-rest-log-spy-declared.mjs :: exit 0",
"node scripts/check-rest-log-spy-declared.mjs --self-test :: exit 0",
"node scripts/check-system-context-census.mjs :: exit 0",
"node scripts/check-system-context-census.mjs --self-test :: exit 0",
"node scripts/check-undeclared-dep-imports.mjs :: exit 0",
"node scripts/check-undeclared-dep-imports.mjs --self-test :: exit 0",
"node scripts/docs-audit/check-affected-docs.mjs :: exit 0",
"node scripts/docs-audit/check-drift-comment.mjs :: exit 0",
"node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0",
"pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0",
"pnpm check:changeset-gate-self-tests :: exit 0",
"pnpm check:cross-package-test-inputs :: exit 0",
"pnpm check:dispatcher-error-vocabulary :: exit 0",
"pnpm check:doc-authoring :: exit 0",
"pnpm check:driver-memory-census :: exit 0",
"pnpm check:dts-closure :: exit 0",
"pnpm check:dual-build-cjs-loads :: exit 0",
"pnpm check:durability-log-level :: exit 0",
"pnpm check:engine-double-contract :: exit 0",
"pnpm check:gitlink-declared :: exit 0",
"pnpm check:issue-citations :: exit 0",
"pnpm check:lean-entry-closure :: exit 0",
"pnpm check:logger-receiver-detach :: exit 0",
"pnpm check:nul-bytes :: exit 0",
"pnpm check:objectql-double-limit :: exit 0",
"pnpm check:objectui-changeset :: exit 0",
"pnpm check:org-identifier :: exit 0",
"pnpm check:page-declaration-shape :: exit 0",
"pnpm check:pm-changeset-deadline-census :: exit 0",
"pnpm check:published-files :: exit 0",
"pnpm check:query-options-erasure :: exit 0",
"pnpm check:refd-timer-probe :: exit 0",
"pnpm check:slot-lookup :: exit 0",
"pnpm check:sourcemap-no-sources-content :: exit 0",
"pnpm check:stack-collection-maps :: exit 0",
"pnpm check:swallow-census-controls :: exit 0",
"pnpm check:test-source-alias :: exit 0",
"pnpm check:tier-file-adoption :: exit 0",
"pnpm check:type-check-coverage :: exit 0",
"pnpm check:type-check-debt :: exit 0",
"pnpm check:watch-hint-literal :: exit 0",
"pnpm check:where-matcher :: exit 0"
],
"extra": [
"node scripts/check-issue-citations.mjs (live) :: exit 0 — 13 citations resolve",
"node scripts/check-system-context-census.mjs :: exit 0 on 0a538af (110 sites); exit 1 on d449675 (new isSystem read in the helper) — fixed by the second commit",
"pnpm check:type-check-debt :: exit 3 PREREQUISITE NOT MET in the final sweep (objectql dist older than engine.ts after the ablation restore) -> pnpm --filter @objectstack/objectql build -> rerun exit 0; the ran list records the rerun",
"pnpm check:dual-build-cjs-loads :: exit 3 PREREQUISITE NOT MET in the first sweep (no workspace dist) -> turbo run build over ./packages/* ./packages// (72/72 tasks) -> exit 0",
"git merge-tree --write-tree --name-only 0a538af 3b9c5f2 (PR #19728 head) in a driver-free bare --shared clone :: exit 0",
"git merge-tree --write-tree --name-only 0a538af b940f32 (origin/main) in the same probe :: exit 0",
"eslint --no-inline-config --format json (3 paths) :: exit 0"
]
},
"files_changed": [
".changeset/19853-readonlywhen-stored-parent.md (+40/-0, @objectstack/objectql patch)",
"packages/objectql/src/engine-readonly-when-parent.test.ts (+236/-0)",
"packages/objectql/src/engine.ts (+174/-11)"
],
"deviations": [
"Reproduction order: the new test block was written first and run against the untouched engine.ts as the A1 probe; no engine edit preceded it.",
"Built the whole workspace (turbo, 72 tasks, 3m15s under the lock) to lift PREREQUISITE NOT MET on two derived gates — wider than the objectql closure build.",
"The record-root sibling was measured with a scratch test file created in the worktree, run once and deleted; never committed.",
"Commit trailers use AGENTS.md's model-free pair (Claude-Session + Co-authored-by: Claude) rather than the harness-suggested model-named trailer.",
"requiredWhen behaviour moves with the shared binding (by #4977's one-binding rule); the claim's no-widening declaration was copied into the PR body as instructed and the measured movement is raised in open_questions rather than silently resolved."
],
"mcp_calls": "0 — no MCP GitHub tool called",
"api_writes": "2 — both through the fleet-write relay as objectstack-fleet[bot] (each one POST /repos/objectstack-ai/objectstack/dispatches): pr_create -> POST /repos/objectstack-ai/objectstack/pulls (draft, #19877); comment -> POST /repos//issues/19853/comments (this report). git push is not counted. No label writes.",
"open_questions": [
{
"question": "The claim declares no widening, but the fix moves update()'s refuse/accept answer in one narrow shape: a payload naming an FK a strip takes back out, on an object with a parent-scoped requiredWhen. A write refused VALIDATION_FAILED against the named header now commits, and a write that cleared a required field under a header that requires it is now refused. Keep the declaration?",
"options": [
"A — keep 'no' (patch): treat the requirement movement as a restored guarantee, not an accept-set change; the changeset already states both directions in consumer terms.",
"B — re-declare 'yes (narrowing)' (minor + ADR-0087 disposition marker in the changeset): treat both directions as accept-set movement."
],
"recommendation": "A: every movement is the declared rule (#4977: lock and requirement share one header) finally being judged against the row the write stores; nothing authorable or published moves, and the refused shape was already a requirement bypass. The contract review is the PM's call."
}
],
"out_of_scope_findings": [
"class: a · record-root sibling of the same defect: a record-scoped readonlyWhen reading a static-readonly field is judged against the caller's forged value of that field, which the static strip then removes. Repro on HEAD 0a538af, real engine: status {readonly: true}, amount {readonlyWhen: "record.status == 'closed'"}, row t1 closed/100; update(t1, {amount: 555}) keeps 100 (control), update(t1, {status: 'open', amount: 999}) stores {status: 'closed', amount: 999}. Seam: packages/objectql/src/engine.ts update() readonlyWhen strip merged view built from the pre-static-strip payload (both branches). Judging the conditional strip over the post-static view would close both roots but moves reason attribution for fields declaring both locks. · dedupe words: readonlyWhen record forged readonly field unlock; static readonly strip after readonlyWhen; merged view pre-strip record root; readonlyWhen judged against stripped value",
"carrier: none (承接者:无) · #4889's landing rule still judges the FK's OWN parent-scoped lock against the header it moves TO (a move off a paid invoice is judged against the destination) — documented behaviour, noted in PR Acceptance notes, not filed"
]
}
Generated by Claude Code
objectstack-fleet commentedon Sep 23, 2026
ACCEPT — round 21 · PR #19877 ·
Fixesdomain:engine#1,session_01TEhopqrWQYBycZzyJHpAZr, written 2026-09-23T14:51Z. Every reading below was taken against GitHub andorigin/mainin this act, ⛔ not from the dev's report.The reproduction came first
Before any edit, on
origin/main2bbb462335with a real engine:update(l1, { amount: 999, invoice: 'inv_b' }), withinvoiceareadonlymaster-detail FK andl1under paidinv_a, committed and stored{ invoice: 'inv_a', amount: 999 }; the controlupdate(l1, { amount: 555 })stayed locked. The bulk path measured the same defect, and the fix covers both.Review
0a538af9d7main; first body lineFixes #19853; no other closing keyword;Clause-②: noat column 0 with the published-text citation execution-duties requires for a removed misrefusal (content/docs/data-modeling/fields.mdx, "Locking a detail from its master"), agreeing with thepatchchangesetengine.ts(settleMasterDetailLanding, shared by the by-id and bulk paths: the FK's own lock judged first against the header it names, the static strip's verdict asked silently,parentresolved from the view the write stores), one test file (16 cases), one changesetengine.tsat the base blob: 11 of the 16 new pins fail, and exactly the five unchanged-behaviour pins passopCtx.contexton the header read is untouchedorigin/maincheck-governed-merges --pr 19877)successor a roster skip on this head before readyOut-of-scope findings — dispositions:
readonlyWhenis judged against a forged value of a staticallyreadonlyfield that the static strip then removes (reproduced by the dev and by the contract reviewer, at head and on the base) → filed A record-scoped readonlyWhen is judged against a static-readonly field value the caller forged and the strip then removes, so the lock is bypassed while the row keeps its stored value #19887.readonlyWhenis unenforced server-side — the field lock fails open, so a paid invoice's frozen lines can be rewritten over the API #4889's landing rule still judges the FK's OWN parent-scoped lock against the header it moves TO → Acceptance notes (documented and pinned behaviour; a source-side guard would be a new rule).readonlyFK with a non-refusing parent-scoped lock also reads two → Acceptance notes (a one-clause correction for a later touch).Landing: ready → queue on this head once CI is fully green.
Generated by Claude Code
fix(objectql): judge a parent-scoped readonlyWhen against the header …
fix(objectql): judge a record-scoped readonlyWhen against the payload…
fix(objectql): settle a chain of readonlyWhen locks on the drop set t…
feat(formula,objectql): read one hop through a lookup in a validation…