Static analysis: expand PowerShell rules catalog#158
Static analysis: expand PowerShell rules catalog#158PrzemyslawKlys merged 104 commits intomasterfrom
Conversation
IntelligenceX ReviewReviewing PR #158: Static analysis: expand PowerShell rules catalog Merge blockers: items in Todo List ✅ and Critical Issues
Summary 📝Good expansion of the PowerShell rules catalog and packs. The new reviewer tests meaningfully reduce drift (docs URL shape + overrides sanity + pack inclusion), and the sync script is unusually careful about determinism and module trust. Todo List ✅
Critical Issues
|
![intelligencex-review[bot]](https://avatars.githubusercontent.com/in/2747853?s=60&v=4){kind=small}
| "language": "powershell", | ||
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSAvoidAssignmentToAutomaticVariable", | ||
| "title": "Changing automtic variables might have undesired side effects", |
There was a problem hiding this comment.
Typos in the rule title/description (“automtic”, “This automatic variables…”) are user-facing and should be corrected before merge.
| "title": "Changing automtic variables might have undesired side effects", | |
| "title": "Changing automatic variables might have undesired side effects", | |
| "description": "These automatic variables are built into PowerShell and are read-only.", |
There was a problem hiding this comment.
IntelligenceX triage: The thread requests fixing user-facing typos/wording in title/description, but the current diff still contains the generic title/description and does not apply the suggested corrected text.
Assessed commit: 398502a
Diff range: current PR files
| "language": "powershell", | ||
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSAvoidGlobalFunctions", | ||
| "title": "Avoid global functiosn and aliases", |
There was a problem hiding this comment.
Title has a typo (“functiosn”) and doesn’t match the docs intent; user-facing strings should be clean.
| "title": "Avoid global functiosn and aliases", | |
| "title": "Avoid global functions and aliases", |
There was a problem hiding this comment.
IntelligenceX triage: The thread calls out a typo and suggests a different, cleaner title. The current diff still uses the unchanged generic title and does not show the suggested correction.
Assessed commit: 398502a
Diff range: current PR files
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSAvoidMultipleTypeAttributes", | ||
| "title": "Avoid multiple type specifiers on parameters", | ||
| "description": "Prameter should not have more than one type specifier.", |
There was a problem hiding this comment.
Description has a typo (“Prameter”) and grammar issues; fix for clarity/search.
| "description": "Prameter should not have more than one type specifier.", | |
| "description": "Parameters should not have more than one type specifier.", |
There was a problem hiding this comment.
IntelligenceX triage: The thread asks to fix a typo/grammar in the description, but the current diff still has the generic description and does not incorporate the suggested wording.
Assessed commit: 398502a
Diff range: current PR files
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSAlignAssignmentStatement", | ||
| "title": "Align assignment statement", | ||
| "description": "Line up assignment statements such that the assignment operator are aligned.", |
There was a problem hiding this comment.
Grammar issue (“operator are”); this is user-facing text.
| "description": "Line up assignment statements such that the assignment operator are aligned.", | |
| "description": "Line up assignment statements such that the assignment operators are aligned.", |
There was a problem hiding this comment.
IntelligenceX triage: The thread suggests a specific grammar-corrected description; the current diff still contains the generic description and does not address the requested user-facing text change.
Assessed commit: 398502a
Diff range: current PR files
There was a problem hiding this comment.
Pull request overview
Adds first-party PowerShell static analysis metadata to the existing Analysis Catalog/Packs system by importing PSScriptAnalyzer rule definitions, expanding the built-in PowerShell packs, and wiring a new PowerShell security pack into the cross-language security default.
Changes:
- Add a generated PowerShell rules catalog (
Analysis/Catalog/rules/powershell/*) sourced from PSScriptAnalyzer. - Expand the
powershell-defaultpack to include a security + correctness baseline, and addpowershell-security-default. - Add a sync script (
scripts/sync-pssa-catalog.ps1) and document regeneration steps inAnalysis/Packs/README.md.
Reviewed changes
Copilot reviewed 75 out of 75 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/sync-pssa-catalog.ps1 | Script to (re)generate PowerShell rule metadata JSON files from PSScriptAnalyzer. |
| Analysis/Packs/powershell-security-default.json | New security-focused PowerShell pack. |
| Analysis/Packs/powershell-default.json | Expands default PowerShell pack to a broader baseline set. |
| Analysis/Packs/powershell-50.json | PowerShell tier pack scaffold (includes powershell-default). |
| Analysis/Packs/powershell-100.json | PowerShell tier pack scaffold (includes powershell-50). |
| Analysis/Packs/powershell-500.json | PowerShell tier pack scaffold (includes powershell-100). |
| Analysis/Packs/all-security-default.json | Extends cross-language security pack to include PowerShell security defaults. |
| Analysis/Packs/README.md | Documents the new PowerShell catalog source and regeneration command. |
| Analysis/Catalog/rules/powershell/PSAlignAssignmentStatement.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidAssignmentToAutomaticVariable.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidDefaultValueForMandatoryParameter.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidDefaultValueSwitchParameter.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidExclaimOperator.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidGlobalAliases.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidGlobalFunctions.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidGlobalVars.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidInvokingEmptyMembers.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidLongLines.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidMultipleTypeAttributes.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidNullOrEmptyHelpMessageAttribute.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidOverwritingBuiltInCmdlets.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidSemicolonsAsLineTerminators.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidShouldContinueWithoutForce.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidTrailingWhitespace.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingAllowUnencryptedAuthentication.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingBrokenHashAlgorithms.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingCmdletAliases.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingComputerNameHardcoded.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingConvertToSecureStringWithPlainText.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingDeprecatedManifestFields.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingDoubleQuotesForConstantString.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingEmptyCatchBlock.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingInvokeExpression.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingPlainTextForPassword.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingPositionalParameters.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingUsernameAndPasswordParams.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingWMICmdlet.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSAvoidUsingWriteHost.json | Update/align PSScriptAnalyzer rule metadata for Write-Host (PowerShell). |
| Analysis/Catalog/rules/powershell/PSDSCDscExamplesPresent.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSDSCDscTestsPresent.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSDSCReturnCorrectTypesForDSCFunctions.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSDSCStandardDSCFunctionsInResource.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSDSCUseIdenticalMandatoryParametersForDSC.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSDSCUseIdenticalParametersForDSC.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSDSCUseVerboseMessageInDSCResource.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSMisleadingBacktick.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSMissingModuleManifestField.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSPlaceCloseBrace.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSPlaceOpenBrace.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSPossibleIncorrectComparisonWithNull.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSPossibleIncorrectUsageOfAssignmentOperator.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSPossibleIncorrectUsageOfRedirectionOperator.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSProvideCommentHelp.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSReservedCmdletChar.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSReservedParams.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSReviewUnusedParameter.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSShouldProcess.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseApprovedVerbs.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseBOMForUnicodeEncodedFile.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseCmdletCorrectly.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseCompatibleCmdlets.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseCompatibleCommands.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseCompatibleSyntax.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseCompatibleTypes.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseConsistentIndentation.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseConsistentWhitespace.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseCorrectCasing.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseDeclaredVarsMoreThanAssignments.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseLiteralInitializerForHashtable.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseOutputTypeCorrectly.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUsePSCredentialType.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseProcessBlockForPipelineCommand.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseShouldProcessForStateChangingFunctions.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseSingularNouns.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseSupportsShouldProcess.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseToExportFieldsInManifest.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseUTF8EncodingForHelpFile.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
| Analysis/Catalog/rules/powershell/PSUseUsingScopeModifierInNewRunspaces.json | Add PSScriptAnalyzer rule metadata (PowerShell). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSAvoidMultipleTypeAttributes", | ||
| "title": "Avoid Multiple Type Attributes", | ||
| "description": "PSScriptAnalyzer rule 'PSAvoidMultipleTypeAttributes'. See docs for details.", |
There was a problem hiding this comment.
Typo in description text (user-facing rule metadata): "Prameter" should be "Parameter". Consider correcting via Analysis/Catalog/overrides/powershell/PSAvoidMultipleTypeAttributes.json.
| "description": "PSScriptAnalyzer rule 'PSAvoidMultipleTypeAttributes'. See docs for details.", | |
| "description": "PSScriptAnalyzer rule 'PSAvoidMultipleTypeAttributes'. Avoid specifying multiple type attributes for a single Parameter.", |
There was a problem hiding this comment.
IntelligenceX triage: The comment requests fixing a typo in the description (and possibly via overrides). The current diff still shows the unmodified generic description and no override evidence is provided.
Assessed commit: 398502a
Diff range: current PR files
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSAvoidAssignmentToAutomaticVariable", | ||
| "title": "Avoid Assignment To Automatic Variable", | ||
| "description": "PSScriptAnalyzer rule 'PSAvoidAssignmentToAutomaticVariable'. See docs for details.", |
There was a problem hiding this comment.
Grammar issue in description text (user-facing rule metadata): "This automatic variables is" should be plural (e.g., "These automatic variables are"). Consider applying via Analysis/Catalog/overrides/powershell/PSAvoidAssignmentToAutomaticVariable.json.
| "description": "PSScriptAnalyzer rule 'PSAvoidAssignmentToAutomaticVariable'. See docs for details.", | |
| "description": "These automatic variables are reserved for PowerShell's internal use and should not be assigned new values.", |
There was a problem hiding this comment.
IntelligenceX triage: The comment requests correcting grammar in the description. The current diff still shows the generic description and does not demonstrate the proposed fix.
Assessed commit: 398502a
Diff range: current PR files
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSPossibleIncorrectComparisonWithNull", | ||
| "title": "Possible Incorrect Comparison With Null", | ||
| "description": "PSScriptAnalyzer rule 'PSPossibleIncorrectComparisonWithNull'. See docs for details.", |
There was a problem hiding this comment.
Typo in description text (user-facing rule metadata): "comaprision" should be "comparison". Consider fixing via Analysis/Catalog/overrides/powershell/PSPossibleIncorrectComparisonWithNull.json so the generated catalog can still be regenerated losslessly.
There was a problem hiding this comment.
IntelligenceX triage: The comment indicates a typo ('comaprision') to correct, but the current diff context shows only the generic description and no corrected text/override is visible.
Assessed commit: 398502a
Diff range: current PR files
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSUseConsistentWhitespace", | ||
| "title": "Use Consistent Whitespace", | ||
| "description": "PSScriptAnalyzer rule 'PSUseConsistentWhitespace'. See docs for details.", |
There was a problem hiding this comment.
Typo in description text (user-facing rule metadata): "assigment" should be "assignment". Consider fixing via an override file under Analysis/Catalog/overrides/powershell/PSUseConsistentWhitespace.json to avoid editing generated content directly.
There was a problem hiding this comment.
IntelligenceX triage: The comment requests fixing a typo ('assigment') in description via override; current diff context shows only the generic description and no fix is evident.
Assessed commit: 398502a
Diff range: current PR files
| "language": "powershell", | ||
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSAvoidGlobalFunctions", | ||
| "title": "Avoid Global Functions", |
There was a problem hiding this comment.
Typo in title text (user-facing rule metadata): "functiosn" should be "functions". Consider fixing via Analysis/Catalog/overrides/powershell/PSAvoidGlobalFunctions.json to keep the generated rule file regeneratable.
There was a problem hiding this comment.
IntelligenceX triage: The comment claims a typo in the title ('functiosn'), but the diff shows the title as 'Avoid Global Functions' already; however, there is no evidence in the provided diff that this was changed from the typo or that an override was added. With unclear diff history, keep open.
Assessed commit: 398502a
Diff range: current PR files
| "language": "powershell", | ||
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSAvoidAssignmentToAutomaticVariable", | ||
| "title": "Avoid Assignment To Automatic Variable", |
There was a problem hiding this comment.
Typo in title text (user-facing rule metadata): "automtic" should be "automatic". Consider fixing via Analysis/Catalog/overrides/powershell/PSAvoidAssignmentToAutomaticVariable.json (so regenerated base metadata remains untouched).
There was a problem hiding this comment.
IntelligenceX triage: The comment claims a typo ('automtic') in the title, but the shown diff title reads correctly as 'Automatic'. Since no evidence indicates the typo existed/was fixed in this PR and no override is shown, it's unclear; keep open.
Assessed commit: 398502a
Diff range: current PR files
scripts/sync-pssa-catalog.ps1
Outdated
| $rules = Get-ScriptAnalyzerRule | Sort-Object RuleName | ||
|
|
||
| foreach ($rule in $rules) { | ||
| $ruleName = [string]$rule.RuleName | ||
| if ([string]::IsNullOrWhiteSpace($ruleName)) { continue } |
There was a problem hiding this comment.
Generated rule titles/descriptions include upstream typos (confirmed in several of the newly added PowerShell rule JSONs). If you want the built-in catalog to look polished while keeping the base files machine-regeneratable, consider adding targeted rule overrides under Analysis/Catalog/overrides/powershell/*.json (or a small post-processing correction map in this script).
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSPossibleIncorrectComparisonWithNull", | ||
| "title": "Possible Incorrect Comparison With Null", | ||
| "description": "PSScriptAnalyzer rule 'PSPossibleIncorrectComparisonWithNull'. See docs for details.", |
There was a problem hiding this comment.
Typo in description text (user-facing rule metadata): "equaltiy" should be "equality". If you want to keep generated rule files pristine, apply this correction via a rule override under Analysis/Catalog/overrides/powershell/PSPossibleIncorrectComparisonWithNull.json.
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSUseConsistentIndentation", | ||
| "title": "Use Consistent Indentation", | ||
| "description": "PSScriptAnalyzer rule 'PSUseConsistentIndentation'. See docs for details.", |
There was a problem hiding this comment.
Typo in description text (user-facing rule metadata): "indenation" should be "indentation". As this content is generated from upstream metadata, consider correcting via Analysis/Catalog/overrides/powershell/PSUseConsistentIndentation.json.
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSMisleadingBacktick", | ||
| "title": "Misleading Backtick", | ||
| "description": "PSScriptAnalyzer rule 'PSMisleadingBacktick'. See docs for details.", |
There was a problem hiding this comment.
Typo in description text (user-facing rule metadata): "whitepsace" should be "whitespace". Consider applying the correction via Analysis/Catalog/overrides/powershell/PSMisleadingBacktick.json.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8dc3b599d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "description": "Cross-language security-focused pack. Currently includes C# security rules.", | ||
| "includes": ["csharp-security-default"], | ||
| "description": "Cross-language security-focused pack.", | ||
| "includes": ["csharp-security-default", "powershell-security-default"], |
There was a problem hiding this comment.
Adding powershell-security-default to this pack means any config using all-security-default now always selects PowerShell rules; in the current flow, selected PowerShell rules trigger RunPowerShellAsync, whose runner script hard-fails when PSScriptAnalyzer is not installed. That creates a regression for C#-only repos that previously used this pack without a PowerShell toolchain, and it can fail CI outright when analyze run --strict is enabled.
Useful? React with 👍 / 👎.
scripts/sync-pssa-catalog.ps1
Outdated
| ($text -replace '[\r\n]+', ' ' -replace '\s+', ' ').Trim() | ||
| } | ||
|
|
||
| function Title-FromRuleName([string]$ruleName) { |
There was a problem hiding this comment.
Static analysis (warning): The cmdlet 'Title-FromRuleName' uses an unapproved verb. (rule PSUseApprovedVerbs)
| @@ -3,10 +3,13 @@ | |||
| "language": "powershell", | |||
| "tool": "PSScriptAnalyzer", | |||
| "toolRuleId": "PSAvoidUsingWriteHost", | |||
There was a problem hiding this comment.
Comment: This change replaces a helpful description with a generic “See docs”; if the intent is “generated parity,” consider moving the richer text into an override so UX doesn’t regress while keeping generated files pristine.
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSAvoidAssignmentToAutomaticVariable", | ||
| "title": "Changing automatic variables might have undesired side effects", | ||
| "description": "This automatic variables is built into PowerShell and readonly.", |
There was a problem hiding this comment.
Description has grammar/accuracy issues (“This automatic variables… readonly.”); user-facing metadata should be correct, and if these files are generated this should be corrected via an override file.
| "description": "This automatic variables is built into PowerShell and readonly.", | |
| "description": "Automatic variables are built into PowerShell and are read-only.", |
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSAvoidDefaultValueForMandatoryParameter", | ||
| "title": "Avoid Default Value For Mandatory Parameter", | ||
| "description": "Mandatory parameter should not be initialized with a default value in the param block because this value will be ignored.. To fix a violation of this rule, please avoid initializing a value for the mandatory parameter in the param block.", |
There was a problem hiding this comment.
Has a double period (“ignored..”) and could be tightened; user-facing text should be clean.
| "description": "Mandatory parameter should not be initialized with a default value in the param block because this value will be ignored.. To fix a violation of this rule, please avoid initializing a value for the mandatory parameter in the param block.", | |
| "description": "Mandatory parameters should not be initialized with a default value in the param block because this value will be ignored. To fix a violation of this rule, avoid initializing a value for the mandatory parameter in the param block.", |
| "language": "powershell", | ||
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSAvoidGlobalFunctions", | ||
| "title": "Avoid global functions and aliases", |
There was a problem hiding this comment.
Title says “functions and aliases” but toolRuleId/rule name is specifically about functions; mismatch can confuse users and tooling that expects titles to match scope.
| "title": "Avoid global functions and aliases", | |
| "title": "Avoid global functions", |
scripts/sync-pssa-catalog.ps1
Outdated
| ($text -replace '[\r\n]+', ' ' -replace '\s+', ' ').Trim() | ||
| } | ||
|
|
||
| function Fix-KnownTypos([string]$text) { |
There was a problem hiding this comment.
Static analysis (warning): The cmdlet 'Fix-KnownTypos' uses an unapproved verb. (rule PSUseApprovedVerbs)
| "language": "powershell", | ||
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSMisleadingBacktick", | ||
| "title": "Misleading Backtick", |
There was a problem hiding this comment.
The description contains “whitepsace” but an override was added to correct this; override application seems broken or not used here, so the effective catalog will still show the typo.
| "language": "powershell", | ||
| "tool": "PSScriptAnalyzer", | ||
| "toolRuleId": "PSUseConsistentIndentation", | ||
| "title": "Use consistent indentation", |
There was a problem hiding this comment.
The description contains “indenation” but an override exists with the correct spelling; please verify override precedence/keying so the corrected text is surfaced.
| @@ -0,0 +1,26 @@ | |||
| namespace IntelligenceX.Tests; | |||
There was a problem hiding this comment.
This PR adds overrides intended to fix generated metadata, but there’s no test that LoadFromWorkspace applies overrides for PowerShell rules; add a targeted assertion (e.g., PSMisleadingBacktick description == corrected) to prevent regressions.
| @@ -0,0 +1,118 @@ | |||
| param( | |||
There was a problem hiding this comment.
Consider enabling strict mode to catch script issues early (this script generates repo artifacts, so failing fast matters).
| param( | |
| Set-StrictMode -Version Latest | |
| param( | |
| [Parameter()][string]$OutDir = (Join-Path -Path $PSScriptRoot -ChildPath (Join-Path -Path '..' -ChildPath (Join-Path -Path 'Analysis' -ChildPath (Join-Path -Path 'Catalog' -ChildPath (Join-Path -Path 'rules' -ChildPath 'powershell'))))) | |
| ) |
| AssertEqual(false, automatic!.Description.Contains("This automatic variables is", StringComparison.OrdinalIgnoreCase), "PSAvoidAssignmentToAutomaticVariable grammar fixed via override"); | ||
| AssertEqual(true, automatic.Description.Contains("read-only", StringComparison.OrdinalIgnoreCase), "PSAvoidAssignmentToAutomaticVariable uses read-only wording"); | ||
|
|
||
| AssertEqual(true, catalog.Rules.TryGetValue("PSAlignAssignmentStatement", out var align), "PSAlignAssignmentStatement exists"); |
There was a problem hiding this comment.
Duplicated/competing workspace root discovery with a hardcoded marker file makes tests fragile; reuse the same root resolver as other tests to avoid inconsistent behavior across environments.
| AssertEqual(true, catalog.Rules.TryGetValue("PSAlignAssignmentStatement", out var align), "PSAlignAssignmentStatement exists"); | |
| var workspace = ResolveWorkspaceRoot(); |
| return Environment.CurrentDirectory; | ||
| } | ||
|
|
||
| private static void TestAnalysisCatalogOverrideInvalidTypeFallsBack() { |
There was a problem hiding this comment.
ResolveBuiltInWorkspaceRoot() duplicates logic and uses a PowerShell-specific marker file; this can break if catalog structure changes. Prefer a single shared resolver used by all analysis tests.
scripts/sync-pssa-catalog.ps1
Outdated
|
|
||
| $json = $obj | ConvertTo-Json -Depth 6 | ||
| $path = Join-Path $OutDir ($ruleName + '.json') | ||
| $json | Set-Content -LiteralPath $path -Encoding UTF8 |
There was a problem hiding this comment.
-Encoding UTF8 may write BOM depending on host/version; use explicit utf8NoBOM to keep generated JSON stable and cross-platform.
| $json | Set-Content -LiteralPath $path -Encoding UTF8 | |
| $json | Set-Content -LiteralPath $path -Encoding utf8NoBOM |
scripts/sync-pssa-catalog.ps1
Outdated
|
|
||
| function Get-KnownTypoFixedText([string]$text) { | ||
| if ([string]::IsNullOrWhiteSpace($text)) { return '' } | ||
| $t = $text |
There was a problem hiding this comment.
Broad text rewriting here conflicts with the “use overrides to clean up metadata” model and can accidentally change semantics; recommend removing this and keeping generator output faithful.
| $t = $text | |
| function Get-KnownTypoFixedText([string]$text) { | |
| # Keep upstream metadata pristine; apply any user-facing fixes via Analysis/Catalog/overrides/powershell/*.json | |
| if ([string]::IsNullOrWhiteSpace($text)) { return '' } | |
| return $text | |
| } |
| $defaultSeverity = Get-DefaultSeverity ([string]$rule.Severity) | ||
|
|
||
| $tags = @('powershell', 'psscriptanalyzer') | ||
| if ($category -eq 'Security') { $tags += 'security' } |
There was a problem hiding this comment.
If generation must be pristine, don’t run descriptions through typo-fixing; only normalize whitespace/newlines.
| if ($category -eq 'Security') { $tags += 'security' } | |
| $title = Compress-Whitespace ([string]$rule.CommonName) |
| if ($category -eq 'Reliability') { $tags += 'reliability' } | ||
| if ($category -eq 'BestPractices') { $tags += 'best-practices' } | ||
|
|
||
| $obj = [ordered]@{ |
There was a problem hiding this comment.
Same for descriptions—avoid semantic-altering substitutions during generation.
| $obj = [ordered]@{ | |
| $description = Compress-Whitespace ([string]$rule.Description) |
| "PSUsePSCredentialType" | ||
| ] | ||
| } | ||
|
|
There was a problem hiding this comment.
File ends with an extra blank line after }; remove to keep pack JSON stable/consistent with others.
| } |
| failed += Run("Analysis catalog validator passes built-in catalog", TestAnalysisCatalogValidatorPassesBuiltInCatalog); | ||
| failed += Run("Analysis catalog validator detects invalid catalog", TestAnalysisCatalogValidatorDetectsInvalidCatalog); | ||
| failed += Run("Analysis catalog validator detects missing rule metadata", TestAnalysisCatalogValidatorDetectsMissingRuleMetadata); | ||
| failed += Run("Analysis packs: all-security includes PowerShell", TestAnalysisPacksAllSecurityIncludesPowerShell); |
There was a problem hiding this comment.
This calls TestAnalysisPacksAllSecurityIncludesPowerShell; ensure the new partial file is compiled under the same #if INTELLIGENCEX_REVIEWER symbol as this entrypoint, otherwise this can break or silently exclude the test depending on build configuration. (Merge-blocking because it can make the suite configuration-dependent.)
| failed += Run("Analysis catalog validator detects invalid catalog", TestAnalysisCatalogValidatorDetectsInvalidCatalog); | ||
| failed += Run("Analysis catalog validator detects missing rule metadata", TestAnalysisCatalogValidatorDetectsMissingRuleMetadata); | ||
| failed += Run("Analysis packs: all-security includes PowerShell", TestAnalysisPacksAllSecurityIncludesPowerShell); | ||
| failed += Run("Analysis catalog rule overrides apply", TestAnalysisCatalogRuleOverridesApply); |
There was a problem hiding this comment.
failed += Run("Analysis catalog PowerShell overrides apply", TestAnalysisCatalogPowerShellOverridesApply); is referenced but the new method is not actually called anywhere else; ensure it’s registered here (looks intended but verify the method name/signature matches exactly and compilation symbol gating doesn’t exclude it).
| failed += Run("Analysis catalog rule overrides apply", TestAnalysisCatalogRuleOverridesApply); | |
| failed += Run("Analysis catalog PowerShell overrides apply", TestAnalysisCatalogPowerShellOverridesApply); |
| } | ||
| } | ||
|
|
||
| private static void TestAnalysisCatalogPowerShellOverridesApply() { |
There was a problem hiding this comment.
This method isn’t executed unless registered in Program.cs; without that, the override validation is dead code and won’t prevent regressions.
scripts/sync-pssa-catalog.ps1
Outdated
| if ($slug.StartsWith('PS', [System.StringComparison]::OrdinalIgnoreCase)) { | ||
| $slug = $slug.Substring(2) | ||
| } | ||
| $slug = $slug.ToLowerInvariant() |
There was a problem hiding this comment.
The slug logic doesn’t match the docs URLs used elsewhere in this PR (e.g., avoidusingwritehost), so generated docs links will be wrong; build the slug by removing the leading PS and stripping punctuation/casing to the Learn slug format, or map from known RuleName -> slug.
PrzemyslawKlys
force-pushed
the
codex/static-policy-powershell
branch
from
c526444 to
33f9b9c
Compare
| } | ||
| AssertEqual("https", uri.Scheme, $"{rule.Id} docs uses https"); | ||
|
|
||
| // Prefer Learn; tolerate harmless canonicalization differences like subdomains. |
There was a problem hiding this comment.
Comment text.
| // Prefer Learn; tolerate harmless canonicalization differences like subdomains. | |
| // Learn sometimes includes a locale segment, e.g. /en-us/powershell/... | |
| var maybeLocale = segments.Length > 0 ? segments[0] : ""; | |
| if (maybeLocale.Length == 5 && maybeLocale[2] == '-') { | |
| offset = 1; | |
| } |
| # Common system-wide module locations. | ||
| if ($runningOnWindows) { | ||
| if ($env:ProgramFiles) { | ||
| $trustedRoots += (Join-Path -Path $env:ProgramFiles -ChildPath (Join-Path -Path 'WindowsPowerShell' -ChildPath 'Modules')) |
There was a problem hiding this comment.
Selecting “latest installed” PSScriptAnalyzer (Sort-Object Version -Descending | Select-Object -First 1) will cause catalog churn across environments; pin/validate an expected major/minor (or emit the chosen version into repo/CI) to keep generated metadata stable.
| // Example: en-us, de-de. We keep this permissive (just shape) so the test doesn't need a locale allowlist. | ||
| if (string.IsNullOrWhiteSpace(value) || value!.Length != 5) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
The locale detection assumes Length == 5, which is brittle if Learn introduces longer locale tags or other path variants; this is a merge-blocker due to CI flake risk.
| } | |
| static bool LooksLikeLearnLocaleSegment(string? value) { | |
| // Example: en-us, de-de. Keep permissive (shape-only) so the test doesn't need a locale allowlist. | |
| if (string.IsNullOrWhiteSpace(value)) { | |
| return false; | |
| } | |
| // Accept patterns like "en-us" (5) and be resilient to future variants. | |
| return value.Length >= 5 && | |
| char.IsLetter(value[0]) && | |
| char.IsLetter(value[1]) && | |
| value[2] == '-' && | |
| value.Skip(3).All(char.IsLetter); | |
| } |
| # Ignore removal errors; Import-Module by explicit path will still fail if the session can't load it. | ||
| Write-Verbose ("Remove-Module PSScriptAnalyzer failed (ignored): {0}" -f $_.Exception.Message) | ||
| } | ||
| Import-Module -Name $module.Path -ErrorAction Stop |
There was a problem hiding this comment.
Importing by -Name can re-resolve from PSModulePath; use -LiteralPath to guarantee you import the validated file.
| Import-Module -Name $module.Path -ErrorAction Stop | |
| Import-Module -LiteralPath $module.Path -ErrorAction Stop |
| offset = 1; | ||
| } | ||
| AssertEqual(true, segments.Length >= (5 + offset), $"{rule.Id} docs path has enough segments"); | ||
| if (segments.Length < (5 + offset)) { |
There was a problem hiding this comment.
Path segment comparisons should be case-insensitive (Learn may canonicalize); otherwise harmless casing breaks CI.
| if (segments.Length < (5 + offset)) { | |
| AssertEqual(true, string.Equals("powershell", segments[offset + 0], StringComparison.OrdinalIgnoreCase), $"{rule.Id} docs uses Learn powershell path"); |
| } | ||
| AssertEqual(true, segments.Length >= (5 + offset), $"{rule.Id} docs path has enough segments"); | ||
| if (segments.Length < (5 + offset)) { | ||
| throw new InvalidOperationException($"Expected {rule.Id} docs path to include '/powershell/utility-modules/psscriptanalyzer/rules/<slug>', got '{path}'."); |
There was a problem hiding this comment.
Same issue for this segment.
| throw new InvalidOperationException($"Expected {rule.Id} docs path to include '/powershell/utility-modules/psscriptanalyzer/rules/<slug>', got '{path}'."); | |
| AssertEqual(true, string.Equals("utility-modules", segments[offset + 1], StringComparison.OrdinalIgnoreCase), $"{rule.Id} docs uses Learn utility-modules path"); |
| AssertEqual(true, segments.Length >= (5 + offset), $"{rule.Id} docs path has enough segments"); | ||
| if (segments.Length < (5 + offset)) { | ||
| throw new InvalidOperationException($"Expected {rule.Id} docs path to include '/powershell/utility-modules/psscriptanalyzer/rules/<slug>', got '{path}'."); | ||
| } |
There was a problem hiding this comment.
Same issue for this segment.
| } | |
| AssertEqual(true, string.Equals("psscriptanalyzer", segments[offset + 2], StringComparison.OrdinalIgnoreCase), $"{rule.Id} docs uses Learn PSScriptAnalyzer path"); |
| if (segments.Length < (5 + offset)) { | ||
| throw new InvalidOperationException($"Expected {rule.Id} docs path to include '/powershell/utility-modules/psscriptanalyzer/rules/<slug>', got '{path}'."); | ||
| } | ||
| AssertEqual("powershell", segments[offset + 0], $"{rule.Id} docs uses Learn powershell path"); |
There was a problem hiding this comment.
Same issue for this segment.
| AssertEqual("powershell", segments[offset + 0], $"{rule.Id} docs uses Learn powershell path"); | |
| AssertEqual(true, string.Equals("rules", segments[offset + 3], StringComparison.OrdinalIgnoreCase), $"{rule.Id} docs uses Learn rules path"); |
Adds a first-party PowerShell rule catalog based on PSScriptAnalyzer (70 rule metadata files), plus a conservative PowerShell default pack (security + correctness baseline) and a new powershell-security-default pack (wired into all-security-default). Includes a small sync script: scripts/sync-pssa-catalog.ps1.