Skip to content
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

Prompting for mandatory parameters mishandles [bool], [switch], [hashtable], and [scriptblock] parameters #4068

Closed
mklement0 opened this issue Jun 21, 2017 · 24 comments
Labels
Committee-Reviewed PS-Committee has reviewed this and made a decision Issue-Enhancement the issue is more of a feature request than a bug Resolution-No Activity Issue has had no activity for 6 months or more Up-for-Grabs Up-for-grabs issues are not high priorities, and may be opportunities for external contributors WG-Engine core PowerShell engine, interpreter, and runtime

Comments

@mklement0
Copy link
Contributor

mklement0 commented Jun 21, 2017

Note: A mandatory [switch] parameter is an edge case, given that switches are usually optional. However, mandatory [bool] parameters are affected as well, as are [hashtable]-and [scriptblock]-typed ones.

The value entered by the user is invariably considered a string, which means:

  • for a mandatory [bool] parameter, any nonempty input - including literal $False - is coerced to $True.

  • for a mandatory [switch] parameter, it is currently impossible to provide input that is accepted.

  • for a mandatory [hashtable] or [scriptblock] parameter, it too is impossible to provide input that is accepted.

Also, for a parameter that has the [AllowNull()] attribute you cannot interactively specify $null (it will be treated as string literal '$null').

Steps to reproduce

# Define a function with mandatory [bool] parameter and invoke it without a value.
PS> function Foo { param( [Parameter(Mandatory)] [bool] $Bar) $Bar }; Foo

cmdlet Foo at command pipeline position 1
Supply values for the following parameters:
Bar: $False   # '$False' is the typed-in input; '0' yields the same result - any nonempty input results in $True

# Ditto for [switch] parameter
PS> function Foo { param( [Parameter(Mandatory)] [switch] $Bar) $Bar }; Foo
Supply values for the following parameters:.
Bar: $False   # '$False' is the typed-in input; *any* input is rejected

# Ditto for [hashtable]
PS> function Foo { param( [Parameter(Mandatory)] [hashtable] $Bar) $Bar }; Foo
Supply values for the following parameters:
Bar: @{ hi = 'there' }   # '@{ hi = 'there' }' is the typed-in input; *any* input is rejected.

# Ditto for [scriptblock]
PS> function Foo { param( [Parameter(Mandatory)] [scriptblock] & $Bar) $Bar }; Foo
Supply values for the following parameters:
Bar: { 1 + 2 }   # '{ 1 + 2 }' is the typed-in input; *any* input is rejected.

# Ditto for $null
PS> Function Foo { param( [Parameter(Mandatory)][AllowNull()]$Bar) $null -eq $Bar }; Foo
Supply values for the following parameters:
Bar: $null  # '$null' is the typed-in input; `$null` is invariably interpreted as *string* '$null'

Expected behavior

False

IsPresent
---------
    False


Name                           Value
----                           -----
hi                             there

3

True

Actual behavior

True

Foo : Cannot process argument transformation on parameter 'Bar'. Cannot convert value "System.String" to type "System.Management.Automation.SwitchParameter".
Boolean parameters accept only Boolean values and numbers, such as $True, $False, 1 or 0.

Foo : Cannot process argument transformation on parameter 'Bar'. Cannot convert the "@{ hi = 'there' }" value of type "System.String" to type "System.Collections.Hashtable".

Foo : Cannot process argument transformation on parameter 'Bar'. Cannot convert the "{ 1 + 2 }" value of type "System.String" to type "System.Management.Automation.ScriptBlock".

False

For the [bool] parameter, the non-empty string literal $False was coerced to $True.
Note that the same would have happened with input 0.

By contrast, if you pass a value on the command line, things work as expected:

# $False, $True and numbers are recognized:
PS> function Foo { param( [Parameter(Mandatory)] [bool] $Bar) $Bar }; Foo -Bar $False
False

# Any other value for [bool] is rejected.
PS> function Foo { param( [Parameter(Mandatory)] [bool] $Bar) $Bar }; Foo -Bar none
Foo : Cannot process argument transformation on parameter 'Bar'. Cannot convert value "System.String" to type "System.Boolean". Boolean parameters accept only Boolean values and 
numbers, such as $True, $False, 1 or 0.

# [switch] parameter currently ONLY accept $True or $False
PS> function Foo { param( [Parameter(Mandatory)] [switch] $Bar) $Bar }; Foo -Bar:$False
IsPresent
---------
    False

PS> function Foo { param( [Parameter(Mandatory)] [hashtable] $Bar) $Bar }; Foo @{ hi = 'there' }

Name                           Value
----                           -----
hi                             there

PS> function Foo { param( [Parameter(Mandatory)] [scriptblock] $Bar) & $Bar }; Foo { 1 + 2}
3

PS> function Foo { param( [Parameter(Mandatory)] [AllowNull()] $Bar) $null -eq $Bar }; Foo $null
True

Environment data

PowerShell Core v6.0.0-beta (v6.0.0-beta.2) on macOS 10.12.5
PowerShell Core v6.0.0-beta (v6.0.0-beta.2) on Ubuntu 16.04.1 LTS
PowerShell Core v6.0.0-beta (v6.0.0-beta.2) on Microsoft Windows 10 Pro (64-bit; v10.0.14393)
Windows PowerShell v5.1.15063.413 on Microsoft Windows 10 Pro (64-bit; v10.0.15063)
@SteveL-MSFT SteveL-MSFT added the WG-Engine core PowerShell engine, interpreter, and runtime label Jun 22, 2017
@TravisEz13 TravisEz13 added the Issue-Enhancement the issue is more of a feature request than a bug label Apr 14, 2018
@mklement0 mklement0 changed the title Prompting for mandatory parameters mishandles [bool] and [switch] parameters Prompting for mandatory parameters mishandles [bool] and [switch] and [hashtable] parameters Jun 8, 2018
@lzybkr
Copy link
Member

lzybkr commented Jun 9, 2018

