From 0bd313717c712d38783fd3ca4b01f6a7ae1098e7 Mon Sep 17 00:00:00 2001 From: Robbie Davis Date: Wed, 1 Jul 2026 14:13:16 -0400 Subject: [PATCH 001/158] fix(downloads): preserve path whitespace Preserve download-client reported path whitespace for torrent source/content paths while keeping destination validation separate. Add user-provided library destination validation for add, move, and root-folder workflows, allowing filesystem roots for root folders while rejecting parent traversal for concrete destinations. --- .../Features/Library/LibraryAddWorkflow.cs | 16 +- .../Features/Library/LibraryMoveWorkflow.cs | 16 +- .../Audiobooks/Catalog/LibraryAddService.cs | 50 ++- .../Contracts/ILibraryAddService.cs | 4 + .../RootFolders/RootFolderService.cs | 22 +- .../Downloads/Common/DownloadClientGateway.cs | 6 +- .../Common/FileUtils.PathCombining.cs | 7 +- .../Common/FileUtils.UserProvidedPaths.cs | 361 ++++++++++++++++++ .../Paths/RemotePathMappingService.cs | 2 +- .../Common/TorrentClientPathMapper.cs | 60 +-- .../QbittorrentImportPathResolver.cs | 2 +- .../TransmissionImportPathResolver.cs | 4 +- .../LibraryController_AddToLibraryTests.cs | 39 +- .../Library/LibraryController_MoveTests.cs | 25 ++ .../RootFolders/RootFolderServiceTests.cs | 93 +++++ .../Common/DownloadClientGatewayTests.cs | 58 +++ tests/Features/Domain/Utils/FileUtilsTests.cs | 178 +++++++++ .../Common/DownloadClientAdapterTests.cs | 27 ++ .../Common/TorrentClientPathMapperTests.cs | 119 ++++++ tests/Mocks/Api/TransmissionApiMock.cs | 36 ++ 20 files changed, 1053 insertions(+), 72 deletions(-) create mode 100644 listenarr.domain/Common/FileUtils.UserProvidedPaths.cs create mode 100644 tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs diff --git a/listenarr.api/Features/Library/LibraryAddWorkflow.cs b/listenarr.api/Features/Library/LibraryAddWorkflow.cs index dfcd9a7bc..d6622e3b2 100644 --- a/listenarr.api/Features/Library/LibraryAddWorkflow.cs +++ b/listenarr.api/Features/Library/LibraryAddWorkflow.cs @@ -67,6 +67,11 @@ public async Task AddAsync(LibraryController.AddToLibraryRequest HistoryMessage = $"Audiobook '{request.Metadata.Title}' added to library from Add New page" }); + if (result.ValidationFailed) + { + return new BadRequestObjectResult(new { message = result.ValidationMessage ?? result.Message }); + } + if (result.AlreadyExists) { return new ConflictObjectResult(new { message = result.Message, audiobook = result.Audiobook }); @@ -133,7 +138,16 @@ public async Task AddAsync(LibraryController.AddToLibraryRequest if (!string.IsNullOrWhiteSpace(request.DestinationPath)) { - audiobook.BasePath = FileUtils.NormalizeStoredPath(request.DestinationPath); + if (!FileUtils.TryNormalizeUserProvidedDirectoryPathForCurrentOs( + request.DestinationPath, + out var normalizedDestinationPath, + out var validationReason, + rejectParentTraversal: true)) + { + return new BadRequestObjectResult(new { message = $"DestinationPath is not valid for this operating system: {validationReason}" }); + } + + audiobook.BasePath = normalizedDestinationPath; _logger.LogInformation("Using custom destination path for audiobook '{Title}': {BasePath}", audiobook.Title, audiobook.BasePath); } diff --git a/listenarr.api/Features/Library/LibraryMoveWorkflow.cs b/listenarr.api/Features/Library/LibraryMoveWorkflow.cs index ed3fc469e..d974b6e58 100644 --- a/listenarr.api/Features/Library/LibraryMoveWorkflow.cs +++ b/listenarr.api/Features/Library/LibraryMoveWorkflow.cs @@ -55,11 +55,6 @@ public async Task EnqueueAsync(int id, LibraryController.MoveRequ return new BadRequestObjectResult(new { message = "DestinationPath is required" }); } - if (FileUtils.IsPathInvalidForCurrentOs(request.DestinationPath)) - { - return new BadRequestObjectResult(new { message = "DestinationPath is not valid for this operating system" }); - } - try { using var scope = _scopeFactory.CreateScope(); @@ -67,8 +62,15 @@ public async Task EnqueueAsync(int id, LibraryController.MoveRequ var settings = await configService.GetApplicationSettingsAsync(); var destinationIsRooted = Path.IsPathRooted(request.DestinationPath!); - var final = FileUtils.CombineWithOptionalBase(settings.OutputPath, request.DestinationPath!); - final = FileUtils.NormalizeStoredPath(final); + var destinationCandidate = FileUtils.CombineWithOptionalBase(settings.OutputPath, request.DestinationPath!); + if (!FileUtils.TryNormalizeUserProvidedDirectoryPathForCurrentOs( + destinationCandidate, + out var final, + out var validationReason, + rejectParentTraversal: true)) + { + return new BadRequestObjectResult(new { message = $"DestinationPath is not valid for this operating system: {validationReason}" }); + } if (!destinationIsRooted && !string.IsNullOrWhiteSpace(settings.OutputPath) && !_fileSystem.TryValidateMutationTarget(final, [settings.OutputPath], out final, out var finalReason)) diff --git a/listenarr.application/Audiobooks/Catalog/LibraryAddService.cs b/listenarr.application/Audiobooks/Catalog/LibraryAddService.cs index af3cce394..e8ec83982 100644 --- a/listenarr.application/Audiobooks/Catalog/LibraryAddService.cs +++ b/listenarr.application/Audiobooks/Catalog/LibraryAddService.cs @@ -17,6 +17,7 @@ */ using System.Security.Cryptography; using System.Text; +using Listenarr.Domain.Common; using Microsoft.Extensions.Logging; namespace Listenarr.Application.Audiobooks.Catalog @@ -149,30 +150,38 @@ public async Task AddToLibraryAsync( var settings = await _configurationService.GetApplicationSettingsAsync(); - // Check validity of given path - var baseDirectory = request.DestinationPath; - if (!string.IsNullOrWhiteSpace(baseDirectory)) + var requestedBaseDirectory = request.DestinationPath; + if (!string.IsNullOrWhiteSpace(requestedBaseDirectory)) { - try - { - Path.GetFullPath(baseDirectory); - } - catch (Exception) + if (!FileUtils.TryNormalizeUserProvidedDirectoryPathForCurrentOs( + requestedBaseDirectory, + out var normalizedRequestedBaseDirectory, + out var validationReason, + rejectParentTraversal: true)) { - baseDirectory = string.Empty; + return ValidationFailure($"DestinationPath is not valid for this operating system: {validationReason}"); } - } - - if (string.IsNullOrWhiteSpace(baseDirectory)) - { - var rootFolder = await _rootFolderService.GetDefaultAsync(); - baseDirectory = rootFolder != null ? rootFolder.Path : settings.OutputPath; - audiobook.BasePath = Path.Join(baseDirectory, _fileNamingService.ApplyNamingPattern(settings.FolderNamingPattern, metadata)); + audiobook.BasePath = normalizedRequestedBaseDirectory; } else { - audiobook.BasePath = baseDirectory; + var rootFolder = await _rootFolderService.GetDefaultAsync(); + var baseDirectory = rootFolder != null ? rootFolder.Path : settings.OutputPath; + + // This validates the Listenarr-owned library destination. Do not use it for + // download-client source paths, which must preserve the client's exact path identity. + var generatedBasePath = Path.Join(baseDirectory, _fileNamingService.ApplyNamingPattern(settings.FolderNamingPattern, metadata)); + if (!FileUtils.TryNormalizeUserProvidedDirectoryPathForCurrentOs( + generatedBasePath, + out var normalizedGeneratedBasePath, + out var validationReason, + rejectParentTraversal: true)) + { + return ValidationFailure($"Generated library destination is not valid for this operating system: {validationReason}"); + } + + audiobook.BasePath = normalizedGeneratedBasePath; } cancellationToken.ThrowIfCancellationRequested(); @@ -363,6 +372,13 @@ private async Task AddHistoryEntryAsync( await _historyRepository.AddAsync(historyEntry, cancellationToken); } + private static LibraryAddOperationResult ValidationFailure(string message) => new() + { + ValidationFailed = true, + Message = message, + ValidationMessage = message + }; + private static string? ToStringOrFirst(object? value) { if (value is List list) diff --git a/listenarr.application/Audiobooks/Contracts/ILibraryAddService.cs b/listenarr.application/Audiobooks/Contracts/ILibraryAddService.cs index 890108c21..24ab82c75 100644 --- a/listenarr.application/Audiobooks/Contracts/ILibraryAddService.cs +++ b/listenarr.application/Audiobooks/Contracts/ILibraryAddService.cs @@ -50,8 +50,12 @@ public sealed class LibraryAddOperationResult public bool AlreadyExists { get; set; } + public bool ValidationFailed { get; set; } + public string Message { get; set; } = string.Empty; + public string? ValidationMessage { get; set; } + public Audiobook? Audiobook { get; set; } } } diff --git a/listenarr.application/Audiobooks/RootFolders/RootFolderService.cs b/listenarr.application/Audiobooks/RootFolders/RootFolderService.cs index 30c947621..94a7ce3b7 100644 --- a/listenarr.application/Audiobooks/RootFolders/RootFolderService.cs +++ b/listenarr.application/Audiobooks/RootFolders/RootFolderService.cs @@ -15,6 +15,7 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . */ +using Listenarr.Domain.Common; using Microsoft.Extensions.Logging; namespace Listenarr.Application.Audiobooks.RootFolders @@ -39,10 +40,9 @@ public RootFolderService(IRootFolderRepository repo, ILogger? public async Task CreateAsync(RootFolder root) { - root.Path = root.Path?.Trim() ?? string.Empty; root.Name = root.Name?.Trim() ?? string.Empty; + root.Path = NormalizeRootFolderPathForStorage(root.Path?.Trim()); - if (string.IsNullOrWhiteSpace(root.Path)) throw new ArgumentException("Path is required"); if (string.IsNullOrWhiteSpace(root.Name)) throw new ArgumentException("Name is required"); var existingByPath = await _repo.GetByPathAsync(root.Path); @@ -85,8 +85,10 @@ public async Task DeleteAsync(int id, int? reassignRootId = null) public async Task UpdateAsync(RootFolder root, bool moveFiles = false, bool deleteEmptySource = true) { if (root == null) throw new ArgumentNullException(nameof(root)); - root.Path = root.Path?.Trim() ?? string.Empty; root.Name = root.Name?.Trim() ?? string.Empty; + root.Path = NormalizeRootFolderPathForStorage(root.Path?.Trim()); + + if (string.IsNullOrWhiteSpace(root.Name)) throw new ArgumentException("Name is required"); var existing = await _repo.GetByIdAsync(root.Id); if (existing == null) throw new KeyNotFoundException("Root folder not found"); @@ -148,5 +150,19 @@ public async Task UpdateAsync(RootFolder root, bool moveFiles = fals return existing; } + + private static string NormalizeRootFolderPathForStorage(string? path) + { + if (!FileUtils.TryNormalizeUserProvidedDirectoryPathForCurrentOs( + path, + out var normalizedPath, + out var validationReason, + allowFileSystemRoot: true)) + { + throw new ArgumentException($"Path is not valid for this operating system: {validationReason}"); + } + + return normalizedPath; + } } } diff --git a/listenarr.application/Downloads/Common/DownloadClientGateway.cs b/listenarr.application/Downloads/Common/DownloadClientGateway.cs index 643c9479f..5ac28f489 100644 --- a/listenarr.application/Downloads/Common/DownloadClientGateway.cs +++ b/listenarr.application/Downloads/Common/DownloadClientGateway.cs @@ -232,12 +232,12 @@ private List GetExternalIds(List downloads) /// private async Task TranslateQueueItemPathsAsync(DownloadClientConfiguration client, QueueItem item) { - if (!string.IsNullOrWhiteSpace(item.RemotePath)) + if (!string.IsNullOrEmpty(item.RemotePath)) { item.LocalPath = await remotePathMappingService.TranslatePathAsync(client, item.RemotePath); } - if (!string.IsNullOrWhiteSpace(item.ContentPath)) + if (!string.IsNullOrEmpty(item.ContentPath)) { item.ContentPath = await remotePathMappingService.TranslatePathAsync(client, item.ContentPath); } @@ -257,7 +257,7 @@ private async Task TranslateQueueItemPathsAsync(DownloadClientConfigu } item.SourceFiles = sourceFiles; } - else if (!string.IsNullOrWhiteSpace(item.ContentPath)) + else if (!string.IsNullOrEmpty(item.ContentPath)) { // Scan ContentPath only after the adapter has supplied a non-empty path. // Active queue snapshots may not be import-ready, so adapters should leave diff --git a/listenarr.domain/Common/FileUtils.PathCombining.cs b/listenarr.domain/Common/FileUtils.PathCombining.cs index 0e710ed02..964f1661e 100644 --- a/listenarr.domain/Common/FileUtils.PathCombining.cs +++ b/listenarr.domain/Common/FileUtils.PathCombining.cs @@ -239,6 +239,11 @@ public static string SafeFileName(string name) return normalized.Length == 0 ? "unknown" : normalized; } + /// + /// Combines a relative candidate path with an optional base path without trimming + /// path-segment whitespace. Callers that must constrain rooted-looking child paths + /// should make those paths relative before calling this helper. + /// public static string CombineWithOptionalBase(string? basePath, string candidatePath) { if (string.IsNullOrEmpty(candidatePath)) @@ -246,7 +251,7 @@ public static string CombineWithOptionalBase(string? basePath, string candidateP return candidatePath; } - if (Path.IsPathRooted(candidatePath) || string.IsNullOrWhiteSpace(basePath)) + if (Path.IsPathRooted(candidatePath) || string.IsNullOrEmpty(basePath)) { return candidatePath; } diff --git a/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs b/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs new file mode 100644 index 000000000..4131c121e --- /dev/null +++ b/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs @@ -0,0 +1,361 @@ +/* + * Listenarr - Audiobook Management System + * Copyright (C) 2024-2026 Listenarr Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +using System.Text.RegularExpressions; + +namespace Listenarr.Domain.Common +{ + public static partial class FileUtils + { + private static readonly Regex WindowsDriveRootPattern = new("^[A-Za-z]:[\\\\/]", RegexOptions.Compiled); + private static readonly Regex WindowsReservedDeviceNamePattern = new( + "^(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])$", + RegexOptions.IgnoreCase | RegexOptions.Compiled); + + /// + /// Validates and normalizes a user-provided directory path that Listenarr will store or create. + /// This must not be used for externally reported download-client source paths, where whitespace + /// and other path identity details must be preserved exactly as reported by the client. + /// + public static bool TryNormalizeUserProvidedDirectoryPathForCurrentOs( + string? path, + out string normalizedPath, + out string reason, + bool allowFileSystemRoot = false, + bool rejectParentTraversal = false) => + TryNormalizeUserProvidedDirectoryPathForOs( + path, + OperatingSystem.IsWindows(), + out normalizedPath, + out reason, + allowFileSystemRoot, + rejectParentTraversal); + + public static bool TryNormalizeUserProvidedDirectoryPathForOs( + string? path, + bool isWindows, + out string normalizedPath, + out string reason, + bool allowFileSystemRoot = false, + bool rejectParentTraversal = false) + { + normalizedPath = string.Empty; + reason = string.Empty; + + if (string.IsNullOrWhiteSpace(path)) + { + reason = "Path is required."; + return false; + } + + var candidate = path; + if (candidate.IndexOf('\0') >= 0) + { + reason = "Path contains invalid characters."; + return false; + } + + if (isWindows) + { + return TryNormalizeWindowsUserProvidedDirectoryPath( + candidate, + out normalizedPath, + out reason, + allowFileSystemRoot, + rejectParentTraversal); + } + + return TryNormalizeUnixUserProvidedDirectoryPath( + candidate, + out normalizedPath, + out reason, + allowFileSystemRoot, + rejectParentTraversal); + } + + private static bool TryNormalizeWindowsUserProvidedDirectoryPath( + string path, + out string normalizedPath, + out string reason, + bool allowFileSystemRoot, + bool rejectParentTraversal) + { + normalizedPath = string.Empty; + reason = string.Empty; + + var rootLength = GetWindowsRootLength(path); + if (rootLength <= 0) + { + reason = "Path must be an absolute directory path."; + return false; + } + + var pathWithoutRoot = path[rootLength..]; + if (string.IsNullOrWhiteSpace(pathWithoutRoot.Trim('/', '\\')) && !allowFileSystemRoot) + { + reason = "Path cannot be the filesystem root."; + return false; + } + + if (!ValidateWindowsDirectorySegments(pathWithoutRoot, rejectParentTraversal, out reason)) + { + return false; + } + + try + { + normalizedPath = OperatingSystem.IsWindows() + ? Path.GetFullPath(path) + : NormalizeWindowsDirectoryPathSyntax(path); + + if (IsWindowsRootOnly(normalizedPath) && !allowFileSystemRoot) + { + normalizedPath = string.Empty; + reason = "Path cannot be the filesystem root."; + return false; + } + + return true; + } + catch (Exception exception) when (exception is not (OperationCanceledException or OutOfMemoryException or StackOverflowException)) + { + normalizedPath = string.Empty; + reason = "Path is not valid for this operating system."; + return false; + } + } + + private static bool TryNormalizeUnixUserProvidedDirectoryPath( + string path, + out string normalizedPath, + out string reason, + bool allowFileSystemRoot, + bool rejectParentTraversal) + { + normalizedPath = string.Empty; + reason = string.Empty; + + if (!path.StartsWith("/", StringComparison.Ordinal)) + { + reason = "Path must be an absolute directory path."; + return false; + } + + if (string.IsNullOrWhiteSpace(path.Trim('/')) && !allowFileSystemRoot) + { + reason = "Path cannot be the filesystem root."; + return false; + } + + if (rejectParentTraversal && ContainsParentDirectorySegment(path, '/')) + { + reason = "Path cannot traverse to a parent directory."; + return false; + } + + try + { + normalizedPath = OperatingSystem.IsWindows() + ? NormalizeUnixDirectoryPathSyntax(path) + : Path.GetFullPath(path); + + if (IsUnixRootOnly(normalizedPath) && !allowFileSystemRoot) + { + normalizedPath = string.Empty; + reason = "Path cannot be the filesystem root."; + return false; + } + + return true; + } + catch (Exception exception) when (exception is not (OperationCanceledException or OutOfMemoryException or StackOverflowException)) + { + normalizedPath = string.Empty; + reason = "Path is not valid for this operating system."; + return false; + } + } + + private static int GetWindowsRootLength(string path) + { + if (WindowsDriveRootPattern.IsMatch(path)) + { + return 3; + } + + if (!path.StartsWith(@"\\", StringComparison.Ordinal) && !path.StartsWith("//", StringComparison.Ordinal)) + { + return 0; + } + + var parts = path.Split(new[] { '\\', '/' }, StringSplitOptions.RemoveEmptyEntries); + if (parts.Length < 2) + { + return 0; + } + + var index = 2; + var separatorsSeen = 0; + while (index < path.Length && separatorsSeen < 2) + { + if (path[index] is '\\' or '/') + { + separatorsSeen++; + } + + index++; + } + + return index; + } + + private static bool ValidateWindowsDirectorySegments(string pathWithoutRoot, bool rejectParentTraversal, out string reason) + { + reason = string.Empty; + var segments = pathWithoutRoot.Split(new[] { '\\', '/' }, StringSplitOptions.RemoveEmptyEntries); + foreach (var segment in segments) + { + if (segment == ".." && rejectParentTraversal) + { + reason = "Path cannot traverse to a parent directory."; + return false; + } + + if (segment is "." or "..") + { + continue; + } + + if (segment.Any(IsInvalidWindowsDirectorySegmentCharacter)) + { + reason = "Path contains invalid characters."; + return false; + } + + if (segment.EndsWith(' ') || segment.EndsWith('.')) + { + reason = "Path segments cannot end with a space or period on Windows."; + return false; + } + + var stem = segment.Split('.', 2)[0]; + if (WindowsReservedDeviceNamePattern.IsMatch(stem)) + { + reason = "Path contains a reserved Windows device name."; + return false; + } + } + + return true; + } + + private static bool IsInvalidWindowsDirectorySegmentCharacter(char character) + { + return character < 32 || character is '<' or '>' or ':' or '"' or '|' or '?' or '*'; + } + + private static bool ContainsParentDirectorySegment(string path, params char[] separators) + { + return path.Split(separators, StringSplitOptions.RemoveEmptyEntries) + .Any(segment => segment == ".."); + } + + private static string NormalizeWindowsDirectoryPathSyntax(string path) + { + var normalizedPath = path.Replace('/', '\\'); + var rootLength = GetWindowsRootLength(normalizedPath); + var root = normalizedPath[..rootLength]; + var pathWithoutRoot = normalizedPath[rootLength..]; + var segments = new List(); + + foreach (var segment in pathWithoutRoot.Split('\\', StringSplitOptions.RemoveEmptyEntries)) + { + if (segment == ".") + { + continue; + } + + if (segment == "..") + { + if (segments.Count > 0) + { + segments.RemoveAt(segments.Count - 1); + } + + continue; + } + + segments.Add(segment); + } + + return segments.Count == 0 + ? root.TrimEnd('\\') + : root.TrimEnd('\\') + "\\" + string.Join("\\", segments); + } + + private static bool IsWindowsRootOnly(string path) + { + var pathWithWindowsSeparators = path.Replace('/', '\\'); + var normalizedPath = pathWithWindowsSeparators.TrimEnd('\\'); + if (Regex.IsMatch(normalizedPath, "^[A-Za-z]:$")) + { + return true; + } + + var rootLength = GetWindowsRootLength(pathWithWindowsSeparators); + if (rootLength <= 0) + { + return false; + } + + var root = pathWithWindowsSeparators[..rootLength].TrimEnd('\\'); + return string.Equals(normalizedPath, root, StringComparison.OrdinalIgnoreCase); + } + + private static string NormalizeUnixDirectoryPathSyntax(string path) + { + var segments = new List(); + foreach (var segment in path.Split('/', StringSplitOptions.RemoveEmptyEntries)) + { + if (segment == ".") + { + continue; + } + + if (segment == "..") + { + if (segments.Count > 0) + { + segments.RemoveAt(segments.Count - 1); + } + + continue; + } + + segments.Add(segment); + } + + return segments.Count == 0 ? "/" : "/" + string.Join("/", segments); + } + + private static bool IsUnixRootOnly(string path) + { + return string.Equals(path.TrimEnd('/'), string.Empty, StringComparison.Ordinal) + || string.Equals(path.TrimEnd('/'), "/", StringComparison.Ordinal); + } + } +} diff --git a/listenarr.infrastructure/Configuration/Paths/RemotePathMappingService.cs b/listenarr.infrastructure/Configuration/Paths/RemotePathMappingService.cs index 7e09c2346..416fd0e5e 100644 --- a/listenarr.infrastructure/Configuration/Paths/RemotePathMappingService.cs +++ b/listenarr.infrastructure/Configuration/Paths/RemotePathMappingService.cs @@ -116,7 +116,7 @@ public async Task DeleteAsync(int id) public async Task TranslatePathAsync(DownloadClientConfiguration client, string remotePath) { - if (string.IsNullOrWhiteSpace(remotePath)) + if (string.IsNullOrEmpty(remotePath)) { return remotePath; } diff --git a/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs b/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs index 614406038..3fa0cd9a8 100644 --- a/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs +++ b/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs @@ -27,22 +27,25 @@ public static List BuildQbittorrentSourceFiles( string savePath, List> files) { - if (string.IsNullOrWhiteSpace(savePath) || files == null || files.Count == 0) + if (string.IsNullOrEmpty(savePath) || files == null || files.Count == 0) { return new List(); } + // External client paths are filesystem identifiers, not user text. Do not trim + // whitespace from path segments; only strip separators when intentionally + // converting a rooted-looking child path into a relative child path. return files .Select(file => file.TryGetValue("name", out var nameEl) ? nameEl.GetString() ?? string.Empty : string.Empty) - .Where(name => !string.IsNullOrWhiteSpace(name)) - .Select(name => CombineWithOptionalBase(savePath, name.Replace('/', Path.DirectorySeparatorChar))) + .Where(name => !string.IsNullOrEmpty(name)) + .Select(name => CombineClientReportedPath(savePath, name.Replace('/', Path.DirectorySeparatorChar))) .Distinct(StringComparer.OrdinalIgnoreCase) .ToList(); } public static List BuildTransmissionSourceFiles(string? downloadDir, JsonElement filesElement) { - if (string.IsNullOrWhiteSpace(downloadDir) || filesElement.ValueKind != JsonValueKind.Array) + if (string.IsNullOrEmpty(downloadDir) || filesElement.ValueKind != JsonValueKind.Array) { return new List(); } @@ -56,12 +59,12 @@ public static List BuildTransmissionSourceFiles(string? downloadDir, Jso } var relativePath = nameProp.GetString(); - if (string.IsNullOrWhiteSpace(relativePath)) + if (string.IsNullOrEmpty(relativePath)) { continue; } - sourceFiles.Add(FileUtils.CombineWithOptionalBase(downloadDir, relativePath)); + sourceFiles.Add(CombineClientReportedPath(downloadDir, relativePath)); } return sourceFiles; @@ -71,14 +74,14 @@ public static string ResolveQbittorrentContentPath( string savePath, List> files) { - if (string.IsNullOrWhiteSpace(savePath) || files == null || files.Count == 0) + if (string.IsNullOrEmpty(savePath) || files == null || files.Count == 0) { return string.Empty; } var fileNames = files .Select(f => f.TryGetValue("name", out var nameEl) ? nameEl.GetString() ?? string.Empty : string.Empty) - .Where(name => !string.IsNullOrWhiteSpace(name)) + .Where(name => !string.IsNullOrEmpty(name)) .ToList(); if (fileNames.Count == 0) @@ -93,8 +96,8 @@ public static string ResolveQbittorrentContentPath( if (fileNames.Count == 1) { return hasNestedPath - ? CombineWithOptionalBase(savePath, firstParts[0]) - : CombineWithOptionalBase(savePath, firstFile); + ? CombineClientReportedPath(savePath, firstParts[0]) + : CombineClientReportedPath(savePath, firstFile); } if (!hasNestedPath) @@ -110,34 +113,39 @@ public static string ResolveQbittorrentContentPath( }); return allShareTopLevel - ? CombineWithOptionalBase(savePath, topLevel) + ? CombineClientReportedPath(savePath, topLevel) : savePath; } - private static string CombineWithOptionalBase(string? basePath, string candidatePath) + private static string CombineClientReportedPath(string? basePath, string candidatePath) { - var normalizedPath = candidatePath.Trim(); - - if (string.IsNullOrEmpty(normalizedPath)) + if (string.IsNullOrEmpty(candidatePath) || string.IsNullOrEmpty(basePath)) { - return normalizedPath; + return candidatePath; } - if (Path.IsPathRooted(normalizedPath) || string.IsNullOrWhiteSpace(basePath)) + var relativePath = candidatePath.TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + if (Path.IsPathRooted(relativePath)) { - return normalizedPath; + var root = Path.GetPathRoot(relativePath) ?? string.Empty; + relativePath = relativePath[root.Length..] + .TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); } - - var relativePath = normalizedPath.TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); - if (Path.IsPathRooted(relativePath)) + else if (HasDriveRootedPrefix(relativePath)) { - return relativePath; + relativePath = relativePath[2..] + .TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); } - var normalizedBasePath = basePath.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); - return string.IsNullOrEmpty(normalizedBasePath) - ? relativePath - : normalizedBasePath + Path.DirectorySeparatorChar + relativePath; + return FileUtils.CombineWithOptionalBase(basePath, relativePath); + } + + private static bool HasDriveRootedPrefix(string path) + { + return path.Length >= 2 + && char.IsLetter(path[0]) + && path[1] == ':' + && (path.Length == 2 || path[2] is '/' or '\\'); } } } diff --git a/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentImportPathResolver.cs b/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentImportPathResolver.cs index e43269734..9b89d635c 100644 --- a/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentImportPathResolver.cs +++ b/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentImportPathResolver.cs @@ -31,7 +31,7 @@ public static List BuildSourceFiles( public static List TranslateSourceFiles(IEnumerable sourceFiles) { return sourceFiles - .Where(path => !string.IsNullOrWhiteSpace(path)) + .Where(path => !string.IsNullOrEmpty(path)) .Distinct(StringComparer.OrdinalIgnoreCase) .ToList(); } diff --git a/listenarr.infrastructure/DownloadClients/Transmission/TransmissionImportPathResolver.cs b/listenarr.infrastructure/DownloadClients/Transmission/TransmissionImportPathResolver.cs index 25e124cc1..f05a74fe5 100644 --- a/listenarr.infrastructure/DownloadClients/Transmission/TransmissionImportPathResolver.cs +++ b/listenarr.infrastructure/DownloadClients/Transmission/TransmissionImportPathResolver.cs @@ -29,14 +29,14 @@ public static bool IsExistingLocalPath(string? path) public static string? BuildContentPath(string? downloadDir, string? name, string? fallbackPath = null) { - return !string.IsNullOrWhiteSpace(downloadDir) && !string.IsNullOrWhiteSpace(name) + return !string.IsNullOrEmpty(downloadDir) && !string.IsNullOrEmpty(name) ? FileUtils.CombineWithOptionalBase(downloadDir, name) : fallbackPath; } public static List BuildSourceFiles(string? downloadDir, JsonElement filesElement) { - return [.. TorrentClientPathMapper.BuildTransmissionSourceFiles(downloadDir, filesElement).Where(path => !string.IsNullOrWhiteSpace(path))]; + return [.. TorrentClientPathMapper.BuildTransmissionSourceFiles(downloadDir, filesElement).Where(path => !string.IsNullOrEmpty(path))]; } } } diff --git a/tests/Features/Api/Features/Library/LibraryController_AddToLibraryTests.cs b/tests/Features/Api/Features/Library/LibraryController_AddToLibraryTests.cs index 98bdc6dd3..9ee36589c 100644 --- a/tests/Features/Api/Features/Library/LibraryController_AddToLibraryTests.cs +++ b/tests/Features/Api/Features/Library/LibraryController_AddToLibraryTests.cs @@ -230,7 +230,7 @@ public async Task AddToLibrary_WithCustomPath_StoresCustomPathAsBasePath() { var controller = _provider.GetRequiredService(); - var customPath = "/custom/audiobooks/Author/Series/Title"; + var customPath = Path.Join(tempRoot, "custom", "audiobooks", "Author", "Series", "Title"); var request = new LibraryController.AddToLibraryRequest { Metadata = new AudibleBookMetadata @@ -250,19 +250,41 @@ public async Task AddToLibrary_WithCustomPath_StoresCustomPathAsBasePath() var stored = (await _audiobookRepository.GetAllAsync()).First(); Assert.NotNull(stored); - // NormalizeStoredPath calls Path.GetFullPath which is platform-dependent: - // on Windows "/custom/..." becomes "C:\custom\...", on Linux it stays "/custom/..." var expectedPath = Path.GetFullPath(customPath); Assert.Equal(expectedPath, stored.BasePath); } + [Fact] + public async Task AddToLibrary_RejectsCustomPathParentTraversal() + { + var controller = _provider.GetRequiredService(); + + var parentSegment = new string('.', 2); + var customPath = Path.Join(tempRoot, "Books", parentSegment, "Other"); + var request = new LibraryController.AddToLibraryRequest + { + Metadata = new AudibleBookMetadata + { + Title = "Traversal Path Test", + Author = "Custom Author" + }, + Monitored = true, + DestinationPath = customPath + }; + + var actionResult = await controller.AddToLibrary(request); + + var badRequest = Assert.IsType(actionResult); + Assert.Contains("DestinationPath", badRequest.Value.ToString(), StringComparison.OrdinalIgnoreCase); + Assert.Empty(await _audiobookRepository.GetAllAsync()); + } + [Fact] public async Task AddToLibrary_HandlesWrongCustomPath() { var controller = _provider.GetRequiredService(); var customPath = "/custom/* ?|<>\0/Author/Series/Title"; - Assert.Throws(() => Path.GetFullPath(customPath)); var request = new LibraryController.AddToLibraryRequest { @@ -279,12 +301,9 @@ public async Task AddToLibrary_HandlesWrongCustomPath() var actionResult = await controller.AddToLibrary(request); // Assert - Assert.IsType(actionResult); - - var stored = (await _audiobookRepository.GetAllAsync()).First(); - Assert.NotNull(stored); - // Uses fallback logic with folder naming pattern - Assert.Equal(Path.Join(tempRoot, "Custom Author"), stored.BasePath); + var badRequest = Assert.IsType(actionResult); + Assert.Contains("DestinationPath", badRequest.Value.ToString(), StringComparison.OrdinalIgnoreCase); + Assert.Empty(await _audiobookRepository.GetAllAsync()); } } } diff --git a/tests/Features/Api/Features/Library/LibraryController_MoveTests.cs b/tests/Features/Api/Features/Library/LibraryController_MoveTests.cs index 9b73ae781..3d65512c4 100644 --- a/tests/Features/Api/Features/Library/LibraryController_MoveTests.cs +++ b/tests/Features/Api/Features/Library/LibraryController_MoveTests.cs @@ -150,6 +150,31 @@ await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() Assert.StartsWith(" listenarr-move-dst-", Path.GetFileName(updated.BasePath), StringComparison.Ordinal); } + [Fact] + [Trait("Method", "EnqueueMove")] + [Trait("Scenario", "RejectsInvalidDestinationPath")] + public async Task MoveAudiobook_RejectsInvalidDestinationPath() + { + var sourcePath = FileService.GetTempDirectory("listenarr-move-src"); + var audiobook = await _audiobookRepository.AddAsync(new AudiobookBuilder() + .WithTitle("Test") + .WithBasePath(sourcePath) + .Build()); + + var controller = _provider.GetRequiredService(); + var request = new LibraryController.MoveRequest + { + DestinationPath = Path.Join(FileService.GetTempPath(), "bad\0target"), + MoveFiles = false + }; + + var result = await controller.EnqueueMove(audiobook.Id, request); + + var badObj = Assert.IsAssignableFrom(result); + Assert.Equal(400, badObj.StatusCode); + Assert.Contains("DestinationPath", badObj.Value?.ToString() ?? string.Empty); + } + [Fact] [Trait("Method", "EnqueueMove")] [Trait("Scenario", "RejectsRelativeDestinationOutsideOutputPath")] diff --git a/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs b/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs index 4e031198f..5351b1517 100644 --- a/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs +++ b/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs @@ -50,6 +50,99 @@ public async Task Create_Throws_WhenPathDuplicate() await Assert.ThrowsAsync(() => svc.CreateAsync(new RootFolder { Name = "B", Path = booksPath })); } + [Fact] + public async Task Create_AllowsFilesystemRootPath() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + + var dbFactory = new TestDbFactory(options); + var repo = new EfRootFolderRepository(dbFactory, Mock.Of>()); + var svc = new RootFolderService(repo, null!); + var filesystemRoot = Path.GetPathRoot(FileUtils.GetAbsolutePath("root")); + Assert.False(string.IsNullOrWhiteSpace(filesystemRoot)); + + var created = await svc.CreateAsync(new RootFolder { Name = "Drive Root", Path = filesystemRoot! }); + + Assert.Equal(Path.GetFullPath(filesystemRoot!), created.Path); + } + + [Fact] + public async Task Create_Throws_WhenPathInvalidForCurrentOs() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + + var dbFactory = new TestDbFactory(options); + var repo = new EfRootFolderRepository(dbFactory, Mock.Of>()); + var svc = new RootFolderService(repo, null!); + + var exception = await Assert.ThrowsAsync(() => + svc.CreateAsync(new RootFolder { Name = "Invalid", Path = "relative-root" })); + Assert.Contains("not valid", exception.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task Create_NormalizesPathBeforeStorage() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + + var dbFactory = new TestDbFactory(options); + var repo = new EfRootFolderRepository(dbFactory, Mock.Of>()); + var svc = new RootFolderService(repo, null!); + var rawPath = Path.Join(rootPath, "."); + + var created = await svc.CreateAsync(new RootFolder { Name = "Normalized", Path = rawPath }); + + Assert.Equal(Path.GetFullPath(rawPath), created.Path); + } + + [Fact] + public async Task Create_Throws_WhenNormalizedPathDuplicate() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + + var normalizedPath = Path.GetFullPath(rootPath); + var db = new ListenArrDbContext(options); + db.RootFolders.Add(new RootFolder { Name = "A", Path = normalizedPath }); + await db.SaveChangesAsync(); + + var dbFactory = new TestDbFactory(options); + var repo = new EfRootFolderRepository(dbFactory, Mock.Of>()); + var svc = new RootFolderService(repo, null!); + + await Assert.ThrowsAsync(() => + svc.CreateAsync(new RootFolder { Name = "B", Path = Path.Join(rootPath, ".") })); + } + + [Fact] + public async Task Update_Throws_WhenPathInvalidForCurrentOs() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + + var db = new ListenArrDbContext(options); + var root = new RootFolder { Name = "R", Path = rootPath }; + db.RootFolders.Add(root); + await db.SaveChangesAsync(); + + var dbFactory = new TestDbFactory(options); + var repo = new EfRootFolderRepository(dbFactory, Mock.Of>()); + var logger = new TestLogger(_output); + var svc = new RootFolderService(repo, logger); + + var exception = await Assert.ThrowsAsync(() => + svc.UpdateAsync(new RootFolder { Id = root.Id, Name = "R2", Path = "relative-root" })); + Assert.Contains("not valid", exception.Message, StringComparison.OrdinalIgnoreCase); + } + [Fact] public async Task Delete_Throws_WhenReferencedWithoutReassign() { diff --git a/tests/Features/Application/Downloads/Common/DownloadClientGatewayTests.cs b/tests/Features/Application/Downloads/Common/DownloadClientGatewayTests.cs index 47bbcf4a5..3ce4e2f02 100644 --- a/tests/Features/Application/Downloads/Common/DownloadClientGatewayTests.cs +++ b/tests/Features/Application/Downloads/Common/DownloadClientGatewayTests.cs @@ -293,5 +293,63 @@ public async Task GetQueueItemAsync_UseContentPath_Directory_Empty() Assert.NotNull(item.SourceFiles); Assert.Empty(item.SourceFiles); } + + [Fact] + [Trait("Method", "GetQueueItemAsync")] + [Trait("Scenario", "Remote path mapping preserves whitespace-bearing path segments")] + public async Task GetQueueItemAsync_PreservesWhitespaceAfterRemotePathMapping() + { + var remoteFile = FileUtils.GetAbsolutePath("downloads", " Book Folder ", "chapter1.m4b"); + var expectedLocalFile = Path.Join(localMapping, " Book Folder ", "chapter1.m4b"); + var downloadCLientAdapterMock = (DownloadCLientAdapterMock)((DownloadClientGateway)downloadClientGateway).ResolveAdapter(client); + downloadCLientAdapterMock.QueueItemMock = new QueueItemBuilder() + .WithRemotePath(remoteFile) + .WithContentPath(remoteFile) + .WithSourceFile(remoteFile) + .WithStatus("completed") + .Build(); + + var item = await downloadClientGateway.GetQueueItemAsync(client, new DownloadBuilder().Build(), new QueueItem()); + + Assert.Equal(expectedLocalFile, item.LocalPath); + Assert.Equal(expectedLocalFile, item.ContentPath); + Assert.Equal([expectedLocalFile], item.SourceFiles); + } + + [Fact] + [Trait("Method", "GetQueueItemAsync")] + [Trait("Scenario", "Directory expansion preserves whitespace-bearing filesystem paths")] + public async Task GetQueueItemAsync_ExpandsWhitespaceBearingDirectoryIntoExactSourceFiles() + { + if (OperatingSystem.IsWindows()) + { + return; + } + + var root = Path.Join(Path.GetTempPath(), "listenarr-gateway-whitespace-" + Guid.NewGuid().ToString("N")); + var sourceDirectory = Path.Join(root, " Book Folder "); + Directory.CreateDirectory(sourceDirectory); + var sourceFile = Path.Join(sourceDirectory, "chapter1.m4b"); + await File.WriteAllTextAsync(sourceFile, "audio"); + + try + { + var downloadCLientAdapterMock = (DownloadCLientAdapterMock)((DownloadClientGateway)downloadClientGateway).ResolveAdapter(client); + downloadCLientAdapterMock.QueueItemMock = new QueueItemBuilder() + .WithContentPath(sourceDirectory) + .WithStatus("completed") + .Build(); + + var item = await downloadClientGateway.GetQueueItemAsync(client, new DownloadBuilder().Build(), new QueueItem()); + var actual = Assert.Single(item.SourceFiles); + + Assert.Equal(FileUtils.NormalizeStoredPath(sourceFile), actual); + Assert.True(File.Exists(actual)); + } + finally + { + try { Directory.Delete(root, true); } catch (IOException ex) { System.Diagnostics.Debug.WriteLine(ex.Message); } catch (UnauthorizedAccessException ex) { System.Diagnostics.Debug.WriteLine(ex.Message); } + } + } } } diff --git a/tests/Features/Domain/Utils/FileUtilsTests.cs b/tests/Features/Domain/Utils/FileUtilsTests.cs index c5a0b9eec..7292f25a0 100644 --- a/tests/Features/Domain/Utils/FileUtilsTests.cs +++ b/tests/Features/Domain/Utils/FileUtilsTests.cs @@ -335,6 +335,56 @@ public void CombineWithOptionalBase_PreservesPathWhitespace() result); } + [Fact] + public void CombineWithOptionalBase_PreservesNestedPathSegmentWhitespace() + { + var result = FileUtils.CombineWithOptionalBase( + FileUtils.GetAbsolutePath("downloads"), + " Book Folder / chapter 01.m4b "); + + Assert.Equal( + FileUtils.GetAbsolutePath("downloads") + Path.DirectorySeparatorChar + " Book Folder / chapter 01.m4b ", + result); + } + + [Fact] + public void CombineWithOptionalBase_PreservesRootedCandidatePath() + { + var candidate = Path.DirectorySeparatorChar + " Book Folder /chapter.m4b"; + + var result = FileUtils.CombineWithOptionalBase( + FileUtils.GetAbsolutePath("downloads"), + candidate); + + Assert.Equal(candidate, result); + } + + [Fact] + public void NormalizeStoredPath_DoesNotTrimPathWhitespace_OnNonWindows() + { + if (OperatingSystem.IsWindows()) + { + return; + } + + var root = Path.Join(Path.GetTempPath(), "listenarr-path-whitespace-" + Guid.NewGuid().ToString("N")); + var whitespaceSegment = " Book Folder "; + var directory = Path.Join(root, whitespaceSegment); + Directory.CreateDirectory(directory); + + try + { + var normalized = FileUtils.NormalizeStoredPath(directory); + + Assert.EndsWith(Path.DirectorySeparatorChar + whitespaceSegment, normalized, StringComparison.Ordinal); + Assert.True(Directory.Exists(normalized)); + } + finally + { + try { Directory.Delete(root, true); } catch (IOException ex) { System.Diagnostics.Debug.WriteLine(ex.Message); } catch (UnauthorizedAccessException ex) { System.Diagnostics.Debug.WriteLine(ex.Message); } + } + } + [Fact] public void CombineRelativePath_JoinsRelativeSegmentsAndTrimsLeadingSeparators() { @@ -478,6 +528,134 @@ public void TryValidateMutationTarget_BlocksDirectorySymlinkEscape() } } + [Theory] + [InlineData(@"C:\Books\Author", true)] + [InlineData(@"C:\", false)] + [InlineData(@"Books\Author", false)] + [InlineData(@"C:\Books\Author ", false)] + [InlineData(@"C:\Books\Author.", false)] + [InlineData(@"C:\Books\NUL", false)] + [InlineData(@"C:\Books\COM1.txt", false)] + [InlineData(@"C:\Books\Bad|Name", false)] + public void TryNormalizeUserProvidedDirectoryPathForOs_UsesWindowsRules(string path, bool expected) + { + var valid = FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + path, + isWindows: true, + out var normalizedPath, + out var reason); + + Assert.Equal(expected, valid); + if (expected) + { + Assert.False(string.IsNullOrWhiteSpace(normalizedPath)); + Assert.Equal(string.Empty, reason); + } + else + { + Assert.False(string.IsNullOrWhiteSpace(reason)); + } + } + + [Fact] + public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsWindowsRootWhenExplicitlyRequested() + { + var separator = new string((char)92, 1); + var driveRoot = "C:" + separator; + var uncRoot = separator + separator + "server" + separator + "share"; + + Assert.True(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + driveRoot, + isWindows: true, + out var normalizedDriveRoot, + out var driveRootReason, + allowFileSystemRoot: true)); + Assert.Equal(string.Empty, driveRootReason); + Assert.Equal("C:", normalizedDriveRoot.TrimEnd((char)92)); + + Assert.True(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + uncRoot, + isWindows: true, + out var normalizedUncRoot, + out var uncRootReason, + allowFileSystemRoot: true)); + Assert.Equal(string.Empty, uncRootReason); + Assert.Contains("server", normalizedUncRoot, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsParentTraversalForDestinations() + { + var separator = new string((char)92, 1); + var windowsTraversal = "C:" + separator + "Books" + separator + ".." + separator + "Other"; + + Assert.False(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + windowsTraversal, + isWindows: true, + out _, + out var windowsReason, + rejectParentTraversal: true)); + Assert.Contains("parent", windowsReason, StringComparison.OrdinalIgnoreCase); + + Assert.False(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + "/media/../other", + isWindows: false, + out _, + out var unixReason, + rejectParentTraversal: true)); + Assert.Contains("parent", unixReason, StringComparison.OrdinalIgnoreCase); + } + + [Theory] + [InlineData("/media/Author", true)] + [InlineData("/media/./Author", true)] + [InlineData("/media/Author ", true)] + [InlineData("/media/NUL", true)] + [InlineData("/media/..", false)] + [InlineData("media/Author", false)] + [InlineData("/", false)] + [InlineData("", false)] + public void TryNormalizeUserProvidedDirectoryPathForOs_UsesUnixRules(string path, bool expected) + { + var valid = FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + path, + isWindows: false, + out var normalizedPath, + out var reason); + + Assert.Equal(expected, valid); + if (expected) + { + Assert.False(string.IsNullOrWhiteSpace(normalizedPath)); + Assert.Equal(string.Empty, reason); + } + else + { + Assert.False(string.IsNullOrWhiteSpace(reason)); + } + } + + [Fact] + public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsNullCharacter() + { + Assert.False(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + "/media/book\0folder", + isWindows: false, + out _, + out var reason)); + Assert.Contains("invalid", reason, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void TryNormalizeUserProvidedDirectoryPathForCurrentOs_NormalizesCurrentHostPath() + { + var path = Path.Join(Path.GetTempPath(), "listenarr-normalize-" + Guid.NewGuid().ToString("N")); + + Assert.True(FileUtils.TryNormalizeUserProvidedDirectoryPathForCurrentOs(path, out var normalizedPath, out var reason)); + Assert.Equal(Path.GetFullPath(path), normalizedPath); + Assert.Equal(string.Empty, reason); + } + [Fact] public async Task FilesHaveSameContentAsync_UsesSizeAndHash() { diff --git a/tests/Features/Infrastructure/DownloadClients/Common/DownloadClientAdapterTests.cs b/tests/Features/Infrastructure/DownloadClients/Common/DownloadClientAdapterTests.cs index f1f0bd3ad..717204718 100644 --- a/tests/Features/Infrastructure/DownloadClients/Common/DownloadClientAdapterTests.cs +++ b/tests/Features/Infrastructure/DownloadClients/Common/DownloadClientAdapterTests.cs @@ -121,6 +121,33 @@ public async Task Transmission_LegacyGetImportItemAsync_PopulatesClientReportedS }, resolved.SourceFiles); } + [Fact] + [Trait("Third-Party", "Transmission")] + [Trait("Method", "GetImportItemAsync")] + public async Task Transmission_LegacyGetImportItemAsync_PreservesWhitespaceBearingFolderPaths() + { + var item = new QueueItem + { + Id = TransmissionApiMock.WHITESPACE_FOLDER_TORRENT.ToString(), + ContentPath = string.Empty + }; + + var download = new DownloadBuilder().Build(); + await _downloadRepository.AddAsync(download); + + var adapter = MockUtils.CreateTransmissionAdapter(_provider); + var resolved = await adapter.GetImportItemAsync(_transmissionClient, download, item); + + Assert.Equal(FileUtils.GetAbsolutePath("downloads", " Book Folder "), resolved.ContentPath); + Assert.Equal( + new[] + { + FileUtils.GetAbsolutePath("downloads", " Book Folder ", "chapter1.m4b"), + FileUtils.GetAbsolutePath("downloads", " Book Folder ", "book.txt") + }, + resolved.SourceFiles); + } + [Theory] [Trait("Third-Party", "Sabnzbd")] [Trait("Method", "GetImportItemAsync")] diff --git a/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs b/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs new file mode 100644 index 000000000..cb8782860 --- /dev/null +++ b/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs @@ -0,0 +1,119 @@ +using System.Text.Json; +using Listenarr.Infrastructure.DownloadClients.Common; +using Listenarr.Tests.Common; + +namespace Listenarr.Tests.Features.Infrastructure.DownloadClients.Common +{ + [Trait("Name", "TorrentClientPathMapperTests")] + [Trait("Category", "DownloadClientPathMapping")] + public class TorrentClientPathMapperTests : BaseTests + { + [Fact] + public void BuildTransmissionSourceFiles_PreservesTopLevelFolderWhitespace() + { + var downloadDir = FileUtils.GetAbsolutePath("downloads"); + using var document = JsonDocument.Parse( + """ + [ + { "name": " Book Folder /chapter1.m4b" } + ] + """); + + var sourceFiles = TorrentClientPathMapper.BuildTransmissionSourceFiles(downloadDir, document.RootElement); + + var expected = FileUtils.CombineWithOptionalBase(downloadDir, " Book Folder /chapter1.m4b"); + Assert.Equal([expected], sourceFiles); + Assert.StartsWith(downloadDir + Path.DirectorySeparatorChar, expected, StringComparison.Ordinal); + Assert.Contains(" Book Folder ", expected, StringComparison.Ordinal); + } + + [Fact] + public void BuildTransmissionSourceFiles_PreservesTrailingFolderWhitespace() + { + var downloadDir = FileUtils.GetAbsolutePath("downloads"); + using var document = JsonDocument.Parse( + """ + [ + { "name": "Book Folder /chapter1.m4b" } + ] + """); + + var sourceFiles = TorrentClientPathMapper.BuildTransmissionSourceFiles(downloadDir, document.RootElement); + + var expected = FileUtils.CombineWithOptionalBase(downloadDir, "Book Folder /chapter1.m4b"); + Assert.Equal([expected], sourceFiles); + Assert.StartsWith(downloadDir + Path.DirectorySeparatorChar, expected, StringComparison.Ordinal); + Assert.Contains("Book Folder ", expected, StringComparison.Ordinal); + } + + [Fact] + public void BuildQbittorrentSourceFiles_PreservesTorrentFolderWhitespace() + { + var savePath = FileUtils.GetAbsolutePath("downloads"); + var files = ParseFiles( + """ + [ + { "name": " Book Folder /chapter1.m4b" } + ] + """); + + var sourceFiles = TorrentClientPathMapper.BuildQbittorrentSourceFiles(savePath, files); + + var expected = Path.Join(savePath, " Book Folder ", "chapter1.m4b"); + Assert.Equal([expected], sourceFiles); + } + + [Fact] + public void ResolveQbittorrentContentPath_PreservesSharedTopLevelFolderWhitespace() + { + var savePath = FileUtils.GetAbsolutePath("downloads"); + var files = ParseFiles( + """ + [ + { "name": " Book Folder /chapter1.m4b" }, + { "name": " Book Folder /chapter2.m4b" } + ] + """); + + var contentPath = TorrentClientPathMapper.ResolveQbittorrentContentPath(savePath, files); + + Assert.Equal(Path.Join(savePath, " Book Folder "), contentPath); + } + + [Fact] + public void BuildQbittorrentSourceFiles_RootedChildPathsStayUnderSavePath() + { + var savePath = FileUtils.GetAbsolutePath("downloads"); + var files = ParseFiles( + """ + [ + { "name": "/ Book Folder /chapter1.m4b" } + ] + """); + + var sourceFile = Assert.Single(TorrentClientPathMapper.BuildQbittorrentSourceFiles(savePath, files)); + + Assert.Equal(Path.Join(savePath, " Book Folder ", "chapter1.m4b"), sourceFile); + Assert.True(FileUtils.IsPathSameOrInside(sourceFile, savePath)); + } + + private static List> ParseFiles(string json) + { + using var document = JsonDocument.Parse(json); + var files = new List>(); + + foreach (var element in document.RootElement.EnumerateArray()) + { + var map = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var property in element.EnumerateObject()) + { + map[property.Name] = property.Value.Clone(); + } + + files.Add(map); + } + + return files; + } + } +} diff --git a/tests/Mocks/Api/TransmissionApiMock.cs b/tests/Mocks/Api/TransmissionApiMock.cs index 3b41474f4..8a255c945 100644 --- a/tests/Mocks/Api/TransmissionApiMock.cs +++ b/tests/Mocks/Api/TransmissionApiMock.cs @@ -8,6 +8,7 @@ public class TransmissionApiMock : BaseApiMock public static readonly int SINGLE_FILE_TORRENT = 1; public static readonly int ANOTHER_SINGLE_FILE_TORRENT = 306; public static readonly int MULTI_FILE_TORRENT = 2; + public static readonly int WHITESPACE_FOLDER_TORRENT = 528; public TransmissionApiMock() { @@ -42,6 +43,10 @@ public static async Task GetTorrent(HttpRequestMessage requ { return AnotherSingleFileTorrentGet(); } + else if (id == WHITESPACE_FOLDER_TORRENT) + { + return WhitespaceFolderTorrentGet(); + } } var response = """ @@ -195,5 +200,36 @@ private static HttpResponseMessage MultiFileTorrentGet() response = MockUtils.PutPathInResponse(response, "{{FILE2}}", Path.Join("Book Folder", "book.txt")); return MockUtils.GetCannedResponse(response); } + + private static HttpResponseMessage WhitespaceFolderTorrentGet() + { + var response = """ + { + "arguments": { + "torrents": [ + { + "id": 528, + "name": " Book Folder ", + "downloadDir": "{{DIR}}", + "files": [ + { + "name": "{{FILE1}}" + }, + { + "name": "{{FILE2}}" + } + ] + } + ] + }, + "result": "success", + "tag": 3 + } + """; + response = MockUtils.PutPathInResponse(response, "{{DIR}}", FileUtils.GetAbsolutePath("downloads")); + response = MockUtils.PutPathInResponse(response, "{{FILE1}}", Path.Join(" Book Folder ", "chapter1.m4b")); + response = MockUtils.PutPathInResponse(response, "{{FILE2}}", Path.Join(" Book Folder ", "book.txt")); + return MockUtils.GetCannedResponse(response); + } } } From 6d0f2658eb8ebdefca4741fbfd0ed06fc9e409b9 Mon Sep 17 00:00:00 2001 From: Robbie Davis Date: Wed, 1 Jul 2026 15:06:52 -0400 Subject: [PATCH 002/158] fix(paths): allow root-folder filesystem roots Allow root-folder configuration to accept filesystem roots, including Windows current-drive roots, while keeping concrete destination paths strict against root-only and parent-traversal targets. Add cross-platform root-folder validation coverage and standardize the new torrent path mapper test header. --- .../Common/FileUtils.UserProvidedPaths.cs | 30 ++++++++++ .../RootFolders/RootFolderServiceTests.cs | 22 ++++++++ tests/Features/Domain/Utils/FileUtilsTests.cs | 55 ++++++++++++++++++- .../Common/TorrentClientPathMapperTests.cs | 17 ++++++ 4 files changed, 123 insertions(+), 1 deletion(-) diff --git a/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs b/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs index 4131c121e..ae6944c58 100644 --- a/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs +++ b/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs @@ -97,6 +97,31 @@ private static bool TryNormalizeWindowsUserProvidedDirectoryPath( normalizedPath = string.Empty; reason = string.Empty; + // Windows accepts \ or / as the current drive root. Root-folder configuration may + // intentionally use that boundary, but concrete destinations must still reject it. + if (IsWindowsCurrentDriveRoot(path)) + { + if (!allowFileSystemRoot) + { + reason = "Path cannot be the filesystem root."; + return false; + } + + try + { + normalizedPath = OperatingSystem.IsWindows() + ? Path.GetFullPath(path) + : path.Replace('/', '\\'); + return true; + } + catch (Exception exception) when (exception is not (OperationCanceledException or OutOfMemoryException or StackOverflowException)) + { + normalizedPath = string.Empty; + reason = "Path is not valid for this operating system."; + return false; + } + } + var rootLength = GetWindowsRootLength(path); if (rootLength <= 0) { @@ -190,6 +215,11 @@ private static bool TryNormalizeUnixUserProvidedDirectoryPath( } } + private static bool IsWindowsCurrentDriveRoot(string path) + { + return path.Length == 1 && (path[0] is '\\' or '/'); + } + private static int GetWindowsRootLength(string path) { if (WindowsDriveRootPattern.IsMatch(path)) diff --git a/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs b/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs index 5351b1517..dfcdc10aa 100644 --- a/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs +++ b/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs @@ -68,6 +68,28 @@ public async Task Create_AllowsFilesystemRootPath() Assert.Equal(Path.GetFullPath(filesystemRoot!), created.Path); } + [Fact] + public async Task Create_AllowsWindowsCurrentDriveRootPath() + { + if (!OperatingSystem.IsWindows()) + { + return; + } + + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + + var dbFactory = new TestDbFactory(options); + var repo = new EfRootFolderRepository(dbFactory, Mock.Of>()); + var svc = new RootFolderService(repo, null!); + var currentDriveRoot = new string((char)92, 1); + + var created = await svc.CreateAsync(new RootFolder { Name = "Current Drive Root", Path = currentDriveRoot }); + + Assert.Equal(Path.GetFullPath(currentDriveRoot), created.Path); + } + [Fact] public async Task Create_Throws_WhenPathInvalidForCurrentOs() { diff --git a/tests/Features/Domain/Utils/FileUtilsTests.cs b/tests/Features/Domain/Utils/FileUtilsTests.cs index 7292f25a0..b8a891e29 100644 --- a/tests/Features/Domain/Utils/FileUtilsTests.cs +++ b/tests/Features/Domain/Utils/FileUtilsTests.cs @@ -563,6 +563,7 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsWindowsRootWhenExpl var separator = new string((char)92, 1); var driveRoot = "C:" + separator; var uncRoot = separator + separator + "server" + separator + "share"; + var currentDriveRoot = separator; Assert.True(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( driveRoot, @@ -571,7 +572,7 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsWindowsRootWhenExpl out var driveRootReason, allowFileSystemRoot: true)); Assert.Equal(string.Empty, driveRootReason); - Assert.Equal("C:", normalizedDriveRoot.TrimEnd((char)92)); + Assert.False(string.IsNullOrWhiteSpace(normalizedDriveRoot)); Assert.True(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( uncRoot, @@ -581,6 +582,44 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsWindowsRootWhenExpl allowFileSystemRoot: true)); Assert.Equal(string.Empty, uncRootReason); Assert.Contains("server", normalizedUncRoot, StringComparison.OrdinalIgnoreCase); + + Assert.True(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + currentDriveRoot, + isWindows: true, + out var normalizedCurrentDriveRoot, + out var currentDriveRootReason, + allowFileSystemRoot: true)); + Assert.Equal(string.Empty, currentDriveRootReason); + Assert.False(string.IsNullOrWhiteSpace(normalizedCurrentDriveRoot)); + + Assert.True(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + "/", + isWindows: true, + out var normalizedForwardSlashRoot, + out var forwardSlashRootReason, + allowFileSystemRoot: true)); + Assert.Equal(string.Empty, forwardSlashRootReason); + Assert.False(string.IsNullOrWhiteSpace(normalizedForwardSlashRoot)); + } + + [Fact] + public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsWindowsRootByDefault() + { + var separator = new string((char)92, 1); + + Assert.False(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + separator, + isWindows: true, + out _, + out var currentDriveRootReason)); + Assert.Contains("root", currentDriveRootReason, StringComparison.OrdinalIgnoreCase); + + Assert.False(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + "/", + isWindows: true, + out _, + out var forwardSlashRootReason)); + Assert.Contains("root", forwardSlashRootReason, StringComparison.OrdinalIgnoreCase); } [Fact] @@ -606,6 +645,20 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsParentTraversalFor Assert.Contains("parent", unixReason, StringComparison.OrdinalIgnoreCase); } + [Fact] + public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsUnixRootWhenExplicitlyRequested() + { + Assert.True(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + "/", + isWindows: false, + out var normalizedRoot, + out var reason, + allowFileSystemRoot: true)); + + Assert.Equal(string.Empty, reason); + Assert.Equal("/", normalizedRoot); + } + [Theory] [InlineData("/media/Author", true)] [InlineData("/media/./Author", true)] diff --git a/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs b/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs index cb8782860..91a1927cd 100644 --- a/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs +++ b/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs @@ -1,3 +1,20 @@ +/* + * Listenarr - Audiobook Management System + * Copyright (C) 2024-2026 Listenarr Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ using System.Text.Json; using Listenarr.Infrastructure.DownloadClients.Common; using Listenarr.Tests.Common; From 6290b23dc95719b02307e888cb2b14e4867f8e3f Mon Sep 17 00:00:00 2001 From: Robbie Davis Date: Wed, 1 Jul 2026 18:29:59 -0400 Subject: [PATCH 003/158] fix(paths): reject parent traversal broadly --- .../RootFolders/RootFolderService.cs | 5 +- .../Common/FileUtils.UserProvidedPaths.cs | 2 + .../Common/TorrentClientPathMapper.cs | 19 ++++- .../LibraryController_AddToLibraryTests.cs | 27 +++++++ .../RootFolders/RootFolderServiceTests.cs | 42 +++++++++++ tests/Features/Domain/Utils/FileUtilsTests.cs | 75 +++++++++++++++++++ .../Common/TorrentClientPathMapperTests.cs | 60 +++++++++++++++ 7 files changed, 228 insertions(+), 2 deletions(-) diff --git a/listenarr.application/Audiobooks/RootFolders/RootFolderService.cs b/listenarr.application/Audiobooks/RootFolders/RootFolderService.cs index 94a7ce3b7..ca4d2b782 100644 --- a/listenarr.application/Audiobooks/RootFolders/RootFolderService.cs +++ b/listenarr.application/Audiobooks/RootFolders/RootFolderService.cs @@ -153,11 +153,14 @@ public async Task UpdateAsync(RootFolder root, bool moveFiles = fals private static string NormalizeRootFolderPathForStorage(string? path) { + // Root folders may be filesystem boundaries, but parent traversal is still + // rejected so the stored boundary is explicit rather than reached indirectly. if (!FileUtils.TryNormalizeUserProvidedDirectoryPathForCurrentOs( path, out var normalizedPath, out var validationReason, - allowFileSystemRoot: true)) + allowFileSystemRoot: true, + rejectParentTraversal: true)) { throw new ArgumentException($"Path is not valid for this operating system: {validationReason}"); } diff --git a/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs b/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs index ae6944c58..73b4ad963 100644 --- a/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs +++ b/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs @@ -45,6 +45,8 @@ public static bool TryNormalizeUserProvidedDirectoryPathForCurrentOs( allowFileSystemRoot, rejectParentTraversal); + // The explicit OS parameter lets tests verify Windows and Unix validation rules + // from any host. Production callers should use TryNormalizeUserProvidedDirectoryPathForCurrentOs. public static bool TryNormalizeUserProvidedDirectoryPathForOs( string? path, bool isWindows, diff --git a/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs b/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs index 3fa0cd9a8..fe50f2524 100644 --- a/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs +++ b/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs @@ -39,6 +39,7 @@ public static List BuildQbittorrentSourceFiles( .Select(file => file.TryGetValue("name", out var nameEl) ? nameEl.GetString() ?? string.Empty : string.Empty) .Where(name => !string.IsNullOrEmpty(name)) .Select(name => CombineClientReportedPath(savePath, name.Replace('/', Path.DirectorySeparatorChar))) + .Where(path => !string.IsNullOrEmpty(path)) .Distinct(StringComparer.OrdinalIgnoreCase) .ToList(); } @@ -64,7 +65,11 @@ public static List BuildTransmissionSourceFiles(string? downloadDir, Jso continue; } - sourceFiles.Add(CombineClientReportedPath(downloadDir, relativePath)); + var sourceFile = CombineClientReportedPath(downloadDir, relativePath); + if (!string.IsNullOrEmpty(sourceFile)) + { + sourceFiles.Add(sourceFile); + } } return sourceFiles; @@ -82,6 +87,7 @@ public static string ResolveQbittorrentContentPath( var fileNames = files .Select(f => f.TryGetValue("name", out var nameEl) ? nameEl.GetString() ?? string.Empty : string.Empty) .Where(name => !string.IsNullOrEmpty(name)) + .Where(name => !ContainsParentDirectorySegment(name)) .ToList(); if (fileNames.Count == 0) @@ -137,9 +143,20 @@ private static string CombineClientReportedPath(string? basePath, string candida .TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); } + if (ContainsParentDirectorySegment(relativePath)) + { + return string.Empty; + } + return FileUtils.CombineWithOptionalBase(basePath, relativePath); } + private static bool ContainsParentDirectorySegment(string path) + { + return path.Split(new[] { '/', '\\' }, StringSplitOptions.RemoveEmptyEntries) + .Any(segment => segment.Length == 2 && segment[0] == '.' && segment[1] == '.'); + } + private static bool HasDriveRootedPrefix(string path) { return path.Length >= 2 diff --git a/tests/Features/Api/Features/Library/LibraryController_AddToLibraryTests.cs b/tests/Features/Api/Features/Library/LibraryController_AddToLibraryTests.cs index 9ee36589c..dc7308d71 100644 --- a/tests/Features/Api/Features/Library/LibraryController_AddToLibraryTests.cs +++ b/tests/Features/Api/Features/Library/LibraryController_AddToLibraryTests.cs @@ -80,6 +80,33 @@ public async Task AddToLibrary_UsesLegacyAuthorField_PopulatesAuthorsAndBasePath Assert.Equal(Path.Join(tempRoot, "Legacy Author"), stored.BasePath); } + [Fact] + public async Task AddToLibrary_WithGeneratedPathFromSanitizedMetadata_Succeeds() + { + await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() + .WithFolderNamingPattern("{Author}/{Title}") + .WithFileNamingPattern("{Title}") + .Build()); + var controller = _provider.GetRequiredService(); + + var request = new LibraryController.AddToLibraryRequest + { + Metadata = new AudibleBookMetadata + { + Title = "Book: The Ending.", + Author = "CON" + }, + Monitored = true + }; + + var actionResult = await controller.AddToLibrary(request); + + Assert.IsType(actionResult); + var stored = (await _audiobookRepository.GetAllAsync()).First(); + Assert.NotNull(stored); + Assert.Equal(Path.Join(tempRoot, "CON_", "Book - The Ending"), stored.BasePath); + } + [Fact] public async Task AddToLibrary_PersistsEditableMetadataFields() { diff --git a/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs b/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs index dfcdc10aa..803dec04d 100644 --- a/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs +++ b/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs @@ -106,6 +106,24 @@ public async Task Create_Throws_WhenPathInvalidForCurrentOs() Assert.Contains("not valid", exception.Message, StringComparison.OrdinalIgnoreCase); } + [Fact] + public async Task Create_Throws_WhenRootFolderPathContainsParentTraversal() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + + var dbFactory = new TestDbFactory(options); + var repo = new EfRootFolderRepository(dbFactory, Mock.Of>()); + var svc = new RootFolderService(repo, null!); + var parentSegment = new string('.', 2); + var traversingPath = Path.Join(rootPath, "Audiobooks", parentSegment, "Shared"); + + var exception = await Assert.ThrowsAsync(() => + svc.CreateAsync(new RootFolder { Name = "Traversal Root", Path = traversingPath })); + Assert.Contains("parent", exception.Message, StringComparison.OrdinalIgnoreCase); + } + [Fact] public async Task Create_NormalizesPathBeforeStorage() { @@ -165,6 +183,30 @@ public async Task Update_Throws_WhenPathInvalidForCurrentOs() Assert.Contains("not valid", exception.Message, StringComparison.OrdinalIgnoreCase); } + [Fact] + public async Task Update_Throws_WhenRootFolderPathContainsParentTraversal() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .Options; + + var db = new ListenArrDbContext(options); + var root = new RootFolder { Name = "R", Path = rootPath }; + db.RootFolders.Add(root); + await db.SaveChangesAsync(); + + var dbFactory = new TestDbFactory(options); + var repo = new EfRootFolderRepository(dbFactory, Mock.Of>()); + var logger = new TestLogger(_output); + var svc = new RootFolderService(repo, logger); + var parentSegment = new string('.', 2); + var traversingPath = Path.Join(rootPath, "Audiobooks", parentSegment, "Shared"); + + var exception = await Assert.ThrowsAsync(() => + svc.UpdateAsync(new RootFolder { Id = root.Id, Name = "R2", Path = traversingPath })); + Assert.Contains("parent", exception.Message, StringComparison.OrdinalIgnoreCase); + } + [Fact] public async Task Delete_Throws_WhenReferencedWithoutReassign() { diff --git a/tests/Features/Domain/Utils/FileUtilsTests.cs b/tests/Features/Domain/Utils/FileUtilsTests.cs index b8a891e29..6186fe8c2 100644 --- a/tests/Features/Domain/Utils/FileUtilsTests.cs +++ b/tests/Features/Domain/Utils/FileUtilsTests.cs @@ -645,6 +645,81 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsParentTraversalFor Assert.Contains("parent", unixReason, StringComparison.OrdinalIgnoreCase); } + [Fact] + public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsRootFolderParentTraversal() + { + var separator = new string((char)92, 1); + var parentSegment = new string('.', 2); + var windowsTraversal = "C:" + separator + "Books" + separator + parentSegment + separator + "Other"; + + Assert.False(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + windowsTraversal, + isWindows: true, + out _, + out var windowsReason, + allowFileSystemRoot: true, + rejectParentTraversal: true)); + Assert.Contains("parent", windowsReason, StringComparison.OrdinalIgnoreCase); + + var unixTraversal = string.Join('/', string.Empty, "media", parentSegment, "other"); + Assert.False(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + unixTraversal, + isWindows: false, + out _, + out var unixReason, + allowFileSystemRoot: true, + rejectParentTraversal: true)); + Assert.Contains("parent", unixReason, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsRootsWhenExplicitlyRequestedAndTraversalRejected() + { + var separator = new string((char)92, 1); + var driveRoot = "C:" + separator; + var uncRoot = separator + separator + "server" + separator + "share"; + + Assert.True(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + driveRoot, + isWindows: true, + out var normalizedDriveRoot, + out var driveRootReason, + allowFileSystemRoot: true, + rejectParentTraversal: true)); + Assert.Equal(string.Empty, driveRootReason); + Assert.False(string.IsNullOrWhiteSpace(normalizedDriveRoot)); + + Assert.True(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + separator, + isWindows: true, + out var normalizedCurrentDriveRoot, + out var currentDriveRootReason, + allowFileSystemRoot: true, + rejectParentTraversal: true)); + Assert.Equal(string.Empty, currentDriveRootReason); + Assert.False(string.IsNullOrWhiteSpace(normalizedCurrentDriveRoot)); + + Assert.True(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + uncRoot, + isWindows: true, + out var normalizedUncRoot, + out var uncRootReason, + allowFileSystemRoot: true, + rejectParentTraversal: true)); + Assert.Equal(string.Empty, uncRootReason); + Assert.False(string.IsNullOrWhiteSpace(normalizedUncRoot)); + + Assert.True(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( + "/", + isWindows: false, + out var normalizedUnixRoot, + out var unixRootReason, + allowFileSystemRoot: true, + rejectParentTraversal: true)); + Assert.Equal(string.Empty, unixRootReason); + Assert.Equal("/", normalizedUnixRoot); + } + [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsUnixRootWhenExplicitlyRequested() { diff --git a/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs b/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs index 91a1927cd..dbcb504d4 100644 --- a/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs +++ b/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs @@ -114,6 +114,66 @@ public void BuildQbittorrentSourceFiles_RootedChildPathsStayUnderSavePath() Assert.True(FileUtils.IsPathSameOrInside(sourceFile, savePath)); } + [Fact] + public void BuildQbittorrentSourceFiles_DropsParentTraversalChildPaths() + { + var savePath = FileUtils.GetAbsolutePath("downloads"); + var parentSegment = new string('.', 2); + var files = ParseFiles( + $$""" + [ + { "name": "{{parentSegment}}/escape.m4b" }, + { "name": " Book Folder /chapter1.m4b" } + ] + """); + + var sourceFiles = TorrentClientPathMapper.BuildQbittorrentSourceFiles(savePath, files); + + var sourceFile = Assert.Single(sourceFiles); + Assert.Equal(Path.Join(savePath, " Book Folder ", "chapter1.m4b"), sourceFile); + Assert.True(FileUtils.IsPathSameOrInside(sourceFile, savePath)); + } + + [Fact] + public void BuildTransmissionSourceFiles_DropsParentTraversalChildPaths() + { + var downloadDir = FileUtils.GetAbsolutePath("downloads"); + var parentSegment = new string('.', 2); + using var document = JsonDocument.Parse( + $$""" + [ + { "name": "Book/{{parentSegment}}/{{parentSegment}}/escape.m4b" }, + { "name": " Book Folder /chapter1.m4b" } + ] + """); + + var sourceFiles = TorrentClientPathMapper.BuildTransmissionSourceFiles(downloadDir, document.RootElement); + + var sourceFile = Assert.Single(sourceFiles); + Assert.Equal(FileUtils.CombineWithOptionalBase(downloadDir, " Book Folder /chapter1.m4b"), sourceFile); + Assert.True(FileUtils.IsPathSameOrInside(sourceFile, downloadDir)); + } + + [Fact] + public void ResolveQbittorrentContentPath_IgnoresParentTraversalTopLevelPath() + { + var savePath = FileUtils.GetAbsolutePath("downloads"); + var parentSegment = new string('.', 2); + var files = ParseFiles( + $$""" + [ + { "name": "{{parentSegment}}/escape.m4b" }, + { "name": " Book Folder /chapter1.m4b" }, + { "name": " Book Folder /chapter2.m4b" } + ] + """); + + var contentPath = TorrentClientPathMapper.ResolveQbittorrentContentPath(savePath, files); + + Assert.Equal(Path.Join(savePath, " Book Folder "), contentPath); + Assert.True(FileUtils.IsPathSameOrInside(contentPath, savePath)); + } + private static List> ParseFiles(string json) { using var document = JsonDocument.Parse(json); From 8f5878587b829179269225c41ff5391cce111d36 Mon Sep 17 00:00:00 2001 From: Robbie Davis Date: Wed, 1 Jul 2026 20:46:25 -0400 Subject: [PATCH 004/158] fix(paths): constrain library move destinations --- .../Features/Library/LibraryMoveWorkflow.cs | 85 +++++++++- .../Common/FileUtils.UserProvidedPaths.cs | 7 +- .../Common/TorrentClientPathMapper.cs | 10 +- .../Library/LibraryController_MoveTests.cs | 151 +++++++++++++++++- .../RootFolders/RootFolderServiceTests.cs | 2 +- tests/Features/Domain/Utils/FileUtilsTests.cs | 27 +++- 6 files changed, 259 insertions(+), 23 deletions(-) diff --git a/listenarr.api/Features/Library/LibraryMoveWorkflow.cs b/listenarr.api/Features/Library/LibraryMoveWorkflow.cs index d974b6e58..50f86da00 100644 --- a/listenarr.api/Features/Library/LibraryMoveWorkflow.cs +++ b/listenarr.api/Features/Library/LibraryMoveWorkflow.cs @@ -59,10 +59,45 @@ public async Task EnqueueAsync(int id, LibraryController.MoveRequ { using var scope = _scopeFactory.CreateScope(); var configService = scope.ServiceProvider.GetRequiredService(); + var rootFolderService = scope.ServiceProvider.GetRequiredService(); var settings = await configService.GetApplicationSettingsAsync(); + var rootFolders = await rootFolderService.GetAllAsync(); + + var allowedMoveRoots = new List(); + var normalizedOutputPath = TryNormalizeMoveRoot(settings.OutputPath, "configured output path"); + AddAllowedMoveRoot(allowedMoveRoots, normalizedOutputPath); + + string? defaultRootPath = null; + foreach (var rootFolder in rootFolders) + { + var normalizedRootPath = TryNormalizeMoveRoot(rootFolder.Path, $"root folder {rootFolder.Id}"); + if (normalizedRootPath == null) + { + continue; + } + + AddAllowedMoveRoot(allowedMoveRoots, normalizedRootPath); + if (rootFolder.IsDefault && defaultRootPath == null) + { + defaultRootPath = normalizedRootPath; + } + } + + if (allowedMoveRoots.Count == 0) + { + return new BadRequestObjectResult(new { message = "DestinationPath must be inside a configured root folder or output path" }); + } var destinationIsRooted = Path.IsPathRooted(request.DestinationPath!); - var destinationCandidate = FileUtils.CombineWithOptionalBase(settings.OutputPath, request.DestinationPath!); + var relativeMoveBase = normalizedOutputPath ?? defaultRootPath ?? allowedMoveRoots.FirstOrDefault(); + if (!destinationIsRooted && string.IsNullOrEmpty(relativeMoveBase)) + { + return new BadRequestObjectResult(new { message = "DestinationPath requires a configured root folder or output path" }); + } + + var destinationCandidate = destinationIsRooted + ? request.DestinationPath! + : FileUtils.CombineWithOptionalBase(relativeMoveBase, request.DestinationPath!); if (!FileUtils.TryNormalizeUserProvidedDirectoryPathForCurrentOs( destinationCandidate, out var final, @@ -71,16 +106,14 @@ public async Task EnqueueAsync(int id, LibraryController.MoveRequ { return new BadRequestObjectResult(new { message = $"DestinationPath is not valid for this operating system: {validationReason}" }); } - if (!destinationIsRooted - && !string.IsNullOrWhiteSpace(settings.OutputPath) - && !_fileSystem.TryValidateMutationTarget(final, [settings.OutputPath], out final, out var finalReason)) + if (!_fileSystem.TryValidateMutationTarget(final, allowedMoveRoots, out final, out var finalReason)) { _logger.LogWarning( "Blocked move destination for audiobook {AudiobookId}: {Destination}. Reason: {Reason}", id, final, finalReason); - return new BadRequestObjectResult(new { message = "DestinationPath must be inside the configured output path" }); + return new BadRequestObjectResult(new { message = "DestinationPath must be inside a configured root folder or output path" }); } if (request.MoveFiles == false) @@ -201,6 +234,48 @@ public async Task RequeueAsync(string jobId) return new AcceptedResult(string.Empty, new { message = "Requeued move job", jobId = newJobId }); } + private string? TryNormalizeMoveRoot(string? path, string description) + { + if (string.IsNullOrWhiteSpace(path)) + { + return null; + } + + if (FileUtils.TryNormalizeUserProvidedDirectoryPathForCurrentOs( + path, + out var normalizedPath, + out var validationReason, + allowFileSystemRoot: true, + rejectParentTraversal: true)) + { + return normalizedPath; + } + + _logger.LogWarning( + "Skipping invalid move boundary from {Description}: {Reason}", + description, + validationReason); + return null; + } + + private static void AddAllowedMoveRoot(List allowedRoots, string? normalizedRoot) + { + if (string.IsNullOrEmpty(normalizedRoot)) + { + return; + } + + var comparison = OperatingSystem.IsWindows() + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + if (allowedRoots.Any(root => string.Equals(root, normalizedRoot, comparison))) + { + return; + } + + allowedRoots.Add(normalizedRoot); + } + private async Task BroadcastQueuedAsync(Guid jobId, int? audiobookId) { try diff --git a/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs b/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs index 73b4ad963..072f2090c 100644 --- a/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs +++ b/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs @@ -300,8 +300,13 @@ private static bool IsInvalidWindowsDirectorySegmentCharacter(char character) return character < 32 || character is '<' or '>' or ':' or '"' or '|' or '?' or '*'; } - private static bool ContainsParentDirectorySegment(string path, params char[] separators) + public static bool ContainsParentDirectorySegment(string path, params char[] separators) { + if (string.IsNullOrEmpty(path) || separators.Length == 0) + { + return false; + } + return path.Split(separators, StringSplitOptions.RemoveEmptyEntries) .Any(segment => segment == ".."); } diff --git a/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs b/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs index fe50f2524..ed6b20acb 100644 --- a/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs +++ b/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs @@ -87,7 +87,7 @@ public static string ResolveQbittorrentContentPath( var fileNames = files .Select(f => f.TryGetValue("name", out var nameEl) ? nameEl.GetString() ?? string.Empty : string.Empty) .Where(name => !string.IsNullOrEmpty(name)) - .Where(name => !ContainsParentDirectorySegment(name)) + .Where(name => !FileUtils.ContainsParentDirectorySegment(name, '/', '\\')) .ToList(); if (fileNames.Count == 0) @@ -143,7 +143,7 @@ private static string CombineClientReportedPath(string? basePath, string candida .TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); } - if (ContainsParentDirectorySegment(relativePath)) + if (FileUtils.ContainsParentDirectorySegment(relativePath, '/', '\\')) { return string.Empty; } @@ -151,12 +151,6 @@ private static string CombineClientReportedPath(string? basePath, string candida return FileUtils.CombineWithOptionalBase(basePath, relativePath); } - private static bool ContainsParentDirectorySegment(string path) - { - return path.Split(new[] { '/', '\\' }, StringSplitOptions.RemoveEmptyEntries) - .Any(segment => segment.Length == 2 && segment[0] == '.' && segment[1] == '.'); - } - private static bool HasDriveRootedPrefix(string path) { return path.Length >= 2 diff --git a/tests/Features/Api/Features/Library/LibraryController_MoveTests.cs b/tests/Features/Api/Features/Library/LibraryController_MoveTests.cs index 3d65512c4..15fd44133 100644 --- a/tests/Features/Api/Features/Library/LibraryController_MoveTests.cs +++ b/tests/Features/Api/Features/Library/LibraryController_MoveTests.cs @@ -34,12 +34,17 @@ public async Task MoveAudiobook_ReturnsBadRequest_WhenSourceDoesNotExist() // Given var controller = _provider.GetRequiredService(); + var outputPath = FileService.GetTempDirectory("listenarr-move-output"); + await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() + .WithOutputPath(outputPath) + .Build()); + var ab = await _audiobookRepository.AddAsync(new AudiobookBuilder() .WithTitle("Test") .WithBasePath(Path.Join(FileService.GetTempPath(), "nonexistent")) .Build()); - var request = new LibraryController.MoveRequest { DestinationPath = Path.Join(FileService.GetTempPath(), "target") }; + var request = new LibraryController.MoveRequest { DestinationPath = Path.Join(outputPath, "target") }; // When var result = await controller.EnqueueMove(ab.Id, request); @@ -64,12 +69,17 @@ public async Task MoveAudiobook_EnqueuesJob_WhenSourceExists() Init(services => services.WithSingleton(mockMoveQueue.Object)); var controller = _provider.GetRequiredService(); + var outputPath = FileService.GetTempDirectory("listenarr-move-output"); + await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() + .WithOutputPath(outputPath) + .Build()); + var ab = await _audiobookRepository.AddAsync(new AudiobookBuilder() .WithTitle("Test") .WithBasePath(FileService.GetTempDirectory("listenarr-move-src")) .Build()); - var target = Path.Join(FileService.GetTempPath(), "listenarr-move-dst"); + var target = Path.Join(outputPath, "listenarr-move-dst"); var request = new LibraryController.MoveRequest { DestinationPath = target }; // When @@ -92,12 +102,17 @@ public async Task MoveAudiobook_UpdatesBasePath_WhenMoveFilesFalse() Init(services => services.WithSingleton(mockMoveQueue.Object)); var controller = _provider.GetRequiredService(); + var outputPath = FileService.GetTempDirectory("listenarr-move-output"); + await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() + .WithOutputPath(outputPath) + .Build()); + var ab = await _audiobookRepository.AddAsync(new AudiobookBuilder() .WithTitle("Test") .WithBasePath(Path.Join(FileService.GetTempPath(), "listenarr-move-src")) .Build()); - var target = Path.Join(FileService.GetTempPath(), "listenarr-move-dst"); + var target = Path.Join(outputPath, "listenarr-move-dst"); var request = new LibraryController.MoveRequest { DestinationPath = target, MoveFiles = false }; // When @@ -155,6 +170,11 @@ await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() [Trait("Scenario", "RejectsInvalidDestinationPath")] public async Task MoveAudiobook_RejectsInvalidDestinationPath() { + var outputPath = FileService.GetTempDirectory("listenarr-move-output"); + await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() + .WithOutputPath(outputPath) + .Build()); + var sourcePath = FileService.GetTempDirectory("listenarr-move-src"); var audiobook = await _audiobookRepository.AddAsync(new AudiobookBuilder() .WithTitle("Test") @@ -175,6 +195,131 @@ public async Task MoveAudiobook_RejectsInvalidDestinationPath() Assert.Contains("DestinationPath", badObj.Value?.ToString() ?? string.Empty); } + [Fact] + [Trait("Method", "EnqueueMove")] + [Trait("Scenario", "AllowsAbsoluteDestinationInsideConfiguredRootFolder")] + public async Task MoveAudiobook_AllowsAbsoluteDestinationInsideConfiguredRootFolder() + { + var rootPath = FileService.GetTempDirectory("listenarr-move-root"); + await _rootFolderRepository.AddAsync(new RootFolderBuilder() + .WithName("Move Root") + .WithPath(rootPath) + .WithIsDefault() + .Build()); + + var audiobook = await _audiobookRepository.AddAsync(new AudiobookBuilder() + .WithTitle("Test") + .WithBasePath(FileService.GetTempDirectory("listenarr-move-src")) + .Build()); + + var controller = _provider.GetRequiredService(); + var target = Path.Join(rootPath, "Author", "Title"); + var request = new LibraryController.MoveRequest { DestinationPath = target, MoveFiles = false }; + + var result = await controller.EnqueueMove(audiobook.Id, request); + + var okObj = Assert.IsAssignableFrom(result); + Assert.Equal(200, okObj.StatusCode); + + var updated = await _audiobookRepository.GetByIdAsync(audiobook.Id); + Assert.NotNull(updated); + Assert.Equal(FileUtils.NormalizeStoredPath(target), updated.BasePath); + } + + [Fact] + [Trait("Method", "EnqueueMove")] + [Trait("Scenario", "AllowsAbsoluteDestinationInsideConfiguredOutputPath")] + public async Task MoveAudiobook_AllowsAbsoluteDestinationInsideConfiguredOutputPath() + { + var outputPath = FileService.GetTempDirectory("listenarr-move-output"); + await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() + .WithOutputPath(outputPath) + .Build()); + + var audiobook = await _audiobookRepository.AddAsync(new AudiobookBuilder() + .WithTitle("Test") + .WithBasePath(FileService.GetTempDirectory("listenarr-move-src")) + .Build()); + + var controller = _provider.GetRequiredService(); + var target = Path.Join(outputPath, "Author", "Title"); + var request = new LibraryController.MoveRequest { DestinationPath = target, MoveFiles = false }; + + var result = await controller.EnqueueMove(audiobook.Id, request); + + var okObj = Assert.IsAssignableFrom(result); + Assert.Equal(200, okObj.StatusCode); + + var updated = await _audiobookRepository.GetByIdAsync(audiobook.Id); + Assert.NotNull(updated); + Assert.Equal(FileUtils.NormalizeStoredPath(target), updated.BasePath); + } + + [Fact] + [Trait("Method", "EnqueueMove")] + [Trait("Scenario", "RejectsAbsoluteDestinationOutsideConfiguredRoots")] + public async Task MoveAudiobook_RejectsAbsoluteDestinationOutsideConfiguredRoots() + { + var outputPath = FileService.GetTempDirectory("listenarr-move-output"); + await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() + .WithOutputPath(outputPath) + .Build()); + + var originalBasePath = FileService.GetTempDirectory("listenarr-move-src"); + var audiobook = await _audiobookRepository.AddAsync(new AudiobookBuilder() + .WithTitle("Test") + .WithBasePath(originalBasePath) + .Build()); + + var controller = _provider.GetRequiredService(); + var outsidePath = Path.Join(FileService.GetTempDirectory("listenarr-move-outside"), "Author", "Title"); + var request = new LibraryController.MoveRequest { DestinationPath = outsidePath, MoveFiles = false }; + + var result = await controller.EnqueueMove(audiobook.Id, request); + + var badObj = Assert.IsAssignableFrom(result); + Assert.Equal(400, badObj.StatusCode); + Assert.Contains("configured root folder or output path", badObj.Value?.ToString() ?? string.Empty); + + var unchanged = await _audiobookRepository.GetByIdAsync(audiobook.Id); + Assert.NotNull(unchanged); + Assert.Equal(originalBasePath, unchanged.BasePath); + } + + [Fact] + [Trait("Method", "EnqueueMove")] + [Trait("Scenario", "UsesDefaultRootFolderForRelativeDestination_WhenOutputPathEmpty")] + public async Task MoveAudiobook_UsesDefaultRootFolderForRelativeDestination_WhenOutputPathEmpty() + { + await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() + .WithOutputPath(string.Empty) + .Build()); + var rootPath = FileService.GetTempDirectory("listenarr-move-root"); + await _rootFolderRepository.AddAsync(new RootFolderBuilder() + .WithName("Default Move Root") + .WithPath(rootPath) + .WithIsDefault() + .Build()); + + var audiobook = await _audiobookRepository.AddAsync(new AudiobookBuilder() + .WithTitle("Test") + .WithBasePath(FileService.GetTempDirectory("listenarr-move-src")) + .Build()); + + var controller = _provider.GetRequiredService(); + var relativeTarget = Path.Join("Author", "Title"); + var request = new LibraryController.MoveRequest { DestinationPath = relativeTarget, MoveFiles = false }; + + var result = await controller.EnqueueMove(audiobook.Id, request); + + var okObj = Assert.IsAssignableFrom(result); + Assert.Equal(200, okObj.StatusCode); + + var updated = await _audiobookRepository.GetByIdAsync(audiobook.Id); + Assert.NotNull(updated); + Assert.Equal(FileUtils.NormalizeStoredPath(Path.Join(rootPath, relativeTarget)), updated.BasePath); + } + [Fact] [Trait("Method", "EnqueueMove")] [Trait("Scenario", "RejectsRelativeDestinationOutsideOutputPath")] diff --git a/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs b/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs index 803dec04d..57b0dc441 100644 --- a/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs +++ b/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs @@ -83,7 +83,7 @@ public async Task Create_AllowsWindowsCurrentDriveRootPath() var dbFactory = new TestDbFactory(options); var repo = new EfRootFolderRepository(dbFactory, Mock.Of>()); var svc = new RootFolderService(repo, null!); - var currentDriveRoot = new string((char)92, 1); + var currentDriveRoot = "\\"; var created = await svc.CreateAsync(new RootFolder { Name = "Current Drive Root", Path = currentDriveRoot }); diff --git a/tests/Features/Domain/Utils/FileUtilsTests.cs b/tests/Features/Domain/Utils/FileUtilsTests.cs index 6186fe8c2..3ee75b29b 100644 --- a/tests/Features/Domain/Utils/FileUtilsTests.cs +++ b/tests/Features/Domain/Utils/FileUtilsTests.cs @@ -560,7 +560,7 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_UsesWindowsRules(string p [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsWindowsRootWhenExplicitlyRequested() { - var separator = new string((char)92, 1); + var separator = "\\"; var driveRoot = "C:" + separator; var uncRoot = separator + separator + "server" + separator + "share"; var currentDriveRoot = separator; @@ -605,7 +605,7 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsWindowsRootWhenExpl [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsWindowsRootByDefault() { - var separator = new string((char)92, 1); + var separator = "\\"; Assert.False(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( separator, @@ -625,7 +625,7 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsWindowsRootByDefau [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsParentTraversalForDestinations() { - var separator = new string((char)92, 1); + var separator = "\\"; var windowsTraversal = "C:" + separator + "Books" + separator + ".." + separator + "Other"; Assert.False(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( @@ -648,7 +648,7 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsParentTraversalFor [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsRootFolderParentTraversal() { - var separator = new string((char)92, 1); + var separator = "\\"; var parentSegment = new string('.', 2); var windowsTraversal = "C:" + separator + "Books" + separator + parentSegment + separator + "Other"; @@ -675,7 +675,7 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsRootFolderParentTr [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsRootsWhenExplicitlyRequestedAndTraversalRejected() { - var separator = new string((char)92, 1); + var separator = "\\"; var driveRoot = "C:" + separator; var uncRoot = separator + separator + "server" + separator + "share"; @@ -720,6 +720,23 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsRootsWhenExplicitly Assert.Equal("/", normalizedUnixRoot); } + [Theory] + [InlineData("../escape", true)] + [InlineData("Book/../../escape", true)] + [InlineData(".../Book", false)] + [InlineData("..hidden/Book", false)] + [InlineData("Book../Title", false)] + public void ContainsParentDirectorySegment_DetectsOnlyLiteralParentSegments(string path, bool expected) + { + Assert.Equal(expected, FileUtils.ContainsParentDirectorySegment(path, '/', '\\')); + } + + [Fact] + public void ContainsParentDirectorySegment_WithUnixSeparator_DoesNotTreatBackslashAsSeparator() + { + Assert.False(FileUtils.ContainsParentDirectorySegment("Book\\..\\Title", '/')); + } + [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsUnixRootWhenExplicitlyRequested() { From e218c0c50a12082c6673590c37cf8aad9c50abc4 Mon Sep 17 00:00:00 2001 From: Robbie Davis Date: Wed, 1 Jul 2026 21:36:51 -0400 Subject: [PATCH 005/158] test(paths): use verbatim backslash literals --- .../Audiobooks/RootFolders/RootFolderServiceTests.cs | 2 +- tests/Features/Domain/Utils/FileUtilsTests.cs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs b/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs index 57b0dc441..74dca04da 100644 --- a/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs +++ b/tests/Features/Application/Audiobooks/RootFolders/RootFolderServiceTests.cs @@ -83,7 +83,7 @@ public async Task Create_AllowsWindowsCurrentDriveRootPath() var dbFactory = new TestDbFactory(options); var repo = new EfRootFolderRepository(dbFactory, Mock.Of>()); var svc = new RootFolderService(repo, null!); - var currentDriveRoot = "\\"; + var currentDriveRoot = @"\"; var created = await svc.CreateAsync(new RootFolder { Name = "Current Drive Root", Path = currentDriveRoot }); diff --git a/tests/Features/Domain/Utils/FileUtilsTests.cs b/tests/Features/Domain/Utils/FileUtilsTests.cs index 3ee75b29b..a75c59cd8 100644 --- a/tests/Features/Domain/Utils/FileUtilsTests.cs +++ b/tests/Features/Domain/Utils/FileUtilsTests.cs @@ -560,7 +560,7 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_UsesWindowsRules(string p [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsWindowsRootWhenExplicitlyRequested() { - var separator = "\\"; + var separator = @"\"; var driveRoot = "C:" + separator; var uncRoot = separator + separator + "server" + separator + "share"; var currentDriveRoot = separator; @@ -605,7 +605,7 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsWindowsRootWhenExpl [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsWindowsRootByDefault() { - var separator = "\\"; + var separator = @"\"; Assert.False(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( separator, @@ -625,7 +625,7 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsWindowsRootByDefau [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsParentTraversalForDestinations() { - var separator = "\\"; + var separator = @"\"; var windowsTraversal = "C:" + separator + "Books" + separator + ".." + separator + "Other"; Assert.False(FileUtils.TryNormalizeUserProvidedDirectoryPathForOs( @@ -648,7 +648,7 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsParentTraversalFor [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsRootFolderParentTraversal() { - var separator = "\\"; + var separator = @"\"; var parentSegment = new string('.', 2); var windowsTraversal = "C:" + separator + "Books" + separator + parentSegment + separator + "Other"; @@ -675,7 +675,7 @@ public void TryNormalizeUserProvidedDirectoryPathForOs_RejectsRootFolderParentTr [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsRootsWhenExplicitlyRequestedAndTraversalRejected() { - var separator = "\\"; + var separator = @"\"; var driveRoot = "C:" + separator; var uncRoot = separator + separator + "server" + separator + "share"; From 4fb112cf9d77bbb7b9f5b315e6dba506c354b755 Mon Sep 17 00:00:00 2001 From: Robbie Davis Date: Thu, 2 Jul 2026 09:56:02 -0400 Subject: [PATCH 006/158] fix(paths): refine destination validation --- .../Features/Library/LibraryAddWorkflow.cs | 9 ++- .../Features/Library/LibraryMoveWorkflow.cs | 15 +++- .../Audiobooks/Catalog/LibraryAddService.cs | 11 ++- .../Common/FileUtils.PathCombining.cs | 27 ------- .../Common/FileUtils.UserProvidedPaths.cs | 17 +++++ listenarr.domain/Common/FileUtils.cs | 9 ++- .../Common/TorrentClientPathMapper.cs | 2 +- .../QbittorrentImportPathResolver.cs | 2 +- .../LibraryController_AddToLibraryTests.cs | 46 ++++++++++++ .../Library/LibraryController_MoveTests.cs | 70 +++++++++++++++++++ tests/Features/Domain/Utils/FileUtilsTests.cs | 20 +++++- .../Common/TorrentClientPathMapperTests.cs | 34 +++++++++ 12 files changed, 225 insertions(+), 37 deletions(-) diff --git a/listenarr.api/Features/Library/LibraryAddWorkflow.cs b/listenarr.api/Features/Library/LibraryAddWorkflow.cs index d6622e3b2..8e101dc1f 100644 --- a/listenarr.api/Features/Library/LibraryAddWorkflow.cs +++ b/listenarr.api/Features/Library/LibraryAddWorkflow.cs @@ -138,13 +138,20 @@ public async Task AddAsync(LibraryController.AddToLibraryRequest if (!string.IsNullOrWhiteSpace(request.DestinationPath)) { + // Preserve valid Unix path-segment whitespace, but reject values that only become + // absolute after trimming accidental leading whitespace. + if (FileUtils.HasLeadingWhitespaceBeforeRootedPath(request.DestinationPath)) + { + return new BadRequestObjectResult(new { message = "DestinationPath is invalid: leading whitespace before an absolute path is not allowed." }); + } + if (!FileUtils.TryNormalizeUserProvidedDirectoryPathForCurrentOs( request.DestinationPath, out var normalizedDestinationPath, out var validationReason, rejectParentTraversal: true)) { - return new BadRequestObjectResult(new { message = $"DestinationPath is not valid for this operating system: {validationReason}" }); + return new BadRequestObjectResult(new { message = $"DestinationPath is invalid: {validationReason}" }); } audiobook.BasePath = normalizedDestinationPath; diff --git a/listenarr.api/Features/Library/LibraryMoveWorkflow.cs b/listenarr.api/Features/Library/LibraryMoveWorkflow.cs index 50f86da00..e5cd5c176 100644 --- a/listenarr.api/Features/Library/LibraryMoveWorkflow.cs +++ b/listenarr.api/Features/Library/LibraryMoveWorkflow.cs @@ -55,6 +55,14 @@ public async Task EnqueueAsync(int id, LibraryController.MoveRequ return new BadRequestObjectResult(new { message = "DestinationPath is required" }); } + // Preserve valid Unix path-segment whitespace, but reject values that only become + // absolute after trimming accidental leading whitespace. Otherwise move would treat + // " /books/Title" as a relative child folder under the configured destination root. + if (FileUtils.HasLeadingWhitespaceBeforeRootedPath(request.DestinationPath)) + { + return new BadRequestObjectResult(new { message = "DestinationPath is invalid: leading whitespace before an absolute path is not allowed." }); + } + try { using var scope = _scopeFactory.CreateScope(); @@ -104,7 +112,7 @@ public async Task EnqueueAsync(int id, LibraryController.MoveRequ out var validationReason, rejectParentTraversal: true)) { - return new BadRequestObjectResult(new { message = $"DestinationPath is not valid for this operating system: {validationReason}" }); + return new BadRequestObjectResult(new { message = $"DestinationPath is invalid: {validationReason}" }); } if (!_fileSystem.TryValidateMutationTarget(final, allowedMoveRoots, out final, out var finalReason)) { @@ -174,7 +182,10 @@ public async Task EnqueueAsync(int id, LibraryController.MoveRequ { var srcFull = Path.GetFullPath(sourcePath).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); var tgtFull = Path.GetFullPath(final).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); - if (string.Equals(srcFull, tgtFull, StringComparison.OrdinalIgnoreCase)) + var pathComparison = OperatingSystem.IsWindows() + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + if (string.Equals(srcFull, tgtFull, pathComparison)) { return new BadRequestObjectResult(new { message = "Source and target paths are identical; nothing to move." }); } diff --git a/listenarr.application/Audiobooks/Catalog/LibraryAddService.cs b/listenarr.application/Audiobooks/Catalog/LibraryAddService.cs index e8ec83982..5090dfa91 100644 --- a/listenarr.application/Audiobooks/Catalog/LibraryAddService.cs +++ b/listenarr.application/Audiobooks/Catalog/LibraryAddService.cs @@ -153,13 +153,20 @@ public async Task AddToLibraryAsync( var requestedBaseDirectory = request.DestinationPath; if (!string.IsNullOrWhiteSpace(requestedBaseDirectory)) { + // Preserve valid Unix path-segment whitespace, but reject values that only become + // absolute after trimming accidental leading whitespace. + if (FileUtils.HasLeadingWhitespaceBeforeRootedPath(requestedBaseDirectory)) + { + return ValidationFailure("DestinationPath is invalid: leading whitespace before an absolute path is not allowed."); + } + if (!FileUtils.TryNormalizeUserProvidedDirectoryPathForCurrentOs( requestedBaseDirectory, out var normalizedRequestedBaseDirectory, out var validationReason, rejectParentTraversal: true)) { - return ValidationFailure($"DestinationPath is not valid for this operating system: {validationReason}"); + return ValidationFailure($"DestinationPath is invalid: {validationReason}"); } audiobook.BasePath = normalizedRequestedBaseDirectory; @@ -178,7 +185,7 @@ public async Task AddToLibraryAsync( out var validationReason, rejectParentTraversal: true)) { - return ValidationFailure($"Generated library destination is not valid for this operating system: {validationReason}"); + return ValidationFailure($"Generated library destination is invalid: {validationReason}"); } audiobook.BasePath = normalizedGeneratedBasePath; diff --git a/listenarr.domain/Common/FileUtils.PathCombining.cs b/listenarr.domain/Common/FileUtils.PathCombining.cs index 964f1661e..c49af1241 100644 --- a/listenarr.domain/Common/FileUtils.PathCombining.cs +++ b/listenarr.domain/Common/FileUtils.PathCombining.cs @@ -196,33 +196,6 @@ private static string NormalizeFullPathForBoundary(string path) return fullPath.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); } - private static bool HasInvalidWindowsPathWhitespace(string path) - { - if (string.IsNullOrWhiteSpace(path)) - { - return true; - } - - var root = Path.GetPathRoot(path); - var pathWithoutRoot = !string.IsNullOrEmpty(root) && path.StartsWith(root, StringComparison.OrdinalIgnoreCase) - ? path[root.Length..] - : path; - - return pathWithoutRoot - .Split(new[] { '\\', '/' }, StringSplitOptions.None) - .Any(IsInvalidWindowsPathSegmentWhitespace); - } - - private static bool IsInvalidWindowsPathSegmentWhitespace(string segment) - { - if (string.IsNullOrEmpty(segment) || segment == "." || segment == "..") - { - return false; - } - - return segment.EndsWith(' ') || segment.EndsWith('.'); - } - /// /// Create a filesystem-safe name from arbitrary text by removing invalid path characters /// and normalizing whitespace. Keeps it conservative to avoid unexpected folder creation. diff --git a/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs b/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs index 072f2090c..4b5507f75 100644 --- a/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs +++ b/listenarr.domain/Common/FileUtils.UserProvidedPaths.cs @@ -217,6 +217,23 @@ private static bool TryNormalizeUnixUserProvidedDirectoryPath( } } + /// + /// Detects values that visually look like absolute paths after accidental leading whitespace. + /// Do not trim user-provided paths before validation because Unix path-segment whitespace is valid. + /// + public static bool HasLeadingWhitespaceBeforeRootedPath(string? path) + { + if (string.IsNullOrEmpty(path) || !char.IsWhiteSpace(path[0])) + { + return false; + } + + var trimmedStart = path.TrimStart(); + return Path.IsPathRooted(trimmedStart) + || IsWindowsCurrentDriveRoot(trimmedStart) + || GetWindowsRootLength(trimmedStart) > 0; + } + private static bool IsWindowsCurrentDriveRoot(string path) { return path.Length == 1 && (path[0] is '\\' or '/'); diff --git a/listenarr.domain/Common/FileUtils.cs b/listenarr.domain/Common/FileUtils.cs index cccc499e9..fcd069a3b 100644 --- a/listenarr.domain/Common/FileUtils.cs +++ b/listenarr.domain/Common/FileUtils.cs @@ -63,12 +63,17 @@ public static bool IsPathInvalidForCurrentOs(string? path) public static bool IsPathInvalidForOs(string? path, bool isWindows) { - if (string.IsNullOrEmpty(path)) + if (string.IsNullOrEmpty(path) || !isWindows) { return false; } - return isWindows && HasInvalidWindowsPathWhitespace(path); + var rootLength = GetWindowsRootLength(path); + var pathWithoutRoot = rootLength > 0 ? path[rootLength..] : path; + return !ValidateWindowsDirectorySegments( + pathWithoutRoot, + rejectParentTraversal: false, + out _); } public static HashSet NormalizeExtensions(IEnumerable? extensions) diff --git a/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs b/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs index ed6b20acb..7fc7142ed 100644 --- a/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs +++ b/listenarr.infrastructure/DownloadClients/Common/TorrentClientPathMapper.cs @@ -40,7 +40,7 @@ public static List BuildQbittorrentSourceFiles( .Where(name => !string.IsNullOrEmpty(name)) .Select(name => CombineClientReportedPath(savePath, name.Replace('/', Path.DirectorySeparatorChar))) .Where(path => !string.IsNullOrEmpty(path)) - .Distinct(StringComparer.OrdinalIgnoreCase) + .Distinct(StringComparer.Ordinal) .ToList(); } diff --git a/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentImportPathResolver.cs b/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentImportPathResolver.cs index 9b89d635c..3dbee9003 100644 --- a/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentImportPathResolver.cs +++ b/listenarr.infrastructure/DownloadClients/Qbittorrent/QbittorrentImportPathResolver.cs @@ -32,7 +32,7 @@ public static List TranslateSourceFiles(IEnumerable sourceFiles) { return sourceFiles .Where(path => !string.IsNullOrEmpty(path)) - .Distinct(StringComparer.OrdinalIgnoreCase) + .Distinct(StringComparer.Ordinal) .ToList(); } diff --git a/tests/Features/Api/Features/Library/LibraryController_AddToLibraryTests.cs b/tests/Features/Api/Features/Library/LibraryController_AddToLibraryTests.cs index dc7308d71..d995d8a23 100644 --- a/tests/Features/Api/Features/Library/LibraryController_AddToLibraryTests.cs +++ b/tests/Features/Api/Features/Library/LibraryController_AddToLibraryTests.cs @@ -281,6 +281,52 @@ public async Task AddToLibrary_WithCustomPath_StoresCustomPathAsBasePath() Assert.Equal(expectedPath, stored.BasePath); } + [Fact] + public async Task AddToLibrary_RejectsCustomPathWithLeadingWhitespaceBeforeAbsolutePath() + { + var controller = _provider.GetRequiredService(); + var customPath = " " + Path.Join(tempRoot, "custom", "audiobooks", "Author", "Title"); + var request = new LibraryController.AddToLibraryRequest + { + Metadata = new AudibleBookMetadata + { + Title = "Leading Space Path Test", + Author = "Custom Author" + }, + Monitored = true, + DestinationPath = customPath + }; + + var actionResult = await controller.AddToLibrary(request); + + var badRequest = Assert.IsType(actionResult); + Assert.Contains("leading whitespace", badRequest.Value.ToString(), StringComparison.OrdinalIgnoreCase); + Assert.Empty(await _audiobookRepository.GetAllAsync()); + } + + [Fact] + public async Task LibraryAddService_RejectsDestinationPathWithLeadingWhitespaceBeforeAbsolutePath() + { + var service = _provider.GetRequiredService(); + var customPath = " " + Path.Join(tempRoot, "custom", "audiobooks", "Author", "Title"); + var request = new LibraryAddOperationRequest + { + Metadata = new AudibleBookMetadata + { + Title = "Leading Space Service Path Test", + Author = "Custom Author" + }, + Monitored = true, + DestinationPath = customPath + }; + + var result = await service.AddToLibraryAsync(request); + + Assert.True(result.ValidationFailed); + Assert.Contains("leading whitespace", result.ValidationMessage, StringComparison.OrdinalIgnoreCase); + Assert.Empty(await _audiobookRepository.GetAllAsync()); + } + [Fact] public async Task AddToLibrary_RejectsCustomPathParentTraversal() { diff --git a/tests/Features/Api/Features/Library/LibraryController_MoveTests.cs b/tests/Features/Api/Features/Library/LibraryController_MoveTests.cs index 15fd44133..a217af9b0 100644 --- a/tests/Features/Api/Features/Library/LibraryController_MoveTests.cs +++ b/tests/Features/Api/Features/Library/LibraryController_MoveTests.cs @@ -320,6 +320,76 @@ await _rootFolderRepository.AddAsync(new RootFolderBuilder() Assert.Equal(FileUtils.NormalizeStoredPath(Path.Join(rootPath, relativeTarget)), updated.BasePath); } + [Fact] + [Trait("Method", "EnqueueMove")] + [Trait("Scenario", "RejectsDestinationPathWithLeadingWhitespaceBeforeAbsolutePath")] + public async Task MoveAudiobook_RejectsDestinationPathWithLeadingWhitespaceBeforeAbsolutePath() + { + var outputPath = FileService.GetTempDirectory("listenarr-move-output"); + await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() + .WithOutputPath(outputPath) + .Build()); + + var sourcePath = FileService.GetTempDirectory("listenarr-move-src"); + var audiobook = await _audiobookRepository.AddAsync(new AudiobookBuilder() + .WithTitle("Test") + .WithBasePath(sourcePath) + .Build()); + + var controller = _provider.GetRequiredService(); + var request = new LibraryController.MoveRequest + { + DestinationPath = " " + Path.Join(outputPath, "target"), + MoveFiles = false + }; + + var result = await controller.EnqueueMove(audiobook.Id, request); + + var badObj = Assert.IsAssignableFrom(result); + Assert.Equal(400, badObj.StatusCode); + Assert.Contains("leading whitespace", badObj.Value?.ToString() ?? string.Empty, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + [Trait("Method", "EnqueueMove")] + [Trait("Scenario", "AllowsCaseOnlyDestinationDifference_OnCaseSensitiveHosts")] + public async Task MoveAudiobook_AllowsCaseOnlyDestinationDifference_OnCaseSensitiveHosts() + { + if (OperatingSystem.IsWindows()) + { + return; + } + + var mockMoveQueue = new Mock(); + var expectedId = Guid.NewGuid(); + mockMoveQueue.Setup(m => m.EnqueueMoveAsync(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(expectedId); + + Init(services => services.WithSingleton(mockMoveQueue.Object)); + var controller = _provider.GetRequiredService(); + + var outputPath = FileService.GetTempDirectory("listenarr-move-output"); + await _applicationSettingsRepository.SaveAsync(new ApplicationSettingsBuilder() + .WithOutputPath(outputPath) + .Build()); + + var sourcePath = Path.Join(outputPath, "CaseOnlyBook"); + Directory.CreateDirectory(sourcePath); + var audiobook = await _audiobookRepository.AddAsync(new AudiobookBuilder() + .WithTitle("Test") + .WithBasePath(sourcePath) + .Build()); + + var targetPath = Path.Join(outputPath, "caseonlybook"); + var request = new LibraryController.MoveRequest { DestinationPath = targetPath }; + + var result = await controller.EnqueueMove(audiobook.Id, request); + + var acceptedObj = Assert.IsAssignableFrom(result); + Assert.Equal(202, acceptedObj.StatusCode); + mockMoveQueue.Verify(m => m.EnqueueMoveAsync(audiobook.Id, FileUtils.NormalizeStoredPath(targetPath), sourcePath), Times.Once); + } + [Fact] [Trait("Method", "EnqueueMove")] [Trait("Scenario", "RejectsRelativeDestinationOutsideOutputPath")] diff --git a/tests/Features/Domain/Utils/FileUtilsTests.cs b/tests/Features/Domain/Utils/FileUtilsTests.cs index a75c59cd8..f507a3382 100644 --- a/tests/Features/Domain/Utils/FileUtilsTests.cs +++ b/tests/Features/Domain/Utils/FileUtilsTests.cs @@ -299,7 +299,11 @@ public void NormalizeStoredPath_DoesNotDropPrefix_WhenMalformedDriveSegmentAppea [InlineData(" folder", false)] [InlineData(@"C:\Program Files\Listenarr", false)] [InlineData(@"C:\media\folder \book.m4b", true)] - public void IsPathInvalidForOs_UsesWindowsWhitespaceRules(string path, bool expected) + [InlineData(@"C:\Books\NUL", true)] + [InlineData(@"C:\Books\COM1.txt", true)] + [InlineData(@"C:\Books\Bad|Name", true)] + [InlineData(@"C:\Books\..\Other", false)] + public void IsPathInvalidForOs_UsesSharedWindowsSegmentRules(string path, bool expected) { Assert.Equal(expected, FileUtils.IsPathInvalidForOs(path, isWindows: true)); } @@ -311,6 +315,7 @@ public void IsPathInvalidForOs_UsesWindowsWhitespaceRules(string path, bool expe [InlineData("folder name", false)] [InlineData(" folder", false)] [InlineData("/media/folder /book.m4b", false)] + [InlineData("/media/NUL", false)] public void IsPathInvalidForOs_AllowsLinuxWhitespacePaths(string path, bool expected) { Assert.Equal(expected, FileUtils.IsPathInvalidForOs(path, isWindows: false)); @@ -737,6 +742,19 @@ public void ContainsParentDirectorySegment_WithUnixSeparator_DoesNotTreatBacksla Assert.False(FileUtils.ContainsParentDirectorySegment("Book\\..\\Title", '/')); } + [Theory] + [InlineData(" /media/Author", true)] + [InlineData(" /media/Author", true)] + [InlineData(@" C:\Books\Author", true)] + [InlineData(@" \\server\share\Books", true)] + [InlineData(" Relative Folder", false)] + [InlineData("/media/Author ", false)] + [InlineData("/media/ Author", false)] + public void HasLeadingWhitespaceBeforeRootedPath_DetectsOnlyAmbiguousRootedInputs(string path, bool expected) + { + Assert.Equal(expected, FileUtils.HasLeadingWhitespaceBeforeRootedPath(path)); + } + [Fact] public void TryNormalizeUserProvidedDirectoryPathForOs_AllowsUnixRootWhenExplicitlyRequested() { diff --git a/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs b/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs index dbcb504d4..e8bdff5dd 100644 --- a/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs +++ b/tests/Features/Infrastructure/DownloadClients/Common/TorrentClientPathMapperTests.cs @@ -80,6 +80,40 @@ public void BuildQbittorrentSourceFiles_PreservesTorrentFolderWhitespace() Assert.Equal([expected], sourceFiles); } + [Fact] + public void BuildQbittorrentSourceFiles_PreservesCaseDistinctSourceFiles() + { + var savePath = FileUtils.GetAbsolutePath("downloads"); + var files = ParseFiles( + """ + [ + { "name": "Book/chapter.m4b" }, + { "name": "Book/Chapter.m4b" } + ] + """); + + var sourceFiles = TorrentClientPathMapper.BuildQbittorrentSourceFiles(savePath, files); + + Assert.Equal( + [ + Path.Join(savePath, "Book", "chapter.m4b"), + Path.Join(savePath, "Book", "Chapter.m4b") + ], + sourceFiles); + } + + [Fact] + public void TranslateSourceFiles_PreservesCaseDistinctSourceFiles() + { + var savePath = FileUtils.GetAbsolutePath("downloads"); + var lower = Path.Join(savePath, "Book", "chapter.m4b"); + var upper = Path.Join(savePath, "Book", "Chapter.m4b"); + + var sourceFiles = QbittorrentImportPathResolver.TranslateSourceFiles([lower, upper]); + + Assert.Equal([lower, upper], sourceFiles); + } + [Fact] public void ResolveQbittorrentContentPath_PreservesSharedTopLevelFolderWhitespace() { From 823b029a0ab5236255e10fb364b9402c10ddc016 Mon Sep 17 00:00:00 2001 From: Robbie Davis Date: Thu, 2 Jul 2026 15:36:05 -0400 Subject: [PATCH 007/158] Fix audiobook move workflow --- .../EditAudiobookModal.moveOptions.spec.ts | 167 ++++++++- fe/src/__tests__/utils/path.spec.ts | 120 +++++++ .../domain/audiobook/EditAudiobookModal.vue | 119 ++++-- fe/src/utils/path.ts | 256 ++++++++++++- .../Features/Library/LibraryMoveWorkflow.cs | 19 +- ...rastructureStartupCompositionExtensions.cs | 14 + .../Workers/WorkerRegistrationExtensions.cs | 1 + .../Moving/AudiobookContentMoveService.cs | 340 ++++++++++++++++++ .../Library/Moving/MoveJobProcessor.cs | 184 +--------- .../Persistence/MoveJobSchemaRepair.cs | 147 ++++++++ .../Repositories/EfMoveQueuePersistence.cs | 58 ++- tests/Builders/ServiceCollectionBuilder.cs | 1 + .../Architecture/BackendArchitectureTests.cs | 1 + .../AudiobookContentMoveServiceTests.cs | 160 +++++++++ .../Library/Moving/MoveJobProcessorTests.cs | 75 ++++ .../Persistence/MoveJobSchemaRepairTests.cs | 143 ++++++++ 16 files changed, 1570 insertions(+), 235 deletions(-) create mode 100644 listenarr.infrastructure/Library/Moving/AudiobookContentMoveService.cs create mode 100644 listenarr.infrastructure/Persistence/MoveJobSchemaRepair.cs create mode 100644 tests/Features/Infrastructure/Library/Moving/AudiobookContentMoveServiceTests.cs create mode 100644 tests/Features/Infrastructure/Persistence/MoveJobSchemaRepairTests.cs diff --git a/fe/src/__tests__/EditAudiobookModal.moveOptions.spec.ts b/fe/src/__tests__/EditAudiobookModal.moveOptions.spec.ts index 30f882c7b..07b3ce975 100644 --- a/fe/src/__tests__/EditAudiobookModal.moveOptions.spec.ts +++ b/fe/src/__tests__/EditAudiobookModal.moveOptions.spec.ts @@ -86,6 +86,10 @@ describe('EditAudiobookModal move options', () => { const { apiService } = await import('@/services/api') expect(apiService.updateAudiobook).toHaveBeenCalledTimes(1) + expect(apiService.updateAudiobook).toHaveBeenCalledWith( + 1, + expect.objectContaining({ basePath: 'C:/root/New Author/New Book' }), + ) expect(apiService.moveAudiobook).toHaveBeenCalledTimes(0) }) @@ -117,14 +121,171 @@ describe('EditAudiobookModal move options', () => { const { apiService } = await import('@/services/api') expect(apiService.updateAudiobook).toHaveBeenCalledTimes(1) + expect(apiService.updateAudiobook).toHaveBeenCalledWith( + 1, + expect.not.objectContaining({ basePath: expect.anything() }), + ) expect(apiService.moveAudiobook).toHaveBeenCalledTimes(1) + expect(apiService.moveAudiobook).toHaveBeenCalledWith(1, 'C:/root/New Author/New Book', { + sourcePath: 'C:\\root\\Some Author\\Some Title', + moveFiles: true, + deleteEmptySource: true, + }) + }) + + it('Destination with parent traversal should be invalid and not call save APIs', async () => { + const wrapper = mount(EditAudiobookModal, { + props: { isOpen: true, audiobook }, + attachTo: document.body, + global: { plugins: [(await import('pinia')).createPinia()] }, + }) + + await new Promise((r) => setTimeout(r, 200)) + ;(wrapper.vm as unknown).selectedRootId = 0 + ;(wrapper.vm as unknown).customRootPath = 'C:\\root\\Some Author\\Some Title\\..' + await wrapper.vm.$nextTick() + + expect(wrapper.text()).toContain('Path traversal is not allowed in the destination folder') + expect( + wrapper.find('button[aria-label="Save destination"]').attributes('disabled'), + ).toBeDefined() + + await (wrapper.vm as unknown).handleSave() + await new Promise((r) => setTimeout(r, 50)) + + const { apiService } = await import('@/services/api') + expect(apiService.updateAudiobook).toHaveBeenCalledTimes(0) + expect(apiService.moveAudiobook).toHaveBeenCalledTimes(0) + }) + + it('Destination segment with trailing whitespace should be invalid and not call save APIs', async () => { + const wrapper = mount(EditAudiobookModal, { + props: { isOpen: true, audiobook }, + attachTo: document.body, + global: { plugins: [(await import('pinia')).createPinia()] }, + }) + + await new Promise((r) => setTimeout(r, 200)) + ;(wrapper.vm as unknown).selectedRootId = 0 + ;(wrapper.vm as unknown).customRootPath = 'C:\\root\\Some Author\\Some Title\\test ' + await wrapper.vm.$nextTick() + + expect(wrapper.text()).toContain( + 'Windows destination folder segments cannot end with a space or period', + ) + expect( + wrapper.find('button[aria-label="Save destination"]').attributes('disabled'), + ).toBeDefined() + + await (wrapper.vm as unknown).handleSave() + await new Promise((r) => setTimeout(r, 50)) + + const { apiService } = await import('@/services/api') + expect(apiService.updateAudiobook).toHaveBeenCalledTimes(0) + expect(apiService.moveAudiobook).toHaveBeenCalledTimes(0) + }) + + it('Destination inside current source should be allowed as a content move', async () => { + const wrapper = mount(EditAudiobookModal, { + props: { isOpen: true, audiobook }, + attachTo: document.body, + global: { plugins: [(await import('pinia')).createPinia()] }, + }) + + await new Promise((r) => setTimeout(r, 200)) + ;(wrapper.vm as unknown).selectedRootId = 0 + ;(wrapper.vm as unknown).customRootPath = 'C:\\root\\Some Author\\Some Title\\ test' + await wrapper.vm.$nextTick() + + expect(wrapper.text()).not.toContain('Source and destination folders cannot overlap') + expect( + wrapper.find('button[aria-label="Save destination"]').attributes('disabled'), + ).toBeUndefined() + + const savePromise = (wrapper.vm as unknown).handleSave() + await new Promise((r) => setTimeout(r, 10)) + const resolver = (wrapper.vm as unknown).moveConfirmResolver + if (resolver) resolver({ proceed: true, moveFiles: true, deleteEmptySource: true }) + await savePromise + await new Promise((r) => setTimeout(r, 50)) + + const { apiService } = await import('@/services/api') + expect(apiService.updateAudiobook).toHaveBeenCalledTimes(0) + expect(apiService.moveAudiobook).toHaveBeenCalledWith( + 1, + 'C:/root/Some Author/Some Title/ test', + { + sourcePath: 'C:\\root\\Some Author\\Some Title', + moveFiles: true, + deleteEmptySource: true, + }, + ) + }) + + it('Windows destination segment with leading whitespace outside source should be allowed', async () => { + const wrapper = mount(EditAudiobookModal, { + props: { isOpen: true, audiobook }, + attachTo: document.body, + global: { plugins: [(await import('pinia')).createPinia()] }, + }) + + await new Promise((r) => setTimeout(r, 200)) + ;(wrapper.vm as unknown).selectedRootId = 0 + ;(wrapper.vm as unknown).customRootPath = 'C:\\root\\Some Author\\Other Title\\ test' + await wrapper.vm.$nextTick() + + expect(wrapper.text()).not.toContain('Windows destination folder segments cannot end') + expect( + wrapper.find('button[aria-label="Save destination"]').attributes('disabled'), + ).toBeUndefined() + + const savePromise = (wrapper.vm as unknown).handleSave() + await new Promise((r) => setTimeout(r, 10)) + const resolver = (wrapper.vm as unknown).moveConfirmResolver + if (resolver) resolver({ proceed: true, moveFiles: true, deleteEmptySource: true }) + await savePromise + await new Promise((r) => setTimeout(r, 50)) + + const { apiService } = await import('@/services/api') expect(apiService.moveAudiobook).toHaveBeenCalledWith( - expect.anything(), - expect.anything(), - expect.objectContaining({ moveFiles: true, deleteEmptySource: true }), + 1, + 'C:/root/Some Author/Other Title/ test', + { + sourcePath: 'C:\\root\\Some Author\\Some Title', + moveFiles: true, + deleteEmptySource: true, + }, ) }) + it('Move-only destination changes should enqueue move without pre-saving BasePath', async () => { + const wrapper = mount(EditAudiobookModal, { + props: { isOpen: true, audiobook }, + attachTo: document.body, + global: { plugins: [(await import('pinia')).createPinia()] }, + }) + + await new Promise((r) => setTimeout(r, 200)) + ;(wrapper.vm as unknown).selectedRootId = 0 + ;(wrapper.vm as unknown).customRootPath = 'C:\\root\\New Author\\New Book' + await wrapper.vm.$nextTick() + + const savePromise = (wrapper.vm as unknown).handleSave() + await new Promise((r) => setTimeout(r, 10)) + const resolver = (wrapper.vm as unknown).moveConfirmResolver + if (resolver) resolver({ proceed: true, moveFiles: true, deleteEmptySource: true }) + await savePromise + await new Promise((r) => setTimeout(r, 50)) + + const { apiService } = await import('@/services/api') + expect(apiService.updateAudiobook).toHaveBeenCalledTimes(0) + expect(apiService.moveAudiobook).toHaveBeenCalledWith(1, 'C:/root/New Author/New Book', { + sourcePath: 'C:\\root\\Some Author\\Some Title', + moveFiles: true, + deleteEmptySource: true, + }) + }) + it('Edition-only changes should persist through updateAudiobook', async () => { const wrapper = mount(EditAudiobookModal, { props: { isOpen: true, audiobook }, diff --git a/fe/src/__tests__/utils/path.spec.ts b/fe/src/__tests__/utils/path.spec.ts index 88ece48b9..9ea589cfd 100644 --- a/fe/src/__tests__/utils/path.spec.ts +++ b/fe/src/__tests__/utils/path.spec.ts @@ -21,6 +21,17 @@ import { trimTrailingSlash, normalizeForCompare, isAbsolutePath, + hasRelativePathSegment, + hasParentTraversalSegment, + hasEmptyMiddlePathSegment, + hasControlCharacter, + hasOuterWhitespace, + hasPathSegmentOuterWhitespace, + hasWindowsTrailingSpaceOrPeriodSegment, + hasWindowsInvalidCharacter, + pathsOverlap, + hasWindowsReservedDeviceSegment, + validateLibraryDestinationPath, stripRootPrefix, } from '@/utils/path' @@ -46,6 +57,115 @@ describe('path utils', () => { expect(isAbsolutePath('relative/path')).toBe(false) }) + it('detects exact relative path segments without blocking periods in names', () => { + expect(hasRelativePathSegment('D:\\Books\\Title\\.')).toBe(true) + expect(hasRelativePathSegment('D:\\Books\\Title\\..')).toBe(true) + expect(hasRelativePathSegment('/books/./title')).toBe(true) + expect(hasRelativePathSegment('/books/../title')).toBe(true) + expect(hasRelativePathSegment('/books/Dr. Seuss')).toBe(false) + expect(hasRelativePathSegment('/books/.metadata')).toBe(false) + expect(hasRelativePathSegment('/books/title...')).toBe(false) + }) + + it('hasParentTraversalSegment detects parent directory traversal', () => { + expect(hasParentTraversalSegment('D:\\Books\\Title\\..')).toBe(true) + expect(hasParentTraversalSegment('/books/title/../other')).toBe(true) + expect(hasParentTraversalSegment('/books/title..')).toBe(false) + expect(hasParentTraversalSegment('/books/.../title')).toBe(false) + expect(hasParentTraversalSegment(null)).toBe(false) + }) + + it('detects empty middle path segments without rejecting roots', () => { + expect(hasEmptyMiddlePathSegment('D:\\Books\\\\Title')).toBe(true) + expect(hasEmptyMiddlePathSegment('/books//title')).toBe(true) + expect(hasEmptyMiddlePathSegment('D:\\Books\\Title')).toBe(false) + expect(hasEmptyMiddlePathSegment('/books/title')).toBe(false) + expect(hasEmptyMiddlePathSegment('D:\\')).toBe(false) + expect(hasEmptyMiddlePathSegment('\\\\server\\share\\Audiobooks')).toBe(false) + expect(hasEmptyMiddlePathSegment('\\\\server\\share\\\\Audiobooks')).toBe(true) + }) + + it('detects control characters and segment whitespace', () => { + expect(hasControlCharacter('D:\\Books\\Title\n')).toBe(true) + expect(hasControlCharacter('D:\\Books\\Title')).toBe(false) + expect(hasOuterWhitespace(' D:\\Books\\Title')).toBe(true) + expect(hasOuterWhitespace('D:\\Books\\Title ')).toBe(true) + expect(hasOuterWhitespace('D:\\Listenarr Test\\Title')).toBe(false) + expect(hasPathSegmentOuterWhitespace('D:\\Books\\test ')).toBe(true) + expect(hasPathSegmentOuterWhitespace('D:\\Books\\ test')).toBe(true) + expect(hasPathSegmentOuterWhitespace('D:\\Listenarr Test\\Title')).toBe(false) + }) + + it('detects Windows-only trailing space or period segments', () => { + expect(hasWindowsTrailingSpaceOrPeriodSegment('D:\\Books\\test ')).toBe(true) + expect(hasWindowsTrailingSpaceOrPeriodSegment('D:\\Books\\test.')).toBe(true) + expect(hasWindowsTrailingSpaceOrPeriodSegment('D:\\Books\\ test')).toBe(false) + expect(hasWindowsTrailingSpaceOrPeriodSegment('/books/test ')).toBe(false) + expect(hasWindowsTrailingSpaceOrPeriodSegment('/books/ test ')).toBe(false) + }) + + it('detects Windows invalid characters and reserved device names', () => { + expect(hasWindowsInvalidCharacter('D:\\Books\\Bad|Folder')).toBe(true) + expect(hasWindowsInvalidCharacter('D:\\Books\\Bad:Folder')).toBe(true) + expect(hasWindowsInvalidCharacter('D:\\Books\\Good Folder')).toBe(false) + expect(hasWindowsReservedDeviceSegment('D:\\Books\\CON')).toBe(true) + expect(hasWindowsReservedDeviceSegment('D:\\Books\\NUL.txt')).toBe(true) + expect(hasWindowsReservedDeviceSegment('D:\\Books\\COM1.folder')).toBe(true) + expect(hasWindowsReservedDeviceSegment('D:\\Books\\Concert')).toBe(false) + }) + + it('detects overlapping source and destination paths', () => { + expect(pathsOverlap('D:\\Books\\Title\\Child', 'D:\\Books\\Title', 'windows')).toBe(true) + expect(pathsOverlap('D:\\Books\\Title', 'D:\\Books\\Title\\Child', 'windows')).toBe(true) + expect(pathsOverlap('D:\\Books\\Title2', 'D:\\Books\\Title', 'windows')).toBe(false) + expect(pathsOverlap('/books/title/child', '/books/title', 'unix')).toBe(true) + expect(pathsOverlap('/books/title2', '/books/title', 'unix')).toBe(false) + }) + + it('validates library destination paths while allowing platform-valid whitespace', () => { + expect(validateLibraryDestinationPath('D:\\Books\\Title\\.')).toContain( + 'Path traversal is not allowed', + ) + expect(validateLibraryDestinationPath('D:\\Books\\Title\\..')).toContain( + 'Path traversal is not allowed', + ) + expect(validateLibraryDestinationPath('D:\\Books\\\\Title')).toContain('empty path segments') + expect(validateLibraryDestinationPath('D:\\Books\\Bad*Folder')).toContain('invalid on Windows') + expect(validateLibraryDestinationPath('D:\\Books\\CON.txt')).toContain('reserved Windows') + expect(validateLibraryDestinationPath('D:\\Books\\test ')).toContain( + 'cannot end with a space or period', + ) + expect(validateLibraryDestinationPath('D:\\Books\\test.')).toContain( + 'cannot end with a space or period', + ) + expect(validateLibraryDestinationPath('D:\\Books\\ test')).toBe(null) + expect(validateLibraryDestinationPath('/books/ test /')).toBe(null) + expect(validateLibraryDestinationPath('D:\\Books\\Dr. Seuss')).toBe(null) + expect(validateLibraryDestinationPath('D:\\Books\\.metadata')).toBe(null) + expect(validateLibraryDestinationPath('D:\\Books\\Title...')).toContain( + 'cannot end with a space or period', + ) + expect(validateLibraryDestinationPath('/books/Title...')).toBe(null) + expect( + validateLibraryDestinationPath('D:\\Books\\Title\\Child', { + pathKind: 'windows', + sourcePath: 'D:\\Books\\Title', + }), + ).toBe(null) + expect( + validateLibraryDestinationPath('/books/title/child', { + pathKind: 'unix', + sourcePath: '/books/title', + }), + ).toBe(null) + expect( + validateLibraryDestinationPath('D:\\Books', { + pathKind: 'windows', + sourcePath: 'D:\\Books\\Title', + }), + ).toBe(null) + }) + it('stripRootPrefix removes root prefix when present', () => { const root = 'C:\\temp\\Isaac Asimov\\Foundation' const full = 'C:\\temp\\Isaac Asimov\\Foundation\\Prelude to Foundation' diff --git a/fe/src/components/domain/audiobook/EditAudiobookModal.vue b/fe/src/components/domain/audiobook/EditAudiobookModal.vue index 7632ab9dd..ce964c0e1 100644 --- a/fe/src/components/domain/audiobook/EditAudiobookModal.vue +++ b/fe/src/components/domain/audiobook/EditAudiobookModal.vue @@ -504,8 +504,9 @@ type="button" class="btn icon-btn btn-primary btn-sm" @click="finishEditingDestination" + :disabled="Boolean(destinationPathValidationError)" aria-label="Save destination" - title="Done" + :title="destinationPathValidationError || 'Done'" > @@ -524,6 +525,10 @@ organizing within the selected root.

+
+ + {{ destinationPathValidationError }} +
@@ -735,8 +740,8 @@ type="button" class="btn btn-primary" @click="handleSave" - :disabled="saving || !hasChanges" - :title="saving ? 'Saving...' : 'Save'" + :disabled="saving || !hasChanges || Boolean(destinationPathValidationError)" + :title="saving ? 'Saving...' : destinationPathValidationError || 'Save'" :aria-label="saving ? 'Saving' : 'Save'" > Saving... @@ -1480,6 +1485,9 @@ import { trimTrailingSlash, normalizeForCompare, isAbsolutePath, + validateLibraryDestinationPath, + detectPathKind, + type PathKind, stripRootPrefix, } from '@/utils/path' @@ -1501,9 +1509,15 @@ function resolveSelectedRootPath(): string | null { return rootPath.value || null } +function selectedDestinationPathKind(): PathKind { + const root = + resolveSelectedRootPath() || rootPath.value || baselineAudiobook.value?.basePath || '' + return detectPathKind(root) +} + function combinedBasePath(): string | null { const r = resolveSelectedRootPath() || '' - const rel = (formData.value.relativePath || '').trim() + const rel = formData.value.relativePath || '' if (!r && !rel) return null if (!r) return rel @@ -1511,9 +1525,9 @@ function combinedBasePath(): string | null { // input as the exact destination where files should be stored. Do NOT // append the relative or naming pattern — return the custom root exactly. if (selectedRootId.value === 0) { - let out = toForward(r) - out = trimTrailingSlash(out) - return out + const pathKind = detectPathKind(r) + const normalized = pathKind === 'windows' ? toForward(r) : r + return trimTrailingSlash(normalized) } if (!rel) return r @@ -1522,9 +1536,20 @@ function combinedBasePath(): string | null { return r + (needsSep ? sep : '') + rel } -// Path-length warning for the destination path +// Path-length warning and validation for the destination path const editDestinationPath = computed(() => combinedBasePath() || '') const { pathLengthWarning: destinationPathWarning } = usePathLengthCheck(editDestinationPath) +const destinationPathValidationError = computed(() => { + const destination = editDestinationPath.value + const source = baselineAudiobook.value?.basePath || '' + const pathKind = selectedDestinationPathKind() + const basePathChanged = destination !== source + + return validateLibraryDestinationPath(destination, { + pathKind, + sourcePath: basePathChanged ? source : null, + }) +}) // Helper: derive relative path from full base and configured root (moved to module scope so it can be reused) function deriveRelativeFromBase( @@ -1534,14 +1559,17 @@ function deriveRelativeFromBase( if (!base) return '' if (!root) return base - const normBase = toForward(base) - const normRoot = toForward(root) + const pathKind = detectPathKind(root) + const normBase = pathKind === 'windows' ? toForward(base) : base + const normRoot = pathKind === 'windows' ? toForward(root) : root const rootWithSlash = normRoot.endsWith('/') ? normRoot : normRoot + '/' - if (normalizeForCompare(normBase) === normalizeForCompare(normRoot)) return '' - if (normalizeForCompare(normBase).startsWith(normalizeForCompare(rootWithSlash))) { + if (normalizeForCompare(normBase, pathKind) === normalizeForCompare(normRoot, pathKind)) return '' + if ( + normalizeForCompare(normBase, pathKind).startsWith(normalizeForCompare(rootWithSlash, pathKind)) + ) { const rel = normBase.slice(rootWithSlash.length).replace(/^\/+/, '') - const useBackslash = root.includes('\\') + const useBackslash = pathKind === 'windows' && root.includes('\\') return useBackslash ? rel.replace(/\//g, '\\') : rel } @@ -1582,9 +1610,14 @@ function startEditingDestination() { * an absolute/full path. This makes the UI stable when toggling edit mode. */ function finishEditingDestination() { + if (destinationPathValidationError.value) { + toast.error('Invalid destination', destinationPathValidationError.value) + return + } + try { const chosenRoot = resolveSelectedRootPath() || rootPath.value - const val = (formData.value.relativePath || '').trim() + const val = formData.value.relativePath || '' if (!chosenRoot) { // No root available — nothing to do @@ -1614,7 +1647,10 @@ function finishEditingDestination() { const relOrVal = formData.value.relativePath || val || '' if ( isAbsolute || - (relOrVal && normalizeForCompare(relOrVal).startsWith(normalizeForCompare(chosenRoot || ''))) + (relOrVal && + normalizeForCompare(relOrVal, detectPathKind(chosenRoot)).startsWith( + normalizeForCompare(chosenRoot || '', detectPathKind(chosenRoot)), + )) ) { formData.value.relativePath = deriveRelativeFromBase( relOrVal || formData.value.basePath || '', @@ -1637,9 +1673,32 @@ async function handleSave() { // If the base path (destination) changed, prompt the user with rich options const combined = combinedBasePath() const originalBase = audiobook.basePath || '' + const pathKind = selectedDestinationPathKind() + const basePathChanged = (combined || '') !== originalBase + const destinationValidationMessage = validateLibraryDestinationPath(combined, { + pathKind, + sourcePath: basePathChanged ? originalBase : null, + }) + if (destinationValidationMessage) { + toast.error('Invalid destination', destinationValidationMessage) + return + } + if ( + basePathChanged && + combined && + originalBase && + normalizeForCompare(combined, pathKind) === normalizeForCompare(originalBase, pathKind) + ) { + toast.error( + 'Invalid destination', + 'Destination folder must be different from the current source folder.', + ) + return + } + let userWantsMove = true let userWantsDeleteEmpty = true - if ((combined || '') !== originalBase) { + if (basePathChanged) { const choice = await askMoveConfirmation(originalBase || '', combined || '') if (!choice || !choice.proceed) return userWantsMove = Boolean(choice.moveFiles) @@ -1692,8 +1751,11 @@ async function handleSave() { updates.runtime = parsedRuntime } - // If user changed destination/base path, include the combined root+relative value in updates - if ((combined || '') !== (audiobook.basePath || '')) { + const shouldPersistBasePathImmediately = basePathChanged && !userWantsMove + + // Physical moves are committed by the move worker after the filesystem operation succeeds. + // Pre-saving BasePath here can leave the library pointing at the destination when enqueue fails. + if (shouldPersistBasePathImmediately) { ;(updates as Partial).basePath = combined ?? undefined } @@ -1740,7 +1802,7 @@ async function handleSave() { JSON.stringify([...(audiobook.tags || [])].sort()) || formData.value.abridged !== Boolean(audiobook.abridged) || formData.value.explicit !== Boolean(audiobook.explicit) || - (combined || '') !== (audiobook.basePath || '') + shouldPersistBasePathImmediately if (hasNonIdentifierChanges) { await apiService.updateAudiobook(audiobook.id, updates) @@ -1755,7 +1817,7 @@ async function handleSave() { } // If base path changed, either update DB without moving or enqueue server-side move and show progress via SignalR - if ((combined || '') !== (audiobook.basePath || '')) { + if (basePathChanged) { if (!userWantsMove) { // User requested a DB-only change toast.info('Destination updated', 'Destination changed without moving files.') @@ -1805,6 +1867,7 @@ async function handleSave() { } catch (moveErr) { console.error('Failed to enqueue move job:', moveErr) toast.error('Move failed', 'Failed to enqueue move job. Please try again.') + return } } } @@ -2023,17 +2086,27 @@ function close() {