Release 2.273.5

* notify on unsecure commands

releaseNote.md

@@ -1,7 +1,6 @@
 ## Features
-  - Add in Log line numbers for streaming logs (#663)
-  - Read env file (#683)
-  - Add Runner File Commands (#684, #693)
+  - Expose retention days in env for toolkit/artifacts package (#714)
+  - Notify on unsecure commands (#731)
 
 ## Bugs
   - N/A

releaseVersion

@@ -1 +1 @@
-2.273.1
+2.273.5

src/Runner.Common/Constants.cs

@@ -140,6 +140,9 @@ namespace GitHub.Runner.Common
 
             public static readonly string InternalTelemetryIssueDataKey = "_internal_telemetry";
             public static readonly string WorkerCrash = "WORKER_CRASH";
+            public static readonly string UnsupportedCommand = "UNSUPPORTED_COMMAND";
+            public static readonly string UnsupportedCommandMessage = "The `{0}` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/";
+            public static readonly string UnsupportedCommandMessageDisabled = "The `{0}` command is disabled. Please upgrade to using Environment Files or opt into unsecure command execution by setting the `ACTIONS_ALLOW_UNSECURE_COMMANDS` environment variable to `true`. For more information see: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/";
         }
 
         public static class RunnerEvent
@@ -198,6 +201,7 @@ namespace GitHub.Runner.Common
                 //
                 // Keep alphabetical
                 //
+                public static readonly string AllowUnsupportedCommands = "ACTIONS_ALLOW_UNSECURE_COMMANDS";
                 public static readonly string RunnerDebug = "ACTIONS_RUNNER_DEBUG";
                 public static readonly string StepDebug = "ACTIONS_STEP_DEBUG";
             }

src/Runner.Worker/ActionCommandManager.cs

@@ -1,4 +1,5 @@
 using GitHub.DistributedTask.Pipelines;
+using GitHub.DistributedTask.Pipelines.ContextData;
 using GitHub.DistributedTask.WebApi;
 using GitHub.Runner.Common.Util;
 using GitHub.Runner.Worker.Container;
@@ -183,6 +184,40 @@ namespace GitHub.Runner.Worker
 
         public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
         {
+            var configurationStore = HostContext.GetService<IConfigurationStore>();
+            var isHostedServer = configurationStore.GetSettings().IsHostedServer;
+            
+            var allowUnsecureCommands = false;
+            bool.TryParse(Environment.GetEnvironmentVariable(Constants.Variables.Actions.AllowUnsupportedCommands), out allowUnsecureCommands);
+
+            // Apply environment from env context, env context contains job level env and action's env block
+#if OS_WINDOWS
+            var envContext = context.ExpressionValues["env"] as DictionaryContextData;
+#else
+            var envContext = context.ExpressionValues["env"] as CaseSensitiveDictionaryContextData;
+#endif
+            if (!allowUnsecureCommands && envContext.ContainsKey(Constants.Variables.Actions.AllowUnsupportedCommands))
+            {
+                bool.TryParse(envContext[Constants.Variables.Actions.AllowUnsupportedCommands].ToString(), out allowUnsecureCommands);
+            }
+
+            // TODO: Eventually remove isHostedServer and apply this to dotcom customers as well
+            if (!isHostedServer && !allowUnsecureCommands)
+            {
+                throw new Exception(String.Format(Constants.Runner.UnsupportedCommandMessageDisabled, this.Command));
+            }
+            else if (!allowUnsecureCommands)
+            {
+                // Log Telemetry and let user know they shouldn't do this
+                var issue = new Issue() 
+                { 
+                    Type = IssueType.Warning, 
+                    Message = String.Format(Constants.Runner.UnsupportedCommandMessage, this.Command)
+                };
+                issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.UnsupportedCommand;
+                context.AddIssue(issue);
+            }
+
             if (!command.Properties.TryGetValue(SetEnvCommandProperties.Name, out string envName) || string.IsNullOrEmpty(envName))
             {
                 throw new Exception("Required field 'name' is missing in ##[set-env] command.");
@@ -282,6 +317,40 @@ namespace GitHub.Runner.Worker
 
         public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
         {
+            var configurationStore = HostContext.GetService<IConfigurationStore>();
+            var isHostedServer = configurationStore.GetSettings().IsHostedServer;
+            
+            var allowUnsecureCommands = false;
+            bool.TryParse(Environment.GetEnvironmentVariable(Constants.Variables.Actions.AllowUnsupportedCommands), out allowUnsecureCommands);
+
+            // Apply environment from env context, env context contains job level env and action's env block
+#if OS_WINDOWS
+            var envContext = context.ExpressionValues["env"] as DictionaryContextData;
+#else
+            var envContext = context.ExpressionValues["env"] as CaseSensitiveDictionaryContextData;
+#endif
+            if (!allowUnsecureCommands && envContext.ContainsKey(Constants.Variables.Actions.AllowUnsupportedCommands))
+            {
+                bool.TryParse(envContext[Constants.Variables.Actions.AllowUnsupportedCommands].ToString(), out allowUnsecureCommands);
+            }
+
+            // TODO: Eventually remove isHostedServer and apply this to dotcom customers as well
+            if (!isHostedServer && !allowUnsecureCommands)
+            {
+                throw new Exception(String.Format(Constants.Runner.UnsupportedCommandMessageDisabled, this.Command));
+            }
+            else if (!allowUnsecureCommands)
+            {
+                // Log Telemetry and let user know they shouldn't do this
+                var issue = new Issue() 
+                { 
+                    Type = IssueType.Warning, 
+                    Message = String.Format(Constants.Runner.UnsupportedCommandMessage, this.Command)
+                };
+                issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.UnsupportedCommand;
+                context.AddIssue(issue);
+            }
+
             ArgUtil.NotNullOrEmpty(command.Data, "path");
             context.Global.PrependPath.RemoveAll(x => string.Equals(x, command.Data, StringComparison.CurrentCulture));
             context.Global.PrependPath.Add(command.Data);

src/Runner.Worker/Container/ContainerInfo.cs

@@ -34,6 +34,9 @@ namespace GitHub.Runner.Worker.Container
             _environmentVariables = container.Environment;
             this.IsJobContainer = isJobContainer;
             this.ContainerNetworkAlias = networkAlias;
+            this.RegistryAuthUsername = container.Credentials?.Username;
+            this.RegistryAuthPassword = container.Credentials?.Password;
+            this.RegistryServer = DockerUtil.ParseRegistryHostnameFromImageName(this.ContainerImage);
 
 #if OS_WINDOWS
             _pathMappings.Add(new PathMapping(hostContext.GetDirectory(WellKnownDirectory.Work), "C:\\__w"));
@@ -79,6 +82,9 @@ namespace GitHub.Runner.Worker.Container
         public string ContainerWorkDirectory { get; set; }
         public string ContainerCreateOptions { get; private set; }
         public string ContainerRuntimePath { get; set; }
+        public string RegistryServer { get; set; }
+        public string RegistryAuthUsername { get; set; }
+        public string RegistryAuthPassword { get; set; }
         public bool IsJobContainer { get; set; }
 
         public IDictionary<string, string> ContainerEnvironmentVariables

src/Runner.Worker/Container/DockerCommandManager.cs

@@ -4,6 +4,7 @@ using System.IO;
 using System.Linq;
 using System.Text.RegularExpressions;
 using System.Threading;
+using System.Threading.Channels;
 using System.Threading.Tasks;
 using GitHub.Runner.Common;
 using GitHub.Runner.Sdk;
@@ -17,6 +18,7 @@ namespace GitHub.Runner.Worker.Container
         string DockerInstanceLabel { get; }
         Task<DockerVersion> DockerVersion(IExecutionContext context);
         Task<int> DockerPull(IExecutionContext context, string image);
+        Task<int> DockerPull(IExecutionContext context, string image, string configFileDirectory);
         Task<int> DockerBuild(IExecutionContext context, string workingDirectory, string dockerFile, string dockerContext, string tag);
         Task<string> DockerCreate(IExecutionContext context, ContainerInfo container);
         Task<int> DockerRun(IExecutionContext context, ContainerInfo container, EventHandler<ProcessDataReceivedEventArgs> stdoutDataReceived, EventHandler<ProcessDataReceivedEventArgs> stderrDataReceived);
@@ -31,6 +33,7 @@ namespace GitHub.Runner.Worker.Container
         Task<int> DockerExec(IExecutionContext context, string containerId, string options, string command, List<string> outputs);
         Task<List<string>> DockerInspect(IExecutionContext context, string dockerObject, string options);
         Task<List<PortMapping>> DockerPort(IExecutionContext context, string containerId);
+        Task<int> DockerLogin(IExecutionContext context, string configFileDirectory, string registry, string username, string password);
     }
 
     public class DockerCommandManager : RunnerService, IDockerCommandManager
@@ -82,9 +85,18 @@ namespace GitHub.Runner.Worker.Container
             return new DockerVersion(serverVersion, clientVersion);
         }
 
-        public async Task<int> DockerPull(IExecutionContext context, string image)
+        public Task<int> DockerPull(IExecutionContext context, string image)
         {
-            return await ExecuteDockerCommandAsync(context, "pull", image, context.CancellationToken);
+            return DockerPull(context, image, null);
+        }
+
+        public async Task<int> DockerPull(IExecutionContext context, string image, string configFileDirectory)
+        {
+            if (string.IsNullOrEmpty(configFileDirectory))
+            {
+                return await ExecuteDockerCommandAsync(context, $"pull", image, context.CancellationToken);
+            }
+            return await ExecuteDockerCommandAsync(context, $"--config {configFileDirectory} pull", image, context.CancellationToken);
         }
 
         public async Task<int> DockerBuild(IExecutionContext context, string workingDirectory, string dockerFile, string dockerContext, string tag)
@@ -346,6 +358,28 @@ namespace GitHub.Runner.Worker.Container
             return DockerUtil.ParseDockerPort(portMappingLines);
         }
 
+        public Task<int> DockerLogin(IExecutionContext context, string configFileDirectory, string registry, string username, string password)
+        {
+            string args = $"--config {configFileDirectory} login {registry} -u {username} --password-stdin";
+            context.Command($"{DockerPath} {args}");
+
+            var input = Channel.CreateBounded<string>(new BoundedChannelOptions(1) { SingleReader = true, SingleWriter = true });
+            input.Writer.TryWrite(password);
+
+            var processInvoker = HostContext.CreateService<IProcessInvoker>();
+
+            return processInvoker.ExecuteAsync(
+                workingDirectory: context.GetGitHubContext("workspace"),
+                fileName: DockerPath,
+                arguments: args,
+                environment: null,
+                requireExitCodeZero: false,
+                outputEncoding: null,
+                killProcessOnCancel: false,
+                redirectStandardIn: input,
+                cancellationToken: context.CancellationToken);
+        }
+
         private Task<int> ExecuteDockerCommandAsync(IExecutionContext context, string command, string options, CancellationToken cancellationToken = default(CancellationToken))
         {
             return ExecuteDockerCommandAsync(context, command, options, null, cancellationToken);

src/Runner.Worker/Container/DockerUtil.cs

@@ -45,5 +45,21 @@ namespace GitHub.Runner.Worker.Container
             }
             return "";
         }
+
+        public static string ParseRegistryHostnameFromImageName(string name)
+        {
+            var nameSplit = name.Split('/');
+            // Single slash is implictly from Dockerhub, unless first part has .tld or :port
+            if (nameSplit.Length == 2 && (nameSplit[0].Contains(":") || nameSplit[0].Contains(".")))
+            {
+                return nameSplit[0];
+            }
+            // All other non Dockerhub registries
+            else if (nameSplit.Length > 2)
+            {
+                return nameSplit[0];
+            }
+            return "";
+        }
     }
 }

src/Runner.Worker/ContainerOperationProvider.cs

@@ -198,12 +198,18 @@ namespace GitHub.Runner.Worker
                 }
             }
 
+            // TODO: Add at a later date. This currently no local package registry to test with
+            // UpdateRegistryAuthForGitHubToken(executionContext, container);
+
+            // Before pulling, generate client authentication if required
+            var configLocation = await ContainerRegistryLogin(executionContext, container);
+
             // Pull down docker image with retry up to 3 times
             int retryCount = 0;
             int pullExitCode = 0;
             while (retryCount < 3)
             {
-                pullExitCode = await _dockerManger.DockerPull(executionContext, container.ContainerImage);
+                pullExitCode = await _dockerManger.DockerPull(executionContext, container.ContainerImage, configLocation);
                 if (pullExitCode == 0)
                 {
                     break;
@@ -220,6 +226,9 @@ namespace GitHub.Runner.Worker
                 }
             }
 
+            // Remove credentials after pulling
+            ContainerRegistryLogout(configLocation);
+
             if (retryCount == 3 && pullExitCode != 0)
             {
                 throw new InvalidOperationException($"Docker pull failed with exit code {pullExitCode}");
@@ -437,5 +446,83 @@ namespace GitHub.Runner.Worker
                 throw new InvalidOperationException($"Failed to initialize, {container.ContainerNetworkAlias} service is {serviceHealth}.");
             }
         }
