From 4ec64d9ed66cfec5493226ae1366d025e0218899 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Tue, 8 Sep 2026 19:29:22 +0200 Subject: [PATCH] refactor: make the acting workspace and user explicit in the entity editors Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YLxwAsiXJ1Au8CBDBmH7iY --- frontend/src/lib/components/Path.svelte | 34 +++++++++++------- .../src/lib/components/ResourceEditor.svelte | 36 ++++++++++--------- .../components/ResourceEditorDrawer.svelte | 23 +++++++----- .../src/lib/components/ResourceForm.svelte | 12 +++++-- .../src/lib/components/VariableEditor.svelte | 26 +++++++++----- .../src/lib/components/VariableForm.svelte | 16 ++++++--- .../schedules/ScheduleEditorInner.svelte | 35 ++++++++++++++---- .../(root)/(logged)/resources/+page.svelte | 1 + .../(root)/(logged)/variables/+page.svelte | 6 +++- 9 files changed, 130 insertions(+), 59 deletions(-) diff --git a/frontend/src/lib/components/Path.svelte b/frontend/src/lib/components/Path.svelte index 37eef75bf5..37fc0c4b70 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, workspaceStore } from '$lib/stores' + import { superadmin, userStore, workspaceStore, type UserExt } from '$lib/stores' import { createEventDispatcher, getContext, untrack } from 'svelte' import { writable } from 'svelte/store' import { Alert, Button } from './common' @@ -85,6 +85,11 @@ * workspace when the editor operates on a workspace other than the one the * top nav points at (see the sessions preview / dev-workspace flows). */ workspaceOverride?: string + /** The user acting in `workspaceOverride`, for the owner suggestion and the folder + * write flags. Defaults to the navigation `$userStore`, who is only a member of the + * navigation workspace — a caller pointed anywhere else must pass the user resolved + * for that workspace (`getUserExt`). */ + actingUser?: UserExt /** 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. * Every other existing path is still refused. */ @@ -110,11 +115,16 @@ size = 'md', drawerOffset = 0, workspaceOverride = undefined, + actingUser = undefined, allowedExistingPath = undefined, warnOnRename = true }: Props = $props() let ws = $derived(workspaceOverride ?? $workspaceStore) + // Sole place this component falls back to the ambient user; everything below + // reads `user` so a caller acting on another workspace is never mixed with + // the navigation user's memberships. + let user = $derived(actingUser ?? $userStore) $effect.pre(() => { if (path == undefined) { @@ -169,17 +179,17 @@ export async function reset() { if (path == '' || path == 'u//' || path?.startsWith('tmp/') || path?.startsWith('hub/')) { - if ($lastMetaUsed == undefined || $lastMetaUsed.owner != $userStore?.username) { + if ($lastMetaUsed == undefined || $lastMetaUsed.owner != user?.username) { meta = { ownerKind: hideUser ? 'folder' : 'user', name: fullNamePlaceholder ?? random_adj() + '_' + namePlaceholder, owner: '' } if (!hideUser) { - if ($userStore?.username?.includes('@')) { - meta.owner = $userStore!.username.split('@')[0].replace(/[^a-zA-Z0-9_]/g, '') + if (user?.username?.includes('@')) { + meta.owner = user!.username.split('@')[0].replace(/[^a-zA-Z0-9_]/g, '') } else { - meta.owner = $userStore!.username! + meta.owner = user!.username! } } } else { @@ -229,9 +239,9 @@ .map((x) => ({ name: x, write: - $userStore?.folders?.includes(x) == true || - ($userStore?.is_admin ?? false) || - ($userStore?.is_super_admin ?? false) + user?.folders?.includes(x) == true || + (user?.is_admin ?? false) || + (user?.is_super_admin ?? false) })) ) } @@ -423,7 +433,7 @@ }) }) $effect.pre(() => { - if (ws && $userStore) { + if (ws && user) { untrack(() => { loadFolders() initPath() @@ -506,7 +516,7 @@ } else { // 'group' is unreachable here (Select only offers user/folder) // but validateName still accepts it for forward-compat. - meta.owner = $userStore?.username?.split('@')[0] ?? '' + meta.owner = user?.username?.split('@')[0] ?? '' } } } @@ -520,7 +530,7 @@
{#if meta.ownerKind === 'user'} {@const userOwnerDisabled = - disabled || !($superadmin || ($userStore?.is_admin ?? false)) || disableEditing} + disabled || !($superadmin || (user?.is_admin ?? false)) || disableEditing}
@@ -246,7 +252,7 @@ workspaceOverride={workspace} /> {/if} - {#if resource_type === 'git_repository' && $workspaceStore && ($userStore?.is_admin || $userStore?.is_super_admin)} + {#if resource_type === 'git_repository' && ws && (actingUser?.is_admin || actingUser?.is_super_admin)} import { VariableService, WorkspaceService } from '$lib/gen' import { createEventDispatcher, untrack } from 'svelte' - import { userStore, workspaceStore } from '$lib/stores' + import { workspaceStore } from '$lib/stores' import { Button } from './common' import Drawer from './common/drawer/Drawer.svelte' import DrawerContent from './common/drawer/DrawerContent.svelte' @@ -38,8 +38,10 @@ // The "current" workspace this editor defaults New/Edit actions to. Session // editors pass their acting workspace so secrets are created/updated there - // rather than in the navigation workspace. Defaults to $workspaceStore. + // rather than in the navigation workspace. let { workspace = undefined }: { workspace?: string } = $props() + // Sole ambient read in this file: the acting workspace is an input, and only its + // default comes from the navigation store. let curWs = $derived(workspace ?? $workspaceStore) let editPath: string | undefined = $state(undefined) @@ -53,6 +55,10 @@ let initialStates: Record = $state({}) let existedInitially: Record = $state({}) let extraPerms: Record> = $state({}) + // The user acting in each loaded workspace, fetched alongside the variable. `undefined` + // stands for "we don't know" — a lookup still in flight or one that failed — and + // `canWrite` refuses for an unknown user, which is the only safe answer: the navigation + // user's rights are another workspace's. let perWsUser: Record = $state({}) let selected: string | undefined = $state(undefined) let pathError = $state('') @@ -106,11 +112,14 @@ pageDrawerSessionSource(VARIABLES_PATH, editPath, selected ?? curWs) ) const current = $derived(selected ? states[selected]?.draft : undefined) - const can_write = $derived.by(() => { + // `undefined` until the selected workspace's permissions and acting user have both + // landed — a pending verdict is neither a grant nor the denial the read-only alert + // announces, so the two must stay distinguishable. + const can_write: boolean | undefined = $derived.by(() => { if (!selected || !edit) return true const perms = extraPerms[selected] - if (!perms) return true - return canWrite(editPath ?? '', perms, perWsUser[selected] ?? $userStore) + if (!perms) return undefined + return canWrite(editPath ?? '', perms, perWsUser[selected]) }) const dirtyWorkspaces = $derived( Object.keys(states).filter((ws) => !draftValuesEqual(states[ws].draft, initialStates[ws])) @@ -154,7 +163,7 @@ const dirtyCanWrite = $derived( dirtyWorkspaces.every((ws) => { const perms = extraPerms[ws] - return !perms || canWrite(editPath ?? '', perms, perWsUser[ws] ?? $userStore) + return !perms || canWrite(editPath ?? '', perms, perWsUser[ws]) }) ) @@ -329,7 +338,7 @@ /> {/snippet}
- {#if !can_write} + {#if can_write === false} You only have read access to this resource and cannot edit it @@ -352,10 +361,11 @@ bind:wsSpecific={current.wsSpecific} {initialPath} deployTo={deployTo.current} - {can_write} + can_write={can_write === true} {edit} onLoadSecret={loadSecret} {workspace} + actingUser={selected ? perWsUser[selected] : undefined} /> {/key} {/if} diff --git a/frontend/src/lib/components/VariableForm.svelte b/frontend/src/lib/components/VariableForm.svelte index 662f949957..058d9c6f54 100644 --- a/frontend/src/lib/components/VariableForm.svelte +++ b/frontend/src/lib/components/VariableForm.svelte @@ -10,7 +10,7 @@ import ToggleButton from './common/toggleButton-v2/ToggleButton.svelte' import { Loader2, RotateCcw } from 'lucide-svelte' import autosize from '$lib/autosize' - import { userStore, workspaceStore } from '$lib/stores' + import { workspaceStore, type UserExt } from '$lib/stores' import { isOwner } from '$lib/utils' import { isEncryptedDraftValue } from '$lib/encryptedDraft' import EncryptedDraftField from './EncryptedDraftField.svelte' @@ -34,6 +34,10 @@ onLoadSecret?: () => void /** Workspace the path is validated against; defaults to the nav workspace. */ workspace?: string | undefined + /** The user acting in `workspace`, resolved by the editor above. `undefined` while + * that lookup is pending or after it failed: every check below then refuses, rather + * than answering with the navigation user's rights in another workspace. */ + actingUser: UserExt | undefined } let { @@ -47,7 +51,8 @@ can_write, edit, onLoadSecret, - workspace = undefined + workspace = undefined, + actingUser }: Props = $props() let ws = $derived(workspace ?? $workspaceStore) @@ -71,13 +76,14 @@
@@ -89,7 +95,7 @@ edit && !hasStagedValue && onLoadSecret?.()} bind:checked={variable.is_secret} - disabled={edit && ($userStore?.operator || isEncryptedDraftValue(variable.value))} + disabled={edit && (actingUser?.operator || isEncryptedDraftValue(variable.value))} /> {#if variable.is_secret} @@ -131,7 +137,7 @@ > Reset - {:else if $userStore?.operator} + {:else if actingUser?.operator}
Operators cannot load secret value
{:else}