PowerShell 7.3 doesn't pass variable values / cmdlet results arguments to child processes when comma is present #18502
Comments
conioh
added
the
Needs-Triage
|
Wow - that is very unfortunate. The workaround for now is to double-quote the argument ( The problem isn't Windows-specific; Unix repro: Note that there's an older, similar bug: |
|
@mklement0, thanks. This is very helpful. I'm sure there are lots of other bugs here (I know some of them up close |
$ $PSNativeCommandArgumentPassing = 'Legacy'
$ /bin/echo a,$PShome
a,/opt/microsoft/powershell/7But: $ /bin/echo -foo=$PShome
-foo=$PShome |
|
There's specialized code added for this new behavior (fixing a different issue) which treats arguments with commas slightly different than before. Variables are no longer expanded when they appear with commas. However, the behavior you want can be easily achieved by creating an expandable string ala |
|
@JamesWTruher, the workaround has already been mentioned. What this issue is about is a (previously) unacknowledged-as-such breaking change, for which there appears to be no good (conceptual) reason - "There's specialized code added for this new behavior" certainly doesn't qualify. Yes, even without the bug at hand compound string tokens behave in inconsistent, but at least predictable ways:
Therefore, even though it is generally advisable to enclose compound tokens that are to form a single argument in |
Great. Then it should be easy to remove this specialized code. Please do that.
Neither the blog post nor the documentation mention any of this. PS C:\> $PSNativeCommandArgumentPassing = "Legacy"
PS C:\> cmd /c echo foo,cd=$(Write-Host "completely insane"; "return")
completely insane
foo,cd=return
PS C:\> $PSNativeCommandArgumentPassing = "Windows"
PS C:\> cmd /c echo foo,cd=$(Write-Host "completely insane"; "return")
completely insane
foo,cd=$(Write-Host"completelyinsane";"return")
PS C:\>Not executing the cmdlet/script block if you're not going to use its return value isn't the bare minimum, it's way below the minimum. |
|
this was an explicit breaking change as part of the native argument passing changes. this behavior is only when commas are used in the open. |
|
@conioh a friendly reminder of our Code of Conduct. Let's keep the discussion civil and professional. We understand your frustration here, but we need to think through both the short term and long term effects. |
@JamesWTruher, may we know
|
@SteveL-MSFT, thank you for the reminder!
Please consider the long-term effects of ¹seemingly unjustified ²undocumented, ³breaking changes for which ⁴no explanation but "trust me, we had a reason" is given on trust towards you, PowerShell and the company you work for. |
|
@conioh I think it's fair feedback, that members of the team should provide more context and reasoning on changes (or at least link to other issues where it was discussed) |
There is no justification for changing the expansion behavior in the context of
That applies to arguments passed to PowerShell commands, but has never applied to external (native) programs (which have no notion of arrays) - and that shouldn't change. (Aside from that, this aspect is incidental to the discussion of whether expansion should occur.) In the past, passing something like |
|
On the meta issue, @SteveL-MSFT
That is encouraging to hear. 🤞for the future. As for the present: Your comment came not only after the initial, bewilderingly cavalier team-member response, but after a follow-up response that also failed to provide a reasoning (while making the spurious claim that "the new behavior requires that you use an expandable string"). I do hope that a proper response is forthcoming, one that addresses the specific arguments made above, namely that this is bug that, even if the change were intentional, would have no conceptual justification, and is therefore an unacceptable breaking change. |
|
@237dmitry, as for |
@JamesWTruher, I ask again, after more than two weeks, may we know
CC @SteveL-MSFT |
@JamesWTruher, I ask again, after a month, may we know
CC @SteveL-MSFT Are these questions difficult to answer or is the information classified? Is it TOP SECRET and available only under a special access program? #professionalism_and_all_that_jazz |
|
@conioh team members are on vacation currently (as am I). Mondays are days the team specifically focuses on community issues and PRs (although given the number of them, you cannot expect priority attention here). In general, being a bit nicer will make people (not just team members, but community members) more likely to want to respond. |
Of course they are currently on vacation, but were they the entire past month? I find it unlikely, but even if they were on vacation for a month, it's reasonable to expect from their employer and manager to assign someone else to fill in. At least that's what happened when my colleagues went on long vacations. Don't know how they do it over there at Microsoft.
Given that this is basically a security vulnerability I can certainly expect priority here and I indeed expect it.
Perhaps. On the other hand between the two of us I'm the one who got an unsolicited upvote (I'm sure you can call all you buddies to give you a hundred upvotes but that won't change or prove anything) and you're the one who raised the issue of professionalism. Could have avoided that, but now that you have I think it's a great issue to explore and I'm just providing another perspective on it. "In general, being a bit more competent and responsible will make people (not just users, but the open source community and the general public) more likely to want to respect you." |
Can you expand on that, give an example scenario? (e.g. What security boundary is crossed, what could an attacker do now which they couldn't do before?). |
|
@HumanEquivalentUnit, we don't need to go down that route, given the following:
|
|
@mklement0 conioh's 'provocative style' and your focus on the 'good conceptual reason' had obscured the important part for me. In case that is similar for anyone else... @JamesWTruher with the new behaviour, if you want to pass the text literally then subexpressions ought not to be evaluated, but they are: PS C:\> cmd /c echo a,x=$(get-item c:\imaginary)
Get-Item: Cannot find path 'C:\imaginary' because it does not exist.
a,x=$(get-itemc:\imaginary)That get-item should not be executed in this code to trigger the exception, is that right? |
|
@HumanEquivalentUnit, that is the wrong thing to focus on. The primary focus should be:
At least that has been my focus throughout this thread. Honestly, you can't fault @conioh for getting frustrated with the team's responses here, primarily by @JamesWTruher (part of a recurring pattern, such as this response, for example), and secondarily - through appeasement and silence - by @SteveL-MSFT. |
|
@mklement0 your comment of April 2021 cited "Fiat decisions handed down with little to no explanation [...] followed by unwillingness to explain or debate the decision." - has that changed, are you honestly expecting a satisfying reason to appear? I suspect the subexpressions still being evaluated has not been noticed; it is an unexpected and potentially dangerous behaviour which ought to be addressed even if the change is staying and we never get an explanation (I think both are likely). Letting it be lost in the noise of comments like "(I'm sure you can call all you buddies to give you a hundred upvotes but that won't change or prove anything)" making the PS Team roll their eyes and stop reading would be a shame. |
If enough people push back long enough, hopefully, yes. I am not condoning abrasive comments, and you're right that they make the arguments less likely to be heard; I was just pointing out that it is not hard to see what triggered them. Yes, a resolution is necessary here, but, given that we're dealing with what is to me an obvious bug that is an accidental fallout from an implementation change, I think it's important to stay focused on that, which I've tried to.
It's been discussed repeatedly and explicitly here, and @conioh even created a separate issue focused on this: #18668 That, along with your comments here, should leave no doubt that we're not only dealing with a breaking change, but also potentially dangerous side effects. |
|
Let's have the Engine WG discuss and make a recommendation |
|
This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you. |
1 similar comment
|
This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you. |
microsoft-github-policy-service
bot
added
Resolution-No Activity
|
This issue has not had any activity in 6 months, if this is a bug please try to reproduce on the latest version of PowerShell and reopen a new issue and reference this issue if this is still a blocker for you. |
|
This issue has been marked as "No Activity" as there has been no activity for 6 months. It has been closed for housekeeping purposes. |
Prerequisites
Steps to reproduce
When launching an executable from PowerShell, with
$PSNativeCommandArgumentPassing <> "Legacy"if the arguments contain a comma, the variables/cmdlets are evaluated, but the results of the evaluation don't go to the child process.We can see that the cmdlet is executed (or attempted):
Expected behavior
Actual behavior
Error details
No response
Environment data
Visuals
No response
The text was updated successfully, but these errors were encountered: