fix: read the provider for url-token repos, await the origin before defaulting, and visit unchecked repos last

The maintenance pass sorted repositories with no recorded check first on
the premise that they cost nothing, but a token-in-URL remote on a host
that is not GitLab is probed every pass and never records a check, so it
held the head of the list ahead of the tokens that expire. Such
repositories now sort last.

The card decided its delivery defaults before the origin lookup landed,
so a freshly picked GitLab repository never got webhook delivery; the two
lookups are awaited together. The resource editor offers to replace a
token only where it is held, not in a fork that borrows it, and the
replace flow refuses a URL it cannot parse instead of keying the token to
it. Attaching a stored credential to a commit-hash probe now requires
admin, matching the installation credential beside it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C1xHmkxuxYb1GYvth1BS75

backend/.sqlx/query-85730ae5368b0942c256909efab6d33a26bb2c4804c8045d09bed2de03996ecc.json

@@ -1,6 +1,6 @@
 {
   "db_name": "PostgreSQL",
-  "query": "SELECT ws.workspace_id, ws.git_sync\n           FROM workspace_settings ws\n           JOIN workspace w ON w.id = ws.workspace_id\n           WHERE NOT w.deleted\n             AND ws.git_sync IS NOT NULL\n             AND jsonb_typeof(ws.git_sync->'repositories') = 'array'\n           ORDER BY (\n             SELECT min((elem->'credential'->>'checked_at')::bigint)\n             FROM jsonb_array_elements(ws.git_sync->'repositories') AS elem\n           ) ASC NULLS FIRST",
+  "query": "SELECT ws.workspace_id, ws.git_sync\n           FROM workspace_settings ws\n           JOIN workspace w ON w.id = ws.workspace_id\n           WHERE NOT w.deleted\n             AND ws.git_sync IS NOT NULL\n             AND jsonb_typeof(ws.git_sync->'repositories') = 'array'\n           ORDER BY (\n             SELECT min((elem->'credential'->>'checked_at')::bigint)\n             FROM jsonb_array_elements(ws.git_sync->'repositories') AS elem\n           ) ASC NULLS LAST",
   "describe": {
     "columns": [
       {
@@ -22,5 +22,5 @@
       true
     ]
   },
-  "hash": "2184f787e6dffc76bfc28f2f2cefc244505996be71d9caec9c6d69e32a9c3d2c"
-}
+  "hash": "85730ae5368b0942c256909efab6d33a26bb2c4804c8045d09bed2de03996ecc"
+}

backend/ee-repo-ref.txt

@@ -1 +1 @@
-746033ec5f4d77343d39f47a839ddbd12c0f5f54
+333dadfd8bdab3102fd23df6c4b2fb31675c69cd

backend/src/monitor.rs

@@ -4774,11 +4774,10 @@ async fn maintain_git_credentials_inner(db: &Pool<Postgres>) -> error::Result<()
     // settings row survives, and rotating a token for one would be pure damage.
     // Least-recently-checked first, so a pass that runs out of budget resumes
     // where it stopped instead of re-checking the same head of the list forever.
-    // A repository with no recorded credential sorts first and stays there,
-    // which is deliberate: it has no token to introspect, so it costs a few
-    // database queries and nothing else. Tens of thousands of them would have to
-    // exist in one instance before they consumed the pass budget ahead of a
-    // repository that does have a token.
+    // A repository with no recorded credential sorts last: it has nothing to
+    // rotate, yet a remote whose URL carries a token on a host that is not
+    // GitLab still costs a probe every pass and never records a check, so put
+    // first it would hold the head of the list ahead of the tokens that expire.
     let rows = sqlx::query!(
         r#"SELECT ws.workspace_id, ws.git_sync
            FROM workspace_settings ws
@@ -4789,7 +4788,7 @@ async fn maintain_git_credentials_inner(db: &Pool<Postgres>) -> error::Result<()
            ORDER BY (
              SELECT min((elem->'credential'->>'checked_at')::bigint)
              FROM jsonb_array_elements(ws.git_sync->'repositories') AS elem
-           ) ASC NULLS FIRST"#
+           ) ASC NULLS LAST"#
     )
     .fetch_all(db)
     .await?;
@@ -4815,7 +4814,10 @@ async fn maintain_git_credentials_inner(db: &Pool<Postgres>) -> error::Result<()
         // within one, the repositories need the same least-recently-checked
         // order or the tail of a large workspace never gets its turn.
         let mut repositories: Vec<_> = settings.repositories.iter().collect();
-        repositories.sort_by_key(|r| r.credential.as_ref().map(|c| c.checked_at));
+        repositories.sort_by_key(|r| {
+            let checked_at = r.credential.as_ref().map(|c| c.checked_at);
+            (checked_at.is_none(), checked_at)
+        });
 
         for repo in repositories {
             if started.elapsed() >= GIT_CREDENTIAL_PASS_BUDGET {

backend/tests/fixtures/git_sync_fork_credential.sql

@@ -45,9 +45,12 @@ INSERT INTO workspace_settings (workspace_id, git_sync) VALUES
     ('orphan-ws', '{"repositories":[{"git_repo_resource_path":"$res:u/admin/repo"}]}');
 
 -- The resource each repository entry names, all pointing at the same repository.
+-- The errored fork carries its token in the URL, the way a repository configured
+-- by hand does: nothing stores a credential for it, so only its recorded check
+-- knows which host it talks to.
 INSERT INTO resource (workspace_id, path, value, resource_type) VALUES
     ('parent-ws',       'u/admin/repo', '{"url":"https://gitlab.com/grp/proj.git"}', 'git_repository'),
     ('fork-ws',         'u/admin/repo', '{"url":"https://gitlab.com/grp/proj.git"}', 'git_repository'),
     ('deep-fork-ws',    'u/admin/repo', '{"url":"https://gitlab.com/grp/proj.git"}', 'git_repository'),
-    ('errored-fork-ws', 'u/admin/repo', '{"url":"https://gitlab.com/grp/proj.git"}', 'git_repository'),
+    ('errored-fork-ws', 'u/admin/repo', '{"url":"https://oauth2:glpat-inline@gitlab.com/grp/proj.git"}', 'git_repository'),
     ('orphan-ws',       'u/admin/repo', '{"url":"https://gitlab.com/grp/proj.git"}', 'git_repository');

backend/tests/git_sync_fork_credential.rs

@@ -39,19 +39,28 @@ async fn credential_status_is_a_workspaces_own(db: Pool<Postgres>) -> anyhow::Re
 }
 
 /// The host a repository talks to is declared when its credential is stored, and
-/// travels with the credential down the fork chain.
+/// travels with the credential down the fork chain. A repository whose token
+/// rides in its URL has no stored credential, so its host is known only from the
+/// check that introspected the token.
 ///
-/// Read from the recorded status instead, a fork answered with the default
+/// Read from the recorded status alone, a fork answered with the default
 /// provider until its own check ran, which is long enough to register a webhook
-/// against the wrong receiver.
+/// against the wrong receiver. Read from the credential alone, a URL-token
+/// repository answered with the default forever.
 #[sqlx::test(fixtures("git_sync_fork_credential"))]
 async fn the_provider_comes_from_the_credential_and_reaches_forks(
     db: Pool<Postgres>,
 ) -> anyhow::Result<()> {
     assert_eq!(
         repo_provider(&db, "parent-ws", REPO).await,
+        GitProvider::GitLab,
+        "with nothing stored, the host the check recorded is the answer"
+    );
+    assert_eq!(
+        repo_provider(&db, "fork-ws", REPO).await,
         GitProvider::GitHub,
-        "with nothing stored there is no declaration to read, so the default stands"
+        "a fork with neither a credential to resolve nor a check of its own \
+         answers the default"
     );
 
     set_git_credential(
@@ -83,6 +92,12 @@ async fn the_provider_comes_from_the_credential_and_reaches_forks(
         GitProvider::GitHub,
         "a workspace outside the chain resolves no credential and no declaration"
     );
+    assert_eq!(
+        repo_provider(&db, "errored-fork-ws", REPO).await,
+        GitProvider::GitLab,
+        "a token carried in the URL is held by nobody, so the parent's credential \
+         is not consulted and the recorded check alone names the host"
+    );
     Ok(())
 }
 

backend/windmill-store/src/resources.rs

@@ -3268,11 +3268,16 @@ async fn get_git_commit_hash(
     })?;
     git_resource.url =
         resolve_azure_devops_url(&db_with_opt_authed, &w_id, &git_resource.url, false).await?;
-    // Safe to attach for a caller-named resource because a credential is stored
-    // under the repository it was issued for: a resource repointed at another
-    // host asks for that repository's credential and finds none.
+    // A credential is stored under the repository it was issued for, so a
+    // resource repointed elsewhere finds none. Which credential can be attached
+    // is bounded by that; who may use it is bounded here, on the same terms as
+    // the installation credential above.
+    let plain_url = git_resource.url.clone();
     git_resource.url =
         windmill_common::git_sync_oss::with_stored_credential(&db, &w_id, git_resource.url).await?;
+    if git_resource.url != plain_url {
+        require_admin(authed.is_admin, &authed.username)?;
+    }
 
     let identities: Vec<String> = query
         .git_ssh_identity

frontend/src/lib/components/ResourceEditor.svelte

@@ -159,20 +159,20 @@
 	// The deployed path, for the same reason as the deployed URL: the server
 	// answers about what is stored, and an unsaved rename names nothing yet.
 	let deployedPath = $derived(selected ? (initialStates[selected]?.path ?? initialPath) : undefined)
-	// Asked of the server rather than read off the resource: the marker this
-	// replaced was a copy of a server fact kept in a client-editable, exported
-	// object, so it went stale on a URL edit, a workspace import, and in a fork.
-	// Re-asked when the saved URL moves, since that is a different repository.
+	// Asked of the server rather than read off the resource: the resource is
+	// client-editable, exported and copied into forks, so nothing written on it
+	// stays true. Re-asked when the saved URL moves, since that is a different
+	// repository.
 	const credentialOrigin = resource(
-		[() => selected, () => deployedPath, () => deployedUrl],
-		async ([ws, path]) =>
-			ws && path
+		[() => selected, () => deployedPath, () => deployedUrl, () => resource_type],
+		async ([ws, path, _url, type]) =>
+			ws && path && type === 'git_repository'
 				? await GitSyncService.getCredentialOrigin({ workspace: ws, path }).catch(() => undefined)
 				: undefined
 	)
-	let managedHost = $derived(
-		credentialOrigin.current?.origin ? credentialOrigin.current.provider : undefined
-	)
+	// Only a credential this workspace holds is its to replace: a fork borrows
+	// its ancestor's, and storing a replacement here would split it in two.
+	let holdsCredential = $derived(credentialOrigin.current?.origin === 'held')
 	// Only an unsaved *URL* blocks replacing the token, not any unsaved change:
 	// opening the drawer materialises schema defaults (`folder: ""`), so a whole-
 	// resource dirty check would disable it the moment the drawer opens.
@@ -423,7 +423,7 @@
 			</Alert>
 		{/if}
 
-		{#if managedHost && selected}
+		{#if holdsCredential && selected}
 			<Alert type="info" title="Windmill holds this repository's access token">
 				<div class="flex flex-col items-start gap-2">
 					<div>

frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte

@@ -278,25 +278,25 @@
 				credentialOrigin = undefined
 				try {
 					// The server answers whether it holds this repository's credential;
-					// the resource cannot, because the marker that used to claim it was
-					// a copy that went stale on a URL edit, an import, and in a fork.
-					// Best-effort: a failure here must not hide the URL below.
-					GitSyncService.getCredentialOrigin({
-						workspace: $workspaceStore,
-						path: repo.git_repo_resource_path
-					})
-						.then((r) => {
-							if (!abortController.signal.aborted) {
-								credentialOrigin = r?.origin
-								managedCredential = r?.origin ? (r.provider ?? 'gitlab') : undefined
-							}
+					// the resource cannot, being client-editable, exported and copied
+					// into forks. Best-effort: a failure here must not hide the URL
+					// below. Awaited alongside the resource because the defaults below
+					// read the answer.
+					const [origin, resource] = await Promise.all([
+						GitSyncService.getCredentialOrigin({
+							workspace: $workspaceStore,
+							path: repo.git_repo_resource_path
+						}).catch(() => undefined),
+						ResourceService.getResource({
+							workspace: $workspaceStore,
+							path: repo.git_repo_resource_path
 						})
-						.catch(() => {})
-					const resource = await ResourceService.getResource({
-						workspace: $workspaceStore,
-						path: repo.git_repo_resource_path
-					})
+					])
 
+					if (!abortController.signal.aborted) {
+						credentialOrigin = origin?.origin
+						managedCredential = origin?.origin ? (origin.provider ?? 'gitlab') : undefined
+					}
 					if (!abortController.signal.aborted && resource?.value) {
 						// Extract git URL from resource value
 						const value = resource.value as Record<string, any>
@@ -651,15 +651,15 @@
 			<div class="text-xs text-secondary">
 				{#if credentialDaysLeft === undefined}
 					Repository token does not expire.
+				{:else if credentialOrigin === 'borrowed'}
+					Repository token expires on {repo.credential.expires_at}, and the workspace that holds it
+					manages renewal.
 				{:else if repo.credential.rotatable && $enterpriseLicense}
 					Repository token expires on {repo.credential.expires_at}, and Windmill renews it
 					automatically.
 				{:else if repo.credential.rotatable}
 					Repository token expires on {repo.credential.expires_at}. Renewing it automatically
 					requires an enterprise license.
-				{:else if credentialOrigin === 'borrowed'}
-					Repository token expires on {repo.credential.expires_at}, and the workspace that holds it
-					manages renewal.
 				{:else}
 					Repository token expires on {repo.credential.expires_at}, and Windmill does not renew it.
 				{/if}

frontend/src/lib/components/git_sync/ReplaceGitCredential.svelte

@@ -52,23 +52,28 @@
 			// repository but only refuses it when something tries to use it, so a
 			// wrong token would otherwise be accepted here and surface as a failed
 			// sync later.
+			// A URL that does not parse names no project to check, and would be
+			// stored as the credential's key verbatim: a `$var:` reference here
+			// keys the token to a repository that does not exist.
 			const parts = repoParts(forRepo)
-			if (parts) {
-				const projects = await GitSyncService.listGitlabProjects({
-					workspace: inWorkspace,
-					// Searched by name rather than listed whole: the listing is one
-					// capped page, so a token that reaches more projects than fit
-					// would not show this one and a working token would be refused.
-					requestBody: {
-						base_url: parts.base,
-						token: candidate,
-						search: parts.project.split('/').pop()
-					}
-				})
-				if (!projects.some((p) => p.path_with_namespace === parts.project)) {
-					error = `That token cannot push to ${parts.project}. Check its role and that it belongs to this project.`
-					return
+			if (!parts) {
+				error = 'The resource URL must name the repository directly to replace its token here.'
+				return
+			}
+			const projects = await GitSyncService.listGitlabProjects({
+				workspace: inWorkspace,
+				// Searched by name rather than listed whole: the listing is one
+				// capped page, so a token that reaches more projects than fit
+				// would not show this one and a working token would be refused.
+				requestBody: {
+					base_url: parts.base,
+					token: candidate,
+					search: parts.project.split('/').pop()
 				}
+			})
+			if (!projects.some((p) => p.path_with_namespace === parts.project)) {
+				error = `That token cannot push to ${parts.project}. Check its role and that it belongs to this project.`
+				return
 			}
 			await GitSyncService.setGitCredential({
 				workspace: inWorkspace,