Skip to content

feat(plugin-sharing,plugin-approvals,metadata-protocol)!: 18.x step 7 part 7a-3 — the organization-less readers of sharing rules, approval units and the seed loader retire - #24828

Merged
objectstack-fleet[bot] merged 57 commits into
mainfrom
claude/issue-24583-7a3-sharing-null-readers
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 57 commits into
mainfrom
claude/issue-24583-7a3-sharing-null-readers

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Part of #24583 — step 7, part 7a-3 (claim 6285134534; governing text: execution record 6285122667, ADR-0131 D2 and D8). The other step-7 parts (7a-1 driver, 7a-2 authz, 7b-*, 7c webhooks) are not in this PR, and #24583 remains open.

Clause-②: yes (narrowing): a declared sharing rule served under single gains the deploymentWide: true key on the wire in place of organization_id: null (widening); the sharing-rule admin reads, the approval unit screen and the scoped seed pre-load stop reading rows with no organization (narrowing). A declared rule keeps applying deployment-wide; nothing an author declares is refused.

What changes

plugin-sharing sharing-rule-service.ts

  • A declared rule's single projection carries a typed marker, deploymentWide: true (DeploymentWideSharingRule, isDeploymentWideSharingRule), and no organization_id key. This is the shape of the declared webhooks' deploymentWide (ruling A, 6278609968). The scope list holds a DEPLOYMENT_WIDE_SCOPE symbol in place of null.
  • criteriaContext loses its NULL branch. It answers one of three named cases:
    • the rule's organization, which is the tenant predicate;
    • a deployment-wide projection, which carries no tenant (D8 computes no predicate under single);
    • a composition with no organization object (the lean embedding, no-organization-object), which also carries no tenant.
    • Otherwise it answers null: a stored row with no organization sweeps nothing and grants nothing, and the service logs it once at error with the remedy.
  • How the sweep runs (decided and pinned):
    • Under single: one deployment-wide pass, and each grant is stamped with its record's organization.
    • Under a wall: one projection per organization, each inside that organization's one predicate, with grants stamped with it. No projection carries the marker under a wall, so Layer 0's predicate is never widened.
  • adminOrgScope and findRuleRowByName read the caller's own organization only.
  • assertCanDeletePlatformGlobalRule is deleted, together with its call.
  • sweepOrphanedRuleGrants and ruleGrantScope follow the marker.

plugin-approvals ApprovalService.businessUnitOrgScope: a strict equality. The ceremony attributes sys_business_unit, and a fresh install stamps it.

metadata-protocol seed-loader.ts: the unownedHolders read is deleted, together with its refuse decision and the refusal text #24681 had rewritten. A scoped load issues no organization_id IS NULL read on any path.

Pins rewritten to the new rule (no assertion weakened):