+
+        private async Task<string> ContainerRegistryLogin(IExecutionContext executionContext, ContainerInfo container)
+        {
+            if (string.IsNullOrEmpty(container.RegistryAuthUsername) || string.IsNullOrEmpty(container.RegistryAuthPassword))
+            {
+                // No valid client config can be generated
+                return "";
+            }
+            var configLocation = Path.Combine(HostContext.GetDirectory(WellKnownDirectory.Temp), $".docker_{Guid.NewGuid()}");
+            try
+            {
+                var dirInfo = Directory.CreateDirectory(configLocation);
+            }
+            catch (Exception e)
+            {
+                throw new InvalidOperationException($"Failed to create directory to store registry client credentials: {e.Message}");
+            }
+            var loginExitCode = await _dockerManger.DockerLogin(
+                executionContext,
+                configLocation,
+                container.RegistryServer,
+                container.RegistryAuthUsername,
+                container.RegistryAuthPassword);
+
+            if (loginExitCode != 0)
+            {
+                throw new InvalidOperationException($"Docker login for '{container.RegistryServer}' failed with exit code {loginExitCode}");
+            }
+            return configLocation;
+        }
+
+        private void ContainerRegistryLogout(string configLocation)
+        {
+            try
+            {
+                if (!string.IsNullOrEmpty(configLocation) && Directory.Exists(configLocation))
+                {
+                    Directory.Delete(configLocation, recursive: true);
+                }
+            }
+            catch (Exception e)
+            {
+                throw new InvalidOperationException($"Failed to remove directory containing Docker client credentials: {e.Message}");
+            }
+        }
+
+        private void UpdateRegistryAuthForGitHubToken(IExecutionContext executionContext, ContainerInfo container)
+        {
+            var registryIsTokenCompatible = container.RegistryServer.Equals("docker.pkg.github.com", StringComparison.OrdinalIgnoreCase);
+            if (!registryIsTokenCompatible)
+            {
+                return;
+            }
+
+            var registryMatchesWorkflow = false;
+
+            // REGISTRY/OWNER/REPO/IMAGE[:TAG]
+            var imageParts = container.ContainerImage.Split('/');
+            if (imageParts.Length != 4)
+            {
+                executionContext.Warning($"Could not identify owner and repo for container image {container.ContainerImage}. Skipping automatic token auth");
+                return;
+            }
+            var owner = imageParts[1];
+            var repo = imageParts[2];
+            var nwo = $"{owner}/{repo}";
+            if (nwo.Equals(executionContext.GetGitHubContext("repository"), StringComparison.OrdinalIgnoreCase))
+            {
+                registryMatchesWorkflow = true;
+            }
+
+            var registryCredentialsNotSupplied = string.IsNullOrEmpty(container.RegistryAuthUsername) && string.IsNullOrEmpty(container.RegistryAuthPassword);
+            if (registryCredentialsNotSupplied && registryMatchesWorkflow)
+            {
+                container.RegistryAuthUsername = executionContext.GetGitHubContext("actor");
+                container.RegistryAuthPassword = executionContext.GetGitHubContext("token");
+            }
+        }
     }
 }

