mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-05 16:03:47 +00:00
fix: refuse an unprovable check and guard the picker on the stored repository
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C1xHmkxuxYb1GYvth1BS75
This commit is contained in:
co-authored by
Claude Opus 5
parent
09280aefd6
commit
c15834bd46
@@ -1 +1 @@
|
||||
6ebc78675dc3510a8abc56a957b96160a4dc7f5f
|
||||
64567911809a2feb4cb0383153939d25585d1f7b
|
||||
|
||||
@@ -1396,19 +1396,24 @@ async fn maybe_post_git_sync_check(
|
||||
// identity existed keeps using the URL it captured at enqueue, which cannot
|
||||
// have been repointed since.
|
||||
let repo_url = match (check.repo.is_some(), row.repo_path.as_deref(), check.repo_url.clone()) {
|
||||
// The resource path is mutable, so following it is only safe when the
|
||||
// marker also carries the identity to check the result against.
|
||||
(true, Some(path), _) => {
|
||||
windmill_common::git_sync_ee::resolve_repo_url_interpolated(db, workspace_id, path)
|
||||
.await
|
||||
}
|
||||
// A marker written before that identity existed captured the URL itself,
|
||||
// which cannot have been repointed since.
|
||||
(_, _, Some(url)) => {
|
||||
windmill_common::variables::get_variable_or_self(url, db, workspace_id).await
|
||||
}
|
||||
(false, Some(path), None) => {
|
||||
windmill_common::git_sync_ee::resolve_repo_url_interpolated(db, workspace_id, path)
|
||||
.await
|
||||
}
|
||||
(_, None, None) => {
|
||||
tracing::error!("git sync-check: marker names no repository");
|
||||
// Neither: nothing here can prove which repository this check belongs to,
|
||||
// and resolving the path anyway is how a preview reaches the wrong one.
|
||||
// Leaving the check unfinished is the safe failure.
|
||||
_ => {
|
||||
tracing::error!(
|
||||
"git sync-check: the marker carries neither a repository identity nor a url; not acting on it"
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
@@ -69,23 +69,54 @@
|
||||
}
|
||||
})
|
||||
|
||||
// Applying replaces whatever is at this path, and anything else pointing at it
|
||||
// would silently start using a different repository.
|
||||
let variableExists = $state(false)
|
||||
// Host and path of a git remote, lowercased with `.git` dropped: the same
|
||||
// identity the backend compares, so "is this the same repository?" gets one
|
||||
// answer on both sides.
|
||||
function repoIdentity(url: string): string | undefined {
|
||||
try {
|
||||
const u = new URL(url.trim())
|
||||
const path = u.pathname
|
||||
.toLowerCase()
|
||||
.replace(/^\/+|\/+$/g, '')
|
||||
.replace(/\.git$/, '')
|
||||
return path ? `${u.host.toLowerCase()}/${path}` : undefined
|
||||
} catch {
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
|
||||
// What already lives at this path decides whether applying is safe. A
|
||||
// suggested path can collide with another project's, and the field is free
|
||||
// text, so the name alone proves nothing: only the repository the stored
|
||||
// value points at does.
|
||||
type Occupant = 'free' | 'same-repo' | 'other'
|
||||
let occupant: Occupant = $state('free')
|
||||
$effect(() => {
|
||||
const path = variablePath
|
||||
const workspace = ws
|
||||
if (!workspace || !path) {
|
||||
variableExists = false
|
||||
const target = project ? repoIdentity(project.http_url_to_repo) : undefined
|
||||
if (!workspace || !path || !target) {
|
||||
occupant = 'free'
|
||||
return
|
||||
}
|
||||
let cancelled = false
|
||||
VariableService.existsVariable({ workspace, path })
|
||||
.then((e) => {
|
||||
if (!cancelled) variableExists = e
|
||||
.then(async (exists) => {
|
||||
if (cancelled) return
|
||||
if (!exists) {
|
||||
occupant = 'free'
|
||||
return
|
||||
}
|
||||
// Reading it decrypts a secret, which is why this is admin-only.
|
||||
const current = await VariableService.getVariableValue({ workspace, path }).catch(
|
||||
() => undefined
|
||||
)
|
||||
if (cancelled) return
|
||||
// Unreadable counts as occupied: it is someone else's until proven otherwise.
|
||||
occupant = current && repoIdentity(current) === target ? 'same-repo' : 'other'
|
||||
})
|
||||
.catch(() => {
|
||||
if (!cancelled) variableExists = false
|
||||
if (!cancelled) occupant = 'other'
|
||||
})
|
||||
return () => {
|
||||
cancelled = true
|
||||
@@ -126,8 +157,10 @@
|
||||
|
||||
async function apply(close: (_: any) => void) {
|
||||
// The token is cleared once stored, and re-applying without one would
|
||||
// overwrite the stored credential with an empty password.
|
||||
if (!ws || !project || !token) return
|
||||
// overwrite the stored credential with an empty password. A path holding
|
||||
// another repository's remote is never written: that would repoint every
|
||||
// resource using it, silently, at this project.
|
||||
if (!ws || !project || !token || occupant === 'other') return
|
||||
applying = true
|
||||
try {
|
||||
const value = repositoryUrl(project)
|
||||
@@ -244,10 +277,14 @@
|
||||
size="sm"
|
||||
inputProps={{ oninput: () => (variablePathTouched = true) }}
|
||||
/>
|
||||
{#if variableExists}
|
||||
{#if occupant === 'other'}
|
||||
<div class="text-2xs font-normal text-red-600 dark:text-red-400">
|
||||
This variable already holds something else. Choose another path, or anything using
|
||||
it would start pointing at this project.
|
||||
</div>
|
||||
{:else if occupant === 'same-repo'}
|
||||
<div class="text-2xs font-normal text-hint">
|
||||
This variable already exists and will be replaced. Anything else using it will
|
||||
point at this repository.
|
||||
This variable already points at this project, and its token will be replaced.
|
||||
</div>
|
||||
{/if}
|
||||
</div>
|
||||
@@ -255,7 +292,7 @@
|
||||
<Button
|
||||
variant="accent"
|
||||
unifiedSize="sm"
|
||||
disabled={!project || !variablePath || !token || applying}
|
||||
disabled={!project || !variablePath || !token || occupant === 'other' || applying}
|
||||
startIcon={{
|
||||
icon: applying ? Loader2 : GitBranch,
|
||||
classes: applying ? 'animate-spin' : ''
|
||||
|
||||
Reference in New Issue
Block a user