Skip to content

fix(plugin-security): enforce both declared write-wideners; the platform baseline becomes explicit-allow (#5492, #5491) - #6684

Merged
os-zhuang merged 6 commits into
mainfrom
claude/issue-5492-write-gate-pair
Aug 8, 2026
Merged

fix(plugin-security): enforce both declared write-wideners; the platform baseline becomes explicit-allow (#5492, #5491)#6684
os-zhuang merged 6 commits into
mainfrom
claude/issue-5492-write-gate-pair

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #5492
Fixes #5491

Paired delivery, per the maintainer ruling of 2026-08-07 16:59Z: one authorization model, two halves, neither of which may land alone ("Landing either card alone strands managers or keeps the hole"). #5492 makes the two declared write-widening mechanisms actually widen; #5491 stops the platform baseline handing out what nobody declared.

1. Premises re-verified against origin/main (e6025e9) — both hold

main moved under the dispatch note (#6612, #6552, #6502 all touched this area in the last 24h), so every anchor was re-measured rather than trusted.

Issue claim Verdict on current main
#5492: the pre-image write gate consults neither modifyAllRecords nor sys_record_share.access_level Holds. security-plugin.ts step 2.7 composes Layer0(tenant) AND Layer1(business RLS) and nothing else — no sharing probe anywhere on the path.
#5492: the effective gate is the platform created_by floor Holds. owner_only_writes / owner_only_deletes in member_default, created_by == current_user.id, applicability positions: ['org_member']. A manager is an org member, so it binds them too.
#5492: modifyAllRecords is inert on ordinary business objects Holds, and the mechanism is confirmed. computeLayeredRlsFilter's Layer-1 superuser short-circuit is gated on posturePermits = isPrivate / tenancyDisabled / isBetterAuthManaged (logical OR); an ordinary tenant object satisfies none, so the bypass never fires there.
#5491: member_default carries a '*' grant union-merged into every member Holds. objects['*'] = {allowRead, allowCreate, allowEdit}; PermissionEvaluator.checkObjectPermission merges most-permissively across all resolved sets, and the baseline resolves additively for every authenticated human.
Dispatch note: "vama-write-path-convergence.test.ts cannot show this defect" Holds. None of its three permission sets authors an RLS policy, so computeRlsFilter is null for all three. A new principal carrying the real member_default shape was required, and this PR brings one.

premise_still_valid: true on both cards.

2. #5492 — the pre-image gate composes by provenance

Row-level write access was two authorities AND-ed together with no knowledge of each other. ISharingService reads all three declared wideners (ownership at write DEPTH, sys_record_share.access_level, the modifyAllRecords bypass); the security gate read only RLS — and inside that RLS sits the platform's own ownership floor, a second implementation of "ownership", the one blind to every widener. It always won.

The gate now asks the authority that owns those mechanisms for its tri-state verdict (checkEdit / checkDelete, landed by #6564) and composes by provenance:

  • allow — a positive basis exists → the declared authority replaces the platform floor;
  • abstain — record sharing does not enforce on this row (public object, no owner field, platform internal) → the floor stays; it is the only row-level write gate such rows have;
  • deny — the floor stays. The refusal belongs to the sharing middleware that produced the verdict; re-raising it here would be the duplicate implementation this change removes, and could only narrow a surface the ruling says may not shrink.

Deliberate boundaries:

  • The security side recomputes nothing. No owner, no depth, no share lookup, no bypass probe. The ADR-0111 D3 verb boundary is therefore inherited: update asks checkEdit, delete asks checkDelete, and "an edit share widens update but not delete" lives in exactly one place.
  • Only the PLATFORM's floor is replaceable. Matched by the same (object, name, using) provenance key ADR-0105 D3 uses for tenant policies (new platform-ownership-policies.ts, deriving its key set from the shipped declaration). An app policy spelling the identical predicate under its own name keeps refusing — ADR-0049, and the ADR-0105 finding F1 lesson that a token match swallows authored policies.
  • Layer 0 is untouched. The tenant wall still AND-composes in every branch.
  • This is not modifyAllRecords bypassing write-side RLS. ADR-0066 ① withholds that on an ordinary business posture and this change leaves it withheld — what changed is that the platform's own floor defers to the platform's own ownership authority.
  • The on-behalf-of path is excluded (ADR-0090 D10): hasWriteBypass already fails closed for a delegated context (ADR-0111 D2), so composing there could only resolve a verdict against the wrong identity. Both principals keep their floors.
  • Cost control: the sharing probe runs only when a platform floor policy is actually applicable for this (principal, object, operation). No floor → no probe.

Fail-closed shape: no plugin-sharing, or an implementation without the tri-state methods, → abstain (nothing consulted, nothing replaced — byte-identical to today). A probe that throws → deny, logged. Both leave the floor standing.

3. #5491 — the platform baseline narrows to explicit-allow

The '*' grant is removed from member_default. Per the ruling: object access now comes from OWDs plus profile / permission-set declarations only. Deny-precedence merge semantics were considered and rejected — sets stay additive (ADR-0090); the fix is to stop shipping a grant nobody asked for, not to invent a veto.

What the set still declares it still enforces, and nothing is newly granted: read on the better-auth identity tables (writes stay denied — that door is better-auth), and sys_user_preference self-service, which moves from an implicit wildcard grant to an explicit entry matching the sys_user_preference_self RLS carve-out the set already shipped. Effective member access there is byte-identical. Every row-level policy is untouched, and the set stays anchor-safe, so its everyone binding is unaffected. admin_full_access / organization_admin / viewer_readonly keep their wildcards — those are granted deliberately to a principal, which is exactly what the baseline was not.

The changeset carries the migration narrative as a behaviour change, not a patch note: deployments relying on the implicit wildcard lose that access, and the two supported ways to declare it back are spelled out, with security/explain named as the pre-upgrade audit tool.

4. Conclusion per authority face

Every face that can answer a per-record write question, stated explicitly — an unmentioned face reads as a missed face.

  1. plugin-security pre-image gate (security-plugin.ts step 2.7) — CHANGED. As above: provenance composition, RlsFilterOptions.dropPlatformOwnershipFloor, new resolveSharingWriteVerdict. computeLayeredRlsFilter gained one optional filter step; every other caller (reads, getReadFilter, the explain engine, the write-check/post-image paths) passes no options and is byte-identical.
  2. Sharing middleware (plugin-sharing) — OUT OF SCOPE, deliberately, and safe. Not touched (it is On objects that carry sharing rules, sharing middleware answers FORBIDDEN before RLS update-wideners are consulted — the identical widener works on rule-less objects #5493's anchor, serialized after this pair). Safe because this change only ever widens the security gate: any row the sharing middleware refused before, it still refuses, and it runs after the security gate on the same request. Measured, not assumed — see the On objects that carry sharing rules, sharing middleware answers FORBIDDEN before RLS update-wideners are consulted — the identical widener works on rule-less objects #5493 control in §6.
  3. resolveSharingCanEdit (ADR-0055 controlled_by_parent master check) — VERIFIED UNAFFECTED. Still reads the two-state canEdit, which the feat(sharing): ISharingService 的每行写判定补三态(放行/不表态/拒绝)(#6428) #6564 contract says is correct for a caller that only ADDS a gate; its truth table is byte-for-byte unchanged. Its inline depth-stash was extracted into resolveWriteScopeForSharing and is now shared with the new tri-state probe — same inputs, same fallbacks, so the two forms of one question cannot drift. controlled-by-parent-sharing.test.ts green, unmodified.
  4. service-storage attachment hooks (sys_attachment parent gate) — VERIFIED UNAFFECTED. Reads canEdit through its own structurally-typed late binding; not touched, no contract change. 22/22 test files green, unmodified. Its permission matrix is covered end-to-end by the dogfood attachments matrix, also green.
  5. plugin-audit comment gate — VERIFIED UNAFFECTED. Same shape, same reasoning. 8/8 test files green, unmodified; the dogfood sys_comment matrix green.

5. Fixture migration (#5491's blast radius)

Removing a grant every fixture silently inherited is the migration story. Nothing was batch re-spelled; each case was triaged on its own.

  • authz-matrix-gate.test.ts — add declarations. Its two member roles carried permissions: [] and got CRUD from the wildcard. They now carry memberBaseline, which re-declares the removed grant byte-for-byte, identity-table carve-out included — so crm_secret's CRUD_DENY cells (a plain wildcard does not cover a private object) and sys_user's write CRUD_DENY cells stay verbatim. member_default still resolves additively and still contributes the owner_only_* RLS the write cells pin. Every expectation in EXPECTED_MATRIX is unchanged.
  • Dogfood showcase fixtures — boot the app the way the CLI boots it. A vanilla bootStack(showcaseStack) used to mean "a member governed by the platform wildcard". Post-ruling it means "a member governed by a deployment that declared no default profile" — i.e. no access at all. New test/showcase-security.ts wires the showcase's own isDefault profile by NAME through appDefaultPermissionSetName, exactly as objectstack dev does (the wiring showcase-d7-default-profile already pins). These fixtures are now more faithful to the running app than before.
  • Two named scaffolding grants, not a wildcard. showcase-public-read-owd needs two member owners of an announcement, and showcase-static-readonly reproduces 安全:静态字段 readonly: true 仅 UI 层生效、服务端不强制 → 审批/状态/金额字段可被直接 PATCH 绕过写入(FLS 缺口) #3003, whose entire subject is "a logged-in non-admin forges a readonly column" — handing it an admin token would delete the property it pins. Both get named object grants on top of the app default; a wildcard here would re-open member_default's * wildcard object grant (C/R/E) union-merges into every org member — app-side explicit-allow object gates are erased on three axes #5491 under a different name.
  • Attachment / comment permission matrices — declare the posture they question. Their headers said in as many words that they relied on "the real member_default wildcard-CRUD set", because the gates under test are the ones layered on top of a wildcard that by itself scopes nothing. Each fixture now declares that permissive baseline itself. allowDelete stays absent — the contrast with the domain set that adds it is half of what those matrices measure.
  • me-apps-and-everyone-baseline — kept vanilla, one case reworked. Its Builtin member_default carries anchor-forbidden bits — every boot logs 'refusing to bind fallback set to everyone' (platform baseline violates its own D5 tier) #2753 assertion is precisely that the bootstrap binds member_default to the everyone anchor, so the configured baseline must stay member_default; the boot is untouched. The delete-bit case can no longer borrow its create right from the wildcard, so it now declares a create-only set and binds it to the member the same way that file's tab probes bind theirs. The delete assertion is unchanged and still ADR-0090 D5's property — and the case can no longer be accused of passing because some wildcard granted create.

No denial assertion anywhere was relaxed or rewritten.

6. Test evidence, per axis the ruling names

New: row-write-widener-composition.test.ts (11 cases) drives the REAL stack — real SecurityPlugin middleware, real SharingService late-bound to the security service's own hasWriteBypass, real sharing middleware, real platform member_default seed — over one in-memory engine, on an ordinary tenant business object (sharingModel: 'private', no access.default, the shape HotCRM ships and the one that withholds the ADR-0066 ① short-circuit). Both write verbs open with the producer's own dispatch predicate (assertEngineUpdateDispatch / assertEngineDeleteDispatch).

Ruling requirement Case Result
cross-owner UPDATE under modifyAllRecords manager PATCHes another owner's row passes, row really changes
cross-owner DELETE under modifyAllRecords manager deletes it passes, row really gone
PATCH by an edit-share target succeeds share target updates the shared record passes
edit share does NOT confer delete (ADR-0111 D3) same target, DELETE refused; profile does hold allowDelete, so it is the row gate refusing
a read-share target must still be denied writes read-share target PATCHes refused
explicit deny still effective unrelated member, no share, no bypass refused
positive probe: profile grants still work member updates their OWN row passes
#5492 E2 regression pin owner-less object, cross-creator UPDATE by a member refused (checkEditabstain, floor holds)

Every refusal asserts the ADR-0112 envelopecode: 'PERMISSION_DENIED', status: 403, and the row-level sentence verbatim — never a bare toThrow().

member-default-explicit-allow.test.ts (25 cases) exercises the real checkObjectPermission merge over the real seed: the baseline alone grants nothing on an app object on any axis; an explicit all-false profile stays denied after the baseline is union-merged in (the exact merge that produced "granted by [member_default]"); a granting profile still works on all four axes; delete is unchanged; identity tables stay readable and write-denied; sys_user_preference self-service survives as an explicit grant; the RLS policies and anchor-safety are intact; the admin sets keep their wildcards.

platform-ownership-policies.test.ts (5 cases) pins the provenance derivation is non-vacuous, names exactly the two shipped write-class floor policies, and refuses to recognise an app-authored policy spelling the same predicate under its own name or on its own object.

Reverse verification — direction predicted before running, both halves

#5492 — predicted: reverting the pre-image composition turns exactly the three widening cases red, and every guard case stays green.

Tests  3 failed | 7 passed (10)
  x cross-owner UPDATE succeeds …
    [Security] Access denied: not permitted to update this 'crm_opportunity' record (row-level security)
  x cross-owner DELETE succeeds …
    [Security] Access denied: not permitted to delete this 'crm_opportunity' record (row-level security)
  x PATCH by the edit-share target succeeds …
    [Security] Access denied: not permitted to update this 'crm_opportunity' record (row-level security)

Exactly as predicted, and the red messages are the issue's own denial string, character for character. The four guard cases (read-share refused, edit-share delete refused, unrelated member refused, own record allowed) and the two E2 floor cases stayed green — i.e. the fix widens and only widens.

#5491 — predicted: restoring the wildcard turns the explicit-allow pins red and nothing else.

Tests  9 failed | 811 passed (820)
  all 9 in member-default-explicit-allow.test.ts:
  x has no `*` key at all …
  x every entry it does ship NAMES its object …
  x read / create / edit: the baseline ALONE grants nothing on an app object
  x read / create / edit: an explicit all-false profile stays denied …
  x an object no set mentions is denied on every axis …

The three delete-axis cases stayed green — delete was already profile-driven, which is why the ruling names only three axes. The migrated matrix and the composition suite stayed green throughout, confirming the fixture migration is faithful rather than compensating.

⛔ Both reversals used git checkout REF -- PATH, never git stash (shared refs/stash, objectui#3430).

7. Mandatory question — what does this do to #5493?

Unaffected. Measured, not reasoned.

#5493 is this composition's mirror image: there the RLS layer admits and the sharing middleware answers FORBIDDEN first. The new suite carries a dedicated control for it — a profile with an app-authored RLS update-widener (stage == 'prospecting'), which OR-combines past the platform ownership floor:

checkEdit -> 'deny'
security pre-image gate -> ADMITS (the authored widener matched)
write -> refused anyway, by the other authority:
  code FORBIDDEN — "insufficient privileges to update crm_opportunity opp_theirs"

Same authority, same code, same message as on main. The control asserts both directions — the refusal message must NOT contain (row-level security) (so the pre-image gate really did admit) and it must positively carry FORBIDDEN (so the case cannot pass by refusing for some third reason).

So #5493 is neither fixed nor made harder. It is arguably made simpler to price: this PR establishes the composition shape its fix will mirror — one authority owns the widening decision, the other defers by provenance instead of running as an independent AND — and it lands the tri-state consumption pattern (allow / abstain / deny, with abstain never read as permission) that the sharing side will need in reverse. It also removes any ambiguity about which gate is under discussion: on by-id writes the security gate now defers, so a FORBIDDEN on an object with sharing rules is unambiguously the middleware's.

8. Verification

Re-run in full after merging current origin/main (see §10) — every number below is from the merged tree.

  • @objectstack/plugin-security: 40 files / 821 tests green, typecheck clean. The three new suites alone: 41 cases green (11 composition incl. the On objects that carry sharing rules, sharing middleware answers FORBIDDEN before RLS update-wideners are consulted — the identical widener works on rule-less objects #5493 control, 25 explicit-allow, 5 provenance).
  • @objectstack/dogfood: 85 files / 520 tests green, 1 skipped — down from 13 files failing before the fixture migration. typecheck clean.
  • Every direct dependent green and unmodified: runtime 112, rest 67, cli 94, service-automation 70, plugin-sharing 14, service-storage 22, plugin-audit 8, plugin-dev 4, verify 4 files.
  • Full workspace: turbo run test over all packages — 130/130 tasks successful, exit 0. turbo run typecheck120/120.
  • Gate list enumerated from .github/workflows/lint.yml and run one by one, not from memory. All 31 ESLint-job gates (lint, slot-lookup, query-options-erasure, verify-stand-in, nul-bytes, doc-authoring, docs-audit-scope, role-word, quick-reference-counts, adr-anchors, org-identifier, authz-resolver, service-providers, route-envelope, error-code-casing, wildcard-fallthrough, meta-type-normalized, init-service-contract, durability-log-level, startup-registry-verdict, objectui-changeset, release-notes, release-body, node-version, workflow-status-functions, shard-attestation, published-files, engine-double-contract, resume-authority-declared, merge-driver, spec-parsed-alias) and every Type-Check-job gate (type-check-coverage, driver-conformance, stall-guard, type-check-debt, the twelve spec-scoped generated/surface gates, skill-frame-sync, skill-compatibility, i18n, i18n-coverage, doc-formula-expressions, examples + downstream-contract typecheck) — all PASS, zero failures.
  • pnpm --filter @objectstack/spec check:generated after the merge: all 10 generated artifacts up to date (no regeneration debt from the merge driver).

9. Scope

Production code touched: packages/plugins/plugin-security/** only, plus two changesets. ⛔ No packages/spec edits (the tri-state contract landed via #6564), ⛔ no plugin-sharing edits (#5493's anchor), ⛔ no content/docs/releases/ edits.

One boundary note, declared rather than buried: the dispatch named the file surface as packages/plugins/plugin-security/** + .changeset/, and the fixture migration in §5 necessarily touches packages/qa/dogfood/test/**. That is test-fixture migration, which the same dispatch anticipated in as many words ("fixtures that relied on the implicit wildcard get explicit grants — that is the migration story"); the hard prohibitions on spec and plugin-sharing were observed exactly.

10. Merge with current main, and the boundary it required

main advanced by ~12 commits while this was in flight, two of which touch the same surface area, so AGENTS.md §10 applies rather than a clean-merge assumption. git merge origin/main (⛔ no rebase, no force-push), then pnpm install --frozen-lockfile, a full rebuild, and rm -rf packages/runtime/.objectstack before trusting any result.

The overlap was read rather than assumed. The incoming security-surface change (#6595) migrates PermissionSetSchema / ObjectPermissionSchema / FieldPermissionSchema / AdminScopeSchema from z.object to strictObject — a strictness migration with no semantic change to modifyAllRecords, viewAllRecords or any allow* bit. So:

  • Neither half of this PR is covered by it. No diff was trimmed, and none was manufactured to look like coverage.
  • But it is exactly the §10 "individually fine, jointly wrong" class: every permission set this PR adds — the seed edit, the matrix memberBaseline, the dogfood fixture baselines — is parsed through the now-strict PermissionSetSchema, so an undeclared key would newly throw. Verified isDefault is a declared key and re-ran everything on the merged tree; the numbers in §8 are all post-merge.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ


Generated by Claude Code

claude added 5 commits August 8, 2026 09:04
…is explicit-allow (#5492, #5491)

The row-level write pre-image gate now composes by provenance, delegating the
widening decision to ISharingService's tri-state verdict (#6564): `allow`
replaces the platform `created_by` ownership floor, `abstain` and `deny` leave
it standing. Layer 0 and every app-authored RLS policy are untouched.

`member_default` drops its `*` object grant: the platform baseline narrows to
explicit-allow, so an app's declared object gate is no longer erased by a
union-merged wildcard.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ
@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 8, 2026 12:49pm

Request Review

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-security, @objectstack/dogfood.

14 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx (via @objectstack/plugin-security)
  • content/docs/kernel/runtime-services/sharing-service.mdx (via @objectstack/plugin-security)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/plugin-security)
  • content/docs/permissions/access-recipes.mdx (via packages/plugins/plugin-security)
  • content/docs/permissions/authorization.mdx (via @objectstack/plugin-security, packages/qa/dogfood)
  • content/docs/permissions/delegated-administration.mdx (via packages/qa/dogfood)
  • content/docs/permissions/explain.mdx (via @objectstack/plugin-security)
  • content/docs/permissions/permissions-matrix.mdx (via packages/plugins/plugin-security)
  • content/docs/permissions/sharing-rules.mdx (via @objectstack/plugin-security)
  • content/docs/plugins/index.mdx (via @objectstack/plugin-security)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-security)
  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-security)
  • content/docs/ui/audience-based-interfaces.mdx (via packages/plugins/plugin-security)
  • content/docs/ui/dashboards.mdx (via @objectstack/plugin-security)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 8, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 8, 2026 13:02
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 8, 2026
Merged via the queue into main with commit 9e9445b Aug 8, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-5492-write-gate-pair branch August 8, 2026 13:24
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/xl tests tooling

Projects

None yet

2 participants