mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-19 00:02:03 +00:00
fix(drafts): close variable draft-secret laundering oracle (sentinel + rehydrate)
save_draft encrypts secret variable values with the workspace key, but the ciphertext was round-tripped to the client and the deploy endpoints decrypted whatever $encrypted: ciphertext the client submitted (variables.rs create/update). Any workspace member who can write a variable path could take an arbitrary workspace-key ciphertext (another user's secret draft via GET /drafts/get with only path-read, or a deployed secret's stored value) and submit it as their own secret variable's value — the server decrypted it and, since they own the path, they read the plaintext back. That bypasses the audited decrypt_secret permission. Fix: the ciphertext never leaves the server. get_variable swaps a draft secret's $encrypted: value for an opaque $draft_secret sentinel (both the draft overlay and the draft-only inner stand-in). On deploy the client sends the sentinel back and the server rehydrates the plaintext from the caller's OWN draft row — the only ciphertext it ever decrypts is one it encrypted for this exact (workspace, path, email). A raw $encrypted: submitted by a client is now rejected outright. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
+57
@@ -0,0 +1,57 @@
|
|||||||
|
{
|
||||||
|
"db_name": "PostgreSQL",
|
||||||
|
"query": "SELECT value as \"value!: sqlx::types::Json<Box<serde_json::value::RawValue>>\"\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<Box<serde_json::value::RawValue>>",
|
||||||
|
"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"
|
||||||
|
}
|
||||||
@@ -463,6 +463,17 @@ pub async fn fetch_draft_only(
|
|||||||
/// endpoints.
|
/// endpoints.
|
||||||
pub const ENCRYPTED_DRAFT_PREFIX: &str = "$encrypted:";
|
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 {
|
fn draft_decrypt_error() -> crate::error::Error {
|
||||||
crate::error::Error::BadRequest(
|
crate::error::Error::BadRequest(
|
||||||
"An encrypted draft secret could not be decrypted (the workspace encryption key may \
|
"An encrypted draft secret could not be decrypted (the workspace encryption key may \
|
||||||
|
|||||||
@@ -37,7 +37,8 @@ use windmill_common::{
|
|||||||
scripts::ScriptHash,
|
scripts::ScriptHash,
|
||||||
user_drafts::{
|
user_drafts::{
|
||||||
decrypt_draft_secret_value, delete_all_drafts_for_path, fetch_draft_only,
|
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},
|
utils::{not_found_if_none, paginate, Pagination, StripPath, WarnAfterExt},
|
||||||
variables::{
|
variables::{
|
||||||
@@ -342,6 +343,110 @@ struct GetVariableQuery {
|
|||||||
get_draft: bool,
|
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<String> {
|
||||||
|
let row = sqlx::query_scalar!(
|
||||||
|
r#"SELECT value as "value!: sqlx::types::Json<Box<serde_json::value::RawValue>>"
|
||||||
|
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<String> {
|
||||||
|
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(
|
async fn get_variable(
|
||||||
authed: ApiAuthed,
|
authed: ApiAuthed,
|
||||||
Extension(user_db): Extension<UserDB>,
|
Extension(user_db): Extension<UserDB>,
|
||||||
@@ -447,7 +552,7 @@ async fn get_variable(
|
|||||||
variable
|
variable
|
||||||
};
|
};
|
||||||
|
|
||||||
let overlay = maybe_overlay_draft(
|
let mut overlay = maybe_overlay_draft(
|
||||||
&db,
|
&db,
|
||||||
&w_id,
|
&w_id,
|
||||||
&authed.email,
|
&authed.email,
|
||||||
@@ -457,6 +562,9 @@ async fn get_variable(
|
|||||||
r,
|
r,
|
||||||
)
|
)
|
||||||
.await?;
|
.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))
|
Ok(Json(overlay))
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -596,14 +704,13 @@ async fn create_variable(
|
|||||||
|
|
||||||
check_path_conflict(&db, &w_id, &variable.path).await?;
|
check_path_conflict(&db, &w_id, &variable.path).await?;
|
||||||
let value = if variable.is_secret && !already_encrypted.unwrap_or(false) {
|
let value = if variable.is_secret && !already_encrypted.unwrap_or(false) {
|
||||||
// Deploying a restored draft sends the draft's `$encrypted:` marker
|
// A restored draft deploys via the `$draft_secret` sentinel —
|
||||||
// as-is — decrypt it back (validating it against the workspace key)
|
// rehydrate the plaintext from the caller's own draft row. A raw
|
||||||
// so it goes through the secret backend like any typed plaintext.
|
// `$encrypted:` ciphertext from the client is rejected (laundering
|
||||||
let plain = if variable.value.starts_with(ENCRYPTED_DRAFT_PREFIX) {
|
// oracle). Fresh plaintext passes through.
|
||||||
decrypt_draft_secret_value(&db, &w_id, &variable.value).await?
|
let plain =
|
||||||
} else {
|
resolve_secret_for_deploy(&db, &w_id, &authed.email, &variable.path, &variable.value)
|
||||||
variable.value.clone()
|
.await?;
|
||||||
};
|
|
||||||
// Use secret backend for encryption (supports both DB and Vault)
|
// Use secret backend for encryption (supports both DB and Vault)
|
||||||
store_secret_value(&db, &w_id, &variable.path, &plain).await?
|
store_secret_value(&db, &w_id, &variable.path, &plain).await?
|
||||||
} else {
|
} else {
|
||||||
@@ -1082,14 +1189,12 @@ async fn update_variable(
|
|||||||
};
|
};
|
||||||
|
|
||||||
let value = if is_secret && !already_encrypted.unwrap_or(false) {
|
let value = if is_secret && !already_encrypted.unwrap_or(false) {
|
||||||
// Deploying a restored draft sends the draft's `$encrypted:`
|
// A restored draft deploys via the `$draft_secret` sentinel —
|
||||||
// marker as-is — decrypt it back (validating it against the
|
// rehydrate from the caller's own draft row (keyed by the
|
||||||
// workspace key) before re-storing through the secret backend.
|
// CURRENT `path`, where the draft was saved, not the renamed
|
||||||
let plain = if nvalue.starts_with(ENCRYPTED_DRAFT_PREFIX) {
|
// `target_path`). A raw `$encrypted:` ciphertext is rejected
|
||||||
decrypt_draft_secret_value(&db, &w_id, &nvalue).await?
|
// (laundering oracle); fresh plaintext passes through.
|
||||||
} else {
|
let plain = resolve_secret_for_deploy(&db, &w_id, &authed.email, path, &nvalue).await?;
|
||||||
nvalue
|
|
||||||
};
|
|
||||||
// Use secret backend for encryption (supports both DB and Vault)
|
// Use secret backend for encryption (supports both DB and Vault)
|
||||||
// Store at target_path (new path if renaming, otherwise current path)
|
// Store at target_path (new path if renaming, otherwise current path)
|
||||||
store_secret_value(&db, &w_id, target_path, &plain).await?
|
store_secret_value(&db, &w_id, target_path, &plain).await?
|
||||||
|
|||||||
@@ -1,10 +1,13 @@
|
|||||||
/** Marker prefix for draft secret values the backend encrypted at rest
|
/** Opaque placeholder the server sends in place of a draft secret's
|
||||||
* with the workspace key (mirrors `ENCRYPTED_DRAFT_PREFIX` in
|
* value (mirrors `DRAFT_SECRET_SENTINEL` in
|
||||||
* `backend/windmill-common/src/user_drafts.rs`). The plaintext cannot be
|
* `backend/windmill-common/src/user_drafts.rs`). The real secret —
|
||||||
* recovered client-side — deploying sends the marker as-is and the
|
* encrypted at rest with the workspace key — NEVER leaves the server:
|
||||||
* deploy endpoints decrypt it server-side. */
|
* `get_variable` swaps the ciphertext for this sentinel. Deploying sends
|
||||||
export const ENCRYPTED_DRAFT_PREFIX = '$encrypted:'
|
* 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 {
|
export function isEncryptedDraftValue(v: unknown): boolean {
|
||||||
return typeof v === 'string' && v.startsWith(ENCRYPTED_DRAFT_PREFIX)
|
return v === DRAFT_SECRET_SENTINEL
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user