diff --git a/backend/windmill-worker/src/prepare_deps.rs b/backend/windmill-worker/src/prepare_deps.rs index 2e16e565c2..161744af40 100644 --- a/backend/windmill-worker/src/prepare_deps.rs +++ b/backend/windmill-worker/src/prepare_deps.rs @@ -116,6 +116,11 @@ const p = { pub struct PrepareRequest { pub code: String, pub language: String, + /// Interpreter the caller will run the script with. The venv must be built against it: + /// site-packages is put on that interpreter's sys.path, and a wheel with a compiled + /// extension built for another version is simply invisible there. + #[serde(default)] + pub python_path: Option, } #[derive(Serialize)] @@ -178,6 +183,17 @@ fn get_proc_envs(cache_env: Option<(&str, &str)>) -> HashMap { envs } +/// CA bundle for the package index, most specific spelling first. A host behind a TLS-intercepting +/// proxy configures it under whichever name its other tooling uses, and the environment is cleared +/// below, so falling back past `PY_INDEX_CERT` is what makes those hosts work at all. +fn index_ca_bundle() -> Option { + INDEX_CERT + .clone() + .or_else(|| non_empty_env("SSL_CERT_FILE")) + .or_else(|| non_empty_env("REQUESTS_CA_BUNDLE")) + .or_else(|| non_empty_env("CURL_CA_BUNDLE")) +} + /// uv registry arguments, mirroring what the job path passes in `python_executor`. fn uv_registry_args() -> Vec { let mut args: Vec = vec![]; @@ -201,7 +217,7 @@ fn uv_registry_args() -> Vec { } /// Prepare Python dependencies using uv -async fn prepare_python_deps_standalone(code: &str) -> PrepareResponse { +async fn prepare_python_deps_standalone(code: &str, python_path: Option<&str>) -> PrepareResponse { // Parse imports from the code let packages = parse_python_imports(code); @@ -242,10 +258,16 @@ async fn prepare_python_deps_standalone(code: &str) -> PrepareResponse { if let Some(timeout) = UV_HTTP_TIMEOUT.as_ref() { common_uv_envs.insert("UV_HTTP_TIMEOUT".to_string(), timeout.to_string()); } - if let Some(cert_path) = INDEX_CERT.as_ref() { + if let Some(cert_path) = index_ca_bundle() { // uv has no `--cert` on `venv`/`pip install` (astral-sh/uv#6715), so a custom CA bundle - // reaches it through SSL_CERT_FILE, as in the job path. - common_uv_envs.insert("SSL_CERT_FILE".to_string(), cert_path.to_string()); + // reaches it through SSL_CERT_FILE, as in the job path. It replaces uv's own roots rather + // than adding to them, so the file has to be a complete bundle. + common_uv_envs.insert("SSL_CERT_FILE".to_string(), cert_path); + } + // The other spelling uv accepts. Like the bundle above it replaces uv's own roots rather than + // adding to them, so a directory holding only a private CA leaves public indexes untrusted. + if let Some(cert_dir) = non_empty_env("SSL_CERT_DIR") { + common_uv_envs.insert("SSL_CERT_DIR".to_string(), cert_dir); } let registry_args = uv_registry_args(); @@ -254,6 +276,13 @@ async fn prepare_python_deps_standalone(code: &str) -> PrepareResponse { // `--seed` resolves pip/setuptools from the index, so the venv also needs the registry // arguments: on a network that only reaches a private mirror it fails without them. let mut venv_args = vec!["venv".to_string(), venv_dir.clone(), "--seed".to_string()]; + // Without this uv picks its own interpreter, and the caller then puts a site-packages built + // for that version on a different interpreter's sys.path: pure-Python packages still import, + // anything with a compiled extension does not, and the error names the missing extension + // rather than the mismatch. + if let Some(python_path) = python_path { + venv_args.extend(["-p".to_string(), python_path.to_string()]); + } venv_args.extend(registry_args.iter().cloned()); let output = Command::new(UV_PATH.as_str()) @@ -375,16 +404,25 @@ async fn prepare_python_deps_standalone(code: &str) -> PrepareResponse { /// Get common environment variables for Bun processes pub fn get_simple_bun_proc_envs() -> HashMap { - get_proc_envs(Some(("BUN_INSTALL_CACHE_DIR", &BUN_CACHE_DIR))) + let mut envs = get_proc_envs(Some(("BUN_INSTALL_CACHE_DIR", &BUN_CACHE_DIR))); + // Bun reads none of the spellings uv does, so a custom CA reaches `bun install` only here. + if let Some(cert_path) = non_empty_env("NODE_EXTRA_CA_CERTS").or_else(index_ca_bundle) { + envs.insert("NODE_EXTRA_CA_CERTS".to_string(), cert_path); + } + envs } /// Prepare dependencies for a script without requiring database access. /// This is meant to be called from the CLI. -pub async fn prepare_deps_standalone(code: &str, language: &str) -> PrepareResponse { +pub async fn prepare_deps_standalone( + code: &str, + language: &str, + python_path: Option<&str>, +) -> PrepareResponse { // Route to the appropriate handler based on language match language { "python3" | "python" => { - return prepare_python_deps_standalone(code).await; + return prepare_python_deps_standalone(code, python_path).await; } "bun" | "typescript" | "deno" => { // Continue with JS/TS handling below @@ -649,7 +687,12 @@ pub async fn run_prepare_deps_cli() -> anyhow::Result<()> { } }; - let response = prepare_deps_standalone(&request.code, &request.language).await; + let response = prepare_deps_standalone( + &request.code, + &request.language, + request.python_path.as_deref(), + ) + .await; println!("{}", serde_json::to_string(&response)?); Ok(()) diff --git a/debugger/Dockerfile b/debugger/Dockerfile index 51e993746a..4c1eb85c98 100644 --- a/debugger/Dockerfile +++ b/debugger/Dockerfile @@ -46,6 +46,7 @@ WORKDIR /app # Copy the debug service files COPY dap_debug_service.ts . COPY dap_websocket_server_bun.ts . +COPY env_passthrough.ts . COPY dap_websocket_server.py . # Expose the default port diff --git a/debugger/README.md b/debugger/README.md index 51c2838f92..614d2bf65e 100644 --- a/debugger/README.md +++ b/debugger/README.md @@ -94,7 +94,8 @@ with the other settings): | `PY_INDEX_URL` / `PIP_INDEX_URL` | Package index (`--index-url`) | PyPI | | `PY_EXTRA_INDEX_URL` / `PIP_EXTRA_INDEX_URL` | Extra indexes, comma-separated (`--extra-index-url`) | - | | `PY_TRUSTED_HOST` / `PIP_TRUSTED_HOST` | Hosts to trust, whitespace-separated (`--trusted-host`) | - | -| `PY_INDEX_CERT` / `PIP_INDEX_CERT` | CA bundle for the index, passed to uv as `SSL_CERT_FILE` | - | +| `PY_INDEX_CERT` / `PIP_INDEX_CERT` | CA bundle for the index, passed to uv as `SSL_CERT_FILE`. Falls back to `SSL_CERT_FILE`, then `REQUESTS_CA_BUNDLE`, then `CURL_CA_BUNDLE`, so a host that configures its CA under any of those names is picked up. Whichever is used **replaces** uv's own roots rather than adding to them, so it has to be a complete bundle: one holding only a private CA leaves every public index untrusted. `bun install` gets the same bundle as `NODE_EXTRA_CA_CERTS`, the only spelling Bun reads | - | +| `SSL_CERT_DIR` | Directory of certificates, forwarded to uv as-is. Replaces uv's roots the same way the bundle does, so a directory holding only a private CA leaves public indexes untrusted | - | | `PY_NATIVE_CERT` / `UV_NATIVE_TLS` | `true` to also trust the platform certificate store (`--native-tls`) | false | | `UV_INDEX_STRATEGY` | uv index strategy | unsafe-best-match | | `UV_HTTP_TIMEOUT` | uv HTTP request timeout, in seconds | uv's own default | @@ -110,6 +111,17 @@ service into each session, since the debugged script needs them for its own outb as a job's script does on a worker. When a proxy is set without a bypass list, `NO_PROXY` defaults to `localhost,127.0.0.1` so calls to `BASE_INTERNAL_URL` are not proxied. +Trust roots are forwarded alongside them: `SSL_CERT_FILE`, `SSL_CERT_DIR`, `REQUESTS_CA_BUNDLE`, +`CURL_CA_BUNDLE` and `NODE_EXTRA_CA_CERTS`. Behind a TLS-intercepting proxy these are what let the +debugged script's own HTTPS calls verify, and installing the CA in the container's system store is +not enough on its own, since `requests` carries its own bundle and Node reads only +`NODE_EXTRA_CA_CERTS`. Registry settings are deliberately not forwarded: they carry credentials and +only the service needs them. + +To install that CA into the container's system store in the first place, set `INIT_SCRIPT` on the +`windmill_extra` container (e.g. `INIT_SCRIPT=update-ca-certificates`). It runs before any service +starts and aborts startup if it fails, the same hook a worker offers. + Keeping the settings out of the session's environment only bounds what the debugged script can read from itself. An unsandboxed session runs under the same user as the service and can still read the service's environment through `/proc`, the same way a job can read a worker's when the worker runs diff --git a/debugger/dap_debug_service.ts b/debugger/dap_debug_service.ts index 405cf15fde..6da3f0b1b0 100644 --- a/debugger/dap_debug_service.ts +++ b/debugger/dap_debug_service.ts @@ -41,6 +41,7 @@ import { join } from 'node:path' // Import the working Bun debug session from the standalone server import { DebugSession as BunDebugSessionWorking, type NsjailConfig } from './dap_websocket_server_bun' +import { sessionEnv } from './env_passthrough' // ============================================================================ // Configuration @@ -352,45 +353,12 @@ interface SpawnOptions { stderr?: 'pipe' | 'inherit' } -/** - * Proxy settings forwarded to a debug session, matching what a worker gives a job's script. - * spawnProcess intentionally does not inherit this process's environment, so an outbound proxy - * is unreachable from a session unless these are passed explicitly. Registry settings are - * deliberately absent: they carry credentials and are consumed by the service itself (see - * PythonDebugSession.prepareDependencies). - */ -const SESSION_PROXY_ENV_VARS = [ - 'HTTP_PROXY', - 'HTTPS_PROXY', - 'NO_PROXY', - // The lowercase spellings take precedence in the worker, so forward both. - 'http_proxy', - 'https_proxy', - 'no_proxy' -] - /** * How long `windmill prepare-deps` may take before the session gives up on it and starts without * the dependencies. Raise it for slow private mirrors, where a large install can outlast the default. */ const PREPARE_DEPS_TIMEOUT_MS = Number(process.env.DAP_PREPARE_DEPS_TIMEOUT_MS) || 120_000 -function sessionProxyEnv(): Record { - const env: Record = {} - for (const key of SESSION_PROXY_ENV_VARS) { - const value = process.env[key] - if (value) { - env[key] = value - } - } - // A proxy without a bypass list would send the script's calls to BASE_INTERNAL_URL through it; - // the worker defaults the same way (PROXY_ENVS in windmill-worker). - if (!env.NO_PROXY && !env.no_proxy && (env.HTTP_PROXY || env.http_proxy || env.HTTPS_PROXY || env.https_proxy)) { - env.NO_PROXY = 'localhost,127.0.0.1' - } - return env -} - /** * Spawn a process, optionally wrapped with nsjail. * This is the key function for sandboxed execution. @@ -551,6 +519,8 @@ class PythonDebugSession extends BaseDebugSession { private envVars: Record = {} private windmillPath?: string private venvPath?: string + private prepareDepsProcess: Subprocess | null = null + private disposed = false private debugMode: boolean constructor(ws: { send: (data: string) => void; close: () => void }, windmillPath?: string, debugMode = false) { @@ -708,10 +678,16 @@ class PythonDebugSession extends BaseDebugSession { try { const proc = spawn({ cmd: [this.windmillPath, 'prepare-deps'], - stdin: new Blob([JSON.stringify({ code, language: 'python3' }) + '\n']), + // The venv has to be built against the interpreter that will run the script: its + // site-packages goes on that interpreter's sys.path, and uv otherwise picks its + // own, which silently leaves compiled extensions unimportable. + stdin: new Blob([ + JSON.stringify({ code, language: 'python3', python_path: config.pythonPath }) + '\n' + ]), stdout: 'pipe', stderr: 'pipe' }) + this.prepareDepsProcess = proc // The launch response is already sent, so an install that never returns would leave the // client waiting on a session that never starts, with nothing on screen. The deadline @@ -729,9 +705,12 @@ class PythonDebugSession extends BaseDebugSession { }) ]) clearTimeout(timer) + this.prepareDepsProcess = null if (!result) { - proc.kill() + // SIGKILL, not the default SIGTERM: prepare-deps does not act on SIGTERM while uv + // is running, so a polite signal leaves it running after the session gave up. + proc.kill('SIGKILL') return warn( `dependency installation timed out after ${PREPARE_DEPS_TIMEOUT_MS / 1000}s` ) @@ -797,7 +776,7 @@ class PythonDebugSession extends BaseDebugSession { this.process = spawnProcess({ cmd, cwd, - env: { PYTHONUNBUFFERED: '1', ...sessionProxyEnv(), ...this.envVars } + env: { PYTHONUNBUFFERED: '1', ...sessionEnv(), ...this.envVars } }) // Read stderr to capture startup messages @@ -1133,6 +1112,16 @@ sys.stdout.flush() this.venvPath = (await this.prepareDependencies(code)) ?? undefined } + // Installing takes long enough for the client to give up meanwhile, and cleanup() has + // then already run: starting the debuggee now would leave a process nothing owns + // executing the script for a session that is gone. Clean up again on the way out, + // since a teardown that landed before the script was written left it behind. + if (this.disposed) { + logger.info('Session torn down during dependency preparation, not starting Python') + await this.cleanup() + return + } + await this.startPythonProcess(cwd) // Re-apply breakpoints to the Python server using the actual script path @@ -1188,6 +1177,14 @@ sys.stdout.flush() } async cleanup(): Promise { + this.disposed = true + + // A client that gives up mid-install must not leave the package manager running + if (this.prepareDepsProcess) { + this.prepareDepsProcess.kill('SIGKILL') + this.prepareDepsProcess = null + } + if (this.debugpyWs) { this.debugpyWs.close() this.debugpyWs = null diff --git a/debugger/dap_websocket_server_bun.ts b/debugger/dap_websocket_server_bun.ts index ff060f3da6..8336c2f812 100644 --- a/debugger/dap_websocket_server_bun.ts +++ b/debugger/dap_websocket_server_bun.ts @@ -25,6 +25,7 @@ import { spawn, type Subprocess } from 'bun' import { mkdtemp, writeFile, unlink, rmdir, symlink } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' +import { sessionEnv } from './env_passthrough' // Types for V8 Inspector Protocol interface V8Message { @@ -1599,7 +1600,10 @@ export class DebugSession { const input = JSON.stringify({ code, language }) + '\n' logger.info(`prepare-deps input length: ${input.length}`) - // Spawn the windmill binary with prepare-deps command + // Spawn the windmill binary with prepare-deps command. This runs in the service + // process, so inheriting its environment is what gives prepare-deps the container's + // index and certificate settings; the allowlist above is what keeps them from the + // debugged script. const proc = spawn({ cmd: [this.windmillPath, 'prepare-deps'], stdin: new Blob([input]), // Use Blob for complete stdin data @@ -1612,7 +1616,9 @@ export class DebugSession { killTimer = setTimeout(() => { timedOut = true logger.error(`prepare-deps timed out after ${PREPARE_DEPS_TIMEOUT_MS}ms`) - proc.kill() + // SIGKILL, not the default SIGTERM: prepare-deps does not act on SIGTERM while the + // package manager is running, so a polite signal leaves it going after the timeout. + proc.kill('SIGKILL') }, PREPARE_DEPS_TIMEOUT_MS) // Wait for completion @@ -1735,12 +1741,16 @@ export class DebugSession { }, 10000) }) - // Only include essential env vars + client-provided ones + // Only include essential env vars + the network-config allowlist + client-provided ones. // Don't inherit all of process.env to keep debugger environment clean const envVars: Record = { // Essential system vars PATH: process.env.PATH || '/usr/bin:/bin', HOME: process.env.HOME, + // Proxy / TLS settings inherited from the container, before the client's env so an + // explicit override still wins. Package-index settings are deliberately absent: this + // runs user-supplied code and index URLs carry registry credentials. + ...sessionEnv(), // Client-provided env vars (WM_WORKSPACE, WM_TOKEN, etc.) // Note: WM_BASE_URL is already overridden by BASE_INTERNAL_URL if set ...this.envVars diff --git a/debugger/env_passthrough.ts b/debugger/env_passthrough.ts new file mode 100644 index 0000000000..f713483d05 --- /dev/null +++ b/debugger/env_passthrough.ts @@ -0,0 +1,43 @@ +/** + * Container network configuration forwarded to a debug session, matching what a worker gives a + * job's script. The session environment is built from an allowlist rather than inherited, so an + * outbound proxy or a private CA is unreachable from a session unless these are passed + * explicitly. Registry settings are deliberately absent: they carry credentials and are consumed + * by the service itself (see PythonDebugSession.prepareDependencies). + * + * Lives in its own module because both session kinds build their own environment, and + * dap_debug_service.ts already imports from dap_websocket_server_bun.ts. + */ +export const SESSION_ENV_VARS = [ + 'HTTP_PROXY', + 'HTTPS_PROXY', + 'NO_PROXY', + // The lowercase spellings take precedence in the worker, so forward both. + 'http_proxy', + 'https_proxy', + 'no_proxy', + // Trust roots for a TLS-intercepting proxy. Installing the CA in the container's system + // store is not enough on its own: requests carries its own bundle and Node reads only + // NODE_EXTRA_CA_CERTS, so a debugged script's own HTTPS calls fail without these. + 'SSL_CERT_FILE', + 'SSL_CERT_DIR', + 'REQUESTS_CA_BUNDLE', + 'CURL_CA_BUNDLE', + 'NODE_EXTRA_CA_CERTS' +] + +export function sessionEnv(): Record { + const env: Record = {} + for (const key of SESSION_ENV_VARS) { + const value = process.env[key] + if (value) { + env[key] = value + } + } + // A proxy without a bypass list would send the script's calls to BASE_INTERNAL_URL through it; + // the worker defaults the same way (PROXY_ENVS in windmill-worker). + if (!env.NO_PROXY && !env.no_proxy && (env.HTTP_PROXY || env.http_proxy || env.HTTPS_PROXY || env.https_proxy)) { + env.NO_PROXY = 'localhost,127.0.0.1' + } + return env +} diff --git a/docker/DockerfileExtra b/docker/DockerfileExtra index 808bc6ed2a..a0b4986102 100644 --- a/docker/DockerfileExtra +++ b/docker/DockerfileExtra @@ -94,6 +94,7 @@ WORKDIR /debugger # Copy debugger files COPY debugger/dap_debug_service.ts . COPY debugger/dap_websocket_server_bun.ts . +COPY debugger/env_passthrough.ts . COPY debugger/dap_websocket_server.py . COPY debugger/nsjail.debug.config.proto . diff --git a/docker/entrypoint-extra.sh b/docker/entrypoint-extra.sh index a06901e25c..e5cc4e61fc 100644 --- a/docker/entrypoint-extra.sh +++ b/docker/entrypoint-extra.sh @@ -33,6 +33,20 @@ if [ ! -w "$HOME" ]; then fi export HOME +# INIT_SCRIPT is the documented hook for preparing the host before anything reaches the network +# (CA certificates, proxies, mounts), matching the worker's INIT_SCRIPT. It must therefore complete +# before any service starts, and a failure has to abort: services that come up with an unprepared +# trust store fail every TLS handshake instead, which is far harder to diagnose. +if [ -n "$INIT_SCRIPT" ]; then + echo "[entrypoint] Running INIT_SCRIPT..." + bash -c "$INIT_SCRIPT" || { + code=$? + echo "[entrypoint] ERROR: INIT_SCRIPT failed with exit code $code, aborting" >&2 + exit "$code" + } + echo "[entrypoint] INIT_SCRIPT completed" +fi + # Setup NETRC if provided (for LSP) if [ -n "$NETRC" ]; then echo "$NETRC" > "$HOME/.netrc"