Repository navigation
fix(service-messaging)!: refuse a notification template html or mjml body that places a hole outside element text and quoted attribute values - #25258
Conversation
…d save hooks Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ApJv4hDHQ6M1JprNZFKzAW
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ApJv4hDHQ6M1JprNZFKzAW
…leads with the refused holes Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ApJv4hDHQ6M1JprNZFKzAW
…lacement judge Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ApJv4hDHQ6M1JprNZFKzAW
…cement save hooks Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ApJv4hDHQ6M1JprNZFKzAW
📓 Docs Drift CheckThis PR changes 1 package(s): 28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 7 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 686fc3443036576632edaea66c5208655f5f2eec && git checkout 686fc3443036576632edaea66c5208655f5f2eec
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0b8bce40e081477132d81b1e11a19cd3cdb331c0 4ca18be6c30c39bf186d02c68236d79b3af1af4a && git checkout -B drift-repro 0b8bce40e081477132d81b1e11a19cd3cdb331c0 && git merge --no-ff 4ca18be6c30c39bf186d02c68236d79b3af1af4a
node scripts/docs-audit/affected-docs.mjs --json 0b8bce40e081477132d81b1e11a19cd3cdb331c0
|
…arries no tracker id Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ApJv4hDHQ6M1JprNZFKzAW
Contract reviewServed-tier: Face: narrowing, declared ① Derived judgmentsA. The accept set of B. Fail-closed under every parser reading the judge claims to fork — right. I walked the scan against the HTML tokenizer and tree-builder rules rather than taking the module note on trust:
C. Is anything the judge ADMITS unsafe? No, under the ruling's criterion (encoding by context). The admitted set is element text (including the two escapable-text elements, where the encoder's reference-encoding is the right encoding) and a quoted value on an attribute that is not read as a URL, a URL list, a handler, a style sheet, markup, or a meta element setting. I checked the URL table against the current and legacy URL-bearing attributes a mail client acts on (href, src, srcset, imagesrcset, action, formaction, cite, data, poster, background, longdesc, usemap, codebase, classid, archive, code, profile, manifest, icon, ping, lowsrc, dynsrc, datasrc, itemtype, itemid, xmlns, every namespaced attribute, and the plugin parameter value); the event-handler prefix and the style attribute are matched after lower-casing; every attribute of a meta element is refused as a class of its own. What remains admitted (class, id, title, alt, width, data-*, target, rel, type, and the like) is text the encoder makes inert in a quoted value. One class the ruling's words do not reach is left admitted: a quoted attribute whose value selects a document setting on a non-meta element (an iframe's sandbox or allow list, a base target). The encoder is correct for it — a value cannot leave the quotes — and a value there changes a setting rather than becoming markup, so it is a semantic choice of the author, not an encoding gap; mail clients do not honour those elements. Named here so it is not read as missed. D. Judged over the merged record on every engine write path — right, verified in the engine. E. The envelope — right, no new error code. F. Hooks bind and unbind — right. G. Public surface — unchanged (right). H. Tests — the claims are pinned, not asserted. 74 unit cases cover both supported placements, every refused class (and the label table's completeness against the closed type), element closing, the comment and declaration edge states, reconsumed characters, the foreign-content, noscript and select / frameset readings, the renderer's grammar and span parity; 20 integration cases cover both doors, import, bulk, predicate, stored-row freeze and repair, read / delete / render, and the plugin's bind / unbind. Test bodies carry holes and markup only, never a value. ② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS |
席位核对 · ACCEPT · 2026-10-22T17:58Z
⛔ Disclosure (#9387): abstract. The PR body, the report
Generated by Claude Code |
Requested by Sales · project thread
Fixes #25082
The notification half of ruling
6335348115(items 2 to 4). The email half is #25189, which is not addressed here.Clause-②: no (narrowing)
Metadata-protocol: none
What changes
renderNotificationHTML-escapes every hole's value in anhtmlormjmlbody (the five characters an HTML parser reads as markup in element text and in a quoted attribute value). That encoding is right in exactly two placements: element text, and a quoted plain-text attribute value. Asys_notification_templatesave whosehtmlormjmlbody places a hole anywhere else is now refused.template-hole-placement.ts) is this package's own, beside the renderer's hole grammar (ruling item 2). It reads the holes through the renderer's own token (templateHoles, new intemplate-renderer.ts), so it judges exactly the spans the renderer substitutes. Its placement table is re-derived from the renderer's encoder, not copied from the email half. It uses the same class names and author-facing labels as the email half, plus one class of its own,meta-attribute(every attribute of a meta element, read as a setting of the document).noscriptstart tag, and at every raw-text or escapable-text start tag once aselectorframesetis open. Foreign content (SVG, MathML) is never treated as left, so every hole after its root is refused. Each of these rules only refuses more than a parser would.notification-template-placement.ts) arebeforeInsert/beforeUpdateonsys_notification_templateat priority 30, bound byMessagingServicePlugin.initon the kernel's ObjectQL engine and unbound indestroy(). They are hooks rather than a validation rule because a CEL condition has no reading of HTML placement; plugin-security's grant-name refusal is the precedent shape.400 VALIDATION_FAILEDwith one finding atbody,code: 'invalid_value', naming each refused hole and its placement (up to five, then a count) and the fix. An import commit fails that row and a bulk update fails that record. No new error code.The plugin logs a warning when the engine it is given has no hook registry, so an embedder without one is told the check is not running.
Tests
src/template-hole-placement.test.ts(74): both supported placements; every refused class; the label table's completeness; elements closing and judging afresh; the script comment escape; abrupt comments, declarations and reconsumed characters; the readings (foreign content never left, a self-closed root opening none, thenoscriptstraddle, theselect/framesetfork, a plain style element not forked); the renderer's grammar and spans; parity withrenderNotification's substitution;isHtmlFormatmatching the html face.src/objects/notification-template-placement.integration.test.ts(20), on a real ObjectQL over SqlDriver throughObjectStackProtocolImplementation,mapDataErrorandrunImport: html and mjml creates refused at the REST data door with nothing stored; the message naming each hole and counting past five; the positive twins; text, markdown and an absent format not judged; a by-id move to html refused; an import commit's failed row; a stored violating row refused on an edit of another column, on a bulk update's row while the other row lands, and on a predicate update that then writes no row; an edit moving the hole or the format accepted; the row still read, deleted and rendered as before; and the plugin binding the hooks on a booted kernel and unbinding them ondestroy().text-field-html-outlets.pin.test.ts: the two new files are classified as non-outlets (they render nothing).pnpm --filter @objectstack/service-messaging test: 58 files, 722 tests pass.typecheckpasses. ESLint over the changed files is clean.Acceptance notes
claude/project-thread-iydt23; it names no issue, so the Duplicate Fix Guard's branch-name hint will not match. The claim comment on mail templates: a hole an author places in a template body may not be encoded for the context it lands in (reach not measured; detail withheld under the disclosure ruling) #25082 is the identity record.index.tsis unchanged.patch(Metadata-protocolnone) with the BREAKING banner and an ADR-0087not-required (no-migration-prescription)disposition;check:adr-0087-registrationreads it.renderTemplate/renderNotificationexports) follows this card and is not addressed here.Generated by Claude Code