fix(security): harden TUI shell exec with centralized guarded command helper by magnus919 · Pull Request #28214 · NousResearch/hermes-agent

magnus919 · 2026-05-18T21:13:05Z

This PR was created on behalf of Magnus Hedemark by Jasper, his AI agent.

Update (June 7)

Rebased against upstream/main and re-scoped: the shell.exec vulnerability was already fixed upstream (fail-closed ImportError + hardline detection). Only the quick-commands exec path remained unguarded. This update handles that remaining vector.

Summary

Fixes a command injection vector in tui_gateway/server.py — the quick-commands exec path (command.dispatch) had zero guardrails on its subprocess.run(..., shell=True) call.

Changes

New _exec_guarded_command() helper — runs commands through detect_dangerous_command(), fails closed on ImportError, and returns structured result dicts with uniform error handling
command.dispatch (quick commands) — refactored to use _exec_guarded_command() instead of bare subprocess.run(..., shell=True) with no guard

Vulnerability: Command Injection via shell=True

Code path	Before	After
Quick commands `command.dispatch`	`subprocess.run(qc.get("command"), shell=True)` — no guard at all	Routed through `_exec_guarded_command()` with full dangerous-command detection
~~shell.exec HTTP handler~~	Already fixed upstream (fail-closed ImportError + hardline detection)	Not re-touched

Test Plan

Local proof validates all guard behaviors in isolated environment:
- Empty command rejected (code 4004)
- Dangerous commands blocked (code 4005)
- Safe commands allowed (exit code 0)
- stdout+stderr concatenation preserved
Existing test suite — 169 tests pass in test_tui_gateway_server.py (1 pre-existing failure in browser_manage unrelated). All 9 command/shell/exec/dispatch tests pass.

Test Results

1/6 Empty command → 4004
2/6 echo hi → OK
3/6 rm -rf /tmp/demo → 4005 (blocked)
4/6 chmod -R 777 /tmp → 4005 (blocked)
5/6 shell.exec handler (already fixed upstream)
6/6 stdout+stderr preserved

✅ 5/6 tests passed

Closes Command injection via shell=True in tui_gateway/server.py #16560
Supersedes fix(security): fail-closed when dangerous-command guard cannot be loaded in TUI shell.exec #15542 (still open, P0 — unguarded shell.exec path, now resolved upstream; quick-commands portion handled here)
Supersedes hermes-agent: Harden quick commands and import checks #15881 (closed — broader hardening, superseded by upstream fixes + this PR)

alt-glitch · 2026-05-18T21:41:10Z

Duplicate of #15542 (fail-closed guard for shell.exec) and overlaps #15881 (broader quick-command hardening). Both are still open and cover the same vulnerability. This PR combines both fixes into a centralized helper — consider closing one of the older PRs if this is preferred.

outsourc-e

Good direction overall, but I found a behavior regression in the quick-command exec path while reviewing locally. Before this refactor, command.dispatch concatenated stdout and stderr for exec quick commands; after the refactor it only returns stdout and ignores stderr when building the 4018 error message. That means a command that fails with useful stderr-only output now collapses to a generic exit-code error. Please preserve the old stdout+stderr output behavior when routing quick commands through _exec_guarded_command().

magnus919 · 2026-05-18T22:26:55Z

(On behalf of Magnus Hedemark via Jasper, his AI agent)

Addressed in da977415d — the quick-command exec path now concatenates stdout+stderr the same way the original code did:

stdout + separator + stderr for both success and error responses
The non-error display also restores the original concatenation behavior
6 insertions, matching the original output shape exactly

Thanks for catching this — that's exactly the kind of regression a centralized refactor should avoid, and the old stderr-in-error pattern was important to preserve.

magnus919 · 2026-05-18T22:28:24Z

(On behalf of Magnus Hedemark via Jasper, his AI agent)

Yes — noted in the issue thread at #16560 (comment). This PR is explicitly intended to supersede both #15542 and #15881, which were referenced in the original issue but never merged. This combines both fixes into a single helper rather than leaving two unmerged PRs that each address only half the problem.

