diff --git a/Changelog.md b/Changelog.md index 35dac4e..2499277 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,19 @@ # Changelog +## v1.121.12 + +Backup / undo honesty — fixes found by a deep audit of the security-feature modules (Group Policy, JEA, NPS, SIEM forwarding). Common theme: a safety-net backup that could silently fail and then be trusted anyway. + +- **A SIEM config change can't destroy your working config on undo.** When rewriting a forwarder config (Splunk/syslog/Winlogbeat), the "undo" restored from a backup — but if that backup silently failed to copy, undo *deleted* the live config instead, silently stopping log forwarding. It now refuses to overwrite a config it couldn't back up. +- **"Back up all GPOs" no longer reports success when it backed up nothing.** A scheduled `GPOBackup` that enumerated GPOs but failed to back up every one still exited 0, leaving an empty folder that later got trusted as a baseline. It now reports failure. +- **GPO drift detection stops hiding real changes.** A GPO whose baseline settings were missing was silently reported as "unchanged"; it's now listed as "not checked" so you know the comparison was incomplete instead of getting a false all-clear. +- **Undoable operations only claim to be undoable when they really are.** A GPO restore and an NPS config import each take a pre-change snapshot first; if that snapshot silently failed, the tool used to still offer an "undo" that quietly did nothing. Both now verify the snapshot exists and tell you plainly when a change isn't undoable. +- **A JEA endpoint that fails validation in Dry-Run is reported as failed**, not queued-and-applied. + +(The audit also confirmed the JEA role/endpoint configuration is correctly least-privilege and constrained, and the VPN/RADIUS policy values are correct.) + +No module or CLI action changes (81 modules, 201 actions). + ## v1.121.11 State-integrity hardening — fixes found by a deep audit of the Dry-Run engine, configuration profile import/export, and the batch (scripted) apply. diff --git a/Header.ps1 b/Header.ps1 index 6ddc69c..43f477e 100644 --- a/Header.ps1 +++ b/Header.ps1 @@ -30,7 +30,7 @@ 7h3 4b1d3r .VERSION - 1.121.11 + 1.121.12 .LAST UPDATED 07/01/2026 diff --git a/Modules/00-Initialization.ps1 b/Modules/00-Initialization.ps1 index 3626eed..a4965fe 100644 --- a/Modules/00-Initialization.ps1 +++ b/Modules/00-Initialization.ps1 @@ -233,7 +233,7 @@ if (-not $PSCommandPath -and $script:ScriptPath) { if (-not $script:ModuleRoot -and $script:ScriptPath) { $script:ModuleRoot = [System.IO.Path]::GetDirectoryName($script:ScriptPath) } -$script:ScriptVersion = "1.121.11" +$script:ScriptVersion = "1.121.12" $script:ScriptStartTime = Get-Date # Post-update cleanup: UpdateSelf / Rollback leave a `.pending-delete` sibling next to RackStack.exe. diff --git a/Modules/71-GPOManager.ps1 b/Modules/71-GPOManager.ps1 index 67d806e..186bed4 100644 --- a/Modules/71-GPOManager.ps1 +++ b/Modules/71-GPOManager.ps1 @@ -139,6 +139,15 @@ function Invoke-GPOBackupAll { $resultColor = if ($fail -eq 0) { "Success" } else { "Warning" } Write-OutputColor " Backup complete: $ok succeeded, $fail failed." -color $resultColor + # Honor the function's own contract ("returns the backup folder path, or $null on failure"). + # If GPOs were enumerated but EVERY Backup-GPO failed ($ok -eq 0), the folder is empty and + # must NOT be reported as a success — otherwise the CLI dispatcher exits 0 and a monitoring + # job trusts an empty folder as a drift/restore baseline. + if ($ok -eq 0) { + Write-OutputColor " No GPOs were backed up — not treating this as a successful baseline." -color "Error" + Add-SessionChange -Category "GroupPolicy" -Description "GPO backup produced no backups ($fail failed)" + return $null + } Add-SessionChange -Category "GroupPolicy" -Description "Backed up $ok GPO(s) to $folder" return $folder } @@ -190,7 +199,7 @@ function Compare-GPODriftAgainst { $baseById = @{} foreach ($b in $baseGpos) { $baseById[$b.Id] = $b } - $added = @(); $removed = @(); $changed = @(); $unchanged = 0 + $added = @(); $removed = @(); $changed = @(); $unchanged = 0; $indeterminate = @() foreach ($b in $baseGpos) { if (-not $liveById.ContainsKey($b.Id)) { $removed += $b.DisplayName } @@ -206,9 +215,12 @@ function Compare-GPODriftAgainst { if ($baseXml -ne $liveXml) { $changed += $g.DisplayName } else { $unchanged++ } } else { - # Settings XML unavailable for a reliable compare — count as unchanged - # rather than raising a false drift. - $unchanged++ + # A reliable compare wasn't possible — either the BASELINE settings XML is missing + # (Get-GPOReport failed during backup, or the file was lost) or the LIVE report is + # transiently unavailable. Do NOT count this as "unchanged": that silently reported + # a genuinely-drifted GPO as clean (a false negative in a compliance check). Track it + # separately so the operator knows the check was incomplete for this GPO. + $indeterminate += $g.DisplayName } } @@ -219,6 +231,7 @@ function Compare-GPODriftAgainst { Removed = $removed Changed = $changed Unchanged = $unchanged + Indeterminate = $indeterminate } } @@ -228,6 +241,7 @@ function Show-GPODriftResult { $addedC = @($Drift.Added).Count $removedC = @($Drift.Removed).Count $changedC = @($Drift.Changed).Count + $indetC = @($Drift.Indeterminate | Where-Object { $_ }).Count $total = $addedC + $removedC + $changedC $driftColor = if ($total -eq 0) { "Success" } else { "Warning" } @@ -240,14 +254,21 @@ function Show-GPODriftResult { foreach ($n in $Drift.Removed) { Write-OutputColor " - $n" -color "Warning" } Write-OutputColor " Changed: $changedC" -color $driftColor foreach ($n in $Drift.Changed) { Write-OutputColor " ~ $n" -color "Warning" } + if ($indetC -gt 0) { + Write-OutputColor " Not checked: $indetC (baseline/live settings unavailable)" -color "Warning" + foreach ($n in $Drift.Indeterminate) { Write-OutputColor " ? $n" -color "Warning" } + } Write-OutputColor "" -color "Info" - if ($total -eq 0) { + if ($total -eq 0 -and $indetC -eq 0) { Write-OutputColor " No drift — live GPOs match the baseline." -color "Success" } + elseif ($total -eq 0) { + Write-OutputColor " No drift among compared GPOs, but $indetC could not be checked (missing baseline settings) — the baseline may be incomplete." -color "Warning" + } else { - Write-OutputColor " Drift detected: $total GPO(s) differ from the baseline." -color "Warning" + Write-OutputColor " Drift detected: $total GPO(s) differ from the baseline$(if ($indetC -gt 0) { " ($indetC not checked)" })." -color "Warning" } - Add-SessionChange -Category "GroupPolicy" -Description "GPO drift check: +$addedC -$removedC ~$changedC vs baseline" + Add-SessionChange -Category "GroupPolicy" -Description "GPO drift check: +$addedC -$removedC ~$changedC ?$indetC vs baseline" } # Interactive: drift detection. @@ -357,8 +378,15 @@ function Restore-GPOInteractive { Restore-GPO -Name $capName -Path $capFolder -ErrorAction Stop | Out-Null }.GetNewClosure() ` -Undo { - if (Test-Path -LiteralPath $capPre) { + # Only restore if the pre-restore snapshot actually produced a backup. Backup-GPO + # writes a {GUID} subfolder on success; an empty $capPre means the Apply-time + # snapshot silently failed, and Restore-GPO from it would be a no-op that falsely + # presents as a successful revert. + $snapDirs = @(Get-ChildItem -LiteralPath $capPre -Directory -ErrorAction SilentlyContinue) + if ($snapDirs.Count -gt 0) { Restore-GPO -Name $capName -Path $capPre -ErrorAction SilentlyContinue | Out-Null + } else { + Write-OutputColor " Undo unavailable: no pre-restore snapshot was captured for '$capName'." -color "Warning" } }.GetNewClosure() Write-OutputColor " Queued (Dry-Run): restore GPO '$capName'." -color "Warning" @@ -369,14 +397,23 @@ function Restore-GPOInteractive { $preFolder = Join-Path $script:TempPath "GPO-PreRestore-$(Get-Date -Format 'yyyyMMdd-HHmmss')" try { $existing = Get-GPO -Name $target -ErrorAction SilentlyContinue + $preSnapshotOk = $false if ($existing) { $null = New-Item -LiteralPath $preFolder -ItemType Directory -Force -ErrorAction SilentlyContinue - Backup-GPO -Guid $existing.Id -Path $preFolder -Comment "RackStack pre-restore" -ErrorAction SilentlyContinue | Out-Null + # Capture the pre-restore snapshot result. The undo restores from $preFolder, so if + # this snapshot silently fails the folder is created-but-empty and the undo no-ops in + # silence — the operator thinks the restore is reversible when it isn't. + $snap = $null + try { $snap = Backup-GPO -Guid $existing.Id -Path $preFolder -Comment "RackStack pre-restore" -ErrorAction Stop } + catch { Write-OutputColor " Warning: could not snapshot '$target' before restore — this restore will NOT be undoable: $($_.Exception.Message)" -color "Warning" } + $preSnapshotOk = ($null -ne $snap) } Restore-GPO -Name $target -Path $folder -ErrorAction Stop | Out-Null Write-OutputColor " Restored GPO '$target'." -color "Success" Add-SessionChange -Category "GroupPolicy" -Description "Restored GPO '$target' from backup" - if ($existing) { + if ($preSnapshotOk) { + # Only register the undo when the snapshot actually succeeded, so 'Undo' is never a + # silent no-op presenting as a successful revert. Add-UndoAction -Category "GroupPolicy" -Description "Restored GPO '$target'" -UndoScript { param($Name, $PrePath) if (Test-Path -LiteralPath $PrePath) { Restore-GPO -Name $Name -Path $PrePath -ErrorAction SilentlyContinue | Out-Null } diff --git a/Modules/72-JEA.ps1 b/Modules/72-JEA.ps1 index 766bf1b..731613f 100644 --- a/Modules/72-JEA.ps1 +++ b/Modules/72-JEA.ps1 @@ -246,7 +246,14 @@ function New-JEAEndpoint { else { $true } }.GetNewClosure() ` -Apply { - Invoke-JEAEndpointBuild -Endpoint $capEndpoint -Group $capGroup -VisibleCmdlets $capCmdlets | Out-Null + # Invoke-JEAEndpointBuild returns $false WITHOUT throwing when the generated .pssc + # fails Test-PSSessionConfigurationFile (it cleans up and returns). The dry-run + # engine treats a non-throwing Apply as success, so a validation failure would be + # reported as a registered security endpoint. Throw to surface it as a real failure + # (and trigger rollback). + if (-not (Invoke-JEAEndpointBuild -Endpoint $capEndpoint -Group $capGroup -VisibleCmdlets $capCmdlets)) { + throw "JEA endpoint '$capEndpoint' failed validation and was not registered." + } }.GetNewClosure() ` -Undo { Unregister-PSSessionConfiguration -Name $capEndpoint -Force -ErrorAction SilentlyContinue diff --git a/Modules/73-NPS.ps1 b/Modules/73-NPS.ps1 index 795bce6..a5b777d 100644 --- a/Modules/73-NPS.ps1 +++ b/Modules/73-NPS.ps1 @@ -253,23 +253,32 @@ function Import-NPSConfig { }.GetNewClosure() ` -Undo { if (Test-Path -LiteralPath $capPre) { [void](Invoke-NetshNps -NetshArgs @("import", "filename=$capPre")) } + else { Write-OutputColor " Undo unavailable: no pre-import NPS backup was captured." -color "Warning" } }.GetNewClosure() Write-OutputColor " Queued (Dry-Run): import NPS config." -color "Warning" Add-SessionChange -Category "DryRun" -Description "Queued NPS config import" return } - # Capture the current config so the operator can roll back. + # Capture the current config so the operator can roll back. Verify the backup actually landed + # — the export result was previously discarded and the "Pre-import backup: ..." line + the + # undo were emitted unconditionally, so a failed export left the operator believing a rollback + # existed while the undo silently no-op'd and the overwrite was permanent. $preFile = Join-Path (Get-NPSBackupDir) "nps-preimport-$(Get-Date -Format 'yyyyMMdd-HHmmss').xml" [void](Invoke-NetshNps -NetshArgs @("export", "filename=$preFile", "exportPSK=YES")) + $preBackupOk = Test-Path -LiteralPath $preFile if (Invoke-NetshNps -NetshArgs @("import", "filename=$file")) { Write-OutputColor " NPS configuration imported." -color "Success" - Write-OutputColor " Pre-import backup: $preFile" -color "Info" + if ($preBackupOk) { + Write-OutputColor " Pre-import backup: $preFile" -color "Info" + Add-UndoAction -Category "Network" -Description "Imported NPS config" -UndoScript { + param($PrePath) + if (Test-Path -LiteralPath $PrePath) { [void](& netsh nps import filename="$PrePath" 2>&1) } + } -UndoParams @{ PrePath = $preFile } + } else { + Write-OutputColor " WARNING: pre-import backup could not be created — this import is NOT undoable." -color "Warning" + } Add-SessionChange -Category "Network" -Description "Imported NPS config from $file" - Add-UndoAction -Category "Network" -Description "Imported NPS config" -UndoScript { - param($PrePath) - if (Test-Path -LiteralPath $PrePath) { [void](& netsh nps import filename="$PrePath" 2>&1) } - } -UndoParams @{ PrePath = $preFile } Clear-MenuCache } } diff --git a/Modules/76-SIEMForwarder.ps1 b/Modules/76-SIEMForwarder.ps1 index 30f3c23..84b178c 100644 --- a/Modules/76-SIEMForwarder.ps1 +++ b/Modules/76-SIEMForwarder.ps1 @@ -323,8 +323,19 @@ function Write-SIEMConfigFile { $leaf = [System.IO.Path]::GetFileName($TargetPath) $backup = $null if (Test-Path -LiteralPath $TargetPath) { + # A prior config exists — it MUST be backed up before we overwrite it, or an Undo can't + # restore it. The old code copied with -EA SilentlyContinue and still returned the backup + # path even if the copy failed; Restore-SIEMConfig then saw the (missing) backup, fell + # through, and DELETED the live config on undo instead of restoring the prior one. Fail + # closed: if we can't take the backup, don't overwrite the existing config at all. $backup = Join-Path $secureDir "$leaf.$stamp.bak" - Copy-Item -LiteralPath $TargetPath -Destination $backup -Force -ErrorAction SilentlyContinue + try { + Copy-Item -LiteralPath $TargetPath -Destination $backup -Force -ErrorAction Stop + } + catch { + Write-OutputColor " Failed to back up existing $leaf; leaving it in place: $($_.Exception.Message)" -color "Error" + return @{ Ok = $false; Backup = $null; Target = $TargetPath } + } Write-OutputColor " Backed up existing $leaf to: $backup" -color "Info" } $stage = Join-Path $secureDir "$leaf.$stamp.stage" diff --git a/README.md b/README.md index de52a81..4c59e0c 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ OpenSSF Best Practices codecov PSScriptAnalyzer 0 errors - 5332 structural tests + 5341 structural tests Pester 312 tests SLSA Level 3

