diff --git a/backend/windmill-api-integration-tests/tests/operator_manage_rights.rs b/backend/windmill-api-integration-tests/tests/operator_manage_rights.rs index ae1979f2fb..ac2b5d8312 100644 --- a/backend/windmill-api-integration-tests/tests/operator_manage_rights.rs +++ b/backend/windmill-api-integration-tests/tests/operator_manage_rights.rs @@ -4,10 +4,10 @@ use sqlx::{Pool, Postgres}; use windmill_common::workspaces::invalidate_operator_rights_cache; use windmill_test_utils::*; -/// Every test here must be `#[serial]`. The rights cache is process-global and keyed by workspace -/// id alone, while `sqlx::test` gives each test its own database under this same id — so two -/// running at once can answer each other's reads from one cached entry, and a withdrawal in one -/// database reads as granted in the other. +/// Every test here must be `#[serial]`, and start by dropping the cache entry. The rights cache is +/// process-global and keyed by workspace id alone, while `sqlx::test` gives each test its own +/// database under this same id — so two running at once can answer each other's reads from one +/// cached entry, and a withdrawal a test ends on reads as withdrawn in the next one's database. const WS: &str = "test-workspace"; fn operator_client() -> reqwest::Client { @@ -53,6 +53,7 @@ async fn set_settings(api: &str, body: serde_json::Value) -> anyhow::Result #[serial] async fn test_operator_manage_rights(db: Pool) -> anyhow::Result<()> { initialize_tracing().await; + invalidate_operator_rights_cache(WS); let server = ApiServer::start(db.clone()).await?; let port = server.addr.port(); let api = format!("http://localhost:{port}/api/w/{WS}"); @@ -145,6 +146,7 @@ async fn test_manage_triggers_covers_a_route_outside_the_shared_handler( db: Pool, ) -> anyhow::Result<()> { initialize_tracing().await; + invalidate_operator_rights_cache(WS); let server = ApiServer::start(db.clone()).await?; let port = server.addr.port(); let api = format!("http://localhost:{port}/api/w/{WS}"); @@ -213,3 +215,61 @@ async fn test_manage_triggers_covers_a_route_outside_the_shared_handler( Ok(()) } + +/// A draft is what the AI chat writes before it deploys, so an operator holding a manage right +/// may draft that kind, and only while they hold it. Builder rights are left off throughout: they +/// open flow drafts, and would otherwise be what admits these. +#[sqlx::test(migrations = "../migrations", fixtures("base", "permissions_test"))] +#[serial] +async fn test_manage_rights_admit_matching_drafts(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + invalidate_operator_rights_cache(WS); + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let api = format!("http://localhost:{port}/api/w/{WS}"); + let c = operator_client(); + + let save = async |kind: &str, path: &str| -> anyhow::Result<(u16, String)> { + let resp = c + .post(format!("{api}/drafts/update/{kind}/{path}")) + .json(&json!({ "value": { "path": path } })) + .send() + .await?; + Ok((resp.status().as_u16(), resp.text().await?)) + }; + let listed = async || -> anyhow::Result { + let resp = c.get(format!("{api}/drafts/list")).send().await?; + assert_eq!(resp.status(), 200); + Ok(resp.json().await?) + }; + + let (status, body) = save("trigger_schedule", "u/operator/sched_draft").await?; + assert_eq!(status, 200, "{body}"); + let (status, body) = save("trigger_http", "u/operator/http_draft").await?; + assert_eq!(status, 200, "{body}"); + // The rights name schedules and triggers, nothing else. + let (status, body) = save("script", "u/operator/script_draft").await?; + assert_eq!(status, 403, "{body}"); + + let drafts = listed().await?; + assert_eq!(drafts.as_array().map(Vec::len), Some(2), "{drafts}"); + + assert_eq!( + set_settings(&api, json!({"manage_schedules": false})).await?, + 200 + ); + let (status, body) = save("trigger_schedule", "u/operator/sched_draft").await?; + assert_eq!(status, 403, "{body}"); + let (status, body) = save("trigger_http", "u/operator/http_draft").await?; + assert_eq!(status, 200, "{body}"); + + assert_eq!( + set_settings(&api, json!({"manage_triggers": false})).await?, + 200 + ); + let (status, body) = save("trigger_http", "u/operator/http_draft").await?; + assert_eq!(status, 403, "{body}"); + assert_eq!(listed().await?, json!([])); + + Ok(()) +} diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index a3cd34f3f8..bd8f4ff307 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -23,7 +23,7 @@ use windmill_common::{ users::resolve_username_to_email, utils::{check_proper_path, strip_json_nul}, variables::{build_crypt, encrypt}, - workspaces::operator_can_build_flows, + workspaces::operator_can_draft, }; pub fn workspaced_service() -> Router { @@ -104,10 +104,10 @@ async fn list_drafts( Path(w_id): Path, Query(query): Query, ) -> Result>> { - // Without builder rights an operator has no drafts of their own (they can't write any, see + // An operator who may draft no kind at all has no drafts of their own (see // `require_can_write_path`), so this list is always empty for them. They can still READ some // collaborators' drafts via `/drafts/get`. - if authed.is_operator && !operator_can_build_flows(&db, &w_id).await? { + if authed.is_operator && !operator_can_draft(&db, &w_id, None).await? { return Ok(Json(vec![])); } let all_users = query.all_users.unwrap_or(false); @@ -1143,8 +1143,7 @@ fn table_for_kind(kind: UserDraftItemKind) -> Option<&'static str> { } /// Resolves to `Ok(())` if `authed` may SAVE a draft at `path`. Operators are -/// rejected, except for flow drafts in a workspace that granted them builder -/// rights. Two layers: +/// rejected, except for a kind they could deploy (`operator_can_draft`). Two layers: /// 1. Claim-based namespace rules (admin, own `u/`, member `g/`, writable /// `f/`) — mirror what RLS reads from the same JWT claims, and are the /// ENTIRE check for draft-only paths (no deployed row for RLS to use). @@ -1161,19 +1160,16 @@ pub(crate) async fn require_can_write_path( if authed.is_admin { return Ok(()); } - // Operators are read-only and never WRITE drafts, except a flow draft where the workspace - // granted the builder right: the kind has to be checked, or the right would open drafts of - // kinds it says nothing about. Read access is deliberately asymmetric: + // Operators WRITE drafts only of a kind they could deploy: flows under the builder right, + // schedules and triggers under the manage rights. The kind has to be checked, or one right + // would open drafts of kinds it says nothing about. Read access is deliberately asymmetric: // `require_can_read_path` has no operator block, so an operator can still READ a draft they // can read via `/drafts/get`, mirroring their read access to deployed content. Intended. - if authed.is_operator { - let granted = - matches!(kind, UserDraftItemKind::Flow) && operator_can_build_flows(db, w_id).await?; - if !granted { - return Err(Error::PermissionDenied( - "operators cannot save drafts".to_string(), - )); - } + if authed.is_operator && !operator_can_draft(db, w_id, Some(kind)).await? { + return Err(Error::PermissionDenied(format!( + "operators cannot save {} drafts in this workspace", + kind.as_str() + ))); } // Cheap claim-based namespace checks first: they evaluate the same JWT // claims RLS reads, so the outcome matches the policies while sparing the diff --git a/backend/windmill-common/src/user_drafts.rs b/backend/windmill-common/src/user_drafts.rs index ebd0ce363f..cd7d6e0959 100644 --- a/backend/windmill-common/src/user_drafts.rs +++ b/backend/windmill-common/src/user_drafts.rs @@ -18,6 +18,7 @@ pub use windmill_types::user_drafts::DraftUserRef; use crate::db::DB; use crate::error::Result; +use crate::workspaces::ManageKind; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; @@ -160,6 +161,21 @@ impl UserDraftItemKind { } } + /// The withdrawable operator right that governs deploying this kind, which is + /// therefore also the right to draft it. Every trigger kind maps here, native ones + /// included: `gate_operator_writes` covers their routers as well. + pub fn manage_kind(&self) -> Option { + use UserDraftItemKind::*; + match self { + TriggerSchedule => Some(ManageKind::Schedules), + TriggerWebhook | TriggerDefaultEmail | TriggerEmail | TriggerHttp + | TriggerWebsocket | TriggerPostgres | TriggerKafka | TriggerNats | TriggerMqtt + | TriggerAmqp | TriggerSqs | TriggerGcp | TriggerAzure | TriggerPoll | TriggerCli + | TriggerNextcloud | TriggerGoogle | TriggerGithub => Some(ManageKind::Triggers), + Script | Flow | App | RawApp | Resource | Variable | DataPipeline => None, + } + } + /// The `draft.value` key holding the user-typed target path — where a /// deploy of this draft would land when the user staged a rename. A script /// draft round-trips its own `path`; flows and apps write a separate diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 7baad65a60..a8862e5ec2 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1322,6 +1322,23 @@ pub async fn operator_can_build_flows(db: &DB, workspace_id: &str) -> Result, +) -> Result { + use crate::user_drafts::UserDraftItemKind; + let rights = operator_rights(db, workspace_id).await?; + Ok(match kind { + None => rights.builder_flows || rights.manage.schedules || rights.manage.triggers, + Some(UserDraftItemKind::Flow) => rights.builder_flows, + Some(kind) => kind.manage_kind().is_some_and(|m| rights.manage.has(m)), + }) +} + /// Gate for a write only a workspace that granted builder rights lets operators perform. `action` /// completes "Operators cannot {action} for security reasons". pub async fn check_operator_can_build_flows( diff --git a/docs/operator-write-rights.md b/docs/operator-write-rights.md index 2c18ecd290..e0725b7268 100644 --- a/docs/operator-write-rights.md +++ b/docs/operator-write-rights.md @@ -54,6 +54,23 @@ It refuses with `PermissionDenied` (403), never `NotAuthorized` (401): the front uncaught 401 as an expired session and logs the user out, so 401 here ejects an operator from the app rather than telling them why. Both integration tests assert the status for that reason. +## Drafts + +An operator may save a draft of a kind they could deploy, and no other: a schedule draft while +they hold `manage_schedules`, a trigger draft (every trigger kind, native ones included) while +they hold `manage_triggers`, a flow draft under `builder_flows`. drafts.rs `require_can_write_path` +asks `operator_can_draft`, which reads the same cached rights as the router gate, and +`UserDraftItemKind::manage_kind` maps each draft kind to its right. Resource and variable drafts +stay refused even though operators edit those items directly. The AI session chat writes +schedules and triggers only as drafts and then deploys them, so without this rule an operator who +may create a schedule on its page could not ask the chat for one. + +Deploying a draft goes through the ordinary create and update routes, so the router gate is still +what decides the write. The draft rule only has to agree with it: a draft the gate would refuse to +deploy should not be saved in the first place. The frontend mirrors the rule in `roleCanDraft` +(`editRights.ts`), which the chat's `write_schedule_draft` / `write_trigger_draft` capabilities +come from. + ## In the UI The shared lists (`TriggerList`, `SchedulesList`, `NativeTriggerTable`) derive a per-row `canEdit` diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index a042c28f44..acae26c061 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -217,6 +217,8 @@ import { RUN_PREVIEW, WRITE_DRAFT, WRITE_FLOW_DRAFT, + WRITE_SCHEDULE_DRAFT, + WRITE_TRIGGER_DRAFT, type SessionAccess, type SessionTool, type SessionToolPolicy @@ -1425,10 +1427,26 @@ const buildGlobalSystemPrompt = ( // rule cannot outlive the tool it describes. An unresolved profile keeps every block. const canWriteDraft = !access || access.has('write_draft') const canWriteFlowDraft = !access || access.has('write_flow_draft') - // A builder operator: flow drafts and nothing else. Blocks about drafting in general + const canDraftSchedules = !access || access.has('write_schedule_draft') + const canDraftTriggers = !access || access.has('write_trigger_draft') + // A builder operator: of the editor kinds, flow drafts only. Blocks about drafting in general // follow `canWriteAnyDraft`; blocks naming scripts, apps or the other kinds keep `canWriteDraft`. const flowsOnly = canWriteFlowDraft && !canWriteDraft - const canWriteAnyDraft = canWriteDraft || canWriteFlowDraft + const canWriteAnyDraft = + canWriteDraft || canWriteFlowDraft || canDraftSchedules || canDraftTriggers + // Both read only where `write_draft` is missing: it implies the two draft rights. + const draftedOperatorKinds = [canDraftSchedules && 'schedules', canDraftTriggers && 'triggers'] + .filter((k): k is string => !!k) + .join(' and ') + const refusedKinds = [ + !flowsOnly && 'flows', + 'scripts', + 'apps', + !canDraftSchedules && 'schedules', + !canDraftTriggers && 'triggers', + 'resources', + 'variables' + ].filter((k): k is string => !!k) const canRunPreview = !access || access.has('run_preview') const canManageSchedules = !access || access.has('manage_schedules') const canManageTriggers = !access || access.has('manage_triggers') @@ -1472,9 +1490,18 @@ The current user's workspace username is "${username}".${instanceLine} ${ canWriteDraft ? 'Use tools to inspect workspace items and create per-user drafts (saved server-side, visible only to this user — not deployed) for scripts, flows, schedules, triggers, resources, variables, and raw apps.' - : `Use tools to inspect workspace items and the workspace's run history, and to run items that are already deployed. You cannot create or edit ${flowsOnly ? '' : 'flows, '}scripts, apps, schedules, triggers, resources or variables here — this user's role does not allow it — so when they ask for such a change, say plainly that you cannot make it rather than describing steps as if you had. Their role is refused ${flowsOnly ? 'scripts and apps' : 'scripts, flows and apps'} outside this chat too, so for those suggest asking a workspace admin rather than creating them in the editor. Resources and variables they can still edit themselves on their pages, so when open_page lists that page, open it for them instead.${when( + : `Use tools to inspect workspace items and the workspace's run history, and to run items that are already deployed. You cannot create or edit ${refusedKinds.slice(0, -1).join(', ')} or ${refusedKinds.at(-1)} here — this user's role does not allow it — so when they ask for such a change, say plainly that you cannot make it rather than describing steps as if you had. Their role is refused ${flowsOnly ? 'scripts and apps' : 'scripts, flows and apps'} outside this chat too, so for those suggest asking a workspace admin rather than creating them in the editor. Resources and variables they can still edit themselves on their pages, so when open_page lists that page, open it for them instead.${when( flowsOnly, '\n\nFlows are the exception: this workspace lets them compose flows out of scripts and flows that are already deployed. Write flow drafts with write_flow and patch_flow_json, using only steps that call a deployed script or flow by path. The server refuses inline code anywhere in the flow, so never add a rawscript step or inline code; when the flow needs logic no deployed script provides, say so and suggest asking a workspace admin for that script.' + )}${when( + !!draftedOperatorKinds, + `\n\nThis workspace lets them create and edit ${draftedOperatorKinds}: write them as drafts with ${[ + canDraftSchedules && 'write_schedule', + canDraftTriggers && + 'write_trigger (call get_trigger_schema first: the config fields differ per kind)' + ] + .filter(Boolean) + .join(' and ')}, then deploy them with deploy_workspace_item once the user asks.` )}` }${when( canWriteAnyDraft, @@ -2592,7 +2619,12 @@ export function getSessionContextPromptSection( // Concatenated onto an already capability-gated prompt, so it has to honour the same // profile rather than assume the gating happened upstream. const canDeploy = !access || access.has('deploy') - const canWriteDraft = !access || access.has('write_draft') || access.has('write_flow_draft') + const canWriteDraft = + !access || + access.has('write_draft') || + access.has('write_flow_draft') || + access.has('write_schedule_draft') || + access.has('write_trigger_draft') const canRunPreview = !access || access.has('run_preview') || access.has('run_flow_preview') // The kinds no deploy rule reaches, unless the workspace withdrew them from this user. const unruledKinds = [ @@ -3867,7 +3899,7 @@ export const globalTools: SessionTool<{}>[] = [ } }, { - requires: WRITE_DRAFT, + requires: WRITE_SCHEDULE_DRAFT, def: createToolDef( writeScheduleToolSchema, 'write_schedule', @@ -3893,7 +3925,7 @@ export const globalTools: SessionTool<{}>[] = [ } }, { - requires: WRITE_DRAFT, + requires: WRITE_TRIGGER_DRAFT, def: createToolDef( writeTriggerSchema, 'write_trigger', @@ -4225,8 +4257,8 @@ export const globalTools: SessionTool<{}>[] = [ app: WRITE_DRAFT, resource: WRITE_DRAFT, variable: WRITE_DRAFT, - schedule: WRITE_DRAFT, - trigger: WRITE_DRAFT + schedule: WRITE_SCHEDULE_DRAFT, + trigger: WRITE_TRIGGER_DRAFT } satisfies Record<(typeof ITEM_TYPES)[number], SessionToolPolicy>, def: createToolDef( rebaseDraftSchema, diff --git a/frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts b/frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts index 2e28375cee..2d44e47ec5 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts @@ -54,7 +54,9 @@ describe('resolveSessionAccess', () => { 'run_flow_preview', 'run_preview', 'write_draft', - 'write_flow_draft' + 'write_flow_draft', + 'write_schedule_draft', + 'write_trigger_draft' ]) }) @@ -62,13 +64,24 @@ describe('resolveSessionAccess', () => { // so withholding `deploy` from an operator would be stricter than the server. it('leaves an operator the deploy-rule capability, and nothing their token refuses', async () => { const caps = await capabilitiesFor({ operator: true }) - expect([...caps].sort()).toEqual(['deploy', 'manage_schedules', 'manage_triggers']) + expect([...caps].sort()).toEqual([ + 'deploy', + 'manage_schedules', + 'manage_triggers', + 'write_schedule_draft', + 'write_trigger_draft' + ]) }) it('takes deploy from the rules alone, so a rule blocks an operator too', async () => { deployRules.mockResolvedValue({ ok: false, refusedBy: 'DisableDirectDeployment' }) const caps = await capabilitiesFor({ operator: true }) - expect([...caps].sort()).toEqual(['manage_schedules', 'manage_triggers']) + expect([...caps].sort()).toEqual([ + 'manage_schedules', + 'manage_triggers', + 'write_schedule_draft', + 'write_trigger_draft' + ]) }) // Both spellings of `authed.is_admin`, which the draft path honours and the handlers @@ -83,7 +96,9 @@ describe('resolveSessionAccess', () => { 'manage_schedules', 'manage_triggers', 'write_draft', - 'write_flow_draft' + 'write_flow_draft', + 'write_schedule_draft', + 'write_trigger_draft' ]) } ) @@ -114,7 +129,9 @@ describe('resolveSessionAccess', () => { 'manage_schedules', 'manage_triggers', 'run_flow_preview', - 'write_flow_draft' + 'write_flow_draft', + 'write_schedule_draft', + 'write_trigger_draft' ]) }) @@ -127,7 +144,9 @@ describe('resolveSessionAccess', () => { operatorSettings: { manage_schedules: false } }) expect(caps.has('manage_schedules')).toBe(false) + expect(caps.has('write_schedule_draft')).toBe(false) expect(caps.has('manage_triggers')).toBe(true) + expect(caps.has('write_trigger_draft')).toBe(true) }) it('grants everything when the role cannot be resolved', async () => { diff --git a/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts b/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts index 2f1e00c09b..00aa820b3c 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts @@ -14,6 +14,8 @@ type OperatorRights = Pick< const ALL_CAPABILITIES: SessionCapability[] = [ 'write_draft', 'write_flow_draft', + 'write_schedule_draft', + 'write_trigger_draft', 'run_preview', 'run_flow_preview', 'deploy', @@ -54,6 +56,12 @@ export function capabilitiesForRole(role: { if (roleCanDraft('flow', user, role.operatorSettings)) { capabilities.add('write_flow_draft') } + if (roleCanDraft('schedule', user, role.operatorSettings)) { + capabilities.add('write_schedule_draft') + } + if (roleCanDraft('trigger', user, role.operatorSettings)) { + capabilities.add('write_trigger_draft') + } if (authorsCode) { capabilities.add('run_preview') capabilities.add('manage_code') diff --git a/frontend/src/lib/components/copilot/chat/global/sessionToolset.test.ts b/frontend/src/lib/components/copilot/chat/global/sessionToolset.test.ts index 4984329b3a..73e5f9713a 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionToolset.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionToolset.test.ts @@ -76,10 +76,11 @@ const REACHABLE_PROFILES = [ [false, true].flatMap((operator) => // `checkDeployRules` bypasses on admin, so an admin has only the one outcome. (isAdmin ? [true] : [false, true]).flatMap((deployRulesPass) => - // The operator rights a workspace can set: none, the builder right, both locks. + // The operator rights a workspace can set: none, the builder right, one lock, both. [ undefined, { builder_flows: true }, + { manage_triggers: false }, { manage_schedules: false, manage_triggers: false } ].map((operatorSettings) => { const access = capabilitiesForRole({ diff --git a/frontend/src/lib/components/copilot/chat/sessionCapabilities.ts b/frontend/src/lib/components/copilot/chat/sessionCapabilities.ts index f96e89dddc..59644cb8cb 100644 --- a/frontend/src/lib/components/copilot/chat/sessionCapabilities.ts +++ b/frontend/src/lib/components/copilot/chat/sessionCapabilities.ts @@ -15,6 +15,11 @@ export type SessionCapability = /** Passes `require_can_write_path` for a flow draft, which an operator also passes where the * workspace grants `builder_flows`. The server then refuses a draft carrying inline code. */ | 'write_flow_draft' + /** Passes `require_can_write_path` for a schedule draft: everyone `write_draft` covers, and an + * operator unless the workspace set `manage_schedules` to false. */ + | 'write_schedule_draft' + /** The same for a trigger draft, read from `manage_triggers`. */ + | 'write_trigger_draft' /** Passes the operator refusal jobs.rs `run_preview_*` makes before starting a job. */ | 'run_preview' /** Passes `run_preview_flow`'s `check_operator_can_build_flows`: composed flows only, for an @@ -50,6 +55,8 @@ export type SessionToolPolicy = readonly SessionCapability[] export const NONE: SessionToolPolicy = [] export const WRITE_DRAFT: SessionToolPolicy = ['write_draft'] export const WRITE_FLOW_DRAFT: SessionToolPolicy = ['write_flow_draft'] +export const WRITE_SCHEDULE_DRAFT: SessionToolPolicy = ['write_schedule_draft'] +export const WRITE_TRIGGER_DRAFT: SessionToolPolicy = ['write_trigger_draft'] export const RUN_PREVIEW: SessionToolPolicy = ['run_preview'] export const RUN_FLOW_PREVIEW: SessionToolPolicy = ['run_flow_preview'] export const DEPLOY: SessionToolPolicy = ['deploy'] diff --git a/frontend/src/lib/editRights.test.ts b/frontend/src/lib/editRights.test.ts index 688901a180..a72873b1e3 100644 --- a/frontend/src/lib/editRights.test.ts +++ b/frontend/src/lib/editRights.test.ts @@ -35,6 +35,15 @@ describe('edit rights', () => { expect(roleCanAuthor('script', adminOperator, undefined)).toBe(false) }) + // drafts.rs admits an operator's draft of a kind they could deploy, and no other. + it('lets an operator draft schedules and triggers until the right is withdrawn', () => { + expect(roleCanDraft('schedule', user({}), undefined)).toBe(true) + expect(roleCanDraft('trigger', user({}), undefined)).toBe(true) + expect(roleCanDraft('schedule', user({}), { manage_schedules: false })).toBe(false) + expect(roleCanDraft('trigger', user({}), { manage_triggers: false })).toBe(false) + expect(roleCanDraft('resource', user({}), undefined)).toBe(false) + }) + // The role alone is not enough: a builder still cannot edit a flow in a folder they only read. it('requires write access to the item on top of the role', () => { const builder = { builder_flows: true } diff --git a/frontend/src/lib/editRights.ts b/frontend/src/lib/editRights.ts index b7a8014936..41738e442e 100644 --- a/frontend/src/lib/editRights.ts +++ b/frontend/src/lib/editRights.ts @@ -31,8 +31,8 @@ type OperatorRights = | undefined /** - * Whether a user's role lets them author items of `kind` — create one, edit it, save its - * drafts — in a workspace with these `operator_settings`. The role half of every edit check; + * Whether a user's role lets them author items of `kind` — create one, edit it, deploy it — + * in a workspace with these `operator_settings` (drafts are `roleCanDraft`). The role half of every edit check; * which items they may write is `canEditItem`'s other half. For an operator: * - code (scripts, apps, agents, pipelines): never; * - flows: where the workspace grants `builder_flows` (`check_operator_can_build_flows`); @@ -63,17 +63,27 @@ export function roleCanAuthor( } /** - * Whether the user may save drafts of `kind` — what every session editor writes. Drafts are the - * one place an admin outranks the operator flag: drafts.rs `require_can_write_path` returns Ok - * on `is_admin` before its operator branch, while the item handlers refuse an operator with no - * admin escape, so `roleCanAuthor` stays the rule for creating, deploying and deleting. + * Whether the user may save drafts of `kind` — what every session editor and the chat's draft + * tools write. Mirrors drafts.rs `require_can_write_path`, which differs from `roleCanAuthor` + * twice: an admin outranks the operator flag there (Ok on `is_admin` before the operator + * branch, where the item handlers have no admin escape), and an operator drafts only the kinds + * they could deploy — flows, schedules and triggers — never resources, variables or groups, + * which they edit directly instead. */ export function roleCanDraft( kind: EditKind, user: Pick | undefined, operatorSettings: OperatorRights ): boolean { - return !!user?.is_admin || !!user?.is_super_admin || roleCanAuthor(kind, user, operatorSettings) + if (user?.is_admin || user?.is_super_admin || !user?.operator) return true + switch (kind) { + case 'flow': + case 'schedule': + case 'trigger': + return roleCanAuthor(kind, user, operatorSettings) + default: + return false + } } /** `roleCanDraft` for the item at `path`, with its own permissions as in `canEditItem`. */