Skip to content

fix(ci): complete injection prevention in translation review workflow#17576

Merged
wackerow merged 1 commit into
devfrom
fix/ci-complete-injection-prevention
Feb 17, 2026
Merged

fix(ci): complete injection prevention in translation review workflow#17576
wackerow merged 1 commit into
devfrom
fix/ci-complete-injection-prevention

Conversation

@pettinarip
Copy link
Copy Markdown
Member

Summary

  • Move remaining GitHub context values to env blocks in "Get PR number" step to prevent shell injection
  • Add numeric validation for PR numbers before downstream usage
  • Apply same pattern to "Post acknowledgment" step for consistency

Context

This extends the security fixes from #17560, which addressed auth bypass and command injection vulnerabilities but left the "Get PR number" step using direct shell interpolation. While the risk was lower (PR numbers are typically numeric), this change applies defense-in-depth by:

  1. Moving github.event_name, github.event.inputs.pr_number, github.event.pull_request.number, and github.event.issue.number to the env: block
  2. Adding explicit numeric validation before outputting the PR number
  3. Applying the same pattern to the "Post acknowledgment" step

Test plan

  • Verify workflow triggers correctly on workflow_dispatch
  • Verify workflow triggers correctly on issue_comment
  • Verify workflow triggers correctly on pull_request
  • Verify numeric validation rejects non-numeric inputs

Move remaining GitHub context values to env blocks to prevent potential
shell injection. Add numeric validation for PR numbers before downstream
usage.

Extends the security fixes from #17560 to cover the "Get PR number" and
"Post acknowledgment" steps that were not addressed in the original PR.
@netlify
Copy link
Copy Markdown

netlify Bot commented Feb 16, 2026

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit 61c5113
🔍 Latest deploy log https://app.netlify.com/projects/ethereumorg/deploys/6992e1aa2e9f2200082dd407
😎 Deploy Preview https://deploy-preview-17576.ethereum.it
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
7 paths audited
Performance: 61 (🟢 up 6 from production)
Accessibility: 94 (no change from production)
Best Practices: 100 (no change from production)
SEO: 100 (no change from production)
PWA: 59 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions Bot added the tooling 🔧 Changes related to tooling of the project label Feb 16, 2026
@wackerow
Copy link
Copy Markdown
Member

Security Review

PR: fix(ci): complete injection prevention in translation review workflow #17576
Reviewed: 2026-02-16
Risk: CLEAR

Findings

No security issues identified.

This PR is a security hardening that closes the remaining injection vectors in the "Get PR number" and "Post acknowledgment" steps, extending the fixes from #17560.

Changes reviewed:

  • "Get PR number" step (.github/workflows/claude-review-translations.yml:84-101): Moves github.event_name, github.event.inputs.pr_number, github.event.pull_request.number, and github.event.issue.number from direct ${{ }} interpolation in run: to env: block. Adds numeric regex validation (^[0-9]+$) before output. Both correct.

  • "Post acknowledgment" step (:162-176): Moves steps.pr.outputs.number and run URL to env vars. Properly quotes "$PR_NUMBER" in the gh pr comment call. Correct.

  • Remaining ${{ }} in file: All remaining references (lines 177-189) are inside with: YAML inputs to the Claude Code Action, not run: blocks — not shell injection vectors.

  • Authorization logic: Unchanged — still uses correct fromJSON('["..."]') array matching (lines 55, 62, 69). No regression.


Areas Reviewed

  • GitHub Workflow Security

Not applicable: API Route Security, SSRF & External Fetching, XSS & Content Injection, Security Headers & Transport, Dependencies & Supply Chain, Secrets & Configuration

Limitations

None.

@wackerow wackerow merged commit d1c7250 into dev Feb 17, 2026
8 checks passed
@wackerow wackerow deleted the fix/ci-complete-injection-prevention branch February 17, 2026 16:37
@fmendezespinoza887-max
Copy link
Copy Markdown

IMG_20260217_115630075_HDR.jpg

@pettinarip pettinarip mentioned this pull request Feb 19, 2026
@pettinarip pettinarip mentioned this pull request Feb 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tooling 🔧 Changes related to tooling of the project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants