strict probe - #7622
Conversation
WalkthroughThe PR adds ChangesStrict probe reachability
Per-host HTTP connection capacity
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Runner
participant httpx
participant InputHelper
participant TemplateExecutor
CLI->>Runner: enable -strict-probe
Runner->>httpx: probe target services
httpx-->>Runner: reachable HTTP targets
Runner->>InputHelper: enable strict URL resolution
Runner->>TemplateExecutor: exclude unreachable HTTP and closed TCP templates
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
internal/runner/reachability.go (1)
130-157: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftSequential, blocking port probing can add significant load-time latency.
reachable[p]is computed via nested sequential loops, dialing one host:port at a time with up to a 2s timeout each. For scans with many hosts and several distinct network-template ports, and especially against internet-facing targets where closed ports are often silently dropped (not RST'd) rather than actively refused, this can add a lot of blocking wall-clock time before the scan's normal request phase even begins.Consider bounding this with a small worker pool (e.g. goroutines + semaphore/errgroup) since
Fastdialer.Dialis already safe for concurrent use elsewhere in the codebase.♻️ Sketch of bounded-concurrency probing
- reachable := map[string]bool{} - for p := range toProbe { - for _, h := range hosts { - res := r.probe(h, p) - if res == portOpen || res == portUnknown { - reachable[p] = true - break - } - } - } + reachable := map[string]bool{} + var mu sync.Mutex + var wg sync.WaitGroup + sem := make(chan struct{}, maxConcurrentProbes) + for p := range toProbe { + wg.Add(1) + go func(p string) { + defer wg.Done() + sem <- struct{}{} + defer func() { <-sem }() + for _, h := range hosts { + res := r.probe(h, p) + if res == portOpen || res == portUnknown { + mu.Lock() + reachable[p] = true + mu.Unlock() + return + } + } + }(p) + } + wg.Wait()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/runner/reachability.go` around lines 130 - 157, Update the reachable-port probing loop in the reachability logic to use bounded concurrency rather than dialing each host:port sequentially. Add a small worker pool or semaphore around r.probe calls, safely coordinate concurrent updates to reachable, and preserve the existing rule that a port is reachable when any host returns portOpen or portUnknown.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/runner/reachability.go`:
- Around line 55-92: Gate the strict reachability probe in
noHTTPServiceReachable (or strictProbeEnabled) on !r.options.DisableHTTPProbe,
so -no-httpx skips the internal httpx probe and web-template prune. In
internal/runner/runner.go lines 682-690, make no further change because the
root-cause guard will make this path no-op; in lines 783-791, make no further
change because the existing warning will then accurately cover both strict-probe
paths.
- Around line 269-281: Update classifyDial to classify transient DNS-resolution
failures as portUnknown rather than portClosed, while preserving portOpen and
existing timeout handling. Detect the relevant DNS/network error condition from
the dial result before the final portClosed fallback, ensuring single-host scans
do not treat temporary resolution failures as definitive closure.
In `@pkg/types/types.go`:
- Line 587: Run go fmt ./... to reformat the struct literal containing
StrictProbe, ensuring its field alignment matches AutomaticScan, Silent, and the
other entries.
In `@README_CN.md`:
- Line 309: Translate the strict-probe description while preserving the flag
name, behavior, and lossless qualifier: update README_CN.md lines 309-309 in
Chinese, README_ES.md lines 309-309 in Spanish, README_ID.md lines 309-309 in
Indonesian, README_JP.md lines 309-309 in Japanese, README_KR.md lines 309-309
in Korean, README_PT-BR.md lines 309-309 in Brazilian Portuguese, and
README_TR.md lines 309-309 in Turkish.
---
Nitpick comments:
In `@internal/runner/reachability.go`:
- Around line 130-157: Update the reachable-port probing loop in the
reachability logic to use bounded concurrency rather than dialing each host:port
sequentially. Add a small worker pool or semaphore around r.probe calls, safely
coordinate concurrent updates to reachable, and preserve the existing rule that
a port is reachable when any host returns portOpen or portUnknown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ac41e4e-543e-4042-82a0-6f8a067ec650
📒 Files selected for processing (17)
README.mdREADME_CN.mdREADME_ES.mdREADME_ID.mdREADME_JP.mdREADME_KR.mdREADME_PT-BR.mdREADME_TR.mdcmd/nuclei/main.gointernal/runner/reachability.gointernal/runner/reachability_test.gointernal/runner/runner.golib/config.gopkg/input/transform.gopkg/input/transform_test.gopkg/protocols/http/httpclientpool/clientpool.gopkg/types/types.go
| func (r *Runner) noHTTPServiceReachable() bool { | ||
| if r.inputProvider == nil { | ||
| return false | ||
| } | ||
| dialers := protocolstate.GetDialersWithId(r.options.ExecutionId) | ||
| if dialers == nil { | ||
| return false // cannot probe within policy — keep web templates | ||
| } | ||
| httpxOptions := httpx.DefaultOptions | ||
| if r.options.AliveHttpProxy != "" { | ||
| httpxOptions.Proxy = r.options.AliveHttpProxy | ||
| } else if r.options.AliveSocksProxy != "" { | ||
| httpxOptions.Proxy = r.options.AliveSocksProxy | ||
| } | ||
| httpxOptions.RetryMax = r.options.Retries | ||
| if r.options.Timeout > 0 { | ||
| httpxOptions.Timeout = time.Duration(r.options.Timeout) * time.Second | ||
| } | ||
| httpxOptions.NetworkPolicy = dialers.NetworkPolicy | ||
| client, err := httpx.New(&httpxOptions) | ||
| if err != nil { | ||
| return false // cannot probe — keep web templates | ||
| } | ||
|
|
||
| anyHTTP := false | ||
| r.inputProvider.Iterate(func(mi *contextargs.MetaInput) bool { | ||
| if strings.HasPrefix(mi.Input, "http://") || strings.HasPrefix(mi.Input, "https://") { | ||
| anyHTTP = true | ||
| return false | ||
| } | ||
| if utils.ProbeURL(mi.Input, client) != "" { | ||
| anyHTTP = true | ||
| return false | ||
| } | ||
| return true | ||
| }) | ||
| return !anyHTTP | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
-no-httpx doesn't actually disable the strict-probe reachability prune — gate it consistently.
strictProbeEnabled() / noHTTPServiceReachable() never check r.options.DisableHTTPProbe, so the web-protocol exclusion prune (which relies on its own internal httpx.New() probe) still runs — and can still exclude HTTP/headless/websocket templates and issue HTTP probes — even when the user passes -no-httpx. This contradicts the warning added alongside the per-input gating path, which claims strict-probe "has no effect" in that combination.
internal/runner/reachability.go#L55-L92: add a!r.options.DisableHTTPProbe(or equivalent) check tostrictProbeEnabled()/noHTTPServiceReachable()so this probe/prune is skipped when httpx probing is disabled.internal/runner/runner.go#L682-L690: once the check above is added, this block will correctly no-op under-no-httpx, matching user intent.internal/runner/runner.go#L783-L791: the existing warning message will then be accurate for both strict-probe code paths, not just the per-input one.
📍 Affects 2 files
internal/runner/reachability.go#L55-L92(this comment)internal/runner/runner.go#L682-L690internal/runner/runner.go#L783-L791
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/runner/reachability.go` around lines 55 - 92, Gate the strict
reachability probe in noHTTPServiceReachable (or strictProbeEnabled) on
!r.options.DisableHTTPProbe, so -no-httpx skips the internal httpx probe and
web-template prune. In internal/runner/runner.go lines 682-690, make no further
change because the root-cause guard will make this path no-op; in lines 783-791,
make no further change because the existing warning will then accurately cover
both strict-probe paths.
| func classifyDial(dial dialFunc, addr string, timeout time.Duration) probeResult { | ||
| ctx, cancel := context.WithTimeout(context.Background(), timeout) | ||
| defer cancel() | ||
| conn, err := dial(ctx, "tcp", addr) | ||
| if err == nil { | ||
| _ = conn.Close() | ||
| return portOpen | ||
| } | ||
| if ne, ok := err.(net.Error); ok && ne.Timeout() { | ||
| return portUnknown | ||
| } | ||
| return portClosed | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
DNS-resolution failures are classified as portClosed, not portUnknown.
classifyDial only treats net.Error.Timeout() as portUnknown; a DNS lookup failure (e.g. temporary resolution issue) surfaces as a non-timeout error and falls through to portClosed. This is mostly cushioned by the multi-host "reachable if open/unknown on ANY host" logic, but for single-host scans (or all hosts failing DNS transiently) it could cause an incorrect "definitively closed" prune, violating the losslessness guarantee.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/runner/reachability.go` around lines 269 - 281, Update classifyDial
to classify transient DNS-resolution failures as portUnknown rather than
portClosed, while preserving portOpen and existing timeout handling. Detect the
relevant DNS/network error condition from the dial result before the final
portClosed fallback, ensuring single-host scans do not treat temporary
resolution failures as definitive closure.
| PerHostRateLimit: options.PerHostRateLimit, | ||
| LeaveDefaultPorts: options.LeaveDefaultPorts, | ||
| AutomaticScan: options.AutomaticScan, | ||
| StrictProbe: options.StrictProbe, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Misaligned struct field breaks gofmt formatting.
StrictProbe: doesn't line up with the other aligned fields in this literal (e.g. AutomaticScan:, Silent:). Run go fmt ./... to fix the column alignment.
As per coding guidelines, "**/*.go: Format Go code using go fmt ./...".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/types/types.go` at line 587, Run go fmt ./... to reformat the struct
literal containing StrictProbe, ensuring its field alignment matches
AutomaticScan, Silent, and the other entries.
Source: Coding guidelines
| -ss, -scan-strategy value strategy to use while scanning(auto/host-spray/template-spray) (default auto) | ||
| -irt, -input-read-timeout value timeout on input read (default 3m0s) | ||
| -nh, -no-httpx disable httpx probing for non-url input | ||
| -stp, -strict-probe skip templates whose target service is unreachable (HTTP on non-web ports, network on closed ports; lossless) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Localize the strict-probe description in every translated README.
The new flag is documented with the same English text across all localized files, so users reading those translations receive untranslated help text.
README_CN.md#L309-L309: translate the description into Chinese.README_ES.md#L309-L309: translate the description into Spanish.README_ID.md#L309-L309: translate the description into Indonesian.README_JP.md#L309-L309: translate the description into Japanese.README_KR.md#L309-L309: translate the description into Korean.README_PT-BR.md#L309-L309: translate the description into Brazilian Portuguese.README_TR.md#L309-L309: translate the description into Turkish.
📍 Affects 7 files
README_CN.md#L309-L309(this comment)README_ES.md#L309-L309README_ID.md#L309-L309README_JP.md#L309-L309README_KR.md#L309-L309README_PT-BR.md#L309-L309README_TR.md#L309-L309
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README_CN.md` at line 309, Translate the strict-probe description while
preserving the flag name, behavior, and lossless qualifier: update README_CN.md
lines 309-309 in Chinese, README_ES.md lines 309-309 in Spanish, README_ID.md
lines 309-309 in Indonesian, README_JP.md lines 309-309 in Japanese,
README_KR.md lines 309-309 in Korean, README_PT-BR.md lines 309-309 in Brazilian
Portuguese, and README_TR.md lines 309-309 in Turkish.
1 participant
Adds
-strict-probe/-stp: skip templates whose target service isn't reachable — HTTP/headless on hosts httpx can't confirm, and network templates on closed ports. No raw-input fallback, no findings lost.Supersedes #7592 (extends its strict-probe from HTTP to network reachability). Also carries the http client-pool idle-conn autotune.
Closes #6651
Summary by CodeRabbit
New Features
-stp/-strict-probeoption to skip templates when target services are unreachable, including unavailable HTTP services and closed TCP ports.Documentation