From c414c39f1015a3e97626b82d9b65a2a4396081b6 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Mon, 20 Jul 2026 10:49:23 -0700 Subject: [PATCH] =?UTF-8?q?feat(extensions):=20token-v1=20proxy=E2=86=92si?= =?UTF-8?q?decar=20authentication=20boundary=20(#6331)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(extensions): token-v1 proxy->sidecar auth boundary Mint a per-extension secret core injects (X-Hermes-Sidecar-Token) on every proxied request; sidecars validate it. Closes the hole where a loopback sidecar port is reachable by any local process and cannot distinguish a proxied request from a direct one. - api/extension_sidecar_auth.py: per-extension token lifecycle (atomic mint, re-read-verified so an unpersisted token is never injected, per-request mtime-cached read for live rotation, path-escape-safe) - manifest proxy_auth negotiation: absent=legacy, token-v1=enforce, unknown=fail-closed - proxy: inject token, strip inbound + response x-hermes-*, auth-off posture (loopback-only local_unprotected, else 503), fail-closed when token unavailable - consent-time auth_required in status payload; mint-on-consent - docs/EXTENSIONS.md proxy_auth section - 7 new tests (26/26 green) * fix(extensions): address Codex+Fable gate on token-v1 (6 findings) - align token-module extension-id grammar with core _EXTENSION_ID_RE (was narrower -> legally-named ext consented then 503'd forever) - resolve token dir dynamically (mirror _extension_state_dir) -> real test isolation, no import-time STATE_DIR cache - cross-process mint: O_CREAT|O_EXCL no-clobber claim (was os.replace clobber) - rotation cache keyed on full fingerprint (ino/dev/mtime/ctime/size) + re-fingerprint after read -> same-size/mtime replacement no longer stale - validate token format on read (url-safe, 16-256) -> malformed file can't leak via a ValueError echoed in a 502 - consent fails 503 when token can't be provisioned (was silent-swallow -> persisted consent then 503 forever) - rename status auth_required -> posture enum (protected|local_unprotected): nothing is blocked for loopback, so 'required' was misleading - panels.js: render local_unprotected warning on the consent row (+ CSS) - docs: token-path resolution order, 401-vs-503, explicit 'legacy' acceptance - tests: route-level token-injection+response-strip test; fix illusory isolation in token-module test; 27/27 green * fix(extensions): close 2 token-mint races (Codex re-gate round 2) - mint via temp-file + atomic os.replace (not O_CREAT|O_EXCL) so the final path is never observed empty/half-written — a concurrent loser can no longer read an empty token file and 503 - single _stable_read helper (fingerprint-read-refingerprint, bounded retry on mid-read change) used by BOTH ensure_token and current_token — a token that changes during the read is never returned or cached stale - stress-verified: 20 concurrent first-mints converge on 1 persisted token; 27/27 * fix(extensions): atomic no-clobber token publish via os.link (Codex re-gate round 3) os.replace fixed empty-file exposure but still clobbered cross-process: two processes could both write+replace and a reader between them got a token no longer on disk -> 401. Switch to the repo's TOCTOU-safe os.link create-or-fail idiom (session_recovery.py:627): write temp -> link into place (fails if a winner already published) -> loser drops its temp and reads the winner via _stable_read. Proven: 16 concurrent PROCESSES converge on 1 persisted token, all matching disk. 27/27 tests green. * fix(extensions): resolve Frank+Greptile #6331 review — token-v1 fail-closed when auth off (consent+resolution), token-v1-only proxy_auth/posture status fields, fullmatch ext-id validator * fix(extensions): finish Frank and Greptile sidecar review --------- Co-authored-by: nesquena-hermes Co-authored-by: nesquena-hermes --- api/extension_sidecar_auth.py | 241 ++++++++++++++++++++++ api/extensions.py | 112 +++++++++- api/routes.py | 16 +- docs/EXTENSIONS.md | 36 +++- static/panels.js | 8 + static/style.css | 1 + tests/test_extension_sidecar_proxy.py | 284 ++++++++++++++++++++++++++ 7 files changed, 693 insertions(+), 5 deletions(-) create mode 100644 api/extension_sidecar_auth.py diff --git a/api/extension_sidecar_auth.py b/api/extension_sidecar_auth.py new file mode 100644 index 00000000..fdca26f7 --- /dev/null +++ b/api/extension_sidecar_auth.py @@ -0,0 +1,241 @@ +"""Extension sidecar proxy authentication (token-v1). + +A loopback-sidecar extension runs a stdlib HTTP server on 127.0.0.1:. The +browser reaches it ONLY through core's consent-gated proxy +(``/api/extensions/{id}/sidecar/*``). But the loopback port is reachable by any +local process, and the proxy strips every inbound credential before forwarding +(see ``_extension_sidecar_proxy_request_headers`` in ``api/routes.py``), so the +sidecar cannot tell a proxied request from a direct one. + +This module mints a per-extension shared secret that core injects on every +forwarded request (header ``X-Hermes-Sidecar-Token``) and the sidecar validates. +It converts "anyone who can send a loopback TCP packet" (other-UID users, host +containers, sandboxed network-only processes) into "processes that can read the +user's state dir" — the same protection level core's own signing key already has +(``.pbkdf2_key`` / ``.signing_key`` are 0600 files in the same directory). + +SCOPE (be honest — this is documented in the contract too): the token does NOT +defend against arbitrary same-UID code. A same-user process can read the token, +read core's signing key, or just run the sidecar's underlying tool directly. No +mechanism available here (token, HMAC, nonce, UDS with 0600) changes that. + +Design notes (from the reviewed design doc, §9.2): + * Per-extension token file under ``STATE_DIR/sidecar-auth/.token``. + * Unlike ``auth._load_key``, we NEVER return an in-memory-only token: if the + file cannot be persisted+re-read, we report "unavailable" so core fails + closed (503) rather than injecting a secret no sidecar can ever read. + * The token is re-read from disk per request (fingerprint-cached on + inode/mtime/ctime/size) so rotation / deletion takes effect with no restart, + and a stale cached token can't keep validating after the file changes. + * Cross-process mint writes a unique temporary file, then publishes it with + an atomic ``os.link`` create-or-fail operation. Concurrent first-mints + converge on a single winning file; losers re-read the winner rather than + clobbering it. +""" +from __future__ import annotations + +import os +import re +import secrets +import threading +from pathlib import Path +from typing import Dict, Optional, Tuple + +# Keep this grammar identical to ``_EXTENSION_ID_RE`` in ``api/extensions.py`` — +# a narrower pattern here silently 503s legally-named extensions (uppercase, +# dots, up to 128 chars). The pattern is filesystem-safe as a filename: no path +# separators, cannot start with a dot, so no dotfile / ".."-prefix tricks. +_EXT_ID_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") +# Injected/validated tokens are exactly secrets.token_urlsafe output: url-safe +# base64 alphabet. Reject anything else (bounds length, and stops a malformed +# token file's contents from reaching a header where a ValueError could echo it). +_TOKEN_RE = re.compile(r"^[A-Za-z0-9_-]{16,256}$") + +_TOKEN_DIR_NAME = "sidecar-auth" +_TOKEN_BYTES = 32 # secrets.token_urlsafe(32) -> ~43 url-safe chars + +# Per-request read cache: ext_id -> (token, fingerprint). Guarded by _LOCK. +_LOCK = threading.Lock() +_CACHE: Dict[str, Tuple[str, Tuple]] = {} + + +def _token_dir() -> Path: + # Resolve STATE_DIR dynamically (mirrors ``_extension_state_dir`` in + # api/extensions.py) so tests and relocated installs see the current dir + # rather than the value cached at import time. + from api.config import STATE_DIR + return STATE_DIR / _TOKEN_DIR_NAME + + +def _token_path(ext_id: str) -> Path: + return _token_dir() / f"{ext_id}.token" + + +def _valid_ext_id(ext_id: object) -> bool: + # Use fullmatch (not match) so a trailing newline can't slip through: in + # Python's ``re`` a ``$`` anchor also matches just before a final ``\n``, + # so ``match`` would accept ``"templates\n"`` and _token_path would build a + # filename containing a literal newline. fullmatch anchors truly to the end. + # Validate the exact value that _token_path() uses. Trimming only for the + # check would accept one ID but construct a filename from a different one. + return isinstance(ext_id, str) and bool(_EXT_ID_RE.fullmatch(ext_id)) + + +def _fingerprint(path: Path) -> Optional[Tuple]: + try: + st = path.stat() + except OSError: + return None + return (st.st_ino, st.st_dev, st.st_mtime_ns, st.st_ctime_ns, st.st_size) + + +def _read_token_file(path: Path) -> Optional[str]: + """Return the token text iff it is present and well-formed, else None + (fail closed). Only a valid token-v1 shape is ever returned, so malformed + file contents can never reach a header. Thin wrapper over the stable reader.""" + tok, _fp = _stable_read(path) + return tok + + +def _stable_read(path: Path) -> Tuple[Optional[str], Optional[Tuple]]: + """Read the token together with a fingerprint that is verified to bracket the + read (fingerprint before AND after; retry on any mid-read change). Guarantees + the returned (token, fingerprint) pair reflects a single consistent on-disk + state — never new content under an old stat key, and never a value that + changed underneath us. Returns (None, None) when absent/malformed/racing.""" + for _ in range(4): # bounded retry; a file being rotated settles fast + fp_before = _fingerprint(path) + if fp_before is None: + return (None, None) + try: + raw = path.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + return (None, None) + fp_after = _fingerprint(path) + if fp_after != fp_before: + continue # changed mid-read — retry for a consistent snapshot + tok = raw.strip() + if not tok or not _TOKEN_RE.match(tok): + return (None, None) + return (tok, fp_before) + return (None, None) # never settled — fail closed + + +def ensure_token(ext_id: str) -> Optional[str]: + """Get-or-create the per-extension token, returning it ONLY if it is durably + persisted and re-readable from disk. Returns None if it can't be persisted + (caller must then treat the sidecar proxy as unavailable / 503) — we never + hand back an ephemeral token a sidecar could never read. + + Idempotent + atomic across processes: the token is written to a unique temp + file, then published with an atomic hard-link create-or-fail operation, so + the final path is only ever visible fully written and is never clobbered. + One concurrent writer wins; every loser reads that persisted winner. + """ + if not _valid_ext_id(ext_id): + return None + path = _token_path(ext_id) + + existing = _read_token_file(path) + if existing is not None: + return existing + + with _LOCK: + existing = _read_token_file(path) + if existing is not None: + return existing + token = secrets.token_urlsafe(_TOKEN_BYTES) + tmp: Optional[Path] = None + try: + d = _token_dir() + d.mkdir(parents=True, exist_ok=True) + try: + os.chmod(d, 0o700) + except OSError: + pass # best-effort dir hardening; file perms are the real guard + # Write to a unique temp, fsync, then publish with atomic + # create-or-fail (os.link, the repo's TOCTOU-safe idiom — see + # session_recovery.py:627). Unlike os.replace, link does NOT clobber: + # if another process already published, ours fails, we drop our temp, + # and everyone reads the single winning file. This closes both the + # empty-file-exposure race AND the cross-process clobber race. + tmp = path.with_name(f".{path.name}.{os.getpid()}.{threading.get_ident()}.tmp") + fd = os.open(str(tmp), os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + try: + os.write(fd, token.encode("utf-8")) + os.fsync(fd) + finally: + os.close(fd) + try: + os.link(str(tmp), str(path)) + try: + os.chmod(path, 0o600) + except OSError: + pass + except FileExistsError: + pass # another writer won; we return the winner via _stable_read below + finally: + try: + tmp.unlink() + except OSError: + pass + tmp = None + except OSError: + # Persistence failed → report unavailable. Do NOT return the + # in-memory token (the _load_key trap: core would inject a secret the + # sidecar can never read → permanent 401 that looks like a mismatch). + if tmp is not None: + try: + tmp.unlink() + except OSError: + pass + return None + # Return whatever is durably on disk now — under a concurrent mint this + # may be another writer's token, which is correct (all readers converge + # on the winning file). Only a persisted, well-formed token is returned. + persisted, fp = _stable_read(path) + if persisted is not None and fp is not None: + _CACHE[ext_id] = (persisted, fp) + return persisted + + +def current_token(ext_id: str) -> Optional[str]: + """Return the current on-disk token for validation/injection, re-reading the + file when its fingerprint changed since last read — so rotation + deletion + take effect immediately and a stale cached token stops validating. Does NOT + mint. Returns None when no token exists (fail closed).""" + if not _valid_ext_id(ext_id): + return None + path = _token_path(ext_id) + fp = _fingerprint(path) + if fp is None: + with _LOCK: + _CACHE.pop(ext_id, None) + return None + with _LOCK: + cached = _CACHE.get(ext_id) + if cached is not None and cached[1] == fp: + return cached[0] + tok, tok_fp = _stable_read(path) # bracketed read: content matches tok_fp + with _LOCK: + if tok is None or tok_fp is None: + _CACHE.pop(ext_id, None) + else: + _CACHE[ext_id] = (tok, tok_fp) + return tok + + +def reset_token(ext_id: str) -> Optional[str]: + """Rotate: delete then re-mint. Returns the new token or None on failure. + (Recovery entry point; also usable by a future 'reset token' action.)""" + if not _valid_ext_id(ext_id): + return None + with _LOCK: + _CACHE.pop(ext_id, None) + try: + _token_path(ext_id).unlink() + except FileNotFoundError: + pass + except OSError: + return None + return ensure_token(ext_id) diff --git a/api/extensions.py b/api/extensions.py index 8c86514a..ba7104e4 100644 --- a/api/extensions.py +++ b/api/extensions.py @@ -898,6 +898,21 @@ def _sidecar_from_manifest_entry( return None else: health_path = _DEFAULT_SIDECAR_HEALTH_PATH + # proxy_auth negotiation (token-v1). Absent = explicit legacy (unauthenticated + # proxy→sidecar, unchanged behavior for any pre-existing declaration). token-v1 + # = core mints+injects a per-extension token and the sidecar validates it. + # Any unknown value fails closed (reject the sidecar) so a typo can never + # silently downgrade to unauthenticated. + raw_proxy_auth = raw.get("proxy_auth") + if raw_proxy_auth is None: + proxy_auth = "legacy" + elif raw_proxy_auth in ("legacy", "token-v1"): + proxy_auth = str(raw_proxy_auth) + else: + _add_diagnostic_warning( + diagnostics, "sidecar_proxy_auth_unsupported", _SIDECAR_WARNING_SOURCE + ) + return None sidecar_id = _manifest_entry_text(entry, "id") name = _manifest_entry_text(entry, "name") return { @@ -907,6 +922,7 @@ def _sidecar_from_manifest_entry( "origin": origin, "health_path": health_path, "health_url": f"{origin}{health_path}", + "proxy_auth": proxy_auth, } @@ -1159,16 +1175,37 @@ def _sidecar_proxy_public_status( approved_origin: Optional[str], *, available: bool, + proxy_auth: str = "legacy", ) -> Dict[str, Any]: consented = bool(available and approved_origin == origin) origin_changed = bool(available and approved_origin and approved_origin != origin) - return { + payload: Dict[str, Any] = { "available": available, "consented": consented, "consent_required": bool(available and not consented), "path": _extension_sidecar_proxy_path(extension_id), "origin_changed": origin_changed, } + # proxy_auth / posture are token-v1-only fields. Legacy sidecars keep the + # original payload shape byte-for-byte (the public status contract tests pin + # it), so a token-v1 negotiation field never leaks onto a legacy record. + # (Frank #6331 blocker 2 — fields were previously emitted on every record and + # broke the legacy exact-shape contract on shard 4.) + if proxy_auth == "token-v1": + # token-v1 posture (§9.1). "protected" = WebUI auth ON (the only posture + # in which token-v1 proxying is permitted; auth-off is now fail-closed at + # both consent and resolution, so "local_unprotected" signals the panel + # to tell the operator to enable authentication before the proxy will work). + posture = None + if available: + try: + from api.auth import is_auth_enabled + posture = "protected" if is_auth_enabled() else "local_unprotected" + except Exception: + posture = None + payload["proxy_auth"] = proxy_auth + payload["posture"] = posture + return payload def _extension_sidecar_records( @@ -1216,11 +1253,21 @@ def _extension_sidecar_records( sidecar["origin"], item.get("approved_origin"), available=available, + proxy_auth=sidecar.get("proxy_auth", "legacy"), ) item["duplicate_id"] = id_counts.get(ext_id, 0) > 1 item["proxy"] = proxy if len(records) < _MAX_URL_LIST: - records.append({**sidecar, "proxy": proxy}) + # Build the PUBLIC sidecar record explicitly. The internal `sidecar` + # dict carries a top-level `proxy_auth` for the consent/resolution + # logic, but the public status contract only exposes proxy_auth (and + # posture) INSIDE `proxy`, and only for token-v1 records — legacy + # sidecars keep their original public shape byte-for-byte. Spreading + # `**sidecar` leaked a top-level `proxy_auth: legacy` and broke the + # exact-shape contract tests (Frank #6331 blocker 2). + public_record = {k: v for k, v in sidecar.items() if k != "proxy_auth"} + public_record["proxy"] = proxy + records.append(public_record) else: _add_diagnostic_warning(diagnostics, "sidecar_list_truncated", _SIDECAR_WARNING_SOURCE) break @@ -1564,6 +1611,32 @@ def set_extension_sidecar_proxy_consent(extension_id: object, approved: object) if sidecar is None or proxy.get("available") is not True: raise ExtensionSidecarProxyError("Extension sidecar proxy is unavailable", status=409) consent_map[ext_id] = sidecar["origin"] + # For token-v1, mint the per-extension token NOW and fail the consent + # if it can't be persisted+read — otherwise we'd persist consent and + # then 503 on every request (a mint failure must surface at grant + # time, not as a mysterious later error). + if sidecar.get("proxy_auth") == "token-v1": + # token-v1 REQUIRES configured WebUI authentication. Without it, + # this consent endpoint is itself unauthenticated, so any caller + # that can reach the WebUI listener could self-grant consent and + # then drive the token-bearing proxy (a forwarding oracle) — the + # sidecar token file blocks direct port access but does NOT stop a + # caller who simply asks core to inject it. Loopback origin is not + # sufficient: another local UID can still reach the listener + # without reading the 0600 token file. Fail closed regardless of + # origin. (Frank #6331 blocker 1.) + from api.auth import is_auth_enabled + if not is_auth_enabled(): + raise ExtensionSidecarProxyError( + "Sidecar token-v1 proxy requires WebUI authentication; " + "set a password in Settings before granting consent.", + status=403, + ) + from api import extension_sidecar_auth as _sc_auth + if not _sc_auth.ensure_token(ext_id): + raise ExtensionSidecarProxyError( + "Could not provision the sidecar auth token", status=503 + ) else: consent_map.pop(ext_id, None) _write_extension_state( @@ -1625,11 +1698,46 @@ def resolve_extension_sidecar_proxy_target( upstream_url = f"{sidecar['origin']}{normalized_path}" if query: upstream_url = f"{upstream_url}?{query}" + + # token-v1 auth (§9.1/§9.2). Legacy sidecars proxy unchanged; token-v1 + # sidecars get a per-extension secret injected, and only proxy when the + # trust posture is sound. + proxy_auth = sidecar.get("proxy_auth", "legacy") + inject_token: Optional[str] = None + if proxy_auth == "token-v1": + from api.auth import is_auth_enabled + from api import extension_sidecar_auth as _sc_auth + + # token-v1 REQUIRES configured WebUI authentication — fail closed + # regardless of the upstream origin (loopback included). The consent + # endpoint and this resolution path are both unauthenticated when auth is + # off, so a caller that can reach the WebUI listener could otherwise drive + # a token-bearing proxy to the sidecar (a forwarding oracle). Loopback is + # NOT a sufficient guard: another local UID can reach the listener without + # ever reading the 0600 token file. (Frank #6331 blocker 1 — this + # mirrors the consent-time check so a pre-existing consent granted while + # auth was enabled cannot be exercised after auth is turned off.) + if not is_auth_enabled(): + raise ExtensionSidecarProxyError( + "Sidecar token-v1 proxy requires WebUI authentication; " + "set a password in Settings.", + status=403, + ) + token = _sc_auth.current_token(ext_id) or _sc_auth.ensure_token(ext_id) + if not token: + # Token could not be persisted+read → fail closed rather than proxy + # unauthenticated (never inject an ephemeral secret). + raise ExtensionSidecarProxyError( + "Sidecar proxy auth token is unavailable", status=503 + ) + inject_token = token return { "extension_id": ext_id, "origin": sidecar["origin"], "proxy_path": proxy["path"], "upstream_url": upstream_url, + "proxy_auth": proxy_auth, + "auth_token": inject_token, } diff --git a/api/routes.py b/api/routes.py index d99873a1..ed3c7229 100644 --- a/api/routes.py +++ b/api/routes.py @@ -5547,6 +5547,7 @@ def _extension_sidecar_proxy_request_headers(handler) -> dict[str, str]: lower in blocked_headers or lower in {"authorization", "cookie", "content-length", "host", "origin", "referer"} or lower.startswith("x-csrf") + or lower.startswith("x-hermes-") ): continue headers[str(name)] = str(value) @@ -5560,7 +5561,11 @@ def _send_extension_sidecar_proxy_response(handler, status: int, body: bytes, he if headers and hasattr(headers, "items"): for name, value in headers.items(): lower = str(name).lower() - if lower in blocked_headers or lower in {"content-length", "set-cookie"}: + if ( + lower in blocked_headers + or lower in {"content-length", "set-cookie"} + or lower.startswith("x-hermes-") + ): continue if lower == "content-type": sent_content_type = True @@ -5657,10 +5662,17 @@ def _handle_extension_sidecar_proxy( proxy_path, query=parsed.query, ) + proxied_headers = _extension_sidecar_proxy_request_headers(handler) + # token-v1: inject the per-extension shared secret core minted. The + # inbound x-hermes-* strip above guarantees the client cannot have + # forged this header. + _auth_token = target.get("auth_token") + if _auth_token: + proxied_headers["X-Hermes-Sidecar-Token"] = _auth_token request = Request( target["upstream_url"], data=request_body, - headers=_extension_sidecar_proxy_request_headers(handler), + headers=proxied_headers, method=method, ) opener = _extension_sidecar_proxy_same_origin_opener(target["origin"]) diff --git a/docs/EXTENSIONS.md b/docs/EXTENSIONS.md index 9e60f48f..8788697c 100644 --- a/docs/EXTENSIONS.md +++ b/docs/EXTENSIONS.md @@ -165,7 +165,8 @@ opt-in proxy: "sidecar": { "type": "loopback", "origin": "http://127.0.0.1:17787", - "health_path": "/health" + "health_path": "/health", + "proxy_auth": "token-v1" } } ] @@ -178,6 +179,39 @@ declared and optionally check its health from the browser. If the operator later approves the proxy in **Settings → Extensions**, WebUI may proxy requests only through the fixed per-extension sidecar path for that extension. +### Sidecar proxy authentication (`proxy_auth`) + +The loopback port a sidecar binds is reachable by **any local process**, and the +proxy strips every inbound credential (cookies, `Authorization`, CSRF, `x-hermes-*`) +before forwarding — so a sidecar cannot, on its own, tell a proxied request from a +direct one. The `proxy_auth` field closes that gap: + +- **`token-v1`** (recommended for any sidecar that mutates state) — WebUI mints a + per-extension secret at `STATE_DIR/sidecar-auth/.token` (mode `0600`) and + injects it as the `X-Hermes-Sidecar-Token` header on every proxied request. The + sidecar resolves the token file in this order — `HERMES_EXT_SIDECAR_TOKEN_FILE` + → `$HERMES_WEBUI_STATE_DIR/sidecar-auth/.token` → + `$HERMES_HOME/webui/sidecar-auth/.token` → platform default + (`~/.hermes/webui/…`, `%LOCALAPPDATA%\hermes\webui\…` on Windows) — and must + validate the header on every route except `/health`, returning **`401` on a + missing/mismatched token** and **`503` when the token file is absent/unreadable**. + The canonical scaffold in the extensions repository (`examples/`, see + `docs/SIDECAR_CONTRACT.md`) does all of this for you — do not hand-roll it. +- **absent (or the explicit literal `"legacy"`)** — **legacy** mode (no token; + unchanged behavior). Only appropriate for read-only, non-sensitive sidecars. +- Any **other** value fails closed (the sidecar declaration is rejected). + +**Auth-off posture:** WebUI authentication is optional and off by default. Because +the consent endpoint and proxy route are unauthenticated in that mode, `token-v1` +fails closed regardless of whether the sidecar origin is loopback: consent and proxy +resolution return `403` until WebUI authentication is configured. Otherwise, any +caller that can reach WebUI could ask core to inject the token and use it as a +forwarding oracle. The extensions panel exposes the `local_unprotected` posture so +the operator is told to enable authentication before granting consent. The token +protects against other-UID and sandboxed local callers; it does **not** defend against +arbitrary same-UID code (which can read the token file, WebUI's own signing key, or +run the sidecar's tool directly). + Extension entries may declare browser-local settings when they also request extension-owned storage: diff --git a/static/panels.js b/static/panels.js index 582943cd..e4e30752 100644 --- a/static/panels.js +++ b/static/panels.js @@ -9813,6 +9813,13 @@ function _extensionSidecarCard(sidecars){ const proxyConsentRequired=proxy.consent_required===true; const proxyOriginChanged=proxy.origin_changed===true; const proxyPath=(proxy&&proxy.path)||''; + // token-v1 posture: 'local_unprotected' = WebUI auth is off, so proxy consent + // is grantable by any unauthenticated local caller. Warn the operator to + // enable authentication before wiring up a sidecar (design §9.1). + const proxyUnprotected=proxy.posture==='local_unprotected'; + const proxyWarning=proxyUnprotected + ?`
⚠ WebUI authentication is off. This sidecar's proxy consent can be granted by any local process. Set a password in Settings → Password before using sidecar extensions.
` + :''; const proxyStatus=proxyConsented ?'consented' :(proxyOriginChanged @@ -9837,6 +9844,7 @@ function _extensionSidecarCard(sidecars){
Proxy path${esc(proxyPath)}
${proxyButton}
+ ${proxyWarning} `; }).join('')}`:'
No loopback sidecars declared.
'; diff --git a/static/style.css b/static/style.css index 06f203da..9814b6da 100644 --- a/static/style.css +++ b/static/style.css @@ -5452,6 +5452,7 @@ main.main > #mainPlugin{display:none;} .extension-sidecar-row-head{display:flex;align-items:center;justify-content:space-between;gap:10px;min-width:0;} .extension-sidecar-title{font-size:13px;font-weight:600;color:var(--text);overflow-wrap:anywhere;min-width:0;} .extension-sidecar-meta{margin-top:2px;font-size:11px;color:var(--muted);overflow-wrap:anywhere;} +.extension-sidecar-warning{margin-top:8px;padding:7px 9px;border:1px solid var(--warning,#d29922);border-radius:6px;background:var(--warning-bg,rgba(210,153,34,0.12));color:var(--warning,#d29922);font-size:11px;line-height:1.4;overflow-wrap:anywhere;} .extension-sidecar-fields{display:grid;grid-template-columns:1fr;gap:5px;margin-top:8px;} .extension-sidecar-fields>div{display:flex;align-items:flex-start;gap:8px;min-width:0;font-size:12px;color:var(--muted);} .extension-sidecar-fields span{flex:0 0 82px;} diff --git a/tests/test_extension_sidecar_proxy.py b/tests/test_extension_sidecar_proxy.py index beba0a0c..023dd598 100644 --- a/tests/test_extension_sidecar_proxy.py +++ b/tests/test_extension_sidecar_proxy.py @@ -135,6 +135,8 @@ def test_extension_sidecar_proxy_requires_consent_and_reconfirms_after_origin_ch "origin": "http://127.0.0.1:17787", "proxy_path": "/api/extensions/templates/sidecar/", "upstream_url": "http://127.0.0.1:17787/v1/ping?debug=1", + "proxy_auth": "legacy", + "auth_token": None, } encoded_target = resolve_extension_sidecar_proxy_target("templates", "v1%2Fprivate") @@ -963,3 +965,285 @@ def test_extension_sidecar_proxy_consent_route_is_wired(monkeypatch): "status": 200, "data": {"ok": True, "id": "templates", "approved": True}, } + + +def test_extension_sidecar_proxy_consent_route_fails_closed_auth_off(tmp_path, monkeypatch): + # Frank #6331 blocker 1 (route-level regression): with WebUI auth OFF, driving + # the REAL consent function through the HTTP route for a token-v1 sidecar must + # return 403 (fail closed), not persist consent. Uses the real + # set_extension_sidecar_proxy_consent (NOT a mock) so the route + auth gate are + # exercised end-to-end. + from api import routes + + monkeypatch.delenv("HERMES_WEBUI_PASSWORD", raising=False) + from api.auth import _invalidate_password_hash_cache, is_auth_enabled + _invalidate_password_hash_cache() + assert is_auth_enabled() is False + _token_v1_manifest(monkeypatch, tmp_path, origin="http://127.0.0.1:17787") + + captured = {} + + def fake_bad(handler, msg, status=400): + captured["status"] = status + captured["msg"] = msg + return True + + def fake_j(handler, data, status=200, headers=None): + captured["status"] = status + captured["data"] = data + return True + + monkeypatch.setattr(routes, "_check_csrf", lambda handler: True) + monkeypatch.setattr(routes, "read_body", lambda handler: {"id": "templates", "approved": True}) + monkeypatch.setattr(routes, "j", fake_j) + monkeypatch.setattr(routes, "bad", fake_bad) + handler = FakeHandler() + + assert routes.handle_post( + handler, + SimpleNamespace(path="/api/extensions/sidecar-proxy-consent"), + ) is True + # Must be a fail-closed 403 from the real consent function, not a 200 grant. + assert captured.get("status") == 403 + assert "data" not in captured + # And no consent must have been persisted. + from api.extensions import resolve_extension_sidecar_proxy_target, ExtensionSidecarProxyError + with pytest.raises(ExtensionSidecarProxyError) as exc: + resolve_extension_sidecar_proxy_target("templates", "v1/ping") + assert exc.value.status == 403 + + +# ── token-v1 proxy auth (§9.1/§9.2) ───────────────────────────────────────── + +def _token_v1_manifest(monkeypatch, tmp_path, origin="http://127.0.0.1:17787"): + return _configure_manifest_extension( + monkeypatch, + tmp_path, + { + "extensions": [ + { + "id": "templates", + "sidecar": { + "type": "loopback", + "origin": origin, + "proxy_auth": "token-v1", + }, + } + ] + }, + ) + + +def test_token_v1_injects_persisted_token_when_auth_enabled(tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_WEBUI_PASSWORD", "pw") + from api.auth import _invalidate_password_hash_cache + _invalidate_password_hash_cache() + _token_v1_manifest(monkeypatch, tmp_path) + from api.extensions import ( + resolve_extension_sidecar_proxy_target, + set_extension_sidecar_proxy_consent, + ) + import api.extension_sidecar_auth as sc + + set_extension_sidecar_proxy_consent("templates", True) + target = resolve_extension_sidecar_proxy_target("templates", "v1/ping") + assert target["proxy_auth"] == "token-v1" + assert target["auth_token"] and len(target["auth_token"]) > 30 + assert target["auth_token"] == sc.current_token("templates") + + +def test_token_v1_auth_off_blocks_consent_and_resolution_fail_closed(tmp_path, monkeypatch): + # Frank #6331 blocker 1: with WebUI auth OFF, token-v1 must fail closed at BOTH + # the consent grant and the resolution path — regardless of a loopback origin. + # A loopback sidecar is NOT a sufficient guard: any caller that can reach the + # (unauthenticated) WebUI listener could otherwise self-grant consent and drive + # the token-bearing proxy (a forwarding oracle); another local UID can reach the + # listener without ever reading the 0600 token file. + monkeypatch.delenv("HERMES_WEBUI_PASSWORD", raising=False) + from api.auth import _invalidate_password_hash_cache, is_auth_enabled + _invalidate_password_hash_cache() + assert is_auth_enabled() is False + _token_v1_manifest(monkeypatch, tmp_path, origin="http://127.0.0.1:17787") + from api.extensions import ( + ExtensionSidecarProxyError, + set_extension_sidecar_proxy_consent, + ) + + # Consent grant is rejected fail-closed (403) even for a loopback origin. + with pytest.raises(ExtensionSidecarProxyError) as consent_exc: + set_extension_sidecar_proxy_consent("templates", True) + assert consent_exc.value.status == 403 + + +def test_token_v1_resolution_fails_closed_when_auth_disabled_after_consent(tmp_path, monkeypatch): + # Defense in depth: even if a consent record somehow exists (e.g. granted while + # auth was enabled, then auth turned off), the resolution path must ALSO fail + # closed so a stale consent can't be exercised without WebUI auth. + monkeypatch.setenv("HERMES_WEBUI_PASSWORD", "pw") + from api.auth import _invalidate_password_hash_cache, is_auth_enabled + _invalidate_password_hash_cache() + _token_v1_manifest(monkeypatch, tmp_path, origin="http://127.0.0.1:17787") + from api.extensions import ( + ExtensionSidecarProxyError, + resolve_extension_sidecar_proxy_target, + set_extension_sidecar_proxy_consent, + ) + + # Grant consent WHILE auth is enabled (allowed). + assert is_auth_enabled() is True + set_extension_sidecar_proxy_consent("templates", True) + # Now disable auth and confirm resolution refuses the pre-existing consent. + monkeypatch.delenv("HERMES_WEBUI_PASSWORD", raising=False) + _invalidate_password_hash_cache() + assert is_auth_enabled() is False + with pytest.raises(ExtensionSidecarProxyError) as exc: + resolve_extension_sidecar_proxy_target("templates", "v1/ping") + assert exc.value.status == 403 + + +def test_unknown_proxy_auth_value_rejects_sidecar(tmp_path, monkeypatch): + _configure_manifest_extension( + monkeypatch, + tmp_path, + { + "extensions": [ + { + "id": "templates", + "sidecar": { + "type": "loopback", + "origin": "http://127.0.0.1:17787", + "proxy_auth": "totally-bogus", + }, + } + ] + }, + ) + from api.extensions import ( + ExtensionSidecarProxyError, + set_extension_sidecar_proxy_consent, + ) + + # Unknown proxy_auth -> _sidecar_from_manifest_entry rejects the record, so the + # sidecar never becomes available/consentable (fail closed, not silent-open). + # The rejection surfaces at consent time (the earliest resolve path). + with pytest.raises(ExtensionSidecarProxyError): + set_extension_sidecar_proxy_consent("templates", True) + + +def test_status_payload_flags_local_unprotected_when_auth_off(tmp_path, monkeypatch): + _token_v1_manifest(monkeypatch, tmp_path) + from api.extensions import get_extension_status + + status = get_extension_status() + sidecars = status.get("sidecars") or [] + tmpl = next((s for s in sidecars if s.get("id") == "templates"), None) + assert tmpl is not None + proxy = tmpl["proxy"] + assert proxy["proxy_auth"] == "token-v1" + assert proxy["posture"] == "local_unprotected" # auth off -> panel warns pre-consent + + +def test_inbound_x_hermes_header_is_stripped(monkeypatch): + from api.routes import _extension_sidecar_proxy_request_headers + + class H: + headers = { + "X-Hermes-Sidecar-Token": "forged", + "X-Custom": "ok", + "Cookie": "secret", + } + + out = _extension_sidecar_proxy_request_headers(H()) + assert "X-Custom" in out + assert not any(k.lower().startswith("x-hermes-") for k in out) + assert not any(k.lower() == "cookie" for k in out) + + +def test_token_module_persists_and_rotates(tmp_path, monkeypatch): + import api.extension_sidecar_auth as sc + + # Patch the dynamic dir resolver (mirrors how _extension_state_dir is patched + # elsewhere) so the token lands in an isolated tmp dir, not the shared session + # state dir. + token_dir = tmp_path / "sidecar-auth" + monkeypatch.setattr(sc, "_token_dir", lambda: token_dir) + sc._CACHE.clear() + tok = sc.ensure_token("templates") + assert tok and sc.current_token("templates") == tok + assert (token_dir / "templates.token").exists() + assert sc.current_token("never") is None # fail closed + assert sc.ensure_token("../evil") is None # path-escape rejected + # IDs are filename components. Reject rather than silently trim whitespace so + # validation and _token_path() always operate on the exact same string. + for invalid_id in ("templates\n", " templates", "templates "): + assert sc.ensure_token(invalid_id) is None + assert not (token_dir / f"{invalid_id}.token").exists() + # canonical (wider) id grammar is honored: uppercase/dot ids work + assert sc.ensure_token("RSS.Feeds") is not None + new = sc.reset_token("templates") + assert new and new != tok and sc.current_token("templates") == new + sc._CACHE.clear() + + +def test_token_v1_route_injects_token_and_strips_response(monkeypatch): + # The 5 most load-bearing lines: the injected token must reach the upstream + # Request, and any x-hermes-* on the response must be stripped before it + # reaches the browser. (Fable coreA item 3.) + from api import routes + + captured = {} + + class FakeResponse: + def __init__(self): + self.status = 200 + self.headers = { + "Content-Type": "application/json", + "X-Hermes-Echo": "leak-me", # must be stripped from client response + } + + def read(self, *_a): + return b"{}" + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + class FakeOpener: + def open(self, request, timeout=10): + captured["headers"] = {k.lower(): v for k, v in request.header_items()} + return FakeResponse() + + monkeypatch.setattr(routes, "_check_csrf", lambda handler: True) + monkeypatch.setattr( + "api.extensions.resolve_extension_sidecar_proxy_target", + lambda extension_id, proxy_path, query="": { + "extension_id": extension_id, + "origin": "http://127.0.0.1:17787", + "proxy_path": "/api/extensions/templates/sidecar/", + "upstream_url": "http://127.0.0.1:17787/v1/ping", + "proxy_auth": "token-v1", + "auth_token": "injected-token-abc", + }, + ) + monkeypatch.setattr( + routes, "_extension_sidecar_proxy_same_origin_opener", lambda o: FakeOpener() + ) + + handler = FakeHandler() + handler.headers = { + "Accept": "application/json", + "Host": "webui.local", + "Origin": "http://webui.local", + "Referer": "http://webui.local/", + } + result = routes.handle_get( + handler, + SimpleNamespace(path="/api/extensions/templates/sidecar/v1/ping", query=""), + ) + assert result is True + # token injected on the way to the sidecar + assert captured["headers"].get("x-hermes-sidecar-token") == "injected-token-abc" + # token/x-hermes header stripped on the way back to the browser + assert handler.header("X-Hermes-Echo") is None