fix(spec)!: HierarchyScopeContext 声明 organizationId 为权威租户字段并转必填 (#5858)

.changeset/hierarchy-scope-organization-authority.md

@@ -0,0 +1,49 @@
+---
+"@objectstack/spec": major
+---
+
+fix(spec)!: `HierarchyScopeContext.organizationId` is the authoritative tenancy field, and it is now required (#5858)
+
+`HierarchyScopeContext` declared `organizationId?: string | null` and
+`tenantId?: string | null` side by side with no doc saying which one carries the
+caller's active organization. That silence had a measured cost (#5852): the
+single in-repo producer (`@objectstack/plugin-sharing`) filled `organizationId`
+from a `SharingExecutionContext` whose only tenancy member is `tenantId`, so the
+read was structurally always `null`, while the real consumer — the enterprise
+`hierarchy-scope-resolver` — reads `organizationId`, saw `null`, and skipped
+tenant isolation. Both ends were individually contract-compliant; the pair was a
+reachable cross-organization read.
+
+The contract now says which field is authoritative, and enforces it structurally
+rather than in prose:
+
+- **`organizationId` is the authority** — the caller's active organization,
+  `null` = platform/unscoped, the same meaning `EvalUser.organizationId`
+  already carries. This is the repo's settled name for the concept, not a new
+  one: #3280 blessed `organizationId` as the developer-facing name for the
+  caller's active org (matching the `organization_id` column and
+  `current_user.organizationId` in RLS), #3290 removed the `session.tenantId`
+  alias in v11, and `scripts/check-org-identifier.mjs` gates it in CI.
+- **It is REQUIRED** — `organizationId: string | null`, never omitted. A
+  producer that forgets to state the caller's org now fails to compile instead
+  of handing every resolver an `undefined` it will read as "unscoped". Stating
+  "no org" is still allowed, but it must be stated: `null` is a value, not an
+  omission.
+- **`tenantId` is retained as a deprecated alias**, not removed. It stays the
+  generic driver-layer tenancy knob (a database-per-tenant kernel legitimately
+  puts an *environment* id there), and its doc now says a resolver must not
+  depend on it alone or use it as a stand-in for a `null` `organizationId`.
+  Its removal is a separate, deliberate retirement.
+- **`IHierarchyScopeResolver.resolveOwnerIds` documents the fail-closed
+  obligation**: when `organizationId` is `null`, an implementation must not
+  build the owner set as though no tenancy constraint applied — "no org" is
+  never "every org". Return owner-only (or throw, which the sharing layer
+  treats the same way); never widen.
+
+**Migration.** Breaking for *producers* of the context only — implementers of
+`IHierarchyScopeResolver` are unaffected (a required property is strictly easier
+to consume). A producer that omitted the key adds `organizationId: <the caller's
+active org, or null>`; a producer that was passing the org under `tenantId` must
+move it, which is the bug this closes rather than a rename to absorb. The only
+in-repo producer already supplied the key, so nothing in this repo changed
+shape.

packages/spec/src/contracts/sharing-service.test.ts

@@ -1,7 +1,11 @@
 // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
 
 import { describe, it, expect } from 'vitest';
-import type { RecordShareRecipientType, SharingRuleRecipientType } from './sharing-service';
+import type {
+  HierarchyScopeContext,
+  RecordShareRecipientType,
+  SharingRuleRecipientType,
+} from './sharing-service';
 import { ShareRecipientType } from '../security/sharing.zod';
 
 /**
@@ -126,3 +130,130 @@ describe('[#5125] ISharingService write-gate bypass documentation parity', () =>
     expect(docOf.get('buildReadFilter')).not.toContain('modifyAllRecords');
   });
 });
+
+/**
+ * [#5858] `HierarchyScopeContext` names ONE authoritative tenancy field.
+ *
+ * The interface declared `organizationId?` and `tenantId?` side by side with no
+ * doc saying which one carries the caller's active organization. The single
+ * in-repo producer filled `organizationId` from a `SharingExecutionContext`
+ * that only has `tenantId`, so the read was structurally always `null`; the
+ * real consumer (cloud `security-enterprise`) reads `organizationId`, saw
+ * `null`, and skipped tenant isolation. Both ends were "contract-compliant" and
+ * the pair leaked across organizations (#5852).
+ *
+ * The fix is on the contract, not on either consumer: `organizationId` is the
+ * authority (repo convention — #3280 blessed the name, #3290 removed the
+ * `session.tenantId` alias, `scripts/check-org-identifier.mjs` gates it) and is
+ * now REQUIRED, so a producer that forgets it fails to compile rather than
+ * handing every resolver an `undefined` org. `tenantId` survives as a
+ * deprecated alias — removal is its own retirement, not a rider here.
+ *
+ * Two kinds of pin below, because the change has two halves: type-level ones
+ * that tsc evaluates (`tsconfig.test.json` compiles this file — #5286), and
+ * prose ones, because prose is the other half of what was missing.
+ */
+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 orgScoped: HierarchyScopeContext = {
+      userId: 'usr_1',
+      organizationId: 'org_east',
+      tenantId: 'env_prod',
+    };
+
+    // THE pin for the required-ness. Omitting the authoritative field is the
+    // exact producer bug #5852 measured; it must not compile. Deleting the `?`
+    // again turns this directive into an unused-`@ts-expect-error` error, and
+    // 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' };
+
+    // 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' };
+
+    expect(platformScoped.organizationId).toBeNull();
+    expect(orgScoped.organizationId).toBe('org_east');
+    expect(missingOrg.userId).toBe('usr_1');
+    expect(tenantOnly.tenantId).toBe('env_prod');
+  });
+
+  it('pins WHICH keys are mandatory, in both directions (compile-time)', () => {
+    // `-?` strips optionality, then `object extends Pick<T, K>` is true exactly
+    // when K was optional — so the union is the mandatory keys.
+    type RequiredKeys<T> = {
+      [K in keyof T]-?: object extends Pick<T, K> ? never : K;
+    }[keyof T];
+
+    const mandatory: Array<RequiredKeys<HierarchyScopeContext>> = ['userId', 'organizationId'];
+
+    // The other direction, and the ⛔-not-deleted guard in one: `tenantId` is
+    // still a member (a removal breaks the reference below) and still OPTIONAL
+    // (making the deprecated alias mandatory would be the mirror mistake).
+    // @ts-expect-error `tenantId` stays optional — it is a deprecated alias, not a second authority (#5858)
+    const notMandatory: RequiredKeys<HierarchyScopeContext> = 'tenantId';
+
+    expect(mandatory).toEqual(['userId', 'organizationId']);
+    expect(notMandatory).toBe('tenantId');
+  });
+
+  it('documents the authority, the deprecation, and the fail-closed obligation', async () => {
+    const ts = (await import('typescript')).default;
+    const { readFileSync } = await import('node:fs');
+    const { dirname, resolve } = await import('node:path');
+    const { fileURLToPath } = await import('node:url');
+
+    const file = resolve(dirname(fileURLToPath(import.meta.url)), 'sharing-service.ts');
+    const source = ts.createSourceFile(
+      file,
+      readFileSync(file, 'utf8'),
+      ts.ScriptTarget.Latest,
+      /* setParentNodes */ true,
+    );
+
+    const memberDocs = (name: string): Map<string, string> => {
+      const iface = source.statements.find(
+        (s): s is import('typescript').InterfaceDeclaration =>
+          ts.isInterfaceDeclaration(s) && s.name.text === name,
+      );
+      expect(iface, `${name} must still be an interface in this file`).toBeDefined();
+      const out = new Map<string, string>();
+      for (const member of iface!.members) {
+        if (!member.name || !ts.isIdentifier(member.name)) continue;
+        // `getFullText` carries leading trivia — the doc comment an IDE shows
+        // on hover — with no assumption about how it is wrapped.
+        out.set(member.name.text, member.getFullText(source));
+      }
+      return out;
+    };
+
+    const ctx = memberDocs('HierarchyScopeContext');
+    // 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.get('organizationId')).toContain('AUTHORITATIVE');
+    expect(ctx.get('organizationId')).toContain('platform/unscoped');
+    expect(ctx.get('organizationId')).toContain('MUST scope its owner set by this field');
+
+    expect(ctx.get('tenantId')).toContain('@deprecated');
+    expect(ctx.get('tenantId')).toContain('Not the authority for hierarchy scoping');
+
+    // Anti-vacuity 2: the search DISCRIMINATES. `userId` is the honest negative
+    // — it is identity, not tenancy, and calling it authoritative would itself
+    // be drift.
+    expect(ctx.get('userId')).not.toContain('AUTHORITATIVE');
+
+    const resolver = memberDocs('IHierarchyScopeResolver');
+    expect([...resolver.keys()]).toEqual(['resolveOwnerIds']);
+    // A `null` organization is "no org", never "every org".
+    expect(resolver.get('resolveOwnerIds')).toContain('Fail CLOSED');
+    expect(resolver.get('resolveOwnerIds')).toContain('never widen');
+  });
+});

