RTOP-315: Fix TOCTOU race in first-user bootstrap (/api/create-user) and reconcile stale RBAC docs

docs/SECURITY.md

@@ -288,6 +288,10 @@ docker run -p 8443:8443 ...            # All interfaces
 - User accounts
 - Hashed passwords
 - Session data
+- `bootstrap_marker`: one-row sentinel inserted alongside the first admin
+  so a lost `users` table cannot reopen the first-user bootstrap window.
+  Full recovery of first-user bootstrap therefore requires removing the
+  entire `restapi.db` file, not clearing individual tables.
 
 **Protection:**
 - File system permissions
@@ -423,7 +427,6 @@ Planned security enhancements:
 - Database encryption at rest
 - Certificate management UI
 - Two-factor authentication
-- Role-based access control
 
 ## Compliance Considerations
 

docs/TROUBLESHOOTING.md

@@ -198,11 +198,16 @@ The OpenPLC Editor handles self-signed certificates automatically. If you're usi
    ```bash
    ls -la /var/run/runtime/restapi.db
    ```
-2. Reset database (WARNING: deletes all users):
+2. Reset database (WARNING: deletes all users and reopens the first-user
+   bootstrap window):
    ```bash
    sudo rm /var/run/runtime/restapi.db
    sudo ./start_openplc.sh
    ```
