From c90650c1aa7e9057998d3257f4efc394f5873dad Mon Sep 17 00:00:00 2001 From: Lily Dayton Date: Mon, 8 Jun 2026 11:36:35 +0300 Subject: [PATCH 1/3] Expand ~ and $VAR in script paths before existence check Fixes #144. The Python handler resolved a script path by joining the raw token onto cwd, so a ~-prefixed path became /~/... with a literal tilde. The file was never found and a safe script fell back to ask. Add core.paths.resolve_arg_path, a no-raise resolver that expands ~, ~user, and $VAR/${VAR} before anchoring relatives to cwd, and route the Python handler through it. Unresolvable tokens (unknown user, undefined var) are left literal and degrade to ask rather than raising. --- src/dippy/cli/python.py | 7 ++----- src/dippy/core/paths.py | 25 +++++++++++++++++++++++++ tests/cli/test_python.py | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 5 deletions(-) create mode 100644 src/dippy/core/paths.py 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..3130df63 --- /dev/null +++ b/src/dippy/core/paths.py @@ -0,0 +1,25 @@ +"""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 ``~``, ``~user``, and ``$VAR``/``${VAR}``, then anchors relative + paths to ``cwd``. Never raises: an unresolvable token (unknown user, + undefined variable) is left literal, so the resulting path simply won't + exist and the caller degrades to asking for confirmation. + """ + expanded = os.path.expandvars(os.path.expanduser(token)) + path = Path(expanded) + 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..c71d741c 100644 --- a/tests/cli/test_python.py +++ b/tests/cli/test_python.py @@ -260,6 +260,39 @@ 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_home_var_path_approved(self, check, tmp_path, monkeypatch): + """A safe script referenced via $HOME should expand and be approved.""" + monkeypatch.setenv("HOME", str(tmp_path)) + (tmp_path / "probe.py").write_text(SAFE_SCRIPT) + result = check("python3 $HOME/probe.py") + assert is_approved(result), "$HOME 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.""" From 1b780848d87bf6bc3e14f780eadae2448889599c Mon Sep 17 00:00:00 2001 From: Lily Dayton Date: Mon, 8 Jun 2026 11:49:50 +0300 Subject: [PATCH 2/3] Drop $VAR expansion to match documented security model The security model (Reference/Security-Model.md) expands ~ during path normalization but keeps variables literal: $HOME stays $HOME and is expanded by the shell after approval, so the hook never guesses a variable's value from its own environment. Remove os.path.expandvars from resolve_arg_path so it only expands ~ and ~user, matching that invariant and still fully fixing #144 (which only concerns ~). Pin the behavior with a test asserting $HOME is not expanded. --- src/dippy/core/paths.py | 15 +++++++++------ tests/cli/test_python.py | 7 ++++--- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/dippy/core/paths.py b/src/dippy/core/paths.py index 3130df63..347946e5 100644 --- a/src/dippy/core/paths.py +++ b/src/dippy/core/paths.py @@ -13,13 +13,16 @@ def resolve_arg_path(token: str, cwd: Path) -> Path: """Resolve a command-line path token to a filesystem path for inspection. - Expands ``~``, ``~user``, and ``$VAR``/``${VAR}``, then anchors relative - paths to ``cwd``. Never raises: an unresolvable token (unknown user, - undefined variable) is left literal, so the resulting path simply won't - exist and the caller degrades to asking for confirmation. + 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. """ - expanded = os.path.expandvars(os.path.expanduser(token)) - path = Path(expanded) + 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 c71d741c..b415cb72 100644 --- a/tests/cli/test_python.py +++ b/tests/cli/test_python.py @@ -273,12 +273,13 @@ def test_tilde_path_approved(self, check, tmp_path, monkeypatch): result = check("python3 ~/probe.py") assert is_approved(result), "tilde path should expand and be approved" - def test_home_var_path_approved(self, check, tmp_path, monkeypatch): - """A safe script referenced via $HOME should expand and be approved.""" + def test_env_var_path_not_expanded(self, check, tmp_path, monkeypatch): + """$VAR paths stay literal (security model): the shell expands them + after approval, so Dippy can't see the file and degrades to ask.""" monkeypatch.setenv("HOME", str(tmp_path)) (tmp_path / "probe.py").write_text(SAFE_SCRIPT) result = check("python3 $HOME/probe.py") - assert is_approved(result), "$HOME path should expand and be approved" + assert needs_confirmation(result), "$HOME must not be expanded" def test_relative_path_anchored_to_cwd(self, check, tmp_path, monkeypatch): """A relative script path should resolve against the working directory.""" From 13862df3da44112377f19188a5fb76b5521d94e9 Mon Sep 17 00:00:00 2001 From: Lily Dayton Date: Mon, 8 Jun 2026 11:53:44 +0300 Subject: [PATCH 3/3] Test resolver contract directly, not the false-ask UX The previous $HOME integration test asserted that a safe script via $HOME degrades to ask. That pinned a UX wart (an unfortunate prompt) as if it were the desired outcome. Replace it with unit tests on resolve_arg_path that assert the actual contract the security model requires: ~ expands to home, relative paths anchor to cwd, $VAR is left literal (not read from the hook env), and an unknown ~user stays literal without raising. The Python integration tests now assert only desirable outcomes (tilde and relative resolve+approve). --- tests/cli/test_python.py | 8 ------- tests/core/test_paths.py | 47 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 tests/core/test_paths.py diff --git a/tests/cli/test_python.py b/tests/cli/test_python.py index b415cb72..5dd142f6 100644 --- a/tests/cli/test_python.py +++ b/tests/cli/test_python.py @@ -273,14 +273,6 @@ def test_tilde_path_approved(self, check, tmp_path, monkeypatch): result = check("python3 ~/probe.py") assert is_approved(result), "tilde path should expand and be approved" - def test_env_var_path_not_expanded(self, check, tmp_path, monkeypatch): - """$VAR paths stay literal (security model): the shell expands them - after approval, so Dippy can't see the file and degrades to ask.""" - monkeypatch.setenv("HOME", str(tmp_path)) - (tmp_path / "probe.py").write_text(SAFE_SCRIPT) - result = check("python3 $HOME/probe.py") - assert needs_confirmation(result), "$HOME must not be expanded" - 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) 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)