mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-28 08:02:43 +00:00
fix: scope cloned app policy and custom path to the fork's creator
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
0459dda1bf
commit
c16ff9472d
+22
@@ -0,0 +1,22 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "INSERT INTO app (workspace_id, path, summary, policy, versions, custom_path)\n VALUES ('test-workspace', 'u/test-user/pub', '', $1, '{}', 'pub-path')\n RETURNING id",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "id",
|
||||
"type_info": "Int8"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Jsonb"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "10cc330ff3f839a55faeffded3d30629a82f8d2150be91cff20ad7e400e0135f"
|
||||
}
|
||||
+26
@@ -0,0 +1,26 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT policy, custom_path FROM app WHERE workspace_id = 'wm-fork-app'",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "policy",
|
||||
"type_info": "Jsonb"
|
||||
},
|
||||
{
|
||||
"ordinal": 1,
|
||||
"name": "custom_path",
|
||||
"type_info": "Text"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": []
|
||||
},
|
||||
"nullable": [
|
||||
false,
|
||||
true
|
||||
]
|
||||
},
|
||||
"hash": "e9ba32def4f06ee51b89819951a8d556534d74b06570bb16be7a2f4530451651"
|
||||
}
|
||||
+14
@@ -0,0 +1,14 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "WITH v AS (\n INSERT INTO app_version (app_id, value, created_by)\n VALUES ($1, '{}'::json, 'test-user') RETURNING id\n )\n UPDATE app SET versions = ARRAY[v.id] FROM v WHERE app.id = $1",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int8"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "fc115fe14c69b9dd7e7571aea6d918bc042b58714ba5d66568770e7f11153ece"
|
||||
}
|
||||
@@ -3,6 +3,97 @@ use sqlx::{Pool, Postgres};
|
||||
|
||||
use windmill_test_utils::*;
|
||||
|
||||
/// Seed an anonymous public app owned by the parent's admin, then fork as `token`. Returns the
|
||||
/// cloned app's policy and custom path.
|
||||
async fn fork_with_public_app(
|
||||
db: &Pool<Postgres>,
|
||||
token: &str,
|
||||
) -> anyhow::Result<(serde_json::Value, Option<String>)> {
|
||||
initialize_tracing().await;
|
||||
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let base_url = format!("http://localhost:{}/api", server.addr.port());
|
||||
|
||||
let app_id = sqlx::query_scalar!(
|
||||
"INSERT INTO app (workspace_id, path, summary, policy, versions, custom_path)
|
||||
VALUES ('test-workspace', 'u/test-user/pub', '', $1, '{}', 'pub-path')
|
||||
RETURNING id",
|
||||
json!({
|
||||
"on_behalf_of": "u/test-user",
|
||||
"on_behalf_of_email": "test@windmill.dev",
|
||||
"execution_mode": "anonymous",
|
||||
})
|
||||
)
|
||||
.fetch_one(db)
|
||||
.await?;
|
||||
// The clone re-aggregates `versions` from `app_version`, so an app without one lands in the
|
||||
// fork with a NULL array.
|
||||
sqlx::query!(
|
||||
"WITH v AS (
|
||||
INSERT INTO app_version (app_id, value, created_by)
|
||||
VALUES ($1, '{}'::json, 'test-user') RETURNING id
|
||||
)
|
||||
UPDATE app SET versions = ARRAY[v.id] FROM v WHERE app.id = $1",
|
||||
app_id
|
||||
)
|
||||
.execute(db)
|
||||
.await?;
|
||||
|
||||
let resp = reqwest::Client::new()
|
||||
.post(format!(
|
||||
"{base_url}/w/test-workspace/workspaces/create_fork"
|
||||
))
|
||||
.header("Authorization", format!("Bearer {token}"))
|
||||
.json(&json!({ "id": "wm-fork-app", "name": "Fork", "color": "#0000ff" }))
|
||||
.send()
|
||||
.await?;
|
||||
assert!(
|
||||
resp.status().is_success(),
|
||||
"creating the fork: {}",
|
||||
resp.text().await?
|
||||
);
|
||||
|
||||
let cloned =
|
||||
sqlx::query!("SELECT policy, custom_path FROM app WHERE workspace_id = 'wm-fork-app'")
|
||||
.fetch_one(db)
|
||||
.await?;
|
||||
Ok((cloned.policy, cloned.custom_path))
|
||||
}
|
||||
|
||||
/// An app policy's `on_behalf_of` is the identity anonymous and publisher executions queue jobs
|
||||
/// under, and the fork's endpoint outlives any revocation in the parent — so a creator who may
|
||||
/// not preserve someone else's identity must not receive one by forking. `test-user-2` is a
|
||||
/// plain member of the parent.
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base"))]
|
||||
async fn test_fork_downgrades_app_policy_for_unprivileged_creator(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
let (policy, custom_path) = fork_with_public_app(&db, "SECRET_TOKEN_2").await?;
|
||||
|
||||
assert_eq!(policy["on_behalf_of"], json!("u/test-user-2"));
|
||||
assert_eq!(policy["on_behalf_of_email"], json!("test2@windmill.dev"));
|
||||
assert_eq!(policy["execution_mode"], json!("publisher"));
|
||||
assert_eq!(custom_path, None);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// An admin could have set any of this through the app API, so their fork keeps the policy — which
|
||||
/// is also what keeps dev workspaces, always admin-created, behaving like their parent. The custom
|
||||
/// path still goes: it is the instance-wide address of the parent's live public app, and two rows
|
||||
/// claiming it make it resolve to either one.
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base"))]
|
||||
async fn test_fork_keeps_app_policy_for_admin_creator(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
let (policy, custom_path) = fork_with_public_app(&db, "SECRET_TOKEN").await?;
|
||||
|
||||
assert_eq!(policy["on_behalf_of"], json!("u/test-user"));
|
||||
assert_eq!(policy["on_behalf_of_email"], json!("test@windmill.dev"));
|
||||
assert_eq!(policy["execution_mode"], json!("anonymous"));
|
||||
assert_eq!(custom_path, None);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A principal only means something in the workspace whose `usr`/`group_` rows define it, and a
|
||||
/// fork copies the creator and the groups but not the rest of the membership. Carrying one over
|
||||
/// blindly would leave a runnable naming somebody who cannot authenticate there; dropping them
|
||||
@@ -106,11 +197,9 @@ async fn test_fork_keeps_only_resolvable_on_behalf_of(db: Pool<Postgres>) -> any
|
||||
.count();
|
||||
assert_eq!(orphaned, 0, "a dropped principal leaves no address behind");
|
||||
assert_eq!(
|
||||
sqlx::query_scalar!(
|
||||
"SELECT on_behalf_of FROM flow WHERE workspace_id = 'wm-fork-obo'"
|
||||
)
|
||||
.fetch_one(&db)
|
||||
.await?,
|
||||
sqlx::query_scalar!("SELECT on_behalf_of FROM flow WHERE workspace_id = 'wm-fork-obo'")
|
||||
.fetch_one(&db)
|
||||
.await?,
|
||||
None
|
||||
);
|
||||
|
||||
|
||||
@@ -5377,16 +5377,17 @@ async fn create_workspace(
|
||||
Ok(format!("Created workspace {}", &nw.id))
|
||||
}
|
||||
|
||||
// `authed_email` is the forker's email — `clone_drafts` only carries this
|
||||
// user's per-user drafts (and the legacy NULL-email workspace draft, if any)
|
||||
// across, since other users aren't added to the fork's `usr` table and
|
||||
// their drafts would dangle as orphans.
|
||||
// `authed` is the forker — `clone_drafts` only carries this user's per-user
|
||||
// drafts (and the legacy NULL-email workspace draft, if any) across, since other
|
||||
// users aren't added to the fork's `usr` table and their drafts would dangle as
|
||||
// orphans; `clone_apps` needs their role to decide whether cloned app policies
|
||||
// may keep someone else's run-as identity.
|
||||
async fn clone_workspace_data(
|
||||
tx: &mut Transaction<'_, Postgres>,
|
||||
db: &DB,
|
||||
source_workspace_id: &str,
|
||||
target_workspace_id: &str,
|
||||
authed_email: &str,
|
||||
authed: &ApiAuthed,
|
||||
) -> Result<()> {
|
||||
// Clone workspace settings (merge with existing basic settings)
|
||||
update_workspace_settings(tx, source_workspace_id, target_workspace_id).await?;
|
||||
@@ -5440,7 +5441,7 @@ async fn clone_workspace_data(
|
||||
clone_flow_nodes(tx, source_workspace_id, target_workspace_id).await?;
|
||||
|
||||
// Clone apps with new IDs and app scripts
|
||||
let _app_id_mapping = clone_apps(tx, source_workspace_id, target_workspace_id).await?;
|
||||
let _app_id_mapping = clone_apps(tx, source_workspace_id, target_workspace_id, authed).await?;
|
||||
|
||||
// Clone raw apps
|
||||
clone_raw_apps(tx, source_workspace_id, target_workspace_id).await?;
|
||||
@@ -5451,7 +5452,7 @@ async fn clone_workspace_data(
|
||||
// own a `usr` row in the fork (see `clone_workspace_full`) so their
|
||||
// drafts would dangle and the home-page `draft_users` aggregate would
|
||||
// surface them as duplicate legacy entries.
|
||||
clone_drafts(tx, source_workspace_id, target_workspace_id, authed_email).await?;
|
||||
clone_drafts(tx, source_workspace_id, target_workspace_id, &authed.email).await?;
|
||||
|
||||
// Clone workspace runnable dependencies and dependency map
|
||||
clone_workspace_runnable_dependencies(tx, source_workspace_id, target_workspace_id).await?;
|
||||
@@ -6392,11 +6393,47 @@ async fn clone_flow_nodes(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Re-point a cloned app policy at the fork's creator, the way `create_app` /
|
||||
/// `update_app` do for a caller who may not preserve someone else's identity.
|
||||
/// `on_behalf_of` is what anonymous and publisher executions queue jobs under, and
|
||||
/// the fork's own endpoint outlives any revocation in the parent, so cloning it
|
||||
/// verbatim would hand the creator an identity they cannot otherwise assume.
|
||||
/// Anonymous apps also lose their unauthenticated endpoint: re-publishing goes
|
||||
/// through the app API, which enforces the workspace's anonymous-deployment rule.
|
||||
fn downgrade_cloned_app_policy(policy: &mut serde_json::Value, authed: &ApiAuthed) {
|
||||
let Some(obj) = policy.as_object_mut() else {
|
||||
return;
|
||||
};
|
||||
obj.insert(
|
||||
"on_behalf_of".to_string(),
|
||||
serde_json::Value::String(username_to_permissioned_as(&authed.username)),
|
||||
);
|
||||
obj.insert(
|
||||
"on_behalf_of_email".to_string(),
|
||||
serde_json::Value::String(authed.email.clone()),
|
||||
);
|
||||
// An absent `execution_mode` deserializes as anonymous (serde default), so it is
|
||||
// one too.
|
||||
let anonymous = obj
|
||||
.get("execution_mode")
|
||||
.and_then(|m| m.as_str())
|
||||
.unwrap_or("anonymous")
|
||||
== "anonymous";
|
||||
if anonymous {
|
||||
obj.insert(
|
||||
"execution_mode".to_string(),
|
||||
serde_json::Value::String("publisher".to_string()),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async fn clone_apps(
|
||||
tx: &mut Transaction<'_, Postgres>,
|
||||
source_workspace_id: &str,
|
||||
target_workspace_id: &str,
|
||||
authed: &ApiAuthed,
|
||||
) -> Result<HashMap<i64, i64>> {
|
||||
let preserve_identity = windmill_common::can_preserve_on_behalf_of(authed);
|
||||
// Get all apps from source workspace
|
||||
let apps = sqlx::query!(
|
||||
"SELECT id, workspace_id, path, summary, policy, versions, extra_perms, custom_path
|
||||
@@ -6416,10 +6453,17 @@ async fn clone_apps(
|
||||
let mut latest_version_ids: HashSet<i64> = HashSet::new();
|
||||
|
||||
// Clone apps with new IDs
|
||||
for app in apps {
|
||||
for mut app in apps {
|
||||
if let Some(¤t_version) = app.versions.last() {
|
||||
latest_version_ids.insert(current_version);
|
||||
}
|
||||
if !preserve_identity {
|
||||
downgrade_cloned_app_policy(&mut app.policy, authed);
|
||||
}
|
||||
// Off cloud a custom path addresses an app instance-wide (`create_app` rejects one
|
||||
// already taken in any workspace), so a clone that kept it would make the parent's
|
||||
// live public URL resolve to either row. Cloud scopes the lookup by workspace.
|
||||
let custom_path = if *CLOUD_HOSTED { app.custom_path } else { None };
|
||||
let new_app_id = sqlx::query_scalar!(
|
||||
"INSERT INTO app (workspace_id, path, summary, policy, versions, extra_perms, custom_path)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7)
|
||||
@@ -6430,7 +6474,7 @@ async fn clone_apps(
|
||||
app.policy,
|
||||
&Vec::<i64>::new(), // Start with empty versions array
|
||||
app.extra_perms,
|
||||
app.custom_path,
|
||||
custom_path,
|
||||
)
|
||||
.fetch_one(&mut **tx)
|
||||
.await?;
|
||||
@@ -7283,14 +7327,8 @@ async fn create_workspace_fork(
|
||||
.await?;
|
||||
|
||||
// Clone all data from the parent workspace using Rust implementation
|
||||
if let Err(e) = clone_workspace_data(
|
||||
&mut tx,
|
||||
&db,
|
||||
&parent_workspace_id,
|
||||
&forked_id,
|
||||
&authed.email,
|
||||
)
|
||||
.await
|
||||
if let Err(e) =
|
||||
clone_workspace_data(&mut tx, &db, &parent_workspace_id, &forked_id, &authed).await
|
||||
{
|
||||
// A genuine `\u0000` in a source `json` value (`app_version.value` /
|
||||
// `flow_version.schema`) aborts the clone when it is re-encoded to jsonb:
|
||||
|
||||
Reference in New Issue
Block a user