Refactor Github Action per b/485167538#19345
Conversation
Summary of ChangesHello @google-admin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors several GitHub Actions to align with updated standards, specifically addressing an internal bug (b/485167538). The core change involves explicitly passing input variables to Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Size Change: -2 B (0%) Total Size: 22.2 MB ℹ️ View Unchanged
|
gemini-cli
Bot
added
the
status/need-issue
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to improve the security of GitHub Actions by using environment variables for inputs in run scripts, which is a good step towards preventing shell injection vulnerabilities. However, a security audit has identified multiple high-severity command injection vulnerabilities. While the migration to environment variables is a correct security practice, the implementation is flawed as it fails to properly quote these variables when used in shell commands. This oversight can still lead to arbitrary command execution if an attacker controls the action's inputs. I've left specific comments with suggestions to add the necessary quotes to fully mitigate these issues.
| --workspace="${INPUTS_CORE_PACKAGE_NAME}" \ | ||
| --no-tag | ||
| npm dist-tag rm ${{ inputs.core-package-name }} false --silent | ||
| npm dist-tag rm ${INPUTS_CORE_PACKAGE_NAME} false --silent |
There was a problem hiding this comment.
A high-severity command injection vulnerability exists here. The INPUTS_CORE_PACKAGE_NAME environment variable is used unquoted in the run script, allowing an attacker to inject malicious shell commands if they control the core-package-name input. Enclose the variable in double quotes to prevent this.
npm dist-tag rm "${INPUTS_CORE_PACKAGE_NAME}" false --silent| --workspace="${INPUTS_CLI_PACKAGE_NAME}" \ | ||
| --no-tag | ||
| npm dist-tag rm ${{ inputs.cli-package-name }} false --silent | ||
| npm dist-tag rm ${INPUTS_CLI_PACKAGE_NAME} false --silent |
There was a problem hiding this comment.
A high-severity command injection vulnerability exists here. The INPUTS_CLI_PACKAGE_NAME environment variable is used unquoted in the run script, allowing an attacker to inject malicious shell commands if they control the cli-package-name input. Enclose the variable in double quotes to prevent this.
npm dist-tag rm "${INPUTS_CLI_PACKAGE_NAME}" false --silent| --workspace="${INPUTS_A2A_PACKAGE_NAME}" \ | ||
| --no-tag | ||
| npm dist-tag rm ${{ inputs.a2a-package-name }} false --silent | ||
| npm dist-tag rm ${INPUTS_A2A_PACKAGE_NAME} false --silent |
There was a problem hiding this comment.
A high-severity command injection vulnerability exists here. The INPUTS_A2A_PACKAGE_NAME environment variable is used unquoted in the run script, allowing an attacker to inject malicious shell commands if they control the a2a-package-name input. Enclose the variable in double quotes to prevent this.
npm dist-tag rm "${INPUTS_A2A_PACKAGE_NAME}" false --silent| working-directory: '${{ inputs.working-directory }}' | ||
| run: | | ||
| npm dist-tag add ${{ inputs.core-package-name }}@${{ inputs.version }} ${{ inputs.channel }} | ||
| npm dist-tag add ${INPUTS_CORE_PACKAGE_NAME}@${INPUTS_VERSION} ${INPUTS_CHANNEL} |
There was a problem hiding this comment.
A high-severity command injection vulnerability exists here. Environment variables are used unquoted in the run script, allowing an attacker to inject malicious shell commands if they control these input values. Enclose the variables in double quotes to ensure they are treated as single strings and prevent this.
npm dist-tag add "${INPUTS_CORE_PACKAGE_NAME}@${INPUTS_VERSION}" "${INPUTS_CHANNEL}"| working-directory: '${{ inputs.working-directory }}' | ||
| run: | | ||
| npm dist-tag add ${{ inputs.cli-package-name }}@${{ inputs.version }} ${{ inputs.channel }} | ||
| npm dist-tag add ${INPUTS_CLI_PACKAGE_NAME}@${INPUTS_VERSION} ${INPUTS_CHANNEL} |
There was a problem hiding this comment.
A high-severity command injection vulnerability exists here. Environment variables are used unquoted in the run script, allowing an attacker to inject malicious shell commands if they control these input values. Enclose the variables in double quotes to ensure they are treated as single strings and prevent this.
npm dist-tag add "${INPUTS_CLI_PACKAGE_NAME}@${INPUTS_VERSION}" "${INPUTS_CHANNEL}"| working-directory: '${{ inputs.working-directory }}' | ||
| run: | | ||
| npm dist-tag add ${{ inputs.a2a-package-name }}@${{ inputs.version }} ${{ inputs.channel }} | ||
| npm dist-tag add ${INPUTS_A2A_PACKAGE_NAME}@${INPUTS_VERSION} ${INPUTS_CHANNEL} |
There was a problem hiding this comment.
A high-severity command injection vulnerability exists here. Environment variables are used unquoted in the run script, allowing an attacker to inject malicious shell commands if they control these input values. Enclose the variables in double quotes to ensure they are treated as single strings and prevent this.
npm dist-tag add "${INPUTS_A2A_PACKAGE_NAME}@${INPUTS_VERSION}" "${INPUTS_CHANNEL}"| working-directory: '${{ inputs.working-directory }}' | ||
| run: | | ||
| echo "Dry run: Would have added tag '${{ inputs.channel }}' to version '${{ inputs.version }}' for ${{ inputs.cli-package-name }}, ${{ inputs.core-package-name }}, and ${{ inputs.a2a-package-name }}." | ||
| echo "Dry run: Would have added tag '${INPUTS_CHANNEL}' to version '${INPUTS_VERSION}' for ${INPUTS_CLI_PACKAGE_NAME}, ${INPUTS_CORE_PACKAGE_NAME}, and ${INPUTS_A2A_PACKAGE_NAME}." |
There was a problem hiding this comment.
The variables in this echo command are unquoted, which is a security risk that can lead to command injection if a variable contains shell metacharacters like $(...). Using printf with format specifiers is the recommended, safer way to print dynamic values in shell scripts.
printf 'Dry run: Would have added tag '\''%s'\'' to version '\''%s'\'' for %s, %s, and %s.\n' "${INPUTS_CHANNEL}" "${INPUTS_VERSION}" "${INPUTS_CLI_PACKAGE_NAME}" "${INPUTS_CORE_PACKAGE_NAME}" "${INPUTS_A2A_PACKAGE_NAME}"
4 participants
This is a http://go/LSC run by http://go/ghss to automatically refactor your Github Actions per http://b/485167538.
This is a PR to help you upgrade to the latest standards in Github Actions.
Please merge this PR to accept the changes. NOTE: if you do not accept this PR, it may be force merged by the GHSS team. See http://b/485167538 for more details.