From ed0dd20ba707c9c09cb0f9990b15cb1779cabde6 Mon Sep 17 00:00:00 2001 From: theKBro Date: Wed, 7 Oct 2026 11:48:09 +0200 Subject: [PATCH 1/2] Fix NSIS installer launch --- .../CoreTests.cs | 159 +++++++++++++ .../src/Devolutions.Pinget.Core/Repository.cs | 14 ++ rust/crates/pinget-core/src/lib.rs | 224 ++++++++++++++++++ 3 files changed, 397 insertions(+) diff --git a/dotnet/src/Devolutions.Pinget.Core.Tests/CoreTests.cs b/dotnet/src/Devolutions.Pinget.Core.Tests/CoreTests.cs index f546fe9..c8cde89 100644 --- a/dotnet/src/Devolutions.Pinget.Core.Tests/CoreTests.cs +++ b/dotnet/src/Devolutions.Pinget.Core.Tests/CoreTests.cs @@ -1877,6 +1877,165 @@ public void BuildArguments_UsesInnoDefaultsWhenManifestOmitsSwitches() Assert.Equal(["/SP-", "/VERYSILENT", "/SUPPRESSMSGBOXES", "/NORESTART"], InstallerDispatch.BuildArguments("inno", InstallerMode.Silent, installer)); } + [Theory] + [InlineData("nullsoft")] + [InlineData("nsis")] + [InlineData("NULLSOFT")] + [InlineData("NSIS")] + public void PrepareInstallerForExecution_PreservesVerifiedNullsoftBytes(string installerType) + { + var directory = TestPaths.CreateTempAppRoot(); + Directory.CreateDirectory(directory); + var payload = "verified NSIS installer bytes"u8.ToArray(); + try + { + foreach (var filename in new[] { "download", "download.php", "custom", "setup.exe", "setup.EXE" }) + { + var original = Path.Combine(directory, filename); + File.WriteAllBytes(original, payload); + var expected = filename.EndsWith(".exe", StringComparison.OrdinalIgnoreCase) + ? original + : Path.ChangeExtension(original, ".exe"); + if (original != expected) + File.WriteAllText(expected, "stale installer"); + + var prepared = Repository.PrepareInstallerForExecution(original, installerType); + Assert.Equal(expected, prepared); + Assert.Equal(payload, File.ReadAllBytes(prepared)); + if (original != prepared) + Assert.False(File.Exists(original)); + File.Delete(prepared); + } + } + finally + { + TestPaths.DeleteAppRoot(directory); + } + } + + [Theory] + [InlineData("exe")] + [InlineData("inno")] + [InlineData("burn")] + [InlineData("msi")] + [InlineData("msix")] + [InlineData("zip")] + [InlineData("portable")] + public void PrepareInstallerForExecution_LeavesOtherTypesUnchanged(string installerType) + { + Assert.Equal("download", Repository.PrepareInstallerForExecution("download", installerType)); + } + + [Fact] + public void PrepareInstallerForExecution_ReportsMissingNullsoftPayload() + { + var directory = TestPaths.CreateTempAppRoot(); + Directory.CreateDirectory(directory); + try + { + Assert.Throws(() => + Repository.PrepareInstallerForExecution(Path.Combine(directory, "download"), "nullsoft")); + } + finally + { + TestPaths.DeleteAppRoot(directory); + } + } + + [Fact] + public void BuildArguments_UsesNullsoftSilentSwitch() + { + var installer = new Installer(); + + Assert.Equal(["/S"], InstallerDispatch.BuildArguments("nullsoft", InstallerMode.SilentWithProgress, installer)); + Assert.Equal(["/S"], InstallerDispatch.BuildArguments("nullsoft", InstallerMode.Silent, installer)); + Assert.Equal(["/S"], InstallerDispatch.BuildArguments("nsis", InstallerMode.SilentWithProgress, installer)); + Assert.Equal(["/S"], InstallerDispatch.BuildArguments("nsis", InstallerMode.Silent, installer)); + } + + [Fact] + public async Task InstallerCompletion_DoesNotWaitForDetachedChildProcess() + { + if (!OperatingSystem.IsWindows()) + return; + + var directory = Path.Combine(Path.GetTempPath(), $"pinget-process-lifecycle-{Guid.NewGuid():N}"); + Directory.CreateDirectory(directory); + var childScriptPath = Path.Combine(directory, "child.ps1"); + var launcherPath = Path.Combine(directory, "launcher.cmd"); + var childPidPath = Path.Combine(directory, "child.pid"); + var childPid = 0; + Task? installTask = null; + + try + { + var quotedPidPath = childPidPath.Replace("'", "''", StringComparison.Ordinal); + File.WriteAllText( + childScriptPath, + $"Set-Content -LiteralPath '{quotedPidPath}' -Value $PID{Environment.NewLine}Start-Sleep -Seconds 15"); + File.WriteAllText( + launcherPath, + $"@echo off{Environment.NewLine}start \"\" /b powershell.exe -NoProfile -File \"{childScriptPath}\"{Environment.NewLine}"); + + var commandInterpreter = Environment.GetEnvironmentVariable("ComSpec") + ?? Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), "cmd.exe"); + var request = new InstallRequest + { + Query = new PackageQuery(), + Mode = InstallerMode.Interactive, + Override = $"/c \"{launcherPath}\"", + }; + installTask = Task.Run(() => InstallerDispatch.Execute( + commandInterpreter, + "exe", + request, + new Manifest { Id = "Test.Lifecycle", Name = "Lifecycle Test", Version = "1.0" }, + new Installer())); + + var exitCode = await installTask.WaitAsync(TimeSpan.FromSeconds(5)); + Assert.Equal(0, exitCode); + + var markerDeadline = DateTime.UtcNow.AddSeconds(2); + while (childPid == 0 && DateTime.UtcNow < markerDeadline) + { + if (File.Exists(childPidPath)) + int.TryParse(await File.ReadAllTextAsync(childPidPath), out childPid); + + if (childPid == 0) + await Task.Delay(25); + } + + Assert.NotEqual(0, childPid); + using var child = Process.GetProcessById(childPid); + Assert.False(child.HasExited, "The child must outlive the installer process whose exit is awaited."); + } + finally + { + if (childPid == 0 && File.Exists(childPidPath)) + int.TryParse(await File.ReadAllTextAsync(childPidPath), out childPid); + + if (childPid != 0) + { + try + { + using var child = Process.GetProcessById(childPid); + child.Kill(entireProcessTree: true); + child.WaitForExit(3000); + } + catch (ArgumentException) { } + catch (InvalidOperationException) { } + } + + if (installTask is { IsCompleted: false }) + { + try { await installTask.WaitAsync(TimeSpan.FromSeconds(2)); } + catch (TimeoutException) { } + } + + Directory.Delete(directory, recursive: true); + } + } + [Fact] public void BuildArguments_AppendsManifestAndCliSwitches() { diff --git a/dotnet/src/Devolutions.Pinget.Core/Repository.cs b/dotnet/src/Devolutions.Pinget.Core/Repository.cs index 8190f4f..6475680 100644 --- a/dotnet/src/Devolutions.Pinget.Core/Repository.cs +++ b/dotnet/src/Devolutions.Pinget.Core/Repository.cs @@ -1009,6 +1009,7 @@ public InstallResult Install(InstallRequest request) var installerPath = FetchInstaller(request, tempDir).InstallerPath; var installerType = (selectedInstaller.InstallerType ?? "exe").ToLowerInvariant(); + installerPath = PrepareInstallerForExecution(installerPath, installerType); var exitCode = InstallerDispatch.Execute(installerPath, installerType, request, manifest, selectedInstaller); return new InstallResult @@ -1023,6 +1024,19 @@ public InstallResult Install(InstallRequest request) }; } + internal static string PrepareInstallerForExecution(string installerPath, string installerType) + { + if (!(installerType.Equals("nullsoft", StringComparison.OrdinalIgnoreCase) || + installerType.Equals("nsis", StringComparison.OrdinalIgnoreCase)) || + Path.GetExtension(installerPath).Equals(".exe", StringComparison.OrdinalIgnoreCase)) + return installerPath; + + // Like WinGet, give the verified NSIS payload an executable extension before ShellExecute. + var executablePath = Path.ChangeExtension(installerPath, ".exe"); + File.Move(installerPath, executablePath, overwrite: true); + return executablePath; + } + public InstallResult Repair(RepairRequest request) { if (string.IsNullOrWhiteSpace(request.ManifestPath) && !OperatingSystem.IsWindows()) diff --git a/rust/crates/pinget-core/src/lib.rs b/rust/crates/pinget-core/src/lib.rs index bcadfac..621a098 100644 --- a/rust/crates/pinget-core/src/lib.rs +++ b/rust/crates/pinget-core/src/lib.rs @@ -2354,6 +2354,7 @@ impl Repository { let installer_path = self.fetch_installer_for_request(request, &temp_dir)?.installer_path; let installer_type = installer.installer_type.as_deref().unwrap_or("exe").to_lowercase(); + let installer_path = prepare_installer_for_execution(&installer_path, &installer_type)?; self.emit_install_progress(InstallProgress::StartingInstaller { installer_type: installer_type.clone(), @@ -9253,6 +9254,21 @@ fn decode_pin_type(pin_type_int: i64) -> PinType { } } +fn prepare_installer_for_execution(installer_path: &Path, installer_type: &str) -> Result { + if !(installer_type.eq_ignore_ascii_case("nullsoft") || installer_type.eq_ignore_ascii_case("nsis")) + || installer_path + .extension() + .is_some_and(|extension| extension.eq_ignore_ascii_case("exe")) + { + return Ok(installer_path.to_path_buf()); + } + + // Like WinGet, give the verified NSIS payload an executable extension before ShellExecute. + let executable_path = installer_path.with_extension("exe"); + replace_file(installer_path, &executable_path).context("failed to prepare NSIS installer for execution")?; + Ok(executable_path) +} + #[cfg(windows)] fn dispatch_installer( installer_path: &Path, @@ -13331,6 +13347,91 @@ Installers: ); } + #[test] + fn prepare_installer_for_execution_preserves_verified_nullsoft_bytes() { + let root = temp_app_root("nullsoft_execution"); + fs::create_dir_all(&root).expect("create fixture directory"); + let payload = b"verified NSIS installer bytes"; + for installer_type in ["nullsoft", "nsis", "NULLSOFT", "NSIS"] { + for filename in ["download", "download.php", "custom", "setup.exe", "setup.EXE"] { + let original = root.join(filename); + fs::write(&original, payload).expect("write verified installer"); + let expected = if filename.to_ascii_lowercase().ends_with(".exe") { + original.clone() + } else { + let executable = original.with_extension("exe"); + fs::write(&executable, b"stale installer").expect("write stale installer"); + executable + }; + let prepared = prepare_installer_for_execution(&original, installer_type) + .expect("prepare NSIS installer"); + assert_eq!(prepared, expected); + assert_eq!(fs::read(&prepared).expect("read prepared installer"), payload); + if original != prepared { + assert!(!original.exists()); + } + fs::remove_file(prepared).expect("remove prepared installer"); + } + } + fs::remove_dir_all(root).expect("remove fixture directory"); + } + + #[test] + fn prepare_installer_for_execution_leaves_other_types_unchanged() { + for installer_type in ["exe", "inno", "burn", "msi", "msix", "zip", "portable"] { + let path = Path::new("download"); + assert_eq!( + prepare_installer_for_execution(path, installer_type).expect("other types are unchanged"), + path + ); + } + } + + #[test] + fn prepare_installer_for_execution_reports_missing_nullsoft_payload() { + let path = temp_app_root("nullsoft_missing_payload").join("download"); + let error = prepare_installer_for_execution(&path, "nullsoft") + .expect_err("a missing installer must not be reported as prepared"); + assert!( + error + .to_string() + .contains("failed to prepare NSIS installer for execution") + ); + } + + #[test] + fn installer_switch_arguments_use_nullsoft_silent_default() { + let manifest = sample_manifest("Test.Package", "1.0.0", "Test Package"); + let installer = Installer::default(); + let mut progress_request = InstallRequest::new(PackageQuery::default()); + progress_request.mode = InstallerMode::SilentWithProgress; + let mut silent_request = InstallRequest::new(PackageQuery::default()); + silent_request.mode = InstallerMode::Silent; + + for installer_type in ["nullsoft", "nsis"] { + assert_eq!( + installer_command_arguments( + installer_type, + &progress_request, + &manifest, + Path::new("installer.exe"), + &installer + ), + vec!["/S".to_owned()] + ); + assert_eq!( + installer_command_arguments( + installer_type, + &silent_request, + &manifest, + Path::new("installer.exe"), + &installer + ), + vec!["/S".to_owned()] + ); + } + } + #[test] fn installer_command_arguments_append_manifest_and_cli_switches() { let installer = Installer { @@ -13617,6 +13718,129 @@ Installers: ); } + #[cfg(windows)] + #[test] + fn shell_execute_waits_for_installer_process_not_detached_children() { + use std::fs; + use std::sync::mpsc; + use std::time::{Duration, Instant}; + + static LIFECYCLE_TEST_COUNTER: AtomicU64 = AtomicU64::new(0); + let directory = std::env::temp_dir().join(format!( + "pinget-process-lifecycle-{}-{}", + std::process::id(), + LIFECYCLE_TEST_COUNTER.fetch_add(1, AtomicOrdering::Relaxed) + )); + fs::create_dir_all(&directory).expect("create lifecycle fixture directory"); + let child_script = directory.join("child.ps1"); + let launcher_script = directory.join("launcher.cmd"); + let child_pid_file = directory.join("child.pid"); + let quoted_pid_file = child_pid_file.to_string_lossy().replace('\'', "''"); + + fs::write( + &child_script, + format!( + "Set-Content -LiteralPath '{quoted_pid_file}' -Value $PID\r\n\ + Start-Sleep -Seconds 15\r\n" + ), + ) + .expect("write child fixture"); + fs::write( + &launcher_script, + format!( + "@echo off\r\nstart \"\" /b powershell.exe -NoProfile -File \"{}\"\r\n", + child_script.display() + ), + ) + .expect("write launcher fixture"); + + let command_interpreter = std::env::var_os("ComSpec").unwrap_or_else(|| r"C:\Windows\System32\cmd.exe".into()); + let args = vec!["/c".to_owned(), launcher_script.to_string_lossy().into_owned()]; + let (sender, receiver) = mpsc::channel(); + thread::spawn(move || { + let result = shell_execute_installer(Path::new(&command_interpreter), &args, false); + let _ = sender.send(result); + }); + + let deadline = Instant::now() + Duration::from_secs(5); + let mut child_pid = None; + let mut completion = None; + while Instant::now() < deadline { + if child_pid.is_none() { + child_pid = fs::read_to_string(&child_pid_file) + .ok() + .and_then(|value| value.trim().parse::().ok()); + } + if let Ok(result) = receiver.try_recv() { + completion = Some(result); + break; + } + thread::sleep(Duration::from_millis(25)); + } + + let marker_deadline = Instant::now() + Duration::from_secs(2); + while child_pid.is_none() && Instant::now() < marker_deadline { + child_pid = fs::read_to_string(&child_pid_file) + .ok() + .and_then(|value| value.trim().parse::().ok()); + thread::sleep(Duration::from_millis(25)); + } + + let child_was_alive_after_launcher = child_pid.is_some_and(test_process_is_running); + if let Some(pid) = child_pid { + terminate_test_process(pid); + } + + if completion.is_none() { + completion = receiver.recv_timeout(Duration::from_secs(2)).ok(); + } + let _ = fs::remove_dir_all(&directory); + + assert!( + completion.is_some(), + "waiting for the launched installer exceeded the five-second test bound" + ); + assert!( + child_was_alive_after_launcher, + "the detached child should still be running after its launcher exits" + ); + assert_eq!(completion.expect("completion received").expect("installer exited"), 0); + } + + #[cfg(windows)] + fn test_process_is_running(pid: u32) -> bool { + use windows_sys::Win32::Foundation::{CloseHandle, WAIT_TIMEOUT}; + use windows_sys::Win32::System::Threading::{OpenProcess, PROCESS_SYNCHRONIZE, WaitForSingleObject}; + + // SAFETY: OpenProcess receives a PID from this test's child marker and requests + // only synchronization access to that process. + let process = unsafe { OpenProcess(PROCESS_SYNCHRONIZE, 0, pid) }; + if process.is_null() { + return false; + } + // SAFETY: process is a valid handle returned by OpenProcess. + let is_running = unsafe { WaitForSingleObject(process, 0) == WAIT_TIMEOUT }; + // SAFETY: process is the owned handle returned by OpenProcess. + let _ = unsafe { CloseHandle(process) }; + is_running + } + + #[cfg(windows)] + fn terminate_test_process(pid: u32) { + use windows_sys::Win32::Foundation::CloseHandle; + use windows_sys::Win32::System::Threading::{OpenProcess, PROCESS_TERMINATE, TerminateProcess}; + + // SAFETY: OpenProcess receives only the PID written by this test's child. + let process = unsafe { OpenProcess(PROCESS_TERMINATE, 0, pid) }; + if process.is_null() { + return; + } + // SAFETY: process is the owned handle returned by OpenProcess. + let _ = unsafe { TerminateProcess(process, 1) }; + // SAFETY: process is the owned handle returned by OpenProcess. + let _ = unsafe { CloseHandle(process) }; + } + #[test] fn decompresses_mszyml_payload() { let payload = "sV: 1.0.0\nvD:\n - v: 1.2.3\n rP: manifests/test.yaml\n s256H: ABCD\n"; From 2d9b42a3530881dfaa0be7b315329fe9ed52ac80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Moreau?= Date: Mon, 19 Oct 2026 10:28:59 -0400 Subject: [PATCH 2/2] Fix CI: rustfmt and harden detached-child lifecycle tests Apply cargo fmt to the NSIS preparation test and give the PowerShell child in the lifecycle tests time to cold-start on loaded CI runners (20s marker wait, 10s completion bound, 30s child lifetime), and run it with -NonInteractive -ExecutionPolicy Bypass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Devolutions.Pinget.Core.Tests/CoreTests.cs | 9 +++++---- rust/crates/pinget-core/src/lib.rs | 15 ++++++++------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/dotnet/src/Devolutions.Pinget.Core.Tests/CoreTests.cs b/dotnet/src/Devolutions.Pinget.Core.Tests/CoreTests.cs index c8cde89..619e6d6 100644 --- a/dotnet/src/Devolutions.Pinget.Core.Tests/CoreTests.cs +++ b/dotnet/src/Devolutions.Pinget.Core.Tests/CoreTests.cs @@ -1972,10 +1972,10 @@ public async Task InstallerCompletion_DoesNotWaitForDetachedChildProcess() var quotedPidPath = childPidPath.Replace("'", "''", StringComparison.Ordinal); File.WriteAllText( childScriptPath, - $"Set-Content -LiteralPath '{quotedPidPath}' -Value $PID{Environment.NewLine}Start-Sleep -Seconds 15"); + $"Set-Content -LiteralPath '{quotedPidPath}' -Value $PID{Environment.NewLine}Start-Sleep -Seconds 30"); File.WriteAllText( launcherPath, - $"@echo off{Environment.NewLine}start \"\" /b powershell.exe -NoProfile -File \"{childScriptPath}\"{Environment.NewLine}"); + $"@echo off{Environment.NewLine}start \"\" /b powershell.exe -NoLogo -NoProfile -NonInteractive -ExecutionPolicy Bypass -File \"{childScriptPath}\"{Environment.NewLine}"); var commandInterpreter = Environment.GetEnvironmentVariable("ComSpec") ?? Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), "cmd.exe"); @@ -1992,10 +1992,11 @@ public async Task InstallerCompletion_DoesNotWaitForDetachedChildProcess() new Manifest { Id = "Test.Lifecycle", Name = "Lifecycle Test", Version = "1.0" }, new Installer())); - var exitCode = await installTask.WaitAsync(TimeSpan.FromSeconds(5)); + var exitCode = await installTask.WaitAsync(TimeSpan.FromSeconds(10)); Assert.Equal(0, exitCode); - var markerDeadline = DateTime.UtcNow.AddSeconds(2); + // PowerShell cold start can take several seconds on loaded CI runners. + var markerDeadline = DateTime.UtcNow.AddSeconds(20); while (childPid == 0 && DateTime.UtcNow < markerDeadline) { if (File.Exists(childPidPath)) diff --git a/rust/crates/pinget-core/src/lib.rs b/rust/crates/pinget-core/src/lib.rs index 621a098..6ad5051 100644 --- a/rust/crates/pinget-core/src/lib.rs +++ b/rust/crates/pinget-core/src/lib.rs @@ -13363,8 +13363,8 @@ Installers: fs::write(&executable, b"stale installer").expect("write stale installer"); executable }; - let prepared = prepare_installer_for_execution(&original, installer_type) - .expect("prepare NSIS installer"); + let prepared = + prepare_installer_for_execution(&original, installer_type).expect("prepare NSIS installer"); assert_eq!(prepared, expected); assert_eq!(fs::read(&prepared).expect("read prepared installer"), payload); if original != prepared { @@ -13741,14 +13741,14 @@ Installers: &child_script, format!( "Set-Content -LiteralPath '{quoted_pid_file}' -Value $PID\r\n\ - Start-Sleep -Seconds 15\r\n" + Start-Sleep -Seconds 30\r\n" ), ) .expect("write child fixture"); fs::write( &launcher_script, format!( - "@echo off\r\nstart \"\" /b powershell.exe -NoProfile -File \"{}\"\r\n", + "@echo off\r\nstart \"\" /b powershell.exe -NoLogo -NoProfile -NonInteractive -ExecutionPolicy Bypass -File \"{}\"\r\n", child_script.display() ), ) @@ -13762,7 +13762,7 @@ Installers: let _ = sender.send(result); }); - let deadline = Instant::now() + Duration::from_secs(5); + let deadline = Instant::now() + Duration::from_secs(10); let mut child_pid = None; let mut completion = None; while Instant::now() < deadline { @@ -13778,7 +13778,8 @@ Installers: thread::sleep(Duration::from_millis(25)); } - let marker_deadline = Instant::now() + Duration::from_secs(2); + // PowerShell cold start can take several seconds on loaded CI runners. + let marker_deadline = Instant::now() + Duration::from_secs(20); while child_pid.is_none() && Instant::now() < marker_deadline { child_pid = fs::read_to_string(&child_pid_file) .ok() @@ -13798,7 +13799,7 @@ Installers: assert!( completion.is_some(), - "waiting for the launched installer exceeded the five-second test bound" + "waiting for the launched installer exceeded the ten-second test bound" ); assert!( child_was_alive_after_launcher,