src/Runner.Worker/GitHubContext.cs

@@ -23,6 +23,7 @@ namespace GitHub.Runner.Worker
             "ref",
             "repository",
             "repository_owner",
+            "retention_days",
             "run_id",
             "run_number",
             "server_url",

src/Runner.Worker/StepsRunner.cs

@@ -103,12 +103,6 @@ namespace GitHub.Runner.Worker
                 bool evaluateStepEnvFailed = false;
                 if (step is IActionRunner actionStep)
                 {
-                    // Shallow copy github context
-                    var gitHubContext = step.ExecutionContext.ExpressionValues["github"] as GitHubContext;
-                    ArgUtil.NotNull(gitHubContext, nameof(gitHubContext));
-                    gitHubContext = gitHubContext.ShallowCopy();
-                    step.ExecutionContext.ExpressionValues["github"] = gitHubContext;
-
                     // Set GITHUB_ACTION
                     step.ExecutionContext.SetGitHubContext("action", actionStep.Action.Name);
 

src/Sdk/DTPipelines/Pipelines/JobContainer.cs

@@ -56,5 +56,36 @@ namespace GitHub.DistributedTask.Pipelines
             get;
             set;
         }
+
+        /// <summary>
+        /// Gets or sets the credentials used for pulling the container iamge.
+        /// </summary>
+        public ContainerRegistryCredentials Credentials
+        {
+            get;
+            set;
+        }
+    }
+
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public sealed class ContainerRegistryCredentials
+    {
+        /// <summary>
+        /// Gets or sets the user to authenticate to a registry with
+        /// </summary>
+        public String Username
+        {
+            get;
+            set;
+        }
+
+        /// <summary>
+        /// Gets or sets the password to authenticate to a registry with
+        /// </summary>
+        public String Password
+        {
+            get;
+            set;
+        }
     }
 }

