diff --git a/src/Runner.Worker/ActionManifestManagerWrapper.cs b/src/Runner.Worker/ActionManifestManagerWrapper.cs index 6d893fd8252..cd673d5de00 100644 --- a/src/Runner.Worker/ActionManifestManagerWrapper.cs +++ b/src/Runner.Worker/ActionManifestManagerWrapper.cs @@ -274,6 +274,20 @@ private GitHub.DistributedTask.Pipelines.ActionStepDefinitionReference ParseActi }; } + // Self-repository reference: $/ or $/path/to/action + if (GitHub.DistributedTask.Pipelines.PipelineConstants.TryParseSelfRepository(uses, out var selfPath, out var selfError)) + { + return new GitHub.DistributedTask.Pipelines.RepositoryPathReference + { + RepositoryType = GitHub.DistributedTask.Pipelines.PipelineConstants.SelfRepositoryAlias, + Path = selfPath + }; + } + if (selfError != null) + { + throw new ArgumentException(selfError, nameof(uses)); + } + // Repository reference: owner/repo@ref or owner/repo/path@ref var atIndex = uses.LastIndexOf('@'); string refPart = null; diff --git a/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConverter.cs b/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConverter.cs index e8c5e91afe7..0381463f3d5 100644 --- a/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConverter.cs +++ b/src/Sdk/DTPipelines/Pipelines/ObjectTemplating/PipelineTemplateConverter.cs @@ -611,7 +611,7 @@ private static ActionStep ConvertToStep( Path = uses.Value }; } - else if (PipelineConstants.TryParseSelfRepository(uses.Value, out var selfPath)) + else if (PipelineConstants.TryParseSelfRepository(uses.Value, out var selfPath, out var selfError)) { result.Reference = new RepositoryPathReference { @@ -619,6 +619,11 @@ private static ActionStep ConvertToStep( Path = selfPath }; } + else if (selfError != null) + { + context.Error(uses, selfError); + return null; + } else { var usesSegments = uses.Value.Split('@'); diff --git a/src/Sdk/DTPipelines/Pipelines/PipelineConstants.cs b/src/Sdk/DTPipelines/Pipelines/PipelineConstants.cs index b2928520cd7..1f836e5c125 100644 --- a/src/Sdk/DTPipelines/Pipelines/PipelineConstants.cs +++ b/src/Sdk/DTPipelines/Pipelines/PipelineConstants.cs @@ -55,23 +55,27 @@ public static class PipelineConstants public const String SelfRepositoryPrefix = "$/"; /// - /// Returns true if the uses value is a self-repository reference (starts with $/), - /// and outputs the subpath after the prefix. + /// Returns true if the uses value is a valid self-repository reference (starts with $/), + /// and outputs the normalized subpath after the prefix. /// - public static bool TryParseSelfRepository(string usesValue, out string path) + public static bool TryParseSelfRepository(string usesValue, out string path, out string error) { if (usesValue != null && usesValue.StartsWith(SelfRepositoryPrefix, StringComparison.Ordinal)) { path = usesValue.Substring(SelfRepositoryPrefix.Length).TrimStart('/'); - if (string.IsNullOrEmpty(path)) + if (path.Contains('@')) { path = null; + error = $"Self-repository references do not support an '@ref' suffix. Actual '{usesValue}'"; return false; } + + error = null; return true; } path = null; + error = null; return false; } diff --git a/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs b/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs index 7f344c4de63..8e26ebddd31 100644 --- a/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs +++ b/src/Sdk/WorkflowParser/Conversion/WorkflowTemplateConverter.cs @@ -1605,7 +1605,8 @@ public static List ConvertToSteps( { id = WorkflowConstants.SelfAlias; } - else if (GitHub.DistributedTask.Pipelines.PipelineConstants.TryParseSelfRepository(action.Uses!.Value, out _)) + else if (GitHub.DistributedTask.Pipelines.PipelineConstants.TryParseSelfRepository(action.Uses!.Value, out _, out var selfError) || + selfError != null) { id = WorkflowConstants.SelfRepositoryAlias; } @@ -1757,9 +1758,15 @@ private static IStep ConvertToStep( With = with, }; - if (!uses.Value.StartsWith(WorkflowTemplateConstants.DockerUriPrefix, StringComparison.Ordinal) && + var isSelfRepository = GitHub.DistributedTask.Pipelines.PipelineConstants.TryParseSelfRepository(uses.Value, out _, out var selfError); + if (selfError != null) + { + context.Error(uses, selfError); + } + else if (!uses.Value.StartsWith(WorkflowTemplateConstants.DockerUriPrefix, StringComparison.Ordinal) && !uses.Value.StartsWith("./") && - !uses.Value.StartsWith(".\\")) + !uses.Value.StartsWith(".\\") && + !isSelfRepository) { var usesSegments = uses.Value.Split('@'); var pathSegments = usesSegments[0].Split(new[] { '/', '\\' }, StringSplitOptions.RemoveEmptyEntries); diff --git a/src/Test/L0/Worker/ActionManagerL0.cs b/src/Test/L0/Worker/ActionManagerL0.cs index 9b3138e5ba0..19626e59376 100644 --- a/src/Test/L0/Worker/ActionManagerL0.cs +++ b/src/Test/L0/Worker/ActionManagerL0.cs @@ -3649,15 +3649,12 @@ await Assert.ThrowsAsync(async () => [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] - public async void PrepareActions_SelfRepository_ResolvesNestedInComposite() + public async void PrepareActions_SelfRepository_ResolvesBareRootNestedInComposite() { - // Composite action at $/actions/parent uses $/actions/child (same repo). - // This tests the batch path fix: $/ refs in nextLevel must be resolved + // Composite action at $/actions/parent uses the same repository's root action. + // This tests that a bare $/ in nextLevel is resolved // BEFORE ResolveNewActionsAsync, otherwise GetDownloadInfoLookupKey throws. - // We pre-stage only the parent action.yml on disk so the composite steps - // are discovered, but we DON'T stage the child — a download failure for - // the child is fine; the important thing is that $/ was resolved - // (no InvalidOperationException from GetDownloadInfoLookupKey). + // Both manifests are pre-staged in the downloaded repository archive. Environment.SetEnvironmentVariable("ACTIONS_BATCH_ACTION_RESOLUTION", "true"); try { @@ -3674,7 +3671,7 @@ public async void PrepareActions_SelfRepository_ResolvesNestedInComposite() jobContext.WorkflowSha = RepoSha; _ec.Setup(x => x.JobContext).Returns(jobContext); - // Stage parent action on disk as a composite that uses $/actions/child. + // Stage parent action on disk as a composite that uses the repository root. // We use rootStepId != default to avoid directory deletion, // and create the watermark + action.yml in the expected location. string actionsDir = Path.Combine(_workFolder, Constants.Path.ActionsDirectory); @@ -3686,11 +3683,10 @@ public async void PrepareActions_SelfRepository_ResolvesNestedInComposite() runs: using: 'composite' steps: - - uses: $/actions/child + - uses: $/ "); - // Stage child action too (as a leaf node action) - Directory.CreateDirectory(Path.Combine(destDir, "actions", "child")); - File.WriteAllText(Path.Combine(destDir, "actions", "child", Constants.Path.ActionManifestYmlFile), @" + // Stage the repository-root action too (as a leaf node action) + File.WriteAllText(Path.Combine(destDir, Constants.Path.ActionManifestYmlFile), @" name: 'Child' description: 'Node child' runs: diff --git a/src/Test/L0/Worker/ActionManifestManagerL0.cs b/src/Test/L0/Worker/ActionManifestManagerL0.cs index 6a3da0c7209..ed76f570807 100644 --- a/src/Test/L0/Worker/ActionManifestManagerL0.cs +++ b/src/Test/L0/Worker/ActionManifestManagerL0.cs @@ -786,6 +786,72 @@ public void Load_ConditionalCompositeAction() } } + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Load_SelfRepositoryCompositeAction() + { + try + { + //Arrange + Setup(); + + var actionManifest = new ActionManifestManager(); + actionManifest.Initialize(_hc); + + //Act + var result = actionManifest.Load(_ec.Object, Path.Combine(TestUtil.GetTestDataPath(), "self_repository_composite_action.yml")); + + //Assert + Assert.Equal("Self Repository Composite", result.Name); + Assert.Equal(ActionExecutionType.Composite, result.Execution.ExecutionType); + + var composite = result.Execution as CompositeActionExecutionDataNew; + Assert.NotNull(composite); + Assert.Equal(7, composite.Steps.Count); + + Assert.Equal("$/", Assert.IsType(composite.Steps[0]).Uses.Value); + Assert.Equal("$/.github/actions/inventory-client", Assert.IsType(composite.Steps[1]).Uses.Value); + Assert.Equal("$/actions/nested/composite", Assert.IsType(composite.Steps[2]).Uses.Value); + + // No template errors should have been reported for the $/ steps + _ec.Verify(x => x.AddIssue(It.Is(s => s.Message.Contains("Expected format")), It.IsAny()), Times.Never); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Load_SelfRepositoryRefRejected() + { + try + { + Setup(); + + var actionManifest = new ActionManifestManager(); + actionManifest.Initialize(_hc); + var manifestPath = Path.Combine(TestUtil.GetTestDataPath(), "self_repository_ref_composite_action.yml"); + + Assert.Throws(() => + actionManifest.Load(_ec.Object, manifestPath)); + _ec.Verify( + x => x.AddIssue( + It.Is(s => + s.Message.StartsWith(manifestPath) && + s.Message.EndsWith("Self-repository references do not support an '@ref' suffix. Actual '$/foo@v1'")), + It.IsAny()), + Times.Once); + } + finally + { + Teardown(); + } + } + [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] diff --git a/src/Test/L0/Worker/ActionManifestParserComparisonL0.cs b/src/Test/L0/Worker/ActionManifestParserComparisonL0.cs index 7551b0f57e6..336953d0c42 100644 --- a/src/Test/L0/Worker/ActionManifestParserComparisonL0.cs +++ b/src/Test/L0/Worker/ActionManifestParserComparisonL0.cs @@ -413,6 +413,143 @@ public void Load_BothParsersRejectInvalidExpressionContext() } } + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Load_SelfRepositoryReferences_BothParsersAgree() + { + try + { + // Arrange — regression test: '$/' self-repository references must be + // accepted by the new parser exactly like the legacy parser, otherwise + // green jobs emit spurious template errors. + Setup(); + _ec.Object.Global.Variables.Set(Constants.Runner.Features.CompareWorkflowParser, "true"); + + var legacyManager = new ActionManifestManagerLegacy(); + legacyManager.Initialize(_hc); + _hc.SetSingleton(legacyManager); + + var newManager = new ActionManifestManager(); + newManager.Initialize(_hc); + _hc.SetSingleton(newManager); + + var wrapper = new ActionManifestManagerWrapper(); + wrapper.Initialize(_hc); + + var manifestPath = Path.Combine(TestUtil.GetTestDataPath(), "self_repository_composite_action.yml"); + + // Act + var result = wrapper.Load(_ec.Object, manifestPath); + + // Assert - no mismatch recorded between the two parsers + Assert.False(_ec.Object.Global.HasActionManifestMismatch); + + Assert.NotNull(result); + Assert.Equal(ActionExecutionType.Composite, result.Execution.ExecutionType); + + var compositeExecution = result.Execution as CompositeActionExecutionData; + Assert.NotNull(compositeExecution); + Assert.Equal(7, compositeExecution.Steps.Count); + + var rootRef = Assert.IsType(compositeExecution.Steps[0].Reference); + Assert.Equal(GitHub.DistributedTask.Pipelines.PipelineConstants.SelfRepositoryAlias, rootRef.RepositoryType); + Assert.Equal(string.Empty, rootRef.Path); + Assert.Null(rootRef.Name); + Assert.Null(rootRef.Ref); + + var selfRepoRef = Assert.IsType(compositeExecution.Steps[1].Reference); + Assert.Equal(GitHub.DistributedTask.Pipelines.PipelineConstants.SelfRepositoryAlias, selfRepoRef.RepositoryType); + Assert.Equal(".github/actions/inventory-client", selfRepoRef.Path); + + var nestedRef = Assert.IsType(compositeExecution.Steps[2].Reference); + Assert.Equal(GitHub.DistributedTask.Pipelines.PipelineConstants.SelfRepositoryAlias, nestedRef.RepositoryType); + Assert.Equal("actions/nested/composite", nestedRef.Path); + + // Sanity: ordinary references are unaffected + var externalRef = Assert.IsType(compositeExecution.Steps[3].Reference); + Assert.Equal("GitHub", externalRef.RepositoryType); + Assert.Equal("actions/checkout", externalRef.Name); + Assert.Equal("v4", externalRef.Ref); + + var localRef = Assert.IsType(compositeExecution.Steps[4].Reference); + Assert.Equal(GitHub.DistributedTask.Pipelines.PipelineConstants.SelfAlias, localRef.RepositoryType); + Assert.Equal("./local-action", localRef.Path); + + var dockerRef = Assert.IsType(compositeExecution.Steps[5].Reference); + Assert.Equal("alpine:3", dockerRef.Image); + } + finally + { + Teardown(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Load_SelfRepositoryRefRejected_BothParsersAgree() + { + try + { + Setup(); + + var legacyManager = new ActionManifestManagerLegacy(); + legacyManager.Initialize(_hc); + var newManager = new ActionManifestManager(); + newManager.Initialize(_hc); + var manifestPath = Path.Combine(TestUtil.GetTestDataPath(), "self_repository_ref_composite_action.yml"); + + var legacyException = Assert.Throws(() => legacyManager.Load(_ec.Object, manifestPath)); + _ec.Verify( + x => x.AddIssue( + It.Is(s => + s.Message.StartsWith(manifestPath) && + s.Message.EndsWith("Self-repository references do not support an '@ref' suffix. Actual '$/foo@v1'")), + It.IsAny()), + Times.AtLeastOnce); + + _ec.Invocations.Clear(); + + var newException = Assert.Throws(() => newManager.Load(_ec.Object, manifestPath)); + _ec.Verify( + x => x.AddIssue( + It.Is(s => + s.Message.StartsWith(manifestPath) && + s.Message.EndsWith("Self-repository references do not support an '@ref' suffix. Actual '$/foo@v1'")), + It.IsAny()), + Times.AtLeastOnce); + Assert.Equal(legacyException.Message, newException.Message); + } + finally + { + Teardown(); + } + } + + [Theory] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + [InlineData("$/", "")] + [InlineData("$/actions/example", "actions/example")] + [InlineData("$//actions/example", "actions/example")] + public void TryParseSelfRepository_AcceptsRootAndNormalizesPath(string uses, string expectedPath) + { + Assert.True(GitHub.DistributedTask.Pipelines.PipelineConstants.TryParseSelfRepository(uses, out var path, out var error)); + Assert.Equal(expectedPath, path); + Assert.Null(error); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void TryParseSelfRepository_RejectsRef() + { + Assert.False(GitHub.DistributedTask.Pipelines.PipelineConstants.TryParseSelfRepository("$/foo@v1", out var path, out var error)); + Assert.Null(path); + Assert.Equal("Self-repository references do not support an '@ref' suffix. Actual '$/foo@v1'", error); + } + private string GetFullExceptionMessage(Exception ex) { var messages = new List(); diff --git a/src/Test/TestData/self_repository_composite_action.yml b/src/Test/TestData/self_repository_composite_action.yml new file mode 100644 index 00000000000..1891d9dc696 --- /dev/null +++ b/src/Test/TestData/self_repository_composite_action.yml @@ -0,0 +1,25 @@ +name: 'Self Repository Composite' +description: 'Test composite action referencing actions in the same repository via $/' +runs: + using: "composite" + steps: + - uses: $/ + id: root + + - uses: $/.github/actions/inventory-client + id: inventory + + - uses: $/actions/nested/composite + id: nested + + - uses: actions/checkout@v4 + id: external + + - uses: ./local-action + id: local + + - uses: docker://alpine:3 + id: docker + + - run: echo done + shell: bash diff --git a/src/Test/TestData/self_repository_ref_composite_action.yml b/src/Test/TestData/self_repository_ref_composite_action.yml new file mode 100644 index 00000000000..b88c567c6c3 --- /dev/null +++ b/src/Test/TestData/self_repository_ref_composite_action.yml @@ -0,0 +1,6 @@ +name: 'Invalid Self Repository Ref' +description: 'Test rejection of a ref on a self-repository action' +runs: + using: "composite" + steps: + - uses: $/foo@v1