Skip to content

ci: harden security-review.yml against script injection#3365

Open
nebeid wants to merge 1 commit into
aws:mainfrom
nebeid:harden-security-review-script-injection
Open

ci: harden security-review.yml against script injection#3365
nebeid wants to merge 1 commit into
aws:mainfrom
nebeid:harden-security-review-script-injection

Conversation

@nebeid

@nebeid nebeid commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Issues:

Addresses V2265233159

Description of changes:

Continuation of 8afd2f6, extending the same script-injection hardening to security-review.yml.

Directly interpolating ${{ github.event.* }} context expressions into run: shell bodies lets Actions splice them in as text before the shell runs — a script-injection risk. This PR hoists each flagged value into the step's env: block and references it as a quoted shell variable instead. Behavior is unchanged.

  • Update commit status: hoist github.event.repository.name, github.event.pull_request.number, and github.event.pull_request.head.sha into env.
  • Post review comment: hoist github.event.repository.name and github.event.pull_request.number into env.

Call-outs:

  • This workflow runs on pull_request_target, so hardening these interpolations matters even though the flagged fields (repo name, PR number, commit SHA) are format-constrained.
  • ${{ github.repository }} is left inline — it is not github.event.* attacker-influenced input.

Testing:

CI-config-only, no product code impact. File parses as valid YAML; no ${{ github.event.* }} interpolations remain in any run: body.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

Move untrusted github.event.* values (repository.name,
pull_request.number, pull_request.head.sha) into env: vars and
reference them as quoted shell variables instead of interpolating
${{ }} templates directly into run: blocks.

Same hardening applied in
aws@8afd2f6
which covered the other workflows but not security-review.yml.
@nebeid
nebeid requested a review from a team as a code owner July 20, 2026 22:37
@github-actions

Copy link
Copy Markdown
Contributor

🔒 Security ReviewView Report

Please review before merging.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.20%. Comparing base (b06d89b) to head (cbf9e16).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3365   +/-   ##
=======================================
  Coverage   78.20%   78.20%           
=======================================
  Files         695      695           
  Lines      124172   124172           
  Branches    17238    17239    +1     
=======================================
+ Hits        97104    97106    +2     
+ Misses      26147    26145    -2     
  Partials      921      921           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nebeid
nebeid requested a review from justsmth July 22, 2026 13:21
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.

3 participants