From 8a55f0b87f13f6dd1e454a69281afb8adba5c6c1 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Tue, 7 Apr 2026 21:59:36 -0600 Subject: [PATCH 1/3] Fix case-sensitive action dedup allowing duplicate downloads The deduplication logic introduced in #4296 used StringComparer.Ordinal, so action references differing only by owner/repo casing (e.g. actions/checkout vs actIONS/checkout) were treated as distinct and downloaded multiple times. Switch to OrdinalIgnoreCase for all dedup collections and the GroupBy in GetDownloadInfoAsync. Fixes #3731 Co-Authored-By: Claude Opus 4.6 --- src/Runner.Worker/ActionManager.cs | 8 +-- src/Test/L0/Worker/ActionManagerL0.cs | 92 +++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/src/Runner.Worker/ActionManager.cs b/src/Runner.Worker/ActionManager.cs index 133129a0177..c5ada62e390 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(StringComparer.OrdinalIgnoreCase) : null; var depth = 0; // We are running at the start of a job @@ -858,7 +858,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext, // Convert to action reference var actionReferences = actions - .GroupBy(x => GetDownloadInfoLookupKey(x)) + .GroupBy(x => GetDownloadInfoLookupKey(x), StringComparer.OrdinalIgnoreCase) .Where(x => !string.IsNullOrEmpty(x.Key)) .Select(x => { @@ -958,7 +958,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext, } } - return actionDownloadInfos.Actions; + return new Dictionary(actionDownloadInfos.Actions, StringComparer.OrdinalIgnoreCase); } /// @@ -968,7 +968,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(StringComparer.OrdinalIgnoreCase); foreach (var action in actions) { var lookupKey = GetDownloadInfoLookupKey(action); diff --git a/src/Test/L0/Worker/ActionManagerL0.cs b/src/Test/L0/Worker/ActionManagerL0.cs index c612ac9d0fa..07f0a817581 100644 --- a/src/Test/L0/Worker/ActionManagerL0.cs +++ b/src/Test/L0/Worker/ActionManagerL0.cs @@ -1453,6 +1453,98 @@ 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); + } + finally + { + Environment.SetEnvironmentVariable("ACTIONS_BATCH_ACTION_RESOLUTION", null); + Teardown(); + } + } + [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] From fad1b2bd9d226581c04f30f84a87f9dedd95bddd Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Tue, 7 Apr 2026 22:09:50 -0600 Subject: [PATCH 2/3] Normalize action references after case-insensitive dedup On case-sensitive filesystems (Linux), after dedup merges differently-cased references into one download, PrepareRepositoryActionAsync and LoadAction would look for directories using the original (non-canonical) casing and fail to find the action. Fix by normalizing each action step's repositoryReference.Name to match downloadInfo.NameWithOwner after dedup lookup, in both the batch and legacy code paths. Also makes the empty-dictionary return path in GetDownloadInfoAsync use OrdinalIgnoreCase for consistency. Co-Authored-By: Claude Opus 4.6 --- src/Runner.Worker/ActionManager.cs | 24 +++++++++++++++++++++++- src/Test/L0/Worker/ActionManagerL0.cs | 15 +++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/Runner.Worker/ActionManager.cs b/src/Runner.Worker/ActionManager.cs index c5ada62e390..26d49f649ed 100644 --- a/src/Runner.Worker/ActionManager.cs +++ b/src/Runner.Worker/ActionManager.cs @@ -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); } @@ -877,7 +883,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext, // Nothing to resolve? if (actionReferences.Count == 0) { - return new Dictionary(); + return new Dictionary(StringComparer.OrdinalIgnoreCase); } // Pass lockfile dependencies to Launch when present, so it can @@ -1347,6 +1353,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) diff --git a/src/Test/L0/Worker/ActionManagerL0.cs b/src/Test/L0/Worker/ActionManagerL0.cs index 07f0a817581..0e37bfc5336 100644 --- a/src/Test/L0/Worker/ActionManagerL0.cs +++ b/src/Test/L0/Worker/ActionManagerL0.cs @@ -1537,6 +1537,21 @@ public async void PrepareActions_DeduplicatesCaseInsensitiveActionReferences() //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 { From edbaa3616d509c9d881aca9458e85185795556a1 Mon Sep 17 00:00:00 2001 From: Stefan Penner Date: Tue, 7 Apr 2026 22:35:38 -0600 Subject: [PATCH 3/3] Use custom comparer to keep ref case-sensitive in dedup keys Replace StringComparer.OrdinalIgnoreCase with ActionLookupKeyComparer that treats the owner/repo portion of "{owner/repo}@{ref}" keys case-insensitively while keeping the ref portion case-sensitive, since git refs are case-sensitive. Co-Authored-By: Claude Opus 4.6 --- src/Runner.Worker/ActionManager.cs | 45 ++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/src/Runner.Worker/ActionManager.cs b/src/Runner.Worker/ActionManager.cs index 26d49f649ed..52177e0a8f3 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.OrdinalIgnoreCase) + ? new Dictionary(ActionLookupKeyComparer.Instance) : null; var depth = 0; // We are running at the start of a job @@ -864,7 +864,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext, // Convert to action reference var actionReferences = actions - .GroupBy(x => GetDownloadInfoLookupKey(x), StringComparer.OrdinalIgnoreCase) + .GroupBy(x => GetDownloadInfoLookupKey(x), ActionLookupKeyComparer.Instance) .Where(x => !string.IsNullOrEmpty(x.Key)) .Select(x => { @@ -883,7 +883,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext, // Nothing to resolve? if (actionReferences.Count == 0) { - return new Dictionary(StringComparer.OrdinalIgnoreCase); + return new Dictionary(ActionLookupKeyComparer.Instance); } // Pass lockfile dependencies to Launch when present, so it can @@ -964,7 +964,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext, } } - return new Dictionary(actionDownloadInfos.Actions, StringComparer.OrdinalIgnoreCase); + return new Dictionary(actionDownloadInfos.Actions, ActionLookupKeyComparer.Instance); } /// @@ -974,7 +974,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.OrdinalIgnoreCase); + var pendingKeys = new HashSet(ActionLookupKeyComparer.Instance); foreach (var action in actions) { var lookupKey = GetDownloadInfoLookupKey(action); @@ -1394,6 +1394,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))