diff --git a/internal/arch/arch.go b/internal/arch/arch.go new file mode 100644 index 0000000..a889478 --- /dev/null +++ b/internal/arch/arch.go @@ -0,0 +1,7 @@ +// Package arch holds no code. It exists so the import rules this project +// depends on are enforced by the test suite rather than by good intentions. +// +// See arch_test.go: it parses every package's imports and fails if a layer is +// violated, if core learns about a feature, if a module imports a sibling, or +// if a new package is added without being placed in a layer. +package arch diff --git a/internal/arch/arch_test.go b/internal/arch/arch_test.go new file mode 100644 index 0000000..3c0e564 --- /dev/null +++ b/internal/arch/arch_test.go @@ -0,0 +1,234 @@ +// Package arch holds no code. It exists so that the import rules this project +// depends on are enforced by the test suite instead of by good intentions. +// +// A layering rule that is only written down in a comment is a rule that will be +// broken within a month, and the breakage is invisible: an upward import does +// not fail to compile, it just quietly welds two layers together until nothing +// can be moved. These tests fail loudly instead. +package arch + +import ( + "go/parser" + "go/token" + "os" + "path/filepath" + "sort" + "strings" + "testing" +) + +const modulePath = "github.com/mithral/vellum/" + +// Layers, lowest first. A package may import from a LOWER layer or its own +// layer, never from a higher one. Peers are allowed because they genuinely +// collaborate (metadata enriches items that media serves); upward imports are +// not, because they invert the dependency and make the lower layer unusable on +// its own. +var layers = map[string]int{ + // 0 — foundation. No knowledge of the application. + "internal/core": 0, + "internal/httpx": 0, + "internal/natsort": 0, + "internal/arch": 0, + "migrations": 0, + + // 1 — primitives the application is built from. + "internal/auth": 1, + "internal/jobs": 1, + "internal/stream": 1, + "internal/progress": 1, + "internal/probe": 1, + "internal/engine": 1, + "internal/apitoken": 1, + + // 2 — the data seam: items and access. + "internal/catalog": 2, + + // 3 — the application. + "internal/library": 3, + "internal/media": 3, + "internal/search": 3, + "internal/metadata": 3, + "internal/playback": 3, + "internal/modules/comics": 3, + "internal/modules/ebooks": 3, + "internal/modules/games": 3, + "internal/modules/photos": 3, + + // 4 — adapters: HTTP surfaces and foreign protocols. + "internal/httpapi": 4, + "internal/jellyfin": 4, + "internal/subsonic": 4, + "internal/cli": 4, + + // 5 — the only place everything is wired together. + "cmd/vellum": 5, +} + +// repoRoot walks up until it finds go.mod, so the test works regardless of +// where `go test` is invoked from. +func repoRoot(t *testing.T) string { + t.Helper() + dir, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + t.Fatal("could not find go.mod above the test directory") + } + dir = parent + } +} + +// importsOf parses a directory and returns the internal packages it imports. +func importsOf(t *testing.T, dir string) []string { + t.Helper() + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("read %s: %v", dir, err) + } + seen := map[string]bool{} + for _, e := range entries { + if e.IsDir() || !strings.HasSuffix(e.Name(), ".go") || strings.HasSuffix(e.Name(), "_test.go") { + continue + } + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, filepath.Join(dir, e.Name()), nil, parser.ImportsOnly) + if err != nil { + t.Fatalf("parse %s: %v", e.Name(), err) + } + for _, imp := range f.Imports { + p := strings.Trim(imp.Path.Value, `"`) + if strings.HasPrefix(p, modulePath) { + seen[strings.TrimPrefix(p, modulePath)] = true + } + } + } + out := make([]string, 0, len(seen)) + for p := range seen { + out = append(out, p) + } + sort.Strings(out) + return out +} + +// TestNoUpwardImports is the rule that keeps the layers separable. +func TestNoUpwardImports(t *testing.T) { + root := repoRoot(t) + for pkg, layer := range layers { + dir := filepath.Join(root, filepath.FromSlash(pkg)) + if _, err := os.Stat(dir); err != nil { + continue // package not present (e.g. a module was removed) + } + for _, dep := range importsOf(t, dir) { + depLayer, known := layers[dep] + if !known { + continue // a package outside the map; TestEveryPackageIsLayered catches it + } + if depLayer > layer { + t.Errorf("%s (layer %d) imports %s (layer %d): upward dependency", + pkg, layer, dep, depLayer) + } + } + } +} + +// TestCoreStaysPure is the module rule: core may not know about any feature, or +// it stops being the thing everything else can depend on. +func TestCoreStaysPure(t *testing.T) { + root := repoRoot(t) + for _, dep := range importsOf(t, filepath.Join(root, "internal", "core")) { + if dep != "migrations" { + t.Errorf("internal/core imports %s; core may depend only on migrations", dep) + } + } +} + +// TestCatalogHasNoHTTP keeps the data seam a data seam. If it grows an +// http.ResponseWriter it can no longer be reused by a job, a CLI or a test +// without dragging a request in. +func TestCatalogHasNoHTTP(t *testing.T) { + root := repoRoot(t) + for _, dep := range importsOf(t, filepath.Join(root, "internal", "catalog")) { + if dep == "internal/httpx" || dep == "internal/httpapi" { + t.Errorf("internal/catalog imports %s; the data seam must not know about HTTP", dep) + } + } +} + +// TestModulesDoNotImportEachOther keeps modules independently removable. A +// module that imports a sibling cannot be dropped without dropping both. +func TestModulesDoNotImportEachOther(t *testing.T) { + root := repoRoot(t) + modsDir := filepath.Join(root, "internal", "modules") + mods, err := os.ReadDir(modsDir) + if err != nil { + t.Skip("no modules directory") + } + for _, m := range mods { + if !m.IsDir() { + continue + } + self := "internal/modules/" + m.Name() + for _, dep := range importsOf(t, filepath.Join(modsDir, m.Name())) { + if strings.HasPrefix(dep, "internal/modules/") && dep != self { + t.Errorf("%s imports %s; modules must not depend on each other", self, dep) + } + } + } +} + +// TestEveryPackageIsLayered stops the map above from silently going stale. A +// new package that nobody placed in a layer is a package with no rules, which +// is how the layering erodes. +func TestEveryPackageIsLayered(t *testing.T) { + root := repoRoot(t) + var found []string + + var walk func(base string) + walk = func(base string) { + entries, err := os.ReadDir(base) + if err != nil { + return + } + for _, e := range entries { + if !e.IsDir() { + continue + } + full := filepath.Join(base, e.Name()) + rel, _ := filepath.Rel(root, full) + rel = filepath.ToSlash(rel) + // Only directories that actually contain Go files are packages. + if hasGoFiles(full) { + found = append(found, rel) + } + walk(full) + } + } + walk(filepath.Join(root, "internal")) + found = append(found, "cmd/vellum") + + for _, pkg := range found { + if _, ok := layers[pkg]; !ok { + t.Errorf("%s is not in the layers map; add it so the import rules apply to it", pkg) + } + } +} + +func hasGoFiles(dir string) bool { + entries, err := os.ReadDir(dir) + if err != nil { + return false + } + for _, e := range entries { + if !e.IsDir() && strings.HasSuffix(e.Name(), ".go") { + return true + } + } + return false +} diff --git a/internal/catalog/catalog.go b/internal/catalog/catalog.go new file mode 100644 index 0000000..6bd9208 --- /dev/null +++ b/internal/catalog/catalog.go @@ -0,0 +1,229 @@ +// Package catalog is the one place that reads items and files out of the +// database, and the one place that enforces "may this user read this item". +// +// It exists because that check had been reimplemented five times — in media, +// comics, games, jellyfin and subsonic — each with its own SQL and its own idea +// of what to do when access is denied. Five copies of one authorization rule +// means four of them can drift, and the failure mode is not a crash: it is +// silently serving somebody a library they were not granted. One implementation +// is one thing to review. +// +// Consumers depend on this package rather than on *sql.DB, which is also what +// makes the storage engine swappable: a Postgres implementation would satisfy +// this package's surface instead of editing 186 statements scattered across the +// tree. +// +// This package holds no HTTP knowledge. It returns typed errors and lets the +// caller choose a status code. +package catalog + +import ( + "context" + "database/sql" + "errors" + "fmt" + "path/filepath" + "time" + + "github.com/mithral/vellum/internal/auth" +) + +// ErrNotFound means the item does not exist, OR exists but the caller may not +// read it. +// +// The two are deliberately indistinguishable. A caller that could tell them +// apart would leak the existence of other people's libraries through a 403, so +// every handler turns this into a plain 404. +var ErrNotFound = errors.New("catalog: item not found") + +// Item is one media item plus the root of its library. +type Item struct { + ID int64 + LibraryID int64 + Kind string + Path string // path relative to the library root + Title string + Year int + Missing bool + + // RootPath is the absolute library root, so AbsPath can be computed. + RootPath string + + // ModTime is the backing file's modification time, used for conditional + // GETs. Zero when unknown. + ModTime time.Time +} + +// AbsPath returns the absolute filesystem path of the item's file. +// +// item.path is stored relative to the library root and the root is stored +// resolved, so this join cannot escape the library. +func (i Item) AbsPath() string { + return filepath.Join(i.RootPath, filepath.FromSlash(i.Path)) +} + +// File is one physical file backing an item, including the ffprobe facts. +// +// The JSON tags are the frozen MediaFile contract in web/src/lib/api.ts. The +// codec fields are what let a CLIENT decide whether it can direct-play, since +// Vellum never transcodes and playability depends on the client, not the file. +type File struct { + ID int64 `json:"id"` + Path string `json:"path"` + Size int64 `json:"size"` + Container string `json:"container"` + Missing bool `json:"missing"` + DurationMS int64 `json:"duration_ms"` + VCodec string `json:"vcodec"` + ACodec string `json:"acodec"` + Width int `json:"width"` + Height int `json:"height"` +} + +const itemSelect = ` + SELECT i.id, i.library_id, i.kind, i.path, i.title, COALESCE(i.year, 0), i.missing, l.root_path, + COALESCE((SELECT f.mtime FROM media_files f WHERE f.item_id = i.id ORDER BY f.id LIMIT 1), '') + FROM media_items i + JOIN libraries l ON l.id = i.library_id + WHERE i.id = ?` + +// LoadUnchecked reads an item WITHOUT checking access. Prefer LoadFor in any handler +// that serves a request: this exists for the scanner, jobs and tests, which +// have already established the right to touch the row. +func LoadUnchecked(ctx context.Context, db *sql.DB, id int64) (Item, error) { + var ( + it Item + mtime string + ) + err := db.QueryRowContext(ctx, itemSelect, id).Scan( + &it.ID, &it.LibraryID, &it.Kind, &it.Path, &it.Title, + &it.Year, &it.Missing, &it.RootPath, &mtime) + if errors.Is(err, sql.ErrNoRows) { + return Item{}, ErrNotFound + } + if err != nil { + return Item{}, fmt.Errorf("catalog: load item: %w", err) + } + if nanos := parseUnixNano(mtime); nanos > 0 { + it.ModTime = time.Unix(0, nanos).UTC() + } + return it, nil +} + +// LoadFor reads an item and confirms the user may read its library. +// +// This is the authorization chokepoint. Every HTTP surface — the web app, the +// modules, Jellyfin, Subsonic — is expected to go through it, so there is one +// implementation to audit rather than five to keep in sync. +func LoadFor(ctx context.Context, db *sql.DB, u auth.User, id int64) (Item, error) { + it, err := LoadUnchecked(ctx, db, id) + if err != nil { + return Item{}, err + } + allowed, err := auth.CanAccessLibrary(ctx, db, u, it.LibraryID) + if err != nil { + return Item{}, fmt.Errorf("catalog: check access: %w", err) + } + if !allowed { + // Deliberately the same error as a missing row: see ErrNotFound. + return Item{}, ErrNotFound + } + return it, nil +} + +// Files returns an item's files in order, with codec facts. +func Files(ctx context.Context, db *sql.DB, itemID int64) ([]File, error) { + rows, err := db.QueryContext(ctx, ` + SELECT id, path, size, container, missing, + duration_ms, vcodec, acodec, width, height + FROM media_files WHERE item_id = ? ORDER BY id`, itemID) + if err != nil { + return nil, fmt.Errorf("catalog: list files: %w", err) + } + defer rows.Close() + + out := []File{} + for rows.Next() { + var f File + if err := rows.Scan(&f.ID, &f.Path, &f.Size, &f.Container, &f.Missing, + &f.DurationMS, &f.VCodec, &f.ACodec, &f.Width, &f.Height); err != nil { + return nil, fmt.Errorf("catalog: scan file: %w", err) + } + out = append(out, f) + } + return out, rows.Err() +} + +// FilesFor returns an item's files after confirming access. Callers that +// already hold an authorized Item should use Files to avoid a second check. +func FilesFor(ctx context.Context, db *sql.DB, u auth.User, itemID int64) ([]File, error) { + if _, err := LoadFor(ctx, db, u, itemID); err != nil { + return nil, err + } + return Files(ctx, db, itemID) +} + +// FileByID returns one file of an item, scoped to that item so a file id from +// another item cannot be fetched through it. +func FileByID(ctx context.Context, db *sql.DB, itemID, fileID int64) (File, error) { + var f File + err := db.QueryRowContext(ctx, ` + SELECT id, path, size, container, missing, + duration_ms, vcodec, acodec, width, height + FROM media_files WHERE item_id = ? AND id = ?`, itemID, fileID).Scan( + &f.ID, &f.Path, &f.Size, &f.Container, &f.Missing, + &f.DurationMS, &f.VCodec, &f.ACodec, &f.Width, &f.Height) + if errors.Is(err, sql.ErrNoRows) { + return File{}, ErrNotFound + } + if err != nil { + return File{}, fmt.Errorf("catalog: load file: %w", err) + } + return f, nil +} + +// FirstFile returns the item's first file, which is what single-file playback +// and cover extraction both want. +func FirstFile(ctx context.Context, db *sql.DB, itemID int64) (File, error) { + var f File + err := db.QueryRowContext(ctx, ` + SELECT id, path, size, container, missing, + duration_ms, vcodec, acodec, width, height + FROM media_files WHERE item_id = ? ORDER BY id LIMIT 1`, itemID).Scan( + &f.ID, &f.Path, &f.Size, &f.Container, &f.Missing, + &f.DurationMS, &f.VCodec, &f.ACodec, &f.Width, &f.Height) + if errors.Is(err, sql.ErrNoRows) { + return File{}, ErrNotFound + } + if err != nil { + return File{}, fmt.Errorf("catalog: first file: %w", err) + } + return f, nil +} + +// FileCount reports how many files back an item. +func FileCount(ctx context.Context, db *sql.DB, itemID int64) (int, error) { + var n int + err := db.QueryRowContext(ctx, + `SELECT count(*) FROM media_files WHERE item_id = ?`, itemID).Scan(&n) + if err != nil { + return 0, fmt.Errorf("catalog: file count: %w", err) + } + return n, nil +} + +// parseUnixNano parses a base-10 nanosecond timestamp, returning 0 on anything +// that is not all digits. +func parseUnixNano(s string) int64 { + if s == "" { + return 0 + } + var n int64 + for i := 0; i < len(s); i++ { + if s[i] < '0' || s[i] > '9' { + return 0 + } + n = n*10 + int64(s[i]-'0') + } + return n +} diff --git a/internal/catalog/catalog_test.go b/internal/catalog/catalog_test.go new file mode 100644 index 0000000..c05350a --- /dev/null +++ b/internal/catalog/catalog_test.go @@ -0,0 +1,221 @@ +package catalog + +import ( + "context" + "database/sql" + "errors" + "io" + "log/slog" + "path/filepath" + "testing" + + "github.com/mithral/vellum/internal/auth" + "github.com/mithral/vellum/internal/core" + "github.com/mithral/vellum/migrations" +) + +// testDB builds a migrated database in a temp directory. +func testDB(t *testing.T) *sql.DB { + t.Helper() + db, err := core.OpenDB(filepath.Join(t.TempDir(), "catalog.db")) + if err != nil { + t.Fatalf("open: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + if err := core.Migrate(context.Background(), db, migrations.FS, log); err != nil { + t.Fatalf("migrate: %v", err) + } + return db +} + +// newLibrary inserts a library and returns its id. +func newLibrary(t *testing.T, db *sql.DB, name string) int64 { + t.Helper() + res, err := db.Exec( + `INSERT INTO libraries (kind, name, root_path) VALUES ('ebooks', ?, ?)`, + name, "/media/"+name) + if err != nil { + t.Fatalf("insert library: %v", err) + } + id, _ := res.LastInsertId() + return id +} + +// newUser inserts a user. isAdmin decides whether grants matter. +func newUser(t *testing.T, db *sql.DB, name string, admin bool) auth.User { + t.Helper() + adminFlag := 0 + if admin { + adminFlag = 1 + } + res, err := db.Exec( + `INSERT INTO users (username, display_name, password_hash, is_admin) VALUES (?, ?, 'x', ?)`, + name, name, adminFlag) + if err != nil { + t.Fatalf("insert user: %v", err) + } + id, _ := res.LastInsertId() + return auth.User{ID: id, Username: name, IsAdmin: admin} +} + +// newItem inserts an item with one file, including codec facts. +func newItem(t *testing.T, db *sql.DB, libID int64, title string) int64 { + t.Helper() + res, err := db.Exec(` + INSERT INTO media_items (library_id, kind, path, title, sort_title, year) + VALUES (?, 'ebooks', ?, ?, ?, 1999)`, libID, title+".epub", title, title) + if err != nil { + t.Fatalf("insert item: %v", err) + } + itemID, _ := res.LastInsertId() + if _, err := db.Exec(` + INSERT INTO media_files (item_id, path, size, mtime, container, duration_ms, + vcodec, acodec, width, height) + VALUES (?, ?, 100, '1700000000000000000', 'mp4', 5000, 'h264', 'aac', 640, 360)`, + itemID, title+".epub"); err != nil { + t.Fatalf("insert file: %v", err) + } + return itemID +} + +func grant(t *testing.T, db *sql.DB, u auth.User, libID int64) { + t.Helper() + if _, err := db.Exec( + `INSERT INTO library_grants (user_id, library_id, can_read) VALUES (?, ?, 1)`, + u.ID, libID); err != nil { + t.Fatalf("grant: %v", err) + } +} + +func TestLoadReturnsCodecFacts(t *testing.T) { + db := testDB(t) + lib := newLibrary(t, db, "A") + itemID := newItem(t, db, lib, "Thing") + + it, err := LoadUnchecked(context.Background(), db, itemID) + if err != nil { + t.Fatal(err) + } + if it.Title != "Thing" || it.LibraryID != lib || it.Kind != "ebooks" { + t.Errorf("unexpected item: %+v", it) + } + if it.RootPath != "/media/A" { + t.Errorf("RootPath = %q", it.RootPath) + } + if it.AbsPath() != "/media/A/Thing.epub" { + t.Errorf("AbsPath = %q", it.AbsPath()) + } + if it.ModTime.IsZero() { + t.Error("ModTime not parsed from the file's mtime") + } + + files, err := Files(context.Background(), db, itemID) + if err != nil { + t.Fatal(err) + } + if len(files) != 1 { + t.Fatalf("files = %d, want 1", len(files)) + } + f := files[0] + if f.VCodec != "h264" || f.ACodec != "aac" || f.Width != 640 || f.DurationMS != 5000 { + t.Errorf("codec facts lost: %+v", f) + } + if f.Missing { + t.Error("a present file reported as missing") + } +} + +// The security property: a missing item and a forbidden item must be +// INDISTINGUISHABLE, or a 403 would leak the existence of another user's +// library. +func TestLoadForForbiddenIsIndistinguishableFromAbsent(t *testing.T) { + db := testDB(t) + ctx := context.Background() + lib := newLibrary(t, db, "Secret") + itemID := newItem(t, db, lib, "Hidden") + + outsider := newUser(t, db, "outsider", false) + granted := newUser(t, db, "granted", false) + grant(t, db, granted, lib) + + // Forbidden: the item exists but this user has no grant. + _, errForbidden := LoadFor(ctx, db, outsider, itemID) + // Absent: no such row at all. + _, errAbsent := LoadFor(ctx, db, outsider, itemID+9999) + + if !errors.Is(errForbidden, ErrNotFound) { + t.Errorf("forbidden error = %v, want ErrNotFound", errForbidden) + } + if !errors.Is(errAbsent, ErrNotFound) { + t.Errorf("absent error = %v, want ErrNotFound", errAbsent) + } + if errForbidden.Error() != errAbsent.Error() { + t.Errorf("errors differ and would leak existence: %q vs %q", + errForbidden, errAbsent) + } + + // Positive control: a granted user does get it. + if _, err := LoadFor(ctx, db, granted, itemID); err != nil { + t.Errorf("granted user denied: %v", err) + } +} + +func TestAdminBypassesGrants(t *testing.T) { + db := testDB(t) + ctx := context.Background() + lib := newLibrary(t, db, "Any") + itemID := newItem(t, db, lib, "Thing") + + admin := newUser(t, db, "root", true) + if _, err := LoadFor(ctx, db, admin, itemID); err != nil { + t.Errorf("admin denied: %v", err) + } +} + +func TestFileByIDIsScopedToItsItem(t *testing.T) { + db := testDB(t) + ctx := context.Background() + lib := newLibrary(t, db, "A") + first := newItem(t, db, lib, "One") + second := newItem(t, db, lib, "Two") + + other, err := FirstFile(ctx, db, second) + if err != nil { + t.Fatal(err) + } + + // Asking for item Two's file through item One must not resolve. + if _, err := FileByID(ctx, db, first, other.ID); !errors.Is(err, ErrNotFound) { + t.Errorf("file id leaked across items: err = %v, want ErrNotFound", err) + } + if _, err := FileByID(ctx, db, second, other.ID); err != nil { + t.Errorf("legitimate lookup failed: %v", err) + } +} + +func TestFilesForRespectsGrants(t *testing.T) { + db := testDB(t) + ctx := context.Background() + lib := newLibrary(t, db, "A") + itemID := newItem(t, db, lib, "Thing") + + outsider := newUser(t, db, "outsider", false) + if _, err := FilesFor(ctx, db, outsider, itemID); !errors.Is(err, ErrNotFound) { + t.Errorf("FilesFor leaked to an ungranted user: %v", err) + } +} + +func TestFileCount(t *testing.T) { + db := testDB(t) + lib := newLibrary(t, db, "A") + itemID := newItem(t, db, lib, "Thing") + + n, err := FileCount(context.Background(), db, itemID) + if err != nil { + t.Fatal(err) + } + if n != 1 { + t.Errorf("FileCount = %d, want 1", n) + } +} diff --git a/internal/jellyfin/dto.go b/internal/jellyfin/dto.go index efc6e09..7f64b6b 100644 --- a/internal/jellyfin/dto.go +++ b/internal/jellyfin/dto.go @@ -1,6 +1,10 @@ package jellyfin -import "database/sql" +import ( + "database/sql" + + "github.com/mithral/vellum/internal/catalog" +) // This file defines a documented SUBSET of the Jellyfin DTOs. Jellyfin's // BaseItemDto has roughly a hundred fields; real clients tolerate the absence @@ -115,33 +119,33 @@ type itemQueryResult struct { StartIndex int `json:"StartIndex"` } -// dbItem is one media_items row plus the file facts and per-user state a DTO -// needs. It is the single shape both the list and single-item paths build. -type dbItem struct { - ID int64 - LibraryID int64 - Kind string - Title string - SortTitle string - Year int - ParentID sql.NullInt64 - Container string - DurationMS int64 - VCodec string - ACodec string - Width int - Height int - Size int64 - Path string - HasCover bool +// itemView is one browsable item assembled for a Jellyfin DTO. +// +// It composes the two types internal/catalog owns — the item row and its first +// file — with the per-user overlay (progress, view count, favourite) and the +// two browse-only columns catalog.Item does not carry (sort title and parent). +// Nothing here reads the database or checks a grant: catalog.LoadFor loads the +// item and enforces the grant, and catalog.FirstFile reads the file, so there +// is exactly one implementation of "may this user read this item" and exactly +// one place item and file rows are read. +type itemView struct { + Item catalog.Item + File catalog.File + + // SortTitle and ParentID are browse-only columns; the DTO maps them to + // SortName and ParentId. + SortTitle string + ParentID sql.NullInt64 + + // HasCover is true when the scanner extracted a primary image. + HasCover bool // Per-user state, zero when the user has never touched the item. - PositionMS int64 - Percent float64 - Finished bool - HasProgress bool - PlayCount int - Favorite bool + PositionMS int64 + Percent float64 + Finished bool + PlayCount int + Favorite bool } // itemType maps a Vellum media kind to the Jellyfin item type. The four types @@ -199,52 +203,54 @@ func kindsForTypes(types []string) []string { return out } -// toBaseItemDto renders one dbItem. includeMediaSources is set from the Fields -// query parameter; lists normally omit sources to keep responses small. -func toBaseItemDto(it dbItem, includeMediaSources bool, apiKey string) BaseItemDto { +// toBaseItemDto renders one itemView. includeMediaSources is set from the +// Fields query parameter; lists normally omit sources to keep responses small. +func toBaseItemDto(v itemView, includeMediaSources bool, apiKey string) BaseItemDto { dto := BaseItemDto{ - Id: itemID(it.ID), - Name: it.Title, - Type: itemType(it.Kind), - MediaType: mediaType(it.Kind), - Container: it.Container, - RunTimeTicks: msToTicks(it.DurationMS), - ProductionYear: it.Year, + Id: itemID(v.Item.ID), + Name: v.Item.Title, + Type: itemType(v.Item.Kind), + MediaType: mediaType(v.Item.Kind), + Container: v.File.Container, + RunTimeTicks: msToTicks(v.File.DurationMS), + ProductionYear: v.Item.Year, IsFolder: false, - SortName: it.SortTitle, - Path: it.Path, + SortName: v.SortTitle, + Path: v.File.Path, } - if it.ParentID.Valid { - dto.ParentId = itemID(it.ParentID.Int64) + if v.ParentID.Valid { + dto.ParentId = itemID(v.ParentID.Int64) } - if it.HasCover { + if v.HasCover { // The tag is the item id: stable, and it changes when the item changes. - dto.ImageTags = map[string]string{"Primary": itemID(it.ID)} + dto.ImageTags = map[string]string{"Primary": itemID(v.Item.ID)} } dto.UserData = &UserItemDataDto{ - PlaybackPositionTicks: msToTicks(it.PositionMS), - PlayedPercentage: it.Percent * 100, - PlayCount: it.PlayCount, - IsFavorite: it.Favorite, - Played: it.Finished, + PlaybackPositionTicks: msToTicks(v.PositionMS), + PlayedPercentage: v.Percent * 100, + PlayCount: v.PlayCount, + IsFavorite: v.Favorite, + Played: v.Finished, } if includeMediaSources { - dto.MediaSources = []MediaSourceInfo{buildMediaSource(it, apiKey)} + dto.MediaSources = []MediaSourceInfo{buildMediaSource(v.Item, v.File, apiKey)} } return dto } // buildMediaSource renders the single direct-play source for an item. It is // shared by PlaybackInfo and by list responses that requested the -// MediaSources field. -func buildMediaSource(it dbItem, apiKey string) MediaSourceInfo { +// MediaSources field. Both inputs come from internal/catalog; a zero File is +// an item with no file, and produces the same defaults a missing file always +// did. +func buildMediaSource(it catalog.Item, f catalog.File, apiKey string) MediaSourceInfo { ms := MediaSourceInfo{ Id: itemID(it.ID), Name: it.Title, - Path: it.Path, - Container: it.Container, - RunTimeTicks: msToTicks(it.DurationMS), - Size: it.Size, + Path: f.Path, + Container: f.Container, + RunTimeTicks: msToTicks(f.DurationMS), + Size: f.Size, SupportsDirectPlay: true, SupportsDirectStream: true, // The hard constraint: no transcoding, ever. @@ -255,15 +261,15 @@ func buildMediaSource(it dbItem, apiKey string) MediaSourceInfo { } ms.DirectStreamUrl = directStreamURL(it, apiKey) - if it.VCodec != "" { + if f.VCodec != "" { ms.MediaStreams = append(ms.MediaStreams, MediaStream{ - Type: "Video", Codec: it.VCodec, Index: 0, IsDefault: true, - Width: it.Width, Height: it.Height, + Type: "Video", Codec: f.VCodec, Index: 0, IsDefault: true, + Width: f.Width, Height: f.Height, }) } - if it.ACodec != "" { + if f.ACodec != "" { ms.MediaStreams = append(ms.MediaStreams, MediaStream{ - Type: "Audio", Codec: it.ACodec, Index: 1, IsDefault: true, + Type: "Audio", Codec: f.ACodec, Index: 1, IsDefault: true, }) } return ms @@ -273,7 +279,7 @@ func buildMediaSource(it dbItem, apiKey string) MediaSourceInfo { // /Videos, everything else (audio, books, photos) from /Audio, which is the // same file-serving path either way. The api_key is embedded because media // elements cannot set headers. -func directStreamURL(it dbItem, apiKey string) string { +func directStreamURL(it catalog.Item, apiKey string) string { base := "/Audio/" + itemID(it.ID) + "/stream" if it.Kind == "video" { base = "/Videos/" + itemID(it.ID) + "/stream" diff --git a/internal/jellyfin/items.go b/internal/jellyfin/items.go index fa5d99f..f7867c8 100644 --- a/internal/jellyfin/items.go +++ b/internal/jellyfin/items.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/mithral/vellum/internal/auth" + "github.com/mithral/vellum/internal/catalog" "github.com/mithral/vellum/internal/httpx" "github.com/mithral/vellum/internal/library" "github.com/mithral/vellum/internal/stream" @@ -159,7 +160,20 @@ func (h *Handlers) handleItem(w http.ResponseWriter, r *http.Request, u auth.Use if !ok { return } - httpx.WriteJSON(w, http.StatusOK, toBaseItemDto(it, true, tokenFromRequest(r))) + v := itemView{Item: it} + if err := h.attachFile(r.Context(), &v); err != nil { + h.fail(w, "load file", err) + return + } + if err := h.loadOverlay(r.Context(), u.ID, it.ID, &v); err != nil { + if errors.Is(err, sql.ErrNoRows) { + httpx.WriteError(w, http.StatusNotFound, "item not found") + return + } + h.fail(w, "load item state", err) + return + } + httpx.WriteJSON(w, http.StatusOK, toBaseItemDto(v, true, tokenFromRequest(r))) } // handleImagePrimary streams the cover bytes the scanner/web already extracted. @@ -197,25 +211,18 @@ func (h *Handlers) handleImagePrimary(w http.ResponseWriter, r *http.Request, u } // authorizedItem loads an item and confirms access, writing a 404 (never a 403, -// so existence is not disclosed) on failure. -func (h *Handlers) authorizedItem(w http.ResponseWriter, r *http.Request, u auth.User, id int64) (dbItem, bool) { - it, err := h.loadDBItem(r.Context(), u.ID, id) - if errors.Is(err, sql.ErrNoRows) { +// so existence is not disclosed) on failure. Both the load and the grant check +// live in catalog.LoadFor, which returns the same ErrNotFound for a missing +// item and an unreadable one. +func (h *Handlers) authorizedItem(w http.ResponseWriter, r *http.Request, u auth.User, id int64) (catalog.Item, bool) { + it, err := catalog.LoadFor(r.Context(), h.db, u, id) + if errors.Is(err, catalog.ErrNotFound) { httpx.WriteError(w, http.StatusNotFound, "item not found") - return dbItem{}, false + return catalog.Item{}, false } if err != nil { h.fail(w, "load item", err) - return dbItem{}, false - } - allowed, err := auth.CanAccessLibrary(r.Context(), h.db, u, it.LibraryID) - if err != nil { - h.fail(w, "check access", err) - return dbItem{}, false - } - if !allowed { - httpx.WriteError(w, http.StatusNotFound, "item not found") - return dbItem{}, false + return catalog.Item{}, false } return it, true } diff --git a/internal/jellyfin/playback.go b/internal/jellyfin/playback.go index 7dce81c..b65e72e 100644 --- a/internal/jellyfin/playback.go +++ b/internal/jellyfin/playback.go @@ -1,7 +1,6 @@ package jellyfin import ( - "database/sql" "errors" "net/http" "os" @@ -10,6 +9,7 @@ import ( "strings" "github.com/mithral/vellum/internal/auth" + "github.com/mithral/vellum/internal/catalog" "github.com/mithral/vellum/internal/httpx" "github.com/mithral/vellum/internal/stream" ) @@ -31,12 +31,17 @@ func (h *Handlers) handlePlaybackInfo(w http.ResponseWriter, r *http.Request, u if !ok { return } + f, err := h.firstFile(r.Context(), id) + if err != nil { + h.fail(w, "load file facts", err) + return + } q := r.URL.Query() directPlay := queryBoolDefault(q.Get("EnableDirectPlay"), true) directStream := queryBoolDefault(q.Get("EnableDirectStream"), true) - src := buildMediaSource(it, tokenFromRequest(r)) + src := buildMediaSource(it, f, tokenFromRequest(r)) resp := PlaybackInfoResponse{MediaSources: []MediaSourceInfo{src}} if !directPlay && !directStream { resp.ErrorCode = "NoCompatibleStream" @@ -75,19 +80,13 @@ func (h *Handlers) serveStream(w http.ResponseWriter, r *http.Request, u auth.Us httpx.WriteError(w, http.StatusNotFound, "item not found") return } - if _, ok := h.authorizedItem(w, r, u, id); !ok { + it, ok := h.authorizedItem(w, r, u, id) + if !ok { return } - var root, rel string - err := h.db.QueryRowContext(r.Context(), ` - SELECT l.root_path, f.path - FROM media_items i - JOIN libraries l ON l.id = i.library_id - JOIN media_files f ON f.item_id = i.id - WHERE i.id = ? - ORDER BY f.id LIMIT 1`, id).Scan(&root, &rel) - if errors.Is(err, sql.ErrNoRows) { + f, err := catalog.FirstFile(r.Context(), h.db, id) + if errors.Is(err, catalog.ErrNotFound) { httpx.WriteError(w, http.StatusNotFound, "item has no file") return } @@ -97,7 +96,8 @@ func (h *Handlers) serveStream(w http.ResponseWriter, r *http.Request, u auth.Us } w.Header().Set("Content-Disposition", "inline") - if err := stream.ServeFile(w, r, filepath.Join(root, filepath.FromSlash(rel))); err != nil { + abs := filepath.Join(it.RootPath, filepath.FromSlash(f.Path)) + if err := stream.ServeFile(w, r, abs); err != nil { if errors.Is(err, os.ErrNotExist) { httpx.WriteError(w, http.StatusNotFound, "file is not available") return diff --git a/internal/jellyfin/query.go b/internal/jellyfin/query.go index f3c9857..705470d 100644 --- a/internal/jellyfin/query.go +++ b/internal/jellyfin/query.go @@ -3,11 +3,13 @@ package jellyfin import ( "context" "database/sql" + "errors" "sort" "strconv" "strings" "github.com/mithral/vellum/internal/auth" + "github.com/mithral/vellum/internal/catalog" ) // defaultLimit caps an unbounded browse so a client that omits Limit cannot @@ -17,29 +19,33 @@ const ( maxLimit = 1000 ) -// itemColumns / itemFrom are shared by the list and single-item queries so the -// two paths can never disagree about what a dbItem contains. +// overlayColumns / overlayFrom carry the per-user state and the two browse-only +// columns (sort title, parent) that catalog.Item does not expose. They never +// read media_files and never check a grant: catalog.LoadFor loads the item and +// enforces the grant, and catalog.FirstFile reads the file, so the one +// authorization rule lives in one place instead of a sixth copy here. // -// File facts come from the item's first media_files row (multi-file items such -// as audiobook chapters are still one browsable item). Per-user state comes -// from a windowed progress join that picks the most recently updated row per -// item, plus item_views and favorites. -const itemColumns = `i.id, i.library_id, i.kind, i.title, i.sort_title, COALESCE(i.year,0), - i.parent_id, - COALESCE(f.container,''), COALESCE(f.duration_ms,0), COALESCE(f.vcodec,''), COALESCE(f.acodec,''), - COALESCE(f.width,0), COALESCE(f.height,0), COALESCE(f.size,0), COALESCE(f.path,''), - p.item_id, COALESCE(p.position,0), COALESCE(p.percent,0), COALESCE(p.finished,0), +// Per-user state comes from a windowed progress join that picks the most +// recently updated row per item, plus item_views and favorites. +const overlayColumns = `i.sort_title, i.parent_id, + COALESCE(p.position,0), COALESCE(p.percent,0), COALESCE(p.finished,0), COALESCE(v.views,0), COALESCE(fa.user_id,0), COALESCE((SELECT 1 FROM assets a WHERE a.item_id=i.id AND a.role='cover' LIMIT 1),0)` -const itemFrom = `FROM media_items i - LEFT JOIN media_files f ON f.id = (SELECT id FROM media_files WHERE item_id=i.id ORDER BY id LIMIT 1) +const overlayFrom = `FROM media_items i LEFT JOIN (SELECT item_id, position, percent, finished, ROW_NUMBER() OVER (PARTITION BY item_id ORDER BY updated_at DESC) AS rn FROM progress WHERE user_id = ?) p ON p.item_id = i.id AND p.rn = 1 LEFT JOIN item_views v ON v.user_id = ? AND v.item_id = i.id LEFT JOIN favorites fa ON fa.user_id = ? AND fa.item_id = i.id` +// itemColumns is the browse projection: the catalog.Item identity columns plus +// the per-user overlay. The list and single-item paths share it so the two can +// never disagree about what a browse row contains. File facts are attached +// separately via catalog.FirstFile. +const itemColumns = `i.id, i.library_id, i.kind, i.path, i.title, COALESCE(i.year,0), i.missing, ` + + overlayColumns + // listQuery is everything the browse query needs. type listQuery struct { userID int64 @@ -56,7 +62,11 @@ type listQuery struct { } // queryItems runs the browse query and its matching count. -func (h *Handlers) queryItems(ctx context.Context, q listQuery) ([]dbItem, int, error) { +// +// The browse rows are drained before the per-item file lookups run: catalog +// reads one file at a time, and issuing those queries against a still-open +// result set is the classic nested-query footgun. +func (h *Handlers) queryItems(ctx context.Context, q listQuery) ([]itemView, int, error) { where, whereArgs := buildWhere(q) var total int @@ -71,7 +81,7 @@ func (h *Handlers) queryItems(ctx context.Context, q listQuery) ([]dbItem, int, } else { order += " ASC" } - sqlText := `SELECT ` + itemColumns + ` ` + itemFrom + + sqlText := `SELECT ` + itemColumns + ` ` + overlayFrom + ` WHERE ` + where + ` ORDER BY ` + order + ` LIMIT ? OFFSET ?` args := []any{q.userID, q.userID, q.userID} @@ -84,67 +94,115 @@ func (h *Handlers) queryItems(ctx context.Context, q listQuery) ([]dbItem, int, } defer rows.Close() - out := []dbItem{} + out := []itemView{} for rows.Next() { - it, err := scanItem(rows) + v, err := scanItem(rows) if err != nil { return nil, 0, err } - out = append(out, it) + out = append(out, v) } - return out, total, rows.Err() + if err := rows.Err(); err != nil { + return nil, 0, err + } + rows.Close() + + for i := range out { + if err := h.attachFile(ctx, &out[i]); err != nil { + return nil, 0, err + } + } + return out, total, nil } -// loadDBItem loads one item with the same columns as the browse query. -func (h *Handlers) loadDBItem(ctx context.Context, userID, id int64) (dbItem, error) { - row := h.db.QueryRowContext(ctx, - `SELECT `+itemColumns+` `+itemFrom+` WHERE i.id = ?`, - userID, userID, userID, id) - return scanItem(row) -} - -// itemVisible reports whether the caller may read the item's library. A -// missing item is false, not an error. -func (h *Handlers) itemVisible(ctx context.Context, u auth.User, id int64) (bool, error) { - var libID int64 - err := h.db.QueryRowContext(ctx, - `SELECT library_id FROM media_items WHERE id = ?`, id).Scan(&libID) - if err == sql.ErrNoRows { - return false, nil - } +// attachFile fills the first-file facts from internal/catalog, the one place +// that reads media_files. An item with no file keeps a zero File, which is +// exactly the defaults the old media_files LEFT JOIN produced. +func (h *Handlers) attachFile(ctx context.Context, v *itemView) error { + f, err := h.firstFile(ctx, v.Item.ID) if err != nil { + return err + } + v.File = f + return nil +} + +// firstFile returns the item's first file, or a zero File when the item has +// none. It is catalog.FirstFile with the "no file" case flattened, for callers +// that only need the facts rather than a 404. +func (h *Handlers) firstFile(ctx context.Context, itemID int64) (catalog.File, error) { + f, err := catalog.FirstFile(ctx, h.db, itemID) + if errors.Is(err, catalog.ErrNotFound) { + return catalog.File{}, nil + } + return f, err +} + +// loadOverlay fills the browse-only and per-user fields of an already-loaded +// item. It reads no file and performs no access check: catalog.LoadFor already +// did both. +func (h *Handlers) loadOverlay(ctx context.Context, userID, itemID int64, v *itemView) error { + row := h.db.QueryRowContext(ctx, + `SELECT `+overlayColumns+` `+overlayFrom+` WHERE i.id = ?`, + userID, userID, userID, itemID) + var ( + parentID sql.NullInt64 + finished int + favUser int64 + hasCover int + ) + if err := row.Scan( + &v.SortTitle, &parentID, + &v.PositionMS, &v.Percent, &finished, + &v.PlayCount, &favUser, &hasCover, + ); err != nil { + return err + } + v.ParentID = parentID + v.Finished = finished == 1 + v.Favorite = favUser != 0 + v.HasCover = hasCover == 1 + return nil +} + +// itemVisible reports whether the caller may read the item's library. A missing +// item is false, not an error. It delegates to catalog.LoadFor, the same +// chokepoint every other surface uses, rather than reimplementing the grant +// rule here. +func (h *Handlers) itemVisible(ctx context.Context, u auth.User, id int64) (bool, error) { + if _, err := catalog.LoadFor(ctx, h.db, u, id); err != nil { + if errors.Is(err, catalog.ErrNotFound) { + return false, nil + } return false, err } - return auth.CanAccessLibrary(ctx, h.db, u, libID) + return true, nil } // scanItem reads one row of the shared item column list. -func scanItem(row interface{ Scan(...any) error }) (dbItem, error) { +func scanItem(row interface{ Scan(...any) error }) (itemView, error) { var ( - it dbItem - parentID sql.NullInt64 - hasProgress sql.NullInt64 - finished int - favUser int64 - hasCover int + v itemView + parentID sql.NullInt64 + finished int + favUser int64 + hasCover int ) err := row.Scan( - &it.ID, &it.LibraryID, &it.Kind, &it.Title, &it.SortTitle, &it.Year, - &parentID, - &it.Container, &it.DurationMS, &it.VCodec, &it.ACodec, - &it.Width, &it.Height, &it.Size, &it.Path, - &hasProgress, &it.PositionMS, &it.Percent, &finished, - &it.PlayCount, &favUser, &hasCover, + &v.Item.ID, &v.Item.LibraryID, &v.Item.Kind, &v.Item.Path, &v.Item.Title, + &v.Item.Year, &v.Item.Missing, + &v.SortTitle, &parentID, + &v.PositionMS, &v.Percent, &finished, + &v.PlayCount, &favUser, &hasCover, ) if err != nil { - return dbItem{}, err + return itemView{}, err } - it.ParentID = parentID - it.HasProgress = hasProgress.Valid - it.Finished = finished == 1 - it.Favorite = favUser != 0 - it.HasCover = hasCover == 1 - return it, nil + v.ParentID = parentID + v.Finished = finished == 1 + v.Favorite = favUser != 0 + v.HasCover = hasCover == 1 + return v, nil } // buildWhere assembles the WHERE clause and its arguments. An empty library set diff --git a/internal/jellyfin/sessions.go b/internal/jellyfin/sessions.go index 6bb0953..5267bf8 100644 --- a/internal/jellyfin/sessions.go +++ b/internal/jellyfin/sessions.go @@ -67,13 +67,12 @@ func (h *Handlers) writePlaybackProgress(w http.ResponseWriter, r *http.Request, // Duration comes from the item's first file, the same source the browse // DTOs use, so PlayedPercentage is consistent across endpoints. - var durationMS int64 - if err := h.db.QueryRowContext(r.Context(), - `SELECT COALESCE((SELECT duration_ms FROM media_files WHERE item_id = ? ORDER BY id LIMIT 1), 0)`, - id).Scan(&durationMS); err != nil { + f, err := h.firstFile(r.Context(), id) + if err != nil { h.fail(w, "load duration", err) return } + durationMS := f.DurationMS posMS := ticksToMS(rep.PositionTicks) if posMS < 0 { diff --git a/internal/media/handlers.go b/internal/media/handlers.go index 512c1e0..d2c22f2 100644 --- a/internal/media/handlers.go +++ b/internal/media/handlers.go @@ -10,6 +10,7 @@ import ( "strconv" "github.com/mithral/vellum/internal/auth" + "github.com/mithral/vellum/internal/catalog" "github.com/mithral/vellum/internal/httpx" "github.com/mithral/vellum/internal/progress" "github.com/mithral/vellum/internal/stream" @@ -158,10 +159,8 @@ func (h *Handlers) handleFileByID(w http.ResponseWriter, r *http.Request, u auth // Confirm the file belongs to this item, so an item id cannot be paired // with another item's file id. - var relPath string - err = h.db.QueryRowContext(r.Context(), - `SELECT path FROM media_files WHERE id = ? AND item_id = ?`, fileID, it.ID).Scan(&relPath) - if errors.Is(err, sql.ErrNoRows) { + f, err := catalog.FileByID(r.Context(), h.db, it.ID, fileID) + if errors.Is(err, catalog.ErrNotFound) { httpx.WriteError(w, http.StatusNotFound, "file not found") return } @@ -170,7 +169,7 @@ func (h *Handlers) handleFileByID(w http.ResponseWriter, r *http.Request, u auth return } - abs := filepath.Join(it.RootPath, filepath.FromSlash(relPath)) + abs := filepath.Join(it.RootPath, filepath.FromSlash(f.Path)) w.Header().Set("Content-Disposition", "inline") if err := stream.ServeFile(w, r, abs); err != nil { if errors.Is(err, os.ErrNotExist) { @@ -276,6 +275,9 @@ func (h *Handlers) handleRecent(w http.ResponseWriter, r *http.Request, u auth.U // authorized loads an item and confirms the caller may read its library. // On failure it has already written the response. +// +// The load and the access check both live in catalog.LoadFor, so this package +// no longer carries its own copy of the "may this user read this item" rule. func (h *Handlers) authorized(w http.ResponseWriter, r *http.Request, u auth.User) (Item, bool) { id, err := strconv.ParseInt(r.PathValue("id"), 10, 64) if err != nil || id <= 0 { @@ -283,8 +285,8 @@ func (h *Handlers) authorized(w http.ResponseWriter, r *http.Request, u auth.Use return Item{}, false } - it, err := LoadItem(r.Context(), h.db, id) - if errors.Is(err, ErrItemNotFound) { + it, err := catalog.LoadFor(r.Context(), h.db, u, id) + if errors.Is(err, catalog.ErrNotFound) { httpx.WriteError(w, http.StatusNotFound, "item not found") return Item{}, false } @@ -292,59 +294,16 @@ func (h *Handlers) authorized(w http.ResponseWriter, r *http.Request, u auth.Use h.fail(w, "load item", err) return Item{}, false } - - allowed, err := auth.CanAccessLibrary(r.Context(), h.db, u, it.LibraryID) - if err != nil { - h.fail(w, "check access", err) - return Item{}, false - } - if !allowed { - httpx.WriteError(w, http.StatusNotFound, "item not found") - return Item{}, false - } return it, true } -// fileDTO is one physical file in an item's detail response. It carries the -// ffprobe codec facts alongside the container, because direct play is a -// decision each client makes for itself: the server reports what is in the file -// and never transcodes. The wire shape is frozen by MediaFile in -// web/src/lib/api.ts. -type fileDTO struct { - ID int64 `json:"id"` - Path string `json:"path"` - Size int64 `json:"size"` - Container string `json:"container"` - Missing bool `json:"missing"` - DurationMS int64 `json:"duration_ms"` - VCodec string `json:"vcodec"` - ACodec string `json:"acodec"` - Width int `json:"width"` - Height int `json:"height"` -} - -func (h *Handlers) files(r *http.Request, itemID int64) ([]fileDTO, error) { - rows, err := h.db.QueryContext(r.Context(), ` - SELECT id, path, size, container, missing, - duration_ms, vcodec, acodec, width, height - FROM media_files WHERE item_id = ? ORDER BY id`, itemID) - if err != nil { - return nil, err - } - defer rows.Close() - - out := []fileDTO{} - for rows.Next() { - var f fileDTO - var missing int - if err := rows.Scan(&f.ID, &f.Path, &f.Size, &f.Container, &missing, - &f.DurationMS, &f.VCodec, &f.ACodec, &f.Width, &f.Height); err != nil { - return nil, err - } - f.Missing = missing == 1 - out = append(out, f) - } - return out, rows.Err() +// files returns an item's physical files with their ffprobe codec facts. +// +// The shape is catalog.File, whose JSON tags are frozen by MediaFile in +// web/src/lib/api.ts. The server reports what is IN the file and never +// transcodes; each client decides whether it can direct-play it. +func (h *Handlers) files(r *http.Request, itemID int64) ([]catalog.File, error) { + return catalog.Files(r.Context(), h.db, itemID) } func (h *Handlers) hasCover(r *http.Request, it Item) bool { diff --git a/internal/media/media.go b/internal/media/media.go index 0358bb7..fda97f0 100644 --- a/internal/media/media.go +++ b/internal/media/media.go @@ -1,97 +1,34 @@ // Package media serves generic item endpoints: detail, the raw file, cover // art, and progress. It knows nothing about any specific media format; modules // register cover extractors and expose their own metadata endpoints. +// +// Item and file ACCESS does not live here any more — it lives in +// internal/catalog, so that the web app, the modules, Jellyfin and Subsonic all +// share one implementation of "may this user read this item" instead of five. +// The names below are kept as aliases so existing callers do not have to move +// in the same step; new code should import catalog directly. package media import ( "context" "database/sql" - "errors" - "fmt" - "path/filepath" - "time" + + "github.com/mithral/vellum/internal/catalog" ) -// ErrItemNotFound means no item matched. -var ErrItemNotFound = errors.New("media: item not found") +// Item is one media item plus the root of its library. See catalog.Item. +type Item = catalog.Item -// Item is one media item plus the root of its library. -type Item struct { - ID int64 - LibraryID int64 - Kind string - Path string // path relative to the library root - Title string - Year int - Missing bool +// FileEntry is one physical file backing an item, with its codec facts. +// See catalog.File. +type FileEntry = catalog.File - // RootPath is the absolute library root, so AbsPath can be computed. - RootPath string - - // ModTime is the backing file's modification time, used for conditional - // GETs. Zero when unknown. - ModTime time.Time -} - -// AbsPath returns the absolute filesystem path of the item's file. -// -// item.path is stored relative to the library root and the root is stored -// resolved, so this join cannot escape the library. -func (i Item) AbsPath() string { - return filepath.Join(i.RootPath, filepath.FromSlash(i.Path)) -} - -// LoadItem loads an item and its library root. -func LoadItem(ctx context.Context, db *sql.DB, id int64) (Item, error) { - var it Item - var mtime string - err := db.QueryRowContext(ctx, ` - SELECT i.id, i.library_id, i.kind, i.path, i.title, COALESCE(i.year, 0), i.missing, l.root_path, - COALESCE((SELECT f.mtime FROM media_files f WHERE f.item_id = i.id ORDER BY f.id LIMIT 1), '') - FROM media_items i - JOIN libraries l ON l.id = i.library_id - WHERE i.id = ?`, id, - ).Scan(&it.ID, &it.LibraryID, &it.Kind, &it.Path, &it.Title, &it.Year, &it.Missing, &it.RootPath, &mtime) - if errors.Is(err, sql.ErrNoRows) { - return Item{}, ErrItemNotFound - } - if err != nil { - return Item{}, fmt.Errorf("media: load item: %w", err) - } - if nanos := parseUnixNano(mtime); nanos > 0 { - it.ModTime = time.Unix(0, nanos).UTC() - } - return it, nil -} - -// parseUnixNano parses a base-10 nanosecond timestamp, returning 0 on anything -// that is not all digits. -func parseUnixNano(s string) int64 { - if s == "" { - return 0 - } - var n int64 - for i := 0; i < len(s); i++ { - if s[i] < '0' || s[i] > '9' { - return 0 - } - n = n*10 + int64(s[i]-'0') - } - return n -} - -// FileEntry is one physical file backing an item. -type FileEntry struct { - ID int64 `json:"id"` - Path string `json:"path"` - Size int64 `json:"size"` - Container string `json:"container"` - Missing bool `json:"missing"` -} +// ErrItemNotFound means no item matched, or the caller may not read it. The two +// are deliberately indistinguishable so a handler cannot leak the existence of +// another user's library. See catalog.ErrNotFound. +var ErrItemNotFound = catalog.ErrNotFound // FileCount returns how many files back an item. func FileCount(ctx context.Context, db *sql.DB, itemID int64) (int, error) { - var n int - err := db.QueryRowContext(ctx, `SELECT count(*) FROM media_files WHERE item_id = ?`, itemID).Scan(&n) - return n, err + return catalog.FileCount(ctx, db, itemID) } diff --git a/internal/metadata/enrich.go b/internal/metadata/enrich.go index 9a314d1..6b65754 100644 --- a/internal/metadata/enrich.go +++ b/internal/metadata/enrich.go @@ -15,6 +15,7 @@ import ( "strings" "time" + "github.com/mithral/vellum/internal/catalog" "github.com/mithral/vellum/internal/core" "github.com/mithral/vellum/internal/library" "github.com/mithral/vellum/internal/media" @@ -76,9 +77,14 @@ func (e *Enricher) Handle(ctx context.Context, payload json.RawMessage) error { // returns an error only for a database failure, because a missing match is a // normal outcome and must not fail the job. func (e *Enricher) EnrichItem(ctx context.Context, itemID int64) error { - it, err := media.LoadItem(ctx, e.db, itemID) + // This runs as a background job, not a request, so there is no user and no + // grant to check. catalog.Load is the unchecked read, which is correct + // here: the job acts on an item the scanner already admitted to the + // catalogue, and a job that silently skipped its work because of a grant + // check it cannot perform would be a bug. + it, err := catalog.LoadUnchecked(ctx, e.db, itemID) if err != nil { - if errors.Is(err, media.ErrItemNotFound) { + if errors.Is(err, catalog.ErrNotFound) { return nil // deleted since the job was queued } return err @@ -150,7 +156,7 @@ func (e *Enricher) EnrichItem(ctx context.Context, itemID int64) error { // so the parent directory is a strong signal that the provider search should // use to disambiguate. Without it, "Random Access Memories" matches a release // by an unrelated artist. -func artistHint(it media.Item) string { +func artistHint(it catalog.Item) string { if library.GroupModeFor(it.Kind) != library.GroupByDirectory { return "" } @@ -237,7 +243,7 @@ func normalizeTitle(s string) string { } // apply writes a chosen candidate back to the database. -func (e *Enricher) apply(ctx context.Context, it media.Item, c Candidate) error { +func (e *Enricher) apply(ctx context.Context, it catalog.Item, c Candidate) error { tx, err := e.db.BeginTx(ctx, nil) if err != nil { return err @@ -301,7 +307,7 @@ func (e *Enricher) apply(ctx context.Context, it media.Item, c Candidate) error } // downloadCover fetches a cover into the cache and records it as an asset. -func (e *Enricher) downloadCover(ctx context.Context, it media.Item, c Candidate) error { +func (e *Enricher) downloadCover(ctx context.Context, it catalog.Item, c Candidate) error { var existing int if err := e.db.QueryRowContext(ctx, `SELECT count(*) FROM assets WHERE item_id = ? AND role = 'cover'`, it.ID).Scan(&existing); err != nil { diff --git a/internal/metadata/enrich_test.go b/internal/metadata/enrich_test.go index 66e52e7..f9bb8c9 100644 --- a/internal/metadata/enrich_test.go +++ b/internal/metadata/enrich_test.go @@ -44,7 +44,7 @@ func seedItem(t *testing.T, db *sql.DB, title string) int64 { libID, title, title).Scan(&itemID); err != nil { t.Fatal(err) } - // A file row so media.LoadItem can resolve a modtime. + // A file row so catalog.Load can resolve a modtime. if _, err := db.ExecContext(ctx, `INSERT INTO media_files (item_id, path, size, mtime) VALUES (?, 'book.epub', 1, '1')`, itemID); err != nil { t.Fatal(err) diff --git a/internal/metadata/handlers.go b/internal/metadata/handlers.go index e7ba5fb..361f7fb 100644 --- a/internal/metadata/handlers.go +++ b/internal/metadata/handlers.go @@ -8,9 +8,9 @@ import ( "strconv" "github.com/mithral/vellum/internal/auth" + "github.com/mithral/vellum/internal/catalog" "github.com/mithral/vellum/internal/httpx" "github.com/mithral/vellum/internal/jobs" - "github.com/mithral/vellum/internal/media" ) // Handlers serves metadata endpoints. @@ -97,20 +97,17 @@ func (h *Handlers) handleItemMetadata(w http.ResponseWriter, r *http.Request, u return } - it, err := media.LoadItem(r.Context(), h.db, id) - if errors.Is(err, media.ErrItemNotFound) { - httpx.WriteError(w, http.StatusNotFound, "item not found") - return - } - if err != nil { + // catalog.LoadFor is the authorization chokepoint: it returns the same + // ErrNotFound for a missing item and an unreadable one, so this stays a + // 404 either way and never leaks the existence of another user's library. + if _, err := catalog.LoadFor(r.Context(), h.db, u, id); err != nil { + if errors.Is(err, catalog.ErrNotFound) { + httpx.WriteError(w, http.StatusNotFound, "item not found") + return + } h.fail(w, "load item", err) return } - allowed, err := auth.CanAccessLibrary(r.Context(), h.db, u, it.LibraryID) - if err != nil || !allowed { - httpx.WriteError(w, http.StatusNotFound, "item not found") - return - } type providerID struct { Provider string `json:"provider"` diff --git a/internal/modules/comics/handlers.go b/internal/modules/comics/handlers.go index 94a6149..a7bc94a 100644 --- a/internal/modules/comics/handlers.go +++ b/internal/modules/comics/handlers.go @@ -9,6 +9,7 @@ import ( "strconv" "github.com/mithral/vellum/internal/auth" + "github.com/mithral/vellum/internal/catalog" "github.com/mithral/vellum/internal/httpx" "github.com/mithral/vellum/internal/media" "github.com/mithral/vellum/internal/stream" @@ -84,6 +85,9 @@ func (h *Handlers) handlePage(w http.ResponseWriter, r *http.Request, u auth.Use } // authorized loads an item, confirms access, and checks it is a comic. +// +// The load and the access check both live in catalog.LoadFor; this package no +// longer reimplements "may this user read this item". func (h *Handlers) authorized(w http.ResponseWriter, r *http.Request, u auth.User) (media.Item, bool) { id, err := strconv.ParseInt(r.PathValue("id"), 10, 64) if err != nil || id <= 0 { @@ -91,8 +95,8 @@ func (h *Handlers) authorized(w http.ResponseWriter, r *http.Request, u auth.Use return media.Item{}, false } - it, err := media.LoadItem(r.Context(), h.db, id) - if errors.Is(err, media.ErrItemNotFound) { + it, err := catalog.LoadFor(r.Context(), h.db, u, id) + if errors.Is(err, catalog.ErrNotFound) { httpx.WriteError(w, http.StatusNotFound, "item not found") return media.Item{}, false } @@ -102,11 +106,6 @@ func (h *Handlers) authorized(w http.ResponseWriter, r *http.Request, u auth.Use return media.Item{}, false } - allowed, err := auth.CanAccessLibrary(r.Context(), h.db, u, it.LibraryID) - if err != nil || !allowed { - httpx.WriteError(w, http.StatusNotFound, "item not found") - return media.Item{}, false - } if it.Kind != Kind { httpx.WriteError(w, http.StatusBadRequest, "item is not a comic") return media.Item{}, false diff --git a/internal/modules/ebooks/handlers.go b/internal/modules/ebooks/handlers.go index 41deab6..0de65b5 100644 --- a/internal/modules/ebooks/handlers.go +++ b/internal/modules/ebooks/handlers.go @@ -8,8 +8,8 @@ import ( "strconv" "github.com/mithral/vellum/internal/auth" + "github.com/mithral/vellum/internal/catalog" "github.com/mithral/vellum/internal/httpx" - "github.com/mithral/vellum/internal/media" ) // Handlers serves ebook-specific endpoints. @@ -40,8 +40,8 @@ func (h *Handlers) handleMetadata(w http.ResponseWriter, r *http.Request, u auth return } - it, err := media.LoadItem(r.Context(), h.db, id) - if errors.Is(err, media.ErrItemNotFound) { + it, err := catalog.LoadFor(r.Context(), h.db, u, id) + if errors.Is(err, catalog.ErrNotFound) { httpx.WriteError(w, http.StatusNotFound, "item not found") return } @@ -51,11 +51,6 @@ func (h *Handlers) handleMetadata(w http.ResponseWriter, r *http.Request, u auth return } - allowed, err := auth.CanAccessLibrary(r.Context(), h.db, u, it.LibraryID) - if err != nil || !allowed { - httpx.WriteError(w, http.StatusNotFound, "item not found") - return - } if it.Kind != Kind { httpx.WriteError(w, http.StatusBadRequest, "item is not an ebook") return diff --git a/internal/modules/games/handlers.go b/internal/modules/games/handlers.go index 31ff5fc..9eba0a3 100644 --- a/internal/modules/games/handlers.go +++ b/internal/modules/games/handlers.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/mithral/vellum/internal/auth" + "github.com/mithral/vellum/internal/catalog" "github.com/mithral/vellum/internal/httpx" "github.com/mithral/vellum/internal/media" ) @@ -174,6 +175,10 @@ func slotName(w http.ResponseWriter, r *http.Request) (string, bool) { return slot, true } +// authorized loads an item, confirms access, and checks it is a game. +// +// The load and the access check both live in catalog.LoadFor; this package no +// longer reimplements "may this user read this item". func (h *Handlers) authorized(w http.ResponseWriter, r *http.Request, u auth.User) (media.Item, bool) { id, err := strconv.ParseInt(r.PathValue("id"), 10, 64) if err != nil || id <= 0 { @@ -181,8 +186,8 @@ func (h *Handlers) authorized(w http.ResponseWriter, r *http.Request, u auth.Use return media.Item{}, false } - it, err := media.LoadItem(r.Context(), h.db, id) - if errors.Is(err, media.ErrItemNotFound) { + it, err := catalog.LoadFor(r.Context(), h.db, u, id) + if errors.Is(err, catalog.ErrNotFound) { httpx.WriteError(w, http.StatusNotFound, "item not found") return media.Item{}, false } @@ -191,11 +196,6 @@ func (h *Handlers) authorized(w http.ResponseWriter, r *http.Request, u auth.Use return media.Item{}, false } - allowed, err := auth.CanAccessLibrary(r.Context(), h.db, u, it.LibraryID) - if err != nil || !allowed { - httpx.WriteError(w, http.StatusNotFound, "item not found") - return media.Item{}, false - } if it.Kind != Kind { httpx.WriteError(w, http.StatusBadRequest, "item is not a game") return media.Item{}, false diff --git a/internal/playback/handlers.go b/internal/playback/handlers.go index 8e62f20..826b70b 100644 --- a/internal/playback/handlers.go +++ b/internal/playback/handlers.go @@ -15,6 +15,7 @@ import ( "net/http" "github.com/mithral/vellum/internal/auth" + "github.com/mithral/vellum/internal/catalog" "github.com/mithral/vellum/internal/engine" "github.com/mithral/vellum/internal/httpx" ) @@ -63,39 +64,24 @@ func (h *Handlers) handleResolve(w http.ResponseWriter, r *http.Request, u auth. return } - // The same access-control chokepoint the media handlers use: load the item's - // library, then ask CanAccessLibrary. An item the user cannot read is a 404, - // not a 403, so its existence is not disclosed. - var libraryID int64 - err := h.db.QueryRowContext(r.Context(), - `SELECT library_id FROM media_items WHERE id = ?`, req.ItemID).Scan(&libraryID) - if errors.Is(err, sql.ErrNoRows) { - httpx.WriteError(w, http.StatusNotFound, "item not found") - return - } - if err != nil { + // catalog.LoadFor is the shared access-control chokepoint: it reads the + // item and confirms the caller may read its library, returning the same + // ErrNotFound for a missing item and an unreadable one. That keeps an + // inaccessible item a 404, never a 403, so its existence is not disclosed. + if _, err := catalog.LoadFor(r.Context(), h.db, u, req.ItemID); err != nil { + if errors.Is(err, catalog.ErrNotFound) { + httpx.WriteError(w, http.StatusNotFound, "item not found") + return + } h.fail(w, "load item", err) return } - allowed, err := auth.CanAccessLibrary(r.Context(), h.db, u, libraryID) - if err != nil { - h.fail(w, "check access", err) - return - } - if !allowed { - httpx.WriteError(w, http.StatusNotFound, "item not found") - return - } - // The first file carries the codec facts the player will try. An item with // no file cannot be played, but is still a 200 with a resolution so the UI // has one shape to render. - var container, vcodec, acodec string - err = h.db.QueryRowContext(r.Context(), - `SELECT container, vcodec, acodec FROM media_files WHERE item_id = ? ORDER BY id LIMIT 1`, - req.ItemID).Scan(&container, &vcodec, &acodec) - if errors.Is(err, sql.ErrNoRows) { + f, err := catalog.FirstFile(r.Context(), h.db, req.ItemID) + if errors.Is(err, catalog.ErrNotFound) { httpx.WriteJSON(w, http.StatusOK, engine.Resolution{ Playable: false, Reason: "nofile", @@ -108,7 +94,7 @@ func (h *Handlers) handleResolve(w http.ResponseWriter, r *http.Request, u auth. return } - res := engine.Resolve(engine.Facts(container, vcodec, acodec), req.Engines) + res := engine.Resolve(engine.Facts(f.Container, f.VCodec, f.ACodec), req.Engines) httpx.WriteJSON(w, http.StatusOK, res) } diff --git a/internal/subsonic/store.go b/internal/subsonic/store.go index 8168d6c..19c0adf 100644 --- a/internal/subsonic/store.go +++ b/internal/subsonic/store.go @@ -3,6 +3,7 @@ package subsonic import ( "context" "database/sql" + "errors" "net/http" "path/filepath" "sort" @@ -11,6 +12,7 @@ import ( "unicode" "github.com/mithral/vellum/internal/auth" + "github.com/mithral/vellum/internal/catalog" ) // libraryRef is an audio library as getMusicFolders shows it. @@ -118,12 +120,22 @@ func (h *Handlers) albumsByArtist(ctx context.Context, libs []int64, personID in []any{personID}, "i.sort_title", 500, 0) } +// loadAlbum authorizes and loads one album for a request. +// +// catalog.LoadFor is the one access rule: it reads the item and confirms the +// caller may read its library, returning the same ErrNotFound for a missing +// item and a forbidden one. The album projection below is Subsonic's own +// presentation concern — song count, duration, artist credit and cover are not +// part of catalog's item model — and no longer decides access. func (h *Handlers) loadAlbum(ctx context.Context, u auth.User, itemID int64) (albumInfo, bool, error) { - libs, err := h.audioLibraryIDs(ctx, u) + it, err := catalog.LoadFor(ctx, h.db, u, itemID) if err != nil { + if errors.Is(err, catalog.ErrNotFound) { + return albumInfo{}, false, nil + } return albumInfo{}, false, err } - albums, err := h.albumsWhere(ctx, libs, ` AND i.id = ?`, []any{itemID}, "i.id", 1, 0) + albums, err := h.albumsWhere(ctx, []int64{it.LibraryID}, ` AND i.id = ?`, []any{itemID}, "i.id", 1, 0) if err != nil { return albumInfo{}, false, err } @@ -179,6 +191,12 @@ func (h *Handlers) tracksForItem(ctx context.Context, itemID int64) ([]track, er return tracks, nil } +// loadTrack authorizes and loads one song (a media_files row) for a request. +// +// The song projection above is Subsonic's own presentation shape — album +// context, artist credit, cover — so it stays here. Access is not its job: +// catalog.LoadFor is the one access rule, returning the same ErrNotFound for a +// missing item and a forbidden one so a 404 cannot leak existence. func (h *Handlers) loadTrack(ctx context.Context, u auth.User, fileIDVal int64) (track, bool, error) { rows, err := h.db.QueryContext(ctx, trackSelect+` WHERE f.id = ? AND f.missing = 0 AND i.missing = 0`, fileIDVal) @@ -193,13 +211,12 @@ func (h *Handlers) loadTrack(ctx context.Context, u auth.User, fileIDVal int64) if len(tracks) == 0 { return track{}, false, nil } - allowed, err := auth.CanAccessLibrary(ctx, h.db, u, tracks[0].LibraryID) - if err != nil { + if _, err := catalog.LoadFor(ctx, h.db, u, tracks[0].ItemID); err != nil { + if errors.Is(err, catalog.ErrNotFound) { + return track{}, false, nil + } return track{}, false, err } - if !allowed { - return track{}, false, nil - } return tracks[0], true, nil }