A reflect:true prop stringifies a function, leaking its source

[vivek7405]

Problem

A reactive property declared reflect: true writes a function value's SOURCE into its HTML attribute during SSR. If that function is an imported 'use server' action, its whole body, closure secrets included, ships to every visitor.

Reproduced against packages/core/src on the #1154 branch:

class R extends WebComponent({ action: prop(String, { reflect: true }) }) {
  constructor() { super(); this.action = someServerAction; }
  render() { return html`<span>x</span>`; }
}
R.register('reflect-probe');

<reflect-probe action="async function leaky(){ const c='REFLECT_SECRET'; return c; }" data-wj-host>…

It is NOT specific to action. The same happens for title, label, or any other reflected property name, verified for all three. This is the generic reflection path stringifying whatever it is handed, so it is a sibling of the form-action leak rather than a case of it.

Design / approach

A function is never a meaningful HTML attribute value. Under every other type branch in _reflectAttribute the value has a sensible serialization; only the fall-through String(value) produces something that is both useless and dangerous.

Two options, and the second is probably right:

1. Refuse, throwing the way fix: refuse a function in form action=, it leaked server action source #1167 does for action=. Loud, consistent with the form-action guard, but reflection runs inside the property setter, so a throw fires from an assignment rather than from a render, and it would surface in a much less obvious place.
2. Skip, treating a function like null and removing the attribute, with a dev-mode warning. This matches what the property-binding path already does for an unserializable value (it drops with [webjs] property binding .x has an unserializable value during SSR. Dropping.), so it keeps the two paths consistent, and there is no legitimate behaviour to preserve.

Prefer 2 unless there is a reason to be louder. Whichever is chosen, the dev warning should say what was dropped and why, because a silently missing attribute is its own confusion.

Implementation notes (for the implementing agent)

Where to edit. packages/core/src/component.js, _reflectAttribute() at L657-682. The leak is the final else at L677:

} else {
  this.setAttribute(attrName, String(value));   // <-- a function stringifies to its source
}

The branch order matters: converter.toAttribute (L665), Boolean (L669), value == null (L672), Object/Array via JSON.stringify (L675, which drops a function to undefined and is therefore already safe), then this fall-through. A String-typed or untyped prop lands here.

_reflectAttribute is called from the property setter at L622-623 (guarded by d.reflect && !d.state && this._connected) and from _reflectDeclaredAttributes() via _activate() at L777-783, which is the SSR path: the server activates the instance, declared reflect props are written then, and appendReflectedAttrs in packages/core/src/render-server.js (L1502-1515) copies the resulting attributes onto the emitted opening tag. So the fix in _reflectAttribute covers both the SSR emission and the client-side setter.

Landmines.

• __reflectingAttribute (L662-663, L680) is a re-entrancy guard: setAttribute triggers attributeChangedCallback, which calls the setter again. Any early return added must not leave that flag set. Returning BEFORE the flag is set is cleanest.
• Do not "fix" this in appendReflectedAttrs. By the time that runs the value is already a string in the DOM, so the source has been written; the only signal left is a heuristic on the string, which is exactly the kind of content-sniffing fix: refuse a function in form action=, it leaked server action source #1167 deliberately avoided.
• A custom converter.toAttribute (L665) runs first and is author-controlled. It is out of scope here: an author who writes one has taken responsibility for the serialization.
• Bun parity: the reflect path is plain DOM API use, but SSR runs it against the server DOM shim, so confirm the behaviour matches on both runtimes and add an assertion if it touches anything runtime-sensitive.

Negative result worth keeping, do not re-derive it. This does NOT chain with #1167's custom-element carve-out. #1167 deliberately allows <my-el .action=${fn}>, because a custom element's .action is an author-defined property that never reflects. Passing a function that way is dropped by the serializer before it reaches the instance property ([webjs] property binding .action has an unserializable value during SSR. Dropping. Detail: Cannot serialize a function.), verified, so the carve-out cannot be used to reach this leak. The author has to assign the function to the reflected prop inside their own component code.

Invariants. AGENTS.md invariant 1 (server-only code stays behind .server.{js,ts}) is what this violates in spirit: the whole point of the boundary is that an action's body never reaches the browser. Invariant 11 applies to any new prose.

Tests + docs surfaces.

• Unit: packages/core/test/ at the component/reflection layer. Assert a function assigned to a reflect: true prop does not put its source in the SSR output, for a String-typed prop and an untyped one, and that a normal string value still reflects byte-identically (the counterfactual: the leak test fails when the guard is reverted).
• Browser: reflection is observable in a real DOM, so a case under packages/core/test/**/browser/ is worth having, the same reasoning as packages/core/test/rendering/browser/form-action-guard.test.js (linkedom does not reflect IDL attributes, so the node layer cannot see part of this).
• Bun: add to test/bun/ only if the reflect path proves runtime-sensitive.
• Docs: if the resolution is "skip with a warning", .agents/skills/webjs/references/components.md (the reactive-properties / reflect section) should say a function is dropped, and website/app/docs/troubleshooting/page.ts should carry the symptom if a warning is emitted.

Acceptance criteria

• A function assigned to a reflect: true property never puts its source in the SSR output, for any property name
• The same holds for a client-side assignment, verified in a real browser where reflection actually happens
• A normal string value still reflects byte-identically to today
• A converter.toAttribute author override is unaffected
• Counterfactual: the leak test fails when the guard is reverted
• Docs updated if the behaviour is a silent drop or emits a warning