diff --git a/.changeset/hierarchy-scope-posture-signal.md b/.changeset/hierarchy-scope-posture-signal.md new file mode 100644 index 0000000000..696a8e23d4 --- /dev/null +++ b/.changeset/hierarchy-scope-posture-signal.md @@ -0,0 +1,61 @@ +--- +"@objectstack/spec": major +--- + +fix(spec)!: `HierarchyScopeContext` carries the tenancy posture, so single-posture DEPTH is legal (#6139) + +Two accepted positions contradicted each other on `main`, and a resolver could +not satisfy both: + +1. `IHierarchyScopeResolver.resolveOwnerIds` obliged an implementation to fail + CLOSED whenever `organizationId` was `null` — unconditionally, "no org is not + every org" (#5852/#5973). +2. Ruling C (#5859, landed in PR #6067) requires a single-posture deployment — + one with no organizations at all — to feed an explicit `null` and still get + hierarchy DEPTH. + +`HierarchyScopeContext` had no way to tell those two `null`s apart, so a +strictly spec-conformant resolver (cloud PR #1196 fails closed unconditionally, +exactly as written) necessarily killed enterprise DEPTH on every single-posture +install. The contract demanded the behaviour the platform ruling forbade. + +**FROM** `{ userId, organizationId: string | null, tenantId?: string | null }`, +with `null` organization ⇒ fail closed, always. +**TO** the same plus a REQUIRED `posture: TenancyPosture` (ADR-0105 D1), with +the obligation now read from BOTH fields: + +- `posture: 'single'` + `organizationId: null` — **legitimate.** There is no + organization dimension at all, so `null` names the one implicit tenant. The + resolver MUST proceed and resolve DEPTH normally; refusing here retires + hierarchy scoping for every org-less deployment. +- `posture: 'group' | 'isolated'` + `organizationId: null` — **fail closed, + strictly.** A wall is in force, so `null` is a missing constraint. This half + is unchanged and unrelaxed: it is what closed the #5852 cross-organization + privilege escalation. + +A structured signal was chosen over prose ("single-posture deployments are +exempt") because prose cannot be read by the code that must act on it: the +resolver runs inside the enterprise package and needs the deployment fact at +call time, not a paragraph. + +`posture` is REQUIRED, on the same terms and for the same reason #5858 made +`organizationId` required: a producer that omits it must fail to COMPILE rather +than hand every resolver an `undefined` to guess about, when one guess leaks +across organizations and the other silently kills DEPTH. This is a breaking +change for anyone CONSTRUCTING a `HierarchyScopeContext`; implementors of +`IHierarchyScopeResolver` are source-compatible, though a resolver keying only +on `organizationId` is no longer conformant and should adopt the two-field read. + +Supplying it costs producers nothing new: the open sharing layer already +resolved the posture to decide whether to consult a resolver at all. That +derivation now lives in one place (`effectiveTenancyPosture()`), with the local +refusal expressed in terms of it, so the refusal and the reported posture cannot +drift apart. It still fails closed — an unresolvable posture reports the +strictest walled posture, never `single`. + +The `showcase-scope-depth` dogfood proofs now run a **spec-conformant** +reference resolver typed against the real interface. The previous fixture took +`c: any` and ignored the tenancy fields entirely, which is why 20 single-posture +e2e proofs stayed green throughout: no spec-conformant resolver was ever +exercised, so CI could not see the contradiction. Verified non-vacuous — with +the old unconditional rule restored, three DEPTH proofs fail. diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index 4d64583768..91dfa15802 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -976,6 +976,15 @@ export class SharingService implements ISharingService { // AUTHORITATIVE (#5858 / PR #5973). Never `(context as any).organizationId`: // no execution context in this repo carries that key. organizationId, + // [#6139] What a `null` organizationId MEANS. Under `single` it is + // the one implicit tenant and DEPTH resolves normally; under + // `group`/`isolated` it is a missing constraint and the resolver's + // fail-closed obligation applies. Without this a spec-conformant + // resolver had to refuse every `null`, which killed DEPTH on exactly + // the single-posture deployments ruling C (#5859) requires it to work + // on. Same value the refusal above keys on — one derivation, so the + // two cannot disagree. + posture: this.effectiveTenancyPosture(), // The @deprecated compatibility alias, carried through unchanged for // resolvers that still read it. It is NOT the authority — a resolver // reading it alone is the shape #5858 ruled out. @@ -1023,19 +1032,46 @@ export class SharingService implements ISharingService { * on precisely the deployments whose configuration is already suspect. */ private organizationScopeRequired(): boolean { + return postureEnforcesWall(this.effectiveTenancyPosture()); + } + + /** + * [#6139] The posture this deployment reports to a + * {@link IHierarchyScopeResolver} — and the single place the answer is + * decided. + * + * `HierarchyScopeContext.posture` is what lets a resolver tell "single + * posture, legitimately no organization" apart from "walled posture, + * organization missing ⇒ fail closed": the two `null`s that were previously + * indistinguishable on that interface, and that demand opposite answers. The + * producer already had to resolve the posture for + * {@link SharingService.organizationScopeRequired}, so reporting it costs + * nothing new. Deriving it TWICE is what would let the local refusal and the + * reported posture drift apart, which is why that predicate is now expressed + * in terms of this one rather than beside it. + * + * Fails CLOSED, exactly as the contract requires of a producer: an + * unresolvable posture (no `tenancy` probe wired, a throwing probe, a value + * outside the vocabulary) reports the strictest WALLED posture, never + * `single`. An unknown posture is not evidence of `single`, and reading it as + * such would restore the #5852 widening on precisely the deployments whose + * configuration is already suspect. + */ + private effectiveTenancyPosture(): TenancyPosture { let probe: SharingTenancyProbe | null | undefined; try { probe = this.tenancy?.(); } catch { - return true; // unresolvable → assume walled + return 'isolated'; // unresolvable → strictest walled posture } - if (!probe) return true; + if (!probe) return 'isolated'; const posture = normalizeTenancyPosture(probe.posture); - if (posture) return postureEnforcesWall(posture); - // Pre-ADR-0105 shape: only `isolationActive === false` is a positive - // statement that no wall is enforced. `undefined` stays unresolved. - if (probe.isolationActive === false) return false; - return true; + if (posture) return posture; + // Pre-ADR-0105 shape: only `isolationActive === false` is a POSITIVE + // statement that no wall is enforced — the wall-less shape `single` names. + // `undefined` stays unresolved and keeps the strict answer. + if (probe.isolationActive === false) return 'single'; + return 'isolated'; } private shouldBypass(object: string, context: SharingExecutionContext): boolean { diff --git a/packages/qa/dogfood/test/showcase-scope-depth.dogfood.test.ts b/packages/qa/dogfood/test/showcase-scope-depth.dogfood.test.ts index 49797be652..fe8382894f 100644 --- a/packages/qa/dogfood/test/showcase-scope-depth.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-scope-depth.dogfood.test.ts @@ -18,6 +18,16 @@ import showcaseStack from '@objectstack/example-showcase'; import { bootStack, type VerifyStack } from '@objectstack/verify'; import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; import { PermissionSetSchema } from '@objectstack/spec/security'; +// [#6139] The reference resolver is typed against the REAL contract, not `any`. +// A fixture that ignores the tenancy fields cannot prove the guarantee this +// test claims: it would stay green even for a resolver that fails closed on +// every `null` organization — which is exactly the spec-conformant shape that +// used to KILL single-posture DEPTH, and exactly what these proofs must catch. +import type { + HierarchyScope, + HierarchyScopeContext, + IHierarchyScopeResolver, +} from '@objectstack/spec/contracts'; const OBJ = '/data/showcase_private_note'; const WHO = ['alice', 'bob', 'carol', 'dave'] as const; @@ -56,10 +66,26 @@ async function bootScopeWorld(scope: 'unit' | 'unit_and_below' | 'own_and_report // Reference hierarchy-scope resolver (test fixture; prod = @objectstack/security-enterprise). // Inlined (no plugin-sharing import) — proves the IHierarchyScopeResolver seam end-to-end. - const refResolver = { - async resolveOwnerIds(c: any, sc: string): Promise { - const meId = c.userId as string; + // + // [#6139] SPEC-CONFORMANT, and typed as such. It previously took `c: any` and + // ignored `organizationId` entirely, which made these 20 single-posture + // proofs blind to the contradiction they were supposed to cover: a resolver + // obeying the old unconditional "null org ⇒ fail closed" obligation would + // have returned owner-only on every single one of these calls (the showcase + // stack boots org-less), and this suite would still have been green. It now + // implements the posture-conditional rule verbatim, so the guarantee that + // enterprise DEPTH works under `single` is actually pinned by the assertions + // below rather than assumed. + const refResolver: IHierarchyScopeResolver = { + async resolveOwnerIds(c: HierarchyScopeContext, sc: HierarchyScope): Promise { + const meId = c.userId; const ids = new Set([meId]); + + // The obligation, both halves. Under a WALLED posture a missing + // organization is a missing constraint: fail closed, never widen + // (#5852). Under `single` there is no organization dimension at all, so + // `null` is the one implicit tenant and DEPTH resolves normally (#6139). + if (c.posture !== 'single' && c.organizationId === null) return [meId]; if (sc === 'own_and_reports') { let frontier: string[] = [meId]; const seen = new Set([meId]); for (let d = 0; d < 20 && frontier.length; d++) { diff --git a/packages/spec/src/contracts/sharing-service.test.ts b/packages/spec/src/contracts/sharing-service.test.ts index 48fa7f7e14..ce1c8a65b3 100644 --- a/packages/spec/src/contracts/sharing-service.test.ts +++ b/packages/spec/src/contracts/sharing-service.test.ts @@ -157,10 +157,15 @@ describe('[#5858] HierarchyScopeContext tenancy authority', () => { it('requires `organizationId` and keeps `tenantId` optional (compile-time)', () => { // A caller with no active org states so EXPLICITLY. `null` is a value the // contract carries (platform/unscoped), never an omission. - const platformScoped: HierarchyScopeContext = { userId: 'usr_1', organizationId: null }; + const platformScoped: HierarchyScopeContext = { + userId: 'usr_1', + organizationId: null, + posture: 'single', + }; const orgScoped: HierarchyScopeContext = { userId: 'usr_1', organizationId: 'org_east', + posture: 'isolated', tenantId: 'env_prod', }; @@ -170,17 +175,29 @@ describe('[#5858] HierarchyScopeContext tenancy authority', () => { // this file carries no entry in `test-typecheck-debt.json` — so its budget // is zero and the gate goes red. // @ts-expect-error `organizationId` is REQUIRED — omitting it must not compile (#5858) - const missingOrg: HierarchyScopeContext = { userId: 'usr_1' }; + const missingOrg: HierarchyScopeContext = { userId: 'usr_1', posture: 'single' }; // The deprecated alias is NOT a substitute: supplying only `tenantId` // leaves the authoritative field unstated, which is the same defect. // @ts-expect-error `tenantId` does not satisfy the authoritative field (#5858) - const tenantOnly: HierarchyScopeContext = { userId: 'usr_1', tenantId: 'env_prod' }; + const tenantOnly: HierarchyScopeContext = { userId: 'usr_1', tenantId: 'env_prod', posture: 'single' }; + + // [#6139] `posture` is REQUIRED on the same terms, for the symmetrical + // reason: without it a resolver cannot tell a legitimately org-less + // `single` deployment from a walled one whose organization went missing, + // so it must guess. One guess leaks across organizations (#5852); the other + // silently retires enterprise DEPTH. Neither is acceptable, so neither is + // reachable — the producer states the posture or does not compile. + // @ts-expect-error `posture` is REQUIRED — omitting it must not compile (#6139) + const missingPosture: HierarchyScopeContext = { userId: 'usr_1', organizationId: null }; expect(platformScoped.organizationId).toBeNull(); + expect(platformScoped.posture).toBe('single'); expect(orgScoped.organizationId).toBe('org_east'); + expect(orgScoped.posture).toBe('isolated'); expect(missingOrg.userId).toBe('usr_1'); expect(tenantOnly.tenantId).toBe('env_prod'); + expect(missingPosture.organizationId).toBeNull(); }); it('pins WHICH keys are mandatory, in both directions (compile-time)', () => { @@ -190,7 +207,14 @@ describe('[#5858] HierarchyScopeContext tenancy authority', () => { [K in keyof T]-?: object extends Pick ? never : K; }[keyof T]; - const mandatory: Array> = ['userId', 'organizationId']; + // [#6139] `posture` joins the mandatory set. It is not decoration on the + // side of `organizationId` — the two are read TOGETHER, and a resolver + // holding one without the other cannot reach a correct verdict. + const mandatory: Array> = [ + 'userId', + 'organizationId', + 'posture', + ]; // The other direction, and the ⛔-not-deleted guard in one: `tenantId` is // still a member (a removal breaks the reference below) and still OPTIONAL @@ -198,7 +222,7 @@ describe('[#5858] HierarchyScopeContext tenancy authority', () => { // @ts-expect-error `tenantId` stays optional — it is a deprecated alias, not a second authority (#5858) const notMandatory: RequiredKeys = 'tenantId'; - expect(mandatory).toEqual(['userId', 'organizationId']); + expect(mandatory).toEqual(['userId', 'organizationId', 'posture']); expect(notMandatory).toBe('tenantId'); }); @@ -236,12 +260,22 @@ describe('[#5858] HierarchyScopeContext tenancy authority', () => { // Anti-vacuity 1: the enumeration found the real members, so a rename or a // deletion cannot quietly empty the assertions. `tenantId` being listed IS // the "not removed" pin — its retirement is a separate, deliberate change. - expect([...ctx.keys()]).toEqual(['userId', 'organizationId', 'tenantId']); + expect([...ctx.keys()]).toEqual(['userId', 'organizationId', 'posture', 'tenantId']); expect(ctx.get('organizationId')).toContain('AUTHORITATIVE'); expect(ctx.get('organizationId')).toContain('platform/unscoped'); expect(ctx.get('organizationId')).toContain('MUST scope its owner set by this field'); + // [#6139] The posture's prose must carry BOTH readings of a `null` + // organization, because carrying only one is how the contradiction arose: + // the field is useless unless it says what each value licenses. + expect(ctx.get('posture')).toContain('REQUIRED'); + expect(ctx.get('posture')).toContain('ONE implicit tenant'); + expect(ctx.get('posture')).toContain('MISSING'); + // …and it must NOT offer `single` as the safe default for an unknown + // posture — that is the exact misreading that would re-open #5852. + expect(ctx.get('posture')).toContain('strictest'); + expect(ctx.get('tenantId')).toContain('@deprecated'); expect(ctx.get('tenantId')).toContain('Not the authority for hierarchy scoping'); @@ -255,6 +289,17 @@ describe('[#5858] HierarchyScopeContext tenancy authority', () => { // A `null` organization is "no org", never "every org". expect(resolver.get('resolveOwnerIds')).toContain('Fail CLOSED'); expect(resolver.get('resolveOwnerIds')).toContain('never widen'); + + // [#6139] Both halves of the posture-conditional obligation, pinned + // together — stating either alone is what produced two accepted positions + // that contradicted each other. The walled half must still read as + // unconditional… + expect(resolver.get('resolveOwnerIds')).toContain('STRICT'); + // …and the `single` half must be an explicit MUST NOT refuse, so a + // resolver author cannot read "fail closed" as the whole rule and kill + // single-posture DEPTH while believing they were being careful. + expect(resolver.get('resolveOwnerIds')).toContain('MUST NOT be refused'); + expect(resolver.get('resolveOwnerIds')).toContain('Read the two fields TOGETHER'); }); }); diff --git a/packages/spec/src/contracts/sharing-service.ts b/packages/spec/src/contracts/sharing-service.ts index 49afe271c3..8fd427fba9 100644 --- a/packages/spec/src/contracts/sharing-service.ts +++ b/packages/spec/src/contracts/sharing-service.ts @@ -29,6 +29,12 @@ * The default implementation lives in `@objectstack/plugin-sharing`. */ +// Type-only: `HierarchyScopeContext.posture` names the ADR-0105 D1 vocabulary +// rather than restating it, so the contract and the wall it defers to cannot +// drift apart (#6139). Erased at compile time, so this module stays the pure +// type-declaration surface it has always been — no runtime coupling added. +import type { TenancyPosture } from '../security/tenancy-posture'; + /** * Recipient categories a `sys_record_share` ROW may carry — mirrors the * `recipient_type` select on `SysRecordShare` @@ -423,6 +429,37 @@ export interface HierarchyScopeContext { * {@link IHierarchyScopeResolver.resolveOwnerIds} for the `null` obligation. */ organizationId: string | null; + /** + * REQUIRED. The deployment's tenancy posture (ADR-0105 D1) — the fact that + * says what a `null` {@link HierarchyScopeContext.organizationId} MEANS. + * + * Without it the two `null`s are indistinguishable, and they demand opposite + * answers (#6139): + * + * - `single` — there is no organization dimension at all, so `null` is the + * ONE implicit tenant, not an absent constraint. A resolver MUST proceed + * and resolve DEPTH normally; refusing here retires hierarchy scoping for + * every org-less deployment, which the ADR-0057 D1 proofs boot on purpose. + * - `group` / `isolated` — a wall is in force, so `null` is a MISSING + * constraint. The fail-closed obligation applies in full force; that is + * the half which closed the #5852 cross-organization leak, and nothing on + * this interface relaxes it. + * + * Required — never optional, never omitted — for the same reason + * {@link HierarchyScopeContext.organizationId} is: a producer that forgets it + * must fail to COMPILE rather than hand every resolver an `undefined` to + * guess about, when either guess is a defect (one leaks across + * organizations, the other silently kills enterprise DEPTH). Producers + * already hold the value — the open sharing layer resolves the posture to + * decide whether to consult a resolver at all — so supplying it is a + * pass-through, not new plumbing. + * + * A producer that cannot resolve the posture MUST report the strictest + * WALLED posture rather than `single`: an unknown posture is not evidence of + * `single`, and reading it as such would restore the widening on precisely + * the deployments whose configuration is already suspect. + */ + posture: TenancyPosture; /** * Generic driver-layer tenancy knob, carried through for kernels that key * isolation off something other than the organization (database-per-tenant @@ -458,12 +495,29 @@ export interface IHierarchyScopeResolver { * Owner ids whose records the caller may see under `scope` (must include the * caller). Empty/throw → caller falls back to owner-only. * - * **Fail CLOSED on a missing organization.** When the authoritative - * {@link HierarchyScopeContext.organizationId} is `null`, an implementation - * MUST NOT build the owner set as though no tenancy constraint applied — - * "no org" is not "every org". Return owner-only (or throw, which the sharing - * layer treats the same way); never widen. Falling back to - * {@link HierarchyScopeContext.tenantId} instead is not fail-closed either. + * **Fail CLOSED on a missing organization — under a WALLED posture.** When + * {@link HierarchyScopeContext.posture} is `group` or `isolated` and the + * authoritative {@link HierarchyScopeContext.organizationId} is `null`, an + * implementation MUST NOT build the owner set as though no tenancy + * constraint applied — "no org" is not "every org". Return owner-only (or + * throw, which the sharing layer treats the same way); never widen. Falling + * back to {@link HierarchyScopeContext.tenantId} instead is not fail-closed + * either. This obligation is STRICT and unconditional within those two + * postures: it is the half that closed #5852. + * + * **Under `single`, a `null` organization is NOT missing information** and + * MUST NOT be refused (#6139). That posture has no organization dimension at + * all, so `null` names the one implicit tenant; the resolver resolves DEPTH + * normally, exactly as it would for a non-null org elsewhere. Refusing here + * is not a conservative choice — it retires hierarchy scoping for every + * org-less deployment, including the single-posture enterprise installs + * ADR-0057 D1's proofs boot deliberately. + * + * Read the two fields TOGETHER; neither alone carries the verdict. `null` + + * `single` is legitimate and widens; `null` + `group`/`isolated` fails + * closed. An implementation that keys only on `organizationId` is not + * conformant — it is the shape this contract used to require, and it kills + * single-posture DEPTH. */ resolveOwnerIds(context: HierarchyScopeContext, scope: HierarchyScope): Promise; }