From b61da0dc3edb82ab42595b6d7d5e7ef0423950bd Mon Sep 17 00:00:00 2001 From: Vivek Date: Tue, 28 Jul 2026 16:47:02 +0530 Subject: [PATCH 01/38] fix: refuse a function in form action=, it leaked server source At SSR a `'use server'` import resolves to the REAL function (the RPC stub exists only in the browser), and `action=` is an ordinary escaped attribute hole, so the renderer stringified the function straight into the served HTML. A page doing what every Next-trained author writes, `
`, published the action's whole body, including any connection string or internal path its closure showed, to every visitor. Both hole shapes leaked identically (`action=${fn}` and `action="${fn}"`), as did a mixed value and `formaction=` on a submit button. Refuse it instead, on the server and on the client, so a client re-render cannot write the source into the live DOM either. The guard is name-based and deliberately narrow: only `action` and `formaction`, where a stringified function is never useful on any tag. Every other attribute keeps its existing behaviour, and a string-valued action is byte-identical to before. The thrown message names the fix without echoing the source. --- .../webjs/references/muscle-memory-gotchas.md | 17 ++++ packages/core/src/form-action.js | 44 ++++++++++ packages/core/src/render-client.js | 14 +++- packages/core/src/render-server.js | 7 ++ .../form-action-attr-guard-client.test.js | 51 ++++++++++++ .../rendering/form-action-attr-guard.test.js | 80 +++++++++++++++++++ 6 files changed, 211 insertions(+), 2 deletions(-) create mode 100644 packages/core/src/form-action.js create mode 100644 packages/core/test/rendering/form-action-attr-guard-client.test.js create mode 100644 packages/core/test/rendering/form-action-attr-guard.test.js diff --git a/.agents/skills/webjs/references/muscle-memory-gotchas.md b/.agents/skills/webjs/references/muscle-memory-gotchas.md index bb1cf6f2e..2d11d8593 100644 --- a/.agents/skills/webjs/references/muscle-memory-gotchas.md +++ b/.agents/skills/webjs/references/muscle-memory-gotchas.md @@ -43,6 +43,23 @@ const users = await getUsers(); There is no React `cache()`, `use()`, or `unstable_cache`. Caching is the `cache()` query helper, `export const revalidate` on a page, or `export const cache` on a GET action. +### A plain function in `` is refused, not stringified + +Next binds a Server Action with ``, and React serializes the binding into hidden fields. WebJs reads the same shape, but only for a real `'use server'` action. Any other function is a hard render error. + +The reason is a source leak. During SSR a `.server.ts` import is the ACTUAL function (the RPC stub exists only in the browser), and `action=` is an ordinary attribute hole, so stringifying it would write the function's body, secrets included, into the HTML every visitor downloads. So the renderer throws instead, on the server and on the client, for `action=` and `formaction=` alike, in every hole shape (`action=${fn}`, `action="${fn}"`, `action="/x/${fn}"`). + +```ts +// WRONG: a local handler. Throws at render; it would have leaked its body. +const onSubmit = async (fd: FormData) => { /* ... */ }; +html``; +// RIGHT: a 'use server' action imported from a *.server.ts module. +import { submitFeedback } from '#modules/feedback/actions/submit-feedback.server.ts'; +html``; +``` + +A string stays a string: `action="/search"` and `action=${'/search'}` are unchanged. Other attributes keep their existing stringify behaviour; only `action` and `formaction` are claimed. + ### `params` and `searchParams` are awaitable AND synchronously readable Next 15/16 made `params` / `searchParams` Promises. WebJs supports BOTH, so either muscle memory is correct. diff --git a/packages/core/src/form-action.js b/packages/core/src/form-action.js new file mode 100644 index 000000000..fdbe77ec1 --- /dev/null +++ b/packages/core/src/form-action.js @@ -0,0 +1,44 @@ +/** + * Form-action binding shared bits (#1154, #1155). + * + * Both renderers (server + client) import from here so the guard cannot + * drift between SSR and hydration. + */ + +/** + * The hidden field that carries a form-bound action's identity + * (`/`) from the SSR'd markup to the server dispatcher (#1155). + * Reserved name; user form fields must not use it. + */ +export const ACTION_FIELD = '__webjs_action'; + +/** + * Refuse to stringify a function interpolated into a form-action attribute + * (#1154). At SSR a `'use server'` import is the REAL server function (the + * RPC stub only exists in the browser), and a plain attribute hole commits + * via `String(val)`, so without this guard the action's SOURCE, secrets + * included, is serialized into the served HTML. The client renderer guards + * the same commit so a client re-render cannot write the source into the + * live DOM either. + * + * Name-based on purpose (`action`, plus `formaction`, the submit-button + * override, which leaks identically): a function stringified under these + * names is never useful on any tag, and other attributes keep today's + * stringify behaviour so the claim stays narrow. + * + * @param {unknown} val the hole's resolved value + * @param {string} attrName the attribute being committed (any case) + * @param {string} [tag] lowercased owner tag, for the error message + */ +export function assertNotFunctionActionAttr(val, attrName, tag) { + if (typeof val !== 'function') return; + const name = String(attrName).toLowerCase(); + if (name !== 'action' && name !== 'formaction') return; + throw new Error( + `[webjs] a function was interpolated into ${name}= on <${tag || 'form'}>. ` + + `Stringifying a function into HTML would leak its source (a 'use server' ` + + `action's body included) to every visitor, so this is refused. ` + + `Bind a 'use server' action exported from a *.server.{js,ts} module, ` + + `or pass a URL string.` + ); +} diff --git a/packages/core/src/render-client.js b/packages/core/src/render-client.js index 7065d8a1c..e7a9b7ac2 100644 --- a/packages/core/src/render-client.js +++ b/packages/core/src/render-client.js @@ -1,6 +1,7 @@ import { isTemplate, MARKER } from './html.js'; import { BINDING_PREFIXES, isBindingPrefix } from './binding-prefixes.js'; import { escapeAttr } from './escape.js'; +import { assertNotFunctionActionAttr } from './form-action.js'; import { isRepeat } from './repeat.js'; import { isUnsafeHTML, isLive, isKeyed, isGuard, isTemplateContent, isRef, isCache, isUntil, isAsyncAppend, isAsyncReplace, isWatch } from './directives.js'; import { Signal } from './signal.js'; @@ -671,7 +672,13 @@ function applyPart(part, value, _prev, allValues) { break; case 'attr': { if (value == null || value === false) part.el.removeAttribute(part.name); - else part.el.setAttribute(part.name, String(value)); + else { + // #1154: refuse to stringify a function into action=/formaction= + // (mirrors the SSR guard, so a client re-render cannot write a + // server action's source into the live DOM). + assertNotFunctionActionAttr(value, part.name, part.el.localName); + part.el.setAttribute(part.name, String(value)); + } break; } case 'prop': @@ -692,7 +699,10 @@ function applyPart(part, value, _prev, allValues) { const mp = /** @type {{ statics: string[], group: number[] }} */ (/** @type any */ (part)); let val = mp.statics[0]; for (let j = 0; j < mp.group.length; j++) { - val += String((allValues ? allValues[mp.group[j]] : value) ?? ''); + const piece = allValues ? allValues[mp.group[j]] : value; + // #1154: same function guard for each piece of a mixed attribute. + assertNotFunctionActionAttr(piece, part.name, part.el.localName); + val += String(piece ?? ''); val += mp.statics[j + 1] || ''; } part.el.setAttribute(part.name, val); diff --git a/packages/core/src/render-server.js b/packages/core/src/render-server.js index d4c5e81b5..b41895169 100644 --- a/packages/core/src/render-server.js +++ b/packages/core/src/render-server.js @@ -1,6 +1,7 @@ import { html, isTemplate } from './html.js'; import { BINDING_PREFIXES } from './binding-prefixes.js'; import { escapeText, escapeAttr } from './escape.js'; +import { assertNotFunctionActionAttr } from './form-action.js'; import { lookup, lookupModuleUrl, allTags } from './registry.js'; import { stylesToString, isCSS } from './css.js'; import { isRepeat } from './repeat.js'; @@ -323,11 +324,17 @@ async function renderTemplate(tr, ctx) { state = 'in-tag'; attrName = ''; } else { + // #1154: never stringify a function into action=/formaction= (it + // would serialize a server action's source into the served HTML). + assertNotFunctionActionAttr(val, attrName, currentTag); out += `"${escapeAttr(String(val ?? ''))}"`; state = 'in-tag'; attrName = ''; } } else if (state === 'attr-quoted' || state === 'attr-unquoted') { + // Same guard for a hole inside a quoted/unquoted value, the + // `action="${fn}"` and mixed `action="/x/${fn}"` shapes (#1154). + assertNotFunctionActionAttr(val, attrName, currentTag); out += escapeAttr(String(val ?? '')); } } diff --git a/packages/core/test/rendering/form-action-attr-guard-client.test.js b/packages/core/test/rendering/form-action-attr-guard-client.test.js new file mode 100644 index 000000000..8974e4f78 --- /dev/null +++ b/packages/core/test/rendering/form-action-attr-guard-client.test.js @@ -0,0 +1,51 @@ +// #1154, client half: the browser renderer refuses the same commit, so a +// client re-render can never write a function's source into the live DOM +// (`applyPart`'s 'attr' and 'attr-mixed' cases). Runs under linkedom. +import { test, before } from 'node:test'; +import assert from 'node:assert/strict'; +import { parseHTML } from 'linkedom'; + +before(() => { + const { window } = parseHTML(''); + globalThis.document = window.document; + globalThis.DocumentFragment = window.DocumentFragment; + globalThis.Node = window.Node; + globalThis.Element = window.Element; + globalThis.Comment = window.Comment; + globalThis.Text = window.Text; + globalThis.NodeFilter = window.NodeFilter; + globalThis.HTMLElement = window.HTMLElement; +}); + +let html, render; +before(async () => { + ({ html } = await import('../../src/html.js')); + ({ render } = await import('../../src/render-client.js')); +}); + +async function fakeAction() { const S = 'CLIENT_SECRET'; return S; } + +test('client render of action=${fn} throws, DOM never carries the source', () => { + const host = document.createElement('div'); + assert.throws( + () => render(html``, host), + /function was interpolated into action=/, + ); + assert.ok(!host.innerHTML.includes('CLIENT_SECRET')); +}); + +test('client render of mixed action="/x/${fn}" throws', () => { + const host = document.createElement('div'); + assert.throws( + () => render(html`
`, host), + /function was interpolated into action=/, + ); + assert.ok(!host.innerHTML.includes('CLIENT_SECRET')); +}); + +test('string action still renders on the client', () => { + const host = document.createElement('div'); + render(html`
`, host); + const form = host.querySelector('form'); + assert.equal(form.getAttribute('action'), '/submit'); +}); diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js new file mode 100644 index 000000000..7ad7a6643 --- /dev/null +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -0,0 +1,80 @@ +// #1154: a function interpolated into a form-action attribute must throw, +// never stringify. At SSR a `'use server'` import is the REAL function, so +// `String(fn)` would serialize the action's source (secrets included) into +// the served HTML. Covers every hole shape that used to leak (unquoted, +// quoted, mixed, and `formaction` on a submit button), plus the byte-identical +// passthrough for string-valued action attributes. +import { test, before } from 'node:test'; +import assert from 'node:assert/strict'; + +let html, renderToString; +before(async () => { + ({ html } = await import('../../src/html.js')); + ({ renderToString } = await import('../../src/render-server.js')); +}); + +// The secret sentinel must never appear in any output, thrown or not. +const SECRET = 'postgres://user:SECRET@host/db'; +async function leaky(input) { const conn = SECRET; return { success: true, conn, input }; } + +test('unquoted action=${fn} throws instead of leaking source', async () => { + await assert.rejects( + () => renderToString(html`
`, { ssr: true }), + /function was interpolated into action=/, + ); +}); + +test('quoted action="${fn}" throws identically', async () => { + await assert.rejects( + () => renderToString(html`
`, { ssr: true }), + /function was interpolated into action=/, + ); +}); + +test('mixed hole action="/x/${fn}" throws', async () => { + await assert.rejects( + () => renderToString(html`
`, { ssr: true }), + /function was interpolated into action=/, + ); +}); + +test('formaction=${fn} on a submit button throws', async () => { + await assert.rejects( + () => renderToString(html`
`, { ssr: true }), + /function was interpolated into formaction=/, + ); +}); + +test('no thrown message ever carries the function source', async () => { + for (const tpl of [ + html`
`, + html`
`, + ]) { + let msg = ''; + try { await renderToString(tpl, { ssr: true }); } catch (e) { msg = String(e && e.message); } + assert.ok(msg.length > 0, 'render must throw'); + assert.ok(!msg.includes('SECRET'), 'error message must not embed the source'); + } +}); + +test('string-valued action renders byte-identically to before', async () => { + assert.equal( + await renderToString(html`
`, { ssr: true }), + '
', + ); + assert.equal( + await renderToString(html`
`, { ssr: true }), + '
', + ); + assert.equal( + await renderToString(html`
`, { ssr: true }), + '
', + ); +}); + +test('functions in OTHER attributes keep the existing stringify behaviour', async () => { + // Narrow claim: only action/formaction throw. Anything else is unchanged + // (arguably also a bug, but out of #1154's scope by design). + const out = await renderToString(html`
`, { ssr: true }); + assert.ok(out.startsWith('
/` identity + * @param {(name: string) => boolean} has whether the form already set an attribute + * @returns {{ forced: string, field: string }} + */ +export function formActionMarkup(id, has) { + let forced = ''; + if (!has('method')) forced += ' method="post"'; + if (!has('enctype')) forced += ' enctype="multipart/form-data"'; + const field = ``; + return { forced, field }; +} + +/** Minimal attribute escape; the identity is framework-generated, so this is belt and braces. */ +function escapeAttrValue(s) { + return String(s).replace(/&/g, '&').replace(/"/g, '"').replace(/} seenAttrs attribute names already written on this tag + * @returns {{ attrValue: string, forced: string, field: string } | null} + */ +export function commitFormActionAttr(val, attrName, tag, seenAttrs) { + if (typeof val !== 'function') return null; + if (!isFormActionAttr(attrName)) { + // A function on any other attribute keeps the pre-existing stringify + // behaviour: widening the claim here would change unrelated rendering. + return null; + } + const id = resolveFormActionOrThrow(val, attrName, tag); + const { forced, field } = formActionMarkup(id, (n) => seenAttrs.has(n)); + // The form posts to its OWN url; the identity rides the hidden field, which + // is what lets one action be bound from any page (#1155) while the failure + // re-render still targets the page the form was on. + return { attrValue: '""', forced, field }; } diff --git a/packages/core/src/render-server.js b/packages/core/src/render-server.js index b41895169..4659e751b 100644 --- a/packages/core/src/render-server.js +++ b/packages/core/src/render-server.js @@ -1,7 +1,7 @@ import { html, isTemplate } from './html.js'; import { BINDING_PREFIXES } from './binding-prefixes.js'; import { escapeText, escapeAttr } from './escape.js'; -import { assertNotFunctionActionAttr } from './form-action.js'; +import { assertNotFunctionActionAttr, commitFormActionAttr } from './form-action.js'; import { lookup, lookupModuleUrl, allTags } from './registry.js'; import { stylesToString, isCSS } from './css.js'; import { isRepeat } from './repeat.js'; @@ -152,6 +152,13 @@ async function renderTemplate(tr, ctx) { let commentDashes = 0; let currentTag = ''; // lowercased tag name currently being parsed let rawTail = ''; // rolling lowercased tail, tracks / + // Form-action binding (#1155). A `
` commits its + // attribute mid-tag, but the identity field it needs is a CHILD, so the + // markup is buffered here and flushed the moment the open tag closes. + // `seenAttrs` tracks what the author already wrote on the current tag so a + // forced `method` / `enctype` never duplicates an explicit one. + let pendingFormField = ''; + let seenAttrs = new Set(); for (let i = 0; i < strings.length; i++) { const s = strings[i]; @@ -195,6 +202,9 @@ async function renderTemplate(tr, ctx) { case 'in-tag': out += c; if (c === '>') { + // The open tag just closed, so a buffered form-action identity + // field can now be emitted as the form's first child (#1155). + if (pendingFormField) { out += pendingFormField; pendingFormField = ''; } state = isRawtextTag(currentTag) ? 'rawtext' : 'text'; if (state === 'rawtext') rawTail = ''; } else if (!/\s/.test(c) && c !== '/') { @@ -213,20 +223,34 @@ async function renderTemplate(tr, ctx) { } break; case 'attr-name': - if (c === '=') { state = 'after-eq'; out += c; } - else if (/\s/.test(c)) { state = 'in-tag'; attrName = ''; out += c; } - else if (c === '>') { state = 'text'; attrName = ''; out += c; } + // Record every attribute the author wrote on this tag, so a forced + // method/enctype (#1155) never duplicates an explicit one. + if (c === '=') { state = 'after-eq'; seenAttrs.add(attrName.toLowerCase()); out += c; } + else if (/\s/.test(c)) { state = 'in-tag'; seenAttrs.add(attrName.toLowerCase()); attrName = ''; out += c; } + else if (c === '>') { + state = 'text'; + seenAttrs.add(attrName.toLowerCase()); + attrName = ''; + out += c; + if (pendingFormField) { out += pendingFormField; pendingFormField = ''; } + } else { attrName += c; out += c; } break; case 'after-eq': if (c === '"' || c === "'") { state = 'attr-quoted'; attrQuote = c; out += c; } else if (/\s/.test(c)) { state = 'in-tag'; attrName = ''; out += c; } - else if (c === '>') { state = 'text'; attrName = ''; out += c; } + else if (c === '>') { + state = 'text'; attrName = ''; out += c; + if (pendingFormField) { out += pendingFormField; pendingFormField = ''; } + } else { state = 'attr-unquoted'; out += c; } break; case 'attr-unquoted': if (/\s/.test(c)) { state = 'in-tag'; attrName = ''; out += c; } - else if (c === '>') { state = 'text'; attrName = ''; out += c; } + else if (c === '>') { + state = 'text'; attrName = ''; out += c; + if (pendingFormField) { out += pendingFormField; pendingFormField = ''; } + } else out += c; break; case 'attr-quoted': @@ -324,16 +348,22 @@ async function renderTemplate(tr, ctx) { state = 'in-tag'; attrName = ''; } else { - // #1154: never stringify a function into action=/formaction= (it - // would serialize a server action's source into the served HTML). - assertNotFunctionActionAttr(val, attrName, currentTag); - out += `"${escapeAttr(String(val ?? ''))}"`; + // A function bound to action=/formaction= is either an identifiable + // server action (emit the identity field, #1155) or refused + // outright (#1154); it is never stringified. + const bound = commitFormActionAttr(val, attrName, currentTag, seenAttrs); + if (bound) { + out += bound.attrValue + bound.forced; + pendingFormField = bound.field; + } else { + out += `"${escapeAttr(String(val ?? ''))}"`; + } state = 'in-tag'; attrName = ''; } } else if (state === 'attr-quoted' || state === 'attr-unquoted') { - // Same guard for a hole inside a quoted/unquoted value, the - // `action="${fn}"` and mixed `action="/x/${fn}"` shapes (#1154). + // A hole INSIDE a value (`action="${fn}"`, mixed `action="/x/${fn}"`) + // cannot carry an identity field, so a function here is always refused. assertNotFunctionActionAttr(val, attrName, currentTag); out += escapeAttr(String(val ?? '')); } @@ -1775,6 +1805,9 @@ async function streamTemplate(tr, ctx, controller) { let rawTail = ''; // Buffer used for attribute handling where we may need to backtrack. let buf = ''; + // Form-action binding (#1155), mirroring renderTemplate. + let pendingFormField = ''; + let seenAttrs = new Set(); for (let i = 0; i < strings.length; i++) { const s = strings[i]; @@ -1788,8 +1821,8 @@ async function streamTemplate(tr, ctx, controller) { case 'tag-open': buf += c; if (c === '!') state = 'bang-1'; - else if (c === '/') { state = 'tag-name'; currentTag = ''; } - else if (/[a-zA-Z]/.test(c)) { state = 'tag-name'; currentTag = c.toLowerCase(); } + else if (c === '/') { state = 'tag-name'; currentTag = ''; seenAttrs = new Set(); } + else if (/[a-zA-Z]/.test(c)) { state = 'tag-name'; currentTag = c.toLowerCase(); seenAttrs = new Set(); } else state = 'text'; break; case 'bang-1': @@ -1818,6 +1851,7 @@ async function streamTemplate(tr, ctx, controller) { case 'in-tag': buf += c; if (c === '>') { + if (pendingFormField) { buf += pendingFormField; pendingFormField = ''; } state = isRawtextTag(currentTag) ? 'rawtext' : 'text'; if (state === 'rawtext') rawTail = ''; } else if (!/\s/.test(c) && c !== '/') { @@ -1836,20 +1870,29 @@ async function streamTemplate(tr, ctx, controller) { } break; case 'attr-name': - if (c === '=') { state = 'after-eq'; buf += c; } - else if (/\s/.test(c)) { state = 'in-tag'; attrName = ''; buf += c; } - else if (c === '>') { state = 'text'; attrName = ''; buf += c; } + if (c === '=') { state = 'after-eq'; seenAttrs.add(attrName.toLowerCase()); buf += c; } + else if (/\s/.test(c)) { state = 'in-tag'; seenAttrs.add(attrName.toLowerCase()); attrName = ''; buf += c; } + else if (c === '>') { + state = 'text'; seenAttrs.add(attrName.toLowerCase()); attrName = ''; buf += c; + if (pendingFormField) { buf += pendingFormField; pendingFormField = ''; } + } else { attrName += c; buf += c; } break; case 'after-eq': if (c === '"' || c === "'") { state = 'attr-quoted'; attrQuote = c; buf += c; } else if (/\s/.test(c)) { state = 'in-tag'; attrName = ''; buf += c; } - else if (c === '>') { state = 'text'; attrName = ''; buf += c; } + else if (c === '>') { + state = 'text'; attrName = ''; buf += c; + if (pendingFormField) { buf += pendingFormField; pendingFormField = ''; } + } else { state = 'attr-unquoted'; buf += c; } break; case 'attr-unquoted': if (/\s/.test(c)) { state = 'in-tag'; attrName = ''; buf += c; } - else if (c === '>') { state = 'text'; attrName = ''; buf += c; } + else if (c === '>') { + state = 'text'; attrName = ''; buf += c; + if (pendingFormField) { buf += pendingFormField; pendingFormField = ''; } + } else buf += c; break; case 'attr-quoted': @@ -1891,11 +1934,21 @@ async function streamTemplate(tr, ctx, controller) { state = 'in-tag'; attrName = ''; } else { - buf += `"${escapeAttr(String(val ?? ''))}"`; + // Same contract as the buffered renderer (they must not diverge; see + // commitFormActionAttr). Before this, a STREAMED page emitted a bound + // action's source verbatim while the buffered path already refused it. + const bound = commitFormActionAttr(val, attrName, currentTag, seenAttrs); + if (bound) { + buf += bound.attrValue + bound.forced; + pendingFormField = bound.field; + } else { + buf += `"${escapeAttr(String(val ?? ''))}"`; + } state = 'in-tag'; attrName = ''; } } else if (state === 'attr-quoted' || state === 'attr-unquoted') { + assertNotFunctionActionAttr(val, attrName, currentTag); buf += escapeAttr(String(val ?? '')); } } diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index 7ad7a6643..878292a9d 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -7,12 +7,18 @@ import { test, before } from 'node:test'; import assert from 'node:assert/strict'; -let html, renderToString; +let html, renderToString, renderToStream; before(async () => { ({ html } = await import('../../src/html.js')); - ({ renderToString } = await import('../../src/render-server.js')); + ({ renderToString, renderToStream } = await import('../../src/render-server.js')); }); +async function drain(stream) { + let out = ''; + for await (const c of stream) out += typeof c === 'string' ? c : new TextDecoder().decode(c); + return out; +} + // The secret sentinel must never appear in any output, thrown or not. const SECRET = 'postgres://user:SECRET@host/db'; async function leaky(input) { const conn = SECRET; return { success: true, conn, input }; } @@ -72,6 +78,30 @@ test('string-valued action renders byte-identically to before', async () => { ); }); +// The STREAMING renderer is a second, independent state machine. It was missed +// by the first pass of this fix and kept emitting the action's whole body while +// the buffered renderer already refused it, so it gets its own coverage rather +// than being assumed to inherit the guard. +test('the streaming renderer refuses the same function (ssr:false path)', async () => { + await assert.rejects( + () => drain(renderToStream(html`
`, { ssr: false })), + /function was interpolated into action=/, + ); +}); + +test('the streaming renderer never emits the source', async () => { + let out = ''; + try { + out = await drain(renderToStream(html`
`, { ssr: false })); + } catch { /* expected */ } + assert.ok(!out.includes('SECRET'), 'streamed output must not carry the function source'); +}); + +test('streaming keeps string-valued actions working', async () => { + const out = await drain(renderToStream(html`
`, { ssr: false })); + assert.match(out, /action="\/submit"/); +}); + test('functions in OTHER attributes keep the existing stringify behaviour', async () => { // Narrow claim: only action/formaction throw. Anything else is unchanged // (arguably also a bug, but out of #1154's scope by design). From d2f302aa1106d3fd70a52df70b4a54649e914adb Mon Sep 17 00:00:00 2001 From: Vivek Date: Tue, 28 Jul 2026 17:08:12 +0530 Subject: [PATCH 03/38] refactor: drop the unreleased form-binding machinery from this branch The identity resolver, the hidden-field emission and the forced method/enctype belong to the form-submission unification, which is a separate change. They were unreachable here anyway (nothing installs a resolver), and shipping dormant code alongside a security fix makes the fix harder to review and to reason about on its own. What stays is the whole of the leak fix, including the streaming-path guard, so this branch is now exactly the bug fix and is independently mergeable. The removed code is recoverable from cf72805a. --- .../webjs/references/muscle-memory-gotchas.md | 13 +- packages/core/src/form-action.js | 135 +++--------------- packages/core/src/render-server.js | 97 ++++--------- 3 files changed, 47 insertions(+), 198 deletions(-) diff --git a/.agents/skills/webjs/references/muscle-memory-gotchas.md b/.agents/skills/webjs/references/muscle-memory-gotchas.md index 2d11d8593..5a947ea3c 100644 --- a/.agents/skills/webjs/references/muscle-memory-gotchas.md +++ b/.agents/skills/webjs/references/muscle-memory-gotchas.md @@ -43,19 +43,18 @@ const users = await getUsers(); There is no React `cache()`, `use()`, or `unstable_cache`. Caching is the `cache()` query helper, `export const revalidate` on a page, or `export const cache` on a GET action. -### A plain function in `
` is refused, not stringified +### A function in `` is refused, not stringified -Next binds a Server Action with ``, and React serializes the binding into hidden fields. WebJs reads the same shape, but only for a real `'use server'` action. Any other function is a hard render error. +Next binds a Server Action with `` and React serializes the binding into hidden fields. WebJs does not read that shape. A function interpolated into `action=` is a hard render error. -The reason is a source leak. During SSR a `.server.ts` import is the ACTUAL function (the RPC stub exists only in the browser), and `action=` is an ordinary attribute hole, so stringifying it would write the function's body, secrets included, into the HTML every visitor downloads. So the renderer throws instead, on the server and on the client, for `action=` and `formaction=` alike, in every hole shape (`action=${fn}`, `action="${fn}"`, `action="/x/${fn}"`). +The reason is a source leak. During SSR a `.server.ts` import is the ACTUAL function (the RPC stub exists only in the browser), and `action=` is an ordinary attribute hole, so stringifying it would write the function's body, secrets included, into the HTML every visitor downloads. The renderer throws instead, on the server and on the client, for `action=` and `formaction=` alike, in every hole shape (`action=${fn}`, `action="${fn}"`, `action="/x/${fn}"`). ```ts -// WRONG: a local handler. Throws at render; it would have leaked its body. -const onSubmit = async (fd: FormData) => { /* ... */ }; -html``; -// RIGHT: a 'use server' action imported from a *.server.ts module. +// WRONG: throws at render; it would have leaked the action's body. import { submitFeedback } from '#modules/feedback/actions/submit-feedback.server.ts'; html``; +// RIGHT: post to the page's own url and handle it in the page's `action` export. +html``; ``` A string stays a string: `action="/search"` and `action=${'/search'}` are unchanged. Other attributes keep their existing stringify behaviour; only `action` and `formaction` are claimed. diff --git a/packages/core/src/form-action.js b/packages/core/src/form-action.js index 4159bed7b..d56debaf5 100644 --- a/packages/core/src/form-action.js +++ b/packages/core/src/form-action.js @@ -1,25 +1,21 @@ /** - * Form-action binding shared bits (#1154, #1155). + * Form-action attribute guard (#1154). * - * Both renderers (server + client) import from here so the guard cannot - * drift between SSR and hydration. + * Both SSR state machines and the client renderer import from here so the + * rule cannot drift between them. It already drifted once: the guard was + * added to the buffered renderer alone, leaving the Suspense-streaming path + * emitting a server action's whole body into the response. */ /** - * The hidden field that carries a form-bound action's identity - * (`/`) from the SSR'd markup to the server dispatcher (#1155). - * Reserved name; user form fields must not use it. - */ -export const ACTION_FIELD = '__webjs_action'; - -/** - * Refuse to stringify a function interpolated into a form-action attribute - * (#1154). At SSR a `'use server'` import is the REAL server function (the - * RPC stub only exists in the browser), and a plain attribute hole commits - * via `String(val)`, so without this guard the action's SOURCE, secrets - * included, is serialized into the served HTML. The client renderer guards - * the same commit so a client re-render cannot write the source into the - * live DOM either. + * Refuse to stringify a function interpolated into a form-action attribute. + * + * At SSR a `'use server'` import is the REAL server function (the RPC stub + * only exists in the browser), and a plain attribute hole commits via + * `String(val)`, so without this guard the action's SOURCE, secrets included, + * is serialized into the served HTML. The client renderer guards the same + * commit so a client re-render cannot write the source into the live DOM + * either. * * Name-based on purpose (`action`, plus `formaction`, the submit-button * override, which leaks identically): a function stringified under these @@ -50,106 +46,7 @@ export function isFormActionAttr(attrName) { */ function formActionError(attrName, tag) { return `[webjs] a function was interpolated into ${String(attrName).toLowerCase()}= ` - + `on <${tag || 'form'}>, and it is not a 'use server' action. Stringifying a ` - + `function into HTML would leak its source (a server action's body included) ` - + `to every visitor, so this is refused. Bind a 'use server' action exported ` - + `from a *.server.{js,ts} module, or pass a URL string.`; -} - -/** - * Resolves a form-bound action function to its wire identity, `/` - * (#1155). Server-only wiring, installed exactly like the CSP nonce provider: - * `@webjsdev/server` calls `setFormActionResolver` at load time, the browser - * bundle never does, so a client-side render keeps refusing every function. - * - * @type {((fn: Function) => string | null) | null} - */ -let _resolver = null; - -/** - * Internal: server-only wiring. Installed once by `@webjsdev/server`. - * @param {(fn: Function) => string | null} fn - */ -export function setFormActionResolver(fn) { - _resolver = fn; -} - -/** - * Resolve a form-bound action to `/`, or throw the refusal above when - * it is not an identifiable `'use server'` action. - * - * Throwing on an unresolvable function is load-bearing for progressive - * enhancement, not defensive: emitting the form WITHOUT the identity field - * would render a form that posts to the page and dispatches nothing, a silent - * no-op with no error anywhere. Failing the render is the only outcome that - * cannot ship a dead form. - * - * @param {Function} val - * @param {string} attrName - * @param {string} [tag] - * @returns {string} the `/` identity - */ -export function resolveFormActionOrThrow(val, attrName, tag) { - const id = _resolver ? _resolver(val) : null; - if (!id) throw new Error(formActionError(attrName, tag)); - return id; -} - -/** - * The hidden field carrying the action identity, plus the attributes the - * framework forces rather than trusting the author to write. - * - * `method="post"`: a form with no method GETs, so the action would never run. - * `enctype="multipart/form-data"`: without it a no-JS file upload silently - * arrives as a filename string instead of the file. - * - * @param {string} id the `/` identity - * @param {(name: string) => boolean} has whether the form already set an attribute - * @returns {{ forced: string, field: string }} - */ -export function formActionMarkup(id, has) { - let forced = ''; - if (!has('method')) forced += ' method="post"'; - if (!has('enctype')) forced += ' enctype="multipart/form-data"'; - const field = ``; - return { forced, field }; -} - -/** Minimal attribute escape; the identity is framework-generated, so this is belt and braces. */ -function escapeAttrValue(s) { - return String(s).replace(/&/g, '&').replace(/"/g, '"').replace(/} seenAttrs attribute names already written on this tag - * @returns {{ attrValue: string, forced: string, field: string } | null} - */ -export function commitFormActionAttr(val, attrName, tag, seenAttrs) { - if (typeof val !== 'function') return null; - if (!isFormActionAttr(attrName)) { - // A function on any other attribute keeps the pre-existing stringify - // behaviour: widening the claim here would change unrelated rendering. - return null; - } - const id = resolveFormActionOrThrow(val, attrName, tag); - const { forced, field } = formActionMarkup(id, (n) => seenAttrs.has(n)); - // The form posts to its OWN url; the identity rides the hidden field, which - // is what lets one action be bound from any page (#1155) while the failure - // re-render still targets the page the form was on. - return { attrValue: '""', forced, field }; + + `on <${tag || 'form'}>. Stringifying a function into HTML would leak its ` + + `source (a 'use server' action's body included) to every visitor, so this ` + + `is refused. Pass a URL string instead.`; } diff --git a/packages/core/src/render-server.js b/packages/core/src/render-server.js index 4659e751b..3d9051f3e 100644 --- a/packages/core/src/render-server.js +++ b/packages/core/src/render-server.js @@ -1,7 +1,7 @@ import { html, isTemplate } from './html.js'; import { BINDING_PREFIXES } from './binding-prefixes.js'; import { escapeText, escapeAttr } from './escape.js'; -import { assertNotFunctionActionAttr, commitFormActionAttr } from './form-action.js'; +import { assertNotFunctionActionAttr } from './form-action.js'; import { lookup, lookupModuleUrl, allTags } from './registry.js'; import { stylesToString, isCSS } from './css.js'; import { isRepeat } from './repeat.js'; @@ -152,13 +152,6 @@ async function renderTemplate(tr, ctx) { let commentDashes = 0; let currentTag = ''; // lowercased tag name currently being parsed let rawTail = ''; // rolling lowercased tail, tracks / - // Form-action binding (#1155). A `` commits its - // attribute mid-tag, but the identity field it needs is a CHILD, so the - // markup is buffered here and flushed the moment the open tag closes. - // `seenAttrs` tracks what the author already wrote on the current tag so a - // forced `method` / `enctype` never duplicates an explicit one. - let pendingFormField = ''; - let seenAttrs = new Set(); for (let i = 0; i < strings.length; i++) { const s = strings[i]; @@ -202,9 +195,6 @@ async function renderTemplate(tr, ctx) { case 'in-tag': out += c; if (c === '>') { - // The open tag just closed, so a buffered form-action identity - // field can now be emitted as the form's first child (#1155). - if (pendingFormField) { out += pendingFormField; pendingFormField = ''; } state = isRawtextTag(currentTag) ? 'rawtext' : 'text'; if (state === 'rawtext') rawTail = ''; } else if (!/\s/.test(c) && c !== '/') { @@ -223,34 +213,20 @@ async function renderTemplate(tr, ctx) { } break; case 'attr-name': - // Record every attribute the author wrote on this tag, so a forced - // method/enctype (#1155) never duplicates an explicit one. - if (c === '=') { state = 'after-eq'; seenAttrs.add(attrName.toLowerCase()); out += c; } - else if (/\s/.test(c)) { state = 'in-tag'; seenAttrs.add(attrName.toLowerCase()); attrName = ''; out += c; } - else if (c === '>') { - state = 'text'; - seenAttrs.add(attrName.toLowerCase()); - attrName = ''; - out += c; - if (pendingFormField) { out += pendingFormField; pendingFormField = ''; } - } + if (c === '=') { state = 'after-eq'; out += c; } + else if (/\s/.test(c)) { state = 'in-tag'; attrName = ''; out += c; } + else if (c === '>') { state = 'text'; attrName = ''; out += c; } else { attrName += c; out += c; } break; case 'after-eq': if (c === '"' || c === "'") { state = 'attr-quoted'; attrQuote = c; out += c; } else if (/\s/.test(c)) { state = 'in-tag'; attrName = ''; out += c; } - else if (c === '>') { - state = 'text'; attrName = ''; out += c; - if (pendingFormField) { out += pendingFormField; pendingFormField = ''; } - } + else if (c === '>') { state = 'text'; attrName = ''; out += c; } else { state = 'attr-unquoted'; out += c; } break; case 'attr-unquoted': if (/\s/.test(c)) { state = 'in-tag'; attrName = ''; out += c; } - else if (c === '>') { - state = 'text'; attrName = ''; out += c; - if (pendingFormField) { out += pendingFormField; pendingFormField = ''; } - } + else if (c === '>') { state = 'text'; attrName = ''; out += c; } else out += c; break; case 'attr-quoted': @@ -348,22 +324,16 @@ async function renderTemplate(tr, ctx) { state = 'in-tag'; attrName = ''; } else { - // A function bound to action=/formaction= is either an identifiable - // server action (emit the identity field, #1155) or refused - // outright (#1154); it is never stringified. - const bound = commitFormActionAttr(val, attrName, currentTag, seenAttrs); - if (bound) { - out += bound.attrValue + bound.forced; - pendingFormField = bound.field; - } else { - out += `"${escapeAttr(String(val ?? ''))}"`; - } + // #1154: never stringify a function into action=/formaction= (it + // would serialize a server action's source into the served HTML). + assertNotFunctionActionAttr(val, attrName, currentTag); + out += `"${escapeAttr(String(val ?? ''))}"`; state = 'in-tag'; attrName = ''; } } else if (state === 'attr-quoted' || state === 'attr-unquoted') { - // A hole INSIDE a value (`action="${fn}"`, mixed `action="/x/${fn}"`) - // cannot carry an identity field, so a function here is always refused. + // Same guard for a hole inside a quoted/unquoted value, the + // `action="${fn}"` and mixed `action="/x/${fn}"` shapes (#1154). assertNotFunctionActionAttr(val, attrName, currentTag); out += escapeAttr(String(val ?? '')); } @@ -1805,9 +1775,6 @@ async function streamTemplate(tr, ctx, controller) { let rawTail = ''; // Buffer used for attribute handling where we may need to backtrack. let buf = ''; - // Form-action binding (#1155), mirroring renderTemplate. - let pendingFormField = ''; - let seenAttrs = new Set(); for (let i = 0; i < strings.length; i++) { const s = strings[i]; @@ -1821,8 +1788,8 @@ async function streamTemplate(tr, ctx, controller) { case 'tag-open': buf += c; if (c === '!') state = 'bang-1'; - else if (c === '/') { state = 'tag-name'; currentTag = ''; seenAttrs = new Set(); } - else if (/[a-zA-Z]/.test(c)) { state = 'tag-name'; currentTag = c.toLowerCase(); seenAttrs = new Set(); } + else if (c === '/') { state = 'tag-name'; currentTag = ''; } + else if (/[a-zA-Z]/.test(c)) { state = 'tag-name'; currentTag = c.toLowerCase(); } else state = 'text'; break; case 'bang-1': @@ -1851,7 +1818,6 @@ async function streamTemplate(tr, ctx, controller) { case 'in-tag': buf += c; if (c === '>') { - if (pendingFormField) { buf += pendingFormField; pendingFormField = ''; } state = isRawtextTag(currentTag) ? 'rawtext' : 'text'; if (state === 'rawtext') rawTail = ''; } else if (!/\s/.test(c) && c !== '/') { @@ -1870,29 +1836,20 @@ async function streamTemplate(tr, ctx, controller) { } break; case 'attr-name': - if (c === '=') { state = 'after-eq'; seenAttrs.add(attrName.toLowerCase()); buf += c; } - else if (/\s/.test(c)) { state = 'in-tag'; seenAttrs.add(attrName.toLowerCase()); attrName = ''; buf += c; } - else if (c === '>') { - state = 'text'; seenAttrs.add(attrName.toLowerCase()); attrName = ''; buf += c; - if (pendingFormField) { buf += pendingFormField; pendingFormField = ''; } - } + if (c === '=') { state = 'after-eq'; buf += c; } + else if (/\s/.test(c)) { state = 'in-tag'; attrName = ''; buf += c; } + else if (c === '>') { state = 'text'; attrName = ''; buf += c; } else { attrName += c; buf += c; } break; case 'after-eq': if (c === '"' || c === "'") { state = 'attr-quoted'; attrQuote = c; buf += c; } else if (/\s/.test(c)) { state = 'in-tag'; attrName = ''; buf += c; } - else if (c === '>') { - state = 'text'; attrName = ''; buf += c; - if (pendingFormField) { buf += pendingFormField; pendingFormField = ''; } - } + else if (c === '>') { state = 'text'; attrName = ''; buf += c; } else { state = 'attr-unquoted'; buf += c; } break; case 'attr-unquoted': if (/\s/.test(c)) { state = 'in-tag'; attrName = ''; buf += c; } - else if (c === '>') { - state = 'text'; attrName = ''; buf += c; - if (pendingFormField) { buf += pendingFormField; pendingFormField = ''; } - } + else if (c === '>') { state = 'text'; attrName = ''; buf += c; } else buf += c; break; case 'attr-quoted': @@ -1934,16 +1891,12 @@ async function streamTemplate(tr, ctx, controller) { state = 'in-tag'; attrName = ''; } else { - // Same contract as the buffered renderer (they must not diverge; see - // commitFormActionAttr). Before this, a STREAMED page emitted a bound - // action's source verbatim while the buffered path already refused it. - const bound = commitFormActionAttr(val, attrName, currentTag, seenAttrs); - if (bound) { - buf += bound.attrValue + bound.forced; - pendingFormField = bound.field; - } else { - buf += `"${escapeAttr(String(val ?? ''))}"`; - } + // The SAME guard as the buffered renderer above. This is a second, + // independent state machine, so it does not inherit that one: before + // this, a STREAMED page emitted a bound function's whole source while + // the buffered path already refused it. + assertNotFunctionActionAttr(val, attrName, currentTag); + buf += `"${escapeAttr(String(val ?? ''))}"`; state = 'in-tag'; attrName = ''; } From edbebb2ff3f337833c689d8ae27eedef94d953da Mon Sep 17 00:00:00 2001 From: Vivek Date: Tue, 28 Jul 2026 17:40:04 +0530 Subject: [PATCH 04/38] refactor: keep isFormActionAttr internal to the guard module It has no consumer outside form-action.js, and the module is internal, so exporting it only widens what a future edit has to consider. --- packages/core/src/form-action.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/form-action.js b/packages/core/src/form-action.js index d56debaf5..4cb80e997 100644 --- a/packages/core/src/form-action.js +++ b/packages/core/src/form-action.js @@ -33,7 +33,7 @@ export function assertNotFunctionActionAttr(val, attrName, tag) { } /** @param {string} attrName @returns {boolean} */ -export function isFormActionAttr(attrName) { +function isFormActionAttr(attrName) { const name = String(attrName).toLowerCase(); return name === 'action' || name === 'formaction'; } From 147b25cf655b00a01db5eb3607c0c511eb6e792b Mon Sep 17 00:00:00 2001 From: Vivek Date: Tue, 28 Jul 2026 18:38:10 +0530 Subject: [PATCH 05/38] fix: guard the .action property binding, the third leak path Review found `.action=${fn}` on a native element unguarded. `action` is a reflected IDL attribute, so assigning a function stringifies its source into the element's own attribute in a real browser. The linkedom tests could not see it (linkedom does not reflect), which is why it survived the earlier passes and why this ships with a real-browser test. A custom element's `.action` is an ordinary author-defined property that never reflects, so a function there stays legitimate and is not refused. Also corrects three things the review caught in this PR's own claims: the streaming machine is reached only through `renderToStream(v, {ssr:false})` and no page render uses it, so the earlier "a streamed page leaked" wording overstated a public-API hole as a live leak; the streaming quoted and mixed holes had no coverage, so that branch could have been deleted with the suite still green; and two client assertions were tautological, since the host is empty whenever the render throws. --- packages/core/src/form-action.js | 8 +- packages/core/src/render-client.js | 9 ++ packages/core/src/render-server.js | 6 +- .../browser/form-action-guard.test.js | 87 +++++++++++++++++++ .../form-action-attr-guard-client.test.js | 42 ++++++++- .../rendering/form-action-attr-guard.test.js | 36 +++++++- 6 files changed, 177 insertions(+), 11 deletions(-) create mode 100644 packages/core/test/rendering/browser/form-action-guard.test.js diff --git a/packages/core/src/form-action.js b/packages/core/src/form-action.js index 4cb80e997..a2c19df2d 100644 --- a/packages/core/src/form-action.js +++ b/packages/core/src/form-action.js @@ -3,8 +3,12 @@ * * Both SSR state machines and the client renderer import from here so the * rule cannot drift between them. It already drifted once: the guard was - * added to the buffered renderer alone, leaving the Suspense-streaming path - * emitting a server action's whole body into the response. + * added to the buffered `renderTemplate` alone, leaving `streamTemplate` + * stringifying the function. That second machine is reached only through + * `renderToStream(v, { ssr: false })`, which no page render uses (the server + * renders every page, Suspense included, via `renderToString`), so it was a + * public-API hole rather than a live page leak. Worth closing on its own + * terms, and worth noting as the reason the rule lives in one place. */ /** diff --git a/packages/core/src/render-client.js b/packages/core/src/render-client.js index e7a9b7ac2..17e0f2b6e 100644 --- a/packages/core/src/render-client.js +++ b/packages/core/src/render-client.js @@ -682,6 +682,15 @@ function applyPart(part, value, _prev, allValues) { break; } case 'prop': + // `.action=${fn}` on a NATIVE element is a leak too, not just the + // attribute form: `action` is a reflected IDL attribute, so assigning a + // function stringifies it into the element's own `action` content + // attribute in a real browser. Guarded on native elements only; a custom + // element's `.action` is an ordinary author-defined property that never + // reflects, and passing a function to one is legitimate. + if (!part.el.localName.includes('-')) { + assertNotFunctionActionAttr(value, part.name, part.el.localName); + } /** @type any */ (part.el)[part.name] = value; break; case 'bool': diff --git a/packages/core/src/render-server.js b/packages/core/src/render-server.js index 3d9051f3e..24bf7576b 100644 --- a/packages/core/src/render-server.js +++ b/packages/core/src/render-server.js @@ -1893,8 +1893,10 @@ async function streamTemplate(tr, ctx, controller) { } else { // The SAME guard as the buffered renderer above. This is a second, // independent state machine, so it does not inherit that one: before - // this, a STREAMED page emitted a bound function's whole source while - // the buffered path already refused it. + // this it stringified the function while the buffered path already + // refused it. Reached only via `renderToStream(v, { ssr: false })`, + // which no page render uses, so this was a public-API hole rather + // than a live page leak. assertNotFunctionActionAttr(val, attrName, currentTag); buf += `"${escapeAttr(String(val ?? ''))}"`; state = 'in-tag'; diff --git a/packages/core/test/rendering/browser/form-action-guard.test.js b/packages/core/test/rendering/browser/form-action-guard.test.js new file mode 100644 index 000000000..6918ef7ed --- /dev/null +++ b/packages/core/test/rendering/browser/form-action-guard.test.js @@ -0,0 +1,87 @@ +/** + * The form-action guard, in a REAL browser (#1154). + * + * The node-side coverage runs under linkedom, which does not implement IDL + * attribute reflection: there, `form.action = fn` sets a plain JS property and + * the `action` content attribute stays untouched, so the leak the property + * binding causes is INVISIBLE to that layer. In a real browser `action` is a + * reflected IDL attribute, so the assignment stringifies the function into the + * element's own markup, which is the actual leak. That difference is exactly + * why this file exists rather than trusting the linkedom tests. + * + * It also pins the real-DOM behaviour of the attribute path: a refused render + * must leave the previously-rendered value in place, never a half-written one. + */ + +import { html } from '../../../src/html.js'; +import { render } from '../../../src/render-client.js'; + +import { assert } from '../../../../../test/browser-assert.js'; + +// The sentinel lives inside the function body, so it appears in the output only +// if the function was stringified. +async function secretAction(formData) { + const CONNECTION = 'postgres://user:BROWSER_LEAK_MARKER@host/db'; + return CONNECTION; +} + +function mount() { + const host = document.createElement('div'); + document.body.appendChild(host); + return host; +} + +suite('form-action guard in a real browser', () => { + + test('action=${fn} throws and writes nothing to the document', () => { + const host = mount(); + let threw = null; + try { render(html``, host); } + catch (e) { threw = e; } + assert.ok(threw, 'render must throw'); + assert.ok(/function was interpolated into action=/.test(threw.message), threw.message); + assert.ok(!document.body.innerHTML.includes('BROWSER_LEAK_MARKER'), 'no source in the document'); + }); + + test('.action=${fn} property binding cannot reflect the source into the attribute', () => { + // THE case linkedom cannot see. Without the guard, `el.action = fn` here + // reflects the stringified function into the live `action` attribute. + const host = mount(); + let threw = null; + try { render(html`
`, host); } + catch (e) { threw = e; } + assert.ok(threw, 'render must throw'); + assert.ok(!document.body.innerHTML.includes('BROWSER_LEAK_MARKER'), 'no source in the document'); + const form = host.querySelector('form'); + if (form) { + assert.ok(!String(form.getAttribute('action') || '').includes('BROWSER_LEAK_MARKER'), 'no source in the action attribute'); + assert.ok(!String(form.action || '').includes('BROWSER_LEAK_MARKER'), 'no source on the action property'); + } + }); + + test('a refused re-render leaves the previously rendered action intact', () => { + const host = mount(); + const tpl = (a) => html`
`; + render(tpl('/submit'), host); + const before = host.querySelector('form').getAttribute('action'); + assert.equal(before, '/submit'); + + let threw = null; + try { render(tpl(secretAction), host); } catch (e) { threw = e; } + assert.ok(threw, 'the re-render must throw'); + assert.equal(host.querySelector('form').getAttribute('action'), '/submit', 'prior value survives'); + assert.ok(!document.body.innerHTML.includes('BROWSER_LEAK_MARKER'), 'no source in the document'); + }); + + test('a real form still submits to a string action', () => { + // The guard must not disturb the ordinary case: a string action has to + // survive into a working, submittable form. + const host = mount(); + render(html`
`, host); + const form = host.querySelector('form'); + assert.equal(form.getAttribute('action'), '/feedback'); + assert.equal(form.method, 'post'); + assert.equal(new FormData(form).get('msg'), 'hi'); + }); + +}); diff --git a/packages/core/test/rendering/form-action-attr-guard-client.test.js b/packages/core/test/rendering/form-action-attr-guard-client.test.js index 8974e4f78..646b56ea4 100644 --- a/packages/core/test/rendering/form-action-attr-guard-client.test.js +++ b/packages/core/test/rendering/form-action-attr-guard-client.test.js @@ -25,13 +25,18 @@ before(async () => { async function fakeAction() { const S = 'CLIENT_SECRET'; return S; } -test('client render of action=${fn} throws, DOM never carries the source', () => { +// NOTE on what these DON'T assert: `createInstance` applies every part before +// it calls `container.replaceChildren(...)`, so a throwing part leaves `host` +// empty no matter what. An `assert.ok(!host.innerHTML.includes(SECRET))` here +// would pass unconditionally and prove nothing. The load-bearing assertion is +// the throw itself, plus the SECOND-render cases below, where the host DOES +// hold a live form and a leak would therefore be observable. +test('client render of action=${fn} throws', () => { const host = document.createElement('div'); assert.throws( () => render(html`
`, host), /function was interpolated into action=/, ); - assert.ok(!host.innerHTML.includes('CLIENT_SECRET')); }); test('client render of mixed action="/x/${fn}" throws', () => { @@ -40,7 +45,38 @@ test('client render of mixed action="/x/${fn}" throws', () => { () => render(html`
`, host), /function was interpolated into action=/, ); - assert.ok(!host.innerHTML.includes('CLIENT_SECRET')); +}); + +// Re-render over an ALREADY-MOUNTED form. Here the host really does hold a live +// element, so if the guard let the value through, the source would be sitting +// in the DOM and this would catch it. +test('re-render swapping a string action for a function throws, live DOM stays clean', () => { + const host = document.createElement('div'); + const tpl = (a) => html`
`; + render(tpl('/ok'), host); + assert.equal(host.querySelector('form').getAttribute('action'), '/ok'); + assert.throws(() => render(tpl(fakeAction), host), /function was interpolated into action=/); + assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); + assert.equal(host.querySelector('form').getAttribute('action'), '/ok', 'the prior value must survive'); +}); + +// `.action=${fn}` is a PROPERTY binding, a different commit site from the +// attribute one. `action` is a reflected IDL attribute, so assigning a function +// writes its source into the element's own `action` in a real browser. +test('client .action=${fn} property binding on a native form throws', () => { + const host = document.createElement('div'); + assert.throws( + () => render(html`
`, host), + /function was interpolated into action=/, + ); +}); + +test('a custom element keeps accepting a function on .action', () => { + // Not a reflected IDL attribute, just an author-defined property, so passing + // a function is legitimate and must not be refused. + const host = document.createElement('div'); + render(html``, host); + assert.equal(typeof host.querySelector('my-widget').action, 'function'); }); test('string action still renders on the client', () => { diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index 878292a9d..220d0c888 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -78,10 +78,12 @@ test('string-valued action renders byte-identically to before', async () => { ); }); -// The STREAMING renderer is a second, independent state machine. It was missed -// by the first pass of this fix and kept emitting the action's whole body while -// the buffered renderer already refused it, so it gets its own coverage rather -// than being assumed to inherit the guard. +// The STREAMING renderer is a second, independent state machine, missed by the +// first pass of this fix. It is reached only through +// `renderToStream(v, { ssr: false })`, which no page render uses (the server +// renders every page, Suspense included, via `renderToString`), so this was a +// public-API hole and not a live page leak. It gets its own coverage rather +// than being assumed to inherit the buffered renderer's guard. test('the streaming renderer refuses the same function (ssr:false path)', async () => { await assert.rejects( () => drain(renderToStream(html`
`, { ssr: false })), @@ -97,6 +99,32 @@ test('the streaming renderer never emits the source', async () => { assert.ok(!out.includes('SECRET'), 'streamed output must not carry the function source'); }); +// The unquoted shape lands in the streaming machine's `after-eq` branch; the +// quoted and mixed shapes land in its SEPARATE `attr-quoted`/`attr-unquoted` +// branch. Without these two, that second branch could be deleted outright and +// the suite would stay green, which is the same guard-asymmetry this whole +// change exists to prevent, just moved into the test layer. +test('the streaming renderer refuses a quoted hole', async () => { + await assert.rejects( + () => drain(renderToStream(html`
`, { ssr: false })), + /function was interpolated into action=/, + ); +}); + +test('the streaming renderer refuses a mixed hole', async () => { + await assert.rejects( + () => drain(renderToStream(html`
`, { ssr: false })), + /function was interpolated into action=/, + ); +}); + +test('the streaming renderer refuses formaction', async () => { + await assert.rejects( + () => drain(renderToStream(html``, { ssr: false })), + /function was interpolated into formaction=/, + ); +}); + test('streaming keeps string-valued actions working', async () => { const out = await drain(renderToStream(html`
`, { ssr: false })); assert.match(out, /action="\/submit"/); From 31ddf507f952b40a6c64294703747bd1048923f4 Mon Sep 17 00:00:00 2001 From: Vivek Date: Tue, 28 Jul 2026 18:39:04 +0530 Subject: [PATCH 06/38] docs: cover the form-action throw on the docs site, fix the action= advice --- .agents/skills/webjs/references/muscle-memory-gotchas.md | 7 +++++-- website/app/docs/migrating-from-nextjs/page.ts | 2 +- website/app/docs/troubleshooting/page.ts | 5 +++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/.agents/skills/webjs/references/muscle-memory-gotchas.md b/.agents/skills/webjs/references/muscle-memory-gotchas.md index 5a947ea3c..2c0f22fb1 100644 --- a/.agents/skills/webjs/references/muscle-memory-gotchas.md +++ b/.agents/skills/webjs/references/muscle-memory-gotchas.md @@ -53,8 +53,11 @@ The reason is a source leak. During SSR a `.server.ts` import is the ACTUAL func // WRONG: throws at render; it would have leaked the action's body. import { submitFeedback } from '#modules/feedback/actions/submit-feedback.server.ts'; html`
`; -// RIGHT: post to the page's own url and handle it in the page's `action` export. -html``; +// RIGHT: omit action entirely to post to the page's own url, and handle the +// submission in that page's `action` export. (Omit it rather than writing +// action="": the HTML spec requires a non-empty URL when the attribute is +// present, so the empty string is a conformance error.) +html``; ``` A string stays a string: `action="/search"` and `action=${'/search'}` are unchanged. Other attributes keep their existing stringify behaviour; only `action` and `formaction` are claimed. diff --git a/website/app/docs/migrating-from-nextjs/page.ts b/website/app/docs/migrating-from-nextjs/page.ts index 005c648c8..6ab4f9573 100644 --- a/website/app/docs/migrating-from-nextjs/page.ts +++ b/website/app/docs/migrating-from-nextjs/page.ts @@ -103,7 +103,7 @@ Counter.register('my-counter');

What ports cleanly, and what does not

Ports directly: the app/ directory layout, dynamic segments ([id], [...rest], [[...rest]]), route groups ((group)), the metadata API, route handlers, middleware, and the loading / error / not-found conventions.

-

Needs rethinking: anything written as a Client Component becomes a web component; anything fetching server data moves into a .server action; React state becomes signals; next/link becomes a plain link. Write progressive-enhancement-first: a <form> plus a server action instead of a fetch in a click handler, since the form works without JavaScript and the client router upgrades it automatically.

+

Needs rethinking: anything written as a Client Component becomes a web component; anything fetching server data moves into a .server action; React state becomes signals; next/link becomes a plain link. Write progressive-enhancement-first: a <form> plus a server action instead of a fetch in a click handler, since the form works without JavaScript and the client router upgrades it automatically. Note the binding differs from Next's: <form action={serverAction}> has no WebJs equivalent and is refused at render (it would leak the action's source into the HTML), so omit action to post to the page's own URL and handle the submission in that page's action export.

Not provided: image and font optimization, i18n, and a static export. WebJs is a no-build, standards-based framework, so these are libraries you layer on, not built-ins.

Next steps: read Getting Started to scaffold an app, Architecture for the execution model in depth, and Progressive Enhancement for the design posture that replaces the Client Component habit.

`; diff --git a/website/app/docs/troubleshooting/page.ts b/website/app/docs/troubleshooting/page.ts index 7c564cd20..583d3ff09 100644 --- a/website/app/docs/troubleshooting/page.ts +++ b/website/app/docs/troubleshooting/page.ts @@ -41,6 +41,11 @@ export default function Troubleshooting() {

Cause: WebJs instruments the native DOM mutation methods on every light-DOM host to keep the slot API live, and it installs those interceptors as own properties ON THE ELEMENT INSTANCE. An own instance property shadows a method defined on the class prototype, so the framework's interceptor wins and your same-named class method is never reached. The instrumented names are append, prepend, before, after, replaceWith, replaceChildren, remove, appendChild, insertBefore, removeChild, and replaceChild. TypeScript does not catch it because a shorter override (zero or one argument) is assignable to the native signature.

Fix: rename the member to a non-native name (appendRow(), removeItem()) and update its call sites. Only these native mutation members are affected; render() and the lifecycle hooks are meant to be overridden. The no-shadowed-native-member check rule catches this ahead of time (it flags only a real instance method, never a static member, a nested-object property, or a static shadow = true component, whose native shadow slots are not instrumented). See Components.

+

An error saying a function was interpolated into action=

+

Symptom: a page or component throws at render with a function was interpolated into action= on <form>, usually right after binding a server action to a form the way Next.js does it.

+

Cause: during SSR an imported 'use server' action is the REAL function (the RPC stub exists only in the browser), and action= is an ordinary attribute hole, so stringifying it would write the action's whole body, including anything its closure shows such as a connection string, into the HTML every visitor downloads. WebJs refuses rather than emitting it. The same applies to formaction= on a submit button, to every hole shape (action=\${fn}, action="\${fn}", action="/x/\${fn}"), and to the .action=\${fn} property form on a native form, where the reflected IDL attribute would carry the source into the DOM.

+

Fix: omit action so the form posts to the page's own URL, and handle the submission in that page's action export (write <form method="post"> rather than action="", which the HTML spec treats as a conformance error since the attribute must be a non-empty URL when present). A string action is unaffected. See Server Actions and Progressive Enhancement.

+

An error saying static properties is no longer supported

Symptom: a component throws at construction with static properties is no longer supported. Declare reactive properties via the factory instead.

Cause: the class body has a hand-written static properties = { ... } field. WebJs declares reactive properties only through the WebComponent({ ... }) base-class factory now, and the runtime throws on a direct static properties.

From c1287434c36f00e4f7716ca5993f7ba2bcf77f4a Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 01:10:32 +0530 Subject: [PATCH 07/38] fix: close four bypasses that walked straight past the action guard The guard matched on the RAW attribute name. Both SSR state machines accumulate the name as authored and only the unquoted `after-eq` branch splits the binding sigil off, so a QUOTED binding hole reached the guard still carrying its prefix. `.action` never equals `action`, so the comparison missed and the renderer fell through to stringifying it, which is the very leak this is supposed to close, reachable by adding a single character. `.action="${fn}"`, `?action="${fn}"` and `@action="${fn}"` all published the function body; verified against the head sources. The value check had the same shape of hole. It asked `typeof val === 'function'`, but every commit site does `String(val)`, and `Array.prototype.toString` runs each element through `String()` too, so `action=${[serverAction]}` leaked exactly as the bare function did. Both are fixed where the rule already lives rather than at the call sites: strip a leading binding prefix before matching, and treat a value as carrying a function if it is one or if an array holds one at any depth. Checking the value's shape rather than sniffing the stringified result keeps a legitimate URL from ever tripping it. Also refuses `.action=${fn}` on a native element during SSR. That path never leaked, since a native `.prop` is dropped server-side, but the client guards the same binding and there `action` reflects, so without this a page renders clean on the server and throws on hydration. Failing at the earliest point is the better half of that trade. --- .../webjs/references/muscle-memory-gotchas.md | 8 +- packages/core/src/form-action.js | 43 +++++++- packages/core/src/render-server.js | 7 ++ .../form-action-attr-guard-client.test.js | 57 ++++++++++ .../rendering/form-action-attr-guard.test.js | 100 ++++++++++++++++++ website/app/docs/troubleshooting/page.ts | 6 +- 6 files changed, 211 insertions(+), 10 deletions(-) diff --git a/.agents/skills/webjs/references/muscle-memory-gotchas.md b/.agents/skills/webjs/references/muscle-memory-gotchas.md index 2c0f22fb1..1704370ed 100644 --- a/.agents/skills/webjs/references/muscle-memory-gotchas.md +++ b/.agents/skills/webjs/references/muscle-memory-gotchas.md @@ -47,16 +47,16 @@ There is no React `cache()`, `use()`, or `unstable_cache`. Caching is the `cache Next binds a Server Action with `` and React serializes the binding into hidden fields. WebJs does not read that shape. A function interpolated into `action=` is a hard render error. -The reason is a source leak. During SSR a `.server.ts` import is the ACTUAL function (the RPC stub exists only in the browser), and `action=` is an ordinary attribute hole, so stringifying it would write the function's body, secrets included, into the HTML every visitor downloads. The renderer throws instead, on the server and on the client, for `action=` and `formaction=` alike, in every hole shape (`action=${fn}`, `action="${fn}"`, `action="/x/${fn}"`). +The reason is a source leak. During SSR a `.server.ts` import is the ACTUAL function (the RPC stub exists only in the browser), and `action=` is an ordinary attribute hole, so stringifying it would write the function's body, secrets included, into the HTML every visitor downloads. The renderer throws instead, on the server and on the client, for `action=` and `formaction=` alike. + +The refusal covers the shape, not one spelling of it. Every hole form is refused (`action=${fn}`, `action="${fn}"`, the mixed `action="/x/${fn}"`), with or without a binding sigil (`.action=`, `?action=`, `@action=`, quoted or not), and whether the function arrives bare or wrapped in an array (`action=${[fn]}`), since an array stringifies each element through `String()` and leaks identically. `.action=${fn}` on a native form is refused at SSR too, even though the property itself is dropped there, so a page cannot render clean on the server and then throw on hydration. ```ts // WRONG: throws at render; it would have leaked the action's body. import { submitFeedback } from '#modules/feedback/actions/submit-feedback.server.ts'; html``; // RIGHT: omit action entirely to post to the page's own url, and handle the -// submission in that page's `action` export. (Omit it rather than writing -// action="": the HTML spec requires a non-empty URL when the attribute is -// present, so the empty string is a conformance error.) +// submission in that page's `action` export. html``; ``` diff --git a/packages/core/src/form-action.js b/packages/core/src/form-action.js index a2c19df2d..71d2f405c 100644 --- a/packages/core/src/form-action.js +++ b/packages/core/src/form-action.js @@ -1,3 +1,5 @@ +import { isBindingPrefix } from './binding-prefixes.js'; + /** * Form-action attribute guard (#1154). * @@ -31,14 +33,49 @@ * @param {string} [tag] lowercased owner tag, for the error message */ export function assertNotFunctionActionAttr(val, attrName, tag) { - if (typeof val !== 'function') return; + if (!carriesFunction(val)) return; if (!isFormActionAttr(attrName)) return; throw new Error(formActionError(attrName, tag)); } -/** @param {string} attrName @returns {boolean} */ +/** + * Does stringifying this value expose a function's source? + * + * The commit sites do `String(val)`, and `Array.prototype.toString` stringifies + * each element through `String()` too, so `action=${[serverAction]}` leaks + * exactly as `action=${serverAction}` does. Recursive because nested arrays + * join the same way. + * + * Deliberately NOT a check on the stringified result: sniffing the output for + * something function-shaped would misfire on a legitimate URL, and the value's + * shape is the thing actually being claimed. An object with a hand-written + * `toString` that returns a function's source is out of scope; that is + * deliberate exfiltration, not the accident this guards. + * + * @param {unknown} val + * @returns {boolean} + */ +function carriesFunction(val) { + if (typeof val === 'function') return true; + return Array.isArray(val) && val.some(carriesFunction); +} + +/** + * Is this the name of a form-action attribute? + * + * Strips a leading binding sigil before comparing. The SSR state machines + * accumulate the AUTHORED name, and only the unquoted `after-eq` branch splits + * the prefix off, so a quoted hole arrives here as `.action` / `?action` / + * `@action` with the sigil still attached. Comparing the raw name let every one + * of those through, which is the same leak wearing a different hat: the + * renderer treats a quoted binding hole as a plain attribute and stringifies it. + * + * @param {string} attrName + * @returns {boolean} + */ function isFormActionAttr(attrName) { - const name = String(attrName).toLowerCase(); + let name = String(attrName).toLowerCase(); + if (isBindingPrefix(name[0])) name = name.slice(1); return name === 'action' || name === 'formaction'; } diff --git a/packages/core/src/render-server.js b/packages/core/src/render-server.js index 24bf7576b..95ef38f63 100644 --- a/packages/core/src/render-server.js +++ b/packages/core/src/render-server.js @@ -290,6 +290,13 @@ async function renderTemplate(tr, ctx) { continue; } if (!currentTag.includes('-')) { + // A native element's `.prop` is dropped at SSR, so this path never + // leaked. It still refuses a function in `.action` so the rule does + // not depend on which renderer sees it first: the client guards the + // same binding (there `action` reflects, so it DOES leak), and a + // page that renders clean on the server and throws on hydration is + // a worse failure than one that refuses at the earliest point. + assertNotFunctionActionAttr(val, name, currentTag); state = 'in-tag'; attrName = ''; continue; diff --git a/packages/core/test/rendering/form-action-attr-guard-client.test.js b/packages/core/test/rendering/form-action-attr-guard-client.test.js index 646b56ea4..963b99215 100644 --- a/packages/core/test/rendering/form-action-attr-guard-client.test.js +++ b/packages/core/test/rendering/form-action-attr-guard-client.test.js @@ -85,3 +85,60 @@ test('string action still renders on the client', () => { const form = host.querySelector('form'); assert.equal(form.getAttribute('action'), '/submit'); }); + +// --- Bypasses found reviewing the first cut of the guard ------------------- +// +// A quoted binding hole compiles to a plain `attr` part whose name still +// carries the sigil, so comparing the raw name let `.action="${fn}"` through +// on this side too. These pin the client half of that fix. + +test('client refuses a quoted property hole .action="${fn}"', () => { + const host = document.createElement('div'); + assert.throws( + () => render(html``, host), + /function was interpolated into \.action=/, + ); + assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); +}); + +test('client refuses a quoted boolean hole ?action="${fn}"', () => { + const host = document.createElement('div'); + assert.throws( + () => render(html`
`, host), + /function was interpolated into \?action=/, + ); + assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); +}); + +test('client refuses a quoted event hole @action="${fn}"', () => { + const host = document.createElement('div'); + assert.throws( + () => render(html`
`, host), + /function was interpolated into @action=/, + ); + assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); +}); + +test('client refuses an array-wrapped function', () => { + const host = document.createElement('div'); + assert.throws( + () => render(html`
`, host), + /function was interpolated into action=/, + ); + assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); +}); + +test('client refuses an array-wrapped function inside a mixed hole', () => { + const host = document.createElement('div'); + assert.throws( + () => render(html`
`, host), + /function was interpolated into action=/, + ); + assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); +}); + +test('client still renders an array of plain strings', () => { + const host = document.createElement('div'); + render(html`
`, host); + assert.equal(host.querySelector('form').getAttribute('action'), '/a,/b'); +}); diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index 220d0c888..ea9d98608 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -136,3 +136,103 @@ test('functions in OTHER attributes keep the existing stringify behaviour', asyn const out = await renderToString(html`
`, { ssr: true }); assert.ok(out.startsWith('
{ + const out = await renderToString(html`
`, { ssr: true }) + .then((html) => html, (e) => e); + assert.ok(out instanceof Error, 'must throw, not render'); + assert.match(out.message, /function was interpolated into \.action=/); + assert.doesNotMatch(out.message, /SECRET/, 'the message must not carry what it withholds'); +}); + +test('quoted boolean hole ?action="${fn}" throws', async () => { + await assert.rejects( + () => renderToString(html`
`, { ssr: true }), + /function was interpolated into \?action=/, + ); +}); + +test('quoted event hole @action="${fn}" throws', async () => { + await assert.rejects( + () => renderToString(html`
`, { ssr: true }), + /function was interpolated into @action=/, + ); +}); + +test('quoted .formaction="${fn}" on a submit button throws', async () => { + await assert.rejects( + () => renderToString(html``, { ssr: true }), + /function was interpolated into \.formaction=/, + ); +}); + +// `String(val)` is what every commit site does, and Array.prototype.toString +// runs each element through String() too, so wrapping the action in an array +// leaked exactly as passing it bare did. + +test('an array-wrapped function action=${[fn]} throws', async () => { + await assert.rejects( + () => renderToString(html`
`, { ssr: true }), + /function was interpolated into action=/, + ); +}); + +test('a nested array action=${[[fn]]} throws (Array toString joins recursively)', async () => { + await assert.rejects( + () => renderToString(html`
`, { ssr: true }), + /function was interpolated into action=/, + ); +}); + +test('an array of plain strings still renders, the array check is not a blanket refusal', async () => { + const out = await renderToString(html`
`, { ssr: true }); + assert.match(out, /action="\/a,\/b"/); +}); + +// `.action` on a NATIVE element is dropped at SSR, so it never leaked there. +// It still refuses, so a page cannot render clean on the server and then throw +// on hydration, where `action` reflects and the leak is real. + +test('.action=${fn} on a native form throws at SSR even though the prop would be dropped', async () => { + await assert.rejects( + () => renderToString(html`
`, { ssr: true }), + /function was interpolated into action=/, + ); +}); + +test('a custom element keeps accepting a function on an unclaimed prop', async () => { + const out = await renderToString(html``, { ssr: true }); + assert.doesNotMatch(out, /SECRET/, 'an unserializable prop is dropped, not serialized'); +}); + +test('a URL object action still renders, only functions are refused', async () => { + const out = await renderToString(html`
`, { ssr: true }); + assert.match(out, /action="https:\/\/example\.com\/p"/); +}); + +// A component's render errors are isolated per component (#469), so a throw +// from inside one does NOT propagate: dev swaps in an error box, prod renders +// the component empty and the page still returns 200. That makes the refusal +// invisible on this path, so what has to be pinned is the security property +// rather than the throw. The leak must not reappear just because the error +// was swallowed. +test('a function action inside a component leaks nothing, even though the throw is isolated', async () => { + const { WebComponent } = await import('../../src/component.js'); + class Guarded extends WebComponent({}) { + render() { return html`
`; } + } + Guarded.register('guarded-leak-form'); + + const out = await renderToString(html`
`, { ssr: true }); + assert.doesNotMatch(out, /SECRET/, 'the isolated error path must not emit the source'); + assert.doesNotMatch(out, /async function/, 'no function source of any kind'); +}); diff --git a/website/app/docs/troubleshooting/page.ts b/website/app/docs/troubleshooting/page.ts index 583d3ff09..a154496c6 100644 --- a/website/app/docs/troubleshooting/page.ts +++ b/website/app/docs/troubleshooting/page.ts @@ -42,9 +42,9 @@ export default function Troubleshooting() {

Fix: rename the member to a non-native name (appendRow(), removeItem()) and update its call sites. Only these native mutation members are affected; render() and the lifecycle hooks are meant to be overridden. The no-shadowed-native-member check rule catches this ahead of time (it flags only a real instance method, never a static member, a nested-object property, or a static shadow = true component, whose native shadow slots are not instrumented). See Components.

An error saying a function was interpolated into action=

-

Symptom: a page or component throws at render with a function was interpolated into action= on <form>, usually right after binding a server action to a form the way Next.js does it.

-

Cause: during SSR an imported 'use server' action is the REAL function (the RPC stub exists only in the browser), and action= is an ordinary attribute hole, so stringifying it would write the action's whole body, including anything its closure shows such as a connection string, into the HTML every visitor downloads. WebJs refuses rather than emitting it. The same applies to formaction= on a submit button, to every hole shape (action=\${fn}, action="\${fn}", action="/x/\${fn}"), and to the .action=\${fn} property form on a native form, where the reflected IDL attribute would carry the source into the DOM.

-

Fix: omit action so the form posts to the page's own URL, and handle the submission in that page's action export (write <form method="post"> rather than action="", which the HTML spec treats as a conformance error since the attribute must be a non-empty URL when present). A string action is unaffected. See Server Actions and Progressive Enhancement.

+

Symptom: a render fails with a function was interpolated into action= on <form>, usually right after binding a server action to a form the way Next.js does it. Where that surfaces depends on which render threw. From a page or layout it propagates, so the nearest error boundary catches it and the message is visible. From inside a component it does not: per-component SSR error isolation contains it, so dev renders an error box in place of the component while production renders the component empty and the page still returns 200. A form that has silently vanished in production, with no error anywhere, is the same bug wearing a disguise; check the server log for the message. In neither case is the action's source emitted.

+

Cause: during SSR an imported 'use server' action is the REAL function (the RPC stub exists only in the browser), and action= is an ordinary attribute hole, so stringifying it would write the action's whole body, including anything its closure shows such as a connection string, into the HTML every visitor downloads. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), with or without a binding sigil (.action=, ?action=, @action=, quoted or not), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

+

Fix: omit action so the form posts to the page's own URL, and handle the submission in that page's action export. A string action is unaffected. See Server Actions and Progressive Enhancement.

An error saying static properties is no longer supported

Symptom: a component throws at construction with static properties is no longer supported. Declare reactive properties via the factory instead.

From f53bb094dad74d0215eb24e7416786301ae7cf3a Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 01:12:13 +0530 Subject: [PATCH 08/38] test: prove the action guard refuses identically on Node and Bun The guard lives in the SSR template state machines, so a runtime divergence here is a divergence in whether a server action's source reaches the served HTML. That is the one place "it passes on Node" is not good enough: an app deployed on Bun would leak silently while the Node suite stayed green. Writing it immediately paid for itself. The streaming machine drops every event/prop binding without the native-vs-custom split the buffered machine makes, so `.action=${fn}` was refused by one SSR renderer and waved through by the other. No unit test covered that asymmetry because both machines produce the same empty output either way; only asserting the THROW across both surfaced it. Guarded there too, so the two machines agree. Asserts on the SECRET marker rather than an exact stringification, since JSC and V8 format function source differently. --- packages/core/src/render-server.js | 6 +++ test/bun/form-action-guard.mjs | 76 +++++++++++++++++++++++++++++ test/bun/form-action-guard.test.mjs | 13 +++++ 3 files changed, 95 insertions(+) create mode 100644 test/bun/form-action-guard.mjs create mode 100644 test/bun/form-action-guard.test.mjs diff --git a/packages/core/src/render-server.js b/packages/core/src/render-server.js index 95ef38f63..18fbb28f1 100644 --- a/packages/core/src/render-server.js +++ b/packages/core/src/render-server.js @@ -1889,6 +1889,12 @@ async function streamTemplate(tr, ctx, controller) { const name = attrName.slice(1); const kind = BINDING_PREFIXES[prefix]; if (kind === 'event' || kind === 'prop') { + // Same native-element carve-out the buffered machine applies: the + // binding is dropped here either way, but a native `.action` DOES + // reflect on the client, so refusing at SSR keeps a page from + // rendering clean on the server and throwing on hydration. A custom + // element's `.action` is an ordinary author prop and stays legal. + if (!currentTag.includes('-')) assertNotFunctionActionAttr(val, name, currentTag); buf = buf.slice(0, attrStart); state = 'in-tag'; attrName = ''; diff --git a/test/bun/form-action-guard.mjs b/test/bun/form-action-guard.mjs new file mode 100644 index 000000000..7c624d264 --- /dev/null +++ b/test/bun/form-action-guard.mjs @@ -0,0 +1,76 @@ +/** + * Cross-runtime proof that the form-action leak guard (#1154) refuses + * identically on Node and Bun. Run from the repo root: + * + * node test/bun/form-action-guard.mjs + * bun test/bun/form-action-guard.mjs + * + * WebJs runs on Node 24+ OR Bun, and the guard sits in the SSR template state + * machines, so a divergence here is a divergence in whether a server action's + * source reaches the served HTML. That is the one failure mode where "it works + * on Node" is not good enough: an app deployed on Bun would leak silently while + * the Node test suite stayed green. + * + * The runtime-sensitive part is not the string building, it is what each engine + * does with `String(fn)` and with the async render path around it. JSC and V8 + * format function source differently, so the assertions check for the SECRET + * marker itself rather than an exact stringification. + */ +import assert from 'node:assert/strict'; + +import { html } from '../../packages/core/src/html.js'; +import { renderToString, renderToStream } from '../../packages/core/src/render-server.js'; + +const runtime = process.versions.bun ? `bun ${process.versions.bun}` : `node ${process.versions.node}`; + +const SECRET = 'postgres://user:BUN_PARITY_SECRET@host/db'; +async function leaky(input) { const conn = SECRET; return { ok: true, conn, input }; } + +async function drain(stream) { + let out = ''; + for await (const c of stream) out += typeof c === 'string' ? c : new TextDecoder().decode(c); + return out; +} + +/** Every shape that must be refused, in both SSR machines. */ +const refused = { + 'action=${fn}': () => html`
`, + 'action="${fn}"': () => html`
`, + 'mixed action="/x/${fn}"': () => html`
`, + 'formaction=${fn}': () => html``, + 'quoted prop .action="${fn}"': () => html`
`, + 'quoted bool ?action="${fn}"': () => html`
`, + 'quoted event @action="${fn}"': () => html`
`, + 'native prop .action=${fn}': () => html`
`, + 'array-wrapped action=${[fn]}': () => html`
`, +}; + +for (const [name, mk] of Object.entries(refused)) { + // Buffered renderer. + let threw = null; + try { await renderToString(mk(), { ssr: true }); } catch (e) { threw = e; } + assert.ok(threw, `[${runtime}] buffered SSR must refuse ${name}`); + assert.match(threw.message, /function was interpolated into/, `[${runtime}] ${name} message`); + assert.ok(!threw.message.includes('BUN_PARITY_SECRET'), + `[${runtime}] the refusal message must not carry the source it withholds (${name})`); + + // Streaming renderer, the second, independent state machine. + let streamThrew = null; + try { await drain(renderToStream(mk(), { ssr: false })); } catch (e) { streamThrew = e; } + assert.ok(streamThrew, `[${runtime}] streaming SSR must refuse ${name}`); + assert.ok(!streamThrew.message.includes('BUN_PARITY_SECRET'), + `[${runtime}] streaming refusal must not carry the source (${name})`); +} + +// The passthrough must stay byte-identical across runtimes: refusing everything +// would also pass the assertions above, so pin what still works. +const okBuffered = await renderToString(html`
`, { ssr: true }); +assert.match(okBuffered, /action="\/submit"/, `[${runtime}] a string action must still render`); + +const okStream = await drain(renderToStream(html`
`, { ssr: false })); +assert.match(okStream, /action="\/submit"/, `[${runtime}] a string action must still stream`); + +const okArray = await renderToString(html`
`, { ssr: true }); +assert.match(okArray, /action="\/a,\/b"/, `[${runtime}] an array of strings is not a function`); + +console.log(`[${runtime}] form-action guard parity OK: ${Object.keys(refused).length} refused shapes, 3 passthroughs`); diff --git a/test/bun/form-action-guard.test.mjs b/test/bun/form-action-guard.test.mjs new file mode 100644 index 000000000..56b8f16bb --- /dev/null +++ b/test/bun/form-action-guard.test.mjs @@ -0,0 +1,13 @@ +/** + * Run the cross-runtime form-action guard proof (#1154) under WHICHEVER runtime + * executes the suite. Picked up by the root `node --test` runner (so `npm test` + * exercises the Node path); CI also runs `bun test/bun/form-action-guard.mjs` + * for the Bun path. The proof is a plain assert script (`form-action-guard.mjs`, + * not `*.test.mjs`, so the runner does not double-run it); importing it runs it + * and throws on any failure. + */ +import { test } from 'node:test'; + +test('the form-action leak guard refuses identically on this runtime (#1154)', async () => { + await import('./form-action-guard.mjs'); +}); From 972511405a942f403019b2e574eb98203f92437b Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 01:32:42 +0530 Subject: [PATCH 09/38] fix: stop the guard crashing on a cyclic array and refusing event bindings Three problems, all introduced by the previous commit rather than found in the original fix. `carriesFunction` recursed without tracking what it had already seen, but the thing it models does not: `Array.prototype.join` has a cycle guard, so `String(a)` on a self-referential array is `''`. A render that used to emit `action=""` became a RangeError. Refusing to leak must not turn into a new way to crash, so the walk now tracks visited arrays and matches the stringification it is standing in for. Guarding `kind === 'event' || kind === 'prop'` in the streaming machine was two mistakes at once. It was a false positive, since an event binding drops its value and never stringifies it, and a function is the legitimate thing to pass one, so `` was refused for no reason. It also created a fresh divergence in the opposite direction from the one it claimed to fix: the buffered machine guards only `prop`, so the same template threw on one renderer and rendered on the other. Now `prop` on both, with the native-element carve-out on both. `?action=${fn}` was refused by nothing while both doc surfaces said it was. It never leaked, a boolean binding stringifies nothing, but a truthy function silently emitted a bare `action=""`, which is never what anyone meant. Refused now on all three commit sites, which makes the documented rule true rather than true-only-when-quoted. The docs now state the carve-outs as a table instead of the sweeping "with or without a binding sigil, quoted or not", which was wrong for `@` and for a custom element's own `.action`. --- .../webjs/references/muscle-memory-gotchas.md | 19 +++- .github/workflows/ci.yml | 6 ++ packages/core/src/form-action.js | 15 ++- packages/core/src/render-client.js | 7 ++ packages/core/src/render-server.js | 26 ++++-- .../browser/form-action-guard.test.js | 40 ++++++-- .../form-action-attr-guard-client.test.js | 93 ++++++++++++------- .../rendering/form-action-attr-guard.test.js | 44 +++++++++ test/bun/form-action-guard.mjs | 45 ++++++++- test/bun/form-action-guard.test.mjs | 6 +- website/app/docs/troubleshooting/page.ts | 3 +- 11 files changed, 250 insertions(+), 54 deletions(-) diff --git a/.agents/skills/webjs/references/muscle-memory-gotchas.md b/.agents/skills/webjs/references/muscle-memory-gotchas.md index 1704370ed..9de86ef93 100644 --- a/.agents/skills/webjs/references/muscle-memory-gotchas.md +++ b/.agents/skills/webjs/references/muscle-memory-gotchas.md @@ -49,7 +49,24 @@ Next binds a Server Action with `
` and React serialize The reason is a source leak. During SSR a `.server.ts` import is the ACTUAL function (the RPC stub exists only in the browser), and `action=` is an ordinary attribute hole, so stringifying it would write the function's body, secrets included, into the HTML every visitor downloads. The renderer throws instead, on the server and on the client, for `action=` and `formaction=` alike. -The refusal covers the shape, not one spelling of it. Every hole form is refused (`action=${fn}`, `action="${fn}"`, the mixed `action="/x/${fn}"`), with or without a binding sigil (`.action=`, `?action=`, `@action=`, quoted or not), and whether the function arrives bare or wrapped in an array (`action=${[fn]}`), since an array stringifies each element through `String()` and leaks identically. `.action=${fn}` on a native form is refused at SSR too, even though the property itself is dropped there, so a page cannot render clean on the server and then throw on hydration. +The refusal covers the shape, not one spelling of it. Every hole form is refused (`action=${fn}`, `action="${fn}"`, the mixed `action="/x/${fn}"`), and so is a function wrapped in an array (`action=${[fn]}`), since an array stringifies each element through `String()` and leaks identically. + +Two carve-outs, both because those bindings never stringify their value: + +| Written as | Refused? | Why | +|---|---|---| +| `action=` / `formaction=` | yes | ordinary attribute, stringified into the HTML | +| `.action=` on a native form | yes | the property reflects, so the source lands in the DOM on the client | +| `.action=` on a custom element | **no** | an ordinary author-defined property, never reflected; a function is a legitimate value | +| `?action=` | yes | never leaked, but it is meaningless, so it is refused rather than silently emitting a bare `action=""` | +| `@action=` unquoted | **no** | an event listener, and a function is exactly what one takes | +| `@action="${fn}"` quoted | yes | quoting makes it an ordinary attribute again, so it leaks | + +That last row is the one to remember: quoting a binding hole turns it back into a plain attribute, which is why invariant 4 requires `@`, `.` and `?` holes to be unquoted. + +`.action=${fn}` on a native form is refused during SSR too, even though the property is dropped there and nothing could leak, so a page cannot render clean on the server and then throw on hydration. + +**Inside a component you may never see the error.** Per-component SSR error isolation contains the throw, so development shows an error box in place of the component and production renders it empty with the page still returning 200. A form that has silently vanished in production is this bug wearing a disguise; the message is in the server log. Nothing leaks either way. ```ts // WRONG: throws at render; it would have leaked the action's body. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 78a8beb08..c20801d30 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -108,6 +108,12 @@ jobs: # so this is the cross-shell parity proof. - name: webjs listener parity on Bun run: bun test/bun/listener.mjs + # The form-action leak guard (#1154) on Bun: a divergence here is a + # divergence in whether a server action's SOURCE reaches the served HTML, + # so it cannot be left to the Node suite alone. Covers both SSR state + # machines, which have twice drifted apart on the binding shapes. + - name: WebJs form-action guard parity on Bun + run: bun test/bun/form-action-guard.mjs # Listener overhead reductions on Bun (#756): the out-of-band IP stamp (no # Request clone), the buffered sync-compression fast path, the streamed-head # non-blocking classifier, and the basePath rebuild spoof guard. Run as the diff --git a/packages/core/src/form-action.js b/packages/core/src/form-action.js index 71d2f405c..c003bb864 100644 --- a/packages/core/src/form-action.js +++ b/packages/core/src/form-action.js @@ -46,6 +46,12 @@ export function assertNotFunctionActionAttr(val, attrName, tag) { * exactly as `action=${serverAction}` does. Recursive because nested arrays * join the same way. * + * Tracks visited arrays because `Array.prototype.join` has a cycle guard and + * this has to match it. A self-referential array stringifies to `''` rather + * than recursing forever, so a naive walk would turn a render that used to + * succeed into a stack overflow. Refusing to leak must not become a new way + * to crash. + * * Deliberately NOT a check on the stringified result: sniffing the output for * something function-shaped would misfire on a legitimate URL, and the value's * shape is the thing actually being claimed. An object with a hand-written @@ -53,11 +59,16 @@ export function assertNotFunctionActionAttr(val, attrName, tag) { * deliberate exfiltration, not the accident this guards. * * @param {unknown} val + * @param {Set} [seen] * @returns {boolean} */ -function carriesFunction(val) { +function carriesFunction(val, seen) { if (typeof val === 'function') return true; - return Array.isArray(val) && val.some(carriesFunction); + if (!Array.isArray(val)) return false; + const visited = seen || new Set(); + if (visited.has(val)) return false; + visited.add(val); + return val.some((v) => carriesFunction(v, visited)); } /** diff --git a/packages/core/src/render-client.js b/packages/core/src/render-client.js index 17e0f2b6e..4c1e6c1f0 100644 --- a/packages/core/src/render-client.js +++ b/packages/core/src/render-client.js @@ -694,10 +694,17 @@ function applyPart(part, value, _prev, allValues) { /** @type any */ (part.el)[part.name] = value; break; case 'bool': + // #1154: never leaked (a boolean binding stringifies nothing), but + // `?action=${fn}` is meaningless in every case, and refusing it keeps + // this renderer agreeing with both SSR machines. + assertNotFunctionActionAttr(value, part.name, part.el.localName); if (value) part.el.setAttribute(part.name, ''); else part.el.removeAttribute(part.name); break; case 'event': + // NOT guarded, deliberately. An event binding never stringifies its + // value, and a function is the legitimate thing to pass one, so + // `` has to keep working. part.handler = typeof value === 'function' ? /** @type any */ (value) : null; break; case 'element': diff --git a/packages/core/src/render-server.js b/packages/core/src/render-server.js index 18fbb28f1..f4586c702 100644 --- a/packages/core/src/render-server.js +++ b/packages/core/src/render-server.js @@ -326,6 +326,10 @@ async function renderTemplate(tr, ctx) { state = 'in-tag'; attrName = ''; } else if (kind === 'bool') { + // Never leaked (a boolean binding stringifies nothing), but + // `?action=${fn}` is meaningless in every case and refusing it keeps + // the rule true for every sigil rather than only the quoted ones. + assertNotFunctionActionAttr(val, name, currentTag); out = out.slice(0, attrStart); if (val) out += `${name}=""`; state = 'in-tag'; @@ -1889,16 +1893,26 @@ async function streamTemplate(tr, ctx, controller) { const name = attrName.slice(1); const kind = BINDING_PREFIXES[prefix]; if (kind === 'event' || kind === 'prop') { - // Same native-element carve-out the buffered machine applies: the - // binding is dropped here either way, but a native `.action` DOES - // reflect on the client, so refusing at SSR keeps a page from - // rendering clean on the server and throwing on hydration. A custom - // element's `.action` is an ordinary author prop and stays legal. - if (!currentTag.includes('-')) assertNotFunctionActionAttr(val, name, currentTag); + // Guard `prop` ONLY, matching the buffered machine. An `@action` + // event binding is dropped here and never stringified, and a + // function is the LEGITIMATE value for one (`` + // listens for an `action` event), so refusing it would be a false + // positive. `.action` differs: on a native element it reflects on + // the client, so refusing at SSR keeps a page from rendering clean + // on the server and throwing on hydration. A custom element's + // `.action` is an ordinary author prop and stays legal. + if (kind === 'prop' && !currentTag.includes('-')) { + assertNotFunctionActionAttr(val, name, currentTag); + } buf = buf.slice(0, attrStart); state = 'in-tag'; attrName = ''; } else if (kind === 'bool') { + // A boolean binding stringifies nothing, so this never leaked, but + // `?action=${fn}` is meaningless in every case (a truthy function + // emits a bare `action=""`), and refusing it keeps the rule the docs + // state true for every sigil rather than true only when quoted. + assertNotFunctionActionAttr(val, name, currentTag); buf = buf.slice(0, attrStart); if (val) buf += `${name}=""`; state = 'in-tag'; diff --git a/packages/core/test/rendering/browser/form-action-guard.test.js b/packages/core/test/rendering/browser/form-action-guard.test.js index 6918ef7ed..8d58406ae 100644 --- a/packages/core/test/rendering/browser/form-action-guard.test.js +++ b/packages/core/test/rendering/browser/form-action-guard.test.js @@ -46,17 +46,43 @@ suite('form-action guard in a real browser', () => { test('.action=${fn} property binding cannot reflect the source into the attribute', () => { // THE case linkedom cannot see. Without the guard, `el.action = fn` here // reflects the stringified function into the live `action` attribute. + // + // Renders the SAME template with a string first, so there is a real, + // attached form to inspect afterwards. On a fresh host a throwing part + // leaves the container empty, and every assertion below would then hold + // vacuously against a form that does not exist, which is exactly what this + // test would be worth nothing for. const host = mount(); + const tpl = (v) => html``; + render(tpl('/submit'), host); + + const form = host.querySelector('form'); + assert.ok(form, 'the string render must produce a form to inspect'); + // Confirms the property really does reflect in this browser. If it did not, + // the leak this test guards could not happen and the test would be inert. + assert.ok(String(form.action).includes('/submit'), 'action must reflect, else this test proves nothing'); + let threw = null; - try { render(html`
`, host); } - catch (e) { threw = e; } + try { render(tpl(secretAction), host); } catch (e) { threw = e; } assert.ok(threw, 'render must throw'); + + const after = host.querySelector('form'); + assert.ok(after, 'the form must still be attached after the refusal'); + assert.ok(!String(after.getAttribute('action') || '').includes('BROWSER_LEAK_MARKER'), 'no source in the action attribute'); + assert.ok(!String(after.action || '').includes('BROWSER_LEAK_MARKER'), 'no source on the action property'); assert.ok(!document.body.innerHTML.includes('BROWSER_LEAK_MARKER'), 'no source in the document'); - const form = host.querySelector('form'); - if (form) { - assert.ok(!String(form.getAttribute('action') || '').includes('BROWSER_LEAK_MARKER'), 'no source in the action attribute'); - assert.ok(!String(form.action || '').includes('BROWSER_LEAK_MARKER'), 'no source on the action property'); - } + }); + + test('a custom element keeps its own .action property, which does not reflect', () => { + // The carve-out the guard depends on: a custom element's `.action` is an + // ordinary author-defined property, so a function there is legitimate and + // nothing reflects into markup. + const host = mount(); + render(html``, host); + const el = host.querySelector('my-action-holder'); + assert.ok(el, 'the custom element renders'); + assert.equal(typeof el.action, 'function', 'the property is kept as a function'); + assert.ok(!document.body.innerHTML.includes('BROWSER_LEAK_MARKER'), 'and nothing reflects into markup'); }); test('a refused re-render leaves the previously rendered action intact', () => { diff --git a/packages/core/test/rendering/form-action-attr-guard-client.test.js b/packages/core/test/rendering/form-action-attr-guard-client.test.js index 963b99215..7e139eb34 100644 --- a/packages/core/test/rendering/form-action-attr-guard-client.test.js +++ b/packages/core/test/rendering/form-action-attr-guard-client.test.js @@ -89,52 +89,69 @@ test('string action still renders on the client', () => { // --- Bypasses found reviewing the first cut of the guard ------------------- // // A quoted binding hole compiles to a plain `attr` part whose name still -// carries the sigil, so comparing the raw name let `.action="${fn}"` through -// on this side too. These pin the client half of that fix. - -test('client refuses a quoted property hole .action="${fn}"', () => { +// carries the sigil, so comparing the raw name let `.action="${fn}"` through on +// this side too. +// +// Each case renders the SAME template with a good value first and only then +// swaps in the bad one, per the note above. That matters twice over: on a fresh +// host a throwing part leaves the container empty, so a "no secret in the DOM" +// assertion would hold with or without the guard; and only a same-template +// re-render patches in place, so only then is there a live form whose surviving +// state is worth asserting. + +/** + * @param {(v: unknown) => unknown} tpl a template taking the value under test + * @param {unknown} bad the value that must be refused + * @param {RegExp} messagePattern + * @param {string} goodRendered what the good value leaves in the DOM + */ +function refusesOnRerender(tpl, bad, messagePattern, goodRendered = '/submit') { const host = document.createElement('div'); - assert.throws( - () => render(html`
`, host), - /function was interpolated into \.action=/, - ); - assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); -}); + render(tpl('/submit'), host); + const before = host.innerHTML; + assert.ok(host.querySelector('form'), 'the good value must render a form'); + assert.ok(before.includes(goodRendered), `expected ${goodRendered} in ${before}`); -test('client refuses a quoted boolean hole ?action="${fn}"', () => { - const host = document.createElement('div'); - assert.throws( - () => render(html`
`, host), - /function was interpolated into \?action=/, - ); + assert.throws(() => render(tpl(bad), host), messagePattern); + + assert.ok(host.querySelector('form'), 'the previously rendered form must still be there'); + assert.equal(host.innerHTML, before, 'the refused render must leave the DOM untouched'); assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); +} + +test('client refuses a quoted property hole .action="${fn}"', () => { + refusesOnRerender((v) => html`
`, fakeAction, /function was interpolated into \.action=/); }); test('client refuses a quoted event hole @action="${fn}"', () => { - const host = document.createElement('div'); - assert.throws( - () => render(html`
`, host), - /function was interpolated into @action=/, - ); - assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); + refusesOnRerender((v) => html`
`, fakeAction, /function was interpolated into @action=/); }); test('client refuses an array-wrapped function', () => { - const host = document.createElement('div'); - assert.throws( - () => render(html`
`, host), - /function was interpolated into action=/, - ); - assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); + refusesOnRerender((v) => html`
`, [fakeAction], /function was interpolated into action=/); }); test('client refuses an array-wrapped function inside a mixed hole', () => { + refusesOnRerender((v) => html`
`, [fakeAction], /function was interpolated into action=/, '/x//submit'); +}); + +test('client refuses an unquoted boolean hole ?action=${fn}', () => { + // A boolean binding renders the bare attribute for any truthy value, so the + // good render leaves `action=""` rather than the value itself. + refusesOnRerender((v) => html`
`, fakeAction, /function was interpolated into action=/, 'action=""'); +}); + +// The carve-outs. An event binding never stringifies its value and a function +// is the legitimate thing to pass one, so refusing it would be a false +// positive. + +test('an unquoted @action=${fn} event binding stays legal', () => { const host = document.createElement('div'); - assert.throws( - () => render(html`
`, host), - /function was interpolated into action=/, - ); - assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); + render(html`
`, host); + const form = host.querySelector('form'); + assert.ok(form, 'the form still renders'); + assert.ok(!form.hasAttribute('action'), 'an event binding writes no action attribute'); + assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'and leaks nothing'); }); test('client still renders an array of plain strings', () => { @@ -142,3 +159,13 @@ test('client still renders an array of plain strings', () => { render(html`
`, host); assert.equal(host.querySelector('form').getAttribute('action'), '/a,/b'); }); + +test('a self-referential array does not crash the render', () => { + // `Array.prototype.join` has a cycle guard, so `String(cyclic)` is ''. The + // function check has to match that rather than recurse forever. + const cyclic = []; + cyclic.push(cyclic); + const host = document.createElement('div'); + render(html`
`, host); + assert.equal(host.querySelector('form').getAttribute('action'), ''); +}); diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index ea9d98608..bfd9094e1 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -236,3 +236,47 @@ test('a function action inside a component leaks nothing, even though the throw assert.doesNotMatch(out, /SECRET/, 'the isolated error path must not emit the source'); assert.doesNotMatch(out, /async function/, 'no function source of any kind'); }); + +// --- Carve-outs, and the machines agreeing on them ------------------------ +// +// A guard that refused everything would satisfy every assertion above, so what +// stays LEGAL has to be pinned just as hard. Both bindings below never +// stringify their value, so neither can leak, and refusing them would break +// ordinary code. + +test('an unquoted @action=${fn} event binding stays legal on both machines', async () => { + const buffered = await renderToString(html`
`, { ssr: true }); + assert.doesNotMatch(buffered, /SECRET/); + const streamed = await drain(renderToStream(html`
`, { ssr: false })); + assert.doesNotMatch(streamed, /SECRET/); +}); + +test('a custom element keeps a function on its own .action property', async () => { + const buffered = await renderToString(html``, { ssr: true }); + assert.doesNotMatch(buffered, /SECRET/, 'an unserializable prop is dropped, never serialized'); + const streamed = await drain(renderToStream(html``, { ssr: false })); + assert.doesNotMatch(streamed, /SECRET/); +}); + +test('an unquoted ?action=${fn} is refused rather than emitting a bare action=""', async () => { + // Never leaked, but a truthy function silently produced `action=""`, which is + // never what anyone meant. Refusing keeps the documented rule true for `?`. + await assert.rejects( + () => renderToString(html`
`, { ssr: true }), + /function was interpolated into action=/, + ); + await assert.rejects( + () => drain(renderToStream(html`
`, { ssr: false })), + /function was interpolated into action=/, + ); +}); + +test('a self-referential array renders instead of overflowing the stack', async () => { + // `Array.prototype.join` has a cycle guard, so `String(cyclic)` is ''. The + // function walk has to match that; a naive recursion turned a render that + // used to succeed into a RangeError. + const cyclic = []; + cyclic.push(cyclic); + const out = await renderToString(html`
`, { ssr: true }); + assert.match(out, /action=""/); +}); diff --git a/test/bun/form-action-guard.mjs b/test/bun/form-action-guard.mjs index 7c624d264..2bb983993 100644 --- a/test/bun/form-action-guard.mjs +++ b/test/bun/form-action-guard.mjs @@ -32,7 +32,15 @@ async function drain(stream) { return out; } -/** Every shape that must be refused, in both SSR machines. */ +/** + * Every shape that must be refused, in both SSR machines. + * + * The UNQUOTED sigil forms are load-bearing here, not padding: the two machines + * route bindings through separate branches, and they have already drifted apart + * on exactly those (one refused `.action=${fn}` while the other dropped it, and + * a later fix over-corrected into refusing `@action=${fn}` on one side only). + * A parity file that enumerates only the quoted shapes cannot see either. + */ const refused = { 'action=${fn}': () => html`
`, 'action="${fn}"': () => html`
`, @@ -42,6 +50,7 @@ const refused = { 'quoted bool ?action="${fn}"': () => html`
`, 'quoted event @action="${fn}"': () => html`
`, 'native prop .action=${fn}': () => html`
`, + 'unquoted bool ?action=${fn}': () => html`
`, 'array-wrapped action=${[fn]}': () => html`
`, }; @@ -54,14 +63,46 @@ for (const [name, mk] of Object.entries(refused)) { assert.ok(!threw.message.includes('BUN_PARITY_SECRET'), `[${runtime}] the refusal message must not carry the source it withholds (${name})`); - // Streaming renderer, the second, independent state machine. + // Streaming renderer, the second, independent state machine. Matches the + // message too: asserting only that SOMETHING threw would be satisfied by any + // unrelated error, which is how a machine that refuses for the wrong reason + // slips through a parity check. let streamThrew = null; try { await drain(renderToStream(mk(), { ssr: false })); } catch (e) { streamThrew = e; } assert.ok(streamThrew, `[${runtime}] streaming SSR must refuse ${name}`); + assert.match(streamThrew.message, /function was interpolated into/, + `[${runtime}] streaming must refuse ${name} for the RIGHT reason`); assert.ok(!streamThrew.message.includes('BUN_PARITY_SECRET'), `[${runtime}] streaming refusal must not carry the source (${name})`); } +/** + * The carve-outs, which matter as much as the refusals: a guard that refused + * everything would satisfy every assertion above. Both machines must AGREE that + * these stay legal. + */ +const allowed = { + 'unquoted event @action=${fn}': () => html`
`, + 'custom-element event @action=${fn}': () => html``, + 'custom-element prop .action=${fn}': () => html``, +}; + +for (const [name, mk] of Object.entries(allowed)) { + const buffered = await renderToString(mk(), { ssr: true }); + assert.ok(!buffered.includes('BUN_PARITY_SECRET'), `[${runtime}] ${name} must not leak (buffered)`); + + const streamed = await drain(renderToStream(mk(), { ssr: false })); + assert.ok(!streamed.includes('BUN_PARITY_SECRET'), `[${runtime}] ${name} must not leak (streaming)`); +} + +// A self-referential array stringifies to '' because `Array.prototype.join` has +// a cycle guard. The function check has to match that rather than recurse, on +// both engines. +const cyclic = []; +cyclic.push(cyclic); +const cyclicOut = await renderToString(html`
`, { ssr: true }); +assert.match(cyclicOut, /action=""/, `[${runtime}] a cyclic array must render, not overflow the stack`); + // The passthrough must stay byte-identical across runtimes: refusing everything // would also pass the assertions above, so pin what still works. const okBuffered = await renderToString(html`
`, { ssr: true }); diff --git a/test/bun/form-action-guard.test.mjs b/test/bun/form-action-guard.test.mjs index 56b8f16bb..e311b3d41 100644 --- a/test/bun/form-action-guard.test.mjs +++ b/test/bun/form-action-guard.test.mjs @@ -1,8 +1,10 @@ /** * Run the cross-runtime form-action guard proof (#1154) under WHICHEVER runtime * executes the suite. Picked up by the root `node --test` runner (so `npm test` - * exercises the Node path); CI also runs `bun test/bun/form-action-guard.mjs` - * for the Bun path. The proof is a plain assert script (`form-action-guard.mjs`, + * exercises the Node path); the Bun path runs twice in CI, as its own + * `bun test/bun/form-action-guard.mjs` step in the `bun` job and again through + * the `scripts/run-bun-tests.js` matrix. The proof is a plain assert script + * (`form-action-guard.mjs`, * not `*.test.mjs`, so the runner does not double-run it); importing it runs it * and throws on any failure. */ diff --git a/website/app/docs/troubleshooting/page.ts b/website/app/docs/troubleshooting/page.ts index a154496c6..d02561114 100644 --- a/website/app/docs/troubleshooting/page.ts +++ b/website/app/docs/troubleshooting/page.ts @@ -43,7 +43,8 @@ export default function Troubleshooting() {

An error saying a function was interpolated into action=

Symptom: a render fails with a function was interpolated into action= on <form>, usually right after binding a server action to a form the way Next.js does it. Where that surfaces depends on which render threw. From a page or layout it propagates, so the nearest error boundary catches it and the message is visible. From inside a component it does not: per-component SSR error isolation contains it, so dev renders an error box in place of the component while production renders the component empty and the page still returns 200. A form that has silently vanished in production, with no error anywhere, is the same bug wearing a disguise; check the server log for the message. In neither case is the action's source emitted.

-

Cause: during SSR an imported 'use server' action is the REAL function (the RPC stub exists only in the browser), and action= is an ordinary attribute hole, so stringifying it would write the action's whole body, including anything its closure shows such as a connection string, into the HTML every visitor downloads. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), with or without a binding sigil (.action=, ?action=, @action=, quoted or not), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

+

Cause: during SSR an imported 'use server' action is the REAL function (the RPC stub exists only in the browser), and action= is an ordinary attribute hole, so stringifying it would write the action's whole body, including anything its closure shows such as a connection string, into the HTML every visitor downloads. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

+

Two things stay legal, because neither stringifies its value: a custom element's .action property (an ordinary author-defined property that never reflects, so <my-el .action=\${fn}> is fine), and an unquoted @action=\${fn} event listener, where a function is exactly what is wanted. Quoting a binding hole turns it back into a plain attribute, so @action="\${fn}" IS refused; that is the practical reason invariant 4 requires @, . and ? holes to be unquoted. ?action=\${fn} is refused as well: it never leaked, but a truthy function would silently emit a bare action="", which is never what anyone meant.

Fix: omit action so the form posts to the page's own URL, and handle the submission in that page's action export. A string action is unaffected. See Server Actions and Progressive Enhancement.

An error saying static properties is no longer supported

From a8c824732f53c99771a7f1f4556873da4ab16833 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 12:35:25 +0530 Subject: [PATCH 10/38] test: restore the dropped quoted-bool case, name the clause each test pins Rewriting these cases around a shared helper quietly dropped `?action="${fn}"`, because a quoted bool renders a literal `?action` attribute and does not fit the helper's good-render step. It was dropped for not fitting, not for being wrong, and that left the shape with no client coverage at all while the commit presented the test work as a widening. Restored, written directly instead of through the helper. The header comment also misdescribed what these tests pin. A quoted single hole compiles to an `attr-mixed` part, not a plain `attr` one, so the quoted cases exercise the guard in the piece loop; neutering the `attr` case leaves them green. Verified by reverting each clause separately rather than reasoning about it. The comment now says which clause each group covers, since the obvious reading is backwards and an edit to the wrong branch would look covered. --- .../form-action-attr-guard-client.test.js | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/packages/core/test/rendering/form-action-attr-guard-client.test.js b/packages/core/test/rendering/form-action-attr-guard-client.test.js index 7e139eb34..c47f2a971 100644 --- a/packages/core/test/rendering/form-action-attr-guard-client.test.js +++ b/packages/core/test/rendering/form-action-attr-guard-client.test.js @@ -88,9 +88,16 @@ test('string action still renders on the client', () => { // --- Bypasses found reviewing the first cut of the guard ------------------- // -// A quoted binding hole compiles to a plain `attr` part whose name still -// carries the sigil, so comparing the raw name let `.action="${fn}"` through on -// this side too. +// A quoted binding hole keeps its sigil in the part's name, so comparing the +// raw name let `.action="${fn}"` through on this side too. +// +// WHICH clause each case pins, verified by reverting them one at a time rather +// than assumed: a quoted single hole compiles to an `attr-mixed` part, NOT a +// plain `attr` one, so the quoted cases below exercise the guard in the +// `attr-mixed` piece loop. Neutering the `attr` case leaves them green. The +// plain `attr` guard is pinned by the UNQUOTED cases (`action=${fn}` and the +// array-wrapped one). Worth stating because the obvious reading is backwards, +// and an edit to the wrong branch would look covered. // // Each case renders the SAME template with a good value first and only then // swaps in the bad one, per the note above. That matters twice over: on a fresh @@ -127,6 +134,24 @@ test('client refuses a quoted event hole @action="${fn}"', () => { refusesOnRerender((v) => html`
`, fakeAction, /function was interpolated into @action=/); }); +test('client refuses a quoted boolean hole ?action="${fn}"', () => { + // Written directly rather than through the helper: a quoted `?action` is an + // ordinary attribute, so the good render leaves a literal `?action="/submit"` + // rather than the `action` the helper checks for. Kept because dropping it + // would leave this shape with NO client coverage; the SSR machines cover it, + // but this is a separate renderer. + const host = document.createElement('div'); + const tpl = (v) => html`
`; + render(tpl('/submit'), host); + const before = host.innerHTML; + assert.ok(host.querySelector('form'), 'the good value must render a form'); + + assert.throws(() => render(tpl(fakeAction), host), /function was interpolated into \?action=/); + + assert.equal(host.innerHTML, before, 'the refused render must leave the DOM untouched'); + assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); +}); + test('client refuses an array-wrapped function', () => { refusesOnRerender((v) => html`
`, [fakeAction], /function was interpolated into action=/); }); From 5baa93059d731b05d50b99bfd0ff087f103cde8f Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 12:37:33 +0530 Subject: [PATCH 11/38] test: cover the streaming native-prop clause in the rendering suite Mapped all twelve guard call sites to the tests that fail when each is reverted. Eleven were pinned by the rendering suite. The streaming machine's native-prop clause was not: deleting it left every test in `packages/core/test/rendering` green, and only `test/bun/form-action-guard.mjs` caught it. That is real coverage, so this is not a hole, but a clause guarded by a single cross-runtime script is one file move away from being unguarded, and the script exists to prove parity rather than to be the only proof a branch works at all. Covered in the rendering suite too. --- .../test/rendering/form-action-attr-guard.test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index bfd9094e1..b144e7ddc 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -280,3 +280,15 @@ test('a self-referential array renders instead of overflowing the stack', async const out = await renderToString(html`
`, { ssr: true }); assert.match(out, /action=""/); }); + +test('the streaming renderer refuses .action=${fn} on a native form', async () => { + // The streaming machine's native-prop clause. Mapping every guard call site + // to a test that fails when it is reverted showed this one pinned ONLY by + // `test/bun/form-action-guard.mjs`, so the whole rendering suite stayed green + // with it deleted. Covered here too, since a clause guarded by a single + // cross-runtime script is one file away from being unguarded. + await assert.rejects( + () => drain(renderToStream(html`
`, { ssr: false })), + /function was interpolated into action=/, + ); +}); From 0080dff0182715d350f3cc4b177eee68ea958fdd Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 12:46:58 +0530 Subject: [PATCH 12/38] test: correct the clause map, and stop hand-rolling a case that fits the helper The previous commit set out to fix an inaccurate comment about which guard clause each test pins, and left three more behind. The quoted-bool case was hand-rolled on a stated reason that is simply false: that a quoted `?action` does not fit the helper's good-render step. It does. Quoting moves the hole into `attr-mixed` exactly like the other quoted shapes, so the good value renders `?action="/submit"` and the helper's default check passes. The reasoning was imported from the UNQUOTED bool case, which is the one that really needs an override, because a boolean binding drops the value and writes a bare `action=""`. Going through the helper also restores the two assertions the hand-rolled body had quietly dropped. The file header still described the client guard as living in the 'attr' and 'attr-mixed' cases. There are four, and the file pins all four, so a reader would have believed 'prop' and 'bool' were unguarded here. The section comment had the same shape of error in the other direction: it credited the unquoted cases with pinning 'attr', which is true of the array-wrapped one and false of the unquoted bool. The header now carries the whole map, and every row of it was established by neutering that clause and recording which tests go red, not by reading the renderer. --- .../form-action-attr-guard-client.test.js | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/packages/core/test/rendering/form-action-attr-guard-client.test.js b/packages/core/test/rendering/form-action-attr-guard-client.test.js index c47f2a971..e5a7c50cb 100644 --- a/packages/core/test/rendering/form-action-attr-guard-client.test.js +++ b/packages/core/test/rendering/form-action-attr-guard-client.test.js @@ -1,6 +1,19 @@ // #1154, client half: the browser renderer refuses the same commit, so a -// client re-render can never write a function's source into the live DOM -// (`applyPart`'s 'attr' and 'attr-mixed' cases). Runs under linkedom. +// client re-render can never write a function's source into the live DOM. +// Runs under linkedom. +// +// Four `applyPart` clauses guard this, and the tests below pin all four. +// Established by neutering each one separately, not by reading, because which +// clause a given hole shape reaches is not what it looks like: +// +// 'attr' unquoted `action=${fn}`, including an array-wrapped value +// 'attr-mixed' EVERY quoted single hole (`action="${fn}"`, `.action="${fn}"`, +// `?action="${fn}"`, `@action="${fn}"`) plus a true mixed value +// 'prop' unquoted `.action=${fn}` on a native element +// 'bool' unquoted `?action=${fn}` +// +// The second row is the counter-intuitive one: quoting a hole does not keep it +// in the 'attr' case, it moves it to 'attr-mixed'. import { test, before } from 'node:test'; import assert from 'node:assert/strict'; import { parseHTML } from 'linkedom'; @@ -91,13 +104,10 @@ test('string action still renders on the client', () => { // A quoted binding hole keeps its sigil in the part's name, so comparing the // raw name let `.action="${fn}"` through on this side too. // -// WHICH clause each case pins, verified by reverting them one at a time rather -// than assumed: a quoted single hole compiles to an `attr-mixed` part, NOT a -// plain `attr` one, so the quoted cases below exercise the guard in the -// `attr-mixed` piece loop. Neutering the `attr` case leaves them green. The -// plain `attr` guard is pinned by the UNQUOTED cases (`action=${fn}` and the -// array-wrapped one). Worth stating because the obvious reading is backwards, -// and an edit to the wrong branch would look covered. +// See the clause map in the file header for which `applyPart` branch each +// shape reaches. Within THIS section: the four quoted cases pin 'attr-mixed', +// the array-wrapped case pins 'attr', and the unquoted `?action=${fn}` case +// pins 'bool'. // // Each case renders the SAME template with a good value first and only then // swaps in the bad one, per the note above. That matters twice over: on a fresh @@ -135,21 +145,11 @@ test('client refuses a quoted event hole @action="${fn}"', () => { }); test('client refuses a quoted boolean hole ?action="${fn}"', () => { - // Written directly rather than through the helper: a quoted `?action` is an - // ordinary attribute, so the good render leaves a literal `?action="/submit"` - // rather than the `action` the helper checks for. Kept because dropping it - // would leave this shape with NO client coverage; the SSR machines cover it, - // but this is a separate renderer. - const host = document.createElement('div'); - const tpl = (v) => html`
`; - render(tpl('/submit'), host); - const before = host.innerHTML; - assert.ok(host.querySelector('form'), 'the good value must render a form'); - - assert.throws(() => render(tpl(fakeAction), host), /function was interpolated into \?action=/); - - assert.equal(host.innerHTML, before, 'the refused render must leave the DOM untouched'); - assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); + // Quoting moves this into 'attr-mixed' like the other quoted holes, so the + // good value renders as a literal `?action="/submit"` and the helper's + // default check fits. (Only the UNQUOTED bool below needs an override, since + // a boolean binding drops the value and writes a bare `action=""`.) + refusesOnRerender((v) => html`
`, fakeAction, /function was interpolated into \?action=/); }); test('client refuses an array-wrapped function', () => { From 48fce507154a30af5d7017662182da93ca1e442a Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 12:52:30 +0530 Subject: [PATCH 13/38] test: isolate the browser hosts so a failure names the right test The browser cases mount into the live document, which the reflection assertions need, but never removed their hosts, so `document.body` accumulated every earlier case's markup and the document-wide leak assertions stopped describing the test they sit in. Found by running the counterfactual rather than by reading: with the prop guard reverted, three cases went red across all three engines, and two of them only because the marker from a PREVIOUS case was still in the document. Someone acting on that would have debugged a test that was fine. With each host removed after its test, the same revert fails exactly one case, the reflection one that actually covers the clause. Verified in Chromium, Firefox and WebKit, both green and under the revert. --- .../rendering/browser/form-action-guard.test.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/core/test/rendering/browser/form-action-guard.test.js b/packages/core/test/rendering/browser/form-action-guard.test.js index 8d58406ae..7fc4351a0 100644 --- a/packages/core/test/rendering/browser/form-action-guard.test.js +++ b/packages/core/test/rendering/browser/form-action-guard.test.js @@ -25,12 +25,29 @@ async function secretAction(formData) { return CONNECTION; } +/** + * A host attached to the live document, which the reflection cases need: an + * IDL attribute only reflects on an element that is really in a document. + * + * Each host is removed after its test. Without that, `document.body` accumulates + * every earlier test's markup, and the document-wide leak assertions below stop + * describing the test they sit in. It is not theoretical: with the prop guard + * reverted, the custom-element case failed on the marker left behind by the + * PREVIOUS case, which is a misattribution that would send someone debugging the + * wrong test. + */ +const mounted = []; function mount() { const host = document.createElement('div'); document.body.appendChild(host); + mounted.push(host); return host; } +teardown(() => { + while (mounted.length) mounted.pop().remove(); +}); + suite('form-action guard in a real browser', () => { test('action=${fn} throws and writes nothing to the document', () => { From 4dce2f3c6d674b3be3bd8308dc6bf8b891f346e1 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 12:58:37 +0530 Subject: [PATCH 14/38] test: pin the scope boundary, and stop duplicating the clause map `functions in OTHER attributes keep the existing stringify behaviour` was the test standing guard over this PR's deliberate narrowness, and it asserted nothing that held it there. Its only check was that the output starts with `
{ // A quoted binding hole keeps its sigil in the part's name, so comparing the // raw name let `.action="${fn}"` through on this side too. // -// See the clause map in the file header for which `applyPart` branch each -// shape reaches. Within THIS section: the four quoted cases pin 'attr-mixed', -// the array-wrapped case pins 'attr', and the unquoted `?action=${fn}` case -// pins 'bool'. +// The file header's clause map is keyed by hole SHAPE, and every test below +// names its shape, so read the clause off that map. There is deliberately no +// per-section restatement of it here: an aggregate summary is a second copy +// that drifts from the first, and both times this section tried to carry one it +// ended up wrong (miscounting the quoted cases, and attributing 'attr' to a +// test that pins 'attr-mixed', since two tests here are array-wrapped and they +// land in different clauses). // // Each case renders the SAME template with a good value first and only then // swaps in the bad one, per the note above. That matters twice over: on a fresh diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index b144e7ddc..fc4431bf7 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -133,8 +133,16 @@ test('streaming keeps string-valued actions working', async () => { test('functions in OTHER attributes keep the existing stringify behaviour', async () => { // Narrow claim: only action/formaction throw. Anything else is unchanged // (arguably also a bug, but out of #1154's scope by design). + // + // This pins the SCOPE BOUNDARY, so it has to assert the source really is + // still written out. `out.startsWith('
`, { ssr: true }); - assert.ok(out.startsWith('
Date: Wed, 29 Jul 2026 13:14:37 +0530 Subject: [PATCH 15/38] test: pin the scope boundary on every renderer, not just the buffered one Strengthening the scope-boundary test last time fixed it on `renderToString` and nowhere else. Applying the exact widening that test exists to catch, dropping function values in every attribute, still left the whole suite green when applied to the streaming machine or to the client. So the claim this PR rests on, that only action and formaction are touched, was pinned on one of three renderers. Covered on all three now, plus both SSR machines in the cross-runtime proof, whose passthroughs were every one of them action-valued and so could not have noticed either. Each new case verified against the widening it guards. Also fixes the browser file's own inertness check, which was inert. It asserted `form.action`, the IDL getter, which returns whatever was assigned even where nothing reflects (linkedom does exactly that), so a non-reflecting DOM would have satisfied the assertion written to prove reflection happens. Reads the content attribute now, and passes in Chromium, Firefox and WebKit. The comment above it overstated its case too: it said every assertion below would hold vacuously on a fresh host, when the two `assert.ok(form)` checks would fail rather than pass. Narrowed to the absence assertions, which are the ones that actually go hollow. --- .../browser/form-action-guard.test.js | 18 +++++++++++------- .../form-action-attr-guard-client.test.js | 15 +++++++++++++++ .../rendering/form-action-attr-guard.test.js | 9 +++++++++ test/bun/form-action-guard.mjs | 13 ++++++++++++- 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/packages/core/test/rendering/browser/form-action-guard.test.js b/packages/core/test/rendering/browser/form-action-guard.test.js index 7fc4351a0..16f8caca4 100644 --- a/packages/core/test/rendering/browser/form-action-guard.test.js +++ b/packages/core/test/rendering/browser/form-action-guard.test.js @@ -65,19 +65,23 @@ suite('form-action guard in a real browser', () => { // reflects the stringified function into the live `action` attribute. // // Renders the SAME template with a string first, so there is a real, - // attached form to inspect afterwards. On a fresh host a throwing part - // leaves the container empty, and every assertion below would then hold - // vacuously against a form that does not exist, which is exactly what this - // test would be worth nothing for. + // attached form to inspect afterwards. Without it, a throwing part leaves + // the container empty and the two "no marker in this element" checks below + // would hold against a form that does not exist. (The `assert.ok(form)` + // checks would FAIL in that case rather than pass, so they are not part of + // the problem; it is specifically the absence assertions that go hollow.) const host = mount(); const tpl = (v) => html`
`; render(tpl('/submit'), host); const form = host.querySelector('form'); assert.ok(form, 'the string render must produce a form to inspect'); - // Confirms the property really does reflect in this browser. If it did not, - // the leak this test guards could not happen and the test would be inert. - assert.ok(String(form.action).includes('/submit'), 'action must reflect, else this test proves nothing'); + // Confirms the property really does REFLECT in this browser, which is the + // premise of the whole file. It has to read the CONTENT ATTRIBUTE: the + // `form.action` IDL getter returns what was assigned even where nothing + // reflects (linkedom does exactly that), so asserting on the getter would + // be an inertness check that is itself inert. + assert.equal(form.getAttribute('action'), '/submit', 'action must reflect, else this test proves nothing'); let threw = null; try { render(tpl(secretAction), host); } catch (e) { threw = e; } diff --git a/packages/core/test/rendering/form-action-attr-guard-client.test.js b/packages/core/test/rendering/form-action-attr-guard-client.test.js index 2045d8f76..8cbd150e9 100644 --- a/packages/core/test/rendering/form-action-attr-guard-client.test.js +++ b/packages/core/test/rendering/form-action-attr-guard-client.test.js @@ -197,3 +197,18 @@ test('a self-referential array does not crash the render', () => { render(html`
`, host); assert.equal(host.querySelector('form').getAttribute('action'), ''); }); + +test('the client keeps the same scope boundary for unclaimed attributes', () => { + // The boundary belongs on EVERY renderer. Pinning it on the buffered SSR path + // alone left the widening it guards against invisible here: dropping function + // values in every attribute in `applyPart` kept the whole suite green. + const host = document.createElement('div'); + render(html`
`, host); + const title = host.querySelector('div').getAttribute('title'); + assert.match(title, /CLIENT_SECRET/, 'an unclaimed attribute still stringifies the function'); + + // Same for the mixed path, which is a separate commit site. + const host2 = document.createElement('div'); + render(html`
`, host2); + assert.match(host2.querySelector('div').getAttribute('title'), /CLIENT_SECRET/, 'and on the mixed path'); +}); diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index fc4431bf7..4bcc38c90 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -145,6 +145,15 @@ test('functions in OTHER attributes keep the existing stringify behaviour', asyn assert.match(out, /async function leaky/, 'specifically, the function source'); }); +test('the streaming renderer keeps the same scope boundary', async () => { + // The boundary has to be pinned on EVERY renderer, not just the buffered one. + // Pinning it on `renderToString` alone left the widening this guards against + // invisible on the other two: dropping function values in every attribute in + // the streaming machine kept the whole suite green. + const out = await drain(renderToStream(html`
`, { ssr: false })); + assert.match(out, /async function leaky/, 'an unclaimed attribute still stringifies on the streaming path'); +}); + // --- Bypasses found reviewing the first cut of the guard ------------------- // // The guard originally compared the RAW attribute name. The SSR state machines diff --git a/test/bun/form-action-guard.mjs b/test/bun/form-action-guard.mjs index 2bb983993..44256ac14 100644 --- a/test/bun/form-action-guard.mjs +++ b/test/bun/form-action-guard.mjs @@ -103,6 +103,17 @@ cyclic.push(cyclic); const cyclicOut = await renderToString(html`
`, { ssr: true }); assert.match(cyclicOut, /action=""/, `[${runtime}] a cyclic array must render, not overflow the stack`); +// The SCOPE boundary, on both machines. Every other passthrough above is +// `action`-valued, so none of them would notice a change that widened the claim +// to drop function values in every attribute. This is the one that would. +const otherBuffered = await renderToString(html`
`, { ssr: true }); +assert.match(otherBuffered, /async function leaky/, + `[${runtime}] an unclaimed attribute must still stringify (buffered)`); + +const otherStreamed = await drain(renderToStream(html`
`, { ssr: false })); +assert.match(otherStreamed, /async function leaky/, + `[${runtime}] an unclaimed attribute must still stringify (streaming)`); + // The passthrough must stay byte-identical across runtimes: refusing everything // would also pass the assertions above, so pin what still works. const okBuffered = await renderToString(html`
`, { ssr: true }); @@ -114,4 +125,4 @@ assert.match(okStream, /action="\/submit"/, `[${runtime}] a string action must s const okArray = await renderToString(html`
`, { ssr: true }); assert.match(okArray, /action="\/a,\/b"/, `[${runtime}] an array of strings is not a function`); -console.log(`[${runtime}] form-action guard parity OK: ${Object.keys(refused).length} refused shapes, 3 passthroughs`); +console.log(`[${runtime}] form-action guard parity OK: ${Object.keys(refused).length} refused shapes, ${Object.keys(allowed).length} carve-outs, 5 passthroughs`); From 71c1d15e15e73f91d86511eaeb3ef8e363bc4744 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 13:35:32 +0530 Subject: [PATCH 16/38] test: assert what the stream actually flushed, not what survived the throw `the streaming renderer never emits the source` could not fail. It drained into `drain()`, whose accumulator is a local that the throw discards, so the `out` the test inspected was always the empty string no matter what the renderer put on the wire. Proved it by making the streaming machine enqueue the source and THEN refuse: the whole file stayed green. The distinction matters more here than in most places. A stream can flush bytes before it refuses, and those bytes are already on their way to the client, so "it threw" and "the client received nothing" are different claims and only the second one is worth making. Drains into a caller-owned sink now, asserts on what was actually flushed, and reds against that same probe. The Bun proof had a quieter version of the same problem. Its `leaky` read the marker from a `const` instead of inlining it, and `String(fn)` reproduces source, so the stringified function contained the identifier and never the marker. Every `!includes(BUN_PARITY_SECRET)` assertion in that file was therefore trivially true, including the one checking that a refusal message does not echo what it withholds. Inlined; that assertion now reds when the message embeds the source, and the two passthroughs assert the marker rather than an engine-specific stringification format, which is what the file header already said they would do. Also corrects the browser comment again. The element-scoped assertions do not hold vacuously on an empty host, they throw; the only one that passes is the document-wide check, which is the weakest of them. --- .../browser/form-action-guard.test.js | 13 +++--- .../rendering/form-action-attr-guard.test.js | 43 ++++++++++++++++--- test/bun/form-action-guard.mjs | 19 ++++++-- 3 files changed, 59 insertions(+), 16 deletions(-) diff --git a/packages/core/test/rendering/browser/form-action-guard.test.js b/packages/core/test/rendering/browser/form-action-guard.test.js index 16f8caca4..2cd30d43c 100644 --- a/packages/core/test/rendering/browser/form-action-guard.test.js +++ b/packages/core/test/rendering/browser/form-action-guard.test.js @@ -65,11 +65,14 @@ suite('form-action guard in a real browser', () => { // reflects the stringified function into the live `action` attribute. // // Renders the SAME template with a string first, so there is a real, - // attached form to inspect afterwards. Without it, a throwing part leaves - // the container empty and the two "no marker in this element" checks below - // would hold against a form that does not exist. (The `assert.ok(form)` - // checks would FAIL in that case rather than pass, so they are not part of - // the problem; it is specifically the absence assertions that go hollow.) + // attached form to inspect afterwards. Without it a throwing part leaves the + // container empty, and the only assertion that would still PASS is the + // document-wide one at the end, which is exactly the one that proves least. + // The element-scoped checks do not go quietly in that case: `assert.ok` + // fails on the null form, and the two `after.getAttribute` / `after.action` + // reads throw a TypeError. So the string render is not protecting them from + // being hollow, it is what gives the test an element to make its real claim + // about at all. const host = mount(); const tpl = (v) => html`
`; render(tpl('/submit'), host); diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index 4bcc38c90..77a557105 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -19,6 +19,25 @@ async function drain(stream) { return out; } +/** + * Drain a stream into a CALLER-OWNED buffer, so what was already flushed + * survives a throw. + * + * `drain()` accumulates into a local and loses it when the iteration throws, so + * a test that drains a refused render and then inspects the result is always + * inspecting an empty string, whatever the renderer actually put on the wire. + * That matters here more than it looks: a streaming renderer can flush bytes + * BEFORE it refuses, and those bytes are already on their way to the client. + * "It threw" is not the same claim as "the client received nothing". + * + * @param {any} stream + * @param {{ text: string }} sink written to as chunks arrive + */ +async function drainInto(stream, sink) { + for await (const c of stream) sink.text += typeof c === 'string' ? c : new TextDecoder().decode(c); + return sink.text; +} + // The secret sentinel must never appear in any output, thrown or not. const SECRET = 'postgres://user:SECRET@host/db'; async function leaky(input) { const conn = SECRET; return { success: true, conn, input }; } @@ -91,12 +110,21 @@ test('the streaming renderer refuses the same function (ssr:false path)', async ); }); -test('the streaming renderer never emits the source', async () => { - let out = ''; - try { - out = await drain(renderToStream(html`
`, { ssr: false })); - } catch { /* expected */ } - assert.ok(!out.includes('SECRET'), 'streamed output must not carry the function source'); +test('the streaming renderer never emits the source, not even before it refuses', async () => { + // Uses `drainInto` on purpose. Draining into a local and reading it after the + // catch always sees an empty string, because the throw discards it, so the + // assertion passes no matter what went out. Verified: making the streaming + // machine enqueue the source and THEN refuse left the whole file green. + // + // The claim being pinned is about the wire, not the exception. A stream can + // flush bytes before it refuses, and those bytes are already gone. + const sink = { text: '' }; + await assert.rejects( + () => drainInto(renderToStream(html`
`, { ssr: false }), sink), + /function was interpolated into action=/, + ); + assert.ok(!sink.text.includes('SECRET'), `flushed bytes must not carry the source, got: ${sink.text}`); + assert.ok(!sink.text.includes('async function'), 'no function source of any kind reached the client'); }); // The unquoted shape lands in the streaming machine's `after-eq` branch; the @@ -151,7 +179,8 @@ test('the streaming renderer keeps the same scope boundary', async () => { // invisible on the other two: dropping function values in every attribute in // the streaming machine kept the whole suite green. const out = await drain(renderToStream(html`
`, { ssr: false })); - assert.match(out, /async function leaky/, 'an unclaimed attribute still stringifies on the streaming path'); + assert.match(out, /SECRET/, 'an unclaimed attribute still stringifies on the streaming path'); + assert.match(out, /async function leaky/, 'specifically, the function source'); }); // --- Bypasses found reviewing the first cut of the guard ------------------- diff --git a/test/bun/form-action-guard.mjs b/test/bun/form-action-guard.mjs index 44256ac14..a089246d9 100644 --- a/test/bun/form-action-guard.mjs +++ b/test/bun/form-action-guard.mjs @@ -23,8 +23,15 @@ import { renderToString, renderToStream } from '../../packages/core/src/render-s const runtime = process.versions.bun ? `bun ${process.versions.bun}` : `node ${process.versions.node}`; -const SECRET = 'postgres://user:BUN_PARITY_SECRET@host/db'; -async function leaky(input) { const conn = SECRET; return { ok: true, conn, input }; } +// The marker is written INLINE, not read from a `const` the body references. +// `String(fn)` reproduces the SOURCE, so a body that says `const conn = SECRET` +// stringifies to the identifier and never contains the marker at all. Every +// `!includes(BUN_PARITY_SECRET)` assertion below would then be trivially true +// and this whole file would prove nothing about leaks. +async function leaky(input) { + const conn = 'postgres://user:BUN_PARITY_SECRET@host/db'; + return { ok: true, conn, input }; +} async function drain(stream) { let out = ''; @@ -106,12 +113,16 @@ assert.match(cyclicOut, /action=""/, `[${runtime}] a cyclic array must render, n // The SCOPE boundary, on both machines. Every other passthrough above is // `action`-valued, so none of them would notice a change that widened the claim // to drop function values in every attribute. This is the one that would. +// Asserts on the SECRET marker, not on a stringification format, per the note +// at the top: the marker is inside the function body and every engine that +// stringifies the function at all will carry it, while `async function leaky` +// is a formatting detail JSC and V8 need not agree on. const otherBuffered = await renderToString(html`
`, { ssr: true }); -assert.match(otherBuffered, /async function leaky/, +assert.ok(otherBuffered.includes('BUN_PARITY_SECRET'), `[${runtime}] an unclaimed attribute must still stringify (buffered)`); const otherStreamed = await drain(renderToStream(html`
`, { ssr: false })); -assert.match(otherStreamed, /async function leaky/, +assert.ok(otherStreamed.includes('BUN_PARITY_SECRET'), `[${runtime}] an unclaimed attribute must still stringify (streaming)`); // The passthrough must stay byte-identical across runtimes: refusing everything From 32a23af0763f3ee5ef7cab7bdd1886a34b5a4b6d Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 13:38:54 +0530 Subject: [PATCH 17/38] test: inline the leak marker so the assertions cannot go tautological The Bun proof had a marker its function body referenced through a `const`, so `String(fn)` reproduced the identifier and never the marker, and every absence assertion in the file was trivially true. This file has the same shape and survived it only by luck: the identifier was itself named SECRET, so `/SECRET/` matched the source by coincidence rather than by design. Renaming that one const would have silently turned every check in the file into a tautology, with nothing failing to say so. Inlined, matching the client and browser files, which already do this. The assertions now bind to a value that genuinely appears in the stringified function rather than to an identifier that happens to share its name. Counterfactual with the guard disabled: 20 of 30 red, and the 10 that stay green are exactly the carve-outs and passthroughs. --- .../test/rendering/form-action-attr-guard.test.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index 77a557105..81859a057 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -39,8 +39,17 @@ async function drainInto(stream, sink) { } // The secret sentinel must never appear in any output, thrown or not. -const SECRET = 'postgres://user:SECRET@host/db'; -async function leaky(input) { const conn = SECRET; return { success: true, conn, input }; } +// The marker is INLINE in the body on purpose. `String(fn)` reproduces source, +// so a body that reads the marker from an outer `const` stringifies to the +// IDENTIFIER and never to the value. These assertions happened to survive that +// only because the identifier was itself called SECRET; renaming it would have +// silently turned every `/SECRET/` check into a tautology. The same shape was a +// live defect in `test/bun/form-action-guard.mjs`, where the marker and the +// identifier did not share a name and nothing matched. +async function leaky(input) { + const conn = 'postgres://user:SECRET_MARKER@host/db'; + return { success: true, conn, input }; +} test('unquoted action=${fn} throws instead of leaking source', async () => { await assert.rejects( From 5af00279a0845f9950d88271c8f4736dc7466862 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 13:49:50 +0530 Subject: [PATCH 18/38] test: say what the streaming test actually proves, and why Bun's source differs Three corrections, all to claims rather than to behaviour. The streaming test was described as pinning "what was flushed". It does not, and cannot: `renderToStream` fails through `controller.error()`, which clears the stream's queue, so a chunk enqueued but never read is destroyed rather than delivered. What the helper captures is what the CONSUMER received, which is the quantity that actually matters, and the queue-clearing is a mitigation rather than a gap. Verified by flushing the buffer and enqueueing the source as a separate chunk: a real consumer gets `

hello

hello

`, { ssr: true }); assert.ok(otherBuffered.includes('BUN_PARITY_SECRET'), `[${runtime}] an unclaimed attribute must still stringify (buffered)`); From 6abe61867c04876c0952df9e17b4674207acef7d Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 13:52:02 +0530 Subject: [PATCH 19/38] test: state precisely when a pre-refusal chunk reaches the consumer The note said a chunk enqueued but not yet read is destroyed rather than delivered. That is true only when it lands in the queue and the refusal follows with no yield. If a read is already pending, or the renderer awaits anything before refusing, the chunk goes straight to the consumer. Checked both: with a 5ms yield before the guard, a consumer receives the whole function body. The reassuring part is that the test already behaves correctly across that split, and this only corrects what the comment claims about it. The delivered case is a real leak and reds here; the destroyed case is not a leak and stays green. Every shape that actually puts the source in front of a consumer fails this test, which is what it is for. --- .../rendering/form-action-attr-guard.test.js | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index 82fd3cbec..3cc963d30 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -30,16 +30,23 @@ async function drain(stream) { * * What this captures is precisely the security-relevant quantity, and it is * narrower than "everything the renderer enqueued". `renderToStream` fails via - * `controller.error()`, which per spec CLEARS the stream's queue, so a chunk - * that was enqueued but not yet read is destroyed rather than delivered. - * Verified: patching the streaming machine to flush its buffer and then enqueue - * the source as a separate chunk, a real consumer receives `

hello

delivered. The same patch plus a 5ms yield before the guard hands the + * consumer the whole function body. + * + * The second is a real leak and this test reds on it. The first is not a leak, + * because nothing reached the client, and this test stays green. So the + * coverage lines up with the thing worth caring about: `sink.text` is what a + * client could actually have seen, and every shape that puts the source in + * front of a consumer fails here. * * @param {any} stream * @param {{ text: string }} sink written to as chunks arrive From cab06a8f43ecbe83ef702390119a693aea3dc3b1 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 14:05:19 +0530 Subject: [PATCH 20/38] fix: read the stream the way a real consumer does, and stop overstating the leak The streaming test drained with `for await`, which turns out to see LESS than other consumer shapes. With the machine patched to enqueue the source and then refuse, a `getReader()` loop receives the whole function body while a `for await` drain sees only the prefix, with no yield anywhere. The determinant is not yield-versus-no-yield as the previous note claimed, it is how many microtask hops the consumer's next read costs against the rejection reaching `controller.error()`. That made the test green on a real leak. `renderToStream` has no in-repo consumer, so the shape an HTTP sink actually uses is the one that has to be pinned, and pinning the shape that sees least made a green result meaningless. Reads through `getReader()` now, and reds on both the no-yield and the yielding probe. Separately, the docs overstated the mechanism. "Anything its closure shows, such as a connection string" is not what happens: `Function.prototype.toString` returns source text, so a value read from an outer binding appears as its identifier, never its value. This PR established that fact itself when a test marker had to be inlined to be visible at all, and then shipped prose contradicting it. What does escape is the body: the logic, the query shapes, the table names, and any literal written inline. Smaller than "all your secrets", still bad, and now stated accurately on both surfaces. --- .../webjs/references/muscle-memory-gotchas.md | 4 +- .../rendering/form-action-attr-guard.test.js | 55 ++++++++++--------- website/app/docs/troubleshooting/page.ts | 2 +- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/.agents/skills/webjs/references/muscle-memory-gotchas.md b/.agents/skills/webjs/references/muscle-memory-gotchas.md index 9de86ef93..0aaaf022d 100644 --- a/.agents/skills/webjs/references/muscle-memory-gotchas.md +++ b/.agents/skills/webjs/references/muscle-memory-gotchas.md @@ -47,7 +47,9 @@ There is no React `cache()`, `use()`, or `unstable_cache`. Caching is the `cache Next binds a Server Action with `` and React serializes the binding into hidden fields. WebJs does not read that shape. A function interpolated into `action=` is a hard render error. -The reason is a source leak. During SSR a `.server.ts` import is the ACTUAL function (the RPC stub exists only in the browser), and `action=` is an ordinary attribute hole, so stringifying it would write the function's body, secrets included, into the HTML every visitor downloads. The renderer throws instead, on the server and on the client, for `action=` and `formaction=` alike. +The reason is a source leak. During SSR a `.server.ts` import is the ACTUAL function (the RPC stub exists only in the browser), and `action=` is an ordinary attribute hole, so stringifying it would write the function's body into the HTML every visitor downloads, including any literal inside it. The renderer throws instead, on the server and on the client, for `action=` and `formaction=` alike. + +Precisely what escapes is the SOURCE, not the closure. `Function.prototype.toString` returns source text, so a value the body reads from an outer binding shows up as its identifier and not its value. That is a smaller leak than "all your secrets" and still a bad one: the body gives away your query shapes, your table and column names, your internal paths, and any credential someone wrote inline. The refusal covers the shape, not one spelling of it. Every hole form is refused (`action=${fn}`, `action="${fn}"`, the mixed `action="/x/${fn}"`), and so is a function wrapped in an array (`action=${[fn]}`), since an array stringifies each element through `String()` and leaks identically. diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index 3cc963d30..5a638a92f 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -28,31 +28,32 @@ async function drain(stream) { * inspecting an empty string, whatever the consumer actually got. "It threw" is * not the same claim as "the client received nothing". * - * What this captures is precisely the security-relevant quantity, and it is - * narrower than "everything the renderer enqueued". `renderToStream` fails via - * `controller.error()`, which per spec clears the stream's QUEUE. Whether a - * chunk enqueued shortly before that reaches the consumer depends on where it - * landed, and both cases were checked rather than assumed: + * Reads through `getReader()` rather than `for await`, deliberately. Different + * consumer shapes see DIFFERENT amounts of what was enqueued before the + * refusal, and the difference is not yield-versus-no-yield, it is how many + * microtask hops the consumer's next `read()` costs relative to the rejection + * reaching `controller.error()`. Measured against a streaming machine patched + * to flush its buffer, enqueue the source as a second chunk, then refuse: * - * enqueued into the queue, then `error()` with no yield in between - * -> destroyed. A consumer patched to flush the buffer and enqueue the - * source as a second chunk receives `

hello

receives only the prefix, until any await intervenes * - * The second is a real leak and this test reds on it. The first is not a leak, - * because nothing reached the client, and this test stays green. So the - * coverage lines up with the thing worth caring about: `sink.text` is what a - * client could actually have seen, and every shape that puts the source in - * front of a consumer fails here. + * A `for await` drain therefore reports LESS than a real consumer can get, and + * `renderToStream` has no in-repo consumer, so the shape an HTTP sink actually + * uses (`getReader`, `pipeTo`) is the one that has to be pinned. Reading the + * most permissive shape is what makes a green result meaningful. * * @param {any} stream * @param {{ text: string }} sink written to as chunks arrive */ async function drainInto(stream, sink) { - for await (const c of stream) sink.text += typeof c === 'string' ? c : new TextDecoder().decode(c); + const reader = stream.getReader(); + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + sink.text += typeof value === 'string' ? value : new TextDecoder().decode(value); + } return sink.text; } @@ -138,15 +139,17 @@ test('the streaming renderer refuses the same function (ssr:false path)', async }); test('the streaming renderer never emits the source, not even before it refuses', async () => { - // Uses `drainInto` on purpose. Draining into a local and reading it after the - // catch always sees an empty string, because the throw discards it, so the - // assertion passes no matter what the consumer got. Verified: making the - // streaming machine enqueue the source and THEN refuse left the whole file - // green. + // Two things this has to get right, both learned the hard way. + // + // It reads into a caller-owned sink, because a helper that accumulates into a + // local loses everything when the iteration throws, and a test inspecting + // that local is inspecting an empty string no matter what the consumer got. // - // The claim being pinned is about what a client could have RECEIVED, not - // about the exception. See `drainInto` for why that is narrower than what the - // renderer enqueued, and why the difference is in our favour. + // And it reads through `getReader()`, because consumer shapes do not see the + // same bytes: with the streaming machine patched to enqueue the source and + // then refuse, a reader loop receives the source while a `for await` drain + // sees only the prefix. Pinning the shape that sees the LEAST would make a + // green result meaningless. See `drainInto`. const sink = { text: '' }; await assert.rejects( () => drainInto(renderToStream(html``, { ssr: false }), sink), diff --git a/website/app/docs/troubleshooting/page.ts b/website/app/docs/troubleshooting/page.ts index d02561114..276f2f53b 100644 --- a/website/app/docs/troubleshooting/page.ts +++ b/website/app/docs/troubleshooting/page.ts @@ -43,7 +43,7 @@ export default function Troubleshooting() {

An error saying a function was interpolated into action=

Symptom: a render fails with a function was interpolated into action= on <form>, usually right after binding a server action to a form the way Next.js does it. Where that surfaces depends on which render threw. From a page or layout it propagates, so the nearest error boundary catches it and the message is visible. From inside a component it does not: per-component SSR error isolation contains it, so dev renders an error box in place of the component while production renders the component empty and the page still returns 200. A form that has silently vanished in production, with no error anywhere, is the same bug wearing a disguise; check the server log for the message. In neither case is the action's source emitted.

-

Cause: during SSR an imported 'use server' action is the REAL function (the RPC stub exists only in the browser), and action= is an ordinary attribute hole, so stringifying it would write the action's whole body, including anything its closure shows such as a connection string, into the HTML every visitor downloads. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

+

Cause: during SSR an imported 'use server' action is the REAL function (the RPC stub exists only in the browser), and action= is an ordinary attribute hole, so stringifying it would write the action's whole body into the HTML every visitor downloads: its logic, the query shapes it builds, and any literal written inside it, a connection string or internal path included. (A value the body reads from an outer binding is safe, since Function.prototype.toString returns source text, so an outer const appears only as its identifier. The body itself is quite enough to give away.) WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

Two things stay legal, because neither stringifies its value: a custom element's .action property (an ordinary author-defined property that never reflects, so <my-el .action=\${fn}> is fine), and an unquoted @action=\${fn} event listener, where a function is exactly what is wanted. Quoting a binding hole turns it back into a plain attribute, so @action="\${fn}" IS refused; that is the practical reason invariant 4 requires @, . and ? holes to be unquoted. ?action=\${fn} is refused as well: it never leaked, but a truthy function would silently emit a bare action="", which is never what anyone meant.

Fix: omit action so the form posts to the page's own URL, and handle the submission in that page's action export. A string action is unaffected. See Server Actions and Progressive Enhancement.

From 034b936396ef3150733068759e5e74f73a2c9584 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 14:20:56 +0530 Subject: [PATCH 21/38] fix: pin the worst-case stream consumer, and stop calling outer values safe Two corrections, and the second is the one that mattered. The docs told readers a secret in an outer `const` does not escape, because `Function.prototype.toString` returns source text and an outer binding appears as its identifier. That is true on Node and false on Bun, which this framework supports first-class and which this very PR ships a parity test for. Bun transpiles a module before the engine sees it and constant-folds a module-scope string literal into the body, so an action that reads an API key from an outer const reports the KEY: node 26 Authorization: `Bearer ${VENDOR_API_KEY}` bun 1.3 Authorization: "Bearer sk_live_..." Reassuring someone that their credential is safe, on a runtime where it is not, is worse than saying nothing, so the reassurance is gone from all four surfaces and replaced with the rule that holds everywhere: assume anything reachable from the action is exposed. The Bun proof's own note was self-contradictory on this too, since the file header already said Bun constant folds. The streaming drain also pinned the wrong consumer, again. A chunk handed to a PENDING read request bypasses the queue that `controller.error()` clears, so consumers form a ladder: `for await` sees least, a sequential reader loop a little more, and a reader with several reads outstanding sees the source itself. The previous fix moved from the bottom of that ladder to one rung up and claimed the top. Now keeps a batch outstanding and reds on the padded probe that defeated the sequential version. --- .../webjs/references/muscle-memory-gotchas.md | 12 +++- .../rendering/form-action-attr-guard.test.js | 63 +++++++++++++------ test/bun/form-action-guard.mjs | 17 +++-- website/app/docs/troubleshooting/page.ts | 3 +- 4 files changed, 68 insertions(+), 27 deletions(-) diff --git a/.agents/skills/webjs/references/muscle-memory-gotchas.md b/.agents/skills/webjs/references/muscle-memory-gotchas.md index 0aaaf022d..e6cf11bca 100644 --- a/.agents/skills/webjs/references/muscle-memory-gotchas.md +++ b/.agents/skills/webjs/references/muscle-memory-gotchas.md @@ -49,7 +49,17 @@ Next binds a Server Action with `
` and React serialize The reason is a source leak. During SSR a `.server.ts` import is the ACTUAL function (the RPC stub exists only in the browser), and `action=` is an ordinary attribute hole, so stringifying it would write the function's body into the HTML every visitor downloads, including any literal inside it. The renderer throws instead, on the server and on the client, for `action=` and `formaction=` alike. -Precisely what escapes is the SOURCE, not the closure. `Function.prototype.toString` returns source text, so a value the body reads from an outer binding shows up as its identifier and not its value. That is a smaller leak than "all your secrets" and still a bad one: the body gives away your query shapes, your table and column names, your internal paths, and any credential someone wrote inline. +What escapes is the SOURCE the runtime reports, and how much that includes depends on the runtime. The body always goes: your query shapes, your table and column names, your internal paths, and any credential written inline. + +Whether an OUTER value goes with it is not something to rely on either way. `Function.prototype.toString` returns source text, so on Node a module-scope `const` the body reads appears as its identifier. Bun transpiles the module first and constant-folds that literal into the body, so the same action reports the VALUE: + +``` +// const VENDOR_API_KEY = 'sk_live_…'; then used as `Bearer ${VENDOR_API_KEY}` +node 26 Authorization: `Bearer ${VENDOR_API_KEY}` identifier only +bun 1.3 Authorization: "Bearer sk_live_…" the key itself +``` + +So treat everything reachable from the action as exposed. That is the assumption the refusal is built on, and it is the only one that holds across runtimes. The refusal covers the shape, not one spelling of it. Every hole form is refused (`action=${fn}`, `action="${fn}"`, the mixed `action="/x/${fn}"`), and so is a function wrapped in an array (`action=${[fn]}`), since an array stringifies each element through `String()` and leaks identically. diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index 5a638a92f..bbd61ee9d 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -28,31 +28,53 @@ async function drain(stream) { * inspecting an empty string, whatever the consumer actually got. "It threw" is * not the same claim as "the client received nothing". * - * Reads through `getReader()` rather than `for await`, deliberately. Different - * consumer shapes see DIFFERENT amounts of what was enqueued before the - * refusal, and the difference is not yield-versus-no-yield, it is how many - * microtask hops the consumer's next `read()` costs relative to the rejection - * reaching `controller.error()`. Measured against a streaming machine patched - * to flush its buffer, enqueue the source as a second chunk, then refuse: + * Keeps SEVERAL reads outstanding at once, which is the shape that sees the + * most, and getting to it took three wrong answers worth writing down. * - * getReader() loop -> receives the source, with no yield anywhere - * for await -> receives only `

hello

, the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

+

Cause: during SSR an imported 'use server' action is the REAL function (the RPC stub exists only in the browser), and action= is an ordinary attribute hole, so stringifying it would write the action's whole body into the HTML every visitor downloads: its logic, the query shapes it builds, and any literal written inside it, a connection string or internal path included.

+

Assume outer values are exposed too. On Node, Function.prototype.toString returns source text, so a module-scope const the body reads appears only as its identifier. That is NOT a guarantee you can rely on: Bun transpiles a module before the engine sees it and constant-folds a module-scope string literal straight into the body, so the same action reports Authorization: "Bearer sk_live_…" where Node reports Bearer \${VENDOR_API_KEY}. Whether a secret in an outer binding escapes therefore depends on the runtime and on how the module was transformed. Treat everything reachable from the action as exposed, which is what the refusal assumes. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

Two things stay legal, because neither stringifies its value: a custom element's .action property (an ordinary author-defined property that never reflects, so <my-el .action=\${fn}> is fine), and an unquoted @action=\${fn} event listener, where a function is exactly what is wanted. Quoting a binding hole turns it back into a plain attribute, so @action="\${fn}" IS refused; that is the practical reason invariant 4 requires @, . and ? holes to be unquoted. ?action=\${fn} is refused as well: it never leaked, but a truthy function would silently emit a bare action="", which is never what anyone meant.

Fix: omit action so the form posts to the page's own URL, and handle the submission in that page's action export. A string action is unaffected. See Server Actions and Progressive Enhancement.

From 02e6729abbaeb28c36cdc8b6ac7a246346c3d201 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 14:34:09 +0530 Subject: [PATCH 22/38] fix: make an undersized drain burst fail loudly instead of reporting clean A fixed batch of 16 reads was the fourth wrong answer for this drain, and it restored exactly the false green the previous commit removed: a leak sitting behind 20 flushed chunks passed. The mechanism the docstring gave was wrong too. Nothing is pending during any of this, since every chunk is enqueued synchronously inside the ReadableStream constructor and `controller.error()` runs a microtask after it returns. What a consumer keeps is what its synchronously-issued reads DEQUEUED before that microtask, one chunk per read, which is why every fixed burst is simply a guess about how much a render flushes first. The bound cannot be removed, since the burst has to be sized before anything is awaited. So exhausting it is now a hard failure that names itself, rather than a clean-looking drain. Verified against leaks behind 3, 20 and 100 pad chunks: all three red. Also corrects the Bun folding claim, which was stated with no conditions. Measured on bun 1.3.14: a module-scope `const` string folds whether or not it is exported and however many times it is read, and a `let` does not. Both doc surfaces now say that, and say not to lean on the boundary, since it belongs to a transpiler. The Bun proof's note also cited the file header as evidence, but that sample shows a function-local const and says nothing about module scope, so it now carries its own measurement. --- .../webjs/references/muscle-memory-gotchas.md | 4 +- .../rendering/form-action-attr-guard.test.js | 72 +++++++++++-------- test/bun/form-action-guard.mjs | 16 +++-- website/app/docs/troubleshooting/page.ts | 2 +- 4 files changed, 56 insertions(+), 38 deletions(-) diff --git a/.agents/skills/webjs/references/muscle-memory-gotchas.md b/.agents/skills/webjs/references/muscle-memory-gotchas.md index e6cf11bca..a9e8937eb 100644 --- a/.agents/skills/webjs/references/muscle-memory-gotchas.md +++ b/.agents/skills/webjs/references/muscle-memory-gotchas.md @@ -51,7 +51,7 @@ The reason is a source leak. During SSR a `.server.ts` import is the ACTUAL func What escapes is the SOURCE the runtime reports, and how much that includes depends on the runtime. The body always goes: your query shapes, your table and column names, your internal paths, and any credential written inline. -Whether an OUTER value goes with it is not something to rely on either way. `Function.prototype.toString` returns source text, so on Node a module-scope `const` the body reads appears as its identifier. Bun transpiles the module first and constant-folds that literal into the body, so the same action reports the VALUE: +Whether an OUTER value goes with it is not something to rely on either way. `Function.prototype.toString` returns source text, so on Node a module-scope `const` the body reads appears as its identifier. Bun transpiles the module before the engine sees it and can fold that literal into the body, so the same action reports the VALUE: ``` // const VENDOR_API_KEY = 'sk_live_…'; then used as `Bearer ${VENDOR_API_KEY}` @@ -59,6 +59,8 @@ node 26 Authorization: `Bearer ${VENDOR_API_KEY}` identifier only bun 1.3 Authorization: "Bearer sk_live_…" the key itself ``` +Measured on bun 1.3.14, a module-scope `const` string folds whether or not it is exported and whether it is read once or many times; a `let` does not. Do not build a habit on that boundary, though, since it belongs to a transpiler and can move under you. + So treat everything reachable from the action as exposed. That is the assumption the refusal is built on, and it is the only one that holds across runtimes. The refusal covers the shape, not one spelling of it. Every hole form is refused (`action=${fn}`, `action="${fn}"`, the mixed `action="/x/${fn}"`), and so is a function wrapped in an array (`action=${[fn]}`), since an array stringifies each element through `String()` and leaks identically. diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index bbd61ee9d..50e5e2a30 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -28,51 +28,61 @@ async function drain(stream) { * inspecting an empty string, whatever the consumer actually got. "It threw" is * not the same claim as "the client received nothing". * - * Keeps SEVERAL reads outstanding at once, which is the shape that sees the - * most, and getting to it took three wrong answers worth writing down. + * Issues a large SYNCHRONOUS burst of reads, and fails loudly if the burst was + * not big enough. Four wrong answers preceded this, so the mechanism is worth + * stating exactly rather than by analogy. * - * `controller.error()` clears the stream's queue, so how much of what was - * enqueued before a refusal survives depends entirely on where each chunk - * landed. A chunk enqueued while a read request is PENDING is handed to that - * request directly and never enters the queue, so the reset cannot destroy it. - * A chunk that lands in the queue is gone. Consumers therefore differ, measured - * against a machine patched to flush its buffer, enqueue three pad chunks and - * then the source, and refuse, with no yield anywhere: + * `renderToStream` enqueues everything from inside the `new ReadableStream` + * constructor, synchronously, and `controller.error()` runs a microtask after + * the constructor returns. No read request can be pending during that, since + * `getReader()` does not exist yet. So the queue is fully populated and then + * cleared, and what a consumer keeps is exactly what its synchronously-issued + * `read()` calls DEQUEUED before the clearing microtask ran: N reads in one + * synchronous run recover N chunks, one for one. * - * for await prefix only - * getReader(), sequential loop prefix + pad0 - * one read always outstanding prefix + pad0 + pad1 - * many reads outstanding prefix + pads + THE SOURCE + * That makes consumers a ladder with no top, measured against a machine patched + * to flush its buffer, enqueue pad chunks, enqueue the source, then refuse: * - * So `for await` sees least and a pipelined reader sees most, and a test that - * drains either of the first two calls a real leak clean. `renderToStream` has - * no in-repo consumer, so nothing constrains which shape a host uses, and the - * only safe thing to pin is the worst case. + * for await prefix only + * sequential getReader() loop prefix + 1 chunk + * burst of N prefix + N chunks + * + * Any FIXED burst is therefore guessable: a batch of 16 silently passed a leak + * sitting behind 20 pad chunks. The bound cannot be removed (the burst has to + * be sized before anything is awaited), so instead exhausting it is made a + * hard failure. If every read in the burst returned a chunk, this throws rather + * than reporting a clean drain, which turns the false green into a red that + * says what to do about it. * * @param {any} stream * @param {{ text: string }} sink written to as chunks arrive */ +const DRAIN_BURST = 4096; + async function drainInto(stream, sink) { const reader = stream.getReader(); const decode = (v) => (typeof v === 'string' ? v : new TextDecoder().decode(v)); for (;;) { - // A batch of concurrently-pending reads. Anything enqueued into one of - // these bypasses the queue, so it survives the error that follows. - const batch = Array.from({ length: 16 }, () => reader.read()); + const burst = Array.from({ length: DRAIN_BURST }, () => reader.read()); let done = false; let failure = null; - for (const pending of batch) { + let chunks = 0; + for (const pending of burst) { try { const { done: d, value } = await pending; if (d) done = true; - else if (value !== undefined) sink.text += decode(value); + else if (value !== undefined) { sink.text += decode(value); chunks++; } } catch (e) { - // Record and keep draining: later reads in the batch may still hold - // chunks that were handed over before the error, and those are exactly - // what this helper exists to catch. failure = failure || e; } } + if (!done && !failure && chunks === DRAIN_BURST) { + throw new Error( + `drainInto exhausted its ${DRAIN_BURST}-read burst without reaching the end or an error. ` + + 'The stream had more queued chunks than the burst could dequeue, so this drain is no ' + + 'longer reading the worst case. Raise DRAIN_BURST.', + ); + } if (failure) throw failure; if (done) break; } @@ -167,12 +177,12 @@ test('the streaming renderer never emits the source, not even before it refuses' // local loses everything when the iteration throws, and a test inspecting // that local is inspecting an empty string no matter what the consumer got. // - // And it reads with several requests outstanding at once, because consumer - // shapes do not see the same bytes and that one sees the most: a chunk handed - // to a pending read bypasses the queue that `error()` clears. A `for await` - // drain, and even a sequential reader loop, both call a real leak clean. - // Pinning anything short of the worst case makes a green result meaningless. - // See `drainInto` for the measured ladder. + // And it reads in one large synchronous burst, because what a consumer keeps + // is exactly what its synchronously-issued reads dequeued before the clearing + // microtask: N reads recover N chunks. A `for await` drain, and even a + // sequential reader loop, both call a real leak clean, and any fixed burst is + // guessable, so `drainInto` treats exhausting its burst as a hard failure + // rather than a clean drain. const sink = { text: '' }; await assert.rejects( () => drainInto(renderToStream(html``, { ssr: false }), sink), diff --git a/test/bun/form-action-guard.mjs b/test/bun/form-action-guard.mjs index 6297ae3f0..82d84ff5a 100644 --- a/test/bun/form-action-guard.mjs +++ b/test/bun/form-action-guard.mjs @@ -41,11 +41,17 @@ const runtime = process.versions.bun ? `bun ${process.versions.bun}` : `node ${p // the marker, which would make every `!includes(BUN_PARITY_SECRET)` assertion // below trivially true and this whole file proof of nothing. // -// On Bun it would be load-bearing in the other direction, and that asymmetry is -// the reason to avoid the construction entirely rather than reason about it per -// runtime: Bun constant-folds a module-scope literal into the body (see the -// header), so the SAME code that hides the marker on Node exposes it here. -// Inlining makes the file behave identically on both. +// On Bun it would be load-bearing in the other direction, which is the reason +// to avoid the construction entirely rather than reason about it per runtime: +// Bun folds a module-scope `const` string into the body, so the SAME code that +// hides the marker on Node exposes it here. Measured directly rather than +// inferred from the header sample above, which shows a FUNCTION-LOCAL const and +// says nothing about module scope: +// +// const K = 'sk_live_VALUE'; …`Bearer ${K}` node -> `Bearer ${K}` +// bun -> "Bearer sk_live_VALUE" +// +// Inlining makes this file behave identically on both. async function leaky(input) { const conn = 'postgres://user:BUN_PARITY_SECRET@host/db'; return { ok: true, conn, input }; diff --git a/website/app/docs/troubleshooting/page.ts b/website/app/docs/troubleshooting/page.ts index cdfefe014..0f01183cd 100644 --- a/website/app/docs/troubleshooting/page.ts +++ b/website/app/docs/troubleshooting/page.ts @@ -44,7 +44,7 @@ export default function Troubleshooting() {

An error saying a function was interpolated into action=

Symptom: a render fails with a function was interpolated into action= on <form>, usually right after binding a server action to a form the way Next.js does it. Where that surfaces depends on which render threw. From a page or layout it propagates, so the nearest error boundary catches it and the message is visible. From inside a component it does not: per-component SSR error isolation contains it, so dev renders an error box in place of the component while production renders the component empty and the page still returns 200. A form that has silently vanished in production, with no error anywhere, is the same bug wearing a disguise; check the server log for the message. In neither case is the action's source emitted.

Cause: during SSR an imported 'use server' action is the REAL function (the RPC stub exists only in the browser), and action= is an ordinary attribute hole, so stringifying it would write the action's whole body into the HTML every visitor downloads: its logic, the query shapes it builds, and any literal written inside it, a connection string or internal path included.

-

Assume outer values are exposed too. On Node, Function.prototype.toString returns source text, so a module-scope const the body reads appears only as its identifier. That is NOT a guarantee you can rely on: Bun transpiles a module before the engine sees it and constant-folds a module-scope string literal straight into the body, so the same action reports Authorization: "Bearer sk_live_…" where Node reports Bearer \${VENDOR_API_KEY}. Whether a secret in an outer binding escapes therefore depends on the runtime and on how the module was transformed. Treat everything reachable from the action as exposed, which is what the refusal assumes. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

+

Assume outer values are exposed too. On Node, Function.prototype.toString returns source text, so a module-scope const the body reads appears only as its identifier. That is NOT a guarantee you can rely on: Bun transpiles a module before the engine sees it and can fold a module-scope string literal straight into the body, so the same action reports Authorization: "Bearer sk_live_…" where Node reports Bearer \${VENDOR_API_KEY}. On bun 1.3.14 a const string folds whether or not it is exported and however many times it is read, while a let does not, but that boundary belongs to a transpiler and can move. Whether a secret in an outer binding escapes therefore depends on the runtime and on how the module was transformed. Treat everything reachable from the action as exposed, which is what the refusal assumes. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

Two things stay legal, because neither stringifies its value: a custom element's .action property (an ordinary author-defined property that never reflects, so <my-el .action=\${fn}> is fine), and an unquoted @action=\${fn} event listener, where a function is exactly what is wanted. Quoting a binding hole turns it back into a plain attribute, so @action="\${fn}" IS refused; that is the practical reason invariant 4 requires @, . and ? holes to be unquoted. ?action=\${fn} is refused as well: it never leaked, but a truthy function would silently emit a bare action="", which is never what anyone meant.

Fix: omit action so the form posts to the page's own URL, and handle the submission in that page's action export. A string action is unaffected. See Server Actions and Progressive Enhancement.

From bf4a9140d92cefdf269a171d026c0957ab3a3b7e Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 14:51:29 +0530 Subject: [PATCH 23/38] fix: correct the stream mechanism, the burst boundary, and the folding claim Three corrections, and the folding one is the reason to stop making that claim at all. The drain's note said everything is enqueued synchronously in the ReadableStream constructor and nothing is ever pending. Instrumenting the controller shows otherwise: `streamTemplate` awaits at every text hole, so a three-hole template puts 2 of 6 chunks in the constructor and the other 4 after it, straight into pending read requests. Both paths exist and both matter; the recovery is per read either way, which is the part that actually justifies the burst. The exhaustion guard also miscounted. A stream holding exactly DRAIN_BURST chunks and then closing looks identical to one that ran over, so it threw on a drain that had missed nothing. It now settles the ambiguity with one more read: ended means clean, still producing means the burst really was short. On Bun's constant folding, the honest answer is to stop enumerating conditions. I claimed export status and read count were the axes. A reviewer claimed the opposite of both. Neither is right: on bun 1.3.14 the FIRST module-scope `const` folds and later ones do not, so moving a declaration changes whether a key reaches the HTML. Three wrong rules in three attempts is the signal that this is a transpiler's internal business and does not belong in guidance. The docs now say that plainly and keep the rule that survives it, which is to treat everything reachable from the action as exposed. That last point applies to this file too: the Bun proof's own reasoning depended on declaration order, since `const runtime` already holds the first slot, so the construction it warns about behaves identically on both runtimes here. Inlining is right for a reason that does not depend on any of it. --- .../webjs/references/muscle-memory-gotchas.md | 4 +- .../rendering/form-action-attr-guard.test.js | 55 +++++++++++-------- test/bun/form-action-guard.mjs | 19 +++---- website/app/docs/troubleshooting/page.ts | 2 +- 4 files changed, 44 insertions(+), 36 deletions(-) diff --git a/.agents/skills/webjs/references/muscle-memory-gotchas.md b/.agents/skills/webjs/references/muscle-memory-gotchas.md index a9e8937eb..e597a2af4 100644 --- a/.agents/skills/webjs/references/muscle-memory-gotchas.md +++ b/.agents/skills/webjs/references/muscle-memory-gotchas.md @@ -59,9 +59,9 @@ node 26 Authorization: `Bearer ${VENDOR_API_KEY}` identifier only bun 1.3 Authorization: "Bearer sk_live_…" the key itself ``` -Measured on bun 1.3.14, a module-scope `const` string folds whether or not it is exported and whether it is read once or many times; a `let` does not. Do not build a habit on that boundary, though, since it belongs to a transpiler and can move under you. +Do not go looking for the rule that decides when it folds. There is one, it is a transpiler's internal business, and it is stranger than anyone guesses: on bun 1.3.14 the FIRST module-scope `const` in a file folds and later ones do not, so moving a declaration up a few lines changes whether your key is in the HTML. Two careful readers of this code got that rule wrong in two different ways before it was measured. -So treat everything reachable from the action as exposed. That is the assumption the refusal is built on, and it is the only one that holds across runtimes. +So treat everything reachable from the action as exposed. That is the assumption the refusal is built on, it is the only one that holds across runtimes, and it is the only one that stays true when the transpiler changes. The refusal covers the shape, not one spelling of it. Every hole form is refused (`action=${fn}`, `action="${fn}"`, the mixed `action="/x/${fn}"`), and so is a function wrapped in an array (`action=${[fn]}`), since an array stringifies each element through `String()` and leaks identically. diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index 50e5e2a30..29bc42b56 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -32,27 +32,29 @@ async function drain(stream) { * not big enough. Four wrong answers preceded this, so the mechanism is worth * stating exactly rather than by analogy. * - * `renderToStream` enqueues everything from inside the `new ReadableStream` - * constructor, synchronously, and `controller.error()` runs a microtask after - * the constructor returns. No read request can be pending during that, since - * `getReader()` does not exist yet. So the queue is fully populated and then - * cleared, and what a consumer keeps is exactly what its synchronously-issued - * `read()` calls DEQUEUED before the clearing microtask ran: N reads in one - * synchronous run recover N chunks, one for one. + * Two things decide how much a consumer keeps, and BOTH matter. Chunks written + * while the `new ReadableStream` constructor is still running land in the queue, + * which `controller.error()` later clears. Chunks written after it returns go + * straight to whatever read requests are pending, bypassing the queue entirely, + * and `streamTemplate` awaits at every text hole, so any template with holes + * writes most of its output on that second path. An earlier version of this + * note claimed everything is enqueued synchronously and nothing is ever + * pending; instrumenting the controller shows 2 of 6 enqueues inside the + * constructor for a three-hole template and 4 after it. * - * That makes consumers a ladder with no top, measured against a machine patched - * to flush its buffer, enqueue pad chunks, enqueue the source, then refuse: + * Either way the recovery is per read: N reads issued in one synchronous run + * recover N chunks. So consumers form a ladder with no top, measured against a + * machine patched to flush its buffer, enqueue pad chunks, enqueue the source, + * then refuse: * - * for await prefix only - * sequential getReader() loop prefix + 1 chunk - * burst of N prefix + N chunks + * for await the prefix alone + * sequential getReader() loop the prefix and one more chunk + * burst of N N chunks in total * * Any FIXED burst is therefore guessable: a batch of 16 silently passed a leak - * sitting behind 20 pad chunks. The bound cannot be removed (the burst has to - * be sized before anything is awaited), so instead exhausting it is made a - * hard failure. If every read in the burst returned a chunk, this throws rather - * than reporting a clean drain, which turns the false green into a red that - * says what to do about it. + * sitting behind 20 pad chunks. The bound cannot be removed, since the burst is + * sized before anything is awaited, so instead running out of it is made a hard + * failure rather than a clean-looking drain. * * @param {any} stream * @param {{ text: string }} sink written to as chunks arrive @@ -76,15 +78,22 @@ async function drainInto(stream, sink) { failure = failure || e; } } - if (!done && !failure && chunks === DRAIN_BURST) { + if (failure) throw failure; + if (done) break; + if (chunks === DRAIN_BURST) { + // Every read came back with a chunk, so the burst may have stopped short + // of the end. May, not did: a stream holding EXACTLY this many chunks and + // then closing looks identical here, and throwing on that would be a + // false alarm. One more read settles it, and it is safe to await now + // because a stream still producing has nothing left to protect. + const { done: ended } = await reader.read(); + if (ended) break; throw new Error( - `drainInto exhausted its ${DRAIN_BURST}-read burst without reaching the end or an error. ` - + 'The stream had more queued chunks than the burst could dequeue, so this drain is no ' - + 'longer reading the worst case. Raise DRAIN_BURST.', + `drainInto ran out of its ${DRAIN_BURST}-read burst with the stream still producing. ` + + 'Chunks beyond the burst were never dequeued, so this drain is no longer reading the ' + + 'worst case and a leak could hide behind them. Raise DRAIN_BURST.', ); } - if (failure) throw failure; - if (done) break; } return sink.text; } diff --git a/test/bun/form-action-guard.mjs b/test/bun/form-action-guard.mjs index 82d84ff5a..a82c1e3c2 100644 --- a/test/bun/form-action-guard.mjs +++ b/test/bun/form-action-guard.mjs @@ -41,17 +41,16 @@ const runtime = process.versions.bun ? `bun ${process.versions.bun}` : `node ${p // the marker, which would make every `!includes(BUN_PARITY_SECRET)` assertion // below trivially true and this whole file proof of nothing. // -// On Bun it would be load-bearing in the other direction, which is the reason -// to avoid the construction entirely rather than reason about it per runtime: -// Bun folds a module-scope `const` string into the body, so the SAME code that -// hides the marker on Node exposes it here. Measured directly rather than -// inferred from the header sample above, which shows a FUNCTION-LOCAL const and -// says nothing about module scope: +// On Bun the same construction MIGHT instead expose the marker, because Bun +// folds a module-scope `const` string into the body. Whether it does is not +// something to reason about per file: on bun 1.3.14 only the FIRST module-scope +// `const` folds, so in this file `const runtime` above already occupies that +// slot and a hoisted marker would stay hidden here while folding in a file that +// declared it first. // -// const K = 'sk_live_VALUE'; …`Bearer ${K}` node -> `Bearer ${K}` -// bun -> "Bearer sk_live_VALUE" -// -// Inlining makes this file behave identically on both. +// That is the whole argument for inlining rather than for picking a side. The +// construction's behaviour depends on declaration order in the containing +// module, which is not a property a test should be sensitive to at all. async function leaky(input) { const conn = 'postgres://user:BUN_PARITY_SECRET@host/db'; return { ok: true, conn, input }; diff --git a/website/app/docs/troubleshooting/page.ts b/website/app/docs/troubleshooting/page.ts index 0f01183cd..030ab1329 100644 --- a/website/app/docs/troubleshooting/page.ts +++ b/website/app/docs/troubleshooting/page.ts @@ -44,7 +44,7 @@ export default function Troubleshooting() {

An error saying a function was interpolated into action=

Symptom: a render fails with a function was interpolated into action= on <form>, usually right after binding a server action to a form the way Next.js does it. Where that surfaces depends on which render threw. From a page or layout it propagates, so the nearest error boundary catches it and the message is visible. From inside a component it does not: per-component SSR error isolation contains it, so dev renders an error box in place of the component while production renders the component empty and the page still returns 200. A form that has silently vanished in production, with no error anywhere, is the same bug wearing a disguise; check the server log for the message. In neither case is the action's source emitted.

Cause: during SSR an imported 'use server' action is the REAL function (the RPC stub exists only in the browser), and action= is an ordinary attribute hole, so stringifying it would write the action's whole body into the HTML every visitor downloads: its logic, the query shapes it builds, and any literal written inside it, a connection string or internal path included.

-

Assume outer values are exposed too. On Node, Function.prototype.toString returns source text, so a module-scope const the body reads appears only as its identifier. That is NOT a guarantee you can rely on: Bun transpiles a module before the engine sees it and can fold a module-scope string literal straight into the body, so the same action reports Authorization: "Bearer sk_live_…" where Node reports Bearer \${VENDOR_API_KEY}. On bun 1.3.14 a const string folds whether or not it is exported and however many times it is read, while a let does not, but that boundary belongs to a transpiler and can move. Whether a secret in an outer binding escapes therefore depends on the runtime and on how the module was transformed. Treat everything reachable from the action as exposed, which is what the refusal assumes. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

+

Assume outer values are exposed too. On Node, Function.prototype.toString returns source text, so a module-scope const the body reads appears only as its identifier. That is NOT a guarantee you can rely on: Bun transpiles a module before the engine sees it and can fold a module-scope string literal straight into the body, so the same action reports Authorization: "Bearer sk_live_…" where Node reports Bearer \${VENDOR_API_KEY}. Do not go looking for the rule that decides when it folds. On bun 1.3.14 the FIRST module-scope const in a file folds and later ones do not, so moving a declaration up a few lines changes whether your key ends up in the HTML. Whether a secret in an outer binding escapes therefore depends on the runtime and on how the module was transformed, and it is not a boundary worth building a habit on. Treat everything reachable from the action as exposed, which is what the refusal assumes. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

Two things stay legal, because neither stringifies its value: a custom element's .action property (an ordinary author-defined property that never reflects, so <my-el .action=\${fn}> is fine), and an unquoted @action=\${fn} event listener, where a function is exactly what is wanted. Quoting a binding hole turns it back into a plain attribute, so @action="\${fn}" IS refused; that is the practical reason invariant 4 requires @, . and ? holes to be unquoted. ?action=\${fn} is refused as well: it never leaked, but a truthy function would silently emit a bare action="", which is never what anyone meant.

Fix: omit action so the form posts to the page's own URL, and handle the submission in that page's action export. A string action is unaffected. See Server Actions and Progressive Enhancement.

From 3fb056b0f5185a73c721b6ecab7b47dfce86057e Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 14:53:39 +0530 Subject: [PATCH 24/38] test: say that the streaming drain proves nothing on the shipped path The docstring explains the enqueue mechanism using a three-hole template, which is accurate for that template but not for the one the test actually renders. Instrumenting the controller for `
` records a single event, the error: the guard refuses before anything is enqueued, so `sink.text` is always empty and both assertions hold trivially. That is worth stating rather than leaving for the next reader to discover, because it is the whole reason the drain had to be got right. On the shipped path the test is inert by construction. Its value is entirely in the counterfactual, where a regression that writes before refusing does enqueue, and where a lazy drain reports clean on a real leak. --- .../core/test/rendering/form-action-attr-guard.test.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index 29bc42b56..c3fc65d0a 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -56,6 +56,15 @@ async function drain(stream) { * sized before anything is awaited, so instead running out of it is made a hard * failure rather than a clean-looking drain. * + * Worth being blunt about what this buys on the SHIPPED path: nothing. The + * template the caller below uses has a single attribute hole and no text hole, + * so the guard refuses before anything is enqueued at all, and instrumenting + * the controller records exactly one event, the error. `sink.text` is therefore + * always empty there, and both assertions hold trivially. All of this machinery + * exists for the counterfactual, where a regression that writes before refusing + * DOES enqueue, and where the difference between draining well and draining + * badly is the difference between catching that and waving it through. + * * @param {any} stream * @param {{ text: string }} sink written to as chunks arrive */ From 1bb033721cc22c762eda39e9f37e2c3be164088a Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 15:04:41 +0530 Subject: [PATCH 25/38] docs: stop stating a rule for Bun's constant folding, there isn't a stable one Four rules have now been proposed for when Bun folds a module-scope const into a function body, each measured, each falsified by the next on the same bun version: export status, read count, declaration position, and whether the module has an import. The last of those looked decisive (no import folds everything, an import folds nothing) until an earlier measurement contradicted it, where a module with no import folded only its first const because each const was read by a different function. The honest conclusion is not a fifth rule. Whatever the optimizer keys on is finer than any of these and is nobody's business to encode. All three doc surfaces now say that four attempts were made and all four failed, which is more useful to a reader than a rule that will be wrong again, and they keep the advice that has survived every round: treat everything reachable from the action as exposed. The Bun proof's note gets the same treatment. Its argument for inlining the marker no longer depends on predicting the fold. A test whose marker is visible only under a transpiler heuristic can go quietly tautological when the heuristic shifts, and that is reason enough to remove the dependency rather than reason about it. Also narrows the drain docstring's account of the two enqueue paths. Both exist, but no template in this file has a text hole, so every chunk in every number quoted there is written during the constructor, and the second path plays no part in any of it. --- .../webjs/references/muscle-memory-gotchas.md | 2 +- .../rendering/form-action-attr-guard.test.js | 19 ++++++++++--------- test/bun/form-action-guard.mjs | 17 ++++++++--------- website/app/docs/troubleshooting/page.ts | 2 +- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/.agents/skills/webjs/references/muscle-memory-gotchas.md b/.agents/skills/webjs/references/muscle-memory-gotchas.md index e597a2af4..33bdc4d8d 100644 --- a/.agents/skills/webjs/references/muscle-memory-gotchas.md +++ b/.agents/skills/webjs/references/muscle-memory-gotchas.md @@ -59,7 +59,7 @@ node 26 Authorization: `Bearer ${VENDOR_API_KEY}` identifier only bun 1.3 Authorization: "Bearer sk_live_…" the key itself ``` -Do not go looking for the rule that decides when it folds. There is one, it is a transpiler's internal business, and it is stranger than anyone guesses: on bun 1.3.14 the FIRST module-scope `const` in a file folds and later ones do not, so moving a declaration up a few lines changes whether your key is in the HTML. Two careful readers of this code got that rule wrong in two different ways before it was measured. +Do not go looking for the rule that decides when it folds. Four separate attempts were made to write that rule down while this guard was being reviewed, each measured, and each falsified by the next: export status, read count, declaration position, and whether the module has an import were all proposed and all produced counterexamples on the same bun version. Whatever the optimizer is really keying on, it is not something to encode in a habit. So treat everything reachable from the action as exposed. That is the assumption the refusal is built on, it is the only one that holds across runtimes, and it is the only one that stays true when the transpiler changes. diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index c3fc65d0a..e5fdfca29 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -32,15 +32,16 @@ async function drain(stream) { * not big enough. Four wrong answers preceded this, so the mechanism is worth * stating exactly rather than by analogy. * - * Two things decide how much a consumer keeps, and BOTH matter. Chunks written - * while the `new ReadableStream` constructor is still running land in the queue, - * which `controller.error()` later clears. Chunks written after it returns go - * straight to whatever read requests are pending, bypassing the queue entirely, - * and `streamTemplate` awaits at every text hole, so any template with holes - * writes most of its output on that second path. An earlier version of this - * note claimed everything is enqueued synchronously and nothing is ever - * pending; instrumenting the controller shows 2 of 6 enqueues inside the - * constructor for a three-hole template and 4 after it. + * Chunks written while the `new ReadableStream` constructor is still running + * land in the queue that `controller.error()` later clears, and that is the + * path that matters here: for the template below, and for the patched machine + * the ladder was measured on, every chunk is written during the constructor. + * (A second path exists, since `streamTemplate` awaits at text holes and a + * chunk written after the constructor returns goes straight to a pending read. + * A three-hole template splits 2 in the constructor and 4 after. No template in + * this file has a text hole, so it plays no part in any of the numbers below, + * and it is mentioned only because two earlier versions of this note explained + * the behaviour with whichever path they happened to be wrong about.) * * Either way the recovery is per read: N reads issued in one synchronous run * recover N chunks. So consumers form a ladder with no top, measured against a diff --git a/test/bun/form-action-guard.mjs b/test/bun/form-action-guard.mjs index a82c1e3c2..f9bc65c8d 100644 --- a/test/bun/form-action-guard.mjs +++ b/test/bun/form-action-guard.mjs @@ -41,16 +41,15 @@ const runtime = process.versions.bun ? `bun ${process.versions.bun}` : `node ${p // the marker, which would make every `!includes(BUN_PARITY_SECRET)` assertion // below trivially true and this whole file proof of nothing. // -// On Bun the same construction MIGHT instead expose the marker, because Bun -// folds a module-scope `const` string into the body. Whether it does is not -// something to reason about per file: on bun 1.3.14 only the FIRST module-scope -// `const` folds, so in this file `const runtime` above already occupies that -// slot and a hoisted marker would stay hidden here while folding in a file that -// declared it first. +// On Bun the same construction MIGHT instead expose the marker, because Bun can +// fold a module-scope `const` string into the body. Whether it does here is not +// worth deriving: four attempts to state the folding rule were measured while +// this guard was reviewed, and each was falsified by the next. // -// That is the whole argument for inlining rather than for picking a side. The -// construction's behaviour depends on declaration order in the containing -// module, which is not a property a test should be sensitive to at all. +// That unpredictability IS the argument for inlining. A test whose marker is +// visible only under some transpiler heuristic is a test that can go quietly +// tautological when the heuristic shifts, which is exactly what it must never +// do. Inlining removes the dependency instead of reasoning about it. async function leaky(input) { const conn = 'postgres://user:BUN_PARITY_SECRET@host/db'; return { ok: true, conn, input }; diff --git a/website/app/docs/troubleshooting/page.ts b/website/app/docs/troubleshooting/page.ts index 030ab1329..5b9115c14 100644 --- a/website/app/docs/troubleshooting/page.ts +++ b/website/app/docs/troubleshooting/page.ts @@ -44,7 +44,7 @@ export default function Troubleshooting() {

An error saying a function was interpolated into action=

Symptom: a render fails with a function was interpolated into action= on <form>, usually right after binding a server action to a form the way Next.js does it. Where that surfaces depends on which render threw. From a page or layout it propagates, so the nearest error boundary catches it and the message is visible. From inside a component it does not: per-component SSR error isolation contains it, so dev renders an error box in place of the component while production renders the component empty and the page still returns 200. A form that has silently vanished in production, with no error anywhere, is the same bug wearing a disguise; check the server log for the message. In neither case is the action's source emitted.

Cause: during SSR an imported 'use server' action is the REAL function (the RPC stub exists only in the browser), and action= is an ordinary attribute hole, so stringifying it would write the action's whole body into the HTML every visitor downloads: its logic, the query shapes it builds, and any literal written inside it, a connection string or internal path included.

-

Assume outer values are exposed too. On Node, Function.prototype.toString returns source text, so a module-scope const the body reads appears only as its identifier. That is NOT a guarantee you can rely on: Bun transpiles a module before the engine sees it and can fold a module-scope string literal straight into the body, so the same action reports Authorization: "Bearer sk_live_…" where Node reports Bearer \${VENDOR_API_KEY}. Do not go looking for the rule that decides when it folds. On bun 1.3.14 the FIRST module-scope const in a file folds and later ones do not, so moving a declaration up a few lines changes whether your key ends up in the HTML. Whether a secret in an outer binding escapes therefore depends on the runtime and on how the module was transformed, and it is not a boundary worth building a habit on. Treat everything reachable from the action as exposed, which is what the refusal assumes. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

+

Assume outer values are exposed too. On Node, Function.prototype.toString returns source text, so a module-scope const the body reads appears only as its identifier. That is NOT a guarantee you can rely on: Bun transpiles a module before the engine sees it and can fold a module-scope string literal straight into the body, so the same action reports Authorization: "Bearer sk_live_…" where Node reports Bearer \${VENDOR_API_KEY}. Do not go looking for the rule that decides when it folds. Four different rules were proposed and measured while this guard was being written, and every one of them was falsified by the next on the same Bun version. Whether a secret in an outer binding escapes therefore depends on the runtime and on how the module was transformed, and it is not a boundary worth building a habit on. Treat everything reachable from the action as exposed, which is what the refusal assumes. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

Two things stay legal, because neither stringifies its value: a custom element's .action property (an ordinary author-defined property that never reflects, so <my-el .action=\${fn}> is fine), and an unquoted @action=\${fn} event listener, where a function is exactly what is wanted. Quoting a binding hole turns it back into a plain attribute, so @action="\${fn}" IS refused; that is the practical reason invariant 4 requires @, . and ? holes to be unquoted. ?action=\${fn} is refused as well: it never leaked, but a truthy function would silently emit a bare action="", which is never what anyone meant.

Fix: omit action so the form posts to the page's own URL, and handle the submission in that page's action export. A string action is unaffected. See Server Actions and Progressive Enhancement.

From 1e43a3df47bf5db1d3cb5c7e7e16e01b1026fbcd Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 18:11:24 +0530 Subject: [PATCH 26/38] fix: contain a throw from an async render commit A commit that threw (a refused binding, a value whose toString throws) escaped the renderError boundary that the fetch half already had. .then(onFulfil, onRejected) does not route onFulfil's own throw to its sibling handler, and _performRender's .then had no rejection handler, so the error surfaced as an unhandled rejection in production, renderError() never ran, __pendingAsyncCommits stayed >= 1 forever, and updateComplete never settled for that instance again. Reached by the #1154 guard, but not caused by it: any synchronous throw from a commit behaves the same, verified with an unrelated failing toString. Refusing to leak must not become a new way to hang a component. --- .agents/skills/webjs/references/components.md | 2 +- packages/core/src/component.js | 18 +++++++++- .../lifecycle/component-lifecycle.test.js | 34 +++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/.agents/skills/webjs/references/components.md b/.agents/skills/webjs/references/components.md index f7b4d5e6e..cfbcd5e4a 100644 --- a/.agents/skills/webjs/references/components.md +++ b/.agents/skills/webjs/references/components.md @@ -175,7 +175,7 @@ Three decoupled concerns, do not conflate them. 2. **The client re-fetch default is stale-while-revalidate.** When a prop or dependency change re-runs `async render()`, the previous content stays until the new render resolves. No blank, no flash, no user code. 3. **`renderFallback()` is the OPTIONAL re-fetch loading UI.** Shown ONLY during a client re-fetch, NEVER on first paint, and it does NOT create a server-streaming boundary. -Errors are isolated per component by default (no user code): a thrown `await` renders a component-scoped error state while siblings render, never bubbling to the route `error.ts`. Override `renderError(error)` only to customize it (dev shows the message, prod stays silent). +Errors are isolated per component by default (no user code): a thrown `await` renders a component-scoped error state while siblings render, never bubbling to the route `error.ts`. Override `renderError(error)` only to customize it (dev shows the message, prod stays silent). The boundary covers the COMMIT as well as the fetch, so a template that throws while being applied (a refused binding, a value whose `toString` throws) reaches `renderError()` too, and `updateComplete` still settles. Those two halves used to disagree: a fetch rejection was contained and a commit throw escaped as an unhandled rejection that also left `updateComplete` pending forever. Decision rules. Use `async render()` for request-time server data that should be in the first paint (the default). Add `renderFallback()` when a client re-fetch's stale content would mislead. Use `Task` / signals for genuinely client-only data (a click, viewport, live updates). For SLOW data where blocking the first byte hurts, wrap the region in `` to stream it (the only way to show a first-paint fallback; see `client-router-and-streaming.md`). Do NOT fetch in `connectedCallback` for data knowable server-side, and do NOT prop-drill what a leaf can fetch itself. diff --git a/packages/core/src/component.js b/packages/core/src/component.js index 6bf121666..4bcd54006 100644 --- a/packages/core/src/component.js +++ b/packages/core/src/component.js @@ -1399,7 +1399,23 @@ class WebComponentBase extends Base { return Promise.resolve(pending).then( (tpl) => { if (token !== this.__renderToken) return; // superseded by a newer render - clientRender(tpl, this._renderRoot); + // The COMMIT throws too, not just the fetch, and a sibling rejection + // handler cannot see it: `.then(onFulfil, onRejected)` never routes + // onFulfil's own throw to onRejected. Left uncaught, the returned + // promise rejected and _performRender's `.then` had no rejection + // handler, so the error escaped as an unhandled rejection, + // renderError() never ran, __pendingAsyncCommits never decremented + // (wedging it >= 1 forever, which also disables the non-committing + // cycle's _resolveUpdate escape), and updateComplete never settled. + // The sync path routes an identical throw to the same boundary, so + // this is what makes the two paths agree. + try { + clientRender(tpl, this._renderRoot); + } catch (commitError) { + this._handleRenderError( + commitError instanceof Error ? commitError : new Error(String(commitError)), + ); + } }, (error) => { if (token !== this.__renderToken) return; diff --git a/packages/core/test/lifecycle/component-lifecycle.test.js b/packages/core/test/lifecycle/component-lifecycle.test.js index a8f2e7749..7657f54f1 100644 --- a/packages/core/test/lifecycle/component-lifecycle.test.js +++ b/packages/core/test/lifecycle/component-lifecycle.test.js @@ -318,6 +318,40 @@ test('renderError catches exceptions thrown from render() and uses its fallback' assert.ok(el, 'component survived a throwing render'); }); +test('a throw from the COMMIT of an async render() is contained like a sync one', async () => { + // Regression: `.then(onFulfil, onRejected)` does not route onFulfil's own + // throw to onRejected, so a commit that threw (a guard refusal, a value with + // a throwing toString) rejected the pending commit, and _performRender's + // `.then` had no rejection handler. The error escaped as an unhandled + // rejection, renderError() never ran, and updateComplete never settled. + // + // Asserted on the three observable consequences rather than the mechanism, + // because each one reds on its own when the try/catch in _commitAsync is + // reverted (measured: renderError not called, updateComplete never settles, + // __pendingAsyncCommits stuck at 1). The value below throws from String(), + // which is what a commit does to an attribute hole. + const boom = { toString() { throw new Error('commit failed'); } }; + let errorArg = null; + class C extends WebComponent { + async render() { await 0; return html`
`; } + renderError(e) { errorArg = e; return html`

fallback

`; } + } + C.register('async-commit-throw'); + const el = document.createElement('async-commit-throw'); + document.body.appendChild(el); + + // Bounded, and crossing the bound is a hard failure that names itself + // rather than a pass: an unsettled updateComplete is the bug. + const settled = await Promise.race([ + Promise.resolve(el.updateComplete).then(() => 'settled', () => 'settled'), + new Promise((r) => setTimeout(() => r('NEVER SETTLED'), 500)), + ]); + assert.equal(settled, 'settled', 'updateComplete must settle after a failed async commit'); + assert.ok(errorArg instanceof Error, 'renderError() receives the commit error'); + assert.match(errorArg.message, /commit failed/, 'and the real error, not a wrapper'); + assert.equal(el.__pendingAsyncCommits, 0, 'the in-flight count is released, not wedged'); +}); + /* -------------------- lazy controllers set: ensure graceful behavior -------------------- */ test('WebComponent without static properties still constructs cleanly', () => { From 352c34839decf1a4db63fd25576758929a9dca4e Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 18:13:08 +0530 Subject: [PATCH 27/38] test: pin the guard's case folding, the one branch nothing covered Mutation-tested every branch of isFormActionAttr against the suite: seven of eight mutants red it, and the survivor was .toLowerCase(). Dropping it kept all 46 unit tests and the whole Bun table green while
and `, host), + /function was interpolated into formaction=/, + ); +}); + +test('client re-render swapping in an upper-case ACTION=${fn} throws, live DOM stays clean', () => { + const host = document.createElement('div'); + const tpl = (a) => html`
`; + render(tpl('/ok'), host); + assert.throws(() => render(tpl(fakeAction), host), /function was interpolated into action=/); + assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'live DOM must not carry the source'); +}); + // Re-render over an ALREADY-MOUNTED form. Here the host really does hold a live // element, so if the guard let the value through, the source would be sitting // in the DOM and this would catch it. diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index e5fdfca29..8a10a33d1 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -149,6 +149,44 @@ test('formaction=${fn} on a submit button throws', async () => { ); }); +// Case normalization was the ONE branch of the guard with no test. Mutation +// testing every branch against this suite, seven of eight mutants red it and +// this was the survivor: dropping `.toLowerCase()` from isFormActionAttr kept +// all 46 unit tests and the whole Bun table green while `
` +// and `
`, { ssr: true }), + /function was interpolated into formaction=/, + ); +}); + +test('upper-case ACTION=${fn} throws', async () => { + await assert.rejects( + () => renderToString(html`
`, { ssr: true }), + /function was interpolated into action=/, + ); +}); + +test('a quoted mixed-case Action="${fn}" throws (sigil strip and case-fold compose)', async () => { + await assert.rejects( + () => renderToString(html`
`, { ssr: true }), + /function was interpolated into action=/, + ); +}); + +test('the streaming renderer folds case too', async () => { + await assert.rejects( + () => drain(renderToStream(html``, { ssr: false })), + /function was interpolated into formaction=/, + ); +}); + test('no thrown message ever carries the function source', async () => { for (const tpl of [ html`
`, diff --git a/test/bun/form-action-guard.mjs b/test/bun/form-action-guard.mjs index f9bc65c8d..f02482769 100644 --- a/test/bun/form-action-guard.mjs +++ b/test/bun/form-action-guard.mjs @@ -81,6 +81,12 @@ const refused = { 'native prop .action=${fn}': () => html`
`, 'unquoted bool ?action=${fn}': () => html`
`, 'array-wrapped action=${[fn]}': () => html`
`, + // Case folding, on both runtimes: every other row here spells the attribute + // lowercase, and with those alone the `.toLowerCase()` in isFormActionAttr + // could be deleted with this whole table still green while `formAction=` + // leaked. camelCase is React's spelling, so it is the likeliest arrival. + 'camelCase formAction=${fn}': () => html``, + 'upper-case ACTION=${fn}': () => html`
`, }; for (const [name, mk] of Object.entries(refused)) { From a77bdbef39b05d234463e6500bf1d3a5c0a37b9e Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 18:13:46 +0530 Subject: [PATCH 28/38] docs: note the action= carve-out where the binding rules are stated The components page states the three binding rules unqualified: an attribute hole is stringified, a native property binding drops at SSR, a truthy boolean hole emits the attribute. The form-action guard makes all three false for action= and formaction=, so a reader following that page writes .action=${handler} on a form and hits a hard render error the page says cannot happen. --- website/app/docs/components/page.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/website/app/docs/components/page.ts b/website/app/docs/components/page.ts index ed413806d..983951db9 100644 --- a/website/app/docs/components/page.ts +++ b/website/app/docs/components/page.ts @@ -645,6 +645,8 @@ render() {

Properties vs Attributes in Templates

Templates support three binding prefixes for setting values on elements:

+

One carve-out applies to all three below: a function in action= or formaction= is refused, not stringified. Stringifying a function writes its source into the HTML, and during SSR an imported 'use server' action is the real function, so that source is the action's whole body. Every hole form is refused under those two names (plain, quoted, mixed, array-wrapped, and .action=\${fn} on a native form, which reflects into the live DOM). A function stays legal on an unquoted @action=\${fn} listener and on a custom element's own .action property, because neither stringifies it. So where the rules below say a value is stringified, that a native property binding drops at SSR, or that a boolean hole emits the attribute when truthy, read them as holding for every attribute except these two. See Troubleshooting.

+

Regular Attributes: attr=\${value}

Sets an HTML attribute. The value is stringified. If the value is null, undefined, or false, the attribute is removed.

From f35ca6d1c667494eda821e89772f57fd689fafd7 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 18:47:35 +0530 Subject: [PATCH 29/38] docs: correct the .action carve-out and drop the process narration Two problems on the same surfaces. The carve-out said a custom element's .action is safe because the property 'never reflects'. A prop declared reflect: true does reflect, via a path outside the template commit sites the guard covers, so it writes String(fn) into the attribute. Measured: a reflecting action prop emits the function body at SSR, and on the client the same binding puts it in a live attribute. The conclusion still holds for a plain property, which is what the sentence now says. The rest is narration of this change's own development ('four rules were proposed while this guard was being written', 'it already drifted once', 'twice drifted apart') sitting on a public docs page, in the agent skill, in a source docblock, and in a CI comment. A reader six months out cannot verify any of it and goes looking through git history for regressions that never shipped. The durable reason survives without the history. Also corrects the blog dogfood conventions, which still taught
bound to a server action, the exact shape now refused. --- .../webjs/references/muscle-memory-gotchas.md | 4 ++-- .github/workflows/ci.yml | 2 +- examples/blog/.agents/rules/workflow.md | 10 +++++++--- examples/blog/CONVENTIONS.md | 10 +++++++--- packages/core/src/form-action.js | 16 +++++++++------- packages/core/src/render-server.js | 9 ++++----- website/app/docs/troubleshooting/page.ts | 4 ++-- 7 files changed, 32 insertions(+), 23 deletions(-) diff --git a/.agents/skills/webjs/references/muscle-memory-gotchas.md b/.agents/skills/webjs/references/muscle-memory-gotchas.md index 33bdc4d8d..c44a83ec0 100644 --- a/.agents/skills/webjs/references/muscle-memory-gotchas.md +++ b/.agents/skills/webjs/references/muscle-memory-gotchas.md @@ -59,7 +59,7 @@ node 26 Authorization: `Bearer ${VENDOR_API_KEY}` identifier only bun 1.3 Authorization: "Bearer sk_live_…" the key itself ``` -Do not go looking for the rule that decides when it folds. Four separate attempts were made to write that rule down while this guard was being reviewed, each measured, and each falsified by the next: export status, read count, declaration position, and whether the module has an import were all proposed and all produced counterexamples on the same bun version. Whatever the optimizer is really keying on, it is not something to encode in a habit. +Do not go looking for the rule that decides when it folds. Export status, read count, declaration position, and whether the module has an import have each been measured as the deciding factor and each produced a counterexample on the same bun version, so whatever the optimizer keys on is finer than any of them. The two rows above are one measurement on two specific versions, not a per-runtime guarantee: read them as proof that the boundary moves, never as a promise that Node keeps an outer binding private. So treat everything reachable from the action as exposed. That is the assumption the refusal is built on, it is the only one that holds across runtimes, and it is the only one that stays true when the transpiler changes. @@ -71,7 +71,7 @@ Two carve-outs, both because those bindings never stringify their value: |---|---|---| | `action=` / `formaction=` | yes | ordinary attribute, stringified into the HTML | | `.action=` on a native form | yes | the property reflects, so the source lands in the DOM on the client | -| `.action=` on a custom element | **no** | an ordinary author-defined property, never reflected; a function is a legitimate value | +| `.action=` on a custom element | **no** | an author-defined property, not a reflected IDL attribute; a function is a legitimate value. Holds for a PLAIN prop: one declared `reflect: true` writes `String(value)` to the attribute on a path outside these commit sites, so it still emits the source | | `?action=` | yes | never leaked, but it is meaningless, so it is refused rather than silently emitting a bare `action=""` | | `@action=` unquoted | **no** | an event listener, and a function is exactly what one takes | | `@action="${fn}"` quoted | yes | quoting makes it an ordinary attribute again, so it leaks | diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c20801d30..3f0ffbde5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -111,7 +111,7 @@ jobs: # The form-action leak guard (#1154) on Bun: a divergence here is a # divergence in whether a server action's SOURCE reaches the served HTML, # so it cannot be left to the Node suite alone. Covers both SSR state - # machines, which have twice drifted apart on the binding shapes. + # machines, which are independent and must be kept in step by hand. - name: WebJs form-action guard parity on Bun run: bun test/bun/form-action-guard.mjs # Listener overhead reductions on Bun (#756): the out-of-band IP stamp (no diff --git a/examples/blog/.agents/rules/workflow.md b/examples/blog/.agents/rules/workflow.md index b2bbef9e0..ca11c6a2e 100644 --- a/examples/blog/.agents/rules/workflow.md +++ b/examples/blog/.agents/rules/workflow.md @@ -139,9 +139,13 @@ self-review loop. is never called on the server, so anything there only runs after hydration. Initial data for components comes from the page function (server-side fetch plus pass as attribute/property), NOT from `fetch` calls - in `connectedCallback`. For write-paths, prefer `` plus - server action over `fetch` plus click handler. The framework upgrades plain - forms to partial-swap submissions automatically. + in `connectedCallback`. For write-paths, prefer a plain `` + posting to the page's own URL, handled by that page's `action` export, over + `fetch` plus a click handler. The framework upgrades plain forms to + partial-swap submissions automatically. Never interpolate a server action + into the attribute (``): that Next binding is + refused at render time, since stringifying the function would leak its + source into the HTML. - **Client navigation is auto-magic.** Real `` and `` get partial-swap behavior with no opt-in. Because layouts persist across navigation, put shared chrome (sidenav, header) in `layout.ts` and diff --git a/examples/blog/CONVENTIONS.md b/examples/blog/CONVENTIONS.md index 8f2ea1403..d6f3855b3 100644 --- a/examples/blog/CONVENTIONS.md +++ b/examples/blog/CONVENTIONS.md @@ -698,9 +698,13 @@ state. Only the *interactivity itself* (the +/- click, the open/close toggle, the tab switch) requires JS. **Default rules:** -- **Forms must work as plain HTML POSTs.** Use `` bound - to a server action. Never `fetch` + a JS click handler for the - happy path. The framework upgrades the form to a partial-swap +- **Forms must work as plain HTML POSTs.** Post to the page's own URL + (omit `action` entirely) and handle the submission in that page's + `action` export. Never `fetch` + a JS click handler for the happy + path. Do NOT interpolate a server action into the attribute + (``): that is the Next binding, it is not + how WebJs reaches an action, and it is refused at render time because + stringifying the function would write its source into the HTML. The framework upgrades the form to a partial-swap submission automatically when the client router is active, and with JS disabled the same form does a full-page POST and works identically end-to-end. diff --git a/packages/core/src/form-action.js b/packages/core/src/form-action.js index c003bb864..11de72b4f 100644 --- a/packages/core/src/form-action.js +++ b/packages/core/src/form-action.js @@ -4,13 +4,15 @@ import { isBindingPrefix } from './binding-prefixes.js'; * Form-action attribute guard (#1154). * * Both SSR state machines and the client renderer import from here so the - * rule cannot drift between them. It already drifted once: the guard was - * added to the buffered `renderTemplate` alone, leaving `streamTemplate` - * stringifying the function. That second machine is reached only through - * `renderToStream(v, { ssr: false })`, which no page render uses (the server - * renders every page, Suspense included, via `renderToString`), so it was a - * public-API hole rather than a live page leak. Worth closing on its own - * terms, and worth noting as the reason the rule lives in one place. + * rule cannot drift between them. There are four commit sites across three + * renderers and they are easy to change one at a time, which is why the + * predicate lives in one module rather than at each site. + * + * The second SSR machine (`streamTemplate`) is reached only through + * `renderToStream(v, { ssr: false })`, which no page render uses: the server + * renders every page, Suspense included, via `renderToString`. Guarding it is + * about the public API surface rather than a live page leak, so do not infer + * from a bug there that pages were affected. */ /** diff --git a/packages/core/src/render-server.js b/packages/core/src/render-server.js index f4586c702..d2e24604d 100644 --- a/packages/core/src/render-server.js +++ b/packages/core/src/render-server.js @@ -1919,11 +1919,10 @@ async function streamTemplate(tr, ctx, controller) { attrName = ''; } else { // The SAME guard as the buffered renderer above. This is a second, - // independent state machine, so it does not inherit that one: before - // this it stringified the function while the buffered path already - // refused it. Reached only via `renderToStream(v, { ssr: false })`, - // which no page render uses, so this was a public-API hole rather - // than a live page leak. + // independent state machine, so it inherits nothing from that one; + // a change to the rule has to land in both. Reached only via + // `renderToStream(v, { ssr: false })`, which no page render uses, so + // this covers the public API surface rather than a page leak. assertNotFunctionActionAttr(val, attrName, currentTag); buf += `"${escapeAttr(String(val ?? ''))}"`; state = 'in-tag'; diff --git a/website/app/docs/troubleshooting/page.ts b/website/app/docs/troubleshooting/page.ts index 5b9115c14..0a80d4f23 100644 --- a/website/app/docs/troubleshooting/page.ts +++ b/website/app/docs/troubleshooting/page.ts @@ -44,8 +44,8 @@ export default function Troubleshooting() {

An error saying a function was interpolated into action=

Symptom: a render fails with a function was interpolated into action= on <form>, usually right after binding a server action to a form the way Next.js does it. Where that surfaces depends on which render threw. From a page or layout it propagates, so the nearest error boundary catches it and the message is visible. From inside a component it does not: per-component SSR error isolation contains it, so dev renders an error box in place of the component while production renders the component empty and the page still returns 200. A form that has silently vanished in production, with no error anywhere, is the same bug wearing a disguise; check the server log for the message. In neither case is the action's source emitted.

Cause: during SSR an imported 'use server' action is the REAL function (the RPC stub exists only in the browser), and action= is an ordinary attribute hole, so stringifying it would write the action's whole body into the HTML every visitor downloads: its logic, the query shapes it builds, and any literal written inside it, a connection string or internal path included.

-

Assume outer values are exposed too. On Node, Function.prototype.toString returns source text, so a module-scope const the body reads appears only as its identifier. That is NOT a guarantee you can rely on: Bun transpiles a module before the engine sees it and can fold a module-scope string literal straight into the body, so the same action reports Authorization: "Bearer sk_live_…" where Node reports Bearer \${VENDOR_API_KEY}. Do not go looking for the rule that decides when it folds. Four different rules were proposed and measured while this guard was being written, and every one of them was falsified by the next on the same Bun version. Whether a secret in an outer binding escapes therefore depends on the runtime and on how the module was transformed, and it is not a boundary worth building a habit on. Treat everything reachable from the action as exposed, which is what the refusal assumes. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

-

Two things stay legal, because neither stringifies its value: a custom element's .action property (an ordinary author-defined property that never reflects, so <my-el .action=\${fn}> is fine), and an unquoted @action=\${fn} event listener, where a function is exactly what is wanted. Quoting a binding hole turns it back into a plain attribute, so @action="\${fn}" IS refused; that is the practical reason invariant 4 requires @, . and ? holes to be unquoted. ?action=\${fn} is refused as well: it never leaked, but a truthy function would silently emit a bare action="", which is never what anyone meant.

+

Assume outer values are exposed too. On Node, Function.prototype.toString returns source text, so a module-scope const the body reads appears only as its identifier. That is NOT a guarantee you can rely on: Bun transpiles a module before the engine sees it and can fold a module-scope string literal straight into the body, so the same action reports Authorization: "Bearer sk_live_…" where Node reports Bearer \${VENDOR_API_KEY}. Do not go looking for the rule that decides when it folds. It is a transpiler's internal choice rather than a documented boundary, and two modules on the same Bun version can differ. Whether a secret in an outer binding escapes therefore depends on the runtime and on how the module was transformed, and it is not a boundary worth building a habit on. Treat everything reachable from the action as exposed, which is what the refusal assumes. WebJs refuses rather than emitting it. The refusal covers the shape rather than one spelling: formaction= as well as action=, every hole form (action=\${fn}, action="\${fn}", the mixed action="/x/\${fn}"), and a function wrapped in an array (action=\${[fn]}), which stringifies its elements the same way. .action=\${fn} on a native form is refused at SSR too, even though the property is dropped there, so a page cannot render clean on the server and then throw on hydration, where the reflected IDL attribute would carry the source into the live DOM.

+

Two things stay legal, because neither stringifies its value: a custom element's .action property (an author-defined property, not the reflected IDL attribute a native <form> has, so <my-el .action=\${fn}> is fine), and an unquoted @action=\${fn} event listener, where a function is exactly what is wanted. One qualification on the first: that holds for a plain property, and NOT if the component declares it reflect: true. Reflection is a separate path that writes String(value) into the attribute without passing through the template commit sites this guard covers, so a reflecting action prop still emits the function's source. That path is a general problem with reflecting any function-valued prop rather than an action one, and it is tracked separately. Quoting a binding hole turns it back into a plain attribute, so @action="\${fn}" IS refused; that is the practical reason invariant 4 requires @, . and ? holes to be unquoted. ?action=\${fn} is refused as well: it never leaked, but a truthy function would silently emit a bare action="", which is never what anyone meant.

Fix: omit action so the form posts to the page's own URL, and handle the submission in that page's action export. A string action is unaffected. See Server Actions and Progressive Enhancement.

An error saying static properties is no longer supported

From 4abec563016d79b331fdfd21e9a0549f9bac5e44 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 18:48:35 +0530 Subject: [PATCH 30/38] test: prove commit containment in a real browser, not just linkedom AGENTS.md holds that a unit test is necessary but not sufficient for a component change, and this path only ever runs in a browser. The linkedom test substitutes a value with a throwing toString on a detached container; the shipped combination is an async render returning .action=${fn} on a native form, where action is a reflected IDL attribute no DOM shim models. Sits beside the existing rejected-fetch test, which covers the other half of the same boundary. Counterfactual: reverting the try/catch reds this test on Chromium, Firefox and WebKit. --- .../browser/async-render-client.test.js | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/packages/core/test/suspense/browser/async-render-client.test.js b/packages/core/test/suspense/browser/async-render-client.test.js index 0790648fc..ef4639734 100644 --- a/packages/core/test/suspense/browser/async-render-client.test.js +++ b/packages/core/test/suspense/browser/async-render-client.test.js @@ -161,6 +161,56 @@ suite('async render() on the client', () => { } }); + // Sibling of the test above, and a different failure: there the FETCH + // rejects, here the fetch resolves and the COMMIT throws. The two used to + // disagree, because `.then(onFulfil, onRejected)` does not route onFulfil's + // own throw to its sibling handler. + // + // Run in a real browser rather than only under linkedom because this is the + // shipped combination: an async render returning the refused `.action=${fn}` + // binding on a native form, where `action` is a reflected IDL attribute that + // a DOM shim does not model. The imported action is an RPC stub on the + // client, still a function, so the guard fires here exactly as it would in + // an app. + test('a throw from the COMMIT of an async render reaches renderError(), not the window', async () => { + const tag = uniq('async-commit-err'); + const origError = console.error; + console.error = () => {}; + const escaped = []; + const onRejection = (e) => { escaped.push(e); e.preventDefault(); }; + window.addEventListener('unhandledrejection', onRejection); + try { + const serverAction = async function saveTodo(input) { return { ok: true }; }; + class C extends WebComponent { + async render() { await tick(2); return html``; } + renderError(e) { return html`

${e.message}

`; } + } + C.register(tag); + const el = document.createElement(tag); + container().appendChild(el); + + // Bounded on purpose, and crossing the bound is a hard failure: an + // updateComplete that never settles is precisely the bug, so awaiting it + // unguarded would hang the run instead of reporting it. + const settled = await Promise.race([ + el.updateComplete.then(() => 'settled', () => 'settled'), + tick(1000).then(() => 'NEVER SETTLED'), + ]); + await tick(5); + + assert.equal(settled, 'settled', 'updateComplete must settle after a failed commit'); + assert.ok(el.querySelector('.err'), 'renderError committed'); + assert.match(el.querySelector('.err').textContent, /interpolated into action=/); + assert.equal(escaped.length, 0, 'the refusal must not reach window.unhandledrejection'); + assert.equal(el.__pendingAsyncCommits, 0, 'the in-flight count is released, not wedged'); + // The whole point of the guard: nothing carrying the source reaches the DOM. + assert.ok(!el.innerHTML.includes('saveTodo'), 'no function source in the live DOM'); + } finally { + window.removeEventListener('unhandledrejection', onRejection); + console.error = origError; + } + }); + test('race guard: the NEW render commits, a later-resolving STALE one is dropped', async () => { const tag = uniq('async-race'); const gates = []; From 347807140e9ffcf8010834a69559b17f6f31061f Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 19:22:03 +0530 Subject: [PATCH 31/38] fix: refuse a function .prop only where the property reflects The prop guard was gated on 'is not a custom element', which is not the same set as 'writes the value into the markup'. action reflects on
, formAction on `, host), + /function was interpolated into formaction=/, + ); +}); + +test('client .action=${fn} on a non-reflecting native element is left alone', () => { + const host = document.createElement('div'); + render(html`
hi
`, host); + const el = host.querySelector('div'); + assert.equal(typeof el.action, 'function', 'the expando is set, as it always was'); + assert.ok(!host.innerHTML.includes('CLIENT_SECRET'), 'and nothing reaches the markup'); +}); + test('a custom element keeps accepting a function on .action', () => { // Not a reflected IDL attribute, just an author-defined property, so passing // a function is legitimate and must not be refused. diff --git a/packages/core/test/rendering/form-action-attr-guard.test.js b/packages/core/test/rendering/form-action-attr-guard.test.js index 8a10a33d1..0c5a97008 100644 --- a/packages/core/test/rendering/form-action-attr-guard.test.js +++ b/packages/core/test/rendering/form-action-attr-guard.test.js @@ -377,6 +377,50 @@ test('.action=${fn} on a native form throws at SSR even though the prop would be ); }); +// The `.prop` guard fires on REFLECTION, not on "is a native element". Those +// are different sets, and gating on the second refused four shapes that never +// leaked. Both sides are pinned here, because the suite passed either way: +// nothing covered a `.prop` binding on a native element that is not a form. +// +// reflects (must throw) .action on , .formAction on `, { ssr: true }), + /function was interpolated into formaction=/, + ); + await assert.rejects( + () => renderToString(html``, { ssr: true }), + /function was interpolated into formaction=/, + ); +}); + +test('.action=${fn} on a native element that does NOT reflect it still renders', async () => { + // A plain expando: nothing is stringified and nothing reaches the markup, so + // refusing it would break a supported binding (the delegated-command shape + // ``, + html`
  • hi
    • `, + html`
    hi
    `, + ]) { + const out = await renderToString(tpl, { ssr: true }); + assert.doesNotMatch(out, /SECRET/, 'a dropped prop must not carry the source'); + assert.match(out, /hi/, 'and the element must still render'); + } +}); + +test('the streaming renderer draws the same reflection boundary', async () => { + await assert.rejects( + () => drain(renderToStream(html``, { ssr: false })), + /function was interpolated into formaction=/, + ); + const out = await drain(renderToStream(html`
    hi
    `, { ssr: false })); + assert.doesNotMatch(out, /SECRET/); + assert.match(out, /hi/); +}); + test('a custom element keeps accepting a function on an unclaimed prop', async () => { const out = await renderToString(html``, { ssr: true }); assert.doesNotMatch(out, /SECRET/, 'an unserializable prop is dropped, not serialized'); diff --git a/test/bun/form-action-guard.mjs b/test/bun/form-action-guard.mjs index f02482769..8e92dfbb3 100644 --- a/test/bun/form-action-guard.mjs +++ b/test/bun/form-action-guard.mjs @@ -87,6 +87,7 @@ const refused = { // leaked. camelCase is React's spelling, so it is the likeliest arrival. 'camelCase formAction=${fn}': () => html``, 'upper-case ACTION=${fn}': () => html`
    `, + 'reflecting prop .formAction=${fn} on a button': () => html``, }; for (const [name, mk] of Object.entries(refused)) { @@ -120,6 +121,10 @@ const allowed = { 'unquoted event @action=${fn}': () => html`
    `, 'custom-element event @action=${fn}': () => html``, 'custom-element prop .action=${fn}': () => html``, + // The `.prop` guard keys on REFLECTION, not on "is a native element", so + // these must stay legal on both runtimes: a plain expando writes no markup. + 'native prop .action=${fn} on a div': () => html`
    `, + 'native prop .action=${fn} on a button': () => html``, }; for (const [name, mk] of Object.entries(allowed)) { From 36771767229c14525e80bc8ce540792027c7aa2c Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 19:22:08 +0530 Subject: [PATCH 32/38] fix: settle the update cycle when the pending commit rejects The commit-throw fix guaranteed _commitAsync never rejects, which is not the same as handling a rejection where it is awaited. update() is a documented override point returning any thenable, so a rejecting one reproduced the original wedge verbatim: __pendingAsyncCommits stuck at 1, updateComplete never settling for the instance, and the escape for later non-committing cycles disabled with it. Handled at the awaiting site rather than only in the one implementation that happens not to reject. --- packages/core/src/component.js | 13 +++++++++- .../lifecycle/component-lifecycle.test.js | 26 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/packages/core/src/component.js b/packages/core/src/component.js index 4bcd54006..118f2e087 100644 --- a/packages/core/src/component.js +++ b/packages/core/src/component.js @@ -1176,7 +1176,7 @@ class WebComponentBase extends Base { // (shouldUpdate=false) running during the fetch does NOT resolve // updateComplete early: the pending commit owns the resolution. this.__pendingAsyncCommits = (this.__pendingAsyncCommits || 0) + 1; - pendingCommit.then(() => { + const settle = () => { // Always decrement first, even when superseded, so the in-flight // count never leaks (a superseded cycle returns below without // committing, but it is no longer pending). @@ -1189,6 +1189,17 @@ class WebComponentBase extends Base { } // --- 7-8 + updateComplete --- this._postCommit(changedProperties); + }; + // A rejection has to settle the cycle too, or the counter stays >= 1 + // forever: that both wedges `updateComplete` for this instance and + // disables the `!this.__pendingAsyncCommits` escape below for every + // later non-committing cycle. `_commitAsync` is written not to reject, + // but it is not the only thenable that reaches here: `update()` is a + // documented override point and may return one, so the handler belongs + // at this site rather than only at the one implementation of it. + pendingCommit.then(settle, (error) => { + this._handleRenderError(error instanceof Error ? error : new Error(String(error))); + settle(); }); return; } diff --git a/packages/core/test/lifecycle/component-lifecycle.test.js b/packages/core/test/lifecycle/component-lifecycle.test.js index 7657f54f1..e2ccce07d 100644 --- a/packages/core/test/lifecycle/component-lifecycle.test.js +++ b/packages/core/test/lifecycle/component-lifecycle.test.js @@ -352,6 +352,32 @@ test('a throw from the COMMIT of an async render() is contained like a sync one' assert.equal(el.__pendingAsyncCommits, 0, 'the in-flight count is released, not wedged'); }); +test('a REJECTED thenable from an update() override settles the cycle too', async () => { + // The commit-throw fix guarantees _commitAsync never rejects, which is not + // the same as handling a rejection at the site that awaits it. update() is a + // documented override point and may return any thenable, so a rejecting one + // reproduced the original wedge verbatim: counter stuck >= 1, updateComplete + // never settling, the error escaping as an unhandled rejection. + let errorArg = null; + class C extends WebComponent { + render() { return html`

    x

    `; } + renderError(e) { errorArg = e; return undefined; } + update() { return Promise.reject(new Error('prepare failed')); } + } + C.register('async-update-reject'); + const el = document.createElement('async-update-reject'); + document.body.appendChild(el); + + const settled = await Promise.race([ + Promise.resolve(el.updateComplete).then(() => 'settled', () => 'settled'), + new Promise((r) => setTimeout(() => r('NEVER SETTLED'), 500)), + ]); + assert.equal(settled, 'settled', 'updateComplete must settle after a rejected commit'); + assert.ok(errorArg instanceof Error, 'the rejection reaches renderError()'); + assert.match(errorArg.message, /prepare failed/); + assert.equal(el.__pendingAsyncCommits, 0, 'the in-flight count is released, not wedged'); +}); + /* -------------------- lazy controllers set: ensure graceful behavior -------------------- */ test('WebComponent without static properties still constructs cleanly', () => { From eed0e7ecc0f4e93e5868f1f2e7d9ea97ab08b540 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 29 Jul 2026 19:23:47 +0530 Subject: [PATCH 33/38] docs: state what the isolation swallows, and the comment hole it misses Three corrections, all where the docs promised more containment than exists. 'Renders the component empty' understates it twice. The isolation replaces the element through its closing tag, so a shell or header component takes the page's whole authored body with it: every route returns 200 with a blank body, which reads like a routing failure. And on a route with a loading.ts the page renders inside a Suspense boundary after the 200 is flushed, where a throw is swallowed with no log and no onError, so the server-log line the docs tell you to check does not exist. 'The refusal covers the shape' has one exception worth naming: a hole inside an HTML comment is emitted raw, so commenting the form out does not disable the interpolation and still ships the body silently. That is the likeliest next move for someone who just hit the error. The carve-out table also gains the two rows the narrowed prop guard makes true. --- .../webjs/references/muscle-memory-gotchas.md | 13 +++++++++++-- website/app/docs/troubleshooting/page.ts | 3 ++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/.agents/skills/webjs/references/muscle-memory-gotchas.md b/.agents/skills/webjs/references/muscle-memory-gotchas.md index c44a83ec0..76ca1e89c 100644 --- a/.agents/skills/webjs/references/muscle-memory-gotchas.md +++ b/.agents/skills/webjs/references/muscle-memory-gotchas.md @@ -63,14 +63,18 @@ Do not go looking for the rule that decides when it folds. Export status, read c So treat everything reachable from the action as exposed. That is the assumption the refusal is built on, it is the only one that holds across runtimes, and it is the only one that stays true when the transpiler changes. -The refusal covers the shape, not one spelling of it. Every hole form is refused (`action=${fn}`, `action="${fn}"`, the mixed `action="/x/${fn}"`), and so is a function wrapped in an array (`action=${[fn]}`), since an array stringifies each element through `String()` and leaks identically. +The refusal covers the shape, not one spelling of it. Every hole form is refused (`action=${fn}`, `action="${fn}"`, the mixed `action="/x/${fn}"`), and so is a function wrapped in an array (`action=${[fn]}`), since an array stringifies each element through `String()` and leaks identically. Casing does not help either: `formAction=` and `ACTION=` fold to the same rule. -Two carve-outs, both because those bindings never stringify their value: +**Commenting the form out does not disable the hole.** A comment is HTML, the interpolation is JavaScript, and the renderer emits a comment's holes raw, so `` still ships the whole action body with no throw and no log. Delete the binding instead of commenting around it. This is the one shape in this section that leaks silently, which is exactly why it is worth knowing. + +The refused and allowed shapes in full. Every "no" row is a binding that stringifies nothing, so refusing it would break working code rather than close a leak: | Written as | Refused? | Why | |---|---|---| | `action=` / `formaction=` | yes | ordinary attribute, stringified into the HTML | | `.action=` on a native form | yes | the property reflects, so the source lands in the DOM on the client | +| `.formAction=` on a button or input | yes | same reason, that is where `formAction` reflects | +| `.action=` on any other native tag | **no** | a plain expando (`
    `, `