From b2fc888e699bcd4a3466a0aac5f8a8696e817f16 Mon Sep 17 00:00:00 2001 From: RaresKeY <158580472+RaresKeY@users.noreply.github.com> Date: Sat, 13 Jun 2026 21:16:18 -0400 Subject: [PATCH] test(auth): harden default local owner matrix --- README.md | 7 +++- THREAT_MODEL.md | 2 +- app.py | 3 +- routes/assistant_routes.py | 7 ++-- routes/research/research_routes.py | 4 +- src/task_scheduler.py | 3 +- tests/test_owner_identity.py | 39 +++++++++++++++++++ ...test_reserved_username_admin_escalation.py | 12 +----- tests/test_security_regressions.py | 27 +++++++++++++ 9 files changed, 84 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 705ec6b68..4b96330b6 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,12 @@ Help is welcome. The best entry points are fresh-install testing, provider setup ## Security -Odysseus is a self-hosted workspace with powerful local tools. Keep auth enabled, keep private data out of Git, and do not expose raw model/service ports publicly. Deployment details are in the [setup guide](docs/setup.md#security-notes). +Odysseus is a self-hosted workspace with powerful local tools. Keep auth enabled, keep private data out of Git, and do not expose raw model/service ports publicly. + +- Keep `AUTH_ENABLED=true` for any network-accessible deployment. +- Keep `LOCALHOST_BYPASS=false` outside local development. + +Deployment details are in the [setup guide](docs/setup.md#security-notes). ## Star History diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index 48665a61d..ee656087c 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -37,7 +37,7 @@ Non-admin defaults are in `core/auth.py:DEFAULT_PRIVILEGES`. Tool enforcement is - **Sessions:** bcrypt passwords, 7-day session tokens stored atomically in `data/sessions.json` via `core/atomic_io.py`. - **2FA:** TOTP with 8 single-use backup codes. Verified after password check, before session issuance. -- **Reserved usernames:** `internal-tool`, `api`, `demo`, `system` cannot be registered or renamed into. Defined in `core/auth.py:RESERVED_USERNAMES`. +- **Reserved usernames:** request sentinels and the Default/Local storage owner cannot be registered or renamed into. Defined in `core/auth.py:RESERVED_USERNAMES`. - `internal-tool` is security-critical: `core/middleware.py:require_admin` treats any request where `request.state.current_user == "internal-tool"` as the in-process tool loopback and grants admin unconditionally. A real account with that name would silently pass every `require_admin` check. - **Orphan sessions:** `validate_token` re-checks that the user record still exists on every call. A deleted user's cookie is dropped on next request rather than continuing to authenticate. diff --git a/app.py b/app.py index e740ad518..5549a907b 100644 --- a/app.py +++ b/app.py @@ -78,6 +78,7 @@ import bcrypt as _bcrypt from src.app_helpers import abs_join, serve_html_with_nonce from src.generated_images import GENERATED_IMAGE_HEADERS, resolve_generated_image_path +from src.owner_identity import auth_disabled from starlette.responses import RedirectResponse # ========= LOGGING ========= @@ -248,7 +249,7 @@ from routes.auth_routes import setup_auth_routes, SESSION_COOKIE auth_manager = AuthManager() app.state.auth_manager = auth_manager -AUTH_ENABLED = os.getenv("AUTH_ENABLED", "true").lower() != "false" +AUTH_ENABLED = not auth_disabled() LOCALHOST_BYPASS = os.getenv("LOCALHOST_BYPASS", "false").lower() == "true" if LOCALHOST_BYPASS: logger.warning("LOCALHOST_BYPASS is enabled, loopback requests bypass authentication. Do not expose this instance to a network.") diff --git a/routes/assistant_routes.py b/routes/assistant_routes.py index 0b609e37f..f16f016e9 100644 --- a/routes/assistant_routes.py +++ b/routes/assistant_routes.py @@ -16,7 +16,7 @@ from pydantic import BaseModel from core.database import SessionLocal, CrewMember, ScheduledTask from src.auth_helpers import get_current_user -from core.auth import RESERVED_USERNAMES +from src.owner_identity import REQUEST_SENTINEL_OWNERS from src.task_scheduler import compute_next_run @@ -90,11 +90,12 @@ def setup_assistant_routes(task_scheduler) -> APIRouter: # check-in tasks seeded. Hitting any /assistant route under one of these # used to seed a full CrewMember + Morning/Midday/Evening tasks under that # owner, which then double-fired alongside the real user's check-ins. - # RESERVED_USERNAMES covers the same set; the `not owner` guard handles "". + # REQUEST_SENTINEL_OWNERS covers request-only identities; Default/Local is a + # reserved login name but remains a valid storage owner. async def _get_or_create(owner: str) -> CrewMember: """Return the per-owner assistant CrewMember, creating it on demand.""" - if not owner or owner in RESERVED_USERNAMES: + if not owner or owner in REQUEST_SENTINEL_OWNERS: raise HTTPException(status_code=400, detail=f"Cannot seed assistant for {owner!r}") db = SessionLocal() try: diff --git a/routes/research/research_routes.py b/routes/research/research_routes.py index fdc650d95..905ee4b92 100644 --- a/routes/research/research_routes.py +++ b/routes/research/research_routes.py @@ -15,7 +15,7 @@ from pydantic import BaseModel, Field from core.middleware import INTERNAL_TOOL_USER from src.endpoint_resolver import resolve_endpoint from src.auth_helpers import _auth_disabled, get_current_user -from core.auth import RESERVED_USERNAMES +from src.owner_identity import REQUEST_SENTINEL_OWNERS from src.constants import DEEP_RESEARCH_DIR _SESSION_ID_RE = re.compile(r"^[a-zA-Z0-9-]{1,128}$") @@ -496,7 +496,7 @@ def setup_research_routes(research_handler, session_manager=None) -> APIRouter: user = require_privilege(request, "can_use_research") if user == INTERNAL_TOOL_USER: tool_owner = (request.headers.get("X-Odysseus-Owner") or "").strip() - if tool_owner and tool_owner not in RESERVED_USERNAMES: + if tool_owner and tool_owner not in REQUEST_SENTINEL_OWNERS: auth_mgr = getattr(request.app.state, "auth_manager", None) if auth_mgr is not None and getattr(auth_mgr, "is_configured", False): try: diff --git a/src/task_scheduler.py b/src/task_scheduler.py index d5b1dad62..30f5619ba 100644 --- a/src/task_scheduler.py +++ b/src/task_scheduler.py @@ -10,6 +10,7 @@ from datetime import datetime, timedelta, timezone from typing import Any, Awaitable, Callable, Dict, Tuple from core.auth import RESERVED_USERNAMES +from src.owner_identity import REQUEST_SENTINEL_OWNERS from src.task_action_policy import ( is_admin_only_task_action, owner_has_admin_task_privileges, @@ -2484,7 +2485,7 @@ class TaskScheduler: # check-ins seeded, which then double-fire alongside the human user's # check-ins. This was the root cause of the duplicate 'Morning check-in' # rows we had to manually clean up. - if not owner or owner in RESERVED_USERNAMES: + if not owner or owner in REQUEST_SENTINEL_OWNERS: logger.info(f"ensure_assistant_defaults: skip synthetic owner {owner!r}") return from core.database import SessionLocal, CrewMember, ScheduledTask diff --git a/tests/test_owner_identity.py b/tests/test_owner_identity.py index 1f3d159ef..8362f79eb 100644 --- a/tests/test_owner_identity.py +++ b/tests/test_owner_identity.py @@ -61,3 +61,42 @@ def test_storage_owner_for_request_uses_default_local_when_auth_disabled(monkeyp request = SimpleNamespace(state=SimpleNamespace(current_user=None)) assert storage_owner_for_request(request) == DEFAULT_LOCAL_OWNER + + +@pytest.mark.parametrize( + "value,expected", + [ + (None, False), + ("", False), + ("true", False), + ("0", False), + ("false", True), + ("FALSE", True), + (" false ", True), + ], +) +def test_auth_disabled_parser_is_centralized(monkeypatch, value, expected): + from src.owner_identity import auth_disabled + + if value is None: + monkeypatch.delenv("AUTH_ENABLED", raising=False) + else: + monkeypatch.setenv("AUTH_ENABLED", value) + + assert auth_disabled() is expected + + +def test_default_local_owner_is_reserved_auth_name_but_valid_storage_owner(): + from src.owner_identity import ( + DEFAULT_LOCAL_OWNER, + REQUEST_SENTINEL_OWNERS, + RESERVED_AUTH_USERNAMES, + effective_storage_owner, + is_default_local_owner, + ) + + assert DEFAULT_LOCAL_OWNER in RESERVED_AUTH_USERNAMES + assert DEFAULT_LOCAL_OWNER not in REQUEST_SENTINEL_OWNERS + assert effective_storage_owner(DEFAULT_LOCAL_OWNER, auth_is_disabled=False) == DEFAULT_LOCAL_OWNER + assert effective_storage_owner(DEFAULT_LOCAL_OWNER, auth_is_disabled=True) == DEFAULT_LOCAL_OWNER + assert is_default_local_owner(f" {DEFAULT_LOCAL_OWNER.upper()} ") diff --git a/tests/test_reserved_username_admin_escalation.py b/tests/test_reserved_username_admin_escalation.py index d70062581..f411ce658 100644 --- a/tests/test_reserved_username_admin_escalation.py +++ b/tests/test_reserved_username_admin_escalation.py @@ -33,17 +33,7 @@ def _fresh_auth_manager(tmp_path): @pytest.mark.parametrize( "name", - [ - "internal-tool", - "api", - "demo", - "system", - DEFAULT_LOCAL_OWNER, - "INTERNAL-TOOL", - " Internal-Tool ", - "Api", - "SYSTEM", - ], + _RESERVED_NAMES + ["INTERNAL-TOOL", " Internal-Tool ", "Api", "SYSTEM"], ) def test_create_user_rejects_reserved_usernames(tmp_path, name): mgr = _fresh_auth_manager(tmp_path) diff --git a/tests/test_security_regressions.py b/tests/test_security_regressions.py index f6a05383d..8e3f57266 100644 --- a/tests/test_security_regressions.py +++ b/tests/test_security_regressions.py @@ -131,6 +131,12 @@ def test_readme_native_quickstart_uses_loopback(): assert "0.0.0.0` only when you intentionally want" in docs +def test_readme_warns_auth_enabled_for_network_access(): + readme = Path("README.md").read_text(encoding="utf-8") + assert "Keep `AUTH_ENABLED=true` for any network-accessible deployment." in readme + assert "Keep `LOCALHOST_BYPASS=false` outside local development." in readme + + def test_ollama_cookbook_runner_does_not_force_public_bind(): route = Path("routes/cookbook_routes.py").read_text(encoding="utf-8") cookbook_js = Path("static/js/cookbook.js").read_text(encoding="utf-8") @@ -738,6 +744,27 @@ def test_require_admin_allows_when_auth_explicitly_disabled(monkeypatch): assert require_admin(_Req()) is None +def test_require_admin_uses_central_auth_disabled_parser(monkeypatch): + from core.middleware import require_admin + + monkeypatch.setenv("AUTH_ENABLED", " false ") + + class _State: + current_user = None + + class _AppState: + auth_manager = None + + class _App: + state = _AppState() + + class _Req: + state = _State() + app = _App() + + assert require_admin(_Req()) is None + + def test_internal_tool_owner_header_logic_requires_known_user(): """Pin the owner-attribution branch used by app.AuthMiddleware without booting the full FastAPI app."""