acceptance: schema-driven invariant fuzzing for bundle configs - #5686
Open
radakam wants to merge 82 commits into
Open
acceptance: schema-driven invariant fuzzing for bundle configs#5686radakam wants to merge 82 commits into
radakam wants to merge 82 commits into
Conversation
Collaborator
Integration test reportCommit: 21d1ebf
8 interesting tests: 4 RECOVERED, 4 SKIP
Top 6 slowest tests (at least 2 minutes):
|
Contributor
Approval status: pending
|
denik
reviewed
Jun 26, 2026
The rename to emit_fuzz_config sorts before envsubst, so ruff I001 flagged the import block in mutate_fuzz_config_check.py.
The invariant suite runs on cloud, and there each seed's deploy/migrate/plan/ destroy round trip against a real workspace takes minutes, tripping the script's 180s per-seed SEED_TIMEOUT and reporting a false hang. The cloud run also adds no signal: drift checking is off unless FUZZ_CHECK_DRIFT is set, which it does not set, so it only re-asserts the no-panic property the local run already covers over the same seeds in seconds.
mutate mode rendered the curated base template with the harness-wide UNIQUE_NAME, so every seed in a run deployed the same bundle root and the same securable names into one shared fake workspace. Anything a seed left behind then read back as remote state for a later seed: the testserver never drops grants on delete, so seeds 34 and 51 of no_drift/mutate failed on a grant an earlier seed had created and destroyed. generate mode already scopes names per seed; do the same here. no_drift/ mutate now runs to seed 71, the real allow_duplicate_names bug.
…gets no_drift, migrate, continue_293, delete_idempotent and destroy_idempotent each carried their own copy of the config render (data copy, init script, envsubst), the destroy-on-exit trap, and the deploy + panic-scan + INPUT_CONFIG_OK sequence. Move all of it into acceptance/bundle/invariant/script.prepare, which the harness picks up for the whole subtree, so each script holds only the invariant it asserts. The helpers are functions rather than prepare-time side effects because the harness concatenates script.prepare outside the subshell it wraps the script in, so a trap installed at prepare time would belong to the outer shell. invariant_deploy takes the deploy log name as its first argument, since the targets log to LOG.deploy, LOG.deploy_initial and LOG.deploy.293. Incidental deltas, all confined to LOG* files (logged, not compared): continue_293 gains LOG.config and a traced cleanup destroy, and destroy_idempotent's trap logs to LOG.destroy rather than LOG.destroy_final.
Drop the comments that restated the line below them and condense the helper docs, keeping only the non-obvious parts: why invariant_setup must be called from the target rather than at prepare time, and what INPUT_CONFIG_OK signals to the fuzzer.
The fuzzer could pass while every seed was rejected, so a broken generator, schema or fixture looked identical to the CLI correctly rejecting random input. Assert that at least one seed deploys, and treat a generator crash as a bug rather than a rejected config. Also fixes classification and reporting the review turned up: - an unstubbed testserver route hit during plan or destroy was reported as a CLI drift bug; check for it before the INPUT_CONFIG_OK marker - repro hints were mangled by the harness env-var replacements; write them to LOG.repro, which is logged verbatim - cleanup skipped destroy unless deploy fully succeeded, leaking resources from a part-way deploy on a real workspace - to_yaml flattened nested lists ([[1,2]] -> [1,2]) and the loader could not read the empty containers it emits - gen_grants/gen_permissions emitted [] for types with no valid privilege, manufacturing the known empty-grants drift bugs instead of new ones - CatalogsCreate dropped connection_name, custom_max_retention_hours and managed_encryption_settings, so the next plan saw a phantom change - mutate + migrate only rediscovers what migrate/test.toml already excludes Drops the schema-type guard (gen_scalar already exits on an unknown type), the write-only FUZZ_CORPUS_DIR, the never-set FUZZ_RESOURCES, both unused argparse entry points and single-caller util.load_plan, whose inlining also restores the raw-plan dump on malformed JSON.
The name is the identity of both resources, so a fake that accepts an empty one stores a resource the CLI cannot find again: deploy reports success and then fails with "internal error: missing entry in state after deploy", and destroy fails with "internal error, missing in state". Verified against a real workspace that both APIs reject it with 400 INVALID_PARAMETER_VALUE, so the fuzz seeds that hit this were reporting a fake-server gap as a CLI bug. Also two harness fixes found while replaying seeds: - pass -count=1 to the fuzz tasks. Only the script reads FUZZ_*, so Go's test cache happily served one seed window's result as another's. - skip the no-seed-deployed assertion for a single-seed replay, where one rejected config is the expected outcome rather than systemic breakage.
Guard INPUT_CONFIG in invariant_cleanup: scripts run under set -u, and a caller that builds its own config leaves the variable unset. Split invariant_render out of invariant_setup so such a caller can override the render alone from its own script.prepare, which the harness concatenates after this file. Lift set -e around the deploy in invariant_deploy so the panic scan still runs when the deploy fails: a config the CLI rejects must not panic on the way out. A clean failure exits with the deploy's own code and prints no INPUT_CONFIG_OK.
no_drift, migrate and continue_293 each ran the same plan + panic-scan + verify_no_drift.py sequence. Move it into invariant_verify_no_drift so a caller whose config the server does not round-trip exactly can override the check alone. migrate now logs the plan to LOG.planjson like the other two.
#6075 replaces the sourced prologue.sh with acceptance/bundle/invariant/script.prepare, which the harness concatenates for the whole subtree, and turns the shared setup into helpers a child script.prepare can override. - Drop prologue.sh along with its FUZZ_SEED branch; no target script sources anything now. - Add fuzz/script.prepare, overriding invariant_render to emit the generated config and run the validate panic probe, and -- unless FUZZ_CHECK_DRIFT is set -- invariant_verify_no_drift with the plan-determinism oracle. - Drop SKIP_DRIFT_CHECK: no_drift and migrate no longer branch on fuzz mode. migrate now runs the determinism oracle in fuzz mode rather than skipping the drift check entirely. - Guard INPUT_CONFIG in migrate/script, which the fuzzer leaves unset.
The fuzzer clears the INPUT_CONFIG matrix, and scripts run under set -u, so the depends_on special case must not dereference it unguarded.
delete_idempotent and destroy_idempotent assert that a delete or destroy re-run against state that still references the removed resources succeeds. That holds regardless of how faithfully the fake server round-trips fields, so unlike no_drift and migrate these targets keep their real oracle under fuzzing, and they exercise the already-gone paths across generated resource types. Both derive workspace paths from test-bundle-$UNIQUE_NAME, which the fuzzer has to render the config with. Scope the per-seed unique name in the seed loop rather than inside emit_fuzz_config.py, where the rewrite never reached the shell, and name generated bundles after the same pattern as the curated configs. 200 seeds per target per mode run clean locally: 143/200 deploy in generate mode (14 testserver gaps) and 169/200 in mutate mode, with no bugs or hangs.
Generate mode adds no axis the curated delete_idempotent and destroy_idempotent tests lack. The already-gone delete path is selected by resource type, and those tests already run every type in the INPUT_CONFIG matrix; generate pins name and display_name, so it cannot vary the identifier the delete call uses, and its dangerous values only reach free-form strings no delete path reads. Mutate can land a dangerous value on the identifier and can reshape grants and permissions, so it keeps both targets. Leaves 5 leaf variants and drops the testserver coverage gaps generate produced for these two.
# Conflicts: # acceptance/bundle/invariant/script.prepare #6075 landed as a pure extraction, so main's script.prepare no longer carries the two deltas this branch had added to it. Keep main's version and re-add only the INPUT_CONFIG guard in invariant_cleanup, which this branch does need: the fuzzer clears the INPUT_CONFIG matrix and scripts run under set -u. The set -e lift around the deploy is not re-added. It is redundant here -- when a seed fails, fuzz/script already scans every LOG.* for panics before classifying it, so a config that panics on the way out is still reported as a bug rather than a rejection.
The seed loop wraps run_seed in `set +e` to capture the seed's exit code. The capped branch spawns a fresh `bash -euo pipefail`, but the uncapped fallback is a plain subshell that inherits `set +e`, so the seed ran without errexit: a config the CLI rejects continued past the failing validate and deploy to `echo INPUT_CONFIG_OK`, and that marker is what tells "deployed, then broke the invariant" from "config rejected". Every seed the CLI legitimately rejected was reported as a bug. macOS has no GNU timeout, so it always takes the fallback and was the only platform failing; FUZZ_SEED_TIMEOUT=0 selects the same branch and reproduces it anywhere. Seed 3 of generate mode (pipelines.*.root_path pointing at a path that does not exist) failed as a bug with the flag and passes without it.
This is the only combination that reaches reconcile on the Terraform-first path with a perturbed config, which is how the empty grants and secret-scope `level` bugs were found in the first place. The nightly (drift on, wide seed window) is expected to stop early on it until those fixes land: seed 7 empties a schema's grants list and hits #6039. The committed run (5 seeds, drift off) stays green, so per-PR CI is unaffected.
Keep the reason, drop the retelling: several of these had grown to four or five lines where one carries the point, and a few restated the line below them. No behaviour change. Verified by comparing against the previous revision: the Python files are AST-identical once docstrings are stripped, and the shell and toml files are identical with comment lines removed.
fuzz/script sat next to the invariant targets at ten times their size, which read as a fifth invariant rather than a harness over the other four. It keeps only what has to be bash -- the schema dump and seed_body, which must be a function so the target it sources sees the merged script.prepare helpers -- and run_fuzz.py owns the seed loop, the time budget, the per-seed cap and the outcome classification. All LOG.* formats are unchanged. Two behaviors change with it. The per-seed cap no longer depends on GNU timeout, so macOS and Windows enforce FUZZ_SEED_TIMEOUT instead of running seeds uncapped, and the SIGQUIT/SIGKILL pair goes to the seed's own process group so a hung CLI dies with it. Log scanning reads bytes, so a config carrying the invalid UTF-8 that gen_fuzz_config injects can no longer break classification itself.
It deploys with the pinned v0.293.0 binary first, and that version does not know many current fields and resource types, so the old CLI would reject most seeds before the current one ran.
The fuzzer is an orchestrator over the invariant targets, not an invariant of its own, but living under invariant/ made it look like a fifth target and it had to blank out the inherited INPUT_CONFIG matrix to work. It now sits at acceptance/bundle/fuzz/ and reaches the shared machinery explicitly: invariant/script.prepare roots configs/ and data/ at INVARIANT_DIR, which defaults to its own directory for the targets and is set by the fuzzer before it sources the helpers. The variable is exported because run_fuzz.py runs each seed in a fresh bash. test.toml only merges along the directory chain, so the engine pin, the ignore patterns and the server stubs are copied from invariant/test.toml.
The target list and fuzz description it gained belong in the fuzz README; this file is about the curated invariant tests. The outside caller is already documented where it matters, in the INVARIANT_DIR comment in script.prepare.
A route the testserver does not model is a coverage gap for a config nobody wrote by
hand, not a missing stub. Get that from the fuzz test's own test.toml -- a 501 [[Server]]
catch-all per method carrying a TESTSERVER_GAP marker that run_fuzz.py classifies on --
instead of IgnoreUnhandledRequests on testserver.Server, so the global unhandled-request
check keeps its teeth everywhere and no new public field or test.toml key is needed.
The catch-alls shadow no real route: wildcard patterns go to ServeMux, which matches
most-specific-first, and exact paths are looked up before the mux. No HEAD entry, because
ServeMux matches a GET pattern for HEAD too, so HEAD /{path...} conflicts with every GET
wildcard and panics at registration; the GET catch-all covers HEAD anyway.
The guard dated from when the fuzzer ran the invariant body with errexit off and classified seeds from a captured exit code, so a failed `bundle plan` left an empty LOG.planjson for verify_no_drift.py to choke on. Both callers now run under `bash -euo pipefail`: a failing plan aborts the seed before the check, and the default fuzz path replaces the check outright with the plan-determinism diff. A 40-seed FUZZ_CHECK_DRIFT=1 window reaches the guard zero times.
Review pass over the whole branch. No behaviour change to the fuzzer: the generator refactor was checked against the previous revision over 850 configs (400 generate seeds, 400 schema-aware mutate seeds, 50 destructive-only) and is byte-identical, and the committed run, both selftests and the curated invariant suite stay green. Two blocks belong in neither this PR nor the repo: - test-fuzz-cover built a -cover CLI and awk-formatted a per-package coverage table. It was scaffolding for arguing the fuzzer's value, not part of it, and nothing referenced it; it also owned the only use of coverage-fuzz.txt, so that .gitignore entry goes with it. - the nightly job's "Report failure" step resolved the PR behind main's HEAD and commented on it. That PR is already merged, and as the comment body itself said the failing seed is most likely pre-existing, so it notified an author who did not cause it. A failing scheduled job is already visible in Actions and LOG.repro is in the job log either way. Its pull-requests: write permission goes too, leaving the block identical to the other test jobs. Leftovers in the generator: - gen_resource took a schema argument it never read and re-assigned gen.unique to the value the constructor had already set; its remaining lines fold into gen_config. - it also duplicated resource_element from mutate_fuzz_config character for character. That helper now lives once, in gen_fuzz_config. - resource_types took both schema and gen when schema is always gen.root -- one caller wrote resource_types(gen.root, gen). - FUZZ_MODE/FUZZ_TARGET were defaulted in three places even though the test.toml matrix always sets them. run_fuzz.py's copy was the harmful one: a wrong default silently prints a wrong repro command. Comments: keep the reason, drop the retelling. Also fixes three that were wrong -- run_fuzz.py and emit_fuzz_config.py credited fuzz/script with exporting env vars that come from the matrix or the caller, and test.toml put FUZZ_TIME_BUDGET in script rather than run_fuzz.py. The note about the no-schema mutate path "keeping its exact RNG stream unchanged" described not churning an older revision; it now states why that path exists at all.
libs/testserver/catalogs.go and models.go go back to main's version, so this branch is confined to the fuzz harness again. Both changes are real fixes the fuzzer found, but they stand on their own and are being reviewed separately: - reject an empty catalog / registered model name, which the real backends do and the fake did not, so a seed that sent one stored a resource the CLI could not find again - echo connection_name, custom_max_retention_hours and managed_encryption_settings back from CatalogsCreate, so the next plan does not see a phantom change The committed 5-seed run, the curated invariant suite and the testserver unit tests all still pass without them, and a 60-seed window is unchanged: it stops in exactly the same place (migrate/mutate seed 19, the secret-scope level bug), so the fuzz test does not depend on either fix landing first.
Every seed failed on Windows, all six variants, and the "no seed deployed" guard caught it: 5 rejected, 0 deployed. Each LOG.check held the same UTF-16LE text -- "Windows Subsystem for Linux has no installed distributions" -- so the seed subprocess was never bash at all. The harness starts the test script with Go's exec.CommandContext(ctx, "bash", ...), and Go's LookPath searches PATH only, so it gets Git bash. Python's subprocess.Popen with a bare program name goes through CreateProcess, whose search order puts System32 ahead of PATH, and System32\bash.exe on the GitHub Windows runners is the WSL launcher stub. It exits non-zero with no distribution installed, which classify() then reads as a rejected config: no generator error, no panic, no INPUT_CONFIG_OK. shutil.which resolves against PATH the way Go does, so parent and child agree on which bash they mean. No guard for a missing bash: this script only ever runs from a bash script the harness launched by resolving the same PATH.
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
3 participants
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.
Bugs found by the fuzzer