Add Schemathesis API fuzzing to the nightly run - #932
Conversation
Exploratory harness for #923: runs the real app (lifespan, Alembic migrations, disposable Postgres database, real bearer-token login) under uvicorn in a background thread and fuzzes all 141 operations from openapi/openapi.json with Schemathesis 4.x. Spike-only code, not wired into CI. Requires 'uv pip install schemathesis' (deliberately not added to pyproject.toml yet) and 'docker compose up -d db'.
Deep exploratory runs want fresh inputs, so KITARU_FUZZ_RANDOM=1 turns off derandomize; the default stays derandomized for reproducibility. The exception capture path was hardcoded to an absolute scratch path.
Generates requests from openapi/openapi.json in both schema-conformant and schema-violating modes and sends them to the real app on a disposable PostgreSQL database, asserting only that the server never answers 5xx. One session-scoped database is shared across the run, which makes "was this rejected?" depend on run order while leaving "did it crash?" well-posed, so negative_data_rejection is not run. Response schema conformance is gated behind an env var until #930 lands. The four defects this found are listed in KNOWN_FAILURES and skip, so a filed crash does not mask the rest of its operation.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
# Conflicts: # uv.lock
#930 fixed the 422 response schemas, so the KITARU_FUZZ_SCHEMA_CONFORMANCE gate on test_response_matches_schema no longer has a reason to exist. The full suite now passes with the check on: 282 passed, 0 skipped.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
tests/server/test_fuzz_api.py imports schemathesis, which lives in the fuzz dependency group. Without it, ty cannot resolve the import and the job fails with four unresolved-import diagnostics.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc41499d42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The module-level skipif only applies after the module imports, and the regular test matrix does not install the fuzz group, so pytest crashed during collection with ModuleNotFoundError. importorskip before the schemathesis imports skips the whole module instead, matching test_otel.
The nightly workflow caches .hypothesis/examples and uploads it as the replay artifact named in the auto-filed issue. database=None left that artifact empty for the api surface, so a randomized failure could not be replayed as the issue instructs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0dabc6486
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
FuzzServer.start() created the database first and only reached stop() through the fixture's teardown, so a failed migration or login left one disposable database and a running uvicorn thread behind per attempt. Startup now cleans up after itself, and stop() tolerates a server that never finished booting. Regression tests cover both failure points. Also spell out the extras in the module's direct-run command, since the module now skips silently without the fuzz group installed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ad870b113
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
GitHub Actions sets CI, which makes tests/conftest.py load the ci profile with database=None, and the fuzz settings inherit that. The api nightly therefore never wrote .hypothesis/examples, leaving the replay artifact empty. The two sibling fuzz recipes already select the nightly profile.
2 participants
Adds Schemathesis fuzzing of the REST API as a third entry in the existing nightly fuzz run, alongside
fuzz-importersandfuzz-mcp. Closes the exploration in #923.Ready for review. The suite was held in draft until #930 fixed the 422 response schemas; that landed in #948, the schema-conformance check now runs by default, and the full suite passes with it on.
What this found, and the fixes it now verifies
The spike behind this PR turned up four server defects and one spec gap. All four have since been fixed and merged, and this branch has been updated onto the fixed
develop:versionpath parameter overflows anINTEGERcolumn and 500s instead of 404ingPOST /api/v1/loginvalidation errors omit theerrorfield their own declared 400 body requiresIdempotency-Keyacross routes crashesPOST /api/v1/api-keys, because the stored row is decrypted before the fingerprint check that would have rejected it| #930 — every hand-raised 422 body violated its own documented response schema | #948 |
KNOWN_FAILURESis now empty as a result, and nothing is gated, so all 141 operations run both checks.The suite then independently confirmed all four fixes hold, including at the depth that originally found them: a full 400-examples-per-operation randomized run over every operation passed with no 5xx and no captured exceptions. #927 and #931 both reproduce reliably at that depth on the pre-fix code, so this is a real check rather than a run that merely failed to look.
With #948 merged,
test_response_matches_schemaruns unconditionally. Before it, that test failed on 9 of 18 operations in a sample; on the merged branch all 141 pass.Reviewer notes
The story of the change is mostly a series of decisions about what not to assert, so that is where the review attention belongs.
Why the app runs under a real server
tests/server/fuzz_server.pyboots the app under uvicorn in a background thread rather than handing it to Schemathesis's in-process ASGI transport, which is what the docs show. Those transports enter and exit a client context around every request, and doing that reruns the app's lifespan, Alembic migrations included, for every generated example. At the depths this suite runs, that is tens of thousands of migration runs. One server per pytest session runs the lifespan once, and the full pass takes under four minutes.If the implementation is wrong here, the symptom is not a wrong answer, it is a suite nobody can afford to run.
Why the only assertion is "no 5xx"
One database is shared for the whole session, because creating a database per generated example would be unusably slow. That means rows an earlier operation writes are visible to a later one.
That is fine for crash hunting and fatal for rejection testing. I confirmed this rather than assuming it:
negative_data_rejectionfailed on three operations in a full run that all passed when run alone, because a resource an earlier operation created made a later "invalid" request succeed. Whether a given input gets rejected depends on run order. Whether the server crashed does not.So negative generation stays on, and schema-violating input is still sent, which is what found #927. Only the rejection assertion is dropped. This is the decision I would most like a second opinion on.
The
KNOWN_FAILURESescape hatchKNOWN_FAILURESintests/server/test_fuzz_api.pymaps a method and path to the issue that owns a filed, reproduced defect there. Without it, a known crash fires on the first example and hides everything behind it in that operation. It is currently empty, which is the healthy state; it held ten entries until the four fixes landed. If it silently grows and stays grown, the suite is quietly measuring less than it appears to.The exception-capture wrapper
ExceptionCaptureAppwraps the app and records unhandled exceptions. It exists because a 500 response body is the stringInternal Server Errorand nothing else, so without it a failure names an endpoint and not the exception behind it. Every traceback quoted in the four issues came from it. It sits outside Starlette's error middleware, so the response the client receives is unchanged; the one behavioral difference is that uvicorn no longer logs the traceback itself, since the wrapper consumes the exception.Reproduction
Needs Docker for the database. From the repo root:
Expect
282 passedin roughly six minutes: 141 operations through the 5xx check and 141 through the schema-conformance check, nothing skipped.To see it catch something, check out any of the four fix commits' parents and rerun. The affected operation fails, and the failure message carries the captured exception rather than a bare 500.
For the nightly configuration (400 examples per operation, randomized, roughly 12 minutes):
Local checks run
just checkpasses on the merged branch.just testis clean apart fromtests/typescript/, which fails in my worktree onpnpm --filter @zenml-io/kitaru buildbecause node modules are not installed there. That is unrelated to this change and I have not touched it.Follow-ups deliberately left out
links, so chains come only from inference offLocationheaders and are shallow. It passed, which proved nothing, so it is not in this PR.components.securitySchemesis empty and no operation declaressecurity:, so the spec never tells any tool that authentication exists. Schemathesis'signored_authcheck is inert until that changes. Everything here runs as one admin account.filterquery parameter. A recursive JSON-encoded filter language in a query string, on every list endpoint, and the highest-signal parser target in the API. Schema-driven generation found nothing there, which is not evidence it is sound; it wants a grammar-aware fuzzer.sessions,session-runs,nodes, annotations, tasks, and imports all take untypedobjectbodies, so generation produces mostly trivial values. They want hand-written strategies.One note on depth
Findings scale steeply, which is the argument for nightly depth over a shallow PR gate:
#927 does not exist at 25 examples per operation. #931 does not exist at 50. A cheap gate would have found neither of the two defects that turned out to matter most.