Motivation, from the evidence rather than from taste. Five packages each
implemented "load an item, then check the caller may read it": media, comics,
games, jellyfin (with its own dbItem type and its own SQL), and subsonic. Five
copies of one authorization rule means four can drift, and the failure mode is
not a crash — it is silently serving somebody a library they were never granted.
One implementation is one thing to review.
internal/catalog now owns item and file access, and the access check.
LoadUnchecked(ctx, db, id) -- no access check; jobs and tests only
LoadFor(ctx, db, u, id) -- load AND enforce the grant, in one call
Files / FilesFor / FileByID / FirstFile / FileCount
The unchecked variant is deliberately named so that reaching for it by accident
is hard: it is the dangerous call, so it should not have the friendlier name.
This came from the interface-design rule that a good interface makes the right
thing easy and the wrong thing hard.
The security property is tested, not asserted: a forbidden item and an absent
item return byte-identical errors, so no handler can leak the existence of
another user's library through a 403. That is now enforced in one place.
Migrated all five surfaces onto it. media, comics, games and ebooks went from
four duplicate loaders to zero. jellyfin's dbItem and loadDBItem are gone.
subsonic's per-item authorization moved while its album/artist aggregate
projections stayed put, because grouping items into albums is a presentation
concern, not a data-access one. The remaining raw SQL everywhere is browse and
aggregate queries; no per-item load decides access any more.
internal/arch enforces the layering instead of documenting it. Five rules, each
proven able to fail by deliberately breaking it:
- no upward imports across six explicit layers
- internal/core may depend only on migrations
- internal/catalog must not know about HTTP, or it stops being reusable
- modules must not import each other
- every package must be placed in a layer, so the map cannot go stale
A test that only ever passes is decoration, so each rule was verified by
introducing the violation and confirming the failure.
Verified: gofmt clean, go build ./... exit 0, go vet exit 0, 20 packages pass,
svelte-check 0/0. End to end against a running server: the vellum API serves
item detail with codec facts, an unknown item 404s, Jellyfin AuthenticateByName
mints a token and /Items browses, and Subsonic ping returns status ok. The
subsonic migration was additionally A/B'd byte-identical against the previous
implementation across ~15 endpoints, and Jellyfin/Subsonic both still return the
same error for a forbidden item as for an absent one.
Known follow-ups, recorded rather than silently skipped:
- internal/jellyfin has no test files at all. Its wire contract is the most
client-visible surface in the tree and is now verified only by hand.
- catalog is item-centric, but Subsonic addresses songs file-first (mf-<id>),
so it resolves file to item through its own projection. A catalog lookup
that takes a file id would close that.
- subsonic loadAlbum now scopes the projection to the item's own library
rather than all visible audio libraries. Equivalent by construction, since
an item's kind tracks its library's kind, but a hand-edited mismatch would
differ.
- PdfReader renders PDFs to canvas with page navigation and page-number
progress, reusing the same integer progress model as comics.
- Reader routes by file container, not media kind, so a PDF in a comics
library and a PDF ebook both open in the PDF reader.
- pdf.js is dynamically imported: the main bundle stays 443 kB and the 474 kB
pdf chunk plus 1.26 MB worker load only when a PDF is actually opened.
- comics cover extractor now only handles zip archives. A PDF in a comics
library is a valid item but not a zip, so it returns no cover instead of
logging an error on every request.
- fixed the page scale to leave slack for the vertical scrollbar, which was
otherwise forcing a horizontal scrollbar on tall pages.
Verified live: a 3-page PDF renders, navigates, and reports 33% progress.
A book is a folder of chapters, not a file. The scanner gained a grouping
mode so a directory of tracks becomes one item with ordered files.
- registry: MediaType.Group (file | directory). Audiobooks and music group by
directory; everything else stays one item per file.
- scanner: builds groups, upserts one item per group, marks items missing when
their directory disappears, and keeps the file-level added/updated/missing
accounting. Tracks are ordered naturally, so chapter 2 precedes chapter 10.
- natsort: one shared natural-order comparator, now used by both the scanner
and the comics module (which had its own copy).
- media: GET /items/{id}/files/{fileID} streams one specific track. It
verifies the file belongs to the item, so an item id cannot be paired with
another item's file id.
No new module was needed for playback: the polymorphic progress table already
keys on file_id, so per-chapter resume fell out of the existing design. The
web player resumes the exact track and offset.
Verified: 3 chapters grouped into one book titled "Mistborn", natural order,
range request returned 206, mismatched file id 404s, per-track progress
round-trips.
- comics: reads pages straight out of the archive on demand. No extraction
pass, so a 200-page CBZ is not materialised for a reader who views three
pages. Pages are read into memory and served via http.ServeContent, which
gives Range for free.
- natural page ordering: page2 sorts before page10. Lexical ordering shuffles
a numbered archive, which is the difference between a readable book and a
scrambled one.
- skips __MACOSX and dotfiles; rejects archives with no images.
- cover is the first page.
- media.Item gained ModTime (from the backing file) so page responses carry a
Last-Modified and support conditional GETs.
- web: ComicReader with a bounded prefetch window (two pages each way), page
jump, keyboard nav, and progress stored as a page number.
Verified: 3-page CBZ served in natural order, out-of-range 404, cover from
page 1, progress round-trip. svelte-check 0/0.
