fix: align the app backfill with the sibling migration and audit the uncached address

This commit is contained in:
Ruben Fiszel
2026-08-04 06:31:56 +00:00
parent e23c35898d
commit 50e4261479
4 changed files with 35 additions and 25 deletions
@@ -15,7 +15,7 @@
]
},
"nullable": [
null
true
]
},
"hash": "5a219a2532517869578c4504ff3153c43903f929ae5d62fbba12610f89c36d55"
@@ -18,12 +18,16 @@
CREATE OR REPLACE FUNCTION pg_temp.permissioned_as_from_email(w_id VARCHAR, email VARCHAR)
RETURNS VARCHAR AS $$
SELECT COALESCE(
-- `username_to_permissioned_as`: an email-shaped username is stored verbatim.
(SELECT CASE WHEN u.username LIKE '%@%' THEN u.username ELSE 'u/' || u.username END
-- `username_to_permissioned_as`: an email-shaped username is its own principal unless
-- it contains a slash, which a reader would split on.
(SELECT CASE WHEN u.username LIKE '%@%' AND u.username NOT LIKE '%/%' THEN u.username
ELSE 'u/' || u.username END
FROM usr u WHERE u.workspace_id = $1 AND u.email = $2),
-- A superadmin acting outside their workspaces has no usr row.
(SELECT CASE WHEN COALESCE(p.username, p.email) LIKE '%@%' THEN COALESCE(p.username, p.email)
ELSE 'u/' || p.username END
(SELECT CASE WHEN COALESCE(p.username, p.email) LIKE '%@%'
AND COALESCE(p.username, p.email) NOT LIKE '%/%'
THEN COALESCE(p.username, p.email)
ELSE 'u/' || COALESCE(p.username, p.email) END
FROM password p WHERE p.email = $2 AND p.super_admin),
(SELECT 'g/' || g.name FROM group_ g
WHERE g.workspace_id = $1
@@ -32,16 +36,21 @@ RETURNS VARCHAR AS $$
$$ LANGUAGE SQL STABLE;
-- A policy naming only the address predates the principal being written to it.
-- A principal wider than `v2_job.permissioned_as` could not be enqueued, so it is not recorded
-- at all — the app falls back to erroring on anonymous execution until someone picks an identity
-- the deploy path accepts. Same cap and reason as the sibling migration 20260801043001.
UPDATE app SET policy = jsonb_set(policy, ARRAY['on_behalf_of'],
to_jsonb(pg_temp.permissioned_as_from_email(workspace_id, policy->>'on_behalf_of_email')))
WHERE policy->>'on_behalf_of' IS NULL
AND pg_temp.permissioned_as_from_email(workspace_id, policy->>'on_behalf_of_email') IS NOT NULL;
AND pg_temp.permissioned_as_from_email(workspace_id, policy->>'on_behalf_of_email') IS NOT NULL
AND length(pg_temp.permissioned_as_from_email(workspace_id, policy->>'on_behalf_of_email')) <= 55;
-- App drafts carry a copy of the policy and are deployed from it, so they need the same.
UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), ARRAY['policy', 'on_behalf_of'],
to_jsonb(pg_temp.permissioned_as_from_email(workspace_id, value->'policy'->>'on_behalf_of_email'))))
WHERE typ IN ('app', 'raw_app')
AND value->'policy'->>'on_behalf_of' IS NULL
AND pg_temp.permissioned_as_from_email(workspace_id, value->'policy'->>'on_behalf_of_email') IS NOT NULL;
AND pg_temp.permissioned_as_from_email(workspace_id, value->'policy'->>'on_behalf_of_email') IS NOT NULL
AND length(pg_temp.permissioned_as_from_email(workspace_id, value->'policy'->>'on_behalf_of_email')) <= 55;
DROP FUNCTION pg_temp.permissioned_as_from_email(VARCHAR, VARCHAR);
+16 -14
View File
@@ -2189,7 +2189,6 @@ async fn create_app_internal<'a>(
&db,
)
.await?;
preserved_on_behalf_of = audited_on_behalf_of(&app.policy, &authed, w_id, &db).await?;
} else {
let folder_default = if windmill_common::can_preserve_on_behalf_of(&authed) {
windmill_common::folders::resolve_folder_default_permissioned_as(&db, w_id, &app.path)
@@ -2201,6 +2200,9 @@ async fn create_app_internal<'a>(
Some(folder_default.unwrap_or_else(|| username_to_permissioned_as(&authed.username)));
}
app.policy.on_behalf_of_email = stored_on_behalf_of_email(&app.policy, w_id, &db).await?;
if should_preserve {
preserved_on_behalf_of = audited_on_behalf_of(&app.policy, &authed);
}
let mut tx = user_db.clone().begin(&authed).await?;
let path = app.path.clone();
@@ -2765,11 +2767,13 @@ async fn update_app_internal<'a>(
&db,
)
.await?;
preserved_on_behalf_of = audited_on_behalf_of(npolicy, &authed, w_id, &db).await?;
} else {
npolicy.on_behalf_of = Some(username_to_permissioned_as(&authed.username));
}
npolicy.on_behalf_of_email = stored_on_behalf_of_email(npolicy, w_id, &db).await?;
if should_preserve {
preserved_on_behalf_of = audited_on_behalf_of(npolicy, &authed);
}
}
let mut tx = user_db.clone().begin(&authed).await?;
@@ -4725,18 +4729,16 @@ async fn stored_on_behalf_of_email(
/// The address to record in the `apps.on_behalf_of` audit entry: the one the app will run as,
/// when it is not the deployer's own. `None` when they match — a deployer handing an app their
/// own identity is not an on-behalf-of deploy.
async fn audited_on_behalf_of(
policy: &Policy,
authed: &ApiAuthed,
w_id: &str,
db: &DB,
) -> Result<Option<String>> {
let Some(permissioned_as) = policy.on_behalf_of.as_deref() else {
return Ok(None);
};
let email =
windmill_common::users::get_email_from_permissioned_as(permissioned_as, w_id, db).await?;
Ok((email != authed.email).then_some(email))
///
/// Reads the address `stored_on_behalf_of_email` just resolved rather than looking it up again:
/// that one is uncached, and a second cached read could write a since-changed address into a
/// durable audit row.
fn audited_on_behalf_of(policy: &Policy, authed: &ApiAuthed) -> Option<String> {
policy
.on_behalf_of_email
.as_deref()
.filter(|email| *email != authed.email)
.map(str::to_string)
}
async fn get_on_behalf_of(policy: &Policy, w_id: &str, db: &DB) -> Result<(String, String)> {
+3 -4
View File
@@ -96,13 +96,12 @@ test("deployItem: never sends the source workspace's on_behalf_of", async () =>
]);
for (const [name, body] of captured) {
expect(body.preserve_on_behalf_of).toBe(true);
// An app carries its identity inside the policy; the others carry it at the top level.
// Both surfaces spell it `on_behalf_of`; only its nesting differs — an app carries the
// identity inside its policy, the others at the top level.
const identity = name === "createApp" ? body.policy : body;
// Both surfaces spell it the same; only its nesting differs.
const principalKey = "on_behalf_of";
// The email is still overridden with the caller's choice...
expect(identity.on_behalf_of_email).toBe("alice@corp");
// ...while the principal is dropped, so the backend derives the target's own.
expect(principalKey in JSON.parse(JSON.stringify(identity))).toBe(false);
expect("on_behalf_of" in JSON.parse(JSON.stringify(identity))).toBe(false);
}
});