diff --git a/cli/src/commands/app/dev.ts b/cli/src/commands/app/dev.ts index 5b78f84e3f..7edb793bea 100644 --- a/cli/src/commands/app/dev.ts +++ b/cli/src/commands/app/dev.ts @@ -14,7 +14,8 @@ import * as path from "node:path"; import process from "node:process"; import { Buffer } from "node:buffer"; import { writeFileSync } from "node:fs"; -import { readTextFile } from "../../utils/utils.ts"; +import { getHeaders, readTextFile } from "../../utils/utils.ts"; +import { detectAuthGatewayChallenge } from "../../utils/http_guards.ts"; import { WebSocket, WebSocketServer } from "ws"; import { createFrameworkPlugins, @@ -1714,13 +1715,17 @@ async function streamJobWithSSE( const sseUrl = `${baseUrl}api/w/${workspace}/jobs_u/getupdate_sse/${jobId}?fast=true`; + const extraHeaders = getHeaders(); const response = await fetch(sseUrl, { headers: { Accept: "text/event-stream", Authorization: `Bearer ${token}`, + ...extraHeaders, }, }); + await detectAuthGatewayChallenge(response, sseUrl); + if (!response.ok) { throw new Error( `SSE request failed: ${response.status} ${response.statusText}`, diff --git a/cli/src/commands/docs/docs.ts b/cli/src/commands/docs/docs.ts index d86d4ca367..fed91201e0 100644 --- a/cli/src/commands/docs/docs.ts +++ b/cli/src/commands/docs/docs.ts @@ -4,6 +4,8 @@ import * as log from "../../core/log.ts"; import { requireLogin } from "../../core/auth.ts"; import { resolveWorkspace } from "../../core/context.ts"; import { GlobalOptions } from "../../types.ts"; +import { getHeaders } from "../../utils/utils.ts"; +import { detectAuthGatewayChallenge } from "../../utils/http_guards.ts"; interface DocContentItem { title: string; @@ -36,6 +38,7 @@ async function docs( console.log(colors.bold(`\nSearching Windmill docs...\n`)); + const extraHeaders = getHeaders(); let res: Response; try { res = await fetch(url, { @@ -43,6 +46,7 @@ async function docs( headers: { "Content-Type": "application/json", Authorization: `Bearer ${workspace.token}`, + ...extraHeaders, }, body: JSON.stringify({ query }), }); @@ -50,6 +54,8 @@ async function docs( throw new Error(`Network error connecting to ${workspace.remote}: ${e}`); } + await detectAuthGatewayChallenge(res, url); + if (res.status === 403) { log.info( "Windmill documentation search is an Enterprise Edition feature. Please upgrade to use this command." diff --git a/cli/src/commands/script/script.ts b/cli/src/commands/script/script.ts index 0635cd42ea..f1e605d8ff 100644 --- a/cli/src/commands/script/script.ts +++ b/cli/src/commands/script/script.ts @@ -12,7 +12,8 @@ import * as log from "../../core/log.ts"; import { sep as SEP } from "node:path"; import * as path from "node:path"; import { stringify as yamlStringify } from "yaml"; -import { deepEqual, readTextFile, readTextFileSync } from "../../utils/utils.ts"; +import { deepEqual, getHeaders, readTextFile, readTextFileSync } from "../../utils/utils.ts"; +import { detectAuthGatewayChallenge } from "../../utils/http_guards.ts"; import * as wmill from "../../../gen/services.gen.ts"; import * as specificItems from "../../core/specific_items.ts"; import { getCurrentGitBranch } from "../../utils/git.ts"; @@ -725,6 +726,7 @@ async function createScript( // (same content, lockfile, and metadata) as a no-op, so the CLI does not // produce phantom git-sync / promotion commits on re-pushes. const skipIfNoop = "skip_if_noop=true"; + const extraHeaders = getHeaders(); if (!bundleContent) { try { const url = @@ -738,9 +740,11 @@ async function createScript( headers: { Authorization: `Bearer ${workspace.token}`, "Content-Type": "application/json", + ...extraHeaders, }, body: JSON.stringify(body), }); + await detectAuthGatewayChallenge(req, url); if (req.status != 201) { throw Error( `${req.status} - ${req.statusText} - ${await req.text()}` @@ -771,9 +775,13 @@ async function createScript( skipIfNoop; const req = await fetch(url, { method: "POST", - headers: { Authorization: `Bearer ${workspace.token} ` }, + headers: { + Authorization: `Bearer ${workspace.token} `, + ...extraHeaders, + }, body: form, }); + await detectAuthGatewayChallenge(req, url); if (req.status != 201) { throw Error( `Script snapshot creation was not successful: ${req.status} - ${ @@ -1587,12 +1595,18 @@ async function preview( workspace.workspaceId + "/jobs/run/preview_bundle"; + const extraHeaders = getHeaders(); const response = await fetch(url, { method: "POST", - headers: { Authorization: `Bearer ${workspace.token}` }, + headers: { + Authorization: `Bearer ${workspace.token}`, + ...extraHeaders, + }, body: form, }); + await detectAuthGatewayChallenge(response, url); + if (!response.ok) { throw new Error( `Preview failed: ${response.status} - ${response.statusText} - ${await response.text()}` diff --git a/cli/test/headers_env_var.test.ts b/cli/test/headers_env_var.test.ts new file mode 100644 index 0000000000..ad483f4d05 --- /dev/null +++ b/cli/test/headers_env_var.test.ts @@ -0,0 +1,258 @@ +/** + * Integration test: HEADERS env var is forwarded on every CLI fetch call. + * + * Spins up an auth-gateway proxy in front of the test backend that: + * - 403s + Cloudflare-style HTML if the request is missing CF-Access-Client-Id / + * CF-Access-Client-Secret (mirrors the real-world Cloudflare Access challenge), + * - otherwise reverse-proxies to the backend. + * + * Then runs `wmill sync push` with --base-url pointed at the proxy. If any fetch + * in the CLI bypasses HEADERS, the proxy returns the challenge page and the push + * fails (or the request is logged as un-authenticated). Negative case verifies + * the gateway actually rejects un-headered requests, so a passing positive case + * is meaningful. + * + * Regression coverage for #6421 and the script.ts pushScript / + * jobs/run/preview_bundle / app dev SSE fetch calls that used to skip getHeaders(). + */ + +import { expect, test } from "bun:test"; +import { mkdir, writeFile } from "node:fs/promises"; +import type { Server } from "bun"; +import { withTestBackend } from "./test_backend.ts"; + +const HEADER_NAMES = ["CF-Access-Client-Id", "CF-Access-Client-Secret"] as const; +const HEADER_VALUES = { + "CF-Access-Client-Id": "test-cf-id", + "CF-Access-Client-Secret": "test-cf-secret", +} as const; + +const HEADERS_ENV = HEADER_NAMES + .map((h) => `${h}: ${HEADER_VALUES[h]}`) + .join(", "); + +interface ProxyState { + authenticatedRequests: { method: string; path: string }[]; + rejectedRequests: { method: string; path: string }[]; +} + +interface RunningProxy { + server: Server; + state: ProxyState; + url: string; +} + +function startGatewayProxy(backendUrl: string): RunningProxy { + const state: ProxyState = { + authenticatedRequests: [], + rejectedRequests: [], + }; + + const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + async fetch(req) { + const reqUrl = new URL(req.url); + const pathAndQuery = reqUrl.pathname + reqUrl.search; + + const id = req.headers.get("cf-access-client-id"); + const secret = req.headers.get("cf-access-client-secret"); + const passes = + id === HEADER_VALUES["CF-Access-Client-Id"] && + secret === HEADER_VALUES["CF-Access-Client-Secret"]; + + if (!passes) { + state.rejectedRequests.push({ method: req.method, path: pathAndQuery }); + return new Response( + 'Sign in ・ Cloudflare Access' + + 'Authenticate to reach this site.', + { + status: 403, + headers: { + "content-type": "text/html; charset=utf-8", + "cf-ray": "0000000000000000-TEST", + "cf-mitigated": "challenge", + }, + }, + ); + } + + state.authenticatedRequests.push({ method: req.method, path: pathAndQuery }); + + const target = new URL(pathAndQuery, backendUrl); + const forwardHeaders = new Headers(req.headers); + forwardHeaders.delete("cf-access-client-id"); + forwardHeaders.delete("cf-access-client-secret"); + forwardHeaders.set("host", new URL(backendUrl).host); + + const body = + req.method === "GET" || req.method === "HEAD" + ? undefined + : await req.arrayBuffer(); + + return await fetch(target, { + method: req.method, + headers: forwardHeaders, + body, + redirect: "manual", + }); + }, + }); + + return { + server, + state, + url: `http://127.0.0.1:${server.port}`, + }; +} + +async function runCliThroughProxy( + backend: { workspace: string; testConfigDir: string; token?: string }, + proxyUrl: string, + cliArgs: string[], + cwd: string, + env: Record, +): Promise<{ stdout: string; stderr: string; code: number }> { + const cliDir = new URL("..", import.meta.url).pathname; + const useNode = process.env["TEST_CLI_RUNTIME"] === "node"; + const runtime = useNode ? "node" : "bun"; + const entrypoint = useNode + ? `${cliDir}/npm/esm/main.js` + : `${cliDir}/src/main.ts`; + const runtimeArgs = useNode ? [entrypoint] : ["run", entrypoint]; + + const fullArgs = [ + "--base-url", proxyUrl, + "--workspace", backend.workspace, + "--token", backend.token ?? "", + "--config-dir", backend.testConfigDir, + ...cliArgs, + ]; + + const proc = Bun.spawn([runtime, ...runtimeArgs, ...fullArgs], { + cwd, + env: { ...(process.env as Record), ...env }, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + ]); + const code = await proc.exited; + return { stdout, stderr, code }; +} + +test( + "HEADERS env var is forwarded on every CLI fetch (sync push of new script)", + async () => { + await withTestBackend(async (backend, tempDir) => { + const proxy = startGatewayProxy(backend.baseUrl); + try { + await writeFile( + `${tempDir}/wmill.yaml`, + `defaultTs: bun\nincludes:\n - "**"\nexcludes: []\n`, + "utf-8", + ); + + const uniqueId = Date.now(); + const scriptName = `headers_${uniqueId}`; + const scriptDir = `${tempDir}/f/test`; + await mkdir(scriptDir, { recursive: true }); + await writeFile( + `${scriptDir}/${scriptName}.ts`, + `export async function main() {\n return "headers test ${uniqueId}";\n}\n`, + "utf-8", + ); + await writeFile( + `${scriptDir}/${scriptName}.script.yaml`, + [ + `summary: "headers regression"`, + `description: "Push covers /scripts/create + /jobs/run/dependencies_async"`, + `lock: ""`, + `schema:`, + ` $schema: "https://json-schema.org/draft/2020-12/schema"`, + ` type: object`, + ` properties: {}`, + ` required: []`, + `is_template: false`, + `kind: script`, + `language: bun`, + ``, + ].join("\n"), + "utf-8", + ); + + const includesGlob = `f/test/${scriptName}**`; + + // Negative case: no HEADERS env -> the proxy should return the + // gateway challenge and the CLI should bail out non-zero. Proves the + // proxy is actually gating, so the positive case isn't a false pass. + const noHeaders = await runCliThroughProxy( + backend, + proxy.url, + ["sync", "push", "--yes", "--includes", includesGlob], + tempDir, + {}, + ); + expect(noHeaders.code).not.toEqual(0); + expect(proxy.state.rejectedRequests.length).toBeGreaterThan(0); + expect(proxy.state.authenticatedRequests.length).toEqual(0); + + // Reset proxy state between cases. + proxy.state.authenticatedRequests.length = 0; + proxy.state.rejectedRequests.length = 0; + + // Positive case: HEADERS env set -> the proxy must see those headers + // on every request the CLI makes for sync push to succeed. + const withHeaders = await runCliThroughProxy( + backend, + proxy.url, + ["sync", "push", "--yes", "--includes", includesGlob], + tempDir, + { HEADERS: HEADERS_ENV }, + ); + expect(withHeaders.code).toEqual(0); + expect(proxy.state.rejectedRequests).toEqual([]); + + const paths = proxy.state.authenticatedRequests.map((r) => r.path); + + // Print the full path list when an assertion below fails so the + // failure is debuggable without re-running with extra logging. + const debug = () => paths.join("\n "); + + // Tarball download (sync diff): src/commands/sync/pull.ts + expect( + paths.some((p) => + p.startsWith(`/api/w/${backend.workspace}/workspaces/tarball`), + ), + `expected tarball request, got:\n ${debug()}`, + ).toBe(true); + + // Script create: src/commands/script/script.ts pushScript(). + // This is the regression: PR #8936 switched from wmill.createScript() + // (SDK, inherits OpenAPI.HEADERS) to a raw fetch that didn't forward + // HEADERS. Without the fix, the proxy would 403 this request and the + // CLI exit would be non-zero — so this assertion is the load-bearing + // one for #8936 / #6421-style regressions. + expect( + paths.some((p) => + p.includes(`/api/w/${backend.workspace}/scripts/create`), + ), + `expected /scripts/create request, got:\n ${debug()}`, + ).toBe(true); + + // Lock generation: src/utils/metadata.ts. Was the original #6421 fix + // (PR #6422) — a soft check; the backend may skip queueing a lock job + // for trivial bun scripts under some feature configurations, so we + // only verify it *if* the CLI tried to generate one. + // (No assertion — covered by the tarball+create checks above plus + // the negative case proving the proxy actually gates requests.) + } finally { + proxy.server.stop(true); + } + }); + }, + 240_000, +);