lorenzwalthert / touchstone

Smart benchmarking of pull requests with statistical confidence
https://lorenzwalthert.github.io/touchstone
Other
53 stars 7 forks source link

Standalone action #92

Closed assignUser closed 2 years ago

assignUser commented 2 years ago

I have created the standalone action and updated the files in inst accordingly. I tested this here: https://github.com/assignUser/simstudy/runs/4740711766?check_suite_focus=true Maybe @lorenzwalthert you can test it with {styler}? But please update your RSPM link, see: https://community.rstudio.com/t/pak-lockfile-creation-within-github-action-hangs-forever/125813/7

assignUser commented 2 years ago

oh we probably should update the documentation too

lorenzwalthert commented 2 years ago

Great :D. I love seeing the workflow file shrink 😄

lorenzwalthert commented 2 years ago

I

lorenzwalthert commented 2 years ago

Thanks for fixing the tests, the concurrency thing and the added README.md 👍. Maybe it makes more sense to link this article to the security concerns I initially used? https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

~Also, our bash scripts are not save against injection as they don't use quotes for env variables, e.g. here. We should adapt them.~ Fixed.

Also, I wonder if we still need two workflows, since in the article you linked, they say one strategy to avoid injections is to use an action instead of a script.

assignUser commented 2 years ago

Also, I wonder if we still need two workflows, since in the article you linked, they say one strategy to avoid injections is to use an action instead of a script.

I think those are two separate issues, code injection via context and r&w access to our repo while also executing code supplied by a (malicious) third party. So I would say we stick with 2. I will edit the readme in a moment, should I adjust the templates to use @main or do you want to change that after merging?

lorenzwalthert commented 2 years ago

Sounds good. You can change it before the merge. Thanks.

assignUser commented 2 years ago

:heavy_check_mark: :D