fix: shape the deploy request instead of the policy the editor renders

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

backend/.sqlx/query-5f1842cd56d68292854c938b24cebfdd57f2f412a8e33f4ac62d15ced48dc7bf.json

@@ -0,0 +1,14 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "UPDATE draft SET value = to_json(to_jsonb(value) #- '{policy,on_behalf_of_email}') WHERE typ IN ('app', 'raw_app') AND value->'policy'->>'on_behalf_of_email' = $1",
+  "describe": {
+    "columns": [],
+    "parameters": {
+      "Left": [
+        "Text"
+      ]
+    },
+    "nullable": []
+  },
+  "hash": "5f1842cd56d68292854c938b24cebfdd57f2f412a8e33f4ac62d15ced48dc7bf"
+}

backend/.sqlx/query-61caaa5c4ae62f618ba18f36330bbf58ff58e8c448874ffc9ca466165e545878.json

@@ -0,0 +1,15 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "DELETE FROM ws_specific\n         WHERE workspace_id = $1 AND item_kind = 'variable' AND path = ANY($2)",
+  "describe": {
+    "columns": [],
+    "parameters": {
+      "Left": [
+        "Text",
+        "TextArray"
+      ]
+    },
+    "nullable": []
+  },
+  "hash": "61caaa5c4ae62f618ba18f36330bbf58ff58e8c448874ffc9ca466165e545878"
+}

backend/.sqlx/query-a7b6731427d51eb34744ca6a39e30d02949f149901ec95323b5725911944df4d.json

@@ -0,0 +1,24 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "WITH survivors AS (\n             SELECT value::text AS rendered FROM resource\n             WHERE workspace_id = $1 AND NOT (path = ANY($2::text[]))\n         )\n         SELECT v.path FROM unnest($3::text[]) AS v(path)\n         WHERE EXISTS (\n             SELECT 1 FROM survivors s\n             WHERE strpos(s.rendered, '\"$var:' || v.path || '\"') > 0\n                OR strpos(s.rendered, '\"$jsonvar:' || v.path || '\"') > 0\n         )",
+  "describe": {
+    "columns": [
+      {
+        "ordinal": 0,
+        "name": "path",
+        "type_info": "Text"
+      }
+    ],
+    "parameters": {
+      "Left": [
+        "Text",
+        "TextArray",
+        "TextArray"
+      ]
+    },
+    "nullable": [
+      null
+    ]
+  },
+  "hash": "a7b6731427d51eb34744ca6a39e30d02949f149901ec95323b5725911944df4d"
+}

backend/.sqlx/query-d1d8b0f8a1e88a04056c245a6f28971a3f9fe209f8092f400c660814f6ee61bd.json

@@ -0,0 +1,14 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "UPDATE app SET policy = policy - 'on_behalf_of_email' WHERE policy->>'on_behalf_of_email' = $1",
+  "describe": {
+    "columns": [],
+    "parameters": {
+      "Left": [
+        "Text"
+      ]
+    },
+    "nullable": []
+  },
+  "hash": "d1d8b0f8a1e88a04056c245a6f28971a3f9fe209f8092f400c660814f6ee61bd"
+}

backend/.sqlx/query-d88b1c445acc5375e9c543dcec81059d0e7018a6cea79ba743693098d223edf5.json

@@ -0,0 +1,23 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "DELETE FROM variable WHERE workspace_id = $1 AND path = ANY($2) RETURNING path",
+  "describe": {
+    "columns": [
+      {
+        "ordinal": 0,
+        "name": "path",
+        "type_info": "Varchar"
+      }
+    ],
+    "parameters": {
+      "Left": [
+        "Text",
+        "TextArray"
+      ]
+    },
+    "nullable": [
+      false
+    ]
+  },
+  "hash": "d88b1c445acc5375e9c543dcec81059d0e7018a6cea79ba743693098d223edf5"
+}

backend/migrations/20260912072924_drop_app_policy_on_behalf_of_email.up.sql

@@ -5,10 +5,13 @@
 -- wherever the address named nobody or resolved to something too wide to enqueue; for those rows
 -- the address is the sole surviving record of the intended identity and nothing can recompute it.
 -- They already fail `get_on_behalf_of` on the missing principal, so keeping it costs nothing.
