diff --git a/CHANGELOG.md b/CHANGELOG.md index c63106c075ff..7752e4eb0483 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,8 @@ - **fix(oauth):** prevent cross-IdP account overwrites by disambiguating OAuth connections on `username` when present, not email alone. (thanks @KunN-21) +- **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) + - **providers (Kiro — Claude Sonnet 5):** the Kiro provider's model catalog was missing `claude-sonnet-5`, so the model could not be selected or routed even on accounts that already had access to it ("claude-sonnet-5 is not supported"). Added the model to the Kiro registry (`open-sse/config/providers/registry/kiro/index.ts`) as a 1M-context / 128K-output Claude model, mirroring the existing Claude entries; the registry `models[]` feeds both the model selector and the live CodeWhisperer `ListAvailableModels` fallback, so the model is now selectable and routable. Regression guard: `tests/unit/kiro-claude-sonnet-5-2267.test.ts`. (thanks [@openbioinfo](https://github.com/openbioinfo)) - **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. Follow-up: the root cause is now also fixed — the `_customAliases` store in `modelDeprecation.ts` is backed by `globalThis` (key `__omniroute_customAliases__`), so the startup and app-route module graphs share **one** store and the route reads the boot-hydrated aliases directly (the DB self-heal remains as a harmless fallback), mirroring the same `globalThis` singleton pattern already applied to `thinkingBudget.ts`/`backgroundTaskDetector.ts` (#5312). Regression guards: `tests/unit/model-aliases-settings-route-selfheal.test.ts` + `tests/unit/model-aliases-globalthis-5777.test.ts`. ([#5777](https://github.com/diegosouzapw/OmniRoute/pull/5777) — thanks [@jleonar2](https://github.com/jleonar2)) diff --git a/src/mitm/manager.ts b/src/mitm/manager.ts index fde526a2f77b..ea8cbe48766c 100644 --- a/src/mitm/manager.ts +++ b/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; + removeDNSEntries?: (hosts: string[], sudoPassword: string) => Promise; + collectManagedHosts?: () => string[]; + } +): Promise { + 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 */ diff --git a/tests/unit/mitm-hosts-cleanup-on-exit.test.ts b/tests/unit/mitm-hosts-cleanup-on-exit.test.ts new file mode 100644 index 000000000000..f39ac8857486 --- /dev/null +++ b/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" + ); +});