Skip to content

[security] reject blocked roots for remote workspaces - #3731

Closed
Hinotoi-agent wants to merge 2 commits into
nesquena:masterfrom
Hinotoi-agent:security/remote-workspace-blocked-roots
Closed

Hinotoi-agent wants to merge 2 commits into
nesquena:masterfrom
Hinotoi-agent:security/remote-workspace-blocked-roots

Conversation

@Hinotoi-agent

Copy link
Copy Markdown
Contributor

Summary

This hardens remote-terminal workspace resolution so the remote-workspace shortcut cannot accept blocked local system roots such as /etc when the configured remote terminal cwd is also under that root.

The normal local workspace path already rejects these roots, but _remote_terminal_workspace_candidate() returned early for paths under the remote cwd before the blocked-root guard ran. Later workspace file operations use s.workspace as a local Path, so accepting a remote cwd like /etc can convert the intended remote-side path into a trusted local workspace root.

Security impact

Before this change, with an SSH/remote terminal profile whose target-side cwd is /etc, workspace resolution accepted /etc as /private/etc on macOS. A session created with that workspace could then pass the workspace to local file helpers, allowing reads from local system files such as hosts through workspace file-read paths.

Redacted local proof before the fix:

remote_terminal_backend=ssh
remote_terminal_cwd=/etc
accepted_workspace=/private/etc
session_workspace=/private/etc
file_read_path=hosts
file_read_size=213
file_read_contains_localhost=True
file_read_first_line=##

After the fix, both registration and trusted workspace resolution reject the same root:

validate_workspace_to_add=REJECTED ValueError: Path points to a system directory: /private/etc
resolve_trusted_workspace=REJECTED ValueError: Path points to a system directory: /private/etc

Fix

  • Apply the existing _is_blocked_workspace_path() guard inside _remote_terminal_workspace_candidate() before accepting candidate paths under the remote cwd.
  • Check both the requested candidate and the configured remote cwd/base path.
  • Add regression coverage for an SSH backend configured with /etc as cwd.

Validation

python3 -m pytest tests/test_remote_terminal_workspace.py tests/test_workspace_blocked_roots_macos.py tests/test_workspace_inaccessible_paths.py -q
41 passed, 1 warning in 2.09s
python3 -m pytest tests/test_remote_terminal_workspace.py tests/test_workspace_blocked_roots_macos.py tests/test_workspace_inaccessible_paths.py tests/test_workspace_symlink_containment.py tests/test_workspace_upload.py tests/test_workspace_git.py -q
108 passed, 1 skipped, 1 warning in 12.97s

Duplicate check

I checked public issue/PR searches for remote terminal workspace/system-root wording and did not find an exact duplicate. The closest visible issue was #3673, but that appears to cover terminal backend behavior rather than this blocked-root workspace/file-boundary case.

@greptile-apps

greptile-apps Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hardens remote-workspace resolution by inserting _is_blocked_workspace_path() checks inside _remote_terminal_workspace_candidate() before the early-return that previously allowed blocked roots (e.g. /etc) to be accepted as valid remote workspaces. The fix correctly closes the explicit registration and resolution paths (validate_workspace_to_add, resolve_trusted_workspace).

  • The two-line guard in api/workspace.py checks both the requested candidate and the configured terminal.cwd, returning None for any blocked path so callers fall through to the existing _is_blocked_workspace_path raise.
  • The new parametrized test covers both the exact blocked root (/etc) and a sub-path (/etc/ssh) against both public validation functions.
  • _profile_default_workspace() still returns terminal.cwd (e.g. /private/etc) without a blocked-root check; after this fix get_last_workspace() always falls back to it for blocked-cwd profiles, flowing an unguarded system root into the media-serving allowed_roots calculation and crashing the session workspace recovery helper.

Confidence Score: 4/5

Safe to merge for the primary attack vector described, but one secondary path through _profile_default_workspace() still returns a blocked system root without validation and should be addressed before this is considered a complete fix.

The explicit workspace validation functions (validate_workspace_to_add, resolve_trusted_workspace) are correctly hardened. However, _profile_default_workspace() returns terminal.cwd without a blocked-root check, and after this fix get_last_workspace() will consistently return that unguarded value for profiles with a blocked cwd — feeding it into the media-serving allowed_roots list and causing the workspace recovery helper to raise an exception rather than fall back gracefully.

api/workspace.py — specifically the _profile_default_workspace() function and how its return value flows through get_last_workspace() into routes.py.

Important Files Changed

