Skip to content

Commit 503be86

Browse files
os-zhuangclaude
andauthored
feat(security)!: reconcile SharingRule authoring with the enforced runtime — group→team, +business_unit, prune guest + owner-type (#1878) (#3557)
The authoring ShareRecipientType enum drifted behind the ADR-0090 D3 rename and the enforced runtime: expandRecipient already expands `team` (sys_team / sys_team_member via TeamGraphService) and `business_unit`, but the enum still offered the pre-rename `group` (silently skipped at seed time with a warn) and omitted both live recipients. Owner-type rules validated but were skipped wholesale (live-membership-dependent). After this change every authorable recipient and rule type is enforced — nothing on the SharingRule surface validates and then silently does nothing (ADR-0078). - spec: rename group→team, add business_unit, drop guest; delete OwnerSharingRuleSchema + OwnerSharingRule (SharingRuleSchema = the criteria form; SharingRuleType = ['criteria']); JSDoc rewritten; explain.zod comment. - plugin-sharing bootstrap: map team→TeamGraphService expansion; retired shapes kept as defensive skips (stale pre-built packages), never match-all. - examples: migrate both never-enforced owner rules to enforced criteria rules (showcase share_open_tasks_with_manager: done==false → manager; crm share_active_leads_with_manager: status not converted/disqualified → sales_manager); crm smoke test asserts criteria-only. - dogfood: rbac-seeding test flips from "owner rule honestly skipped" to "criteria replacement seeds + enforces" (criteria_json {done:false}, recipient position/manager — proven on a real boot: ruleCount 4→5, showcase_task now hook-bound); authz-conformance sharing-rules row updated. - docs: permissions/sharing-rules.mdx recipient table (all enforced) + retired-shapes note; references regen; manifest key security/OwnerSharingRule dropped; api-surface −2 exports. Verified: spec 6882 + plugin-sharing 101 + crm 20 + showcase 58 + dogfood 17 tests green; spec/downstream-contract/examples tsc clean; check:docs / api-surface / skill-refs / skill-examples in sync. Ships minor per the launch-window breaking-as-minor policy; changeset carries the FROM→TO migration. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent f390eae commit 503be86

14 files changed

Lines changed: 284 additions & 250 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/plugin-sharing": minor
4+
---
5+
6+
feat(security)!: reconcile the SharingRule authoring surface with the enforced runtime — rename `group``team`, add `business_unit`, prune `guest` + owner-type rules (#1878)
7+
8+
The authoring `ShareRecipientType` enum had drifted behind the ADR-0090 D3
9+
rename and the enforced runtime: the runtime expands `team` (via
10+
`sys_team`/`sys_team_member`) and `business_unit`, but the authoring enum
11+
still offered the pre-rename `group` (silently skipped at seed time) and
12+
omitted the two live recipients. After this change **every authorable
13+
recipient and rule type is enforced** — nothing on the SharingRule surface
14+
validates and then silently does nothing (ADR-0078).
15+
16+
- **`sharedWith.type: 'group'``'team'`** (wire-rename): the enum member is
17+
renamed to match the runtime vocabulary and now maps through the seed
18+
bootstrap to the live `TeamGraphService` expansion. Flat `sys_team`
19+
membership; enforced.
20+
- **`business_unit` added** to the authoring enum — exactly one business
21+
unit's members (no subtree; use `unit_and_subordinates` for the subtree).
22+
The runtime + bootstrap already enforced it; only the enum omitted it.
23+
- **`guest` removed** — it had no runtime recipient mapping. Anonymous access
24+
is served by the public-form grant and share links, not sharing rules.
25+
- **Owner-type rules removed** (`type: 'owner'`, `ownedBy`,
26+
`OwnerSharingRuleSchema` + its type export): they depend on live
27+
team/position membership, which the static materialiser cannot track, so
28+
they validated but never materialised a share. They return as an enforced
29+
form if membership-reactive re-materialisation is designed.
30+
`SharingRuleSchema` is now the criteria form; the `queue` recipient stays
31+
runtime-reserved (no `sys_queue` yet) and deliberately non-authorable.
32+
33+
**Migration** (stale definitions now fail parse with the valid options listed):
34+
- `sharedWith: { type: 'group', … }``sharedWith: { type: 'team', … }`.
35+
- `sharedWith: { type: 'guest', … }` → delete the rule; expose the records
36+
via a public form or share link instead.
37+
- `type: 'owner'` rules → rewrite as a `type: 'criteria'` rule scoping the
38+
rows by field values (see the migrated examples:
39+
`share_open_tasks_with_manager` in app-showcase,
40+
`share_active_leads_with_manager` in app-crm), or use a scope-depth grant.

content/docs/permissions/sharing-rules.mdx

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -132,27 +132,32 @@ export const AccountTeamSharingRule = defineSharingRule({
132132

133133
### Recipient types
134134

135-
`sharedWith` (and `ownedBy`) accept a `{ type, value }` recipient:
135+
`sharedWith` accepts a `{ type, value }` recipient. **Every authorable
136+
recipient is enforced**each expands to concrete users at seed time and
137+
materializes `sys_record_share` grants:
136138

137139
| `type` | Shares with |
138140
|:--|:--|
139141
| `user` | A single user |
140-
| `group` | All members of a public group**declared but not enforced**: skipped at seed time with a warning (ADR-0049) |
142+
| `team` | All members of a `sys_team` (flat collaboration grouping) |
141143
| `position` | Everyone assigned that position (flat expansionpositions have no tree) |
142144
| `unit_and_subordinates` | Everyone in that **business unit and every unit beneath it** (the BU tree is the one hierarchyADR-0090 D3) |
143-
| `guest` | Public / anonymous (`guest`) principals**declared but not enforced**: no runtime recipient mapping, skipped at seed time with a warning (ADR-0049) |
145+
| `business_unit` | Everyone in exactly that business unit (no subtree) |
144146

145147
A criteria `condition` must be compilable by the CELfilter pushdown
146148
compiler. A condition the compiler cannot lower is **skipped and logged
147149
never seeded as a permissive match-all** (ADR-0049): a bad condition
148150
under-shares rather than over-shares.
149151

150-
## Owner-Based Sharing Rules
151-
152-
> **[Experimentalnot enforced.]** Owner-based (`type: 'owner'`) rules
153-
> depend on live position membership and have no static `criteria_json`
154-
> equivalent, so the seeder **skips them with a warning**they materialize
155-
> no record shares today. Use a criteria rule or a scope-depth grant.
152+
> **Retired shapes.** The pre-ADR-0090 `group` recipient was renamed to
153+
> `team`, and the `guest` recipient was removedanonymous access is served
154+
> by the [public-form grant](/docs/permissions/authorization) and share
155+
> links, not sharing rules. Owner-based rules (`type: 'owner'`, `ownedBy`)
156+
> were also removed: they depended on live membership the static seeder
157+
> cannot track, so they validated but never materialized a share. Use a
158+
> criteria rule or a scope-depth grant instead; none of these shapes parses
159+
> anymore, so a stale definition fails loudly at authoring time instead of
160+
> silently doing nothing (ADR-0078).
156161

157162
## Analytics and Dataset Read Scope
158163

content/docs/references/security/sharing.mdx

Lines changed: 6 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ The baseline security posture for an object.
1616
## TypeScript Usage
1717

1818
```typescript
19-
import { CriteriaSharingRule, OWDModel, OwnerSharingRule, ShareRecipientType, SharingLevel, SharingRule, SharingRuleType } from '@objectstack/spec/security';
20-
import type { CriteriaSharingRule, OWDModel, OwnerSharingRule, ShareRecipientType, SharingLevel, SharingRule, SharingRuleType } from '@objectstack/spec/security';
19+
import { CriteriaSharingRule, OWDModel, ShareRecipientType, SharingLevel, SharingRule, SharingRuleType } from '@objectstack/spec/security';
20+
import type { CriteriaSharingRule, OWDModel, ShareRecipientType, SharingLevel, SharingRule, SharingRuleType } from '@objectstack/spec/security';
2121

2222
// Validate data
2323
const result = CriteriaSharingRule.parse(data);
@@ -37,7 +37,7 @@ const result = CriteriaSharingRule.parse(data);
3737
| **object** | `string` || Target Object Name |
3838
| **active** | `boolean` | optional | |
3939
| **accessLevel** | `Enum<'read' \| 'edit' \| 'full'>` | optional | |
40-
| **sharedWith** | `{ type: Enum<'user' \| 'group' \| 'position' \| 'unit_and_subordinates' \| 'guest'>; value: string }` || The recipient of the shared access |
40+
| **sharedWith** | `{ type: Enum<'user' \| 'team' \| 'position' \| 'unit_and_subordinates' \| 'business_unit'>; value: string }` || The recipient of the shared access |
4141
| **type** | `'criteria'` || |
4242
| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` || Predicate (CEL). e.g. P`record.department == "Sales"` |
4343

@@ -54,36 +54,17 @@ const result = CriteriaSharingRule.parse(data);
5454
* `controlled_by_parent`
5555

5656

57-
---
58-
59-
## OwnerSharingRule
60-
61-
### Properties
62-
63-
| Property | Type | Required | Description |
64-
| :--- | :--- | :--- | :--- |
65-
| **name** | `string` || Unique rule name (snake_case) |
66-
| **label** | `string` | optional | Human-readable label |
67-
| **description** | `string` | optional | Administrative notes |
68-
| **object** | `string` || Target Object Name |
69-
| **active** | `boolean` || |
70-
| **accessLevel** | `Enum<'read' \| 'edit' \| 'full'>` || |
71-
| **sharedWith** | `{ type: Enum<'user' \| 'group' \| 'position' \| 'unit_and_subordinates' \| 'guest'>; value: string }` || The recipient of the shared access |
72-
| **type** | `'owner'` || |
73-
| **ownedBy** | `{ type: Enum<'user' \| 'group' \| 'position' \| 'unit_and_subordinates' \| 'guest'>; value: string }` || Source group/position whose records are being shared |
74-
75-
7657
---
7758

7859
## ShareRecipientType
7960

8061
### Allowed Values
8162

8263
* `user`
83-
* `group`
64+
* `team`
8465
* `position`
8566
* `unit_and_subordinates`
86-
* `guest`
67+
* `business_unit`
8768

8869

8970
---
@@ -101,14 +82,6 @@ const result = CriteriaSharingRule.parse(data);
10182

10283
## SharingRule
10384

104-
### Union Options
105-
106-
This schema accepts one of the following structures:
107-
108-
#### Option 1
109-
110-
**Type:** `criteria`
111-
11285
### Properties
11386

11487
| Property | Type | Required | Description |
@@ -119,40 +92,17 @@ This schema accepts one of the following structures:
11992
| **object** | `string` || Target Object Name |
12093
| **active** | `boolean` | optional | |
12194
| **accessLevel** | `Enum<'read' \| 'edit' \| 'full'>` | optional | |
122-
| **sharedWith** | `{ type: Enum<'user' \| 'group' \| 'position' \| 'unit_and_subordinates' \| 'guest'>; value: string }` || The recipient of the shared access |
95+
| **sharedWith** | `{ type: Enum<'user' \| 'team' \| 'position' \| 'unit_and_subordinates' \| 'business_unit'>; value: string }` || The recipient of the shared access |
12396
| **type** | `'criteria'` || |
12497
| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` || Predicate (CEL). e.g. P`record.department == "Sales"` |
12598

126-
---
127-
128-
#### Option 2
129-
130-
**Type:** `owner`
131-
132-
### Properties
133-
134-
| Property | Type | Required | Description |
135-
| :--- | :--- | :--- | :--- |
136-
| **name** | `string` || Unique rule name (snake_case) |
137-
| **label** | `string` | optional | Human-readable label |
138-
| **description** | `string` | optional | Administrative notes |
139-
| **object** | `string` || Target Object Name |
140-
| **active** | `boolean` | optional | |
141-
| **accessLevel** | `Enum<'read' \| 'edit' \| 'full'>` | optional | |
142-
| **sharedWith** | `{ type: Enum<'user' \| 'group' \| 'position' \| 'unit_and_subordinates' \| 'guest'>; value: string }` || The recipient of the shared access |
143-
| **type** | `'owner'` || |
144-
| **ownedBy** | `{ type: Enum<'user' \| 'group' \| 'position' \| 'unit_and_subordinates' \| 'guest'>; value: string }` || Source group/position whose records are being shared |
145-
146-
---
147-
14899

149100
---
150101

151102
## SharingRuleType
152103

153104
### Allowed Values
154105

155-
* `owner`
156106
* `criteria`
157107

158108

examples/app-crm/src/security/sharing-rules.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,21 @@ export const HighValueOpportunitySharingRule = defineSharingRule({
2222
});
2323

2424
/**
25-
* Owner-based sharing: leads owned by a Sales Rep are shared read-only
26-
* with their manager so managers can coach on individual pipelines.
25+
* Criteria-based sharing: in-flight leads (not yet converted or disqualified)
26+
* are shared read-only with Sales Managers for coaching visibility.
27+
*
28+
* Replaces the retired owner-based `share_rep_leads_with_manager` rule:
29+
* `type: 'owner'` (`ownedBy`) no longer parses — it depended on live position
30+
* membership and was silently skipped at seed time (ADR-0078). The enforced
31+
* criteria form scopes the same coaching set by pipeline state instead.
2732
*/
2833
export const RepLeadSharingRule = defineSharingRule({
29-
type: 'owner',
30-
name: 'share_rep_leads_with_manager',
31-
label: "Rep's Leads → Manager (read-only)",
32-
description: "Share each rep's leads with the Sales Manager role for coaching visibility.",
34+
type: 'criteria',
35+
name: 'share_active_leads_with_manager',
36+
label: 'Active Leads → Manager (read-only)',
37+
description: 'Share in-flight (not converted/disqualified) leads with Sales Managers for coaching visibility.',
3338
object: 'crm_lead',
34-
ownedBy: {
35-
type: 'position',
36-
value: 'sales_rep',
37-
},
39+
condition: "record.status != 'converted' && record.status != 'disqualified'",
3840
accessLevel: 'read',
3941
sharedWith: {
4042
type: 'position',

examples/app-crm/test/smoke.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,12 @@ describe('app-crm minimal metadata bundle', () => {
8888
expect(stack.i18n!.supportedLocales).toContain('zh-CN');
8989
});
9090

91-
it('has sharing rules (criteria + owner types)', () => {
91+
it('has criteria sharing rules (the enforced form — owner-type was retired)', () => {
9292
const rules = stack.sharingRules ?? [];
9393
expect(rules.length).toBeGreaterThanOrEqual(2);
94-
expect(rules.some((r) => r.type === 'criteria')).toBe(true);
95-
expect(rules.some((r) => r.type === 'owner')).toBe(true);
94+
// `type: 'owner'` no longer parses (never enforced; ADR-0078): every
95+
// declared rule is the enforced criteria form.
96+
expect(rules.every((r) => r.type === 'criteria')).toBe(true);
9697
});
9798

9899
// #3420 — official examples must boot warning-free. A generic (non-better-auth)

examples/app-showcase/src/security/sharing-rules.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -72,22 +72,23 @@ export const NewInquiryFieldOpsRule = defineSharingRule({
7272
});
7373

7474
/**
75-
* owner-based: a contributor's tasks are shared read-only with managers.
75+
* criteria-based: open (not-done) tasks are shared read-only with managers
76+
* for oversight.
7677
*
77-
* [experimental — not enforced] Owner-type rules depend on live position
78-
* membership and have no static `criteria_json` equivalent, so the seed
79-
* bootstrap SKIPS them (logged) rather than seeding a permissive match-all
80-
* (ADR-0049: nothing silently over-shares). Kept here to demonstrate the
81-
* authoring shape; managers actually reach contributor tasks via the
82-
* criteria rules above and their depth grants.
78+
* This replaces the retired owner-based `share_contributor_tasks_with_manager`
79+
* demonstration rule: `type: 'owner'` (`ownedBy`) no longer parses — it
80+
* depended on live position membership, which the static materialiser cannot
81+
* track, so it validated but was silently skipped at seed time (ADR-0078:
82+
* nothing on the authoring surface may be silently inert). The enforced
83+
* equivalent is a criteria rule scoping the rows managers need.
8384
*/
8485
export const ContributorTaskSharingRule = defineSharingRule({
85-
type: 'owner',
86-
name: 'share_contributor_tasks_with_manager',
87-
label: 'Contributor Tasks → Manager',
88-
description: "Share each contributor's tasks with managers for oversight.",
86+
type: 'criteria',
87+
name: 'share_open_tasks_with_manager',
88+
label: 'Open Tasks → Manager',
89+
description: 'Share open (not-done) tasks with managers for oversight.',
8990
object: 'showcase_task',
90-
ownedBy: { type: 'position', value: 'contributor' },
91+
condition: 'record.done == false',
9192
accessLevel: 'read',
9293
sharedWith: { type: 'position', value: 'manager' },
9394
active: true,

packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,21 @@
44
* bootstrapDeclaredSharingRules — seed stack-declared `sharingRules` into
55
* `sys_sharing_rule` (ADR-0057 D6, closes #2077; reconciles #1887).
66
*
7-
* The spec authoring shape (`SharingRuleSchema`: CEL `condition`, `ownedBy`,
7+
* The spec authoring shape (`SharingRuleSchema`: CEL `condition`,
88
* `sharedWith{type,value}`) diverges from the enforced runtime shape
99
* (`criteria_json` JSON filter + `recipient_type`/`recipient_id`). ADR-0057 D6
10-
* makes the RUNTIME shape canonical and translates only the directly-mappable
11-
* fields. Parts the runtime cannot enforce statically are SKIPPED (logged
12-
* `[experimental]`) rather than seeded as a match-all rule — silently
13-
* over-sharing would be worse than not enforcing (ADR-0049):
14-
* - `owner`-type rules (`ownedBy`): role membership is dynamic, no static
15-
* `criteria_json` equivalent.
10+
* makes the RUNTIME shape canonical and translates the authorable fields.
11+
* Every currently-authorable recipient (`user` / `team` / `position` /
12+
* `unit_and_subordinates` / `business_unit`) maps 1:1 and ENFORCES — the
13+
* retired `group`/`guest` recipients and `owner`-type rules no longer parse
14+
* at the spec (ADR-0078; `group` was renamed → `team`). What the runtime
15+
* still cannot enforce is SKIPPED (logged) rather than seeded as a match-all
16+
* rule — silently over-sharing would be worse than not enforcing (ADR-0049):
1617
* - a CEL `condition` the canonical compiler cannot lower (functions,
1718
* cross-object traversal) — ADR-0058 D2. Compound predicates (AND/OR,
18-
* comparisons, null, in) DO lower now and are enforced (ADR-0058 D3, #1887).
19-
* - `sharedWith.type` of `group`/`guest`: no runtime recipient mapping.
19+
* comparisons, null, in) DO lower and are enforced (ADR-0058 D3, #1887).
20+
* - defensively, any stale pre-built package that still registers an old
21+
* `owner`-type / unmapped-recipient shape.
2022
*
2123
* Seeding upserts via `SharingRuleService.defineRule` (idempotent by name) and
2224
* MUST run before `listRules()`/`bindRuleHooks` so the lifecycle hooks bind to
@@ -35,11 +37,17 @@ type Logger = { info?: (m: string, meta?: any) => void; warn?: (m: string, meta?
3537
function mapRecipientType(t: unknown): SharingRuleRecipientType | null {
3638
switch (t) {
3739
case 'user': return 'user';
40+
// Flat sys_team membership (ADR-0090 D3 vocabulary; the pre-D3 `group`
41+
// spelling was retired) — expanded by TeamGraphService in expandRecipient.
42+
case 'team': return 'team';
3843
case 'position': return 'position';
3944
// ADR-0057 D5: business-unit subtree recipient.
4045
case 'business_unit': return 'business_unit' as SharingRuleRecipientType;
4146
case 'unit_and_subordinates': return 'unit_and_subordinates' as SharingRuleRecipientType;
42-
default: return null; // group / guest — no runtime mapping yet
47+
// Defensive only: the authoring enum matches the cases above 1:1, but a
48+
// stale pre-built package could still register a retired shape — skip,
49+
// never seed match-all.
50+
default: return null;
4351
}
4452
}
4553

@@ -96,9 +104,11 @@ export async function bootstrapDeclaredSharingRules(
96104
logger?.warn?.('[sharing-rule] skipped (unmappable recipient) [experimental]', { rule: r.name, sharedWith: r.sharedWith?.type });
97105
skipped += 1; continue;
98106
}
99-
// owner-type rules have no static criteria_json equivalent.
107+
// Defensive: `owner`-type rules were removed from the authoring spec
108+
// (live-membership-dependent, no static criteria_json equivalent) — this
109+
// guards stale pre-built packages that still register the old shape.
100110
if (r.type === 'owner') {
101-
logger?.warn?.('[sharing-rule] skipped owner-based rule (no static criteria) [experimental]', { rule: r.name });
111+
logger?.warn?.('[sharing-rule] skipped owner-based rule (retired authoring shape — use a criteria rule)', { rule: r.name });
102112
skipped += 1; continue;
103113
}
104114
// criteria rules: translate CEL → filter. Empty condition = match-all (intentional).

packages/qa/dogfood/test/authz-conformance.matrix.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,8 @@ export const AUTHZ_CONFORMANCE: AuthzPrimitive[] = [
141141
note: 'Proof file carries the forged owner_id/organization_id submit case; the route-level matrix is covered unit-side in rest public-form-routes.test.ts + plugin-security security-plugin.test.ts (publicFormGrant strip suite).' },
142142
{ id: 'record-share', summary: 'manual record shares (sys_record_share)', state: 'enforced',
143143
enforcement: 'plugin-sharing/sharing-service.ts buildReadFilter/canEdit' },
144-
{ id: 'sharing-rules', summary: 'criteria/owner sharing rules', state: 'enforced',
145-
enforcement: 'plugin-sharing/sharing-rule-service.ts (materialized into sys_record_share)', proof: 'showcase-bu-hierarchy-sharing.dogfood.test.ts' },
144+
{ id: 'sharing-rules', summary: 'criteria sharing rules (recipients: user/team/position/unit_and_subordinates/business_unit)', state: 'enforced',
145+
enforcement: 'plugin-sharing/sharing-rule-service.ts (materialized into sys_record_share); every authorable recipient expands in expandRecipient. The never-enforced owner-type rules + group/guest recipients were removed from the authoring spec (ADR-0078; group renamed → team)', proof: 'showcase-bu-hierarchy-sharing.dogfood.test.ts' },
146146
{ id: 'hierarchy-widening', summary: 'hierarchy widening — a unit + its subordinate units gain access', state: 'enforced',
147147
enforcement: 'plugin-sharing/business-unit-graph.ts BusinessUnitGraphService subtree (business_unit recipient) — ADR-0057 D5 re-homed off the never-existent sys_position.parent', proof: 'showcase-bu-hierarchy-sharing.dogfood.test.ts' },
148148
{ id: 'rls-compiler-fail-closed', summary: 'uncompilable RLS predicate is surfaced/denied, not dropped', state: 'enforced',

0 commit comments

Comments
 (0)