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

[Core] Add az.ps1 entry script for PowerShell #23514

Merged
merged 2 commits into from
Aug 19, 2022
Merged

[Core] Add az.ps1 entry script for PowerShell #23514

merged 2 commits into from
Aug 19, 2022

Conversation

jiasli
Copy link
Member

@jiasli jiasli commented Aug 11, 2022

Context

Fix #20972

If an argument contains special characters like | and &, even though the character is double quoted, it still gets executed. This is because PowerShell launches cmd.exe to execute az.cmd script. Double quotes are parsed by PowerShell so not passed to cmd.exe during this process. cmd.exe sees the raw | and & and executes them.

However, when az.cmd is invoked from cmd.exe (Command Prompt), it is launched in-process, and double quotes are preserved, leading to inconsistent behavior between cmd.exe and PowerShell.

Change

This PR adds az.ps1 entry script for PowerShell, so that PowerShell can natively execute az.ps1 and no longer launches cmd.exe to execute az.cmd.

Since PowerShell 7.3, double quotes are now preserved correctly when calling a native executable (PowerShell/PowerShell#1995 (comment)), thus fixing #15529.

Testing Guide

az --debug '"ab"'
cli.knack.cli: Command arguments: ['--debug', '"ab"']

az --debug "a|b"
cli.knack.cli: Command arguments: ['--debug', 'a|b']

az --debug "a&b"
cli.knack.cli: Command arguments: ['--debug', 'a&b']

@@ -0,0 +1,2 @@
$env:AZ_INSTALLER="MSI"
& "$PSScriptRoot\..\python.exe" -IBm azure.cli $args
Copy link

@SteveL-MSFT SteveL-MSFT Aug 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work correctly with WinPS 5.1, right? Unfortunately the case of az '{"test":1}' by the time the $args is processed, you would get {test:1} on any PS older than 7.3 (which makes it no longer valid JSON), and there's no way to know what the original intent. So perhaps the best thing is to iterate through $args and if | or & is found, throw an error that it would only work with 7.3+?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in Windows PowerShell 5.1, and indeed double quotes are lost, but luckily | or & isn't causing any problems to us. :)

> az2 --debug "a|b" "a&b" '"ab"'
cli.knack.cli: Command arguments: ['--debug', 'a|b', 'a&b', 'ab']

@yonzhan
Copy link
Collaborator

yonzhan commented Aug 11, 2022

add a ps1 file as the entry script to bypass 'PowerShell -> cmd' issue.

@yonzhan yonzhan added this to the Aug 2022 (2022-09-06) milestone Aug 11, 2022
@jiasli
Copy link
Member Author

jiasli commented Aug 12, 2022

