mirror of
https://github.com/pewdiepie-archdaemon/odysseus.git
synced 2026-06-17 10:15:27 -04:00
fix(security): don't grant tool access in the pre-setup window (#3506)
* fix(security): don't grant tool access in the pre-setup window owner_is_admin_or_single_user() returned True whenever auth was not configured, which conflated two very different states: - intentional single-user mode (operator set AUTH_ENABLED=false), and - the pre-setup window (auth enabled, but no admin created yet). In the second state, blocked_tools_for_owner() returned an empty set, so server-execution tools (bash/python) and other admin-only tools were ungated. The auth middleware already 401s /api/ requests pre-setup, but a caller that bypasses it (trusted loopback / internal-tool path) could reach those tools before setup completed. Treat "not configured" as admin only when auth is intentionally disabled (AUTH_ENABLED=false), mirroring the AUTH_ENABLED parsing in app.py and core.middleware. Single-user mode is preserved; the pre-setup window is now non-admin as defense-in-depth. Adds regression tests for both states. Fixes #3201 Supported by Claude Opus 4.8 * refactor(security): reuse _auth_disabled() instead of a duplicate helper Addresses review on #3506: src/auth_helpers.py already has _auth_disabled() with the identical AUTH_ENABLED parse. Drop the duplicate _auth_intentionally_disabled() and call the existing helper via a lazy import inside owner_is_admin_or_single_user (mirroring the lazy core.auth import) to avoid any import cycle. Removes the now-unused `import os`. Behaviour and the two regression tests are unchanged. Supported by Claude Opus 4.8 --------- Co-authored-by: SurprisedDuck <288741682+SurprisedDuck@users.noreply.github.com>
This commit is contained in:
+15
-2
@@ -162,13 +162,26 @@ def is_public_blocked_tool(tool_name: Optional[str]) -> bool:
|
|||||||
|
|
||||||
|
|
||||||
def owner_is_admin_or_single_user(owner: Optional[str]) -> bool:
|
def owner_is_admin_or_single_user(owner: Optional[str]) -> bool:
|
||||||
"""Return True for admins, or when auth is not configured yet."""
|
"""Return True for admins, or in intentional single-user mode.
|
||||||
|
|
||||||
|
Single-user mode means the operator explicitly disabled auth
|
||||||
|
(``AUTH_ENABLED=false``) — the local/self-host default where the owner has
|
||||||
|
full access to their own box.
|
||||||
|
|
||||||
|
The pre-setup window (auth ENABLED but no admin created yet) is treated as
|
||||||
|
NON-admin: returning True there would hand server-execution tools
|
||||||
|
(``bash``/``python``) to any caller before setup completes. The auth
|
||||||
|
middleware already 401s ``/api/`` requests pre-setup, so this is
|
||||||
|
defense-in-depth for callers that bypass it (e.g. trusted loopback).
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
from core.auth import AuthManager
|
from core.auth import AuthManager
|
||||||
|
|
||||||
auth = AuthManager()
|
auth = AuthManager()
|
||||||
if not auth.is_configured:
|
if not auth.is_configured:
|
||||||
return True
|
from src.auth_helpers import _auth_disabled
|
||||||
|
|
||||||
|
return _auth_disabled()
|
||||||
return bool(owner and auth.is_admin(owner))
|
return bool(owner and auth.is_admin(owner))
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
logger.warning("Unable to evaluate owner admin status: %s", exc)
|
logger.warning("Unable to evaluate owner admin status: %s", exc)
|
||||||
|
|||||||
@@ -647,6 +647,60 @@ def test_public_agent_policy_hides_sensitive_tools(monkeypatch):
|
|||||||
assert "manage_tasks" in blocked
|
assert "manage_tasks" in blocked
|
||||||
|
|
||||||
|
|
||||||
|
def test_presetup_does_not_grant_admin_tools_when_auth_enabled(monkeypatch):
|
||||||
|
"""Pre-setup window: auth is enabled but no admin user exists yet.
|
||||||
|
|
||||||
|
This must NOT be treated as single-user/admin at the tool layer — the
|
||||||
|
server-execution tools (bash/python) stay blocked as defense-in-depth so
|
||||||
|
an unauthenticated caller that slips past the auth middleware (e.g. via a
|
||||||
|
loopback bypass) can't reach an RCE before setup completes.
|
||||||
|
"""
|
||||||
|
monkeypatch.delenv("AUTH_ENABLED", raising=False) # default: enabled
|
||||||
|
auth_mod = _install_core_auth_stub(monkeypatch)
|
||||||
|
|
||||||
|
class FakeAuth:
|
||||||
|
is_configured = False
|
||||||
|
|
||||||
|
def is_admin(self, username):
|
||||||
|
return False
|
||||||
|
|
||||||
|
monkeypatch.setattr(auth_mod, "AuthManager", lambda: FakeAuth())
|
||||||
|
|
||||||
|
from src.tool_security import (
|
||||||
|
blocked_tools_for_owner,
|
||||||
|
owner_is_admin_or_single_user,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert owner_is_admin_or_single_user(None) is False
|
||||||
|
blocked = blocked_tools_for_owner(None)
|
||||||
|
assert "bash" in blocked
|
||||||
|
assert "python" in blocked
|
||||||
|
|
||||||
|
|
||||||
|
def test_single_user_mode_keeps_full_tool_access_when_auth_disabled(monkeypatch):
|
||||||
|
"""Intentional single-user mode (AUTH_ENABLED=false) keeps full tool
|
||||||
|
access even with no admin user — this is the default local/self-host UX
|
||||||
|
and must not regress."""
|
||||||
|
monkeypatch.setenv("AUTH_ENABLED", "false")
|
||||||
|
auth_mod = _install_core_auth_stub(monkeypatch)
|
||||||
|
|
||||||
|
class FakeAuth:
|
||||||
|
is_configured = False
|
||||||
|
|
||||||
|
def is_admin(self, username):
|
||||||
|
return False
|
||||||
|
|
||||||
|
monkeypatch.setattr(auth_mod, "AuthManager", lambda: FakeAuth())
|
||||||
|
|
||||||
|
from src.tool_security import (
|
||||||
|
blocked_tools_for_owner,
|
||||||
|
owner_is_admin_or_single_user,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert owner_is_admin_or_single_user(None) is True
|
||||||
|
assert blocked_tools_for_owner(None) == set()
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_webhook_tool_reuses_private_url_validation():
|
async def test_webhook_tool_reuses_private_url_validation():
|
||||||
class FakeDb:
|
class FakeDb:
|
||||||
|
|||||||
Reference in New Issue
Block a user