mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: gate create_folder on the deploy capability
This commit is contained in:
@@ -2265,13 +2265,9 @@ export class AIChatManager {
|
||||
this.systemMessage = { ...target, content: `${target.content}\n\n${section}` }
|
||||
}
|
||||
|
||||
// Resolve what the user may do in the workspace this send targets, so the toolset
|
||||
// never advertises a tool whose every call the backend would refuse. Deliberately
|
||||
// re-resolved per send rather than cached for the session's life: a role change (or
|
||||
// a transient `whoami` failure, which resolves fail-open) must take effect on the
|
||||
// next message, not only on a workspace switch.
|
||||
//
|
||||
// Only sessions are filtered; the global side-panel chat keeps the full toolset.
|
||||
// Re-resolved per send rather than cached for the session's life, so a role change —
|
||||
// or a transient `whoami` failure, which resolves fail-open — takes effect on the next
|
||||
// message rather than only on a workspace switch. Only sessions are filtered.
|
||||
private resolveSessionAccessForSend = async (workspace: string) => {
|
||||
if (!this.isSessionChat || !workspace) {
|
||||
this.sessionAccess = undefined
|
||||
@@ -2721,12 +2717,10 @@ export class AIChatManager {
|
||||
base = self.planMode.decorateSystemMessage(base)
|
||||
return base
|
||||
},
|
||||
// The one place every tool source converges — the static global set, the
|
||||
// pipeline and MCP tools `configureGlobalMode` appends, and plan mode's,
|
||||
// added here. Filtering here rather than at `globalToolsFor` is what makes
|
||||
// the capability check cover the dynamic sources too, and the array is both
|
||||
// advertised to the model and used to dispatch the call, so a withheld tool
|
||||
// is unreachable rather than merely unlisted.
|
||||
// The one place every tool source converges, which is why the capability
|
||||
// filter belongs here and not in `globalToolsFor`. This same array both
|
||||
// advertises the tools and dispatches the calls, so a withheld tool is
|
||||
// unreachable rather than merely unlisted.
|
||||
get tools() {
|
||||
return filterSessionTools([...self.tools, ...self.planMode.tools], self.sessionAccess)
|
||||
},
|
||||
|
||||
@@ -1207,7 +1207,11 @@ type FolderPromptContext = { folders?: string[]; foldersRead?: string[]; isAdmin
|
||||
// non-exhaustive hint alongside permission-agnostic guidance (the complete set
|
||||
// needs a folder-listing tool — follow-up).
|
||||
// Capped so a folder-heavy workspace can't dominate the prompt.
|
||||
function buildFolderGuidance(username: string, ctx?: FolderPromptContext): string {
|
||||
function buildFolderGuidance(
|
||||
username: string,
|
||||
ctx?: FolderPromptContext,
|
||||
canCreateFolder: boolean = true
|
||||
): string {
|
||||
if (!ctx) return ''
|
||||
const MAX = 40
|
||||
const writable = ctx.folders ?? []
|
||||
@@ -1223,7 +1227,7 @@ function buildFolderGuidance(username: string, ctx?: FolderPromptContext): strin
|
||||
writable.length > 0
|
||||
? ` Folders here include ${fmt(writable)} (you can also write to others not listed).`
|
||||
: ''
|
||||
return `- As a workspace admin you can write to any existing folder.${known} If the user names a folder, use it; if they explicitly ask for a new folder, create it with \`create_folder\`; otherwise ask them which folder to use rather than guessing or creating one unprompted.`
|
||||
return `- As a workspace admin you can write to any existing folder.${known} If the user names a folder, use it;${canCreateFolder ? ' if they explicitly ask for a new folder, create it with `create_folder`;' : ''} otherwise ask them which folder to use rather than guessing${canCreateFolder ? ' or creating one unprompted' : ''}.`
|
||||
}
|
||||
// Everything below states the writable set as fact, including the empty case, so an
|
||||
// unresolved role has to say nothing at all: "you have no shared folders" is a claim,
|
||||
@@ -1234,11 +1238,15 @@ function buildFolderGuidance(username: string, ctx?: FolderPromptContext): strin
|
||||
const lines: string[] = []
|
||||
if (writable.length > 0) {
|
||||
lines.push(
|
||||
`- Folders you can write to in this workspace: ${fmt(writable)}. For shared/team work, pick the one whose purpose matches the request; if none clearly fits, ask which folder to use (askUserQuestion) rather than inventing a path. Use \`create_folder\` only when the user explicitly asks for a new folder.`
|
||||
`- Folders you can write to in this workspace: ${fmt(writable)}. For shared/team work, pick the one whose purpose matches the request; if none clearly fits, ask which folder to use (askUserQuestion) rather than inventing a path.${canCreateFolder ? ' Use `create_folder` only when the user explicitly asks for a new folder.' : ''}`
|
||||
)
|
||||
} else {
|
||||
lines.push(
|
||||
`- You have no shared folders you can write to in this workspace, so use \`u/${username}/<name>\`. If the user explicitly asks for a shared folder, create one with \`create_folder\` (you become an owner); otherwise ask before placing shared work rather than inventing an \`f/<folder>/...\` path.`
|
||||
`- You have no shared folders you can write to in this workspace, so use \`u/${username}/<name>\`. ${
|
||||
canCreateFolder
|
||||
? 'If the user explicitly asks for a shared folder, create one with `create_folder` (you become an owner); otherwise ask'
|
||||
: 'If the user explicitly asks for a shared folder, say plainly that you cannot create one here; otherwise ask'
|
||||
} before placing shared work rather than inventing an \`f/<folder>/...\` path.`
|
||||
)
|
||||
}
|
||||
if (readOnly.length > 0) {
|
||||
@@ -1266,7 +1274,7 @@ const buildGlobalSystemPrompt = (
|
||||
// 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 : '')
|
||||
const folderGuidance = buildFolderGuidance(username, folderCtx)
|
||||
const folderGuidance = buildFolderGuidance(username, folderCtx, canDeploy)
|
||||
const folderGuidanceBlock = folderGuidance ? `\n${folderGuidance}` : ''
|
||||
// `previewTools` doubles as "this is a session chat" — sessions are the only
|
||||
// chats that get the preview tool set. The alpha heads-up only makes sense
|
||||
@@ -1313,7 +1321,10 @@ Path conventions:
|
||||
- \`u/${username}/<name>\` — your personal scope. Default for ad-hoc, exploratory, or scratch work.
|
||||
- \`f/<folder>/<name>\` — a shared folder scope; the <folder> must already exist (a bare \`f/<name>\` with no folder segment is INVALID and will fail).
|
||||
- If the user supplies a fully qualified \`f/<folder>/...\` path, use that exact path; they have already chosen the folder. Do not ask for folder confirmation or substitute a \`u/${username}/...\` path unless a tool rejects it.
|
||||
- Default a bare name with no namespace prefix (e.g. "create a flow called myflow") to \`u/${username}/<name>\`. Never invent an \`f/<folder>/...\` path for a folder that does not exist; create one with \`create_folder\` only when the user explicitly asks for a new folder.${folderGuidanceBlock}`
|
||||
- Default a bare name with no namespace prefix (e.g. "create a flow called myflow") to \`u/${username}/<name>\`. Never invent an \`f/<folder>/...\` path for a folder that does not exist${when(
|
||||
canDeploy,
|
||||
'; create one with `create_folder` only when the user explicitly asks for a new folder'
|
||||
)}.${folderGuidanceBlock}`
|
||||
)}
|
||||
|
||||
Rules:${when(
|
||||
@@ -1380,7 +1391,7 @@ ${pipelineBullet}`
|
||||
canWriteDraft,
|
||||
`
|
||||
- After writing or substantially editing a script / flow / app draft, show it via open_preview(kind, path) so the user sees the editor and live preview right next to the chat. First check whether it is already shown: if unsure, call get_preview_status. Only call open_preview (or offer to) when no preview is open or it is showing a different item — don't re-open a preview already showing the item you just edited.
|
||||
- Building a data pipeline: call open_preview(kind="pipeline", path="<folder>") as the FIRST step, before creating any node — this opens the pipeline editor the user reviews in. path is the folder, not an item; an empty or not-yet-created folder is fine (create_folder first if needed, then open it). Opening it registers build_pipeline_node / edit_pipeline_node — use ONLY those to add or change pipeline nodes, never write_script for a pipeline node — they apply directly as unsaved drafts on the canvas (no separate accept/reject step) that the user reviews and deploys. Do not write pipeline scripts without first opening the editor.`
|
||||
- Building a data pipeline: call open_preview(kind="pipeline", path="<folder>") as the FIRST step, before creating any node — this opens the pipeline editor the user reviews in. path is the folder, not an item; an empty folder is fine${when(canDeploy, ', and a not-yet-created one too (create_folder first, then open it)')}. Opening it registers build_pipeline_node / edit_pipeline_node — use ONLY those to add or change pipeline nodes, never write_script for a pipeline node — they apply directly as unsaved drafts on the canvas (no separate accept/reject step) that the user reviews and deploys. Do not write pipeline scripts without first opening the editor.`
|
||||
)}
|
||||
- When debugging a running raw app, call get_app_runtime_logs to read the live preview's browser console output. It needs the raw app preview open (open_preview kind="raw_app").
|
||||
- To inspect what actually rendered in a running raw app (verify an edit landed on screen, diagnose a blank/empty or wrong view, answer "what's showing"), use search_dom (regex over the live HTML) and read_dom (a line-numbered window). Pass a \`selector\` to scope to an element — prefer the selector from a DOM element chip the user attached — or omit it for the whole page. When a chip lists an \`app_path\`, pass it too so the RIGHT app is read (several previews can be open; a query without \`app_path\` hits the visible one). The DOM is read live and is never in context; no match means the element isn't rendered. Both need the raw app preview open.
|
||||
|
||||
@@ -75,23 +75,11 @@ export async function resolveSessionAccess(workspace: string): Promise<SessionAc
|
||||
capabilities.add('run_preview')
|
||||
}
|
||||
|
||||
// Deploy authorization is operation-shaped, not role-shaped: protection rulesets,
|
||||
// `wm_deployers` membership and per-ruleset bypass lists all feed into it. Reuse the
|
||||
// existing mirror rather than re-deriving it, so the chat and the visible deploy
|
||||
// button can never disagree.
|
||||
//
|
||||
// `is_admin` is folded first because that helper tests `me.is_admin` alone, while the
|
||||
// rule it mirrors receives `authed.is_admin` — superadmin included. Without this a
|
||||
// superadmin who is a plain member of a workspace under `RestrictDeployToDeployers`
|
||||
// would lose the deploy tools the backend grants them.
|
||||
//
|
||||
// `DisableDirectDeployment` is checked on top because `checkDeployPermission` covers
|
||||
// only the operator and `RestrictDeployToDeployers` halves, while the endpoints the
|
||||
// deploy tools call go through the backend's `check_deploy_rules`, which gates on both.
|
||||
// Mirrors the backend's `check_deploy_rules`, which gates on BOTH protection rules.
|
||||
// `checkDeployPermission` carries only the operator and `RestrictDeployToDeployers`
|
||||
// halves, so `DisableDirectDeployment` is checked on top. An admin bypasses every
|
||||
// rule, which is why the rulesets are fetched for everyone else only.
|
||||
const authedAdmin = isAuthedAdmin(me)
|
||||
// An admin bypasses every rule, so the rulesets are only worth fetching for everyone
|
||||
// else. `wm_deployers` membership does not help here: it satisfies
|
||||
// `RestrictDeployToDeployers` alone, never `DisableDirectDeployment`.
|
||||
const rulesets = authedAdmin ? undefined : await fetchProtectionRulesForWorkspace(workspace)
|
||||
const deployAllowed =
|
||||
(await checkDeployPermission(workspace, { ...me, is_admin: authedAdmin })).ok &&
|
||||
|
||||
@@ -105,10 +105,19 @@ describe('session tool policies', () => {
|
||||
const readOnly = accessWith(['deploy'])
|
||||
expect(sessionToolAllowed('get_instructions', readOnly)).toBe(false)
|
||||
expect(sessionToolAllowed('search_npm_packages', readOnly)).toBe(false)
|
||||
expect(sessionToolAllowed('create_folder', readOnly)).toBe(false)
|
||||
expect(sessionToolAllowed('search_docs', readOnly)).toBe(true)
|
||||
})
|
||||
|
||||
// create_folder carries both axes: the backend runs it through check_deploy_rules,
|
||||
// so drafting alone is not enough to make it usable.
|
||||
it('withholds create_folder from a session that cannot deploy', () => {
|
||||
expect(sessionToolAllowed('create_folder', accessWith(['write_draft', 'run_preview']))).toBe(
|
||||
false
|
||||
)
|
||||
expect(sessionToolAllowed('create_folder', accessWith(['write_draft', 'deploy']))).toBe(true)
|
||||
expect(sessionToolAllowed('create_folder', accessWith(['deploy']))).toBe(false)
|
||||
})
|
||||
|
||||
// The prompt is documentation OF the toolset, so it must never name a tool the same
|
||||
// profile withheld — an instruction to call a tool the model was not given is what
|
||||
// produces invented calls and promises the chat cannot keep.
|
||||
|
||||
@@ -92,9 +92,11 @@ export const SESSION_TOOL_POLICIES: Record<string, SessionToolPolicy> = {
|
||||
get_trigger_schema: AUTHORING_AID,
|
||||
get_schedule_schema: AUTHORING_AID,
|
||||
get_db_schema: AUTHORING_AID,
|
||||
// An operator's token may create a folder, but a folder exists to hold
|
||||
// authored items, so it goes with them rather than with the reads.
|
||||
create_folder: AUTHORING_AID,
|
||||
// Folder creation runs through the backend's `check_deploy_rules` (folders.rs
|
||||
// `create_folder`), so a workspace that blocks deploys refuses it too — hence
|
||||
// `deploy`, even though a folder is not a deployed item. It is also useless
|
||||
// without something to put in it, hence the authoring relevance.
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user