mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-26 17:12:27 +03:00
* fix(cli): remove MITM DNS spoof entries before killing server process (port from 9router#1809) stopMitm() killed the spawned MITM server process first and only removed the /etc/hosts DNS-spoof entries afterward. During that window any client whose DNS still resolved a target host to 127.0.0.1 but whose MITM listener was already dead got connect ECONNREFUSED 127.0.0.1:443 — exactly the community-confirmed workaround (stop DNS before stopping the server) proves. Swap the two steps so DNS is always cleared first, mirroring the ordering already used by repairMitm() and handleExitCleanup(). Reported-by: dionisius95 (https://github.com/decolua/9router/issues/1809) * refactor(mitm): extract repair planning out of manager to respect the file-size cap The #1809 DNS-before-kill ordering fix pushed src/mitm/manager.ts to 813 lines, over the 800-line cap check:file-size enforces for non-frozen files. Move the pure repair-planning pieces (collectManagedHosts, the RepairPlan shape and its filesystem/cert/DNS sweep) into a sibling src/mitm/repair.ts. The in-memory session bookkeeping repairMitm() owns — cached sudo password, orphaned flag, PID file — deliberately stays in manager.ts, so the seam is "plan the repair" vs "own the session". manager.ts is now 731 lines; behavior is unchanged. The DNS-first ordering fix and its regression guard (tests/unit/mitm-stop-dns-before-kill-1809.ts) are untouched and still pass. * fix(mitm): split stopMitm() DNS/kill steps to fix complexity ratchet regression stopMitm()'s new DNS-before-kill ordering (#1809) pushed its cyclomatic complexity to 18 (max 15), regressing the complexity ratchet from 2056 to 2057. Extract the DNS-removal step and the process-kill step (in-memory + PID-file fallback) into two private helpers, mirroring the existing performRepairSteps() extraction pattern in repair.ts. Behavior unchanged; complexity back at 2056 (cognitive-complexity drops to 889, one under baseline).
83 lines
3.1 KiB
TypeScript
83 lines
3.1 KiB
TypeScript
/**
|
|
* Regression test for upstream issue #1809: "connect ECONNREFUSED 127.0.0.1:443"
|
|
* after stopping the MITM proxy.
|
|
*
|
|
* Root cause: stopMitm() killed the spawned MITM server process FIRST, and only
|
|
* removed the /etc/hosts DNS-spoof entries AFTER. During that window any client
|
|
* whose DNS still resolved the target host to 127.0.0.1 (from startMitm's spoof)
|
|
* but whose MITM listener was already dead got ECONNREFUSED — exactly the
|
|
* community-confirmed workaround ("stop DNS before stopping the server") proves.
|
|
*
|
|
* This test drives stopMitm() with real DI: a fake serverProcess standing in for
|
|
* the spawned MITM child, and dependency-injected DNS-removal functions that
|
|
* record the order in which they are invoked relative to the process kill. The
|
|
* fix must remove DNS entries before killing the server process so no window
|
|
* exists where DNS points at 127.0.0.1 with nothing listening there.
|
|
*
|
|
* Uses the project's DATA_DIR-tmp + resetDbInstance pattern so the Node native
|
|
* test runner does not hang 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";
|
|
import { EventEmitter } from "node:events";
|
|
|
|
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-mitm-stop-order-"));
|
|
process.env.DATA_DIR = TEST_DATA_DIR;
|
|
|
|
const core = await import("../../src/lib/db/core.ts");
|
|
const manager = await import("../../src/mitm/manager.ts");
|
|
|
|
test.after(() => {
|
|
core.resetDbInstance();
|
|
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
|
});
|
|
|
|
test("stopMitm removes DNS entries before killing the MITM server process (#1809)", async () => {
|
|
const events: string[] = [];
|
|
|
|
// Fake child process standing in for the spawned MITM server.
|
|
const fakeProc = new EventEmitter() as EventEmitter & {
|
|
killed: boolean;
|
|
kill: (signal?: string) => boolean;
|
|
};
|
|
fakeProc.killed = false;
|
|
fakeProc.kill = (signal?: string) => {
|
|
events.push(`kill:${signal}`);
|
|
fakeProc.killed = true;
|
|
return true;
|
|
};
|
|
|
|
manager.__setServerProcessForTest(fakeProc as unknown as import("child_process").ChildProcess, 4242);
|
|
|
|
const removeDNSEntry = async () => {
|
|
events.push("removeDNSEntry");
|
|
};
|
|
const removeDNSEntries = async () => {
|
|
events.push("removeDNSEntries");
|
|
};
|
|
const collectManagedHosts = () => ["fake.example.test"];
|
|
|
|
await manager.stopMitm("fake-sudo-password", {
|
|
removeDNSEntry,
|
|
removeDNSEntries,
|
|
collectManagedHosts,
|
|
});
|
|
|
|
const firstKillIndex = events.findIndex((e) => e.startsWith("kill:"));
|
|
const firstDnsIndex = events.findIndex(
|
|
(e) => e === "removeDNSEntry" || e === "removeDNSEntries"
|
|
);
|
|
|
|
assert.ok(firstKillIndex !== -1, "server process kill was never invoked");
|
|
assert.ok(firstDnsIndex !== -1, "DNS removal was never invoked");
|
|
assert.ok(
|
|
firstDnsIndex < firstKillIndex,
|
|
`DNS entries must be removed BEFORE the MITM server process is killed ` +
|
|
`(got order: ${JSON.stringify(events)}) — otherwise a client whose DNS still ` +
|
|
`points at 127.0.0.1 hits a dead listener and gets ECONNREFUSED (#1809)`
|
|
);
|
|
});
|