Expand ~ in script paths before the existence check - #148
Merged
Conversation
Fixes #144. The Python handler resolved a script path by joining the raw token onto cwd, so a ~-prefixed path became <CWD>/~/... 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.
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.
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).
ldayton
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #144.
Problem
The Python handler resolved a script path by joining the raw token onto the working directory. A
~-prefixed path likepython3 ~/scripts/probe.pybecame<CWD>/~/scripts/probe.pywith a literal tilde —pathlibdoesn't treat~as absolute, so it got anchored to cwd. The file was never found, and a safe, statically-analyzable script fell back toask. This defeats Dippy's own preference forpython3 file.pyoverpython3 -c "...": the file form is the one meant to get the fast path.It also contradicted the documented security model, which already states path normalization expands
~/bar→/home/user/barbefore matching — the Python handler just wasn't doing it.Fix
New
core.paths.resolve_arg_path(token, cwd)— a small, no-raise resolver that expands~/~user, then anchors relative paths to cwd. The Python handler routes through it instead of open-codingcwd / Path(token).Two deliberate design choices, both matching the security model:
os.path.expanduser, notPath.expanduser().Path.expanduser()raisesRuntimeErroron an unknown~user(verified on 3.7–3.14), which would turn a gracefulaskinto a crashed hook. Theos.pathfunction leaves unresolvable tokens literal, so they degrade toask.$HOME) are left literal. The model keeps$HOMEas$HOMEand lets the shell expand it after approval, so the hook never guesses a variable's value from its own environment. The resolver only normalizes~and relative paths.Placing this in
core/makes it reusable for future handlers that grow script analysis (script.py,uv.py, ...) rather than re-introducing the same gap.Scope
Only the Python handler does script-file existence checks today;
shell.pydelegates to inner-command checking and never stats a path, sobash ~/x.shdoesn't hit this bug currently.Tests
tests/core/test_paths.py): the resolver contract —~expands to home, relative anchors to cwd, absolute is preserved,$VARis left literal (not read from the hook env), unknown~userstays literal without raising.tests/cli/test_python.py): tilde and relative script paths resolve and approve; unknown~userdegrades toaskwithout crashing.Full suite: 10,906 passing.