fix: keep a replaced principal's address off the wire, and map by live membership

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

backend/migrations/20260912072924_drop_app_policy_on_behalf_of_email.up.sql

@@ -1,6 +1,14 @@
 -- The app policy's address is derived from its principal on every read and no longer written,
 -- so a stored copy is inert and, once an account is renamed or its address changes, wrong.
-UPDATE app SET policy = policy - 'on_behalf_of_email' WHERE policy ? 'on_behalf_of_email';
+--
+-- Only where there is a principal to derive from. `20260911085221` left `on_behalf_of` unset
+-- 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.
+UPDATE app SET policy = policy - 'on_behalf_of_email'
+WHERE policy ? 'on_behalf_of_email' AND policy ? 'on_behalf_of';
 
 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';
+WHERE typ IN ('app', 'raw_app')
+  AND to_jsonb(value) #> '{policy}' ? 'on_behalf_of_email'
+  AND to_jsonb(value) #> '{policy}' ? 'on_behalf_of';

backend/windmill-api/openapi.yaml

@@ -33950,7 +33950,7 @@ components:
         on_behalf_of_email:
           type: string
           readOnly: true
-          description: Address of `on_behalf_of`, derived from it on every read and never stored. Absent when the policy names no identity. Read-only - sending it is ignored, so a deploy that means to keep an identity has to name it in `on_behalf_of`.
+          description: Address of `on_behalf_of`, derived from it on every read and never stored. Absent when the policy names no identity. Read-only - sending it beside `on_behalf_of` is ignored, and sending it alone with `preserve_on_behalf_of` is rejected, so a deploy that means to keep an identity has to name it in `on_behalf_of`.
         sandbox:
           type: boolean
           description: >

cli/src/commands/workspace/merge.ts

@@ -188,14 +188,22 @@ function triggerService(kind: TriggerDeployKind) {
  *
  * 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.
+ * else — so a `u/` name, which is per-workspace, is re-read as the source member it names and
+ * looked up again in the target. That is the mapping the backend did for as long as the address
+ * itself travelled; without it the app lands on whoever holds that username in the target, or on
+ * nobody.
  *
- * 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.
+ * 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.
+ *
+ * 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.
  */
 export async function preservedIdentity(
   deployProvider: DeployProvider,
@@ -203,25 +211,34 @@ export async function preservedIdentity(
   path: string,
   workspaceFrom: string,
   workspaceTo: string,
-  usernames: Map<string, { username: string; email: string }>
+  usernamesInSource: Map<string, { username: string; email: string }>,
+  usernamesInTarget: 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 { lookupEmailByUsername, 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;
+  )?.policy as { on_behalf_of?: string } | undefined;
   const principal = policy?.on_behalf_of;
-  if (!principal?.startsWith("u/") || !policy?.on_behalf_of_email) {
+  if (!principal?.startsWith("u/")) {
+    return principal;
+  }
+  const email = await lookupEmailByUsername(
+    workspaceFrom,
+    principal.slice(2),
+    usernamesInSource
+  );
+  if (!email) {
     return principal;
   }
   return `u/${await lookupUsernameByEmail(
     workspaceTo,
-    policy.on_behalf_of_email,
-    usernames
+    email,
+    usernamesInTarget
   )}`;
 }
 
@@ -585,8 +602,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`).
+  // One `listUsers` per workspace for the whole run, shared by every app that maps an identity
+  // across (`preservedIdentity`).
+  const usernamesInSource = new Map<string, { username: string; email: string }>();
   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
@@ -626,6 +644,7 @@ async function mergeWorkspaces(
             diff.path,
             workspaceFrom,
             workspaceTo,
+            usernamesInSource,
             usernamesInTarget
           );
         } catch (e: any) {

cli/src/core/permissioned_as.ts

@@ -58,6 +58,23 @@ async function ensureUserCache(
   }
 }
 
+/**
+ * The address a workspace member's username belongs to, or `undefined` when the workspace has no
+ * such member. Absence is an answer, not a failure: a principal naming no `usr` row is an
+ * instance-level identity (a superadmin acting outside their workspaces), which is not
+ * workspace-scoped and so means the same thing in every workspace.
+ *
+ * Reads `usr` through `listUsers`, so it is never the cached address a derived read would give.
+ */
+export async function lookupEmailByUsername(
+  workspace: string,
+  username: string,
+  cache: Map<string, { username: string; email: string }>
+): Promise<string | undefined> {
+  await ensureUserCache(workspace, cache);
+  return cache.get(username)?.email;
+}
+
 export async function lookupUsernameByEmail(
   workspace: string,
   email: string,

cli/test/preserved_identity_unit.test.ts

@@ -0,0 +1,83 @@
+import { expect, test } from "bun:test";
+import { preservedIdentity } from "../src/commands/workspace/merge.ts";
+
+// A merge reads an app's identity out of the source workspace, but `u/` names are per-workspace,
+// so what it sends has to be the target's name for the same account. Getting this wrong hands the
+// app to whoever holds that username in the target, silently — so pin each branch.
+//
+// Both caches are pre-seeded, which is also what keeps this off the network: `ensureUserCache`
+// short-circuits on a non-empty map.
+function seeded(...users: { username: string; email: string }[]) {
+  const cache = new Map<string, { username: string; email: string }>();
+  for (const u of users) {
+    cache.set(u.username, u);
+    cache.set(u.email, u);
+  }
+  return cache;
+}
+
+function providerFor(on_behalf_of: string | undefined) {
+  return { getAppByPath: async () => ({ policy: { on_behalf_of } }) } as any;
+}
+
+const source = () => seeded({ username: "alice", email: "alice@corp" });
+const target = () => seeded({ username: "alice_dst", email: "alice@corp" });
+
+test("preservedIdentity: a member's principal is remapped to the target's name for them", async () => {
+  expect(
+    await preservedIdentity(
+      providerFor("u/alice"),
+      "app" as any,
+      "f/x/a",
+      "src",
+      "dst",
+      source(),
+      target(),
+    ),
+  ).toBe("u/alice_dst");
+});
+
+test("preservedIdentity: a group carries over by name", async () => {
+  expect(
+    await preservedIdentity(
+      providerFor("g/ops"),
+      "app" as any,
+      "f/x/a",
+      "src",
+      "dst",
+      source(),
+      target(),
+    ),
+  ).toBe("g/ops");
+});
+
+// An instance superadmin has no `usr` row in either workspace, so there is nothing to map — and
+// nothing to map *to*: the backend resolves that principal through the instance-wide `password`
+// fallback, which means the same account in every workspace.
+test("preservedIdentity: an identity with no member behind it carries over untouched", async () => {
+  expect(
+    await preservedIdentity(
+      providerFor("u/superadmin-external"),
+      "app" as any,
+      "f/x/a",
+      "src",
+      "dst",
+      source(),
+      target(),
+    ),
+  ).toBe("u/superadmin-external");
+});
+
+test("preservedIdentity: a member absent from the target fails rather than falling back", async () => {
+  expect(
+    preservedIdentity(
+      providerFor("u/alice"),
+      "app" as any,
+      "f/x/a",
+      "src",
+      "dst",
+      source(),
+      seeded({ username: "bob", email: "bob@corp" }),
+    ),
+  ).rejects.toThrow(/Could not find username for email/);
+});

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

@@ -318,6 +318,11 @@
 				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 = ''