diff --git a/backend/.sqlx/query-15f53f40a49b538ca0d4456f69b8d801e8c121f1e0cd904a77ce55bcf9bc5053.json b/backend/.sqlx/query-15f53f40a49b538ca0d4456f69b8d801e8c121f1e0cd904a77ce55bcf9bc5053.json new file mode 100644 index 0000000000..6510374dfc --- /dev/null +++ b/backend/.sqlx/query-15f53f40a49b538ca0d4456f69b8d801e8c121f1e0cd904a77ce55bcf9bc5053.json @@ -0,0 +1,29 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT resource_type, value AS \"value: sqlx::types::Json>\"\n FROM resource WHERE workspace_id = $1 AND path = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "resource_type", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "value: sqlx::types::Json>", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + false, + true + ] + }, + "hash": "15f53f40a49b538ca0d4456f69b8d801e8c121f1e0cd904a77ce55bcf9bc5053" +} diff --git a/backend/.sqlx/query-748b63f877498d6d279fdf23894611e11c7a05d69b4b7907f8909aec82fb1eb2.json b/backend/.sqlx/query-748b63f877498d6d279fdf23894611e11c7a05d69b4b7907f8909aec82fb1eb2.json deleted file mode 100644 index d710d59055..0000000000 --- a/backend/.sqlx/query-748b63f877498d6d279fdf23894611e11c7a05d69b4b7907f8909aec82fb1eb2.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "INSERT INTO resource (workspace_id, path, value, description, resource_type, extra_perms, edited_at, created_by)\n SELECT $2, path, value, description, resource_type, extra_perms, edited_at, created_by\n FROM resource\n WHERE workspace_id = $1", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Text", - "Varchar" - ] - }, - "nullable": [] - }, - "hash": "748b63f877498d6d279fdf23894611e11c7a05d69b4b7907f8909aec82fb1eb2" -} diff --git a/backend/.sqlx/query-bf763c3ae4951537033d18ce48d3eb83d5c19e32aef199bf161e8ba8d1adf176.json b/backend/.sqlx/query-bf763c3ae4951537033d18ce48d3eb83d5c19e32aef199bf161e8ba8d1adf176.json new file mode 100644 index 0000000000..d6b261b56c --- /dev/null +++ b/backend/.sqlx/query-bf763c3ae4951537033d18ce48d3eb83d5c19e32aef199bf161e8ba8d1adf176.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO resource (workspace_id, path, value, description, resource_type, extra_perms, edited_at, created_by)\n SELECT $2, path,\n CASE WHEN $3::text IS NOT NULL AND resource_type = 'ai_agent'\n AND jsonb_typeof(value) = 'object' AND value ? 'on_behalf_of'\n THEN jsonb_set(value, '{on_behalf_of}', to_jsonb($3::text))\n ELSE value END,\n description, resource_type, extra_perms, edited_at, created_by\n FROM resource\n WHERE workspace_id = $1", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Varchar", + "Text" + ] + }, + "nullable": [] + }, + "hash": "bf763c3ae4951537033d18ce48d3eb83d5c19e32aef199bf161e8ba8d1adf176" +} diff --git a/backend/migrations/20260925112726_agent_on_behalf_of.down.sql b/backend/migrations/20260925112726_agent_on_behalf_of.down.sql new file mode 100644 index 0000000000..db77ee094c --- /dev/null +++ b/backend/migrations/20260925112726_agent_on_behalf_of.down.sql @@ -0,0 +1,4 @@ +ALTER TABLE resource DISABLE TRIGGER record_resource_version_update_trigger; +UPDATE resource SET value = value - 'on_behalf_of' + WHERE resource_type = 'ai_agent' AND jsonb_typeof(value) = 'object' AND value ? 'on_behalf_of'; +ALTER TABLE resource ENABLE TRIGGER record_resource_version_update_trigger; diff --git a/backend/migrations/20260925112726_agent_on_behalf_of.up.sql b/backend/migrations/20260925112726_agent_on_behalf_of.up.sql new file mode 100644 index 0000000000..5c047a9d11 --- /dev/null +++ b/backend/migrations/20260925112726_agent_on_behalf_of.up.sql @@ -0,0 +1,43 @@ +-- An AI agent's value carries the identity it runs as when run from its own page, as a flow's or +-- a script's `on_behalf_of` is: a principal (`u/`, `g/`), resolved by the server on +-- every write of the agent. An agent saved before this ran as whoever ran it; give it the +-- account that wrote its current version, the one a write now records. + +-- Mirrors `users::username_to_permissioned_as`, as the sibling backfills do. `pg_temp` lives for +-- the whole session and migrations share one connection, so replace any earlier copy and drop +-- this one at the end. +CREATE OR REPLACE FUNCTION pg_temp.username_to_permissioned_as(name VARCHAR) +RETURNS VARCHAR AS $$ + SELECT CASE + WHEN $1 LIKE '%@%' AND $1 LIKE '%/%' THEN 'u/' || $1 + WHEN $1 LIKE '%@%' THEN $1 + WHEN $1 LIKE 'group-%' THEN 'g/' || substr($1, 7) + ELSE 'u/' || $1 + END; +$$ LANGUAGE SQL IMMUTABLE; + +-- Filling in the identity is not an edit of the agent, so it records no version. +ALTER TABLE resource DISABLE TRIGGER record_resource_version_update_trigger; + +-- A principal wider than `v2_job.permissioned_as` could not be enqueued, so it is left unset, as +-- the sibling backfills leave it (20260801043001, 20260911085221). +UPDATE resource r + SET value = jsonb_set(r.value, '{on_behalf_of}', + to_jsonb(pg_temp.username_to_permissioned_as(v.created_by))) + FROM ( + SELECT DISTINCT ON (workspace_id, path) workspace_id, path, created_by + FROM resource_version + ORDER BY workspace_id, path, id DESC + ) v + WHERE r.workspace_id = v.workspace_id + AND r.path = v.path + AND r.resource_type = 'ai_agent' + AND jsonb_typeof(r.value) = 'object' + AND NOT (r.value ? 'on_behalf_of') + AND v.created_by IS NOT NULL + AND v.created_by <> '' + AND length(pg_temp.username_to_permissioned_as(v.created_by)) <= 55; + +ALTER TABLE resource ENABLE TRIGGER record_resource_version_update_trigger; + +DROP FUNCTION pg_temp.username_to_permissioned_as(VARCHAR); diff --git a/backend/windmill-api-integration-tests/tests/agent_on_behalf_of.rs b/backend/windmill-api-integration-tests/tests/agent_on_behalf_of.rs new file mode 100644 index 0000000000..2947a35923 --- /dev/null +++ b/backend/windmill-api-integration-tests/tests/agent_on_behalf_of.rs @@ -0,0 +1,115 @@ +use serde_json::json; +use sqlx::{Pool, Postgres}; + +use windmill_test_utils::*; + +const AGENT: &str = "u/test-user-3/agent"; + +async fn post( + base_url: &str, + token: &str, + route: &str, + body: serde_json::Value, +) -> anyhow::Result<(u16, String)> { + let resp = reqwest::Client::new() + .post(format!("{base_url}/w/test-workspace/{route}")) + .header("Authorization", format!("Bearer {token}")) + .json(&body) + .send() + .await?; + Ok((resp.status().as_u16(), resp.text().await?)) +} + +async fn agent_identity(db: &Pool) -> anyhow::Result> { + Ok(sqlx::query_scalar::<_, Option>( + "SELECT value->>'on_behalf_of' FROM resource + WHERE workspace_id = 'test-workspace' AND path = $1", + ) + .bind(AGENT) + .fetch_one(db) + .await?) +} + +/// The identity an agent runs as is in its value, which anyone who can write the agent writes: it +/// is kept only for an admin asking to keep it, and every other write makes it the writer's own, +/// so a writer cannot change what runs while it keeps running as someone else. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_agent_write_resets_identity(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let base_url = format!("http://localhost:{}/api", server.addr.port()); + let value = json!({ "system_prompt": "hi", "on_behalf_of": "u/test-user-2" }); + + let (status, body) = post( + &base_url, + "SECRET_TOKEN", + "resources/create", + json!({ "path": AGENT, "resource_type": "ai_agent", "value": value, + "preserve_on_behalf_of": true }), + ) + .await?; + assert!((200..300).contains(&status), "create: {status} {body}"); + assert_eq!(agent_identity(&db).await?.as_deref(), Some("u/test-user-2")); + + // The agent's owner, not an admin, asking to keep it. + let (status, body) = post( + &base_url, + "SECRET_TOKEN_3", + &format!("resources/update/{AGENT}"), + json!({ "value": value, "preserve_on_behalf_of": true }), + ) + .await?; + assert!((200..300).contains(&status), "update: {status} {body}"); + assert_eq!(agent_identity(&db).await?.as_deref(), Some("u/test-user-3")); + + Ok(()) +} + +/// Reading an agent is what allows running it, and the run is the agent's identity, not the +/// caller's: `test-user-2`, a plain member, runs an agent that runs as `test-user-3`. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_agent_run_is_its_identity_and_needs_read(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let base_url = format!("http://localhost:{}/api", server.addr.port()); + + sqlx::query( + "INSERT INTO resource (workspace_id, path, value, resource_type, extra_perms, created_by) + VALUES ('test-workspace', $1, $2, 'ai_agent', '{}', 'test-user-3')", + ) + .bind(AGENT) + .bind(json!({ "system_prompt": "hi", "on_behalf_of": "u/test-user-3" })) + .execute(&db) + .await?; + + let run = format!("jobs/run/agent/{AGENT}"); + let (status, body) = post(&base_url, "SECRET_TOKEN_2", &run, json!({})).await?; + assert_eq!(status, 404, "an unreadable agent: {body}"); + + sqlx::query( + "UPDATE resource SET extra_perms = '{\"u/test-user-2\": false}' + WHERE workspace_id = 'test-workspace' AND path = $1", + ) + .bind(AGENT) + .execute(&db) + .await?; + let (status, body) = post( + &base_url, + "SECRET_TOKEN_2", + &run, + json!({ "user_message": "hi" }), + ) + .await?; + assert_eq!(status, 201, "a readable agent: {body}"); + + let (permissioned_as, created_by) = sqlx::query_as::<_, (String, String)>( + "SELECT permissioned_as, created_by FROM v2_job WHERE id = $1::uuid", + ) + .bind(&body) + .fetch_one(&db) + .await?; + assert_eq!(permissioned_as, "u/test-user-3"); + assert_eq!(created_by, "test-user-2"); + + Ok(()) +} diff --git a/backend/windmill-api-integration-tests/tests/agent_runs.rs b/backend/windmill-api-integration-tests/tests/agent_runs.rs deleted file mode 100644 index b0302187cb..0000000000 --- a/backend/windmill-api-integration-tests/tests/agent_runs.rs +++ /dev/null @@ -1,60 +0,0 @@ -use serde_json::json; -use sqlx::{Pool, Postgres}; - -use windmill_test_utils::*; - -const AGENT: &str = "u/test-user-3/agent"; - -async fn run(base_url: &str, token: &str) -> anyhow::Result<(u16, String)> { - let resp = reqwest::Client::new() - .post(format!( - "{base_url}/w/test-workspace/jobs/run/agent/{AGENT}" - )) - .header("Authorization", format!("Bearer {token}")) - .json(&json!({ "user_message": "hi" })) - .send() - .await?; - Ok((resp.status().as_u16(), resp.text().await?)) -} - -/// Reading an agent is what allows running it, and the run is the caller's own: `test-user-2`, a -/// plain member, runs `test-user-3`'s agent once it may read it, lending nobody's permissions. -#[sqlx::test(migrations = "../migrations", fixtures("base"))] -async fn test_agent_run_needs_read_and_runs_as_caller(db: Pool) -> anyhow::Result<()> { - initialize_tracing().await; - let server = ApiServer::start(db.clone()).await?; - let base_url = format!("http://localhost:{}/api", server.addr.port()); - - sqlx::query( - "INSERT INTO resource (workspace_id, path, value, resource_type, extra_perms, created_by) - VALUES ('test-workspace', $1, $2, 'ai_agent', '{}', 'test-user-3')", - ) - .bind(AGENT) - .bind(json!({ "system_prompt": "hi" })) - .execute(&db) - .await?; - - let (status, body) = run(&base_url, "SECRET_TOKEN_2").await?; - assert_eq!(status, 404, "an unreadable agent: {body}"); - - sqlx::query( - "UPDATE resource SET extra_perms = '{\"u/test-user-2\": false}' - WHERE workspace_id = 'test-workspace' AND path = $1", - ) - .bind(AGENT) - .execute(&db) - .await?; - let (status, body) = run(&base_url, "SECRET_TOKEN_2").await?; - assert_eq!(status, 201, "a readable agent: {body}"); - - let (permissioned_as, created_by) = sqlx::query_as::<_, (String, String)>( - "SELECT permissioned_as, created_by FROM v2_job WHERE id = $1::uuid", - ) - .bind(&body) - .fetch_one(&db) - .await?; - assert_eq!(permissioned_as, "u/test-user-2"); - assert_eq!(created_by, "test-user-2"); - - Ok(()) -} diff --git a/backend/windmill-api-integration-tests/tests/fork_clone_on_behalf_of.rs b/backend/windmill-api-integration-tests/tests/fork_clone_on_behalf_of.rs index bc6c10f1ee..10fc0a8028 100644 --- a/backend/windmill-api-integration-tests/tests/fork_clone_on_behalf_of.rs +++ b/backend/windmill-api-integration-tests/tests/fork_clone_on_behalf_of.rs @@ -62,14 +62,31 @@ async fn fork_with_public_app( /// 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. +/// not preserve someone else's identity must not receive one by forking. The same goes for the +/// one an agent's value carries. `test-user-2` is a plain member of the parent. #[sqlx::test(migrations = "../migrations", fixtures("base"))] async fn test_fork_repoints_app_identity_for_unprivileged_creator( db: Pool, ) -> anyhow::Result<()> { + sqlx::query( + "INSERT INTO resource (workspace_id, path, value, resource_type, extra_perms, created_by) + VALUES ('test-workspace', 'u/test-user/agent', $1, 'ai_agent', '{}', 'test-user')", + ) + .bind(json!({ "system_prompt": "hi", "on_behalf_of": "u/test-user" })) + .execute(&db) + .await?; let (policy, custom_path) = fork_with_public_app(&db, "SECRET_TOKEN_2").await?; + assert_eq!( + sqlx::query_scalar::<_, String>( + "SELECT value->>'on_behalf_of' FROM resource + WHERE workspace_id = 'wm-fork-app' AND path = 'u/test-user/agent'" + ) + .fetch_one(&db) + .await?, + "u/test-user-2" + ); + assert_eq!(policy["on_behalf_of"], json!("u/test-user-2")); assert_eq!(policy["on_behalf_of_email"], json!("test2@windmill.dev")); // `execution_mode` rides along untouched — see `clone_apps`. @@ -312,8 +329,9 @@ async fn test_fork_keeps_only_resolvable_on_behalf_of(db: Pool) -> any Ok(()) } -/// Apps, schedules, triggers and their drafts cannot drop an identity the way scripts and flows -/// do, so one naming nobody in the fork goes to its creator while one that still resolves stays. +/// Apps, agents, schedules, triggers and their drafts cannot drop an identity the way scripts and +/// flows do, so one naming nobody in the fork goes to its creator while one that still resolves +/// stays. /// Forked as an admin, whose app policies the clone otherwise keeps. #[sqlx::test(migrations = "../migrations", fixtures("base"))] async fn test_fork_repoints_unresolvable_identities(db: Pool) -> anyhow::Result<()> { @@ -367,6 +385,13 @@ async fn test_fork_repoints_unresolvable_identities(db: Pool) -> anyho )) .execute(&db) .await?; + sqlx::query( + "INSERT INTO resource (workspace_id, path, value, resource_type, extra_perms, created_by) + VALUES ('test-workspace', 'u/test-user/stranger', $1, 'ai_agent', '{}', 'test-user')", + ) + .bind(json!({ "system_prompt": "hi", "on_behalf_of": "u/test-user-2" })) + .execute(&db) + .await?; sqlx::query( "INSERT INTO schedule (workspace_id, path, edited_by, schedule, script_path, email, permissioned_as, enabled) VALUES ('test-workspace', 'u/test-user/stranger', 'test-user', '0 0 * * * *', 'u/test-user/s', 'test2@windmill.dev', 'u/test-user-2', false)", @@ -424,6 +449,10 @@ async fn test_fork_repoints_unresolvable_identities(db: Pool) -> anyho text("SELECT value->>'permissioned_as' FROM draft WHERE workspace_id = 'wm-fork-repoint' AND typ = 'trigger_websocket'").await?, "u/test-user" ); + assert_eq!( + text("SELECT value->>'on_behalf_of' FROM resource WHERE workspace_id = 'wm-fork-repoint' AND path = 'u/test-user/stranger'").await?, + "u/test-user" + ); assert_eq!( text("SELECT permissioned_as || ' ' || email FROM schedule WHERE workspace_id = 'wm-fork-repoint'").await?, "u/test-user test@windmill.dev" diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 8ebd4eeb26..124992cb01 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -6672,7 +6672,7 @@ async fn clone_workspace_data( clone_resource_types(tx, source_workspace_id, target_workspace_id).await?; // Clone resources - clone_resources(tx, source_workspace_id, target_workspace_id).await?; + clone_resources(tx, source_workspace_id, target_workspace_id, authed).await?; // Clone variables (including external secret backend replication) clone_variables(tx, db, source_workspace_id, target_workspace_id).await?; @@ -7205,18 +7205,30 @@ async fn clone_resource_types( Ok(()) } +/// An agent's value carries the identity it runs as, re-pointed at the fork's creator when they +/// may not preserve someone else's, as `clone_apps` does an app's policy. The rest is swept by +/// `repoint_unresolvable_cloned_identities` once the fork's membership is final. async fn clone_resources( tx: &mut Transaction<'_, Postgres>, source_workspace_id: &str, target_workspace_id: &str, + authed: &ApiAuthed, ) -> Result<()> { + let repoint = (!windmill_common::can_preserve_on_behalf_of(authed)) + .then(|| username_to_permissioned_as(&authed.username)); sqlx::query!( "INSERT INTO resource (workspace_id, path, value, description, resource_type, extra_perms, edited_at, created_by) - SELECT $2, path, value, description, resource_type, extra_perms, edited_at, created_by + SELECT $2, path, + CASE WHEN $3::text IS NOT NULL AND resource_type = 'ai_agent' + AND jsonb_typeof(value) = 'object' AND value ? 'on_behalf_of' + THEN jsonb_set(value, '{on_behalf_of}', to_jsonb($3::text)) + ELSE value END, + description, resource_type, extra_perms, edited_at, created_by FROM resource WHERE workspace_id = $1", source_workspace_id, target_workspace_id, + repoint, ) .execute(&mut **tx) .await?; @@ -7703,6 +7715,18 @@ async fn repoint_unresolvable_cloned_identities( .await?; } + sqlx::query(&format!( + "UPDATE resource SET value = jsonb_set(value, '{{on_behalf_of}}', to_jsonb($2::text)) + WHERE workspace_id = $1 AND resource_type = 'ai_agent' + AND jsonb_typeof(value) = 'object' AND value ? 'on_behalf_of' + AND NOT ({})", + principal_resolves_sql("(value->>'on_behalf_of')") + )) + .bind(target_workspace_id) + .bind(&principal) + .execute(&mut **tx) + .await?; + // `email` is still written for workers that predate `permissioned_as`. sqlx::query(&format!( "UPDATE schedule SET permissioned_as = $2, email = $3 @@ -13052,17 +13076,18 @@ async fn compare_two_flows( }); } -/// The policy minus the identity pair. Deploying cannot converge a difference there — the -/// target recomputes the identity from the deployer's own choice, which offers its current -/// value, the deployer, or a typed-in one, never the source's — so listing an app for it -/// alone leaves an entry no deploy can clear. `script` and `flow` compare no identity either. -fn policy_without_identity(policy: &serde_json::Value) -> serde_json::Value { - let mut policy = policy.clone(); - if let Some(obj) = policy.as_object_mut() { +/// An app's policy or an agent's value minus the identity it carries. Deploying cannot converge +/// a difference there — the target recomputes the identity from the deployer's own choice, which +/// offers its current value, the deployer, or a typed-in one, never the source's — so listing an +/// item for it alone leaves an entry no deploy can clear. `script` and `flow` compare no identity +/// either. +fn without_identity(value: &serde_json::Value) -> serde_json::Value { + let mut value = value.clone(); + if let Some(obj) = value.as_object_mut() { obj.remove("on_behalf_of"); obj.remove("on_behalf_of_email"); } - policy + value } async fn compare_two_apps( @@ -13105,7 +13130,7 @@ async fn compare_two_apps( // Check metadata and content differences if let (Some(source), Some(target)) = (&source_app, &target_app) { if source.summary != target.summary - || policy_without_identity(&source.policy) != policy_without_identity(&target.policy) + || without_identity(&source.policy) != without_identity(&target.policy) || source.value != target.value || source.raw_app != target.raw_app { @@ -13183,7 +13208,14 @@ async fn compare_two_resources( // Check metadata differences if let (Some(source), Some(target)) = (&source_resource, &target_resource) { - if source.value != target.value + let agent = source.resource_type == "ai_agent" && target.resource_type == "ai_agent"; + let values_differ = if agent { + source.value.as_ref().map(without_identity) + != target.value.as_ref().map(without_identity) + } else { + source.value != target.value + }; + if values_differ || source.description != target.description || source.resource_type != target.resource_type { diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 010f422346..ff084a44e4 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -15960,8 +15960,7 @@ paths: summary: run a saved AI agent description: | Runs the `ai_agent` resource at `path` as stored, for any caller who can read it - (operators included), as the caller, who needs access to the agent's AI resource and to - whatever its tools use. With `memory_id`, the run is a turn + (operators included), as the agent's `on_behalf_of`. With `memory_id`, the run is a turn of the caller's conversation with the agent; without, a single run. operationId: runAgent tags: @@ -31607,6 +31606,9 @@ components: type: string ws_specific: type: boolean + preserve_on_behalf_of: + type: boolean + description: ai_agent only. Keep the identity `value.on_behalf_of` names (`u/`, `g/`) rather than the caller's own. Honoured for admins and members of wm_deployers; otherwise every write of an agent sets it to the caller. required: - path - value @@ -31631,6 +31633,9 @@ components: type: string ws_specific: type: boolean + preserve_on_behalf_of: + type: boolean + description: ai_agent only. Keep the identity `value.on_behalf_of` names (`u/`, `g/`) rather than the caller's own. Honoured for admins and members of wm_deployers; otherwise every write of an agent sets it to the caller. Resource: type: object diff --git a/backend/windmill-api/src/agent_runs.rs b/backend/windmill-api/src/agent_runs.rs index 065c3815fb..a5bb5756c2 100644 --- a/backend/windmill-api/src/agent_runs.rs +++ b/backend/windmill-api/src/agent_runs.rs @@ -2,8 +2,8 @@ //! //! The run is built here from the stored agent rather than sent by the client, which is what lets //! anyone who can read the agent run it, operators included, the way a deployed flow runs by path: -//! a preview takes arbitrary code, this takes only the run's inputs. It runs as the caller, who -//! needs access to the agent's AI resource and to whatever its tools use. +//! a preview takes arbitrary code, this takes only the run's inputs. It runs as the agent's +//! `on_behalf_of`, which the server resolves on every write of the agent. use std::collections::HashMap; @@ -59,11 +59,16 @@ pub(crate) async fn run_agent( .fetch_optional(&mut *tx) .await?; tx.commit().await?; - let Some(sqlx::types::Json(value)) = not_found_if_none(value, "Agent", path)? else { + let Some(sqlx::types::Json(mut value)) = not_found_if_none(value, "Agent", path)? else { return Err(Error::BadRequest(format!( "Agent {path} has no configuration" ))); }; + // Who the agent runs as, not one of its inputs. + let agent_obo = value + .as_object_mut() + .and_then(|o| o.remove("on_behalf_of")) + .and_then(|v| v.as_str().map(str::to_string)); let config = config_to_draft(value)?; // A chat turn is filed where the agent's chat lists its conversations, which a flow cannot @@ -76,10 +81,31 @@ pub(crate) async fn run_agent( }; let flow_value = agent_flow(&config, chat)?; - let push_authed = authed.clone().into(); + let on_behalf_of = + windmill_common::on_behalf_of_from_permissioned_as(agent_obo.as_deref(), &w_id, &db) + .await?; + // As `run_flow` picks the identity of a deployed flow: the agent's own when it has one, the + // caller's otherwise, which lends nobody's permissions. + let (email, permissioned_as, push_authed, isolation) = if let Some(obo) = on_behalf_of.as_ref() + { + ( + &obo.email, + obo.permissioned_as.clone(), + None, + PushIsolationLevel::IsolatedRoot(db.clone()), + ) + } else { + ( + &authed.email, + username_to_permissioned_as(&authed.username), + Some(authed.clone().into()), + PushIsolationLevel::Isolated(user_db.clone(), authed.clone().into()), + ) + }; + let (uuid, mut tx) = push( &db, - PushIsolationLevel::Isolated(user_db.clone(), authed.clone().into()), + isolation, &w_id, JobPayload::RawFlow { value: flow_value, @@ -88,8 +114,8 @@ pub(crate) async fn run_agent( }, PushArgs::from(&args), authed.display_username(), - &authed.email, - username_to_permissioned_as(&authed.username), + email, + permissioned_as, authed.token_prefix.as_deref(), authed.username_override.as_deref(), None, @@ -106,7 +132,7 @@ pub(crate) async fn run_agent( None, None, None, - Some(&push_authed), + push_authed.as_ref(), false, None, authed.trigger_or_fallback(None), @@ -118,7 +144,8 @@ pub(crate) async fn run_agent( set_flow_memory_id(&mut tx, uuid, memory_id).await?; } if chat { - // A real conversation, not a test one: this is the agent as deployed, not a draft. + // The caller's conversation, whoever the agent runs as, and a real one: this is the + // agent as deployed, not a draft under test. handle_chat_conversation_messages( &mut tx, &authed, diff --git a/backend/windmill-api/src/workspaces_export.rs b/backend/windmill-api/src/workspaces_export.rs index ea79aaa572..544984f164 100644 --- a/backend/windmill-api/src/workspaces_export.rs +++ b/backend/windmill-api/src/workspaces_export.rs @@ -933,7 +933,14 @@ pub(crate) async fn tarball_workspace( .fetch_all(&mut *tx) .await?; - for resource in resources { + for mut resource in resources { + // An agent's identity belongs to this workspace, and whoever writes it elsewhere gets + // their own, as `to_string_without_metadata` leaves out a flow's. + if resource.resource_type == "ai_agent" { + if let Some(obj) = resource.value.as_mut().and_then(|v| v.as_object_mut()) { + obj.remove("on_behalf_of"); + } + } let resource_str = &to_string_without_metadata(&resource, ExtraPermsBehavior::Drop, None).unwrap(); archive diff --git a/backend/windmill-store/src/resources.rs b/backend/windmill-store/src/resources.rs index 4485a78dd4..89ebb0996e 100644 --- a/backend/windmill-store/src/resources.rs +++ b/backend/windmill-store/src/resources.rs @@ -234,6 +234,10 @@ pub struct CreateResource { pub labels: Option>, #[serde(default)] pub ws_specific: Option, + /// `ai_agent` only: keep the identity `value.on_behalf_of` names, for a caller allowed to + /// (see `resolve_agent_on_behalf_of`). + #[serde(default)] + pub preserve_on_behalf_of: Option, } #[derive(Deserialize)] struct EditResource { @@ -243,6 +247,7 @@ struct EditResource { resource_type: Option, labels: Option>, ws_specific: Option, + preserve_on_behalf_of: Option, } #[derive(Deserialize)] @@ -1213,7 +1218,20 @@ async fn create_resource( check_path_conflict(&mut tx, &w_id, &resource.path).await?; } - let res_value = resource.value.unwrap_or_default(); + let mut res_value = resource.value.unwrap_or_default(); + if resource.resource_type == "ai_agent" { + res_value = resolve_agent_raw_value( + &mut tx, + &authed, + &db, + &w_id, + &resource.path, + &res_value, + resource.preserve_on_behalf_of.unwrap_or(false), + true, + ) + .await?; + } let raw_json = sqlx::types::Json(res_value.as_ref()); if resource.path.starts_with("f/app_themes/") { @@ -2054,9 +2072,6 @@ async fn update_resource( if let Some(npath) = &ns.path { sqlb.set_str("path", npath); } - if let Some(nvalue) = &ns.value { - sqlb.set_str("value", nvalue.to_string()); - } if let Some(nrt) = &ns.resource_type { sqlb.set_str("resource_type", nrt); } @@ -2164,6 +2179,48 @@ async fn update_resource( } } + let current = sqlx::query!( + "SELECT resource_type, value AS \"value: sqlx::types::Json>\" + FROM resource WHERE workspace_id = $1 AND path = $2", + &w_id, + path + ) + .fetch_optional(&mut *tx) + .await?; + let resource_type = ns + .resource_type + .clone() + .or_else(|| current.as_ref().map(|c| c.resource_type.clone())); + // Retyping a resource into an agent makes its value an agent's, identity included, so it is + // resolved as a written one is. + let retyped = ns.resource_type.as_deref() == Some("ai_agent") + && current + .as_ref() + .is_some_and(|c| c.resource_type != "ai_agent"); + let nvalue = match &ns.value { + Some(v) => Some(v.clone()), + None if retyped => current.and_then(|c| c.value).map(|v| v.0), + None => None, + }; + if let Some(nvalue) = nvalue { + let nvalue = if resource_type.as_deref() == Some("ai_agent") { + resolve_agent_raw_value( + &mut tx, + &authed, + &db, + &w_id, + ns.path.as_deref().unwrap_or(path), + &nvalue, + ns.preserve_on_behalf_of.unwrap_or(false), + false, + ) + .await? + } else { + nvalue + }; + sqlb.set_str("value", nvalue.to_string()); + } + let sql = sqlb.sql().map_err(|e| Error::internal_err(e.to_string()))?; let npath_o: Option = sqlx::query_scalar(&sql) .fetch_optional(&mut *tx) @@ -2333,6 +2390,91 @@ async fn update_resource_value( Ok(format!("value of resource {} updated", path)) } +/// Rewrites the identity an `ai_agent` runs as (`value.on_behalf_of`), resolved on every write of +/// it as a flow's `on_behalf_of` is on every deploy: the writer's own, unless one allowed to keep +/// the one the value names asks to, and a folder's default for a new agent. The value is written +/// by anyone who can write the agent, so the key it arrives with is only a request. +async fn resolve_agent_on_behalf_of( + tx: &mut Transaction<'_, Postgres>, + authed: &ApiAuthed, + db: &DB, + w_id: &str, + path: &str, + value: &mut serde_json::Value, + preserve: bool, + created: bool, +) -> Result<()> { + let Some(obj) = value.as_object_mut() else { + return Ok(()); + }; + let mut requested = obj + .remove("on_behalf_of") + .and_then(|v| v.as_str().map(str::to_string)); + let mut preserve = preserve; + let explicit = + requested.is_some() && preserve && windmill_common::can_preserve_on_behalf_of(authed); + if created && !explicit && windmill_common::can_preserve_on_behalf_of(authed) { + if let Some((_, folder_default)) = + windmill_common::folders::resolve_folder_default_on_behalf_of(db, w_id, path).await? + { + requested = Some(folder_default); + preserve = true; + } + } + let own = windmill_common::users::username_to_permissioned_as(&authed.username); + // Always an identity: nothing requested is the writer's own. + let resolved = windmill_common::resolve_on_behalf_of( + None, + Some(requested.as_deref().unwrap_or(&own)), + preserve, + authed, + w_id, + db, + ) + .await?; + if let Some(resolved) = resolved.as_ref() { + obj.insert("on_behalf_of".to_string(), resolved.clone().into()); + } + if let Some(kept) = windmill_common::check_on_behalf_of_preservation( + resolved.as_deref(), + preserve, + authed, + &own, + ) { + audit_log( + &mut **tx, + authed, + "resources.on_behalf_of", + if created { + ActionKind::Create + } else { + ActionKind::Update + }, + w_id, + Some(path), + Some([("on_behalf_of", kept.as_str())].into()), + ) + .await?; + } + Ok(()) +} + +/// `resolve_agent_on_behalf_of` on a value as the request carried it. +async fn resolve_agent_raw_value( + tx: &mut Transaction<'_, Postgres>, + authed: &ApiAuthed, + db: &DB, + w_id: &str, + path: &str, + value: &RawValue, + preserve: bool, + created: bool, +) -> Result> { + let mut parsed: serde_json::Value = serde_json::from_str(value.get())?; + resolve_agent_on_behalf_of(tx, authed, db, w_id, path, &mut parsed, preserve, created).await?; + Ok(serde_json::value::to_raw_value(&parsed)?) +} + /// Write a resource's value and run everything that has to follow it: a version row, the audit /// entry, deployment metadata, the webhook, and dependent CI tests. Shared with version restore /// so a restored value is indistinguishable downstream from any other edit. @@ -2361,6 +2503,20 @@ async fn set_resource_value( let mut tx = user_db.clone().begin(authed).await?; + let mut value = value; + if let Some(v) = value.as_mut() { + let resource_type = sqlx::query_scalar!( + "SELECT resource_type FROM resource WHERE workspace_id = $1 AND path = $2", + w_id, + path + ) + .fetch_optional(&mut *tx) + .await?; + if resource_type.as_deref() == Some("ai_agent") { + resolve_agent_on_behalf_of(&mut tx, authed, db, w_id, path, v, false, false).await?; + } + } + // `RETURNING resource_type` rather than a second lookup: the advisory below has to know the // type to leave `state` and `cache` alone, and this statement already runs. let updated = sqlx::query_scalar!( diff --git a/cli/src/commands/resource/resource.ts b/cli/src/commands/resource/resource.ts index 7cef4caf1f..ef967cd441 100644 --- a/cli/src/commands/resource/resource.ts +++ b/cli/src/commands/resource/resource.ts @@ -20,6 +20,11 @@ import { Resource } from "../../../gen/types.gen.ts"; import { readInlinePathSync, readTextFile } from "../../utils/utils.ts"; import { isWorkspaceSpecificFile } from "../../core/specific_items.ts"; import { getCurrentGitBranch } from "../../utils/git.ts"; +import { + buildPermissionedAsContext, + type PermissionedAsContext, +} from "../../core/permissioned_as.ts"; +import { readEffectiveSyncBehavior } from "../../core/conf.ts"; export interface ResourceFile { value: any; @@ -83,6 +88,7 @@ export async function pushResource( // only round-trip the canonical layout); the standalone `resource push` // command pushes a single explicit file, where any pointer is fine. enforceCanonicalFileset?: boolean, + permissionedAsContext?: PermissionedAsContext, ): Promise { remotePath = removeType(remotePath, "resource"); try { @@ -93,6 +99,11 @@ export async function pushResource( } catch { // flow doesn't exist } + const claimed = agentOnBehalfOf( + resource, + localResource, + permissionedAsContext + ); // Helper function to resolve inline content const resolveInlineContent = async () => { @@ -133,11 +144,23 @@ export async function pushResource( // Only resolve inline content if we're actually updating await resolveInlineContent(); + if (claimed) { + log.info(`Preserving ${claimed} as permissioned_as for agent ${remotePath}`); + } await wmill.updateResource({ workspace: workspace, path: remotePath.replaceAll(SEP, "/"), - requestBody: { ...localResource, ...(wsSpecific !== undefined ? { ws_specific: wsSpecific } : {}) }, + requestBody: { + ...localResource, + ...(wsSpecific !== undefined ? { ws_specific: wsSpecific } : {}), + ...(claimed + ? { + value: { ...localResource.value, on_behalf_of: claimed }, + preserve_on_behalf_of: true, + } + : {}), + }, }); } else { // New resource - resolve inline content @@ -163,6 +186,41 @@ export async function pushResource( } } +/** + * An agent runs as the identity in its value's `on_behalf_of`, which the backend + * sets to whoever writes the agent. Like an app's, it is never taken from the + * tracked file, which the pull leaves it out of: the deployed one is claimed + * back only by a pusher the backend lets keep it, an admin or a `wm_deployers` + * member. Strips the key from `localResource` (and from the deployed value, so + * an unchanged agent still compares equal) and returns the identity to claim. + */ +export function agentOnBehalfOf( + deployed: ResourceFile | Resource | undefined, + localResource: ResourceFile, + permissionedAsContext: PermissionedAsContext | undefined +): string | undefined { + // What the push writes is the tracked file's type: an agent retyped into + // another resource keeps its value exactly as the file states it. + if (localResource.resource_type !== "ai_agent") { + return undefined; + } + const local = localResource.value; + if (local && typeof local === "object") { + delete local.on_behalf_of; + } + const value = deployed?.resource_type === "ai_agent" ? deployed.value : undefined; + const onBehalfOf = + value && typeof value === "object" ? value.on_behalf_of : undefined; + if (deployed && onBehalfOf !== undefined) { + const { on_behalf_of: _, ...rest } = value; + deployed.value = rest; + } + if (!onBehalfOf || !permissionedAsContext?.userIsAdminOrDeployer) { + return undefined; + } + return onBehalfOf; +} + type PushOptions = GlobalOptions; async function push(opts: PushOptions, filePath: string, remotePath: string) { const workspace = await resolveWorkspace(opts); @@ -184,7 +242,13 @@ async function push(opts: PushOptions, filePath: string, remotePath: string) { remotePath, undefined, parseFromFile(filePath), - filePath // Pass the local file path for branch-specific inline content resolution + filePath, // Pass the local file path for branch-specific inline content resolution + undefined, + undefined, + await buildPermissionedAsContext( + workspace.workspaceId, + await readEffectiveSyncBehavior(opts, workspace) + ) ); log.info(colors.bold.underline.green(`Resource ${remotePath} pushed`)); } diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 6185decc5a..15a4eda4a7 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -5918,6 +5918,7 @@ export async function push( resourceFilePath, isFileResWsSpecific ? true : undefined, true, + permissionedAsContext, ); } // Already-synced parents got the full content this run. diff --git a/cli/src/core/permissioned_as.ts b/cli/src/core/permissioned_as.ts index d0301373da..231da0c014 100644 --- a/cli/src/core/permissioned_as.ts +++ b/cli/src/core/permissioned_as.ts @@ -246,6 +246,15 @@ export async function preCheckPermissionedAs( if (match) { currentOwner = match[1]; } + } else if (typeStr === "resource") { + // An agent always runs as someone, and the file never says who. + if (/["']?resource_type["']?\s*:\s*["']?ai_agent\b/.test(beforeContent)) { + wouldChangeItems.push({ + path: change.path, + currentOwner: "(agent owner)", + }); + } + continue; } else if (typeStr.endsWith("_trigger")) { wouldChangeItems.push({ path: change.path, diff --git a/cli/src/types.ts b/cli/src/types.ts index 866b2be21e..1a7b895ffe 100644 --- a/cli/src/types.ts +++ b/cli/src/types.ts @@ -248,7 +248,7 @@ export async function pushObj( } else if (typeEnding === "resource") { if (!alreadySynced.includes(p)) { alreadySynced.push(p); - await pushResource(workspace, p, befObj, newObj, originalLocalPath || p, wsSpecific, true); + await pushResource(workspace, p, befObj, newObj, originalLocalPath || p, wsSpecific, true, permissionedAsContext); } } else if (typeEnding === "resource-type") { await pushResourceType(workspace, p, befObj, newObj); diff --git a/cli/test/deploy_on_behalf_of_unit.test.ts b/cli/test/deploy_on_behalf_of_unit.test.ts index 7e21b24832..a0db663589 100644 --- a/cli/test/deploy_on_behalf_of_unit.test.ts +++ b/cli/test/deploy_on_behalf_of_unit.test.ts @@ -105,3 +105,28 @@ test("deployItem: never sends the source workspace's on_behalf_of", async () => expect("on_behalf_of" in JSON.parse(JSON.stringify(identity))).toBe(false); } }); + +// An agent carries its principal inside its resource value, and is handed the chosen one as a +// principal. The source's must never be sent: without a choice the key is dropped, leaving the +// backend to make the agent the deployer's. +test("deployItem: an agent takes the chosen principal, never the source's", async () => { + const captured: any[] = []; + const provider = { + existsResource: async () => true, + getResource: async () => ({ + path: "f/x/agent", + resource_type: "ai_agent", + description: "", + value: { system_prompt: "hi", on_behalf_of: "u/alice" }, + }), + updateResource: async (p: any) => void captured.push(p.requestBody), + } as any; + + await deployItem(provider, "resource" as any, "f/x/agent", "src", "dst", "u/bob"); + await deployItem(provider, "resource" as any, "f/x/agent", "src", "dst", undefined); + + expect(captured[0].value).toEqual({ system_prompt: "hi", on_behalf_of: "u/bob" }); + expect(captured[0].preserve_on_behalf_of).toBe(true); + expect(captured[1].value).toEqual({ system_prompt: "hi" }); + expect(captured[1].preserve_on_behalf_of).toBe(false); +}); diff --git a/cli/test/resource_push_agent_obo_unit.test.ts b/cli/test/resource_push_agent_obo_unit.test.ts new file mode 100644 index 0000000000..d9b8361c8a --- /dev/null +++ b/cli/test/resource_push_agent_obo_unit.test.ts @@ -0,0 +1,57 @@ +/** + * An agent's run-as identity lives in its value, which the pull leaves out. A + * push must neither see that gap as a change nor let the file pick who the + * agent runs as: only the deployed identity is claimed back, and only by a + * pusher the backend lets keep it. + */ + +import { expect, test } from "bun:test"; +import { isSuperset } from "../src/types.ts"; +import { agentOnBehalfOf } from "../src/commands/resource/resource.ts"; + +const ctx = (userIsAdminOrDeployer: boolean) => ({ + userCache: new Map(), + userIsAdminOrDeployer, + userEmail: "me@x.dev", +}); + +function agents(localObo?: string) { + const deployed = { + resource_type: "ai_agent", + value: { system_prompt: "hi", on_behalf_of: "u/bob" }, + } as any; + const local = { + resource_type: "ai_agent", + value: { system_prompt: "hi", ...(localObo ? { on_behalf_of: localObo } : {}) }, + } as any; + return { deployed, local }; +} + +test("an unchanged agent still compares equal to the deployed one", () => { + const { deployed, local } = agents(); + agentOnBehalfOf(deployed, local, ctx(true)); + expect(isSuperset(local, deployed)).toBe(true); +}); + +test("an admin or deployer claims the deployed identity, never the file's", () => { + const { deployed, local } = agents("u/admin"); + expect(agentOnBehalfOf(deployed, local, ctx(true))).toBe("u/bob"); + expect(local.value.on_behalf_of).toBeUndefined(); +}); + +test("anyone else claims nothing", () => { + const { deployed, local } = agents("u/bob"); + expect(agentOnBehalfOf(deployed, local, ctx(false))).toBeUndefined(); + expect(agentOnBehalfOf(agents().deployed, agents().local, undefined)).toBeUndefined(); + expect(local.value.on_behalf_of).toBeUndefined(); +}); + +test("other resources are left alone, an agent retyped into one included", () => { + for (const deployedType of ["json", "ai_agent"]) { + const deployed = { resource_type: deployedType, value: { on_behalf_of: "u/bob" } } as any; + const local = { resource_type: "json", value: { on_behalf_of: "u/x" } } as any; + expect(agentOnBehalfOf(deployed, local, ctx(true))).toBeUndefined(); + expect(local.value.on_behalf_of).toBe("u/x"); + expect(deployed.value.on_behalf_of).toBe("u/bob"); + } +}); diff --git a/cli/windmill-utils-internal/src/deploy.ts b/cli/windmill-utils-internal/src/deploy.ts index ac0b775e86..d6e48f9663 100644 --- a/cli/windmill-utils-internal/src/deploy.ts +++ b/cli/windmill-utils-internal/src/deploy.ts @@ -630,14 +630,25 @@ export async function deployItem( workspace: workspaceFrom, path, }); + let value = resource.value ?? ""; + // An agent's value carries the principal it runs as, as a trigger's row does: the source's + // names nobody in the target, so it is replaced by the chosen one or dropped, leaving the + // backend to make it the deployer's. + const agent = resource.resource_type === "ai_agent" && typeof value === "object"; + if (agent) { + const { on_behalf_of: _, ...rest } = value; + value = onBehalfOf ? { ...rest, on_behalf_of: onBehalfOf } : rest; + } + const agentPreserve = agent ? { preserve_on_behalf_of: preserveOnBehalfOf } : {}; if (alreadyExists) { await provider.updateResource({ workspace: workspaceTo, path, requestBody: { path, - value: resource.value ?? "", + value, description: resource.description ?? "", + ...agentPreserve, }, }); } else { @@ -645,9 +656,10 @@ export async function deployItem( workspace: workspaceTo, requestBody: { path, - value: resource.value ?? "", + value, resource_type: resource.resource_type, description: resource.description ?? "", + ...agentPreserve, }, }); } @@ -902,6 +914,11 @@ export async function getItemValue( return variable.value; } else if (kind === "resource") { const resource = await provider.getResource({ workspace, path }); + // Each workspace resolves its own agent identity, which a deploy never copies. + if (resource.resource_type === "ai_agent" && resource.value) { + const { on_behalf_of: _, ...value } = resource.value; + return value; + } return resource.value; } else if (kind === "resource_type") { const rt = await provider.getResourceType({ workspace, path }); @@ -987,7 +1004,8 @@ function stripTriggerOrScheduleRuntimeFields(row: unknown): unknown { /** * Fetch the on_behalf_of value for a deployable item. - * Returns an email for flows/scripts/apps, or undefined if not applicable. + * Returns an email for flows/scripts/apps, a principal (u/username, g/group) for + * triggers/schedules and agents, or undefined if not applicable. */ export async function getOnBehalfOf( provider: DeployProvider, @@ -1005,6 +1023,12 @@ export async function getOnBehalfOf( } else if (kind === "app" || kind === "raw_app") { const app = await provider.getAppByPath({ workspace, path }); return app.policy?.on_behalf_of_email; + } else if (kind === "resource") { + // Only an agent runs as anyone, and like a trigger it records the principal. + const resource = await provider.getResource({ workspace, path }); + return resource.resource_type === "ai_agent" + ? resource.value?.on_behalf_of + : undefined; } else if (kind === "schedule") { const schedule = await provider.getSchedule({ workspace, path }); return schedule.permissioned_as; diff --git a/frontend/src/lib/components/CompareWorkspaces.svelte b/frontend/src/lib/components/CompareWorkspaces.svelte index 375e73480e..67e44add03 100644 --- a/frontend/src/lib/components/CompareWorkspaces.svelte +++ b/frontend/src/lib/components/CompareWorkspaces.svelte @@ -554,12 +554,13 @@ } async function fetchOnBehalfOfInfo(diffs: WorkspaceItemDiff[]) { - // Runnables carry an email; triggers/schedules carry `permissioned_as`. - // `getOnBehalfOf` handles the dispatch — we just need to skip kinds that - // have no on_behalf_of concept (resource, variable, folder, resource_type). + // Runnables carry an email; triggers/schedules and agents carry `permissioned_as`. + // `getOnBehalfOf` handles the dispatch (a resource that is not an agent has none) — we + // just need to skip kinds that have no on_behalf_of concept (variable, folder, resource_type). const itemsWithOnBehalfOf = diffs.filter( (d) => - ['flow', 'script', 'app', 'raw_app'].includes(d.kind) || isTriggerOrScheduleKind(d.kind) + ['flow', 'script', 'app', 'raw_app', 'resource'].includes(d.kind) || + isTriggerOrScheduleKind(d.kind) ) for (const diff of itemsWithOnBehalfOf) { for (const workspace of [currentWorkspaceId, parentWorkspaceId]) { @@ -607,17 +608,22 @@ }) ) + /** Triggers, schedules and agents record who they run as by principal, the rest by address. */ + function usesPrincipal(kind: Kind): boolean { + return kind === 'trigger' || isTriggerOrScheduleKind(kind) || kind === 'resource' + } + /** * Get the on_behalf_of value for deployment based on user's choice. - * Returns an email for flows/scripts/apps, or permissioned_as (u/username, g/group) for triggers/schedules. + * Returns an email for flows/scripts/apps, or permissioned_as (u/username, g/group) for + * triggers/schedules and agents. */ function getOnBehalfOfForDeploy(itemKey: string, kind: Kind): string | undefined { const choice = onBehalfOfChoice[itemKey] if (choice === 'target') return getTargetOnBehalfOf(itemKey) if (choice === 'custom') { const details = customOnBehalfOf[itemKey] - const wantsPermissionedAs = kind === 'trigger' || isTriggerOrScheduleKind(kind) - return wantsPermissionedAs ? details?.permissionedAs : details?.email + return usesPrincipal(kind) ? details?.permissionedAs : details?.email } // 'me' or undefined = don't pass, backend will use deploying user's identity return undefined @@ -629,7 +635,7 @@ * returning undefined clears the source item's value rather than keeping it. */ function getOnBehalfOfPermissionedAsForDeploy(itemKey: string, kind: Kind): string | undefined { - if (kind === 'trigger' || isTriggerOrScheduleKind(kind)) return undefined + if (usesPrincipal(kind)) return undefined if (onBehalfOfChoice[itemKey] !== 'custom') return undefined return customOnBehalfOf[itemKey]?.permissionedAs } diff --git a/frontend/src/lib/components/DeployWorkspace.svelte b/frontend/src/lib/components/DeployWorkspace.svelte index a01e845246..778b47543d 100644 --- a/frontend/src/lib/components/DeployWorkspace.svelte +++ b/frontend/src/lib/components/DeployWorkspace.svelte @@ -108,16 +108,22 @@ return needsOnBehalfOfSelection(kind, sourceOnBehalfOfInfo[statusPath]) } + /** Triggers and agents record who they run as by principal, the rest by address. */ + function usesPrincipal(kind: Kind): boolean { + return kind === 'trigger' || kind === 'resource' + } + /** * Get the on_behalf_of value for deployment based on user's choice. - * Returns an email for flows/scripts/apps, or permissioned_as (u/username, g/group) for triggers/schedules. + * Returns an email for flows/scripts/apps, or permissioned_as (u/username, g/group) for + * triggers/schedules and agents. */ function getOnBehalfOfForDeploy(statusPath: string, kind: Kind): string | undefined { const choice = onBehalfOfChoice[statusPath] if (choice === 'target') return targetOnBehalfOfInfo[statusPath] if (choice === 'custom') { const details = customOnBehalfOf[statusPath] - return kind === 'trigger' ? details?.permissionedAs : details?.email + return usesPrincipal(kind) ? details?.permissionedAs : details?.email } // 'me' or undefined = don't pass, backend will use deploying user's identity return undefined @@ -132,7 +138,7 @@ statusPath: string, kind: Kind ): string | undefined { - if (kind === 'trigger' || onBehalfOfChoice[statusPath] !== 'custom') return undefined + if (usesPrincipal(kind) || onBehalfOfChoice[statusPath] !== 'custom') return undefined return customOnBehalfOf[statusPath]?.permissionedAs } @@ -203,7 +209,7 @@ // Fetch on_behalf_of_email for flows, scripts, apps, and triggers from both workspaces for (const dep of sortedSet.filter((d) => - ['flow', 'script', 'app', 'trigger'].includes(d.kind) + ['flow', 'script', 'app', 'trigger', 'resource'].includes(d.kind) )) { const key = computeStatusPath(dep.kind, dep.path) let source: string | undefined diff --git a/frontend/src/lib/components/OnBehalfOfSelector.svelte b/frontend/src/lib/components/OnBehalfOfSelector.svelte index 67648936ae..fb8586b2c9 100644 --- a/frontend/src/lib/components/OnBehalfOfSelector.svelte +++ b/frontend/src/lib/components/OnBehalfOfSelector.svelte @@ -19,7 +19,9 @@ kind === 'app' || kind === 'raw_app' || kind === 'trigger' || - isTriggerOrScheduleKind(kind) + isTriggerOrScheduleKind(kind) || + // Only an agent's value carries an identity, so no other resource has a source value. + kind === 'resource' if (!supported) return false return !!sourceValue } diff --git a/frontend/src/lib/components/flows/agentDraft.svelte.ts b/frontend/src/lib/components/flows/agentDraft.svelte.ts index b76f9bb471..a7e44941f3 100644 --- a/frontend/src/lib/components/flows/agentDraft.svelte.ts +++ b/frontend/src/lib/components/flows/agentDraft.svelte.ts @@ -144,18 +144,29 @@ export function agentDraftDeployRefusal( */ type AgentWriteResult = { ok: true } | { ok: false; error: string; notAnAgent?: true } +/** Who a deploy makes the agent run as. The server resolves it as it does a flow's: without + * `preserve`, or from someone not allowed to keep another identity, it is the deployer. */ +export interface AgentRunAs { + permissionedAs: string | undefined + preserve: boolean +} + async function writeAgentResource( workspace: string, fromPath: string, state: AgentResourceState, - noDeployed: boolean + noDeployed: boolean, + runAs?: AgentRunAs ): Promise { const body = { path: state.path, - value: state.args, + value: runAs?.permissionedAs + ? { ...state.args, on_behalf_of: runAs.permissionedAs } + : state.args, description: state.description, labels: state.labels, - ws_specific: state.wsSpecific + ws_specific: state.wsSpecific, + preserve_on_behalf_of: runAs?.preserve || undefined } try { if (noDeployed) { @@ -221,9 +232,12 @@ export interface AgentDraftHandle { /** Why this path cannot be edited here, if it cannot. Render it instead of the form. */ readonly refusal: string | undefined readonly sync: TriggerDraftSync + /** Who the deployed agent runs as when run from its page; undefined when nothing is deployed + * or it runs as whoever runs it. */ + readonly onBehalfOf: string | undefined /** Write the current state to the resource and drop the draft. Resolves to the path written, * which differs from the one loaded when the draft renames the agent, or undefined on failure. */ - deploy: () => Promise + deploy: (runAs?: AgentRunAs) => Promise } /** @@ -237,6 +251,7 @@ export function useAgentDraft(opts: AgentDraftOptions): AgentDraftHandle { let loading = $state(true) let noDeployed = $state(false) let canWriteResource = $state(true) + let onBehalfOf = $state(undefined) /** Guards the load against a path that changed under a slow response. */ let loadedFor = $state(undefined) /** Why the loaded path cannot be edited here, if it cannot. Gates the sync for as long as that @@ -324,10 +339,15 @@ export function useAgentDraft(opts: AgentDraftOptions): AgentDraftHandle { return } noDeployed = Boolean((r as any).no_deployed) + // Who it runs as is the deploy's to decide, not a setting the form or a draft holds. + const { on_behalf_of, ...args } = (r.value ?? {}) as AIAgentConfig & { + on_behalf_of?: string + } + onBehalfOf = noDeployed ? undefined : on_behalf_of const deployedState: AgentResourceState = { path: r.path, description: r.description ?? '', - args: (r.value ?? {}) as AIAgentConfig, + args, // Only where nothing else answers for the type: with a deployed row both the // create below and the review page's `deployDraft` read it from there, and // carrying it would make every draft the generic resource editor writes — which @@ -408,7 +428,7 @@ export function useAgentDraft(opts: AgentDraftOptions): AgentDraftHandle { }) }) - async function deploy(): Promise { + async function deploy(runAs?: AgentRunAs): Promise { const ws = opts.workspace() const fromPath = opts.path() const s = state @@ -424,7 +444,7 @@ export function useAgentDraft(opts: AgentDraftOptions): AgentDraftHandle { // made during the request as saved, and the banner would clear on a value the server never // received; against the snapshot it stays a draft, which is what it is. const submitted = structuredClone($state.snapshot(s)) as AgentResourceState - const written = await writeAgentResource(ws, fromPath, submitted, noDeployed) + const written = await writeAgentResource(ws, fromPath, submitted, noDeployed, runAs) if (!written.ok) { // A path that is no longer an agent tears this editor down; anything else is a plain error // the user can retry from the form as it stands. @@ -440,6 +460,10 @@ export function useAgentDraft(opts: AgentDraftOptions): AgentDraftHandle { logReusableAgentUsage(noDeployed ? 'saved' : 'updated') deployed = submitted noDeployed = false + // What the server resolved, which a request to keep an identity does not decide alone. + void ResourceService.getResource({ workspace: ws, path: submitted.path }) + .then((r) => (onBehalfOf = (r.value as { on_behalf_of?: string } | undefined)?.on_behalf_of)) + .catch(() => {}) const renamed = submitted.path !== fromPath // Only when the form still holds exactly what was sent. `discard` resets the handle's cell to // what it is given, and the apply-effect copies that back over the form: against an edit made @@ -479,6 +503,9 @@ export function useAgentDraft(opts: AgentDraftOptions): AgentDraftHandle { get refusal() { return refusal }, + get onBehalfOf() { + return onBehalfOf + }, sync, deploy } diff --git a/frontend/src/lib/components/flows/content/AgentEditorHost.svelte b/frontend/src/lib/components/flows/content/AgentEditorHost.svelte index b9521683b1..2d903a0811 100644 --- a/frontend/src/lib/components/flows/content/AgentEditorHost.svelte +++ b/frontend/src/lib/components/flows/content/AgentEditorHost.svelte @@ -51,7 +51,8 @@ keepsManagedMemory } from '../agentFormFields' import { toolDisplayName, type AgentTool } from '../agentToolUtils' - import { useAgentDraft } from '../agentDraft.svelte' + import { useAgentDraft, type AgentRunAs } from '../agentDraft.svelte' + import PermissionedAsLine from '$lib/components/triggers/PermissionedAsLine.svelte' import Path from '$lib/components/Path.svelte' import Label from '$lib/components/Label.svelte' import { sendUserToast } from '$lib/toast' @@ -444,13 +445,14 @@ let pathError = $state('') /** Who the next deploy makes the agent run as, as the "Permissioned as" line picked it. */ + let runAs = $state(undefined) export function deploy(): Promise { if (pathError) { sendUserToast(`Cannot deploy the agent: ${pathError}`, true) return Promise.resolve(false) } - return draft.deploy().then(async (written) => { + return draft.deploy(runAs).then(async (written) => { // The path the write landed on, which a rename moves off the one this editor opened. if (written) await onSaved?.(written) return written !== undefined @@ -566,13 +568,17 @@ disabled={readOnly} /> - - {#if !readOnly && draft.state?.path?.startsWith('f/')} - - Anyone who runs this agent needs access to its AI resource, and to the resources and - workspace scripts its tools use. - + + {#if !readOnly} +
+ + (runAs = { permissionedAs, preserve })} + /> +
{/if} {/if} diff --git a/frontend/src/lib/utils_draft_deploy.test.ts b/frontend/src/lib/utils_draft_deploy.test.ts index 5f503989c8..6eddfe093d 100644 --- a/frontend/src/lib/utils_draft_deploy.test.ts +++ b/frontend/src/lib/utils_draft_deploy.test.ts @@ -122,6 +122,25 @@ describe('deployDraft preserves on_behalf_of', () => { }) ) }) + + it('agent: claims the deployed identity, which its draft never carries', async () => { + vi.mocked(ResourceService.getResource).mockResolvedValueOnce({ + path: 'f/support/triage_agent', + resource_type: 'ai_agent', + value: { system_prompt: 'deployed', on_behalf_of: 'u/alice' }, + draft: { path: 'f/support/triage_agent', args: { system_prompt: 'drafted' } } + } as any) + + expect(await deployDraft('resource', 'f/support/triage_agent', 'ws')).toEqual({ success: true }) + expect(ResourceService.updateResource).toHaveBeenCalledWith( + expect.objectContaining({ + requestBody: expect.objectContaining({ + value: { system_prompt: 'drafted', on_behalf_of: 'u/alice' }, + preserve_on_behalf_of: true + }) + }) + ) + }) }) // The resource branch reads the item again when the deploy lands, and falls back to the deployed diff --git a/frontend/src/lib/utils_draft_deploy.ts b/frontend/src/lib/utils_draft_deploy.ts index 31b973f9a5..3614b344e2 100644 --- a/frontend/src/lib/utils_draft_deploy.ts +++ b/frontend/src/lib/utils_draft_deploy.ts @@ -233,8 +233,14 @@ function canonicalizeDraftDiffValue(kind: DraftKind, raw: any, isDraft: boolean) if (kind === 'resource') { // draft: { args, description, resource_type, labels, wsSpecific } // deployed row: { value, description, resource_type, labels, ws_specific } + // An agent's identity is in its deployed value and never in a draft, which is not an edit. + let value = (isDraft ? raw.args : raw.value) ?? {} + if (raw.resource_type === 'ai_agent') { + const { on_behalf_of: _, ...rest } = value + value = rest + } return { - value: (isDraft ? raw.args : raw.value) ?? {}, + value, description: raw.description ?? '', resource_type: raw.resource_type ?? undefined, labels: raw.labels ?? undefined, @@ -639,12 +645,18 @@ export async function deployDraft( } }) } else { + // As for an app: an agent draft carries no identity, so the deployed one is kept (the + // backend resets it to the deploying user without the flag, gated by + // can_preserve_on_behalf_of). + const agentObo = + deployed.resource_type === 'ai_agent' ? deployed.value?.on_behalf_of : undefined await ResourceService.updateResource({ workspace, path, requestBody: { path: d.path ?? path, - value: d.args ?? {}, + value: agentObo ? { ...(d.args ?? {}), on_behalf_of: agentObo } : (d.args ?? {}), + preserve_on_behalf_of: agentObo ? true : undefined, description: d.description ?? '', labels: d.labels, ws_specific: d.wsSpecific diff --git a/frontend/src/lib/utils_workspace_deploy.ts b/frontend/src/lib/utils_workspace_deploy.ts index d56c1f6541..ff42e7ba4e 100644 --- a/frontend/src/lib/utils_workspace_deploy.ts +++ b/frontend/src/lib/utils_workspace_deploy.ts @@ -196,6 +196,11 @@ export type DeployConflict = { hit: boolean } * A group kept as the target identity travels as its synthetic `group-*@windmill.dev` * address, which an admin-created account holding it would win on the backend: known and * accepted, see `users::permissioned_as_from_email`. + * + * An agent carries its principal inside its resource value, and is handed the chosen one as a + * principal, as a trigger is. Every read here drops the source's, so a diff never shows it and + * the published `deployItem` copies none; the write then stamps the chosen one, or none, which + * leaves the backend to make it the deployer's. */ function makeProvider( onBehalfOfPrincipal?: string, @@ -224,6 +229,17 @@ function makeProvider( } } : app + const agents = new Set() + const withAgentIdentity = >(requestBody: T, path: string): T => + agents.has(path) && requestBody.value && typeof requestBody.value === 'object' + ? { + ...requestBody, + value: onBehalfOf + ? { ...requestBody.value, on_behalf_of: onBehalfOf } + : requestBody.value, + preserve_on_behalf_of: onBehalfOf !== undefined + } + : requestBody const refuseUpdate = (): never => { if (conflict) conflict.hit = true throw new Error('item already exists in the target workspace') @@ -276,9 +292,23 @@ function makeProvider( createVariable: (p) => VariableService.createVariable(p), updateVariable: (p) => VariableService.updateVariable(p), deleteVariable: (p) => VariableService.deleteVariable(p), - getResource: (p) => ResourceService.getResource(p), - createResource: (p) => ResourceService.createResource(p), - updateResource: (p) => ResourceService.updateResource(p), + getResource: async (p) => { + const resource = await ResourceService.getResource(p) + if (resource.resource_type !== 'ai_agent' || !resource.value) return resource + agents.add(p.path) + const { on_behalf_of: _, ...value } = resource.value as Record + return { ...resource, value } + }, + createResource: (p) => + ResourceService.createResource({ + ...p, + requestBody: withAgentIdentity(p.requestBody, p.requestBody.path) + }), + updateResource: (p) => + ResourceService.updateResource({ + ...p, + requestBody: withAgentIdentity(p.requestBody, p.path) + }), deleteResource: (p) => ResourceService.deleteResource(p), getResourceType: (p) => ResourceService.getResourceType(p), createResourceType: (p) => ResourceService.createResourceType(p), @@ -569,6 +599,17 @@ export async function getOnBehalfOf( return undefined } } + if (kind === 'resource') { + // Only an agent runs as anyone, and like a trigger it records the principal. + try { + const resource = await ResourceService.getResource({ workspace, path }) + return resource.resource_type === 'ai_agent' + ? (resource.value as { on_behalf_of?: string } | undefined)?.on_behalf_of + : undefined + } catch { + return undefined + } + } return sharedGetOnBehalfOf(makeProvider(), kind as DeployKind, path, workspace) }