mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
docs: state each capability invariant once, and document the draft discard
The asymmetric admin/operator precedence was restated three times in sessionAccess.ts and again in its test, the fail-open rationale twice, and the deploy split across four sites. Each now lives at the one place someone would break it, within the four-line budget, with the other sites pointing at it. Ungating discard_local_draft left it undocumented for the read-only profile, which is the profile the backend exemption exists for: the only bullet naming it sits under the draft-writing gate, beneath an opener saying no change is possible. Add the one line that profile needs.
This commit is contained in:
AlexRV12
@@ -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.'
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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<SessionAc
|
||||
|
||||
const capabilities = new Set<SessionCapability>()
|
||||
|
||||
// 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')
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -92,10 +92,8 @@ export const SESSION_TOOL_POLICIES: Record<string, SessionToolPolicy> = {
|
||||
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<string, SessionToolPolicy> = {
|
||||
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,
|
||||
|
||||
|
||||
Reference in New Issue
Block a user