Fix cancellation token race in EvaluateAndCompare

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Salman Chishti <salmanmkc@GitHub.com>

.github/workflows/build.yml

@@ -78,7 +78,7 @@ jobs:
     # Upload runner package tar.gz/zip as artifact
     - name: Publish Artifact
       if: github.event_name != 'pull_request'
-      uses: actions/upload-artifact@v6
+      uses: actions/upload-artifact@v7
       with:
         name: runner-package-${{ matrix.runtime }}
         path: |

.github/workflows/docker-publish.yml

@@ -41,7 +41,7 @@ jobs:
         uses: docker/setup-buildx-action@v3
 
       - name: Log into registry ${{ env.REGISTRY }}
-        uses: docker/login-action@v3
+        uses: docker/login-action@v4
         with:
           registry: ${{ env.REGISTRY }}
           username: ${{ github.actor }}

.github/workflows/release.yml

@@ -118,7 +118,7 @@ jobs:
     # Upload runner package tar.gz/zip as artifact.
     - name: Publish Artifact
       if: github.event_name != 'pull_request'
-      uses: actions/upload-artifact@v6
+      uses: actions/upload-artifact@v7
       with:
         name: runner-packages-${{ matrix.runtime }}
         path: |
@@ -312,7 +312,7 @@ jobs:
         uses: docker/setup-buildx-action@v3
 
       - name: Log into registry ${{ env.REGISTRY }}
-        uses: docker/login-action@v3
+        uses: docker/login-action@v4
         with:
           registry: ${{ env.REGISTRY }}
           username: ${{ github.actor }}

src/Runner.Worker/ExecutionContext.cs

@@ -77,8 +77,7 @@ namespace GitHub.Runner.Worker
 
         List<string> StepEnvironmentOverrides { get; }
 
-        ExecutionContext Root { get; }
-        ExecutionContext Parent { get; }
+        IExecutionContext Root { get; }
 
         // Initialize
         void InitializeJob(Pipelines.AgentJobRequestMessage message, CancellationToken token);
@@ -251,7 +250,9 @@ namespace GitHub.Runner.Worker
             }
         }
 
-        public ExecutionContext Root
+        IExecutionContext IExecutionContext.Root => Root;
+
+        private ExecutionContext Root
         {
             get
             {
@@ -266,13 +267,7 @@ namespace GitHub.Runner.Worker
             }
         }
 
-        public ExecutionContext Parent
-        {
-            get
-            {
-                return _parentExecutionContext;
-            }
-        }
+
 
         public JobContext JobContext
         {

src/Runner.Worker/PipelineTemplateEvaluatorWrapper.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Threading;
 using GitHub.Actions.WorkflowParser;
 using GitHub.DistributedTask.Expressions2;
 using GitHub.DistributedTask.ObjectTemplating.Tokens;
@@ -226,8 +227,12 @@ namespace GitHub.Runner.Worker
             Func<TNew> newEvaluator,
             Func<TLegacy, TNew, bool> resultComparer)
         {
-            // Capture cancellation state before evaluation
-            var cancellationRequestedBefore = _context.CancellationToken.IsCancellationRequested;
+            // Use the root (job-level) cancellation token to detect cancellation race conditions.
+            // The step-level token only fires on step timeout, not on job cancellation.
+            // Job cancellation mutates JobContext.Status which expression functions read,
+            // so we need the root token to properly detect cancellation between evaluator runs.
+            var rootCancellationToken = _context.Root?.CancellationToken ?? CancellationToken.None;
+            var cancellationRequestedBefore = rootCancellationToken.IsCancellationRequested;
 
             // Legacy evaluator
             var legacyException = default(Exception);
@@ -261,7 +266,7 @@ namespace GitHub.Runner.Worker
                 }
 
                 // Capture cancellation state after evaluation
-                var cancellationRequestedAfter = _context.CancellationToken.IsCancellationRequested;
+                var cancellationRequestedAfter = rootCancellationToken.IsCancellationRequested;
 
                 // Compare results or exceptions
                 bool hasMismatch = false;

src/Sdk/DTExpressions2/Expressions2/ExpressionParser.cs

@@ -20,7 +20,7 @@ namespace GitHub.DistributedTask.Expressions2
             IEnumerable<IFunctionInfo> functions,
             Boolean allowCaseFunction = true)
         {
-            var context = new ParseContext(expression, trace, namedValues, functions, allowCaseFunction);
+            var context = new ParseContext(expression, trace, namedValues, functions, allowCaseFunction: allowCaseFunction);
             context.Trace.Info($"Parsing expression: <{expression}>");
             return CreateTree(context);
         }

src/Test/L0/Sdk/ExpressionParserL0.cs

@@ -0,0 +1,104 @@
+using GitHub.DistributedTask.Expressions2;
+using GitHub.DistributedTask.Expressions2.Sdk;
+using GitHub.DistributedTask.ObjectTemplating;
+using System;
+using System.Collections.Generic;
+using Xunit;
+
+namespace GitHub.Runner.Common.Tests.Sdk
+{
+    /// <summary>
+    /// Regression tests for ExpressionParser.CreateTree to verify that
+    /// allowCaseFunction does not accidentally set allowUnknownKeywords.
+    /// </summary>
+    public sealed class ExpressionParserL0
+    {
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Sdk")]
+        public void CreateTree_RejectsUnrecognizedNamedValue()
+        {
+            // Regression: allowCaseFunction was passed positionally into
+            // the allowUnknownKeywords parameter, causing all named values
+            // to be silently accepted.
+            var parser = new ExpressionParser();
+            var namedValues = new List<INamedValueInfo>
+            {
+                new NamedValueInfo<ContextValueNode>("inputs"),
+            };
+
+            var ex = Assert.Throws<ParseException>(() =>
+                parser.CreateTree("github.event.repository.private", null, namedValues, null));
+
+            Assert.Contains("Unrecognized named-value", ex.Message);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Sdk")]
+        public void CreateTree_AcceptsRecognizedNamedValue()
+        {
+            var parser = new ExpressionParser();
+            var namedValues = new List<INamedValueInfo>
+            {
+                new NamedValueInfo<ContextValueNode>("inputs"),
+            };
+
+            var node = parser.CreateTree("inputs.foo", null, namedValues, null);
+
+            Assert.NotNull(node);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Sdk")]
+        public void CreateTree_CaseFunctionWorks_WhenAllowed()
+        {
+            var parser = new ExpressionParser();
+            var namedValues = new List<INamedValueInfo>
+            {
+                new NamedValueInfo<ContextValueNode>("github"),
+            };
+
+            var node = parser.CreateTree("case(github.event_name, 'push', 'Push Event')", null, namedValues, null, allowCaseFunction: true);
+
+            Assert.NotNull(node);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Sdk")]
+        public void CreateTree_CaseFunctionRejected_WhenDisallowed()
+        {
+            var parser = new ExpressionParser();
+            var namedValues = new List<INamedValueInfo>
+            {
+                new NamedValueInfo<ContextValueNode>("github"),
+            };
+
+            var ex = Assert.Throws<ParseException>(() =>
+                parser.CreateTree("case(github.event_name, 'push', 'Push Event')", null, namedValues, null, allowCaseFunction: false));
+
+            Assert.Contains("Unrecognized function", ex.Message);
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Sdk")]
+        public void CreateTree_CaseFunctionDoesNotAffectUnknownKeywords()
+        {
+            // The key regression test: with allowCaseFunction=true (default),
+            // unrecognized named values must still be rejected.
+            var parser = new ExpressionParser();
+            var namedValues = new List<INamedValueInfo>
+            {
+                new NamedValueInfo<ContextValueNode>("inputs"),
+            };
+
+            var ex = Assert.Throws<ParseException>(() =>
+                parser.CreateTree("github.ref", null, namedValues, null, allowCaseFunction: true));
+
+            Assert.Contains("Unrecognized named-value", ex.Message);
+        }
+    }
+}

src/Test/L0/Worker/ActionManifestManagerL0.cs

@@ -928,6 +928,58 @@ namespace GitHub.Runner.Common.Tests.Worker
             }
         }
 
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void Load_ContainerAction_RejectsInvalidExpressionContext()
+        {
+            try
+            {
+                // Arrange
+                Setup();
+
+                var actionManifest = new ActionManifestManager();
+                actionManifest.Initialize(_hc);
+
+                // Act & Assert — github is not a valid context for container-runs-env (only inputs is allowed)
+                var ex = Assert.Throws<ArgumentException>(() =>
+                    actionManifest.Load(_ec.Object, Path.Combine(TestUtil.GetTestDataPath(), "dockerfileaction_env_invalid_context.yml")));
+
+                Assert.Contains("Failed to load", ex.Message);
+            }
+            finally
+            {
+                Teardown();
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void Load_ContainerAction_AcceptsValidExpressionContext()
+        {
+            try
+            {
+                // Arrange
+                Setup();
+
+                var actionManifest = new ActionManifestManager();
+                actionManifest.Initialize(_hc);
+
+                // Act — inputs is a valid context for container-runs-env
+                var result = actionManifest.Load(_ec.Object, Path.Combine(TestUtil.GetTestDataPath(), "dockerfileaction_arg_env_expression.yml"));
+
+                // Assert
+                var containerAction = result.Execution as ContainerActionExecutionDataNew;
+                Assert.NotNull(containerAction);
+                Assert.Equal("${{ inputs.entryPoint }}", containerAction.Environment[1].Value.ToString());
+            }
+            finally
+            {
+                Teardown();
+            }
+        }
+
         private void Setup([CallerMemberName] string name = "")
         {
             _ecTokenSource?.Dispose();

src/Test/L0/Worker/ActionManifestManagerLegacyL0.cs

@@ -926,6 +926,58 @@ namespace GitHub.Runner.Common.Tests.Worker
             }
         }
 
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void Load_ContainerAction_RejectsInvalidExpressionContext()
+        {
+            try
+            {
+                // Arrange
+                Setup();
+
+                var actionManifest = new ActionManifestManagerLegacy();
+                actionManifest.Initialize(_hc);
+
+                // Act & Assert — github is not a valid context for container-runs-env (only inputs is allowed)
+                var ex = Assert.Throws<ArgumentException>(() =>
+                    actionManifest.Load(_ec.Object, Path.Combine(TestUtil.GetTestDataPath(), "dockerfileaction_env_invalid_context.yml")));
+
+                Assert.Contains("Failed to load", ex.Message);
+            }
+            finally
+            {
+                Teardown();
+            }
+        }
+
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void Load_ContainerAction_AcceptsValidExpressionContext()
+        {
+            try
+            {
+                // Arrange
+                Setup();
+
+                var actionManifest = new ActionManifestManagerLegacy();
+                actionManifest.Initialize(_hc);
+
+                // Act — inputs is a valid context for container-runs-env
+                var result = actionManifest.Load(_ec.Object, Path.Combine(TestUtil.GetTestDataPath(), "dockerfileaction_arg_env_expression.yml"));
+
+                // Assert
+                var containerAction = result.Execution as ContainerActionExecutionData;
+                Assert.NotNull(containerAction);
+                Assert.Equal("${{ inputs.entryPoint }}", containerAction.Environment[1].Value.ToString());
+            }
+            finally
+            {
+                Teardown();
+            }
+        }
+
         private void Setup([CallerMemberName] string name = "")
         {
             _ecTokenSource?.Dispose();

src/Test/L0/Worker/ActionManifestParserComparisonL0.cs

@@ -379,6 +379,40 @@ namespace GitHub.Runner.Common.Tests.Worker
             }
         }
 
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void Load_BothParsersRejectInvalidExpressionContext()
+        {
+            try
+            {
+                // Arrange — regression test: both parsers must reject github context
+                // in container-runs-env (only inputs is allowed per schema)
+                Setup();
+                _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true");
+
+                var legacyManager = new ActionManifestManagerLegacy();
+                legacyManager.Initialize(_hc);
+                _hc.SetSingleton<IActionManifestManagerLegacy>(legacyManager);
+
+                var newManager = new ActionManifestManager();
+                newManager.Initialize(_hc);
+                _hc.SetSingleton<IActionManifestManager>(newManager);
+
+                var wrapper = new ActionManifestManagerWrapper();
+                wrapper.Initialize(_hc);
+
+                var manifestPath = Path.Combine(TestUtil.GetTestDataPath(), "dockerfileaction_env_invalid_context.yml");
+
+                // Act & Assert — both parsers should reject, wrapper should throw
+                Assert.Throws<ArgumentException>(() => wrapper.Load(_ec.Object, manifestPath));
+            }
+            finally
+            {
+                Teardown();
+            }
+        }
+
         private string GetFullExceptionMessage(Exception ex)
         {
             var messages = new List<string>();

src/Test/L0/Worker/PipelineTemplateEvaluatorWrapperL0.cs

@@ -19,6 +19,7 @@ namespace GitHub.Runner.Common.Tests.Worker
     public sealed class PipelineTemplateEvaluatorWrapperL0
     {
         private CancellationTokenSource _ecTokenSource;
+        private CancellationTokenSource _rootTokenSource;
         private Mock<IExecutionContext> _ec;
         private TestHostContext _hc;
 
@@ -65,7 +66,7 @@ namespace GitHub.Runner.Common.Tests.Worker
 
                 var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object, allowServiceContainerCommand: false);
 
-                // Call EvaluateAndCompare directly: the new evaluator cancels the token
+                // Call EvaluateAndCompare directly: the new evaluator cancels the root token
                 // and returns a different value, forcing hasMismatch = true.
                 // Because cancellation flipped during the evaluation window, the
                 // mismatch should be skipped.
@@ -74,7 +75,7 @@ namespace GitHub.Runner.Common.Tests.Worker
                     () => "legacy-value",
                     () =>
                     {
-                        _ecTokenSource.Cancel();
+                        _rootTokenSource.Cancel();
                         return "different-value";
                     },
                     (legacy, @new) => string.Equals(legacy, @new, StringComparison.Ordinal));
@@ -88,6 +89,43 @@ namespace GitHub.Runner.Common.Tests.Worker
             }
         }
 
