fix: check direct-deployment lock and superadmin in the deploy preflight

`checkDeployPermission` mirrors the server's `check_deploy_rules` so the deploy
UI can disable an action with a reason instead of letting the click come back
403. It modelled only `RestrictDeployToDeployers`, leaving two terms out:

- `DisableDirectDeployment` was never evaluated. In a workspace carrying only
  that rule the preflight allowed the deploy and the request 403'd.
- The server bypasses on `ApiAuthed.is_admin`, which is `usr.is_admin ||
  super_admin`, while `whoami` reports the two separately. A superadmin who is
  a plain member of the workspace was refused a deploy the server allows.

Evaluate `DisableDirectDeployment` first, as the server does, so the same
message wins when both rules block, and add the superadmin term to the shared
ruleset bypass helper. `wm_deployers` membership is an implicit pass on
`RestrictDeployToDeployers` alone, so it no longer short-circuits the rules
fetch the way admin does — a deployer is still bound by a direct-deployment
lock, and a test pins that.

The operator refusal stays above the admin/superadmin short-circuit: the server
refuses operators in the item handlers whatever their global role, so a
superadmin who is an operator in the workspace is still refused. Its doc no
longer presents that term as part of the `check_deploy_rules` mirror, since the
rule carries no operator term and refusing every kind here is deliberately
stricter than the server.

Callers no longer name which rules the preflight covers. That list rots at every
site that repeats it, so it lives only at the preflight itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
AlexRV12
2026-08-18 17:30:53 +02:00
parent 9f517d5a40
commit 6f23f14aae
6 changed files with 127 additions and 30 deletions
@@ -304,9 +304,9 @@
let selectedItems = $state<string[]>([])
let deploying = $state(false)
// Whether the user may deploy drafts into this workspace — fills the
// `RestrictDeployToDeployers` (+ operator) gap via the shared util, same as the
// fork compare page and the session review drawer. Fail-open while resolving.
// Whether the user may deploy drafts into this workspace, via the shared util —
// same as the fork compare page and the session review drawer. Fail-open while
// resolving.
let deployPerm = $state<DeployPermission>({ ok: true })
$effect(() => {
const ws = currentWorkspaceId
@@ -972,10 +972,9 @@
fetchPermissions()
})
// Can the user actually deploy into the target workspace? Fills the frontend
// gap for the `RestrictDeployToDeployers` rule (+ operator), shared with the
// session review drawer via the same checkDeployPermission util. Cached per
// workspace; `deployPerm` tracks whichever side the current direction targets.
// Can the user actually deploy into the target workspace? Shared with the session
// review drawer via the same checkDeployPermission util. Cached per workspace;
// `deployPerm` tracks whichever side the current direction targets.
let deployPerms = $state<Record<string, DeployPermission>>({})
const deployPermFetched = new Set<string>()
$effect(() => {
@@ -208,9 +208,9 @@ export function useSessionDeployModel(getArgs: () => SessionDeployModelArgs) {
})
// ── Deploy permission ────────────────────────────────────────────────────
// Preflight the shared checkDeployPermission (operator / RestrictDeployToDeployers)
// for the session workspace so the button disables with a reason instead of
// failing on click. `ok` defaults true while resolving (fail-open).
// Preflight the shared checkDeployPermission for the session workspace so the
// button disables with a reason instead of failing on click. `ok` defaults
// true while resolving (fail-open).
let deployPerm = $state<DeployPermission>({ ok: true })
let deployPermFetchedFor = ''
$effect(() => {
@@ -1,11 +1,21 @@
import { describe, it, expect } from 'vitest'
import { describe, it, expect, vi } from 'vitest'
import type { ProtectionRuleset, ProtectionRuleKind, User } from './gen'
import {
checkDeployPermission,
checkPathWritePermission,
diffActionableInDirection,
diffCreatesInTarget,
diffRemovesInTarget
} from './utils_workspace_deploy'
// Only the rules fetch is stubbed: the bypass logic it feeds is the code under test here,
// so it has to stay real.
let rulesets: ProtectionRuleset[] = []
vi.mock('$lib/workspaceProtectionRules.svelte', async (importOriginal) => ({
...((await importOriginal()) as object),
fetchProtectionRulesForWorkspace: async () => rulesets
}))
/** The row shape the fork comparison returns for an item the parent has and the
* fork does not: one write on the fork side, whatever that write was. */
const parentOnly = { ahead: 1, behind: 0, exists_in_source: true, exists_in_fork: false }
@@ -116,3 +126,72 @@ describe('per-item write permission in the deploy target', () => {
})
})
})
describe('workspace-level deploy permission', () => {
const ruleset = (name: string, rules: ProtectionRuleKind[]): ProtectionRuleset => ({
name,
rules,
bypass_users: [],
bypass_groups: []
})
const member: User = {
email: 'alice@windmill.dev',
username: 'alice',
is_admin: false,
is_super_admin: false,
operator: false,
disabled: false,
created_at: '2024-01-01T00:00:00Z',
groups: [],
folders: [],
folders_read: [],
folders_owners: []
}
const permission = (me: User, rules: ProtectionRuleset[]) => {
rulesets = rules
return checkDeployPermission('prod', me)
}
it('refuses a member when direct deployment is disabled', async () => {
const res = await permission(member, [ruleset('lock', ['DisableDirectDeployment'])])
expect(res.ok).toBe(false)
expect(res.reason).toContain('prod')
})
// wm_deployers is an implicit pass on RestrictDeployToDeployers only. Letting it
// short-circuit the whole check — as an "is this user a deployer?" early return would —
// walks a deployer straight through a deploy-locked workspace.
it('still refuses a wm_deployers member when direct deployment is disabled', async () => {
const deployer = { ...member, groups: ['wm_deployers'] }
expect((await permission(deployer, [ruleset('lock', ['DisableDirectDeployment'])])).ok).toBe(
false
)
expect((await permission(deployer, [ruleset('gate', ['RestrictDeployToDeployers'])])).ok).toBe(
true
)
})
// whoami reports is_admin and is_super_admin separately; the server sees them merged.
it('lets a superadmin who is a plain member deploy', async () => {
const su = { ...member, is_super_admin: true }
expect((await permission(su, [ruleset('lock', ['DisableDirectDeployment'])])).ok).toBe(true)
})
it('reports the direct-deployment refusal when both rules block', async () => {
const res = await permission(member, [
ruleset('lock', ['DisableDirectDeployment', 'RestrictDeployToDeployers'])
])
expect(res.reason).toContain('Direct deployment')
})
// The server refuses an operator in the item handler regardless of their global role: a
// superadmin who is an operator in the workspace still gets 401 creating a script. So the
// operator term has to stay above the admin/superadmin short-circuit — hoisting the admin
// checks would offer a deploy the server refuses.
it('refuses an operator who is also a superadmin', async () => {
const res = await permission({ ...member, operator: true, is_super_admin: true }, [])
expect(res.ok).toBe(false)
expect(res.reason).toContain('operator')
})
})
+33 -15
View File
@@ -706,13 +706,16 @@ export async function createFolderIfAbsent(
export type DeployPermission = { ok: boolean; reason?: string }
/**
* Whether the current user may deploy into `workspace`. Mirrors the server-side
* deploy authorization (`check_user_against_rule` in windmill-common) so the UI
* can disable the action with a reason instead of letting the click 403:
* - operators can never deploy;
* - when the `RestrictDeployToDeployers` protection rule is active, only
* admins, `wm_deployers` members (implicitly), and per-ruleset bypass
* users/groups may deploy.
* Whether the current user may deploy into `workspace`. Mirrors `check_deploy_rules` in
* windmill-common so the UI can disable the action with a reason instead of letting the
* click 403: `DisableDirectDeployment` is evaluated before `RestrictDeployToDeployers`, so
* the same message wins here as on the server when both block; admins and superadmins bypass
* both rules, while `wm_deployers` members bypass only the latter.
*
* The operator refusal is not part of that mirror. The server refuses operators in the item
* handlers instead, and for fewer kinds, so refusing them for everything here is deliberately
* stricter than the server rather than a faithful copy of it.
*
* Fails open on any error the server still enforces on the actual deploy.
* Shared by the session dock and the compare page so both gate identically.
*/
@@ -726,17 +729,32 @@ export async function checkDeployPermission(
if (me.operator) {
return { ok: false, reason: "You're an operator in this workspace — operators can't deploy" }
}
// Admins and wm_deployers members always satisfy RestrictDeployToDeployers
// (the backend allows wm_deployers implicitly, so check it before the
// per-ruleset bypass_users/bypass_groups fallback).
const isDeployer = me.is_admin || (me.groups ?? []).includes('wm_deployers')
if (!isDeployer) {
const userInfo = {
is_admin: !!me.is_admin,
is_super_admin: !!me.is_super_admin,
username: me.username,
groups: me.groups ?? []
}
// A superadmin who is only a plain member of the workspace still bypasses every rule,
// so dropping either term here refuses a deploy the server accepts.
if (!userInfo.is_admin && !userInfo.is_super_admin) {
const rulesets = await fetchProtectionRulesForWorkspace(workspace)
const userInfo = { is_admin: !!me.is_admin, username: me.username, groups: me.groups ?? [] }
if (!canUserBypassRuleKindInRulesets(rulesets, 'RestrictDeployToDeployers', userInfo)) {
if (!canUserBypassRuleKindInRulesets(rulesets, 'DisableDirectDeployment', userInfo)) {
return {
ok: false,
reason: 'Only workspace admins and members of wm_deployers can deploy here'
reason: `Direct deployment to ${workspace} is disabled — fork the workspace or open a pull request`
}
}
// `wm_deployers` membership is an implicit pass on this rule only, so it cannot
// short-circuit the fetch above the way admin does.
const isDeployer = userInfo.groups.includes('wm_deployers')
if (
!isDeployer &&
!canUserBypassRuleKindInRulesets(rulesets, 'RestrictDeployToDeployers', userInfo)
) {
return {
ok: false,
reason: `Only workspace admins and members of wm_deployers can deploy to ${workspace}`
}
}
}
@@ -4,7 +4,7 @@ import type { UserExt } from './stores'
// The slice of the user identity the bypass checks read — structural, so
// callers can pass a whoami response (normalised groups) as well as the
// UserExt store value.
export type RuleBypassUser = Pick<UserExt, 'is_admin' | 'username' | 'groups'>
export type RuleBypassUser = Pick<UserExt, 'is_admin' | 'is_super_admin' | 'username' | 'groups'>
// Mirrors DEV_WORKSPACE_LOCK_RULE_NAME in windmill-common. The pairing owns this rule by name:
// attaching a dev workspace creates it, detaching deletes it. The API refuses to create or delete
@@ -102,11 +102,12 @@ export async function fetchProtectionRulesForWorkspace(
* Checks if a user can bypass a specific ruleset
* @param ruleset The protection ruleset to check
* @param userInfo The user information
* @returns true if user can bypass (is admin, in bypass_users, or has group in bypass_groups)
* @returns true if user can bypass (is admin or superadmin, in bypass_users, or has group in bypass_groups)
*/
export function canUserBypassRule(ruleset: ProtectionRuleset, userInfo: RuleBypassUser): boolean {
// Admin always bypasses
if (userInfo.is_admin) {
// The server bypasses on `ApiAuthed.is_admin`, which it builds as `usr.is_admin ||
// super_admin`, so testing `is_admin` alone understates a superadmin who is a plain member.
if (userInfo.is_admin || userInfo.is_super_admin) {
return true
}