From 1aa8e1665e2539e569006bbc7aa39e93f665ec29 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Wed, 8 Jul 2026 18:39:07 +0200 Subject: [PATCH] fix(git-sync): license-gate preserved auto-pull; attach strips parent-only settings - edit_git_sync_repository re-checks the runtime Enterprise gate against the EFFECTIVE repo state after preservation: the older-client arm copies the existing auto_pull back, which the request-side check never saw - attach_dev_workspace now mirrors the fork-creation copy on the attached workspace's own git sync: promotion repos dropped, auto_pull/fork PRs/PR error stripped, and any managed webhook deleted after commit (the attached workspace is parent-managed and must not keep pulling its old tracked branch) - integration test: attaching an auto-pull-enabled workspace strips it Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PP5gBSPfo1YtkL1sWVAjJm --- .../windmill-api-workspaces/src/workspaces.rs | 57 +++++++++++++++++++ integration_tests/test/git_sync_test.py | 52 +++++++++++++++++ 2 files changed, 109 insertions(+) diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 42d946b9d7..3e6a1b516b 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -3218,6 +3218,13 @@ async fn edit_git_sync_repository( } _ => {} } + // The request-side license gate above only saw the submitted config; the + // preservation can resurrect an enabled auto_pull (None arm), so re-check + // the effective state before it gets written and reconciled. + #[cfg(feature = "enterprise")] + if updated.auto_pull.as_ref().is_some_and(|a| a.enabled) { + check_auto_pull_license().await?; + } *existing_repo = updated; } else { // Repository doesn't exist, add it as a new repository @@ -6291,6 +6298,45 @@ async fn attach_dev_workspace( .execute(&mut *tx) .await?; + // The attached workspace is now parent-managed like any fork: its own + // auto-pull (and webhook), fork PRs, and promotion repos must not stay + // live — they'd keep pulling/pushing against its pre-attach tracked + // branch. Mirror the fork-creation copy: keep sync repos only, strip the + // parent-only fields, and delete any managed webhook after commit. + #[allow(unused_mut)] + let mut stripped_webhooks: Vec<(String, i64)> = Vec::new(); + if let Some(git_sync) = sqlx::query_scalar!( + "SELECT git_sync FROM workspace_settings WHERE workspace_id = $1", + &dev_w_id + ) + .fetch_optional(&mut *tx) + .await? + .flatten() + { + if let Ok(mut settings) = serde_json::from_value::(git_sync) { + settings + .repositories + .retain(|r| !r.use_individual_branch.unwrap_or(false)); + for r in settings.repositories.iter_mut() { + if let Some(hook) = r.auto_pull.as_ref().and_then(|a| a.webhook_id) { + stripped_webhooks.push((r.git_repo_resource_path.clone(), hook)); + } + r.auto_pull = None; + r.fork_open_prs = false; + r.open_pr_error = None; + } + let serialized = + serde_json::to_value(&settings).map_err(|e| Error::internal_err(e.to_string()))?; + sqlx::query!( + "UPDATE workspace_settings SET git_sync = $1 WHERE workspace_id = $2", + serialized, + &dev_w_id + ) + .execute(&mut *tx) + .await?; + } + } + if req.lock_prod_deploy || req.lock_prod_forking { lock_prod_workspace( &mut tx, @@ -6316,6 +6362,17 @@ async fn attach_dev_workspace( // The dev workspace's parent just changed (none -> prod); drop its cached fork->parent mapping // so per-workspace job tags route to the prod family immediately rather than after the TTL. windmill_queue::tags::invalidate_fork_parent_cache(&dev_w_id); + // Best-effort: the hooks captured before the strip above are unreachable now + // (their auto_pull is gone), so remove them from GitHub. + #[cfg(all(feature = "enterprise", feature = "private"))] + for (path, hook_id) in stripped_webhooks { + if let Ok(url) = windmill_common::git_sync_ee::resolve_repo_url(&db, &dev_w_id, &path).await + { + let _ = + windmill_common::git_sync_ee::delete_repo_webhook(&db, &dev_w_id, &url, hook_id) + .await; + } + } // Drop the cached ancestor chains too — the workspace existed BEFORE the attach, so a // cached empty chain reads as "not a fork" and its ducklake jobs would write the shared // lake until the TTL. Descendants' chains also gained the new root. diff --git a/integration_tests/test/git_sync_test.py b/integration_tests/test/git_sync_test.py index 6b4042a302..3eda1fcc93 100644 --- a/integration_tests/test/git_sync_test.py +++ b/integration_tests/test/git_sync_test.py @@ -1004,6 +1004,58 @@ class TestGitSyncAutoPull(GitSyncTestBase): f"{response.status_code}: {response.content.decode()}", ) + def test_attach_dev_workspace_strips_auto_pull(self): + """Attaching a workspace as a dev workspace strips its own auto-pull: + dev/fork sync is parent-managed, so a pre-attach auto-pull must not + keep pulling the old tracked branch.""" + candidate_id = f"it-dev-cand-{uuid.uuid4().hex[:8]}" + self._fork_workspaces_to_cleanup.append(candidate_id) + candidate = WindmillClient(workspace=candidate_id) + + repo_name, _ = self._create_test_repo() + resource_path = f"u/admin/git_sync_{repo_name.replace('-', '_')}" + candidate.create_resource( + path=resource_path, + resource_type="git_repository", + value={ + "url": self._gitea.get_docker_clone_url(repo_name), + "branch": "main", + "is_github_app": False, + }, + update_if_exists=True, + ) + candidate.configure_git_sync({ + "repositories": [{ + "git_repo_resource_path": f"$res:{resource_path}", + "use_individual_branch": False, + "group_by_folder": False, + "settings": {"include_type": ["script"], "include_path": ["**"]}, + "auto_pull": {"enabled": True, "mode": "polling"}, + }], + }) + + response = self._client._client.post( + f"/api/w/{self._client._workspace}/workspaces/attach_dev_workspace", + json={"dev_workspace_id": candidate_id, "dev_workspace_label": "dev"}, + ) + self.assertEqual( + response.status_code // 100, + 2, + f"attach_dev_workspace failed: {response.content.decode()}", + ) + + repo_settings = None + for repo in (candidate.get_workspace_settings().get("git_sync") or {}).get( + "repositories", [] + ): + if resource_path in repo.get("git_repo_resource_path", ""): + repo_settings = repo + self.assertIsNotNone(repo_settings, "candidate lost its sync repo on attach") + self.assertIsNone( + repo_settings.get("auto_pull"), + f"auto_pull survived the attach: {repo_settings}", + ) + def test_webhook_receiver_ignores_unknown_deliveries(self): """An unsolicited webhook delivery (no registered hook) is not an error and enqueues nothing."""