Skip to content

Potential fix for code scanning alert no. 100: Code injection - #263

Merged
burgholzer merged 1 commit into
mainfrom
alert-autofix-100
Nov 29, 2025
Merged

Potential fix for code scanning alert no. 100: Code injection#263
burgholzer merged 1 commit into
mainfrom
alert-autofix-100

Conversation

@burgholzer

Copy link
Copy Markdown
Member

Potential fix for https://github.com/munich-quantum-toolkit/workflows/security/code-scanning/100

To fix this issue, we must ensure that untrusted user input (inputs.runs-on) is not interpolated or injected directly into shell commands in the run: section. The safest fix is to pass the input value into an environment variable (e.g., RUNS_ON) and then reference that variable using shell-variable expansion ($RUNS_ON) within the command. This ensures any dangerous characters in the input are interpreted as literal filenames, not as shell metacharacters.

Steps:

  • Update the run steps (lines 80 and 83) that use ${{ inputs.runs-on }} to instead reference a shell environment variable.
  • Provide the variable (e.g., RUNS_ON) using the env: keyword in the step, assigning it to the input value.
  • In the run: command, replace coverage-${{ inputs.runs-on }}.xml with coverage-$RUNS_ON.xml.
  • Do the same for any other interpolation of inputs.runs-on in shell code (here, lines 80 and 83). Artifact names and paths passed to actions don't need this change unless they're used as shell commands.
  • No new methods or imports are required; GitHub Actions' syntax suffices for this fix.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: Lukas Burgholzer <burgholzer@me.com>
@burgholzer
burgholzer marked this pull request as ready for review November 29, 2025 10:44
@burgholzer burgholzer added continuous integration Anything related to the CI setup fix Fix for something that is not working labels Nov 29, 2025
@coderabbitai

coderabbitai Bot commented Nov 29, 2025

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Optimized GitHub Actions workflow configuration for Python testing to improve consistency and maintainability of test coverage reporting across different testing environments.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

This pull request updates a GitHub Actions workflow configuration file to introduce an environment variable RUNS_ON instead of directly interpolating the workflow input inputs.runs-on into coverage report file paths. The variable is applied consistently to two test steps.

Changes

Cohort / File(s) Change Summary
GitHub Actions workflow configuration
\.github/workflows/reusable-python-tests.yml``
Introduces RUNS_ON environment variable and updates coverage report file path references from coverage-${{ inputs.runs-on }}.xml to coverage-$RUNS_ON.xml in two test steps: "🐍 Test with minimal versions" and "🐍 Test"

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify environment variable assignment is correctly scoped to the intended steps
  • Confirm the variable substitution works correctly across both steps without regression
  • Ensure file path references remain functionally equivalent

Suggested labels

continuous integration, fix

Poem

🐰 Hops with glee through workflows anew,
Environment variables, clear and true,
No more inputs need interpolation's dance,
CI pipelines march onward at a glance!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description clearly explains the security fix, provides implementation steps, and references the GitHub security alert, but is missing the formal template structure and checklist items. Consider adding the structured template sections (Dependencies, Checklist) and completing the verification items for a more complete pull request description.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main objective: fixing a code injection vulnerability by replacing direct input interpolation with an environment variable approach.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch alert-autofix-100

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a217f9 and 2b92ea9.

📒 Files selected for processing (1)
  • .github/workflows/reusable-python-tests.yml (1 hunks)
🔇 Additional comments (3)
.github/workflows/reusable-python-tests.yml (3)

80-82: Security fix correctly applies environment variable indirection to prevent code injection.

The change properly mitigates the code-injection vulnerability by moving the untrusted inputs.runs-on value out of the shell command line and into an environment variable. Shell variable expansion ($RUNS_ON) treats the expanded value as a literal string, preventing interpretation of metacharacters as shell syntax.


85-87: Security fix applied consistently to second test step.

The same environment variable pattern is correctly applied to the second run: step, ensuring both coverage report commands are protected from code injection.


28-29: Other uses of inputs.runs-on are appropriately unchanged.

Uses on lines 28 (job name), 29 (runs-on: directive), 89 (step name), and 92 (artifact name parameter) are safe from shell-injection risks since they are not interpolated into shell commands. They correctly remain as direct context references.

Also applies to: 89-89, 92-92


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@burgholzer
burgholzer enabled auto-merge (squash) November 29, 2025 10:49
@burgholzer
burgholzer merged commit c07db5c into main Nov 29, 2025
5 checks passed
@burgholzer
burgholzer deleted the alert-autofix-100 branch November 29, 2025 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

continuous integration Anything related to the CI setup fix Fix for something that is not working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant