v0.1.1: security & correctness fixes from external review

.github/workflows/ci.yml

@@ -31,7 +31,7 @@ jobs:
         run: dotnet build --configuration Release --no-restore
 
       - name: Pack
-        run: dotnet pack --configuration Release --no-build --output ./artifacts -p:PackageOutputPath=${{ github.workspace }}/artifacts
+        run: dotnet pack --configuration Release --no-build --output ./artifacts
 
       - name: Upload package artifact
         uses: actions/upload-artifact@v6

.gitignore

@@ -437,8 +437,5 @@ FodyWeavers.xsd
 *.msm
 *.msp
 
-# PackageOutputPath in csproj uses Windows path C:\nuget-local\ — on Linux/macOS
-# this resolves as a literal `C:/` directory under the project. CI overrides the
-# path; ignore the leak.
-**/C:/
+# Local NuGet pack output (csproj writes to artifacts/packages on Release builds).
 artifacts/

CHANGELOG.md

@@ -9,6 +9,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Security & correctness fixes (planned 0.1.1)
+- **Lock before staging mutation.** `UpdateInstaller.InstallAsync` now acquires `.update.lock` before any change to `.update/<tag>/`. Previously a second installer could wipe a first installer's in-flight staging directory on its way to losing the lock race.
+- **Rollback on swap failure.** A copy failure mid-swap now restores the install directory from `.old/` instead of leaving it half-populated. New `UpdateInstaller.RestoreFromOld` helper.
+- **Asset-name validation.** New `UpdateInstaller.ValidateAssetName` rejects path separators, parent references, rooted paths, and any name whose `Path.GetFileName` doesn't round-trip — closing a path-traversal vector for malicious or misconfigured sources.
+- **HTTPS enforcement in `HttpManifestSource`.** Plain-HTTP manifest URLs and asset URLs are now rejected by default. Opt in via `SelfUpdaterOptions.AllowInsecureManifestSource = true` (tests, internal mirrors, local dev). Plain HTTP defeats SHA-256 verification because the SHA itself is MITM-able.
+- **TOCTOU-safe install path.** `UpdateCommand` now fetches the release once and installs that exact instance — no second source query between display and install. New `ISelfUpdater.GetLatestReleaseAsync()` and `ISelfUpdater.InstallAsync(RemoteRelease, ...)` overloads. The parameterless `InstallAsync` is kept as a convenience for non-interactive consumers (TOCTOU window documented).
+
+### Build hygiene
+- `<GeneratePackageOnBuild>` is now Release-only; ordinary `dotnet build` and `dotnet test` no longer produce `.nupkg` files. Output path moved from `C:\nuget-local\` to `$(MSBuildThisFileDirectory)..\..\artifacts\packages` — platform-neutral, repo-local, and gitignored.
+
 ### Added — initial public release (planned 0.1.0)
 - **Pluggable update sources** — `IUpdateSource` contract with three built-in implementations:
   - `HttpGitHubReleaseSource` (default) — public GitHub Releases via HttpClient.

src/NextIteration.SpectreConsole.SelfUpdate/Commands/UpdateCommand.cs

@@ -64,10 +64,13 @@ protected override async Task<int> ExecuteAsync(CommandContext context, Settings
             var current = _checker.GetCurrentVersion() ?? "dev";
             _console.MarkupLineInterpolated(CultureInfo.InvariantCulture, $"Current version: [bold]{current}[/]");
 
-            UpdateInfo? info;
+            // Fetch the release once and use it for both display and install,
+            // so the user confirms exactly the release that gets installed
+            // (no TOCTOU window between "what's latest?" and "install latest").
+            RemoteRelease? release;
             try
             {
-                info = await _checker.CheckAsync(cancellationToken).ConfigureAwait(false);
+                release = await _selfUpdater.GetLatestReleaseAsync(cancellationToken).ConfigureAwait(false);
             }
             catch (Exception ex) when (ex is not OperationCanceledException)
             {
@@ -76,21 +79,21 @@ protected override async Task<int> ExecuteAsync(CommandContext context, Settings
                 return 1;
             }
 
-            if (info is null)
+            if (release is null)
             {
                 _console.MarkupLine("[red]Could not determine the latest release. The update source returned no result.[/]");
                 return 1;
             }
 
-            _console.MarkupLineInterpolated(CultureInfo.InvariantCulture, $"Latest release: [bold]{info.LatestTag}[/]");
+            _console.MarkupLineInterpolated(CultureInfo.InvariantCulture, $"Latest release: [bold]{release.Tag}[/]");
 
-            if (!settings.Force && !info.IsUpdateAvailable)
+            if (!settings.Force && !IsUpdateAvailable(current, release.Tag))
             {
                 _console.MarkupLine("[green]Already up to date.[/]");
                 return 0;
             }
 
-            if (!settings.Yes && !PromptToContinue(info))
+            if (!settings.Yes && !PromptToContinue(release.Tag))
             {
                 _console.MarkupLine("Aborted.");
                 return 2;
@@ -102,7 +105,7 @@ await _console.Status().StartAsync("Updating…", async statusContext =>
                 {
                     var progress = new Progress<UpdateProgressEvent>(evt =>
                         statusContext.Status(StageLabel(evt.Stage)));
-                    await _selfUpdater.InstallAsync(progress, cancellationToken).ConfigureAwait(false);
+                    await _selfUpdater.InstallAsync(release, progress, cancellationToken).ConfigureAwait(false);
                 }).ConfigureAwait(false);
             }
             catch (UpdateException ex)
@@ -112,17 +115,24 @@ await _console.Status().StartAsync("Updating…", async statusContext =>
             }
 
             _console.MarkupLineInterpolated(CultureInfo.InvariantCulture,
-                $"[green]Installed [bold]{info.LatestTag}[/]. Re-run the CLI to use the new version.[/]");
+                $"[green]Installed [bold]{release.Tag}[/]. Re-run the CLI to use the new version.[/]");
             return 0;
         }
 
-        private bool PromptToContinue(UpdateInfo info)
+        private bool PromptToContinue(string tag)
         {
             return _console.Confirm(
-                $"Install [bold]{info.LatestTag}[/] over the current install in {Markup.Escape(_installer.InstallDirectory)}?",
+                $"Install [bold]{tag}[/] over the current install in {Markup.Escape(_installer.InstallDirectory)}?",
                 defaultValue: true);
         }
 
+        private static bool IsUpdateAvailable(string current, string latestTag)
+        {
+            // Defer to the same comparator the checker uses so behaviour is
+            // identical between the cached probe and this fresh one.
+            return Pipeline.UpdateChecker.IsNewer(current, latestTag);
+        }
+
         private static string StageLabel(UpdateStage stage) => stage switch
         {
             UpdateStage.Downloading => "Downloading…",

src/NextIteration.SpectreConsole.SelfUpdate/ISelfUpdater.cs

@@ -2,9 +2,10 @@ namespace NextIteration.SpectreConsole.SelfUpdate
 {
     /// <summary>
     /// The high-level façade most consumers interact with. Composes the
-    /// registered <see cref="IUpdateChecker"/> and
-    /// <see cref="IUpdateInstaller"/> behind a single service. Resolved from
-    /// DI after calling <c>services.AddSelfUpdater(...)</c>.
+    /// registered <see cref="IUpdateChecker"/>,
+    /// <see cref="IUpdateSource"/>, and <see cref="IUpdateInstaller"/>
+    /// behind a single service. Resolved from DI after calling
+    /// <c>services.AddSelfUpdater(...)</c>.
     /// </summary>
     public interface ISelfUpdater
     {
@@ -14,20 +15,42 @@ public interface ISelfUpdater
         Task<UpdateInfo?> CheckAsync(CancellationToken ct = default);
 
         /// <summary>
-        /// Resolve the latest release on the configured channel, download
-        /// it, run the verifier pipeline, extract the archive, and swap the
-        /// new files into the install directory. Throws
+        /// Hit the configured <see cref="IUpdateSource"/> for the current
+        /// latest release on the configured channel. Returns
+        /// <see langword="null"/> when nothing is available. Use this when
+        /// you want to display "what would be installed?" to the user
+        /// before calling <see cref="InstallAsync(RemoteRelease, IProgress{UpdateProgressEvent}, CancellationToken)"/>
+        /// — the same release instance can be passed to install so the
+        /// displayed and installed versions are guaranteed to match (no
+        /// TOCTOU window between display and install).
+        /// </summary>
+        Task<RemoteRelease?> GetLatestReleaseAsync(CancellationToken ct = default);
+
+        /// <summary>
+        /// Install the supplied release: download, run the verifier
+        /// pipeline, extract the archive, and swap the new files into the
+        /// install directory. Throws <see cref="UpdateException"/> on any
+        /// failure. Prefer this overload when you've already shown the
+        /// user a specific release — passing it back avoids the second
+        /// source query the parameterless overload performs.
+        /// </summary>
+        Task InstallAsync(
+            RemoteRelease release,
+            IProgress<UpdateProgressEvent>? progress = null,
+            CancellationToken ct = default);
+
+        /// <summary>
+        /// Convenience: query the source for the latest release on the
+        /// configured channel and install it. Throws
         /// <see cref="UpdateException"/> when no release is available or
-        /// when any pipeline stage fails.
+        /// when any pipeline stage fails. <b>Has a TOCTOU window</b>: the
+        /// release returned by the source here may differ from the one a
+        /// prior <see cref="CheckAsync"/> reported. For interactive UIs,
+        /// prefer the <see cref="GetLatestReleaseAsync"/> +
+        /// <see cref="InstallAsync(RemoteRelease, IProgress{UpdateProgressEvent}, CancellationToken)"/>
+        /// pair so the user confirms exactly the release that gets
+        /// installed.
         /// </summary>
-        /// <param name="progress">Optional progress sink.</param>
-        /// <param name="ct">Cancellation token.</param>
-        /// <remarks>
-        /// v0.1 always installs the latest release on the configured
-        /// channel — there is no "install a specific older tag" overload.
-        /// Tag-pinned installs may arrive in a later version once every
-        /// source can resolve a release by tag.
-        /// </remarks>
         Task InstallAsync(
             IProgress<UpdateProgressEvent>? progress = null,
             CancellationToken ct = default);

src/NextIteration.SpectreConsole.SelfUpdate/NextIteration.SpectreConsole.SelfUpdate.csproj

@@ -11,11 +11,11 @@
 
 	<PropertyGroup>
 		<PackageId>NextIteration.SpectreConsole.SelfUpdate</PackageId>
-		<Version>0.1.0</Version>
+		<Version>0.1.1</Version>
 		<Authors>Stuart Meeks</Authors>
 		<Description>Self-update for Spectre.Console CLIs: pluggable update sources (GitHub Releases over HTTP, GitHub Releases via gh CLI for private repos, generic HTTPS manifest, custom), SHA-256 verification, atomic file swap, and a drop-in `update` command.</Description>
-		<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
-		<PackageOutputPath>C:\nuget-local\</PackageOutputPath>
+		<GeneratePackageOnBuild Condition="'$(Configuration)' == 'Release'">true</GeneratePackageOnBuild>
+		<PackageOutputPath>$(MSBuildThisFileDirectory)..\..\artifacts\packages</PackageOutputPath>
 		<IncludeBuildOutput>true</IncludeBuildOutput>
 		<PackageLicenseExpression>MIT</PackageLicenseExpression>
 		<PackageReadmeFile>README.md</PackageReadmeFile>

src/NextIteration.SpectreConsole.SelfUpdate/Pipeline/SelfUpdater.cs

@@ -33,6 +33,15 @@ public SelfUpdater(
         public Task<UpdateInfo?> CheckAsync(CancellationToken ct = default) =>
             _checker.CheckAsync(ct);
 
+        public Task<RemoteRelease?> GetLatestReleaseAsync(CancellationToken ct = default) =>
+            _source.GetLatestAsync(_options.Channel, ct);
+
+        public Task InstallAsync(RemoteRelease release, IProgress<UpdateProgressEvent>? progress = null, CancellationToken ct = default)
+        {
+            ArgumentNullException.ThrowIfNull(release);
+            return _installer.InstallAsync(release, progress, ct);
+        }
+
         public async Task InstallAsync(IProgress<UpdateProgressEvent>? progress = null, CancellationToken ct = default)
         {
             var release = await _source.GetLatestAsync(_options.Channel, ct).ConfigureAwait(false);