fix: re-encrypt git sync secrets on workspace key rotation (#11218)

* fix: re-encrypt git sync credentials and webhook secrets on workspace key rotation

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore: point ee-repo-ref at the git sync key rotation companion

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* chore: update ee-repo-ref to bc3ef08c8e4233508c023e6ee847a3cd0b8be43b

This commit updates the EE repository reference after PR #814 was merged in windmill-ee-private.

Previous ee-repo-ref: 8121eac421c5d026f36e2edec140f3c79b23d2cd

New ee-repo-ref: bc3ef08c8e4233508c023e6ee847a3cd0b8be43b

Automated by sync-ee-ref workflow.

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>

backend/ee-repo-ref.txt

@@ -1 +1 @@
-f2fced19fcae81de7f6dac545010ce404c052e1b
+bc3ef08c8e4233508c023e6ee847a3cd0b8be43b

backend/windmill-api-integration-tests/tests/workspace_encryption_key_git_sync.rs

@@ -275,6 +275,67 @@ async fn test_encryption_key_rotation_dispatches_batched_git_sync(
     Ok(())
 }
 
+/// Stored repository tokens and webhook secrets are encrypted under the
+/// workspace key but never synced, so a rotation has to carry them over even
+/// when the caller skips re-encrypting variables.
+#[sqlx::test(migrations = "../migrations", fixtures("base"))]
+async fn test_encryption_key_rotation_reencrypts_git_sync_secrets(
+    db: Pool<Postgres>,
+) -> anyhow::Result<()> {
+    use windmill_common::variables::{build_crypt, crypt_from_key_with_suffix, decrypt, encrypt};
+    initialize_tracing().await;
+
+    create_folder(&db, "28103").await?;
+    create_git_repo_resource(&db).await?;
+    let sync_script_path = "f/28103/test_sync_script_git_secrets";
+    create_sync_script(&db, sync_script_path).await?;
+    setup_git_sync_config(&db, sync_script_path).await?;
+
+    let mc = build_crypt(&db, "test-workspace").await?;
+    sqlx::query(
+        r#"
+        UPDATE workspace_settings SET
+            git_credentials = jsonb_build_array(jsonb_build_object(
+                'token', $1::text, 'repo_identity', 'https://gitlab.example.com/grp/proj')),
+            git_sync = jsonb_set(git_sync, '{repositories,0,auto_pull}', jsonb_build_object(
+                'enabled', true, 'mode', 'webhook', 'webhook_id', 1, 'webhook_secret', $2::text))
+        WHERE workspace_id = 'test-workspace'
+        "#,
+    )
+    .bind(encrypt(&mc, "stored-token"))
+    .bind(encrypt(&mc, "hook-secret"))
+    .execute(&db)
+    .await?;
+
+    let server = ApiServer::start(db.clone()).await?;
+    let port = server.addr.port();
+    let base = format!("http://localhost:{port}/api/w/test-workspace/workspaces");
+
+    let new_key = "c".repeat(64);
+    let resp = authed(client().post(format!("{base}/encryption_key")))
+        .json(&json!({"new_key": new_key, "skip_reencrypt": true}))
+        .send()
+        .await?;
+    assert_eq!(
+        resp.status(),
+        200,
+        "set_encryption_key failed: {}",
+        resp.text().await?
+    );
+
+    let (token, secret): (String, String) = sqlx::query_as(
+        "SELECT git_credentials->0->>'token', git_sync#>>'{repositories,0,auto_pull,webhook_secret}'
+         FROM workspace_settings WHERE workspace_id = 'test-workspace'",
+    )
+    .fetch_one(&db)
+    .await?;
+    let new_mc = crypt_from_key_with_suffix(&new_key, "");
+    assert_eq!(decrypt(&new_mc, token)?, "stored-token");
+    assert_eq!(decrypt(&new_mc, secret)?, "hook-secret");
+
+    Ok(())
+}
+
 /// Regression test for the non-debouncing fallback: a workspace whose sync
 /// script predates hub version 28103 must still receive git-sync jobs for the
 /// encryption_key entry and every re-encrypted secret. Before the fallback was

backend/windmill-api-workspaces/src/workspaces.rs

@@ -38,7 +38,7 @@ use windmill_common::global_settings::HTTP_ROUTE_WORKSPACED_ROUTE;
 use windmill_common::query_builders::{render_db_quoted_identifier, DbType};
 use windmill_common::users::username_to_permissioned_as;
 use windmill_common::variables::{
-    build_crypt, decrypt, encrypt, SECRET_SALT, WORKSPACE_CRYPT_CACHE,
+    crypt_from_key_with_suffix, decrypt, encrypt, WORKSPACE_CRYPT_CACHE,
 };
 use windmill_common::worker::{to_raw_value, CLOUD_HOSTED};
 use windmill_common::workspaces::GitRepositorySettings;
@@ -5690,9 +5690,6 @@ async fn set_encryption_key(
         ));
     }
 
