mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-03 08:02:19 +00:00
fix: let instances withhold signing secrets from the settings API (#11483)
* fix: never return the instance signing secrets from the settings API Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix: tell wmill instance get-config users that jwt_secret is not exported Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * chore: point ee-repo-ref at the oidc signing key fix Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix: block rsa_keys from agent workers and keep get-config stdout pure yaml Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix: keep server secrets in exports by default behind EXPORT_SERVER_SECRETS Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * chore: update ee-repo-ref to 43b2ce8866a393b2666b17647ddb1466afc70bb1 This commit updates the EE repository reference after PR #842 was merged in windmill-ee-private. Previous ee-repo-ref: 3a0d0c3f45eeb9f5fe8d50ce3798239aa9101a22 New ee-repo-ref: 43b2ce8866a393b2666b17647ddb1466afc70bb1 Automated by sync-ee-ref workflow. * fix: withhold server secrets on any non-true EXPORT_SERVER_SECRETS value Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
windmill-internal-app[bot]
parent
98274a7336
commit
99e96d3f78
@@ -1 +1 @@
|
||||
71b8c1042fd8188c2d2882476f12b671cb5ba421
|
||||
43b2ce8866a393b2666b17647ddb1466afc70bb1
|
||||
|
||||
@@ -200,3 +200,81 @@ async fn test_alert_job_queue_waiting_in_global_settings(db: Pool<Postgres>) ->
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn read_secret_surfaces(
|
||||
base: &str,
|
||||
names: &[&str],
|
||||
) -> anyhow::Result<Vec<(String, u16, String)>> {
|
||||
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<Postgres>) -> 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(())
|
||||
}
|
||||
|
||||
@@ -1235,7 +1235,7 @@ async fn get_instance_config(
|
||||
authed: ApiAuthed,
|
||||
) -> JsonResult<InstanceConfig> {
|
||||
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<Response> {
|
||||
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))
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<sqlx::Postgres>) -> anyhow::Result<Self> {
|
||||
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<sqlx::Postgres>) -> anyhow::Result<Self> {
|
||||
Self::read_from_db(db, server_secrets_exported()).await
|
||||
}
|
||||
|
||||
async fn read_from_db(
|
||||
db: &sqlx::Pool<sqlx::Postgres>,
|
||||
include_server_secrets: bool,
|
||||
) -> anyhow::Result<Self> {
|
||||
// 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<String, serde_json::Value> = 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))?;
|
||||
|
||||
@@ -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<string, unknown>);
|
||||
if (opts.outputFile) {
|
||||
|
||||
Reference in New Issue
Block a user