Skip to content

Commit a89bc12

Browse files
authored
Merge pull request #162 from h1d3mun3/fix/macos-sweep-path-pin
fix(macos): pin PATH on the share sweep's guest round trip
2 parents 69de8cd + 037348e commit a89bc12

2 files changed

Lines changed: 64 additions & 3 deletions

File tree

augur

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3835,7 +3835,31 @@ _refresh_macos_shares_locked() {
38353835
(( n > 2000 )) && [[ "$quiet" != quiet ]] && info "Refreshing ${n} changed shared file(s) in the guest — this takes a moment."
38363836

38373837
# One round trip: the program is an argument, so stdin stays free for the NUL-separated list.
3838-
out="$(ssh_macos "$vm" "python3 -c '$(_macos_msync_program)' ${_MACOS_SWEEP_TRIES} ${_MACOS_SWEEP_DETAIL}" < "$list" 2>&1)" || {
3838+
#
3839+
# PATH-PINNED, for the reason verify_macos_egress_locked already pins its own probes (augur:1748-
3840+
# 1755): augur's `~/.augur-env` puts `$HOME/.local/bin` FIRST in the PATH of every guest shell,
3841+
# including the non-login shell `ssh <host> <cmd>` runs, which still sources `~/.zshenv`. A guest
3842+
# that plants `~/.local/bin/python3` therefore chooses what this call returns.
3843+
#
3844+
# WHAT THAT WOULD BUY, and why this site matters more than the tripwire's three: this is the
3845+
# MECHANISM, not the check. A shadowed interpreter here can print `ok=N` while invalidating
3846+
# nothing, so the operator is told their edit landed when it did not — and the edit that did not
3847+
# land may be the one tightening the rules on a guest that is misbehaving. It runs at four attach
3848+
# points AND every `AUGUR_MACOS_REFRESH_INTERVAL` seconds for the life of the VM, unattended,
3849+
# where verify_macos_share_freshness runs twice per `up`. Shadowing only the CHECK would make the
3850+
# self-test lie while real sweeps still worked; shadowing THIS stops the sweeps.
3851+
#
3852+
# `/usr/bin/python3` is on the sealed, read-only system volume with SIP on, so a project clone
3853+
# cannot replace it and this cannot cause a false "not found" — measured on a 26.6 guest: `/` is
3854+
# `apfs, sealed, local, read-only`, `csrutil status: enabled`, and `ls -lO /usr/bin/python3` shows
3855+
# `restricted`. (An earlier draft of this comment said the reason was the clone's lack of sudo.
3856+
# That is false — augur:3202-3204 uses password sudo against project VMs — and the seal is why.)
3857+
#
3858+
# NOT A COMPLETE ANSWER, deliberately stated so nobody reads it as one: PATH pinning does not stop
3859+
# a shell FUNCTION or an alias. `python3() { … }` in `~/.zshenv` — the file augur itself appends
3860+
# to — defeats this, measured. It removes the cheapest PATH-resolved vector and matches the
3861+
# sibling; it does not make the guest's answer trustworthy. The sibling's pin has the same hole.
3862+
out="$(ssh_macos "$vm" "export PATH=/usr/bin:/bin:/usr/sbin:/sbin; python3 -c '$(_macos_msync_program)' ${_MACOS_SWEEP_TRIES} ${_MACOS_SWEEP_DETAIL}" < "$list" 2>&1)" || {
38393863
warn "Could not refresh the guest's view of the shared files: ${out%%$'\n'*}"
38403864
warn " Host-side edits may not be visible in the guest (issues #124/#135). \`down --macos && up --macos\` clears it."
38413865
rm -f "$pending" "$list" 2>/dev/null || true

tests/41_macos_share_refresh.sh

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,26 @@ SSHLOG="$TMPD/sshlog" # the remote command string, one per call
5656
STDINLOG="$TMPD/stdinlog" # the NUL-separated path list it was fed
5757
: > "$SSHLOG"; : > "$STDINLOG"
5858
SSH_OUT="ok=3"; SSH_RC=0
59-
ssh_macos() { # only ever driven with the piped `python3 -c` shape here
59+
ssh_macos() {
6060
printf '%s\n' "$*" >> "$SSHLOG"
61-
cat > "$STDINLOG"
61+
# ONLY THE SWEEP IS PIPED FROM THE HOST. Its NUL-separated list arrives on this function's stdin;
62+
# nothing else in this file does that. The header used to say "only ever driven with the piped
63+
# `python3 -c` shape here" and an unconditional `cat` was correct while that held — it stopped
64+
# holding when the mode-reconcile arms began driving `cmd_claude_macos` and `cmd_shell_macos`,
65+
# which reach this stub as `-t` interactive shapes with nothing piped. There the `cat` drains
66+
# whatever stdin the SUITE inherited: /dev/null under CI (invisible), a TTY on a developer's
67+
# terminal (`make offline-tests` stops dead with no error and no timeout). That is precisely the
68+
# failure tests/39 exists for, and it was live on main — see its header for the two files that
69+
# shipped in this state before.
70+
#
71+
# `printf %s` is excluded ahead of the `python3 -c` arm rather than after it because the freshness
72+
# tripwire's probes carry BOTH: their pipe is inside the remote command string, so from here they
73+
# are unpiped like the interactive shapes. Matching `python3 -c` alone would regrow the bug the
74+
# first time this file drives them.
75+
case "$*" in
76+
*"printf %s"*) : ;; # tripwire probes — the pipe is REMOTE, not from us
77+
*"python3 -c"*) cat > "$STDINLOG" ;; # the sweep — the list comes from the host
78+
esac
6279
printf '%s' "$SSH_OUT"
6380
return "$SSH_RC"
6481
}
@@ -300,6 +317,26 @@ if tail -1 "$SSHLOG" | grep -q " ${_MACOS_SWEEP_TRIES} ${_MACOS_SWEEP_DETAIL}$";
300317
ok "the retry count and the detail cap are passed, in that order"
301318
else fail "the retry count and the detail cap are passed, in that order" "$(tail -1 "$SSHLOG")"; fi
302319

320+
section "the sweep's interpreter cannot be shadowed through PATH"
321+
322+
# THE MECHANISM, NOT THE CHECK. augur's own ~/.augur-env puts $HOME/.local/bin FIRST in the PATH of
323+
# every guest shell — including the non-login shell `ssh <host> <cmd>` runs, which still sources
324+
# ~/.zshenv — so an unpinned `python3` here is chosen by the guest. Shadowing the freshness tripwire
325+
# would make the self-test lie while real sweeps still worked; shadowing THIS makes the sweep print
326+
# `ok=N` while invalidating nothing, and the edit that then never lands may be the operator tightening
327+
# the rules on a guest that is misbehaving. verify_macos_egress_locked pins its own probes for the
328+
# same reason (augur:1748-1755); this arm is what keeps the two from drifting apart again.
329+
#
330+
# Asserted on the FIRST line, before `python3`, because order is the property: a pin appended after
331+
# the interpreter has already resolved is not a pin.
332+
if head -1 "$SSHLOG" | grep -q "export PATH=/usr/bin:/bin:/usr/sbin:/sbin; *python3 -c "; then
333+
ok "the sweep pins PATH to system dirs ahead of \`python3\`"
334+
else fail "the sweep pins PATH to system dirs ahead of \`python3\`" "a guest-planted ~/.local/bin/python3 would decide what this round trip reports: $(head -1 "$SSHLOG")"; fi
335+
# The pin must not have cost the protocol: the program is still an ARGUMENT (stdin stays free for the
336+
# NUL list), and _macos_msync_program still contains no single quote of its own.
337+
if [[ "$(tr -dc '\0' < "$STDINLOG" | wc -c | tr -d ' ')" -gt 0 ]]; then ok "…without disturbing the NUL-separated list on stdin"
338+
else fail "…without disturbing the NUL-separated list on stdin" "the pin was added in a way that consumed stdin or broke the quoting"; fi
339+
303340
section "--share-refresh off stops the sweep — at every call site at once"
304341

305342
# `off` is enforced INSIDE refresh_macos_shares rather than by an `if` copied to the four attach call

0 commit comments

Comments
 (0)