diff --git a/.changeset/22464-security-service-controlled-by-parent-write.md b/.changeset/22464-security-service-controlled-by-parent-write.md new file mode 100644 index 00000000000..a04a97c325b --- /dev/null +++ b/.changeset/22464-security-service-controlled-by-parent-write.md @@ -0,0 +1,13 @@ +--- +'@objectstack/spec': minor +--- + +`ISecurityService` declares an optional, feature-detected member that answers the existing ADR-0055 master-detail write check: `checkControlledByParentWrite(object, recordId, context)`. + +Clause-②: yes (widening) + +- **What it answers.** It answers what the master-detail write check decides for an update of one record by the caller. A by-id write to a `controlled_by_parent` record requires edit access to its master, and the write path checks each master up the chain: the object-level `update` grant, the master's write row-level security, and record sharing. The answer equals what a by-id update of the same record gets from that check, the ADR-0090 D10 delegator leg included. It adds no verdict: it exposes the one the write path already reaches, so a gate on a record's attachments or comments can judge the record as its own update is judged. +- **What it resolves with.** `ControlledByParentWriteOutcome` is a discriminated union. `allow` means the check does not refuse. `deny` names the leg that refused (`ControlledByParentWriteDenialLeg`: `object_permission`, `row_level_security`, `record_sharing` or `master_chain`). `not_applicable` means the object is not `controlled_by_parent`. `unresolvable` names why the check reached no verdict (`ControlledByParentWriteUnresolvedReason`: a missing `master_detail` relation, a record that does not exist, or an empty master reference). A store fault is not an outcome: the member rejects with the engine's own error, so the fault keeps its declared status (a datasource outage is `503`). +- **Optional, and absence is typed.** A security service without the member still satisfies the contract, and an unguarded call does not compile. Absence means no master check is composed for a consumer to ask, which is not a policy that admits. + +Nothing an author writes changes, and no runtime behaviour changes. `@objectstack/plugin-security` does not serve the member yet. An implementation typed as `ISecurityService` that serves this name must serve it under the declared signature. diff --git a/packages/plugins/plugin-security/src/registered-security-service-members.pin.test.ts b/packages/plugins/plugin-security/src/registered-security-service-members.pin.test.ts index 315b2e0d56f..c41c8bec13d 100644 --- a/packages/plugins/plugin-security/src/registered-security-service-members.pin.test.ts +++ b/packages/plugins/plugin-security/src/registered-security-service-members.pin.test.ts @@ -68,6 +68,7 @@ const DECLARED_MEMBERS = { resolveWriteScope: 'required', describeDelegationNarrowing: 'optional', checkAuthoredRowWrite: 'optional', + checkControlledByParentWrite: 'optional', explain: 'required', describeDelegableScope: 'required', listAudienceBindingSuggestions: 'required', diff --git a/packages/spec/api-surface/contracts.json b/packages/spec/api-surface/contracts.json index 9830b1eb56b..bcec4d90b5f 100644 --- a/packages/spec/api-surface/contracts.json +++ b/packages/spec/api-surface/contracts.json @@ -64,6 +64,9 @@ "ConnectorSourcePullRequest (interface)", "ConnectorSourcePullResult (interface)", "ConnectorSourcePullSummary (interface)", + "ControlledByParentWriteDenialLeg (type)", + "ControlledByParentWriteOutcome (type)", + "ControlledByParentWriteUnresolvedReason (type)", "CoreServiceContract (type)", "CoreServiceContracts (interface)", "CounterIncrOptions (interface)", diff --git a/packages/spec/export-origins/contracts.json b/packages/spec/export-origins/contracts.json index 910f1feede4..533eb49dd30 100644 --- a/packages/spec/export-origins/contracts.json +++ b/packages/spec/export-origins/contracts.json @@ -64,6 +64,9 @@ "ConnectorSourcePullRequest": "src/contracts/automation-service.ts#ConnectorSourcePullRequest (interface)", "ConnectorSourcePullResult": "src/contracts/automation-service.ts#ConnectorSourcePullResult (interface)", "ConnectorSourcePullSummary": "src/contracts/automation-service.ts#ConnectorSourcePullSummary (interface)", + "ControlledByParentWriteDenialLeg": "src/contracts/security-service.ts#ControlledByParentWriteDenialLeg (type)", + "ControlledByParentWriteOutcome": "src/contracts/security-service.ts#ControlledByParentWriteOutcome (type)", + "ControlledByParentWriteUnresolvedReason": "src/contracts/security-service.ts#ControlledByParentWriteUnresolvedReason (type)", "CoreServiceContract": "src/contracts/core-service-contracts.ts#CoreServiceContract (type)", "CoreServiceContracts": "src/contracts/core-service-contracts.ts#CoreServiceContracts (interface)", "CounterIncrOptions": "src/contracts/cluster-service.ts#CounterIncrOptions (interface)", diff --git a/packages/spec/src/contracts/security-service.test.ts b/packages/spec/src/contracts/security-service.test.ts index 07ef229b301..5ede5c1a8a5 100644 --- a/packages/spec/src/contracts/security-service.test.ts +++ b/packages/spec/src/contracts/security-service.test.ts @@ -6,6 +6,9 @@ import type { AuthoredRowWriteVerdict, AuthoredRowWriteOperation, OwnershipFloorAlternate, + ControlledByParentWriteOutcome, + ControlledByParentWriteDenialLeg, + ControlledByParentWriteUnresolvedReason, } from './security-service'; /** @@ -642,4 +645,134 @@ describe('Security Service Contract', () => { }; expect(notOneLimb.operation).toBe('all'); }); + + it('checkControlledByParentWrite is OPTIONAL — absence is typed, and it means no master check, not allow', async () => { + // A security service without the member still satisfies the contract, and + // a consumer cannot reach the master check without handling the absent + // case first. + const withoutIt: ISecurityService = makeService(); + expect(typeof withoutIt.checkControlledByParentWrite).toBe('undefined'); + + // The unguarded call does not compile. Never invoked: its only job is to + // make the COMPILER prove the point. + const mustNotCompileWithoutAGuard = () => + // @ts-expect-error possibly undefined — a consumer must feature-detect first + withoutIt.checkControlledByParentWrite('order_line', 'r1', { userId: 'u1' }); + expect(typeof mustNotCompileWithoutAGuard).toBe('function'); + + // The shape a gate writes. Absence proceeds, because the parent's own + // update meets no master check there either, and it is reported as its own + // state: nothing measured the master, so it is never relabelled `allow`. + const gate = async (svc: ISecurityService) => { + if (typeof svc.checkControlledByParentWrite !== 'function') return 'no-master-check'; + const answer = await svc.checkControlledByParentWrite('order_line', 'r1', { userId: 'u1' }); + return answer.outcome === 'allow' || answer.outcome === 'not_applicable' ? 'proceed' : 'refuse'; + }; + await expect(gate(withoutIt)).resolves.toBe('no-master-check'); + + const seen: unknown[] = []; + const withIt = makeService({ + checkControlledByParentWrite: async (object, recordId, context) => { + seen.push([object, recordId, context]); + return { outcome: 'deny', leg: 'record_sharing' }; + }, + }); + await expect(gate(withIt)).resolves.toBe('refuse'); + expect(seen).toEqual([['order_line', 'r1', { userId: 'u1' }]]); + await expect(gate(makeService({ checkControlledByParentWrite: async () => ({ outcome: 'allow' }) }))) + .resolves.toBe('proceed'); + }); + + it('ControlledByParentWriteOutcome: deny names its leg, unresolvable its reason, and neither arm can omit it (compile-time)', () => { + const everyOutcome: ControlledByParentWriteOutcome[] = [ + { outcome: 'allow' }, + { outcome: 'deny', leg: 'object_permission' }, + { outcome: 'not_applicable' }, + { outcome: 'unresolvable', reason: 'record_not_found' }, + ]; + expect(everyOutcome).toHaveLength(4); + + // A refusal with no leg, and a non-verdict with no reason, are not + // outcomes: the union holds each field to its arm. + // @ts-expect-error a `deny` must name the leg that refused + const denyWithoutLeg: ControlledByParentWriteOutcome = { outcome: 'deny' }; + // @ts-expect-error an `unresolvable` must name why no verdict was reached + const unresolvableWithoutReason: ControlledByParentWriteOutcome = { outcome: 'unresolvable' }; + expect([denyWithoutLeg, unresolvableWithoutReason]).toHaveLength(2); + + // `abstain` is the sharing verdict's state, not this check's: the object + // either has no master check (`not_applicable`) or the check answers. + // @ts-expect-error `abstain` is not an outcome of the master-detail check + const notAnOutcome: ControlledByParentWriteOutcome = { outcome: 'abstain' }; + expect(notAnOutcome.outcome).toBe('abstain'); + + // Both vocabularies are closed, so a consumer branches on a value and + // never parses the refusal's prose. + const everyLeg: ControlledByParentWriteDenialLeg[] = [ + 'object_permission', + 'row_level_security', + 'record_sharing', + 'master_chain', + ]; + const everyReason: ControlledByParentWriteUnresolvedReason[] = [ + 'master_detail_relation_missing', + 'record_not_found', + 'master_reference_missing', + ]; + expect(everyLeg).toHaveLength(4); + expect(everyReason).toHaveLength(3); + // A store fault is a REJECTION with its own status, never a reason here. + // @ts-expect-error a store fault rejects; it is not a reason the check gives + const faultIsNotAReason: ControlledByParentWriteUnresolvedReason = 'store_fault'; + expect(faultIsNotAReason).toBe('store_fault'); + + // An exhaustive consumer: every arm is handled, and a fifth arm added to + // the union would stop this switch compiling at the `never` line. + const decide = (answer: ControlledByParentWriteOutcome): 'proceed' | 'refuse' => { + switch (answer.outcome) { + case 'allow': + case 'not_applicable': + return 'proceed'; + case 'deny': + case 'unresolvable': + return 'refuse'; + default: { + const unhandled: never = answer; + return unhandled; + } + } + }; + expect(everyOutcome.map(decide)).toEqual(['proceed', 'refuse', 'proceed', 'refuse']); + }); + + it('checkControlledByParentWrite: a store fault REJECTS with its own status, and a gate propagates it', async () => { + // The check lets a read fault propagate rather than report it as an + // access answer, so the member rejects with the engine's own error. A gate + // that propagates the rejection keeps the declared 503; one that caught it + // into `deny` would answer an outage as a refusal, and one that caught it + // into `allow` would fail open. + const outage = Object.assign(new Error('datasource unavailable'), { + code: 'ERR_DATASOURCE_UNAVAILABLE', + status: 503, + }); + const failing = makeService({ + checkControlledByParentWrite: async () => { + throw outage; + }, + }); + const gate = async (svc: ISecurityService) => { + if (typeof svc.checkControlledByParentWrite !== 'function') return 'no-master-check'; + const answer = await svc.checkControlledByParentWrite('order_line', 'r1', { userId: 'u1' }); + return answer.outcome === 'allow' || answer.outcome === 'not_applicable' ? 'proceed' : 'refuse'; + }; + await expect(gate(failing)).rejects.toMatchObject({ code: 'ERR_DATASOURCE_UNAVAILABLE', status: 503 }); + + // A non-verdict is an outcome, and the gate refuses on it: a record whose + // access IS derived from a master and whose check reached no verdict is + // not a record nothing refuses. + const missingRow = makeService({ + checkControlledByParentWrite: async () => ({ outcome: 'unresolvable', reason: 'record_not_found' }), + }); + await expect(gate(missingRow)).resolves.toBe('refuse'); + }); }); diff --git a/packages/spec/src/contracts/security-service.ts b/packages/spec/src/contracts/security-service.ts index 60695f7288d..90c88efd110 100644 --- a/packages/spec/src/contracts/security-service.ts +++ b/packages/spec/src/contracts/security-service.ts @@ -44,6 +44,10 @@ * answers a question a composing caller may use to WIDEN, so its failure mode * is the one that changes nothing: `abstain`. It never reports `admit` for a * reason it did not measure, and it never throws outward. + * - **Gate outcomes refuse, and a fault rejects.** {@link ISecurityService.checkControlledByParentWrite} + * answers a question a composing gate uses to REFUSE, so every outcome but + * `allow` and `not_applicable` refuses. A store fault is not an outcome: it + * rejects with its own status and never becomes a verdict. * * That distinction is load-bearing: the field projection is only ever a * cosmetic narrowing on top of enforcement that already happened (the read path @@ -289,6 +293,104 @@ export interface OwnershipFloorAlternate { readonly using: string; } +/** + * [ADR-0055] Which leg of the master-detail write check refused, in a `deny` + * from {@link ISecurityService.checkControlledByParentWrite}. + * + * The check asks whether the caller may EDIT the record's master. It asks that + * of each master up the `controlled_by_parent` chain until it reaches a master + * that governs its own rows. Every value here is a refusal the check already + * makes, `403 PERMISSION_DENIED`, and the check tells them apart by the reason + * its refusal states: + * + * - `object_permission`: the principal holds no object-level `update` grant on + * a master. + * - `row_level_security`: a master row is outside the principal's write + * row-level security on that master. + * - `record_sharing`: record sharing does not let the principal edit a master + * row, and no app-authored update policy on that master admits the row + * ({@link ISecurityService.checkAuthoredRowWrite}). + * - `master_chain`: the chain itself cannot be walked to a master that governs + * its own rows. It re-enters an object it has already visited, it exceeds + * the depth bound, or a master ABOVE the record's own master has no relation + * to derive from, is not present, or has an empty master reference. Above the + * first hop the check answers these as refusals, not as defects of the + * request, so they are a leg here and never `unresolvable`. + * + * The first three legs run on every hop, so `object_permission` can name the + * master's own master. A closed vocabulary: a consumer branches on it and + * never parses the refusal's prose. + */ +export type ControlledByParentWriteDenialLeg = + | 'object_permission' + | 'row_level_security' + | 'record_sharing' + | 'master_chain'; + +/** + * [ADR-0055] Why the master-detail write check reached no verdict, in an + * `unresolvable` from {@link ISecurityService.checkControlledByParentWrite}. + * + * These are the check's three outcomes that are not answers about access. A + * by-id update answers each with its own code, never with the access refusal: + * + * - `master_detail_relation_missing`: the object declares `controlled_by_parent` + * and has no `master_detail` relation to derive access from. A metadata + * defect (`422 INVALID_METADATA`). + * - `record_not_found`: the record `(object, recordId)` does not exist + * (`404 RECORD_NOT_FOUND`). This is the addressed record, not its master. A + * missing master row is not one of these outcomes: on the first hop the legs + * judge it as they judge any master row, and above the first hop it is the + * `master_chain` leg. + * - `master_reference_missing`: the record's stored master reference is empty + * (`422 MISSING_REQUIRED_FIELD`). + */ +export type ControlledByParentWriteUnresolvedReason = + | 'master_detail_relation_missing' + | 'record_not_found' + | 'master_reference_missing'; + +/** + * [ADR-0055] What {@link ISecurityService.checkControlledByParentWrite} + * resolves with: the master-detail write check's own outcome for a by-id update + * of one record. + * + * - `allow`: the check does not refuse. Every leg passes on every master up the + * chain for the principal (and, on an on-behalf-of context, for the delegator + * too), or the context is a system one, which the write path never checks. + * - `deny`: the check refuses, on the {@link ControlledByParentWriteDenialLeg} + * it names. + * - `not_applicable`: the object does not declare `controlled_by_parent`, so + * no master check exists for it. Not a verdict about the record. + * - `unresolvable`: the check reached no verdict, for the + * {@link ControlledByParentWriteUnresolvedReason} it names. + * + * A consumer that composes this into a gate proceeds on `allow` and on + * `not_applicable`, and refuses on every other outcome. ⛔ `unresolvable` is + * not `not_applicable`: no verdict was reached about a record whose access IS + * derived from a master, so reading it as "nothing here refuses" fails open. + * + * **A store fault is not an outcome. It is a rejection.** Wherever the check + * lets a fault propagate (its reads of the record and of each master up the + * chain do), the member rejects with the engine's own error, unchanged, so the + * fault keeps its declared status (a datasource outage is `503`). ⛔ It never + * resolves `deny` or `unresolvable` for a fault: an outage reported as + * "refused" or "record not found" is a false statement about the record. Two + * legs fail closed on their own probes by design (the row-level security leg + * and the record sharing leg), and their `deny` is then the check's answer, + * reported as it stands. + * + * A DISCRIMINATED UNION, for the reason {@link DelegationNarrowing} gives: as + * one shape with optional fields, a consumer that forgets to check `outcome` + * reads a `deny` with no `leg` as an answer. Here `leg` exists only on `deny` + * and `reason` only on `unresolvable`, so the compiler holds each to its arm. + */ +export type ControlledByParentWriteOutcome = + | { readonly outcome: 'allow' } + | { readonly outcome: 'deny'; readonly leg: ControlledByParentWriteDenialLeg } + | { readonly outcome: 'not_applicable' } + | { readonly outcome: 'unresolvable'; readonly reason: ControlledByParentWriteUnresolvedReason }; + /** * Public contract for the `security` service. * @@ -843,6 +945,71 @@ export interface ISecurityService { context?: SecurityContext, ): Promise; + /** + * [ADR-0055] What the master-detail write check answers for an UPDATE of + * `recordId` on `object` by `context`. + * + * A by-id write to a `controlled_by_parent` record requires EDIT access to + * its master. The write path checks it on every master up the chain: the + * object-level `update` grant, the master's write row-level security, and + * record sharing (whose refusal an app-authored update policy on the master + * can lift, {@link checkAuthoredRowWrite}). This member exposes that check + * to code outside the write path, such as a gate on a record's attachments or + * comments that must judge the record as its own update is judged. It adds no + * verdict: every outcome it resolves with is one the check already reaches, + * as {@link ControlledByParentWriteOutcome} sets out. + * + * **The answer equals what a by-id update of the same record gets from this + * check.** It is computed by the write path's own composition, never + * re-derived. The same legs run in the same order over the same chain. For an + * on-behalf-of context the ADR-0090 D10 delegator leg is included: the check + * runs for the principal's resolved permission sets and then for the + * delegator's, and the first refusal is the answer. So a consumer that + * refuses on `deny` refuses exactly the principals this check refuses on the + * record's own update. It answers the master check alone: the gates the + * update meets on the record itself (its object-level grant, its own + * row-level security and sharing) are not part of the answer. + * + * The contexts around the check answer as the write path does: + * - A system context bypasses the write path, and answers `allow`. + * - A principal whose permission sets resolve to none is judged over that + * empty list. It holds no `update` grant on any master, so it answers `deny` + * on `object_permission` (ADR-0056 D2). + * - Where the write path refuses the CONTEXT before any gate runs, the member + * rejects with that same refusal, `403 PERMISSION_DENIED`. Those cases are a + * context with no principal at all (ADR-0096 D5), permission sets that + * cannot be resolved, and an on-behalf-of link naming a delegator who does + * not exist (ADR-0090 D10). They refuse the request, so they are not + * answers about the master. + * - A store fault rejects with the engine's own error, unchanged (see + * {@link ControlledByParentWriteOutcome}). + * + * Every rejection is a refusal. A consumer propagates it as it is, and ⛔ + * never catches it into `allow`. + * + * **Absence is the absence of a master check, not a policy that admits.** A + * kernel whose security service omits this member composes no master-detail + * write check for a consumer to ask, and there the parent's own by-id update + * meets no master check either. A consumer that proceeds when the member is + * absent therefore gives the answer the parent's update gets. ⛔ It does not + * report that as `allow`: nothing measured the master, so a log line or an + * explanation must not claim the master was judged editable. An + * implementation whose write path runs the check serves this member from the + * same composition, and that is what keeps absence meaning "no master check". + * + * **OPTIONAL, so every consumer handles its absence.** A security service + * that predates it omits it, and consumers feature-detect + * (`typeof svc.checkControlledByParentWrite === 'function'`). Declaring it + * optional makes that a property of the type rather than a promise in prose: + * the unguarded call does not compile, so a consumer cannot skip the absent + * branch by accident. + */ + checkControlledByParentWrite?( + object: string, + recordId: string, + context?: SecurityContext, + ): Promise; + /** * Explain WHY access is granted or denied — the decision plus the layers that * produced it (permission sets, object permissions, RLS, sharing, field mask).