Composite Action Step Markers (#4243)

src/Runner.Common/ActionCommand.cs

@@ -204,6 +204,26 @@ namespace GitHub.Runner.Common
             return unescaped;
         }
 
+        /// <summary>
+        /// Escapes special characters in a value using the standard action command escape mappings.
+        /// Iterates in reverse so that '%' is escaped first to avoid double-encoding.
+        /// </summary>
+        public static string EscapeValue(string value)
+        {
+            if (string.IsNullOrEmpty(value))
+            {
+                return value;
+            }
+
+            string escaped = value;
+            for (int i = _escapeMappings.Length - 1; i >= 0; i--)
+            {
+                escaped = escaped.Replace(_escapeMappings[i].Token, _escapeMappings[i].Replacement);
+            }
+
+            return escaped;
+        }
+
         private static string UnescapeProperty(string escaped)
         {
             if (string.IsNullOrEmpty(escaped))

src/Runner.Common/Constants.cs

@@ -174,6 +174,7 @@ namespace GitHub.Runner.Common
                 public static readonly string CompareWorkflowParser = "actions_runner_compare_workflow_parser";
                 public static readonly string SetOrchestrationIdEnvForActions = "actions_set_orchestration_id_env_for_actions";
                 public static readonly string SendJobLevelAnnotations = "actions_send_job_level_annotations";
+                public static readonly string EmitCompositeMarkers = "actions_runner_emit_composite_markers";
             }
 
             // Node version migration related constants
@@ -288,6 +289,7 @@ namespace GitHub.Runner.Common
                 public static readonly string ForcedActionsNodeVersion = "ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION";
                 public static readonly string PrintLogToStdout = "ACTIONS_RUNNER_PRINT_LOG_TO_STDOUT";
                 public static readonly string ActionArchiveCacheDirectory = "ACTIONS_RUNNER_ACTION_ARCHIVE_CACHE";
+                public static readonly string EmitCompositeMarkers = "ACTIONS_RUNNER_EMIT_COMPOSITE_MARKERS";
             }
 
             public static class System

src/Runner.Worker/Handlers/CompositeActionHandler.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -226,6 +227,11 @@ namespace GitHub.Runner.Worker.Handlers
         {
             ArgUtil.NotNull(embeddedSteps, nameof(embeddedSteps));
 
+            bool emitCompositeMarkers =
+                (ExecutionContext.Global.Variables.GetBoolean(Constants.Runner.Features.EmitCompositeMarkers) ?? false)
+                || StringUtil.ConvertToBoolean(
+                    System.Environment.GetEnvironmentVariable(Constants.Variables.Agent.EmitCompositeMarkers));
+
             foreach (IStep step in embeddedSteps)
             {
                 Trace.Info($"Processing embedded step: DisplayName='{step.DisplayName}'");
@@ -297,6 +303,20 @@ namespace GitHub.Runner.Worker.Handlers
                     SetStepConclusion(step, TaskResult.Failed);
                 }
 
+                // Marker ID uses the step's fully qualified context name (ScopeName.ContextName),
+                // which encodes the full composite nesting chain at any depth.
+                var markerId = emitCompositeMarkers ? step.ExecutionContext.GetFullyQualifiedContextName() : null;
+                var stepStopwatch = default(Stopwatch);
+                var endMarkerEmitted = false;
+
+                // Emit start marker after full context setup so display name expressions resolve correctly
+                if (emitCompositeMarkers)
+                {
+                    step.TryUpdateDisplayName(out _);
+                    ExecutionContext.Output($"##[start-action display={EscapeProperty(SanitizeDisplayName(step.DisplayName))};id={EscapeProperty(markerId)}]");
+                    stepStopwatch = Stopwatch.StartNew();
+                }
+
                 // Register Callback
                 CancellationTokenRegistration? jobCancelRegister = null;
                 try
@@ -381,6 +401,14 @@ namespace GitHub.Runner.Worker.Handlers
                         // Condition is false
                         Trace.Info("Skipping step due to condition evaluation.");
                         SetStepConclusion(step, TaskResult.Skipped);
+
+                        if (emitCompositeMarkers)
+                        {
+                            stepStopwatch.Stop();
+                            ExecutionContext.Output($"##[end-action id={EscapeProperty(markerId)};outcome=skipped;conclusion=skipped;duration_ms=0]");
+                            endMarkerEmitted = true;
+                        }
+
                         continue;
                     }
                     else if (conditionEvaluateError != null)
