diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 0d0bb78414..a030c50540 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -71b8c1042fd8188c2d2882476f12b671cb5ba421 +43b2ce8866a393b2666b17647ddb1466afc70bb1 diff --git a/backend/windmill-api-integration-tests/tests/settings.rs b/backend/windmill-api-integration-tests/tests/settings.rs index 348d945a86..7fd60ed8e1 100644 --- a/backend/windmill-api-integration-tests/tests/settings.rs +++ b/backend/windmill-api-integration-tests/tests/settings.rs @@ -200,3 +200,81 @@ async fn test_alert_job_queue_waiting_in_global_settings(db: Pool) -> Ok(()) } + +async fn read_secret_surfaces( + base: &str, + names: &[&str], +) -> anyhow::Result> { + let mut paths = vec![ + "instance_config".to_string(), + "instance_config/yaml".to_string(), + ]; + #[cfg(feature = "enterprise")] + paths.push("list_global".to_string()); + paths.extend(names.iter().map(|n| format!("global/{n}"))); + let mut out = vec![]; + for path in paths { + let resp = authed(client().get(format!("{base}/{path}"))) + .send() + .await?; + let status = resp.status().as_u16(); + out.push((path, status, resp.text().await?)); + } + Ok(out) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_server_secrets_follow_export_flag(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let secrets = [ + ("jwt_secret", json!("planted-jwt-secret")), + ("rsa_keys", json!({"private_key": "planted-rsa-key"})), + ( + "custom_instance_replication_pwd", + json!("planted-replication-pwd"), + ), + ( + "external_instance_pg_state", + json!({"admin_pwd": "planted-pg-state"}), + ), + ]; + for (name, value) in &secrets { + sqlx::query( + "INSERT INTO global_settings (name, value) VALUES ($1, $2) + ON CONFLICT (name) DO UPDATE SET value = EXCLUDED.value", + ) + .bind(name) + .bind(value) + .execute(&db) + .await?; + } + let names: Vec<&str> = secrets.iter().map(|(n, _)| *n).collect(); + let server = ApiServer::start(db.clone()).await?; + let base = format!("http://localhost:{}/api/settings", server.addr.port()); + + // Default: a full export, so `get-config` and `pull` can migrate an instance. + std::env::remove_var("EXPORT_SERVER_SECRETS"); + for (path, status, body) in read_secret_surfaces(&base, &names).await? { + assert_2xx(status, &body, &path); + assert!( + body.contains("planted-"), + "{path} dropped a server secret: {body}" + ); + } + + std::env::set_var("EXPORT_SERVER_SECRETS", "0"); + let withheld = read_secret_surfaces(&base, &names).await; + std::env::remove_var("EXPORT_SERVER_SECRETS"); + for (path, status, body) in withheld? { + if path.starts_with("global/") { + assert_eq!(status, 400, "GET {path} returned {status}"); + } else { + assert_2xx(status, &body, &path); + } + assert!( + !body.contains("planted-"), + "{path} returned a server secret: {body}" + ); + } + Ok(()) +} diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index 71cbcc8460..7e2ec536c0 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -1235,7 +1235,7 @@ async fn get_instance_config( authed: ApiAuthed, ) -> JsonResult { require_super_admin(&db, &authed).await?; - let config = InstanceConfig::from_db(&db) + let config = InstanceConfig::from_db_for_api(&db) .await .map_err(|e| error::Error::internal_err(e.to_string()))?; Ok(Json(config)) @@ -1246,7 +1246,7 @@ async fn get_instance_config_yaml( authed: ApiAuthed, ) -> error::Result { require_super_admin(&db, &authed).await?; - let config = InstanceConfig::from_db(&db) + let config = InstanceConfig::from_db_for_api(&db) .await .map_err(|e| error::Error::internal_err(e.to_string()))?; let yaml = config @@ -1386,6 +1386,11 @@ pub async fn get_global_setting( { require_super_admin(&db, &authed).await?; } + if instance_config::is_withheld_server_secret(&key) { + return Err(error::Error::BadRequest(format!( + "{key} is a server secret and this server does not export it (EXPORT_SERVER_SECRETS=false)" + ))); + } let value = sqlx::query!("SELECT value FROM global_settings WHERE name = $1", key) .fetch_optional(&db) .await? @@ -1428,7 +1433,10 @@ async fn list_global_settings( require_super_admin(&db, &authed).await?; let settings = sqlx::query_as!(GlobalSetting, "SELECT name, value FROM global_settings") .fetch_all(&db) - .await?; + .await? + .into_iter() + .filter(|s| !instance_config::is_withheld_server_secret(&s.name)) + .collect(); Ok(Json(settings)) } diff --git a/backend/windmill-common/src/global_settings.rs b/backend/windmill-common/src/global_settings.rs index 61c83e311f..e1a731f3e6 100644 --- a/backend/windmill-common/src/global_settings.rs +++ b/backend/windmill-common/src/global_settings.rs @@ -431,8 +431,9 @@ pub const CANCEL_STRANDED_JOBS_AFTER_DAYS_SETTING: &str = "cancel_stranded_jobs_ /// server keeps to itself, add it here. pub const AGENT_WORKER_BLOCKED_SETTINGS: &[&str] = &[ // Instance identity / auth secrets — disclosure enables privilege escalation - // or impersonation. + // or impersonation. `rsa_keys` signs job OIDC tokens; only the server mints them. JWT_SECRET_SETTING, + "rsa_keys", OAUTH_SETTING, SMTP_SETTING, SCIM_TOKEN_SETTING, @@ -1144,6 +1145,7 @@ mod tests { // secrets. They must never be served by the agent-worker endpoint. for key in [ JWT_SECRET_SETTING, + "rsa_keys", OAUTH_SETTING, SMTP_SETTING, SCIM_TOKEN_SETTING, diff --git a/backend/windmill-common/src/instance_config.rs b/backend/windmill-common/src/instance_config.rs index 1bb323ed3d..a4164cd2c5 100644 --- a/backend/windmill-common/src/instance_config.rs +++ b/backend/windmill-common/src/instance_config.rs @@ -1021,10 +1021,40 @@ pub const PROTECTED_SETTINGS: &[&str] = &[ "min_keep_alive_version", ]; +/// Secrets the server signs with or generated for itself. `jwt_secret` signs API and job +/// tokens and `rsa_keys` signs job OIDC tokens, so reading either is enough to mint tokens +/// for any user. Superadmin reads return them only while `server_secrets_exported()`. +/// Withholding them from reads does not make them unwritable: the two signing keys are not +/// in `HIDDEN_SETTINGS`, so config can still set them (an operator ConfigMap may set +/// `jwt_secret`). +pub const SERVER_SECRET_SETTINGS: &[&str] = &[ + "jwt_secret", + "rsa_keys", + "custom_instance_replication_pwd", + "external_instance_pg_state", +]; + +/// Whether superadmin API reads (settings list, single-setting read, config export) return +/// `SERVER_SECRET_SETTINGS`. On by default so `wmill instance get-config` and `pull` carry a +/// full migration; `EXPORT_SERVER_SECRETS=false` withholds them from every API response, so a +/// leaked superadmin token cannot take the signing keys. Fails closed: once set to a +/// non-empty value, only `true`/`1`/`yes`/`on` keep exporting, so `0` or a typo withholds. +/// Read per call: the flag is cheap and tests flip it in-process. +pub fn server_secrets_exported() -> bool { + match std::env::var("EXPORT_SERVER_SECRETS") { + Ok(v) if !v.trim().is_empty() => matches!( + v.trim().to_ascii_lowercase().as_str(), + "true" | "1" | "yes" | "on" + ), + _ => true, + } +} + +pub fn is_withheld_server_secret(name: &str) -> bool { + SERVER_SECRET_SETTINGS.contains(&name) && !server_secrets_exported() +} + /// Internal settings that are never exposed via the API or included in config exports. -/// Note: jwt_secret is intentionally NOT hidden — it is included in YAML exports so that -/// operators can set it via ConfigMap. It is protected from deletion (PROTECTED_SETTINGS) -/// and from being set to empty/null, and its value is partially redacted in log output. pub const HIDDEN_SETTINGS: &[&str] = &[ "uid", "min_keep_alive_version", @@ -1569,6 +1599,21 @@ pub fn resolve_env_refs(settings: &mut GlobalSettings) -> Result<(), String> { impl InstanceConfig { /// Read the full instance configuration from the database. pub async fn from_db(db: &sqlx::Pool) -> anyhow::Result { + Self::read_from_db(db, true).await + } + + /// The instance configuration as an API response may carry it: `from_db` without + /// `SERVER_SECRET_SETTINGS` unless `server_secrets_exported()`. It holds admin-entered + /// credentials (`license_key`, `scim_token`, SMTP and OAuth secrets), so callers must + /// have checked superadmin. + pub async fn from_db_for_api(db: &sqlx::Pool) -> anyhow::Result { + Self::read_from_db(db, server_secrets_exported()).await + } + + async fn read_from_db( + db: &sqlx::Pool, + include_server_secrets: bool, + ) -> anyhow::Result { // Read global_settings table → flat map → deserialize into GlobalSettings let rows: Vec<(String, serde_json::Value)> = sqlx::query_as("SELECT name, value FROM global_settings") @@ -1577,6 +1622,9 @@ impl InstanceConfig { let map: serde_json::Map = rows .into_iter() .filter(|(name, _)| !HIDDEN_SETTINGS.contains(&name.as_str())) + .filter(|(name, _)| { + include_server_secrets || !SERVER_SECRET_SETTINGS.contains(&name.as_str()) + }) .collect(); let global_settings: GlobalSettings = serde_json::from_value(serde_json::Value::Object(map))?; diff --git a/cli/src/commands/instance/instance.ts b/cli/src/commands/instance/instance.ts index 35530ddfd3..f2285a5316 100644 --- a/cli/src/commands/instance/instance.ts +++ b/cli/src/commands/instance/instance.ts @@ -685,6 +685,11 @@ async function getConfig(opts: InstanceSyncOptions & { outputFile?: string; show if (config.global_settings.license_key) config.global_settings.license_key = "***"; if (config.global_settings.jwt_secret) config.global_settings.jwt_secret = "***"; } + if (config?.global_settings && !config.global_settings.jwt_secret) { + log.infoStderr( + "jwt_secret, rsa_keys and the instance database secrets are not part of the export: the server withholds them (EXPORT_SERVER_SECRETS=false). This export cannot fully restore or migrate the instance." + ); + } const yaml = yamlStringify(config as Record); if (opts.outputFile) {