Skip to content

fix(objectql)!: an update compares a value the caller sends only against the stored row as the caller reads it - #31063

Merged
objectstack-fleet[bot] merged 38 commits into
mainfrom
claude/sec-hardening-r15
Nov 7, 2026
Merged

objectstack-fleet[bot] merged 38 commits into
mainfrom
claude/sec-hardening-r15

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

security#1 · os-tesla · session_01L9qibvynRbCtMndW8EHNGc

Clause-②: yes (narrowing) — a non-system update that sends a secret: reference is refused even when the record already holds it, and a non-system update naming the master-detail FK of a detail with a parent-scoped requiredWhen, in a FK column it may edit but not read, no longer lets a row that already lacks a required value rest under the header it names.
Metadata-protocol: none

维护者速读

  • 改了什么:update 时把调用方送来的值与存储行比较的两处(secret 引用保留、master-detail 改挂时的前置检查读取旧表头),都改成只按调用方自己的读门所见来比较;系统写不变。
  • 为什么改:对一个调用方能写不能读的列,update 的答复不应随该列的存储值而变;此前这两处直接比较未投影的存储行。
  • 风险与回滚:收窄。非系统写送回 secret: 引用一律 400(含记录自己的引用);FK 不可读者点名表头时,缺必填值的遗留行不再“留存”。回滚即还原两处 engine 代码。
  • 先看:packages/objectql/src/engine.ts 按 id 与批量两处 priorMasterIdsCallerReads 调用;packages/objectql/src/engine-secret-internal-fields.ts 的保留分支;packages/objectql/src/engine-master-detail-related.ts 新增的 priorMasterIdsCallerReads 与预览 previousParents 的同步。
  • 两分钟验证:pnpm --filter @objectstack/plugin-security exec vitest run src/reference-forgiveness-readable-column.test.ts,预期 44 个全绿。
  • 需要你做的:无,席位落地。

What changes

An update compares a value the caller sends with the stored row in two more places than the reference check, and both now read the stored row only as the caller's own read door serves it (storedReferencesCallerReads, the one projection the reference check already uses), by id and in bulk. A system write is unchanged at both.

  • secret reference keep (encryptSecretFields). A non-system write keeps a secret: reference only where every row it lands on serves exactly that reference to the caller's read door. The read path serves a secret column masked to every caller, so no reference is kept: every non-system reference is refused 400 VALIDATION_ERROR, one answer whatever it names and whatever the column holds. Before, the record's own stored reference was accepted and kept. The echoed mask, a new value and null behave as before; the system principal keeps the pass-through. The refusal sentence now says a reference is kept only where the caller reads it in the column, and names omitting the field or sending back the mask as the way to leave it unchanged.
  • Master-detail repoint pre-check (update, by id and bulk). On a detail with a parent-scoped requiredWhen, when the write names the FK, the header the ADR-0113 pre-check reads as previousParent comes from the stored FK only as the caller's read door serves it (priorMasterIdsCallerReads). A FK the caller cannot read as stored leaves the prior header unknown, bound as the unresolved header, so whether a second header is read, and which, does not depend on the stored value. A FK the caller reads is judged as before, and a write that does not name the FK reads nothing more.
  • The validate preview (resolvePreviewParents, reached only from validate(), whose stored rows are already projected through servedAsStored) now binds the prior header the way the write does: when the patch names the FK over a stored FK the caller was not served as stored, previousParents[i] is the unresolved header (null). Preview and write now give one verdict for that caller, whatever header the row hung off.
  • content/docs/permissions/system-context.mdx row 23e states the new answer.

