From 01048ae7ce137b5a17de9fa535a4b6cf8aa86966 Mon Sep 17 00:00:00 2001 From: AlexRV12 <71396855+AlexRV12@users.noreply.github.com> Date: Thu, 20 Aug 2026 10:50:10 +0200 Subject: [PATCH] fix: match the server's deploy gate exactly, never exceed it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The filter must be as strict as the server and no stricter. Schedules and triggers reach no deploy rule — `check_deploy_rules` runs only from the gated kinds' handlers — so no workspace refuses `deploy_workspace_item` or `delete_workspace_item` outright, whatever refusal `checkDeployPermission` reports. Gating them on a deploy capability withheld operations the server performs. Neither tool now requires a capability. Deploying still needs a draft to deploy, so it keeps the authoring relevance; deleting a deployed item does not, so it is ungated. `deploy` returns to one capability, covering the kinds the rules gate, and `create_folder` — whose kind is one of them — is the only tool that names it. The session-state note now states which kinds a refusing workspace still accepts. --- .../components/copilot/chat/global/core.ts | 48 +++++++++---------- .../copilot/chat/global/sessionAccess.test.ts | 19 +------- .../copilot/chat/global/sessionAccess.ts | 27 ++++------- .../chat/global/sessionToolset.test.ts | 35 +++++++------- .../copilot/chat/global/sessionToolset.ts | 12 ++--- 5 files changed, 55 insertions(+), 86 deletions(-) diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index bfefe22680..001265f836 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -1269,10 +1269,11 @@ const buildGlobalSystemPrompt = ( // SESSION_TOOL_POLICIES, so a rule cannot outlive the tool it describes. An // unresolved profile keeps every block. 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 — see SESSION_TOOL_POLICIES. - const canCreateFolder = !access || access.capabilities.has('deploy_gated_kinds') + // `deploy` covers only the kinds `check_deploy_rules` gates, and `create_folder` is + // the one tool here that names such a kind — the deploy tools take theirs as an + // argument, so they ship with the drafts they act on. + const canCreateFolder = !access || access.capabilities.has('deploy') // 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. const when = (cond: boolean, block: string) => (cond ? block : '') @@ -1311,7 +1312,7 @@ 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, edit or deploy anything here — this user's role does not allow it — so when they ask for a change, say plainly that you cannot make it rather than describing steps as if you had." + : "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 anything here — this user's role does not allow it — so when they ask for a change, say plainly that you cannot make it rather than describing steps as if you had." }${when( // Every line here is about choosing a path for something NEW, down to the folder // guidance; with nothing to create, the whole block is context tax. @@ -1336,20 +1337,17 @@ Rules:${when( )} - Use list_workspace_items to find items and read_workspace_item before changing an existing item. For triggers, pass trigger_kind. - If the user message includes an ACTIVE EDITOR section, treat it as the currently open item and use it for references like "this", "current", or "open editor".${activePreviewRule}${when( - canDeploy, + canWriteDraft, ` - Use deploy_workspace_item only after the user explicitly asks to deploy. It persists a draft to the workspace.` )}${when( canWriteDraft, ` -- To undo something you created or changed in this chat, use discard_local_draft: everything you write is a draft until it is explicitly deployed, so "delete it" / "never mind" / "remove that" about your own work means discarding the draft (it also clears the matching open editor draft).${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.' - )}` +- To undo something you created or changed in this chat, use discard_local_draft: everything you write is a draft until it is explicitly deployed, so "delete it" / "never mind" / "remove that" about your own work means discarding the draft (it also clears the matching open editor draft). 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.` +- Two changes are still open to you where the server allows them: discard_local_draft drops a draft this user left behind, and delete_workspace_item removes an item already deployed in the workspace. You cannot create or edit 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, @@ -1371,7 +1369,7 @@ ${pipelineBullet}` - To see what a flow run actually did per step — statuses and results across the whole execution tree, subflow steps and loop iterations included — use get_flow_run_details with the run id (it also works while the flow is still running). Pass step to read one step's result in full (capped at 12k chars). Prefer it over get_job_logs when you need step results rather than logs. - Use open_page to show a workspace page with filters applied — Runs, Schedules, Variables, Resources, Assets, Audit logs, or Workspace settings on a specific tab (e.g. "open the failed runs of f/foo/bar", "open the schedule for X", "open the git sync settings"). Carry over every filter the user described — Runs takes the page's whole filter set (time window, path, user, folder, label, tag, worker, trigger kind, args/result, ...), so don't drop a criterion just because it wasn't in the request's main clause. Only the pages listed for this user in the tool are available; don't offer pages that aren't listed. Don't use it as a substitute for list_runs when you just need the data yourself. - Whenever you ask the user to perform a manual step in the UI — fill in a resource's credentials, set a secret variable's value, adjust a schedule or setting — call open_page in the same message, targeted at that item (pass open with its path to land in its edit drawer, or the page's filters otherwise). Never just describe where to click.${when( - canDeploy, + canWriteDraft, ` - When the user is happy with the changes and wants to review or deploy them, use open_page with page "compare" — it opens the Compare & Deploy review page.${ previewTools @@ -1379,12 +1377,9 @@ ${pipelineBullet}` : ' Pass items (":" entries naming the items you changed) so the review is scoped to them — omitting items preselects every pending change in the workspace' }, or mode ("draft" or "fork") to force which comparison is shown. Prefer offering this review page over calling deploy_workspace_item directly when several items changed.` )} -- For a Windmill operation no other tool covers (workers, queue state, a run's args, ...), use search_api_endpoints to find a REST endpoint, then call_api_get for reads or call_api_endpoint for mutations (the user is asked to confirm those). Always prefer a dedicated tool when one exists; endpoints for authoring or deleting scripts, flows, apps, schedules, resources, or variables are not available through the API catalog tools${when( - canWriteDraft || canDeploy, - ` — use ${[canWriteDraft && 'the draft tools', canDeploy && 'delete_workspace_item'] - .filter(Boolean) - .join(' and ')} instead` - )}. +- For a Windmill operation no other tool covers (workers, queue state, a run's args, ...), use search_api_endpoints to find a REST endpoint, then call_api_get for reads or call_api_endpoint for mutations (the user is asked to confirm those). Always prefer a dedicated tool when one exists; endpoints for authoring or deleting scripts, flows, apps, schedules, resources, or variables are not available through the API catalog tools${` — use ${[canWriteDraft && 'the draft tools', 'delete_workspace_item'] + .filter(Boolean) + .join(' and ')} instead`}. - runScriptByPath / runFlowByPath from the API catalog run the DEPLOYED version of an item. Use them only when the user explicitly asks to run the deployed version, and read the item with read_workspace_item version: "deployed" first so the arguments match the deployed input schema (a draft may have different inputs).${when( canRunPreview && canWriteDraft, ' To test something you are editing or just wrote, always use test_run_script, test_run_flow, or test_run_step — they run the draft.' @@ -1436,7 +1431,7 @@ Raw apps: - Use write_app_runnable and delete_app_runnable for backend runnables. - Use init_app only after confirming framework, path, and summary with the user.` )}${when( - canDeploy, + canWriteDraft, ` - Use deploy_workspace_item after explicit user deploy intent; raw app deploy bundles JS/CSS before saving.` )} @@ -2386,14 +2381,15 @@ export function getSessionContextPromptSection( // profile rather than assume the gating happened upstream. Each branch keeps its // "where work lands" fact either way. const canDeploy = !access || access.capabilities.has('deploy') - const canDeployGatedKinds = !access || access.capabilities.has('deploy_gated_kinds') const canWriteDraft = !access || access.capabilities.has('write_draft') const canRunPreview = !access || access.capabilities.has('run_preview') const targets = [ 'reads', canWriteDraft && 'drafts', canRunPreview && 'test runs', - canDeploy && 'deploys' + // `deploy_workspace_item` ships with the draft tools: it deploys a draft, so a + // session that cannot write one has nothing to deploy. + canWriteDraft && 'deploys' ] .filter(Boolean) .join(', ') @@ -2409,19 +2405,19 @@ export function getSessionContextPromptSection( ) } else if (ctx.parentWorkspaceId && ctx.isDevWorkspace) { lines.push( - `- Operating workspace: "${ctx.workspaceId}" — the user's persistent DEV WORKSPACE, forked from workspace "${ctx.parentWorkspaceId}". ${canDeploy ? 'deploy_workspace_item publishes' : 'Changes land'} into the dev workspace only; the user reviews & promotes changes into "${ctx.parentWorkspaceId}" from the session's deploy panel. Never present a change as live in "${ctx.parentWorkspaceId}".` + `- Operating workspace: "${ctx.workspaceId}" — the user's persistent DEV WORKSPACE, forked from workspace "${ctx.parentWorkspaceId}". ${canWriteDraft ? 'deploy_workspace_item publishes' : 'Changes land'} into the dev workspace only; the user reviews & promotes changes into "${ctx.parentWorkspaceId}" from the session's deploy panel. Never present a change as live in "${ctx.parentWorkspaceId}".` ) } else if (ctx.parentWorkspaceId) { lines.push( - `- Operating workspace: "${ctx.workspaceId}" — an ephemeral STAGED FORK of workspace "${ctx.parentWorkspaceId}", created for session work. ${canDeploy ? 'deploy_workspace_item publishes' : 'Changes land'} into the fork only, and the user reviews & promotes fork changes into "${ctx.parentWorkspaceId}" from the session's deploy panel. Never present a change as live in "${ctx.parentWorkspaceId}".` + `- Operating workspace: "${ctx.workspaceId}" — an ephemeral STAGED FORK of workspace "${ctx.parentWorkspaceId}", created for session work. ${canWriteDraft ? 'deploy_workspace_item publishes' : 'Changes land'} into the fork only, and the user reviews & promotes fork changes into "${ctx.parentWorkspaceId}" from the session's deploy panel. Never present a change as live in "${ctx.parentWorkspaceId}".` ) } else if (ctx.forkParentUnknown) { lines.push( - `- Operating workspace: "${ctx.workspaceId}" — a fork whose parent workspace is not currently visible to this user. ${canDeploy ? 'deploy_workspace_item publishes' : 'Changes land'} into the fork only; the user promotes changes from the session's deploy panel. Never present a change as live in any other workspace.` + `- Operating workspace: "${ctx.workspaceId}" — a fork whose parent workspace is not currently visible to this user. ${canWriteDraft ? 'deploy_workspace_item publishes' : 'Changes land'} into the fork only; the user promotes changes from the session's deploy panel. Never present a change as live in any other workspace.` ) } else if (ctx.workspaceId) { lines.push( - `- Operating workspace: "${ctx.workspaceId}" — the live workspace itself, not a fork.${canDeploy ? ' deploy_workspace_item publishes directly to everyone in it.' : ''}` + `- Operating workspace: "${ctx.workspaceId}" — the live workspace itself, not a fork.${canWriteDraft ? ' deploy_workspace_item publishes directly to everyone in it.' : ''}` ) } else { lines.push( @@ -2430,9 +2426,9 @@ export function getSessionContextPromptSection( } // 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) { + if (canWriteDraft && !canDeploy) { 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.' + "- This workspace refuses direct deployment for this user, except for schedules and triggers — those are the only kinds deploy_workspace_item and delete_workspace_item can still act on. Scripts, flows, apps, resources and variables must be promoted from the session's deploy panel (fork or pull request); do not offer to deploy or delete them directly." ) } return lines.join('\n') 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 8fdc735c0f..0be7ee4f40 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts @@ -43,12 +43,7 @@ describe('resolveSessionAccess', () => { it('gives a developer every capability', async () => { const caps = await capabilitiesFor({}) - expect([...caps].sort()).toEqual([ - 'deploy', - 'deploy_gated_kinds', - 'run_preview', - 'write_draft' - ]) + expect([...caps].sort()).toEqual(['deploy', 'run_preview', 'write_draft']) }) it('leaves an operator only what their token can still do', async () => { @@ -82,18 +77,6 @@ describe('resolveSessionAccess', () => { expect(caps.has('write_draft')).toBe(true) }) - // 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, - reason: 'direct deployment disabled', - refusedBy: 'DisableDirectDeployment' - }) - const caps = await capabilitiesFor({}) - expect(caps.has('deploy')).toBe(true) - expect(caps.has('deploy_gated_kinds')).toBe(false) - }) - 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 93b752f9bc..1e01c68e14 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionAccess.ts @@ -11,11 +11,10 @@ import { checkDeployPermission } from '$lib/utils_workspace_deploy' export type SessionCapability = | 'write_draft' | 'run_preview' - /** May deploy at least something. */ + /** May deploy the kinds `check_deploy_rules` gates. There is no capability for the + * rest — schedules and triggers reach no rule (`kindGatedByDeployRules`), so every + * user may deploy those and a capability for them would always be present. */ | 'deploy' - /** 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 = { /** The workspace these capabilities were resolved against — a session targets its @@ -24,12 +23,7 @@ export type SessionAccess = { capabilities: ReadonlySet } -const ALL_CAPABILITIES: SessionCapability[] = [ - 'write_draft', - 'run_preview', - 'deploy', - 'deploy_gated_kinds' -] +const ALL_CAPABILITIES: SessionCapability[] = ['write_draft', 'run_preview', 'deploy'] /** 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, @@ -68,14 +62,11 @@ export async function resolveSessionAccess(workspace: string): Promise { expect(sessionToolAllowed('write_script', readOnly)).toBe(false) expect(sessionToolAllowed('write_variable', readOnly)).toBe(false) expect(sessionToolAllowed('deploy_workspace_item', readOnly)).toBe(false) + expect(sessionToolAllowed('delete_workspace_item', readOnly)).toBe(true) expect(sessionToolAllowed('test_run_script', readOnly)).toBe(false) expect(sessionToolAllowed('exec_datatable_sql', readOnly)).toBe(false) expect(sessionToolAllowed('list_workspace_items', readOnly)).toBe(true) @@ -108,7 +109,7 @@ describe('session tool policies', () => { // Relevance is the second axis: these need no capability, so `requires` alone // would keep advertising them to a session that can never author anything. it('drops authoring aids when drafts cannot be written', () => { - const readOnly = accessWith(['deploy', 'deploy_gated_kinds']) + const readOnly = accessWith(['deploy']) expect(sessionToolAllowed('get_instructions', readOnly)).toBe(false) expect(sessionToolAllowed('search_npm_packages', readOnly)).toBe(false) expect(sessionToolAllowed('search_docs', readOnly)).toBe(true) @@ -120,19 +121,18 @@ describe('session tool policies', () => { expect(sessionToolAllowed('create_folder', accessWith(['write_draft', 'run_preview']))).toBe( false ) - expect( - sessionToolAllowed('create_folder', accessWith(['write_draft', 'deploy_gated_kinds'])) - ).toBe(true) - expect(sessionToolAllowed('create_folder', accessWith(['deploy_gated_kinds']))).toBe(false) + expect(sessionToolAllowed('create_folder', accessWith(['write_draft', 'deploy']))).toBe(true) + expect(sessionToolAllowed('create_folder', accessWith(['deploy']))).toBe(false) }) - // 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) - expect(sessionToolAllowed('delete_workspace_item', locked)).toBe(true) - expect(sessionToolAllowed('create_folder', locked)).toBe(false) + // Schedules and triggers reach no deploy rule, so a workspace that refuses this user's + // deploys still accepts those two kinds. Gating the kind-taking tools on `deploy` would + // withhold operations the server performs. + it('keeps the deploy tools when only the gated kinds are refused', () => { + const noDeploy = accessWith(['write_draft', 'run_preview']) + expect(sessionToolAllowed('deploy_workspace_item', noDeploy)).toBe(true) + expect(sessionToolAllowed('delete_workspace_item', noDeploy)).toBe(true) + expect(sessionToolAllowed('create_folder', noDeploy)).toBe(false) }) // Drafts must stay cleanable after a role change. @@ -160,9 +160,8 @@ describe('session tool policies', () => { ['read-only', [], true], ['drafts, no deploy', ['write_draft', 'run_preview'], true], ['drafts only', ['write_draft'], true], - ['direct-deployment lock', ['write_draft', 'run_preview', 'deploy'], true], - ['drafts, no preview', ['write_draft', 'deploy', 'deploy_gated_kinds'], false], - ['deploy, no drafts', ['deploy', 'deploy_gated_kinds'], false] + ['drafts, no preview', ['write_draft', 'deploy'], false], + ['deploy, no drafts', ['deploy'], false] ] as [string, SessionCapability[], boolean][])( 'never names a withheld tool in the assembled prompt (%s)', (_label, capabilities, reachable) => { @@ -215,10 +214,10 @@ describe('session tool policies', () => { it('treats write_draft and deploy as independent', () => { const draftsOnly = accessWith(['write_draft']) expect(sessionToolAllowed('write_script', draftsOnly)).toBe(true) - expect(sessionToolAllowed('deploy_workspace_item', draftsOnly)).toBe(false) + expect(sessionToolAllowed('create_folder', draftsOnly)).toBe(false) - const deployOnly = accessWith(['deploy', 'deploy_gated_kinds']) + const deployOnly = accessWith(['deploy']) expect(sessionToolAllowed('write_script', deployOnly)).toBe(false) - expect(sessionToolAllowed('deploy_workspace_item', deployOnly)).toBe(true) + expect(sessionToolAllowed('create_folder', deployOnly)).toBe(false) }) }) diff --git a/frontend/src/lib/components/copilot/chat/global/sessionToolset.ts b/frontend/src/lib/components/copilot/chat/global/sessionToolset.ts index d3099d8a00..3e6b8fc77f 100644 --- a/frontend/src/lib/components/copilot/chat/global/sessionToolset.ts +++ b/frontend/src/lib/components/copilot/chat/global/sessionToolset.ts @@ -17,7 +17,6 @@ export type SessionToolPolicy = { const NONE: SessionToolPolicy = { requires: [] } const AUTHORING_AID: SessionToolPolicy = { requires: [], relevance: 'authoring' } const WRITE_DRAFT: SessionToolPolicy = { requires: ['write_draft'] } -const DEPLOY: SessionToolPolicy = { requires: ['deploy'] } const RUN_PREVIEW: SessionToolPolicy = { requires: ['run_preview'] } /** @@ -94,7 +93,7 @@ export const SESSION_TOOL_POLICIES: Record = { get_db_schema: AUTHORING_AID, // `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' }, + create_folder: { requires: ['deploy'], 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 // change nothing themselves. And it is a posture the USER selects, so withholding @@ -130,10 +129,11 @@ export const SESSION_TOOL_POLICIES: Record = { rebase_draft: WRITE_DRAFT, // ── Deployed-object mutations ─────────────────────────────────────────── - // 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, + // Neither requires `deploy`: both take the kind as an argument, and schedules and + // triggers reach no deploy rule, so no workspace refuses these outright. Deploying + // still needs a draft to deploy, which deleting does not. + deploy_workspace_item: AUTHORING_AID, + delete_workspace_item: NONE, // ── Preview execution ─────────────────────────────────────────────────── test_run_script: RUN_PREVIEW,