feat(backend-service): publish the OpenAPI contract for /api and fuzz it weekly (#3325) - #3382
Merged
Merged
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
Xore
enabled auto-merge (squash)
September 27, 2026 02:29
Xore
force-pushed
the
oc/3325-backend-openapi-contract
branch
from
September 27, 2026 03:54
dac6a48 to
2e05801
Compare
… it weekly (#3325) Adds a machine-readable OpenAPI 3.1 contract for backend-service's /api surface -- 130 registered paths, 139 operations -- and the two gates that keep it from drifting, plus the weekly fuzz job the issue asked for. The contract is hand-maintained in a new src/openapi.rs rather than derived with utoipa. Almost every handler returns Json<Value>, and the property the issue wants pinned lives in the route table and the extractor signatures, not the response bodies, so a #[utoipa::path] on 138 handlers would learn only that GET /api/v1/events takes a Query<EventsQuery> and answers 401 without a service token. Three gates, failing in different directions on purpose: - contract_covers_every_router_route reads main.rs and fails if the router and the contract disagree about which (path, method) pairs exist. A route added without a contract row is the direction that rots, since it drops a fuzz target silently. - checked_in_contract_is_current fails if the committed openapi.json is not what the module renders. - quality.yml runs the same generator as a diff -u in both backend jobs, because a test failure says "stale" while the diff says what changed. Response bodies stay free-form on purpose: 138 transcribed shapes would be 138 chances to assert something the code does not enforce. The document pins the auth tier, the request parameters (including the enums the handlers actually validate), the declared status codes, and every response's media type. weekly-schemathesis.yml runs Mondays 04:23 UTC and on workflow_dispatch, booting the service against a single-node Elasticsearch service container -- without a cluster every ES-backed route answers 502 and the run measures nothing. Only the auth gate can fail the job; both schemathesis passes are advisory, because this API validates aggressively enough that a hard gate would be red on its first run and train everyone to ignore it. That gate is scripts/check-api-auth-tier.py, not schemathesis's ignored_auth. ignored_auth skips any operation the contract declares public, so demoting a live route in the document turns the check green while the route keeps serving anonymous callers -- reproduced on this service, where marking /api/v1/events public gave a passing run and one warning. The script reads the same document for expected answers but never trusts it about which routes are secured; that direction is the Rust test, which asserts the public set is exactly /healthz and /metrics. One row does not follow the file's pattern and is worth flagging in review: GET /api/v1/webhook-delivery, added by #3373 on main during this branch's rebase, is the only ES-backed read that declares no 502. Its handler catches the cluster error and answers 200 with `available: false` and the reason in the body, so a stated empty card beats a page that fails to render. Every neighbouring diagnostics row 502s because its extractor turns a dead cluster into a status code; copying the pattern here would have been a guess the source does not support. Running the job before shipping it found eight real omissions in the first draft of the contract, all now fixed: 415 and 422 from axum's Json<T> rejection on all 25 body routes, 400 from its Query<T> rejection, four services routes answering JSON errors declared as text/plain, a text/plain export declared as JSON, 404 from the correlations and reporter-stats lookups, and 422/501 from the report renderer's own error mapping. None of them are reachable from the router's source -- nothing in main.rs mentions 415 -- so the builders now add the extractor-level statuses instead of leaving each row to remember. The passes report those two finding classes by name so a regression in the document is visible even though it cannot fail the job. Against a booted service the document is now clean: 136 secured operations refuse an anonymous caller, and the authenticated pass finds nothing against the contract itself. Refs #3325
…3325) The rebase onto #3388 pulled in main's /livez and /readyz, which the contract test caught as registered-but-undocumented. Documented both, and widened the public-route invariant in the same test to name all four public routes instead of prefix-matching two. Zizmor: this branch introduces weekly-schemathesis.yml, which #3388 never saw, so it kept floating action tags and expanded a workflow_dispatch input inline into a run block. Pin both actions and route max_examples through env like every other value in that run block.
…nto the library being transcribed by hand, and that needs a shape the crate did not have: `#[utoipa::path]` annotations live on the handlers, and the second binary that prints the document cannot see a first binary's modules. So the modules move into `src/lib.rs`, which already existed for exactly this reason, and the route table moves with them so the document is generated from the same `Router` the process serves. The move is mechanical on purpose. No module, type or function is renamed or split, and the ~320 `crate::` references the handlers make resolve to the same items as before, because `AppState` and the probe handlers landed in the same place the crate root used to be. `main.rs` keeps what is genuinely a process: the environment, the #2183 boot gate, state construction, the listener. `allow_unauth_dev_from_env` becomes `pub` because `main.rs` now reaches it through the library rather than declaring it in the crate root. One test changes, and only in what it reads: `contract_covers_every_router_route` parsed `.route(...)` out of `src/main.rs`, which no longer holds the route table. It reads `src/lib.rs` now. Every assertion is unchanged -- including the `registered.len() > 100` guard that would have caught a stale path -- so the gate is exactly as strong, pointed at the file the code moved to. 544 passed, 0 failed, 1 ignored: same as before the move. `openapi.json` is byte-identical, which the checked_in_contract_is_current gate asserts.
…3325) Replaces the 1820-line hand-written table in src/openapi.rs with utoipa/utoipa-axum, so the contract is derived from the annotations and the route table rather than transcribed alongside them. The 128 /api paths and 4 health probes come out byte-identical, which the the_document_is_byte_stable gate asserts against the committed openapi.json. The route table is now an index, not a second copy. Every entry moved from `.route(path, method(handler))` to `.routes(utoipa_axum::routes!(handler))`, in the original order, one handler per call: `routes!` panics if two handlers in a single call share an HTTP method, and the table has methods that repeat across adjacent entries. Paths and methods now come from `#[utoipa::path]`, so they cannot disagree with the annotations the document is built from. `#[derive(OpenApi)]` is deliberately not used. It tags every operation with a module-path tag; utoipa-axum's OpenApiRouter does not, and the committed document's tag list is per-area, not per-module. `OpenApiRouter::default()` rather than `new()`, because `new()` fills info.contact and info.license from utoipa's own Cargo.toml. The `axum_extras` feature stays off so the annotations are authoritative rather than inferred from extractor types. 141 handlers annotated, `inline(...)` throughout -- no components/schemas and no $refs anywhere in the document. The two `Option<Json<T>>` bodies and the `Option<T>` parameters cannot express `pattern`/`maxLength`/`minimum`/ `maximum` or inline enums at container level through utoipa's derive, so 16 named parameter-shape types live in a new src/contract.rs behind a `contract_schema!` macro; STORE_NAMES moved there with them and is re-exported from openapi.rs for the handside allowlist test. A post-generation transform in render() reconciles the document with the committed one. Six steps, each documented at the point of implementation and counted in the module doc: merge the middleware 401, derive `operationId` and `tags` as pure functions of the path, read `Option` as an absence rather than a null (this is also what turns `x-optional-body` into `required: false`), strip the descriptions utoipa derives from doc comments, pin the document-level literals, and emit `parameters: []` on operations that take none. contract_covers_every_router_route is rewritten, not just repointed. It parsed `.route(...)` calls out of the source, and that form no longer exists. The two doors that remain open are now the two it closes: the BYPASSING_ROUTES list (`.route(`, `.route_service(`, `.nest_service(`) must not appear in src/lib.rs, because OpenApiRouter inherits all three as pass-throughs that serve a route with no OpenAPI operation; and every `#[utoipa::path]` in src/**/*.rs must be routed. Both directions were exercised rather than assumed -- injecting a bare `.route("/api/v1/sneaky")` and removing an existing `routes!` entry each fail with the intended message. One intentional line of difference in openapi.json. The serviceToken security-scheme description reads "lib.rs's require_service_token" where it used to read "main.rs's", because Stage 1 moved that middleware out of the crate root. Publishing the old string would state something untrue about where the check lives. That is the only byte that changed, in 296852. 546 passed, 0 failed, 1 ignored: the 544 baseline plus the endless-stream invariant and the new byte-stability test. `cargo build --release` succeeds (the Dockerfile copies src/ wholesale), and clippy --lib --all-targets is clean.
… module move broke (#3325) The rebase onto #3400 brought main's fixes in beside #3325's move of the handler modules and the route table out of `src/main.rs` and into `src/lib.rs`. Three things were red as a result. None of them is a behaviour regression; all three are places where a gate reads a file, and the move changed which file. `zizmor` reported a blocking `artipacked` on this branch's own `weekly-schemathesis.yml`: an `actions/checkout` without `persist-credentials: false`. #3388 added that to every checkout in the tree and #3400 finished the job, but this workflow was added after that work and never met it. Fixed the way every other one is -- the `with:` block, and the version comment normalised from `# v7` to the `# v7.0.1` that the other 38 checkouts in the tree spell. No rule was added to the ADVISORY allowlist; the gate now reports only the allowlisted `dangerous-triggers` finding, which is what it reported on main. `scripts/tests/test_3316_image_boot_smoke.py` asserted that each path the boot smoke probes is registered by looking for `.route("<path>"` in `src/main.rs`. Two moves broke that string, not one: the table moved to `src/lib.rs`, and then #3325's own commit replaced every `.route(path, method(handler))` with `.routes(utoipa_axum::routes!(handler))`, which takes the path off the handler's `#[utoipa::path]`. So there is no path literal in the table at all any more. It now reads the paths declared by the annotations, which is where they live, and keeps the literal form so a table written in plain axum is still checked rather than passing on an empty scan. The scan is guarded on `assertGreaterEqual(len(declared), 4)`, and both patterns require a leading `/`, because `openapi.rs`'s own `BYPASSING_ROUTES` holds the literal strings `".route("` and friends and a looser pattern reads that Rust string as a route registration. `scripts/tests/test_3315_image_revision.py` pinned the Rust normalizer to the shared revision corpus by asserting on `src/main.rs` too, and `normalize_revision`, `REVISION_UNKNOWN` and the test module that reads the corpus all moved to `src/lib.rs`. Repointed at the file they moved to. It reads one named file rather than scanning the tree: an assertion against all of `src/` dumps every module into the failure message, and if the next move relocates these the assertion should fail with the file it looked in. Both test repoints are verified to still fail for the right reason -- renaming the `/livez` annotation, dropping the `/readyz` annotation, and pointing the corpus `include_str!` at a PNG each fail with the intended message, so the gates are not weakened into passing.
…ives (#3325) The rebase's own conflict resolution made these three claims false, and the file is one this branch already owns, so they are corrected here rather than left for the next reader to trip over. "the source of truth is the operation table in src/openapi.rs" was true when the table was hand-written there and stopped being true when it was generated from the `#[utoipa::path]` annotations and the route table in src/lib.rs. "contract_covers_every_router_route reads src/main.rs" named a file that no longer holds a route table. "Fix those by editing the row in operations()" named a function that no longer exists -- the edit point is the annotation on the handler, or the shape it names in src/contract.rs. The counts were stale in the same way: the section claimed 129 registered /api paths and 138 operations, and the committed document has 132 paths and 141 operations -- 128 /api paths and 137 /api operations behind the token, plus the four public probes. Now stated as counted, with the public set named. No behaviour and no generated contract changes: `openapi.json` is byte-identical before and after, which `the_document_is_byte_stable` and `checked_in_contract_is_current` both assert.
Xore
force-pushed
the
oc/3325-backend-openapi-contract
branch
from
September 27, 2026 12:05
91dbb08 to
c1b2e45
Compare
This was referenced Sep 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3325
The
/apisurface had no machine-readable description, so nothing could saywhether a route was secured, which parameters it took, or what it answered.
This adds a checked-in OpenAPI 3.1 contract for all 130 registered paths /
139 operations, three gates that stop it drifting, and the weekly fuzz job
the issue asked for.
The contract
Hand-maintained in a new
src/openapi.rs(with alibtarget so it istestable) rather than derived via
utoipa. Almost every handler returnsJson<Value>, and what the issue wants pinned — the auth tier, the acceptedparameters, the status codes — lives in the route table and the extractor
signatures, not in the response bodies. A
#[utoipa::path]on 139 handlerswould have learned that
GET /api/v1/eventstakes aQuery<EventsQuery>andanswers 401 without a token, and nothing else.
Response bodies stay free-form on purpose. Transcribing 139 shapes would be
139 chances to assert something the code does not enforce; the document pins
the auth tier, the parameters (including the enums the handlers actually
validate), the declared status codes, and every response's media type.
Three gates, failing in different directions
contract_covers_every_router_routemain.rsregisters a(path, method)the contract lackschecked_in_contract_is_currentopenapi.jsonis not what the module rendersquality.ymldiff -u openapi.json -The first is the one that matters: a route added without a contract row drops
a fuzz target silently, and that is the direction that rots.
The drift test earned its keep during review.
mainmergedGET /api/v1/webhook-delivery(#3373) while this branch was open, and thegate failed on it immediately with the exact row to add.
Weekly fuzz job
.github/workflows/weekly-schemathesis.yml— Mondays 04:23 UTC, plusworkflow_dispatchwithmax_examplesandbase_url. It boots the serviceagainst a single-node Elasticsearch service container: without a cluster every
ES-backed route answers 502 and the run measures nothing. Three passes
(unauthenticated, authenticated, auth-tier) with artifacts and cleanup.
Advisory by design. Both schemathesis passes are
continue-on-error; onlyscripts/check-api-auth-tier.pycan fail the job. This API validatesaggressively enough that a hard gate would be red on its first run and train
everyone to ignore it.
The auth gate is a repo script rather than schemathesis's
ignored_auth,because
ignored_authskips any operation the contract declares public — somarking a live route public turns the check green while the route keeps
serving anonymous callers. Reproduced on this service: marking
/api/v1/eventspublic produced a passing run and one warning. The scriptreads the document for expected answers but never trusts it about which routes
are secured; that direction is the Rust test, which asserts the public set is
exactly
/healthzand/metrics.Running it before shipping it
found eight real omissions in the first draft, all fixed: 415 and 422 from
axum's
Json<T>rejection across all 25 body routes, 400 from itsQuery<T>rejection, fourservicesroutes answering JSON errors declaredtext/plain, atext/plainexport declared JSON, 404 from thecorrelationsandreporter-statslookups, and 422/501 from the reportrenderer's own error mapping. None are reachable from the router's source —
nothing in
main.rsmentions 415 — so the builders add extractor-levelstatuses rather than trusting each row to remember.
Against a booted service the document is clean: 136 secured operations refuse
an anonymous caller, and the authenticated pass finds nothing against the
contract itself.
One row that breaks the pattern
GET /api/v1/webhook-deliveryis the only ES-backed read that declares no502. Its handler catches the cluster error and answers 200 with
available: falseand the reason in the body — a stated empty card beats apage that fails to render. Every neighbouring diagnostics row 502s because its
extractor turns a dead cluster into a status code. Copying the pattern here
would have been a guess the source does not support, so the row carries a
comment saying so.
Verified
cargo build --all-targets0 warnings · 536 tests pass ·cargo clippy -D warningsclean · diff gate clean ·check-doc-paths-exist.pyclean ·actionlintclean on the new workflow ·check-ai-attribution.pyclean · document validates against OpenAPI 3.1.0.Note for review
src/main.rsis deliberately untouched — the drift test reads its130
.route(registrations, so any edit there is a place the gate can startdisagreeing with the contract.