Skip to content

Make sudo.ps1 support multi-line commands #39

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.

Sign up for GitHub

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

Open
Jaykul wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Make sudo.ps1 support multi-line commands #39

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ef20fe9
Make sudo.ps1 support multi-line commands
Jaykul Feb 9, 2024
2eee42f
Move OS test to be first so we fail on the important things
Jaykul Feb 13, 2024
243d876
Add documentation comments (help)
Jaykul Feb 13, 2024
d19f0fa
Whitespace fix
Jaykul Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 55 additions & 64 deletions scripts/sudo.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,87 +1,78 @@
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.
[CmdletBinding(DefaultParameterSetName = "Script")]
param(
[Parameter(Mandatory, Position = 0, ParameterSetName = "Script")]
[scriptblock]$ScriptBlock,

# open question - Should -NoProfile be used when invoking PowerShell
BEGIN {
[switch]$NoProfile,

[Parameter(Mandatory, Position = 0, ParameterSetName = "Command")]
[string]$Command,

[Parameter(Position = 1, ValueFromRemainingArguments)]
[Alias("Args")]
[PSObject[]]$ArgumentList
)
begin {
if ($__SUDO_TEST -ne $true) {
$SUDOEXE = "sudo.exe"
}
else {
if ($null -eq $SUDOEXE) {
throw "variable SUDOEXE has not been set for testing"
}
$Env:SUDOEXE = "sudo.exe"
Copy link
Copy Markdown
Author

@Jaykul Jaykul Feb 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This seems overly complicated. One must set two variables: SUDOEXE and __SUDO_TEST, to actually be able to use an alternate executable for testing? I'm not sure why this was here...

I would prefer to just have a parameter SudoPath that you can set to whatever you want for testing purposes, but that defaults to just sudo.exe and assumes that sudo.exe is in the PATH.

NOTE: I changed SUDOEXE to an environment variable. I feel like if we're using a mock sudo, you need to be able to set the environment stuff once (i.e. in your pipeline, or in your machine scope), instead of needing to set multiple flags for different wrapper scripts.

I'd rather remove these, but if we want to keep them, should __SUDO_TEST also be an environment variable?

} elseif (!$Env:SUDOEXE) {
throw "Environment variable SUDOEXE has not been set for testing"
}

if ([Environment]::OSVersion.Platform -ne "Win32NT") {
if ($IsLinux -or $IsMacOS) {
Copy link
Copy Markdown

@shailist shailist Feb 12, 2024

Choose a reason for hiding this comment

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

$IsLinux and $IsMacOS are only available since Powershell 7 / Core 6. The previous approach was more portable.

Copy link
Copy Markdown
Author

@Jaykul Jaykul Feb 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The only place where they are not available is Windows PowerShell -- thus, when they are missing, they are false, so the logic works without involving classes or string comparisons.

That is to say: in PowerShell, when a variable doesn't exist, it returns $null and in a boolean context, it's $false

As a bonus, variables can be faked for testing, where that static property cannot. 😉

throw "This script works only on Microsoft Windows"
}

if ($null -eq (Get-Command -Type Application -Name "$SUDOEXE" -ErrorAction Ignore)) {
throw "'$SUDOEXE' cannot be found."
if (!(Get-Command -Type Application -Name $Env:SUDOEXE -ErrorAction Ignore)) {
throw "Env:SUDOEXE is set to '$Env:SUDOEXE' but it cannot be found."
Copy link
Copy Markdown

@shailist shailist Feb 12, 2024

Choose a reason for hiding this comment

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

Suggested change
throw "Env:SUDOEXE is set to '$Env:SUDOEXE' but it cannot be found."
throw "Env:SUDOEXE is set to '$Env:SUDOEXE' but cannot be found."

Copy link
Copy Markdown

@StarNumber12046 StarNumber12046 Feb 12, 2024

Choose a reason for hiding this comment

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

maybe "that cannot be found." or keep "it"

Copy link
Copy Markdown
Author

@Jaykul Jaykul Feb 13, 2024

Choose a reason for hiding this comment

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

Yeah, I think this:
"Env:SUDOEXE is set to 'C:\Windows\Sudo.exe' but it cannot be found"

Is better than:
"Env:SUDOEXE is set to 'C:\Windows\Sudo.exe' but cannot be found"

}

$psProcess = Get-Process -id $PID
if (($null -eq $psProcess) -or ($psProcess.Count -ne 1)) {
throw "Cannot retrieve process for '$PID'"
$thisPowerShell = (Get-Process -Id $PID).MainModule.FileName
Copy link
Copy Markdown
Author

@Jaykul Jaykul Feb 13, 2024

Choose a reason for hiding this comment

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

I want to point out that I dramatically simplified this, just because there isn't really any way this can go wrong. $PID is a constant in PowerShell and can't changed, so this is always going to return the full path to the executable.

if (!$thisPowerShell) {
throw "Cannot determine PowerShell executable path."
}

$thisPowerShell = $psProcess.MainModule.FileName
if ($null -eq $thisPowerShell) {
throw "Cannot determine path to '$psProcess'"
}

function convertToBase64EncodedString([string]$cmdLine) {
$bytes = [System.Text.Encoding]::Unicode.GetBytes($cmdLine)
function ConvertToBase64EncodedString([string]$InputObject) {
$bytes = [System.Text.Encoding]::Unicode.GetBytes($InputObject)
[Convert]::ToBase64String($bytes)
}

$MI = $MyInvocation
}

END {
$cmdArguments = $args

# short-circuit if the user provided a scriptblock, then we will use it and ignore any other arguments
if ($cmdArguments.Count -eq 1 -and $cmdArguments[0] -is [scriptblock]) {
$scriptBlock = $cmdArguments[0]
$encodedCommand = convertToBase64EncodedString -cmdLine ($scriptBlock.ToString())
if (($psversiontable.psversion.major -eq 7) -and ($__SUDO_DEBUG -eq $true)) {
Trace-Command -PSHOST -name param* -Expression { & $SUDOEXE "$thisPowerShell" -e $encodedCommand }
end {
# If the first parameter is the name of an executable, just run that without PowerShell
if ($PSCmdlet.ParameterSetName -eq "Command") {
if (@(Get-Command $Command -ErrorAction Ignore)[0].CommandType -eq "Application") {
# NOTE: this assumes that all the parameters can be just strings
if ($PSBoundParameters.Contains("Debug")) {
Copy link
Copy Markdown

@trackd trackd Apr 7, 2024

Choose a reason for hiding this comment

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

this should be ContainsKey

Trace-Command -PSHost -Name param* -Expression { & $Env:SUDOEXE $Command @ArgumentList }
} else {
& $Env:SUDOEXE $Command $ArgumentList
}
return
} else {
# In this case, we're going to need to _make_ a scriptblock out of $MyInvocation.Statement
# NOT $MyInvocation.Line because there might be more than one line in the statement
# IISReset and Jaykul apologize for the reflection, but we need to support old versions of PowerShell
$Statement = [System.Management.Automation.InvocationInfo].GetMember(
'_scriptPosition',
[System.Reflection.BindingFlags]'NonPublic,Instance'
)[0].GetValue($MyInvocation).Text.
Comment on lines +74 to +77
Copy link
Copy Markdown

@trackd trackd Apr 7, 2024

Choose a reason for hiding this comment

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

the trailing . after Text needs to be removed

# Strip the 'sudo' or 'sudo.ps1` or whatever off the front of the statement
$Statement = $Statement.SubString($MyInvocation.InvocationName.Length).Trim()
$EncodedCommand = ConvertToBase64EncodedString $Statement
}
else {
& $SUDOEXE "$thisPowerShell" -e $encodedCommand
}
return
} else {
$EncodedCommand = ConvertToBase64EncodedString $scriptBlock
}

$cmdLine = $MI.Line
$sudoOffset = $cmdLine.IndexOf($MI.InvocationName)
$cmdLineWithoutScript = $cmdLine.SubString($sudoOffset + 5)
$cmdLineAst = [System.Management.Automation.Language.Parser]::ParseInput($cmdLineWithoutScript, [ref]$null, [ref]$null)
$commandAst = $cmdLineAst.Find({$args[0] -is [System.Management.Automation.Language.CommandAst]}, $false)
$commandName = $commandAst.GetCommandName()
$isApplication = Get-Command -Type Application -Name $commandName -ErrorAction Ignore | Select-Object -First 1
$isCmdletOrScript = Get-Command -Type Cmdlet,ExternalScript -Name $commandName -ErrorAction Ignore | Select-Object -First 1
$switches = @("-NoLogo", "-NonInteractive")
if ($NoProfile) { $switches += "-NoProfile" }

# if the command is a native command, just invoke it
if ($null -ne $isApplication) {
if (($psversiontable.psversion.major -eq 7) -and ($__SUDO_DEBUG -eq $true)) {
trace-command -PSHOST -name param* -Expression { & $SUDOEXE $cmdArguments }
}
else {
& $SUDOEXE $cmdArguments
}
}
elseif ($null -ne $isCmdletOrScript) {
$encodedCommand = convertToBase64EncodedString($cmdLineWithoutScript)
if (($psversiontable.psversion.major -eq 7) -and ($__SUDO_DEBUG -eq $true)) {
trace-command -PSHOST -name param* -Expression { & $SUDOEXE -nologo -e $encodedCommand }
}
else {
& $SUDOEXE $thisPowerShell -nologo -e $encodedCommand
}
}
else {
throw "Cannot find '$commandName'"
if ($PSBoundParameters.Contains("Debug")) {
Copy link
Copy Markdown

@trackd trackd Apr 7, 2024

Choose a reason for hiding this comment

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

this should also be ContainsKey

Trace-Command -PSHost -Name param* -Expression { & $Env:SUDOEXE $ThisPowerShell @switches -EncodedCommand $encodedCommand }
} else {
& $Env:SUDOEXE $ThisPowerShell @switches -EncodedCommand $encodedCommand
}
}