fix(security)!: enforce real scopes and tenancy (#898, P0.4)

CLAUDE.md

@@ -54,7 +54,9 @@ Issue **import + traceability** (#565) is **complete**: `POST /api/v2/integratio
 
 **Phase 3.5C is complete** — `CaptureGlitchModal` form (description/markdown, source, scope, gate obligations, severity, expiry) reachable from the PROOF9 page and the persistent sidebar "Capture Glitch" button. REQ detail view (`/proof/[req_id]`) ships markdown description rendering, `ProofScope` metadata display, obligations table with `Latest Run` column, sortable/filterable evidence history, and empty-state CTA. Backend: `ScopeOut` model on `RequirementResponse`. Issues #568, #569.
 
-**v2 API auth enforcement (#336) is complete** — all 22 v2 REST routers require auth (`require_auth`: JWT Bearer or `X-API-Key`) via router-level dependencies in `server.py`; env-gated `CODEFRAME_AUTH_REQUIRED` (default **ON**; set `false` for local dev — read at request time). Streams never carry the JWT in the URL (#745): an authenticated `POST /auth/stream-ticket` (write scope; `has_scope`, so admin implies write) mints a 60s **single-use** ticket (`codeframe/auth/stream_tickets.py`, in-process store — multi-worker caveat documented in the module), redeemed as `?ticket=` **only** on the two SSE routes (allowlist `_QUERY_TICKET_PATHS` in `codeframe/auth/dependencies.py`) and by `authenticate_websocket` for the two WS routes; `?token=<JWT>` is no longer accepted anywhere. Frontend fetches a fresh ticket per (re)connect via `fetchStreamTicket()`/`withStreamTicket()`; `useEventSource`/`useTerminalSocket` take an async `buildUrl` re-resolved on every retry. `/auth/register` admits only the bootstrap first user (403 after; seeded `!DISABLED!` admin excluded; in-process lock closes the TOCTOU window). Web UI: `/login` page (sign-in + create-first-account), proactive client-side auth guard in `AppLayout` (token-present → allow; no token → `checkAuthAccess` probe → allow only on explicit 2xx, else fail closed to `/login`; #651, #783), axios Bearer interceptor for reactive 401→`/login` redirect, SSE/WS hooks probe the `require_auth`-gated `/api/v2/settings/keys` (which respects `CODEFRAME_AUTH_REQUIRED`) on stream failure to catch token expiry (#651), SSE hooks append a fresh stream ticket, sidebar logout; `/auth/*` proxied in `next.config.js`. Backend tests run auth-off via root `tests/conftest.py` `setdefault`; `tests/ui/test_v2_auth_enforcement.py` opts back in.
+**v2 API auth enforcement (#336) is complete** — all 22 v2 REST routers require auth (`require_auth`: JWT Bearer or `X-API-Key`) via router-level dependencies in `server.py`; env-gated `CODEFRAME_AUTH_REQUIRED` (default **ON**; set `false` for local dev — read at request time). Streams never carry the JWT in the URL (#745): an authenticated `POST /auth/stream-ticket` (write scope; `has_scope`, so admin implies write) mints a 60s **single-use** ticket (`codeframe/auth/stream_tickets.py`, in-process store — multi-worker caveat documented in the module), redeemed as `?ticket=` **only** on the two SSE routes (allowlist `_QUERY_TICKET_PATHS` in `codeframe/auth/dependencies.py`) and by `authenticate_websocket` for the two WS routes; `?token=<JWT>` is no longer accepted anywhere. Frontend fetches a fresh ticket per (re)connect via `fetchStreamTicket()`/`withStreamTicket()`; `useEventSource`/`useTerminalSocket` take an async `buildUrl` re-resolved on every retry. `/auth/register` admits only the bootstrap first user (403 after; seeded `!DISABLED!` admin excluded; in-process lock closes the TOCTOU window), and `UserManager.on_after_register` promotes that sole account to `is_superuser`.
+
+**Scopes are real, not decorative (#898)** — a JWT principal's scopes now derive from its user row: `[read, write]` always, plus `admin` only when `is_superuser`. So `require_scope(SCOPE_ADMIN)` (credential storage, GitHub PAT storage, PR merge) genuinely refuses a non-superuser browser session. Corollaries: only a superuser may mint an `admin`-scoped API key (`api_key_router.create_api_key`, else any user could self-escalate); the API-key router carries a router-level `require_method_scope` so a read-scope key can no longer DELETE keys; `SchemaManager._ensure_bootstrap_superuser` backfills admin to the earliest login-capable account when an upgraded instance has none (otherwise the change silently strips the operator's admin). The auth-disabled synthetic principal is unchanged — it still carries all scopes, being the single-operator local opt-out. Workspace registry ownership is write-once in the same change: `upsert` never reassigns a non-NULL `owner_user_id`, so user B cannot take over user A's `repo_path` by re-registering it. Web UI: `/login` page (sign-in + create-first-account), proactive client-side auth guard in `AppLayout` (token-present → allow; no token → `checkAuthAccess` probe → allow only on explicit 2xx, else fail closed to `/login`; #651, #783), axios Bearer interceptor for reactive 401→`/login` redirect, SSE/WS hooks probe the `require_auth`-gated `/api/v2/settings/keys` (which respects `CODEFRAME_AUTH_REQUIRED`) on stream failure to catch token expiry (#651), SSE hooks append a fresh stream ticket, sidebar logout; `/auth/*` proxied in `next.config.js`. Backend tests run auth-off via root `tests/conftest.py` `setdefault`; `tests/ui/test_v2_auth_enforcement.py` opts back in.
 
 **PROOF9 merge gate is enforced (#731)** — `POST /api/v2/pr/{n}/merge` blocks while open (non-waived) requirements exist (409 with a blocking-requirement summary; ledger failure → explicit 500, never a silent pass-through). Explicit bypass: `override: true` + `override_reason` in `MergePRRequest`; the override is persisted to the `pr_merge_overrides` ledger table (actor from `require_auth`, reason, bypassed requirements, timestamp) and surfaced as `merge_override` on `GET /api/v2/pr/history` items. `cf pr merge` enforces the same gate on the cwd workspace (`--override --reason "..."` to bypass, same audit record; no workspace in cwd → no gate). Scope is workspace-global open requirements — the same condition as the frontend `canMerge` in `PRStatusPanel`. Known follow-ups (not blockers): per-branch scope filtering, `vacuous_pass` distinction (#556), and no frontend override UI yet (API/CLI-only).
 

codeframe/auth/api_key_router.py

@@ -15,10 +15,15 @@
 from fastapi import APIRouter, Depends, HTTPException, Request, status
 from pydantic import BaseModel, field_validator
 
-from codeframe.auth.dependencies import get_current_user, require_auth
+from codeframe.auth.dependencies import (
+    get_current_user,
+    require_auth,
+    require_method_scope,
+)
 from codeframe.auth.models import User
 from codeframe.auth.api_keys import (
     validate_scopes,
+    SCOPE_ADMIN,
     SCOPE_READ,
     SCOPE_WRITE,
 )
@@ -27,7 +32,15 @@
 
 logger = logging.getLogger(__name__)
 
-router = APIRouter(prefix="/api/auth/api-keys", tags=["auth", "api-keys"])
+# The method-scope guard applies to the whole router (#898). Without it this
+# router proved authentication only, so a scopes:["read"] key could DELETE its
+# owner's write/admin keys — a mutation from a credential that proved read
+# access. GET needs read; POST/DELETE need write.
+router = APIRouter(
+    prefix="/api/auth/api-keys",
+    tags=["auth", "api-keys"],
+    dependencies=[Depends(require_method_scope)],
+)
 
 
 # =============================================================================
@@ -151,12 +164,25 @@ async def create_api_key(
     The full API key is returned only once. Store it securely - it cannot
     be retrieved again.
 
+    A key can never grant more than its creator holds: only an ``is_superuser``
+    account may mint an ``admin``-scoped key (#898). Otherwise the new
+    JWT-scope derivation would be a formality — any signed-in user could issue
+    themselves an admin key and use it to store credentials or merge PRs.
+
     Args:
         body: API key configuration (name, scopes, optional expiration)
 
     Returns:
         Created API key details including the full key (shown once)
     """
+    # getattr, matching require_auth: a principal lacking the column must fail
+    # closed to 403, never raise into a 500 that leaves the route ungated.
+    if SCOPE_ADMIN in body.scopes and not getattr(current_user, "is_superuser", False):
+        raise HTTPException(
+            status_code=status.HTTP_403_FORBIDDEN,
+            detail="Only an admin account may create an admin-scoped API key",
+        )
+
     service = get_api_key_service(request)
 
     result = service.create_api_key(

codeframe/auth/dependencies.py

@@ -4,8 +4,9 @@
 - JWT Bearer tokens (existing FastAPI Users integration)
 - API keys via X-API-Key header (new for programmatic access)
 
-API keys use scope-based permissions (read, write, admin).
-JWT tokens get full permissions for backward compatibility.
+Both credential types use scope-based permissions (read, write, admin). API-key
+scopes are stored on the key; JWT scopes are derived from the user record —
+read+write always, admin only for ``is_superuser`` accounts (issue #898).
 """
 
 import logging
@@ -256,6 +257,43 @@ async def get_current_user_optional(
 # =============================================================================
 
 
+def _scopes_within_owner_grant(db: Any, key_record: Dict[str, Any]) -> list:
+    """Clamp a key's stored scopes to what its owner currently holds (#898).
+
+    ``admin`` is the only scope tied to the user record, so this drops it when
+    the owning account is not ``is_superuser``. Enforcing it here rather than at
+    creation alone makes the invariant *continuous*: it also covers admin keys
+    persisted before the creation guard existed, and a key whose owner is later
+    demoted — neither of which a one-time migration would keep true.
+
+    Fails closed: a missing user row or an unreadable ``is_superuser`` drops
+    admin rather than granting it.
+    """
+    scopes = list(key_record.get("scopes") or [])
+    if SCOPE_ADMIN not in scopes:
+        return scopes
+
+    try:
+        row = db.conn.execute(
+            "SELECT is_superuser FROM users WHERE id = ?", (key_record["user_id"],)
+        ).fetchone()
+        is_superuser = bool(row[0]) if row is not None else False
+    except Exception as e:
+        logger.error(f"Could not verify API key owner's admin status: {e}")
+        is_superuser = False
+
+    if is_superuser:
+        return scopes
+
+    logger.warning(
+        "API key %s carries admin scope but its owner (user_id=%s) is not a "
+        "superuser; admin dropped for this request (#898).",
+        key_record.get("id"),
+        key_record.get("user_id"),
+    )
+    return [s for s in scopes if s != SCOPE_ADMIN]
+
+
 async def get_api_key_auth(
     api_key: Optional[str] = Security(api_key_header),
     request: Request = None,
@@ -320,7 +358,7 @@ async def get_api_key_auth(
         return {
             "type": "api_key",
             "user_id": key_record["user_id"],
-            "scopes": key_record["scopes"],
+            "scopes": _scopes_within_owner_grant(db, key_record),
             "key_id": key_record["id"],
         }
 
@@ -377,11 +415,18 @@ def _resolve(principal: Dict[str, Any]) -> Dict[str, Any]:
 
     # Fall back to JWT
     if jwt_user is not None:
+        # Scopes come from the user record (#898). Handing every session
+        # [read, write, admin] made require_scope(SCOPE_ADMIN) decorative:
+        # anyone with a browser token could store credentials and merge PRs.
+        # getattr, not attribute access: a principal that somehow lacks the
+        # column must fail closed to non-admin rather than 500.
+        scopes = [SCOPE_READ, SCOPE_WRITE]
+        if getattr(jwt_user, "is_superuser", False):
+            scopes.append(SCOPE_ADMIN)
         return _resolve({
             "type": "jwt",
             "user_id": jwt_user.id,
-            # JWT users get all scopes for backward compatibility
-            "scopes": [SCOPE_READ, SCOPE_WRITE, SCOPE_ADMIN],
+            "scopes": scopes,
             "user": jwt_user,
         })
 
@@ -414,9 +459,10 @@ async def require_method_scope(
 
     Safe methods (GET/HEAD/OPTIONS) require the ``read`` scope; mutating
     methods (POST/PUT/PATCH/DELETE) require ``write``. Admin-only routes layer
-    their own ``Depends(require_scope("admin"))`` on top. JWT principals and the
-    auth-disabled synthetic principal both carry all scopes, so this only
-    constrains scoped API keys — a read-only key can no longer mutate state.
+    their own ``Depends(require_scope("admin"))`` on top. JWT principals carry
+    read+write (admin only when ``is_superuser``, #898) and the auth-disabled
+    synthetic principal carries all scopes, so in practice this guard constrains
+    scoped API keys — a read-only key can no longer mutate state.
     """
     required = SCOPE_READ if request.method.upper() in _SAFE_METHODS else SCOPE_WRITE
     if not has_scope(auth, required):

codeframe/auth/manager.py

@@ -15,6 +15,14 @@
 
 from codeframe.auth.models import User
 
+# Re-exported, never redefined. The placeholder password for the seeded
+# bootstrap admin (id=1) is owned by the layer that writes it
+# (SchemaManager._ensure_default_admin_user). Two independent copies would have
+# to stay byte-identical forever: the registration gate, the bootstrap
+# promotion and the admin backfill all compare against it, so any drift makes
+# fresh deploys unclaimable and silently strips an upgraded deploy's admin.
+from codeframe.platform_store.schema_manager import DISABLED_PASSWORD
+
 logger = logging.getLogger(__name__)
 
 # Get configuration from environment
@@ -167,6 +175,59 @@ async def on_after_register(self, user: User, request: Optional[Request] = None)
             "User registered",
             extra={"user_id": user.id, "email": user.email}
         )
+        await self._promote_if_bootstrap_user(user)
+
+    async def _promote_if_bootstrap_user(self, user: User) -> None:
+        """Grant ``is_superuser`` to the instance's first real account (#898).
+
+        Admin scope derives from ``is_superuser``, and
+        ``fastapi_users.get_register_router`` forces the field to ``False`` on
+        every registration — so without this no principal would ever hold admin
+        and credential storage, GitHub PAT storage and PR merge would be
+        permanently 403.
+
+        ``/auth/register`` already admits exactly one login-capable account
+        (issues #336, #897), so that account is the operator. Two guards, both
+        evaluated *inside* a *single atomic UPDATE* rather than read-then-write:
+
+        - this must be the only login-capable account, and
+        - no login-capable superuser may exist yet.
+
+        Atomicity matters because ``_register_lock`` is an ``asyncio.Lock`` —
+        in-process only, so it does not serialize across uvicorn/gunicorn
+        workers. With count-then-write, two racing first-time registrations can
+        each observe two users and neither promote, leaving the instance with
+        zero admins and no in-product way back. Letting the database arbitrate
+        removes that window.
+        """
+        from sqlalchemy import text
+
+        session = getattr(self.user_db, "session", None)
+        if session is None:  # pragma: no cover - non-SQLAlchemy user_db
+            return
+
+        result = await session.execute(
+            text(
+                "UPDATE users SET is_superuser = 1 "
+                "WHERE id = :uid"
+                "  AND (SELECT COUNT(*) FROM users"
+                "       WHERE hashed_password != :disabled) = 1"
+                "  AND NOT EXISTS ("
+                "      SELECT 1 FROM users"
+                "      WHERE is_superuser = 1 AND hashed_password != :disabled"
+                "  )"
+            ),
+            {"uid": user.id, "disabled": DISABLED_PASSWORD},
+        )
+        await session.commit()
+
+        if result.rowcount:
+            # Keep the in-memory row consistent with what was just written, so
+            # the registration response does not report is_superuser=false.
+            await session.refresh(user)
+            logger.info(
+                "Promoted bootstrap user to superuser", extra={"user_id": user.id}
+            )
 
     async def on_after_login(
         self, user: User, request: Optional[Request] = None, response=None

codeframe/auth/router.py

@@ -11,7 +11,12 @@
 from sqlalchemy import func, select
 
 from codeframe.auth.schemas import UserCreate, UserRead, UserUpdate
-from codeframe.auth.manager import auth_backend, fastapi_users, get_async_session_maker
+from codeframe.auth.manager import (
+    DISABLED_PASSWORD,
+    auth_backend,
+    fastapi_users,
+    get_async_session_maker,
+)
 from codeframe.auth.models import User
 from codeframe.auth.api_key_router import router as api_key_router
 from codeframe.auth.dependencies import require_auth
@@ -30,11 +35,10 @@ class StreamTicketResponse(BaseModel):
     expires_in: int
 
 
-# Placeholder password for the seeded bootstrap admin (id=1). It cannot match
-# any bcrypt hash, so that account can never log in. It is therefore NOT a real
-# account and does not close the registration window. See SchemaManager
-# ._ensure_default_admin_user.
-_DISABLED_PASSWORD = "!DISABLED!"
+# ``DISABLED_PASSWORD`` (imported from auth.manager) is the placeholder password
+# for the seeded bootstrap admin (id=1). It cannot match any bcrypt hash, so that
+# account can never log in. It is therefore NOT a real account and does not close
+# the registration window. See SchemaManager._ensure_default_admin_user.
 
 # Serializes the bootstrap registration check-then-create window. The count
 # check (here) and the user INSERT (fastapi-users route handler) run in
@@ -198,7 +202,7 @@ async def allow_registration(request: Request):
             result = await session.execute(
                 select(func.count())
                 .select_from(User)
-                .where(User.hashed_password != _DISABLED_PASSWORD)
+                .where(User.hashed_password != DISABLED_PASSWORD)
             )
             real_user_count = result.scalar_one()