From dd65fcaf8cafad5aa017ec2d363544ba5f350d15 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 09:53:19 +0000 Subject: [PATCH 1/6] fix(plugin-sharing): per-grant catch in both reconcile loops so a refused grant no longer aborts the pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After #14484 `sys_record_share` is tenant-scoped in the #13491 ledger, so an organization-less system insert on it is refused loudly with `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` on a walled install. A platform-global sharing rule materialising a grant onto an organization-less record resolves no organization and meets that refusal; with no per-grant catch it propagated and that rule's reconcile pass aborted mid-loop, taking the pass's stale-row revocations with it. Those revocations are the security-relevant half: a stale over-grant persisted across every later pass, which met the same record and died in the same place. Both loops now attempt each grant individually. A refusal is logged with the rule, object, record, recipient and the engine's code, counted in the pass result, and the pass continues. The catch is narrow — only `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` is absorbed. `record-share-organization-stamp.test.ts` deliberately pins the abort on the other error a pass can meet here (the scoped update half answering `RECORD_NOT_FOUND`), a shape the 2026-09-02 contract review left standing; a catch-all would retire that decision as a side effect. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- ...econcile-per-grant-organization-refusal.md | 58 +++ packages/plugins/plugin-sharing/src/index.ts | 4 + .../reconcile-refused-grant-continues.test.ts | 447 ++++++++++++++++++ .../src/sharing-rule-service.ts | 219 ++++++--- 4 files changed, 663 insertions(+), 65 deletions(-) create mode 100644 .changeset/sharing-reconcile-per-grant-organization-refusal.md create mode 100644 packages/plugins/plugin-sharing/src/reconcile-refused-grant-continues.test.ts diff --git a/.changeset/sharing-reconcile-per-grant-organization-refusal.md b/.changeset/sharing-reconcile-per-grant-organization-refusal.md new file mode 100644 index 0000000000..f01b04c0b2 --- /dev/null +++ b/.changeset/sharing-reconcile-per-grant-organization-refusal.md @@ -0,0 +1,58 @@ +--- +"@objectstack/plugin-sharing": minor +--- + +fix(plugin-sharing): one refused grant no longer aborts a sharing rule's reconcile pass — its stale-row revocations still run (#14754) + +After #14484 `sys_record_share` is `tenant-scoped` in the #13491 ledger, so on a +walled install an organization-less system insert on it is refused loudly with +`ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` (#8844). `SharingService.grant` +resolves the organization on every path that can; a platform-global sharing +rule (`organization_id = null`, its sweep unscoped) materialising a grant onto +an organization-LESS record resolves none, and meets that refusal. + +`SharingRuleService.reconcile` / `reconcileForRecord` had no per-grant catch, so +the refusal propagated and **that rule's pass aborted mid-loop**. Two things +were lost, and they are not equally serious: + +- the remaining grants — recoverable, the next pass writes them; +- **the stale-row revocations of that pass** — not recoverable by waiting, + because every subsequent pass meets the same organization-less record and + dies in the same place. A stale over-grant of that rule therefore persisted + indefinitely, and the record kept aborting the pass until it was repaired by + hand. That is the security-relevant half. + +Measured while pinning this, and it sharpens the point: the engine returns +organization-less rows **last** in a rule's criteria sweep (the driver's +NULL-org compatibility arm is appended to the scoped arm). So a refused grant +is nearly always one of the final attempts of a pass, and what an abort +destroyed was hardly ever "the remaining grants" — it was almost entirely the +revoke loop that runs after the whole upsert loop. + +Both loops now attempt each grant individually. A refusal is logged with the +rule, object, record, recipient and the engine's own code, counted, and the +pass **continues** — the remaining grants and, above all, the stale-row +revocations still run. + +**The catch is deliberately narrow.** Only +`ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` is absorbed; every other error +rethrows unchanged. A catch-all would swallow real defects and report a pass +that "completed" having written nothing. It would also silently retire a +reviewed decision: `record-share-organization-stamp.test.ts` deliberately pins +the abort on the OTHER error a reconcile pass can meet here — the scoped update +half answering `RECORD_NOT_FOUND` for a row stamped with a different +organization — which the 2026-09-02 contract review left standing on "loud +beats a wrong count". Those three pins are unchanged and still green. + +**Why `minor` rather than `patch`.** The repair is a bug fix, but it reports +through a new key. `reconcile` / `reconcileForRecord` / `evaluateRule` / +`evaluateAllForRecord` now return `SharingRuleReconcilePassResult` — the spec's +`SharingRuleEvaluationResult` plus `grantsRefused: number` — and that type is +newly exported from the package index. Purely additive: the contract in +`@objectstack/spec` is untouched, its six declared fields are unchanged, and a +consumer typed against `ISharingRuleService` keeps compiling as it did. Same +shape as `fix(runtime): tell an action handler when its caller-scope record load +was refused` (#14143), which shipped `minor` for the same reason. + +`grantsRefused > 0` does **not** mean the pass failed. It means the pass met a +record it cannot grant on and carried on — which is the whole point. diff --git a/packages/plugins/plugin-sharing/src/index.ts b/packages/plugins/plugin-sharing/src/index.ts index abc2b3aa9b..7fce572acc 100644 --- a/packages/plugins/plugin-sharing/src/index.ts +++ b/packages/plugins/plugin-sharing/src/index.ts @@ -23,6 +23,10 @@ export { export { SharingRuleService, type SharingRuleServiceOptions, + // [#14754] The spec's `SharingRuleEvaluationResult` plus the pass's + // `grantsRefused` count. Additive: the six declared fields are unchanged, and + // a consumer typed against the spec contract keeps compiling untouched. + type SharingRuleReconcilePassResult, } from './sharing-rule-service.js'; export { ShareLinkService, diff --git a/packages/plugins/plugin-sharing/src/reconcile-refused-grant-continues.test.ts b/packages/plugins/plugin-sharing/src/reconcile-refused-grant-continues.test.ts new file mode 100644 index 0000000000..36a7dbf501 --- /dev/null +++ b/packages/plugins/plugin-sharing/src/reconcile-refused-grant-continues.test.ts @@ -0,0 +1,447 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#14754] One refused grant no longer takes a rule's whole reconcile pass with + * it — and, above all, no longer takes that pass's STALE-ROW REVOCATIONS. + * + * ## The shape, and which half of it is the security half + * + * After #14484 `sys_record_share` is `tenant-scoped` in the #13491 ledger, so + * on a walled install an organization-less system insert on it is refused + * loudly with `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` (#8844). + * `SharingService.grant` resolves the organization on every path that can; a + * platform-global rule (`organization_id = null`, its sweep unscoped) + * materialising onto an organization-LESS record resolves none, and meets the + * refusal. + * + * Before this card the refusal propagated out of the reconcile loop and that + * rule's pass ABORTED mid-loop. Two things were lost, and they are not equally + * serious: + * + * - the remaining grants — recoverable, the next pass writes them; + * - **the stale-row revocations of that pass** — NOT recoverable by waiting, + * because every subsequent pass meets the same organization-less record and + * dies at the same place. A stale over-grant of that rule therefore persists + * indefinitely. That is the security half, and it is pinned SEPARATELY + * below: a catch that swallowed the refusal and then skipped the revocation + * anyway would satisfy the "pass continued" half while leaving the defect + * exactly where it was. + * + * ## Why a real driver and a real engine + * + * The refusal under test is the ENGINE's, raised inside + * `Engine.resolveSystemInsertOrganization` from the ledger classification and + * the deployment posture. A fake engine would have to imitate the very thing + * whose behaviour decides the case. So these cases run a real `SqlDriver` on + * better-sqlite3 `:memory:` behind a real `ObjectQL` on an `isolated` posture, + * the way `record-share-organization-stamp.test.ts` does for the #14484 stamp. + * + * ## Why the fixture's organization-less record is in the MIDDLE + * + * "The pass continued" is only observable if a grant was still ATTEMPTED after + * the refused one. The sweep returns the matched records in insertion order, so + * the fixture inserts `rec_orgless` second of three matching records — and the + * attempt order is asserted rather than assumed, because a fixture that + * silently reordered would turn this pin into a tautology. + * + * ## The catch is NARROW, and one case here holds that line + * + * Only `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` is absorbed. The last case + * feeds the loop a DIFFERENT engine error and pins that it still propagates — + * the same direction `record-share-organization-stamp.test.ts` pins for the + * scoped-update `RECORD_NOT_FOUND` shape the 2026-09-02 review left standing. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; + +import { SharingService } from './sharing-service.js'; +import { SharingRuleService } from './sharing-rule-service.js'; + +const OBJECT = 'os14754_deal'; +const ORG_A = 'org_a'; +const ORG_B = 'org_b'; + +const SYSTEM: ExecutionContext = { isSystem: true, positions: [], permissions: [] }; +const WON = { stage: 'won' }; + +const DEAL_FIELDS: Record> = { + id: { type: 'text', name: 'id', label: 'Id', primary: true }, + stage: { type: 'text', name: 'stage', label: 'Stage' }, + owner_id: { type: 'text', name: 'owner_id', label: 'Owner' }, + organization_id: { type: 'text', name: 'organization_id', label: 'Org' }, +}; + +const SHARE_FIELDS: Record> = { + id: { type: 'text', name: 'id', label: 'Id', primary: true }, + organization_id: { type: 'text', name: 'organization_id', label: 'Org' }, + object_name: { type: 'text', name: 'object_name', label: 'Object' }, + record_id: { type: 'text', name: 'record_id', label: 'Record' }, + recipient_type: { type: 'text', name: 'recipient_type', label: 'Recipient type' }, + recipient_id: { type: 'text', name: 'recipient_id', label: 'Recipient' }, + access_level: { type: 'text', name: 'access_level', label: 'Access' }, + source: { type: 'text', name: 'source', label: 'Source' }, + source_id: { type: 'text', name: 'source_id', label: 'Source id' }, + reason: { type: 'text', name: 'reason', label: 'Reason' }, + granted_by: { type: 'text', name: 'granted_by', label: 'Grantor' }, + created_at: { type: 'text', name: 'created_at', label: 'Created' }, + updated_at: { type: 'text', name: 'updated_at', label: 'Updated' }, +}; + +const RULE_FIELDS: Record> = { + id: { type: 'text', name: 'id', label: 'Id', primary: true }, + organization_id: { type: 'text', name: 'organization_id', label: 'Org' }, + name: { type: 'text', name: 'name', label: 'Name' }, + label: { type: 'text', name: 'label', label: 'Label' }, + description: { type: 'text', name: 'description', label: 'Description' }, + object_name: { type: 'text', name: 'object_name', label: 'Object' }, + criteria_json: { type: 'text', name: 'criteria_json', label: 'Criteria' }, + recipient_type: { type: 'text', name: 'recipient_type', label: 'Recipient type' }, + recipient_id: { type: 'text', name: 'recipient_id', label: 'Recipient' }, + access_level: { type: 'text', name: 'access_level', label: 'Access' }, + active: { type: 'boolean', name: 'active', label: 'Active' }, + managed_by: { type: 'text', name: 'managed_by', label: 'Managed by' }, + customized: { type: 'boolean', name: 'customized', label: 'Customized' }, + created_at: { type: 'text', name: 'created_at', label: 'Created' }, + updated_at: { type: 'text', name: 'updated_at', label: 'Updated' }, +}; + +interface ShareRow { + id: string; + record_id: string; + recipient_id: string; + organization_id: string | null; + source_id: string | null; +} + +const open: SqlDriver[] = []; + +/** + * Five records. MEASURED sweep order (not insertion order — see below): + * + * rec_first ORG_A won grant lands + * rec_last ORG_B won grant lands + * rec_orgless (null) won grant REFUSED on a walled posture + * rec_orgless_2 (null) won grant REFUSED — attempted ONLY if the pass continued + * rec_stale ORG_A lost not matched; carries the stale row that must be REVOKED + * + * ⚠️ Measured on this fixture: the engine returns the organization-LESS rows + * LAST, after every organization-carrying row, whatever the insertion order + * (raw driver order is `rec_first, rec_orgless, rec_last, …`; the engine's + * filtered read answers `rec_first, rec_last, rec_orgless, …`). That is the + * driver's NULL-org compatibility arm being appended to the scoped arm, the + * same "and — through the driver's compatibility arm — deal_p1, in that order" + * `record-share-organization-stamp.test.ts` records. + * + * The consequence matters for this card and is why the fixture holds TWO + * organization-less records: on the real shape a refused grant is usually one + * of the LAST attempts of the pass, so what an abort destroyed was hardly ever + * "the remaining grants" — it was almost entirely the STALE-ROW REVOCATIONS + * that run after the whole upsert loop. The security half is not merely the + * more serious half, it is very nearly the ONLY half. A second refusal is + * therefore the order-independent witness that the loop survived the first. + */ +async function boot(posture: 'single' | 'isolated' | 'group' = 'isolated') { + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + open.push(driver); + + const ql = new ObjectQL(); + ql.registerDriver(driver as never, true); + await ql.init(); + ql.setTenancyPostureProvider(() => posture); + ql.registerObject({ name: OBJECT, label: 'Deal', sharingModel: 'private', fields: DEAL_FIELDS } as never); + ql.registerObject({ name: 'sys_record_share', label: 'Record Share', isSystem: true, fields: SHARE_FIELDS } as never); + ql.registerObject({ name: 'sys_sharing_rule', label: 'Sharing Rule', isSystem: true, fields: RULE_FIELDS } as never); + await driver.initObjects([ + { name: OBJECT, fields: DEAL_FIELDS } as never, + { name: 'sys_record_share', fields: SHARE_FIELDS } as never, + { name: 'sys_sharing_rule', fields: RULE_FIELDS } as never, + ]); + + await driver.create(OBJECT, { id: 'rec_first', stage: 'won', owner_id: 'u_a', organization_id: ORG_A } as never); + await driver.create(OBJECT, { id: 'rec_orgless', stage: 'won', owner_id: 'u_p' } as never); + await driver.create(OBJECT, { id: 'rec_last', stage: 'won', owner_id: 'u_b', organization_id: ORG_B } as never); + await driver.create(OBJECT, { id: 'rec_orgless_2', stage: 'won', owner_id: 'u_p' } as never); + await driver.create(OBJECT, { id: 'rec_stale', stage: 'lost', owner_id: 'u_a', organization_id: ORG_A } as never); + + const warn = vi.fn(); + const sharing = new SharingService({ engine: ql as never }); + const rules = new SharingRuleService({ engine: ql as never, sharing, logger: { warn } }); + + return { + driver, + ql, + sharing, + rules, + warn, + /** Every `sys_record_share` row, read straight off the driver. */ + shares: async (): Promise => { + const rows = (await driver.find('sys_record_share', {} as never)) as any[]; + return rows + .map((r) => ({ + id: String(r.id), + record_id: String(r.record_id), + recipient_id: String(r.recipient_id), + organization_id: r.organization_id ?? null, + source_id: r.source_id ?? null, + })) + .sort((a, b) => a.record_id.localeCompare(b.record_id) || a.recipient_id.localeCompare(b.recipient_id)); + }, + /** A pre-existing rule grant the next pass should find stale and revoke. */ + seedStaleRow: (id: string, recordId: string, ruleId: string, recipientId = 'u_plat', organizationId: string | null = ORG_A) => + driver.create('sys_record_share', { + id, + object_name: OBJECT, + record_id: recordId, + recipient_type: 'user', + recipient_id: recipientId, + access_level: 'read', + source: 'rule', + source_id: ruleId, + ...(organizationId ? { organization_id: organizationId } : {}), + created_at: '2026-01-01T00:00:00Z', + } as never), + /** Records the recordId of every grant ATTEMPT, in order, and calls through. */ + traceGrants: () => { + const attempted: string[] = []; + const original = sharing.grant.bind(sharing); + vi.spyOn(sharing, 'grant').mockImplementation((async (input: any, ctx: any) => { + attempted.push(String(input.recordId)); + return original(input, ctx); + }) as never); + return attempted; + }, + }; +} + +afterEach(async () => { + vi.restoreAllMocks(); + while (open.length) await open.pop()?.disconnect?.(); +}); + +const platformGlobalRule = (rules: SharingRuleService, name: string, accessLevel: 'read' | 'edit' = 'read') => + rules.defineRule( + { + name, + label: 'Platform won', + object: OBJECT, + criteria: WON, + recipientType: 'user', + recipientId: 'u_plat', + accessLevel, + } as never, + SYSTEM, + ); + +describe('[#14754] reconcile: a refused grant is counted and the pass CONTINUES', () => { + it('CONTROL: on an `isolated` posture the organization-less record really is refused — the precondition, measured', async () => { + const { ql, shares } = await boot('isolated'); + await expect( + ql.insert( + 'sys_record_share', + { + id: 'shr_bare', + object_name: OBJECT, + record_id: 'rec_orgless', + recipient_type: 'user', + recipient_id: 'u_plat', + access_level: 'read', + source: 'rule', + }, + { context: SYSTEM } as never, + ), + ).rejects.toMatchObject({ code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED', status: 500 }); + expect(await shares()).toEqual([]); + }); + + it('HALF 1 — the pass does not abort: the refusal is counted, and the grant AFTER it still lands', async () => { + const { rules, shares } = await boot('isolated'); + const rule = await platformGlobalRule(rules, 'os14754_continues'); + expect(rule.organization_id).toBeNull(); + + const result = await rules.evaluateRule(rule.id, SYSTEM); + + // It RESOLVED. Before this card it REJECTED with the engine's refusal. + expect(result.grantsRefused).toBe(2); + expect(result.grantsCreated).toBe(2); + expect(result.matchedRecords).toBe(4); + + // Both organization-carrying records got their grant; neither refused one has any. + const rows = await shares(); + expect(rows.map((r) => [r.record_id, r.organization_id])).toEqual([ + ['rec_first', ORG_A], + ['rec_last', ORG_B], + ]); + }); + + it('HALF 1, continuation witness — a grant is still ATTEMPTED after the first refusal', async () => { + const booted = await boot('isolated'); + const rule = await platformGlobalRule(booted.rules, 'os14754_order'); + const attempted = booted.traceGrants(); + + const result = await booted.rules.evaluateRule(rule.id, SYSTEM); + + // The measured order — organization-carrying rows first, the NULL-org + // compatibility arm appended. Asserted rather than assumed, so a change in + // the engine's read shape shows up here instead of quietly turning the + // relation below into a tautology. + expect(attempted).toEqual(['rec_first', 'rec_last', 'rec_orgless', 'rec_orgless_2']); + + // The load-bearing relation: the FIRST refusal is not the last attempt of + // the pass, and the attempt after it was made. A pass that aborted at the + // first refusal could not have attempted `rec_orgless_2` at all, and could + // not have counted two refusals. + const firstRefusalAt = attempted.indexOf('rec_orgless'); + expect(firstRefusalAt).toBeLessThan(attempted.length - 1); + expect(result.grantsRefused).toBe(2); + }); + + it('HALF 2 (the security half) — the STALE ROW of that same pass is REVOKED', async () => { + const { rules, shares, seedStaleRow } = await boot('isolated'); + const rule = await platformGlobalRule(rules, 'os14754_revokes'); + + // `rec_stale` is `lost`, so the rule does not match it: a completed pass + // revokes its leftover grant. It is the witness that the pass reached its + // revoke loop at all. + await seedStaleRow('shr_stale', 'rec_stale', rule.id); + expect((await shares()).map((r) => r.id)).toEqual(['shr_stale']); + + const result = await rules.evaluateRule(rule.id, SYSTEM); + + expect(result.grantsRevoked).toBe(1); + expect(result.grantsRefused).toBe(2); + // The row is GONE from the table — not merely counted. + const rows = await shares(); + expect(rows.map((r) => r.id)).not.toContain('shr_stale'); + expect(rows.map((r) => r.record_id)).toEqual(['rec_first', 'rec_last']); + }); + + it('the refusal is LOGGED with the rule, object, record and the engine code', async () => { + const { rules, warn } = await boot('isolated'); + const rule = await platformGlobalRule(rules, 'os14754_logs'); + + await rules.evaluateRule(rule.id, SYSTEM); + + const refusals = warn.mock.calls.filter((c) => String(c[0]).includes('refused by the engine organization rule')); + // One line per refused GRANT, naming the record — the operator's only route + // from "this rule reports refusals" to "these are the records to repair". + expect(refusals).toHaveLength(2); + expect(refusals.map((c) => (c[1] as any).record)).toEqual(['rec_orgless', 'rec_orgless_2']); + expect(refusals[0]![1]).toMatchObject({ + rule: 'os14754_logs', + object: OBJECT, + record: 'rec_orgless', + recipient: 'u_plat', + code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED', + }); + }); + + it('MEASURED BOUNDARY — the refusal gates the INSERT half only: an organization-less row that ALREADY exists is UPDATED, not refused', async () => { + const { driver, rules, shares } = await boot('isolated'); + const rule = await platformGlobalRule(rules, 'os14754_update_half', 'read'); + + // A pre-existing rule grant on the organization-less record, carrying no + // organization (which is how it got there before #14484). The pass wants + // to raise it to `edit`. + await driver.create('sys_record_share', { + id: 'shr_orgless_old', object_name: OBJECT, record_id: 'rec_orgless', recipient_type: 'user', + recipient_id: 'u_plat', access_level: 'read', source: 'rule', source_id: rule.id, + created_at: '2026-01-01T00:00:00Z', + } as never); + + const raised = await rules.defineRule( + { name: 'os14754_update_half', label: 'Platform won', object: OBJECT, criteria: WON, recipientType: 'user', recipientId: 'u_plat', accessLevel: 'edit' } as never, + SYSTEM, + ); + expect(raised.id).toBe(rule.id); + + const result = await rules.evaluateRule(rule.id, SYSTEM); + + // The engine's rule refuses an INSERT ("Insert on '' was + // REFUSED"); the update half is not gated by it, and `grant` sends an + // organization-less update UNSCOPED (`tenantId: undefined`), which reaches + // the NULL row. So `rec_orgless` is UPDATED here while `rec_orgless_2` — + // which has no row yet — is refused. Measured, and recorded because it + // bounds this card: a refusal cannot reach the `cur` (update) branch of + // either loop today. + const rows = await shares(); + const updatedRow = rows.find((r) => r.id === 'shr_orgless_old'); + expect(updatedRow).toBeDefined(); + expect(result.grantsUpdated).toBe(1); + expect(result.grantsRefused).toBe(1); + expect(result.grantsRevoked).toBe(0); + }); + + it('an unrelated engine error is NOT absorbed — the catch is narrow, and stays narrow', async () => { + const { ql, rules } = await boot('isolated'); + const rule = await platformGlobalRule(rules, 'os14754_narrow'); + + // A different failure on the same seam: the grant's own insert blows up + // with something that is not the organization refusal. + const originalInsert = ql.insert.bind(ql); + vi.spyOn(ql, 'insert').mockImplementation((async (object: string, doc: any, options?: any) => { + if (object === 'sys_record_share') { + const err: any = new Error('simulated driver outage'); + err.code = 'ERR_DRIVER_UNAVAILABLE'; + throw err; + } + return originalInsert(object, doc, options); + }) as never); + + await expect(rules.evaluateRule(rule.id, SYSTEM)).rejects.toMatchObject({ code: 'ERR_DRIVER_UNAVAILABLE' }); + }); +}); + +describe('[#14754] reconcileForRecord: the same two halves on the per-record hook path', () => { + it('HALF 2 — the refused grant does not stop that record\'s OWN stale revocation', async () => { + const { rules, shares, seedStaleRow } = await boot('isolated'); + const rule = await platformGlobalRule(rules, 'os14754_hook_revokes'); + + // A recipient the rule no longer expands to, on the organization-less + // record itself. The pass's grant for `u_plat` is refused; `u_gone`'s row + // is stale and must still go. + await seedStaleRow('shr_hook_stale', 'rec_orgless', rule.id, 'u_gone', null); + expect((await shares()).map((r) => r.id)).toEqual(['shr_hook_stale']); + + const [result] = await rules.evaluateAllForRecord(OBJECT, 'rec_orgless', SYSTEM); + + expect(result).toMatchObject({ grantsRefused: 1, grantsRevoked: 1, grantsCreated: 0 }); + expect(await shares()).toEqual([]); + }); + + it('HALF 1 — one rule refused no longer aborts the whole per-record sweep: the NEXT rule still reconciles', async () => { + const { rules, shares, seedStaleRow } = await boot('isolated'); + const refusing = await platformGlobalRule(rules, 'os14754_hook_a'); + const following = await platformGlobalRule(rules, 'os14754_hook_b', 'edit'); + + // The second rule's own stale row on the same record — reachable only if + // the first rule's refusal did not abort `evaluateAllForRecord`. + await seedStaleRow('shr_hook_b_stale', 'rec_orgless', following.id, 'u_gone', null); + + const results = await rules.evaluateAllForRecord(OBJECT, 'rec_orgless', SYSTEM); + + expect(results).toHaveLength(2); + expect(results.map((r) => r.ruleId)).toEqual([refusing.id, following.id]); + expect(results.every((r) => r.grantsRefused === 1)).toBe(true); + expect(results[1]!.grantsRevoked).toBe(1); + expect(await shares()).toEqual([]); + }); + + it('an organization-CARRYING record on the same posture is untouched by any of this', async () => { + const { rules, shares } = await boot('isolated'); + const rule = await platformGlobalRule(rules, 'os14754_hook_control'); + + const [result] = await rules.evaluateAllForRecord(OBJECT, 'rec_first', SYSTEM); + + expect(result).toMatchObject({ grantsRefused: 0, grantsCreated: 1 }); + expect((await shares()).map((r) => [r.record_id, r.organization_id, r.source_id])).toEqual([ + ['rec_first', ORG_A, rule.id], + ]); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts index 42a94b415a..dc843b134f 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts @@ -21,6 +21,14 @@ import { BUILTIN_IDENTITY_PLATFORM_ADMIN } from '@objectstack/spec/identity'; // — one honouring the 1/0 and 'false' storage shapes, one not — is how the // enforcement hole this closes gets re-opened one seam over. import { isRowActive } from '@objectstack/core'; +// [#14754] The engine's organization refusal for a system write on a +// tenant-scoped object (#8844). Imported as a TYPE only: the runtime check is a +// `code` compare, which is the convention that class itself documents ("a +// caller that catches it identifies it by `code`") so the check survives the +// package boundary where two copies of that module can exist. Typing the +// literal FROM the class is what keeps the two spellings from drifting — a +// typo here would not be a failing test, it would be a catch that never fires. +import type { SystemWriteOrganizationRequiredError } from '@objectstack/objectql'; import type { SharingEngine } from './sharing-service.js'; import type { SharingService } from './sharing-service.js'; import { normalizeAccessLevel, normalizeStoredAccessLevel } from './access-level.js'; @@ -70,6 +78,35 @@ function rowFromRule(row: any): SharingRuleRow { }; } +/** + * [#14754] The one engine refusal a reconcile pass absorbs per grant. + * + * Spelled once, and typed from the engine's own declaration so it cannot drift + * from the code the engine actually throws. + */ +const ENGINE_ORGANIZATION_REFUSAL_CODE: SystemWriteOrganizationRequiredError['code'] = + 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED'; + +/** + * [#14754] What one reconcile pass did, plus the grants the engine's + * organization rule REFUSED in it. + * + * Extends the spec's {@link SharingRuleEvaluationResult} rather than changing + * it: the contract lives in `@objectstack/spec` and is another lane's to move, + * and every declared consumer keeps compiling against the six fields it always + * had. The seventh is additive and rides along for the callers that want it — + * the boot backfill's aggregate and this package's own pins. + * + * ⛔ `grantsRefused > 0` is NOT "the pass failed". It is the pass reporting that + * it met a record it cannot grant on (an organization-less record under a + * walled posture) and CONTINUED — which is the whole point: the remaining + * grants and, above all, the stale-row revocations still ran. + */ +export interface SharingRuleReconcilePassResult extends SharingRuleEvaluationResult { + /** Grants the engine's organization rule refused; each one is also logged. */ + grantsRefused: number; +} + export interface SharingRuleServiceOptions { engine: SharingEngine; sharing: SharingService; @@ -655,14 +692,15 @@ export class SharingRuleService implements ISharingRuleService { } as any); } - async evaluateRule(idOrName: string, context: ExecutionContext): Promise { + async evaluateRule(idOrName: string, context: ExecutionContext): Promise { this.assertCanManageRules(context); // [ADR-0111 D6] const rule = await this.getRule(idOrName, context); if (!rule) throw new Error('RULE_NOT_FOUND'); if (!rule.active) { // Inactive — purge any leftover grants and report revoke count. const revoked = await this.purgeRuleGrants(rule.id); - return { ruleId: rule.id, matchedRecords: 0, expandedUsers: 0, grantsCreated: 0, grantsUpdated: 0, grantsRevoked: revoked }; + // [#14754] An inactive rule attempts no grant, so it can refuse none. + return { ruleId: rule.id, matchedRecords: 0, expandedUsers: 0, grantsCreated: 0, grantsUpdated: 0, grantsRevoked: revoked, grantsRefused: 0 }; } const matches = await this.findMatchingRecords(rule); const users = await this.expandRecipient(rule); @@ -749,10 +787,10 @@ export class SharingRuleService implements ISharingRuleService { object: string, recordId: string, context: ExecutionContext, - ): Promise { + ): Promise { const rules = await this.listRules({ object }, context); if (rules.length === 0) return []; - const results: SharingRuleEvaluationResult[] = []; + const results: SharingRuleReconcilePassResult[] = []; // [#8710] ONE pass, so N rules naming the same position pay ONE catalogue // read — and the memo dies with this call, so the next pass re-reads and a // deactivation is honoured immediately. @@ -1236,11 +1274,86 @@ export class SharingRuleService implements ISharingRuleService { } } + /** + * [#14754] Materialise ONE rule grant, absorbing the engine's organization + * refusal so the pass survives it. + * + * ## The defect this closes, and which half of it is the security half + * + * `sys_record_share` is tenant-scoped in the #13491 ledger (#14484), so on a + * walled install an organization-less system insert on it is refused loudly + * with {@link ENGINE_ORGANIZATION_REFUSAL_CODE}. `SharingService.grant` + * resolves the organization on every path that can; a platform-global rule + * (`organization_id = null`, its sweep unscoped) materialising onto an + * organization-less record resolves none, and meets the refusal. + * + * Without this catch the refusal propagated out of the reconcile loop and + * that rule's pass ABORTED mid-loop. The grants already written stayed; the + * remaining grants — and **the stale-row revocations of that pass** — did + * not happen. The revocation half is the security-relevant one: a stale + * over-grant of that rule persisted across every pass that met the same + * record, and that record kept aborting the pass until it was repaired. + * + * ## Why the catch is narrow, and must stay narrow + * + * ONLY {@link ENGINE_ORGANIZATION_REFUSAL_CODE} is absorbed; everything else + * rethrows unchanged. Two reasons, and the second is measured: + * + * - A catch-all would swallow real defects — a driver outage, a criteria + * bug, a permission failure — and report a pass that "completed" having + * written nothing, which is the silence #6783 spent a card removing. + * - `record-share-organization-stamp.test.ts` DELIBERATELY pins the abort on + * the OTHER error a reconcile pass can meet here: the scoped update half + * answering `RECORD_NOT_FOUND` (404) for a row stamped with a different + * organization. That shape was reviewed on 2026-09-02 and left standing + * ("loud beats a wrong count"). Widening this catch to any error would + * retire that decision as a side effect, and its three pins are what + * notices. + * + * Returns `true` when the grant landed, `false` when it was refused — + * never throws for the refusal, and the caller counts what it is told. + */ + private async grantOrAbsorbOrganizationRefusal( + rule: SharingRuleRow, + recordId: string, + recipientId: string, + ): Promise { + try { + await this.sharing.grant( + { + object: rule.object_name, + recordId, + recipientType: 'user', + recipientId, + accessLevel: rule.access_level, + source: 'rule', + sourceId: rule.id, + reason: `rule:${rule.name}`, + } as any, + this.criteriaContext(rule), + ); + return true; + } catch (err: any) { + if (err?.code !== ENGINE_ORGANIZATION_REFUSAL_CODE) throw err; + this.logger?.warn?.( + '[sharing-rule] grant refused by the engine organization rule — counted, pass continues', + { + rule: rule.name ?? rule.id, + object: rule.object_name, + record: recordId, + recipient: recipientId, + code: ENGINE_ORGANIZATION_REFUSAL_CODE, + }, + ); + return false; + } + } + private async reconcile( rule: SharingRuleRow, matchedIds: string[], users: string[], - ): Promise { + ): Promise { const existing = await this.engine.find('sys_record_share', { where: { source: 'rule', source_id: rule.id }, fields: ['id', 'record_id', 'recipient_id', 'access_level'], @@ -1257,43 +1370,30 @@ export class SharingRuleService implements ISharingRuleService { let created = 0; let updated = 0; let revoked = 0; + let refused = 0; - // Upsert desired. + // Upsert desired. [#14754] Per grant, not per pass: a refusal is counted + // and the loop goes on, so the revoke loop below is REACHED. for (const [k, want] of desired.entries()) { const cur = existingMap.get(k); if (cur) { + // [#14754] Taken out of the stale set BEFORE the grant is attempted. + // The row is DESIRED; a refused level update must leave it standing. + // Deleting it only on success would drop it through to the revoke loop + // and retract a grant the rule still wants — this card's defect + // inverted, and strictly worse than the abort it replaces. + existingMap.delete(k); if (cur.access_level !== rule.access_level) { - await this.sharing.grant( - { - object: rule.object_name, - recordId: want.record_id, - recipientType: 'user', - recipientId: want.recipient_id, - accessLevel: rule.access_level, - source: 'rule', - sourceId: rule.id, - reason: `rule:${rule.name}`, - } as any, - this.criteriaContext(rule), - ); - updated += 1; + if (await this.grantOrAbsorbOrganizationRefusal(rule, want.record_id, want.recipient_id)) { + updated += 1; + } else { + refused += 1; + } } - existingMap.delete(k); - } else { - await this.sharing.grant( - { - object: rule.object_name, - recordId: want.record_id, - recipientType: 'user', - recipientId: want.recipient_id, - accessLevel: rule.access_level, - source: 'rule', - sourceId: rule.id, - reason: `rule:${rule.name}`, - } as any, - this.criteriaContext(rule), - ); + } else if (await this.grantOrAbsorbOrganizationRefusal(rule, want.record_id, want.recipient_id)) { created += 1; + } else { + refused += 1; } } // Revoke stale. @@ -1309,6 +1409,7 @@ export class SharingRuleService implements ISharingRuleService { grantsCreated: created, grantsUpdated: updated, grantsRevoked: revoked, + grantsRefused: refused, }; } @@ -1317,7 +1418,7 @@ export class SharingRuleService implements ISharingRuleService { recordId: string, match: boolean, users: string[], - ): Promise { + ): Promise { const existing = await this.engine.find('sys_record_share', { where: { source: 'rule', source_id: rule.id, record_id: recordId }, fields: ['id', 'record_id', 'recipient_id', 'access_level'], @@ -1330,43 +1431,30 @@ export class SharingRuleService implements ISharingRuleService { let created = 0; let updated = 0; let revoked = 0; + let refused = 0; if (match) { + // [#14754] Per grant, exactly as in {@link reconcile}: one recipient the + // engine's organization rule refuses no longer takes the rest of this + // record's pass — nor its revoke loop — with it. for (const userId of users) { const cur = existingMap.get(userId); if (cur) { + // [#14754] Out of the stale set BEFORE the attempt — see the twin + // comment in {@link reconcile}: a refused level update must leave the + // desired row standing, never fall through to the revoke loop. + existingMap.delete(userId); if (cur.access_level !== rule.access_level) { - await this.sharing.grant( - { - object: rule.object_name, - recordId, - recipientType: 'user', - recipientId: userId, - accessLevel: rule.access_level, - source: 'rule', - sourceId: rule.id, - reason: `rule:${rule.name}`, - } as any, - this.criteriaContext(rule), - ); - updated += 1; + if (await this.grantOrAbsorbOrganizationRefusal(rule, recordId, userId)) { + updated += 1; + } else { + refused += 1; + } } - existingMap.delete(userId); - } else { - await this.sharing.grant( - { - object: rule.object_name, - recordId, - recipientType: 'user', - recipientId: userId, - accessLevel: rule.access_level, - source: 'rule', - sourceId: rule.id, - reason: `rule:${rule.name}`, - } as any, - this.criteriaContext(rule), - ); + } else if (await this.grantOrAbsorbOrganizationRefusal(rule, recordId, userId)) { created += 1; + } else { + refused += 1; } } } @@ -1384,6 +1472,7 @@ export class SharingRuleService implements ISharingRuleService { grantsCreated: created, grantsUpdated: updated, grantsRevoked: revoked, + grantsRefused: refused, }; } From 7919e803667f4a66ada8e204d83545f3ee976a39 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 10:28:46 +0000 Subject: [PATCH 2/6] docs(permissions): re-anchor the system-context census to the two shifted elevation reads `check:check-system-context-census` failed with pure line rot: the per-grant catch added 37 lines above `sharing-rule-service.ts`'s two `context?.isSystem` elevation reads, so the census page's anchors pointed at :157/:382 while the reads now sit at :194/:419. Re-anchored with the gate's own `--fix`; no prose and no behaviour changed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- content/docs/permissions/system-context.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 6990eb7383..120559d401 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -137,7 +137,7 @@ The largest single consumer — **20 of the 109 sites**. | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` | | 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:449`, `:503`, `:507`, `:580`, `:610` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | -| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:157`, `:382` | +| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:194`, `:419` | ### 4. Approvals, reports, attachments, comments, knowledge From 83f307090e21896c372e9fa0d5f59129024c5ec3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 02:08:03 +0000 Subject: [PATCH 3/6] docs(permissions): re-derive the row-39 census anchors on the merged tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge with origin/main moved `sharing-rule-service.ts` — main added the `warnOnEmptyUnitExpansion` limb above both elevation reads — so the anchors this branch re-anchored before the merge point at the pre-merge lines. Machine re-derived with `pnpm gen:system-context-census`; nothing hand-edited. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --- content/docs/permissions/system-context.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 6d3cdd1631..a8462c12e6 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -137,7 +137,7 @@ The largest single consumer — **17 of the 106 sites**. | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1088` | | 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:469`, `:523`, `:527`, `:600`, `:630` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | -| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:194`, `:419` | +| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:202`, `:427` | ### 4. Approvals, reports, attachments, comments, knowledge From 48788814c7bcbae0a92d5dea4dc43b91fbb42bc3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 05:09:15 +0000 Subject: [PATCH 4/6] docs(permissions): re-derive the system-context census anchors on the merged tree The os-regen merge driver does not text-merge this file. Regenerated with `pnpm gen:system-context-census` on the committed merge, as its own commit: main's line rot (rows 21, 56, 63 and the misconception table) is re-anchored, and row 39's branch anchors are re-derived unchanged. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WXyGTWPbbreqXow7Z2pZCk --- content/docs/permissions/system-context.mdx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index a8462c12e6..286f578dc6 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -112,7 +112,7 @@ that silently does not happen. | 18 | **`readonly` strip bypassed — UPDATE, single row** | objectql | Get: a `readonly` field CAN be written. Lose: the protection that stops a caller seeding e.g. `approval_status` | `objectql/src/engine.ts:11290` | | 19 | **`readonly` strip bypassed — UPDATE, bulk/predicate** | objectql | Same, on the multi-row path | `objectql/src/engine.ts:11473` | | 20 | **`readonly` strip bypassed — INSERT (engine pass)** | objectql | Same, on create | `objectql/src/engine.ts:10025` | -| 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1747` | +| 21 | **`readonly` strip bypassed — INSERT (protocol ingress)** | metadata-protocol | `isSystem` is the **only** exemption here. `preserveAudit` is deliberately not read on this path (#6640) — a non-system historical import is still stripped on create | `metadata-protocol/src/protocol.ts:1795` | | 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing | `objectql/src/engine.ts:10073`, `readonly-strict-errors.ts:66` | | 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `objectql/src/engine.ts:5892` | | 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `objectql/src/engine.ts:3736`, `:3746`, `:3773` | @@ -164,7 +164,7 @@ The largest single consumer — **17 of the 106 sites**. | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` | | 54 | Package REST route capability gate bypassed | rest | Get: package read/write over REST without `manage_metadata` / `studio.access` / `setup.access` | `package-routes.ts:102` | | 55 | Package domain capability gates bypassed | runtime | Get: package management and package-inventory reads without the capability | `domains/packages.ts:145`, `:178` | -| 56 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `activation-gate.ts:139`, `:190` | +| 56 | Activation write / authoring refusals do not fire | runtime | Get: activation artifacts writable and authorable without the activation-authoring capability | `activation-gate.ts:138`, `:189` | | 57 | Automation run-state read, flow-authoring write and unrelated-screen read all pass | runtime | Get: run state, flow writes and screen reads with no grant | `domains/automation.ts:254`, `:545`, `:635` | | 58 | Audience-binding suggestion recording skipped | plugin-security | Lose: install-time suggestions are not recorded for system callers | `suggested-audience-bindings.ts:703` | | 59 | Email-template / webhook provenance stamps skipped | plugin-email, plugin-webhooks | Lose: the row is not marked as an admin customization | `email-template-provenance.ts:59`, `webhook-provenance.ts:50` | @@ -180,7 +180,7 @@ a reader tracing where elevation travels needs them. | # | Site | Package | What it does | |:--|:---|:---|:---| | 62 | `objectql/src/engine.ts:3543` | objectql | Propagates `isSystem` into the hook session so hooks can tell engine self-writes from user writes | -| 63 | `objectql/src/engine.ts:14463` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | +| 63 | `objectql/src/engine.ts:14523` | objectql | `ScopedContext.isSystem` getter — re-exposes the underlying execution context's flag | | 64 | `plugin-reports/src/report-service.ts:556` | plugin-reports | Threads the flag into the engine call that runs a report | | 65 | `body-runner.ts:279` | runtime | Rebuilds an `ExecutionContext` from a hook session, carrying the flag across | @@ -196,7 +196,7 @@ assuming `isSystem` covers it is a documented source of bugs. | "It suppresses triggers / record-change automation" | **No.** Only `skipTriggers` does. A bare `{ isSystem: true }` on a seed write re-fired automation on freshly seeded rows and wedged first boot | `metadata-protocol/src/seed-loader.ts:1971` (rationale at `:1881`–`1883`, #3760), `flow.zod.ts:702` | | "It skips the state machine" | **No.** That is `skipStateMachine`, carried by seed replay and by `treatAsHistorical` imports | `objectql/src/engine.ts` FSM gate; see [State Machine](/docs/protocol/objectql/state-machine) | | "It skips validation rules" | **No.** Field shape, `format`, `script` and the rest still run. The `readonly` strip runs *before* validation precisely so a discarded value is not judged | `objectql/src/engine.ts:10008`–`10025` | -| "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1540` (#3493 / #6640) | +| "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1580` (#3493 / #6640) | | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` | | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:299` | | "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1520`, `:1549`; `domains/actions.ts:404` | From d6573d8263d806d6c5abf9d12eb886f8c2ff981b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 05:14:51 +0000 Subject: [PATCH 5/6] chore(spec): record the plugin-sharing organization-refusal MATCH as a provenance waiver (#14937) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:error-code-provenance` scans for `constdef` stamp sites and cannot tell a package that RECOGNISES a registered code from one that EMITS it. The per-grant catch added for #14754 spells the engine's refusal code once, as `ENGINE_ORGANIZATION_REFUSAL_CODE`, and compares an incoming `err.code` against it — the gate reads that as an unlisted stamp site under `@objectstack/plugin-sharing`. Adjudicated on #14937 (maintainer ruling A, 2026-09-04): record it as a `PROVENANCE_WAIVERS` row naming `@objectstack/objectql` — the real emitter, whose owner key already carries the code (#8844) — rather than widening the gate or evading it with a bare inline literal. The row comes out together with the stamp site when #14936 lands and objectql publishes a recognizer. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WXyGTWPbbreqXow7Z2pZCk --- packages/spec/src/api/error-code-ledger.zod.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index 886736d354..b1a5e83f58 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -1181,6 +1181,21 @@ export const PROVENANCE_WAIVERS: readonly ProvenanceWaiver[] = [ 'protocol layer — the registered emitter — turns it into the 403 the wire carries ' + '(ADR-0010 §3.3). Spec ships schemas and pure helpers, never an HTTP door.', }, + { + package: '@objectstack/plugin-sharing', + code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED', + registeredUnder: '@objectstack/objectql', + reason: 'Matches the code, never emits it (#14754, adjudicated on #14937 — maintainer ' + + 'ruling A, 2026-09-04): `ENGINE_ORGANIZATION_REFUSAL_CODE` in ' + + '`plugin-sharing/src/sharing-rule-service.ts` is a `constdef` the per-grant catch in BOTH ' + + 'reconcile loops compares an incoming `err.code` against, so exactly one engine refusal is ' + + 'absorbed and a refused grant no longer aborts the pass or its stale-row revocations. The ' + + 'emitter is `@objectstack/objectql` (`SystemWriteOrganizationRequiredError`, ' + + 'tenancy/system-write-organization.ts) and the objectql owner key already carries the row ' + + '(#8844). Recognising a code is not emitting it; the named constant is typed FROM the ' + + 'engine\'s own declaration so it cannot drift from what the engine throws. Removed together ' + + 'with the stamp site when #14936 lands and objectql publishes a recognizer.', + }, { package: '@objectstack/types', code: 'VALIDATION_FAILED', From bb7da10c2e2a228d13d9c67f1462edeb6bb3f661 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 05:16:13 +0000 Subject: [PATCH 6/6] docs(changeset): declare the wire surface `grantsRefused` reaches, and the client-type lag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The evaluate route is a ledgered SDK route whose handler passes the service return value through unfiltered, so the seventh key is on the response body every caller already receives. The SDK method's declared resolved type is the spec's six-field `SharingRuleEvaluationResult`, which cannot name it — a client-type lag, additive on the wire, tracked as #14969. Declared here rather than lifted, because the spec type is a `domain:spec` single-owner file. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WXyGTWPbbreqXow7Z2pZCk --- ...econcile-per-grant-organization-refusal.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/.changeset/sharing-reconcile-per-grant-organization-refusal.md b/.changeset/sharing-reconcile-per-grant-organization-refusal.md index f01b04c0b2..252419e584 100644 --- a/.changeset/sharing-reconcile-per-grant-organization-refusal.md +++ b/.changeset/sharing-reconcile-per-grant-organization-refusal.md @@ -56,3 +56,24 @@ was refused` (#14143), which shipped `minor` for the same reason. `grantsRefused > 0` does **not** mean the pass failed. It means the pass met a record it cannot grant on and carried on — which is the whole point. + +**Wire surface — declared, not lifted.** `grantsRefused` reaches the wire. +`POST /api/v1/sharing/rules/:idOrName/evaluate` is a ledgered **SDK** route — +`packages/rest/src/rest-route-ledger.ts:390`, the row carrying +`disposition: 'sdk'` and `client: 'shares.rules.evaluate'` — and its REST handler +passes the service return value through **unfiltered** +(`packages/rest/src/rest-server.ts:11108`–`:11109`: +`const result = await svc.evaluateRule(req.params.idOrName, context ?? {})` +followed by `res.json(result)`). So the seventh key is on the response body every +caller of that route already receives. The SDK method declares +`SharingRuleEvaluationResult` as its resolved type +(`packages/client/src/index.ts:4766`, unwrapped at `:4771` through +`unwrapResponse` parameterised on that same type), and that type is the spec's +six-field contract — so the **declared client type cannot name the seventh key**. +That is a client-type **lag**, not a contract break: the key is additive on the +wire, every declared field is unchanged, and a consumer typed against +`SharingRuleEvaluationResult` keeps compiling exactly as before. Lifting the type +is not this PR's to do — `SharingRuleEvaluationResult` lives in +`@objectstack/spec`, a `domain:spec` single-owner file — so the lag is declared +here and tracked as the follow-up #14969, which lifts `grantsRefused?: number` +(optional) into `SharingRuleEvaluationResult`.