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

fix: github workflow vulnerable to script injection #2663

Merged
merged 5 commits into from
Sep 11, 2024

Conversation

diogoteles08
Copy link
Contributor

Hi! I'm Diogo from Google's Open Source Security Team(GOSST) and I'm dropping by to suggest this small change that will enhance the security of your repository by preventing script injection attacks through your GitHub workflows.

In the piece of code I changed, you were directly using the value of a variable that comes from a user's input, so a malicious user could exploit that input and use it to run arbitrary code. By using an intermediate environment variable, the value of the expression is stored in memory, used as a variable and doesn't interact with the script generation process. This mitigates the script injection risks and also keeps your workflow running exactly as before.

You can find more information about this on this github documentation or in this gitguardian blogpost.

Cheers!

@diogoteles08 diogoteles08 requested review from a team as code owners August 12, 2024 17:45
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. api: storage Issues related to the googleapis/java-storage API. labels Aug 12, 2024
@BenWhitehead
Copy link
Collaborator

@diegomarquezp @blakeli0 Can you address the issue outlined here from @diogoteles08?

@diogoteles08
Copy link
Contributor Author

Hey! Just realized that this workflow is also used in other repos from this same org -- I've already created some PRs to them before realizing they could have the same contributors, sorry ><). Namely:

That said, I'll wait for your input on how we should follow here. Let me know if I should raise similar PRs on the other repos, or if there is any specific source that I should change and then the change can be mirrored to the others.

@blakeli0
Copy link

Hey! Just realized that this workflow is also used in other repos from this same org -- I've already created some PRs to them before realizing they could have the same contributors, sorry ><). Namely:

That said, I'll wait for your input on how we should follow here. Let me know if I should raise similar PRs on the other repos, or if there is any specific source that I should change and then the change can be mirrored to the others.

Thanks @diogoteles08 for reporting this issue! Yes it is used in a few other repos as well, our team will handle all the necessary changes. If you need a way to track it, can you create an issue in our component(173303) internally?

Separately, only contributors with write access can create branches, which are very selective, so this issue should not have any impact at this moment.

@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Aug 19, 2024
@diegomarquezp
Copy link
Contributor

diegomarquezp commented Aug 19, 2024

@diogoteles08, letting you know here as well that I pushed 1 or 2 commits to these PRs (thanks!) to also inline the repo.full_name in the if statement of this yaml. For other repos that use this workflow I created a PR as well (example).

Comment on lines 19 to 20
paths:
- 'generation_config.yaml'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put these lines back? They were added in 34e99b1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restored

@diegomarquezp diegomarquezp added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 5, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 5, 2024
@BenWhitehead BenWhitehead merged commit 9151ac2 into googleapis:main Sep 11, 2024
16 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/java-storage API. size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants