mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-21 16:02:28 +00:00
fix(cli): forward HEADERS env var on every backend fetch call (#9075)
Several `fetch()` callers in the CLI bypassed `OpenAPI.HEADERS` and skipped the `HEADERS` env var, causing requests to fail behind auth gateways like Cloudflare Access (same shape as #6421): - `pushScript()` `/scripts/create` and `/scripts/create_snapshot` — regressed in #8936 when the call switched from `wmill.createScript()` (SDK) to a raw `fetch` for the `skip_if_noop` query param. - Script preview `/jobs/run/preview_bundle`. - App dev `/jobs_u/getupdate_sse` SSE stream. - `wmill docs` `/api/inkeep`. All four now spread `getHeaders()` and call `detectAuthGatewayChallenge()` so a Cloudflare/SSO challenge surfaces a clear error instead of an opaque JSON parse failure. Adds `test/headers_env_var.test.ts`: spins up an auth-gateway proxy that 403s requests missing `CF-Access-Client-Id` / `CF-Access-Client-Secret` and otherwise reverse-proxies to the test backend, then runs `wmill sync push` of a fresh script through the proxy. Negative case (no `HEADERS` env) verifies the proxy actually gates; positive case asserts every request including `/scripts/create` reaches the backend with the headers attached. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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}`,
|
||||
|
||||
@@ -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."
|
||||
|
||||
@@ -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()}`
|
||||
|
||||
@@ -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(
|
||||
'<!DOCTYPE html><title>Sign in ・ Cloudflare Access</title>' +
|
||||
'<body>Authenticate to reach this site.</body>',
|
||||
{
|
||||
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<string, string>,
|
||||
): 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<string, string>), ...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,
|
||||
);
|
||||
Reference in New Issue
Block a user