From c66694e7ae64b067c751527d3753823b9ec92e04 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 01:48:51 +0000 Subject: [PATCH 1/3] feat(sharing): admit a share to an external member on a public object On an effective-public object the D7 inert-grant verdict now reads the recipient's population: a user recipient whose sys_member.role in the record's organization is exactly `external` is admitted, because the external OWD floor consults its share rows there. Every other recipient keeps the 422 SHARING_NOT_ENABLED. The rule materialiser skips the internal recipients a rule expands to, keeps its pass going, and reports the skipped half in one note line and one log line per pass. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01AjwMndyviznGagEUcX7QVx --- .../src/sharing-rule-service.ts | 114 +++++++--- .../plugin-sharing/src/sharing-service.ts | 125 ++++++++++- .../src/system-caller-inert-grant.test.ts | 195 +++++++++++++++++- 3 files changed, 397 insertions(+), 37 deletions(-) diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts index d6aacf7bb78..3c89c6d4f88 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts @@ -39,6 +39,7 @@ import { } from '@objectstack/objectql'; import type { SharingEngine } from './sharing-service.js'; import type { SharingService } from './sharing-service.js'; +import { isInternalRecipientRefusal } from './sharing-service.js'; import { normalizeAccessLevel, normalizeStoredAccessLevel } from './access-level.js'; import { parseCriteria, isMatchAllCriteria, MATCH_ALL_CRITERIA_MESSAGE } from './rule-criteria.js'; import { TeamGraphService, teamHasMember } from './team-graph.js'; @@ -390,6 +391,22 @@ function byName(a: SharingRuleRow, b: SharingRuleRow): number { export interface SharingRuleReconcilePassResult extends SharingRuleEvaluationResult { /** Grants the engine's organization rule refused; each one is also logged. */ grantsRefused: number; + /** + * [#25549] One line, present only when the pass skipped recipients: the rule + * is anchored on a public object, where a share reaches an `external` member + * of the record's organization only, so no row was written for the internal + * recipients it expands to. The count is in the pass's log line. + */ + note?: string; +} + +/** What one attempted grant of a pass came to. */ +type RuleGrantOutcome = 'granted' | 'refused' | 'internal'; + +/** [#25549] The pass's one-line note when it skipped internal recipients. */ +function internalHalfNote(object: string): string { + return `"${object}" has a public sharing model: rows were written for external recipients only; ` + + 'the internal recipients this rule expands to already reach its records, so none was written for them'; } export interface SharingRuleServiceOptions { @@ -1231,6 +1248,7 @@ export class SharingRuleService implements ISharingRuleService { total.grantsUpdated += pass.grantsUpdated; total.grantsRevoked += pass.grantsRevoked; total.grantsRefused += pass.grantsRefused; + if (pass.note) total.note = pass.note; } return total; } @@ -1635,6 +1653,7 @@ export class SharingRuleService implements ISharingRuleService { pass.expandedUsers = 1; const matchedIds = await this.findMatchingRecords(rule); pass.matchedRecords = matchedIds.length; + let skipped = 0; for (const recordId of matchedIds) { const cur = held.get(recordId); if (cur) { @@ -1642,12 +1661,15 @@ export class SharingRuleService implements ISharingRuleService { // {@link reconcile}: a refused level update leaves the desired row. held.delete(recordId); if (cur.access_level === rule.access_level) continue; - if (await this.grantOrAbsorbOrganizationRefusal(rule, recordId, userId)) pass.grantsUpdated += 1; - else pass.grantsRefused += 1; - } else if (await this.grantOrAbsorbOrganizationRefusal(rule, recordId, userId)) { - pass.grantsCreated += 1; + const outcome = await this.grantOrAbsorbOrganizationRefusal(rule, recordId, userId); + if (outcome === 'granted') pass.grantsUpdated += 1; + else if (outcome === 'refused') pass.grantsRefused += 1; + else skipped += 1; } else { - pass.grantsRefused += 1; + const outcome = await this.grantOrAbsorbOrganizationRefusal(rule, recordId, userId); + if (outcome === 'granted') pass.grantsCreated += 1; + else if (outcome === 'refused') pass.grantsRefused += 1; + else skipped += 1; } } // The user's grants on records the rule no longer matches — what the @@ -1656,7 +1678,7 @@ export class SharingRuleService implements ISharingRuleService { await this.sharing.revoke(String(stale.id), SYSTEM_CTX); pass.grantsRevoked += 1; } - return pass; + return this.withInternalHalfNote(rule, pass, skipped); } // ── internals ───────────────────────────────────────────────────── @@ -2291,18 +2313,26 @@ export class SharingRuleService implements ISharingRuleService { * 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 — + * Returns `'granted'` when the grant landed, `'refused'` when it was refused — * never throws for the refusal, and the caller counts what it is told. + * + * [#25549] …and `'internal'` for the one other refusal a pass absorbs: on a + * public object the grant service admits an `external` member of the + * record's organization only ({@link isInternalRecipientRefusal}). That + * recipient is skipped, not counted as refused — the row would never be + * consulted for it — and the pass reports the skipped half in one line + * ({@link withInternalHalfNote}). Every other D7 refusal (bypass, owner-less, + * `controlled_by_parent`, phantom anchor) still aborts the rule's pass. */ private async grantOrAbsorbOrganizationRefusal( rule: SharingRuleRow, recordId: string, recipientId: string, - ): Promise { + ): Promise { // A rule that may sweep nowhere matched nothing, so it never asks for a // grant; refused here too, so no caller can write one around the sweep. const context = this.sweepContextOrReport(rule); - if (!context) return false; + if (!context) return 'refused'; try { await this.sharing.grant( { @@ -2317,8 +2347,9 @@ export class SharingRuleService implements ISharingRuleService { } as any, context, ); - return true; + return 'granted'; } catch (err: any) { + if (isInternalRecipientRefusal(err)) return 'internal'; if (!isSystemWriteOrganizationRequiredError(err)) throw err; this.logger?.warn?.( '[sharing-rule] grant refused by the engine organization rule — counted, pass continues', @@ -2330,10 +2361,29 @@ export class SharingRuleService implements ISharingRuleService { code: SYSTEM_WRITE_ORGANIZATION_REQUIRED_CODE, }, ); - return false; + return 'refused'; } } + /** + * [#25549] Put the one-line note on a pass that skipped internal recipients, + * and log the count once per pass — never once per skipped grant, which on a + * rule over N records and M staff would be N×M lines of the same fact. + */ + private withInternalHalfNote( + rule: SharingRuleRow, + pass: SharingRuleReconcilePassResult, + skipped: number, + ): SharingRuleReconcilePassResult { + if (skipped === 0) return pass; + this.logger?.info?.('[sharing-rule] internal recipients skipped on a public object — external recipients only', { + rule: rule.name ?? rule.id, + object: rule.object_name, + skipped, + }); + return { ...pass, note: internalHalfNote(rule.object_name) }; + } + /** * Diff a whole-rule pass's desired pairs against the rule's materialised * grants: upsert what is wanted, revoke the remainder. @@ -2362,6 +2412,7 @@ export class SharingRuleService implements ISharingRuleService { let updated = 0; let revoked = 0; let refused = 0; + let skipped = 0; // Upsert desired. [#14754] Per grant, not per pass: a refusal is counted // and the loop goes on, so the revoke loop below is REACHED. @@ -2375,16 +2426,16 @@ export class SharingRuleService implements ISharingRuleService { // inverted, and strictly worse than the abort it replaces. existingMap.delete(k); if (cur.access_level !== rule.access_level) { - if (await this.grantOrAbsorbOrganizationRefusal(rule, want.record_id, want.recipient_id)) { - updated += 1; - } else { - refused += 1; - } + const outcome = await this.grantOrAbsorbOrganizationRefusal(rule, want.record_id, want.recipient_id); + if (outcome === 'granted') updated += 1; + else if (outcome === 'refused') refused += 1; + else skipped += 1; } - } else if (await this.grantOrAbsorbOrganizationRefusal(rule, want.record_id, want.recipient_id)) { - created += 1; } else { - refused += 1; + const outcome = await this.grantOrAbsorbOrganizationRefusal(rule, want.record_id, want.recipient_id); + if (outcome === 'granted') created += 1; + else if (outcome === 'refused') refused += 1; + else skipped += 1; } } // Revoke stale. @@ -2393,7 +2444,7 @@ export class SharingRuleService implements ISharingRuleService { revoked += 1; } - return { + return this.withInternalHalfNote(rule, { ruleId: rule.id, matchedRecords: desiredSet.matchedRecords, expandedUsers: desiredSet.expandedUsers, @@ -2401,7 +2452,7 @@ export class SharingRuleService implements ISharingRuleService { grantsUpdated: updated, grantsRevoked: revoked, grantsRefused: refused, - }; + }, skipped); } private async reconcileForRecord( @@ -2423,6 +2474,7 @@ export class SharingRuleService implements ISharingRuleService { let updated = 0; let revoked = 0; let refused = 0; + let skipped = 0; if (match) { // [#14754] Per grant, exactly as in {@link reconcile}: one recipient the @@ -2436,16 +2488,16 @@ export class SharingRuleService implements ISharingRuleService { // desired row standing, never fall through to the revoke loop. existingMap.delete(userId); if (cur.access_level !== rule.access_level) { - if (await this.grantOrAbsorbOrganizationRefusal(rule, recordId, userId)) { - updated += 1; - } else { - refused += 1; - } + const outcome = await this.grantOrAbsorbOrganizationRefusal(rule, recordId, userId); + if (outcome === 'granted') updated += 1; + else if (outcome === 'refused') refused += 1; + else skipped += 1; } - } else if (await this.grantOrAbsorbOrganizationRefusal(rule, recordId, userId)) { - created += 1; } else { - refused += 1; + const outcome = await this.grantOrAbsorbOrganizationRefusal(rule, recordId, userId); + if (outcome === 'granted') created += 1; + else if (outcome === 'refused') refused += 1; + else skipped += 1; } } } @@ -2456,7 +2508,7 @@ export class SharingRuleService implements ISharingRuleService { revoked += 1; } - return { + return this.withInternalHalfNote(rule, { ruleId: rule.id, matchedRecords: match ? 1 : 0, expandedUsers: users.length, @@ -2464,7 +2516,7 @@ export class SharingRuleService implements ISharingRuleService { grantsUpdated: updated, grantsRevoked: revoked, grantsRefused: refused, - }; + }, skipped); } /** Revoke every grant `scope` names ({@link ruleGrantScope}), one row at a time through the sharing service. */ diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index 4a87201a20e..c1f616f503c 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -32,6 +32,7 @@ import { resolveTenantFieldName } from '@objectstack/objectql'; // [Ruling of record 6355206139] The EXTERNAL rung's OWD floor — the stamp // plugin-security makes and this service reads, named once in core. import { owdForAudience } from '@objectstack/core'; +import { MEMBERSHIP_ROLE_EXTERNAL } from '@objectstack/spec/identity'; import { WRITE_ACCESS_LEVELS, normalizeAccessLevel } from './access-level.js'; import { hasPhantomOwnerAnchor } from './federated-phantom-anchors.js'; import { @@ -85,6 +86,36 @@ function makeShareId(): string { /** System-elevated context for the plugin's own queries / mutations. */ const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; +/** + * [#25549] A grant recipient's standing in the record's organization, the one + * recipient fact the D7 verdict reads ({@link SharingService} `inertGrantReason`). + */ +type RecipientStanding = 'external' | 'internal' | 'unknown-organization'; + +/** [#25549] Marks a D7 refusal that only the recipient's population caused. */ +const INTERNAL_RECIPIENT_REFUSAL = Symbol.for('objectstack.sharing.internalRecipientRefusal'); + +/** + * [#25549] Is `err` a grant refused because its recipient is not an external + * member of the record's organization, on an object whose verdict the + * recipient's population decides? The rule materialiser reads it to skip that + * recipient and keep its pass going; every other refusal still aborts. + */ +export function isInternalRecipientRefusal(err: unknown): boolean { + return typeof err === 'object' && err !== null && (err as any)[INTERNAL_RECIPIENT_REFUSAL] === true; +} + +/** + * [#25549] Is this stored `sys_member.role` list exactly `external`? The twin of + * plugin-auth's `isExternalMembershipRole` (this package does not depend on + * plugin-auth): distinct, trimmed, lower-cased tokens, exactly one, `external`. + */ +function isExternalMembershipRole(raw: unknown): boolean { + if (typeof raw !== 'string') return false; + const tokens = [...new Set(raw.split(',').map((t) => t.trim().toLowerCase()).filter(Boolean))]; + return tokens.length === 1 && tokens[0] === MEMBERSHIP_ROLE_EXTERNAL; +} + /** * [ADR-0138 D8] Recipient values `RecordShareRecipientType` no longer declares, * each with the refusal {@link SharingService.grant} answers when one still @@ -1347,8 +1378,27 @@ export class SharingService implements ISharingService { * The non-system pre-flight still turns an unresolvable name into NOT_FOUND * ({@link assertSharingEnforced}); that is an EXISTENCE verdict, which is a * different act and stays where it was. + * + * ## The one arm the recipient's population decides (#25549) + * + * The `public` arm used to be caller-free AND recipient-free: a public model + * reads every row for everyone, so no row was ever consulted. That stopped + * being true for one population. An EXTERNAL principal meets an owner_only + * floor on a public object ({@link owdForPrincipal}, ruling of record + * 6355206139), so `buildReadFilter`, `checkEdit` and `checkDelete` DO read + * its `sys_record_share` rows there. The verdict therefore takes the + * recipient's population as a second input — still never the granter's: + * + * - `recipient` omitted — "for every recipient": the arm refuses, as before; + * - `'external'` — the recipient is an `external` member of the record's + * organization, and the arm admits it (`owner_id` present and not a + * federated phantom anchor, since the floor composes over that column); + * - `'internal'` / `'unknown-organization'` — the arm refuses, naming why. + * + * Every other arm (bypass, `controlled_by_parent` — ADR-0111 D7.2, owner-less, + * phantom anchor) ignores the population: no gate reads a row there for anyone. */ - private inertGrantReason(object: string): string | null { + private inertGrantReason(object: string, recipient?: RecipientStanding): string | null { if (this.bypassObjects.has(object)) { return `'${object}' bypasses record sharing; a share row on it would never be consulted`; } @@ -1359,7 +1409,18 @@ export class SharingService implements ISharingService { if (declared === 'controlled_by_parent') { return `'${object}' is controlled by its parent (master-detail); share the master record instead`; } - if (effectiveSharingModel(schema) === 'public' || !hasOwnerField(schema)) { + const isPublic = effectiveSharingModel(schema) === 'public'; + if (isPublic || !hasOwnerField(schema)) { + // [#25549] The external floor composes over `owner_id`, so the arm can lift + // only where that column is real: present, and not a phantom anchor. + if (recipient !== undefined && isPublic && hasOwnerField(schema) && !hasPhantomOwnerAnchor(schema)) { + if (recipient === 'external') return null; + return recipient === 'internal' + ? `'${object}' has a public sharing model, so a share row on it is consulted only for an ` + + `external member of the record's organization, and the recipient is not one` + : `'${object}' has a public sharing model, so a share row on it is consulted only for an ` + + `external member of the record's organization, and that organization could not be resolved`; + } return `'${object}' is not under record-sharing enforcement ` + `(public sharing model or no '${OWNER_FIELD}' field); a share row on it would never be consulted`; } @@ -1419,10 +1480,66 @@ export class SharingService implements ISharingService { * input to this question). */ private assertNotInertGrant(object: string): void { + // [#25549] The arm the recipient's population decides is the recipient + // half's ({@link assertRecipientNotInert}), asked once the organization is known. + if (this.recipientDecidesInertness(object)) return; const reason = this.inertGrantReason(object); if (reason) throw new Error(`SHARING_NOT_ENABLED: ${reason}`); } + /** + * [#25549] Does the D7 verdict on `object` depend on the recipient — refused + * for every recipient, admitted for an external one? Defined by the verdict + * itself, so it cannot drift from {@link inertGrantReason}. + */ + private recipientDecidesInertness(object: string): boolean { + return this.inertGrantReason(object) !== null && this.inertGrantReason(object, 'external') === null; + } + + /** + * [#25549] The recipient half of the D7 pre-flight, run for every caller once + * the grant's organization is resolved: on an object whose verdict the + * recipient's population decides, refuse unless the recipient is an + * `external` member of THAT organization. A user who is external in one + * organization and a member of another is judged where the record lives. + * + * The refusal is the same `SHARING_NOT_ENABLED` (422) as the object half, + * marked with {@link INTERNAL_RECIPIENT_REFUSAL} so the rule materialiser can + * skip the recipient and keep its pass going ({@link isInternalRecipientRefusal}). + */ + private async assertRecipientNotInert( + object: string, + recipientId: string, + organizationId: string | null, + ): Promise { + if (!this.recipientDecidesInertness(object)) return; + const standing: RecipientStanding = organizationId + ? ((await this.isExternalMemberIn(recipientId, organizationId)) ? 'external' : 'internal') + : 'unknown-organization'; + const reason = this.inertGrantReason(object, standing); + if (reason) { + throw Object.assign(new Error(`SHARING_NOT_ENABLED: ${reason}`), { [INTERNAL_RECIPIENT_REFUSAL]: true }); + } + } + + /** + * [#25549] Is `userId` an `external` member of `organizationId`? The same + * population test as the stage-1 doors (plugin-auth `isExternalMembershipRole`): + * a `sys_member` row in that organization whose role list is exactly + * `external`. The authorization resolver projects `org_external` from a list + * that CONTAINS it; the two agree on every row the engine admits, because a + * list carrying `external` beside another role is refused at write. + */ + private async isExternalMemberIn(userId: string, organizationId: string): Promise { + const rows = await this.engine.find('sys_member', { + where: { user_id: userId, organization_id: organizationId }, + fields: ['role'], + limit: 10, + context: { ...SYSTEM_CTX, tenantId: organizationId }, + }); + return (Array.isArray(rows) ? rows : []).some((m: any) => isExternalMembershipRole(m?.role)); + } + /** * The NON-system pre-flight: the object must EXIST, and a share row on it * must be one the gates would consult. @@ -1540,6 +1657,10 @@ export class SharingService implements ISharingService { await this.assertCanManageShares(input.object, input.recordId, context); organizationId = await this.resolveDirectGrantOrganization(input, context); } + // [#25549] The recipient half, on both paths: after authorization, so a + // refused caller never learns a recipient's population, and after the + // organization, because the population is judged in it. + await this.assertRecipientNotInert(input.object, input.recipientId, organizationId); // Upsert: if a row with same (object, record, recipient, source) exists, // update its access level / reason; otherwise insert a new one. `source` diff --git a/packages/plugins/plugin-sharing/src/system-caller-inert-grant.test.ts b/packages/plugins/plugin-sharing/src/system-caller-inert-grant.test.ts index ce4569badbf..a9b660a92f5 100644 --- a/packages/plugins/plugin-sharing/src/system-caller-inert-grant.test.ts +++ b/packages/plugins/plugin-sharing/src/system-caller-inert-grant.test.ts @@ -188,6 +188,15 @@ const ALL_SCHEMAS: Record = { const INERT_OBJECTS = Object.keys(INERT_SCHEMAS); +/** + * [#25549] The classes on which a RULE's pass still aborts. On the public one + * (`whiteboard`) the grant service admits an `external` member of the record's + * organization and refuses everyone else, so a rule pass there skips its + * internal recipients and goes on — pinned below and in + * `external-recipient-public-object.test.ts`. + */ +const RULE_ABORTING_OBJECTS = INERT_OBJECTS.filter((o) => o !== 'whiteboard'); + function seedRows(engine: ReturnType) { for (const object of ['account', ...INERT_OBJECTS]) { engine._tables[object] = [ @@ -342,13 +351,23 @@ describe('[#8207] the sharing-rule evaluator can no longer materialise inert row accessLevel: 'read', } as any, SYS); - it.each(INERT_OBJECTS)('evaluating a rule on %s mints NOTHING (it minted a row pre-fix)', async (object) => { + it.each(RULE_ABORTING_OBJECTS)('evaluating a rule on %s mints NOTHING (it minted a row pre-fix)', async (object) => { await define(object); // `evaluateRule` does not swallow the refusal — its callers do (next block). await expect(rules.evaluateRule(`share_${object}_gold`, SYS)).rejects.toThrow(/SHARING_NOT_ENABLED/); expect(engine._tables.sys_record_share ?? []).toHaveLength(0); }); + it('[#25549] a rule on the PUBLIC object mints nothing for an internal recipient, and says so in one line', async () => { + await define('whiteboard'); + // `usr_grantee` holds no `external` membership, so the public arm refuses it — + // a skip the pass absorbs and reports, not an abort. + const result = await rules.evaluateRule('share_whiteboard_gold', SYS); + expect(result).toMatchObject({ matchedRecords: 1, expandedUsers: 1, grantsCreated: 0, grantsRefused: 0 }); + expect(result.note).toMatch(/external recipients only/); + expect(engine._tables.sys_record_share ?? []).toHaveLength(0); + }); + it('ANTI-VACUITY: the same rule shape on an ordinary object still materialises', async () => { await define('account'); const result = await rules.evaluateRule('share_account_gold', SYS); @@ -367,11 +386,11 @@ describe('[#8207] the sharing-rule evaluator can no longer materialise inert row // The path the card names as the one most likely to trip. `evaluateRule` // throws for the inert rule; `backfillRuleGrants` catches per rule, warns, // and carries on, so the ordinary rule is still reconciled. - for (const object of ['account', ...INERT_OBJECTS]) await define(object); + for (const object of ['account', ...RULE_ABORTING_OBJECTS]) await define(object); const logged: Array<{ msg: string; meta: any }> = []; const reconciled = await backfillRuleGrants( rules, - [{ name: 'share_account_gold' }, ...INERT_OBJECTS.map(o => ({ name: `share_${o}_gold` }))], + [{ name: 'share_account_gold' }, ...RULE_ABORTING_OBJECTS.map(o => ({ name: `share_${o}_gold` }))], { info: (msg: string, meta?: any) => logged.push({ msg, meta }), warn: (msg: string, meta?: any) => logged.push({ msg, meta }), @@ -380,7 +399,7 @@ describe('[#8207] the sharing-rule evaluator can no longer materialise inert row expect(reconciled).toBe(1); // the ordinary rule, and only it expect(logged.some(l => l.msg.includes('boot rule backfill done'))).toBe(true); // Each refusal reaches the operator naming the rule AND the reason. - for (const object of INERT_OBJECTS) { + for (const object of RULE_ABORTING_OBJECTS) { const warn = logged.find(l => l.meta?.rule === `share_${object}_gold`); expect(warn?.msg).toMatch(/boot rule backfill failed for rule/); expect(warn?.meta?.error).toMatch(/SHARING_NOT_ENABLED/); @@ -404,3 +423,171 @@ describe('[#8207] the sharing-rule evaluator can no longer materialise inert row expect(engine._tables.sys_record_share).toHaveLength(0); }); }); + +// ───────────────────────────────────────────────────────────────────── +// 3 — [#25549] the public arm, decided by the recipient's population +// ───────────────────────────────────────────────────────────────────── + +/** + * An external principal meets an owner_only floor on a public object (ruling of + * record 6355206139, letter B), so a share row there IS consulted — for that + * population only. The D7 verdict on a public object therefore admits a `user` + * recipient whose `sys_member.role` in the RECORD's organization is exactly + * `external`, and keeps the 422 for everyone else. It still never reads the + * granter. These pins hold the four directions: external admitted and live, + * internal refused, a user judged in the record's organization (not in another + * it belongs to), and a rule that expands to both halves. + */ +describe('[#25549] a public object admits a share to an external member of the record’s organization', () => { + const BOARD = { + name: 'board', + sharingModel: 'public_read_write', + fields: { id: {}, tier: {}, owner_id: { ...OWNER_FIELD_DEF }, organization_id: {} }, + }; + const OWNER = { userId: 'usr_owner', tenantId: 'org_a' } as any; + const externalIn = (org: string, userId: string) => + ({ userId, tenantId: org, audience: 'external', posture: 'EXTERNAL' }) as any; + + let engine: ReturnType; + let svc: SharingService; + beforeEach(() => { + engine = makeEngine({ ...ALL_SCHEMAS, board: BOARD }); + svc = new SharingService({ engine: engine as any }); + engine._tables.board = [ + { id: 'b1', tier: 'gold', owner_id: 'usr_owner', organization_id: 'org_a' }, + { id: 'b2', tier: 'gold', owner_id: 'usr_owner', organization_id: 'org_a' }, + { id: 'b3', tier: 'silver', owner_id: 'usr_owner', organization_id: 'org_a' }, + ]; + engine._tables.sys_member = [ + { id: 'm1', user_id: 'usr_ext', organization_id: 'org_a', role: 'external' }, + { id: 'm2', user_id: 'usr_staff', organization_id: 'org_a', role: 'member' }, + // External in org_b, a plain member of org_a — the record's organization. + { id: 'm3', user_id: 'usr_dual', organization_id: 'org_a', role: 'member' }, + { id: 'm4', user_id: 'usr_dual', organization_id: 'org_b', role: 'external' }, + // External in org_a, a member of org_b. + { id: 'm5', user_id: 'usr_dual2', organization_id: 'org_a', role: 'external' }, + { id: 'm6', user_id: 'usr_dual2', organization_id: 'org_b', role: 'member' }, + // A list the write guard refuses today; the test is "exactly external". + { id: 'm7', user_id: 'usr_mixed', organization_id: 'org_a', role: 'external,member' }, + ]; + }); + + it('an EXTERNAL recipient is admitted, the row carries the record’s organization, and it is LIVE for that record only', async () => { + await expect( + svc.grant({ object: 'board', recordId: 'b1', recipientId: 'usr_ext' }, OWNER), + ).resolves.toMatchObject({ object_name: 'board', recipient_id: 'usr_ext', organization_id: 'org_a' }); + // The floored external reads the row it was handed beside the (empty) owner arm, + // and not b2, which nobody shared to it. + expect(await svc.buildReadFilter('board', externalIn('org_a', 'usr_ext'))).toEqual({ + $or: [{ owner_id: 'usr_ext' }, { id: { $in: ['b1'] } }], + }); + }); + + it('an INTERNAL recipient keeps the 422 — the row would never be consulted for it', async () => { + const err = await svc + .grant({ object: 'board', recordId: 'b1', recipientId: 'usr_staff' }, OWNER) + .then(() => null, (e: Error) => e); + // The `CODE:` prefix is what the REST layer maps to 422 (`respondSharingError`). + expect(err?.message.startsWith('SHARING_NOT_ENABLED:')).toBe(true); + expect(err?.message).toMatch(/consulted only for an external member/); + expect(ERROR_CODE_LEDGER['@objectstack/plugin-sharing']).toContain('SHARING_NOT_ENABLED'); + expect(engine._tables.sys_record_share ?? []).toHaveLength(0); + }); + + it('a user is judged in the RECORD’s organization, not in another one it belongs to', async () => { + // External in org_b only: refused on an org_a record. + await expect( + svc.grant({ object: 'board', recordId: 'b1', recipientId: 'usr_dual' }, OWNER), + ).rejects.toThrow(/^SHARING_NOT_ENABLED:/); + // External in org_a, member of org_b: admitted on the same record. + await expect( + svc.grant({ object: 'board', recordId: 'b1', recipientId: 'usr_dual2' }, OWNER), + ).resolves.toMatchObject({ recipient_id: 'usr_dual2', organization_id: 'org_a' }); + }); + + it('a role list is external only when it is exactly `external`', async () => { + await expect( + svc.grant({ object: 'board', recordId: 'b1', recipientId: 'usr_mixed' }, OWNER), + ).rejects.toThrow(/^SHARING_NOT_ENABLED:/); + }); + + it('the verdict never reads the granter: a SYSTEM caller gets the same two answers', async () => { + await expect( + svc.grant({ object: 'board', recordId: 'b1', recipientId: 'usr_ext', source: 'rule', sourceId: 'srule_x' }, SYS), + ).resolves.toMatchObject({ recipient_id: 'usr_ext', organization_id: 'org_a' }); + const asSystem = await svc + .grant({ object: 'board', recordId: 'b1', recipientId: 'usr_staff' }, SYS) + .then(() => null, (e: Error) => e.message); + const asUser = await svc + .grant({ object: 'board', recordId: 'b1', recipientId: 'usr_staff' }, OWNER) + .then(() => null, (e: Error) => e.message); + expect(asSystem).toBe(asUser); + }); + + it('the D1 management gate still runs first: a caller who cannot manage the record learns nothing about the recipient', async () => { + await expect( + svc.grant({ object: 'board', recordId: 'b1', recipientId: 'usr_ext' }, { userId: 'usr_nobody', tenantId: 'org_a' } as any), + ).rejects.toThrow(/^PERMISSION_DENIED:/); + }); + + it('a rule that expands to staff AND externals writes rows for the externals only, and says so in one line', async () => { + const logs: Array<{ msg: string; meta: any }> = []; + const rules = new SharingRuleService({ + engine: engine as any, + sharing: svc, + logger: { warn: () => {}, info: (msg: string, meta?: any) => { logs.push({ msg, meta }); } }, + }); + engine._tables.sys_team_member = [ + { id: 't1', team_id: 'team_vendors', user_id: 'usr_ext' }, + { id: 't2', team_id: 'team_vendors', user_id: 'usr_staff' }, + ]; + await rules.defineRule({ + name: 'share_board_gold', + label: 'Share gold boards', + object: 'board', + criteria: { tier: 'gold' }, + recipientType: 'team', + recipientId: 'team_vendors', + accessLevel: 'read', + } as any, SYS); + const result = await rules.evaluateRule('share_board_gold', SYS); + // Two gold records × two members: two rows for the external, none for the staff + // member, and the skip is neither a refusal nor an abort. + expect(result).toMatchObject({ matchedRecords: 2, expandedUsers: 2, grantsCreated: 2, grantsRefused: 0 }); + expect(result.note).toBe( + '"board" has a public sharing model: rows were written for external recipients only; ' + + 'the internal recipients this rule expands to already reach its records, so none was written for them', + ); + expect((engine._tables.sys_record_share ?? []).map((r) => r.recipient_id)).toEqual(['usr_ext', 'usr_ext']); + // The count reaches the log once per pass, not once per skipped grant. + const skipLines = logs.filter((l) => l.msg.includes('internal recipients skipped')); + expect(skipLines).toHaveLength(1); + expect(skipLines[0].meta).toMatchObject({ rule: 'share_board_gold', object: 'board', skipped: 2 }); + }); + + it('ANTI-VACUITY: a rule that expands to externals only carries no note', async () => { + const rules = new SharingRuleService({ engine: engine as any, sharing: svc, logger: { warn: () => {}, info: () => {} } }); + await rules.defineRule({ + name: 'share_board_ext', + label: 'Share gold boards with the supplier', + object: 'board', + criteria: { tier: 'gold' }, + recipientType: 'user', + recipientId: 'usr_ext', + accessLevel: 'read', + } as any, SYS); + const result = await rules.evaluateRule('share_board_ext', SYS); + expect(result).toMatchObject({ grantsCreated: 2 }); + expect(result.note).toBeUndefined(); + }); + + it('the other D7 arms ignore the population: an owner-less public object still refuses an external', async () => { + engine = makeEngine({ ...ALL_SCHEMAS, board: { ...BOARD, fields: { id: {}, tier: {}, organization_id: {} } } }); + engine._tables.board = [{ id: 'b1', tier: 'gold', organization_id: 'org_a' }]; + engine._tables.sys_member = [{ id: 'm1', user_id: 'usr_ext', organization_id: 'org_a', role: 'external' }]; + svc = new SharingService({ engine: engine as any }); + await expect( + svc.grant({ object: 'board', recordId: 'b1', recipientId: 'usr_ext', source: 'rule' }, SYS), + ).rejects.toThrow(/not under record-sharing enforcement/); + }); +}); From 9824f0212754c4712baf4cc90fe967ef58a43517 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 01:54:52 +0000 Subject: [PATCH 2/3] =?UTF-8?q?feat(lint):=20a=20sharing=20rule=20on=20a?= =?UTF-8?q?=20public=20object=20is=20advice=20=E2=80=94=20effective=20for?= =?UTF-8?q?=20external=20recipients=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sharing-rule-object-not-shareable drops from error to warning on its public arm: the runtime now admits a grant there for an external member of the record's organization, so the rule is live for its external half. The os explain entry, the docs sentence in sharing-rules.mdx, the permissions index paragraph and the app-crm comment say the same. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01AjwMndyviznGagEUcX7QVx --- content/docs/permissions/index.mdx | 5 ++- content/docs/permissions/sharing-rules.mdx | 6 +++ examples/app-crm/objectstack.config.ts | 9 ++-- packages/lint/src/authoring-rules.ts | 11 +++-- packages/lint/src/rule-explanations.ts | 41 +++++++++++-------- ...lidate-sharing-rule-enforceability.test.ts | 16 ++++---- .../validate-sharing-rule-enforceability.ts | 35 +++++++++++----- 7 files changed, 80 insertions(+), 43 deletions(-) diff --git a/content/docs/permissions/index.mdx b/content/docs/permissions/index.mdx index 67791a5a4a4..7bc3d775904 100644 --- a/content/docs/permissions/index.mdx +++ b/content/docs/permissions/index.mdx @@ -43,8 +43,9 @@ export const KeyAccountQualifiedContactRule = defineSharingRule({ Note the object it is anchored on. `showcase_contact` is OWD `private`, which is what makes this grant one a read gate actually consults — sharing only ever WIDENS a baseline, so a rule on an object that is already `public_read_write` -grants nothing and is refused at boot. That is an authoring-time build error -(`sharing-rule-object-not-shareable`), not a runtime surprise. +grants nothing to staff, and reaches only the external (portal) members it names. +The authoring-time advisory `sharing-rule-object-not-shareable` says so before +anything boots. Because AI agents act through the same permission-aware surface, these rules bound agent access exactly as they bound users ([Actions as Tools](/docs/ai/actions-as-tools)). diff --git a/content/docs/permissions/sharing-rules.mdx b/content/docs/permissions/sharing-rules.mdx index bbe260644fa..bf09d759871 100644 --- a/content/docs/permissions/sharing-rules.mdx +++ b/content/docs/permissions/sharing-rules.mdx @@ -137,6 +137,12 @@ the rows it owns and the rows granted to it: the object (for example `supplier in current_user.`). Such a policy lifts the owner-only floor for that object, and the policy bounds the rows. +On a `public_read_write` object, or a system object with no `sharingModel`, a +share or sharing rule takes effect for external recipients only: the grant is +accepted for a recipient who is an `external` member of the record's +organization, every other recipient is refused with `SHARING_NOT_ENABLED` +(422), and a rule's evaluation writes rows for its external recipients alone. + `externalSharingModel`, the external dial ADR-0090 D11 declared, was removed in `@objectstack/spec` 18 (ADR-0049 enforce-or-remove): while it existed no request resolved an external principal, so the value changed no answer. The schema diff --git a/examples/app-crm/objectstack.config.ts b/examples/app-crm/objectstack.config.ts index 10ffc96fc6b..a8913736cc0 100644 --- a/examples/app-crm/objectstack.config.ts +++ b/examples/app-crm/objectstack.config.ts @@ -109,10 +109,11 @@ export default defineStack({ // were removed under ADR-0049 enforce-or-remove (#9698), the same call // #9237 made for app-showcase's two. // - // ⛔ Do not re-add one on a public object — `sharing-rule-object-not-shareable` - // now fails the build, and its message states the two honest fixes. Giving - // this app a LIVE sharing demonstration means giving it a `private` object - // first; that is an access-matrix change (ADR-0090 D6 review), not a rider. + // ⛔ Do not re-add one for staff on a public object — such a rule reaches + // external (portal) members only, and `sharing-rule-object-not-shareable` + // says so. Giving this app a LIVE sharing demonstration for staff means + // giving it a `private` object first; that is an access-matrix change + // (ADR-0090 D6 review), not a rider. // Seed data data: CrmSeedData, diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 7671ba5376e..ed66d4da33b 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -2222,12 +2222,15 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ // does not lower means the rule is SKIPPED by enforcement. // - `object` decides whether the grant is refused outright: reconcile hands // each row to `SharingService.grant`, whose ADR-0111 D7 pre-flight THROWS - // `SHARING_NOT_ENABLED` when the anchor's effective sharing model is - // `public` or it is a `controlled_by_parent` detail. Both are decidable - // from authored metadata; the other arms of that verdict (`owner_id`, the + // `SHARING_NOT_ENABLED` when the anchor is a `controlled_by_parent` + // detail, and, when its effective sharing model is `public`, for every + // recipient who is not an `external` member of the record's organization + // (#25549: that arm is advice, a `warning`). Both are decidable from + // authored metadata; the other arms of that verdict (`owner_id`, the // bypass set, federated anchors) are not, and are excluded by name. // - // Either way the grant is declared and does not exist. The lint calls the + // Either way the grant is declared and does not exist (for staff, on the + // public arm). The lint calls the // same compiler and mirrors the same verdict function, from the same inputs // — the verdict cannot drift from the consumers'. Gating for the ADR-0078 // reason `SharingRuleSchema`'s own docblock states: the whole authorable diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 4a2d09e775c..adeccb4afd5 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -1670,22 +1670,31 @@ const SHARING_RULE_RUNTIME_VARIABLE_CONDITION_EXPLANATION: RuleExplanation = { const SHARING_RULE_OBJECT_NOT_SHAREABLE_EXPLANATION: RuleExplanation = { rule: 'sharing-rule-object-not-shareable', - covers: 'why a rule on a public object grants nothing', - paragraphs: [ - 'Sharing only ever WIDENS an organization-wide default (OWD) baseline, and on the widest baseline there is ' + - 'nothing left to widen. An object\'s effective sharing model is `public` when it declares ' + - '`sharingModel: \'public_read_write\'`, or when it declares none and is a system object (`isSystem: true` ' + - 'or a `sys_` name), which ADR-0090 D1 resolves to public. A custom object with no `sharingModel` resolves ' + - 'to private and is not reported.', - SHARING_INERT_GRANT, - 'Here the refusal reads "\'OBJECT\' is not under record-sharing enforcement". Measured: `buildReadFilter` ' + - 'returns `null` for such an object — NO record-level filter at all — so every principal already reads ' + - 'every row, and the rule advertises a restriction that does not exist.', - 'To fix it, decide which half is wrong. If the ACCESS is right — everyone should read and write these ' + - 'records — the rule is dead metadata: delete it (ADR-0049 enforce-or-remove). If the RULE is right — ' + - 'only the named audience should reach these records — the object\'s OWD is the defect: set ' + - '`sharingModel: \'private\'` (the owner plus explicit shares) or `\'public_read\'`, and the rule starts ' + - 'enforcing. Do not re-home the rule onto another public object; that moves the inertness instead of ' + + covers: 'why a rule on a public object reaches externals only', + paragraphs: [ + 'Sharing only ever WIDENS an organization-wide default (OWD) baseline, and for the people who work in the ' + + 'organization a public baseline has nothing left to widen. An object\'s effective sharing model is ' + + '`public` when it declares `sharingModel: \'public_read_write\'`, or when it declares none and is a ' + + 'system object (`isSystem: true` or a `sys_` name), which ADR-0090 D1 resolves to public. A custom object ' + + 'with no `sharingModel` resolves to private and is not reported.', + 'An EXTERNAL member of the organization (`sys_member.role` exactly `external`: a supplier\'s or a ' + + 'customer\'s portal user) is different. Its baseline on every object is the rows it owns, so a share is ' + + 'how it reaches a record of a public object. `SharingService.grant` therefore admits a grant on such an ' + + 'object for a recipient who is an external member of the record\'s organization, and its ' + + '`assertNotInertGrant` pre-flight (ADR-0111 D7) refuses everyone else with SHARING_NOT_ENABLED: "\'OBJECT\' ' + + 'has a public sharing model, so a share row on it is consulted only for an external member". The rule\'s ' + + 'pass writes rows for the external recipients it expands to, skips the internal ones, and says so in one ' + + 'line (`note` on the evaluate result, and one log line per pass).', + 'Measured for an internal reader: `buildReadFilter` returns `null` on such an object — NO record-level ' + + 'filter at all — so every member already reads every row, and for them the rule advertises a restriction ' + + 'that does not exist. That is why this is advice and not an error: it is right when the rule\'s audience ' + + 'is external, and only half of what it says when the audience includes staff.', + 'To act on it, decide which half is wrong. If the audience is external — the rule hands a supplier the ' + + 'records it should see — keep it. If the ACCESS is right for staff and the rule names staff, the staff ' + + 'half is dead metadata: narrow the recipients (ADR-0049 enforce-or-remove). If only the named audience ' + + 'should reach these records, staff included, the object\'s OWD is the defect: set ' + + '`sharingModel: \'private\'` (the owner plus explicit shares) or `\'public_read\'`, and the rule enforces ' + + 'for everyone. Do not re-home the rule onto another public object; that moves the inertness instead of ' + 'removing it.', ], }; diff --git a/packages/lint/src/validate-sharing-rule-enforceability.test.ts b/packages/lint/src/validate-sharing-rule-enforceability.test.ts index c62d72b67f0..cccb612b459 100644 --- a/packages/lint/src/validate-sharing-rule-enforceability.test.ts +++ b/packages/lint/src/validate-sharing-rule-enforceability.test.ts @@ -319,11 +319,13 @@ const anchoredOn = (owd: unknown, extra: Record = {}) => ({ }); describe('validateSharingRuleEnforceability — an anchor no gate would consult goes RED', () => { - it('flags `public_read_write` — nothing left to widen', () => { + it('advises on `public_read_write` — effective for external recipients only (#25549)', () => { const findings = validateSharingRuleEnforceability(anchoredOn('public_read_write')); expect(findings).toHaveLength(1); expect(findings[0]).toMatchObject({ - severity: 'error', + // Advice, not a refusal: the runtime admits the grant for a recipient who + // is an `external` member of the record's organization. + severity: 'warning', rule: SHARING_RULE_OBJECT_NOT_SHAREABLE, // The defect is on `object`, not on `condition` — a different field with // a different fix, so it gets its own path. @@ -333,12 +335,12 @@ describe('validateSharingRuleEnforceability — an anchor no gate would consult // An author must be able to act on this: it has to name the object, the // rule (the `where` above), and WHY the rule cannot take effect. expect(findings[0].message).toBe( - 'anchor object "crm_opportunity" declares sharingModel \'public_read_write\', so the grant is refused ' + - '(SHARING_NOT_ENABLED) and the rule restricts nothing', + 'anchor object "crm_opportunity" declares sharingModel \'public_read_write\', so the rule is effective for ' + + 'external recipients only', ); expect(explanationOf(SHARING_RULE_OBJECT_NOT_SHAREABLE)).toMatch(/`buildReadFilter` returns `null`/); - // …and both honest fixes, because which one is right is the author's call. - expect(findings[0].hint).toMatch(/delete it/); + // …and both honest choices, because which one is right is the author's call. + expect(findings[0].hint).toMatch(/Keep it if its audience is external/); expect(findings[0].hint).toMatch(/sharingModel: 'private'/); }); @@ -654,7 +656,7 @@ describe('[#22161] one-line verdicts — the four ids', () => { const MOVED: Record = { [SHARING_RULE_UNLOWERABLE_CONDITION]: ['`createDeclaredSharingRuleSource`', 'one WARN line in the log', 'ADR-0049', 'SharingRuleService', 'INVALID_FILTER'], [SHARING_RULE_RUNTIME_VARIABLE_CONDITION]: ['MATERIALISED', 'no "current user"', 'the rule is SKIPPED'], - [SHARING_RULE_OBJECT_NOT_SHAREABLE]: ['WIDENS', 'ADR-0090 D1', '`assertNotInertGrant`', 'not under record-sharing enforcement', 'restriction that does not exist'], + [SHARING_RULE_OBJECT_NOT_SHAREABLE]: ['WIDENS', 'ADR-0090 D1', '`assertNotInertGrant`', 'consulted only for an external member', 'restriction that does not exist'], [SHARING_RULE_OBJECT_CONTROLLED_BY_PARENT]: ['ADR-0055', 'no shares to widen', '`assertNotInertGrant`', 'whatever the MASTER grants them'], }; diff --git a/packages/lint/src/validate-sharing-rule-enforceability.ts b/packages/lint/src/validate-sharing-rule-enforceability.ts index 4a75b2eb1d6..fd5711be3dc 100644 --- a/packages/lint/src/validate-sharing-rule-enforceability.ts +++ b/packages/lint/src/validate-sharing-rule-enforceability.ts @@ -61,6 +61,13 @@ * a warning. It fails closed (the recipient under-sees rather than over-sees), * which is why it was survivable, not why it is acceptable. * + * [#25549] One arm is no longer an `error`: on a `public` anchor the runtime + * admits the grant for a recipient who is an `external` member of the record's + * organization (the external floor consults a share row there) and refuses the + * rest, so that rule IS live for its external half. Whether a rule's audience + * is external is runtime data, not metadata, so the arm reports a `warning` + * saying what the rule does — "effective for external recipients only". + * * Measured before shipping: every sharing-rule condition and RLS predicate * declared anywhere in this repo (examples, platform permission sets) lowers * cleanly, so the gate turns nothing red that works today. @@ -439,10 +446,13 @@ function masterOf(obj: AnyRec): string | undefined { * `buildReadFilter`, all three postures), the arms differ in the direction * that decides the wording: * - * - **`public` OWD** → grant refused, `buildReadFilter` returns `null`, zero - * share rows. Nothing is filtered, so the intended audience already reads - * every row — and so does everyone else. Nobody UNDER-sees; the harm is - * that the declaration advertises a restriction that does not exist. + * - **`public` OWD** → grant refused for an internal recipient, + * `buildReadFilter` returns `null` for an internal reader, zero share rows + * for staff. Nothing is filtered for them, so the intended staff audience + * already reads every row — and so does everyone else. Nobody UNDER-sees; + * the harm is that the declaration advertises a restriction that does not + * exist. An `external` recipient is admitted and its row is consulted + * (#25549), which is why this arm is a `warning`. * - **`controlled_by_parent`** → grant refused with a DIFFERENT reason * ("share the master record instead"), and the detail's visibility comes * from its master's path (ADR-0055), not from this rule. Here the author @@ -524,21 +534,26 @@ function anchorFindings( ? `declares sharingModel 'public_read_write'` : 'declares no sharingModel and is a system object, which resolves to public'; + // [#25549] Advice, not a refusal: the runtime admits this rule's grants for + // the recipients who are `external` members of the record's organization + // (their owner_only floor consults a share row on a public object) and skips + // the rest. Whether the rule's audience is external is runtime data + // (`sys_member.role`), not metadata, so this door can say what the rule does + // and cannot say it is wrong. return [{ - severity: 'error', + severity: 'warning', rule: SHARING_RULE_OBJECT_NOT_SHAREABLE, where, path, // [#22161] One verdict sentence; why a public baseline has nothing to - // widen, and the measured read filter, is the `os explain` entry's. + // widen for staff, and the measured read filter, is the `os explain` entry's. message: - `anchor object "${object}" ${declared}, so the grant is refused (SHARING_NOT_ENABLED) and the rule ` + - 'restricts nothing', - // [#22161] One remedy line naming both honest fixes; how to tell which + `anchor object "${object}" ${declared}, so the rule is effective for external recipients only`, + // [#22161] One remedy line naming the honest choices; how to tell which // half is wrong, and why re-homing the rule fixes nothing, are the // `os explain` entry's. hint: - `If everyone should reach these records, delete it; if only its audience should, set "${object}"'s ` + + `Keep it if its audience is external; if staff should be narrowed too, set "${object}"'s ` + `sharingModel: 'private' or 'public_read'`, }]; } From a1260e572f68a044e90b689eb0f360fd4f2813a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 24 Oct 2026 02:04:07 +0000 Subject: [PATCH 3/3] test(dogfood): an owner shares one public_read_write invoice with an external On a showcase boot the record's owner shares one invoice with an external member over the REST share door (201) and the external then reads that invoice and no other; the same share to an ordinary member answers 422 SHARING_NOT_ENABLED. Adds the changeset. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01AjwMndyviznGagEUcX7QVx --- .../25549-external-share-public-object.md | 15 +++++++ .../external-member-directory.dogfood.test.ts | 41 ++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 .changeset/25549-external-share-public-object.md diff --git a/.changeset/25549-external-share-public-object.md b/.changeset/25549-external-share-public-object.md new file mode 100644 index 00000000000..86303f69b20 --- /dev/null +++ b/.changeset/25549-external-share-public-object.md @@ -0,0 +1,15 @@ +--- +'@objectstack/plugin-sharing': minor +'@objectstack/lint': minor +--- + +feat: a share or sharing rule on a public object reaches an external member of the record's organization + +Clause-②: yes (widening: the grant door and the materialiser admit one recipient population on effective-public objects) +Metadata-protocol: semantic + +An external member (`sys_member.role` exactly `external`) reads only the rows it owns and the rows shared to it, on every object. On a `public_read_write` object, or a system object with no `sharingModel`, a share to it used to be refused as one that would never take effect. For that member it is the only thing that does. + +- **Manual shares.** `SharingService.grant` now admits a `user` recipient on such an object when the recipient is an `external` member of the record's organization. Every other recipient keeps the 422 `SHARING_NOT_ENABLED`, with a message that says the share is consulted for external members only. A user who is external in one organization and a member of another is judged in the record's organization. `controlled_by_parent`, owner-less, federated phantom-anchor and bypass objects are refused for every recipient, as before. +- **Sharing rules.** A rule's pass writes rows for the external members it expands to and skips the internal ones without failing the pass. When it skips any, the evaluate result carries a one-line `note`, and the pass logs the count once. +- **Lint.** `sharing-rule-object-not-shareable` on a public object is now a warning: the rule is effective for external recipients only. `os explain` says when to keep it and when to change the object's `sharingModel`. diff --git a/packages/qa/dogfood/test/external-member-directory.dogfood.test.ts b/packages/qa/dogfood/test/external-member-directory.dogfood.test.ts index a922c3f0a44..f85823e8e96 100644 --- a/packages/qa/dogfood/test/external-member-directory.dogfood.test.ts +++ b/packages/qa/dogfood/test/external-member-directory.dogfood.test.ts @@ -33,6 +33,14 @@ // - list-teams and get-full-organization's teams answer an external the // teams it sits on; list-invitations answers it its own invitations. // +// [#25549] A share to an external on a `public_read_write` object: +// +// - the record's owner shares one invoice with an external over the REST +// share door and gets 201; the external then reads that invoice, and no +// other, where before it read none; +// - the same share to an ordinary member is refused 422 +// `SHARING_NOT_ENABLED`: on a public object no gate consults it for staff. +// // Fixtures are synthetic. ⚠️ No test title states a value. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -179,12 +187,12 @@ describe('[#25233] an external organization member reads its own identity rows a // same read on both objects, and neither owns any of the rows. const portalLines = await catalogPermissionSet(ql, portalLinesSet.name); expect(portalLines, 'fixture invoice permission set served by the security catalog').toBeTruthy(); - for (const who of ['supplierD', 'member'] as const) { + for (const who of ['supplierD', 'member', 'supplierB'] as const) { await ql.insert('sys_user_permission_set', { user_id: uid[who], permission_set: portalLines.name, organization_id: orgId }, { context: SYS }); } const account = await ql.insert('showcase_account', { name: 'Account 25233', status: 'active', organization_id: orgId }, { context: SYS }); const product = await ql.insert('showcase_product', { name: 'Product 25233', organization_id: orgId }, { context: SYS }); - for (const key of ['shared', 'other'] as const) { + for (const key of ['shared', 'other', 'grant'] as const) { const inv = await ql.insert(MASTER, { name: `INV-25233-${key}`, account: String(account.id), status: 'draft', owner_id: uid.owner, organization_id: orgId, }, { context: SYS }); @@ -362,6 +370,35 @@ describe('[#25233] an external organization member reads its own identity rows a expect((await auth('supplierB', `list-invitations?organizationId=${orgId}`)).body).toEqual([]); }); + it('[#25549] the owner shares one public_read_write invoice with an external, who then reads it and no other', async () => { + // Before: the floored external owns no invoice and holds no share, so it reads none. + const before = await ids('supplierB', `/data/${MASTER}?limit=200`); + expect(before.status).toBe(200); + expect(before.ids).toEqual([]); + + const res = await stack.apiAs(token.owner, 'POST', `/data/${MASTER}/${row.inv_grant}/shares`, { + recipientId: uid.supplierB, accessLevel: 'read', + }); + expect(res.status).toBe(201); + const share = await res.json() as Row; + expect(share).toMatchObject({ object_name: MASTER, record_id: row.inv_grant, recipient_id: uid.supplierB, organization_id: orgId }); + + const after = await ids('supplierB', `/data/${MASTER}?limit=200`); + expect(after.status).toBe(200); + expect(after.ids).toEqual([row.inv_grant]); + }); + + it('[#25549] the same share to an ordinary member is refused 422 SHARING_NOT_ENABLED', async () => { + const res = await stack.apiAs(token.owner, 'POST', `/data/${MASTER}/${row.inv_grant}/shares`, { + recipientId: uid.member, accessLevel: 'read', + }); + expect(res.status).toBe(422); + const body = await res.json() as { error?: { code?: string; message?: string } }; + expect(body.error?.code).toBe('SHARING_NOT_ENABLED'); + const rows = await findRows(ql, 'sys_record_share', { object_name: MASTER, record_id: row.inv_grant, recipient_id: uid.member }); + expect(rows).toHaveLength(0); + }); + it('E5: an owner cannot give a member external beside another role', async () => { const [row] = await findRows(ql, 'sys_member', { user_id: uid.member, organization_id: orgId }, 1); const res = await stack.apiAs(token.owner, 'POST', '/auth/organization/update-member-role', {