Skip to content

fix(@angular/cli): serialize configuration as a single argv token in run_target strategies#33657

Open
herdiyana256 wants to merge 1 commit into
angular:mainfrom
herdiyana256:fix-mcp-run-target-configuration-flag-injection
Open

fix(@angular/cli): serialize configuration as a single argv token in run_target strategies#33657
herdiyana256 wants to merge 1 commit into
angular:mainfrom
herdiyana256:fix-mcp-run-target-configuration-flag-injection

Conversation

@herdiyana256

Copy link
Copy Markdown
Contributor

build-target-strategy.ts, generic-target-strategy.ts, and unit-test-strategy.ts all pushed the configuration value as a separate argv element after '-c'. Since the ng CLI's argument parser does not consume a following token as the value of a string option when that token itself starts with a dash, a configuration value crafted to look like a flag (e.g. "--outputPath=...") is instead parsed as an independent, legitimately-declared option of the target's builder, silently overriding it.

Serialize configuration as a single '--configuration=value' token, matching the format serializeOptions() already uses for every other option, which is not affected by this because the value is bound to the key within one argv element.

Updated the two existing spec assertions that checked the old argv shape.

…run_target strategies

build-target-strategy.ts, generic-target-strategy.ts, and
unit-test-strategy.ts all pushed the configuration value as a separate
argv element after '-c'. Since the ng CLI's argument parser does not
consume a following token as the value of a string option when that
token itself starts with a dash, a configuration value crafted to look
like a flag (e.g. "--outputPath=...") is instead parsed as an
independent, legitimately-declared option of the target's builder,
silently overriding it.

Serialize configuration as a single '--configuration=value' token,
matching the format serializeOptions() already uses for every other
option, which is not affected by this because the value is bound to
the key within one argv element.

Updated the two existing spec assertions that checked the old argv
shape.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the target execution strategies (build, generic, and unit test) to use the long-form --configuration=value argument instead of the short-hand -c flag when constructing command-line arguments. The corresponding unit tests have also been updated to reflect this change. I have no feedback to provide as there are no review comments.

@geritzpatmar-max geritzpatmar-max left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants