preset-validation: fix crashes, wire validator into YAML load, validate interactsh_server - #3105
Merged
Merged
Conversation
…onzero on bad config
- validate.py: guard _classify_loc against short loc tuples. config.modules
given as a list (e.g., {"config": {"modules": ["nuclei"]}}) previously
hit IndexError in the loc classifier.
- validate.py: skip module_dirs pre-pass when the value is not a list.
A string module_dirs (e.g., "/tmp/foo") used to iterate characters and
trigger filesystem calls, surfacing as PermissionError.
- validate.py: skip the modules-list post-pass when the value is not a
list, so `modules: "nuclei"` no longer cascades into six bogus
per-character suggestions on top of the real type error.
- validate.py: route len(loc)==1 extra_forbidden suggestions through
PresetSchema field names so `modlues:` suggests `modules` instead of
`module_dirs`, `flgas:` suggests `flags`, etc.
- validate.py: validate_preset_file catches FileNotFoundError, OSError,
and yaml.YAMLError and returns a clean error instead of raising.
- preset.py: call validate_preset() at the top of from_dict so typos in
YAML preset files (loaded via -p or include:) surface up front instead
of flowing through to bake() as raw TypeError or AttributeError.
- cli.py: BBOTArgumentError and BBOTError handlers in _main return False
and log at ERROR level; main() exits 1 on falsy return. Validation
failures now produce a non-zero exit so CI and scripts can detect them.
- webbrute: extensions accepts str or list[str]. Matches the chain_lists
runtime contract and the bundled web/dirbust-heavy.yml preset, which
passes a YAML list.
- tests: eight new cases in test_validate_preset.py covering each fix. Four
existing tests in test_presets.py updated to use real module names
(the prior fake `testpreset1`...`testpreset5` markers are now caught
by the stricter validator).
ausmaster
approved these changes
May 18, 2026
Contributor
📊 Performance Benchmark Report
📈 Detailed Results (All Benchmarks)
🎯 Performance Summary✅ No significant performance changes detected (all changes <10%) 🐍 Python Version 3.11.15 |
The prior commit changed _main()'s BBOTArgumentError and BBOTError handlers to `return False` and added a `sys.exit(1)` in main() so bbot would exit non-zero on validation failure. That broke test_cli_customheaders, which asserts `result is None` from `_main()` on invalid `--custom-headers` input, and the failure cascaded across all test-distros jobs and Python 3.14 in CI. Adhere to the existing convention: log the error and return None. Real-CLI exit code stays 0 on validation failures, matching every other CLI-arg failure path in BBOT. Users still see the [ERRR] or [WARN] line; scripts that need to detect failure can grep stderr or check log output.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## preset-validation #3105 +/- ##
=================================================
+ Coverage 90% 90% +1%
=================================================
Files 452 452
Lines 39341 39437 +96
=================================================
+ Hits 35235 35326 +91
- Misses 4106 4111 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
A domain typed without its TLD (a single-label value with no dot) used
to pass preset-load validation, then surface as "Failed to register
with an interactsh server" warnings deep in the scan from every module
that uses interactsh. Catch the typo up front instead.
- models.py: add a shared _validate_fqdn_or_ip helper. Accepts None,
empty string, any IP (v4/v6), or a hostname containing at least one
dot that passes is_dns_name. Rejects single-label values like
'localhost'.
- models.py: attach the helper as a field_validator to
BBOTConfig.interactsh_server.
- environ.py: skip None values in flatten_config. str(None) was writing
the literal "None" into BBOT_INTERACTSH_SERVER and other BBOT_* env
vars, which pydantic-settings then merges back into the config dict
during validation and trips the new FQDN check on every test that
follows. Also a latent bug independent of this validator: any field
that defaults to null was getting "None" injected on round-trip.
- validate.py: handle the pydantic 'value_error' kind in _format_msg so
ValueError messages raised inside a field_validator surface cleanly
("not a valid FQDN..." rather than "Value error, not a valid FQDN...").
- tests: cover accept-or-reject behavior for interactsh_server,
including a single-label hostname case.
Follow-up candidates flagged during the field survey (not addressed in
this commit): web.http_proxy (URL), output module URLs (splunk,
elastic, slack, discord, webhook, rabbitmq), database hosts (postgres,
mysql, mongo, neo4j), and the baddns custom_nameservers list typing.
Worth their own discussion since each carries some compatibility risk
for users with nonstandard internal endpoints.
aconite33
force-pushed
the
preset-validation-fixes
branch
from
May 19, 2026 02:28
e56370f to
a74e065
Compare
The FQDN-or-IP helper landed in `bbot/core/config/models.py` next to
the pydantic schemas, but BBOT already has a dedicated home for public
validators: `bbot/core/helpers/validators.py`, which holds
`validate_host`, `validate_port`, `validate_url`, `validate_email`, and
friends. Putting the new helper anywhere else fragments the validator
API surface and makes the function harder to find for module authors
who want to opt in.
Mechanical move only, no behavior change:
- validators.py: define `validate_fqdn_or_ip` next to `validate_host`,
with a docstring noting the relationship to `validate_host` (this one
is stricter and does not normalize, so a pydantic field_validator
sees the input unchanged).
- models.py: drop the local definition. Import the helper from
`bbot.core.helpers.validators` and keep the existing field_validator
binding on `BBOTConfig.interactsh_server`.
- models.py.__all__: remove the entry. The helper is no longer defined
here.
Module authors who want FQDN-or-IP validation on their own
`class Config` fields can now do:
from bbot.core.helpers.validators import validate_fqdn_or_ip
from pydantic import field_validator
class Config(BaseModuleConfig):
host: str = Field("127.0.0.1", description="...")
_validate_host = field_validator("host")(validate_fqdn_or_ip)
liquidsec
approved these changes
May 21, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #3058 addressing the issues found in this fuzz comment, plus an additional FQDN check uncovered while testing.
What this PR does
Crash fixes in
validate_preset()_classify_locnow guards against shortloctuples.validate_preset({"config": {"modules": ["nuclei"]}})used to raiseIndexError; it now returns a clean error.module_dirspre-pass now skips non-list shapes.validate_preset({"module_dirs": "/tmp/foo"})used to iterate characters and surface asPermissionError; it now returns a clean type error.validate_preset({"modules": "nuclei"})used to emit one correct type error plus six bogus per-character lookups; it now emits just the one type error.Better suggestions for top-level typos
_format_msgnow drawsextra_forbiddensuggestions fromPresetSchemafield names whenlen(loc) == 1. Before, top-level typos consulted the dotted config-path universe and produced things like:modlues:→ "Did you mean 'module_dirs'?"flgas:→ "Did you mean 'file_blobs'?"targest:→ "Did you mean 'aggregate'?"After:
modlues:→ "Did you mean 'modules'?"flgas:→ "Did you mean 'flags'?"targest:→ "Did you mean 'target'?"YAML preset files now run through the validator
validate_preset()is now called fromPreset.from_dict(), which coversfrom_yaml_file,from_yaml_string, andinclude:directives. Previously the validator only ran for-cCLI args, so typos in-p preset.ymlwere silently accepted and bad values crashed later insidebake()(e.g.,scope.strict: "yesplease"→TypeErrorfrom radixtarget;scope:as a list →AttributeError).interactsh_serveris validated as an FQDN or IPFound while fuzzing: a domain typed without its TLD (a single-label value with no dot) used to pass preset-load validation and then surface as "Failed to register with an interactsh server" warnings from every interactsh-using module mid-scan. A new
validate_fqdn_or_iphelper attached as afield_validatortoBBOTConfig.interactsh_serverrejects this up front. AcceptsNone, empty string, any IPv4/IPv6, or a hostname containing at least one dot. Rejects single-label values likelocalhost.The helper lives in
bbot/core/helpers/validators.pyalongsidevalidate_host,validate_port,validate_url, and friends. Module authors who want the same check on their ownclass Configfields can opt in with one line:Other fields surveyed but left alone (each carries some compatibility risk for users with internal endpoints, worth its own discussion):
web.http_proxy, output module URLs (splunk, elastic, slack, discord, webhook, rabbitmq), database hosts (postgres, mysql, mongo, neo4j), and thebaddnscustom_nameserverslist typing. Each can opt in tovalidate_fqdn_or_ip(or a future URL validator) in its own module.flatten_configskipsNone(latent bug)While wiring the FQDN validator, surfaced a separate bug in
bbot/scanner/preset/environ.py:flatten_configwas doingstr(v)on every value, which wrote the literal string"None"intoBBOT_*env vars for any null field. Pydantic-settings then merged that back into the config dict during validation, defeating any field validator that expects a real value. Now skipped.validate_preset_file()no longer leaksFileNotFoundErrorCatches
FileNotFoundError,OSError, andyaml.YAMLErrorand returns a singlePresetValidationErrorinstead.webbruteschema bumped to accept listsextensionswas declaredstrbut the bundledweb/dirbust-heavy.ymlpreset passes a YAML list, and the runtime feeds it throughchain_listswhich handles both. The strict YAML validation surfaced this. Schema is nowUnion[str, list[str]].CLI exit code: unchanged from baseline
An earlier version of this PR tried to make bbot exit 1 on validation failure. That broke
test_cli_customheaders, which asserts_main()returnsNoneon invalid--custom-headersinput, and the failure cascaded across the test-distros matrix and Python 3.14. Reverted in commit02cdec64e. The CLI keeps the existing convention: validation errors are logged ([ERRR]or[WARN]) and_mainreturnsNone. Users still see the error; the exit code stays 0 like every other arg-failure path in BBOT.Tests
test_validate_preset.pycovering each fix, including the single-label-hostname FQDN repro.test_presets.pyupdated. They used fake module names (testpreset1...testpreset5,asdf) as opaque config markers; the stricter validator now rejects those, so the tests use real module names with the universalmodule_timeoutfield.test_validate_preset.py+test_presets.py+test_config.py+test_cli.py::test_cli_customheaderspass locally.Fuzz battery before/after
config.modulesas listIndexErrormodule_dirsas stringPermissionErroron/run/dockermodules: "nuclei"PresetSchemavalidate_preset_file("/missing")FileNotFoundErrorraisedinteractsh_serveras a single-label hostname (missing TLD)Commits
b10514e17— Original crash, typo-suggestion, YAML-load wiring, and webbrute-schema fixes.02cdec64e— Revert the cli.py exit-code change to match the existing test framework convention.c77dc5588—interactsh_serverFQDN validation +flatten_configNoneskip +value_errorformatter polish + tests.a74e065a9— Cherry-pick of @liquidsec'sd423533f4fromretirejs-test-version-range-brittleness. Unrelated to this PR — retire.js's vulnerability DB drifted upstream (CVE-2020-11022 affected-versions range moved from[1.2.0 ...to[1.12.0 ...), breaking the brittle full-string assertion intest_module_retirejs. Picked the existing fix in to get CI green. If that branch lands onpreset-validationfirst, this commit becomes a no-op merge.2d15ac687— Mechanical move ofvalidate_fqdn_or_ipfrombbot/core/config/models.pytobbot/core/helpers/validators.pyso it lives next to BBOT's other public validators. No behavior change.Known item left alone
Pydantic v2 lax bool coercion still accepts
"yes" / "true" / "1" / "on"etc. forscope.strict. Flagged in the original review as P1. Not addressed here because addingstrict=Trueto every bool field is a broader change worth its own discussion.