Premise measured first (step 7's "after certification")

Every reader retired here was dead on all three trees:

tree tenant-column tables rows with NULL organization_id
certified database (ceremoniedFixture over the 18.0 fixture), after the ceremony and again after a HEAD boot 66 0
fresh birth-marked single install (showcase --fresh) all 0 (sys_business_unit 5 rows, all stamped)
fresh isolated install, real organizations runtime all 0

One live path remains. It is kept, not deleted: the lean embedding with no organization object, which still writes NULL rows by design. It keeps the deployment-wide sweep, keyed on the composition fact (sys_organization not registered), never on a row's NULL. The production os serve boot without OS_AUTH_SECRET is that same composition today; 7b-3 makes it refuse.

Proof

Final head 357025dc97. The boots, harness and suites ran on 62227f5632, which differs from it only by main's README commit.

  • R1: a fresh showcase boot on a random port, run on the pre-merge head and again on the final head:
    • health 200, seeded-admin sign-in 200, 0 error lines;
    • 0 NULL organization_id rows in any tenant table;
    • the declared rule share_key_account_qualified_contacts_with_managers materialised a stamped grant.
  • R2 + R3: upgrade-18x-harness.dogfood.test.ts + organization-ownership-boot-refusal-order.dogfood.test.ts passed, together with the two sharing dogfood files: 33/33. This run includes the ceremonied fixture, and the v17 refusal fired before the ceremony.
  • R4:
    • An isolated os serve with @objectstack/organizations (invite-only membership, owner email set), on both heads: health 200, sign-in 200, 0 error lines, 0 NULL rows. GET /sharing/rules serves both declared rules as per-organization projections, with no marker. The grant is stamped.
    • Step 4's 7 walled e2e cases (serve-organizations-host-resolution + serve-organizations-mount-failure, OS_TEST_TIERS=nightly): 7/7.
  • Suites:
    • plugin-sharing 51 files / 1181 tests;
    • plugin-approvals 86 / 1147;
    • metadata-protocol 251 / 20815;
    • runtime 431 / 6781;
    • typecheck of plugin-sharing, plugin-approvals, metadata-protocol, runtime and dogfood: exit 0.
  • Gates: dispatch-gates --commands on the 12 code paths derived 75 commands. All 75 ran, and every exit code was captured before any pipe.
    • 73 exited 0 on the first run.
    • check-system-context-census was red: content/docs/permissions/system-context.mdx still cited the deleted assertCanDeletePlatformGlobalRule. The citation was dropped, and the re-run exited 0.
    • check-plugin-teardown-shape --self-test was a shallow-clone artefact: its pinned fixture commit was absent. It exited 0 once that commit was fetched.
    • Re-derived on the final 15 paths (1,641 changed lines, under the 3,000 line): the 26 newly derived docs families plus the 15 ratchet and test-reading families, 41 commands, all exit 0.
  • More pins the suites found, rewritten:
    • packages/runtime/src/standalone-stack-seeder-declaration-copy.test.ts: the served declared shape now carries the marker.
    • sharing-rule-positions-name-authority.test.ts: retitled. Its delete case is refused by the scope gate, not by the retired guard.
  • A sweep of 11 more candidate files, all green:
    • 7 dogfood files (scope-depth ×3, BU projection, BU/user delete, single platform insert, single-tenant identity create);
    • the cli declared-rule-mirrors integration test;
    • the rest canonical-query-AST test;
    • the cloud-connection posture-gate test.
  • Ablation of the main pin: restoring the NULL branch (return SYSTEM_CTX for every org-less rule) turned exactly the two predicted cases red ("a STORED rule with NO organization …", whole-rule and per-record), and the other 6 stayed green. Restore was proven by blob == HEAD and an empty git diff HEAD.

Acceptance notes

  • main's recipient-membership-recompute.ts (ruleReadsRowsOf, landed in 0b7286ff) still reads rule.organization_id == null as "reads every row". With the marker the deployment-wide projection has no organization_id, so it is still counted as affected, which is the right answer. The spelling is a NULL-shaped reader outside this part's file surface.
  • sharing-plugin.ts's resolveDeclaredRuleScopes still spells single as [null]. setDeclaredRuleScopes translates it at that one door, so no NULL travels past it. Retyping the plugin side is outside the claimed surface.
  • approval-service.ts teamIsProvablyOutsideOrg keeps its "no organization ⇒ leave routing" limb (a team screen). It is not in this part's surface, and its doc cross-reference was corrected.
  • The ORG-STAMPED pin in rule-criteria-org-scope.test.ts still expects deal_p1 through the driver's NULL record arm. That is 7a-1's arm and 7a-1's pin. Separately, an org-less RECORD matched by two walled projections aborts the second pass with RECORD_NOT_FOUND. That case cannot occur on a certified database, and it goes away with 7a-1.
  • File surface: beyond the claim's list, this PR edits one hand-written docs page (content/docs/permissions/system-context.mdx, row 40: the deleted guard's citation, which its census gate requires) and pins in packages/runtime and packages/qa/dogfood. It touches no docs/adr/** and no content/docs/releases/.
  • The seed pin files keep their null-organization-holder names: scripts/engine-double-contract.pinned.json registers them by path.

Generated by Claude Code

claude added 30 commits October 19, 2026 13:06
…r retired; webhook activation door

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…he 18.0 dual read; door pins for webhook

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…asserts no row and a dispatch

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…the checklist, the webhooks guide and the showcase; changeset

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…tcher's switch read

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…posture builds no wall, as its 18.0 seeded copy stamped with the Default Organization was

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…red writes; the declared-webhook double is drivable by the limit gate

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…does not implement

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…authors; checklist and admitsOrganization docblock say a declared webhook is deployment-wide under single

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…the fixture's customized declared-webhook mirror

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…n fail on a double delivery

Claude-Session: https://claude.ai/code/session_01PDSxBZE561TFe1N2iEjKXZ
Co-authored-by: Claude <noreply@anthropic.com>
…0g-webhooks-registry

# Conflicts:
#	content/docs/permissions/tenant-audit-census.mdx
#	docs/audits/2026-08-tenant-audit-write-call-sites.counts.md
…0g-webhooks-registry

# Conflicts:
#	content/docs/permissions/tenant-audit-census.mdx
#	docs/audits/2026-08-tenant-audit-write-call-sites.counts.md
…-registry' into claude/issue-24581-ceremony-open
…he ceremony, then boot; boot-refusal-order step 2 back on the binary
…el — the cipher names no organization

Claude-Session: https://claude.ai/code/session_014UATXWqAXqkaZkbG92ox6V
Co-authored-by: Claude <noreply@anthropic.com>
…sys_flow_credential leave reach)

Claude-Session: https://claude.ai/code/session_014UATXWqAXqkaZkbG92ox6V
Co-authored-by: Claude <noreply@anthropic.com>
… marker; an org-less row's delete by an org admin is pinned a no-op

Claude-Session: https://claude.ai/code/session_01XFEb4fSDbe4fK94S3x5FKX
Co-authored-by: Claude <noreply@anthropic.com>
…o driver NULL arm, declared rules deployment-wide by marker

Claude-Session: https://claude.ai/code/session_01XFEb4fSDbe4fK94S3x5FKX
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review (delta after PASS at 357025d)

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 236d39d
Local-runs: none
Scope: packages/spec/src/contracts/sharing-service.ts + .changeset/24583-7a3-sharing-null-readers.md, packages/plugins/plugin-sharing/src/sharing-rule-service.ts, packages/plugins/plugin-sharing/src/sharing-rule.test.ts, packages/plugins/plugin-sharing/src/rule-criteria-org-scope.test.ts, packages/plugins/plugin-sharing/src/sharing-plugin.ts, packages/drivers/driver-sql/src/sql-driver.ts (at 7c2eab9), packages/plugins/plugin-approvals/src/approval-service.ts (main's hunk only), packages/client/src/index.ts, packages/spec/scripts/build-api-surface.ts, packages/spec/scripts/check-generated.ts, packages/spec/api-surface/contracts.json, packages/spec/api-surface-signatures.json, packages/spec/export-origins/contracts.json, packages/spec/scripts/build-spec-changes.ts, scripts/check-adr-0087-registration.mjs, docs/adr/0131-total-organization-ownership-no-null-organization-id.md, AGENTS.md

Read-only delta review of PR #24828 at its new head, in a detached worktree, against the PASS record at 357025dc9. Commits since the PASS: 228e74a00 (R1 + R2), 7d8257d60 (changeset), 32b3d1c54 (merge of origin/main through 7c2eab929), 236d39daf (R4). Merge-base moved ddb3360a4 to 7c2eab929 = the 7a-1 landing (#24825). The PR's own diff is 16 files, +698/-985; packages/spec/src/contracts/sharing-service.ts is the one new file, which makes this a SCHEMA face.

① Derived judgments

1. R1 — SharingRuleRow.deploymentWide?: true matches the runtime exactly.

  • Contract: packages/spec/src/contracts/sharing-service.ts:484-492, an optional literal-true member beside the unchanged organization_id?: string | null (:483). Runtime: the only writer of the key is projectDeclaredRule (sharing-rule-service.ts:206-220), whose spread at :209 picks { deploymentWide: true } for DEPLOYMENT_WIDE_SCOPE and { organization_id: scope } otherwise, so a single projection carries the marker and no organization_id key, and a walled projection carries its organization and no marker. The scope list is a unique symbol (:154), the plugin's [null] is translated at one door (setDeclaredRuleScopes :539-540), and resolveDeclaredRuleScopes returns [null] exactly when the posture enforces no wall (sharing-plugin.ts:94-99), so "the one projection a declared rule is served as under the single posture" is the literal behaviour; a lean embedding is that posture. Stored rows never carry it: rowFromRule (:125-146) copies a fixed field list, defineRule writes fixed columns, no sys_sharing_rule column exists. The narrowing type DeploymentWideSharingRule (:168-170) and the guard isDeploymentWideSharingRule (:178-180, marker AND isDeclaredSharingRule :230-232) are unchanged; the marker still cannot be forged through a stored row.
  • TSDoc (:485-491): every clause verified — set on a DECLARED rule's deployment-sweeping projection, the one served under single; carries no organization_id; absent on every stored row and on per-organization projections under a wall. ADR-0131 D2 and D8 exist (docs/adr/0131-...md:242, :578). One nit, non-blocking: the interface-level comment above it (:477-480, "Stored shape ... Maps 1-to-1 to sys_sharing_rule") is now slightly stale, since the same type serves a declared projection with a non-column key; the member's own TSDoc says so, and no reader is misled.
  • Grading: additive optional member; @objectstack/spec: minor added to the shared changeset frontmatter (.changeset/24583-7a3-sharing-null-readers.md:2) with a bullet stating the contract change (:17). AGENTS.md §3 yes takes at least minor; sharing the feat(...)!: changeset without naming spec in the subject scope is the line's convention (precedents feat(automation)! and feat(plugin-webhooks,runtime)!, both bumping spec minor). No major changeset anywhere.
  • Spec gates: none needs an artifact or an entry. api-surface/contracts.json:297 records SharingRuleRow (interface) as a name-and-kind row only; api-surface-signatures.json hashes defineX factories only (build-api-surface.ts:166-171, every key is define*); export-origins/contracts.json:297 is name to file; the declaration map does not list it. The liveness gate covers authorable schema properties (packages/spec/scripts/liveness/ has no contracts reference) and the ADR-0087 registration gate's own text says a type under packages/spec/src/contracts/ is not an object definition (scripts/check-adr-0087-registration.mjs:230-258), so the existing adr-0087: not-required (no-migration-prescription) arm stands; check:spec-changes is a pure projection of the D2/D3 registries (build-spec-changes.ts:5-7). The dev's check:generated exit 0 with nothing regenerated is what these inputs predict; Spec property liveness, Lint & Repo Gates, Lint · repo & tooling gates, Build Core and Type Check · source gates are green on this head. @objectstack/client re-exports the type from @objectstack/spec/contracts (packages/client/src/index.ts:12-13, 176-177), so the member flows through with no client source change.

2. sharing-rule-service.ts since the PASS — TSDoc only. The whole inter-head diff is three comment lines on DeploymentWideSharingRule (:165-167, "the marker itself is declared on the spec contract ... this type narrows it to present"); main did not touch the file. No executable line moved, so every PASS judgment stands: the marker-cannot-be-forged chain (isDeploymentWideSharingRule requires marker AND managed_by === 'package' && id === name; rowFromRule never copies the marker), the walled per-organization scoping (criteriaContext :1783 — string organization first, marker or lean composition to SYSTEM_CTX second, else refused), and organizationsComposed (:1794-1797) reading the real engine's getSchema.

3. R2 — pinned, both the row and its grants, with something to lose. sharing-rule.test.ts:1299-1309 in the [#7761] block: the organization-less row (defineRule under BOOT, :1236-1240; BOOT carries no organization, :1216) is first evaluated under BOOT and grantsOf(seededId) (:1224-1225, source === 'rule' && source_id === ruleId off the unscoped table) is asserted toHaveLength(1); the engine double's getSchema returns undefined (:75), so organizationsComposed() is false and the lean sweep really grants p_red. Then deleteRule(seededId, ORG1_ADMIN) resolves (no throw), the row is still in sys_sharing_rule and the grant count is still 1. Runtime path: deleteRule :1158-1175 resolves through findRuleRow :1104-1126 (by-id under adminOrgScope, then by-name under the same scope) — both miss, declaredRuleNamed returns null with no declared source, silent return. A regression to an unfiltered by-id lookup inside deleteRule would now purge the grant and red this pin, which is the exposure the PASS named. Residual (small): the by-name address deleteRule(SEEDED-name, ORG1_ADMIN) and a system-context delete of the same row are not pinned; the by-id pin exercises the scoped by-name fallthrough only with a string that matches no row's name.

4. R4 — reconciled, holds 7a-1, keeps 7a-3, weakens nothing.

  • 7a-1's ORG-STAMPED expectation is kept verbatim in both passes: expect(granted).not.toContain('deal_p1'), toEqual(['deal_a1']), matchedRecords 1 (rule-criteria-org-scope.test.ts:307-309); the per-record ORG-STAMPED case grants 0 (:393-417). Every line 7a-1 added that the PR's final diff removes is header prose (the "platform-global sweep", "OR field IS NULL" and "The Layer-0 tenant wall's strict equality annihilates the driver's platform bucket: #2734's fix is defeated on every walled read, and the org-less RBAC catalog reads ZERO for every principal #10103 compatibility note" sentences) — none is an assertion.
  • The roster is identical to the PASS head: 8 cases, the two ORG-STAMPED ones plus the 7a-3 set (DECLARED single x2, DECLARED under a wall, STORED no-organization x2, lean CONTROL; :276, :312, :329, :346, :367, :393, :419, :429).
  • Header: the file now has zero hits for platform-global, IS NULL, OR field and compatibility arm; :32 states the driver turns tenantId into organization_id = ?, and the deal_p1 section (:61-68) states that only the deployment-wide sweep reaches it — pinned at :325 (['deal_p1', null]). The title (:4-5) names the declared rule's deployment-wide projection in place of 7a-1's "platform-global rule".
  • The removed deal_p1 pre-delete is safe and the case is now STRONGER: with deal_p1 present, the DECLARED-under-a-wall case asserts grantsAtRest() equals exactly [deal_a1 ORG_A, deal_b1 ORG_B, deal_b2 ORG_B] (:340-344), so a walled projection reaching the organization-less record would red it. At the merge-base the driver has no NULL arm: SqlDriver.applyTenantScope (sql-driver.ts:15587-15621 at 7c2eab929) emits whereIn(field, tenantIds) or where(field, String(tenantId)) and nothing else, with the v17 OR field IS NULL arm named as retired in its own comment. The PASS's "second interaction" note (RECORD_NOT_FOUND on the second walled projection) is closed the same way: no walled projection matches deal_p1, so none re-reads it.

5. Merge hygiene — clean. 32b3d1c54 merges origin/main at exactly 7c2eab929 (its second parent); every file it changed against the PR side is in main's ddb3360a4..7c2eab929 range (set difference empty). Comparing each PR file's own diff against its own merge-base, 11 of 16 files are byte-identical hunk sets to the PASS head; the 5 that differ are precisely R1 (sharing-service.ts new, sharing-rule-service.ts TSDoc, changeset), R2 (sharing-rule.test.ts) and R4 plus the 7a-1 merge resolution (rule-criteria-org-scope.test.ts). Main's only change to a PR file besides that test is approval-service.ts (#24753, decideAndResume, far from the PR's five hunks at :2582-2851, which hash identical). origin/main has since moved one commit past the merge-base (41646a57), touching none of the PR's files.

6. CI on 236d39daf (40 check-runs): every completed run is success (27, including Build Core, Build Docs, Check Changeset, Governed Surface Queue Guard, Spec property liveness, Dogfood Regression Gate, the three completed Type Check gates, all eight Lint gates and the claim/single-writer guards) or skipped (6 opt-in/matrix placeholders). Still in_progress: Test Core (1/6) through (6/6) and Type Check · workspace. No failed check. This PASS does not stand in for their green: under AGENTS.md's queue rule in_progress is not a pass and a red shard is blocking.

② Semver level

minor on @objectstack/spec (new optional contract member, additive), unchanged minor with the BREAKING bang, the FROM/TO migration and the ADR-0087 disposition on @objectstack/plugin-sharing, @objectstack/plugin-approvals, @objectstack/metadata-protocol. Public surface added since the PASS: SharingRuleRow.deploymentWide?: true. Removed: none. No generated artifact, liveness row or ADR-0087 registry entry is owed for the member.

③ Boundary flags

  • R1: closed. The marker is on the contract, documented, graded minor, with no gate artifact owed.
  • R2: closed for the by-id address, row and grants both asserted. Residual, non-blocking: the by-name address and a system-context delete of the organization-less row are unpinned (about 6 lines in the same block).
  • R4: closed. 7a-1's expectation kept, 7a-3's cases kept, header names neither retired arm, the pre-delete's removal strengthens the walled case.
  • R3 (carried by 7b-0/7b-3): unchanged — the seed loader's NULL holder is unseen, not impossible, until NOT NULL at createTable.
  • Open question 1 of the report: unchanged — the plugin still spells single as [null] at setDeclaredRuleScopes's door, translated at one line.
  • Nit (spec, non-blocking): the interface-level TSDoc of SharingRuleRow (sharing-service.ts:477-480) still reads "Stored shape ... maps 1-to-1 to sys_sharing_rule"; a one-line amendment can ride the next step-7 part that touches the spec contract.
  • CI: Test Core 1/6–6/6 and Type Check · workspace in progress at review time; the lane seat reads them green before queueing.

Implemented-by: claude/issue-24583-7a3-sharing-null-readers
Reviewed-by: session_01Y5583bs2RDd53MH6kM1ca3

VERDICT: PASS

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 10:42
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 20, 2026 10:42
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 7cacad5 Oct 20, 2026
43 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24583-7a3-sharing-null-readers branch October 20, 2026 11:21
This was referenced Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants