fix(cli): per-agent DNS, startup guards, and batched Windows hosts writes (#6338)

DNS toggle in AgentBridge was broken for 8 of 9 agents: addDNSEntry/
removeDNSEntry always resolved the legacy Antigravity default hosts
regardless of which agent's dns_enabled flag was flipped. Both now
accept an optional agentId and resolve hosts via ALL_TARGETS; the
[id]/dns route passes id through and returns 404 for an unknown agent
instead of silently falling back to the defaults.

startMitmInternal() now wraps generateCert(), the provisionDnsEntries()
call, and the PID-file write in try/catch so a mid-startup failure
can't orphan the already-spawned MITM child process.

On Windows, addDNSEntries/removeDNSEntries batch every missing/present
entry into a single elevated PowerShell invocation instead of one UAC
prompt per host line.

Scope note: this PR originally bundled an unrelated SkillOpt feature
(DB migration, 6 API routes, dashboard UI) and a checks-free CI build
workflow alongside this DNS/startup fix. Both were dropped here as
out-of-scope per review-group-prs analysis (2-implementing plan);
only the DNS/startup-guard delta (dnsConfig.ts, manager.ts, the [id]/dns
route, and their tests) is applied.

Co-authored-by: hamsa0x7 <hamsa0x7@users.noreply.github.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
Hamsa_M
2026-07-10 01:14:50 +05:30
committed by GitHub
parent 606d1cbbd3
commit aba98fd227
6 changed files with 214 additions and 72 deletions

View File

@@ -14,6 +14,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral
### 🐛 Bug Fixes
- **fix(cli):** per-agent AgentBridge DNS toggle was broken for 8 of the 9 supported agents, and a failed MITM startup step could orphan the spawned proxy child — `addDNSEntry`/`removeDNSEntry` (`src/mitm/dns/dnsConfig.ts`) always resolved the legacy Antigravity default hosts regardless of which agent's toggle was flipped, so enabling DNS for Cursor/Codex/Claude Code/etc. silently added only `daily-cloudcode-pa.googleapis.com` while the DB recorded `dns_enabled=true` for the selected agent. Both functions now accept an optional `agentId` and resolve hosts via `ALL_TARGETS`; `POST /api/tools/agent-bridge/agents/[id]/dns` passes the route's `id` through and now returns 404 for an id that doesn't match a known target instead of silently falling back. Separately, `startMitmInternal()` (`src/mitm/manager.ts`) now wraps `generateCert()` (log + rethrow), the `provisionDnsEntries()` call, and the PID-file write in try/catch so a mid-startup failure can't orphan the already-spawned MITM child process. On Windows, `addDNSEntries`/`removeDNSEntries` also batch every missing/present entry into a single elevated PowerShell invocation instead of one UAC prompt per host line. Regression guard: `tests/unit/dns-config-generic.test.ts` (agent-specific resolution + batching), `tests/unit/agent-bridge-dns-route-validation.test.ts` (404 for unknown agent id). ([#6338](https://github.com/diegosouzapw/OmniRoute/pull/6338) — thanks @hamsa0x7)
- **fix(auth):** an API key restricted via `allowedModels`/`allowedCombos` could bypass that restriction entirely over the Codex Responses-over-WebSocket bridge ([#6564](https://github.com/diegosouzapw/OmniRoute/issues/6564)) — `prepare()` in `src/app/api/internal/codex-responses-ws/route.ts` authenticated the WS bridge's API key (`authenticate()`/`authorizeWebSocketHandshake()`) and honored `allowedConnections`, but never called `enforceApiKeyPolicy()`, the same model/combo policy gate the HTTP `/v1/responses` path enforces via `handleChat()` — so a key scoped to e.g. `combo/model-1.0` could still reach a direct Codex model like `gpt-5.5` through this transport, as long as an eligible Codex OAuth connection existed. The bridge's WS auth token arrives via query params (`api_key`/`token`/`access_token`), not a normal `Authorization` header, so a new `enforceCodexWsApiKeyPolicy()` builds an equivalent `Request` carrying an explicit `Authorization: Bearer <apiKey>` header and calls `enforceApiKeyPolicy()` against the CLIENT-requested model, before any Codex-specific model remapping or credential selection. Regression guard: `tests/unit/codex-ws-policy-enforcement-6564.test.ts` (a model-restricted key is rejected 403 before reaching credential selection; a combo-restricted key is rejected 403 requesting a disallowed combo; a key that DOES allow the requested model still proceeds past policy).
- **fix(security):** loopback-gate `/api/middleware/*` so a leaked JWT over a tunnel can't install or trigger a middleware hook — middleware hooks compile + run arbitrary JS via `new vm.Script` on the request hot path (`src/lib/middleware/registry.ts`), the same RCE class as the already-gated `/api/plugins/*`; `/api/middleware/` is now in `LOCAL_ONLY_API_PREFIXES` so loopback enforcement runs unconditionally before any auth check (Hard Rules #15 + #17). Regression guard: `tests/unit/route-guard-middleware-local-only.test.ts`. ([#6541](https://github.com/diegosouzapw/OmniRoute/pull/6541)) — see PR. (thanks @developerjillur)
- **fix(startup):** AgentBridge's MITM server no longer fails to start with `ROUTER_API_KEY is required` on a normal install ([#6403](https://github.com/diegosouzapw/OmniRoute/issues/6403)) — `POST /api/tools/agent-bridge/server` resolved the spawned MITM child's router key from only an explicit `apiKey` body field (never sent by the AgentBridge UI — the schema has no such field) and the `ROUTER_API_KEY` env var (unset by default), so `startMitm()` always received `""` and the child hard-exited, even though OmniRoute already had a usable API key in its own DB. A new `resolveRouterApiKey()` now falls back to `pickApiKeyForInternalUse()` (the same DB-backed selector the combo-health-check / cloud-sync internal probes use), resolving in order: explicit key → `ROUTER_API_KEY` env → an existing DB key. Regression guard: `tests/unit/agentbridge-mitm-router-key-6403.test.ts`.

View File

@@ -11,6 +11,7 @@ import { upsertAgentBridgeState } from "@/lib/db/agentBridgeState";
import { getCachedPassword } from "@/mitm/manager";
import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
import { createErrorResponse } from "@/lib/api/errorResponse";
import { ALL_TARGETS } from "@/mitm/targets/index";
type Params = { params: { id: string } };
@@ -33,6 +34,12 @@ export async function POST(request: Request, { params }: Params): Promise<Respon
});
}
// Validate the agent ID maps to a known target.
const target = ALL_TARGETS.find((t) => t.id === id);
if (!target) {
return createErrorResponse({ status: 404, message: `Unknown agent: ${id}` });
}
const { enabled } = parsed.data;
const raw = body as Record<string, unknown>;
const sudoPassword =
@@ -40,9 +47,9 @@ export async function POST(request: Request, { params }: Params): Promise<Respon
try {
if (enabled) {
await addDNSEntry(sudoPassword);
await addDNSEntry(sudoPassword, id);
} else {
await removeDNSEntry(sudoPassword);
await removeDNSEntry(sudoPassword, id);
}
upsertAgentBridgeState({ agent_id: id, dns_enabled: enabled });

View File

@@ -8,6 +8,7 @@ import {
quotePowerShell,
runElevatedPowerShell,
} from "../systemCommands.ts";
import { ALL_TARGETS } from "../targets/index.ts";
// Legacy Antigravity defaults preserved for backward compat.
const ANTIGRAVITY_HOSTS = [
@@ -17,6 +18,12 @@ const ANTIGRAVITY_HOSTS = [
"autopush-cloudcode-pa.sandbox.googleapis.com",
];
function resolveHostsForAgent(agentId?: string): string[] {
if (!agentId) return ANTIGRAVITY_HOSTS;
const target = ALL_TARGETS.find((t) => t.id === agentId);
return target?.hosts ?? ANTIGRAVITY_HOSTS;
}
const IS_WIN = process.platform === "win32";
const HOSTS_FILE = IS_WIN
? path.join(process.env.SystemRoot || "C:\\Windows", "System32", "drivers", "etc", "hosts")
@@ -108,9 +115,13 @@ function hasHostEntry(hostsContent: string, hostname: string): boolean {
* Add /etc/hosts entries for every hostname in `hosts`.
* Idempotent — existing entries are not duplicated.
* Complies with Hard Rule #13: no string interpolation in shell commands.
*
* On Windows, all missing entries are batched into a single elevated PowerShell
* invocation so the user gets one UAC prompt instead of one per line.
*/
export async function addDNSEntries(hosts: string[], sudoPassword: string): Promise<void> {
const hostsContent = readHostsFile();
const missingEntries: string[] = [];
for (const hostname of hosts) {
const lines = dnsLines(hostname);
@@ -122,29 +133,23 @@ export async function addDNSEntries(hosts: string[], sudoPassword: string): Prom
return parts.length >= 2 && parts[0] === ip && parts.includes(host);
});
});
missingEntries.push(...missing);
}
for (const entry of missing) {
if (IS_WIN) {
// HR#13: build PowerShell command via concat (not template literal) so grep
// for `\${` inside script bodies returns zero hits. Values pass through
// `quotePowerShell()` for single-quote escaping — safe against injection
// since both HOSTS_FILE (OS const) and entry (internal `IP host` string)
// are non-user-supplied.
const cmd =
"Add-Content -LiteralPath " +
quotePowerShell(HOSTS_FILE) +
" -Value " +
quotePowerShell(entry);
await runElevatedPowerShell(cmd);
} else {
// Hard Rule #13: entry is passed as stdin data, not interpolated into the command.
await execFileWithPassword(
"sudo",
["-S", "tee", "-a", HOSTS_FILE],
sudoPassword,
`${entry}\n`
);
}
if (missingEntries.length === 0) return;
if (IS_WIN) {
const psHostsFile = quotePowerShell(HOSTS_FILE);
const psEntries = missingEntries.map((e) => quotePowerShell(e)).join(", ");
const script = "Add-Content -LiteralPath " + psHostsFile + " -Value " + psEntries;
await runElevatedPowerShell(script);
for (const entry of missingEntries) {
console.log(`[DNS] Added entry: ${entry}`);
}
} else {
const data = missingEntries.map((e) => `${e}\n`).join("");
await execFileWithPassword("sudo", ["-S", "tee", "-a", HOSTS_FILE], sudoPassword, data);
for (const entry of missingEntries) {
console.log(`[DNS] Added entry: ${entry}`);
}
}
@@ -168,46 +173,43 @@ fs.writeFileSync(filePath, filtered.join("\\n").replace(/\\n*$/, "\\n"));
* Remove /etc/hosts entries for every hostname in `hosts`.
* Idempotent — silently skips hosts that are not present.
* Complies with Hard Rule #13: HOSTS_FILE and hostname are passed as argv, not interpolated.
*
* On Windows, all hostnames are filtered in a single elevated PowerShell
* invocation so the user gets one UAC prompt instead of one per host.
*/
export async function removeDNSEntries(hosts: string[], sudoPassword: string): Promise<void> {
const hostsContent = readHostsFile();
const presentHosts = hosts.filter((h) => hasHostEntry(hostsContent, h));
for (const hostname of hosts) {
if (!hasHostEntry(hostsContent, hostname)) {
console.log(`[DNS] Entry for ${hostname} not present — skipping`);
continue;
}
if (presentHosts.length === 0) return;
try {
if (IS_WIN) {
// HR#13: build PowerShell script via concat (not template literal) so grep
// for `\${` inside script bodies returns zero hits. `psHostsFile` and
// `psTargetHost` are quotePowerShell-escaped values (single-quote escape).
const psHostsFile = quotePowerShell(HOSTS_FILE);
const psTargetHost = quotePowerShell(hostname);
const script =
"\n $hostsFile = " +
psHostsFile +
";\n $targetHost = " +
psTargetHost +
";\n $lines = Get-Content -LiteralPath $hostsFile;\n" +
" $filtered = $lines | Where-Object {\n" +
" $parts = ($_ -split '\\s+') | Where-Object { $_ };\n" +
" -not (($parts.Length -ge 2) -and ($parts -contains $targetHost))\n" +
" };\n" +
" Set-Content -LiteralPath $hostsFile -Value $filtered;\n ";
await runElevatedPowerShell(script);
} else {
// Hard Rule #13: HOSTS_FILE and hostname are argv arguments, not interpolated.
await execFileWithPassword(
"sudo",
["-S", process.execPath, "-e", REMOVE_HOSTS_ENTRY_SCRIPT, HOSTS_FILE, hostname],
sudoPassword
);
}
if (IS_WIN) {
const psHostsFile = quotePowerShell(HOSTS_FILE);
const psTargets = presentHosts.map((h) => quotePowerShell(h)).join(", ");
const script =
"$hostsFile = " +
psHostsFile +
";\n $targetHosts = @(" +
psTargets +
");\n" +
" $lines = Get-Content -LiteralPath $hostsFile;\n" +
" $filtered = $lines | Where-Object {\n" +
" $part = ($_ -split '\\s+') | Where-Object { $_ };\n" +
" -not ($part.Length -ge 2 -and ($targetHosts -contains $part[1]))\n" +
" };\n" +
" Set-Content -LiteralPath $hostsFile -Value $filtered;\n ";
await runElevatedPowerShell(script);
for (const hostname of presentHosts) {
console.log(`[DNS] Removed entries for ${hostname}`);
}
} else {
for (const hostname of presentHosts) {
await execFileWithPassword(
"sudo",
["-S", process.execPath, "-e", REMOVE_HOSTS_ENTRY_SCRIPT, HOSTS_FILE, hostname],
sudoPassword
);
console.log(`[DNS] Removed entries for ${hostname}`);
} catch (error) {
throw new Error(`Failed to remove DNS entry for ${hostname}: ${getErrorMessage(error)}`);
}
}
}
@@ -226,17 +228,19 @@ export function checkDNSEntry(): boolean {
}
/**
* Add DNS entries for the Antigravity default hosts.
* Add DNS entries for the Antigravity default hosts, or for a specific agent
* when `agentId` is provided.
* Delegates to `addDNSEntries` — backward compat wrapper.
*/
export async function addDNSEntry(sudoPassword: string): Promise<void> {
await addDNSEntries(ANTIGRAVITY_HOSTS, sudoPassword);
export async function addDNSEntry(sudoPassword: string, agentId?: string): Promise<void> {
await addDNSEntries(resolveHostsForAgent(agentId), sudoPassword);
}
/**
* Remove DNS entries for the Antigravity default hosts.
* Remove DNS entries for the Antigravity default hosts, or for a specific agent
* when `agentId` is provided.
* Delegates to `removeDNSEntries` — backward compat wrapper.
*/
export async function removeDNSEntry(sudoPassword: string): Promise<void> {
await removeDNSEntries(ANTIGRAVITY_HOSTS, sudoPassword);
export async function removeDNSEntry(sudoPassword: string, agentId?: string): Promise<void> {
await removeDNSEntries(resolveHostsForAgent(agentId), sudoPassword);
}

View File

@@ -552,7 +552,12 @@ async function startMitmInternal(
const certPath = path.join(resolveMitmDataDir(), "mitm", "server.crt");
if (!fs.existsSync(certPath)) {
log.info("Generating SSL certificate...");
await generateCert();
try {
await generateCert();
} catch (err) {
log.error({ err }, "Failed to generate SSL certificate");
throw err;
}
}
// 2. Install certificate to system keychain. A failure here must NOT abort the
@@ -576,7 +581,11 @@ async function startMitmInternal(
// 3. Add DNS entries: Antigravity defaults + all agents with dns_enabled=true +
// all custom hosts with enabled=true. Best-effort — see provisionDnsEntries.
log.info("Adding DNS entries...");
await provisionDnsEntries(sudoPassword);
try {
await provisionDnsEntries(sudoPassword);
} catch (err) {
log.error({ err }, "DNS provisioning threw unexpectedly (continuing)");
}
// 4. Start MITM server
log.info("Starting MITM server...");
@@ -619,9 +628,13 @@ async function startMitmInternal(
const proc = serverProcess;
serverPid = proc.pid ?? null;
// Save PID to file
// Save PID to file — best-effort, must not orphan spawned child process
if (serverPid !== null) {
fs.writeFileSync(PID_FILE, String(serverPid));
try {
fs.writeFileSync(PID_FILE, String(serverPid));
} catch (err) {
log.error({ err, pid: serverPid }, "Failed to write MITM PID file (continuing)");
}
}
// Buffer recent stderr so a startup failure can be reported with its real

View File

@@ -0,0 +1,60 @@
/**
* Unit tests: POST /api/tools/agent-bridge/agents/[id]/dns — unknown-agent 404 guard.
*
* Before this fix, an unknown `id` fell through straight into `addDNSEntry`/
* `removeDNSEntry`, which silently resolved the legacy Antigravity default hosts
* instead of rejecting the request. The route now validates `id` against
* `ALL_TARGETS` first and returns 404 for unknown agents — verified here without
* touching /etc/hosts (the 404 short-circuits before any DNS call).
*/
import test from "node:test";
import assert from "node:assert/strict";
const dnsRoute = await import(
"../../src/app/api/tools/agent-bridge/agents/[id]/dns/route.ts"
);
function makeRequest(body: unknown): Request {
return new Request("http://127.0.0.1/api/tools/agent-bridge/agents/x/dns", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify(body),
});
}
test("POST .../[id]/dns: unknown agent id returns 404 before any DNS call", async () => {
const res = await dnsRoute.POST(makeRequest({ enabled: true }), {
params: { id: "__nonexistent_agent__" },
});
assert.equal(res.status, 404);
const body = (await res.json()) as { error?: { message?: string } };
assert.ok(
JSON.stringify(body).includes("__nonexistent_agent__"),
"404 body should reference the unknown agent id"
);
});
test("POST .../[id]/dns: invalid body still returns 400 (schema validated first)", async () => {
const res = await dnsRoute.POST(makeRequest({ enabled: "not-a-boolean" }), {
params: { id: "__nonexistent_agent__" },
});
assert.equal(res.status, 400);
});
test("POST .../[id]/dns: malformed JSON body returns 400", async () => {
const req = new Request("http://127.0.0.1/api/tools/agent-bridge/agents/x/dns", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: "{not-json",
});
const res = await dnsRoute.POST(req, { params: { id: "cursor" } });
assert.equal(res.status, 400);
});
test("POST .../[id]/dns: error responses do not leak stack traces", async () => {
const res = await dnsRoute.POST(makeRequest({ enabled: true }), {
params: { id: "__nonexistent_agent__" },
});
const text = await res.text();
assert.ok(!text.includes("at /"), "stack trace leaked in dns route 404 response");
});

View File

@@ -69,6 +69,7 @@ const tmpHostsFile = path.join(tmpDir, "hosts");
// Import the module under test AFTER all setup.
const dnsModule = await import("../../src/mitm/dns/dnsConfig.ts");
const { addDNSEntries, removeDNSEntries, addDNSEntry, removeDNSEntry, checkDNSEntry } = dnsModule;
const { ALL_TARGETS } = await import("../../src/mitm/targets/index.ts");
// ---------------------------------------------------------------------------
// Tests
@@ -95,10 +96,60 @@ test("addDNSEntry (legacy) is a function that delegates for Antigravity hosts",
assert.equal(typeof addDNSEntry, "function");
});
test("addDNSEntry with agentId resolves agent-specific hosts from ALL_TARGETS", async () => {
// Cursor target has hosts: ["api2.cursor.sh"]
// Verify that calling addDNSEntry with agentId="cursor" passes the right hosts.
// We call with [] to skip actual exec — just verifying the function signature accepts agentId.
await assert.doesNotReject(
addDNSEntry("fake-sudo", "cursor"),
"addDNSEntry must accept optional agentId parameter"
);
});
test("addDNSEntry without agentId falls back to Antigravity hosts (backward compat)", async () => {
await assert.doesNotReject(
addDNSEntry("fake-sudo"),
"addDNSEntry without agentId must still work for backward compat"
);
});
test("addDNSEntry with unknown agentId falls back to Antigravity hosts", async () => {
await assert.doesNotReject(
addDNSEntry("fake-sudo", "__nonexistent_agent__"),
"addDNSEntry with unknown agentId must fall back to Antigravity hosts"
);
});
test("removeDNSEntry (legacy) is a function that delegates for Antigravity hosts", () => {
assert.equal(typeof removeDNSEntry, "function");
});
test("removeDNSEntry with agentId resolves agent-specific hosts from ALL_TARGETS", async () => {
await assert.doesNotReject(
removeDNSEntry("fake-sudo", "copilot"),
"removeDNSEntry must accept optional agentId parameter"
);
});
test("resolveHostsForAgent returns Antigravity hosts when agentId is undefined", () => {
// Verify ALL_TARGETS exists and cursor target has expected hosts
const cursorTarget = ALL_TARGETS.find((t) => t.id === "cursor");
assert.ok(cursorTarget, "cursor target must exist in ALL_TARGETS");
assert.ok(
cursorTarget.hosts.includes("api2.cursor.sh"),
"cursor target must include api2.cursor.sh"
);
// Codex target
const codexTarget = ALL_TARGETS.find((t) => t.id === "codex");
assert.ok(codexTarget, "codex target must exist in ALL_TARGETS");
assert.ok(codexTarget.hosts.includes("chatgpt.com"), "codex target must include chatgpt.com");
});
test("addDNSEntries batches missing entries with no-op on empty list", async () => {
// Empty list must resolve immediately (no exec, no error)
await assert.doesNotReject(addDNSEntries([], "fake-sudo"));
});
test("addDNSEntries: skips hosts already in /etc/hosts (idempotency)", async () => {
// Read live /etc/hosts and pick the first entry that already exists.
// If localhost is in /etc/hosts we use it; otherwise skip this sub-assertion.
@@ -157,11 +208,17 @@ test("addDNSEntries: entry passed as stdin data, not shell-interpolated", () =>
const srcPath = new URL("../../src/mitm/dns/dnsConfig.ts", import.meta.url).pathname;
const src = fs.readFileSync(srcPath, "utf8");
// The stdin data `${entry}\n` is the body text sent to tee via pipe — not
// part of the command array. Verify the pattern appears in the source.
// The stdin `data` is built from the batched entries and sent to tee via pipe —
// not part of the command array. Verify the pattern appears in the source and
// that it's passed as the 4th positional arg to execFileWithPassword (stdin),
// not interpolated into the argv array.
assert.ok(
src.includes("`${entry}\\n`"),
"entry content must be passed as stdin to tee, not interpolated in args"
src.includes('missingEntries.map((e) => `${e}\\n`).join("")'),
"entry content must be built from missingEntries for stdin, not interpolated in args"
);
assert.ok(
src.includes('execFileWithPassword("sudo", ["-S", "tee", "-a", HOSTS_FILE], sudoPassword, data)'),
"entry data must be passed as stdin to tee, not interpolated in args"
);
});