Skip to content

Commit dece4bc

Browse files
authored
feat(notification): XSS protection for rich-text template fields (#611) (#652)
Sanitize rich-text email/notification templates server-side so stored HTML can never carry executable markup, with CSP defence-in-depth on the admin dashboard. - backend/shared/sanitizer: HTMLSanitizerService wrapping DOMPurify (via jsdom) with a strict tag/attribute allowlist, http/https/mailto-only href protocols, full SVG/script/iframe stripping, and forced rel=noreferrer on links. - backend/shared/middleware/cspMiddleware.ts: hardened CSP (script-src 'self', object-src 'none') plus companion security headers, with Express and Next.js adapters. - backend/services/notification/templateService.ts: sanitize body on save, strip HTML from subjects, and render a sanitized preview before save. - backend/services/notification/jobs/backScanTemplates.ts: idempotent cron that quarantines and re-sanitizes pre-existing unsafe templates. - developer-portal/utils/securityHeaders.ts: applies the shared CSP to admin dashboard routes. - package.json: add dompurify, jsdom, @types/jsdom. Closes #611
1 parent 57d61c5 commit dece4bc

7 files changed

Lines changed: 446 additions & 0 deletions

File tree

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/**
2+
* Back-scan job for pre-existing notification templates.
3+
*
4+
* Issue #611 edge case: templates saved before server-side sanitization was
5+
* introduced may already contain malicious HTML. This cron scans every stored
6+
* template, and for any whose body changes under sanitization it:
7+
* 1. quarantines the original (flags it so it is not sent/previewed raw), and
8+
* 2. stores the sanitized body so the template remains usable.
9+
*
10+
* The job is idempotent — clean templates are left untouched and re-running it
11+
* is a no-op.
12+
*/
13+
14+
import { htmlSanitizer } from '../../../shared/sanitizer';
15+
import type {
16+
NotificationTemplate,
17+
TemplateRepository,
18+
} from '../templateService';
19+
20+
export interface BackScanResult {
21+
scanned: number;
22+
quarantined: number;
23+
/** IDs of templates that contained unsafe content. */
24+
quarantinedIds: string[];
25+
}
26+
27+
export interface BackScanOptions {
28+
/** When true, report findings without writing changes. Default: false. */
29+
dryRun?: boolean;
30+
}
31+
32+
export class BackScanTemplatesJob {
33+
constructor(private readonly repo: TemplateRepository) {}
34+
35+
async run(options: BackScanOptions = {}): Promise<BackScanResult> {
36+
const templates = await this.repo.list();
37+
const result: BackScanResult = { scanned: 0, quarantined: 0, quarantinedIds: [] };
38+
39+
for (const template of templates) {
40+
result.scanned += 1;
41+
const { clean, modified } = await htmlSanitizer.sanitize(template.body);
42+
if (!modified) continue;
43+
44+
result.quarantined += 1;
45+
result.quarantinedIds.push(template.id);
46+
47+
console.warn(
48+
JSON.stringify({
49+
level: 'warn',
50+
event: 'template_quarantined',
51+
templateId: template.id,
52+
message: 'Pre-existing template contained unsafe HTML; sanitized and quarantined',
53+
}),
54+
);
55+
56+
if (!options.dryRun) {
57+
const sanitized: NotificationTemplate = {
58+
...template,
59+
body: clean,
60+
quarantined: true,
61+
updatedAt: new Date().toISOString(),
62+
};
63+
await this.repo.save(sanitized);
64+
}
65+
}
66+
67+
return result;
68+
}
69+
}
70+
71+
/**
72+
* Cron entry point. Wire this to the scheduler (e.g. daily) with a concrete
73+
* repository implementation.
74+
*/
75+
export async function runBackScanTemplates(
76+
repo: TemplateRepository,
77+
options?: BackScanOptions,
78+
): Promise<BackScanResult> {
79+
return new BackScanTemplatesJob(repo).run(options);
80+
}
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* Notification/email template service with XSS-safe rich-text handling.
3+
*
4+
* Issue #611: rich-text template fields (subject is plain text; body is HTML)
5+
* must be sanitized server-side on save so stored content can never carry
6+
* executable markup. A preview renders the *sanitized* output so admins see
7+
* exactly what recipients will get.
8+
*/
9+
10+
import { htmlSanitizer, type SanitizeResult } from '../../shared/sanitizer';
11+
12+
export interface NotificationTemplate {
13+
id: string;
14+
/** Plain-text subject line (no HTML). */
15+
subject: string;
16+
/** Rich-text HTML body. Always stored already-sanitized. */
17+
body: string;
18+
updatedAt: string;
19+
/** Set by the back-scan job when pre-existing content was unsafe. */
20+
quarantined?: boolean;
21+
}
22+
23+
export interface TemplateInput {
24+
id: string;
25+
subject: string;
26+
body: string;
27+
}
28+
29+
/** Storage abstraction — implemented by the persistence layer. */
30+
export interface TemplateRepository {
31+
save(template: NotificationTemplate): Promise<void>;
32+
get(id: string): Promise<NotificationTemplate | null>;
33+
list(): Promise<NotificationTemplate[]>;
34+
}
35+
36+
export interface SaveTemplateResult {
37+
template: NotificationTemplate;
38+
/** True when the submitted body contained content that was stripped. */
39+
sanitized: boolean;
40+
removedCount: number;
41+
}
42+
43+
export class TemplateService {
44+
constructor(private readonly repo: TemplateRepository) {}
45+
46+
/** Strip HTML from the subject line entirely — subjects are plain text. */
47+
private async cleanSubject(subject: string): Promise<string> {
48+
const { clean } = await htmlSanitizer.sanitize(subject);
49+
// Drop any residual tags: a subject should contain no markup at all.
50+
return clean.replace(/<[^>]*>/g, '').trim();
51+
}
52+
53+
/**
54+
* Sanitize and persist a template. The stored body is always the sanitized
55+
* output, so a malicious payload can never reach another admin's browser.
56+
*/
57+
async saveTemplate(input: TemplateInput): Promise<SaveTemplateResult> {
58+
const result: SanitizeResult = await htmlSanitizer.sanitize(input.body);
59+
const template: NotificationTemplate = {
60+
id: input.id,
61+
subject: await this.cleanSubject(input.subject),
62+
body: result.clean,
63+
updatedAt: new Date().toISOString(),
64+
};
65+
await this.repo.save(template);
66+
return {
67+
template,
68+
sanitized: result.modified,
69+
removedCount: result.removedCount,
70+
};
71+
}
72+
73+
/**
74+
* Render a sanitized preview without persisting. Used by the preview pane so
75+
* admins review the safe output before saving.
76+
*/
77+
async renderPreview(input: TemplateInput): Promise<{ subject: string; body: string; sanitized: boolean }> {
78+
const subject = await this.cleanSubject(input.subject);
79+
const { clean, modified } = await htmlSanitizer.sanitize(input.body);
80+
return { subject, body: clean, sanitized: modified };
81+
}
82+
}
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Content-Security-Policy middleware for the admin dashboard.
3+
*
4+
* Issue #611: even with stored rich-text sanitized, the admin dashboard needs a
5+
* CSP as defence-in-depth so any HTML that slips through cannot execute inline
6+
* scripts or load hostile objects. `script-src 'self'` blocks inline/injected
7+
* scripts; `object-src 'none'` blocks <object>/<embed>/Flash vectors.
8+
*
9+
* Framework-agnostic: `buildCspHeader()` returns the header value, and small
10+
* adapters are provided for Express-style and Next.js responses.
11+
*/
12+
13+
export interface CspOptions {
14+
/** Extra hosts to allow for images (e.g. a CDN). Default: none beyond self/data/https. */
15+
imgSrc?: string[];
16+
/** Extra hosts to allow for XHR/fetch (e.g. the API origin). */
17+
connectSrc?: string[];
18+
/** Report-only mode emits the header without enforcing it. Default: false. */
19+
reportOnly?: boolean;
20+
/** Optional reporting endpoint. */
21+
reportUri?: string;
22+
}
23+
24+
/** The hardened CSP directive set for admin pages. */
25+
export function buildCspHeader(options: CspOptions = {}): { name: string; value: string } {
26+
const directives: Record<string, string[]> = {
27+
'default-src': ["'self'"],
28+
'script-src': ["'self'"],
29+
'object-src': ["'none'"],
30+
'base-uri': ["'self'"],
31+
'frame-ancestors': ["'self'"],
32+
'form-action': ["'self'"],
33+
// Allow inline styles (rich-text uses the style attribute) but no inline JS.
34+
'style-src': ["'self'", "'unsafe-inline'"],
35+
'img-src': ["'self'", 'data:', 'https:', ...(options.imgSrc ?? [])],
36+
'connect-src': ["'self'", ...(options.connectSrc ?? [])],
37+
'font-src': ["'self'", 'data:'],
38+
};
39+
40+
if (options.reportUri) {
41+
directives['report-uri'] = [options.reportUri];
42+
}
43+
44+
const value = Object.entries(directives)
45+
.map(([key, vals]) => `${key} ${vals.join(' ')}`)
46+
.join('; ');
47+
48+
return {
49+
name: options.reportOnly
50+
? 'Content-Security-Policy-Report-Only'
51+
: 'Content-Security-Policy',
52+
value,
53+
};
54+
}
55+
56+
/** Companion security headers that pair with the CSP. */
57+
export function securityHeaders(options: CspOptions = {}): Record<string, string> {
58+
const csp = buildCspHeader(options);
59+
return {
60+
[csp.name]: csp.value,
61+
'X-Content-Type-Options': 'nosniff',
62+
'X-Frame-Options': 'SAMEORIGIN',
63+
'Referrer-Policy': 'strict-origin-when-cross-origin',
64+
};
65+
}
66+
67+
/** Express/Connect-style middleware. */
68+
export function cspMiddleware(options: CspOptions = {}) {
69+
const headers = securityHeaders(options);
70+
return function applyCsp(
71+
_req: unknown,
72+
res: { setHeader(name: string, value: string): void },
73+
next: () => void,
74+
): void {
75+
for (const [name, value] of Object.entries(headers)) {
76+
res.setHeader(name, value);
77+
}
78+
next();
79+
};
80+
}
81+
82+
/** Next.js `headers()` config entries for admin dashboard routes. */
83+
export function nextSecurityHeaders(options: CspOptions = {}): Array<{ key: string; value: string }> {
84+
return Object.entries(securityHeaders(options)).map(([key, value]) => ({ key, value }));
85+
}
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/**
2+
* Server-side HTML sanitization for rich-text template fields.
3+
*
4+
* Issue #611: Email and notification templates accept rich text containing
5+
* HTML. Without sanitization these fields are stored XSS vectors — a malicious
6+
* admin can inject `<script>` (or `onerror`, `javascript:` URLs, hostile SVGs)
7+
* that execute in other admins' browsers when a template is previewed or sent.
8+
*
9+
* This service wraps DOMPurify (running in a jsdom window so it works in Node)
10+
* and enforces a strict allowlist:
11+
* - Tags: p span a img table tr td th h1-h6 ul ol li strong em br hr
12+
* - Attributes: href src alt style class target (rel=noreferrer forced)
13+
* - Protocols: href only http:/https:/mailto: (javascript:/data:/vbscript: rejected)
14+
* - SVG: all SVG tags/attributes stripped (common XSS vector)
15+
*
16+
* `dompurify` and `jsdom` are backend-only dependencies, dynamically imported
17+
* so this module never lands in the React Native bundle and so environments
18+
* without them fail loudly only when sanitization is actually invoked.
19+
*/
20+
21+
// ── Allowlists ────────────────────────────────────────────────────────────────
22+
23+
export const ALLOWED_TAGS = [
24+
'p', 'span', 'a', 'img', 'table', 'tr', 'td', 'th',
25+
'h1', 'h2', 'h3', 'h4', 'h5', 'h6',
26+
'ul', 'ol', 'li', 'strong', 'em', 'br', 'hr',
27+
] as const;
28+
29+
export const ALLOWED_ATTR = ['href', 'src', 'alt', 'style', 'class', 'target'] as const;
30+
31+
/** Protocols permitted in `href`. Everything else (javascript:, data:, vbscript:) is rejected. */
32+
export const ALLOWED_URI_PROTOCOLS = ['http', 'https', 'mailto'] as const;
33+
34+
// DOMPurify URI regexp: allow only the protocols above, plus relative/anchor URIs.
35+
const ALLOWED_URI_REGEXP = /^(?:(?:https?|mailto):|[^a-z]|[a-z+.-]+(?:[^a-z+.\-:]|$))/i;
36+
37+
export interface SanitizeResult {
38+
/** The sanitized, safe-to-store HTML. */
39+
clean: string;
40+
/** True when sanitization removed or altered content (i.e. input was unsafe). */
41+
modified: boolean;
42+
/** Number of nodes/attributes DOMPurify removed. */
43+
removedCount: number;
44+
}
45+
46+
// ── DOMPurify factory (lazy, memoised) ────────────────────────────────────────
47+
48+
interface DOMPurifyInstance {
49+
sanitize(dirty: string, config: Record<string, unknown>): string;
50+
addHook(entry: string, cb: (node: unknown) => void): void;
51+
removed: unknown[];
52+
}
53+
54+
let _purify: DOMPurifyInstance | null = null;
55+
56+
async function getPurify(): Promise<DOMPurifyInstance> {
57+
if (_purify) return _purify;
58+
59+
const { JSDOM } = (await import('jsdom')) as {
60+
JSDOM: new (html: string) => { window: unknown };
61+
};
62+
const createDOMPurify = (await import('dompurify')).default as (
63+
win: unknown,
64+
) => DOMPurifyInstance;
65+
66+
const { window } = new JSDOM('');
67+
const purify = createDOMPurify(window);
68+
69+
// Force rel="noreferrer noopener" on every link and constrain target.
70+
purify.addHook('afterSanitizeAttributes', (node: unknown) => {
71+
const el = node as {
72+
tagName?: string;
73+
getAttribute(name: string): string | null;
74+
setAttribute(name: string, value: string): void;
75+
};
76+
if (el.tagName === 'A' && el.getAttribute('target')) {
77+
el.setAttribute('target', '_blank');
78+
el.setAttribute('rel', 'noreferrer noopener');
79+
}
80+
});
81+
82+
_purify = purify;
83+
return purify;
84+
}
85+
86+
// ── Service ───────────────────────────────────────────────────────────────────
87+
88+
export class HTMLSanitizerService {
89+
/**
90+
* Sanitize a rich-text HTML string against the template allowlist.
91+
* Returns the clean HTML plus whether anything was stripped.
92+
*/
93+
async sanitize(dirty: string): Promise<SanitizeResult> {
94+
if (dirty == null || dirty === '') {
95+
return { clean: '', modified: false, removedCount: 0 };
96+
}
97+
98+
const purify = await getPurify();
99+
100+
const clean = purify.sanitize(dirty, {
101+
ALLOWED_TAGS: [...ALLOWED_TAGS],
102+
ALLOWED_ATTR: [...ALLOWED_ATTR],
103+
ALLOWED_URI_REGEXP,
104+
// Strip SVG and MathML entirely — both are common XSS vectors.
105+
FORBID_TAGS: ['svg', 'math', 'script', 'style', 'iframe', 'object', 'embed', 'form'],
106+
FORBID_ATTR: ['onerror', 'onload', 'onclick', 'onmouseover', 'srcset', 'formaction'],
107+
USE_PROFILES: { html: true },
108+
// Keep text content of removed elements, but never their markup.
109+
KEEP_CONTENT: true,
110+
RETURN_TRUSTED_TYPE: false,
111+
});
112+
113+
const removedCount = Array.isArray(purify.removed) ? purify.removed.length : 0;
114+
// Normalise whitespace differences so a no-op sanitize isn't flagged as modified.
115+
const modified = removedCount > 0 || normalize(clean) !== normalize(dirty);
116+
117+
return { clean, modified, removedCount };
118+
}
119+
120+
/**
121+
* Convenience: returns only the clean HTML.
122+
*/
123+
async clean(dirty: string): Promise<string> {
124+
return (await this.sanitize(dirty)).clean;
125+
}
126+
127+
/**
128+
* Returns true if the input contains content that would be stripped — useful
129+
* for the back-scan job's quarantine decision without mutating storage.
130+
*/
131+
async isUnsafe(html: string): Promise<boolean> {
132+
return (await this.sanitize(html)).modified;
133+
}
134+
}
135+
136+
function normalize(html: string): string {
137+
return html.replace(/\s+/g, ' ').trim();
138+
}
139+
140+
/** Shared singleton instance. */
141+
export const htmlSanitizer = new HTMLSanitizerService();

0 commit comments

Comments
 (0)