fix(providers): treat unreliable web-cookie /models probe status as unsupported, not valid (#7857) (#7959)

This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-07-21 08:39:32 -03:00
committed by GitHub
parent 25499bf94d
commit 5996acdcc7
5 changed files with 113 additions and 4 deletions

View File

@@ -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)

View File

@@ -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.

View File

@@ -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 &&

View File

@@ -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 () => {

View File

@@ -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"
);
});