Skip to content

Commit a364675

Browse files
committed
Fix: Add concurrency lock in Reflection Widget sendMessage (#6171)
1 parent 6459fba commit a364675

File tree

1 file changed

+74
-10
lines changed

1 file changed

+74
-10
lines changed

js/widgets/reflection.js

Lines changed: 74 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,6 @@ class ReflectionMatrix {
296296
async updateProjectCode() {
297297
const code = await this.activity.prepareExport();
298298
if (code === this.code) {
299-
console.log("No changes in code detected.");
300299
return; // No changes in code
301300
}
302301

@@ -308,8 +307,6 @@ class ReflectionMatrix {
308307
if (data.algorithm !== "unchanged") {
309308
this.projectAlgorithm = data.algorithm; // update algorithm
310309
this.code = code;
311-
} else {
312-
console.log("No changes in algorithm detected.");
313310
}
314311
this.botReplyDiv(data, false, false);
315312
} else {
@@ -416,7 +413,6 @@ class ReflectionMatrix {
416413
*/
417414
async generateAnalysis() {
418415
try {
419-
console.log("Summary stored", this.summary);
420416
const response = await fetch(`${this.PORT}/analysis`, {
421417
method: "POST",
422418
headers: { "Content-Type": "application/json" },
@@ -500,6 +496,7 @@ class ReflectionMatrix {
500496
* @returns {void}
501497
*/
502498
sendMessage() {
499+
if (this.typingDiv) return;
503500
const text = this.input.value.trim();
504501
if (text === "") return;
505502
this.chatHistory.push({
@@ -575,8 +572,11 @@ class ReflectionMatrix {
575572
*/
576573
saveReport(data) {
577574
const key = "musicblocks_analysis";
578-
localStorage.setItem(key, data.response);
579-
console.log("Conversation saved in localStorage.");
575+
try {
576+
localStorage.setItem(key, data.response);
577+
} catch (e) {
578+
console.warn("Could not save analysis report to localStorage:", e);
579+
}
580580
}
581581

582582
/** Reads the analysis report from localStorage.
@@ -615,13 +615,76 @@ class ReflectionMatrix {
615615
URL.revokeObjectURL(url);
616616
}
617617

618+
/**
619+
* Escapes HTML special characters to prevent XSS attacks.
620+
* @param {string} text - The text to escape.
621+
* @returns {string} - The escaped text.
622+
*/
623+
escapeHTML(text) {
624+
const escapeMap = {
625+
"&": "&",
626+
"<": "&lt;",
627+
">": "&gt;",
628+
'"': "&quot;",
629+
"'": "&#x27;"
630+
};
631+
return text.replace(/[&<>"']/g, char => escapeMap[char]);
632+
}
633+
634+
/**
635+
* Sanitizes HTML content using DOMParser to prevent XSS.
636+
* Removes unsafe attributes and ensures links are safe.
637+
* @param {string} htmlString - The HTML string to sanitize.
638+
* @returns {string} - The sanitized HTML string.
639+
*/
640+
sanitizeHTML(htmlString) {
641+
const parser = new DOMParser();
642+
const doc = parser.parseFromString(htmlString, "text/html");
643+
644+
// Sanitize links
645+
const links = doc.getElementsByTagName("a");
646+
for (let i = 0; i < links.length; i++) {
647+
const link = links[i];
648+
const href = link.getAttribute("href");
649+
650+
// If no href, or it's unsafe, remove the attribute
651+
if (!href || this.isUnsafeUrl(href)) {
652+
link.removeAttribute("href");
653+
} else {
654+
// Enforce security attributes for external links
655+
link.setAttribute("target", "_blank");
656+
link.setAttribute("rel", "noopener noreferrer");
657+
}
658+
}
659+
660+
return doc.body.innerHTML;
661+
}
662+
663+
/**
664+
* Checks if a URL is unsafe (javascript:, data:, vbscript:).
665+
* @param {string} url - The URL to check.
666+
* @returns {boolean} - True if unsafe, false otherwise.
667+
*/
668+
isUnsafeUrl(url) {
669+
const trimmed = url.trim().toLowerCase();
670+
const unsafeSchemes = ["javascript:", "data:", "vbscript:"];
671+
// Check if it starts with any unsafe scheme
672+
// Note: DOMParser handles HTML entity decoding, so we check the raw attribute safely here
673+
// But for extra safety against control characters, we rely on the fact that
674+
// we are operating on the parsed DOM attribute.
675+
return unsafeSchemes.some(scheme => trimmed.replace(/\s+/g, "").startsWith(scheme));
676+
}
677+
618678
/**
619679
* Converts Markdown text to HTML.
620680
* @param {string} md - The Markdown text.
621681
* @returns {string} - The converted HTML text.
622682
*/
623683
mdToHTML(md) {
624-
let html = md;
684+
// Step 1: Escape HTML first to prevent XSS attacks from raw tags
685+
let html = this.escapeHTML(md);
686+
687+
// Step 2: Convert Markdown syntax to HTML
625688

626689
// Headings
627690
html = html.replace(/^###### (.*$)/gim, "<h6>$1</h6>");
@@ -635,12 +698,13 @@ class ReflectionMatrix {
635698
html = html.replace(/\*\*(.*?)\*\*/gim, "<b>$1</b>");
636699
html = html.replace(/\*(.*?)\*/gim, "<i>$1</i>");
637700

638-
// Links
639-
html = html.replace(/\[(.*?)\]\((.*?)\)/gim, "<a href='$2' target='_blank'>$1</a>");
701+
// Links - Create raw anchor tags, sanitization happens in Step 3
702+
html = html.replace(/\[(.*?)\]\((.*?)\)/gim, "<a href='$2'>$1</a>");
640703

641704
// Line breaks
642705
html = html.replace(/\n/gim, "<br>");
643706

644-
return html.trim();
707+
// Step 3: Sanitize the generated HTML using DOMParser
708+
return this.sanitizeHTML(html);
645709
}
646710
}

0 commit comments

Comments
 (0)