Skip to content

fix(tui): pass profile_home to slash_worker subprocess (#40677) - #40959

Closed
iamlukethedev wants to merge 2 commits into
NousResearch:mainfrom
iamlukethedev:fix/40677-profile-home-slash-worker
Closed

fix(tui): pass profile_home to slash_worker subprocess (#40677)#40959
iamlukethedev wants to merge 2 commits into
NousResearch:mainfrom
iamlukethedev:fix/40677-profile-home-slash-worker

Conversation

@iamlukethedev

Copy link
Copy Markdown
Contributor

Problem

Profile-local skills are unavailable in Dashboard/TUI/Desktop GUI because the _SlashWorker subprocess is spawned with os.environ.copy() but does NOT receive the profile-specific HERMES_HOME from the parent session. This causes the subprocess to search ~/.hermes instead of the active profile's skills directory.

Solution

  1. Modify _SlashWorker.init to accept optional profile_home parameter
  2. When profile_home is provided, set env['HERMES_HOME'] = profile_home before spawning the subprocess
  3. Update all 4 call sites to pass profile_home=session.get('profile_home')
  4. Add regression tests for profile-home propagation

Testing

  • Full TUI gateway test suite: 107 tests pass (including 4 new regression tests)
  • Tests cover:
    • profile_home parameter acceptance ✓
    • backward compatibility (None, omitted) ✓
    • argv correctness ✓

Fixes #40677

…list bypass

## Summary
Fix critical RCE vulnerability in tui_gateway shell.exec JSON-RPC method that
allowed arbitrary command execution through denylist bypass attacks.

## Vulnerability Details
- **Issue:** NousResearch#36847 (Critical / RCE via CWE-184)
- **Root cause:** Bypassable regex-based denylist in detect_dangerous_command() + fail-open error handling
- **Impact:** Arbitrary command execution via TUI gateway protocol without user approval

## Attack Vectors (All Now Blocked)
- Backslash escaping: `r\m -rf /home/victim`
- Command substitution: `$(echo rm) -rf /home/victim`
- Parameter expansion: `${0/x/r}m -rf /home/victim`

## Solution
### tools/approval.py
- New structural tokenization layer using shlex for robust parsing
- Pre-check regex for command substitution/parameter expansion syntax
- Dual-layer defense (structural + regex patterns) for comprehensive coverage
- Properly handles quoted strings, pipes, logical operators, subshells

### tui_gateway/server.py
- Changed fail-open to fail-closed error handling on ImportError
- Security logging when approval module unavailable
- Returns error code 5001 to block execution, not silently skip

### tests/tools/test_approval.py
- 28 comprehensive regression tests covering all bypass vectors
- Tests for shell operators (pipes, logical operators, subshells)
- False positive prevention tests (safe commands with 'rm' in quotes)
- TUI gateway security verification tests

## Testing
✅ 224/224 approval tests pass
✅ 16/16 shell escape bypass detection tests
✅ 8/8 TUI gateway security tests
✅ No regressions in existing tests

## Security Notes
- Structural tokenization is the primary defense (catches shell-specific bypasses)
- Regex patterns serve as secondary defense for complex constructs
- Fail-closed design: missing approval module blocks execution
- Defense-in-depth approach prevents future bypass vectors

Fixes NousResearch#36847
…cal skill discovery (NousResearch#40677)

## Problem
Profile-local skills are unavailable in Dashboard/TUI/Desktop GUI because the
_SlashWorker subprocess is spawned with os.environ.copy() but does NOT receive
the profile-specific HERMES_HOME from the parent session. This causes the
subprocess to search ~/.hermes instead of the active profile's skills directory.

## Solution
1. Modify _SlashWorker.__init__ to accept optional profile_home parameter
2. When profile_home is provided, set env['HERMES_HOME'] = profile_home before
   spawning the subprocess
3. Update all 4 call sites to pass profile_home=session.get('profile_home')
4. Add regression tests for profile-home propagation

## Testing
- Full TUI gateway test suite: 107 tests pass
- New tests cover:
  - profile_home parameter acceptance
  - backward compatibility (None, omitted)
  - argv correctness

Fixes NousResearch#40677
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) tool/skills Skills system (list, view, manage) labels Jun 7, 2026
@teknium1

teknium1 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Merged via PR #60180 (#60180). Your profile_home commit was cherry-picked onto current main with your authorship preserved in git log (rebase-merge) — reapplied onto the current hermes_subprocess_env env builder since _SlashWorker had evolved since your branch. Note: your branch's other commit (the #36847 denylist fix) was already fixed on main independently, so only the profile_home commit was salvaged. Paired with @jplew's #56689 (call-time skills-dir resolution) to close #40677 end to end. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Profile-local skills are unavailable in Dashboard/TUI/Desktop GUI because child processes use the root HERMES_HOME

3 participants