packages/spec/src/contracts/sharing-service.ts

@@ -393,9 +393,40 @@ export interface IBusinessUnitGraphService {
  */
 export type HierarchyScope = 'unit' | 'unit_and_below' | 'own_and_reports';
 
+/**
+ * Caller identity a {@link IHierarchyScopeResolver} resolves a
+ * {@link HierarchyScope} against.
+ *
+ * **`organizationId` is the AUTHORITATIVE tenancy field** — the one a resolver
+ * scopes its owner-set query by. It is REQUIRED (`string | null`, never
+ * omitted) so a producer that forgets to supply it fails to compile instead of
+ * silently handing every resolver an `undefined` org: two sides can each look
+ * contract-compliant while the pair leaks across organizations (#5852/#5858).
+ * The name follows the repo-wide convention: #3280 made `organizationId` the
+ * blessed developer-facing name for the caller's active org (matching the
+ * `organization_id` column and `current_user.organizationId` in RLS) and #3290
+ * removed the `session.tenantId` alias in v11; `scripts/check-org-identifier.mjs`
+ * keeps it that way.
+ */
 export interface HierarchyScopeContext {
   userId: string;
-  organizationId?: string | null;
+  /**
+   * AUTHORITATIVE. Active organization ID of the caller (`null` =
+   * platform/unscoped) — same meaning as `EvalUser.organizationId`. A resolver
+   * MUST scope its owner set by this field; see
+   * {@link IHierarchyScopeResolver.resolveOwnerIds} for the `null` obligation.
+   */
+  organizationId: string | null;
+  /**
+   * Generic driver-layer tenancy knob, carried through for kernels that key
+   * isolation off something other than the organization (database-per-tenant
+   * deployments legitimately put an *environment* id here).
+   *
+   * @deprecated Not the authority for hierarchy scoping — a resolver MUST NOT
+   * depend on it alone, and MUST NOT treat it as a substitute for a `null`
+   * {@link HierarchyScopeContext.organizationId}. Read `organizationId`.
+   * Retained for compatibility only; removal goes through the retirement flow.
+   */
   tenantId?: string | null;
 }
 
@@ -420,6 +451,13 @@ 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.
    */
   resolveOwnerIds(context: HierarchyScopeContext, scope: HierarchyScope): Promise<string[]>;
 }