diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index 573a506ef7..bfefe22680 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -1271,9 +1271,7 @@ const buildGlobalSystemPrompt = ( const canWriteDraft = !access || access.capabilities.has('write_draft') const canDeploy = !access || access.capabilities.has('deploy') const canRunPreview = !access || access.capabilities.has('run_preview') - // `create_folder` needs the stronger half: a folder is one of the kinds - // `check_deploy_rules` gates, so a direct-deployment lock refuses it while the - // deploy tools stay usable for schedules and triggers. + // `create_folder` needs the stronger half — see SESSION_TOOL_POLICIES. const canCreateFolder = !access || access.capabilities.has('deploy_gated_kinds') // Each gated block carries its own leading newline, so dropping one leaves no blank // line behind and a full-access prompt is byte-for-byte the ungated text. @@ -1348,6 +1346,10 @@ Rules:${when( canDeploy, ' Use delete_workspace_item only to remove an item that is already deployed in the workspace; it mutates the workspace and fails if nothing is deployed at that path.' )}` + )}${when( + !canWriteDraft, + ` +- Discarding your OWN draft is the one change you can still make: if the user asks to drop or clean up a draft they left behind, use discard_local_draft. You cannot write or deploy one.` )} - Use diff to review changes — before deploying, or when the user asks what changed. It is read-only: without arguments it lists every draft in the workspace with its change status; with type+path it returns that item's unified diff (for multi-file apps, pass file to read one file's diff). In a fork, pass against="parent_workspace" to compare the deployed fork with its parent workspace instead. Pass search to grep changed lines across all diffs.${when( canWriteDraft, @@ -2426,9 +2428,8 @@ export function getSessionContextPromptSection( '- No operating workspace is set yet; the user picks one (or a new staged fork) before the first message is sent.' ) } - // Without this the model reads "deploys" among its targets and has no way to know - // which kinds the workspace refuses, so it would keep proposing script and flow - // deploys that come back 403. + // Without this the model reads "deploys" among its targets with no way to know which + // kinds are refused, and keeps proposing script deploys that come back 403. if (canDeploy && !canDeployGatedKinds) { lines.push( '- Direct deployment is disabled in this workspace: only schedules and triggers can be deployed with deploy_workspace_item. Scripts, flows, apps, resources, variables and folders must be promoted from the session\'s deploy panel (fork or pull request) — do not offer to deploy them.' 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 bc3bbbaf41..8fdc735c0f 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts @@ -57,11 +57,8 @@ describe('resolveSessionAccess', () => { expect([...caps]).toEqual([]) }) - // The two gates have OPPOSITE precedence in the backend, so a role ladder would - // get one of them wrong: drafts.rs returns Ok for `authed.is_admin` before the - // operator branch, while jobs.rs checks the operator flag first with no escape. - // `authed.is_admin` is `usr.is_admin || super_admin`, so both spellings must land - // on the same side of it. + // Pins the per-capability precedence documented in resolveSessionAccess, for both + // spellings of `authed.is_admin`. it.each([{ is_admin: true }, { is_super_admin: true }])( 'lets an admin who is also an operator write drafts but not run previews (%o)', async (role) => { @@ -72,9 +69,7 @@ describe('resolveSessionAccess', () => { } ) - // Deploy is operation-shaped: protection rulesets can block a plain developer, and - // wm_deployers can unblock a non-admin. The resolver must not second-guess the shared - // preflight, and must not let a deploy refusal take drafting down with it. + // A deploy refusal must not take drafting down with it. it('takes deploy from the shared permission check, not from the role', async () => { deployPermission.mockResolvedValue({ ok: false, @@ -87,9 +82,7 @@ describe('resolveSessionAccess', () => { expect(caps.has('write_draft')).toBe(true) }) - // A direct-deployment lock is the one refusal that does not cover every kind: the - // server still accepts schedule and trigger deploys, so the resolver must keep the - // weaker half rather than dropping deploy wholesale. + // The one refusal that does not cover every kind. it('keeps the ungated half of deploy under a direct-deployment lock', async () => { deployPermission.mockResolvedValue({ ok: false, @@ -101,8 +94,6 @@ describe('resolveSessionAccess', () => { expect(caps.has('deploy_gated_kinds')).toBe(false) }) - // Fail open, matching checkDeployPermission: a transient whoami failure must not - // strip a session's toolset — the server is still the enforcement point. it('grants everything when the role cannot be resolved', async () => { whoami.mockRejectedValueOnce(new Error('network')) const access = await resolveSessionAccess('ws') diff --git a/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts b/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts index eb3ed32ccc..93b752f9bc 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts @@ -4,23 +4,17 @@ import { checkDeployPermission } from '$lib/utils_workspace_deploy' /** * What a user may do in ONE workspace, as the AI session toolset needs to know it. * - * These are permission facts, never relevance judgements — a capability is absent - * only when the backend would refuse the call. Best-effort, not a boundary: the token - * is the enforcement point, and every failure resolves OPEN — a failed `whoami` yields - * every capability, a failed rules fetch reads as no rule active. So this narrows what - * the model is offered; it does not guarantee it is never offered a tool that would 401. - * - * The rules below are NOT a role ladder: the backend's precedence between "admin" - * and "operator" differs per capability. Collapsing them into one ordering would get - * `write_draft` wrong for a superadmin whose workspace role is operator. + * Permission facts, never relevance judgements: a capability is absent only when the + * backend would refuse the call. Best-effort, not a boundary — the token is the + * enforcement point, so this narrows what the model is offered and guarantees nothing. */ export type SessionCapability = | 'write_draft' | 'run_preview' - /** May deploy at least something: no operator or deployers-only refusal. */ + /** May deploy at least something. */ | 'deploy' - /** May also deploy the kinds `check_deploy_rules` gates — everything except - * schedules and triggers, which no rule covers (`kindGatedByDeployRules`). */ + /** May also deploy the kinds `check_deploy_rules` gates — everything but schedules + * and triggers (`kindGatedByDeployRules`), which no rule covers. */ | 'deploy_gated_kinds' export type SessionAccess = { @@ -37,10 +31,10 @@ const ALL_CAPABILITIES: SessionCapability[] = [ 'deploy_gated_kinds' ] -/** Benefit of the doubt: an unresolvable role must not blank the toolset, since a - * transient failure would otherwise tell a developer mid-session that they cannot - * author anything — a worse and far less legible outcome than the 403 they get by - * trying. Mirrors the fail-open contract of `checkDeployPermission`. */ +/** Fail open, here and at every resolution failure below: blanking a toolset on a + * transient error tells a developer mid-session that they cannot author anything, + * which is worse and far less legible than the 403 they get by trying. Matches + * `checkDeployPermission`, which fails open for the same reason. */ export function fullSessionAccess(workspace: string): SessionAccess { return { workspace, capabilities: new Set(ALL_CAPABILITIES) } } @@ -63,26 +57,20 @@ export async function resolveSessionAccess(workspace: string): Promise() - // windmill-api/src/drafts.rs `require_can_write_path`: `authed.is_admin` returns Ok - // BEFORE the operator branch, so an admin who is also an operator may still save - // drafts. That field is `usr.is_admin || super_admin` (windmill-api-auth/src/auth.rs) - // while `whoami` reports the two separately, hence the OR. + // Per-capability precedence, NOT a role ladder: drafts.rs `require_can_write_path` + // returns Ok on `authed.is_admin` BEFORE its operator branch, while jobs.rs + // `run_preview_*` refuses operators first with no admin escape. `authed.is_admin` is + // `usr.is_admin || super_admin` (auth.rs), which `whoami` reports as two fields. if (me.is_admin || me.is_super_admin || !me.operator) { capabilities.add('write_draft') } - - // windmill-api/src/jobs.rs `run_preview_script` / `run_preview_flow_job`: the operator - // check comes first and has no admin escape — the opposite precedence to drafts. if (!me.operator) { capabilities.add('run_preview') } - // `checkDeployPermission` carries every term of the backend's `check_deploy_rules`, - // superadmin included, so it is the whole answer. Splitting its verdict in two mirrors - // `deployPermissionForKind`: a direct-deployment lock stops the kinds that reach - // `check_deploy_rules` and nothing else, so schedules and triggers stay deployable — - // the same narrowing the Compare page applies. An operator or deployers-only refusal - // covers every kind, so it takes both. + // `checkDeployPermission` is the whole gate; its verdict splits the way + // `deployPermissionForKind` does — a direct-deployment lock refuses only the gated + // kinds, every other refusal covers all of them. const deploy = await checkDeployPermission(workspace, me) if (deploy.ok) { capabilities.add('deploy') 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 2cc5ed1d0f..ae2877c747 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionToolset.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionToolset.test.ts @@ -126,10 +126,8 @@ describe('session tool policies', () => { expect(sessionToolAllowed('create_folder', accessWith(['deploy_gated_kinds']))).toBe(false) }) - // A direct-deployment lock stops the kinds check_deploy_rules gates and nothing else, - // so the kind-taking deploy tools survive it while create_folder — a gated kind — does - // not. Collapsing the two capabilities back into one silently blocks the schedule and - // trigger deploys the server accepts. + // Collapsing the two deploy capabilities back into one would silently block the + // schedule and trigger deploys the server accepts. it('keeps the deploy tools but not create_folder under a direct-deployment lock', () => { const locked = accessWith(['write_draft', 'run_preview', 'deploy']) expect(sessionToolAllowed('deploy_workspace_item', locked)).toBe(true) @@ -137,8 +135,7 @@ describe('session tool policies', () => { expect(sessionToolAllowed('create_folder', locked)).toBe(false) }) - // The backend exempts discarding your OWN draft from require_can_write_path precisely - // so drafts stay cleanable after a role change; gating it here would strand them. + // Drafts must stay cleanable after a role change. it('keeps discard_local_draft without write_draft, but not rebase_draft', () => { const readOnly = accessWith([]) expect(sessionToolAllowed('discard_local_draft', readOnly)).toBe(true) diff --git a/frontend/src/lib/components/copilot/chat/global/sessionToolset.ts b/frontend/src/lib/components/copilot/chat/global/sessionToolset.ts index d76d0db911..d3099d8a00 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionToolset.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionToolset.ts @@ -92,10 +92,8 @@ export const SESSION_TOOL_POLICIES: Record = { get_trigger_schema: AUTHORING_AID, get_schedule_schema: AUTHORING_AID, get_db_schema: AUTHORING_AID, - // Folder creation runs through the backend's `check_deploy_rules` (folders.rs - // `create_folder`), and `folder` is one of the gated kinds — so a direct-deployment - // lock refuses it even though a folder is not a deployed item. It is also useless - // without something to put in it, hence the authoring relevance. + // `folder` is one of the gated kinds (folders.rs `create_folder` runs + // `check_deploy_rules`), and a folder is useless without something to put in it. create_folder: { requires: ['deploy_gated_kinds'], relevance: 'authoring' }, // Ungated on purpose, for two reasons. Plan mode's deliverable is a plan artifact, // which is worth producing for someone else to execute even when this user can @@ -126,15 +124,14 @@ export const SESSION_TOOL_POLICIES: Record = { write_app_runnable: WRITE_DRAFT, delete_app_runnable: WRITE_DRAFT, // Ungated on purpose: discarding your OWN draft skips `require_can_write_path` - // (drafts.rs), and the exemption exists precisely so a user who has LOST write - // access can still clean up drafts they left behind. Gating it here would strand - // that cleanup. Rebasing is not exempt — it writes a fresh draft. + // (drafts.rs) so that a user who has LOST write access can still clean up. Rebasing + // is not exempt — it writes a fresh draft. discard_local_draft: NONE, rebase_draft: WRITE_DRAFT, // ── Deployed-object mutations ─────────────────────────────────────────── - // Both take the kind as an argument, so they stay available under a direct-deployment - // lock: schedules and triggers are still deployable, and the prompt says which. + // The weaker capability on purpose: both take the kind as an argument, so a session + // that may deploy only the ungated kinds can still use them. deploy_workspace_item: DEPLOY, delete_workspace_item: DEPLOY,