From b2c38ef4079c9458d25f225fe67f04377cc37df9 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Wed, 10 Jun 2026 17:41:22 +0200 Subject: [PATCH] fix(drafts): don't clobber a secret draft when autosaving the $draft_secret sentinel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After reload the client holds the $draft_secret sentinel for a secret variable (never the ciphertext). Editing some OTHER field (description, labels) triggers an autosave carrying value="$draft_secret" — and save_draft's encrypt_secret_variable_value, seeing a non-empty, non-$encrypted: string, encrypted the literal sentinel, overwriting the real ciphertext in the draft row and losing the secret. Treat the sentinel as "secret unchanged": restore the $encrypted: ciphertext already stored in this user's draft row instead of encrypting the placeholder (falling back to empty only if there's no prior ciphertext). The new lookup reuses the same query shape as the deploy- time rehydrate, so no new offline cache entry. Co-Authored-By: Claude Fable 5 --- backend/windmill-api/src/drafts.rs | 65 ++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/backend/windmill-api/src/drafts.rs b/backend/windmill-api/src/drafts.rs index d22ed90660..c990f0b55c 100644 --- a/backend/windmill-api/src/drafts.rs +++ b/backend/windmill-api/src/drafts.rs @@ -17,7 +17,7 @@ use serde::{Deserialize, Serialize}; use windmill_common::{ db::UserDB, error::{Error, Result}, - user_drafts::{UserDraftItemKind, ENCRYPTED_DRAFT_PREFIX}, + user_drafts::{UserDraftItemKind, DRAFT_SECRET_SENTINEL, ENCRYPTED_DRAFT_PREFIX}, variables::{build_crypt, encrypt}, }; @@ -169,7 +169,7 @@ async fn save_draft( // variable deploy endpoints decrypt the marker back before // persisting for real. let serialized = if kind == UserDraftItemKind::Variable { - encrypt_secret_variable_value(&db, &w_id, value.0.get()).await? + encrypt_secret_variable_value(&db, &w_id, email, path, value.0.get()).await? } else { serde_json::to_string(value).unwrap() }; @@ -266,7 +266,20 @@ async fn save_draft( /// by autosave) pass through untouched. Unexpected shapes pass through /// unchanged — the draft store is schema-less by design and a malformed /// draft is the editor's problem, not a save error. -async fn encrypt_secret_variable_value(db: &DB, w_id: &str, raw: &str) -> Result { +/// +/// The client never holds the ciphertext — on reload it gets the +/// `$draft_secret` sentinel. So an autosave triggered by editing some +/// OTHER field (description, labels) carries `value == "$draft_secret"`, +/// meaning "secret unchanged". We must NOT encrypt that literal (it would +/// clobber the real ciphertext); instead restore the ciphertext already +/// stored in this user's draft row. +async fn encrypt_secret_variable_value( + db: &DB, + w_id: &str, + email: &str, + path: &str, + raw: &str, +) -> Result { let Ok(mut v) = serde_json::from_str::(raw) else { return Ok(raw.to_string()); }; @@ -279,7 +292,15 @@ async fn encrypt_secret_variable_value(db: &DB, w_id: &str, raw: &str) -> Result if let Some(serde_json::Value::String(s)) = v.get_mut("variable").and_then(|x| x.get_mut("value")) { - if !s.is_empty() && !s.starts_with(ENCRYPTED_DRAFT_PREFIX) { + if s == DRAFT_SECRET_SENTINEL { + // "Secret unchanged" — restore the ciphertext from the + // existing draft row rather than encrypting the sentinel. + // If somehow no prior ciphertext exists, fall back to empty + // (there was no real secret to preserve). + *s = existing_draft_secret_ciphertext(db, w_id, email, path) + .await? + .unwrap_or_default(); + } else if !s.is_empty() && !s.starts_with(ENCRYPTED_DRAFT_PREFIX) { let mc = build_crypt(db, w_id).await?; *s = format!("{ENCRYPTED_DRAFT_PREFIX}{}", encrypt(&mc, s)); } @@ -288,6 +309,42 @@ async fn encrypt_secret_variable_value(db: &DB, w_id: &str, raw: &str) -> Result Ok(v.to_string()) } +/// The `$encrypted:` ciphertext currently stored in this user's variable +/// draft row at `path` (if any). Used to preserve an unchanged secret +/// when an autosave carries the `$draft_secret` sentinel. Returns `None` +/// when there's no draft row or its stored value isn't an `$encrypted:` +/// marker. +async fn existing_draft_secret_ciphertext( + db: &DB, + w_id: &str, + email: &str, + path: &str, +) -> Result> { + let row = sqlx::query_scalar!( + r#"SELECT value as "value!: sqlx::types::Json>" + FROM draft + WHERE workspace_id = $1 AND email = $2 AND path = $3 + AND typ = $4"#, + w_id, + email, + path, + UserDraftItemKind::Variable as UserDraftItemKind, + ) + .fetch_optional(db) + .await?; + let Some(row) = row else { + return Ok(None); + }; + let v: serde_json::Value = serde_json::from_str(row.0.get())?; + let existing = v + .get("variable") + .and_then(|x| x.get("value")) + .and_then(|x| x.as_str()) + .filter(|x| x.starts_with(ENCRYPTED_DRAFT_PREFIX)) + .map(|x| x.to_string()); + Ok(existing) +} + #[derive(Deserialize, Debug)] pub struct GetDraftQuery { /// Workspace username of the draft owner to fetch. Omit to fetch the