-
Notifications
You must be signed in to change notification settings - Fork 5
Fix Code Injection issues #56
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
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR centralizes workflow inputs as environment variables and refactors shell steps to reference them directly, aiming to reduce code injection risk and improve script reliability.
- Expose
versions
,tool
, andpacks
inputs viaenv
mappings - Add
set -e
to fail fast on shell errors in matrix setup and linting steps - Refactor CodeQL pack path resolution to use an env var
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
.github/workflows/python-testing.yml | Added VERSIONS env, switched to $VERSIONS , added set -e |
.github/workflows/python-linting.yml | Added VERSIONS and TOOL envs, switched to env vars, added set -e |
.github/workflows/codeql-packs.yml | Added PACKS env, refactored PACK_PATH to use $PACKS |
Comments suppressed due to low confidence (1)
.github/workflows/codeql-packs.yml:52
- [nitpick] The env var
PACKS
(plural) holds a single pack path, which can be confusing. Consider renaming it toPACK
orPACK_NAME
for clarity.
PACK_PATH="${PACKS}/qlpack.yml"
echo "Version Input :: $versions" | ||
matrix=$(echo "$versions" | tr "," "\n" | awk '{print "\""$1"\""}' | paste -sd "," -) | ||
set -e | ||
echo "Version Input :: $VERSIONS" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shell interpolation of $VERSIONS
without validation can allow unintended commands or malformed input to break the matrix setup. Consider validating or sanitizing the input (e.g., enforcing a semver pattern) before using it in the shell pipeline.
echo "Version Input :: $VERSIONS" | |
echo "Version Input :: $VERSIONS" | |
if [[ ! "$VERSIONS" =~ ^[0-9]+\.[0-9]+(\.[0-9]+)?(,[0-9]+\.[0-9]+(\.[0-9]+)?)*$ ]]; then | |
echo "Error: Invalid versions input. Must be a comma-separated list of semver versions (e.g., '3.9,3.10,3.11')." | |
exit 1 | |
fi |
Copilot uses AI. Check for mistakes.
matrix=$(echo "$versions" | tr "," "\n" | awk '{print "\""$1"\""}' | paste -sd "," -) | ||
set -e | ||
echo "Version Input :: $VERSIONS" | ||
matrix=$(echo "$VERSIONS" | tr "," "\n" | awk '{print "\""$1"\""}' | paste -sd "," -) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above, unvalidated $VERSIONS
input is interpolated directly into shell commands, which risks code injection. Add explicit input validation or restrict accepted formats to a safe whitelist.
Copilot uses AI. Check for mistakes.
No description provided.