fix: scope app deployment history to the workspace and its app

backend/.sqlx/query-a462d8661fee58129ec52952ee08a33fcc2e22b47d352030d55e1375c885cd36.json

@@ -1,6 +1,6 @@
 {
   "db_name": "PostgreSQL",
-  "query": "SELECT path FROM app WHERE id = $1",
+  "query": "SELECT a.path FROM app a JOIN app_version av ON av.app_id = a.id\n        WHERE a.id = $1 AND a.workspace_id = $2 AND av.id = $3",
   "describe": {
     "columns": [
       {
@@ -11,6 +11,8 @@
     ],
     "parameters": {
       "Left": [
+        "Int8",
+        "Text",
         "Int8"
       ]
     },
@@ -18,5 +20,5 @@
       false
     ]
   },
-  "hash": "abb56dec024600d0d7a63212f63c3296196898003707f2cb3a890690173a51a5"
+  "hash": "a462d8661fee58129ec52952ee08a33fcc2e22b47d352030d55e1375c885cd36"
 }

backend/.sqlx/query-b29cf43a194f4a61a67ce95a5485dc287df1b1d79bf5040216ee48583405cd8b.json

@@ -1,6 +1,6 @@
 {
   "db_name": "PostgreSQL",
-  "query": "SELECT a.id as app_id, av.id as version_id, dm.deployment_msg as deployment_msg\n        FROM app a LEFT JOIN app_version av ON a.id = av.app_id LEFT JOIN deployment_metadata dm ON av.id = dm.app_version\n        WHERE a.workspace_id = $1 AND a.path = $2\n        ORDER BY created_at DESC",
+  "query": "SELECT a.id as app_id, av.id as version_id, dm.deployment_msg as deployment_msg\n        FROM app a LEFT JOIN app_version av ON a.id = av.app_id LEFT JOIN deployment_metadata dm ON av.id = dm.app_version AND dm.workspace_id = a.workspace_id\n        WHERE a.workspace_id = $1 AND a.path = $2\n        ORDER BY created_at DESC",
   "describe": {
     "columns": [
       {
@@ -31,5 +31,5 @@
       true
     ]
   },
-  "hash": "d02c1cfefc7d5f87ca4555a092f5fbc777a91271b80d0e488a8cae79afd56d8f"
+  "hash": "b29cf43a194f4a61a67ce95a5485dc287df1b1d79bf5040216ee48583405cd8b"
 }

backend/windmill-api-integration-tests/tests/app_history_cross_workspace.rs

@@ -0,0 +1,146 @@
+//! `app_version.id` is a global sequence, so an app-history deployment message
+//! addressed by `(app_id, app_version)` alone crosses workspaces. This pins both
+//! halves: the write refuses a version that is not the workspace's app's, and the
+//! read only joins deployment metadata owned by the workspace.
+
+use serde_json::json;
+use sqlx::{Pool, Postgres};
+use windmill_test_utils::*;
+
+fn client() -> reqwest::Client {
+    reqwest::Client::new()
+}
+
+fn authed(builder: reqwest::RequestBuilder) -> reqwest::RequestBuilder {
+    builder.header("Authorization", "Bearer SECRET_TOKEN")
+}
+
+fn new_app(path: &str) -> serde_json::Value {
+    json!({
+        "path": path,
+        "summary": "Test app",
+        "value": { "type": "rawapp", "inline_script": null },
+        "policy": { "execution_mode": "anonymous", "triggerables": {} }
+    })
+}
+
+/// Creates the app and returns its `(app_id, version_id)`.
+async fn create_app(port: u16, workspace: &str, path: &str) -> anyhow::Result<(i64, i64)> {
+    let base = format!("http://localhost:{port}/api/w/{workspace}/apps");
+    let resp = authed(client().post(format!("{base}/create")))
+        .json(&new_app(path))
+        .send()
+        .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(status, 201, "create {path} in {workspace}: {body}");
+
+    let latest = authed(client().get(format!("{base}/get_latest_version/{path}")))
+        .send()
+        .await?
+        .json::<serde_json::Value>()
+        .await?;
+    Ok((
+        latest["app_id"].as_i64().unwrap(),
+        latest["version"].as_i64().unwrap(),
+    ))
+}
+
+async fn update_history(
+    port: u16,
+    workspace: &str,
+    app_id: i64,
+    version: i64,
+    msg: &str,
+) -> anyhow::Result<(u16, String)> {
+    let resp = authed(client().post(format!(
+        "http://localhost:{port}/api/w/{workspace}/apps/history_update/a/{app_id}/v/{version}"
+    )))
+    .json(&json!({ "deployment_msg": msg }))
+    .send()
+    .await?;
+    let status = resp.status().as_u16();
+    Ok((status, resp.text().await?))
+}
+
+async fn history(port: u16, workspace: &str, path: &str) -> anyhow::Result<serde_json::Value> {
+    Ok(authed(client().get(format!(
+        "http://localhost:{port}/api/w/{workspace}/apps/history/p/{path}"
+    )))
+    .send()
+    .await?
+    .json()
+    .await?)
+}
+
+#[sqlx::test(
+    migrations = "../migrations",
+    fixtures("base", "app_history_second_workspace")
+)]
+async fn test_app_history_is_workspace_scoped(db: Pool<Postgres>) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    let server = ApiServer::start(db.clone()).await?;
+    let port = server.addr.port();
+
+    let victim_path = "u/test-user/victim_app";
+    let attacker_path = "u/test-user/attacker_app";
+    let (victim_app, victim_version) = create_app(port, "test-workspace", victim_path).await?;
+    let (attacker_app, attacker_version) =
+        create_app(port, "test-workspace-2", attacker_path).await?;
+
+    // Own app, another workspace's version: the version must belong to the app.
+    let (status, body) = update_history(
+        port,
+        "test-workspace-2",
+        attacker_app,
+        victim_version,
+        "injected",
+    )
+    .await?;
+    assert_eq!(status, 404, "foreign version must be rejected: {body}");
+
+    // Another workspace's app entirely: the app must belong to the workspace.
+    let (status, body) = update_history(
+        port,
+        "test-workspace-2",
+        victim_app,
+        victim_version,
+        "injected",
+    )
+    .await?;
+    assert_eq!(status, 404, "foreign app must be rejected: {body}");
+
+    // The legitimate update still lands, and only in its own workspace.
+    let (status, body) = update_history(
+        port,
+        "test-workspace-2",
+        attacker_app,
+        attacker_version,
+        "deployed by owner",
+    )
+    .await?;
+    assert_eq!(status, 200, "own app and version must be accepted: {body}");
+    assert_eq!(
+        history(port, "test-workspace-2", attacker_path).await?[0]["deployment_msg"],
+        "deployed by owner"
+    );
+
+    // A deployment_metadata row owned by another workspace but pointing at this
+    // app's version must not surface — the read joins on workspace too.
+    sqlx::query(
+        "INSERT INTO deployment_metadata (workspace_id, path, app_version, deployment_msg)
+         VALUES ('test-workspace-2', $1, $2, 'injected')",
+    )
+    .bind(victim_path)
+    .bind(victim_version)
+    .execute(&db)
+    .await?;
+
+    assert_eq!(
+        history(port, "test-workspace", victim_path).await?[0]["deployment_msg"],
+        serde_json::Value::Null,
+        "another workspace's deployment metadata must not surface"
+    );
+
+    Ok(())
+}

backend/windmill-api-integration-tests/tests/fixtures/app_history_second_workspace.sql

@@ -0,0 +1,17 @@
+-- Additive fixture: a second workspace sharing the `base` fixture's admin user,
+-- so the same caller can hold an app in each workspace.
+
+INSERT INTO workspace (id, name, owner) VALUES
+	('test-workspace-2', 'test-workspace-2', 'test-user');
+
+INSERT INTO usr(workspace_id, email, username, is_admin, role) VALUES
+	('test-workspace-2', 'test@windmill.dev', 'test-user', true, 'Admin');
+
+INSERT INTO workspace_key(workspace_id, kind, key) VALUES
+	('test-workspace-2', 'cloud', 'test-key-2');
+
+INSERT INTO workspace_settings (workspace_id) VALUES
+	('test-workspace-2');
+
+INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES
+	('test-workspace-2', 'all', 'All users', '{}');

backend/windmill-api/src/apps.rs

@@ -546,7 +546,7 @@ async fn list_apps(
     if lq.with_deployment_msg.unwrap_or(false) {
         sqlb.join("deployment_metadata dm")
             .left()
-            .on("dm.app_version = app.versions[array_upper(app.versions, 1)]")
+            .on("dm.app_version = app.versions[array_upper(app.versions, 1)] AND dm.workspace_id = app.workspace_id")
             .fields(&["dm.deployment_msg"]);
     }
 
@@ -1055,7 +1055,7 @@ async fn get_app_history(
     let mut tx = user_db.begin(&authed).await?;
     let query_result = sqlx::query!(
         "SELECT a.id as app_id, av.id as version_id, dm.deployment_msg as deployment_msg
-        FROM app a LEFT JOIN app_version av ON a.id = av.app_id LEFT JOIN deployment_metadata dm ON av.id = dm.app_version
+        FROM app a LEFT JOIN app_version av ON a.id = av.app_id LEFT JOIN deployment_metadata dm ON av.id = dm.app_version AND dm.workspace_id = a.workspace_id
         WHERE a.workspace_id = $1 AND a.path = $2
         ORDER BY created_at DESC",
         w_id,
@@ -1084,7 +1084,7 @@ async fn get_latest_version(
     let mut tx = user_db.begin(&authed).await?;
     let row = sqlx::query!(
         "SELECT a.id as app_id, av.id as version_id, dm.deployment_msg as deployment_msg
-        FROM app a LEFT JOIN app_version av ON a.id = av.app_id LEFT JOIN deployment_metadata dm ON av.id = dm.app_version
+        FROM app a LEFT JOIN app_version av ON a.id = av.app_id LEFT JOIN deployment_metadata dm ON av.id = dm.app_version AND dm.workspace_id = a.workspace_id
         WHERE a.workspace_id = $1 AND a.path = $2
         ORDER BY created_at DESC",
         w_id,
@@ -1112,15 +1112,24 @@ async fn update_app_history(
     Json(app_history_update): Json<AppHistoryUpdate>,
 ) -> Result<()> {
     let mut tx = user_db.begin(&authed).await?;
-    let app_path = sqlx::query_scalar!("SELECT path FROM app WHERE id = $1", app_id)
-        .fetch_optional(&mut *tx)
-        .await?;
+    // `app_version.id` is a global sequence, so the version has to be tied back to the app and
+    // the app to the workspace: without both, the row this writes lands on another workspace's
+    // deployment history.
+    let app_path = sqlx::query_scalar!(
+        "SELECT a.path FROM app a JOIN app_version av ON av.app_id = a.id
+        WHERE a.id = $1 AND a.workspace_id = $2 AND av.id = $3",
+        app_id,
+        w_id,
+        app_version,
+    )
+    .fetch_optional(&mut *tx)
+    .await?;
 
     let Some(app_path) = app_path else {
         tx.commit().await?;
-        return Err(Error::NotFound(
-            format!("App with ID {app_id} not found").to_string(),
-        ));
+        return Err(Error::NotFound(format!(
+            "App with ID {app_id} and version {app_version} not found"
+        )));
     };
 
     check_scopes(&authed, || format!("apps:write:{}", &app_path))?;