diff --git a/src/dippy/cli/python.py b/src/dippy/cli/python.py index d0caf761..4f13be72 100644 --- a/src/dippy/cli/python.py +++ b/src/dippy/cli/python.py @@ -14,6 +14,7 @@ from typing import NamedTuple from dippy.cli import Classification, HandlerContext +from dippy.core.paths import resolve_arg_path COMMANDS = ["python", "python3"] + [f"python3.{v}" for v in range(8, 20)] @@ -731,11 +732,7 @@ def _find_script_path(tokens: list[str], cwd: Path) -> tuple[Path | None, int]: continue # Found the script path - script_path = Path(token) - if not script_path.is_absolute(): - script_path = cwd / script_path - - return script_path.resolve(), i + return resolve_arg_path(token, cwd), i return None, -1 diff --git a/src/dippy/core/paths.py b/src/dippy/core/paths.py new file mode 100644 index 00000000..347946e5 --- /dev/null +++ b/src/dippy/core/paths.py @@ -0,0 +1,28 @@ +"""Path resolution for command-argument inspection. + +Resolves path-like command tokens to filesystem paths so handlers can +inspect the referenced file (e.g. static-analyze a script before approving). +""" + +from __future__ import annotations + +import os +from pathlib import Path + + +def resolve_arg_path(token: str, cwd: Path) -> Path: + """Resolve a command-line path token to a filesystem path for inspection. + + Expands ``~`` and ``~user``, then anchors relative paths to ``cwd`` — + matching the path normalization described in the security model. Variables + (``$HOME``) are intentionally left literal: the shell expands those after + approval, so we never guess their value from the hook's own environment. + + Never raises: an unresolvable token (e.g. an unknown user) is left literal, + so the resulting path simply won't exist and the caller degrades to asking + for confirmation. + """ + path = Path(os.path.expanduser(token)) + if not path.is_absolute(): + path = cwd / path + return path.resolve() diff --git a/tests/cli/test_python.py b/tests/cli/test_python.py index b8fec89d..5dd142f6 100644 --- a/tests/cli/test_python.py +++ b/tests/cli/test_python.py @@ -260,6 +260,32 @@ def foo( assert needs_confirmation(result), "syntax error should be blocked" +SAFE_SCRIPT = "import json\nprint(json.dumps({'ok': True}))\n" + + +class TestPythonScriptPathExpansion: + """Tests for resolving script paths before analysis (issue #144).""" + + def test_tilde_path_approved(self, check, tmp_path, monkeypatch): + """A safe script referenced via ~ should expand and be approved.""" + monkeypatch.setenv("HOME", str(tmp_path)) + (tmp_path / "probe.py").write_text(SAFE_SCRIPT) + result = check("python3 ~/probe.py") + assert is_approved(result), "tilde path should expand and be approved" + + def test_relative_path_anchored_to_cwd(self, check, tmp_path, monkeypatch): + """A relative script path should resolve against the working directory.""" + (tmp_path / "probe.py").write_text(SAFE_SCRIPT) + monkeypatch.chdir(tmp_path) + result = check("python3 probe.py") + assert is_approved(result), "relative path should anchor to cwd" + + def test_unknown_user_degrades_to_ask(self, check): + """An unknown ~user must not crash; it degrades to confirmation.""" + result = check("python3 ~nosuchuser12345/probe.py") + assert needs_confirmation(result), "unknown ~user should degrade to ask" + + class TestPythonComplexScripts: """Tests for more complex Python scripts.""" diff --git a/tests/core/test_paths.py b/tests/core/test_paths.py new file mode 100644 index 00000000..9ccd7115 --- /dev/null +++ b/tests/core/test_paths.py @@ -0,0 +1,47 @@ +""" +Tests for the argument-path resolver. + +Covers the path normalization contract from the security model: ``~`` and +relative paths are resolved, while variables are left literal (the shell +expands those after approval). +""" + +from __future__ import annotations + +from pathlib import Path + +from dippy.core.paths import resolve_arg_path + + +class TestResolveArgPath: + def test_tilde_expands_to_home(self, tmp_path, monkeypatch): + """~ resolves to the home directory (security model: ~/bar -> /home/user/bar).""" + monkeypatch.setenv("HOME", str(tmp_path)) + result = resolve_arg_path("~/script.py", cwd=Path("/some/other/dir")) + assert result == (tmp_path / "script.py").resolve() + assert "~" not in str(result) + + def test_dollar_var_left_literal(self, tmp_path, monkeypatch): + """$VAR is NOT expanded: the shell expands it after approval, so the + resolver must not read it from the hook's own environment.""" + monkeypatch.setenv("HOME", str(tmp_path)) + result = resolve_arg_path("$HOME/script.py", cwd=tmp_path) + assert "$HOME" in str(result) + assert result != (tmp_path / "script.py").resolve() + + def test_relative_anchored_to_cwd(self, tmp_path): + """Relative paths resolve against cwd.""" + result = resolve_arg_path("sub/script.py", cwd=tmp_path) + assert result == (tmp_path / "sub" / "script.py").resolve() + + def test_absolute_preserved(self, tmp_path): + """Absolute paths are not re-anchored to cwd.""" + target = tmp_path / "script.py" + result = resolve_arg_path(str(target), cwd=Path("/some/other/dir")) + assert result == target.resolve() + + def test_unknown_user_left_literal_without_raising(self, tmp_path): + """An unknown ~user must not raise; it stays literal so the path + simply won't exist and the caller can degrade to ask.""" + result = resolve_arg_path("~nosuchuser12345/script.py", cwd=tmp_path) + assert "~nosuchuser12345" in str(result)