mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-05 08:02:18 +00:00
fix: nested template literals in step inputs, and unresolvable $args tags (#10856)
* fix(frontend): keep nested template literals intact in template inputs Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: fail a flow step with an unresolvable $args tag instead of hanging Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): surface input expression errors when running a step test Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): treat an escaped \${ as literal text when escaping backticks Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: accept the string "null" as a tag component, reject only JSON null Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: leave a same_worker step's inert tag alone, log an unresolved flow tag Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): escape every backtick when the template walk desynchronizes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: leave a dedicated runnable's inert step tag alone Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: reroute a step only when its own tag is what failed to resolve Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: name the inert-tag guard step_is_pulled_by_tag Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: reject a tag only when it interpolates to nothing at all Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): validate the template walk instead of trusting a balanced stack Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: describe what an unresolvable tag actually interpolates to Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(frontend): decide template escaping with a real parser, not a hand-rolled scan Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: name is_flow_step on push now that it is load-bearing Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): heal an expression escaped before nested templates were handled Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: reroute a step whose tag reads args that failed to evaluate Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: never hand a job that failed before running to a dedicated runner Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: reroute only a step whose args failed, leave other tags untouched Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: drop the post-preprocessor tag fallback, leaving tag resolution untouched Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: leave interpolate_args exactly as it was Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test: use a generic example in the template literal tests Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): show an expression escaped by the old rule as it was authored Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): surface input expression errors from every step-run entry point Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: state what is_dedicated_worker actually reads Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): heal only text whose backticks were all escaped by the old rule Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): match the old rule textually so an authored backslash still heals Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(frontend): heal only expressions the old rule broke, never ones that parse Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
9213319a74
commit
8f349c032a
@@ -4671,6 +4671,12 @@ pub async fn check_debouncing_within_limits(
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether the tag's queue name is computed from the job's arguments, so that a caller holding
|
||||
/// arguments it could not build knows the tag cannot be built either.
|
||||
pub fn tag_reads_args(tag: &str) -> bool {
|
||||
RE_ARG_TAG.is_match(tag)
|
||||
}
|
||||
|
||||
pub fn interpolate_args(x: String, args: &PushArgs, workspace_id: &str) -> String {
|
||||
// Save this value to avoid parsing twice
|
||||
let workspaced = x.as_str().replace("$workspace", workspace_id).to_string();
|
||||
|
||||
@@ -632,6 +632,28 @@ pub enum JobPayload {
|
||||
},
|
||||
}
|
||||
|
||||
impl JobPayload {
|
||||
/// Whether the payload itself declares a dedicated worker, in which case `push` replaces
|
||||
/// whatever tag it is handed and the caller's tag never reaches the queue.
|
||||
///
|
||||
/// This reads what the payload carries, not what `push` will conclude: a `SingleStepFlow`
|
||||
/// loads the flag from the script row at push time and reports `false` here. That only
|
||||
/// matters to a caller reasoning about the tag, and for those the answer is the same either
|
||||
/// way, since `push` replaces the tag in exactly the case this misses.
|
||||
pub fn is_dedicated_worker(&self) -> bool {
|
||||
let dedicated_worker = match self {
|
||||
JobPayload::ScriptHash { dedicated_worker, .. }
|
||||
| JobPayload::FlowScript { dedicated_worker, .. }
|
||||
| JobPayload::Dependencies { dedicated_worker, .. }
|
||||
| JobPayload::FlowDependencies { dedicated_worker, .. }
|
||||
| JobPayload::Flow { dedicated_worker, .. } => dedicated_worker,
|
||||
JobPayload::Code(raw) => &raw.dedicated_worker,
|
||||
_ => &None,
|
||||
};
|
||||
dedicated_worker.is_some_and(|x| x)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Serialize, Deserialize, Debug)]
|
||||
pub struct SkipHandler {
|
||||
pub path: String,
|
||||
|
||||
@@ -3532,7 +3532,13 @@ pub async fn run_worker(
|
||||
job.kind,
|
||||
JobKind::Script | JobKind::Preview | JobKind::FlowScript
|
||||
) {
|
||||
if !dedicated_workers.is_empty() {
|
||||
// A job carrying a pre-run error never runs its code: it only has to be
|
||||
// pulled so `handle_queued_job` can fail it. Both hand-off paths below
|
||||
// dispatch by path and return before that check, so a job sent down them
|
||||
// would run with whatever arguments survived the failure.
|
||||
let fails_before_running = job.pre_run_error.is_some();
|
||||
|
||||
if !dedicated_workers.is_empty() && !fails_before_running {
|
||||
let dedicated_worker_tx = job.runnable_path.as_ref().and_then(|path| {
|
||||
// For flow steps inside branches/loops, runnable_path includes
|
||||
// nesting segments (e.g. f/flow/branchone-0/a) but the dedicated
|
||||
@@ -3577,7 +3583,7 @@ pub async fn run_worker(
|
||||
NextJob::Http(_) => None,
|
||||
};
|
||||
|
||||
if let Some(flow_runners) = flow_runners {
|
||||
if let Some(flow_runners) = flow_runners.filter(|_| !fails_before_running) {
|
||||
let key_o = job.flow_step_id.as_ref().map(|x| x.to_string());
|
||||
if let Some(key) = key_o {
|
||||
if let Some(flow_runner_tx) = flow_runners.runners.get(&key) {
|
||||
|
||||
@@ -70,9 +70,9 @@ use windmill_queue::schedule::get_schedule_opt;
|
||||
use windmill_queue::{
|
||||
add_completed_job, add_completed_job_error, append_logs, get_mini_pulled_job,
|
||||
insert_concurrency_key_capped, interpolate_args,
|
||||
report_error_to_workspace_handler_or_critical_side_channel, try_schedule_next_job, CanceledBy,
|
||||
FlowRunners, MiniCompletedJob, MiniPulledJob, PushArgs, PushIsolationLevel, SameWorkerPayload,
|
||||
WrappedError,
|
||||
report_error_to_workspace_handler_or_critical_side_channel, tag_reads_args,
|
||||
try_schedule_next_job, CanceledBy, FlowRunners, MiniCompletedJob, MiniPulledJob, PushArgs,
|
||||
PushIsolationLevel, SameWorkerPayload, WrappedError,
|
||||
};
|
||||
|
||||
use windmill_audit::audit_oss::audit_log;
|
||||
@@ -4403,6 +4403,23 @@ async fn push_next_flow_job(
|
||||
payload_tag.tag.as_deref(),
|
||||
);
|
||||
|
||||
// `push_args` is empty once the input transforms failed, so a tag reading `$args[...]`
|
||||
// interpolates to a queue nobody serves and the step sits there instead of reporting
|
||||
// the error. Send it to the flow's tag, which a worker is provably serving right now.
|
||||
//
|
||||
// A step handed over by id, or one whose tag `push` replaces, never reaches a worker
|
||||
// through its tag, so rewriting theirs would be noise.
|
||||
let step_is_pulled_by_tag = !continue_on_same_worker
|
||||
&& !continue_with_runners
|
||||
&& !payload_tag.payload.is_dedicated_worker();
|
||||
let reroute_to_flow_tag =
|
||||
err.is_some() && step_is_pulled_by_tag && tag.as_deref().is_some_and(tag_reads_args);
|
||||
let tag = if reroute_to_flow_tag {
|
||||
Some(flow_job.tag.clone())
|
||||
} else {
|
||||
tag
|
||||
};
|
||||
|
||||
let (email, permissioned_as) = if let Some(on_behalf_of) = payload_tag.on_behalf_of.as_ref()
|
||||
{
|
||||
(&on_behalf_of.email, on_behalf_of.permissioned_as.clone())
|
||||
@@ -4421,8 +4438,7 @@ async fn push_next_flow_job(
|
||||
.as_deref()
|
||||
.filter(|t| !t.is_empty() && *t != flow_job.tag.as_str())
|
||||
{
|
||||
let is_super_admin =
|
||||
windmill_common::auth::is_super_admin_email(db, email).await?;
|
||||
let is_super_admin = windmill_common::auth::is_super_admin_email(db, email).await?;
|
||||
check_tag_available_for_workspace_internal(
|
||||
db,
|
||||
&flow_job.workspace_id,
|
||||
@@ -6155,9 +6171,7 @@ pub async fn script_to_payload(
|
||||
.await?
|
||||
.prefetch_cached(&db)
|
||||
.await?;
|
||||
let on_behalf_of = script_info
|
||||
.on_behalf_of(&flow_job.workspace_id, db)
|
||||
.await?;
|
||||
let on_behalf_of = script_info.on_behalf_of(&flow_job.workspace_id, db).await?;
|
||||
let ScriptHashInfo {
|
||||
tag,
|
||||
cache_ttl,
|
||||
|
||||
Generated
+3
-42
@@ -29,6 +29,7 @@
|
||||
"@windmill-labs/svelte-dnd-action": "^0.9.44",
|
||||
"@xterm/addon-fit": "^0.10.0",
|
||||
"@xyflow/svelte": "^1.0.0",
|
||||
"acorn": "^8.15.0",
|
||||
"ag-charts-community": "^9.0.1",
|
||||
"ag-charts-enterprise": "^9.0.1",
|
||||
"ag-grid-community": "^31.3.4",
|
||||
@@ -1754,7 +1755,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1771,7 +1771,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1788,7 +1787,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1805,7 +1803,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1822,7 +1819,6 @@
|
||||
"cpu": [
|
||||
"arm"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1839,7 +1835,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1856,7 +1851,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1873,7 +1867,6 @@
|
||||
"cpu": [
|
||||
"ppc64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1890,7 +1883,6 @@
|
||||
"cpu": [
|
||||
"s390x"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1907,7 +1899,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1924,7 +1915,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1941,7 +1931,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1958,7 +1947,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -1975,7 +1963,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -7582,7 +7569,7 @@
|
||||
"version": "1.21.7",
|
||||
"resolved": "https://registry.npmjs.org/jiti/-/jiti-1.21.7.tgz",
|
||||
"integrity": "sha512-/imKNG4EbWNrVjoNC/1H5/9GFy+tqjGBHCaSsN+P2RnPqjsLmv6UD3Ej+Kj8nBWaRAwyk7kK5ZUc+OEatnTR3A==",
|
||||
"dev": true,
|
||||
"devOptional": true,
|
||||
"license": "MIT",
|
||||
"bin": {
|
||||
"jiti": "bin/jiti.js"
|
||||
@@ -8278,7 +8265,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -8299,7 +8285,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -8320,7 +8305,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -8341,7 +8325,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -8362,7 +8345,6 @@
|
||||
"cpu": [
|
||||
"arm"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -8383,7 +8365,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -8404,7 +8385,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -8425,7 +8405,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -8446,7 +8425,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -8467,7 +8445,6 @@
|
||||
"cpu": [
|
||||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -8488,7 +8465,6 @@
|
||||
"cpu": [
|
||||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
@@ -13194,21 +13170,6 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/svelte-check/node_modules/picomatch": {
|
||||
"version": "4.0.5",
|
||||
"resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.5.tgz",
|
||||
"integrity": "sha512-RvwwcruNjI1ncT5xRakeyS9Lf8lcItv34KD+aif+VH9kduAyfYBipGh12274xtenIPZ119/R9BdTBa8gAwSh0A==",
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"peer": true,
|
||||
"engines": {
|
||||
"node": ">=12"
|
||||
},
|
||||
"funding": {
|
||||
"url": "https://github.com/sponsors/jonschlinkert"
|
||||
}
|
||||
},
|
||||
"node_modules/svelte-eslint-parser": {
|
||||
"version": "0.43.0",
|
||||
"resolved": "https://registry.npmjs.org/svelte-eslint-parser/-/svelte-eslint-parser-0.43.0.tgz",
|
||||
@@ -13988,7 +13949,7 @@
|
||||
"version": "5.9.3",
|
||||
"resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz",
|
||||
"integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==",
|
||||
"dev": true,
|
||||
"devOptional": true,
|
||||
"license": "Apache-2.0",
|
||||
"bin": {
|
||||
"tsc": "bin/tsc",
|
||||
|
||||
@@ -105,6 +105,7 @@
|
||||
"@windmill-labs/svelte-dnd-action": "^0.9.44",
|
||||
"@xterm/addon-fit": "^0.10.0",
|
||||
"@xyflow/svelte": "^1.0.0",
|
||||
"acorn": "^8.15.0",
|
||||
"ag-charts-community": "^9.0.1",
|
||||
"ag-charts-enterprise": "^9.0.1",
|
||||
"ag-grid-community": "^31.3.4",
|
||||
|
||||
@@ -30,6 +30,7 @@
|
||||
import type { InputTransform } from '$lib/gen'
|
||||
import TemplateEditor from './TemplateEditor.svelte'
|
||||
import { setInputCat as computeInputCat, isCodeInjection } from '$lib/utils'
|
||||
import { escapeTemplateBackticks } from '$lib/utils/templateLiteral'
|
||||
import { FunctionSquare, InfoIcon } from 'lucide-svelte'
|
||||
import { getResourceTypes } from './resourceTypesStore'
|
||||
import type { FlowCopilotContext } from './copilot/flow'
|
||||
@@ -253,7 +254,7 @@
|
||||
arg.expr = getDefaultExpr(
|
||||
argName,
|
||||
previousModuleId,
|
||||
`\`${rawValue.toString().replaceAll('`', '\\`')}\``
|
||||
`\`${escapeTemplateBackticks(rawValue.toString())}\``
|
||||
)
|
||||
arg.type = 'javascript'
|
||||
propertyType = 'static'
|
||||
@@ -687,7 +688,7 @@
|
||||
argName,
|
||||
previousModuleId,
|
||||
staticTemplate
|
||||
? `\`${arg?.value?.toString().replaceAll('`', '\\`') ?? ''}\``
|
||||
? `\`${escapeTemplateBackticks(arg?.value?.toString() ?? '')}\``
|
||||
: arg.value
|
||||
? '(' + JSON.stringify(arg?.value, null, 4) + ')'
|
||||
: ''
|
||||
|
||||
@@ -50,14 +50,19 @@
|
||||
let jobProgressReset: () => void = () => {}
|
||||
let stepHistoryLoader = getStepHistoryLoaderContext()
|
||||
|
||||
// Every explicit run re-evaluates the args with errors surfaced. The reactive evaluations
|
||||
// that follow each flow edit stay quiet, so without this a failing expression is silently
|
||||
// `undefined` in what the run is built from. Manually edited args are preserved across the
|
||||
// refresh by `initializeFromSchema`.
|
||||
export function runTestWithStepArgs() {
|
||||
const args = stepsInputArgs.getStepArgs(mod.id)
|
||||
runTest(args)
|
||||
}
|
||||
|
||||
export function loadArgsAndRunTest() {
|
||||
stepsInputArgs?.updateStepArgs(mod.id, flowStateStore.val, flowStore?.val, previewArgs?.val)
|
||||
runTestWithStepArgs()
|
||||
stepsInputArgs?.updateStepArgs(
|
||||
mod.id,
|
||||
flowStateStore.val,
|
||||
flowStore?.val,
|
||||
previewArgs?.val,
|
||||
true
|
||||
)
|
||||
runTest(stepsInputArgs.getStepArgs(mod.id))
|
||||
}
|
||||
|
||||
// A step's timeout is an InputTransform. Only a static numeric value can be applied
|
||||
|
||||
@@ -19,6 +19,7 @@
|
||||
import { computeGlobalContext, eval_like } from './eval'
|
||||
import { deepEqual } from 'fast-equals'
|
||||
import { deepMergeWithPriority, isCodeInjection, readFieldsRecursively } from '$lib/utils'
|
||||
import { escapeTemplateBackticks } from '$lib/utils/templateLiteral'
|
||||
import sum from 'hash-sum'
|
||||
import { createDispatcherIfMounted } from '$lib/createDispatcherIfMounted'
|
||||
|
||||
@@ -271,7 +272,7 @@
|
||||
if ((input.type === 'template' || input.type == 'templatev2') && isCodeInjection(input.eval)) {
|
||||
try {
|
||||
const r = await eval_like(
|
||||
'`' + input.eval.replaceAll('`', '\\`') + '`',
|
||||
'`' + escapeTemplateBackticks(input.eval) + '`',
|
||||
computeGlobalContext($worldStore, id, fullContext),
|
||||
$stateStore,
|
||||
$mode == 'dnd',
|
||||
|
||||
@@ -464,7 +464,7 @@
|
||||
btnClasses="px-1 py-1.5 bg-surface"
|
||||
on:click={() => {
|
||||
outputPicker?.toggleOpen(true)
|
||||
moduleTest?.loadArgsAndRunTest()
|
||||
moduleTest?.runTestWithStepArgs()
|
||||
}}
|
||||
dropdownItems={[
|
||||
{
|
||||
|
||||
@@ -125,12 +125,15 @@ export class StepsInputArgs {
|
||||
initializeFromSchema(
|
||||
mod: FlowModule,
|
||||
schema: { properties?: Record<string, any> },
|
||||
pickableProperties: PickableProperties | undefined
|
||||
pickableProperties: PickableProperties | undefined,
|
||||
// Off for the reactive re-evaluations that follow every flow edit; on for an explicit
|
||||
// run, where a failing expression would otherwise become `undefined` unseen.
|
||||
showError: boolean = false
|
||||
) {
|
||||
const args = Object.fromEntries(
|
||||
Object.keys(schema.properties ?? {}).map((k) => [
|
||||
k,
|
||||
evalValue(k, mod, pickableProperties, false)
|
||||
evalValue(k, mod, pickableProperties, showError)
|
||||
])
|
||||
)
|
||||
|
||||
@@ -158,14 +161,16 @@ export class StepsInputArgs {
|
||||
id: string,
|
||||
flowState: FlowState | undefined,
|
||||
flow: OpenFlow | undefined,
|
||||
previewArgs: Record<string, any> | undefined
|
||||
previewArgs: Record<string, any> | undefined,
|
||||
showError: boolean = false
|
||||
) {
|
||||
if (id === 'failure' && flow && flow.value.failure_module && flowState) {
|
||||
const picker = getFailureStepPropPicker(flowState, flow, previewArgs)
|
||||
this.initializeFromSchema(
|
||||
flow.value.failure_module,
|
||||
flowState['failure']?.schema ?? {},
|
||||
picker.pickableProperties
|
||||
picker.pickableProperties,
|
||||
showError
|
||||
)
|
||||
return
|
||||
}
|
||||
@@ -193,7 +198,12 @@ export class StepsInputArgs {
|
||||
false
|
||||
)
|
||||
const pickableProperties = stepPropPicker.pickableProperties
|
||||
this.initializeFromSchema(modules[0], flowState[id]?.schema ?? {}, pickableProperties)
|
||||
this.initializeFromSchema(
|
||||
modules[0],
|
||||
flowState[id]?.schema ?? {},
|
||||
pickableProperties,
|
||||
showError
|
||||
)
|
||||
}
|
||||
|
||||
removeExtraKey(moduleId: string, keys: string[]) {
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
} from '$lib/gen'
|
||||
import { workspaceStore } from '$lib/stores'
|
||||
import { cleanExpr, emptySchema } from '$lib/utils'
|
||||
import { unescapeTemplateBackticks } from '$lib/utils/templateLiteral'
|
||||
import { get } from 'svelte/store'
|
||||
import type { FlowModuleState } from './flowState'
|
||||
import { type PickableProperties, dfs } from './previousResults'
|
||||
@@ -219,7 +220,7 @@ export function codeToStaticTemplate(code?: string): string | undefined {
|
||||
if (lines.length == 1) {
|
||||
const line = lines[0].trim()
|
||||
if (line[0] == '`' && line.charAt(line.length - 1) == '`') {
|
||||
return line.slice(1, line.length - 1).replaceAll('\\`', '`')
|
||||
return unescapeTemplateBackticks(line.slice(1, line.length - 1))
|
||||
} else {
|
||||
return `\$\{${line}\}`
|
||||
}
|
||||
|
||||
@@ -0,0 +1,133 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { escapeTemplateBackticks, unescapeTemplateBackticks } from './templateLiteral'
|
||||
|
||||
// Template mode stores its value as a JS template literal, so backticks in the text have to be
|
||||
// escaped. Escaping them inside `${...}` too is what broke nested template literals: a backslash
|
||||
// is a syntax error in expression position.
|
||||
describe('escapeTemplateBackticks', () => {
|
||||
const nested =
|
||||
'--input ${flow_input.iter.value}${results.config ? ` --config ${results.config}` : ""} --host ${flow_input.hostname}'
|
||||
|
||||
it('leaves a nested template literal inside an interpolation intact', () => {
|
||||
const expr = '`' + escapeTemplateBackticks(nested) + '`'
|
||||
expect(expr).not.toContain('\\`')
|
||||
expect(
|
||||
new Function('flow_input', 'results', 'return ' + expr)(
|
||||
{ iter: { value: 'data.csv' }, hostname: 'host1' },
|
||||
{ config: '/tmp/cfg.json' }
|
||||
)
|
||||
).toBe('--input data.csv --config /tmp/cfg.json --host host1')
|
||||
})
|
||||
|
||||
it('still escapes a backtick in the literal text', () => {
|
||||
expect(escapeTemplateBackticks('a ` b')).toBe('a \\` b')
|
||||
expect(escapeTemplateBackticks('a ` ${x} ` b')).toBe('a \\` ${x} \\` b')
|
||||
})
|
||||
|
||||
it('leaves an escaped interpolation as literal text', () => {
|
||||
// `\\${...}` is escaped in the template source, so the backticks inside it are literal
|
||||
// text and still need escaping.
|
||||
expect(escapeTemplateBackticks('\\${foo `bar`}')).toBe('\\${foo \\`bar\\`}')
|
||||
expect(unescapeTemplateBackticks('\\${foo \\`bar\\`}')).toBe('\\${foo `bar`}')
|
||||
expect(
|
||||
() => new Function('return `' + escapeTemplateBackticks('\\${foo `bar`}') + '`')
|
||||
).not.toThrow()
|
||||
})
|
||||
|
||||
// Braces, quotes, regex literals and comments all hide backticks and braces from anything
|
||||
// short of a real lexer, which is why the parser decides.
|
||||
it('handles text only a lexer can read correctly', () => {
|
||||
const inputs = [
|
||||
'${ x["}"] } `',
|
||||
"${ f({ a: '`' }) } `",
|
||||
"${ x.replace(/'/g, '') } `",
|
||||
'${ /* ` */ x } `',
|
||||
'{"match": ${/{/.test(flow_input.x)}, "literal": "`x`"}'
|
||||
]
|
||||
for (const v of inputs) {
|
||||
expect(() => new Function('return `' + escapeTemplateBackticks(v) + '`')).not.toThrow()
|
||||
expect(unescapeTemplateBackticks(escapeTemplateBackticks(v))).toBe(v)
|
||||
}
|
||||
})
|
||||
|
||||
// The failure that matters most is not a syntax error but literal text quietly becoming code:
|
||||
// here the author's `+ flow_input.y +` must stay text rather than being evaluated.
|
||||
it('never lets literal text escape into the expression', () => {
|
||||
const v = '{"match": ${/{/.test(flow_input.x)}, "literal": "` + flow_input.y + `"}'
|
||||
const evaluated = new Function('flow_input', 'return `' + escapeTemplateBackticks(v) + '`')({
|
||||
x: 'x',
|
||||
y: 'LEAKED'
|
||||
})
|
||||
expect(evaluated).not.toContain('LEAKED')
|
||||
expect(evaluated).toContain('` + flow_input.y + `')
|
||||
})
|
||||
|
||||
// An expression the old blanket rule broke — it escaped backticks inside `${...}`, which
|
||||
// does not parse — comes back as the author typed it, instead of showing the backslashes
|
||||
// and escaping them one deeper on every save.
|
||||
it('heals an expression the old rule broke', () => {
|
||||
const broken = '-p ${a}${b ? \\` --x ${c}\\` : ""}'
|
||||
const clean = '-p ${a}${b ? ` --x ${c}` : ""}'
|
||||
expect(unescapeTemplateBackticks(broken)).toBe(clean)
|
||||
expect(escapeTemplateBackticks(clean)).toBe(clean)
|
||||
})
|
||||
|
||||
// A text that already parses is left alone: an over-escaped legacy value and a backslash the
|
||||
// author wrote are the same bytes, so healing on looks alone would drop a real character.
|
||||
it('leaves an expression that already parses alone', () => {
|
||||
const run = (body: string) => new Function('return `' + body + '`')()
|
||||
for (const stored of ['${"\\`"}', '${"a\\\\`"}']) {
|
||||
expect(unescapeTemplateBackticks(stored)).toBe(stored)
|
||||
expect(run(escapeTemplateBackticks(unescapeTemplateBackticks(stored)))).toBe(run(stored))
|
||||
}
|
||||
})
|
||||
|
||||
// ...but a backtick escaped inside a nested template belongs there and must survive.
|
||||
it('leaves an escaped backtick that is inside a nested template alone', () => {
|
||||
const stored = '${cond ? `a\\`b` : ""}'
|
||||
expect(unescapeTemplateBackticks(stored)).toBe(stored)
|
||||
})
|
||||
|
||||
// Escapes the author wrote inside a nested template are not the old rule's doing, and
|
||||
// stripping them changes what the expression means — here into chained tagged templates,
|
||||
// which throw. Only a text whose backticks are *all* escaped came from the old rule.
|
||||
it('leaves escapes that belong to a nested template alone', () => {
|
||||
const run = (body: string) => new Function('flag', 'value', 'return `' + body + '`')(true, 'X')
|
||||
for (const stored of ['${flag ? `\\`\\`${value}\\`\\`` : ""}', '${flag ? `a\\`b` : ""}']) {
|
||||
expect(unescapeTemplateBackticks(stored)).toBe(stored)
|
||||
expect(escapeTemplateBackticks(unescapeTemplateBackticks(stored))).toBe(stored)
|
||||
expect(run(stored)).toBe(run(escapeTemplateBackticks(unescapeTemplateBackticks(stored))))
|
||||
}
|
||||
})
|
||||
|
||||
it('round-trips through unescapeTemplateBackticks', () => {
|
||||
for (const v of [
|
||||
nested,
|
||||
'a ` b',
|
||||
'${ x["}"] } `',
|
||||
'plain',
|
||||
'${a}${b}',
|
||||
'\\${a}',
|
||||
'${cond ? `a\\`b` : ""}'
|
||||
]) {
|
||||
expect(unescapeTemplateBackticks(escapeTemplateBackticks(v))).toBe(v)
|
||||
}
|
||||
})
|
||||
// The guarantee that matters: opening a flow in the editor and saving it back must not change
|
||||
// the stored expression, including for a value that only the all-or-nothing fallback can
|
||||
// handle.
|
||||
it('never rewrites the stored expression on a view/save cycle', () => {
|
||||
const inputs = [
|
||||
'{"match": ${/{/.test(flow_input.x)}, "literal": "`x`"}',
|
||||
'${cond ? `a\\`b` : ""}',
|
||||
"${ x.replace(/'/g, '') } `",
|
||||
'a ` b',
|
||||
'${ /* ` */ x } `'
|
||||
]
|
||||
for (const v of inputs) {
|
||||
const stored = escapeTemplateBackticks(v)
|
||||
expect(() => new Function('return `' + stored + '`')).not.toThrow()
|
||||
expect(escapeTemplateBackticks(unescapeTemplateBackticks(stored))).toBe(stored)
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,59 @@
|
||||
import { parseExpressionAt } from 'acorn'
|
||||
|
||||
/**
|
||||
* Template mode stores what the author typed as a JS template literal, so the text is spliced
|
||||
* between backticks. A backtick in the literal part has to be escaped or it ends the literal
|
||||
* early — but one inside a `${...}` must not be, since a backslash is a syntax error in
|
||||
* expression position and a nested template literal there is legitimate.
|
||||
*
|
||||
* Telling those apart means knowing where each `${...}` ends, which needs a real JS lexer:
|
||||
* regex literals, comments and nested templates all hide braces and backticks from anything
|
||||
* simpler. So rather than escaping selectively, ask the parser whether the text already reads as
|
||||
* one template literal. If it does, it needs no escaping at all; if it does not, escape every
|
||||
* backtick, which is what this did before nested templates were supported.
|
||||
*
|
||||
* Known limitation: a value mixing a bare literal backtick with a nested template cannot be
|
||||
* expressed either way, and gets the all-or-nothing fallback. Escaping the literal one by hand
|
||||
* makes the whole value parse and it is then kept verbatim.
|
||||
*/
|
||||
function isCompleteTemplateBody(text: string): boolean {
|
||||
const source = '`' + text + '`'
|
||||
try {
|
||||
const node = parseExpressionAt(source, 0, { ecmaVersion: 'latest' })
|
||||
// The type is what rejects a body that closes its own literal early: `` ` + evil() + ` ``
|
||||
// parses, but as a concatenation, and would evaluate the author's literal text. The span
|
||||
// rejects a body that stops short, like `` `a` x ``.
|
||||
return node.type === 'TemplateLiteral' && node.start === 0 && node.end === source.length
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
/** Escape `text` so it can be wrapped in backticks and mean what the author typed. */
|
||||
export function escapeTemplateBackticks(text: string): string {
|
||||
// No backtick means nothing to escape and nothing to decide, which is every ordinary value.
|
||||
if (!text.includes('`')) {
|
||||
return text
|
||||
}
|
||||
return isCompleteTemplateBody(text) ? text : text.replaceAll('`', '\\`')
|
||||
}
|
||||
|
||||
/** Inverse of {@link escapeTemplateBackticks}, for turning an expression back into a template. */
|
||||
export function unescapeTemplateBackticks(text: string): string {
|
||||
if (!text.includes('`')) {
|
||||
return text
|
||||
}
|
||||
const unescaped = text.replaceAll('\\`', '`')
|
||||
if (escapeTemplateBackticks(unescaped) === text) {
|
||||
return unescaped
|
||||
}
|
||||
// Only an expression the old blanket rule *broke* is healed: it escaped backticks inside
|
||||
// `${...}` too, which does not parse, while the unescaped form does. A text that already
|
||||
// parses is left alone even if it looks over-escaped, because the two are indistinguishable
|
||||
// from the text alone and guessing changes what the expression means — `${"a\\\\`"}` is a
|
||||
// backslash the author wrote, not one the old rule added.
|
||||
if (!isCompleteTemplateBody(text) && isCompleteTemplateBody(unescaped)) {
|
||||
return unescaped
|
||||
}
|
||||
return text
|
||||
}
|
||||
Reference in New Issue
Block a user