The only string in our alert email that nobody on our side wrote, and the four characters standing in front of it
dev.to Security0 views
Notifio watches a rental search page and tells you when something new appears on it. The thing it tells you is the listing's own title, which means there is exactly one string in the whole product that nobody on our side wrote, and we put it in an email.
That string takes a long route. It comes off a page in a browser running on the user's machine, gets written to a file in their app data directory, is posted as JSON to an API route that authenticates with their licence, is interpolated into an HTML document on a server, handed to Resend, and delivered to the user's own inbox. Six hops, four of them places where it is rendered.
Three of those four escape it for free. The interesting one does not.
Four destinations, one of which is hand-built HTML
The on-disk history. A ring buffer of the last 200 finds, written with JSON.stringify, which has no HTML semantics and cannot be confused into any:
/** * How many finds to keep. A busy hunt produces a few hundred a week; this is * enough to cover "what did I miss overnight" without the file growing forever. */constMAX_FINDS=200;
The app window. React, rendering the title as a text child, so escaping is a property of JSX rather than a thing anybody had to remember:
The desktop notification. Electron's Notification takes title and body as plain strings and hands them to the OS notification centre, which has no markup to speak of:
The email. Which is a template literal of hand-written table HTML, because that is what email is, and is therefore the one place where a stranger's string meets string concatenation:
Four characters. The ampersand first, which is the one ordering rule that matters: do it last and < becomes < and then &lt;, and your reader sees the markup you were trying to hide.
The notable absence is the apostrophe. ' is not escaped, which is fine here and is fine for a reason that has nothing to do with this function: every attribute in every one of these templates is double-quoted. A title containing 't throw a 500 mid-request. if (!alerts.every((a)=>a&&typeofa.siteName==="string"&&Array.isArray(a.newListings))){returnNextResponse.json({error:"Each alert requires siteName and newListings[]."},{status:400});}
siteName is a string and newListings is an array. Nothing checks what is inside the array. A listing of { title: null } or { url: { nested: true } } passes, and escHtml absorbs it, because the first thing it does is String(str) on a parameter already typed as string.
So the comment is true and it is true by accident. The route does not throw a 500 on a malformed listing, not because the validation covers it, but because a defensive String() call in the escaper eight files away catches what the validator missed. What the user gets is an email with the word "null" where a room should be. That is the better of the two failures, and nobody chose it.
The reply branch of the same route does check two levels down, which shows the author knew the difference:
listingTitle is narrowed to a string or an empty string, and confirmed fails closed on anything that is not literally true. The listings branch is the one that handles the untrusted field, and it is the one that does less.
Nothing asserts the URL is a URL
The other half is the href. escHtml makes a URL safe to put inside a double-quoted attribute. It does not make it a URL. Nothing between the page it was read from and the <a href> in your inbox checks that the string starts with http.
In practice mail clients are the backstop, and they are a good one: a non-http scheme in an email link is refused by every client worth naming. I am not comfortable relying on that, because it is somebody else's decision protecting us, and because the same string is also an href in the app's own window, where React will render whatever scheme it is handed.
The fix is one check in one place, and the reason it has to be written carefully is a product constraint rather than a security one. These alerts are the entire value of the app: a listing is gone in minutes, and an email that arrives late is worth nothing. So the hardening cannot be "reject the batch". If one listing in a batch of five has a URL that does not parse as http or https, the right behaviour is to drop that listing, send the other four, and log the one that was dropped. A 400 on the whole request because one title was malformed would turn a hardening change into a missed room, which is the one failure the product does not get to have.
That principle already shows up elsewhere in the app, which is why I think it is the right read. The reply notification cannot fail the reply:
/** * Never throws: the reply itself has already happened by the time this runs, so * a failed email must not turn a successful send into a logged failure. */
And the desktop notification cannot fail a poll:
/** * Electron is required lazily. The monitor can be run outside Electron for * development, and a missing notification is never a reason to fail a poll. */
Same rule, three places: the thing that tells you about a find is never allowed to be the thing that loses it.
What I would change
Three things, in order of how much they matter.
Validate each listing at the route: title coerced to a string, url parsed with new URL() and required to be http: or https:, bad entries dropped rather than rejected, and the drop counted in the log so a site that starts producing junk is visible rather than quietly thinner. This is the one with a real consequence.
Make escHtml's sufficiency explicit rather than incidental. Either add ' to the list, which costs one replace and removes the dependency on a template convention nobody wrote down, or write the convention down in a comment above the function. I prefer the first, because comments do not survive a copy-paste into a new email template and a replace does.
And type the parameter honestly. escHtml(str: string) that calls String(str) is a function whose signature says it trusts its caller and whose body says it does not. escHtml(value: unknown) would be the truth, and would stop the next person from deleting the String() call as redundant, which it looks exactly like.
If you want to see the receiving end of all this, the help page covers what the alerts do and the supported sites hub has a guide per site explaining what each one's own notifications send instead. The listing titles in those emails are the only text in the product that arrives from outside it, which is the whole reason this file gets read more carefully than the rest.
