From e115b0155c724a260c9d62e7bb58c1838a3bc620 Mon Sep 17 00:00:00 2001
From: SurprisedDuck <jannik.theiss@googlemail.com>
Date: Wed, 10 Jun 2026 14:37:26 +0200
Subject: [PATCH] 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>
---
 src/tool_security.py             | 17 ++++++++--
 tests/test_review_regressions.py | 54 ++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/src/tool_security.py b/src/tool_security.py
index 82d2c3d67..6b7bc90df 100644
--- a/src/tool_security.py
+++ b/src/tool_security.py
@@ -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:
-    """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:
         from core.auth import AuthManager
 
         auth = AuthManager()
         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))
     except Exception as exc:
         logger.warning("Unable to evaluate owner admin status: %s", exc)
diff --git a/tests/test_review_regressions.py b/tests/test_review_regressions.py
index b3988f88e..fe782f151 100644
--- a/tests/test_review_regressions.py
+++ b/tests/test_review_regressions.py
@@ -647,6 +647,60 @@ def test_public_agent_policy_hides_sensitive_tools(monkeypatch):
     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
 async def test_webhook_tool_reuses_private_url_validation():
     class FakeDb: