[ee] fix: drop a removed repo's credential and honor the workspace override

This commit is contained in:
hugocasa
2026-09-04 16:06:01 +02:00
parent 37375f5110
commit 7377923cd5
5 changed files with 79 additions and 19 deletions
+1 -1
View File
@@ -1 +1 @@
66e4efa6c1b25b50071712a19d81b269b2b15c05
c8deb48ca538225feca9853fce2f1afd0499c48f
@@ -3923,10 +3923,10 @@ async fn edit_git_sync_config(
// The whole-config save only writes the DB below; the managed GitHub webhooks
// are reconciled after the commit is durable (like the per-repository endpoint):
// `post_commit` carries the saved repos to reconcile + the hooks of repos this
// save removed, to delete.
// `post_commit` carries the saved repos to reconcile, the hooks of repos this
// save removed, and the paths of those repos, whose credentials go with them.
#[cfg(all(feature = "enterprise", feature = "private"))]
let post_commit: Option<(WorkspaceGitSyncSettings, Vec<(String, i64)>)>;
let post_commit: Option<(WorkspaceGitSyncSettings, Vec<(String, i64)>, Vec<String>)>;
if let Some(mut git_sync_settings) = new_config.git_sync_settings {
// Client-supplied server-owned auto-pull state is never trusted: strip it up
@@ -4028,6 +4028,27 @@ async fn edit_git_sync_config(
.collect()
})
.unwrap_or_default();
// Repos this save drops entirely: the workspace no longer syncs them, so the
// credential it holds for each goes with them. A separate list from the
// webhooks above, which only covers repos that had one — otherwise a dropped
// repo that never had a webhook would keep its token, and re-adding the same
// path and URL later would silently authenticate with it.
#[cfg(all(feature = "enterprise", feature = "private"))]
let removed_repos: Vec<String> = existing
.as_ref()
.map(|e| {
e.repositories
.iter()
.filter(|old| {
!git_sync_settings
.repositories
.iter()
.any(|n| n.git_repo_resource_path == old.git_repo_resource_path)
})
.map(|old| old.git_repo_resource_path.clone())
.collect()
})
.unwrap_or_default();
if let Some(existing) = &existing {
for repo in git_sync_settings.repositories.iter_mut() {
let Some(old) = existing
@@ -4066,7 +4087,7 @@ async fn edit_git_sync_config(
.await?;
#[cfg(all(feature = "enterprise", feature = "private"))]
{
post_commit = Some((git_sync_settings, removed_webhooks));
post_commit = Some((git_sync_settings, removed_webhooks, removed_repos));
}
} else {
// Clearing the whole config removes every repo — delete all their webhooks.
@@ -4084,6 +4105,7 @@ async fn edit_git_sync_config(
.flatten()
.and_then(|v| serde_json::from_value(v).ok());
let removed_webhooks: Vec<(String, i64)> = existing
.as_ref()
.map(|e| {
e.repositories
.iter()
@@ -4096,7 +4118,19 @@ async fn edit_git_sync_config(
.collect()
})
.unwrap_or_default();
post_commit = Some((WorkspaceGitSyncSettings::default(), removed_webhooks));
let removed_repos: Vec<String> = existing
.map(|e| {
e.repositories
.iter()
.map(|r| r.git_repo_resource_path.clone())
.collect()
})
.unwrap_or_default();
post_commit = Some((
WorkspaceGitSyncSettings::default(),
removed_webhooks,
removed_repos,
));
}
sqlx::query!(
"UPDATE workspace_settings SET git_sync = NULL WHERE workspace_id = $1",
@@ -4112,7 +4146,7 @@ async fn edit_git_sync_config(
// and delete the webhooks of repos this save removed. Best-effort — a failure
// leaves polling on.
#[cfg(all(feature = "enterprise", feature = "private"))]
if let Some((mut settings, removed_webhooks)) = post_commit {
if let Some((mut settings, removed_webhooks, removed_repos)) = post_commit {
for repo in settings.repositories.iter_mut() {
// `sync_repo_webhook` writes back the webhook fields it changes itself:
// the remote hook and the record of it have to move together, so
@@ -4144,6 +4178,10 @@ async fn edit_git_sync_config(
.await;
}
}
// After the webhook deletions above, which authenticate with these credentials.
for path in removed_repos {
let _ = windmill_common::git_sync_ee::delete_git_credential(&db, &w_id, &path).await;
}
}
// Trigger git sync for git sync settings changes
+26 -11
View File
@@ -54,15 +54,26 @@ group.
In the resource form for a `git_repository` resource, use the **GitLab** button:
paste the instance URL and the token, pick a project from the list, and Windmill
stores the whole remote URL, credential included, in a **secret variable** and
points the resource at it (`"url": "$var:u/you/gitlab_host_group_project_url"`).
It refuses to write over a variable already holding a different repository, so a
path collision cannot silently repoint an existing resource.
keeps the token for you. The resource itself gets the plain remote URL
(`"url": "https://gitlab.com/group/project.git"`), with no credential in it.
Renewal rewrites whichever of the two holds the URL, so a URL pasted straight
into the resource is renewed as well. The variable is still the better place for
it: the credential stays out of the resource, and everything else that references
the variable keeps working when the token changes. What cannot be renewed is a
The token is stored encrypted on the workspace, keyed by the resource's path, and
recorded against the repository it was issued for. Nothing reads it back out over
the API: the server attaches it when it talks to GitLab, and a sync job receives
it only against its own job token. Because it is bound to one repository,
repointing the resource's `url` at somewhere else does not carry the token along;
a repository that genuinely moved needs its token entered again.
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.
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:..."`),
and renewal rewrites whichever of the two holds it. What cannot be renewed is a
variable held in an external secret backend, which Windmill can read but does not
own the write to; that is reported on the repository.
@@ -71,9 +82,13 @@ own the write to; that is reported on the repository.
Windmill reads `expires_at` from the token itself and shows it on the repository
in the workspace's git sync settings. Within three weeks of expiry it rotates the
token through GitLab's own `POST /personal_access_tokens/self/rotate`, writes the
replacement back to the variable, and verifies it. Only the token can rotate
itself, so a token without `api` (or `self_rotate`) is a permanent warning rather
than something Windmill can fix.
replacement back where the credential is stored, and verifies it. Only the token
can rotate itself, so a token without `api` (or `self_rotate`) is a permanent
warning rather than something Windmill can fix.
Only the workspace that stores a credential rotates it. A fork reading its
parent's shows the same expiry but is not itself rotatable, so one rotation
serves the whole family instead of each fork racing to renew its own copy.
Rotation is deliberately never retried. GitLab revokes the old token the instant
it issues the replacement, and presenting an already-rotated token to `/rotate`
@@ -30,8 +30,12 @@
linkedSecretCandidates?: string[] | undefined
description?: string | undefined
/** Path the resource is being saved at, passed through to the GitLab picker
* so the credential variable it creates takes the resource's own path. */
* 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
onSynced?: () => void
}
@@ -44,6 +48,7 @@
linkedSecretCandidates = undefined,
description = $bindable(undefined),
resourcePath = undefined,
workspace = undefined,
onSynced = undefined
}: Props = $props()
@@ -260,6 +265,7 @@
{resourceType}
{args}
{resourcePath}
{workspace}
onArgsUpdate={(newArgs) => {
args = newArgs
rawCode = JSON.stringify(args, null, 2)
@@ -1389,6 +1389,7 @@
{resourceType}
{resourceTypeInfo}
resourcePath={path}
workspace={effectiveWorkspace}
bind:args
bind:isValid
onSynced={getResourceTypeInfo}