@@ -389,13 +417,31 @@ namespace GitHub.Runner.Worker.Handlers
                         step.ExecutionContext.Error(conditionEvaluateError);
                         SetStepConclusion(step, TaskResult.Failed);
                         ExecutionContext.Result = TaskResult.Failed;
+
+                        if (emitCompositeMarkers)
+                        {
+                            stepStopwatch.Stop();
+                            ExecutionContext.Output($"##[end-action id={EscapeProperty(markerId)};outcome=failure;conclusion=failure;duration_ms={stepStopwatch.ElapsedMilliseconds}]");
+                            endMarkerEmitted = true;
+                        }
+
                         break;
                     }
                     else
                     {
                         await RunStepAsync(step);
-                    }
 
+                        if (emitCompositeMarkers)
+                        {
+                            stepStopwatch.Stop();
+                            // Outcome = raw result before continue-on-error (null when continue-on-error didn't fire)
+                            // Result = final result after continue-on-error
+                            var outcome = (step.ExecutionContext.Outcome ?? step.ExecutionContext.Result ?? TaskResult.Succeeded).ToActionResult().ToString().ToLowerInvariant();
+                            var conclusion = (step.ExecutionContext.Result ?? TaskResult.Succeeded).ToActionResult().ToString().ToLowerInvariant();
+                            ExecutionContext.Output($"##[end-action id={EscapeProperty(markerId)};outcome={outcome};conclusion={conclusion};duration_ms={stepStopwatch.ElapsedMilliseconds}]");
+                            endMarkerEmitted = true;
+                        }
+                    }
                 }
                 finally
                 {
@@ -404,6 +450,14 @@ namespace GitHub.Runner.Worker.Handlers
                         jobCancelRegister?.Dispose();
                         jobCancelRegister = null;
                     }
+
+                    if (emitCompositeMarkers && !endMarkerEmitted)
+                    {
+                        stepStopwatch.Stop();
+                        var outcome = (step.ExecutionContext.Outcome ?? step.ExecutionContext.Result ?? TaskResult.Failed).ToActionResult().ToString().ToLowerInvariant();
+                        var conclusion = (step.ExecutionContext.Result ?? TaskResult.Failed).ToActionResult().ToString().ToLowerInvariant();
+                        ExecutionContext.Output($"##[end-action id={EscapeProperty(markerId)};outcome={outcome};conclusion={conclusion};duration_ms={stepStopwatch.ElapsedMilliseconds}]");
+                    }
                 }
                 // Check failed or cancelled
                 if (step.ExecutionContext.Result == TaskResult.Failed || step.ExecutionContext.Result == TaskResult.Canceled)
@@ -470,5 +524,44 @@ namespace GitHub.Runner.Worker.Handlers
             step.ExecutionContext.Result = result;
             step.ExecutionContext.UpdateGlobalStepsContext();
         }
