Refactor Github Action per b/485167538#19373
Conversation
|
Hi @google-admin, thank you so much for your contribution to Gemini CLI! We really appreciate the time and effort you've put into this. We're making some updates to our contribution process to improve how we track and review changes. Please take a moment to review our recent discussion post: Improving Our Contribution Process & Introducing New Guidelines. Key Update: Starting January 26, 2026, the Gemini CLI project will require all pull requests to be associated with an existing issue. Any pull requests not linked to an issue by that date will be automatically closed. Thank you for your understanding and for being a part of our community! |
|
Hi there! Thank you for your contribution to Gemini CLI. To improve our contribution process and better track changes, we now require all pull requests to be associated with an existing issue, as announced in our recent discussion and as detailed in our CONTRIBUTING.md. This pull request is being closed because it is not currently linked to an issue. Once you have updated the description of this PR to link an issue (e.g., by adding How to link an issue: Thank you for your understanding and for being a part of our community! |
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 systematically refactors several custom GitHub Actions to adhere to a recommended best practice for handling input variables. The core change involves transitioning from direct inline access of 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
|
![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 is an automated refactoring to improve the security of your GitHub Actions by moving inline context expressions to environment variables. This is a valuable security enhancement. However, my review identified several instances where these new environment variables are used unsafely within run scripts, creating potential command injection vulnerabilities. One of these is a critical vulnerability involving an authentication token. The review comments provide specific code suggestions to properly quote variables and use safer shell commands to address these issues.
| run: |- | ||
| echo ""@google-gemini:registry=https://npm.pkg.github.com"" > ~/.npmrc | ||
| echo ""//npm.pkg.github.com/:_authToken=${{ inputs.github-token }}"" >> ~/.npmrc | ||
| echo ""//npm.pkg.github.com/:_authToken=${INPUTS_GITHUB_TOKEN}"" >> ~/.npmrc |
There was a problem hiding this comment.
Using echo with a variable that is not properly quoted within a double-quoted string can lead to command injection if the variable contains shell metacharacters (e.g., $()). Since INPUTS_GITHUB_TOKEN comes from a secret, its content should be treated as untrusted in this context. Using printf with a format string is a safer way to print the value without it being interpreted by the shell.
printf "//npm.pkg.github.com/:_authToken=%s\n" "$INPUTS_GITHUB_TOKEN" >> ~/.npmrc| --title "Release ${INPUTS_RELEASE_TAG}" \ | ||
| --notes-start-tag "${INPUTS_PREVIOUS_TAG}" \ | ||
| --generate-notes \ | ||
| ${{ inputs.npm-tag != 'latest' && '--prerelease' || '' }} |
There was a problem hiding this comment.
This refactoring is incomplete. The run script still contains a GitHub Actions expression that depends on an input (inputs.npm-tag). To complete the security hardening and improve readability, this logic should be moved into the shell script itself, using an environment variable for the input.
I recommend adding INPUTS_NPM_TAG: ${{ inputs.npm-tag }} to the env block for this step and then modifying the run script to handle the logic in bash:
PRERELEASE_ARG=""
if [[ "${INPUTS_NPM_TAG}" != "latest" ]]; then
PRERELEASE_ARG="--prerelease"
fi
gh release create "${INPUTS_RELEASE_TAG}" \
bundle/gemini.js \
--target "${STEPS_RELEASE_BRANCH_OUTPUTS_BRANCH_NAME}" \
--title "Release ${INPUTS_RELEASE_TAG}" \
--notes-start-tag "${INPUTS_PREVIOUS_TAG}" \
--generate-notes \
$PRERELEASE_ARG| 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.
The environment variables are used without quotes. This can lead to word splitting and globbing by the shell, and potentially command injection if the variables contain special characters. It's a security best practice to always quote variables in shell scripts.
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.
The environment variables are used without quotes. This can lead to word splitting and globbing by the shell, and potentially command injection if the variables contain special characters. It's a security best practice to always quote variables in shell scripts.
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.
The environment variables are used without quotes. This can lead to word splitting and globbing by the shell, and potentially command injection if the variables contain special characters. It's a security best practice to always quote variables in shell scripts.
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 package name variables are unquoted in this echo command. This could lead to command injection if a package name contained shell metacharacters. For safer logging, it's best to quote the variables.
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}'."| if [ "$gemini_version" != "${{ inputs.expected-version }}" ]; then | ||
| echo "❌ NPM Version mismatch: Got $gemini_version from ${{ inputs.npm-package }}, expected ${{ inputs.expected-version }}" | ||
| if [ "$gemini_version" != "${INPUTS_EXPECTED_VERSION}" ]; then | ||
| echo "❌ NPM Version mismatch: Got $gemini_version from ${INPUTS_NPM_PACKAGE}, expected ${INPUTS_EXPECTED_VERSION}" |
There was a problem hiding this comment.
Variables in this echo statement are unquoted. This can lead to unexpected behavior or command injection if the variables contain shell metacharacters. It's safer to quote them, even in log messages.
echo "❌ NPM Version mismatch: Got '$gemini_version' from '${INPUTS_NPM_PACKAGE}', expected '${INPUTS_EXPECTED_VERSION}'"| echo "❌ NPX Run Version mismatch: Got $gemini_version from ${{ inputs.npm-package }}, expected ${{ inputs.expected-version }}" | ||
| gemini_version=$(npx --prefer-online "${INPUTS_NPM_PACKAGE}" --version) | ||
| if [ "$gemini_version" != "${INPUTS_EXPECTED_VERSION}" ]; then | ||
| echo "❌ NPX Run Version mismatch: Got $gemini_version from ${INPUTS_NPM_PACKAGE}, expected ${INPUTS_EXPECTED_VERSION}" |
There was a problem hiding this comment.
Variables in this echo statement are unquoted. This can lead to unexpected behavior or command injection if the variables contain shell metacharacters. It's safer to quote them, even in log messages.
echo "❌ NPX Run Version mismatch: Got '$gemini_version' from '${INPUTS_NPM_PACKAGE}', expected '${INPUTS_EXPECTED_VERSION}'"
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.