From b1b278bb8d96fca1ffbe2f3e5c3dd3fe6d40966c Mon Sep 17 00:00:00 2001 From: sysadmin Date: Tue, 9 Jun 2026 02:46:00 +0100 Subject: [PATCH] fix(welcome): correct -Modules arg encoding so hardening subset actually runs (+ real integration test) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit powershell.exe -File binds a single-quoted comma list like '00','03','05' as ONE string element, not a [string[]] array, so Invoke-Hardening.ps1's -contains filter matched nothing and all hardening modules were silently skipped. Fix: adopt a CSV-split contract — Invoke-Hardening.ps1 now accepts [string]$Modules and splits on ',' internally ($ModuleList = $Modules -split ','); ApplyService passes a bare CSV token (e.g. 00,03,05) with no surrounding quotes. Empirically verified via ProcessStartInfo: candidate (a) '00','03','05' → COUNT=1 (bug); candidate (b) 00,03,05 → single string, correctly split by the script; candidate (c) space-separated → PS positional-parameter error. PARSE OK confirmed. Adds ApplyServiceHardeningIntegrationTests: copies the real Invoke-Hardening.ps1 into a temp dir with harmless dummy 0*.ps1 stubs, runs ApplyService with the real ProcessRunner for modules ["00","05"], and asserts ran.txt contains RAN 00 and RAN 05 but NOT RAN 03 or RAN 07. Test fails on the old encoding and passes with the fix (regression-checked). --- windows/hardening/Invoke-Hardening.ps1 | 7 +- .../Apply/ApplyService.cs | 5 +- .../ApplyServiceHardeningIntegrationTests.cs | 113 ++++++++++++++++++ 3 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 windows/welcome/tests/SilverOS.Welcome.Tests/ApplyServiceHardeningIntegrationTests.cs diff --git a/windows/hardening/Invoke-Hardening.ps1 b/windows/hardening/Invoke-Hardening.ps1 index 81d2b25..7d6a852 100644 --- a/windows/hardening/Invoke-Hardening.ps1 +++ b/windows/hardening/Invoke-Hardening.ps1 @@ -2,13 +2,16 @@ <# Runs the §A-H modules (optionally a subset) then Verify. -Modules "00","03","05" -> run only those numeric-prefixed modules (default: all 0*). -ParamsJson '{"wdac":"audit"}' -> exported as $env:SM_PARAMS for modules to read. #> -[CmdletBinding()] param([string[]]$Modules, [string]$ParamsJson) +[CmdletBinding()] param([string]$Modules, [string]$ParamsJson) $ErrorActionPreference = 'Continue' $here = Split-Path -Parent $MyInvocation.MyCommand.Path if ($ParamsJson) { $env:SM_PARAMS = $ParamsJson } +# Accept Modules as a single CSV token (e.g. "00,03,05") so that ProcessStartInfo +# -File binding delivers it reliably as one string regardless of quoting. +$ModuleList = if ($Modules) { $Modules -split ',' } else { @() } Write-Host "=== SilverMetal hardening modules ===" $all = Get-ChildItem (Join-Path $here '0*.ps1') | Sort-Object Name -if ($Modules) { $all = $all | Where-Object { $Modules -contains $_.Name.Substring(0,2) } } +if ($ModuleList.Count -gt 0) { $all = $all | Where-Object { $ModuleList -contains $_.Name.Substring(0,2) } } foreach ($f in $all) { Write-Host "--> $($f.Name)" try { & $f.FullName } catch { Write-Warning "$($f.Name) FAILED: $_" } diff --git a/windows/welcome/src/SilverOS.Welcome.Core/Apply/ApplyService.cs b/windows/welcome/src/SilverOS.Welcome.Core/Apply/ApplyService.cs index 866381e..c513451 100644 --- a/windows/welcome/src/SilverOS.Welcome.Core/Apply/ApplyService.cs +++ b/windows/welcome/src/SilverOS.Welcome.Core/Apply/ApplyService.cs @@ -8,7 +8,10 @@ public sealed class ApplyService(IProcessRunner runner, IAccountService accounts public async Task RunAsync(ApplyRequest req, IProgress progress, CancellationToken ct = default) { progress.Report(new("Applying hardening", 10)); - var mods = string.Join(",", req.Flavour.Hardening.Modules.Select(m => $"'{m}'")); + // Pass modules as a single bare CSV token (e.g. 00,03,05). + // powershell.exe -File receives single-quoted tokens as one literal string, not an array, + // so Invoke-Hardening.ps1 accepts [string]$Modules and splits on ',' internally. + var mods = string.Join(",", req.Flavour.Hardening.Modules); var pjson = JsonSerializer.Serialize(req.Flavour.Hardening.Params).Replace("\"", "\\\""); var script = Path.Combine(hardeningDir, "Invoke-Hardening.ps1"); var res = await runner.RunAsync("powershell.exe", diff --git a/windows/welcome/tests/SilverOS.Welcome.Tests/ApplyServiceHardeningIntegrationTests.cs b/windows/welcome/tests/SilverOS.Welcome.Tests/ApplyServiceHardeningIntegrationTests.cs new file mode 100644 index 0000000..e63bbd7 --- /dev/null +++ b/windows/welcome/tests/SilverOS.Welcome.Tests/ApplyServiceHardeningIntegrationTests.cs @@ -0,0 +1,113 @@ +using Moq; +using SilverOS.Welcome.Core.Apply; +using SilverOS.Welcome.Core.Flavours; +using Xunit; + +/// +/// Real integration test: proves that ApplyService passes -Modules with the correct +/// encoding so that Invoke-Hardening.ps1's subset filter actually works through the +/// real ProcessStartInfo / PowerShell boundary. +/// +/// SAFETY: only harmless dummy .ps1 files are executed — never the real 0*.ps1 hardening +/// modules. Invoke-Hardening.ps1 is copied into a temp dir and run against dummy stubs. +/// +public class ApplyServiceHardeningIntegrationTests +{ + /// Walk up from the test binary to find the repo root (same as ShippedFlavoursTests). + private static string HardeningDir() + { + var d = AppContext.BaseDirectory; + while (d is not null && !Directory.Exists(Path.Combine(d, "windows", "hardening"))) + d = Directory.GetParent(d)?.FullName; + return Path.Combine(d!, "windows", "hardening"); + } + + [Fact] + public async Task Subset_filter_runs_only_requested_modules_via_real_powershell() + { + // ---- Arrange: set up a temp sandbox ---- + var tmp = Path.Combine(Path.GetTempPath(), $"sm_integ_{Guid.NewGuid():N}"); + Directory.CreateDirectory(tmp); + try + { + // Copy the REAL Invoke-Hardening.ps1 (the one we just patched) into the temp dir. + var realInvoke = Path.Combine(HardeningDir(), "Invoke-Hardening.ps1"); + File.Copy(realInvoke, Path.Combine(tmp, "Invoke-Hardening.ps1")); + + // Create harmless dummy module stubs. Each just appends its prefix to ran.txt. + var ranFile = Path.Combine(tmp, "ran.txt").Replace("\\", "\\\\"); + foreach (var (prefix, name) in new[] { + ("00", "00-a.ps1"), + ("03", "03-b.ps1"), + ("05", "05-c.ps1"), + ("07", "07-d.ps1"), + }) + { + // Single quotes around prefix so the string itself is written, not executed. + await File.WriteAllTextAsync( + Path.Combine(tmp, name), + $"'RAN {prefix}' | Out-File -Append \"{ranFile.Replace("\\\\", "\\\\")}\""); + } + + // Dummy Verify script — no-op so Invoke-Hardening.ps1's Verify step succeeds. + await File.WriteAllTextAsync( + Path.Combine(tmp, "Verify-SilverMetalWindows.ps1"), + "# no-op verify"); + + // ---- Arrange: mocked services so apply completes without touching real OS ---- + var acct = new Mock(); + acct.Setup(a => a.CreateAccountsAsync( + It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny())) + .Returns(Task.CompletedTask); + + var bl = new Mock(); + bl.Setup(b => b.EnableAsync(It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + var boot = new Mock(); + boot.Setup(b => b.TearDownAsync(It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + var sut = new ApplyService( + runner: new ProcessRunner(), + accounts: acct.Object, + bitlocker: bl.Object, + bootstrap: boot.Object, + hardeningDir: tmp); + + // Flavour requests modules 00 and 05 only — 03 and 07 must be skipped. + var flavour = new FlavourManifest + { + Id = "test", + Hardening = new HardeningSpec { Modules = new[] { "00", "05" } } + }; + var req = new ApplyRequest(flavour, "alice", "pw", "adminpw", "123456", "sm-bootstrap"); + + // ---- Act ---- + await sut.RunAsync(req, new Progress(_ => { })); + + // ---- Assert: ran.txt should contain only 00 and 05 markers ---- + Assert.True(File.Exists(Path.Combine(tmp, "ran.txt")), + "ran.txt was not created — no module ran at all (subset filter matched nothing)"); + + var ran = await File.ReadAllTextAsync(Path.Combine(tmp, "ran.txt")); + + Assert.Contains("RAN 00", ran, StringComparison.Ordinal); + Assert.Contains("RAN 05", ran, StringComparison.Ordinal); + Assert.DoesNotContain("RAN 03", ran, StringComparison.Ordinal); + Assert.DoesNotContain("RAN 07", ran, StringComparison.Ordinal); + + // ---- Assert: the rest of the apply pipeline also completed ---- + acct.Verify(a => a.CreateAccountsAsync( + "alice", "pw", "adminpw", It.IsAny()), Times.Once); + bl.Verify(b => b.EnableAsync("123456", It.IsAny()), Times.Once); + boot.Verify(b => b.TearDownAsync("sm-bootstrap", It.IsAny()), Times.Once); + } + finally + { + // Clean up — ignore errors (locked files etc.) to avoid masking test failure. + try { Directory.Delete(tmp, recursive: true); } catch { /* ignore */ } + } + } +}