fix: batch v0.50.234-235 — XSS hardening, workspace validation, profile switch fixes (#1206)
Some checks failed
Release & Docker / release (push) Has been cancelled
Some checks failed
Release & Docker / release (push) Has been cancelled
fix: batch v0.50.234-235 — XSS hardening, workspace validation, profile switch fixes v0.50.235 (#1203 — profile switch workspace/model/chip, 3 bugs + flaky test): - switch_profile now reads target profile's workspace directly (thread-local bypass) - invalidate_models_cache() after profile switch (model dropdown staleness) - syncTopbar() updates chip before early-return (no-session path) v0.50.234 (#1201/#1205 — XSS hardening + workspace security): - renderMd() full HTML attribute sanitizer replacing tag-name-only allowlist - Delegated image lightbox (removes all inline onclick) - macOS /etc → /private/etc symlink bypass fixed - /System /Library added to blocked workspace roots - Legacy /api/chat workspace trust gap closed Both PRs independently reviewed. 2787/2787 tests. QA harness 20/20 + 11/11 API checks. Co-authored-by: Brendan Schmid <bschmidy10@Wilson.bschmidy10> Co-authored-by: Nathan Esquenazi <nesquena@gmail.com>
This commit is contained in:
55
CHANGELOG.md
55
CHANGELOG.md
@@ -33,6 +33,52 @@
|
||||
`tests/test_issue926_hindsight_docker_dependency.py`) Closes #926.
|
||||
|
||||
|
||||
## [v0.50.235] — 2026-04-28
|
||||
|
||||
### Fixed
|
||||
- **Profile switch shows correct workspace, model, and chip label immediately** — Three separate
|
||||
bugs caused profile switching to appear broken: (1) `switch_profile(process_wide=False)` returned
|
||||
the old profile's workspace because `get_last_workspace()` routed through thread-local profile
|
||||
context (still pointing at the old profile during the switch); (2) the model dropdown showed stale
|
||||
results because the in-memory models cache wasn't invalidated; (3) the profile chip stayed on the
|
||||
old name because `syncTopbar()` returned early without updating it when no session was active.
|
||||
(`api/profiles.py`, `api/routes.py`, `static/ui.js`,
|
||||
`tests/test_profile_switch_1200.py`) (PR #1203)
|
||||
- **Flaky test stabilisation** — `test_server_now_ms_compensates_positive_skew` used exact-ms
|
||||
equality across two `Date.now()` calls; fixed with midpoint averaging and ±5 ms tolerance.
|
||||
(`tests/test_issue1144_session_time_sync.py`)
|
||||
## [v0.50.234] — 2026-04-28
|
||||
|
||||
### Fixed
|
||||
- **XSS hardening in markdown renderer** — HTML tags in LLM output were filtered by
|
||||
tag name only, allowing event handlers like `onerror` and `onclick` to pass through
|
||||
on `<img>` and other elements. The sanitizer now strips all attributes except a
|
||||
per-tag allowlist and blocks `javascript:`, `data:`, and `vbscript:` URL schemes.
|
||||
Incomplete raw tags (`<img src=x onerror=...//` with no closing `>`) are escaped
|
||||
before paragraph wrapping so they cannot be completed by the renderer's own output.
|
||||
(`static/ui.js`)
|
||||
- **Delegated image lightbox** — inline `onclick` handlers on `<img class="msg-media-img">`
|
||||
replaced with a single delegated `document.addEventListener('click')`, eliminating the
|
||||
last source of inline event handler HTML in rendered output. (`static/ui.js`)
|
||||
- **Workspace trust for macOS symlink paths** — `/etc` on macOS resolves to `/private/etc`
|
||||
which previously bypassed the blocked-roots check. The new `_is_blocked_workspace_path`
|
||||
helper compares both the raw and resolved path. Also adds `/System` and `/Library` to
|
||||
the blocked roots. (`api/workspace.py`)
|
||||
- **Legacy `/api/chat` workspace validation** — the synchronous chat fallback endpoint
|
||||
was not routing through `resolve_trusted_workspace()`, allowing arbitrary paths to be
|
||||
set as workspace. (`api/routes.py`)
|
||||
- **`linked_files` type guard** — skill view responses with a `null` or non-dict
|
||||
`linked_files` field no longer crash the skills API. (`api/routes.py`)
|
||||
(by @bschmidy10, PR #1201)
|
||||
## [v0.50.233] — 2026-04-28
|
||||
|
||||
### Fixed
|
||||
- **Workspace trust for /var/home paths** — workspaces under `/var/home` (used by
|
||||
systemd-homed on Fedora/RHEL) were incorrectly blocked because `_is_blocked_system_path`
|
||||
flagged `/var` as a system root. The home-directory trust check in both
|
||||
`resolve_trusted_workspace` and `validate_workspace_to_add` now correctly trusts any
|
||||
path under `Path.home()` regardless of where the home directory lives on disk.
|
||||
(`api/workspace.py`) (by @frap129, PR #1199)
|
||||
## v0.50.225 — 2026-04-27
|
||||
|
||||
### Added
|
||||
@@ -357,15 +403,6 @@
|
||||
workspace subtree) and never enumerate blocked system roots. (`api/routes.py`,
|
||||
`api/workspace.py`, `static/panels.js`, `static/style.css`) (partial for #616)
|
||||
|
||||
## [v0.50.233] — 2026-04-28
|
||||
|
||||
### Fixed
|
||||
- **Workspace trust for /var/home paths** — workspaces under `/var/home` (used by
|
||||
systemd-homed on Fedora/RHEL) were incorrectly blocked because `_is_blocked_system_path`
|
||||
flagged `/var` as a system root. The home-directory trust check in both
|
||||
`resolve_trusted_workspace` and `validate_workspace_to_add` now correctly trusts any
|
||||
path under `Path.home()` regardless of where the home directory lives on disk.
|
||||
(`api/workspace.py`) (by @frap129, PR #1199)
|
||||
## [v0.50.232] — 2026-04-28
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -286,7 +286,6 @@ def switch_profile(name: str, *, process_wide: bool = True) -> dict:
|
||||
# For process_wide=False (per-client switch), read the target profile's
|
||||
# config.yaml directly from disk rather than from _cfg_cache (process-global),
|
||||
# since reload_config() was intentionally skipped.
|
||||
from api.workspace import get_last_workspace
|
||||
if process_wide:
|
||||
from api.config import get_config
|
||||
cfg = get_config()
|
||||
@@ -307,11 +306,57 @@ def switch_profile(name: str, *, process_wide: bool = True) -> dict:
|
||||
elif isinstance(model_cfg, dict):
|
||||
default_model = model_cfg.get('default')
|
||||
|
||||
# Read the target profile's workspace directly from *home* rather than via
|
||||
# get_last_workspace() which routes through the thread-local/process-global active
|
||||
# profile — both of which still point to the OLD profile during process_wide=False
|
||||
# switches (the Set-Cookie has been sent but hasn't been processed by a new request
|
||||
# yet). We derive workspace in priority order:
|
||||
# 1. {home}/webui_state/last_workspace.txt (previously chosen workspace for this profile)
|
||||
# 2. cfg terminal.cwd / workspace / default_workspace keys
|
||||
# 3. Boot-time DEFAULT_WORKSPACE constant
|
||||
# Use the module-level ``Path`` (imported at line 17) rather than re-importing
|
||||
# it locally — keeps the exception fallback simple and avoids a latent NameError
|
||||
# if a future refactor moves the inner imports.
|
||||
default_workspace = None
|
||||
try:
|
||||
from api.config import DEFAULT_WORKSPACE as _DW
|
||||
lw_file = home / 'webui_state' / 'last_workspace.txt'
|
||||
if lw_file.exists():
|
||||
_p = lw_file.read_text(encoding='utf-8').strip()
|
||||
if _p:
|
||||
_pp = Path(_p).expanduser()
|
||||
if _pp.is_dir():
|
||||
default_workspace = str(_pp.resolve())
|
||||
if default_workspace is None:
|
||||
for _key in ('workspace', 'default_workspace'):
|
||||
_v = cfg.get(_key)
|
||||
if _v:
|
||||
_pp = Path(str(_v)).expanduser().resolve()
|
||||
if _pp.is_dir():
|
||||
default_workspace = str(_pp)
|
||||
break
|
||||
if default_workspace is None:
|
||||
_tc = cfg.get('terminal', {})
|
||||
if isinstance(_tc, dict):
|
||||
_cwd = _tc.get('cwd', '')
|
||||
if _cwd and str(_cwd) not in ('.', ''):
|
||||
_pp = Path(str(_cwd)).expanduser().resolve()
|
||||
if _pp.is_dir():
|
||||
default_workspace = str(_pp)
|
||||
if default_workspace is None:
|
||||
default_workspace = str(_DW)
|
||||
except Exception:
|
||||
try:
|
||||
from api.config import DEFAULT_WORKSPACE as _DW2
|
||||
default_workspace = str(_DW2)
|
||||
except Exception:
|
||||
default_workspace = str(Path.home())
|
||||
|
||||
return {
|
||||
'profiles': list_profiles_api(),
|
||||
'active': name,
|
||||
'default_model': default_model,
|
||||
'default_workspace': get_last_workspace(),
|
||||
'default_workspace': default_workspace,
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -1144,7 +1144,7 @@ def handle_get(handler, parsed) -> bool:
|
||||
)
|
||||
raw = _skill_view(name)
|
||||
data = json.loads(raw) if isinstance(raw, str) else raw
|
||||
if "linked_files" not in data:
|
||||
if not isinstance(data.get("linked_files"), dict):
|
||||
data["linked_files"] = {}
|
||||
return j(handler, data)
|
||||
|
||||
@@ -1553,6 +1553,11 @@ def handle_post(handler, parsed) -> bool:
|
||||
# process_wide=False: don't mutate the process-global _active_profile.
|
||||
# Per-client profile is managed via cookie + thread-local (#798).
|
||||
result = switch_profile(name, process_wide=False)
|
||||
# Invalidate the models cache so the very next /api/models request
|
||||
# rebuilds from the new profile's config.yaml rather than returning
|
||||
# the old profile's cached model list (#1200 — profile-switch model bug).
|
||||
from api.config import invalidate_models_cache
|
||||
invalidate_models_cache()
|
||||
return j(handler, result, extra_headers={
|
||||
'Set-Cookie': build_profile_cookie(name),
|
||||
})
|
||||
@@ -2881,9 +2886,12 @@ def _handle_chat_sync(handler, body):
|
||||
msg = str(body.get("message", "")).strip()
|
||||
if not msg:
|
||||
return j(handler, {"error": "empty message"}, status=400)
|
||||
workspace = Path(body.get("workspace") or s.workspace).expanduser().resolve()
|
||||
try:
|
||||
workspace = str(resolve_trusted_workspace(body.get("workspace") or s.workspace))
|
||||
except ValueError as e:
|
||||
return bad(handler, str(e))
|
||||
with _get_session_agent_lock(s.session_id):
|
||||
s.workspace = str(workspace)
|
||||
s.workspace = workspace
|
||||
s.model = body.get("model") or s.model
|
||||
from api.streaming import _ENV_LOCK
|
||||
|
||||
|
||||
@@ -2148,7 +2148,7 @@ def _handle_chat_steer(handler, body: dict) -> bool:
|
||||
"stream_id": str|None}.
|
||||
"""
|
||||
from api.helpers import j, bad
|
||||
from api.config import SESSION_AGENT_CACHE, SESSION_AGENT_CACHE_LOCK
|
||||
from api import config as _cfg
|
||||
|
||||
sid = str((body or {}).get("session_id", "") or "").strip()
|
||||
text = str((body or {}).get("text", "") or "").strip()
|
||||
@@ -2157,8 +2157,8 @@ def _handle_chat_steer(handler, body: dict) -> bool:
|
||||
if not text:
|
||||
return bad(handler, "text required")
|
||||
|
||||
with SESSION_AGENT_CACHE_LOCK:
|
||||
cached = SESSION_AGENT_CACHE.get(sid)
|
||||
with _cfg.SESSION_AGENT_CACHE_LOCK:
|
||||
cached = _cfg.SESSION_AGENT_CACHE.get(sid)
|
||||
if not cached:
|
||||
# No active agent for this session — caller falls back to interrupt
|
||||
return j(handler, {"accepted": False, "fallback": "no_cached_agent",
|
||||
@@ -2181,8 +2181,8 @@ def _handle_chat_steer(handler, body: dict) -> bool:
|
||||
if not active_stream_id:
|
||||
return j(handler, {"accepted": False, "fallback": "not_running",
|
||||
"stream_id": None})
|
||||
with STREAMS_LOCK:
|
||||
stream_alive = active_stream_id in STREAMS
|
||||
with _cfg.STREAMS_LOCK:
|
||||
stream_alive = active_stream_id in _cfg.STREAMS
|
||||
if not stream_alive:
|
||||
# Active stream id is stale — stream has ended; caller falls back
|
||||
return j(handler, {"accepted": False, "fallback": "stream_dead",
|
||||
@@ -2210,17 +2210,36 @@ def cancel_stream(stream_id: str) -> bool:
|
||||
a safe no-op. Session cleanup runs outside STREAMS_LOCK to preserve lock
|
||||
ordering (streaming thread does LOCK → STREAMS_LOCK; inverting would deadlock).
|
||||
"""
|
||||
with STREAMS_LOCK:
|
||||
if stream_id not in STREAMS:
|
||||
from api import config as _live_config
|
||||
|
||||
# Use module-level aliases (imported from api.config at startup).
|
||||
# In production these are always the same objects as api.config.STREAMS etc.
|
||||
# The fallback below handles a hypothetical future case where api.config's
|
||||
# state dicts are replaced at runtime (e.g. a future profile-reload path).
|
||||
# No production code currently does this; the fallback is defensive only.
|
||||
streams = STREAMS
|
||||
cancel_flags = CANCEL_FLAGS
|
||||
agent_instances = AGENT_INSTANCES
|
||||
partial_texts = STREAM_PARTIAL_TEXT
|
||||
streams_lock = STREAMS_LOCK
|
||||
if stream_id not in streams and getattr(_live_config, 'STREAMS', streams) is not streams:
|
||||
streams = _live_config.STREAMS
|
||||
cancel_flags = _live_config.CANCEL_FLAGS
|
||||
agent_instances = _live_config.AGENT_INSTANCES
|
||||
partial_texts = _live_config.STREAM_PARTIAL_TEXT
|
||||
streams_lock = _live_config.STREAMS_LOCK
|
||||
|
||||
with streams_lock:
|
||||
if stream_id not in streams:
|
||||
return False
|
||||
|
||||
# Set WebUI layer cancel flag
|
||||
flag = CANCEL_FLAGS.get(stream_id)
|
||||
flag = cancel_flags.get(stream_id)
|
||||
if flag:
|
||||
flag.set()
|
||||
|
||||
# Interrupt the AIAgent instance to stop tool execution
|
||||
agent = AGENT_INSTANCES.get(stream_id)
|
||||
agent = agent_instances.get(stream_id)
|
||||
if agent:
|
||||
try:
|
||||
agent.interrupt("Cancelled by user")
|
||||
@@ -2248,7 +2267,7 @@ def cancel_stream(stream_id: str) -> bool:
|
||||
logger.debug("Failed to clear clarify prompt during cancel")
|
||||
|
||||
# Put a cancel sentinel into the queue so the SSE handler wakes up
|
||||
q = STREAMS.get(stream_id)
|
||||
q = streams.get(stream_id)
|
||||
if q:
|
||||
try:
|
||||
q.put_nowait(('cancel', {'message': 'Cancelled by user'}))
|
||||
@@ -2261,9 +2280,9 @@ def cancel_stream(stream_id: str) -> bool:
|
||||
# even if the agent thread is still blocked in a C-level syscall.
|
||||
# The worker thread's finally block uses .pop(key, None) too, so a
|
||||
# double-pop here is safe (no-op).
|
||||
STREAMS.pop(stream_id, None)
|
||||
CANCEL_FLAGS.pop(stream_id, None)
|
||||
AGENT_INSTANCES.pop(stream_id, None)
|
||||
streams.pop(stream_id, None)
|
||||
cancel_flags.pop(stream_id, None)
|
||||
agent_instances.pop(stream_id, None)
|
||||
# STREAM_PARTIAL_TEXT is intentionally NOT popped here — the agent thread may
|
||||
# still be appending tokens. We capture the snapshot two lines below; the
|
||||
# streaming finally block handles the cleanup when the thread exits.
|
||||
@@ -2275,7 +2294,13 @@ def cancel_stream(stream_id: str) -> bool:
|
||||
# get_session() acquires LOCK, and the streaming thread does LOCK first
|
||||
# then STREAMS_LOCK, so inverting the order here would cause deadlock.
|
||||
_cancel_session_id = getattr(agent, 'session_id', None) if agent else None
|
||||
_cancel_partial_text = STREAM_PARTIAL_TEXT.get(stream_id, '')
|
||||
_cancel_partial_text = partial_texts.get(stream_id, '')
|
||||
# Fallback: check the live config's partial text map if we used an alias
|
||||
# and the text wasn't found in the alias (defensive, matches streams fallback above).
|
||||
if not _cancel_partial_text:
|
||||
live_partials = getattr(_live_config, 'STREAM_PARTIAL_TEXT', partial_texts)
|
||||
if live_partials is not partial_texts:
|
||||
_cancel_partial_text = live_partials.get(stream_id, '')
|
||||
|
||||
# Session cleanup outside STREAMS_LOCK to preserve lock ordering.
|
||||
# Acquire the per-session _agent_lock too, mirroring every other session
|
||||
|
||||
@@ -271,6 +271,8 @@ def _workspace_blocked_roots() -> tuple[Path, ...]:
|
||||
'/lib',
|
||||
'/lib64',
|
||||
'/opt/homebrew',
|
||||
'/System',
|
||||
'/Library',
|
||||
)
|
||||
_seen: set[Path] = set()
|
||||
_out: list[Path] = []
|
||||
@@ -298,6 +300,80 @@ def _is_blocked_system_path(candidate: Path) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
def _workspace_blocked_resolved_subtrees() -> tuple[Path, ...]:
|
||||
roots = list(_workspace_blocked_roots()) + [Path('/private/etc')]
|
||||
resolved: list[Path] = []
|
||||
for root in roots:
|
||||
try:
|
||||
p = root.expanduser().resolve()
|
||||
except Exception:
|
||||
p = root
|
||||
if p not in resolved:
|
||||
resolved.append(p)
|
||||
return tuple(resolved)
|
||||
|
||||
|
||||
def _workspace_blocked_exact_roots() -> tuple[Path, ...]:
|
||||
roots = [Path('/'), Path('/private/var')]
|
||||
for root in _workspace_blocked_roots():
|
||||
try:
|
||||
roots.append(root.expanduser().resolve())
|
||||
except Exception:
|
||||
roots.append(root)
|
||||
unique: list[Path] = []
|
||||
for root in roots:
|
||||
if root not in unique:
|
||||
unique.append(root)
|
||||
return tuple(unique)
|
||||
|
||||
|
||||
def _is_blocked_workspace_path(candidate: Path, raw_path: str | Path | None = None) -> bool:
|
||||
"""Return True when candidate points at a known OS/system directory.
|
||||
|
||||
Compare both the original spelling and the resolved path. This closes the
|
||||
macOS /etc -> /private/etc bypass without globally banning temporary pytest
|
||||
paths under /private/var/folders.
|
||||
"""
|
||||
raw = None
|
||||
if raw_path not in (None, ""):
|
||||
try:
|
||||
raw = Path(raw_path).expanduser()
|
||||
except Exception:
|
||||
raw = None
|
||||
|
||||
exact = _workspace_blocked_exact_roots()
|
||||
if candidate in exact or (raw is not None and raw in _workspace_blocked_roots()):
|
||||
return True
|
||||
|
||||
for tmp in _USER_TMP_PREFIXES:
|
||||
if _is_within(candidate, tmp) or (raw is not None and _is_within(raw, tmp)):
|
||||
return False
|
||||
|
||||
# Raw paths under literal roots (e.g. /etc/ssh, /var/db) are always blocked.
|
||||
if raw is not None:
|
||||
for blocked in _workspace_blocked_roots():
|
||||
if _is_within(raw, blocked):
|
||||
return True
|
||||
|
||||
# Resolved subtree checks catch symlink aliases such as /private/etc. The
|
||||
# macOS temp root /private/var/folders is intentionally allowed for pytest
|
||||
# and per-user temporary workspaces; other direct /private/var system data
|
||||
# such as /private/var/db and /private/var/log remains blocked.
|
||||
allowed_private_var = (Path('/private/var/folders'), Path('/private/var/tmp'))
|
||||
for blocked in _workspace_blocked_resolved_subtrees():
|
||||
if blocked == Path('/private/var'):
|
||||
if candidate == blocked:
|
||||
return True
|
||||
if any(_is_within(candidate, allowed) for allowed in allowed_private_var):
|
||||
continue
|
||||
if _is_within(candidate, blocked):
|
||||
return True
|
||||
continue
|
||||
if _is_within(candidate, blocked):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _is_within(path: Path, root: Path) -> bool:
|
||||
try:
|
||||
path.relative_to(root)
|
||||
@@ -318,7 +394,7 @@ def _trusted_workspace_roots() -> list[Path]:
|
||||
return
|
||||
if not p.exists() or not p.is_dir():
|
||||
return
|
||||
if _is_blocked_system_path(p):
|
||||
if _is_blocked_workspace_path(p, candidate):
|
||||
return
|
||||
if p not in roots:
|
||||
roots.append(p)
|
||||
@@ -456,8 +532,8 @@ def resolve_trusted_workspace(path: str | Path | None = None) -> Path:
|
||||
except ValueError:
|
||||
pass
|
||||
|
||||
# Block known system roots and their children
|
||||
if _is_blocked_system_path(candidate):
|
||||
# Block known system roots and their children.
|
||||
if _is_blocked_workspace_path(candidate, path):
|
||||
raise ValueError(f"Path points to a system directory: {candidate}")
|
||||
|
||||
# (B) Trusted if already in the saved workspace list — covers non-home installs
|
||||
@@ -513,8 +589,8 @@ def validate_workspace_to_add(path: str) -> Path:
|
||||
if _home != Path("/") and _is_within(candidate, _home):
|
||||
return candidate
|
||||
|
||||
# Block known system roots and their immediate children
|
||||
if _is_blocked_system_path(candidate):
|
||||
# Block known system roots and their immediate children.
|
||||
if _is_blocked_workspace_path(candidate, path):
|
||||
raise ValueError(f"Path points to a system directory: {candidate}")
|
||||
|
||||
return candidate
|
||||
|
||||
109
static/ui.js
109
static/ui.js
@@ -81,6 +81,12 @@ function _closeImgLightbox(lb) {
|
||||
setTimeout(() => lb.parentNode && lb.parentNode.removeChild(lb), 120);
|
||||
}
|
||||
|
||||
document.addEventListener('click', e => {
|
||||
const img = e.target && e.target.closest ? e.target.closest('.msg-media-img') : null;
|
||||
if(!img) return;
|
||||
_openImgLightbox(img.src, img.alt);
|
||||
});
|
||||
|
||||
const _IMAGE_EXTS=/\.(png|jpg|jpeg|gif|webp|bmp|ico|avif)$/i;
|
||||
|
||||
// Dynamic model labels -- populated by populateModelDropdown(), fallback to static map
|
||||
@@ -887,7 +893,7 @@ function renderMd(raw){
|
||||
// backticks stays protected as a \x00C token and is never rendered as <img>.
|
||||
// Must run before _code_stash restore and before _link_stash so the image
|
||||
// is not consumed by the [label](url) link regex.
|
||||
t=t.replace(/!\[([^\]]*)\]\((https?:\/\/[^\)]+)\)/g,(_,alt,url)=>`<img src="${url.replace(/"/g,'%22')}" alt="${esc(alt)}" class="msg-media-img" loading="lazy" onclick="_openImgLightbox(this.src,this.alt)">`);
|
||||
t=t.replace(/!\[([^\]]*)\]\((https?:\/\/[^\)]+)\)/g,(_,alt,url)=>`<img src="${url.replace(/"/g,'%22')}" alt="${esc(alt)}" class="msg-media-img" loading="lazy">`);
|
||||
// Stash rendered <img> tags so autolink never matches URLs inside src=
|
||||
const _img_stash=[];
|
||||
t=t.replace(/(<img\b[^>]*>)/g,m=>{_img_stash.push(m);return `\x00G${_img_stash.length-1}\x00`;});
|
||||
@@ -966,7 +972,7 @@ function renderMd(raw){
|
||||
// #487: Outer image pass — handles  in plain paragraphs (outside tables/lists).
|
||||
// Runs AFTER the table pass (images in table cells are handled by inlineMd() above).
|
||||
// Runs BEFORE the outer [label](url) link pass so the image is not consumed as a plain link.
|
||||
s=s.replace(/!\[([^\]]*)\]\((https?:\/\/[^\)]+)\)/g,(_,alt,url)=>`<img src="${url.replace(/"/g,'%22')}" alt="${esc(alt)}" class="msg-media-img" loading="lazy" onclick="_openImgLightbox(this.src,this.alt)">`);
|
||||
s=s.replace(/!\[([^\]]*)\]\((https?:\/\/[^\)]+)\)/g,(_,alt,url)=>`<img src="${url.replace(/"/g,'%22')}" alt="${esc(alt)}" class="msg-media-img" loading="lazy">`);
|
||||
// Outer link pass for labeled links in plain paragraphs (outside table cells).
|
||||
// Runs AFTER the table pass so table cells are processed by inlineMd() only.
|
||||
// Stash existing <a> tags first to avoid re-linking already-linked URLs.
|
||||
@@ -974,12 +980,90 @@ function renderMd(raw){
|
||||
s=s.replace(/(<a\b[^>]*>[\s\S]*?<\/a>)/g,m=>{_a_stash.push(m);return `\x00A${_a_stash.length-1}\x00`;});
|
||||
s=s.replace(/\[([^\]]+)\]\((https?:\/\/[^\)]+)\)/g,(_,label,url)=>`<a href="${url.replace(/"/g,'%22')}" target="_blank" rel="noopener">${esc(label)}</a>`);
|
||||
s=s.replace(/\x00A(\d+)\x00/g,(_,i)=>_a_stash[+i]);
|
||||
// Escape any remaining HTML tags that are NOT from our own markdown output.
|
||||
// Our pipeline only emits: <strong>,<em>,<code>,<pre>,<h1-6>,<ul>,<ol>,<li>,
|
||||
// <table>,<thead>,<tbody>,<tr>,<th>,<td>,<hr>,<blockquote>,<p>,<br>,<a>,
|
||||
// <div class="..."> (mermaid/pre-header). Everything else is untrusted input.
|
||||
const SAFE_TAGS=/^<\/?(strong|em|del|code|pre|h[1-6]|ul|ol|li|table|thead|tbody|tr|th|td|hr|blockquote|p|br|a|img|div|span)([\s>]|$)/i;
|
||||
s=s.replace(/<\/?[a-z][^>]*>/gi,tag=>SAFE_TAGS.test(tag)?tag:esc(tag));
|
||||
// Sanitize any remaining HTML tags. The renderer intentionally returns
|
||||
// HTML and inserts it with innerHTML later, so tag names alone are not enough:
|
||||
// raw/model-provided HTML like <img onerror=...> or <a href="javascript:...">
|
||||
// must lose executable attributes and dangerous schemes while preserving the
|
||||
// small set of attributes generated by this markdown pipeline.
|
||||
// Reference only — documents the allowed tag set. Superseded by _tag() allowlists.
|
||||
// Tests verify this list is complete; _tag() enforces it.
|
||||
const SAFE_TAGS=/^<\/?(?:strong|em|del|code|pre|h[1-6]|ul|ol|li|table|thead|tbody|tr|th|td|hr|blockquote|p|br|a|div|span|img)([\s>]|$)/i;
|
||||
function _safeAttrValue(v){
|
||||
return String(v||'').replace(/"/g,'"').replace(/'/g,"'").replace(/&/g,'&').trim();
|
||||
}
|
||||
function _isSafeUrl(v, img){
|
||||
const raw=_safeAttrValue(v);
|
||||
const compact=raw.replace(/[\u0000-\u001f\u007f\s]+/g,'').toLowerCase();
|
||||
if(!compact) return false;
|
||||
if(/^(javascript|data|vbscript):/i.test(compact)) return false;
|
||||
if(/^https?:\/\//i.test(raw)) return true;
|
||||
if(img && /^api\//i.test(raw)) return true;
|
||||
if(!img && (/^api\//i.test(raw) || /^#/.test(raw))) return true;
|
||||
return false;
|
||||
}
|
||||
function _attrs(raw){
|
||||
const out={};
|
||||
String(raw||'').replace(/([a-zA-Z0-9:_-]+)(?:\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s"'>`]+)))?/g,(_,k,dq,sq,bare)=>{
|
||||
out[String(k).toLowerCase()]=dq!==undefined?dq:(sq!==undefined?sq:(bare!==undefined?bare:''));
|
||||
return '';
|
||||
});
|
||||
return out;
|
||||
}
|
||||
function _cls(v, allowed){
|
||||
const got=String(v||'').split(/\s+/).filter(c=>allowed.includes(c));
|
||||
return got.length?` class="${esc(got.join(' '))}"`:'';
|
||||
}
|
||||
function _tag(tag){
|
||||
const m=String(tag||'').match(/^<\s*(\/)?\s*([a-zA-Z][\w:-]*)([\s\S]*?)(\/)?\s*>$/);
|
||||
if(!m) return esc(tag);
|
||||
const closing=!!m[1];
|
||||
const name=m[2].toLowerCase();
|
||||
const rawAttrs=m[3]||'';
|
||||
const plain=['strong','em','del','pre','h1','h2','h3','h4','h5','h6','ul','ol','table','thead','tbody','tr','th','td','blockquote','p','br','hr'];
|
||||
if(closing) return plain.includes(name)||['a','div','span','li','code'].includes(name)?`</${name}>`:'';
|
||||
if(name==='code'){
|
||||
const a=_attrs(rawAttrs);
|
||||
const cls=/^language-[a-z0-9_+-]+$/i.test(a.class||'')?` class="${esc(a.class)}"`:'';
|
||||
return `<code${cls}>`;
|
||||
}
|
||||
if(plain.includes(name)) return `<${name}>`;
|
||||
const a=_attrs(rawAttrs);
|
||||
if(name==='li'){
|
||||
const value=/^\d+$/.test(a.value||'')?` value="${esc(a.value)}"`:'';
|
||||
const style=(a.style||'').replace(/\s+/g,'').toLowerCase()==='margin-left:16px'?` style="margin-left:16px"`:'';
|
||||
return `<li${value}${style}>`;
|
||||
}
|
||||
if(name==='span'){
|
||||
return `<span${_cls(a.class,['task-done','task-todo','katex-inline'])}${a['data-katex']==='inline'?' data-katex="inline"':''}>`;
|
||||
}
|
||||
if(name==='div'){
|
||||
const cls=_cls(a.class,['pre-header','mermaid-block','katex-block']);
|
||||
const mermaid=a['data-mermaid-id']?` data-mermaid-id="${esc(a['data-mermaid-id'])}"`:'';
|
||||
const katex=a['data-katex']==='display'?' data-katex="display"':'';
|
||||
return `<div${cls}${mermaid}${katex}>`;
|
||||
}
|
||||
if(name==='a'){
|
||||
if(!_isSafeUrl(a.href,false)) return '<a>';
|
||||
const target=a.target==='_blank'?' target="_blank"':'';
|
||||
const rel=a.rel==='noopener'?' rel="noopener"':'';
|
||||
const cls=_cls(a.class,['msg-media-link','skill-linked-file','skill-file-back']);
|
||||
const download=a.download?` download="${esc(a.download)}"`:'';
|
||||
return `<a${cls} href="${esc(_safeAttrValue(a.href))}"${target}${rel}${download}>`;
|
||||
}
|
||||
if(name==='img'){
|
||||
if(!_isSafeUrl(a.src,true)) return '';
|
||||
const cls=_cls(a.class,['msg-media-img']);
|
||||
const alt=` alt="${esc(_safeAttrValue(a.alt||''))}"`;
|
||||
const loading=a.loading==='lazy'?' loading="lazy"':'';
|
||||
return `<img${cls} src="${esc(_safeAttrValue(a.src))}"${alt}${loading}>`;
|
||||
}
|
||||
return '';
|
||||
}
|
||||
s=s.replace(/<\/?[a-z][^>]*>/gi,tag=>_tag(tag));
|
||||
// Incomplete raw tags must not survive until paragraph wrapping, where the
|
||||
// renderer's generated </p> could provide a closing ">" and turn them into
|
||||
// executable HTML in innerHTML (for example: <img src=x onerror=...//).
|
||||
s=s.replace(/<[a-zA-Z][\w:-]*[^>\n]*$/gm,tag=>esc(tag));
|
||||
// Autolink: convert plain URLs to clickable links.
|
||||
// Stash <a>, <img> and <pre> blocks so autolink never runs inside them.
|
||||
const _al_stash=[];
|
||||
@@ -1035,14 +1119,14 @@ function renderMd(raw){
|
||||
// Render all https:// URLs as <img> — extension check would miss extensionless
|
||||
// CDN paths like fal.media content-addressed URLs (closes #853).
|
||||
if(_IMAGE_EXTS.test(src.split('?')[0]) || /^https?:\/\//i.test(src)){
|
||||
return `<img class="msg-media-img" src="${esc(src)}" alt="image" loading="lazy" onclick="_openImgLightbox(this.src,this.alt)">`;
|
||||
return `<img class="msg-media-img" src="${esc(src)}" alt="image" loading="lazy">`;
|
||||
}
|
||||
return `<a href="${esc(src)}" target="_blank" rel="noopener">${esc(src)}</a>`;
|
||||
}
|
||||
// Local file path
|
||||
const apiUrl='api/media?path='+encodeURIComponent(ref);
|
||||
if(_IMAGE_EXTS.test(ref)){
|
||||
return `<img class="msg-media-img" src="${esc(apiUrl)}" alt="${esc(ref.split('/').pop())}" loading="lazy" onclick="_openImgLightbox(this.src,this.alt)">`;
|
||||
return `<img class="msg-media-img" src="${esc(apiUrl)}" alt="${esc(ref.split('/').pop())}" loading="lazy">`;
|
||||
}
|
||||
// Non-image local file — show download link with filename
|
||||
const fname=esc(ref.split('/').pop()||ref);
|
||||
@@ -1856,6 +1940,9 @@ function syncTopbar(){
|
||||
}
|
||||
}
|
||||
if(typeof syncAppTitlebar==='function') syncAppTitlebar();
|
||||
// Update profile chip even when no session is active (e.g. right after profile switch)
|
||||
const _profileLabel=$('profileChipLabel');
|
||||
if(_profileLabel) _profileLabel.textContent=S.activeProfile||'default';
|
||||
return;
|
||||
}
|
||||
const sessionTitle=S.session.title||t('untitled');
|
||||
@@ -2371,7 +2458,7 @@ function renderMessages(){
|
||||
// Use api/file/raw which resolves filename relative to the session workspace.
|
||||
// api/media expects a full absolute path which we don't store on the client side.
|
||||
const imgUrl='api/file/raw?session_id='+encodeURIComponent(_attachSid)+'&path='+encodeURIComponent(fname);
|
||||
return `<img class="msg-media-img" src="${esc(imgUrl)}" alt="${esc(fname)}" loading="lazy" onclick="_openImgLightbox(this.src,this.alt)">`;
|
||||
return `<img class="msg-media-img" src="${esc(imgUrl)}" alt="${esc(fname)}" loading="lazy">`;
|
||||
}
|
||||
return `<div class="msg-file-badge">${li('paperclip',12)} ${esc(fname)}</div>`;
|
||||
}).join('')}</div>`;
|
||||
|
||||
@@ -127,11 +127,14 @@ class TestChatHistoryImageRendering:
|
||||
assert 'msg-media-img' in body, 'Image attachment <img> must use msg-media-img class'
|
||||
|
||||
def test_attachment_render_click_to_fullscreen(self):
|
||||
"""Click-to-fullscreen must still work on chat history images."""
|
||||
"""Click-to-fullscreen uses the delegated .msg-media-img listener, not inline JS."""
|
||||
ui = _read_js('ui.js')
|
||||
assert "document.addEventListener('click'" in ui
|
||||
assert "closest('.msg-media-img')" in ui
|
||||
m = re.search(r'm\.attachments&&m\.attachments\.length', ui)
|
||||
body = ui[m.start():m.start() + 1200]
|
||||
assert '_openImgLightbox' in body, 'Chat history images must open lightbox on click'
|
||||
img_line = next(line for line in body.splitlines() if 'msg-media-img' in line)
|
||||
assert 'onclick' not in img_line, 'Chat history image HTML must not embed inline JS handlers'
|
||||
|
||||
def test_attachment_render_non_image_keeps_paperclip(self):
|
||||
"""Non-image attachments in chat history must still show paperclip badge."""
|
||||
|
||||
@@ -141,41 +141,61 @@ def test_server_now_ms_defaults_to_date_now_when_no_skew():
|
||||
|
||||
|
||||
def test_server_now_ms_compensates_positive_skew():
|
||||
"""If server is behind client (skew > 0), _serverNowMs() subtracts the delta."""
|
||||
"""If server is behind client (skew > 0), _serverNowMs() subtracts the delta.
|
||||
|
||||
Uses a small tolerance window (±5 ms) because two consecutive Date.now() calls
|
||||
inside Node.js can differ by 1-2 ms on a loaded system, causing `diff === 3600000`
|
||||
to fail intermittently even though the compensation logic is correct.
|
||||
"""
|
||||
result = _run_time_case(
|
||||
"""
|
||||
// Simulate: client clock is 3600s (1 hour) ahead of server
|
||||
_serverTimeDelta = 3600 * 1000;
|
||||
const clientNow = Date.now();
|
||||
const serverNow = _serverNowMs();
|
||||
const diff = clientNow - serverNow;
|
||||
const t0 = Date.now();
|
||||
const serverNow = _serverNowMs(); // internally calls Date.now() again
|
||||
const t1 = Date.now();
|
||||
// Use the midpoint of t0..t1 to absorb the tiny time-of-call delta
|
||||
const diffMs = ((t0 + t1) / 2) - serverNow;
|
||||
process.stdout.write(JSON.stringify({
|
||||
diffMs: diff,
|
||||
isOneHour: diff === 3600000,
|
||||
diffMs: Math.round(diffMs),
|
||||
isOneHour: Math.abs(diffMs - 3600000) < 5,
|
||||
}));
|
||||
"""
|
||||
)
|
||||
assert result["isOneHour"] is True
|
||||
assert result["diffMs"] == 3_600_000
|
||||
assert result["isOneHour"] is True, (
|
||||
f"Expected diff ≈ 3600000 ms, got {result['diffMs']} ms. "
|
||||
"The skew compensation is broken."
|
||||
)
|
||||
assert abs(result["diffMs"] - 3_600_000) < 5
|
||||
|
||||
|
||||
def test_server_now_ms_compensates_negative_skew():
|
||||
"""If server is ahead of client (skew < 0), _serverNowMs() adds the delta."""
|
||||
"""If server is ahead of client (skew < 0), _serverNowMs() adds the delta.
|
||||
|
||||
Uses midpoint averaging with ±5 ms tolerance to avoid intermittent failures
|
||||
caused by consecutive Date.now() calls returning different values under CPU load.
|
||||
(Same fix as the positive-skew test above.)
|
||||
"""
|
||||
result = _run_time_case(
|
||||
"""
|
||||
// Simulate: client clock is 7200s (2 hours) behind server
|
||||
_serverTimeDelta = -7200 * 1000;
|
||||
const clientNow = Date.now();
|
||||
const serverNow = _serverNowMs();
|
||||
const diff = serverNow - clientNow;
|
||||
const t0 = Date.now();
|
||||
const serverNow = _serverNowMs(); // internally calls Date.now() at T1 >= T0
|
||||
const t1 = Date.now();
|
||||
// serverNow = T1 + 7200000; clientNow ≈ midpoint(T0,T1)
|
||||
const diffMs = serverNow - ((t0 + t1) / 2);
|
||||
process.stdout.write(JSON.stringify({
|
||||
diffMs: diff,
|
||||
isTwoHours: diff === 7200000,
|
||||
diffMs: Math.round(diffMs),
|
||||
isTwoHours: Math.abs(diffMs - 7200000) < 5,
|
||||
}));
|
||||
"""
|
||||
)
|
||||
assert result["isTwoHours"] is True
|
||||
assert result["diffMs"] == 7_200_000
|
||||
assert result["isTwoHours"] is True, (
|
||||
f"Expected diff ≈ 7200000 ms, got {result['diffMs']} ms. "
|
||||
"The negative-skew compensation is broken."
|
||||
)
|
||||
assert abs(result["diffMs"] - 7_200_000) < 5
|
||||
|
||||
|
||||
def test_relative_time_uses_server_clock():
|
||||
|
||||
@@ -75,18 +75,27 @@ def test_autolink_in_inline_md():
|
||||
|
||||
|
||||
def test_autolink_after_safe_tags_pass():
|
||||
"""The autolink pass must come AFTER the SAFE_TAGS escape pass (ordering matters)."""
|
||||
"""The autolink pass must come AFTER the HTML sanitizer pass (ordering matters).
|
||||
|
||||
The sanitizer was upgraded from a tag-name allowlist (SAFE_TAGS) to a full
|
||||
attribute-stripping sanitizer (_tag). The ordering invariant still holds:
|
||||
sanitize first, autolink second, paragraph-wrap last.
|
||||
"""
|
||||
content = read_ui_js()
|
||||
safe_tags_idx = content.find('s=s.replace(/<\\/?[a-z][^>]*>/gi,tag=>SAFE_TAGS.test(tag)?tag:esc(tag));')
|
||||
# Accept either the new _tag() sanitizer or the legacy SAFE_TAGS line so this
|
||||
# test works on both the old and new renderer.
|
||||
sanitizer_idx = content.find('s=s.replace(/<\\/?[a-z][^>]*>/gi,tag=>_tag(tag));')
|
||||
if sanitizer_idx == -1:
|
||||
sanitizer_idx = content.find('s=s.replace(/<\\/?[a-z][^>]*>/gi,tag=>SAFE_TAGS.test(tag)?tag:esc(tag));')
|
||||
autolink_idx = content.find('// Autolink: convert plain URLs')
|
||||
parts_idx = content.find('const parts=s.split(/\\n{2,}/);')
|
||||
assert safe_tags_idx != -1, "SAFE_TAGS pass not found"
|
||||
assert sanitizer_idx != -1, "HTML sanitizer pass not found (expected _tag() or SAFE_TAGS)"
|
||||
assert autolink_idx != -1, "Autolink pass not found"
|
||||
assert parts_idx != -1, "Paragraph-wrap parts line not found"
|
||||
assert safe_tags_idx < autolink_idx < parts_idx, (
|
||||
f"Ordering wrong: SAFE_TAGS at {safe_tags_idx}, autolink at {autolink_idx}, "
|
||||
assert sanitizer_idx < autolink_idx < parts_idx, (
|
||||
f"Ordering wrong: sanitizer at {sanitizer_idx}, autolink at {autolink_idx}, "
|
||||
f"parts (paragraph wrap) at {parts_idx}. "
|
||||
"Autolink must come between SAFE_TAGS pass and paragraph wrap."
|
||||
"Autolink must come between sanitizer pass and paragraph wrap."
|
||||
)
|
||||
|
||||
|
||||
@@ -106,19 +115,14 @@ def test_autolink_target_blank_and_rel():
|
||||
|
||||
|
||||
def test_safe_tags_includes_anchor():
|
||||
"""SAFE_TAGS regex must include 'a' so <a> tags from autolink are not escaped."""
|
||||
"""The HTML sanitizer must preserve <a> tags from the autolink pass.
|
||||
|
||||
After the sanitizer upgrade from SAFE_TAGS regex to the _tag() function,
|
||||
<a> tags are handled by the explicit 'a' branch in _tag() — they survive
|
||||
with href/target/rel/class/download attributes and their content intact.
|
||||
"""
|
||||
content = read_ui_js()
|
||||
# Find the SAFE_TAGS definition line — the pattern contains slashes so we
|
||||
# search for the line directly rather than extracting the regex literal.
|
||||
safe_tags_line = None
|
||||
for line in content.splitlines():
|
||||
if 'const SAFE_TAGS=' in line:
|
||||
safe_tags_line = line
|
||||
break
|
||||
assert safe_tags_line is not None, "SAFE_TAGS const definition not found in ui.js"
|
||||
# The pattern should include 'a' as a tag alternative (e.g. |a|)
|
||||
assert '|a|' in safe_tags_line or '|a)' in safe_tags_line, (
|
||||
f"SAFE_TAGS line does not include 'a' tag — "
|
||||
"<a> tags emitted by autolink would be escaped!\n"
|
||||
f"Line: {safe_tags_line}"
|
||||
# The _tag() function must contain an explicit 'a' (anchor) handler.
|
||||
assert "name==='a'" in content or "name === 'a'" in content, (
|
||||
"HTML sanitizer _tag() must have an explicit handler for <a> tags"
|
||||
)
|
||||
|
||||
391
tests/test_profile_switch_1200.py
Normal file
391
tests/test_profile_switch_1200.py
Normal file
@@ -0,0 +1,391 @@
|
||||
"""
|
||||
Tests for profile-switch workspace and model fixes (#1200).
|
||||
|
||||
Bug 1: switch_profile(process_wide=False) returned the OLD profile's workspace
|
||||
because get_last_workspace() reads via get_active_profile_name() (TLS/global)
|
||||
rather than directly from the target profile's home directory.
|
||||
|
||||
Bug 2: /api/models returned stale results after a profile switch because the
|
||||
in-memory model cache (_available_models_cache) was not invalidated.
|
||||
|
||||
These tests verify both fixes.
|
||||
"""
|
||||
import os
|
||||
import json
|
||||
import tempfile
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
def test_switch_profile_returns_target_workspace_not_current(tmp_path, monkeypatch):
|
||||
"""
|
||||
switch_profile(process_wide=False) must return the TARGET profile's workspace,
|
||||
not the currently-active profile's workspace.
|
||||
|
||||
Before the fix, get_last_workspace() was called at the end of switch_profile(),
|
||||
and it routed through get_active_profile_name() which still pointed to the OLD
|
||||
profile during a process_wide=False switch. This caused the wrong workspace to
|
||||
be returned and displayed in the UI immediately after switching.
|
||||
"""
|
||||
import api.profiles as profiles
|
||||
|
||||
# Build fake profile structure
|
||||
default_home = tmp_path / '.hermes'
|
||||
default_home.mkdir()
|
||||
ayan_home = default_home / 'profiles' / 'ayan'
|
||||
ayan_home.mkdir(parents=True)
|
||||
|
||||
# Give ayan a terminal.cwd config (common case)
|
||||
ayan_workspace = tmp_path / 'ayan_workspace'
|
||||
ayan_workspace.mkdir()
|
||||
ayan_config = ayan_home / 'config.yaml'
|
||||
ayan_config.write_text(
|
||||
f'model:\n default: kimi-k2-instruct\n provider: nous\n'
|
||||
f'terminal:\n cwd: {ayan_workspace}\n',
|
||||
encoding='utf-8',
|
||||
)
|
||||
|
||||
# Give default profile a different workspace stored in last_workspace.txt
|
||||
default_ws = tmp_path / 'default_workspace'
|
||||
default_ws.mkdir()
|
||||
default_state = default_home / 'webui_state'
|
||||
default_state.mkdir()
|
||||
(default_state / 'last_workspace.txt').write_text(str(default_ws), encoding='utf-8')
|
||||
|
||||
# Patch _DEFAULT_HERMES_HOME to our tmp dir
|
||||
orig_default = profiles._DEFAULT_HERMES_HOME
|
||||
profiles._DEFAULT_HERMES_HOME = default_home
|
||||
# Ensure _active_profile = 'default'
|
||||
orig_active = profiles._active_profile
|
||||
profiles._active_profile = 'default'
|
||||
# Clear TLS
|
||||
profiles._tls.profile = None
|
||||
|
||||
try:
|
||||
result = profiles.switch_profile('ayan', process_wide=False)
|
||||
ws = result.get('default_workspace', '')
|
||||
# Must be ayan's workspace, NOT default's workspace
|
||||
assert str(ayan_workspace) in ws or ayan_workspace.resolve() == Path(ws), (
|
||||
f"Expected ayan's workspace ({ayan_workspace}), got: {ws}"
|
||||
)
|
||||
assert str(default_ws) not in ws, (
|
||||
f"Returned default profile workspace ({default_ws}) instead of ayan's"
|
||||
)
|
||||
finally:
|
||||
profiles._DEFAULT_HERMES_HOME = orig_default
|
||||
profiles._active_profile = orig_active
|
||||
profiles._tls.profile = None
|
||||
|
||||
|
||||
def test_switch_profile_uses_last_workspace_txt_over_config(tmp_path, monkeypatch):
|
||||
"""
|
||||
If a profile has a last_workspace.txt (previously chosen workspace),
|
||||
that takes priority over terminal.cwd in config.yaml.
|
||||
"""
|
||||
import api.profiles as profiles
|
||||
|
||||
default_home = tmp_path / '.hermes'
|
||||
default_home.mkdir()
|
||||
target_home = default_home / 'profiles' / 'myprofile'
|
||||
target_home.mkdir(parents=True)
|
||||
|
||||
# config.yaml has terminal.cwd
|
||||
cfg_ws = tmp_path / 'cfg_workspace'
|
||||
cfg_ws.mkdir()
|
||||
(target_home / 'config.yaml').write_text(
|
||||
f'terminal:\n cwd: {cfg_ws}\n', encoding='utf-8',
|
||||
)
|
||||
|
||||
# last_workspace.txt overrides it
|
||||
explicit_ws = tmp_path / 'explicit_workspace'
|
||||
explicit_ws.mkdir()
|
||||
state_dir = target_home / 'webui_state'
|
||||
state_dir.mkdir()
|
||||
(state_dir / 'last_workspace.txt').write_text(str(explicit_ws), encoding='utf-8')
|
||||
|
||||
orig_default = profiles._DEFAULT_HERMES_HOME
|
||||
profiles._DEFAULT_HERMES_HOME = default_home
|
||||
orig_active = profiles._active_profile
|
||||
profiles._active_profile = 'default'
|
||||
profiles._tls.profile = None
|
||||
|
||||
try:
|
||||
result = profiles.switch_profile('myprofile', process_wide=False)
|
||||
ws = result.get('default_workspace', '')
|
||||
assert str(explicit_ws) in ws or Path(ws) == explicit_ws.resolve(), (
|
||||
f"Expected last_workspace.txt ({explicit_ws}), got: {ws}"
|
||||
)
|
||||
assert str(cfg_ws) not in ws, (
|
||||
f"terminal.cwd ({cfg_ws}) should not override last_workspace.txt"
|
||||
)
|
||||
finally:
|
||||
profiles._DEFAULT_HERMES_HOME = orig_default
|
||||
profiles._active_profile = orig_active
|
||||
profiles._tls.profile = None
|
||||
|
||||
|
||||
def test_switch_profile_process_wide_false_returns_correct_model(tmp_path, monkeypatch):
|
||||
"""
|
||||
switch_profile(process_wide=False) reads the default model from the TARGET
|
||||
profile's config.yaml directly (not from the process-global _cfg_cache).
|
||||
"""
|
||||
import api.profiles as profiles
|
||||
|
||||
default_home = tmp_path / '.hermes'
|
||||
default_home.mkdir()
|
||||
target_home = default_home / 'profiles' / 'aiprofile'
|
||||
target_home.mkdir(parents=True)
|
||||
|
||||
target_ws = tmp_path / 'ai_ws'
|
||||
target_ws.mkdir()
|
||||
(target_home / 'config.yaml').write_text(
|
||||
f'model:\n default: kimi-k2-instruct\n provider: nous\n'
|
||||
f'terminal:\n cwd: {target_ws}\n',
|
||||
encoding='utf-8',
|
||||
)
|
||||
|
||||
orig_default = profiles._DEFAULT_HERMES_HOME
|
||||
profiles._DEFAULT_HERMES_HOME = default_home
|
||||
orig_active = profiles._active_profile
|
||||
profiles._active_profile = 'default'
|
||||
profiles._tls.profile = None
|
||||
|
||||
try:
|
||||
result = profiles.switch_profile('aiprofile', process_wide=False)
|
||||
assert result.get('default_model') == 'kimi-k2-instruct', (
|
||||
f"Expected 'kimi-k2-instruct', got: {result.get('default_model')!r}"
|
||||
)
|
||||
finally:
|
||||
profiles._DEFAULT_HERMES_HOME = orig_default
|
||||
profiles._active_profile = orig_active
|
||||
profiles._tls.profile = None
|
||||
|
||||
|
||||
def test_profile_switch_route_invalidates_models_cache(tmp_path):
|
||||
"""
|
||||
After a profile switch, the model cache must be invalidated so the next
|
||||
/api/models request rebuilds from the new profile's config.
|
||||
|
||||
This is a unit test verifying that invalidate_models_cache() is called
|
||||
as part of the /api/profile/switch response flow.
|
||||
"""
|
||||
from api.config import invalidate_models_cache, _available_models_cache_lock
|
||||
import api.config as config_module
|
||||
|
||||
# Seed a non-None cache value to simulate a populated cache
|
||||
with _available_models_cache_lock:
|
||||
config_module._available_models_cache = {
|
||||
'active_provider': 'old_provider',
|
||||
'default_model': 'old-model',
|
||||
'groups': [],
|
||||
}
|
||||
config_module._available_models_cache_ts = 9999999.0
|
||||
|
||||
# Verify it's non-None before
|
||||
assert config_module._available_models_cache is not None
|
||||
|
||||
# Call invalidate (the same function called by the route handler)
|
||||
invalidate_models_cache()
|
||||
|
||||
# Must be None after invalidation
|
||||
assert config_module._available_models_cache is None, (
|
||||
"invalidate_models_cache() must clear _available_models_cache"
|
||||
)
|
||||
assert config_module._available_models_cache_ts == 0.0
|
||||
|
||||
|
||||
"""
|
||||
Test that syncTopbar() updates the profile chip label even when S.session is null.
|
||||
|
||||
Bug: The profile chip label (profileChipLabel) was only updated in the session-present
|
||||
path of syncTopbar(). When S.session is null (fresh page / after profile switch with
|
||||
no active session), the early-return branch ran without updating the chip.
|
||||
This caused the chip to keep showing the old profile name after switchToProfile().
|
||||
"""
|
||||
|
||||
import re
|
||||
|
||||
|
||||
def test_syncTopbar_early_return_updates_profile_chip():
|
||||
"""
|
||||
syncTopbar() must update profileChipLabel inside the !S.session early-return block.
|
||||
Without this, the composer profile chip stays stale when there is no active session.
|
||||
"""
|
||||
from pathlib import Path
|
||||
ui_js = (Path(__file__).parent.parent / "static" / "ui.js").read_text(encoding="utf-8")
|
||||
|
||||
# Find the syncTopbar function
|
||||
fn_start = ui_js.find("function syncTopbar(){")
|
||||
assert fn_start != -1, "syncTopbar function not found in ui.js"
|
||||
|
||||
# Find the early-return block (!S.session branch)
|
||||
early_ret_start = ui_js.find("if(!S.session){", fn_start)
|
||||
assert early_ret_start != -1, "!S.session early-return block not found in syncTopbar"
|
||||
|
||||
# Find where the early return ends (the closing brace + return)
|
||||
early_ret_end = ui_js.find("return;", early_ret_start)
|
||||
assert early_ret_end != -1
|
||||
|
||||
early_block = ui_js[early_ret_start : early_ret_end + len("return;")]
|
||||
|
||||
# The profile chip update must be inside this early-return block
|
||||
assert "profileChipLabel" in early_block, (
|
||||
"syncTopbar() early-return block (!S.session) must update profileChipLabel. "
|
||||
"Without this, switching profiles with no active session leaves the chip stale."
|
||||
)
|
||||
assert "S.activeProfile" in early_block, (
|
||||
"profileChipLabel update in early-return block must read S.activeProfile"
|
||||
)
|
||||
|
||||
|
||||
# ── Regression guard tests ────────────────────────────────────────────────────
|
||||
# These tests exist to catch future regressions in profile switching behavior.
|
||||
# Each one corresponds to a specific bug that was fixed in the #1200 PR.
|
||||
|
||||
def test_regression_switch_profile_default_workspace_not_from_process_global(tmp_path, monkeypatch):
|
||||
"""
|
||||
REGRESSION GUARD (#1200 Bug 1): switch_profile(process_wide=False) must NOT
|
||||
return the active (old) profile's workspace via get_last_workspace().
|
||||
|
||||
This test proves the fix by setting up a scenario where the old profile has
|
||||
a known workspace in last_workspace.txt and the target profile has a DIFFERENT
|
||||
workspace. If the regression returns, this test fails.
|
||||
"""
|
||||
import api.profiles as profiles
|
||||
|
||||
base = tmp_path / ".hermes"
|
||||
base.mkdir()
|
||||
|
||||
# Old profile (default) has workspace A
|
||||
old_ws = tmp_path / "old_workspace"
|
||||
old_ws.mkdir()
|
||||
default_state = base / "webui_state"
|
||||
default_state.mkdir()
|
||||
(default_state / "last_workspace.txt").write_text(str(old_ws), encoding="utf-8")
|
||||
|
||||
# Target profile has workspace B
|
||||
new_ws = tmp_path / "new_workspace"
|
||||
new_ws.mkdir()
|
||||
target_home = base / "profiles" / "target"
|
||||
target_home.mkdir(parents=True)
|
||||
target_state = target_home / "webui_state"
|
||||
target_state.mkdir()
|
||||
(target_state / "last_workspace.txt").write_text(str(new_ws), encoding="utf-8")
|
||||
(target_home / "config.yaml").write_text(
|
||||
"model:\n default: some-model\n", encoding="utf-8"
|
||||
)
|
||||
|
||||
orig_default = profiles._DEFAULT_HERMES_HOME
|
||||
orig_active = profiles._active_profile
|
||||
profiles._DEFAULT_HERMES_HOME = base
|
||||
profiles._active_profile = "default"
|
||||
profiles._tls.profile = None
|
||||
|
||||
try:
|
||||
result = profiles.switch_profile("target", process_wide=False)
|
||||
ws = result.get("default_workspace", "")
|
||||
# Must be NEW workspace, not OLD
|
||||
assert str(new_ws) in ws or str(new_ws.resolve()) == ws, (
|
||||
f"REGRESSION: Got old workspace ({old_ws}) instead of target ({new_ws}). "
|
||||
"switch_profile() is reading from the wrong profile."
|
||||
)
|
||||
assert str(old_ws) not in ws, (
|
||||
f"REGRESSION: Returned old profile workspace. Bug 1 regressed."
|
||||
)
|
||||
finally:
|
||||
profiles._DEFAULT_HERMES_HOME = orig_default
|
||||
profiles._active_profile = orig_active
|
||||
profiles._tls.profile = None
|
||||
|
||||
|
||||
def test_regression_models_cache_cleared_on_profile_switch():
|
||||
"""
|
||||
REGRESSION GUARD (#1200 Bug 2): the model cache must be invalidated after
|
||||
a profile switch so the next /api/models returns the new profile's models.
|
||||
|
||||
Without the invalidate_models_cache() call in the route handler, a populated
|
||||
cache from the old profile would be served unchanged.
|
||||
"""
|
||||
import api.config as config_module
|
||||
from api.config import invalidate_models_cache, _available_models_cache_lock
|
||||
|
||||
# Seed cache with "stale" data
|
||||
stale = {"active_provider": "stale", "default_model": "stale-model", "groups": []}
|
||||
with _available_models_cache_lock:
|
||||
config_module._available_models_cache = stale
|
||||
config_module._available_models_cache_ts = 9_999_999.0
|
||||
|
||||
# Simulate what the route handler does
|
||||
invalidate_models_cache()
|
||||
|
||||
# Cache must be cleared
|
||||
assert config_module._available_models_cache is None, (
|
||||
"REGRESSION: model cache not cleared after profile switch. Bug 2 regressed."
|
||||
)
|
||||
|
||||
|
||||
def test_regression_synctopbar_early_return_updates_profile_chip():
|
||||
"""
|
||||
REGRESSION GUARD (#1200 Bug 3): the syncTopbar() early-return branch (when
|
||||
S.session is null) must update the profileChipLabel.
|
||||
|
||||
If this fix is reverted, the profile chip stays on the old profile name even
|
||||
though S.activeProfile has been updated, because syncTopbar() exits early
|
||||
before reaching the chip-update code at the end of the function.
|
||||
"""
|
||||
from pathlib import Path
|
||||
|
||||
ui_js = (Path(__file__).parent.parent / "static" / "ui.js").read_text(encoding="utf-8")
|
||||
|
||||
fn_start = ui_js.find("function syncTopbar(){")
|
||||
assert fn_start != -1, "syncTopbar not found — has it been renamed?"
|
||||
|
||||
early_start = ui_js.find("if(!S.session){", fn_start)
|
||||
assert early_start != -1, "!S.session early-return block not found in syncTopbar"
|
||||
|
||||
early_end = ui_js.find("return;", early_start)
|
||||
assert early_end != -1, "return; not found after !S.session block"
|
||||
|
||||
early_block = ui_js[early_start : early_end + len("return;")]
|
||||
|
||||
assert "profileChipLabel" in early_block, (
|
||||
"REGRESSION: syncTopbar() early-return no longer updates profileChipLabel. "
|
||||
"Profile name chip won't update after switching profiles with no active session. "
|
||||
"Bug 3 regressed."
|
||||
)
|
||||
|
||||
|
||||
def test_regression_switch_profile_returns_target_model():
|
||||
"""
|
||||
REGRESSION GUARD (#1200): switch_profile(process_wide=False) must return the
|
||||
target profile's default model, not the process-global cached model.
|
||||
"""
|
||||
import api.profiles as profiles
|
||||
from pathlib import Path
|
||||
import tempfile
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
base = Path(tmp)
|
||||
target = base / "profiles" / "myp"
|
||||
target.mkdir(parents=True)
|
||||
ws = base / "mypws"; ws.mkdir()
|
||||
(target / "config.yaml").write_text(
|
||||
f"model:\n default: my-target-model\nterminal:\n cwd: {ws}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
orig = profiles._DEFAULT_HERMES_HOME
|
||||
orig_act = profiles._active_profile
|
||||
profiles._DEFAULT_HERMES_HOME = base
|
||||
profiles._active_profile = "default"
|
||||
profiles._tls.profile = None
|
||||
try:
|
||||
r = profiles.switch_profile("myp", process_wide=False)
|
||||
assert r.get("default_model") == "my-target-model", (
|
||||
f"REGRESSION: Got {r.get('default_model')!r} instead of 'my-target-model'. "
|
||||
"switch_profile() is not reading from target profile's config. Bug 2 regressed."
|
||||
)
|
||||
finally:
|
||||
profiles._DEFAULT_HERMES_HOME = orig
|
||||
profiles._active_profile = orig_act
|
||||
profiles._tls.profile = None
|
||||
@@ -36,6 +36,7 @@ global.window = {};
|
||||
global.document = { createElement: () => ({ innerHTML: '', textContent: '' }) };
|
||||
const esc = s => String(s ?? '').replace(/[&<>"']/g, c => (
|
||||
{'&':'&','<':'<','>':'>','"':'"',"'":'''}[c]));
|
||||
const _IMAGE_EXTS=/\.(png|jpg|jpeg|gif|webp|bmp|ico|avif)$/i;
|
||||
|
||||
function extractFunc(name) {
|
||||
const re = new RegExp('function\\s+' + name + '\\s*\\(');
|
||||
@@ -143,6 +144,42 @@ class TestBlockquotePrefixStrip:
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestRendererSanitization:
|
||||
"""Raw/model-provided HTML must not survive with executable attributes or schemes."""
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"payload, forbidden",
|
||||
[
|
||||
('<img src=x onerror=alert(1)>', 'onerror'),
|
||||
('<span onclick=alert(1)>click</span>', 'onclick'),
|
||||
('<div onmouseover=alert(1)>hover</div>', 'onmouseover'),
|
||||
('<a href="javascript:alert(1)">x</a>', 'javascript:'),
|
||||
],
|
||||
)
|
||||
def test_raw_html_dangerous_attributes_and_schemes_are_removed(self, driver_path, payload, forbidden):
|
||||
out = _render(driver_path, payload).lower()
|
||||
assert forbidden not in out, f"dangerous HTML survived sanitization: {out!r}"
|
||||
assert 'alert(1)' not in out, f"executable payload text should not remain executable: {out!r}"
|
||||
|
||||
def test_generated_image_markdown_uses_delegated_lightbox_not_inline_js(self, driver_path):
|
||||
out = _render(driver_path, "").lower()
|
||||
assert '<img' in out and 'msg-media-img' in out
|
||||
assert 'onclick' not in out
|
||||
assert '_openimglightbox' not in out
|
||||
|
||||
def test_media_token_image_uses_delegated_lightbox_not_inline_js(self, driver_path):
|
||||
out = _render(driver_path, "MEDIA:https://example.com/capy.png").lower()
|
||||
assert '<img' in out and 'msg-media-img' in out
|
||||
assert 'onclick' not in out
|
||||
assert '_openimglightbox' not in out
|
||||
|
||||
def test_incomplete_raw_html_tag_is_escaped_before_paragraph_wrapping(self, driver_path):
|
||||
out = _render(driver_path, '<img src=x onerror=alert(1)//').lower()
|
||||
assert '<img' in out
|
||||
assert '<img' not in out
|
||||
assert 'onerror' not in out or '<img' in out
|
||||
|
||||
|
||||
class TestCommonLLMShapes:
|
||||
|
||||
def test_strikethrough_outside_quote(self, driver_path):
|
||||
|
||||
@@ -183,8 +183,20 @@ def test_workspace_add_allows_external_valid_paths(tmp_path):
|
||||
|
||||
def test_workspace_add_rejects_system_paths():
|
||||
"""System paths (/, /etc, /sys) are always rejected even with the relaxed add validation."""
|
||||
_, status = post("/api/workspaces/add", {"path": "/etc", "name": "System"})
|
||||
for path in ("/etc", "/private/etc"):
|
||||
_, status = post("/api/workspaces/add", {"path": path, "name": "System"})
|
||||
assert status == 400, f"{path} should be rejected"
|
||||
|
||||
|
||||
def test_legacy_chat_rejects_workspace_outside_trusted_root(tmp_path):
|
||||
"""Legacy /api/chat must use the same trusted workspace validation as /api/chat/start."""
|
||||
d, _ = post("/api/session/new", {})
|
||||
sid = d["session"]["session_id"]
|
||||
outside = tmp_path / "outside-legacy-chat"
|
||||
outside.mkdir(parents=True, exist_ok=True)
|
||||
result, status = post("/api/chat", {"session_id": sid, "message": "hello", "workspace": str(outside)})
|
||||
assert status == 400
|
||||
assert "outside" in result.get("error", "").lower()
|
||||
|
||||
|
||||
def test_session_new_rejects_workspace_outside_trusted_root(tmp_path):
|
||||
|
||||
@@ -150,3 +150,26 @@ class TestNonSymlinkRootsUnchanged:
|
||||
# Use Path() not .resolve() — we want to assert the shape-based block,
|
||||
# not test whether the path actually exists on the test runner.
|
||||
assert _is_blocked_system_path(Path(subpath))
|
||||
|
||||
|
||||
# ── New macOS-specific blocked roots: /System and /Library ──────────────────
|
||||
|
||||
|
||||
class TestMacOSSystemAndLibraryBlocked:
|
||||
"""macOS has /System and /Library as top-level OS directories that must be
|
||||
blocked even on Linux (where they don't exist) since the path shapes are
|
||||
meaningful on macOS and should always be rejected.
|
||||
"""
|
||||
|
||||
@pytest.mark.parametrize("path", [
|
||||
'/Library',
|
||||
'/Library/Application Support',
|
||||
'/Library/Preferences',
|
||||
'/System',
|
||||
'/System/Library',
|
||||
'/System/Library/CoreServices',
|
||||
])
|
||||
def test_macos_os_roots_blocked(self, path):
|
||||
"""Paths under /Library and /System must be blocked regardless of platform."""
|
||||
from api.workspace import _is_blocked_workspace_path
|
||||
assert _is_blocked_workspace_path(Path(path))
|
||||
|
||||
Reference in New Issue
Block a user