fix(mitm): clean up privileged hosts entries on exit when possible

[diegosouzapw]

Summary

• installCleanupHandlers() only killed the spawned MITM child process on SIGINT/SIGTERM and always left the privileged /etc/hosts entries in place, requiring a manual dashboard Repair — even when a sudo password was already cached for the current session.
• The signal handler now best-effort reverts every managed /etc/hosts entry when a cached password is available, and falls back to the existing orphaned-state flag (manual Repair) when it is not.

Changes

• Extracted handleExitCleanup(signal, depsOverride?) from installCleanupHandlers() in src/mitm/manager.ts so the shutdown behavior is directly unit-testable without sending a real OS signal to the test process.
• On SIGINT/SIGTERM, after terminating the spawned MITM child:
  • If getCachedPassword() returns a cached sudo password, best-effort calls the same removeDNSEntry() + removeDNSEntries(collectManagedHosts()) pair stopMitm() already uses, reverting every managed host.
  • If no password is cached, falls back to flagging _orphanedStateDetected exactly as before (dashboard Repair still available).
• Uses the existing sudo -S / execFileWithPassword path (src/mitm/systemCommands.ts) — not a direct fs.writeFileSync on /etc/hosts — so Hard Rule deps: bump qs from 6.14.1 to 6.14.2 #13 (no shell string-interpolation) and the project's existing privilege model are preserved.

Attribution

Thanks to @manhdzzz for the original implementation.

Testing

• New regression test: tests/unit/mitm-hosts-cleanup-on-exit.test.ts
  • With a cached password: asserts removeDNSEntry/removeDNSEntries are invoked with the cached password and the managed host set, and orphanedStateDetected stays false.
  • With no cached password: asserts removeDNSEntry/removeDNSEntries are not invoked and orphanedStateDetected becomes true.
• node --import tsx/esm --test tests/unit/mitm-hosts-cleanup-on-exit.test.ts — 2/2 passing.
• Full existing MITM suite re-run for regressions (38/38 passing): mitm-manager-repair, mitm-manager-cleanup-symmetry, mitm-diagnostics, mitm-sudo-gate-822, mitm-sudo-graceful-degrade, mitm-upstream-ca-wiring, mitm-system-commands-prefix-3641, dns-config-generic.
• npm run typecheck:core — clean.
• npx eslint src/mitm/manager.ts tests/unit/mitm-hosts-cleanup-on-exit.test.ts — clean.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[gemini-code-assist]

Code Review

This pull request extracts the exit-cleanup logic from installCleanupHandlers into a testable handleExitCleanup function and adds corresponding unit tests to verify hosts cleanup behavior. Feedback on these changes highlights two key issues: first, because handleExitCleanup is asynchronous, calling it synchronously inside the signal handler can lead to the process exiting before cleanup completes or hanging; second, setting the in-memory _orphanedStateDetected flag right before process exit is a no-op since the state is lost upon termination.

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]

Since handleExitCleanup is asynchronous, simply calling void handleExitCleanup(signal) means the cleanup process runs concurrently with the process exit sequence. If there are other signal listeners that call process.exit() synchronously, the process will terminate before the asynchronous cleanup (which spawns a child process to revert /etc/hosts) can complete. Conversely, if there are no other listeners and the application has persistent handles (like an HTTP server or database pool), registering a signal listener without calling process.exit() or re-sending the signal will cause the process to hang indefinitely on Ctrl+C.

By making onSignal asynchronous, awaiting handleExitCleanup, and then re-sending the signal to the process if no other listeners remain, we ensure that:

1. The cleanup is fully completed before the default exit behavior is triggered.
2. The process does not hang when there are no other signal listeners.
3. The correct exit status/signal is preserved.

  const onSignal = async (signal: string) => {
    await handleExitCleanup(signal);
    if (process.listenerCount(signal) === 0) {
      process.kill(process.pid, signal);
    }
  };

[gemini-code-assist]

Setting the in-memory _orphanedStateDetected = true right before the process exits is a no-op because this state is lost when the process terminates. On the next application startup, _orphanedStateDetected will reset to false.

Furthermore, if the child process exits cleanly, it deletes the PID_FILE, meaning the stale PID file check in getMitmStatus() won't detect the orphaned state either, even though /etc/hosts remains spoofed.

To fix this robustly, consider updating getMitmStatus() (outside this diff) to also flag orphanedStateDetected if !running && dnsConfigured is true. This naturally detects orphaned DNS entries across restarts without relying on transient in-memory state or the presence of a stale PID file.