From 4c4dcbd6db01db95fc75368614f36e4c982bcf23 Mon Sep 17 00:00:00 2001 From: iamedwardngo <129360513+iamedwardngo@users.noreply.github.com> Date: Mon, 29 Jun 2026 01:38:49 +0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20Security=20?= =?UTF-8?q?hardening=20(#5241)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harden docs i18n rendering: path-traversal guard (cookie-controlled locale confined to docs/i18n via pure resolveSafeI18nSectionDir) + markdown XSS sanitization (DOMPurify allowlist). Review fixes: declared dompurify dep + allowlist, cleaned sanitizer config, extracted+tested the real path helper, removed agent scratch. Thanks @iamedwardngo! --- config/quality/dependency-allowlist.json | 1 + package-lock.json | 1 + package.json | 1 + src/app/docs/[...slug]/page.tsx | 13 ++++- src/lib/docsI18nPath.ts | 33 ++++++++++++ src/lib/docsSanitizer.ts | 42 +++++++++++++++ .../unit/security/docs-path-traversal.test.ts | 52 +++++++++++++++++++ tests/unit/security/docs-sanitization.test.ts | 27 ++++++++++ 8 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 src/lib/docsI18nPath.ts create mode 100644 src/lib/docsSanitizer.ts create mode 100644 tests/unit/security/docs-path-traversal.test.ts create mode 100644 tests/unit/security/docs-sanitization.test.ts diff --git a/config/quality/dependency-allowlist.json b/config/quality/dependency-allowlist.json index 4a6363c35d..ded3006030 100644 --- a/config/quality/dependency-allowlist.json +++ b/config/quality/dependency-allowlist.json @@ -46,6 +46,7 @@ "concurrently", "cross-env", "csv-stringify", + "dompurify", "dpdm", "electron", "electron-builder", diff --git a/package-lock.json b/package-lock.json index 19b80e3ccd..f0d52fc7cb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -30,6 +30,7 @@ "clsx": "^2.1.1", "commander": "^15.0.0", "csv-stringify": "^6.7.0", + "dompurify": "^3.4.11", "express": "^5.2.1", "fetch-socks": "^1.3.3", "fflate": "^0.8.3", diff --git a/package.json b/package.json index 3ff935039c..71eac3f6e8 100644 --- a/package.json +++ b/package.json @@ -222,6 +222,7 @@ "clsx": "^2.1.1", "commander": "^15.0.0", "csv-stringify": "^6.7.0", + "dompurify": "^3.4.11", "express": "^5.2.1", "fetch-socks": "^1.3.3", "fflate": "^0.8.3", diff --git a/src/app/docs/[...slug]/page.tsx b/src/app/docs/[...slug]/page.tsx index de27cd6e5e..bb375fb49f 100644 --- a/src/app/docs/[...slug]/page.tsx +++ b/src/app/docs/[...slug]/page.tsx @@ -8,6 +8,8 @@ import { DEFAULT_LOCALE, LOCALE_COOKIE } from "@/i18n/config"; import fs from "node:fs"; import path from "node:path"; import { marked } from "marked"; +import { sanitizeDocsHtml } from "@/lib/docsSanitizer"; +import { resolveSafeI18nSectionDir } from "@/lib/docsI18nPath"; // ── Locale detection ──────────────────────────────────────────────────────── @@ -28,8 +30,13 @@ function getDocsLocale(): string { function tryI18nFallback(slug: string[], locale: string): string | null { if (!locale || locale === "en") return null; + // 🛡️ Path traversal prevention — `locale` is a user-controllable cookie, so + // validate segments + confine the resolved dir to docs/i18n before any fs read. + // Centralized in resolveSafeI18nSectionDir (pure, unit-tested). const docsRoot = path.resolve(process.cwd(), "docs"); - const sectionDir = path.join(docsRoot, "i18n", locale, "docs", ...slug.slice(0, -1)); + const sectionDir = resolveSafeI18nSectionDir(docsRoot, locale, slug); + if (!sectionDir) return null; + if (!fs.existsSync(sectionDir)) return null; // Fumadocs lowercases slugs — match case-insensitively against i18n dir @@ -55,7 +62,9 @@ function tryI18nFallback(slug: string[], locale: string): string | null { ? raw.slice(bodyMatch.index + bodyMatch[0].length).trim() : raw; - return marked.parse(body) as string; + // 🛡️ Sentinel: XSS protection via server-side sanitization of rendered markdown + const html = marked.parse(body) as string; + return sanitizeDocsHtml(html); } // ── Page component ────────────────────────────────────────────────────────── diff --git a/src/lib/docsI18nPath.ts b/src/lib/docsI18nPath.ts new file mode 100644 index 0000000000..6565b9540a --- /dev/null +++ b/src/lib/docsI18nPath.ts @@ -0,0 +1,33 @@ +import path from "node:path"; + +// A locale or slug segment may only contain ASCII letters, digits and dashes. +// This blocks `..`, `/`, `\0`, and absolute paths before any filesystem access. +const SEGMENT_RE = /^[a-z0-9-]+$/i; + +/** + * Resolve the i18n section directory for a docs slug under `/i18n`, or + * return `null` when the locale/slug contain anything but `[a-z0-9-]` or the + * resolved path escapes `/i18n`. + * + * Pure path math — no filesystem access — so it is directly unit-testable and + * is the single chokepoint guarding the i18n docs fallback against path + * traversal (the `locale` comes from a user-controllable cookie). The prefix + * check uses `i18nRoot + path.sep` (not a bare `startsWith`) so a sibling like + * `…/i18n-evil` cannot satisfy it. + */ +export function resolveSafeI18nSectionDir( + docsRoot: string, + locale: string, + slug: string[] +): string | null { + if (!locale || !SEGMENT_RE.test(locale)) return null; + if (!slug.length || !slug.every((s) => SEGMENT_RE.test(s))) return null; + + const i18nRoot = path.join(docsRoot, "i18n"); + const sectionDir = path.resolve(i18nRoot, locale, "docs", ...slug.slice(0, -1)); + + if (sectionDir !== i18nRoot && !sectionDir.startsWith(i18nRoot + path.sep)) { + return null; + } + return sectionDir; +} diff --git a/src/lib/docsSanitizer.ts b/src/lib/docsSanitizer.ts new file mode 100644 index 0000000000..c5b41143d1 --- /dev/null +++ b/src/lib/docsSanitizer.ts @@ -0,0 +1,42 @@ +import createDOMPurify from "dompurify"; +import { JSDOM } from "jsdom"; + +type Sanitizer = ReturnType; + +let sanitizer: Sanitizer | null = null; + +// Curated allowlist for HTML rendered from trusted-repo markdown (marked output). +// Explicit ALLOWED_TAGS/ATTR (no USE_PROFILES — when USE_PROFILES is set DOMPurify +// ignores ALLOWED_TAGS entirely) so the surviving tag set is deterministic and +// reviewable. Covers GFM output: headings, lists, tables, code, images, GFM +// task-list checkboxes (`input[type=checkbox]`), and collapsible details blocks. +const ALLOWED_TAGS = [ + "h1", "h2", "h3", "h4", "h5", "h6", "blockquote", "p", "a", "ul", "ol", + "li", "ins", "del", "sub", "sup", "em", "strong", "span", "hr", "br", + "div", "table", "thead", "caption", "tbody", "tr", "th", "td", "pre", + "code", "img", "details", "summary", "input", +]; +const ALLOWED_ATTR = [ + "href", "name", "target", "src", "alt", "title", "class", "id", "type", + "checked", "disabled", "rel", +]; + +/** + * Get or create a server-side DOMPurify instance (jsdom window — DOMPurify needs a DOM). + */ +function getSanitizer(): Sanitizer { + if (!sanitizer) { + const window = new JSDOM("").window; + sanitizer = createDOMPurify(window as unknown as Window); + } + return sanitizer; +} + +/** + * Sanitize HTML content for documentation display. + * @param html The raw HTML to sanitize + */ +export function sanitizeDocsHtml(html: string): string { + const purify = getSanitizer(); + return purify.sanitize(html, { ALLOWED_TAGS, ALLOWED_ATTR }); +} diff --git a/tests/unit/security/docs-path-traversal.test.ts b/tests/unit/security/docs-path-traversal.test.ts new file mode 100644 index 0000000000..c3d054af43 --- /dev/null +++ b/tests/unit/security/docs-path-traversal.test.ts @@ -0,0 +1,52 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import path from "node:path"; +import { resolveSafeI18nSectionDir } from "../../../src/lib/docsI18nPath.ts"; + +const DOCS_ROOT = path.resolve("/srv/app", "docs"); +const I18N_ROOT = path.join(DOCS_ROOT, "i18n"); + +test("resolveSafeI18nSectionDir rejects traversal in the locale (cookie-controlled)", () => { + const badLocales = ["..", "../..", "/etc", "es/../../etc", "es\0", "..%2f", "en.", "es/x"]; + for (const locale of badLocales) { + assert.strictEqual( + resolveSafeI18nSectionDir(DOCS_ROOT, locale, ["overview"]), + null, + `Should reject bad locale: ${JSON.stringify(locale)}` + ); + } +}); + +test("resolveSafeI18nSectionDir rejects traversal in slug segments", () => { + const badSlugs = [[".."], ["..", "foo"], ["/etc", "passwd"], ["foo", "..", "bar"], ["a/b"]]; + for (const slug of badSlugs) { + assert.strictEqual( + resolveSafeI18nSectionDir(DOCS_ROOT, "pt-BR", slug), + null, + `Should reject bad slug: ${slug.join("/")}` + ); + } +}); + +test("resolveSafeI18nSectionDir confines the resolved dir to docs/i18n", () => { + // Valid input → a dir strictly under i18nRoot. + const ok = resolveSafeI18nSectionDir(DOCS_ROOT, "pt-BR", ["architecture", "overview"]); + assert.ok(ok, "valid locale+slug should resolve"); + assert.ok( + ok === I18N_ROOT || ok.startsWith(I18N_ROOT + path.sep), + `resolved dir must stay under i18nRoot, got ${ok}` + ); + assert.strictEqual(ok, path.join(I18N_ROOT, "pt-BR", "docs", "architecture")); +}); + +test("resolveSafeI18nSectionDir allows valid locale/slug patterns", () => { + const goodLocales = ["en", "pt-BR", "zh-CN", "es"]; + for (const locale of goodLocales) { + assert.ok( + resolveSafeI18nSectionDir(DOCS_ROOT, locale, ["getting-started"]), + `Should allow good locale: ${locale}` + ); + } + assert.ok(resolveSafeI18nSectionDir(DOCS_ROOT, "es", ["v1-migration"])); + assert.strictEqual(resolveSafeI18nSectionDir(DOCS_ROOT, "es", []), null, "empty slug → null"); +}); diff --git a/tests/unit/security/docs-sanitization.test.ts b/tests/unit/security/docs-sanitization.test.ts new file mode 100644 index 0000000000..cc1d77a3f1 --- /dev/null +++ b/tests/unit/security/docs-sanitization.test.ts @@ -0,0 +1,27 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { sanitizeDocsHtml } from "../../../src/lib/docsSanitizer.ts"; + +test("sanitizeDocsHtml removes dangerous tags and attributes", () => { + const malicious = ' link
content
'; + const sanitized = sanitizeDocsHtml(malicious); + + assert.ok(!sanitized.includes("