Fix silent Copilot MDM hook-install failure

[MohamedAklamaash]

Summary

• GitHub Copilot was the only MDM tool that installed unbound.py per-user by downloading it to a root-owned staging dir and shutil.copyfile-ing it into each home after privilege-drop. The dropped user had to read the staged file; when it couldn't (swallowed chmod widening, or a non-traversable /var/folders temp path on macOS under sudo), the copy failed silently and main() still exited 0 — so the orchestrator (mdm/onboard.py) marked Copilot installed when no hook was written.
• Root cause fix: read the hook script once as root into memory, then write it as the target user — the only cross-privilege op is now a write into the user's own home (no staged read). This matches how claude-code/codex/cursor already behave and removes a root-owned, world-readable file from disk entirely.
• Exit-code fix: main() now returns a bool and __main__ exits non-zero when no/partial user install succeeds, so a silent failure can no longer be reported as success.

Changes

• New modular helpers in copilot/hooks/mdm/setup.py:
  • _fetch_hook_script(gateway_url) — root-only download into an unpredictable mkdtemp dir, read into memory with the gateway URL applied, cleaned up in finally.
  • _apply_gateway_url(text, gateway_url) — pure string rewrite (replaces the old path-based rewrite_gateway_url_in_file).
  • _write_file(path, data, mode) — O_NOFOLLOW open + fchmod (umask-proof), reused for both unbound.py and unbound.json.
• install_hooks_for_user(username, home_dir, script_text) — writes both files via _write_file inside _run_as_user; no more shutil.copyfile from a staged path.
• Dropped the staging-dir 0o711/0o644 widening (dead once nothing reads the file post-drop).
• notify_setup_complete is only called on full success, so the backend isn't told setup completed on a failed run.

Test plan

• New integration tests at the outermost layer (install_hooks_for_user, main()): files actually land + are executable; O_NOFOLLOW refuses a symlinked path; gateway rewrite vs identity; main() returns False / exits non-zero on no-users, partial-failure, and download-failure, and notifies only on success.
• Full copilot suite green: 26 passed (9 new + 17 existing).
• Reviewed by elite-pr-reviewer (APPROVE) and /security-review (no High/Critical; net-strengthens the root↔user boundary).

Notes

• Out of scope (flagged): mdm/onboard.py:89 passes supports_backfill=True for Copilot while the comment at :82-85 says Copilot has no backfill — harmless, left as-is.

🤖 Generated with Claude Code

---

Note

Medium Risk
Touches privileged MDM install and orchestrator exit semantics; changes narrow the root→user file boundary but alter production onboarding failure signaling.

Overview
Fixes silent Copilot MDM hook install failures by changing how unbound.py reaches each user profile and how setup reports success.

Install path: The hook is now downloaded once as root via _fetch_hook_script, gateway URL applied in memory (_apply_gateway_url), and install_hooks_for_user writes script_text as the target user with _write_file (O_NOFOLLOW + fchmod) instead of shutil.copyfile from a root-owned staging file the dropped user might not read. Staging-dir permission widening is removed.

Success contract: main() returns a bool on every path; __main__ exits non-zero when install is missing, partial, or download fails. notify_setup_complete runs only when all discovered users install successfully.

^{Reviewed by Cursor Bugbot for commit e7b2d85. Bugbot is set up for automated code reviews on this repo. Configure here.}

Greptile Summary

This PR replaces Copilot's MDM hook installer's root-owned staging-dir + shutil.copyfile approach with an in-memory read (as root) / per-user write (as target UID) flow, matching how claude-code, codex, and cursor already behave. It also fixes main() to return a meaningful bool so __main__ exits non-zero on any failure and notify_setup_complete is only called on full success.

• _fetch_hook_script downloads unbound.py once as root into a mkdtemp dir, reads it into memory (applying _apply_gateway_url), then wipes the staging dir in finally — eliminating the world-readable root-owned file.
• _write_file uses O_NOFOLLOW | O_CREAT | O_TRUNC and an explicit fchmod to make writes symlink-safe and umask-proof; install_hooks_for_user now calls it inside _run_as_user's privilege-dropped fork, replacing the old shutil.copyfile + os.chmod sequence.
• main() exit-code contract hardened: all early-out paths return False, the loop checks installed_count == len(user_homes), and __main__ calls sys.exit(1) when ok is falsy.

Confidence Score: 4/5

Safe to merge; the refactor correctly moves to in-memory script text + per-user write and the exit-code contract is properly threaded through. One minor write-before-chmod ordering issue in _write_file worth cleaning up but not blocking.

The install flow change is clean and the logic is sound throughout. The one finding — fchmod called before f.write in _write_file, leaving an executable-but-empty file on disk for a brief window — is a real ordering issue but unlikely to bite in practice; it does not affect the correctness of the main bugfix or the exit-code contract.

copilot/hooks/mdm/setup.py — specifically the _write_file helper around the fchmod/write ordering

Important Files Changed

Filename

Overview

copilot/hooks/mdm/setup.py

Core MDM install flow refactored: hook script read into memory as root via _fetch_hook_script, then written per-user as the target user via _write_file (O_NOFOLLOW + explicit fchmod); main() now returns bool and main exits non-zero on any failure; notify_setup_complete guarded by success flag. One minor ordering issue: fchmod is called before f.write in _write_file, leaving a brief window where the file is executable-but-empty.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Root as main() [root]
    participant Fetch as _fetch_hook_script
    participant Staging as mkdtemp staging dir
    participant Fork as _run_as_user fork [uid=user]
    participant Home as ~/.copilot/hooks

    Root->>Fetch: gateway_url
    Fetch->>Staging: download_file(SCRIPT_URL)
    Staging-->>Fetch: unbound.py on disk
    Fetch->>Fetch: read_text() → apply_gateway_url()
    Fetch->>Staging: rmtree (finally)
    Fetch-->>Root: script_text (in memory)

    loop for each (username, home_dir)
        Root->>Fork: install_hooks_for_user(username, home_dir, script_text)
        Fork->>Home: mkdir -p ~/.copilot/hooks
        Fork->>Home: _write_file(unbound.py, script_text, 0o755) [O_NOFOLLOW + fchmod]
        Fork->>Home: _write_file(unbound.json, config, 0o644) [O_NOFOLLOW + fchmod]
        Fork-->>Root: True / None (exit code)
    end

    alt all installs succeeded
        Root->>Root: notify_setup_complete()
        Root-->>Caller: return True → sys.exit(0)
    else partial or no installs
        Root-->>Caller: return False → sys.exit(1)
    end

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Root as main() [root]
    participant Fetch as _fetch_hook_script
    participant Staging as mkdtemp staging dir
    participant Fork as _run_as_user fork [uid=user]
    participant Home as ~/.copilot/hooks

    Root->>Fetch: gateway_url
    Fetch->>Staging: download_file(SCRIPT_URL)
    Staging-->>Fetch: unbound.py on disk
    Fetch->>Fetch: read_text() → apply_gateway_url()
    Fetch->>Staging: rmtree (finally)
    Fetch-->>Root: script_text (in memory)

    loop for each (username, home_dir)
        Root->>Fork: install_hooks_for_user(username, home_dir, script_text)
        Fork->>Home: mkdir -p ~/.copilot/hooks
        Fork->>Home: _write_file(unbound.py, script_text, 0o755) [O_NOFOLLOW + fchmod]
        Fork->>Home: _write_file(unbound.json, config, 0o644) [O_NOFOLLOW + fchmod]
        Fork-->>Root: True / None (exit code)
    end

    alt all installs succeeded
        Root->>Root: notify_setup_complete()
        Root-->>Caller: return True → sys.exit(0)
    else partial or no installs
        Root-->>Caller: return False → sys.exit(1)
    end

Loading

_{Reviews (3): Last reviewed commit: "Remove test_install.py from this PR" | Re-trigger Greptile}

[vigneshsubbiah16]

✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)

---

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head d2c8cc05 · 2026-06-23T07:53Z

[MohamedAklamaash]

Addressed all three Greptile P2s in 5d2e870:

• _fetch_hook_script could raise instead of returning None — now catches a read failure (corrupt/transient), debug_prints the reason, and returns None, so main() prints the clean "Failed to download unbound.py" message (and still exits non-zero) instead of a raw traceback.
• _write_file fd leak if os.fdopen raises — the fd is now closed before re-raising.
• Silent failure reason lost in the fork — _install now debug_prints the underlying error before re-raising, so a per-user install failure is diagnosable in the MDM debug log (which is always on). This directly supports the motivation for this PR.

All checks green; full copilot suite 26 passed.

[vigneshsubbiah16]

✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)

---

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head e7b2d85c · 2026-06-23T10:32Z