Skip to content

Fix script injection risk by passing inputs via env vars#37

Merged
webknjaz merged 1 commit intore-actors:unstable/v1from
illera88:fix/script-injection-inputs
Feb 8, 2026
Merged

Fix script injection risk by passing inputs via env vars#37
webknjaz merged 1 commit intore-actors:unstable/v1from
illera88:fix/script-injection-inputs

Conversation

@illera88
Copy link
Contributor

@illera88 illera88 commented Feb 6, 2026

Summary

  • Moves ${{ inputs.* }} interpolation from the run: script body into the env: block, referencing values as shell variables instead.
  • This prevents potential script injection via crafted input values — environment variables are assigned before the shell interprets the script, so values can never break out of their string context.
  • Also eliminates the heredoc complexity, improving readability.

Problem

The current action.yml interpolates inputs directly into the bash script body using ${{ inputs.* }} inside heredocs. If an attacker controls any input value (e.g. through a dynamically constructed allowed-failures sourced from untrusted data), they could inject arbitrary shell commands by crafting a value that breaks out of the heredoc delimiter.

Solution

Pass inputs through the env: block instead:

env:
  INPUT_ALLOWED_FAILURES: ${{ inputs.allowed-failures }}
  INPUT_ALLOWED_SKIPS: ${{ inputs.allowed-skips }}
  INPUT_JOBS: ${{ inputs.jobs }}
run: |
  python -m normalize_needed_jobs_status \
    "$INPUT_ALLOWED_FAILURES" \
    "$INPUT_ALLOWED_SKIPS" \
    "$INPUT_JOBS"

This is the recommended pattern per GitHub's security hardening docs.

Move action input interpolation from the shell script body into the
env block. This prevents potential script injection via crafted input
values, since environment variables are assigned before the shell
interprets the script — values can never break out of their string
context.

This also eliminates the heredoc complexity, improving readability.
@illera88
Copy link
Contributor Author

illera88 commented Feb 6, 2026

The pre-commit action fails for something unrelated to my PR

Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, interesting. This is similar to what I was going to do but a little simpler. It won't be releasable immediately, though.
In the future, though, please submit draft advisories instead of going public right away.

@illera88
Copy link
Contributor Author

illera88 commented Feb 7, 2026

The CI is failing. Can you fix that please?

@webknjaz
Copy link
Member

webknjaz commented Feb 8, 2026

Alright.. I'm merging this as is but will have to do some maintenance before I can tag it as a stable release.

@webknjaz webknjaz merged commit b4ca9c2 into re-actors:unstable/v1 Feb 8, 2026
13 of 16 checks passed
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.

2 participants

Comments