fix: prevent admin privilege escalation via signup#22
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef89774051
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if row and row[1] == "!unusable" and normalized == admin_email: | ||
| raise ValueError("email_exists") |
There was a problem hiding this comment.
This guard locks out admin bootstrap on fresh deployments: _ensure_admin_account seeds OPENCMO_ADMIN_EMAIL with password_hash='!unusable', and authenticate_user rejects unusable hashes, so the only in-app way to claim that row was signup updating the password. After this change, signup for that email always returns email_exists, leaving no API path to ever set an admin password and making /api/v1/admin/* inaccessible unless operators manually edit the DB.
Useful? React with 👍 / 👎.
* fix: validate and dedupe geo ask platforms * fix: bind docker compose port to localhost by default * fix: prevent verify-email auth bypass for verified users * fix: fail closed for account-scoped publish credentials * fix: prevent admin privilege escalation via signup * test: update admin/publisher tests for security fixes - test_publishers.py: replace env vars with llm.set_request_keys() since publish credentials no longer fall back to os.environ (account-scoped). - test_trial_platform.py: add _seed_admin() helper that activates the bootstrapped !unusable admin row directly, since signup can no longer claim that row to prevent admin privilege escalation. * test: fix ruff I001 import order
|
Incorporated via #27 → main. |
1 participant
Motivation
password_hash == "!unusable") or implicitly create the configured admin email as an admin, allowing unauthenticated privilege escalation on fresh/unclaimed deployments.Description
create_user_with_accountadded a guard that treats an existing!unusableadmin row as taken and raisesValueError("email_exists")for signups that try to claimOPENCMO_ADMIN_EMAILby matching the normalized email."user"rather than being promoted to"admin"when they matchOPENCMO_ADMIN_EMAIL.src/opencmo/storage/accounts.pyand preserves normal signup/account creation and the explicit bootstrap logic that runs at DB init.Testing
pytest tests/test_trial_platform.py::test_signup_login_me_and_logoutandpytest tests/test_trial_platform.py::test_admin_summary_requires_admin tests/test_trial_platform.py::test_admin_account_actions_update_and_disable_access, but the test runs failed to collect/execute because the environment in this container is missing runtime dependencies (aiosqlite) and the test runner could not import theopencmopackage.src/opencmo/storage/accounts.py.Codex Task