src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConstants.cs

@@ -14,6 +14,7 @@ namespace GitHub.DistributedTask.Pipelines.ObjectTemplating
         public const String Clean= "clean";
         public const String Container = "container";
         public const String ContinueOnError = "continue-on-error";
+        public const String Credentials = "credentials";
         public const String Defaults = "defaults";
         public const String Env = "env";
         public const String Event = "event";
@@ -45,6 +46,7 @@ namespace GitHub.DistributedTask.Pipelines.ObjectTemplating
         public const String Options = "options";
         public const String Outputs = "outputs";
         public const String OutputsPattern = "needs.*.outputs";
+        public const String Password = "password";
         public const String Path = "path";
         public const String Pool = "pool";
         public const String Ports = "ports";
@@ -68,6 +70,7 @@ namespace GitHub.DistributedTask.Pipelines.ObjectTemplating
         public const String Success = "success";
         public const String Template = "template";
         public const String TimeoutMinutes = "timeout-minutes";
+        public const String Username = "username";
         public const String Uses = "uses";
         public const String VmImage = "vmImage";
         public const String Volumes = "volumes";

src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConverter.cs

@@ -209,6 +209,30 @@ namespace GitHub.DistributedTask.Pipelines.ObjectTemplating
             return (Int32)numberToken.Value;
         }
 
