fix(api/skills): sanitize error messages before returning them to clients

changelog.d/fixes/9088-skills-routes-error-sanitization.md

@@ -0,0 +1 @@
+- **fix(api/skills):** the `/api/skills/**` routes now run caught error messages through `sanitizeErrorMessage` before returning them, so a filesystem failure no longer leaks an absolute path (e.g. `/home/<user>/.omniroute/skills/...`) to the client; the `{ error: string }` response shape is preserved for the dashboard ([#9088](https://github.com/diegosouzapw/OmniRoute/pull/9088))

src/app/api/skills/[id]/route.ts

@@ -4,6 +4,7 @@ import { skillRegistry } from "@/lib/skills/registry";
 import { z } from "zod";
 import { validateBody, isValidationFailure } from "@/shared/validation/helpers";
 import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
+import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 
 const updateSkillSchema = z.object({
   enabled: z.boolean().optional(),
@@ -22,7 +23,7 @@ export async function DELETE(_request: Request, props: { params: Promise<{ id: s
     }
     return NextResponse.json({ success: true });
   } catch (err: unknown) {
-    const error = err instanceof Error ? err.message : String(err);
+    const error = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
     return NextResponse.json({ error }, { status: 500 });
   }
 }
@@ -71,7 +72,7 @@ export async function PUT(request: Request, props: { params: Promise<{ id: strin
       mode: validation.data.mode,
     });
   } catch (err: unknown) {
-    const error = err instanceof Error ? err.message : String(err);
+    const error = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
     return NextResponse.json({ error }, { status: 500 });
   }
 }

src/app/api/skills/executions/route.ts

@@ -4,6 +4,7 @@ import { parsePaginationParams, buildPaginatedResponse } from "@/shared/types/pa
 import { z } from "zod";
 import { validateBody, isValidationFailure } from "@/shared/validation/helpers";
 import { isAuthenticated } from "@/shared/utils/apiAuth";
+import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 
 export async function GET(request: Request) {
   if (!(await isAuthenticated(request))) {
@@ -21,7 +22,7 @@ export async function GET(request: Request) {
     );
     return NextResponse.json(buildPaginatedResponse(executions, total, params));
   } catch (err: unknown) {
-    const error = err instanceof Error ? err.message : String(err);
+    const error = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
     return NextResponse.json({ error }, { status: 500 });
   }
 }
@@ -51,7 +52,7 @@ export async function POST(request: Request) {
     });
     return NextResponse.json({ execution });
   } catch (err: unknown) {
-    const error = err instanceof Error ? err.message : String(err);
+    const error = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
     if (error.includes("disabled")) {
       return NextResponse.json({ error }, { status: 503 });
     }

src/app/api/skills/install/route.ts

@@ -3,6 +3,7 @@ import { z } from "zod";
 import { skillRegistry } from "@/lib/skills/registry";
 import { validateBody, isValidationFailure } from "@/shared/validation/helpers";
 import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
+import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 
 const installManifestSchema = z.object({
   name: z.string().min(1).max(100),
@@ -44,7 +45,7 @@ export async function POST(request: Request) {
 
     return NextResponse.json({ success: true, id: skill.id });
   } catch (err: unknown) {
-    const error = err instanceof Error ? err.message : String(err);
+    const error = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
     return NextResponse.json({ error }, { status: 500 });
   }
 }

src/app/api/skills/marketplace/install/route.ts

@@ -5,6 +5,7 @@ import { skillRegistry } from "@/lib/skills/registry";
 import { getSkillsProviderSetting } from "@/lib/skills/providerSettings";
 
 import { isAuthenticated } from "@/shared/utils/apiAuth";
+import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 
 const marketplaceInstallSchema = z.object({
   name: z.string().min(1).max(64),
@@ -53,7 +54,7 @@ export async function POST(request: Request) {
 
     return NextResponse.json({ success: true, id: skill.id });
   } catch (err: unknown) {
-    const error = err instanceof Error ? err.message : String(err);
+    const error = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
     return NextResponse.json({ error }, { status: 500 });
   }
 }

src/app/api/skills/marketplace/route.ts

@@ -2,6 +2,7 @@ import { NextResponse } from "next/server";
 import { getSettings } from "@/lib/db/settings";
 import { isAuthenticated } from "@/shared/utils/apiAuth";
 import { getSkillsProviderSetting } from "@/lib/skills/providerSettings";
+import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 
 const POPULAR_BY_PROVIDER = {
   skillsmp: ["web-search", "file-reader", "sql-assistant", "devops-helper", "docs-assistant"],
@@ -55,7 +56,7 @@ export async function GET(request: Request) {
     const data = await res.json();
     return NextResponse.json({ skills: data.data?.skills || data.skills || [] });
   } catch (err: unknown) {
-    const error = err instanceof Error ? err.message : String(err);
+    const error = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
     return NextResponse.json({ error }, { status: 500 });
   }
 }

src/app/api/skills/route.ts

@@ -4,6 +4,7 @@ import { parsePaginationParams, buildPaginatedResponse } from "@/shared/types/pa
 import { getSkillsProviderSetting } from "@/lib/skills/providerSettings";
 import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
 import { matchesSearch } from "@/shared/utils/turkishText";
+import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 
 const POPULAR_BY_PROVIDER = {
   skillsmp: ["web-search", "file-reader", "sql-assistant", "devops-helper", "docs-assistant"],
@@ -56,7 +57,7 @@ export async function GET(request?: Request) {
       popularDefaults: POPULAR_BY_PROVIDER[provider],
     });
   } catch (err: unknown) {
-    const error = err instanceof Error ? err.message : String(err);
+    const error = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
     return NextResponse.json({ error }, { status: 500 });
   }
 }

src/app/api/skills/skillssh/install/route.ts

@@ -5,6 +5,7 @@ import { skillRegistry } from "@/lib/skills/registry";
 import { isAuthenticated } from "@/shared/utils/apiAuth";
 import { fetchSkillMd } from "@/lib/skills/skillssh";
 import { getSkillsProviderSetting } from "@/lib/skills/providerSettings";
+import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 
 const skillsshInstallSchema = z.object({
   name: z.string().min(1).max(64),
@@ -55,7 +56,7 @@ export async function POST(request: Request) {
 
     return NextResponse.json({ success: true, id: skill.id });
   } catch (err: unknown) {
-    const error = err instanceof Error ? err.message : String(err);
+    const error = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
     return NextResponse.json({ error }, { status: 500 });
   }
 }

src/app/api/skills/skillssh/route.ts

@@ -1,6 +1,7 @@
 import { NextResponse } from "next/server";
 import { isAuthenticated } from "@/shared/utils/apiAuth";
 import { searchSkillsSh } from "@/lib/skills/skillssh";
+import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 
 export async function GET(request: Request) {
   if (!(await isAuthenticated(request))) {
@@ -22,7 +23,7 @@ export async function GET(request: Request) {
       })),
     });
   } catch (err: unknown) {
-    const error = err instanceof Error ? err.message : String(err);
+    const error = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
     return NextResponse.json({ error }, { status: 500 });
   }
 }

