diff --git a/app.py b/app.py index a89f80143c..79a6d07a51 100644 --- a/app.py +++ b/app.py @@ -77,6 +77,7 @@ def register_static_mime_types() -> None: import bcrypt as _bcrypt from src.app_helpers import abs_join, serve_html_with_nonce +from src.constants import PROXY_TUNNEL_HEADERS from src.generated_images import GENERATED_IMAGE_HEADERS, resolve_generated_image_path from starlette.responses import RedirectResponse @@ -328,15 +329,6 @@ def _refresh_token_cache(): _token_cache.update(new_map) app.state._token_cache_dirty = False - # Headers that prove a request was forwarded by a proxy/tunnel (cloudflared, - # nginx, Caddy, Tailscale Funnel, …). cloudflared connects to the app FROM - # 127.0.0.1, so without this check every tunneled request would look like - # loopback and could bypass auth. - _PROXY_FWD_HEADERS = ( - "cf-connecting-ip", "cf-ray", "cf-visitor", - "x-forwarded-for", "x-forwarded-host", "x-real-ip", "forwarded", - ) - def _is_trusted_loopback(request: Request) -> bool: """True ONLY for a DIRECT loopback connection with no proxy/tunnel forwarding headers. A bare ``client.host in ('127.0.0.1','::1')`` check is @@ -348,8 +340,8 @@ def _is_trusted_loopback(request: Request) -> bool: host = request.client.host if request.client else None if host not in ("127.0.0.1", "::1"): return False - for _h in _PROXY_FWD_HEADERS: - if request.headers.get(_h): + for _h in PROXY_TUNNEL_HEADERS: + if _h in request.headers: return False return True diff --git a/docs/setup.md b/docs/setup.md index fd63e02a53..857f83bb5c 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -396,6 +396,10 @@ Odysseus serves plain HTTP on its app port. Docker Compose binds Odysseus and th 4. Keep raw service and model ports internal-only. Cloudflare Access, Tailscale, Caddy, nginx, and Traefik can all fit this pattern; none are required by Odysseus. If your access layer reaches Odysseus on the same host, proxy to `http://127.0.0.1:7000` and keep `AUTH_ENABLED=true`, `LOCALHOST_BYPASS=false`, and `SECURE_COOKIES=true`. + +Create the first administrator before enabling the proxy by opening `http://127.0.0.1:7000/setup` directly on the Odysseus host. First-run setup is rejected through reverse proxies and other network-facing hostnames. + +For a headless remote host, establish an SSH tunnel before enabling the proxy: `ssh -L 7000:127.0.0.1:7000 user@host`, then open `http://127.0.0.1:7000/setup` in the local browser. Platforms that cannot provide loopback access must complete bootstrap before exposing the service; Odysseus intentionally provides no remote setup bypass. `ALLOWED_ORIGINS` lists exact permitted origins for cross-origin browser/API clients; ordinary same-origin reverse-proxy access usually does not need a special CORS entry. Common internal-only ports from the default docs/compose setup: diff --git a/routes/auth_routes.py b/routes/auth_routes.py index 5c7a4e04ab..7350aeed5a 100644 --- a/routes/auth_routes.py +++ b/routes/auth_routes.py @@ -13,7 +13,13 @@ from core.atomic_io import atomic_write_json, atomic_write_text from core.auth import AuthManager, RESERVED_USERNAMES, SetAdminResult, TOKEN_TTL -from src.constants import DEEP_RESEARCH_DIR, MEMORY_FILE, PASSWORD_MIN_LENGTH, SKILLS_DIR +from src.constants import ( + DEEP_RESEARCH_DIR, + MEMORY_FILE, + PASSWORD_MIN_LENGTH, + PROXY_TUNNEL_HEADERS, + SKILLS_DIR, +) from src.rate_limiter import RateLimiter from src.settings_scrub import scrub_settings from src.settings import ( @@ -84,6 +90,41 @@ class SetOpenRegistrationRequest(BaseModel): SESSION_COOKIE = "odysseus_session" +_LOOPBACK_HOSTS = {"127.0.0.1", "::1", "localhost"} +def _request_from_loopback(request: Request) -> bool: + """Return true only for a direct localhost request. + + A reverse proxy on the same host also connects from loopback, so the TCP + peer alone is not sufficient. Require both the peer and requested host to + be localhost and reject requests carrying proxy forwarding headers. + + A misconfigured same-host proxy that rewrites Host to loopback and strips + every forwarding header is indistinguishable from a direct request. The + deployment contract therefore also requires completing setup before the + proxy is enabled, as documented in docs/setup.md. + """ + client_host = ( + str(request.client.host if request.client else "") + .strip() + .rstrip(".") + .lower() + ) + request_host = ( + str(getattr(getattr(request, "url", None), "hostname", "") or "") + .strip() + .rstrip(".") + .lower() + ) + + if any(name in request.headers for name in PROXY_TUNNEL_HEADERS): + return False + + return ( + client_host in _LOOPBACK_HOSTS + and request_host in _LOOPBACK_HOSTS + ) + + def setup_auth_routes(auth_manager: AuthManager) -> APIRouter: router = APIRouter(prefix="/api/auth", tags=["auth"]) @@ -102,6 +143,11 @@ async def first_run_setup(body: SetupRequest, request: Request): raise HTTPException(429, "Too many requests — try again later") if auth_manager.is_configured: raise HTTPException(400, "Already configured") + if not _request_from_loopback(request): + raise HTTPException( + 403, + "First-run setup must be completed directly from localhost", + ) if len(body.password) < PASSWORD_MIN_LENGTH: raise HTTPException(400, f"Password must be at least {PASSWORD_MIN_LENGTH} characters") if len(body.username.strip()) < 1: diff --git a/routes/email_helpers.py b/routes/email_helpers.py index b04be60668..d808fb7ce6 100644 --- a/routes/email_helpers.py +++ b/routes/email_helpers.py @@ -890,12 +890,14 @@ def _get_email_config(account_id: str | None = None, owner: str = "") -> dict: """Return IMAP/SMTP config as a dict. Resolution order: - 1. If account_id given → that specific EmailAccount row. + 1. If account_id given → that specific enabled EmailAccount row only. 2. Else → the row with is_default=True (scoped to `owner` when given). 3. Else → the first enabled row (scoped to `owner` when given). - 4. Else → legacy flat keys in data/settings.json (kept for envs - where the migration hasn't run yet or accounts table is empty). - 5. Else → env vars (SMTP_HOST / IMAP_HOST / ...). + 4. For unscoped single-user callers only, legacy flat keys in + data/settings.json (kept for envs where the migration hasn't run yet + or accounts table is empty). + 5. For unscoped single-user callers only, env vars + (SMTP_HOST / IMAP_HOST / ...). Returned dict always has the same shape as before; an `account_id` key is added so callers can stamp derivative records (email_ai_replies etc.). @@ -933,11 +935,13 @@ def _owner_or_matching_legacy_account(query): row = None # Fallback path — restrict to this owner's accounts so we don't # leak another user's default mailbox to an unconfigured user. - if row is None: + # An explicit selection is authoritative: a missing, disabled, or + # invisible row must not silently turn into a different account. + if row is None and not account_id: q = db.query(_EA).filter(_EA.is_default == True, _EA.enabled == True) # noqa: E712 q = _owner_or_matching_legacy_account(q) row = q.first() - if row is None: + if row is None and not account_id: q = db.query(_EA).filter(_EA.enabled == True) # noqa: E712 q = _owner_or_matching_legacy_account(q) row = q.order_by(_EA.created_at.asc()).first() @@ -972,27 +976,38 @@ def _owner_or_matching_legacy_account(query): finally: db.close() except Exception as e: + # Owner-scoped and explicit-account calls must fail closed. Falling + # through here would replace a transient database error with shared + # settings/environment credentials, or replace an explicit selection + # with an unrelated legacy account. + if owner or account_id: + raise logger.debug(f"email_accounts lookup failed, falling back to settings.json: {e}") # Legacy fallback — flat keys in settings.json / env vars - settings = _load_settings() + # These credentials predate owner scoping and carry no safe tenant + # attribution. Preserve them only for trusted single-user callers that did + # not explicitly select an account. + legacy_allowed = not owner and not account_id + settings = _load_settings() if legacy_allowed else {} + environ = os.environ if legacy_allowed else {} cfg = { "account_id": resolved_id, "account_name": "legacy", - "smtp_host": settings.get("smtp_host", os.environ.get("SMTP_HOST", "")), - "smtp_port": int(settings.get("smtp_port", os.environ.get("SMTP_PORT", "465")) or 465), + "smtp_host": settings.get("smtp_host", environ.get("SMTP_HOST", "")), + "smtp_port": int(settings.get("smtp_port", environ.get("SMTP_PORT", "465")) or 465), "smtp_security": _smtp_security_mode({ - "smtp_security": settings.get("smtp_security", os.environ.get("SMTP_SECURITY", "")), - "smtp_port": settings.get("smtp_port", os.environ.get("SMTP_PORT", "465")), + "smtp_security": settings.get("smtp_security", environ.get("SMTP_SECURITY", "")), + "smtp_port": settings.get("smtp_port", environ.get("SMTP_PORT", "465")), }), - "smtp_user": settings.get("smtp_user", os.environ.get("SMTP_USER", "")), - "smtp_password": settings.get("smtp_password", os.environ.get("SMTP_PASSWORD", "")), - "imap_host": settings.get("imap_host", os.environ.get("IMAP_HOST", "")), - "imap_port": int(settings.get("imap_port", os.environ.get("IMAP_PORT", "993")) or 993), - "imap_user": settings.get("imap_user", os.environ.get("IMAP_USER", "")), - "imap_password": settings.get("imap_password", os.environ.get("IMAP_PASSWORD", "")), + "smtp_user": settings.get("smtp_user", environ.get("SMTP_USER", "")), + "smtp_password": settings.get("smtp_password", environ.get("SMTP_PASSWORD", "")), + "imap_host": settings.get("imap_host", environ.get("IMAP_HOST", "")), + "imap_port": int(settings.get("imap_port", environ.get("IMAP_PORT", "993")) or 993), + "imap_user": settings.get("imap_user", environ.get("IMAP_USER", "")), + "imap_password": settings.get("imap_password", environ.get("IMAP_PASSWORD", "")), "imap_starttls": settings.get("imap_starttls", True), - "from_address": settings.get("email_from", os.environ.get("EMAIL_FROM", "")), + "from_address": settings.get("email_from", environ.get("EMAIL_FROM", "")), } if not (cfg["smtp_host"] and cfg["smtp_user"] and cfg["smtp_password"]): logger.warning("SMTP not configured — add an Email Account in Settings or set env vars") diff --git a/routes/email_pollers.py b/routes/email_pollers.py index 8408103daa..94c2e885bd 100644 --- a/routes/email_pollers.py +++ b/routes/email_pollers.py @@ -33,6 +33,8 @@ from routes.email_helpers import ( _strip_think, _extract_reply, _apply_email_style_mechanics, _load_settings, _save_settings, _get_email_config, + _account_visible_to_owner, + EmailNotConfiguredError, _send_smtp_message, _imap_connect, _imap, _decode_header, _detect_sent_folder, _detect_spam_folder, _imap_move, @@ -100,6 +102,24 @@ def _owner_for_email_account(account_id: str | None) -> str: return "" +def _account_visible_to_task_owner(account_id: str | None, owner: str) -> bool: + """Check an explicit task account against the canonical visibility policy.""" + if not account_id or not owner: + return True + from core.database import SessionLocal as _SL, EmailAccount as _EA + + db = _SL() + try: + row = db.query(_EA).filter(_EA.id == account_id).first() + return ( + row is not None + and bool(row.enabled) + and _account_visible_to_owner(row, owner) + ) + finally: + db.close() + + # ── Routes ── async def _emit_progress(progress_cb, message: str): @@ -119,6 +139,7 @@ async def _run_auto_summarize_once(do_summary: bool = True, do_reply: bool = Tru days_back: int = 1, account_id: str | None = None, max_process: int | None = None, + owner: str = "", progress_cb=None) -> str: """One iteration of the email scan. Temporarily flips settings flags so the existing background-loop logic runs exactly once for the requested ops.""" @@ -137,6 +158,7 @@ async def _run_auto_summarize_once(do_summary: bool = True, do_reply: bool = Tru days_back=days_back, account_id=account_id, max_process=max_process, + owner=owner, progress_cb=progress_cb, ) finally: @@ -176,7 +198,7 @@ def _latest_inbox_fallback_uids(conn, reconnect): return [], reconnect() -async def _auto_summarize_pass(days_back: int = 1, account_id: str | None = None, max_process: int | None = None, progress_cb=None) -> str: +async def _auto_summarize_pass(days_back: int = 1, account_id: str | None = None, max_process: int | None = None, owner: str = "", progress_cb=None) -> str: """Single pass of the auto-summarize/reply scan. When account_id is None, iterates over every enabled account in @@ -194,19 +216,29 @@ async def _auto_summarize_pass(days_back: int = 1, account_id: str | None = None .order_by(_EA.is_default.desc(), _EA.created_at.asc()) .all() ) + if owner: + rows = [r for r in rows if _account_visible_to_owner(r, owner)] ids = [r.id for r in rows] names = {r.id: r.name for r in rows} finally: db.close() except Exception: + if owner: + raise ids = [] names = {} + if not ids and owner: + raise EmailNotConfiguredError( + "No email accounts are available to this task owner" + ) if len(ids) <= 1: - # Single-account (or zero rows — fallback to legacy settings.json lookup) + # Unscoped zero-account callers retain the single-user legacy + # settings fallback. Owner-scoped callers fail closed above. return await _auto_summarize_pass_single( days_back=days_back, account_id=(ids[0] if ids else None), max_process=max_process, + owner=owner, progress_cb=progress_cb, ) outs = [] @@ -217,6 +249,7 @@ async def _auto_summarize_pass(days_back: int = 1, account_id: str | None = None days_back=days_back, account_id=aid, max_process=max_process, + owner=owner, progress_cb=progress_cb, ) outs.append(f"[{names.get(aid, aid[:8])}] {result}") @@ -228,11 +261,12 @@ async def _auto_summarize_pass(days_back: int = 1, account_id: str | None = None days_back=days_back, account_id=account_id, max_process=max_process, + owner=owner, progress_cb=progress_cb, ) -async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None = None, max_process: int | None = None, progress_cb=None) -> str: +async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None = None, max_process: int | None = None, owner: str = "", progress_cb=None) -> str: """Single pass of the auto-summarize/reply scan for ONE account. Reads current settings flags.""" import asyncio @@ -249,11 +283,18 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None return "Nothing to do" # Owner of the account being processed. All calendar + mailbox reads/writes - # below are scoped to this user: the multi-account fan-out runs every user's - # mailbox, so an unscoped pass would disclose/mutate other tenants' data. + # below are scoped to this user. Owner-filtered fan-out must never process + # another tenant's mailbox, or it could disclose/mutate that tenant's data. # One resolution feeds both the mailbox path (account_owner) and upstream's # calendar path (_acct_owner, which expects None rather than ""). - account_owner = _owner_for_email_account(account_id) + if owner and account_id and not _account_visible_to_task_owner(account_id, owner): + raise PermissionError( + "Email account is not available to this task owner" + ) + + # A scheduled task owner is authoritative. Only trusted legacy/system + # callers without an owner may derive authority from the account row. + account_owner = owner or _owner_for_email_account(account_id) _acct_owner = account_owner or None conn = None diff --git a/src/builtin_actions.py b/src/builtin_actions.py index 727d2699d1..1fa6150b44 100644 --- a/src/builtin_actions.py +++ b/src/builtin_actions.py @@ -539,6 +539,7 @@ async def action_summarize_emails(owner: str, **kwargs) -> Tuple[str, bool]: do_summary=True, do_reply=False, account_id=_email_task_account_id(kwargs), + owner=owner, ) if _result_is_config_error(result): return result, False @@ -559,6 +560,7 @@ async def action_draft_email_replies(owner: str, **kwargs) -> Tuple[str, bool]: do_reply=True, account_id=_email_task_account_id(kwargs), days_back=7, + owner=owner, progress_cb=kwargs.get("progress_cb"), ) if _result_is_config_error(result): @@ -1067,6 +1069,7 @@ async def action_extract_email_events(owner: str, **kwargs) -> Tuple[str, bool]: days_back=days_back, account_id=account_id, max_process=max_process, + owner=owner, ), timeout=timeout, ) @@ -1353,7 +1356,7 @@ async def action_daily_brief(owner: str, **kwargs) -> Tuple[str, bool]: recent_subjects: list[tuple[str, str]] = [] try: import email as _email - conn = _imap_connect(None) + conn = _imap_connect(None, owner=owner) try: conn.select("INBOX", readonly=True) status, data = conn.search(None, "UNSEEN") diff --git a/src/constants.py b/src/constants.py index eceeb6eb09..082557b75e 100644 --- a/src/constants.py +++ b/src/constants.py @@ -105,6 +105,21 @@ # Auth policy PASSWORD_MIN_LENGTH = 8 +# Headers added by reverse proxies and tunnels. A loopback TCP peer carrying +# any of these headers is not a direct localhost request. +PROXY_TUNNEL_HEADERS = ( + "cf-connecting-ip", + "cf-connecting-ipv6", + "cf-pseudo-ipv4", + "cf-ray", + "cf-visitor", + "forwarded", + "x-forwarded-for", + "x-forwarded-host", + "x-forwarded-proto", + "x-real-ip", +) + # Default parameters DEFAULT_TEMPERATURE = 1.0 DEFAULT_MAX_TOKENS = 0 diff --git a/tests/test_auth_policy.py b/tests/test_auth_policy.py index 8fceeccc0d..8b5aced5e4 100644 --- a/tests/test_auth_policy.py +++ b/tests/test_auth_policy.py @@ -150,7 +150,11 @@ def _change_password_endpoint(auth_manager): def test_setup_rejects_short_password(tmp_path): mgr = _make_manager(tmp_path) endpoint, SetupRequest = _setup_endpoint(mgr) - request = SimpleNamespace(client=SimpleNamespace(host="127.0.0.1")) + request = SimpleNamespace( + client=SimpleNamespace(host="127.0.0.1"), + headers={}, + url=SimpleNamespace(hostname="localhost"), + ) body = SetupRequest(username="admin", password="short") with pytest.raises(HTTPException) as exc: @@ -197,7 +201,11 @@ def test_change_password_rejects_short_password(tmp_path): def test_setup_accepts_exactly_min_length_password(tmp_path): mgr = _make_manager(tmp_path) endpoint, SetupRequest = _setup_endpoint(mgr) - request = SimpleNamespace(client=SimpleNamespace(host="127.0.0.1")) + request = SimpleNamespace( + client=SimpleNamespace(host="127.0.0.1"), + headers={}, + url=SimpleNamespace(hostname="localhost"), + ) body = SetupRequest(username="admin", password="12345678") result = asyncio.run(endpoint(body=body, request=request)) @@ -208,7 +216,11 @@ def test_setup_accepts_exactly_min_length_password(tmp_path): def test_setup_rejects_seven_char_password(tmp_path): mgr = _make_manager(tmp_path) endpoint, SetupRequest = _setup_endpoint(mgr) - request = SimpleNamespace(client=SimpleNamespace(host="127.0.0.1")) + request = SimpleNamespace( + client=SimpleNamespace(host="127.0.0.1"), + headers={}, + url=SimpleNamespace(hostname="localhost"), + ) body = SetupRequest(username="admin", password="1234567") with pytest.raises(HTTPException) as exc: diff --git a/tests/test_email_task_account_owner_scope.py b/tests/test_email_task_account_owner_scope.py new file mode 100644 index 0000000000..f58a8df2dc --- /dev/null +++ b/tests/test_email_task_account_owner_scope.py @@ -0,0 +1,533 @@ +"""Regression tests for explicit email-account ownership in scheduled tasks.""" + +from unittest import mock + +import pytest + + +def _make_db(): + from sqlalchemy import create_engine + from sqlalchemy.orm import sessionmaker + + from core.database import Base + + engine = create_engine( + "sqlite:///:memory:", + connect_args={"check_same_thread": False}, + ) + Base.metadata.create_all(engine) + return sessionmaker(bind=engine) + + +def _make_account( + Factory, + account_id, + owner, + imap_user, + *, + enabled=True, + is_default=False, +): + from core.database import EmailAccount + + db = Factory() + db.add( + EmailAccount( + id=account_id, + owner=owner, + name=account_id, + enabled=enabled, + is_default=is_default, + imap_host="imap.example.com", + imap_user=imap_user, + smtp_host="smtp.example.com", + smtp_user=imap_user, + from_address=imap_user, + ) + ) + db.commit() + db.close() + + +def _legacy_config(): + return { + "imap_host": "legacy-imap.example.com", + "imap_user": "legacy@example.com", + "imap_password": "legacy-secret", + "smtp_host": "legacy-smtp.example.com", + "smtp_user": "legacy@example.com", + "smtp_password": "legacy-secret", + "email_from": "legacy@example.com", + } + + +@pytest.mark.asyncio +async def test_email_task_rejects_foreign_explicit_account(monkeypatch): + from routes import email_pollers + + imap_calls = [] + + monkeypatch.setattr( + email_pollers, + "_load_settings", + lambda: {"email_auto_summarize": True}, + ) + monkeypatch.setattr(email_pollers, "_account_visible_to_task_owner", lambda *_args: False) + + def fake_imap_connect(account_id=None, owner=""): + imap_calls.append((account_id, owner)) + raise AssertionError( + "IMAP must not be opened for a foreign account" + ) + + monkeypatch.setattr( + email_pollers, + "_imap_connect", + fake_imap_connect, + ) + + with pytest.raises( + PermissionError, + match="not available to this task owner", + ): + await email_pollers._auto_summarize_pass_single( + account_id="bob-account", + owner="alice", + ) + + assert imap_calls == [] + + +@pytest.mark.asyncio +async def test_email_task_rejects_disabled_explicit_account(monkeypatch): + """A disabled selection must not be replaced by another enabled account.""" + from routes import email_pollers + + Factory = _make_db() + _make_account( + Factory, + "alice-disabled", + "alice", + "alice-disabled@example.com", + enabled=False, + ) + _make_account( + Factory, + "alice-default", + "alice", + "alice-default@example.com", + is_default=True, + ) + + monkeypatch.setattr( + email_pollers, + "_load_settings", + lambda: {"email_auto_summarize": True}, + ) + monkeypatch.setattr( + email_pollers, + "_imap_connect", + lambda *_args, **_kwargs: pytest.fail( + "a disabled explicit account must fail before opening IMAP" + ), + ) + + with mock.patch("core.database.SessionLocal", Factory): + with pytest.raises( + PermissionError, + match="not available to this task owner", + ): + await email_pollers._auto_summarize_pass_single( + account_id="alice-disabled", + owner="alice", + ) + + +@pytest.mark.asyncio +async def test_email_task_with_no_visible_accounts_does_not_enter_single_pass(monkeypatch): + """An owner-scoped fan-out must not reinterpret zero rows as legacy mode.""" + from routes import email_pollers + from routes.email_helpers import EmailNotConfiguredError + + Factory = _make_db() + _make_account(Factory, "bob-account", "bob", "bob@example.com") + monkeypatch.setattr( + email_pollers, + "_auto_summarize_pass_single", + lambda **_kwargs: pytest.fail( + "zero visible owner accounts must fail before the single-account path" + ), + ) + + with mock.patch("core.database.SessionLocal", Factory): + with pytest.raises( + EmailNotConfiguredError, + match="No email accounts are available to this task owner", + ): + await email_pollers._auto_summarize_pass(owner="alice") + + +@pytest.mark.asyncio +async def test_unscoped_zero_account_fanout_preserves_legacy_single_pass(monkeypatch): + """Single-user callers without an owner keep the legacy fallback path.""" + from routes import email_pollers + + Factory = _make_db() + calls = [] + + async def fake_single(**kwargs): + calls.append((kwargs["account_id"], kwargs["owner"])) + return "legacy processed" + + monkeypatch.setattr(email_pollers, "_auto_summarize_pass_single", fake_single) + + with mock.patch("core.database.SessionLocal", Factory): + result = await email_pollers._auto_summarize_pass() + + assert result == "legacy processed" + assert calls == [(None, "")] + + +@pytest.mark.asyncio +async def test_email_task_fanout_propagates_account_discovery_failure(monkeypatch): + """A scoped discovery error must not be converted into legacy selection.""" + from core import database + from routes import email_pollers + + class FailingSession: + def __init__(self): + self.closed = False + + def query(self, _model): + raise RuntimeError("database unavailable") + + def close(self): + self.closed = True + + db = FailingSession() + monkeypatch.setattr(database, "SessionLocal", lambda: db) + monkeypatch.setattr( + email_pollers, + "_auto_summarize_pass_single", + lambda **_kwargs: pytest.fail( + "discovery failures must not enter the single-account path" + ), + ) + + with pytest.raises(RuntimeError, match="database unavailable"): + await email_pollers._auto_summarize_pass(owner="alice") + + assert db.closed is True + + +def test_scoped_email_config_does_not_use_deployment_legacy_credentials(monkeypatch): + """A user with no visible account must not inherit shared legacy secrets.""" + from routes import email_helpers + + Factory = _make_db() + monkeypatch.setattr(email_helpers, "_load_settings", _legacy_config) + + with mock.patch("core.database.SessionLocal", Factory): + cfg = email_helpers._get_email_config(owner="alice") + + assert cfg.get("account_id") is None + assert cfg.get("imap_host") == "" + assert cfg.get("imap_password") == "" + assert cfg.get("smtp_host") == "" + assert cfg.get("smtp_password") == "" + + +def test_unscoped_email_config_preserves_single_user_legacy_fallback(monkeypatch): + """The fail-closed guard must retain legacy config for single-user calls.""" + from routes import email_helpers + + Factory = _make_db() + monkeypatch.setattr(email_helpers, "_load_settings", _legacy_config) + + with mock.patch("core.database.SessionLocal", Factory): + cfg = email_helpers._get_email_config() + + assert cfg.get("account_id") is None + assert cfg.get("imap_host") == "legacy-imap.example.com" + assert cfg.get("imap_password") == "legacy-secret" + assert cfg.get("smtp_host") == "legacy-smtp.example.com" + assert cfg.get("smtp_password") == "legacy-secret" + + +def test_scoped_email_config_propagates_account_lookup_failure(monkeypatch): + """A database failure must not switch an owner-scoped call to legacy secrets.""" + from core import database + from routes import email_helpers + + class FailingSession: + def __init__(self): + self.closed = False + + def query(self, _model): + raise RuntimeError("database unavailable") + + def close(self): + self.closed = True + + db = FailingSession() + monkeypatch.setattr(database, "SessionLocal", lambda: db) + monkeypatch.setattr( + email_helpers, + "_load_settings", + lambda: pytest.fail("scoped lookup failures must not read legacy settings"), + ) + + with pytest.raises(RuntimeError, match="database unavailable"): + email_helpers._get_email_config(owner="alice") + + assert db.closed is True + + +def test_explicit_disabled_config_does_not_fall_back_to_default(monkeypatch): + """An explicit disabled ID must not silently resolve another account.""" + from routes import email_helpers + + Factory = _make_db() + _make_account( + Factory, + "alice-disabled", + "alice", + "alice-disabled@example.com", + enabled=False, + ) + _make_account( + Factory, + "alice-default", + "alice", + "alice-default@example.com", + is_default=True, + ) + monkeypatch.setattr(email_helpers, "_load_settings", _legacy_config) + + with mock.patch("core.database.SessionLocal", Factory): + cfg = email_helpers._get_email_config( + account_id="alice-disabled", + owner="alice", + ) + + assert cfg.get("account_id") is None + assert cfg.get("imap_host") == "" + assert cfg.get("smtp_host") == "" + + +@pytest.mark.asyncio +async def test_email_task_allows_matching_explicit_account(monkeypatch): + from routes import email_pollers + + imap_calls = [] + + class FakeImap: + def select(self, *_args, **_kwargs): + return "OK", [] + + def uid(self, *_args, **_kwargs): + return "OK", [b""] + + def logout(self): + return None + + def fake_imap_connect(account_id=None, owner=""): + imap_calls.append((account_id, owner)) + return FakeImap() + + monkeypatch.setattr( + email_pollers, + "_load_settings", + lambda: {"email_auto_summarize": True}, + ) + monkeypatch.setattr(email_pollers, "_account_visible_to_task_owner", lambda *_args: True) + monkeypatch.setattr( + email_pollers, + "_imap_connect", + fake_imap_connect, + ) + monkeypatch.setattr( + email_pollers, + "_latest_inbox_fallback_uids", + lambda conn, _reconnect: ([], conn), + ) + + result = await email_pollers._auto_summarize_pass_single( + account_id="alice-account", + owner="alice", + ) + + assert result == "No recent emails" + assert imap_calls == [("alice-account", "alice")] + + +@pytest.mark.asyncio +async def test_email_task_fanout_filters_accounts_by_owner(monkeypatch): + from core import database + from routes import email_pollers + + class FakeColumn: + def __eq__(self, _other): + return True + + def asc(self): + return self + + def desc(self): + return self + + class FakeEmailAccount: + enabled = FakeColumn() + is_default = FakeColumn() + created_at = FakeColumn() + + class Row: + def __init__(self, account_id, name, owner, imap_user): + self.id = account_id + self.name = name + self.owner = owner + self.imap_user = imap_user + self.from_address = imap_user + + rows = [ + Row("alice-primary", "Alice primary", "alice", "alice"), + Row("legacy-alice", "Legacy Alice", "", "alice"), + Row("bob-primary", "Bob primary", "bob", "bob"), + Row("legacy-other", "Legacy other", "", "other"), + Row("alice-secondary", "Alice secondary", "alice", "alice"), + ] + + class FakeQuery: + def filter(self, *_args, **_kwargs): + return self + + def order_by(self, *_args, **_kwargs): + return self + + def all(self): + return list(rows) + + class FakeDb: + def __init__(self): + self.closed = False + + def query(self, model): + assert model is FakeEmailAccount + return FakeQuery() + + def close(self): + self.closed = True + + db = FakeDb() + calls = [] + + async def fake_single(**kwargs): + calls.append((kwargs["account_id"], kwargs["owner"])) + return "processed" + + monkeypatch.setattr(database, "EmailAccount", FakeEmailAccount) + monkeypatch.setattr(database, "SessionLocal", lambda: db) + monkeypatch.setattr( + email_pollers, + "_auto_summarize_pass_single", + fake_single, + ) + + result = await email_pollers._auto_summarize_pass(owner="alice") + + assert calls == [ + ("alice-primary", "alice"), + ("legacy-alice", "alice"), + ("alice-secondary", "alice"), + ] + assert result == ( + "[Alice primary] processed\n" + "[Legacy Alice] processed\n" + "[Alice secondary] processed" + ) + assert db.closed is True + + +@pytest.mark.asyncio +async def test_email_task_explicit_accounts_use_canonical_legacy_visibility(monkeypatch): + """Explicit IDs use the same mailbox-match policy as account fan-out.""" + from routes import email_pollers + + Factory = _make_db() + _make_account(Factory, "alice-owned", "alice", "alice") + _make_account(Factory, "legacy-alice", "", "alice") + _make_account(Factory, "bob-owned", "bob", "bob") + _make_account(Factory, "legacy-other", "", "other") + + class FakeImap: + def select(self, *_args, **_kwargs): + return "OK", [] + + def uid(self, *_args, **_kwargs): + return "OK", [b""] + + def logout(self): + return None + + imap_calls = [] + + def fake_imap_connect(account_id=None, owner=""): + imap_calls.append((account_id, owner)) + return FakeImap() + + monkeypatch.setattr( + email_pollers, + "_load_settings", + lambda: {"email_auto_summarize": True}, + ) + monkeypatch.setattr(email_pollers, "_imap_connect", fake_imap_connect) + monkeypatch.setattr( + email_pollers, + "_latest_inbox_fallback_uids", + lambda conn, _reconnect: ([], conn), + ) + + with mock.patch("core.database.SessionLocal", Factory): + assert await email_pollers._auto_summarize_pass_single( + account_id="alice-owned", + owner="alice", + ) == "No recent emails" + assert await email_pollers._auto_summarize_pass_single( + account_id="legacy-alice", + owner="alice", + ) == "No recent emails" + for account_id in ("bob-owned", "legacy-other", "missing"): + with pytest.raises(PermissionError, match="not available to this task owner"): + await email_pollers._auto_summarize_pass_single( + account_id=account_id, + owner="alice", + ) + + assert imap_calls == [ + ("alice-owned", "alice"), + ("legacy-alice", "alice"), + ] + + +def test_email_task_account_authorization_propagates_database_errors(monkeypatch): + """Database failures must not be reported as ordinary access denials.""" + from core import database + from routes import email_pollers + + class FailingSession: + def __init__(self): + self.closed = False + + def query(self, _model): + raise RuntimeError("database unavailable") + + def close(self): + self.closed = True + + db = FailingSession() + monkeypatch.setattr(database, "SessionLocal", lambda: db) + + with pytest.raises(RuntimeError, match="database unavailable"): + email_pollers._account_visible_to_task_owner("alice-account", "alice") + + assert db.closed is True diff --git a/tests/test_first_run_setup_locality.py b/tests/test_first_run_setup_locality.py new file mode 100644 index 0000000000..1a451912d9 --- /dev/null +++ b/tests/test_first_run_setup_locality.py @@ -0,0 +1,85 @@ +"""Regression tests for direct-local first-run setup enforcement.""" + +from types import SimpleNamespace + +import pytest + + +def _request( + *, + client_host: str, + request_host: str, + headers: dict[str, str] | None = None, +): + return SimpleNamespace( + client=SimpleNamespace(host=client_host), + url=SimpleNamespace(hostname=request_host), + headers=headers or {}, + ) + + +def test_first_run_setup_allows_direct_localhost(): + from routes.auth_routes import _request_from_loopback + + request = _request( + client_host="127.0.0.1", + request_host="localhost", + ) + + assert _request_from_loopback(request) is True + + +@pytest.mark.parametrize( + ("client_host", "request_host", "headers"), + [ + ( + "203.0.113.44", + "localhost", + {"x-forwarded-for": "127.0.0.1"}, + ), + ( + "127.0.0.1", + "odysseus.example.com", + {"x-forwarded-for": "203.0.113.44"}, + ), + ( + "127.0.0.1", + "localhost", + {"forwarded": "for=203.0.113.44;proto=https"}, + ), + ( + "127.0.0.1", + "localhost", + {"x-real-ip": "203.0.113.44"}, + ), + ( + "127.0.0.1", + "localhost", + {"cf-connecting-ip": "203.0.113.44"}, + ), + ( + "127.0.0.1", + "localhost", + {"cf-connecting-ipv6": "2001:db8::44"}, + ), + ( + "127.0.0.1", + "localhost", + {"cf-pseudo-ipv4": "203.0.113.44"}, + ), + ], +) +def test_first_run_setup_rejects_remote_or_proxied_requests( + client_host, + request_host, + headers, +): + from routes.auth_routes import _request_from_loopback + + request = _request( + client_host=client_host, + request_host=request_host, + headers=headers, + ) + + assert _request_from_loopback(request) is False