Skip to content

fix: prevent shell injection vulnerabilities (SEC-4316)#154

Merged
pkanoongo merged 1 commit intomainfrom
fix/semgrep-shell-injection-sec-4316
Oct 21, 2025
Merged

fix: prevent shell injection vulnerabilities (SEC-4316)#154
pkanoongo merged 1 commit intomainfrom
fix/semgrep-shell-injection-sec-4316

Conversation

@pkanoongo
Copy link
Contributor

🔒 Security Fix: Shell Injection Prevention

⚠️ This PR is in DRAFT status for review. Mark as "Ready for review" when approved.

Issue

Fixes 7 shell injection vulnerabilities detected by Semgrep in GitHub Actions workflows.

Jira: SEC-4316
Semgrep Rule: yaml.github-actions.security.run-shell-injection.run-shell-injection
Severity: High
Findings:

  • Line 38: action.yaml
  • Line 96: action.yaml
  • Line 141: action.yaml
  • Line 32: commitlint/action.yaml
  • Line 48: commitlint/action.yaml
  • Line 79: commitlint/action.yaml
  • Line 109: commitlint/action.yaml

🔍 What Changed

  • Moved all GitHub Actions inputs to environment variables before use in shell scripts
  • Prevents potential command injection attacks
  • Zero functional changes - purely security hardening

🛡️ Security Context

Direct interpolation of ${{ inputs.* }} in shell scripts can allow command injection if untrusted input is provided.

Vulnerable Pattern:

run: |
  if [[ -n "${{ inputs.github-token }}" ]]; then  # ❌ Can execute arbitrary commands
    git config --global url."https://x-access-token:${{ inputs.github-token }}@github.com/"
  fi

Secure Pattern:

env:
  GITHUB_TOKEN: ${{ inputs.github-token }}
run: |
  if [[ -n "$GITHUB_TOKEN" ]]; then  # ✅ Treated as data, not code
    git config --global url."https://x-access-token:${GITHUB_TOKEN}@github.com/"
  fi

📚 References

✅ Testing Checklist

  • All existing functionality preserved
  • No breaking changes to action interface
  • Workflow YAML syntax validated
  • Pre-commit checks passing

🎯 Review Checklist for Maintainer

  • Review all file changes
  • Verify environment variable naming is clear
  • Confirm no functional regressions
  • Check CI/CD results
  • Mark as "Ready for review" when approved

- Use environment variables instead of direct input interpolation
- Addresses Semgrep finding: run-shell-injection
- Fixes SEC-4316

Changes:
- action.yaml: Fixed 3 shell injection vulnerabilities
- commitlint/action.yaml: Fixed 4 shell injection vulnerabilities
@github-actions
Copy link

Release notes preview

Below is a preview of the release notes if your PR gets merged.


3.3.1 (2025-10-20)

Bug Fixes

  • prevent shell injection in GitHub Actions workflows (805652f)

Miscellaneous

  • add sec scan: add sec scan (8165632)
  • add sec scan: add sec scan (a8e4d31)
  • security scan: security scan (1297a1e)
  • deps: update actions/checkout action to v5 (f4e5778)
  • deps: update actions/setup-node action to v5 (f6d6c5d)
  • deps: update actions/setup-python action to v6 (c1dee1f)
  • deps: update pre-commit hook alessandrojcm/commitlint-pre-commit-hook to v9.23.0 (54460e1)
  • deps: update pre-commit hook pre-commit/mirrors-eslint to v9.33.0 (0529d05)
  • deps: update pre-commit hook pre-commit/mirrors-eslint to v9.34.0 (c932269)
  • deps: update pre-commit hook pre-commit/mirrors-eslint to v9.35.0 (185a4eb)
  • deps: update pre-commit hook pre-commit/mirrors-eslint to v9.36.0 (f9d66f4)
  • deps: update pre-commit hook pre-commit/mirrors-eslint to v9.37.0 (ab48f63)
  • deps: update pre-commit hook pre-commit/mirrors-eslint to v9.38.0 (cc72c32)
  • deps: update pre-commit hook pre-commit/pre-commit-hooks to v6 (478af8d)
  • deps: update pre-commit hook rhysd/actionlint to v1.7.8 (e3cf0d0)

@pkanoongo pkanoongo marked this pull request as ready for review October 21, 2025 15:45
@pkanoongo pkanoongo requested a review from a team as a code owner October 21, 2025 15:45
@pkanoongo pkanoongo merged commit a791abb into main Oct 21, 2025
10 checks passed
@pkanoongo pkanoongo deleted the fix/semgrep-shell-injection-sec-4316 branch October 21, 2025 16:48
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