mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: default Path's owner controls to the user acting in its workspace
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
43ed8c6d56
commit
b2596c6adc
@@ -26,7 +26,7 @@
|
||||
AzureTriggerService,
|
||||
EmailTriggerService
|
||||
} from '$lib/gen'
|
||||
import { superadmin, userStore, type UserExt } from '$lib/stores'
|
||||
import { superadmin, type UserExt } from '$lib/stores'
|
||||
import { createEventDispatcher, getContext, untrack } from 'svelte'
|
||||
import { writable } from 'svelte/store'
|
||||
import { Alert, Button } from './common'
|
||||
@@ -46,11 +46,13 @@
|
||||
import { twMerge } from 'tailwind-merge'
|
||||
import InputError from './InputError.svelte'
|
||||
import {
|
||||
useOperatingUser,
|
||||
useOperatingWorkspace,
|
||||
useOperatingWorkspaceHref
|
||||
} from '$lib/components/operatingWorkspace.svelte'
|
||||
|
||||
const operatingWorkspace = useOperatingWorkspace()
|
||||
const operatingUser = useOperatingUser()
|
||||
const operatingHref = useOperatingWorkspaceHref()
|
||||
|
||||
type PathKind =
|
||||
@@ -92,9 +94,8 @@
|
||||
* operating workspace (see `useOperatingWorkspace`). */
|
||||
workspaceOverride?: string
|
||||
/** The user acting in `workspaceOverride`, for the owner suggestion and the folder
|
||||
* write flags. Omit it to stand in the navigation `$userStore`, who is a member of
|
||||
* the navigation workspace only; pass `null` for "not known (yet)", which that user
|
||||
* must not answer for either. */
|
||||
* write flags. Omit it to stand in the user acting in the operating workspace; pass
|
||||
* `null` for "not known (yet)", which no user must answer for. */
|
||||
actingUser?: UserExt | null
|
||||
/** One path that does not count as taken, for a caller creating something that may
|
||||
* already have written there itself — a setup flow correcting its own failed attempt.
|
||||
@@ -127,10 +128,10 @@
|
||||
}: Props = $props()
|
||||
|
||||
let ws = $derived(workspaceOverride ?? $operatingWorkspace)
|
||||
// Sole place this component falls back to the ambient user, and only for a caller that
|
||||
// passed none; everything below reads `user`, so a caller acting on another workspace is
|
||||
// never mixed with the navigation user's memberships.
|
||||
let user = $derived(actingUser === undefined ? $userStore : (actingUser ?? undefined))
|
||||
// Sole place this component falls back to an ambient user, and only for a caller that passed
|
||||
// none: the one acting in `ws`, never the navigation user, whose memberships belong to
|
||||
// another workspace. Everything below reads `user`.
|
||||
let user = $derived(actingUser === undefined ? operatingUser.in(ws) : (actingUser ?? undefined))
|
||||
|
||||
$effect.pre(() => {
|
||||
if (path == undefined) {
|
||||
|
||||
@@ -72,6 +72,20 @@ function readsNavigationStore(file: string): boolean {
|
||||
// Roles are per workspace, so a permission judged from `$userStore` inside a fork's editor
|
||||
// answers about the parent: it disables edits the user may make, or offers ones the backend
|
||||
// then refuses. Identity (username, email) is the same person in either, and stays.
|
||||
// Following one alias, the shape that slipped past this before: `let user = $derived(… $userStore
|
||||
// …)` with the permission checks reading `user`. Aliasing the navigation user for identity —
|
||||
// their username, or which workspace they are on — is fine and stays.
|
||||
function judgesByAliasedNavigationUser(code: string): boolean {
|
||||
const aliases = [...code.matchAll(/(?:let|const)\s+(\w+)\s*=\s*\$derived[^\n]*\$userStore/g)].map(
|
||||
(m) => m[1]
|
||||
)
|
||||
return aliases.some((name) =>
|
||||
new RegExp(
|
||||
`${name}(?:\\?|!)?\\.(?:is_admin|is_super_admin|operator|folders|groups)\\b|canWrite\\((?:[^()]|\\([^()]*\\))*?\\b${name}\\b|isOwner\\((?:[^()]|\\([^()]*\\))*?\\b${name}\\b`
|
||||
).test(code)
|
||||
)
|
||||
}
|
||||
|
||||
const PERMISSION_READ =
|
||||
/canWrite\((?:[^()]|\([^()]*\))*?\$userStore|isOwner\((?:[^()]|\([^()]*\))*?\$userStore|\$userStore(?:\?|!)?\.(?:is_admin|is_super_admin|operator|folders|groups)\b/
|
||||
|
||||
@@ -95,9 +109,11 @@ describe('components under a session editor', () => {
|
||||
it('judge permissions by the user acting in that workspace', () => {
|
||||
const reachable = reachableComponents()
|
||||
expect(reachable).toContain('triggers/PermissionedAsLine.svelte')
|
||||
const offenders = reachable.filter(
|
||||
(f) => !(f in NAVIGATION_JUDGES) && PERMISSION_READ.test(instanceCode(f))
|
||||
)
|
||||
const offenders = reachable.filter((f) => {
|
||||
if (f in NAVIGATION_JUDGES) return false
|
||||
const code = instanceCode(f)
|
||||
return PERMISSION_READ.test(code) || judgesByAliasedNavigationUser(code)
|
||||
})
|
||||
expect(offenders).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user