From edc0ee3d00d16d381e4f8d2bcb13f19fa4eeaf49 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Wed, 7 Oct 2026 18:44:35 -0700 Subject: [PATCH] fix: HTML escaping covers attribute context escapeHtml now escapes & < > " ' (shared web/js/html-escape.js, loaded by admin, gallery and metadata pages) instead of the textContent/innerHTML round-trip that left quotes intact, so a tag, category, checkpoint or file name containing a quote can no longer break out of a data-*, value or title attribute. Inline onclick handlers that interpolated server data now carry data-action and data-* attributes and are dispatched through one delegated listener per page (web/js/data-actions.js); the img onerror fallback uses data-fallback-src with a capturing error listener. Every remaining template value written into an attribute or text node (prompts, checkpoints, file paths, log lines, directory names, categories, suggestions) is escaped. metadata.html no longer builds an "explorer /select" shell command from the file name (Electron-only branch); the path is copied to the clipboard. --- tests/js/data-actions.test.js | 49 ++++++++++ tests/js/html-escape.test.js | 56 ++++++++++++ web/admin.html | 2 + web/gallery.html | 2 + web/js/admin.js | 164 +++++++++++++++++++--------------- web/js/data-actions.js | 51 +++++++++++ web/js/gallery.js | 117 +++++++++++++----------- web/js/html-escape.js | 42 +++++++++ web/metadata.html | 58 +++++------- 9 files changed, 379 insertions(+), 162 deletions(-) create mode 100644 tests/js/data-actions.test.js create mode 100644 tests/js/html-escape.test.js create mode 100644 web/js/data-actions.js create mode 100644 web/js/html-escape.js diff --git a/tests/js/data-actions.test.js b/tests/js/data-actions.test.js new file mode 100644 index 0000000..c449801 --- /dev/null +++ b/tests/js/data-actions.test.js @@ -0,0 +1,49 @@ +// Run with: node --test tests/js/ +const test = require("node:test"); +const assert = require("node:assert/strict"); +const { readAction, dispatch } = require("../../web/js/data-actions.js"); + +test("readAction parses promptId and index as integers", () => { + const parsed = readAction({ action: "open-film", promptId: "42", index: "3" }); + assert.equal(parsed.action, "open-film"); + assert.equal(parsed.promptId, 42); + assert.equal(parsed.index, 3); +}); + +test("readAction turns missing or non-numeric ids into null", () => { + assert.equal(readAction({ action: "edit" }).promptId, null); + assert.equal(readAction({ action: "edit", promptId: "abc" }).promptId, null); + assert.equal(readAction({ action: "edit", index: "" }).index, null); +}); + +test("readAction passes string fields through untouched", () => { + const parsed = readAction({ action: "remove-tag", tag: `a "quoted" tag`, url: "/x?y=1&z=2", copyText: "p" }); + assert.equal(parsed.tag, `a "quoted" tag`); + assert.equal(parsed.url, "/x?y=1&z=2"); + assert.equal(parsed.copyText, "p"); +}); + +test("readAction tolerates a missing dataset", () => { + assert.equal(readAction(undefined).action, ""); + assert.equal(readAction(null).promptId, null); +}); + +test("dispatch calls the matching handler with the parsed action and extras", () => { + const calls = []; + const handled = dispatch( + { action: "edit", promptId: "7" }, + { edit: (a, el) => calls.push([a.promptId, el]) }, + "element", + ); + assert.equal(handled, true); + assert.deepEqual(calls, [[7, "element"]]); +}); + +test("dispatch returns false and calls nothing for an unknown or empty action", () => { + let called = 0; + const handlers = { edit: () => called++ }; + assert.equal(dispatch({ action: "other" }, handlers), false); + assert.equal(dispatch({}, handlers), false); + assert.equal(dispatch({ action: "constructor" }, handlers), false, "no prototype lookups"); + assert.equal(called, 0); +}); diff --git a/tests/js/html-escape.test.js b/tests/js/html-escape.test.js new file mode 100644 index 0000000..bdda8ad --- /dev/null +++ b/tests/js/html-escape.test.js @@ -0,0 +1,56 @@ +// Run with: node --test tests/js/ +const test = require("node:test"); +const assert = require("node:assert/strict"); +const { escapeHtml, escapeAttr } = require("../../web/js/html-escape.js"); + +test("escapes the five HTML-significant characters", () => { + assert.equal(escapeHtml(`&<>"'`), "&<>"'"); +}); + +test("a double quote cannot break out of an attribute", () => { + const payload = `" onmouseover="alert(1)`; + const escaped = escapeHtml(payload); + assert.ok(!escaped.includes(`"`), escaped); + assert.equal(escaped, `" onmouseover="alert(1)`); + assert.equal(``, ``); +}); + +test("a single quote cannot break out of a single-quoted attribute", () => { + const escaped = escapeHtml(`' onclick='alert(1)`); + assert.ok(!escaped.includes(`'`), escaped); + assert.equal(escaped, `' onclick='alert(1)`); +}); + +test("a script tag is neutralised in text context", () => { + assert.equal(escapeHtml(""), "<script>x</script>"); +}); + +test("unicode and ordinary text pass through unchanged", () => { + assert.equal(escapeHtml("héllo wörld 日本語 🙂"), "héllo wörld 日本語 🙂"); + assert.equal(escapeHtml("plain text, no specials"), "plain text, no specials"); +}); + +test("already escaped text is escaped again, never double-decoded", () => { + assert.equal(escapeHtml("&"), "&amp;"); +}); + +test("null and undefined become the empty string", () => { + assert.equal(escapeHtml(null), ""); + assert.equal(escapeHtml(undefined), ""); +}); + +test("numbers and booleans are coerced to strings", () => { + assert.equal(escapeHtml(42), "42"); + assert.equal(escapeHtml(0), "0"); + assert.equal(escapeHtml(1.5), "1.5"); + assert.equal(escapeHtml(false), "false"); +}); + +test("objects are coerced through String()", () => { + assert.equal(escapeHtml({ toString: () => "" }), "<b>"); +}); + +test("escapeAttr is the same function as escapeHtml", () => { + assert.equal(escapeAttr, escapeHtml); + assert.equal(escapeAttr(`a"b`), "a"b"); +}); diff --git a/web/admin.html b/web/admin.html index 6431ebc..efd35bb 100644 --- a/web/admin.html +++ b/web/admin.html @@ -5,6 +5,8 @@ PromptManager - Admin Dashboard + + diff --git a/web/gallery.html b/web/gallery.html index 78eb762..96bc44c 100644 --- a/web/gallery.html +++ b/web/gallery.html @@ -5,6 +5,8 @@ PromptManager - Gallery + + diff --git a/web/js/admin.js b/web/js/admin.js index 651f5cb..0846fa1 100644 --- a/web/js/admin.js +++ b/web/js/admin.js @@ -23,6 +23,23 @@ this.tagsPage = null; + // Elements rendered with data-action="..." (prompt cards, film strips, log files, + // review chips) are dispatched here instead of through inline onclick handlers, + // so user-provided strings never land inside JavaScript source. + this.actionHandlers = { + "copy-prompt": ({ promptId }) => this.copyPromptToClipboard(promptId), + "add-tags": ({ promptId }) => this.addTag(promptId), + "remove-tag": ({ promptId, tag }) => this.removeTag(promptId, tag), + "toggle-tags": ({ promptId }) => this.toggleMainTags(promptId), + gallery: ({ promptId }) => this.viewGallery(promptId), + edit: ({ promptId }) => this.editPrompt(promptId), + delete: ({ promptId }) => this.deletePrompt(promptId), + "open-film": ({ promptId, index }) => this.openFilmStripViewer(promptId, index), + "download-log": ({ filename }) => this.downloadLogFile(filename), + "copy-text": ({ copyText }) => this.copyToClipboard(copyText), + "remove-review-tag": ({ index }) => this.removeReviewTag(index), + }; + this.init(); } @@ -110,6 +127,11 @@ // Modals this.bindModalEvents(); + // One delegated click handler for every data-action element; images declare a + // data-fallback-src instead of an inline onerror handler + document.addEventListener("click", (e) => this.handleActionClick(e)); + document.addEventListener("error", (e) => this.applyImageFallback(e.target), true); + // Auto-search on filter changes ["searchCategory", "searchFolder"].forEach((id) => { document.getElementById(id).addEventListener("change", () => this.search()); @@ -118,7 +140,7 @@ // Sort dropdown: options come from PromptListSort; sorting runs on the server const sortSelect = document.getElementById("sortBy"); sortSelect.innerHTML = PromptListSort.SORT_OPTIONS - .map((o) => ``) + .map((o) => ``) .join(""); sortSelect.value = PromptListSort.DEFAULT_SORT; sortSelect.addEventListener("change", () => this.reloadPrompts()); @@ -462,6 +484,19 @@ return this.listMode === "search" ? this.search() : this.loadRecentPrompts(1); } + handleActionClick(e) { + const target = e.target.closest("[data-action]"); + if (target) DataActions.dispatch(target.dataset, this.actionHandlers, target); + } + + /** Swap a thumbnail for its full-size image once, when the thumbnail fails to load. */ + applyImageFallback(img) { + if (!img || img.tagName !== "IMG" || !img.dataset.fallbackSrc) return; + const fallback = img.dataset.fallbackSrc; + delete img.dataset.fallbackSrc; + img.src = fallback; + } + renderPrompts() { const container = document.getElementById("resultsList"); const count = document.getElementById("resultsCount"); @@ -491,15 +526,6 @@ this.addStarHoverBehavior(prompt.id); }); - // Delegated click handler for remove-tag buttons (avoids inline onclick XSS) - container.querySelectorAll('.remove-tag-btn').forEach(btn => { - btn.addEventListener('click', (e) => { - const promptId = parseInt(e.target.dataset.promptId); - const tag = e.target.dataset.tag; - window.admin.removeTag(promptId, tag); - }); - }); - // Load film strips for each prompt (async, non-blocking) this.loadAllFilmStrips(this.prompts); } @@ -550,8 +576,8 @@
-
${this.escapeHtml(prompt.text)}
-
@@ -559,15 +585,15 @@
📁 - ${this.escapeHtml(category)} + ${escapeHtml(category)}
📅 ${created}
- ${runLabel ? `
+ ${runLabel ? `
🔁 - ${this.escapeHtml(runLabel)} + ${escapeHtml(runLabel)}
` : ""}
@@ -580,11 +606,11 @@
${tags.slice(0, 10).map(tag => ` - ${this.escapeHtml(tag)} - + ${escapeHtml(tag)} + `).join("")} - @@ -594,13 +620,13 @@
${tags.slice(10).map(tag => ` - ${this.escapeHtml(tag)} - + ${escapeHtml(tag)} + `).join("")}
- @@ -614,13 +640,13 @@
- - -
@@ -676,12 +702,6 @@ }); } - escapeHtml(text) { - const div = document.createElement("div"); - div.textContent = text; - return div.innerHTML; - } - showNotification(message, type = "info") { const notification = document.createElement("div"); @@ -765,7 +785,7 @@ const data = await response.json(); const dirs = data.settings?.monitored_directories || []; if (dirs.length > 0) { - statusEl.innerHTML = dirs.map(d => `
✓ ${d}
`).join(''); + statusEl.innerHTML = dirs.map(d => `
✓ ${escapeHtml(d)}
`).join(''); } else { statusEl.textContent = 'No directories being monitored (auto-detect on restart)'; } @@ -1723,7 +1743,7 @@ html += `
- ${this.escapeHtml(category)} + ${escapeHtml(category)} ${status}
`; @@ -1737,7 +1757,7 @@ (result.status === 'warning' ? 'bg-pm-warning/20 border-pm-warning' : 'bg-pm-error-tint border-pm-error'); html += `
`; - html += `

${this.escapeHtml(category)}

`; + html += `

${escapeHtml(category)}

`; if (result.message) { html += `

${result.message}

`; @@ -1780,7 +1800,7 @@ html += `
`; for (const [dep, available] of Object.entries(result.dependencies)) { const status = available ? '✅' : '❌'; - html += `

${status} ${dep}

`; + html += `

${status} ${escapeHtml(dep)}

`; } html += `
`; } @@ -2209,13 +2229,13 @@ container.innerHTML = data.files.map(file => `
- ${file.filename} + ${escapeHtml(file.filename)} ${file.is_main ? 'Active' : ''}
Size: ${this.formatBytes(file.size)} | Modified: ${new Date(file.modified).toLocaleString()}
- @@ -2274,13 +2294,13 @@ return `
- ${timestamp.split(' ')[1]} - ${log.level} - ${log.logger} - ${log.message} + ${escapeHtml(timestamp.split(' ')[1])} + ${escapeHtml(log.level)} + ${escapeHtml(log.logger)} + ${escapeHtml(log.message)}
- ${log.filename}:${log.lineno} + ${escapeHtml(log.filename)}:${escapeHtml(log.lineno)}
`; @@ -2600,8 +2620,8 @@

File Path

-
- ${filePath} +
+ ${escapeHtml(filePath)}
@@ -2610,7 +2630,7 @@

Resources used

-
${checkpoint}
+
${escapeHtml(checkpoint)}
ComfyUI Generated
CHECKPOINT @@ -2629,7 +2649,7 @@
- ${positivePrompt.substring(0, 200)}${positivePrompt.length > 200 ? '...' : ''} + ${escapeHtml(positivePrompt.substring(0, 200))}${positivePrompt.length > 200 ? '...' : ''}
${positivePrompt.length > 200 ? '' : ''}
@@ -2645,7 +2665,7 @@
- ${negativePrompt.substring(0, 200)}${negativePrompt.length > 200 ? '...' : ''} + ${escapeHtml(negativePrompt.substring(0, 200))}${negativePrompt.length > 200 ? '...' : ''}
${negativePrompt.length > 200 ? '' : ''}
@@ -2654,12 +2674,12 @@

Other metadata

- CFG SCALE: ${cfgScale} - STEPS: ${steps} - SAMPLER: ${sampler} + CFG SCALE: ${escapeHtml(cfgScale)} + STEPS: ${escapeHtml(steps)} + SAMPLER: ${escapeHtml(sampler)}
- SEED: ${seed} + SEED: ${escapeHtml(seed)}
@@ -2783,7 +2803,7 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`; if (!this.currentMetadata) return; const prompt = type === 'positive' ? this.currentMetadata.positivePrompt : this.currentMetadata.negativePrompt; - const safeType = this.escapeHtml(type.charAt(0).toUpperCase() + type.slice(1)); + const safeType = escapeHtml(type.charAt(0).toUpperCase() + type.slice(1)); const newWindow = window.open('', '_blank'); const doc = newWindow.document; doc.open(); @@ -3058,7 +3078,7 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`;

File Path

- ${metadata.imagePath} + ${escapeHtml(metadata.imagePath)}
@@ -3067,7 +3087,7 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`;

Resources used

-
${metadata.checkpoint}
+
${escapeHtml(metadata.checkpoint)}
ComfyUI Generated
CHECKPOINT @@ -3086,7 +3106,7 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`;
- ${metadata.positivePrompt.substring(0, 200)}${metadata.positivePrompt.length > 200 ? '...' : ''} + ${escapeHtml(metadata.positivePrompt.substring(0, 200))}${metadata.positivePrompt.length > 200 ? '...' : ''}
${metadata.positivePrompt.length > 200 ? '' : ''}
@@ -3102,7 +3122,7 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`;
- ${metadata.negativePrompt.substring(0, 200)}${metadata.negativePrompt.length > 200 ? '...' : ''} + ${escapeHtml(metadata.negativePrompt.substring(0, 200))}${metadata.negativePrompt.length > 200 ? '...' : ''}
${metadata.negativePrompt.length > 200 ? '' : ''}
@@ -3111,12 +3131,12 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`;

Other metadata

- CFG SCALE: ${metadata.cfgScale} - STEPS: ${metadata.steps} - SAMPLER: ${metadata.sampler} + CFG SCALE: ${escapeHtml(metadata.cfgScale)} + STEPS: ${escapeHtml(metadata.steps)} + SAMPLER: ${escapeHtml(metadata.sampler)}
- SEED: ${metadata.seed} + SEED: ${escapeHtml(metadata.seed)}
@@ -3198,7 +3218,7 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`; showFullPrompt(type) { if (this.currentMetadata) { const prompt = type === 'positive' ? this.currentMetadata.positivePrompt : this.currentMetadata.negativePrompt; - const safeType = this.escapeHtml(type.charAt(0).toUpperCase() + type.slice(1)); + const safeType = escapeHtml(type.charAt(0).toUpperCase() + type.slice(1)); const newWindow = window.open('', '_blank'); const doc = newWindow.document; doc.open(); @@ -3370,7 +3390,7 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`; this.addPromptTags.forEach(tag => { const chip = document.createElement('span'); chip.className = 'tag-chip inline-flex items-center px-2 py-1 bg-pm-accent text-pm text-xs rounded cursor-pointer hover:bg-pm-accent-hover'; - chip.innerHTML = `${tag} ×`; + chip.innerHTML = `${escapeHtml(tag)} ×`; chip.addEventListener('click', () => { this.addPromptTags = this.addPromptTags.filter(t => t !== tag); this.renderAddPromptTags(); @@ -3397,8 +3417,8 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`; } suggestionsContainer.innerHTML = matchingTags.map(tag => ` -
- ${tag} +
+ ${escapeHtml(tag)}
`).join(''); @@ -3419,7 +3439,7 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`; populateAddPromptCategories() { const datalist = document.getElementById('addPromptCategoryList'); - datalist.innerHTML = this.categories.map(cat => `
@@ -986,9 +1009,9 @@

File Info

- ${fileName} + ${escapeHtml(fileName)}
Type: ${fileExt} Video @@ -1185,7 +1208,7 @@

File Path

- ${metadata.imagePath || 'Unknown'} + ${escapeHtml(metadata.imagePath || 'Unknown')}
@@ -1195,7 +1218,7 @@
Model: - ${metadata.checkpoint || 'Unknown'} + ${escapeHtml(metadata.checkpoint || 'Unknown')}
@@ -1237,19 +1260,19 @@
CFG SCALE: - ${metadata.cfgScale || 'Unknown'} + ${escapeHtml(metadata.cfgScale || 'Unknown')}
STEPS: - ${metadata.steps || 'Unknown'} + ${escapeHtml(metadata.steps || 'Unknown')}
SAMPLER: - ${metadata.sampler || 'Unknown'} + ${escapeHtml(metadata.sampler || 'Unknown')}
SEED: - ${metadata.seed || 'Unknown'} + ${escapeHtml(metadata.seed || 'Unknown')}
@@ -1406,7 +1429,7 @@ showFullPrompt(type) { if (this.currentMetadata) { const prompt = type === 'positive' ? this.currentMetadata.positivePrompt : this.currentMetadata.negativePrompt; - const safeType = this.escapeHtml(type.charAt(0).toUpperCase() + type.slice(1)); + const safeType = escapeHtml(type.charAt(0).toUpperCase() + type.slice(1)); const newWindow = window.open('', '_blank'); const doc = newWindow.document; doc.open(); @@ -1487,13 +1510,7 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`; if (!text || text === 'No prompt found' || text === 'No negative prompt found') { return `${text || 'No prompt found'}`; } - return this.escapeHtml(text); - } - - escapeHtml(text) { - const div = document.createElement('div'); - div.textContent = text; - return div.innerHTML; + return escapeHtml(text); } unescapeHtml(text) { @@ -2205,7 +2222,7 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`; ${image.media_type === 'video' ? `
${image.thumbnail_url ? ` - Video thumbnail + Video thumbnail ` : ` @@ -2214,26 +2231,26 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`;
VIDEO
` : ` - ${this.escapeHtml(image.filename)} `}
-
${this.escapeHtml(image.filename)}
+
${escapeHtml(image.filename)}
Size: ${this.formatFileSize(image.size)} • Modified: ${new Date(image.modified_time * 1000).toLocaleDateString()}
-
- ${image.relative_path} +
+ ${escapeHtml(image.relative_path)}
@@ -2242,7 +2259,7 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`; KEEP (Oldest) ` : ` @@ -2874,7 +2891,7 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`; populateCategoryDatalist() { const datalist = document.getElementById('categoryList'); datalist.innerHTML = this.addPromptState.allCategories - .map(cat => `
- ${negativePrompt.substring(0, 200)}${negativePrompt.length > 200 ? '...' : ''} + ${escapeHtml(negativePrompt.substring(0, 200))}${negativePrompt.length > 200 ? '...' : ''}
${negativePrompt.length > 200 ? '' : ''}
@@ -267,12 +254,12 @@ Seed: ${currentWorkflowData.seed || 'Unknown'}`;

Other metadata

- CFG SCALE: ${cfgScale} - STEPS: ${steps} - SAMPLER: ${sampler} + CFG SCALE: ${escapeHtml(cfgScale)} + STEPS: ${escapeHtml(steps)} + SAMPLER: ${escapeHtml(sampler)}
- SEED: ${seed} + SEED: ${escapeHtml(seed)}
@@ -289,6 +276,9 @@ Seed: ${currentWorkflowData.seed || 'Unknown'}`;
`; + dataPanel.querySelector('[data-action="open-file"]').addEventListener('click', (e) => { + openInFileExplorer(e.currentTarget.dataset.path); + }); // Store the full data for later use currentWorkflowData = { @@ -304,12 +294,6 @@ Seed: ${currentWorkflowData.seed || 'Unknown'}`; }; } - function escapeHtml(text) { - const div = document.createElement('div'); - div.textContent = text; - return div.innerHTML; - } - function showFullPrompt(type) { if (currentWorkflowData) { const prompt = type === 'positive' ? currentWorkflowData.positivePrompt : currentWorkflowData.negativePrompt; @@ -343,7 +327,7 @@ Seed: ${currentWorkflowData.seed || 'Unknown'}`; ComfyUI Workflow Data

ComfyUI Workflow JSON

-
${JSON.stringify(currentWorkflowData.workflow, null, 2)}
+
${escapeHtml(JSON.stringify(currentWorkflowData.workflow, null, 2))}
`);