fix(local): exec lookup, worktree parsing and dirs bounds (0.2.1) #2

Merged

dozken merged 2 commits from fix/terminal-exec-and-dirs-hardening into main

2026-09-23 20:15:38 +02:00

dozken commented

2026-09-23 19:53:00 +02:00

Owner

Summary

Security review of protocol 2 (RKD #1065 review, #968). This releases as 0.2.1.

Program lookup matches the exec. A bare name such as git now comes only from absolute $PATH entries, and the resolved absolute path is what gets spawned. Before, the check skipped relative entries but portable-pty's own search used them, so with . or an empty entry in PATH a preset git ran the folder's own ./git. A name containing / is a path from the tab's folder. The program's argv[0] is now the resolved path, because portable-pty cannot set it separately.
Worktrees are parsed with --porcelain -z. A worktree path containing newlines could forge extra entries, for example listing /etc.
git runs with the repo's config switched off: -c core.fsmonitor=false -c core.hooksPath=/dev/null, GIT_CONFIG_NOSYSTEM=1, no prompt, no optional locks. The folders are the client's choice, so the repo may belong to anyone.
dirs is bounded:
- at most 50 worktrees per repo and 200 entries in total
- the answer is truncated to fit a frame
- one dirs runs at a time per connection
- run_bounded kills git's whole process group at the deadline and no longer waits for EOF, so a background grandchild holding the pipe can't block the answer
- output is capped at 1 MiB

Tests

fmt, clippy -D warnings, cargo test --workspace (terminal suite passed 3 more times in a row). New tests for each fix, including a real-server exec test with a hostile PATH, a real newline worktree, the caps, single flight, and grandchild and timeout handling. 14 hand-applied mutations, all killed.

https://claude.ai/code/session_01P4hUduqyzHTYckP7dX5fXV

## Summary Security review of protocol 2 (RKD #1065 review, #968). This releases as 0.2.1. - **Program lookup matches the exec.** A bare name such as `git` now comes only from absolute `$PATH` entries, and the resolved absolute path is what gets spawned. Before, the check skipped relative entries but portable-pty's own search used them, so with `.` or an empty entry in `PATH` a preset `git` ran the folder's own `./git`. A name containing `/` is a path from the tab's folder. The program's argv[0] is now the resolved path, because portable-pty cannot set it separately. - **Worktrees are parsed with `--porcelain -z`.** A worktree path containing newlines could forge extra entries, for example listing `/etc`. - **git runs with the repo's config switched off:** `-c core.fsmonitor=false -c core.hooksPath=/dev/null`, `GIT_CONFIG_NOSYSTEM=1`, no prompt, no optional locks. The folders are the client's choice, so the repo may belong to anyone. - **`dirs` is bounded:** - at most 50 worktrees per repo and 200 entries in total - the answer is truncated to fit a frame - one `dirs` runs at a time per connection - `run_bounded` kills git's whole process group at the deadline and no longer waits for EOF, so a background grandchild holding the pipe can't block the answer - output is capped at 1 MiB ## Tests fmt, clippy `-D warnings`, `cargo test --workspace` (terminal suite passed 3 more times in a row). New tests for each fix, including a real-server exec test with a hostile `PATH`, a real newline worktree, the caps, single flight, and grandchild and timeout handling. 14 hand-applied mutations, all killed. https://claude.ai/code/session_01P4hUduqyzHTYckP7dX5fXV

dozken added 1 commit

2026-09-23 19:53:00 +02:00

fix(terminal): exec what open checked, harden dirs

CI / rust (pull_request) Failing after 39s

Details

858aa490b1

A preset's program is now resolved once, to an absolute path, and that
path is what the PTY runs. Before, open checked only absolute $PATH
entries but portable-pty searched again with relative ones joined to the
cwd, so `PATH=.:/usr/bin` or a leading `:` made a preset `git` run the
folder's own ./git, and `bin/dev` was checked in the folder but searched
in PATH. Bare names come from absolute $PATH entries only; names with a
`/` are absolute or relative to the folder, never searched. portable-pty
sets arg0 to the program it is given, so argv[0] is the resolved path.

dirs:
- `git worktree list --porcelain -z`, parsed on NULs: a worktree path
  with newlines can no longer forge a record (e.g. /etc).
- git runs with core.fsmonitor=false, core.hooksPath=/dev/null and
  GIT_CONFIG_NOSYSTEM=1: the folders are the client's, the repo config
  may be anyone's.
- At most 50 worktrees per repository and 200 folders in all; the
  encoded dir_list is cut to fit MAX_FRAME.
- One dirs at a time per connection; a queued one whose connection has
  gone is dropped without running.
- run_bounded reads stdout non-blocking instead of joining a reader
  thread on EOF (a background grandchild holding the pipe hung the answer
  forever), runs git in its own process group and kills the group at the
  deadline, and caps output at 1 MiB.

Both crates 0.2.1.

Claude-Session: https://claude.ai/code/session_01P4hUduqyzHTYckP7dX5fXV

dozken added 1 commit

2026-09-23 20:12:44 +02:00

test(local): a killed orphan left as a zombie counts as gone

CI / rust (pull_request) Successful in 39s

Details

755cbc6200

CI's container has a PID 1 that does not reap orphans, so the killed
background sleep stays a zombie and kill(pid, 0) still finds it. Read
its state from /proc on Linux; the kill itself was working.

Claude-Session: https://claude.ai/code/session_01P4hUduqyzHTYckP7dX5fXV

dozken merged commit 990c4a60e7 into main

2026-09-23 20:15:38 +02:00

dozken deleted branch fix/terminal-exec-and-dirs-hardening

2026-09-23 20:15:38 +02:00

dozken referenced this pull request from a commit

2026-09-23 20:15:39 +02:00

fix(local): exec lookup, worktree parsing and dirs bounds (0.2.1) (#2)