-    // Build the previous cipher before the transaction (reads from cache/pool)
-    let previous_encryption_key = build_crypt(&db, w_id.as_str()).await?;
-
     let mut tx = db.begin().await?;
 
     // Under the row's lock, so two rotations racing serialize and each sees the key the
@@ -5726,17 +5723,14 @@ async fn set_encryption_key(
         None
     };
 
+    // From the keys read and written under the lock, never from `build_crypt`: its
+    // cache can still hold a key an earlier rotation replaced, and the git-sync
+    // secrets below are skipped rather than failed when they do not decrypt.
+    let previous_encryption_key = crypt_from_key_with_suffix(&previous_key, "");
+    let new_encryption_key = crypt_from_key_with_suffix(&request.new_key, "");
+
     let mut reencrypted_secret_paths: Vec<String> = Vec::new();
     if !request.skip_reencrypt.unwrap_or(false) {
-        // Build the new cipher directly from the key string, since the transaction
-        // hasn't committed yet and build_crypt() would read the old key from the pool.
-        let crypt_key = if let Some(ref salt) = SECRET_SALT.as_ref() {
-            format!("{}{}", request.new_key, salt)
-        } else {
-            request.new_key.clone()
-        };
-        let new_encryption_key = magic_crypt::new_magic_crypt!(crypt_key, 256);
-
         let mut truncated_new_key = request.new_key.clone();
         truncated_new_key.truncate(8);
         tracing::warn!(
@@ -5776,6 +5770,14 @@ async fn set_encryption_key(
         }
     }
 
+    reencrypt_git_sync_secrets(
+        &mut tx,
+        &w_id,
+        &previous_encryption_key,
+        &new_encryption_key,
+    )
+    .await?;
+
     tx.commit().await?;
 
     // Invalidate the cache only after the transaction has committed
@@ -5813,6 +5815,64 @@ async fn set_encryption_key(
     return Ok(());
 }
 
+/// Move the git-sync secrets the server keeps under the workspace key (stored
+/// repository tokens, webhook secrets) to the new key. They are never synced, so
+/// unlike variables they are still under the old key when the caller skips
+/// re-encryption.
+async fn reencrypt_git_sync_secrets(
+    conn: &mut sqlx::PgConnection,
+    w_id: &str,
+    old: &magic_crypt::MagicCrypt256,
+    new: &magic_crypt::MagicCrypt256,
+) -> Result<()> {
+    let Some((mut credentials, mut git_sync)) =
+        sqlx::query_as::<_, (serde_json::Value, Option<serde_json::Value>)>(
+            "SELECT git_credentials, git_sync FROM workspace_settings WHERE workspace_id = $1 FOR UPDATE",
+        )
+        .bind(w_id)
+        .fetch_optional(&mut *conn)
+        .await?
+    else {
+        return Ok(());
+    };
+    let reencrypt = |value: &mut serde_json::Value| {
+        let Some(ciphertext) = value.as_str() else {
+            return;
+        };
+        match decrypt(old, ciphertext.to_string()) {
+            Ok(plain) => *value = serde_json::Value::String(encrypt(new, &plain)),
+            // Left by an earlier rotation and unrecoverable either way; failing here
+            // would block every later rotation of the workspace.
+            Err(e) => tracing::warn!(
+                "a git-sync secret of workspace {w_id} does not decrypt under its current key, leaving it as is: {e}"
+            ),
+        }
+    };
+    for entry in credentials.as_array_mut().into_iter().flatten() {
+        if let Some(token) = entry.get_mut("token") {
+            reencrypt(token);
+        }
+    }
+    let repositories = git_sync
+        .as_mut()
+        .and_then(|g| g.get_mut("repositories"))
+        .and_then(|r| r.as_array_mut());
+    for repo in repositories.into_iter().flatten() {
+        if let Some(secret) = repo.pointer_mut("/auto_pull/webhook_secret") {
+            reencrypt(secret);
+        }
+    }
+    sqlx::query(
+        "UPDATE workspace_settings SET git_credentials = $2, git_sync = $3 WHERE workspace_id = $1",
+    )
+    .bind(w_id)
+    .bind(credentials)
+    .bind(git_sync)
+    .execute(&mut *conn)
+    .await?;
+    Ok(())
+}
+
 #[derive(Serialize)]
 struct UsedTriggers {
     pub websocket_used: bool,