Skip to content

Commit

Permalink
Merge pull request #181 from gerardog/Fix.PowershellReadConsoleOutput
Browse files Browse the repository at this point in the history
Fix elevations problems with Windows PowerShell and Notepad
  • Loading branch information
gerardog authored Oct 1, 2022
2 parents 34be524 + 59af3cc commit 12438bb
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 35 deletions.
25 changes: 16 additions & 9 deletions build/02-test.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,19 @@ $failure=$false

pushd $PSScriptRoot\..

dotnet test .\src\gsudo.sln --logger "trx;LogFileName=$((gi .).FullName)\TestResults.trx" --logger:"console;verbosity=normal" -v quiet -p:WarningLevel=0
if (! $?) { $failure = $true }
if ($failure) { exit 1 } # fail fast
dotnet build .\src\gsudo.sln || $(exit $LASTEXITCODE)

$env:path=(Get-Item .\src\gsudo\bin\net7.0\).FullName+";"+$env:path
$originalPath = $env:path

gsudo -k > $null
$env:path=(Get-Item .\src\gsudo.Tests\bin\Debug\net7.0\).FullName+";" + [String]::Join(";", (($ENV:Path).Split(";") -notlike "*gsudo*" | % {$_ -replace "\\$" }))

gsudo -k
gsudo --debug cache on -p 0 -d 1
$env:nokill=1
gsudo -d --debug --integrity medium dotnet test .\src\gsudo.sln --no-build --logger "trx;LogFileName=$((gi .).FullName)\TestResults.trx" --logger:"console;verbosity=normal" -v quiet -p:WarningLevel=0

if (! $?) { $failure = $true }
if ($failure) { exit 1 } # fail fast

$script = {
$ProgressPreference = "SilentlyContinue";
Expand All @@ -34,15 +40,16 @@ $script = {
Invoke-Pester -Configuration $configuration
}


gsudo --debug cache on -p 0 -d 1
Write-Verbose -verbose "Running PowerShell Tests on Windows PowerShell (v5.x)"
powershell $script -outputformat text
gsudo --integrity medium powershell -noprofile $script -outputformat text
if (! $?) { $failure = $true }

gsudo cache on -p 0 -d 1
Write-Verbose -verbose "Running PowerShell Tests on Pwsh Core (v7.x)"
pwsh $script
gsudo --integrity medium pwsh -noprofile $script
if (! $?) { $failure = $true }

.\src\gsudo\bin\net7.0\gsudo.exe -k
gsudo.exe -k

if ($failure) { exit 1 }
2 changes: 1 addition & 1 deletion src/gsudo.Tests/CmdTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public static void StartCacheSession()
new ProcessStartInfo()
{
FileName = "cmd",
Arguments = $" /c start \"gsudo Service\" \"{gsudoPath}\" --debug cache on --pid 0 --duration 0:1:0 ",
Arguments = $" /c start \"gsudo Service\" \"{gsudoPath}\" \"{gsudoPath}\" --debug cache on --pid 0 --duration 0:1:0 ",
Verb = "RunAs"
}
)?.WaitForExit();
Expand Down
9 changes: 9 additions & 0 deletions src/gsudo.Tests/PowershellTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,5 +105,14 @@ public virtual void PS_EchoDoubleQuotesTest()
;
Assert.AreEqual(0, p.ExitCode);
}


[TestMethod]
public void PS_WriteProgress()
{
var p = new TestProcess($"{PS_FILENAME} {PS_ARGS}\r\n./gsudo Write-Progress -Activity \"Test\"; exit\r\n");
p.WaitForExit();
Assert.IsFalse(p.GetStdOut().Contains("internal error", StringComparison.OrdinalIgnoreCase));
}
}
}
6 changes: 4 additions & 2 deletions src/gsudo/Commands/RunCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace gsudo.Commands
{
public class RunCommand : ICommand
{
public IEnumerable<string> CommandToRun { get; set; }
public IList<string> CommandToRun { get; set; }
private string GetArguments() => GetArgumentsString(CommandToRun, 1);

public async Task<int> Execute()
Expand All @@ -33,7 +33,9 @@ public async Task<int> Execute()
CommandToRun = CommandToRunGenerator.AugmentCommand(CommandToRun.ToArray());

bool isWindowsApp = ProcessFactory.IsWindowsApp(CommandToRun.FirstOrDefault());
var elevationMode = GetElevationMode(isWindowsApp);
var elevationMode = GetElevationMode(isWindowsApp);

CommandToRun = CommandToRunGenerator.FixCommandExceptions(CommandToRun);

if (!isRunningAsDesiredUser)
CommandToRun = CommandToRunGenerator.AddCopyEnvironment(CommandToRun, elevationMode);
Expand Down
11 changes: 3 additions & 8 deletions src/gsudo/Helpers/ArgumentsHelper.cs
Original file line number Diff line number Diff line change
@@ -1,19 +1,14 @@
using gsudo;
using gsudo.Commands;
using gsudo.Native;
using gsudo.Native;
using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;

namespace gsudo.Helpers
{
public static class ArgumentsHelper
{
public static IEnumerable<string> SplitArgs(string args)
{
public static IList<string> SplitArgs(string args)
{
args = args.Trim();
var results = new List<string>();
int pushed = 0;
Expand Down
4 changes: 2 additions & 2 deletions src/gsudo/Helpers/CommandLineParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -219,14 +219,14 @@ private ICommand ParseVerb()
}

if (arg.In("run"))
return new RunCommand() { CommandToRun = args };
return new RunCommand() { CommandToRun = args.ToArray() };

args.AddFirst(arg);

if (arg == "!!" || arg.StartsWith("!", StringComparison.InvariantCulture))
return new BangBangCommand() { Pattern = string.Join(" ", args) };

return new RunCommand() { CommandToRun = args };
return new RunCommand() { CommandToRun = args.ToArray() };
}

#region Posix option matching functions
Expand Down
48 changes: 37 additions & 11 deletions src/gsudo/Helpers/CommandToRunGenerator.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
using gsudo.Native;
using Microsoft.VisualBasic;
using System;
using System.Collections;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Security.AccessControl;
Expand Down Expand Up @@ -119,7 +121,7 @@ Running ./gsudo {command} should elevate the powershell command.
}
}

return DoFixIfIsMicrosoftStoreApp(currentShellExeName, newArgs.ToArray());
return newArgs.ToArray();
}
else if (currentShell == Shell.Yori)
{
Expand Down Expand Up @@ -185,8 +187,7 @@ Running ./gsudo {command} should elevate the powershell command.
if (exename != null && CreateProcessSupportedExtensions.Contains(Path.GetExtension(exename)))
{
args[0] = $"\"{exename}\"";
var newArgs = DoFixIfIsMicrosoftStoreApp(exename, args);
return newArgs.ToArray();
return args;
}
else
{
Expand All @@ -200,25 +201,50 @@ Running ./gsudo {command} should elevate the powershell command.
}
}

private static string[] DoFixIfIsMicrosoftStoreApp(string targetExe, string[] args)
internal static IList<string> FixCommandExceptions(IList<string> args)
{
// -- Workaround for https://github.com/gerardog/gsudo/issues/65

// ISSUE: Apps installed via Microsoft Store, need a special attribute in it's security token to work (WIN://SYSAPPID),
string targetFullPath = args.First().UnQuote();
string targetFileName = Path.GetFileName(targetFullPath);

var ExceptionDict = Settings.ExceptionList.Value
.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries)
.Select(x => x.Split(new string[] { ":=" }, StringSplitOptions.None))
.ToDictionary(x => x.First(), x => x.Skip(1).FirstOrDefault(), StringComparer.OrdinalIgnoreCase);

if (
// ISSUE 1:
// -- https://github.com/gerardog/gsudo/issues/65
// Apps installed via Microsoft Store, need a special attribute in it's security token to work (WIN://SYSAPPID),
// That attrib is inserted by CreateProcess() Api, but gsudo replaces the special token with regular but elevated one
// which doesnt have the attribute. So the app fails to load.

// WORKAROUND: The CreateProcess(pwsh.exe) call must be already elevated so that Api can manipulate the final token,
// and the easiest way I found is delegate the final CreateProcess to an elevated CMD instance: To elevate "cmd /c pwsh.exe" instead.

if (targetExe.IndexOf("\\WindowsApps\\", StringComparison.OrdinalIgnoreCase) >= 0) // Terrible but cheap Microsoft Store App detection.
targetFullPath.IndexOf("\\WindowsApps\\", StringComparison.OrdinalIgnoreCase) >= 0) // Terrible but cheap Microsoft Store App detection.
{
Logger.Instance.Log("Applying workaround for target app installed via MSStore.", LogLevel.Debug);

return new string[] {
Environment.GetEnvironmentVariable("COMSPEC"),
"/s /c" ,
$"\"{string.Join(" ", args)}\""};
}
else if (ExceptionDict.ContainsKey(targetFileName))
{
// ISSUE 2: https://github.com/gerardog/gsudo/issues/131
// notepad won't open notepad on CMD / Win11:
// It appears that notepad opens a Microsoft Store version of Notepad.exe. It fails to load using sudo.
// ISSUE 3: https://github.com/gerardog/gsudo/issues/180
// Strange console "Access Denied" error while

string action = ExceptionDict[targetFileName];

if (string.IsNullOrEmpty(action))
action = $"\"{Environment.GetEnvironmentVariable("COMSPEC")}\" /s /c \"{{0}}\"";

Logger.Instance.Log($"Found {targetFileName} in Exception List with Action=\"{action}\".", LogLevel.Debug);

return ArgumentsHelper.SplitArgs(String.Format(CultureInfo.InvariantCulture, action, string.Join(" ", args))).ToList();
}
else
return args;
// -- End of workaround.
Expand All @@ -230,7 +256,7 @@ private static string[] DoFixIfIsMicrosoftStoreApp(string targetExe, string[] ar
/// </summary>
/// <remarks>CopyNetworkShares is *the best I could do*. Too much verbose, asks for passwords, etc. Far from ideal.</remarks>
/// <returns>a modified args list</returns>
internal static IEnumerable<string> AddCopyEnvironment(IEnumerable<string> args, ElevationRequest.ConsoleMode mode)
internal static IList<string> AddCopyEnvironment(IList<string> args, ElevationRequest.ConsoleMode mode)
{
if (Settings.CopyEnvironmentVariables || Settings.CopyNetworkShares)
{
Expand Down
9 changes: 8 additions & 1 deletion src/gsudo/Settings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ class Settings
new RegistrySetting<bool>(nameof(SecurityEnforceUacIsolation),
defaultValue: false,
deserializer: bool.Parse,
scope: RegistrySettingScope.GlobalOnly);

public static RegistrySetting<string> ExceptionList { get; } =
new RegistrySetting<string>(nameof(ExceptionList),
defaultValue: "notepad.exe;powershell.exe;",
deserializer: (string s)=>s,
scope: RegistrySettingScope.GlobalOnly);

public static IDictionary<string, RegistrySetting> AllKeys =>
Expand All @@ -92,7 +98,8 @@ class Settings
CopyEnvironmentVariables,
CopyNetworkShares,
PowerShellLoadProfile,
SecurityEnforceUacIsolation);
SecurityEnforceUacIsolation,
ExceptionList);

internal static TimeSpan TimeSpanParseWithInfinite(string value)
{
Expand Down
2 changes: 1 addition & 1 deletion src/gsudo/gsudo.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
<PackageReference Include="System.Security.Principal.Windows" Version="4.6.0" />
</ItemGroup>
<Target Name="PreBuild" BeforeTargets="PreBuildEvent">
<Exec Command="echo Killing running gsudo commands.&#xD;&#xA;gsudo -k 2&gt;nul&#xD;&#xA;%25windir%25\sysnative\tskill gsudo&#xD;&#xA;exit 0" />
<Exec Command="IF NOT DEFINED nokill echo Killing running gsudo commands.&#xD;&#xA;IF NOT DEFINED nokill gsudo -k 2&gt;nul&#xD;&#xA;IF NOT DEFINED nokill %25windir%25\sysnative\tskill gsudo&#xD;&#xA;exit 0" />
</Target>
<Target Name="PostBuildMacros">
<GetAssemblyIdentity AssemblyFiles="$(TargetPath)">
Expand Down

0 comments on commit 12438bb

Please sign in to comment.