diff --git a/changelog.d/fixes/1037-vercel-relay-sso-protection-check.md b/changelog.d/fixes/1037-vercel-relay-sso-protection-check.md new file mode 100644 index 00000000000..e03d00dd02d --- /dev/null +++ b/changelog.d/fixes/1037-vercel-relay-sso-protection-check.md @@ -0,0 +1 @@ +- **fix(api):** Vercel Relay deploy now checks the Deployment Protection (SSO) PATCH response and surfaces `ssoProtectionWarning` when Vercel rejects it, instead of silently activating a relay that later returns an undiagnosed `403 Access denied`. (thanks @ricatix) diff --git a/src/app/api/settings/proxy/vercel-deploy/route.ts b/src/app/api/settings/proxy/vercel-deploy/route.ts index 42f432f2c6b..6d6b859396e 100644 --- a/src/app/api/settings/proxy/vercel-deploy/route.ts +++ b/src/app/api/settings/proxy/vercel-deploy/route.ts @@ -98,6 +98,92 @@ export default async function handler(req) { */ export const __buildRelayFunctionForTest = buildRelayFunction; +/** + * Disable Vercel project SSO/Deployment Protection so the relay is publicly + * reachable. The PATCH response was previously fired-and-forgotten + * (`.catch(() => {})`, no `res.ok` check) — if Vercel rejects or no-ops the + * request (plan does not allow disabling protection, an under-scoped token, + * etc.), the relay still got saved and activated as a healthy proxy pool, + * and later requests through it failed with an undiagnosed + * `403 Access denied` from Vercel's own deployment protection. Callers must + * now check `.ok` and surface the failure instead of assuming success. + */ +async function disableSsoProtection( + vercelApiBase: string, + projectId: string, + token: string +): Promise<{ ok: boolean; status?: number }> { + try { + const res = await fetch(`${vercelApiBase}/v9/projects/${projectId}`, { + method: "PATCH", + headers: { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ ssoProtection: null }), + }); + return { ok: res.ok, status: res.status }; + } catch { + return { ok: false }; + } +} + +/** + * Test-only hook exposing `disableSsoProtection` so the regression test can + * assert the PATCH response is checked instead of silently swallowed. Not + * part of the route contract. + */ +export const __disableSsoProtectionForTest = disableSsoProtection; + +/** + * Builds the sanitized error response for a rejected Vercel deployment + * request. Extracted from POST to keep the handler's cognitive complexity + * within the ratchet — parses the canonical `{ error: { message } } }` shape + * and never forwards raw upstream error text (may contain project IDs, team + * slugs, deployment hashes or internal Vercel error strings). + */ +async function buildDeployErrorResponse(deployRes: Response) { + let upstreamMessage = "Vercel API rejected the deployment"; + try { + const parsed = (await deployRes.json().catch(() => null)) as { + error?: { message?: string }; + } | null; + const candidate = parsed?.error?.message; + if (typeof candidate === "string" && candidate.trim()) { + upstreamMessage = candidate.trim().slice(0, 200); + } + } catch { + /* fall through to generic message */ + } + return createErrorResponse({ + status: deployRes.status, + message: `Vercel deployment failed: ${upstreamMessage}`, + type: "upstream_error", + }); +} + +/** + * Disables Vercel SSO/Deployment Protection for the deployed project and + * returns a caller-facing warning when it could not be disabled. Extracted + * from POST to keep the handler's cognitive complexity within the ratchet. + * See `disableSsoProtection` doc comment for the bug this guards against. + */ +async function resolveSsoProtectionWarning( + projectId: string | undefined, + vercelApiBase: string, + token: string +): Promise { + if (!projectId) return undefined; + const ssoResult = await disableSsoProtection(vercelApiBase, projectId, token); + if (ssoResult.ok) return undefined; + return ( + "Could not disable Vercel Deployment Protection (SSO) for this project" + + (ssoResult.status ? ` (status ${ssoResult.status})` : "") + + ". Requests through this relay may fail with a 403 Access denied from " + + "Vercel until protection is disabled manually in the Vercel dashboard." + ); +} + async function pollDeployment(deploymentApiUrl: string, token: string): Promise<"READY" | "ERROR"> { for (let i = 0; i < POLL_MAX_ATTEMPTS; i++) { await new Promise((r) => setTimeout(r, POLL_INTERVAL_MS)); @@ -171,27 +257,9 @@ export async function POST(request: Request) { }); if (!deployRes.ok) { - // Avoid forwarding 200 bytes of raw Vercel error text — it may contain - // project IDs, team slugs, deployment hashes or internal Vercel error - // strings. Parse the canonical { error: { message } } shape and surface - // only the human-readable message (or a generic fallback). - let upstreamMessage = "Vercel API rejected the deployment"; - try { - const parsed = (await deployRes.json().catch(() => null)) as { - error?: { message?: string }; - } | null; - const candidate = parsed?.error?.message; - if (typeof candidate === "string" && candidate.trim()) { - upstreamMessage = candidate.trim().slice(0, 200); - } - } catch { - /* fall through to generic message */ - } - return createErrorResponse({ - status: deployRes.status, - message: `Vercel deployment failed: ${upstreamMessage}`, - type: "upstream_error", - }); + // Avoid forwarding raw Vercel error text — it may contain project IDs, + // team slugs, deployment hashes or internal Vercel error strings. + return buildDeployErrorResponse(deployRes); } const deployment = (await deployRes.json()) as { @@ -208,17 +276,17 @@ export async function POST(request: Request) { }); } - // Disable Vercel SSO protection so the relay is publicly accessible - if (deployment.projectId) { - await fetch(`${VERCEL_API_BASE}/v9/projects/${deployment.projectId}`, { - method: "PATCH", - headers: { - Authorization: `Bearer ${token}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ ssoProtection: null }), - }).catch(() => {}); - } + // Disable Vercel SSO protection so the relay is publicly accessible. + // The PATCH response is checked — if Vercel rejects/no-ops it (plan + // doesn't allow disabling protection, under-scoped token, etc.) the + // relay is still deployed and saved, but the caller is warned so a + // later `403 Access denied` can be diagnosed as Vercel-side deployment + // protection rather than an upstream provider rejection. + const ssoProtectionWarning = await resolveSsoProtectionWarning( + deployment.projectId, + VERCEL_API_BASE, + token + ); // Poll until READY const deploymentApiUrl = `${VERCEL_API_BASE}/v13/deployments/${deployment.id}`; @@ -254,6 +322,7 @@ export async function POST(request: Request) { success: true, relayUrl: `https://${deployment.url}`, poolProxyId: poolProxy?.id, + ...(ssoProtectionWarning ? { ssoProtectionWarning } : {}), }); } catch (error) { return createErrorResponseFromUnknown(error, "Vercel deploy failed"); diff --git a/tests/unit/vercel-deploy-sso-protection-check.test.ts b/tests/unit/vercel-deploy-sso-protection-check.test.ts new file mode 100644 index 00000000000..d3fb945e965 --- /dev/null +++ b/tests/unit/vercel-deploy-sso-protection-check.test.ts @@ -0,0 +1,95 @@ +// Regression guard for upstream report: "Vercel Relay with Codex returns 403 +// Access denied and lacks source diagnostics". +// +// Root cause: the Vercel deploy route disables project SSO/Deployment +// Protection via a PATCH request, but fired it with `.catch(() => {})` and +// never inspected `res.ok`. If Vercel rejects or no-ops the PATCH (plan +// doesn't allow disabling protection, stale/under-scoped token, etc.), the +// relay is still saved and activated as a healthy proxy pool — later +// requests routed through it fail with an undiagnosed `403 Access denied` +// from Vercel's own deployment protection, indistinguishable from an +// upstream-provider 403. +// +// Fix: check the PATCH response and surface the failure back to the caller +// (`ssoProtectionWarning` in the JSON response) instead of silently +// swallowing it, so the UI/API consumer can diagnose the 403 source. +import { describe, it, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; +import { __disableSsoProtectionForTest } from "../../src/app/api/settings/proxy/vercel-deploy/route"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "..", ".."); +const ROUTE_PATH = join( + ROOT, + "src/app/api/settings/proxy/vercel-deploy/route.ts" +); + +describe("disableSsoProtection — checks the Vercel PATCH response instead of swallowing it", () => { + const originalFetch = global.fetch; + + afterEach(() => { + global.fetch = originalFetch; + }); + + it("reports failure when Vercel rejects the PATCH (e.g. plan does not allow disabling protection)", async () => { + global.fetch = (async () => + new Response(JSON.stringify({ error: { message: "Forbidden" } }), { + status: 403, + })) as typeof fetch; + + const result = await __disableSsoProtectionForTest( + "https://api.vercel.com", + "proj_123", + "test-token" + ); + + assert.equal(result.ok, false, "must report ok:false on a non-2xx PATCH response"); + assert.equal(result.status, 403); + }); + + it("reports success when Vercel accepts the PATCH", async () => { + global.fetch = (async () => new Response(null, { status: 200 })) as typeof fetch; + + const result = await __disableSsoProtectionForTest( + "https://api.vercel.com", + "proj_123", + "test-token" + ); + + assert.equal(result.ok, true); + }); + + it("reports failure (not a thrown exception) when the PATCH request itself fails", async () => { + global.fetch = (async () => { + throw new Error("network down"); + }) as typeof fetch; + + const result = await __disableSsoProtectionForTest( + "https://api.vercel.com", + "proj_123", + "test-token" + ); + + assert.equal(result.ok, false); + }); +}); + +describe("vercel-deploy route — wires the SSO-protection check into the response", () => { + const src = readFileSync(ROUTE_PATH, "utf8"); + + it("no longer fires the PATCH with a silent `.catch(() => {})`", () => { + assert.ok( + !/ssoProtection:\s*null[\s\S]*?\.catch\(\s*\(\)\s*=>\s*\{\s*\}\s*\)/.test(src), + "the ssoProtection PATCH must not be silently swallowed with .catch(() => {})" + ); + }); + + it("surfaces a warning in the JSON response when disabling SSO protection failed", () => { + assert.ok( + src.includes("ssoProtectionWarning"), + "POST handler must surface ssoProtectionWarning in the response payload when the PATCH failed" + ); + }); +});