diff --git a/src/lib/notifications/__tests__/strip-html.test.ts b/src/lib/notifications/__tests__/strip-html.test.ts new file mode 100644 index 000000000..063d0ae3b --- /dev/null +++ b/src/lib/notifications/__tests__/strip-html.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect } from "vitest"; +import { stripHtml } from "@/lib/notifications/strip-html"; + +/** + * The five plain-text senders used to strip tags with a local single-pass + * regex replace (CodeQL: incomplete multi-character sanitization). The + * hardened shared helper re-applies the strip until the string stops + * changing, so a tag re-formed by an earlier removal cannot survive. These + * cases pin the invariant that matters — no tag-shaped sequence in any + * output — and identity on plain input. + */ +describe("stripHtml", () => { + it("leaves plain input unchanged", () => { + expect(stripHtml("Blood pressure logged: 120/80")).toBe( + "Blood pressure logged: 120/80", + ); + expect(stripHtml("")).toBe(""); + }); + + it("strips simple tags exactly like the old single pass", () => { + expect(stripHtml("urgent reading")).toBe("urgent reading"); + expect(stripHtml("line
break")).toBe("linebreak"); + expect(stripHtml("a < b and b > a")).toBe("a a"); + }); + + it("removes the classic nested-tag bypass shapes", () => { + expect(stripHtml("<script>alert(1)")).toBe("script>alert(1)"); + expect(stripHtml("<script>")).toBe("script>"); + expect(stripHtml("<<script>>payload")).toBe("script>>payload"); + }); + + it("never lets a tag-shaped sequence survive any adversarial input", () => { + const hostile = [ + "<script>alert(1)</script>", + "ipt>alert(1)ipt>", + "<".repeat(64) + "b>".repeat(64) + "script>alert(1)", + '', + "e>f>g>", + ]; + for (const input of hostile) { + expect(stripHtml(input), input).not.toMatch(/<[^>]*>/); + } + }); + + it("handles unterminated brackets without stripping legitimate text", () => { + // No closing `>` anywhere after the `<` — nothing tag-shaped to remove. + expect(stripHtml("]*>/g, ""); -} - /** * Whitelist the metadata keys we send through to iOS. The dispatcher * payload's `metadata` field is shared across every channel — Telegram diff --git a/src/lib/notifications/senders/email.ts b/src/lib/notifications/senders/email.ts index 813a1660a..86afb748e 100644 --- a/src/lib/notifications/senders/email.ts +++ b/src/lib/notifications/senders/email.ts @@ -10,6 +10,7 @@ import { getEvent } from "@/lib/logging/context"; import { recordPushAttemptForPayload } from "@/lib/notifications/senders/push-attempt-record"; import { loadEmailConfig } from "@/lib/notifications/senders/email-config"; import { plainPushText } from "@/lib/notifications/strip-emoji"; +import { stripHtml } from "@/lib/notifications/strip-html"; /** * SMTP / email sender (v1.17.1). @@ -126,10 +127,7 @@ export async function sendViaEmail( // Plain text only — strip HTML + decorative emoji on routine reminders. const subject = plainPushText(payload.title, payload.eventType); - const text = plainPushText( - payload.message.replace(/<[^>]*>/g, ""), - payload.eventType, - ); + const text = plainPushText(stripHtml(payload.message), payload.eventType); try { await transport.transporter.sendMail({ diff --git a/src/lib/notifications/senders/ntfy.ts b/src/lib/notifications/senders/ntfy.ts index ade424ac6..ea966adac 100644 --- a/src/lib/notifications/senders/ntfy.ts +++ b/src/lib/notifications/senders/ntfy.ts @@ -8,6 +8,7 @@ import { getEvent } from "@/lib/logging/context"; import { recordPushAttemptForPayload } from "@/lib/notifications/senders/push-attempt-record"; import { safeFetch } from "@/lib/safe-fetch"; import { plainPushText } from "@/lib/notifications/strip-emoji"; +import { stripHtml } from "@/lib/notifications/strip-html"; import { isUrgentPayload } from "@/lib/notifications/types"; /** @@ -57,10 +58,7 @@ export async function sendViaNtfy( } // Strip HTML tags for ntfy (plain text only) + emoji on routine reminders - const body = plainPushText( - payload.message.replace(/<[^>]*>/g, ""), - payload.eventType, - ); + const body = plainPushText(stripHtml(payload.message), payload.eventType); // requirePublicHost adds the DNS-rebinding pin (issue #217) on top // of the input-time isPublicUrl guard already enforced when the diff --git a/src/lib/notifications/senders/web-push.ts b/src/lib/notifications/senders/web-push.ts index 753f793a4..cf26df4f3 100644 --- a/src/lib/notifications/senders/web-push.ts +++ b/src/lib/notifications/senders/web-push.ts @@ -9,6 +9,7 @@ import { recordPushAttemptForPayload } from "@/lib/notifications/senders/push-at import { isPublicUrl } from "@/lib/validations/notifications"; import { safeFetch } from "@/lib/safe-fetch"; import { plainPushText } from "@/lib/notifications/strip-emoji"; +import { stripHtml } from "@/lib/notifications/strip-html"; import { isManagedGuardianDelivery } from "@/lib/notifications/managed-delivery"; /** @@ -102,10 +103,7 @@ export async function sendViaWebPush( const pushPayload = JSON.stringify({ title: plainPushText(payload.title, payload.eventType), - body: plainPushText( - payload.message.replace(/<[^>]*>/g, ""), - payload.eventType, - ), + body: plainPushText(stripHtml(payload.message), payload.eventType), // Discreet mode (cycle privacy): the coalescing `tag` must not name the // event. Web-Push payloads are VAPID-encrypted (only the SW sees this), // but the discreet contract still requires no event name on the wire — diff --git a/src/lib/notifications/senders/webhook.ts b/src/lib/notifications/senders/webhook.ts index fd01eeac8..34de62f72 100644 --- a/src/lib/notifications/senders/webhook.ts +++ b/src/lib/notifications/senders/webhook.ts @@ -8,6 +8,7 @@ import { getEvent } from "@/lib/logging/context"; import { recordPushAttemptForPayload } from "@/lib/notifications/senders/push-attempt-record"; import { safeFetch } from "@/lib/safe-fetch"; import { plainPushText } from "@/lib/notifications/strip-emoji"; +import { stripHtml } from "@/lib/notifications/strip-html"; /** * Send a notification via a generic outbound webhook (v1.17.1). @@ -49,10 +50,7 @@ export async function sendViaWebhook( // a relay rule can route without leaking the cycle event name. const body = JSON.stringify({ title: plainPushText(payload.title, payload.eventType), - message: plainPushText( - payload.message.replace(/<[^>]*>/g, ""), - payload.eventType, - ), + message: plainPushText(stripHtml(payload.message), payload.eventType), eventType: payload.discreet ? "reminder" : payload.eventType, // v1.18.4 — an explicitly urgent event maps to `urgent` so a relay // rule (Gotify priority, Discord mention, Home Assistant automation) diff --git a/src/lib/notifications/strip-html.ts b/src/lib/notifications/strip-html.ts new file mode 100644 index 000000000..2c1645945 --- /dev/null +++ b/src/lib/notifications/strip-html.ts @@ -0,0 +1,37 @@ +/** + * Strip HTML tags from notification content, hardened against + * multi-character bypasses. + * + * Every plain-text sender (APNs, Web Push, webhook, ntfy, email) used to + * carry its own single-pass `replace(/<[^>]*>/g, "")` — the shape CodeQL + * flags as incomplete multi-character sanitization, because a removal can + * re-form the very pattern it removed (`<script>` loses `` and the + * remaining characters close ranks). Whether a given regex happens to be + * idempotent is an argument about that regex; five copies of the argument + * across five senders is how the next edit quietly breaks one of them. + * + * This helper makes the guarantee structural instead: re-apply the strip + * until the string stops changing, so a tag re-formed by an earlier + * removal is removed on the next pass no matter how deep the nesting. The + * iteration cap bounds CPU on adversarial input; a string still changing + * at the cap is pathological by construction, and for that remnant the + * angle brackets themselves are dropped so no `<...>` sequence can ever + * survive. Plain input without tags returns unchanged, same as the old + * single pass. + */ + +const TAG_RE = /<[^>]*>/g; +const MAX_STRIP_PASSES = 10; + +/** Remove HTML tags, re-applying until stable so stripped tags cannot re-form. */ +export function stripHtml(text: string): string { + let out = text; + for (let i = 0; i < MAX_STRIP_PASSES; i++) { + const next = out.replace(TAG_RE, ""); + if (next === out) return next; + out = next; + } + // Still changing after the cap — nesting this deep is adversarial. + // Drop the brackets outright so nothing tag-shaped remains. + return out.replace(/[<>]/g, ""); +}