starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-09-08 08:04:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: classify the renewal state once so the card cannot contradict itself

Browse Source 
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C1xHmkxuxYb1GYvth1BS75
This commit is contained in:
hugocasa
2026-09-07 16:52:00 +02:00
co-authored by Claude Opus 5
parent 5c507c6943
commit 4e4c5c2c8f
3 changed files with 66 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/windmill-common/src/git_sync_oss.rs
+5 -3
View File
@@ -5,9 +5,11 @@ pub use crate::git_sync_ee::*;
use sqlx::{Pool, Postgres};
use url::Url;
/// Gated on the pair for the same reason as [`with_stored_credential`] below:
/// `private` alone would leave this undefined, which only stays harmless while
/// every caller is itself `enterprise`-gated.
/// Gated on the pair to match [`with_stored_credential`] below, whose callers
/// reach it through this facade un-gated and so depend on it. Nothing routes
/// here today (the one caller imports the enterprise item directly), so this is
/// for uniformity: the next plain caller would otherwise find no definition
/// under `private` without `enterprise`.
#[cfg(not(all(feature = "private", feature = "enterprise")))]
pub async fn get_github_app_token_internal(
_db: &Pool<Postgres>,
frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte
+51 -23
View File
@@ -155,13 +155,34 @@
/** 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<string | undefined>(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')
)
/**
* Why this repository's token is or is not being renewed.
*
* Renewal needs a scope that permits self-rotation and a credential the
* workspace holds, and the alert and the quiet status line below both describe
* that state at different day counts. Reading it independently in each has
* twice produced contradictory advice, so it is classified once here and both
* render the answer.
*
* `inherited` is reachable only on a fork: forking copies the resource's
* marker but not the credential, so the fork borrows its ancestor's and reports
* `rotatable: false` while the ancestor renews it on schedule. Any other
* workspace whose stored credential stopped matching its URL loses its recorded
* status entirely and never reaches here.
*/
let renewal = $derived.by(() => {
const credential = repo?.credential
if (!credential) return undefined
if (credential.rotatable) return $enterpriseLicense ? 'renewed' : 'needs-license'
// The marker comes from the resource, so until that load lands there is no
// answer yet and guessing one would state the opposite of the truth.
if (loadingResourceInfo) return 'unknown'
if (canSelfRotate) return managedCredential ? 'inherited' : 'not-held'
return managedCredential ? 'no-scope' : 'no-scope-and-not-held'
})
// 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.
@@ -226,23 +247,25 @@
}
}
if (days > 30) return undefined
// Renewal needs two things, and the advice differs by which is missing: a
// scope that permits self-rotation, and a credential this workspace holds.
// 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 remedy = managedCredential
? // The remedy lives with the credential, which the resource owns; saying
// where stops the warning being a dead end.
` Replace it on the ${repo?.git_repo_resource_path?.replace(/^\$res:/, '') ?? 'repository'} resource.`
: ' Connect the repository with the GitLab button to hand the token to Windmill, or replace it before it expires.'
// Nothing for this workspace to act on: the holder renews it, and telling a
// fork admin to replace it would split the credential in two.
if (renewal === 'inherited' || renewal === 'unknown') return undefined
const remedy =
renewal === 'no-scope'
? // The remedy lives with the credential, which the resource owns; saying
// where stops the warning being a dead end.
` Replace it on the ${repo?.git_repo_resource_path?.replace(/^\$res:/, '') ?? 'repository'} resource.`
: ' Connect the repository with the GitLab button to hand the token to Windmill, or replace it before it expires.'
const cause =
renewal === 'not-held'
? 'Windmill is not the holder of this token, so it does not renew it.'
: renewal === 'no-scope'
? 'Give the token the api or self_rotate scope so Windmill can renew it.'
: 'Windmill is not the holder of this token, and could not renew it anyway without the api or self_rotate scope.'
return {
type: days <= 7 ? ('error' as const) : days <= 14 ? ('warning' as const) : ('info' as const),
title: `Repository token ${when}`,
body:
(canSelfRotate
? 'Windmill is not the holder of this token, so it does not renew it.'
: 'Give the token the api or self_rotate scope so Windmill can renew it.') + remedy
body: cause + remedy
}
})
@@ -642,21 +665,26 @@
<div class="text-xs text-secondary">
{#if credentialDaysLeft === undefined}
Repository token does not expire.
{:else if repo.credential.rotatable && $enterpriseLicense}
{:else if renewal === 'renewed'}
Repository token expires on {repo.credential.expires_at}, and Windmill renews it
automatically.
{:else if repo.credential.rotatable}
{:else if renewal === 'needs-license'}
Repository token expires on {repo.credential.expires_at}. Renewing it automatically
requires an enterprise license.
{:else if canSelfRotate}
{:else if renewal === 'inherited'}
Repository token expires on {repo.credential.expires_at}, and the workspace that holds it
renews it.
{:else if renewal === 'not-held'}
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}
{:else if renewal === 'no-scope'}
Repository token expires on {repo.credential.expires_at}. Give it the api or self_rotate
scope so Windmill can renew it.
{:else}
{:else if renewal === 'no-scope-and-not-held'}
Repository token expires on {repo.credential.expires_at}. Windmill renews only a token it
holds, and this one would also need the api or self_rotate scope.
{:else}
Repository token expires on {repo.credential.expires_at}.
{/if}
</div>
{/if}
frontend/src/lib/components/git_sync/ReplaceGitCredential.svelte
+10 -6
View File
@@ -40,18 +40,22 @@
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.
// Pinned before the first await, all three: the field stays editable and the
// props follow the drawer's selected workspace and deployed URL, so
// re-reading any of them afterwards would store the token against something
// the check never validated.
const candidate = token
const forRepo = repoUrl
const inWorkspace = workspace
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
// wrong token would otherwise be accepted here and surface as a failed
// sync later.
const parts = repoParts(repoUrl)
const parts = repoParts(forRepo)
if (parts) {
const projects = await GitSyncService.listGitlabProjects({
workspace,
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.
@@ -67,8 +71,8 @@
}
}
await GitSyncService.setGitCredential({
workspace,
requestBody: { repo_url: repoUrl, token: candidate }
workspace: inWorkspace,
requestBody: { repo_url: forRepo, token: candidate }
})
token = ''
sendUserToast('Token replaced')