ci: (workflows & actions) interpolate into env vars#2105
Conversation
Move ${{ ... }} interpolations from 'run:' scripts to env vars to avoid
shell injection shenanigans. These workflows had low exposure before
(only available to trusted users), but now the exposure is even lower.
Move ${{ ... }} interpolations from 'run:' scripts to env vars to avoid
shell injection shenanigans.
robbkidd
requested review from
ahayworth,
arielvalentin,
dazuma,
ericmustin,
fbogsany,
kaylareopelle,
mwear,
plantfansam,
robertlaurin and
xuan-cao-swi
as code owners
|
I exercised these changes locally as much as possible. |
kaylareopelle
left a comment
kaylareopelle
left a comment
There was a problem hiding this comment.
Thank you for doing this, @robbkidd! One question before I approve.
| @@ -119,7 +122,9 @@ runs: | |||
| - name: Build Gem | |||
| shell: bash | |||
| if: "${{ inputs.build == 'true' }}" | |||
There was a problem hiding this comment.
Is this an input we need to be concerned about?
There was a problem hiding this comment.
No, but it's good to be suspicious and ask why!
The if: field is evaluated by whatever YAML processing GitHub Actions is doing. The value of inputs.build is not passed to a shell. The equality check is performed by Action YAMLing, so sh hax0rs.activate shenanigans == 'true' would result in false as the value for if:.
As I understand the vulnerability, it only presents when ${{ … }} appears directly within specifically a run: block since that's passed on for a shell to execute.
There was a problem hiding this comment.
Thank you! I appreciate your explanation! 🎉
3 participants
Move ${{ ... }} interpolations from 'run:' scripts to env vars to avoid shell injection shenanigans. YAML string handling works as hoped for the variables whose values are strings with spaces in them, like flags and named-things-to-be-released.