mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-19 00:02:03 +00:00
feat(dev-workspace): reflect existing protection rules in lock toggles (#10093)
* feat(dev-workspace): reflect existing protection rules in lock toggles When creating or attaching a dev workspace, the "block direct edits" and "prevent forking" toggles now check the root workspace's current protection rules. If a restriction is already enforced by an existing rule, its toggle is shown on but locked, with a note, instead of offering a fresh default that could misrepresent the effect. The value sent to the backend is derived so it stays consistent with what the locked toggle shows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: clarify fail-open comment on dev-workspace lock toggles Reword the protection-rule fetch comment so the fallback path isn't misread as dropping protection: a failed fetch falls back to the editable default-on toggle, and any real rule still enforces server-side. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(dev-workspace): lock protection toggles until rules load The lock toggles derived alreadyBlocks* from an async fetch, so during the load window (and the first frame before loading flips) they were editable and the effective value could be false. A user could turn a lock off and submit before an existing rule was detected, omitting the reserved rule and silently leaving prod unprotected once that existing rule was later removed. Treat "rules not yet known" (loading || current === undefined) the same as "already enforced": lock the toggle on and keep the effective value true during that window, so the request can never submit false before the fetch resolves. Submission stays available (a hung fetch degrades to over-protection, not a blocked form). Also fixes the stale-value flash when switching base workspace. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(dev-workspace): honor rule bypasses and guard stale protection fetches Two issues in the protection-rule awareness for the dev-workspace lock toggles: - Bypassable rules became unconditional locks. alreadyBlocks* used isRuleActiveInRulesets, which ignores bypass_users/bypass_groups, and forced the request flag to true. The reserved dev_workspace_lock rule is created with empty bypass lists, so layering it over an existing rule that let specific users through revoked their deploy/forking access. Switch to isRuleUnconditionallyActiveInRulesets so a toggle is only shown as already enforced (locked) when an existing rule has no bypasses; a bypassable rule stays editable, making the lock the user's explicit choice. - A stale protection fetch could apply another base's rules. The generated client can't take an abort signal, so a delayed response for a previous base could overwrite the newly selected one. Tag each result with its workspace and only trust a result matching the current base; also throw AbortError from a superseded fetch so it can't overwrite current. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: condense protection helper comment to four lines Trim the isRuleUnconditionallyActiveInRulesets doc comment to satisfy the AGENTS.md ≤4-line comment rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(dev-workspace): align already-enforced note under the toggle label The note used ml-8, landing under the toggle switch rather than aligned with the switch edge or the label, so it read as floating. Bump to ml-11 so it lines up under the label as helper text for that toggle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
hugocasa
GitHub
@@ -10,7 +10,11 @@
|
||||
import { base } from '$lib/base'
|
||||
import { findCanonicalDevWorkspace } from '$lib/utils/workspaceHierarchy'
|
||||
import { devBadgeText, devLabelKey, devLabelNoun } from '$lib/utils/devWorkspaceLabel'
|
||||
import { loadProtectionRules } from '$lib/workspaceProtectionRules.svelte'
|
||||
import {
|
||||
loadProtectionRules,
|
||||
fetchProtectionRulesForWorkspace,
|
||||
isRuleUnconditionallyActiveInRulesets
|
||||
} from '$lib/workspaceProtectionRules.svelte'
|
||||
import { GitFork, ExternalLink } from 'lucide-svelte'
|
||||
import { resource } from 'runed'
|
||||
|
||||
@@ -55,6 +59,48 @@
|
||||
let busy = $state(false)
|
||||
let labelBusy = $state(false)
|
||||
|
||||
// If this workspace already blocks direct deploy / forking through an existing protection rule, keep
|
||||
// the matching lock toggle on but locked: attaching only manages its own reserved dev-workspace rule,
|
||||
// so turning it "off" here couldn't lift a separately-defined block. Fetched only while the attach form
|
||||
// is on screen; a failed fetch falls back to the editable default-on toggle (real rules still enforce).
|
||||
const rootProtectionRules = resource(
|
||||
() => (!parentId && !pairedDev ? $workspaceStore : undefined),
|
||||
async (ws, _prev, { signal }) => {
|
||||
if (!ws) return undefined
|
||||
const rules = await fetchProtectionRulesForWorkspace(ws)
|
||||
// The generated client can't take an abort signal, so drop a superseded response here: a late
|
||||
// result for a previously selected workspace must not overwrite the current one's rules.
|
||||
if (signal.aborted) throw new DOMException('superseded', 'AbortError')
|
||||
return { ws, rules }
|
||||
}
|
||||
)
|
||||
// Only trust a result that belongs to the current workspace (guards the in-flight window and any
|
||||
// out-of-order response); undefined means "not known yet" and is treated as locked below.
|
||||
let rootRules = $derived.by(() => {
|
||||
const current = rootProtectionRules.current
|
||||
return current && current.ws === $workspaceStore ? current.rules : undefined
|
||||
})
|
||||
// Only a rule with no bypass users/groups matches the empty-bypass reserved lock we would create; a
|
||||
// bypassable rule stays editable, otherwise forcing the lock on would revoke the bypassed users'
|
||||
// direct-deploy / forking access.
|
||||
let alreadyBlocksDeploy = $derived(
|
||||
isRuleUnconditionallyActiveInRulesets(rootRules ?? [], 'DisableDirectDeployment')
|
||||
)
|
||||
let alreadyBlocksForking = $derived(
|
||||
isRuleUnconditionallyActiveInRulesets(rootRules ?? [], 'DisableWorkspaceForking')
|
||||
)
|
||||
// Until the fetch resolves for the current workspace its rules are unknown. Treat each lock as
|
||||
// engaged during that window so the toggle is locked on and the effective value stays true:
|
||||
// otherwise a user could turn a lock off and attach before an existing rule is detected, sending
|
||||
// false and omitting the reserved rule — leaving prod unprotected if that rule is later removed.
|
||||
let rulesUnknown = $derived(rootProtectionRules.loading || rootRules === undefined)
|
||||
let deployLocked = $derived(alreadyBlocksDeploy || rulesUnknown)
|
||||
let forkingLocked = $derived(alreadyBlocksForking || rulesUnknown)
|
||||
// Sent to the backend: a locked restriction (enforced or not-yet-known) stays on regardless of the
|
||||
// toggle's raw state, keeping the request consistent with what the locked toggle shows.
|
||||
let effectiveLockProdDeploy = $derived(deployLocked || lockProdDeploy)
|
||||
let effectiveLockProdForking = $derived(forkingLocked || lockProdForking)
|
||||
|
||||
// A standalone root workspace, or an existing fork of this prod (same family), can be attached.
|
||||
// A fork parented to a different workspace can't (the backend rejects a parent that isn't this
|
||||
// prod), so it's excluded here.
|
||||
@@ -92,8 +138,8 @@
|
||||
workspace: $workspaceStore,
|
||||
requestBody: {
|
||||
dev_workspace_id: selectedDevId,
|
||||
lock_prod_deploy: lockProdDeploy,
|
||||
lock_prod_forking: lockProdForking,
|
||||
lock_prod_deploy: effectiveLockProdDeploy,
|
||||
lock_prod_forking: effectiveLockProdForking,
|
||||
dev_workspace_label: attachLabel
|
||||
}
|
||||
})
|
||||
@@ -217,13 +263,44 @@
|
||||
Change to {attachLabel === 'staging' ? 'dev' : 'staging'}
|
||||
</button>
|
||||
</div>
|
||||
<Toggle
|
||||
bind:checked={lockProdDeploy}
|
||||
options={{
|
||||
right: 'Block direct edits in this workspace (deploy via the dev workspace)'
|
||||
}}
|
||||
/>
|
||||
<Toggle bind:checked={lockProdForking} options={{ right: 'Prevent forking this workspace' }} />
|
||||
{#if deployLocked}
|
||||
<div class="flex flex-col gap-0.5">
|
||||
<Toggle
|
||||
checked
|
||||
disabled
|
||||
options={{
|
||||
right: 'Block direct edits in this workspace (deploy via the dev workspace)'
|
||||
}}
|
||||
/>
|
||||
{#if alreadyBlocksDeploy}
|
||||
<span class="text-2xs text-secondary ml-11"
|
||||
>Already enforced by an existing protection rule</span
|
||||
>
|
||||
{/if}
|
||||
</div>
|
||||
{:else}
|
||||
<Toggle
|
||||
bind:checked={lockProdDeploy}
|
||||
options={{
|
||||
right: 'Block direct edits in this workspace (deploy via the dev workspace)'
|
||||
}}
|
||||
/>
|
||||
{/if}
|
||||
{#if forkingLocked}
|
||||
<div class="flex flex-col gap-0.5">
|
||||
<Toggle checked disabled options={{ right: 'Prevent forking this workspace' }} />
|
||||
{#if alreadyBlocksForking}
|
||||
<span class="text-2xs text-secondary ml-11"
|
||||
>Already enforced by an existing protection rule</span
|
||||
>
|
||||
{/if}
|
||||
</div>
|
||||
{:else}
|
||||
<Toggle
|
||||
bind:checked={lockProdForking}
|
||||
options={{ right: 'Prevent forking this workspace' }}
|
||||
/>
|
||||
{/if}
|
||||
<div class="flex gap-2">
|
||||
<Button variant="accent" disabled={busy || !selectedDevId} onclick={attach}>
|
||||
Attach dev workspace
|
||||
|
||||
@@ -21,6 +21,10 @@
|
||||
findWorkspaceDescendants
|
||||
} from '$lib/utils/workspaceHierarchy'
|
||||
import { useForkableWorkspaces } from '$lib/utils/useForkableWorkspaces.svelte'
|
||||
import {
|
||||
fetchProtectionRulesForWorkspace,
|
||||
isRuleUnconditionallyActiveInRulesets
|
||||
} from '$lib/workspaceProtectionRules.svelte'
|
||||
import { resource } from 'runed'
|
||||
import { Badge, Button } from '$lib/components/common'
|
||||
import { devBadgeText } from '$lib/utils/devWorkspaceLabel'
|
||||
@@ -143,6 +147,48 @@
|
||||
baseWorkspaceEntry?.name ?? baseWorkspaceId ?? 'the root workspace'
|
||||
)
|
||||
|
||||
// If the root already blocks direct deploy / forking through an existing protection rule, keep the
|
||||
// matching lock toggle on but locked: this flow only manages its own reserved dev-workspace rule, so
|
||||
// turning it "off" here couldn't lift a separately-defined block. On a failed fetch we fall back to the
|
||||
// editable default-on toggle, which can't drop protection (any real rule still enforces server-side).
|
||||
const rootProtectionRules = resource(
|
||||
() => (canDesignateDevWorkspace && createAsDevWorkspace ? baseWorkspaceId : undefined),
|
||||
async (ws, _prev, { signal }) => {
|
||||
if (!ws) return undefined
|
||||
const rules = await fetchProtectionRulesForWorkspace(ws)
|
||||
// The generated client can't take an abort signal, so drop a superseded response here: a late
|
||||
// result for a previous base must not overwrite the newly selected base's rules.
|
||||
if (signal.aborted) throw new DOMException('superseded', 'AbortError')
|
||||
return { ws, rules }
|
||||
}
|
||||
)
|
||||
// Only trust a result that belongs to the currently selected base (guards the in-flight window and
|
||||
// any out-of-order response); undefined means "not known yet" and is treated as locked below.
|
||||
let rootRules = $derived.by(() => {
|
||||
const current = rootProtectionRules.current
|
||||
return current && current.ws === baseWorkspaceId ? current.rules : undefined
|
||||
})
|
||||
// Only a rule with no bypass users/groups matches the empty-bypass reserved lock we would create; a
|
||||
// bypassable rule stays editable, otherwise forcing the lock on would revoke the bypassed users'
|
||||
// direct-deploy / forking access.
|
||||
let rootAlreadyBlocksDeploy = $derived(
|
||||
isRuleUnconditionallyActiveInRulesets(rootRules ?? [], 'DisableDirectDeployment')
|
||||
)
|
||||
let rootAlreadyBlocksForking = $derived(
|
||||
isRuleUnconditionallyActiveInRulesets(rootRules ?? [], 'DisableWorkspaceForking')
|
||||
)
|
||||
// Until the fetch resolves for the selected base its rules are unknown. Treat each lock as engaged
|
||||
// during that window so the toggle is locked on and the effective value stays true: otherwise a user
|
||||
// could turn a lock off and submit before an existing rule is detected, sending false and omitting
|
||||
// the reserved rule — leaving prod unprotected if that existing rule is later removed.
|
||||
let rootRulesUnknown = $derived(rootProtectionRules.loading || rootRules === undefined)
|
||||
let deployLocked = $derived(rootAlreadyBlocksDeploy || rootRulesUnknown)
|
||||
let forkingLocked = $derived(rootAlreadyBlocksForking || rootRulesUnknown)
|
||||
// Sent to the backend: a locked restriction (enforced or not-yet-known) stays on regardless of the
|
||||
// toggle's raw state, keeping the request consistent with what the locked toggle shows.
|
||||
let effectiveLockProdDeploy = $derived(deployLocked || lockProdDeploy)
|
||||
let effectiveLockProdForking = $derived(forkingLocked || lockProdForking)
|
||||
|
||||
let id = $state('')
|
||||
let name = $state('')
|
||||
let username = $state('')
|
||||
@@ -306,8 +352,8 @@
|
||||
dev_workspace_label: createAsDevWorkspace ? devWorkspaceLabel : undefined,
|
||||
// Send the lock intent in this first phase too so the backend can reject a non-admin's
|
||||
// locked-dev request before any branch is created (avoids dangling branches).
|
||||
lock_prod_deploy: createAsDevWorkspace && lockProdDeploy,
|
||||
lock_prod_forking: createAsDevWorkspace && lockProdForking,
|
||||
lock_prod_deploy: createAsDevWorkspace && effectiveLockProdDeploy,
|
||||
lock_prod_forking: createAsDevWorkspace && effectiveLockProdForking,
|
||||
copy_members: copyMembers
|
||||
}
|
||||
})
|
||||
@@ -374,8 +420,8 @@
|
||||
shared_ducklakes: forkDucklakeSection?.getSharedDucklakes() ?? [],
|
||||
is_dev_workspace: createAsDevWorkspace,
|
||||
dev_workspace_label: createAsDevWorkspace ? devWorkspaceLabel : undefined,
|
||||
lock_prod_deploy: createAsDevWorkspace && lockProdDeploy,
|
||||
lock_prod_forking: createAsDevWorkspace && lockProdForking,
|
||||
lock_prod_deploy: createAsDevWorkspace && effectiveLockProdDeploy,
|
||||
lock_prod_forking: createAsDevWorkspace && effectiveLockProdForking,
|
||||
copy_members: copyMembers
|
||||
}
|
||||
})
|
||||
@@ -769,11 +815,37 @@
|
||||
dev workspace and promoted here.
|
||||
</span>
|
||||
</div>
|
||||
<Toggle
|
||||
bind:checked={lockProdDeploy}
|
||||
options={{ right: 'Block direct edits (deploy via the dev workspace)' }}
|
||||
/>
|
||||
<Toggle bind:checked={lockProdForking} options={{ right: 'Prevent forking' }} />
|
||||
{#if deployLocked}
|
||||
<div class="flex flex-col gap-0.5">
|
||||
<Toggle
|
||||
checked
|
||||
disabled
|
||||
options={{ right: 'Block direct edits (deploy via the dev workspace)' }}
|
||||
/>
|
||||
{#if rootAlreadyBlocksDeploy}
|
||||
<span class="text-2xs text-secondary ml-11"
|
||||
>Already enforced by an existing protection rule</span
|
||||
>
|
||||
{/if}
|
||||
</div>
|
||||
{:else}
|
||||
<Toggle
|
||||
bind:checked={lockProdDeploy}
|
||||
options={{ right: 'Block direct edits (deploy via the dev workspace)' }}
|
||||
/>
|
||||
{/if}
|
||||
{#if forkingLocked}
|
||||
<div class="flex flex-col gap-0.5">
|
||||
<Toggle checked disabled options={{ right: 'Prevent forking' }} />
|
||||
{#if rootAlreadyBlocksForking}
|
||||
<span class="text-2xs text-secondary ml-11"
|
||||
>Already enforced by an existing protection rule</span
|
||||
>
|
||||
{/if}
|
||||
</div>
|
||||
{:else}
|
||||
<Toggle bind:checked={lockProdForking} options={{ right: 'Prevent forking' }} />
|
||||
{/if}
|
||||
</div>
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
@@ -182,6 +182,23 @@ export function isRuleActiveInRulesets(
|
||||
return rulesets.some((ruleset) => ruleset.rules.includes(ruleKind))
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a rule kind is enforced with no bypass users/groups in at least one ruleset, the only case
|
||||
* that matches the empty-bypass reserved dev-workspace lock. A bypassable rule does not, since adding
|
||||
* the unconditional lock would revoke those users' access; callers keep such a toggle editable.
|
||||
*/
|
||||
export function isRuleUnconditionallyActiveInRulesets(
|
||||
rulesets: ProtectionRuleset[],
|
||||
ruleKind: ProtectionRuleKind
|
||||
): boolean {
|
||||
return rulesets.some(
|
||||
(ruleset) =>
|
||||
ruleset.rules.includes(ruleKind) &&
|
||||
ruleset.bypass_users.length === 0 &&
|
||||
ruleset.bypass_groups.length === 0
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if user can bypass a rule kind in given rulesets (workspace-agnostic version)
|
||||
* @param rulesets Array of protection rulesets to check
|
||||
|
||||
Reference in New Issue
Block a user