From ed36bd72643dc458c125e985d744aa46d7272d6a Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Fri, 29 May 2026 01:45:59 -0300 Subject: [PATCH] fix(cli): restore omniroute logs command stream (#2756) (#2810) Integrated into release/v3.8.6. --- CHANGELOG.md | 1 + src/app/api/cli-tools/logs/route.ts | 158 +++++++++++++++++++++++++ src/app/api/logs/console/route.ts | 3 +- tests/unit/cli-logs-route.test.ts | 173 ++++++++++++++++++++++++++++ 4 files changed, 334 insertions(+), 1 deletion(-) create mode 100644 src/app/api/cli-tools/logs/route.ts create mode 100644 tests/unit/cli-logs-route.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a5e36cfc2..480ce042d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ ### 🔧 Bug Fixes +- **cli:** restore `omniroute logs` command — create missing `/api/cli-tools/logs` route that `log-streamer.ts` was calling, returning filtered pino log entries with `follow` and `filter` query-param support (#2756) - **fix(opencode-go,opencode-zen):** mark qwen3.7-max / 3.6-plus / 3.5-plus as supportsVision:false to stop forwarding image blocks to vision-incapable upstream models ([#2822]) - **nous-research:** append /chat/completions to provider baseUrl so DefaultExecutor's default URL builder hits the correct endpoint instead of returning 404 ([#2826]) - **fix(quota):** honor explicit per-connection `quotaPreflightEnabled: false` even when the provider has global window defaults — adds early-return guard before the AND-of-negations gate in auth.ts ([#2831]) diff --git a/src/app/api/cli-tools/logs/route.ts b/src/app/api/cli-tools/logs/route.ts new file mode 100644 index 0000000000..2c105f68c1 --- /dev/null +++ b/src/app/api/cli-tools/logs/route.ts @@ -0,0 +1,158 @@ +/** + * CLI Log Stream API — GET /api/cli-tools/logs + * + * Reads the application log file and returns matching entries. + * Called by the CLI `omniroute logs` command via + * `src/lib/cli-helper/log-streamer.ts`. + * + * Query params: + * - follow: boolean — kept for forward-compat; ignored in this + * implementation (streaming follow-mode is handled client-side + * by the log-streamer's ReadableStream). + * - filter: comma-separated strings — entries whose `component`, + * `module`, or `msg` fields match ANY token are included. + * Case-insensitive substring match. + * - limit: max number of entries to return — default 500, max 2000. + * + * Auth: Tier 3 MANAGEMENT — requireManagementAuth (same as all + * other /api/cli-tools/* routes). + * + * Note: this route reads logs only and spawns no child processes, + * so it does NOT require isLocalOnlyPath() classification. + */ + +import { NextRequest, NextResponse } from "next/server"; +import { readFileSync, existsSync } from "fs"; +import { getAppLogFilePath } from "@/lib/logEnv"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; +import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error.ts"; + +// Map pino numeric levels to string levels +const NUMERIC_LEVEL_MAP: Record = { + 10: "trace", + 20: "debug", + 30: "info", + 40: "warn", + 50: "error", + 60: "fatal", +}; + +function parseLevel(raw: string | number): string { + if (typeof raw === "number") { + return NUMERIC_LEVEL_MAP[raw] || "info"; + } + return String(raw).toLowerCase(); +} + +function stringifyLogValue(value: unknown): string { + if (value === undefined || value === null) return ""; + if (typeof value === "string") return value; + if (value instanceof Error) return value.message || value.name; + if (typeof value === "number" || typeof value === "boolean" || typeof value === "bigint") { + return String(value); + } + try { + const json = JSON.stringify(value); + return typeof json === "string" ? json : String(value); + } catch { + return String(value); + } +} + +/** + * GET /api/cli-tools/logs + */ +export async function GET(req: NextRequest) { + const authError = await requireManagementAuth(req); + if (authError) return authError; + + try { + const { searchParams } = new URL(req.url); + + // `follow` is accepted for forward-compat but not used server-side; + // the CLI's ReadableStream already handles reconnection client-side. + const _follow = searchParams.get("follow") === "true"; + + // Comma-separated filter tokens (e.g. "router,oauth") + const filterRaw = searchParams.get("filter") || ""; + const filterTokens = filterRaw + .split(",") + .map((t) => t.trim().toLowerCase()) + .filter(Boolean); + + const rawLimit = parseInt(searchParams.get("limit") || "500", 10); + const limit = Math.min(Number.isFinite(rawLimit) && rawLimit > 0 ? rawLimit : 500, 2000); + + const logPath = getAppLogFilePath(); + + if (!existsSync(logPath)) { + return NextResponse.json([], { status: 200 }); + } + + const raw = readFileSync(logPath, "utf-8"); + const lines = raw.trim().split("\n").filter(Boolean); + + const oneHourAgo = Date.now() - 60 * 60 * 1000; + const entries: Record[] = []; + + for (const line of lines) { + try { + const entry = JSON.parse(line) as Record; + + // Filter by time (last 1 hour) + const ts = entry.time || entry.timestamp; + if (ts) { + const entryTime = new Date(ts as string | number).getTime(); + if (entryTime < oneHourAgo) continue; + } + + // Normalize fields + entry.level = parseLevel(entry.level as string | number); + entry.msg = stringifyLogValue(entry.msg ?? entry.message ?? ""); + entry.message = stringifyLogValue(entry.message ?? entry.msg); + if (entry.component !== undefined) entry.component = stringifyLogValue(entry.component); + if (entry.module !== undefined) entry.module = stringifyLogValue(entry.module); + + // Normalize timestamp field + if (entry.time && !entry.timestamp) { + entry.timestamp = entry.time; + } + + // Apply filter tokens — entry is included if ANY token matches + // component, module, or msg (case-insensitive substring) + if (filterTokens.length > 0) { + const haystack = [ + String(entry.component || ""), + String(entry.module || ""), + String(entry.msg || ""), + ] + .join(" ") + .toLowerCase(); + + const matches = filterTokens.some((token) => haystack.includes(token)); + if (!matches) continue; + } + + entries.push(entry); + } catch { + // Skip unparseable lines + } + } + + // Return last N entries (most recent) + const result = entries.slice(-limit); + + return NextResponse.json(result, { + status: 200, + headers: { + "Cache-Control": "no-store, no-cache, must-revalidate", + }, + }); + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + return NextResponse.json( + { error: sanitizeErrorMessage(message) || "Failed to read logs" }, + { status: 500 } + ); + } +} diff --git a/src/app/api/logs/console/route.ts b/src/app/api/logs/console/route.ts index 5b7d9cf9d2..ca32c2af6d 100644 --- a/src/app/api/logs/console/route.ts +++ b/src/app/api/logs/console/route.ts @@ -69,7 +69,8 @@ export async function GET(req: NextRequest) { try { const { searchParams } = new URL(req.url); const levelFilter = searchParams.get("level") || "all"; - const limit = Math.min(parseInt(searchParams.get("limit") || "500", 10), 2000); + const rawLimit = parseInt(searchParams.get("limit") || "500", 10); + const limit = Math.min(Number.isFinite(rawLimit) && rawLimit > 0 ? rawLimit : 500, 2000); const componentFilter = searchParams.get("component") || ""; const logPath = getLogFilePath(); diff --git a/tests/unit/cli-logs-route.test.ts b/tests/unit/cli-logs-route.test.ts new file mode 100644 index 0000000000..721f712ba1 --- /dev/null +++ b/tests/unit/cli-logs-route.test.ts @@ -0,0 +1,173 @@ +/** + * Tests for /api/cli-tools/logs route (fix #2756). + * + * Verifies: + * - GET returns 200 with valid JSON array body. + * - `filter` param filters log lines by text. + * - Error responses do NOT leak stack traces (hard rule #12). + * - log-streamer.ts points to the correct URL (/api/cli-tools/logs). + * - Non-numeric `limit` param does NOT bypass the 2000-entry cap. + */ + +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 { updateSettings } from "../../src/lib/db/settings"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cli-logs-route-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +// Write a small pino-format log file before route is imported +const logDir = path.join(process.cwd(), "logs", "application"); +fs.mkdirSync(logDir, { recursive: true }); +const logPath = path.join(logDir, "app.log"); +process.env.APP_LOG_FILE_PATH = logPath; + +const now = Date.now(); +const lines = [ + JSON.stringify({ level: 30, msg: "provider connected", component: "router", time: now }), + JSON.stringify({ level: 40, msg: "rate limit hit", component: "rateLimit", time: now }), + JSON.stringify({ level: 20, msg: "debug trace output", component: "debug", time: now }), + "not-valid-json-should-be-skipped", +]; +fs.writeFileSync(logPath, lines.join("\n") + "\n", "utf-8"); + +const { GET } = await import( + "../../src/app/api/cli-tools/logs/route.ts" +); + +test.before(async () => { + await updateSettings({ requireLogin: false }); +}); + +test.after(async () => { + await updateSettings({ requireLogin: true }); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + try { + fs.unlinkSync(logPath); + } catch { + // best effort + } +}); + +// Helper — make a request; auth passes because requireLogin is set to false in test.before +function makeReq(queryString = "") { + const url = `http://localhost/api/cli-tools/logs${queryString ? `?${queryString}` : ""}`; + return new Request(url); +} + +test("GET /api/cli-tools/logs returns 200 with JSON array when log file exists", async () => { + const res = await GET(makeReq()); + assert.equal(res.status, 200); + + const body = await res.json(); + assert.ok(Array.isArray(body), "body should be an array"); + assert.ok(body.length >= 3, `expected at least 3 entries, got ${body.length}`); +}); + +test("GET /api/cli-tools/logs respects filter param", async () => { + const res = await GET(makeReq("filter=rateLimit")); + assert.equal(res.status, 200); + + const body = await res.json(); + assert.ok(Array.isArray(body)); + // Only the "rate limit hit" entry matches component=rateLimit + assert.ok( + body.every((e: { component?: string; msg?: string }) => { + const comp = (e.component || "").toLowerCase(); + const msg = (e.msg || "").toLowerCase(); + return comp.includes("ratelimit") || msg.includes("ratelimit") || comp.includes("rate"); + }), + "filter should restrict results to matching component/message" + ); +}); + +test("GET /api/cli-tools/logs returns empty array when log file does not exist", async () => { + const origPath = process.env.APP_LOG_FILE_PATH; + process.env.APP_LOG_FILE_PATH = "/tmp/omniroute-nonexistent-cli-logs-test.log"; + + const res = await GET(makeReq()); + assert.equal(res.status, 200); + const body = await res.json(); + assert.ok(Array.isArray(body)); + assert.equal(body.length, 0); + + process.env.APP_LOG_FILE_PATH = origPath; +}); + +test("GET /api/cli-tools/logs error response does not leak stack traces (hard rule #12)", async () => { + // Simulate an internal error path by temporarily breaking the log path to a dir + const origPath = process.env.APP_LOG_FILE_PATH; + // Point to a directory so readFileSync throws + process.env.APP_LOG_FILE_PATH = TEST_DATA_DIR; + + const res = await GET(makeReq()); + // Should respond with 500 or empty (route may handle gracefully), but must NOT leak stack + const text = await res.text(); + assert.ok(!text.includes(" at "), "Response must not contain stack trace frames"); + + process.env.APP_LOG_FILE_PATH = origPath; +}); + +test("GET /api/cli-tools/logs limit=abc does not bypass the 2000-entry cap", async () => { + // Write more than 2000 entries so we can verify the cap is applied + const manyLines: string[] = []; + for (let i = 0; i < 2100; i++) { + manyLines.push(JSON.stringify({ level: 30, msg: `entry ${i}`, component: "test", time: now })); + } + const origPath = process.env.APP_LOG_FILE_PATH; + const bigLogPath = path.join(logDir, "big.log"); + fs.writeFileSync(bigLogPath, manyLines.join("\n") + "\n", "utf-8"); + process.env.APP_LOG_FILE_PATH = bigLogPath; + + try { + const res = await GET(makeReq("limit=abc")); + assert.equal(res.status, 200); + const body = await res.json(); + assert.ok(Array.isArray(body), "body should be an array"); + // Non-numeric limit must fall back to default (500), not bypass the cap with NaN + assert.ok( + body.length <= 500, + `Non-numeric limit=abc should fall back to default 500, got ${body.length}` + ); + } finally { + process.env.APP_LOG_FILE_PATH = origPath; + fs.unlinkSync(bigLogPath); + } +}); + +test("log-streamer.ts calls /api/cli-tools/logs (correct URL, not the missing route)", async () => { + const { createLogStream } = await import("../../src/lib/cli-helper/log-streamer.ts"); + // Inspect the source to verify the URL used; we mock fetch to capture it + const captured: string[] = []; + const origFetch = globalThis.fetch; + + globalThis.fetch = (async (url: string) => { + captured.push(typeof url === "string" ? url : String(url)); + // Return a mock Response with a body so the stream doesn't error immediately + return new Response(new ReadableStream({ start(c) { c.close(); } }), { status: 200 }); + }) as typeof fetch; + + try { + const { stream, stop } = createLogStream({ baseUrl: "http://localhost:20128" }); + const reader = stream.getReader(); + // Consume until done (mock stream closes immediately) + await reader.read().catch(() => {}); + stop(); + } finally { + globalThis.fetch = origFetch; + } + + assert.ok(captured.length > 0, "fetch should have been called"); + assert.ok( + captured.some((u) => u.includes("/api/cli-tools/logs")), + `Expected /api/cli-tools/logs in fetched URL, got: ${captured[0]}` + ); + assert.ok( + !captured.some((u) => u.includes("/api/logs/console")), + "log-streamer should not call /api/logs/console" + ); +});