By the way, I am playing around by calling cmd.exe (https://ss64.com/nt/cmd.html) myself from Command Prompt and it looks like it has some weird behavior that I need to add extra double quotes (denoted by ^):

According to https://ss64.com/nt/syntax-esc.html

To launch a batch script with spaces in the script Path and other parameters, all requiring quotes:

CMD /k ""c:\batch files\test.cmd" "Parameter 1 with space" "Parameter2 with space""
       ^                                                                          ^

My testing:

D:\test.cmd contains

@echo %*
D:\>cmd /c "D:\test.cmd" "parameter1 parameter2"
'D:\test.cmd" "parameter1' is not recognized as an internal or external command,
operable program or batch file.

D:\>cmd /c ""D:\test.cmd" "parameter1 parameter2""
"parameter1 parameter2"

While in Bash

/home/user2/test.sh contains

echo "$@"
user2@DESKTOP-A79F1:~$ bash "/home/user2/test.sh" "parameter1 parameter2"
parameter1 parameter2
user2@DESKTOP-A79F1:~$ bash ""/home/user2/test.sh" "parameter1 parameter2""
bash: /home/user2/test.sh parameter1: No such file or directory

No weird outer double quotes are needed.

@jiasli jiasli marked this pull request as ready for review August 16, 2022 08:15
@jiasli jiasli changed the title [Misc.] Add az.ps1 entry script for PowerShell [Core] Add az.ps1 entry script for PowerShell Aug 19, 2022
@jiasli jiasli merged commit a55f924 into Azure:dev Aug 19, 2022
@jiasli jiasli deleted the ps1 branch August 19, 2022 06:42
@jiasli
Copy link
Member Author

jiasli commented Aug 20, 2022

I also found another weird PowerShell behavior while writing this PR - PowerShell sometimes even depends on the space location to determine the behavior:

> python -c "import sys; print(sys.argv)" '\"a b\"c'
['-c', '"a', 'b"c']
> python -c "import sys; print(sys.argv)" '\"ab\" c'
['-c', '"ab" c']

Also consider these commands:

> python -c "import sys; print(sys.argv)" '""a b""'
['-c', '"a b"']
> python -c "import sys; print(sys.argv)" '\"a b\"'
['-c', '"a', 'b"']

They should be grammatically equivalent, as in Win32 API within double quotes, "" and \" both mean literal double quote sign. However, they are not, because in the second example, PowerShell somehow recognizes double quotes and thinks space is not “on the root level of the string”, so doesn’t surround the argument with double quotes when passing it to python.exe, making the argument split.

> python -c "import sys; print(sys.argv)" '\"a b\" '
['-c', '"a b" ']

In the above example, I put an extra space “on the root level” and PowerShell passed the arguments double-quoted-ly to python.exe.

Comment on lines +1 to +14
$env:AZ_INSTALLER="PIP"

if (Test-Path "$PSScriptRoot\python.exe") {
# Perfer python.exe in venv
& "$PSScriptRoot\python.exe" -m azure.cli $args
}
else {
# Run system python.exe
python.exe -m azure.cli $args
}

# SIG # Begin signature block
# MIInqgYJKoZIhvcNAQcCoIInmzCCJ5cCAQExDzANBglghkgBZQMEAgEFADB5Bgor
# BgEEAYI3AgEEoGswaTA0BgorBgEEAYI3AgEeMCYCAwEAAAQQH8w7YFlLCE63JNLG
Copy link
Member Author

@jiasli jiasli Aug 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one problem with az.ps1 for pip.

  • ESRP code-sign was executed on Azure DevOps's Windows agent. When git checks out the source code on Windows, the end of line sequence is CRLF (\r\n). This is the default of Windows git:
    image
    so the signature was generated for the CRLF version of az.ps1.
  • However, wheels are built on Linux agent, converting az.ps1's end of line sequence to LF (\n).
  • Even when installing with pip -e, pip converts CRLF to LF (for simplicity I guess).

That is, on Windows,

  • az.ps1 in source code contains CRLF
  • az.ps1 installed in Scripts folder contains LF

Since the signature was generated for the CRLF version of az.ps1, but the installed az.ps1 is LF version, the signature doesn't match the file content.

Even if we ESRP code sign the LF version, az.ps1 will be checked out as CRLF in the source code, still making the signature mismatch. That is, the signature will never match the file content of both az.ps1s - one az.ps1's signature must be out of place.

@a3code
Copy link

a3code commented Oct 4, 2022

Hello everyone, could please explain how I should run scripts right now
I am a little bit confused, on azure DevOps I have a script that had run az in the next way:
Start-Process "az" -ArgumentList $azArgs -NoNewWindow -Wait
azArgs is a commend to set some settings that is.
and currently I see an error :
Start-Process : This command cannot be run due to the error: %1 is not a valid Win32 application
how do I need to change it to make it work after this fix?
Should I just remove Start-Process ?

@yonzhan
Copy link
Collaborator

yonzhan commented Oct 5, 2022

We will revert this script and roll out on 10.12. Everything will work normally by then.

@ishepherd
Copy link

@jiasli @yonzhan Will you be adding test coverage for all the many issues people are reporting? Looks like a nice opportunity to fill coverage gaps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue to set password for secret at KeyVauklt by azure CLI
7 participants