diff --git a/src/Runner.Worker/ActionManager.cs b/src/Runner.Worker/ActionManager.cs index 79c0de5f706..609c3a18211 100644 --- a/src/Runner.Worker/ActionManager.cs +++ b/src/Runner.Worker/ActionManager.cs @@ -84,7 +84,7 @@ public sealed class ActionManager : RunnerService, IActionManager // Stack-local cache: same action (owner/repo@ref) is resolved only once, // even if it appears at multiple depths in a composite tree. var resolvedDownloadInfos = batchActionResolution - ? new Dictionary(StringComparer.Ordinal) + ? new Dictionary(ActionLookupKeyComparer.Instance) : null; var depth = 0; // We are running at the start of a job @@ -228,6 +228,9 @@ public sealed class ActionManager : RunnerService, IActionManager { throw new Exception($"Missing download info for {lookupKey}"); } + // Normalize the reference name to match the resolved download info so that + // directory paths are consistent on case-sensitive filesystems (Linux). + NormalizeRepositoryReference(action, downloadInfo); await DownloadRepositoryActionAsync(executionContext, downloadInfo); } @@ -414,6 +417,9 @@ public sealed class ActionManager : RunnerService, IActionManager throw new Exception($"Missing download info for {lookupKey}"); } + // Normalize the reference name to match the resolved download info so that + // directory paths are consistent on case-sensitive filesystems (Linux). + NormalizeRepositoryReference(action, downloadInfo); await DownloadRepositoryActionAsync(executionContext, downloadInfo); } @@ -858,7 +864,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext, // Convert to action reference var actionReferences = actions - .GroupBy(x => GetDownloadInfoLookupKey(x)) + .GroupBy(x => GetDownloadInfoLookupKey(x), ActionLookupKeyComparer.Instance) .Where(x => !string.IsNullOrEmpty(x.Key)) .Select(x => { @@ -877,7 +883,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext, // Nothing to resolve? if (actionReferences.Count == 0) { - return new Dictionary(); + return new Dictionary(ActionLookupKeyComparer.Instance); } // Resolve download info @@ -953,7 +959,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext, } } - return actionDownloadInfos.Actions; + return new Dictionary(actionDownloadInfos.Actions, ActionLookupKeyComparer.Instance); } /// @@ -963,7 +969,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext, private async Task ResolveNewActionsAsync(IExecutionContext executionContext, List actions, Dictionary resolvedDownloadInfos) { var actionsToResolve = new List(); - var pendingKeys = new HashSet(StringComparer.Ordinal); + var pendingKeys = new HashSet(ActionLookupKeyComparer.Instance); foreach (var action in actions) { var lookupKey = GetDownloadInfoLookupKey(action); @@ -1342,6 +1348,22 @@ private ActionSetupInfo PrepareRepositoryActionAsync(IExecutionContext execution } } + /// + /// After case-insensitive deduplication, the resolved download info may use + /// a different casing than the action step's reference. Normalize the reference + /// so that directory paths (used by DownloadRepositoryActionAsync, + /// PrepareRepositoryActionAsync, and LoadAction) are consistent on + /// case-sensitive filesystems. + /// + private static void NormalizeRepositoryReference(Pipelines.ActionStep action, WebApi.ActionDownloadInfo downloadInfo) + { + if (action.Reference is Pipelines.RepositoryPathReference repoRef && + !string.Equals(repoRef.Name, downloadInfo.NameWithOwner, StringComparison.Ordinal)) + { + repoRef.Name = downloadInfo.NameWithOwner; + } + } + private static string GetDownloadInfoLookupKey(Pipelines.ActionStep action) { if (action.Reference.Type != Pipelines.ActionSourceType.Repository) @@ -1367,6 +1389,41 @@ private static string GetDownloadInfoLookupKey(Pipelines.ActionStep action) return $"{repositoryReference.Name}@{repositoryReference.Ref}"; } + /// + /// Compares action lookup keys ("{owner/repo}@{ref}") with case-insensitive + /// owner/repo (GitHub names are case-insensitive) and case-sensitive ref + /// (git refs are case-sensitive). + /// + private sealed class ActionLookupKeyComparer : IEqualityComparer + { + public static readonly ActionLookupKeyComparer Instance = new(); + + public bool Equals(string x, string y) + { + if (ReferenceEquals(x, y)) return true; + if (x is null || y is null) return false; + + var xAt = x.LastIndexOf('@'); + var yAt = y.LastIndexOf('@'); + if (xAt < 0 || yAt < 0) return StringComparer.OrdinalIgnoreCase.Equals(x, y); + + // Name (owner/repo) is case-insensitive; @ref is case-sensitive + return x.AsSpan(0, xAt).Equals(y.AsSpan(0, yAt), StringComparison.OrdinalIgnoreCase) + && x.AsSpan(xAt).Equals(y.AsSpan(yAt), StringComparison.Ordinal); + } + + public int GetHashCode(string obj) + { + if (obj is null) return 0; + var at = obj.LastIndexOf('@'); + if (at < 0) return StringComparer.OrdinalIgnoreCase.GetHashCode(obj); + + return HashCode.Combine( + StringComparer.OrdinalIgnoreCase.GetHashCode(obj.Substring(0, at)), + StringComparer.Ordinal.GetHashCode(obj.Substring(at))); + } + } + private AuthenticationHeaderValue CreateAuthHeader(IExecutionContext executionContext, string downloadUrl, string token) { if (string.IsNullOrEmpty(token)) diff --git a/src/Test/L0/Worker/ActionManagerL0.cs b/src/Test/L0/Worker/ActionManagerL0.cs index 501402a88b4..7d404380cdd 100644 --- a/src/Test/L0/Worker/ActionManagerL0.cs +++ b/src/Test/L0/Worker/ActionManagerL0.cs @@ -1449,6 +1449,113 @@ public async void PrepareActions_DeduplicatesResolutionAcrossDepthLevels() } } + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async void PrepareActions_DeduplicatesCaseInsensitiveActionReferences() + { + // Verifies that action references differing only by owner/repo casing + // are deduplicated and resolved only once. + // Regression test for https://github.com/actions/runner/issues/3731 + Environment.SetEnvironmentVariable("ACTIONS_BATCH_ACTION_RESOLUTION", "true"); + try + { + //Arrange + Setup(); + // Each action step with pre+post needs 2 IActionRunner instances (pre + post) + for (int i = 0; i < 6; i++) + { + _hc.EnqueueInstance(new Mock().Object); + } + + var allResolvedKeys = new List(); + _jobServer.Setup(x => x.ResolveActionDownloadInfoAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((Guid scopeIdentifier, string hubName, Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken) => + { + var result = new ActionDownloadInfoCollection { Actions = new Dictionary() }; + foreach (var action in actions.Actions) + { + var key = $"{action.NameWithOwner}@{action.Ref}"; + allResolvedKeys.Add(key); + result.Actions[key] = new ActionDownloadInfo + { + NameWithOwner = action.NameWithOwner, + Ref = action.Ref, + ResolvedNameWithOwner = action.NameWithOwner, + ResolvedSha = $"{action.Ref}-sha", + TarballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/tarball/{action.Ref}", + ZipballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/zipball/{action.Ref}", + }; + } + return Task.FromResult(result); + }); + + var actions = new List + { + new Pipelines.ActionStep() + { + Name = "action1", + Id = Guid.NewGuid(), + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TingluoHuang/runner_L0", + Ref = "RepositoryActionWithWrapperActionfile_Node", + RepositoryType = "GitHub" + } + }, + new Pipelines.ActionStep() + { + Name = "action2", + Id = Guid.NewGuid(), + Reference = new Pipelines.RepositoryPathReference() + { + Name = "tingluohuang/RUNNER_L0", + Ref = "RepositoryActionWithWrapperActionfile_Node", + RepositoryType = "GitHub" + } + }, + new Pipelines.ActionStep() + { + Name = "action3", + Id = Guid.NewGuid(), + Reference = new Pipelines.RepositoryPathReference() + { + Name = "TINGLUOHUANG/Runner_L0", + Ref = "RepositoryActionWithWrapperActionfile_Node", + RepositoryType = "GitHub" + } + } + }; + + //Act + await _actionManager.PrepareActionsAsync(_ec.Object, actions); + + //Assert + // All three references should deduplicate to a single resolve call + Assert.Equal(1, allResolvedKeys.Count); + + // Verify all actions are usable: the download went to one directory and + // reference normalization ensures all three steps find the action there. + // The completed watermark proves the download + prepare succeeded. + Assert.True(File.Exists(Path.Combine( + _hc.GetDirectory(WellKnownDirectory.Actions), + "TingluoHuang/runner_L0", + "RepositoryActionWithWrapperActionfile_Node.completed"))); + + // Verify the references were normalized to the canonical casing + foreach (var action in actions) + { + var repoRef = action.Reference as Pipelines.RepositoryPathReference; + Assert.Equal("TingluoHuang/runner_L0", repoRef.Name); + } + } + finally + { + Environment.SetEnvironmentVariable("ACTIONS_BATCH_ACTION_RESOLUTION", null); + Teardown(); + } + } + [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")]