diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index e892a473..593ed10c 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -20,7 +20,7 @@ jobs: contents: write steps: - name: Checkout code - uses: actions/checkout@v4 # v6.0.2 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - name: Create Archive # Nous reproduisons ici la logique d'exclusion de votre script make_release_zip.ps1 @@ -40,7 +40,7 @@ jobs: -x "*.zip" - name: Update Nightly Release - uses: softprops/action-gh-release@v2 + uses: softprops/action-gh-release@v2 # TODO: pin to commit SHA for supply-chain safety with: token: ${{ secrets.GITHUB_TOKEN }} tag_name: nightly diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 687c0971..21266044 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -48,6 +48,25 @@ jobs: - name: Audit JS dependencies run: npm audit --audit-level=high + js-test: + runs-on: ubuntu-22.04 + permissions: + contents: read + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + + - name: Set up Node.js + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version: "20" + cache: "npm" + + - name: Install JS dependencies + run: npm ci --ignore-scripts + + - name: Run JS tests (vitest) + run: npm run test:js + test: runs-on: ubuntu-22.04 permissions: @@ -55,7 +74,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.10", "3.12", "3.13", "3.14"] + python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -80,10 +99,18 @@ jobs: run: | python -m pip install --upgrade pip pip install -r requirements.txt - pip install pytest pytest-asyncio + pip install pytest pytest-asyncio pytest-cov - name: Run pytest - run: python -m pytest -q + run: python -m pytest -q --cov=mjr_am_backend --cov=mjr_am_shared --cov-report=xml --cov-report=term-missing:skip-covered + + - name: Upload coverage report + if: matrix.python-version == '3.12' + uses: actions/upload-artifact@v4 + with: + name: coverage-xml + path: coverage.xml + retention-days: 14 typecheck: runs-on: ubuntu-22.04 diff --git a/.github/workflows/release-on-tag.yml b/.github/workflows/release-on-tag.yml index 750fd00c..e1dfd2ca 100644 --- a/.github/workflows/release-on-tag.yml +++ b/.github/workflows/release-on-tag.yml @@ -15,7 +15,7 @@ jobs: contents: write steps: - name: Checkout code - uses: actions/checkout@v4 # v4.0.2 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: submodules: true @@ -34,7 +34,7 @@ jobs: -x "*.zip" - name: Create Release - uses: softprops/action-gh-release@v1 + uses: softprops/action-gh-release@v2 # upgraded from v1; TODO: pin to commit SHA with: files: ComfyUI-Majoor-AssetsManager_release.zip generate_release_notes: true @@ -54,7 +54,7 @@ jobs: if: ${{ github.repository_owner == 'MajoorWaldi' }} steps: - name: Check out code - uses: actions/checkout@v4 # v4.0.2 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: submodules: true - name: Publish Custom Node diff --git a/.gitignore b/.gitignore index 0559c29d..5b3a54cb 100644 --- a/.gitignore +++ b/.gitignore @@ -91,14 +91,11 @@ AGENTS.md ROADMAP.md VERSION eslint.config.js -ROADMAP.md -VERSION -AGENTS.md -eslint.config.js -VERSION -.claude/settings.local.json .claude/settings.local.json +# Local directories that must never be tracked +Users/ + # Local untracked docs, assets, and test outputs (added by assistant) AUDIT_EVENTS.md PERFORMANCE_AUDIT.md @@ -165,6 +162,9 @@ reports_radon_raw_backend.json reports_vulture.txt reports_vulture_backend.txt +# Accidental files that must never reappear in git +{console.error(e) + # Extra local temp artifacts (pytest/radon scratch) .tmp_* tests/__pytest_tmp_py*/ @@ -174,10 +174,10 @@ reports_radon* radon_* audit_findings.md docs/SECURITY_AUDIT_2026-02-27.md -docs/SECURITY_AUDIT_2026-02-27.md audit_tracking.md docs/CODE_AUDIT_2026-02-28.md docs/GENINFO_AUDIT_REPORT.md docs/GENINFO_FIXES_SUMMARY.md docs/REFACTORING_SUMMARY.md README.md.backup +docs/CODE_AUDIT_2026-03-01.md diff --git a/docs/INSTALLATION.md b/docs/INSTALLATION.md index a8b77c3e..d8f37d85 100644 --- a/docs/INSTALLATION.md +++ b/docs/INSTALLATION.md @@ -54,6 +54,12 @@ cd ComfyUI-Majoor-AssetsManager pip install -r requirements.txt ``` +> **Faster installs with `uv`**: If you have [uv](https://github.com/astral-sh/uv) installed, you can use it as a drop-in replacement for significantly faster dependency resolution: +> ```bash +> uv pip install -r requirements.txt +> ``` +> Install `uv` with `pip install uv` or see the [uv documentation](https://docs.astral.sh/uv/) for other methods. + ### Step 3: Restart ComfyUI Stop your ComfyUI server completely and restart it to load the new extension. diff --git a/js/app/i18n.js b/js/app/i18n.js index 9f79c699..7fc7397f 100644 --- a/js/app/i18n.js +++ b/js/app/i18n.js @@ -1135,7 +1135,9 @@ export const t = (key, defaultOrParams, params) => { if (actualParams && typeof actualParams === "object") { // Replace {key} with values (regex handles {key} without spaces) Object.entries(actualParams).forEach(([k, v]) => { - text = text.replace(new RegExp(`\\{${k}\\}`, "g"), String(v)); + // Use replaceAll with a literal template string instead of new RegExp to avoid + // ReDoS risk when keys contain regex metacharacters. + text = text.replaceAll(`{${k}}`, String(v)); }); } diff --git a/js/app/runtimeState.js b/js/app/runtimeState.js index 21630d1a..5f8586f4 100644 --- a/js/app/runtimeState.js +++ b/js/app/runtimeState.js @@ -1,3 +1,15 @@ +/** + * Global state key registry for Majoor Assets Manager. + * + * All `window`-level keys used by this extension are listed here to avoid + * accidental collisions with other extensions or user scripts. + * + * | Key / Symbol | File | Purpose | + * |-----------------------------------------------------------|-----------------|----------------------------------| + * | Symbol.for("majoor.assets_manager.runtime_state") | runtimeState.js | Extension runtime state object | + * | window["__MJR_VIDEO_THUMBS__"] | Card.js | Video thumbnail manager singleton| + */ + const RUNTIME_STATE_KEY = Symbol.for("majoor.assets_manager.runtime_state"); function _runtimeRoot() { diff --git a/js/app/toast.js b/js/app/toast.js index 8ae599d5..92fa34f2 100644 --- a/js/app/toast.js +++ b/js/app/toast.js @@ -35,6 +35,11 @@ function _startCleanerTimer() { for (const fn of toRemove) { _fallbackErrorToastDismissers.delete(fn); } + // Stop the interval when no active toasts remain (prevents a timer leak). + if (_fallbackErrorToastDismissers.size === 0) { + clearInterval(_cleanerTimer); + _cleanerTimer = null; + } }, 30000); } diff --git a/js/components/Viewer_impl.js b/js/components/Viewer_impl.js index aa083f15..bc8f1466 100644 --- a/js/components/Viewer_impl.js +++ b/js/components/Viewer_impl.js @@ -539,7 +539,7 @@ function createViewer() { `; const genInfoHeaderLeft = genInfoHeader.cloneNode(true); // Shallow clone structure // safe: clearing existing static node content only - genInfoHeaderLeft.innerHTML = ""; + genInfoHeaderLeft.replaceChildren(); const genInfoTitleLeft = document.createElement("div"); genInfoTitleLeft.textContent = "Generation Info (A)"; genInfoTitleLeft.style.cssText = "font-size: 13px; font-weight: 600;"; @@ -988,8 +988,8 @@ function createViewer() { if (!open) { stopGenInfoFetch(); - try { genInfoBody.innerHTML = ""; } catch (e) { console.debug?.(e); } - try { genInfoBodyLeft.innerHTML = ""; } catch (e) { console.debug?.(e); } + try { genInfoBody.replaceChildren(); } catch (e) { console.debug?.(e); } + try { genInfoBodyLeft.replaceChildren(); } catch (e) { console.debug?.(e); } return; } } catch { @@ -1003,8 +1003,8 @@ function createViewer() { state._genInfoAbort = ac; const renderNow = ({ left, leftExtra, right, rightExtra, single }) => { - try { genInfoBody.innerHTML = ""; } catch (e) { console.debug?.(e); } - try { genInfoBodyLeft.innerHTML = ""; } catch (e) { console.debug?.(e); } + try { genInfoBody.replaceChildren(); } catch (e) { console.debug?.(e); } + try { genInfoBodyLeft.replaceChildren(); } catch (e) { console.debug?.(e); } const onRetry = () => { try { @@ -1219,7 +1219,7 @@ function createViewer() { const renderBadges = () => { const clear = (el) => { try { - if (el) el.innerHTML = ""; + if (el) el.replaceChildren(); } catch (e) { console.debug?.(e); } }; clear(badgesBar); @@ -1442,19 +1442,19 @@ function createViewer() { try { if (state.mode !== VIEWER_MODES.SINGLE) { destroyMediaProcessorsIn(singleView); - singleView.innerHTML = ""; + singleView.replaceChildren(); } } catch (e) { console.debug?.(e); } try { if (state.mode !== VIEWER_MODES.AB_COMPARE) { destroyMediaProcessorsIn(abView); - abView.innerHTML = ""; + abView.replaceChildren(); } } catch (e) { console.debug?.(e); } try { if (state.mode !== VIEWER_MODES.SIDE_BY_SIDE) { destroyMediaProcessorsIn(sideView); - sideView.innerHTML = ""; + sideView.replaceChildren(); } } catch (e) { console.debug?.(e); } @@ -1515,7 +1515,7 @@ function createViewer() { destroyMediaProcessorsIn(singleView); } catch (e) { console.debug?.(e); } try { - singleView.innerHTML = ""; + singleView.replaceChildren(); const err = document.createElement("div"); err.className = "mjr-viewer-media"; err.style.cssText = "color:#ff9a9a; font-size:13px; padding:16px; text-align:center;"; @@ -2036,15 +2036,15 @@ function createViewer() { // Free DOM resources (and ensure playback stops even if `pause()` was ignored). try { destroyMediaProcessorsIn(singleView); - singleView.innerHTML = ""; + singleView.replaceChildren(); } catch (e) { console.debug?.(e); } try { destroyMediaProcessorsIn(abView); - abView.innerHTML = ""; + abView.replaceChildren(); } catch (e) { console.debug?.(e); } try { destroyMediaProcessorsIn(sideView); - sideView.innerHTML = ""; + sideView.replaceChildren(); } catch (e) { console.debug?.(e); } try { @@ -2055,11 +2055,11 @@ function createViewer() { } catch (e) { console.debug?.(e); } try { genInfoOverlay.style.display = "none"; - genInfoBody.innerHTML = ""; + genInfoBody.replaceChildren(); } catch (e) { console.debug?.(e); } try { genInfoOverlayLeft.style.display = "none"; - genInfoBodyLeft.innerHTML = ""; + genInfoBodyLeft.replaceChildren(); } catch (e) { console.debug?.(e); } overlay.style.display = 'none'; diff --git a/js/features/grid/VirtualGrid.js b/js/features/grid/VirtualGrid.js index 2d6bbfc6..48ffe474 100644 --- a/js/features/grid/VirtualGrid.js +++ b/js/features/grid/VirtualGrid.js @@ -222,7 +222,7 @@ export class VirtualGrid { this.rafId = 0; } this.renderPending = false; - this.container.innerHTML = ""; + this.container.replaceChildren(); this.renderedItems.clear(); this._pool.length = 0; } diff --git a/js/features/viewer/FloatingViewer.js b/js/features/viewer/FloatingViewer.js index 3634b3c9..bcfe9ad8 100644 --- a/js/features/viewer/FloatingViewer.js +++ b/js/features/viewer/FloatingViewer.js @@ -162,7 +162,7 @@ export class FloatingViewer { this._genBtn = null; this._genDropdown = null; this._captureBtn = null; - this._genInfoSelections = new Set(["prompt"]); + this._genInfoSelections = new Set(["genTime"]); this._mode = MFV_MODES.SIMPLE; this._mediaA = null; this._mediaB = null; @@ -207,7 +207,10 @@ export class FloatingViewer { closeBtn.type = "button"; closeBtn.className = "mjr-icon-btn"; closeBtn.title = t("tooltip.closeViewer", "Close viewer"); - closeBtn.innerHTML = ''; + const _closeBtnIcon = document.createElement("i"); + _closeBtnIcon.className = "pi pi-times"; + _closeBtnIcon.setAttribute("aria-hidden", "true"); + closeBtn.appendChild(_closeBtnIcon); closeBtn.addEventListener("click", () => { window.dispatchEvent(new CustomEvent(EVENTS.MFV_CLOSE)); }); @@ -250,7 +253,10 @@ export class FloatingViewer { this._genBtn = document.createElement("button"); this._genBtn.type = "button"; this._genBtn.className = "mjr-icon-btn"; - this._genBtn.innerHTML = ''; + const _genBtnIcon = document.createElement("i"); + _genBtnIcon.className = "pi pi-info-circle"; + _genBtnIcon.setAttribute("aria-hidden", "true"); + this._genBtn.appendChild(_genBtnIcon); this._genBtn.addEventListener("click", (e) => { e.stopPropagation(); if (this._genDropdown?.classList?.contains("is-visible")) { @@ -267,7 +273,10 @@ export class FloatingViewer { this._captureBtn.type = "button"; this._captureBtn.className = "mjr-icon-btn"; this._captureBtn.title = t("tooltip.captureView", "Save view as image"); - this._captureBtn.innerHTML = ''; + const _captureBtnIcon = document.createElement("i"); + _captureBtnIcon.className = "pi pi-download"; + _captureBtnIcon.setAttribute("aria-hidden", "true"); + this._captureBtn.appendChild(_captureBtnIcon); this._captureBtn.addEventListener("click", () => this._captureView()); bar.appendChild(this._captureBtn); @@ -373,7 +382,7 @@ export class FloatingViewer { // Fallback to candidate prompt if norm.prompt is empty if (!out.prompt && candidate?.prompt) out.prompt = String(candidate.prompt || ""); - const genTimeMs = fileData.generation_time_ms ?? candidate?.generation_time_ms ?? candidate?.geninfo?.generation_time_ms ?? 0; + const genTimeMs = fileData.generation_time_ms ?? fileData.metadata_raw?.generation_time_ms ?? candidate?.generation_time_ms ?? candidate?.geninfo?.generation_time_ms ?? 0; if (genTimeMs && Number.isFinite(Number(genTimeMs)) && genTimeMs > 0 && genTimeMs < 86400000) { out.genTime = (Number(genTimeMs) / 1000).toFixed(1) + "s"; } @@ -395,39 +404,59 @@ export class FloatingViewer { out.cfg = (meta?.cfg != null) ? String(meta.cfg) : (meta?.cfg_scale != null ? String(meta.cfg_scale) : ""); out.step = (meta?.steps != null) ? String(meta.steps) : ""; - const genTimeMsFallback = fileData.generation_time_ms ?? meta?.generation_time_ms ?? 0; + const genTimeMsFallback = fileData.generation_time_ms ?? fileData.metadata_raw?.generation_time_ms ?? meta?.generation_time_ms ?? 0; if (genTimeMsFallback && Number.isFinite(Number(genTimeMsFallback)) && genTimeMsFallback > 0 && genTimeMsFallback < 86400000) { out.genTime = (Number(genTimeMsFallback) / 1000).toFixed(1) + "s"; } return out; } - _formatGenInfoHTML(fileData) { + /** + * Build a DocumentFragment of gen-info rows using the DOM API. + * Returns null when no fields are selected or all are empty. + * Using the DOM API instead of innerHTML string concatenation eliminates + * any XSS risk from metadata values and avoids fragile HTML escaping. + */ + _buildGenInfoDOM(fileData) { const fields = this._getGenFields(fileData); - if (!fields) return ""; - const parts = []; - // Show only selected fields from dropdown + if (!fields) return null; + const frag = document.createDocumentFragment(); const order = ["prompt", "seed", "model", "lora", "sampler", "scheduler", "cfg", "step", "genTime"]; for (const k of order) { - // Only show if user has selected this field in the dropdown if (!this._genInfoSelections.has(k)) continue; const v = (fields[k] != null) ? String(fields[k]) : ""; if (!v) continue; - let label = k.charAt(0).toUpperCase() + k.slice(1); if (k === "lora") label = "LoRA"; else if (k === "cfg") label = "CFG"; else if (k === "genTime") label = "Gen Time"; - + const div = document.createElement("div"); + div.dataset.field = k; + const strong = document.createElement("strong"); + strong.textContent = `${label}: `; + div.appendChild(strong); if (k === "prompt") { // Truncate long prompts to 240 chars - const short = v.length > 240 ? v.slice(0, 240) + "…" : v; - parts.push(`
${label}: ${escapeHtml(short)}
`); + const short = v.length > 240 ? v.slice(0, 240) + "\u2026" : v; + div.appendChild(document.createTextNode(short)); + } else if (k === "genTime") { + // Color-code gen time (matches FileInfoSection.js colour scheme) + const secs = parseFloat(v); + let gtColor = "#4CAF50"; // green < 10s + if (secs >= 60) gtColor = "#FF9800"; // orange + else if (secs >= 30) gtColor = "#FFC107"; // yellow + else if (secs >= 10) gtColor = "#8BC34A"; // light green + const span = document.createElement("span"); + span.style.color = gtColor; + span.style.fontWeight = "600"; + span.textContent = v; + div.appendChild(span); } else { - parts.push(`
${label}: ${escapeHtml(v)}
`); + div.appendChild(document.createTextNode(v)); } + frag.appendChild(div); } - return parts.join(""); + return frag.childNodes.length > 0 ? frag : null; } // ── Mode ────────────────────────────────────────────────────────────────── @@ -454,7 +483,10 @@ export class FloatingViewer { [MFV_MODES.SIDE]: { icon: "pi-table", label: "Mode: Side-by-Side (click to switch)" }, }; const { icon = "pi-image", label = "" } = cfg[this._mode] || {}; - this._modeBtn.innerHTML = ``; + const _modeBtnIcon = document.createElement("i"); + _modeBtnIcon.className = `pi ${icon}`; + _modeBtnIcon.setAttribute("aria-hidden", "true"); + this._modeBtn.replaceChildren(_modeBtnIcon); this._modeBtn.title = label; } @@ -464,10 +496,16 @@ export class FloatingViewer { if (!this._liveBtn) return; this._liveBtn.classList.toggle("mjr-live-active", Boolean(active)); if (active) { - this._liveBtn.innerHTML = ''; + const _liveIconActive = document.createElement("i"); + _liveIconActive.className = "pi pi-circle-fill"; + _liveIconActive.setAttribute("aria-hidden", "true"); + this._liveBtn.replaceChildren(_liveIconActive); this._liveBtn.title = t("tooltip.liveStreamOn", "Live Stream: ON — click to disable"); } else { - this._liveBtn.innerHTML = ''; + const _liveIconInactive = document.createElement("i"); + _liveIconInactive.className = "pi pi-circle"; + _liveIconInactive.setAttribute("aria-hidden", "true"); + this._liveBtn.replaceChildren(_liveIconInactive); this._liveBtn.title = t("tooltip.liveStreamOff", "Live Stream: OFF — click to follow"); } } @@ -611,7 +649,7 @@ export class FloatingViewer { this._dragging = true; startX = e.clientX; startY = e.clientY; startPanX = this._panX; startPanY = this._panY; - try { contentEl.setPointerCapture(e.pointerId); } catch {} + try { contentEl.setPointerCapture(e.pointerId); } catch (e) { console.debug?.(e); } this._applyTransform(); }, sig); @@ -626,7 +664,7 @@ export class FloatingViewer { if (!panActive) return; panActive = false; this._dragging = false; - try { contentEl.releasePointerCapture(e.pointerId); } catch {} + try { contentEl.releasePointerCapture(e.pointerId); } catch (e) { console.debug?.(e); } this._applyTransform(); }; contentEl.addEventListener("pointerup", endPan, sig); @@ -645,7 +683,7 @@ export class FloatingViewer { /** Remove all pan/zoom event listeners. */ _destroyPanZoom() { - try { this._panzoomAC?.abort(); } catch {} + try { this._panzoomAC?.abort(); } catch (e) { console.debug?.(e); } this._panzoomAC = null; this._dragging = false; } @@ -683,11 +721,11 @@ export class FloatingViewer { wrap.className = "mjr-mfv-simple-container"; wrap.appendChild(mediaEl); // Gen info overlay for SIMPLE mode - const infoHtml = this._formatGenInfoHTML(this._mediaA); - if (infoHtml) { + const infoFrag = this._buildGenInfoDOM(this._mediaA); + if (infoFrag) { const ol = document.createElement("div"); ol.className = "mjr-mfv-geninfo"; - ol.innerHTML = infoHtml; + ol.appendChild(infoFrag); wrap.appendChild(ol); } this._contentEl.appendChild(wrap); @@ -730,21 +768,21 @@ export class FloatingViewer { // Gen info overlays are placed at the container level (outside the clipped // layers) so they are never truncated by layerB's clip-path. Each overlay // is bounded to its own half, mirroring the canvas capture layout. - const infoA = this._formatGenInfoHTML(this._mediaA); + const fragA = this._buildGenInfoDOM(this._mediaA); let genInfoAEl = null; - if (infoA) { + if (fragA) { genInfoAEl = document.createElement("div"); genInfoAEl.className = "mjr-mfv-geninfo-a"; - genInfoAEl.innerHTML = infoA; + genInfoAEl.appendChild(fragA); // Limit right edge to divider so it doesn't bleed into B side. genInfoAEl.style.right = `calc(${100 - pct}% + 8px)`; } - const infoB = this._formatGenInfoHTML(this._mediaB); + const fragB = this._buildGenInfoDOM(this._mediaB); let genInfoBEl = null; - if (infoB) { + if (fragB) { genInfoBEl = document.createElement("div"); genInfoBEl.className = "mjr-mfv-geninfo-b"; - genInfoBEl.innerHTML = infoB; + genInfoBEl.appendChild(fragB); // Start at the divider — overrides CSS left:8px so it is never // clipped by layerB's clip-path. genInfoBEl.style.left = `calc(${pct}% + 8px)`; @@ -801,11 +839,11 @@ export class FloatingViewer { sideA.appendChild(_makeLabel("A", "left")); // Gen info overlay for left - const infoA = this._formatGenInfoHTML(this._mediaA); - if (infoA) { + const fragSideA = this._buildGenInfoDOM(this._mediaA); + if (fragSideA) { const oa = document.createElement("div"); oa.className = "mjr-mfv-geninfo-a"; - oa.innerHTML = infoA; + oa.appendChild(fragSideA); sideA.appendChild(oa); } @@ -816,11 +854,11 @@ export class FloatingViewer { sideB.appendChild(_makeLabel("B", "right")); // Gen info overlay for right - const infoB = this._formatGenInfoHTML(this._mediaB); - if (infoB) { + const fragSideB = this._buildGenInfoDOM(this._mediaB); + if (fragSideB) { const ob = document.createElement("div"); ob.className = "mjr-mfv-geninfo-b"; - ob.innerHTML = infoB; + ob.appendChild(fragSideB); sideB.appendChild(ob); } @@ -1140,8 +1178,8 @@ export class FloatingViewer { this._modeBtn = null; this._liveBtn = null; this._captureBtn = null; - try { document.removeEventListener("click", this._handleDocClick); } catch {} - try { this._genDropdown?.remove(); } catch {} + try { document.removeEventListener("click", this._handleDocClick); } catch (e) { console.debug?.(e); } + try { this._genDropdown?.remove(); } catch (e) { console.debug?.(e); } this._mediaA = null; this._mediaB = null; this.isVisible = false; diff --git a/js/features/viewer/floatingViewerManager.js b/js/features/viewer/floatingViewerManager.js index 8d9f681c..3e9ad2d2 100644 --- a/js/features/viewer/floatingViewerManager.js +++ b/js/features/viewer/floatingViewerManager.js @@ -20,7 +20,7 @@ import { reportError } from "../../utils/logging.js"; /** @type {FloatingViewer | null} */ let _instance = null; -let _liveActive = false; +let _liveActive = true; let _selectionListenerBound = false; let _fetchAC = null; // AbortController for the latest in-flight batch fetch diff --git a/js/features/viewer/sideBySide.js b/js/features/viewer/sideBySide.js index bd74370e..e5478f79 100644 --- a/js/features/viewer/sideBySide.js +++ b/js/features/viewer/sideBySide.js @@ -6,7 +6,9 @@ function _escHtml(s) { return String(s ?? "") .replace(/&/g, "&") .replace(//g, ">"); + .replace(/>/g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); } function _extractGridGenInfo(asset) { diff --git a/mjr_am_backend/features/geninfo/graph_converter.py b/mjr_am_backend/features/geninfo/graph_converter.py index edb75996..e2d98b52 100644 --- a/mjr_am_backend/features/geninfo/graph_converter.py +++ b/mjr_am_backend/features/geninfo/graph_converter.py @@ -3,7 +3,7 @@ from __future__ import annotations from collections import deque -from typing import Any, cast +from typing import Any, TypeGuard, cast DEFAULT_MAX_GRAPH_NODES = 5000 DEFAULT_MAX_GRAPH_DEPTH = 100 @@ -97,7 +97,7 @@ def _sink_priority(node: dict[str, Any], node_id: str | None = None) -> tuple[in return (_sink_group(ct), _sink_images_tiebreak(node), _sink_node_id_tiebreak(node_id)) -def _is_link(value: Any) -> bool: +def _is_link(value: Any) -> TypeGuard[list]: if not isinstance(value, (list, tuple)) or len(value) != 2: return False a, b = value[0], value[1] diff --git a/mjr_am_backend/features/geninfo/parser_impl.py b/mjr_am_backend/features/geninfo/parser_impl.py index d2572e20..b1e03d71 100644 --- a/mjr_am_backend/features/geninfo/parser_impl.py +++ b/mjr_am_backend/features/geninfo/parser_impl.py @@ -675,7 +675,7 @@ def _collect_all_samplers_from_sinks( sampler_values = _extract_sampler_values(nodes_by_id, sampler_node, sampler_id, ins, advanced, confidence, trace) - export_sampler = {} + export_sampler: dict[str, Any] = {} for k in ["sampler_name", "scheduler"]: if k in sampler_values and sampler_values[k] is not None: export_sampler[k] = str(sampler_values[k]) diff --git a/mjr_am_backend/features/index/enricher.py b/mjr_am_backend/features/index/enricher.py index a048adca..5539430a 100644 --- a/mjr_am_backend/features/index/enricher.py +++ b/mjr_am_backend/features/index/enricher.py @@ -365,7 +365,8 @@ class MetadataEnricher: if res.ok and res.data: payload = dict(res.data[0]) from ...routes.registry import PromptServer - PromptServer.instance.send_sync("mjr-asset-updated", payload) + from ...utils import sanitize_for_json + PromptServer.instance.send_sync("mjr-asset-updated", sanitize_for_json(payload)) except Exception: pass diff --git a/mjr_am_backend/features/index/searcher.py b/mjr_am_backend/features/index/searcher.py index 9822ea20..92120d47 100644 --- a/mjr_am_backend/features/index/searcher.py +++ b/mjr_am_backend/features/index/searcher.py @@ -22,6 +22,10 @@ from ...shared import Result, get_logger logger = get_logger(__name__) def _normalize_extension(value) -> str: + # NOTE: This is an intentional local copy of the same helper in + # mjr_am_backend/routes/handlers/filesystem.py. A shared import would + # create a cross-package dependency (features/index → routes/handlers) + # which would invert the dependency hierarchy. Keep both copies in sync. if value is None: return "" try: @@ -479,9 +483,10 @@ class IndexSearcher: return Result.Ok([]) try: + escaped_prefix = _escape_like_pattern(clean_prefix) res = await self.db.aquery( - "SELECT term FROM asset_metadata_vocab WHERE term LIKE ? ORDER BY doc DESC LIMIT ?", - (f"{clean_prefix}%", limit) + "SELECT term FROM asset_metadata_vocab WHERE term LIKE ? ESCAPE '\\' ORDER BY doc DESC LIMIT ?", + (f"{escaped_prefix}%", limit) ) if not res.ok: return Result.Ok([]) diff --git a/mjr_am_backend/features/index/service.py b/mjr_am_backend/features/index/service.py index efdc425e..e099ff28 100644 --- a/mjr_am_backend/features/index/service.py +++ b/mjr_am_backend/features/index/service.py @@ -16,6 +16,7 @@ from mjr_am_shared.scan_throttle import mark_directory_indexed from ...adapters.db.sqlite import Sqlite from ...shared import Result, get_logger +from ...utils import sanitize_for_json from ..metadata import MetadataService from .enricher import MetadataEnricher from .metadata_helpers import MetadataHelpers @@ -230,7 +231,7 @@ class IndexService: if batch_res.ok and batch_res.data: for asset in batch_res.data: try: - _PS.instance.send_sync("mjr-asset-added", dict(asset)) + _PS.instance.send_sync("mjr-asset-added", sanitize_for_json(dict(asset))) except Exception as exc: logger.debug("Failed to push mjr-asset-added for one asset: %s", exc) except Exception as e: diff --git a/mjr_am_backend/routes/core/security.py b/mjr_am_backend/routes/core/security.py index 5462d68f..549c3a20 100644 --- a/mjr_am_backend/routes/core/security.py +++ b/mjr_am_backend/routes/core/security.py @@ -95,6 +95,12 @@ def _safe_mode_enabled() -> bool: Safe Mode is enabled by default to reduce risk when ComfyUI is exposed to remote clients (via --listen, tunnels, reverse proxies, etc.). + + NOTE: This result is cached on the first call via @lru_cache and does NOT update at + runtime. Changing the MAJOOR_SAFE_MODE environment variable requires a **process + restart** to take effect. This is intentional — safe mode is a startup-time security + policy, not a hot-reloadable setting. The cache is cleared in tests via + ``_safe_mode_enabled.cache_clear()``. """ # Default: enabled when unset. try: diff --git a/mjr_am_backend/routes/handlers/assets_impl.py b/mjr_am_backend/routes/handlers/assets_impl.py index 97a00f39..4b0fc5bd 100644 --- a/mjr_am_backend/routes/handlers/assets_impl.py +++ b/mjr_am_backend/routes/handlers/assets_impl.py @@ -20,7 +20,7 @@ from mjr_am_backend.shared import sanitize_error_message as _safe_error_message from ..assets.path_guard import ( build_download_response as _pg_build_download_response, - delete_file_best_effort as _delete_file_best_effort, + delete_file_best_effort as _delete_file_safe, is_resolved_path_allowed as _is_resolved_path_allowed, safe_download_filename as _pg_safe_download_filename, ) @@ -70,7 +70,7 @@ _filename_reserved_error = _fv.filename_reserved_error _validate_filename = _fv.validate_filename _is_resolved_path_allowed = _pg.is_resolved_path_allowed -_delete_file_best_effort = _pg.delete_file_best_effort +_delete_file_safe = _pg.delete_file_best_effort _pg_build_download_response = _pg.build_download_response _pg_safe_download_filename = _pg.safe_download_filename @@ -987,7 +987,7 @@ def register_asset_routes(routes: web.RouteTableDef) -> None: ) try: - del_res = _delete_file_best_effort(resolved) + del_res = _delete_file_safe(resolved) if not del_res.ok: raise RuntimeError(str(del_res.error or "delete failed")) except Exception as exc: @@ -1341,7 +1341,7 @@ def register_asset_routes(routes: web.RouteTableDef) -> None: resolved = asset_info["resolved"] if resolved and resolved.exists() and resolved.is_file(): try: - del_res = _delete_file_best_effort(resolved) + del_res = _delete_file_safe(resolved) if not del_res.ok: file_deletion_errors.append({"asset_id": asset_info["id"], "error": str(del_res.error or "delete failed")}) continue @@ -1373,6 +1373,14 @@ def register_asset_routes(routes: web.RouteTableDef) -> None: SQLITE_IN_MAX = 900 async def _delete_where_in(column: str, table: str, values: list[Any]) -> Result: + # Allowlist guards against SQL injection if callers ever pass non-literal names. + _allowed: dict[str, frozenset[str]] = { + "assets": frozenset({"id", "filepath"}), + "scan_journal": frozenset({"id", "filepath"}), + "metadata_cache": frozenset({"id", "filepath"}), + } + if table not in _allowed or column not in _allowed[table]: + return Result.Err("INVALID_INPUT", f"Disallowed delete target: {table!r}.{column!r}") if not values: return Result.Ok(True) for chunk in _chunks(values, SQLITE_IN_MAX): @@ -1505,8 +1513,10 @@ def _resolve_download_path(filepath: Any) -> Path | web.Response: try: if candidate.is_symlink(): return web.Response(status=403, text="Symlinked file not allowed") - except Exception: - pass + except Exception as _e: + # is_symlink() can raise on some platforms/edge cases; the path + # will be re-checked by resolve(strict=True) below. + logger.debug("is_symlink check failed (OS edge case): %s", _e) try: resolved = candidate.resolve(strict=True) except (OSError, RuntimeError, ValueError): diff --git a/mjr_am_backend/routes/handlers/scan.py b/mjr_am_backend/routes/handlers/scan.py index 83c24433..2d4816d8 100644 --- a/mjr_am_backend/routes/handlers/scan.py +++ b/mjr_am_backend/routes/handlers/scan.py @@ -901,6 +901,11 @@ def register_scan_routes(routes: web.RouteTableDef) -> None: return _json_response(Result.Ok({"cleared": cleared, "scan_summary": scan_summary, "rebuild_fts": None})) async def _clear_table(table: str, prefixes: list[str] | None) -> Result[int]: + # Allowlist guards against accidental SQL injection if callers ever pass + # non-literal table names. + _allowed_tables = frozenset({"assets", "scan_journal", "metadata_cache", "asset_metadata"}) + if table not in _allowed_tables: + return Result.Err("INVALID_INPUT", f"Attempt to clear unknown table: {table!r}") total_deleted = 0 if prefixes is None: res = await db.aexecute(f"DELETE FROM {table}") @@ -1054,13 +1059,18 @@ def register_scan_routes(routes: web.RouteTableDef) -> None: # Verify we are cleaning the right folder and safeguard critical files if reindex and INDEX_DIR_PATH.exists(): def _cleanup_index_dir(): + # Keep-list: every name/prefix listed here is preserved unchanged. + # IMPORTANT: when adding a new persistent state file to the index directory, + # register its name (or prefix) below to prevent it from being wiped on + # a full reindex. Using an explicit constant avoids silent data loss. + _KEEP_PREFIXES: frozenset[str] = frozenset({"assets.sqlite"}) # DB + WAL/SHM + _KEEP_NAMES: frozenset[str] = frozenset({"custom_roots.json"}) for item in INDEX_DIR_PATH.iterdir(): - if item.name.startswith("assets.sqlite"): + if any(item.name.startswith(p) for p in _KEEP_PREFIXES): continue if item == COLLECTIONS_DIR_PATH: continue - # Preserve user configuration/state stored in the index directory. - if item.name == "custom_roots.json": + if item.name in _KEEP_NAMES: continue try: diff --git a/mjr_am_backend/routes/handlers/search_impl.py b/mjr_am_backend/routes/handlers/search_impl.py index 685f6865..b67db0b9 100644 --- a/mjr_am_backend/routes/handlers/search_impl.py +++ b/mjr_am_backend/routes/handlers/search_impl.py @@ -405,8 +405,8 @@ def register_search_routes(routes: web.RouteTableDef) -> None: if browser_result.ok and isinstance(browser_result.data, dict): assets = browser_result.data.get("assets") or [] browser_result.data["assets"] = await _hydrate_browser_assets_from_db(svc, assets) - except Exception: - pass + except Exception as _e: + logger.debug("Failed to hydrate browser assets from DB: %s", _e) return _json_response(browser_result) root_result = resolve_custom_root(str(root_id or "")) if not root_result.ok: diff --git a/mjr_am_backend/utils.py b/mjr_am_backend/utils.py index 8efa7600..741deccb 100644 --- a/mjr_am_backend/utils.py +++ b/mjr_am_backend/utils.py @@ -3,6 +3,7 @@ Utility helpers shared across backend modules. """ from __future__ import annotations +import math import os from typing import Any @@ -53,3 +54,18 @@ def env_float(name: str, default: float) -> float: return float(raw) except (ValueError, TypeError): return default + + +def sanitize_for_json(obj: Any) -> Any: + """Recursively replace NaN/Infinity float values with ``None`` so the + result is valid JSON. ComfyUI nodes routinely store ``float('nan')`` in + ``is_changed`` to signal *always re-execute*; Python's ``json.dumps`` + emits that as the bare token ``NaN`` which browsers reject. + """ + if isinstance(obj, float): + return None if (math.isnan(obj) or math.isinf(obj)) else obj + if isinstance(obj, dict): + return {k: sanitize_for_json(v) for k, v in obj.items()} + if isinstance(obj, list): + return [sanitize_for_json(v) for v in obj] + return obj diff --git a/mypy.ini b/mypy.ini index 9c2e5e88..5d12d709 100644 --- a/mypy.ini +++ b/mypy.ini @@ -1,5 +1,5 @@ [mypy] -python_version = 3.12 +python_version = 3.10 files = mjr_am_backend,mjr_am_shared # Repository root has a hyphen in its folder name; require explicit bases. diff --git a/requirements.txt b/requirements.txt index 52be76c0..aee091be 100644 --- a/requirements.txt +++ b/requirements.txt @@ -21,7 +21,7 @@ watchdog>=3.0.0,<7.0.0 # Windows-specific: Windows Property System metadata support # Only required on Windows; gracefully skipped on other platforms -pywin32>=300,<311; sys_platform == 'win32' +pywin32>=300,<312; sys_platform == 'win32' # Note: tkinter is required for native folder browser functionality # tkinter is included in standard Python distribution but may be missing in minimal installs diff --git a/tests/config/test_config.py b/tests/config/test_config.py new file mode 100644 index 00000000..5ce02a7c --- /dev/null +++ b/tests/config/test_config.py @@ -0,0 +1,102 @@ +""" +Tests for mjr_am_backend.config + +Covers the env-var helper functions (_env_int, _env_float, _env_bool) +and basic validation of module-level constants. +""" +import os + +import pytest + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _set_env(monkeypatch, name: str, value: str) -> None: + monkeypatch.setenv(name, value) + + +def _unset_env(monkeypatch, name: str) -> None: + monkeypatch.delenv(name, raising=False) + + +# --------------------------------------------------------------------------- +# _env_int +# --------------------------------------------------------------------------- + +class TestEnvInt: + def test_returns_default_when_not_set(self, monkeypatch): + from mjr_am_backend.config import _env_int # type: ignore[attr-defined] + _unset_env(monkeypatch, "MJR_TEST_INT") + assert _env_int(42, "MJR_TEST_INT") == 42 + + def test_parses_valid_int(self, monkeypatch): + from mjr_am_backend.config import _env_int # type: ignore[attr-defined] + _set_env(monkeypatch, "MJR_TEST_INT", "99") + assert _env_int(0, "MJR_TEST_INT") == 99 + + def test_clamps_to_min(self, monkeypatch): + from mjr_am_backend.config import _env_int # type: ignore[attr-defined] + _set_env(monkeypatch, "MJR_TEST_INT", "-5") + assert _env_int(0, "MJR_TEST_INT", min_value=0) == 0 + + def test_clamps_to_max(self, monkeypatch): + from mjr_am_backend.config import _env_int # type: ignore[attr-defined] + _set_env(monkeypatch, "MJR_TEST_INT", "1000") + assert _env_int(0, "MJR_TEST_INT", max_value=100) == 100 + + def test_returns_default_on_invalid(self, monkeypatch): + from mjr_am_backend.config import _env_int # type: ignore[attr-defined] + _set_env(monkeypatch, "MJR_TEST_INT", "not_a_number") + assert _env_int(7, "MJR_TEST_INT") == 7 + + +# --------------------------------------------------------------------------- +# _env_float +# --------------------------------------------------------------------------- + +class TestEnvFloat: + def test_returns_default_when_not_set(self, monkeypatch): + from mjr_am_backend.config import _env_float # type: ignore[attr-defined] + _unset_env(monkeypatch, "MJR_TEST_FLOAT") + assert _env_float(3.14, "MJR_TEST_FLOAT") == pytest.approx(3.14) + + def test_parses_valid_float(self, monkeypatch): + from mjr_am_backend.config import _env_float # type: ignore[attr-defined] + _set_env(monkeypatch, "MJR_TEST_FLOAT", "2.5") + assert _env_float(0.0, "MJR_TEST_FLOAT") == pytest.approx(2.5) + + def test_clamps_to_min(self, monkeypatch): + from mjr_am_backend.config import _env_float # type: ignore[attr-defined] + _set_env(monkeypatch, "MJR_TEST_FLOAT", "-1.0") + assert _env_float(0.0, "MJR_TEST_FLOAT", min_value=0.0) == pytest.approx(0.0) + + def test_clamps_to_max(self, monkeypatch): + from mjr_am_backend.config import _env_float # type: ignore[attr-defined] + _set_env(monkeypatch, "MJR_TEST_FLOAT", "999.9") + assert _env_float(1.0, "MJR_TEST_FLOAT", max_value=10.0) == pytest.approx(10.0) + + def test_returns_default_on_invalid(self, monkeypatch): + from mjr_am_backend.config import _env_float # type: ignore[attr-defined] + _set_env(monkeypatch, "MJR_TEST_FLOAT", "bad") + assert _env_float(1.5, "MJR_TEST_FLOAT") == pytest.approx(1.5) + + +# --------------------------------------------------------------------------- +# Module-level constants: basic sanity +# --------------------------------------------------------------------------- + +class TestConfigConstants: + def test_output_root_is_path_like(self): + from mjr_am_backend.config import OUTPUT_ROOT # type: ignore[attr-defined] + from pathlib import Path + assert isinstance(OUTPUT_ROOT, (str, Path)) + + def test_search_max_limit_positive(self): + from mjr_am_backend.config import SEARCH_MAX_LIMIT # type: ignore[attr-defined] + assert SEARCH_MAX_LIMIT > 0 + + def test_to_thread_timeout_positive(self): + from mjr_am_backend.config import TO_THREAD_TIMEOUT_S # type: ignore[attr-defined] + assert TO_THREAD_TIMEOUT_S > 0 diff --git a/tests/features/test_assets_impl_routes.py b/tests/features/test_assets_impl_routes.py index d28c4a5d..913f152e 100644 --- a/tests/features/test_assets_impl_routes.py +++ b/tests/features/test_assets_impl_routes.py @@ -357,7 +357,7 @@ async def test_delete_asset_validation_paths(monkeypatch, tmp_path: Path): monkeypatch.setattr(m, "_read_json", _read_json2) monkeypatch.setattr(m, "_normalize_path", lambda v: Path(v) if v else None) monkeypatch.setattr(m, "_is_resolved_path_allowed", lambda _p: True) - monkeypatch.setattr(m, "_delete_file_best_effort", lambda _p: Result.Ok(True)) + monkeypatch.setattr(m, "_delete_file_safe", lambda _p: Result.Ok(True)) req2 = make_mocked_request("POST", "/mjr/am/asset/delete", app=app) resp2 = await (await app.router.resolve(req2)).handler(req2) @@ -450,7 +450,7 @@ async def test_assets_delete_bulk_paths(monkeypatch, tmp_path: Path): db.aquery_in = _aquery_in monkeypatch.setattr(m, "_read_json", _read_json_ok) - monkeypatch.setattr(m, "_delete_file_best_effort", lambda _p: Result.Ok(True)) + monkeypatch.setattr(m, "_delete_file_safe", lambda _p: Result.Ok(True)) req2 = make_mocked_request("POST", "/mjr/am/assets/delete", app=app) resp2 = await (await app.router.resolve(req2)).handler(req2) @@ -649,7 +649,7 @@ async def test_assets_delete_partial_errors(monkeypatch, tmp_path: Path): monkeypatch.setattr(m, "_is_path_allowed", lambda *_args, **_kwargs: True) monkeypatch.setattr(m, "_is_path_allowed_custom", lambda *_args, **_kwargs: False) monkeypatch.setattr(m, "_is_resolved_path_allowed", lambda _p: True) - monkeypatch.setattr(m, "_delete_file_best_effort", _del) + monkeypatch.setattr(m, "_delete_file_safe", _del) req = make_mocked_request("POST", "/mjr/am/assets/delete", app=app) resp = await (await app.router.resolve(req)).handler(req) @@ -935,7 +935,7 @@ async def test_delete_asset_db_error_and_delete_failure(monkeypatch, tmp_path: P monkeypatch.setattr(m, "_read_json", _read_json) monkeypatch.setattr(m, "_is_resolved_path_allowed", lambda _p: True) - monkeypatch.setattr(m, "_delete_file_best_effort", lambda _p: Result.Err("DELETE_FAILED", "bad")) + monkeypatch.setattr(m, "_delete_file_safe", lambda _p: Result.Err("DELETE_FAILED", "bad")) req = make_mocked_request("POST", "/mjr/am/asset/delete", app=app) resp = await (await app.router.resolve(req)).handler(req) assert _json(resp).get("code") == "DELETE_FAILED" diff --git a/tox.ini b/tox.ini index 8dbf438c..3a452ca3 100644 --- a/tox.ini +++ b/tox.ini @@ -7,5 +7,7 @@ skip_missing_interpreters = True deps = -rrequirements.txt pytest + pytest-asyncio + aiohttp commands = python -m pytest -q