Skip to content

fix(plugin-security): explain reports a Modify All-superseded row-conditioned field lock as binding on no row - #29637

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-n2gs05
Nov 3, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-n2gs05

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Steve · project thread
Fixes #29610

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

维护者速读

  • 改了什么:os explain 的字段级说明层,对持有 Modify All Data 的调用者,不再把带 when 行条件的字段锁说成"按行判、其余处一律锁",改为单列一句"已被 Modify All 字段票覆盖,锁在任何行都不生效",并点名持票的权限集。
  • 为什么改:裁决 6573563853 字母 A(fix(plugin-security): a View All / Modify All set reads every field of its object, so an app field mask no longer hides fields from admins #29387 已落地)后,Modify All 的字段票在逐条票之后、任何 when 判定下都把每个字段投为可编辑;说明文字却还在讲锁,管理员排查字段权限时会被误导。
  • 风险与回滚:只改说明文字,执行面零改动;回滚即 revert 本 PR。View All 与无票调用者的句子与今天一致。
  • 先看:packages/plugins/plugin-security/src/permission-evaluator.ts(conditionedFieldEntries 读同一个 superuserFieldVote)、explain-engine.ts 的 conditionedSentence、explain-engine.test.ts 三条 [#29610] 用例。
  • 两分钟验证:pnpm --filter @objectstack/plugin-security exec vitest run src/explain-engine.test.ts -t 29610,预期 3 条全绿。
  • 需要你做的:无,席位落地。

What changes

The FLS layer of the access explanation listed every field permission that carries a when as "judged per row by the write gate, applied as the lock everywhere else". For a caller whose sets hold Modify All Data on the object that was untrue: since ruling of record 6573563853 letter A (PR #29387), getFieldPermissions applies the super-user field vote after the per-entry votes, under every when verdict, so such a lock binds on no row. Measured through the public explain route (see Verification): the write gate admits the field for that caller while the report called it locked.

  • Seam (no second copy of the rule). PermissionEvaluator.conditionedFieldEntries now names, per entry, the sets whose Modify All vote supersedes it (supersededBy). It reads superuserFieldVote from @objectstack/core, the same function getFieldPermissions applies, and attributes the sets through superuserBypassSets (the one holdsSuperuserBypass predicate). No extraction was needed: the vote already lives in core and both readers now call it. The explain engine only words the result.
  • Wording: annotate, not drop. Superseded entries move into their own sentence, in the "Superseded by the View/Modify All Data bypass [...]" form the record-level sharing attribution already uses: N row-conditioned field permission(s) are superseded by the Modify All Data field vote [SETS], which votes every field editable on every row, so the lock binds on no row: [...]. Entries the vote does not supersede keep the existing sentence, counted on their own.
  • View All supersedes nothing. It votes only the read bit, which a conditioned entry already casts (the spec refuses readable: false beside a when), so the edit lock still binds per row. Measured: the write gate refuses the field on a matching row for a View All holder.
  • D10. Each principal's entries are judged against its own sets. The agent and delegator masks intersect, so a delegator's lock still binds when only the agent holds the vote.
  • The hidden/partial mask is untouched. Enforcement is unchanged.

Forced test edit: security-plugin.test.ts [#25866] conditionedFieldEntries lists the row-conditioned entries of the object, as authored pins the entry shape with toEqual, so both expected entries gain supersededBy: [].

Tests

explain-engine.test.ts, next to the [#25866] row-condition test:

  • [#29610] a Modify All set supersedes a row-conditioned lock — the entry is listed as superseded by the vote, names the set, and is not called "judged per row".
  • [#29610] a View All set does NOT supersede it — read-side control: the row-conditioned sentence stays.
  • [#29610] a caller without the vote keeps the row-conditioned sentence — control.

Acceptance notes

Verification

Head 8ec800e4 (fix ca1b6e6a, the round-2 changeset declaration commit, and merges of origin/main; the latest merge brought plugin-security files from main, so the package runs and gates below were repeated on this head).

Measurement through the public explain route, before and after (scratch dogfood boot, not committed)

A throwaway dogfood boot measured three callers that each hold a when-conditioned field entry on one object: one without the vote (control), one with View All, one with Modify All. Each read the explain route's FLS layer, and the write gate's answer for that field on a row the condition names was read beside it.

  • Write gate: control refused, View All refused, Modify All admitted (the ruled behaviour).
  • Before (main 5bd615ef): all three were told the entry is "judged per row by the write gate, applied as the lock everywhere else", Modify All included.
  • After (ca1b6e6a): control and View All unchanged; Modify All reads "2 row-conditioned field permission(s) are superseded by the Modify All Data field vote [the MA set], which votes every field editable on every row, so the lock binds on no row".
Red first on main, ablation
  • Red on main (tests added, fix absent): Tests 1 failed | 2 passed — the Modify All case: expected 'No field-level masking applies. 1 fie…' not to contain 'judged per row by the write gate'.
  • Ablation via scripts/ablation-replace.mjs on permission-evaluator.ts: the vote read was disabled with a string-literal guard. Anchor 1 to 0, blob f0269b4c to 57f704a0. Same failure, Tests 1 failed | 2 passed. Restored: blob equals HEAD f0269b4c, git diff HEAD empty. The test imports the evaluator from src by relative path, so no dist leg applies.
Package runs and gates
  • pnpm --filter @objectstack/plugin-security typecheck exit 0; pnpm --filter @objectstack/plugin-security test: Test Files 241 passed (241), Tests 5205 passed, 45 skipped (5250). Lock VERDICT command-exit 0, on ca1b6e6a, 1fcfe8f5 and 8ec800e4 (5206 passed on the last, one test more from main).
  • Lint: root eslint --no-inline-config on packages/plugins/plugin-security exit 0 (the package has no lint script).
  • dispatch-gates --commands on 8ec800e4 (and before it on 1fcfe8f5): 62 derived, 62 run, all exit 0; --ran reconciliation: 62 run, 0 NOT-MEASURED, 0 UNRUN. On 1fcfe8f5 three gates first exited on a missing prerequisite (check-plugin-teardown-shape --self-test exit 3 needed the pinned fixture commit fetched; check:dts-closure exit 1 and check:dual-build-cjs-loads exit 3 needed a declaration build of every package) and were re-run green after it.
  • Dogfood battery: not owed (no ! in the title, no (narrowing) arm).

…ditioned field lock as binding on no row

The FLS layer of the access explanation listed every `when`-conditioned
field entry as a lock applied everywhere a row is not judged. A caller
whose sets hold Modify All on the object gets the super-user field vote
after the per-entry votes under every verdict, so that lock binds on no
row for them. conditionedFieldEntries now names the superseding sets from
the same superuserFieldVote getFieldPermissions applies, and the layer
words those entries as superseded. View All and the control keep the
existing sentence.

Claude-Session: https://claude.ai/code/session_01ADHkmSGbDCYYvHbU6am1V2
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

5 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not 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 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 101 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 — 17 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 322cf9068f143f39cbfbc7527294db24f01cd649 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7971e1cd249c0b76b62e85886cf2b9d072a4f3d3 — the merge of head 8ec800e416727581b828060a616db4ef9d45feca into base 322cf9068f143f39cbfbc7527294db24f01cd649, 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 7971e1cd249c0b76b62e85886cf2b9d072a4f3d3 && git checkout 7971e1cd249c0b76b62e85886cf2b9d072a4f3d3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 322cf9068f143f39cbfbc7527294db24f01cd649 8ec800e416727581b828060a616db4ef9d45feca && git checkout -B drift-repro 322cf9068f143f39cbfbc7527294db24f01cd649 && git merge --no-ff 8ec800e416727581b828060a616db4ef9d45feca

node scripts/docs-audit/affected-docs.mjs --json 322cf9068f143f39cbfbc7527294db24f01cd649

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Reviewed-by: session_01ADHkmQxwoGb5fSnrFkpxmt
Implemented-by: session_01ADHkmSGbDCYYvHbU6am1V2
Head-sha: 1fcfe8f51292360fba6441352aa136d6424a9280
Local-runs: none
Scope: none (reviewFace on the PR's file list returns no hits: no packages/spec/src/**, no governed path, PR body and changeset carry Clause-②: no with no (narrowing) arm; served at CONTRACT_REVIEW_TIER, a tier above the owed floor, as the seat's standing order for this card asks) + packages/plugins/plugin-security/src/permission-evaluator.ts (ConditionedFieldEntry, conditionedFieldEntries, getFieldPermissions, superuserBypassSets, hasSuperuserReadBypass, hasSuperuserWriteBypass), packages/plugins/plugin-security/src/explain-engine.ts (explainObjectPass §4 fls: conditioned, binding, superseded, conditionedSentence; the record-level "Superseded by the View/Modify All Data bypass" sentence at the sharing attribution; ExplainEngineDeps.evaluator), packages/plugins/plugin-security/src/index.ts (the exports entry: PermissionEvaluator, explainAccess, ExplainEngineDeps, ExplainInput), packages/plugins/plugin-security/package.json (files, exports, no private), packages/core/src/security/effective-object-permissions.ts (holdsSuperuserBypass, superuserFieldVote, buildEffectiveFieldPermissions) and its index, packages/spec/src/security/permission.zod.ts (the readable: false beside when refinement) and packages/spec/src/security/explain.zod.ts (ExplainLayerSchema: detail: z.string(), contributors), packages/plugins/plugin-security/src/explain-engine.test.ts (makeDeps, flsOf, the [#25866] and three [#29610] cases) and security-plugin.test.ts (the [#25866] conditionedFieldEntries pin), .changeset/29610-explain-modify-all-conditioned-field.md, the card body of #29610, the claim 6590021019, the dev report 6590869175 and the seat delta for #29610.

Face: none of the four (no packages/spec/src/** path, no governed path, no narrowing; check-governed-merges --pr 29637 answers 0 of 5 paths hit, 13 surfaces governing, 132 changed lines, under 3,000). Merge-base 4d8095ba90d8855f5b1e012d378b9e48b3a5668e; net diff 5 files, 122 insertions, 10 deletions; two commits, the fix ca1b6e6aa7 and a merge of origin/main 1fcfe8f512. The PR file list equals the dev report's files_changed. main has moved 5 commits since the merge-base; the one that touches plugin-security (#29575, 4008e6290f) moves src/objects/default-permission-sets.ts and a row-scope test, neither cited here; nothing on packages/core/src/security or packages/spec/src/security. No re-merge is owed (discipline §10). PR author objectstack-fleet[bot], branch claude/project-thread-n2gs05, the one claim 6590021019 names; no relay author to name; draft; no review, no competing Contract-review-claim:. The authored commit carries the model-free trailer pair; the merge commit carries none, as a merge does.

① Derived judgments

  1. Enforcement is unchanged. Every hunk of permission-evaluator.ts read: (a) the ConditionedFieldEntry interface gains supersededBy: string[] and its docblock, a type-only change; (b) the docblock of conditionedFieldEntries gains one sentence; (c) inside conditionedFieldEntries one const supersededBy = superuserFieldVote(permissionSets, objectName)?.editable ? this.superuserBypassSets(objectName, permissionSets, { bit: 'modify' }) : [] and one supersededBy: [...supersededBy] on the pushed entry. getFieldPermissions, superuserBypassSets, hasSuperuserReadBypass, hasSuperuserWriteBypass, checkObjectPermission and resolveObjectPermission are untouched. conditionedFieldEntries has exactly two callers in the tree at this head, both in explainObjectPass §4 of explain-engine.ts (the agent's sets and, under D10, the delegator's); no gate, middleware, masker or /auth/me/permissions merge reads it, and the two functions it now calls are pure reads (holdsSuperuserBypass over objects[name] || objects['*']). The hidden/partial mask (deps.getFieldMask, intersectFieldMasks, deps.getPartialMaskRules, hidden, partial, maskNarrows) is byte-unchanged. Right: no accept set and no enforcement answer moves.
  2. One vote, no second copy of the rule. The explain engine reads only e.supersededBy; the rule itself ("a Modify All set votes editable on every field after the per-entry votes, under every when verdict") is read once, by superuserFieldVote from @objectstack/core, the same call with the same arguments that getFieldPermissions makes (superuserFieldVote(permissionSets, objectName)) and that buildEffectiveFieldPermissions makes for /auth/me/permissions. The attribution comes from superuserBypassSets(…, { bit: 'modify' }), which is holdsSuperuserBypass(ps, objectName, 'modify') per set, the very predicate superuserFieldVote returns editable: true on; so the list is non-empty exactly when the vote is editable, and the sentence can never name a set the merge did not read, nor read a vote no set is named for. This is the shape superuserBypassSets' own docblock prescribes for the record level ("the layer's contributors attribution and the enforcement decision are the same list"). No extraction was needed: the seam the delta asked for already lives in core since fix(plugin-security): a View All / Modify All set reads every field of its object, so an app field mask no longer hides fields from admins #29387, and both readers now call it. The ablation disabled the vote read in conditionedFieldEntries and the Modify All case went red, which is the right anchor for "the report reads the vote, not a restatement". Right.
  3. View All supersedes nothing here, correctly. superuserFieldVote returns { readable: true, editable: false } for a set that holds viewAllRecords only, so ?.editable is false and supersededBy is []. In getFieldPermissions that vote sets readable = true on the fields the merge holds and leaves editable as the per-entry votes left it; a conditioned entry's read bit is already true (permission.zod.ts refuses readable: false beside a when, the refinement at the end of FieldPermissionSchema), and fieldPermissionVote under unjudged grants no edit, so for a View All holder the conditioned entry's edit lock still binds per row exactly as before. The second [#29610] case pins the old sentence for an hr_auditor set with allowRead + viewAllRecords and not.toContain('superseded'). The docblock on the new member states this reason in the same words. Right, and the dev's measurement (the write gate refuses the field for the View All caller) agrees with the code.
  4. D10. The agent's entries are judged against sets and the delegator's against delegatorSets, each call computing its own supersededBy; the enforcement for a delegated call intersects the two masks, so a delegator's conditioned lock binds when only the agent holds the vote, and the report now says so (the entry lands in binding). The superseding set names in the sentence are the union over both principals' superseded entries; each entry still names its declaring set. Right.
  5. Wording. Superseded entries move into a sentence of their own, N row-conditioned field permission(s) are superseded by the Modify All Data field vote [SETS], which votes every field editable on every row, so the lock binds on no row: [...], the verb and bracketed-set form the record-level sharing attribution already uses (Superseded by the View/Modify All Data bypass [SETS] — see the vama_bypass layer.); binding entries keep the existing sentence with their own count. The delta's "annotate or drop, whichever matches how the report already words other overrides" is met by annotate, with the precedent named. One cosmetic reading, not a fix: for an entry authored editable: true with a when, the superseded sentence prints (editable when …) followed by "so the lock binds on no row"; the "lock" there is the row-blind one (no edit grant off the matched rows), which the vote does remove, so the sentence is true, only less literal for that shape. Right.
  6. The wire and the SDK are untouched. The explain report crosses HTTP as ExplainDecisionSchema (packages/spec/src/security/explain.zod.ts), whose fls layer is { layer, verdict, detail: z.string(), contributors[] }; ConditionedFieldEntry never leaves the engine, and only the prose of detail changes. No spec file, no packages/client*, no ui/** moves. Right.
  7. Where supersededBy does land: on the package's published type graph. @objectstack/plugin-security publishes (package.json has no private, files: ["dist", "README.md", "CHANGELOG.md"], exports["."] → dist/index.d.mts), and src/index.ts re-exports the PermissionEvaluator class and the ExplainEngineDeps type (evaluator: PermissionEvaluator). ConditionedFieldEntry is not re-exported by name, but it is reachable from the entry through the return type of the public method conditionedFieldEntries, and by the repo's own definition of the public surface (execution-duties: 「公开面 = 包构建后的入口声明所达 … 所达 = 再导出名,加经其 props、参数与返回类型可达的每个类型」) a type reachable through a return type is on it. The member is required (string[], never optional), so a consumer that hands explainAccess a structurally typed evaluator of its own must now return it; nothing in this repo does (the only constructor of the shape is the evaluator; makeDeps in the explain tests uses new PermissionEvaluator()), and the one toEqual pin that spells the shape is the forced edit in ③. The ruling on what this means for Clause-② is in ②.
  8. The forced test edit is honest. security-plugin.test.ts [#25866] conditionedFieldEntries lists the row-conditioned entries of the object, as authored builds makePermSet('hr', {}, {...}) with an empty objects map, so no set holds either super-user bit and supersededBy: [] is the true value of the new member for that fixture; the pin is exact toEqual, so the member had to be spelled, and nothing else in the case moves (the two conditioned entries, their set, when and editable are as before). Declared as a deviation in the report and named in the PR body with its reason. Right.
  9. Files outside the declared surface. The claim's surface is explain-engine.ts, "the shared vote seam in plugin-security if an extraction is needed", explain-engine.test.ts and a changeset. permission-evaluator.ts is where conditionedFieldEntries, the seam the explain engine reads, lives, and the change there is the seam change the claim's clause anticipates (no extraction, because the vote already lives in core; the seam is widened by one output member instead). security-plugin.test.ts is the forced edit of ①.8. No packages/spec/**, no ui/**. Accepted, both named.
  10. Tests pin the rule next to [#25866], as the delta asks, in the fls reports partial masking describe: the Modify All case (not.toContain('judged per row by the write gate'), the superseded sentence with [hr_admin], binds on no row, the entry still listed with its declaring set), the View All read-side control and the no-vote control. Red first on main 5bd615ef (1 failed, the Modify All case; the two controls pass on main by construction, which is what makes them controls), ablation red with the vote read disabled, restored to blob equality. The three cases drive the real PermissionEvaluator through explainAccess, not a stub, so the vote they exercise is core's. Right.
  11. Body declarations and trailers. Fixes #29610 first line; Clause-②: no and Metadata-protocol: none line-initial on the PR body and the changeset, copied from the claim as the standing order requires; the six-line 维护者速读; a ## Verification naming the head; the forced edit named. Right in form; the Clause-② value is ruled on in ②.

② Semver level

  • Changeset .changeset/29610-explain-modify-all-conditioned-field.md: '@objectstack/plugin-security': patch, headline fix(plugin-security): …, line-initial Clause-②: no and Metadata-protocol: none, a body that states the rule and names the new member. Check Changeset and Lint · PM dispatch-gates are success on this head. Only @objectstack/plugin-security moves in the diff and it is the one package the changeset bumps; packages/core is read, not touched.
  • The level is patch whatever the Clause-② line says, and it is right. AGENTS.md step 3: "The level follows the kind alone: a kind takes at least minor on a package the diff moves, none takes patch; a non-protocol change, widening or breaking, ships patch and keeps its BREAKING banner and ADR-0087 disposition." The kind is none (nothing a metadata author can write changes: FieldPermissionSchema, its when, the super-user bits and their meaning are as on main); no (narrowing) arm, so no banner, no adr-0087: marker and no Skills-docs: line are owed. Right.
  • Clause-② ruling: by the letter of the criterion this is yes (widening), not no. AGENTS.md's own text gives the grammar and the consequence, not the test: "Two line-initial declarations, in the PR body and the changeset body: Clause-②: yes|no plus at most one arm from the closed pair (widening)/(narrowing) ((narrowing) is BREAKING, no (widening) malformed)" — a widening is spelled yes (widening), never no. The test lives in the skill AGENTS.md delegates to: 「判据:本卡放宽接受集或扩大公开面吗」 with the public surface defined as everything the built entry declaration reaches through props, parameters and return types (①.7), and 「放宽接受集或扩大公开面的卡,不论多小,即条款②」; the seat's own standing order to the dev says the same ("A change that widens an accept set or public surface is Clause-②: yes"). A required member added to a type a published entry reaches through a public method's return type enlarges that surface, however small, and it is a new public name. The dev's ground for no ("output only; no accept set moves; nothing outside the evaluator constructs the type") is all true and is why the level stays patch and why no consumer breaks today, but the criterion is "accept set OR public surface", and 「拉回已声明契约不触它」 covers the prose (the sentence is pulled back to what ruling 6573563853 A and getFieldPermissions already declare), not the member, which no contract declared before. My ruling: Clause-②: yes (widening), Metadata-protocol: none, level patch unchanged. What that costs: the three carriers of the line (the claim 6590021019, the PR body, the changeset) are restated; the changeset edit is a one-line commit on a prose face, so this record continues to govern the moved head with a Regen-provenance: line (contract-review.md: 「纯重生成或只动散文面原记录续管」) and no second review is owed; the review the yes face owes is TIER_DEFAULT (contract-review.md: 「仅凭 Clause-②: yes 申报而欠 ⇒ TIER_DEFAULT」), which this CONTRACT_REVIEW_TIER record already exceeds. The lane does not move: plugin-security stays domain:server, and nothing here is the spec lane's.
  • The seat should know its own precedent before it rules. The fix(plugin-security): a View All / Modify All set reads every field of its object, so an app field mask no longer hides fields from admins #29387 record (this seat's, for the same ruling) accepted Clause-②: no on a diff that added two new named exports to @objectstack/core's security entry (superuserFieldVote, holdsSuperuserBypass); that record judged the authorization widening ("a runtime authorization answer, which Clause-② does not cover", which is the 「运行时权限/安全行为变更不是条款②」 negative boundary and is right) and placed the new exports under the level question ("additive on a none kind, so the rule yields patch") without ruling on them as surface. It is silent on this question, not a ruling for no. If the seat nonetheless reads Clause-② as the spec-contract and accept-set face only, so that an additive member on a plugin's return type stays no, then no is consistent with fix(plugin-security): a View All / Modify All set reads every field of its object, so an app field mask no longer hides fields from admins #29387 and this item lapses; the ACCEPT should then say so in one line, so the two records read as one position. A no left standing after the seat has agreed it is a widening is the shape landing-operations calls 「错误的 no 是可审计的假申报」, which is why I do not leave it to silence.

③ Boundary flags

  • Dev open_questions 1 (keep Clause-②: no, A; or restate yes, B). My view is B, yes (widening), for the reasons in ②, with the level unchanged at patch and the restatement a prose-only head move. The seat rules at ACCEPT (standing order item 5: a new public name is the seat's call); should-fix 1 carries the mechanics.
  • Dev open_questions 2 (annotate, A; or drop, B). A, annotate, as shipped. (1) The delta says to pick whichever matches how the report already words overrides, and the report's one existing override form is an annotation ("Superseded by the View/Modify All Data bypass [SETS] — see the vama_bypass layer"), never an omission. (2) An admin diagnosing why a declared lock does not bind needs the entry on the page with the reason beside it; dropping it would make the FLS layer disagree with the fields map the admin can read in the set itself, and "Absence must be loud" (AGENTS.md, Route & surface ownership §3) cuts the same way. (3) The four axes: measured pull is the card's own class (admins and AI authors reading os explain); long-term shape is one vote with two readers, which annotate keeps and drop keeps too; the AI-error axis prefers the report that names the superseding set over one that silently omits a declared entry; nothing new is added to any declared surface by the choice itself. No fork for the maintainer.
  • Naming, supersededBy (the seat's call). I would accept it. It is the verb the report already uses for the same vote at record level ("Superseded by …"), the value is a list of set names in the same form superuserBypassSets returns and the contributors attribution carries, it is camelCase as Prime Directive Implement ObjectStack protocol specification with Zod schemas and TypeScript interfaces #3 asks of a TS key, and it is required with [] meaning "binds", so a reader can never mistake an absent key for "not superseded". The alternatives I weighed: overriddenBy (not the report's word), supersedingSets (reads as the holder, not the relation), supersededBySets (longer, no gain). One thing the name does not say is that only the Modify All vote can populate it; the docblock says so in its first sentence, which is where that belongs.
  • Security family. The delta calls the card security-family (the card carries no security label; p2). The PR title, body, changeset and the three test names state the rule ("a Modify All set supersedes a row-conditioned lock", "the lock binds on no row"); the Verification section's collapsible measurement block describes the measurement by class (three callers, the explain route's FLS layer, the write gate's answer) with no command, route path, payload, object or set definition; the test fixtures are the ordinary leave_request.amount corpus the existing [#25866] case already uses; the report's security_findings is none. What the enforcement does for a Modify All holder is the ruled behaviour of record (6573563853 A, landed in fix(plugin-security): a View All / Modify All set reads every field of its object, so an app field mask no longer hides fields from admins #29387 and documented in content/docs/permissions/), not a bypass, and the defect is the report's prose. No reproduction detail is leaked. The commit message likewise states the rule only.
  • Deviations 2 to 4 of the report (the OS_SKIP_DTS=1 first build, redone with declarations before any reading; no MCP writes for assignee/reviewer; no PR subscription) are procedural and leave nothing in the tree beyond the 5 files. Report counts: mcp_calls: 1 (a read), api_writes: 2 (pr_create, the report comment), as the standing order asks.
  • Check-runs on head 1fcfe8f5 at 2026-11-02T23:18Z: 45 runs; 20 completed/success, 13 completed/skipped, 12 in_progress, 0 failed or cancelled. Of the ten required contexts: Build Core, Governed Surface Queue Guard, Test UI and Live E2E UI are success; Lint UI, Type Check UI and Build & E2E UI are skipped by their ui/** path filter, as expected for a diff with no ui/** path; Lint & Repo Gates has every sub-job success except Lint · repo & tooling gates, still running; TypeScript Type Check has source gates success and workspace, debt ledger, consumer gates still running; Test Core shards 1 to 8 are all still running. Every completed run is success or skipped. This record judges the tree and the completed conclusions; the all-green landing precondition is the seat's to read again before queueing. The Docs Drift Check comment is advisory and lists nothing.
  • Governed surface: none; size 132 changed lines; labels documentation, size/m, tests, tooling (the auto-labeller's; none is a pm state); draft, as the standing order requires until ACCEPT; no auto-merge.

④ Design judgment

Not owed on this face (no schema, governed or narrowing path). The one design question the card raises, whether the explain engine should derive the override from the vote or restate the rule, is answered in ①.2: it derives, from the one function both merges apply, with the attribution from the one predicate that function reads. The member is placed where the report's other overrides are attributed (on the evaluator's output, read by the engine), which is the shape superuserBypassSets' docblock set for the record level; I see no sustainable alternative that keeps one reader of the rule and still lets the layer name the sets.

VERDICT: PASS

Must-fix: none.
Should-fix (no landing blocker; the first is for the seat's ACCEPT, the rest fold into it):

  1. Clause-② line, the seat's ruling at ACCEPT. My ruling is yes (widening) (② above). If the seat adopts it: amend the claim with a line-initial Clause-②: yes (widening) (its amendment practice), have the dev restate the line on the PR body and in the changeset (Metadata-protocol: none and patch unchanged), and let this record carry the one-line changeset commit with a Regen-provenance: line, since only a prose face moves; the yes face owes TIER_DEFAULT, already exceeded here. If the seat keeps no on its fix(plugin-security): a View All / Modify All set reads every field of its object, so an app field mask no longer hides fields from admins #29387 reading, say so in the ACCEPT in one line so the two records agree; no edit is then owed.
  2. For the seat's ACCEPT, not the dev: read the check-runs again once Test Core (all eight shards), TypeScript Type Check (workspace, debt ledger, consumer gates) and Lint · repo & tooling gates complete; and record the naming ruling on supersededBy (my view: accept) and the annotate ruling (A) in the ACCEPT, as the standing order's open-question clause asks.
  3. Optional wording, no head move required: if the seat wants the superseded sentence literal for an editable: true + when entry as well, "so the lock binds on no row" could read "so the entry's row condition decides nothing for this caller"; I did not find this worth a commit.

Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt

Hand-off · seat 2 signs off

Seat 2 is signing off on the maintainer's order. This PR moves to the next domain:server seat, and card #29610 is back in pm:queue. The release note is 6591594770; it lists what is done and what is left.

  • Contract review: PASS (6591028293), no must-fix. Seat rulings: annotate (A), the name supersededBy is accepted, and Clause-②: yes (widening) (claim amendment 6591039615).
  • Owed before landing:
    • The PR body line-initial Clause-②: yes (widening), if the card thread's issue_patch did not land.
    • A check-run re-read.
    • The seat's ACCEPT, then pr_ready + automerge_enable.
  • Do not merge this PR before that ACCEPT.

Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-warren · session_01JQwxbXKRzgnVvjzxaYxLUf

维护者速读

  • 改了什么:os explain 字段级说明层把被 Modify All 字段票覆盖的带 when 字段锁单列为"已被覆盖、任何行都不生效",并点名持票权限集;执行面不变。
  • 为什么改:裁决 6573563853 字母 A 之后 Modify All 字段票在任何 when 下都投可编辑,旧说明仍称"其余处一律锁",误导排查。
  • 风险与回滚:只改说明文字与 conditionedFieldEntries 返回形状(新增 supersededBy);回滚即 revert。
  • 先看:permission-evaluator.ts 的 conditionedFieldEntries、explain-engine.ts 的 conditionedSentence。
  • 两分钟验证:pnpm --filter @objectstack/plugin-security exec vitest run src/explain-engine.test.ts -t 29610,3 条全绿。
  • 需要你做的:无,席位落地。
  • 席位意见:前任席位契约复审 PASS、裁定 annotate 与 supersededBy 命名;本席接手后重读 diff 与 head 差量,同意落地。

席位核对

ACCEPT · domain:server#2 · head 8ec800e416727581b828060a616db4ef9d45feca · 2026-11-03T00:18Z · seat review of record at CONTRACT_REVIEW_TIER by the previous holder (6591028293, PASS on 1fcfe8f5); this seat took the card over at seating (claim 6591911991) and re-read the delta

  1. Shape: draft, base main, body line 3 Fixes #29610, line-initial Clause-②: yes (widening) and Metadata-protocol: none (body and changeset agree). 5 files, +122/-10: permission-evaluator.ts (ConditionedFieldEntry.supersededBy, read from core's superuserFieldVote and attributed through superuserBypassSets, the same vote getFieldPermissions applies), explain-engine.ts (binding vs superseded sentences), three [#29610] tests, one forced toEqual edit in security-plugin.test.ts, one patch changeset. Seat read the diff: no enforcement path changes; the hidden/partial mask is untouched. Not governed (check-governed-merges --pr 29637: 0 of 5 paths), 132 lines.
  2. Premise measured: dev report 6590869175: red on main 5bd615ef (1 failed / 2 passed, the Modify All case), ablation of the vote read reproduces the red, restore clean; explain route measured before and after on a scratch boot. Delta from the reviewed head 1fcfe8f5 to 8ec800e4 = af481885 (changeset Clause-②: no → yes (widening), per claim amendment 6591039615) + a merge of origin/main; no PR-path change besides that line.
  3. Gates on this head: check-runs pinned to 8ec800e4: 52 total, 37 success, 15 skipped, 0 failed. Dev: dispatch-gates --ran 62 / 62 / 0 NOT-MEASURED on this head, plugin-security typecheck 0, tests 5206 passed.
  4. Accepted deviations: the forced toEqual edit in security-plugin.test.ts ([#25866] entry shape gains supersededBy: []); Clause-② corrected from no to yes (widening) after review (published return type gains a required member). No banner, no adr-0087 marker owed.
  5. Out of scope: lint: a permission set that declares View All or Modify All on an object beside a readable:false / editable:false entry on that object gets a finding; the mask no longer binds its own holders #29544 (lint for the super-user bit beside a mask) and spec(security): the field-permission when description and the authorization guide still say only declaring sets vote; a View All / Modify All set now votes on every field #29543 (when description wording) stay their own cards. Skills-docs: not owed (Metadata-protocol: none).
  6. Landing: not governed, under 3000 lines ⇒ the seat lands it (pr_ready + automerge_enable). At MERGED: landing record on plugin-security: os explain's FLS layer reports a row-conditioned field lock as applying "everywhere else" to a caller holding Modify All, whose vote overrides that lock since #29387 #29610 (closed by Fixes), pm:dispatched removed.

Generated by Claude Code

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

1 participant