diff --git a/skills/shadow-frog-dream/_worktree_safety.py b/skills/shadow-frog-dream/_worktree_safety.py index 05c0b19..4fc70fa 100644 --- a/skills/shadow-frog-dream/_worktree_safety.py +++ b/skills/shadow-frog-dream/_worktree_safety.py @@ -36,7 +36,7 @@ import os import re import sys -from pathlib import Path +from pathlib import Path, PurePath # Same regex `dream-setup.sh` validates --slug and --namespace against. # Keep these in lockstep — if one widens, the other must follow. @@ -49,7 +49,7 @@ # the check on macOS (verified empirically: macOS `realpath /tmp` returns # `/private/tmp` which is not in the list, so the literal `/tmp` check is # what catches it). -_FORBIDDEN_BASES = frozenset({ +_UNIX_FORBIDDEN_BASES = frozenset({ "/", "/bin", "/boot", "/dev", "/etc", "/home", "/lib", "/lib32", "/lib64", "/Library", "/mnt", "/media", "/opt", "/private", "/proc", "/root", @@ -70,6 +70,56 @@ def _strip_macos_private(p: str) -> str: return p +def _get_windows_forbidden_bases() -> set[str]: + """Sensitive Windows base dirs, read from the environment at call time + (%SystemRoot%, %ProgramFiles%, %USERPROFILE%, ...); empty on POSIX. + A function (not a constant) because these are runtime/env-derived, and so + they stay monkeypatch-able in tests. + """ + if os.name != "nt": + return set() + env_vars = ("SystemRoot", "windir", "ProgramFiles", "ProgramFiles(x86)", + "ProgramData", "USERPROFILE", "PUBLIC") + return {os.path.normpath(v) for v in map(os.environ.get, env_vars) if v} + + +def _normalize_path(p: str) -> str: + """Normalize a path into a comparison key. `os.path.normcase` lowercases and + unifies `/`->`\\` on Windows (no-op on POSIX); stripping macOS's `/private` + prefix lets `/tmp` and its resolved `/private/tmp` form compare equal. + """ + return os.path.normcase(_strip_macos_private(p)) + + +def _is_filesystem_root(p: str) -> bool: + """True if `p` is a filesystem root with no parent to sweep under: POSIX + `/`, a Windows drive root (`C:\\`), or a UNC share root + (`\\\\server\\share`). A root is its own parent: `dirname(p) == p`. + """ + p = os.path.normpath(p) + return os.path.dirname(p) == p + + +# The two sources of "sensitive base" are ASYMMETRIC ON PURPOSE: +# * POSIX roots are fixed, known-at-author-time paths -> a CONSTANT +# (`_UNIX_FORBIDDEN_BASES`). +# * Windows roots come from the environment (%SystemRoot% etc.), vary by +# machine/user, and aren't known until runtime -> a FUNCTION. +# `_get_forbidden_bases()` hides the split so callers just ask "what's forbidden here?". +def _get_forbidden_bases() -> set[str]: + """Sensitive-base comparison keys for the CURRENT OS, plus the user's home. + Selects the platform-appropriate source: the static `_UNIX_FORBIDDEN_BASES` + on POSIX, the env-derived `_get_windows_forbidden_bases()` on Windows. + """ + raw = _get_windows_forbidden_bases() if os.name == "nt" else _UNIX_FORBIDDEN_BASES + keys = {_normalize_path(p) for p in raw} + home = os.path.expanduser("~") + if home and home != "~": + keys.add(_normalize_path(home)) + keys.add(_normalize_path(os.path.realpath(home))) + return keys + + class UnsafePath(ValueError): """Raised when the candidate path fails any of the safety rules.""" @@ -88,37 +138,41 @@ def safe_worktree_path(path: str, base: str) -> Path: raise UnsafePath(f"empty or non-string base: {base!r}") # Rule 2: absolute. - if not path.startswith("/"): + if not os.path.isabs(path): raise UnsafePath(f"worktree path is not absolute: {path!r}") - if not base.startswith("/"): + if not os.path.isabs(base): raise UnsafePath(f"base is not absolute: {base!r}") # Rule 3: no ".." traversal in the literal input. Catches things that # would otherwise normalize past the base. - for part in path.split("/"): - if part == "..": - raise UnsafePath(f"worktree path contains '..': {path!r}") - for part in base.split("/"): - if part == "..": - raise UnsafePath(f"base contains '..': {base!r}") + if ".." in PurePath(path).parts: + raise UnsafePath(f"worktree path contains '..': {path!r}") + if ".." in PurePath(base).parts: + raise UnsafePath(f"base contains '..': {base!r}") # Resolve the base: this is what we compare against. base_res = os.path.realpath(base) - # Rule 4: refuse sensitive bases. Compare against three normalizations - # so a symlink shenanigan (`$HOME/my-worktrees → /`) AND macOS's - # implicit `/tmp → /private/tmp` redirection both fail closed. + # Rule 4: refuse sensitive bases. Build ONE normalized candidate set from the + # literal input AND the symlink-resolved form (so a symlink like + # `$HOME/dreams -> /` can't smuggle a root past us). Normalizing is required + # for the (b) set-match and harmless for the (a) root test, so we share it. base_norm = os.path.normpath(base) - candidates = {base_norm, base_res, _strip_macos_private(base_res)} - forbidden = set(_FORBIDDEN_BASES) - home = os.path.expanduser("~") - if home and home != "~": - forbidden.add(home) - forbidden.add(_strip_macos_private(os.path.realpath(home))) - if candidates & forbidden: + candidates = {_normalize_path(base_norm), _normalize_path(base_res)} + + # (a) Refuse a filesystem root (`/`, `C:\`, `\\server\share`): it has no + # parent to sweep under. + for candidate in candidates: + if _is_filesystem_root(candidate): + raise UnsafePath( + f"refusing sweep: base is a sensitive root " + f"(filesystem root): {base!r}" + ) + + # (b) Refuse a base that matches a known-sensitive dir. + if candidates & _get_forbidden_bases(): raise UnsafePath( - f"refusing sweep: base resolves to a sensitive root: " - f"{base!r} (literal={base_norm!r} resolved={base_res!r})" + f"refusing sweep: base resolves to a sensitive root: {base!r}" ) # Resolve the path's PARENT (not the path itself — the leaf may not diff --git a/skills/shadow-frog-dream/dream-reconcile.py b/skills/shadow-frog-dream/dream-reconcile.py index ee53b46..524fe7f 100755 --- a/skills/shadow-frog-dream/dream-reconcile.py +++ b/skills/shadow-frog-dream/dream-reconcile.py @@ -36,6 +36,7 @@ import subprocess import sys from datetime import datetime, timezone +from pathlib import Path # Shared safety gate for `rm -rf `. Lives next to this script so # bash callers (dream-cleanup.sh, dream-gc.sh) and this module share ONE @@ -1108,8 +1109,9 @@ def rebuild_top_index(repo_root, dry_run=False): continue shadow_path = os.path.join(root, fname) rel_shadow = os.path.relpath(shadow_path, shadow_dir) - # The shadow path mirrors the source path with `.md` appended. - source_path = rel_shadow[:-3] + # The shadow path mirrors the source path with `.md` appended; + # shadow artifacts always record POSIX (forward-slash) paths. + source_path = Path(rel_shadow[:-3]).as_posix() language = _shadow_language(shadow_path) names = _shadow_symbol_names(shadow_path) diff --git a/skills/shadow-frog-init/shadow-init.py b/skills/shadow-frog-init/shadow-init.py index 744fa10..208e480 100755 --- a/skills/shadow-frog-init/shadow-init.py +++ b/skills/shadow-frog-init/shadow-init.py @@ -223,7 +223,7 @@ def _walk_files(repo_root): for fname in filenames: full = Path(dirpath) / fname try: - rel = str(full.relative_to(root)) + rel = full.relative_to(root).as_posix() except ValueError: continue result.append(rel) diff --git a/tests/skills/shadow_frog_dream/test_worktree_safety.py b/tests/skills/shadow_frog_dream/test_worktree_safety.py index f621027..fe95a43 100644 --- a/tests/skills/shadow_frog_dream/test_worktree_safety.py +++ b/tests/skills/shadow_frog_dream/test_worktree_safety.py @@ -16,7 +16,7 @@ sys.path.insert(0, str(SKILL_DIR)) from _worktree_safety import ( # noqa: E402 - UnsafePath, _FORBIDDEN_BASES, _strip_macos_private, safe_worktree_path + UnsafePath, _UNIX_FORBIDDEN_BASES, _strip_macos_private, safe_worktree_path ) sys.path.pop(0) @@ -26,6 +26,7 @@ # =========================================================================== class TestHappyPath: + @pytest.mark.skipif(os.name == "nt", reason="POSIX absolute-path semantics") def test_canonical_shape_under_default_base(self): p = safe_worktree_path( "/tmp/shadowfrog-dreams/proj/dream-foo", @@ -97,6 +98,7 @@ def test_rejects_relative_base(self): # Rule 3: no ".." traversal in literal input # =========================================================================== +@pytest.mark.skipif(os.name == "nt", reason="POSIX absolute-path semantics") class TestTraversal: @pytest.mark.parametrize("path", [ "/tmp/shadowfrog-dreams/../etc/passwd", @@ -127,6 +129,7 @@ class TestSensitiveBases: # macOS /private prefixed forms — must ALSO refuse. "/private/tmp", "/private/etc", "/private/var", ]) + @pytest.mark.skipif(os.name == "nt", reason="POSIX absolute-path semantics") def test_rejects_sensitive_base(self, base): # Use a path shape that would pass shape-check, so only Rule 4 can fail. with pytest.raises(UnsafePath, match="sensitive root"): @@ -299,7 +302,51 @@ def test_strips_only_leading_private_segment(self, inp, expected): class TestForbiddenBaseListInvariants: def test_tmp_var_etc_are_forbidden(self): for must_be_listed in ("/tmp", "/var", "/etc", "/home", "/Users"): - assert must_be_listed in _FORBIDDEN_BASES, ( - f"{must_be_listed} must stay in _FORBIDDEN_BASES to keep " + assert must_be_listed in _UNIX_FORBIDDEN_BASES, ( + f"{must_be_listed} must stay in _UNIX_FORBIDDEN_BASES to keep " f"the safety gate trustworthy" ) + + +# =========================================================================== +# Windows-only: drive/UNC roots, %SystemRoot% etc., backslash traversal, and a +# real C:\ worktree. Mirror the POSIX-literal cases (skipped on Windows) against +# Windows path semantics. +# =========================================================================== + +@pytest.mark.skipif(os.name != "nt", reason="Windows path semantics") +class TestWindowsPaths: + def test_accepts_real_windows_worktree(self, tmp_path): + base = tmp_path / "my-dreams" + target = base / "proj" / "dream-t01" + target.mkdir(parents=True) + assert safe_worktree_path(str(target), str(base)).exists() + + def test_rejects_drive_root(self): + with pytest.raises(UnsafePath, match="sensitive root"): + safe_worktree_path("C:\\ns\\dream-foo", "C:\\") + + def test_rejects_unc_share_root(self): + with pytest.raises(UnsafePath, match="sensitive root"): + safe_worktree_path( + "\\\\server\\share\\ns\\dream-foo", "\\\\server\\share", + ) + + @pytest.mark.parametrize("var", ["SystemRoot", "windir", "ProgramFiles"]) + def test_rejects_windows_system_dirs(self, var): + base = os.environ.get(var) + if not base: + pytest.skip(f"%{var}% not set") + with pytest.raises(UnsafePath, match="sensitive root"): + safe_worktree_path(os.path.join(base, "ns", "dream-foo"), base) + + def test_rejects_backslash_traversal(self): + with pytest.raises(UnsafePath, match=r"'\.\.'"): + safe_worktree_path( + "C:\\base\\..\\Windows\\ns\\dream-foo", "C:\\base\\..\\Windows", + ) + + def test_rejects_relative_base(self): + # Absolute worktree path, relative base -> Rule 2's base check. + with pytest.raises(UnsafePath, match="not absolute"): + safe_worktree_path("C:\\dreams\\ns\\dream-foo", "dreams") diff --git a/tests/skills/shadow_frog_init/test_shadow_init.py b/tests/skills/shadow_frog_init/test_shadow_init.py index f8a81ed..8bfd6cc 100644 --- a/tests/skills/shadow_frog_init/test_shadow_init.py +++ b/tests/skills/shadow_frog_init/test_shadow_init.py @@ -938,7 +938,7 @@ def test_walk_files_lists_all_non_excluded(shadow_init, tmp_path): result = shadow_init._walk_files(tmp_path) # Note: walk does NOT filter by extension — that's discover_files's job assert "a.py" in result - assert os.path.join("sub", "b.txt") in result + assert "sub/b.txt" in result # ---------------------------------------------------------------------------