Skip to content

fix(ci): patch shell injection and unsafe pull_request_target#32

Open
stevefulme1 wants to merge 7 commits into
redhat-cop:mainfrom
stevefulme1:fix/ci-security-vulnerabilities
Open

fix(ci): patch shell injection and unsafe pull_request_target#32
stevefulme1 wants to merge 7 commits into
redhat-cop:mainfrom
stevefulme1:fix/ci-security-vulnerabilities

Conversation

@stevefulme1

Copy link
Copy Markdown
Contributor

Summary

  • gitleaks.yml: Removed pull_request_target trigger that checked out untrusted PR code with repo write permissions. The pull_request trigger already covers same-repo PRs safely.
  • slack-pr-notifications.yml: Moved all user-controlled GitHub context values (PR title, user login, review state) from inline ${{ }} interpolation in run: steps to env: block variables, preventing shell injection via crafted PR titles.

Both vulnerabilities were identified by Semgrep static analysis:

  • pull-request-target-code-checkout (ERROR)
  • run-shell-injection (ERROR)

Test plan

  • Semgrep scan returns clean on both files after the fix
  • Gitleaks workflow still triggers on push and pull_request to main
  • Slack notifications still fire correctly on PR open/close/review events

🤖 Generated with Claude Code

@spyrexd spyrexd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Does this supersede #31 ?

spyrexd
spyrexd previously approved these changes May 19, 2026
@stevefulme1 stevefulme1 force-pushed the fix/ci-security-vulnerabilities branch from 4739472 to 870a5a7 Compare June 18, 2026 12:43
docs: regenerate network_mgmt README after OVN layer2 merge
@stevefulme1 stevefulme1 deployed to external-ci June 29, 2026 18:13 — with GitHub Actions Active
@sabre1041

Copy link
Copy Markdown
Contributor

@stevefulme1 this looks good. Only outstanding item is that there is a COPYING file that appears to be almost exactly the same as the LICENSE. One we resolve this discussion, we should be able to integrate these changes

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