From 7b9e8e4fc2adc0fe52b5c2ca9c45470aca55ba0c Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Tue, 27 Oct 2026 23:44:41 -0400 Subject: [PATCH] fix(scanner): match library folders only at directory boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bare string-prefix test (strings.CutPrefix / strings.HasPrefix) let a registered folder whose path was a string prefix of a sibling claim the sibling's files: "/app/audiobooks" matched "/app/audiobookssd/Brian Jacques/...", so getRelativePath sliced the remnant "sd/" into every stored relative path for that library (prod scan log: "[AUDIO] Created folder book 'Taggerung' (41 tracks) at sd/Brian Jacques/Redwall/Book 14"). Scanner lookups built their keys with the same function (getMediaItemByFilePath, scanStateLookup), so writes and lookups were consistently poisoned and no scan ever corrected them; everything outside the scanner resolves paths by joining back to the roots (ResolveMediaPath), so cover/track URLs 404ed for files sitting one directory away. The same unchecked HasPrefix pattern also drove five folder-membership sites, so a sibling-prefix pair misattributed more than paths: owning-library resolution (processMediaFile, maybeProcessAudioBook), per-library-type extension allowlisting (isScannableFile), and folder-structure metadata extraction on both create and update paths — a file could be scanned under the wrong library's rules entirely. folderContains(folder, path) is now the single boundary-aware primitive: path must equal the folder or continue with a filepath.Separator (after Clean). All seven sites go through it, so no folder name — however similar to another — can interfere with any other's files again, in any order of registration; trailing slashes on registered folders are normalized. Registration order still decides for genuinely overlapping (nested) registered roots: pre-existing behavior, deliberately unchanged. Regression tests (media_scanner_path_test.go, pure unit — no DB) pin the prod trap (audiobooks/audiobookssd), order-independence, trailing slashes, mid-word siblings, and the helper itself. Repair of already-poisoned rows is deliberately separate and safe only under this fix: a rescan repoints them via the SHA-256 move path (MoveMediaItemFilePath) once lookups stop producing poisoned keys. --- internal/services/media_scanner.go | 28 +++- .../services/media_scanner_audio_multifile.go | 4 +- internal/services/media_scanner_path_test.go | 134 ++++++++++++++++++ 3 files changed, 157 insertions(+), 9 deletions(-) create mode 100644 internal/services/media_scanner_path_test.go diff --git a/internal/services/media_scanner.go b/internal/services/media_scanner.go index 348fc9d6..6a563766 100644 --- a/internal/services/media_scanner.go +++ b/internal/services/media_scanner.go @@ -954,7 +954,7 @@ func (s *MediaScanner) isScannableFile(path string) bool { // Find which folder this file belongs to var folder string for _, f := range s.folders { - if strings.HasPrefix(path, f) { + if folderContains(f, path) { folder = f break } @@ -1117,7 +1117,7 @@ func (s *MediaScanner) processMediaFile(ctx context.Context, path string) (bool, folderLibrary := s.folderLibrary s.scanStateMu.RUnlock() for _, folder := range s.folders { - if !strings.HasPrefix(path, folder) { + if !folderContains(folder, path) { continue } if libID, cached := folderLibrary[folder]; cached { @@ -1260,7 +1260,7 @@ func (s *MediaScanner) processMediaFile(ctx context.Context, path string) (bool, // Use the root folder that contains this file var rootFolder string for _, folder := range s.folders { - if strings.HasPrefix(path, folder) { + if folderContains(folder, path) { rootFolder = folder break } @@ -4052,7 +4052,7 @@ func (s *MediaScanner) updateMediaItemWithMetadata(ctx context.Context, existing // could clear series/author that a sidecar didn't carry (Libation // writes "series": null while the folder layout knows the series). for _, folder := range s.folders { - if strings.HasPrefix(path, folder) { + if folderContains(folder, path) { if folderMetadata := s.extractFolderStructureMetadata(path, folder); folderMetadata != nil { if metadata.Title == "" && folderMetadata.Title != "" { metadata.Title = folderMetadata.Title @@ -5061,13 +5061,27 @@ func (s *MediaScanner) extractHashInfo(filePath string) (*HashInfo, *FormatInfo, return hashInfo, formatInfo, nil } +// folderContains reports whether path is folder itself or lies beneath it +// at a directory boundary. This is the only path-correct containment test: +// a bare strings.HasPrefix(path, folder) also matches string-prefix +// siblings ("/app/audiobooks" claims "/app/audiobookssd/..."), which once +// sliced a stray "sd/" into every stored relative path for that library +// and resolved those files against the wrong root (folder ownership, +// library attribution, extension allowlisting, cover 404s). +func folderContains(folder, path string) bool { + folder = filepath.Clean(folder) + return path == folder || strings.HasPrefix(path, folder+string(filepath.Separator)) +} + func (s *MediaScanner) getRelativePath(absolutePath string) string { // Get the base folder paths from scanner for _, baseFolder := range s.folders { - // Check if path is within this base folder - if relPath, ok := strings.CutPrefix(absolutePath, baseFolder); ok { - return strings.TrimPrefix(relPath, "/") + // Check if path is within this base folder — boundary-aware, so a + // sibling whose name merely extends ours can never be cut. + if !folderContains(baseFolder, absolutePath) { + continue } + return strings.TrimPrefix(absolutePath[len(filepath.Clean(baseFolder)):], "/") } // Fallback: if no match, return as-is (shouldn't happen) return absolutePath diff --git a/internal/services/media_scanner_audio_multifile.go b/internal/services/media_scanner_audio_multifile.go index e5a7ef34..81641a32 100644 --- a/internal/services/media_scanner_audio_multifile.go +++ b/internal/services/media_scanner_audio_multifile.go @@ -178,7 +178,7 @@ func (s *MediaScanner) maybeProcessAudioBook(ctx context.Context, path string) ( // so any scannable audio file belongs to one. rootFolder := "" for _, folder := range s.folders { - if strings.HasPrefix(path, folder) { + if folderContains(folder, path) { rootFolder = folder break } @@ -731,7 +731,7 @@ func (s *MediaScanner) rescanMultiFileAudioBook(ctx context.Context, item databa rootFolder := "" for _, folder := range s.folders { - if strings.HasPrefix(fullPath, folder) { + if folderContains(folder, fullPath) { rootFolder = folder break } diff --git a/internal/services/media_scanner_path_test.go b/internal/services/media_scanner_path_test.go new file mode 100644 index 00000000..8bc6e7d3 --- /dev/null +++ b/internal/services/media_scanner_path_test.go @@ -0,0 +1,134 @@ +package services + +import "testing" + +// Regression tests for directory-boundary folder matching. +// +// getRelativePath (and the folder-membership checks across the scanner) +// once used bare string-prefix matching, so a registered folder whose path +// was a string prefix of a sibling's ("/app/audiobooks" vs +// "/app/audiobookssd") claimed the sibling's files: relative paths were +// cut mid-component into "sd/..."-style garbage, stored that way, and +// resolved against the wrong root — covers 404ing while the bytes sat one +// directory away. folderContains pins the rule: a folder only ever claims +// itself or paths beneath it at a real separator boundary, regardless of +// names, registration order, or trailing slashes. + +func TestFolderContains(t *testing.T) { + tests := []struct { + name string + folder string + path string + want bool + }{ + {"exact root", "/app/books", "/app/books", true}, + {"direct child", "/app/books", "/app/books/x.epub", true}, + {"deep child", "/app/books", "/app/books/a/b/c.epub", true}, + {"sibling extension", "/app/books", "/app/books2", false}, + {"sibling extension child", "/app/books", "/app/books2/x.epub", false}, + {"mid-word sibling", "/data/lib", "/data/library/x.epub", false}, + {"suffix without separator", "/app/books", "/app/books-backup/x.epub", false}, + {"unrelated", "/app/books", "/opt/other/x.epub", false}, + {"prefix inside path segment", "/app/books", "/app/series/books/x.epub", false}, + {"trailing slash in folder", "/app/books/", "/app/books/x.epub", true}, + {"trailing slash exact", "/app/books/", "/app/books", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := folderContains(tt.folder, tt.path); got != tt.want { + t.Errorf("folderContains(%q, %q) = %v, want %v", tt.folder, tt.path, got, tt.want) + } + }) + } +} + +func TestGetRelativePathFolderBoundary(t *testing.T) { + // The prod trap: /app/audiobooks registered before /app/audiobookssd. + // String-prefix cutting produced "sd/Brian Jacques/..." for every file + // in the second folder; the boundary rule must make that impossible. + newScanner := func(folders []string) *MediaScanner { + s := NewMediaScanner(nil) + s.folders = folders // SetFolders requires a live DB; tests only need path relativization + return s + } + + siblingFolders := []string{"/app/audiobooks", "/app/audiobookssd"} + + tests := []struct { + name string + folders []string + path string + want string + }{ + { + name: "sibling-prefix folder keeps its full name in relative path", + folders: siblingFolders, + path: "/app/audiobookssd/Brian Jacques/Redwall/Book 01 - Redwall/file.m4b", + want: "Brian Jacques/Redwall/Book 01 - Redwall/file.m4b", + }, + { + name: "first registered folder unaffected", + folders: siblingFolders, + path: "/app/audiobooks/Author/Series/file.mp3", + want: "Author/Series/file.mp3", + }, + { + name: "exact root yields empty path", + folders: siblingFolders, + path: "/app/audiobookssd", + want: "", + }, + { + name: "order independence: sibling registered first", + folders: []string{"/app/audiobookssd", "/app/audiobooks"}, + path: "/app/audiobookssd/Brian Jacques/Redwall/Book 01 - Redwall/file.m4b", + want: "Brian Jacques/Redwall/Book 01 - Redwall/file.m4b", + }, + { + name: "order independence: first folder still correct when second registered first", + folders: []string{"/app/audiobookssd", "/app/audiobooks"}, + path: "/app/audiobooks/Author/Series/file.mp3", + want: "Author/Series/file.mp3", + }, + { + name: "trailing slash on registered folders", + folders: []string{"/app/audiobooks/", "/app/audiobookssd/"}, + path: "/app/audiobookssd/Brian Jacques/file.m4b", + want: "Brian Jacques/file.m4b", + }, + { + name: "mid-word sibling folders", + folders: []string{"/data/lib", "/data/library"}, + path: "/data/library/author/book.epub", + want: "author/book.epub", + }, + { + name: "author/series/title layout preserved under true root", + folders: siblingFolders, + path: "/app/audiobookssd/Brian Jacques/Redwall/Book 14 - The Taggerung/Taggerung.mp3", + want: "Brian Jacques/Redwall/Book 14 - The Taggerung/Taggerung.mp3", + }, + { + name: "path outside every folder returned as-is", + folders: siblingFolders, + path: "/app/audiobooksBackup/x.mp3", + want: "/app/audiobooksBackup/x.mp3", + }, + { + name: "unrelated path returned as-is", + folders: siblingFolders, + path: "/elsewhere/x.mp3", + want: "/elsewhere/x.mp3", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := newScanner(tt.folders) + if got := s.getRelativePath(tt.path); got != tt.want { + t.Errorf("getRelativePath(%q) with folders %v = %q, want %q", tt.path, tt.folders, got, tt.want) + } + }) + } +}