diff --git a/bot_bottle/backend/docker/compose.py b/bot_bottle/backend/docker/compose.py index 7cfe940b..d8a828bc 100644 --- a/bot_bottle/backend/docker/compose.py +++ b/bot_bottle/backend/docker/compose.py @@ -72,9 +72,10 @@ def list_compose_projects( result = subprocess.run( argv, capture_output=True, text=True, check=False, ) - except FileNotFoundError: - # docker binary not on PATH — same shape as a daemon-down - # error from the caller's POV: no projects discoverable. + except OSError: + # docker unavailable — not on PATH, or on it but not executable by + # this user. Same shape as a daemon-down error from the caller's + # POV: no projects discoverable. return [] if result.returncode != 0: if warn_on_error: diff --git a/bot_bottle/backend/docker/enumerate.py b/bot_bottle/backend/docker/enumerate.py index 8bfd91cf..9e7a2727 100644 --- a/bot_bottle/backend/docker/enumerate.py +++ b/bot_bottle/backend/docker/enumerate.py @@ -74,7 +74,8 @@ def _query_services_by_project() -> dict[str, set[str]]: ], capture_output=True, text=True, check=False, ) - except FileNotFoundError: + except OSError: + # docker missing, or on PATH but not executable by this user. return {} if r.returncode != 0: return {} diff --git a/bot_bottle/backend/firecracker/netpool.py b/bot_bottle/backend/firecracker/netpool.py index e3c9508e..d7496d12 100644 --- a/bot_bottle/backend/firecracker/netpool.py +++ b/bot_bottle/backend/firecracker/netpool.py @@ -177,14 +177,20 @@ def gw_slot() -> Slot: # --- fail-closed verification --------------------------------------- def _run_ok(argv: list[str]) -> bool: - """Run a probe command, treating a missing binary as failure + """Run a probe command, treating an unavailable binary as failure (rather than crashing) so callers can stay fail-closed.""" try: return subprocess.run( argv, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False, ).returncode == 0 - except FileNotFoundError: + except OSError: + # Not only "missing". A name on PATH that isn't executable by this + # user raises PermissionError, and CPython reports that EACCES in + # preference to the ENOENT from the other PATH entries — which is + # how `doctor` came to die with a traceback on a fresh macOS + # account. Any OSError means the probe couldn't run, which for a + # fail-closed check is indistinguishable from "not present". return False @@ -244,7 +250,9 @@ def overlapping_routes() -> list[RouteConflict]: ["ip", "-json", "route", "show", "table", "all"], capture_output=True, text=True, check=False, ) - except FileNotFoundError: + except OSError: + # Missing, or present-but-not-executable for this user; either way + # there are no routes we can enumerate. See _run_ok. return [] if proc.returncode != 0 or not proc.stdout.strip(): return [] diff --git a/scripts/macos-install-test.sh b/scripts/macos-install-test.sh index d0a4ba88..f9c70480 100755 --- a/scripts/macos-install-test.sh +++ b/scripts/macos-install-test.sh @@ -89,14 +89,28 @@ user_exists() { dscl . -read "/Users/$USER_NAME" >/dev/null 2>&1; } # Run a shell snippet as the throwaway user in a fresh login shell. run_as_user() { sudo -u "$USER_NAME" -i sh -c "$1"; } -# `bot-bottle doctor` as the throwaway user. Non-zero when the entry point -# never made it onto that user's PATH, or when doctor itself is unhappy. +# `bot-bottle doctor` as the throwaway user. Non-zero when no entry point was +# installed at all, or when doctor itself is unhappy. +# +# Deliberately does NOT require `bot-bottle` on PATH: install.sh prints the +# PATH line rather than editing a shell profile, so on a fresh account the +# entry point is installed and working but not on PATH. Demanding PATH here +# would fail every run for a reason the installer intends. doctor_as_user() { - if ! run_as_user 'command -v bot-bottle >/dev/null 2>&1'; then - echo " bot-bottle is not on PATH for $USER_NAME" - return 1 - fi - run_as_user 'bot-bottle doctor' + # shellcheck disable=SC2016 # $HOME/$bb must expand in the *target* user's + # shell, not in this one — that's the whole point of the single quotes. + run_as_user ' + for bb in "$HOME/.local/bin/bot-bottle" "$HOME/.bot-bottle/venv/bin/bot-bottle"; do + if [ -x "$bb" ]; then + command -v bot-bottle >/dev/null 2>&1 \ + || echo " (not on PATH — running $bb directly, as install.sh advises)" + exec "$bb" doctor + fi + done + command -v bot-bottle >/dev/null 2>&1 && exec bot-bottle doctor + echo " no bot-bottle entry point found for this user" >&2 + exit 1 + ' } # --- commands -------------------------------------------------------- diff --git a/tests/unit/test_compose.py b/tests/unit/test_compose.py index 95f5301e..f17d0527 100644 --- a/tests/unit/test_compose.py +++ b/tests/unit/test_compose.py @@ -43,6 +43,17 @@ class TestProjectNaming(unittest.TestCase): class TestComposeProjectListing(unittest.TestCase): + def test_compose_ls_empty_when_docker_unusable(self): + # Missing is the obvious case; present-but-not-executable raises + # PermissionError instead, which must not escape as a crash. + for exc in (FileNotFoundError, PermissionError(13, "Permission denied", "docker")): + with self.subTest(exc=type(exc).__name__): + with mock.patch( + "bot_bottle.backend.docker.compose.subprocess.run", + side_effect=exc, + ): + self.assertEqual([], list_compose_projects()) + def test_compose_ls_error_warns_by_default(self): with ( mock.patch( diff --git a/tests/unit/test_firecracker_helpers.py b/tests/unit/test_firecracker_helpers.py index 5ae6062c..4e8804b8 100644 --- a/tests/unit/test_firecracker_helpers.py +++ b/tests/unit/test_firecracker_helpers.py @@ -56,6 +56,21 @@ class TestNetpoolProbes(unittest.TestCase): with patch.object(netpool.subprocess, "run", side_effect=FileNotFoundError): self.assertFalse(netpool._run_ok(["nft"])) + def test_run_ok_false_on_unexecutable_binary(self): + # A name on PATH that this user can't execute raises PermissionError, + # not FileNotFoundError — CPython reports that EACCES in preference to + # the ENOENT from the other PATH entries. Catching only the latter made + # `doctor` die with a traceback on a fresh macOS account. + with patch.object(netpool.subprocess, "run", + side_effect=PermissionError(13, "Permission denied", "ip")): + self.assertFalse(netpool._run_ok(["ip", "link", "show", "bbfc0"])) + + def test_overlapping_routes_empty_when_ip_unusable(self): + for exc in (FileNotFoundError, PermissionError(13, "Permission denied", "ip")): + with self.subTest(exc=type(exc).__name__), \ + patch.object(netpool.subprocess, "run", side_effect=exc): + self.assertEqual([], netpool.overlapping_routes()) + def test_tap_and_nft_probes(self): with patch.object(netpool, "_run_ok", return_value=True) as ok: self.assertTrue(netpool.tap_present("bbfc0"))