ETC,BLD,SEC: quoting to prevent CWE-78, and add branch-checkout-args #8
Conversation
- The (already-prvileged) github actions user controls most(?)/all of these parameters which need to be quoted before passing to a shell
- It's easier to do shell/yaml code review when things are already shell-quoted; even if unnecessarily per shellcheck, It's easier to review when already shell-quoted.
- "CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')"
https://cwe.mitre.org/data/definitions/78.html
action.yml
Outdated
| branch-checkout-args: | ||
| description: Arguments to pass to `git checkout`: `git checkout ${checkout-args} "${branch}"` | ||
| required: false | ||
| default: '' |
There was a problem hiding this comment.
This is maybe not necessary. I think i was thinking of git clone args like --depth=1
There was a problem hiding this comment.
There was a problem hiding this comment.
@westurner, sorry for my super late reply. I am going back to the maintenance of this repository.
I have to have a look at your changes. Do you still think that this improves the action? I didn't understand if your "This is maybe not necessary" affects your whole PR.
Again, sorry. I hope you still want to discuss about it.
There was a problem hiding this comment.
Thanks for coming back to this project! Before you merge, please also add quotes around the description value since it contains a colon which the YAML parser does not expect.
|
Let's release a new version. Thanks for your contribution! |
|
Thanks!
…On Tue, Jan 23, 2024, 5:02 PM Diego Prada-Gracia ***@***.***> wrote:
Let's release a new version. Thanks for your contribution!
—
Reply to this email directly, view it on GitHub
<#8 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAMNS644GGB5STR4AV3PLTYQAXN5AVCNFSM6AAAAAA2NIBYP6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBWHE4TENZSHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
… On Tue, Jan 23, 2024, 5:23 PM Wes Turner ***@***.***> wrote:
Thanks!
On Tue, Jan 23, 2024, 5:02 PM Diego Prada-Gracia ***@***.***>
wrote:
> Let's release a new version. Thanks for your contribution!
>
> —
> Reply to this email directly, view it on GitHub
> <#8 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAAMNS644GGB5STR4AV3PLTYQAXN5AVCNFSM6AAAAAA2NIBYP6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBWHE4TENZSHE>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
The (already-prvileged) github actions user controls most(?)/all of these parameters which need to be quoted before passing to a shell
It's easier to do shell/yaml code review when things are already shell-quoted; even if unnecessarily per shellcheck, It's easier to review when already shell-quoted.
"CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')"
https://cwe.mitre.org/data/definitions/78.html