Files
bike-app/apps/api/README.md
T
BBergleandClaude Opus 5 e7392a5723
CI / Repo hygiene (pull_request) Successful in 2s
CI / Web (lint, typecheck, build) (pull_request) Successful in 13s
CI / Migrations reversible (pull_request) Successful in 6s
CI / API (lint, types, tests) (pull_request) Successful in 52s
refactor(api): move from Postgres+RLS to single-engine SQLite
Reverses a shipped, tested, merged decision (D4/PR #2) rather than building
on it — see docs/DECISIONS.md D15 for the full record: what was rejected
(Postgres as a second container; Postgres+PostGIS bundled inside the single
container via a supervisor), what this costs (no database-level RLS, no
PostGIS, procrastinate needs replacing — all stated as a concern before this
was decided, and reaffirmed anyway, which is the user's call to make about
their own instance).

The one invariant-critical consequence: isolation between users now rests
entirely on the repository-layer scope (db.py's `Scope.select()`), not two
layers. CLAUDE.md's invariant #4 is revised accordingly. This is not a
downgrade-and-hope — `Scope` is built so an unfiltered query against a
user-owned table is structurally harder to write than a scoped one (there is
no method on `Scope` that returns one), and
tests/test_auth.py::test_scoped_session_blocks_cross_user_reads replaces the
old RLS proof with the same empirical standard: it doesn't trust the query
builder filters correctly because the code reads correctly, it registers two
real users and checks.
test_unscoped_session_can_see_every_user_when_misused is the deliberately
alarming companion — it demonstrates exactly what a reviewer must now catch,
since nothing else will.

Six real, non-obvious SQLite behaviours found and fixed by actually running
this against a real file, not assumed from docs:

- Foreign keys, ON DELETE CASCADE included, are OFF by default per
  connection — deleting a user silently left orphaned sessions/api_tokens,
  no error either way. Fixed with PRAGMA foreign_keys=ON on every connect.
- Transactions default to DEFERRED, which only takes a write lock on the
  first actual write — a real check-then-act race for invite redemption
  (two concurrent redemptions could both read used_count < max_uses as true
  before either commits). Fixed by disabling the driver's implicit BEGIN and
  issuing BEGIN IMMEDIATE ourselves — SQLAlchemy's own documented recipe for
  this, not improvised.
- DateTime(timezone=True) does NOT round-trip tzinfo on SQLite — a tz-aware
  datetime goes in, a naive one comes back out, and every
  `expires_at < datetime.now(UTC)` comparison in auth/service.py then raises
  TypeError. Fixed once at the Base level with a UTCDateTime TypeDecorator
  rather than per-column.
- Uuid(as_uuid=True) stores as 32-char hex with NO hyphens on SQLite, not
  str(uuid)'s hyphenated form. A test fixture that raw-inserted the
  hyphenated form left rows the ORM's own later UPDATE (via
  invite.used_count += 1's autoflush) could never match by primary key,
  updating zero rows and raising StaleDataError. Fixed by using .hex to
  match exactly what the ORM itself writes.
- BEGIN IMMEDIATE applies to every transaction, reads included — a
  long-lived test fixture that autobegins a transaction via a bare read and
  never explicitly closes it holds SQLite's exclusive write lock for the
  rest of the test, and a later scoped_session() call fails with "database
  is locked". Not an app-code bug (every real session block closes cleanly
  on exit), but real enough to document since the next person writing a
  test against the db_auth fixture will hit it too.
- Python's sqlite3 module deprecates its own implicit datetime adapter as of
  3.12 — silent today, warns on every raw-SQL datetime bind. Only ever hit
  test fixture code (the ORM path never uses it, confirmed by running the
  ORM-only health test with warnings promoted to errors and it stayed
  clean); fixed there with an explicit .isoformat() rather than left for a
  future Python version to turn into a real failure.

Also, since with_for_update() silently no-ops on SQLite (confirmed —
SQLAlchemy emits no SQL for it, no error either) rather than actually
locking anything: removed it from register()'s invite-redemption query and
corrected the comment to attribute the concurrency guarantee to BEGIN
IMMEDIATE, where it now actually lives.

One PR, not several, for the same reason PR #2 was: the migration, the
models, db.py, and the docs recording why are five views of one decision —
splitting them wouldn't make review easier, just disconnected. 552
insertions / 548 deletions across 17 files, most of it necessarily touching
what PR #2 shipped rather than net-new code.

Deliberately deferred, not solved here: PostGIS's replacement for spatial
storage, procrastinate's replacement for background jobs, and the
EXCLUDE USING gist constraint's replacement for component_installs — none
of those tables exist yet (Phase 1-2), so none of it is broken, and
docs/DECISIONS.md D15 records exactly what each future phase needs to
decide before it can be built. .gitea/workflows/deploy pipeline (PR #4,
built for the old 3-container Postgres compose stack) was closed as
superseded rather than merged; the single-container image build is
follow-up work, not part of this change.

Verified: ruff check, ruff format --check, and mypy --strict all clean.
13/13 pytest passing against a real SQLite file, including with
DeprecationWarning promoted to an error (confirms the sqlite3 adapter
deprecation fix actually holds, not just that it's quiet by default). Full
alembic upgrade -> downgrade -1 -> upgrade cycle run clean. alembic check
clean with no include_object filter needed at all now (SQLite starts with
nothing but what our own migrations create — no PostGIS/TIGER noise to
filter out in the first place). CI's exact migration command sequence
reproduced locally end to end before touching the workflow file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-21 15:42:44 -04:00

129 lines
7.2 KiB
Markdown

# apps/api
Python 3.12 / FastAPI / SQLAlchemy 2.0 async / Alembic / SQLite. See `docs/PLAN.md` for the
overall design and `docs/DECISIONS.md` (D15 especially) for why the database is SQLite and not
the Postgres+PostGIS setup Phase 0 originally shipped with.
## Layout
```
velodrome/
app.py FastAPI app factory
config.py Settings (env-driven, see below)
db.py Single engine + the repository-layer scope — read this first, it's the
load-bearing module for user isolation now that there's no RLS
ids.py UUIDv7 generation
models/ SQLAlchemy models
auth/ Password hashing, session service, FastAPI auth dependencies
api/v1/ Route handlers
schemas/ Pydantic request/response models
alembic/ Migrations. 0001_baseline.py creates the identity tables — no roles, no
RLS, no GRANTs, none of those concepts exist in SQLite.
tests/ pytest, against a real SQLite file, never mocked
```
## Why isolation is enforced in Python now, not the database
Phase 0 originally ran two Postgres roles (`velodrome_app`/`velodrome_auth`) with row-level
security as a database-enforced isolation layer. SQLite has no roles, no session variables, and no
policy engine — there is no database-level backstop anymore. This is the single most important
thing to understand before touching `auth/` or `db.py`:
- **`scoped_session(user_id)`** — for every query against a user-owned table once identity is
known. Yields a `Scope`, whose `select()` is the *only* way to build a query through it, and
every query it builds is pre-filtered to that `user_id` on any model that declares one. There is
no method on `Scope` that returns an unfiltered query — see `tests/test_auth.py`'s
`test_scope_select_rejects_models_without_user_id` for what happens if you try it on a model
that isn't user-owned (`Invite`, scoped by `created_by` rather than `user_id`, is the real
example used there).
- **`auth_session()`** — used *only* by `auth/service.py`, for the narrow set of lookups that must
happen *before* identity is known: login by email, a session by its token hash, an invite by its
code hash, plus the inserts that create those rows. Every one of those queries is an exact match
on a unique key, never an unfiltered scan — that discipline is what makes it safe to use a plain
session here instead of `Scope`.
- **`unscoped_session()`** — a plain session with no scoping applied at all, for health checks and
anything that never touches a user-owned table. `tests/test_auth.py`'s
`test_unscoped_session_can_see_every_user_when_misused` demonstrates, deliberately, what happens
if this gets used on a user-owned table instead of `scoped_session` — it sees *everyone's* rows.
That test exists to make the point vivid: reaching for `unscoped_session()` (or a raw
`auth_session()` query) against a user-owned table is a review-blocking mistake now, not a style
preference, because there's nothing else standing behind it.
If you're adding a new table with per-user data: give it a `user_id` column and query it *only*
through `scoped_session`. See CLAUDE.md's invariant #4.
## Two SQLite behaviours that don't match the defaults you'd expect
Both confirmed empirically against real `aiosqlite`, not assumed from docs — see `db.py`'s
`_configure_sqlite_for_concurrent_writers` for the fixes:
1. **Foreign keys, `ON DELETE CASCADE` included, are OFF by default per connection.** Without
`PRAGMA foreign_keys=ON`, deleting a user silently leaves its sessions/api_tokens behind
instead of cascading — no error either way.
2. **Transactions default to DEFERRED**, which only takes a write lock on the first actual write —
leaving a real check-then-act race window (e.g. two concurrent redemptions of the same invite
code both reading `used_count < max_uses` as true before either commits). Every transaction on
this engine issues `BEGIN IMMEDIATE` instead, which takes the write lock up front. One
consequence worth knowing if you're writing tests: a session that autobegins a transaction via
a bare read and never explicitly commits/rolls back holds that write lock until the session
closes — see the comment above `await db_auth.commit()` in
`test_scoped_session_blocks_cross_user_reads` for a real example of this biting a long-lived
test fixture.
Also worth knowing: `Uuid(as_uuid=True)` stores as 32-char hex **with no hyphens** on SQLite, not
`str(uuid)`'s hyphenated form. This only matters if you ever write a UUID into this schema via raw
SQL instead of the ORM (as the test fixtures do, to set up state without going through the API) —
use `.hex`, not `str()`, or the ORM's own later queries against that row won't match it. See the
docstring on `_seed_invite` in `tests/test_auth.py` for the failure this caused when it was
gotten wrong.
## Environment variables
| Variable | Used by | Notes |
|---|---|---|
| `VELODROME_DATABASE_URL` | The app and Alembic, both | e.g. `sqlite+aiosqlite:////data/velodrome.db`. One DSN — there's no separate migration role anymore since SQLite has no roles to separate. |
| `VELODROME_SECRET_KEY` | The app | Not yet used (arrives with the Bryton credential encryption in a later phase); declared now so the settings shape is stable. |
| `VELODROME_ENVIRONMENT` | The app | `development` / `test` / `production`. Gates the session cookie's `Secure` flag — see the comment in `api/v1/auth.py` before changing this condition. |
| `VELODROME_PUBLIC_URL` | The app | Used for the CSRF `Origin` check on cookie-authenticated mutations. |
## Running locally
```bash
uv sync --all-extras
export VELODROME_DATABASE_URL=sqlite+aiosqlite:///./velodrome-dev.db
uv run alembic upgrade head
uv run uvicorn velodrome.app:app --reload
```
`GET /api/v1/healthz` should return `{"status": "ok"}`; `GET /api/v1/docs` has interactive
Swagger UI outside production.
## Testing
```bash
uv run ruff check . && uv run ruff format --check .
uv run mypy --strict velodrome
uv run pytest
```
Tests run against a real SQLite file in a temp directory (never `:memory:`, which gives each
separate connection its own isolated database rather than one shared one, and never mocks for DB
behaviour, per CLAUDE.md). The suite runs the real Alembic migration at session start, not a
parallel schema-creation shortcut, so it's exercising the exact same path a real deploy uses.
The test worth reading first if you're new to this codebase is
`tests/test_auth.py::test_scoped_session_blocks_cross_user_reads` — it doesn't trust that
`Scope.select()` filters correctly because the code reads correctly; it proves it by registering
two real users and confirming a scoped read for one never returns the other's row, even though
both exist in the same table.
## A note on `alembic check`
CI's `migrations` job runs `alembic check` to catch drift between the ORM models and the actual
migrations. There's no `include_object` filter in `alembic/env.py` anymore — Phase 0's version
needed one to exclude PostGIS/TIGER's own pre-installed tables from the comparison, but SQLite
starts with nothing but what this app's own migrations create, so there's no foreign-table noise
to filter out in the first place.