From febf5c4a28f291290cbdf07a746be8e4a7e6a0c3 Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Fri, 14 Aug 2026 23:16:09 -0400 Subject: [PATCH] fix(transcode): allow catalogued symlink media (#644) --- cmd/silo/main.go | 2 +- internal/scanner/file_repo.go | 20 ++++ .../scanner/file_repo_active_path_test.go | 69 ++++++++++++++ internal/transcodenode/path_authorizer.go | 73 +++++--------- .../transcodenode/path_authorizer_test.go | 94 +++++++++---------- internal/transcodenode/server_test.go | 2 +- 6 files changed, 161 insertions(+), 99 deletions(-) create mode 100644 internal/scanner/file_repo_active_path_test.go diff --git a/cmd/silo/main.go b/cmd/silo/main.go index 015c0c8d..71a9da4c 100644 --- a/cmd/silo/main.go +++ b/cmd/silo/main.go @@ -730,7 +730,7 @@ func main() { handler = srv.Handler() } else { srv := transcodenode.NewServer(watcher, tracker) - srv.SetInputPathAuthorizer(transcodenode.NewMediaRootAuthorizer(catalog.NewFolderRepository(pool))) + srv.SetInputPathAuthorizer(transcodenode.NewCatalogPathAuthorizer(scanner.NewFileRepository(pool))) srv.SetFFmpegLogSink(playback.NewSlogFFmpegLogSink(slog.Default(), nodeID)) // Read jellycompat reconstruction recipes central wrote at transcode // start, so this node can rebuild a Jellyfin transcode after its own diff --git a/internal/scanner/file_repo.go b/internal/scanner/file_repo.go index 8a3e441b..7c2605b0 100644 --- a/internal/scanner/file_repo.go +++ b/internal/scanner/file_repo.go @@ -1943,6 +1943,26 @@ func (r *FileRepository) GetByPath(ctx context.Context, path string) (*models.Me return scanMediaFile(r.pool.QueryRow(ctx, query, path)) } +// IsActivePath reports whether path is the exact logical path of a media file +// that is still active in the catalog. Scanner paths are authoritative here: +// they deliberately preserve readable symlinks instead of replacing them with +// their physical targets. +func (r *FileRepository) IsActivePath(ctx context.Context, path string) (bool, error) { + var active bool + err := r.pool.QueryRow(ctx, ` + SELECT EXISTS ( + SELECT 1 + FROM media_files + WHERE file_path = $1 + AND missing_since IS NULL + ) + `, path).Scan(&active) + if err != nil { + return false, fmt.Errorf("checking active media file path: %w", err) + } + return active, nil +} + // GetByHash retrieves a media file by its file hash. func (r *FileRepository) GetByHash(ctx context.Context, hash string) (*models.MediaFile, error) { query := `SELECT ` + fileColumns + ` FROM media_files WHERE file_hash = $1 LIMIT 1` diff --git a/internal/scanner/file_repo_active_path_test.go b/internal/scanner/file_repo_active_path_test.go new file mode 100644 index 00000000..9e2ffea3 --- /dev/null +++ b/internal/scanner/file_repo_active_path_test.go @@ -0,0 +1,69 @@ +package scanner + +import ( + "context" + "fmt" + "os" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgxpool" +) + +func TestFileRepositoryIsActivePath(t *testing.T) { + dsn := os.Getenv("SILO_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("SILO_TEST_DATABASE_URL is not set") + } + + ctx := context.Background() + pool, err := pgxpool.New(ctx, dsn) + if err != nil { + t.Fatalf("connect test database: %v", err) + } + t.Cleanup(pool.Close) + + suffix := time.Now().UnixNano() + var folderID int + if err := pool.QueryRow(ctx, ` + INSERT INTO media_folders (type, name, enabled) + VALUES ('movies', 'Active Path Test', true) + RETURNING id + `).Scan(&folderID); err != nil { + t.Fatalf("seed folder: %v", err) + } + t.Cleanup(func() { + _, _ = pool.Exec(ctx, `DELETE FROM media_files WHERE media_folder_id = $1`, folderID) + _, _ = pool.Exec(ctx, `DELETE FROM media_folders WHERE id = $1`, folderID) + }) + + activePath := fmt.Sprintf("/tmp/silo-active-path-%d.mkv", suffix) + missingPath := fmt.Sprintf("/tmp/silo-missing-path-%d.mkv", suffix) + if _, err := pool.Exec(ctx, ` + INSERT INTO media_files (media_folder_id, file_path, missing_since) + VALUES ($1, $2, NULL), ($1, $3, NOW()) + `, folderID, activePath, missingPath); err != nil { + t.Fatalf("seed media files: %v", err) + } + + repo := NewFileRepository(pool) + for _, test := range []struct { + name string + path string + want bool + }{ + {name: "active", path: activePath, want: true}, + {name: "missing", path: missingPath, want: false}, + {name: "unknown", path: fmt.Sprintf("/tmp/silo-unknown-path-%d.mkv", suffix), want: false}, + } { + t.Run(test.name, func(t *testing.T) { + got, err := repo.IsActivePath(ctx, test.path) + if err != nil { + t.Fatalf("IsActivePath: %v", err) + } + if got != test.want { + t.Fatalf("IsActivePath(%q) = %v, want %v", test.path, got, test.want) + } + }) + } +} diff --git a/internal/transcodenode/path_authorizer.go b/internal/transcodenode/path_authorizer.go index 73542236..29b6909b 100644 --- a/internal/transcodenode/path_authorizer.go +++ b/internal/transcodenode/path_authorizer.go @@ -5,54 +5,50 @@ import ( "os" "path/filepath" "strings" - - "github.com/Silo-Server/silo-server/internal/models" ) // InputPathAuthorizer approves a local media input before a node passes it to -// FFmpeg. Implementations must reject protocol URLs and paths outside managed -// library roots. +// FFmpeg. Implementations must reject protocol URLs and paths outside the +// authoritative media catalog. type InputPathAuthorizer interface { Allowed(ctx context.Context, path string) (bool, error) } -type mediaFolderSource interface { - List(ctx context.Context) ([]*models.MediaFolder, error) +type catalogPathSource interface { + IsActivePath(ctx context.Context, path string) (bool, error) } -// MediaRootAuthorizer resolves the deployment's current library roots and -// permits only existing, absolute filesystem paths contained by those roots. -type MediaRootAuthorizer struct { - folders mediaFolderSource +// CatalogPathAuthorizer permits only existing regular files whose exact +// logical path is active in the media catalog. The scanner deliberately keeps +// logical paths for readable symlinks, so catalog membership is the correct +// authority: resolving the target and requiring it to remain under the logical +// library root would reject media layouts the scanner explicitly supports. +type CatalogPathAuthorizer struct { + paths catalogPathSource } -// NewMediaRootAuthorizer creates an FFmpeg input authorizer backed by the -// authoritative media-folder repository. -func NewMediaRootAuthorizer(folders mediaFolderSource) *MediaRootAuthorizer { - return &MediaRootAuthorizer{folders: folders} +// NewCatalogPathAuthorizer creates an FFmpeg input authorizer backed by the +// authoritative media-file catalog. +func NewCatalogPathAuthorizer(paths catalogPathSource) *CatalogPathAuthorizer { + return &CatalogPathAuthorizer{paths: paths} } -// Allowed reports whether path resolves inside one of the configured media -// roots. Symlinks are resolved on both sides so a link cannot escape a root. -func (a *MediaRootAuthorizer) Allowed(ctx context.Context, path string) (bool, error) { - if a == nil || a.folders == nil || !plainAbsolutePath(path) { +// Allowed reports whether path is an active catalog entry that resolves to a +// regular file on this node. os.Stat follows scanner-approved symlinks while +// rejecting dangling links, directories, and other non-regular inputs. +func (a *CatalogPathAuthorizer) Allowed(ctx context.Context, path string) (bool, error) { + if a == nil || a.paths == nil || !plainAbsolutePath(path) { return false, nil } - folders, err := a.folders.List(ctx) + active, err := a.paths.IsActivePath(ctx, path) if err != nil { return false, err } - for _, folder := range folders { - if folder == nil { - continue - } - for _, root := range folder.Paths { - if existingPathWithinRoot(root, path) { - return true, nil - } - } + if !active { + return false, nil } - return false, nil + info, err := os.Stat(path) + return err == nil && info.Mode().IsRegular(), nil } func plainAbsolutePath(path string) bool { @@ -60,25 +56,6 @@ func plainAbsolutePath(path string) bool { return path != "" && !strings.ContainsRune(path, '\x00') && filepath.IsAbs(path) } -func existingPathWithinRoot(root, target string) bool { - if !plainAbsolutePath(root) || !plainAbsolutePath(target) { - return false - } - resolvedRoot, err := filepath.EvalSymlinks(filepath.Clean(root)) - if err != nil { - return false - } - resolvedTarget, err := filepath.EvalSymlinks(filepath.Clean(target)) - if err != nil { - return false - } - info, err := os.Stat(resolvedTarget) - if err != nil || !info.Mode().IsRegular() { - return false - } - return resolvedPathContained(resolvedRoot, resolvedTarget) -} - // pathWithinRoot validates a not-yet-created output by resolving the root and // target parent. The caller creates the basename only after this check. func pathWithinRoot(root, target string) bool { diff --git a/internal/transcodenode/path_authorizer_test.go b/internal/transcodenode/path_authorizer_test.go index 4965147e..0ebb8adc 100644 --- a/internal/transcodenode/path_authorizer_test.go +++ b/internal/transcodenode/path_authorizer_test.go @@ -7,41 +7,63 @@ import ( "path/filepath" "runtime" "testing" - - "github.com/Silo-Server/silo-server/internal/models" ) -type staticMediaFolders struct { - folders []*models.MediaFolder - err error +type staticCatalogPaths struct { + active map[string]bool + err error } -func (s staticMediaFolders) List(context.Context) ([]*models.MediaFolder, error) { - return s.folders, s.err +func (s staticCatalogPaths) IsActivePath(_ context.Context, path string) (bool, error) { + return s.active[path], s.err } -func TestMediaRootAuthorizerAllowsOnlyExistingFilesWithinLibraryRoots(t *testing.T) { - root := t.TempDir() - mediaPath := filepath.Join(root, "movie.mkv") +func TestCatalogPathAuthorizerAllowsCataloguedRegularFilesAndSymlinks(t *testing.T) { + mediaPath := filepath.Join(t.TempDir(), "movie.mkv") if err := os.WriteFile(mediaPath, []byte("media"), 0o600); err != nil { t.Fatal(err) } - authorizer := NewMediaRootAuthorizer(staticMediaFolders{ - folders: []*models.MediaFolder{{Paths: []string{root}}}, - }) - - allowed, err := authorizer.Allowed(context.Background(), mediaPath) - if err != nil || !allowed { - t.Fatalf("approved media path: allowed = %v, err = %v", allowed, err) + target := filepath.Join(t.TempDir(), "outside.mkv") + if err := os.WriteFile(target, []byte("media"), 0o600); err != nil { + t.Fatal(err) } + link := filepath.Join(t.TempDir(), "linked.mkv") + if err := os.Symlink(target, link); err != nil { + t.Skipf("symlinks not supported on this platform: %v", err) + } + authorizer := NewCatalogPathAuthorizer(staticCatalogPaths{active: map[string]bool{ + mediaPath: true, + link: true, + }}) + + for _, path := range []string{mediaPath, link} { + allowed, err := authorizer.Allowed(context.Background(), path) + if err != nil || !allowed { + t.Fatalf("approved media path %q: allowed = %v, err = %v", path, allowed, err) + } + } +} + +func TestCatalogPathAuthorizerRejectsUnsafeOrUncataloguedInputs(t *testing.T) { + existingUncatalogued := filepath.Join(t.TempDir(), "outside.mkv") + if err := os.WriteFile(existingUncatalogued, []byte("media"), 0o600); err != nil { + t.Fatal(err) + } + missingCatalogued := filepath.Join(t.TempDir(), "missing.mkv") + directoryCatalogued := t.TempDir() + authorizer := NewCatalogPathAuthorizer(staticCatalogPaths{active: map[string]bool{ + missingCatalogued: true, + directoryCatalogued: true, + }}) for _, candidate := range []string{ "relative/movie.mkv", "http://example.test/movie.mkv", - "file:" + mediaPath, - "concat:" + mediaPath + "|" + mediaPath, - filepath.Join(t.TempDir(), "outside.mkv"), - filepath.Join(root, "missing.mkv"), + "file:" + existingUncatalogued, + "concat:" + existingUncatalogued + "|" + existingUncatalogued, + existingUncatalogued, + missingCatalogued, + directoryCatalogued, } { allowed, err := authorizer.Allowed(context.Background(), candidate) if err != nil { @@ -53,35 +75,9 @@ func TestMediaRootAuthorizerAllowsOnlyExistingFilesWithinLibraryRoots(t *testing } } -func TestMediaRootAuthorizerRejectsSymlinkEscape(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("symlink semantics differ on Windows") - } - root := t.TempDir() - outside := filepath.Join(t.TempDir(), "outside.mkv") - if err := os.WriteFile(outside, []byte("media"), 0o600); err != nil { - t.Fatal(err) - } - link := filepath.Join(root, "linked.mkv") - if err := os.Symlink(outside, link); err != nil { - t.Fatal(err) - } - authorizer := NewMediaRootAuthorizer(staticMediaFolders{ - folders: []*models.MediaFolder{{Paths: []string{root}}}, - }) - - allowed, err := authorizer.Allowed(context.Background(), link) - if err != nil { - t.Fatal(err) - } - if allowed { - t.Fatal("symlink escape was allowed") - } -} - -func TestMediaRootAuthorizerPropagatesRepositoryErrors(t *testing.T) { +func TestCatalogPathAuthorizerPropagatesRepositoryErrors(t *testing.T) { wantErr := errors.New("database unavailable") - authorizer := NewMediaRootAuthorizer(staticMediaFolders{err: wantErr}) + authorizer := NewCatalogPathAuthorizer(staticCatalogPaths{err: wantErr}) allowed, err := authorizer.Allowed(context.Background(), "/media/movie.mkv") if allowed || !errors.Is(err, wantErr) { t.Fatalf("allowed = %v, err = %v", allowed, err) diff --git a/internal/transcodenode/server_test.go b/internal/transcodenode/server_test.go index 34efe304..27794e1a 100644 --- a/internal/transcodenode/server_test.go +++ b/internal/transcodenode/server_test.go @@ -375,7 +375,7 @@ func TestHandleDownloadPrepareRejectsUnavailableConfig(t *testing.T) { func TestHandleStartRejectsUnapprovedInputPath(t *testing.T) { server := newTestServer(t) - server.inputPaths = NewMediaRootAuthorizer(staticMediaFolders{}) + server.inputPaths = NewCatalogPathAuthorizer(staticCatalogPaths{}) body := []byte(`{"session_id":"unsafe-input","input_path":"http://example.test/movie.mkv"}`) req := httptest.NewRequest(http.MethodPost, "/transcode/start", bytes.NewReader(body))