From 18388175d7ec8bc81629728b9c43ef121d62a1b1 Mon Sep 17 00:00:00 2001 From: Uttam Deb Date: Sun, 7 Jun 2026 12:21:49 +0600 Subject: [PATCH] Sanitize UI & escaping; persist edits; API tweak Add input/output sanitization and escaping across draft and review UIs, improve status class handling, and make edit persistence more robust. Changes include: - Backend: refine_explanation now returns a merged refined_question with detailed_explanation and model/timestamp metadata. - UI: store pipeline config on load for DraftController use. - Add escapeHtml and sanitizeRenderedHtml helpers to prevent XSS and strip dangerous attributes/tags from rendered HTML/Markdown. - Escape IDs, filenames, status messages and other user-provided strings when injecting HTML to DOM. - Normalize status-based class names via statusClassName to avoid invalid CSS classes. - DraftController: derive paperCode from filename, use pipelineConfig default subject, and ensure safe IDs in generated cards and action buttons. - ReviewController: escape editable fields when rendering, sanitize Markdown output before inserting into DOM, and sanitize preview content. - saveCurrentEdits: changed to support force saves, guard against missing draft data, and only save when needed. - Publish/refine flows now await saves; acceptAll/rejectCurrentQuestion made async and properly await publish/save operations. refineQuestion handling updated to accept either a full refined_question or fallback to updating generated_content.detailed_explanation. These changes aim to harden the UI against XSS, ensure consistent class names, and make the save/publish/refine flows more reliable. --- qc_viewer/routers/automation.py | 10 ++ qc_viewer/static/js/automate_ui.js | 1 + qc_viewer/static/js/controllers/drafts.js | 52 ++++++---- qc_viewer/static/js/controllers/review.js | 113 +++++++++++++++------- 4 files changed, 123 insertions(+), 53 deletions(-) diff --git a/qc_viewer/routers/automation.py b/qc_viewer/routers/automation.py index 2ebb9a4..5bcea0d 100644 --- a/qc_viewer/routers/automation.py +++ b/qc_viewer/routers/automation.py @@ -355,8 +355,18 @@ async def refine_explanation(request: RefineRequest): # For refinement, we want a more capable model routing_engine.config.model_routing.generation = "openai/gpt-4o" refined = routing_engine.generate_content(prompt, task_type="generation") + refined_question = dict(request.original_q) + original_generated = refined_question.get("generated_content") + generated_content = ( + dict(original_generated) + if isinstance(original_generated, dict) + else {} + ) + generated_content["detailed_explanation"] = refined + refined_question["generated_content"] = generated_content return { "explanation": refined, + "refined_question": refined_question, "model": routing_engine.config.model_routing.generation, "timestamp": datetime.now().isoformat(), } diff --git a/qc_viewer/static/js/automate_ui.js b/qc_viewer/static/js/automate_ui.js index 0258036..d206ee7 100644 --- a/qc_viewer/static/js/automate_ui.js +++ b/qc_viewer/static/js/automate_ui.js @@ -31,6 +31,7 @@ export const AutomationUI = { const resp = await fetch('/api/automate/config'); if (!resp.ok) throw new Error('Failed to fetch config'); const config = await resp.json(); + this.pipelineConfig = config; if (config.workspace) { const curSelect = document.getElementById('curriculumSelect'); diff --git a/qc_viewer/static/js/controllers/drafts.js b/qc_viewer/static/js/controllers/drafts.js index f2c4e75..fb18f4e 100644 --- a/qc_viewer/static/js/controllers/drafts.js +++ b/qc_viewer/static/js/controllers/drafts.js @@ -1,5 +1,17 @@ import { AutomationAPI } from '../automate_api.js'; +const escapeHtml = (value) => String(value ?? '').replace(/[&<>"']/g, (char) => ({ + '&': '&', + '<': '<', + '>': '>', + '"': '"', + "'": ''' +}[char])); + +const statusClassName = (status) => String(status || 'unknown') + .toLowerCase() + .replace(/[^a-z0-9_-]/g, '') || 'unknown'; + export const DraftController = { activeStreams: {}, @@ -12,8 +24,8 @@ export const DraftController = { text.textContent = `Uploading ${file.name}...`; try { - const subject = document.getElementById('curriculumSelect')?.value || 'General'; - const paperCode = document.getElementById('targetTableSelect')?.value || ''; + const subject = this.pipelineConfig?.workspace?.default_subject || 'General'; + const paperCode = file.name.replace(/\.[^/.]+$/, '').trim() || file.name; // Upload the file const result = await AutomationAPI.uploadPDF(file, subject, paperCode); @@ -27,9 +39,12 @@ export const DraftController = { // IMMEDIATE HANDOVER: Inject the card right away without waiting for any fetches if (result && result.id) { + const safeResultId = escapeHtml(result.id); const handoverState = { id: result.id, filename: file.name, + subject, + paper_code: paperCode, status: 'PROCESSING', progress: 10, status_message: 'Initializing AI pipeline...', @@ -48,10 +63,10 @@ export const DraftController = { // Prepend the new card HTML const dateStr = 'Just now'; const newCardHTML = ` -
+
-

${file.name}

+

${escapeHtml(file.name)}

${dateStr} @@ -67,9 +82,9 @@ export const DraftController = {

- +
- +
`; @@ -127,6 +142,7 @@ export const DraftController = { let hasProcessing = false; draftList.innerHTML = drafts.map(d => { const status = d.status || 'UNKNOWN'; + const statusClass = statusClassName(status); const isProcessing = status === 'PROCESSING' || status === 'EXTRACTING'; if (isProcessing) hasProcessing = true; @@ -150,15 +166,15 @@ export const DraftController = { const statusMsg = d.status_message || (isProcessing ? 'Processing...' : ''); return ` -
+
-

${d.filename || 'Untitled Document'}

+

${escapeHtml(d.filename || 'Untitled Document')}

${qCount > 0 ? `${qCount} Questions` : ''}

- ${dateStr}${reviewedStr}${timeStr} - ${statusMsg ? ` • ${statusMsg}` : ''} + ${escapeHtml(`${dateStr}${reviewedStr}${timeStr}`)} + ${statusMsg ? ` • ${escapeHtml(statusMsg)}` : ''}

${isProcessing ? `
@@ -168,10 +184,10 @@ export const DraftController = { ` : ''}
- ${status} + ${escapeHtml(status)}
${this.renderActionButton(d)} - +
@@ -183,7 +199,7 @@ export const DraftController = { if (e.target.closest('.action-buttons')) return; const id = card.dataset.id; const d = drafts.find(x => x.id === id); - if (d.status === 'PROCESSED' || d.status === 'REVIEW_READY') this.openReview(id); + if (d && (d.status === 'PROCESSED' || d.status === 'REVIEW_READY')) this.openReview(id); }; }); @@ -291,7 +307,7 @@ export const DraftController = { const badgeEl = card.querySelector('.status-badge'); if (badgeEl) { badgeEl.textContent = status; - badgeEl.className = `status-badge status-${String(status).toLowerCase()}`; + badgeEl.className = `status-badge status-${statusClassName(status)}`; } const mini = card.querySelector('.mini-progress-container'); const pctEl = card.querySelector('.draft-progress-pct') @@ -302,7 +318,7 @@ export const DraftController = { const actions = card.querySelector('.action-buttons'); if (actions && typeof this.renderActionButton === 'function') { const id = d.id; - actions.innerHTML = `${this.renderActionButton(d)}`; + actions.innerHTML = `${this.renderActionButton(d)}`; actions.querySelector('.btn-delete')?.addEventListener('click', (e) => { e.stopPropagation(); this.deleteDraft(id); @@ -334,8 +350,8 @@ export const DraftController = { renderActionButton(d) { if (d.status === 'PROCESSED' || d.status === 'REVIEW_READY') { return `
- - + +
`; } else if (d.status === 'FAILED') { return `Error`; @@ -343,7 +359,7 @@ export const DraftController = { return `
- +
`; } else { diff --git a/qc_viewer/static/js/controllers/review.js b/qc_viewer/static/js/controllers/review.js index 555373d..0d74329 100644 --- a/qc_viewer/static/js/controllers/review.js +++ b/qc_viewer/static/js/controllers/review.js @@ -1,5 +1,36 @@ import { AutomationAPI } from '../automate_api.js'; +const escapeHtml = (value) => String(value ?? '').replace(/[&<>"']/g, (char) => ({ + '&': '&', + '<': '<', + '>': '>', + '"': '"', + "'": ''' +}[char])); + +const sanitizeRenderedHtml = (html) => { + const template = document.createElement('template'); + template.innerHTML = html; + template.content + .querySelectorAll('script, iframe, object, embed, link, meta, style') + .forEach((node) => node.remove()); + template.content.querySelectorAll('*').forEach((node) => { + [...node.attributes].forEach((attr) => { + const name = attr.name.toLowerCase(); + const value = attr.value.trim().toLowerCase(); + if ( + name.startsWith('on') || + name === 'style' || + ((name === 'href' || name === 'src' || name === 'xlink:href') && + (value.startsWith('javascript:') || value.startsWith('data:text/html'))) + ) { + node.removeAttribute(attr.name); + } + }); + }); + return template.innerHTML; +}; + export const ReviewController = { async openReview(id) { try { @@ -26,7 +57,7 @@ export const ReviewController = {
Q${q.question_number}${(q.extraction_warnings && q.extraction_warnings.length) ? '⚠' : ''}: - ${q.text.substring(0, 50)}... + ${escapeHtml(String(q.text || '').substring(0, 50))}...
${q.status !== 'INJECTED' ? ` @@ -81,7 +112,7 @@ export const ReviewController = {
${this.renderImageWrapper(q[`option_${opt}_diagram_base64`], `option_${opt}_diagram_base64`, true)} - +
`).join('')} @@ -94,7 +125,7 @@ export const ReviewController = {
🛡️ High-Integrity Assessment: AI Critique
- +
@@ -115,7 +146,7 @@ export const ReviewController = {
${Object.entries(hia.viva_probes || {}).map(([stage, probe]) => `
-

${stage}

+

${escapeHtml(stage)}

`).join('')} @@ -128,7 +159,7 @@ export const ReviewController = { ${qType !== 'mcq' ? warnBanner : ''}
- +
@@ -138,13 +169,13 @@ export const ReviewController = {
- +
- +
`; @@ -220,7 +251,7 @@ export const ReviewController = { // Convert Markdown to HTML try { - const html = marked.parse(text || ''); + const html = sanitizeRenderedHtml(marked.parse(text || '')); container.innerHTML = html; // Trigger MathJax typesetting @@ -247,25 +278,31 @@ export const ReviewController = { } }, - async saveCurrentEdits(manual = false) { - if (this.currentQuestionIndex === null || !this.isDirty) return; - const q = this.currentDraftData.questions[this.currentQuestionIndex]; + async saveCurrentEdits(manual = false, force = false) { + if (!this.currentDraftData?.id || (!this.isDirty && !force)) return; + + if (this.currentQuestionIndex !== null) { + const q = this.currentDraftData.questions[this.currentQuestionIndex]; - q.text = document.getElementById('edit-text').value; - const optA = document.getElementById('edit-opt-A'); - const optB = document.getElementById('edit-opt-B'); - const optC = document.getElementById('edit-opt-C'); - const optD = document.getElementById('edit-opt-D'); - if (optA && optB && optC && optD && q.options) { - q.options.A = optA.value; - q.options.B = optB.value; - q.options.C = optC.value; - q.options.D = optD.value; - } + const textEl = document.getElementById('edit-text'); + if (textEl) q.text = textEl.value; + const optA = document.getElementById('edit-opt-A'); + const optB = document.getElementById('edit-opt-B'); + const optC = document.getElementById('edit-opt-C'); + const optD = document.getElementById('edit-opt-D'); + if (optA && optB && optC && optD && q.options) { + q.options.A = optA.value; + q.options.B = optB.value; + q.options.C = optC.value; + q.options.D = optD.value; + } - if (!q.generated_content) q.generated_content = {}; - q.generated_content.core_concept = document.getElementById('edit-core-concept').value; - q.generated_content.detailed_explanation = document.getElementById('edit-explanation').value; + if (!q.generated_content) q.generated_content = {}; + const coreEl = document.getElementById('edit-core-concept'); + const expEl = document.getElementById('edit-explanation'); + if (coreEl) q.generated_content.core_concept = coreEl.value; + if (expEl) q.generated_content.detailed_explanation = expEl.value; + } const statusEl = document.getElementById('saveStatus'); if (statusEl) statusEl.textContent = 'Saving changes...'; @@ -290,7 +327,7 @@ export const ReviewController = { }, async publishQuestion(index) { - this.saveCurrentEdits(); + await this.saveCurrentEdits(); const q = this.currentDraftData.questions[index]; const gen = q.generated_content || {}; const tableName = document.getElementById('targetTableSelect').value; @@ -325,26 +362,26 @@ export const ReviewController = { await AutomationAPI.publishQuestion(payload); q.status = 'INJECTED'; this.showToast(`✅ Injected Q${q.question_number} to ${tableName}`); - this.saveCurrentEdits(); + await this.saveCurrentEdits(false, true); this.renderReviewList(); } catch (error) { this.showToast('❌ Injection failed', 'danger'); } }, - acceptAll() { + async acceptAll() { if (!confirm('Inject all processed questions into the production database?')) return; - this.currentDraftData.questions.forEach((q, i) => { + for (const [i, q] of this.currentDraftData.questions.entries()) { if (q.status !== 'INJECTED' && q.status !== 'REJECTED') { - this.publishQuestion(i); + await this.publishQuestion(i); } - }); + } }, - rejectCurrentQuestion() { + async rejectCurrentQuestion() { if (this.currentQuestionIndex === null) return; const q = this.currentDraftData.questions[this.currentQuestionIndex]; q.status = 'REJECTED'; this.showToast(`🚫 Q${q.question_number} rejected`); - this.saveCurrentEdits(); + await this.saveCurrentEdits(false, true); this.renderReviewList(); }, @@ -364,10 +401,16 @@ export const ReviewController = { try { const result = await AutomationAPI.refineQuestion({ feedback, original_q: q }); - this.currentDraftData.questions[this.currentQuestionIndex] = result.refined_question; + if (result.refined_question) { + this.currentDraftData.questions[this.currentQuestionIndex] = result.refined_question; + } else { + if (!q.generated_content) q.generated_content = {}; + q.generated_content.detailed_explanation = result.explanation || q.generated_content.detailed_explanation || ''; + } this.previewQuestion(this.currentQuestionIndex); + this.setDirty(true); + await this.saveCurrentEdits(false, true); this.showToast('✨ Content refined successfully'); - this.saveCurrentEdits(); } catch (e) { this.showToast('Refine failed', 'danger'); } },