+   The runtime holds a `bootstrap_marker` row inside `restapi.db` to prevent
+   a second admin being created once the first exists. Clearing individual
+   tables will leave this sentinel in place and keep bootstrap closed;
+   full recovery requires removing the entire `restapi.db` file.
 3. Check .env file exists:
    ```bash
    ls -la /var/run/runtime/.env

tests/pytest/restapi/test_create_user_race.py

@@ -0,0 +1,103 @@
+# SPDX-License-Identifier: MIT
+# Copyright (c) 2026 Autonomy®
+
+"""Regression test for the first-admin bootstrap TOCTOU race.
+
+Before the fix, N concurrent POST /api/create-user calls on an empty database
+all passed the User.query.first() check and all committed with role=admin.
+The fix serializes the bootstrap branch with a threading.Lock, re-checks
+inside the lock, and writes a UNIQUE sentinel row in the same transaction
+as the first user. This test fires N concurrent requests and asserts exactly
+one admin lands.
+"""
+
+import threading
+
+from webserver import restapi
+
+
+def _race_create_user(app, n: int) -> list[tuple[str, int]]:
+    """Fire ``n`` concurrent create-user POSTs, released by a shared barrier.
+
+    Each thread owns its own test client. The barrier lines every thread up
+    before the simultaneous release so the contention window is as tight as
+    the host can produce.
+    """
+    results: list[tuple[str, int]] = []
+    results_lock = threading.Lock()
+    release = threading.Barrier(n)
+
+    def post(index: int) -> None:
+        username = f"racer{index}"
+        client = app.test_client()
+        release.wait(timeout=10)
+        resp = client.post(
+            "/api/create-user",
+            json={"username": username, "password": "race-test-pw-12"},
+        )
+        with results_lock:
+            results.append((username, resp.status_code))
+
+    threads = [threading.Thread(target=post, args=(i,)) for i in range(n)]
+    for t in threads:
+        t.start()
+    for t in threads:
+        t.join()
+    return results
+
+
+def test_concurrent_bootstrap_creates_exactly_one_admin(app):
+    """Five concurrent POSTs on an empty DB produce one admin and four refusals."""
+    try:
+        results = _race_create_user(app, n=5)
+
+        successes = [r for r in results if r[1] == 201]
+        refusals = [r for r in results if r[1] != 201]
+
+        assert len(successes) == 1, (
+            f"expected exactly one 201, got {len(successes)} out of {len(results)}: {results}"
+        )
+        # Losing racers see "a user exists" inside the lock and return 401. A
+        # lock bypass that reached the commit would land on IntegrityError
+        # and also 401.
+        for username, status in refusals:
+            assert status == 401, f"losing racer {username} got {status}, expected 401"
+
+        with app.app_context():
+            users = restapi.User.query.all()
+            assert len(users) == 1
+            assert users[0].role == restapi.ADMIN_ROLE
+            markers = restapi.BootstrapMarker.query.all()
+            assert len(markers) == 1, "sentinel row must land in the same transaction"
+    finally:
+        # The worker threads each opened a pooled connection; dispose the pool
+        # so no stale schema-inspection cache survives into the next test.
+        with app.app_context():
+            restapi.db.engine.dispose()
+
+
+def test_bootstrap_sentinel_blocks_a_second_bootstrap_after_user_deletion(app):
+    """Even if the user table is wiped later, the sentinel keeps bootstrap closed.
+
+    This exercises the belt-and-suspenders path: the lock's re-check reads BOTH
+    User and BootstrapMarker, so a device whose users were deleted outside the
+    API cannot be re-bootstrapped without authentication.
+    """
+    client = app.test_client()
+    first = client.post(
+        "/api/create-user",
+        json={"username": "founder", "password": "pw-abcd-1234"},
+    )
+    assert first.status_code == 201, first.get_json()
+
+    with app.app_context():
+        restapi.User.query.delete()
+        restapi.db.session.commit()
+        assert restapi.User.query.count() == 0
+        assert restapi.BootstrapMarker.query.count() == 1
+
+    second = client.post(
+        "/api/create-user",
+        json={"username": "pretender", "password": "pw-abcd-1234"},
+    )
+    assert second.status_code == 401

webserver/restapi.py

@@ -4,6 +4,7 @@
 import base64
 import json
 import os
+import threading
 from typing import Callable, Optional
 
 from flask import Blueprint, Flask, current_app, jsonify, request
@@ -18,6 +19,7 @@
 from flask_sqlalchemy import SQLAlchemy
 from sqlalchemy import inspect as sa_inspect
 from sqlalchemy import text as sa_text
+from sqlalchemy.exc import IntegrityError
 from werkzeug.security import check_password_hash, generate_password_hash
 
 import webserver.config
@@ -135,10 +137,10 @@ def restapi_capabilities():
 
 jwt_blacklist = set()
 
-# Role-based access control.  For now there are exactly two roles: ``admin``
-# (may manage every account) and ``user`` (may edit only its own account and
-# cannot create or delete accounts).  Enforcement lives server-side in the
-# endpoints below — the editor UI mirrors it but is never the boundary.
+# RBAC: two roles. ``admin`` manages accounts and retrieves projects.
+# ``user`` operates the PLC (upload, start/stop, debug, status/logs) and
+# edits its own account. Enforcement is server-side; the editor UI mirrors
+# it but is never the boundary.
 ADMIN_ROLE = "admin"
 USER_ROLE = "user"
 ROLES = (ADMIN_ROLE, USER_ROLE)
@@ -186,6 +188,25 @@ def to_dict(self):
         return {"id": self.id, "username": self.username, "role": self.role}
 
 
+class BootstrapMarker(db.Model):  # type: ignore[name-defined]
+    """One-row sentinel inserted alongside the first admin.
+
+    Primary key is implicitly ``UNIQUE``, so a second concurrent writer that
+    races past ``_bootstrap_lock`` collides on commit and the create-user
+    handler catches it as ``IntegrityError``. The row is written in the same
+    transaction as the first ``User``, so either both land or neither does.
+    """
+
+    __tablename__ = "bootstrap_marker"
+    id: int = db.Column(db.Integer, primary_key=True)
+
+
+# Serializes the unauthenticated bootstrap branch of create-user. Only one
+# thread runs the check / hash / commit sequence at a time; the authoritative
+# re-check inside the lock closes the window the TOCTOU race opened.
+_bootstrap_lock = threading.Lock()
+
+
 def admin_count() -> int:
     """Number of accounts holding the admin role (used by last-admin guards)."""
     return User.query.filter_by(role=ADMIN_ROLE).count()
@@ -351,19 +372,19 @@ def create_user():
         logger.error("Error checking for users: %s", e)
         return jsonify({"msg": f"User creation error: {e}"}), 401
 
-    # Bootstrap: with no users yet, anyone may create the FIRST account and it
-    # is always an admin (someone has to be able to manage accounts). Once any
-    # user exists, only an authenticated admin may create further accounts.
+    # First account on an empty DB is created unauthenticated and is always
+    # admin. The bootstrap branch is serialized so two racing callers cannot
+    # both land as admin. See _bootstrap_first_admin.
     if not users_exist:
-        role = ADMIN_ROLE
-    else:
-        if verify_jwt_in_request(optional=True) is None:
-            return jsonify({"msg": "Authentication required"}), 401
-        if not (current_user and current_user.is_admin()):
-            return jsonify({"msg": "Admin privileges required"}), 403
-        role = (request.get_json() or {}).get("role", USER_ROLE)
-        if role not in ROLES:
-            return jsonify({"msg": f"Invalid role. Must be one of: {', '.join(ROLES)}"}), 400
+        return _bootstrap_first_admin()
+
+    if verify_jwt_in_request(optional=True) is None:
+        return jsonify({"msg": "Authentication required"}), 401
+    if not (current_user and current_user.is_admin()):
+        return jsonify({"msg": "Admin privileges required"}), 403
+    role = (request.get_json() or {}).get("role", USER_ROLE)
+    if role not in ROLES:
+        return jsonify({"msg": f"Invalid role. Must be one of: {', '.join(ROLES)}"}), 400
 
     data = request.get_json()
     username = data.get("username")
@@ -384,6 +405,37 @@ def create_user():
     return jsonify({"msg": "User created", "id": user.id, "role": user.role}), 201
 
 
+def _bootstrap_first_admin():
+    """Create the first admin on an empty database, serialized against races.
+
+    Called from ``create_user`` only when the pre-lock check sees no users.
+    Inside the lock an authoritative re-check against ``User`` and
+    ``BootstrapMarker`` closes the TOCTOU window; the UNIQUE PK on
+    ``BootstrapMarker`` catches a lock bypass as ``IntegrityError``.
+    """
+    with _bootstrap_lock:
+        if User.query.first() is not None or BootstrapMarker.query.first() is not None:
+            return jsonify({"msg": "Authentication required"}), 401
+
+        data = request.get_json() or {}
+        username = data.get("username")
+        password = data.get("password")
+        if not username or not password:
+            return jsonify({"msg": "Missing username or password"}), 400
+
+        user = User(username=username, role=ADMIN_ROLE)
+        user.set_password(password)
+        db.session.add(user)
+        db.session.add(BootstrapMarker(id=1))
+        try:
+            db.session.commit()
+        except IntegrityError:
+            db.session.rollback()
+            return jsonify({"msg": "Authentication required"}), 401
+
+        return jsonify({"msg": "User created", "id": user.id, "role": user.role}), 201
+
+
 # verify existing users individually
 @restapi_bp.route("/get-user-info/<int:user_id>", methods=["GET"])
 @jwt_required()