starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-08-18 16:02:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: replicate all secrets on fork when external backend is configured (#10060)

Browse Source 
* fix: replicate all secrets on fork with external backend (WIN-2161)

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

* test: add Azure KV fork secret-replication reproduction (WIN-2161)

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

* style: condense clone_variables invariant comment (WIN-2161)

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

* test: drive real create_fork handler in Azure KV repro (WIN-2161)

Replace the windmill-common test that mirrored clone_variables' loop with an
end-to-end test in windmill-api-integration-tests that exercises the real
migration, create_fork and variable-read endpoints against a local Azure KV
emulator. Verified it fails (404 "not found in Azure Key Vault") without the
fix and passes with it; unique per-run ids keep it robust to the emulator's
persistent state.

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

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-07-12 10:19:20 +02:00
committed by GitHub
parent 5cde2d5b67
commit 92b7f375a9
4 changed files with 198 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/.sqlx/query-b6de5fbd5fa89e9a8ab8a61982d4f6e02030d8581a7e8c1f0673c7f6dae78fb3.json
Generated
+22
View File
@@ -0,0 +1,22 @@
{
"db_name": "PostgreSQL",
"query": "SELECT path FROM variable\n WHERE workspace_id = $1 AND is_secret = true AND value != ''",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "path",
"type_info": "Varchar"
}
],
"parameters": {
"Left": [
"Text"
]
},
"nullable": [
false
]
},
"hash": "b6de5fbd5fa89e9a8ab8a61982d4f6e02030d8581a7e8c1f0673c7f6dae78fb3"
}
backend/.sqlx/query-e4a46d47aee96473a2bd0f1dbdc819520a2ba987fd29859845c2177563ab8cfb.json
Generated
-28
View File
@@ -1,28 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "SELECT path, value FROM variable\n WHERE workspace_id = $1 AND is_secret = true AND value != ''",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "path",
"type_info": "Varchar"
},
{
"ordinal": 1,
"name": "value",
"type_info": "Varchar"
}
],
"parameters": {
"Left": [
"Text"
]
},
"nullable": [
false,
false
]
},
"hash": "e4a46d47aee96473a2bd0f1dbdc819520a2ba987fd29859845c2177563ab8cfb"
}
backend/windmill-api-integration-tests/tests/fork_secret_replication_azure.rs
+161
View File
@@ -0,0 +1,161 @@
//! End-to-end regression test for WIN-2161.
//!
//! Reproduces, through real product code, the state after a database-to-external
//! migration: a secret that was created under the database backend and then
//! *migrated* to an external backend (Azure Key Vault). Migration writes the
//! plaintext to the store but
//! leaves the encrypted ciphertext in `variable.value` (it never rewrites it to
//! a `$azure_kv:` marker). The bug: `clone_variables` only replicated
//! marker-valued secrets, so forking left the migrated secret unreplicated and
//! reads in the fork failed with "not found in Azure Key Vault".
//!
//! This drives the real `/migrate_secrets_to_azure_kv`, `/create_fork` and
//! `variables/get_value` endpoints against a local Azure Key Vault emulator
//! (lowkey-vault), which the `AzureKeyVaultBackend` talks to via its
//! static-token / self-signed-cert emulator mode.
//!
//! Run it:
//! ```bash
//! podman run -d --name lowkey -p 8443:8443 \
//! -e LOWKEY_ARGS="--LOWKEY_VAULT_NAMES=default" \
//! docker.io/nagyesta/lowkey-vault:7.3.0
//!
//! RUN_AZURE_KV_TESTS=1 cargo test -p windmill-api-integration-tests \
//! --features private,enterprise --test fork_secret_replication_azure -- --nocapture
//! ```
#[cfg(all(feature = "private", feature = "enterprise"))]
mod azure_fork {
use serde_json::json;
use sqlx::{Pool, Postgres};
use windmill_common::variables::{build_crypt, encrypt};
use windmill_test_utils::*;
fn client() -> reqwest::Client {
reqwest::Client::new()
}
fn authed(builder: reqwest::RequestBuilder) -> reqwest::RequestBuilder {
builder.header("Authorization", "Bearer SECRET_TOKEN")
}
fn vault_url() -> String {
std::env::var("AZURE_KV_URL").unwrap_or_else(|_| "https://localhost:8443".to_string())
}
/// The Azure settings for the emulator: a static token switches the backend
/// into emulator mode (no Entra ID, self-signed certs accepted).
fn azure_settings() -> serde_json::Value {
json!({
"vault_url": vault_url(),
"tenant_id": "emulator-tenant",
"client_id": "emulator-client",
"token": "emulator-token",
})
}
#[sqlx::test(migrations = "../migrations", fixtures("base"))]
async fn migrated_secret_is_replicated_on_fork(db: Pool<Postgres>) -> anyhow::Result<()> {
if std::env::var("RUN_AZURE_KV_TESTS").as_deref() != Ok("1") {
eprintln!("skipping: set RUN_AZURE_KV_TESTS=1 and start lowkey-vault to run");
return Ok(());
}
initialize_tracing().await;
// The Azure KV emulator persists across runs; derive unique names per run
// so a secret written by a previous run can't mask a regression.
let suffix = uuid::Uuid::new_v4().simple().to_string();
let short = &suffix[..8];
let source_ws = "test-workspace";
let path = format!("u/test-user/db_password_{short}");
let path = path.as_str();
let plaintext = "s3cr3t-value";
let ciphertext = {
let mc = build_crypt(&db, source_ws).await?;
encrypt(&mc, plaintext)
};
sqlx::query(
"INSERT INTO variable (workspace_id, path, value, is_secret, description, extra_perms)
VALUES ($1, $2, $3, true, '', '{}')",
)
.bind(source_ws)
.bind(path)
.bind(&ciphertext)
.execute(&db)
.await?;
sqlx::query(
"INSERT INTO global_settings (name, value) VALUES ('secret_backend', $1)
ON CONFLICT (name) DO UPDATE SET value = EXCLUDED.value",
)
.bind(json!({
"type": "AzureKeyVault",
"vault_url": vault_url(),
"tenant_id": "emulator-tenant",
"client_id": "emulator-client",
"token": "emulator-token",
}))
.execute(&db)
.await?;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
let resp = authed(client().post(format!(
"http://localhost:{port}/api/settings/migrate_secrets_to_azure_kv"
)))
.json(&azure_settings())
.send()
.await?;
let status = resp.status();
let body = resp.text().await?;
assert_eq!(status, 200, "migrate_secrets_to_azure_kv failed: {body}");
let report: serde_json::Value = serde_json::from_str(&body)?;
assert!(
report["migrated_count"].as_i64().unwrap_or(0) >= 1,
"expected at least one migrated secret: {report}"
);
// Assert the source resolves before forking, so a fork-read failure is
// attributable to replication rather than a broken seed.
let resp = authed(client().get(format!(
"http://localhost:{port}/api/w/{source_ws}/variables/get_value/{path}"
)))
.send()
.await?;
assert_eq!(resp.status(), 200, "source read: {}", resp.text().await?);
assert_eq!(resp.json::<String>().await?, plaintext);
let fork_ws = format!("wm-fork-az{short}");
let fork_ws = fork_ws.as_str();
let resp = authed(client().post(format!(
"http://localhost:{port}/api/w/{source_ws}/workspaces/create_fork"
)))
.json(&json!({ "id": fork_ws, "name": "Azure Fork Test" }))
.send()
.await?;
assert_eq!(resp.status(), 200, "create_fork: {}", resp.text().await?);
// The fork resolves the secret only if it was replicated under the fork's
// own workspace-id key in the external store.
let resp = authed(client().get(format!(
"http://localhost:{port}/api/w/{fork_ws}/variables/get_value/{path}"
)))
.send()
.await?;
let status = resp.status();
let body = resp.text().await?;
assert_eq!(
status, 200,
"forked secret must resolve, got {status}: {body}"
);
assert_eq!(
serde_json::from_str::<String>(&body)?,
plaintext,
"fork should return the replicated plaintext"
);
Ok(())
}
}
backend/windmill-api-workspaces/src/workspaces.rs
+15 -16
View File
@@ -71,9 +71,7 @@ use hyper::StatusCode;
use serde::{Deserialize, Serialize};
use sqlx::{FromRow, Postgres, Row, Transaction};
use windmill_common::oauth2::InstanceEvent;
use windmill_common::secret_backend::{
get_secret_backend, is_external_stored_value, is_vault_backend_configured,
};
use windmill_common::secret_backend::{get_secret_backend, is_vault_backend_configured};
use windmill_common::utils::not_found_if_none;
lazy_static::lazy_static! {
@@ -4703,15 +4701,13 @@ async fn clone_variables(
.execute(&mut **tx)
.await?;
// With an external secret backend (Vault / Azure KV / AWS SM), the copied
// `value` is only a `$vault:`/`$azure_kv:`/`$aws_sm:` marker: the actual
// secret lives in the external store under a key derived from
// (workspace_id, path). The row copy above therefore leaves the fork's
// markers pointing at keys that don't exist — replicate each secret under
// the fork's workspace id.
// With an external backend the secret lives in the store under (workspace_id,
// path), so the row copy above leaves the fork pointing at keys that don't
// exist. Replicate every secret, not just marker-valued ones: migration writes
// to the store without rewriting `value` to a `$...:` marker.
if is_vault_backend_configured(db).await? {
let secret_variables = sqlx::query!(
"SELECT path, value FROM variable
"SELECT path FROM variable
WHERE workspace_id = $1 AND is_secret = true AND value != ''",
target_workspace_id,
)
@@ -4719,10 +4715,7 @@ async fn clone_variables(
.await?;
let backend = get_secret_backend(db).await?;
for variable in secret_variables
.into_iter()
.filter(|v| is_external_stored_value(&v.value))
{
for variable in secret_variables {
match backend
.get_secret(source_workspace_id, &variable.path)
.await
@@ -5810,8 +5803,14 @@ async fn create_workspace_fork(
.await?;
// Clone all data from the parent workspace using Rust implementation
if let Err(e) =
clone_workspace_data(&mut tx, &db, &parent_workspace_id, &forked_id, &authed.email).await
if let Err(e) = clone_workspace_data(
&mut tx,
&db,
&parent_workspace_id,
&forked_id,
&authed.email,
)
.await
{
// A genuine `\u0000` in a source `json` value (`app_version.value` /
// `flow_version.schema`) aborts the clone when it is re-encoded to jsonb: