Skip to content

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

Merged
merged 3 commits into from
Jun 5, 2025
Merged

Fix Code Injection issues #56

merged 3 commits into from
Jun 5, 2025

Conversation

GeekMasher
Copy link
Contributor

No description provided.

@Copilot Copilot AI review requested due to automatic review settings June 5, 2025 13:45
@GeekMasher GeekMasher requested a review from a team as a code owner June 5, 2025 13:45
@GeekMasher GeekMasher requested review from felickz and adrienpessu June 5, 2025 13:45
Copy link

github-actions bot commented Jun 5, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copy link

@Copilot Copilot AI left a 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, and packs inputs via env 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 to PACK or PACK_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"
Copy link
Preview

Copilot AI Jun 5, 2025

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.

Suggested change
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 "," -)
Copy link
Preview

Copilot AI Jun 5, 2025

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.

@GeekMasher GeekMasher merged commit 6ab3926 into main Jun 5, 2025
5 of 6 checks passed
@GeekMasher GeekMasher deleted the GeekMasher-patch-1 branch June 5, 2025 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant