Skip to content

SSR stringifies a function-valued form action, leaking server source #1154

Description

@vivek7405

Problem

html`<form action=${importedAction}>`silently leaks server source into the SSR'd HTML. At SSR a'use server'import is the REAL function (the RPC stub only exists in the browser), andaction=is an ordinary escaped attribute hole, so the rendererString()s the function into the attribute. Verified against render-server.js` on 2026-07-28:

<form action="async function createTodo(input) { const DB_SECRET = 'postgres://user:pw@host'; return { success: true }; }" method="post">

This is exactly what a Next.js-trained agent writes on day one (<form action={serverAction}> is the canonical Next form), so it is a footgun with a security payload: whatever the action closure shows (connection strings, internal paths, query shapes) ships to every visitor. Quoted (action="${fn}") and unquoted (action=${fn}) holes both leak identically (verified).

Design / approach

Make a function-valued action/formaction attribute hole a hard error at render time, never a stringify. This is the standalone security floor; the companion issue (unify form submissions on <form action=${fn}>, filed alongside, same PR) then turns the same hole shape into the real feature. The error message should point at that feature once it exists ("a form action must be a 'use server' action; got a plain function" or similar), and in the interim just refuse loudly.

Scope the claim precisely:

  • <form action=${fn}> and <form action="${fn}"> with a function value: throw (both commit through the same sites, verified byte-identical today).
  • <button formaction=${fn}> / <input formaction=${fn}>: same treatment, same leak class.
  • A function inside a MIXED hole (action="/x/${fn}"): throw too; there is no sane serialization.
  • A string/URL value: byte-identical passthrough to today (regression-gate this).
  • Any OTHER attribute receiving a function still stringifies as today; do not widen the claim beyond action/formaction in this issue (the generic case is arguably also a bug, but changing it here risks collateral behaviour change).

Implementation notes (for the implementing agent)

  • Where to edit (server renderer): packages/core/src/render-server.js, the plain-attribute commit in the after-eq state: L326 (quoted, out += `"${escapeAttr(String(val ?? ''))}"`) and L331 (unquoted). The current tag name is available as currentTagand the attribute name asattrNamein that state machine; branch ontypeof val === 'function'+ attr nameaction/formactionbefore theString()`.
  • Where to edit (client renderer): packages/core/src/render-client.js L674 (part.el.setAttribute(part.name, String(value))) is the client-side commit for attribute parts; a client re-render of the same template would set the leaked source as the live DOM attribute. Same guard there (the part knows its element and name).
  • Landmines:
    • The SSR renderer is a single-pass string state machine, not a DOM builder; the attribute name at L326/331 carries no binding prefix (plain attribute), so you are in the kind === undefined fall-through, not the event/prop/bool branches above it.
    • escape.js escapeAttr does not escape ' (documented at render-server.js:568); irrelevant once you throw, but do not "fix" it in passing.
    • Do NOT throw on webjs-suspense's .fallback or other prop bindings; those route through earlier branches and never reach L326.
    • Error surfacing: a page-render throw goes through the existing error-boundary path and (dev) the overlay via ssrOpts.onDevError; throwing a plain Error with a clear message is enough, no new plumbing.
  • Invariants: AGENTS.md invariant 4 (unquoted holes for @/./?) is untouched; action= is a plain attribute. The differential-elision invariant is untouched (no elision change).
  • Tests:
    • Unit (core): packages/core/test/rendering/, new test: function-valued action/formaction (quoted, unquoted, mixed) throws; string action="/x" renders byte-identically (counterfactual: the throw test fails when the guard is reverted).
    • Client side: a browser test asserting the client renderer refuses the same commit (test/**/browser/ layer, run via npm run test:browser).
    • Bun parity: the renderer is runtime-neutral core; no listener/serializer surface touched, so no new test/bun/* file is required for THIS issue (the unification issue does require one).
  • Docs surfaces: .agents/skills/webjs/references/muscle-memory-gotchas.md (the Next-habit table: <form action={fn}> entry) + references/data-and-actions.md. Root AGENTS.md only if the unification issue does not land in the same PR (it will; let that issue own the doc rewrite).

Acceptance criteria

  • renderToString(html`<form action=${fn}>`) throws with a message naming the fix; no function source ever reaches the emitted HTML
  • Same for quoted and mixed holes, and for formaction on button/input
  • String-valued action output is byte-identical to before (regression test)
  • Client renderer refuses the same commit (browser test)
  • Counterfactual: reverting the guard makes the leak test fail
  • Gotchas reference updated with the Next-habit entry

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

  • Status
    Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions