fix: store the git credential only once the resource is saved

backend/ee-repo-ref.txt

@@ -1 +1 @@
-cf48cb048f413cc922cce08b303e40dfebb6b897
+6f03a04595174228a4609b76725394c7feedea00

docs/git-sync-gitlab-setup.md

@@ -68,8 +68,15 @@ Give the resource its final path before picking a project. The token is filed
 under that path, so renaming afterwards leaves it behind.
 
 Forks of the workspace read this one copy rather than getting their own, so
-renewal reaches all of them at once and no fork holds a credential a fork admin
-could read.
+renewal reaches all of them at once and the token is not duplicated into every
+descendant workspace.
+
+Treat workspace admin as equivalent to holding the token. An admin of the
+workspace, or of any fork below it, can point a repository at a sync script they
+wrote and have that job request the credential, exactly as they can for a GitHub
+App installation token. Storing it this way keeps it out of the variables API and
+out of every fork's own storage; it is not a boundary against the admins of those
+workspaces.
 
 A URL with the token written into it keeps working, whether it sits in the
 resource or in a secret variable the resource points at (`"url": "$var:..."`),

frontend/src/lib/components/ApiConnectForm.svelte

@@ -29,13 +29,13 @@
 		isValid?: boolean
 		linkedSecretCandidates?: string[] | undefined
 		description?: string | undefined
-		/** Path the resource is being saved at, passed through to the GitLab picker
-		 * so the credential it stores is keyed by the resource's own path. */
-		resourcePath?: string
 		/** Workspace the resource is being saved into, which is not always the one
 		 * being navigated. The GitLab picker has to store the credential where the
 		 * resource will look for it. */
 		workspace?: string
+		/** A git credential the picker chose, for the drawer to store once it has
+		 * saved the resource and its path is final. */
+		onCredentialSelected?: (credential: { token: string; repoUrl: string }) => void
 		onSynced?: () => void
 	}
 
@@ -47,8 +47,8 @@
 		isValid = $bindable(true),
 		linkedSecretCandidates = undefined,
 		description = $bindable(undefined),
-		resourcePath = undefined,
 		workspace = undefined,
+		onCredentialSelected,
 		onSynced = undefined
 	}: Props = $props()
 
@@ -264,8 +264,8 @@
 		<GitLabIntegration
 			{resourceType}
 			{args}
-			{resourcePath}
 			{workspace}
+			{onCredentialSelected}
 			onArgsUpdate={(newArgs) => {
 				args = newArgs
 				rawCode = JSON.stringify(args, null, 2)

frontend/src/lib/components/AppConnectInner.svelte

@@ -12,6 +12,7 @@
 		sortResourceTypesByMatch
 	} from './resourceTypeDisplay'
 	import {
+		GitSyncService,
 		OauthService,
 		ResourceService,
 		WorkspaceService,
@@ -75,6 +76,7 @@
 	}: Props = $props()
 
 	let effectiveWorkspace = $derived(workspace ?? $workspaceStore!)
+	let pendingGitCredential: { token: string; repoUrl: string } | undefined = $state(undefined)
 
 	let isValid = $state(true)
 
@@ -952,6 +954,19 @@
 					}
 				})
 			}
+			// After the resource exists, so the path the credential is filed under is
+			// the one that was actually saved and a cancelled form writes nothing.
+			if (pendingGitCredential) {
+				await GitSyncService.setGitCredential({
+					workspace: effectiveWorkspace,
+					requestBody: {
+						repo_path: path,
+						repo_url: pendingGitCredential.repoUrl,
+						token: pendingGitCredential.token
+					}
+				})
+				pendingGitCredential = undefined
+			}
 			dispatch('refresh', path)
 			dispatch('close')
 			sendUserToast(
@@ -1388,8 +1403,16 @@
 						{linkedSecretCandidates}
 						{resourceType}
 						{resourceTypeInfo}
-						resourcePath={path}
 						workspace={effectiveWorkspace}
+						onCredentialSelected={(c) => {
+							pendingGitCredential = c
+							// `forceSecretValue` files a git_repository's `url` in a secret
+							// variable, for the URLs that carry a token in them. The picker's
+							// does not — the token is stored separately — so that variable
+							// would hold nothing secret and add a second place to keep in
+							// step with the resource.
+							linkedSecrets = linkedSecrets.filter((f) => f !== 'url')
+						}}
 						bind:args
 						bind:isValid
 						onSynced={getResourceTypeInfo}

frontend/src/lib/components/GitLabIntegration.svelte

@@ -16,9 +16,9 @@
 		 * one being navigated: the credential has to land where the resource will
 		 * look for it. */
 		workspace?: string
-		/** Path the resource is being saved at. The stored credential is keyed by
-		 * it, so the picker cannot run before the resource has a path. */
-		resourcePath?: string
+		/** The picked project's token, handed over for the form to store once the
+		 * resource is saved and its path is final. */
+		onCredentialSelected?: (credential: { token: string; repoUrl: string }) => void
 		onArgsUpdate?: (args: Record<string, any>) => void
 	}
 
