Land F-05/F-08/F-09/F-12 + SnapshotSettings field-drop onto main

.github/sync-over-async-allowlist.json

@@ -1,10 +1,9 @@
 {
   "$schema": "https://json-schema.org/draft/2020-12/schema",
-  "$comment": "Allowlist of intentional sync-over-async (.GetAwaiter().GetResult()) call-sites. Category A = host requires synchronous entry-point; Category B = avoidable (never listed here). Review entries when they expire -- if the host constraint still exists, renew the date.",
+  "$comment": "Allowlist of intentional sync-over-async (.GetAwaiter().GetResult()) call-sites. Category A = host requires synchronous entry-point; Category B = avoidable (never listed here). Entries are identified semantically by file + pattern + method, NOT by line number — line numbers drift and the linter reports the accurate current line at scan time. Review entries when they expire; if the host constraint still exists, renew the date.",
   "entries": [
     {
       "file": "src/Tidalarr/Integration/LidarrNative/TidalLidarrDownloadClient.cs",
-      "line": 340,
       "pattern": "EnsureValidSessionAsync().GetAwaiter().GetResult()",
       "method": "TidalLidarrDownloadClient.Test()",
       "category": "A",
@@ -14,7 +13,6 @@
     },
     {
       "file": "src/Tidalarr/Integration/LidarrNative/TidalLidarrIndexer.cs",
-      "line": 506,
       "pattern": "searchTask.GetAwaiter().GetResult()",
       "method": "TidalLidarrIndexer.ParseResponse()",
       "category": "A",
@@ -24,7 +22,6 @@
     },
     {
       "file": "src/Tidalarr/Integration/TidalModule.cs",
-      "line": 110,
       "pattern": "LegacyTokenMigration.MigrateIfPresentAsync(...).GetAwaiter().GetResult()",
       "method": "TidalModule.ConfigureServices()",
       "category": "A",
@@ -34,7 +31,6 @@
     },
     {
       "file": "src/Tidalarr/Infrastructure/Storage/PKCEStateStore.cs",
-      "line": 284,
       "pattern": "reader.LoadAsync().GetAwaiter().GetResult()",
       "method": "PKCEStateStore.LoadState()",
       "category": "A",
@@ -44,21 +40,19 @@
     },
     {
       "file": "src/Tidalarr/Infrastructure/Storage/PKCEStateStore.cs",
-      "line": 301,
       "pattern": "writer.SaveAsync(...).GetAwaiter().GetResult()",
       "method": "PKCEStateStore.SaveState()",
       "category": "A",
-      "reason": "Sync IPKCEStateStore.SaveState() — same constraint as LoadState (line 284).",
+      "reason": "Sync IPKCEStateStore.SaveState() — same constraint as PKCEStateStore.LoadState().",
       "owner": "alex",
       "expiresOn": "2026-08-01"
     },
     {
       "file": "src/Tidalarr/Infrastructure/Storage/PKCEStateStore.cs",
-      "line": 330,
       "pattern": "writer.ClearAsync().GetAwaiter().GetResult()",
       "method": "PKCEStateStore.ClearState()",
       "category": "A",
-      "reason": "Sync IPKCEStateStore.ClearState() — same constraint as LoadState (line 284).",
+      "reason": "Sync IPKCEStateStore.ClearState() — same constraint as PKCEStateStore.LoadState().",
       "owner": "alex",
       "expiresOn": "2026-08-01"
     }

CLAUDE.md

@@ -640,3 +640,18 @@ brainarr — the per-plugin glue is ~30 lines.
 | Test | Root Cause | Fix |
 |------|-----------|-----|
 | `HostVersionCouplingTests.DirectoryPackagesProps_Should_Match_HostVersions_For_Coupled_Dependencies` | Test reads FluentValidation.dll from `ext/Lidarr/_output` which may not exist in all dev environments (Docker-only assembly) | Guard with `Skip` when assembly directory is missing, or document required setup |
+
+## Ecosystem consolidation & parity discipline
+
+This plugin is one of five copy-paste-adjacent Lidarr streaming plugins (amazonmusicarr, applemusicarr,
+tidalarr, qobuzarr, brainarr) sharing `Lidarr.Plugin.Common`. **Every bug here is likely a bug class** present
+in the sibling plugins too. Before shipping a fix to any shared-surface concern (auth/retry, rate-limit /
+Retry-After, catalog→ReleaseInfo field mapping, path/SSRF guards, token store, pagination, date/number
+parsing): **sweep the other plugins + Common for the same pattern, fix every instance, and push shared logic
+down into Common** (plugins adopt it via a thin DI subclass; the out-of-tree DRM seam stays plugin-owned +
+public — never consolidated, because ILRepack internalizes Common in the merged DLL). Common changes go via an
+**isolated-worktree PR from origin/main**, must re-pin `ext-common-sha.txt`, and must keep this plugin's parity
+tests green (the parity matrix is a contract). Verify the actual mechanism before assuming a class sweeps —
+raw-JSON alias-probing plugins and typed-DTO plugins are vulnerable to different bug classes.
+
+**Canonical rules:** `ext/Lidarr.Plugin.Common/AGENTS.md` → "Ecosystem Consolidation & Parity Discipline".

ext-common-sha.txt

@@ -1 +1 @@
-83aa0491fca791f9d26c9b481293609bae92c3e5
+961dfaea60710f61f885f101f3c233baf7a650cb

src/Tidalarr/Domain/Streaming/TidalChunkDownloader.cs

@@ -23,7 +23,12 @@ public class TidalChunkDownloader(HttpClient httpClient, ILogger<TidalChunkDownl
 {
     private readonly HttpClient _httpClient = httpClient;
     private readonly ILogger<TidalChunkDownloader>? _logger = logger;
-    private readonly ChunkedHttpAssembler _assembler = new(httpClient, logger as ILogger<ChunkedHttpAssembler>);
+    // SSRF policy for segment fetches. Chunk URLs come from Tidal's authenticated manifest. The guard's value
+    // here is blocking literal private/loopback/CGNAT/metadata-host targets — the real SSRF vectors. We keep
+    // AllowHttp=true (Tidal manifests can serve http segment URLs; the assembler had no scheme restriction
+    // before #618, so https-only would be a behaviour regression) and ResolveDns=false (skip a per-host lookup
+    // on the hot path; the manifest source is trusted). Net result is strictly more protection than before.
+    private readonly ChunkedHttpAssembler _assembler = new(httpClient, logger as ILogger<ChunkedHttpAssembler>, new RemoteMediaUriPolicy { AllowHttp = true, ResolveDns = false });
     private const int ChunkBufferSize = 65536;
 
     /// <summary>

src/Tidalarr/Integration/LidarrNative/TidalLidarrDownloadClient.cs

@@ -129,23 +129,12 @@
             // HostBridgeDownloadOrchestrator (Common Wave A item 2):
             //   snapshot → generate downloadId → insert into tracker → fire-and-forget doWork → return id
             //
-            // Snapshotter for TidalLidarrDownloadClientSettings: field-by-field copy so the
-            // doWork closure is insulated from live settings changes (ProbeOnly-race pattern).
-            // Reference-typed fields: none in this settings class — all primitives/strings.
+            // Snapshotter: a copy of the settings so the doWork closure is insulated from live settings
+            // changes (ProbeOnly-race pattern). See SnapshotSettings — a reflection copy so no field is dropped.
             return _downloadOrchestrator.StartTrackedDownloadAsync<HostBridgeDownloadItem, TidalLidarrDownloadClientSettings>(
                 settings: Settings,
                 tracker: ActiveDownloads,
-                snapshotter: s => new TidalLidarrDownloadClientSettings
-                {
-                    ConfigPath = s.ConfigPath,
-                    DownloadPath = s.DownloadPath,
-                    PreferredQuality = s.PreferredQuality,
-                    IncludeMqa = s.IncludeMqa,
-                    ExtractFlac = s.ExtractFlac,
-                    DownloadDelay = s.DownloadDelay,
-                    MaxConcurrentTrackDownloads = s.MaxConcurrentTrackDownloads,
-                    MaxConcurrentChunkDownloads = s.MaxConcurrentChunkDownloads
-                },
+                snapshotter: SnapshotSettings,
                 itemFactory: (_, downloadId) =>
                 {
                     HostBridgeDownloadItem item = new()
@@ -397,6 +386,26 @@
         }
     }
 
+    /// <summary>
+    /// Field-by-field snapshot of the settings the background download reads, captured synchronously before
+    /// any await. The previous hand-written initializer silently dropped SaveSyncedLyrics + UseLRCLIB, so the
+    /// background download ignored the user's lyric settings. Reflection-copies every read-write property —
+    /// structurally cannot drop a field. Internal so a unit test can pin the contract.
+    /// </summary>
+    internal static TidalLidarrDownloadClientSettings SnapshotSettings(TidalLidarrDownloadClientSettings live)
+    {
+        TidalLidarrDownloadClientSettings snapshot = new();
+        foreach (var p in typeof(TidalLidarrDownloadClientSettings).GetProperties())
+        {
+            if (p.CanRead && p.CanWrite && p.GetIndexParameters().Length == 0)
+            {
+                p.SetValue(snapshot, p.GetValue(live));
+            }
+        }
+
+        return snapshot;
+    }
+
     private static string ExtractAlbumIdFromRelease(ReleaseInfo release)
         => PrefixedReleaseGuidParser.ExtractAlbumId(release?.Guid, release?.InfoUrl, "tidal");
 

src/Tidalarr/Integration/TidalDownloadClient.cs

@@ -4,6 +4,7 @@
 using Microsoft.Extensions.Logging;
 using Lidarr.Plugin.Common.Base;
 using Lidarr.Plugin.Abstractions.Models;
+using Lidarr.Plugin.Common.HostBridge;
 using Lidarr.Plugin.Common.Security;
 using Lidarr.Plugin.Common.Utilities;
 using Tidalarr.Core.Interfaces;
@@ -31,6 +32,27 @@ public class TidalDownloadClient(
     protected override string ServiceName => "Tidal";
     protected override string ProtocolName => "tidal";
 
+    /// <summary>
+    /// F-09: true when <paramref name="outputPath"/> resolves inside the configured
+    /// <paramref name="downloadRoot"/> or the system temp dir (the two legitimate write roots — the temp
+    /// dir is allowed because <see cref="DownloadTrackAsync"/> stages there before the host imports).
+    /// Canonical-form containment via Common's <see cref="PathTraversalGuard.IsPathWithinRoot"/> (resolves
+    /// <c>..</c>, defends sibling-prefix + case-twin escapes).
+    /// </summary>
+    internal static bool IsOutputPathAllowed(string outputPath, string? downloadRoot)
+        => PathTraversalGuard.IsPathWithinRoot(outputPath, downloadRoot, Path.GetTempPath());
+
+    /// <summary>Throws <see cref="UnauthorizedAccessException"/> when <paramref name="outputPath"/> would
+    /// write outside the configured download path — call before any mkdir/write/move/delete/tag.</summary>
+    private void EnsureOutputPathAllowed(string outputPath)
+    {
+        if (!IsOutputPathAllowed(outputPath, Settings.DownloadPath))
+        {
+            throw new UnauthorizedAccessException(
+                $"Refusing to write outside the configured download path: '{outputPath}'.");
+        }
+    }
+
     // Implement required abstract methods from BaseStreamingDownloadClient
     protected override async Task<bool> AuthenticateAsync()
     {
@@ -117,6 +139,9 @@ public async Task<EnhancedDownloadResult> DownloadTrackEnhancedAsync(
     {
         try
         {
+            // F-09: refuse a destination outside the configured download path before any filesystem work.
+            EnsureOutputPathAllowed(outputPath);
+
             // Step 1: Get track metadata
             StreamingTrack track = await GetTrackAsync(trackId).ConfigureAwait(false);
             TidalQuality quality = preferredQuality ?? Settings.PreferredQuality;
@@ -135,7 +160,10 @@ public async Task<EnhancedDownloadResult> DownloadTrackEnhancedAsync(
                 Logger?.LogDebug($"Download progress: {p.CompletedChunks}/{p.TotalChunks} chunks ({p.ProgressPercentage:F1}%)");
             });
 
-            using MemoryStream audioStream = await this._chunkDownloader.DownloadAndAssembleAsync(manifest, Settings.DownloadDelay, progress, cancellationToken).ConfigureAwait(false);
+            // F-08: assemble to a temp file-backed stream instead of buffering the whole track in a
+            // MemoryStream — large hi-res tracks no longer pin their full size on the managed heap.
+            // Sequential (maxConcurrency 1) to preserve this path's historical ordering contract.
+            await using Stream audioStream = await this._chunkDownloader.DownloadAndAssembleToFileStreamAsync(manifest, Settings.DownloadDelay, maxConcurrentChunkDownloads: 1, progress, cancellationToken).ConfigureAwait(false);
 
             // Step 5: Save assembled audio with correct extension
             string tempPath = outputPath + manifest.FileExtension;
@@ -215,6 +243,9 @@ public async Task<StreamingDownloadResult> DownloadTrackWithMetadataAsync(
     {
         try
         {
+            // F-09: refuse a destination outside the configured download path before any filesystem work.
+            EnsureOutputPathAllowed(outputPath);
+
             StreamingTrack track = await GetTrackAsync(trackId).ConfigureAwait(false);
             TidalQuality quality = preferredQuality ?? Settings.PreferredQuality;
             TidalStreamInfo streamInfo = await this._streamService.GetStreamInfoAsync(trackId, quality).ConfigureAwait(false);

tests/Tidalarr.Tests/ChunkDownloaderCovTests.cs

@@ -105,8 +105,10 @@ public async Task DownloadAndAssembleToFileStreamAsync_WithProgress_ReportsProgr
             KeyId: null,
             SecurityToken: null);
 
-        // Act
-        Progress<ChunkDownloadProgress> progress = new(p => progressReports.Add(p));
+        // Act — use a SYNCHRONOUS IProgress: System.Progress<T> posts callbacks to the threadpool with no
+        // ordering/timing guarantee, so progressReports[0] could observe the count-2 report before count-1
+        // (a real race seen flaking in CI). A synchronous collector makes order + count deterministic.
+        IProgress<ChunkDownloadProgress> progress = new SyncProgress<ChunkDownloadProgress>(p => progressReports.Add(p));
         await using Stream _ = await downloader.DownloadAndAssembleToFileStreamAsync(
             manifest, chunkDelayMs: 0, maxConcurrentChunkDownloads: 1, progress: progress);