-
Notifications
You must be signed in to change notification settings - Fork 392
Add git pre-commit hooks for code quality validation #2534
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
Open
dpaulson45
wants to merge
1
commit into
main
Choose a base branch
from
dpaul-GitHooks
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. | ||
|
|
||
| <# | ||
| .SYNOPSIS | ||
| Blocks commits when test files exceed the max pipeline run count. | ||
| .DESCRIPTION | ||
| Scans staged .Tests.ps1 files for SetDefaultRunOfHealthChecker calls. | ||
| Blocks the commit 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 and strings | ||
| try { | ||
| $content = Get-Content -Path $file -Raw -ErrorAction Stop | ||
| } catch { | ||
| Write-Host " BLOCKED: $file - Unable to read file: $($_.Exception.Message)" -ForegroundColor Red | ||
| $failed = $true | ||
| continue | ||
| } | ||
| $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' | ||
|
dpaulson45 marked this conversation as resolved.
|
||
| }).Count | ||
|
|
||
| if ($runCount -gt $maxRunsPerFile) { | ||
| Write-Host " BLOCKED: $file has $runCount pipeline runs (max: $maxRunsPerFile)" -ForegroundColor Red | ||
|
dpaulson45 marked this conversation as resolved.
|
||
| 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 | ||
| } | ||
| } | ||
|
|
||
| # Use 'git commit --no-verify' to bypass if needed | ||
| if ($failed) { | ||
| Write-Host "`nTest run count check found issues. See above." -ForegroundColor Red | ||
| return 1 | ||
|
dpaulson45 marked this conversation as resolved.
|
||
| } | ||
|
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
|
||
|
|
||
| return 0 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. | ||
|
|
||
| <# | ||
| .SYNOPSIS | ||
| Runs PSScriptAnalyzer and CodeFormatter checks on staged PowerShell files. | ||
| .DESCRIPTION | ||
| Blocks commits with PSScriptAnalyzer violations and CodeFormatter issues using | ||
| the repository's PSScriptAnalyzerSettings.psd1, CustomRules.psm1, and | ||
| CodeFormatterChecks to match the CI pipeline. | ||
| #> | ||
| [CmdletBinding()] | ||
| param( | ||
| [Parameter(Mandatory)] | ||
| [string[]]$Files | ||
| ) | ||
|
|
||
| # 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 (Join-Path -Path (Join-Path -Path $repoRoot -ChildPath ".build") -ChildPath "CodeFormatterChecks") -ChildPath "CustomRules.psm1" | ||
| $codeFormatterChecksPath = Join-Path -Path (Join-Path -Path $repoRoot -ChildPath ".build") -ChildPath "CodeFormatterChecks" | ||
|
|
||
| # Dot-source CodeFormatter check functions | ||
| . (Join-Path -Path $codeFormatterChecksPath -ChildPath "CheckContainsCurlyQuotes.ps1") | ||
| . (Join-Path -Path $codeFormatterChecksPath -ChildPath "CheckFileHasNewlineAtEndOfFile.ps1") | ||
| . (Join-Path -Path $codeFormatterChecksPath -ChildPath "CheckMultipleEmptyLines.ps1") | ||
| . (Join-Path -Path $codeFormatterChecksPath -ChildPath "CheckScriptFileHasBOM.ps1") | ||
| . (Join-Path -Path $codeFormatterChecksPath -ChildPath "CheckScriptFileHasComplianceHeader.ps1") | ||
| . (Join-Path -Path $codeFormatterChecksPath -ChildPath "CheckScriptFormat.ps1") | ||
| . (Join-Path -Path $codeFormatterChecksPath -ChildPath "CheckTokenTypeCasing.ps1") | ||
|
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
|
||
|
|
||
| # 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 >= 1.24 not installed." -ForegroundColor Yellow | ||
| return 0 | ||
| } | ||
|
|
||
| Import-Module PSScriptAnalyzer -MinimumVersion "1.24" -ErrorAction Stop | ||
|
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
|
||
|
|
||
| # Check if EncodingAnalyzer is available for BOM checks | ||
| $hasEncodingAnalyzer = $null -ne (Get-Module -ListAvailable -Name EncodingAnalyzer | Select-Object -First 1) | ||
| if ($hasEncodingAnalyzer) { | ||
| Import-Module EncodingAnalyzer -ErrorAction SilentlyContinue | ||
| } else { | ||
| Write-Host " NOTE: EncodingAnalyzer not installed - BOM checks will be skipped." -ForegroundColor Yellow | ||
| } | ||
|
|
||
| $hasViolations = $false | ||
|
|
||
| foreach ($file in $Files) { | ||
| if (-not (Test-Path $file)) { continue } | ||
|
|
||
| $fileInfo = Get-Item -Path $file | ||
|
dpaulson45 marked this conversation as resolved.
|
||
|
|
||
| # Run CodeFormatter checks (report only, no auto-fix) | ||
| if (CheckFileHasNewlineAtEndOfFile $fileInfo $false) { $hasViolations = $true } | ||
| if (CheckScriptFileHasComplianceHeader $fileInfo $false) { $hasViolations = $true } | ||
| if (CheckTokenTypeCasing $fileInfo $false "Keyword") { $hasViolations = $true } | ||
| if (CheckTokenTypeCasing $fileInfo $false "Operator") { $hasViolations = $true } | ||
| if (CheckMultipleEmptyLines $fileInfo $false) { $hasViolations = $true } | ||
| if (CheckContainsCurlyQuotes $fileInfo $false) { $hasViolations = $true } | ||
|
|
||
| if ($hasEncodingAnalyzer) { | ||
| if (CheckScriptFileHasBOM $fileInfo $false) { $hasViolations = $true } | ||
| } | ||
|
|
||
| $formatResults = @(CheckScriptFormat $fileInfo $false) | ||
| if ($formatResults.Length -gt 0 -and $formatResults[0] -eq $true) { | ||
| $hasViolations = $true | ||
| } | ||
|
|
||
| # Run PSScriptAnalyzer with repo settings and custom rules | ||
| $params = @{ | ||
| Path = $file | ||
| Severity = @('ParseError', 'Error', 'Warning') | ||
| Settings = $settingsPath | ||
| CustomRulePath = $customRulesPath | ||
| IncludeDefaultRules = $true | ||
| } | ||
|
|
||
| $results = Invoke-ScriptAnalyzer @params | ||
|
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
|
||
|
|
||
| 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 -in @('Error', 'ParseError')) { 'Red' } else { 'Yellow' }) | ||
| } | ||
| } | ||
|
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
|
||
| } | ||
|
|
||
| if ($hasViolations) { | ||
| Write-Host "`nCode quality checks found violations. Fix before committing." -ForegroundColor Red | ||
| return 1 | ||
|
dpaulson45 marked this conversation as resolved.
|
||
| } else { | ||
| Write-Host " Code quality checks: no issues found." -ForegroundColor Green | ||
| return 0 | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,193 @@ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. | ||
|
|
||
| <# | ||
| .SYNOPSIS | ||
| Scans test data files for sensitive data patterns. | ||
| .DESCRIPTION | ||
| Checks staged test data files for email addresses and domain names outside | ||
| allowed test domains, public IP addresses, and credential-like patterns. | ||
| #> | ||
| [CmdletBinding()] | ||
| param( | ||
| [Parameter(Mandatory)] | ||
| [string[]]$Files | ||
| ) | ||
|
|
||
| $failed = $false | ||
|
|
||
| # 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 microsoftonline cloudapp | ||
| $allowedDomains = @( | ||
| 'fabrikam\.com', | ||
| 'example\.com', | ||
| 'contoso\.com', | ||
| 'contoso\.lab', | ||
| '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 '|' | ||
|
|
||
| # 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]\.)' | ||
|
|
||
| # Specific email addresses that are allowed regardless of domain | ||
| $allowedEmails = @( | ||
| 'ExToolsFeedback@microsoft.com' | ||
| ) | ||
|
|
||
| foreach ($file in $Files) { | ||
| if (-not (Test-Path $file)) { continue } | ||
|
|
||
| try { | ||
| $lines = @(Get-Content -Path $file -ErrorAction Stop) | ||
| } catch { | ||
| Write-Host " BLOCKED: $file - Unable to read file: $($_.Exception.Message)" -ForegroundColor Red | ||
| $failed = $true | ||
| continue | ||
| } | ||
|
dpaulson45 marked this conversation as resolved.
|
||
| if ($lines.Count -eq 0) { continue } | ||
|
|
||
| $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)' | ||
|
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
|
||
| $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++ | ||
|
|
||
| # 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 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 | ||
| $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) | ||
| $matchIndex = $fqdnMatch.Index | ||
| if ($matchIndex -ge 1 -and $line.Substring($matchIndex - 1, 1) -in @('\', '@')) { continue } | ||
| if ($fqdn -match '(?i)^System\.') { continue } | ||
| if ($fqdn -notmatch "(?i)(?:^|\.)($allowedDomainsPattern)$") { | ||
| $fqdnLower = $fqdn.ToLower() | ||
| if (-not $domainsInFile.ContainsKey($fqdnLower)) { | ||
| $domainsInFile[$fqdnLower] = 0 | ||
| } | ||
| $domainsInFile[$fqdnLower]++ | ||
| } | ||
| } | ||
| } | ||
|
|
||
| # 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') | ||
|
dpaulson45 marked this conversation as resolved.
|
||
| 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 | ||
| } | ||
|
dpaulson45 marked this conversation as resolved.
|
||
| } | ||
| } | ||
|
|
||
| # Check for credential patterns in multiple formats | ||
| $credentialFound = $false | ||
|
|
||
| # Assignment form: password = "value", secret: 'value' | ||
| $assignMatches = [regex]::Matches($line, $assignRegex) | ||
| 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, $xmlAttrRegex) | ||
| foreach ($m in $xmlAttrMatches) { | ||
| if ($m.Groups[1].Value -notin $safeValues) { | ||
| $credentialFound = $true | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| # CLIXML element form: N="Password">value</ | ||
| if (-not $credentialFound) { | ||
| $clixmlMatches = [regex]::Matches($line, $clixmlRegex) | ||
| 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 -Property Value -Descending) { | ||
| Write-Host " BLOCKED: $file - Unrecognized domain: $($entry.Key) ($($entry.Value) occurrences)" -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 | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)" | ||
|
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
|
||
| exec pwsh -NoProfile -NonInteractive -File "$HOOK_DIR/pre-commit.ps1" | ||
|
dpaulson45 marked this conversation as resolved.
dpaulson45 marked this conversation as resolved.
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.