Prompting is by design not interpreted as PowerShell - the justification being security. From the prompt, it doesn't feel like you're being asked for valid PowerShell (and indeed you aren't, e.g. you can enter a string with spaces and not need quotes), so you might input something that would have unfortunate consequences if it were interpreted as PowerShell.

Also - if this were to change, care would be needed to ensure it doesn't create a way around restricted language mode.

There was some discussion about special casing true/false and variants, I don't recall any conclusions.

@mklement0
Copy link
Contributor Author

mklement0 commented Jun 9, 2018

@lzybkr:

Indeed, you shouldn't be able to enter just any expression.

What you should be able to enter, though, is tokens that successfully - and meaningfully - bind to the parameter triggering the prompt.

If that condition cannot be met, even putting up the prompt is a waste of the user's time and frustration is very likely to ensue.

@mklement0
Copy link
Contributor Author

P.S.:

In the case of a mandatory [bool] parameter (granted, probably rare), it goes beyond frustration:

If you mean to input $False, the command will happily proceed with the opposite of your intent.

The larger issue is that the prompting feature in its present form is both incomplete (see this issue and the inability to enter !-prefixed values) and inconvenient (no tab completion, no re-prompting on invalid input), and therefore currently of little use.

@mklement0
Copy link
Contributor Author

mklement0 commented Jun 9, 2018

On second thought:

it doesn't feel like you're being asked for valid PowerShell (and indeed you aren't, e.g. you can enter a string with spaces and not need quotes

Arguably, the very same rules as in argument mode should apply (even though the user supplying literals is the far likelier scenario).

Yes, values with spaces being implicitly considered a single argument is unique to prompting, however.

From a security perspective, however, there's no good reason to accept less than what could be passed directly as arguments.
If users are allowed to pass expressions / entire command sequences inside $(...) as arguments, there's no reason not to allow them to do that interactively.

Someone who wanted to be more restrictive should implement custom prompting.

That said, the following is, of course, a valid concern that needs to be addressed, as you state:

if this were to change, care would be needed to ensure it doesn't create a way around restricted language mode.

@mklement0
Copy link
Contributor Author

@lzybkr: Sorry for the revision frenzy around my previous comment - I initially hadn't considered all implications of your comments.

I get that this is a tricky issue that requires serious thought, but whatever the outcome is, for the user to trust and use the feature it is important that the rules and limitations are clear, documented, and not too complex to remember.

For instance, one way of resolving the issue, if allowing arbitrary expressions turns out to be infeasible, is to restrict input to:

  • literals, including hashtable literals composed of only literals and the variables noted below

  • variables (constants) $Null, $False, and $True

    • with literals 0 and 1 mapping to $False and True for [bool] params

@mklement0 mklement0 changed the title Prompting for mandatory parameters mishandles [bool] and [switch] and [hashtable] parameters Prompting for mandatory parameters mishandles [bool], [switch], [hashtable], and [scriptblock] parameters Jun 9, 2018
@mklement0
Copy link
Contributor Author

@lzybkr:

I've added [scriptblock] to the list of parameter types that aren't supported by the prompts, as well as the inability to enter $null.

I think that covers all literal syntax forms now (including quasi-literals $True, $False, and $null), but I may still be missing some.

As for a quick and safe way (though not user-friendly) to resolve this issue:

Given that it makes no sense to display a prompt in these cases - you won't be able to enter any / the intended value - a terminating error should be thrown instead.

@mklement0
Copy link
Contributor Author

Let me attempt a pragmatic summary:

  • The automatic prompting mechanism for mandatory parameters that weren't bound by a command-line argument is currently broken with respect to several parameter data types:

    • For parameters typed [hashtable], [scriptblock], [bool], [switch] (and others?), the automatic prompting:
      • either fundamentally prevents entering values successfully
      • or prevents meaningful input ([bool], [switch] invariably bind $True, whatever the user inputs).
  • The minimum we should do is to document this limitation - @SteveL-MSFT, can I ask you to add the documentation-needed label?

    • In addition to the data-type limitation, it should also be made clear that only literal input is supported (no support for variable references, expressions).
  • A simple fix that would spare users frustration and unexpected behavior is to simply disable automatic prompting for said types - report a statement-terminating error instead.

    • This fix too would require documenting.
  • A proper fix would be nontrivial and would require addressing the conceptual issues @lzybkr raises, but it would also be an opportunity to address other existing quirks (inability to enter !-prefixed values, lack of tab completion, no re-prompting on invalid input).

@SteveL-MSFT
Copy link
Member

Before adding the Documentation-Needed label, I think we should agree on the behavior. At a minimum, it seems that erroring and not prompting for these types may be a good enough solution and better than the current experience.

If we wanted to prompt for [bool] and [switch], the prompt should just be a Y/N type of prompt. Given the security and complexity concerns, it doesn't make sense to try to prompt for [hashtable] and [scriptblock].

@mklement0
Copy link
Contributor Author

Thanks, @SteveL-MSFT - makes sense.

If we wanted to prompt for [bool] and [switch], the prompt should just be a Y/N type of prompt.

That's a great idea and bypasses the problem of not being able to use expressions (variables $True and $False) in the input.

So my vote is for an amended version of the simple fix proposed above:

  • modify [bool] and [switch] prompts to present a Y/N choice.

  • do not prompt for [scriptblock] and [hashtable] and throw a statement-terminating error instead.

@mklement0
Copy link
Contributor Author

@SteveL-MSFT:

On a related noted, I've created #7093 to improve the experience with the !-prefixed prompt commands, which also require documenting.

In short: if we eliminate them as proposed, we get a fix for #4066 for free.

@SteveL-MSFT
Copy link
Member

@mklement0 I think we can do this in stages. I would think it's relatively simple to throw a statement-terminating error for [bool], [switch], [scriptblock], and [hashtable] now and consider more targeted prompting in the future.

@SteveL-MSFT SteveL-MSFT added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Jun 20, 2018
@SteveL-MSFT
Copy link
Member

@PowerShell/powershell-committee reviewed this and agree that throwing an error is a better experience than the current prompt that fails with an error implying it should work. To support an improved experience, we should have a RFC.

@SteveL-MSFT SteveL-MSFT added Up-for-Grabs Up-for-grabs issues are not high priorities, and may be opportunities for external contributors Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Jun 20, 2018
@yecril71pl
Copy link
Contributor

yecril71pl commented Jun 18, 2020

If users are allowed to pass expressions / entire command sequences inside $(...) as arguments, there's no reason not to allow them to do that interactively.

Users need not be able to pass any parameters, since the script at hand may abuse this misfeature to read parameters to internal commands. Also, PowerShell should have a non-interactive mode. I am not happy when my scripts wait forever for my input.

@vexx32
Copy link
Collaborator

vexx32 commented Jun 18, 2020

PowerShell should have a non-interactive mode

pwsh -NonInteractive

@mklement0
Copy link
Contributor Author

mklement0 commented Jun 18, 2020

Users need not be able to pass any parameters

The point was that user can already pass the output (result) of any expression as a command-line argument (assuming the language mode in effect allows it) and that that expression is evaluated before the target command sees it, which has no bearing on the the command (script, function, cmdlet) being called (this is not the type of security concern as with, say, SQL injection).

The same applies in principle to interactive input of an argument value - though additional constraints there definitely makes sense:

To recap my earlier statement:

Indeed, you shouldn't be able to enter just any expression.
What you should be able to enter, though, is tokens that successfully - and meaningfully - bind to the parameter triggering the prompt.


That said, if I understand the committee decision correctly, the upshot is:

  • For now, at least spare users the frustration of a prompt that can't be answered, by reporting a statement-terminating error instead - anyone can submit a PR.

    • Personally, given the inconsistent experience, I think it would make sense to also provide a (preference-variable-based?) opt-in to disabling mandatory-parameter prompts altogether.

    • pwsh -NonInteractive definitely disables these prompts too, but it disables all interactive prompts, notably including Read-Host.

  • Longer-term, someone should author an RFC for an improved prompting experience.

@yecril71pl
Copy link
Contributor

yecril71pl commented Jun 18, 2020

Users need not be able to pass any parameters

The point was that user can already pass the output (result) of any expression as a command-line argument (assuming the language mode in effect allows it)

Many enterprise desktop users can only run applications and scripts that are pre-approved by IT. Such users would not be allowed to start PowerShell as such, or to edit any scripts, even if they were smart enough to do that, which most of them are not.
Also, especially when you are a power user, you do not want to do the equivalent of rm -rf $UNSET/* inadvertently. That could happen if expressions were interpreted.

@mklement0
Copy link
Contributor Author

mklement0 commented Jun 18, 2020

Again: no one has asked for the ability to enter arbitrary commands (expressions, variable references) into an interactive mandatory-parameter prompt.

Perhaps I should have been clearer in saying that you should be able to enter (quasi-)literals of the target parameter's type, such as $true, $false, and { ... } (for script blocks - though you could debate there whether the enclosing braces should be required), and, if the parameter allows it, maybe $null.
With @{ ... } literals (hash tables), the line gets blurry, given that entry values can be expressions.

Therefore - if we want to improve the experience - it makes sense to author an RFC to come up with a user experience that is (a) predictable and (b) flexible enough while avoiding inadvertent evaluation, which are non-trivial problems to solve.

Then again, pragmatically speaking, reporting an error in lieu of prompting for unsupported types may be good enough (I've removed the word "stopgap" from my previous comment.)
You could argue that especially [hashtable] and [scriptblock] parameters are inherently ill-suited to interactive input - especially by less experienced users.

The only thing I personally think is missing is the previously suggested opt-in to disable the mandatory-parameter prompts altogether (independently of the -NonInteractive CLI switch).

I think we could even consider reversing the logic: requiring an opt-in if you want these prompts; this technically breaking change would only affect scripts that were explicitly designed to have the user answer such prompts triggered by incomplete calls inside the script - which strikes me as unlikely.

@yecril71pl
Copy link
Contributor

Perhaps I should have been clearer in saying that you should be able to enter (quasi-)literals of the target parameter's type, such as $true, $false, and { ... } (for script blocks - though you could debate there whether the enclosing braces should be required), and, if the parameter allows it, maybe $null.

$true is not a quasi-literal, it is a variable reference, so requiring this $ would only cause confusion. The shell’s representation on output is just True—but interactive logical valuation should be interpreted according to the current culture. The calling function should not be able to & a user­‑provided script block (script blocks for DSL would be OK, although it would be highly impractical to interactively enter rescue values for them).

@mklement0
Copy link
Contributor Author

Taking a step back, regarding disabling the prompts altogether:

If technically feasible, a simpler solution would be to restrict the mandatory-parameter prompts solely to commands submitted at the command prompt of an interactive session.

E.g., Submitting just Get-Content at the command prompt would prompt for a -Path argument, but calling Get-Content from inside a script or function or via the CLI wouldn't.

That way, without the need for a new preference variable, we could retain the prompts in the context of the command-prompt experience while taking away the pain of having scripts that mistakenly lack mandatory arguments hang in automation contexts where -NonInteractive wasn't explicitly used.

@yecril71pl
Copy link
Contributor

Taking a step back, regarding disabling the prompts altogether:

If technically feasible, a simpler solution would be to restrict the mandatory-parameter prompts solely to commands submitted at the command prompt of an interactive session.

E.g., Submitting just Get-Content at the command prompt would prompt for a -Path argument, but calling Get-Content from inside a script or function or via the CLI wouldn't.

That way, without the need for a new preference variable, we could retain the prompts in the context of the command-prompt experience while taking away the pain of having scripts that mistakenly lack mandatory arguments hang in automation contexts where -NonInteractive wasn't explicitly used.

A preference variable would still be necessary because it is a breaking change—they may be scripts in the wild that abuse this feature to ask users in the middle of something..

@mklement0
Copy link
Contributor Author

mklement0 commented Jun 18, 2020

so requiring this $ would only cause confusion.
interactive logical valuation should be interpreted according to the current culture.

Fair enough, but such questions need to be resolved in the yet-to-be-if-ever written RFC.

The calling function should not be able to & a user­‑provided script block

Assuming you mean the called function: It should, and it can - if the language mode allows it; to put it differently: you need to use the language mode to control this ability.

A preference variable would still be necessary because it is a breaking change—they may be scripts in the wild that abuse this feature to ask users in the middle of something..

Yes, it is technically breaking, as stated (in the context of the earlier suggestion of reversing the opt-in logic), but there's also the concept of a Bucket 3: Unlikely Grey Area breaking change, where users are unlikely to depend on the old behavior, in which case the benefits of the breaking change may outweigh the risk of breaking existing code. Personally, this strikes me as such a change.

(And a preference variable could still be introduced, separately, to enable users to never present these prompts.)

@mklement0
Copy link
Contributor Author

P.S.: Irrespective of whether the currently unsupported types will ever be supported by the prompts following a RFC, I want to reiterate that the current UX with these prompts is generally poor: no tab completion, no re-prompting on invalid input (and !-prefixed values invariably getting interpreted as prompt-related commands). These aspects are worth improving independently of the question what types should be supported and how.

@Jaykul
Copy link
Contributor

Jaykul commented Jul 22, 2023

It seems like this is fairly straightforward and doesn't break anything that's not already broken.

@PowerShell/powershell-committee reviewed this and agree that throwing an error is a better experience than the current prompt that fails with an error implying it should work. To support an improved experience, we should have a RFC.

Was an RFC ever created for this?

@microsoft-github-policy-service microsoft-github-policy-service bot added the Resolution-No Activity Issue has had no activity for 6 months or more label Jan 18, 2024
Copy link
Contributor

This issue has not had any activity in 6 months, if there is no further activity in 7 days, the issue will be closed automatically.

Activity in this case refers only to comments on the issue. If the issue is closed and you are the author, you can re-open the issue using the button below. Please add more information to be considered during retriage. If you are not the author but the issue is impacting you after it has been closed, please submit a new issue with updated details and a link to this issue and the original.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committee-Reviewed PS-Committee has reviewed this and made a decision Issue-Enhancement the issue is more of a feature request than a bug Resolution-No Activity Issue has had no activity for 6 months or more Up-for-Grabs Up-for-grabs issues are not high priorities, and may be opportunities for external contributors WG-Engine core PowerShell engine, interpreter, and runtime
Projects
None yet
Development

No branches or pull requests

7 participants