diff --git a/backend/windmill-api-integration-tests/tests/workspace_dependencies.rs b/backend/windmill-api-integration-tests/tests/workspace_dependencies.rs new file mode 100644 index 0000000000..0132a98a61 --- /dev/null +++ b/backend/windmill-api-integration-tests/tests/workspace_dependencies.rs @@ -0,0 +1,57 @@ +use serde_json::json; +use sqlx::{Pool, Postgres}; + +#[allow(unused_imports)] +use windmill_test_utils::*; + +/// An admin of one workspace must not be able to write dependency files into another +/// by naming it in the request body. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_create_rejects_body_workspace_other_than_path( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + + // test-user-2 is a plain user of test-workspace and an admin of its own workspace. + sqlx::query("INSERT INTO workspace (id, name, owner) VALUES ('own-workspace', 'own-workspace', 'test-user-2')") + .execute(&db) + .await?; + sqlx::query("INSERT INTO usr (workspace_id, email, username, is_admin, role) VALUES ('own-workspace', 'test2@windmill.dev', 'test-user-2', true, 'Admin')") + .execute(&db) + .await?; + + let (_client, port, _server) = init_client(db.clone()).await; + + let resp = reqwest::Client::new() + .post(format!( + "http://localhost:{port}/api/w/own-workspace/workspace_dependencies/create" + )) + .header("Authorization", "Bearer SECRET_TOKEN_2") + .json(&json!({ + "workspace_id": "test-workspace", + "language": "python3", + "content": "requests==2.28.0" + })) + .send() + .await?; + let status = resp.status().as_u16(); + let body = resp.text().await?; + assert_eq!( + status, 400, + "expected the mismatched body to be rejected, got {body}" + ); + assert!( + body.contains("does not match"), + "expected the workspace mismatch rejection, got {body}" + ); + + let written: i64 = sqlx::query_scalar("SELECT count(*) FROM workspace_dependencies") + .fetch_one(&db) + .await?; + assert_eq!( + written, 0, + "no dependency file may be written in either workspace" + ); + + Ok(()) +} diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 5f2bd9e66c..c308559c0b 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -10872,6 +10872,8 @@ paths: text/plain: schema: type: string + "400": + description: the body's workspace_id does not match the workspace in the path /w/{workspace}/workspace_dependencies/archive/{language}: post: @@ -29504,6 +29506,7 @@ components: properties: workspace_id: type: string + description: must equal the workspace in the request path language: $ref: "#/components/schemas/ScriptLang" name: diff --git a/backend/windmill-api/src/workspace_dependencies.rs b/backend/windmill-api/src/workspace_dependencies.rs index 194cc1f0d5..170006f44a 100644 --- a/backend/windmill-api/src/workspace_dependencies.rs +++ b/backend/windmill-api/src/workspace_dependencies.rs @@ -34,13 +34,20 @@ async fn create( authed: ApiAuthed, // Extension(user_db): Extension, Extension(db): Extension, + Path(w_id): Path, Json(nwd): Json, ) -> error::Result<(StatusCode, String)> { - tracing::info!(workspace_id = %nwd.workspace_id, name = ?nwd.name, language = ?nwd.language, "create workspace dependencies"); + tracing::info!(workspace_id = %w_id, name = ?nwd.name, language = ?nwd.language, "create workspace dependencies"); require_admin(authed.is_admin, &authed.username)?; + // `require_admin` vouches for the path workspace only; the body must not target another. + if nwd.workspace_id != w_id { + return Err(error::Error::BadRequest(format!( + "workspace_id `{}` in the request body does not match the workspace `{}` in the path", + nwd.workspace_id, w_id + ))); + } let dep_path = WorkspaceDependencies::to_path(&nwd.name, nwd.language)?; - let w_id = nwd.workspace_id.clone(); let email = authed.email.clone(); let username = authed.username.clone(); diff --git a/backend/windmill-dep-map/src/workspace_dependencies.rs b/backend/windmill-dep-map/src/workspace_dependencies.rs index 03db1405e5..f4dd404133 100644 --- a/backend/windmill-dep-map/src/workspace_dependencies.rs +++ b/backend/windmill-dep-map/src/workspace_dependencies.rs @@ -11,6 +11,8 @@ use crate::{ #[derive(sqlx::FromRow, Clone, Serialize, Deserialize, Hash, Debug)] pub struct NewWorkspaceDependencies { + /// Trusted as-is by `create`: a caller deserializing this from a request must first check it + /// against the workspace the caller is authorized in. pub workspace_id: String, pub language: ScriptLang, pub name: Option,