diff --git a/.changeset/sdui-parser-brace-literal-subset.md b/.changeset/sdui-parser-brace-literal-subset.md new file mode 100644 index 0000000000..0659453dd5 --- /dev/null +++ b/.changeset/sdui-parser-brace-literal-subset.md @@ -0,0 +1,29 @@ +--- +'@objectstack/sdui-parser': minor +--- + +sdui-parser: `interpretBrace` materializes the JS literal subset, in lockstep with objectui + +The html tier's braced attribute values accepted strict JSON only, so the spelling every +JSX author and every AI author writes — `columns={['name','amount']}` — compiled to the +deferred `{ $expr }` marker that nothing downstream evaluates, and the author's data +binding vanished at render. Under the maintainer's ruling on objectui#6614 (Q1-A, +2026-08-28) `interpretBrace` now materializes the JS **literal subset**: exactly two +widenings over JSON — single-quoted strings (value position and key position) and unquoted +identifier object keys. + +Everything else JSON refuses is still refused and still becomes `{ $expr }`: trailing +commas, comments, array holes, spreads, `undefined` / `NaN` / `Infinity`, `+1` / `.5` / +`1.` / `0x1f`, template literals, and every genuine expression. `JSON.parse` still runs +first and untouched, so strict-JSON behaviour is invariant by construction, and the subset +contains no identifier lookup and no operator — the widening moves habitual spellings onto +the materialized side, it does not move the data/code boundary (ADR-0080: this tier parses, +never executes). + +An authored `__proto__` key is written as an own data property, the way `JSON.parse` gives +it, never through the prototype setter — a plain assignment in the unquoted-key path would +hand untrusted page source a prototype-pollution lever the strict-JSON path never had. + +The `inert-expression` diagnostic message is reworded to match: the old text advised +writing the value as JSON with double-quoted strings and keys, which now names a legal +spelling as the wrong one. Diagnostic **codes** are unchanged. diff --git a/packages/sdui-parser/src/__tests__/inert-expression.test.ts b/packages/sdui-parser/src/__tests__/inert-expression.test.ts index b55b1b913d..3796c909dc 100644 --- a/packages/sdui-parser/src/__tests__/inert-expression.test.ts +++ b/packages/sdui-parser/src/__tests__/inert-expression.test.ts @@ -1,33 +1,43 @@ /** - * `inert-expression` — the html tier's silent-vanish hole for braced non-JSON - * values, ported into this copy in lockstep with objectui PR #6613. + * `inert-expression` — the html tier's silent-vanish hole for braced values + * this tier cannot materialize. Ported into this copy in lockstep with + * objectui's `packages/sdui-parser` (objectui#6613, message reworded by + * objectui#6614). * - * `interpretBrace` materializes strict-JSON values only; anything else — the - * single-quoted array every JSX author writes, unquoted object keys, any JS - * expression — becomes the deferred `{ $expr }` marker, and NOTHING downstream - * evaluates that marker (this tier parses, never executes — ADR-0080; no - * renderer consumes `$expr`). So `columns={['name','amount']}` used to compile - * with ZERO diagnostics into a value every renderer's defensive non-array read - * degrades to "no columns declared": rows render, the author's whole data - * binding is eaten, and no surface ever says why. That is ADR-0078's prohibited - * parsed-but-silently-inert state, reported from production as objectui#6598. + * `interpretBrace` materializes strict JSON plus the JS LITERAL SUBSET + * (objectui#6614 Q1-A, ruled 2026-08-28); a GENUINE EXPRESSION still becomes + * the deferred `{ $expr }` marker, and NOTHING downstream evaluates that marker + * (this tier parses, never executes — ADR-0080; no renderer consumes `$expr`). + * Such a value reaches the renderer as an opaque object, every defensive + * non-array read degrades it to "not declared", and the author's binding is + * eaten in silence. That is ADR-0078's prohibited parsed-but-silently-inert + * state, reported from production as objectui#6598. * * WHY THIS FILE EXISTS HERE AND NOT ONLY THERE. There are two copies of this * parser — objectui's `packages/sdui-parser` and this repo's hoisted - * `@objectstack/sdui-parser` — and the invariant is that both copies agree on - * the accepted grammar AND on diagnostic codes. If they drift, the save gate - * and the renderer speak different dialects: a page can save clean and render - * inert, or the reverse — surface-dependent, and therefore intermittent from - * the author's point of view. These pins are the objectstack half of that - * lockstep; the emitted diagnostic is byte-equal to objectui's. + * `@objectstack/sdui-parser` — and the invariant (#12719) is that both copies + * agree on the accepted grammar AND on diagnostic codes. If they drift, the + * save gate and the renderer speak different dialects: a page can save clean + * and render inert, or the reverse — surface-dependent, and therefore + * intermittent from the author's point of view. These pins are the objectstack + * half of that lockstep. * - * Severity is pinned as WARNING deliberately (the objectui#5709 precedent for - * inert authored keys), and `ok` is pinned true alongside it: this port reports - * an ALREADY-inert state, so it must leave the accept/reject set exactly where - * it stood. Escalating to error, widening the accepted literal grammar (single - * quotes / unquoted keys — objectui#6614), and base-prop (`style`) coverage are - * open contract decisions; a change to any of those should move these pins - * consciously, not by accident. + * ⭐ WHAT MOVED IN #6614 Q1-A, AND WHY IT IS NOT AN ACCIDENT. This file + * originally pinned `columns={['name','amount']}`, `columns={[{field:"name"}]}` + * and `options={{pageSize: 25}}` as WARNING cases, and said in so many words + * that widening the literal grammar "should move these pins consciously, not by + * accident". Q1-A widened it, so those three spellings now MATERIALIZE and are + * correct — the whole point of the ruling. They moved to + * `literal-subset-6614.test.ts`, which pins their values; each was replaced + * here by a genuine expression, so this file still pins the same FACT (an inert + * braced value is never silent) on the same side of the new boundary. + * + * Severity stays WARNING deliberately (the objectui#5709 precedent for inert + * authored keys), and `ok` is pinned true alongside it: this diagnostic reports + * an ALREADY-inert state, so it leaves the accept/reject set exactly where it + * stood. ⛔ Escalation to error is objectui#6614 **Q2**, which lands at the SAVE + * GATE once the framework wires the registry manifest into `validate-jsx-pages` + * (#12719 records that gap) — not here, and not at render. */ import { describe, expect, it } from 'vitest'; import { compile } from '../index.js'; @@ -47,9 +57,12 @@ const manifest: Manifest = { }, }; -describe('inert-expression: braced non-JSON on a declared input warns instead of vanishing', () => { - it('single-quoted array — the JSX habit — draws the warning and stays in the tree as $expr', () => { - const r = compile(``, manifest); +describe('inert-expression: a braced EXPRESSION on a declared input warns instead of vanishing', () => { + it('a method call — the shape #6598 could not materialize — warns and stays as $expr', () => { + const r = compile( + ` r.name)} />`, + manifest, + ); expect(r.diagnostics).toEqual([ expect.objectContaining({ severity: 'warning', @@ -60,33 +73,37 @@ describe('inert-expression: braced non-JSON on a declared input warns instead of ]); // The marker itself is unchanged — the tree still carries the deferred // value; only the silence is gone. - expect(r.tree?.columns).toEqual({ $expr: "['name','amount']" }); + expect(r.tree?.columns).toEqual({ $expr: 'rows.map((r) => r.name)' }); + // Warning, not error: the page still compiles (the objectui#5709 posture). + expect(r.ok).toBe(true); }); - it('the message carries the FIX, not merely the complaint', () => { + it('the message names the CURRENT accepted grammar, not a now-legal spelling', () => { // An arrival pin, not a departure pin: "stopped being silent" is satisfied - // by any diagnostic at all. What this port owes the author is the remedy — - // name JSON, and show the corrected spelling next to the broken one. A - // message rewrite that drops the remedy turns this red. + // by any diagnostic at all. What this port owes the author is advice that + // is still TRUE after objectui#6614 Q1-A — the pre-#6614 wording told the + // author to "write it as JSON (double-quoted strings and keys)" and named + // `columns={['name','amount']}` as the wrong form, which would now send + // them to edit working source. const [d] = compile( - ``, + ` r.name)} />`, manifest, ).diagnostics; - expect(d.message).toMatch(/JSON/); - expect(d.message).toContain('double-quoted strings and keys'); - expect(d.message).toContain('columns={["name","amount"]}'); - expect(d.message).toContain(`columns={['name','amount']}`); + expect(d.message).not.toMatch(/double-quoted/); + expect(d.message).toContain('LITERALS only'); + expect(d.message).toContain(`columns={['name','amount']} works`); + expect(d.message).toContain('columns={rows.map((r) => r.name)} cannot'); }); - it('unquoted object keys draw the same warning', () => { - const r = compile(``, manifest); + it('a bare identifier draws the same warning', () => { + const r = compile(``, manifest); expect(r.diagnostics).toEqual([ expect.objectContaining({ severity: 'warning', code: 'inert-expression' }), ]); }); it('an $expr on an object-typed input is covered too', () => { - const r = compile(``, manifest); + const r = compile(``, manifest); expect(r.diagnostics).toEqual([ expect.objectContaining({ severity: 'warning', code: 'inert-expression', tag: 'list-view' }), ]); @@ -108,14 +125,14 @@ describe('inert-expression: braced non-JSON on a declared input warns instead of }); it('the accept/reject set does not move — every inert spelling still compiles', () => { - // The load-bearing property of this port: it reports an ALREADY-inert + // The load-bearing property: this diagnostic reports an ALREADY-inert // state, so `ok` (no error-severity diagnostic — the save gate's pass/fail) // is exactly what it was before the diagnostic existed. Escalating the // severity to error is objectui#6614's Q2 and would land here first. for (const source of [ - ``, - ``, - ``, + ` r.name)} />`, + ``, + ``, ]) { const r = compile(source, manifest); expect(r.ok).toBe(true); @@ -124,7 +141,7 @@ describe('inert-expression: braced non-JSON on a declared input warns instead of }); it('an $expr on an UNKNOWN prop keeps drawing unknown-prop, not a double report', () => { - const r = compile(``, manifest); + const r = compile(``, manifest); expect(r.diagnostics).toEqual([ expect.objectContaining({ severity: 'warning', code: 'unknown-prop' }), ]); diff --git a/packages/sdui-parser/src/__tests__/literal-subset-6614.test.ts b/packages/sdui-parser/src/__tests__/literal-subset-6614.test.ts new file mode 100644 index 0000000000..a9539e0ffe --- /dev/null +++ b/packages/sdui-parser/src/__tests__/literal-subset-6614.test.ts @@ -0,0 +1,390 @@ +/** + * objectui#6614 Q1-A — `interpretBrace` materializes the JS LITERAL SUBSET. + * The objectstack half of the #12719 lockstep (this card: #12977). + * + * Maintainer ruling on objectui#6614, 2026-08-28, verbatim and untranslated: + * 「6614 也同意」 ⇒ Q1-A · Q2-A · Q3-A, adopting that card's recommendation + * whole. This file pins Q1-A and ONLY Q1-A, in this repo's copy of the parser. + * + * WHY THE RULING EXISTS. The html tier is the untrusted-safe DATA tier — + * parsed, never executed (ADR-0080) — and it is the only safe carrier for + * runtime AI or tenant authoring. Before this change it accepted strict JSON + * inside braces while calling itself JSX, so `columns={['name','amount']}` — + * the spelling every JSX author and every AI author writes — compiled to the + * deferred `{ $expr }` marker that nothing downstream evaluates, and the + * author's whole data binding vanished at render (objectui#6598, reported from + * production after eight spellings were tried). That is a trap, not a contract. + * + * WHY THIS FILE EXISTS IN THIS REPO. #12719's invariant is that both copies of + * the parser byte-agree on the accepted grammar and on diagnostic codes. It + * carried two obligations and landed only one (the diagnostic, #12811); the + * grammar half was withheld until objectui#6614 was ruled, and this file is + * that half arriving. objectui landed first deliberately: `interpretBrace` + * emits no diagnostic in either dialect, so an objectui-first window means a + * page saves exactly as it did and now renders correctly, whereas + * objectstack-first would have meant the save gate materialising while the + * renderer still deferred — "saves clean, renders inert", the very defect + * objectui#6598 is. + * + * ⭐ WHY THE REFUSAL PINS BELOW MATTER AS MUCH AS THE POSITIVE ONES. A suite + * that only asserted the newly-legal spellings would pass just as well against + * a parser that started accepting EVERYTHING — including code — which is the + * one failure this widening must never ship. So every positive pin here has a + * refusal pin naming the first thing on the far side of the boundary, and the + * refusal table is itself guarded against silently emptying. + * + * THE BOUNDARY, stated once. Exactly two widenings over JSON: + * 1. single-quoted strings (value position AND key position); + * 2. unquoted identifier object keys. + * Everything else JSON refuses is still refused: trailing commas, comments, + * holes, spreads, `undefined`/`NaN`/`Infinity`, `+1`/`.5`/`1.`/`0x1f`, template + * literals, identifiers, member access, calls, and every operator. The subset + * contains no identifier lookup and no operator, so there is nothing in it to + * execute — the widening moves habitual spellings onto the materialized side, + * it does not move the data/code boundary. + * + * ⛔ NOT IN SCOPE HERE, by the same ruling: Q2 (escalating `inert-expression` + * from warning to error — that lands at the SAVE GATE once the framework wires + * the registry manifest into `validate-jsx-pages`; #12719 records that gap) and + * Q3 (base-prop `$expr` inertness, sequenced deliberately AFTER this change so + * no warning is added for spellings this change is about to legalise). + */ +import { describe, expect, it } from 'vitest'; +import { compile, interpretBrace, parseJsx } from '../index.js'; +import type { Manifest } from '../types.js'; + +const manifest: Manifest = { + components: { + 'list-view': { + type: 'list-view', + namespace: 'plugin-list', + inputs: [ + { name: 'objectName', type: 'string', required: true }, + { name: 'columns', type: 'array' }, + { name: 'options', type: 'object' }, + { name: 'title', type: 'string' }, + { name: 'pageSize', type: 'number' }, + { name: 'enabled', type: 'boolean' }, + ], + }, + }, +}; + +/** The braced attribute value, as it reaches the renderer. */ +const propOf = (braced: string, prop = 'columns'): unknown => + parseJsx(``).tree?.[prop]; + +describe('#6614 Q1-A — the two ruled widenings materialize', () => { + it('the production spelling: a single-quoted array reaches the renderer as a real array', () => { + // objectui#6598's page, byte-for-byte. This is the assertion the whole card + // exists for: before the ruling this was `{ $expr: "['name','amount']" }`. + expect(propOf(`['name','amount']`)).toEqual(['name', 'amount']); + expect(interpretBrace(`['name','amount']`)).toEqual(['name', 'amount']); + }); + + it('a single-quoted string in value position materializes to the string itself', () => { + expect(propOf(`'Accounts'`, 'title')).toBe('Accounts'); + // No quotes survive into the value — the point is normalization, not a + // literal copy of the source text. + expect(interpretBrace(`'Accounts'`)).toBe('Accounts'); + }); + + it('unquoted identifier object keys materialize', () => { + expect(propOf(`{pageSize: 25}`, 'options')).toEqual({ pageSize: 25 }); + expect(propOf(`[{field:"name"},{field:"amount"}]`)).toEqual([ + { field: 'name' }, + { field: 'amount' }, + ]); + // `$` and `_` are identifier characters, and a digit is legal after the first. + expect(interpretBrace(`{_a: 1, $b: 2, c3: 3}`)).toEqual({ _a: 1, $b: 2, c3: 3 }); + }); + + it('a single-quoted KEY is covered — it is widening #1 in key position', () => { + expect(interpretBrace(`{'pageSize': 25}`)).toEqual({ pageSize: 25 }); + }); + + it('the two widenings compose, and nest', () => { + expect(interpretBrace(`{cols: ['name','amount'], page: {size: 25, deep: ['x']}}`)).toEqual({ + cols: ['name', 'amount'], + page: { size: 25, deep: ['x'] }, + }); + }); + + it('escapes inside a single-quoted string follow JSON, plus `\\u0027`', () => { + expect(interpretBrace(`'a\\'b'`)).toBe("a'b"); + expect(interpretBrace(`'tab\\there'`)).toBe('tab\there'); + expect(interpretBrace(`'\\u0041'`)).toBe('A'); + // A double quote needs no escape inside single quotes, and vice versa. + expect(interpretBrace(`'say "hi"'`)).toBe('say "hi"'); + }); + + it('the materialized spellings draw NO diagnostic — the page is simply correct now', () => { + for (const source of [ + ``, + ``, + ``, + ``, + ``, + ]) { + const r = compile(source, manifest); + expect(r.diagnostics, source).toEqual([]); + expect(r.ok, source).toBe(true); + } + }); + + it('a materialized value is type-checked like any other — the widening does not skip checkType', () => { + // Single quotes get it PAST the marker; they do not get it past the + // manifest. A string where an array is declared is still a mismatch. + const r = compile(``, manifest); + expect(r.diagnostics).toEqual([ + expect.objectContaining({ code: 'type-mismatch', tag: 'list-view' }), + ]); + }); +}); + +/** + * The far side of the boundary. ⭐ This table IS the durable half of the port: + * it pins the RULING (exactly two widenings), not merely today's code, so a + * later widening past those two additions turns it red rather than passing + * quietly. + */ +const REFUSED: Array<[label: string, braced: string]> = [ + ['a bare identifier', `columns`], + ['member access', `ctx.user.name`], + ['a function call', `getColumns()`], + ['a method call on a literal', `['a'].concat(b)`], + ['the card-quoted expression', `rows.map((r) => r.name)`], + ['an arrow function', `() => 1`], + ['arithmetic', `1 + 2`], + ['a literal that merely STARTS the expression', `['a'] + x`], + ['a ternary', `a ? b : c`], + ['logical short-circuit', `flag && ['a']`], + ['a template literal', '`col-${n}`'], + ['an array spread', `[...cols]`], + ['an object spread', `{...base, pageSize: 25}`], + ['a trailing comma in an array', `['a',]`], + ['a trailing comma in an object', `{a: 1,}`], + ['an array hole', `[,1]`], + ['a block comment', `/* cols */ ['a']`], + ['a line comment', `['a'] // cols`], + ['undefined', `undefined`], + ['NaN', `NaN`], + ['Infinity', `Infinity`], + ['a leading plus', `+1`], + ['a bare fractional point', `.5`], + ['a trailing decimal point', `1.`], + ['a hex number', `0x1f`], + ['an octal-ish number', `010`], + ['a non-JSON escape', `'\\x41'`], + ['an unterminated string', `['a`], + ['a computed key', `{[k]: 1}`], + ['a keyword glued to an identifier', `nullish`], + ['an empty brace', ``], +]; + +describe('#6614 Q1-A — the far side of the boundary is still refused', () => { + it('⭐ the refusal table is non-empty and still names every class the ruling refuses', () => { + // A guard whose success condition equals its total-failure condition must + // REFUSE. `it.each([])` registers zero cases and reports a clean run, so an + // emptied or thinned table would look exactly like a passing suite. Assert + // the walk has something to walk, and that each class named in the ruling + // is still represented — deleting a row to make a widening "pass" now costs + // a red test here first. + expect(REFUSED.length).toBeGreaterThanOrEqual(31); + const labels = REFUSED.map(([label]) => label).join(' | '); + for (const required of [ + 'identifier', + 'member access', + 'function call', + 'arrow function', + 'arithmetic', + 'ternary', + 'template literal', + 'spread', + 'trailing comma', + 'hole', + 'comment', + 'undefined', + 'NaN', + 'Infinity', + 'plus', + 'fractional point', + 'decimal point', + 'hex', + 'computed key', + ]) { + expect(labels, `refusal class missing from the table: ${required}`).toContain(required); + } + }); + + it.each(REFUSED)('%s stays the deferred `{ $expr }` marker', (_label, braced) => { + expect(interpretBrace(braced)).toEqual({ $expr: braced.trim() }); + }); + + it('a refused value still draws the warning-severity `inert-expression` diagnostic', () => { + const r = compile( + ` r.name)} />`, + manifest, + ); + expect(r.diagnostics).toEqual([ + expect.objectContaining({ + severity: 'warning', + code: 'inert-expression', + tag: 'list-view', + message: expect.stringContaining('"columns"'), + }), + ]); + expect(r.tree?.columns).toEqual({ $expr: 'rows.map((r) => r.name)' }); + // ⛔ Q2 is NOT this change: the render-side severity stays `warning`, and + // the page still compiles. Escalation belongs at the save gate. + expect(r.ok).toBe(true); + }); + + it('the diagnostic no longer advises a spelling that now works', () => { + const r = compile(``, manifest); + const message = r.diagnostics[0].message; + // The pre-#6614 message said "write it as JSON (double-quoted strings and + // keys)" and named `columns={['name','amount']}` as the WRONG form. That + // spelling is now correct, so the old advice would send an author to edit + // working source. Pin that it is gone. + expect(message).not.toMatch(/double-quoted/); + expect(message).toContain('LITERALS only'); + }); +}); + +describe('#6614 Q1-A — strict JSON is byte-identical to before', () => { + /** + * The invariance is structural, not incidental: `interpretBrace` still calls + * `JSON.parse` FIRST and untouched, so an input JSON accepts never reaches + * the new reader at all. This pins the property that structure guarantees. + */ + const STRICT_JSON = [ + `["name","amount"]`, + `[{"field":"name","label":"Full Name"}]`, + `{"pageSize":25}`, + `{}`, + `[]`, + `"plain string"`, + `25`, + `-0`, + `1e3`, + `-1.5e-3`, + `true`, + `false`, + `null`, + `{"nested":{"deep":[1,2,{"x":null}]}}`, + `"escaped \\" quote"`, + `"\\u00e9"`, + `{"__proto__":1}`, + `{"a":1,"a":2}`, + ]; + + it.each(STRICT_JSON)('%s parses exactly as JSON.parse does', (source) => { + expect(interpretBrace(source)).toEqual(JSON.parse(source)); + }); + + it('-0 keeps its sign, as JSON.parse gives it', () => { + expect(Object.is(interpretBrace(`-0`), -0)).toBe(true); + }); + + it('a strict-JSON page compiles with the same zero diagnostics it always did', () => { + for (const source of [ + ``, + ``, + ``, + ``, + ]) { + const r = compile(source, manifest); + expect(r.diagnostics, source).toEqual([]); + expect(r.ok, source).toBe(true); + } + }); +}); + +describe('#6614 Q1-A — the widening opens no execution or pollution lever', () => { + /** + * ⚠️ THE SECURITY CLAUSE, ahead of the grammar. This tier's whole reason to + * exist is that untrusted page source is safe to PARSE. A real JS object + * literal `{__proto__: {...}}` invokes the prototype SETTER; `JSON.parse` + * creates an ordinary own data property. Matching JS here would be WRONG: a + * plain `out[key] = value` in the unquoted-key path would hand untrusted page + * source a prototype-pollution lever the strict-JSON path never had — that is + * a widening of ATTACK SURFACE, not a parser detail. + * + * So the assertion is on the PROPERTY DESCRIPTOR, in every key spelling the + * grammar now admits. Reading the value back would pass against the + * vulnerable implementation too, because the setter's target is readable + * through the prototype chain — a read is not evidence here. + */ + const PROTO_SPELLINGS: Array<[label: string, braced: string]> = [ + ['unquoted (the newly-legal key spelling)', `{__proto__: {polluted: true}}`], + ['single-quoted key (widening #1 in key position)', `{'__proto__': {'polluted': true}}`], + ['double-quoted key (the strict-JSON path, unchanged)', `{"__proto__": {"polluted": true}}`], + ]; + + it.each(PROTO_SPELLINGS)( + 'an authored `__proto__` key — %s — becomes an OWN DATA property, never the prototype', + (_label, braced) => { + const value = interpretBrace(braced) as Record; + + // 1. It is an own property, not something reached through the chain. + expect(Object.prototype.hasOwnProperty.call(value, '__proto__')).toBe(true); + + // 2. It is a DATA property — `value`, not `get`/`set`. This is the + // assertion a prototype-setter implementation fails. + const descriptor = Object.getOwnPropertyDescriptor(value, '__proto__'); + expect(descriptor).toBeDefined(); + expect(descriptor).toEqual( + expect.objectContaining({ enumerable: true, writable: true, configurable: true }), + ); + expect(descriptor && 'value' in descriptor).toBe(true); + expect(descriptor?.get).toBeUndefined(); + expect(descriptor?.set).toBeUndefined(); + expect(descriptor?.value).toEqual({ polluted: true }); + + // 3. The object's own prototype is untouched, and nothing leaked to + // `Object.prototype` — the actual pollution the lever would buy. + expect(Object.getPrototypeOf(value)).toBe(Object.prototype); + expect(({} as Record).polluted).toBeUndefined(); + expect((Object.prototype as Record).polluted).toBeUndefined(); + }, + ); + + it('the DEFERRED path gains no lever either — a refused `__proto__` source stays a plain marker', () => { + // The negative side of the clause. An expression-valued `__proto__` key is + // refused by the grammar and comes back as `{ $expr }`; that marker is + // constructed here, from an object literal whose only key is `$expr`, so + // authored text can never reach a key position at all. Pinned so a future + // "helpful" marker that echoes parsed keys cannot reopen the hole. + const marker = interpretBrace(`{__proto__: ctx.evil}`) as Record; + expect(marker).toEqual({ $expr: '{__proto__: ctx.evil}' }); + expect(Object.keys(marker)).toEqual(['$expr']); + expect(Object.prototype.hasOwnProperty.call(marker, '__proto__')).toBe(false); + expect(Object.getPrototypeOf(marker)).toBe(Object.prototype); + expect(({} as Record).polluted).toBeUndefined(); + }); + + it('a nested `__proto__` key is an own data property too — the guard is not top-level only', () => { + const value = interpretBrace(`{opts: {__proto__: {polluted: true}}}`) as { + opts: Record; + }; + expect(Object.prototype.hasOwnProperty.call(value.opts, '__proto__')).toBe(true); + expect(Object.getPrototypeOf(value.opts)).toBe(Object.prototype); + expect(({} as Record).polluted).toBeUndefined(); + }); + + it('a key named like a prototype member does not read through to it', () => { + const value = interpretBrace(`{constructor: 'x', toString: 'y'}`) as Record; + expect(value.constructor).toBe('x'); + expect(value.toString).toBe('y'); + }); + + it('duplicate keys take the last value, as both JSON and a JS literal do', () => { + expect(interpretBrace(`{a: 1, a: 2}`)).toEqual({ a: 2 }); + }); + + it('event handlers and raw-HTML injection stay forbidden regardless of the value grammar', () => { + const r = parseJsx(``); + expect(r.diagnostics.map((d) => d.code)).toEqual(['forbidden-attr', 'forbidden-attr']); + }); +}); diff --git a/packages/sdui-parser/src/parse.ts b/packages/sdui-parser/src/parse.ts index f5af6ca7df..b832904b61 100644 --- a/packages/sdui-parser/src/parse.ts +++ b/packages/sdui-parser/src/parse.ts @@ -280,15 +280,247 @@ class Parser { /** * Interpret a braced attribute value `{...}`. - * JSON-literal values (numbers, booleans, null, strings, arrays, objects with - * quoted keys) are materialized. Anything else is kept as a deferred expression - * marker `{ $expr }` — typed and validated later, NEVER evaluated here. + * + * Strict-JSON values are materialized by `JSON.parse`, exactly as they always + * were. Beyond that, the JS **literal subset** below is materialized too + * (objectui#6614 Q1-A, maintainer ruling 2026-08-28). Anything left over — a + * genuine expression — is kept as the deferred marker `{ $expr }`: typed and + * validated later, drawing `inert-expression`, and NEVER evaluated here. + * + * ORDER IS LOAD-BEARING. `JSON.parse` runs FIRST and is untouched, so every + * input JSON accepts takes byte-identically the path it took before the literal + * subset existed. The reader below only ever sees strings `JSON.parse` has + * already thrown on, which makes strict-JSON invariance a property of the + * structure rather than of a test. + * + * LOCKSTEP: this grammar is the port of objectui's `packages/sdui-parser` copy + * (objectui#6614). The two copies must agree on the accepted grammar AND on + * diagnostic codes — if they drift, the save gate and the renderer speak + * different dialects and a page can save clean and render inert + * (objectstack#12719 states the invariant; #12977 carries this half of it). + * Change this block only together with the objectui copy. */ export function interpretBrace(raw: string): unknown { const trimmed = raw.trim(); try { return JSON.parse(trimmed); } catch { - return { $expr: trimmed }; + const literal = readLiteral(trimmed); + return literal === NOT_LITERAL ? { $expr: trimmed } : literal; + } +} + +/* ---------------------- the JS literal subset (#6614) ---------------------- */ + +/** + * EXACTLY TWO widenings over JSON, and nothing else: + * + * 1. **single-quoted strings** — `{'name'}`, `{['name','amount']}`, and in + * key position `{{'pageSize': 25}}`; + * 2. **unquoted identifier object keys** — `{{pageSize: 25}}`. + * + * Everything else JSON refuses is still refused and still becomes `{ $expr }`: + * trailing commas, comments, array holes, spreads, `undefined` / `NaN` / + * `Infinity`, `+1` / `.5` / `1.` / `0x1f`, template literals, and every genuine + * expression — identifiers, member access, calls, operators, ternaries. + * + * That list is deliberately short. This is a VALUE grammar, not an evaluator: + * it contains no identifier lookup and no operator, so there is nothing here to + * execute (ADR-0080 — this tier parses, never executes). The widening moves the + * spellings an author writes by habit onto the materialized side; it does not + * move the boundary between data and code. + */ +const NOT_LITERAL = Symbol('not-a-literal'); + +/** JSON's whitespace set, not JS's — narrower, and one less thing to diverge. */ +const LITERAL_WS = /[ \t\n\r]/; +const IDENT_START = /[A-Za-z_$]/; +const IDENT_CHAR = /[A-Za-z0-9_$]/; +/** JSON's number grammar verbatim: no leading `+`, no `.5`, no `1.`, no hex. */ +const NUMBER = /^-?(?:0|[1-9][0-9]*)(?:\.[0-9]+)?(?:[eE][+-]?[0-9]+)?/; +/** JSON's escape set. `\'` is added for single-quoted strings only. */ +const SIMPLE_ESCAPE: Record = { + '"': '"', + '\\': '\\', + '/': '/', + b: '\b', + f: '\f', + n: '\n', + r: '\r', + t: '\t', +}; + +function readLiteral(src: string): unknown { + const reader = new LiteralReader(src); + const value = reader.value(); + if (value === NOT_LITERAL) return NOT_LITERAL; + reader.ws(); + // Trailing anything means the input was an expression that merely STARTS with + // a literal (`['a'] + x`, `1 + 2`). Refuse the whole input. + return reader.done() ? value : NOT_LITERAL; +} + +class LiteralReader { + private pos = 0; + + constructor(private readonly src: string) {} + + done(): boolean { + return this.pos >= this.src.length; + } + + ws(): void { + while (this.pos < this.src.length && LITERAL_WS.test(this.src[this.pos])) this.pos++; + } + + value(): unknown { + this.ws(); + const c = this.src[this.pos]; + if (c === undefined) return NOT_LITERAL; + if (c === '"' || c === "'") return this.string(c); + if (c === '[') return this.array(); + if (c === '{') return this.object(); + if (this.keyword('true')) return true; + if (this.keyword('false')) return false; + if (this.keyword('null')) return null; + return this.number(); + } + + /** A keyword only when it is not the prefix of a longer identifier. */ + private keyword(word: string): boolean { + if (!this.src.startsWith(word, this.pos)) return false; + const after = this.src[this.pos + word.length]; + if (after !== undefined && IDENT_CHAR.test(after)) return false; + this.pos += word.length; + return true; + } + + private number(): unknown { + const m = NUMBER.exec(this.src.slice(this.pos)); + if (!m) return NOT_LITERAL; + this.pos += m[0].length; + return Number(m[0]); + } + + private string(quote: string): unknown { + this.pos++; // opening quote + let out = ''; + for (;;) { + const c = this.src[this.pos]; + if (c === undefined) return NOT_LITERAL; // unterminated + if (c === quote) { + this.pos++; + return out; + } + if (c === '\\') { + const esc = this.src[this.pos + 1]; + if (esc === undefined) return NOT_LITERAL; + if (esc === 'u') { + const hex = this.src.slice(this.pos + 2, this.pos + 6); + if (!/^[0-9a-fA-F]{4}$/.test(hex)) return NOT_LITERAL; + out += String.fromCharCode(parseInt(hex, 16)); + this.pos += 6; + continue; + } + // `\'` is legal inside a single-quoted string only — JSON's set otherwise. + if (esc === "'" && quote === "'") { + out += "'"; + this.pos += 2; + continue; + } + const simple = SIMPLE_ESCAPE[esc]; + if (simple === undefined) return NOT_LITERAL; // `\x41`, `\0`, line continuation + out += simple; + this.pos += 2; + continue; + } + // JSON forbids raw control characters inside a string; so does this. + if (c < ' ') return NOT_LITERAL; + out += c; + this.pos++; + } + } + + private array(): unknown { + this.pos++; // '[' + const out: unknown[] = []; + this.ws(); + if (this.src[this.pos] === ']') { + this.pos++; + return out; + } + for (;;) { + const item = this.value(); + if (item === NOT_LITERAL) return NOT_LITERAL; + out.push(item); + this.ws(); + const c = this.src[this.pos]; + // A trailing comma leaves `value()` facing `]`, which it refuses — so + // `['a',]` is NOT in the subset. Only two widenings were ruled. + if (c === ',') { + this.pos++; + continue; + } + if (c === ']') { + this.pos++; + return out; + } + return NOT_LITERAL; + } + } + + private object(): unknown { + this.pos++; // '{' + const out: Record = {}; + this.ws(); + if (this.src[this.pos] === '}') { + this.pos++; + return out; + } + for (;;) { + this.ws(); + const key = this.key(); + if (key === NOT_LITERAL) return NOT_LITERAL; + this.ws(); + if (this.src[this.pos] !== ':') return NOT_LITERAL; + this.pos++; + const item = this.value(); + if (item === NOT_LITERAL) return NOT_LITERAL; + // ⚠️ Plain `out[key] = item` would hand an authored `__proto__` key the + // prototype SETTER. `JSON.parse` creates an ordinary own data property, + // and this path must too: the whole point of this tier is that untrusted + // source is safe to parse, so a widening must not open a + // prototype-pollution lever the strict-JSON path never had. + Object.defineProperty(out, key as string, { + value: item, + writable: true, + enumerable: true, + configurable: true, + }); + this.ws(); + const c = this.src[this.pos]; + if (c === ',') { + this.pos++; + continue; + } + if (c === '}') { + this.pos++; + return out; + } + return NOT_LITERAL; + } + } + + /** A quoted string, or a bare identifier — the second ruled widening. */ + private key(): unknown { + const c = this.src[this.pos]; + if (c === '"' || c === "'") return this.string(c); + if (c !== undefined && IDENT_START.test(c)) { + const start = this.pos; + this.pos++; + while (this.pos < this.src.length && IDENT_CHAR.test(this.src[this.pos])) this.pos++; + return this.src.slice(start, this.pos); + } + return NOT_LITERAL; } } diff --git a/packages/sdui-parser/src/validate.ts b/packages/sdui-parser/src/validate.ts index 095cde7feb..150ccdb824 100644 --- a/packages/sdui-parser/src/validate.ts +++ b/packages/sdui-parser/src/validate.ts @@ -93,24 +93,39 @@ export function validateTree(tree: SchemaElement | null, manifest: Manifest): Va // spellings on a data block, all eaten without a single diagnostic — // rows rendered, zero data columns). ADR-0078 prohibits exactly this // parsed-but-silently-inert state, so name it at compile time, with - // the fix in the message. Warning, not error, per the objectui#5709 - // precedent for inert authored keys — escalation to error (and any - // widening of the accepted literal grammar, e.g. single-quoted - // strings) is a contract decision tracked on objectui#6598. + // the fix in the message. + // + // The message must name the CURRENT accepted grammar, and objectui#6614 + // (Q1-A, ruled 2026-08-28) moved it: `interpretBrace` now materializes + // the JS literal subset, so single-quoted strings and unquoted + // identifier keys REACH the renderer and can no longer draw this + // warning. The old wording ("write it as JSON, double-quoted") named a + // now-legal spelling as the illegal one — advice that would have sent + // an author to edit working source. What is left on this side of the + // boundary is a genuine expression, so that is what the message names. + // + // Warning, not error, per the objectui#5709 precedent for inert + // authored keys. ⛔ Escalation to error is objectui#6614 Q2 and is + // deliberately NOT part of this change: it belongs at the SAVE GATE, + // once the framework wires the registry manifest into + // `validate-jsx-pages` (#12719 records that gap). // // LOCKSTEP: this diagnostic is the byte-equal port of objectui's - // `packages/sdui-parser` copy (objectui PR #6613). The two copies - // must agree on the accepted grammar AND on diagnostic codes — if - // they drift, the save gate and the renderer speak different - // dialects and a page can save clean and render inert. Change this - // block only together with the objectui copy. + // `packages/sdui-parser` copy (objectui#6613, message reworded by + // objectui#6614). The two copies must agree on the accepted grammar + // AND on diagnostic codes — if they drift, the save gate and the + // renderer speak different dialects and a page can save clean and + // render inert. Change this block only together with the objectui + // copy. diagnostics.push({ severity: 'warning', code: 'inert-expression', message: `<${node.type}> prop "${key}" is a braced expression this tier never evaluates — ` + - `the value will be silently ignored at render. Write it as JSON ` + - `(double-quoted strings and keys), e.g. columns={["name","amount"]} not columns={['name','amount']}`, + `the value will be silently ignored at render. This tier materializes LITERALS only ` + + `(strings, numbers, booleans, null, arrays, objects; quotes may be single or double, ` + + `object keys may be unquoted), e.g. columns={['name','amount']} works — ` + + `columns={rows.map((r) => r.name)} cannot`, tag: node.type, }); } else {