# ADR-005: One item-access chokepoint, `internal/catalog`

## Status
Accepted

## Date
2026-10-12

## Context

Five different packages needed the same thing: load a media item, then check the
caller is allowed to read it.

- `internal/media`
- `internal/modules/comics`
- `internal/modules/games`
- `internal/jellyfin` — with its own `dbItem` type and its own SQL
- `internal/subsonic` — with its own per-item queries

Each had its own implementation, and each decided for itself what to do when
access was denied.

## Decision

One package owns item access **and** the access check: `internal/catalog`.

## Alternatives Considered

### Leave the duplication

- **Pros:** no refactor; each package stays self-contained.
- **Cons:** five copies of one authorization rule means four can drift from the
  fifth. The failure mode is not a crash — it is silently serving somebody a
  library they were never granted. That is a data-leak bug, and it would be found
  by accident, if at all.
- **Rejected:** this is the highest-value thing in the codebase to have exactly
  one of.

### A shared helper that packages call with their own SQL

- **Pros:** smaller change.
- **Cons:** leaves five copies of the item-loading SQL, so a schema change still
  has to be made in five places.
- **Rejected:** it halves the problem instead of removing it.

### A generic repository layer for everything

- **Pros:** one place for all SQL.
- **Cons:** pulls browse lists, home shelves and Subsonic album aggregates into a
  generic layer where they do not belong. Those are each package's own
  presentation concern, and moving them is a 186-statement rewrite.
- **Rejected:** the chokepoint is the security-critical, most-duplicated path —
  not every query.

## The security property

`catalog.ItemFor` returns the **same** `ErrNotFound` for an item that does not
exist and an item the caller may not read.

This is deliberate. If the two were distinguishable, a 403 would confirm that
another user's library contains that item, leaking its shape. The test asserts
the two responses are **byte-identical**, not merely both non-200.

## Consequences

- Five duplicate implementations became one. `internal/jellyfin`'s `dbItem` type
  and its own item SQL are gone.
- All four HTTP surfaces — web, modules, Jellyfin, Subsonic — go through the same
  check, so an API token can never see more than the web app can.
- An unreadable item is **404, never 403**, everywhere.
- Aggregate and list queries deliberately stayed where they are. `catalog` is
  item-centric; Subsonic is file-first (`mf-<id>`) and resolves file→item through
  its own projection. A catalog lookup that takes a file id would close that gap.
- The seam is a `Store` interface with a SQLite implementation, so the storage
  engine is swappable. One consumer (`internal/playback`) is threaded onto it;
  the rest still use the package-level wrappers, and the remaining threading is
  mechanical.