mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 13:22:14 +00:00
Centralise notification HTML stripping behind one hardened helper
Five senders (APNs, Web Push, webhook, ntfy, email) each carried their own single-pass replace(/<[^>]*>/g, "") — the shape CodeQL flags as incomplete multi-character sanitization, because a removal can re-form the pattern it removed. Whether one particular regex happens to be idempotent is an argument about that regex; five copies of the argument is how the next edit quietly breaks one of them. The shared stripHtml re-applies the strip until the string stops changing, with an iteration cap and a bracket-dropping fallback so no tag-shaped sequence can survive any input. Plain text passes through unchanged, same as before. Unit tests pin the nested-tag bypass shapes and the no-surviving-tag invariant. Resolves CodeQL alerts 38-42.
This commit is contained in:
1 parent
511ddf5de1
commit
347e14d26b
7 files changed
+96
-20
No files matched your search
@@ -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("<b>urgent</b> reading")).toBe("urgent reading");
|
||||
expect(stripHtml("line<br/>break")).toBe("linebreak");
|
||||
expect(stripHtml("a < b and b > a")).toBe("a a");
|
||||
});
|
||||
|
||||
it("removes the classic nested-tag bypass shapes", () => {
|
||||
expect(stripHtml("<<b>script>alert(1)")).toBe("script>alert(1)");
|
||||
expect(stripHtml("<<b>script>")).toBe("script>");
|
||||
expect(stripHtml("<<<i><b>script>>payload")).toBe("script>>payload");
|
||||
});
|
||||
|
||||
it("never lets a tag-shaped sequence survive any adversarial input", () => {
|
||||
const hostile = [
|
||||
"<<b>script>alert(1)<</b>/script>",
|
||||
"<scr<b>ipt>alert(1)</scr<b>ipt>",
|
||||
"<".repeat(64) + "b>".repeat(64) + "script>alert(1)",
|
||||
'<img src=">" onerror=alert(1)>',
|
||||
"<a<b<c<d>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("<div")).toBe("<div");
|
||||
expect(stripHtml("2 < 3 <b")).toBe("2 < 3 <b");
|
||||
});
|
||||
});
|
||||
@@ -31,6 +31,7 @@ import { getEvent } from "@/lib/logging/context";
|
||||
import type { SendOutcome } from "@/lib/notifications/retry-policy";
|
||||
import { recordPushAttemptForPayload } from "@/lib/notifications/senders/push-attempt-record";
|
||||
import { plainPushText } from "@/lib/notifications/strip-emoji";
|
||||
import { stripHtml } from "@/lib/notifications/strip-html";
|
||||
import { isManagedGuardianDelivery } from "@/lib/notifications/managed-delivery";
|
||||
|
||||
export interface ApnsPayload {
|
||||
@@ -998,10 +999,6 @@ export async function sendApnsRawPush(
|
||||
};
|
||||
}
|
||||
|
||||
function stripHtml(s: string): string {
|
||||
return s.replace(/<[^>]*>/g, "");
|
||||
}
|
||||
|
||||
/**
|
||||
* Whitelist the metadata keys we send through to iOS. The dispatcher
|
||||
* payload's `metadata` field is shared across every channel — Telegram
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 —
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 (`<<b>script>` loses `<b>` 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, "");
|
||||
}
|
||||
Reference in new issue
Block a user