+        [Fact]
+        [Trait("Level", "L0")]
+        [Trait("Category", "Worker")]
+        public void EvaluateAndCompare_SkipsMismatchRecording_WhenRootCancellationOccursBetweenEvaluators()
+        {
+            // Simulates job-level cancellation firing between legacy and new evaluator runs.
+            // Root is mocked with a separate CancellationTokenSource to exercise the
+            // _context.Root?.CancellationToken path (the job-level token).
+            try
+            {
+                Setup();
+                _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true");
+
+                var wrapper = new PipelineTemplateEvaluatorWrapper(_hc, _ec.Object, allowServiceContainerCommand: false);
+
+                // Legacy evaluator cancels the root token (simulating job cancel) and returns a value.
+                // The new evaluator returns a different value. The mismatch should be skipped.
+                var result = wrapper.EvaluateAndCompare<string, string>(
+                    "TestRootCancellationSkip",
+                    () =>
+                    {
+                        var legacyValue = "legacy-value";
+                        _rootTokenSource.Cancel();
+                        return legacyValue;
+                    },
+                    () => "different-value",
+                    (legacy, @new) => string.Equals(legacy, @new, StringComparison.Ordinal));
+
+                Assert.Equal("legacy-value", result);
+                Assert.False(_ec.Object.Global.HasTemplateEvaluatorMismatch);
+            }
+            finally
+            {
+                Teardown();
+            }
+        }
+
         [Fact]
         [Trait("Level", "L0")]
         [Trait("Category", "Worker")]
