buildkite-plugins / monorepo-diff-buildkite-plugin

Run separate pipelines for each folder in your monorepo
MIT License
20 stars 6 forks source link

Plugin fails if the git commit triggering it contains special characters #15

Open xphir opened 3 months ago

xphir commented 3 months ago

There seems to be a bug with the way Git commit strings are handled. Specifically, we have found that the plugin will error out if a Git commit contains a special character such as $.

Upon examining the code, this error is thrown during the pipeline upload of the trigger step, as the message property (which is based on the Git commit) contains values that don't pass validation.

This impacts my team sporadically, as we occasionally have commits that contain characters that cause this bug. I did some testing, so if you have a Git commit such as this commit $$$ should cause issues, it is then translated into the trigger step of:

  - trigger: whatever
    build:
      message: this commit $$$ should cause issues
      branch: some-branch
      commit: HEAD

You end up having a failed pipeline upload. I think it should be noted that the "raw" Git commit string then becomes the message property.

To clarify this further, without the plugin, I tried to add the step above to its own pipeline, but that leads to a failure like so:

2024-06-06 02:59:46 FATAL  Pipeline interpolation of "root.yaml" failed: Expected identifier to start with a letter, got  
🚨 Error: The command exited with status 1

However, it seems like the UI properly handles a message with special characters, as creating a build with the message this commit $$$ should cause issues works just fine.

My hunch is that the fix for this is string sanitation of the Git commit before it is set as the message property in the trigger step of the pipeline upload.

Worth noting, I haven't tried every single special character to confirm if it fails or succeeds, but I would assume that you already have these values as they would be part of the step validation logic?

tomowatt commented 3 months ago

Hey @xphir Thank you for raising, we will check out what is happening there!

sj26 commented 2 days ago

I just hit this today as well. The plugin needs to escape interpolations.