Skip to content

Commit f00d8d4

Browse files
authored
fix(sharing)!: remove the full access level — it promised delete/transfer/share and granted edit (#3865) (#3901)
`sys_sharing_rule.access_level` / `sys_record_share.access_level` offered three levels, the third documented as "Full Access (Transfer, Share, Delete)". No code path granted transfer, re-share, or delete because of it: both enforcement sites matched `access_level in ('edit','full')`, so `full` was byte-equivalent to `edit` while telling admins otherwise — declared-but-unenforced metadata (ADR-0078, ADR-0049), the same defect that retired the `queue` recipient. Record sharing widens WHICH ROWS a principal reaches, never WHICH VERBS they may use — the split Salesforce enforces (sharing rules stop at Read-Only / Read-Write) and Dataverse enforces by AND-ing shared access rights against the security role's own privilege. Delete and transfer belong to ownership, the ADR-0057 DEPTH scopes, and admin scope. Authoring rejects, enforcement tolerates, data normalises (the ADR-0090 D4 idiom): - `SharingLevel` / `ShareAccessLevel` narrow to `read | edit`; both objects' `Field.select` follow, so the Setup dropdown drops the misleading option. - `grant()` / `defineRule()` gain the access-level validation they never had — previously ANY value was persisted verbatim, so a typo'd level became a grant no gate would ever match. `full` normalises to `edit`; anything unrecognised is a `VALIDATION_FAILED` mapped to HTTP 400. - The read/write gates keep matching `edit`/`full` on purpose: narrowing them would silently REVOKE every not-yet-migrated grant. - A boot backfill normalises stored `full` rows (writing with `isSystem` so the provenance hook does not mark a package-seeded rule `customized`), and the protocol-17 `sharing-rule-access-level-full-to-edit` conversion rewrites declarative stacks at load. Lossless by construction — the two levels were already equivalent, so the rewrite cannot change an access decision. The explain surfaces keep accepting `'full'`: they REPORT stored rows, which must stay explainable. Closes #3865
1 parent 8f9689f commit f00d8d4

27 files changed

Lines changed: 736 additions & 56 deletions
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/plugin-sharing": minor
4+
"@objectstack/plugin-security": patch
5+
---
6+
7+
fix(sharing): remove the `full` access level — it promised delete/transfer/share and granted `edit` (#3865)
8+
9+
`sys_sharing_rule.access_level` / `sys_record_share.access_level` offered three
10+
levels, the third documented as **Full Access (Transfer, Share, Delete)**. No
11+
code path granted transfer, re-share, or delete because of it: both enforcement
12+
sites matched `access_level in ('edit','full')`, so `full` was byte-equivalent
13+
to `edit`. An admin picking "Full Access" in Setup was told they had granted
14+
delete rights and had not — declared-but-unenforced metadata (ADR-0078,
15+
ADR-0049), the same defect that retired the `queue` recipient before it.
16+
17+
Measured on showcase, a `full` recipient got `read: allowed`, `update: allowed`,
18+
`delete: DENIED` — and the denial came from `decidedBy=object_crud`, i.e. the
19+
object-level CRUD gate rejected the delete *before* sharing was consulted at
20+
all. That is not an oversight to patch around; it is the model working. Record
21+
sharing widens **which rows** a principal reaches, never **which verbs** they
22+
may use — the same split Salesforce enforces (its sharing rules stop at
23+
Read-Only / Read-Write; Full Access is owner / hierarchy / Modify All only,
24+
never grantable by a rule) and Dataverse enforces by AND-ing every shared access
25+
right against the security role's own privilege. Delete and transfer belong to
26+
ownership, the ADR-0057 DEPTH scopes, and admin scope.
27+
28+
**What changed**
29+
30+
- `SharingLevel` (spec/security) and `ShareAccessLevel` (spec/contracts) are now
31+
`read | edit`. The `Field.select` on both objects offers the same two, so the
32+
Setup dropdown no longer shows the misleading option.
33+
- `SharingService.grant()` and `SharingRuleService.defineRule()` gained the
34+
access-level validation they never had: `full` normalises to `edit`, and an
35+
unrecognised level is a `VALIDATION_FAILED` (HTTP 400) instead of being
36+
persisted verbatim as a grant no gate would ever match.
37+
- Enforcement stays deliberately wider than authoring — the read/write gates
38+
still match `edit`/`full` — so a row written before this release keeps
39+
working. Narrowing them would silently *revoke* access.
40+
- A boot backfill normalises stored `full` rows on both tables, and the
41+
`sharing-rule-access-level-full-to-edit` conversion rewrites declarative
42+
stacks at load, so nothing needs consumer action.
43+
44+
**Migration.** None. `full` and `edit` were already behaviourally identical, so
45+
rewriting one to the other cannot change an access decision — unlike the OWD
46+
`sharingModel: 'full'` alias retired in ADR-0090 D4, which changed posture and
47+
had to be delegated to the author. A stack that still authors `accessLevel:
48+
'full'` converts at load with a deprecation notice; stored rows normalise at
49+
next boot. Code that pinned the `ShareAccessLevel` type to `'full'` no longer
50+
compiles — use `'edit'`.
51+
52+
Reviving a real per-record delete grant is a separate design (a capability mask
53+
AND-ed with object CRUD, plus the share-administration model that would have to
54+
authorise re-sharing), not a fourth enum member.

content/docs/permissions/sharing-rules.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ export const AccountTeamSharingRule = defineSharingRule({
125125
// Who to share with (a single recipient — see the recipient types below)
126126
sharedWith: { type: 'position', value: 'sales_manager' },
127127
128-
// Access level granted: read | edit | full
128+
// Access level granted: read | edit
129129
accessLevel: 'edit',
130130
});
131131
```

content/docs/protocol/objectql/security.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ Share records whose fields match a CEL predicate (`CriteriaSharingRuleSchema` in
280280
name: share_enterprise_accounts
281281
type: criteria
282282
object: account
283-
accessLevel: read # read | edit | full
283+
accessLevel: read # read | edit
284284
condition: 'record.account_type == "Enterprise"'
285285
sharedWith:
286286
type: position # user | group | position | unit_and_subordinates | guest
@@ -308,7 +308,7 @@ sharedWith:
308308

309309
> **Enforcement status.** Criteria rules with `user` / `position` / `unit_and_subordinates` recipients compile and enforce (the CEL condition lowers to a runtime filter that materializes `sys_record_share` grants, ADR-0058 D3). Owner-type rules and `group`/`guest` recipients are `[experimental — not enforced]`: the seed bootstrap skips them (logged) rather than seeding a permissive match-all (ADR-0049).
310310

311-
> `accessLevel` is one of `read`, `edit`, or `full`. `full` additionally grants transfer/share/delete.
311+
> `accessLevel` is one of `read` or `edit`. Sharing widens **which rows** a principal reaches, never **which verbs** they may use — delete and transfer come from ownership, the ADR-0057 DEPTH scopes, or admin scope, and are checked by the object-level CRUD gate before sharing is consulted at all. A third level `full` ("Full Access — transfer/share/delete") was authorable through protocol 16 but never granted any of those verbs: both enforcement sites matched `edit`/`full` alike, so it was equivalent to `edit` while telling admins otherwise, and it was removed (#3865, ADR-0078). Stacks still authoring it are rewritten to `edit` at load by the `sharing-rule-access-level-full-to-edit` conversion.
312312

313313
### Public Share Links
314314

content/docs/references/security/explain.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ ADR-0095 D2 posture rung — PLATFORM_ADMIN crosses the tenant wall where object
153153
| :--- | :--- | :--- | :--- |
154154
| **kind** | `Enum<'tenant_filter' \| 'owd_baseline' \| 'ownership' \| 'record_share' \| 'sharing_rule' \| 'team' \| 'territory' \| 'rls_policy'>` || The row-visibility source kind evaluated for this record at this layer. |
155155
| **name** | `string` || Stable identifier of the concrete rule, share, or policy that was evaluated. |
156-
| **grants** | `Enum<'read' \| 'edit' \| 'full'>` | optional | Access level a sharing source grants on the record (mirrors SharingLevel). |
156+
| **grants** | `Enum<'read' \| 'edit' \| 'full'>` | optional | Access level a sharing source grants on the record (authorable: read/edit; `full` appears only for legacy rows pending normalisation). |
157157
| **via** | `string` | optional | How the rule reached the principal — recipient group/position, ownership, or the matching criteria. |
158158
| **predicate** | `any` | optional | The row predicate this rule contributed, when it is filter-shaped (null = unrestricted). |
159159
| **effect** | `Enum<'admits' \| 'excludes' \| 'neutral'>` || The rule's effect on THIS record: admits, excludes, or neutral. |

content/docs/references/security/sharing.mdx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const result = CriteriaSharingRule.parse(data);
3636
| **description** | `string` | optional | Administrative notes |
3737
| **object** | `string` || Target Object Name |
3838
| **active** | `boolean` | optional | |
39-
| **accessLevel** | `Enum<'read' \| 'edit' \| 'full'>` | optional | |
39+
| **accessLevel** | `Enum<'read' \| 'edit'>` | optional | |
4040
| **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"` |
@@ -75,7 +75,6 @@ const result = CriteriaSharingRule.parse(data);
7575

7676
* `read`
7777
* `edit`
78-
* `full`
7978

8079

8180
---
@@ -91,7 +90,7 @@ const result = CriteriaSharingRule.parse(data);
9190
| **description** | `string` | optional | Administrative notes |
9291
| **object** | `string` || Target Object Name |
9392
| **active** | `boolean` | optional | |
94-
| **accessLevel** | `Enum<'read' \| 'edit' \| 'full'>` | optional | |
93+
| **accessLevel** | `Enum<'read' \| 'edit'>` | optional | |
9594
| **sharedWith** | `{ type: Enum<'user' \| 'team' \| 'position' \| 'unit_and_subordinates' \| 'business_unit'>; value: string }` || The recipient of the shared access |
9695
| **type** | `'criteria'` || |
9796
| **condition** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` || Predicate (CEL). e.g. P`record.department == "Sales"` |

docs/notes/crm-development-standards.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ export const MySharingRule = defineSharingRule({
294294
value: 'role_name',
295295
},
296296

297-
accessLevel: 'edit', // 'read' | 'edit' | 'full'
297+
accessLevel: 'edit', // 'read' | 'edit'
298298
});
299299
```
300300

packages/plugins/plugin-security/src/explain-engine.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,15 @@ export interface ExplainEngineDeps {
188188
fetchRecord?: (object: string, recordId: string, engineOperation: string) => Promise<Record<string, unknown> | null>;
189189
/** The sharing service's own read-filter contribution for the object (owner-match OR granted-ids), same as enforcement AND-s in. */
190190
sharingReadFilter?: (object: string, context: any) => Promise<unknown | null>;
191-
/** The concrete `sys_record_share` rows attached to the record (for `rules[]` attribution). */
191+
/**
192+
* The concrete `sys_record_share` rows attached to the record (for `rules[]`
193+
* attribution).
194+
*
195+
* `access_level` stays wider than the authorable `read`/`edit`: explain
196+
* REPORTS stored rows, and a `full` row written before #3865 retired that
197+
* level (normalised to `edit` by the sharing plugin's boot backfill) must
198+
* still be explainable rather than crash the panel.
199+
*/
192200
listRecordShares?: (
193201
object: string,
194202
recordId: string,
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#3865] The access-level vocabulary after `full` was retired.
5+
*
6+
* `full` was declared "Full Access (Transfer, Share, Delete)" but every
7+
* enforcement site matched `access_level in ('edit','full')`, so it granted
8+
* exactly what `edit` grants while telling admins otherwise — the
9+
* declared-but-unenforced trap ADR-0078 bans. These tests pin the three-layer
10+
* posture that replaced it: authoring rejects, enforcement tolerates, data
11+
* normalises.
12+
*/
13+
14+
import { describe, it, expect } from 'vitest';
15+
import {
16+
ACCESS_LEVELS,
17+
WRITE_ACCESS_LEVELS,
18+
isKnownAccessLevel,
19+
normalizeAccessLevel,
20+
normalizeStoredAccessLevel,
21+
} from './access-level.js';
22+
23+
describe('access-level vocabulary', () => {
24+
it('offers exactly read and edit for authoring', () => {
25+
expect([...ACCESS_LEVELS]).toEqual(['read', 'edit']);
26+
});
27+
28+
it('keeps the write gate wider than the authorable set', () => {
29+
// The whole point of the tolerance layer: a row persisted before the boot
30+
// backfill ran must keep working. If this ever narrows to ['edit'], every
31+
// un-migrated `full` grant silently loses write access.
32+
expect([...WRITE_ACCESS_LEVELS]).toEqual(['edit', 'full']);
33+
});
34+
});
35+
36+
describe('normalizeAccessLevel (authoring path)', () => {
37+
it('passes the authorable levels through unchanged', () => {
38+
expect(normalizeAccessLevel('read')).toBe('read');
39+
expect(normalizeAccessLevel('edit')).toBe('edit');
40+
});
41+
42+
it("normalises the retired 'full' to 'edit'", () => {
43+
// Lossless precisely because `full` was inert — the two already behaved
44+
// identically, so no principal gains or loses anything.
45+
expect(normalizeAccessLevel('full')).toBe('edit');
46+
});
47+
48+
it('throws VALIDATION_FAILED on an unrecognised level', () => {
49+
// Coercing an unknown level would recreate the bug in a new spot: a grant
50+
// that looks issued and that no gate matches.
51+
expect(() => normalizeAccessLevel('admin')).toThrow(/VALIDATION_FAILED/);
52+
expect(() => normalizeAccessLevel('owner')).toThrow(/VALIDATION_FAILED/);
53+
expect(() => normalizeAccessLevel(7)).toThrow(/VALIDATION_FAILED/);
54+
});
55+
56+
it('applies the fallback only for a missing value', () => {
57+
expect(normalizeAccessLevel(undefined, 'read')).toBe('read');
58+
expect(normalizeAccessLevel(null, 'edit')).toBe('edit');
59+
expect(normalizeAccessLevel('', 'read')).toBe('read');
60+
// A PRESENT but invalid value must never fall back — that would swallow
61+
// the author's mistake.
62+
expect(() => normalizeAccessLevel('nope', 'read')).toThrow(/VALIDATION_FAILED/);
63+
});
64+
65+
it('requires a level when no fallback is supplied', () => {
66+
expect(() => normalizeAccessLevel(undefined)).toThrow(/VALIDATION_FAILED/);
67+
});
68+
});
69+
70+
describe('normalizeStoredAccessLevel (read path)', () => {
71+
it('normalises known levels without throwing', () => {
72+
expect(normalizeStoredAccessLevel('read')).toBe('read');
73+
expect(normalizeStoredAccessLevel('edit')).toBe('edit');
74+
expect(normalizeStoredAccessLevel('full')).toBe('edit');
75+
});
76+
77+
it('fails CLOSED to read on unrecognised or missing stored data', () => {
78+
// Never throws: projecting an existing row must not take down the rule
79+
// evaluator or an admin list view over one bad byte. Degrades to the
80+
// NARROWEST level rather than trusting it.
81+
expect(normalizeStoredAccessLevel('admin')).toBe('read');
82+
expect(normalizeStoredAccessLevel(undefined)).toBe('read');
83+
expect(normalizeStoredAccessLevel(null)).toBe('read');
84+
expect(normalizeStoredAccessLevel({})).toBe('read');
85+
});
86+
});
87+
88+
describe('isKnownAccessLevel', () => {
89+
it('accepts authorable and retired-but-stored levels', () => {
90+
expect(isKnownAccessLevel('read')).toBe(true);
91+
expect(isKnownAccessLevel('edit')).toBe(true);
92+
expect(isKnownAccessLevel('full')).toBe(true);
93+
});
94+
95+
it('rejects anything else', () => {
96+
expect(isKnownAccessLevel('admin')).toBe(false);
97+
expect(isKnownAccessLevel(undefined)).toBe(false);
98+
expect(isKnownAccessLevel(1)).toBe(false);
99+
});
100+
});
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Share access-level vocabulary — the one place `read` / `edit` (and the
5+
* retired `full`) are defined for this plugin.
6+
*
7+
* ## Why `full` is gone (#3865)
8+
*
9+
* `full` was declared as "Full Access (Transfer, Share, Delete)" but **no code
10+
* path ever granted transfer, re-share, or delete because of it**: the read
11+
* gate (`buildReadFilter`), the bulk-write gate (`buildWriteFilter`) and the
12+
* per-record gate (`canEdit`) all matched `access_level in ('edit','full')`, so
13+
* it was byte-equivalent to `edit`. An admin picking "Full Access" in Setup was
14+
* told they had granted delete rights and had not — declared-but-unenforced
15+
* metadata, the exact authoring trap ADR-0078 / ADR-0049 ban, and the reason
16+
* `recipient_type: 'queue'` was removed before it.
17+
*
18+
* It is not coming back as an enum member. Record sharing widens **which rows**
19+
* a principal reaches, never **which verbs** they may use — the same split
20+
* Salesforce enforces (its sharing rules stop at Read-Only / Read-Write; Full
21+
* Access is owner / hierarchy / Modify All only, never grantable by a rule) and
22+
* Dataverse enforces by AND-ing every shared access right against the security
23+
* role's own privilege. Delete and transfer belong to ownership, the ADR-0057
24+
* DEPTH scopes, and admin scope. A future per-record delete grant would be a
25+
* capability mask AND-ed with object CRUD, not a fourth level.
26+
*
27+
* ## The three-layer posture (ADR-0090 D4 idiom)
28+
*
29+
* 1. **Authoring rejects** — `SharingLevel` / the `Field.select` enums no longer
30+
* offer `full`; {@link normalizeAccessLevel} turns an explicit `full` from an
31+
* older client into `edit` and rejects anything unrecognised outright.
32+
* 2. **Runtime tolerates** — the enforcement gates keep matching
33+
* {@link WRITE_ACCESS_LEVELS}, so a row persisted before the backfill ran is
34+
* still honoured (it means `edit`; refusing it would silently REVOKE access).
35+
* 3. **Data normalises** — a boot backfill rewrites stored `full` rows to
36+
* `edit`. Behaviour-preserving by construction, because the two were already
37+
* equivalent.
38+
*/
39+
40+
import type { ShareAccessLevel } from '@objectstack/spec/contracts';
41+
42+
/** The authorable levels, in widening order. */
43+
export const ACCESS_LEVELS: readonly ShareAccessLevel[] = ['read', 'edit'];
44+
45+
/**
46+
* Retired spellings that map losslessly onto an authorable level.
47+
*
48+
* `full` → `edit` is lossless *because* `full` was inert: the two already
49+
* behaved identically, so rewriting one to the other cannot change any access
50+
* decision. (Contrast the OWD `sharingModel: 'full'` alias retired in ADR-0090
51+
* D4, which had no equivalent target and had to be delegated to the author.)
52+
*/
53+
const RETIRED_ACCESS_LEVELS: Readonly<Record<string, ShareAccessLevel>> = {
54+
full: 'edit',
55+
};
56+
57+
/**
58+
* Levels that open the write gate, INCLUDING the retired `full`.
59+
*
60+
* Deliberately wider than {@link ACCESS_LEVELS}: enforcement reads persisted
61+
* rows, which may predate normalisation. Dropping `full` here would silently
62+
* revoke access from every not-yet-migrated grant — a fail-open→fail-closed
63+
* flip nobody asked for. Authoring narrowness and enforcement tolerance are
64+
* different jobs.
65+
*/
66+
export const WRITE_ACCESS_LEVELS: readonly string[] = ['edit', 'full'];
67+
68+
/** `true` when `value` is a stored level this plugin still honours. */
69+
export function isKnownAccessLevel(value: unknown): boolean {
70+
return typeof value === 'string'
71+
&& (ACCESS_LEVELS.includes(value as ShareAccessLevel) || value in RETIRED_ACCESS_LEVELS);
72+
}
73+
74+
/**
75+
* Normalise an inbound access level to the authorable vocabulary.
76+
*
77+
* - `read` / `edit` pass through.
78+
* - `full` normalises to `edit` — quietly, because it is what `full` already
79+
* meant; failing an old client's request would be a regression with no
80+
* security benefit.
81+
* - anything else (including `null`/`undefined` when no `fallback` is given)
82+
* throws `VALIDATION_FAILED`, which the REST layer maps to a 400. Silently
83+
* coercing an unrecognised level would be the same silent-inertness bug in a
84+
* new spot: a caller asking for `admin` must be told it does not exist, not
85+
* handed a `read` grant they did not request.
86+
*
87+
* @param value the inbound level (typically from an HTTP body or a rule row)
88+
* @param fallback level to use when `value` is null/undefined; omit to require one
89+
*/
90+
export function normalizeAccessLevel(
91+
value: unknown,
92+
fallback?: ShareAccessLevel,
93+
): ShareAccessLevel {
94+
if (value == null || value === '') {
95+
if (fallback !== undefined) return fallback;
96+
throw new Error('VALIDATION_FAILED: accessLevel is required');
97+
}
98+
if (typeof value === 'string') {
99+
if (ACCESS_LEVELS.includes(value as ShareAccessLevel)) return value as ShareAccessLevel;
100+
const retired = RETIRED_ACCESS_LEVELS[value];
101+
if (retired) return retired;
102+
}
103+
throw new Error(
104+
`VALIDATION_FAILED: accessLevel must be one of ${ACCESS_LEVELS.join(' | ')} `
105+
+ `(received ${JSON.stringify(value)})`,
106+
);
107+
}
108+
109+
/**
110+
* Normalise a level read back OUT of storage. Never throws.
111+
*
112+
* The write-path twin ({@link normalizeAccessLevel}) rejects an unrecognised
113+
* level so the caller learns immediately. A read path cannot do that: the row
114+
* already exists, and throwing while projecting it would take down the rule
115+
* evaluator or the admin list view over one bad byte. So this one fails
116+
* **closed** — an unrecognised stored level degrades to `read`, the narrowest
117+
* level, rather than being trusted or crashing (the same fail-closed posture
118+
* `effectiveSharingModel` takes for an unknown `sharingModel`).
119+
*/
120+
export function normalizeStoredAccessLevel(value: unknown): ShareAccessLevel {
121+
try {
122+
return normalizeAccessLevel(value, 'read');
123+
} catch {
124+
return 'read';
125+
}
126+
}

0 commit comments

Comments
 (0)