Skip to content

Security: Reports surface ignores caller identity — cross-user/cross-tenant IDOR (read/delete any report) + scheduled reports bypass owner RLS #2980

Description

@os-zhuang

Surfaced by the ADR-0096 execution-surface audit (the #2849 bug-class sweep). Two related defects on the reports surface, same root: ReportService discards the caller's ExecutionContext and runs against sys_saved_report / the report's target object with SYSTEM_CTX ({ isSystem: true }), so RLS/FLS/ownership/tenant scoping are all bypassed. The REST route layer only calls enforceAuth (authentication), not ownership.

Finding 1 — IDOR: any authenticated user can read or delete any saved report

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

  • getReport(reportId, _context) (:244) — _context unused; queries sys_saved_report by id with SYSTEM_CTX (:246).
  • deleteReport(reportId, _context) (:251) — same; deletes by id + cascades schedules with SYSTEM_CTX (:255-260).
  • listReports({ object, ownerId }) (:231) — ownerId is caller-supplied and optional; with it omitted, returns up to 500 reports across all owners with SYSTEM_CTX (:238-239).

The routes (packages/rest/src/rest-server.ts):

  • GET /data/reports/:id (:5448) → svc.getReport(req.params.id, context)
  • DELETE /data/reports/:id (:5472) → svc.deleteReport(req.params.id, context)
  • GET /data/reports (:5400) → svc.listReports({ object, ownerId: q.ownerId }, context)

Each does enforceAuth(req, res, context) then hands the id/query to the service, which throws the context away. Impact: an authenticated low-privilege user reads or deletes any other user's saved report by id (cross-owner; cross-tenant in tenancy.mode='multi'), and enumerates all reports via the list endpoint.

Finding 2 — Scheduled reports run as system, exfiltrating the target object

report-service.ts dispatchDue() (:411-425) executes each due schedule via:

const report = await this.getReport(schedule.report_id, { isSystem: true });      // :413
const result = await this.executeReport({ ...report, format: fmt }, { isSystem: true }, false); // :425

executeReport threads its context into the target-object query (:299-306), so { isSystem: true } makes the scheduled run bypass the report owner's RLS/sharing and email out every row of the target object. The interactive path (:299, caller-scoped) is correct; the scheduled path is not. Unlike flows post-#1888, reports have no owner-scoped runAs — and there is no warning/lint here.

Suggested fix

  • get/delete/listReports must run under the caller's context (respect _context), or enforce an explicit owner/tenant predicate against sys_saved_report. SYSTEM_CTX is legitimate only for server-controlled metadata stamps (last_run_at), not for authorization-bearing reads/deletes.
  • dispatchDue must resolve the schedule/report owner (owner_id, already on the row) to a real ExecutionContext and run executeReport under it — the reports analogue of ADR-0073 runAs:'user'/owner-scoping — not { isSystem: true }.

Relationship to ADR-0096 / #2849

This is the TRUSTED-IMPLICIT class the ADR names (a surface that ignores the caller and rides system authority). It is not the #2849 empty-principal fall-open (it's an unconditional SYSTEM_CTX bypass), but it is exactly what the ADR-0096 D4 conformance matrix + D2 systemContext(reason) audit would have caught. Tracking under the ADR-0096 evidence base; fix independently of the mechanism.

Refs: ADR-0096 (PR #2975), #2849, ADR-0073 (runAs posture).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority:p1High: required for production / M2

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions