fix: map an app's identity into the workspace a merge deploys to

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EYWxvwmteVyzcWenJfbnzs

backend/.sqlx/query-08a3f282c1f8bded7991baeb5dc14131e707e4438fd1ddf38112fe66ad625691.json

@@ -1,16 +0,0 @@
-{
-  "db_name": "PostgreSQL",
-  "query": "UPDATE app SET policy = jsonb_set(policy, ARRAY['on_behalf_of'], to_jsonb($1::text))\n         WHERE policy->>'on_behalf_of' = $2 AND workspace_id = $3",
-  "describe": {
-    "columns": [],
-    "parameters": {
-      "Left": [
-        "Text",
-        "Text",
-        "Text"
-      ]
-    },
-    "nullable": []
-  },
-  "hash": "08a3f282c1f8bded7991baeb5dc14131e707e4438fd1ddf38112fe66ad625691"
-}

backend/.sqlx/query-234fb711a4047a084cb18e18557c45db885a37b7a7473453747c55f3378599de.json

@@ -0,0 +1,16 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "UPDATE app SET policy =\n            jsonb_set(policy, ARRAY['on_behalf_of'], to_jsonb($1::text)) - 'on_behalf_of_email'\n         WHERE policy->>'on_behalf_of' = $2 AND workspace_id = $3",
+  "describe": {
+    "columns": [],
+    "parameters": {
+      "Left": [
+        "Text",
+        "Text",
+        "Text"
+      ]
+    },
+    "nullable": []
+  },
+  "hash": "234fb711a4047a084cb18e18557c45db885a37b7a7473453747c55f3378599de"
+}

backend/.sqlx/query-c38271cd19861d39f7070799c8ee5eec2d171431d3ce710263eeb16010becfae.json

@@ -1,16 +0,0 @@
-{
-  "db_name": "PostgreSQL",
-  "query": "UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), ARRAY['policy', 'on_behalf_of'], to_jsonb($1::text))) WHERE typ IN ('app', 'raw_app') AND value->'policy'->>'on_behalf_of' = $2 AND workspace_id = $3",
-  "describe": {
-    "columns": [],
-    "parameters": {
-      "Left": [
-        "Text",
-        "Text",
-        "Text"
-      ]
-    },
-    "nullable": []
-  },
-  "hash": "c38271cd19861d39f7070799c8ee5eec2d171431d3ce710263eeb16010becfae"
-}

backend/.sqlx/query-f9ada4073facedfd8d1a3ba83a2894927bd74aedbe176502c6e44ad0072ac608.json

@@ -0,0 +1,16 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), ARRAY['policy', 'on_behalf_of'], to_jsonb($1::text)) #- '{policy,on_behalf_of_email}') WHERE typ IN ('app', 'raw_app') AND value->'policy'->>'on_behalf_of' = $2 AND workspace_id = $3",
+  "describe": {
+    "columns": [],
+    "parameters": {
+      "Left": [
+        "Text",
+        "Text",
+        "Text"
+      ]
+    },
+    "nullable": []
+  },
+  "hash": "f9ada4073facedfd8d1a3ba83a2894927bd74aedbe176502c6e44ad0072ac608"
+}

backend/tests/preserve_on_behalf_of.rs

@@ -556,6 +556,28 @@ async fn test_preserve_on_behalf_of(db: Pool<Postgres>) -> anyhow::Result<()> {
         "the principal is stored, not the account the address named"
     );
 
+    // ========================================
+    // 9c. App: an identity named only by address is refused, not silently dropped
+    // ========================================
+
+    // What a client written before the principal existed sends for "keep this identity". There
+    // is nothing to keep it by, and deploying it as the pusher would widen what the app runs as.
+    let resp = authed(client().post(format!("{base}/apps/create")), "SECRET_TOKEN")
+        .json(&new_app_with_on_behalf_of(
+            "u/test-user/app_address_only",
+            None,
+            Some("original@windmill.dev"),
+            true,
+        ))
+        .send()
+        .await?;
+    assert_eq!(
+        resp.status(),
+        400,
+        "an address-only identity must be refused: {}",
+        resp.text().await?
+    );
+
     // ========================================
     // 10. Schedule: Admin preserves email and edited_by
     // ========================================

backend/windmill-api-workspaces/src/workspaces.rs

@@ -6995,6 +6995,9 @@ fn repoint_cloned_app_identity(policy: &mut serde_json::Value, authed: &ApiAuthe
         "on_behalf_of".to_string(),
         serde_json::Value::String(username_to_permissioned_as(&authed.username)),
     );
+    // A policy an older peer wrote carries the source's address, and that peer prefers it at
+    // execution — so leaving it would hand the fork exactly the identity this re-point withholds.
+    obj.remove("on_behalf_of_email");
 }
 
 async fn clone_apps(

backend/windmill-api/src/apps.rs

@@ -495,6 +495,12 @@ pub struct Policy {
     /// the request. [`attach_on_behalf_of_email`] puts it here, and states which reads may.
     #[serde(skip_deserializing, skip_serializing_if = "Option::is_none")]
     pub on_behalf_of_email: Option<String>,
+    /// The same key on the way in, taken only so that a request naming the identity by address
+    /// alone can be refused. Silently ignoring it would redeploy the app as whoever pushed it,
+    /// which is how a client written before the principal existed names an identity. An echo of
+    /// a read carries the principal too, so this is never the whole of what a caller sent.
+    #[serde(default, rename = "on_behalf_of_email", skip_serializing)]
+    pub submitted_on_behalf_of_email: Option<String>,
     //paths:
     // - script/<path>
     // - flow/<path>
@@ -2472,6 +2478,8 @@ async fn create_app_internal<'a>(
             ));
         }
     }
+    refuse_address_only_identity(&app.policy, app.preserve_on_behalf_of)?;
+
     // Resolve the on-behalf-of defaults on the (non-RLS) pool *before* opening
     // the RLS transaction below: doing these lookups mid-transaction would hold
     // a second simultaneous connection while `tx` is still checked out. The race this
@@ -3410,6 +3418,8 @@ async fn update_app_internal<'a>(
     // stays.
     let mut preserved_on_behalf_of: Option<String> = None;
     if let Some(npolicy) = ns.policy.as_mut() {
+        refuse_address_only_identity(npolicy, ns.preserve_on_behalf_of)?;
+
         let should_preserve = ns.preserve_on_behalf_of.unwrap_or(false)
             && windmill_common::can_preserve_on_behalf_of(&authed)
             && npolicy.on_behalf_of.is_some();
@@ -4574,6 +4584,7 @@ async fn upload_s3_file_from_app(
             triggerables_v2: None,
             on_behalf_of: None,
             on_behalf_of_email: None,
+            submitted_on_behalf_of_email: None,
             s3_inputs: Some(vec![S3Input {
                 file_key_regex: file_key_regex,
                 allow_user_resources: query.force_viewer_allow_user_resources.unwrap_or(false),
@@ -4999,6 +5010,7 @@ async fn get_on_behalf_authed_from_app(
             triggerables_v2: None,
             on_behalf_of: None,
             on_behalf_of_email: None,
+            submitted_on_behalf_of_email: None,
             s3_inputs: None,
             allowed_s3_keys: Some(force_allowed_s3_keys),
             sandbox: None,
@@ -5023,6 +5035,7 @@ async fn get_on_behalf_authed_from_app(
                 triggerables_v2: None,
                 on_behalf_of: None,
                 on_behalf_of_email: None,
+                submitted_on_behalf_of_email: None,
                 s3_inputs: None,
                 allowed_s3_keys: None,
                 sandbox: None,
@@ -5543,6 +5556,27 @@ async fn app_load_csv_preview() -> Result<()> {
     ))
 }
 
+/// Refuse a deploy that asks to keep an identity it names only by address.
+///
+/// The address is no longer an identity the policy can store, so honouring such a request is
+/// impossible and ignoring it is worse than failing: `should_preserve` would come out false and
+/// the app would be redeployed as whoever pushed it — widening, not narrowing, what it runs as.
+/// Only the ambiguous shape is refused; a client echoing back a read sends the principal beside
+/// the address, and one that means "run as me" sends neither.
+fn refuse_address_only_identity(policy: &Policy, preserve: Option<bool>) -> Result<()> {
+    if preserve.unwrap_or(false)
+        && policy.on_behalf_of.is_none()
+        && policy.submitted_on_behalf_of_email.is_some()
+    {
+        return Err(Error::BadRequest(
+            "an app policy names its identity in on_behalf_of ('u/{username}' or 'g/{group}'); \
+             on_behalf_of_email is derived from it and cannot be preserved on its own"
+                .to_string(),
+        ));
+    }
+    Ok(())
+}
+
 /// Attach to a policy on its way out the address its principal resolves to.
 ///
 /// Derived here rather than left to the caller because the app bundle reads it as `ctx.author`

backend/windmill-api/src/offboarding.rs

@@ -842,8 +842,8 @@ async fn offboard_user_from_workspace<'c>(
     let new_prefix = reassign_to.to_string();
     let departing = departing_principal(username);
 
-    // The app policy stores an address beside its principal, and script/flow keep one for the
-    // workers that still read it, so the replacement's is resolved here.
+    // Script and flow rows keep an address beside their principal for the workers that still
+    // read it, so the replacement's is resolved here.
     // resolve_new_permissioned_as already validated the user exists.
     let new_on_behalf_of_user_username = new_permissioned_as
         .strip_prefix("u/")
@@ -956,8 +956,12 @@ async fn offboard_user_from_workspace<'c>(
     .await?
     .unwrap_or(0);
 
+    // The address is dropped, not rewritten: an app policy carries none, and a peer still
+    // running the release before that writes one on every save — and prefers it at execution.
+    // Leaving one behind here is what would keep the departing member's identity live.
     sqlx::query!(
-        "UPDATE app SET policy = jsonb_set(policy, ARRAY['on_behalf_of'], to_jsonb($1::text))
+        "UPDATE app SET policy =
+            jsonb_set(policy, ARRAY['on_behalf_of'], to_jsonb($1::text)) - 'on_behalf_of_email'
          WHERE policy->>'on_behalf_of' = $2 AND workspace_id = $3",
         &new_permissioned_as,
         departing.as_deref(),
@@ -969,7 +973,7 @@ async fn offboard_user_from_workspace<'c>(
     // An app draft carries a copy of the deployed policy and is deployed from it, so it needs
     // the same principal rewritten — the draft sweep above only covers scripts and flows.
     sqlx::query!(
-        r#"UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), ARRAY['policy', 'on_behalf_of'], to_jsonb($1::text))) WHERE typ IN ('app', 'raw_app') AND value->'policy'->>'on_behalf_of' = $2 AND workspace_id = $3"#,
+        r#"UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), ARRAY['policy', 'on_behalf_of'], to_jsonb($1::text)) #- '{policy,on_behalf_of_email}') WHERE typ IN ('app', 'raw_app') AND value->'policy'->>'on_behalf_of' = $2 AND workspace_id = $3"#,
         new_permissioned_as,
         departing.as_deref(),
         w_id

cli/src/commands/workspace/merge.ts

@@ -182,6 +182,49 @@ function triggerService(kind: TriggerDeployKind) {
   }
 }
 
+/**
+ * The identity to preserve for one item, in the form its kind travels in and valid in the
+ * *target* workspace.
+ *
+ * A flow or script sends an address, which the backend resolves against the target, so the
+ * source's answer carries over untouched. An app sends a principal — its policy stores nothing
+ * else — and `u/` names are per-workspace, so the source's is mapped through the address it
+ * resolves to. That mapping is what the backend used to do for as long as the address itself
+ * travelled; without it the app would land on whoever holds that username in the target, or on
+ * nobody. A group carries over by name: `g/ops` means the target's ops group, which is the
+ * identity a fork and its parent are meant to share.
+ *
+ * Throws when the source's account has no counterpart in the target, which is the honest answer
+ * — deploying as the pusher instead would silently widen what the app runs as.
+ */
+export async function preservedIdentity(
+  deployProvider: DeployProvider,
+  kind: DeployKind,
+  path: string,
+  workspaceFrom: string,
+  workspaceTo: string,
+  usernames: Map<string, { username: string; email: string }>
+): Promise<string | undefined> {
+  if (kind !== "app" && kind !== "raw_app") {
+    return await getOnBehalfOf(deployProvider, kind, path, workspaceFrom);
+  }
+  const { lookupUsernameByEmail } = await import("../../core/permissioned_as.ts");
+  const policy = (
+    await deployProvider.getAppByPath({ workspace: workspaceFrom, path })
+  )?.policy as
+    | { on_behalf_of?: string; on_behalf_of_email?: string }
+    | undefined;
+  const principal = policy?.on_behalf_of;
+  if (!principal?.startsWith("u/") || !policy?.on_behalf_of_email) {
+    return principal;
+  }
+  return `u/${await lookupUsernameByEmail(
+    workspaceTo,
+    policy.on_behalf_of_email,
+    usernames
+  )}`;
+}
+
 /**
  * Apply kind-specific transforms before sending a trigger to create/update.
  * Currently only GCP needs special handling (subscription_id wipe + base_endpoint
@@ -542,6 +585,9 @@ async function mergeWorkspaces(
   }
   let successCount = 0;
   let failCount = 0;
+  // One `listUsers` of the target for the whole run, shared by every app that maps an identity
+  // into it (`preservedIdentity`).
+  const usernamesInTarget = new Map<string, { username: string; email: string }>();
   // Datatable migrations deployed (not deleted) into the target. Deploying a
   // migration only upserts its definition — the target schema is unchanged until
   // the migration is run — so offer to run them afterwards (like the push path).
@@ -571,23 +617,32 @@ async function mergeWorkspaces(
       );
     } else {
       let onBehalfOf: string | undefined;
+      let identityError: string | undefined;
       if (opts.preserveOnBehalfOf) {
-        onBehalfOf = await getOnBehalfOf(
-          provider,
-          diff.kind as DeployKind,
-          diff.path,
-          workspaceFrom
-        );
+        try {
+          onBehalfOf = await preservedIdentity(
+            provider,
+            diff.kind as DeployKind,
+            diff.path,
+            workspaceFrom,
+            workspaceTo,
+            usernamesInTarget
+          );
+        } catch (e: any) {
+          identityError = e?.message ?? String(e);
+        }
       }
 
-      result = await deployItem(
-        provider,
-        diff.kind as DeployKind,
-        diff.path,
-        workspaceFrom,
-        workspaceTo,
-        onBehalfOf
-      );
+      result = identityError
+        ? { success: false, error: identityError }
+        : await deployItem(
+            provider,
+            diff.kind as DeployKind,
+            diff.path,
+            workspaceFrom,
+            workspaceTo,
+            onBehalfOf
+          );
     }
 
     if (result.success) {

cli/windmill-utils-internal/src/deploy.ts

@@ -510,13 +510,14 @@ export async function deployItem(
         workspace: workspaceFrom,
         path,
       });
-      // The source policy's principal names nobody in the target, so it is always replaced:
-      // `getOnBehalfOf` read this one out of the *target*, where `u/alice` and `g/ops` mean
-      // what they say. Unlike the flow and script branches above, which still carry an
-      // address the backend resolves, the app's identity travels as the principal itself —
-      // a group has no address of its own to travel as. The address the read derived goes
-      // too: it is the source's, and a server old enough to still read it would take the
-      // pair as naming two accounts.
+      // Unlike the flow and script branches above, which carry an address for the backend to
+      // resolve, an app's identity travels as the principal itself — its policy stores nothing
+      // else, and a group has no address of its own to travel as. So the source policy's
+      // principal is always replaced by `onBehalfOf`, which **the caller must have made valid
+      // in the target**: `u/` names are per-workspace, so a caller reading the source has to map
+      // it (`preservedIdentity` in the CLI's merge does), and one reading the target can send it
+      // as is. The address the read derived goes too: it is the source's, and a server old
+      // enough to still read it would take the pair as naming two accounts.
       const app = {
         ...rawApp,
         policy: {

frontend/src/lib/components/CompareWorkspaces.svelte

@@ -49,6 +49,7 @@
 		diffRemovesInTarget,
 		getItemValue,
 		getOnBehalfOf,
+		deployItemIdentityIsPrincipal,
 		type DeployPermission,
 		type DeployResult
 	} from '$lib/utils_workspace_deploy'
@@ -56,7 +57,6 @@
 	import { isTriggerOrScheduleKind } from 'windmill-utils-internal'
 	import Tooltip from './Tooltip.svelte'
 	import OnBehalfOfSelector, {
-		deployItemIdentityIsPrincipal,
 		needsOnBehalfOfSelection,
 		type OnBehalfOfChoice,
 		type OnBehalfOfDetails

frontend/src/lib/components/DeployWorkspace.svelte

@@ -31,7 +31,8 @@
 		checkItemExists,
 		deployItem,
 		getItemValue,
-		getOnBehalfOf
+		getOnBehalfOf,
+		deployItemIdentityIsPrincipal
 	} from '$lib/utils_workspace_deploy'
 	import type { App } from './apps/types'
 	import { getAllGridItems } from './apps/editor/appUtils'
@@ -39,7 +40,6 @@
 	import type { Runnable } from './raw_apps/utils'
 	import WorkspaceDeployLayout from './WorkspaceDeployLayout.svelte'
 	import OnBehalfOfSelector, {
-		deployItemIdentityIsPrincipal,
 		needsOnBehalfOfSelection,
 		type OnBehalfOfChoice,
 		type OnBehalfOfDetails

frontend/src/lib/components/OnBehalfOfSelector.svelte

@@ -8,21 +8,6 @@
 		permissionedAs: string
 	}
 
-	/**
-	 * Whether a deploy carries this kind's identity as a principal (`u/alice`, `g/ops`) rather
-	 * than as an address. Apps sit with triggers and schedules: an app's policy stores only the
-	 * principal, and a group has no address of its own to travel as. Flows and scripts still send
-	 * an address, which the backend resolves against the target workspace.
-	 *
-	 * Both halves of a deploy read this — what `getOnBehalfOf` fetches out of the target and what
-	 * is sent back — so a kind moved between the two sides has to move in `deployItem` with it.
-	 */
-	export function deployItemIdentityIsPrincipal(kind: string): boolean {
-		return (
-			kind === 'app' || kind === 'raw_app' || kind === 'trigger' || isTriggerOrScheduleKind(kind)
-		)
-	}
-
 	/**
 	 * Check if an item needs on_behalf_of selection.
 	 * Shows the selector when the source item has an on_behalf_of value set.

frontend/src/lib/components/UpdateDevWorkspaceModal.svelte

@@ -12,12 +12,12 @@
 		createFolderIfAbsent,
 		deployItem,
 		getOnBehalfOfOrThrow,
+		deployItemIdentityIsPrincipal,
 		type DeployResult,
 		type DeployTargetAccess
 	} from '$lib/utils_workspace_deploy'
 	import { COMPARE_ITEMS_PARAM } from '$lib/components/sessions/modifiedItemsMask'
 	import OnBehalfOfSelector, {
-		deployItemIdentityIsPrincipal,
 		needsOnBehalfOfSelection,
 		type OnBehalfOfChoice,
 		type OnBehalfOfDetails

frontend/src/lib/utils_workspace_deploy.ts

@@ -42,6 +42,7 @@ import {
 	checkItemExists as sharedCheckItemExists,
 	getOnBehalfOf as sharedGetOnBehalfOf,
 	getItemValue as sharedGetItemValue,
+	isTriggerOrScheduleKind,
 	type DeployProvider,
 	type DeployKind,
 	type DeployResult,
@@ -173,6 +174,19 @@ function legacyTriggerKind(kind: TriggerDeployKind) {
 /** An identity in both formats a deploy may carry it in — see `deployItemIdentityIsPrincipal`. */
 export type AppIdentity = { email: string; permissionedAs: string }
 
+/**
+ * Whether a deploy carries this kind's identity as a principal (`u/alice`, `g/ops`) rather
+ * than as an address. Apps sit with triggers and schedules: an app's policy stores only the
+ * principal, and a group has no address of its own to travel as. Flows and scripts still send
+ * an address, which the backend resolves against the target workspace.
+ *
+ * Both halves of a deploy read this — what `getOnBehalfOf` fetches out of the target and what is
+ * sent back — so a kind moved between the two sides has to move in `deployItem` with it.
+ */
+export function deployItemIdentityIsPrincipal(kind: string): boolean {
+	return kind === 'app' || kind === 'raw_app' || kind === 'trigger' || isTriggerOrScheduleKind(kind)
+}
+
 /**
  * Set when a create-only deploy was refused because the target turned out to already have the
  * item. Carried on an object rather than matched out of the error text: `deployItem` swallows