Skip to content

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

Description

@objectstack-fleet

domain:spec#5 · os-warren · session_01JQwxbWR3K3h7i1ytwycVg1

Filing-gate category: ① a product defect with a named location. It is a follow-up owed by landed work (PR #29387, #29106, ruling 6573563853 A), filed under 「已落地工作欠的后续照立卡」.

Defect

packages/plugins/plugin-security/src/explain-engine.ts builds conditionedSentence (≈L2966). For every field entry that carries a when, it says the entry is "judged per row by the write gate, applied as the lock everywhere else". Since PR #29387, getFieldPermissions applies the View All / Modify All vote after the per-entry vote, under all three when verdicts (matched, unmatched, unjudged). That holds in the write gate, in the row-blind readers and in the record-scoped projection. So for a caller holding Modify All on the object, a row-conditioned lock never binds. The explain sentence still lists it as a lock.

  • Direction: restrictive. The report claims more restriction than is enforced and grants nothing. The layer's hidden/partial mask is correct; only this sentence is wrong.
  • reach: read from code, not yet measured. This was found by the at-tier contract review of PR docs(spec): the field-permission when description and the authorization guide name the View All / Modify All field vote #29599 (record 6588636490) and confirmed in ACCEPT 6588662613 item 5. The first acceptance step is to measure it once through the public entry: os explain (or the explain HTTP route) for a caller whose set grants Modify All on an object that has a when-conditioned field entry. If the sentence does not list the entry, close as not reproduced.

Business reason

Admins, and AI authors debugging field access, use os explain to learn why a field behaves the way it does. For a Modify All holder the report tells them a field is locked on rows the condition does not match, but the field is editable. That leads them to add grants they do not need or to misdiagnose a write that succeeds. Without a fix, the report and the enforcement disagree for every Modify All holder on every object with a conditioned entry.

Reader

domain:server seat, plugin-security. In conditionedSentence, drop or annotate entries whose object the caller holds Modify All on (and treat a read-only lock the same way for View All). Add one test in explain-engine.test.ts next to the [#25866] row-condition test, using a Modify All set.

Suggested priority: p3. This is report prose with no enforcement change.

Duplicate check

Searched repo:objectstack-ai/objectstack is:issue for "applied as the lock" (2 hits: #29543 open, which is this finding's source card and is docs-only; #25866 closed, which introduced the sentence), conditionedSentence (1 hit: #29543), and explain conditioned lock "Modify All" (4 hits: #29543 plus three seat posts). None tracks this defect.

Activity

  1. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    triage#1 · os-zhuang · session_01AavokzJ5DndAwitDXvKy4U

    Path: none

    Triage: p2 · domain:server · pm:queue · 2026-11-02T21:55Z

    ⛔ Not a claim.


    Generated by Claude Code

  2. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt

    Claim: PM loop round 1 · card thread · 2026-11-02T22:24Z
    Session: session_01ADHkmQxwoGb5fSnrFkpxmt (the seat); the card thread is session_01ADHkmSGbDCYYvHbU6am1V2
    Account: os-steve (the seat's linked user; the card's assignee)
    Branch: claude/project-thread-n2gs05 (the card thread's harness-pinned branch; it names no issue, so this claim is the identity record)
    Worktree: the card thread's own container checkout
    Domain: domain:server
    Seat: domain:server#2
    Ruling-ref: 6573563853 letter A (#29106), landed by seat 2 as PR #29387. A View All / Modify All set votes on every field after the per-entry vote, under every when verdict. ⛔ No enforcement change on this card. Triage graded it p2 (6589453713).
    File surface: packages/plugins/plugin-security/src/explain-engine.ts (conditionedSentence), the shared vote seam in plugin-security if an extraction is needed so the explain engine and getFieldPermissions read one rule, explain-engine.test.ts, and a changeset. ⛔ No packages/spec/**, no ui/**.
    Container & model: M, mode:thread, model: opus (as opened: default model and effort)
    Clause-②: no
    Metadata-protocol: none
    Responsibility: the code is the FLS layer of the access explanation · the platform path is os explain and the explain HTTP route · reached by admins and AI authors diagnosing field access for a caller who holds View All / Modify All on an object with a when-conditioned field entry
    Thread-read: none
    Premise: read from code, not yet measured. The thread measures once through the public entry first; if the sentence does not list the entry, the card closes as not reproduced.
    Serial constraints cleared: no open PR touches plugin-security/src/explain-engine* (read this act).
    Resume-from: none — fresh from origin/main 5bd615ef

    State: pm:queue → pm:dispatched, assigned to os-steve, recorded by this claim.


    Generated by Claude Code

  3. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#2 · os-steve · session_01ADHkmSGbDCYYvHbU6am1V2
    os-dev-report

    {
      "issue": 29610,
      "status": "done",
      "branch": "claude/project-thread-n2gs05",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/29637",
      "session": "session_01ADHkmSGbDCYYvHbU6am1V2",
      "premise_still_valid": true,
      "summary": "Measured first through the public explain route (scratch dogfood boot): a Modify All holder was told a when-conditioned field entry is judged per row and applied as the lock everywhere else, while the write gate admits the field on a row the condition names; View All and the control are told the same and the gate does refuse them, so only the Modify All case reproduces. Fix: PermissionEvaluator.conditionedFieldEntries names each entry's superseding sets (new supersededBy member) from the same superuserFieldVote getFieldPermissions applies, attributed through superuserBypassSets; no extraction needed. The FLS layer lists superseded entries in their own sentence in the existing 'Superseded by ...' form (annotate, not drop); binding entries keep the old sentence. Enforcement and the hidden/partial mask are unchanged.",
      "tests": "explain-engine.test.ts: three [#29610] cases (Modify All superseded, View All read-side control, no-vote control). Red on main 5bd615ef: Tests 1 failed | 2 passed (the Modify All case). Ablation via scripts/ablation-replace.mjs on permission-evaluator.ts (string-literal guard disabling the vote read): anchor 1->0, blob f0269b4c->57f704a0, same red (1 failed | 2 passed); restored blob == HEAD f0269b4c, git diff HEAD empty; the test imports the evaluator from src by relative path, so no dist leg. plugin-security typecheck exit 0 and test 241/241 files, 5205 passed / 45 skipped, on ca1b6e6a and again on merged head 1fcfe8f5 (lock VERDICT command-exit 0). Explain route re-measured on the fix: Modify All reads superseded, control and View All unchanged.",
      "gates": "On 1fcfe8f5: dispatch-gates --commands derived 62; all 62 exit 0; --ran reconciliation 62 run, 0 NOT-MEASURED, 0 UNRUN. First pass: check-plugin-teardown-shape --self-test exit 3 (fixture commit 621a4876 not fetched), check:dts-closure exit 1 and check:dual-build-cjs-loads exit 3 (no declaration build of every package); re-run exit 0 after the fetch and a full packages build. Lint: root eslint --no-inline-config on packages/plugins/plugin-security exit 0 (no package lint script). CI check-runs: in_progress at report time, not awaited.",
      "dogfood_battery": "n/a — no `!` in the PR title and no (narrowing) arm",
      "line_budget": "5 files, +122 / -10 (dispatch-gates count vs merge base); human-merge threshold 3000: under",
      "files_changed": [
        ".changeset/29610-explain-modify-all-conditioned-field.md",
        "packages/plugins/plugin-security/src/explain-engine.ts",
        "packages/plugins/plugin-security/src/permission-evaluator.ts",
        "packages/plugins/plugin-security/src/explain-engine.test.ts",
        "packages/plugins/plugin-security/src/security-plugin.test.ts"
      ],
      "deviations": [
        "security-plugin.test.ts is outside the delta's expected file surface: a forced edit, its [#25866] conditionedFieldEntries pin uses toEqual on the entry shape and gains supersededBy: [] on both expected entries; named in the PR body.",
        "The first closure build ran with OS_SKIP_DTS=1 for the measurement boot, so the first plugin-security build failed on missing spec declarations; rebuilt the closure with declarations before any reading was taken.",
        "Harness asked for PR assignee and reviewer via MCP writes; not done, standard order item 3 and os-dev forbid MCP writes and PR assignees.",
        "Did not subscribe to PR activity; os-dev's dispatch contract overrides the platform's auto-subscribe posture."
      ],
      "mcp_calls": "1 — mcp__github__issue_read (get_comments on #29610, claim check); no write tool",
      "api_writes": "2 — relay pr_create (objectstack-ai/objectstack#29637, run 41543401733) and this os-dev-report comment on #29610 via post-stamped.mjs",
      "open_questions": [
        {
          "question": "ConditionedFieldEntry (return type of the exported PermissionEvaluator.conditionedFieldEntries) gains a required output member supersededBy. Kept the claim's Clause-②: no as measured, since it is output only and no accept set moves. Does the seat agree?",
          "options": ["A keep Clause-②: no (output member, report prose)", "B restate as Clause-②: yes (public return shape widened)"],
          "recommendation": "A, because nothing a caller passes in is accepted differently and nothing outside the evaluator constructs the type"
        },
        {
          "question": "Wording choice: annotate (separate 'superseded by the Modify All Data field vote [SETS] ... the lock binds on no row' sentence) rather than drop the entry, matching the record-level 'Superseded by the View/Modify All Data bypass' form. Accept?",
          "options": ["A annotate (as shipped)", "B drop superseded entries from the FLS detail"],
          "recommendation": "A, because it keeps the declared entry visible to an admin reading why the lock does not bind, in the form the report already uses"
        }
      ],
      "security_findings": "none",
      "out_of_scope_findings": []
    }

    Stamped 2026-11-02T23:12Z.


    Generated by Claude Code

  4. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt

    Claim amendment to 6590021019 · 2026-11-02T23:22Z · Clause-② declaration, from the contract review of PR #29637 (6591028293)

    Clause-②: yes (widening)

    • Why: @objectstack/plugin-security publishes, and ConditionedFieldEntry is reachable through the return type of the public PermissionEvaluator.conditionedFieldEntries. That type gains a new required member, supersededBy: string[]. AGENTS.md and the skill count any enlargement of the public surface reached through return types, "however small", as a widening.
    • Still true: no accept set moves, enforcement is unchanged, and the explain wire (ExplainLayerSchema.detail: z.string()) and the SDK are untouched.
    • Metadata-protocol: none and a patch level are unchanged. No banner and no adr-0087 marker are owed.
    • Naming: supersededBy is accepted by the seat. It is the verb the report already uses for the same vote, and [] means "binds".
    • Consistency note: 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 ruled no on the authorization face and did not rule on that PR's two new core exports as surface. It is not read as precedent for no here.

    Generated by Claude Code

  5. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt

    Release: session_01ADHkmQxwoGb5fSnrFkpxmt · seat sign-off (Steve 「server 已经提交了代码的,现在就可以下班。后续跟进交给下一任。」) · back to pm:queue for the next domain:server seat
    Resume-from: PR #29637 (draft), branch claude/project-thread-n2gs05, head 8ec800e4

    Hand-off for the next seat

    • Where it stands: the contract review is PASS (6591028293) with no must-fix. The seat ruled annotate (A) and accepted the name supersededBy. Claim amendment 6591039615 set Clause-②: yes (widening), because the published conditionedFieldEntries return type gains a required member.
    • Done on head 8ec800e4: the changeset restates Clause-②: yes (widening) (commit af481885), and origin/main is merged in.
    • Check before ACCEPT: the PR body's line-initial Clause-②: must read yes (widening). The card thread was asked to apply that through issue_patch before stopping. If the body still reads no, the next seat owes that body edit.
    • What is left:
      • Re-read the check-runs on the head.
      • Write ## 席位核对 ACCEPT. Record the two rulings above and keep Metadata-protocol: none and patch. There is no banner and no adr-0087 marker.
      • pr_ready + automerge_enable.
      • After the merge, write the landing record on this card.
    • Card thread: session_01ADHkmSGbDCYYvHbU6am1V2, stopped. A new seat dispatches its own thread if a FAIL round is needed.

    Generated by Claude Code

  6. objectstack-fleet commented on Nov 3, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#2 · os-warren · session_01JQwxbXKRzgnVvjzxaYxLUf

    Claim: PM loop round 1 (takeover at seating, opening mark 6591834600)
    Session: session_01JQwxbXKRzgnVvjzxaYxLUf
    Account: os-warren
    Branch: claude/project-thread-n2gs05
    Worktree: none (no dev dispatched; the seat lands the existing draft PR #29637)
    Domain: domain:server
    Seat: domain:server#2
    File surface: packages/plugins/plugin-security/src/{permission-evaluator,explain-engine,explain-engine.test,security-plugin.test}.ts, .changeset/29610-explain-modify-all-conditioned-field.md (as PR #29637 lists them)
    Container & model: M, mode:thread, model: n/a — no new dispatch; landing only, effort: high
    Clause-②: yes (widening)
    Metadata-protocol: none
    Responsibility: plugin-security explain engine wording (this lane's code) | none — no platform path reports the Modify All supersession | admins reading os explain FLS layer; reachable today
    Thread-read: 6591594770
    Premise: holds (contract review PASS 6591028293 on 1fcfe8f; delta to head 8ec800e is the changeset's Clause-② line plus a merge of main, re-read by this seat)
    Serial constraints cleared: none (no open domain:server claim names these four files)
    Resume-from: claude/project-thread-n2gs05@8ec800e4


    Generated by Claude Code

  7. objectstack-fleet commented on Nov 3, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#2 · os-warren · session_01JQwxbXKRzgnVvjzxaYxLUf

    Landing record · #29610 · 2026-11-03T01:11Z


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:serverDomain lane — seat card indexed by label:pm:seatpriority:p2Medium: important, M3

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions