Repository navigation
Commit a39389e
fix(service-messaging)!: an html or mjml notification template escapes every hole for markup, and each outlet package pins its outlet list (ruling 3C) (#25076)
Fixes #25061
Clause-②: no (narrowing)
Carries out ruling **3C** on #25040 (`6308462370`): no server-side
stripping or refusal of markup in text fields; every outlet that renders
a text field into HTML escapes it by context, the mail template engine
by default, with a pin on the outlet list. Disclosure: the #9387 ruling
(`5323310017`) applies to this PR. Fixtures are neutral markup (a bold
or italic element, an ampersand, quotes), and nothing here describes a
request.
## What changed
- **`@objectstack/service-messaging` (the one outlet that did not
escape).** `renderNotification` rendered a `sys_notification_template`
body of format `html` or `mjml` by placing each `{{ payload.x }}` value
into the markup as stored. The email channel sends that string as the
mail's html part, so a payload text field carrying markup characters (a
display name, a record title, a comment preview) reached the recipient's
mail client as markup. Each hole's value in an html or mjml body is now
HTML-escaped, in element text and quoted attributes alike; the
template's own markup renders as written. The subject and a `markdown`
or `text` body are plain text and stay verbatim. No unescaped hole form
is added (none existed, and nothing in the tree needs one); a template
that needs a pre-rendered fragment uses an email template, whose
renderer has one.
- **Pins, in three NEW files named by the rule**
(`src/text-field-html-outlets.pin.test.ts` in each package that holds an
outlet): one pin on the outlet list itself, and per-entry pins that
drive the real render path.
- **Changeset:**
`.changeset/25061-notification-template-html-escape.md`,
`@objectstack/service-messaging` minor, BREAKING (patch round 1; it was
a patch before the seat ruled question 1 B).
Nothing was edited in `packages/spec`, `packages/objectql` or
`packages/platform-objects`.
## Outlet census
The census is the AST walk each pin file carries: a shipped source file
"touches HTML" when an identifier or a string in its CODE names `html`,
or a string carries an HTML element tag (comments never count). I ran
the same walk over every `domain:services` package
(`packages/services/*/src`, `packages/connectors/*/src`,
`packages/triggers/*/src`, `packages/plugins/*/src`) at `c10cc2a71b`,
seeded by `git grep` for escaper helpers, `text/html`, `body_html` and
HTML tags next to interpolation. Every hit is classified below.
| Package | File | Role | Escapes by context? |
|:--|:--|:--|:--|
| plugin-email | `template-engine.ts` | outlet: the engine.
`renderTemplate` is the HTML face, `renderPlainTextTemplate` the
plain-text faces | yes, by default, unchanged: a double-braced hole is
HTML-escaped in `body_html` and verbatim in the subject and `body_text`;
the triple-braced hole is the one unescaped form |
| plugin-email | `email-service.ts` | outlet:
`resolveAndRenderTemplate`, behind `sendTemplate` and `renderTemplate` |
yes: `body_html` goes through the HTML face |
| plugin-email | `templates/auth-templates.ts` | outlet: the built-in
auth templates | yes: every text variable is double-braced; see the
raw-form reading below |
| plugin-email | `index.ts`, `template-loader.ts`, `transports/smtp.ts`,
`transports/resend.ts`, `transports/postmark.ts` | not an outlet:
re-export, column mapping, and handing the rendered html part to the
provider | n/a |
| service-messaging | `template-renderer.ts` | outlet:
`renderNotification` for an html or mjml body | **was no; fixed here** |
| service-messaging | `email-channel.ts` | not an outlet: forwards the
rendered html; its template arm delegates to plugin-email's engine | n/a
|
| service-messaging | `objects/notification-template.object.ts` | not an
outlet: declares the `format` vocabulary | n/a |
| plugin-approvals | `action-link-pages.ts` | outlet: the session-less
confirm and result pages an approval link opens | yes, unchanged: every
request text field goes through its escaper |
| plugin-approvals | `approval-service.ts`, `approvals-plugin.ts` | not
an outlet: wrap or serve that rendered page as `text/html` | n/a |
| service-mailbox | `ingest/mail-ingest-service.ts`,
`ingest/parse-message.ts`, `objects/sys-mail-message.object.ts` | not an
outlet: stores the html part of INBOUND mail; renders nothing | n/a |
- **HTML exports:** none on the server. Record exports are csv, xlsx and
json (`packages/rest/src/export-format.ts`). The print service renders
the console's own record page in a headless browser, so the UI renders
the value there; the server puts no field value into markup.
- **Integration payloads:** no builder emits HTML. Webhooks serialize
JSON. `connector-slack` posts caller-authored text verbatim, in Slack's
markup, not HTML. `service-sms` sends plain text. The messaging digest
renders plain text.
- **Other lanes (named for the seat, not touched):**
`packages/cli/src/commands/serve.ts` (host-not-found page) escapes its
one value. `packages/plugins/plugin-hono-server/src/adapter.ts` carries
an already-rendered page. One further out-of-lane HTML builder, outside
ruling 3C's text-field scope, is named to the seat privately. The UI
renderer lives in objectui.
## The engine's raw form, measured
- **What exists.** `plugin-email`'s engine has exactly one unescaped
form, the triple-braced hole. `service-messaging`'s renderer had none
(every hole was unescaped), and it still has none.
- **Who uses it.** The built-in auth templates have 20 sites: password
reset, verify email, magic link, invitation accept and
account-invitation set-password, times four locales. Each one names a
`type: 'url'` variable inside an `href` attribute, for a link the
platform minted. The only other uses are plugin-email's own tests and
the authoring doc (`content/docs/automation/email-templates.mdx`).
`examples/app-showcase`'s authored template uses double braces only.
- **Kept, and pinned.** The built-ins keep that raw form. The new pin
holds them to it: in every built-in body, each unescaped hole must name
a declared url variable and sit inside an `href` attribute. Every text
variable must render escaped through the real `EmailService`.
## `sys_user.name` maxLength: reading
The declaration, at base `4bf079c693`, is `Field.text`, `required`,
`maxLength: 255`
(`packages/platform-objects/src/identity/sys-user.object.ts:776-781`). I
measured it on a real ObjectQL with SqlDriver (in-memory sqlite) under
the system context, which is the context the auth adapter writes
`sys_user` with (`asAttributedSystem` in plugin-auth's
`objectql-adapter.ts`). I used a scratch test that is not committed.
| write | answer |
|:--|:--|
| insert, name of 255 characters (control) | accepted, 255 stored |
| insert, name of 256 characters | refused `VALIDATION_FAILED`, field
error `max_length`, constraint maxLength 255 / actual 256 |
| update an existing row's name to 256 characters | refused
`VALIDATION_FAILED`; the stored value is unchanged |
**Reading:** enforced at the write seam, by the objectql write
validator's bounded-string branch, which is #11875's rule (`text` is in
`BOUNDED_STRING_FIELD_TYPES`). No edit is owed in `platform-objects` or
`objectql`.
## Pins and ablation
There is one leg per pinned behaviour. Each mutation went through
`scripts/ablation-replace.mjs` in wrap mode (the anchor must hit once;
landing is proven by on-disk counts and a blob change). Each restore is
`git checkout HEAD -- PATH`, proven by restored blob equal to the HEAD
blob and an empty `git diff HEAD`. No mutation needs a rebuild: every
pin imports its subject from `src`.
| leg | mutation (on disk) | reading on the pin file |
|:--|:--|:--|
| A | service-messaging html face encodes verbatim | red, 3 failed of 8:
html, mjml, email-channel path. The subject and plain-text pins stay
green, as they should |
| B | an unlisted service-messaging file gains an html-named export |
red, 1 failed of 8: the outlet-list pin |
| C | plugin-email `renderTemplate` encodes verbatim | red, 32 failed of
66: engine, email-service and every built-in escape pin |
| D | the en-US default footer's app-name hole becomes triple-braced |
red, 10 failed of 66: raw-hole placement and escape, for the five en-US
templates on that footer |
| E | an unlisted plugin-email file gains an html-named export | red, 1
failed of 66: the outlet-list pin |
| F | approval summary rows stop escaping | red, 2 failed of 5: confirm
and result pages |
| G | an unlisted plugin-approvals file gains an html-named export |
red, 1 failed of 5: the outlet-list pin |
Legs B, E and G first ran as no-ops: each replacement contained its own
anchor, so `ablation-replace` refused the leg as not landed and restored
before any test ran. They were re-run with replacements that do not
contain the anchor (B2, E2, G2), and the readings above are from those
re-runs. All legs ran at `c10cc2a71b`, and every restore was proven.
## Tests and gates (final head `1960b459bb`)
Every run went under `scripts/pm/os-verify-lock.sh`, after the closure
build (`pnpm --filter 'PKG^...' --filter PKG build` for the three
packages; exit 0).
| command | head | result |
|:--|:--|:--|
| `pnpm --filter @objectstack/service-messaging test` | `1960b459bb` |
exit 0, Test Files 55/55, Tests 610/610 |
| `pnpm --filter @objectstack/service-messaging typecheck` |
`1960b459bb` | exit 0; the tsc program lists the new pin file
(`--listFiles` count 1) |
| `pnpm --filter @objectstack/plugin-email test` | `73f7bf8fe5` | exit
0, Test Files 36/36, Tests 643/643 |
| `pnpm --filter @objectstack/plugin-email typecheck` | `73f7bf8fe5` |
exit 0; `check:test-typecheck: OK`, with 0 errors held and the new file
in the test program (count 1) |
| `pnpm --filter @objectstack/plugin-approvals test` | `73f7bf8fe5` |
exit 0, Test Files 94/94, Tests 1213/1213 |
| `pnpm --filter @objectstack/plugin-approvals typecheck` | `73f7bf8fe5`
| exit 0; `check:test-typecheck: OK` (ledger unchanged, the new file is
in the test program, count 1) |
The only commit after `73f7bf8fe5` is `1960b459bb`, which edits the
service-messaging pin file alone. Its package re-ran on that head, and
so did the ablation legs that read the file (A and B, again 3/8 and 1/8
red).
**Derived gate families.** `node scripts/pm/dispatch-gates.mjs
--commands --repo objectstack-ai/objectstack` derived 68 commands. All
68 ran, each with its exit captured before any pipe, and `--ran`
reconciles **68 derived, 68 run, 0 NOT-MEASURED, 0 UNRUN** (a derived
zero: every line records an exit code).
- On the first pass at `73f7bf8fe5`, 65 exited 0.
- `check:engine-double-contract` went red there, because the first draft
of the channel-path pin added a new engine double. The pin now hands the
channel a stored-row store instead, and the gate is exit 0 at
`1960b459bb`.
- `check:i18n` and `check:dual-build-cjs-loads` exited 3 (prerequisite
not met: no full build). After a full build, both are exit 0 at
`1960b459bb`.
- The ratchet families were re-run at `1960b459bb`, all exit 0:
`check:engine-double-contract`, `check:cross-package-test-inputs`,
`check:test-source-alias`, `check:type-check-coverage`,
`check:type-check-debt`, `check:objectql-double-limit` and
`check:nul-bytes`.
The repo-wide scans and the CI-only lanes (Test Core shards, Build Core,
the type-check lanes, `test:repo`) are CI's. I did not re-run them
locally.
**Dogfood battery:** ~~n/a. The title has no `!` and the claim has no
`(narrowing)` arm (see open question 1).~~ Superseded by patch round 1:
the claim moved to `(narrowing)`, and the full battery ran on head
`7aeddce6f3` (see below).
## Acceptance notes
- The list pin's radius is per package. Each package that holds an
outlet today carries its own census, and the census reads only that
package's source, so no cross-package input declaration is needed. A new
outlet in a package with no list is not seen. The alternatives are in
the open questions.
- `service-messaging`'s html face has no unescaped hole by design. The
migration line in the changeset names the email-template route for a
pre-rendered fragment.
- These new pin files each scan their own package with the `typescript`
parser, which is already a devDependency of all three. No gate, script
or `turbo.json` entry was added.
## Open questions for the seat
1. **Clause-②: does the html-face escaping count as a narrowing?** It
changes what an existing html or mjml `sys_notification_template` row
renders when a hole's value carries an ampersand, an angle bracket or a
quote. That includes a row that relied on a payload value rendering as
markup. Measured reach in this tree at `73f7bf8fe5`:
- authored html or mjml notification-template rows: 0 (no seed, no
example, 0 rows in the dogfood upgrade fixtures `fixture-18.0` and
`fixture-18.0.0`, and no `format: 'html'` or `'mjml'` outside
`packages/spec`);
- producers that put pre-rendered HTML into a payload: 0 of the 3
messaging producers (approvals `notify`, the audit mention writer, the
automation notify node);
- rows stored in deployments: not measurable from here.
The PR is declared `Clause-②: no` as claimed. If the seat amends the
claim to `no (narrowing)`, this PR owes a `!` title, a BREAKING
changeset with its ADR-0087 disposition, and the full dogfood battery on
the final head.
2. **The list pin's radius.** Options:
- (A) per package, as landed here;
- (B) one lane-wide census test, which needs a cross-package input
declaration in `scripts/cross-package-test-inputs.mjs` and `turbo.json`,
outside this claim's file surface;
- (C) a repo-level census in another lane's `repo` vitest project.
3. **One out-of-lane HTML builder** (outside ruling 3C's text-field
scope) is named to the seat privately, for the seat to declare
cross-lane. Its reach is not measured.
## Patch round 1
- Patch round 1: the changeset now ships as a BREAKING minor
(`fix(service-messaging)!`, `Clause-②: no (narrowing)`, ADR-0087
`not-required (no-migration-prescription)`), head `7aeddce6f3`.
- Full dogfood battery on head `7aeddce6f3`: VERDICT command-exit 0,
Test Files 286 passed / 3 skipped (289), Tests 2387 passed / 18 skipped
(2405), 0 failed.
---
_Generated by [Claude
Code](https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent f46ef4d commit a39389e
5 files changed
Lines changed: 839 additions & 2 deletions
File tree
- .changeset
- packages
- plugins
- plugin-approvals/src
- plugin-email/src
- services/service-messaging/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
0 commit comments