fix: reject prompt variables in the reserved underscore namespace - #14450
fix: reject prompt variables in the reserved underscore namespace#14450tarciorodrigues wants to merge 15 commits into
Conversation
Keys prefixed with an underscore are node-template metadata (_type,
_frontend_node_flow_id, ...), not component fields. The frontend filters that
namespace out of every render path, so a variable such as {_x} was accepted by
validate_prompt and written into the template, but never produced an input field
or a handle -- it could not be given a value and resolved to an empty string at
run time with no warning.
Reject the name instead, the same way the seven names in _INVALID_NAMES are
already rejected for colliding with template keys.
Refs LE-2144
Parametrized over the names reported in LE-2144 (_x, _, __y) plus the two template
metadata keys that used to fail with an opaque HTTP 500 (_type,
_frontend_node_flow_id), against controls that must keep working (var, a_b, var_1,
private_).
test_underscore_variables_accepted asserted that {{_private}} was accepted. It was
written to cover underscores in variable names generally, not to settle whether a
leading underscore should be allowed -- the case now uses a trailing underscore, which
is the convention for escaping a reserved name and is unaffected by this change.
Refs LE-2144
Single source of truth for the frontend side of the rule, mirroring RESERVED_VARIABLE_PREFIX in lfx. Every place that highlights or lists a prompt variable reads from here, so the editor and the API cannot drift apart -- three independent extractors already disagreed about this namespace. The invalid style uses a wavy red underline over the existing accent-red-foreground token, which is already tuned for light and dark, instead of introducing a background colour of its own. Refs LE-2144
…t editor A name starting with an underscore now renders with a wavy red underline in the template, and its badge turns red with a tooltip explaining the rule. The user sees the problem on the offending characters while typing, instead of finding out only when Check & Save fails. varHighlightHTML takes the bare identifier separately from the rendered text, because mustache renders the braces inside the span. The tooltip is only ever set from an i18n constant, never from user input, so a quote inside a variable name cannot break the attribute. Refs LE-2144
Same treatment as the f-string editor. The mustache highlighter matched on the whole
{{name}} run, so the bare identifier is now captured from the regex group and passed
separately -- the rendered text keeps its braces while the rule reads the name.
Refs LE-2144
The template preview rendered on the node has its own highlighter, so without this the
red marking would disappear as soon as the editor modal closed. Both preview
components now read the same predicate as the editors.
The mustache preview test asserted the regular highlight class for {{_private}}; it now
asserts the invalid one, plus a new case pinning that an underscore anywhere other than
the first character stays valid.
Refs LE-2144
…itor regexHighlight captures four groups -- the code fence, then the opening brace run, the name and the closing brace run. The editor's callback omitted the fence parameter, so every capture was shifted by one: lenOpen read the (undefined) fence and was always 0, isVariable was never true, and no variable had ever been highlighted inside the modal. The node preview, which destructures the same regex correctly, did highlight -- which is why the gap went unnoticed. Restoring it is a prerequisite for the red marking to be visible where the user types. Code fences are now skipped explicitly, as the preview already does. Adds unit tests pinning the regex group order and the markup emitted for reserved and regular names. Refs LE-2144
…review This is the highlighter the user actually sees: parameterRenderComponent renders AccordionPromptComponent whenever the inspection panel is enabled, and it has its own copy of the highlight logic in prompt-highlight.ts. Without this the red marking only reached the legacy preview path. Both branches -- f-string and double brackets -- now read the shared predicate, and the double-bracket branch captures the bare name from its regex group instead of only the whole match. Refs LE-2144
The frontend renders validation errors through react-markdown, where bare underscores pair up into emphasis markers and vanish: the message reached the user as "Invalid input variables: x. Variable names cannot start with '' ..." -- naming neither the offending variable nor the rule. Wrapping the names and the prefix in backticks keeps them literal in the toast and reads fine everywhere else. Covered by a test so the escaping is not dropped later. Refs LE-2144
…itor
The editor passed only the bare name to varHighlightHTML, so a restored highlight
rendered "Hello _x" instead of "Hello {_x}" -- the literal variable was gone from the
preview. The surrounding `literal` string only carries the extra braces of an escaped
run, not the variable's own pair. The node preview always rendered them; the editor now
matches.
Refs LE-2144
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe change reserves the ChangesReserved Prompt Variables
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant PromptModal
participant variableHighlightClass
participant varHighlightHTML
participant PromptBadge
PromptModal->>variableHighlightClass: classify variable name
variableHighlightClass-->>PromptModal: invalid or regular CSS class
PromptModal->>varHighlightHTML: pass variableName and invalidTitle
varHighlightHTML-->>PromptModal: highlighted preview HTML
PromptModal->>PromptBadge: render reserved-variable state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 6 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (6 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/frontend/src/modals/promptModal/utils/var-highlight-html.tsx`:
- Around line 14-16: Update the title construction in the var-highlight HTML
utility to HTML-escape invalidTitle before interpolating it into the
double-quoted attribute, preserving the existing conditional behavior. Use the
project’s established escaping utility if available, and add a test covering a
title containing double quotes to verify the generated attribute remains valid.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f8e4dbb7-6339-446f-a6b8-0cf1941b14fa
📒 Files selected for processing (24)
src/backend/tests/unit/components/prompts/test_validate_prompt_mustache.pysrc/backend/tests/unit/components/prompts/test_validate_prompt_reserved_prefix.pysrc/frontend/src/components/core/parameterRenderComponent/components/accordionPromptComponent/helpers/__tests__/prompt-highlight.test.tssrc/frontend/src/components/core/parameterRenderComponent/components/accordionPromptComponent/helpers/prompt-highlight.tssrc/frontend/src/components/core/parameterRenderComponent/components/mustachePromptComponent/__tests__/mustachePromptComponent.test.tsxsrc/frontend/src/components/core/parameterRenderComponent/components/mustachePromptComponent/index.tsxsrc/frontend/src/components/core/parameterRenderComponent/components/promptComponent/__tests__/promptComponent.test.tsxsrc/frontend/src/components/core/parameterRenderComponent/components/promptComponent/index.tsxsrc/frontend/src/locales/de.jsonsrc/frontend/src/locales/en.jsonsrc/frontend/src/locales/es.jsonsrc/frontend/src/locales/fr.jsonsrc/frontend/src/locales/ja.jsonsrc/frontend/src/locales/pt.jsonsrc/frontend/src/locales/zh-Hans.jsonsrc/frontend/src/modals/mustachePromptModal/index.tsxsrc/frontend/src/modals/promptModal/index.tsxsrc/frontend/src/modals/promptModal/utils/__tests__/var-highlight-html.test.tssrc/frontend/src/modals/promptModal/utils/var-highlight-html.tsxsrc/frontend/src/style/applies.csssrc/frontend/src/types/components/index.tssrc/frontend/src/utils/__tests__/promptVariables.test.tssrc/frontend/src/utils/promptVariables.tssrc/lfx/src/lfx/base/prompts/api_utils.py
…ibute en, pt and es quote the reserved prefix with a plain double quote. Interpolated raw into title="...", that quote closed the attribute: the browser read only "Variable names can't start with " and scattered the rest of the sentence into stray attributes, so the tooltip never stated the rule it exists to state. Escaping the value keeps the sentence whole in every locale. The regression test asserts what the DOM reads back, not the generated string, so an unescaped character shows up as a broken tooltip instead of passing on a substring match. Refs LE-2144
github-actions
Bot
added
bug
autofix.ci flagged one unformatted render() call in the file added by da24fa8. Typing defaultProps as InputProps<string, PromptAreaComponentType> instead of `as any` also clears the noExplicitAny the same file introduced; the remaining one in promptModal/index.tsx predates this branch. Refs LE-2144
Cristhianzl
left a comment
Cristhianzl
left a comment
There was a problem hiding this comment.
⚠️ Important (preferably this PR)
I1 — as any in the new test file (banned pattern in this repo)
File: src/frontend/src/components/core/parameterRenderComponent/components/promptComponent/__tests__/promptComponent.test.tsx (the defaultProps declaration)
Issue: The new test builds its props object with } as any;. The repo baseline (.claude/CLAUDE.md § Code style) bans : any / as any outright, tests included, and the sibling test files in this PR get by without it.
Why it matters: as any silences the compiler on the whole props object — if PromptAreaComponent's props change shape, this test keeps compiling and fails at runtime (or worse, silently tests the wrong contract).
Suggested fix: Type it against the component's props (InputProps<string, PromptAreaComponentType>) and fill only what the test needs, e.g. satisfies Partial<...> plus a typed spread, or reuse whatever pattern mustachePromptComponent.test.tsx uses for its defaultProps.
Code reference
const defaultProps = {
field_name: "template",
...
readonly: false,
} as any;I2 — CI is red: autofix wants to reformat the new test and the matrix was cancelled
File: same test file as I1 (formatting only)
Issue: The autofix job failed emitting a Prettier/Biome diff for promptComponent.test.tsx, and most of the remaining jobs (Ruff style, backend matrix, several bundle jobs) ended CANCELLED, so the PR currently has no green test signal at all.
Why it matters: The PR adds five test files across backend and frontend; none of them have run in CI yet. Nothing can be merged on a cancelled matrix.
Suggested fix: Run make format_frontend locally (fixing I1 in the same pass), push, and let the full matrix run.
💡 Recommended (can ship as a follow-up)
R1 — The expected rejection surfaces as HTTP 500
File: src/backend/base/langflow/api/v1/validate.py:61-62
Issue: post_validate_prompt wraps every exception — including the new, fully expected ValueError for a reserved name — in HTTPException(status_code=500). This is pre-existing behavior (it is exactly how the old _type crash surfaced), but this PR turns the 500 path into a routine user-facing outcome: every Check & Save on {_x} now logs as a server error.
Why it matters: 5xx means "server fault" (repo rules/api.md); dashboards and alerting on 5xx rates will count user typos as outages, and clients cannot distinguish "your input is invalid" from "the server broke".
Suggested fix: Follow-up PR: except ValueError as e: raise HTTPException(status_code=400, detail=str(e)) before the generic handler, keeping the message unchanged. Worth checking that the frontend toast reads detail the same way for 400 as for 500.
Code reference
except Exception as e:
raise HTTPException(status_code=500, detail=str(e)) from eR2 — The badge list is now duplicated verbatim across both modals
Files: src/frontend/src/modals/promptModal/index.tsx and src/frontend/src/modals/mustachePromptModal/index.tsx (the wordsHighlight map)
Issue: The ~35-line ShadTooltip + Badge block — including the new reserved branch, errorStatic variant, test ids, and the 59/56-char truncation — is character-for-character identical in the two modals. The duplication predates this PR, but the PR doubles its size and gives it behavior (the reserved branch) that must now be kept in sync by hand.
Why it matters: DRY threshold (5+ lines × 2 occurrences) is well past; the next change to the reserved-name UX has to be made twice or the two editors drift.
Suggested fix: Extract a PromptVariableBadge (or a PromptVariablesList) component beside promptVariables.ts and use it in both modals. Fine as a follow-up.
📝 Nice-to-have
N1 — Spaced mustache tags get no inline warning
{{ _x }} (with spaces) is a valid mustache tag — mustache_template_vars trims to _x, so the backend rejects it on Check & Save — but the highlight regex /\{\{([a-zA-Z_][a-zA-Z0-9_]*)\}\}/ and the badge list don't match it, so the user gets no red underline or badge while typing. Pre-existing limitation of the highlighter (spaced tags were never highlighted), so the failure mode is "no early warning", not "wrong warning". A follow-up could allow optional whitespace in the highlight regexes.
N2 — key={index} carried through the badge rewrite
Both modals still key the tooltip and the badge by array index (the inner key={index} on Badge is redundant — only the outermost element of the map needs a key). Since the rewrite touched these lines anyway, keying by `${variableName}-${index}` and dropping the inner key would be a free cleanup.
The inline marking added earlier in this series knew only the reserved `_`
prefix, so `1var`, `my var` and `code` -- all refused by Check & Save -- kept
the visual treatment of a valid name. One family of rejections, two opposite
treatments in the editor.
`invalidVariableReason` now mirrors the four rejection paths of
`validate_prompt` in the order the backend applies them (leading digit,
invalid character, reserved prefix, reserved name), and each reports its own
message, so the reason shown while typing is the one Check & Save would give.
`promptVariableFieldName` mirrors Python's `string.Formatter`, which ends the
field name at the first `!` or `:`. Without it `{x:>10}` and the JSON literal
`{"a": 1}` -- both accepted by the backend -- would be marked invalid, turning
the fix into a false positive on templates that work today.
The footer hint no longer promises "any chosen name", which the marking
contradicts.
Refs LE-2144
github-actions
Bot
added
bug
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-1.12.0 #14450 +/- ##
==================================================
+ Coverage 61.51% 63.55% +2.04%
==================================================
Files 2408 2395 -13
Lines 240471 244232 +3761
Branches 36217 37607 +1390
==================================================
+ Hits 147914 155225 +7311
+ Misses 90617 87056 -3561
- Partials 1940 1951 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
`Lint Frontend / Run Biome` lints the files changed in the PR, so the `(document as any).caretPositionFromPoint(...)` in the prompt editor -- older than this series, but inside a file the PR touches -- failed the job with `lint/suspicious/noExplicitAny`. The cast is no longer needed: the DOM lib types `caretPositionFromPoint` on `Document`, so the call type-checks as written. The runtime guard stays, since Safari still does not implement it. The hand-written `DocumentWithCaretPosition` shim in the double-bracket editor goes with it -- it declared its own `CaretPosition` with only `offset`, which conflicted with the lib's and was reported as TS2430. Refs LE-2144
github-actions
Bot
added
bug
|
Extending the marking — the first of your two outcomes. Two commits.
|
| Name | Reason reported | f-string {x} |
mustache {{x}} |
Check & Save |
|---|---|---|---|---|
_x |
reserved prefix | red + tooltip + red badge | same | refuses |
1var |
leading digit | red + tooltip + red badge | not recognized as a variable | refuses |
my var |
invalid character | red + tooltip, no badge | not recognized as a variable | refuses |
code |
reserved name | red + tooltip + red badge | same | refuses |
var a_b var_1 |
— | untouched | untouched | accepts |
Two rows do not end up identical to the others, both for reasons older than this series:
my vargets no badge — and never got a gray one either.checkVariablesdrops any name containing an invalid character from the badge row. What it did have was the indigo highlight of a valid name, and that is what changed. Removing the filter would put JSON , so I left it alone.- **mustache does not recognize1varorm.**SIMPLE_VARIABLE_PATTERNis[a-zA-Z_][a-zA-Z0-9_]*, so there is no spanothing was scoped out, there is nothing tomark. Widening that regex changes what counte, which is larger than this ticket. Bothnames still refuse at Check & Save, with thesage._xandcodeare marked in both syntaxes:<img width="740" height="84" alt="03-editor-src="https://github.com/user-attachments/ass1-f563fba4a623" />The same predicate drives the preview on theore this fix shows the name as rejectedinstead of as a working variable:<img width="480" height="382" alt="04-node-psrc="https://github.com/user-attachments/ass4-505370d6b2f9" /> One detail is worth flagging because it is not obvious from the diff: the predicate reads the **field name**, the raw text between the braces. f-string exon'sstring.Formatter, which ends the fieldname at the first!or:— so{x:>10}{"a": 1}is the field"a", both accepted bythe backend today. Reading the raw text woul JSON literal in every existing prompt asinvalid, a worse failure than the one being dNamemirrors that cut, the tests pin{x:>10},{x!r}and{"a": 1}as valid, the first screenshot.On the minor: the footer hint no longer prom now reads *"Prompt variables can be createdinside curly brackets, e.g. {variable_name}.number or "_", and can't contain spaces orpunctuation"*, translated in all seven localve.The controls are unchanged end to end:{{va_1}}still highlight normally, Check & Savestill succeeds, and the node still gets threandles.<img width="420" height="377" alt="05-controsrc="https://github.com/user-attachments/ass0-116d895bdf2b" />###fix(prompts): drop the caret-position alintlints the files changed in the PR, so the(document as any).caretPositionFromPoint(...)inprompthan this series, but in a file the PR touches— failed the job withlint/suspicious/noExp needed: the DOM lib types the call. It isgone, and so is the hand-writtenDocumentWihe double-bracket editor, which declared itsownCaretPositionwith onlyoffsetand w runtime guard stays, since Safari still doesnot implement the API.Gates: frontend 5912/5912 in 524 suites s/unit/components/prompts/` 49/49 · biomeclean on all 21 frontend files of the diff, CI job runs · no new tsc errors. Everymessage above was read back from the DOM witan from the generated string, which is how the earlier quote-escaping bug was caught.
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
github-actions
Bot
added
bug
2 participants
Refs LE-2144
Why
In the Prompt Template component a variable named
{_x}was accepted byvalidate_promptand written into the node template, but never rendered. Keys prefixed with an underscore are template metadata —_type,_frontend_node_flow_id,_frontend_node_folder_id— and the frontend filters that namespace out of every render path, so no input field and no handle were ever created. The variable could not be given a value by typing or by connecting an edge, and resolved to an empty string at run time with a success toast and no warning:'Hello {_x}, how are you?'produced'Hello , how are you?'.Seven names are already rejected for exactly this reason —
code,input_variables,output_parser,partial_variables,template,template_format,validate_template. The underscore prefix is the rest of that same family and had been left out.Rejecting the name also closes a second symptom that was never reported:
{_type}collides with a plain metadata string, soadd_new_variables_to_templatereadtemplate["_type"]["value"]on astrand the request failed withHTTP 500: string indices must be integers, not 'str'. It now fails validation up front with an actionable message.What
Backend.
validate_promptrejects any variable name starting with_, in both the f-string and the mustache path, naming only the offending variables. The names and the prefix are wrapped in backticks because the frontend renders this message through react-markdown, where bare underscores pair up into emphasis markers and disappear.Frontend. A single predicate,
isReservedVariableName, mirrors the backend rule and drives every place a prompt variable is highlighted or listed. A reserved name renders with a wavy red underline over the existingaccent-red-foregroundtoken, tuned for both themes, and its badge turns red with a tooltip naming the rule. The user sees the problem on the offending characters while typing, rather than only when Check & Save fails.The rule is spelled two ways on purpose, because it reaches the user through two different renderers. The backend message goes through react-markdown, where a bare underscore is an emphasis marker, so the names and the prefix are backticked and arrive literal. The editor tooltip is a plain
titleattribute, which markdown never touches, so it quotes the prefix instead. Same rule, two spellings — matching them would break one of the two surfaces. The tooltip text is HTML-escaped before it enters the attribute, so the plain double quote that en, pt and es use around the prefix cannot close it early.Two pre-existing defects had to be fixed for that to be visible:
regexHighlightcaptures four groups — the code fence, the opening brace run, the name, the closing run. The f-string editor's callback omitted the fence parameter, shifting every capture by one, solenOpenwas always 0 and nothing had ever been highlighted inside the modal. The node preview destructures the same regex correctly, which is why the gap went unnoticed.parameterRenderComponentrendersAccordionPromptComponentwhenever the inspection panel is enabled, and it holds its own copy of the highlight logic inprompt-highlight.ts. Without it the marking only reached the legacy preview path.Existing flows are unaffected: the three call sites in
prompt.pyalready catchValueErrorand degrade silently, so a saved flow containing{_x}still opens.How to validate
Hello {_x}, meet {var}.On the node,{_x}carries a wavy red underline while{var}keeps the regular highlight._xbadge is red and thevarbadge is unchanged.Invalid input variables: _x. Variable names cannot start with _ because that prefix is reserved for internal template fields.Before this change the same template saved successfully and produced a node with no field and no handle.Hello {_type}!and click Check & Save. The same message appears. Before this change the request failed with500: string indices must be integers, not 'str'.{var},{a_b},{var_1}and{private_}are unaffected in both syntaxes: they still create their field and handle. Turn on Use Double Brackets and repeat with{{_x}}for the same result as S1.Check both themes and confirm the console reports no new error.
Screenshots
S1 — the node marks the reserved name, and only that one
`{_x}` is indistinguishable from a working variable
`{_x}` underlined in red, `{var}` untouched
Detail: the marking sits on the offending characters, next to a normally highlighted variable
S2 — the badge separates the two cases
Grey badge, identical to a valid name
Red `_x` badge beside the unchanged `var` badge
S3 — the tooltip names the rule
Hovering the red badge explains why the name is refused, before Check & Save is clicked
S4 — Check & Save refuses instead of reporting success
"Prompt is ready", and the node has no field and no handle
Refused, naming the variable and the rule
S5 —
{_type}no longer reaches the template writer`500: string indices must be integers, not 'str'`
Same actionable message as every other reserved name
S6 — the red stays legible in dark mode
Marking, badge and tooltip readable on the dark ground
The node preview uses the same token, tuned for dark
Tests
Backend,
src/backend/tests/unit/components/prompts/test_validate_prompt_reserved_prefix.py: parametrized over_x,_,__y,_type,_frontend_node_flow_idagainst the controlsvar,a_b,var_1,private_,x, in both syntaxes; plus the message naming only the offending variables and keeping its underscores through markdown.Frontend:
promptVariables.test.tsmirrors the same case list;var-highlight-html.test.tspins the emitted markup, the tooltip rule and the capture order ofregexHighlight, and reads the tooltip back from the DOM so an unescaped character surfaces as a truncated attribute instead of passing a substring match;prompt-highlight.test.tsandpromptComponent.test.tsxcover both preview paths in both syntaxes.test_underscore_variables_acceptedasserted that{{_private}}was accepted, and the mustache preview test asserted the regular highlight class for the same name. Both were written to cover underscores in variable names generally, not to settle whether a leading underscore should be allowed; they now use a trailing underscore and the invalid class respectively, and new cases pin that an underscore anywhere other than the first character stays valid.Summary by CodeRabbit
New Features
_are now identified as reserved and rejected with clear, localized guidance.Bug Fixes