fix: use os.pathsep, add tests, update tips for multi-root support

- Use os.pathsep instead of literal ':' so Windows paths (C:\dir) and
  the Windows separator ';' work correctly.
- Add 9 tests covering multi-root behavior: writes inside first/second
  root, writes outside all roots, trailing/leading/double separators,
  all-separators edge case, static deny priority, duplicate dedup.
- Update hermes_cli/tips.py tip string to mention multiple paths.
- Update docs to mention os.pathsep / ; on Windows.

Follow-up for salvaged PR #49557.
This commit is contained in:
kshitijk4poor 2026-06-27 03:33:57 +05:30 committed by kshitij
parent d15cc9bc83
commit cdb1dfbc49
5 changed files with 96 additions and 7 deletions

View file

@ -79,6 +79,95 @@ class TestSafeWriteRoot:
assert _is_write_denied(os.path.expanduser("~/.ssh/id_rsa")) is True
class TestMultipleSafeWriteRoots:
"""HERMES_WRITE_SAFE_ROOT with multiple colon-separated directories."""
def test_write_inside_first_root_allowed(self, tmp_path: Path, monkeypatch):
root_a = tmp_path / "workspace_a"
root_b = tmp_path / "workspace_b"
child = root_a / "subdir" / "file.txt"
os.makedirs(child.parent, exist_ok=True)
os.makedirs(root_b, exist_ok=True)
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", f"{root_a}{os.pathsep}{root_b}")
assert _is_write_denied(str(child)) is False
def test_write_inside_second_root_allowed(self, tmp_path: Path, monkeypatch):
root_a = tmp_path / "workspace_a"
root_b = tmp_path / "workspace_b"
child = root_b / "subdir" / "file.txt"
os.makedirs(child.parent, exist_ok=True)
os.makedirs(root_a, exist_ok=True)
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", f"{root_a}{os.pathsep}{root_b}")
assert _is_write_denied(str(child)) is False
def test_write_outside_all_roots_denied(self, tmp_path: Path, monkeypatch):
root_a = tmp_path / "workspace_a"
root_b = tmp_path / "workspace_b"
outside = tmp_path / "other" / "file.txt"
os.makedirs(root_a, exist_ok=True)
os.makedirs(root_b, exist_ok=True)
os.makedirs(outside.parent, exist_ok=True)
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", f"{root_a}{os.pathsep}{root_b}")
assert _is_write_denied(str(outside)) is True
def test_trailing_separator_ignored(self, tmp_path: Path, monkeypatch):
root = tmp_path / "workspace"
inside = root / "file.txt"
os.makedirs(root, exist_ok=True)
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", f"{root}{os.pathsep}")
assert _is_write_denied(str(inside)) is False
def test_leading_separator_ignored(self, tmp_path: Path, monkeypatch):
root = tmp_path / "workspace"
inside = root / "file.txt"
os.makedirs(root, exist_ok=True)
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", f"{os.pathsep}{root}")
assert _is_write_denied(str(inside)) is False
def test_double_separator_ignored(self, tmp_path: Path, monkeypatch):
root_a = tmp_path / "workspace_a"
root_b = tmp_path / "workspace_b"
os.makedirs(root_a, exist_ok=True)
os.makedirs(root_b, exist_ok=True)
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", f"{root_a}{os.pathsep}{os.pathsep}{root_b}")
# Both roots should still be active
assert _is_write_denied(str(root_a / "file.txt")) is False
assert _is_write_denied(str(root_b / "file.txt")) is False
def test_all_separators_yields_empty_set(self, tmp_path: Path, monkeypatch):
target = tmp_path / "regular.txt"
monkeypatch.setenv("HERMES_WRITE_SAFE_ROOT", os.pathsep * 3)
assert _is_write_denied(str(target)) is False
def test_static_deny_still_wins_with_multiple_roots(self, tmp_path: Path, monkeypatch):
"""Static deny list takes priority even when multiple safe roots include home."""
root = tmp_path / "workspace"
os.makedirs(root, exist_ok=True)
monkeypatch.setenv(
"HERMES_WRITE_SAFE_ROOT",
f"{root}{os.pathsep}{os.path.expanduser('~')}",
)
assert _is_write_denied(os.path.expanduser("~/.ssh/id_rsa")) is True
def test_duplicate_roots_deduplicated(self, tmp_path: Path, monkeypatch):
root = tmp_path / "workspace"
inside = root / "file.txt"
os.makedirs(root, exist_ok=True)
monkeypatch.setenv(
"HERMES_WRITE_SAFE_ROOT",
f"{root}{os.pathsep}{root}",
)
assert _is_write_denied(str(inside)) is False
class TestCheckSensitivePathMacOSBypass:
"""Verify _check_sensitive_path blocks /private/etc paths (issue #8734)."""