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.
A 50k-file library used to wedge the database. This wave fixes the four
reasons why, plus a timestamp-ordering bug the work exposed.
Chunked scans (internal/library/scan.go)
- One transaction wrapped the whole reconciliation, holding SQLite's single
write lock for the entire scan. Now 500 groups per transaction, and the two
missing-file passes are batched too.
- Verified: a 20,000-file scan reports partial progress (scanned=1500 mid-scan)
and a concurrent progress save returns 200 throughout. Under the old code
that save blocked for the whole scan.
- Behaviour change: a scan is no longer all-or-nothing. A crash now leaves a
consistent partial scan. The scan is idempotent, so re-running finishes it.
- res.Errors is capped at 100 (a broken mount used to append one string per
file, then serialise the lot into the job row); the overflow is counted.
Concurrent job workers (internal/jobs/jobs.go)
- Run() was single-threaded, so a scan stalled every enrichment job behind it.
Now 4 workers with jittered idle waits.
- Handlers run concurrently, so Register/lookup take an RWMutex, and a
panicking handler is recovered and marked failed rather than killing a worker
and stranding a row in 'running'.
- RecoverStale() requeues jobs left 'running' by a dead process; called at boot
from main.go. Previously such a job stayed 'running' forever and the UI
polled it forever.
Explore (internal/media/discover.go)
- Full-scanned and sorted the whole library on every home load, and was
deterministic, so it showed the same 30 items every visit. Now a per-kind
fair-share id range scan with a random start, wrap-around, dedup and
interleaving. The doc comment claimed per-kind sampling before; now it does.
Indexes (migrations/0008_query_indexes.sql)
- Benchmarked with ANALYZE over 60k items: browse page 1 13.37ms -> 0.03ms
(484x, full scan -> covering index), deep page 8x, recently added 3.2x.
- A candidate index for the Continue shelf was built, measured and REJECTED:
the planner never chose it because `finished = 0` selects the majority of a
user's rows, so it had no selectivity and would have cost a write on every
progress save. Recorded in the migration rather than shipped as dead weight.
Canonical timestamps (internal/core/stamp.go)
- time.RFC3339Nano trims trailing zeros, so its fraction width varies and the
strings are not lexicographically sortable: ".5Z" > ".55Z" as strings though
.500s < .550s. That broke every ORDER BY <timestamp> and MAX(timestamp),
including the Continue shelf and Most Viewed. It was wrong even in columns
written 100% by Go, so it was not a cross-format issue.
- All 14 write sites now use core.Stamp, matching SQLite's own default format
exactly. Read paths are untouched: RFC3339Nano parses both shapes, so
existing rows keep working.
Also: busy_timeout 5s -> 15s. Chunked scans still delay a concurrent write, and
measured tail latency on a first scan of 15k new files reached 3.3s, leaving
almost no margin at 5s. Waiting beats failing with SQLITE_BUSY and losing a
playback position. A 10ms inter-batch yield was implemented, measured, found to
have no effect distinguishable from noise (overlapping ranges), and removed.
Verified end to end, not just compiled: orphan recovery logged at boot, scan
returns 202 in 0.7ms, Explore varies across loads, concurrent writes survive a
scan. gofmt/vet clean, 14 packages pass, -race clean on the queue, svelte-check
0/0, fresh install applies all 8 migrations.
The RomM replacement.
- registry: a games media type with ~35 ROM extensions across NES, SNES, GB/GBA,
N64, NDS, Genesis/Master System/Game Gear, PC Engine, Neo Geo Pocket,
WonderSwan, Atari, Coleco, Intellivision, and PlayStation.
- games: platform detection from the file extension, mapping each to an
EmulatorJS core. Platforms with no browser core are recognised but marked
not-playable so the UI offers a download instead of a broken player.
- migration 0005: game_saves keyed by (user, item, slot). Save states are user
data, not media — two people playing the same ROM keep their own progress.
- save endpoints with a 16 MiB cap and a validated slot name; the upload is
read through a LimitReader so an unbounded body is refused rather than
buffered.
- igdb provider: Twitch client-credentials flow with the token cached for its
~2 month lifetime, Apicalypse query body, and cover URLs normalised from
protocol-relative t_thumb to https t_cover_big.
- searchTitle: strips "(USA)", "[!]", "(Rev 1)" and similar release tags before
searching. "Sonic the Hedgehog (USA)" matched nothing; "Sonic the Hedgehog"
matches immediately. Stored titles keep the original.
- web: GamePlayer embeds EmulatorJS and adds Load/Save-to-server buttons.
Verified live against IGDB: Sonic matched (id 316900) with real cover art; a
4 KB save uploaded, listed, downloaded byte-identical, and deleted.
mithral