diff --git a/changelog.d/fixes/7857-web-cookie-validation-probe.md b/changelog.d/fixes/7857-web-cookie-validation-probe.md new file mode 100644 index 0000000000..cb88be58f1 --- /dev/null +++ b/changelog.d/fixes/7857-web-cookie-validation-probe.md @@ -0,0 +1 @@ +- fix(providers): treat a non-401/403 response from the web-cookie `/models` validation probe (redirect, login-HTML 200, 404, 405, 429) as `unsupported` instead of `valid: true` for providers whose registry `baseUrl` is a conversation/completion endpoint rather than a real API root (#7857) diff --git a/src/lib/providers/validation.ts b/src/lib/providers/validation.ts index 591fb77f35..f1d645b4ba 100644 --- a/src/lib/providers/validation.ts +++ b/src/lib/providers/validation.ts @@ -37,6 +37,7 @@ import { validationWrite, toValidationErrorResult, toWebCookieValidationErrorResult, + WEB_COOKIE_PROVIDERS_WITHOUT_MODELS_API, } from "./validation/transport"; import { validateDeepSeekWebProvider, @@ -162,6 +163,21 @@ export async function validateWebCookieProvider({ // Attempt a minimal request to check if the session is valid // Use /models endpoint or a minimal completion request depending on the provider const baseUrl = normalizeBaseUrl(entry.baseUrl || ""); + + // Defense-in-depth: only an http(s) baseUrl without a query string is safe to + // probe by blindly appending `/models`. A ws(s):// baseUrl (e.g. copilot-web) is + // already rejected by the outbound URL guard downstream, but reject it explicitly + // here for the honest "unsupported" result instead of a confusing security-block + // message — this also covers a future http(s) baseUrl carrying a query string, + // which the guard does not currently block (#7857 acceptance criteria). + if (!/^https?:\/\//i.test(baseUrl) || baseUrl.includes("?")) { + return { + valid: false, + error: "Provider validation not supported", + unsupported: true, + }; + } + const testUrl = `${baseUrl}/models`; const res = await validationRead( @@ -185,6 +201,20 @@ export async function validateWebCookieProvider({ }; } + // #7857: for providers whose baseUrl is a conversation/completion endpoint rather + // than a real API root, the /models path never existed upstream — a redirect, + // login-HTML 200, 404, 405, or 429 from it is not a meaningful auth signal and is + // indistinguishable from a genuinely valid session. Report the same honest + // "unsupported" result the !entry branch above already gives its no-registry + // siblings, instead of a false `valid: true`. + if (WEB_COOKIE_PROVIDERS_WITHOUT_MODELS_API.has(provider)) { + return { + valid: false, + error: "Provider validation not supported", + unsupported: true, + }; + } + // Any other response (200, 404, 405, 429, ...) means the cookie was accepted — // a 401/403 from the /models probe is the only definitive "session expired" signal // for web-cookie auth, so a non-auth status is treated as a valid session. diff --git a/src/lib/providers/validation/transport.ts b/src/lib/providers/validation/transport.ts index 8fb49af5ce..c486ff41fc 100644 --- a/src/lib/providers/validation/transport.ts +++ b/src/lib/providers/validation/transport.ts @@ -107,6 +107,28 @@ export function isSecurityBlockError(error: unknown): boolean { // #7542 plan-file, "Risks"). const WEB_COOKIE_PROVIDERS_WITH_UNRELIABLE_MODELS_PROBE = new Set(["lmarena"]); +// #7857 — web-cookie providers whose registry `baseUrl` is a conversation/completion +// endpoint, not a real API root (e.g. huggingchat's baseUrl is +// "https://huggingface.co/chat/conversation", not "https://huggingface.co"). Appending +// `/models` to these produces a path the upstream never served, so its status +// (200/404/405/429/redirect/login-HTML) carries no meaningful auth signal — it is NOT +// distinguishable from a genuinely valid session. A 401/403 from the same probe IS still +// treated as a real SESSION_EXPIRED signal (some of these hosts auth-gate every path, +// including nonexistent ones), so providers here still get probed; only the non-401/403 +// branch is short-circuited to the honest "unsupported" result instead of `valid: true`. +// lmarena is deliberately NOT in this set — it already degrades via the +// WEB_COOKIE_PROVIDERS_WITH_UNRELIABLE_MODELS_PROBE/REDIRECT_BLOCKED path above (#7542). +export const WEB_COOKIE_PROVIDERS_WITHOUT_MODELS_API = new Set([ + "huggingchat", + "chatgpt-web", + "grok-web", + "notion-web", + "t3-web", + "yuanbao-web", + "copilot-web", + "copilot-m365-web", +]); + export function toWebCookieValidationErrorResult(provider: string, error: unknown) { if ( error instanceof SafeOutboundFetchError && diff --git a/tests/unit/provider-validation-web-cookie-auth007.test.ts b/tests/unit/provider-validation-web-cookie-auth007.test.ts index 344b1e865b..9b665a9d59 100644 --- a/tests/unit/provider-validation-web-cookie-auth007.test.ts +++ b/tests/unit/provider-validation-web-cookie-auth007.test.ts @@ -56,9 +56,13 @@ test("should_return_AUTH_007_when_models_endpoint_returns_403", async () => { assert.equal(result.error, "SESSION_EXPIRED"); }); -test("should_return_valid_when_models_endpoint_returns_200", async () => { - // A non-401/403 status from the /models probe means the cookie was accepted, - // so the session is treated as valid. +test("should_return_unsupported_when_models_endpoint_returns_200_for_a_conversation_baseUrl", async () => { + // #7857: chatgpt-web's registry baseUrl is a conversation endpoint + // ("https://chatgpt.com/backend-api/conversation"), not a real API root, so + // "${baseUrl}/models" is a path that never existed upstream. A 200 from that + // nonsense path (e.g. a login-page SPA shell) is not a meaningful auth signal and + // is indistinguishable from a genuinely valid session — it must be reported as + // unsupported, not silently accepted as valid. mockFetch(200, JSON.stringify({ ok: true, data: [] })); const result = await validateWebCookieProvider({ @@ -68,7 +72,8 @@ test("should_return_valid_when_models_endpoint_returns_200", async () => { }); assert.equal(fetchCalls, 1, "the /models probe must be the mocked fetch, not the live network"); - assert.equal(result.valid, true); + assert.equal(result.valid, false); + assert.equal(result.unsupported, true); }); test("should_return_unsupported_when_provider_not_in_registry", async () => { diff --git a/tests/unit/web-cookie-validation-false-positive-7857.test.ts b/tests/unit/web-cookie-validation-false-positive-7857.test.ts new file mode 100644 index 0000000000..7351407874 --- /dev/null +++ b/tests/unit/web-cookie-validation-false-positive-7857.test.ts @@ -0,0 +1,51 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +let nextResponse: { status: number; body: string } = { status: 404, body: "Not Found" }; +let fetchCalls = 0; +let lastUrl = ""; + +const { validateWebCookieProvider } = await import("../../src/lib/providers/validation.ts"); + +globalThis.fetch = (async (input: RequestInfo | URL) => { + fetchCalls++; + lastUrl = typeof input === "string" ? input : input instanceof URL ? input.href : input.url; + return new Response(nextResponse.body, { status: nextResponse.status }); +}) as typeof fetch; + +function mockFetch(status: number, body: string) { + nextResponse = { status, body }; + fetchCalls = 0; + lastUrl = ""; +} + +test("BUG #7857: a 404 from huggingchat's nonsense /models probe path is reported valid:true", async () => { + mockFetch(404, "Not Found"); + const result = await validateWebCookieProvider({ + provider: "huggingchat", + apiKey: "garbage_cookie_value_that_was_never_valid=xyz", + providerSpecificData: {}, + }); + assert.equal(fetchCalls, 1); + assert.equal(lastUrl, "https://huggingface.co/chat/conversation/models"); + assert.equal( + result.valid, + false, + "a 404 from a nonsense probe path must never be reported as a valid cookie session" + ); +}); + +test("BUG #7857: same false positive reproduces for grok-web (conversations/new endpoint)", async () => { + mockFetch(405, "Method Not Allowed"); + const result = await validateWebCookieProvider({ + provider: "grok-web", + apiKey: "sso=garbage; sso-rw=garbage", + providerSpecificData: {}, + }); + assert.equal(fetchCalls, 1); + assert.equal( + result.valid, + false, + "a 405 from a nonsense probe path must never be reported as a valid cookie session" + ); +});