diff --git a/.gitignore b/.gitignore index ec8a94f..5ed4cc3 100644 --- a/.gitignore +++ b/.gitignore @@ -59,3 +59,4 @@ docker-compose.override.yaml CLAUDE.md .codex/ AGENTS.md +.serena diff --git a/lib/allowlist.js b/lib/allowlist.js index 41e5f22..da36f15 100644 --- a/lib/allowlist.js +++ b/lib/allowlist.js @@ -1,5 +1,7 @@ const fs = require('node:fs'); +const RAW_TEXT_BYPASS_TAGS = Object.freeze(['xmp']); + const DEFAULT_ALLOWLIST = Object.freeze({ allowedTags: Object.freeze([ 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'p', 'blockquote', 'ul', 'ol', 'li', 'br', 'hr', @@ -16,8 +18,22 @@ const DEFAULT_ALLOWLIST = Object.freeze({ }), allowedSchemes: Object.freeze(['http', 'https', 'mailto']), disallowedTagsMode: 'discard', + nonTextTags: Object.freeze(['script', 'style', 'textarea', 'option', ...RAW_TEXT_BYPASS_TAGS]), }); +function hardenAllowlist(config) { + const allowedTags = Array.isArray(config.allowedTags) ? config.allowedTags : []; + if (allowedTags.includes('xmp')) return config; + + const nonTextTags = new Set(Array.isArray(config.nonTextTags) ? config.nonTextTags : []); + for (const tag of RAW_TEXT_BYPASS_TAGS) nonTextTags.add(tag); + + return { + ...config, + nonTextTags: [...nonTextTags], + }; +} + function loadAllowlist({ path = process.env.ALLOWLIST_FILE } = {}) { if (!path) return DEFAULT_ALLOWLIST; @@ -53,7 +69,7 @@ function loadAllowlist({ path = process.env.ALLOWLIST_FILE } = {}) { throw new Error('ALLOWLIST_FILE: "allowedSchemes" must be an array'); } - return parsed; + return hardenAllowlist(parsed); } -module.exports = { DEFAULT_ALLOWLIST, loadAllowlist }; +module.exports = { DEFAULT_ALLOWLIST, loadAllowlist, hardenAllowlist }; diff --git a/package-lock.json b/package-lock.json index b0f9893..1e90b6f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "markdown-security", - "version": "2.2.0", + "version": "2.3.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "markdown-security", - "version": "2.2.0", + "version": "2.3.0", "license": "MIT", "dependencies": { "ajv": "^8.20.0", @@ -14,7 +14,7 @@ "express-rate-limit": "^8.5.1", "pino": "^10.3.1", "pino-http": "^11.0.0", - "sanitize-html": "^2.17.3" + "sanitize-html": "^2.17.4" }, "devDependencies": { "@cyclonedx/cyclonedx-npm": "^4.2.1", @@ -2546,6 +2546,12 @@ "node": ">= 8" } }, + "node_modules/dayjs": { + "version": "1.11.20", + "resolved": "https://registry.npmjs.org/dayjs/-/dayjs-1.11.20.tgz", + "integrity": "sha512-YbwwqR/uYpeoP4pu043q+LTDLFBLApUP6VxRihdfNTqu4ubqMlGDLd6ErXhEgsyvY0K6nCs7nggYumAN+9uEuQ==", + "license": "MIT" + }, "node_modules/debug": { "version": "4.4.3", "resolved": "https://registry.npmjs.org/debug/-/debug-4.4.3.tgz", @@ -4641,6 +4647,15 @@ "node": ">=6" } }, + "node_modules/launder": { + "version": "1.7.1", + "resolved": "https://registry.npmjs.org/launder/-/launder-1.7.1.tgz", + "integrity": "sha512-mU6WRz5EusL9ZZuiZ5SO4Y6C0P9PAUR9iwdb6bzj4KDihm28DiHFw+/yk9DBH4f+Pv1wuzQ4e2jV3oQ7mkIqvw==", + "license": "MIT", + "dependencies": { + "dayjs": "^1.11.7" + } + }, "node_modules/leven": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/leven/-/leven-3.1.0.tgz", @@ -6127,15 +6142,16 @@ "license": "MIT" }, "node_modules/sanitize-html": { - "version": "2.17.3", - "resolved": "https://registry.npmjs.org/sanitize-html/-/sanitize-html-2.17.3.tgz", - "integrity": "sha512-Kn4srCAo2+wZyvCNKCSyB2g8RQ8IkX/gQs2uqoSRNu5t9I2qvUyAVvRDiFUVAiX3N3PNuwStY0eNr+ooBHVWEg==", + "version": "2.17.4", + "resolved": "https://registry.npmjs.org/sanitize-html/-/sanitize-html-2.17.4.tgz", + "integrity": "sha512-2HW7v2ol/uAM7sX4hbD8Z59OGWmAPrvjL8E71UWlBcj6m+kcF6ilQBLny+cIgY214QJeJT5tQuxKKqX0SQqjGQ==", "license": "MIT", "dependencies": { "deepmerge": "^4.2.2", "escape-string-regexp": "^4.0.0", "htmlparser2": "^10.1.0", "is-plain-object": "^5.0.0", + "launder": "^1.7.1", "parse-srcset": "^1.0.2", "postcss": "^8.3.11" } diff --git a/package.json b/package.json index 4161724..dc44a89 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "express-rate-limit": "^8.5.1", "pino": "^10.3.1", "pino-http": "^11.0.0", - "sanitize-html": "^2.17.3" + "sanitize-html": "^2.17.4" }, "devDependencies": { "@cyclonedx/cyclonedx-npm": "^4.2.1", diff --git a/tests/allowlist.test.js b/tests/allowlist.test.js index 41e32c1..714774b 100644 --- a/tests/allowlist.test.js +++ b/tests/allowlist.test.js @@ -20,6 +20,7 @@ describe("loadAllowlist", () => { expect(loadAllowlist({ path: undefined })).toBe(DEFAULT_ALLOWLIST); expect(DEFAULT_ALLOWLIST.allowedTags).toContain("p"); expect(DEFAULT_ALLOWLIST.allowedTags).not.toContain("script"); + expect(DEFAULT_ALLOWLIST.nonTextTags).toContain("xmp"); }); it("reads a JSON file when a path is provided", () => { @@ -34,6 +35,22 @@ describe("loadAllowlist", () => { const loaded = loadAllowlist({ path: file }); expect(loaded.allowedTags).toEqual(["p", "em"]); expect(loaded.disallowedTagsMode).toBe("escape"); + expect(loaded.nonTextTags).toContain("xmp"); + } finally { + removeFixture(file); + } + }); + + it("does not force xmp into nonTextTags when xmp is explicitly allowed", () => { + const file = writeFixture("xmp-allowed", { + allowedTags: ["xmp"], + allowedAttributes: {}, + disallowedTagsMode: "discard", + }); + + try { + const loaded = loadAllowlist({ path: file }); + expect(loaded.nonTextTags).toBeUndefined(); } finally { removeFixture(file); } diff --git a/tests/validation.test.js b/tests/validation.test.js index 081f038..2ed3bcc 100644 --- a/tests/validation.test.js +++ b/tests/validation.test.js @@ -163,6 +163,21 @@ describe("Markdown Validator API", () => { }); }); + describe("sanitize-html hardening", () => { + it("drops xmp raw-text payloads instead of re-emitting active HTML", async () => { + const response = await request(app) + .post("/validate") + .send({ markdown: "