Skip to content

Commit db3fee3

Browse files
fix(plugin-sharing): a plain member's own share-link list is self-scoped (ADR-0111) (#21403)
Fixes #21328 Clause-②: no ## What was wrong `GET /api/v1/share-links` refused every plain member: a caller holding `member_default` and no `sys_share_link` grant was refused with no filter, with an object filter, and with any record. The console's Share dialog loads this list when it opens, so it failed for that whole class of user on every record. Both share-link doors already force `createdBy` to the caller, because ADR-0111's surface table rules the list self-scoped. But `ShareLinkService.listLinks` read `sys_share_link` under the caller's context, and that read needs an object-level grant the member baseline does not carry. An admin's list answered. Measured through this PR's route pin (showcase boot via `@objectstack/verify`). At the merge base `ee75aae1a`, and again with the elevation ablated at `8682b7b24`, the plain member's six list cases answered **500** with code `PERMISSION_DENIED`. The admin's list answered 200 with exactly the admin's own link. The Acceptance notes explain why this composition answers 500 where the card's hosted composition answered 403. ## The change (the triage direction quoted on the card) - **One creator rule.** `isLinkCreator(row, context)` in `share-link-service.ts` holds when the caller has a non-empty user identity and the row's `created_by` equals it. `revokeLink` and `listLinks` both read this rule, and there is no second creator test. - **`listLinks`.** The read runs under the system context only when the creator filter passes that rule, which means the filter names the caller. In that case the `created_by` constraint is the caller's identity, written server-side, and each returned row must pass `isLinkCreator` before it leaves. Every other shape keeps the caller's context: no creator filter, another user as creator, an empty or absent identity, and an admin listing someone else's links. A system caller keeps its bypass. `listLinks` keeps its single `isSystem` read, so the system-context census stays at 114. - **Projection unchanged.** The engine strips both `internal` columns under any context. The token comes back through the existing privileged accessor, and the password hash never does (route pin `[secret]`). - ⛔ **No permission-set edit.** The member still holds no `sys_share_link` grant (route pins `[persona]` and `[no-grant]`). ### What this deliberately changes beyond the list - **`revokeLink`'s creator check now refuses a caller with no user identity.** Before, `undefined === undefined` let an identity-less, non-system caller revoke a link whose `created_by` the driver omitted. Neither HTTP door reaches that case, because both answer 401 first. The new behaviour is pinned fail-closed. The creator, non-creator and ADR-0111 D8 record-manager paths are unchanged, and their existing pins pass untouched. - **`share-link-enforcement-context.test.ts`.** The pin `listLinks reads under the same whole envelope` read the envelope off the `sys_share_link` read. The route's list is always the caller's own, and that list is now the system read, so the pin now observes the envelope at the `listLinks` call. It checks that every key the resolver produced arrives, using the same comparison as the `createLink` pin, and asserts that the read is the self-scoped one. The property the file pins is unchanged: the route hands enforcement the whole envelope (ruling A, commit 8e13ca8). - **`@objectstack/spec`.** The `IShareLinkService.listLinks` doc comment said every listing is read under `context`. It now describes the self-scoped own list. This is a doc comment only. ## Tests (head `8682b7b24`) - **Route pin, real composition.** `packages/qa/dogfood/test/share-links-self-list.dogfood.test.ts` has 11 cases, and all 11 pass. A plain member lists exactly their own links, both with no filter and with the Share dialog's object and record filter. Member B's links and the admin's sit on the same record and stay absent. A `createdBy` query naming B is ignored. A plain member with no links gets `[]`. The row carries its token and no `password_hash`. The admin's list is unchanged, and an anonymous caller gets 401 `UNAUTHENTICATED`. - **Service pins.** `share-link-service.test.ts` and `share-link-enforcement-context.test.ts` pass 52 of 52. The service pins cover: - a member's own list; - the record filter; - a foreign creator and an unfiltered list, both refused under the caller's context with `PERMISSION_DENIED` / 403; - no identity (absent, and empty), which never takes the elevated path; - the admin path; - the system bypass; - a dropped `created_by` predicate, which still leaks no foreign row; - revoke and list reading one rule. - **Package runs.** `pnpm --filter @objectstack/plugin-sharing test` passes 928 tests in 38 files, and its `typecheck` is OK. `@objectstack/spec` passes `typecheck`, and its `test` passes 17529 tests in 598 files. `@objectstack/dogfood` passes `typecheck`, and its tsc program includes the new test. - **Ablations, service level.** Each leg ran from the committed head through `scripts/ablation-replace.mjs`, and each restore was proven to leave the blob equal to HEAD with `git diff HEAD` empty. | Mutation | Red | Green | |:--|:--|:--| | Identity check removed from `isLinkCreator` | 3: no-identity list, no-identity revoke, system bypass | 49 | | Server-side constraint removed (identity `where` and creator post-filter) | 6, including other creators' links on the same record | 46 | | Identity `where` only removed | 1 | 51 | | Creator post-filter only removed | 1 (the dropped-predicate pin) | 51 | | Elevation removed | 5: the own-list pins and the envelope-call pin | 47 | - **Ablation, route level.** The elevation was removed and plugin-sharing rebuilt. `ablation-dist-preflight` found the marker in 2 dist files. Result: 6 red (the own, empty, secret and foreign cases, at 500 `PERMISSION_DENIED`) and 5 green (persona, no-grant, fixture, admin, anonymous, which the test header says sit still). For the restore leg, plugin-sharing was rebuilt, the marker was absent from dist, and all 11 passed. The first route mutation (a `false &&` guard) was constant-folded out of dist. The preflight caught that before any test ran, so it measured nothing, and the leg was redone with a marker the bundler keeps. - **Gates.** `dispatch-gates --commands` at this head derives 89 commands, and all exit 0. `check:dual-build-cjs-loads` first exited 3 (`PREREQUISITE NOT MET`, because unrelated packages had not been built). Those packages were built from the turbo cache, and the rerun exited 0. The `--ran` reconciliation reports 89 derived, 89 run and 0 NOT MEASURED, a figure derived from the recorded exit codes. - **Lint (narrowed).** eslint `--no-inline-config` over the 5 changed TS files reports 0 errors and 0 warnings in all 5 files, and the config resolves for each. The narrowing is safe because the config enables no type-aware rules, and its local plugins read only their baselines. So this diff cannot move the verdict on any untouched file. ## Docs `content/docs/**` (outside `releases/` and `references/`) and `skills/**` hold no sentence this change makes false. It makes true ADR-0111's surface-table entry for share links, "list is self-scoped", for every signed-in caller. ## Acceptance notes - **Which door served.** In this composition, plugin-sharing's `registerShareLinkRoutes` serves `/api/v1/share-links`, not the runtime `/share-links` domain. The error status shows it: that door answers 500 where the dispatcher's mapper answers 403. The runtime domain forwards `createdBy` as the caller's id and the full envelope to the same `listLinks`, so the service fix covers both doors. The runtime door has no separate route pin here. - **Finding, for the seat to file (a defect of a different class, not fixed here).** The plugin door's catch maps the security middleware's `PERMISSION_DENIED` refusal (which carries `statusCode` 403 and no `status`) to HTTP 500. It is still reachable after this PR: `POST /api/v1/share-links` by a plain member, on an object they cannot read, answered 500 with code `PERMISSION_DENIED` on the showcase boot. - **The own list is not organization-walled.** The self-scoped read runs under the system context, so it sits outside the tenant wall, as `revokeLink`'s creator read already does. Share-link rows are inserted under the system context with `organization_id` null (measured on the showcase boot). A caller's own list therefore spans organizations in the environment's database, the same reach `revokeLink` already gives the creator. This follows the ruled direction. It is an observation only, with no wrong answer measured. - **No ADR anchor added.** There is no `scripts/adr-anchors` entry for `share-link-service.ts`. The ADR-0111 id stays in the code. An anchor would be outside the claimed surface, and no one is set to carry it. - **Out of scope.** Who may mint a link (#21329) is not addressed here. - **Base not merged.** `origin/main` has moved since the merge base. None of the upstream changes touch this PR's six files. --- _Generated by [Claude Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 45efcfa commit db3fee3

