Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions .changeset/adr-0111-verb-boundary-delete.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
---
"@objectstack/spec": minor
"@objectstack/plugin-sharing": minor
"@objectstack/plugin-security": minor
---

fix(sharing)!: an edit-level share no longer grants delete (ADR-0111 D3, the verb boundary)

`update` and `delete` shared one `canEdit` gate, and `canEdit` accepts an
`edit`-level share — so one "edit" grant silently conferred delete, the
opposite error from the retired `full` level. A share widens *which rows* a
principal reaches, never *which verbs* they may use (Salesforce Read/Write
cannot delete; Dataverse `Delete` is a distinct privilege; Odoo splits
`write`/`unlink`).

- `ISharingService.canDelete(object, recordId, context)` — ownership (widened
by write DEPTH) or the `modifyAllRecords` super-user bypass ONLY; an `edit`
or legacy `full` share does not confer it. `canEdit` is unchanged (the
update gate, share included).
- `SharingService.buildWriteFilter` takes a `verb` parameter: a bulk
`delete({multi:true})` scopes to the owner/DEPTH set alone (no share
widening), while a bulk `update` keeps it.
- The sharing middleware routes `delete` through `canDelete` and logs a
specific fail-closed reason on denial (ADR-0111 D10).
- `/security/explain` consults `canDelete` for a `delete` operation, so the
record-level explanation matches enforcement.

**Breaking**: a caller who could delete a record *only* through an edit-level
share (and holds object-level delete CRUD) can no longer delete it — delete now
requires ownership, write depth, or Modify All Data. No new delete access level
is introduced; a future per-record delete grant would be a capability mask
AND-ed with object CRUD, not a fourth share level.
2 changes: 1 addition & 1 deletion content/docs/kernel/index.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,7 +16,7 @@ The kernel is ObjectStack's runtime: it loads your metadata artifact, hosts plug
| API | Stability | What it does |
| :--- | :--- | :--- |
| [`services.data`](/docs/kernel/runtime-services/data-service) | stable | CRUD and queries with the caller's permission context |
| [`services.sharing`](/docs/kernel/runtime-services/sharing-service) | stable | `buildReadFilter`, `canEdit`, `canManageShares`, `grant`/`revoke`, `listShares` |
| [`services.sharing`](/docs/kernel/runtime-services/sharing-service) | stable | `buildReadFilter`, `canEdit`, `canDelete`, `canManageShares`, `grant`/`revoke`, `listShares` |
| [`services.email`](/docs/kernel/runtime-services/email-service) | stable | `send`, `sendTemplate` |
| [`services.queue`](/docs/kernel/runtime-services/queue-service) | stable | Background work and queues |
| [`services.settings`](/docs/kernel/runtime-services/settings-service) | stable | App/environment settings |
Expand Down
12 changes: 11 additions & 1 deletion content/docs/kernel/runtime-services/sharing-service.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -13,6 +13,7 @@ description: Record-level sharing and editability checks.
```ts
services.sharing.buildReadFilter(object: string, context: SharingExecutionContext): Promise<unknown | null>
services.sharing.canEdit(object: string, recordId: string, context: SharingExecutionContext): Promise<boolean>
services.sharing.canDelete(object: string, recordId: string, context: SharingExecutionContext): Promise<boolean>
services.sharing.canManageShares(object: string, recordId: string, context: SharingExecutionContext): Promise<boolean>
services.sharing.grant(input: GrantShareInput, context: SharingExecutionContext): Promise<RecordShare>
services.sharing.revoke(shareId: string, context: SharingExecutionContext, scope?: { object: string; recordId: string }): Promise<void>
Expand All@@ -27,10 +28,19 @@ the record — its owner, a holder of Modify All Data on the object, or system
context. A deployment without `@objectstack/plugin-security` fails closed to
owner-only. Pass `{ isSystem: true }` only from platform-internal machinery.

## The verb boundary (ADR-0111 D3)

A share widens *which rows* a principal reaches, never *which verbs* they may
use. `canEdit` (the **update** gate) accepts an `edit`-level share; `canDelete`
(the **delete** gate) does **not** — delete is ownership (widened by write
DEPTH) or the `modifyAllRecords` super-user bypass only. Delete is not a share
level and never will be; a future per-record delete grant would be a capability
mask AND-ed with object CRUD, not a fourth `access_level`.

## Returns

- `buildReadFilter`: `null` means unrestricted read; otherwise returns an engine filter
- `canEdit` / `canManageShares`: boolean decisions (they return `false` rather than throwing)
- `canEdit` / `canDelete` / `canManageShares`: boolean decisions (they return `false` rather than throwing)
- `grant`/`listShares`: normalized `RecordShare` rows

## Typical Errors
Expand Down
2 changes: 1 addition & 1 deletion content/docs/protocol/objectql/security.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -308,7 +308,7 @@ sharedWith:

> **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).

> `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.
> `accessLevel` is one of `read` or `edit`. Sharing widens **which rows** a principal reaches, never **which verbs** they may use — an `edit` share opens *update*, not *delete*: delete comes from ownership, the ADR-0057 DEPTH scopes, or the `modifyAllRecords` bypass, enforced by the sharing layer's own `canDelete` gate (distinct from the `canEdit` update gate) on top of the object-level CRUD gate (ADR-0111 D3). 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.

### Public Share Links

Expand Down
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
# ADR-0111: Record-share management authority and the verb boundary — sharing needs "who may manage a share" and "which verbs a level grants"

**Status**: Accepted (2026-07-30) — **P0 implemented**(D1/D2/D4/D5/D6/D7/D9: `canManageShares` + `hasWriteBypass`in `plugin-sharing/src/sharing-service.ts` / `plugin-security/src/security-plugin.ts`; verified by the #3902 Mallory reproduction in `plugin-sharing/src/sharing-service.test.ts` and the D6 gate suite in `sharing-rule.test.ts`). **D3 (verb boundary) and D8 (share-link rulings) are not yet implemented** — they land as the separate P1 / follow-up PRs this ADR's rollout section names.
**Status**: Accepted (2026-07-30) — **P0 + P1 implemented**. P0 (D1/D2/D4/D5/D6/D7/D9): `canManageShares` + `hasWriteBypass`, verified by the #3902 Mallory reproduction. P1 (D3, the verb boundary): `canDelete` + verb-split `buildWriteFilter` in `plugin-sharing/src/sharing-service.ts`, routed by the middleware and `/security/explain`, verified by the "edit share cannot delete" suite in `plugin-sharing/src/sharing-service.test.ts`. **D8 (share-link rulings) and the DEPTH management extension (D1 D-future) are not yet implemented** — they land as the follow-up PRs this ADR's rollout section names.
**Deciders**: ObjectStack Protocol Architects
**Builds on**: [ADR-0049](./0049-no-unenforced-security-properties.md) (enforce-or-remove — a security property that parses but enforces nothing is worse than absent), [ADR-0057](./0057-erp-authorization-core-business-units-and-scope-depth.md) (DEPTH scopes + the `sys_record_share` / `sys_sharing_rule` split), [ADR-0066](./0066-unified-authorization-model.md) (unified capability model; `modifyAllRecords` super-user bit), [ADR-0078](./0078-no-silently-inert-metadata.md) (no silently inert metadata — a persisted share level or recipient type that no gate consults is exactly this), [ADR-0090](./0090-permission-model-v2-concept-convergence.md) (D1 secure-default OWD, D4 retired aliases, D10 delegated identity intersection), [ADR-0091](./0091-grant-lifecycle-and-recertification.md) (time-boxed grants — the lifecycle axis this ADR deliberately does not re-open)
**Consumers**: `@objectstack/plugin-sharing` (`sharing-service.ts`, `sharing-rule-service.ts`, `share-link-service.ts`, `sharing-plugin.ts`), `@objectstack/plugin-security` (`ISecurityService` — a write-bypass probe), `@objectstack/rest` (`rest-server.ts` sharing / sharing-rule / share-link routes), `@objectstack/spec` (`contracts/sharing-service.ts`, `security/capabilities.ts`)
Expand Down
18 changes: 14 additions & 4 deletions packages/plugins/plugin-security/src/explain-engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -202,8 +202,15 @@ export interface ExplainEngineDeps {
recordId: string,
context: any,
) => Promise<Array<{ id?: string; recipient_type?: string; recipient_id?: string; access_level?: 'read' | 'edit' | 'full'; source?: string; source_id?: string }>>;
/** The sharing service's per-record write gate (`canEdit`) — the by-construction verdict for write operations. */
/** The sharing service's per-record UPDATE gate (`canEdit`) — the by-construction verdict for update operations. */
canEditRecord?: (object: string, recordId: string, context: any) => Promise<boolean>;
/**
* [ADR-0111 D3] The sharing service's per-record DELETE gate (`canDelete`) —
* the by-construction verdict for a delete operation. Narrower than
* `canEditRecord`: an edit-level share opens update but not delete, so the
* explanation for a `delete` must consult this rather than the update gate.
*/
canDeleteRecord?: (object: string, recordId: string, context: any) => Promise<boolean>;
}

