diff --git a/backend/tests/workspace_export.rs b/backend/tests/workspace_export.rs index 8568b8ff24..5894ca8394 100644 --- a/backend/tests/workspace_export.rs +++ b/backend/tests/workspace_export.rs @@ -1,6 +1,6 @@ use sqlx::postgres::Postgres; use sqlx::Pool; -use windmill_test_utils::{initialize_tracing, ApiServer}; +use windmill_test_utils::{initialize_tracing, set_jwt_secret, ApiServer}; /// Integration test: exercises every explicit-column query in `tarball_workspace`. /// @@ -287,3 +287,77 @@ async fn test_tarball_export_gates_values_on_item_scopes(db: Pool) -> Ok(()) } + +/// `settings.json` carries the admin-managed integration config that `get_settings` +/// is admin-only for (the webhook URL, ai_config, git_sync, handler extra_args), so +/// `include_settings` takes the same admin check as `get_settings` rather than +/// riding on the route's `workspaces:read`. Git sync exports settings through the +/// same route, so the gate must still admit its system identity. +#[sqlx::test(fixtures("base"))] +async fn test_tarball_export_settings_are_admin_only(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().await; + let server = ApiServer::start(db.clone()).await?; + let base_url = format!("http://localhost:{}", server.addr.port()); + + sqlx::query( + r#"UPDATE workspace_settings + SET webhook = 'https://hook.example/?token=WEBHOOK_SECRET', + ai_config = '{"providers":{"openai":{"api_key":"AI_CONFIG_SECRET"}}}'::jsonb + WHERE workspace_id = 'test-workspace'"#, + ) + .execute(&db) + .await?; + + let export = async |token: &str| -> anyhow::Result<(u16, String)> { + let resp = reqwest::Client::new() + .get(format!( + "{base_url}/api/w/test-workspace/workspaces/tarball?include_settings=true&settings_version=v2" + )) + .bearer_auth(token) + .send() + .await?; + let status = resp.status().as_u16(); + // Lossy: a successful export is a tar, not UTF-8. Only the values matter here. + Ok(( + status, + String::from_utf8_lossy(&resp.bytes().await?).into_owned(), + )) + }; + + // SECRET_TOKEN_2 belongs to test-user-2, a non-admin member of test-workspace. + let (status, body) = export("SECRET_TOKEN_2").await?; + assert_eq!(status, 403, "non-admin exported settings: {body}"); + + let (status, body) = export("SECRET_TOKEN").await?; + assert_eq!(status, 200, "admin denied settings: {body}"); + assert!( + body.contains("WEBHOOK_SECRET") && body.contains("AI_CONFIG_SECRET"), + "admin got no settings" + ); + + // Git sync pushes the workspace to the repo by exporting it under + // `superadmin_sync@windmill.dev`, which belongs to no workspace: the job token + // it runs with is the export's only admin claim. + let sync_email = windmill_common::users::SUPERADMIN_SYNC_EMAIL; + let sync_token = windmill_common::auth::create_token_for_owner( + &db, + "test-workspace", + sync_email, + "git-sync", + 300, + sync_email, + &uuid::Uuid::new_v4(), + None, + None, + ) + .await?; + let (status, body) = export(&sync_token).await?; + assert_eq!(status, 200, "git-sync identity denied settings: {body}"); + assert!( + body.contains("WEBHOOK_SECRET"), + "git-sync identity got no settings" + ); + + Ok(()) +} diff --git a/backend/windmill-api/src/workspaces_export.rs b/backend/windmill-api/src/workspaces_export.rs index 9116021626..05cd354cd4 100644 --- a/backend/windmill-api/src/workspaces_export.rs +++ b/backend/windmill-api/src/workspaces_export.rs @@ -643,6 +643,16 @@ pub(crate) async fn tarball_workspace( windmill_api_auth::forbid_scoped_token_workspace_key(&authed)?; } + // settings.json carries the admin-managed integration config that `get_settings` + // is admin-only for (ai_config, the webhook URL, git_sync, handler extra_args), + // so it takes the same check. Not a per-field redaction: fields silently dropped + // from settings.json come back as null on the next `wmill sync push`. + if include_settings.unwrap_or(false) && !authed.is_admin { + return Err(Error::PermissionDenied( + "include_settings requires workspace admin".to_string(), + )); + } + // The route is gated by workspaces:read, but the tarball also carries the item // values that the per-item routes gate on their own domain (get_resource_value, // get_variable). A whole-workspace export cannot be confined to a path, so it @@ -1626,13 +1636,7 @@ pub(crate) async fn tarball_workspace( slack_name: row.slack_name.clone(), slack_command_script: row.slack_command_script.clone(), slack_oauth_client_id: row.slack_oauth_client_id.clone(), - // Mirror the non-admin redaction in `get_settings`: the OAuth - // client secret is admin-only and must not leak via tarball. - slack_oauth_client_secret: if authed.is_admin { - row.slack_oauth_client_secret.clone() - } else { - None - }, + slack_oauth_client_secret: row.slack_oauth_client_secret.clone(), }; serde_json::to_value(settings) .map(|v| serde_json::to_string_pretty(&v).ok())