Skip to content

Commit ccc2dcb

Browse files
committed
fix(stx): also drop reserved keywords from createSafeFunction params
The previous fix dropped only non-identifier names from the function parameter list. JavaScript also rejects identifiers that match reserved words ("Cannot use the keyword 'class' as a parameter name"), and strict-mode-reserved words like `let`, `static`, `arguments`, `eval`. Real templates trip this constantly: Vue-/Tailwind-style components spread `class`, `style`, `for`, `data-*`, `aria-*` etc. into the evaluator as prop bag keys. A single offending key — `class`, `for`, `let` — used to crash every `@if` / `{{ }}` / safe-evaluation in the template. The Stacks dashboard hit this on every UI component that takes a `class` prop (Select, Input, Button, …). Add a RESERVED_PARAM_NAMES Set covering every word the spec or strict mode forbids in parameter position, and skip those keys alongside the existing identifier filter. Same projection wrapper keeps callers' positional values aligned with the surviving columns. Tests cover individual reserved words, a grab-bag of common HTML attribute names, and an end-to-end `@if(disabled)` case where the context also has a `class` key (the real AppButton scenario).
1 parent 146780d commit ccc2dcb

2 files changed

Lines changed: 81 additions & 14 deletions

File tree

packages/stx/src/safe-evaluator.ts

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,32 @@ export function safeEvaluateObject(expression: string, context: Record<string, u
450450
}
451451
}
452452

453+
/**
454+
* Names that JavaScript rejects as function parameters even when they look
455+
* like valid identifiers. Built from the spec's reserved word set plus the
456+
* strict-mode-only reserved words (we always emit `'use strict'` in the
457+
* generated function body) plus the two names — `arguments` and `eval` —
458+
* that strict mode forbids in parameter position. Used by createSafeFunction
459+
* to filter incoming context keys so a single bad name (`class`, `for`,
460+
* `let`, …) doesn't crash the entire @if / {{ }} / safe-evaluation pipeline.
461+
*/
462+
const RESERVED_PARAM_NAMES = new Set([
463+
// Reserved words (ES2015+)
464+
'break', 'case', 'catch', 'class', 'const', 'continue', 'debugger',
465+
'default', 'delete', 'do', 'else', 'enum', 'export', 'extends', 'false',
466+
'finally', 'for', 'function', 'if', 'import', 'in', 'instanceof', 'new',
467+
'null', 'return', 'super', 'switch', 'this', 'throw', 'true', 'try',
468+
'typeof', 'var', 'void', 'while', 'with', 'yield',
469+
// Strict-mode-reserved
470+
'let', 'static', 'implements', 'interface', 'package', 'private',
471+
'protected', 'public',
472+
// Always banned in strict-mode parameter position
473+
'arguments', 'eval',
474+
// `await` is reserved inside async functions; we wrap the body in async
475+
// semantics via try/catch in callers, so be conservative.
476+
'await',
477+
])
478+
453479
/**
454480
* Create a sandboxed function that can only access the provided context
455481
* This is a safer alternative to `new Function()` that validates expressions first
@@ -469,24 +495,34 @@ export function createSafeFunction(expression: string, contextKeys: string[]): (
469495
// Validate the expression first
470496
const sanitizedExpr = sanitizeExpression(expression)
471497

472-
// Drop context keys that aren't valid JS identifiers BEFORE handing them
473-
// to `new Function(...keys, body)` — JS rejects param names like
474-
// `passed-class`, `data-id`, `1bad`, `foo.bar` with "Unexpected token
475-
// '-'" / "Unexpected number". Real templates regularly spread HTML-style
476-
// kebab-case attribute keys into the evaluator (component prop bags,
477-
// dataset entries, model rows with hyphenated columns); a single
478-
// hyphenated key used to silently break every `@if` / `{{ }}` directive
479-
// in the template. They can't be referenced from the expression anyway
480-
// — JS has no syntax to read `passed-class` directly — so dropping is
481-
// both safe and what callers expect.
498+
// Drop context keys that JS rejects as function-parameter names BEFORE
499+
// handing them to `new Function(...keys, body)`. Three classes of
500+
// rejected names:
501+
//
502+
// 1. Non-identifiers: `passed-class`, `data-id`, `1bad`, `foo.bar`
503+
// ("Unexpected token '-'" / "Unexpected number").
504+
// 2. Reserved words: `class`, `const`, `function`, `if`, `for`, …
505+
// ("Cannot use the keyword 'class' as a parameter name").
506+
// 3. Strict-mode-reserved words: `let`, `static`, `implements`, …
507+
// (we always emit `'use strict'` in the function body).
508+
//
509+
// Real templates spread HTML-style attribute names into the evaluator
510+
// constantly — Vue-/Tailwind-style components use `class`, `style`,
511+
// `for`, `data-*`, `aria-*`, etc. as prop bag keys. A single offender
512+
// used to crash every `@if` / `{{ }}` evaluation in the template; this
513+
// filter makes the evaluator survive whatever the caller spreads in.
514+
// Dropped keys can't be referenced from the expression anyway — JS has
515+
// no syntax to read `class` or `passed-class` as identifiers in user
516+
// code — so dropping is both safe and what callers expect.
482517
const validIdent = /^[A-Za-z_$][\w$]*$/
483518
const validIndices: number[] = []
484519
const filteredKeys: string[] = []
485520
for (let i = 0; i < contextKeys.length; i++) {
486-
if (validIdent.test(contextKeys[i])) {
487-
validIndices.push(i)
488-
filteredKeys.push(contextKeys[i])
489-
}
521+
const key = contextKeys[i]
522+
if (!validIdent.test(key)) continue
523+
if (RESERVED_PARAM_NAMES.has(key)) continue
524+
validIndices.push(i)
525+
filteredKeys.push(key)
490526
}
491527

492528
// Create the function with strict mode

packages/stx/test/utils/safe-function-context-filtering.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,37 @@ describe('createSafeFunction — context key filtering', () => {
3838
const fn = createSafeFunction('$x + _y', ['$x', '_y'])
3939
expect(fn(2, 3)).toBe(5)
4040
})
41+
42+
it('drops `class` (reserved word) — common HTML attribute name', () => {
43+
const fn = createSafeFunction('disabled', ['disabled', 'class', 'for'])
44+
expect(fn(true, 'btn-primary', 'email-input')).toBe(true)
45+
expect(fn(false, 'btn-primary', 'email-input')).toBe(false)
46+
})
47+
48+
it('drops every other reserved word that shows up as a context key', () => {
49+
// A grab-bag of names that real templates use as bag keys but that
50+
// JS rejects in parameter position.
51+
const reservedKeys = ['if', 'for', 'class', 'new', 'this', 'super', 'let', 'const', 'return']
52+
const allKeys = ['name', ...reservedKeys, 'count']
53+
const fn = createSafeFunction('name + ":" + count', allKeys)
54+
const values = ['ok', 1, 2, 3, 4, 5, 6, 7, 8, 9, 42]
55+
expect(fn(...values)).toBe('ok:42')
56+
})
57+
58+
it('drops `arguments` and `eval` (strict-mode-only reserved param names)', () => {
59+
const fn = createSafeFunction('x', ['x', 'arguments', 'eval'])
60+
expect(fn(7, [], () => 0)).toBe(7)
61+
})
62+
63+
it('@if condition still evaluates when context has a `class` key (the real-world AppButton case)', () => {
64+
const result = processConditionals(
65+
'<button @if(disabled) disabled @endif>x</button>',
66+
{ disabled: true, class: 'btn-primary', for: 'email' },
67+
'test.stx',
68+
)
69+
expect(result).not.toContain('If Error')
70+
expect(result).toContain('disabled')
71+
})
4172
})
4273

4374
describe('processConditionals — robust to hyphenated context keys', () => {

0 commit comments

Comments
 (0)