fix(cli): code-review — C1/C2/C3/I1/I3/I4/I5/m1/m2/m4

C1: sanitize opts.name and backupId against path traversal (replace /\\ with _) C2: read backup files locally as base64 instead of sending local path to cloud API C3: implement markOAuthDone/markOAuthFailed via module-level callbacks registered in useEffect — TUI now transitions to DONE/FAILED when polling signals completion I1: fix matchesGlob — equality-only for non-glob patterns (removes false-positive startsWith), and support multi-wildcard patterns (e.g. **.json) by iterating parts I3: replace two hardcoded English strings in tunnel.mjs with t() calls; add tunnel.notAvailable and tunnel.noTunnels to en.json and pt-BR.json I4: log HTTP 4xx errors in runKeysAddCommand and return 1 instead of falling through silently to DB write on client errors I5: truncate err.message to 100 chars in ProvidersTestAll.jsx and test-provider.mjs m1: fix EvalWatch StatusBadge — completed state now uses status="ok" instead of "running" m2: replace hardcoded ~/.omniroute/backup-schedule.json with resolveDataDir()-based path m4: remove ...opts spread from all apiFetch calls in keys.mjs — pass only explicit options (method, body, retry, acceptNotOk) to avoid leaking apiKey/baseUrl/yes/etc.
2026-07-26 09:52:11 +03:00 · 2026-05-15 09:48:30 -03:00
parent 09db1129a1
commit 69a2b27a33
10 changed files with 105 additions and 41 deletions
--- a/bin/cli/commands/backup.mjs
+++ b/bin/cli/commands/backup.mjs
@@ -8,7 +8,6 @@ import {
 } from "node:fs";
 import { createCipheriv, createDecipheriv, randomBytes, scryptSync } from "node:crypto";
 import { dirname, join, extname, basename } from "node:path";
-import { homedir } from "node:os";
 import { resolveDataDir } from "../data-dir.mjs";
 import { apiFetch, isServerUp } from "../api.mjs";
 import { t } from "../i18n.mjs";
