From 5d42438a7213e683ce298a7ae81fc3775716f597 Mon Sep 17 00:00:00 2001 From: Mika Kuns Date: Wed, 15 Apr 2026 10:56:39 +0200 Subject: [PATCH] fix(installer): UninstallRunner abort-on-stop-fail + path guard + partial-failure reporting Co-Authored-By: Claude Sonnet 4.6 --- .../Core/UninstallRunner.cs | 85 ++++++++++++++++--- 1 file changed, 72 insertions(+), 13 deletions(-) diff --git a/src/ClaudeDo.Installer/Core/UninstallRunner.cs b/src/ClaudeDo.Installer/Core/UninstallRunner.cs index fb31569..b3d5b0d 100644 --- a/src/ClaudeDo.Installer/Core/UninstallRunner.cs +++ b/src/ClaudeDo.Installer/Core/UninstallRunner.cs @@ -17,18 +17,26 @@ public sealed class UninstallRunner public async Task RunAsync(IProgress progress, CancellationToken ct) { - // 1) Stop + delete service. + // 1) Validate install dir up front — refuse obviously unsafe paths. + // Prevents Directory.Delete(recursive:true) from wiping C:\ or C:\Program Files\. + if (!IsSafeInstallDir(_context.InstallDirectory, out var safeError)) + return StepResult.Fail($"Refusing to uninstall: {safeError}"); + + // 2) Stop service. If stop fails we MUST abort — deleting a service whose + // process is still running leaves orphan locked binaries under the install dir + // which Directory.Delete will silently skip. progress.Report("Stopping worker service..."); var stopResult = await _stopService.ExecuteAsync(_context, progress, ct); if (!stopResult.Success) - { - progress.Report($"(Ignored) {stopResult.ErrorMessage}"); - } + return StepResult.Fail( + $"Cannot uninstall: worker service did not stop cleanly. {stopResult.ErrorMessage} " + + "Kill the worker manually and re-run uninstall."); + // 3) Unregister service. progress.Report("Unregistering service..."); await ProcessRunner.RunAsync("sc.exe", $"delete {StopServiceStep.ServiceName}", null, progress, ct); - // 2) Remove shortcuts. + // 4) Remove shortcuts (best-effort — a stuck .lnk must not block the rest). progress.Report("Removing shortcuts..."); TryDeleteFile(Path.Combine( Environment.GetFolderPath(Environment.SpecialFolder.CommonDesktopDirectory), @@ -37,32 +45,83 @@ public sealed class UninstallRunner Environment.GetFolderPath(Environment.SpecialFolder.CommonStartMenu), "Programs", "ClaudeDo.lnk")); - // 3) Delete install directory. + // 5) Delete install directory. Track success so we can report partial state. + var failures = new List(); if (Directory.Exists(_context.InstallDirectory)) { progress.Report($"Deleting {_context.InstallDirectory}..."); - TryDeleteDir(_context.InstallDirectory); + if (!TryDeleteDir(_context.InstallDirectory, out var err)) + failures.Add($"install dir ({_context.InstallDirectory}): {err}"); } - // 4) Delete ~/.todo-app (config + DB + logs) — user opted into full removal. + // 6) Delete ~/.todo-app (config + DB + logs) — user opted into full removal. var appData = Paths.AppDataRoot(); if (Directory.Exists(appData)) { progress.Report($"Deleting {appData}..."); - TryDeleteDir(appData); + if (!TryDeleteDir(appData, out var err)) + failures.Add($"app data ({appData}): {err}"); + } + + if (failures.Count > 0) + { + return StepResult.Fail( + "Uninstall partially succeeded — the following could not be removed:\n " + + string.Join("\n ", failures)); } progress.Report("Uninstall complete."); return StepResult.Ok(); } - private static void TryDeleteFile(string path) + /// + /// Guards against catastrophic recursive-delete paths. The install dir must be + /// a non-root directory with a non-empty name (e.g. reject "", "C:\\", "/"). + /// + private static bool IsSafeInstallDir(string path, out string reason) { - try { if (File.Exists(path)) File.Delete(path); } catch { /* best effort */ } + if (string.IsNullOrWhiteSpace(path)) + { + reason = "install directory is empty"; + return false; + } + + string full; + try { full = Path.GetFullPath(path); } + catch (Exception ex) + { + reason = $"install directory is not a valid path: {ex.Message}"; + return false; + } + + var name = Path.GetFileName(full.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)); + if (string.IsNullOrEmpty(name)) + { + reason = $"install directory resolves to a drive root ({full})"; + return false; + } + + reason = ""; + return true; } - private static void TryDeleteDir(string path) + private static void TryDeleteFile(string path) { - try { Directory.Delete(path, recursive: true); } catch { /* best effort */ } + try { if (File.Exists(path)) File.Delete(path); } catch { /* best effort — single shortcut */ } + } + + private static bool TryDeleteDir(string path, out string error) + { + try + { + Directory.Delete(path, recursive: true); + error = ""; + return true; + } + catch (Exception ex) + { + error = ex.Message; + return false; + } } }