Although the `clone_url` and `sha` are safe, other similar aspects of the pull request head are not (e.g. `head.ref`, `pull_request.title` etc) and these must not be interpolated.
So let's use the convention of putting such data into environment variables, where the contents are not interpolated into the bash commands and are instead passed directly to the called program. https://docs.github.com/en/actions/reference/security/secure-use#use-an-intermediate-environment-variable --- Just to be clear, there's no problem with what we were doing before with those specific variables, but the interpolation is a habit we want to avoid getting into. I was reviewing this workflow because it uses `pull_request_target`, which runs with elevated permissions compared to the permissions normally available for `pull_request` workflows. It's been leveraged in recent security incidents elsewhere (e.g. [TanStack](https://tanstack.com/blog/npm-supply-chain-compromise-postmortem), so I think the hardening is worth doing. You can view, comment on, or merge this pull request online at: https://github.com/openstreetmap/openstreetmap-website/pull/7084 -- Commit Summary -- * Avoid string interpolation into bash commands -- File Changes -- M .github/workflows/danger.yml (10) -- Patch Links -- https://github.com/openstreetmap/openstreetmap-website/pull/7084.patchhttps://github.com/openstreetmap/openstreetmap-website/pull/7084.diff -- Reply to this email directly or view it on GitHub: https://github.com/openstreetmap/openstreetmap-website/pull/7084 You are receiving this because you are subscribed to this thread. Message ID: <openstreetmap/openstreetmap-website/pull/[email protected]>
_______________________________________________ rails-dev mailing list [email protected] https://lists.openstreetmap.org/listinfo/rails-dev