Tests

  • packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts, two new top-level describes on the real security plugin:
    • pre-check: with the FK { readable: false, editable: true }, naming a header answers identically whether the line hangs off that header or another, by id and in bulk, and nothing is written; controls: an edit that does not name the FK lands, a caller who reads the FK keeps the legacy rest and the repoint refusal, a system write is judged as before.
    • secret reference: the row's own reference, another row's and one naming nothing get one answer, by id and in bulk, for a caller who can and who cannot read the column; controls: the mask echo and a new value, and the system pass-through.
  • packages/objectql/src/secret-field-own-ref-write.test.ts pinned the old keep of a record's own reference. It now pins the rule above: the record's own reference is refused exactly as another's, by id and on a predicate update, and a beforeUpdate hook that spreads the stored row back is judged on the reference it carries (one that omits the secret column lands).
  • packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts pinned the same keep over REST and now pins the same refusal.
  • packages/objectql/src/validate-preview-parent.test.ts, a new top-level describe: for a caller not served the FK, preview and write give the same required verdict whatever header the line hangs off; control: a caller served the FK keeps the legacy rest and the repoint refusal, in both.

Acceptance notes

  • No shipped flow, seed, example or console path sends a secret: reference from a non-system write: every in-repo writer of a secret column (webhooks, SSO client secret, connector connections and credentials, flow credentials) writes under a system context, the CLI ownership apply works below the engine, and no shipped hook writes ctx.previous back into an update payload. The one in-repo reliance was the two suites above.
  • A third-party beforeUpdate hook that spreads ctx.previous into the payload on an object with a secret field is now refused for a non-system caller; the changeset names the fix (omit the secret columns).
  • The controlled-by-parent write gate (plugin-security, ADR-0055 §3) authorizes a detail write against the master its stored row hangs off, read as stored. That is the authorization model itself, not a comparison of a sent value with the stored row, so this PR does not change it.
  • On a driver that leaves an unset FK out of the stored row entirely, a non-system caller who names the FK now has the prior header bound as unresolved, as for a hidden FK; a driver that stores the FK as null is judged as before.

Verification

Ablations (each committed first, mutated through scripts/ablation-replace.mjs, restored to HEAD and verified empty)
  • Secret keep compared against the stored rows as read: objectql own-reference suite 4 failed / 16 passed; security suite 4 failed / 40 passed (by id and bulk, for both grants). Restored: blob equals HEAD, git diff HEAD empty.
  • By-id prior FK read off the stored row: security suite 1 failed / 43 passed (the by-id hidden-FK pin: the line resting under the stored header was written). Restored as above.
  • Bulk prior FK read off the stored rows: security suite 1 failed / 43 passed (the bulk hidden-FK pin). Restored as above.
  • Preview prior header bound as before (no unknown arm): preview pin file 1 failed / 23 passed (the preview admitted what the write refused). Restored as above.
Suites and gates

Code readings are on f49cf38154; the final head 82a9a6bbda adds only one changeset sentence, and the changeset gates were re-run on it.

  • pnpm --filter @objectstack/objectql test: Test Files 563 passed (563), Tests 11361 passed (11361). typecheck and check:test-typecheck OK.
  • pnpm --filter @objectstack/plugin-security test: Test Files 248 passed (248), Tests 5773 passed, 45 skipped. typecheck and check:test-typecheck OK.
  • Full dogfood battery, pnpm --filter @objectstack/dogfood test under the verify lock: VERDICT command-exit 0, Test Files 430 passed, 3 skipped (433); Tests 3572 passed, 41 skipped (3613).
  • Derived gates (dispatch-gates --commands, 10 paths): 97 derived, 95 exit 0. check:type-check-debt first exited 3 (prerequisite) and passed on a rerun after its own full build. Two stay NOT MEASURED locally, exit 3, prerequisite not met (a full build): spec check:skill-examples and check:dual-build-cjs-loads; CI runs both. dispatch-gates --ran with exit codes: 97 accounted, 0 unrun.
  • Re-run on 82a9a6bbda: check-adr-0087-registration (the changeset reads as breaking, clause-② narrowing, not-required no-migration-prescription), check-changeset-no-major, check-empty-changeset, check:nul-bytes, check:doc-authoring, all exit 0.

Generated by Claude Code

claude added 30 commits November 2, 2026 12:26
… the caller can read

The write's reference check now asks the caller's own read door for every
`lookup` and `user` value a non-system caller supplies, exactly as it already
did for a `master_detail` header: a target the read door does not return
answers `reference_not_found`, the answer of a target that exists nowhere.
System writes keep their exemption; a value the row already stores is not
judged again.

Claude-Session: https://claude.ai/code/session_01ADHkmSZhbvtSoFM7YTkq6B
Co-authored-by: Claude <noreply@anthropic.com>
…edits for the caller-read rule

Claude-Session: https://claude.ai/code/session_01ADHkmSZhbvtSoFM7YTkq6B
Co-authored-by: Claude <noreply@anthropic.com>
The parked implementation edited engine.ts bodies that main has since moved
into facets: the reference-check docblock now lives in
engine-referential-integrity.ts and the caller-readable reference helper in
engine-references-and-master-detail.ts. The resolution keeps main's layout
and applies the parked change where the code now lives.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
…on rules

A reference a caller cannot read answers reference_not_found on every write
door (insert, update by id, bulk update, and the validate preview), whatever
a validation rule would answer. The one-hop predicate read's docblock states
the rule that replaces the retired write-without-reading-the-parent persona.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
…scope covers

A delegation's admin scope over a business-unit subtree (ADR-0090 D12) also
grants read on the units inside that scope, so delegated administration can
name the units it administers. The read is admitted only for a read of
sys_business_unit the caller's sets do not grant, on the caller's own behalf,
and is bounded to the union of the subtrees its scopes cover.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
…eference check

A write that is not a system write has its references checked before the
record's validation rules; a system write keeps the order it had, so its
answer is unchanged. One predicate, referencesCheckedBeforeRules, is read by
insert, update by id, bulk update and the validate preview.

Pins: each door answers reference_not_found for an unreadable reference a
rule would trip on, and for one that exists nowhere, while a readable one
still answers the rule; a system write and a flow run's system write keep
the rule's answer; a read of the target that fails is refused, and a
reference to an unregistered object is admitted.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
The engine's reference check now reads a user or lookup target through the
caller's own read door, so suites whose subject is another predicate on the
same write provision sys_user with a row for each user they name, and give
their callers the org_user_ids the runtime fills from sys_member.

The write-preview parity controls read the related account, and a new block
drives the editor who cannot: both doors answer the lookup's
reference_not_found, never the rule. The tenant-scope suite counts the rule's
related reads alone, and a user the caller cannot read is now answered on the
reference check.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
The records-forms predicate-relationship-traversal item takes revision 7: a
member who cannot read the lookup parent may not name it, and is answered
reference_not_found for a tripping and a non-tripping parent alike. The
dogfood case is rewritten to that rule, and the changeset states the refused
writes and the delegate's business-unit read.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
…reference check

The write's reference check reads each lookup or user value it names through
the caller's read door, projected to id alone. The read-count pins measure
what the audit summary reads, so that read is left out of their counters.
The log line for a failed caller read names a referenced record rather than a
master-detail header, as the read now serves lookup and user values too.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
…ds its scope's units

The error catalog's reference callout and the field reference's referential
integrity section state that a non-system write may name, in a lookup,
master_detail or user field, only a record its caller can read, before the
record's validation rules. The delegated administration page states that an
admin scope grants read on the business units inside its subtree.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
…first

With the reference check before the record's validation rules, a non-system
write that names a master-detail header it cannot read, or one that exists
nowhere, answers the field's reference_not_found rather than the rule's
unevaluable refusal; a header the row already stores is still bound by the
rules, and a system write keeps the rules first. The header-read counts add
the reference check's read door, and the stored-metadata suites count only
reads that ask for more than id.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
…nd the delegate-read pins type their options

The system-context page's reference-check row names referencesCheckedBeforeRules, and its site register is regenerated.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
The reference check composes #30205's ban check with the caller-read rule:
a `lookup` or `user` value naming a `sys_user` row asks the caller's read
door first (callerReadableReferenceRow), then the one elevated probe that
answers the row's existence and its ban. A user the caller cannot read
answers reference_not_found, and its ban is not read. The banned-user rig
gives its caller the read of a user's id a member has, and pins both.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
… rule is what answers it

A non-system write now meets the reference check, and with it the ban check,
before the record's validation rules; the control's peer keeps `banned`
true for the rule to read, with a ban no longer in force.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
Revision 7 of records-forms.predicate-relationship-traversal, the dogfood
case and the objectql pins say what the write answers: a reference the
caller cannot read is reference_not_found, before the record's validation
rules.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
…writer reads

A grant's holder is a `user` value, so a non-system write naming it must
name a user its caller can read; the suite provisions its three users, as
the sibling grant suites do, so the set the row names is what it measures.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
…e probe names

A write may name, in a lookup or user field, only a record the writer can
read (#29029). The verifier's permission sets now read every record of each
object a probed detail's required reference names outside its master chain,
and the fixture's manager position reads the people a contract's signer
names.

Claude-Session: https://claude.ai/code/session_01Utp6CYQjMkN514R78sCrTT
Co-authored-by: Claude <noreply@anthropic.com>
…d values the caller may read

A refused reference value an update carries back is forgiven only when every
row the write lands on stores it as the caller's own read door serves that
row. A column the caller cannot read holds no stored value for it, so it
forgives nothing and the value is judged like any other. A caller who reads
the column keeps the forgiveness; a system write compares the rows as
stored, as before.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…mpares only stored values the caller may read

For master_detail, lookup and user, by id and in bulk: a column the caller
may edit but not read answers a value the rows store exactly as a value they
do not. Controls: a caller who reads the column keeps the forgiveness, the
same grant edits another column, and a system write keeps its exemption.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…ed values the caller may read

The changeset and the two pages that describe the forgiveness say that a
column the caller may edit but not read holds no stored value for it.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…s the forgiveness's system-write read

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…y service loads at module top

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
…d row as the caller reads it

A non-system write's secret reference is kept only where the caller's own
read door serves the column as stored, and a master-detail repoint's prior
header is read off the stored foreign key only as the caller reads it.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
… a stored value the caller cannot read

Pins the secret-reference keep and the master-detail repoint pre-check, by
id and in bulk, with the real security plugin; rewrites the own-reference
suites to the rule that a secret column reads masked.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
# Conflicts:
#	packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts
…not served as the write binds it

When the patch names the master-detail FK over a stored FK the caller's read
door did not serve as stored, the preview binds the prior header as
unresolved, as the write does, so preview and write give one verdict.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 6, 2026
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 5 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/permissions/system-context.mdx (via encryptSecretFields (symbol, a method of class ObjectQL; a top-level function))
What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: ObjectQL (symbol, 80 pages)
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 63 of 224 client-bound route-ledger rows — the other 161 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 161: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 57 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 104 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 21 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 022b4e7eef42c54c3489175af3f3be870a994a9d → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7f145756918dedec6df82e70116380331bdbf6d2 — the merge of head 82a9a6bbdad7b753e92d22009da814e295f8d138 into base 022b4e7eef42c54c3489175af3f3be870a994a9d, 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 7f145756918dedec6df82e70116380331bdbf6d2 && git checkout 7f145756918dedec6df82e70116380331bdbf6d2
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 022b4e7eef42c54c3489175af3f3be870a994a9d 82a9a6bbdad7b753e92d22009da814e295f8d138 && git checkout -B drift-repro 022b4e7eef42c54c3489175af3f3be870a994a9d && git merge --no-ff 82a9a6bbdad7b753e92d22009da814e295f8d138

node scripts/docs-audit/affected-docs.mjs --json 022b4e7eef42c54c3489175af3f3be870a994a9d

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

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 022b4e7eef42c54c3489175af3f3be870a994a9d → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

security#1 · os-tesla · session_01L9qibvynRbCtMndW8EHNGc

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 82a9a6bbdad7b753e92d22009da814e295f8d138
Local-runs: none
Scope: packages/objectql/src/engine.ts, packages/objectql/src/engine-secret-internal-fields.ts, packages/objectql/src/engine-master-detail-related.ts, packages/objectql/src/engine-validate.ts, packages/objectql/src/secret-field-own-ref-write.test.ts, packages/objectql/src/validate-preview-parent.test.ts, packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts, packages/qa/dogfood/test/secret-field-own-ref-write.dogfood.test.ts, .changeset/update-stored-row-caller-read-secret-ref-and-repoint.md, content/docs/permissions/system-context.mdx + packages/objectql/src/engine-referential-integrity.ts (storedReferencesCallerReads), packages/objectql/src/engine-top-level.ts (servedAsStored, masterIdOf), packages/objectql/src/validation/rule-validator.ts (the ADR-0113 pre-check and previousParent), packages/objectql/src/engine-references-and-master-detail.ts (resolveMasterDetailParent, resolveMasterDetailParents), packages/objectql/src/engine-insert.ts (readUpdatePriorRow), packages/core/src/utils/internal-write-response.ts, packages/triggers/trigger-record-change/src/record-change-trigger.ts, packages/plugins/plugin-webhooks/src/webhook-headers-gate.ts, packages/services/service-automation/src/connector-connections.ts, packages/services/service-automation/src/connector-credentials.ts, packages/services/service-automation/src/flow-credential-channel.ts, packages/plugins/plugin-auth/src/sso-client-secret.ts, packages/plugins/plugin-webhooks/src/migrate-webhook-secrets.ts, ui/packages/components/src/renderers/form/form.tsx, examples/app-showcase/src/data/objects/invoice.object.ts, examples/app-showcase/src/security/permission-sets.ts, docs/adr/0055-master-detail-controlled-by-parent.md, docs/adr/0113-required-write-contract-vs-column-constraint.md

Reviewed at 2026-11-06T23:51Z, read-only: the PR body, its file list, the three-dot diff against the merge base bd1b7685 on origin/main, the check-runs on the head, and the cited code on main. The recogniser reads no face off the paths; the changeset's Clause-②: yes (narrowing) line puts the review on the narrowing face, where the floor is every non-prose path of the diff, listed above with the two prose paths and every file the hunks make a claim about.

Check-runs on the head as they stand. 15 completed green, among them Check Changeset, Governed Surface Queue Guard, Dogfood Regression Gate, Test UI, Spec property liveness, PR-body guards and Lint · PM dispatch-gates; 21 still running: Build Core, Build Docs, Test Core 1/8 to 8/8, Type Check · workspace / source gates / debt ledger / consumer gates, Lint · ESLint / repo & tooling gates / docs & code guards / CI, release & engine gates / scoped · slot lookup & corpus / scoped · query-options erasure, and Live E2E UI; 12 skipped by their own conditions (UI-only and opt-in lanes). The queue guard reads this record together with those; nothing here is a verdict on a run that has not finished.

① Derived judgments

The PR narrows two accept sets of ObjectQL.update for a non-system caller, both by routing the comparison of a sent value against the stored row through storedReferencesCallerReads, the one projection PR #31031 already uses for the reference check: the stored row is kept only in the columns the caller's own read door serves exactly as stored. A system caller is returned the rows unprojected at the top of that function, so every system write is unchanged. Each change, judged:

S1 — a secret: reference on a non-system update is refused, the record's own included. Right, and reached by no shipped caller.

  • Mechanism as read: encryptSecretFields keeps a reference only when every row the write lands on serves that same value in that column to the caller. maskSecretFields runs on every find and findOne with no system carve-out and turns every set secret column into the mask, and servedAsStored(mask, reference) is false, so the projection never carries a reference and the keep arm is unreachable for a non-system caller. The echoed mask is still dropped before this branch; null still clears; the empty string is still refused; a new value is still sealed. The refusal still names the column and never the value, and now says how to leave the field unchanged (omit it or send the mask). One answer whatever the reference names and whatever the column holds, by id and on a predicate update, matching the ADR-0112 VALIDATION_ERROR/400 pair the row 23e of system-context.mdx now states.
  • Consumer hunt. Every shipped secret column sits on five platform objects (sys_webhook.signing_secret / headers_secret, sys_sso_provider.oidc_client_secret, sys_connector_connection, sys_connector_credential.value, sys_flow_credential.value); no example app or shipped business object declares one. Each writer path, checked:
    • Console record forms (ui/packages/components form renderer): an object-derived secret maps to a password input; the row is read through the data door, so the form holds the mask and echoes it on save; the engine drops the key. No-op, as before.
    • Console settings pages (datasource, connector connections, flow-node credentials, SettingsField): dedicated routes, "blank keeps the stored value", a hasSecret flag rather than the value. None sends a reference.
    • Write responses: maskCredentialFieldsInWriteResponse in @objectstack/core masks every credential-class column on every write response, so a client that re-saves what a PATCH returned sends the mask, never a reference.
    • REST export → import (update / upsert modes): the export route reads through engine.find under the caller, so the file carries the mask; a re-import echoes it.
    • Webhooks: sys_webhook is user-writable through the generic data API and the Setup form; the headers gate lets the mask and a reference through to the engine, which answers; the form echoes the mask. The secret migration writes under a system context.
    • SSO provider: the client secret is sealed by a hook that updates under isSystem: true; registration is a server-only platform-admin route.
    • Connector connections, connector credentials, flow credentials: every write in service-automation carries isSystem: true (with a userId for attribution). Pass-through, unchanged.
    • Flows and automation: the record-change trigger serves record and previous to a flow through the ADR-0100 mask, so a flow not running as system that copies a credential column sends the mask. A flow with runAs: 'system' is a system write.
    • CLI: data commands go over HTTP as the API-key user (masked reads); the secret-reference union is read-only at driver level; ownership apply works below the engine.
    • MCP data tools: HTTP as the agent's user; inbound HTTP never carries the system flag; reads masked.
    • SDK: no handling of a secret: value anywhere in the client.
    • Seeds: the seed loader writes under a system context and compares credential columns against the mask.
    • Shipped beforeUpdate hooks that write the stored row back into the payload: none. The three in-tree { ...previous, ...data } merges (external membership, notification template placement, the flow record decoupler) build a judged view and write nothing back; none of those objects carries a secret column.
      The dev's two claims hold in the tree: every in-repo secret writer uses a system context, and the masked echo is a no-op. The one in-repo reliance on the old keep was the two suites the PR rewrites. The pre-image a hook sees (readUpdatePriorRow, a direct driver read) does carry the stored reference, so a third-party hook that spreads ctx.previous onto the payload of one of those five objects is refused for a non-system caller; the changeset names it and the fix.

S2 — a master-detail repoint pre-check binds the prior header as unresolved when the caller is not served the stored FK; the validate preview binds it the same way. Right, with one edge noted under ③.

  • Mechanism as read, by id: namesMaster is true exactly when the landed payload view carries the FK key (settleMasterDetailLanding keeps the key only when the FK lands); only then is priorMasterIdsCallerReads asked, one caller-context find by id projected to the FK. null (not served, hidden, transformed, or the row not returned) binds previousParent to null; in evaluateValidationRules a null previousParent leaves parent unbound for the pre-evaluation, the predicate faults, and a legacy violation cannot be proven, so the row is refused required — the fail-closed rule the pre-check already states ("enforcement stays on unless a legacy violation is proven"). { id } reproduces the old masterIdOf(fk, null, priorRecord) verdict exactly, and resolveMasterDetailParent is now handed { [fk]: id } in place of the whole pre-image, which it only ever read that one key of. A payload that does not name the FK reads nothing more and keeps previousParent undefined, so it defaults to parent as before.
  • In bulk: the matched priorRows are projected once; the new lookup is keyed by row identity, and the consumer loop iterates the same priorRows array with no reassignment in between, so the identity holds. Rows bound unknown get null; known rows get the header from one batched elevated read, as before.
  • Preview: resolvePreviewParents already receives stored rows projected through servedAsStored; it now binds null when the patch names the FK and the projected row lacks it, so preview and write agree for that caller. The new pin in validate-preview-parent.test.ts holds this with the security-shaped hide mask.
  • Consumer hunt. S2 reaches a non-system caller only on an object with a parent-scoped requiredWhen AND a grant of { readable: false, editable: true } on the master-detail FK. Every shipped requiredWhen (showcase invoice issued_on, paid_on; line description) is record-scoped — hasParentScopedRequiredWhen is false for all of them — and every shipped readable: false grant (showcase Client Liaison on project budget/spent/budget_remaining) is paired with editable: false. No shipped object, grant, form, flow, import, CLI or MCP path meets the narrowed answer. A caller who reads the FK is answered as before (control pins in both suites); a system write is answered from the rows as stored.

Tests: the four suites pin the narrowed answer on the real security plugin by id and in bulk, with the mask echo, a new value, a reading caller and a system caller as controls, and the dogfood suite pins it over REST. Nothing in the diff is prose-only; the docs row 23e matches the code.

② Semver level

@objectstack/objectql: patch with a BREAKING banner, Clause-②: yes (narrowing) and Metadata-protocol: none on both the PR body and the changeset, and one ADR-0087 marker not-required (no-migration-prescription). Correct under the Post-Task Checklist rule: no metadata key, accepted value or type changes for an author, so the kind is none and takes patch; the narrowing is a non-protocol breaking change that keeps its banner and disposition. The migration prescription the body carries (omit the field or send the mask; a hook drops its secret columns) is the right one and is the sentence the refusal itself now gives. The ! in the title matches.

Clause-②: yes (narrowing)
Metadata-protocol: none

③ Boundary flags

  1. The controlled-by-parent write gate (ADR-0055 §3) authorises a by-id detail update against the stored master even when the caller cannot read the FK column. Separate ruling; it does not block this PR. The gate reads the stored FK to decide master EDIT access — the authorization model of its own ADR ("editing a detail requires edit access to its master"), not a comparison of a value the caller sent with the stored row. The rule this PR makes real ("an update's answer to a column the caller may write but not read must not depend on what the column holds") would, applied to the gate, amend ADR-0055 §3 itself (either deny every such write or stop judging the real master); that is an ADR amendment, a card of its own, and nothing in this diff touches assertControlledByParentWrite. Recorded here as a follow-up to file, not as a defect of this PR.
  2. A third-party beforeUpdate hook that spreads ctx.previous onto the payload of an object with a secret column is now refused for a non-system caller. Correct and disclosed: the pre-image is a raw driver read, so such a hook writes a reference the caller never read; the changeset names the fix. No shipped hook does this.
  3. A driver that omits an unset FK from the stored row binds the prior header as unresolved for a non-system caller who names the FK, while a driver that stores null is judged as before. Disclosed, direction closed, not blocking: on the omitting driver a caller who CAN read the FK, attaching a row that hangs off no master and already lacks a required value, is refused required instead of resting; on a null-storing driver the same caller rests under the landing header. The refusing answer is the one ADR-0113 D1 plus the pre-check's fail-closed rule actually give (a row with no prior header has no proven legacy violation), so the divergence is the null-storing driver's older rest, not a new hole; the SQL drivers store null. Follow-up: one verdict for both storage shapes, filed against the pre-check, not this PR.
  4. A hook that writes the stored FK back onto the payload counts as naming it on the repoint arm. Same shape as flag 2 for the FK; the changeset names the hook case for the secret arm only. A sentence to add, not a code change.

④ Design judgment

The narrowing is the published rule made real, not a capability taken away at the consumer. The published spec already says a non-system write may seal a new value or keep its own record's reference, and the read door already serves a secret column masked to every caller; the PR resolves "its own" to the value the caller reads, which is the only reading under which an update's answer cannot vary with a stored value the caller may not read — the invariant #31031 introduced for references and this PR extends to the two remaining sent-versus-stored comparisons on the update path. Nothing a consumer can obtain through the published surface (a data read, a write response, an export, a trigger payload) is a value the narrowed arm refuses, so no capability a published door grants is withdrawn; what is withdrawn is a keep that only a privileged read or a hook could ever have exercised. On the repoint arm, the pre-check already refused when the prior header could not be evaluated; the PR only stops a non-system caller's stored FK from supplying that header when the caller is not served it, and binds the preview the same way so validate() predicts the write. The one-read cost lands only on a write that names the FK on an object with a parent-scoped requiredWhen, or sends a reference.

Attempts to break it, all read against the cited code: a payload FK whose value the strip takes out (caller may not edit) does not name the master, so the hidden-column branch is never entered for it; a system caller is returned unprojected rows at the top of the projection and keeps both pass-throughs; a secret column that is also internal is omitted rather than masked and meets the same refusal; a row the caller's read door does not return at all (tenant scope, sharing, soft-delete) is bound unknown on the FK arm and refused on the secret arm, both the closed direction; the bulk lookup's identity keying survives because the matched array is the one iterated; the preview's stored rows were already projected, so the new null arm cannot admit what the write refuses. I found no path where the narrowed answer is wider than before, and none where it depends on the stored value of a column the caller is not served.

Must-fix: none.
Should-fix (follow-ups, none blocking): the webhook headers gate's docblock still describes a secret: reference as a value the engine leaves alone — stale for a non-system caller, a doc edit in a file outside this diff; flag 3's one-verdict-per-storage-shape follow-up on the pre-check; flag 4's sentence in the changeset; flag 1 filed as an ADR-0055 §3 card.

Implemented-by: claude/sec-hardening-r15
Reviewed-by: session_01L9qibvynRbCtMndW8EHNGc

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

security#1 · os-tesla · session_01L9qibvynRbCtMndW8EHNGc

席位核对

ACCEPT · 2026-11-06T23:53Z · head 82a9a6bbdad7b753e92d22009da814e295f8d138

  • Contract review: PASS at this head, CONTRACT_REVIEW_TIER on the narrowing face (the changeset's Clause-②: yes (narrowing) line), record 6698963723. No must-fix. Its should-fixes (the webhook headers gate docblock, a verdict for key-omitting drivers, a changeset sentence for a hook that writes the FK back, and the controlled-by-parent question) go into this seat's follow-up list; none changes what lands here.
  • What lands: a non-system update compares a value it sends against the stored row only in the columns the caller's own read door serves as stored, the same projection fix(objectql)!: a non-system write may name, in a lookup or user field, only a record the caller can read #31031 uses for references. A secret: reference sent by a non-system caller is refused, the record's own included; the mask, null and a new value behave as before. A master-detail repoint naming an FK column the caller may edit but not read is judged as if the caller read nothing there. Every system write is unchanged.
  • Consumer hunt (record S1): every shipped writer of a secret column is a system write, echoes the mask, or uses a "blank keeps the stored value" route; none sends a reference.
  • Checks: every check-run on this head is success (38) or skipped (12, by their own conditions). git merge-tree against current main is clean.
  • Gates (dev, at this head): the derived dispatch-gates --commands set run with none unmeasured; objectql and plugin-security suites green; the dogfood battery passed; ablations of both halves red as predicted and restored.
  • Shape: base main, no closing keyword. Patch for @objectstack/objectql, with Clause-②: yes (narrowing) and Metadata-protocol: none.
  • Governed: check-governed-merges.mjs --pr 31063: not governed; 576 changed lines ≤ 3000 ⇒ Tier S.
  • Landing authority: the maintainer's standing approval for this seat's security PRs at Tier S (2026-11-06 card answer in the seat's thread) and 「你应该是使用 pm-dispatch 技能自主审核和落地 pr」.
  • Next: relay pr_ready + automerge_enable.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 6, 2026 23:55
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 6, 2026 23:55
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 6, 2026
Merged via the queue into main with commit cad764d Nov 7, 2026
51 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/sec-hardening-r15 branch November 7, 2026 00:32
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/l tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants