mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-05 14:52:09 +03:00
fix(mitm): gate sudo prompts on server platform, not browser UA
The MITM control surface decided whether to prompt for a sudo password by
reading `navigator.userAgent` in the dashboard tool card and by checking
`process.platform === "win32"` in the API route. Two real cases broke:
1. Windows browser hitting a Linux/macOS server: the card skipped the modal
(UA said Windows), POST/DELETE rejected the request with `Missing
sudoPassword` (server said Linux). The MITM never started.
2. Linux server running as root, under NOPASSWD sudoers, or in a minimal
container without `sudo` on PATH: the card forced an unnecessary password
modal even though `sudo` would never have prompted.
Fix:
- `src/mitm/dns/dnsConfig.ts` gains `isSudoAvailable()`,
`canRunSudoWithoutPassword()`, and `isSudoPasswordRequired()`. The probe
uses `execFileSync("sudo", ["-n", "true"], { stdio: "ignore" })` — fixed
args, no shell expansion, per Hard Rule #13.
- `GET /api/cli-tools/antigravity-mitm` now reports `isWin` and
`needsSudoPassword` so the UI can decide based on the server's platform.
- POST/DELETE drop the unconditional non-Windows password requirement and
only return 400 when sudo is genuinely needed (`!isWin && !pwd &&
!isRoot() && isSudoPasswordRequired()`). The cached-password write also
guards against caching an empty string.
- `AntigravityToolCard.tsx` replaces the `navigator.userAgent` check with
`status?.isWin === true`, `status?.hasCachedPassword === true`, and
`status?.needsSudoPassword === false`.
TDD: `tests/unit/mitm-sudo-gate-822.test.ts` pins the helper contract on
the native platform (5 cases). Helpers short-circuit on Windows / root /
no-sudo before invoking `sudo`, so the tests never exercise real sudo.
Co-authored-by: Rezky Hamid <hiepau1231@gmail.com>
Inspired-by: https://github.com/decolua/9router/pull/822
This commit is contained in:
@@ -53,6 +53,7 @@ _In development — bullets added per PR; finalized at release._
|
||||
- **fix(sse): combo routing now skips a provider whose credentials are all disabled instead of failing the whole request** — when a combo like `antigravity/opus → github/opus` hit a leg whose only configured connections were disabled (or where no connections existed at all), `handleNoCredentials` returned `400 BAD_REQUEST`, which the combo target loop treats as a hard stop (combo's 400-break guard from PR #4316 / issue #4279 prevents infinite fallback loops on body-specific 4xx errors). The combo therefore died on the first leg even when later targets were perfectly healthy. The no-active-credentials branch now returns `404 NOT_FOUND` with `"No active credentials for provider: <p>"` instead — `404` flows through `checkFallbackError` as `shouldFallback: true` (generic-error catch-all path in `open-sse/services/accountFallback.ts`), so the next combo target is tried. The log level for this branch also drops from `error` to `warn` because zero active credentials is an expected operator-driven state, not a server fault. Inspired-by upstream decolua/9router PR #336. (thanks @East-rayyy)
|
||||
- **fix(dashboard): Manual Config modal "Copy" button now works on HTTP / non-secure deployments** — the copy handler in `ManualConfigModal` re-implemented the Clipboard-API-with-`execCommand`-fallback inline and gated the modern path on `window.isSecureContext`, so some non-secure-context browsers (and any future drift) silently lost the fallback. Migrated to the shared `useCopyToClipboard` hook (which delegates to `src/shared/utils/clipboard.ts`), giving consistent HTTP/HTTPS behavior with the rest of the dashboard and removing the duplicated code path. (thanks @anuragg-saxenaa)
|
||||
- **fix(dashboard): enable Codex Apply / Reset buttons when the CLI is installed** — on the Codex CLI tool card the **Apply** button was disabled whenever `selectedApiKey` was empty, but the local default `sk_omniroute` key is a valid choice when cloud mode is off or no API keys are configured — so Apply was stuck disabled even when the configuration was otherwise complete. **Reset** was also disabled when `codexStatus.hasOmniRoute` was false, which made it impossible to clear Codex configuration on installs that had never been pointed at OmniRoute. The disabled logic is now extracted into a pure helper (`codexButtonState.ts`) covered by unit tests: Apply is disabled only when no model is selected, or when cloud mode is on **and** keys exist **and** none is picked; Reset is disabled only while a reset is in flight. (thanks @anuragg-saxenaa)
|
||||
- **fix(mitm):** gate the sudo password prompt on the **server** platform, not the browser. The MITM control surface previously decided whether to ask for a sudo password by reading the browser's `navigator.userAgent`, which broke a Windows browser hitting a Linux server (no prompt → request rejected with `Missing sudoPassword`) and also forced an unnecessary modal on Linux hosts running as root, with NOPASSWD sudoers, or in minimal containers with no `sudo` binary on PATH. `GET /api/cli-tools/antigravity-mitm` now reports `isWin` and `needsSudoPassword` (probed via a safe `execFileSync("sudo", ["-n", "true"])`, per Hard Rule #13), and the Antigravity tool card uses the server-reported status to decide whether to show the modal. The POST/DELETE handlers stop returning 400 when sudo is genuinely not required. (thanks @hiepau1231)
|
||||
- **fix(embeddings):** forward output dimensions to Gemini for consistent embedding dims. (thanks @nguyenha935)
|
||||
- **fix(translator):** sanitize Read tool args from non-Anthropic models to prevent retry loops. (thanks @GodrezJr2)
|
||||
- **fix(usage):** reuse Gemini CLI project ID for quota checks (avoid re-discovery). (thanks @Delcado19)
|
||||
|
||||
@@ -87,11 +87,18 @@ export default function AntigravityToolCard({
|
||||
}
|
||||
};
|
||||
|
||||
// Windows uses UAC dialog, no sudo needed
|
||||
const isWindows = typeof navigator !== "undefined" && navigator.userAgent?.includes("Windows");
|
||||
// MITM elevation is decided by the *server* OS, not by this browser's user
|
||||
// agent. The server reports `isWin` and `needsSudoPassword` in GET status —
|
||||
// a Windows browser hitting a Linux server still needs sudo, and a Linux
|
||||
// browser hitting a Windows server does not (#822).
|
||||
const serverIsWindows = status?.isWin === true;
|
||||
const canRunWithoutPassword =
|
||||
serverIsWindows ||
|
||||
status?.hasCachedPassword === true ||
|
||||
status?.needsSudoPassword === false;
|
||||
|
||||
const handleStart = () => {
|
||||
if (isWindows || status?.hasCachedPassword) {
|
||||
if (canRunWithoutPassword) {
|
||||
doStart("");
|
||||
} else {
|
||||
setShowPasswordModal(true);
|
||||
@@ -100,7 +107,7 @@ export default function AntigravityToolCard({
|
||||
};
|
||||
|
||||
const handleStop = () => {
|
||||
if (isWindows || status?.hasCachedPassword) {
|
||||
if (canRunWithoutPassword) {
|
||||
doStop("");
|
||||
} else {
|
||||
setShowPasswordModal(true);
|
||||
|
||||
@@ -9,6 +9,7 @@ import { cliMitmStartSchema, cliMitmStopSchema } from "@/shared/validation/schem
|
||||
import { isValidationFailure, validateBody } from "@/shared/validation/helpers";
|
||||
import { resolveApiKey } from "@/shared/services/apiKeyResolver";
|
||||
import { isRoot } from "@/mitm/systemCommands";
|
||||
import { isSudoPasswordRequired } from "@/mitm/dns/dnsConfig";
|
||||
|
||||
// GET - Check MITM status
|
||||
export async function GET(request) {
|
||||
@@ -18,12 +19,21 @@ export async function GET(request) {
|
||||
try {
|
||||
const { getMitmStatus, getCachedPassword } = await import("@/mitm/manager.runtime");
|
||||
const status = await getMitmStatus();
|
||||
const isWin = process.platform === "win32";
|
||||
const hasCachedPassword = !!getCachedPassword();
|
||||
// Probe sudo availability so the UI can hide the password modal on hosts
|
||||
// where it's unnecessary (Windows, root user, NOPASSWD sudoers, minimal
|
||||
// containers without sudo). MITM elevation is decided by the server OS,
|
||||
// not by the browser's user agent — see PR title.
|
||||
const needsSudoPassword = !isWin && !hasCachedPassword && isSudoPasswordRequired();
|
||||
return NextResponse.json({
|
||||
running: status.running,
|
||||
pid: status.pid || null,
|
||||
dnsConfigured: status.dnsConfigured || false,
|
||||
certExists: status.certExists || false,
|
||||
hasCachedPassword: !!getCachedPassword(),
|
||||
hasCachedPassword,
|
||||
isWin,
|
||||
needsSudoPassword,
|
||||
});
|
||||
} catch (error) {
|
||||
console.log("Error getting MITM status:", sanitizeErrorMessage(error));
|
||||
@@ -71,12 +81,15 @@ export async function POST(request) {
|
||||
const isRootUser = !isWin && isRoot();
|
||||
const pwd = sudoPassword || getCachedPassword() || "";
|
||||
|
||||
if (!isWin && !pwd && !isRootUser) {
|
||||
// Require a sudo password only when the OS actually needs one. Skips the
|
||||
// prompt on Windows (UAC), root, NOPASSWD sudoers, and minimal containers
|
||||
// without sudo on PATH (#822).
|
||||
if (!isWin && !pwd && !isRootUser && isSudoPasswordRequired()) {
|
||||
return NextResponse.json({ error: "Missing sudoPassword" }, { status: 400 });
|
||||
}
|
||||
|
||||
const result = await startMitm(apiKey, pwd);
|
||||
if (!isWin) setCachedPassword(pwd);
|
||||
if (!isWin && pwd) setCachedPassword(pwd);
|
||||
|
||||
return NextResponse.json({
|
||||
success: true,
|
||||
@@ -124,7 +137,10 @@ export async function DELETE(request) {
|
||||
const isRootUser = !isWin && isRoot();
|
||||
const pwd = sudoPassword || getCachedPassword() || "";
|
||||
|
||||
if (!isWin && !pwd && !isRootUser) {
|
||||
// Require a sudo password only when the OS actually needs one. Skips the
|
||||
// prompt on Windows (UAC), root, NOPASSWD sudoers, and minimal containers
|
||||
// without sudo on PATH (#822).
|
||||
if (!isWin && !pwd && !isRootUser && isSudoPasswordRequired()) {
|
||||
return NextResponse.json({ error: "Missing sudoPassword" }, { status: 400 });
|
||||
}
|
||||
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
import { execFileSync } from "child_process";
|
||||
import fs from "fs";
|
||||
import path from "path";
|
||||
import {
|
||||
execFileWithPassword,
|
||||
getErrorMessage,
|
||||
isRoot,
|
||||
quotePowerShell,
|
||||
runElevatedPowerShell,
|
||||
} from "../systemCommands.ts";
|
||||
@@ -20,6 +22,51 @@ const HOSTS_FILE = IS_WIN
|
||||
? path.join(process.env.SystemRoot || "C:\\Windows", "System32", "drivers", "etc", "hosts")
|
||||
: "/etc/hosts";
|
||||
|
||||
/**
|
||||
* Return true if `sudo` is available on PATH. Windows always reports `true`
|
||||
* (no sudo concept — UAC handles elevation). Minimal containers without sudo
|
||||
* also report `false`, so callers can fall through to the no-elevation path.
|
||||
*/
|
||||
export function isSudoAvailable(): boolean {
|
||||
if (IS_WIN) return true;
|
||||
try {
|
||||
// `which sudo` exits 0 when found, non-zero otherwise. Fixed args, no
|
||||
// shell expansion — safe per Hard Rule #13.
|
||||
execFileSync("which", ["sudo"], { stdio: "ignore", windowsHide: true });
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Return true when MITM elevation can proceed without prompting for a sudo
|
||||
* password — i.e. Windows (UAC handles it), root user, no sudo binary
|
||||
* (minimal container), or `sudo -n true` succeeds (passwordless NOPASSWD).
|
||||
*/
|
||||
export function canRunSudoWithoutPassword(): boolean {
|
||||
if (IS_WIN) return true;
|
||||
if (isRoot()) return true;
|
||||
if (!isSudoAvailable()) return true;
|
||||
try {
|
||||
// `sudo -n true` exits 0 when the user can run sudo without a password
|
||||
// (cached credential or NOPASSWD). Exits non-zero otherwise. Fixed args.
|
||||
execFileSync("sudo", ["-n", "true"], { stdio: "ignore", windowsHide: true });
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Server-side helper for the MITM API: true when a sudo password must be
|
||||
* collected from the user before invoking privileged commands.
|
||||
* False on Windows, root, missing-sudo containers, or NOPASSWD sudoers.
|
||||
*/
|
||||
export function isSudoPasswordRequired(): boolean {
|
||||
return !IS_WIN && isSudoAvailable() && !canRunSudoWithoutPassword();
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the set of /etc/hosts lines for a given hostname.
|
||||
* Both IPv4 and IPv6 are needed — modern systems often resolve IPv6 first.
|
||||
|
||||
88
tests/unit/mitm-sudo-gate-822.test.ts
Normal file
88
tests/unit/mitm-sudo-gate-822.test.ts
Normal file
@@ -0,0 +1,88 @@
|
||||
/**
|
||||
* PR #822: gate sudo prompts on the server platform.
|
||||
*
|
||||
* The MITM control surface previously decided whether to prompt for a sudo
|
||||
* password using the *browser's* `navigator.userAgent` and a non-Windows
|
||||
* unconditional gate on the API route. That broke two real cases:
|
||||
*
|
||||
* 1. Windows browser hitting a Linux server (no prompt → request 400s).
|
||||
* 2. Linux server running as root or under NOPASSWD sudoers (unnecessary
|
||||
* modal blocks the user even though sudo would never ask).
|
||||
*
|
||||
* The fix:
|
||||
* - `dnsConfig.ts` exposes `canRunSudoWithoutPassword()` /
|
||||
* `isSudoPasswordRequired()` that probe the actual server state.
|
||||
* - The route surfaces `isWin` + `needsSudoPassword` so the UI can decide
|
||||
* based on the server's platform, not the browser's.
|
||||
*
|
||||
* These tests pin the *pure* helper contract — no real `sudo` is invoked
|
||||
* because every probe path is short-circuited before it tries `sudo -n true`
|
||||
* (Windows / root / no-sudo-on-PATH).
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
canRunSudoWithoutPassword,
|
||||
isSudoAvailable,
|
||||
isSudoPasswordRequired,
|
||||
} from "../../src/mitm/dns/dnsConfig.ts";
|
||||
|
||||
test("isSudoAvailable returns a boolean on the current platform", () => {
|
||||
const result = isSudoAvailable();
|
||||
assert.equal(typeof result, "boolean");
|
||||
// Windows reports true unconditionally (no sudo concept).
|
||||
if (process.platform === "win32") {
|
||||
assert.equal(result, true);
|
||||
}
|
||||
});
|
||||
|
||||
test("canRunSudoWithoutPassword short-circuits to true on Windows and root", () => {
|
||||
const result = canRunSudoWithoutPassword();
|
||||
assert.equal(typeof result, "boolean");
|
||||
|
||||
if (process.platform === "win32") {
|
||||
assert.equal(result, true, "Windows uses UAC, never needs sudo password");
|
||||
return;
|
||||
}
|
||||
|
||||
// Linux/macOS: root user always passes without a password.
|
||||
const isRootUser = !!(process.getuid && process.getuid() === 0);
|
||||
if (isRootUser) {
|
||||
assert.equal(result, true, "root user never needs sudo password");
|
||||
}
|
||||
});
|
||||
|
||||
test("isSudoPasswordRequired returns false on Windows", () => {
|
||||
if (process.platform !== "win32") {
|
||||
// Can't simulate Windows from a non-Windows test runner; assert the
|
||||
// contract holds on the native platform.
|
||||
const result = isSudoPasswordRequired();
|
||||
assert.equal(typeof result, "boolean");
|
||||
return;
|
||||
}
|
||||
assert.equal(isSudoPasswordRequired(), false);
|
||||
});
|
||||
|
||||
test("isSudoPasswordRequired returns false when running as root on POSIX", () => {
|
||||
if (process.platform === "win32") return;
|
||||
const isRootUser = !!(process.getuid && process.getuid() === 0);
|
||||
if (!isRootUser) {
|
||||
// Skip — we can't elevate from the test runner. This branch is covered
|
||||
// by the contract: isSudoPasswordRequired === !IS_WIN && isSudoAvailable
|
||||
// && !canRunSudoWithoutPassword, and canRunSudoWithoutPassword returns
|
||||
// early when isRoot() is true.
|
||||
return;
|
||||
}
|
||||
assert.equal(isSudoPasswordRequired(), false);
|
||||
});
|
||||
|
||||
test("isSudoPasswordRequired is consistent with canRunSudoWithoutPassword on POSIX", () => {
|
||||
if (process.platform === "win32") return;
|
||||
if (!isSudoAvailable()) {
|
||||
// No sudo binary → never required.
|
||||
assert.equal(isSudoPasswordRequired(), false);
|
||||
return;
|
||||
}
|
||||
// When sudo *is* available, requirement is the inverse of "can run without".
|
||||
assert.equal(isSudoPasswordRequired(), !canRunSudoWithoutPassword());
|
||||
});
|
||||
Reference in New Issue
Block a user