mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
🛡️ Sentinel: Security hardening (#5241)
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!
This commit is contained in:
@@ -46,6 +46,7 @@
|
||||
"concurrently",
|
||||
"cross-env",
|
||||
"csv-stringify",
|
||||
"dompurify",
|
||||
"dpdm",
|
||||
"electron",
|
||||
"electron-builder",
|
||||
|
||||
1
package-lock.json
generated
1
package-lock.json
generated
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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 ──────────────────────────────────────────────────────────
|
||||
|
||||
33
src/lib/docsI18nPath.ts
Normal file
33
src/lib/docsI18nPath.ts
Normal file
@@ -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 `<docsRoot>/i18n`, or
|
||||
* return `null` when the locale/slug contain anything but `[a-z0-9-]` or the
|
||||
* resolved path escapes `<docsRoot>/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;
|
||||
}
|
||||
42
src/lib/docsSanitizer.ts
Normal file
42
src/lib/docsSanitizer.ts
Normal file
@@ -0,0 +1,42 @@
|
||||
import createDOMPurify from "dompurify";
|
||||
import { JSDOM } from "jsdom";
|
||||
|
||||
type Sanitizer = ReturnType<typeof createDOMPurify>;
|
||||
|
||||
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 });
|
||||
}
|
||||
52
tests/unit/security/docs-path-traversal.test.ts
Normal file
52
tests/unit/security/docs-path-traversal.test.ts
Normal file
@@ -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");
|
||||
});
|
||||
27
tests/unit/security/docs-sanitization.test.ts
Normal file
27
tests/unit/security/docs-sanitization.test.ts
Normal file
@@ -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 = '<script>alert("xss")</script><img src="x" onerror="alert(1)"> <a href="javascript:alert(1)">link</a> <div onclick="evil()">content</div>';
|
||||
const sanitized = sanitizeDocsHtml(malicious);
|
||||
|
||||
assert.ok(!sanitized.includes("<script>"), "Should remove <script>");
|
||||
assert.ok(!sanitized.includes("onerror"), "Should remove onerror");
|
||||
assert.ok(!sanitized.includes("onclick"), "Should remove onclick");
|
||||
assert.ok(!sanitized.includes("javascript:"), "Should remove javascript: URLs");
|
||||
assert.ok(sanitized.includes("<a>link</a>") || sanitized.includes("<a >link</a>") || sanitized.includes("link"), "Should keep safe content");
|
||||
});
|
||||
|
||||
test("sanitizeDocsHtml allows safe tags and attributes", () => {
|
||||
const safe = '<h1>Title</h1><p>Paragraph with <strong>bold</strong> and <em>italic</em>.</p><ul><li>Item</li></ul><pre><code>code</code></pre>';
|
||||
const sanitized = sanitizeDocsHtml(safe);
|
||||
|
||||
assert.ok(sanitized.includes("<h1>Title</h1>"), "Should allow <h1>");
|
||||
assert.ok(sanitized.includes("<p>"), "Should allow <p>");
|
||||
assert.ok(sanitized.includes("<strong>"), "Should allow <strong>");
|
||||
assert.ok(sanitized.includes("<ul>"), "Should allow <ul>");
|
||||
assert.ok(sanitized.includes("<li>"), "Should allow <li>");
|
||||
assert.ok(sanitized.includes("<pre>"), "Should allow <pre>");
|
||||
assert.ok(sanitized.includes("<code>"), "Should allow <code>");
|
||||
});
|
||||
Reference in New Issue
Block a user