diff --git a/@omniroute/opencode-plugin/package.json b/@omniroute/opencode-plugin/package.json index a998eae655..22d50af183 100644 --- a/@omniroute/opencode-plugin/package.json +++ b/@omniroute/opencode-plugin/package.json @@ -26,7 +26,7 @@ "scripts": { "build": "tsup", "clean": "rm -rf dist", - "test": "node --import tsx/esm --test tests/scaffold.test.ts tests/auth.test.ts tests/options-schema.test.ts tests/multi-instance.test.ts tests/fetch-interceptor.test.ts tests/provider.test.ts tests/gemini-sanitize.test.ts tests/combos.test.ts tests/config-shim.test.ts tests/features.test.ts", + "test": "node --import tsx/esm --test tests/scaffold.test.ts tests/auth.test.ts tests/options-schema.test.ts tests/multi-instance.test.ts tests/fetch-interceptor.test.ts tests/provider.test.ts tests/gemini-sanitize.test.ts tests/combos.test.ts tests/config-shim.test.ts tests/features.test.ts tests/usable-combo.test.ts tests/disk-snapshot-perms.test.ts", "prepublishOnly": "npm run clean && npm run build && npm test" }, "keywords": [ diff --git a/@omniroute/opencode-plugin/src/index.ts b/@omniroute/opencode-plugin/src/index.ts index 6170878d0f..0e917a6f08 100644 --- a/@omniroute/opencode-plugin/src/index.ts +++ b/@omniroute/opencode-plugin/src/index.ts @@ -1371,15 +1371,27 @@ export function isUsableCombo( ): boolean { const steps = Array.isArray(combo.models) ? combo.models : []; if (steps.length === 0) return true; - let sawKnownProvider = false; + // The provider id is folded INTO the full model string by OmniRoute's + // `normalizeComboRecord` (e.g. "cc/claude-opus-4-7") — combo member refs do + // NOT carry a separate `providerId` field. Derive the prefix from `step.model` + // and apply the same subtract-filter verdict as `isUsableRawModelId`. + let sawResolvableMember = false; for (const step of steps) { - const pid = (step as unknown as { providerId?: unknown }).providerId; - if (typeof pid !== "string" || pid.length === 0) continue; - sawKnownProvider = true; - if (usable.canonicals.has(pid) || usable.aliases.has(pid)) return true; + // Nested combo refs carry no model id we can resolve to a provider here. + if (step?.kind === "combo-ref") continue; + const modelId = typeof step?.model === "string" ? step.model : ""; + const slash = modelId.indexOf("/"); + if (slash <= 0) continue; // no provider prefix to evaluate + sawResolvableMember = true; + const prefix = modelId.slice(0, slash); + if (usable.aliases.has(prefix) || usable.canonicals.has(prefix)) return true; + // Unknown prefix (not in the known-alias universe) → can't prove + // unroutable; keep. Known-but-not-usable prefixes keep scanning. + if (!usable.knownAliases.has(prefix)) return true; } - // No member declared a providerId → can't prove unroutable; keep. - if (!sawKnownProvider) return true; + // No member resolved to a provider prefix → can't prove unroutable; keep. + if (!sawResolvableMember) return true; + // Every resolvable member used a known-but-non-usable prefix → drop. return false; } @@ -2126,7 +2138,7 @@ export function createGeminiSanitizingFetch(inner: typeof fetch): typeof fetch { // Streaming body — skip with one-shot warning. if (!geminiStreamingWarningEmitted) { geminiStreamingWarningEmitted = true; - // eslint-disable-next-line no-console + console.warn( "[omniroute-plugin] sanitizeGemini: streaming Request body, skipping schema strip (Gemini may reject)" ); @@ -2584,7 +2596,9 @@ export type OmniRouteDiskSnapshotReader = ( export const defaultDiskSnapshotWriter: OmniRouteDiskSnapshotWriter = async (providerId, entry) => { try { const file = diskSnapshotPath(providerId); - await mkdir(path.dirname(file), { recursive: true }); + // Restrict perms to the owner: the snapshot lives alongside auth.json + // (0o600) and embeds provider topology + masked connection records. + await mkdir(path.dirname(file), { recursive: true, mode: 0o700 }); const snapshot: OmniRouteDiskSnapshot = { v: 1, rawModels: entry.rawModels, @@ -2594,7 +2608,7 @@ export const defaultDiskSnapshotWriter: OmniRouteDiskSnapshotWriter = async (pro rawConnections: entry.rawConnections, writtenAt: Date.now(), }; - await writeFile(file, JSON.stringify(snapshot), "utf8"); + await writeFile(file, JSON.stringify(snapshot), { encoding: "utf8", mode: 0o600 }); } catch { // Soft-fail; caller already has the in-memory cache. } diff --git a/@omniroute/opencode-plugin/tests/disk-snapshot-perms.test.ts b/@omniroute/opencode-plugin/tests/disk-snapshot-perms.test.ts new file mode 100644 index 0000000000..cf3a2750da --- /dev/null +++ b/@omniroute/opencode-plugin/tests/disk-snapshot-perms.test.ts @@ -0,0 +1,66 @@ +/** + * Regression test for the disk-snapshot file permissions (release/v3.8.2 + * review finding C2). The snapshot embeds provider topology + connection + * records and lives alongside auth.json (0o600), so it must NOT be readable by + * group/other. Before the fix it was written with the default (typically + * world-readable 0o644) mode. + */ + +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 { + defaultDiskSnapshotWriter, + diskSnapshotPath, + type OmniRouteFetchCacheEntry, +} from "../src/index.js"; + +function makeEntry(): Omit { + return { + rawModels: [], + rawCombos: [], + rawEnrichment: new Map(), + rawCompressionCombos: [], + rawConnections: [], + }; +} + +test("defaultDiskSnapshotWriter writes an owner-only (no group/other) snapshot", async (t) => { + // POSIX-only assertion; Windows does not honor numeric file modes. + if (process.platform === "win32") { + t.skip("file mode semantics are POSIX-only"); + return; + } + + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-disk-perms-")); + const prevDataDir = process.env.OPENCODE_DATA_DIR; + process.env.OPENCODE_DATA_DIR = tmp; + + try { + await defaultDiskSnapshotWriter("perm-test", makeEntry()); + + const file = diskSnapshotPath("perm-test"); + assert.ok(fs.existsSync(file), "snapshot file should be written"); + + const fileMode = fs.statSync(file).mode & 0o777; + assert.equal( + fileMode & 0o077, + 0, + `snapshot must not be group/other accessible (got ${fileMode.toString(8)})` + ); + + const dirMode = fs.statSync(path.dirname(file)).mode & 0o777; + assert.equal( + dirMode & 0o077, + 0, + `plugins dir must not be group/other accessible (got ${dirMode.toString(8)})` + ); + } finally { + if (prevDataDir === undefined) delete process.env.OPENCODE_DATA_DIR; + else process.env.OPENCODE_DATA_DIR = prevDataDir; + fs.rmSync(tmp, { recursive: true, force: true }); + } +}); diff --git a/@omniroute/opencode-plugin/tests/usable-combo.test.ts b/@omniroute/opencode-plugin/tests/usable-combo.test.ts new file mode 100644 index 0000000000..90d550a86c --- /dev/null +++ b/@omniroute/opencode-plugin/tests/usable-combo.test.ts @@ -0,0 +1,85 @@ +/** + * Regression tests for `isUsableCombo` (release/v3.8.2 code review, finding C1). + * + * The combo member refs returned by `/api/combos` do NOT carry a separate + * `providerId` field — OmniRoute's `normalizeComboRecord` folds the provider + * id INTO the full model string (e.g. "cc/claude-opus-4-7"). The previous + * implementation read `step.providerId` (always `undefined`), so the + * `usableOnly` combo filter silently never dropped anything. These tests pin + * the corrected behavior: the verdict is derived from the `step.model` prefix, + * mirroring `isUsableRawModelId`'s subtract-filter semantics. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; + +import { isUsableCombo, type OmniRouteRawCombo } from "../src/index.js"; + +/** Build a `usable` set bundle for the tests. */ +function buildUsable(opts: { aliases?: string[]; canonicals?: string[]; known?: string[] }): { + aliases: Set; + canonicals: Set; + knownAliases: Set; +} { + return { + aliases: new Set(opts.aliases ?? []), + canonicals: new Set(opts.canonicals ?? []), + // knownAliases is the union of every prefix the universe is aware of — + // usable or not. Default to including the usable aliases too. + knownAliases: new Set([...(opts.known ?? []), ...(opts.aliases ?? [])]), + }; +} + +function combo(models: OmniRouteRawCombo["models"]): OmniRouteRawCombo { + return { id: "c1", name: "Test Combo", models }; +} + +test("isUsableCombo: member with a usable alias prefix → keep", () => { + const usable = buildUsable({ aliases: ["cc"], known: ["cc", "dead"] }); + const c = combo([{ kind: "model", model: "cc/claude-opus-4-7" }]); + assert.equal(isUsableCombo(c, usable), true); +}); + +test("isUsableCombo: all members known-but-NOT-usable → drop (the C1 regression)", () => { + // Before the fix this returned true unconditionally because step.providerId + // was always undefined. Now the known-but-unusable "dead" prefix is dropped. + const usable = buildUsable({ aliases: ["cc"], known: ["cc", "dead"] }); + const c = combo([ + { kind: "model", model: "dead/legacy-model" }, + { kind: "model", model: "dead/another" }, + ]); + assert.equal(isUsableCombo(c, usable), false); +}); + +test("isUsableCombo: unknown prefix → keep (cannot prove unroutable)", () => { + const usable = buildUsable({ aliases: ["cc"], known: ["cc", "dead"] }); + const c = combo([{ kind: "model", model: "agentrouter/mystery" }]); + assert.equal(isUsableCombo(c, usable), true); +}); + +test("isUsableCombo: mixed non-usable + usable member → keep", () => { + const usable = buildUsable({ aliases: ["cc"], known: ["cc", "dead"] }); + const c = combo([ + { kind: "model", model: "dead/legacy" }, + { kind: "model", model: "cc/claude-opus-4-7" }, + ]); + assert.equal(isUsableCombo(c, usable), true); +}); + +test("isUsableCombo: zero members → keep", () => { + const usable = buildUsable({ aliases: ["cc"], known: ["cc"] }); + assert.equal(isUsableCombo(combo([]), usable), true); + assert.equal(isUsableCombo(combo(undefined), usable), true); +}); + +test("isUsableCombo: only combo-ref steps (no resolvable model) → keep", () => { + const usable = buildUsable({ aliases: ["cc"], known: ["cc", "dead"] }); + const c = combo([{ kind: "combo-ref", comboName: "nested" }]); + assert.equal(isUsableCombo(c, usable), true); +}); + +test("isUsableCombo: usable canonical prefix → keep", () => { + const usable = buildUsable({ canonicals: ["anthropic"], known: ["anthropic", "dead"] }); + const c = combo([{ kind: "model", model: "anthropic/claude-opus-4-7" }]); + assert.equal(isUsableCombo(c, usable), true); +}); diff --git a/docs/security/STEALTH_GUIDE.md b/docs/security/STEALTH_GUIDE.md index 9ff970c577..02d4bd1210 100644 --- a/docs/security/STEALTH_GUIDE.md +++ b/docs/security/STEALTH_GUIDE.md @@ -88,7 +88,7 @@ Applied to: `system` blocks, all `messages[].content`, and `tools[].description` For third-party Anthropic relays that only accept "real Claude Code" traffic: -- `CLAUDE_CODE_COMPATIBLE_USER_AGENT = "claude-cli/2.1.137 (external, sdk-cli)"` +- `CLAUDE_CODE_COMPATIBLE_USER_AGENT = "claude-cli/2.1.146 (external, sdk-cli)"` - `CLAUDE_CODE_COMPATIBLE_STAINLESS_PACKAGE_VERSION = "0.81.0"` - `CLAUDE_CODE_COMPATIBLE_STAINLESS_RUNTIME_VERSION = "v24.3.0"` - `anthropic-beta = "claude-code-20250219,interleaved-thinking-2025-05-14,effort-2025-11-24"` @@ -212,7 +212,7 @@ All MITM endpoints require management auth (`requireCliToolsAuth`). The sudo pas | Variable | Default | | ------------------------ | --------------------------------------------- | -| `CLAUDE_USER_AGENT` | `claude-cli/2.1.145 (external, cli)` | +| `CLAUDE_USER_AGENT` | `claude-cli/2.1.146 (external, cli)` | | `CODEX_USER_AGENT` | `codex-cli/0.132.0 (Windows 10.0.26200; x64)` | | `GITHUB_USER_AGENT` | `GitHubCopilotChat/0.45.1` | | `ANTIGRAVITY_USER_AGENT` | `antigravity/2.0.1 darwin/arm64` | diff --git a/open-sse/config/antigravityModelAliases.ts b/open-sse/config/antigravityModelAliases.ts index eccc1be5b4..8f3551e585 100644 --- a/open-sse/config/antigravityModelAliases.ts +++ b/open-sse/config/antigravityModelAliases.ts @@ -45,33 +45,6 @@ export const ANTIGRAVITY_PUBLIC_MODELS = Object.freeze([ supportsVision: true, toolCalling: true, }, - { - id: "gemini-3.5-flash-preview", - name: "Gemini 3.5 Flash (High)", - contextLength: 1048576, - maxOutputTokens: 65536, - supportsReasoning: true, - supportsVision: true, - toolCalling: true, - }, - { - id: "gemini-3-flash-agent", - name: "Gemini 3.5 Flash (High)", - contextLength: 1048576, - maxOutputTokens: 65536, - supportsReasoning: true, - supportsVision: true, - toolCalling: true, - }, - { - id: "gemini-3.5-flash-low", - name: "Gemini 3.5 Flash (Low)", - contextLength: 1048576, - maxOutputTokens: 65536, - supportsReasoning: true, - supportsVision: true, - toolCalling: true, - }, { id: "gemini-3-flash-preview", name: "Gemini 3 Flash", diff --git a/open-sse/config/embeddingRegistry.ts b/open-sse/config/embeddingRegistry.ts index 7379f777ed..549c503d6a 100644 --- a/open-sse/config/embeddingRegistry.ts +++ b/open-sse/config/embeddingRegistry.ts @@ -327,7 +327,12 @@ export function parseEmbeddingModel( * Get all embedding models as a flat list */ export function getAllEmbeddingModels() { - const models = []; + const models: Array<{ + id: string; + name: string; + provider: string; + dimensions: number | undefined; + }> = []; for (const [providerId, config] of Object.entries(EMBEDDING_PROVIDERS)) { for (const model of config.models) { models.push({ diff --git a/open-sse/services/accountFallback.ts b/open-sse/services/accountFallback.ts index e172760748..6798434c5e 100644 --- a/open-sse/services/accountFallback.ts +++ b/open-sse/services/accountFallback.ts @@ -174,6 +174,22 @@ const MODEL_ACCESS_DENIED_PATTERNS = [ /\bmodel[\s\S]{0,60}?\b(?:access|permission)\b/i, ]; +// Pure credential/authentication failures — the key or token itself is bad, which +// is NOT a model-availability problem. Some providers phrase these as a 400 that +// also mentions the model (e.g. "invalid api key for model X"), which would +// otherwise trip MODEL_ACCESS_DENIED_PATTERNS above and trigger combo fallback +// across every target, masking the real "fix your credential" error. When the +// text clearly indicates a bad credential, the regex-based model-access detection +// is suppressed (structured codes/types like model_not_found are unaffected). +const AUTH_CREDENTIAL_ERROR_PATTERNS = [ + /\b(?:invalid|incorrect|expired|missing|revoked)\s+api[\s_-]?key\b/i, + /\bapi[\s_-]?key\s+(?:is\s+)?(?:invalid|incorrect|expired|missing|revoked|not\s+valid)\b/i, + /\bauthentication\s+(?:failed|error|required)\b/i, + /\b(?:invalid|expired|missing|revoked)\s+(?:token|credentials?|bearer)\b/i, + /\bunauthorized\b/i, + /\bnot\s+authenticated\b/i, +]; + // Malformed request patterns — the model rejected the message format but a different // provider/model in the combo may accept it. const MALFORMED_REQUEST_PATTERNS = [ @@ -1270,7 +1286,14 @@ export function checkFallbackError( typeof structuredError?.code === "string" ? structuredError.code.toLowerCase() : ""; const structuredType = typeof structuredError?.type === "string" ? structuredError.type.toLowerCase() : ""; - const matchesModelAccessPattern = MODEL_ACCESS_DENIED_PATTERNS.some((p) => p.test(errorStr)); + // A clear bad-credential error must never be reclassified as model-access + // (which would silently exhaust every combo target). Structured detection + // below still catches genuine model_not_found / not_found_error codes. + const looksLikeAuthCredentialError = AUTH_CREDENTIAL_ERROR_PATTERNS.some((p) => + p.test(errorStr) + ); + const matchesModelAccessPattern = + !looksLikeAuthCredentialError && MODEL_ACCESS_DENIED_PATTERNS.some((p) => p.test(errorStr)); const isModelAccessDeniedStructured = !!structuredError && @@ -1344,7 +1367,9 @@ export function getEarliestRateLimitedUntil( /** * Format rateLimitedUntil to human-readable "reset after Xm Ys" */ -export function formatRetryAfter(rateLimitedUntil: string | Date | null | undefined): string { +export function formatRetryAfter( + rateLimitedUntil: string | number | Date | null | undefined +): string { if (!rateLimitedUntil) return ""; const diffMs = new Date(rateLimitedUntil).getTime() - Date.now(); if (diffMs <= 0) return "reset after 0s"; diff --git a/open-sse/services/autoCombo/modePacks.ts b/open-sse/services/autoCombo/modePacks.ts index fe0d23ac95..e75e29327b 100644 --- a/open-sse/services/autoCombo/modePacks.ts +++ b/open-sse/services/autoCombo/modePacks.ts @@ -12,6 +12,7 @@ import type { ScoringWeights } from "./scoring"; export const MODE_PACKS: Record = { // Prioritize latency → health. tierPriority replaces 0.05 from stability. + // tierAffinity/specificityMatch stay at 0 (manifest-routing-only weights). "ship-fast": { quota: 0.15, health: 0.3, @@ -20,6 +21,8 @@ export const MODE_PACKS: Record = { taskFit: 0.1, stability: 0.0, tierPriority: 0.05, + tierAffinity: 0, + specificityMatch: 0, }, // Prioritize cost. tierPriority replaces 0.05 from stability. "cost-saver": { @@ -30,6 +33,8 @@ export const MODE_PACKS: Record = { taskFit: 0.1, stability: 0.05, tierPriority: 0.05, + tierAffinity: 0, + specificityMatch: 0, }, // Prioritize task fitness. tierPriority replaces 0.05 from latencyInv. "quality-first": { @@ -40,6 +45,8 @@ export const MODE_PACKS: Record = { taskFit: 0.4, stability: 0.15, tierPriority: 0.05, + tierAffinity: 0, + specificityMatch: 0, }, // Prioritize quota availability. tierPriority replaces 0.05 from taskFit. "offline-friendly": { @@ -50,6 +57,8 @@ export const MODE_PACKS: Record = { taskFit: 0.0, stability: 0.1, tierPriority: 0.05, + tierAffinity: 0, + specificityMatch: 0, }, }; diff --git a/open-sse/services/autoCombo/pipelineRouter.ts b/open-sse/services/autoCombo/pipelineRouter.ts index 828ab263a7..e957fd83f7 100644 --- a/open-sse/services/autoCombo/pipelineRouter.ts +++ b/open-sse/services/autoCombo/pipelineRouter.ts @@ -277,8 +277,21 @@ export async function handlePipelineCombo({ const pipelineConfig = buildPipelineConfig(promptText, taskType); // ── Extract available models from combo ──────────────────────────────────── - const comboModels = (combo as Record).models as string[] | undefined; - const availableModels = comboModels?.length ? comboModels : ["deepseek-chat"]; + // `combo.models` is an array of model-config entries (objects with a `.model` + // field), not plain strings — older code cast it to `string[]` and passed the + // raw objects to `getTaskFitness`, which then string-matched `"[object Object]"` + // and always fell back to the default model. Normalize to model-name strings. + const rawComboModels = (combo as Record).models; + const comboModels = Array.isArray(rawComboModels) + ? rawComboModels + .map((entry) => { + if (typeof entry === "string") return entry; + const model = (entry as Record | null)?.model; + return typeof model === "string" ? model : null; + }) + .filter((model): model is string => typeof model === "string" && model.length > 0) + : []; + const availableModels = comboModels.length ? comboModels : ["deepseek-chat"]; // ── Create stage executor ───────────────────────────────────────────────── const stageExecutor = createStageExecutor(body, handleChatCore, log, availableModels, taskType); @@ -289,31 +302,36 @@ export async function handlePipelineCombo({ ((settings as Record).max_reflection_loops as number) ?? 1; - // Track reflection loops - let reflectionCount = 0; const wrappedExecutor = async (args: StageExecutorArgs) => { // fitnessTier is now passed by the pipeline engine via StageExecutorArgs return stageExecutor({ ...args, fitnessTier: args.fitnessTier as FitnessTier | undefined }); }; - const result = await executePipeline(pipelineConfig, wrappedExecutor); + let result = await executePipeline(pipelineConfig, wrappedExecutor); // ── Handle reflection loops ─────────────────────────────────────────────── - // If reflect failed and we haven't exceeded max loops, re-run execute+reflect - if (result.reflectVerdict === "fail" && reflectionCount < maxReflectionLoops) { + // While the reflect stage reports "fail", re-run the whole pipeline up to + // `maxReflectionLoops` times (previously this was a single `if`, so the + // configured loop count above 1 was silently ignored). Each re-run is a fresh + // pipeline that internally applies its own reflect→fix correction (see + // executePipeline). The first retry that passes wins; if none pass we keep the + // original result. + let reflectionCount = 0; + while (result.reflectVerdict === "fail" && reflectionCount < maxReflectionLoops) { reflectionCount++; log.info( "PIPELINE", `Reflection failed, re-running (loop ${reflectionCount}/${maxReflectionLoops})` ); - // Re-execute with corrected context from reflection const retryConfig = buildPipelineConfig(promptText, taskType); const retryResult = await executePipeline(retryConfig, wrappedExecutor); - // Use retry result if it passed, otherwise keep original + // Adopt the retry only if it passed; otherwise keep scanning until the loop + // budget is exhausted, then fall through with the original result. if (retryResult.reflectVerdict === "pass") { - return retryResult; + result = retryResult; + break; } } @@ -334,3 +352,67 @@ export async function handlePipelineCombo({ ); return result; } + +/** + * Convert a buffered {@link PipelineResult} into an HTTP `Response`. + * + * `handlePipelineCombo` resolves to a `PipelineResult` (the pipeline buffers + * every stage as non-streaming and exposes only the final text), but the combo + * routing layer hands its return value to callers that expect a `Response`. + * This adapter bridges that gap, emitting an OpenAI-compatible + * `chat.completion` body — or a single-chunk SSE stream when the client + * requested `stream: true` — so the pipeline output actually reaches the client + * instead of being silently dropped. + */ +export function buildPipelineResponse( + result: PipelineResult, + body: Record +): Response { + const model = typeof body.model === "string" && body.model.length > 0 ? body.model : "auto"; + const id = `chatcmpl-pipeline-${Date.now()}`; + const created = Math.floor(Date.now() / 1000); + const content = result.text ?? ""; + + // Non-streaming: standard OpenAI chat.completion JSON body. + if (body.stream !== true) { + const payload = { + id, + object: "chat.completion", + created, + model, + choices: [ + { + index: 0, + message: { role: "assistant", content }, + finish_reason: "stop", + }, + ], + usage: { prompt_tokens: 0, completion_tokens: 0, total_tokens: 0 }, + }; + return new Response(JSON.stringify(payload), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + + // Streaming: emit one content chunk, a terminal finish chunk, then [DONE]. + const chunk = (delta: Record, finishReason: string | null) => + `data: ${JSON.stringify({ + id, + object: "chat.completion.chunk", + created, + model, + choices: [{ index: 0, delta, finish_reason: finishReason }], + })}\n\n`; + + const sse = chunk({ role: "assistant", content }, null) + chunk({}, "stop") + "data: [DONE]\n\n"; + + return new Response(sse, { + status: 200, + headers: { + "Content-Type": "text/event-stream; charset=utf-8", + "Cache-Control": "no-cache", + Connection: "keep-alive", + }, + }); +} diff --git a/open-sse/services/codexQuotaFetcher.ts b/open-sse/services/codexQuotaFetcher.ts index fe2d421d9b..a551c272c8 100644 --- a/open-sse/services/codexQuotaFetcher.ts +++ b/open-sse/services/codexQuotaFetcher.ts @@ -143,7 +143,7 @@ function getDominantResetAt(quota: { export async function fetchCodexQuota( connectionId: string, connection?: Record -): Promise { +): Promise { // Check cache first const cached = quotaCache.get(connectionId); if (cached && Date.now() - cached.fetchedAt < CACHE_TTL_MS) { diff --git a/open-sse/services/combo.ts b/open-sse/services/combo.ts index 92d44b9b25..b763ae804a 100644 --- a/open-sse/services/combo.ts +++ b/open-sse/services/combo.ts @@ -29,7 +29,7 @@ import { selectProvider as selectAutoProvider } from "./autoCombo/engine.ts"; import { selectWithStrategy } from "./autoCombo/routerStrategy.ts"; import { getTaskFitness } from "./autoCombo/taskFitness.ts"; import { parseAutoPrefix } from "./autoCombo/autoPrefix.ts"; -import { handlePipelineCombo } from "./autoCombo/pipelineRouter.ts"; +import { handlePipelineCombo, buildPipelineResponse } from "./autoCombo/pipelineRouter.ts"; import { calculateFactors, calculateScore, @@ -292,7 +292,7 @@ function buildExecutionKey(path: string[], stepId: string): string { return [...path, stepId].join(">"); } -function normalizeRuntimeStep(entry, comboName, index, allCombos, path = []) { +function normalizeRuntimeStep(entry, comboName, index, allCombos, path: string[] = []) { const step = normalizeComboStep(entry, { comboName, index, @@ -337,7 +337,7 @@ function getDirectComboTargets(combo) { ); } -function getTopLevelRuntimeSteps(combo, allCombos, path = []) { +function getTopLevelRuntimeSteps(combo, allCombos, path: string[] = []) { return (combo.models || []) .map((entry, index) => normalizeRuntimeStep(entry, combo.name, index, allCombos, path)) .filter((entry): entry is ComboRuntimeStep => entry !== null); @@ -364,10 +364,10 @@ function getCompositeTierStepOrder(combo): string[] { if (!normalizedTierName || !stepId) return null; return [normalizedTierName, { stepId, fallbackTier }] as const; }) - .filter(Boolean) + .filter((entry): entry is NonNullable => entry !== null) ); - let currentTier = defaultTier; + let currentTier: string | null = defaultTier; while (currentTier && tierEntries.has(currentTier) && !visitedTiers.has(currentTier)) { visitedTiers.add(currentTier); const entry = tierEntries.get(currentTier); @@ -417,11 +417,11 @@ function orderRuntimeStepsByCompositeTiers(steps: ComboRuntimeStep[], combo): Co return ordered; } -function getOrderedTopLevelRuntimeSteps(combo, allCombos, path = []) { +function getOrderedTopLevelRuntimeSteps(combo, allCombos, path: string[] = []) { return orderRuntimeStepsByCompositeTiers(getTopLevelRuntimeSteps(combo, allCombos, path), combo); } -function expandRuntimeStep(step, allCombos, visited = new Set(), depth = 0, path = []) { +function expandRuntimeStep(step, allCombos, visited = new Set(), depth = 0, path: string[] = []) { if (step.kind === "model") return [step]; if (depth > MAX_COMBO_DEPTH) return []; @@ -440,7 +440,7 @@ export function resolveNestedComboTargets( allCombos, visited = new Set(), depth = 0, - path = [] + path: string[] = [] ) { const directTargets = (combo.models || []) .map((entry, index) => normalizeRuntimeStep(entry, combo.name, index, null, path)) @@ -534,7 +534,7 @@ export function resolveNestedComboModels(combo, allCombos, visited = new Set(), visited.add(combo.name); const combos = Array.isArray(allCombos) ? allCombos : allCombos?.combos || []; - const resolved = []; + const resolved: string[] = []; for (const entry of combo.models || []) { const modelName = normalizeModelEntry(entry).model; @@ -1417,6 +1417,7 @@ function resolveWeightedTargets(combo, allCombos) { hasCompositeTierRuntimeOrder(combo) ); const expandedTargets = orderedSteps.flatMap((step) => { + if (!step) return []; if (!allCombos) { return step.kind === "model" ? [step] : []; } @@ -1448,7 +1449,7 @@ function scoreAutoTargets( const factors = calculateFactors( candidate as ProviderCandidate, candidates, - taskType, + taskType ?? "", getTaskFitness ); return { @@ -1456,7 +1457,7 @@ function scoreAutoTargets( score: calculateScore(factors, weights), }; }) - .filter(Boolean) + .filter((entry): entry is NonNullable => entry !== null) .sort((a, b) => b.score - a.score); } @@ -1717,7 +1718,7 @@ export async function handleComboChat({ const autoVariant = autoParsed.valid ? autoParsed.variant : undefined; if (autoVariant === "smart" || config.pipeline_enabled) { try { - return await handlePipelineCombo({ + const pipelineRaw = await handlePipelineCombo({ body, combo, handleChatCore: handleSingleModel, @@ -1725,9 +1726,22 @@ export async function handleComboChat({ settings, signal, }); + // handlePipelineCombo resolves to a PipelineResult (buffered text) or, + // in the streaming-final-stage case, a Response. Callers downstream + // (chat.ts → withSessionHeader) require a Response, so adapt the + // PipelineResult here instead of leaking the raw object. + return pipelineRaw instanceof Response + ? pipelineRaw + : buildPipelineResponse(pipelineRaw, body); } catch (pipelineErr) { - if (pipelineErr instanceof Error && pipelineErr.message === "PIPELINE_DISABLED") { + const pipelineMsg = pipelineErr instanceof Error ? pipelineErr.message : ""; + if (pipelineMsg === "PIPELINE_DISABLED") { log.info("COMBO", "Pipeline disabled, falling through to standard auto routing"); + } else if (pipelineMsg === "PIPELINE_TOKEN_THRESHOLD") { + log.info( + "COMBO", + "Pipeline skipped (prompt below token threshold), falling through to standard auto routing" + ); } else { log.warn("COMBO", "Pipeline dispatch failed, falling through to standard auto routing", { err: pipelineErr, @@ -1825,8 +1839,8 @@ export async function handleComboChat({ const candidates = await buildAutoCandidates(eligibleTargets, combo.name); if (candidates.length > 0) { - let selectedProvider = null; - let selectedModel = null; + let selectedProvider: string | null = null; + let selectedModel: string | null = null; let selectionReason = ""; if (routingStrategy !== "rules") { @@ -2047,9 +2061,9 @@ export async function handleComboChat({ } } - let lastError = null; - let earliestRetryAfter = null; - let lastStatus = null; + let lastError: string | null = null; + let earliestRetryAfter: string | null = null; + let lastStatus: number | null = null; const startTime = Date.now(); let fallbackCount = 0; let recordedAttempts = 0; @@ -2226,8 +2240,8 @@ export async function handleComboChat({ // Extract error info from response let errorText = result.statusText || ""; - let errorBody = null; - let retryAfter = null; + let errorBody: any = null; + let retryAfter: string | null = null; try { const cloned = result.clone(); try { @@ -2462,9 +2476,9 @@ async function handleRoundRobinCombo({ const clientRequestedStream = body?.stream === true; const startTime = Date.now(); - let lastError = null; - let lastStatus = null; - let earliestRetryAfter = null; + let lastError: string | null = null; + let lastStatus: number | null = null; + let earliestRetryAfter: string | number | null = null; let globalAttempts = 0; let fallbackCount = 0; let recordedAttempts = 0; @@ -2613,7 +2627,7 @@ async function handleRoundRobinCombo({ // Extract error info let errorText = result.statusText || ""; - let retryAfter = null; + let retryAfter: string | number | null = null; let errorBody: { error?: { code?: string | null; message?: string | null } | string; message?: string | null; diff --git a/open-sse/utils/error.ts b/open-sse/utils/error.ts index 9cf1bf10ef..5061fb1ddf 100644 --- a/open-sse/utils/error.ts +++ b/open-sse/utils/error.ts @@ -212,8 +212,8 @@ export function parseAntigravityRetryTime(message) { */ export async function parseUpstreamError(response, provider = null) { let message = ""; - let retryAfterMs = null; - let responseBody = null; + let retryAfterMs: number | null = null; + let responseBody: unknown = null; let errorCode = undefined; let errorType = undefined; diff --git a/src/app/api/cache/stats/route.ts b/src/app/api/cache/stats/route.ts index 2d5436d83a..bdeedb7bec 100644 --- a/src/app/api/cache/stats/route.ts +++ b/src/app/api/cache/stats/route.ts @@ -1,6 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { getPromptCache } from "@/lib/cacheLayer"; import { isAuthenticated } from "@/shared/utils/apiAuth"; +import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error.ts"; export async function GET(req: NextRequest) { if (!(await isAuthenticated(req))) { @@ -12,7 +13,7 @@ export async function GET(req: NextRequest) { const stats = cache.getStats(); return NextResponse.json(stats); } catch (error) { - return NextResponse.json({ error: (error as Error).message }, { status: 500 }); + return NextResponse.json({ error: sanitizeErrorMessage(error) }, { status: 500 }); } } @@ -26,6 +27,6 @@ export async function DELETE(req: NextRequest) { cache.clear(); return NextResponse.json({ success: true, message: "Cache cleared" }); } catch (error) { - return NextResponse.json({ error: (error as Error).message }, { status: 500 }); + return NextResponse.json({ error: sanitizeErrorMessage(error) }, { status: 500 }); } } diff --git a/src/app/api/cli-tools/codex-profiles/route.ts b/src/app/api/cli-tools/codex-profiles/route.ts index 65126fd282..78c99d7068 100644 --- a/src/app/api/cli-tools/codex-profiles/route.ts +++ b/src/app/api/cli-tools/codex-profiles/route.ts @@ -171,7 +171,7 @@ export async function POST(request) { }; await ensureProfilesDir(); - const profilePath = path.join(PROFILES_DIR, `${profileId}.json`); + const profilePath = safeProfilePath(`${profileId}.json`); await fs.writeFile(profilePath, JSON.stringify(profile, null, 2)); return NextResponse.json({ @@ -217,7 +217,7 @@ export async function PUT(request) { } const { profileId } = validation.data; - const profilePath = path.join(PROFILES_DIR, `${profileId}.json`); + const profilePath = safeProfilePath(`${profileId}.json`); let profile; try { const raw = await fs.readFile(profilePath, "utf-8"); @@ -286,7 +286,7 @@ export async function DELETE(request) { } const { profileId } = validation.data; - const profilePath = path.join(PROFILES_DIR, `${profileId}.json`); + const profilePath = safeProfilePath(`${profileId}.json`); try { await fs.unlink(profilePath); } catch (err) { diff --git a/src/app/api/cli-tools/hermes-agent-settings/route.ts b/src/app/api/cli-tools/hermes-agent-settings/route.ts index f8a8d0c5de..30d0f270ce 100644 --- a/src/app/api/cli-tools/hermes-agent-settings/route.ts +++ b/src/app/api/cli-tools/hermes-agent-settings/route.ts @@ -4,10 +4,12 @@ import path from "path"; import os from "os"; import { requireCliToolsAuth } from "@/lib/api/requireCliToolsAuth"; import { getCliPrimaryConfigPath } from "@/shared/services/cliRuntime"; +import { validateBaseUrl } from "@/lib/cli-helper/config-generator"; import { generateHermesAgentConfig, getCurrentHermesAgentRoles, } from "@/lib/cli-helper/config-generator/hermes-agent"; +import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error.ts"; /** * Dedicated endpoint for Hermes Agent (the advanced Nous Research terminal agent). @@ -43,7 +45,10 @@ export async function GET(request: Request) { return NextResponse.json({ success: true, roles, firstSetupAt }); } catch (error) { - return NextResponse.json({ success: false, error: String(error) }, { status: 500 }); + return NextResponse.json( + { success: false, error: sanitizeErrorMessage(error) }, + { status: 500 } + ); } } @@ -64,6 +69,10 @@ export async function POST(request: Request) { return NextResponse.json({ error: "baseUrl is required" }, { status: 400 }); } + if (typeof baseUrl !== "string" || !validateBaseUrl(baseUrl)) { + return NextResponse.json({ error: "baseUrl must be a valid http(s) URL" }, { status: 400 }); + } + if (!Array.isArray(selections) || selections.length === 0) { return NextResponse.json( { error: "selections must be a non-empty array of { role, model }" }, diff --git a/src/app/api/db-backups/import/route.ts b/src/app/api/db-backups/import/route.ts index 2afcfccce6..4dfbe9cf90 100644 --- a/src/app/api/db-backups/import/route.ts +++ b/src/app/api/db-backups/import/route.ts @@ -159,7 +159,7 @@ export async function POST(request: Request) { // The DB was replaced wholesale — re-hydrate the in-memory Global System Prompt so it // reflects the imported settings without requiring a restart (#2470). try { - const importedSettings = getSettings(); + const importedSettings = await getSettings(); if (importedSettings.systemPrompt) { setSystemPromptConfig(importedSettings.systemPrompt); } diff --git a/src/app/api/memory/route.ts b/src/app/api/memory/route.ts index ea7d413f7c..61d076b91d 100644 --- a/src/app/api/memory/route.ts +++ b/src/app/api/memory/route.ts @@ -1,12 +1,11 @@ import { NextResponse } from "next/server"; import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; -import { listMemories, createMemory } from "@/lib/memory/store"; +import { listMemories, createMemory, getMemoryTokensUsed } from "@/lib/memory/store"; import { memoryCache } from "@/lib/memory/cache"; import { MemoryType } from "@/lib/memory/types"; import { parsePaginationParams, buildPaginatedResponse } from "@/shared/types/pagination"; import { z } from "zod"; import { validateBody, isValidationFailure } from "@/shared/validation/helpers"; -import { getDbInstance } from "@/lib/db/core"; const createMemorySchema = z.object({ content: z.string().min(1), @@ -48,15 +47,9 @@ export async function GET(request: Request) { page: offset === undefined ? paginationParams.page : undefined, }); - // Compute total tokens across all memories using SQL (avoids loading all content into memory) - const db = getDbInstance(); - const tokenResult = db - .prepare( - "SELECT COALESCE(SUM((LENGTH(content) + 3) / 4), 0) as tokensUsed FROM memories" + - (apiKeyId ? " WHERE api_key_id = ?" : "") - ) - .get(...(apiKeyId ? [apiKeyId] : [])) as { tokensUsed: number }; - const tokensUsed = tokenResult?.tokensUsed ?? 0; + // Total tokens across all memories (computed in SQL inside the domain module + // to avoid loading every memory's content into process memory). + const tokensUsed = getMemoryTokensUsed(apiKeyId); // Compute hit rate from memory cache const cacheStats = memoryCache.stats(); diff --git a/src/lib/cloudAgent/credentials.ts b/src/lib/cloudAgent/credentials.ts index 177ba0a7ae..c7f739d21d 100644 --- a/src/lib/cloudAgent/credentials.ts +++ b/src/lib/cloudAgent/credentials.ts @@ -2,21 +2,10 @@ import { getDbInstance } from "@/lib/db/core"; import { encrypt, decrypt } from "@/lib/db/encryption"; import type { AgentCredentials } from "@/lib/cloudAgent/baseAgent"; -/** - * Ensure cloud_agent_credentials table exists. - * Should be replaced by a proper migration in db/migrations/. - */ -export function ensureCredentialsTable(): void { - const db = getDbInstance(); - db.exec(` - CREATE TABLE IF NOT EXISTS cloud_agent_credentials ( - provider_id TEXT PRIMARY KEY, - api_key_encrypted TEXT NOT NULL, - base_url TEXT, - updated_at TEXT NOT NULL DEFAULT (datetime('now')) - ) - `); -} +// The `cloud_agent_credentials` table is provisioned by migration +// `061_cloud_agent_credentials.sql` at database initialization (see +// src/lib/db/migrations/). Do not create it inline here — the project +// migration policy requires versioned, transaction-wrapped DDL. /** Mask API key for display — show last 4 chars only */ export function maskApiKey(key: string): string { @@ -26,7 +15,6 @@ export function maskApiKey(key: string): string { /** Get decrypted credentials for a provider */ export function getCloudAgentCredentialFromDb(providerId: string): AgentCredentials | null { - ensureCredentialsTable(); const db = getDbInstance(); const row = db .prepare( @@ -51,7 +39,6 @@ export function listCloudAgentCredentials(): Array<{ baseUrl: string | null; updatedAt: string; }> { - ensureCredentialsTable(); const db = getDbInstance(); const rows = db .prepare( @@ -81,7 +68,6 @@ export function saveCloudAgentCredential( apiKey: string, baseUrl?: string ): void { - ensureCredentialsTable(); const encrypted = encrypt(apiKey); if (!encrypted) throw new Error("Failed to encrypt API key"); @@ -98,7 +84,6 @@ export function saveCloudAgentCredential( /** Delete credentials for a provider */ export function deleteCloudAgentCredential(providerId: string): void { - ensureCredentialsTable(); const db = getDbInstance(); db.prepare("DELETE FROM cloud_agent_credentials WHERE provider_id = ?").run(providerId); } diff --git a/src/lib/db/adapters/betterSqliteAdapter.ts b/src/lib/db/adapters/betterSqliteAdapter.ts index 42b1c96599..899ae99d49 100644 --- a/src/lib/db/adapters/betterSqliteAdapter.ts +++ b/src/lib/db/adapters/betterSqliteAdapter.ts @@ -37,8 +37,8 @@ export function createBetterSqliteAdapter(db: import("better-sqlite3").Database) (db.transaction(fn) as unknown as { immediate: () => void }).immediate(); }, - backup(destination: string): Promise { - return db.backup(destination); + async backup(destination: string): Promise { + await db.backup(destination); }, checkpoint(mode = "TRUNCATE"): void { diff --git a/src/lib/db/migrations/061_cloud_agent_credentials.sql b/src/lib/db/migrations/061_cloud_agent_credentials.sql new file mode 100644 index 0000000000..c6d3bdd3fb --- /dev/null +++ b/src/lib/db/migrations/061_cloud_agent_credentials.sql @@ -0,0 +1,9 @@ +-- Migration 061: Cloud agent credentials (encrypted API keys for cloud coding agents) +-- Previously created inline via ensureCredentialsTable() in src/lib/cloudAgent/credentials.ts; +-- promoted to a proper versioned migration per the project migration policy. +CREATE TABLE IF NOT EXISTS cloud_agent_credentials ( + provider_id TEXT PRIMARY KEY, + api_key_encrypted TEXT NOT NULL, + base_url TEXT, + updated_at TEXT NOT NULL DEFAULT (datetime('now')) +); diff --git a/src/lib/embeddings/service.ts b/src/lib/embeddings/service.ts index e6e9b45b8f..74db7bb51a 100644 --- a/src/lib/embeddings/service.ts +++ b/src/lib/embeddings/service.ts @@ -37,7 +37,7 @@ export async function createEmbeddingResponse( try { const combo = await getComboByName(modelStr); if (combo) { - let allCombos = []; + let allCombos: any[] = []; try { allCombos = await getCombos(); } catch {} @@ -57,9 +57,11 @@ export async function createEmbeddingResponse( connectionId: target?.connectionId || options.connectionId, }); }, + isModelAvailable: undefined, log, settings, allCombos, + relayOptions: undefined, signal: undefined, }); } @@ -146,7 +148,7 @@ export async function createEmbeddingResponse( ); } - let credentials = null; + let credentials: Awaited> | null = null; if (providerConfig.authType !== "none") { credentials = await getProviderCredentials(credentialsProviderId); if (!credentials) { @@ -167,7 +169,10 @@ export async function createEmbeddingResponse( const result = await handleEmbedding({ body, - credentials, + // getProviderCredentials returns a richer connection object; handleEmbedding + // only reads apiKey/accessToken, both present at runtime. Bridge the wider + // selection type to the handler's narrow credential shape. + credentials: credentials as { apiKey?: string; accessToken?: string } | null, log, resolvedProvider: providerConfig, resolvedModel, diff --git a/src/lib/memory/store.ts b/src/lib/memory/store.ts index 5f9542d410..b32cbea8d4 100644 --- a/src/lib/memory/store.ts +++ b/src/lib/memory/store.ts @@ -426,3 +426,18 @@ export async function listMemories(filters: { byType, }; } + +/** + * Total estimated tokens across stored memories (4 chars ≈ 1 token), computed in + * SQL so we never load every memory's content into process memory. Scoped to a + * single API key when `apiKeyId` is provided, otherwise counts all memories. + */ +export function getMemoryTokensUsed(apiKeyId?: string): number { + const db = getDbInstance(); + const stmt = db.prepare( + "SELECT COALESCE(SUM((LENGTH(content) + 3) / 4), 0) as tokensUsed FROM memories" + + (apiKeyId ? " WHERE api_key_id = ?" : "") + ); + const row = stmt.get(...(apiKeyId ? [apiKeyId] : [])) as { tokensUsed: number } | undefined; + return row?.tokensUsed ?? 0; +} diff --git a/src/shared/validation/schemas.ts b/src/shared/validation/schemas.ts index 216f8e16b9..ef6ea33ddd 100644 --- a/src/shared/validation/schemas.ts +++ b/src/shared/validation/schemas.ts @@ -1944,7 +1944,15 @@ export const codexProfileNameSchema = z.object({ }); export const codexProfileIdSchema = z.object({ - profileId: z.string().trim().min(1, "profileId is required"), + // profileId is interpolated into a filesystem path (`/.json`). + // Constrain to a safe slug charset so request bodies cannot smuggle path + // separators or `..` segments and escape PROFILES_DIR (path traversal). + profileId: z + .string() + .trim() + .min(1, "profileId is required") + .regex(/^[a-zA-Z0-9._-]+$/, "profileId contains invalid characters") + .refine((v) => v !== "." && v !== "..", "profileId is invalid"), }); export const guideSettingsSaveSchema = z diff --git a/tests/integration/pipeline-combo.test.ts b/tests/integration/pipeline-combo.test.ts index e55eba1999..8023088db5 100644 --- a/tests/integration/pipeline-combo.test.ts +++ b/tests/integration/pipeline-combo.test.ts @@ -385,3 +385,42 @@ test("pipeline-combo: reflection fail triggers re-execution with corrected conte // Should have made multiple calls due to reflection retry assert.ok(calls.length >= 3, `Expected >= 3 calls for retry, got ${calls.length}`); }); + +test("pipeline-combo: max_reflection_loops>1 re-runs the pipeline that many times (regression: loop count was silently ignored)", async () => { + // Reflect ALWAYS fails, so the outer reflection loop should keep re-running + // the whole pipeline until the configured budget is exhausted. Each pipeline + // run hits the reflect stage exactly once, so reflect calls == loops + 1. + const reflectCallsFor = async (maxLoops: number): Promise => { + let reflectCalls = 0; + const { handler } = createMockHandleChatCore((body) => { + const messages = body.messages as Array<{ role: string; content: string }>; + const systemMsg = messages.find((m) => m.role === "system")?.content || ""; + if (systemMsg.includes("quality reviewer")) { + reflectCalls++; + return '{"status":"fail","issues":["always fails"],"corrected":"c"}'; + } + return "execute output"; + }); + + await handlePipelineCombo({ + body: makeBody([ + { + role: "user", + content: "Write a robust sorting algorithm in Python with edge case handling", + }, + ]), + combo: makeCombo({ max_reflection_loops: maxLoops }), + handleChatCore: handler, + log: mockLog, + settings: makeSettings({ max_reflection_loops: maxLoops }), + }); + return reflectCalls; + }; + + const oneLoop = await reflectCallsFor(1); + const threeLoops = await reflectCallsFor(3); + + assert.equal(oneLoop, 2, `Expected 2 reflect calls with 1 loop, got ${oneLoop}`); + assert.equal(threeLoops, 4, `Expected 4 reflect calls with 3 loops, got ${threeLoops}`); + assert.ok(threeLoops > oneLoop, "Higher max_reflection_loops must produce more pipeline re-runs"); +}); diff --git a/tests/unit/account-fallback-service.test.ts b/tests/unit/account-fallback-service.test.ts index e885a66398..c23c5e9da9 100644 --- a/tests/unit/account-fallback-service.test.ts +++ b/tests/unit/account-fallback-service.test.ts @@ -153,6 +153,33 @@ test("checkFallbackError keeps generic 400 client errors terminal", () => { }); }); +test("checkFallbackError treats a genuine 400 model-access error as combo fallback", () => { + const result = checkFallbackError(400, "The model `foo` does not exist or is not available"); + assert.equal(result.shouldFallback, true); + assert.equal(result.reason, RateLimitReason.MODEL_CAPACITY); +}); + +test("checkFallbackError does NOT treat a bad-credential 400 as model-access fallback", () => { + // Phrased so it would otherwise match MODEL_ACCESS_DENIED_PATTERNS ("...api key + // ... model"), but the bad-credential signal must keep it terminal so the real + // auth error surfaces instead of silently exhausting every combo target. + const result = checkFallbackError(400, "Invalid API key provided for model gpt-4o"); + assert.deepEqual(result, { + shouldFallback: false, + cooldownMs: 0, + reason: RateLimitReason.UNKNOWN, + }); +}); + +test("checkFallbackError still honors structured model_not_found even with credential-like text", () => { + // Structured codes are authoritative and unaffected by the credential guard. + const result = checkFallbackError(400, "unauthorized-ish blob", 0, null, "openai", null, null, { + code: "model_not_found", + }); + assert.equal(result.shouldFallback, true); + assert.equal(result.reason, RateLimitReason.MODEL_CAPACITY); +}); + test("filterAvailableAccounts skips exclusion and active cooldowns but keeps recovered ones", () => { withMockedNow(1_700_000_000_000, () => { const accounts = [ diff --git a/tests/unit/antigravity-model-aliases.test.ts b/tests/unit/antigravity-model-aliases.test.ts index 08a5469e31..177dc6a67b 100644 --- a/tests/unit/antigravity-model-aliases.test.ts +++ b/tests/unit/antigravity-model-aliases.test.ts @@ -102,6 +102,26 @@ test("ANTIGRAVITY_PUBLIC_MODELS exposes captured Antigravity 2.0.1 names and cap ); }); +test("ANTIGRAVITY_PUBLIC_MODELS has no duplicate model IDs", () => { + const ids = ANTIGRAVITY_PUBLIC_MODELS.map((model) => model.id); + const seen = new Set(); + const duplicates = ids.filter((id) => { + if (seen.has(id)) return true; + seen.add(id); + return false; + }); + assert.deepEqual(duplicates, [], `duplicate model IDs found: ${duplicates.join(", ")}`); +}); + +test("gemini-3-flash-agent keeps its Agent display name (not the Flash High duplicate)", () => { + // A duplicate entry previously overwrote this name with "Gemini 3.5 Flash (High)" + // because the id-keyed name map kept the last occurrence. + assert.equal( + getClientVisibleAntigravityModelName("gemini-3-flash-agent"), + "Gemini 3.5 Flash Agent" + ); +}); + test("AntigravityExecutor.transformRequest resolves alias models before dispatching upstream", async () => { const executor = new AntigravityExecutor(); const result = await executor.transformRequest( diff --git a/tests/unit/cloud-agent-credentials.test.ts b/tests/unit/cloud-agent-credentials.test.ts new file mode 100644 index 0000000000..75f6c592ff --- /dev/null +++ b/tests/unit/cloud-agent-credentials.test.ts @@ -0,0 +1,81 @@ +/** + * Cloud-agent credentials CRUD + migration coverage. + * + * Release/v3.8.2 review finding: the `cloud_agent_credentials` table used to be + * created inline via `ensureCredentialsTable()` on every call (violating the + * versioned-migration policy). That inline DDL was removed in favor of + * migration `061_cloud_agent_credentials.sql`. These tests prove the table is + * provisioned by the normal DB-init migration run and that encrypt-at-rest + * CRUD still works end to end — with NO lazy table creation. + */ + +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-cloud-agent-creds-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "cloud-agent-creds-test-secret"; + +const core = await import("../../src/lib/db/core.ts"); +const creds = await import("../../src/lib/cloudAgent/credentials.ts"); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("migration 061 provisions cloud_agent_credentials (table exists after DB init)", () => { + const db = core.getDbInstance(); + const row = db + .prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?") + .get("cloud_agent_credentials") as { name?: string } | undefined; + assert.equal( + row?.name, + "cloud_agent_credentials", + "table must be created by migration, not inline" + ); +}); + +test("ensureCredentialsTable is no longer exported (inline DDL removed)", () => { + assert.equal( + (creds as Record).ensureCredentialsTable, + undefined, + "lazy table creation must be gone — migration owns the schema" + ); +}); + +test("save → get round-trips and decrypts the API key", () => { + creds.saveCloudAgentCredential("devin", "sk-secret-123", "https://api.devin.example"); + const got = creds.getCloudAgentCredentialFromDb("devin"); + assert.deepEqual(got, { apiKey: "sk-secret-123", baseUrl: "https://api.devin.example" }); +}); + +test("get returns null for unknown provider", () => { + assert.equal(creds.getCloudAgentCredentialFromDb("does-not-exist"), null); +}); + +test("save upserts (ON CONFLICT) rather than duplicating", () => { + creds.saveCloudAgentCredential("jules", "sk-first"); + creds.saveCloudAgentCredential("jules", "sk-second", "https://jules.example"); + const got = creds.getCloudAgentCredentialFromDb("jules"); + assert.deepEqual(got, { apiKey: "sk-second", baseUrl: "https://jules.example" }); +}); + +test("list returns masked keys, never the plaintext", () => { + creds.saveCloudAgentCredential("codex-cloud", "sk-supersecretvalue"); + const list = creds.listCloudAgentCredentials(); + const entry = list.find((c) => c.providerId === "codex-cloud"); + assert.ok(entry, "saved provider must appear in the list"); + assert.equal(entry.apiKey, "****alue"); + assert.ok(!entry.apiKey.includes("supersecret"), "plaintext key must never be returned"); +}); + +test("delete removes the credential", () => { + creds.saveCloudAgentCredential("temp", "sk-temp"); + assert.ok(creds.getCloudAgentCredentialFromDb("temp")); + creds.deleteCloudAgentCredential("temp"); + assert.equal(creds.getCloudAgentCredentialFromDb("temp"), null); +}); diff --git a/tests/unit/memory-store.test.ts b/tests/unit/memory-store.test.ts index c86de71d37..a629f064f1 100644 --- a/tests/unit/memory-store.test.ts +++ b/tests/unit/memory-store.test.ts @@ -350,3 +350,14 @@ test("listMemories page parameter defaults to page 1 when omitted with limit", a ); assert.equal(defaultPage.total, 2); }); + +test("getMemoryTokensUsed sums estimated tokens, optionally scoped to an api key", async () => { + // Token estimate is floor((LENGTH(content) + 3) / 4) per row (SQLite integer math). + insertMemoryRow({ id: "tok-1", apiKeyId: "key-a", content: "aaaa" }); // (4+3)/4 = 1 + insertMemoryRow({ id: "tok-2", apiKeyId: "key-b", content: "aaaaaaaa" }); // (8+3)/4 = 2 + + assert.equal(store.getMemoryTokensUsed("key-a"), 1); + assert.equal(store.getMemoryTokensUsed("key-b"), 2); + assert.equal(store.getMemoryTokensUsed(), 3); // all memories + assert.equal(store.getMemoryTokensUsed("missing-key"), 0); +}); diff --git a/tests/unit/pipeline-router.test.ts b/tests/unit/pipeline-router.test.ts index 9e0b69ef0d..e87424a4c4 100644 --- a/tests/unit/pipeline-router.test.ts +++ b/tests/unit/pipeline-router.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import { handlePipelineCombo, + buildPipelineResponse, FITNESS_TIERS, } from "../../open-sse/services/autoCombo/pipelineRouter.ts"; @@ -239,6 +240,115 @@ test("handlePipelineCombo classifies reasoning prompts correctly", async () => { assert.ok(stages.length >= 1, "Should have at least execute stage"); }); +// --------------------------------------------------------------------------- +// combo.models normalization — entries are model-config OBJECTS, not strings. +// (release/v3.8.2 review, finding B: the old `as string[]` cast passed raw +// objects to getTaskFitness, so stages always resolved to the default model.) +// --------------------------------------------------------------------------- + +test("handlePipelineCombo resolves stage models from object-form combo.models", async () => { + const log = makeLogger(); + const longCodePrompt = + "Write a function to sort an array using quicksort algorithm in TypeScript with proper type annotations and error handling for edge cases including empty arrays null values and duplicate elements with comprehensive JSDoc documentation"; + const body = makeBody([{ role: "user", content: longCodePrompt }]); + // Real combos store entries as objects with a `.model` field, not bare strings. + const combo = { + name: "test-combo", + models: [ + { model: "gpt-4o", priority: 1 }, + { model: "deepseek-reasoner", priority: 2 }, + ], + strategy: "priority", + config: { pipeline_enabled: true }, + }; + const settings = makeSettings(); + + const seenModels: unknown[] = []; + const recordingHandleChatCore = async (_b: Record, modelStr?: string) => { + seenModels.push(modelStr); + return new Response( + JSON.stringify({ + choices: [{ message: { role: "assistant", content: "ok" }, index: 0 }], + }), + { status: 200, headers: { "content-type": "application/json" } } + ); + }; + + await handlePipelineCombo({ + body, + combo, + handleChatCore: recordingHandleChatCore, + log, + settings, + }); + + const comboNames = new Set(["gpt-4o", "deepseek-reasoner"]); + const resolved = seenModels.filter((m) => m !== undefined); + assert.ok(resolved.length > 0, "at least one stage should resolve a model from the combo pool"); + for (const m of resolved) { + assert.equal(typeof m, "string", "stage model must be a string, not a raw config object"); + assert.ok( + comboNames.has(m as string), + `resolved model "${String(m)}" should come from the combo pool, not the default fallback` + ); + } +}); + +// --------------------------------------------------------------------------- +// buildPipelineResponse — adapts a PipelineResult into an HTTP Response +// (release/v3.8.2 review, finding C-1: callers expect a Response, not a +// PipelineResult; before the fix the buffered pipeline text was silently lost) +// --------------------------------------------------------------------------- + +function makePipelineResult(text: string): PipelineResult { + return { + text, + stages: [{ stage: "execute", text }], + fallback: false, + reflectVerdict: "pass", + }; +} + +test("buildPipelineResponse: non-streaming → OpenAI chat.completion JSON with text", async () => { + const res = buildPipelineResponse(makePipelineResult("hello from pipeline") as never, { + model: "auto/smart", + stream: false, + }); + assert.ok(res instanceof Response); + assert.equal(res.status, 200); + assert.match(res.headers.get("content-type") ?? "", /application\/json/); + const json = (await res.json()) as { + object: string; + model: string; + choices: Array<{ message: { role: string; content: string }; finish_reason: string }>; + }; + assert.equal(json.object, "chat.completion"); + assert.equal(json.model, "auto/smart"); + assert.equal(json.choices[0].message.role, "assistant"); + assert.equal(json.choices[0].message.content, "hello from pipeline"); + assert.equal(json.choices[0].finish_reason, "stop"); +}); + +test("buildPipelineResponse: streaming → SSE chunks carrying the text + [DONE]", async () => { + const res = buildPipelineResponse(makePipelineResult("streamed output") as never, { + model: "auto/smart", + stream: true, + }); + assert.ok(res instanceof Response); + assert.match(res.headers.get("content-type") ?? "", /text\/event-stream/); + const body = await res.text(); + assert.match(body, /"object":"chat\.completion\.chunk"/); + assert.match(body, /streamed output/); + assert.match(body, /"finish_reason":"stop"/); + assert.match(body, /data: \[DONE\]/); +}); + +test("buildPipelineResponse: defaults model to 'auto' when body has none", async () => { + const res = buildPipelineResponse(makePipelineResult("x") as never, {}); + const json = (await res.json()) as { model: string }; + assert.equal(json.model, "auto"); +}); + // --------------------------------------------------------------------------- // Type for test result access // --------------------------------------------------------------------------- diff --git a/tests/unit/route-error-sanitization-v382.test.ts b/tests/unit/route-error-sanitization-v382.test.ts new file mode 100644 index 0000000000..23d6e9b342 --- /dev/null +++ b/tests/unit/route-error-sanitization-v382.test.ts @@ -0,0 +1,72 @@ +/** + * Static guard tests for the error-sanitization fixes in release/v3.8.2. + * + * Review findings: two authenticated routes returned raw error text in their + * HTTP body (`(error as Error).message` / `String(error)`), violating the + * project's error-sanitization policy (CLAUDE.md hard rule 12, + * docs/security/ERROR_SANITIZATION.md). These guards pin the fix in source so + * the anti-pattern cannot silently return. (Static-source assertions mirror the + * established style of cli-tools-auth-hardening.test.ts.) + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); + +function readRoute(rel: string): string { + return fs.readFileSync(path.join(REPO_ROOT, rel), "utf8"); +} + +const CACHE_STATS = "src/app/api/cache/stats/route.ts"; +const HERMES = "src/app/api/cli-tools/hermes-agent-settings/route.ts"; +const DB_IMPORT = "src/app/api/db-backups/import/route.ts"; + +test("db-backups/import awaits getSettings() before re-hydrating the system prompt", () => { + // getSettings() is async; without `await`, importedSettings is a Promise and + // `.systemPrompt` is always undefined, so the #2470 re-hydration silently + // never fires after a DB import. + const src = readRoute(DB_IMPORT); + assert.match( + src, + /const importedSettings = await getSettings\(\);/, + "must await getSettings() so systemPrompt re-hydration actually runs" + ); + assert.ok( + !/const importedSettings = getSettings\(\);/.test(src), + "must not read systemPrompt off an un-awaited Promise" + ); +}); + +test("cache/stats route routes errors through sanitizeErrorMessage", () => { + const src = readRoute(CACHE_STATS); + assert.match(src, /import \{ sanitizeErrorMessage \}/, "must import sanitizeErrorMessage"); + assert.match(src, /sanitizeErrorMessage\(error\)/, "catch blocks must sanitize the error"); + assert.ok( + !/\(error as Error\)\.message/.test(src), + "must not put raw (error as Error).message in the response body" + ); +}); + +test("hermes-agent-settings GET sanitizes its error response (no raw String(error))", () => { + const src = readRoute(HERMES); + assert.match(src, /import \{ sanitizeErrorMessage \}/, "must import sanitizeErrorMessage"); + assert.match(src, /sanitizeErrorMessage\(error\)/, "GET catch must sanitize the error"); + assert.ok( + !/error: String\(error\)/.test(src), + "must not return raw String(error) in the response body" + ); +}); + +test("hermes-agent-settings POST validates baseUrl as an http(s) URL", () => { + const src = readRoute(HERMES); + assert.match( + src, + /import \{ validateBaseUrl \}/, + "must import the shared validateBaseUrl helper" + ); + assert.match(src, /validateBaseUrl\(baseUrl\)/, "POST must validate the supplied baseUrl"); +}); diff --git a/tests/unit/t06-schema-hardening.test.ts b/tests/unit/t06-schema-hardening.test.ts index ff66e4ef9d..68615abdb9 100644 --- a/tests/unit/t06-schema-hardening.test.ts +++ b/tests/unit/t06-schema-hardening.test.ts @@ -12,6 +12,7 @@ import { pricingSyncRequestSchema, updateTaskRoutingSchema, taskRoutingActionSchema, + codexProfileIdSchema, } from "../../src/shared/validation/schemas.ts"; test("translatorDetectSchema rejects empty body object", () => { @@ -162,3 +163,32 @@ test("taskRoutingActionSchema accepts detect action with object body", () => { }); assert.equal(validation.success, true); }); + +test("codexProfileIdSchema accepts a normal slug profileId", () => { + const validation = validateBody(codexProfileIdSchema, { profileId: "my-work-profile_2" }); + assert.equal(validation.success, true); +}); + +test("codexProfileIdSchema rejects path-traversal profileId (escape PROFILES_DIR)", () => { + // profileId is interpolated into `/.json` and used for + // fs.readFile / fs.unlink. A `..` segment or path separator must be rejected + // at validation so the request cannot read or delete files outside the dir. + for (const evil of [ + "../../../../etc/passwd", + "..\\..\\windows\\system32\\config", + "foo/bar", + "/etc/shadow", + "..", + ".", + "with space", + "a$(whoami)", + ]) { + const validation = validateBody(codexProfileIdSchema, { profileId: evil }); + assert.equal(validation.success, false, `expected rejection for profileId="${evil}"`); + } +}); + +test("codexProfileIdSchema rejects empty/whitespace profileId", () => { + assert.equal(validateBody(codexProfileIdSchema, { profileId: "" }).success, false); + assert.equal(validateBody(codexProfileIdSchema, { profileId: " " }).success, false); +});