diff --git a/.env.example b/.env.example index d399bd6..d069da8 100644 --- a/.env.example +++ b/.env.example @@ -76,3 +76,21 @@ # python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())" # Must stay constant — changing it invalidates all stored sessions. NAVI_AUTH_ENCRYPTION_KEY= + +# ── CORS ──────────────────────────────────────────────────────────────────────── +# REQUIRED when NAVI_AUTH_ENABLED=true (server refuses to start without it). +# Comma-separated list of origins allowed to call the API with credentials. +# Ignored when NAVI_AUTH_ENABLED=false (then any origin is allowed, no credentials). +NAVI_ALLOWED_ORIGINS=https://navi.your-domain.com + +# ── Webhooks (gnexus-auth logout) ───────────────────────────────────────────────── +# HMAC-SHA256 secret shared with gnexus-auth. Copy from its admin panel. +# Empty + NAVI_AUTH_ENABLED=true → webhook endpoint answers 503. +# Empty + NAVI_AUTH_ENABLED=false → accepted unsigned, with a warning in the log. +GNAUTH_WEBHOOK_SECRET= + +# ── Cookie security ─────────────────────────────────────────────────────────────── +# The session cookie Secure flag is set automatically when GNAUTH_BASE_URL is +# https://. Set this to force it on for an https navi deploy behind an http +# auth server (leave empty otherwise). +# NAVI_AUTH_COOKIE_SECURE=true diff --git a/docs/auth.md b/docs/auth.md index 3a27fed..b8dc85e 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -708,3 +708,25 @@ This allows tests to run without a running gnexus-auth instance. For unit tests that don't start the full app, auth is simply not injected. + +--- + +## Known issues + +### Short and unpredictable auth-session lifetime + +The effective session lifetime is **not** the 30-day `navi_auth_session` cookie (`NAVI_AUTH_COOKIE_MAX_AGE_DAYS`); it is governed by the **access/refresh token lifecycle**, which is controlled by gnexus-auth. The cookie outlives the session it points to, so users get logged out unpredictably when a token refresh fails. + +Root causes identified in `navi/auth/deps.py` and `navi/api/routes/auth.py`: + +1. **Session expiry is bound to the access token, not an independent session.** `user_auth_sessions.expires_at` is set to `token_set.expires_at` (access-token lifetime, short) at creation (`auth.py:227`) and on every refresh (`deps.py:179-180`). There is no independent session lifetime and no sliding renewal based on `last_used_at` (the column is written in `_touch_auth_session` but never read). The real session lifetime = how long refresh succeeds, which is gnexus-auth's policy, not Navi's. +2. **`expires_at = token_set.expires_at or now()`.** If gnexus-auth ever returns `expires_in=0` or omits it, the row is marked expired immediately → refresh on every request → fragility. +3. **Transient refresh errors log the user out.** On a network/5xx error during refresh (`deps.py:196-215`), after the 30 s `_user_cache` TTL expires the user resolves to `None` → 401 → login screen. No retry/backoff on the refresh itself. +4. **Refresh-token rotation race in multi-worker deployments.** The per-session refresh lock (`deps.py:40,159`) is in-process only; under `gunicorn -w N` two workers can refresh with the same refresh token simultaneously; if gnexus-auth rotates with reuse detection, the session is invalidated. +5. **Android-only: the cookie is set without `Max-Age`/`Expires` and without `flush()`.** `MainActivity.kt:75-78,105-108` use `CookieManager.setCookie(serverUrl, "navi_auth_session=$sid; Path=/")` — a session cookie that is cleared unpredictably on app restart, unlike the browser flow which sets a 30-day `Max-Age`. +6. ~~**Latent bug:** `_cleanup_refresh_locks` compares `time.time()` against `_user_cache[sid][1]` which is `time.monotonic()` — different clocks, so the cleanup condition was always true and purged all refresh locks on every cache miss.~~ **Fixed** — cleanup now uses `time.monotonic()`. + +Proposed fixes (not yet implemented): +- Separate the Navi session lifetime (independent, with sliding renewal on `last_used_at`) from the access-token lifetime. +- Add retry/backoff on transient refresh failure; do not log out on temporary errors. +- On Android, set the cookie with `Max-Age` (read from the backend session lifetime) and call `CookieManager.flush()`. diff --git a/navi/api/routes/auth.py b/navi/api/routes/auth.py index a9f814a..6e00584 100644 --- a/navi/api/routes/auth.py +++ b/navi/api/routes/auth.py @@ -1,7 +1,10 @@ """Auth endpoints for gnexus-auth OAuth integration.""" import asyncio +import json +import re from datetime import datetime, timezone +from urllib.parse import quote import structlog from typing import Annotated @@ -38,6 +41,16 @@ _mobile_auth_states.pop(k, None) +def _cookie_secure() -> bool: + """Whether the auth cookie should carry the Secure flag. + + Explicit NAVI_AUTH_COOKIE_SECURE wins; otherwise https on the gnexus-auth + side implies a TLS-terminated deployment, so the cookie should be + Secure too (the auth portal and Navi share the same infrastructure). + """ + return settings.navi_auth_cookie_secure or settings.gnauth_base_url.startswith("https://") + + def _get_redirect_uri() -> str: """Return the configured redirect_uri.""" # Always use the configured redirect_uri so reverse proxies are handled @@ -254,7 +267,7 @@ f"Path=/; " f"SameSite={settings.navi_auth_cookie_samesite}" ) - if settings.navi_auth_cookie_secure: + if _cookie_secure(): cookie_str += "; Secure" return Response( @@ -271,7 +284,16 @@ """Bridge page for Android: attempts an automatic deep-link back into the native app via Chrome Intent URL, and falls back to a manual button for browsers that block automatic scheme navigation.""" - intent_url = f"intent://auth/callback?sid={sid}#Intent;scheme=navi;package=com.navi.client;end" + # sid is an auth session id (uuid4().hex) — anything else is either a + # broken link or an injection attempt. Reflected into href and a JS + # string, it must never reach the page unvalidated. + if not re.fullmatch(r"[0-9a-f]{32}", sid or ""): + raise HTTPException(status_code=400, detail="Invalid session id") + + intent_url = ( + f"intent://auth/callback?sid={quote(sid, safe='')}" + f"#Intent;scheme=navi;package=com.navi.client;end" + ) html = ( "" '' @@ -310,7 +332,7 @@ '' '' '' ) - return Response(content=html, media_type="text/html") + # Defense in depth: sid is validated above, but the CSP keeps any injected + # content from loading external resources. Inline style/script are required + # for the bridge page itself. + return Response( + content=html, + media_type="text/html", + headers={ + "Content-Security-Policy": ( + "default-src 'none'; style-src 'unsafe-inline'; script-src 'unsafe-inline'" + ), + }, + ) @router.post("/logout") @@ -346,7 +379,7 @@ f"Path=/; " f"SameSite={settings.navi_auth_cookie_samesite}" ) - if settings.navi_auth_cookie_secure: + if _cookie_secure(): cookie_str += "; Secure" response.headers["Set-Cookie"] = cookie_str diff --git a/navi/api/routes/messages.py b/navi/api/routes/messages.py index c4fa2c9..a3daee2 100644 --- a/navi/api/routes/messages.py +++ b/navi/api/routes/messages.py @@ -5,7 +5,7 @@ from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel -from navi.api.deps import get_agent, get_session_store, require_user +from navi.api.deps import get_agent, get_orchestrator, get_session_store, require_user from navi.auth import User from navi.auth.deps import check_session_access from navi.core import Agent, SessionStore @@ -31,11 +31,22 @@ raise HTTPException(status_code=404, detail="Session not found") check_session_access(session, user) - # Set user context for tool sandboxing + # Guard against a second run on the same session (e.g. an active WS turn): + # mark_busy publishes "running" to is_running()/stop() the same way the + # WS path does. The lock is held only for the check-and-mark, not the run + # itself, so a long REST turn does not stall other requests. + orchestrator = get_orchestrator() + async with orchestrator.session_lock(session_id): + if orchestrator.is_running(session_id): + raise HTTPException(status_code=409, detail="Agent is already running for this session") + orchestrator.mark_busy(session_id) + + # Set user context for tool sandboxing (reset in finally — these + # contextvars otherwise leak to the next request on the same task). from navi.tools._internal.base import current_user_id as _uid_var, current_user_role as _role_var, current_user_info as _uinfo_var - _uid_var.set(user.id) - _role_var.set(user.role) - _uinfo_var.set(user.model_dump(mode="json")) + uid_token = _uid_var.set(user.id) + role_token = _role_var.set(user.role) + uinfo_token = _uinfo_var.set(user.model_dump(mode="json")) try: reply = await agent.run(session_id, body.content) @@ -46,3 +57,8 @@ raise HTTPException(status_code=500, detail=str(e)) except NaviError as e: raise HTTPException(status_code=500, detail=str(e)) + finally: + _uid_var.reset(uid_token) + _role_var.reset(role_token) + _uinfo_var.reset(uinfo_token) + await orchestrator.clear_busy(session_id) diff --git a/navi/api/routes/webhooks.py b/navi/api/routes/webhooks.py index 72cd2a8..93af75a 100644 --- a/navi/api/routes/webhooks.py +++ b/navi/api/routes/webhooks.py @@ -2,7 +2,7 @@ import structlog from fastapi import APIRouter, HTTPException, Request -from gnexus_gauth.exceptions import WebhookPayloadException +from gnexus_gauth.exceptions import WebhookPayloadException, WebhookVerificationException from navi.auth.client import get_gauth_client from navi.config import settings @@ -21,21 +21,38 @@ - session.revoked → invalidate matching session - client.roles_changed / client.permissions_changed → update user role/permissions """ - from navi.config import settings if not settings.gnauth_client_id or not settings.gnauth_client_secret: raise HTTPException(status_code=503, detail="OAuth is not configured") + # Unsigned webhooks let anyone forge session invalidations (global logout, + # user blocked), so in auth mode we refuse to process them at all. + if not settings.gnauth_webhook_secret: + if settings.navi_auth_enabled: + raise HTTPException(status_code=503, detail="Webhook secret is not configured") + log.warning( + "webhook.unverified_mode", + reason="GNAUTH_WEBHOOK_SECRET empty; auth disabled — accepting unsigned webhooks", + ) + raw_body = await request.body() body_text = raw_body.decode("utf-8") client = get_gauth_client() - # For now, log and acknowledge. Full HMAC verification can be added when webhook - # secret is configured in gnexus-auth admin panel. - try: - event = client.parse_webhook(body_text) - except WebhookPayloadException: - raise HTTPException(status_code=400, detail="Invalid JSON payload") + if settings.gnauth_webhook_secret: + try: + event = client.verify_and_parse_webhook( + body_text, dict(request.headers), settings.gnauth_webhook_secret + ) + except WebhookVerificationException: + raise HTTPException(status_code=403, detail="Invalid webhook signature") + except WebhookPayloadException: + raise HTTPException(status_code=400, detail="Invalid JSON payload") + else: + try: + event = client.parse_webhook(body_text) + except WebhookPayloadException: + raise HTTPException(status_code=400, detail="Invalid JSON payload") event_type = event.event_type target = event.target_identifiers diff --git a/navi/api/websocket.py b/navi/api/websocket.py index 143fc41..bb8c06d 100644 --- a/navi/api/websocket.py +++ b/navi/api/websocket.py @@ -32,6 +32,7 @@ from navi.auth.deps import get_current_user, get_current_user_ws from navi.auth import User from navi.auth.deps import check_session_access +from navi.config import settings from navi.core import SessionStore router = APIRouter(tags=["websocket"]) @@ -103,7 +104,11 @@ session = await store.get(session_id) if session is None: raise HTTPException(status_code=404, detail="Session not found") - if user is None and session.user_id is not None: + # In auth mode an anonymous caller may not touch any session — including + # legacy sessions with no owner (they give access to tools with + # unrestricted defaults). Without auth the request resolves to the + # anonymous admin, so this branch never fires there. + if user is None and (session.user_id is not None or settings.navi_auth_enabled): raise HTTPException(status_code=401, detail="Authentication required") if user is not None: check_session_access(session, user) @@ -147,11 +152,18 @@ # Accept the WebSocket before checking access so that auth failures can be # sent as WebSocket close codes rather than HTTP 403 on the upgrade request. await websocket.accept() - orchestrator.add_websocket(session_id, websocket) log.info("ws.accepted", session_id=session_id) if user is None: - # Anonymous users may only connect to legacy sessions (no owner). + # In auth mode anonymous connections are rejected outright — legacy + # sessions (no owner) would otherwise hand an anonymous client the + # full tool surface (terminal/filesystem with unrestricted defaults). + # Without auth, requests resolve to the anonymous admin, so only the + # ownerless-legacy rule remains relevant there. + if settings.navi_auth_enabled: + log.warning("ws.anonymous_denied", session_id=session_id) + await websocket.close(code=4003, reason="Authentication required") + return if session.user_id is not None: log.warning("ws.anonymous_denied", session_id=session_id) await websocket.close(code=4003, reason="Authentication required") @@ -165,6 +177,10 @@ await websocket.close(code=4003, reason="Access denied") return + # Registered only after all access checks pass, so a rejected socket never + # lingers in the orchestrator's subscriber list. + orchestrator.add_websocket(session_id, websocket) + queue: asyncio.Queue | None = None current_run = None diff --git a/navi/auth/deps.py b/navi/auth/deps.py index 537168a..2d735ab 100644 --- a/navi/auth/deps.py +++ b/navi/auth/deps.py @@ -48,7 +48,9 @@ def _cleanup_refresh_locks() -> None: """Remove stale refresh locks to prevent unbounded memory growth.""" - now = time.time() + # Cache timestamps are written with time.monotonic() (see _set_cached_user), + # so the freshness check must compare against the same clock. + now = time.monotonic() stale = [sid for sid, lock in _refresh_locks.items() if sid not in _user_cache or _user_cache[sid][1] + _USER_CACHE_TTL < now] for sid in stale: _refresh_locks.pop(sid, None) diff --git a/navi/config.py b/navi/config.py index aa5df28..4c9538c 100644 --- a/navi/config.py +++ b/navi/config.py @@ -92,6 +92,12 @@ gnauth_user_role_slug: str = "navi_user" gnauth_profile_path: str = "/account/profile" # appended to gnauth_base_url for profile links + # HMAC secret for gnexus-auth webhooks (from the gnexus-auth admin panel). + # When empty and navi_auth_enabled=true, the webhook endpoint refuses to + # process unsigned events (503). When auth is disabled, unsigned webhooks + # are accepted with a warning. + gnauth_webhook_secret: str = "" + # Master auth switch. When false, Navi skips OAuth/API-token checks and # treats every request as the anonymous admin user. Use only for trusted # single-user/local deployments. @@ -108,6 +114,12 @@ navi_auth_cookie_samesite: str = "lax" navi_auth_cookie_max_age_days: int = 30 + # Comma-separated list of origins allowed by CORS when navi_auth_enabled=true + # (cookie-credentialed requests need an explicit origin list — "*" is not + # accepted by browsers together with credentials). Required in auth mode: + # the app refuses to start with an empty list. Ignored when auth is disabled. + navi_allowed_origins: str = "" + # LLM call timeouts # complete() is non-streaming (planning, compression) — blocked until full response llm_complete_timeout: int = 120 @@ -160,5 +172,9 @@ def terminal_user_allowed_commands_list(self) -> list[str]: return [c.strip() for c in self.terminal_user_allowed_commands.split(",") if c.strip()] + @property + def navi_allowed_origins_list(self) -> list[str]: + return [o.strip() for o in self.navi_allowed_origins.split(",") if o.strip()] + settings = Settings() diff --git a/navi/core/registry.py b/navi/core/registry.py index 1ed9217..28bf849 100644 --- a/navi/core/registry.py +++ b/navi/core/registry.py @@ -1,5 +1,7 @@ """Registries for tools, profiles, and LLM backends.""" +import structlog + from navi.config import settings from navi.exceptions import ProfileNotFound, ToolNotFound from navi.llm.base import LLMBackend @@ -36,6 +38,8 @@ from navi.tools._internal.logging_middleware import LoggingMiddleware from navi.context_providers._loader import ContextProviderRegistry +log = structlog.get_logger() + class ToolRegistry: def __init__(self) -> None: diff --git a/navi/main.py b/navi/main.py index 7d6093c..3336a58 100644 --- a/navi/main.py +++ b/navi/main.py @@ -7,11 +7,12 @@ import logging import structlog -from fastapi import FastAPI +from fastapi import Depends, FastAPI from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import FileResponse from fastapi.staticfiles import StaticFiles +from navi.api.deps import require_admin from navi.api.routes import agents, api_tokens, auth, health, messages, sessions, webhooks from navi.api.routes.admin import router as admin_router from navi.api.websocket import router as ws_router @@ -34,6 +35,16 @@ @asynccontextmanager async def lifespan(app: FastAPI): log = structlog.get_logger() + + # Fail fast: credentialed CORS needs an explicit origin list — browsers reject + # "*" together with credentials, and an implicit empty list would silently + # lock out (or leave open) cookie-based access. + if settings.navi_auth_enabled and not settings.navi_allowed_origins_list: + raise RuntimeError( + "NAVI_ALLOWED_ORIGINS must be set (comma-separated list) when NAVI_AUTH_ENABLED=true. " + "Refusing to start with permissive CORS in auth mode." + ) + container = await create_container() app.state.container = container @@ -129,10 +140,20 @@ lifespan=lifespan, ) +# Credentialed CORS requires an explicit origin list (browsers refuse "*" +# together with credentials). In no-auth mode there are no cookies to send, +# so a permissive policy is acceptable there. +if settings.navi_auth_enabled: + _cors_origins = settings.navi_allowed_origins_list + _cors_credentials = True +else: + _cors_origins = ["*"] + _cors_credentials = False + app.add_middleware( CORSMiddleware, - allow_origins=["*"], - allow_credentials=True, + allow_origins=_cors_origins, + allow_credentials=_cors_credentials, allow_methods=["*"], allow_headers=["*"], ) @@ -146,7 +167,9 @@ app.include_router(ws_router) app.include_router(webhooks.router) app.include_router(admin_router) -app.include_router(eval_router) +# Eval endpoints spend LLM tokens and read session data — admin only. +# (With auth disabled require_admin resolves to the anonymous admin user.) +app.include_router(eval_router, dependencies=[Depends(require_admin)]) app.mount("/assets", StaticFiles(directory=str(_base / "webclient" / "dist" / "assets")), name="assets") app.mount("/images", StaticFiles(directory=str(_base / "webclient" / "dist" / "images")), name="images") @@ -170,14 +193,17 @@ return FileResponse(str(_base / "webclient" / "dist" / "index.html"), headers={"Cache-Control": "no-store"}) -@app.get("/debug", include_in_schema=False) -async def debug() -> FileResponse: - return FileResponse("debug/index.html", headers={"Cache-Control": "no-store"}) +if not settings.navi_auth_enabled: + # Debug panels are a local-development tool — not exposed in auth mode. + + @app.get("/debug", include_in_schema=False) + async def debug() -> FileResponse: + return FileResponse("debug/index.html", headers={"Cache-Control": "no-store"}) -@app.get("/debug/eval", include_in_schema=False) -async def debug_eval() -> FileResponse: - return FileResponse("debug/eval/index.html", headers={"Cache-Control": "no-store"}) + @app.get("/debug/eval", include_in_schema=False) + async def debug_eval() -> FileResponse: + return FileResponse("debug/eval/index.html", headers={"Cache-Control": "no-store"}) @app.get("/admin", include_in_schema=False) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 7b9e693..dfbb152 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -99,6 +99,9 @@ return fake_user monkeypatch.setattr("navi.auth.deps.get_current_user_ws", _fake_get_user_ws) monkeypatch.setattr("navi.api.deps.get_current_user_ws", _fake_get_user_ws) + # The WS handler calls get_current_user_ws directly (not via Depends), + # so the module-level name it captured must be patched too. + monkeypatch.setattr("navi.api.websocket.get_current_user_ws", _fake_get_user_ws) app.dependency_overrides[get_current_user_ws] = lambda: fake_user app.dependency_overrides[require_user] = lambda: fake_user app.dependency_overrides[require_admin] = lambda: fake_user diff --git a/tests/integration/test_api_routes.py b/tests/integration/test_api_routes.py index 30dbabd..c3e310e 100644 --- a/tests/integration/test_api_routes.py +++ b/tests/integration/test_api_routes.py @@ -300,6 +300,93 @@ response = client.post("/sessions/nonexistent/messages", json={"content": "hi"}) assert response.status_code == 404 + @pytest.mark.anyio + async def test_send_message_conflict_when_running(self, client, make_session, mock_deps): + """A REST message during an active run is rejected with 409, not interleaved.""" + session = await make_session("secretary") + + from navi.main import app + orchestrator = app.state.container.orchestrator + orchestrator.create_run(session.id) # simulate an active WS turn + + response = client.post(f"/sessions/{session.id}/messages", json={"content": "hi"}) + assert response.status_code == 409 + orchestrator.clear_run(session.id) + + @pytest.mark.anyio + async def test_send_message_marks_busy_and_clears_after(self, client, make_session, mock_deps): + """The REST run publishes busy state for its duration and clears it after.""" + session = await make_session("secretary") + + from navi.main import app + orchestrator = app.state.container.orchestrator + seen_running = [] + + class BusyProbeAgent: + async def run(self, session_id, user_message, images=None): + seen_running.append(orchestrator.is_running(session_id)) + return "done" + + app.state.container._agent = BusyProbeAgent() + + response = client.post(f"/sessions/{session.id}/messages", json={"content": "hi"}) + assert response.status_code == 200 + assert seen_running == [True], "agent run must observe is_running() == True" + assert orchestrator.is_running(session.id) is False, "busy flag must clear after the run" + + +class TestSecurityGates: + def test_eval_requires_auth_for_anonymous(self, client, monkeypatch): + """Eval endpoints (token spend + session reads) must reject anonymous callers.""" + from navi.config import Settings + import navi.auth.deps as auth_deps + from navi.main import app + + monkeypatch.setattr( + auth_deps, + "settings", + Settings( + _env_file=None, + navi_persona_file="", + navi_auth_enabled=True, + gnauth_client_id="cid", + gnauth_client_secret="csecret", + ), + ) + + saved_admin = None + saved_user = None + from navi.api.deps import get_current_user, require_admin + + if require_admin in app.dependency_overrides: + saved_admin = app.dependency_overrides.pop(require_admin) + # get_current_user is overridden by the conftest to a fake admin — + # remove the override so the real resolution (no cookie → anonymous) runs. + if get_current_user in app.dependency_overrides: + saved_user = app.dependency_overrides.pop(get_current_user) + + try: + response = client.get("/eval/stats") + assert response.status_code == 401 + finally: + if saved_admin is not None: + app.dependency_overrides[require_admin] = saved_admin + if saved_user is not None: + app.dependency_overrides[get_current_user] = saved_user + + def test_debug_routes_match_auth_mode(self, mock_deps): + """Debug panels are registered only in no-auth (local dev) mode.""" + from navi.main import app + from navi.config import settings as main_settings + + route_paths = {getattr(r, "path", None) for r in app.routes} + if main_settings.navi_auth_enabled: + assert "/debug" not in route_paths + assert "/debug/eval" not in route_paths + else: + assert "/debug" in route_paths + assert "/debug/eval" in route_paths + class TestAdmin: def test_list_users(self, client, mock_deps): diff --git a/tests/integration/test_websocket.py b/tests/integration/test_websocket.py index f9d0653..d66ac22 100644 --- a/tests/integration/test_websocket.py +++ b/tests/integration/test_websocket.py @@ -175,3 +175,30 @@ if msg.get("type") in ("stream_end", "error", "stream_stopped", "session_sync"): break return msgs + + +class TestWebSocketAnonymous: + @pytest.mark.anyio + async def test_anonymous_rejected_when_auth_enabled(self, client, make_session, monkeypatch): + """With auth on, a client without a session cookie cannot attach — + even to a legacy ownerless session (regression: anonymous WS + unrestricted + terminal/filesystem defaults = remote code execution).""" + from starlette.testclient import WebSocketDisconnect + + from navi.api import websocket as ws_mod + from navi.config import Settings + + session = await make_session("secretary") + assert session.user_id is None # legacy session in the fake store + + monkeypatch.setattr(ws_mod, "settings", Settings(_env_file=None, navi_persona_file="", navi_auth_enabled=True)) + + async def _anonymous_user(websocket): + return None + + monkeypatch.setattr(ws_mod, "get_current_user_ws", _anonymous_user) + + with client.websocket_connect(f"/ws/sessions/{session.id}") as ws: + with pytest.raises(WebSocketDisconnect) as exc_info: + ws.receive_json() + assert exc_info.value.code == 4003 diff --git a/tests/unit/api/test_auth_mobile_done.py b/tests/unit/api/test_auth_mobile_done.py new file mode 100644 index 0000000..047cae3 --- /dev/null +++ b/tests/unit/api/test_auth_mobile_done.py @@ -0,0 +1,44 @@ +"""Tests for the /auth/mobile-done bridge page — sid validation and output escaping.""" + +import uuid + +import pytest +from fastapi.testclient import TestClient + + +@pytest.fixture +def client(): + from navi.main import app + + return TestClient(app) + + +def test_mobile_done_valid_sid_renders_intent(client): + sid = uuid.uuid4().hex + resp = client.get(f"/auth/mobile-done?sid={sid}") + + assert resp.status_code == 200 + body = resp.text + assert f"sid={sid}" in body + # Auto deep-link must survive (JS string is json-escaped) + assert "window.location.href=" in body + assert "Content-Security-Policy" in resp.headers + + +def test_mobile_done_rejects_malformed_sid(client): + """Anything that is not a 32-char lowercase hex sid is refused — this is the + reflected-XSS vector (sid lands in href and a JS string).""" + for bad in ["abc123", '">', "", "0" * 33, "Z" * 32]: + resp = client.get("/auth/mobile-done", params={"sid": bad}) + assert resp.status_code == 400, f"sid={bad!r} must be rejected" + + +def test_mobile_done_no_injection_in_html(client): + """Even with a valid-format sid, the rendered page contains no script + breakout payloads (static markup quotes like lang="en"> are fine).""" + sid = uuid.uuid4().hex + resp = client.get(f"/auth/mobile-done?sid={sid}") + assert "ok') + expect(html).not.toContain(' { + const html = renderMarkdown('') + expect(html).not.toContain('onerror') + }) + + it('neutralizes javascript: hrefs', () => { + const html = renderMarkdown('[click](javascript:alert(1))') + expect(html).not.toContain('javascript:') + }) + + it('blocks data:text/html but keeps data:image', () => { + const evil = renderMarkdown('[x](data:text/html;base64,PHNjcmlwdD4=)') + expect(evil).not.toContain('data:text/html') + + const img = renderMarkdown('![pic](data:image/png;base64,aGVsbG8=)') + expect(img).toContain('data:image/png;base64,aGVsbG8=') + }) + + it('preserves code block structure: copy button, data-code, hljs classes', () => { + const html = renderMarkdown('```python\nprint("hi")\n```') + expect(html).toContain('code-block') + expect(html).toContain('copy-btn') + expect(html).toContain('data-code="') + expect(html).toContain('hljs language-python') + expect(html).toContain(' { + const html = renderMarkdown('| a | b |\n| --- | --- |\n| 1 | 2 |\n\n- [x] done') + expect(html).toContain('
{ + const evil = renderMarkdown('![a](javascript:alert(1))') + expect(evil).not.toContain('javascript:') + expect(evil).toContain('is-broken') + + const proto = renderMarkdown('![a](https://example.com/x.png)') + expect(proto).toContain('src="https://example.com/x.png"') + + const rel = renderMarkdown('![a](/static/x.png)') + expect(rel).toContain('src="/static/x.png"') + + const schemeless = renderMarkdown('![a](//evil.com/x.png)') + expect(schemeless).not.toContain('//evil.com') + }) + + it('rendered images carry no inline onerror attribute', () => { + const html = renderMarkdown('![a](https://example.com/y.png)') + expect(html).not.toContain('onerror=') + }) +}) + +describe('attachImageLightbox', () => { + it('marks broken images via the error event', () => { + const el = document.createElement('div') + el.innerHTML = renderMarkdown('![a](https://example.com/broken.png)') + attachImageLightbox(el) + + const img = el.querySelector('img.msg-md-image') + const link = el.querySelector('a.msg-md-image-link') + expect(img).toBeTruthy() + + img.dispatchEvent(new Event('error')) + expect(img.classList.contains('is-broken')).toBe(true) + expect(link.classList.contains('is-broken')).toBe(true) + }) +}) \ No newline at end of file diff --git a/webclient/tests/unit/composables/useWebSocket.test.js b/webclient/tests/unit/composables/useWebSocket.test.js index ac44d02..581fe17 100644 --- a/webclient/tests/unit/composables/useWebSocket.test.js +++ b/webclient/tests/unit/composables/useWebSocket.test.js @@ -30,6 +30,12 @@ // Stub global WebSocket before each test beforeEach(() => { vi.stubGlobal('WebSocket', MockWebSocket) + // happy-dom does not provide localStorage — getWsUrl reads it for the API token + vi.stubGlobal('localStorage', { + getItem: vi.fn(() => null), + setItem: vi.fn(), + removeItem: vi.fn(), + }) setActivePinia(createPinia()) }) diff --git a/webclient/vitest.config.js b/webclient/vitest.config.js index d7c547a..d701c02 100644 --- a/webclient/vitest.config.js +++ b/webclient/vitest.config.js @@ -1,20 +1,29 @@ import { defineConfig } from 'vitest/config' import vue from '@vitejs/plugin-vue' import { fileURLToPath, URL } from 'node:url' +import { resolve } from 'node:path' export default defineConfig({ plugins: [vue()], resolve: { alias: { '@': fileURLToPath(new URL('./src', import.meta.url)), + // Mirror vite.config.js — src/ imports the ui-kit by bare name. + 'gnexus-ui-kit/vue': resolve(__dirname, 'vendor/gnexus-ui-kit/dist/vue/index.js'), + 'gnexus-ui-kit/css': resolve(__dirname, 'vendor/gnexus-ui-kit/dist/css/kit.css'), }, }, test: { environment: 'happy-dom', globals: true, include: ['tests/**/*.test.js'], + // NOTE: DOMPurify is incompatible with happy-dom — its base + // Node.prototype nodeName getter returns "" for elements, so + // DOMPurify strips every tag. Tests that exercise DOMPurify + // (useMarkdown.test.js) opt into jsdom via a @vitest-environment + // pragma. alias: { '@': fileURLToPath(new URL('./src', import.meta.url)), }, }, -}) +}) \ No newline at end of file