starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-09-21 00:02:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: take the whole deploy gate from the shared preflight

Browse Source 
`checkDeployPermission` now evaluates `DisableDirectDeployment` and folds superadmin
into the admin bypass itself, so the resolver's local composition of those two terms
is redundant. Delegate outright and drop the protection-rule fetch it needed, along
with the two tests that restated rule semantics the preflight's own suite now pins.

The preflight's per-kind narrowing stays unused: the filter runs on tool names, before
the model has named a kind, so a direct-deployment lock withholds the deploy tools for
schedules and triggers too.
This commit is contained in:
AlexRV12
2026-09-09 14:20:20 +02:00
parent 6e1806e65d
commit 4a812034ec
2 changed files with 24 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/lib/components/copilot/chat/global/sessionAccess.test.ts
+15 -61
View File
@@ -1,19 +1,12 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { whoami, deployPermission, protectionRules } = vi.hoisted(() => ({
const { whoami, deployPermission } = vi.hoisted(() => ({
whoami: vi.fn(),
deployPermission: vi.fn(),
protectionRules: vi.fn()
deployPermission: vi.fn()
}))
vi.mock('$lib/gen', () => ({ UserService: { whoami } }))
vi.mock('$lib/utils_workspace_deploy', () => ({ checkDeployPermission: deployPermission }))
// Only the fetch is stubbed — the bypass evaluation is the real one, so the test
// exercises the rule semantics rather than a restatement of them.
vi.mock('$lib/workspaceProtectionRules.svelte', async (importOriginal) => ({
...(await importOriginal<typeof import('$lib/workspaceProtectionRules.svelte')>()),
fetchProtectionRulesForWorkspace: protectionRules
}))
import { resolveSessionAccess } from './sessionAccess'
@@ -46,7 +39,6 @@ describe('resolveSessionAccess', () => {
beforeEach(() => {
vi.clearAllMocks()
deployPermission.mockResolvedValue({ ok: true })
protectionRules.mockResolvedValue([])
})
it('gives a developer every capability', async () => {
@@ -63,24 +55,21 @@ describe('resolveSessionAccess', () => {
// 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.
it('lets an admin who is also an operator write drafts but not run previews', async () => {
deployPermission.mockResolvedValue({ ok: false, reason: 'operators cannot deploy' })
const caps = await capabilitiesFor({ is_admin: true, operator: true })
expect(caps.has('write_draft')).toBe(true)
expect(caps.has('run_preview')).toBe(false)
})
// `ApiAuthed.is_admin` is `usr.is_admin || super_admin`, but whoami reports the two
// separately — so a superadmin who is an operator must resolve the same way.
it('treats a superadmin as an admin for the draft gate', async () => {
deployPermission.mockResolvedValue({ ok: false, reason: 'operators cannot deploy' })
const caps = await capabilitiesFor({ is_super_admin: true, operator: true })
expect(caps.has('write_draft')).toBe(true)
expect(caps.has('run_preview')).toBe(false)
})
// `authed.is_admin` is `usr.is_admin || super_admin`, so both spellings must land
// on the same side of it.
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) => {
deployPermission.mockResolvedValue({ ok: false, reason: 'operators cannot deploy' })
const caps = await capabilitiesFor({ ...role, operator: true })
expect(caps.has('write_draft')).toBe(true)
expect(caps.has('run_preview')).toBe(false)
}
)
// 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 it.
// 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.
it('takes deploy from the shared permission check, not from the role', async () => {
deployPermission.mockResolvedValue({ ok: false, reason: 'restricted to deployers' })
const caps = await capabilitiesFor({})
@@ -88,41 +77,6 @@ describe('resolveSessionAccess', () => {
expect(caps.has('write_draft')).toBe(true)
})
// checkDeployPermission tests `me.is_admin` alone, while the backend rule it mirrors
// receives `authed.is_admin` (workspace admin OR superadmin). The resolver folds the
// two before delegating, so a superadmin keeps deploy under RestrictDeployToDeployers.
it('presents a superadmin as admin to the deploy check', async () => {
whoami.mockResolvedValueOnce(user({ is_super_admin: true }))
await resolveSessionAccess('ws')
expect(deployPermission).toHaveBeenCalledWith(
'ws',
expect.objectContaining({ is_admin: true }),
// An admin bypasses every ruleset, so none are fetched to hand over.
undefined
)
})
// `checkDeployPermission` covers only the operator and RestrictDeployToDeployers halves
// of the gate, but the endpoints the deploy tools call run the backend's
// `check_deploy_rules`, which blocks on DisableDirectDeployment too. Drafting is
// untouched by that rule — it is the deploy that the workspace refuses.
it('withholds deploy under DisableDirectDeployment without a bypass', async () => {
protectionRules.mockResolvedValue([
{ name: 'lock', rules: ['DisableDirectDeployment'], bypass_users: [], bypass_groups: [] }
])
const caps = await capabilitiesFor({})
expect(caps.has('deploy')).toBe(false)
expect(caps.has('write_draft')).toBe(true)
})
it('keeps deploy under DisableDirectDeployment for a bypass user', async () => {
protectionRules.mockResolvedValue([
{ name: 'lock', rules: ['DisableDirectDeployment'], bypass_users: ['u'], bypass_groups: [] }
])
const caps = await capabilitiesFor({})
expect(caps.has('deploy')).toBe(true)
})
// 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 () => {
frontend/src/lib/components/copilot/chat/global/sessionAccess.ts
+9 -30
View File
@@ -1,9 +1,5 @@
import { UserService, type User } from '$lib/gen'
import { checkDeployPermission } from '$lib/utils_workspace_deploy'
import {
canUserBypassRuleKindInRulesets,
fetchProtectionRulesForWorkspace
} from '$lib/workspaceProtectionRules.svelte'
/**
* What a user may do in ONE workspace, as the AI session toolset needs to know it.
@@ -45,15 +41,6 @@ export function hasCapabilities(
return requires.every((c) => access.capabilities.has(c))
}
/**
* `ApiAuthed.is_admin` is `usr.is_admin || super_admin` (windmill-api-auth/src/auth.rs),
* while `whoami` reports the two separately — so every rule below that mirrors an
* `authed.is_admin` check must OR them back together.
*/
function isAuthedAdmin(me: User): boolean {
return !!me.is_admin || !!me.is_super_admin
}
export async function resolveSessionAccess(workspace: string): Promise<SessionAccess> {
let me: User
try {
@@ -66,8 +53,9 @@ export async function resolveSessionAccess(workspace: string): Promise<SessionAc
// 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.
if (isAuthedAdmin(me) || !me.operator) {
// drafts. That field is `usr.is_admin || super_admin` (windmill-api-auth/src/auth.rs)
// while `whoami` reports the two separately, hence the OR.
if (me.is_admin || me.is_super_admin || !me.operator) {
capabilities.add('write_draft')
}
@@ -77,21 +65,12 @@ export async function resolveSessionAccess(workspace: string): Promise<SessionAc
capabilities.add('run_preview')
}
// 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)
const rulesets = authedAdmin ? undefined : await fetchProtectionRulesForWorkspace(workspace)
const deployAllowed =
(await checkDeployPermission(workspace, { ...me, is_admin: authedAdmin })).ok &&
(authedAdmin ||
canUserBypassRuleKindInRulesets(rulesets ?? [], 'DisableDirectDeployment', {
is_admin: false,
username: me.username,
groups: me.groups ?? []
}))
if (deployAllowed) {
// `checkDeployPermission` already carries every term of the backend's `check_deploy_rules`,
// superadmin included, so it is the whole answer here. Its per-kind narrowing
// (`deployPermissionForKind`) deliberately is not: this filter runs on tool NAMES, before
// the model has named a kind, so a direct-deployment lock also withholds the deploy tools
// for schedules and triggers — kinds the server would still accept.
if ((await checkDeployPermission(workspace, me)).ok) {
capabilities.add('deploy')
}