From e2748fe9617861f31025a6d9dbc1f258a9381341 Mon Sep 17 00:00:00 2001 From: Nathan Esquenazi Date: Tue, 5 May 2026 02:12:57 +0000 Subject: [PATCH] Apply Opus pre-release SHOULD-FIX (absorbed in stage-299) Per Opus advisor on stage-299: 1. Bounded WIKI_PATH walk + forbidden-root guard (api/routes.py) - _LLM_WIKI_MAX_FILES = 10000 caps rglob iteration (prevents hangs on symlink loops or pathologically-large trees) - _LLM_WIKI_FORBIDDEN_ROOTS blocklist refuses '/' '/etc' '/usr' '/var' '/opt' '/sys' '/proc' even if WIKI_PATH is misconfigured to point at them - Self-DoS prevention: /api/wiki/status fires on every Insights tab open via Promise.all, and unbounded rglob would block the endpoint 2. URL-scheme guard for docs_url interpolation (static/panels.js) - rawDocsUrl is regex-validated against /^https?:\/\//i before being interpolated into the attribute - esc() HTML-escapes but doesn't validate URL scheme; docs_url is server-controlled today but the contributor scaffolded it for potential config-driven use, so future-proof against javascript: scheme XSS 6 regression tests in tests/test_stage299_opus_fixes.py pin both fixes. --- api/routes.py | 31 +++++++++++ static/panels.js | 5 +- tests/test_stage299_opus_fixes.py | 85 +++++++++++++++++++++++++++++++ 3 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 tests/test_stage299_opus_fixes.py diff --git a/api/routes.py b/api/routes.py index 6d85dcbf..85bb6627 100644 --- a/api/routes.py +++ b/api/routes.py @@ -1804,6 +1804,15 @@ def _llm_wiki_config_path() -> str | None: ) +# Cap WIKI walks to prevent self-DoS if WIKI_PATH points at /, /etc, /home, etc. +# Real LLM wikis have under a few thousand files; 10k is generous and catches misconfig. +_LLM_WIKI_MAX_FILES = 10000 +# Refuse to walk these system roots even if explicitly configured. +_LLM_WIKI_FORBIDDEN_ROOTS = frozenset( + str(Path(p).expanduser().resolve()) for p in ("/", "/etc", "/usr", "/var", "/opt", "/sys", "/proc") +) + + def _llm_wiki_resolve_path() -> tuple[Path, str, bool]: hermes_home = _llm_wiki_active_hermes_home() raw = os.getenv("WIKI_PATH") or _llm_wiki_env_file_path(hermes_home) @@ -1832,8 +1841,20 @@ def _llm_wiki_safe_iso(ts: float | None) -> str | None: def _llm_wiki_count_files(root: Path) -> int: if not root.exists() or not root.is_dir(): return 0 + # Defense in depth: refuse to walk forbidden system roots even if WIKI_PATH + # was set to one. The endpoint is auth-gated but a misconfigured server + # shouldn't self-DoS by rglob'ing all of /etc on every Insights load. + try: + if str(root.resolve()) in _LLM_WIKI_FORBIDDEN_ROOTS: + return 0 + except Exception: + return 0 count = 0 + iterated = 0 for item in root.rglob("*"): + iterated += 1 + if iterated > _LLM_WIKI_MAX_FILES: + break # bounded — prevents hangs on symlink loops or huge trees try: if item.is_file() and not any(part.startswith(".") for part in item.relative_to(root).parts): count += 1 @@ -1844,11 +1865,21 @@ def _llm_wiki_count_files(root: Path) -> int: def _llm_wiki_page_files(wiki_path: Path) -> list[Path]: pages: list[Path] = [] + # Defense in depth: refuse forbidden system roots. + try: + if str(wiki_path.resolve()) in _LLM_WIKI_FORBIDDEN_ROOTS: + return pages + except Exception: + return pages + iterated = 0 for dirname in _LLM_WIKI_PAGE_DIRS: section = wiki_path / dirname if not section.exists() or not section.is_dir(): continue for item in section.rglob("*.md"): + iterated += 1 + if iterated > _LLM_WIKI_MAX_FILES: + return pages # bounded try: rel = item.relative_to(section) if item.is_file() and not any(part.startswith(".") for part in rel.parts): diff --git a/static/panels.js b/static/panels.js index 6f955db1..ea5ddf3f 100644 --- a/static/panels.js +++ b/static/panels.js @@ -2141,7 +2141,10 @@ function _renderLlmWikiStatus(d) { const isError = status.status === 'error'; const badgeClass = isReady ? 'ok' : isError ? 'err' : isEmpty ? 'warn' : 'muted'; const badgeText = isReady ? 'Available' : isError ? 'Error' : isEmpty ? 'Empty' : 'Unavailable'; - const docsUrl = status.docs_url || 'https://hermes-agent.nousresearch.com/docs/user-guide/skills/bundled/research/research-llm-wiki'; + const rawDocsUrl = status.docs_url || 'https://hermes-agent.nousresearch.com/docs/user-guide/skills/bundled/research/research-llm-wiki'; + // Guard against unsafe URL schemes (e.g. js: / data:) if docs_url ever + // becomes config-driven. esc() HTML-escapes but doesn't validate URL scheme. + const docsUrl = /^https?:\/\//i.test(rawDocsUrl) ? rawDocsUrl : '#'; const toggleNote = status.toggle_available ? 'Toggle available from configured Hermes Agent setting.' : (status.toggle_reason || 'No stable LLM Wiki on/off config flag was detected, so this panel is read-only.'); diff --git a/tests/test_stage299_opus_fixes.py b/tests/test_stage299_opus_fixes.py new file mode 100644 index 00000000..8c1aeb64 --- /dev/null +++ b/tests/test_stage299_opus_fixes.py @@ -0,0 +1,85 @@ +"""Regression test for the Opus SHOULD-FIX bounds applied in stage-299. + +PR #1664 introduced /api/wiki/status with `_llm_wiki_count_files` and +`_llm_wiki_page_files` that walk WIKI_PATH via `rglob`. Without bounds, +a misconfigured WIKI_PATH=/ or symlink loop would hang the endpoint. + +These tests pin the defenses applied per Opus advisor on stage-299: +- A constant cap on iteration (_LLM_WIKI_MAX_FILES) for both functions +- A forbidden-roots blocklist (_LLM_WIKI_FORBIDDEN_ROOTS) that includes + '/' / '/etc' / '/usr' / '/var' / '/opt' / '/sys' / '/proc' (resolved + to absolute strings) +- Bounded behavior: if WIKI_PATH points at a forbidden root, both + functions return 0/empty without iterating +""" +from pathlib import Path + +ROUTES_PY = Path(__file__).parent.parent / "api" / "routes.py" + + +def _read_source(): + return ROUTES_PY.read_text() + + +def test_wiki_max_files_constant_present(): + src = _read_source() + assert "_LLM_WIKI_MAX_FILES" in src + assert "_LLM_WIKI_FORBIDDEN_ROOTS" in src + # Make sure cap is reasonable (≥ a few thousand, ≤ 100k) + assert "10000" in src or "_LLM_WIKI_MAX_FILES = 10" in src + + +def test_count_files_has_iteration_cap(): + src = _read_source() + # Locate _llm_wiki_count_files body + start = src.find("def _llm_wiki_count_files(") + end = src.find("\ndef ", start + 1) + body = src[start:end] + assert "_LLM_WIKI_MAX_FILES" in body + assert "_LLM_WIKI_FORBIDDEN_ROOTS" in body + assert "iterated > _LLM_WIKI_MAX_FILES" in body or "iterated >= _LLM_WIKI_MAX_FILES" in body + + +def test_page_files_has_iteration_cap(): + src = _read_source() + start = src.find("def _llm_wiki_page_files(") + end = src.find("\ndef ", start + 1) + body = src[start:end] + assert "_LLM_WIKI_MAX_FILES" in body + assert "_LLM_WIKI_FORBIDDEN_ROOTS" in body + + +def test_forbidden_roots_includes_system_paths(): + src = _read_source() + # Find the constant definition + start = src.find("_LLM_WIKI_FORBIDDEN_ROOTS = ") + end = src.find(")\n", start) + 1 + decl = src[start:end + 1] + for forbidden in ("/", "/etc", "/usr", "/var"): + assert f'"{forbidden}"' in decl, f"Forbidden root {forbidden!r} not in _LLM_WIKI_FORBIDDEN_ROOTS" + + +def test_count_files_returns_zero_for_forbidden_root(tmp_path, monkeypatch): + """Behavioral test: walking a forbidden root returns 0 without iterating.""" + import importlib + routes = importlib.import_module("api.routes") + + forbidden_root = Path("/etc") + if forbidden_root.exists(): # skip on systems without /etc (Windows) + result = routes._llm_wiki_count_files(forbidden_root) + assert result == 0, "Walking /etc should return 0 (forbidden root guard)" + + +def test_render_llm_wiki_status_uses_url_scheme_guard(): + """Opus SHOULD-FIX #1: docs_url interpolated into href must be scheme-guarded.""" + panels_js = (Path(__file__).parent.parent / "static" / "panels.js").read_text() + # Find the _renderLlmWikiStatus function body + start = panels_js.find("function _renderLlmWikiStatus") + end = panels_js.find("\nfunction ", start + 1) + body = panels_js[start:end] + # Must use a scheme-guarded form, not raw esc() + assert "/^https?:" in body or "test(rawDocsUrl)" in body or "test(docsUrl)" in body, ( + "Expected URL scheme guard (e.g. /^https?:\\/\\//.test(...)) before " + "interpolating docsUrl into href to prevent javascript: scheme XSS " + "if docs_url ever becomes config-driven." + )