From f204b8d93554863288c6aa69b76c4eea9a856fb1 Mon Sep 17 00:00:00 2001 From: Ethan Dennis Date: Thu, 2 Jun 2022 18:10:10 +0000 Subject: [PATCH] Add `--password-stdin` option to securely provide creds (#41) * Add password-stdin to pipe access token * Lint * Update var naming --- .../Services/DockerServiceTests.cs | 34 +++++++++----- src/Valet/App.cs | 5 ++- src/Valet/Commands/Update.cs | 9 +++- src/Valet/Interfaces/IDockerService.cs | 2 +- src/Valet/Interfaces/IProcessService.cs | 3 +- src/Valet/Services/DockerService.cs | 12 +++-- src/Valet/Services/ProcessService.cs | 44 +++++++++---------- 7 files changed, 65 insertions(+), 44 deletions(-) diff --git a/src/Valet.UnitTests/Services/DockerServiceTests.cs b/src/Valet.UnitTests/Services/DockerServiceTests.cs index f9830f5..10550e7 100644 --- a/src/Valet.UnitTests/Services/DockerServiceTests.cs +++ b/src/Valet.UnitTests/Services/DockerServiceTests.cs @@ -38,7 +38,8 @@ public async Task UpdateImageAsync_NoCredentialsProvided_PullsLatest_ReturnsTrue $"pull {server}/{image}:{version}", It.IsAny(), It.IsAny?>(), - It.IsAny() + It.IsAny(), + null ) ).Returns(Task.CompletedTask); @@ -68,10 +69,11 @@ public async Task UpdateImageAsync_InvalidCredentialsProvided_ReturnsFalse() _processService.Setup(handler => handler.RunAsync( "docker", - $"login {server} --password {password} --username {username}", + $"login {server} --username {username} --password-stdin", It.IsAny(), It.IsAny?>(), - It.IsAny() + It.IsAny(), + password ) ).Returns(Task.CompletedTask); @@ -101,10 +103,11 @@ public async Task UpdateImageAsync_ValidCredentialsProvided_ReturnsTrue() _processService.Setup(handler => handler.RunAsync( "docker", - $"login {server} --password {password} --username {username}", + $"login {server} --username {username} --password-stdin", It.IsAny(), It.IsAny?>(), - It.IsAny() + It.IsAny(), + password ) ).Returns(Task.CompletedTask); @@ -114,7 +117,8 @@ public async Task UpdateImageAsync_ValidCredentialsProvided_ReturnsTrue() $"pull {server}/{image}:{version}", It.IsAny(), It.IsAny?>(), - It.IsAny() + It.IsAny(), + null ) ).Returns(Task.CompletedTask); @@ -145,7 +149,8 @@ public async Task ExecuteCommandAsync_InvokesDocker_ReturnsTrue() $"run --rm -t -v \"{Directory.GetCurrentDirectory()}\":/data {server}/{image}:{version} {string.Join(' ', arguments)}", Directory.GetCurrentDirectory(), new[] { new System.ValueTuple("MSYS_NO_PATHCONV", "1") }, - true + true, + null ) ).Returns(Task.CompletedTask); @@ -175,7 +180,8 @@ public async Task ExecuteCommandAsync_InvokesDocker_WithEnvironmentVariables_Ret $"run --rm -t --env GITHUB_ACCESS_TOKEN=foo --env GITHUB_INSTANCE_URL=https://github.fabrikam.com --env JENKINS_ACCESS_TOKEN=bar -v \"{Directory.GetCurrentDirectory()}\":/data {server}/{image}:{version} {string.Join(' ', arguments)}", Directory.GetCurrentDirectory(), new[] { new System.ValueTuple("MSYS_NO_PATHCONV", "1") }, - true + true, + null ) ).Returns(Task.CompletedTask); @@ -196,7 +202,8 @@ public void VerifyDockerRunningAsync_IsRunning_NoException() "info", It.IsAny(), It.IsAny?>(), - It.IsAny() + It.IsAny(), + null ) ).Returns(Task.CompletedTask); @@ -214,7 +221,8 @@ public void VerifyDockerRunningAsync_NotRunning_ThrowsException() "info", It.IsAny(), It.IsAny?>(), - It.IsAny() + It.IsAny(), + null ) ).ThrowsAsync(new Exception()); @@ -236,7 +244,8 @@ public void VerifyImagePresentAsync_IsPresent_NoException() $"image inspect {server}/{image}:{version}", It.IsAny(), It.IsAny?>(), - It.IsAny() + It.IsAny(), + null ) ).Returns(Task.CompletedTask); @@ -259,7 +268,8 @@ public void VerifyImagePresentAsync_NotPresent_ThrowsException() $"image inspect {server}/{image}:{version}", It.IsAny(), It.IsAny?>(), - It.IsAny() + It.IsAny(), + null ) ).ThrowsAsync(new Exception()); diff --git a/src/Valet/App.cs b/src/Valet/App.cs index 951d0b3..a288c02 100644 --- a/src/Valet/App.cs +++ b/src/Valet/App.cs @@ -14,7 +14,7 @@ public App(IDockerService dockerService) _dockerService = dockerService; } - public async Task UpdateValetAsync(string? username = null, string? password = null) + public async Task UpdateValetAsync(string? username = null, string? password = null, bool passwordStdin = false) { await _dockerService.VerifyDockerRunningAsync().ConfigureAwait(false); @@ -26,7 +26,8 @@ await _dockerService.UpdateImageAsync( ValetContainerRegistry, "latest", username, - password + password, + passwordStdin ); return 0; diff --git a/src/Valet/Commands/Update.cs b/src/Valet/Commands/Update.cs index 92d1809..4d968a3 100644 --- a/src/Valet/Commands/Update.cs +++ b/src/Valet/Commands/Update.cs @@ -21,14 +21,21 @@ public class Update : BaseCommand IsRequired = false, }; + private static readonly Option PasswordStdInOption = new(new[] { "--password-stdin" }) + { + Description = "Access token from standard input to authenticate with GHCR (requires read:packages scope).", + IsRequired = false, + }; + protected override Command GenerateCommand(App app) { var command = base.GenerateCommand(app); command.AddOption(UsernameOption); command.AddOption(PasswordOption); + command.AddOption(PasswordStdInOption); - command.Handler = CommandHandler.Create((string? username, string? password) => app.UpdateValetAsync(username, password)); + command.Handler = CommandHandler.Create((string? username, string? password, bool passwordStdin) => app.UpdateValetAsync(username, password, passwordStdin)); return command; } diff --git a/src/Valet/Interfaces/IDockerService.cs b/src/Valet/Interfaces/IDockerService.cs index aa81302..23e953c 100644 --- a/src/Valet/Interfaces/IDockerService.cs +++ b/src/Valet/Interfaces/IDockerService.cs @@ -2,7 +2,7 @@ namespace Valet.Interfaces; public interface IDockerService { - Task UpdateImageAsync(string image, string server, string version, string? username, string? password); + Task UpdateImageAsync(string image, string server, string version, string? username, string? password, bool passwordStdin = false); Task ExecuteCommandAsync(string image, string server, string version, params string[] arguments); diff --git a/src/Valet/Interfaces/IProcessService.cs b/src/Valet/Interfaces/IProcessService.cs index 6cbe977..b7f2a8f 100644 --- a/src/Valet/Interfaces/IProcessService.cs +++ b/src/Valet/Interfaces/IProcessService.cs @@ -7,6 +7,7 @@ Task RunAsync( string arguments, string? cwd = null, IEnumerable<(string, string)>? environmentVariables = null, - bool output = true + bool output = true, + string? inputForStdIn = null ); } \ No newline at end of file diff --git a/src/Valet/Services/DockerService.cs b/src/Valet/Services/DockerService.cs index 8c7c2dc..28b6964 100644 --- a/src/Valet/Services/DockerService.cs +++ b/src/Valet/Services/DockerService.cs @@ -23,18 +23,24 @@ public DockerService(IProcessService processService) _processService = processService; } - public async Task UpdateImageAsync(string image, string server, string version, string? username, string? password) + public async Task UpdateImageAsync(string image, string server, string version, string? username, string? password, bool passwordStdin = false) { + if (passwordStdin && Console.IsInputRedirected) + { + password = await Console.In.ReadToEndAsync(); + } + if (!string.IsNullOrWhiteSpace(username) && !string.IsNullOrWhiteSpace(password)) { await _processService.RunAsync( "docker", - $"login {server} --password {password} --username {username}" + $"login {server} --username {username} --password-stdin", + inputForStdIn: password ).ConfigureAwait(false); } else { - Console.WriteLine("No GHCR credentials provided."); + Console.WriteLine("INFO: using cached credentials because no GHCR credentials were provided."); } await _processService.RunAsync( diff --git a/src/Valet/Services/ProcessService.cs b/src/Valet/Services/ProcessService.cs index d7b0f57..d010999 100644 --- a/src/Valet/Services/ProcessService.cs +++ b/src/Valet/Services/ProcessService.cs @@ -5,14 +5,14 @@ namespace Valet.Services; public class ProcessService : IProcessService { - public Task RunAsync( + public async Task RunAsync( string filename, string arguments, string? cwd = null, IEnumerable<(string, string)>? environmentVariables = null, - bool output = true) + bool output = true, + string? inputForStdIn = null) { - var tcs = new TaskCompletionSource(); var cts = new CancellationTokenSource(); var startInfo = new ProcessStartInfo @@ -21,6 +21,7 @@ public Task RunAsync( Arguments = arguments, RedirectStandardOutput = true, RedirectStandardError = true, + RedirectStandardInput = true, UseShellExecute = false, WorkingDirectory = cwd, CreateNoWindow = true @@ -34,37 +35,32 @@ public Task RunAsync( } } - var process = new Process + using var process = new Process { StartInfo = startInfo, - EnableRaisingEvents = true, + EnableRaisingEvents = true }; + process.Start(); - void OnProcessExited(object? sender, EventArgs args) + if (!string.IsNullOrWhiteSpace(inputForStdIn)) { - process.Exited -= OnProcessExited; - - cts.Cancel(); - if (process.ExitCode == 0) - { - tcs.TrySetResult(true); - } - else - { - var error = process.StandardError.ReadToEnd(); - tcs.TrySetException(new Exception(error)); - } - - process.Dispose(); + var writer = process.StandardInput; + writer.AutoFlush = true; + await writer.WriteAsync(inputForStdIn); + writer.Close(); } - process.Exited += OnProcessExited; - process.Start(); - ReadStream(process.StandardOutput, output, cts.Token); ReadStream(process.StandardError, output, cts.Token); - return tcs.Task; + await process.WaitForExitAsync(cts.Token); + + cts.Cancel(); + if (process.ExitCode != 0) + { + var error = await process.StandardError.ReadToEndAsync(); + throw new Exception(error); + } } private void ReadStream(StreamReader reader, bool output, CancellationToken ctx)