actions/runner
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix: remove stale flag gate, validate repo format, add edge case tests

Browse Source 
- Remove AddCheckRunIdToJobContext flag gate from job context hydration.
  The flag is permanently force-true server-side (acquirejobhandler.go:1658).
  The server controls what to send via its own feature flags.

- Add owner/repo format validation to DeriveWorkflowRefComponents()

- Add tests for PR merge refs, tag refs, invalid repo format, and
  .github repo names

- Update check_run_id flag-disabled test to reflect always-copy behavior
This commit is contained in:
Salman Muin Kayser Chishti
2026-04-10 14:04:28 +00:00
committed by GitHub
parent 4342a1b8f0
commit 04ee71a5d1
4 changed files with 72 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Runner.Worker/ExecutionContext.cs
+8 -11
View File
@@ -892,21 +892,18 @@ namespace GitHub.Runner.Worker
Trace.Info("Initializing Job context");
var jobContext = new JobContext();
if (Global.Variables.GetBoolean(Constants.Runner.Features.AddCheckRunIdToJobContext) ?? false)
ExpressionValues.TryGetValue("job", out var jobDictionary);
if (jobDictionary != null)
{
ExpressionValues.TryGetValue("job", out var jobDictionary);
if (jobDictionary != null)
foreach (var pair in jobDictionary.AssertDictionary("job"))
{
foreach (var pair in jobDictionary.AssertDictionary("job"))
{
jobContext[pair.Key] = pair.Value;
}
jobContext[pair.Key] = pair.Value;
}
// Derive workflow_repository and workflow_file_path from workflow_ref
// if the server sent workflow_ref but not the decomposed fields
jobContext.DeriveWorkflowRefComponents();
}
// Derive workflow_repository and workflow_file_path from workflow_ref
// if the server sent workflow_ref but not the decomposed fields
jobContext.DeriveWorkflowRefComponents();
ExpressionValues["job"] = jobContext;
Trace.Info("Initialize GitHub context");
src/Runner.Worker/JobContext.cs
+12 -2
View File
@@ -172,14 +172,24 @@ namespace GitHub.Runner.Worker
return;
}
var repo = pathPart.Substring(0, markerIndex);
var filePath = pathPart.Substring(markerIndex + 1); // skip leading '/'
// Validate repo is owner/repo format (must have at least one slash with non-empty segments)
var slashIndex = repo.IndexOf('/');
if (slashIndex <= 0 || slashIndex >= repo.Length - 1)
{
return;
}
if (WorkflowRepository == null || WorkflowRepository == "")
{
WorkflowRepository = pathPart.Substring(0, markerIndex);
WorkflowRepository = repo;
}
if (WorkflowFilePath == null || WorkflowFilePath == "")
{
WorkflowFilePath = pathPart.Substring(markerIndex + 1); // skip leading '/'
WorkflowFilePath = filePath;
}
}
}
src/Test/L0/Worker/ExecutionContextL0.cs
+18 -30
View File
@@ -1203,19 +1203,19 @@ namespace GitHub.Runner.Common.Tests.Worker
}
}
// TODO: this test can be deleted when `AddCheckRunIdToJobContext` is fully rolled out
// AddCheckRunIdToJobContext is now permanently enabled server-side (hardcoded to "true"
// in acquirejobhandler.go). The runner always copies ContextData["job"] entries, so the
// flag-disabled test is no longer applicable. Replaced with a test that verifies
// check_run_id is always hydrated regardless of the flag value.
[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
public void InitializeJob_HydratesJobContextWithCheckRunId_FeatureFlagDisabled()
public void InitializeJob_HydratesJobContextWithCheckRunId_AlwaysCopied()
{
using (TestHostContext hc = CreateTestContext())
{
// Arrange: Create a job request message and make sure the feature flag is disabled
var variables = new Dictionary<string, VariableValue>()
{
[Constants.Runner.Features.AddCheckRunIdToJobContext] = new VariableValue("false"),
};
// Arrange: No feature flag set at all
var variables = new Dictionary<string, VariableValue>();
var jobRequest = new Pipelines.AgentJobRequestMessage(new TaskOrchestrationPlanReference(), new TimelineReference(), Guid.NewGuid(), "some job name", "some job name", null, null, null, variables, new List<MaskHint>(), new Pipelines.JobResources(), new Pipelines.ContextData.DictionaryContextData(), new Pipelines.WorkspaceOptions(), new List<Pipelines.ActionStep>(), null, null, null, null, null);
var pagingLogger = new Moq.Mock<IPagingLogger>();
var jobServerQueue = new Moq.Mock<IJobServerQueue>();
@@ -1233,9 +1233,9 @@ namespace GitHub.Runner.Common.Tests.Worker
// Act
ec.InitializeJob(jobRequest, CancellationToken.None);
// Assert
// Assert: check_run_id is always copied regardless of flag
Assert.NotNull(ec.JobContext);
Assert.Null(ec.JobContext.CheckRunId); // with the feature flag disabled we should not have added a CheckRunId to the JobContext
Assert.Equal(123456, ec.JobContext.CheckRunId);
}
}
@@ -1246,11 +1246,8 @@ namespace GitHub.Runner.Common.Tests.Worker
{
using (TestHostContext hc = CreateTestContext())
{
// Arrange: Create a job request message with the feature flag enabled
var variables = new Dictionary<string, VariableValue>()
{
[Constants.Runner.Features.AddCheckRunIdToJobContext] = new VariableValue("true"),
};
// Arrange
var variables = new Dictionary<string, VariableValue>();
var jobRequest = new Pipelines.AgentJobRequestMessage(new TaskOrchestrationPlanReference(), new TimelineReference(), Guid.NewGuid(), "some job name", "some job name", null, null, null, variables, new List<MaskHint>(), new Pipelines.JobResources(), new Pipelines.ContextData.DictionaryContextData(), new Pipelines.WorkspaceOptions(), new List<Pipelines.ActionStep>(), null, null, null, null, null);
var pagingLogger = new Moq.Mock<IPagingLogger>();
var jobServerQueue = new Moq.Mock<IJobServerQueue>();
@@ -1283,15 +1280,12 @@ namespace GitHub.Runner.Common.Tests.Worker
[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
public void InitializeJob_WorkflowIdentityNotSet_WhenFeatureFlagDisabled()
public void InitializeJob_WorkflowIdentityNotSet_WhenServerSendsNoData()
{
using (TestHostContext hc = CreateTestContext())
{
// Arrange: Create a job request message with the feature flag disabled
var variables = new Dictionary<string, VariableValue>()
{
[Constants.Runner.Features.AddCheckRunIdToJobContext] = new VariableValue("false"),
};
// Arrange: Server sends no workflow identity in job context
var variables = new Dictionary<string, VariableValue>();
var jobRequest = new Pipelines.AgentJobRequestMessage(new TaskOrchestrationPlanReference(), new TimelineReference(), Guid.NewGuid(), "some job name", "some job name", null, null, null, variables, new List<MaskHint>(), new Pipelines.JobResources(), new Pipelines.ContextData.DictionaryContextData(), new Pipelines.WorkspaceOptions(), new List<Pipelines.ActionStep>(), null, null, null, null, null);
var pagingLogger = new Moq.Mock<IPagingLogger>();
var jobServerQueue = new Moq.Mock<IJobServerQueue>();
@@ -1300,17 +1294,14 @@ namespace GitHub.Runner.Common.Tests.Worker
var ec = new Runner.Worker.ExecutionContext();
ec.Initialize(hc);
// Arrange: Add workflow identity to the job context
var jobContext = new Pipelines.ContextData.DictionaryContextData();
jobContext["workflow_ref"] = new StringContextData("my-org/my-repo/.github/workflows/reusable.yml@refs/heads/main");
jobContext["workflow_sha"] = new StringContextData("abc123def456");
jobRequest.ContextData["job"] = jobContext;
// Arrange: empty job context
jobRequest.ContextData["job"] = new Pipelines.ContextData.DictionaryContextData();
jobRequest.ContextData["github"] = new Pipelines.ContextData.DictionaryContextData();
// Act
ec.InitializeJob(jobRequest, CancellationToken.None);
// Assert: properties should not be populated when flag is off
// Assert: no workflow identity
Assert.NotNull(ec.JobContext);
Assert.Null(ec.JobContext.WorkflowRef);
Assert.Null(ec.JobContext.WorkflowSha);
@@ -1327,10 +1318,7 @@ namespace GitHub.Runner.Common.Tests.Worker
using (TestHostContext hc = CreateTestContext())
{
// Arrange: Server sends all 4 fields explicitly
var variables = new Dictionary<string, VariableValue>()
{
[Constants.Runner.Features.AddCheckRunIdToJobContext] = new VariableValue("true"),
};
var variables = new Dictionary<string, VariableValue>();
var jobRequest = new Pipelines.AgentJobRequestMessage(new TaskOrchestrationPlanReference(), new TimelineReference(), Guid.NewGuid(), "some job name", "some job name", null, null, null, variables, new List<MaskHint>(), new Pipelines.JobResources(), new Pipelines.ContextData.DictionaryContextData(), new Pipelines.WorkspaceOptions(), new List<Pipelines.ActionStep>(), null, null, null, null, null);
var pagingLogger = new Moq.Mock<IPagingLogger>();
var jobServerQueue = new Moq.Mock<IJobServerQueue>();
src/Test/L0/Worker/JobContextL0.cs
+34
View File
@@ -219,5 +219,39 @@ namespace GitHub.Runner.Common.Tests.Worker
Assert.Equal("my-org/my-repo", ctx.WorkflowRepository);
Assert.Equal(".github/workflows/deploy.yml", ctx.WorkflowFilePath);
}
[Fact]
public void DeriveWorkflowRefComponents_HandlesPRMergeRef()
{
var ctx = new JobContext();
ctx.WorkflowRef = "my-org/my-repo/.github/workflows/ci.yml@refs/pull/42/merge";
ctx.DeriveWorkflowRefComponents();
Assert.Equal("my-org/my-repo", ctx.WorkflowRepository);
Assert.Equal(".github/workflows/ci.yml", ctx.WorkflowFilePath);
}
[Fact]
public void DeriveWorkflowRefComponents_HandlesTagRef()
{
var ctx = new JobContext();
ctx.WorkflowRef = "my-org/my-repo/.github/workflows/release.yml@refs/tags/v1.0.0";
ctx.DeriveWorkflowRefComponents();
Assert.Equal("my-org/my-repo", ctx.WorkflowRepository);
Assert.Equal(".github/workflows/release.yml", ctx.WorkflowFilePath);
}
[Fact]
public void DeriveWorkflowRefComponents_RejectsInvalidRepoFormat()
{
// No owner/repo slash — should no-op
var ctx = new JobContext();
ctx.WorkflowRef = "noslash.github/workflows/ci.yml@refs/heads/main";
ctx.DeriveWorkflowRefComponents();
Assert.Null(ctx.WorkflowRepository);
Assert.Null(ctx.WorkflowFilePath);
}
}
}