[Feature] : API ENDPOINTS PR 1: Foundation and scaffolding#1130
Conversation
cfsmp3
left a comment
There was a problem hiding this comment.
Note the two high complexity errors (validation.py and status.py), also see Claude review below.
Reading the rview, clearly H1 must be addressed before we can merge or things will break.
The rest could be addressed (possibly are) later on in the stack.
But let's make sure we can merge and test each PR in order but not at the same time, i.e. merging this diff shouldn't break the system, even if by itself it doesn't do anything useful (since it's scaffolding).
Claude review follows:
HIGH (blocker):
- H1 — ApiToken model added with NO migration. The api_token table won't exist on real MySQL. Tests pass only because tests/base.py does create_all from models → masks the gap. PR2's auth endpoints break at runtime. #1117 had this migration (d4f8e2a1b3c7); dropped in the split. Must add it, chained off master head c8f3a2b1d4e5.
MEDIUM (carryover, code unchanged):
- ~770 lines of security middleware merge untested — and with no routes yet, the before_request hooks never even fire in this PR. Deferred to PR2/3. Risk noted.
- Rate limiter unbounded memory (no hard cap, eviction only every 100 req).
- Auth timing oracle (no-candidate path skips argon2 verify → leaks whether a prefix exists).
- _get_client_ip comment wrong (ProxyFix means remote_addr is from XFF).
LOW/NIT: run-level missing→fail path untested; stale is_dummy_row "DEPLOYMENT PREREQUISITE" docstring; N+1 footgun in the batch_get_run_data wrappers; pytest conftest.py inert under nose2; generic 429 handler hardcodes wrong limits.
8e813fc to
9950853
Compare
9950853 to
beb4fe9
Compare
|
New / still open:
|
214fbd2 to
b9a645a
Compare
b9a645a to
d1e2392
Compare
|



[Feature]
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
API foundation and core infrastructure (PR 1/6)
Summary
First of six stacked PRs introducing a JSON REST API for the sample platform
(supersedes #1117). This PR adds only the foundation — blueprint, middleware,
the token model, shared services, and schemas. No route endpoints are added
here; they land in PRs 2–6. Because there are no routes yet, the blueprint
hooks don't fire for real requests, so middleware is tested over HTTP starting
in PR 2 (auth) and PR 3 (system/runs).
What's included
Blueprint & wiring —
mod_apimounted at/api/v1(run.py);ProxyFix(x_for=1)sorequest.remote_addrreflects the real client IP behindnginx (needed for IP-based rate limiting).
Middleware — Bearer-token auth with
@require_scope/@require_roles(authfailures are returned after rate limiting, so unauthenticated floods are still
limited); structured JSON errors; fixed-window rate limiting (5/15min token
creation by IP, 20/min writes, 120/min reads by token) with
X-RateLimit-*headers; security headers; declarative request/query/path validation.
Rate-limit state is per-process — see Known caveat #1 (PR 6).
Token model (
models/api_token.py) — opaquespci_-prefixed tokens with256 bits of entropy. Only a SHA-256 hash is stored, verified with
hmac.compare_digest(constant-time). A slow password KDF (argon2/bcrypt) isdeliberately not used: it adds latency to every request and buys nothing against
brute force on a high-entropy random secret — the standard approach for API
tokens. User passwords continue to use passlib/bcrypt in
mod_auth.Status service (
services/status.py) — single source of truth forrun/sample status, batch-loaded to avoid N+1. Correctly treats "no output when
output was expected" as a failure (comparing actual result files against the
expected
RegressionTestOutputrows) rather than a silent pass.Schemas & utils — shared pagination/error schemas and response helpers.
Existing files touched
run.py: register the API blueprint;ProxyFix(x_for=1).mod_auth/models.py: adduser.github_login(used by the API to authorizefork-run triggers from PR 3; populated at OAuth login in PR 6).
requirements.txt: addmarshmallow..pycodestylerc: exclude.venv*.Migration
d4f8e2a1b3c7createsapi_tokenand addsuser.github_login.github_loginis bundled here (rather than a later migration) because PR 2 references it.
Chains from the current alembic head
c8f3a2b1d4e5.Testing
Unit tests for the token model, status service, and utils; existing suite
passes; isort/pydocstyle/pycodestyle/mypy --strict clean.
Next
PR 2 (#1131): the
/auth/tokensendpoints.