Skip to content

fix(security): shell-escape PRD-controlled values in shell commands - #196

Closed
sksizer wants to merge 3 commits into
mainfrom
security-review/2026-04-15T18-31-52
Closed

sksizer wants to merge 3 commits into
mainfrom
security-review/2026-04-15T18-31-52

Conversation

@sksizer

@sksizer sksizer commented Apr 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • Command injection fix: PRD metadata (titles, IDs, paths) substituted via format_string into shell commands was not escaped, allowing malicious PRD titles like $(rm -rf /) or foo; rm -rf / to inject arbitrary shell commands
  • Added shell_escape parameter to RunContext.format_string() using shlex.quote, enabled in _run_shell() so all shell-bound substitutions are safely quoted
  • Added comprehensive tests covering subshell injection, semicolon injection, and paths with spaces

Test plan

  • pytest python/darkfactory/workflow/_core_test.py — verify all shell-escape tests pass
  • Confirm existing workflow runs still execute correctly (no over-quoting of template literals)
  • Review that all format_string call sites destined for shell use pass shell_escape=True

sksizer and others added 3 commits April 15, 2026 17:39
PRD metadata (titles, IDs, etc.) flows through format_string into shell
commands via run_shell. Malicious PRD titles like "$(rm -rf /)" or
"foo; rm -rf /" could inject arbitrary commands. Add shell_escape param
using shlex.quote to neutralize metacharacters when values are destined
for shell execution.
@sksizer sksizer closed this Apr 26, 2026
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.

1 participant