Drift audit 2026-06-02 — 26 findings shipped #75

Merged
bvandeusen merged 11 commits from dev into main 2026-06-02 19:21:53 -04:00
8 changed files with 88 additions and 25 deletions
Showing only changes of commit 47d2f61161 - Show all commits
+28 -9
View File
@@ -69,15 +69,32 @@ func TestAdminLibraryCoverage_MixedRowsReturnCorrectBuckets(t *testing.T) {
sourceNone := "none" sourceNone := "none"
sourceSidecar := "sidecar" sourceSidecar := "sidecar"
sourceMbcaa := "mbcaa" sourceMbcaa := "mbcaa"
sourceEmbedded := "embedded"
sourceTheaudiodb := "theaudiodb"
sourceDeezer := "deezer"
sourceLastfm := "lastfm"
mbid1 := "11111111-1111-1111-1111-111111111111" mbid1 := "11111111-1111-1111-1111-111111111111"
mbid2 := "22222222-2222-2222-2222-222222222222" mbid2 := "22222222-2222-2222-2222-222222222222"
mbid3 := "33333333-3333-3333-3333-333333333333" mbid3 := "33333333-3333-3333-3333-333333333333"
mbid4 := "44444444-4444-4444-4444-444444444444"
mbid5 := "55555555-5555-5555-5555-555555555555"
mbid6 := "66666666-6666-6666-6666-666666666666"
mbid7 := "77777777-7777-7777-7777-777777777777"
// Cover every cover_art_source value the migrations allow so a future
// addition without updating the rollup query trips this test —
// regression guard for drift #557, the gap that hid #556 (deezer +
// lastfm omission introduced by migration 0020 but never wired into
// the rollup whitelist).
rows := []seed{ rows := []seed{
{title: "WithArtSidecar", source: &sourceSidecar, mbid: &mbid1}, // with_art {title: "WithArtSidecar", source: &sourceSidecar, mbid: &mbid1}, // with_art
{title: "WithArtMbcaa", source: &sourceMbcaa, mbid: &mbid2}, // with_art {title: "WithArtMbcaa", source: &sourceMbcaa, mbid: &mbid2}, // with_art
{title: "PendingHasMbid", source: nil, mbid: &mbid3}, // pending (eligible) {title: "WithArtEmbedded", source: &sourceEmbedded, mbid: &mbid4}, // with_art
{title: "PendingNoMbid", source: nil, mbid: nil}, // pending + pending_no_mbid {title: "WithArtTheaudiodb", source: &sourceTheaudiodb, mbid: &mbid5}, // with_art
{title: "Settled", source: &sourceNone, mbid: nil}, // settled (no mbid is fine here) {title: "WithArtDeezer", source: &sourceDeezer, mbid: &mbid6}, // with_art
{title: "WithArtLastfm", source: &sourceLastfm, mbid: &mbid7}, // with_art
{title: "PendingHasMbid", source: nil, mbid: &mbid3}, // pending (eligible)
{title: "PendingNoMbid", source: nil, mbid: nil}, // pending + pending_no_mbid
{title: "Settled", source: &sourceNone, mbid: nil}, // settled (no mbid is fine here)
} }
for _, s := range rows { for _, s := range rows {
if _, err := pool.Exec(context.Background(), ` if _, err := pool.Exec(context.Background(), `
@@ -100,11 +117,13 @@ func TestAdminLibraryCoverage_MixedRowsReturnCorrectBuckets(t *testing.T) {
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
t.Fatalf("decode: %v", err) t.Fatalf("decode: %v", err)
} }
if resp.Total != 5 { if resp.Total != 9 {
t.Errorf("Total = %d, want 5", resp.Total) t.Errorf("Total = %d, want 9", resp.Total)
} }
if resp.WithArt != 2 { // Six with_art rows — one per valid cover_art_source value
t.Errorf("WithArt = %d, want 2", resp.WithArt) // (sidecar, mbcaa, embedded, theaudiodb, deezer, lastfm).
if resp.WithArt != 6 {
t.Errorf("WithArt = %d, want 6", resp.WithArt)
} }
if resp.Pending != 2 { if resp.Pending != 2 {
t.Errorf("Pending = %d, want 2", resp.Pending) t.Errorf("Pending = %d, want 2", resp.Pending)
+8 -5
View File
@@ -126,7 +126,8 @@ func (q *Queries) GetAlbumByID(ctx context.Context, id pgtype.UUID) (Album, erro
const getAlbumCoverageRollup = `-- name: GetAlbumCoverageRollup :one const getAlbumCoverageRollup = `-- name: GetAlbumCoverageRollup :one
SELECT SELECT
COUNT(*) AS total, COUNT(*) AS total,
COUNT(*) FILTER (WHERE cover_art_source IN ('sidecar','embedded','mbcaa','theaudiodb')) AS with_art, COUNT(*) FILTER (WHERE cover_art_source IN
('sidecar','embedded','mbcaa','theaudiodb','deezer','lastfm')) AS with_art,
COUNT(*) FILTER (WHERE cover_art_source IS NULL) AS pending, COUNT(*) FILTER (WHERE cover_art_source IS NULL) AS pending,
COUNT(*) FILTER (WHERE cover_art_source = 'none') AS settled, COUNT(*) FILTER (WHERE cover_art_source = 'none') AS settled,
COUNT(*) FILTER (WHERE cover_art_source IS NULL AND mbid IS NULL) AS pending_no_mbid COUNT(*) FILTER (WHERE cover_art_source IS NULL AND mbid IS NULL) AS pending_no_mbid
@@ -151,10 +152,12 @@ type GetAlbumCoverageRollupRow struct {
// Invariant: with_art + pending + settled = total. (pending_no_mbid is // Invariant: with_art + pending + settled = total. (pending_no_mbid is
// not part of the sum — it's a subset of pending, surfaced separately.) // not part of the sum — it's a subset of pending, surfaced separately.)
// //
// The IN list below ('sidecar','embedded','mbcaa','theaudiodb') must stay in sync // The IN list below must stay in sync with the cover_art_source CHECK
// with the cover_art_source CHECK constraint in migration // constraint — currently relaxed by migration 0020 to include 'deezer'
// 0016_album_cover_source.up.sql. If a new source value is added there // and 'lastfm', and migration 0030 further relaxed it to any non-empty
// without updating this query, with_art will silently undercount. // string. If a new source value is added without updating this query,
// with_art will silently undercount. Drift #556 caught the deezer +
// lastfm omission introduced by migration 0020.
func (q *Queries) GetAlbumCoverageRollup(ctx context.Context) (GetAlbumCoverageRollupRow, error) { func (q *Queries) GetAlbumCoverageRollup(ctx context.Context) (GetAlbumCoverageRollupRow, error) {
row := q.db.QueryRow(ctx, getAlbumCoverageRollup) row := q.db.QueryRow(ctx, getAlbumCoverageRollup)
var i GetAlbumCoverageRollupRow var i GetAlbumCoverageRollupRow
+8 -5
View File
@@ -156,13 +156,16 @@ SELECT a.id AS album_id,
-- Invariant: with_art + pending + settled = total. (pending_no_mbid is -- Invariant: with_art + pending + settled = total. (pending_no_mbid is
-- not part of the sum — it's a subset of pending, surfaced separately.) -- not part of the sum — it's a subset of pending, surfaced separately.)
-- --
-- The IN list below ('sidecar','embedded','mbcaa','theaudiodb') must stay in sync -- The IN list below must stay in sync with the cover_art_source CHECK
-- with the cover_art_source CHECK constraint in migration -- constraint — currently relaxed by migration 0020 to include 'deezer'
-- 0016_album_cover_source.up.sql. If a new source value is added there -- and 'lastfm', and migration 0030 further relaxed it to any non-empty
-- without updating this query, with_art will silently undercount. -- string. If a new source value is added without updating this query,
-- with_art will silently undercount. Drift #556 caught the deezer +
-- lastfm omission introduced by migration 0020.
SELECT SELECT
COUNT(*) AS total, COUNT(*) AS total,
COUNT(*) FILTER (WHERE cover_art_source IN ('sidecar','embedded','mbcaa','theaudiodb')) AS with_art, COUNT(*) FILTER (WHERE cover_art_source IN
('sidecar','embedded','mbcaa','theaudiodb','deezer','lastfm')) AS with_art,
COUNT(*) FILTER (WHERE cover_art_source IS NULL) AS pending, COUNT(*) FILTER (WHERE cover_art_source IS NULL) AS pending,
COUNT(*) FILTER (WHERE cover_art_source = 'none') AS settled, COUNT(*) FILTER (WHERE cover_art_source = 'none') AS settled,
COUNT(*) FILTER (WHERE cover_art_source IS NULL AND mbid IS NULL) AS pending_no_mbid COUNT(*) FILTER (WHERE cover_art_source IS NULL AND mbid IS NULL) AS pending_no_mbid
+9 -2
View File
@@ -23,13 +23,20 @@ import (
syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
) )
// audioExtensions is the v1 set. Duration extraction is not wired, so all of // audioExtensions is the set the scanner indexes. Keep in sync with
// these get duration_ms=0 until an ffprobe / native-decoder pass lands. // `internal/api/media.go` MIME detection — the stream handler must be
// able to serve every extension the scanner indexes, and there is no
// point in adding extensions to the stream handler that the scanner
// will silently skip. Drift #571 caught the divergence after .opus,
// .aac, and .wav were added to media.go but not here.
var audioExtensions = map[string]bool{ var audioExtensions = map[string]bool{
".mp3": true, ".mp3": true,
".m4a": true, ".m4a": true,
".flac": true, ".flac": true,
".ogg": true, ".ogg": true,
".opus": true,
".aac": true,
".wav": true,
} }
type Stats struct { type Stats struct {
+7 -1
View File
@@ -106,7 +106,13 @@ export type LikedIdsResponse = {
}; };
export type EventRequest = export type EventRequest =
| { type: 'play_started'; track_id: string; client_id?: string } // `source` is the system-playlist variant the play came from
// (for_you, discover, deep_cuts, …); empty/undefined for library
// / user-playlist / radio / Subsonic. Server stores it on
// play_events.source and uses it to advance the rotation. Drift
// #555: the type was missing `source` so callers had no typed slot
// for it and a "clean up extra properties" refactor could drop it.
| { type: 'play_started'; track_id: string; client_id?: string; source?: string }
| { type: 'play_ended'; play_event_id: string; duration_played_ms: number } | { type: 'play_ended'; play_event_id: string; duration_played_ms: number }
| { type: 'play_skipped'; play_event_id: string; position_ms: number }; | { type: 'play_skipped'; play_event_id: string; position_ms: number };
+18
View File
@@ -30,4 +30,22 @@ describe('isPublicRoute', () => {
expect(isPublicRoute('/login/extra')).toBe(false); expect(isPublicRoute('/login/extra')).toBe(false);
expect(isPublicRoute('/loginx')).toBe(false); expect(isPublicRoute('/loginx')).toBe(false);
}); });
// Regression guard for drift #558: /forgot-password and the
// /reset-password/<token> deep links must be reachable without a
// session — the reset flow is entered by clicking an email link
// while signed out, so the auth gate redirecting to /login broke
// account recovery.
test('/forgot-password is public', () => {
expect(isPublicRoute('/forgot-password')).toBe(true);
});
test('/reset-password/<token> is public', () => {
expect(isPublicRoute('/reset-password/abc-123')).toBe(true);
expect(isPublicRoute('/reset-password/')).toBe(true);
});
test('/reset-password (no trailing slash) is NOT public — only the token sub-path', () => {
expect(isPublicRoute('/reset-password')).toBe(false);
});
}); });
+9 -2
View File
@@ -2,8 +2,15 @@
// when the visitor is unauthenticated. Bootstrap-admin self-registration // when the visitor is unauthenticated. Bootstrap-admin self-registration
// (#376) requires /register to be reachable without a session — without // (#376) requires /register to be reachable without a session — without
// /register here, the login page's "Register" link bounces back to /login. // /register here, the login page's "Register" link bounces back to /login.
const PUBLIC_ROUTES = new Set(['/login', '/register']); //
// Drift #558: /forgot-password and /reset-password/<token> were missing
// from this set. The email-link reset flow is by definition entered
// without a session — a signed-out user clicking their reset link was
// being bounced to /login, breaking account recovery.
const PUBLIC_ROUTES = new Set(['/login', '/register', '/forgot-password']);
const PUBLIC_PREFIXES = ['/reset-password/'];
export function isPublicRoute(pathname: string): boolean { export function isPublicRoute(pathname: string): boolean {
return PUBLIC_ROUTES.has(pathname); if (PUBLIC_ROUTES.has(pathname)) return true;
return PUBLIC_PREFIXES.some((p) => pathname.startsWith(p));
} }
@@ -274,7 +274,7 @@
<button <button
type="button" type="button"
onclick={confirmDeleteFile} onclick={confirmDeleteFile}
class="rounded bg-action-danger px-3 py-2 text-sm text-action-fg hover:opacity-90" class="rounded bg-action-destructive px-3 py-2 text-sm text-action-fg hover:opacity-90"
>Delete file</button> >Delete file</button>
</div> </div>
{/if} {/if}