Skip to content

Fix positional arg bug in ExpressionParser.CreateTree#4279

Merged
ericsciple merged 1 commit intomainfrom
users/ericsciple/26-03-case
Mar 5, 2026
Merged

Fix positional arg bug in ExpressionParser.CreateTree#4279
ericsciple merged 1 commit intomainfrom
users/ericsciple/26-03-case

Conversation

@ericsciple
Copy link
Collaborator

allowCaseFunction was passed positionally into the allowUnknownKeywords, causing the legacy parser to silently accept any named-value in expressions during schema validation.

@ericsciple ericsciple force-pushed the users/ericsciple/26-03-case branch from 7772cef to 627937f Compare March 5, 2026 19:36
allowCaseFunction was passed positionally into the allowUnknownKeywords
parameter of ParseContext, causing the legacy parser to silently accept
any named-value in expressions during schema validation.
@ericsciple ericsciple force-pushed the users/ericsciple/26-03-case branch from 627937f to 2c8827e Compare March 5, 2026 19:48
@ericsciple ericsciple marked this pull request as ready for review March 5, 2026 20:23
@ericsciple ericsciple requested a review from a team as a code owner March 5, 2026 20:23
Copilot AI review requested due to automatic review settings March 5, 2026 20:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a positional-argument mixup in ExpressionParser.CreateTree that caused allowCaseFunction to incorrectly toggle allowUnknownKeywords, letting invalid named-values slip through schema validation.

Changes:

  • Pass allowCaseFunction into ParseContext via a named argument to avoid the incorrect parameter binding.
  • Add regression tests for expression parsing and for action manifest parsing (legacy/new/wrapper) to ensure invalid contexts are rejected.
  • Add new action manifest test data covering invalid env expression context usage.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/Sdk/DTExpressions2/Expressions2/ExpressionParser.cs Fixes the root cause by using a named argument for allowCaseFunction when constructing ParseContext.
src/Test/L0/Sdk/ExpressionParserL0.cs Adds regression tests ensuring unknown named-values are rejected and case() behavior doesn’t affect keyword validation.
src/Test/L0/Worker/ActionManifestParserComparisonL0.cs Adds a wrapper-level test asserting both parsers reject invalid expression context in container env.
src/Test/L0/Worker/ActionManifestManagerLegacyL0.cs Adds legacy parser tests for rejecting invalid env contexts and accepting valid ones.
src/Test/L0/Worker/ActionManifestManagerL0.cs Adds new parser tests for rejecting invalid env contexts and accepting valid ones.
src/Test/TestData/dockerfileaction_env_invalid_context.yml Adds a manifest fixture that contains a valid and an invalid expression context in runs.env.

@ericsciple ericsciple merged commit 1138dd8 into main Mar 5, 2026
12 checks passed
@ericsciple ericsciple deleted the users/ericsciple/26-03-case branch March 5, 2026 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants