fix(plugin-security): enforce both declared write-wideners; the platform baseline becomes explicit-allow (#5492, #5491) - #6684
Merged
Conversation
…r provenance
…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
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 14 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This was referenced Aug 8, 2026
os-zhuang
marked this pull request as ready for review
August 8, 2026 13:02
This was referenced Aug 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 holdmainmoved under the dispatch note (#6612, #6552, #6502 all touched this area in the last 24h), so every anchor was re-measured rather than trusted.mainmodifyAllRecordsnorsys_record_share.access_levelsecurity-plugin.tsstep 2.7 composesLayer0(tenant) AND Layer1(business RLS)and nothing else — no sharing probe anywhere on the path.created_byfloorowner_only_writes/owner_only_deletesinmember_default,created_by == current_user.id, applicabilitypositions: ['org_member']. A manager is an org member, so it binds them too.modifyAllRecordsis inert on ordinary business objectscomputeLayeredRlsFilter's Layer-1 superuser short-circuit is gated onposturePermits = isPrivate / tenancyDisabled / isBetterAuthManaged(logical OR); an ordinary tenant object satisfies none, so the bypass never fires there.member_defaultcarries a'*'grant union-merged into every memberobjects['*'] = {allowRead, allowCreate, allowEdit};PermissionEvaluator.checkObjectPermissionmerges most-permissively across all resolved sets, and the baseline resolves additively for every authenticated human.vama-write-path-convergence.test.tscannot show this defect"computeRlsFilterisnullfor all three. A new principal carrying the realmember_defaultshape was required, and this PR brings one.premise_still_valid: trueon 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.
ISharingServicereads all three declared wideners (ownership at write DEPTH,sys_record_share.access_level, themodifyAllRecordsbypass); 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:
updateaskscheckEdit,deleteaskscheckDelete, and "aneditshare widens update but not delete" lives in exactly one place.(object, name, using)provenance key ADR-0105 D3 uses for tenant policies (newplatform-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.modifyAllRecordsbypassing 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.hasWriteBypassalready 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.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 frommember_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_preferenceself-service, which moves from an implicit wildcard grant to an explicit entry matching thesys_user_preference_selfRLS 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 itseveryonebinding is unaffected.admin_full_access/organization_admin/viewer_readonlykeep 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/explainnamed 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.
security-plugin.tsstep 2.7) — CHANGED. As above: provenance composition,RlsFilterOptions.dropPlatformOwnershipFloor, newresolveSharingWriteVerdict.computeLayeredRlsFiltergained 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.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.resolveSharingCanEdit(ADR-0055controlled_by_parentmaster check) — VERIFIED UNAFFECTED. Still reads the two-statecanEdit, 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 intoresolveWriteScopeForSharingand 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.tsgreen, unmodified.service-storageattachment hooks (sys_attachmentparent gate) — VERIFIED UNAFFECTED. ReadscanEditthrough 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.plugin-auditcomment gate — VERIFIED UNAFFECTED. Same shape, same reasoning. 8/8 test files green, unmodified; the dogfoodsys_commentmatrix 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 carriedpermissions: []and got CRUD from the wildcard. They now carrymemberBaseline, which re-declares the removed grant byte-for-byte, identity-table carve-out included — socrm_secret's CRUD_DENY cells (a plain wildcard does not cover aprivateobject) andsys_user's write CRUD_DENY cells stay verbatim.member_defaultstill resolves additively and still contributes theowner_only_*RLS the write cells pin. Every expectation inEXPECTED_MATRIXis unchanged.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. Newtest/showcase-security.tswires the showcase's ownisDefaultprofile by NAME throughappDefaultPermissionSetName, exactly asobjectstack devdoes (the wiringshowcase-d7-default-profilealready pins). These fixtures are now more faithful to the running app than before.showcase-public-read-owdneeds two member owners of an announcement, andshowcase-static-readonlyreproduces 安全:静态字段 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-openmember_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.member_defaultwildcard-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.allowDeletestays 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 bindsmember_defaultto theeveryoneanchor, so the configured baseline must staymember_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 — realSecurityPluginmiddleware, realSharingServicelate-bound to the security service's ownhasWriteBypass, real sharing middleware, real platformmember_defaultseed — over one in-memory engine, on an ordinary tenant business object (sharingModel: 'private', noaccess.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).modifyAllRecordsmodifyAllRecordsallowDelete, so it is the row gate refusingcheckEdit→abstain, floor holds)Every refusal asserts the ADR-0112 envelope —
code: 'PERMISSION_DENIED',status: 403, and the row-level sentence verbatim — never a baretoThrow().member-default-explicit-allow.test.ts(25 cases) exercises the realcheckObjectPermissionmerge 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_preferenceself-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.
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.
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, nevergit stash(sharedrefs/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: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 carryFORBIDDEN(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, withabstainnever 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,typecheckclean. 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.typecheckclean.runtime112,rest67,cli94,service-automation70,plugin-sharing14,service-storage22,plugin-audit8,plugin-dev4,verify4 files.turbo run testover all packages — 130/130 tasks successful, exit 0.turbo run typecheck— 120/120..github/workflows/lint.ymland 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:generatedafter 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. ⛔ Nopackages/specedits (the tri-state contract landed via #6564), ⛔ noplugin-sharingedits (#5493's anchor), ⛔ nocontent/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 touchespackages/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 onspecandplugin-sharingwere observed exactly.10. Merge with current
main, and the boundary it requiredmainadvanced 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), thenpnpm install --frozen-lockfile, a full rebuild, andrm -rf packages/runtime/.objectstackbefore trusting any result.The overlap was read rather than assumed. The incoming security-surface change (#6595) migrates
PermissionSetSchema/ObjectPermissionSchema/FieldPermissionSchema/AdminScopeSchemafromz.objecttostrictObject— a strictness migration with no semantic change tomodifyAllRecords,viewAllRecordsor anyallow*bit. So:memberBaseline, the dogfood fixture baselines — is parsed through the now-strictPermissionSetSchema, so an undeclared key would newly throw. VerifiedisDefaultis 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