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

CHANGELOG.md

@@ -32,6 +32,8 @@
 
 ### 🔧 Bug Fixes
 
+- **fix(mitm):** best-effort revert privileged /etc/hosts entries on exit when a sudo password is cached, instead of always leaving orphaned state. (thanks @manhdzzz)
+
 - **settings (model aliases — self-heal after restart):** the Settings → Routing page showed "No exact-match aliases configured" after a server restart even though the aliases were persisted in the DB. Aliases are held in a module-local `_customAliases` map in `modelDeprecation.ts` that the boot path hydrates, but Next.js compiles the app-route module graph separately from the startup graph (the same webpack chunk-splitting class as #5312), so the `GET /api/settings/model-aliases` handler read a different, un-hydrated copy. The handler now self-heals: when its in-memory alias map is empty it reads `settings.modelAliases` from the DB (via the existing `getSettings()` db module — no raw SQL in the route) and repopulates the map, so the UI reflects the persisted aliases on the first GET after a restart. Regression guard: `tests/unit/model-aliases-settings-route-selfheal.test.ts`. ([#5777](https://github.com/diegosouzapw/OmniRoute/pull/5777) — thanks [@jleonar2](https://github.com/jleonar2))
 
 - **providers (grok-cli token auto-refresh):** grok-cli OAuth tokens were never proactively refreshed before their real expiry. `mapTokens` hardcoded `expiresIn: 21600` (6 h) regardless of the token's actual lifetime, so the persisted `expiresAt` was always "now + 6 h" and the proactive `tokenHealthCheck` sweep (refresh when `expiresAt - now < 5 min`) fired 6 h after import instead of shortly before the token really expired. `mapTokens` now computes `expiresIn` from the authoritative `expires_at` field in `~/.grok/auth.json` (ISO → epoch-seconds) with a fallback to the JWT `exp` claim (payload-only decode, no signature trust); the hardcoded `21600` is kept only when neither is present. Regression guards: 3 new cases in `tests/unit/grok-cli-oauth.test.ts` (JWT `exp`, JSON `expires_at`, and the fallback). ([#5775](https://github.com/diegosouzapw/OmniRoute/pull/5775) — thanks [@Chewji9875](https://github.com/Chewji9875))

src/mitm/manager.ts

@@ -309,28 +309,88 @@ export async function repairMitm(sudoPassword: string): Promise<{ repaired: stri
 
 /**
  * Best-effort JS surrogate for ProxyBridge's library destructor + crash signal
- * handler. On SIGINT/SIGTERM we terminate the spawned child and warn that
- * privileged cleanup (DNS/CA/proxy) still requires a Repair — we have no sudo
- * password in a signal handler. Idempotent; never blocks process exit. (Gap 7.)
+ * handler. On SIGINT/SIGTERM we terminate the spawned child and, when a sudo
+ * password is already cached for this session (getCachedPassword()), also
+ * best-effort revert the privileged /etc/hosts entries — so a clean Ctrl+C /
+ * tray-quit does not always leave orphaned state for a manual Repair. Without
+ * a cached password we cannot prompt for one inside a signal handler, so we
+ * fall back to flagging `_orphanedStateDetected`, exactly as before.
+ * Idempotent; never blocks process exit. (Gap 7.)
  */
 export function installCleanupHandlers(): void {
   if (_cleanupHandlersInstalled) return;
   _cleanupHandlersInstalled = true;
   const onSignal = (signal: string) => {
-    try {
-      if (serverProcess && !serverProcess.killed) serverProcess.kill("SIGTERM");
-    } catch {
-      // ignore
-    }
-    log.warn(
-      { signal },
-      "MITM parent received signal — child terminated; run Repair if DNS/CA/proxy were applied."
-    );
+    void handleExitCleanup(signal);
   };
   process.once("SIGINT", () => onSignal("SIGINT"));
   process.once("SIGTERM", () => onSignal("SIGTERM"));
 }
 
+/**
+ * Exit-cleanup body extracted from installCleanupHandlers() so it is directly
+ * unit-testable (no real OS signal needs to be delivered to the test
+ * process). Terminates the spawned MITM child, then:
+ *   - if a sudo password is cached in this session, best-effort reverts every
+ *     managed /etc/hosts entry via the same `removeDNSEntry` +
+ *     `removeDNSEntries(collectManagedHosts())` pair stopMitm() uses, so the
+ *     hosts file does not stay spoofed after a clean signal-driven exit;
+ *   - otherwise flags `_orphanedStateDetected` (surfaced by getMitmStatus()
+ *     for the dashboard's one-click Repair) — we have no way to prompt for a
+ *     password inside a signal handler. (Gap 7.)
+ * @param _depsOverride - optional dependency override, used in tests for DI.
+ */
+export async function handleExitCleanup(
+  signal: string,
+  _depsOverride?: {
+    getCachedPassword?: () => string | null;
+    removeDNSEntry?: (sudoPassword: string) => Promise<void>;
+    removeDNSEntries?: (hosts: string[], sudoPassword: string) => Promise<void>;
+    collectManagedHosts?: () => string[];
+  }
+): Promise<void> {
+  const deps = {
+    getCachedPassword: _depsOverride?.getCachedPassword ?? getCachedPassword,
+    removeDNSEntry: _depsOverride?.removeDNSEntry ?? removeDNSEntry,
+    removeDNSEntries: _depsOverride?.removeDNSEntries ?? removeDNSEntries,
+    collectManagedHosts: _depsOverride?.collectManagedHosts ?? collectManagedHosts,
+  };
+
+  try {
+    if (serverProcess && !serverProcess.killed) serverProcess.kill("SIGTERM");
+  } catch {
+    // ignore
+  }
+
+  const sudoPassword = deps.getCachedPassword();
+  if (!sudoPassword) {
+    _orphanedStateDetected = true;
+    log.warn(
+      { signal },
+      "MITM parent received signal — child terminated; no cached sudo password, run Repair if DNS/CA/proxy were applied."
+    );
+    return;
+  }
+
+  try {
+    await deps.removeDNSEntry(sudoPassword);
+    const managed = deps.collectManagedHosts();
+    if (managed.length > 0) {
+      await deps.removeDNSEntries(managed, sudoPassword);
+    }
+    log.info(
+      { signal },
+      "MITM parent received signal — child terminated and privileged /etc/hosts entries reverted."
+    );
+  } catch (err) {
+    _orphanedStateDetected = true;
+    log.error(
+      { err, signal },
+      "MITM parent received signal — hosts cleanup failed; run Repair if DNS/CA/proxy were applied."
+    );
+  }
+}
+
 /**
  * Get MITM status
  */

tests/unit/mitm-hosts-cleanup-on-exit.test.ts

@@ -0,0 +1,134 @@
+/**
+ * Gap 7 follow-up: installCleanupHandlers() previously only killed the spawned
+ * MITM child on SIGINT/SIGTERM and always left `/etc/hosts` state for a manual
+ * Repair, even when a sudo password was already cached in this session
+ * (getCachedPassword()/setCachedPassword()). handleExitCleanup() — extracted
+ * from the signal handler so it is directly unit-testable without sending a
+ * real OS signal to the test process — now best-effort reverts every managed
+ * host (collectManagedHosts(), mirroring stopMitm()'s DNS teardown) when a
+ * password is cached, and falls back to flagging `_orphanedStateDetected`
+ * (surfaced via getMitmStatus().orphanedStateDetected) exactly as before when
+ * no password is available.
+ *
+ * DATA_DIR-tmp + resetDbInstance pattern prevents the Node test runner from
+ * hanging on open SQLite handles (CLAUDE.md PII learning #3).
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-mitm-exit-cleanup-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const core = await import("../../src/lib/db/core.ts");
+const manager = await import("../../src/mitm/manager.ts");
+
+async function resetStorage() {
+  core.resetDbInstance();
+  for (let attempt = 0; attempt < 10; attempt++) {
+    try {
+      if (fs.existsSync(TEST_DATA_DIR)) {
+        fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+      }
+      break;
+    } catch (error: unknown) {
+      const code = (error as { code?: string } | null)?.code;
+      if ((code === "EBUSY" || code === "EPERM") && attempt < 9) {
+        await new Promise((resolve) => setTimeout(resolve, 50 * (attempt + 1)));
+      } else {
+        throw error;
+      }
+    }
+  }
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+}
+
+test.beforeEach(async () => {
+  await resetStorage();
+  manager.clearCachedPassword();
+});
+
+test.after(async () => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+test("handleExitCleanup: with a cached sudo password, best-effort reverts managed /etc/hosts entries", async () => {
+  manager.setCachedPassword("s3cr3t-session-password");
+
+  const removeDNSEntryCalls: string[] = [];
+  const removeDNSEntriesCalls: Array<{ hosts: string[]; sudoPassword: string }> = [];
+  const managedHosts = ["daily-cloudcode-pa.googleapis.com", "api.individual.githubcopilot.com"];
+
+  await manager.handleExitCleanup("SIGINT", {
+    removeDNSEntry: async (sudoPassword: string) => {
+      removeDNSEntryCalls.push(sudoPassword);
+    },
+    removeDNSEntries: async (hosts: string[], sudoPassword: string) => {
+      removeDNSEntriesCalls.push({ hosts, sudoPassword });
+    },
+    collectManagedHosts: () => managedHosts,
+  });
+
+  assert.deepEqual(
+    removeDNSEntryCalls,
+    ["s3cr3t-session-password"],
+    "must revert the legacy Antigravity DNS entries with the cached password"
+  );
+  assert.equal(removeDNSEntriesCalls.length, 1, "must revert the managed host set exactly once");
+  assert.deepEqual(
+    removeDNSEntriesCalls[0].hosts,
+    managedHosts,
+    "must pass collectManagedHosts() output through to removeDNSEntries"
+  );
+  assert.equal(
+    removeDNSEntriesCalls[0].sudoPassword,
+    "s3cr3t-session-password",
+    "must forward the cached sudo password to removeDNSEntries"
+  );
+
+  const status = await manager.getMitmStatus();
+  assert.equal(
+    status.orphanedStateDetected,
+    false,
+    "a successful hosts revert must not flag orphaned state"
+  );
+});
+
+test("handleExitCleanup: with NO cached password, falls back to orphaned-state flag and skips DNS removal", async () => {
+  manager.clearCachedPassword();
+  assert.equal(manager.getCachedPassword(), null, "precondition: no password cached in this session");
+
+  let removeDNSEntryCalled = false;
+  let removeDNSEntriesCalled = false;
+
+  await manager.handleExitCleanup("SIGTERM", {
+    removeDNSEntry: async () => {
+      removeDNSEntryCalled = true;
+    },
+    removeDNSEntries: async () => {
+      removeDNSEntriesCalled = true;
+    },
+    collectManagedHosts: () => ["should-not-be-used.invalid"],
+  });
+
+  assert.equal(
+    removeDNSEntryCalled,
+    false,
+    "must NOT attempt privileged DNS removal without a cached password"
+  );
+  assert.equal(
+    removeDNSEntriesCalled,
+    false,
+    "must NOT attempt privileged DNS removal without a cached password"
+  );
+
+  const status = await manager.getMitmStatus();
+  assert.equal(
+    status.orphanedStateDetected,
+    true,
+    "must fall back to the orphaned-state flag for a manual Repair when no password is cached"
+  );
+});