From 54841b7549d5c9719d4dc3cb43e282ba057cd0f3 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 16 Mar 2026 14:10:29 +0000 Subject: [PATCH] feat: support multiple secret variables during resource creation (#8386) * feat: support multiple secret variables during resource creation When creating a resource, users can now select multiple fields to be stored as secret variables. If only one field is selected, behavior is unchanged (single variable with same path as resource). If multiple fields are selected, each gets its own variable with _field_name appended to the resource path. Closes #8384 Co-authored-by: windmill-internal-app[bot] * fix: delete all linked secret variables when resource is deleted When a resource with multiple secret fields is deleted, also delete variables matching the {path}_{field_name} pattern in addition to the exact path variable. Each deleted variable gets its own deployment metadata update and webhook notification. Co-authored-by: Diego Imbert Co-Authored-By: Claude Opus 4.6 * Update backend/windmill-store/src/resources.rs Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> * fix: only delete linked variables that are actually referenced in resource value Instead of deleting variables purely based on path prefix matching (which could accidentally delete unrelated variables), the deletion now reads the resource value first, extracts all $var: references, and only deletes variables that are actually used in the resource. Co-authored-by: Diego Imbert Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: windmill-internal-app[bot] Co-authored-by: Diego Imbert Co-authored-by: Claude Opus 4.6 Co-authored-by: Diego Imbert <70353967+diegoimbert@users.noreply.github.com> Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> --- backend/windmill-store/src/resources.rs | 77 +++++++++-- .../src/lib/components/ApiConnectForm.svelte | 12 +- .../src/lib/components/AppConnectInner.svelte | 124 ++++++++++++++---- frontend/src/lib/components/SchemaForm.svelte | 30 +++-- 4 files changed, 182 insertions(+), 61 deletions(-) diff --git a/backend/windmill-store/src/resources.rs b/backend/windmill-store/src/resources.rs index 8d5ba23b67..531a31ea07 100644 --- a/backend/windmill-store/src/resources.rs +++ b/backend/windmill-store/src/resources.rs @@ -883,6 +883,15 @@ async fn delete_resource( } let mut tx = user_db.begin(&authed).await?; + // Fetch the resource value before deleting, so we can find linked $var: references + let resource_value: Option> = sqlx::query_scalar( + "SELECT value FROM resource WHERE path = $1 AND workspace_id = $2", + ) + .bind(path) + .bind(&w_id) + .fetch_optional(&mut *tx) + .await?; + let deleted_path = sqlx::query_scalar!( "DELETE FROM resource WHERE path = $1 AND workspace_id = $2 RETURNING path", path, @@ -891,13 +900,32 @@ async fn delete_resource( .fetch_optional(&mut *tx) .await?; not_found_if_none(deleted_path, "Resource", &path)?; - let deleted_linked_variable = sqlx::query_scalar!( - "DELETE FROM variable WHERE path = $1 AND workspace_id = $2 RETURNING path", - path, - w_id - ) - .fetch_optional(&mut *tx) - .await?; + + // Collect all $var: paths referenced in the resource value + let mut linked_var_paths: Vec = Vec::new(); + if let Some(Some(value)) = resource_value { + collect_var_refs(&value, &mut linked_var_paths); + } + + // Delete linked variables that are actually referenced in the resource value + let deleted_linked_variables: Vec = if linked_var_paths.is_empty() { + Vec::new() + } else { + let placeholders: Vec = linked_var_paths + .iter() + .enumerate() + .map(|(i, _)| format!("${}", i + 2)) + .collect(); + let query = format!( + "DELETE FROM variable WHERE workspace_id = $1 AND path IN ({}) RETURNING path", + placeholders.join(", ") + ); + let mut q = sqlx::query_scalar::<_, String>(&query).bind(&w_id); + for var_path in &linked_var_paths { + q = q.bind(var_path); + } + q.fetch_all(&mut *tx).await? + }; audit_log( &mut *tx, &authed, @@ -927,19 +955,19 @@ async fn delete_resource( WebhookMessage::DeleteResource { workspace: w_id.clone(), path: path.to_owned() }, ); - if deleted_linked_variable.is_some() { + for var_path in &deleted_linked_variables { handle_deployment_metadata( &authed.email, &authed.username, &db, &w_id, DeployedObject::Variable { - path: path.to_string(), - parent_path: Some(path.to_string()), + path: var_path.clone(), + parent_path: Some(var_path.clone()), }, Some(format!( "Variable '{}' deleted (linked resource deleted)", - path + var_path )), true, None, @@ -948,13 +976,38 @@ async fn delete_resource( webhook.send_message( w_id.clone(), - WebhookMessage::DeleteVariable { workspace: w_id, path: path.to_owned() }, + WebhookMessage::DeleteVariable { + workspace: w_id.clone(), + path: var_path.clone(), + }, ); } Ok(format!("resource {} deleted", path)) } +/// Recursively collect all `$var:path` references from a JSON value. +fn collect_var_refs(value: &serde_json::Value, out: &mut Vec) { + match value { + serde_json::Value::String(s) => { + if let Some(var_path) = s.strip_prefix("$var:") { + out.push(var_path.to_string()); + } + } + serde_json::Value::Object(m) => { + for v in m.values() { + collect_var_refs(v, out); + } + } + serde_json::Value::Array(arr) => { + for v in arr { + collect_var_refs(v, out); + } + } + _ => {} + } +} + async fn delete_resources_bulk( authed: ApiAuthed, Extension(db): Extension, diff --git a/frontend/src/lib/components/ApiConnectForm.svelte b/frontend/src/lib/components/ApiConnectForm.svelte index 8a79bd7109..636a20f290 100644 --- a/frontend/src/lib/components/ApiConnectForm.svelte +++ b/frontend/src/lib/components/ApiConnectForm.svelte @@ -21,7 +21,7 @@ resourceType: string resourceTypeInfo: ResourceType | undefined args?: Record | any - linkedSecret?: string | undefined + linkedSecrets?: string[] isValid?: boolean linkedSecretCandidates?: string[] | undefined description?: string | undefined @@ -31,7 +31,7 @@ resourceType, resourceTypeInfo, args = $bindable({}), - linkedSecret = $bindable(undefined), + linkedSecrets = $bindable([]), isValid = $bindable(true), linkedSecretCandidates = undefined, description = $bindable(undefined) @@ -152,7 +152,7 @@ /> @@ -246,9 +246,7 @@ {/await} {:else if resourceTypeInfo?.is_fileset} -
- Fileset -
+
Fileset
{:else if resourceTypeInfo?.format_extension}
@@ -273,7 +271,7 @@ onlyMaskPassword noDelete {linkedSecretCandidates} - bind:linkedSecret + bind:linkedSecrets isValid {schema} bind:args diff --git a/frontend/src/lib/components/AppConnectInner.svelte b/frontend/src/lib/components/AppConnectInner.svelte index 5b4faf2fe6..d7b37fcdd9 100644 --- a/frontend/src/lib/components/AppConnectInner.svelte +++ b/frontend/src/lib/components/AppConnectInner.svelte @@ -84,14 +84,22 @@ ) } - let linkedSecret: string | undefined = $state(undefined) + let linkedSecrets: string[] = $state([]) let linkedSecretCandidates: string[] | undefined = $state(undefined) - function computeLinkedSecret(resourceType: string, argsKeys: string[], passwords: string[]) { + function computeDefaultLinkedSecrets( + resourceType: string, + argsKeys: string[], + passwords: string[] + ): string[] { linkedSecretCandidates = computeCandidates(resourceType, argsKeys, passwords) - return ( - forceSecretValue(resourceType) ?? - linkedSecretCandidates?.sort((ua, ub) => linkedSecretValue(ub) - linkedSecretValue(ua))?.[0] - ) + const forced = forceSecretValue(resourceType) + if (forced) { + return [forced] + } + const best = linkedSecretCandidates?.sort( + (ua, ub) => linkedSecretValue(ub) - linkedSecretValue(ua) + )?.[0] + return best ? [best] : [] } let scopes: string[] = $state([]) @@ -194,7 +202,7 @@ args['password'] == '' && args['api_key'] == '' && args['key'] == '' && - linkedSecret != undefined + linkedSecrets.length > 0 : false)) || step == 3 || (step == 4 && pathError != '') || @@ -317,13 +325,13 @@ const passwords = newArgsKeys.filter((x) => { return props?.[x]?.password }) - if (!linkedSecret) { - linkedSecret = computeLinkedSecret(resourceType, newArgsKeys, passwords) + if (linkedSecrets.length === 0) { + linkedSecrets = computeDefaultLinkedSecrets(resourceType, newArgsKeys, passwords) } } export async function next() { if (step == 1) { - linkedSecret = undefined + linkedSecrets = [] if (manual) { getResourceTypeInfo() args = {} @@ -408,14 +416,30 @@ if (step == 2) return throw Error('Path is not set') } - let exists = await VariableService.existsVariable({ - workspace: $workspaceStore!, - path - }) - if (exists) { - throw Error(`Variable at path ${path} already exists. Delete it or pick another path`) + // Check if variable paths already exist + if (!manual || linkedSecrets.length <= 1) { + const exists = await VariableService.existsVariable({ + workspace: $workspaceStore!, + path + }) + if (exists) { + throw Error(`Variable at path ${path} already exists. Delete it or pick another path`) + } + } else { + for (const secretField of linkedSecrets) { + const varPath = `${path}_${secretField}` + const exists = await VariableService.existsVariable({ + workspace: $workspaceStore!, + path: varPath + }) + if (exists) { + throw Error( + `Variable at path ${varPath} already exists. Delete it or pick another path` + ) + } + } } - exists = await ResourceService.existsResource({ + let exists = await ResourceService.existsResource({ workspace: $workspaceStore!, path }) @@ -462,25 +486,65 @@ const resourceValue = args - let saveVariable = false - if (!manual || linkedSecret != undefined) { - let v = manual ? args[linkedSecret ?? ''] : value + let savedVariableCount = 0 + if (!manual) { + // OAuth flow: single secret variable for the token + if (typeof value == 'string' && value != '' && !value.startsWith('$var:')) { + savedVariableCount++ + await VariableService.createVariable({ + workspace: $workspaceStore!, + requestBody: { + path, + value: value, + is_secret: true, + description: emptyString(description) + ? `OAuth token for ${resourceType}` + : description, + is_oauth: true, + account: account + } + }) + resourceValue['token'] = `$var:${path}` + } + } else if (linkedSecrets.length === 1) { + // Single secret: use the resource path as variable name (original behavior) + const secretField = linkedSecrets[0] + const v = args[secretField] if (typeof v == 'string' && v != '' && !v.startsWith('$var:')) { - saveVariable = true + savedVariableCount++ await VariableService.createVariable({ workspace: $workspaceStore!, requestBody: { path, value: v, is_secret: true, - description: emptyString(description) - ? `${manual ? 'Token' : 'OAuth token'} for ${resourceType}` - : description, - is_oauth: !manual, - account: account + description: emptyString(description) ? `Token for ${resourceType}` : description, + is_oauth: false } }) - resourceValue[linkedSecret ?? 'token'] = `$var:${path}` + resourceValue[secretField] = `$var:${path}` + } + } else if (linkedSecrets.length > 1) { + // Multiple secrets: append _field_name to each variable path + for (const secretField of linkedSecrets) { + const v = args[secretField] + if (typeof v == 'string' && v != '' && !v.startsWith('$var:')) { + const varPath = `${path}_${secretField}` + savedVariableCount++ + await VariableService.createVariable({ + workspace: $workspaceStore!, + requestBody: { + path: varPath, + value: v, + is_secret: true, + description: emptyString(description) + ? `${secretField} for ${resourceType}` + : description, + is_oauth: false + } + }) + resourceValue[secretField] = `$var:${varPath}` + } } } @@ -495,7 +559,9 @@ }) dispatch('refresh', path) dispatch('close') - sendUserToast(`Saved resource${saveVariable ? ' and variable' : ''} path: ${path}`) + sendUserToast( + `Saved resource${savedVariableCount > 0 ? ` and ${savedVariableCount} variable${savedVariableCount > 1 ? 's' : ''}` : ''} path: ${path}` + ) step = 1 resourceType = '' } @@ -738,7 +804,7 @@ {#key resourceTypeInfo} shouldHideNoInputs?: boolean compact?: boolean - linkedSecret?: string | undefined + linkedSecrets?: string[] linkedSecretCandidates?: string[] | undefined noVariablePicker?: boolean flexWrap?: boolean @@ -86,7 +86,7 @@ defaultValues = {}, shouldHideNoInputs = false, compact = false, - linkedSecret = $bindable(undefined), + linkedSecrets = $bindable([]), linkedSecretCandidates = undefined, noVariablePicker = false, flexWrap = false, @@ -333,7 +333,7 @@ {variableEditor} {itemPicker} {pickForField} - password={linkedSecret == argName} + password={linkedSecrets.includes(argName)} extra={formerProperty} {showSchemaExplorer} simpleTooltip={schemaFieldTooltip[argName]} @@ -398,22 +398,24 @@ customErrorMessage={prop?.customErrorMessage} bind:properties={ () => prop?.properties, - (v) => { if (prop) prop.properties = v } + (v) => { + if (prop) prop.properties = v + } } bind:order={ () => prop?.order, - (v) => { if (prop) prop.order = v } + (v) => { + if (prop) prop.order = v + } } nestedRequired={prop?.required} itemsType={prop?.items} - disabled={disabledArgs.includes(argName) || - disabled || - prop?.disabled} + disabled={disabledArgs.includes(argName) || disabled || prop?.disabled} {compact} {variableEditor} {itemPicker} bind:pickForField - password={linkedSecret == argName} + password={linkedSecrets.includes(argName)} extra={prop} {showSchemaExplorer} simpleTooltip={schemaFieldTooltip[argName]} @@ -440,12 +442,14 @@ {#if linkedSecretCandidates?.includes(argName)}
{ if (e.detail === 'secret') { - linkedSecret = argName - } else if (linkedSecret == argName) { - linkedSecret = undefined + if (!linkedSecrets.includes(argName)) { + linkedSecrets = [...linkedSecrets, argName] + } + } else { + linkedSecrets = linkedSecrets.filter((s) => s !== argName) } }} >