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

refactor: replace the managed-credential marker with a server answer

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 17:36:43 +02:00
co-authored by Claude Opus 5
parent 01cbbfec56
commit 4735901970
8 changed files with 172 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/ee-repo-ref.txt
+1 -1
View File
@@ -1 +1 @@
7b607d736cc23f0ff845599fd1ac9fad4c4fd307
746033ec5f4d77343d39f47a839ddbd12c0f5f54
backend/tests/git_sync_fork_credential.rs
+89 -15
View File
@@ -15,6 +15,7 @@ use windmill_common::git_sync_ee::{
git_credential_for_url, repo_provider, repo_supports_managed_git_features, set_git_credential,
GitProvider,
};
use windmill_common::workspaces::GitCredentialProvider;
const REPO: &str = "$res:u/admin/repo";
const URL: &str = "https://gitlab.com/grp/proj.git";
@@ -25,21 +26,11 @@ async fn credential_status_is_a_workspaces_own(db: Pool<Postgres>) -> anyhow::Re
repo_supports_managed_git_features(&db, "parent-ws", REPO).await,
"the workspace holding the recorded status qualifies"
);
assert_eq!(
repo_provider(&db, "parent-ws", REPO).await,
GitProvider::GitLab,
"and its provider comes from that record"
);
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"
);
assert_eq!(
repo_provider(&db, "fork-ws", REPO).await,
GitProvider::GitHub,
"so it answers with the default provider until its own check runs"
);
assert!(
!repo_supports_managed_git_features(&db, "errored-fork-ws", REPO).await,
"a workspace whose own credential failed stays disqualified"
@@ -47,6 +38,54 @@ async fn credential_status_is_a_workspaces_own(db: Pool<Postgres>) -> anyhow::Re
Ok(())
}
/// The host a repository talks to is declared when its credential is stored, and
/// travels with the credential down the fork chain.
///
/// Read from the recorded status instead, a fork answered with the default
/// provider until its own check ran, which is long enough to register a webhook
/// against the wrong receiver.
#[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::GitHub,
"with nothing stored there is no declaration to read, so the default stands"
);
set_git_credential(
&db,
"parent-ws",
URL,
"glpat-secret",
GitCredentialProvider::Gitlab,
)
.await?;
assert_eq!(
repo_provider(&db, "parent-ws", REPO).await,
GitProvider::GitLab,
"the workspace that stored it reads its own declaration"
);
assert_eq!(
repo_provider(&db, "fork-ws", REPO).await,
GitProvider::GitLab,
"and a fork resolving that credential reads it too, without a check of its own"
);
assert_eq!(
repo_provider(&db, "deep-fork-ws", REPO).await,
GitProvider::GitLab,
"two levels down as well"
);
assert_eq!(
repo_provider(&db, "orphan-ws", REPO).await,
GitProvider::GitHub,
"a workspace outside the chain resolves no credential and no declaration"
);
Ok(())
}
/// The stored credential is shared with forks and keyed by one repository.
///
/// Both properties are the point of keeping it in `workspace_settings` under the
@@ -57,7 +96,14 @@ async fn credential_status_is_a_workspaces_own(db: Pool<Postgres>) -> anyhow::Re
async fn a_fork_reads_an_ancestors_credential_for_the_bound_repository_only(
db: Pool<Postgres>,
) -> anyhow::Result<()> {
set_git_credential(&db, "parent-ws", URL, "glpat-secret").await?;
set_git_credential(
&db,
"parent-ws",
URL,
"glpat-secret",
GitCredentialProvider::Gitlab,
)
.await?;
assert_eq!(
git_credential_for_url(&db, "parent-ws", URL)
@@ -104,8 +150,22 @@ async fn a_fork_reads_an_ancestors_credential_for_the_bound_repository_only(
async fn each_repository_keeps_its_own_credential(db: Pool<Postgres>) -> anyhow::Result<()> {
const OTHER_URL: &str = "https://gitlab.com/grp/other.git";
set_git_credential(&db, "parent-ws", URL, "glpat-first").await?;
set_git_credential(&db, "parent-ws", OTHER_URL, "glpat-second").await?;
set_git_credential(
&db,
"parent-ws",
URL,
"glpat-first",
GitCredentialProvider::Gitlab,
)
.await?;
set_git_credential(
&db,
"parent-ws",
OTHER_URL,
"glpat-second",
GitCredentialProvider::Gitlab,
)
.await?;
assert_eq!(
git_credential_for_url(&db, "parent-ws", URL)
@@ -121,7 +181,14 @@ async fn each_repository_keeps_its_own_credential(db: Pool<Postgres>) -> anyhow:
Some("glpat-second")
);
set_git_credential(&db, "parent-ws", URL, "glpat-replacement").await?;
set_git_credential(
&db,
"parent-ws",
URL,
"glpat-replacement",
GitCredentialProvider::Gitlab,
)
.await?;
assert_eq!(
git_credential_for_url(&db, "parent-ws", URL)
.await?
@@ -147,7 +214,14 @@ async fn each_repository_keeps_its_own_credential(db: Pool<Postgres>) -> anyhow:
async fn a_credential_is_not_served_over_a_downgraded_transport(
db: Pool<Postgres>,
) -> anyhow::Result<()> {
set_git_credential(&db, "parent-ws", URL, "glpat-secret").await?;
set_git_credential(
&db,
"parent-ws",
URL,
"glpat-secret",
GitCredentialProvider::Gitlab,
)
.await?;
assert_eq!(
git_credential_for_url(&db, "parent-ws", "http://gitlab.com/grp/proj.git").await?,
None
backend/windmill-api/openapi.yaml
+41 -6
View File
@@ -2947,6 +2947,47 @@ paths:
items:
$ref: "#/components/schemas/GitlabProject"
/w/{workspace}/git_sync/credential/origin:
get:
tags:
- Git Sync
summary: Where a repository's credential comes from
description: >-
Whether Windmill holds this repository's access token, and which host it
talks to. `held` means this workspace stores it, `borrowed` means an
ancestor does and it is not this workspace's to replace. Both absent
means the repository authenticates with whatever its URL carries.
Returns no secret. Requires workspace admin.
operationId: getCredentialOrigin
parameters:
- $ref: "#/components/parameters/WorkspaceId"
- name: path
in: query
required: true
description: >-
Path of the git repository resource, with or without the `$res:`
prefix. A path rather than a URL, because a resource URL may carry a
token and a URL in a query string lands in logs.
schema:
type: string
responses:
"200":
description: where the credential comes from
content:
application/json:
schema:
type: object
properties:
origin:
type: string
enum:
- held
- borrowed
provider:
type: string
enum:
- gitlab
/w/{workspace}/git_sync/credential:
post:
tags:
@@ -34354,12 +34395,6 @@ components:
rotatable:
type: boolean
description: whether this workspace renews the credential itself
renewed:
type: boolean
description: >-
whether the credential is renewed at all, by whichever workspace
holds it; true for a fork reading an ancestor's, which nothing local
renews but nothing needs to
checked_at:
type: integer
format: int64
backend/windmill-common/src/workspaces.rs
-10
View File
@@ -446,16 +446,6 @@ pub struct GitCredentialStatus {
/// a token carried in the repository URL is the operator's to manage, and one
/// resolved from an ancestor is the ancestor's, so neither is renewed here.
pub rotatable: bool,
/// Whether the credential gets renewed at all, by whichever workspace holds
/// it. Equals `rotatable` for the holder and is also true for a fork that
/// borrows an ancestor's, which nothing local renews but nothing needs to.
///
/// The UI shows expiry warnings on exactly `!renewed`, so this is what keeps
/// a fork from nagging about a token its ancestor renews on schedule. Kept
/// server-side because only the server can tell a borrowed credential from an
/// unheld one, which the resource alone cannot express.
#[serde(default)]
pub renewed: bool,
/// Unix timestamp (seconds) of the last check.
pub checked_at: i64,
/// Why the last check or rotation failed, cleared by the next success.
frontend/src/lib/components/GitLabIntegration.svelte
-5
View File
@@ -116,11 +116,6 @@
...args,
url,
is_github_app: false,
// The URL carries no credential, so without this the resource is
// indistinguishable from a public remote: it is what tells the rest of
// the UI the token is Windmill's to keep and renew, and which host it
// belongs to.
managed_credential: 'gitlab',
branch: args.branch || chosen.default_branch || undefined
})
token = ''
frontend/src/lib/components/ResourceEditor.svelte
+24 -3
View File
@@ -1,13 +1,18 @@
<script lang="ts">
import type { Schema } from '$lib/common'
import { ResourceService, WorkspaceService, type Resource, type ResourceType } from '$lib/gen'
import {
GitSyncService,
ResourceService,
WorkspaceService,
type Resource,
type ResourceType
} from '$lib/gen'
import { canWrite } from '$lib/utils'
import { createEventDispatcher, untrack } from 'svelte'
import { userStore, workspaceStore } from '$lib/stores'
import { sendUserToast } from '$lib/toast'
import { clearJsonSchemaResourceCache } from './schema/jsonSchemaResource.svelte'
import ResourceForm from './ResourceForm.svelte'
import { managedCredentialHost } from './git_sync/managedCredential'
import ReplaceGitCredential from './git_sync/ReplaceGitCredential.svelte'
import { invalidateWorkspacePaths } from './PathNameAutocomplete.svelte'
import Alert from './common/alert/Alert.svelte'
@@ -145,13 +150,29 @@
let loadingSchema = $derived(resourceTypeResource.loading)
let current = $derived(selected ? states[selected]?.draft : undefined)
let managedHost = $derived(managedCredentialHost(current?.args))
// The saved URL, not the draft's: a credential is bound to the repository it
// is issued for, so binding one to an edit that has not landed yet would tie
// it to something the resource does not point at.
let deployedUrl = $derived(
selected ? ((fetchedResources[selected]?.value as any)?.url as string | undefined) : undefined
)
// 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.
const credentialOrigin = resource(
[() => selected, () => deployedPath, () => deployedUrl],
async ([ws, path]) =>
ws && path
? await GitSyncService.getCredentialOrigin({ workspace: ws, path }).catch(() => undefined)
: undefined
)
let managedHost = $derived(
credentialOrigin.current?.origin ? credentialOrigin.current.provider : undefined
)
// 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.
frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte
+17 -5
View File
@@ -23,8 +23,7 @@
import GitSyncModeDisplay from './GitSyncModeDisplay.svelte'
import Toggle from '$lib/components/Toggle.svelte'
import EEOnly from '$lib/components/EEOnly.svelte'
import { ResourceService, VariableService } from '$lib/gen'
import { managedCredentialHost } from './managedCredential'
import { GitSyncService, ResourceService, VariableService } from '$lib/gen'
let {
idx = null,
@@ -152,8 +151,8 @@
let loadingResourceInfo = $state(false)
// Only GitHub App-backed repos can register webhooks; PAT repos poll only.
let isGithubApp = $state(false)
/** The host named by the resource's `managed_credential`, when Windmill holds
* the repository's token rather than it being written into the URL. */
/** 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<string | undefined>(undefined)
// Whether Windmill itself holds a credential for the repository, which is
// what the managed features (webhooks, pull requests, commit checks) need.
@@ -267,6 +266,20 @@
isGithubApp = false
managedCredential = 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) {
managedCredential = r?.origin ? (r.provider ?? 'gitlab') : undefined
}
})
.catch(() => {})
const resource = await ResourceService.getResource({
workspace: $workspaceStore,
path: repo.git_repo_resource_path
@@ -276,7 +289,6 @@
// Extract git URL from resource value
const value = resource.value as Record<string, any>
isGithubApp = value?.is_github_app === true
managedCredential = managedCredentialHost(value)
// A newly added sync connection defaults to pulling from Git only
// when the repository is app-backed (instant webhook delivery).
// Polling is opt-in for token repositories, and fork/dev workspaces
frontend/src/lib/components/git_sync/managedCredential.ts
-32
View File
@@ -1,32 +0,0 @@
/** A git repository resource value, as far as this predicate cares. */
type GitRepositoryValue = { url?: string; managed_credential?: string } | undefined | null
/** True when the remote authenticates itself, i.e. it carries a `user@` or
* `user:password@` userinfo component. Mirrors the server's rule, which is what
* decides whether it attaches the stored credential at all. */
function urlCarriesCredential(url: string | undefined): boolean {
return /:\/\/[^/@]+@/.test(url ?? '')
}
/**
* The host whose token Windmill holds for this repository, or undefined when it
* holds none.
*
* A URL that carries its own credential wins over the marker: the server skips
* the stored credential for such a URL, so honouring a stale marker here would
* have the UI promise renewal for a token nothing renews. That happens whenever
* someone puts a token back in the URL without clearing the marker, which is why
* this is checked rather than trusting the marker alone.
*
* A `$var:` URL is treated the same way. Only the server can resolve it, so
* whether it carries a token is unknowable here, and claiming a managed
* credential would be a guess: the picker always writes a plain URL, so nothing
* this marker legitimately describes reaches us as a variable reference.
*/
export function managedCredentialHost(value: GitRepositoryValue): string | undefined {
const host = value?.managed_credential
if (!host || host === 'none') return undefined
const url = value?.url
if (url?.startsWith('$var:')) return undefined
return urlCarriesCredential(url) ? undefined : host
}