diff --git a/backend/windmill-api-integration-tests/tests/drafts.rs b/backend/windmill-api-integration-tests/tests/drafts.rs index c46938d28c..72a44b4fa6 100644 --- a/backend/windmill-api-integration-tests/tests/drafts.rs +++ b/backend/windmill-api-integration-tests/tests/drafts.rs @@ -439,6 +439,34 @@ async fn test_transfer_drafts_to_fork(db: Pool) -> anyhow::Result<()> assert_eq!(fork_count("u/test-user/a").await?, 1); assert_eq!(fork_count("u/test-user/b").await?, 1); + // An app policy naming someone who is not a member of the fork is re-pointed at the user, as + // the fork's own clone does: a deploy in the fork rejects an identity that does not resolve. + let r = c + .post(save_url(port, "raw_app", "u/test-user/app")) + .json(&json!({ "value": { "policy": { + "on_behalf_of": "u/not-in-fork", "on_behalf_of_email": "ghost@windmill.dev" + } } })) + .send() + .await?; + assert_eq!(r.status(), 200, "{}", r.text().await?); + let r = c + .post(&transfer_url) + .json(&json!({ + "target_workspace": fork, + "items": [{ "kind": "raw_app", "path": "u/test-user/app" }] + })) + .send() + .await?; + assert_eq!(r.status(), 200, "{}", r.text().await?); + let on_behalf_of: String = sqlx::query_scalar( + "SELECT value->'policy'->>'on_behalf_of' FROM draft + WHERE workspace_id = $1 AND path = 'u/test-user/app'", + ) + .bind(fork) + .fetch_one(&db) + .await?; + assert_eq!(on_behalf_of, "u/test-user"); + // Only a fork of the source is a valid target. let r = c .post(format!( diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 26d147301d..aee50e14c9 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -8580,6 +8580,65 @@ fn principal_resolves_sql(principal: &str) -> String { ) } +/// The draft half of `repoint_unresolvable_cloned_identities`: an app policy or trigger in a draft +/// copied into `target_workspace_id` that names nobody there is re-pointed at `authed`. `items` +/// narrows it to `authed`'s own rows at those keys, for drafts copied into an existing workspace. +pub async fn repoint_unresolvable_draft_identities( + tx: &mut Transaction<'_, Postgres>, + target_workspace_id: &str, + authed: &ApiAuthed, + items: Option<&[(windmill_common::user_drafts::UserDraftItemKind, String)]>, +) -> Result<()> { + let principal = username_to_permissioned_as(&authed.username); + let kinds: Option> = items.map(|i| i.iter().map(|(k, _)| k.as_str()).collect()); + let paths: Option> = items.map(|i| i.iter().map(|(_, p)| p.as_str()).collect()); + + // A draft holding a genuine NUL escape (the rule of `json_text_has_nul_escape`) is left as it + // is, since parsing it would abort the fork. The check must stay in a CASE: json `->>` raises on + // a NUL anywhere in the value, and Postgres reorders plain AND conditions. + let nul_escape = r"(^|[^\\])(\\\\)*\\u0000"; + sqlx::query(&format!( + "UPDATE draft SET value = to_json(jsonb_set(jsonb_set(to_jsonb(value), + ARRAY['policy', 'on_behalf_of'], to_jsonb($2::text)), + ARRAY['policy', 'on_behalf_of_email'], to_jsonb($3::text))) + WHERE workspace_id = $1 AND typ IN ('app', 'raw_app') + AND ($5::text[] IS NULL OR (email = $7 + AND (typ::text, path) IN (SELECT * FROM unnest($5::text[], $6::text[])))) + AND CASE WHEN value::text ~ $4 THEN false + ELSE value->'policy'->>'on_behalf_of' IS NOT NULL AND NOT ({}) END", + principal_resolves_sql("(value->'policy'->>'on_behalf_of')") + )) + .bind(target_workspace_id) + .bind(&principal) + .bind(&authed.email) + .bind(nul_escape) + .bind(kinds.as_deref()) + .bind(paths.as_deref()) + .bind(&authed.email) + .execute(&mut **tx) + .await?; + + sqlx::query(&format!( + "UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), + ARRAY['permissioned_as'], to_jsonb($2::text))) + WHERE workspace_id = $1 AND starts_with(typ::text, 'trigger_') + AND ($4::text[] IS NULL OR (email = $6 + AND (typ::text, path) IN (SELECT * FROM unnest($4::text[], $5::text[])))) + AND CASE WHEN value::text ~ $3 THEN false + ELSE value->>'permissioned_as' IS NOT NULL AND NOT ({}) END", + principal_resolves_sql("(value->>'permissioned_as')") + )) + .bind(target_workspace_id) + .bind(&principal) + .bind(nul_escape) + .bind(kinds.as_deref()) + .bind(paths.as_deref()) + .bind(&authed.email) + .execute(&mut **tx) + .await?; + Ok(()) +} + /// Re-point the identities a fork clones verbatim at its creator when they name nobody in the fork, /// once its membership is final so copied members keep theirs. Unlike scripts and flows these /// cannot drop the identity: an app deploy rejects a preserved one that does not resolve, and @@ -8604,39 +8663,7 @@ async fn repoint_unresolvable_cloned_identities( .execute(&mut **tx) .await?; - // A draft holding a genuine NUL escape (the rule of `json_text_has_nul_escape`) is left as it - // is, since parsing it would abort the fork. The check must stay in a CASE: json `->>` raises on - // a NUL anywhere in the value, and Postgres reorders plain AND conditions. - let nul_escape = r"(^|[^\\])(\\\\)*\\u0000"; - sqlx::query(&format!( - "UPDATE draft SET value = to_json(jsonb_set(jsonb_set(to_jsonb(value), - ARRAY['policy', 'on_behalf_of'], to_jsonb($2::text)), - ARRAY['policy', 'on_behalf_of_email'], to_jsonb($3::text))) - WHERE workspace_id = $1 AND typ IN ('app', 'raw_app') - AND CASE WHEN value::text ~ $4 THEN false - ELSE value->'policy'->>'on_behalf_of' IS NOT NULL AND NOT ({}) END", - principal_resolves_sql("(value->'policy'->>'on_behalf_of')") - )) - .bind(target_workspace_id) - .bind(&principal) - .bind(&authed.email) - .bind(nul_escape) - .execute(&mut **tx) - .await?; - - sqlx::query(&format!( - "UPDATE draft SET value = to_json(jsonb_set(to_jsonb(value), - ARRAY['permissioned_as'], to_jsonb($2::text))) - WHERE workspace_id = $1 AND starts_with(typ::text, 'trigger_') - AND CASE WHEN value::text ~ $3 THEN false - ELSE value->>'permissioned_as' IS NOT NULL AND NOT ({}) END", - principal_resolves_sql("(value->>'permissioned_as')") - )) - .bind(target_workspace_id) - .bind(&principal) - .bind(nul_escape) - .execute(&mut **tx) - .await?; + repoint_unresolvable_draft_identities(tx, target_workspace_id, authed, None).await?; let column_resolves = principal_resolves_sql("permissioned_as"); diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index e2f8406bd4..b19ec1ef5e 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -1203,6 +1203,15 @@ async fn transfer_drafts( Some(&locked), ) .await?; + // A copied app policy or trigger may name someone who is not a member of the fork, as a + // fork's own clone may; fork creation re-points those at the user, and so does this. + windmill_api_workspaces::workspaces::repoint_unresolvable_draft_identities( + &mut tx, + target, + target_authed, + Some(&copied), + ) + .await?; } let mut removed = vec![];