@@ -98,15 +97,25 @@ export function registerRestore(program) {
 }

 function matchesGlob(fileName, pattern) {
-  if (!pattern.includes("*")) return fileName === pattern || fileName.startsWith(pattern);
+  if (!pattern.includes("*")) return fileName === pattern;
  const parts = pattern.split("*");
-  if (parts.length !== 2) return false;
-  const [prefix, suffix] = parts;
-  return (
-    fileName.startsWith(prefix) &&
-    fileName.endsWith(suffix) &&
-    fileName.length >= prefix.length + suffix.length
-  );
+  let pos = 0;
+  for (let i = 0; i < parts.length; i++) {
+    const part = parts[i];
+    if (!part) continue;
+    if (i === 0) {
+      if (!fileName.startsWith(part)) return false;
+      pos = part.length;
+    } else if (i === parts.length - 1) {
+      if (!fileName.endsWith(part)) return false;
+      if (fileName.length < pos + part.length) return false;
+    } else {
+      const idx = fileName.indexOf(part, pos);
+      if (idx === -1) return false;
+      pos = idx + part.length;
+    }
+  }
+  return true;
 }

 function shouldExclude(fileName, patterns) {
@@ -155,7 +164,8 @@ export async function runBackupCommand(opts = {}) {
  const dataDir = resolveDataDir();
  const backupDir = getBackupDir();
  const timestamp = new Date().toISOString().replace(/[:.]/g, "-").slice(0, 19);
-  const backupName = opts.name ? `omniroute-backup-${opts.name}` : `omniroute-backup-${timestamp}`;
+  const safeName = opts.name ? String(opts.name).replace(/[/\\]/g, "_") : null;
+  const backupName = safeName ? `omniroute-backup-${safeName}` : `omniroute-backup-${timestamp}`;
  const backupPath = join(backupDir, backupName);
  const excludePatterns = opts.exclude || [];

@@ -262,9 +272,14 @@ async function _uploadBackupToCloud(backupPath, info) {
    return 1;
  }
  try {
+    // Read files locally and send as base64 — never send local path to server
+    const files = {};
+    for (const fname of readdirSync(backupPath)) {
+      files[fname] = readFileSync(join(backupPath, fname)).toString("base64");
+    }
    const res = await apiFetch("/api/db-backups/cloud", {
      method: "POST",
-      body: { backupPath, info },
+      body: { files, info },
      retry: false,
      timeout: 30000,
      acceptNotOk: true,
@@ -280,9 +295,12 @@ async function _uploadBackupToCloud(backupPath, info) {
  }
 }

-const BACKUP_SCHEDULE_PATH = join(homedir(), ".omniroute", "backup-schedule.json");
+function getSchedulePath() {
+  return join(resolveDataDir(), "backup-schedule.json");
+}

 export async function runBackupAutoEnableCommand(opts = {}) {
+  const schedulePath = getSchedulePath();
  const schedule = {
    enabled: true,
    cron: opts.cron || "0 3 * * *",
@@ -291,30 +309,32 @@ export async function runBackupAutoEnableCommand(opts = {}) {
    retention: opts.retention || null,
    updatedAt: new Date().toISOString(),
  };
-  mkdirSync(dirname(BACKUP_SCHEDULE_PATH), { recursive: true });
-  writeFileSync(BACKUP_SCHEDULE_PATH, JSON.stringify(schedule, null, 2), "utf8");
+  mkdirSync(dirname(schedulePath), { recursive: true });
+  writeFileSync(schedulePath, JSON.stringify(schedule, null, 2), "utf8");
  console.log(t("backup.auto.enabled", { cron: schedule.cron }));
  console.log(t("backup.auto.hint"));
  return 0;
 }

 export async function runBackupAutoDisableCommand() {
-  if (existsSync(BACKUP_SCHEDULE_PATH)) {
-    const schedule = JSON.parse(readFileSync(BACKUP_SCHEDULE_PATH, "utf8"));
+  const schedulePath = getSchedulePath();
+  if (existsSync(schedulePath)) {
+    const schedule = JSON.parse(readFileSync(schedulePath, "utf8"));
    schedule.enabled = false;
    schedule.updatedAt = new Date().toISOString();
-    writeFileSync(BACKUP_SCHEDULE_PATH, JSON.stringify(schedule, null, 2), "utf8");
+    writeFileSync(schedulePath, JSON.stringify(schedule, null, 2), "utf8");
  }
  console.log(t("backup.auto.disabled"));
  return 0;
 }

 export async function runBackupAutoStatusCommand() {
-  if (!existsSync(BACKUP_SCHEDULE_PATH)) {
+  const schedulePath = getSchedulePath();
+  if (!existsSync(schedulePath)) {
    console.log(t("backup.auto.notConfigured"));
    return 0;
  }
-  const schedule = JSON.parse(readFileSync(BACKUP_SCHEDULE_PATH, "utf8"));
+  const schedule = JSON.parse(readFileSync(schedulePath, "utf8"));
  const statusLabel = schedule.enabled ? "\x1b[32m● enabled\x1b[0m" : "\x1b[31m○ disabled\x1b[0m";
  console.log(`${t("backup.auto.title")}: ${statusLabel}`);
  console.log(`  cron:      ${schedule.cron}`);
@@ -368,7 +388,8 @@ export async function runRestoreCommand(backupId, opts = {}) {
    return 0;
  }

-  const backupPath = join(backupDir, `omniroute-backup-${backupId}`);
+  const safeBackupId = String(backupId).replace(/[/\\]/g, "_");
+  const backupPath = join(backupDir, `omniroute-backup-${safeBackupId}`);
  if (!existsSync(backupPath)) {
    console.error(t("backup.notFound", { name: backupId }));
    return 1;
--- a/bin/cli/commands/keys.mjs
+++ b/bin/cli/commands/keys.mjs
@@ -178,11 +178,16 @@ export async function runKeysAddCommand(provider, apiKey, opts = {}) {
        method: "POST",
        body: { provider: providerLower, apiKey: key },
        retry: false,
+        acceptNotOk: true,
      });
      if (res.ok) {
        console.log(t("keys.added", { provider: providerLower }));
        return 0;
      }
+      if (res.status >= 400 && res.status < 500) {
+        console.error(t("common.error", { message: `HTTP ${res.status}` }));
+        return 1;
+      }
    } catch {}
  }

@@ -341,7 +346,7 @@ export async function runKeysRegenerateCommand(id, opts = {}) {
  }
  const res = await apiFetch(`/api/v1/registered-keys/${encodeURIComponent(id)}/regenerate`, {
    method: "POST",
-    ...opts,
+    retry: false,
  });
  if (!res.ok) {
    console.error(t("common.error", { message: `HTTP ${res.status}` }));
@@ -367,7 +372,7 @@ export async function runKeysRevokeCommand(id, opts = {}) {
  }
  const res = await apiFetch(`/api/v1/registered-keys/${encodeURIComponent(id)}/revoke`, {
    method: "POST",
-    ...opts,
+    retry: false,
  });
  if (!res.ok) {
    console.error(t("common.error", { message: `HTTP ${res.status}` }));
@@ -380,7 +385,7 @@ export async function runKeysRevokeCommand(id, opts = {}) {
 export async function runKeysRevealCommand(id, opts = {}) {
  process.stderr.write(t("keys.revealWarning") + "\n");
  const res = await apiFetch(`/api/v1/registered-keys/${encodeURIComponent(id)}/reveal`, {
-    ...opts,
+    retry: false,
  });
  if (!res.ok) {
    console.error(t("common.error", { message: `HTTP ${res.status}` }));
@@ -395,7 +400,7 @@ export async function runKeysUsageCommand(id, opts = {}) {
  const limit = opts.limit || "20";
  const res = await apiFetch(
    `/api/v1/registered-keys/${encodeURIComponent(id)}/usage?limit=${limit}`,
-    { ...opts }
+    { retry: false }
  );
  if (!res.ok) {
    console.error(t("common.error", { message: `HTTP ${res.status}` }));
@@ -418,8 +423,8 @@ export async function runKeysUsageCommand(id, opts = {}) {

 export async function runKeysPolicyShowCommand(id, opts = {}) {
  const res = await apiFetch(`/api/v1/registered-keys/${encodeURIComponent(id)}/policy`, {
-    ...opts,
    acceptNotOk: true,
+    retry: false,
  });
  if (!res.ok) {
    console.error(t("common.error", { message: `HTTP ${res.status}` }));
@@ -453,8 +458,8 @@ export async function runKeysPolicySetCommand(id, opts = {}) {
  const res = await apiFetch(`/api/v1/registered-keys/${encodeURIComponent(id)}/policy`, {
    method: "PATCH",
    body,
-    ...opts,
    acceptNotOk: true,
+    retry: false,
  });
  if (!res.ok) {
    console.error(t("common.error", { message: `HTTP ${res.status}` }));
@@ -467,8 +472,8 @@ export async function runKeysPolicySetCommand(id, opts = {}) {
 export async function runKeysExpirationListCommand(opts = {}) {
  const days = Number(opts.days || 30);
  const res = await apiFetch(`/api/v1/registered-keys?expiring=true&days=${days}`, {
-    ...opts,
    acceptNotOk: true,
+    retry: false,
  });
  if (!res.ok) {
    console.error(t("common.error", { message: `HTTP ${res.status}` }));
@@ -510,8 +515,8 @@ export async function runKeysRotateCommand(id, opts = {}) {
  const res = await apiFetch(`/api/v1/registered-keys/${encodeURIComponent(id)}/rotate`, {
    method: "POST",
    body: { gracePeriodMs: gracePeriod },
-    ...opts,
    acceptNotOk: true,
+    retry: false,
  });
  if (!res.ok) {
    console.error(t("common.error", { message: `HTTP ${res.status}` }));
--- a/bin/cli/commands/test-provider.mjs
+++ b/bin/cli/commands/test-provider.mjs
@@ -194,9 +194,10 @@ async function _runSingleTest(provider, model) {
    const data = res.ok ? await res.json() : { success: false, error: `HTTP ${res.status}` };
    return { ...data, durationMs };
  } catch (err) {
+    const msg = err instanceof Error ? err.message : String(err);
    return {
      success: false,
-      error: err instanceof Error ? err.message : String(err),
+      error: msg.slice(0, 100),
      durationMs: Date.now() - startMs,
    };
  }
--- a/bin/cli/commands/tunnel.mjs
+++ b/bin/cli/commands/tunnel.mjs
@@ -90,7 +90,7 @@ export async function runTunnelListCommand(opts = {}) {
  try {
    const res = await apiFetch("/api/tunnels", { retry: false, timeout: 5000, acceptNotOk: true });
    if (!res.ok) {
-      console.log("Tunnel info not available.");
+      console.log(t("tunnel.notAvailable"));
      return 0;
    }

@@ -103,7 +103,7 @@ export async function runTunnelListCommand(opts = {}) {

    console.log(`\n\x1b[1m\x1b[36m${t("tunnel.title")}\x1b[0m\n`);
    if (!Array.isArray(tunnels) || tunnels.length === 0) {
-      console.log("  No active tunnels.");
+      console.log(t("tunnel.noTunnels"));
      return 0;
    }

--- a/bin/cli/locales/en.json
+++ b/bin/cli/locales/en.json
@@ -447,6 +447,8 @@
    "tailOpt": "Number of log lines to show",
    "typeRequired": "Tunnel type is required.",
    "noLogs": "No logs available.",
+    "notAvailable": "Tunnel info not available.",
+    "noTunnels": "No active tunnels.",
    "infoTitle": "Tunnel info: {type}",
    "rotated": "Tunnel URL rotated: {url}",
    "confirmRotate": "Rotate tunnel {type}? (a new URL will be generated)"
--- a/bin/cli/locales/pt-BR.json
+++ b/bin/cli/locales/pt-BR.json
@@ -447,6 +447,8 @@
    "tailOpt": "Número de linhas de log a exibir",
    "typeRequired": "Tipo de túnel é obrigatório.",
    "noLogs": "Nenhum log disponível.",
+    "notAvailable": "Informações do túnel não disponíveis.",
+    "noTunnels": "Nenhum túnel ativo.",
    "infoTitle": "Info do túnel: {type}",
    "rotated": "URL do túnel rotacionada: {url}",
    "confirmRotate": "Rotacionar túnel {type}? (uma nova URL será gerada)"
--- a/bin/cli/tui/EvalWatch.jsx
+++ b/bin/cli/tui/EvalWatch.jsx
@@ -94,11 +94,7 @@ function EvalWatchApp({ runId, suiteId, baseUrl, apiKey, onExit }) {
          <Box marginBottom={1}>
            <StatusBadge
              status={
-                run.status === "running"
-                  ? "running"
-                  : run.status === "completed"
-                    ? "running"
-                    : "error"
+                run.status === "running" ? "running" : run.status === "completed" ? "ok" : "error"
              }
            />
            <Text> {run.status} </Text>
--- a/bin/cli/tui/OAuthFlow.jsx
+++ b/bin/cli/tui/OAuthFlow.jsx
@@ -12,6 +12,9 @@ const PHASE = {
  CANCELLED: "cancelled",
 };

+let _globalDone = null;
+let _globalFail = null;
+
 function OAuthFlowApp({ provider, url, deviceCode, onCancel, onDone, onFail }) {
  const [phase, setPhase] = useState(PHASE.WAITING);
  const [result, setResult] = useState(null);
@@ -25,8 +28,21 @@ function OAuthFlowApp({ provider, url, deviceCode, onCancel, onDone, onFail }) {
  }, []);

  useEffect(() => {
-    if (!onDone && !onFail) return;
+    _globalDone = (res) => {
+      setResult(res);
+      setPhase(PHASE.DONE);
+      onDone?.(res);
+    };
+    _globalFail = (err) => {
+      setError(typeof err === "string" ? err : (err?.message ?? String(err)));
+      setPhase(PHASE.FAILED);
+      onFail?.(err);
+    };
    setPhase(PHASE.POLLING);
+    return () => {
+      _globalDone = null;
+      _globalFail = null;
+    };
  }, []);

  useInput((input, key) => {
@@ -99,7 +115,7 @@ function OAuthFlowApp({ provider, url, deviceCode, onCancel, onDone, onFail }) {
          </Box>
        ) : phase === PHASE.DONE ? (
          <Box>
-            <StatusBadge status="running" />
+            <StatusBadge status="ok" />
            <Text> Authorized: {result?.email ?? result?.account ?? "connected"}</Text>
          </Box>
        ) : phase === PHASE.FAILED ? (
@@ -168,5 +184,10 @@ export async function startOAuthTui({ provider, url, deviceCode }) {
  });
 }

-export function markOAuthDone(result) {}
-export function markOAuthFailed(error) {}
+export function markOAuthDone(result) {
+  _globalDone?.(result);
+}
+
+export function markOAuthFailed(error) {
+  _globalFail?.(error);
+}
--- a/bin/cli/tui/ProvidersTestAll.jsx
+++ b/bin/cli/tui/ProvidersTestAll.jsx
@@ -48,10 +48,11 @@ async function testOne(provider, model, baseUrl, apiKey) {
    const data = res.ok ? await res.json() : { success: false, error: `HTTP ${res.status}` };
    return { status: data.success ? STATUS.PASS : STATUS.FAIL, latencyMs, error: data.error };
  } catch (err) {
+    const msg = err instanceof Error ? err.message : String(err);
    return {
      status: STATUS.FAIL,
      latencyMs: Date.now() - start,
-      error: err instanceof Error ? err.message : String(err),
+      error: msg.slice(0, 100),
    };
  }
 }