diff --git a/CHANGELOG.md b/CHANGELOG.md index d8957053..5fe1464e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,16 @@ ### Fixed +## v0.50.221 — 2026-04-26 + +### Fixed +- **Custom providers model dropdown** — models dict keys in `custom_providers[].models` now all appear in the dropdown; previously only the singular `model` field was read. (`api/config.py`) [#1111 @bergeouss] Closes #1106 +- **Custom providers SSRF false positive** — hostnames from user-configured `custom_providers[].base_url` are now trusted through the SSRF check; local inference servers (llama.cpp, vLLM, TabbyAPI) no longer blocked. (`api/config.py`) [#1113 @bergeouss] Closes #1105 +- **Mobile/iPad session navigation** — tap no longer fails on first touch; replaced hover-triggered layout-shift pattern with `onpointerup` + right/middle-click filter + `touch-action:manipulation`. Desktop hover padding restored via `@media (hover:hover)` so mouse users are unaffected. (`static/sessions.js`, `static/style.css`) [#1110 @sheng-di] +- **Pasted/dragged images render inline** — image attachments now show as `` with click-to-fullscreen instead of a paperclip badge. Hoisted `_IMAGE_EXTS` to module scope (was causing `ReferenceError` in `renderMessages`); added `avif` support. (`static/ui.js`) [#1109 @bergeouss] Closes #1095 +- **Copy buttons on HTTP** — `_copyText()` helper checks `isSecureContext` and falls back to `execCommand('copy')` for plain-HTTP self-hosted installs. Silent failure in `addCopyButtons` fixed with error feedback. All 6 locales get `copy_failed` key. (`static/ui.js`, `static/i18n.js`) [#1107 @bergeouss] Closes #1096 + + ## v0.50.220 — 2026-04-26 ### Fixed diff --git a/ROADMAP.md b/ROADMAP.md index 43a407ba..540bfc0a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -3,7 +3,7 @@ > Goal: Full 1:1 parity with the Hermes CLI experience via a clean dark web UI. > Everything you can do from the CLI terminal, you can do from this UI. > -> Last updated: v0.50.220 (April 26, 2026) — 2481 tests collected +> Last updated: v0.50.221 (April 26, 2026) — 2511 tests collected > Tests: 2107 collected (`pytest tests/ --collect-only -q`) > Source: / diff --git a/TESTING.md b/TESTING.md index f58667de..fe30f4a0 100644 --- a/TESTING.md +++ b/TESTING.md @@ -8,7 +8,7 @@ > Prerequisites: SSH tunnel is active on port 8787. Open http://localhost:8787 in browser. > Server health check: curl http://127.0.0.1:8787/health should return {"status":"ok"}. > -> Automated coverage: 2481 tests collected via `pytest tests/ --collect-only -q`. Includes onboarding coverage for bootstrap/static wizard presence, real provider config persistence (`config.yaml` + `.env`), the `/api/onboarding/*` backend, the onboarding skip/existing-config guard, and CSS regression coverage for smooth thinking/tool card disclosure animation. +> Automated coverage: 2511 tests collected via `pytest tests/ --collect-only -q`. Includes onboarding coverage for bootstrap/static wizard presence, real provider config persistence (`config.yaml` + `.env`), the `/api/onboarding/*` backend, the onboarding skip/existing-config guard, and CSS regression coverage for smooth thinking/tool card disclosure animation. > Run: `pytest tests/ -v --timeout=60` > > Local regression focus: verify that a previously closed workspace panel stays visually closed from first paint through boot completion on desktop refresh; there should be no brief open-then-close flash. diff --git a/api/config.py b/api/config.py index 1ad055ce..e6021a1b 100644 --- a/api/config.py +++ b/api/config.py @@ -1501,6 +1501,20 @@ def get_available_models() -> dict: import socket + # Build set of hostnames from custom_providers config — these are + # user-explicitly configured endpoints and should not be blocked by SSRF. + _ssrf_trusted_hosts: set[str] = set() + _custom_providers_cfg = cfg.get("custom_providers", []) + if isinstance(_custom_providers_cfg, list): + for _cp in _custom_providers_cfg: + if not isinstance(_cp, dict): + continue + _cp_base = (_cp.get("base_url") or "").strip() + if _cp_base: + _cp_parsed = urlparse(_cp_base if "://" in _cp_base else f"http://{_cp_base}") + if _cp_parsed.hostname: + _ssrf_trusted_hosts.add(_cp_parsed.hostname.lower()) + parsed_url = urlparse( endpoint_url if "://" in endpoint_url else f"http://{endpoint_url}" ) @@ -1521,7 +1535,7 @@ def get_available_models() -> dict: "lmstudio", "lm-studio", ) - ) + ) or (parsed_url.hostname or "").lower() in _ssrf_trusted_hosts if not is_known_local: raise ValueError( f"SSRF: resolved hostname to private IP {addr[0]}" @@ -1564,22 +1578,34 @@ def get_available_models() -> dict: for _cp in _custom_providers_cfg: if not isinstance(_cp, dict): continue - _cp_model = _cp.get("model", "") _cp_name = (_cp.get("name") or "").strip() - if _cp_model and _cp_model not in _seen_custom_ids: - _cp_label = _get_label_for_model(_cp_model, []) - _seen_custom_ids.add(_cp_model) - if _cp_name: - _slug = "custom:" + _cp_name.lower().replace(" ", "-") - if _slug not in _named_custom_groups: - _named_custom_groups[_slug] = (_cp_name, []) + _slug = ("custom:" + _cp_name.lower().replace(" ", "-")) if _cp_name else None + + # Collect model IDs: singular "model" field first, then "models" dict keys + _cp_model_ids: list[str] = [] + _cp_model = _cp.get("model", "") + if _cp_model: + _cp_model_ids.append(_cp_model) + _cp_models_dict = _cp.get("models") + if isinstance(_cp_models_dict, dict): + for _m_id in _cp_models_dict: + if isinstance(_m_id, str) and _m_id.strip() and _m_id not in _cp_model_ids: + _cp_model_ids.append(_m_id.strip()) + + for _cp_model in _cp_model_ids: + if _cp_model and _cp_model not in _seen_custom_ids: + _cp_label = _get_label_for_model(_cp_model, []) + _seen_custom_ids.add(_cp_model) + if _slug: + if _slug not in _named_custom_groups: + _named_custom_groups[_slug] = (_cp_name, []) detected_providers.add(_slug) - _named_custom_groups[_slug][1].append( - {"id": _cp_model, "label": _cp_label} - ) - else: - auto_detected_models.append({"id": _cp_model, "label": _cp_label}) - detected_providers.add("custom") + _named_custom_groups[_slug][1].append( + {"id": _cp_model, "label": _cp_label} + ) + else: + auto_detected_models.append({"id": _cp_model, "label": _cp_label}) + detected_providers.add("custom") _has_custom_providers = isinstance(_custom_providers_cfg, list) and len(_custom_providers_cfg) > 0 if active_provider and active_provider != "custom" and not _has_custom_providers: diff --git a/static/i18n.js b/static/i18n.js index 58675e1b..8b8b7331 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -24,6 +24,7 @@ const LOCALES = { regenerate: 'Regenerate response', copy: 'Copy', copied: 'Copied!', + copy_failed: 'Copy failed', you: 'You', thinking: 'Thinking', expand_all: 'Expand all', @@ -668,6 +669,7 @@ const LOCALES = { regenerate: 'Сгенерировать ответ заново', copy: 'Копировать', copied: 'Скопировано!', + copy_failed: '\u041e\u0448\u0438\u0431\u043a\u0430 \u043a\u043e\u043f\u0438\u0440\u043e\u0432\u0430\u043d\u0438\u044f', you: 'Вы', thinking: 'Думаю', expand_all: 'Развернуть всё', @@ -1261,6 +1263,7 @@ const LOCALES = { regenerate: 'Regenerar respuesta', copy: 'Copiar', copied: '¡Copiado!', + copy_failed: 'Error al copiar', you: 'Tú', thinking: 'Pensando', expand_all: 'Expandir todo', @@ -1846,6 +1849,7 @@ const LOCALES = { regenerate: 'Antwort regenerieren', copy: 'Kopieren', copied: 'Kopiert!', + copy_failed: 'Kopieren fehlgeschlagen', you: 'Du', thinking: 'Nachdenken', expand_all: 'Alle ausklappen', @@ -2212,6 +2216,7 @@ const LOCALES = { regenerate: '\u91cd\u65b0\u751f\u6210\u56de\u590d', copy: '\u590d\u5236', copied: '\u5df2\u590d\u5236', + copy_failed: '\u590d\u5236\u5931\u8d25', you: '\u4f60', thinking: '\u601d\u8003\u8fc7\u7a0b', expand_all: '\u5168\u90e8\u5c55\u5f00', @@ -2795,6 +2800,7 @@ const LOCALES = { regenerate: '\u91cd\u65b0\u751f\u6210\u56de\u8986', copy: '\u8907\u88fd', copied: '\u5df2\u8907\u88fd', + copy_failed: '\u8907\u88fd\u5931\u6557', you: '\u4f60', thinking: '\u601d\u8003\u904e\u7a0b', expand_all: '\u5168\u90e8\u5c55\u958b', diff --git a/static/sessions.js b/static/sessions.js index 49bec951..a8b74729 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -1030,16 +1030,32 @@ function renderSessionListFromCache(){ actions.appendChild(menuBtn); el.appendChild(actions); - // Use a click timer to distinguish single-click (navigate) from double-click (rename). - // This prevents loadSession from firing on the first click of a double-click, - // which would re-render the list and destroy the dblclick target before it fires. - let _clickTimer=null; - el.onclick=async(e)=>{ - if(_renamingSid) return; // ignore while any rename is active + // Use pointerup + manual double-tap detection instead of onclick/ondblclick. + // onclick/ondblclick are unreliable on touch devices (iPad Safari especially): + // hover-triggered layout shifts, ghost clicks, and 300ms delay all break + // single-tap navigation. pointerup fires immediately on both mouse & touch. + let _lastTapTime=0; + let _tapTimer=null; + el.onpointerup=(e)=>{ + if(e.pointerType==='mouse' && e.button!==0) return; // ignore right/middle click + if(_renamingSid) return; if(actions.contains(e.target)) return; - clearTimeout(_clickTimer); - _clickTimer=setTimeout(async()=>{ - _clickTimer=null; + const now=Date.now(); + if(now-_lastTapTime<350){ + // Double-tap: rename + clearTimeout(_tapTimer); + _tapTimer=null; + _lastTapTime=0; + startRename(); + return; + } + _lastTapTime=now; + // Single tap: wait to ensure it's not the first of a double-tap, + // then navigate + clearTimeout(_tapTimer); + _tapTimer=setTimeout(async()=>{ + _tapTimer=null; + _lastTapTime=0; if(_renamingSid) return; // For CLI sessions, import into WebUI store first (idempotent) if(s.is_cli_session){ @@ -1049,14 +1065,7 @@ function renderSessionListFromCache(){ } await loadSession(s.session_id);renderSessionListFromCache(); if(typeof closeMobileSidebar==='function')closeMobileSidebar(); - }, 220); - }; - el.ondblclick=async(e)=>{ - e.stopPropagation(); - e.preventDefault(); - clearTimeout(_clickTimer); // cancel the pending single-click navigation - _clickTimer=null; - startRename(); + }, 300); }; return el; } diff --git a/static/style.css b/static/style.css index 8ea1601f..a3fab38a 100644 --- a/static/style.css +++ b/static/style.css @@ -242,9 +242,14 @@ and attention indicator (26x26 at right:6px) still need 40px reserved when they're visible — covered by the hover / streaming / unread / menu-open / focus-within rule below. */ - .session-item{padding:8px 8px;margin-bottom:2px;border-radius:8px;cursor:pointer;font-size:13px;color:var(--muted);transition:background .15s,color .15s;display:flex;align-items:flex-start;gap:8px;min-width:0;position:relative;} - .session-item.streaming,.session-item.unread,.session-item:hover,.session-item:focus-within,.session-item.menu-open{padding-right:40px;} + .session-item{padding:8px 8px;margin-bottom:2px;border-radius:8px;cursor:pointer;font-size:13px;color:var(--muted);transition:background .15s,color .15s;display:flex;align-items:flex-start;gap:8px;min-width:0;position:relative;touch-action:manipulation;-webkit-tap-highlight-color:transparent;} + .session-item.streaming,.session-item.unread,.session-item:focus-within,.session-item.menu-open{padding-right:40px;} .session-item:hover{background:var(--hover-bg);color:var(--text);} + /* Restore hover padding-right only for mouse (hover:hover) devices. + Touch/iPad (hover:none) must NOT expand padding-right on :hover — + the expansion causes a layout-reflow mid-tap that moves session-actions + under the finger, triggering stopPropagation and swallowing navigation. */ + @media (hover:hover){.session-item:hover{padding-right:40px;}} .session-item.active{background:var(--accent-bg);color:var(--accent);} .session-item.streaming .session-title{color:var(--accent);} .session-item.streaming .session-title-row{color:var(--text);} diff --git a/static/ui.js b/static/ui.js index 08e5ac94..f7b2cc5b 100644 --- a/static/ui.js +++ b/static/ui.js @@ -50,6 +50,7 @@ function _setCompressionSessionLock(sid){ window._compressionLockSid=sid||null; } const esc=s=>String(s??'').replace(/[&<>"']/g,c=>({'&':'&','<':'<','>':'>','"':'"',"'":'''}[c])); +const _IMAGE_EXTS=/\.(png|jpg|jpeg|gif|webp|bmp|ico|avif)$/i; // Dynamic model labels -- populated by populateModelDropdown(), fallback to static map let _dynamicModelLabels={}; @@ -747,7 +748,6 @@ function renderMd(raw){ // Detect MEDIA: tokens emitted by the agent (e.g. screenshots, // generated images) and replace them with inline or download links. // Stashed so the path/URL is never processed as markdown. - const _IMAGE_EXTS=/\.(png|jpg|jpeg|gif|webp|bmp|ico)$/i; const media_stash=[]; s=s.replace(/MEDIA:([^\s\)\]]+)/g,(_,raw_ref)=>{ media_stash.push(raw_ref); @@ -1526,14 +1526,27 @@ function showPromptDialog(opts={}){ } +function _copyText(text){ + if(navigator.clipboard && window.isSecureContext){ + return navigator.clipboard.writeText(text); + } + return new Promise((resolve,reject)=>{ + const ta=document.createElement('textarea'); + ta.value=text;ta.style.cssText='position:fixed;left:-9999px;top:-9999px;opacity:0'; + document.body.appendChild(ta);ta.select(); + try{document.execCommand('copy');resolve();} + catch(e){reject(e);} + finally{document.body.removeChild(ta);} + }); +} function copyMsg(btn){ const row=btn.closest('[data-raw-text]'); const text=row?row.dataset.rawText:''; if(!text)return; - navigator.clipboard.writeText(text).then(()=>{ + _copyText(text).then(()=>{ const orig=btn.innerHTML;btn.innerHTML=li('check',13);btn.style.color='var(--blue)'; setTimeout(()=>{btn.innerHTML=orig;btn.style.color='';},1500); - }).catch(()=>showToast('Copy failed')); + }).catch(()=>showToast(t('copy_failed'))); } // ── Reconnect banner (B4/B5: reload resilience) ── @@ -2235,7 +2248,14 @@ function renderMessages(){ const isLastAssistant=!isUser&&vi===visWithIdx.length-1; let filesHtml=''; if(m.attachments&&m.attachments.length){ - filesHtml=`
${m.attachments.map(f=>`
${li('paperclip',12)} ${esc(f)}
`).join('')}
`; + filesHtml=`
${m.attachments.map(f=>{ + const fname=f.split('/').pop()||f; + if(_IMAGE_EXTS.test(fname)){ + const imgUrl='api/media?path='+encodeURIComponent(f); + return `${esc(fname)}`; + } + return `
${li('paperclip',12)} ${esc(fname)}
`; + }).join('')}
`; } const bodyHtml = isUser ? esc(String(content)).replace(/\n/g,'
') : renderMd(_stripXmlToolCallsDisplay(String(content))); const isEditableUser=isUser&&rawIdx===lastUserRawIdx; @@ -2745,10 +2765,10 @@ function addCopyButtons(container){ btn.textContent=t('copy'); btn.onclick=(e)=>{ e.stopPropagation(); - navigator.clipboard.writeText(codeEl.textContent).then(()=>{ + _copyText(codeEl.textContent).then(()=>{ btn.textContent=t('copied'); setTimeout(()=>{btn.textContent=t('copy');},1500); - }); + }).catch(()=>{btn.textContent=t('copy_failed');setTimeout(()=>{btn.textContent=t('copy');},1500);}); }; const header=pre.previousElementSibling; if(header&&header.classList.contains('pre-header')){ diff --git a/tests/test_issue1095_pasted_images.py b/tests/test_issue1095_pasted_images.py new file mode 100644 index 00000000..709d7b90 --- /dev/null +++ b/tests/test_issue1095_pasted_images.py @@ -0,0 +1,52 @@ +"""Tests for #1095 — pasted images render as inline previews, not paperclip badges.""" +import os +import re +import pytest + + +def _read_js(name): + with open(os.path.join('static', name)) as f: + return f.read() + + +class TestAttachmentImageRendering: + """User message attachments with image extensions should render as , not paperclip badges.""" + + def test_attachments_block_uses_image_check(self): + ui = _read_js('ui.js') + # Find the attachments rendering block + assert 'm.attachments' in ui + # Must check file extension before rendering + assert '_IMAGE_EXTS.test(' in ui, '_IMAGE_EXTS not used in attachment rendering' + + def test_image_attachments_use_img_tag(self): + """Image attachments should produce with api/media?path=, not paperclip badge.""" + ui = _read_js('ui.js') + # Find the attachments section + m = re.search(r"m\.attachments&&m\.attachments\.length", ui) + assert m, 'attachments rendering block not found' + body = ui[m.start():m.start() + 1000] + # Should have img tag with api/media + assert 'msg-media-img' in body, 'attachments must render images with msg-media-img class' + assert 'api/media?path=' in body, 'image attachments must use api/media endpoint' + + def test_non_image_attachments_keep_paperclip(self): + """Non-image attachments must still show paperclip badge.""" + ui = _read_js('ui.js') + m = re.search(r"m\.attachments&&m\.attachments\.length", ui) + body = ui[m.start():m.start() + 1000] + assert "msg-file-badge" in body, 'non-image attachments must still use paperclip badge' + + def test_image_click_to_full(self): + """Inline image attachments should support click-to-fullscreen (toggle class).""" + ui = _read_js('ui.js') + m = re.search(r"m\.attachments&&m\.attachments\.length", ui) + body = ui[m.start():m.start() + 1000] + assert "msg-media-img--full" in body, 'image attachments should toggle full-screen on click' + + def test_uses_filename_not_full_path(self): + """Non-image badge should display filename, not full path.""" + ui = _read_js('ui.js') + m = re.search(r"m\.attachments&&m\.attachments\.length", ui) + body = ui[m.start():m.start() + 1000] + assert ".split('/').pop()" in body, 'should extract filename from path for display' diff --git a/tests/test_issue1096_copy_buttons.py b/tests/test_issue1096_copy_buttons.py new file mode 100644 index 00000000..048bcb96 --- /dev/null +++ b/tests/test_issue1096_copy_buttons.py @@ -0,0 +1,100 @@ +"""Tests for #1096 — copy buttons work without HTTPS (execCommand fallback).""" +import os +import re +import pytest + + +# ── Helpers ───────────────────────────────────────────────────────────────────── + +def _read_js(name): + path = os.path.join('static', name) + with open(path) as f: + return f.read() + + +def _read_i18n(): + return _read_js('i18n.js') + + +def _read_ui(): + return _read_js('ui.js') + + +# ── _copyText helper ──────────────────────────────────────────────────────────── + +class TestCopyTextHelperExists: + """_copyText() helper must exist with secure context check + execCommand fallback.""" + + def test_function_exists(self): + ui = _read_ui() + assert 'function _copyText(' in ui, '_copyText helper missing from ui.js' + + def test_uses_secure_context_check(self): + ui = _read_ui() + assert 'isSecureContext' in ui, 'isSecureContext check missing — fallback wont trigger on HTTP' + + def test_uses_exec_command_fallback(self): + ui = _read_ui() + assert 'execCommand' in ui, 'execCommand fallback missing — copy fails on HTTP' + + +# ── copyMsg() uses helper ────────────────────────────────────────────────────── + +class TestCopyMsgUsesHelper: + + def test_copy_msg_exists(self): + ui = _read_ui() + assert 'function copyMsg(' in ui + + def test_copy_msg_calls_helper_not_clipboard_directly(self): + """copyMsg must use _copyText, not navigator.clipboard.writeText.""" + ui = _read_ui() + # Find copyMsg function body + m = re.search(r'function copyMsg\(', ui) + assert m, 'copyMsg function not found' + body = ui[m.start():m.start() + 600] + assert '_copyText(' in body, 'copyMsg must call _copyText helper' + assert 'navigator.clipboard.writeText' not in body, 'copyMsg must NOT call navigator.clipboard directly' + + def test_copy_msg_has_i18n_error(self): + """Error message must use t() for i18n, not hardcoded string.""" + ui = _read_ui() + m = re.search(r'function copyMsg\(', ui) + body = ui[m.start():m.start() + 600] + assert "t('copy_failed')" in body, 'copyMsg error must use t("copy_failed") not hardcoded string' + + +# ── Code block copy uses helper ──────────────────────────────────────────────── + +class TestCodeBlockCopyUsesHelper: + + def test_add_copy_buttons_exists(self): + ui = _read_ui() + assert 'function addCopyButtons(' in ui + + def test_code_copy_calls_helper(self): + """addCopyButtons must use _copyText, not navigator.clipboard directly.""" + ui = _read_ui() + m = re.search(r'function addCopyButtons\(', ui) + assert m, 'addCopyButtons function not found' + body = ui[m.start():m.start() + 2000] + assert '_copyText(' in body, 'addCopyButtons must call _copyText helper' + assert 'navigator.clipboard.writeText' not in body, 'addCopyButtons must NOT call navigator.clipboard directly' + + def test_code_copy_has_catch_handler(self): + """Code block copy must have .catch() — previously had none (silent failure).""" + ui = _read_ui() + m = re.search(r'function addCopyButtons\(', ui) + body = ui[m.start():m.start() + 2000] + assert '.catch(' in body, 'Code block copy button has no .catch() handler' + + +# ── i18n ──────────────────────────────────────────────────────────────────────── + +class TestCopyFailedI18n: + + def test_copy_failed_in_all_locales(self): + """copy_failed key must exist in all 6 locale blocks.""" + i18n = _read_i18n() + count = i18n.count('copy_failed') + assert count == 6, f'Expected copy_failed in 6 locale blocks, found {count}' diff --git a/tests/test_issue1105_ssrf_custom_providers.py b/tests/test_issue1105_ssrf_custom_providers.py new file mode 100644 index 00000000..12fbf6d4 --- /dev/null +++ b/tests/test_issue1105_ssrf_custom_providers.py @@ -0,0 +1,148 @@ +"""Tests for #1105 — SSRF check allows user-configured custom_providers hostnames. + +The SSRF check blocks requests to private IPs unless the hostname is in a +hardcoded allowlist. This fix extracts hostnames from custom_providers config +and adds them to the trusted set, so user-explicitly configured local endpoints +(ollama, llama.cpp, vLLM, TabbyAPI, etc.) are not blocked. +""" +import os +import pytest + + +# ---------- Source-code analysis tests ---------- + +def test_ssrf_trusted_hosts_variable_exists(): + """The _ssrf_trusted_hosts set must be built from custom_providers config.""" + with open("api/config.py") as f: + src = f.read() + assert "_ssrf_trusted_hosts" in src + assert "_ssrf_trusted_hosts: set[str] = set()" in src + + +def test_ssrf_trusted_hosts_populated_from_custom_providers(): + """Trusted hosts are extracted by iterating custom_providers[].base_url.""" + with open("api/config.py") as f: + src = f.read() + # Must read custom_providers from cfg + assert 'cfg.get("custom_providers"' in src + # Must extract base_url from each entry + assert '_cp.get("base_url")' in src + # Must parse hostname with urlparse + assert "_cp_parsed.hostname" in src + # Must add to trusted set + assert "_ssrf_trusted_hosts.add" in src + + +def test_ssrf_check_uses_trusted_hosts(): + """The SSRF check must consult _ssrf_trusted_hosts before blocking.""" + with open("api/config.py") as f: + src = f.read() + # The is_known_local check must include _ssrf_trusted_hosts + assert "in _ssrf_trusted_hosts" in src + + +def test_ssrf_known_local_still_present(): + """Original hardcoded allowlist must still be present (no regression).""" + with open("api/config.py") as f: + src = f.read() + for keyword in ("ollama", "localhost", "127.0.0.1", "lmstudio", "lm-studio"): + assert keyword in src, f"Missing hardcoded allowlist entry: {keyword}" + + +def test_ssrf_block_still_present(): + """SSRF ValueError must still be raised for unknown private IPs.""" + with open("api/config.py") as f: + src = f.read() + assert 'SSRF: resolved hostname to private IP' in src + + +# ---------- Functional tests (mocked socket) ---------- + +def test_custom_provider_hostname_added_to_trusted(): + """A hostname from custom_providers base_url is added to trusted set.""" + import api.config as config + import socket + from unittest.mock import patch, MagicMock + + old_cfg = dict(config.cfg) + try: + config.cfg.update({ + "model": {"model": "my-model", "base_url": "http://my-llama-server:8080/v1"}, + "custom_providers": [ + {"name": "my-llama", "base_url": "http://my-llama-server:8080/v1", "model": "llama-3"} + ], + "providers": {}, + }) + config.invalidate_models_cache() + + # Mock socket.getaddrinfo to return a private IP for my-llama-server + private_addr = ("192.168.1.100", None) + mock_getaddrinfo = MagicMock(return_value=[ + (socket.AF_INET, socket.SOCK_STREAM, 6, "", private_addr) + ]) + + # Mock urllib to prevent actual HTTP call + mock_urlopen = MagicMock() + mock_urlopen.read.return_value = b'{"data": [{"id": "test-model"}]}' + mock_urlopen.__enter__ = MagicMock(return_value=mock_urlopen) + mock_urlopen.__exit__ = MagicMock(return_value=False) + + with patch("socket.getaddrinfo", mock_getaddrinfo), \ + patch("urllib.request.urlopen", mock_urlopen): + # Should NOT raise ValueError (SSRF) because hostname is in trusted set + result = config.get_available_models() + + # Verify models were returned (auto-detection succeeded) + assert result is not None + assert "groups" in result + + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config.invalidate_models_cache() + + +def test_unknown_private_ip_still_blocked(): + """A private IP from a hostname NOT in custom_providers is still blocked. + + The SSRF ValueError is caught by the broad `except Exception` around + the custom endpoint fetch (line ~1571), so get_available_models() doesn't + crash — but no models are auto-detected from that endpoint. + """ + import api.config as config + import socket + from unittest.mock import patch, MagicMock + + old_cfg = dict(config.cfg) + try: + config.cfg.update({ + "model": {"model": "test", "base_url": "http://unknown-local-server:9999/v1"}, + "custom_providers": [ + {"name": "other", "base_url": "http://other-server:8080/v1", "model": "x"} + ], + "providers": {}, + }) + config.invalidate_models_cache() + + # Mock socket.getaddrinfo to return a private IP for unknown-local-server + private_addr = ("10.0.0.50", None) + mock_getaddrinfo = MagicMock(return_value=[ + (socket.AF_INET, socket.SOCK_STREAM, 6, "", private_addr) + ]) + + with patch("socket.getaddrinfo", mock_getaddrinfo): + # Should NOT crash (ValueError is caught internally) + result = config.get_available_models() + + # But no models should be auto-detected from the blocked endpoint + assert result is not None + assert "groups" in result + # Verify no group with "unknown-local-server" models exists + for group in result["groups"]: + provider_name = group.get("provider", "") + assert "unknown-local-server" not in provider_name + + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config.invalidate_models_cache() diff --git a/tests/test_issue1106_custom_providers_models.py b/tests/test_issue1106_custom_providers_models.py new file mode 100644 index 00000000..6e34e7fa --- /dev/null +++ b/tests/test_issue1106_custom_providers_models.py @@ -0,0 +1,198 @@ +"""Tests for #1106 — custom_providers[].models dict keys populate model dropdown.""" +import pytest +import api.config as config + + +def _reset(): + try: + config.invalidate_models_cache() + except Exception: + pass + + +def _models_with_cfg(model_cfg=None, custom_providers=None, active_provider=None): + """Temporarily patch config.cfg, call get_available_models(), restore. + + Also pins _cfg_mtime to prevent reload_config() from overwriting patches. + """ + old_cfg = dict(config.cfg) + old_mtime = config._cfg_mtime + config.cfg.clear() + if model_cfg: + config.cfg["model"] = model_cfg + if custom_providers is not None: + config.cfg["custom_providers"] = custom_providers + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + try: + return config.get_available_models() + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config._cfg_mtime = old_mtime + + +def _all_model_ids(result): + """Extract all model IDs from all groups.""" + ids = [] + for g in result.get("groups", []): + for m in g.get("models", []): + ids.append(m["id"]) + return ids + + +def _group_for(result, provider_name): + """Get a group by provider name.""" + for g in result.get("groups", []): + if g.get("provider") == provider_name: + return g + return None + + +class TestCustomProvidersModelsDict: + """custom_providers entries with a 'models' dict should populate all keys in the dropdown.""" + + def test_models_dict_keys_appear_in_dropdown(self): + """Each key in custom_providers[].models should appear as a selectable model.""" + result = _models_with_cfg( + model_cfg={"provider": "custom"}, + custom_providers=[ + { + "name": "Llama-swap", + "base_url": "http://llama-swap:8880/v1", + "model": "unsloth-qwen3.6-35b-a3b", + "models": { + "unsloth-qwen3.6-35b-a3b": {"context_length": 262144}, + "gemma4-26b": {}, + "qwen3.5-27b": {}, + "qwen3-coder-30b": {}, + }, + } + ], + ) + ids = _all_model_ids(result) + for expected in ["unsloth-qwen3.6-35b-a3b", "gemma4-26b", "qwen3.5-27b", "qwen3-coder-30b"]: + assert expected in ids, f"Expected '{expected}' in model IDs, got {ids}" + + def test_models_dict_without_model_field_still_works(self): + """If only 'models' dict is present (no singular 'model'), all dict keys should appear.""" + result = _models_with_cfg( + model_cfg={"provider": "custom"}, + custom_providers=[ + { + "name": "Local-LLM", + "base_url": "http://localhost:8080/v1", + "models": { + "llama-3-8b": {}, + "mistral-7b": {}, + }, + } + ], + ) + ids = _all_model_ids(result) + assert "llama-3-8b" in ids + assert "mistral-7b" in ids + + def test_no_duplicates_when_model_and_models_overlap(self): + """If 'model' value also appears in 'models' dict, it should not be duplicated.""" + result = _models_with_cfg( + model_cfg={"provider": "custom"}, + custom_providers=[ + { + "name": "MyServer", + "base_url": "http://myserver:8000/v1", + "model": "base-model", + "models": { + "base-model": {}, + "other-model": {}, + }, + } + ], + ) + ids = _all_model_ids(result) + assert ids.count("base-model") == 1, f"'base-model' should appear exactly once, got {ids.count('base-model')}" + assert "other-model" in ids + + def test_unnamed_provider_models_dict_works(self): + """custom_providers without 'name' should still populate 'Custom' group.""" + result = _models_with_cfg( + model_cfg={"provider": "custom"}, + custom_providers=[ + { + "model": "my-model", + "models": { + "extra-model-a": {}, + "extra-model-b": {}, + }, + } + ], + ) + ids = _all_model_ids(result) + for expected in ["my-model", "extra-model-a", "extra-model-b"]: + assert expected in ids, f"Expected '{expected}' in model IDs, got {ids}" + + def test_empty_models_dict_is_ignored(self): + """An empty 'models' dict should not break anything.""" + result = _models_with_cfg( + model_cfg={"provider": "custom"}, + custom_providers=[ + { + "name": "TestServer", + "model": "only-model", + "models": {}, + } + ], + ) + ids = _all_model_ids(result) + assert "only-model" in ids + + def test_non_string_models_keys_are_skipped(self): + """Non-string keys in models dict should be silently skipped.""" + result = _models_with_cfg( + model_cfg={"provider": "custom"}, + custom_providers=[ + { + "name": "TestServer", + "model": "valid-model", + "models": { + "another-valid": {}, + 123: {}, # non-string key + None: {}, # non-string key + }, + } + ], + ) + ids = _all_model_ids(result) + assert "valid-model" in ids + assert "another-valid" in ids + + def test_multiple_custom_providers_each_keep_models_separate(self): + """Multiple named custom_providers should each have their own models.""" + result = _models_with_cfg( + model_cfg={"provider": "custom"}, + custom_providers=[ + { + "name": "Server-A", + "model": "model-a1", + "models": {"model-a2": {}}, + }, + { + "name": "Server-B", + "model": "model-b1", + "models": {"model-b2": {}}, + }, + ], + ) + group_a = _group_for(result, "Server-A") + group_b = _group_for(result, "Server-B") + assert group_a is not None, "Server-A group missing" + assert group_b is not None, "Server-B group missing" + ids_a = [m["id"] for m in group_a["models"]] + ids_b = [m["id"] for m in group_b["models"]] + assert "model-a1" in ids_a and "model-a2" in ids_a + assert "model-b1" in ids_b and "model-b2" in ids_b + # No cross-contamination + assert "model-b1" not in ids_a + assert "model-a1" not in ids_b diff --git a/tests/test_issue856_pinned_indicator_layout.py b/tests/test_issue856_pinned_indicator_layout.py index c46adf37..2609857b 100644 --- a/tests/test_issue856_pinned_indicator_layout.py +++ b/tests/test_issue856_pinned_indicator_layout.py @@ -90,7 +90,14 @@ def test_timestamp_hidden_when_attention_state_is_present(): # focus-within all expand to 40px to make room for the absolute action # button + attention indicator. assert ".session-item{padding:8px 8px;" in STYLE_CSS - assert ".session-item.streaming,.session-item.unread,.session-item:hover,.session-item:focus-within,.session-item.menu-open{padding-right:40px;}" in STYLE_CSS + # PR #1110: :hover removed from the COMBINED padding-right rule (touch layout-shift fix). + # Instead, hover padding is restored via @media (hover:hover) which only applies to + # devices with a real hover capability (mouse). Touch/iPad devices satisfy hover:none + # and skip that block, preventing the layout-reflow mid-tap bug. + assert ".session-item.streaming,.session-item.unread,.session-item:focus-within,.session-item.menu-open{padding-right:40px;}" in STYLE_CSS + # Desktop hover padding restored via media query (mouse devices only) + assert "@media (hover:hover)" in STYLE_CSS + assert ".session-item:hover{padding-right:40px;}" in STYLE_CSS assert ".session-item{min-height:44px;padding:10px 40px 10px 12px;}" in STYLE_CSS # Timestamp now uses margin-left:auto inside the flex row instead of # absolute positioning. This stops the title's flex:1 bound from running @@ -142,3 +149,14 @@ def test_apperror_path_calls_render_session_list(): "apperror handler must call renderSessionList() so the streaming indicator " "clears immediately on server errors, not after a 5s poll delay" ) + + +def test_pointerup_ignores_non_primary_mouse_buttons(): + """Right-click and middle-click must not trigger session navigation. + onpointerup fires for all mouse buttons; we filter to button===0 + (primary). pointerType==='mouse' scopes the check to mouse only — + touch/stylus always report button===0 so they're unaffected.""" + assert "e.pointerType==='mouse' && e.button!==0" in SESSIONS_JS, ( + "pointerup handler must filter out non-primary mouse buttons " + "(right-click / middle-click must not navigate)" + ) diff --git a/tests/test_workspace_panel_session_list.py b/tests/test_workspace_panel_session_list.py index 57f1d6fc..95f7d2d5 100644 --- a/tests/test_workspace_panel_session_list.py +++ b/tests/test_workspace_panel_session_list.py @@ -220,18 +220,23 @@ class TestProjectDotPlacement: assert ".session-item{min-height:44px;padding:10px 40px 10px 12px;}" in STYLE_CSS def test_session_item_expands_padding_on_hover_and_attention(self): - """When hover/focus/menu-open/streaming/unread shows the action - button or attention indicator, padding-right expands to 40px to - reserve space for them (they're position:absolute at right:6px, - 26px wide → 32px footprint, 40px gives 8px breathing).""" + """PR #1110: Touch layout-shift fix — :hover removed from the COMBINED + padding-right selector. Touch devices (iPad, phone) see hover:none so + they skip the @media (hover:hover) block below. Mouse devices see + hover:hover and get the padding-right on hover. + streaming/unread/focus-within/menu-open expand to 40px for all devices.""" + # Touch-safe combined rule (no :hover in this one) sel = ( ".session-item.streaming,.session-item.unread," - ".session-item:hover,.session-item:focus-within," + ".session-item:focus-within," ".session-item.menu-open" ) idx = STYLE_CSS.find(sel) assert idx >= 0, ( - f"Combined hover/streaming/unread padding rule not found" + "Combined streaming/unread/focus-within/menu-open padding rule not found" ) rule = STYLE_CSS[idx: STYLE_CSS.find("}", idx)] assert "padding-right:40px" in rule + # Desktop hover padding restored via @media (hover:hover) — mouse devices only + assert "@media (hover:hover)" in STYLE_CSS + assert ".session-item:hover{padding-right:40px;}" in STYLE_CSS