+--
+-- Tested through `->>` rather than `?`: a policy `Policy` wrote serializes an absent principal as
+-- an explicit JSON null, which `?` counts as present.
 UPDATE app SET policy = policy - 'on_behalf_of_email'
-WHERE policy ? 'on_behalf_of_email' AND policy ? 'on_behalf_of';
+WHERE policy ? 'on_behalf_of_email' AND policy->>'on_behalf_of' IS NOT NULL;
 
 UPDATE draft SET value = to_json(to_jsonb(value) #- '{policy,on_behalf_of_email}')
 WHERE typ IN ('app', 'raw_app')
   AND to_jsonb(value) #> '{policy}' ? 'on_behalf_of_email'
-  AND to_jsonb(value) #> '{policy}' ? 'on_behalf_of';
+  AND value->'policy'->>'on_behalf_of' IS NOT NULL;

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

@@ -626,6 +626,15 @@ async fn test_change_user_email(db: Pool<Postgres>) -> anyhow::Result<()> {
     )
     .execute(&db)
     .await?;
+    // The shape a peer still on the release before this one re-adds on every save: it prefers
+    // that address on reads, so one left behind would answer with the address this change moves
+    // away from for as long as that peer is live.
+    sqlx::query!(
+        "INSERT INTO app(workspace_id, path, summary, policy, versions)
+         VALUES ('test-workspace', 'u/test-user-2/legacy', '', '{\"on_behalf_of\": \"u/test-user-2\", \"on_behalf_of_email\": \"test2@windmill.dev\"}'::jsonb, '{}')"
+    )
+    .execute(&db)
+    .await?;
     sqlx::query!(
         "INSERT INTO folder(workspace_id, name, display_name, owners, extra_perms, default_permissioned_as)
          VALUES ('test-workspace', 'fold', 'fold', '{}', '{}'::jsonb,
@@ -679,6 +688,17 @@ async fn test_change_user_email(db: Pool<Postgres>) -> anyhow::Result<()> {
         "app policy should carry only the new address: {policy}"
     );
 
+    let legacy = sqlx::query_scalar!(
+        "SELECT policy::text FROM app WHERE path = 'u/test-user-2/legacy' AND workspace_id = 'test-workspace'"
+    )
+    .fetch_one(&db)
+    .await?
+    .unwrap_or_default();
+    assert!(
+        !legacy.contains("on_behalf_of_email"),
+        "an address a peer re-added is dropped, not left naming the old account: {legacy}"
+    );
+
     // The rules keep their order, since the folder resolver takes the first glob that matches.
     let rules = sqlx::query_scalar!(
         "SELECT default_permissioned_as::text FROM folder WHERE workspace_id = 'test-workspace' AND name = 'fold'"

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

@@ -2062,6 +2062,25 @@ async fn change_user_email(
     .execute(&mut *tx)
     .await?;
 
+    // An app policy carries no address, so there is none to move — but a peer still running the
+    // release before this one writes it on every save and prefers it on reads, so one re-added
+    // after the migration would answer with the address this change is moving away from, for as
+    // long as that peer is live. Dropped rather than rewritten: the key has no place here, and
+    // every reader that matters derives it.
+    sqlx::query!(
+        "UPDATE app SET policy = policy - 'on_behalf_of_email' WHERE policy->>'on_behalf_of_email' = $1",
+        &old_email
+    )
+    .execute(&mut *tx)
+    .await?;
+
+    sqlx::query!(
+        r#"UPDATE draft SET value = to_json(to_jsonb(value) #- '{policy,on_behalf_of_email}') WHERE typ IN ('app', 'raw_app') AND value->'policy'->>'on_behalf_of_email' = $1"#,
+        &old_email
+    )
+    .execute(&mut *tx)
+    .await?;
+
     // ---- permissioned_as naming the account by its address ----
     // `usr.username` is constrained to `[\w-]+`, so a workspace member is always named
     // `u/{username}` and their principals survive an address change untouched. The address form

cli/src/commands/workspace/merge.ts

@@ -193,17 +193,19 @@ function triggerService(kind: TriggerDeployKind) {
  * itself travelled; without it the app lands on whoever holds that username in the target, or on
  * nobody.
  *
- * Two principals are *not* workspace-scoped and carry over untouched: `g/ops` means the target's
- * ops group, which is the identity a fork and its parent are meant to share, and a `u/` name with
- * no member behind it is an instance-level superadmin, valid in every workspace through the same
- * `password` fallback the backend resolves it by.
+ * `g/ops` carries over by name: it means the target's ops group, which is the identity a fork and
+ * its parent are meant to share. So does a `u/` name with no member behind it in *either*
+ * workspace — that is an instance-level superadmin, which the backend resolves through the same
+ * `password` fallback everywhere. It has to be absent from both: the target's own `usr` row wins
+ * over that fallback, so carrying the name into a workspace that has a member of its own by that
+ * name would hand the app to them.
  *
  * The member's address comes from `usr` via `listUsers`, never from the app read's derived
  * `on_behalf_of_email`: that one is served from a cache that may be a notify poll stale, and this
  * answer is about to be persisted as the target's identity.
  *
- * Throws when a source member has no counterpart in the target, which is the honest answer —
- * deploying as the pusher instead would silently widen what the app runs as.
+ * Throws rather than guessing whenever the two workspaces disagree about who a name belongs to.
+ * Deploying as the pusher instead would silently widen what the app runs as.
  */
 export async function preservedIdentity(
   deployProvider: DeployProvider,
@@ -227,19 +229,27 @@ export async function preservedIdentity(
   if (!principal?.startsWith("u/")) {
     return principal;
   }
+  const username = principal.slice(2);
   const email = await lookupEmailByUsername(
     workspaceFrom,
-    principal.slice(2),
+    username,
     usernamesInSource
   );
-  if (!email) {
-    return principal;
+  if (email) {
+    return `u/${await lookupUsernameByEmail(
+      workspaceTo,
+      email,
+      usernamesInTarget
+    )}`;
   }
-  return `u/${await lookupUsernameByEmail(
-    workspaceTo,
-    email,
-    usernamesInTarget
-  )}`;
+  if (await lookupEmailByUsername(workspaceTo, username, usernamesInTarget)) {
+    throw new Error(
+      `'${principal}' names no member of ${workspaceFrom} but does name one of ${workspaceTo}, ` +
+        `so it cannot be carried across: deploying it would run the app as that account. ` +
+        `Set the app's identity in ${workspaceTo} explicitly.`
+    );
+  }
+  return principal;
 }
 
 /**

cli/test/preserved_identity_unit.test.ts

@@ -68,6 +68,22 @@ test("preservedIdentity: an identity with no member behind it carries over untou
   ).toBe("u/superadmin-external");
 });
 
+// ...but only while the target has no member of that name. `resolve_username_to_email` prefers a
+// `usr` row over the `password` fallback, so carrying it across would run the app as that member.
+test("preservedIdentity: a name the target has a member for is refused, not carried", async () => {
+  expect(
+    preservedIdentity(
+      providerFor("u/saext"),
+      "app" as any,
+      "f/x/a",
+      "src",
+      "dst",
+      source(),
+      seeded({ username: "saext", email: "someone-else@corp" }),
+    ),
+  ).rejects.toThrow(/cannot be carried across/);
+});
+
 test("preservedIdentity: a member absent from the target fails rather than falling back", async () => {
   expect(
     preservedIdentity(

frontend/src/lib/components/apps/editor/AppEditorHeader.svelte

@@ -64,7 +64,7 @@
 	import type { DiffDrawerI } from '$lib/components/diff_drawer'
 	import AppEditorHeaderDeploy from './AppEditorHeaderDeploy.svelte'
 	import { computeSecretUrl } from './appDeploy.svelte'
-	import { updatePolicy } from './appPolicy'
+	import { policyForDeploy, updatePolicy } from './appPolicy'
 	import { editInForkAllowed, editInForkLabel, openEditInFork } from '$lib/utils/editInFork'
 	import { isCloudHosted } from '$lib/cloud'
 
@@ -253,7 +253,7 @@
 					value: $app,
 					path,
 					summary: $summary,
-					policy,
+					policy: policyForDeploy(policy),
 					deployment_message: deploymentMsg,
 					custom_path: customPath,
 					preserve_on_behalf_of: preserveOnBehalfOf || undefined,
@@ -357,7 +357,7 @@
 			requestBody: {
 				value: $app!,
 				summary: $summary,
-				policy,
+				policy: policyForDeploy(policy),
 				path: npath,
 				deployment_message: deploymentMsg,
 				// custom_path requires admin so to accept update without it, we need to send as undefined when non-admin (when undefined, it will be ignored)
@@ -410,7 +410,7 @@
 		await AppService.updateApp({
 			workspace: $workspaceStore!,
 			path: $appPath,
-			requestBody: { policy }
+			requestBody: { policy: policyForDeploy(policy) }
 		})
 		if (message) {
 			sendUserToast(message)

frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte

@@ -318,11 +318,6 @@
 				selected={onBehalfOfChoice}
 				onSelect={(choice, details) => {
 					onBehalfOfChoice = choice
-					// `policy` came from a read, which derives the address from the principal being
-					// replaced here. It is inert on this release, but a peer still on the one before
-					// reads the pair and refuses it as naming two accounts, so the deploy fails outright
-					// mid-rollover. Every other writer of this policy clears it for the same reason.
-					delete policy.on_behalf_of_email
 					if (choice === 'me') {
 						policy.on_behalf_of = `u/${$userStore?.username}`
 						customOnBehalfOfEmail = ''

frontend/src/lib/components/apps/editor/appPolicy.ts

@@ -189,6 +189,18 @@ export async function updatePolicy(app: App, currentPolicy: Policy | undefined):
 	return next
 }
 
+/**
+ * The policy as a deploy should send it.
+ *
+ * A read derives `on_behalf_of_email` from the principal, and the editor keeps it to show who the
+ * app runs as — but sending it back beside a principal the user has just changed reads, to a peer
+ * that still stores the address, as a pair naming two accounts, and the deploy is refused. Built
+ * as a copy so the object the editor renders from keeps the address it displays.
+ */
+export function policyForDeploy<T extends Record<string, any>>(policy: T): T {
+	return { ...policy, on_behalf_of_email: undefined }
+}
+
 export async function processRunnable(
 	id: string,
 	runnable: Runnable,

frontend/src/lib/rawAppDeploy.ts

@@ -23,6 +23,7 @@ import {
 	normalizeRawAppData
 } from '$lib/components/raw_apps/rawAppDraftValue'
 import { stateSnapshot } from '$lib/svelte5Utils.svelte'
+import { policyForDeploy } from '$lib/components/apps/editor/appPolicy'
 
 /**
  * Deploy an explicit raw-app value — one the user edited as JSON, or one
@@ -73,7 +74,7 @@ export async function deployRawAppValue({
 				// version can still carry the pre-`data` datatable shapes.
 				value: { files, runnables, data: normalizeRawAppData(plainValue) },
 				summary: summary ?? '',
-				policy,
+				policy: policyForDeploy(policy),
 				deployment_message: deploymentMessage,
 				// custom_path changes require admin (see deployRawAppDraft).
 				custom_path: isAdmin ? (customPath ?? '') : undefined,
@@ -140,7 +141,7 @@ export async function deployRawAppDraft(
 					path: targetPath,
 					value: rawAppValue,
 					summary,
-					policy,
+					policy: policyForDeploy(policy),
 					deployment_message: deploymentMessage,
 					custom_path: isAdmin ? (value.custom_path ?? '') : undefined,
 					// Preserve the policy's on_behalf_of: this draft-deploy path has no
@@ -160,7 +161,7 @@ export async function deployRawAppDraft(
 					path: targetPath,
 					value: rawAppValue,
 					summary,
-					policy,
+					policy: policyForDeploy(policy),
 					deployment_message: deploymentMessage,
 					custom_path: value.custom_path,
 					// Preserve the policy's on_behalf_of (see update branch above).