release(v1.9.5): harden folder tree DOM, add a11y to “Load more”, and guard folder paths

This commit is contained in:
Ryan
2025-11-13 05:32:33 -05:00
committed by GitHub
parent 01cfa597b9
commit 8cdff954d5
2 changed files with 104 additions and 42 deletions

View File

@@ -1,5 +1,27 @@
# Changelog # Changelog
## Changes 11/13/2025 (v1.9.5)
release(v1.9.5): harden folder tree DOM, add a11y to “Load more”, and guard folder paths
- Replace innerHTML-based row construction in folderManager.js with safe DOM APIs
(createElement, textContent, dataset). All user-derived strings now use
textContent; only locally-generated SVG remains via innerHTML.
- Add isSafeFolderPath() client-side guard; fail closed on suspicious paths
before rendering clickable nodes.
- “Load more” button rebuilt with proper a11y:
- aria-label, optional aria-controls to the UL
- aria-busy + disabled during fetch; restore state only if the node is still
present (Node.isConnected).
- Keep lazy tree + cursor pagination behavior intact; chevrons/icons continue to
hydrate from server hints (hasSubfolders/nonEmpty) once available.
- Addresses CodeQL XSS findings by removing unsafe HTML interpolation and
avoiding HTML interpretation of extracted text.
No breaking changes; security + UX polish on top of v1.9.4.
---
## Changes 11/13/2025 (v1.9.4) ## Changes 11/13/2025 (v1.9.4)
release(v1.9.4): lazy folder tree, cursor pagination, ACL-safe chevrons, and “Load more” (closes #66) release(v1.9.4): lazy folder tree, cursor pagination, ACL-safe chevrons, and “Load more” (closes #66)

View File

@@ -805,25 +805,37 @@ async function ensureChildrenLoaded(folder, ulEl) {
if (nextCursor && !moreLi) { if (nextCursor && !moreLi) {
moreLi = document.createElement('li'); moreLi = document.createElement('li');
moreLi.className = 'load-more'; moreLi.className = 'load-more';
moreLi.innerHTML = `<button type="button" class="btn btn-ghost">${t('load_more') || 'Load more'}</button>`;
moreLi.querySelector('button')?.addEventListener('click', async (e) => { const btn = document.createElement('button');
const btn = e.currentTarget; btn.type = 'button';
const prev = btn.textContent; btn.className = 'btn btn-ghost';
btn.disabled = true; btn.textContent = t('load_more') || 'Load more';
btn.setAttribute('aria-busy', 'true'); btn.setAttribute('aria-label', t('load_more') || 'Load more');
btn.textContent = (t('loading') || 'Loading…'); if (ulEl.id) btn.setAttribute('aria-controls', ulEl.id);
btn.addEventListener('click', async (e) => {
const b = e.currentTarget;
const prevText = b.textContent;
b.disabled = true;
b.setAttribute('aria-busy', 'true');
b.textContent = t('loading') || 'Loading…';
try { try {
await loadMoreChildren(folder, ulEl, moreLi); await loadMoreChildren(folder, ulEl, moreLi);
} finally { } finally {
// If moreLi still exists (not removed because we reached the end), restore // If the "load more" node still exists (wasn't removed because we reached end),
if (document.body.contains(moreLi)) { // restore the button state.
btn.disabled = false; if (moreLi.isConnected) {
btn.removeAttribute('aria-busy'); b.disabled = false;
btn.textContent = (t('load_more') || 'Load more'); b.removeAttribute('aria-busy');
b.textContent = t('load_more') || 'Load more';
} }
} }
}); });
moreLi.appendChild(btn);
ulEl.appendChild(moreLi); ulEl.appendChild(moreLi);
} else if (!nextCursor && moreLi) { } else if (!nextCursor && moreLi) {
moreLi.remove(); moreLi.remove();
} }
@@ -1000,53 +1012,81 @@ export function openColorFolderModal(folder) {
/* ---------------------- /* ----------------------
DOM builders & DnD DOM builders & DnD
----------------------*/ ----------------------*/
function isSafeFolderPath(p) {
// Client-side defense-in-depth; server already enforces safe segments.
// Allows letters/numbers/space/_-. and slashes between segments.
return /^(root|[A-Za-z0-9][A-Za-z0-9 _\-.]*)(\/[A-Za-z0-9][A-Za-z0-9 _\-.]*)*$/.test(String(p || ''));
}
function makeChildLi(parentPath, item) { function makeChildLi(parentPath, item) {
const it = normalizeItem(item); const it = normalizeItem(item);
if (!it) return document.createElement('li'); if (!it) return document.createElement('li');
const { name, locked } = it; const { name, locked } = it;
const fullPath = parentPath === 'root' ? name : `${parentPath}/${name}`; const fullPath = parentPath === 'root' ? name : `${parentPath}/${name}`;
if (!isSafeFolderPath(fullPath)) {
// Fail closed if something looks odd; dont render a clickable node.
return document.createElement('li');
}
// <li class="folder-item" role="treeitem" aria-expanded="false">
const li = document.createElement('li'); const li = document.createElement('li');
li.className = 'folder-item'; li.className = 'folder-item';
li.setAttribute('role', 'treeitem'); li.setAttribute('role', 'treeitem');
li.setAttribute('aria-expanded', 'false'); li.setAttribute('aria-expanded', 'false');
const optClass = 'folder-option' + (locked ? ' locked' : ''); // <div class="folder-row">
li.innerHTML = ` const row = document.createElement('div');
<div class="folder-row"> row.className = 'folder-row';
<span class="folder-spacer" aria-hidden="true"></span>
<span class="${optClass}" ${locked ? '' : 'draggable="true"'} data-folder="${fullPath}">
<span class="folder-icon" aria-hidden="true" data-kind="empty">${folderSVG('empty', { locked })}</span>
<span class="folder-label">${escapeHTML(name)}</span>
</span>
</div>
<ul class="folder-tree collapsed" role="group"></ul>
`;
const opt = li.querySelector('.folder-option'); // <span class="folder-spacer" aria-hidden="true"></span>
const ul = li.querySelector(':scope > ul.folder-tree'); const spacer = document.createElement('span');
spacer.className = 'folder-spacer';
spacer.setAttribute('aria-hidden', 'true');
// If server told us about subfolders or non-empty, apply immediately // <span class="folder-option[ locked]" [draggable]>
if (typeof item.hasSubfolders === 'boolean') { const opt = document.createElement('span');
updateToggleForOption(fullPath, item.hasSubfolders); opt.className = 'folder-option' + (locked ? ' locked' : '');
// Also stash on DOM so primeChildToggles can see it without refetch if (!locked) opt.setAttribute('draggable', 'true');
opt.dataset.hasSubs = item.hasSubfolders ? '1' : '0'; // Use dataset instead of attribute string interpolation.
} opt.dataset.folder = fullPath;
if (typeof item.nonEmpty === 'boolean') {
setFolderIconForOption(opt, item.nonEmpty ? 'paper' : 'empty');
opt.dataset.nonEmpty = item.nonEmpty ? '1' : '0';
}
// <span class="folder-icon" aria-hidden="true" data-kind="empty">[svg]</span>
const icon = document.createElement('span');
icon.className = 'folder-icon';
icon.setAttribute('aria-hidden', 'true');
icon.dataset.kind = 'empty';
// Safe: SVG is generated locally, not from user input.
// nosemgrep: javascript.browser.security.dom-xss.innerhtml
icon.innerHTML = folderSVG('empty', { locked });
// <span class="folder-label">name</span>
const label = document.createElement('span');
label.className = 'folder-label';
// Critical: never innerHTML here — textContent avoids XSS.
label.textContent = name;
opt.append(icon, label);
row.append(spacer, opt);
li.append(row);
// <ul class="folder-tree collapsed" role="group"></ul>
const ul = document.createElement('ul');
ul.className = 'folder-tree collapsed';
ul.setAttribute('role', 'group');
li.append(ul);
// Wire DnD / click the same as before
if (!locked) { if (!locked) {
opt.addEventListener('dragstart', (ev) => { opt.addEventListener('dragstart', (ev) => {
try { ev.dataTransfer.setData("application/x-filerise-folder", fullPath); } catch {} try { ev.dataTransfer.setData('application/x-filerise-folder', fullPath); } catch {}
try { ev.dataTransfer.setData("text/plain", fullPath); } catch {} try { ev.dataTransfer.setData('text/plain', fullPath); } catch {}
ev.dataTransfer.effectAllowed = "move"; ev.dataTransfer.effectAllowed = 'move';
}); });
opt.addEventListener("dragover", folderDragOverHandler); opt.addEventListener('dragover', folderDragOverHandler);
opt.addEventListener("dragleave", folderDragLeaveHandler); opt.addEventListener('dragleave', folderDragLeaveHandler);
opt.addEventListener("drop", (e) => handleDropOnFolder(e, fullPath)); opt.addEventListener('drop', (e) => handleDropOnFolder(e, fullPath));
opt.addEventListener("click", () => selectFolder(fullPath)); opt.addEventListener('click', () => selectFolder(fullPath));
} else { } else {
opt.addEventListener('click', async (e) => { opt.addEventListener('click', async (e) => {
e.preventDefault(); e.stopPropagation(); e.preventDefault(); e.stopPropagation();