From 40c7d262e15666c1dba427f52fda26db6d586470 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Mon, 7 Sep 2026 18:43:35 +0200 Subject: [PATCH] fix: the card reads the credential origin for managed controls and honours the licence for a borrowed token Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01C1xHmkxuxYb1GYvth1BS75 --- backend/ee-repo-ref.txt | 2 +- backend/tests/git_sync_fork_credential.rs | 22 +++++++-- .../git_sync/GitSyncRepositoryCard.svelte | 47 ++++++++----------- 3 files changed, 40 insertions(+), 31 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 196de6f879..29cdf1f052 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -422a7406f76e57bdb3e89a08971242e4a8a465bb +4802f7f1052bcca74d3daeb3330d97786f8a4b54 diff --git a/backend/tests/git_sync_fork_credential.rs b/backend/tests/git_sync_fork_credential.rs index c87de259f5..7e31136727 100644 --- a/backend/tests/git_sync_fork_credential.rs +++ b/backend/tests/git_sync_fork_credential.rs @@ -20,16 +20,32 @@ use windmill_common::workspaces::GitCredentialProvider; const REPO: &str = "$res:u/admin/repo"; const URL: &str = "https://gitlab.com/grp/proj.git"; +/// A repository is managed when a credential is held for the repository its +/// URL names now and the last check found it healthy. The recorded status is +/// keyed by resource path, so alone it would outlive a repoint; the held +/// credential alone says nothing about whether the host still accepts it. #[sqlx::test(fixtures("git_sync_fork_credential"))] async fn credential_status_is_a_workspaces_own(db: Pool) -> anyhow::Result<()> { + assert!( + !repo_supports_managed_git_features(&db, "parent-ws", REPO).await, + "a healthy status with nothing held behind it does not qualify" + ); + set_git_credential( + &db, + "parent-ws", + URL, + "glpat-secret", + GitCredentialProvider::Gitlab, + ) + .await?; assert!( repo_supports_managed_git_features(&db, "parent-ws", REPO).await, - "the workspace holding the recorded status qualifies" + "the workspace holding both the credential and the recorded status qualifies" ); assert!( !repo_supports_managed_git_features(&db, "fork-ws", REPO).await, - "a fork with no record of its own does not borrow the parent's: the status \ - describes one repository, and this fork's resource could name another" + "a fork borrowing the credential with no record of its own does not: the \ + status describes one repository, and this fork's resource could name another" ); assert!( !repo_supports_managed_git_features(&db, "errored-fork-ws", REPO).await, diff --git a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte index 5428776220..056617e7ea 100644 --- a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte +++ b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte @@ -151,24 +151,19 @@ let loadingResourceInfo = $state(false) // Only GitHub App-backed repos can register webhooks; PAT repos poll only. let isGithubApp = $state(false) - /** Whether Windmill holds this repository's credential, answered by the server - * rather than inferred from the resource. `undefined` until the lookup lands. */ - let managedCredential = $state(undefined) - /** `held` when this workspace stores the credential, `borrowed` when an - * ancestor does. A borrowed one is not this workspace's to renew or replace, - * which is what keeps a fork from warning about a token it must not touch. */ + /** Where the credential Windmill uses for this repository lives, answered by + * the server rather than inferred from the resource: `held` when this + * workspace stores it, `borrowed` when an ancestor does. A borrowed one is not + * this workspace's to renew or replace, which is what keeps a fork from + * warning about a token it must not touch. `undefined` until the lookup + * lands, and when nothing in the chain holds one. */ let credentialOrigin = $state<'held' | 'borrowed' | undefined>(undefined) // Whether Windmill itself holds a credential for the repository, which is // what the managed features (webhooks, pull requests, commit checks) need. // A GitHub App installation qualifies, and so does a token the server keeps. - // - // The resource has to answer this, not just the recorded credential status: - // that status is written when the repository is saved, and the defaults below - // only apply to a connection that has not been saved yet, so relying on it - // alone left every managed control hidden while a repository was being set up. - let hasManagedCredential = $derived( - isGithubApp || managedCredential != null || (repo?.credential != null && !repo.credential.error) - ) + // Not the recorded status: that is keyed by resource path and outlives a + // repoint, while the origin follows the repository the URL names now. + let hasManagedCredential = $derived(isGithubApp || credentialOrigin !== undefined) const MS_PER_DAY = 86_400_000 @@ -210,8 +205,9 @@ days <= 0 ? 'has expired' : days === 1 ? 'expires tomorrow' : `expires in ${days} days` // Renewed here, or by the workspace above that holds it. Either way this // workspace has nothing to do, and telling a fork to replace a borrowed - // token would split the credential in two. - if ((credential.rotatable && $enterpriseLicense) || credentialOrigin === 'borrowed') { + // token would split the credential in two. Renewal is licensed per + // instance, so a fork knows as well as its parent when nothing renews. + if ((credential.rotatable || credentialOrigin === 'borrowed') && $enterpriseLicense) { // A token Windmill renews needs no countdown: a renewal that fails records // an error, which is handled above. Reaching the expiry date anyway is the // one state that proves renewal never happened, and it is the only one @@ -228,14 +224,14 @@ } } if (days > 30) return undefined - // Why nothing renews it is the server's answer to give, not this card's to - // infer: it alone can tell a token the operator owns from one an ancestor - // holds and renews. The card asks only whether anything renews it, and says - // where to act when nothing does. + const where = + credentialOrigin === 'borrowed' + ? 'in the workspace that holds it' + : `on the ${repo?.git_repo_resource_path?.replace(/^\$res:/, '') ?? 'repository'} resource` return { type: days <= 7 ? ('error' as const) : days <= 14 ? ('warning' as const) : ('info' as const), title: `Repository token ${when}`, - body: `Windmill does not renew this token. Replace it on the ${repo?.git_repo_resource_path?.replace(/^\$res:/, '') ?? 'repository'} resource${days <= 0 ? ' to restore sync.' : ' before it expires.'}` + body: `Windmill does not renew this token. Replace it ${where}${days <= 0 ? ' to restore sync.' : ' before it expires.'}` } }) @@ -274,7 +270,6 @@ // Clear stale app state up front so a resource change or a failed // fetch can't leave webhook/fork controls showing for the wrong repo. isGithubApp = false - managedCredential = undefined credentialOrigin = undefined try { // The server answers whether it holds this repository's credential; @@ -295,7 +290,6 @@ 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 @@ -396,7 +390,6 @@ } else { resourceInfo = null isGithubApp = false - managedCredential = undefined credentialOrigin = undefined } } @@ -651,13 +644,13 @@
{#if credentialDaysLeft === undefined} Repository token does not expire. - {:else if credentialOrigin === 'borrowed'} + {:else if credentialOrigin === 'borrowed' && $enterpriseLicense} Repository token expires on {repo.credential.expires_at}, and the workspace that holds it - manages renewal. + renews it. {:else if repo.credential.rotatable && $enterpriseLicense} Repository token expires on {repo.credential.expires_at}, and Windmill renews it automatically. - {:else if repo.credential.rotatable} + {:else if repo.credential.rotatable || credentialOrigin === 'borrowed'} Repository token expires on {repo.credential.expires_at}. Renewing it automatically requires an enterprise license. {:else}