Reserve internal sentinel usernames
`core.middleware.require_admin` grants admin to any request whose `request.state.current_user == "internal-tool"` — the sentinel meant only for the in-process tool-loopback path. But the normal cookie auth path (app.py) sets `current_user` to the raw username, and neither `create_user` nor the signup route reserved that name. As a result an account literally named "internal-tool" was silently treated as admin by every `require_admin`-gated route. With self-service signup enabled this is an anonymous -> admin privilege escalation. Reserve the full synthetic-owner set the codebase already special-cases — "internal-tool", "api", "demo", "system" (see `_SYNTHETIC_OWNERS` in routes/assistant_routes.py and the matching guards in src/task_scheduler.py and routes/research_routes.py). "api" collides with the bearer-token owner sentinel; "demo"/"system" would leave a real account denied an assistant and inconsistently owner-scoped. Refuse to create or rename into any reserved name (case/space-normalized), and reject empty usernames while we're here. Adds a regression test. Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
24
core/auth.py
24
core/auth.py
@@ -40,6 +40,22 @@ DEFAULT_AUTH_PATH = os.path.join(
|
||||
)
|
||||
TOKEN_TTL = 60 * 60 * 24 * 7 # 7 days
|
||||
|
||||
# Usernames the auth + middleware layer reserve as internal "synthetic owner"
|
||||
# sentinels; they must never belong to a real account. The most dangerous is
|
||||
# "internal-tool": `core.middleware.require_admin` treats any request whose
|
||||
# `current_user == "internal-tool"` as the in-process tool loopback and grants
|
||||
# admin, and because the cookie auth path sets `current_user` to the raw
|
||||
# username, an account literally named "internal-tool" would be silently
|
||||
# treated as an admin by every `require_admin`-gated route. "api" collides with
|
||||
# the bearer-token owner-attribution sentinel. "demo"/"system" round out the
|
||||
# synthetic-owner set the rest of the codebase already special-cases (see
|
||||
# `_SYNTHETIC_OWNERS` in routes/assistant_routes.py and the matching guards in
|
||||
# src/task_scheduler.py / routes/research_routes.py) — a real account with one
|
||||
# of those names would be denied an assistant and inconsistently owner-scoped.
|
||||
# Refuse to create or rename into any of them so the sentinels can't be
|
||||
# impersonated. (Keep this in sync with that synthetic-owner set.)
|
||||
RESERVED_USERNAMES = frozenset({"internal-tool", "api", "demo", "system"})
|
||||
|
||||
|
||||
def _hash_password(password: str) -> str:
|
||||
return bcrypt.hashpw(password.encode("utf-8"), bcrypt.gensalt()).decode("utf-8")
|
||||
@@ -177,6 +193,11 @@ class AuthManager:
|
||||
def create_user(self, username: str, password: str, is_admin: bool = False) -> bool:
|
||||
"""Create a new user account."""
|
||||
username = username.strip().lower()
|
||||
if not username:
|
||||
return False
|
||||
if username in RESERVED_USERNAMES:
|
||||
logger.warning("Refused to create reserved username '%s'", username)
|
||||
return False
|
||||
if username in self.users:
|
||||
return False
|
||||
if "users" not in self._config:
|
||||
@@ -230,6 +251,9 @@ class AuthManager:
|
||||
requesting_user = (requesting_user or "").strip().lower()
|
||||
if not old_username or not new_username:
|
||||
return False
|
||||
if new_username in RESERVED_USERNAMES:
|
||||
logger.warning("Refused to rename '%s' into reserved username '%s'", old_username, new_username)
|
||||
return False
|
||||
if old_username not in self.users:
|
||||
return False
|
||||
if new_username in self.users:
|
||||
|
||||
Reference in New Issue
Block a user