Powershell: Invoke-Expression validates nonexistent parameter default values.

[mklement0]

Note: The repro below uses the ValidateSet attribute to demonstrate the problem, but at least one other attribute is affected too, namely ValidatePattern - possibly more. Presumably, fixing the problem for one attribute also fixes it for the others.

_Update_:

• The core issue is that when script block text with parameter declarations is invoked via Invoke-Expression, default-parameter-value validation for parameters that have Validate* attributes takes place in an _overzealous_ manner: even if there is _no_ default value, the implied value of $null is evaluated against the validation attributes - however, it should be possible to simply _omit_ a default value for validation-constrained parameters (and that is how it works with _direct_ invocations of functions / scripts / script-block literals).

  • @SeeminglyScience's comment below provides helpful analysis of the root cause, source-code links, and a suggested fix.

• As an aside: The helpful aspect of this validation - namely if there _is_ a default value - is currently missing from _direct_ function / script / script-block literal invocations; remedying that is the subject of #8795.

Steps to reproduce

. { param([ValidateSet("one", "two")] $foo) "hi" }
# This command should be equivalent to the above.
Invoke-Expression 'param([ValidateSet("one", "two")] $foo) "hi"'

Expected behavior

hi
hi

Actual behavior

hi
Invoke-Expression : The attribute cannot be added because variable foo with value  would no longer be valid.

PowerShell Core v6.2.0-preview.3 on macOS 10.14.2
PowerShell Core v6.2.0-preview.3 on Ubuntu 18.04.1 LTS
PowerShell Core v6.2.0-preview.3 on Microsoft Windows 10 Pro (64-bit; Version 1803, OS Build: 17134.471)
Windows PowerShell v5.1.17134.407 on Microsoft Windows 10 Pro (64-bit; Version 1803, OS Build: 17134.471)

[BrucePay]

@mklement0 If you add a valid initializer then it's fine

PSCore (2:59) >  Invoke-Expression 'param([ValidateSet("one", "two")] $foo = "one") "hi"'
hi

Note that the behaviour is the same with the InvokeScript method:

$ExecutionContext.InvokeCommand.InvokeScript('param([ValidateSet("one", "two")] $foo) "hi"')

as well as

{param([ValidateSet("one", "two")] $foo) "hi}.Invoke()

Why the error isn't raised when running a script does warrant further investigation.

[vexx32]

@BrucePay that discrepancy is weird. This pattern of declaring a parameter with a validation attribute and then not giving it a default value is pretty common and has been the way it is understood to work for a long time.

I think the better question is why is this error appearing in this context when this pattern has been used for a long time now in declaring parameters. :confused:

[SeeminglyScience]

The scenarios that fail use ScriptBlock.InvokeWithPipeImpl (or any of the other *Invoke* methods that eventually hit it)

The scenarios that don't fail are from a compiled command AST, which creates a command processor instead.

More specifically, here's the line from the InvokeWithPipe code path where it fails. I believe it should use PSVariable.AddParameterAttributesNoChecks instead of that constructor.

Note Same thing happens with the Where/ForEach magic methods as well as ForEach-Object

[mklement0]

Thanks, @vexx32 and @SeeminglyScience.

@BrucePay:

@vexx32 is correct that not specifying a value has always worked and should work in all situations.
In fact, even specifying an _invalid_ default value has always worked:

PS> . { param([ValidateSet("one", "two")] $foo = "invalid") $foo }
invalid

Also, the fact that & ([scriptblock]::Create('param([ValidateSet("one", "two")] $foo = "invalid") $foo'))') works fine too supports @SeeminglyScience's findings.

I haven't delved into the code, but, functionally, it seems that the default value / lack thereof is validated when it shouldn't be.

[mklement0]

Thinking about this some more, based on @BrucePay's response and @SeeminglyScience's findings:

• It sounds like the - sensible - original design intent was indeed to check whether a validation-constrained parameter's default value passes the validation and to complain at parse time, if not.

  • Currently only the Invoke-Expression code path enforces that check, but it does so _overzealously_, because _not using a default value at all_ should definitely be considered acceptable.

• Conversely, the fact that regular invocation - perhaps accidentally - _bypass_ the check opened the door for behavior that is arguably _too permissive_:

  • As demonstrated, default values that are _invalid_ in terms of the parameter's validation attributes are happily accepted.

I see two possible resolutions:

• Make Invoke-Expression invocations as permissive as the AST-based invocations.

• Fix the Invoke-Expression invocations to accept the case where a validation-constrained parameter has _no_ default value - while still enforcing the validation of default values - _and make AST-based invocations do the same_.

  • Caveat: Technically, that would be a _breaking change_, as it is conceivable that there are functions out there that use an invalid default value as an _explicit_ signal that no argument was passed, along the lines of: . { param([ValidateSet("one", "two")] $foo="<none>") $foo }

[SeeminglyScience]

@mklement0 I believe it's just a bug. I think the ScriptBlock.InvokeWithPipeImpl code path should just use the PSVariable.AddParameterAttributesNoChecks method, like the AST compiler code path already does.

[mklement0]

@SeeminglyScience: That's definitely the most straightforward way to resolve this issue.

However, the question is whether we see value in _preventing invalid default values_, which currently doesn't happen (and the bug fix wouldn't give us).

I personally see value in that (e.g., during refactoring of a validation set you may forget to update the default value accordingly), but there are two considerations:

• backward compatibility, as stated above

• performance (introducing additional checks; seemingly even script blocks for ValidateScript attributes are executed during the validation; e.g.: iex 'param([ValidateScript({ $_ -eq "A" })] $foo="B") $foo')

[vexx32]

@mklement0 while I do agree that preventing invalid default values is probably a good idea — after all, one can generally check $PSBoundParameters.ContainsKey() if there is a need to verify if a particular parameter is supplied — I think it would be best if that were questioned in a separate issue, rather than attempting to answer both questions in this one issue. 🙂

[mklement0]

Good point, @vexx32 - please see #8795

[johngeorgef1]

Also interestingly it seems that if you declare a parameter with validation, then change the value of that variable inside the script to something that is _not_ a valid value; it will throw this exception.

e.g.

Save this as a file and execute, supplying one of the three _valid_ options for this parameter [ biff, bash, bosh ];

[Cmdletbinding()]
Param(
    [Parameter(Mandatory=$True)]
    [ValidateSet("biff","bash","bosh")]
    [String]$Word
)

$ErrorActionPreference = "Stop";
Write-Host "Let's give this thing something to do";

$Word = "boosh"; # This will throw an exception

aaand..

PS C:\DevOps\Snips> .\param-validation.ps1 biff
Let's give this thing something to do
C:\DevOps\Snips\param-validation.ps1 : The variable cannot be validated because the value boosh is not a valid value for the Word variable.
At line:1 char:1
+ .\param-validation.ps1 biff
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : MetadataError: (:) [param-validation.ps1], ValidationMetadataException
    + FullyQualifiedErrorId : ValidateSetFailure,param-validation.ps1


PS C:\DevOps\Snips> 

It's quite interesting to learn that the params are _continually_ evaluated to see whether they confirm to the validation attributes.

-J

[mklement0]

@johngeorgef1, this is actually standard behavior, and not specific to Invoke-Expression use:

& { param([ValidateRange(1,2)] $foo) $foo = 3 }
MetadataError: The variable cannot be validated because the value 3 is not a valid value for the foo variable.

It is not well known, but you can apply constraints to _any_ variable, not just a _parameter_ variable:

[ValidateRange(1,2)] $foo = 1;  # regular variable with validation attribute
$foo = 3 # same error as above

While that is a powerful feature, constraints that apply to _parameter_ variables only are inappropriately applied in at least one case: #10426