Harden against set -e/pipefail crashes and sed marker edge cases

Fixes from a two-agent bug review, all verified:
- active_uplink/tether_ip never fail (bare assignments were killing the
  TUI when offline or on mid-check unplug; netns-tested)
- all TUI menu actions guarded with || true; risky steps inside
  cmd_install/tui_switch/revive_usbmuxd individually guarded since
  || true disables errexit inside the callee
- pause on tui_connect error paths so warnings survive the redraw
- tui_switch: require non-empty uplink before claiming success
- strip_windowrules refuses to range-delete without the end marker;
  newline guard before append (both fixture-tested)
- probes drain output instead of grep -q (pipefail SIGPIPE)
- removed dead pair_wait; quoted .desktop Exec path; priority rejects
  unknown args; unpair reports failure; help no longer prints set line

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
28allday 2026-07-03 20:10:56 +01:00
parent c48020f02d
commit b9c78d982b

View file

@ -40,21 +40,26 @@ tether_iface() {
return 1 return 1
} }
phone_on_usb() { lsusb 2>/dev/null | grep -qi 'apple.*iphone'; } # Probes drain output fully ([[ -n $(...) ]]) rather than grep -q, which can
phone_visible() { idevice_id -l 2>/dev/null | grep -q .; } # SIGPIPE the producer and misreport under pipefail.
phone_on_usb() { [[ -n $(lsusb 2>/dev/null | grep -i 'apple.*iphone' || true) ]]; }
phone_visible() { [[ -n $(idevice_id -l 2>/dev/null || true) ]]; }
phone_paired() { idevicepair validate >/dev/null 2>&1; } phone_paired() { idevicepair validate >/dev/null 2>&1; }
phone_name() { ideviceinfo -k DeviceName 2>/dev/null || echo "iPhone"; } phone_name() { ideviceinfo -k DeviceName 2>/dev/null || echo "iPhone"; }
# Both must never fail: bare `x=$(...)` assignments under set -e abort the
# whole script (and close the TUI window) if the pipeline fails — which
# happens when the interface vanishes mid-check or no default route exists.
tether_ip() { tether_ip() {
local iface=$1 local iface=$1
ip -4 -br addr show "$iface" 2>/dev/null | awk '{print $3}' ip -4 -br addr show "$iface" 2>/dev/null | awk '{print $3}' || true
} }
# Interface currently carrying the default route. # Interface currently carrying the default route (empty if offline).
active_uplink() { active_uplink() {
ip route get 1.1.1.1 2>/dev/null \ ip route get 1.1.1.1 2>/dev/null \
| awk '{for(i=1;i<NF;i++) if($i=="dev"){print $(i+1); exit}}' | awk '{for(i=1;i<NF;i++) if($i=="dev"){print $(i+1); exit}}' || true
} }
current_metric() { current_metric() {
@ -69,7 +74,7 @@ net_test() {
# usbmuxd drops devices that were plugged in before it started # usbmuxd drops devices that were plugged in before it started
# ("device unconfigured") — a restart rescans the bus. # ("device unconfigured") — a restart rescans the bus.
revive_usbmuxd() { revive_usbmuxd() {
as_root systemctl restart usbmuxd as_root systemctl restart usbmuxd || { warn "Could not restart usbmuxd (sudo declined?)"; return 1; }
sleep 2 sleep 2
} }
@ -111,14 +116,31 @@ reload_networkd() {
fi fi
} }
# Delete our marker-delimited block. Only range-delete when BOTH markers are
# present — with the end marker missing, sed's range would eat everything
# from the begin marker to EOF, including unrelated user config.
strip_windowrules() {
grep -q '>>> omatether windowrules begin' "$WINDOWS_CONF" || return 0
if ! grep -q '<<< omatether windowrules end' "$WINDOWS_CONF"; then
warn "windows.conf has our begin marker but no end marker — leaving it untouched."
return 1
fi
sed -i '/>>> omatether windowrules begin/,/<<< omatether windowrules end/d' "$WINDOWS_CONF"
}
# Small centered floating window for the TUI (user-level Hyprland rule; # Small centered floating window for the TUI (user-level Hyprland rule;
# remove-then-append so re-installs stay idempotent). # remove-then-append so re-installs stay idempotent).
install_windowrules() { install_windowrules() {
[[ -f $WINDOWS_CONF ]] || return 0 # not a Hyprland/Omarchy box — skip [[ -f $WINDOWS_CONF ]] || return 0 # not a Hyprland/Omarchy box — skip
if ! grep -q 'omatether windowrules begin' "$WINDOWS_CONF"; then if ! grep -q 'omatether windowrules begin' "$WINDOWS_CONF"; then
cp "$WINDOWS_CONF" "$WINDOWS_CONF.bak.$(date +%s)" cp "$WINDOWS_CONF" "$WINDOWS_CONF.bak.$(date +%s)" || return 1
fi
strip_windowrules || return 1
# Ensure the file ends with a newline, or our begin marker glues onto the
# last existing line and the next strip would delete that line too.
if [[ -s $WINDOWS_CONF && $(tail -c1 "$WINDOWS_CONF") != "" ]]; then
printf '\n' >> "$WINDOWS_CONF"
fi fi
sed -i '/>>> omatether windowrules begin/,/<<< omatether windowrules end/d' "$WINDOWS_CONF"
cat >> "$WINDOWS_CONF" <<'EOF' cat >> "$WINDOWS_CONF" <<'EOF'
# >>> omatether windowrules begin # >>> omatether windowrules begin
# omatether — small floating window, centered. # omatether — small floating window, centered.
@ -132,19 +154,23 @@ EOF
remove_windowrules() { remove_windowrules() {
[[ -f $WINDOWS_CONF ]] || return 0 [[ -f $WINDOWS_CONF ]] || return 0
sed -i '/>>> omatether windowrules begin/,/<<< omatether windowrules end/d' "$WINDOWS_CONF" strip_windowrules || return 1
hyprctl reload >/dev/null 2>&1 || true hyprctl reload >/dev/null 2>&1 || true
} }
# --------------------------------------------------------- CLI commands ---- # --------------------------------------------------------- CLI commands ----
# NOTE: every risky step is explicitly guarded — the TUI calls this with
# `|| true`, which disables errexit inside the function, so an unguarded
# failure would otherwise plow on into the next step.
cmd_install() { cmd_install() {
say "Installing iPhone USB tethering support…" say "Installing iPhone USB tethering support…"
as_root pacman -S --needed --noconfirm usbmuxd libimobiledevice gum as_root pacman -S --needed --noconfirm usbmuxd libimobiledevice gum \
|| { warn "Package install failed — no internet, or sudo declined."; return 1; }
ok "usbmuxd + libimobiledevice installed (usbmuxd starts on demand via udev)" ok "usbmuxd + libimobiledevice installed (usbmuxd starts on demand via udev)"
write_netfile "$METRIC_FALLBACK" write_netfile "$METRIC_FALLBACK" || { warn "Could not write $NETFILE."; return 1; }
reload_networkd reload_networkd || { warn "networkd reload failed."; return 1; }
ok "networkd profile written: $NETFILE (fallback priority, metric $METRIC_FALLBACK)" ok "networkd profile written: $NETFILE (fallback priority, metric $METRIC_FALLBACK)"
# Floating TUI launcher for Walker. Uses a dedicated app-id (rather than # Floating TUI launcher for Walker. Uses a dedicated app-id (rather than
@ -156,21 +182,21 @@ cmd_install() {
[Desktop Entry] [Desktop Entry]
Name=iPhone Tether Name=iPhone Tether
Comment=Use an iPhone's internet over USB Comment=Use an iPhone's internet over USB
Exec=xdg-terminal-exec --app-id=omatether -e $self Exec=xdg-terminal-exec --app-id=omatether -e "$self"
Icon=phone Icon=phone
Terminal=false Terminal=false
Type=Application Type=Application
Categories=Network; Categories=Network;
EOF EOF
systemctl --user restart elephant.service 2>/dev/null || true systemctl --user restart elephant.service 2>/dev/null || true
install_windowrules install_windowrules || warn "Windowrule install skipped — see above."
ok "Walker launcher installed: 'iPhone Tether' (small floating window)" ok "Walker launcher installed: 'iPhone Tether' (small floating window)"
# If the phone was plugged in before usbmuxd existed it won't be seen yet; # If the phone was plugged in before usbmuxd existed it won't be seen yet;
# restart usbmuxd to rescan. (Do NOT `udevadm trigger` — re-firing add # restart usbmuxd to rescan. (Do NOT `udevadm trigger` — re-firing add
# events on a connected iPhone wedges it: "device unconfigured".) # events on a connected iPhone wedges it: "device unconfigured".)
if phone_on_usb && ! phone_visible; then if phone_on_usb && ! phone_visible; then
revive_usbmuxd revive_usbmuxd || true
fi fi
say "" say ""
@ -193,20 +219,10 @@ do_pair_once() {
return 1 return 1
} }
# Retry pairing until it succeeds or times out (arg: seconds, default 120).
pair_wait() {
local deadline=$(( SECONDS + ${1:-120} ))
while (( SECONDS < deadline )); do
idevicepair pair >/dev/null 2>&1 && return 0
sleep 5
done
return 1
}
cmd_pair() { cmd_pair() {
command -v idevicepair >/dev/null || die "libimobiledevice not installed — run: $0 install" command -v idevicepair >/dev/null || die "libimobiledevice not installed — run: $0 install"
if ! phone_visible; then if ! phone_visible; then
phone_on_usb && revive_usbmuxd if phone_on_usb; then revive_usbmuxd || true; fi
phone_visible || die "No iPhone detected over USB. Plug it in and try again." phone_visible || die "No iPhone detected over USB. Plug it in and try again."
fi fi
@ -220,7 +236,11 @@ cmd_pair() {
} }
cmd_unpair() { cmd_unpair() {
idevicepair unpair && ok "Unpaired." if idevicepair unpair; then
ok "Unpaired."
else
die "Unpair failed — is a phone connected and paired?"
fi
} }
cmd_status() { cmd_status() {
@ -246,7 +266,7 @@ cmd_status() {
addr=$(tether_ip "$iface") addr=$(tether_ip "$iface")
if [[ -n $addr ]]; then if [[ -n $addr ]]; then
ok "IPv4: $addr" ok "IPv4: $addr"
ip route show default dev "$iface" | sed 's/^/ /' ip route show default dev "$iface" 2>/dev/null | sed 's/^/ /' || true
if net_test "$iface"; then if net_test "$iface"; then
ok "Internet via $iface works" ok "Internet via $iface works"
else else
@ -264,26 +284,29 @@ cmd_priority() {
[[ -f $NETFILE ]] || die "Not installed — run: $0 install" [[ -f $NETFILE ]] || die "Not installed — run: $0 install"
case ${1:-} in case ${1:-} in
high) high)
write_netfile "$METRIC_PREFERRED" write_netfile "$METRIC_PREFERRED" || die "Could not write $NETFILE."
reload_networkd reload_networkd || die "networkd reload failed."
ok "Tether now PREFERRED (metric $METRIC_PREFERRED) — traffic routes via the phone when plugged in" ok "Tether now PREFERRED (metric $METRIC_PREFERRED) — traffic routes via the phone when plugged in"
;; ;;
low) low)
write_netfile "$METRIC_FALLBACK" write_netfile "$METRIC_FALLBACK" || die "Could not write $NETFILE."
reload_networkd reload_networkd || die "networkd reload failed."
ok "Tether now FALLBACK (metric $METRIC_FALLBACK) — ethernet/wifi win when available" ok "Tether now FALLBACK (metric $METRIC_FALLBACK) — ethernet/wifi win when available"
;; ;;
*) "")
say "Current metric: $(current_metric) (ethernet=100, wifi=600)" say "Current metric: $(current_metric) (ethernet=100, wifi=600)"
say "Usage: $0 priority <high|low>" say "Usage: $0 priority <high|low>"
;; ;;
*)
die "Unknown priority '$1' — use high or low."
;;
esac esac
} }
cmd_uninstall() { cmd_uninstall() {
as_root rm -f "$NETFILE" as_root rm -f "$NETFILE"
rm -f "$DESKTOP_FILE" rm -f "$DESKTOP_FILE"
remove_windowrules remove_windowrules || warn "Windowrule removal skipped — see above."
as_root networkctl reload as_root networkctl reload
systemctl --user restart elephant.service 2>/dev/null || true systemctl --user restart elephant.service 2>/dev/null || true
ok "Removed $NETFILE, the Walker launcher and windowrules (usbmuxd/libimobiledevice left installed)" ok "Removed $NETFILE, the Walker launcher and windowrules (usbmuxd/libimobiledevice left installed)"
@ -330,16 +353,19 @@ tui_header() {
echo echo
} }
# Wait for Enter so warnings aren't wiped by the next dashboard redraw.
pause() { gum input --placeholder "Press Enter to continue…" >/dev/null || true; }
tui_connect() { tui_connect() {
say "" say ""
if ! phone_visible; then if ! phone_visible; then
phone_on_usb && revive_usbmuxd if phone_on_usb; then revive_usbmuxd || true; fi
fi fi
if ! phone_visible; then if ! phone_visible; then
# shellcheck disable=SC2016 # expansion happens in the child shell # shellcheck disable=SC2016 # expansion happens in the child shell
gum spin --title "Plug the iPhone in via USB…" -- bash -c \ gum spin --title "Plug the iPhone in via USB…" -- bash -c \
'for i in $(seq 1 24); do idevice_id -l 2>/dev/null | grep -q . && exit 0; sleep 5; done; exit 1' \ 'for i in $(seq 1 24); do idevice_id -l 2>/dev/null | grep -q . && exit 0; sleep 5; done; exit 1' \
|| { warn "No iPhone appeared. Check the cable and try again."; return 1; } || { warn "No iPhone appeared. Check the cable and try again."; pause; return 1; }
fi fi
if ! phone_paired; then if ! phone_paired; then
@ -348,7 +374,7 @@ tui_connect() {
# shellcheck disable=SC2016 # expansion happens in the child shell # shellcheck disable=SC2016 # expansion happens in the child shell
gum spin --title "Waiting for Trust… (unlock the phone)" -- bash -c \ gum spin --title "Waiting for Trust… (unlock the phone)" -- bash -c \
'for i in $(seq 1 24); do idevicepair pair >/dev/null 2>&1 && exit 0; sleep 5; done; exit 1' \ 'for i in $(seq 1 24); do idevicepair pair >/dev/null 2>&1 && exit 0; sleep 5; done; exit 1' \
|| { warn "Pairing timed out. Unlock the phone and pick Connect again."; return 1; } || { warn "Pairing timed out. Unlock the phone and pick Connect again."; pause; return 1; }
ok "Paired with $(phone_name)" ok "Paired with $(phone_name)"
fi fi
@ -361,31 +387,28 @@ tui_connect() {
warn "Paired, but no connection yet. On the phone enable:" warn "Paired, but no connection yet. On the phone enable:"
say " Settings → Personal Hotspot → Allow Others to Join: ON" say " Settings → Personal Hotspot → Allow Others to Join: ON"
fi fi
gum input --placeholder "Press Enter to continue…" >/dev/null || true pause
} }
tui_switch() { tui_switch() {
local target=$1 iface local target=$1 iface metric=$METRIC_FALLBACK
say "" say ""
if [[ $target == phone ]]; then [[ $target == phone ]] && metric=$METRIC_PREFERRED
write_netfile "$METRIC_PREFERRED" write_netfile "$metric" || { warn "Could not update $NETFILE (sudo declined?)"; pause; return 1; }
else reload_networkd || { warn "networkd reload failed."; pause; return 1; }
write_netfile "$METRIC_FALLBACK"
fi
reload_networkd
gum spin --title "Applying routes…" -- sleep 3 || true gum spin --title "Applying routes…" -- sleep 3 || true
local uplink local uplink
uplink=$(active_uplink) uplink=$(active_uplink)
iface=$(tether_iface || true) iface=$(tether_iface || true)
if [[ $target == phone && $uplink == "$iface" ]]; then if [[ -n $uplink && $target == phone && $uplink == "$iface" ]]; then
ok "All traffic now routes via the iPhone ($iface)" ok "All traffic now routes via the iPhone ($iface)"
elif [[ $target == normal && $uplink != "$iface" ]]; then elif [[ -n $uplink && $target == normal && $uplink != "$iface" ]]; then
ok "Back to normal — traffic routes via ${uplink:-your usual connection}" ok "Back to normal — traffic routes via $uplink"
else else
warn "Route didn't settle as expected (uplink: ${uplink:-none}). Check Status." warn "Route didn't settle as expected (uplink: ${uplink:-none}). Check Status."
fi fi
gum input --placeholder "Press Enter to continue…" >/dev/null || true pause
} }
cmd_tui() { cmd_tui() {
@ -466,12 +489,14 @@ cmd_tui() {
local choice local choice
choice=$(gum choose --header "$(printf '%*s%s' "$((mpad + 2))" '' 'What do you want to do?')" "${pmenu[@]}") || break choice=$(gum choose --header "$(printf '%*s%s' "$((mpad + 2))" '' 'What do you want to do?')" "${pmenu[@]}") || break
choice=${choice#"${choice%%[![:space:]]*}"} # trim the centering pad choice=${choice#"${choice%%[![:space:]]*}"} # trim the centering pad
# Every action is guarded with || true: a non-zero return from a bare
# call here would trip set -e and close the TUI window without a trace.
case $choice in case $choice in
"Install tethering support") cmd_install; gum input --placeholder "Press Enter…" >/dev/null || true ;; "Install tethering support") cmd_install || true; pause ;;
"Connect phone") tui_connect ;; "Connect phone") tui_connect || true ;;
"Switch internet → iPhone") tui_switch phone ;; "Switch internet → iPhone") tui_switch phone || true ;;
"Switch back → ethernet/wifi") tui_switch normal ;; "Switch back → ethernet/wifi") tui_switch normal || true ;;
"Status (full check)") say ""; cmd_status || true; gum input --placeholder "Press Enter…" >/dev/null || true ;; "Status (full check)") say ""; cmd_status || true; pause ;;
"Refresh") ;; "Refresh") ;;
"Quit"|"") break ;; "Quit"|"") break ;;
esac esac
@ -489,7 +514,7 @@ case ${1:-tui} in
priority) shift; cmd_priority "${1:-}" ;; priority) shift; cmd_priority "${1:-}" ;;
uninstall) cmd_uninstall ;; uninstall) cmd_uninstall ;;
help|-h|--help) help|-h|--help)
sed -n '2,11p' "$0" | sed 's/^# \{0,1\}//' sed -n '2,10p' "$0" | sed 's/^# \{0,1\}//'
;; ;;
*) die "Unknown command: $1 (try: $0 help)" ;; *) die "Unknown command: $1 (try: $0 help)" ;;
esac esac