perf(ci): cut test suite wall time via teardown, poll, and dep-install fixes - #3404
Open
singlerider wants to merge 79 commits into
Open
perf(ci): cut test suite wall time via teardown, poll, and dep-install fixes#3404singlerider wants to merge 79 commits into
singlerider wants to merge 79 commits into
Conversation
HttpCompare.compare_body ran DeepDiff with ignore_order=True on line-lists. When two bodies are highly similar but differ on many lines (the exact shape wildcard detection produces on large catch-all hosts), DeepDiff's pairing is quadratic in the differing-line count and holds the GIL, stalling entire scans for tens of minutes. Short-circuit to 'different' when the multiset difference of two line-lists exceeds web.http_compare_max_differing_lines (default 500) before invoking DeepDiff. Caps worst-case comparison at a couple seconds. Dict inputs (XML/JSON) keep the full diff path unchanged. Fixes #3339
defaults.yml added the key; the strict pydantic WebConfig model must accept it or test_defaults_yml_validates_against_schema fails.
- dict inputs still take the DeepDiff path (guard must not short-circuit them) - reordered identical line-lists stay equal (multiset is order-insensitive) - at-threshold vs over-threshold boundary around max_differing_lines - http_compare_max_differing_lines config value reaches the helper
Four focused tests, all failing against the current guard: filtered_lines_not_counted asserts a subject differing only in lines the baseline already marked dynamic still compares equal. The guard counts those lines before DeepDiff excludes them, so it returns a false "changed" on any page with many nonces. bounds_dict_bodies asserts an XML/JSON body settles in bounded time. xmltodict.parse succeeds on ordinary HTML, so those bodies are dicts and skip the list-only guard into the unbounded quadratic path. threshold_boundary makes the at-threshold case one DeepDiff calls a match, so the assertion can distinguish the guard firing from the lists simply differing. null_threshold_config covers an explicit null, which resolves to None and raises TypeError on the comparison. Replaces the previous boundary assertions, which expected False on both sides and passed with the feature deleted.
The guard counted raw list items, which broke two ways. It ignored ddiff_filters, the paths the baseline established as dynamic. Those lines are excluded from the DeepDiff that follows, but they were still counted toward the threshold, so a page with enough per-request nonces reported its body as changed on every probe. That generates findings rather than suppressing them. It also required both sides to be lists, but xmltodict.parse succeeds on ordinary HTML, so those bodies arrive as dicts and dropped into the unbounded quadratic DeepDiff the guard exists to prevent. _leaf_counts walks either shape, skips any subtree whose path is already filtered, and counts leaves, so the threshold sees the same content the diff will.
http_compare_max_differing_lines is Optional[int], so an explicit null is valid config. dict.get returns None for present-but-null, not the default, and the threshold comparison then raised TypeError on every body compare.
…l fixes The suite spent most of its wall time waiting rather than working. Each fix targets a specific source of idle time: test harness: - FastShutdownHTTPServer overrides thread_target to poll every 5ms instead of socketserver's 0.5s default, so every httpserver fixture teardown no longer pays half a second - stop_server busy-wait drops from 100ms to 5ms per check - remove an unconditional 0.5s sleep in test_manager_scope_accuracy that waited on module init the scan already guarantees scanner: - main scan loop polls at 2ms while events are flowing and backs off exponentially to 100ms when idle, instead of a flat 100ms sleep that throttled every event batch helpers: - cache the CloudCheck instance and the wordninja LanguageModel, both of which were rebuilt per scan despite identical inputs - search_format_dict short-circuits when no placeholder is present and uses a single compiled regex pass instead of one str.replace per kwarg deps installer: - batch every module's pip deps into one resolver pass before the per module loop, so pip resolves once instead of once per module. Modules with custom pip_constraints are excluded since they must resolve against their own set.
ruff format rejected the class sitting between two import groups. It belongs after the imports regardless, so move it there rather than padding the original spot with a blank line.
distro_tests.yml invoked pytest with no -n, so all six distro containers ran the entire suite serially at ~32 minutes each, while tests.yml ran the same suite in parallel at ~15. The distro matrix, not the test matrix, was setting the wall time for the whole PR. Use the same -n/--dist loadgroup flags tests.yml already uses. worker_count.py bounds the count by cores and available RAM, so the container gets a worker count it can actually feed.
The distro matrix ran the full 151-module suite on all six containers. Module tests exercise module logic, which does not vary by distro and is already covered five times over by the Python matrix in tests.yml. Six redundant full-suite runs were setting the wall time for every PR. The question the distro matrix exists to answer is narrower: does bbot install, resolve deps, and run here. Scope it to the files that actually probe that surface (e2e, cli, depsinstaller, command, files, python_api, config, dns) and let tests.yml own module coverage.
test_cli_args was a single 292s test wrapping roughly forty independent cli._main() invocations. xdist cannot split one test, so it set a hard floor on wall time no matter how many workers were available. Each section already set argv, called, and asserted in isolation, so the split is mechanical. --install-all-deps gets its own test since it dominates the runtime and now occupies a worker without blocking the other three sections.
Both tests walked all 116 scan modules in a single body, so xdist could not split them and they sat at 259s and 246s respectively, setting a floor on wall time alongside test_cli_args. Parametrize both over a module shard so each case loads its own subset. The union of the shards is the full module list, verified for both the scan and output sets, so no module loses coverage. Four shards measured better than eight: each shard pays scanner init once, so past a point the fixed cost outweighs the split.
worker_count capped at core count, so GitHub's 4-core runners ran 4 workers while every worker sat waiting on subprocesses, local HTTP servers, and DNS. The suite is I/O-bound, so cores are the wrong ceiling. Double the cpu ceiling and leave the memory bound untouched, since memory is what actually OOM-kills a run. A 4-core runner now gets 8 workers; the 16-core/16GB case the memory guard was written for still resolves to 16.
The core-cap guards pinned the old ceiling and failed once workers were oversubscribed. Assert against OVERSUBSCRIBE rather than a literal so the guards track the constant instead of restating it. The memory-cap and never-zero guards are untouched: memory is still the bound that prevents an OOM-killed run.
This reverts commit df3cdb8.
This reverts commit eb3c58d.
xdist only parallelises within a single runner, so the suite was pinned to one 4-core machine per Python version and roughly 3800s of work could not go faster than about 950s of wall time. Add BBOT_TEST_SHARDS/BBOT_TEST_SHARD, sharding on the sorted nodeid at collection time, and fan the matrix out to four shards per version. Every test lands in exactly one shard, verified against a full collection: 869 tests, union of the four shards, no overlap and nothing dropped. Deselected tests are reported through pytest_deselected so the counts in each job stay honest about what ran.
setup_before_prep monkeypatches neo4j.AsyncGraphDatabase, which imports neo4j, but the pip_install lived in setup_after_prep and therefore ran later. The test only passed because something earlier in the session had already pulled the package in, an ordering dependency that sharding exposed as ModuleNotFoundError. Move the install ahead of the patch so the test stands on its own. Verified against an environment with neo4j uninstalled.
Sharding added a second matrix dimension, renaming every job from "test (3.10)" to "test (3.10, 0)". The protecc ruleset on dev requires the un-sharded context names, so those four contexts were never reported and PRs hung on Expected forever. Add a test_gate job matrixed on python-version only, named to emit the exact required contexts, gated on the sharded matrix result.
This reverts commit 4326dcd.
This reverts commit 65a3ccc.
…s workers" This reverts commit b2ae7b6.
This reverts commit 3f31ace.
Every xdist worker got its own BBOT home, and dependency installs are keyed on that home, so all 126 modules' deps were installed once per worker instead of once per run. That was the single largest fixed cost in the suite and it scaled with worker count. Split the home into two halves: - cache, tools and lib move to BBOT_SHARED_DEPS_DIR when set. These hold install state, downloaded binaries and libs, all of which are keyed on content and safe to share between concurrent scans. - scans, temp and the rest stay per worker, so scan output and the sessionfinish cleanup remain isolated exactly as before. The module hash now keys on the deps directory rather than bbot_home, otherwise a shared install would still be invalidated per worker. Concurrent installs into the shared dir are serialized with an flock on the deps dir, and setup status is re-read after acquiring it, so a worker that waited picks up what the holder just installed instead of redoing it.
The shared deps dir was only wired into ConfigAwareHelper, but BBOTCore
exposes its own cache_dir/tools_dir/lib_dir off home, and those are what
set BBOT_TOOLS for the ansible playbooks. Installs therefore targeted a
tools dir under the per-worker home that nothing had created, and every
ansible download task failed with:
dest '/root/.bbot/tools' must be an existing dir
Move the redirect to BBOTCore.deps_home so core and helper resolve the
same paths, and have the helper and the module hash read it from there
instead of recomputing it. temp and scans stay on home.
Writing the module preload cache is now atomic (temp file plus rename)
since the cache dir is shared between concurrent scans and a reader must
never see a half-written pickle.
Both tests hardcoded the tools dir as home/tools, which only held while
deps lived under the scan home. With the install-once dirs redirected,
BBOT_TOOLS is the shared path and both failed:
assert '/tmp/.bbot_test_shared/tools' == '/tmp/.bbot_python_api_test_gw1/tools'
assert '/tmp/.bbot_test_gw3/tools' in [...]
Assert against helpers.tools_dir so the check follows wherever tools
resolve. test_python_api keeps its home coverage by asserting scans_dir
still lands under the configured home, which is the half of the split
these tests were really pinning.
The nuclei module tests built template paths from the per-worker home,
but nuclei downloads its templates into the tools dir, which now resolves
to the shared location. The module updated templates under the shared
dir and was then handed a path under the worker dir:
Could not find template '/tmp/.bbot_test_gw3/tools/nuclei-state/templates/...'
Could not run nuclei: no templates provided for scan
Export BBOT_TEST_TOOLS_DIR alongside the shared dir and build the three
template paths from it, so the tests reference the same tools dir the
module installs into. The XDG env paths stay on the per-worker home,
since those intentionally point at throwaway dirs.
Two problems, one root cause: the test called _prep() when it only ever reads class attributes off the module instances. The async-method check was dead. not_async was reset at the top of each iteration of the module loop while the assert sat outside it, so only the last module's methods were ever examined. Hoist the list out of the loop and report the offending qualnames. _prep() also runs setup() on every module. The ones that talk to a service spend their whole connect-retry budget failing against a host that isn't running: rabbitmq alone takes 29s of a hardcoded 30-attempt loop, with postgres and mysql behind it. Worse, _prep() then drops the failures from scan.modules, so the modules most likely to regress were deleted before the loop could inspect them. load_modules() instantiates every module without dialing anything, which is all the assertions need. The test now covers 151 modules instead of 110 and takes 1.9s instead of 47.6s.
The per_domain_only assertions never ran. The loop only probed modules watching URL, but all four per_domain_only modules (azure_tenant, emailformat, skymem, viewdns) watch DNS_NAME, so the branch was unreachable and its assertions had rotted since the per_host_only rework in 0111db7: _per_host_tracker has not been written to since that commit; dedup state moved to _incoming_dup_tracker. The reason string also changed from "per_domain_only enabled and already seen domain" to "module has already seen it (per_domain_only=True)". Probe DNS_NAME modules too so the branch is reached, and assert against the state and reason string the implementation actually produces. Verified negatively: the old _per_host_tracker assertion fails when the branch is genuinely executed. Separately, _prep() already calls setup_modules() internally, so the explicit setup_modules() call after it re-ran setup() on every module. The test only reads dedup state off each module, so load_modules() is enough. Modules that dial an absent service (rabbitmq, postgres, mysql) were each burning their full connect-retry budget, twice. 128 modules loaded vs 99 after setup_modules() pops the failures, so coverage rises: 75 modules probed, 4 per_domain_only exercised where previously 0. Test body drops from ~12.3s to 0.9s.
Every trufflehog invocation forked a second copy of itself and paid the Go package init twice. The re-exec comes from the overseer self-update library: main() hands control to overseer.RunErr(), which starts the real program as a child process with OVERSEER_IS_SLAVE=1 in its environment. Both parent and child run the full init chain, and that chain is not cheap: 1616 packages, dominated by go-re2 at 488ms compiling the detector regex set. Measured on the pinned 3.97.0 binary: a stdin scan of a single line takes 4.6s wall, of which the scan itself is 1.8ms per trufflehog's own "finished scanning" log. GODEBUG=inittrace=1 shows 3232 init lines for a real run against 1615 for an invalid flag, exactly 1616 packages initialized twice, and ps shows the second pid appearing 2.5s in. trufflehog exposes --local-dev for precisely this, it skips overseer and runs the program directly. --no-update already told it not to fetch updates, so the overseer wrapper was pure overhead in every bbot code path. bbot spawns one process per event, so the cost is paid per CODE_REPOSITORY, FILESYSTEM, HTTP_RESPONSE and RAW_TEXT event, not once per scan. Verified: - Output equivalence across every source mode bbot uses (stdin, filesystem, git, plus aws/slack/http secret fixtures): DetectorName, DecoderName, Raw, RawV2, Verified and SourceMetadata identical, same returncode, same result counts. - The one field that differs under git, repository_local_path, embeds the pid of the scanning process and already differs between two consecutive plain runs. Nothing in bbot reads it. - Binary is not modified and no update is attempted, checked with and without --no-update; mtime and size unchanged, zero update-related stderr. - Per invocation 4.6s -> 2.4s, a 1.95x speedup measured over 6 source modes. - In the test suite, subprocess time 103.2s -> 59.7s over 20 invocations. - NEGATIVE CONTROL, same file with the change stashed: 63.0s vs 42.7s, 5 passed both ways. This is the proof the change is load-bearing. - test_module_trufflehog, test_module_badsecrets, test_module_web_report, test_module_github_codesearch, test_modules_basic all pass. - ruff check + ruff format --check clean. This is product code, so real scans get the same 2x on every trufflehog call, not just CI.
as_completed defaults to max_concurrent=20, so the wall time of this test was exactly sum(sleeps)/20. With 1000 sleeps drawn from random.random() that sum is ~496s, giving a measured 25.3s of pure idling. Nothing was being exercised during it: the coroutines only slept. Divide the sleeps by 100. The scheduling behavior under test is unchanged, only the constant shrinks. Verified across divisors 1, 20, 50, 100 and 200 that completion stays out of submission order and all 1000 results still come back distinct, so scaling does not collapse the test into a trivially ordered drain. While here, cover the branches this test was walking past. It only ever checked the happy path at the default limit, leaving three untested: - max_concurrent is now asserted to be saturated, not merely respected. Peak in-flight count is exactly the limit, confirmed stable at 1, 5, 20 and 50 over 25 runs each. - max_concurrent=None (the unlimited path) was never entered. - a raising coroutine is yielded rather than swallowed, and does not abort the remaining tasks. as_completed has explicit exception handling that nothing exercised. Also assert completion order differs from submission order, which is the actual contract and was previously unchecked because results went straight into a set. Both new assertion groups verified load-bearing by mutation: forcing the limit to 3 fails on peak == limit, and swallowing raised tasks fails on len(yielded) == 4. test_async_helpers 25.33s -> 1.82s. Full file 20 passed, ~45s -> 21.4s.
liquidsec
requested changes
Aug 28, 2026
liquidsec
left a comment
liquidsec
left a comment
Collaborator
There was a problem hiding this comment.
Solid direction, and several of these are real wins. The per-worker port stride, the wait_for_container deadline with the OOM hint, and the not_async fix in test_module_loading (the old list was reset inside the loop, so that assertion was close to a no-op) are all clear improvements.
Six things below block merge for me. There is a longer list of smaller notes I'll add separately so this comment stays actionable.
1. install() deadlocks the event loop (installer.py:161-175)
_install_lock() at line 221 calls fcntl.flock(f, fcntl.LOCK_EX) synchronously, and install() awaits self._install(*modules) inside the with block. install() is itself awaited from Scanner._prep() at scanner.py:687, on the event loop.
So scan A takes the lock, hits an await inside install_core_deps / _batch_pip_install / pip_install, and yields. Scan B in the same process (its own DepsInstaller, its own fd, and flock treats separate file descriptions independently even within one process) enters with self._install_lock(): and blocks the only event loop thread. A can never resume to release it. That is a permanent deadlock for any embedder running two scans, which the Python API supports today.
Single-scan is not fine either: on a cold cache, every worker blocks its whole loop, in-flight DNS and HTTP included, for as long as another process spends installing.
_update_templates in this same PR gets this right (await self.helpers.run_in_executor_io(self._acquire_template_lock)). The installer needs the same treatment: acquire in the IO executor, or hold the lock only across sync work.
2. compare_body short-circuit is not verdict-preserving (diff.py:273-286)
Counter subtraction counts repetitions. DeepDiff with ignore_order=True does not. So the fast path is not a fast path, it is a different comparison, and it runs first:
>>> DeepDiff(["x"]*1000 + ["end"], ["x"]*100 + ["end"],
... ignore_order=True, view="tree", threshold_to_diff_deeper=0).keys()
[] # old compare_body -> True
>>> sum((Counter(a) - Counter(b)).values())
900 # new code -> False, before DeepDiff runsThat is not a contrived input. response.text.split("\n") on a real HTML page yields hundreds of identical blank and whitespace-only lines, and any probe that shifts their count by more than 500 now reads as "body differs."
Everything downstream treats a body difference as evidence: paramminer_headers, bypass403, url_manipulation and lightfuzz emit findings off it, and web.py:_probe_wildcard_host uses the same compare_body to decide wildcard-ness, so a wildcard host gets classified as non-wildcard and every 404 becomes a URL event. This needs to bail only when the shortcut and DeepDiff agree, e.g. compare distinct-value sets rather than multiset counts.
3. Three shared-state races from the new shared deps dir
The shared dir is the right idea, but three writers reach it without holding the lock that protects it.
nuclei.py:87. _update_templates() acquires and releases the template lock, and then setup() does shutil.rmtree(self.nuclei_state_dir) outside it. Worker A finds _templates_installed() False precisely because worker B is mid-extract under the lock, and A wipes B's tree (plus B's config and cache) while B is still writing. Both retry, B's update produces a half tree, and setup returns "Failed to install nuclei templates after retry". The _templates_installed() check and the rmtree need to be inside the same lock.
installer.py:144. _discard_corrupt_fact_cache() runs in __init__, before any locking exists. Process A holds the install lock mid-ansible_run while ansible's jsonfile plugin writes fact_cache/localhost; process B reads it mid-write, json.loads raises, and B unlinks it. A's playbook then loses its facts and anything gated on ansible_facts['os_family'] sees an undefined variable. It is also one-shot, so an entry corrupted after __init__ is never repaired, which is the case the comment says it guards against.
installer.py:505. The shutil.rmtree(self.ansible_artifact_dir / ident) is not in a try/finally. The loop above it does e["event_data"]["res"]["msg"], and a runner_on_failed event without a msg key (routine for unarchive and get_url, which report stderr) raises KeyError first. Artifacts used to live under data_dir/<module> and got wiped by that module's next run at line 468; now they are under the shared dir keyed by a fresh uuid4, so nothing ever reclaims them.
4. The shared test dir is never cleaned up (conftest.py:55 vs :519)
os.environ.setdefault("BBOT_SHARED_DEPS_DIR", str(BBOT_TEST_SHARED_DIR)) redirects cache_dir, tools_dir, lib_dir and temp_dir (core.py:78-92), but pytest_sessionfinish only does shutil.rmtree(BBOT_TEST_DIR). /tmp/.bbot_test_shared now outlives every session and every branch.
That is not just disk. module_preload_cache lives there, and a stale preload cache across a module rename is exactly the phantom module-not-found failure that is miserable to diagnose, with no rm -rf in the loop anymore to clear it. setup_status.json, the fact cache, 13k nuclei templates and every #{BBOT_TEMP} build tree (massdns, medusa, node) accumulate the same way.
Keeping it across sessions is a defensible choice given the whole point is install-once, but then it needs an explicit invalidation story, and the preload cache in particular needs to key on something that changes when modules do.
5. _prep() -> load_modules() drops setup() coverage for every module (test_modules_basic.py:540, :348)
_prep() ran setup_modules(), which awaited _setup() on all ~110 loaded modules. That made test_module_loading the only place a module whose setup() raises (bad config access, missing attribute, un-awaited coroutine) got caught tree-wide. After this change only class attributes are read, so such a module passes here and fails only in its own module test, or for a module without a triggering fixture, not at all.
The rewritten assertions in both tests are better than what they replace, so this is about the setup coverage specifically. If the target is the rabbitmq/postgres/mysql connect-retry budget, gate those three rather than dropping setup for the other ~107.
Note
test_manager_scope_accuracy_correct failing here is the known flake, not this PR.
The py3.13 job on PR #3404 (job 98941202149) ran 6h0m18s and was killed by the workflow limit. The scan itself finished in under a second: the debug artifact shows "Finished websocket module test" and "No unfinished tasks detected", then "Cancelling 2 orphaned tasks after websocket" and nothing further. Three of four xdist workers went idle and the run never completed. The test served on a task and cancelled it in check(). Server.close() does not close synchronously, it spawns an internal _close() task which is what eventually resolves closed_waiter. The fixture teardown in base.py cancels every remaining task and gathers them, so it cancelled that internal task mid-flight. wait_closed() awaits closed_waiter under asyncio.shield, so once its resolver is cancelled the shield swallows the cancellation and the await can never complete. The gather then blocks forever. Reproduced deterministically outside pytest, 5/5 runs wedged with the server task cancelled while a connection was open, matching the 2 orphaned tasks the CI artifact reports (Server._close and the server coroutine). Fixed by awaiting serve() directly instead of wrapping it in a task, and shutting the server down through close() plus wait_closed(). That drains the internal task before teardown runs, so no orphan is left to cancel. Verified zero orphaned tasks remain after the test, where previously there were two. Shutdown runs in _execute_scan rather than check() because the fixture and the test body run on different event loops, and awaiting the server from check() raises "attached to a different loop". All assertions unchanged.
The websocket hang (2f40328) was one instance of a general defect. Fixture teardown cancels every leftover task and then does an unbounded `asyncio.gather` on them. A task that absorbs its cancellation, an await shielded from it whose resolver was itself cancelled first, never completes, so that gather blocks forever. Nothing caps it. The workflow passes `--timeout 1200` together with `-o timeout_func_only=true`, and per pytest-timeout that setting evaluates the timeout against the test function body only, ignoring fixture time. Teardown is therefore uncovered, which is how a 20 minute cap silently became the 6 hour job limit on job 98941202149. Wait with a timeout instead, and warn with the surviving tasks when they outlast it. Abandoning a stuck task is strictly better than wedging the worker: the process is torn down at session end regardless. Verified with a module test that leaves a deliberately wedged orphan. On the current tree it passes in 15.7s; with this change stashed the same test returns no result at all and is killed at 240s despite `--timeout 150`, confirming both the hang and that the func-only timeout cannot catch it. No assertions changed, nothing split. Regression: 57 passed under `-n 2`.
The benchmark.yml change from 3aa43a7 cannot be pushed: the available OAuth token lacks the `workflow` scope, so the remote rejects any push whose net diff touches a workflow file. That single mid-stack commit was gating 11 unpushed commits, two of which cure a 6 hour py3.13 hang. GitHub evaluates the net diff of the push rather than each commit, so restoring the file forward puts the workflow back to its origin state and lets the rest of the stack land. No rebase, no history rewrite. Reapply once the token has `workflow` scope: gh auth refresh -h github.com -s workflow git revert 37161606a
test_module_kafka was the #2 slot in the suite at 101.8s, but the local warm run is 13.2s. The gap is image pull: CI has no pre-pull step, so docker run pulls implicitly on every run, and the test started two containers totalling ~785MB (wurstmeister/kafka 468MB plus zookeeper:3.9 317MB). Kafka has not needed zookeeper since KRaft went GA. One apache/kafka-native:4.1.2 broker at ~150MB replaces both, so the cold pull drops by ~635MB and the second container bring-up disappears entirely. Readiness was also wrong. wait_for_port_open plus a fixed sleep(1) only proves docker bound the port, which it does at container create time while the broker is still booting; the fixed sleep was papering over that race. Now it probes an actual produce round-trip via the existing wait_for_container helper, on a separate bbot_readiness topic so the topic under assertion is untouched. Measured, same machine, images removed first so both paths pull cold: before 207.7s -> after 57.5s (3.6x) Warm: 13.2s -> 4.3s. check() and its assertion are byte identical. Verified the failure path stays bounded: against a dead broker it raises in 10.1s rather than hanging.
38a82bf replaced the port wait with an aiokafka produce round-trip to close a real readiness race. That probe lives in setup_before_prep, which runs BEFORE scan._prep(), and _prep() is what pip-installs a module's deps_pip. On CI aiokafka is therefore not importable yet, so every one of the 5 python jobs errored with ModuleNotFoundError at collection of TestKafka. It passed locally only because the dep was already in the venv. Go back to wait_for_port_open(9092). That is sufficient here: is_port_open only reports ready once a connection survives its settle window, and the KRaft broker opens its listener as the last step of boot, so the docker proxy race that motivated the produce probe is already covered. Measured against the real broker, probing at the instant each method reports ready and immediately attempting a real produce: port_open 1.39s, api_versions 0.87s, metadata 0.94s, all three followed by PRODUCE OK. Held 0/5 failures with the port probe under 2x-cpu load. The consumer in check() still imports aiokafka, which is correct, that runs after _prep(). Verified with a scoped control that blocks aiokafka for exactly the setup_before_prep phase, reproducing the CI condition: pre-fix tree fails with the identical ModuleNotFoundError, post-fix tree succeeds in 2.41s. Test passes 3/3 consecutively at ~5.7s. No assertions touched.
timeout_func_only=true scopes the 1200s timeout to the test body only, so time spent in fixture setup and teardown is never counted. That is exactly where this suite's worst wedges have happened: the websocket hang (2f40328) burned 6h0m18s and was killed by the workflow limit, not by pytest, because it wedged in fixture teardown cancelling orphaned tasks. The orphan-cancel bound (52e41fa) fixed that specific hang, but the blind spot itself is still open, and every module test does its real work in the module_test fixture rather than in the body. Measured the semantics directly rather than trusting the flag name. With timeout_func_only=true and --timeout 3: an 8s sleep in fixture setup passes, an 8s sleep in teardown passes, only a body sleep is caught. With timeout_func_only=false all three are caught, setup and teardown as ERROR. The timeout also becomes cumulative across phases, so the budget has to clear the slowest whole test, not just its body. Confirmed with a 2s setup plus 2s body against --timeout 3: passes under the old setting, trips under the new one. Worst real test across all five green py jobs is test_cli_args at 150.9s to 189.6s wall including setup, against a 1200s budget, so the margin is 6.3x. Nothing is near the line. Verified: bbot/test/test_step_1/ under the new setting, 298 passed, zero timeouts. badsecrets plus webbrute_shortnames plus modules_basic, 11 passed. ruff check and ruff format clean. No test logic touched, no assertions changed, nothing split.
dns_regexes_yara compiled a Python regex object per target and then only ever read .pattern back off it. The compiled objects were never matched against anything: their single consumer, dns_yara_rules_uncompiled, splices the source string into YARA rule text, and YARA does its own compilation. So every target paid a full regex compile whose only product was the string that went into it. That is invisible on a normal scan and brutal on a big one. Profiled test_huge_target_list (10,005 targets) under cProfile: scan._prep spent 16.3s in excavate setup, 16.1s of that in _generate_dns_regexes, and 12.4s of that inside regex._compile for 10,032 patterns that were immediately discarded. Split the generation in two. _generate_dns_regex_patterns builds the source strings, _generate_dns_regexes compiles them for dns_regexes (which really does match, via oauth.py and helpers.re.findall_multi), and the YARA property now takes the uncompiled strings. Equivalence is exact, not approximate: the old path emitted re.compile(p, re.I).pattern and the new one emits p. Verified those are byte-identical across 10,013 targets including punycode, underscores, dashes, mixed case, IPs and multi-label public suffixes. Rule text for the single, multi and huge target shapes is unchanged, and dns_regexes still yields compiled objects (10,005 of them, all with .finditer). Note the rule-dict hash is not stable run to run, before or after this change: dns_strings derives from a set, so ordering varies and the $dns_name_N numbering shifts with it. Confirmed pre-existing by hashing twice on the unmodified tree. Not introduced here. test_huge_target_list 7.65s -> 3.36s. It sits on gw2, the binding worker in the py3.13 job (599s busy, 0s slack), so this comes off the critical path rather than off a worker that was already idle. Verified: test_step_1 302 passed, test_module_excavate 52 passed, test_regexes 6 passed. ruff check and ruff format clean. No assertions changed, no tests split.
Reverts a49a86d. Not a correctness retreat: that commit is still right, and the reasoning and measurements in its message stand. The push credential is a gh OAuth token with scopes gist, project, read:org, repo and no workflow scope, so any push whose net diff touches .github/workflows is rejected outright. a49a86d edits tests.yml, and the perf work now sits on top of it, so the workflow commit blocks a change that has nothing to do with workflows. Confirmed GitHub judges the net tree diff rather than per-commit: with this revert on top the same push is accepted. Reverting rather than reordering because rebase is off limits on this branch, and this keeps a49a86d and its evidence in history. Same precedent as 3c1125d for benchmark.yml. TO RESTORE (one time, needs a human at a browser): gh auth refresh -h github.com -s workflow git revert 6ed6e058e && git push origin perf/ci-suite-walltime
MinimalWordPredictor.predict() linear-scanned every entry in the model, calling str.startswith on all of them, then sorted the full match list. Each loaded model holds ~2.1M words, so a single predict() cost ~165ms regardless of how few words could possibly match. test_module_webbrute_shortnames makes 30 predict() calls, one per hint per fuzz strategy. Measured inside the scan: 9.20s of the 24.7s scan was predict(), and cProfile attributed 63M startswith calls and 12.4s of tottime to that one comprehension. Now each model is bucketed once by the first two characters of every word, and predict() scans only the matching bucket. Prefixes shorter than the key length fall back to the full scan, so behavior is unchanged for them. The index is built eagerly in setup() rather than lazily, because predict() runs in the cpu executor and concurrent callers would otherwise each rebuild it. Class-level defaults cover the unpickled instances, which never run __init__. Output is exactly identical, not merely equivalent: 3222 comparisons across both real models (the module test's own prefixes, edge cases including empty, non-ascii, uppercase and punctuation, plus 500 sampled prefixes) at top_n 250/25/1 produce byte-identical result lists. Ties preserve their old ordering because the sort remains a stable sort on frequency over a candidate set kept in model insertion order. predict() 9.20s -> 1.72s, index build 0.48s and 0.53s. Test wall time 28.4s -> 25.4s locally. Verified: 14 passed across test_module_webbrute_shortnames, test_module_iis_shortnames and test_module_webbrute. ruff clean.
TestNucleiTechnology ran 21.9s locally / 38s in CI. Traced the whole cost to a single nuclei subprocess (19.4s of the 21.9s), then timestamped nuclei's own stderr phases against a standalone apache-flavoured server: 0.4 280 templates loaded 1.0 automatic scan clustered, begins 6.1 tech-detect finishes (the phase the test actually asserts on) 21.3 scan completed Three 5s stalls account for 15s of the 21s: kafka-topics-list (9092) twice and CVE-2021-44521 (cassandra 9042), each dialing a closed port and waiting out a read timeout. The fixture is a pytest-httpserver on one HTTP port, so no tcp-protocol template can ever match. That time buys zero coverage. Corrects two findings from the previous audit run: 1. Narrowing `templates` does NOT help. TestNucleiTechnology runs in `-as` (automatic scan) mode, where nuclei selects templates from wappalyzer fingerprints and ignores `-templates` for execution. Measured: loading 38 templates instead of 280 still executes 280 and still takes 22.2s. 2. The earlier 73s baseline was measured at `-rate-limit 10`, inherited from the parent class. TestNucleiTechnology does not inherit that override, so it runs at the 150 default. Real baseline is 21-22s, not 73s. Adds `etypes` as a real module option mapping to nuclei's `-exclude-type`, defaulting to "" so behavior is unchanged for every existing config. The test opts into `etypes: tcp`. Result: TestNucleiTechnology 21.9s -> 7.6s, whole file 78.8s -> 62.1s, 9 passed both before and after. The TECHNOLOGY assertion is untouched and still passes, since tech-detect completes at t=6.1 well before the excluded templates ran. Also passes `-timeout self.scan.http_timeout`. bbot never sent `-timeout`, so nuclei silently used its own 10s default and `web.http_timeout` was ignored by this module alone while http, webbrute, telerik and ntlm all honour it. This is a config-fidelity fix, NOT a perf win: A/B at 3 runs each showed no wall-time effect (default 23.7/23.7/23.8s vs -timeout 3 at 23.6/23.7/23.7s) because the tcp dialers do not observe it. The default value passed is 10, identical to nuclei's own default, so nothing changes unless the user set http_timeout. Both flags are pinned by asserting on the captured command line. Verified negatively: dropping either flag from the builder fails the test.
…loads The py3.14 job failed with all 9 nuclei tests erroring in scan._prep() on "Failed to install nuclei templates after retry". The log shows 66 template update attempts, 0 successes, 33 wipes and 33 hard failures for what should have been a single install. py3.10 through 3.13 passed on the same commit. Two defects in the setup() repair path, both amplifiers rather than the originating fetch failure: The repair branch treated "no templates on disk" as the stale-marker corruption it was written for, so a plain download failure took the wipe and re-download path. nuclei had produced no files, so there was nothing corrupt to repair, and the retry just doubled the request count against a source that was already failing. Every one of the 33 module setups paid two downloads instead of one. _run_template_update now returns its classified outcome and the repair is skipped when the update reported failure, so a bad fetch fails after one attempt. The wipe also ran outside the template lock that 79c361a added, while the update it guards runs inside. shutil.rmtree removes the whole nuclei-state dir, so a worker entering repair could delete the tree another worker was mid-extract into, turning one worker's failure into its neighbours'. The update, verify, wipe and retry sequence now lives in a single _ensure_templates that holds the lock across all four. The lock file sits in tools_dir, not under nuclei-state, so the wipe cannot destroy it. The healthy path is unchanged: an update that lands templates still returns before any repair, and the stale-marker case still wipes and retries exactly once. Both behaviours are regression-pinned and verified negatively. Restoring the wipe outside the lock fails test_nuclei_repair_wipe_holds_the_lock with "state dir was wiped without holding the template lock"; removing the fail-fast guard fails test_nuclei_failed_download_does_not_wipe_and_refetch with "a failed download must not trigger a second fetch". No assertions were removed or weakened. Full file: 11 passed, up from 9.
…ptors excavate.setup() calls find_subclasses() on every sibling module to collect their ExcavateRules. That used inspect.getmembers(), which getattrs every name on the object, including BaseModule's ~20 properties. One of them, memory_usage, walks the module's own __dict__ via get_size(). setup_modules() runs all module setup() coroutines concurrently through as_completed(), so while excavate introspects a sibling, that sibling is still assigning its own self.<attr> in setup(). The get_size() walk then loses the race and raises "dictionary changed size during iteration", which hard-fails excavate for the whole scan. Observed on py3.10 in CI (12 occurrences in one job) while 3.11-3.14 were green: pure timing, nothing version-specific. find_subclasses only ever wants classes, and classes live in namespace dicts, so reading vars() off the instance and its MRO gets the same result without invoking a single descriptor. Snapshots each namespace under retry so the read itself cannot tear. Ordering is now name-sorted rather than getmembers' incidental sort, and first definition wins on override, which matches the previous MRO-flattened behavior. Verified equivalent against the old implementation on 303 BaseModule classes (0 mismatches), the excavate class (12 rules), and ParameterExtractor (10 rules). Race test pinned and negatively verified: the old path raises 53/3000 iterations, the new path 0/3000, and restoring getmembers() fails the new test.
The lock-free precheck added in b9bdb4e only helps a worker that needs nothing at all. A worker needing one uninstalled module still took the exclusive install.lock, and the holder installs its entire module list under a single hold, so the waiter paid the holder's whole chain. Measured on CI run 33447872808 (py3.13, green): one worker ran a serial ansible chain from 22:50:30 to 22:51:44 covering 18 modules (portscan 16s, retirejs 12s, gowitness 13s, medusa 18s). All four workers went silent for that window and reported PASSED within 3s of each other. The released waiter then logged "already done" for every module in 60ms; it blocked 74s to do nothing. Across the run, three or more workers were stalled simultaneously for 274s of 605s wall. Two changes make the wait proportional to what the caller actually needs: - install() polls with LOCK_EX|LOCK_NB instead of blocking, and after each observed publish rechecks _all_deps_satisfied. A waiter whose deps land mid-chain returns without ever taking the lock. Anything still unresolved falls through and installs under the lock as before. - _install() publishes setup_status per module rather than once in the finally. Progress was previously invisible until the whole chain ended, so there was nothing for a waiter to observe. The blocking flock was also called from async code, stalling the event loop for the duration; polling with asyncio.sleep yields instead. This is why the pre-fix regression test hangs rather than merely failing when the holder is in-process. _install_lock is now unused and removed. Verified: waiter returns in 1.73s vs 6.51s blocked pre-fix, with the holder as a real subprocess. Negative check on the old implementation fails with "waiter blocked 6.51s". Unsatisfied waiters still take the lock and install (asserted separately). test_modules_basic.py and test_presets.py 38 passed. test_depsinstaller::test_depsinstaller fails locally on the clean tree too (ansible/sudo), confirmed by stashing.
_batch_pip_install() resolves every module's pip specs in one pass, then install_module() immediately ran pip again for each module individually. Those repeats install nothing: pip re-resolves, finds the package present, and exits. Measured on a CI runner they are ~0.6s each of pure subprocess overhead, 22 calls in the py3.13 job, and they run inside the exclusive install lock so every other xdist worker waits behind them. install_module() now skips its pip call only for specs _batch_pip_install() actually installed in this session, tracked in _batch_installed. Scoping it to the batch's own specs rather than a general "is it satisfied" probe keeps pip_install's --upgrade semantics intact: a spec the batch did not cover, a stale cache entry whose package has since vanished, and force_install all still take the normal install path. Verified: - cold path, 4 modules with uninstalled deps: 4 per module pip subprocesses before, 0 after, identical succeeded/failed sets - negatively verified against the pre-fix tree, which shows the 4 calls - force_install and unsatisfied specs still invoke pip - test_depsinstaller.py matches the clean tree exactly (only the known environmental ansible/sudo failure) - test_modules_basic.py + test_presets.py: 38 passed
The holder installs its whole module set under one lock hold and publishes setup_status per module, so a waiter is released only once its own modules have been walked. _install() walked them in caller order, which interleaves modules needing nothing but pip with ones that compile from source. _batch_pip_install() has already satisfied every pip spec before the loop starts, so a pip-only module is pure bookkeeping. Walking it behind medusa, portscan, retirejs, gowitness and dnsbrute strands its waiters for the length of those builds. Measured on py3.13 run 33527062147: the batch pip pass completed at 15:41:29 but kafka's status was not published until 15:42:49, because kafka sat behind the ansible chain in list order. aiokafka had been on disk for 80s. The kafka test was credited 96.0s; it was blocked, not slow. Same for mongo, nats, mysql, rabbitmq, neo4j, sqlite, postgres and web_report, all pip-only and all published in the final 600ms of the chain. Sort cheapest-first so every pip-only module publishes before any build starts. The sort is a stable partition, so relative order within each group is unchanged and the succeeded/failed sets are identical. Verified: partition exact and stable over all 149 modules; the 15 modules classified heavy match the 15 ansible spans in the CI log exactly; unknown module names are preserved rather than dropped; with install_module stubbed, every pip-only module now publishes before the first heavy build (kafka at 2.2% of chain wall time, previously last). Negatively verified by stashing: pre-fix the first heavy build starts at index 5 while cheap modules keep publishing through index 31.
dotnetnuke, lightfuzz, generic_ssrf and host_header all sleep 5s in finish() before their final interactsh poll. The guard only checks that interactsh is enabled and registered, which happens in setup() regardless of whether the module ever sends a probe. A scan that loads one of these modules but produces no events still pays the full 5s, and the scan loop cannot exit until finish() returns. Profiling `bbot -m dotnetnuke` showed 6.2s of a 10.6s run parked in epoll, with a single 5.29s gap between "Completed finish()" and "Completed final finish()". Sampling modules_finished confirmed dotnetnuke as the module holding the scan open. This is one of 34 CLI invocations in test_cli_args and was the dominant cost in that test on a warm runner. Gate the sleep on the canary registry being non-empty. Each of these modules resolves incoming interactions through that dict and returns early on a miss, so an empty registry means every possible callback would be discarded anyway. The dict is written at the probe sites themselves, so it is non-empty exactly when a probe went out, which is exactly when the settle window has meaning. `bbot -m dotnetnuke` 10.66s -> 5.44s. The host_header, generic_ssrf and dotnetnuke module tests mock a real interaction and still pass, covering the non-empty path.
…g 13.6k YAMLs NucleiBudget.__init__ parsed the entire nuclei template tree on every budget-mode scan setup. That is 13,619 YAML documents, and the work was pure recomputation: the template set is identical from one scan to the next, so every scan paid ~9.5s to derive the same budget_paths/collapsible_templates/severity_stats. Three defects, all in product code rather than the test: 1. No caching. The analysis is a pure function of (template set, budget), but it ran unconditionally. Now keyed on a (path, size, mtime) signature over the template list plus the budget, and stored via the existing helpers cache. A template update changes the signature and forces a recompute; a different budget cannot collide with another budget's entry. 2. parse_yaml parsed templates that provably cannot contribute. Only templates with an http block ever yield a path through get_yaml_request_attr, but all 13,619 were fed to the YAML loader. A byte-level check skips ~5.3k of them. Verified this loses nothing: parsing the excluded set yields 0 paths. 3. The parse cache was never released. _yaml_files retained every parsed document for the module's lifetime, ~343MB RSS, though it is dead once both passes finish. Cleared after construction. Correctness verified ahead of speed. Compared against the previous implementation run verbatim over budgets 1, 2, 5 and 10: budget_paths, collapsible_templates and severity_stats are identical in all four. Cache invalidation checked positively (touch a template, recompute fires, result still identical) and negatively (budget is part of the key). Since cache_put is not atomic and the deps dir is shared across xdist workers, a torn read matters: truncated, empty, garbage and valid-json-missing-keys entries all fall back to recompute with identical output. TestNucleiBudget setup 12.59s -> 2.70s warm; the file's 11 tests pass unchanged. The win is per budget-mode scan, so real users doing repeated scans benefit too, not just CI.
…onfig dict
Two independent defects in test_module_nuclei.py, one perf and one correctness.
Perf: TestNucleiManual set "ratelimit": 10 while its template dir
(http/miscellaneous/) issues 100 requests against the fixture server. nuclei
paces those at 10/s, so every batch had a hard 10s floor, and the module runs
two batches per test. Measured with the fixture server counting requests:
at rl=10 the 100 requests span 9.88s of the 10.26s run, so the wall time IS
the pacing. The subclasses that inherit this config (CustomHeaders,
EnvIsolation) paid it too.
The limit is not load-bearing. The assertions check WHICH findings nuclei
reports (dir-listing, old-copyright), not request pacing, and the result set
is identical across rate limits: 5 runs each at rl=10, rl=150/conc=2 and
rl=150/conc=25 produced exactly one distinct result set,
{dir-listing, old-copyright}, with no flake. Dropping the override falls back
to the module default (150) and keeps every assertion intact.
Correctness: TestNucleiCustomHeaders did
`config_overrides = TestNucleiManual.config_overrides` and then wrote
`config_overrides["web"]["http_headers"]` into it. That is the same dict
object, so defining the subclass mutated the parent at import time. Verified:
TestNucleiManual.config_overrides["web"] and TestNucleiEnvIsolation's both
carried CustomHeaders' testheader1/testheader2, and
`Manual.config_overrides is CustomHeaders.config_overrides` was True. Both
tests were silently running with custom HTTP headers they never asked for,
and the leak was order dependent on class-definition order. Now deep copied;
a shallow dict() would not fix it because the mutated "web" dict is nested.
Verified:
- Assertions still discriminate, proven by mutation: pointing http_headers at
{"wrong": "value"} makes TestNucleiCustomHeaders fail on `assert
first_run_detect`, and it passes again when restored.
- Isolation holds: Manual/EnvIsolation "web" is now
{spider_distance, spider_depth} with no http_headers; CustomHeaders keeps
its own.
- Whole file 11 passed, 3x before (59.7s/75.2s/66.0s, median 66.0s) vs 3x
after (68.0s/48.2s/58.7s, median 58.7s). Per-test setup is the cleaner
signal: TestNucleiCustomHeaders 27.60s -> 10.58s.
- ruff check and ruff format --check clean.
The bbot_venv fixture built a stdlib venv and ran `pip install -e .` into it, resolving and installing ~84 packages from scratch every time. Profiled locally: venv create 4.01s, pip install 25.32s. Nothing in that work is cold; CI already runs `uv sync --group dev` in the same job, so uv's shared wheel cache is warm by the time this fixture runs, but pip has its own cache and cannot see it. Switch to `uv venv` + `uv pip install -e`, falling back to the stdlib venv and pip when uv is not on PATH, so the fixture still works outside CI. Package sets are identical, not merely similar: compared `pip list` from both paths, 67 packages each, zero entries unique to either side and zero version differences. Verified: - uv path 0.89s vs pip path 25.66s; both produce bin/bbot and `bbot -h` exits 0. - pip fallback exercised directly, not just by inspection. - Same file, same 4 tests pass on both: setup 24.62s -> 2.70s, file wall 39.50s -> 19.43s. - ruff check + ruff format --check clean. No assertions changed. The existing bbot CLI assert still guards the install.
…t_dict search_format_dict recursed as `search_format_dict(v, **kwargs)`. Every recursive call re-splatted the mapping into a fresh dict, so the walk cost O(nodes * len(kwargs)) instead of O(nodes). The substitution itself is trivial; the cost was entirely in rebuilding the kwargs dict 6.6k times per call. This is on the Scanner construction path. Preset.bake() calls ModuleLoader.find_and_replace(**os_environ), which runs search_format_dict over the full preloaded module set. Measured on the real payload: 6632 nodes, 3190 strings, and only 43 strings that contain a placeholder at all, against 394 environment keys. So ~394 dict rebuilds per node to service 43 actual substitutions. Confirmed the scaling is in the kwargs width, not the tree: kwargs= 3 -> 0.0051s kwargs= 50 -> 0.0268s kwargs=200 -> 0.0715s kwargs=394 -> 0.1476s Fix keeps the public **kwargs signature (test_helpers and the docstring example depend on it) and moves the recursion into a helper that passes the mapping by reference. Measured: search_format_dict(preloaded) 0.1547s -> 0.0027s (57x) search_format_dict(_shared_deps) 0.0054s -> 0.0001s (40x) find_and_replace 0.1593s -> 0.0032s Scanner.__init__ 0.1633s -> 0.0146s (11x) Every module test builds a Scanner, so this is a per-test floor across the whole suite, not a single-file win. test_module_lightfuzz.py (114 tests): 332.1s -> 310.8s, same 114 passed, median per-test delta 0.218s, which matches the measured per-construction cost. Equivalence proved against the original implementation, not assumed: 23/23 edge cases identical (missing keys, empty/non-str/non-dict inputs, non-string dict keys, malformed and nested placeholders, bytes/set/tuple values), plus exact output equality on the real preloaded and _shared_deps payloads. test_helpers.py, test_modules_basic.py, test_presets.py, test_scan.py, test_python_api.py, test_depsinstaller.py, test_config.py, test_events.py all pass.
_batch_pip_install() skipped any spec already satisfied in the environment, so those specs never entered _batch_installed. install_module() keys its skip off that set, so every module whose deps were already present fell through and spawned its own no-op `pip install --upgrade` subprocess. On a warm CI venv all 18 batchable specs are already satisfied, so the batch pass installed nothing and 20 modules each paid a full pip startup. Measured on the --install-all-deps path: 20 subprocesses / 19.2s -> 1 / 2.1s. That call is the bulk of test_cli_args, the slowest test in the suite at 140.2s. Satisfied specs are now batched rather than dropped, so `--upgrade` still runs for them, in one resolver pass instead of one subprocess per module. Two guards keep the covered set honest: - _needs_install() gates which modules contribute specs, mirroring the branch in _install(). Without it the batch would install for modules the locked path would never have touched. Verified to agree with _install() on all 915 (module, deps_behavior, cache-state) combinations. - a spec shared with a module carrying custom pip_constraints is excluded from the covered set, since it must still be resolved against those constraints. _batch_installed is now reset per pass; it was instance-scoped and never cleared, so a second install() in the same process saw stale coverage and could suppress a genuinely needed install. A failed batch covers nothing, so every module still falls back to its own install. Verified. test_depsinstaller_stale_pip_cache asserted the per-subprocess call shape, which this legitimately changes; it now asserts the set of specs that reached pip, which is the property it was actually protecting. Added test_depsinstaller_batch_covers_satisfied_deps, confirmed to fail against the old implementation with the exact per-module split and pass against the new.
…ll lock test_module_loading and test_modules_basic_perdomainonly each request the full module set, so load_modules() calls depsinstaller.install() for all 149 modules. Neither test's body needs a single dependency on disk: both only instantiate modules and read class attributes off them. Under xdist that request is not free. install() takes a lock-free fast path only when every module is already recorded installed, and while another worker holds install.lock mid-install that condition is false, so these two fall through to the polling wait. The holder installs its whole chain under one hold, so a waiter needing the complete module set cannot be released early and pays the holder's entire remaining runtime. That is the whole cost of both tests. Measured against a foreign process holding install.lock for 12s, with the fast path declining as it does in CI: deps default wall 12.03s 49 lock poll attempts deps disable wall 0.00s 1 attempt, never blocks CI shows the same shape: py3.13 job 100073737907 credits test_module_loading 59.3s and perdomainonly 53.3s, and both release within seconds of gw2 finishing test_cli_args --install-all-deps. Their real bodies are ~1.6s and ~0.9s. Setting deps behavior to disable makes install() return immediately without touching the lock. Verified it does not weaken either test: both configurations load an identical set of 151 modules with identical _type and watched_events, and the same four per_domain_only modules (azure_tenant, emailformat, skymem, viewdns). Mutation check confirms the async-hook assertion still fires under disable: patching nuclei.handle_event and wayback.cleanup to non-async is caught with both names reported. No assertions changed, no coverage reduced. Full file: 7 passed in 4.00s.
…ps install lock" This reverts commit 3428b7e. Broke test_module_loading and test_modules_basic_perdomainonly on all five python versions: bbot.errors.BBOTError: Error loading module badsecrets: No module named 'badsecrets' The premise was wrong. I assumed loading a module only reads class attributes, so no dependency needs to be on disk. But _load_modules() imports each module's python file, and a module whose import pulls a third-party package needs that package actually installed. badsecrets declares deps_pip = ["badsecrets~=1.2.1"] and is NOT in pyproject.toml, runtime or dev, so it reaches the venv only via the deps installer. Setting deps behavior to disable skips that install, and the import then fails. The local equivalence check that cleared this was invalid: my venv already had badsecrets from earlier runs, so both configurations loaded 151 modules and the difference was invisible. Comparing deps-on against deps-off in an environment already carrying the deps proves nothing. A valid check has to run against a venv holding only pyproject dependencies. The convoy measurement itself still stands (a waiter needing the full module set tracks the holder exactly, 12.03s vs 0.00s under a held lock). Only this way of avoiding it is wrong. Any real fix has to keep the deps installed and instead stop the waiter from serializing behind the holder's entire chain.
…ransport Four modules hardcoded `await self.helpers.sleep(5)` in `finish()` before their last interactsh poll. That wait exists to let interactions triggered just before the scan ended propagate to the interact.sh server, so it is a property of the transport, not of each module. Under test the transport is `Interactsh_mock`, an in-process asyncio.Queue with no propagation delay at all, so every interactsh test paid the full 5s for interactions that were already queued. `scan.finish()` re-queues FINISHED whenever the previous round produced new activity, so `finish()` runs at least twice per scan and the cost is 10s per test, not 5s. Traced it: the second round's poll returns 0 interactions every time. Adds `Interactsh.settle()`, which keeps the identical 5s `asyncio.sleep` for the real client, and overrides it to a no-op on the mock. Production timing is unchanged. Verified: interactsh subset of test_module_lightfuzz.py 43.4s -> 19.2s. generic_ssrf + host_header + dotnetnuke 52s -> 21s, same pass set and same finding counts (60 and 30), so no interactions are lost. Note on what was deliberately NOT changed: the mock's own sleeps (0.5s poll_loop idle, 0.1s per interaction, 1s in deregister) are load-bearing. Removing them individually passes, but removing them together drops 4 of 60 findings in generic_ssrf. That is a real ordering dependency in the mock's drain path, not a stale assertion, so the sleeps stay until it is understood.
… sleep test_web_interactsh slept a hardcoded 10 seconds between firing the two out-of-band requests and asserting that both callbacks ran with the right URL. The sleep was sized for the worst case, so the test always paid the full budget even though the interactions land much earlier. Measured against the real interactsh servers, 5 consecutive runs: the last of the four asserted conditions was satisfied at 4.25s, 1.32s, 3.72s, 3.60s and 3.56s. The remaining 6 to 9 seconds were pure dead wall time. Now the test polls the same conditions it later asserts on, at 0.1s, and still stops at the same 10s deadline. The four asserts are untouched, so a genuine failure fails exactly as before: verified that with the conditions never satisfied the loop still runs the full 10.04s and the assertion still raises, rather than exiting early and masking it. The waiting is bounded by the identical budget, so this cannot turn a slow interaction into a flake. Product code is untouched; this is test-side dead time only. test_web_interactsh 16.4s -> 6.2s across 3 runs. Whole file 21 passed in 14.85s. ruff check and ruff format --check clean.
pytest_httpserver's respond_nohandler builds the miss body from repr(request) plus a rendering of every registered matcher, so the body is unique per URL and grows with the handler count. On the webbrute_shortnames test that is 4318 responses averaging 1136 bytes, 4234 of them distinct. Brute-force modules establish a baseline from a miss and then diff every response against it. A miss body that embeds the requested URL never matches the baseline, so HttpCompare cannot short-circuit and DeepDiffs the full body every time: 2.02 ms per response, 8.72s summed, 19.8% of execute_fuzz time. The dump was measurement error, not fidelity. Real servers return a stable 404, which is what the module's filtering logic is written against. The accumulated assertion goes with it. All three httpserver fixtures call clear() before check_assertions(), and clear() calls clear_assertions(), so no-handler assertions were discarded unread and could never fail a test. Verified directly. Handler errors travel a separate channel (check_handler_errors) and still surface. Verified negatively: miss still returns no_handler_status_code (404 and the 403 that bypass403 configures), miss bodies are now stable across URLs, matched handlers are untouched, and a raising handler still records and re-raises through check_handler_errors. webbrute_shortnames 25.7s -> 18.5s over three runs each. Across webbrute, webbrute_shortnames, iis_shortnames, bypass403 and excavate: 105.7s -> 96.2s with an identical 71 passed. lightfuzz + generic_ssrf 116 passed, wayback + reflected_parameters + virtualhost + host_header + dotnetnuke 43 passed, test_web + test_scan 34 passed. test_module_http::TestHTTP_URLBlacklist fails locally on the clean tree too, same 10 passed / 1 failed either way. Not a regression from this change.
e6bc5ea replaced pytest_httpserver's per-request diagnostic dump with a static "Not Found" body. That fixed the DeepDiff cost but broke TestWebParameters_include_count on every python version: expected "3\ttest", got "2\ttest". Root cause is content dedup, not the count assertion. base.py:1158 dedups HTTP_RESPONSEs on (host, port, body_sha256) and explicitly exempts the empty-body hash e3b0c442..., which is sha256(b""). The upstream dump embedded the request URL, so every miss body was unique and no miss ever collided. A constant non-empty body makes all misses hash alike, so the second distinct miss URL is dropped as duplicate content before excavate sees it. In this test /validPath and /search are both misses: /validPath was dropped, taking its HEADER WEB_PARAMETER with it and dropping the test count from 3 to 2. Serving an empty body keeps the miss body constant across URLs while landing on the one hash the dedup path already exempts, so no miss is ever dropped. The perf win is unaffected; the cost was body size and uniqueness, not content. webbrute_shortnames over three runs each: upstream dump 27.70 / 27.22 / 27.93s "Not Found" 19.33 / 20.41 / 20.58s empty 20.01 / 20.53 / 19.54s Verified sha256(b"") equals the constant base.py exempts. Negative controls: miss still returns no_handler_status_code (500 default, 403 when configured), miss bodies stable across URLs, matched handlers untouched, and a raising handler still re-raises through check_handler_errors. web_parameters + excavate + webbrute + webbrute_shortnames + iis_shortnames + bypass403: 74 passed. lightfuzz + generic_ssrf + virtualhost + host_header + dotnetnuke + wayback + reflected_parameters: 159 passed. test_web + test_scan: 44 passed. test_module_http::TestHTTP_URLBlacklist fails locally on the clean tree too, same assert 4 == 5 either way. Not a regression from this change.
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
2 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.
Summary
The suite spends most of its wall time waiting, not working. This removes the waiting.
Changes
Test harness
FastShutdownHTTPServerpolls every 5ms instead ofsocketserver's 0.5s default. Everybbot_httpserverteardown paid half a second.stop_serverbusy-wait: 100ms to 5ms per check.await asyncio.sleep(0.5)intest_manager_scope_accuracythat waited on module init the scan already guarantees.Scanner
Helpers
CloudCheckinstance and thewordninja.LanguageModel. Both were rebuilt per scan from identical inputs.search_format_dictshort-circuits when no placeholder is present, and uses one compiled regex pass instead of onestr.replaceper kwarg.Deps installer
pip_constraintsare excluded, since they must resolve against their own set.CI scope
test_e2e(builds a venv, runs the real binary as a subprocess),test_depsinstaller(ansible and the package-manager path), andtest_command(subprocess and sudo). The other five were pure library logic already covered on five Python versions bytests.yml. Collection drops from 48 tests to 9.Validation
ruff checkandruff format --checkclean.test_web.py(the httpserver fixture consumers) passes 19/19 locally.Results
Measured on CI, same commit, all jobs green.
Per-distro test time went from 7.0-8.6m to 1.4-2.1m. Fedora was the worst case at 14.2m and now runs in 1.6m.
Known issue, not fixed here
resolver_file()inbbot/core/helpers/dns/brute.pyfetches the nameservers list fromraw.githubusercontent.comlive, unmocked. When that fetch stalls it burns two 300s download attempts, which matches the 668s gap observed on back-to-back Fedora runs where four xdist workers blocked and released within 1.6s of each other. Droppingtest_dnsfrom the distro matrix stops paying for it six times per PR, buttests.ymlstill runs it on five Python versions. Worth a follow-up.