-
Notifications
You must be signed in to change notification settings - Fork 3k
add strict probe option for httpx #6652
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughA new strict probe feature is added to Nuclei via a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/runner/runner.go (1)
700-713: CloseInputHelperbefore early-return whenStrictProbefinds no URLsWhen
StrictProbeis true and the httpx probe returns zero URLs, the code returns immediately without closingInputHelper:if r.options.StrictProbe && urlCount == 0 { r.Logger.Info().Msgf("Strict probe mode: No URLs found from httpx probe, skipping scan") return nil }This leaves the
InputHelper(created at line 573) unclosed. The cleanup code at line 757 is only reached in the normal execution path afterrunStandardEnumeration. To ensure consistent resource cleanup, close the helper before returning:if r.options.StrictProbe && urlCount == 0 { r.Logger.Info().Msgf("Strict probe mode: No URLs found from httpx probe, skipping scan") + if executorOpts.InputHelper != nil { + _ = executorOpts.InputHelper.Close() + } return nil }Note: The behavior of skipping the entire scan (not just HTTP templates) is intentional per the feature documentation ("no fallback to raw input"), but the resource must still be cleaned up properly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmd/nuclei/main.go(1 hunks)internal/runner/inputs.go(3 hunks)internal/runner/runner.go(1 hunks)pkg/types/types.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Format Go code usinggo fmt ./...
Run static analysis usinggo vet ./...
Files:
cmd/nuclei/main.gointernal/runner/inputs.gointernal/runner/runner.gopkg/types/types.go
internal/runner/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Input provider setup should be configured before engine creation for concurrent target processing
Files:
internal/runner/inputs.gointernal/runner/runner.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint
🔇 Additional comments (2)
cmd/nuclei/main.go (1)
419-439: CLI wiring for--strict-probe/-splooks correctThe new optimization flag cleanly binds
options.StrictProbewith a sensible description, and-spdoes not conflict with existing short flags (-spmis distinct). No issues from a parsing or configuration perspective.internal/runner/inputs.go (1)
20-87: URL counting and new return signature look correct; be mindful that0can mean “no probe run”The refactor of
initializeTemplatesHTTPInputto:
- allocate a
hybrid.HybridMap,- atomically count successful
ProbeURLresults, and- return
(hm, urlCount, err)is sound and concurrency-safe (atomic increments in the goroutines, single
Load()afterswg.Wait()).One nuance: in the
MultiFormatInputProviderbranch you now returnhm, 0, nilwithout ever running httpx, sourlCount == 0can mean either “probe executed and found nothing” or “probe was deliberately skipped for this input type”. The current caller (RunEnumeration) treatsurlCount == 0as “nothing found” whenStrictProbeis enabled, so any future callers that add more logic based onurlCountshould take this ambiguity into account (e.g., by introducing a sentinel value or a separate “probed” flag if needed).
| // StrictProbe stops scanning when httpx probe returns 0 URLs (no fallback to raw input) | ||
| StrictProbe bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StrictProbe is not propagated by Options.Copy()
You’ve added StrictProbe to types.Options but Options.Copy() doesn’t copy it, so any code using a cloned options struct (e.g., authOptions := r.options.Copy()) will always see StrictProbe == false regardless of the original setting. This is inconsistent with how neighboring fields like DisableHTTPProbe and LeaveDefaultPorts are handled and can lead to confusing behavior as usage of Copy() expands.
Recommend including StrictProbe in the copy constructor:
DebugRequests: options.DebugRequests,
DebugResponse: options.DebugResponse,
DisableHTTPProbe: options.DisableHTTPProbe,
+ StrictProbe: options.StrictProbe,
LeaveDefaultPorts: options.LeaveDefaultPorts,Also applies to: 476-686
🤖 Prompt for AI Agents
In pkg/types/types.go around lines 204-205, Options.Copy() does not propagate
the new StrictProbe bool field so copies always have StrictProbe==false; update
the copy constructor(s) (including the other copy blocks referenced around lines
476-686) to set StrictProbe on the returned struct from the receiver (i.e.,
include StrictProbe: o.StrictProbe alongside DisableHTTPProbe/LeaveDefaultPorts
etc.) so cloned Options preserve the original StrictProbe value.
dwisiswant0
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my view, the proposal doesn't fully address the broader set of scenarios. By introducing -strict-probe, we would effectively enforce that all inputs MUST be valid HTTP/S targets by default, which creates a significant limitation because our ecosystem isn't limited to HTTP-protocol-based templates. We have DNS, JavaScript, and Network protocols too, each of which has its own expected input formats.
With -strict-probe enabled, these non-HTTP/S templates would end up being skipped entirely, which breaks their intended functionality and reduces coverage. This effectively narrows the scope of the scanner and could mislead users by silently skipping entire (set of) protocols of templates.
Because of this, I think the proposal doesn’t sufficiently address the full operational context and needs mature design consideration.
|
@dwisiswant0 This optional flag allows users to decide whether to keep scanning the specified target, based on the output of the httpx integration as stated. We might need to make it a bit more explicit, maybe |
|
@dogancanbakir - what I was trying to say is, even if a target cannot be probed by httpx, it should still remain eligible for scanning through other non-HTTP protocols. So the inability to probe via httpx shouldn't automatically exclude it from the rest of the non-HTTP-protocol-based templates. |
|
and
By "default", I mean the flag's standard behavior when users enable it. |
|
Proposal:
The way I see that, with But, additionally, that approach would end up skipping any HTTP-protocol-based template that overrides its own host & port settings. $ grep -Pnr "\{\{Host(name)?\}\}:[0-9]{1,5}" http/
http/vulnerabilities/unifi/unifi-nfc-credentials.yaml:20: @Host: {{Hostname}}:9780
http/vulnerabilities/unifi/unifi-create-user.yaml:23: @Host: {{Hostname}}:9780
http/cves/2025/CVE-2025-52665.yaml:55: @Host: {{Hostname}}:9780
http/cves/2019/CVE-2019-8451.yaml:41: url=https://{{Host}}:443@{{interactsh-url}}
http/cves/2024/CVE-2024-6396.yaml:54: @Host: http://{{Host}}:43800
http/cves/2024/CVE-2024-6396.yaml:56: Host: {{Host}}:43800
http/cves/2018/CVE-2018-8024.yaml:33: - "{{Host}}:4040/jobs/?\"'><script>alert(document.domain)</script>"
http/cves/2020/CVE-2020-9480.yaml:50: "spark.master": "spark://{{Hostname}}:6066" |
This is the desired behavior explicitly requested by the community user, and I don't see any issue with this, as long as we don't change nuclei's default behavior. |
|
Tagging the requester @sixteen250. I want to elaborate a bit more on the concern I'm trying to highlight, because the current behavior of the patch can lead to unintended side effects across the entire scanning flow. From what I understand, the logic effectively works like this: The issue here is that This logic cascades too broadly, causing the scanner to skip ALL protocols simply because httpx cannot probe one or two ports. As a result, templates for DNS, Network, JavaScript, etc., protocols never get a chance to run, even though the host itself might still be very much alive on other ports/services. tl;dr; This That's the intent behind my concern, defining the boundaries of what |
|
To make the scenario easier to visualize, let's use the example provided by the requester, @sixteen250, which uses port 3306 (MySQL). If the user supplies a target like But that behavior is incorrect, because while the target is not running an HTTP service, we DO have a number of templates that are specifically intended to scan MySQL on port 3306. Ex: Those templates are explicitly designed to interact with MySQL, meaning they remain completely valid even if httpx cannot probe the host. So dropping the target solely because httpx fails ends up preventing a whole set of relevant, non-HTTP-protocol-based templates from running. That's the core of the issue: |
|
Yeah i agree, disabling all templates if http is not detected would be overly strict, we instead we should still check other port templates / protocols etc cc @tarunKoyalwar |
|
Thanks for looping me in @dwisiswant0. Regarding the naming proposed by @dogancanbakir: I agree that renaming it to -httpx-strict-probe (or similar) makes it much more explicit that this flag controls the behavior of the httpx integration specifically. Regarding the logic concern: I strongly agree with @dwisiswant0's assessment. My original issue was indeed about the inefficiency and timeouts caused by sending HTTP requests to non-HTTP ports. However, completely dropping the target if the probe fails would lead to missed vulnerabilities on non-Web services (like the MySQL example mentioned). The ideal behavior for this flag would be: If httpx probe fails → Skip ONLY http protocol templates → Continue scanning with network/javascript/dns templates. This would solve the timeout performance issue without sacrificing coverage for other services. |
|
Thanks for the clarification @sixteen250. I got the impression that the desired behavior was the completely skip the target from the following statement:
Anyway, I'll make necessary changes to the PR. Thanks @dwisiswant0 and all for the input! |
Proposed changes
closes #6651
Checklist
Summary by CodeRabbit
New Features
--strict-probe(-sp) CLI flag to enforce strict HTTP probe mode. When enabled, scanning stops if the HTTP probe returns zero URLs, preventing fallback to raw input.✏️ Tip: You can customize this high-level summary in your review settings.