From b2596c6adc4e69e33bb91eca77c61209b31415ce Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Fri, 18 Sep 2026 16:08:43 +0200 Subject: [PATCH] fix: default Path's owner controls to the user acting in its workspace Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/lib/components/Path.svelte | 17 +++++++------- .../lib/components/operatingWorkspace.test.ts | 22 ++++++++++++++++--- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/frontend/src/lib/components/Path.svelte b/frontend/src/lib/components/Path.svelte index 4ea14ed6f9..68604a4a91 100644 --- a/frontend/src/lib/components/Path.svelte +++ b/frontend/src/lib/components/Path.svelte @@ -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) { diff --git a/frontend/src/lib/components/operatingWorkspace.test.ts b/frontend/src/lib/components/operatingWorkspace.test.ts index 72ce0203b3..1c14aa7262 100644 --- a/frontend/src/lib/components/operatingWorkspace.test.ts +++ b/frontend/src/lib/components/operatingWorkspace.test.ts @@ -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([]) }) })