6 files changed

Lines changed: 512 additions & 12 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@objectstack/plugin-sharing": patch
3+
"@objectstack/spec": patch
4+
---
5+
6+
A plain member's share-link list now answers: `GET /api/v1/share-links` is self-scoped for every signed-in caller, as ADR-0111 rules it
7+
8+
Clause-②: no
9+
10+
`ShareLinkService.listLinks` read `sys_share_link` under the caller's context. Both share-link doors force the list's `createdBy` to the caller, but the read still needed an object-level grant on `sys_share_link`, and the platform's member baseline does not grant one. So every plain member's list was refused, with or without an object filter, and the Share dialog, which loads this list when it opens, showed an error for them on every record. An admin's list answered.
11+
12+
- The caller's own list is now read under the system context. This happens only when the caller has a non-empty user identity and the creator filter equals it. The read is constrained server-side to that identity, and each row it returns must pass the creator rule before it leaves.
13+
- One creator rule now serves both `listLinks` and `revokeLink`. It never matches a caller with no user identity. Neither HTTP door reaches that case, because both answer 401 first, so for an internal caller with no user identity, `revokeLink` now refuses a link whose `created_by` is absent or empty instead of treating it as theirs.
14+
- Every other list shape keeps the caller's context, as before: no creator filter, another user as creator, no user identity, or an admin listing someone else's links. A system caller keeps its bypass.
15+
- The rows carry the same columns as before. The token comes back so the console can build the link URL, and the password hash never does.
16+
- `@objectstack/spec`: the `IShareLinkService.listLinks` doc comment now describes the self-scoped own list. It previously said every listing is read under `context`. This is a doc comment only, with no type or export change.
17+
- ⛔ No permission set changes, and no new grant on `sys_share_link`.

