4 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
mithral
|
87c17d4854 |
Make the login limiter correct behind a reverse proxy
Some checks failed
CI / ci (push) Has been cancelled
ClientIP used r.RemoteAddr and ignored X-Forwarded-For. That was right about not trusting the header from an untrusted peer, but it is only half the design: behind a proxy every request arrives from the proxy, so the limiter key in handlers.go was "<proxy-ip>|<username>" for every client on the internet. One attacker locked out a username for everyone, an attacker rotating usernames never hit a limit, and sessions.ip recorded the proxy so the device list was useless. - VELLUM_TRUSTED_PROXIES is a CIDR list. X-Forwarded-For is honoured only when RemoteAddr is inside it, and the chain is walked right-to-left so the first address that is not itself a trusted proxy wins. Everything to its left was appended by a client we do not control, so a spoofed leftmost entry is discarded. - VELLUM_PUBLIC_URL is the external origin. CookieSecure now defaults to true when it is https://, so a TLS deployment that forgets VELLUM_COOKIE_SECURE no longer ships a plaintext session cookie. - A malformed CIDR fails the boot rather than being skipped: silently dropping a proxy network widens the trust boundary, which is the exact failure this setting exists to prevent. ClientIP became a method on Authenticator so it can see the trusted set, which is why handlers.go and main.go changed as well. Verified red-green: neutering the trusted-peer path fails five ClientIP tests, including the spoofed-leftmost case. Known gap, pre-existing and not fixed here: this limiter still covers only /api/v1/auth/login. Jellyfin AuthenticateByName and Subsonic auth call auth.Authenticate with no limiting, so the same brute force is available through those two surfaces. |
||
|
mithral
|
00c1c0d619 |
Wave 2: correctness and security
TouchSession throttle (internal/auth)
- Middleware wrote sessions.last_seen on EVERY authenticated request, taking
SQLite's single write lock each time. For media that is severe: one seek in
an audio file fires several Range requests, each taking the write lock and
competing with the scanner and progress saves.
- Now touched at most once per 5 minutes, decided from the LastSeen already
loaded by SessionByToken, so no extra query. A zero interval degrades to the
old per-request behaviour, so a hand-built Authenticator cannot silently stop
updating.
- Verified end to end with an AFTER UPDATE trigger: 20 requests now cause 0
session writes, down from 20.
Response writer forwarding (internal/httpapi)
- statusRecorder wrapped every ResponseWriter but implemented only Write and
WriteHeader, so it did not implement http.Flusher. That silently disabled
early flushing (progressive audio/video) and io.ReaderFrom, which is what
lets io.Copy use sendfile for multi-GB files. Now forwards Flush, Hijack and
ReadFrom, and WriteHeader is idempotent.
Security headers (internal/httpapi)
- Added nosniff, X-Frame-Options: DENY, Referrer-Policy and a CSP. Outermost in
the chain so a recovered panic's 500 still carries them.
- CORRECTION to an earlier claim: I said a malicious EPUB was stored XSS.
That was wrong. epub.js sets iframe sandbox to "allow-same-origin" and only
appends "allow-scripts" when settings.allowScriptedContent is true, which
defaults to false and Reader.svelte never sets. EPUB content cannot execute
script. The CSP is still worth having as defence in depth, but that specific
justification does not hold.
Final flush on teardown (AudiobookPlayer, Reader)
- The last playback/reading position was saved with a fire-and-forget fetch in
onDestroy, which the browser cancels on navigate-away — losing exactly the
thing worth persisting. Now persisted via fetch with keepalive: true, wired
to pagehide and visibilitychange(hidden), latched so the overlapping events
fire one request, and reset when the page becomes visible again so a bfcache
restore still flushes later. sendBeacon was not usable: it is POST-only and
progress is PUT-only.
- Also removed the queueMicrotask track-swap race in AudiobookPlayer: play() is
now driven by onLoadedMetadata once the new src has actually loaded.
Honest readers (Reader, ComicReader)
- registry.go advertises .mobi/.azw3/.fb2 under ebooks, but every ebooks item
was handed to epub.js, which cannot parse them. Now only .epub uses epub.js;
other containers show a message naming the format plus a download link. Items
stay catalogued and downloadable rather than silently vanishing.
- .cbr/.rar comics say the archive format is unsupported instead of surfacing
the server's raw 422 (the server reads zip only).
Verified in a real browser, not just compiled: all six reader surfaces render
(epub iframe, cbz images, audio element, photo, pdf canvas) with zero CSP
violations; .mobi shows the honest message with no epub.js iframe. gofmt/vet
clean, 14 packages pass, svelte-check 0/0.
Known issue, NOT introduced here and NOT fixed: the EPUB reader logs
"Error: No Element Provided" x3 and a TypeError in epub.js injectIdentifier
(doc.getElementsByTagName("head")[0] undefined). Proven pre-existing by A/B
against
|
||
|
mithral
|
c50b3cfb3b |
Wave 1: survive a real library
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. |
||
|
mithral
|
9772e55c73 |
Phase 1: accounts, sessions, per-library grants
Multi-user from the first migration, as planned. - auth: argon2id password hashing (PHC encoded, parameters travel with the hash), opaque session tokens stored only as SHA-256, login rate limiting, and account enumeration resistance (unknown user hashes anyway) - sessions: cookie or Bearer auth, HttpOnly + SameSite=Lax, revocation on password change - grants: library_grants is the single access-control chokepoint. Admins see every library; everyone else sees exactly their can_read grants - bootstrap: VELLUM_ADMIN_USER/PASSWORD create the first admin only on an empty database. No default password is ever invented. - httpapi: mounts modules on a shared mux, session middleware innermost - httpx: shared JSON helpers, strict body decoding Gate: two users see different libraries. Tests cover that, plus session expiry, token-at-rest, grant upsert, and the full login/me/logout flow. |