From e953af69351f09ee5f064ef677ad4174af00467e Mon Sep 17 00:00:00 2001 From: hugocasa Date: Mon, 7 Sep 2026 16:18:55 +0200 Subject: [PATCH] fix: pin the token the replace flow checked, and derive the scope test once Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01C1xHmkxuxYb1GYvth1BS75 --- .../components/git_sync/GitSyncRepositoryCard.svelte | 10 ++++++++-- .../components/git_sync/ReplaceGitCredential.svelte | 7 +++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte index 1f6161d116..bf5642d9c0 100644 --- a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte +++ b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte @@ -155,6 +155,13 @@ /** The host named by the resource's `managed_credential`, when Windmill holds * the repository's token rather than it being written into the URL. */ let managedCredential = $state(undefined) + /** Whether the token may rotate itself. Renewal also needs Windmill to hold it, + * which `managedCredential` stands in for; the card states both, and the alert + * and the quiet status line below must not disagree about either, so each is + * derived once here rather than recomputed per branch. */ + let canSelfRotate = $derived( + (repo?.credential?.scopes ?? []).some((s) => s === 'api' || s === 'self_rotate') + ) // 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. @@ -224,7 +231,6 @@ // Scopes answer the first; `managedCredential` is the client's signal for the // second, and naming the URL as the cause would be wrong for a fork, which // borrows an ancestor's token and holds none of its own. - const canSelfRotate = (credential.scopes ?? []).some((s) => s === 'api' || s === 'self_rotate') const remedy = managedCredential ? // The remedy lives with the credential, which the resource owns; saying // where stops the warning being a dead end. @@ -642,7 +648,7 @@ {:else if repo.credential.rotatable} Repository token expires on {repo.credential.expires_at}. Renewing it automatically requires an enterprise license. - {:else if (repo.credential.scopes ?? []).some((s) => s === 'api' || s === 'self_rotate')} + {:else if canSelfRotate} Repository token expires on {repo.credential.expires_at}. Windmill renews only a token it holds, and it does not hold this one. {:else if managedCredential} diff --git a/frontend/src/lib/components/git_sync/ReplaceGitCredential.svelte b/frontend/src/lib/components/git_sync/ReplaceGitCredential.svelte index 9f5324f4fb..878214e0de 100644 --- a/frontend/src/lib/components/git_sync/ReplaceGitCredential.svelte +++ b/frontend/src/lib/components/git_sync/ReplaceGitCredential.svelte @@ -40,6 +40,9 @@ if (!token || saving) return saving = true error = undefined + // Pinned before the first await: the field stays editable while the check + // runs, so re-reading it afterwards would store a token the check never saw. + const candidate = token try { // Check the token before storing it. The server binds a credential to its // repository but only refuses it when something tries to use it, so a @@ -54,7 +57,7 @@ // would not show this one and a working token would be refused. requestBody: { base_url: parts.base, - token, + token: candidate, search: parts.project.split('/').pop() } }) @@ -65,7 +68,7 @@ } await GitSyncService.setGitCredential({ workspace, - requestBody: { repo_url: repoUrl, token } + requestBody: { repo_url: repoUrl, token: candidate } }) token = '' sendUserToast('Token replaced')