Filename Overview
api/workspace.py Adds blocked-root guard inside _remote_terminal_workspace_candidate() before the _is_within early-return; fix is correct for the explicit registration/resolution paths but _profile_default_workspace() still returns blocked cwd values without validation.
tests/test_remote_terminal_workspace.py Adds parametrized regression test covering both the exact blocked root (/etc) and a sub-path (/etc/ssh); tests both validate_workspace_to_add and resolve_trusted_workspace.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["validate_workspace_to_add(path)\nor resolve_trusted_workspace(path)"] --> B["_remote_terminal_workspace_candidate(path)"]
    B --> C{"remote cwd\nconfigured?"}
    C -- No --> D["return None"]
    C -- Yes --> E["resolve candidate & base"]
    E --> F{"NEW: _is_blocked_workspace_path\n(candidate) OR (base)?"}
    F -- Yes --> G["return None ✅ (fix)"]
    F -- No --> H{"candidate == base\nor _is_within(base)?"}
    H -- No --> I["return None"]
    H -- Yes --> J["return candidate"]
    G --> K["Caller falls through to\n_is_blocked_workspace_path check"]
    K --> L["ValueError: Path points to\na system directory ✅"]

    M["get_last_workspace()"] --> N["valid_last_workspace(stored_path)"]
    N --> B2["_remote_terminal_workspace_candidate(stored_path)"]
    B2 --> O{"blocked cwd?"}
    O -- Yes --> P["return None (after fix)"]
    P --> Q["valid_last_workspace returns None"]
    Q --> R["_profile_default_workspace()"]
    R --> S["returns terminal.cwd resolved\n⚠️ no blocked-root check"]
    S --> T["get_last_workspace returns /private/etc"]
    T --> U["media handler: allowed_roots += /private/etc ⚠️"]
    T --> V["resolve_trusted_workspace raises ValueError — recovery broken ⚠️"]
Loading

Comments Outside Diff (1)

  1. api/workspace.py, line 103-144 (link)

    P1 _profile_default_workspace() returns a blocked system root without validation

    _profile_default_workspace() returns terminal.cwd resolved — /private/etc on macOS — without ever calling _is_blocked_workspace_path. This becomes a problem because this fix (correctly) makes _remote_terminal_workspace_candidate() return None for every path under a blocked cwd, which causes get_last_workspace() to always fall back to _profile_default_workspace() for these profiles. That fallback value is then used in two places without a blocked-root guard:

    1. _resolve_chat_workspace_with_recovery (line 11391) calls resolve_trusted_workspace(get_last_workspace()) as a recovery fallback. After this fix, that call raises ValueError for a profile with cwd="/etc", breaking workspace recovery for any session on that profile — the recovery helper itself crashes.

    2. The media file serving handler (lines 9421-9424) adds Path(get_last_workspace()).resolve() to allowed_roots if it .is_dir(). With the fallback returning /private/etc, any request to that endpoint with path under /etc passes the within_allowed check.

    The minimal fix is to add a blocked-root check at the terminal.cwd branch of _profile_default_workspace(), or have get_last_workspace() validate the return value of _profile_default_workspace() against the same guard.

Reviews (2): Last reviewed commit: "test: cover remote blocked root subpaths" | Re-trigger Greptile

Comment thread tests/test_remote_terminal_workspace.py Outdated
@Hinotoi-agent

Copy link
Copy Markdown
Contributor Author

Addressed in 184892b0.

I expanded the regression test to cover both the blocked root (/etc) and a subpath under it (/etc/ssh) when the remote terminal cwd is /etc, and tightened the expected error to Path points to a system directory instead of allowing the fallback Path does not exist branch.

Local validation:

python3 -m pytest tests/test_remote_terminal_workspace.py -q
6 passed, 1 warning in 2.17s

python3 -m pytest tests/test_remote_terminal_workspace.py tests/test_workspace_git.py -q
43 passed, 1 warning in 11.38s

SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
…fix nesquena#3731) (nesquena#3744)

* fix: reject blocked roots for remote workspaces

* test: cover remote blocked root subpaths

* docs(changelog): v0.51.296 security fix + backfill v0.51.295 entries

- v0.51.296: nesquena#3731 remote-workspace blocked-root rejection.
- Backfill the v0.51.295 release block (the nesquena#3739 model-pick entry + promote the
  nesquena#3570 revert out of [Unreleased]) which a stage-rebuild dropped from the prior
  release's CHANGELOG. git-describe versioning makes CHANGELOG-after-tag acceptable.

---------

Co-authored-by: hinotoi-agent <paperlantern.agent@gmail.com>
Co-authored-by: nesquena-hermes <[email protected]>
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