Skip to content

Commit 4beed26

Browse files
committed
fix(security): close reports IDOR, RAG retrieval fall-open, and bulk-write OWD gap (#2980, #2981, #2982)
Three confirmed-exploitable execution-surface authz holes from the ADR-0096 class sweep, each fixed at its enforcement point; no change for correctly- scoped callers. - #2980 reports: getReport/deleteReport/saveReport/listReports were caller- identity-blind (read/wrote sys_saved_report with a system context) → any authenticated user could read/delete/overwrite/enumerate any report. Now owner-scoped (system read of the protection-locked metadata object, authz by owner match); create/overwrite can't spoof ownership. Scheduled dispatch no longer runs isSystem (which emailed the target object's whole table past the owner's RLS): resolves the owner via a new resolveOwnerContext seam and fails closed when unresolved (ADR-0073 user-less identity consumer). - #2981 knowledge/RAG: applyPermissionFilter returned all hits on missing OR system context. Missing identity now fails closed (drops object-backed hits, keeps ACL-less file/http); only an explicit system context passes through. - #2982 sharing: bulk update({multi:true})/deleteMany had no single id to canEdit-gate, skipping owner scoping on private/public_read objects. New SharingService.buildWriteFilter (edit-set analogue of buildReadFilter) is AND-ed into the multi-write AST, incl. the on-behalf-of delegator intersection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0155bjzJZxZaZTKSMSvg6Aum
1 parent 1728a5a commit 4beed26

9 files changed

Lines changed: 473 additions & 28 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
'@objectstack/plugin-reports': patch
3+
'@objectstack/service-knowledge': patch
4+
'@objectstack/plugin-sharing': patch
5+
---
6+
7+
fix(security): close three execution-surface authz holes surfaced by the #2849 class sweep (#2980, #2981, #2982)
8+
9+
Three independent, confirmed-exploitable defects where an execution surface
10+
ignored the caller's identity or fell open on a missing one. Each is fixed at
11+
its own enforcement point; none change behaviour for correctly-scoped callers.
12+
13+
- **#2980 — reports IDOR + scheduled-report RLS bypass.** `ReportService`
14+
discarded the caller's context and read/wrote `sys_saved_report` with a system
15+
context, so any authenticated user could read, delete, or overwrite any saved
16+
report by id (cross-owner / cross-tenant), and `listReports` enumerated all
17+
owners. `getReport`/`deleteReport`/`saveReport`/`listReports` are now
18+
owner-scoped (system read of the protection-locked metadata object, but
19+
authorization enforced by owner match); create/overwrite can no longer spoof
20+
ownership. Scheduled dispatch no longer runs `isSystem` (which emailed the
21+
target object's entire table past the owner's RLS): it resolves the owner to a
22+
real RLS-bearing context via a new `resolveOwnerContext` seam and **fails
23+
closed** (skips + marks the schedule failed) when the owner can't be resolved,
24+
rather than running elevated. Wiring that resolver is the reports-surface
25+
consumer of ADR-0073's user-less identity resolution.
26+
27+
- **#2981 — knowledge/RAG retrieval fall-open.** `applyPermissionFilter` returned
28+
every hit when the context was missing *or* system. A missing identity is no
29+
longer treated as a grant: object-backed hits fail closed (dropped, keeping
30+
ACL-less file/http hits), and only an **explicit** system context passes
31+
through. Closes the agent path where an omitted `ToolExecutionContext.actor`
32+
yielded unfiltered semantic search over the whole corpus.
33+
34+
- **#2982 — bulk-write OWD gap.** `update({multi:true})` / `deleteMany` had no
35+
single id to `canEdit`-gate, so owner scoping was skipped on private (and
36+
public_read) objects. A new `SharingService.buildWriteFilter` (the edit-set
37+
analogue of `buildReadFilter`) is AND-ed into the write AST for multi writes,
38+
constraining them to rows the caller may edit — including the on-behalf-of
39+
delegator intersection.
40+
41+
Tracked as the motivating evidence of ADR-0096 (execution-surface identity
42+
admission); the mechanism that would prevent the class structurally is separate.

packages/plugins/plugin-reports/src/report-service.test.ts

Lines changed: 99 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ describe('ReportService', () => {
119119
email,
120120
clock: { now: () => now },
121121
maxRows: 5000,
122+
// Scheduled runs execute under the owner's resolved context (#2980).
123+
// The fake engine ignores context, so this just lets dispatch proceed
124+
// under a non-elevated identity instead of failing closed.
125+
resolveOwnerContext: async (ownerId: string) =>
126+
ownerId ? { userId: ownerId, tenantId: 't1', positions: [], permissions: [] } : null,
122127
});
123128
// seed the underlying object the report will query.
124129
engine._tables['lead'] = [
@@ -170,9 +175,9 @@ describe('ReportService', () => {
170175
// Regression: the query sorted with the non-canonical `direction: 'desc'`
171176
// key, which SortNode strips — so it sorted ascending (oldest first).
172177
engine._tables['sys_saved_report'] = [
173-
{ id: 'r_old', name: 'Old', object_name: 'lead', query_json: '{}', updated_at: '2026-01-01T00:00:00Z' },
174-
{ id: 'r_new', name: 'New', object_name: 'lead', query_json: '{}', updated_at: '2026-03-01T00:00:00Z' },
175-
{ id: 'r_mid', name: 'Mid', object_name: 'lead', query_json: '{}', updated_at: '2026-02-01T00:00:00Z' },
178+
{ id: 'r_old', name: 'Old', object_name: 'lead', query_json: '{}', owner_id: 'u1', updated_at: '2026-01-01T00:00:00Z' },
179+
{ id: 'r_new', name: 'New', object_name: 'lead', query_json: '{}', owner_id: 'u1', updated_at: '2026-03-01T00:00:00Z' },
180+
{ id: 'r_mid', name: 'Mid', object_name: 'lead', query_json: '{}', owner_id: 'u1', updated_at: '2026-02-01T00:00:00Z' },
176181
];
177182
const rows = await svc.listReports({ object: 'lead' }, CTX);
178183
expect(rows.map(r => r.id)).toEqual(['r_new', 'r_mid', 'r_old']);
@@ -368,7 +373,11 @@ describe('ReportService', () => {
368373
});
369374

370375
it('dispatchDue: still runs (no mail) when email service absent', async () => {
371-
const svcNoMail = new ReportService({ engine: engine as any, clock: { now: () => now } });
376+
const svcNoMail = new ReportService({
377+
engine: engine as any,
378+
clock: { now: () => now },
379+
resolveOwnerContext: async (ownerId: string) => ({ userId: ownerId, positions: [], permissions: [] }),
380+
});
372381
const r = await svcNoMail.saveReport({ name: 'A', object: 'lead', query: {} }, CTX);
373382
await svcNoMail.scheduleReport({ reportId: r.id, recipients: ['x@t'] }, CTX);
374383
engine._tables['sys_report_schedule'][0].next_run_at = new Date(now.getTime() - 1).toISOString();
@@ -377,4 +386,90 @@ describe('ReportService', () => {
377386
expect(result.fired).toBe(1);
378387
expect(engine._tables['sys_report_schedule'][0].last_status).toBe('ok');
379388
});
389+
390+
// ─── Authorization (#2980) ──────────────────────────────────────
391+
describe('access control', () => {
392+
const OTHER = { userId: 'u2', tenantId: 't1', positions: [], permissions: [] };
393+
394+
it('getReport: a non-owner cannot read another user\'s report (not-found)', async () => {
395+
const r = await svc.saveReport({ name: 'Mine', object: 'lead', query: {} }, CTX);
396+
expect(await svc.getReport(r.id, CTX)).not.toBeNull(); // owner sees it
397+
expect(await svc.getReport(r.id, OTHER)).toBeNull(); // stranger cannot
398+
});
399+
400+
it('deleteReport: a non-owner cannot delete another user\'s report', async () => {
401+
const r = await svc.saveReport({ name: 'Mine', object: 'lead', query: {} }, CTX);
402+
await expect(svc.deleteReport(r.id, OTHER)).rejects.toThrow(/REPORT_NOT_FOUND/);
403+
expect(engine._tables['sys_saved_report'].length).toBe(1); // still there
404+
await svc.deleteReport(r.id, CTX); // owner can
405+
expect(engine._tables['sys_saved_report'].length).toBe(0);
406+
});
407+
408+
it('saveReport: a non-owner cannot overwrite another user\'s report by id', async () => {
409+
const r = await svc.saveReport({ name: 'Mine', object: 'lead', query: {} }, CTX);
410+
await expect(
411+
svc.saveReport({ id: r.id, name: 'Hijacked', object: 'lead', query: {} }, OTHER),
412+
).rejects.toThrow(/REPORT_NOT_FOUND/);
413+
expect(engine._tables['sys_saved_report'][0].name).toBe('Mine');
414+
});
415+
416+
it('saveReport: a caller cannot assign ownership to someone else on create', async () => {
417+
const r = await svc.saveReport(
418+
{ name: 'X', object: 'lead', query: {}, ownerId: 'victim' } as any,
419+
CTX,
420+
);
421+
expect(r.owner_id).toBe('u1');
422+
});
423+
424+
it('listReports: only the caller\'s own reports are returned', async () => {
425+
await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX); // u1
426+
await svc.saveReport({ name: 'B', object: 'lead', query: {} }, OTHER); // u2
427+
const mine = await svc.listReports({}, CTX);
428+
expect(mine.map(r => r.name)).toEqual(['A']);
429+
const theirs = await svc.listReports({}, OTHER);
430+
expect(theirs.map(r => r.name)).toEqual(['B']);
431+
});
432+
433+
it('listReports: a caller-supplied ownerId cannot widen past the caller', async () => {
434+
await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX);
435+
await svc.saveReport({ name: 'B', object: 'lead', query: {} }, OTHER);
436+
expect(await svc.listReports({ ownerId: 'u2' }, CTX)).toEqual([]); // u1 asking for u2 ⇒ nothing
437+
});
438+
439+
it('system context sees all reports (scheduler / tooling path)', async () => {
440+
await svc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX);
441+
await svc.saveReport({ name: 'B', object: 'lead', query: {} }, OTHER);
442+
const all = await svc.listReports({}, { isSystem: true } as any);
443+
expect(all.length).toBe(2);
444+
});
445+
446+
it('dispatchDue: fails closed (no RLS bypass) when no owner resolver is configured', async () => {
447+
const noResolver = new ReportService({ engine: engine as any, email, clock: { now: () => now } });
448+
const r = await noResolver.saveReport({ name: 'A', object: 'lead', query: {} }, CTX);
449+
await noResolver.scheduleReport({ reportId: r.id, recipients: ['x@t'] }, CTX);
450+
engine._tables['sys_report_schedule'][0].next_run_at = new Date(now.getTime() - 1).toISOString();
451+
452+
const result = await noResolver.dispatchDue();
453+
expect(result.fired).toBe(0);
454+
expect(result.failed).toBe(1);
455+
expect(email._sent.length).toBe(0); // nothing emailed — no elevated run
456+
expect(engine._tables['sys_report_schedule'][0].last_status).toBe('failed');
457+
expect(engine._tables['sys_report_schedule'][0].last_error).toMatch(/RLS bypassed/);
458+
});
459+
460+
it('dispatchDue: runs under the owner context the resolver returns', async () => {
461+
const seen: Array<string | undefined> = [];
462+
const spySvc = new ReportService({
463+
engine: engine as any, email, clock: { now: () => now },
464+
resolveOwnerContext: async (ownerId) => { seen.push(ownerId); return { userId: ownerId, positions: [], permissions: [] }; },
465+
});
466+
const r = await spySvc.saveReport({ name: 'A', object: 'lead', query: {} }, CTX);
467+
await spySvc.scheduleReport({ reportId: r.id, recipients: ['x@t'], format: 'csv' }, CTX);
468+
engine._tables['sys_report_schedule'][0].next_run_at = new Date(now.getTime() - 1).toISOString();
469+
470+
const result = await spySvc.dispatchDue();
471+
expect(result.fired).toBe(1);
472+
expect(seen).toEqual(['u1']); // resolved the owner, not a system context
473+
});
474+
});
380475
});

0 commit comments

Comments
 (0)