@@ -26,7 +26,7 @@
 		resourceType,
 		args = {},
 		workspace = undefined,
-		resourcePath = undefined,
+		onCredentialSelected,
 		onArgsUpdate
 	}: Props = $props()
 
@@ -38,7 +38,6 @@
 	let projects: GitlabProject[] = $state([])
 	let selectedProject: string | undefined = $state(undefined)
 	let loading = $state(false)
-	let applying = $state(false)
 	let listError: string | undefined = $state(undefined)
 
 	// Shown alongside the GitHub App button and on the same terms, so the two
@@ -55,7 +54,6 @@
 	let enabled = $derived(!!$enterpriseLicense)
 
 	let project = $derived(projects.find((p) => p.path_with_namespace === selectedProject))
-	let hasPath = $derived(!!resourcePath && resourcePath !== '')
 
 	async function listProjects() {
 		if (!ws) return
@@ -79,37 +77,26 @@
 		}
 	}
 
-	async function apply(close: (_: any) => void) {
-		if (!ws || !project || !token || !resourcePath) return
-		// Everything this writes is read once, here, before the first await. The
-		// selector stays live while the request is in flight, so re-reading it
-		// later could store one project's token against another's URL.
-		const workspace = ws
+	function apply(close: (_: any) => void) {
+		if (!project || !token) return
 		const chosen = project
-		const repoPath = resourcePath
 		const url = chosen.http_url_to_repo
-		applying = true
-		try {
-			await GitSyncService.setGitCredential({
-				workspace,
-				requestBody: { repo_path: repoPath, repo_url: url, token }
-			})
-			onArgsUpdate?.({
-				...args,
-				url,
-				is_github_app: false,
-				branch: args.branch || chosen.default_branch || undefined
-			})
-			token = ''
-			projects = []
-			selectedProject = undefined
-			sendUserToast(`Windmill stored the token for ${chosen.path_with_namespace}`)
-			close(null)
-		} catch (err) {
-			sendUserToast(`Could not store the token: ${err?.body ?? err?.message}`, true)
-		} finally {
-			applying = false
-		}
+		// Handed to the form instead of stored now. The credential is filed under
+		// the resource's path, which is not settled until the resource is saved,
+		// and writing here would outlive an edit the user then cancels: picking a
+		// different project and backing out would have replaced a working token.
+		onCredentialSelected?.({ token, repoUrl: url })
+		onArgsUpdate?.({
+			...args,
+			url,
+			is_github_app: false,
+			branch: args.branch || chosen.default_branch || undefined
+		})
+		token = ''
+		projects = []
+		selectedProject = undefined
+		sendUserToast(`${chosen.path_with_namespace} selected. Its token is stored when you save.`)
+		close(null)
 	}
 </script>
 
@@ -151,8 +138,8 @@
 						</div>
 						<TextInput bind:value={token} size="sm" inputProps={{ type: 'password' }} />
 						<div class="text-2xs font-normal text-hint">
-							Windmill keeps it for this repository and hands it only to this workspace's sync jobs.
-							Forks of this workspace use it without holding a copy.
+							Windmill keeps it for this repository and hands it to this workspace's sync jobs.
+							Forks read this one copy instead of storing their own, so renewal reaches them all.
 						</div>
 					</div>
 					<div class="flex flex-col gap-y-1">
@@ -186,29 +173,17 @@
 								}))}
 								bind:value={selectedProject}
 								clearable={false}
-								disabled={applying}
 							/>
 						</div>
-						{#if hasPath}
-							<div class="text-2xs font-normal text-hint">
-								Stored for the resource at {resourcePath}. Give the resource its final path before
-								applying, so the token stays with it.
-							</div>
-						{:else}
-							<Alert type="warning" title="The resource needs a path first" size="xs">
-								The token is kept against the resource's path. Name the resource, then pick the
-								project.
-							</Alert>
-						{/if}
+						<div class="text-2xs font-normal text-hint">
+							The token is stored when you save the resource, under whatever path you save it at.
+						</div>
 						<div class="flex justify-end">
 							<Button
 								variant="accent"
 								unifiedSize="sm"
-								disabled={!project || !token || !hasPath || applying}
-								startIcon={{
-									icon: applying ? Loader2 : GitBranch,
-									classes: applying ? 'animate-spin' : ''
-								}}
+								disabled={!project || !token}
+								startIcon={{ icon: GitBranch }}
 								onclick={() => apply(close)}
 							>
 								Use this project

frontend/src/lib/components/ResourceEditor.svelte

@@ -1,6 +1,12 @@
 <script lang="ts">
 	import type { Schema } from '$lib/common'
-	import { ResourceService, WorkspaceService, type Resource, type ResourceType } from '$lib/gen'
+	import {
+		GitSyncService,
+		ResourceService,
+		WorkspaceService,
+		type Resource,
+		type ResourceType
+	} from '$lib/gen'
 	import { canWrite } from '$lib/utils'
 	import { createEventDispatcher, untrack } from 'svelte'
 	import { userStore, workspaceStore } from '$lib/stores'
@@ -320,6 +326,8 @@
 		current.path = npath
 	}
 
+	let pendingGitCredential: { token: string; repoUrl: string } | undefined = $state(undefined)
+
 	export async function save(): Promise<void> {
 		const dirty = dirtyWorkspaces
 		try {
@@ -363,7 +371,20 @@
 				// Path now exists server-side — drop the autocomplete cache so
 				// it shows up immediately instead of after the 60s TTL.
 				invalidateWorkspacePaths(ws)
+				// Only now is the path the credential is filed under settled, so a
+				// cancelled edit leaves the repository's existing token alone.
+				if (pendingGitCredential) {
+					await GitSyncService.setGitCredential({
+						workspace: ws,
+						requestBody: {
+							repo_path: s.path,
+							repo_url: pendingGitCredential.repoUrl,
+							token: pendingGitCredential.token
+						}
+					})
+				}
 			}
+			pendingGitCredential = undefined
 			sendUserToast(
 				dirty.length > 1 ? `Saved resource in ${dirty.length} workspaces` : `Saved resource`
 			)
@@ -404,6 +425,7 @@
 					{resourceToEdit}
 					onLoadResourceType={() => resourceTypeResource.refetch()}
 					workspace={selected}
+					onCredentialSelected={(c) => (pendingGitCredential = c)}
 				/>
 			{/key}
 		{/if}

frontend/src/lib/components/ResourceForm.svelte

@@ -48,6 +48,9 @@
 		/** Workspace the path is validated against and the connection is tested in;
 		 * defaults to the nav workspace. */
 		workspace?: string | undefined
+		/** A git credential the picker chose, for the editor to store once it has
+		 * saved the resource and its path is final. */
+		onCredentialSelected?: (credential: { token: string; repoUrl: string }) => void
 	}
 
 	let {
@@ -69,7 +72,8 @@
 		loadingSchema,
 		resourceToEdit,
 		onLoadResourceType,
-		workspace = undefined
+		workspace = undefined,
+		onCredentialSelected
 	}: Props = $props()
 
 	let ws = $derived(workspace ?? $workspaceStore)
@@ -262,7 +266,7 @@
 				resourceType={resource_type}
 				{args}
 				workspace={ws}
-				resourcePath={path}
+				{onCredentialSelected}
 				onArgsUpdate={(newArgs) => {
 					args = newArgs
 					// The raw editor is also what a workspace missing the resource type