Skip to content

fix: a function in any template hole stringifies its source, not just action= #1175

Description

@vivek7405

Problem

A function interpolated into ANY template hole is stringified through String(val), which returns its source text. #1167 closed that for action= / formaction= attributes, the shape a Next-trained author writes by accident. Every other hole position still emits the full body, including an imported 'use server' action's, since at SSR that import is the real function.

Measured at the #1167 head, one function whose body contains a connection string:

comment          <!-- <form action=${fn}> -->   leaks=true
text child       <div>${fn}</div>               leaks=true
script body      <script>${fn}</script>         leaks=true
unrelated attr   <div title=${fn}></div>        leaks=true

The comment case is the one worth calling out, because it is reached by a specific and likely accident: an author hits #1167's new refusal, comments the form out to get the page rendering again, and the source ships silently. An HTML comment is still served bytes, and the interpolation is JavaScript, so commenting it out disables nothing. There is no throw, no log, and no dev-overlay signal.

Two sibling paths are already tracked and are NOT this issue: #1169 (a reflect: true prop reflects the source into an attribute) and #1170 (a function in an on* inline-handler attribute). This one is the general case underneath all of them.

#1167 deliberately scoped itself to action= / formaction= and has a test pinning that boundary, which was the right call for a security fix that needed to land. That leaves the general question open, and it is currently answered only by a paragraph in .agents/skills/webjs/references/muscle-memory-gotchas.md telling authors not to comment the binding out.

Design / approach

The question to settle is whether a function is EVER meaningful output in a template hole. Interpolating one is almost certainly a mistake in every position:

  • text child: renders the source as visible page text
  • comment: ships the source in served bytes
  • attribute: ships the source in an attribute value
  • <script> / <style> raw text: ships the source, and note that WebJs already forbids interpolating into these (no-interpolation-in-raw-text-element), because the client drops the hole on hydrate

The legitimate function positions are the ones with a binding prefix: @event=${fn}, .prop=${fn} on a custom element, and a directive. Those do not go through String().

So the candidate rule is broad and simple: refuse a bare function value in any non-binding hole, replacing #1167's name-based predicate with a position-based one. That is a much smaller rule than the current one and it closes the whole class rather than a list of names.

Weigh against it:

An alternative worth pricing: keep rendering but emit nothing (drop the value, as the property path already does for unserializable values) plus a dev warning. That closes the disclosure with no new throw and no crash surface, at the cost of silence in production.

Implementation notes (for the implementing agent)

Where to edit:

  • packages/core/src/form-action.js: assertNotFunctionActionAttr() and isFormActionAttr() are the current name-based predicate. A position-based rule replaces the second, not the first.
  • packages/core/src/render-server.js: the buffered machine's commit sites, state === 'comment' at L245-249, state === 'rawtext' at L250, plus the attribute branches around L336-352. The streaming machine repeats all of these around L1905-1938. Both are independent state machines; a rule added to one does NOT apply to the other, which has been the recurring trap in this area.
  • packages/core/src/render-client.js: applyPart's attr / attr-mixed / prop / bool clauses (L673-722) and the child-commit paths.
  • packages/server/src/check.js if a static rule is wanted alongside the runtime one.

Landmines:

Invariants to respect:

  • AGENTS.md invariant 1: server-only code never reaches the browser.
  • AGENTS.md invariant 4: @ / . / ? holes must be unquoted, which is what makes a quoted one refusable.

Tests + docs surfaces:

  • Unit: both SSR machines and the client, one case per hole position, plus the full carve-out set.
  • Bun parity: extend test/bun/form-action-guard.mjs, which already carries refused / allowed tables.
  • Counterfactual per call site, the bar fix: refuse a function in form action=, it leaked server action source #1167 set: every guard site maps to a test that reds when that site alone is reverted.
  • Docs: .agents/skills/webjs/references/muscle-memory-gotchas.md (remove the "commenting it out still leaks" caveat once it no longer does), website/app/docs/troubleshooting/page.ts, website/app/docs/components/page.ts (the binding-rules carve-out note), and AGENTS.md if the rule becomes an invariant.

Acceptance criteria

  • A decision is recorded on refuse-vs-drop, and on whether it applies in prod or dev only
  • A function in a comment hole no longer ships the action's source
  • The same holds for a text child and an unrelated attribute, on both SSR machines and the client
  • @event=${fn}, a custom element's .prop=${fn}, and every directive still work
  • A self-referential array still renders rather than overflowing the stack
  • A counterfactual proves each call site's test reds when that site alone is reverted
  • Bun parity green
  • The doc caveats added by fix: refuse a function in form action=, it leaked server action source #1167 are removed once they are no longer true

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