Skip to content

Commit a78b66a

Browse files
stefanpennerclaude
andcommitted
Fix case-sensitive action dedup allowing duplicate downloads
The deduplication logic introduced in actions#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 actions#3731 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 7711dc5 commit a78b66a

File tree

2 files changed

+96
-4
lines changed

2 files changed

+96
-4
lines changed

src/Runner.Worker/ActionManager.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public sealed class ActionManager : RunnerService, IActionManager
8484
// Stack-local cache: same action (owner/repo@ref) is resolved only once,
8585
// even if it appears at multiple depths in a composite tree.
8686
var resolvedDownloadInfos = batchActionResolution
87-
? new Dictionary<string, WebApi.ActionDownloadInfo>(StringComparer.Ordinal)
87+
? new Dictionary<string, WebApi.ActionDownloadInfo>(StringComparer.OrdinalIgnoreCase)
8888
: null;
8989
var depth = 0;
9090
// We are running at the start of a job
@@ -858,7 +858,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext,
858858

859859
// Convert to action reference
860860
var actionReferences = actions
861-
.GroupBy(x => GetDownloadInfoLookupKey(x))
861+
.GroupBy(x => GetDownloadInfoLookupKey(x), StringComparer.OrdinalIgnoreCase)
862862
.Where(x => !string.IsNullOrEmpty(x.Key))
863863
.Select(x =>
864864
{
@@ -953,7 +953,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext,
953953
}
954954
}
955955

956-
return actionDownloadInfos.Actions;
956+
return new Dictionary<string, WebApi.ActionDownloadInfo>(actionDownloadInfos.Actions, StringComparer.OrdinalIgnoreCase);
957957
}
958958

959959
/// <summary>
@@ -963,7 +963,7 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext,
963963
private async Task ResolveNewActionsAsync(IExecutionContext executionContext, List<Pipelines.ActionStep> actions, Dictionary<string, WebApi.ActionDownloadInfo> resolvedDownloadInfos)
964964
{
965965
var actionsToResolve = new List<Pipelines.ActionStep>();
966-
var pendingKeys = new HashSet<string>(StringComparer.Ordinal);
966+
var pendingKeys = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
967967
foreach (var action in actions)
968968
{
969969
var lookupKey = GetDownloadInfoLookupKey(action);

src/Test/L0/Worker/ActionManagerL0.cs

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,6 +1449,98 @@ public async void PrepareActions_DeduplicatesResolutionAcrossDepthLevels()
14491449
}
14501450
}
14511451

1452+
[Fact]
1453+
[Trait("Level", "L0")]
1454+
[Trait("Category", "Worker")]
1455+
public async void PrepareActions_DeduplicatesCaseInsensitiveActionReferences()
1456+
{
1457+
// Verifies that action references differing only by owner/repo casing
1458+
// are deduplicated and resolved only once.
1459+
// Regression test for https://github.com/actions/runner/issues/3731
1460+
Environment.SetEnvironmentVariable("ACTIONS_BATCH_ACTION_RESOLUTION", "true");
1461+
try
1462+
{
1463+
//Arrange
1464+
Setup();
1465+
// Each action step with pre+post needs 2 IActionRunner instances (pre + post)
1466+
for (int i = 0; i < 6; i++)
1467+
{
1468+
_hc.EnqueueInstance<IActionRunner>(new Mock<IActionRunner>().Object);
1469+
}
1470+
1471+
var allResolvedKeys = new List<string>();
1472+
_jobServer.Setup(x => x.ResolveActionDownloadInfoAsync(It.IsAny<Guid>(), It.IsAny<string>(), It.IsAny<Guid>(), It.IsAny<Guid>(), It.IsAny<ActionReferenceList>(), It.IsAny<CancellationToken>()))
1473+
.Returns((Guid scopeIdentifier, string hubName, Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken) =>
1474+
{
1475+
var result = new ActionDownloadInfoCollection { Actions = new Dictionary<string, ActionDownloadInfo>() };
1476+
foreach (var action in actions.Actions)
1477+
{
1478+
var key = $"{action.NameWithOwner}@{action.Ref}";
1479+
allResolvedKeys.Add(key);
1480+
result.Actions[key] = new ActionDownloadInfo
1481+
{
1482+
NameWithOwner = action.NameWithOwner,
1483+
Ref = action.Ref,
1484+
ResolvedNameWithOwner = action.NameWithOwner,
1485+
ResolvedSha = $"{action.Ref}-sha",
1486+
TarballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/tarball/{action.Ref}",
1487+
ZipballUrl = $"https://api.github.com/repos/{action.NameWithOwner}/zipball/{action.Ref}",
1488+
};
1489+
}
1490+
return Task.FromResult(result);
1491+
});
1492+
1493+
var actions = new List<Pipelines.ActionStep>
1494+
{
1495+
new Pipelines.ActionStep()
1496+
{
1497+
Name = "action1",
1498+
Id = Guid.NewGuid(),
1499+
Reference = new Pipelines.RepositoryPathReference()
1500+
{
1501+
Name = "TingluoHuang/runner_L0",
1502+
Ref = "RepositoryActionWithWrapperActionfile_Node",
1503+
RepositoryType = "GitHub"
1504+
}
1505+
},
1506+
new Pipelines.ActionStep()
1507+
{
1508+
Name = "action2",
1509+
Id = Guid.NewGuid(),
1510+
Reference = new Pipelines.RepositoryPathReference()
1511+
{
1512+
Name = "tingluohuang/RUNNER_L0",
1513+
Ref = "RepositoryActionWithWrapperActionfile_Node",
1514+
RepositoryType = "GitHub"
1515+
}
1516+
},
1517+
new Pipelines.ActionStep()
1518+
{
1519+
Name = "action3",
1520+
Id = Guid.NewGuid(),
1521+
Reference = new Pipelines.RepositoryPathReference()
1522+
{
1523+
Name = "TINGLUOHUANG/Runner_L0",
1524+
Ref = "RepositoryActionWithWrapperActionfile_Node",
1525+
RepositoryType = "GitHub"
1526+
}
1527+
}
1528+
};
1529+
1530+
//Act
1531+
await _actionManager.PrepareActionsAsync(_ec.Object, actions);
1532+
1533+
//Assert
1534+
// All three references should deduplicate to a single resolve call
1535+
Assert.Equal(1, allResolvedKeys.Count);
1536+
}
1537+
finally
1538+
{
1539+
Environment.SetEnvironmentVariable("ACTIONS_BATCH_ACTION_RESOLUTION", null);
1540+
Teardown();
1541+
}
1542+
}
1543+
14521544
[Fact]
14531545
[Trait("Level", "L0")]
14541546
[Trait("Category", "Worker")]

0 commit comments

Comments
 (0)