From 8d315ef340c1fa6b62220a0e3625478345b24674 Mon Sep 17 00:00:00 2001 From: MohamedAklamaash Date: Tue, 23 Jun 2026 13:10:39 +0530 Subject: [PATCH 1/7] Fix silent Copilot MDM hook-install failure Copilot was the only MDM tool installing per-user by downloading unbound.py to a root-owned staging dir and copyfile-ing it into each home after privilege-drop. If the dropped user could not read the staged file, the copy failed silently and main() still exited 0, so the orchestrator marked Copilot installed when no hook was written. - Read the hook script once as root and write it as the target user, so the only cross-privilege op is a write into the user's own home (no staged read). - Add modular _fetch_hook_script / _apply_gateway_url / _write_file helpers; _write_file uses O_NOFOLLOW + fchmod. - main() now returns success and exits non-zero when no user gets installed, so the MDM orchestrator stops reporting a silent failure as success. - Add outermost-layer tests for install_hooks_for_user and main() exit semantics. Co-Authored-By: Claude Opus 4.8 --- copilot/hooks/mdm/setup.py | 103 +++++++++++++-------------- copilot/hooks/mdm/test_install.py | 111 ++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 54 deletions(-) create mode 100644 copilot/hooks/mdm/test_install.py diff --git a/copilot/hooks/mdm/setup.py b/copilot/hooks/mdm/setup.py index c7627bd4..f64f5bf0 100644 --- a/copilot/hooks/mdm/setup.py +++ b/copilot/hooks/mdm/setup.py @@ -644,17 +644,30 @@ def download_file(url: str, dest_path: Path) -> bool: return False -def rewrite_gateway_url_in_file(path: Path, gateway_url: str) -> None: - """Replace the hardcoded default gateway URL inside a downloaded unbound.py.""" +def _apply_gateway_url(text: str, gateway_url: str) -> str: if not gateway_url or gateway_url == DEFAULT_GATEWAY_URL: - return + return text + return text.replace(f'"{DEFAULT_GATEWAY_URL}"', f'"{gateway_url}"') + + +def _fetch_hook_script(gateway_url: str) -> Optional[str]: + staging_dir = Path(tempfile.mkdtemp(prefix="unbound-copilot-")) + source_script = staging_dir / "unbound.py" try: - text = path.read_text(encoding="utf-8") - new_text = text.replace(f'"{DEFAULT_GATEWAY_URL}"', f'"{gateway_url}"') - if new_text != text: - path.write_text(new_text, encoding="utf-8") - except Exception as e: - debug_print(f"Could not rewrite gateway URL in {path}: {e}") + if not download_file(SCRIPT_URL, source_script): + return None + return _apply_gateway_url(source_script.read_text(encoding="utf-8"), gateway_url) + finally: + shutil.rmtree(staging_dir, ignore_errors=True) + + +def _write_file(path: Path, data: str, mode: int) -> None: + flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC | getattr(os, 'O_NOFOLLOW', 0) + fd = os.open(str(path), flags, mode) + with os.fdopen(fd, 'w', encoding='utf-8') as f: + if os.name == "posix": + os.fchmod(f.fileno(), mode) + f.write(data) def _copilot_hooks_config(script_path: Path) -> Dict: @@ -688,27 +701,18 @@ def _copilot_hooks_config(script_path: Path) -> Dict: return {"version": 1, "hooks": hooks} -def install_hooks_for_user(username: str, home_dir: Path, gateway_url: str, source_script: Path) -> bool: - """Install unbound.py and unbound.json into a user's ~/.copilot/hooks. - Privilege-drops to the target user before any FS op — curl already ran as - root to fetch source_script; the per-user write happens post-drop.""" +def install_hooks_for_user(username: str, home_dir: Path, script_text: str) -> bool: + """Install unbound.py and unbound.json into a user's ~/.copilot/hooks, writing + as the target user so a symlink in their home cannot redirect a root write.""" hooks_dir = home_dir / ".copilot" / "hooks" script_path = hooks_dir / "unbound.py" hooks_json = hooks_dir / "unbound.json" - system = platform.system().lower() + script_mode = 0o755 if platform.system().lower() in ("darwin", "linux") else 0o644 def _install(): hooks_dir.mkdir(parents=True, exist_ok=True) - shutil.copyfile(str(source_script), str(script_path)) - if system in ["darwin", "linux"]: - os.chmod(script_path, 0o755) - rewrite_gateway_url_in_file(script_path, gateway_url) - - config = _copilot_hooks_config(script_path) - flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC | getattr(os, 'O_NOFOLLOW', 0) - fd = os.open(str(hooks_json), flags, 0o644) - with os.fdopen(fd, 'w', encoding='utf-8') as f: - json.dump(config, f, indent=2) + _write_file(script_path, script_text, script_mode) + _write_file(hooks_json, json.dumps(_copilot_hooks_config(script_path), indent=2), 0o644) return True ok = bool(_run_as_user(username, _install)) @@ -1281,7 +1285,7 @@ def main(): if clear_mode: clear_setup() - return + return True print("=" * 60) print("Copilot Hooks - MDM Setup") @@ -1295,7 +1299,7 @@ def main(): ) print("This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False base_url = "https://backend.getunbound.ai" gateway_url = DEFAULT_GATEWAY_URL @@ -1334,46 +1338,34 @@ def main(): print("\nMissing required argument: --api-key") print("Usage: sudo python3 setup.py --api-key [--backend-url ] [--app_name ] [--debug] [--backfill]") print(" Or: sudo python3 setup.py --clear [--debug]") - return + return False print("\nGetting device identifier...") device_id = get_device_identifier() if not device_id: print("Failed to get device identifier") - return + return False debug_print(f"Device identifier: {device_id}") print("Device identifier retrieved") print("\nFetching API key from MDM...") api_key = fetch_api_key_from_mdm(base_url, app_name, auth_api_key, device_id) if not api_key: - return + return False print("API key received") print("\nSetting environment variables system-wide...") - success, _ = set_env_var_system_wide("UNBOUND_COPILOT_API_KEY", api_key) - if not success: + env_ok, _ = set_env_var_system_wide("UNBOUND_COPILOT_API_KEY", api_key) + if not env_ok: print("Failed to set UNBOUND_COPILOT_API_KEY") - return + return False debug_print("UNBOUND_COPILOT_API_KEY set successfully") - # Download unbound.py once as root into a private root-owned temp dir. - # mkdtemp gives an unpredictable name so a local user cannot pre-create or - # symlink the path the root curl writes to. The dir is then widened to - # 0o711 (traverse, still no listing) and the file to 0o644 so the per-user - # installs — which run after privilege-drop — can read the script. print("\nDownloading hook script...") - staging_dir = Path(tempfile.mkdtemp(prefix="unbound-copilot-")) - source_script = staging_dir / "unbound.py" - if not download_file(SCRIPT_URL, source_script): + script_text = _fetch_hook_script(gateway_url) + if script_text is None: print("Failed to download unbound.py") - shutil.rmtree(staging_dir, ignore_errors=True) - return - try: - os.chmod(staging_dir, 0o711) - os.chmod(source_script, 0o644) - except OSError: - pass + return False state = detect_install_state() @@ -1382,20 +1374,19 @@ def main(): installed_count = 0 for username, home_dir in user_homes: write_unbound_config_for_user(username, home_dir, api_key, urls={"base_url": base_url, "gateway_url": gateway_url, "frontend_url": frontend_url}) - if install_hooks_for_user(username, home_dir, gateway_url, source_script): + if install_hooks_for_user(username, home_dir, script_text): installed_count += 1 - shutil.rmtree(staging_dir, ignore_errors=True) - + success = bool(user_homes) and installed_count == len(user_homes) if not user_homes: print("No user home directories found") - elif installed_count == len(user_homes): + elif success: print(f"Installed Copilot hooks for {installed_count} user(s)") else: print(f"Installed Copilot hooks for {installed_count} of {len(user_homes)} user(s) — {len(user_homes) - installed_count} failed") print("\n" + "=" * 60) - print("Setup Complete!") + print("Setup Complete!" if success else "Setup Failed") print("=" * 60) notify_setup_complete(api_key, "copilot", backend_url=base_url, install_state=state, serial_number=device_id) @@ -1403,12 +1394,16 @@ def main(): if backfill_mode: run_backfill(api_key, base_url, user_homes) + return success + if __name__ == "__main__": try: - main() + ok = main() except KeyboardInterrupt: print("\n\nSetup cancelled.") + sys.exit(1) except Exception as e: print(f"\nError: {e}") - exit(1) + sys.exit(1) + sys.exit(0 if ok else 1) diff --git a/copilot/hooks/mdm/test_install.py b/copilot/hooks/mdm/test_install.py new file mode 100644 index 00000000..8c578982 --- /dev/null +++ b/copilot/hooks/mdm/test_install.py @@ -0,0 +1,111 @@ +"""Install-path tests for the Copilot MDM setup (WEB copilot-mdm-install-fix). + +Covers the bug where per-user install silently failed and main() still exited 0: +- install_hooks_for_user must write unbound.py + unbound.json into the user's home + (it runs as the target user; running as the current user is permitted). +- main() must return False (non-zero exit) when no user gets the hook installed, + so the MDM orchestrator stops reporting a silent failure as success. +""" +import getpass +import importlib.util +import json +import os +import platform +import stat +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +_spec = importlib.util.spec_from_file_location( + "_copilot_mdm_setup", os.path.join(os.path.dirname(__file__), "setup.py") +) +mdm = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(mdm) + +_SCRIPT = "#!/usr/bin/env python3\nprint('unbound copilot hook')\n" + + +class TestInstallHooksForUser(unittest.TestCase): + def setUp(self): + self.home = Path(tempfile.mkdtemp()) + self.me = getpass.getuser() + # _run_as_user forks + setuid/setgroups (root-only); the install bug lived + # in the inner write path, so run it directly under the test user. + p = mock.patch.object(mdm, "_run_as_user", lambda _u, fn, *a, **k: fn(*a, **k)) + p.start() + self.addCleanup(p.stop) + + def test_writes_script_and_config(self): + ok = mdm.install_hooks_for_user(self.me, self.home, _SCRIPT) + self.assertTrue(ok) + + script_path = self.home / ".copilot" / "hooks" / "unbound.py" + hooks_json = self.home / ".copilot" / "hooks" / "unbound.json" + self.assertEqual(script_path.read_text(), _SCRIPT) + + config = json.loads(hooks_json.read_text()) + self.assertEqual(config["version"], 1) + self.assertIn("PreToolUse", config["hooks"]) + + @unittest.skipIf(platform.system().lower() == "windows", "POSIX mode bits") + def test_script_is_executable(self): + mdm.install_hooks_for_user(self.me, self.home, _SCRIPT) + script_path = self.home / ".copilot" / "hooks" / "unbound.py" + self.assertTrue(os.stat(script_path).st_mode & stat.S_IXUSR) + + +def _patch_main_deps(homes, install_results=True, script_text=_SCRIPT): + install = ( + mock.Mock(side_effect=install_results) + if isinstance(install_results, list) + else mock.Mock(return_value=install_results) + ) + patches = [ + mock.patch.object(mdm.sys, "argv", ["setup.py", "--api-key", "k"]), + mock.patch.object(mdm, "check_admin_privileges", return_value=True), + mock.patch.object(mdm, "get_device_identifier", return_value="dev-1"), + mock.patch.object(mdm, "fetch_api_key_from_mdm", return_value="api-key"), + mock.patch.object(mdm, "set_env_var_system_wide", return_value=(True, True)), + mock.patch.object(mdm, "_fetch_hook_script", return_value=script_text), + mock.patch.object(mdm, "detect_install_state", return_value="fresh"), + mock.patch.object(mdm, "get_all_user_homes", return_value=homes), + mock.patch.object(mdm, "write_unbound_config_for_user"), + mock.patch.object(mdm, "install_hooks_for_user", install), + mock.patch.object(mdm, "notify_setup_complete"), + ] + return patches, install + + +class TestMainExitSemantics(unittest.TestCase): + def _run_main(self, homes, install_results=True, script_text=_SCRIPT): + patches, install = _patch_main_deps(homes, install_results, script_text) + for p in patches: + p.start() + self.addCleanup(mock.patch.stopall) + return mdm.main(), install + + def test_full_success_returns_true(self): + result, _ = self._run_main([("u1", Path("/tmp/h1"))]) + self.assertTrue(result) + + def test_no_users_returns_false(self): + result, install = self._run_main([]) + self.assertFalse(result) + install.assert_not_called() + + def test_partial_failure_returns_false(self): + result, _ = self._run_main( + [("u1", Path("/tmp/h1")), ("u2", Path("/tmp/h2"))], + install_results=[True, False], + ) + self.assertFalse(result) + + def test_download_failure_returns_false(self): + result, install = self._run_main([("u1", Path("/tmp/h1"))], script_text=None) + self.assertFalse(result) + install.assert_not_called() + + +if __name__ == "__main__": + unittest.main() From d2c8cc05df1728b231604c70a9698a4a372ce4d5 Mon Sep 17 00:00:00 2001 From: MohamedAklamaash Date: Tue, 23 Jun 2026 13:15:56 +0530 Subject: [PATCH 2/7] Address review: gate completion notify on success, add symlink/gateway tests - Only call notify_setup_complete when every user was installed, so the backend is not told setup completed on a partial/failed run (exit code already reflects this to the orchestrator). - Add tests: O_NOFOLLOW refuses a symlinked unbound.py path; _apply_gateway_url rewrite vs identity; notify is/ isn't called on success vs partial failure. Co-Authored-By: Claude Opus 4.8 --- copilot/hooks/mdm/setup.py | 3 +- copilot/hooks/mdm/test_install.py | 58 ++++++++++++++++++++++++------- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/copilot/hooks/mdm/setup.py b/copilot/hooks/mdm/setup.py index f64f5bf0..c9c96936 100644 --- a/copilot/hooks/mdm/setup.py +++ b/copilot/hooks/mdm/setup.py @@ -1389,7 +1389,8 @@ def main(): print("Setup Complete!" if success else "Setup Failed") print("=" * 60) - notify_setup_complete(api_key, "copilot", backend_url=base_url, install_state=state, serial_number=device_id) + if success: + notify_setup_complete(api_key, "copilot", backend_url=base_url, install_state=state, serial_number=device_id) if backfill_mode: run_backfill(api_key, base_url, user_homes) diff --git a/copilot/hooks/mdm/test_install.py b/copilot/hooks/mdm/test_install.py index 8c578982..11e23966 100644 --- a/copilot/hooks/mdm/test_install.py +++ b/copilot/hooks/mdm/test_install.py @@ -31,8 +31,15 @@ def setUp(self): self.home = Path(tempfile.mkdtemp()) self.me = getpass.getuser() # _run_as_user forks + setuid/setgroups (root-only); the install bug lived - # in the inner write path, so run it directly under the test user. - p = mock.patch.object(mdm, "_run_as_user", lambda _u, fn, *a, **k: fn(*a, **k)) + # in the inner write path, so run it directly under the test user while + # preserving its contract (return None on failure). + def _direct(_u, fn, *a, **k): + try: + return fn(*a, **k) + except Exception: + return None + + p = mock.patch.object(mdm, "_run_as_user", _direct) p.start() self.addCleanup(p.stop) @@ -54,6 +61,28 @@ def test_script_is_executable(self): script_path = self.home / ".copilot" / "hooks" / "unbound.py" self.assertTrue(os.stat(script_path).st_mode & stat.S_IXUSR) + @unittest.skipIf(platform.system().lower() == "windows", "O_NOFOLLOW is POSIX") + def test_symlinked_script_path_is_refused(self): + hooks_dir = self.home / ".copilot" / "hooks" + hooks_dir.mkdir(parents=True) + target = Path(tempfile.mkdtemp()) / "evil" + (hooks_dir / "unbound.py").symlink_to(target) + ok = mdm.install_hooks_for_user(self.me, self.home, _SCRIPT) + self.assertFalse(ok) + self.assertFalse(target.exists()) + + +class TestApplyGatewayUrl(unittest.TestCase): + def test_rewrites_non_default_url(self): + text = f'GATEWAY = "{mdm.DEFAULT_GATEWAY_URL}"' + out = mdm._apply_gateway_url(text, "https://gw.example.com") + self.assertIn('"https://gw.example.com"', out) + self.assertNotIn(mdm.DEFAULT_GATEWAY_URL, out) + + def test_default_url_is_unchanged(self): + text = f'GATEWAY = "{mdm.DEFAULT_GATEWAY_URL}"' + self.assertEqual(mdm._apply_gateway_url(text, mdm.DEFAULT_GATEWAY_URL), text) + def _patch_main_deps(homes, install_results=True, script_text=_SCRIPT): install = ( @@ -61,6 +90,7 @@ def _patch_main_deps(homes, install_results=True, script_text=_SCRIPT): if isinstance(install_results, list) else mock.Mock(return_value=install_results) ) + notify = mock.Mock() patches = [ mock.patch.object(mdm.sys, "argv", ["setup.py", "--api-key", "k"]), mock.patch.object(mdm, "check_admin_privileges", return_value=True), @@ -72,39 +102,43 @@ def _patch_main_deps(homes, install_results=True, script_text=_SCRIPT): mock.patch.object(mdm, "get_all_user_homes", return_value=homes), mock.patch.object(mdm, "write_unbound_config_for_user"), mock.patch.object(mdm, "install_hooks_for_user", install), - mock.patch.object(mdm, "notify_setup_complete"), + mock.patch.object(mdm, "notify_setup_complete", notify), ] - return patches, install + return patches, install, notify class TestMainExitSemantics(unittest.TestCase): def _run_main(self, homes, install_results=True, script_text=_SCRIPT): - patches, install = _patch_main_deps(homes, install_results, script_text) + patches, install, notify = _patch_main_deps(homes, install_results, script_text) for p in patches: p.start() self.addCleanup(mock.patch.stopall) - return mdm.main(), install + return mdm.main(), install, notify - def test_full_success_returns_true(self): - result, _ = self._run_main([("u1", Path("/tmp/h1"))]) + def test_full_success_returns_true_and_notifies(self): + result, _, notify = self._run_main([("u1", Path("/tmp/h1"))]) self.assertTrue(result) + notify.assert_called_once() def test_no_users_returns_false(self): - result, install = self._run_main([]) + result, install, notify = self._run_main([]) self.assertFalse(result) install.assert_not_called() + notify.assert_not_called() - def test_partial_failure_returns_false(self): - result, _ = self._run_main( + def test_partial_failure_returns_false_and_does_not_notify(self): + result, _, notify = self._run_main( [("u1", Path("/tmp/h1")), ("u2", Path("/tmp/h2"))], install_results=[True, False], ) self.assertFalse(result) + notify.assert_not_called() def test_download_failure_returns_false(self): - result, install = self._run_main([("u1", Path("/tmp/h1"))], script_text=None) + result, install, notify = self._run_main([("u1", Path("/tmp/h1"))], script_text=None) self.assertFalse(result) install.assert_not_called() + notify.assert_not_called() if __name__ == "__main__": From 5d2e870c7aaff7feafc202dc75d860304cbc6b6c Mon Sep 17 00:00:00 2001 From: MohamedAklamaash Date: Tue, 23 Jun 2026 13:25:26 +0530 Subject: [PATCH 3/7] Address Greptile P2s: surface failure reasons, guard fd, fail soft on read - _fetch_hook_script catches a read failure (corrupt/transient) and returns None so main() prints the clean download-failed message instead of a raw traceback. - _install debug_prints the underlying reason before re-raising, so a per-user install failure is diagnosable (it is swallowed in the _run_as_user fork). - _write_file closes the fd if os.fdopen raises before taking ownership. Co-Authored-By: Claude Opus 4.8 --- copilot/hooks/mdm/setup.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/copilot/hooks/mdm/setup.py b/copilot/hooks/mdm/setup.py index c9c96936..8ca3adc4 100644 --- a/copilot/hooks/mdm/setup.py +++ b/copilot/hooks/mdm/setup.py @@ -657,6 +657,9 @@ def _fetch_hook_script(gateway_url: str) -> Optional[str]: if not download_file(SCRIPT_URL, source_script): return None return _apply_gateway_url(source_script.read_text(encoding="utf-8"), gateway_url) + except Exception as e: + debug_print(f"Could not read downloaded unbound.py: {e}") + return None finally: shutil.rmtree(staging_dir, ignore_errors=True) @@ -664,7 +667,12 @@ def _fetch_hook_script(gateway_url: str) -> Optional[str]: def _write_file(path: Path, data: str, mode: int) -> None: flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC | getattr(os, 'O_NOFOLLOW', 0) fd = os.open(str(path), flags, mode) - with os.fdopen(fd, 'w', encoding='utf-8') as f: + try: + f = os.fdopen(fd, 'w', encoding='utf-8') + except Exception: + os.close(fd) + raise + with f: if os.name == "posix": os.fchmod(f.fileno(), mode) f.write(data) @@ -710,10 +718,14 @@ def install_hooks_for_user(username: str, home_dir: Path, script_text: str) -> b script_mode = 0o755 if platform.system().lower() in ("darwin", "linux") else 0o644 def _install(): - hooks_dir.mkdir(parents=True, exist_ok=True) - _write_file(script_path, script_text, script_mode) - _write_file(hooks_json, json.dumps(_copilot_hooks_config(script_path), indent=2), 0o644) - return True + try: + hooks_dir.mkdir(parents=True, exist_ok=True) + _write_file(script_path, script_text, script_mode) + _write_file(hooks_json, json.dumps(_copilot_hooks_config(script_path), indent=2), 0o644) + return True + except Exception as e: + debug_print(f"install for {username} failed: {e}") + raise ok = bool(_run_as_user(username, _install)) debug_print(f"{'Installed' if ok else 'Failed to install'} Copilot hooks for {username}") From e7b2d85ca74f70d0b8a56e85c923b16e542a4e51 Mon Sep 17 00:00:00 2001 From: MohamedAklamaash Date: Tue, 23 Jun 2026 15:58:20 +0530 Subject: [PATCH 4/7] Remove test_install.py from this PR Co-Authored-By: Claude Opus 4.8 --- copilot/hooks/mdm/test_install.py | 145 ------------------------------ 1 file changed, 145 deletions(-) delete mode 100644 copilot/hooks/mdm/test_install.py diff --git a/copilot/hooks/mdm/test_install.py b/copilot/hooks/mdm/test_install.py deleted file mode 100644 index 11e23966..00000000 --- a/copilot/hooks/mdm/test_install.py +++ /dev/null @@ -1,145 +0,0 @@ -"""Install-path tests for the Copilot MDM setup (WEB copilot-mdm-install-fix). - -Covers the bug where per-user install silently failed and main() still exited 0: -- install_hooks_for_user must write unbound.py + unbound.json into the user's home - (it runs as the target user; running as the current user is permitted). -- main() must return False (non-zero exit) when no user gets the hook installed, - so the MDM orchestrator stops reporting a silent failure as success. -""" -import getpass -import importlib.util -import json -import os -import platform -import stat -import tempfile -import unittest -from pathlib import Path -from unittest import mock - -_spec = importlib.util.spec_from_file_location( - "_copilot_mdm_setup", os.path.join(os.path.dirname(__file__), "setup.py") -) -mdm = importlib.util.module_from_spec(_spec) -_spec.loader.exec_module(mdm) - -_SCRIPT = "#!/usr/bin/env python3\nprint('unbound copilot hook')\n" - - -class TestInstallHooksForUser(unittest.TestCase): - def setUp(self): - self.home = Path(tempfile.mkdtemp()) - self.me = getpass.getuser() - # _run_as_user forks + setuid/setgroups (root-only); the install bug lived - # in the inner write path, so run it directly under the test user while - # preserving its contract (return None on failure). - def _direct(_u, fn, *a, **k): - try: - return fn(*a, **k) - except Exception: - return None - - p = mock.patch.object(mdm, "_run_as_user", _direct) - p.start() - self.addCleanup(p.stop) - - def test_writes_script_and_config(self): - ok = mdm.install_hooks_for_user(self.me, self.home, _SCRIPT) - self.assertTrue(ok) - - script_path = self.home / ".copilot" / "hooks" / "unbound.py" - hooks_json = self.home / ".copilot" / "hooks" / "unbound.json" - self.assertEqual(script_path.read_text(), _SCRIPT) - - config = json.loads(hooks_json.read_text()) - self.assertEqual(config["version"], 1) - self.assertIn("PreToolUse", config["hooks"]) - - @unittest.skipIf(platform.system().lower() == "windows", "POSIX mode bits") - def test_script_is_executable(self): - mdm.install_hooks_for_user(self.me, self.home, _SCRIPT) - script_path = self.home / ".copilot" / "hooks" / "unbound.py" - self.assertTrue(os.stat(script_path).st_mode & stat.S_IXUSR) - - @unittest.skipIf(platform.system().lower() == "windows", "O_NOFOLLOW is POSIX") - def test_symlinked_script_path_is_refused(self): - hooks_dir = self.home / ".copilot" / "hooks" - hooks_dir.mkdir(parents=True) - target = Path(tempfile.mkdtemp()) / "evil" - (hooks_dir / "unbound.py").symlink_to(target) - ok = mdm.install_hooks_for_user(self.me, self.home, _SCRIPT) - self.assertFalse(ok) - self.assertFalse(target.exists()) - - -class TestApplyGatewayUrl(unittest.TestCase): - def test_rewrites_non_default_url(self): - text = f'GATEWAY = "{mdm.DEFAULT_GATEWAY_URL}"' - out = mdm._apply_gateway_url(text, "https://gw.example.com") - self.assertIn('"https://gw.example.com"', out) - self.assertNotIn(mdm.DEFAULT_GATEWAY_URL, out) - - def test_default_url_is_unchanged(self): - text = f'GATEWAY = "{mdm.DEFAULT_GATEWAY_URL}"' - self.assertEqual(mdm._apply_gateway_url(text, mdm.DEFAULT_GATEWAY_URL), text) - - -def _patch_main_deps(homes, install_results=True, script_text=_SCRIPT): - install = ( - mock.Mock(side_effect=install_results) - if isinstance(install_results, list) - else mock.Mock(return_value=install_results) - ) - notify = mock.Mock() - patches = [ - mock.patch.object(mdm.sys, "argv", ["setup.py", "--api-key", "k"]), - mock.patch.object(mdm, "check_admin_privileges", return_value=True), - mock.patch.object(mdm, "get_device_identifier", return_value="dev-1"), - mock.patch.object(mdm, "fetch_api_key_from_mdm", return_value="api-key"), - mock.patch.object(mdm, "set_env_var_system_wide", return_value=(True, True)), - mock.patch.object(mdm, "_fetch_hook_script", return_value=script_text), - mock.patch.object(mdm, "detect_install_state", return_value="fresh"), - mock.patch.object(mdm, "get_all_user_homes", return_value=homes), - mock.patch.object(mdm, "write_unbound_config_for_user"), - mock.patch.object(mdm, "install_hooks_for_user", install), - mock.patch.object(mdm, "notify_setup_complete", notify), - ] - return patches, install, notify - - -class TestMainExitSemantics(unittest.TestCase): - def _run_main(self, homes, install_results=True, script_text=_SCRIPT): - patches, install, notify = _patch_main_deps(homes, install_results, script_text) - for p in patches: - p.start() - self.addCleanup(mock.patch.stopall) - return mdm.main(), install, notify - - def test_full_success_returns_true_and_notifies(self): - result, _, notify = self._run_main([("u1", Path("/tmp/h1"))]) - self.assertTrue(result) - notify.assert_called_once() - - def test_no_users_returns_false(self): - result, install, notify = self._run_main([]) - self.assertFalse(result) - install.assert_not_called() - notify.assert_not_called() - - def test_partial_failure_returns_false_and_does_not_notify(self): - result, _, notify = self._run_main( - [("u1", Path("/tmp/h1")), ("u2", Path("/tmp/h2"))], - install_results=[True, False], - ) - self.assertFalse(result) - notify.assert_not_called() - - def test_download_failure_returns_false(self): - result, install, notify = self._run_main([("u1", Path("/tmp/h1"))], script_text=None) - self.assertFalse(result) - install.assert_not_called() - notify.assert_not_called() - - -if __name__ == "__main__": - unittest.main() From c32e8d2df967efdba39163dabef77ac47307d4d1 Mon Sep 17 00:00:00 2001 From: MohamedAklamaash Date: Mon, 29 Jun 2026 13:45:56 +0530 Subject: [PATCH 5/7] Propagate MDM setup failures via exit code across all tools The MDM orchestrator (mdm/onboard.py) gates each tool's success on the subprocess returncode. Every tool's setup.py main() returned None on its error paths and __main__ exited 0 unless an exception was raised, so any mid-run failure (missing privileges, MDM key fetch, env var write, hook config) reported success to the orchestrator. This mirrors the exit-code contract landed for copilot in #171: - main() returns True on success / clear, False on every error path - __main__ captures the result and exits 1 on KeyboardInterrupt, on exception, or when main() returns falsy; 0 only on real success Applied to augment, claude-code (gateway + hooks), codex (gateway + hooks), cursor, and gemini-cli. The copilot per-user read-as-root/ write-as-user refactor is not ported: those tools download the hook script as the dropped target user into the user's own home, so they never had copilot's cross-privilege read failure. Co-Authored-By: Claude Opus 4.8 --- augment/hooks/mdm/setup.py | 22 +++++++++++++--------- claude-code/gateway/mdm/setup.py | 24 ++++++++++++++---------- claude-code/hooks/mdm/setup.py | 22 +++++++++++++--------- codex/gateway/mdm/setup.py | 24 ++++++++++++++---------- codex/hooks/mdm/setup.py | 22 +++++++++++++--------- cursor/mdm/setup.py | 22 +++++++++++++--------- gemini-cli/gateway/mdm/setup.py | 22 +++++++++++++--------- 7 files changed, 93 insertions(+), 65 deletions(-) diff --git a/augment/hooks/mdm/setup.py b/augment/hooks/mdm/setup.py index 0279e34a..df451213 100644 --- a/augment/hooks/mdm/setup.py +++ b/augment/hooks/mdm/setup.py @@ -1252,7 +1252,7 @@ def main(): if clear_mode: clear_setup() - return + return True print("=" * 60) print("Augment Code Hooks - MDM Setup") @@ -1266,7 +1266,7 @@ def main(): ) print("This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False base_url = "https://backend.getunbound.ai" gateway_url = DEFAULT_GATEWAY_URL @@ -1301,27 +1301,27 @@ def main(): print("\nMissing required argument: --api-key") print("Usage: sudo python3 setup.py --api-key [--backend-url ] [--app_name ] [--debug]") print(" Or: sudo python3 setup.py --clear [--debug]") - return + return False print("\nGetting device identifier...") device_id = get_device_identifier() if not device_id: print("Failed to get device identifier") - return + return False debug_print(f"Device identifier: {device_id}") print("Device identifier retrieved") print("\nFetching API key from MDM...") api_key = fetch_api_key_from_mdm(base_url, app_name, auth_api_key, device_id) if not api_key: - return + return False print("API key received") print("\nSetting environment variables system-wide...") success, _ = set_env_var_system_wide("UNBOUND_AUGMENT_API_KEY", api_key) if not success: print("Failed to set UNBOUND_AUGMENT_API_KEY") - return + return False debug_print("UNBOUND_AUGMENT_API_KEY set successfully") # Write the per-user unbound config now (needed by the managed hook; harmless @@ -1336,7 +1336,7 @@ def main(): print("\nConfiguring Augment managed hooks...") if not setup_managed_hooks(gateway_url=gateway_url): print("Failed to configure managed hooks") - return + return False managed_dir = get_managed_settings_dir() print(f"Created managed hooks in {managed_dir}") @@ -1357,12 +1357,16 @@ def main(): notify_setup_complete(api_key, "augment_code", backend_url=base_url, install_state=state, serial_number=device_id) + return True + if __name__ == "__main__": try: - main() + ok = main() except KeyboardInterrupt: print("\n\nSetup cancelled.") + sys.exit(1) except Exception as e: print(f"\nError: {e}") - exit(1) + sys.exit(1) + sys.exit(0 if ok else 1) diff --git a/claude-code/gateway/mdm/setup.py b/claude-code/gateway/mdm/setup.py index d33a4dd9..aa4eea5a 100644 --- a/claude-code/gateway/mdm/setup.py +++ b/claude-code/gateway/mdm/setup.py @@ -905,7 +905,7 @@ def main(): if clear_mode: clear_setup() - return + return True print("=" * 60) print("Claude Code - MDM Setup") @@ -919,7 +919,7 @@ def main(): ) print("❌ This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False base_url = "https://backend.getunbound.ai" gateway_url = DEFAULT_GATEWAY_URL @@ -950,13 +950,13 @@ def main(): print("\n❌ Missing required argument: --api-key") print("Usage: sudo python3 setup.py --api-key [--backend-url ] [--app_name ] [--debug]") print(" Or: sudo python3 setup.py --clear [--debug]") - return + return False print("\n🔍 Getting device identifier...") device_id = get_device_identifier() if not device_id: print("❌ Failed to get device identifier") - return + return False debug_print(f"Device identifier: {device_id}") print("✅ Device identifier retrieved") @@ -966,7 +966,7 @@ def main(): print("\n🔑 Fetching API key from MDM...") claude_api_key = fetch_api_key_from_mdm(base_url, app_name, auth_api_key, device_id) if not claude_api_key: - return + return False print("✅ API key received") print("\n📝 Setting environment variables system-wide...") @@ -977,13 +977,13 @@ def main(): success, env_changed = set_env_var_system_wide("UNBOUND_API_KEY", claude_api_key) if not success: print(f"❌ Failed to set UNBOUND_API_KEY") - return + return False debug_print("UNBOUND_API_KEY set successfully") success, url_changed = set_env_var_system_wide("ANTHROPIC_BASE_URL", gateway_url) if not success: print(f"❌ Failed to set ANTHROPIC_BASE_URL") - return + return False debug_print("ANTHROPIC_BASE_URL set successfully") # Remove leftover hooks scripts, strip leftover user-level Unbound @@ -1000,7 +1000,7 @@ def main(): print(f"✅ Created managed settings in {managed_dir}") else: print("❌ Failed to configure managed settings") - return + return False print("\n" + "=" * 60) print("Setup Complete!") @@ -1008,12 +1008,16 @@ def main(): notify_setup_complete(claude_api_key, "unbound-claude-code", backend_url=base_url, install_state=install_state, serial_number=device_id) + return True + if __name__ == "__main__": try: - main() + ok = main() except KeyboardInterrupt: print("\n\n⚠️ Setup cancelled.") + sys.exit(1) except Exception as e: print(f"\n❌ Error: {e}") - exit(1) + sys.exit(1) + sys.exit(0 if ok else 1) diff --git a/claude-code/hooks/mdm/setup.py b/claude-code/hooks/mdm/setup.py index bd4de6ec..47322aea 100644 --- a/claude-code/hooks/mdm/setup.py +++ b/claude-code/hooks/mdm/setup.py @@ -1534,7 +1534,7 @@ def main(): if clear_mode: clear_setup() - return + return True print("=" * 60) print("Claude Code Hooks - MDM Setup") @@ -1548,7 +1548,7 @@ def main(): ) print("This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False base_url = "https://backend.getunbound.ai" gateway_url = DEFAULT_GATEWAY_URL @@ -1587,20 +1587,20 @@ def main(): print("\nMissing required argument: --api-key") print("Usage: sudo python3 setup.py --api-key [--backend-url ] [--app_name ] [--debug] [--backfill]") print(" Or: sudo python3 setup.py --clear [--debug]") - return + return False print("\nGetting device identifier...") device_id = get_device_identifier() if not device_id: print("Failed to get device identifier") - return + return False debug_print(f"Device identifier: {device_id}") print("Device identifier retrieved") print("\nFetching API key from MDM...") api_key = fetch_api_key_from_mdm(base_url, app_name, auth_api_key, device_id) if not api_key: - return + return False print("API key received") print("\nSetting environment variables system-wide...") @@ -1612,7 +1612,7 @@ def main(): success, _ = set_env_var_system_wide("UNBOUND_CLAUDE_API_KEY", api_key) if not success: print("Failed to set UNBOUND_CLAUDE_API_KEY") - return + return False debug_print("UNBOUND_CLAUDE_API_KEY set successfully") # Remove gateway artifacts, strip leftover user-level Unbound hooks @@ -1630,7 +1630,7 @@ def main(): print(f"Created managed hooks in {managed_dir}") else: print("Failed to configure managed hooks") - return + return False print("\n" + "=" * 60) print("Setup Complete!") @@ -1641,12 +1641,16 @@ def main(): if backfill_mode: run_backfill(api_key, base_url, get_all_user_homes()) + return True + if __name__ == "__main__": try: - main() + ok = main() except KeyboardInterrupt: print("\n\nSetup cancelled.") + sys.exit(1) except Exception as e: print(f"\nError: {e}") - exit(1) + sys.exit(1) + sys.exit(0 if ok else 1) diff --git a/codex/gateway/mdm/setup.py b/codex/gateway/mdm/setup.py index 4ab8c981..9c1056f5 100644 --- a/codex/gateway/mdm/setup.py +++ b/codex/gateway/mdm/setup.py @@ -868,7 +868,7 @@ def main(): if clear_mode: clear_setup() - return + return True print("=" * 60) print("Codex - MDM Setup") @@ -882,7 +882,7 @@ def main(): ) print("❌ This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False base_url = "https://backend.getunbound.ai" gateway_url = DEFAULT_GATEWAY_URL @@ -913,13 +913,13 @@ def main(): print("\n❌ Missing required argument: --api-key") print("Usage: sudo python3 setup.py --api-key [--backend-url ] [--app_name ] [--debug]") print(" Or: sudo python3 setup.py --clear [--debug]") - return + return False print("\n🔍 Getting device identifier...") device_id = get_device_identifier() if not device_id: print("❌ Failed to get device identifier") - return + return False debug_print(f"Device identifier: {device_id}") print("✅ Device identifier retrieved") @@ -929,7 +929,7 @@ def main(): print("\n🔑 Fetching API key from MDM...") codex_api_key = fetch_api_key_from_mdm(base_url, app_name, auth_api_key, device_id) if not codex_api_key: - return + return False print("✅ API key received") # Remove leftover hooks setup env var @@ -940,14 +940,14 @@ def main(): success, env_changed = set_env_var_system_wide("OPENAI_API_KEY", codex_api_key) if not success: print(f"❌ Failed to set OPENAI_API_KEY") - return + return False debug_print("OPENAI_API_KEY set successfully") print("\n📝 Configuring all users...") user_homes = get_all_user_homes() if not user_homes: print("❌ No user home directories found") - return + return False config_count = 0 for username, home_dir in user_homes: if write_codex_config_for_user(username, home_dir, f"{gateway_url.rstrip('/')}/v1"): @@ -958,7 +958,7 @@ def main(): if config_count == 0: print("❌ Failed to configure codex for any users") - return + return False print(f"✅ Configured {config_count} user(s)") @@ -971,12 +971,16 @@ def main(): notify_setup_complete(codex_api_key, "unbound-codex", backend_url=base_url, install_state=install_state, serial_number=device_id) + return True + if __name__ == "__main__": try: - main() + ok = main() except KeyboardInterrupt: print("\n\n⚠️ Setup cancelled by user.") + sys.exit(1) except Exception as e: print(f"\n❌ An error occurred: {e}") - exit(1) + sys.exit(1) + sys.exit(0 if ok else 1) diff --git a/codex/hooks/mdm/setup.py b/codex/hooks/mdm/setup.py index c3496db4..b6a77d85 100644 --- a/codex/hooks/mdm/setup.py +++ b/codex/hooks/mdm/setup.py @@ -1678,7 +1678,7 @@ def main(): if clear_mode: clear_setup() - return + return True print("=" * 60) print("Codex Hooks - MDM Setup") @@ -1692,7 +1692,7 @@ def main(): ) print("This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False base_url = "https://backend.getunbound.ai" gateway_url = DEFAULT_GATEWAY_URL @@ -1731,20 +1731,20 @@ def main(): print("\nMissing required argument: --api-key") print("Usage: sudo python3 setup.py --api-key [--backend-url ] [--app_name ] [--debug] [--backfill]") print(" Or: sudo python3 setup.py --clear [--debug]") - return + return False print("\nGetting device identifier...") device_id = get_device_identifier() if not device_id: print("Failed to get device identifier") - return + return False debug_print(f"Device identifier: {device_id}") print("Device identifier retrieved") print("\nFetching API key from MDM...") api_key = fetch_api_key_from_mdm(base_url, app_name, auth_api_key, device_id) if not api_key: - return + return False print("API key received") print("\nSetting environment variables system-wide...") @@ -1755,7 +1755,7 @@ def main(): success, _ = set_env_var_system_wide("UNBOUND_CODEX_API_KEY", api_key) if not success: print("Failed to set UNBOUND_CODEX_API_KEY") - return + return False debug_print("UNBOUND_CODEX_API_KEY set successfully") # codex 0.125 discovers hooks from ~/.codex/hooks.json (user layer), not the @@ -1777,7 +1777,7 @@ def main(): if user_homes and installed == 0: print("Failed to configure codex hooks for any user") - return + return False print("\n" + "=" * 60) print("Setup Complete!") @@ -1788,12 +1788,16 @@ def main(): if backfill_mode: run_backfill(api_key, base_url, get_all_user_homes()) + return True + if __name__ == "__main__": try: - main() + ok = main() except KeyboardInterrupt: print("\n\nSetup cancelled.") + sys.exit(1) except Exception as e: print(f"\nError: {e}") - exit(1) + sys.exit(1) + sys.exit(0 if ok else 1) diff --git a/cursor/mdm/setup.py b/cursor/mdm/setup.py index b6740e09..0f7dff5c 100644 --- a/cursor/mdm/setup.py +++ b/cursor/mdm/setup.py @@ -1044,7 +1044,7 @@ def main(): # If clear mode, run cleanup and exit if clear_mode: clear_setup() - return + return True print("=" * 60) print("Unbound Cursor Hooks - MDM Setup") @@ -1060,7 +1060,7 @@ def main(): ) print("❌ This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False # Parse arguments base_url = "https://backend.getunbound.ai" @@ -1096,14 +1096,14 @@ def main(): print("\n❌ Missing required argument: --api-key") print("Usage: sudo python3 setup.py --api-key [--backend-url ] [--app_name ] [--debug]") print(" Or: sudo python3 setup.py --clear [--debug]") - return + return False # Get device identifier print("\n🔍 Getting device serial number...") serial_number = get_device_identifier() if not serial_number: print("❌ Failed to get device serial number") - return + return False debug_print(f"Serial number: {serial_number}") print("✅ Serial number retrieved") @@ -1111,7 +1111,7 @@ def main(): print("\n🔑 Fetching API key from MDM...") cursor_api_key = fetch_api_key_from_mdm(base_url, app_name, auth_api_key, serial_number) if not cursor_api_key: - return + return False print("✅ API key received") # Set environment variable @@ -1119,7 +1119,7 @@ def main(): success, env_changed, message = set_env_var("UNBOUND_CURSOR_API_KEY", cursor_api_key) if not success: print(f"❌ Failed to set environment variable: {message}") - return + return False print(f"✅ Environment variable set ({message})") # Write API key to ~/.unbound/config.json for each user and remove user-level hooks @@ -1142,7 +1142,7 @@ def main(): hooks_success, hooks_changed = setup_hooks(gateway_url=gateway_url) if not hooks_success: print("\n❌ Failed to setup hooks") - return + return False debug_print("Hooks setup complete") print("\n" + "=" * 60) @@ -1160,12 +1160,16 @@ def main(): print("=" * 60) print("\n") + return True + if __name__ == "__main__": try: - main() + ok = main() except KeyboardInterrupt: print("\n\n⚠️ Setup cancelled.") + sys.exit(1) except Exception as e: print(f"\n❌ Error: {e}") - exit(1) + sys.exit(1) + sys.exit(0 if ok else 1) diff --git a/gemini-cli/gateway/mdm/setup.py b/gemini-cli/gateway/mdm/setup.py index 65ad03c2..eed865f1 100644 --- a/gemini-cli/gateway/mdm/setup.py +++ b/gemini-cli/gateway/mdm/setup.py @@ -682,7 +682,7 @@ def main(): if clear_mode: clear_setup() - return + return True print("=" * 60) print("Gemini CLI - MDM Setup") @@ -696,7 +696,7 @@ def main(): ) print("❌ This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False base_url = "https://backend.getunbound.ai" gateway_url = DEFAULT_GATEWAY_URL @@ -727,13 +727,13 @@ def main(): print("\n❌ Missing required argument: --api-key") print("Usage: sudo python3 setup.py --api-key [--backend-url ] [--app_name ] [--debug]") print(" Or: sudo python3 setup.py --clear [--debug]") - return + return False print("\n🔍 Getting device identifier...") device_id = get_device_identifier() if not device_id: print("❌ Failed to get device identifier") - return + return False debug_print(f"Device identifier: {device_id}") print("✅ Device identifier retrieved") @@ -743,20 +743,20 @@ def main(): print("\n🔑 Fetching API key from MDM...") gemini_api_key = fetch_api_key_from_mdm(base_url, app_name, auth_api_key, device_id) if not gemini_api_key: - return + return False print("✅ API key received") print("\n📝 Setting environment variables system-wide...") success, env_changed = set_env_var_system_wide("GEMINI_API_KEY", gemini_api_key) if not success: print(f"❌ Failed to set GEMINI_API_KEY") - return + return False debug_print("GEMINI_API_KEY set successfully") success, url_changed = set_env_var_system_wide("GOOGLE_GEMINI_BASE_URL", f"{gateway_url.rstrip('/')}/v1") if not success: print(f"❌ Failed to set GOOGLE_GEMINI_BASE_URL") - return + return False debug_print("GOOGLE_GEMINI_BASE_URL set successfully") for username, home_dir in get_all_user_homes(): @@ -790,12 +790,16 @@ def main(): notify_setup_complete(gemini_api_key, "gemini-cli", backend_url=base_url, install_state=install_state, serial_number=device_id) + return True + if __name__ == "__main__": try: - main() + ok = main() except KeyboardInterrupt: print("\n\n⚠️ Setup cancelled by user.") + sys.exit(1) except Exception as e: print(f"\n❌ An error occurred: {e}") - exit(1) + sys.exit(1) + sys.exit(0 if ok else 1) From 2838ad03b3d92c41d2dee22c2d13a93721b32418 Mon Sep 17 00:00:00 2001 From: MohamedAklamaash Date: Fri, 3 Jul 2026 12:01:17 +0530 Subject: [PATCH 6/7] Gate Copilot backfill on install success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cursor Bugbot (medium): run_backfill ran whenever --backfill was set, even when hook install failed (success=False). notify_setup_complete is already gated on success; backfill was not, so a failed setup could still upload sessions and advance per-user cutoffs — creating a capture gap since the hooks aren't installed to record going forward. Gate backfill on success too. Co-Authored-By: Claude Opus 4.8 --- copilot/hooks/mdm/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/copilot/hooks/mdm/setup.py b/copilot/hooks/mdm/setup.py index 79facc46..7be30b64 100644 --- a/copilot/hooks/mdm/setup.py +++ b/copilot/hooks/mdm/setup.py @@ -1415,7 +1415,7 @@ def main(): if success: notify_setup_complete(api_key, "copilot", backend_url=base_url, install_state=state, serial_number=device_id) - if backfill_mode: + if success and backfill_mode: run_backfill(api_key, base_url, user_homes) return success From b6b5a0f7e0bb5d27152b43e2206f7d00c67c8f62 Mon Sep 17 00:00:00 2001 From: MohamedAklamaash Date: Fri, 3 Jul 2026 12:24:13 +0530 Subject: [PATCH 7/7] Gate codex-hooks partial install + make --clear teardown truthful MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security consensus review (2 findings): 1. codex hooks: a partial per-user install (e.g. 1 of N users) still returned True, called notify_setup_complete/run_backfill, and the orchestrator would not retry — leaving some users unmonitored. Mirror the Copilot gating: success = bool(user_homes) and installed == len, and gate notify, backfill, and the return value on success. 2. --clear reported success unconditionally (clear_setup(); return True), so a failed teardown still exited 0 and stale hooks/env vars/API-key material could persist undetected during offboarding. clear_setup now returns a bool (False on admin-denied or any teardown failure) and main propagates it. Applied across all 8 MDM setup.py scripts. Co-Authored-By: Claude Opus 4.8 --- augment/hooks/mdm/setup.py | 11 +++++++---- claude-code/gateway/mdm/setup.py | 11 +++++++---- claude-code/hooks/mdm/setup.py | 11 +++++++---- codex/gateway/mdm/setup.py | 11 +++++++---- codex/hooks/mdm/setup.py | 32 +++++++++++++++++++++----------- copilot/hooks/mdm/setup.py | 12 ++++++++---- cursor/mdm/setup.py | 13 +++++++++---- gemini-cli/gateway/mdm/setup.py | 12 ++++++++---- 8 files changed, 74 insertions(+), 39 deletions(-) diff --git a/augment/hooks/mdm/setup.py b/augment/hooks/mdm/setup.py index c0b40b6b..07df1da1 100644 --- a/augment/hooks/mdm/setup.py +++ b/augment/hooks/mdm/setup.py @@ -1184,7 +1184,7 @@ def _is_unbound(cmd: str) -> bool: return "failed" -def clear_setup(): +def clear_setup() -> bool: print("=" * 60) print("Augment Code Hooks - Clearing MDM Setup") print("=" * 60) @@ -1192,8 +1192,9 @@ def clear_setup(): if not check_admin_privileges(): print("This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False + teardown_failed = False print("\nClearing environment variables...") # Windows `reg delete HKLM\...` is machine-wide; fall through with a # placeholder so the removal runs even if C:\Users has no profiles. @@ -1220,6 +1221,7 @@ def clear_setup(): elif not_found: print(f"API_KEY not set, nothing to clear for {not_found} user(s)") if failed: + teardown_failed = True print(f"Failed to clear API_KEY for {failed} user(s)") print("\nClearing managed hooks...") @@ -1230,11 +1232,13 @@ def clear_setup(): elif status == "not_found": print(f"Managed hooks not found in {managed_dir}") else: + teardown_failed = True print(f"Failed to clear managed hooks in {managed_dir}") print("\n" + "=" * 60) print("Clear Complete!") print("=" * 60) + return not teardown_failed def detect_install_state() -> Optional[str]: @@ -1291,8 +1295,7 @@ def main(): print("[backfill] Augment backfill is not supported.") if clear_mode: - clear_setup() - return True + return clear_setup() print("=" * 60) print("Augment Code Hooks - MDM Setup") diff --git a/claude-code/gateway/mdm/setup.py b/claude-code/gateway/mdm/setup.py index aa4eea5a..f4f3f9f9 100644 --- a/claude-code/gateway/mdm/setup.py +++ b/claude-code/gateway/mdm/setup.py @@ -807,7 +807,7 @@ def _clear_env_var_across_users(var_name: str, user_homes, label: str = None) -> return cleared, not_found, failed -def clear_setup(): +def clear_setup() -> bool: print("=" * 60) print("Claude Code - Clearing MDM Setup") print("=" * 60) @@ -815,8 +815,9 @@ def clear_setup(): if not check_admin_privileges(): print("This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False + teardown_failed = False print("\nClearing environment variables...") user_homes = get_all_user_homes() or ([(None, None)] if platform.system().lower() == "windows" else []) @@ -830,6 +831,7 @@ def clear_setup(): elif not (f1 or f2): print(f"API_KEY not set, nothing to clear for {n1} user(s)") if f1 or f2: + teardown_failed = True print(f"Failed to clear for {max(f1, f2)} user(s)") print("\nClearing managed settings...") @@ -840,11 +842,13 @@ def clear_setup(): elif status == "not_found": print(f"Managed settings not found in {managed_dir}") else: + teardown_failed = True print(f"Failed to clear managed settings in {managed_dir}") print("\n" + "=" * 60) print("Clear Complete!") print("=" * 60) + return not teardown_failed def detect_install_state() -> Optional[str]: @@ -904,8 +908,7 @@ def main(): debug_print("Debug mode enabled") if clear_mode: - clear_setup() - return True + return clear_setup() print("=" * 60) print("Claude Code - MDM Setup") diff --git a/claude-code/hooks/mdm/setup.py b/claude-code/hooks/mdm/setup.py index 606f9c84..8d6515bc 100644 --- a/claude-code/hooks/mdm/setup.py +++ b/claude-code/hooks/mdm/setup.py @@ -1117,7 +1117,7 @@ def clear_managed_hooks() -> str: return "failed" -def clear_setup(): +def clear_setup() -> bool: print("=" * 60) print("Claude Code Hooks - Clearing MDM Setup") print("=" * 60) @@ -1125,8 +1125,9 @@ def clear_setup(): if not check_admin_privileges(): print("This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False + teardown_failed = False print("\nClearing environment variables...") # Windows `reg delete HKLM\...` is machine-wide; fall through with a # placeholder so the removal runs even if C:\Users has no profiles. @@ -1153,6 +1154,7 @@ def clear_setup(): elif not_found: print(f"API_KEY not set, nothing to clear for {not_found} user(s)") if failed: + teardown_failed = True print(f"Failed to clear API_KEY for {failed} user(s)") print("\nClearing managed hooks...") @@ -1163,11 +1165,13 @@ def clear_setup(): elif status == "not_found": print(f"Managed hooks not found in {managed_dir}") else: + teardown_failed = True print(f"Failed to clear managed hooks in {managed_dir}") print("\n" + "=" * 60) print("Clear Complete!") print("=" * 60) + return not teardown_failed def _backfill_collect_session(transcript_path: Path) -> Optional[Dict]: @@ -1611,8 +1615,7 @@ def main(): DEBUG = True if clear_mode: - clear_setup() - return True + return clear_setup() print("=" * 60) print("Claude Code Hooks - MDM Setup") diff --git a/codex/gateway/mdm/setup.py b/codex/gateway/mdm/setup.py index 9c1056f5..a2a36e8e 100644 --- a/codex/gateway/mdm/setup.py +++ b/codex/gateway/mdm/setup.py @@ -671,7 +671,7 @@ def _clear_env_var_across_users(var_name: str, user_homes, label: str = None) -> return cleared, not_found, failed -def clear_setup(): +def clear_setup() -> bool: print("=" * 60) print("Codex - Clearing MDM Setup") print("=" * 60) @@ -679,8 +679,9 @@ def clear_setup(): if not check_admin_privileges(): print("This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False + teardown_failed = False print("\nClearing environment variables...") user_homes = get_all_user_homes() or ([(None, None)] if platform.system().lower() == "windows" else []) @@ -694,15 +695,18 @@ def clear_setup(): elif not (f1 or f2): print(f"API_KEY not set, nothing to clear for {n1} user(s)") if f1 or f2: + teardown_failed = True print(f"Failed to clear for {max(f1, f2)} user(s)") for username, home_dir in user_homes: if home_dir is not None: if not remove_codex_config_base_url_for_user(username, home_dir): + teardown_failed = True debug_print(f"Could not remove openai_base_url from codex config for {username}") print("\n" + "=" * 60) print("Clear Complete!") print("=" * 60) + return not teardown_failed def remove_hooks_unbound_script_for_user(username: str, home_dir: Path) -> None: @@ -867,8 +871,7 @@ def main(): debug_print("Debug mode enabled") if clear_mode: - clear_setup() - return True + return clear_setup() print("=" * 60) print("Codex - MDM Setup") diff --git a/codex/hooks/mdm/setup.py b/codex/hooks/mdm/setup.py index c89ef698..99e628ed 100644 --- a/codex/hooks/mdm/setup.py +++ b/codex/hooks/mdm/setup.py @@ -1248,7 +1248,7 @@ def _disable(): debug_print(f"Removed codex_hooks feature for {username}") -def clear_setup(): +def clear_setup() -> bool: print("=" * 60) print("Codex Hooks - Clearing MDM Setup") print("=" * 60) @@ -1256,7 +1256,9 @@ def clear_setup(): if not check_admin_privileges(): print("This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False + + teardown_failed = False print("\nClearing environment variables...") # Windows `reg delete HKLM\...` is machine-wide; fall through with a @@ -1287,6 +1289,7 @@ def clear_setup(): elif not_found: print(f"API_KEY not set, nothing to clear for {not_found} user(s)") if failed: + teardown_failed = True print(f"Failed to clear API_KEY for {failed} user(s)") print("\nClearing managed hooks...") @@ -1297,12 +1300,15 @@ def clear_setup(): elif status == "not_found": print(f"Managed hooks not found in {managed_dir}") else: + teardown_failed = True print(f"Failed to clear managed hooks in {managed_dir}") print("\n" + "=" * 60) print("Clear Complete!") print("=" * 60) + return not teardown_failed + def _backfill_session_id_from_filename(transcript_path: Path) -> Optional[str]: # Only `rollout--.jsonl` — never a generic stem fallback. @@ -1758,8 +1764,7 @@ def main(): DEBUG = True if clear_mode: - clear_setup() - return True + return clear_setup() print("=" * 60) print("Codex Hooks - MDM Setup") @@ -1856,20 +1861,25 @@ def main(): print(f"Registered codex hooks for {username}") installed += 1 - if user_homes and installed == 0: - print("Failed to configure codex hooks for any user") - return False + success = bool(user_homes) and installed == len(user_homes) + if not user_homes: + print("No user home directories found") + elif success: + print(f"Registered codex hooks for {installed} user(s)") + else: + print(f"Registered codex hooks for {installed} of {len(user_homes)} user(s) — {len(user_homes) - installed} failed") print("\n" + "=" * 60) - print("Setup Complete!") + print("Setup Complete!" if success else "Setup Failed") print("=" * 60) - notify_setup_complete(api_key, "codex", backend_url=base_url, install_state=state, serial_number=device_id) + if success: + notify_setup_complete(api_key, "codex", backend_url=base_url, install_state=state, serial_number=device_id) - if backfill_mode: + if success and backfill_mode: run_backfill(api_key, base_url, get_all_user_homes()) - return True + return success if __name__ == "__main__": diff --git a/copilot/hooks/mdm/setup.py b/copilot/hooks/mdm/setup.py index 7be30b64..7ff2f969 100644 --- a/copilot/hooks/mdm/setup.py +++ b/copilot/hooks/mdm/setup.py @@ -1239,7 +1239,7 @@ def notify_setup_complete(api_key: str, tool_type: str, backend_url: str = "http debug_print(f"Could not notify backend: {e}") -def clear_setup(): +def clear_setup() -> bool: print("=" * 60) print("Copilot Hooks - Clearing MDM Setup") print("=" * 60) @@ -1247,7 +1247,9 @@ def clear_setup(): if not check_admin_privileges(): print("This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False + + teardown_failed = False print("\nClearing environment variables...") # Windows `reg delete HKLM\...` is machine-wide; fall through with a @@ -1286,16 +1288,19 @@ def clear_setup(): elif env_not_found: print(f"API_KEY not set, nothing to clear for {env_not_found} user(s)") if env_failed: + teardown_failed = True print(f"Failed to clear API_KEY for {env_failed} user(s)") if hooks_cleared: print(f"Cleared Copilot hooks for {hooks_cleared} user(s)") if hooks_failed: + teardown_failed = True print(f"Failed to clear Copilot hooks for {hooks_failed} user(s)") print("\n" + "=" * 60) print("Clear Complete!") print("=" * 60) + return not teardown_failed def main(): @@ -1307,8 +1312,7 @@ def main(): DEBUG = True if clear_mode: - clear_setup() - return True + return clear_setup() print("=" * 60) print("Copilot Hooks - MDM Setup") diff --git a/cursor/mdm/setup.py b/cursor/mdm/setup.py index 67719998..9a89bf38 100644 --- a/cursor/mdm/setup.py +++ b/cursor/mdm/setup.py @@ -867,7 +867,7 @@ def restart_cursor() -> bool: return False -def clear_setup(): +def clear_setup() -> bool: """Remove hooks and environment variables set by the setup script.""" print("=" * 60) print("Unbound Cursor Hooks - Clearing Setup") @@ -877,8 +877,9 @@ def clear_setup(): if not check_admin_privileges(): print("❌ This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False + teardown_failed = False # Remove enterprise hooks files (NOT the entire Cursor directory) print("\nClearing enterprise hooks...") enterprise_dir = get_enterprise_hooks_dir() @@ -891,6 +892,7 @@ def clear_setup(): hooks_json.unlink() print("Cleared enterprise hooks.json") except Exception as e: + teardown_failed = True print(f"Failed to clear hooks.json: {e}") # Remove hooks directory @@ -900,6 +902,7 @@ def clear_setup(): shutil.rmtree(hooks_dir) print("Cleared enterprise hooks directory") except Exception as e: + teardown_failed = True print(f"Failed to clear hooks directory: {e}") # Remove environment variable from all users @@ -912,6 +915,7 @@ def clear_setup(): elif status == "not_found": print("API_KEY not set, nothing to clear") else: + teardown_failed = True print("Failed to clear API_KEY") else: user_homes = get_all_user_homes() @@ -936,6 +940,7 @@ def clear_setup(): elif not_found: print(f"API_KEY not set, nothing to clear for {not_found} user(s)") if failed: + teardown_failed = True print(f"Failed to clear API_KEY for {failed} user(s)") # Per-user hook logs live in ~/.cursor/hooks on every platform; remove them @@ -954,6 +959,7 @@ def clear_setup(): print("=" * 60) print("\nNote: Restart your terminal or log out/in for env var changes to take effect") print("=" * 60) + return not teardown_failed def fetch_api_key_from_mdm(base_url: str, app_name: str, auth_api_key: str, serial_number: str) -> str: @@ -1071,8 +1077,7 @@ def main(): # If clear mode, run cleanup and exit if clear_mode: - clear_setup() - return True + return clear_setup() print("=" * 60) print("Unbound Cursor Hooks - MDM Setup") diff --git a/gemini-cli/gateway/mdm/setup.py b/gemini-cli/gateway/mdm/setup.py index eed865f1..53e483a5 100644 --- a/gemini-cli/gateway/mdm/setup.py +++ b/gemini-cli/gateway/mdm/setup.py @@ -573,7 +573,7 @@ def _clear_env_var_across_users(var_name: str, user_homes, label: str = None) -> return cleared, not_found, failed -def clear_setup(): +def clear_setup() -> bool: print("=" * 60) print("Gemini CLI - Clearing MDM Setup") print("=" * 60) @@ -581,8 +581,9 @@ def clear_setup(): if not check_admin_privileges(): print("This script requires administrator/root privileges") print(" Please re-run with sudo.") - return + return False + teardown_failed = False print("\nClearing environment variables...") # Windows `reg delete HKLM\...` is machine-wide; fall through with a # placeholder so the removal runs even if C:\Users has no profiles. @@ -598,6 +599,7 @@ def clear_setup(): elif not (f1 or f2): print(f"API_KEY not set, nothing to clear for {n1} user(s)") if f1 or f2: + teardown_failed = True print(f"Failed to clear for {max(f1, f2)} user(s)") if platform.system().lower() == "windows": @@ -610,6 +612,7 @@ def clear_setup(): settings_path.unlink() debug_print(f"Removed {settings_path}") except Exception as e: + teardown_failed = True print(f"Failed to clear managed settings file: {e}") if settings_dir.exists(): try: @@ -624,11 +627,13 @@ def clear_setup(): if status == "cleared": print("Cleared managed settings path") elif status == "failed": + teardown_failed = True print("Failed to clear managed settings path") print("\n" + "=" * 60) print("Clear Complete!") print("=" * 60) + return not teardown_failed def detect_install_state() -> Optional[str]: @@ -681,8 +686,7 @@ def main(): debug_print("Debug mode enabled") if clear_mode: - clear_setup() - return True + return clear_setup() print("=" * 60) print("Gemini CLI - MDM Setup")