scan filters - #7622
Conversation
WalkthroughThe PR adds strict reachability and technology filtering to scan planning, skips incompatible template-target pairs, adds scan-scoped HTTP response caching, exposes CLI and SDK options, updates localized documentation, and derives bounded per-host HTTP idle connection capacity. ChangesScan planning and target filtering
HTTP transport and response reuse
Documentation Estimated code review effort: 5 (Critical) | ~100 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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
Neo - PR Security ReviewNo security issues found in the delta since What Neo reviewed
Comment |
There was a problem hiding this comment.
Actionable comments posted: 12
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)
693-700: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTwo points on the load-time protocol exclusion.
- The code appends to
r.options.ExcludeProtocols, which mutates shared options. When an SDK caller reuses the sametypes.Optionsacross runs, the exclusion persists into later scans that may have reachable HTTP targets. Build a local copy forloaderConfig, or record the exclusion so it is not appended twice.- Line 699 calls
gologger.Info()while the surrounding code usesr.Logger. User.Loggerso the message honors the per-runner logger configuration.🤖 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/runner.go` around lines 693 - 700, Update the load-time exclusion flow around strictProbeEnabled and noHTTPServiceReachable so web-protocol exclusions are applied to a local loader configuration or otherwise do not mutate shared r.options.ExcludeProtocols across runs. Preserve the exclusion for the current scan and prevent duplicate additions, and replace gologger.Info().Msgf with the runner-specific r.Logger logging path.
🧹 Nitpick comments (10)
pkg/core/executors.go (1)
171-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the filtered-skip progress accounting into one helper. Both execution paths repeat the same nil checks, the same
ExpectedRequestsOverride <= 0guard, and the same minimum-one-request clamp. One helper onEnginekeeps the accounting rule in a single place.
pkg/core/executors.go#L171-L184: replace the inline accounting block with a call to a newEnginemethod, for examplee.creditFilteredSkip(template).pkg/core/executors.go#L239-L248: replace the inline accounting block with the same helper call, usingtpl.🤖 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/core/executors.go` around lines 171 - 184, Extract the duplicated filtered-skip progress accounting into an Engine helper such as creditFilteredSkip(template), preserving the existing nil checks, ExpectedRequestsOverride <= 0 guard, and minimum-one-request clamp. Replace the inline block at pkg/core/executors.go lines 171-184 with the helper call, and replace the corresponding block at lines 239-248 with the same helper using tpl.pkg/core/execute_options.go (1)
70-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe override ignores the clustering reduction.
ExpectedRequestsOverridecomes from the planner, which estimates against the unclustered template set.totalReqAfterClusteringaccounts for merged requests. When both clustering and reachability filtering apply, the progress total can overstate the real work and the bar can finish below 100%. This affects only the progress display.Consider scaling the override by the clustering ratio, or documenting that the override wins by design.
🤖 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/core/execute_options.go` around lines 70 - 77, The progress total in the initialization logic should account for clustering when applying ExpectedRequestsOverride. Scale the planner’s override using the ratio between totalReqAfterClustering and the unclustered request count, while preserving the existing clustered total when no override is provided, so Progress.Init reflects the actual work and reaches completion.pkg/templates/cluster.go (1)
344-357: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid the per-call slice allocation when nothing is filtered.
operatorsForInputruns on every input execution and always allocates. When every member passes, return the original slice.♻️ Proposed change
out := make([]*clusteredOperator, 0, len(e.operators)) for _, op := range e.operators { if e.options.ClusterMemberFilter(op.templateID, op.templateInfo, mi) { out = append(out, op) } } + if len(out) == len(e.operators) { + return e.operators + } return out🤖 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/templates/cluster.go` around lines 344 - 357, Update ClusterExecuter.operatorsForInput to return e.operators directly when the filter allows every member, avoiding allocation in the common unfiltered-result case; only allocate and return a separate slice once a member is excluded, while preserving the existing nil-filter behavior.pkg/core/plan/plan_test.go (1)
79-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the probe-budget fall-through branch.
decideReachabilityskips probing only whenprobeCost > max(baseline*2, 500)andGroupCount <= 1. The case whereprobeCost > baselinebut stays under that bound still returns a full probe. No test covers that branch. A test would lock the budget threshold.🤖 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/core/plan/plan_test.go` around lines 79 - 107, Add a test alongside TestDecideReachabilityProbeBudgetSkip and TestDecideReachabilityFull that exercises decideReachability when probeCost exceeds baseline but does not exceed max(baseline*2, 500), with GroupCount <= 1. Assert that the decision performs a full probe, builds reachability, enables the reachability filter, and reports the full-reachability reason.pkg/core/plan/plan.go (1)
168-173: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the local
maxhelper. The module targets Go 1.26, so the existing calls can use the Go 1.21+ builtinmax.🤖 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/core/plan/plan.go` around lines 168 - 173, Remove the local max helper function and update its call sites to use Go’s builtin max directly, preserving the existing argument order and behavior.pkg/core/techfilter/techfilter_test.go (2)
62-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the dead branch at lines 62-64.
&& falsemakes the condition unreachable. The block is a leftover debug artifact, and the assertion that follows at line 65 already covers the case.♻️ Proposed cleanup
- if !Allow(nginx, cms) && false { - // cms macro is on nginx? No — nginx profile has webserver, not cms. - } if Allow(nginx, cms) { t.Fatal("cms-bound template must not run on nginx-only host") }🤖 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/core/techfilter/techfilter_test.go` around lines 62 - 67, Remove the unreachable `if !Allow(nginx, cms) && false` debug branch from the test, leaving the existing `Allow(nginx, cms)` assertion unchanged.
41-86: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a case for templates whose only non-generic tags are year or source tags.
TemplateProductTagsbinds any tag that is missing fromgenericTags. A template taggedcve,cve2021,oastis therefore reported as tech-bound and is skipped on every fingerprinted host. A test such asAllow(nginx, tagged("cve", "cve2021", "oast"))documents the intended behavior and would catch the classification gap described onpkg/core/techfilter/techfilter.go.🤖 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/core/techfilter/techfilter_test.go` around lines 41 - 86, Extend TestAllowFailOpenAndMatch with a template tagged only by a generic tag plus year/source tags, such as cve, cve2021, and oast, and assert Allow returns true for the nginx profile. Use the existing tagged helper and test structure to document that year/source-only metadata remains runnable rather than being classified as tech-bound.internal/runner/runner.go (2)
984-984: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRun
go fmt ./...on the branch. Two literals appear to be misaligned, which indicates the changed files were not formatted before commit.gofmt -lfails CI on either one.
internal/runner/runner.go#L984: alignTechBoundTemplates:with the otherscanplan.Inputfields.internal/runner/host_group_test.go#L110-L114: align the value column of thebyInputmap literal.As per coding guidelines, "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 `@internal/runner/runner.go` at line 984, Run go fmt ./... to format the branch, ensuring TechBoundTemplates aligns with the other scanplan.Input fields in internal/runner/runner.go:984 and the byInput map literal’s value column is aligned in internal/runner/host_group_test.go:110-114.Source: Coding guidelines
800-802: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the same strict-probe predicate in both branches.
Line 796 and line 797 call
r.strictProbeEnabled(). Line 800 readsr.options.StrictProbedirectly. IfstrictProbeEnabled()applies any additional condition, the warning fires in cases where strict probe is not actually active, or stays silent when it is.♻️ Proposed change
- } else if r.options.StrictProbe && r.options.DisableHTTPProbe { + } else if r.strictProbeEnabled() && r.options.DisableHTTPProbe {🤖 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/runner.go` around lines 800 - 802, Update the else-if condition in the runner validation flow to use r.strictProbeEnabled() instead of reading r.options.StrictProbe directly. Keep the existing r.options.DisableHTTPProbe check and warning unchanged.internal/runner/reachability_test.go (1)
111-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider making these two probes independent of the ambient network.
Both tests depend on external network behavior. A resolver that hijacks NXDOMAIN can return an address for
no-such-host.invalid, and some networks answer TEST-NET-1 with an ICMP unreachable instead of dropping the packet. In both casesclassifyDialcan returnportClosedand the test fails for environmental reasons.A deterministic option is to inject a fake dial function that returns the specific error class under test, since
classifyDialalready accepts aDialContextparameter.♻️ Example of a hermetic variant
func TestClassifyDialTimeoutIsUnknown(t *testing.T) { - d := &net.Dialer{} - // Blackhole-ish: TEST-NET-1 drop. Short timeout should classify unknown. - got := classifyDial(d.DialContext, "192.0.2.1:65535", 50*time.Millisecond) + dial := func(ctx context.Context, network, addr string) (net.Conn, error) { + <-ctx.Done() + return nil, ctx.Err() + } + got := classifyDial(dial, "192.0.2.1:65535", 50*time.Millisecond) if got != portUnknown { t.Fatalf("classifyDial(timeout) = %v, want portUnknown", got) } }🤖 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_test.go` around lines 111 - 127, Make TestClassifyDialDNSFailureIsUnknown and TestClassifyDialTimeoutIsUnknown hermetic by replacing their real net.Dialer calls with injected fake DialContext functions that return representative DNS-resolution and timeout errors respectively. Continue invoking classifyDial through its DialContext parameter and preserve the expected portUnknown assertions without relying on external host or network behavior.
🤖 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/host_group.go`:
- Around line 291-297: Update the openByHost initialization in the host-group
flow so explicit ports are pre-seeded only when probePorts is false; when
probing is enabled, let probe results determine whether the port remains open
while preserving the existing non-probed fallback.
- Around line 380-405: Update resolveHTTPReachability to detect
MultiFormatInputProvider inputs before treating missing helper.InputsHTTP
entries as non-HTTP. Preserve the existing fallback reachability checks for
these inputs, or bypass the reachability filter for them, so web templates are
not skipped when request.URL.String() lacks an HTTP scheme.
In `@internal/runner/runner.go`:
- Around line 1003-1039: Update executeTemplatesWithScanPlan so failures from
buildHostReachability and buildTechReachability are logged and do not return an
error. When either helper fails, discard its index/filter, retain the full
unfiltered template plan, and continue scan execution; preserve the existing
optimized path when the helper succeeds.
In `@internal/runner/tech_filter.go`:
- Around line 163-188: Update estimateTechFilteredExecutions and the
execution-time filtering path to precompute each template’s TemplateProductTags
once and reuse the result when matching profiles, rather than calling
techfilter.Allow for every template-host pair. Store the cached tags on the
template or in techReachabilityIndex, and preserve the existing workflow,
missing-host, and request-count behavior.
- Around line 95-125: Update buildTechReachability to resolve bare inputs
through the resolved URLs stored by InputsHTTP before calling fingerprintTarget,
while preserving the existing scheme order for targets without a mapped URL.
Track the number of eligible HTTP targets separately from len(idx.byInput), and
use that count in the fingerprinted summary so unresolvable or untagged targets
are represented accurately.
- Around line 147-153: Update the cache lookup and seeding flow around the
existing cache and signature handling so requests whose headers, cookies, or
signature state affect the response are not served from the method-and-URL-only
cache; either incorporate those request semantics into the cache key or bypass
caching for such requests. Ensure cache lookup occurs after signature handling
and CookieJar attachment, and apply the same eligibility rule when seeding both
seedURL and resp.Request.URL.
In `@pkg/core/plan/plan_test.go`:
- Around line 155-189: Run gofmt (or go fmt ./...) to align the
TechBoundTemplates fields in the Input struct literals used by
TestDecideTechFilterOffByDefault, TestDecideTechFilterSkippedWhenNoBound, and
TestDecideTechFilterOn, without changing test behavior.
In `@pkg/core/plan/plan.go`:
- Around line 53-65: Rename the local variable stratReason to strategyReason
throughout the surrounding plan-building logic, including its declaration and
use when assigning p.Reason; preserve all existing behavior.
In `@pkg/core/techfilter/techfilter.go`:
- Around line 157-174: Update Allow so non-matching templates are filtered only
when profile.Tags contains at least one tag from the same category macro as the
template’s bound product tags, rather than relying on the broad
profile.HasTags() check. Preserve fail-open behavior for nil or unbound
templates and profiles without relevant category tags, while retaining filtering
for well-fingerprinted stacks.
- Around line 17-29: Update TemplateProductTags and its genericTags
classification so unknown non-product tags remain generic instead of becoming
product bindings; prefer binding only tags recognized by the known
Wappalyzer/category vocabulary or productAliases. Preserve legitimate
product/category matching while allowing templates containing tags such as year,
source, and technique tokens to fail open rather than being silently rejected by
Allow.
In `@pkg/protocols/http/httprespcache/cache.go`:
- Around line 54-67: Bind cached responses to the effective request context:
update httprespcache.Key and KeyFromRequest to include Request.Host and all
representation-changing headers, including authorization and cookies, in a
deterministic cache identity. In pkg/protocols/http/request.go:844-855, move
lookup until after input.CookieJar is applied or bypass the cache whenever
cookie-jar or other unrepresented state is used. Add regression tests covering
distinct Host overrides, authorization headers, and cookie-jar sessions.
- Around line 106-119: Update the cache Set path around Entry creation and
c.entries insertion to enforce scan-wide byte and entry limits, rejecting or
evicting responses when either budget is exhausted. Add the required cache
configuration/state and ensure accounting is updated consistently when entries
are added or removed. Extend tests covering both byte-budget and entry-limit
behavior.
---
Outside diff comments:
In `@internal/runner/runner.go`:
- Around line 693-700: Update the load-time exclusion flow around
strictProbeEnabled and noHTTPServiceReachable so web-protocol exclusions are
applied to a local loader configuration or otherwise do not mutate shared
r.options.ExcludeProtocols across runs. Preserve the exclusion for the current
scan and prevent duplicate additions, and replace gologger.Info().Msgf with the
runner-specific r.Logger logging path.
---
Nitpick comments:
In `@internal/runner/reachability_test.go`:
- Around line 111-127: Make TestClassifyDialDNSFailureIsUnknown and
TestClassifyDialTimeoutIsUnknown hermetic by replacing their real net.Dialer
calls with injected fake DialContext functions that return representative
DNS-resolution and timeout errors respectively. Continue invoking classifyDial
through its DialContext parameter and preserve the expected portUnknown
assertions without relying on external host or network behavior.
In `@internal/runner/runner.go`:
- Line 984: Run go fmt ./... to format the branch, ensuring TechBoundTemplates
aligns with the other scanplan.Input fields in internal/runner/runner.go:984 and
the byInput map literal’s value column is aligned in
internal/runner/host_group_test.go:110-114.
- Around line 800-802: Update the else-if condition in the runner validation
flow to use r.strictProbeEnabled() instead of reading r.options.StrictProbe
directly. Keep the existing r.options.DisableHTTPProbe check and warning
unchanged.
In `@pkg/core/execute_options.go`:
- Around line 70-77: The progress total in the initialization logic should
account for clustering when applying ExpectedRequestsOverride. Scale the
planner’s override using the ratio between totalReqAfterClustering and the
unclustered request count, while preserving the existing clustered total when no
override is provided, so Progress.Init reflects the actual work and reaches
completion.
In `@pkg/core/executors.go`:
- Around line 171-184: Extract the duplicated filtered-skip progress accounting
into an Engine helper such as creditFilteredSkip(template), preserving the
existing nil checks, ExpectedRequestsOverride <= 0 guard, and
minimum-one-request clamp. Replace the inline block at pkg/core/executors.go
lines 171-184 with the helper call, and replace the corresponding block at lines
239-248 with the same helper using tpl.
In `@pkg/core/plan/plan_test.go`:
- Around line 79-107: Add a test alongside TestDecideReachabilityProbeBudgetSkip
and TestDecideReachabilityFull that exercises decideReachability when probeCost
exceeds baseline but does not exceed max(baseline*2, 500), with GroupCount <= 1.
Assert that the decision performs a full probe, builds reachability, enables the
reachability filter, and reports the full-reachability reason.
In `@pkg/core/plan/plan.go`:
- Around line 168-173: Remove the local max helper function and update its call
sites to use Go’s builtin max directly, preserving the existing argument order
and behavior.
In `@pkg/core/techfilter/techfilter_test.go`:
- Around line 62-67: Remove the unreachable `if !Allow(nginx, cms) && false`
debug branch from the test, leaving the existing `Allow(nginx, cms)` assertion
unchanged.
- Around line 41-86: Extend TestAllowFailOpenAndMatch with a template tagged
only by a generic tag plus year/source tags, such as cve, cve2021, and oast, and
assert Allow returns true for the nginx profile. Use the existing tagged helper
and test structure to document that year/source-only metadata remains runnable
rather than being classified as tech-bound.
In `@pkg/templates/cluster.go`:
- Around line 344-357: Update ClusterExecuter.operatorsForInput to return
e.operators directly when the filter allows every member, avoiding allocation in
the common unfiltered-result case; only allocate and return a separate slice
once a member is excluded, while preserving the existing nil-filter behavior.
🪄 Autofix
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: 96884e64-e874-44bf-976a-3b40bd89eff1
📒 Files selected for processing (22)
README.mdcmd/nuclei/main.gointernal/runner/host_group.gointernal/runner/host_group_test.gointernal/runner/reachability.gointernal/runner/reachability_test.gointernal/runner/runner.gointernal/runner/tech_filter.gopkg/core/engine.gopkg/core/execute_options.gopkg/core/executors.gopkg/core/plan/plan.gopkg/core/plan/plan_test.gopkg/core/techfilter/techfilter.gopkg/core/techfilter/techfilter_test.gopkg/protocols/common/protocolstate/memoizer.gopkg/protocols/http/httprespcache/cache.gopkg/protocols/http/httprespcache/cache_test.gopkg/protocols/http/request.gopkg/protocols/protocols.gopkg/templates/cluster.gopkg/types/types.go
🚧 Files skipped from review as they are similar to previous changes (4)
- README.md
- pkg/types/types.go
- cmd/nuclei/main.go
- internal/runner/reachability.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@pkg/protocols/http/httprespcache/cache.go`:
- Around line 99-106: Update the header filtering in KeyFromRequest to prevent
cached responses from being reused across differing representation contexts:
reject User-Agent, Accept, Accept-Language, Accept-Encoding,
Upgrade-Insecure-Requests, and Cache-Control requests, always bypassing
Cache-Control rather than allowing it through. Preserve caching only for
requests with headers that cannot alter the response, and add regression
coverage using distinct Accept-Language or User-Agent values.
🪄 Autofix
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: cf02f85b-ce37-4996-b02e-46dc7c46871a
📒 Files selected for processing (11)
internal/runner/host_group.gointernal/runner/host_group_test.gointernal/runner/runner.gointernal/runner/tech_filter.gointernal/runner/tech_filter_test.gopkg/core/techfilter/techfilter.gopkg/core/techfilter/techfilter_test.gopkg/input/transform.gopkg/protocols/http/httprespcache/cache.gopkg/protocols/http/httprespcache/cache_test.gopkg/protocols/http/request.go
🚧 Files skipped from review as they are similar to previous changes (6)
- pkg/protocols/http/request.go
- pkg/input/transform.go
- internal/runner/tech_filter.go
- internal/runner/host_group_test.go
- internal/runner/host_group.go
- internal/runner/runner.go
| for k := range req.Header { | ||
| switch strings.ToLower(k) { | ||
| case "user-agent", "accept", "accept-language", "accept-encoding", | ||
| "connection", "upgrade-insecure-requests", "cache-control", "pragma": | ||
| continue | ||
| default: | ||
| return false | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not reuse responses across different header contexts.
KeyFromRequest uses only the method and URL. This allow-list accepts User-Agent, Accept*, Upgrade-Insecure-Requests, and Cache-Control. These headers can change the response representation or require a cache bypass. Two templates can then receive the wrong cached response for the same URL.
Reject these headers from caching, or add a deterministic representation-header identity to the cache key. Always bypass requests with Cache-Control. Add regression tests with distinct Accept-Language or User-Agent values.
🤖 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/protocols/http/httprespcache/cache.go` around lines 99 - 106, Update the
header filtering in KeyFromRequest to prevent cached responses from being reused
across differing representation contexts: reject User-Agent, Accept,
Accept-Language, Accept-Encoding, Upgrade-Insecure-Requests, and Cache-Control
requests, always bypassing Cache-Control rather than allowing it through.
Preserve caching only for requests with headers that cannot alter the response,
and add regression coverage using distinct Accept-Language or User-Agent values.
1 participant
ScanPlan (
-ss auto), reachability filter (-stp), opt-in tech filter (-tf+ shared GET cache).-stpskips unreachable template×host pairs; explicithost:portkeeps network templates on that port.-tffingerprints HTTP hosts and skips unmatched product/macro tags (off by default).Bench (local Docker)
-tfwall (20 nginx, 280 tpl)-tfexec-tfHTTP conns-stpexec mixed (21 hosts, 400 tpl)-stpwall web-heavy (38 hosts, 705 tpl)-stpexec web-heavyCloses #6651
Supersedes #7592
Summary by CodeRabbit
New Features
-strict-probe(-stp) to skip templates targeting unreachable services.-tech-filter(-tf) to run technology-specific templates only when matching technologies are detected.Performance
Documentation