Skip to content
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

BUG : display_results.sh doesn't seem to be escaping user input properly #89

Closed
Zeitsperre opened this issue Aug 5, 2024 · 3 comments
Closed

Comments

@Zeitsperre
Copy link

Hi,

I happened upon this project last week and have been seeing what it could look like to integrate this into a somewhat complex CI pipeline I manage. I'm hoping this action might be the push we need to reduce our CI complexity a bit or self-host our own runners. Neat project!

The issue I'm having is that when trying to run display-results, it seems as though the input fields of some steps might not be able to handle some special characters (brackets, maybe?).

Example: https://github.com/Ouranosinc/xclim/actions/runs/10255754010/job/28373433987#step:13:1

Error:

Run /home/runner/work/_actions/green-coding-solutions/eco-ci-energy-estimation/7333e9a8e8036e5bf69d59dc8bb63d984b0f55c8/scripts/display_results.sh -dt true -dg true -db true -b "1863/merge" -r 10255754010 -R "Ouranosinc/xclim" -sd true -sc true -s "github"
/home/runner/work/_actions/green-coding-solutions/eco-ci-energy-estimation/7333e9a8e8036e5bf69d59dc8bb63d984b0f55c8/scripts/display_results.sh: eval: line 72: unexpected EOF while looking for matching `"'

I imagine this might simply be solved by escaping more of the user input fields.

Additional Resources

The workflow file (testing branch, being actively worked on): https://github.com/Ouranosinc/xclim/blob/energy-usage/.github/workflows/main.yml

@ribalba
Copy link
Member

ribalba commented Aug 6, 2024

Thanks for raising this issue.

Could you please try the version:

green-coding-solutions/eco-ci-energy-estimation@escape-test

You can:

      - name: Start Measurement
        uses: green-coding-solutions/eco-ci-energy-estimation@escape-test
        with:
          task: start-measurement

There has been a massive redesign of the eco-ci code and I hope this version will fix it.

@Zeitsperre
Copy link
Author

Zeitsperre commented Aug 6, 2024

@ribalba That seems to have fixed the issue. Feel free to examine the build logs if you're interested!

PR: Ouranosinc/xclim#1863

I won't be merging my branch until a stable release is made with the fixes.

Thanks!

@ribalba
Copy link
Member

ribalba commented Aug 7, 2024

Pre-Release is here: https://github.com/green-coding-solutions/eco-ci-energy-estimation/releases/tag/v4.0-rc4
Will become stable the next few days.

@ribalba ribalba closed this as completed Aug 7, 2024
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

No branches or pull requests

2 participants