Skip to content

Commit 66312ae

Browse files
fix: harden routing, encoding, error handling, and XSS in frontend modules
1 parent d497eb6 commit 66312ae

8 files changed

Lines changed: 66 additions & 31 deletions

File tree

static/js/app.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ import { bulkExport, downloadSession } from './export.js';
1212

1313
// ==================== Router ====================
1414

15+
function safeDecode(str) {
16+
try { return decodeURIComponent(str); } catch { return null; }
17+
}
18+
1519
function handleRoute() {
1620
if (state.navInProgress) return;
1721
window.scrollTo(0, 0);
@@ -20,7 +24,8 @@ function handleRoute() {
2024
const parts = hash.slice(9);
2125
const slashIdx = parts.indexOf('/');
2226
if (slashIdx > 0) {
23-
const project = decodeURIComponent(parts.slice(0, slashIdx));
27+
const project = safeDecode(parts.slice(0, slashIdx));
28+
if (!project) { showProjects(); return; }
2429
const sessionId = parts.slice(slashIdx + 1);
2530
if (state.currentProject === project && state.cachedSessions.length > 0 && document.getElementById('sidebar')) {
2631
document.querySelectorAll('.sidebar-item').forEach(el => el.classList.remove('active'));
@@ -31,7 +36,9 @@ function handleRoute() {
3136
showWorkspace(project, sessionId);
3237
}
3338
} else {
34-
showWorkspace(decodeURIComponent(parts));
39+
const project = safeDecode(parts);
40+
if (!project) { showProjects(); return; }
41+
showWorkspace(project);
3542
}
3643
} else if (hash === '#search') {
3744
showSearchPage();

static/js/export.js

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export async function downloadSession(project, sessionId) {
4949
const handle = await getFileHandle(fname, [{ description: 'Markdown', accept: { 'text/markdown': ['.md'] } }]);
5050
if (!handle) return;
5151
try {
52-
const res = await fetch(`/api/export/session/${encodeURIComponent(project)}/${sessionId}`);
52+
const res = await fetch(`/api/export/session/${encodeURIComponent(project)}/${encodeURIComponent(sessionId)}`);
5353
if (!res.ok) throw new Error(`Download failed: ${res.status}`);
5454
const blob = await res.blob();
5555
await writeToHandle(handle, blob, fname);
@@ -72,8 +72,13 @@ async function getFileHandle(suggestedName, fileTypes) {
7272
async function writeToHandle(handle, blob, fallbackName) {
7373
if (handle !== 'fallback') {
7474
const writable = await handle.createWritable();
75-
await writable.write(blob);
76-
await writable.close();
75+
try {
76+
await writable.write(blob);
77+
await writable.close();
78+
} catch (e) {
79+
try { await writable.abort(); } catch { /* ignore abort errors */ }
80+
throw e;
81+
}
7782
} else {
7883
const url = URL.createObjectURL(blob);
7984
const a = document.createElement('a');

static/js/projects.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ export async function showProjects() {
2424
fetch('/api/projects'),
2525
fetch('/api/export/state').catch(() => null),
2626
]);
27+
if (!projRes.ok) {
28+
let msg = `Failed to load projects (${projRes.status})`;
29+
try { const body = await projRes.json(); if (body.error) msg = body.error; } catch { try { msg = await projRes.text() || msg; } catch { /* ignore */ } }
30+
loadingBar.done();
31+
smoothSet(content, `<div class="loading"><p class="text-danger">${esc(msg)}</p></div>`);
32+
return;
33+
}
2734
const projects = await projRes.json();
2835
loadingBar.done();
2936

@@ -44,7 +51,7 @@ export async function showProjects() {
4451
const d = new Date(exportState.last_export_time);
4552
if (!isNaN(d.getTime())) {
4653
hasPreviousExport = true;
47-
const sessionCount = exportState.last_export_session_count ?? exportState.export_count ?? 0;
54+
const sessionCount = Math.max(0, parseInt(exportState.last_export_session_count ?? exportState.export_count ?? 0, 10) || 0);
4855
lastExportHtml = `<p class="text-muted text-sm" style="margin:0">Last export: ${d.toLocaleString()} (${sessionCount} sessions in last export)</p>`;
4956
}
5057
}

static/js/search.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,18 @@ export async function doSearch() {
3939

4040
try {
4141
const res = await fetch(`/api/search?q=${encodeURIComponent(query)}&limit=50`);
42+
if (!res.ok) {
43+
let msg = `Search failed (${res.status})`;
44+
try { msg = await res.text() || msg; } catch { /* ignore */ }
45+
throw new Error(msg);
46+
}
4247
const results = await res.json();
4348

4449
let html = `<p class="text-muted text-sm">${results.length} result${results.length !== 1 ? 's' : ''}</p><br>`;
4550
html += '<div class="search-results">';
4651

4752
for (const r of results) {
48-
html += `<div class="search-result" onclick="window.location.hash='#project/${encodeURIComponent(r.project)}/${r.session_id}'">
53+
html += `<div class="search-result" onclick="window.location.hash='#project/${encodeURIComponent(r.project)}/${encodeURIComponent(r.session_id)}'">
4954
<div><strong>${esc(r.title)}</strong> <span class="text-muted text-sm">${esc(r.project)} &bull; ${esc(r.role)}</span></div>
5055
<div class="snippet">...${esc(r.snippet)}...</div>
5156
</div>`;

static/js/sessions.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export async function showWorkspace(projectName, selectedSessionId) {
5454
const errorClass = s.error ? ' sidebar-item-error' : '';
5555
const errorDetail = s.error_detail ? `<div class="error-detail">${esc(s.error_detail)}</div>` : '';
5656
const modelBadge = models ? `<span style="font-size:0.65rem;opacity:0.6;display:block;margin-top:1px">${esc(models)}</span>` : '';
57-
sidebar += `<button class="sidebar-item${isActive}${errorClass}" onclick="selectSession('${esc(projectName)}','${esc(s.id)}')" id="sidebar-${s.id}">
57+
sidebar += `<button class="sidebar-item${isActive}${errorClass}" onclick="selectSession(${JSON.stringify(projectName)},${JSON.stringify(s.id)})" id="sidebar-${esc(s.id)}">
5858
<div class="sidebar-item-title">${esc(title)}</div>
5959
${errorDetail}
6060
<div class="sidebar-item-time">${esc(ts)}${modelBadge}</div>
@@ -96,7 +96,7 @@ export async function showWorkspace(projectName, selectedSessionId) {
9696

9797
export function selectSession(projectName, sessionId) {
9898
closeSidebar();
99-
window.location.hash = `#project/${encodeURIComponent(projectName)}/${sessionId}`;
99+
window.location.hash = `#project/${encodeURIComponent(projectName)}/${encodeURIComponent(sessionId)}`;
100100
}
101101

102102
export async function loadSession(projectName, sessionId) {
@@ -105,7 +105,7 @@ export async function loadSession(projectName, sessionId) {
105105
loadingBar.start();
106106

107107
try {
108-
const res = await fetch(`/api/sessions/${encodeURIComponent(projectName)}/${sessionId}`);
108+
const res = await fetch(`/api/sessions/${encodeURIComponent(projectName)}/${encodeURIComponent(sessionId)}`);
109109
if (!res.ok) {
110110
loadingBar.done();
111111
const err = await res.json().catch(() => ({ error: `HTTP ${res.status}` }));
@@ -151,7 +151,7 @@ export async function loadSession(projectName, sessionId) {
151151
<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2"><rect x="9" y="9" width="13" height="13" rx="2" ry="2"/><path d="M5 15H4a2 2 0 0 1-2-2V4a2 2 0 0 1 2-2h9a2 2 0 0 1 2 2v1"/></svg>
152152
Copy All
153153
</button>
154-
<button class="btn btn-outline btn-sm" onclick="downloadSession('${esc(projectName)}','${sessionId}')">
154+
<button class="btn btn-outline btn-sm" onclick="downloadSession(${JSON.stringify(projectName)},${JSON.stringify(sessionId)})">
155155
<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2"><path d="M21 15v4a2 2 0 0 1-2 2H5a2 2 0 0 1-2-2v-4"/><polyline points="7 10 12 15 17 10"/><line x1="12" y1="15" x2="12" y2="3"/></svg>
156156
Download
157157
</button>
@@ -392,8 +392,8 @@ function renderToolResult(parsed) {
392392
}
393393

394394
export function copyAll() {
395-
const msgs = document.querySelector('.messages-container');
396-
if (!msgs) return;
397-
const text = msgs.innerText;
395+
const sessionEl = document.querySelector('.session-content-inner') || document.querySelector('#session-content');
396+
if (!sessionEl) return;
397+
const text = sessionEl.innerText;
398398
navigator.clipboard.writeText(text).then(() => showToast('Copied to clipboard', 'success'));
399399
}

static/js/shared/markdown.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,7 @@ export function renderMarkdown(text) {
3939
if (typeof DOMPurify !== 'undefined') {
4040
return DOMPurify.sanitize(parsed);
4141
}
42-
// DOMPurify not yet loaded — return parsed but log a warning
43-
console.warn('[renderMarkdown] DOMPurify not available; output is unsanitized');
44-
return parsed;
42+
throw new Error('DOMPurify not available');
4543
} catch (e) { /* fall through to escaped fallback */ }
4644
}
4745
// Fallback: fully escaped output with basic code block conversion (no DOMPurify needed)

static/js/shared/utils.js

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export function truncate(s, max) {
1313
export function formatTs(ts) {
1414
try {
1515
const d = new Date(ts);
16+
if (isNaN(d.getTime())) return ts;
1617
const mm = String(d.getUTCMonth() + 1).padStart(2, '0');
1718
const dd = String(d.getUTCDate()).padStart(2, '0');
1819
const yyyy = d.getUTCFullYear();
@@ -29,6 +30,7 @@ export function formatTs(ts) {
2930
export function formatDate(ts) {
3031
try {
3132
const d = new Date(ts);
33+
if (isNaN(d.getTime())) return ts ? ts.slice(0, 10) : '';
3234
const mm = String(d.getUTCMonth() + 1).padStart(2, '0');
3335
const dd = String(d.getUTCDate()).padStart(2, '0');
3436
const yyyy = d.getUTCFullYear();
@@ -37,7 +39,7 @@ export function formatDate(ts) {
3739
}
3840

3941
export function formatSize(bytes) {
40-
if (!bytes) return '?';
42+
if (bytes == null) return '?';
4143
if (bytes < 1024) return bytes + ' B';
4244
if (bytes < 1048576) return (bytes / 1024).toFixed(1) + ' KB';
4345
return (bytes / 1048576).toFixed(1) + ' MB';
@@ -59,9 +61,20 @@ export function showToast(message, type = 'info') {
5961
const icons = { success: '\u2713', error: '\u2717', info: '\u2139' };
6062
const toast = document.createElement('div');
6163
toast.className = `toast toast-${type}`;
62-
toast.innerHTML = `<span class="toast-icon">${icons[type] || icons.info}</span><span class="toast-text">${message}</span><button class="toast-close">\u00d7</button><div class="toast-progress"></div>`;
64+
const iconSpan = document.createElement('span');
65+
iconSpan.className = 'toast-icon';
66+
iconSpan.textContent = icons[type] || icons.info;
67+
const textSpan = document.createElement('span');
68+
textSpan.className = 'toast-text';
69+
textSpan.textContent = message;
70+
const closeBtn = document.createElement('button');
71+
closeBtn.className = 'toast-close';
72+
closeBtn.textContent = '\u00d7';
73+
const progress = document.createElement('div');
74+
progress.className = 'toast-progress';
75+
toast.append(iconSpan, textSpan, closeBtn, progress);
6376
document.body.appendChild(toast);
64-
toast.querySelector('.toast-close').addEventListener('click', () => { toast.classList.remove('show'); setTimeout(() => toast.remove(), 300); });
77+
closeBtn.addEventListener('click', () => { toast.classList.remove('show'); setTimeout(() => toast.remove(), 300); });
6578
requestAnimationFrame(() => toast.classList.add('show'));
6679
setTimeout(() => {
6780
toast.classList.remove('show');
@@ -74,16 +87,16 @@ export function showConfirm(message, onConfirm) {
7487
overlay.className = 'confirm-overlay';
7588
const dialog = document.createElement('div');
7689
dialog.className = 'confirm-dialog';
77-
dialog.innerHTML = `
78-
<div class="confirm-header">
79-
<span class="confirm-icon">?</span>
80-
<span class="confirm-title">Confirm Action</span>
81-
</div>
82-
<p class="confirm-message">${message}</p>
83-
<div class="confirm-actions">
84-
<button class="confirm-btn confirm-cancel">Cancel</button>
85-
<button class="confirm-btn confirm-ok">Confirm</button>
86-
</div>`;
90+
const header = document.createElement('div');
91+
header.className = 'confirm-header';
92+
header.innerHTML = '<span class="confirm-icon">?</span><span class="confirm-title">Confirm Action</span>';
93+
const msgEl = document.createElement('p');
94+
msgEl.className = 'confirm-message';
95+
msgEl.textContent = message;
96+
const actions = document.createElement('div');
97+
actions.className = 'confirm-actions';
98+
actions.innerHTML = '<button class="confirm-btn confirm-cancel">Cancel</button><button class="confirm-btn confirm-ok">Confirm</button>';
99+
dialog.append(header, msgEl, actions);
87100
overlay.appendChild(dialog);
88101
document.body.appendChild(overlay);
89102
requestAnimationFrame(() => overlay.classList.add('show'));

tests/test_xss_sanitization.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ def test_only_shared_markdown_calls_marked_parse(self):
114114
assert not violations, (
115115
"The following JS files call marked.parse() directly (issue #295). "
116116
"All markdown rendering must go through renderMarkdown() in "
117-
f"shared/markdown.js:\n " + "\n ".join(violations)
117+
"shared/markdown.js:\n " + "\n ".join(violations)
118118
)
119119

120120
def test_no_raw_marked_parse_to_inner_html(self):

0 commit comments

Comments
 (0)