@@ -862,6 +900,8 @@ namespace GitHub.Runner.Common.Tests.Worker
         {
             _ecTokenSource?.Dispose();
             _ecTokenSource = new CancellationTokenSource();
+            _rootTokenSource?.Dispose();
+            _rootTokenSource = new CancellationTokenSource();
 
             _hc = new TestHostContext(this, name);
 
@@ -877,6 +917,9 @@ namespace GitHub.Runner.Common.Tests.Worker
                     WriteDebug = true,
                 });
             _ec.Setup(x => x.CancellationToken).Returns(_ecTokenSource.Token);
+            var rootEc = new Mock<IExecutionContext>();
+            rootEc.Setup(x => x.CancellationToken).Returns(_rootTokenSource.Token);
+            _ec.Setup(x => x.Root).Returns(rootEc.Object);
             _ec.Setup(x => x.ExpressionValues).Returns(expressionValues);
             _ec.Setup(x => x.ExpressionFunctions).Returns(expressionFunctions);
             _ec.Setup(x => x.Write(It.IsAny<string>(), It.IsAny<string>())).Callback((string tag, string message) => { _hc.GetTrace().Info($"{tag}{message}"); });
@@ -885,6 +928,7 @@ namespace GitHub.Runner.Common.Tests.Worker
 
         private void Teardown()
         {
+            _rootTokenSource?.Dispose();
             _hc?.Dispose();
         }
     }

src/Test/TestData/dockerfileaction_env_invalid_context.yml

@@ -0,0 +1,13 @@
+name: 'Action With Invalid Context'
+description: 'Docker action that uses github context in env (only inputs is allowed)'
+inputs:
+  my-input:
+    description: 'A test input'
+    required: false
+    default: 'hello'
+runs:
+  using: 'docker'
+  image: 'Dockerfile'
+  env:
+    VALID: '${{ inputs.my-input }}'
+    INVALID: '${{ github.event.repository.private }}'