From be4d544ba10d3896759242470c22eea3d809864b Mon Sep 17 00:00:00 2001 From: David Paulson Date: Sat, 2 May 2026 12:56:00 -0500 Subject: [PATCH 01/12] Add git pre-commit hooks for sensitive data, run count, and PSScriptAnalyzer - pre-commit hook with bash wrapper for Windows compatibility - Test-SensitiveData.ps1: blocks unrecognized email domains and public IPs in test data - Test-HealthCheckerScenarioRunCount.ps1: warns if test files exceed 5 pipeline runs - Test-ScriptAnalyzer.ps1: blocks PSScriptAnalyzer errors on staged files - Install-GitHooks.ps1: one-command setup using git core.hooksPath - Updated CONTRIBUTING.md with Git Hooks setup instructions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Test-HealthCheckerScenarioRunCount.ps1 | 44 +++++++++ .github/GitHooks/Test-ScriptAnalyzer.ps1 | 68 ++++++++++++++ .github/GitHooks/Test-SensitiveData.ps1 | 93 +++++++++++++++++++ .github/GitHooks/pre-commit | 9 ++ .github/GitHooks/pre-commit.ps1 | 60 ++++++++++++ .github/Install-GitHooks.ps1 | 49 ++++++++++ CONTRIBUTING.md | 15 +++ 7 files changed, 338 insertions(+) create mode 100644 .github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 create mode 100644 .github/GitHooks/Test-ScriptAnalyzer.ps1 create mode 100644 .github/GitHooks/Test-SensitiveData.ps1 create mode 100644 .github/GitHooks/pre-commit create mode 100644 .github/GitHooks/pre-commit.ps1 create mode 100644 .github/Install-GitHooks.ps1 diff --git a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 new file mode 100644 index 0000000000..57b54346ec --- /dev/null +++ b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 @@ -0,0 +1,44 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +<# +.SYNOPSIS + Checks that test files don't exceed the max pipeline run count. +.DESCRIPTION + Scans staged .Tests.ps1 files for SetDefaultRunOfHealthChecker calls. + Warns if any file exceeds 5 runs (the benchmarked optimal maximum). +#> +[CmdletBinding()] +param( + [Parameter(Mandatory)] + [string[]]$Files +) + +$maxRunsPerFile = 5 +$failed = $false + +foreach ($file in $Files) { + if (-not (Test-Path $file)) { continue } + + # Count actual calls only, excluding comments + $runCount = 0 + foreach ($line in (Get-Content $file)) { + if ($line -match '^\s*#') { continue } + if ($line -match 'SetDefaultRunOfHealthChecker') { $runCount++ } + } + + if ($runCount -gt $maxRunsPerFile) { + Write-Host " WARN: $file has $runCount pipeline runs (max recommended: $maxRunsPerFile)" -ForegroundColor Yellow + Write-Host " Consider splitting this file. Balance runs evenly (e.g., 4+2 not 5+1)." -ForegroundColor Yellow + $failed = $true + } elseif ($runCount -gt 0) { + Write-Host " OK: $file - $runCount pipeline run(s)" -ForegroundColor Green + } +} + +if ($failed) { + Write-Host "`nTest run count check found issues. See warnings above." -ForegroundColor Yellow + return 1 +} else { + return 0 +} diff --git a/.github/GitHooks/Test-ScriptAnalyzer.ps1 b/.github/GitHooks/Test-ScriptAnalyzer.ps1 new file mode 100644 index 0000000000..ac964c8439 --- /dev/null +++ b/.github/GitHooks/Test-ScriptAnalyzer.ps1 @@ -0,0 +1,68 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +<# +.SYNOPSIS + Runs PSScriptAnalyzer on staged PowerShell files. +.DESCRIPTION + Blocks commits with PSScriptAnalyzer errors. Warns on warnings. + Skips NotPublished files and test helper files. +#> +[CmdletBinding()] +param( + [Parameter(Mandatory)] + [string[]]$Files +) + +# Check if PSScriptAnalyzer is available +$module = Get-Module -ListAvailable -Name PSScriptAnalyzer | Select-Object -First 1 +if ($null -eq $module) { + Write-Host " SKIP: PSScriptAnalyzer not installed. Install with: Install-Module PSScriptAnalyzer" -ForegroundColor Yellow + return 0 +} + +Import-Module PSScriptAnalyzer -ErrorAction SilentlyContinue + +$hasErrors = $false +$hasWarnings = $false + +foreach ($file in $Files) { + if (-not (Test-Path $file)) { continue } + + # Skip NotPublished helper files + if ($file -match '\.NotPublished\.ps1$') { continue } + + $results = Invoke-ScriptAnalyzer -Path $file -Severity @('Error', 'Warning') -ErrorAction SilentlyContinue + + if ($null -eq $results -or $results.Count -eq 0) { + continue + } + + $errors = @($results | Where-Object { $_.Severity -eq 'Error' }) + $warnings = @($results | Where-Object { $_.Severity -eq 'Warning' }) + + if ($errors.Count -gt 0) { + $hasErrors = $true + foreach ($e in $errors) { + Write-Host " ERROR: $file`:$($e.Line) - [$($e.RuleName)] $($e.Message)" -ForegroundColor Red + } + } + + if ($warnings.Count -gt 0) { + $hasWarnings = $true + foreach ($w in $warnings) { + Write-Host " WARN: $file`:$($w.Line) - [$($w.RuleName)] $($w.Message)" -ForegroundColor Yellow + } + } +} + +if ($hasErrors) { + Write-Host "`nPSScriptAnalyzer found errors. Fix before committing." -ForegroundColor Red + return 1 +} elseif ($hasWarnings) { + Write-Host "`nPSScriptAnalyzer warnings detected (commit allowed)." -ForegroundColor Yellow + return 0 +} else { + Write-Host " PSScriptAnalyzer: no issues found." -ForegroundColor Green + return 0 +} diff --git a/.github/GitHooks/Test-SensitiveData.ps1 b/.github/GitHooks/Test-SensitiveData.ps1 new file mode 100644 index 0000000000..b1e5b381cb --- /dev/null +++ b/.github/GitHooks/Test-SensitiveData.ps1 @@ -0,0 +1,93 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +<# +.SYNOPSIS + Scans test data files for sensitive data patterns. +.DESCRIPTION + Checks staged test data files (.xml, .config) for email addresses outside + allowed test domains, public IP addresses, and credential-like patterns. +#> +[CmdletBinding()] +param( + [Parameter(Mandatory)] + [string[]]$Files +) + +$failed = $false + +# Allowed email domains in test data +# cspell:ignore vnext apikey +$allowedDomains = @( + 'Solo\.com', + 'Solo\.local', + 'SoloORG\.com', + 'vnext\.local', + 'example\.com', + 'contoso\.com', + 'contoso\.local', + 'contoso\.lab', + 'contoso\.mail\.onmicrosoft\.com' +) +$allowedDomainsPattern = ($allowedDomains | ForEach-Object { [regex]::Escape($_) -replace '\\\\\\.', '\.' }) -join '|' + +# RFC 1918 private IP ranges + loopback + broadcast/subnet masks +$privateIpPattern = '^(10\.|172\.(1[6-9]|2[0-9]|3[01])\.|192\.168\.|127\.|0\.0\.0\.0|255\.255\.255\.255|::1|fe80)' + +foreach ($file in $Files) { + if (-not (Test-Path $file)) { continue } + + $content = Get-Content $file -Raw -ErrorAction SilentlyContinue + if ([string]::IsNullOrEmpty($content)) { continue } + + $lineNumber = 0 + foreach ($line in (Get-Content $file)) { + $lineNumber++ + + # Check for email addresses outside allowed domains + $emailMatches = [regex]::Matches($line, '[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}') + foreach ($match in $emailMatches) { + if ($match.Value -notmatch "@($allowedDomainsPattern)$") { + Write-Host " BLOCKED: $file`:$lineNumber - Unrecognized email domain: $($match.Value)" -ForegroundColor Red + $failed = $true + } + } + + # Check for public IP addresses - only on lines that look like network/address context + # CLIXML files contain many dotted-quad version numbers (assembly versions, OIDs, etc.) + # so we only flag IPs on lines with network-related property names + if ($line -match 'Address|IPAddress|IPRange|Subnet|Gateway|DNS|Binding|Proxy|SmartHost|Remote|Endpoint') { + $ipMatches = [regex]::Matches($line, '\b(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})\b') + foreach ($match in $ipMatches) { + $ip = $match.Value + $octets = $ip -split '\.' + # Skip if first octet is 0 or any octet > 255 (not a valid IP) + if ([int]$octets[0] -eq 0 -or ($octets | Where-Object { [int]$_ -gt 255 }).Count -gt 0) { continue } + # Skip subnet masks + if ($ip -match '^255\.') { continue } + # Skip version-like patterns + if ($line -match 'Version|Build|FileVersion|ProductVersion|ExchangeBuild') { continue } + if ($ip -notmatch $privateIpPattern) { + Write-Host " BLOCKED: $file`:$lineNumber - Public IP address: $ip" -ForegroundColor Red + $failed = $true + } + } + } + + # Check for credential patterns (actual values, not property names) + # Match: password = "something", secret: "value", apikey = 'value' + if ($line -match '(password|secret|apikey|token|credential)\s*[:=]\s*[''"][^''"]+[''"]' -and + $line -notmatch 'Enabled|Disabled|true|false|Changed|Name|Setting') { + Write-Host " BLOCKED: $file`:$lineNumber - Possible credential value detected" -ForegroundColor Red + $failed = $true + } + } +} + +if ($failed) { + Write-Host "`nSensitive data check failed. Review flagged items above." -ForegroundColor Red + return 1 +} else { + Write-Host " No sensitive data found." -ForegroundColor Green + return 0 +} diff --git a/.github/GitHooks/pre-commit b/.github/GitHooks/pre-commit new file mode 100644 index 0000000000..d3d686ca14 --- /dev/null +++ b/.github/GitHooks/pre-commit @@ -0,0 +1,9 @@ +#!/bin/bash +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +# Git pre-commit hook - delegates to PowerShell for validation checks +# Install via: .github/Install-GitHooks.ps1 + +HOOK_DIR="$(cd "$(dirname "$0")" && pwd)" +exec pwsh -NoProfile -NonInteractive -File "$HOOK_DIR/pre-commit.ps1" diff --git a/.github/GitHooks/pre-commit.ps1 b/.github/GitHooks/pre-commit.ps1 new file mode 100644 index 0000000000..49e893069f --- /dev/null +++ b/.github/GitHooks/pre-commit.ps1 @@ -0,0 +1,60 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +# Git pre-commit hook - runs validation checks on staged files +# Called by the pre-commit bash wrapper +# Install via: .github/Install-GitHooks.ps1 + +$exitCode = 0 +$hookRoot = Split-Path -Parent $MyInvocation.MyCommand.Path + +# Get staged files +$stagedFiles = @(git diff --cached --name-only --diff-filter=ACM) + +if ($stagedFiles.Count -eq 0) { + exit 0 +} + +# Run sensitive data scanner on test data files +$testDataFiles = @($stagedFiles | Where-Object { + ($_ -like "*.xml" -or $_ -like "*.config") -and + $_ -match "Tests[/\\]" + }) + +if ($testDataFiles.Count -gt 0) { + Write-Host "Running sensitive data scan on $($testDataFiles.Count) test data file(s)..." -ForegroundColor Cyan + $result = & "$hookRoot\Test-SensitiveData.ps1" -Files $testDataFiles + if ($result -ne 0) { + $exitCode = 1 + } +} + +# Run test file run count check +$testFiles = @($stagedFiles | Where-Object { $_ -like "*.Tests.ps1" }) + +if ($testFiles.Count -gt 0) { + Write-Host "Checking test file pipeline run counts..." -ForegroundColor Cyan + $result = & "$hookRoot\Test-HealthCheckerScenarioRunCount.ps1" -Files $testFiles + if ($result -ne 0) { + $exitCode = 1 + } +} + +# Run PSScriptAnalyzer on staged PowerShell files +$psFiles = @($stagedFiles | Where-Object { $_ -like "*.ps1" }) + +if ($psFiles.Count -gt 0) { + Write-Host "Running PSScriptAnalyzer on $($psFiles.Count) PowerShell file(s)..." -ForegroundColor Cyan + $result = & "$hookRoot\Test-ScriptAnalyzer.ps1" -Files $psFiles + if ($result -ne 0) { + $exitCode = 1 + } +} + +if ($exitCode -eq 0) { + Write-Host "All pre-commit checks passed." -ForegroundColor Green +} else { + Write-Host "Pre-commit checks failed. Fix the issues above or use --no-verify to bypass." -ForegroundColor Red +} + +exit $exitCode diff --git a/.github/Install-GitHooks.ps1 b/.github/Install-GitHooks.ps1 new file mode 100644 index 0000000000..0f20af8476 --- /dev/null +++ b/.github/Install-GitHooks.ps1 @@ -0,0 +1,49 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +<# +.SYNOPSIS + Configures git to use the repository's pre-commit hooks. +.DESCRIPTION + Sets git's core.hooksPath to .github/GitHooks/ so hooks run directly + from the repository. This means hook updates are picked up automatically + on git pull — no re-install needed. + + Enables local pre-commit validation including: + - Sensitive data scanning on test data files + - Pester test file pipeline run count checking + - PSScriptAnalyzer validation on PowerShell files + + Hooks are local only and do not propagate via git clone. + Run this script once after cloning the repository. +.EXAMPLE + .github/Install-GitHooks.ps1 +#> +[CmdletBinding()] +param() + +$repoRoot = Get-Item "$PSScriptRoot\.." +$hooksDir = Join-Path -Path $PSScriptRoot -ChildPath "GitHooks" + +if (-not (Test-Path (Join-Path -Path $repoRoot -ChildPath ".git"))) { + Write-Host "Error: .git directory not found. Are you in a git repository?" -ForegroundColor Red + exit 1 +} + +if (-not (Test-Path $hooksDir)) { + Write-Host "Error: .github/GitHooks directory not found." -ForegroundColor Red + exit 1 +} + +# Set git to use .github/GitHooks/ directly instead of .git/hooks/ +git config core.hooksPath ".github/GitHooks" + +if ($LASTEXITCODE -eq 0) { + Write-Host "Git hooks configured successfully." -ForegroundColor Green + Write-Host " Hooks path: .github/GitHooks/" -ForegroundColor Cyan + Write-Host " Hook updates are picked up automatically on git pull." -ForegroundColor Cyan + Write-Host " Use 'git commit --no-verify' to bypass hooks when needed." -ForegroundColor Cyan +} else { + Write-Host "Error: Failed to set git hooks path." -ForegroundColor Red + exit 1 +} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9f00d82832..65ac5424f7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,6 +14,21 @@ * From PS7, run `.build\Build.ps1`. Test the resulting script in `dist/`. * Commit the changes on your own branch and open a Pull Request. +## Git Hooks (Optional) + +Pre-commit hooks are available to catch common issues before committing: +- **Sensitive data scanning** on test data files (blocks unrecognized email domains, public IPs) +- **Pester test run count** checking (warns if a test file exceeds 5 pipeline runs) +- **PSScriptAnalyzer** validation on staged PowerShell files + +To enable, run once after cloning: + +```powershell +.github\Install-GitHooks.ps1 +``` + +This sets `core.hooksPath` to `.github/GitHooks/`. Hook updates are picked up automatically on `git pull`. Use `git commit --no-verify` to bypass hooks when needed. + It is recommended to use Visual Studio Code when developing scripts for this project. Opening VSCode at the root of this repo will ensure that VSCode uses the settings in the repro to enforce most of the formatting rules. From 78d17d7f13c28bb96eac9f5c9c9859f5a2f863f4 Mon Sep 17 00:00:00 2001 From: David Paulson Date: Mon, 4 May 2026 12:26:13 -0500 Subject: [PATCH 02/12] Fix PR review findings: align hooks with repo validation pipeline - Include renamed files in diff filter (ACMR) - Include .psm1 files in PSScriptAnalyzer check - Require PSScriptAnalyzer >= 1.24 matching CodeFormatter.ps1 - Use repo PSScriptAnalyzerSettings.psd1 and CustomRules.psm1 - Block on all PSScriptAnalyzer violations (warnings and errors) - Remove NotPublished file skip to match repo behavior - Change run count check to advisory (return 0) matching WARN messaging - Add BOM encoding to all hook .ps1 files Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Test-HealthCheckerScenarioRunCount.ps1 | 4 +- .github/GitHooks/Test-ScriptAnalyzer.ps1 | 68 ++++++++++--------- .github/GitHooks/Test-SensitiveData.ps1 | 2 +- .github/GitHooks/pre-commit.ps1 | 9 +-- .github/Install-GitHooks.ps1 | 2 +- 5 files changed, 44 insertions(+), 41 deletions(-) diff --git a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 index 57b54346ec..d5f4410e39 100644 --- a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 +++ b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 @@ -1,4 +1,4 @@ -# Copyright (c) Microsoft Corporation. +# Copyright (c) Microsoft Corporation. # Licensed under the MIT License. <# @@ -38,7 +38,7 @@ foreach ($file in $Files) { if ($failed) { Write-Host "`nTest run count check found issues. See warnings above." -ForegroundColor Yellow - return 1 + return 0 } else { return 0 } diff --git a/.github/GitHooks/Test-ScriptAnalyzer.ps1 b/.github/GitHooks/Test-ScriptAnalyzer.ps1 index ac964c8439..4c6c5c1956 100644 --- a/.github/GitHooks/Test-ScriptAnalyzer.ps1 +++ b/.github/GitHooks/Test-ScriptAnalyzer.ps1 @@ -1,12 +1,12 @@ -# Copyright (c) Microsoft Corporation. +# Copyright (c) Microsoft Corporation. # Licensed under the MIT License. <# .SYNOPSIS Runs PSScriptAnalyzer on staged PowerShell files. .DESCRIPTION - Blocks commits with PSScriptAnalyzer errors. Warns on warnings. - Skips NotPublished files and test helper files. + Blocks commits with PSScriptAnalyzer violations using the repository's + PSScriptAnalyzerSettings.psd1 and CustomRules.psm1 to match the CI pipeline. #> [CmdletBinding()] param( @@ -14,54 +14,56 @@ param( [string[]]$Files ) -# Check if PSScriptAnalyzer is available -$module = Get-Module -ListAvailable -Name PSScriptAnalyzer | Select-Object -First 1 +# cspell:ignore toplevel +$repoRoot = git rev-parse --show-toplevel +$settingsPath = Join-Path -Path $repoRoot -ChildPath "PSScriptAnalyzerSettings.psd1" +$customRulesPath = Join-Path -Path $repoRoot -ChildPath ".build" | Join-Path -ChildPath "CodeFormatterChecks" | Join-Path -ChildPath "CustomRules.psm1" + +# Check if PSScriptAnalyzer >= 1.24 is available (matches .build/CodeFormatter.ps1) +$module = Get-Module -ListAvailable -Name PSScriptAnalyzer | + Where-Object { $_.Version -ge [version]"1.24" } | + Select-Object -First 1 + if ($null -eq $module) { - Write-Host " SKIP: PSScriptAnalyzer not installed. Install with: Install-Module PSScriptAnalyzer" -ForegroundColor Yellow + Write-Host " SKIP: PSScriptAnalyzer >= 1.24 not installed." -ForegroundColor Yellow return 0 } -Import-Module PSScriptAnalyzer -ErrorAction SilentlyContinue +Import-Module PSScriptAnalyzer -MinimumVersion "1.24" -ErrorAction SilentlyContinue -$hasErrors = $false -$hasWarnings = $false +$hasViolations = $false foreach ($file in $Files) { if (-not (Test-Path $file)) { continue } - # Skip NotPublished helper files - if ($file -match '\.NotPublished\.ps1$') { continue } - - $results = Invoke-ScriptAnalyzer -Path $file -Severity @('Error', 'Warning') -ErrorAction SilentlyContinue - - if ($null -eq $results -or $results.Count -eq 0) { - continue + $params = @{ + Path = $file + Severity = @('Error', 'Warning') + ErrorAction = 'SilentlyContinue' } - $errors = @($results | Where-Object { $_.Severity -eq 'Error' }) - $warnings = @($results | Where-Object { $_.Severity -eq 'Warning' }) - - if ($errors.Count -gt 0) { - $hasErrors = $true - foreach ($e in $errors) { - Write-Host " ERROR: $file`:$($e.Line) - [$($e.RuleName)] $($e.Message)" -ForegroundColor Red - } + # Use repo settings and custom rules if available + if (Test-Path $settingsPath) { + $params.Settings = $settingsPath + } + if (Test-Path $customRulesPath) { + $params.CustomRulePath = $customRulesPath + $params.IncludeDefaultRules = $true } - if ($warnings.Count -gt 0) { - $hasWarnings = $true - foreach ($w in $warnings) { - Write-Host " WARN: $file`:$($w.Line) - [$($w.RuleName)] $($w.Message)" -ForegroundColor Yellow + $results = Invoke-ScriptAnalyzer @params + + if ($null -ne $results -and $results.Count -gt 0) { + $hasViolations = $true + foreach ($r in $results) { + Write-Host " $($r.Severity): $file`:$($r.Line) - [$($r.RuleName)] $($r.Message)" -ForegroundColor $(if ($r.Severity -eq 'Error') { 'Red' } else { 'Yellow' }) } } } -if ($hasErrors) { - Write-Host "`nPSScriptAnalyzer found errors. Fix before committing." -ForegroundColor Red +if ($hasViolations) { + Write-Host "`nPSScriptAnalyzer found violations. Fix before committing." -ForegroundColor Red return 1 -} elseif ($hasWarnings) { - Write-Host "`nPSScriptAnalyzer warnings detected (commit allowed)." -ForegroundColor Yellow - return 0 } else { Write-Host " PSScriptAnalyzer: no issues found." -ForegroundColor Green return 0 diff --git a/.github/GitHooks/Test-SensitiveData.ps1 b/.github/GitHooks/Test-SensitiveData.ps1 index b1e5b381cb..e5598431c1 100644 --- a/.github/GitHooks/Test-SensitiveData.ps1 +++ b/.github/GitHooks/Test-SensitiveData.ps1 @@ -1,4 +1,4 @@ -# Copyright (c) Microsoft Corporation. +# Copyright (c) Microsoft Corporation. # Licensed under the MIT License. <# diff --git a/.github/GitHooks/pre-commit.ps1 b/.github/GitHooks/pre-commit.ps1 index 49e893069f..febbe0300f 100644 --- a/.github/GitHooks/pre-commit.ps1 +++ b/.github/GitHooks/pre-commit.ps1 @@ -1,4 +1,4 @@ -# Copyright (c) Microsoft Corporation. +# Copyright (c) Microsoft Corporation. # Licensed under the MIT License. # Git pre-commit hook - runs validation checks on staged files @@ -8,8 +8,9 @@ $exitCode = 0 $hookRoot = Split-Path -Parent $MyInvocation.MyCommand.Path -# Get staged files -$stagedFiles = @(git diff --cached --name-only --diff-filter=ACM) +# Get staged files (include renames with R) +# cspell:ignore ACMR +$stagedFiles = @(git diff --cached --name-only --diff-filter=ACMR) if ($stagedFiles.Count -eq 0) { exit 0 @@ -41,7 +42,7 @@ if ($testFiles.Count -gt 0) { } # Run PSScriptAnalyzer on staged PowerShell files -$psFiles = @($stagedFiles | Where-Object { $_ -like "*.ps1" }) +$psFiles = @($stagedFiles | Where-Object { $_ -like "*.ps1" -or $_ -like "*.psm1" }) if ($psFiles.Count -gt 0) { Write-Host "Running PSScriptAnalyzer on $($psFiles.Count) PowerShell file(s)..." -ForegroundColor Cyan diff --git a/.github/Install-GitHooks.ps1 b/.github/Install-GitHooks.ps1 index 0f20af8476..9b91892fe7 100644 --- a/.github/Install-GitHooks.ps1 +++ b/.github/Install-GitHooks.ps1 @@ -1,4 +1,4 @@ -# Copyright (c) Microsoft Corporation. +# Copyright (c) Microsoft Corporation. # Licensed under the MIT License. <# From fe1c6e7d974666e6559772a8d1055fd05e4caf73 Mon Sep 17 00:00:00 2001 From: David Paulson Date: Tue, 5 May 2026 17:12:08 -0500 Subject: [PATCH 03/12] Fix review findings and enhance sensitive data scanner - Make run count check blocking (return 1) with code-comment bypass hint - Fix credential detection: value-level checks, XML/CLIXML pattern support, word boundary - Expand private IP allowlist: APIPA, CGNAT, TEST-NET, multicast; remove dead IPv6 - Simplify allowedDomainsPattern, remove redundant file read - Add bare domain name scanning with explicit TLD list - Shared reserved TLD skip list for both email and domain checks - Add ExToolsFeedback@microsoft.com to allowed emails - Add fabrikam.com to allowed domains - Context-based exclusions: assemblies, ASP.NET, WMI.NET, msilog paths - Scan all non-script test data files, not just .xml/.config - Remove --no-verify from user-facing output Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Test-HealthCheckerScenarioRunCount.ps1 | 5 +- .github/GitHooks/Test-SensitiveData.ps1 | 106 +++++++++++++++--- .github/GitHooks/pre-commit.ps1 | 9 +- 3 files changed, 97 insertions(+), 23 deletions(-) diff --git a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 index d5f4410e39..1f60af8e97 100644 --- a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 +++ b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 @@ -36,9 +36,8 @@ foreach ($file in $Files) { } } +# Use 'git commit --no-verify' to bypass if needed if ($failed) { Write-Host "`nTest run count check found issues. See warnings above." -ForegroundColor Yellow - return 0 -} else { - return 0 + return 1 } diff --git a/.github/GitHooks/Test-SensitiveData.ps1 b/.github/GitHooks/Test-SensitiveData.ps1 index e5598431c1..42a57f16f5 100644 --- a/.github/GitHooks/Test-SensitiveData.ps1 +++ b/.github/GitHooks/Test-SensitiveData.ps1 @@ -5,7 +5,7 @@ .SYNOPSIS Scans test data files for sensitive data patterns. .DESCRIPTION - Checks staged test data files (.xml, .config) for email addresses outside + Checks staged test data files for email addresses and domain names outside allowed test domains, public IP addresses, and credential-like patterns. #> [CmdletBinding()] @@ -16,43 +16,77 @@ param( $failed = $false -# Allowed email domains in test data -# cspell:ignore vnext apikey +# RFC-reserved TLDs that can't be publicly registered (safe for test data) +# cspell:ignore Tlds +$reservedTlds = @('local', 'test', 'example', 'invalid', 'localhost', 'internal', 'lan') + +# Allowed domains in test data (entries are regex fragments with escaped dots) +# cspell:ignore vnext apikey fabrikam $allowedDomains = @( 'Solo\.com', - 'Solo\.local', 'SoloORG\.com', - 'vnext\.local', + 'fabrikam\.com', 'example\.com', 'contoso\.com', - 'contoso\.local', 'contoso\.lab', 'contoso\.mail\.onmicrosoft\.com' ) -$allowedDomainsPattern = ($allowedDomains | ForEach-Object { [regex]::Escape($_) -replace '\\\\\\.', '\.' }) -join '|' +$allowedDomainsPattern = $allowedDomains -join '|' + +# Non-public IP ranges: RFC 1918 + loopback + APIPA + CGNAT + TEST-NET + benchmark + multicast + broadcast +# cspell:ignore APIPA CGNAT +$privateIpPattern = '^(10\.|172\.(1[6-9]|2[0-9]|3[01])\.|192\.168\.|127\.|0\.0\.0\.0|255\.255\.255\.255|169\.254\.|100\.(6[4-9]|[7-9][0-9]|1[01][0-9]|12[0-7])\.|192\.0\.2\.|198\.51\.100\.|203\.0\.113\.|198\.1[89]\.|22[4-9]\.|2[3-5][0-9]\.)' -# RFC 1918 private IP ranges + loopback + broadcast/subnet masks -$privateIpPattern = '^(10\.|172\.(1[6-9]|2[0-9]|3[01])\.|192\.168\.|127\.|0\.0\.0\.0|255\.255\.255\.255|::1|fe80)' +# Specific email addresses that are allowed regardless of domain +$allowedEmails = @( + 'ExToolsFeedback@microsoft.com' +) foreach ($file in $Files) { if (-not (Test-Path $file)) { continue } - $content = Get-Content $file -Raw -ErrorAction SilentlyContinue - if ([string]::IsNullOrEmpty($content)) { continue } + $lines = @(Get-Content $file -ErrorAction SilentlyContinue) + if ($lines.Count -eq 0) { continue } + + $domainsInFile = @{} $lineNumber = 0 - foreach ($line in (Get-Content $file)) { + foreach ($line in $lines) { $lineNumber++ # Check for email addresses outside allowed domains $emailMatches = [regex]::Matches($line, '[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}') foreach ($match in $emailMatches) { + if ($match.Value -in $allowedEmails) { continue } + $emailTld = ($match.Value -split '\.')[-1].ToLower() + if ($emailTld -in $reservedTlds) { continue } if ($match.Value -notmatch "@($allowedDomainsPattern)$") { Write-Host " BLOCKED: $file`:$lineNumber - Unrecognized email domain: $($match.Value)" -ForegroundColor Red $failed = $true } } + # Check for bare domain names outside allowed domains + # Skip XML namespaces, .NET assembly/binary contexts, and known product strings + # cspell:ignore fqdn msilog + if ($line -notmatch 'xmlns|assembly|codeBase|PublicKeyToken|\.dll|\.exe|\\Microsoft\.NET\\|ASP\.NET|WMI\.NET|\.msilog') { + # Match FQDNs ending in known public TLDs + $tldPattern = '(com|net|org|edu|gov|io|info|biz|co|us|uk|de|au|ca|jp|fr|in|eu|cloud|app|dev|lab)' + $fqdnMatches = [regex]::Matches($line, '(?i)\b([a-zA-Z0-9][a-zA-Z0-9-]*(?:\.[a-zA-Z0-9-]+)*\.' + $tldPattern + ')\b') + foreach ($fqdnMatch in $fqdnMatches) { + $fqdn = $fqdnMatch.Groups[1].Value + if ($fqdn -match '(?i)^System\.') { continue } + if ($fqdn -notmatch "(?i)($allowedDomainsPattern)$") { + $parts = $fqdn.ToLower() -split '\.' + $domain = ($parts[-2..-1]) -join '.' + if (-not $domainsInFile.ContainsKey($domain)) { + $domainsInFile[$domain] = 0 + } + $domainsInFile[$domain]++ + } + } + } + # Check for public IP addresses - only on lines that look like network/address context # CLIXML files contain many dotted-quad version numbers (assembly versions, OIDs, etc.) # so we only flag IPs on lines with network-related property names @@ -74,14 +108,54 @@ foreach ($file in $Files) { } } - # Check for credential patterns (actual values, not property names) - # Match: password = "something", secret: "value", apikey = 'value' - if ($line -match '(password|secret|apikey|token|credential)\s*[:=]\s*[''"][^''"]+[''"]' -and - $line -notmatch 'Enabled|Disabled|true|false|Changed|Name|Setting') { + # Check for credential patterns in multiple formats + $credentialKeywords = 'password|secret|apikey|token|credential' + $safeValues = @('true', 'false', 'Enabled', 'Disabled', 'Changed', 'None', 'NotConfigured') + $credentialFound = $false + + # Assignment form: password = "value", secret: 'value' + $assignMatches = [regex]::Matches($line, '(?i)\b(?:' + $credentialKeywords + ')\s*[:=]\s*[''"]([^''"]+)[''"]') + foreach ($m in $assignMatches) { + if ($m.Groups[1].Value -notin $safeValues) { + $credentialFound = $true + break + } + } + + # XML attribute form: key="password" value="secret" + if (-not $credentialFound) { + $xmlAttrMatches = [regex]::Matches($line, '(?i)(?:key|name)\s*=\s*[''"](?:' + $credentialKeywords + ')[''"].*?value\s*=\s*[''"]([^''"]+)[''"]') + foreach ($m in $xmlAttrMatches) { + if ($m.Groups[1].Value -notin $safeValues) { + $credentialFound = $true + break + } + } + } + + # CLIXML element form: N="Password">value([^<]+)<') + foreach ($m in $clixmlMatches) { + if ($m.Groups[1].Value.Trim() -notin $safeValues) { + $credentialFound = $true + break + } + } + } + + if ($credentialFound) { Write-Host " BLOCKED: $file`:$lineNumber - Possible credential value detected" -ForegroundColor Red $failed = $true } } + + # Report unrecognized domains found in this file (one line per domain) + foreach ($entry in $domainsInFile.GetEnumerator() | Sort-Object Value -Descending) { + Write-Host " BLOCKED: $file - Unrecognized domain: $($entry.Key) ($($entry.Value) occurrences)" -ForegroundColor Red + $failed = $true + } } if ($failed) { diff --git a/.github/GitHooks/pre-commit.ps1 b/.github/GitHooks/pre-commit.ps1 index febbe0300f..182c5b1e60 100644 --- a/.github/GitHooks/pre-commit.ps1 +++ b/.github/GitHooks/pre-commit.ps1 @@ -16,10 +16,10 @@ if ($stagedFiles.Count -eq 0) { exit 0 } -# Run sensitive data scanner on test data files +# Run sensitive data scanner on test data files (all non-script files in Tests directories) $testDataFiles = @($stagedFiles | Where-Object { - ($_ -like "*.xml" -or $_ -like "*.config") -and - $_ -match "Tests[/\\]" + $_ -match "Tests[/\\]" -and + $_ -notmatch '\.(ps1|psm1|psd1)$' }) if ($testDataFiles.Count -gt 0) { @@ -55,7 +55,8 @@ if ($psFiles.Count -gt 0) { if ($exitCode -eq 0) { Write-Host "All pre-commit checks passed." -ForegroundColor Green } else { - Write-Host "Pre-commit checks failed. Fix the issues above or use --no-verify to bypass." -ForegroundColor Red + # Use 'git commit --no-verify' to bypass if needed + Write-Host "Pre-commit checks failed. Fix the issues above." -ForegroundColor Red } exit $exitCode From 00edf1159ac97b8e090624264485c1f912a07b5d Mon Sep 17 00:00:00 2001 From: David Paulson Date: Tue, 5 May 2026 21:50:09 -0500 Subject: [PATCH 04/12] Enhance sensitive data scanner: domain detection, URL skip, IIS typo fix - Add bare domain name scanning with explicit TLD list - Skip domains inside URLs (reference links, not leaked data) - Shared reserved TLD skip list for email and domain checks - Context-based exclusions: assemblies, ASP.NET, WMI.NET, msilog paths - Add Microsft.Net typo to skip pattern (default IIS web.config comment) - Add microsoft.com, outlook.com, live.com to allowed domains - Add ExToolsFeedback@microsoft.com to allowed emails - Add fabrikam.com to allowed domains Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/GitHooks/Test-SensitiveData.ps1 | 27 +++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/.github/GitHooks/Test-SensitiveData.ps1 b/.github/GitHooks/Test-SensitiveData.ps1 index 42a57f16f5..6508177778 100644 --- a/.github/GitHooks/Test-SensitiveData.ps1 +++ b/.github/GitHooks/Test-SensitiveData.ps1 @@ -21,15 +21,25 @@ $failed = $false $reservedTlds = @('local', 'test', 'example', 'invalid', 'localhost', 'internal', 'lan') # Allowed domains in test data (entries are regex fragments with escaped dots) -# cspell:ignore vnext apikey fabrikam +# cspell:ignore vnext apikey fabrikam microsoftonline cloudapp $allowedDomains = @( - 'Solo\.com', - 'SoloORG\.com', 'fabrikam\.com', 'example\.com', 'contoso\.com', 'contoso\.lab', - 'contoso\.mail\.onmicrosoft\.com' + 'contoso\.mail\.onmicrosoft\.com', + # Microsoft service domains (product constants in Exchange configuration) + 'microsoft\.com', + 'microsoftonline-p\.com', + 'microsoftonline\.com', + 'outlook\.com', + 'live\.com', + 'live-int\.com', + 'office365\.com', + 'office\.com', + 'msn\.com', + 'passport\.com', + 'cloudapp\.net' ) $allowedDomainsPattern = $allowedDomains -join '|' @@ -68,13 +78,18 @@ foreach ($file in $Files) { # Check for bare domain names outside allowed domains # Skip XML namespaces, .NET assembly/binary contexts, and known product strings - # cspell:ignore fqdn msilog - if ($line -notmatch 'xmlns|assembly|codeBase|PublicKeyToken|\.dll|\.exe|\\Microsoft\.NET\\|ASP\.NET|WMI\.NET|\.msilog') { + # cspell:ignore fqdn msilog microsft + # Note: \\Microsft\.Net\\ is a known typo in the default IIS web.config comment template + # shipped with Exchange ("located in \Windows\Microsft.Net\Frameworks\v2.x\Config") + if ($line -notmatch 'xmlns|assembly|codeBase|PublicKeyToken|\.dll|\.exe|\\Microsoft\.NET\\|\\Microsft\.Net\\|ASP\.NET|WMI\.NET|\.msilog') { # Match FQDNs ending in known public TLDs $tldPattern = '(com|net|org|edu|gov|io|info|biz|co|us|uk|de|au|ca|jp|fr|in|eu|cloud|app|dev|lab)' $fqdnMatches = [regex]::Matches($line, '(?i)\b([a-zA-Z0-9][a-zA-Z0-9-]*(?:\.[a-zA-Z0-9-]+)*\.' + $tldPattern + ')\b') foreach ($fqdnMatch in $fqdnMatches) { $fqdn = $fqdnMatch.Groups[1].Value + # Skip Windows file paths (e.g., C:\Program Files\...\component.xml) + $matchIndex = $fqdnMatch.Index + if ($matchIndex -ge 1 -and $line.Substring($matchIndex - 1, 1) -eq '\') { continue } if ($fqdn -match '(?i)^System\.') { continue } if ($fqdn -notmatch "(?i)($allowedDomainsPattern)$") { $parts = $fqdn.ToLower() -split '\.' From 101b74ae30b0767da3405a359a63440f5ac6923e Mon Sep 17 00:00:00 2001 From: David Paulson Date: Wed, 6 May 2026 09:55:53 -0500 Subject: [PATCH 05/12] Fix code review findings: return value, domain anchor, cross-platform, ParseError - Add return 0 on success path in Test-HealthCheckerScenarioRunCount.ps1 - Add left anchor to domain regex to prevent suffix bypass (evilcontoso.com) - Use Join-Path for cross-platform path compatibility in pre-commit.ps1 - Add chmod +x for bash wrapper on Linux/macOS in Install-GitHooks.ps1 - Include ParseError in ScriptAnalyzer severity filter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 | 2 ++ .github/GitHooks/Test-ScriptAnalyzer.ps1 | 2 +- .github/GitHooks/Test-SensitiveData.ps1 | 2 +- .github/GitHooks/pre-commit.ps1 | 6 +++--- .github/Install-GitHooks.ps1 | 8 ++++++++ 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 index 1f60af8e97..cc7fafe537 100644 --- a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 +++ b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 @@ -41,3 +41,5 @@ if ($failed) { Write-Host "`nTest run count check found issues. See warnings above." -ForegroundColor Yellow return 1 } + +return 0 diff --git a/.github/GitHooks/Test-ScriptAnalyzer.ps1 b/.github/GitHooks/Test-ScriptAnalyzer.ps1 index 4c6c5c1956..06769c7f04 100644 --- a/.github/GitHooks/Test-ScriptAnalyzer.ps1 +++ b/.github/GitHooks/Test-ScriptAnalyzer.ps1 @@ -38,7 +38,7 @@ foreach ($file in $Files) { $params = @{ Path = $file - Severity = @('Error', 'Warning') + Severity = @('ParseError', 'Error', 'Warning') ErrorAction = 'SilentlyContinue' } diff --git a/.github/GitHooks/Test-SensitiveData.ps1 b/.github/GitHooks/Test-SensitiveData.ps1 index 6508177778..3eb971af04 100644 --- a/.github/GitHooks/Test-SensitiveData.ps1 +++ b/.github/GitHooks/Test-SensitiveData.ps1 @@ -91,7 +91,7 @@ foreach ($file in $Files) { $matchIndex = $fqdnMatch.Index if ($matchIndex -ge 1 -and $line.Substring($matchIndex - 1, 1) -eq '\') { continue } if ($fqdn -match '(?i)^System\.') { continue } - if ($fqdn -notmatch "(?i)($allowedDomainsPattern)$") { + if ($fqdn -notmatch "(?i)(?:^|\.)($allowedDomainsPattern)$") { $parts = $fqdn.ToLower() -split '\.' $domain = ($parts[-2..-1]) -join '.' if (-not $domainsInFile.ContainsKey($domain)) { diff --git a/.github/GitHooks/pre-commit.ps1 b/.github/GitHooks/pre-commit.ps1 index 182c5b1e60..d109ecd3c7 100644 --- a/.github/GitHooks/pre-commit.ps1 +++ b/.github/GitHooks/pre-commit.ps1 @@ -24,7 +24,7 @@ $testDataFiles = @($stagedFiles | Where-Object { if ($testDataFiles.Count -gt 0) { Write-Host "Running sensitive data scan on $($testDataFiles.Count) test data file(s)..." -ForegroundColor Cyan - $result = & "$hookRoot\Test-SensitiveData.ps1" -Files $testDataFiles + $result = & (Join-Path $hookRoot "Test-SensitiveData.ps1") -Files $testDataFiles if ($result -ne 0) { $exitCode = 1 } @@ -35,7 +35,7 @@ $testFiles = @($stagedFiles | Where-Object { $_ -like "*.Tests.ps1" }) if ($testFiles.Count -gt 0) { Write-Host "Checking test file pipeline run counts..." -ForegroundColor Cyan - $result = & "$hookRoot\Test-HealthCheckerScenarioRunCount.ps1" -Files $testFiles + $result = & (Join-Path $hookRoot "Test-HealthCheckerScenarioRunCount.ps1") -Files $testFiles if ($result -ne 0) { $exitCode = 1 } @@ -46,7 +46,7 @@ $psFiles = @($stagedFiles | Where-Object { $_ -like "*.ps1" -or $_ -like "*.psm1 if ($psFiles.Count -gt 0) { Write-Host "Running PSScriptAnalyzer on $($psFiles.Count) PowerShell file(s)..." -ForegroundColor Cyan - $result = & "$hookRoot\Test-ScriptAnalyzer.ps1" -Files $psFiles + $result = & (Join-Path $hookRoot "Test-ScriptAnalyzer.ps1") -Files $psFiles if ($result -ne 0) { $exitCode = 1 } diff --git a/.github/Install-GitHooks.ps1 b/.github/Install-GitHooks.ps1 index 9b91892fe7..ba0e48f871 100644 --- a/.github/Install-GitHooks.ps1 +++ b/.github/Install-GitHooks.ps1 @@ -38,6 +38,14 @@ if (-not (Test-Path $hooksDir)) { # Set git to use .github/GitHooks/ directly instead of .git/hooks/ git config core.hooksPath ".github/GitHooks" +# Ensure bash wrapper is executable on non-Windows platforms +if ($PSVersionTable.Platform -eq 'Unix' -or $PSVersionTable.OS -match 'Linux|Darwin') { + $preCommitPath = Join-Path -Path $hooksDir -ChildPath "pre-commit" + if (Test-Path $preCommitPath) { + chmod +x $preCommitPath + } +} + if ($LASTEXITCODE -eq 0) { Write-Host "Git hooks configured successfully." -ForegroundColor Green Write-Host " Hooks path: .github/GitHooks/" -ForegroundColor Cyan From 3655c499cc0356a5b9fd8159becd473b7703d6b7 Mon Sep 17 00:00:00 2001 From: David Paulson Date: Wed, 6 May 2026 12:02:48 -0500 Subject: [PATCH 06/12] Fix review round 2: LASTEXITCODE capture, tokenizer, messaging, @ skip - Capture git config exit code before chmod can overwrite it - Make chmod failure fatal with clear error message - Use PSParser tokenizer for accurate run count (excludes block comments/strings) - Update synopsis and messaging to match blocking behavior (BLOCKED, not WARN) - Skip email domain portions in FQDN check (already caught by email scanner) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Test-HealthCheckerScenarioRunCount.ps1 | 20 +++++++++---------- .github/GitHooks/Test-SensitiveData.ps1 | 4 ++-- .github/Install-GitHooks.ps1 | 7 ++++++- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 index cc7fafe537..5adf0bd458 100644 --- a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 +++ b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 @@ -3,10 +3,10 @@ <# .SYNOPSIS - Checks that test files don't exceed the max pipeline run count. + Blocks commits when test files exceed the max pipeline run count. .DESCRIPTION Scans staged .Tests.ps1 files for SetDefaultRunOfHealthChecker calls. - Warns if any file exceeds 5 runs (the benchmarked optimal maximum). + Blocks the commit if any file exceeds 5 runs (the benchmarked optimal maximum). #> [CmdletBinding()] param( @@ -20,15 +20,15 @@ $failed = $false foreach ($file in $Files) { if (-not (Test-Path $file)) { continue } - # Count actual calls only, excluding comments - $runCount = 0 - foreach ($line in (Get-Content $file)) { - if ($line -match '^\s*#') { continue } - if ($line -match 'SetDefaultRunOfHealthChecker') { $runCount++ } - } + # Count actual calls only, excluding comments and strings + $content = Get-Content $file -Raw + $tokens = [System.Management.Automation.PSParser]::Tokenize($content, [ref]$null) + $runCount = @($tokens | Where-Object { + $_.Type -eq 'Command' -and $_.Content -eq 'SetDefaultRunOfHealthChecker' + }).Count if ($runCount -gt $maxRunsPerFile) { - Write-Host " WARN: $file has $runCount pipeline runs (max recommended: $maxRunsPerFile)" -ForegroundColor Yellow + Write-Host " BLOCKED: $file has $runCount pipeline runs (max: $maxRunsPerFile)" -ForegroundColor Red Write-Host " Consider splitting this file. Balance runs evenly (e.g., 4+2 not 5+1)." -ForegroundColor Yellow $failed = $true } elseif ($runCount -gt 0) { @@ -38,7 +38,7 @@ foreach ($file in $Files) { # Use 'git commit --no-verify' to bypass if needed if ($failed) { - Write-Host "`nTest run count check found issues. See warnings above." -ForegroundColor Yellow + Write-Host "`nTest run count check found issues. See above." -ForegroundColor Red return 1 } diff --git a/.github/GitHooks/Test-SensitiveData.ps1 b/.github/GitHooks/Test-SensitiveData.ps1 index 3eb971af04..74a46e8f68 100644 --- a/.github/GitHooks/Test-SensitiveData.ps1 +++ b/.github/GitHooks/Test-SensitiveData.ps1 @@ -87,9 +87,9 @@ foreach ($file in $Files) { $fqdnMatches = [regex]::Matches($line, '(?i)\b([a-zA-Z0-9][a-zA-Z0-9-]*(?:\.[a-zA-Z0-9-]+)*\.' + $tldPattern + ')\b') foreach ($fqdnMatch in $fqdnMatches) { $fqdn = $fqdnMatch.Groups[1].Value - # Skip Windows file paths (e.g., C:\Program Files\...\component.xml) + # Skip Windows file paths and email domain portions (already checked by email scanner) $matchIndex = $fqdnMatch.Index - if ($matchIndex -ge 1 -and $line.Substring($matchIndex - 1, 1) -eq '\') { continue } + if ($matchIndex -ge 1 -and $line.Substring($matchIndex - 1, 1) -in @('\', '@')) { continue } if ($fqdn -match '(?i)^System\.') { continue } if ($fqdn -notmatch "(?i)(?:^|\.)($allowedDomainsPattern)$") { $parts = $fqdn.ToLower() -split '\.' diff --git a/.github/Install-GitHooks.ps1 b/.github/Install-GitHooks.ps1 index ba0e48f871..33aa071617 100644 --- a/.github/Install-GitHooks.ps1 +++ b/.github/Install-GitHooks.ps1 @@ -37,16 +37,21 @@ if (-not (Test-Path $hooksDir)) { # Set git to use .github/GitHooks/ directly instead of .git/hooks/ git config core.hooksPath ".github/GitHooks" +$gitConfigResult = $LASTEXITCODE # Ensure bash wrapper is executable on non-Windows platforms if ($PSVersionTable.Platform -eq 'Unix' -or $PSVersionTable.OS -match 'Linux|Darwin') { $preCommitPath = Join-Path -Path $hooksDir -ChildPath "pre-commit" if (Test-Path $preCommitPath) { chmod +x $preCommitPath + if ($LASTEXITCODE -ne 0) { + Write-Host "Error: Failed to set executable permission on pre-commit hook." -ForegroundColor Red + exit 1 + } } } -if ($LASTEXITCODE -eq 0) { +if ($gitConfigResult -eq 0) { Write-Host "Git hooks configured successfully." -ForegroundColor Green Write-Host " Hooks path: .github/GitHooks/" -ForegroundColor Cyan Write-Host " Hook updates are picked up automatically on git pull." -ForegroundColor Cyan From dffc67ec6a77ca1135f6a2b884224d93145629eb Mon Sep 17 00:00:00 2001 From: David Paulson Date: Thu, 7 May 2026 13:58:01 -0500 Subject: [PATCH 07/12] Fix review round 3: git CWD, rev-parse validation, Import-Module, docs - Use git -C to target correct repo regardless of CWD in installer - Validate git rev-parse exit code, fail if repo root unavailable - Change Import-Module to -ErrorAction Stop (visible errors vs silent) - Update CONTRIBUTING.md: warns -> blocks for run count check Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/GitHooks/Test-ScriptAnalyzer.ps1 | 8 +++++++- .github/Install-GitHooks.ps1 | 2 +- CONTRIBUTING.md | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/GitHooks/Test-ScriptAnalyzer.ps1 b/.github/GitHooks/Test-ScriptAnalyzer.ps1 index 06769c7f04..32ee4c85ee 100644 --- a/.github/GitHooks/Test-ScriptAnalyzer.ps1 +++ b/.github/GitHooks/Test-ScriptAnalyzer.ps1 @@ -16,6 +16,12 @@ param( # cspell:ignore toplevel $repoRoot = git rev-parse --show-toplevel + +if ($LASTEXITCODE -ne 0 -or [string]::IsNullOrEmpty($repoRoot)) { + Write-Host " Error: Unable to determine repository root." -ForegroundColor Red + return 1 +} + $settingsPath = Join-Path -Path $repoRoot -ChildPath "PSScriptAnalyzerSettings.psd1" $customRulesPath = Join-Path -Path $repoRoot -ChildPath ".build" | Join-Path -ChildPath "CodeFormatterChecks" | Join-Path -ChildPath "CustomRules.psm1" @@ -29,7 +35,7 @@ if ($null -eq $module) { return 0 } -Import-Module PSScriptAnalyzer -MinimumVersion "1.24" -ErrorAction SilentlyContinue +Import-Module PSScriptAnalyzer -MinimumVersion "1.24" -ErrorAction Stop $hasViolations = $false diff --git a/.github/Install-GitHooks.ps1 b/.github/Install-GitHooks.ps1 index 33aa071617..6447f10c8a 100644 --- a/.github/Install-GitHooks.ps1 +++ b/.github/Install-GitHooks.ps1 @@ -36,7 +36,7 @@ if (-not (Test-Path $hooksDir)) { } # Set git to use .github/GitHooks/ directly instead of .git/hooks/ -git config core.hooksPath ".github/GitHooks" +git -C $repoRoot.FullName config core.hooksPath ".github/GitHooks" $gitConfigResult = $LASTEXITCODE # Ensure bash wrapper is executable on non-Windows platforms diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 65ac5424f7..91008e75c8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,7 +18,7 @@ Pre-commit hooks are available to catch common issues before committing: - **Sensitive data scanning** on test data files (blocks unrecognized email domains, public IPs) -- **Pester test run count** checking (warns if a test file exceeds 5 pipeline runs) +- **Pester test run count** checking (blocks if a test file exceeds 5 pipeline runs) - **PSScriptAnalyzer** validation on staged PowerShell files To enable, run once after cloning: From 12f380ea9a5bc441923526da9404b2cd5c32d249 Mon Sep 17 00:00:00 2001 From: David Paulson Date: Thu, 7 May 2026 14:40:05 -0500 Subject: [PATCH 08/12] Add .psd1 to ScriptAnalyzer filter and fix ParseError color - Include .psd1 manifest files in PSScriptAnalyzer hook filter - Color ParseError severity as Red instead of Yellow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/GitHooks/Test-ScriptAnalyzer.ps1 | 2 +- .github/GitHooks/pre-commit.ps1 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/GitHooks/Test-ScriptAnalyzer.ps1 b/.github/GitHooks/Test-ScriptAnalyzer.ps1 index 32ee4c85ee..895e1d93fe 100644 --- a/.github/GitHooks/Test-ScriptAnalyzer.ps1 +++ b/.github/GitHooks/Test-ScriptAnalyzer.ps1 @@ -62,7 +62,7 @@ foreach ($file in $Files) { if ($null -ne $results -and $results.Count -gt 0) { $hasViolations = $true foreach ($r in $results) { - Write-Host " $($r.Severity): $file`:$($r.Line) - [$($r.RuleName)] $($r.Message)" -ForegroundColor $(if ($r.Severity -eq 'Error') { 'Red' } else { 'Yellow' }) + Write-Host " $($r.Severity): $file`:$($r.Line) - [$($r.RuleName)] $($r.Message)" -ForegroundColor $(if ($r.Severity -in @('Error', 'ParseError')) { 'Red' } else { 'Yellow' }) } } } diff --git a/.github/GitHooks/pre-commit.ps1 b/.github/GitHooks/pre-commit.ps1 index d109ecd3c7..53ab4328a3 100644 --- a/.github/GitHooks/pre-commit.ps1 +++ b/.github/GitHooks/pre-commit.ps1 @@ -42,7 +42,7 @@ if ($testFiles.Count -gt 0) { } # Run PSScriptAnalyzer on staged PowerShell files -$psFiles = @($stagedFiles | Where-Object { $_ -like "*.ps1" -or $_ -like "*.psm1" }) +$psFiles = @($stagedFiles | Where-Object { $_ -like "*.ps1" -or $_ -like "*.psm1" -or $_ -like "*.psd1" }) if ($psFiles.Count -gt 0) { Write-Host "Running PSScriptAnalyzer on $($psFiles.Count) PowerShell file(s)..." -ForegroundColor Cyan From 04aff6431936294d2c3aac5d665cdfde4ca0774d Mon Sep 17 00:00:00 2001 From: David Paulson Date: Thu, 7 May 2026 15:11:02 -0500 Subject: [PATCH 09/12] Use named -Path parameter on Get-Content calls Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 | 2 +- .github/GitHooks/Test-SensitiveData.ps1 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 index 5adf0bd458..7847e3d76f 100644 --- a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 +++ b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 @@ -21,7 +21,7 @@ foreach ($file in $Files) { if (-not (Test-Path $file)) { continue } # Count actual calls only, excluding comments and strings - $content = Get-Content $file -Raw + $content = Get-Content -Path $file -Raw $tokens = [System.Management.Automation.PSParser]::Tokenize($content, [ref]$null) $runCount = @($tokens | Where-Object { $_.Type -eq 'Command' -and $_.Content -eq 'SetDefaultRunOfHealthChecker' diff --git a/.github/GitHooks/Test-SensitiveData.ps1 b/.github/GitHooks/Test-SensitiveData.ps1 index 74a46e8f68..bc2895d23b 100644 --- a/.github/GitHooks/Test-SensitiveData.ps1 +++ b/.github/GitHooks/Test-SensitiveData.ps1 @@ -55,7 +55,7 @@ $allowedEmails = @( foreach ($file in $Files) { if (-not (Test-Path $file)) { continue } - $lines = @(Get-Content $file -ErrorAction SilentlyContinue) + $lines = @(Get-Content -Path $file -ErrorAction SilentlyContinue) if ($lines.Count -eq 0) { continue } $domainsInFile = @{} From 1f91ddf3c790cf2537947ffceba951cef8392630 Mon Sep 17 00:00:00 2001 From: David Paulson Date: Thu, 7 May 2026 16:54:12 -0500 Subject: [PATCH 10/12] Fix review round 4: cross-platform paths, named params, email subdomain, stdout guard - Use Join-Path for cross-platform path in Install-GitHooks.ps1 - Use named parameters on Split-Path/Join-Path calls in pre-commit.ps1 - Align email subdomain validation with FQDN check in Test-SensitiveData.ps1 - Guard stdout capture with null check and Select-Object -Last 1 - Reformat piped Join-Path for readability in Test-ScriptAnalyzer.ps1 - Document PSScriptAnalyzer prerequisite in CONTRIBUTING.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/GitHooks/Test-ScriptAnalyzer.ps1 | 4 +++- .github/GitHooks/Test-SensitiveData.ps1 | 2 +- .github/GitHooks/pre-commit.ps1 | 14 +++++++------- .github/Install-GitHooks.ps1 | 4 ++-- CONTRIBUTING.md | 2 ++ 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/.github/GitHooks/Test-ScriptAnalyzer.ps1 b/.github/GitHooks/Test-ScriptAnalyzer.ps1 index 895e1d93fe..7f58cc2083 100644 --- a/.github/GitHooks/Test-ScriptAnalyzer.ps1 +++ b/.github/GitHooks/Test-ScriptAnalyzer.ps1 @@ -23,7 +23,9 @@ if ($LASTEXITCODE -ne 0 -or [string]::IsNullOrEmpty($repoRoot)) { } $settingsPath = Join-Path -Path $repoRoot -ChildPath "PSScriptAnalyzerSettings.psd1" -$customRulesPath = Join-Path -Path $repoRoot -ChildPath ".build" | Join-Path -ChildPath "CodeFormatterChecks" | Join-Path -ChildPath "CustomRules.psm1" +$customRulesPath = Join-Path -Path $repoRoot -ChildPath ".build" | + Join-Path -ChildPath "CodeFormatterChecks" | + Join-Path -ChildPath "CustomRules.psm1" # Check if PSScriptAnalyzer >= 1.24 is available (matches .build/CodeFormatter.ps1) $module = Get-Module -ListAvailable -Name PSScriptAnalyzer | diff --git a/.github/GitHooks/Test-SensitiveData.ps1 b/.github/GitHooks/Test-SensitiveData.ps1 index bc2895d23b..b85eb19736 100644 --- a/.github/GitHooks/Test-SensitiveData.ps1 +++ b/.github/GitHooks/Test-SensitiveData.ps1 @@ -70,7 +70,7 @@ foreach ($file in $Files) { if ($match.Value -in $allowedEmails) { continue } $emailTld = ($match.Value -split '\.')[-1].ToLower() if ($emailTld -in $reservedTlds) { continue } - if ($match.Value -notmatch "@($allowedDomainsPattern)$") { + if ($match.Value -notmatch "@(?:.*\.)?($allowedDomainsPattern)$") { Write-Host " BLOCKED: $file`:$lineNumber - Unrecognized email domain: $($match.Value)" -ForegroundColor Red $failed = $true } diff --git a/.github/GitHooks/pre-commit.ps1 b/.github/GitHooks/pre-commit.ps1 index 53ab4328a3..a11273c13c 100644 --- a/.github/GitHooks/pre-commit.ps1 +++ b/.github/GitHooks/pre-commit.ps1 @@ -6,7 +6,7 @@ # Install via: .github/Install-GitHooks.ps1 $exitCode = 0 -$hookRoot = Split-Path -Parent $MyInvocation.MyCommand.Path +$hookRoot = Split-Path -Path $MyInvocation.MyCommand.Path -Parent # Get staged files (include renames with R) # cspell:ignore ACMR @@ -24,8 +24,8 @@ $testDataFiles = @($stagedFiles | Where-Object { if ($testDataFiles.Count -gt 0) { Write-Host "Running sensitive data scan on $($testDataFiles.Count) test data file(s)..." -ForegroundColor Cyan - $result = & (Join-Path $hookRoot "Test-SensitiveData.ps1") -Files $testDataFiles - if ($result -ne 0) { + $result = & (Join-Path -Path $hookRoot -ChildPath "Test-SensitiveData.ps1") -Files $testDataFiles + if ($null -ne $result -and ($result | Select-Object -Last 1) -ne 0) { $exitCode = 1 } } @@ -35,8 +35,8 @@ $testFiles = @($stagedFiles | Where-Object { $_ -like "*.Tests.ps1" }) if ($testFiles.Count -gt 0) { Write-Host "Checking test file pipeline run counts..." -ForegroundColor Cyan - $result = & (Join-Path $hookRoot "Test-HealthCheckerScenarioRunCount.ps1") -Files $testFiles - if ($result -ne 0) { + $result = & (Join-Path -Path $hookRoot -ChildPath "Test-HealthCheckerScenarioRunCount.ps1") -Files $testFiles + if ($null -ne $result -and ($result | Select-Object -Last 1) -ne 0) { $exitCode = 1 } } @@ -46,8 +46,8 @@ $psFiles = @($stagedFiles | Where-Object { $_ -like "*.ps1" -or $_ -like "*.psm1 if ($psFiles.Count -gt 0) { Write-Host "Running PSScriptAnalyzer on $($psFiles.Count) PowerShell file(s)..." -ForegroundColor Cyan - $result = & (Join-Path $hookRoot "Test-ScriptAnalyzer.ps1") -Files $psFiles - if ($result -ne 0) { + $result = & (Join-Path -Path $hookRoot -ChildPath "Test-ScriptAnalyzer.ps1") -Files $psFiles + if ($null -ne $result -and ($result | Select-Object -Last 1) -ne 0) { $exitCode = 1 } } diff --git a/.github/Install-GitHooks.ps1 b/.github/Install-GitHooks.ps1 index 6447f10c8a..037b0f679a 100644 --- a/.github/Install-GitHooks.ps1 +++ b/.github/Install-GitHooks.ps1 @@ -22,11 +22,11 @@ [CmdletBinding()] param() -$repoRoot = Get-Item "$PSScriptRoot\.." +$repoRoot = Get-Item (Join-Path -Path $PSScriptRoot -ChildPath "..") $hooksDir = Join-Path -Path $PSScriptRoot -ChildPath "GitHooks" if (-not (Test-Path (Join-Path -Path $repoRoot -ChildPath ".git"))) { - Write-Host "Error: .git directory not found. Are you in a git repository?" -ForegroundColor Red + Write-Host "Error: .git not found. Are you in a git repository?" -ForegroundColor Red exit 1 } diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 91008e75c8..4b9225d19a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,6 +29,8 @@ To enable, run once after cloning: This sets `core.hooksPath` to `.github/GitHooks/`. Hook updates are picked up automatically on `git pull`. Use `git commit --no-verify` to bypass hooks when needed. +PSScriptAnalyzer is required for full hook coverage. Install with `Install-Module PSScriptAnalyzer`. Without it, the analyzer check is skipped. + It is recommended to use Visual Studio Code when developing scripts for this project. Opening VSCode at the root of this repo will ensure that VSCode uses the settings in the repro to enforce most of the formatting rules. From 5a82b5bc792b0cd22b522d8d17813d6f1df188ab Mon Sep 17 00:00:00 2001 From: David Paulson Date: Thu, 7 May 2026 17:40:07 -0500 Subject: [PATCH 11/12] Hoist patterns outside loop, named Sort-Object, remove SilentlyContinue - Move tldPattern, credential regexes, and safeValues outside per-line loop - Use named -Property parameter on Sort-Object - Remove ErrorAction SilentlyContinue from Invoke-ScriptAnalyzer Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/GitHooks/Test-ScriptAnalyzer.ps1 | 5 ++--- .github/GitHooks/Test-SensitiveData.ps1 | 24 +++++++++++++++--------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/.github/GitHooks/Test-ScriptAnalyzer.ps1 b/.github/GitHooks/Test-ScriptAnalyzer.ps1 index 7f58cc2083..517c210a00 100644 --- a/.github/GitHooks/Test-ScriptAnalyzer.ps1 +++ b/.github/GitHooks/Test-ScriptAnalyzer.ps1 @@ -45,9 +45,8 @@ foreach ($file in $Files) { if (-not (Test-Path $file)) { continue } $params = @{ - Path = $file - Severity = @('ParseError', 'Error', 'Warning') - ErrorAction = 'SilentlyContinue' + Path = $file + Severity = @('ParseError', 'Error', 'Warning') } # Use repo settings and custom rules if available diff --git a/.github/GitHooks/Test-SensitiveData.ps1 b/.github/GitHooks/Test-SensitiveData.ps1 index b85eb19736..5d5e60a463 100644 --- a/.github/GitHooks/Test-SensitiveData.ps1 +++ b/.github/GitHooks/Test-SensitiveData.ps1 @@ -60,6 +60,16 @@ foreach ($file in $Files) { $domainsInFile = @{} + # Patterns defined once outside the per-line loop for performance + $tldPattern = '(com|net|org|edu|gov|io|info|biz|co|us|uk|de|au|ca|jp|fr|in|eu|cloud|app|dev|lab)' + $fqdnRegex = '(?i)\b([a-zA-Z0-9][a-zA-Z0-9-]*(?:\.[a-zA-Z0-9-]+)*\.' + $tldPattern + ')\b' + $credentialKeywords = 'password|secret|apikey|token|credential' + $safeValues = @('true', 'false', 'Enabled', 'Disabled', 'Changed', 'None', 'NotConfigured') + $assignRegex = '(?i)\b(?:' + $credentialKeywords + ')\s*[:=]\s*[''"]([^''"]+)[''"]' + $xmlAttrRegex = '(?i)(?:key|name)\s*=\s*[''"](?:' + $credentialKeywords + ')[''"].*?value\s*=\s*[''"]([^''"]+)[''"]' + # cspell:ignore clixml + $clixmlRegex = '(?i)N\s*=\s*[''"](?:' + $credentialKeywords + ')[''"]>([^<]+)<' + $lineNumber = 0 foreach ($line in $lines) { $lineNumber++ @@ -83,8 +93,7 @@ foreach ($file in $Files) { # shipped with Exchange ("located in \Windows\Microsft.Net\Frameworks\v2.x\Config") if ($line -notmatch 'xmlns|assembly|codeBase|PublicKeyToken|\.dll|\.exe|\\Microsoft\.NET\\|\\Microsft\.Net\\|ASP\.NET|WMI\.NET|\.msilog') { # Match FQDNs ending in known public TLDs - $tldPattern = '(com|net|org|edu|gov|io|info|biz|co|us|uk|de|au|ca|jp|fr|in|eu|cloud|app|dev|lab)' - $fqdnMatches = [regex]::Matches($line, '(?i)\b([a-zA-Z0-9][a-zA-Z0-9-]*(?:\.[a-zA-Z0-9-]+)*\.' + $tldPattern + ')\b') + $fqdnMatches = [regex]::Matches($line, $fqdnRegex) foreach ($fqdnMatch in $fqdnMatches) { $fqdn = $fqdnMatch.Groups[1].Value # Skip Windows file paths and email domain portions (already checked by email scanner) @@ -124,12 +133,10 @@ foreach ($file in $Files) { } # Check for credential patterns in multiple formats - $credentialKeywords = 'password|secret|apikey|token|credential' - $safeValues = @('true', 'false', 'Enabled', 'Disabled', 'Changed', 'None', 'NotConfigured') $credentialFound = $false # Assignment form: password = "value", secret: 'value' - $assignMatches = [regex]::Matches($line, '(?i)\b(?:' + $credentialKeywords + ')\s*[:=]\s*[''"]([^''"]+)[''"]') + $assignMatches = [regex]::Matches($line, $assignRegex) foreach ($m in $assignMatches) { if ($m.Groups[1].Value -notin $safeValues) { $credentialFound = $true @@ -139,7 +146,7 @@ foreach ($file in $Files) { # XML attribute form: key="password" value="secret" if (-not $credentialFound) { - $xmlAttrMatches = [regex]::Matches($line, '(?i)(?:key|name)\s*=\s*[''"](?:' + $credentialKeywords + ')[''"].*?value\s*=\s*[''"]([^''"]+)[''"]') + $xmlAttrMatches = [regex]::Matches($line, $xmlAttrRegex) foreach ($m in $xmlAttrMatches) { if ($m.Groups[1].Value -notin $safeValues) { $credentialFound = $true @@ -150,8 +157,7 @@ foreach ($file in $Files) { # CLIXML element form: N="Password">value([^<]+)<') + $clixmlMatches = [regex]::Matches($line, $clixmlRegex) foreach ($m in $clixmlMatches) { if ($m.Groups[1].Value.Trim() -notin $safeValues) { $credentialFound = $true @@ -167,7 +173,7 @@ foreach ($file in $Files) { } # Report unrecognized domains found in this file (one line per domain) - foreach ($entry in $domainsInFile.GetEnumerator() | Sort-Object Value -Descending) { + foreach ($entry in $domainsInFile.GetEnumerator() | Sort-Object -Property Value -Descending) { Write-Host " BLOCKED: $file - Unrecognized domain: $($entry.Key) ($($entry.Value) occurrences)" -ForegroundColor Red $failed = $true } From 3c9b7bc785a6a7fe8854088e16b17e76452209ab Mon Sep 17 00:00:00 2001 From: David Paulson Date: Fri, 8 May 2026 09:04:35 -0500 Subject: [PATCH 12/12] Fix PR review findings: fail-closed error handling Addressed: - Test-SensitiveData.ps1: Replace Get-Content -ErrorAction SilentlyContinue with try/catch and -ErrorAction Stop so unreadable/locked files block the commit instead of silently passing (fail closed instead of fail open) - Test-HealthCheckerScenarioRunCount.ps1: Capture PSParser::Tokenize parse errors instead of discarding them via [ref]\, emit warning when parse errors exist so contributors know the run count may be inaccurate - pre-commit.ps1: Add \0 check after git diff --cached to fail early if git is unavailable or not in a repository, instead of silently passing Not addressed (left for author): - Test-ScriptAnalyzer.ps1 settings fallback: Author confirmed 3x this is intentional since PSScriptAnalyzerSettings.psd1 and CustomRules.psm1 are committed files Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 | 8 +++++++- .github/GitHooks/Test-SensitiveData.ps1 | 8 +++++++- .github/GitHooks/pre-commit.ps1 | 5 +++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 index 7847e3d76f..791e9d0706 100644 --- a/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 +++ b/.github/GitHooks/Test-HealthCheckerScenarioRunCount.ps1 @@ -22,7 +22,13 @@ foreach ($file in $Files) { # Count actual calls only, excluding comments and strings $content = Get-Content -Path $file -Raw - $tokens = [System.Management.Automation.PSParser]::Tokenize($content, [ref]$null) + $parseErrors = $null + $tokens = [System.Management.Automation.PSParser]::Tokenize($content, [ref]$parseErrors) + + if ($null -ne $parseErrors -and $parseErrors.Count -gt 0) { + Write-Host " WARN: $file has $($parseErrors.Count) parse error(s) - run count may be inaccurate" -ForegroundColor Yellow + } + $runCount = @($tokens | Where-Object { $_.Type -eq 'Command' -and $_.Content -eq 'SetDefaultRunOfHealthChecker' }).Count diff --git a/.github/GitHooks/Test-SensitiveData.ps1 b/.github/GitHooks/Test-SensitiveData.ps1 index 5d5e60a463..dcd623d361 100644 --- a/.github/GitHooks/Test-SensitiveData.ps1 +++ b/.github/GitHooks/Test-SensitiveData.ps1 @@ -55,7 +55,13 @@ $allowedEmails = @( foreach ($file in $Files) { if (-not (Test-Path $file)) { continue } - $lines = @(Get-Content -Path $file -ErrorAction SilentlyContinue) + try { + $lines = @(Get-Content -Path $file -ErrorAction Stop) + } catch { + Write-Host " BLOCKED: $file - Unable to read file: $($_.Exception.Message)" -ForegroundColor Red + $failed = $true + continue + } if ($lines.Count -eq 0) { continue } $domainsInFile = @{} diff --git a/.github/GitHooks/pre-commit.ps1 b/.github/GitHooks/pre-commit.ps1 index a11273c13c..123fb10b86 100644 --- a/.github/GitHooks/pre-commit.ps1 +++ b/.github/GitHooks/pre-commit.ps1 @@ -12,6 +12,11 @@ $hookRoot = Split-Path -Path $MyInvocation.MyCommand.Path -Parent # cspell:ignore ACMR $stagedFiles = @(git diff --cached --name-only --diff-filter=ACMR) +if ($LASTEXITCODE -ne 0) { + Write-Host "Error: git diff failed. Are you in a git repository?" -ForegroundColor Red + exit 1 +} + if ($stagedFiles.Count -eq 0) { exit 0 }