Skip to content

company: cross-tenant read/update returns 403 — lets scoped callers enumerate the tenant table #173

@CryptoJones

Description

@CryptoJones

Problem

GET /v1/company/:id and PATCH /v1/company/:id distinguish two
adjacent error states by status code:

  • 404 "Not found" when :id does not exist (or is soft-deleted)
  • 403 "Invalid Authorization Key" when the company exists but
    belongs to a tenant other than the caller's

A non-master caller can iterate compId values and observe which IDs
yield 403 (exists, just not yours) vs 404 (doesn't exist). That
reveals the size and sparsity of the entire tenant table to anyone
holding a single scoped key — a small but real information leak.

Fix

Collapse both cases into 404 with the same "Not found." body. The
caller can no longer distinguish "this id is populated" from "this id
is absent" by status code or body shape.

  • Master-key callers continue to see every company (no behavior
    change for them)
  • Scoped callers continue to receive 200 + their own row
  • Soft-deleted rows (compArch: true) continue to map to 404

Acceptance

  • getById: non-master + existing-but-not-yours → 404
  • update: non-master + existing-but-not-yours → 404
  • Tests in tests/api/company.test.js pin both above
  • Master-key path unchanged

Follow-up not in scope

Timing-side leak: a 404-after-findByPk is measurably slower than the
404-before-findByPk path used for genuinely-absent IDs. Reordering
the auth check before the DB lookup would close it but expands the
diff beyond this one-concern PR.

Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions