Skip to content

security: fix shell injection risk in _goal_coverage#7

Open
shigel wants to merge 1 commit into
mainfrom
s2s/fix-shell-injection
Open

security: fix shell injection risk in _goal_coverage#7
shigel wants to merge 1 commit into
mainfrom
s2s/fix-shell-injection

Conversation

@shigel

@shigel shigel commented Apr 9, 2026

Copy link
Copy Markdown
Owner

Summary

autoforge/harness/run_goal.py:129 passed test_cmd (read from autoforge.yml) to subprocess.run(..., shell=True). If the YAML config comes from an untrusted source, this allows command injection.

Fix

# Before
result = subprocess.run(test_cmd, shell=True, timeout=300)

# After
import shlex
result = subprocess.run(shlex.split(test_cmd), timeout=300)

shlex.split handles quoted arguments correctly. This preserves the existing behavior for normal commands like pytest -v while eliminating the shell injection vector.

How this was detected

This issue was detected automatically by the Survey2Ship s2s scan command:

$ s2s scan --repo-root /path/to/autoforge
Scanned 74 files in 0.1s
High: 4, Medium: 2, Low: 0
  [MEDIUM] autoforge/harness/run_goal.py:125 — Unsafe pattern: shell_injection_risk

Survey2Ship is a research-driven autonomous improvement loop. This finding
is based on claim-0003 (arxiv:2604.07223)
which identifies agent execution traces as an under-explored vulnerability surface.

Verification

After the fix:

$ s2s scan --repo-root /path/to/autoforge
High: 4, Medium: 1, Low: 0   (was Medium: 2)

The shell_injection_risk finding is gone. The remaining 4 HIGH findings are in tests/core/test_hooks.py and appear to be intentional fake credentials for testing.


Generated by Survey2Ship SecurityScanner

The test_cmd from YAML config was passed to subprocess with shell=True,
allowing command injection if the autoforge.yml file is untrusted.

Fix: use shlex.split() and avoid shell=True.

Detected by [Survey2Ship](https://github.com/shigel/survey2ship) SecurityScanner.
Research basis: claim-0003 (arxiv:2604.07223) — agent execution traces
as an under-explored vulnerability surface.
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