fix(reports): owner-gate the saved-report schedule routes (#2980)

.changeset/report-schedule-owner-gate.md

@@ -0,0 +1,30 @@
+---
+"@objectstack/plugin-reports": patch
+"@objectstack/rest": patch
+---
+
+fix(reports): owner-gate the saved-report schedule routes (#2980)
+
+The report read/run/delete routes are owner-isolated (a caller may only touch a
+report they own, denied as `REPORT_NOT_FOUND` to avoid leaking that the id
+exists), but the two schedule routes bypassed that gate: `unscheduleReport` and
+`listSchedules` took the caller `context` as `_context` and never consulted it,
+querying under the system context (RLS-bypassing). Any authenticated caller
+could therefore delete another owner's report schedule — a cross-owner
+destructive write — or list another owner's schedules (leaking recipient
+addresses and cron), by supplying an id.
+
+Both now resolve the schedule's parent report and require the caller to own it,
+mirroring the sibling routes:
+
+- **`unscheduleReport`** loads the schedule, then its report, and deletes only
+  when `canAccessReport` holds; a cross-owner attempt throws `REPORT_NOT_FOUND`
+  (mapped to `404` by the REST layer, deny-as-404 anti-enumeration), while a
+  genuinely-absent schedule stays idempotent. `scheduleReport` (create) was
+  already gated via `getReport`, so only the delete/list doors were open.
+- **`listSchedules`** returns an empty list to any non-system caller who cannot
+  access the report it is scoped to — the same non-leaking posture as
+  `listReports`. The scheduler's system context still sees every schedule.
+
+No authoring-surface or metadata change; existing owner-path behavior is
+unchanged.

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

@@ -443,6 +443,34 @@ describe('ReportService', () => {
       expect(all.length).toBe(2);
     });
 
+    it('unscheduleReport: a non-owner cannot delete another user\'s schedule', async () => {
+      const r = await svc.saveReport({ name: 'Mine', object: 'lead', query: {} }, CTX);
+      const s = await svc.scheduleReport({ reportId: r.id, recipients: ['x@t'] }, CTX);
+      // stranger is denied as not-found and the schedule survives untouched
+      await expect(svc.unscheduleReport(s.id, OTHER)).rejects.toThrow(/REPORT_NOT_FOUND/);
+      expect(engine._tables['sys_report_schedule'].length).toBe(1);
+      // owner can
+      await svc.unscheduleReport(s.id, CTX);
+      expect(engine._tables['sys_report_schedule'].length).toBe(0);
+    });
+
+    it('unscheduleReport: an unknown schedule id is idempotent, not a leak', async () => {
+      await expect(svc.unscheduleReport('rsch_nope', OTHER)).resolves.toBeUndefined();
+    });
+
+    it('listSchedules: a non-owner cannot see another user\'s schedules', async () => {
+      const r = await svc.saveReport({ name: 'Mine', object: 'lead', query: {} }, CTX);
+      await svc.scheduleReport({ reportId: r.id, recipients: ['x@t'] }, CTX);
+      expect((await svc.listSchedules({ reportId: r.id }, OTHER)).length).toBe(0); // stranger sees nothing
+      expect((await svc.listSchedules({ reportId: r.id }, CTX)).length).toBe(1);   // owner sees it
+    });
+
+    it('listSchedules: system context (dispatcher) still sees schedules', async () => {
+      const r = await svc.saveReport({ name: 'Mine', object: 'lead', query: {} }, CTX);
+      await svc.scheduleReport({ reportId: r.id, recipients: ['x@t'] }, CTX);
+      expect((await svc.listSchedules({ reportId: r.id }, { isSystem: true } as any)).length).toBe(1);
+    });
+
     it('dispatchDue: fails closed (no RLS bypass) when no owner resolver is configured', async () => {
       const noResolver = new ReportService({ engine: engine as any, email, clock: { now: () => now } });
       const r = await noResolver.saveReport({ name: 'A', object: 'lead', query: {} }, CTX);

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

@@ -312,6 +312,14 @@ export class ReportService implements IReportService {
     return Array.isArray(rows) && rows[0] ? rows[0] : null;
   }
 
+  /** Raw metadata read of a report schedule by id (no authz — callers gate). */
+  private async loadScheduleRow(scheduleId: string): Promise<any | null> {
+    const rows = await this.engine.find('sys_report_schedule', {
+      where: { id: scheduleId }, limit: 1, context: SYSTEM_CTX,
+    });
+    return Array.isArray(rows) && rows[0] ? rows[0] : null;
+  }
+
   // ── Report CRUD ────────────────────────────────────────────────
 
   async saveReport(input: SaveReportInput, context: SharingExecutionContext): Promise<SavedReport> {
@@ -532,15 +540,34 @@ export class ReportService implements IReportService {
     return rowFromSchedule(row);
   }
 
-  async unscheduleReport(scheduleId: string, _context: SharingExecutionContext): Promise<void> {
+  async unscheduleReport(scheduleId: string, context: SharingExecutionContext): Promise<void> {
     if (!scheduleId) throw new Error('VALIDATION_FAILED: scheduleId is required');
+    const schedule = await this.loadScheduleRow(scheduleId);
+    if (!schedule) return; // idempotent — nothing to drop (mirrors deleteReport)
+    // A schedule is owned through its report (#2980): a caller may only delete
+    // the schedules of a report they own. Others get a not-found so the delete
+    // neither fires nor reveals the schedule's existence — deny-as-404, never a
+    // cross-owner 2xx.
+    const report = await this.loadReportRow(schedule.report_id);
+    if (!this.canAccessReport(report, context)) {
+      throw new Error(`REPORT_NOT_FOUND: ${scheduleId}`);
+    }
     await this.engine.delete('sys_report_schedule', { where: { id: scheduleId }, context: SYSTEM_CTX });
   }
 
   async listSchedules(
     filter: { reportId?: string } | undefined,
-    _context: SharingExecutionContext,
+    context: SharingExecutionContext,
   ): Promise<ReportSchedule[]> {
+    // Schedules are owned through their report (#2980): a non-system caller may
+    // only list the schedules of a report they can access. The route always
+    // supplies the parent report id; a caller who cannot see that report gets an
+    // empty list — never another owner's recipients/cron — the same non-leaking
+    // posture as listReports. System/tooling (the dispatcher) still sees all.
+    if (!context?.isSystem) {
+      if (!filter?.reportId) return [];
+      if (!(await this.getReport(filter.reportId, context))) return [];
+    }
     const f: any = {};
     if (filter?.reportId) f.report_id = filter.reportId;
     const rows = await this.engine.find('sys_report_schedule', {

packages/rest/src/rest-server.ts

@@ -8451,6 +8451,7 @@ export class RestServer {
                     await svc.unscheduleReport(req.params.scheduleId, context ?? {});
                     res.status(204).end();
                 } catch (error: any) {
+                    if (handleValidation(res, error)) return; // REPORT_NOT_FOUND → 404 (deny-as-404, anti-enumeration)
                     logError('[REST] Unschedule report error:', error);
                     res.status(500).json({ code: 'SCHEDULE_DELETE_FAILED', error: String(error?.message ?? error).slice(0, 500) });
                 }