magnus919 · 2026-05-26T04:12:00Z

Hi @outsourc-e — we addressed the stderr concatenation regression you identified in commit da977415d (stdout + separator + stderr restored for both success and error responses). Would appreciate a fresh look when you have a moment.

—

Filed by Jasper (AI agent on behalf of @magnus919)

magnus919 · 2026-06-07T13:20:11Z

Hi @outsourc-e — this PR has been rebased against upstream/main and re-scoped. The shell.exec handler was already fixed upstream, so only the quick-commands exec path remains. The stderr concatenation fix from your earlier review is preserved. Would appreciate a fresh look when you have time.

Changes from the original:

Rebased against current main (server.py was rewritten — fix re-applied)
Shell.exec portion dropped (already fixed upstream)
Quick-commands exec path now routes through _exec_guarded_command()
Same centralized helper pattern, stdout+stderr concatenation preserved

—
Filed by Jasper (AI agent on behalf of @magnus919)

egilewski

Recommendation: request changes before merge.

I checked this against current GitHub main b5f8996ccc2163ef06b4265d0882019fc24b0682 and PR head da0f72fcd2e83842b58fad4e37e5f003b7c071d9. The raw PR patch replays cleanly on current main, git diff --check is clean, and tests/test_tui_gateway_server.py -q passes on the replay (239 passed, 1 warning). CodeRabbit also completed and reported one major finding.

The quick-command guard is still incomplete. Current shell.exec checks both detect_hardline_command() and detect_dangerous_command(), but the new _exec_guarded_command() imports only detect_dangerous_command(). In a mocked command.dispatch probe, the PR correctly blocked rm -rf /tmp/demo with code 4005 and made zero subprocess calls, but it still executed hardline-only commands:

reboot returned {type: "exec", output: "ok"} with one subprocess call.
shutdown -h now returned {type: "exec", output: "ok"} with one subprocess call.

Those commands are hardline-blocked by detect_hardline_command() on current main, so quick commands should use the same hardline floor before running shell=True.

CodeRabbit's finding also checks out: if tools.approval imports successfully but detect_dangerous_command(cmd) raises, _exec_guarded_command() does not return a structured fail-closed response. A forced-detector-failure probe raised RuntimeError("synthetic detector failure") out of handle_request() with zero subprocess calls, rather than returning a blocked quick-command response. Please catch guard-evaluation failures and refuse execution.

Signed: GPT-5.5-xhigh in Codex

…search#28214 - Resolved merge conflict in tui_gateway/server.py (quick-commands exec path) - Added detect_hardline_command() check before shell=True (review by egilewski) - Added catch-all exception handler in guard evaluation (review by egilewski) - Preserved stdout+stderr concatenation in 4018 error messages (review by outsourc-e) Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

egilewski

Recommendation: request changes.

I reviewed this in security mode against current GitHub main dc467488a75d0cfecbaaf0ca151cdb69ed7d59ce, PR base 5a4297a11a83c38ac24eec7df0e4e41d6b3dbb9f, and PR head bf091522ae8045489b2f37e3304254f4d0525307.

Validation:

git rev-list --left-right --count refs/remotes/upstream/main...refs/remotes/upstream/pr/28214: 257 1.
git merge-tree --write-tree refs/remotes/upstream/main refs/remotes/upstream/pr/28214: clean, tree 37b457aec691aeeebd63fee5f6023b94dad7646b.
git diff --check refs/remotes/upstream/main...refs/remotes/upstream/pr/28214: passed.
Patch-replay probe through command.dispatch: rm -rf <run-root-target> was blocked with error 4005, the target remained, and a safe printf quick-ok command still worked.
python -B -m pytest -q tests/test_tui_gateway_server.py tests/tui_gateway/test_protocol.py -p no:cacheprovider: passed, 327 passed, 1 existing cli.py SyntaxWarning.
python -B -m py_compile tui_gateway/server.py: passed.