‎packages/plugins/plugin-sharing/src/share-link-enforcement-context.test.ts‎

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,10 @@ async function bootPlugin(opts: { session: () => any; tables: Record<string, any
171171
await plugin.start(ctx);
172172
for (const handler of hooks['kernel:ready'] ?? []) await handler();
173173

174-
return { engine, http };
174+
/** A service the plugin registered — the very instance its routes call. */
175+
const registered = (name: string): any =>
176+
(ctx.registerService.mock.calls as Array<[string, unknown]>).find(([n]) => n === name)?.[1];
177+
return { engine, http, registered };
175178
}
176179

177180
interface Captured { status: number; body: any }
@@ -268,17 +271,41 @@ describe('[#6206] share-link routes hand ENFORCEMENT the whole authz envelope',
268271
expect(enforcementCtx.isSystem).toBe(false);
269272
});
270273

271-
it('listLinks reads under the same whole envelope', async () => {
274+
// [#21328] Observed at the CALL, not at the read. This pin used to read the
275+
// envelope off the `sys_share_link` read, because that read ran under it.
276+
// The route's list is always the caller's own (it forces `createdBy`), and
277+
// ADR-0111 rules that list self-scoped, so `listLinks` now reads it under the
278+
// system context, constrained to the caller — the read no longer carries the
279+
// envelope, by design. What this file pins is unchanged: the route hands
280+
// `listLinks` every key the resolver produced, so a shape that does read
281+
// under the caller's context (another creator, no creator) gets all of it.
282+
it('listLinks is handed the same whole envelope', async () => {
272283
const tables = fixtureTables();
273-
const { engine, http } = await bootPlugin({ session: signedIn, tables });
284+
const { engine, http, registered } = await bootPlugin({ session: signedIn, tables });
285+
const listLinks = vi.spyOn(registered('shareLinks'), 'listLinks');
274286

275287
const res = await drive(http, `GET ${BASE}`, { query: {} });
276288
expect(res.status).toBe(200);
277289

290+
expect(listLinks).toHaveBeenCalledTimes(1);
291+
const [filter, handed] = listLinks.mock.calls[0] as [any, any];
292+
expect(filter.createdBy, 'the route forces the caller\'s own id').toBe(USER);
293+
294+
const seam: any = await bootRequestContext({
295+
userId: USER,
296+
email: EMAIL,
297+
activeOrganizationId: ORG,
298+
});
299+
const dropped = Object.keys(seam).filter((k) => !(k in handed));
300+
expect(dropped, 'keys the route dropped on the way into listLinks').toEqual([]);
301+
expect(handed.accessible_org_ids).toEqual([ORG]);
302+
expect(handed.posture).toBe('MEMBER');
303+
expect(handed.isSystem).toBe(false);
304+
305+
// …and the read itself is the self-scoped one.
278306
const listRead = engine.finds.filter((f) => f.object === 'sys_share_link').pop();
279307
expect(listRead).toBeDefined();
280-
expect(listRead!.context.accessible_org_ids).toEqual([ORG]);
281-
expect(listRead!.context.posture).toBe('MEMBER');
308+
expect(listRead!.context.isSystem).toBe(true);
282309
});
283310

284311
it('an unresolvable request is still anonymous → 401, and nothing is enforced', async () => {

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

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,170 @@ describe('ShareLinkService', () => {
451451
),
452452
).rejects.toMatchObject({ status: 404 });
453453
});
454+
455+
// [#21328] The creator half of revoke authority is the one creator rule
456+
// `listLinks` also reads, and that rule names no creator for a context
457+
// without a user identity. Compared bare, `undefined === undefined` let an
458+
// identity-less (non-system) caller revoke a link whose `created_by` the
459+
// driver omitted. Neither HTTP door reaches this — both answer 401 first —
460+
// so it is the internal-caller edge of the same rule, pinned fail-closed.
461+
it('a caller with no user identity is the creator of nothing', async () => {
462+
engine._tables.sys_share_link = [
463+
{ id: 'shl_omitted', token: 'tok_omitted_0001', object_name: 'ai_conversations', record_id: 'c1', revoked_at: null },
464+
{ id: 'shl_empty', token: 'tok_empty_000001', object_name: 'ai_conversations', record_id: 'c1', revoked_at: null, created_by: '' },
465+
];
466+
await expect(service.revokeLink('shl_omitted', {})).rejects.toMatchObject({ status: 403, code: 'FORBIDDEN' });
467+
await expect(service.revokeLink('shl_empty', { userId: '' })).rejects.toMatchObject({ status: 403, code: 'FORBIDDEN' });
468+
expect(engine._tables.sys_share_link.map((r) => r.revoked_at)).toEqual([null, null]);
469+
});
470+
});
471+
472+
// ── [#21328] the caller's OWN list is self-scoped (ADR-0111 surface table) ──
473+
//
474+
// ADR-0111 rules the list "self-scoped", and both doors force `createdBy` to
475+
// the caller — but the read ran under the caller's context, so it demanded an
476+
// object-level grant on `sys_share_link` the member baseline does not carry,
477+
// and every plain member's list was refused. The fix reads the caller's OWN
478+
// list under the system context; every other shape keeps the caller's
479+
// context. The double below overrides only `find`, to model the one gate
480+
// that matters: a non-system context whose sets grant nothing on
481+
// `sys_share_link` is refused the way the security middleware refuses it
482+
// (`PERMISSION_DENIED`, 403).
483+
describe('[#21328] listLinks: the caller\'s own list is self-scoped', () => {
484+
const CONVO = { object: 'ai_conversations', recordId: 'c1', audience: 'link_only', permission: 'view' } as const;
485+
const ON_C1 = { object: 'ai_conversations', recordId: 'c1' } as const;
486+
487+
/** Every `find` the service issues, with the context it ran under. */
488+
let seen: Array<{ object: string; where: any; context: any }>;
489+
let svc: ShareLinkService;
490+
let mine: string[];
491+
let theirs: string[];
492+
let admins: string[];
493+
494+
/** `who` hold an object-level read grant on `sys_share_link`; nobody else does. */
495+
const grantedTo = (who: string[]) => ({
496+
...engine,
497+
async find(object: string, options?: any) {
498+
const ctx = options?.context ?? {};
499+
seen.push({ object, where: options?.where, context: ctx });
500+
if (object === 'sys_share_link' && ctx.isSystem !== true && !who.includes(ctx.userId)) {
501+
throw Object.assign(
502+
new Error('You do not have permission to perform this action.'),
503+
{ code: 'PERMISSION_DENIED', statusCode: 403 },
504+
);
505+
}
506+
return engine.find(object, options);
507+
},
508+
});
509+
510+
const listed = (links: Array<{ id: string }>) => links.map((l) => l.id).sort();
511+
const shareLinkReads = () => seen.filter((s) => s.object === 'sys_share_link');
512+
513+
beforeEach(async () => {
514+
// Three creators' links on the SAME record, so a record filter cannot
515+
// separate them — only the creator rule can.
516+
mine = [
517+
(await service.createLink(CONVO, { userId: 'alice' })).id,
518+
(await service.createLink({ ...CONVO, label: 'second' }, { userId: 'alice' })).id,
519+
].sort();
520+
theirs = [(await service.createLink(CONVO, { userId: 'bob' })).id];
521+
admins = [(await service.createLink(CONVO, { userId: 'admin' })).id];
522+
// Rows no caller's identity can match: one whose `created_by` the driver
523+
// omitted, one minted under an empty identity.
524+
engine._tables.sys_share_link.push(
525+
{ id: 'shl_omitted', token: 'tok_omitted_0001', object_name: 'ai_conversations', record_id: 'c1', revoked_at: null },
526+
{ id: 'shl_empty', token: 'tok_empty_000001', object_name: 'ai_conversations', record_id: 'c1', revoked_at: null, created_by: '' },
527+
);
528+
seen = [];
529+
svc = new ShareLinkService({ engine: grantedTo(['admin']) as any });
530+
});
531+
532+
it('a member with no sys_share_link grant lists exactly their own links', async () => {
533+
const links = await svc.listLinks({ createdBy: 'alice' }, { userId: 'alice' });
534+
expect(listed(links)).toEqual(mine);
535+
536+
const [read] = shareLinkReads();
537+
expect(read.context.isSystem, 'the own list is the one read that runs under the system context').toBe(true);
538+
expect(read.where.created_by, 'constrained server-side to the caller').toBe('alice');
539+
});
540+
541+
it('the system read trusts no query predicate alone: a dropped created_by constraint still leaks no foreign row', async () => {
542+
// A driver that silently drops a `where` key answers with the WIDER set —
543+
// under the system context, every creator's tokens. The rows that leave
544+
// must still pass the creator rule.
545+
const dropsCreator = {
546+
...engine,
547+
async find(object: string, options?: any) {
548+
const where = { ...(options?.where ?? {}) };
549+
delete where.created_by;
550+
return engine.find(object, { ...options, where });
551+
},
552+
};
553+
const leaky = new ShareLinkService({ engine: dropsCreator as any });
554+
expect(listed(await leaky.listLinks({ ...ON_C1, createdBy: 'alice' }, { userId: 'alice' }))).toEqual(mine);
555+
});
556+
557+
it('through the record filter, other creators\' links on the same record are absent', async () => {
558+
const links = await svc.listLinks({ ...ON_C1, createdBy: 'alice' }, { userId: 'alice' });
559+
expect(listed(links)).toEqual(mine);
560+
// Each listed row still carries its token — the console builds the URL from it.
561+
expect(links.every((l) => typeof l.token === 'string' && l.token.length > 0)).toBe(true);
562+
});
563+
564+
it('a foreign creator filter keeps the caller\'s context, and is refused', async () => {
565+
await expect(svc.listLinks({ ...ON_C1, createdBy: 'bob' }, { userId: 'alice' }))
566+
.rejects.toMatchObject({ code: 'PERMISSION_DENIED', statusCode: 403 });
567+
expect(shareLinkReads()).toHaveLength(1);
568+
expect(shareLinkReads()[0].context).toMatchObject({ userId: 'alice' });
569+
expect(shareLinkReads()[0].context.isSystem).toBeUndefined();
570+
});
571+
572+
it('an unfiltered list keeps the caller\'s context, and is refused', async () => {
573+
await expect(svc.listLinks({}, { userId: 'alice' }))
574+
.rejects.toMatchObject({ code: 'PERMISSION_DENIED', statusCode: 403 });
575+
expect(shareLinkReads()).toHaveLength(1);
576+
expect(shareLinkReads()[0].context.isSystem).toBeUndefined();
577+
});
578+
579+
it('a caller with no user identity never takes the elevated path', async () => {
580+
// Absent identity and absent creator filter: `undefined === undefined`.
581+
await expect(svc.listLinks({}, {}))
582+
.rejects.toMatchObject({ code: 'PERMISSION_DENIED', statusCode: 403 });
583+
await expect(svc.listLinks({ ...ON_C1 }, {}))
584+
.rejects.toMatchObject({ code: 'PERMISSION_DENIED', statusCode: 403 });
585+
// Empty identity and empty creator filter: `'' === ''`.
586+
await expect(svc.listLinks({ createdBy: '' }, { userId: '' }))
587+
.rejects.toMatchObject({ code: 'PERMISSION_DENIED', statusCode: 403 });
588+
expect(shareLinkReads()).toHaveLength(3);
589+
expect(shareLinkReads().every((r) => r.context.isSystem !== true)).toBe(true);
590+
});
591+
592+
it('the admin path is unchanged: a granted caller lists anyone\'s links under their own context', async () => {
593+
const all = await svc.listLinks({}, { userId: 'admin' });
594+
expect(listed(all)).toEqual([...mine, ...theirs, ...admins, 'shl_empty', 'shl_omitted'].sort());
595+
const bobs = await svc.listLinks({ createdBy: 'bob' }, { userId: 'admin' });
596+
expect(listed(bobs)).toEqual(theirs);
597+
expect(shareLinkReads().map((r) => r.context.isSystem)).toEqual([undefined, undefined]);
598+
// …and the admin's OWN list answers exactly their own links, as it did.
599+
expect(listed(await svc.listLinks({ ...ON_C1, createdBy: 'admin' }, { userId: 'admin' }))).toEqual(admins);
600+
});
601+
602+
it('a system caller keeps its bypass', async () => {
603+
const links = await svc.listLinks({ object: 'ai_conversations' }, { isSystem: true });
604+
expect(listed(links)).toEqual([...mine, ...theirs, ...admins, 'shl_empty', 'shl_omitted'].sort());
605+
expect(shareLinkReads()[0].context.isSystem).toBe(true);
606+
// A system caller asking for its own links gets the same rows either way.
607+
expect(listed(await svc.listLinks({ createdBy: 'alice' }, { isSystem: true, userId: 'alice' }))).toEqual(mine);
608+
});
609+
610+
it('revoke and list read one creator rule: what you may revoke as creator is what your list shows', async () => {
611+
const links = await svc.listLinks({ createdBy: 'alice' }, { userId: 'alice' });
612+
for (const link of links) await expect(svc.revokeLink(link.id, { userId: 'alice' })).resolves.toBeUndefined();
613+
await expect(svc.revokeLink(theirs[0], { userId: 'alice' })).rejects.toMatchObject({ status: 403, code: 'FORBIDDEN' });
614+
// Revoked links leave the default list and come back under includeRevoked.
615+
expect(await svc.listLinks({ createdBy: 'alice' }, { userId: 'alice' })).toEqual([]);
616+
expect(listed(await svc.listLinks({ createdBy: 'alice', includeRevoked: true }, { userId: 'alice' }))).toEqual(mine);
617+
});
454618
});
455619
});
456620

0 commit comments

Comments
 (0)