feat(security): fail-closed authored-row-write verdict on ISecurityService (#5493 step 1) - #6841
Merged
Merged
Conversation
…rvice (#5493 step 1) (#6735) `ISecurityService` gains an optional, verdict-shaped, by-id method `checkAuthoredRowWrite(object, recordId, operation, context)` returning `'admit' | 'abstain'`, plus `AuthoredRowWriteVerdict` / `AuthoredRowWriteOperation`. The question it answers is the one no existing surface could: does an APP-AUTHORED row-level policy admit this row for this write, on its own, with the platform's ownership floor taken out by PROVENANCE? Every other method reports the COMPOSED RLS verdict, and inside that composition sits the platform's own wildcard write floor (`created_by == current_user.id`, on the `member_default` baseline every authenticated member resolves). #5493 probe E-A measured the gap: a creator who is no longer the owner is admitted by the floor and refused by sharing with a byte-identical envelope, so a deferral keyed on the composed answer hands transferred records back to their former creators. Implementation reuses the #6684 provenance split — the SAME `computeLayeredRlsFilter` the middleware enforces with, driven by the SAME `dropPlatformOwnershipFloor` knob. No second RLS evaluator. A null Layer 1 is read as `abstain` (nothing authored is gating the write), and Layer 0 stays AND-ed in so the tenant wall holds. Fail-closed in the `abstain` direction throughout: the method is OPTIONAL, so a deployment without it behaves byte-for-byte as today; a principal-less or on-behalf-of context, an unresolvable probe and a thrown lookup all return `abstain`, and nothing throws outward. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M
…w verdict types (#6735) `check:api-surface` reported 0 breaking / 2 added — the expected additive product of declaring `AuthoredRowWriteVerdict` and `AuthoredRowWriteOperation` on the security-service contract. Regenerated with `pnpm --filter @objectstack/spec gen:api-surface`; the delta is exactly those two entries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 115 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
github-actions
Bot
added
documentation
This was referenced Aug 9, 2026
os-zhuang
marked this pull request as ready for review
August 9, 2026 01:07
This was referenced Aug 9, 2026
This was referenced Aug 9, 2026
os-zhuang
pushed a commit
that referenced
this pull request
Aug 9, 2026
…xport `resolveI18nLabel (function)` on `./ui`. 0 breaking (nothing removed or narrowed), 1 added — the delta `check:api-surface` asked for. Regenerated only after a post-rebase `pnpm --filter @objectstack/spec build`: run against the pre-rebase dist it also DROPPED `AuthoredRowWriteOperation` / `AuthoredRowWriteVerdict` from `contracts.json`, which #6841 had added to the source in the meantime — the AGENTS.md §9 stale-artefact trap, in mirror image. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M
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
2 participants
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 #6735
Part of #5493 (maintainer ruling 2026-08-08, #5493 comment 5226377985: Q1 = A, Q2 = A1). This card is the head of the chain: it lands, then #5493 step 2 (plugin-sharing consumption) dispatches.
What this adds
ISecurityServicegains an optional, verdict-shaped, by-id method, plus two named types:@objectstack/plugin-securityimplements it on the registeredsecurityservice.Why no existing surface could answer it
Every other method on this contract reports the composed RLS verdict, and sitting inside that composition is the platform's own wildcard write floor —
owner_only_writes/owner_only_deletes,created_by == current_user.id, shipped on themember_defaultbaseline every authenticated member resolves additively.So "the composed RLS admits this row" is true for the row's creator whether or not any app policy mentions the row at all. That makes it a measurably different question, not a cheaper spelling of the same one — and #5493's probe E-A is where the difference costs something: a creator who is no longer the owner (a record transferred away from them) is admitted by the platform floor and refused by sharing with a byte-identical envelope. A deferral keyed on the composed answer would hand transferred records back to their former creators.
Separating the two requires policy provenance, which is deliberately private to plugin-security (
platform-ownership-policies.ts, ADR-0105 D3 keying) — an authorable "this is a floor" flag would hand authors a switch that turns their own policy off. Hence a method on the service rather than a derivation consumers could do themselves.No second RLS evaluator
The verdict is read off the same
computeLayeredRlsFilterthe middleware enforces with, driven by the samedropPlatformOwnershipFloorknob #6684 landed for the by-id write pre-image gate. Two consequences worth naming, both load-bearing and both pinned:layer1 == nullis read asabstain, never as "admitted." Layer 1 is null exactly when no authored predicate is gating the write: the applicable set was empty, or the ADR-0066 ① posture-gated superuser short-circuit skipped business RLS wholesale. A superuser bypass is not an authored admission, and reporting it as one would re-open E-A from the other side. (The field-existence net's deny sentinel is not null, so it flows through the probe and matches nothing — alsoabstain.)Signature trade-offs (delegated to the implementer by the issue)
Optional (
checkAuthoredRowWrite?) rather than required. The counter-precedent is real and I want it on the record: #6428 declaredISharingService.checkEdit/checkDeleteas required members and had the consumer feature-detect at runtime, and all eleven existingISecurityServicemethods are required. Three measurements moved me the other way:abstainin every other case — including a missing method"). A required declaration asserts the method is always there, which contradicts the clause it is supposed to implement. Optional makes the type system agree with the ruling.typeof svc.getReadableFields === 'function'), andpackages/rest/src/rest-server.tstypes the whole service asPartial< ISecurityService >. So "required" is a promise the codebase already declines to rely on — and the repo carries a held type error in this very contract's test stub from exactly that pattern (test-typecheck-debt.json, 1 error onsecurity-service.test.ts, tracked by@ts-expect-error退役 pin 在packages/spec里是幽灵检查:tsconfig 把**/*.test.ts排除出唯一的tsc --noEmit#5286).svc.checkAuthoredRowWrite(...)unguarded compiles under a required declaration and throws at runtime against any partial implementation; that throw would then have to be caught to reachabstain. Under an optional declaration it does not compile. There is a compile-time pin for this insecurity-service.test.ts.Optional members are also an established convention on this repo's contracts for capability extensions (
ai-service,analytics-service,auth-service,automation-serviceall use them).Two states, not three. No
deny. This surface is evidence, not a gate: the caller already holds a refusal and asks only whether a declared widener speaks for the row. "No evidence" and "evidence against" are the same instruction to that caller — keep refusing — so a third state would be one nobody could act on differently.The fail direction is the inverse of
SharingWriteVerdict's, deliberately. There a failed lookup must bedeny, becauseabstainhands the decision on. Here the caller usesadmitto widen, so the answer that changes nothing isabstain. Both are the same discipline (#6564: a failure is never a widening); read the fail direction off what the caller does with the verdict, never off the state's name. The contract says so explicitly.Operation is the RLS write vocabulary (
update/delete), not the engine verb list. A caller mapspurgeontodeleteandtransfer/restoreontoupdateitself — the same mapping the engine's by-id write pre-image gate already applies before it collects policies. Keeping it on the caller's side means a new lifecycle verb cannot silently acquire a widening path here by being spelled into a wider union.Reverse verification — the pin can go red
Per the dispatch: a pin that cannot fail is not a pin. Three probes, each restored afterwards (the restore was confirmed byte-identical by diffing the working patch before and after).
1. The E-A pin goes red on a naive implementation. Flipping
dropPlatformOwnershipFloortofalse— i.e. deferring to composed RLS with the floor included:2. Removing the provenance pre-check as well — the fully naive "defer to composed RLS" shape — takes both E-A pins red:
3. Downstream really reads the new
.d.ts. Feeding a wrong-typed implementation ('allow') to the registration turned plugin-security's typecheck red against the freshly built spec dist, which is the check that the contract addition is not being read from a stale artefact:The E-A pin also carries its own control.
TRANSFERRED_UPDATE_IS_ADMITTED_BY_THE_COMPOSED_PATHdrives the real security middleware and proves the composed path admits the very row the verdict abstains on. Without it theabstainassertions would be refusing something nothing offers, which is no pin at all.Tests
packages/plugins/plugin-security/src/authored-row-write-verdict.test.ts(new, 13 cases) drives the realSecurityPlugin, the real RLS compiler and the realmember_defaultseed — the floor has to be the shipped one, because the separation under test is provenance, not shape. Covers both E-A readings (creator with a non-matching authored policy; creator with no authored policy at all), the positive admit, the verb boundary, the tenant wall, the superuser short-circuit, and the fail-closed set (throwing probe, unknown record, unknown object, principal-less context, on-behalf-of context).packages/spec/src/contracts/security-service.test.tsgains three compile-time/semantic pins: the closed verdict union, the closed operation union, optionality, and that absence andabstainare one instruction.The new fake engine's
update()/delete()open withassertEngineUpdateDispatch/assertEngineDeleteDispatchfrom@objectstack/metadata-core, copied from the pinned fake inrow-write-widener-composition.test.ts.Boundaries honoured
plugin-sharingedits — that 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 step 2.Relationship to the queued cards
checkEdit-style depth threading is not needed — the verdict is self-contained.metadata.maskObjectFieldson MetadataEndpointsConfigSchema andgetMetadataReadableFieldson ISecurityService (ADR-0106 follow-through) #6622 (getMetadataReadableFieldsonISecurityService): unchanged, and marginally easier. It touches the same two files but a disjoint region: Declaremetadata.maskObjectFieldson MetadataEndpointsConfigSchema andgetMetadataReadableFieldson ISecurityService (ADR-0106 follow-through) #6622 promotes an existingObject.assignextension (security-plugin.tsaround the registration) into the typed literal, whereas this PR adds a new member. No semantic coupling. The one thing it inherits is a decided precedent for optional-vs-required on this contract, which it can follow or argue against explicitly.Generated by Claude Code