diff --git a/changelog.d/fixes/10849-search-provider-opaque-400.md b/changelog.d/fixes/10849-search-provider-opaque-400.md new file mode 100644 index 0000000000..a8982fb194 --- /dev/null +++ b/changelog.d/fixes/10849-search-provider-opaque-400.md @@ -0,0 +1 @@ +- fix(api): POST /v1/search now replies with a named `Unknown search provider: ` error (and field-named validation messages) instead of an opaque `Invalid request` for unrecognized or short-alias provider ids like `brave`/`serper` (#10849) diff --git a/open-sse/config/searchRegistry.ts b/open-sse/config/searchRegistry.ts index ce20777cb0..9230fa1b0e 100644 --- a/open-sse/config/searchRegistry.ts +++ b/open-sse/config/searchRegistry.ts @@ -303,6 +303,19 @@ export const SEARCH_CREDENTIAL_FALLBACKS: Record = { export const SEARCH_PROVIDER_ALIASES: Record = { "jina-ai": "jina-search", jina: "jina-search", + brave: "brave-search", + serper: "serper-search", + perplexity: "perplexity-search", + exa: "exa-search", + tavily: "tavily-search", + "google-pse": "google-pse-search", + linkup: "linkup-search", + ollama: "ollama-search", + searchapi: "searchapi-search", + youcom: "youcom-search", + searxng: "searxng-search", + zai: "zai-search", + duckduckgo: "duckduckgo-free", }; export function resolveSearchProviderId(providerId: string): string { diff --git a/src/app/api/v1/search/route.ts b/src/app/api/v1/search/route.ts index 7f9b1011aa..29f642211e 100644 --- a/src/app/api/v1/search/route.ts +++ b/src/app/api/v1/search/route.ts @@ -19,7 +19,11 @@ import * as log from "@/sse/utils/logger"; import { toJsonErrorPayload } from "@/shared/utils/upstreamError"; import { enforceApiKeyPolicy } from "@/shared/utils/apiKeyPolicy"; import { v1SearchSchema } from "@/shared/validation/schemas"; -import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; +import { + formatValidationMessage, + isValidationFailure, + validateBody, +} from "@/shared/validation/helpers"; import { recordCost } from "@/domain/costRules"; import { computeCacheKey, @@ -120,7 +124,7 @@ async function postHandler(request: Request, context: unknown) { const validation = validateBody(v1SearchSchema, rawBody); if (isValidationFailure(validation)) { - return errorResponse(HTTP_STATUS.BAD_REQUEST, validation.error.message); + return errorResponse(HTTP_STATUS.BAD_REQUEST, formatValidationMessage(validation.error)); } const body = validation.data; diff --git a/src/shared/validation/helpers.ts b/src/shared/validation/helpers.ts index b10ca0c7bd..4e486c7287 100644 --- a/src/shared/validation/helpers.ts +++ b/src/shared/validation/helpers.ts @@ -56,6 +56,19 @@ export function isValidationFailure( return validation.success === false; } +/** + * Build a human-readable 400 message from a validation failure, naming the + * first offending field instead of the generic "Invalid request" (#10849). + * Intended for routes that reply with a single message string (e.g. + * `errorResponse()`) rather than the full `{ message, details }` envelope + * returned by `validatedJsonBody()`. + */ +export function formatValidationMessage(error: ValidationErrorPayload): string { + const [first] = error.details; + if (!first) return error.message; + return first.field ? `${first.field}: ${first.message}` : first.message; +} + /** * Result of attempting to parse and validate a JSON body against a Zod schema. * diff --git a/src/shared/validation/schemas/apiV1.ts b/src/shared/validation/schemas/apiV1.ts index ba8f61067b..35c53d3f5d 100644 --- a/src/shared/validation/schemas/apiV1.ts +++ b/src/shared/validation/schemas/apiV1.ts @@ -568,27 +568,16 @@ export const v1SearchSchema = z .trim() .min(1, "Query is required") .max(500, "Query must be 500 characters or fewer"), - provider: z - .enum([ - "serper-search", - "brave-search", - "perplexity-search", - "exa-search", - "tavily-search", - "firecrawl", - "google-pse-search", - "linkup-search", - "ollama-search", - "searchapi-search", - "youcom-search", - "searxng-search", - "zai-search", - "jina-search", - "jina-ai", - "jina", - "duckduckgo-free", - ]) - .optional(), + // Not a z.enum: the runtime catalog (SEARCH_PROVIDERS + SEARCH_PROVIDER_ALIASES in + // open-sse/config/searchRegistry.ts) is the source of truth via resolveSearchProvider(), + // which already returns a named "Unknown search provider: " error for bad ids (see + // src/app/api/v1/search/route.ts). A hard-coded enum here would 400 before that check + // ever runs, hiding the informative message behind a generic Zod failure (#10849). + // Known catalog ids as of this writing: serper-search, brave-search, perplexity-search, + // exa-search, tavily-search, firecrawl, google-pse-search, linkup-search, ollama-search, + // searchapi-search, youcom-search, searxng-search, zai-search, jina-search, jina-ai, + // jina, duckduckgo-free (plus short aliases resolved by SEARCH_PROVIDER_ALIASES). + provider: z.string().min(1).optional(), max_results: z.coerce.number().int().min(1).max(100).default(5), search_type: z.enum(["web", "news"]).default("web"), offset: z.coerce.number().int().min(0).default(0), diff --git a/tests/unit/firecrawl-search.test.ts b/tests/unit/firecrawl-search.test.ts index 601fa1a8ad..c37a6a9b99 100644 --- a/tests/unit/firecrawl-search.test.ts +++ b/tests/unit/firecrawl-search.test.ts @@ -12,8 +12,13 @@ import test from "node:test"; import assert from "node:assert/strict"; -const { SEARCH_PROVIDERS, SEARCH_CREDENTIAL_FALLBACKS, getSearchProvider, selectProvider } = - await import("../../open-sse/config/searchRegistry.ts"); +const { + SEARCH_PROVIDERS, + SEARCH_CREDENTIAL_FALLBACKS, + getSearchProvider, + selectProvider, + resolveSearchProvider, +} = await import("../../open-sse/config/searchRegistry.ts"); const { handleSearch } = await import("../../open-sse/handlers/search.ts"); const { v1SearchSchema } = await import("../../src/shared/validation/schemas.ts"); @@ -54,8 +59,18 @@ test("v1SearchSchema accepts firecrawl for search (unified id)", () => { search_type: "news", }); assert.equal(news.success, true); + // #10849: v1SearchSchema.provider is a free-form string, not a hard-coded enum, so + // the runtime catalog (resolveSearchProvider()) is the source of truth for whether an + // id is valid — the legacy "firecrawl-search" id is still rejected, just downstream of + // the schema (route.ts replies "Unknown search provider: firecrawl-search") instead of + // by an opaque schema-level 400. const legacy = v1SearchSchema.safeParse({ query: "q", provider: "firecrawl-search" }); - assert.equal(legacy.success, false, "legacy firecrawl-search id is not accepted"); + assert.equal(legacy.success, true, "provider is a free-form string at the schema layer"); + assert.equal( + resolveSearchProvider("firecrawl-search"), + null, + "legacy firecrawl-search id does not resolve to a registered provider" + ); }); test("handleSearch firecrawl hits /v2/search with sources web and normalizes data.web", async () => { diff --git a/tests/unit/search-provider-opaque-400-10849.test.ts b/tests/unit/search-provider-opaque-400-10849.test.ts new file mode 100644 index 0000000000..431675f3bb --- /dev/null +++ b/tests/unit/search-provider-opaque-400-10849.test.ts @@ -0,0 +1,69 @@ +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-search-10849-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const searchRoute = await import("../../src/app/api/v1/search/route.ts"); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +function makeRequest(body: unknown) { + return new Request("http://localhost/v1/search", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +type ErrorBody = { error?: { message: string } }; + +test("#10849: unknown provider id returns 'Unknown search provider: ', not opaque 'Invalid request'", async () => { + const response = await searchRoute.POST(makeRequest({ query: "test", provider: "grok" }), {}); + const body = (await response.json()) as ErrorBody; + + assert.equal(response.status, 400); + assert.match( + body.error?.message ?? "", + /Unknown search provider: grok/, + `expected a named-provider message, got: ${body.error?.message}` + ); +}); + +test("#10849: short alias 'brave' resolves like existing 'jina' aliases (not an opaque 400)", async () => { + const response = await searchRoute.POST(makeRequest({ query: "test", provider: "brave" }), {}); + const body = (await response.json()) as ErrorBody; + + assert.notEqual( + body.error?.message, + "Invalid request", + `expected a named provider error, got opaque: ${JSON.stringify(body.error)}` + ); +}); + +test("#10849: a genuinely bad field surfaces a non-generic, field-named 400 message", async () => { + const response = await searchRoute.POST( + makeRequest({ query: "test", search_type: "bogus" }), + {} + ); + const body = (await response.json()) as ErrorBody; + + assert.equal(response.status, 400); + assert.notEqual( + body.error?.message, + "Invalid request", + `expected a field-named message, got opaque: ${JSON.stringify(body.error)}` + ); + assert.match( + body.error?.message ?? "", + /search_type/, + `expected the message to name the offending field, got: ${body.error?.message}` + ); +}); diff --git a/tests/unit/search-registry.test.ts b/tests/unit/search-registry.test.ts index d4b2eea735..b5df867b18 100644 --- a/tests/unit/search-registry.test.ts +++ b/tests/unit/search-registry.test.ts @@ -381,11 +381,17 @@ test("v1SearchSchema rejects query over 500 chars", async () => { assert.ok(!result.success); }); -test("v1SearchSchema rejects invalid provider", async () => { +test("v1SearchSchema accepts any non-empty provider string; the catalog rejects unknown ids (#10849)", async () => { const { v1SearchSchema } = await import("../../src/shared/validation/schemas.ts"); + const { resolveSearchProvider } = await import("../../open-sse/config/searchRegistry.ts"); + // provider is a free-form string at the schema layer — resolveSearchProvider() (backing + // POST /v1/search) is the runtime source of truth, and returns null for unknown ids so + // the route can reply with a named "Unknown search provider: " error instead of an + // opaque schema-level 400. const result = v1SearchSchema.safeParse({ query: "test", provider: "google" }); - assert.ok(!result.success); + assert.ok(result.success); + assert.equal(resolveSearchProvider("google"), null); }); test("v1SearchSchema accepts tavily provider", async () => {