fix(ng-dev): prevent Git option injection in GitClient#3752
fix(ng-dev): prevent Git option injection in GitClient#3752josephperrott wants to merge 2 commits into
Conversation
![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 introduces security enhancements and input validation across several scripts and GitHub Actions. Specifically, it updates the artifact packaging action to use environment variables instead of direct interpolation, adds symbolic link checks and secure file writing to prevent link-following attacks in the metadata injection script, and introduces validation to ensure Git references do not start with a hyphen. Feedback on these changes includes a recommendation to use -- in the cp command to prevent option injection if the deployment directory input starts with a hyphen, and a suggestion to validate that all required command-line arguments are provided and non-empty in the metadata injection script.
| cp -R "${{inputs.deploy-directory}}" "$dir" | ||
| dir="$RUNNER_TEMP/pack-and-upload-tmp-dir" | ||
| rm -rf "$dir" | ||
| cp -R "$DEPLOY_DIR_INPUT" "$dir" |
There was a problem hiding this comment.
To prevent potential option injection vulnerabilities if DEPLOY_DIR_INPUT starts with a hyphen (-), use -- to separate options from positional arguments in the cp command.
cp -R -- "$DEPLOY_DIR_INPUT" "$dir"| } | ||
|
|
||
| async function main() { | ||
| const [deployDirPath, prNumber, buildRevision] = process.argv.slice(2); |
There was a problem hiding this comment.
Validate that all required command-line arguments (deployDirPath, prNumber, and buildRevision) are provided and non-empty. Otherwise, missing arguments could result in writing the string "undefined" to metadata files or throwing unhandled runtime TypeErrors.
const [deployDirPath, prNumber, buildRevision] = process.argv.slice(2);
if (!deployDirPath || !prNumber || !buildRevision) {
throw new Error('Missing required arguments: deployDirPath, prNumber, and buildRevision must be provided.');
}
josephperrott
force-pushed
the
fix/sec-git-option-injection-0ac0bcd1
branch
2 times, most recently
from
5fe1dec to
7bfcf9a
Compare
Adds validation assertions to verify that Git branch names, references,
or commit SHAs passed to GitClient methods do not start with a hyphen
('-'). This prevents attackers from injecting Git command-line options
via user-controlled Git references.
Implemented assertValidGitRef in git-client.ts and applied it to hasCommit,
checkout, and allChangesFilesSince.
Created new unit tests in ng-dev/utils/test/git-client.spec.ts to verify
validation throws errors for invalid references and passes for valid ones.
Vulnerability: 0ac0bcd1
josephperrott
force-pushed
the
fix/sec-git-option-injection-0ac0bcd1
branch
from
7bfcf9a to
0ae77b4
Compare
1 participant
This PR resolves an option injection vulnerability in the GitClient utility.
Changes
GitClientmethods to reject refs or SHAs starting with a hyphen-.ng-dev/utils/test/git-client.spec.ts.bazel test //ng-dev/utils/test:test.Vulnerability: 0ac0bcd1