diff --git a/Rules/InvalidMultiDotValue.cs b/Rules/InvalidMultiDotValue.cs new file mode 100644 index 000000000..09b723996 --- /dev/null +++ b/Rules/InvalidMultiDotValue.cs @@ -0,0 +1,154 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Management.Automation.Language; + +#if !CORECLR +using System.ComponentModel.Composition; +#endif + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + + /// + /// Rule that reports an error when an unquoted value contains multiple dots, + /// which is likely an attempt to construct a version number (e.g., 1.2.3) + /// that is not properly quoted and thus misinterpreted as a double with member access. + /// + public class InvalidMultiDotValue : ConfigurableRule + { + + /// + /// Construct an object of InvalidMultiDotValue type. + /// + public InvalidMultiDotValue() { + Enable = false; + } + + /// + /// Analyzes the given ast to find the [violation] + /// + /// AST to be analyzed. This should be non-null + /// Name of file that corresponds to the input AST. + /// A an enumerable type containing the violations + public override IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + // Find all MemberExpressionAst nodes representing invalid unquoted multi-dot values + IEnumerable invalidAsts = ast.FindAll(testAst => + // An expression with 3 or more dots is seen as a double with an additional property + testAst is MemberExpressionAst memberAst && + // The first two values are seen as a double + memberAst.Expression.StaticType == typeof(double) && + // the rest is seen as a member of type int or double + memberAst.Member is ConstantExpressionAst constantAst && + ( + constantAst.StaticType == typeof(int) || // e.g.: [Version]1.2.3 + constantAst.StaticType == typeof(double) // e.g.: [Version]1.2.3.4 + ), + true + ); + + if (invalidAsts != null) { + var correctionDescription = Strings.InvalidMultiDotValueCorrectionDescription; + foreach (MemberExpressionAst invalidAst in invalidAsts) + { + var corrections = new List { + new CorrectionExtent( + invalidAst.Extent.StartLineNumber, + invalidAst.Extent.EndLineNumber, + invalidAst.Extent.StartColumnNumber, + invalidAst.Extent.EndColumnNumber, + "'" + invalidAst.Extent.Text + "'", + fileName, + correctionDescription + ) + }; + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.InvalidMultiDotValueError, + invalidAst.Extent.Text + ), + invalidAst.Extent, + GetName(), + DiagnosticSeverity.Error, + fileName, + invalidAst.Extent.Text, + suggestedCorrections: corrections + ); + } + } + } + + /// + /// Retrieves the common name of this rule. + /// + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.InvalidMultiDotValueCommonName); + } + + /// + /// Retrieves the description of this rule. + /// + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.InvalidMultiDotValueDescription); + } + + /// + /// Retrieves the name of this rule. + /// + public override string GetName() + { + return string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.InvalidMultiDotValueName); + } + + /// + /// Retrieves the severity of the rule: error, warning or information. + /// + public override RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// Gets the severity of the returned diagnostic record: error, warning, or information. + /// + /// + public DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Warning; + } + + /// + /// Retrieves the name of the module/assembly the rule is from. + /// + public override string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + + /// + /// Retrieves the type of the rule, Builtin, Managed or Module. + /// + public override SourceType GetSourceType() + { + return SourceType.Builtin; + } + } +} + diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 2a04fd759..96f87fdca 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1221,6 +1221,21 @@ The insecure AllowUnencryptedAuthentication switch was used. This should be avoided except for compatibility with legacy systems. + + Invalid Multi-Dot Value + + + PowerShell does not support an implicit value with multiple dots. Any unquoted value with 2 or more dots will result in `$null`. + + + InvalidMultiDotValue + + + The unquoted '{0}' expression is not a valid syntax. Types with multiple dots need to be constructed from either a quoted string or individual components. + + + Quote the value that contains multiple dots + AvoidUsingAllowUnencryptedAuthentication diff --git a/Tests/Rules/InvalidMultiDotValue.tests.ps1 b/Tests/Rules/InvalidMultiDotValue.tests.ps1 new file mode 100644 index 000000000..7d4bc9e68 --- /dev/null +++ b/Tests/Rules/InvalidMultiDotValue.tests.ps1 @@ -0,0 +1,191 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +[Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'False positive')] +param() + +BeforeAll { + $ruleName = "PSInvalidMultiDotValue" + $ruleMessage = "The unquoted '{0}' expression is not a valid syntax. Types with multiple dots need to be constructed from either a quoted string or individual components." + $correctionDescription = 'Quote the value that contains multiple dots' +} + +Describe "InvalidMultiDotValue" { + + BeforeAll { + $Settings = @{ + IncludeRules = @($ruleName) + Rules = @{ $ruleName = @{ Enable = $true } } + } + } + + Context "Violates" { + It "3 version components" { + $scriptDefinition = { $version = 1.2.3 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be '1.2.3' + $violations.Message | Should -Be ($ruleMessage -f '1.2.3') + $violations.RuleSuppressionID | Should -Be '1.2.3' + $violations.SuggestedCorrections.Text | Should -Be "'1.2.3'" + $violations.SuggestedCorrections.Description | Should -Be $correctionDescription + } + + It "4 version components" { + $scriptDefinition = { $version = 1.2.3.4 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be '1.2.3.4' + $violations.Message | Should -Be ($ruleMessage -f '1.2.3.4') + $violations.RuleSuppressionID | Should -Be '1.2.3.4' + $violations.SuggestedCorrections.Text | Should -Be "'1.2.3.4'" + $violations.SuggestedCorrections.Description | Should -Be $correctionDescription + } + + + It "With class initializer" { + $scriptDefinition = { $version = [Version]1.2.3 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be '1.2.3' + $violations.Message | Should -Be ($ruleMessage -f '1.2.3') + $violations.RuleSuppressionID | Should -Be '1.2.3' + $violations.SuggestedCorrections.Text | Should -Be "'1.2.3'" + $violations.SuggestedCorrections.Description | Should -Be $correctionDescription + } + + It "As parameter" { + $scriptDefinition = { + param( + [Version]$version = 1.2.3 + ) + Write-Verbose $version + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be '1.2.3' + $violations.Message | Should -Be ($ruleMessage -f '1.2.3') + $violations.RuleSuppressionID | Should -Be '1.2.3' + $violations.SuggestedCorrections.Text | Should -Be "'1.2.3'" + $violations.SuggestedCorrections.Description | Should -Be $correctionDescription + } + + # Even an IP address is apparently expect below. + # The violation message and description presume a version + # is expected because this is the more commonly used type. + It "IP Address" { + $scriptDefinition = { $IP = [System.Net.IPAddress]127.0.0.1 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Error + $violations.Extent.Text | Should -Be '127.0.0.1' + $violations.Message | Should -Be ($ruleMessage -f '127.0.0.1') + $violations.RuleSuppressionID | Should -Be '127.0.0.1' + $violations.SuggestedCorrections.Text | Should -Be "'127.0.0.1'" + $violations.SuggestedCorrections.Description | Should -Be $correctionDescription + } + } + + Context "Compliant" { + It "From string" { + $scriptDefinition = { $Version = [Version]'1.2.3' }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It "From version components" { + $scriptDefinition = { $Version = [Version]::new(1, 2, 3, 4) }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It "From (bare) double" { + $scriptDefinition = { $Version = [Version]1.2 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + + It "Dot notation" { #PowerShell:27356 + $scriptDefinition = { + $1.2.3.4 + $intKeys = @{ 1 = @{ 2 = @{ 3 = @{ 4 = 'test' } } } } + $intKeys.1.2.3.4 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + } + + Context "Disabled" { + + BeforeAll { + $Settings = @{ + IncludeRules = @($ruleName) + Rules = @{ $ruleName = @{ Enable = $false } } + } + } + + It "ConvertFrom-SecureString -AsPlainText" { + $scriptDefinition = { $version = 1.2.3 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + } + + Context "Suppressed" { + It "All" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSInvalidMultiDotValue', '', Justification = 'Test')] + param() + $version = 1.2.3 + $IP = [System.Net.IPAddress]127.0.0.1 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It "1.2.3" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSInvalidMultiDotValue', '1.2.3', Justification = 'Test')] + param() + $version = 1.2.3 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It "127.0.0.1" { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSInvalidMultiDotValue', '127.0.0.1', Justification = 'Test')] + param() + $IP = [System.Net.IPAddress]127.0.0.1 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + } + + Context "Fixing" { + + BeforeAll { # See request: #1938 + $tempFile = Join-Path $TestDrive 'TestScript.ps1' + } + + It "Version" { + Set-Content -LiteralPath $tempFile -Value {$version = 1.2.3}.ToString() -NoNewLine + $violations = Invoke-ScriptAnalyzer -Path $tempFile -Settings $Settings -fix + Get-Content -LiteralPath $tempFile -Raw | Should -Be {$version = '1.2.3'}.ToString() + } + + It "IP Address" { + Set-Content -LiteralPath $tempFile -Value {$IP = [System.Net.IPAddress]127.0.0.1}.ToString() -NoNewLine + $violations = Invoke-ScriptAnalyzer -Path $tempFile -Settings $Settings -fix + Get-Content -LiteralPath $tempFile -Raw | Should -Be {$IP = [System.Net.IPAddress]'127.0.0.1'}.ToString() + } + } +} \ No newline at end of file diff --git a/docs/Rules/InvalidMultiDotValue.md b/docs/Rules/InvalidMultiDotValue.md new file mode 100644 index 000000000..19ca0f59e --- /dev/null +++ b/docs/Rules/InvalidMultiDotValue.md @@ -0,0 +1,62 @@ +--- +description: Invalid unquoted multi-dot value construction +ms.date: 04/24/2024 +ms.topic: reference +title: InvalidMultiDotValue +--- +# InvalidMultiDotValue + +**Severity Level: Error** + +## Description + +PowerShell doesn't support unquoted literal values with multiple dots (`.`). Any value with two or +more dots results in `$null`. This rule identifies instances where such values are used, which can +lead to unexpected behavior or errors in the code. + +To create values of the intended type, enclose the value in quotes and use type-casting or use type +constructor methods to create the appropriate object. + + +## Example + +### Wrong + +```powershell +$version = 1.2.3 +``` + +or even: + +```powershell +$IP = [System.Net.IPAddress]127.0.0.1 +``` + +Where both examples will result in `$null` instead of any specific object. + +### Correct + +```powershell +# Use type-casting with quoted value +$IP = [System.Net.IPAddress]'127.0.0.1' +$version = [Version]'1.2.3' + +# Use type constructor method +$version = [Version]::new(1, 2, 3) +``` + +## Configuration + +```powershell +Rules = @{ + PSAvoidExclaimOperator = @{ + Enable = $true + } +} +``` + +### Parameters + +- `Enable`: **bool** (Default value is `$false`) + + Enable or disable the rule during ScriptAnalyzer invocation. diff --git a/docs/Rules/README.md b/docs/Rules/README.md index fca031e33..ffed0e716 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -48,6 +48,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [DSCUseIdenticalMandatoryParametersForDSC](./DSCUseIdenticalMandatoryParametersForDSC.md) | Error | Yes | | | [DSCUseIdenticalParametersForDSC](./DSCUseIdenticalParametersForDSC.md) | Error | Yes | | | [DSCUseVerboseMessageInDSCResource](./DSCUseVerboseMessageInDSCResource.md) | Error | Yes | | +| [InvalidMultiDotValue](./InvalidMultiDotValue.md) | Error | No | Yes | | [MisleadingBacktick](./MisleadingBacktick.md) | Warning | Yes | | | [MissingModuleManifestField](./MissingModuleManifestField.md) | Warning | Yes | | | [PlaceCloseBrace](./PlaceCloseBrace.md) | Warning | No | Yes |