diff --git a/RackStack.ps1 b/RackStack.ps1 index 3e42c24..a33656c 100644 --- a/RackStack.ps1 +++ b/RackStack.ps1 @@ -13,7 +13,7 @@ Environment-specific settings are configured via defaults.json. .VERSION - 1.121.11 + 1.121.12 .NOTES - Requires Windows Server 2012 R2 or later (or Windows 10/11 for testing) - Must be run as Administrator diff --git a/RackStack.psd1 b/RackStack.psd1 index c212d6e..b923ada 100644 --- a/RackStack.psd1 +++ b/RackStack.psd1 @@ -1,6 +1,6 @@ @{ RootModule = 'RackStack.psm1' - ModuleVersion = '1.121.11' + ModuleVersion = '1.121.12' GUID = 'c19b8e71-4a35-4f2b-9d06-8a24f7bc0e91' Author = 'TheAbider' CompanyName = 'TheAbider' diff --git a/Tests/Run-Tests.ps1 b/Tests/Run-Tests.ps1 index 3e85e96..7ae400b 100644 --- a/Tests/Run-Tests.ps1 +++ b/Tests/Run-Tests.ps1 @@ -1,6 +1,6 @@ <# .SYNOPSIS - Automated Test Runner for RackStack v1.121.11 + Automated Test Runner for RackStack v1.121.12 .DESCRIPTION Comprehensive non-interactive test suite covering: @@ -10015,6 +10015,47 @@ catch { Write-TestResult "State-Integrity Hardening Tests" $false $_.Exception.Message } +# ============================================================================ +# SECTION 199: BACKUP / UNDO SAFETY-NET HONESTY (v1.121.12) +# ============================================================================ +# Guards the fixes from the security-feature-module audit (71-76). Common class: a backup/ +# snapshot taken with -EA SilentlyContinue (or its result discarded), then trusted downstream +# as if it exists — so a silently-failed backup made the exit code lie, missed drift, or (worst) +# made an "undo" delete the live config. +# [0] GPOBackup returns $null (exit 1) when 0 GPOs actually backed up. +# [1] GPO drift tracks an Indeterminate bucket instead of calling a missing baseline "unchanged". +# [2] JEA dry-run apply throws on a validation failure (return $false) so it isn't marked applied. +# [3] NPS import verifies the pre-import backup landed before claiming it / registering undo. +# [4] GPO restore only registers an undo when the pre-restore snapshot actually succeeded. +# [5] SIEM config write fails closed if it can't back up the prior config (undo never deletes it). +Write-SectionHeader "SECTION 199: BACKUP / UNDO SAFETY-NET HONESTY" + +try { + $gpoRaw = Get-Content "$modulesPath\71-GPOManager.ps1" -Raw + $jeaRaw = Get-Content "$modulesPath\72-JEA.ps1" -Raw + $npsRaw = Get-Content "$modulesPath\73-NPS.ps1" -Raw + $siemRaw = Get-Content "$modulesPath\76-SIEMForwarder.ps1" -Raw + + # [0] + Write-TestResult "71-GPO: backup returns null when nothing was backed up [0]" ($gpoRaw -match 'if \(\$ok -eq 0\) \{[\s\S]{0,400}return \$null') + # [1] + Write-TestResult "71-GPO: drift tracks an Indeterminate bucket [1]" (($gpoRaw -match '\$indeterminate = @\(\)') -and ($gpoRaw -match 'Indeterminate = \$indeterminate')) + Write-TestResult "71-GPO: unverifiable compare is indeterminate, not unchanged [1]" ($gpoRaw -match 'reliable compare wasn''t possible[\s\S]{0,600}\$indeterminate \+= \$g\.DisplayName') + # [2] + Write-TestResult "72-JEA: dry-run apply throws on validation failure [2]" ($jeaRaw -match 'if \(-not \(Invoke-JEAEndpointBuild[\s\S]{0,200}throw') + # [3] + Write-TestResult "73-NPS: import verifies the pre-import backup exists [3]" (($npsRaw -match '\$preBackupOk = Test-Path -LiteralPath \$preFile') -and ($npsRaw -match 'if \(\$preBackupOk\)')) + Write-TestResult "73-NPS: dry-run undo warns when no backup was captured [3]" ($npsRaw -match 'Undo unavailable: no pre-import NPS backup') + # [4] + Write-TestResult "71-GPO: restore undo gated on a successful snapshot [4]" (($gpoRaw -match '\$preSnapshotOk = \(\$null -ne \$snap\)') -and ($gpoRaw -match 'if \(\$preSnapshotOk\)')) + Write-TestResult "71-GPO: dry-run restore undo verifies snapshot content [4]" ($gpoRaw -match '\$snapDirs = @\(Get-ChildItem -LiteralPath \$capPre -Directory') + # [5] + Write-TestResult "76-SIEM: config write fails closed if it can't back up the prior config [5]" ($siemRaw -match 'Copy-Item -LiteralPath \$TargetPath -Destination \$backup -Force -ErrorAction Stop[\s\S]{0,260}return @\{ Ok = \$false; Backup = \$null') +} +catch { + Write-TestResult "Backup/Undo Safety-Net Tests" $false $_.Exception.Message +} + # ============================================================================ # SECTION 174: DOCUMENTATION FRESHNESS (counts must match the codebase) # ============================================================================