+
+        /// <summary>
+        /// Escapes marker property values so they cannot break the ##[command key=value] format.
+        /// Delegates to ActionCommand.EscapeValue which escapes `;`, `]`, `\r`, `\n`, and `%`.
+        /// </summary>
+        internal static string EscapeProperty(string value)
+        {
+            return ActionCommand.EscapeValue(value);
+        }
+
+        /// <summary>Maximum character length for display names in markers to prevent log bloat.</summary>
+        internal const int MaxDisplayNameLength = 1000;
+
+        /// <summary>
+        /// Normalizes a step display name for safe embedding in a marker property.
+        /// Trims leading whitespace, drops everything after the first newline, and
+        /// truncates to <see cref="MaxDisplayNameLength"/> characters.
+        /// </summary>
+        internal static string SanitizeDisplayName(string displayName)
+        {
+            if (string.IsNullOrEmpty(displayName)) return displayName;
+
+            // Take first line only (FormatStepName in ActionRunner.cs already does this
+            // for most cases, but be defensive for any code path that skips it)
+            var result = displayName.TrimStart(' ', '\t', '\r', '\n');
+            var firstNewLine = result.IndexOfAny(new[] { '\r', '\n' });
+            if (firstNewLine >= 0)
+            {
+                result = result.Substring(0, firstNewLine);
+            }
+
+            // Truncate excessively long names
+            if (result.Length > MaxDisplayNameLength)
+            {
+                result = result.Substring(0, MaxDisplayNameLength);
+            }
+
+            return result;
+        }
     }
 }

src/Runner.Worker/Handlers/OutputManager.cs

@@ -90,6 +90,14 @@ namespace GitHub.Runner.Worker.Handlers
                 }
             }
 