export interface ExplainInput {
Expand DownExpand Up@@ -609,9 +616,12 @@ async function applyRecordAttribution(
? await deps.sharingReadFilter(object, context).catch(() => null)
: undefined;
const sharingMatches = sharingFilter === undefined ? undefined : matches(sharingFilter);
// Write ops: the by-construction verdict is the sharing service's own canEdit.
const canEdit = !isRead && deps.canEditRecord && recordExists
? await deps.canEditRecord(object, recordId, context).catch(() => undefined)
// Write ops: the by-construction verdict is the sharing service's own gate.
// [ADR-0111 D3] delete has its own narrower gate (an edit share does not
// confer delete), so a `delete` explanation consults canDelete, not canEdit.
const writeGate = engineOp === 'delete' ? deps.canDeleteRecord : deps.canEditRecord;
const canEdit = !isRead && writeGate && recordExists
? await writeGate(object, recordId, context).catch(() => undefined)
: undefined;
const anyShareAdmits = shareRules.some((r) => r.effect === 'admits');
let sharingOutcome: ExplainRecordAttribution['outcome'];
Expand Down
5 changes: 5 additions & 0 deletions packages/plugins/plugin-security/src/security-plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2212,6 +2212,11 @@ export class SecurityPlugin implements Plugin {
...(sharing && typeof sharing.canEdit === 'function'
? { canEditRecord: (o: string, rid: string, c: any) => sharing.canEdit(o, rid, c) }
: {}),
// [ADR-0111 D3] The narrower delete gate — an edit share opens update
// but not delete, so a delete explanation must consult this.
...(sharing && typeof sharing.canDelete === 'function'
? { canDeleteRecord: (o: string, rid: string, c: any) => sharing.canDelete(o, rid, c) }
: {}),
},
{ object, operation, context: targetContext, recordId },
);
Expand Down
53 changes: 36 additions & 17 deletions packages/plugins/plugin-sharing/src/sharing-plugin.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -421,7 +421,7 @@ export class SharingServicePlugin implements Plugin {
if (this.options.enforce === false) {
ctx.logger.info('SharingServicePlugin: enforcement disabled (enforce=false) — share-link service still registered');
} else {
const mw = buildSharingMiddleware(this.service);
const mw = buildSharingMiddleware(this.service, ctx.logger as any);
if (typeof engine.registerMiddleware === 'function') {
engine.registerMiddleware(mw, { object: '*' });
ctx.logger.info('SharingServicePlugin: enforcement middleware installed');
Expand DownExpand Up@@ -586,9 +586,13 @@ export class SharingServicePlugin implements Plugin {
/**
* Build the engine middleware that injects read filters and gates
* write operations. Exported so it can be unit-tested without booting
* a kernel.
* a kernel. `log` is optional — the [ADR-0111 D10] delete-denial breadcrumb
* is best-effort and absent in unit tests.
*/
export function buildSharingMiddleware(service: SharingService): EngineMiddleware {
export function buildSharingMiddleware(
service: SharingService,
log?: { warn?: (msg: string, meta?: any) => void },
): EngineMiddleware {
return async function sharingMiddleware(ctx: OperationContext, next: () => Promise<void>) {
const op = ctx.operation;
const exec = ctx.context as any;
Expand DownExpand Up@@ -644,24 +648,40 @@ export function buildSharingMiddleware(service: SharingService): EngineMiddlewar
return next();
}

// WRITES — gate on canEdit for update / delete.
// WRITES — gate on the per-VERB check. [ADR-0111 D3] update and delete no
// longer share a gate: `canEdit` accepts an edit-level share, `canDelete`
// does not (a share widens which rows a principal reaches, never which
// verbs). The middleware picks the gate by `op`.
if (op === 'update' || op === 'delete') {
const verb: 'update' | 'delete' = op;
const gate = (o: string, id: string, c: any) =>
verb === 'delete' ? service.canDelete(o, id, c) : service.canEdit(o, id, c);
const data: any = ctx.data;
const options: any = ctx.options;
const id = inferTargetId(data, options);
if (id != null) {
let ok = await service.canEdit(ctx.object, String(id), exec ?? {});
// [ADR-0090 D10] The delegator must ALSO be able to edit the row — an
// on-behalf-of write may only touch rows the delegator could touch.
let ok = await gate(ctx.object, String(id), exec ?? {});
// [ADR-0090 D10] The delegator must ALSO be able to perform the write —
// an on-behalf-of write may only touch rows the delegator could touch.
if (ok && exec?.onBehalfOf?.userId) {
ok = await service.canEdit(ctx.object, String(id), {
ok = await gate(ctx.object, String(id), {
...exec,
userId: exec.onBehalfOf.userId,
onBehalfOf: undefined,
__writeScope: exec.__delegatorWriteScope,
});
}
if (!ok) {
// [ADR-0111 D10] A fail-closed delete denial gets a specific,
// greppable reason so the "edit-share does not grant delete"
// tightening is diagnosable rather than a mystery 403.
if (verb === 'delete') {
log?.warn?.(
`[sharing] delete denied on ${ctx.object} ${id}: an edit-level share does not grant delete; ` +
`delete requires ownership, write depth, or Modify All Data (ADR-0111 D3)`,
{ object: ctx.object, recordId: String(id), userId: exec?.userId },
);
}
const err: any = new Error(
`FORBIDDEN: insufficient privileges to ${op} ${ctx.object} ${id}`,
);
Expand All@@ -672,21 +692,20 @@ export function buildSharingMiddleware(service: SharingService): EngineMiddlewar
return next();
}

// Bulk (multi) write — no single id to canEdit-gate (#2982). AND the
// editable-rows filter into the AST so the update/delete only touches
// rows the caller may edit, exactly as the read path scopes finds. The
// engine honours ast.where operation-agnostically (same seam the RLS
// write filter uses). Without this, a `multi:true` write on an
// owner-scoped object would hit every matching row, including peers'.
let writeFilter = await service.buildWriteFilter(ctx.object, exec ?? {});
// [ADR-0090 D10] Intersect the delegator's editable set for on-behalf-of.
// Bulk (multi) write — no single id to gate (#2982). AND the writable-rows
// filter into the AST so the update/delete only touches rows the caller
// may write, exactly as the read path scopes finds. The verb is threaded
// through so a bulk DELETE scopes to owned rows alone (no share widening),
// while a bulk UPDATE keeps the edit-share widening (ADR-0111 D3).
let writeFilter = await service.buildWriteFilter(ctx.object, exec ?? {}, verb);
// [ADR-0090 D10] Intersect the delegator's writable set for on-behalf-of.
if (exec?.onBehalfOf?.userId) {
const delFilter = await service.buildWriteFilter(ctx.object, {
...exec,
userId: exec.onBehalfOf.userId,
onBehalfOf: undefined,
__writeScope: exec.__delegatorWriteScope,
});
}, verb);
writeFilter = composeAnd(writeFilter, delFilter);
}
if (writeFilter) {
Expand Down
Loading
Loading