+        internal static ContainerRegistryCredentials ConvertToContainerCredentials(TemplateToken token)
+        {
+            var credentials = token.AssertMapping(PipelineTemplateConstants.Credentials);
+            var result = new ContainerRegistryCredentials();
+            foreach (var credentialProperty in credentials)
+            {
+                var propertyName = credentialProperty.Key.AssertString($"{PipelineTemplateConstants.Credentials} key");
+                switch (propertyName.Value)
+                {
+                    case PipelineTemplateConstants.Username:
+                        result.Username = credentialProperty.Value.AssertString(PipelineTemplateConstants.Username).Value;
+                        break;
+                    case PipelineTemplateConstants.Password:
+                        result.Password = credentialProperty.Value.AssertString(PipelineTemplateConstants.Password).Value;
+                        break;
+                    default:
+                        propertyName.AssertUnexpectedValue($"{PipelineTemplateConstants.Credentials} key {propertyName}");
+                        break;
+                }
+            }
+
+            return result;
+        }
+
         internal static JobContainer ConvertToJobContainer(
             TemplateContext context,
             TemplateToken value,
@@ -275,6 +299,9 @@ namespace GitHub.DistributedTask.Pipelines.ObjectTemplating
                             }
                             result.Volumes = volumeList;
                             break;
+                        case PipelineTemplateConstants.Credentials:
+                            result.Credentials = ConvertToContainerCredentials(containerPropertyPair.Value);
+                            break;
                         default:
                             propertyName.AssertUnexpectedValue($"{PipelineTemplateConstants.Container} key");
                             break;

src/Sdk/DTPipelines/workflow-v1.0.json

@@ -373,7 +373,8 @@
           "options": "non-empty-string",
           "env": "container-env",
           "ports": "sequence-of-non-empty-string",
-          "volumes": "sequence-of-non-empty-string"
+          "volumes": "sequence-of-non-empty-string",
+          "credentials": "container-registry-credentials"
         }
       }
     },
@@ -404,6 +405,20 @@
       ]
     },
 
+    "container-registry-credentials": {
+      "context": [
+        "secrets",
+        "env",
+        "github"
+      ],
+      "mapping": {
+        "properties": {
+          "username": "non-empty-string",
+          "password": "non-empty-string"
+        }
+      }
+    },
+
     "container-env": {
       "mapping": {
         "loose-key-type": "non-empty-string",

src/Test/L0/Container/DockerUtilL0.cs

@@ -126,5 +126,23 @@ namespace GitHub.Runner.Common.Tests.Worker.Container
             Assert.NotNull(result5);
             Assert.Equal("/foo/bar:/baz", result5);
         }
+
+        [Theory]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        [InlineData("dockerhub/repo", "")]
+        [InlineData("localhost/doesnt_work", "")]
+        [InlineData("localhost:port/works", "localhost:port")]
+        [InlineData("host.tld/works", "host.tld")]
+        [InlineData("ghcr.io/owner/image", "ghcr.io")]
+        [InlineData("gcr.io/project/image", "gcr.io")]
+        [InlineData("myregistry.azurecr.io/namespace/image", "myregistry.azurecr.io")]
+        [InlineData("account.dkr.ecr.region.amazonaws.com/image", "account.dkr.ecr.region.amazonaws.com")]
+        [InlineData("docker.pkg.github.com/owner/repo/image", "docker.pkg.github.com")]
+        public void ParseRegistryHostnameFromImageName(string input, string expected)
+        {
+            var actual = DockerUtil.ParseRegistryHostnameFromImageName(input);
+            Assert.Equal(expected, actual);
+        }
     }
 }

src/runnerversion

@@ -1 +1 @@
-2.273.1
+2.273.5