-
-
Notifications
You must be signed in to change notification settings - Fork 2k
feat(registry): add GPO override warning alerts and WhatIf dry-run previews #611
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
ca28792
c606514
80d8f1d
4038148
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -133,6 +133,12 @@ function Restore-RegistryBackupState { | |
|
|
||
| $friendlyTarget = GetFriendlyRegistryBackupTarget -Target ([string]$Backup.Target) | ||
|
|
||
| $isWhatIf = $null -ne $script:Params -and $script:Params.ContainsKey("WhatIf") | ||
| if ($isWhatIf) { | ||
| Write-Host "[WhatIf] Restore registry backup for $friendlyTarget" -ForegroundColor Cyan | ||
| return [PSCustomObject]@{ Result = $true } | ||
| } | ||
|
Comment on lines
+136
to
+140
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WhatIf handler should return a result indicator for consistency. The function returns void during WhatIf mode, but the caller in According to the review stack context, Proposed fix to return a result indicator $isWhatIf = $null -ne $script:Params -and $script:Params.ContainsKey("WhatIf")
if ($isWhatIf) {
Write-Host "[WhatIf] Restore registry backup for $friendlyTarget" -ForegroundColor Cyan
- return
+ return [PSCustomObject]@{ Result = $true }
}Then update the caller in - Restore-RegistryBackupState -Backup $backup
- $restoreResult.RestoredRegistry = $true
+ $restoreOpResult = Restore-RegistryBackupState -Backup $backup
+ if ($restoreOpResult -and $restoreOpResult.Result) {
+ $restoreResult.RestoredRegistry = $true
+ }🤖 Prompt for AI Agents |
||
|
|
||
| $restoreAction = { | ||
| param($normalizedBackup) | ||
|
|
||
|
|
@@ -148,9 +154,10 @@ function Restore-RegistryBackupState { | |
| Write-Host "Restore requires loading target user hive." | ||
| Invoke-WithLoadedRestoreHive -Target $Backup.Target -ScriptBlock $restoreAction -ArgumentObject $Backup | ||
| Write-Host "Restore completed for $friendlyTarget." | ||
| return | ||
| return [PSCustomObject]@{ Result = $true } | ||
| } | ||
|
|
||
| & $restoreAction $Backup | ||
| Write-Host "Restore completed for $friendlyTarget." | ||
| return [PSCustomObject]@{ Result = $true } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,3 +81,4 @@ function Get-RegistryFilePathForFeature { | |
|
|
||
| return Join-Path $script:RegfilesPath $RegistryKey | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -220,6 +220,17 @@ else { | |
| Start-Transcript -Path $script:DefaultLogPath -Append -IncludeInvocationHeader -Force | Out-Null | ||
| } | ||
|
|
||
| # Check if the device is domain-joined and warn the user (Group Policy may override changes) | ||
| try { | ||
| $computerSystem = Get-CimInstance Win32_ComputerSystem -ErrorAction SilentlyContinue | ||
| if ($null -ne $computerSystem -and $computerSystem.PartOfDomain) { | ||
| Write-Warning "This machine is domain-joined. Group Policy may override changes made by Win11Debloat." | ||
| } | ||
| } | ||
| catch { | ||
| # Suppress WMI/CIM query and null reference errors to ensure the script does not crash on startup | ||
| } | ||
|
Comment on lines
+230
to
+232
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: In PowerShell, using an empty catch block is considered a poor design practice and is explicitly flagged as a warning by the PSScriptAnalyzer rule AvoidUsingEmptyCatchBlock [1]. Empty catch blocks swallow errors, which makes debugging significantly more difficult because the script provides no feedback that a failure occurred [2][3]. For non-critical startup checks, using Write-Verbose is a significantly better practice than an empty catch block because it provides visibility into the failure without cluttering the output stream for normal users [4][2]. However, the most robust approach for production-grade scripts is to ensure failures are logged properly [2][3]. Best practices for error handling in these scenarios include: 1. Avoid Silent Failures: Never leave a catch block empty [2][3][5]. At a minimum, you should log the error using Write-Verbose, Write-Warning, or an appropriate logging mechanism so that you have a trail if the check fails [4][2]. 2. Distinguish Error Types: Ensure that the cmdlets inside your try block are actually raising terminating errors, as many cmdlets are non-terminating by default [2]. Use -ErrorAction Stop or set $ErrorActionPreference = 'Stop' within the scope to ensure exceptions are caught [4][2][3]. 3. Log Effectively: In the catch block, capture the error information (using $_ or $PSItem) [6][7]. If the error is non-critical, Write-Verbose is appropriate; if it requires attention, consider Write-Warning or Write-Error [1][4][2]. 4. Consider Cleanup: If the startup check involves resources that need to be released (e.g., database connections, file handles), use a finally block to ensure cleanup occurs regardless of whether the check succeeded or failed [4][2][8]. By replacing an empty catch block with a logged event, you ensure that your script fails "loudly enough" to be debugged without creating unnecessary noise during normal operation [3]. Citations:
🏁 Script executed: #!/bin/bash
# Find and examine the Win11Debloat.ps1 file around lines 230-232
fd -t f "Win11Debloat.ps1" | head -1 | xargs -I {} sh -c 'wc -l "{}" && sed -n "225,235p" "{}" | cat -n'Repository: Raphire/Win11Debloat Length of output: 1042 Replace empty catch block with verbose logging to aid diagnostics. The catch block at lines 230-232 silently suppresses all WMI/CIM query failures without logging, making it harder to diagnose startup issues. While the try block correctly handles non-critical domain-join detection, the catch should emit a Proposed fix catch {
- # Suppress WMI/CIM query and null reference errors to ensure the script does not crash on startup
+ Write-Verbose "Domain-join check skipped: $($_.Exception.Message)"
}🧰 Tools🪛 PSScriptAnalyzer (1.25.0)[warning] 230-232: Empty catch block is used. Please use Write-Error or throw statements in catch blocks. (PSAvoidUsingEmptyCatchBlock) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
|
|
||
| # Check if script has all required files | ||
| if (-not ((Test-Path $script:DefaultSettingsFilePath) -and (Test-Path $script:AppsListFilePath) -and (Test-Path $script:RegfilesPath) -and (Test-Path $script:AssetsPath) -and (Test-Path $script:AppSelectionSchema) -and (Test-Path $script:ApplyChangesWindowSchema) -and (Test-Path $script:SharedStylesSchema) -and (Test-Path $script:BubbleHintSchema) -and (Test-Path $script:RestoreBackupWindowSchema) -and (Test-Path $script:FeaturesFilePath))) { | ||
| Write-Error "Win11Debloat is unable to find required files, please ensure all script files are present" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
In PowerShell, does using "$path:" inside a double-quoted string produce an InvalidVariableReferenceWithDrive parse error, and is "${path}:" the correct fix?💡 Result:
Yes, placing a colon (:) immediately after a variable name inside a double-quoted string in PowerShell causes a parse error, typically identified by the error ID InvalidVariableReferenceWithDrive [1][2][3]. This happens because PowerShell's parser interprets the dollar sign, the variable name, and the immediately following colon as a namespace variable reference (or drive-qualified reference), such as the notation used for environment variables (e.g.,$env:PATH) [1][4]. When the characters following the colon do not form a valid path or item name in that context, the parser throws an error because it expects a valid drive-qualified reference [1][2][5]. Using curly braces to delimit the variable name, as in "$ {path}:", is the correct and standard fix [6][1][2]. This syntax explicitly tells the PowerShell parser that the variable name is "$path" and that the colon is a literal character following the variable, rather than part of the variable reference itself [6][1][5]. Top results: [6][1][2]
Citations:
🏁 Script executed:
Repository: Raphire/Win11Debloat
Length of output: 5268
Fix invalid variable interpolation in string formatting.
Line 26 uses
$path:inside a double-quoted string, which PowerShell parses as an invalid drive-qualified variable reference, triggering anInvalidVariableReferenceWithDriveerror. Use${path}:instead to explicitly delimit the variable name from the following literal colon.Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[error] 26-26: Variable reference is not valid. ':' was not followed by a valid variable name character. Consider using ${} to delimit the name.
(InvalidVariableReferenceWithDrive)
[warning] Missing BOM encoding for non-ASCII encoded file 'ImportRegistryFile.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Source: Linters/SAST tools