security: 5 vulnerability fixes (#20, #21, #22, #23, #25)

docker-compose.yml

@@ -2,7 +2,7 @@ services:
   opencmo:
     build: .
     ports:
-      - "8080:8080"
+      - "127.0.0.1:8080:8080"
     volumes:
       - opencmo_data:/data
     env_file:

src/opencmo/llm.py

@@ -63,6 +63,16 @@
     "OPENAI_BASE_URL",
     "OPENCMO_MODEL_DEFAULT",
 })
+_ACCOUNT_SCOPED_SECRET_KEYS = frozenset({
+    "REDDIT_CLIENT_ID",
+    "REDDIT_CLIENT_SECRET",
+    "REDDIT_USERNAME",
+    "REDDIT_PASSWORD",
+    "TWITTER_API_KEY",
+    "TWITTER_API_SECRET",
+    "TWITTER_ACCESS_TOKEN",
+    "TWITTER_ACCESS_SECRET",
+})
 
 # ---------------------------------------------------------------------------
 # ContextVar — per-request key isolation (asyncio Task-local)
@@ -178,7 +188,12 @@ def get_key(name: str, default: str | None = None) -> str | None:
     if val:
         return val
 
-    # 3. os.environ
+    # 3. Sensitive account-scoped secrets must fail closed to avoid
+    # cross-account credential bleed through process-global env.
+    if name in _ACCOUNT_SCOPED_SECRET_KEYS:
+        return default
+
+    # 4. os.environ
     val = os.environ.get(name)
     if val:
         return val
@@ -222,13 +237,17 @@ async def get_key_async(name: str, default: str | None = None) -> str | None:
         except Exception:
             pass
 
-    # 4. For core router defaults, prefer env/.env over persisted DB settings.
+    # 4. Sensitive account-scoped secrets must not fall through to system/env.
+    if name in _ACCOUNT_SCOPED_SECRET_KEYS:
+        return default
+
+    # 5. For core router defaults, prefer env/.env over persisted DB settings.
     if name in _ENV_PRIORITY_KEYS:
         val = os.environ.get(name)
         if val:
             return val
 
-    # 5. System fallback (admin account → legacy settings table)
+    # 6. System fallback (admin account → legacy settings table)
     try:
         from opencmo import storage
         val = await storage.get_system_setting(name)
@@ -237,7 +256,7 @@ async def get_key_async(name: str, default: str | None = None) -> str | None:
     except Exception:
         pass  # DB may not be initialized yet
 
-    # 6. os.environ
+    # 7. os.environ
     val = os.environ.get(name)
     if val:
         return val

src/opencmo/storage/accounts.py

@@ -177,6 +177,7 @@ async def get_user_account(user_id: int) -> dict | None:
 
 async def create_user_with_account(email: str, password: str, name: str = "") -> tuple[dict, dict]:
     normalized = (email or "").strip().lower()
+    admin_email = os.environ.get("OPENCMO_ADMIN_EMAIL", "hello@aidcmo.com").strip().lower()
     if not is_valid_email(normalized):
         raise ValueError("invalid_email")
     if len(password or "") < MIN_PASSWORD_LENGTH:
@@ -189,6 +190,8 @@ async def create_user_with_account(email: str, password: str, name: str = "") ->
     try:
         existing = await db.execute("SELECT id, password_hash FROM users WHERE email = ?", (normalized,))
         row = await existing.fetchone()
+        if row and row[1] == "!unusable" and normalized == admin_email:
+            raise ValueError("email_exists")
         if row and row[1] != "!unusable":
             raise ValueError("email_exists")
 
@@ -206,7 +209,7 @@ async def create_user_with_account(email: str, password: str, name: str = "") ->
                     normalized,
                     hash_password(password),
                     name.strip(),
-                    "admin" if normalized == os.environ.get("OPENCMO_ADMIN_EMAIL", "hello@aidcmo.com").strip().lower() else "user",
+                    "user",
                 ),
             )
             user_id = int(cursor.lastrowid)

src/opencmo/tools/geo_ask.py

@@ -50,10 +50,19 @@ def _select_providers(platform_names: list[str] | None) -> tuple[list[GeoProvide
     case_map = {p.name.lower(): p for p in enabled}
     selected: list[GeoProvider] = []
     unknown: list[str] = []
+    seen: set[str] = set()
     for name in platform_names:
-        prov = enabled_by_name.get(name) or case_map.get(name.lower())
+        if not isinstance(name, str):
+            unknown.append(str(name))
+            continue
+        normalized = name.strip()
+        key = normalized.lower()
+        if not normalized or key in seen:
+            continue
+        seen.add(key)
+        prov = enabled_by_name.get(normalized) or case_map.get(key)
         if prov is None:
-            unknown.append(name)
+            unknown.append(normalized)
         else:
             selected.append(prov)
     return selected, unknown

src/opencmo/web/app.py

@@ -1558,11 +1558,7 @@ async def api_v1_auth_verify_email(payload: _AuthVerifyEmailRequest, request: Re
         return JSONResponse({"ok": False, "error": "user_not_found"}, status_code=404)
 
     if await storage.is_user_verified(payload.user_id):
-        # Idempotent: already verified -> just sign them in.
-        account = await storage.get_user_account(payload.user_id)
-        if account is None or account["status"] != "active":
-            return JSONResponse({"ok": False, "error": "account_unavailable"}, status_code=403)
-        return await _json_with_session(request, user, account)
+        return JSONResponse({"ok": False, "error": "already_verified"}, status_code=400)
 
     result = await storage.consume_verification_code(payload.user_id, payload.code, purpose="signup")
     if not result.get("ok"):

src/opencmo/web/routers/projects.py

@@ -400,8 +400,23 @@ async def api_v1_geo_ask(project_id: int, request: Request):
         return JSONResponse({"error": "query too long (max 500 chars)"}, status_code=400)
 
     platforms = body.get("platforms")
-    if platforms is not None and not isinstance(platforms, list):
-        return JSONResponse({"error": "platforms must be a list of strings"}, status_code=400)
+    if platforms is not None:
+        if not isinstance(platforms, list):
+            return JSONResponse({"error": "platforms must be a list of strings"}, status_code=400)
+        if len(platforms) > 20:
+            return JSONResponse({"error": "platforms too long (max 20 entries)"}, status_code=400)
+        if any(not isinstance(name, str) or not name.strip() for name in platforms):
+            return JSONResponse({"error": "platforms must be a list of non-empty strings"}, status_code=400)
+        # Collapse duplicate provider names (case-insensitive) to prevent fan-out amplification.
+        deduped: list[str] = []
+        seen: set[str] = set()
+        for name in platforms:
+            key = name.strip().lower()
+            if key in seen:
+                continue
+            seen.add(key)
+            deduped.append(name.strip())
+        platforms = deduped
 
     from dataclasses import asdict
 

tests/test_email_verification.py

@@ -205,6 +205,31 @@ def test_verified_user_login_succeeds(verification_db):
         assert login.json()["authenticated"] is True
 
 
+def test_verify_email_already_verified_requires_login_flow(verification_db):
+    with TestClient(app) as client:
+        signup = _signup(client, "already@example.test")
+        user_id = signup["user_id"]
+        code = _last_code_for("already@example.test")
+
+        first = client.post(
+            "/api/v1/auth/verify-email",
+            json={"user_id": user_id, "code": code},
+        )
+        assert first.status_code == 200, first.text
+        assert first.json()["authenticated"] is True
+
+        client.post("/api/v1/auth/logout")
+        client.cookies.clear()
+
+        second = client.post(
+            "/api/v1/auth/verify-email",
+            json={"user_id": user_id, "code": "000000"},
+        )
+        assert second.status_code == 400, second.text
+        assert second.json()["error"] == "already_verified"
+        assert "opencmo_session" not in client.cookies
+
+
 def test_existing_legacy_users_remain_verified_after_backfill(tmp_path, monkeypatch):
     """A user created before this feature should keep working — backfill runs
     once at ensure_db() startup."""

tests/test_geo_ask.py

@@ -201,6 +201,20 @@ def test_select_providers_reports_unknown(stub_registry):
     assert unknown == ["Nope"]
 
 
+
+
+def test_select_providers_deduplicates_case_insensitive(stub_registry):
+    stub_registry.append(_StubProvider("StubA", result=_make_result("StubA")))
+    selected, unknown = _select_providers(["StubA", "stuba", " StubA "])
+    assert [p.name for p in selected] == ["StubA"]
+    assert unknown == []
+
+
+def test_select_providers_non_string_is_unknown(stub_registry):
+    stub_registry.append(_StubProvider("StubA", result=_make_result("StubA")))
+    selected, unknown = _select_providers(["StubA", 123])
+    assert [p.name for p in selected] == ["StubA"]
+    assert unknown == ["123"]
 def test_list_available_platforms_reports_status(stub_registry):
     stub_registry.append(_StubProvider("StubA", result=_make_result("StubA")))
     items = list_available_platforms()
@@ -318,3 +332,39 @@ def test_geo_ask_endpoint_platforms_must_be_list(client):
         json={"query": "hi", "platforms": "not-a-list"},
     )
     assert resp.status_code == 400
+
+
+def test_geo_ask_endpoint_platforms_max_entries(client):
+    pid = _seed_project()
+    resp = client.post(
+        f"/api/v1/projects/{pid}/geo/ask",
+        json={"query": "hi", "platforms": ["Perplexity"] * 21},
+    )
+    assert resp.status_code == 400
+    assert "max 20" in resp.json()["error"]
+
+
+def test_geo_ask_endpoint_platforms_must_be_non_empty_strings(client):
+    pid = _seed_project()
+    resp = client.post(
+        f"/api/v1/projects/{pid}/geo/ask",
+        json={"query": "hi", "platforms": ["Perplexity", "", 123]},
+    )
+    assert resp.status_code == 400
+    assert "non-empty strings" in resp.json()["error"]
+
+
+def test_geo_ask_endpoint_platforms_deduped_before_call(client):
+    pid = _seed_project()
+    fake_response = GeoAskResponse(query="hi", results=[], total_duration_ms=1, query_lang="en")
+    with patch(
+        "opencmo.tools.geo_ask.ask_platforms",
+        new=AsyncMock(return_value=fake_response),
+    ) as ask_mock:
+        resp = client.post(
+            f"/api/v1/projects/{pid}/geo/ask",
+            json={"query": "hi", "platforms": ["Perplexity", "perplexity", "  Perplexity  "]},
+        )
+    assert resp.status_code == 200
+    assert ask_mock.await_count == 1
+    assert ask_mock.await_args.kwargs["platform_names"] == ["Perplexity"]