From b7b4c3129620dec953612579fd17f6a8cd93eb20 Mon Sep 17 00:00:00 2001 From: Benny Date: Mon, 21 Sep 2026 22:30:13 -0400 Subject: [PATCH 1/2] feat(auth): add `velodrome create-admin` to bootstrap the first user A fresh deployment could not be used. Registration requires a valid invite code, invites can only be issued by an existing admin, and a newly migrated database has neither -- so there was no way to create the first account. docs/PLAN.md always called for this command; it was never built during Phase 0, and D16 (single-container deploy) made the gap reachable. Adds a console script -- `[project.scripts]` -> velodrome.cli:main -- which installs into the same venv as alembic and uvicorn, so the deployed image already has it on PATH: docker exec -it velodrome velodrome create-admin --email you@example.com The account-creating logic is `auth.service.create_admin`, not something in cli.py, so that `db.auth_session` stays confined to auth/service.py as its docstring requires. Its lookup is an exact match on a unique key, which is the pattern db.py documents as safe on that session. Deliberate constraints, all covered by tests (see docs/DECISIONS.md D18): - Refuses an email that already exists rather than updating the row. An operator re-running a months-old command from shell history means "create", never "reset the password"; silently accepting would make this an undocumented password-reset tool that any container-exec grants. - Not restricted to "only when there are zero users". The restriction buys nothing -- reaching the command already requires process execution inside the container, which already permits rewriting the SQLite file directly -- while removing the cases that do happen: a second admin, and recovering an instance whose only admin was lost. - No --password flag. An argument lands in shell history, in ps output, and in the Docker daemon's record of the exec'd command. A TTY prompt (with confirmation) and --password-stdin are the two forms that avoid all three. - Pydantic's ValidationError is never printed verbatim: its rendering embeds the offending value, which for a short password prints the password itself. Only loc and msg are shown (CLAUDE.md invariant #5). role="admin" is recorded but nothing enforces it yet -- there is no admin-only endpoint until invite management in Phase 1. ROLE_ADMIN/ ROLE_MEMBER become named constants, and AuthenticatedSession carries the role for that future check. It is deliberately absent from UserOut, so no HTTP response and no OpenAPI contract changes. The register endpoint's password and display-name constraints move to named aliases in schemas/auth so the CLI applies exactly the same rules rather than a drifting copy. Verified: ruff check, ruff format --check, mypy --strict, and the full pytest suite (28 passed) from apps/api/; `alembic check` reports no model drift. Also smoke-tested end to end against a scratch database -- creation, the duplicate-email refusal, and the no-TTY message all behave as described. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01R2ZKeWkZV7ehf7fivrAkkG --- apps/api/pyproject.toml | 6 + apps/api/tests/test_cli.py | 308 ++++++++++++++++++++++++++ apps/api/velodrome/auth/service.py | 73 +++++- apps/api/velodrome/cli.py | 181 +++++++++++++++ apps/api/velodrome/models/__init__.py | 4 +- apps/api/velodrome/models/identity.py | 14 +- apps/api/velodrome/schemas/auth.py | 12 +- 7 files changed, 588 insertions(+), 10 deletions(-) create mode 100644 apps/api/tests/test_cli.py create mode 100644 apps/api/velodrome/cli.py diff --git a/apps/api/pyproject.toml b/apps/api/pyproject.toml index 50df949..841004e 100644 --- a/apps/api/pyproject.toml +++ b/apps/api/pyproject.toml @@ -15,6 +15,12 @@ dependencies = [ "uuid6>=2024.7.10", ] +[project.scripts] +# Installs into the venv's bin/ next to `alembic` and `uvicorn`, which deploy/entrypoint.sh +# already invokes by their installed script names — so `docker exec velodrome velodrome ...` works +# against the deployed image with no extra wiring (the Dockerfile puts /app/.venv/bin on PATH). +velodrome = "velodrome.cli:main" + [project.optional-dependencies] dev = [ "ruff>=0.7", diff --git a/apps/api/tests/test_cli.py b/apps/api/tests/test_cli.py new file mode 100644 index 0000000..51b8287 --- /dev/null +++ b/apps/api/tests/test_cli.py @@ -0,0 +1,308 @@ +"""Tests for the operator CLI (`velodrome create-admin`). + +These drive `cli.run()` directly against the same real SQLite database and real Argon2id hashing +every other test uses — no mocks, per CLAUDE.md's test policy. That matters more than usual here: +this command is the only way to create the first account on a fresh deployment, it is run exactly +once by a human who has no way to debug it, and the failure mode of "it printed success but the +password doesn't actually work" is indistinguishable from a broken deployment. So the central test +below doesn't assert on a return code — it creates an admin through the CLI and then logs in as +that admin over HTTP, proving the hash the CLI wrote is one the login path accepts. + +The other thing under test is what the CLI *refuses* to do: overwrite an existing account, and +echo a rejected password back to the terminal (CLAUDE.md invariant #5). +""" + +import io +import sys +import tomllib +from pathlib import Path + +import httpx +import pytest +from sqlalchemy import text +from sqlalchemy.ext.asyncio import AsyncSession + +from velodrome import cli +from velodrome.models import ROLE_ADMIN, ROLE_MEMBER + +_PASSWORD = "correct horse battery staple" +_OTHER_PASSWORD = "an entirely different passphrase" + + +class _FakeTty: + """Stands in for `sys.stdin` attached to a terminal, so `_read_password` takes the prompt + branch rather than the pipe branch. `readline` raises rather than returning a value: if the + prompt path ever silently starts reading stdin instead of calling getpass, that's a behaviour + change this should fail on, not absorb.""" + + def isatty(self) -> bool: + return True + + def readline(self) -> str: + raise AssertionError("the prompt path must not read stdin directly") + + +def _pipe(password: str, *, newline: str = "\n") -> io.StringIO: + """stdin as a pipe (isatty() is False on StringIO), carrying one line.""" + return io.StringIO(f"{password}{newline}") + + +async def test_create_admin_creates_an_account_that_can_actually_log_in( + client: httpx.AsyncClient, db_auth: AsyncSession, monkeypatch: pytest.MonkeyPatch +) -> None: + """The load-bearing test: bootstrap an admin through the CLI, then log in as them over HTTP. + + This is deliberately an end-to-end assertion rather than "did a row appear with a hash in it". + The whole point of the command is to produce working credentials on a deployment where nobody + can yet log in to check, so the only assertion worth making is that the credentials work + through the same endpoint a real operator would use next. + """ + monkeypatch.setattr(sys, "stdin", _pipe(_PASSWORD)) + code = await cli.run( + ["create-admin", "--email", "boss@example.com", "--name", "Boss", "--password-stdin"] + ) + assert code == 0 + + resp = await client.post( + "/api/v1/auth/login", json={"email": "boss@example.com", "password": _PASSWORD} + ) + assert resp.status_code == 200, resp.text + assert "vd_session" in resp.cookies + + me = await client.get("/api/v1/auth/me") + assert me.status_code == 200 + assert me.json()["email"] == "boss@example.com" + assert me.json()["display_name"] == "Boss" + + +async def test_create_admin_records_the_admin_role( + db_auth: AsyncSession, monkeypatch: pytest.MonkeyPatch +) -> None: + """The role is the one thing that distinguishes this from registration, and nothing enforces + it yet (docs/DECISIONS.md D17) — so nothing else in the suite would notice if it silently + wrote `member`. Asserted against the stored column directly, and against ROLE_MEMBER too, so + this fails loudly rather than passing vacuously if the default ever changes.""" + monkeypatch.setattr(sys, "stdin", _pipe(_PASSWORD)) + assert await cli.run(["create-admin", "--email", "boss@example.com", "--password-stdin"]) == 0 + + role = ( + await db_auth.execute( + text("SELECT role FROM users WHERE email = :email"), {"email": "boss@example.com"} + ) + ).scalar_one() + await db_auth.commit() + assert role == ROLE_ADMIN + assert role != ROLE_MEMBER + + +async def test_create_admin_defaults_display_name_to_the_email_local_part( + db_auth: AsyncSession, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr(sys, "stdin", _pipe(_PASSWORD)) + assert await cli.run(["create-admin", "--email", "benny@example.com", "--password-stdin"]) == 0 + + name = ( + await db_auth.execute( + text("SELECT display_name FROM users WHERE email = :email"), + {"email": "benny@example.com"}, + ) + ).scalar_one() + await db_auth.commit() + assert name == "benny" + + +async def test_create_admin_refuses_an_existing_email_without_touching_the_account( + client: httpx.AsyncClient, db_auth: AsyncSession, monkeypatch: pytest.MonkeyPatch +) -> None: + """Re-running the bootstrap command must not become an undocumented password reset. + + "Refused" is asserted three ways, because exit code 1 alone would also be satisfied by a + command that failed *after* corrupting the row: the original password must still work, the new + one must not, and the display name must be unchanged. + """ + monkeypatch.setattr(sys, "stdin", _pipe(_PASSWORD)) + assert ( + await cli.run( + ["create-admin", "--email", "boss@example.com", "--name", "Boss", "--password-stdin"] + ) + == 0 + ) + + monkeypatch.setattr(sys, "stdin", _pipe(_OTHER_PASSWORD)) + second = await cli.run( + ["create-admin", "--email", "boss@example.com", "--name", "Impostor", "--password-stdin"] + ) + assert second == 1 + + still_works = await client.post( + "/api/v1/auth/login", json={"email": "boss@example.com", "password": _PASSWORD} + ) + assert still_works.status_code == 200, "the original password must survive a refused re-run" + + rejected = await client.post( + "/api/v1/auth/login", json={"email": "boss@example.com", "password": _OTHER_PASSWORD} + ) + assert rejected.status_code == 401, "the refused run's password must never become valid" + + name = ( + await db_auth.execute( + text("SELECT display_name FROM users WHERE email = :email"), + {"email": "boss@example.com"}, + ) + ).scalar_one() + await db_auth.commit() + assert name == "Boss" + + +async def test_create_admin_error_message_never_echoes_the_password( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """CLAUDE.md invariant #5, in the one place it's easy to breach by accident. + + Pydantic's default rendering of a ValidationError embeds the offending value — for a + too-short password that means printing the password itself to the operator's terminal, and + into whatever captured that output (a CI log, a `script` session, a scrollback buffer shared + in a bug report). `_validate` strips it deliberately; this proves it stays stripped. + """ + secret = "short" + monkeypatch.setattr(sys, "stdin", _pipe(secret)) + code = await cli.run(["create-admin", "--email", "boss@example.com", "--password-stdin"]) + assert code == 1 + + captured = capsys.readouterr() + assert secret not in captured.out + assert secret not in captured.err + # ...while still being a useful message: it must name the offending field and the rule. + assert "password" in captured.err + assert "at least 8" in captured.err + + +async def test_create_admin_rejects_a_malformed_email( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + monkeypatch.setattr(sys, "stdin", _pipe(_PASSWORD)) + code = await cli.run(["create-admin", "--email", "not-an-email", "--password-stdin"]) + assert code == 1 + assert "email" in capsys.readouterr().err + + +async def test_password_stdin_rejects_an_empty_line( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """An empty pipe is nearly always `$PASSWORD` being unset in the operator's shell. Failing + loudly beats creating an account whose password is the empty string.""" + monkeypatch.setattr(sys, "stdin", io.StringIO("")) + code = await cli.run(["create-admin", "--email", "boss@example.com", "--password-stdin"]) + assert code == 1 + assert "empty" in capsys.readouterr().err + + +async def test_password_stdin_preserves_a_trailing_space( + client: httpx.AsyncClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """Only the line ending is stripped, not surrounding whitespace — a password with a trailing + space is legitimate, and trimming it would create an account whose password can never be typed + back in. Proven through a real login rather than by inspecting the hash.""" + padded = f"{_PASSWORD} " + monkeypatch.setattr(sys, "stdin", _pipe(padded, newline="\r\n")) + assert await cli.run(["create-admin", "--email", "boss@example.com", "--password-stdin"]) == 0 + + resp = await client.post( + "/api/v1/auth/login", json={"email": "boss@example.com", "password": padded} + ) + assert resp.status_code == 200, "the trailing space must be part of the stored password" + + trimmed = await client.post( + "/api/v1/auth/login", json={"email": "boss@example.com", "password": _PASSWORD} + ) + assert trimmed.status_code == 401 + + +async def test_without_a_tty_or_password_stdin_it_explains_how_to_run_it( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """The most likely first-run mistake is `docker exec` without `-t`. getpass would otherwise + fail with a bare OSError, so the command catches it first and prints both working forms.""" + monkeypatch.setattr(sys, "stdin", io.StringIO("")) + code = await cli.run(["create-admin", "--email", "boss@example.com"]) + assert code == 1 + + err = capsys.readouterr().err + assert "docker exec -it" in err + assert "--password-stdin" in err + + +async def test_prompt_path_requires_the_confirmation_to_match( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """A typo in a password nobody can see, on the one account that can't be recovered by another + admin, is worth a second prompt.""" + monkeypatch.setattr(sys, "stdin", _FakeTty()) + answers = iter([_PASSWORD, _OTHER_PASSWORD]) + monkeypatch.setattr(cli.getpass, "getpass", lambda prompt="": next(answers)) + + code = await cli.run(["create-admin", "--email", "boss@example.com"]) + assert code == 1 + assert "did not match" in capsys.readouterr().err + + +async def test_prompt_path_creates_the_account_when_both_entries_match( + client: httpx.AsyncClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """The interactive path is the one the docs tell operators to use, so it gets the same + end-to-end login proof as the piped path.""" + monkeypatch.setattr(sys, "stdin", _FakeTty()) + answers = iter([_PASSWORD, _PASSWORD]) + monkeypatch.setattr(cli.getpass, "getpass", lambda prompt="": next(answers)) + + assert await cli.run(["create-admin", "--email", "boss@example.com"]) == 0 + + resp = await client.post( + "/api/v1/auth/login", json={"email": "boss@example.com", "password": _PASSWORD} + ) + assert resp.status_code == 200 + + +async def test_success_output_never_contains_the_password( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """The failure path is covered above; the success path prints more, so it gets its own check. + The realistic leak here is a well-meaning "created with password: ..." confirmation line.""" + monkeypatch.setattr(sys, "stdin", _pipe(_PASSWORD)) + assert await cli.run(["create-admin", "--email", "boss@example.com", "--password-stdin"]) == 0 + + captured = capsys.readouterr() + assert _PASSWORD not in captured.out + assert _PASSWORD not in captured.err + assert "boss@example.com" in captured.out + assert ROLE_ADMIN in captured.out + + +async def test_a_missing_required_option_exits_two_not_one( + capsys: pytest.CaptureFixture[str], +) -> None: + """argparse's usage errors exit 2; a refusal from the command itself exits 1. A script driving + this should be able to tell "you called it wrong" from "it ran and declined".""" + with pytest.raises(SystemExit) as exc: + await cli.run(["create-admin"]) + assert exc.value.code == 2 + capsys.readouterr() + + +async def test_no_subcommand_is_a_usage_error(capsys: pytest.CaptureFixture[str]) -> None: + with pytest.raises(SystemExit) as exc: + await cli.run([]) + assert exc.value.code == 2 + capsys.readouterr() + + +def test_the_console_script_is_registered_under_the_name_the_docs_use() -> None: + """deploy/README.md and the CLI's own error messages tell operators to run + `docker exec -it velodrome velodrome create-admin`. That only works because pyproject declares + the console script, which nothing else in the test suite would exercise — an editable install + imports `velodrome.cli` fine whether or not the entry point exists. Asserted against the + manifest so renaming the module or the function fails here rather than on a deployment. + """ + pyproject = Path(__file__).resolve().parents[1] / "pyproject.toml" + manifest = tomllib.loads(pyproject.read_text(encoding="utf-8")) + assert manifest["project"]["scripts"]["velodrome"] == "velodrome.cli:main" diff --git a/apps/api/velodrome/auth/service.py b/apps/api/velodrome/auth/service.py index 29ab49a..fa04a87 100644 --- a/apps/api/velodrome/auth/service.py +++ b/apps/api/velodrome/auth/service.py @@ -23,7 +23,7 @@ from velodrome.auth.security import ( ) from velodrome.config import get_settings from velodrome.db import auth_session -from velodrome.models import Invite, Session, User +from velodrome.models import ROLE_ADMIN, Invite, Session, User class AuthError(Exception): @@ -51,6 +51,14 @@ class AuthenticatedSession: user_id: UUID email: str display_name: str + # Carried here so the role a user actually has is available wherever identity is — an + # authorization check on the first admin-only endpoint (realistically invite management) is + # then a comparison against a value already in hand, not another query bolted on later. This + # is data plumbing, not an authorization mechanism: nothing reads it yet, deliberately, since + # there is no admin-only endpoint to protect (docs/DECISIONS.md D18). It is not a secret and + # is not in any response model — `schemas.auth.UserOut` deliberately doesn't declare it, so + # adding it here changes no HTTP response and no OpenAPI contract. + role: str async def register( @@ -103,7 +111,58 @@ async def register( invite.used_count += 1 return AuthenticatedSession( - user_id=user.id, email=user.email, display_name=user.display_name + user_id=user.id, + email=user.email, + display_name=user.display_name, + role=user.role, + ) + + +async def create_admin(*, email: str, password: str, display_name: str) -> AuthenticatedSession: + """Create a user with the admin role, with no invite. Operator path only — see velodrome.cli. + + This is the one deliberate hole in "open signup does not exist": a fresh deployment has no + users and therefore nobody who can issue the first invite, so the first account has to come + from outside the HTTP API. It lives here rather than in cli.py because this is where + `auth_session` belongs (see db.py's docstring — nothing outside this module imports it), and + because the lookup below is exactly the pattern that module documents as safe: an exact match + on a unique key, never a scan. + + It is not reachable over HTTP and never will be — nothing in `api/` calls it. Reaching it + requires the ability to run a process inside the container, which is already the ability to + read and rewrite the SQLite file directly, so it grants an operator-turned-attacker nothing + they did not already have. + + Refuses outright if the email is taken, rather than updating the row — see docs/DECISIONS.md + D18. Creating an account and resetting an existing account's password are different operations + with different blast radii, and an operator re-running a bootstrap command they last ran + months ago means the first, never the second. The check-then-insert is safe against a + concurrent `register()` for the same email the same way invite redemption is: db.py issues + `BEGIN IMMEDIATE`, so the two transactions serialize instead of interleaving, with the unique + index on `users.email` as the backstop underneath that. + """ + async with auth_session() as db: + async with db.begin(): + existing = ( + await db.execute(select(User).where(User.email == email)) + ).scalar_one_or_none() + if existing is not None: + raise EmailAlreadyRegistered("an account with this email already exists") + + user = User( + email=email, + display_name=display_name, + password_hash=hash_password(password), + role=ROLE_ADMIN, + ) + db.add(user) + await db.flush() # populate user.id before we reference it below + + return AuthenticatedSession( + user_id=user.id, + email=user.email, + display_name=user.display_name, + role=user.role, ) @@ -140,7 +199,10 @@ async def login( db.add(session_row) return raw_token, AuthenticatedSession( - user_id=user.id, email=user.email, display_name=user.display_name + user_id=user.id, + email=user.email, + display_name=user.display_name, + role=user.role, ) @@ -184,7 +246,10 @@ async def validate_session(raw_token: str) -> AuthenticatedSession: ) return AuthenticatedSession( - user_id=user.id, email=user.email, display_name=user.display_name + user_id=user.id, + email=user.email, + display_name=user.display_name, + role=user.role, ) diff --git a/apps/api/velodrome/cli.py b/apps/api/velodrome/cli.py new file mode 100644 index 0000000..5cd7b3a --- /dev/null +++ b/apps/api/velodrome/cli.py @@ -0,0 +1,181 @@ +"""Operator CLI — `velodrome `. + +Installed as a console script (`[project.scripts]` in pyproject.toml) into the same venv as +`alembic` and `uvicorn`, which `deploy/entrypoint.sh` already invokes by their installed names, so +the deployed container has this on PATH with no extra wiring: + + docker exec -it velodrome velodrome create-admin --email you@example.com + +It exists because there is otherwise **no way to create the first user**. Registration requires a +valid invite (docs/PLAN.md "Auth": open signup does not exist as a setting), invites are created by +an existing admin, and a fresh database has neither — so a new deployment is unusable without a +path in from outside the HTTP API. docs/PLAN.md always called for this command; it was simply +never built during Phase 0. See docs/DECISIONS.md D18 for the three decisions recorded here: why +it refuses to touch an existing account, why it is *not* restricted to the very first user, and +why `role="admin"` is recorded but not yet enforced anywhere. + +argparse rather than Typer/Click: one command with three options does not justify a runtime +dependency the deployed image would have to carry, and the stdlib covers this case completely. + +There is deliberately **no `--password` flag** — see `_read_password`. +""" + +import argparse +import asyncio +import getpass +import sys +from collections.abc import Sequence + +from pydantic import BaseModel, EmailStr, ValidationError + +from velodrome.auth import service +from velodrome.config import get_settings +from velodrome.schemas.auth import DisplayName, Password + + +class CliError(Exception): + """An operator-facing failure: printed as `error: `, exit code 1. + + Distinct from argparse's own usage errors, which exit 2 — so a script driving this can tell + "you called it wrong" apart from "it ran and refused". + """ + + +class _CreateAdminInput(BaseModel): + """The same constraints the HTTP register endpoint applies, reused rather than restated — a + CLI-created account must not be able to hold a password the API would have rejected. + """ + + email: EmailStr + password: Password + display_name: DisplayName + + +def _validate(*, email: str, password: str, display_name: str) -> _CreateAdminInput: + try: + return _CreateAdminInput(email=email, password=password, display_name=display_name) + except ValidationError as exc: + # Deliberately not `str(exc)`: pydantic's rendered message embeds the offending value + # ("... [type=string_too_short, input_value='hunter2', input_type=str]"), which for the + # password field prints the password to the operator's terminal and into whatever + # captures that output. CLAUDE.md invariant #5 — only `loc` and `msg` are safe to show. + details = "; ".join( + f"{'.'.join(str(part) for part in err['loc'])}: {err['msg']}" + for err in exc.errors(include_url=False, include_input=False) + ) + raise CliError(f"invalid input — {details}") from exc + + +def _read_password(*, from_stdin: bool) -> str: + """Prompt for a password, or read one line from stdin. + + No `--password` flag exists on purpose: an argument lands in the operator's shell history, in + `ps` output for as long as the process runs, and — because the realistic invocation here is + `docker exec` — in the Docker daemon's own record of the exec'd command. A TTY prompt and a + pipe are the two forms that avoid all three, and they're the same two forms `docker login` + offers for exactly this reason. + """ + if from_stdin: + line = sys.stdin.readline() + # Strip only the line ending, not surrounding whitespace — a trailing space in a password + # is legitimate, and silently trimming it would create a password that can never be typed + # back in correctly. + password = line.rstrip("\r\n") + if not password: + raise CliError("--password-stdin was given but the first line of stdin was empty") + return password + + if not sys.stdin.isatty(): + raise CliError( + "no terminal available to prompt on. Either allocate one (note the -t):\n" + " docker exec -it velodrome velodrome create-admin --email you@example.com\n" + "or pipe the password in:\n" + " printf '%s' \"$PASSWORD\" | docker exec -i velodrome \\\n" + " velodrome create-admin --email you@example.com --password-stdin" + ) + + password = getpass.getpass("Password: ") + if password != getpass.getpass("Confirm password: "): + raise CliError("passwords did not match") + return password + + +async def _create_admin(args: argparse.Namespace) -> int: + email: str = args.email + # The local part is a reasonable default for a name nobody but the operator will see until + # they change it in the UI; it keeps the common invocation to a single flag. + display_name: str = args.name if args.name is not None else email.partition("@")[0] + + password = _read_password(from_stdin=args.password_stdin) + validated = _validate(email=email, password=password, display_name=display_name) + + try: + created = await service.create_admin( + email=str(validated.email), + password=validated.password, + display_name=validated.display_name, + ) + except service.EmailAlreadyRegistered as exc: + raise CliError( + f"an account already exists for {email} — refusing to modify it. This command only " + "ever creates a new account; it will not reset an existing one's password (see " + "docs/DECISIONS.md D18). To add a different admin, re-run with another --email." + ) from exc + + print("Created admin user:") + print(f" id {created.user_id}") + print(f" email {created.email}") + print(f" display name {created.display_name}") + print(f" role {created.role}") + print() + print(f"Log in at {get_settings().public_url}") + print( + "Note: the admin role is recorded on the account but nothing enforces it yet — no " + "admin-only endpoint exists (docs/DECISIONS.md D18)." + ) + return 0 + + +def _build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser( + prog="velodrome", + description="Velodrome operator commands. Run inside the container, e.g. " + "`docker exec -it velodrome velodrome create-admin --email you@example.com`.", + ) + subcommands = parser.add_subparsers(dest="command", required=True) + + create_admin = subcommands.add_parser( + "create-admin", + help="create a user with the admin role, bypassing the invite requirement", + description="Create a user with the admin role, bypassing the invite requirement. This is " + "how the first account on a fresh deployment is made — registration needs an invite, and " + "a fresh database has none. Refuses to modify an account that already exists.", + ) + create_admin.add_argument("--email", required=True, help="the account's email address") + create_admin.add_argument( + "--name", + default=None, + help="display name (default: the part of the email address before the @)", + ) + create_admin.add_argument( + "--password-stdin", + action="store_true", + help="read the password from the first line of stdin instead of prompting for it", + ) + return parser + + +async def run(argv: Sequence[str] | None = None) -> int: + """The async entrypoint. `main` wraps this in `asyncio.run`; tests call it directly.""" + args = _build_parser().parse_args(argv) + try: + if args.command == "create-admin": + return await _create_admin(args) + except CliError as exc: + print(f"error: {exc}", file=sys.stderr) + return 1 + raise AssertionError(f"unhandled command {args.command!r}") # pragma: no cover + + +def main(argv: Sequence[str] | None = None) -> int: + return asyncio.run(run(argv)) diff --git a/apps/api/velodrome/models/__init__.py b/apps/api/velodrome/models/__init__.py index 7f01319..06bf375 100644 --- a/apps/api/velodrome/models/__init__.py +++ b/apps/api/velodrome/models/__init__.py @@ -1,4 +1,4 @@ from velodrome.models.base import Base -from velodrome.models.identity import ApiToken, Invite, Session, User +from velodrome.models.identity import ROLE_ADMIN, ROLE_MEMBER, ApiToken, Invite, Session, User -__all__ = ["ApiToken", "Base", "Invite", "Session", "User"] +__all__ = ["ROLE_ADMIN", "ROLE_MEMBER", "ApiToken", "Base", "Invite", "Session", "User"] diff --git a/apps/api/velodrome/models/identity.py b/apps/api/velodrome/models/identity.py index ba99fa0..088ba4e 100644 --- a/apps/api/velodrome/models/identity.py +++ b/apps/api/velodrome/models/identity.py @@ -24,6 +24,16 @@ def _now_utc() -> datetime: return datetime.now(UTC) +# The only two values `users.role` and `invites.role` are ever set to. Named constants so the set +# is discoverable from one place: `velodrome.cli`'s create-admin writes ROLE_ADMIN, and +# registration copies whatever role the redeemed invite carries. Nothing *enforces* a role yet — +# no admin-only endpoint exists (docs/DECISIONS.md D18) — and the column stays a plain string +# rather than a DB-level enum or CHECK constraint so adding a third role later is an application +# change, not a migration. +ROLE_MEMBER = "member" +ROLE_ADMIN = "admin" + + class User(Base): __tablename__ = "users" @@ -31,7 +41,7 @@ class User(Base): email: Mapped[str] = mapped_column(String(320), unique=True, nullable=False) display_name: Mapped[str] = mapped_column(String(200), nullable=False) password_hash: Mapped[str] = mapped_column(Text, nullable=False) - role: Mapped[str] = mapped_column(String(20), nullable=False, default="member") + role: Mapped[str] = mapped_column(String(20), nullable=False, default=ROLE_MEMBER) timezone: Mapped[str] = mapped_column(String(64), nullable=False, default="UTC") # Display-only, per CLAUDE.md invariant #3 — storage is always SI, this never touches a query. unit_system: Mapped[str] = mapped_column(String(10), nullable=False, default="imperial") @@ -53,7 +63,7 @@ class Invite(Base): Uuid(as_uuid=True), ForeignKey("users.id"), nullable=False ) email: Mapped[str | None] = mapped_column(String(320), nullable=True) - role: Mapped[str] = mapped_column(String(20), nullable=False, default="member") + role: Mapped[str] = mapped_column(String(20), nullable=False, default=ROLE_MEMBER) expires_at: Mapped[datetime] = mapped_column(nullable=False) max_uses: Mapped[int] = mapped_column(nullable=False, default=1) used_count: Mapped[int] = mapped_column(nullable=False, default=0) diff --git a/apps/api/velodrome/schemas/auth.py b/apps/api/velodrome/schemas/auth.py index e60bb03..d2679b2 100644 --- a/apps/api/velodrome/schemas/auth.py +++ b/apps/api/velodrome/schemas/auth.py @@ -6,15 +6,23 @@ simply not being listed here is what keeps password_hash/token_hash out of every adding a new field, ask whether it belongs in a response before adding it, not after. """ +from typing import Annotated from uuid import UUID from pydantic import BaseModel, EmailStr, Field +# Named aliases rather than inline constraints, because these two rules are also applied outside +# the HTTP layer: `velodrome.cli` validates `create-admin`'s input against exactly the same ones, +# so an account created from the CLI can't hold a password the register endpoint would have +# rejected. Defined once here so the two can't drift apart. +Password = Annotated[str, Field(min_length=8, max_length=200)] +DisplayName = Annotated[str, Field(min_length=1, max_length=200)] + class RegisterRequest(BaseModel): email: EmailStr - password: str = Field(min_length=8, max_length=200) - display_name: str = Field(min_length=1, max_length=200) + password: Password + display_name: DisplayName invite_code: str = Field(min_length=1, max_length=200) -- 2.54.0 From d0c0d98307d3ce22f1252f7292424967378a51e8 Mon Sep 17 00:00:00 2001 From: Benny Date: Mon, 21 Sep 2026 22:30:38 -0400 Subject: [PATCH 2/2] docs: record D18 (admin bootstrap) and the deploy bootstrap step The deploy README described how to start the container but not how to get into it, which left the first-run experience at a login page nobody can get past. Adds the actual command, both the interactive and the piped form, and says why there is no --password flag. D18 records the three decisions worth arguing with later rather than rediscovering: why this is a CLI instead of a bootstrap HTTP endpoint or an env var (both rejected, with reasons), why it refuses an existing email, why it is not restricted to the first user, and why the admin role is recorded but not yet enforced. Numbered D18 because D17 was taken by the registry-TLS decision that merged while this branch was in flight. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01R2ZKeWkZV7ehf7fivrAkkG --- deploy/README.md | 35 +++++++++++++++++++++++++ docs/DECISIONS.md | 67 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) diff --git a/deploy/README.md b/deploy/README.md index fa00fd2..41fb2fe 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -43,6 +43,41 @@ front of port 8080 — this container only ever serves plain HTTP itself. `GET http://:8080/api/v1/healthz` should return `{"status": "ok"}` once it's up. +## Create the first admin user + +**A fresh deployment has no users and you cannot sign up for one.** Registration requires an invite +code, invites are issued by an existing admin, and a new database has neither — so the first account +is created from inside the container (`docs/DECISIONS.md` D18 for why it's a CLI and not a +first-run web page): + +```sh +docker exec -it velodrome velodrome create-admin --email you@example.com +``` + +That prompts for the password twice and prints the new account's id, email and role. Then log in at +`VELODROME_PUBLIC_URL`. Note the **`-t`** — without a TTY there's nothing to prompt on; the command +says so rather than hanging. Add `--name "Your Name"` to set a display name (it defaults to the part +of the email before the `@`); it's editable in the UI later either way. + +For a non-interactive run (a provisioning script), pipe the password in instead — note `-i` rather +than `-it`: + +```sh +printf '%s' "$ADMIN_PASSWORD" | docker exec -i velodrome \ + velodrome create-admin --email you@example.com --password-stdin +``` + +There is deliberately no `--password` flag: an argument would land in your shell history, in `ps` +output, and in the Docker daemon's record of the exec'd command. + +Re-run it with a different `--email` to add another admin. Re-running it with an email that already +exists **refuses and changes nothing** — it is not a password-reset tool, and there isn't one yet +(D18). Minimum password length is 8 characters, the same rule the register endpoint applies. + +Nothing enforces the admin role yet — no admin-only endpoint exists — so today this differs from an +invited account only in the role recorded on it. Invite management in a later phase is what starts +reading it. + ## Environment variables All read by `apps/api/velodrome/config.py` (prefix `VELODROME_`) — the app and Alembic both read diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index dca3727..fdac4a5 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -289,6 +289,73 @@ they're ready to make that call deliberately, not bundled into this fix. and certs (`vaultwarden.bbergle.com` etc.) — untouched, new proxy host only. No other container on the Unraid host was restarted, reconfigured, or otherwise touched to make this work. +### D18 — Admin bootstrap is a CLI command, not an HTTP endpoint or a first-run mode + +**Chosen:** `velodrome create-admin`, a console script (`[project.scripts]` in +`apps/api/pyproject.toml` → `velodrome.cli:main`) installed into the same venv as `alembic` and +`uvicorn`, so the deployed image already has it on PATH: + +```sh +docker exec -it velodrome velodrome create-admin --email you@example.com +``` + +**Why this exists at all:** registration requires a valid invite code (`docs/PLAN.md` "Auth" — open +signup does not exist, not even as a setting), invites can only be created by an existing admin, +and a freshly migrated database has neither. A new deployment was therefore unusable: there was no +way to create the first account. `docs/PLAN.md` always called for this command; it was simply never +built during Phase 0, and the gap only became visible once D16 made a real deployment possible. + +**Why a CLI rather than the alternatives:** +- *A bootstrap HTTP endpoint that works only while the users table is empty* — rejected. It puts an + unauthenticated account-creating route on the public internet permanently, whose safety depends + entirely on a row count staying zero. The window is real (between first start and first login), + it's the exact window where the deployment is least watched, and the failure is silent: whoever + wins the race owns the instance. +- *An env var like `VELODROME_INITIAL_ADMIN_PASSWORD`* — rejected. A password in an env var is + visible in `docker inspect`, in the Unraid template's saved config on disk, and in the container's + own `/proc/1/environ` for the process's whole life. D16 deliberately moved configuration into + Unraid's UI, which would mean the bootstrap password sitting in that UI indefinitely. +- *Seeding a default account in a migration* — rejected outright. It would mean a known-credential + account existing on every deployment, and it contradicts the reason migrations are schema-only. + +**Why it refuses an email that already exists, rather than updating it:** creating an account and +resetting an existing account's password are different operations with different blast radii, and +the realistic scenario — an operator re-running a command they last ran months ago, from shell +history — means the first, never the second. Silently accepting it would make this an undocumented +password-reset tool that any container-exec grants, and would make the command's behaviour depend +on state the operator can't see. It exits 1 and says what it refused. A genuine password reset is a +separate future command that should have to say so in its name. + +**Why it is *not* restricted to "only when there are zero users":** that restriction sounds safer +and isn't. It buys nothing — the command already requires the ability to run a process inside the +container, which is already the ability to read and rewrite the SQLite file directly, so a +restriction only constrains the legitimate operator, never an attacker who is by definition already +past it. Meanwhile it removes the two cases that actually happen: a second admin for a family +member, and recovering an instance whose only admin account was lost. The invite system remains the +normal path for adding users; this stays the operator's escape hatch. + +**Why `role="admin"` is recorded but nothing enforces it yet:** there is no admin-only endpoint to +protect. Invite management — the first thing that genuinely needs the distinction — is Phase 1. +Writing the column now means the first account is correctly marked when that check does arrive, +rather than needing a data fix-up later; writing an *enforcement* mechanism now would be guessing at +the shape of a check with no caller. `ROLE_ADMIN`/`ROLE_MEMBER` are named constants in +`models/identity.py`, and the column stays a plain string rather than a DB enum or CHECK constraint +so a third role later is an application change, not a migration. `AuthenticatedSession.role` carries +the value for that future check; it is deliberately absent from `schemas.auth.UserOut`, so this +changes no HTTP response and no OpenAPI contract. + +**Why there is no `--password` flag:** an argument lands in shell history, in `ps` output for the +process's lifetime, and — because the realistic invocation is `docker exec` — in the Docker daemon's +record of the exec'd command. A TTY prompt (with confirmation) and `--password-stdin` are the two +forms that avoid all three, which is the same pair `docker login` offers for the same reason. +Pydantic's `ValidationError` rendering is also deliberately not printed verbatim: it embeds the +offending value, which for a too-short password prints the password to the terminal. Only `loc` and +`msg` are shown (CLAUDE.md invariant #5); `tests/test_cli.py` asserts this on both the failure and +success paths. + +**argparse, not Typer/Click:** one command with three options doesn't justify a runtime dependency +the deployed image has to carry. + --- ## Deliberately deferred -- 2.54.0