diff --git a/CHANGELOG.md b/CHANGELOG.md index 64098b3fd6..857b85deaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral ### 🐛 Bug Fixes +- **fix(cli):** the dashboard's Claude Code CLI card could report "Not detected"/"Not installed" even when Claude Code was genuinely installed and previously used ([#6701](https://github.com/diegosouzapw/OmniRoute/issues/6701)) — `getCliRuntimeStatus()` (`src/shared/services/cliRuntime.ts`) determined `installed` purely from binary resolution (known install paths + a `where`/`which` PATH search), with no fallback when that lookup fails for reasons unrelated to whether the CLI is actually installed (stale PATH inherited by a long-running/background process, the binary having moved, an install method not yet catalogued, etc.) — even though `~/.claude/settings.json` on disk proves the tool was installed and used before. Upstream 9router's equivalent route already has this exact fallback. A new `withSettingsFallback()` (`src/shared/services/cliInstallFallback.ts`) restores 9router parity: when the binary lookup's own reason is `"not_found"` (never for deliberate security rejections like unsafe/relative env overrides or symlink escapes) and the tool's settings file exists on disk, `installed` now reports `true`. Regression guard: `tests/unit/repro-6701-claude-detect-fallback.test.ts`. - **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(guardrails):** Vision Bridge's individual-model auto-reroute (route an image-bearing request straight to a vision-capable model instead of describe-then-forward) could bypass a policy-restricted API key's model allowlist/budget ([#6640](https://github.com/diegosouzapw/OmniRoute/pull/6640)) — `VisionBridgeGuardrail.preCall()` (`src/lib/guardrails/visionBridge.ts`) swaps `body.model` to the best available vision-capable model, but that swap happens in the guardrail pipeline AFTER `chat.ts` already called `enforceApiKeyPolicy()` against the ORIGINAL model, so a key scoped to a narrow `allowedModels` list could still execute against an unvetted (and possibly costlier) vision model the reroute picked. `chat.ts` now re-validates any guardrail-driven model change against the same per-key allowlist (`isModelAllowedForKey`) before honoring it, falling back to the original already-approved model when the reroute target is not allowed. The reroute path also now honors an explicit `settings.visionBridgeModel` operator override (previously ignored, unlike the combo/describe path a few lines below it, which already respects it via `getVisionBridgeConfig`). Regression guard: `tests/unit/guardrails/visionBridge.test.ts` (22 tests). (thanks @herjarsa) - **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 ` 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). diff --git a/src/shared/services/cliInstallFallback.ts b/src/shared/services/cliInstallFallback.ts new file mode 100644 index 0000000000..dfe57c3bd8 --- /dev/null +++ b/src/shared/services/cliInstallFallback.ts @@ -0,0 +1,69 @@ +import fsSync from "fs"; + +/** + * #6701 — 9router-parity fallback for CLI install detection. + * + * `getCliRuntimeStatus()` in `cliRuntime.ts` determines `installed` from + * binary resolution alone (known install paths + a `where`/`which` PATH + * search). If the binary is not currently resolvable — stale PATH inherited + * by a long-running/background OmniRoute process, the binary having moved, + * or an install method we don't enumerate yet — it used to unconditionally + * report `installed:false`, even when the tool's own settings/config file on + * disk proves it was installed and used before. + * + * Upstream 9router's equivalent route + * (`src/app/api/cli-tools/claude-settings/route.js::checkClaudeInstalled()`) + * has a second-chance fallback: when `where`/`which` fails, it still reports + * `installed:true` if the settings file exists. This restores that fallback + * for any CLI tool that declares a `settings` config path (currently + * `claude` and `droid` — see `CLI_TOOLS` in `cliRuntime.ts`). + * + * Only applies when the lookup's own reason is "not_found" — i.e. the binary + * genuinely couldn't be located on PATH/known install paths. Deliberate + * security rejections (unsafe/relative env override paths, symlink escapes, + * suspicious file sizes, etc.) must stay `installed:false` regardless of + * whether a settings file happens to exist. + */ +export interface NotInstalledResult { + installed: false; + runnable: boolean; + command: string | null; + commandPath: string | null; + reason: string; + runtimeMode: string; + requiresBinary: boolean; +} + +export interface SettingsFallbackResult { + installed: true; + runnable: false; + command: string | null; + commandPath: null; + reason: "settings_found_binary_unresolved"; + runtimeMode: string; + requiresBinary: boolean; +} + +/** + * Given the resolved settings-file path for a tool (or undefined if the tool + * has none) and the "not installed" result the binary lookup already + * produced, return a settings-fallback result when the settings file exists + * on disk, or the original "not installed" result unchanged otherwise. + */ +export const withSettingsFallback = ( + settingsPath: string | undefined, + notInstalledResult: NotInstalledResult +): NotInstalledResult | SettingsFallbackResult => { + if (notInstalledResult.reason !== "not_found") return notInstalledResult; + if (!settingsPath || !fsSync.existsSync(settingsPath)) return notInstalledResult; + + return { + installed: true, + runnable: false, + command: notInstalledResult.command, + commandPath: null, + reason: "settings_found_binary_unresolved", + runtimeMode: notInstalledResult.runtimeMode, + requiresBinary: notInstalledResult.requiresBinary, + }; +}; diff --git a/src/shared/services/cliRuntime.ts b/src/shared/services/cliRuntime.ts index 5ccfd27221..c7e778a9a0 100644 --- a/src/shared/services/cliRuntime.ts +++ b/src/shared/services/cliRuntime.ts @@ -5,7 +5,7 @@ import path from "path"; import { spawn, execFileSync } from "child_process"; import { getHermesHome } from "@/lib/cli-helper/config-generator/hermesHome"; import { getCachedLoginShellPath, mergeShellPath } from "./loginShellPath"; - +import { withSettingsFallback } from "./cliInstallFallback"; const VALID_RUNTIME_MODES = new Set(["auto", "host", "container"]); const FALSE_VALUES = new Set(["0", "false", "no", "off"]); @@ -1085,7 +1085,7 @@ export const getCliRuntimeStatus = async (toolId: string) => { const command = located.command; if (!located.installed) { - return { + return withSettingsFallback(getCliConfigPaths(toolId)?.settings, { installed: false, runnable: false, command, @@ -1093,7 +1093,7 @@ export const getCliRuntimeStatus = async (toolId: string) => { reason: located.reason || "not_found", runtimeMode, requiresBinary, - }; + }); } if (located.reason === "not_executable") { diff --git a/tests/unit/repro-6701-claude-detect-fallback.test.ts b/tests/unit/repro-6701-claude-detect-fallback.test.ts new file mode 100644 index 0000000000..c17b6e1705 --- /dev/null +++ b/tests/unit/repro-6701-claude-detect-fallback.test.ts @@ -0,0 +1,74 @@ +/** + * Repro for #6701 — Claude Code CLI reported "not found" in OmniRoute's + * dashboard even though the user has used it before (settings.json present) + * and upstream 9router (same machine, same settings.json) reports it as + * "Connected". + * + * Root cause: `getCliRuntimeStatus()` in src/shared/services/cliRuntime.ts + * only ever answers `installed` from binary resolution (known install paths + * + PATH lookup via `where.exe`/`command -v`). If the CLI binary is not + * currently resolvable (stale PATH inherited by a long-running/background + * OmniRoute process, binary moved, etc.) it unconditionally reports + * installed:false — even when `~/.claude/settings.json` proves the tool was + * installed and used before. + * + * Upstream 9router's equivalent route (src/app/api/cli-tools/claude-settings/route.js) + * has a second-chance fallback: if `where`/`which` fails, it still reports + * installed:true when the settings file exists on disk. OmniRoute's rewrite + * into cliRuntime.ts dropped that fallback, which is the concrete regression + * relative to 9router this issue's screenshots capture. + * + * This test forces the binary lookup to fail deterministically (CLI_CLAUDE_BIN + * pointed at a path that does not exist) while a real settings.json sits under + * an isolated CLI_CONFIG_HOME. Expected (post-fix, 9router-parity) behavior: + * installed should stay true because the settings file is present. Current + * code returns installed:false / reason:"not_found" — this is the RED proof. + */ + +import { describe, it, before, after } 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 { getCliRuntimeStatus } = await import("../../src/shared/services/cliRuntime.ts"); + +describe("#6701 — claude detection should fall back to settings.json when binary is unresolvable", () => { + let configHome: string; + const prevBin = process.env.CLI_CLAUDE_BIN; + const prevConfigHome = process.env.CLI_CONFIG_HOME; + + before(() => { + // Isolated config home *within* os.homedir() (CLI_CONFIG_HOME validation + // requires this) so we never touch the real ~/.claude directory. + configHome = fs.mkdtempSync(path.join(os.homedir(), ".omniroute-test-6701-")); + const claudeDir = path.join(configHome, ".claude"); + fs.mkdirSync(claudeDir, { recursive: true }); + fs.writeFileSync( + path.join(claudeDir, "settings.json"), + JSON.stringify({ env: { ANTHROPIC_BASE_URL: "http://localhost:20128" } }, null, 2) + ); + + // Force the binary lookup to fail deterministically regardless of host state. + process.env.CLI_CLAUDE_BIN = path.join(os.tmpdir(), "definitely-not-a-real-claude-binary-6701"); + process.env.CLI_CONFIG_HOME = configHome; + }); + + after(() => { + fs.rmSync(configHome, { recursive: true, force: true }); + if (prevBin === undefined) delete process.env.CLI_CLAUDE_BIN; + else process.env.CLI_CLAUDE_BIN = prevBin; + if (prevConfigHome === undefined) delete process.env.CLI_CONFIG_HOME; + else process.env.CLI_CONFIG_HOME = prevConfigHome; + }); + + it("reports installed:true when settings.json exists, even if the binary can't be resolved", async () => { + const result = await getCliRuntimeStatus("claude"); + + assert.equal( + result.installed, + true, + `Expected installed:true (9router-parity settings.json fallback), got installed:${result.installed} reason:${result.reason}` + ); + }); +});