From 997db1d6d12cfc9f18dc8e0cbd334cac34cc47ab Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sat, 1 Aug 2026 22:13:26 +0000 Subject: [PATCH] fix: derive the app identity on the draft-table and version reads too --- backend/tests/preserve_on_behalf_of.rs | 17 +++++++++++++++ backend/windmill-api/src/apps.rs | 29 +++++++++++++++++++++++++- backend/windmill-api/src/drafts.rs | 22 +++++++++++++++++++ 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/backend/tests/preserve_on_behalf_of.rs b/backend/tests/preserve_on_behalf_of.rs index 4c94123c45..36bb5173ee 100644 --- a/backend/tests/preserve_on_behalf_of.rs +++ b/backend/tests/preserve_on_behalf_of.rs @@ -1456,6 +1456,23 @@ async fn test_app_draft_policy_derives_on_behalf_of_email(db: Pool) -> "Draft policy address should be derived from its principal, not the stored one" ); + // The AI chat deploys the draft it reads straight from the draft table instead. + let resp = authed( + client().get(format!("{base}/drafts/get_own/app/{path}")), + "SECRET_TOKEN", + ) + .send() + .await?; + assert_eq!(resp.status(), 200, "Should get own draft"); + let own: serde_json::Value = resp.json().await?; + assert_eq!( + own["value"]["policy"] + .get("on_behalf_of_email") + .and_then(|v| v.as_str()), + Some("original@windmill.dev"), + "Draft read straight from the draft table should derive its address too" + ); + // Deploy the draft exactly as the editor received it. let resp = authed( client().post(format!("{base}/apps/update/{path}")), diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 0463aceada..413268ec4c 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -332,6 +332,30 @@ async fn derive_draft_policy_on_behalf_of_email( Ok(()) } +/// The same repair for a draft served straight out of the draft table rather than through +/// `get_app`, which is how the AI chat and the other-users' -draft banner read app drafts. +pub(crate) async fn derive_stored_draft_policy_on_behalf_of_email( + db: &DB, + w_id: &str, + value: &mut sqlx::types::Json>, +) -> Result<()> { + if !value.0.get().contains("on_behalf_of") { + return Ok(()); + } + let Ok(mut obj) = + serde_json::from_str::>(value.0.get()) + else { + return Ok(()); + }; + let Some(policy) = obj.get_mut("policy").and_then(|p| p.as_object_mut()) else { + return Ok(()); + }; + if derive_on_behalf_of_email_in_place(db, w_id, policy, true).await? { + value.0 = to_raw_value(&obj); + } + Ok(()) +} + #[derive(Serialize)] pub struct AppHistory { pub app_id: i64, @@ -1224,7 +1248,10 @@ async fn get_app_by_id( check_scopes(&authed, || format!("apps:read:{}", &app.path))?; - derive_policy_on_behalf_of_email(&db, &w_id, &mut app.policy, false).await?; + // Round-tripping: the deployment-history drawer redeploys the version it fetched here, so + // an address left stale by another replica's email change would come back as a + // contradicting pair. + derive_policy_on_behalf_of_email(&db, &w_id, &mut app.policy, true).await?; Ok(Json(app)) } diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index b44b0e64be..b81ed0f03f 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -618,6 +618,10 @@ async fn get_draft_for_user( .fetch_optional(&db) .await?; + let mut row = row; + if let Some(row) = row.as_mut() { + derive_app_draft_identity(&db, &w_id, kind, &mut row.value).await?; + } row.map(Json).ok_or_else(|| { Error::NotFound(format!( "no draft for {} at {path}", @@ -654,9 +658,27 @@ async fn get_own_draft( ) .fetch_optional(&db) .await?; + let mut row = row; + if let Some(row) = row.as_mut() { + derive_app_draft_identity(&db, &w_id, kind, &mut row.value).await?; + } Ok(Json(row)) } +/// An app draft is deployed from its own copy of the policy, so the address beside its +/// principal has to be derived here too — see `apps::derive_stored_draft_policy_on_behalf_of_email`. +async fn derive_app_draft_identity( + db: &DB, + w_id: &str, + kind: UserDraftItemKind, + value: &mut sqlx::types::Json>, +) -> Result<()> { + if matches!(kind, UserDraftItemKind::App | UserDraftItemKind::RawApp) { + crate::apps::derive_stored_draft_policy_on_behalf_of_email(db, w_id, value).await?; + } + Ok(()) +} + /// The deployed table RLS resolves item-level `extra_perms` against. /// Delegates to `UserDraftItemKind::deployed_table()` (the shared single /// source); `None` kinds fall through to the path-only access check.