fix(security): scope tabular-review document_ids by access (CWE-639)

✅ merged · #13 · easterbrooka/mike ← easterbrooka/mike · opened 15d ago by easterbrooka · merged 15d ago by easterbrooka · self · +97-6 across 2 files · ↗ on GitHub

From the PR description

The tabular-review routes accept user-supplied document_ids in request bodies (POST /tabular-review, PATCH /:reviewId) and stale cell rows on byte-fetching paths (POST /:reviewId/regenerate-cell, POST /:reviewId/generate). None of those paths checked whether the caller can read those documents - a free-account attacker could plant foreign UUIDs into their own review and have the server fetch the bytes from R2 + run an LLM extraction over them, returning verbatim text via the standard review GET.

Adds filterAccessibleDocumentIds(documentIds, userId, userEmail, db) next to the existing access helpers (owner-of-doc OR project member), and applies it at the four entry points:

  • POST /tabular-review drop unauthorised on insert
  • PATCH /:reviewId drop newly-added unauthorised; keep already-attached cells so non-owner collaborators don't accidentally orphan rows they can't directly access
  • POST /:reviewId/regenerate-cell refuse byte fetch when caller has no access to the underlying doc
  • POST /:reviewId/generate filter docIds before parallel LLM fetch (defense-in-depth for legacy cells planted before this fix)

Fails closed silently rather than 403'ing so legacy clients that pass stale ids don't error out the whole review.

Detected by Aeon + manual review. Severity: high CWE-639 (Authorization Bypass Through User-Controlled Key)

Our analysis

Close IDOR in tabular review document access — read the full analysis →

Think the analysis missed something the PR description covers?

Capture this PR into my fork

Download a Markdown prompt that tells Claude how to port every commit in this PR into your working tree. Run it via claude -p < capture-pull-13.md from inside the repo you want the changes in.

⬇ Download capture-pull-13.md