Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix positional arg bug in ExpressionParser.CreateTree#4279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| using GitHub.DistributedTask.Expressions2; | ||
| using GitHub.DistributedTask.Expressions2.Sdk; | ||
| using GitHub.DistributedTask.ObjectTemplating; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using Xunit; | ||
| namespace GitHub.Runner.Common.Tests.Sdk | ||
| { | ||
| /// <summary> | ||
| /// Regression tests for ExpressionParser.CreateTree to verify that | ||
| /// allowCaseFunction does not accidentally set allowUnknownKeywords. | ||
| /// </summary> | ||
| public sealed class ExpressionParserL0 | ||
| { | ||
| [Fact] | ||
| [Trait("Level", "L0")] | ||
| [Trait("Category", "Sdk")] | ||
| public void CreateTree_RejectsUnrecognizedNamedValue() | ||
| { | ||
| // Regression: allowCaseFunction was passed positionally into | ||
| // the allowUnknownKeywords parameter, causing all named values | ||
| // to be silently accepted. | ||
| var parser = new ExpressionParser(); | ||
| var namedValues = new List<INamedValueInfo> | ||
| { | ||
| new NamedValueInfo<ContextValueNode>("inputs"), | ||
| }; | ||
| var ex = Assert.Throws<ParseException>(() => | ||
| parser.CreateTree("github.event.repository.private", null, namedValues, null)); | ||
| Assert.Contains("Unrecognized named-value", ex.Message); | ||
| } | ||
| [Fact] | ||
| [Trait("Level", "L0")] | ||
| [Trait("Category", "Sdk")] | ||
| public void CreateTree_AcceptsRecognizedNamedValue() | ||
| { | ||
| var parser = new ExpressionParser(); | ||
| var namedValues = new List<INamedValueInfo> | ||
| { | ||
| new NamedValueInfo<ContextValueNode>("inputs"), | ||
| }; | ||
| var node = parser.CreateTree("inputs.foo", null, namedValues, null); | ||
| Assert.NotNull(node); | ||
| } | ||
| [Fact] | ||
| [Trait("Level", "L0")] | ||
| [Trait("Category", "Sdk")] | ||
| public void CreateTree_CaseFunctionWorks_WhenAllowed() | ||
| { | ||
| var parser = new ExpressionParser(); | ||
| var namedValues = new List<INamedValueInfo> | ||
| { | ||
| new NamedValueInfo<ContextValueNode>("github"), | ||
| }; | ||
| var node = parser.CreateTree("case(github.event_name, 'push', 'Push Event')", null, namedValues, null, allowCaseFunction: true); | ||
| Assert.NotNull(node); | ||
| } | ||
| [Fact] | ||
| [Trait("Level", "L0")] | ||
| [Trait("Category", "Sdk")] | ||
| public void CreateTree_CaseFunctionRejected_WhenDisallowed() | ||
| { | ||
| var parser = new ExpressionParser(); | ||
| var namedValues = new List<INamedValueInfo> | ||
| { | ||
| new NamedValueInfo<ContextValueNode>("github"), | ||
| }; | ||
| var ex = Assert.Throws<ParseException>(() => | ||
| parser.CreateTree("case(github.event_name, 'push', 'Push Event')", null, namedValues, null, allowCaseFunction: false)); | ||
| Assert.Contains("Unrecognized function", ex.Message); | ||
| } | ||
| [Fact] | ||
| [Trait("Level", "L0")] | ||
| [Trait("Category", "Sdk")] | ||
| public void CreateTree_CaseFunctionDoesNotAffectUnknownKeywords() | ||
| { | ||
| // The key regression test: with allowCaseFunction=true (default), | ||
| // unrecognized named values must still be rejected. | ||
| var parser = new ExpressionParser(); | ||
| var namedValues = new List<INamedValueInfo> | ||
| { | ||
| new NamedValueInfo<ContextValueNode>("inputs"), | ||
| }; | ||
| var ex = Assert.Throws<ParseException>(() => | ||
| parser.CreateTree("github.ref", null, namedValues, null, allowCaseFunction: true)); | ||
| Assert.Contains("Unrecognized named-value", ex.Message); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -928,6 +928,58 @@ public void Evaluate_Default_Input() | ||
| } | ||
| } | ||
| [Fact] | ||
| [Trait("Level", "L0")] | ||
| [Trait("Category", "Worker")] | ||
| public void Load_ContainerAction_RejectsInvalidExpressionContext() | ||
| { | ||
| try | ||
| { | ||
| // Arrange | ||
| Setup(); | ||
| var actionManifest = new ActionManifestManager(); | ||
| actionManifest.Initialize(_hc); | ||
| // Act & Assert — github is not a valid context for container-runs-env (only inputs is allowed) | ||
| var ex = Assert.Throws<ArgumentException>(() => | ||
| actionManifest.Load(_ec.Object, Path.Combine(TestUtil.GetTestDataPath(), "dockerfileaction_env_invalid_context.yml"))); | ||
| Assert.Contains("Failed to load", ex.Message); | ||
| } | ||
| finally | ||
| { | ||
| Teardown(); | ||
| } | ||
| } | ||
| [Fact] | ||
| [Trait("Level", "L0")] | ||
| [Trait("Category", "Worker")] | ||
| public void Load_ContainerAction_AcceptsValidExpressionContext() | ||
| { | ||
| try | ||
| { | ||
| // Arrange | ||
| Setup(); | ||
| var actionManifest = new ActionManifestManager(); | ||
| actionManifest.Initialize(_hc); | ||
| // Act — inputs is a valid context for container-runs-env | ||
| var result = actionManifest.Load(_ec.Object, Path.Combine(TestUtil.GetTestDataPath(), "dockerfileaction_arg_env_expression.yml")); | ||
| // Assert | ||
| var containerAction = result.Execution as ContainerActionExecutionDataNew; | ||
| Assert.NotNull(containerAction); | ||
| Assert.Equal("${{ inputs.entryPoint }}", containerAction.Environment[1].Value.ToString()); | ||
ericsciple marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| finally | ||
| { | ||
| Teardown(); | ||
| } | ||
| } | ||
| private void Setup([CallerMemberName] string name = "") | ||
| { | ||
| _ecTokenSource?.Dispose(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -926,6 +926,58 @@ public void Evaluate_Default_Input() | ||
| } | ||
| } | ||
| [Fact] | ||
| [Trait("Level", "L0")] | ||
| [Trait("Category", "Worker")] | ||
| public void Load_ContainerAction_RejectsInvalidExpressionContext() | ||
| { | ||
| try | ||
| { | ||
| // Arrange | ||
| Setup(); | ||
| var actionManifest = new ActionManifestManagerLegacy(); | ||
| actionManifest.Initialize(_hc); | ||
| // Act & Assert — github is not a valid context for container-runs-env (only inputs is allowed) | ||
| var ex = Assert.Throws<ArgumentException>(() => | ||
| actionManifest.Load(_ec.Object, Path.Combine(TestUtil.GetTestDataPath(), "dockerfileaction_env_invalid_context.yml"))); | ||
| Assert.Contains("Failed to load", ex.Message); | ||
| } | ||
| finally | ||
| { | ||
| Teardown(); | ||
| } | ||
| } | ||
| [Fact] | ||
| [Trait("Level", "L0")] | ||
| [Trait("Category", "Worker")] | ||
| public void Load_ContainerAction_AcceptsValidExpressionContext() | ||
| { | ||
| try | ||
| { | ||
| // Arrange | ||
| Setup(); | ||
| var actionManifest = new ActionManifestManagerLegacy(); | ||
| actionManifest.Initialize(_hc); | ||
| // Act — inputs is a valid context for container-runs-env | ||
| var result = actionManifest.Load(_ec.Object, Path.Combine(TestUtil.GetTestDataPath(), "dockerfileaction_arg_env_expression.yml")); | ||
| // Assert | ||
| var containerAction = result.Execution as ContainerActionExecutionData; | ||
| Assert.NotNull(containerAction); | ||
| Assert.Equal("${{ inputs.entryPoint }}", containerAction.Environment[1].Value.ToString()); | ||
ericsciple marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| finally | ||
| { | ||
| Teardown(); | ||
| } | ||
| } | ||
| private void Setup([CallerMemberName] string name = "") | ||
| { | ||
| _ecTokenSource?.Dispose(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| name: 'Action With Invalid Context' | ||
| description: 'Docker action that uses github context in env (only inputs is allowed)' | ||
ericsciple marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| inputs: | ||
| my-input: | ||
| description: 'A test input' | ||
| required: false | ||
| default: 'hello' | ||
| runs: | ||
| using: 'docker' | ||
| image: 'Dockerfile' | ||
| env: | ||
| VALID: '${{ inputs.my-input }}' | ||
| INVALID: '${{ github.event.repository.private }}' | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.