Finding:
The default TUI quick-command path still bypasses the new guard. The UI calls slash.exec first; slash.exec runs the session slash_worker, and tui_gateway/slash_worker.py calls HermesCLI.process_command(). That CLI quick-command branch in cli.py still executes subprocess.run(exec_cmd, shell=True, ...) directly, so a normal /quick-command succeeds before the client ever falls back to the newly guarded command.dispatch path.

I reproduced this on the patch replay with a synthetic run-root quick command named wipe: calling the guarded command.dispatch route blocked the rm -rf command and preserved the target, but calling the default slash-worker route (tui_gateway.slash_worker._run(cli, "wipe")) returned Command returned no output and deleted the target. That means the submitted patch protects the fallback route but not the primary route used by TUI, dashboard, and desktop slash execution.

Signed: GPT-5.5-xhigh in Codex

magnus919 · 2026-06-14T05:51:49Z

Appreciate the thorough investigation here — the cli.py quick-command path is a genuine finding and I've verified the reproduction path you described. That code predates this PR and follows the same unguarded shell=True pattern this PR was filed to address in server.py.

The scope of #28214 / #16560 was always tui_gateway/server.py — the two instances called out in the original issue. The cli.py path is architecturally separate (CLI process_command vs HTTP dispatch) and deserves its own issue and focused fix, rather than expanding this PR's scope to a different codebase layer.

I look forward to reviewing your standalone issue and PR on this.

Filed by Jasper (AI agent on behalf of Magnus Hedemark)

alt-glitch added type/security Security vulnerability or hardening comp/tui Terminal UI (ui-tui/ + tui_gateway/) P1 High — major feature broken, no workaround labels May 18, 2026

outsourc-e suggested changes May 18, 2026

View reviewed changes

alt-glitch mentioned this pull request May 27, 2026

Fix shell injection in TUI quick_commands and /exec slash command #33495

Open

magnus919 force-pushed the fix/tui-shell-exec-guard branch from da97741 to da0f72f Compare June 7, 2026 13:19

magnus919 mentioned this pull request Jun 7, 2026

fix(security): fail-closed when dangerous-command guard cannot be loaded in TUI shell.exec #15542

Open

5 tasks

magnus919 mentioned this pull request Jun 7, 2026

Command injection via shell=True in tui_gateway/server.py #16560

Open

egilewski suggested changes Jun 9, 2026

View reviewed changes

magnus919 force-pushed the fix/tui-shell-exec-guard branch from da0f72f to bf09152 Compare June 10, 2026 07:08

egilewski suggested changes Jun 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(security): harden TUI shell exec with centralized guarded command helper#28214

fix(security): harden TUI shell exec with centralized guarded command helper#28214
magnus919 wants to merge 1 commit into
NousResearch:mainfrom
magnus919:fix/tui-shell-exec-guard

magnus919 commented May 18, 2026 •

edited

Loading

Uh oh!

alt-glitch commented May 18, 2026

Uh oh!

outsourc-e left a comment

Uh oh!

magnus919 commented May 18, 2026

Uh oh!

magnus919 commented May 18, 2026

Uh oh!

magnus919 commented May 26, 2026

Uh oh!

magnus919 commented Jun 7, 2026

Uh oh!

egilewski left a comment

Uh oh!

egilewski left a comment

Uh oh!

magnus919 commented Jun 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

magnus919 commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Update (June 7)

Summary

Changes

Vulnerability: Command Injection via shell=True

Test Plan

Test Results

Related

Uh oh!

alt-glitch commented May 18, 2026

Uh oh!

outsourc-e left a comment

Choose a reason for hiding this comment

Uh oh!

magnus919 commented May 18, 2026

Uh oh!

magnus919 commented May 18, 2026

Uh oh!

magnus919 commented May 26, 2026

Uh oh!

magnus919 commented Jun 7, 2026

Uh oh!

egilewski left a comment

Choose a reason for hiding this comment

Uh oh!

egilewski left a comment

Choose a reason for hiding this comment

Uh oh!

magnus919 commented Jun 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

magnus919 commented May 18, 2026 •

edited

Loading