Revert "[SINT-4157] sanitise ultimate-pipeline.yml"#7627
Conversation
This reverts commit c2c3b52.
|
Since we must trigger this ourselves there really isn't a risk here at the moment, will let the SINT team follow up to resolve. This is blocking the testing of #7514 |
There was a problem hiding this comment.
💡 Codex Review
dd-trace-dotnet/.azure-pipelines/ultimate-pipeline.yml
Lines 4249 to 4250 in 013311b
The revert removes the branch-name regex entirely, yet the branch value is still injected directly into the JSON body sent via curl -d. Git permits branch names containing characters such as quotes or braces, so a PR from a fork could supply a value that breaks the JSON or injects extra fields (e.g., toggling is_release_version) when the payload is built here. Rather than dropping the check, the branch name should be validated or JSON-escaped before constructing the dispatch request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
## Summary of changes ## Reason for change Follow-up of #7582, that got reverted in #7627. The present PR updates the regular expression: the hyphen should be at the end or escaped to avoid being interpreted as a range operator. ## Implementation details From [source](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html): > The <hyphen-minus> character shall be treated as itself if it occurs first (after an initial '^', if any) or last in the list, or as an ending range point in a range expression. ## Test coverage Tested locally with this script [test-branch-name.sh](https://github.com/user-attachments/files/22740574/test-branch-name.sh) and got the following result, which reproduces the error in the PR and validates the fix from this PR. In particular: `maximo/otel-metrics-api-exporter` and `maximo/otel-metrics-api-exporter.` are authorized patterns, while `maximo/otel-metrics-api-exporter\;` is not. Test results confirm that the corrected regex now implements this behavior correctly ``` $ bash ./test-branch-name.sh maximo/otel-metrics-api-exporter Handling maximo/otel-metrics-api-exporter reverted regex NOT matched (branch name deemed UNSAFE, will exit) corrected regex matched (branch name deemed safe) $ bash ./test-branch-name.sh maximo/otel-metrics-api-exporter. Handling maximo/otel-metrics-api-exporter. reverted regex NOT matched (branch name deemed UNSAFE, will exit) corrected regex matched (branch name deemed safe) $ bash ./test-branch-name.sh maximo/otel-metrics-api-exporter\; Handling maximo/otel-metrics-api-exporter; reverted regex NOT matched (branch name deemed UNSAFE, will exit) corrected regex NOT matched (branch name deemed UNSAFE, will exit) ``` ## Other details <!-- Fixes #{issue} --> <!--⚠️ Note: Where possible, please obtain 2 approvals prior to merging. Unless CODEOWNERS specifies otherwise, for external teams it is typically best to have one review from a team member, and one review from apm-dotnet. Trivial changes do not require 2 reviews. MergeQueue is NOT enabled in this repository. If you have write access to the repo, the PR has 1-2 approvals (see above), and all of the required checks have passed, you can use the Squash and Merge button to merge the PR. If you don't have write access, or you need help, reach out in the #apm-dotnet channel in Slack. -->
Reverts #7582
This is preventing us to run the action as expected as the regex appears to be incorrect.
Since we must trigger this ourselves there really isn't a risk here at the moment, will let the SINT team follow up to resolve.
This is blocking the testing of #7514
https://dev.azure.com/datadoghq/dd-trace-dotnet/_build/results?buildId=188595&view=logs&j=8299a7f3-61e2-5626-a12e-85b7b20deab1&t=e26f8d61-df24-5d93-564c-322f7221200d