From 9c310cef0116327ff064da9d6501e8e6aaabeb6c Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sun, 2 Aug 2026 21:23:56 +0000 Subject: [PATCH] refactor: make the app policy's address optional instead of derived on read --- ...952d3c36312a1f9f97226004466a80a737b23.json | 17 -- ...660f9d501ac7ae32c2d21e31d8f92cec94fa3.json | 17 -- ...86f0ecfcf63daa0b185994a410eb2fe41fad9.json | 23 -- backend/tests/preserve_on_behalf_of.rs | 164 -------------- backend/windmill-api/src/apps.rs | 202 ++---------------- backend/windmill-api/src/drafts.rs | 21 -- backend/windmill-common/src/min_version.rs | 8 + docs/app-policy-email-removal.md | 36 ++++ 8 files changed, 65 insertions(+), 423 deletions(-) delete mode 100644 backend/.sqlx/query-02c1a4d76fd7edf99c93be8ff57952d3c36312a1f9f97226004466a80a737b23.json delete mode 100644 backend/.sqlx/query-89fe60a488284d3b0a3162ff0f1660f9d501ac7ae32c2d21e31d8f92cec94fa3.json delete mode 100644 backend/.sqlx/query-dadf78bae0299b24f6798e7a2cc86f0ecfcf63daa0b185994a410eb2fe41fad9.json create mode 100644 docs/app-policy-email-removal.md diff --git a/backend/.sqlx/query-02c1a4d76fd7edf99c93be8ff57952d3c36312a1f9f97226004466a80a737b23.json b/backend/.sqlx/query-02c1a4d76fd7edf99c93be8ff57952d3c36312a1f9f97226004466a80a737b23.json deleted file mode 100644 index 27ff1712db..0000000000 --- a/backend/.sqlx/query-02c1a4d76fd7edf99c93be8ff57952d3c36312a1f9f97226004466a80a737b23.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "INSERT INTO draft (workspace_id, email, path, typ, value)\n VALUES ($1, $2, $3, 'app', $4)", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Varchar", - "Varchar", - "Varchar", - "Json" - ] - }, - "nullable": [] - }, - "hash": "02c1a4d76fd7edf99c93be8ff57952d3c36312a1f9f97226004466a80a737b23" -} diff --git a/backend/.sqlx/query-89fe60a488284d3b0a3162ff0f1660f9d501ac7ae32c2d21e31d8f92cec94fa3.json b/backend/.sqlx/query-89fe60a488284d3b0a3162ff0f1660f9d501ac7ae32c2d21e31d8f92cec94fa3.json deleted file mode 100644 index bbeb3f9ddf..0000000000 --- a/backend/.sqlx/query-89fe60a488284d3b0a3162ff0f1660f9d501ac7ae32c2d21e31d8f92cec94fa3.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "INSERT INTO draft (workspace_id, email, path, typ, value)\n VALUES ($1, $2, $3, 'raw_app', $4)", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Varchar", - "Varchar", - "Varchar", - "Json" - ] - }, - "nullable": [] - }, - "hash": "89fe60a488284d3b0a3162ff0f1660f9d501ac7ae32c2d21e31d8f92cec94fa3" -} diff --git a/backend/.sqlx/query-dadf78bae0299b24f6798e7a2cc86f0ecfcf63daa0b185994a410eb2fe41fad9.json b/backend/.sqlx/query-dadf78bae0299b24f6798e7a2cc86f0ecfcf63daa0b185994a410eb2fe41fad9.json deleted file mode 100644 index 6ae3f60e49..0000000000 --- a/backend/.sqlx/query-dadf78bae0299b24f6798e7a2cc86f0ecfcf63daa0b185994a410eb2fe41fad9.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT email FROM usr WHERE workspace_id = $1 AND username = $2", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "email", - "type_info": "Varchar" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - false - ] - }, - "hash": "dadf78bae0299b24f6798e7a2cc86f0ecfcf63daa0b185994a410eb2fe41fad9" -} diff --git a/backend/tests/preserve_on_behalf_of.rs b/backend/tests/preserve_on_behalf_of.rs index 65139ebb69..1361c6c5db 100644 --- a/backend/tests/preserve_on_behalf_of.rs +++ b/backend/tests/preserve_on_behalf_of.rs @@ -1391,171 +1391,7 @@ async fn test_app_update_preserves_on_behalf_of(db: Pool) -> anyhow::R Ok(()) } -/// A draft saved while the address was stored independently can name an account its principal -/// no longer resolves to. The editor deploys the draft's policy verbatim, so reading it back -/// has to derive the address too, or that draft can never be deployed again. -#[sqlx::test(fixtures("preserve_on_behalf_of"))] -async fn test_app_draft_policy_derives_on_behalf_of_email(db: Pool) -> anyhow::Result<()> -{ - initialize_tracing().await; - let server = ApiServer::start(db.clone()).await?; - let port = server.addr.port(); - let base = format!("http://localhost:{port}/api/w/test-workspace"); - let path = "u/original-user/app_with_drifted_draft"; - - let resp = authed(client().post(format!("{base}/apps/create")), "SECRET_TOKEN") - .json(&new_app_with_on_behalf_of( - path, - Some("u/original-user"), - Some("original@windmill.dev"), - true, - )) - .send() - .await?; - assert_eq!( - resp.status(), - 201, - "Should create app: {}", - resp.text().await? - ); - - sqlx::query!( - "INSERT INTO draft (workspace_id, email, path, typ, value) - VALUES ($1, $2, $3, 'app', $4)", - "test-workspace", - "test@windmill.dev", - path, - json!({ - "path": path, - "summary": "Drifted draft", - "value": { "type": "rawapp", "inline_script": null }, - "policy": { - "execution_mode": "anonymous", - "triggerables": {}, - "on_behalf_of": "u/original-user", - "on_behalf_of_email": "stale@windmill.dev" - } - }) - ) - .execute(&db) - .await?; - - let resp = authed( - client().get(format!("{base}/apps/get/p/{path}?get_draft=true")), - "SECRET_TOKEN", - ) - .send() - .await?; - assert_eq!(resp.status(), 200, "Should get app with draft"); - let body: serde_json::Value = resp.json().await?; - let draft_policy = &body["draft"]["policy"]; - assert_eq!( - draft_policy.get("on_behalf_of_email").and_then(|v| v.as_str()), - Some("original@windmill.dev"), - "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}")), - "SECRET_TOKEN", - ) - .json(&json!({ - "summary": "Drifted draft", - "policy": draft_policy, - "preserve_on_behalf_of": true - })) - .send() - .await?; - assert_eq!( - resp.status(), - 200, - "Deploying the draft back should not be rejected as a contradicting identity: {}", - resp.text().await? - ); - - Ok(()) -} - -/// A draft is stored unvalidated, so its principal is whatever its author typed. Resolving it -/// through the instance-wide fallback would let any member turn a guessed username into a -/// non-member superadmin's address. -#[sqlx::test(fixtures("preserve_on_behalf_of"))] -async fn test_app_draft_derivation_does_not_disclose_non_member_email( - db: Pool, -) -> anyhow::Result<()> { - initialize_tracing().await; - - let server = ApiServer::start(db.clone()).await?; - let port = server.addr.port(); - let base = format!("http://localhost:{port}/api/w/test-workspace"); - let path = "u/test-user-2/leak_probe"; - - // test-user-2 is an ordinary member; superadmin-external has no `usr` row here. - sqlx::query!( - "INSERT INTO draft (workspace_id, email, path, typ, value) - VALUES ($1, $2, $3, 'raw_app', $4)", - "test-workspace", - "test2@windmill.dev", - path, - json!({ - "path": path, - "summary": "Probe", - "value": { "type": "rawapp", "inline_script": null }, - "policy": { - "execution_mode": "anonymous", - "triggerables": {}, - "on_behalf_of": "u/superadmin-external", - "on_behalf_of_email": "stale@windmill.dev" - } - }) - ) - .execute(&db) - .await?; - - for (url, at) in [ - (format!("{base}/drafts/get_own/raw_app/{path}"), "value"), - ( - format!("{base}/apps/get/p/{path}?get_draft=true&raw_app=true"), - "draft", - ), - ] { - let resp = authed(client().get(&url), "SECRET_TOKEN_2").send().await?; - assert_eq!(resp.status(), 200, "Should read own draft at {url}"); - let body = resp.text().await?; - assert!( - !body.contains("superadmin-external@windmill.dev"), - "Draft derivation must not disclose a non-member superadmin's address via {url}: {body}" - ); - // Dropped, not left stale: the principal is one the deploy path accepts, so a stale - // address beside it would reject the draft forever. - let policy = &serde_json::from_str::(&body)?[at]["policy"]; - assert!( - policy.get("on_behalf_of_email").is_none(), - "Unresolvable principal should leave no address behind via {url}: {policy}" - ); - } - - Ok(()) -} /// Test schedule update preserves email/edited_by correctly #[sqlx::test(fixtures("preserve_on_behalf_of"))] diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 356f9823c5..9cafc0cb54 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -256,162 +256,10 @@ pub struct AppWithLastVersionAndStarred { pub starred: Option, } -/// Fill `policy.on_behalf_of_email` from `policy.on_behalf_of` on every read path that returns -/// a policy, so what a client sees is a function of the principal rather than whatever copy the -/// row happens to hold. -/// -/// `round_trips` picks the lookup. A read whose policy the client sends back on deploy must use -/// the uncached one: `resolve_on_behalf_of` validates the returned pair against the uncached -/// address, so a cached read would echo a stale address and 400 its own deploy for the minute -/// after an email change. The serving reads keep the cache — `get_public_app_by_secret` is an -/// anonymous page load, and nothing it returns comes back as a write. -/// -/// The substring test keeps a policy with no principal off the parse-and-reserialize path -/// entirely, so an app without an on-behalf-of identity pays nothing either way. -async fn derive_policy_on_behalf_of_email( - db: &DB, - w_id: &str, - policy: &mut sqlx::types::Json>, - round_trips: bool, -) -> Result<()> { - if !policy.0.get().contains("on_behalf_of") { - return Ok(()); - } - let Ok(mut obj) = - serde_json::from_str::>(policy.0.get()) - else { - return Ok(()); - }; - if derive_on_behalf_of_email_in_place(db, w_id, &mut obj, round_trips).await? { - policy.0 = to_raw_value(&obj); - } - Ok(()) -} -/// The draft variant of the derivation. A draft is stored exactly as its editor sent it, with -/// no `resolve_on_behalf_of` pass, so its principal is caller-controlled: anyone who can save a -/// draft under their own `u/` path chooses it. `u/` is therefore resolved against workspace -/// membership alone, never `resolve_username_to_email`'s instance `password` fallback, which -/// would turn a guessed username into a non-member superadmin's address. -/// -/// A principal naming no member has its address key dropped rather than answered or left as it -/// was: an external superadmin is a principal the deploy path accepts, so leaving a stale -/// address beside it would keep 400ing a draft that is otherwise fine, while dropping it lets -/// `resolve_on_behalf_of` derive from the principal alone. Nothing is disclosed either way. -/// -/// `g/` and the bare-address form skip the lookup: the first is synthetic and the second is -/// already the address, so neither tells the caller anything their draft did not. -async fn derive_draft_on_behalf_of_email_in_place( - db: &DB, - w_id: &str, - obj: &mut serde_json::Map, -) -> Result { - let Some(permissioned_as) = obj - .get("on_behalf_of") - .and_then(|v| v.as_str()) - .map(|s| s.to_string()) - else { - return Ok(false); - }; - let email = match permissioned_as.strip_prefix(windmill_common::users::PERMISSIONED_AS_USER_PREFIX) - { - Some(username) => { - let member = sqlx::query_scalar!( - "SELECT email FROM usr WHERE workspace_id = $1 AND username = $2", - w_id, - username - ) - .fetch_optional(db) - .await?; - match member { - Some(email) => email, - None => return Ok(obj.remove("on_behalf_of_email").is_some()), - } - } - None => { - windmill_common::users::get_email_from_permissioned_as_uncached( - &permissioned_as, - w_id, - db, - ) - .await? - } - }; - obj.insert("on_behalf_of_email".to_string(), json!(email)); - Ok(true) -} -/// Same derivation on an already-parsed policy. Reports whether it rewrote the address, so -/// callers holding the raw form only pay to re-serialize when there was a principal to derive. -async fn derive_on_behalf_of_email_in_place( - db: &DB, - w_id: &str, - obj: &mut serde_json::Map, - round_trips: bool, -) -> Result { - let Some(permissioned_as) = obj - .get("on_behalf_of") - .and_then(|v| v.as_str()) - .map(|s| s.to_string()) - else { - return Ok(false); - }; - let email = if round_trips { - windmill_common::users::get_email_from_permissioned_as_uncached(&permissioned_as, w_id, db) - .await? - } else { - windmill_common::users::get_email_from_permissioned_as(&permissioned_as, w_id, db).await? - }; - obj.insert("on_behalf_of_email".to_string(), json!(email)); - Ok(true) -} -/// A draft carries its own copy of the policy and is deployed from it verbatim, so it needs the -/// same derivation as the deployed one. A pair that drifted while the address was still stored -/// independently would otherwise survive here — the backfill only fills in a missing principal — -/// and deploying the draft unchanged would be rejected as a contradicting identity. -async fn derive_draft_policy_on_behalf_of_email( - db: &DB, - w_id: &str, - draft: Option<&mut serde_json::Value>, -) -> Result<()> { - let Some(policy) = draft - .and_then(|d| d.get_mut("policy")) - .and_then(|p| p.as_object_mut()) - else { - return Ok(()); - }; - derive_draft_on_behalf_of_email_in_place(db, w_id, policy).await?; - 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. -/// -/// Not an authorization boundary: it resolves an address for whatever workspace and principal -/// it is handed, so a caller acting for a user MUST already have established their read access -/// to that draft. -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_draft_on_behalf_of_email_in_place(db, w_id, policy).await? { - value.0 = to_raw_value(&obj); - } - Ok(()) -} #[derive(Serialize)] pub struct AppHistory { @@ -471,10 +319,12 @@ pub struct S3Key { #[derive(Serialize, Deserialize, Debug, Clone, Default)] pub struct Policy { pub on_behalf_of: Option, - /// Derived from `on_behalf_of`, never taken from the request: reads fill it from the - /// principal and writes store what the principal resolves to. Accepted on write so a client - /// naming only the address still resolves to a principal, and rejected there when the two - /// disagree. + /// The address `on_behalf_of` resolves to. Every write stores what the principal resolves + /// to, so it is not taken from the request except when a client names only the address — + /// which is how a cross-workspace deploy carries an identity — and it is rejected when the + /// two disagree. Optional on read: a policy without it executes by deriving from the + /// principal, which is what makes removing it a change of default (`MIN_VERSION_DERIVES_ + /// APP_POLICY_EMAIL`, `docs/app-policy-email-removal.md`). #[serde(skip_serializing_if = "Option::is_none")] pub on_behalf_of_email: Option, //paths: @@ -1077,7 +927,7 @@ async fn get_app( check_scopes(&authed, || format!("apps:read:{}", path))?; let mut tx = user_db.begin(&authed).await?; - let mut app_o = if query.with_starred_info.unwrap_or(false) { + let app_o = if query.with_starred_info.unwrap_or(false) { sqlx::query_as::<_, AppWithLastVersionAndStarred>( "SELECT app.id, app.path, app.summary, app.versions, app.policy, app.custom_path, app.extra_perms, app_version.value, @@ -1112,10 +962,6 @@ async fn get_app( }; tx.commit().await?; - if let Some(app) = app_o.as_mut() { - derive_policy_on_behalf_of_email(&db, &w_id, &mut app.app.policy, true).await?; - } - // No deployed row + `get_draft`: fall back to the draft table; see scripts.rs. // Draft kind comes from the deployed row's `raw_app` flag, or for a draft-only // path from the caller's `raw_app` query param. @@ -1125,7 +971,7 @@ async fn get_app( None if query.raw_app.unwrap_or(false) => UserDraftItemKind::RawApp, None => UserDraftItemKind::App, }; - let mut overlay = overlay_or_draft_only( + let overlay = overlay_or_draft_only( &db, &w_id, &authed.email, @@ -1136,14 +982,12 @@ async fn get_app( || windmill_common::error::Error::NotFound(format!("App not found at path {path}")), ) .await?; - derive_draft_policy_on_behalf_of_email(&db, &w_id, overlay.draft.as_mut()).await?; Ok(Json(overlay)) } async fn get_app_lite( authed: ApiAuthed, Extension(user_db): Extension, - Extension(db): Extension, Path((w_id, path)): Path<(String, StripPath)>, ) -> JsonResult { let path = path.to_path(); @@ -1165,8 +1009,7 @@ async fn get_app_lite( tx.commit().await?; - let mut app = not_found_if_none(app_o, "App", path)?; - derive_policy_on_behalf_of_email(&db, &w_id, &mut app.policy, false).await?; + let app = not_found_if_none(app_o, "App", path)?; Ok(Json(app)) } @@ -1283,7 +1126,6 @@ async fn custom_path_exists( async fn get_app_by_id( authed: ApiAuthed, Extension(user_db): Extension, - Extension(db): Extension, Path((w_id, id)): Path<(String, i64)>, ) -> JsonResult { let mut tx = user_db.begin(&authed).await?; @@ -1301,14 +1143,10 @@ async fn get_app_by_id( .await?; tx.commit().await?; - let mut app = not_found_if_none(app_o, "App", id.to_string())?; + let app = not_found_if_none(app_o, "App", id.to_string())?; check_scopes(&authed, || format!("apps:read:{}", &app.path))?; - // 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)) } @@ -1376,7 +1214,6 @@ async fn get_public_app_by_secret( app.bundle_secret = Some(compute_bundle_secret(&db, &w_id, &app.versions).await?); } - derive_policy_on_behalf_of_email(&db, &w_id, &mut app.policy, false).await?; Ok(Json(app)) } @@ -4860,12 +4697,10 @@ fn audited_on_behalf_of(policy: &Policy, authed: &ApiAuthed) -> Option { /// The identity an anonymous or publisher execution runs as. /// -/// The address becomes the job's `permissioned_as_email`, from which -/// `fetch_authed_from_permissioned_as` derives the superadmin flag and the instance groups, so it -/// must not be stale. It is read through `EMAIL_CACHE` anyway: the `notify_user_email_change` -/// trigger evicts the key on every change that can move it, on every replica, the same way this -/// route's own `APP_POLICY_CACHE` is kept honest. Resolving per execution instead would put the -/// only uncached query on a path that otherwise serves entirely from cache. +/// `on_behalf_of_email` is optional: every write stores it, so it is present on anything this +/// release deployed, and it is only derived for a policy that predates that. Deriving is the +/// fallback rather than the rule so that removing the key later is a change of default, not a +/// change of behavior — see `docs/app-policy-email-removal.md`. async fn get_on_behalf_of(policy: &Policy, w_id: &str, db: &DB) -> Result<(String, String)> { let permissioned_as = policy .on_behalf_of @@ -4877,8 +4712,13 @@ async fn get_on_behalf_of(policy: &Policy, w_id: &str, db: &DB) -> Result<(Strin ) })? .to_string(); - let email = - windmill_common::users::get_email_from_permissioned_as(&permissioned_as, w_id, db).await?; + let email = match policy.on_behalf_of_email.as_deref() { + Some(email) => email.to_string(), + None => { + windmill_common::users::get_email_from_permissioned_as(&permissioned_as, w_id, db) + .await? + } + }; Ok((permissioned_as, email)) } diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index b81ed0f03f..8584cb65ce 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -618,10 +618,6 @@ 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}", @@ -658,26 +654,9 @@ 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 diff --git a/backend/windmill-common/src/min_version.rs b/backend/windmill-common/src/min_version.rs index 80b3871f3d..787af12a05 100644 --- a/backend/windmill-common/src/min_version.rs +++ b/backend/windmill-common/src/min_version.rs @@ -8,6 +8,14 @@ use semver::Version; // reads that column is live, and those runnables run as their deployer with no error anywhere. pub const MIN_VERSION_SUPPORTS_ON_BEHALF_OF_PRINCIPAL: VC = vc(1, 776, 0, "On-behalf-of principal"); + +/// Names the release that stops *requiring* `policy.on_behalf_of_email`: an app policy carrying +/// only a principal executes, deriving the address from it. The key is still written, so a +/// replica below this version keeps finding what it expects. Nothing gates on this at runtime; +/// it exists so `vc()`'s compile-time assert fires once `MIN_KEEP_ALIVE_VERSION` passes it, at +/// which point the key can go — see `docs/app-policy-email-removal.md`. +pub const MIN_VERSION_DERIVES_APP_POLICY_EMAIL: VC = + vc(1, 778, 0, "App policy email derived from on_behalf_of"); pub const MIN_VERSION_SUPPORTS_NODE_DEBOUNCING: VC = vc(1, 658, 0, "Flow node debouncing"); pub const MIN_VERSION_SUPPORTS_TOKEN_HASH: VC = vc(1, 659, 0, "Token hash storage"); pub const MIN_VERSION_SUPPORTS_SYNC_JOBS_DEBOUNCING: VC = vc(1, 602, 0, "Sync jobs debouncing"); diff --git a/docs/app-policy-email-removal.md b/docs/app-policy-email-removal.md new file mode 100644 index 0000000000..bdb11cc1c8 --- /dev/null +++ b/docs/app-policy-email-removal.md @@ -0,0 +1,36 @@ +# Removing `policy.on_behalf_of_email` + +An app's identity is `policy.on_behalf_of`; the address beside it is a function of that +principal. `on_behalf_of_email` is no longer required — a policy carrying only a principal +executes, deriving the address — but it is still written on every save, because a replica below +`MIN_VERSION_DERIVES_APP_POLICY_EMAIL` (`windmill-common/src/min_version.rs`) errors outright +when the key is absent, and a rolling deploy runs both versions at once. + +When that constraint stops compiling, no supported replica requires the key. + +## Release A — code only, key still written + +1. `stored_on_behalf_of_email` in `windmill-api/src/apps.rs`, and the `update_app_internal` / + `create_app` call sites that store what it returns. +2. The CLI and frontend workspace-deploy paths that send an address for a cross-workspace + deploy (`cli/windmill-utils-internal/src/deploy.ts`, + `frontend/src/lib/utils_workspace_deploy.ts`). They send the address *instead of* a + principal, which is the one case where it is the only identity available — so this step is + really "stop sending it once the target can resolve a principal itself", not a deletion. + +## Release B — once every replica runs A + +3. Drop the key from `Policy` and stop reading it in `get_on_behalf_of`, which then always + derives. A migration can strip it from stored policies, or leave it as an ignored field. + +Removing it in release A would break the replicas still on the release before it: their +`get_on_behalf_of` requires the key and 400s every anonymous and publisher app without it. + +## Why the address is not derived on read + +Read paths return the stored address verbatim rather than recomputing it. Deriving on read means +resolving a principal that, for a draft, is caller-controlled — which turns the read into an +oracle for addresses the caller cannot otherwise see, and leaves a principal that resolves to +nobody with no address at all. Both were live defects while the read paths did derive. The +address a policy carries is written from its principal on every save, so the stored value is +already the derived one for anything this release wrote.