+            // Strip runner-controlled markers from user output to prevent injection
+            if (!String.IsNullOrEmpty(line) &&
+                (line.Contains("##[start-action") || line.Contains("##[end-action")))
+            {
+                line = line.Replace("##[start-action", @"##[\start-action")
+                           .Replace("##[end-action", @"##[\end-action");
+            }
+
             // Problem matchers
             if (_matchers.Length > 0)
             {

src/Test/L0/Worker/Handlers/CompositeActionHandlerL0.cs

@@ -0,0 +1,271 @@
+using System;
+using System.Collections.Generic;
+using System.Runtime.CompilerServices;
+using GitHub.DistributedTask.WebApi;
+using GitHub.Runner.Common.Util;
+using GitHub.Runner.Sdk;
+using GitHub.Runner.Worker;
+using GitHub.Runner.Worker.Handlers;
+using Moq;
+using Xunit;
+using DTWebApi = GitHub.DistributedTask.WebApi;
+
+namespace GitHub.Runner.Common.Tests.Worker.Handlers
+{
+    public sealed class CompositeActionHandlerL0
+    {
+        // Test EscapeProperty helper logic via reflection or by testing the markers output
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void EscapeProperty_EscapesSpecialCharacters()
+        {
+            // Test the escaping logic that would be applied
+            var input = "value;with%special\r\n]chars";
+            var escaped = EscapeProperty(input);
+            Assert.Equal("value%3Bwith%25special%0D%0A%5Dchars", escaped);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void EscapeProperty_HandlesNullAndEmpty()
+        {
+            Assert.Null(EscapeProperty(null));
+            Assert.Equal("", EscapeProperty(""));
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void SanitizeDisplayName_TruncatesLongNames()
+        {
+            var longName = new string('a', 1500);
+            var sanitized = SanitizeDisplayName(longName);
+            Assert.Equal(CompositeActionHandler.MaxDisplayNameLength, sanitized.Length);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void SanitizeDisplayName_TakesFirstLineOnly()
+        {
+            var multiline = "First line\nSecond line\nThird line";
+            var sanitized = SanitizeDisplayName(multiline);
+            Assert.Equal("First line", sanitized);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void SanitizeDisplayName_TrimsLeadingWhitespace()
+        {
+            var withLeading = "   \n  \t  Actual name\nSecond line";
+            var sanitized = SanitizeDisplayName(withLeading);
+            Assert.Equal("Actual name", sanitized);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void SanitizeDisplayName_HandlesCarriageReturn()
+        {
+            var withCR = "First line\r\nSecond line";
+            var sanitized = SanitizeDisplayName(withCR);
+            Assert.Equal("First line", sanitized);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void SanitizeDisplayName_HandlesNullAndEmpty()
+        {
+            Assert.Null(SanitizeDisplayName(null));
+            Assert.Equal("", SanitizeDisplayName(""));
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void EmitMarkers_DisplayNameEscaping()
+        {
+            // Verify that special characters in display names get escaped properly
+            var displayName = "Step with semicolons; and more; here";
+            var escaped = EscapeProperty(SanitizeDisplayName(displayName));
+            Assert.Equal("Step with semicolons%3B and more%3B here", escaped);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void EmitMarkers_DisplayNameWithBrackets()
+        {
+            var displayName = "Step with [brackets] inside";
+            var escaped = EscapeProperty(SanitizeDisplayName(displayName));
+            Assert.Equal("Step with [brackets%5D inside", escaped);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void StripUserEmittedMarkers_StartAction()
+        {
+            // Simulate what OutputManager does to strip markers
+            var userLine = "##[start-action display=Fake;id=fake]";
+            var stripped = StripMarkers(userLine);
+            Assert.Equal(@"##[\start-action display=Fake;id=fake]", stripped);
+            Assert.DoesNotContain("##[start-action", stripped);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void StripUserEmittedMarkers_EndAction()
+        {
+            var userLine = "##[end-action id=fake;outcome=success;conclusion=success;duration_ms=100]";
+            var stripped = StripMarkers(userLine);
+            Assert.Equal(@"##[\end-action id=fake;outcome=success;conclusion=success;duration_ms=100]", stripped);
+            Assert.DoesNotContain("##[end-action", stripped);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void StripUserEmittedMarkers_PreservesOtherCommands()
+        {
+            var userLine = "##[group]My Group";
+            var stripped = StripMarkers(userLine);
+            Assert.Equal("##[group]My Group", stripped);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void StripUserEmittedMarkers_HandlesEmbeddedMarkers()
+        {
+            var userLine = "Some text ##[start-action display=fake;id=fake] more text";
+            var stripped = StripMarkers(userLine);
+            Assert.Equal(@"Some text ##[\start-action display=fake;id=fake] more text", stripped);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void TaskResultToActionResult_Success()
+        {
+            var result = GitHub.DistributedTask.WebApi.TaskResult.Succeeded;
+            var actionResult = result.ToActionResult();
+            Assert.Equal(ActionResult.Success, actionResult);
+            Assert.Equal("success", actionResult.ToString().ToLowerInvariant());
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void TaskResultToActionResult_Failure()
+        {
+            var result = GitHub.DistributedTask.WebApi.TaskResult.Failed;
+            var actionResult = result.ToActionResult();
+            Assert.Equal(ActionResult.Failure, actionResult);
+            Assert.Equal("failure", actionResult.ToString().ToLowerInvariant());
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void TaskResultToActionResult_Cancelled()
+        {
+            var result = GitHub.DistributedTask.WebApi.TaskResult.Canceled;
+            var actionResult = result.ToActionResult();
+            Assert.Equal(ActionResult.Cancelled, actionResult);
+            Assert.Equal("cancelled", actionResult.ToString().ToLowerInvariant());
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void TaskResultToActionResult_Skipped()
+        {
+            var result = GitHub.DistributedTask.WebApi.TaskResult.Skipped;
+            var actionResult = result.ToActionResult();
+            Assert.Equal(ActionResult.Skipped, actionResult);
+            Assert.Equal("skipped", actionResult.ToString().ToLowerInvariant());
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void MarkerFormat_StartAction()
+        {
+            var display = "My Step";
+            var id = "my-step";
+            var marker = $"##[start-action display={EscapeProperty(display)};id={EscapeProperty(id)}]";
+            Assert.Equal("##[start-action display=My Step;id=my-step]", marker);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void MarkerFormat_EndAction()
+        {
+            var id = "my-step";
+            var outcome = "success";
+            var conclusion = "success";
+            var durationMs = 1234;
+            var marker = $"##[end-action id={EscapeProperty(id)};outcome={outcome};conclusion={conclusion};duration_ms={durationMs}]";
+            Assert.Equal("##[end-action id=my-step;outcome=success;conclusion=success;duration_ms=1234]", marker);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void MarkerFormat_NestedId()
+        {
+            var prefix = "outer-composite";
+            var contextName = "inner-step";
+            var stepId = $"{prefix}.{contextName}";
+            Assert.Equal("outer-composite.inner-step", stepId);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void MarkerFormat_SkippedStep()
+        {
+            var id = "skipped-step";
+            var marker = $"##[end-action id={EscapeProperty(id)};outcome=skipped;conclusion=skipped;duration_ms=0]";
+            Assert.Equal("##[end-action id=skipped-step;outcome=skipped;conclusion=skipped;duration_ms=0]", marker);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void MarkerFormat_ContinueOnError()
+        {
+            // When continue-on-error is true and step fails:
+            // outcome = failure (raw result)
+            // conclusion = success (after continue-on-error applied)
+            var id = "failing-step";
+            var marker = $"##[end-action id={EscapeProperty(id)};outcome=failure;conclusion=success;duration_ms=500]";
+            Assert.Equal("##[end-action id=failing-step;outcome=failure;conclusion=success;duration_ms=500]", marker);
+        }
+
+        // Helper methods that call the real production code
+        private static string EscapeProperty(string value) =>
+            CompositeActionHandler.EscapeProperty(value);
+
+        private static string SanitizeDisplayName(string displayName) =>
+            CompositeActionHandler.SanitizeDisplayName(displayName);
+
+        private static string StripMarkers(string line)
+        {
+            if (!string.IsNullOrEmpty(line) &&
+                (line.Contains("##[start-action") || line.Contains("##[end-action")))
+            {
+                line = line.Replace("##[start-action", @"##[\start-action")
+                           .Replace("##[end-action", @"##[\end-action");
+            }
+            return line;
+        }
+    }
+}

src/Test/L0/Worker/OutputManagerL0.cs

@@ -1006,6 +1006,66 @@ namespace GitHub.Runner.Common.Tests.Worker
             }
         }
 
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void StripCompositeMarkers_StartAction()
+        {
+            using (Setup())
+            using (_outputManager)
+            {
+                Process("##[start-action display=Fake;id=fake]");
+                Assert.Single(_messages);
+                Assert.Contains(@"##[\start-action", _messages[0]);
+                Assert.DoesNotContain("##[start-action", _messages[0]);
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void StripCompositeMarkers_EndAction()
+        {
+            using (Setup())
+            using (_outputManager)
+            {
+                Process("##[end-action id=fake;outcome=success;conclusion=success;duration_ms=100]");
+                Assert.Single(_messages);
+                Assert.Contains(@"##[\end-action", _messages[0]);
+                Assert.DoesNotContain("##[end-action", _messages[0]);
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void StripCompositeMarkers_PreservesOtherCommands()
+        {
+            using (Setup())
+            using (_outputManager)
+            {
+                Process("##[group]My Group");
+                // Should not be stripped (not a composite marker)
+                Assert.Single(_messages);
+                Assert.Equal("##[group]My Group", _messages[0]);
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void StripCompositeMarkers_EmbeddedInLine()
+        {
+            using (Setup())
+            using (_outputManager)
+            {
+                Process("Some text ##[start-action display=fake;id=fake] more text");
+                Assert.Single(_messages);
+                Assert.Contains(@"##[\start-action", _messages[0]);
+                Assert.DoesNotContain("##[start-action", _messages[0]);
+            }
+        }
+
         private TestHostContext Setup(
             [CallerMemberName] string name = "",
             IssueMatchersConfig matchers = null,