From 1a0d588d3a5ee126dc9f18f2bcbdbf69441f41d4 Mon Sep 17 00:00:00 2001 From: Sven Pfleiderer Date: Fri, 4 Feb 2022 13:46:30 -0800 Subject: [PATCH] Add support for Step Summary (#1642) * First prototype of step summary environment variable * Fix file contention issue * Try to simplify cleaning up file references * use step id as md file name, queue file attachment * separate logic into attachment summary func * Fix indentation * Add (experimental) feature flag support * reorganize summary upload determination logic * file i/o exception handling + pr feedback * Revert changes for now to reintroduce them later * Add skeleton SetStepSummaryCommand * Update step summary feature flag name * Port ShouldUploadAttachment from previous iteration * Port QueueStepSummaryUpload from previous iteration * Improve exception handling when uploading attachment * Add some minor logging improvements * Refuse to upload files larger than 128k * Implement secrets scrubbing * Add TODO comment to remove debugging temp files * Add first tests * Add test for secret masking * Add some naming/style fixes suggested in feedback * inline check for feature flag * Inline method for style consistency * Make sure that scrubbed file doesn't exist before creating it * Rename SetStepSummaryCommand to CreateStepSummaryCommand * Fix error handling messages * Fix file command name when registering extension * Remove unnecessary file deletion Co-authored-by: Rob Herley --- src/Runner.Common/ExtensionManager.cs | 1 + src/Runner.Worker/FileCommandManager.cs | 67 +++++ src/Runner.Worker/GitHubContext.cs | 1 + src/Sdk/DTWebApi/WebApi/TaskAttachment.cs | 6 + .../L0/Worker/CreateStepSummaryCommandL0.cs | 241 ++++++++++++++++++ 5 files changed, 316 insertions(+) create mode 100644 src/Test/L0/Worker/CreateStepSummaryCommandL0.cs diff --git a/src/Runner.Common/ExtensionManager.cs b/src/Runner.Common/ExtensionManager.cs index a432d6d39..29f99b628 100644 --- a/src/Runner.Common/ExtensionManager.cs +++ b/src/Runner.Common/ExtensionManager.cs @@ -60,6 +60,7 @@ namespace GitHub.Runner.Common case "GitHub.Runner.Worker.IFileCommandExtension": Add(extensions, "GitHub.Runner.Worker.AddPathFileCommand, Runner.Worker"); Add(extensions, "GitHub.Runner.Worker.SetEnvFileCommand, Runner.Worker"); + Add(extensions, "GitHub.Runner.Worker.CreateStepSummaryCommand, Runner.Worker"); break; case "GitHub.Runner.Listener.Check.ICheckExtension": Add(extensions, "GitHub.Runner.Listener.Check.InternetCheck, Runner.Listener"); diff --git a/src/Runner.Worker/FileCommandManager.cs b/src/Runner.Worker/FileCommandManager.cs index aea76f10a..f45528a5c 100644 --- a/src/Runner.Worker/FileCommandManager.cs +++ b/src/Runner.Worker/FileCommandManager.cs @@ -259,4 +259,71 @@ namespace GitHub.Runner.Worker return text.Substring(originalIndex, lfIndex - originalIndex); } } + + public sealed class CreateStepSummaryCommand : RunnerService, IFileCommandExtension + { + private const int _attachmentSizeLimit = 128 * 1024; + + public string ContextName => "step_summary"; + public string FilePrefix => "step_summary_"; + + public Type ExtensionType => typeof(IFileCommandExtension); + + public void ProcessCommand(IExecutionContext context, string filePath, ContainerInfo container) + { + if (!context.Global.Variables.GetBoolean("DistributedTask.UploadStepSummary") ?? true) + { + Trace.Info("Step Summary is disabled; skipping attachment upload"); + return; + } + + if (String.IsNullOrEmpty(filePath) || !File.Exists(filePath)) + { + Trace.Info($"Step Summary file ({filePath}) does not exist; skipping attachment upload"); + return; + } + + try + { + var fileSize = new FileInfo(filePath).Length; + if (fileSize == 0) + { + Trace.Info($"Step Summary file ({filePath}) is empty; skipping attachment upload"); + return; + } + + if (fileSize > _attachmentSizeLimit) + { + context.Error($"$GITHUB_STEP_SUMMARY supports content up a size of {_attachmentSizeLimit / 1024}k got {fileSize / 1024}k"); + Trace.Info($"Step Summary file ({filePath}) is too large ({fileSize} bytes); skipping attachment upload"); + + return; + } + + Trace.Verbose($"Step Summary file exists: {filePath} and has a file size of {fileSize} bytes"); + var scrubbedFilePath = filePath + "-scrubbed"; + + using (var streamReader = new StreamReader(filePath)) + using (var streamWriter = new StreamWriter(scrubbedFilePath)) + { + string line; + while ((line = streamReader.ReadLine()) != null) + { + var maskedLine = HostContext.SecretMasker.MaskSecrets(line); + streamWriter.WriteLine(maskedLine); + } + } + + var attachmentName = context.Id.ToString(); + + Trace.Info($"Queueing file ({filePath}) for attachment upload ({attachmentName})"); + context.QueueAttachFile(ChecksAttachmentType.StepSummary, attachmentName, scrubbedFilePath); + } + catch (Exception e) + { + Trace.Error($"Error while processing file ({filePath}): {e}"); + context.Error($"Failed to create step summary using 'GITHUB_STEP_SUMMARY': {e.Message}"); + } + } + } } diff --git a/src/Runner.Worker/GitHubContext.cs b/src/Runner.Worker/GitHubContext.cs index 589c17720..80f47b307 100644 --- a/src/Runner.Worker/GitHubContext.cs +++ b/src/Runner.Worker/GitHubContext.cs @@ -34,6 +34,7 @@ namespace GitHub.Runner.Worker "run_number", "server_url", "sha", + "step_summary", "workflow", "workspace", }; diff --git a/src/Sdk/DTWebApi/WebApi/TaskAttachment.cs b/src/Sdk/DTWebApi/WebApi/TaskAttachment.cs index 0b55a06d2..fac99a417 100644 --- a/src/Sdk/DTWebApi/WebApi/TaskAttachment.cs +++ b/src/Sdk/DTWebApi/WebApi/TaskAttachment.cs @@ -102,4 +102,10 @@ namespace GitHub.DistributedTask.WebApi public static readonly String FileAttachment = "DistributedTask.Core.FileAttachment"; public static readonly String DiagnosticLog = "DistributedTask.Core.DiagnosticLog"; } + + [GenerateAllConstants] + public class ChecksAttachmentType + { + public static readonly String StepSummary = "Checks.Step.Summary"; + } } diff --git a/src/Test/L0/Worker/CreateStepSummaryCommandL0.cs b/src/Test/L0/Worker/CreateStepSummaryCommandL0.cs new file mode 100644 index 000000000..f75fb8e01 --- /dev/null +++ b/src/Test/L0/Worker/CreateStepSummaryCommandL0.cs @@ -0,0 +1,241 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Text; +using System.Runtime.CompilerServices; +using GitHub.Runner.Common.Util; +using GitHub.Runner.Sdk; +using GitHub.Runner.Worker; +using Moq; +using Xunit; +using DTWebApi = GitHub.DistributedTask.WebApi; +using GitHub.DistributedTask.WebApi; + +namespace GitHub.Runner.Common.Tests.Worker +{ + public sealed class CreateStepSummaryCommandL0 + { + private Mock _executionContext; + private List> _issues; + private Variables _variables; + private string _rootDirectory; + private CreateStepSummaryCommand _createStepCommand; + private ITraceWriter _trace; + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CreateStepSummaryCommand_FeatureDisabled() + { + using (var hostContext = Setup(featureFlagState: "false")) + { + var stepSummaryFile = Path.Combine(_rootDirectory, "feature-off"); + + _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, It.IsAny(), It.IsAny()), Times.Never()); + + Assert.Equal(0, _issues.Count); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CreateStepSummaryCommand_FileNull() + { + using (var hostContext = Setup()) + { + _createStepCommand.ProcessCommand(_executionContext.Object, null, null); + + _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, It.IsAny(), It.IsAny()), Times.Never()); + Assert.Equal(0, _issues.Count); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CreateStepSummaryCommand_DirectoryNotFound() + { + using (var hostContext = Setup()) + { + var stepSummaryFile = Path.Combine(_rootDirectory, "directory-not-found", "env"); + + _createStepCommand.ProcessCommand(_executionContext.Object, stepSummaryFile, null); + + _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, It.IsAny(), It.IsAny()), Times.Never()); + Assert.Equal(0, _issues.Count); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CreateStepSummaryCommand_FileNotFound() + { + using (var hostContext = Setup()) + { + var stepSummaryFile = Path.Combine(_rootDirectory, "file-not-found"); + + _createStepCommand.ProcessCommand(_executionContext.Object, stepSummaryFile, null); + + _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, It.IsAny(), It.IsAny()), Times.Never()); + Assert.Equal(0, _issues.Count); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CreateStepSummaryCommand_EmptyFile() + { + using (var hostContext = Setup()) + { + var stepSummaryFile = Path.Combine(_rootDirectory, "empty-file"); + File.Create(stepSummaryFile).Dispose(); + + _createStepCommand.ProcessCommand(_executionContext.Object, stepSummaryFile, null); + + _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, It.IsAny(), It.IsAny()), Times.Never()); + Assert.Equal(0, _issues.Count); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CreateStepSummaryCommand_LargeFile() + { + using (var hostContext = Setup()) + { + var stepSummaryFile = Path.Combine(_rootDirectory, "empty-file"); + File.WriteAllBytes(stepSummaryFile, new byte[128 * 1024 + 1]); + + _createStepCommand.ProcessCommand(_executionContext.Object, stepSummaryFile, null); + + _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, It.IsAny(), It.IsAny()), Times.Never()); + Assert.Equal(1, _issues.Count); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CreateStepSummaryCommand_Simple() + { + using (var hostContext = Setup()) + { + var stepSummaryFile = Path.Combine(_rootDirectory, "simple"); + var content = new List + { + "# This is some markdown content", + "", + "## This is more markdown content", + }; + WriteContent(stepSummaryFile, content); + + _createStepCommand.ProcessCommand(_executionContext.Object, stepSummaryFile, null); + + _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, _executionContext.Object.Id.ToString(), stepSummaryFile + "-scrubbed"), Times.Once()); + Assert.Equal(0, _issues.Count); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CreateStepSummaryCommand_ScrubSecrets() + { + using (var hostContext = Setup()) + { + // configure secretmasker to actually mask secrets + hostContext.SecretMasker.AddRegex("Password=.*"); + hostContext.SecretMasker.AddRegex("ghs_.*"); + + var stepSummaryFile = Path.Combine(_rootDirectory, "simple"); + var scrubbedFile = stepSummaryFile + "-scrubbed"; + var content = new List + { + "# Password=ThisIsMySecretPassword!", + "", + "# GITHUB_TOKEN ghs_verysecuretoken", + }; + WriteContent(stepSummaryFile, content); + + _createStepCommand.ProcessCommand(_executionContext.Object, stepSummaryFile, null); + + var scrubbedFileContents = File.ReadAllText(scrubbedFile); + Assert.DoesNotContain("ThisIsMySecretPassword!", scrubbedFileContents); + Assert.DoesNotContain("ghs_verysecuretoken", scrubbedFileContents); + + _executionContext.Verify(e => e.QueueAttachFile(ChecksAttachmentType.StepSummary, _executionContext.Object.Id.ToString(), scrubbedFile), Times.Once()); + Assert.Equal(0, _issues.Count); + } + } + + private void WriteContent( + string path, + List content, + string newline = null) + { + if (string.IsNullOrEmpty(newline)) + { + newline = Environment.NewLine; + } + + var encoding = new UTF8Encoding(true); // Emit BOM + var contentStr = string.Join(newline, content); + File.WriteAllText(path, contentStr, encoding); + } + + private TestHostContext Setup([CallerMemberName] string name = "", string featureFlagState = "true") + { + _issues = new List>(); + + var hostContext = new TestHostContext(this, name); + + // Trace + _trace = hostContext.GetTrace(); + + _variables = new Variables(hostContext, new Dictionary + { + { "MySecretName", new VariableValue("My secret value", true) }, + { "DistributedTask.UploadStepSummary", featureFlagState }, + }); + + // Directory for test data + var workDirectory = hostContext.GetDirectory(WellKnownDirectory.Work); + ArgUtil.NotNullOrEmpty(workDirectory, nameof(workDirectory)); + Directory.CreateDirectory(workDirectory); + _rootDirectory = Path.Combine(workDirectory, nameof(CreateStepSummaryCommandL0)); + Directory.CreateDirectory(_rootDirectory); + + // Execution context + _executionContext = new Mock(); + _executionContext.Setup(x => x.Global) + .Returns(new GlobalContext + { + EnvironmentVariables = new Dictionary(VarUtil.EnvironmentVariableKeyComparer), + WriteDebug = true, + Variables = _variables, + }); + _executionContext.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())) + .Callback((DTWebApi.Issue issue, string logMessage) => + { + _issues.Add(new Tuple(issue, logMessage)); + var message = !string.IsNullOrEmpty(logMessage) ? logMessage : issue.Message; + _trace.Info($"Issue '{issue.Type}': {message}"); + }); + _executionContext.Setup(x => x.Write(It.IsAny(), It.IsAny())) + .Callback((string tag, string message) => + { + _trace.Info($"{tag}{message}"); + }); + + //CreateStepSummaryCommand + _createStepCommand = new CreateStepSummaryCommand(); + _createStepCommand.Initialize(hostContext); + + return hostContext; + } + } +}