diff --git a/backend/.sqlx/query-c407fd54e2e2b461fc0883261616134b60d800ff262bf704a2e3aaa90471bd3f.json b/backend/.sqlx/query-c407fd54e2e2b461fc0883261616134b60d800ff262bf704a2e3aaa90471bd3f.json new file mode 100644 index 0000000000..a3cbc7bd2e --- /dev/null +++ b/backend/.sqlx/query-c407fd54e2e2b461fc0883261616134b60d800ff262bf704a2e3aaa90471bd3f.json @@ -0,0 +1,57 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT value as \"value!: sqlx::types::Json>\"\n FROM draft\n WHERE workspace_id = $1 AND email = $2 AND path = $3\n AND typ = $4", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "value!: sqlx::types::Json>", + "type_info": "Json" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + { + "Custom": { + "name": "draft_kind", + "kind": { + "Enum": [ + "script", + "flow", + "app", + "raw_app", + "resource", + "variable", + "trigger_schedule", + "trigger_webhook", + "trigger_default_email", + "trigger_email", + "trigger_http", + "trigger_websocket", + "trigger_postgres", + "trigger_kafka", + "trigger_nats", + "trigger_mqtt", + "trigger_sqs", + "trigger_gcp", + "trigger_azure", + "trigger_poll", + "trigger_cli", + "trigger_nextcloud", + "trigger_google", + "trigger_github" + ] + } + } + } + ] + }, + "nullable": [ + false + ] + }, + "hash": "c407fd54e2e2b461fc0883261616134b60d800ff262bf704a2e3aaa90471bd3f" +} diff --git a/backend/windmill-common/src/user_drafts.rs b/backend/windmill-common/src/user_drafts.rs index 2997c07ca3..60d0cff03d 100644 --- a/backend/windmill-common/src/user_drafts.rs +++ b/backend/windmill-common/src/user_drafts.rs @@ -463,6 +463,17 @@ pub async fn fetch_draft_only( /// endpoints. pub const ENCRYPTED_DRAFT_PREFIX: &str = "$encrypted:"; +/// Placeholder the client receives in place of a draft secret's +/// `$encrypted:` ciphertext. The ciphertext NEVER leaves the server — +/// `get_variable` swaps it for this sentinel before responding. On deploy +/// the client sends the sentinel back, and the server rehydrates the real +/// secret from the caller's OWN draft row (see +/// `rehydrate_secret_from_own_draft`). This is what closes the +/// ciphertext-laundering oracle: the server only ever decrypts a +/// ciphertext it produced for this exact (workspace, path, email), never +/// one a client hands it. +pub const DRAFT_SECRET_SENTINEL: &str = "$draft_secret"; + fn draft_decrypt_error() -> crate::error::Error { crate::error::Error::BadRequest( "An encrypted draft secret could not be decrypted (the workspace encryption key may \ diff --git a/backend/windmill-store/src/variables.rs b/backend/windmill-store/src/variables.rs index 220c3601c2..1ca7853139 100644 --- a/backend/windmill-store/src/variables.rs +++ b/backend/windmill-store/src/variables.rs @@ -37,7 +37,8 @@ use windmill_common::{ scripts::ScriptHash, user_drafts::{ decrypt_draft_secret_value, delete_all_drafts_for_path, fetch_draft_only, - maybe_overlay_draft, UserDraftItemKind, WithDraftOverlay, ENCRYPTED_DRAFT_PREFIX, + maybe_overlay_draft, UserDraftItemKind, WithDraftOverlay, DRAFT_SECRET_SENTINEL, + ENCRYPTED_DRAFT_PREFIX, }, utils::{not_found_if_none, paginate, Pagination, StripPath, WarnAfterExt}, variables::{ @@ -342,6 +343,110 @@ struct GetVariableQuery { get_draft: bool, } +/// Replace a draft secret's `$encrypted:` ciphertext with the opaque +/// `DRAFT_SECRET_SENTINEL` in a draft JSON value (the editor's +/// `VariableState` shape: `{ variable: { value, is_secret, .. }, .. }`), +/// so the ciphertext never reaches the client. No-op for non-secret or +/// non-encrypted values. Operates in place. +fn scrub_secret_draft_value(v: &mut serde_json::Value) { + let Some(var) = v.get_mut("variable") else { + return; + }; + let is_secret = var + .get("is_secret") + .and_then(|x| x.as_bool()) + .unwrap_or(false); + if !is_secret { + return; + } + if let Some(serde_json::Value::String(s)) = var.get_mut("value") { + if s.starts_with(ENCRYPTED_DRAFT_PREFIX) { + *s = DRAFT_SECRET_SENTINEL.to_string(); + } + } +} + +/// Scrub draft secret ciphertext out of a variable get-by-path overlay +/// before it goes on the wire — both the `draft` field and, for +/// draft-only variables, the `inner` stand-in (which `fetch_draft_only` +/// fills from the same draft JSON). +fn scrub_secret_overlay(overlay: &mut WithDraftOverlay) { + if let Some(draft) = overlay.draft.as_mut() { + scrub_secret_draft_value(draft); + } + if overlay.no_deployed { + scrub_secret_draft_value(&mut overlay.inner); + } +} + +/// Rehydrate a secret variable's plaintext for deploy from the CALLER'S +/// OWN draft row — the only place a `$encrypted:` ciphertext is ever +/// decrypted. Invoked when the client deploys with the +/// `DRAFT_SECRET_SENTINEL` placeholder (it never holds the ciphertext). +/// The server thus only decrypts ciphertext it produced for this exact +/// `(workspace, path, email)`, so a stolen ciphertext can't be laundered +/// into plaintext by submitting it at a writable path. +async fn rehydrate_secret_from_own_draft( + 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 Err(Error::BadRequest( + "No saved draft secret to deploy at this path. Re-enter the secret value.".to_string(), + )); + }; + let v: serde_json::Value = serde_json::from_str(row.0.get())?; + let value = v + .get("variable") + .and_then(|x| x.get("value")) + .and_then(|x| x.as_str()) + .unwrap_or(""); + if value.starts_with(ENCRYPTED_DRAFT_PREFIX) { + decrypt_draft_secret_value(db, w_id, value).await + } else { + // The draft already holds plaintext (e.g. a value typed but not + // yet round-tripped through encryption) — use it as-is. + Ok(value.to_string()) + } +} + +/// Resolve the deploy-time secret value for a variable, closing the +/// laundering oracle. The client may send the `DRAFT_SECRET_SENTINEL` +/// (rehydrate from its own draft) or fresh plaintext — but NEVER a +/// `$encrypted:` ciphertext, which is rejected outright. +async fn resolve_secret_for_deploy( + db: &DB, + w_id: &str, + email: &str, + draft_path: &str, + submitted: &str, +) -> Result { + if submitted == DRAFT_SECRET_SENTINEL { + rehydrate_secret_from_own_draft(db, w_id, email, draft_path).await + } else if submitted.starts_with(ENCRYPTED_DRAFT_PREFIX) { + Err(Error::BadRequest( + "A draft secret must be deployed via its placeholder, not a raw ciphertext." + .to_string(), + )) + } else { + Ok(submitted.to_string()) + } +} + async fn get_variable( authed: ApiAuthed, Extension(user_db): Extension, @@ -447,7 +552,7 @@ async fn get_variable( variable }; - let overlay = maybe_overlay_draft( + let mut overlay = maybe_overlay_draft( &db, &w_id, &authed.email, @@ -457,6 +562,9 @@ async fn get_variable( r, ) .await?; + // Never ship a draft secret's ciphertext to the client — replace it + // with the opaque sentinel. Deploy rehydrates from the draft row. + scrub_secret_overlay(&mut overlay); Ok(Json(overlay)) } @@ -596,14 +704,13 @@ async fn create_variable( check_path_conflict(&db, &w_id, &variable.path).await?; let value = if variable.is_secret && !already_encrypted.unwrap_or(false) { - // Deploying a restored draft sends the draft's `$encrypted:` marker - // as-is — decrypt it back (validating it against the workspace key) - // so it goes through the secret backend like any typed plaintext. - let plain = if variable.value.starts_with(ENCRYPTED_DRAFT_PREFIX) { - decrypt_draft_secret_value(&db, &w_id, &variable.value).await? - } else { - variable.value.clone() - }; + // A restored draft deploys via the `$draft_secret` sentinel — + // rehydrate the plaintext from the caller's own draft row. A raw + // `$encrypted:` ciphertext from the client is rejected (laundering + // oracle). Fresh plaintext passes through. + let plain = + resolve_secret_for_deploy(&db, &w_id, &authed.email, &variable.path, &variable.value) + .await?; // Use secret backend for encryption (supports both DB and Vault) store_secret_value(&db, &w_id, &variable.path, &plain).await? } else { @@ -1082,14 +1189,12 @@ async fn update_variable( }; let value = if is_secret && !already_encrypted.unwrap_or(false) { - // Deploying a restored draft sends the draft's `$encrypted:` - // marker as-is — decrypt it back (validating it against the - // workspace key) before re-storing through the secret backend. - let plain = if nvalue.starts_with(ENCRYPTED_DRAFT_PREFIX) { - decrypt_draft_secret_value(&db, &w_id, &nvalue).await? - } else { - nvalue - }; + // A restored draft deploys via the `$draft_secret` sentinel — + // rehydrate from the caller's own draft row (keyed by the + // CURRENT `path`, where the draft was saved, not the renamed + // `target_path`). A raw `$encrypted:` ciphertext is rejected + // (laundering oracle); fresh plaintext passes through. + let plain = resolve_secret_for_deploy(&db, &w_id, &authed.email, path, &nvalue).await?; // Use secret backend for encryption (supports both DB and Vault) // Store at target_path (new path if renaming, otherwise current path) store_secret_value(&db, &w_id, target_path, &plain).await? diff --git a/frontend/src/lib/encryptedDraft.ts b/frontend/src/lib/encryptedDraft.ts index c0062058a7..842b73791a 100644 --- a/frontend/src/lib/encryptedDraft.ts +++ b/frontend/src/lib/encryptedDraft.ts @@ -1,10 +1,13 @@ -/** Marker prefix for draft secret values the backend encrypted at rest - * with the workspace key (mirrors `ENCRYPTED_DRAFT_PREFIX` in - * `backend/windmill-common/src/user_drafts.rs`). The plaintext cannot be - * recovered client-side — deploying sends the marker as-is and the - * deploy endpoints decrypt it server-side. */ -export const ENCRYPTED_DRAFT_PREFIX = '$encrypted:' +/** Opaque placeholder the server sends in place of a draft secret's + * value (mirrors `DRAFT_SECRET_SENTINEL` in + * `backend/windmill-common/src/user_drafts.rs`). The real secret — + * encrypted at rest with the workspace key — NEVER leaves the server: + * `get_variable` swaps the ciphertext for this sentinel. Deploying sends + * the sentinel back and the server rehydrates the plaintext from the + * caller's own draft row. A field holding this value is "secret set, + * hidden, unchanged" — masked in the UI, not editable in place. */ +export const DRAFT_SECRET_SENTINEL = '$draft_secret' export function isEncryptedDraftValue(v: unknown): boolean { - return typeof v === 'string' && v.startsWith(ENCRYPTED_DRAFT_PREFIX) + return v === DRAFT_SECRET_SENTINEL }