diff --git a/backend/routers/agent_shell.py b/backend/routers/agent_shell.py index 64aa5ab..058d675 100644 --- a/backend/routers/agent_shell.py +++ b/backend/routers/agent_shell.py @@ -15,16 +15,12 @@ import struct import sys import termios from typing import Any +from urllib.parse import urlsplit from fastapi import APIRouter, Depends, HTTPException, Query, WebSocket, WebSocketDisconnect from pydantic import BaseModel, Field -from auth import ( - _current_admin_key, - _debug_mode_enabled, - _is_trusted_local_runtime_host, - require_local_operator, -) +from auth import _current_admin_key, require_local_operator from services.agent_shell_settings import ( get_agent_shell_settings, set_agent_shell_working_directory, @@ -44,21 +40,76 @@ def _set_winsize(fd: int, rows: int, cols: int) -> None: fcntl.ioctl(fd, termios.TIOCSWINSZ, winsize) +def _configured_agent_shell_origins() -> set[str]: + """Return explicitly configured browser origins accepted by the shell.""" + allowed: set[str] = set() + for env_name in ("SHADOWBROKER_AGENT_SHELL_ALLOWED_ORIGINS", "CORS_ORIGINS"): + for raw in os.environ.get(env_name, "").split(","): + candidate = raw.strip() + if not candidate: + continue + try: + parsed = urlsplit(candidate) + except ValueError: + continue + if parsed.scheme in {"http", "https"} and parsed.hostname: + allowed.add(f"{parsed.scheme.lower()}://{parsed.netloc.lower()}") + return allowed + + +def _agent_shell_origin_allowed(ws: WebSocket) -> bool: + """Reject drive-by browser origins while preserving local/LAN dashboard use.""" + origin = str(ws.headers.get("origin", "") or "").strip() + if not origin: + # Non-browser clients do not normally send Origin; they still need an + # explicit one-time token or X-Admin-Key below. + return True + try: + parsed_origin = urlsplit(origin) + except ValueError: + return False + if parsed_origin.scheme not in {"http", "https"} or not parsed_origin.hostname: + return False + + normalized_origin = f"{parsed_origin.scheme.lower()}://{parsed_origin.netloc.lower()}" + if normalized_origin in _configured_agent_shell_origins(): + return True + + # The browser UI connects directly to the backend port on the same host + # (for example localhost:3000 -> localhost:8000). Match the hostname, not + # the port, so normal source, Docker-host, and LAN deployments stay silent. + host_header = str(ws.headers.get("host", "") or "").strip() + if not host_header: + return False + try: + target_host = urlsplit(f"//{host_header}").hostname + except ValueError: + return False + return bool(target_host and parsed_origin.hostname.lower() == target_host.lower()) + + async def _authorize_agent_shell_ws( ws: WebSocket, - admin_key_query: str = "", ws_token_query: str = "", ) -> None: - host = (ws.client.host or "").lower() if ws.client else "" - if _is_trusted_local_runtime_host(host) or (_debug_mode_enabled() and host == "test"): - return + """Require an explicit capability; source IP is never authorization.""" + if not _agent_shell_origin_allowed(ws): + await ws.close(code=4403, reason="agent shell origin rejected") + raise WebSocketDisconnect() + if consume_agent_shell_ws_token(ws_token_query): return - admin_key = _current_admin_key() - presented = str(admin_key_query or ws.headers.get("x-admin-key", "") or "").strip() - if admin_key and presented and hmac.compare_digest(presented.encode(), admin_key.encode()): - return - await ws.close(code=4403, reason="local operator access only") + + # Browser WebSocket clients cannot set arbitrary authentication headers. + # Keep header auth only for non-browser/native tooling, and never accept a + # long-lived ADMIN_KEY in the URL query string. + if not ws.headers.get("origin"): + admin_key = _current_admin_key() + presented = str(ws.headers.get("x-admin-key", "") or "").strip() + if admin_key and presented and hmac.compare_digest(presented.encode(), admin_key.encode()): + return + + await ws.close(code=4403, reason="agent shell credential required") raise WebSocketDisconnect() @@ -149,12 +200,11 @@ async def agent_shell_websocket( cwd: str = Query(default=""), cols: int = Query(default=80), rows: int = Query(default=24), - admin_key: str = Query(default=""), ws_token: str = Query(default=""), ) -> None: await ws.accept() try: - await _authorize_agent_shell_ws(ws, admin_key, ws_token) + await _authorize_agent_shell_ws(ws, ws_token) except WebSocketDisconnect: return diff --git a/backend/services/agent_shell_ws_token.py b/backend/services/agent_shell_ws_token.py index 9653b98..72ec452 100644 --- a/backend/services/agent_shell_ws_token.py +++ b/backend/services/agent_shell_ws_token.py @@ -13,25 +13,31 @@ _store: dict[str, float] = {} _lock = Lock() -def _purge_expired(*, force: bool = False) -> None: +def _purge_expired_locked(now: float) -> None: + """Drop expired tokens while the caller holds ``_lock``.""" + expired = [token for token, expires in _store.items() if expires <= now] + for token in expired: + _store.pop(token, None) + + +def _purge_expired() -> None: now = time.time() with _lock: - expired = [token for token, expires in _store.items() if expires <= now] - for token in expired: - _store.pop(token, None) - if force and len(_store) > _MAX_ACTIVE_TOKENS: - for token in list(_store.keys())[: len(_store) - _MAX_ACTIVE_TOKENS]: - _store.pop(token, None) + _purge_expired_locked(now) def mint_agent_shell_ws_token() -> tuple[str, int]: - """Return (token, expires_in_seconds).""" - _purge_expired() + """Return (token, expires_in_seconds) without exceeding the store bound.""" token = secrets.token_urlsafe(32) - expires_at = time.time() + _TOKEN_TTL_SECONDS + now = time.time() + expires_at = now + _TOKEN_TTL_SECONDS with _lock: + _purge_expired_locked(now) if len(_store) >= _MAX_ACTIVE_TOKENS: - _purge_expired(force=True) + # Evict the token that will expire soonest. This keeps the store + # bounded without recursively acquiring the non-reentrant lock. + oldest = min(_store, key=_store.get) + _store.pop(oldest, None) _store[token] = expires_at return token, int(_TOKEN_TTL_SECONDS) @@ -43,6 +49,7 @@ def consume_agent_shell_ws_token(token: str) -> bool: return False now = time.time() with _lock: + _purge_expired_locked(now) expires_at = _store.pop(cleaned, None) return expires_at is not None and expires_at > now diff --git a/backend/services/updater.py b/backend/services/updater.py index 33c65d8..c9c6d17 100644 --- a/backend/services/updater.py +++ b/backend/services/updater.py @@ -31,16 +31,15 @@ DOCKER_UPDATE_COMMANDS = ( "docker compose pull && docker compose up -d" ) -# Issue #231: baked-in release digests. Loaded lazily, used as a fallback -# verification source when the release's SHA256SUMS.txt asset can't be -# fetched (e.g. transient network failure during update). +# Issue #231: baked-in release digests. Loaded lazily and treated as an +# independent trust source because they ship with the already-installed code. _RELEASE_DIGESTS_FILE = ( Path(__file__).resolve().parent.parent / "data" / "release_digests.json" ) -# Pattern for the maintainer's signed source-archive release asset. This -# is the file we prefer over the auto-generated ``zipball_url`` because -# the maintainer's build process publishes it with a matching entry in -# SHA256SUMS.txt — the zipball does not have a signed digest. +# Pattern for the maintainer's source-archive release asset. The matching +# SHA256SUMS.txt is still useful as a consistency check, but because both are +# delivered by the same release channel it is not sufficient by itself to +# authorize executable source replacement. _SOURCE_ASSET_PATTERN = re.compile(r"^ShadowBroker_v\d", re.IGNORECASE) _SHA256SUMS_ASSET_NAME = "SHA256SUMS.txt" @@ -134,15 +133,13 @@ def _validate_update_url(url: str, *, allow_release_page: bool = False) -> str: def _download_release(temp_dir: str) -> tuple: """Fetch latest release info and download the source zip archive. - Issue #231: prefer the maintainer's signed release asset (matching - ``ShadowBroker_v*.zip``) over the auto-generated ``zipball_url``, - because the maintainer's release process publishes a matching entry - in SHA256SUMS.txt for the named asset but NOT for the zipball. + Prefer the maintainer's named release asset (matching + ``ShadowBroker_v*.zip``) over the auto-generated ``zipball_url`` so a + stable filename can be matched against a pre-pinned digest. Returns (zip_path, version_tag, download_url, release_url, asset_name, - sha256sums_url) — the last two are empty strings when the release - doesn't publish a signed asset, falling back to the legacy zipball - path. + sha256sums_url) — the last two are empty strings when the release does not + publish a named asset. """ logger.info("Fetching latest release info from GitHub...") _validate_update_url(GITHUB_RELEASES_URL) @@ -155,8 +152,6 @@ def _download_release(temp_dir: str) -> tuple: release_url = str(release.get("html_url") or GITHUB_RELEASES_PAGE_URL).strip() _validate_update_url(release_url, allow_release_page=True) - # Prefer the maintainer-signed release asset. Fall back to the - # auto-generated zipball if the release doesn't publish one. assets = release.get("assets") or [] asset_name = "" asset_url = "" @@ -175,7 +170,7 @@ def _download_release(temp_dir: str) -> tuple: if asset_url: zip_url = asset_url logger.info( - "Using signed release asset %s (sha256sums=%s)", + "Using release asset %s (sha256sums=%s)", asset_name, "yes" if sha256sums_url else "no", ) @@ -184,10 +179,10 @@ def _download_release(temp_dir: str) -> tuple: if not zip_url: raise RuntimeError("Latest release is missing a source archive URL") logger.warning( - "Release does not publish a signed ShadowBroker_v*.zip asset — " - "falling back to auto-generated zipball_url. Integrity will be " - "verified against the baked-in release_digests.json (if present) " - "or HTTPS-only otherwise." + "Release does not publish a ShadowBroker_v*.zip asset — falling " + "back to auto-generated zipball_url. In-place installation will " + "still require an explicit MESH_UPDATE_SHA256 pin or a matching " + "baked-in digest." ) _validate_update_url(zip_url) @@ -219,17 +214,7 @@ def _compute_sha256(zip_path: str) -> str: def _load_baked_in_release_digests() -> dict: - """Return the ``release_digests.json`` mapping, or an empty dict. - - Schema (issue #231): - { - "": { - "": "", - ... - }, - ... - } - """ + """Return the ``release_digests.json`` mapping, or an empty dict.""" try: raw = _RELEASE_DIGESTS_FILE.read_text(encoding="utf-8") parsed = json.loads(raw) @@ -254,12 +239,7 @@ def _load_baked_in_release_digests() -> dict: def _fetch_sha256sums(sha256sums_url: str) -> dict[str, str]: - """Download a SHA256SUMS.txt and return {filename: digest_hex_lower}. - - Standard ``sha256sum`` format: `` `` per line. The - leading ``*`` binary-mode marker (e.g. `` *``) is - handled. - """ + """Download a SHA256SUMS.txt and return {filename: digest_hex_lower}.""" try: _validate_update_url(sha256sums_url) except RuntimeError as exc: @@ -268,15 +248,18 @@ def _fetch_sha256sums(sha256sums_url: str) -> dict[str, str]: try: resp = requests.get(sha256sums_url, timeout=15) resp.raise_for_status() + _validate_update_url(resp.url) except requests.RequestException as exc: logger.info("SHA256SUMS fetch failed: %s", exc) return {} + except RuntimeError as exc: + logger.warning("SHA256SUMS redirect rejected: %s", exc) + return {} out: dict[str, str] = {} for line in resp.text.splitlines(): line = line.strip() if not line or line.startswith("#"): continue - # Tolerant split: handle both ` ` and ` *`. parts = line.split(None, 1) if len(parts) != 2: continue @@ -295,34 +278,26 @@ def _validate_zip_hash( sha256sums_url: str = "", release_tag: str = "", ) -> str: - """Verify the downloaded archive against trusted digest sources. + """Verify an executable source update against an independent trust source. - Issue #231: previously this returned silently when ``MESH_UPDATE_SHA256`` - was unset, which made the auto-updater a supply-chain RCE vector on any - compromise of the GitHub release pipeline. The chain now is: + Authorization order: - 1. ``MESH_UPDATE_SHA256`` env var (operator override — preserved for - power-users who want to pin an exact digest manually) - 2. ``SHA256SUMS.txt`` release asset (primary — the maintainer's - release process already publishes this) - 3. Baked-in ``backend/data/release_digests.json`` (second line of - defense for releases that lack the SHA256SUMS asset, or when the - asset can't be fetched at update time) - 4. HTTPS-only fallback with a loud warning (preserves the auto-update - flow during transient outages — but never silently) + 1. ``MESH_UPDATE_SHA256`` — explicit operator pin. + 2. ``backend/data/release_digests.json`` — a digest already shipped in + the installed application. - A mismatch from a source that DID respond is fatal: the update is - refused and the existing install keeps running. Only the "no source - reachable at all" case falls back to HTTPS-only. - - Returns a short human-readable description of which source verified - the archive (used in the update-success message). + A release ``SHA256SUMS.txt`` is fetched only as a consistency diagnostic. + Because it is controlled by the same GitHub release channel as the ZIP, a + matching value cannot independently authenticate a compromised release. + If neither independent source is available, the updater fails closed and + leaves the existing installation untouched. """ actual = _compute_sha256(zip_path) - # Source 1: explicit operator override. override = os.environ.get("MESH_UPDATE_SHA256", "").strip().lower() if override: + if len(override) != 64 or any(c not in "0123456789abcdef" for c in override): + raise RuntimeError("MESH_UPDATE_SHA256 must be a 64-character hexadecimal SHA-256 digest") if actual == override: return f"verified via MESH_UPDATE_SHA256 ({actual[:16]}...)" raise RuntimeError( @@ -330,21 +305,6 @@ def _validate_zip_hash( f"expected={override[:16]}..." ) - # Source 2: SHA256SUMS.txt asset from the release. - sums_map: dict[str, str] = {} - if sha256sums_url and asset_name: - sums_map = _fetch_sha256sums(sha256sums_url) - - sums_expected = sums_map.get(asset_name) if asset_name else None - if sums_expected: - if actual == sums_expected: - return f"verified via release SHA256SUMS.txt ({actual[:16]}...)" - raise RuntimeError( - f"Update SHA-256 mismatch vs release SHA256SUMS.txt: " - f"archive={actual[:16]}..., expected={sums_expected[:16]}..." - ) - - # Source 3: baked-in digest list. baked = _load_baked_in_release_digests() baked_expected = "" if release_tag and asset_name: @@ -357,23 +317,23 @@ def _validate_zip_hash( f"archive={actual[:16]}..., expected={baked_expected[:16]}..." ) - # Source 4: HTTPS-only fallback. We keep onboarding/auto-update working - # during transient outages (no SHA256SUMS reachable AND no baked-in - # entry for this release), but surface the degraded posture loudly so - # the operator can see it in logs and the maintainer can populate the - # digest list on the next release bump. - logger.warning( - "Update integrity check fell back to HTTPS-only trust " - "(no SHA256SUMS.txt response and no baked-in digest for " - "release=%s asset=%s). The archive SHA-256 is %s. Once the " - "release ships a SHA256SUMS.txt asset OR backend/data/" - "release_digests.json is updated with this release, the secure " - "path will activate automatically.", - release_tag or "unknown", - asset_name or "unknown", - actual, + release_checksum_note = "" + if sha256sums_url and asset_name: + sums_expected = _fetch_sha256sums(sha256sums_url).get(asset_name) + if sums_expected: + if actual != sums_expected: + raise RuntimeError( + f"Update SHA-256 mismatch vs release SHA256SUMS.txt: " + f"archive={actual[:16]}..., expected={sums_expected[:16]}..." + ) + release_checksum_note = " The same-release SHA256SUMS.txt matched, but is not an independent trust root." + + raise RuntimeError( + "Update refused: no independent archive digest is trusted for " + f"release={release_tag or 'unknown'} asset={asset_name or 'unknown'}." + f"{release_checksum_note} Pin MESH_UPDATE_SHA256 or ship a matching " + "backend/data/release_digests.json entry before enabling in-place installation." ) - return f"https-only (no digest source reachable, archive={actual[:16]}...)" def _is_source_checkout(project_root: str) -> bool: @@ -399,7 +359,6 @@ def _backup_current(project_root: str, temp_dir: str) -> str: if not os.path.isdir(dir_path): continue for root, dirs, files in os.walk(dir_path): - # Prune protected directories from walk dirs[:] = [d for d in dirs if d not in _PROTECTED_DIRS] for fname in files: full = os.path.join(root, fname) @@ -438,8 +397,6 @@ def _extract_and_copy(zip_path: str, project_root: str, temp_dir: str) -> int: raise RuntimeError(f"Updater refused archive path traversal entry: {member.filename}") zf.extractall(extract_dir) - # Detect wrapper folder: if extracted root has a single directory that - # itself contains frontend/ or backend/, use it as the real base. base = extract_dir entries = [e for e in os.listdir(base) if not e.startswith(".")] if len(entries) == 1: @@ -454,7 +411,6 @@ def _extract_and_copy(zip_path: str, project_root: str, temp_dir: str) -> int: skipped = 0 for root, dirs, files in os.walk(base): - # Prune protected directories so os.walk never descends into them dirs[:] = [d for d in dirs if d not in _PROTECTED_DIRS] for fname in files: @@ -466,7 +422,6 @@ def _extract_and_copy(zip_path: str, project_root: str, temp_dir: str) -> int: continue dst = os.path.abspath(os.path.join(project_root, rel)) - # Safety: never write outside the project root (zip path traversal) if not dst.startswith(os.path.abspath(project_root)): logger.warning(f"Safety skip (path traversal): {rel}") skipped += 1 diff --git a/backend/tests/test_security_hardening_2026_08.py b/backend/tests/test_security_hardening_2026_08.py new file mode 100644 index 0000000..6098521 --- /dev/null +++ b/backend/tests/test_security_hardening_2026_08.py @@ -0,0 +1,130 @@ +from __future__ import annotations + +import asyncio +import hashlib +import inspect +from types import SimpleNamespace + +import pytest +from fastapi import WebSocketDisconnect + +from routers import agent_shell +from services import agent_shell_ws_token as ws_tokens +from services import updater + + +class _FakeWebSocket: + def __init__(self, headers: dict[str, str] | None = None, host: str = "127.0.0.1") -> None: + self.headers = headers or {} + self.client = SimpleNamespace(host=host) + self.closed: tuple[int, str] | None = None + + async def close(self, code: int = 1000, reason: str = "") -> None: + self.closed = (code, reason) + + +def setup_function() -> None: + ws_tokens.reset_agent_shell_ws_tokens_for_tests() + + +def teardown_function() -> None: + ws_tokens.reset_agent_shell_ws_tokens_for_tests() + + +def test_agent_shell_query_no_longer_accepts_admin_key() -> None: + parameters = inspect.signature(agent_shell.agent_shell_websocket).parameters + assert "admin_key" not in parameters + assert "ws_token" in parameters + + +def test_loopback_source_ip_is_not_agent_shell_authorization() -> None: + ws = _FakeWebSocket({"host": "127.0.0.1:8000"}) + with pytest.raises(WebSocketDisconnect): + asyncio.run(agent_shell._authorize_agent_shell_ws(ws, "")) + assert ws.closed is not None + assert ws.closed[0] == 4403 + + +def test_browser_one_time_token_allows_same_host_origin() -> None: + token, _ = ws_tokens.mint_agent_shell_ws_token() + ws = _FakeWebSocket( + { + "host": "127.0.0.1:8000", + "origin": "http://127.0.0.1:3000", + } + ) + asyncio.run(agent_shell._authorize_agent_shell_ws(ws, token)) + + # The capability is single-use. + second = _FakeWebSocket( + { + "host": "127.0.0.1:8000", + "origin": "http://127.0.0.1:3000", + } + ) + with pytest.raises(WebSocketDisconnect): + asyncio.run(agent_shell._authorize_agent_shell_ws(second, token)) + + +def test_cross_origin_browser_is_rejected_before_token_use() -> None: + token, _ = ws_tokens.mint_agent_shell_ws_token() + ws = _FakeWebSocket( + { + "host": "127.0.0.1:8000", + "origin": "https://attacker.example", + } + ) + with pytest.raises(WebSocketDisconnect): + asyncio.run(agent_shell._authorize_agent_shell_ws(ws, token)) + assert ws.closed is not None + assert ws.closed[0] == 4403 + # Rejection happens before capability consumption. + assert ws_tokens.consume_agent_shell_ws_token(token) is True + + +def test_agent_shell_token_store_stays_bounded_without_recursive_locking() -> None: + minted = [ws_tokens.mint_agent_shell_ws_token()[0] for _ in range(400)] + assert len(ws_tokens._store) <= ws_tokens._MAX_ACTIVE_TOKENS + assert ws_tokens.consume_agent_shell_ws_token(minted[-1]) is True + + +def test_updater_refuses_same_release_checksum_as_sole_trust_root(tmp_path, monkeypatch) -> None: + archive = tmp_path / "ShadowBroker_v9.9.9.zip" + archive.write_bytes(b"archive-under-test") + digest = hashlib.sha256(archive.read_bytes()).hexdigest() + + monkeypatch.delenv("MESH_UPDATE_SHA256", raising=False) + monkeypatch.setattr(updater, "_load_baked_in_release_digests", lambda: {}) + monkeypatch.setattr( + updater, + "_fetch_sha256sums", + lambda _url: {archive.name: digest}, + ) + + with pytest.raises(RuntimeError, match="no independent archive digest"): + updater._validate_zip_hash( + str(archive), + asset_name=archive.name, + sha256sums_url="https://github.com/BigBodyCobain/Shadowbroker/releases/download/v9.9.9/SHA256SUMS.txt", + release_tag="v9.9.9", + ) + + +def test_updater_accepts_preinstalled_baked_digest(tmp_path, monkeypatch) -> None: + archive = tmp_path / "ShadowBroker_v9.9.9.zip" + archive.write_bytes(b"trusted-archive") + digest = hashlib.sha256(archive.read_bytes()).hexdigest() + + monkeypatch.delenv("MESH_UPDATE_SHA256", raising=False) + monkeypatch.setattr( + updater, + "_load_baked_in_release_digests", + lambda: {"v9.9.9": {archive.name: digest}}, + ) + + note = updater._validate_zip_hash( + str(archive), + asset_name=archive.name, + release_tag="v9.9.9", + ) + assert "baked-in digest" in note diff --git a/frontend/src/__tests__/csp/cspNoncePlumbing.test.ts b/frontend/src/__tests__/csp/cspNoncePlumbing.test.ts index f7a4695..fe7b2c9 100644 --- a/frontend/src/__tests__/csp/cspNoncePlumbing.test.ts +++ b/frontend/src/__tests__/csp/cspNoncePlumbing.test.ts @@ -5,7 +5,7 @@ * 1. Document CSP remains hydration-safe for the Next.js runtime * 2. CSP is deterministic across repeated requests * 3. next.config.ts no longer owns a static CSP header - * 4. Proxy does not break API/static routes (matcher exclusion) + * 4. Proxy screens API routes before handlers while static assets stay excluded * 5. Google Fonts domains are preserved in CSP * 6. Production CSP preserves required directives */ @@ -117,12 +117,54 @@ describe('next.config.ts CSP removal', () => { }); // --------------------------------------------------------------------------- -// 4. Proxy does not break API/static routes +// 4. Proxy screens APIs while keeping document/static behavior intact // --------------------------------------------------------------------------- -describe('proxy matcher exclusions', () => { - it('excludes /api paths', () => { - expect(matcherExcludes('/api/mesh/events')).toBe(true); +describe('proxy matcher and privileged API boundary', () => { + it('includes /api paths so the request-boundary security guard runs', () => { + expect(matcherExcludes('/api/mesh/events')).toBe(false); + }); + + it('non-sensitive API requests pass through without document CSP', () => { + expect(getCsp('/api/mesh/events')).toBe(''); + }); + + it('rejects hostile cross-origin privileged API requests before route handling', () => { + const req = new NextRequest('http://localhost/api/settings/tor/reset-identity', { + method: 'POST', + headers: { + host: 'localhost', + origin: 'https://evil.example', + 'sec-fetch-site': 'cross-site', + }, + }); + const response = proxy(req); + expect(response.status).toBe(403); + expect(response.headers.get('Content-Security-Policy')).toBeNull(); + }); + + it('preserves a legitimate reverse-proxy Forwarded host on an internal direct Host', () => { + const req = new NextRequest('http://frontend:3000/api/settings/api-keys', { + method: 'GET', + headers: { + host: 'frontend:3000', + origin: 'https://shadowbroker.example', + forwarded: 'for=172.18.0.1;proto=https;host="shadowbroker.example"', + }, + }); + expect(proxy(req).status).not.toBe(403); + }); + + it('does not trust spoofed X-Forwarded-Host on a public direct Host', () => { + const req = new NextRequest('https://shadowbroker.example/api/settings/api-keys', { + method: 'GET', + headers: { + host: 'shadowbroker.example', + origin: 'https://evil.example', + 'x-forwarded-host': 'evil.example', + }, + }); + expect(proxy(req).status).toBe(403); }); it('excludes /_next/static paths', () => { diff --git a/frontend/src/__tests__/csp/cspProductionHardening.test.ts b/frontend/src/__tests__/csp/cspProductionHardening.test.ts index dab662e..f0eb3e9 100644 --- a/frontend/src/__tests__/csp/cspProductionHardening.test.ts +++ b/frontend/src/__tests__/csp/cspProductionHardening.test.ts @@ -6,7 +6,7 @@ * inline fallback required by the Next.js production runtime * 2. Dev CSP retains 'unsafe-inline' and 'unsafe-eval' * 3. Unchanged directives (style-src, font-src, worker-src, etc.) intact - * 4. API/static route exclusions remain intact + * 4. API security-boundary matching and static route exclusions remain intact * 5. isDev is evaluated per-request (not cached at module load) */ @@ -177,12 +177,12 @@ describe('unchanged directives in production', () => { }); // --------------------------------------------------------------------------- -// 4. API/static route exclusions remain intact +// 4. API security boundary / static route exclusions remain intact // --------------------------------------------------------------------------- -describe('matcher exclusions unchanged', () => { - it('excludes /api paths', () => { - expect(matcherExcludes('/api/mesh/events')).toBe(true); +describe('matcher security boundary and exclusions', () => { + it('includes /api paths so privileged requests can be screened', () => { + expect(matcherExcludes('/api/mesh/events')).toBe(false); }); it('excludes /_next/static paths', () => { diff --git a/frontend/src/proxy.ts b/frontend/src/proxy.ts index cc6f223..d5327a2 100644 --- a/frontend/src/proxy.ts +++ b/frontend/src/proxy.ts @@ -1,9 +1,9 @@ /** - * Phase 5F-A: CSP nonce plumbing proxy. + * Phase 5F-A: CSP nonce plumbing and privileged-request CSRF boundary. * - * Generates a per-request cryptographic nonce and emits a dynamic - * Content-Security-Policy header for document (page) responses. - * API routes, static assets, and image optimization paths are excluded. + * The API guard runs before route handlers so a browser request rejected as + * cross-origin can never be forwarded through the trusted frontend container + * to a backend local-operator route. */ import { NextRequest, NextResponse } from 'next/server'; @@ -36,7 +36,148 @@ function buildCsp(nonce: string, strictScripts = false): string { return directives.join('; '); } +function isPrivilegedApiPath(pathname: string): boolean { + const path = pathname.replace(/\/+$/, ''); + return ( + path === '/api/refresh' || + path === '/api/debug-latest' || + path === '/api/system/update' || + path === '/api/layers' || + path === '/api/ais/feed' || + path === '/api/mesh/infonet/ingest' || + path === '/api/mesh/meshtastic/send' || + path === '/api/wormhole' || + path.startsWith('/api/wormhole/') || + path === '/api/settings' || + path.startsWith('/api/settings/') || + path.startsWith('/api/ai/') || + path === '/api/ai' || + path.startsWith('/api/tools/') || + path === '/api/tools' || + path.startsWith('/api/mesh/peers') || + path.startsWith('/api/agent-shell/') || + path === '/api/agent-shell' || + path === '/api/sar/mode-b' || + path.startsWith('/api/sar/mode-b/') || + path === '/api/sar/aois' || + path.startsWith('/api/sar/aois/') + ); +} + +function normalizeHeaderHost(host: string | null): string { + return (host || '').trim().replace(/^"|"$/g, '').toLowerCase(); +} + +function hostnameFromHeaderHost(host: string): string { + const normalized = normalizeHeaderHost(host); + if (!normalized) return ''; + try { + return new URL(`http://${normalized}`).hostname.toLowerCase(); + } catch { + return normalized.replace(/:\d+$/, '').toLowerCase(); + } +} + +function isPrivateIpv4(hostname: string): boolean { + const parts = hostname.split('.').map((part) => Number(part)); + if (parts.length !== 4 || parts.some((part) => !Number.isInteger(part) || part < 0 || part > 255)) { + return false; + } + const [first, second] = parts; + return first === 10 || (first === 172 && second >= 16 && second <= 31) || (first === 192 && second === 168); +} + +function isInternalProxyHost(host: string): boolean { + const hostname = hostnameFromHeaderHost(host); + if (!hostname || hostname === 'localhost' || hostname === '127.0.0.1' || hostname === '::1') { + return false; + } + return ( + !hostname.includes('.') || + isPrivateIpv4(hostname) || + hostname.endsWith('.internal') || + hostname.endsWith('.docker') + ); +} + +function forwardedHostCandidates(request: NextRequest): string[] { + const hosts = new Set(); + const directHost = normalizeHeaderHost(request.headers.get('host')); + if (directHost) hosts.add(directHost); + + // Only honor forwarding metadata when the direct Host itself looks like an + // internal reverse-proxy/container address. A public/localhost request may + // not promote attacker-supplied forwarded host values into trusted origins. + if (!isInternalProxyHost(directHost)) { + return [...hosts]; + } + + const forwardedHost = request.headers.get('x-forwarded-host'); + if (forwardedHost) { + for (const value of forwardedHost.split(',')) { + const host = normalizeHeaderHost(value); + if (host) hosts.add(host); + } + } + + const forwarded = request.headers.get('forwarded'); + if (forwarded) { + const hostPattern = /(?:^|[;,])\s*host=(?:"([^"]+)"|([^;,]+))/gi; + let match: RegExpExecArray | null; + while ((match = hostPattern.exec(forwarded)) !== null) { + const host = normalizeHeaderHost(match[1] || match[2] || ''); + if (host) hosts.add(host); + } + } + + return [...hosts]; +} + +function isSameOriginOrNonBrowser(request: NextRequest): boolean { + const fetchSite = (request.headers.get('sec-fetch-site') || '').trim().toLowerCase(); + const origin = (request.headers.get('origin') || '').trim(); + + // Modern browsers label ambient cross-site requests even when a particular + // request shape omits Origin (for example navigations/resource loads). + if (fetchSite === 'cross-site' || fetchSite === 'same-site') return false; + + if (!origin) { + // CLI/native/server-to-server callers do not send Sec-Fetch-Site. Normal + // dashboard browser calls are same-origin. Both remain frictionless. + return !fetchSite || fetchSite === 'same-origin'; + } + + let originHost = ''; + try { + originHost = new URL(origin).host.toLowerCase(); + } catch { + return false; + } + if (!originHost) return false; + + return forwardedHostCandidates(request).includes(originHost); +} + export function proxy(request: NextRequest) { + if (isPrivilegedApiPath(request.nextUrl.pathname) && !isSameOriginOrNonBrowser(request)) { + return NextResponse.json( + { detail: 'Cross-origin privileged request denied' }, + { + status: 403, + headers: { + 'Cache-Control': 'no-store, max-age=0', + Pragma: 'no-cache', + }, + }, + ); + } + + // API requests only need the security boundary above. CSP applies to page + // responses, not JSON/API traffic. + if (request.nextUrl.pathname.startsWith('/api/')) { + return NextResponse.next(); + } + const nonce = Buffer.from(crypto.randomUUID()).toString('base64'); // Forward a nonce for staged CSP support. Strict script-src is opt-in until @@ -60,12 +201,10 @@ export function proxy(request: NextRequest) { export const config = { matcher: [ /* - * Match all document/page paths. Exclude: - * - /api/* (API routes — handled by route handlers) - * - /_next/static/* (static assets) - * - /_next/image/* (image optimization) - * - /favicon.ico (browser icon) + * Match pages AND API routes so privileged browser traffic is screened + * before the catch-all API proxy can forward it. Exclude only static/image + * assets and the favicon. */ - '/((?!api|_next/static|_next/image|favicon.ico).*)', + '/((?!_next/static|_next/image|favicon.ico).*)', ], };