Modernize lager uninstall to match what the modern install creates

[danielrmerskine]

Summary

lager uninstall had not kept pace with several releases of install changes. An audit against what today's lager install / lager box config apply actually create found six problems, all fixed here:

1. The --all udev glob never matched the shipped rules. It removed lager-*.rules / *lager*.rules, but the instrument rules file is 99-instrument.rules — so the primary artifact survived a "complete removal".
2. Half the modern artifacts were never removed at all: the usbtmc modprobe blacklist, the lager-box-config sudoers file, the firewall helper script (/usr/local/lib/lager/secure_box_firewall.sh), the lager sysctl config, and the lager group.
3. Privileged removals silently no-op'd on boxes without broad passwordless sudo. Every sudo step ran over BatchMode SSH with || true, so each one failed silently and printed "done" — reproduced live: a plain uninstall reported "Removing /etc/lager directory... done" while the directory survived.
4. "deploy keys" removal aimed at the wrong side. It deleted box-side private keys that modern installs never create, while the actual access artifact — this machine's pubkey in the box's authorized_keys — survived.
5. --all ignored --keep-config and deleted /etc/lager regardless.
6. --dry-run inspected the stale artifact list and used sudo du under BatchMode, reporting /etc/lager as "(not found)" on boxes where it was present.

Design

• Single source of truth: the privileged removal steps live in a module-level spec (UNINSTALL_ALL_PRIV_STEPS) shared by the confirmation listing, --dry-run, the removal session, and the unit tests — pinned by test/unit/test_uninstall_spec.py so the list cannot silently drift from what install creates again.
• One interactive privileged session (the pattern lager update already uses): all sudo steps run in a single ssh -t session with per-step OK/FAIL results written to a results file and read back — at most one sudo password prompt, honest per-step reporting, failures summarized at the end. Sudoers grants are removed last so earlier steps can ride them or the cached sudo timestamp.
• authorized_keys revocation: matched exactly by the local ~/.ssh/lager_box.pub key blob (comments vary across old installs), falling back to the default key comment; runs as the last remote operation since it cuts BatchMode access. The output warns that the next SSH will need a password.
• Deliberately left in place (and stated in the output): docker itself (packages, buildx plugin, the daemon.json dns entry) and pip/apt packages installed for lager workflows.

Validation (hardware, 2026-07-13)

• JUL-28 (login user without passwordless sudo, full --all wipe): exactly one sudo password prompt in the privileged session; every step reported individually; post-verify confirmed all artifacts gone (/etc/lager, udev rules, blacklist, sudoers files, firewall script, lager group, third_party). The subsequent reinstall's own SSH check reported "Passwordless SSH not configured" — independent proof the authorized_keys revocation worked — and the box restored fully on v0.31.7.
• PRD-1 (broad NOPASSWD, --all --keep-config): zero password prompts; /etc/lager step correctly skipped and saved_nets.json survived untouched while all --all artifacts were removed; reinstall restored the box end-to-end (lager hello OK, instruments enumerate).
• The dry-run on the half-uninstalled JUL-28 caught a bug in its own first version (multi-path ls hiding files when any sibling path was missing) — fixed and covered by the ; true handling.
• Unit suite: 1546 passed; the 5 failures are pre-existing on v0.31.7 main (verified byte-identical test files).

Notes for reviewers

• docker image prune -af behavior is unchanged (it has always removed ALL unused images); the confirmation listing now says so explicitly.
• Legacy removals (deploy keys, box-side ssh config sed, pigpio container) are retained for old boxes.

[gemini-code-assist]

Code Review

This pull request significantly improves the lager uninstall command to ensure it completely and reliably removes all artifacts created by modern installs. Key changes include consolidating privileged removal steps into a single interactive SSH session (reducing sudo password prompts and preventing silent failures on boxes without passwordless sudo), honoring --keep-config alongside --all, and removing the local machine's key from the remote authorized_keys. Unit tests were also added to pin this uninstallation specification. Feedback on the changes suggests using a user-specific path instead of a hardcoded /tmp file for temporary results to avoid security risks, using a context manager with explicit encoding when opening the local public key file, and joining remote commands with semicolons instead of newlines to prevent terminal echo issues during interactive SSH sessions.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Using a hardcoded path in /tmp can lead to conflicts or security issues (such as symlink attacks or permission errors) in multi-user environments if another user has already created the file with restrictive permissions. Since this path is only executed on the remote box via shell commands, using a user-specific path like ~/.lager-uninstall-results.txt is much safer and avoids shared directory conflicts.

Suggested change

	_PRIV_RESULTS_PATH = "/tmp/lager-uninstall-results.txt"
	_PRIV_RESULTS_PATH = "~/.lager-uninstall-results.txt"

[gemini-code-assist]

Opening a file without a with statement leaves the file descriptor open until garbage collection runs, which can lead to resource leaks. Additionally, it is safer to specify an explicit encoding (like utf-8) to prevent potential decoding errors on systems with different default locales.

Suggested change

	fields = open(pub_path).read().strip().split()
	if len(fields) >= 2:
	return fields[1]
	with open(pub_path, "r", encoding="utf-8") as f:
	fields = f.read().strip().split()
	if len(fields) >= 2:
	return fields[1]

[gemini-code-assist]

When executing commands interactively over SSH with a pseudo-TTY (ssh -t), passing a multi-line string joined by newlines can sometimes cause issues with terminal echo, line-by-line execution, or race conditions with interactive prompts (like sudo password prompts). Joining the commands with ; instead of \n ensures they are sent as a single robust command line.

Suggested change

	ssh_cmd.extend([ssh_host, "\n".join(wrapped)])
	ssh_cmd.extend([ssh_host, "; ".join(wrapped)])

[danielrmerskine]

Adopted all three findings in the latest commit:

1. Results file moved out of /tmp (security-medium): now ~/.lager-uninstall-results.txt — a fixed name in world-writable /tmp would let another user pre-create or symlink the path and swallow or poison the per-step results. Note: lager update uses the same /tmp results-file pattern for its privileged session; flagged as a follow-up there.
2. with open(..., encoding="utf-8") in lager_key_matcher(), with UnicodeDecodeError handled alongside OSError.
3. Privileged-session payload joined with "; " instead of newlines — behaviorally identical (each element is a complete compound statement, and the newline form ran cleanly through the interactive sudo prompt during hardware validation on both boxes), adopted for robustness and convention.

Since 1 and 3 alter the exact payload that was hardware-validated, the new payload shape was re-verified in a stubbed-ssh sandbox against the real step spec: all 8 steps execute, tilde expansion works, and the results file lands in $HOME with every step reported.