tests/unit/skills-routes-error-sanitization.test.ts

@@ -0,0 +1,73 @@
+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";
+
+// The /api/skills/** routes returned `{ error: err.message }` verbatim on a 500,
+// leaking absolute filesystem paths (skillRegistry writes to disk, so EACCES/ENOSPC
+// surface a real path). These routes are consumed by the dashboard as `{ error: string }`
+// (OmniMarketplaceTab.tsx, OmniSkillsPageClient.tsx), so the fix keeps the string shape
+// and only routes the message through sanitizeErrorMessage (Hard Rule #12: no path/stack leak).
+//
+// mock.module is unreliable under this tsx/ESM + node:test runner (see
+// instrumentation-warm-catalog-cache.test.ts), so the singleton method is monkey-patched.
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-skills-sanitize-"));
+const ORIGINAL_DATA_DIR = process.env.DATA_DIR;
+const ORIGINAL_OMNIROUTE_API_KEY = process.env.OMNIROUTE_API_KEY;
+
+process.env.DATA_DIR = TEST_DATA_DIR;
+delete process.env.OMNIROUTE_API_KEY;
+
+const core = await import("../../src/lib/db/core.ts");
+const { skillRegistry } = await import("../../src/lib/skills/registry.ts");
+const skillsRoute = await import("../../src/app/api/skills/route.ts");
+
+const LEAKY_PATH = "/home/testuser/.omniroute/skills/evil/handler.js";
+
+function resetStorage() {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+}
+
+test.beforeEach(() => {
+  delete process.env.OMNIROUTE_API_KEY;
+  resetStorage();
+});
+
+test.after(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR;
+  else process.env.DATA_DIR = ORIGINAL_DATA_DIR;
+  if (ORIGINAL_OMNIROUTE_API_KEY === undefined) delete process.env.OMNIROUTE_API_KEY;
+  else process.env.OMNIROUTE_API_KEY = ORIGINAL_OMNIROUTE_API_KEY;
+});
+
+test("GET /api/skills sanitizes an absolute path out of a 500 error body", async () => {
+  const original = skillRegistry.loadFromDatabase.bind(skillRegistry);
+  skillRegistry.loadFromDatabase = async () => {
+    throw new Error(`EACCES: permission denied, open ${LEAKY_PATH}`);
+  };
+
+  try {
+    const res = await skillsRoute.GET(new Request("http://localhost/api/skills"));
+    assert.equal(res.status, 500);
+
+    const body = (await res.json()) as { error?: unknown };
+    // Shape preserved: still a plain string (the dashboard reads data.error as a string).
+    assert.equal(typeof body.error, "string");
+    const error = body.error as string;
+
+    // The leaked absolute path must be gone; sanitizeErrorMessage replaces it with <path>.
+    assert.ok(
+      !error.includes(LEAKY_PATH) && !error.includes("/home/testuser"),
+      `error must not leak the absolute path, got: ${JSON.stringify(error)}`
+    );
+    assert.ok(error.length > 0, "error should still carry a non-empty message");
+  } finally {
+    skillRegistry.loadFromDatabase = original;
+  }
+});