Skip to content

Commit e5e8b10

Browse files
authored
feat(sharing): a record's share-manager may revoke any share-link on that record (ADR-0111 D8, framework) (#3986)
ShareLinkService.revokeLink now admits a record share-manager (owner / modifyAllRecords via the late-bound canManageShares probe), not only the link's creator — fails closed when the probe is absent or throws. Mint authority (publicSharing opt-in + record visibility) documented as the D8 decision it already enforced.
1 parent 195ad76 commit e5e8b10

6 files changed

Lines changed: 124 additions & 7 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@objectstack/plugin-sharing": minor
3+
---
4+
5+
feat(sharing): a record's share-manager may revoke any share-link on that record (ADR-0111 D8)
6+
7+
`ShareLinkService.revokeLink` was creator-or-system only, so a record's owner or
8+
a Modify-All admin could not kill a link someone else minted on their record —
9+
their record's exposure, but not their link to revoke. Revoke authority now
10+
also admits a record **share-manager**, probed via the sharing service's
11+
late-bound `canManageShares` (owner / `modifyAllRecords`). The probe fails
12+
closed: a deployment without it (or a throwing probe) keeps the pre-D8
13+
creator-only behaviour. Mint authority is unchanged and now documented as the
14+
D8 decision it always enforced — the object's `publicSharing` opt-in AND the
15+
caller's visibility of the record.

content/docs/protocol/objectql/security.mdx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,14 @@ publicSharing:
324324
maxExpiryDays: 30
325325
```
326326

327+
**Who may mint and revoke** (ADR-0111 D8). Minting a link requires the object's
328+
`publicSharing` opt-in **and** that the caller can see the record — an opted-in
329+
object deliberately delegates re-share power to anyone who can view the row.
330+
Revoking a link is allowed for the link's **creator**, a **record share-manager**
331+
(the record's owner or a `modifyAllRecords` admin — the same authority as
332+
`canManageShares`), or system context: a link someone else minted on your record
333+
is your record's exposure to kill, not only its creator's.
334+
327335
---
328336

329337
## 6. Field-Level Encryption

docs/adr/0111-record-share-management-authority-and-verb-boundary.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# ADR-0111: Record-share management authority and the verb boundary — sharing needs "who may manage a share" and "which verbs a level grants"
22

3-
**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.
3+
**Status**: Accepted (2026-07-30) — **P0 + P1 + D8 (framework) 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. D8 (share-link re-share): `ShareLinkService.revokeLink` now admits a record share-manager via the late-bound `canManageShares` probe, and the mint-authority ruling (publicSharing opt-in + visibility) is enforced as before — verified in `share-link-service.test.ts`. **Still open**: the cloud-side D8 wiring (`HttpDispatcher.handleShareLinks` + a `.objectstack-sha` bump, in `objectstack-ai/cloud`) and the DEPTH management extension (D1 D-future) the follow-ups this ADR's rollout section names.
44
**Deciders**: ObjectStack Protocol Architects
55
**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)
66
**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`)

packages/plugins/plugin-sharing/src/share-link-service.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,54 @@ describe('ShareLinkService', () => {
189189
expect(engine._tables.sys_share_link[0].revoked_at).not.toBeNull();
190190
});
191191

192+
// [ADR-0111 D8] A record's share-manager (owner / Modify All) may revoke a
193+
// link someone ELSE minted on their record — the manage probe is consulted
194+
// when the caller is neither the creator nor system.
195+
it('a record share-manager may revoke a link another user minted', async () => {
196+
const seen: Array<[string, string, string]> = [];
197+
const svc = new ShareLinkService({
198+
engine: engine as any,
199+
canManageShares: async (object, recordId, ctx: any) => {
200+
seen.push([object, recordId, ctx.userId]);
201+
return ctx.userId === 'manager'; // manager can manage c1; nobody else
202+
},
203+
});
204+
const link = await svc.createLink(
205+
{ object: 'ai_conversations', recordId: 'c1', audience: 'link_only', permission: 'view' },
206+
{ userId: 'creator' },
207+
);
208+
// A bystander who cannot manage the record is still denied.
209+
await expect(svc.revokeLink(link.id, { userId: 'bystander' })).rejects.toMatchObject({ status: 403 });
210+
expect(engine._tables.sys_share_link[0].revoked_at).toBeNull();
211+
// The record's manager can, though they did not create the link.
212+
await expect(svc.revokeLink(link.id, { userId: 'manager' })).resolves.toBeUndefined();
213+
expect(engine._tables.sys_share_link[0].revoked_at).not.toBeNull();
214+
// The probe was asked about the LINK's (object, record), not anything else.
215+
expect(seen).toContainEqual(['ai_conversations', 'c1', 'manager']);
216+
});
217+
218+
it('without a manage probe (pre-D8 / probe-less deployment), only creator and system revoke', async () => {
219+
// Default service has no canManageShares — a non-creator is denied even
220+
// if they would be a manager, because the deployment cannot ask.
221+
const link = await service.createLink(
222+
{ object: 'ai_conversations', recordId: 'c1', audience: 'link_only', permission: 'view' },
223+
{ userId: 'creator' },
224+
);
225+
await expect(service.revokeLink(link.id, { userId: 'someone_else' })).rejects.toMatchObject({ status: 403 });
226+
});
227+
228+
it('a throwing manage probe fails CLOSED (denies a non-creator)', async () => {
229+
const svc = new ShareLinkService({
230+
engine: engine as any,
231+
canManageShares: async () => { throw new Error('probe down'); },
232+
});
233+
const link = await svc.createLink(
234+
{ object: 'ai_conversations', recordId: 'c1', audience: 'link_only', permission: 'view' },
235+
{ userId: 'creator' },
236+
);
237+
await expect(svc.revokeLink(link.id, { userId: 'bystander' })).rejects.toMatchObject({ status: 403 });
238+
});
239+
192240
it('an HTTP caller cannot mint a link for a record it cannot see (403, not 404)', async () => {
193241
// The record-access re-read runs under the caller context; a record the
194242
// caller cannot see (here: does not exist) yields a fail-closed 403 for an

packages/plugins/plugin-sharing/src/share-link-service.ts

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,18 @@ export interface ShareLinkServiceOptions {
170170
* without `publicSharing.enabled=true` are rejected with 422.
171171
*/
172172
permissive?: boolean;
173+
/**
174+
* [ADR-0111 D8] Late-bound record-share management probe (the sharing
175+
* service's `canManageShares`). Lets a record's OWNER / Modify-All admin
176+
* revoke a link someone else minted on their record — not just the link's
177+
* creator. Absent → only the creator (and system) may revoke, the pre-D8
178+
* behaviour, so a deployment without the sharing service degrades safely.
179+
*/
180+
canManageShares?: (
181+
object: string,
182+
recordId: string,
183+
context: ShareLinkExecutionContext,
184+
) => Promise<boolean>;
173185
}
174186

175187
/**
@@ -186,12 +198,18 @@ export class ShareLinkService implements IShareLinkService {
186198
private readonly permissive: boolean;
187199
private readonly hashPassword: (plain: string) => Promise<string>;
188200
private readonly verifyPassword: (plain: string, hash: string) => Promise<boolean>;
201+
private readonly canManageShares?: (
202+
object: string,
203+
recordId: string,
204+
context: ShareLinkExecutionContext,
205+
) => Promise<boolean>;
189206

190207
constructor(opts: ShareLinkServiceOptions) {
191208
this.engine = opts.engine;
192209
this.permissive = opts.permissive ?? false;
193210
this.hashPassword = opts.hashPassword ?? defaultHashPassword;
194211
this.verifyPassword = opts.verifyPassword ?? defaultVerifyPassword;
212+
this.canManageShares = opts.canManageShares;
195213
}
196214

197215
async createLink(
@@ -204,6 +222,11 @@ export class ShareLinkService implements IShareLinkService {
204222
const schema = this.engine.getSchema?.(input.object);
205223
const policy = getPolicy(schema);
206224

225+
// [ADR-0111 D8] Mint authority = the object's `publicSharing` opt-in (this
226+
// check) AND the caller's visibility of the record (the RLS-scoped read
227+
// below). An object that opts into publicSharing deliberately delegates
228+
// re-share power to anyone who can SEE the record — a stated decision, not
229+
// an accident. Objects that do not opt in cannot be link-shared at all.
207230
if (!policy.enabled && !this.permissive && !context.isSystem) {
208231
throw makeError(
209232
422,
@@ -289,16 +312,29 @@ export class ShareLinkService implements IShareLinkService {
289312
const filter = idOrToken.startsWith('shl_') ? { id: idOrToken } : { token: idOrToken };
290313
const rows = await this.engine.find('sys_share_link', {
291314
where: filter,
292-
fields: ['id', 'revoked_at', 'created_by'],
315+
// object_name / record_id are needed for the [ADR-0111 D8] record-manager
316+
// revoke path below.
317+
fields: ['id', 'revoked_at', 'created_by', 'object_name', 'record_id'],
293318
limit: 1,
294319
context: SYSTEM_CTX,
295320
} as any);
296321
const row = Array.isArray(rows) ? (rows[0] as any) : undefined;
297322
if (!row) return; // No-op when missing
298-
// [Finding-2] Only the link's creator may revoke it (internal/system callers
299-
// bypass). Previously the caller context was ignored, so any client could
300-
// revoke any user's link by id/token (sharing DoS).
301-
if (!context.isSystem && row.created_by !== context.userId) {
323+
324+
// Who may revoke this link:
325+
// - system / internal callers (bypass),
326+
// - the link's CREATOR ([Finding-2]: the caller context used to be
327+
// ignored, so any client could revoke any user's link — a sharing DoS),
328+
// - [ADR-0111 D8] a record SHARE-MANAGER (the record's owner or a
329+
// Modify-All admin): a link someone else minted on your record is your
330+
// record's exposure to kill, not only its creator's. Probed via the
331+
// late-bound sharing service; absent → creator-only (pre-D8 behaviour).
332+
let permitted = context.isSystem === true || row.created_by === context.userId;
333+
if (!permitted && this.canManageShares && row.object_name && row.record_id) {
334+
permitted = await this.canManageShares(String(row.object_name), String(row.record_id), context)
335+
.catch(() => false);
336+
}
337+
if (!permitted) {
302338
throw makeError(403, 'FORBIDDEN', 'Not permitted to revoke this share link');
303339
}
304340
if (row.revoked_at) return; // Already revoked

packages/plugins/plugin-sharing/src/sharing-plugin.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,17 @@ export class SharingServicePlugin implements Plugin {
489489
// access can set `registerShareLinkRoutes: false` and call the
490490
// service via `ctx.getService('shareLinks')`.
491491
try {
492-
this.linkService = new ShareLinkService({ engine: engine as SharingEngine });
492+
this.linkService = new ShareLinkService({
493+
engine: engine as SharingEngine,
494+
// [ADR-0111 D8] Let a record's share-manager (owner / Modify All)
495+
// revoke a link someone else minted on their record. `this.service`
496+
// is always constructed above — even under `enforce: false` (the
497+
// multi-tenant share-link-only config), where only the RLS middleware
498+
// is skipped — so the probe is available in every posture.
499+
canManageShares: this.service
500+
? (o, r, c) => this.service!.canManageShares(o, r, c as any)
501+
: undefined,
502+
});
493503
ctx.registerService('shareLinks', this.linkService);
494504

495505
if (this.options.registerShareLinkRoutes !== false) {

0 commit comments

Comments
 (0)