From 628ab5692e1825001eac0aaa92638c0067a508ea Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 6 May 2026 23:48:32 +0000 Subject: [PATCH] fix(cli): detect upstream auth-gateway HTML responses and add poll heartbeat (#9065) * fix(cli): detect upstream auth-gateway HTML responses and add poll heartbeat * fix(cli): guard tar fallback, tighten cheap-path, case-insensitive ct, add tests --- cli/src/commands/app/app_metadata.ts | 6 ++ cli/src/commands/flow/flow_metadata.ts | 6 ++ cli/src/commands/sync/pull.ts | 5 + cli/src/core/context.ts | 12 ++- cli/src/main.ts | 5 + cli/src/utils/http_guards.ts | 66 ++++++++++++ cli/src/utils/job_polling.ts | 31 ++++-- cli/src/utils/metadata.ts | 6 ++ cli/test/http_guards_unit.test.ts | 142 +++++++++++++++++++++++++ 9 files changed, 267 insertions(+), 12 deletions(-) create mode 100644 cli/src/utils/http_guards.ts create mode 100644 cli/test/http_guards_unit.test.ts diff --git a/cli/src/commands/app/app_metadata.ts b/cli/src/commands/app/app_metadata.ts index 0a098af05a..d34755d589 100644 --- a/cli/src/commands/app/app_metadata.ts +++ b/cli/src/commands/app/app_metadata.ts @@ -24,6 +24,7 @@ import { workspaceDependenciesLanguages, } from "../../utils/script_common.ts"; import { generateHash, getHeaders, readTextFile, writeIfChanged } from "../../utils/utils.ts"; +import { detectAuthGatewayChallenge } from "../../utils/http_guards.ts"; import { exts } from "../script/script.ts"; import { FSFSElement, yamlOptions } from "../sync/sync.ts"; import { Workspace } from "../workspace/workspace.ts"; @@ -782,6 +783,11 @@ async function generateInlineScriptLock( } ); + await detectAuthGatewayChallenge( + queueResponse, + `${workspace.remote}api/w/${workspace.workspaceId}/jobs/run/dependencies_async`, + ); + if (!queueResponse.ok) { const text = await queueResponse.text(); throw new Error( diff --git a/cli/src/commands/flow/flow_metadata.ts b/cli/src/commands/flow/flow_metadata.ts index 25f074a50d..f33ef07bcc 100644 --- a/cli/src/commands/flow/flow_metadata.ts +++ b/cli/src/commands/flow/flow_metadata.ts @@ -21,6 +21,7 @@ import { extractInlineScripts as extractInlineScriptsForFlows, extractCurrentMap import { newPathAssigner } from "../../../windmill-utils-internal/src/path-utils/path-assigner.ts"; import { generateHash, getHeaders, readTextFile, writeIfChanged } from "../../utils/utils.ts"; +import { detectAuthGatewayChallenge } from "../../utils/http_guards.ts"; import { exts } from "../script/script.ts"; import { FSFSElement, yamlOptions } from "../sync/sync.ts"; import { Workspace } from "../workspace/workspace.ts"; @@ -426,6 +427,11 @@ export async function updateFlow( } ); + await detectAuthGatewayChallenge( + queueResponse, + `${workspace.remote}api/w/${workspace.workspaceId}/jobs/run/flow_dependencies_async`, + ); + if (!queueResponse.ok) { let bodyText = ""; try { diff --git a/cli/src/commands/sync/pull.ts b/cli/src/commands/sync/pull.ts index 0b4da237e1..b89d81b91c 100644 --- a/cli/src/commands/sync/pull.ts +++ b/cli/src/commands/sync/pull.ts @@ -7,6 +7,7 @@ import { extract } from "tar-stream"; import { Readable } from "node:stream"; import { Workspace } from "../workspace/workspace.ts"; import { getHeaders } from "../../utils/utils.ts"; +import { detectAuthGatewayChallenge } from "../../utils/http_guards.ts"; /** * Adapter that wraps tar entries in a JSZip-compatible interface @@ -110,6 +111,8 @@ export async function downloadZip( const zipUrl = baseUrl + "archive_type=zip" + baseParams; const zipResponse = await fetch(zipUrl, { headers: requestHeaders, method: "GET" }); + await detectAuthGatewayChallenge(zipResponse, zipUrl); + if (zipResponse.ok) { log.debug("Downloaded zip archive successfully"); const blob = await zipResponse.blob(); @@ -124,6 +127,8 @@ export async function downloadZip( const tarUrl = baseUrl + "archive_type=tar" + baseParams; const tarResponse = await fetch(tarUrl, { headers: requestHeaders, method: "GET" }); + await detectAuthGatewayChallenge(tarResponse, tarUrl); + if (tarResponse.ok) { log.debug("Downloaded tar archive successfully"); return await parseTarResponse(tarResponse); diff --git a/cli/src/core/context.ts b/cli/src/core/context.ts index dc822e5b98..a4ec705473 100644 --- a/cli/src/core/context.ts +++ b/cli/src/core/context.ts @@ -8,6 +8,7 @@ import { Table } from "@cliffy/table"; import { loginInteractive } from "./login.ts"; import { GlobalOptions } from "../types.ts"; import { getHeaders } from "../utils/utils.ts"; +import { detectAuthGatewayChallenge } from "../utils/http_guards.ts"; import { getActiveWorkspace, @@ -704,10 +705,13 @@ export async function fetchVersion(baseUrl: string): Promise { } } - const response = await fetch( - new URL(new URL(baseUrl).origin + "/api/version"), - { headers: requestHeaders, method: "GET" } - ); + const versionUrl = new URL(new URL(baseUrl).origin + "/api/version"); + const response = await fetch(versionUrl, { + headers: requestHeaders, + method: "GET", + }); + + await detectAuthGatewayChallenge(response, versionUrl.toString()); if (!response.ok) { // Consume response body even on error to avoid resource leak diff --git a/cli/src/main.ts b/cli/src/main.ts index c037930ea9..50c173bce5 100755 --- a/cli/src/main.ts +++ b/cli/src/main.ts @@ -29,6 +29,7 @@ import dev from "./commands/dev/dev.ts"; import { GlobalOptions } from "./types.ts"; import { OpenAPI } from "../gen/index.ts"; import { getHeaders } from "./utils/utils.ts"; +import { detectAuthGatewayChallenge } from "./utils/http_guards.ts"; import { setShowDiffs } from "./core/conf.ts"; import { NpmProvider } from "./utils/upgrade.ts"; import { pull as hubPull } from "./commands/hub/hub.ts"; @@ -277,6 +278,10 @@ async function main() { if (extraHeaders) { OpenAPI.HEADERS = extraHeaders; } + OpenAPI.interceptors.response.use(async (response) => { + await detectAuthGatewayChallenge(response); + return response; + }); await command.parse(args); } catch (e) { if (e && typeof e === "object" && "name" in e && e.name === "ApiError") { diff --git a/cli/src/utils/http_guards.ts b/cli/src/utils/http_guards.ts new file mode 100644 index 0000000000..3d5c415a0f --- /dev/null +++ b/cli/src/utils/http_guards.ts @@ -0,0 +1,66 @@ +// Detects responses that look like an upstream auth-gateway challenge +// (Cloudflare Access SSO page, Vercel auth wall, basic-auth form, etc.) and +// surfaces a clear error instead of letting the caller treat the HTML body as +// a typed Windmill response. + +const ACCESS_TITLE = /\s*Sign in[^<]*Cloudflare Access\s*<\/title>/i; +const HTML_DOCTYPE = /^\s*<(!doctype|html)/i; + +export class AuthGatewayChallengeError extends Error { + override name = "AuthGatewayChallengeError"; + constructor( + public url: string, + public cfRay: string | undefined, + public cfMitigated: string | undefined, + public status: number, + public bodySnippet: string, + ) { + const cfPart = [ + cfRay ? `cf-ray=${cfRay}` : null, + cfMitigated ? `cf-mitigated=${cfMitigated}` : null, + ] + .filter(Boolean) + .join(", "); + super( + `Got an HTML response from ${url} (status ${status}${cfPart ? `, ${cfPart}` : ""}). ` + + `The request was intercepted by an upstream auth gateway (likely Cloudflare Access) ` + + `before reaching Windmill. Verify the runner is on the right network or pass service-token headers via the HEADERS env var ` + + `(e.g. HEADERS="CF-Access-Client-Id: <id>, CF-Access-Client-Secret: <secret>"). ` + + `Body starts with: ${JSON.stringify(bodySnippet.slice(0, 120))}`, + ); + } +} + +export async function detectAuthGatewayChallenge( + response: Response, + url?: string, +): Promise<void> { + const contentType = (response.headers.get("content-type") ?? "").toLowerCase(); + const cfMitigated = response.headers.get("cf-mitigated") ?? undefined; + const looksHtml = contentType.includes("text/html"); + + // Cheap check first; only peek the body when something already smells off. + if (!looksHtml && cfMitigated !== "challenge") return; + + let snippet = ""; + try { + snippet = (await response.clone().text()).slice(0, 256); + } catch { + /* body unreadable — fall through */ + } + + const isChallenge = + cfMitigated === "challenge" || + ACCESS_TITLE.test(snippet) || + (looksHtml && HTML_DOCTYPE.test(snippet)); + + if (!isChallenge) return; + + throw new AuthGatewayChallengeError( + url || response.url || "(unknown)", + response.headers.get("cf-ray") ?? undefined, + cfMitigated, + response.status, + snippet, + ); +} diff --git a/cli/src/utils/job_polling.ts b/cli/src/utils/job_polling.ts index f55f7f3485..bf72c2bfe5 100644 --- a/cli/src/utils/job_polling.ts +++ b/cli/src/utils/job_polling.ts @@ -6,6 +6,7 @@ const DEFAULT_FAST_POLL_INTERVAL_MS = 100; const DEFAULT_FAST_POLL_DURATION_MS = 2000; const DEFAULT_SLOW_POLL_INTERVAL_MS = 2000; const QUEUE_LOG_INTERVAL_MS = 5000; +const HEARTBEAT_INTERVAL_MS = 60000; const MAX_CONSECUTIVE_POLL_ERRORS = 10; export type JobCompletion = { result: unknown; success: boolean }; @@ -14,32 +15,32 @@ export async function logQueueStatus( workspace: string, jobId: string, label: string = "Job ", -): Promise<void> { +): Promise<boolean> { try { const job: any = await wmill.getJob({ workspace, id: jobId }); - if (!job) return; + if (!job) return false; if (job.running === true) { log.info( colors.gray(`${label}${jobId}: running, waiting for completion...`), ); - return; + return true; } if (typeof job.running !== "boolean") { - return; + return false; } const scheduledFor = job.scheduled_for as string | undefined; if (!scheduledFor) { log.info(colors.gray(`${label}${jobId}: queued, waiting for executor...`)); - return; + return true; } const scheduledForMs = new Date(scheduledFor).getTime(); if (!Number.isFinite(scheduledForMs)) { log.info(colors.gray(`${label}${jobId}: queued, waiting for executor...`)); - return; + return true; } try { @@ -59,11 +60,13 @@ export async function logQueueStatus( colors.gray(`${label}${jobId}: queued, waiting for executor...`), ); } + return true; } catch { log.info(colors.gray(`${label}${jobId}: queued, waiting for executor...`)); + return true; } } catch { - // getJob may fail transiently; ignore and retry on next tick + return false; } } @@ -86,6 +89,7 @@ export async function pollJobWithQueueLogging( const label = options?.label ? `[${options.label}] ` : "Job "; const startedAt = Date.now(); let lastQueueLogAt = Date.now(); + let lastHeartbeatAt = Date.now(); let consecutiveErrors = 0; while (true) { @@ -108,6 +112,7 @@ export async function pollJobWithQueueLogging( `${label}${jobId}: error checking job status (${consecutiveErrors}/${MAX_CONSECUTIVE_POLL_ERRORS}): ${err?.message ?? err}`, ), ); + lastHeartbeatAt = Date.now(); if (consecutiveErrors >= MAX_CONSECUTIVE_POLL_ERRORS) { throw new Error( `Giving up polling job ${jobId} after ${MAX_CONSECUTIVE_POLL_ERRORS} consecutive errors. Last error: ${err?.message ?? err}`, @@ -117,7 +122,17 @@ export async function pollJobWithQueueLogging( if (Date.now() - lastQueueLogAt >= QUEUE_LOG_INTERVAL_MS) { lastQueueLogAt = Date.now(); - await logQueueStatus(workspace, jobId, label); + const logged = await logQueueStatus(workspace, jobId, label); + if (logged) lastHeartbeatAt = Date.now(); + } + + if (Date.now() - lastHeartbeatAt >= HEARTBEAT_INTERVAL_MS) { + lastHeartbeatAt = Date.now(); + log.info( + colors.gray( + `${label}${jobId}: still polling, queue status unavailable...`, + ), + ); } const delayMs = diff --git a/cli/src/utils/metadata.ts b/cli/src/utils/metadata.ts index 7aa4372ebf..456336dae2 100644 --- a/cli/src/utils/metadata.ts +++ b/cli/src/utils/metadata.ts @@ -22,6 +22,7 @@ import { inferContentTypeFromFilePath } from "./script_common.ts"; import { getModuleFolderSuffix, isModuleEntryPoint, scriptPathToRemotePath } from "./resource_folders.ts"; import { findCodebase, yamlOptions } from "../commands/sync/sync.ts"; import { generateHash, readInlinePathSync, getHeaders, readTextFile, readTextFileSync } from "./utils.ts"; +import { detectAuthGatewayChallenge } from "./http_guards.ts"; import { SyncCodebase } from "./codebase.ts"; import { argSigToJsonSchemaType } from "../../windmill-utils-internal/src/parse/parse-schema.ts"; @@ -642,6 +643,11 @@ async function fetchScriptLock( } ); + await detectAuthGatewayChallenge( + queueResponse, + `${workspace.remote}api/w/${workspace.workspaceId}/jobs/run/dependencies_async`, + ); + if (!queueResponse.ok) { let bodyText = ""; try { diff --git a/cli/test/http_guards_unit.test.ts b/cli/test/http_guards_unit.test.ts new file mode 100644 index 0000000000..f45d4433c4 --- /dev/null +++ b/cli/test/http_guards_unit.test.ts @@ -0,0 +1,142 @@ +import { expect, test } from "bun:test"; +import { + AuthGatewayChallengeError, + detectAuthGatewayChallenge, +} from "../src/utils/http_guards.ts"; + +const URL = "https://windmill.example.com/api/w/dev/scripts/create"; + +function htmlResponse(body: string, headers: Record<string, string> = {}) { + return new Response(body, { + status: 200, + headers: { "content-type": "text/html; charset=utf-8", ...headers }, + }); +} + +test("throws on Cloudflare Access SSO HTML", async () => { + const body = + '<!DOCTYPE html><title>Sign in ・ Cloudflare Access...'; + const res = htmlResponse(body, { + "cf-ray": "8a1234abcdef-ATL", + "cf-mitigated": "challenge", + }); + + await expect(detectAuthGatewayChallenge(res, URL)).rejects.toBeInstanceOf( + AuthGatewayChallengeError, + ); +}); + +test("error includes cf-ray, cf-mitigated, status, body snippet", async () => { + const body = 'Sign in ・ Cloudflare Access'; + const res = htmlResponse(body, { + "cf-ray": "ray-123", + "cf-mitigated": "challenge", + }); + + try { + await detectAuthGatewayChallenge(res, URL); + throw new Error("expected to throw"); + } catch (e) { + expect(e).toBeInstanceOf(AuthGatewayChallengeError); + const err = e as AuthGatewayChallengeError; + expect(err.url).toBe(URL); + expect(err.cfRay).toBe("ray-123"); + expect(err.cfMitigated).toBe("challenge"); + expect(err.status).toBe(200); + expect(err.message).toContain("ray-123"); + expect(err.message).toContain("cf-mitigated=challenge"); + expect(err.message).toContain("Sign in"); + } +}); + +test("throws on generic HTML body even without cf headers", async () => { + const body = "Login required"; + const res = htmlResponse(body); + + await expect(detectAuthGatewayChallenge(res, URL)).rejects.toBeInstanceOf( + AuthGatewayChallengeError, + ); +}); + +test("throws on cf-mitigated=challenge even with non-HTML content type", async () => { + // CF Access can challenge non-HTML responses too; the header alone is enough. + const res = new Response('Sign in ・ Cloudflare Access', { + status: 200, + headers: { + "content-type": "application/octet-stream", + "cf-mitigated": "challenge", + }, + }); + + await expect(detectAuthGatewayChallenge(res, URL)).rejects.toBeInstanceOf( + AuthGatewayChallengeError, + ); +}); + +test("normalizes content-type case (Text/HTML)", async () => { + const body = ""; + const res = new Response(body, { + status: 200, + headers: { "content-type": "Text/HTML; charset=UTF-8" }, + }); + + await expect(detectAuthGatewayChallenge(res, URL)).rejects.toBeInstanceOf( + AuthGatewayChallengeError, + ); +}); + +test("does NOT throw on JSON happy-path response", async () => { + const res = new Response(JSON.stringify({ id: "abc" }), { + status: 200, + headers: { "content-type": "application/json" }, + }); + + await expect(detectAuthGatewayChallenge(res, URL)).resolves.toBeUndefined(); +}); + +test("does NOT throw on cf-mitigated values other than 'challenge' (e.g. 'block') with non-HTML body", async () => { + // If CF marks something with cf-mitigated: block on a non-HTML response, the + // cheap path should skip the challenge predicate entirely. + const res = new Response('{"error":"blocked"}', { + status: 403, + headers: { + "content-type": "application/json", + "cf-mitigated": "block", + }, + }); + + await expect(detectAuthGatewayChallenge(res, URL)).resolves.toBeUndefined(); +}); + +test("does NOT throw on text/plain body that isn't an auth challenge", async () => { + const res = new Response("just some plaintext", { + status: 200, + headers: { "content-type": "text/plain" }, + }); + + await expect(detectAuthGatewayChallenge(res, URL)).resolves.toBeUndefined(); +}); + +test("does NOT throw when body is empty", async () => { + // Body absent (e.g. 204) — treat as non-challenge regardless of headers. + const res = new Response(null, { + status: 204, + headers: { "content-type": "text/html" }, + }); + + await expect(detectAuthGatewayChallenge(res, URL)).resolves.toBeUndefined(); +}); + +test("falls back to response.url when caller url is omitted", async () => { + const body = "Sign in ・ Cloudflare Access"; + const res = htmlResponse(body); + + try { + await detectAuthGatewayChallenge(res); + throw new Error("expected to throw"); + } catch (e) { + const err = e as AuthGatewayChallengeError; + // Response constructor doesn't set .url, so we expect the unknown sentinel. + expect(err.url).toBe("(unknown)"); + } +});