Conversation
Neo - PR Security ReviewNo security issues found Hardening Notes
Comment |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds per-host HTTP Server-header detection and a HostTechCache to record technology hints and skip templates whose tags don't match detected tech; exposes a CLI flag to disable this filtering. Changes
Sequence DiagramsequenceDiagram
participant Client as HTTP Client
participant HTTP as HTTP Requester
participant Cache as HostTechCache
participant Executor as Template Executor
participant Engine as Core Engine
Client->>HTTP: Send request to target URL
HTTP->>Client: Receive response (may include Server header)
HTTP->>Cache: RecordServerHeader(host, serverHeader)
Cache->>Cache: Parse header -> required tags set
Executor->>Engine: Start ExecuteWithResults(ctx)
Engine->>Executor: Provide ExecutorOptions (includes HostTechCache)
Executor->>Cache: ShouldSkipTemplate(host, templateTags)
Cache-->>Executor: skip? (true/false)
alt not skipped
Executor->>Engine: Run template execution -> produce results
else skipped
Executor-->>Engine: Return early (no results)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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: 5
🧹 Nitpick comments (9)
cmd/nuclei/main.go (1)
437-445: Flag placement in "headless" group is misleading.The
-disable-tech-filterflag is placed in the "Headless" group, but this feature relates to HTTP response header analysis for template filtering, not headless browser functionality. Consider moving it to the "Optimizations" or "Filtering" group for better discoverability.♻️ Suggested placement in "optimization" group
Move the flag to line ~435 in the "optimization" group:
flagSet.CreateGroup("optimization", "Optimizations", flagSet.IntVar(&options.Timeout, "timeout", 10, "time to wait in seconds before timeout"), + flagSet.BoolVarP(&options.DisableTechStackFiltering, "disable-tech-filter", "dtf", false, "disable automatic template filtering based on Server response header tech detection"), // ... other optimization flags ) flagSet.CreateGroup("headless", "Headless", - flagSet.BoolVarP(&options.DisableTechStackFiltering, "disable-tech-filter", "dtf", false, "disable automatic template filtering based on Server response header tech detection"), flagSet.BoolVar(&options.Headless, "headless", false, "enable templates that require headless browser support (root user on Linux will disable sandbox)"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/nuclei/main.go` around lines 437 - 445, The DisableTechStackFiltering flag (options.DisableTechStackFiltering), currently added via flagSet.BoolVarP inside the flagSet.CreateGroup("headless", ...) block, is misgrouped; remove the flagSet.BoolVarP(&options.DisableTechStackFiltering, "disable-tech-filter", "dtf", ...) entry from the headless group and re-add the exact same BoolVarP call to the appropriate group (e.g., the existing "optimization" or "filtering" flag group CreateGroup call) so the flag appears with related flags (preserve the flag name, shorthand "dtf" and help text); ensure you only move the declaration and not change the option variable name or help string.pkg/protocols/http/request.go (1)
925-933: Inconsistent indentation and nested conditionals.The added code has irregular indentation (mixed tabs/spaces visible in the annotation). Consider flattening the conditionals for clarity:
♻️ Suggested cleanup
- if resp != nil { - if serverHdr := resp.Header.Get("Server"); serverHdr != "" { - if tc := request.options.HostTechCache; tc != nil { - tc.RecordServerHeader(input.MetaInput.Input, serverHdr) - } - } - } - + if resp != nil && request.options.HostTechCache != nil { + if serverHdr := resp.Header.Get("Server"); serverHdr != "" { + request.options.HostTechCache.RecordServerHeader(input.MetaInput.Input, serverHdr) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/http/request.go` around lines 925 - 933, The new Server header handling in request.go uses nested conditionals with inconsistent indentation; please normalize whitespace to the project's style and flatten the logic by combining checks (e.g., if resp != nil && resp.Header.Get("Server") != "" && request.options.HostTechCache != nil) or by early-returning when resp is nil, then getting serverHdr and calling request.options.HostTechCache.RecordServerHeader(input.MetaInput.Input, serverHdr) only when non-empty; ensure you reference resp, serverHdr, request.options.HostTechCache (tc) and tc.RecordServerHeader and keep a single consistent indentation level throughout.pkg/core/hosttechcache/hosttechcache.go (1)
43-45: Limited technology detection — currently only Apache.The detection logic only recognizes
"apache"in the Server header. While this is intentional per the comment ("intentionally simple"), consider documenting a roadmap or TODO for extending this to other common technologies (nginx, IIS, etc.) to make the feature more broadly useful.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/core/hosttechcache/hosttechcache.go` around lines 43 - 45, The current Server header detection only checks for "apache" using strings.Contains(lower, "apache") and appends to requiredTags; add a TODO or roadmap comment near that check (and/or update detection logic) to indicate planned support for other common servers (nginx, iis, lite-speed, cloudflare, etc.), and document where to extend it (the strings.Contains(lower, "...") block that manipulates requiredTags) so future work can add additional branches or a map-driven detector for more technologies.pkg/tmplexec/exec.go (1)
282-285: Remove verbose "ALLOWED" debug logging.Logging every template that is not skipped will be extremely noisy in debug mode, as most templates won't have matching tech hints and will be allowed. Only the "SKIPPED" log is actionable for debugging.
♻️ Remove verbose allowed logging
if tc.ShouldSkipTemplate(host, tags) { gologger.Debug().Msgf("[tech-filter] SKIPPED template '%s' (tags: %v) for host '%s' — no matching tech hint", e.options.TemplateID, tags, host) return nil, nil - } else { - gologger.Debug().Msgf("[tech-filter] ALLOWED template '%s' (tags: %v) for host '%s'", - e.options.TemplateID, tags, host) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/tmplexec/exec.go` around lines 282 - 285, The noisy Debug log for allowed templates should be removed (or suppressed); locate the gologger.Debug().Msgf call inside the tech-filter branch in exec.go (the block that logs "[tech-filter] ALLOWED template '%s' (tags: %v) for host '%s'") and delete that call (or demote it to a less noisy level such as Trace) so only the actionable "SKIPPED" log remains; ensure no other code paths rely on the removed log call.pkg/protocols/protocols.go (3)
93-93: Add documentation comment for consistency.Other fields in
ExecutorOptionshave inline documentation comments explaining their purpose. Consider adding one forHostTechCache:// HostErrorsCache is an optional cache for handling host errors HostErrorsCache hosterrorscache.CacheInterface + // HostTechCache is an optional cache for storing detected technology stack per host HostTechCache *hosttechcache.HostTechCache🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/protocols.go` at line 93, Add an inline documentation comment for the ExecutorOptions struct field HostTechCache explaining its purpose and usage (e.g., that it holds a pointer to a hosttechcache.HostTechCache used for caching host/tech lookup data in executors). Update the comment near the HostTechCache field in the ExecutorOptions definition so it matches the style and level of detail of the other documented fields.
478-479: Formatting issues: inconsistent spacing and missing trailing newline.Line 478 has extra spaces before
=, inconsistent with other assignments in the function. Also, the file is missing a trailing newline. Rungo fmt ./...to fix both.- e.HostTechCache = n.HostTechCache -} + e.HostTechCache = n.HostTechCache +} +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/protocols.go` around lines 478 - 479, The assignment line setting e.HostTechCache = n.HostTechCache has inconsistent spacing and the file lacks a trailing newline; run go fmt (e.g., go fmt ./...) or adjust the formatting so the assignment aligns with the other lines in the function and add a final newline to the file to resolve both issues (look for the assignment of HostTechCache in the function where e.HostTechCache is set).
282-282: Formatting: inconsistent alignment.Extra tabs/spaces compared to adjacent fields. Run
go fmt ./...to normalize.- HostTechCache: e.HostTechCache, + HostTechCache: e.HostTechCache,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/protocols/protocols.go` at line 282, Field alignment for the struct literal includes inconsistent spacing around the HostTechCache entry (e.HostTechCache) compared to adjacent fields; run gofmt (e.g., go fmt ./...) or adjust the whitespace so the struct fields align consistently and remove extra tabs/spaces around the HostTechCache: e.HostTechCache should match the formatting pattern used by the surrounding fields in the same composite literal.pkg/core/engine.go (2)
72-72: Missing trailing newline at end of file.The file should end with a newline character. Run
go fmt ./...to fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/core/engine.go` at line 72, The file pkg/core/engine.go is missing a trailing newline at EOF; open the file (e.g., around the closing brace of the Engine-related code), add a single newline character at the end of the file, save, and run go fmt ./... (or your editor's Go formatting) to ensure the file ends with a newline and formatting is applied.
25-25: Minor formatting: inconsistent field alignment.The
HostTechCachefield uses double tabs between the name and type, while other struct fields use single tabs. Rungo fmt ./...to normalize.- HostTechCache *hosttechcache.HostTechCache + HostTechCache *hosttechcache.HostTechCache🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/core/engine.go` at line 25, The HostTechCache field has inconsistent spacing (double tab) before its type (hosttechcache.HostTechCache); update the struct so the field uses the same single-tab alignment as the other fields (adjust the spacing for the HostTechCache declaration) and then run go fmt ./... to normalize formatting across the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/core/hosttechcache/hosttechcache.go`:
- Around line 39-67: RecordServerHeader currently stores hints keyed by the
transformed input (e.g. full URL) which mismatches lookups in ShouldSkipTemplate
that use the original raw input; fix by normalizing the cache key to the same
host:port form in both places: implement a helper (e.g., normalizeHostKey(input
string) string) that tries net/url.Parse(input) and returns url.Host when parse
succeeds, otherwise returns the original input, and use that helper when setting
c.hints in RecordServerHeader (and in ShouldSkipTemplate lookups) so keys are
consistently host[:port] across recording and lookup; preserve existing locking
around c.hints and keep TechHint/Tags behavior unchanged.
In `@pkg/core/hosttechcache/hosttestcache_test.go`:
- Around line 1-7: The test file name has a typo: rename the test file from
hosttestcache_test.go to hosttechcache_test.go so it matches the package and
implementation (package hosttechcache / hosttechcache.go); update any test
runner references if present and ensure the test functions in the file (e.g.,
any Test... functions) remain unchanged after renaming.
In `@pkg/protocols/http/request.go`:
- Around line 925-933: The code records the Server header from the initial resp
before any redirect handling; to fix this, move the tc.RecordServerHeader call
so it records the Server header from the final response after redirect chain
processing (or alternatively record each redirect response's Server header if
you want multiple hints). Concretely, update the logic around resp and redirect
handling in the request execution flow (look for resp,
request.options.HostTechCache, tc.RecordServerHeader and input.MetaInput.Input)
so that either (a) recording happens once using the last response seen after
redirects, or (b) iterate over the redirect responses and call
tc.RecordServerHeader for each Server header encountered.
In `@pkg/tmplexec/exec.go`:
- Around line 274-287: The tech-filter branch currently returns (nil, nil) which
makes skipped templates indistinguishable from zero-match executions; define a
package-level sentinel error (e.g. ErrTemplateSkipped) in pkg/tmplexec and
change the tc.ShouldSkipTemplate branch to return (nil, ErrTemplateSkipped)
instead of (nil, nil), then update callers (notably executeAllSelfContained and
the workflow executors) to check errors.Is(err, ErrTemplateSkipped) and handle
it as a skip (do not set match = true / treat as successful match) so statistics
and control flow correctly reflect skipped templates; reference symbols:
tc.ShouldSkipTemplate, e.options.TemplateID, executeAllSelfContained, and
ErrTemplateSkipped.
In `@pkg/types/types.go`:
- Around line 135-137: The Options.Copy() method fails to copy the new
DisableTechStackFiltering field, causing it to be lost when duplicating options;
update the Options.Copy() implementation to set the
copied.Options.DisableTechStackFiltering = o.DisableTechStackFiltering (i.e.,
copy the DisableTechStackFiltering boolean from the source Options into the
returned copy), placing it alongside the other scalar copies such as
NoHostErrors to ensure the flag is preserved during duplication.
---
Nitpick comments:
In `@cmd/nuclei/main.go`:
- Around line 437-445: The DisableTechStackFiltering flag
(options.DisableTechStackFiltering), currently added via flagSet.BoolVarP inside
the flagSet.CreateGroup("headless", ...) block, is misgrouped; remove the
flagSet.BoolVarP(&options.DisableTechStackFiltering, "disable-tech-filter",
"dtf", ...) entry from the headless group and re-add the exact same BoolVarP
call to the appropriate group (e.g., the existing "optimization" or "filtering"
flag group CreateGroup call) so the flag appears with related flags (preserve
the flag name, shorthand "dtf" and help text); ensure you only move the
declaration and not change the option variable name or help string.
In `@pkg/core/engine.go`:
- Line 72: The file pkg/core/engine.go is missing a trailing newline at EOF;
open the file (e.g., around the closing brace of the Engine-related code), add a
single newline character at the end of the file, save, and run go fmt ./... (or
your editor's Go formatting) to ensure the file ends with a newline and
formatting is applied.
- Line 25: The HostTechCache field has inconsistent spacing (double tab) before
its type (hosttechcache.HostTechCache); update the struct so the field uses the
same single-tab alignment as the other fields (adjust the spacing for the
HostTechCache declaration) and then run go fmt ./... to normalize formatting
across the codebase.
In `@pkg/core/hosttechcache/hosttechcache.go`:
- Around line 43-45: The current Server header detection only checks for
"apache" using strings.Contains(lower, "apache") and appends to requiredTags;
add a TODO or roadmap comment near that check (and/or update detection logic) to
indicate planned support for other common servers (nginx, iis, lite-speed,
cloudflare, etc.), and document where to extend it (the strings.Contains(lower,
"...") block that manipulates requiredTags) so future work can add additional
branches or a map-driven detector for more technologies.
In `@pkg/protocols/http/request.go`:
- Around line 925-933: The new Server header handling in request.go uses nested
conditionals with inconsistent indentation; please normalize whitespace to the
project's style and flatten the logic by combining checks (e.g., if resp != nil
&& resp.Header.Get("Server") != "" && request.options.HostTechCache != nil) or
by early-returning when resp is nil, then getting serverHdr and calling
request.options.HostTechCache.RecordServerHeader(input.MetaInput.Input,
serverHdr) only when non-empty; ensure you reference resp, serverHdr,
request.options.HostTechCache (tc) and tc.RecordServerHeader and keep a single
consistent indentation level throughout.
In `@pkg/protocols/protocols.go`:
- Line 93: Add an inline documentation comment for the ExecutorOptions struct
field HostTechCache explaining its purpose and usage (e.g., that it holds a
pointer to a hosttechcache.HostTechCache used for caching host/tech lookup data
in executors). Update the comment near the HostTechCache field in the
ExecutorOptions definition so it matches the style and level of detail of the
other documented fields.
- Around line 478-479: The assignment line setting e.HostTechCache =
n.HostTechCache has inconsistent spacing and the file lacks a trailing newline;
run go fmt (e.g., go fmt ./...) or adjust the formatting so the assignment
aligns with the other lines in the function and add a final newline to the file
to resolve both issues (look for the assignment of HostTechCache in the function
where e.HostTechCache is set).
- Line 282: Field alignment for the struct literal includes inconsistent spacing
around the HostTechCache entry (e.HostTechCache) compared to adjacent fields;
run gofmt (e.g., go fmt ./...) or adjust the whitespace so the struct fields
align consistently and remove extra tabs/spaces around the HostTechCache:
e.HostTechCache should match the formatting pattern used by the surrounding
fields in the same composite literal.
In `@pkg/tmplexec/exec.go`:
- Around line 282-285: The noisy Debug log for allowed templates should be
removed (or suppressed); locate the gologger.Debug().Msgf call inside the
tech-filter branch in exec.go (the block that logs "[tech-filter] ALLOWED
template '%s' (tags: %v) for host '%s'") and delete that call (or demote it to a
less noisy level such as Trace) so only the actionable "SKIPPED" log remains;
ensure no other code paths rely on the removed log call.
🪄 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
Run ID: 8df61c21-6359-4cf6-b2e7-84e2cac8e032
📒 Files selected for processing (10)
cmd/nuclei/main.gopkg/core/engine.gopkg/core/execute_options.gopkg/core/executors.gopkg/core/hosttechcache/hosttechcache.gopkg/core/hosttechcache/hosttestcache_test.gopkg/protocols/http/request.gopkg/protocols/protocols.gopkg/tmplexec/exec.gopkg/types/types.go
| func (c *HostTechCache) RecordServerHeader(host, serverHeader string) { | ||
| lower := strings.ToLower(serverHeader) | ||
|
|
||
| var requiredTags []string | ||
| if strings.Contains(lower, "apache") { | ||
| requiredTags = append(requiredTags, "apache") | ||
| } | ||
|
|
||
| c.mu.Lock() | ||
| defer c.mu.Unlock() | ||
|
|
||
| if len(requiredTags) == 0 { | ||
| if _, exists := c.hints[host]; exists { | ||
| gologger.Debug().Msgf("[tech-filter] CLEARED hint for host '%s' (unrecognised Server header: '%s')", | ||
| host, serverHeader) | ||
| } | ||
| delete(c.hints, host) | ||
| return | ||
| } | ||
|
|
||
| gologger.Debug().Msgf("[tech-filter] RECORDED hint for host '%s' — Server: '%s' → required tags: %v", | ||
| host, serverHeader, requiredTags) | ||
|
|
||
| hint := &TechHint{Tags: make(map[string]struct{}, len(requiredTags))} | ||
| for _, t := range requiredTags { | ||
| hint.Tags[t] = struct{}{} | ||
| } | ||
| c.hints[host] = hint | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how MetaInput.Input is populated in different contexts
# Check HTTP request context - what value is passed to RecordServerHeader
rg -n -A5 'RecordServerHeader\(' --type go
# Check template executor context - what value is passed to ShouldSkipTemplate
rg -n -A5 'ShouldSkipTemplate\(' --type go
# Check how MetaInput.Input is set/modified during execution
rg -n -B3 -A3 'MetaInput\.Input\s*=' --type goRepository: projectdiscovery/nuclei
Length of output: 22286
🏁 Script executed:
# Check the context around where ShouldSkipTemplate is called in tmplexec/exec.go
rg -n -B15 'ShouldSkipTemplate\(' pkg/tmplexec/exec.go | head -60Repository: projectdiscovery/nuclei
Length of output: 783
🏁 Script executed:
# Check the context around line 928 in http/request.go where RecordServerHeader is called
rg -n -B20 'RecordServerHeader\(' pkg/protocols/http/request.go | head -80Repository: projectdiscovery/nuclei
Length of output: 891
🏁 Script executed:
# Check what GetCopyIfHostOutdated does and how MetaInput.Input is modified
rg -n -B5 -A10 'GetCopyIfHostOutdated\(input' pkg/protocols/http/request.go | head -100Repository: projectdiscovery/nuclei
Length of output: 2050
🏁 Script executed:
# Check how ctx.Input is created/passed in template execution
rg -n -B10 'ExecuteWithResults' pkg/tmplexec/exec.go | grep -A10 "func.*ExecuteWithResults"Repository: projectdiscovery/nuclei
Length of output: 525
🏁 Script executed:
# Check how ctx (ScanContext) Input is set up initially
rg -n 'type ScanContext' --type go -A20Repository: projectdiscovery/nuclei
Length of output: 1467
🏁 Script executed:
# Look for how template execution flow receives its input context
rg -n 'ExecuteWithResults' pkg/runner/runner.go -B10 -A5Repository: projectdiscovery/nuclei
Length of output: 127
🏁 Script executed:
# Search for where ExecuteWithResults is called
rg -n 'ExecuteWithResults' --type go -B5 -A2Repository: projectdiscovery/nuclei
Length of output: 40798
🏁 Script executed:
# Search for InputHelper and what it transforms
rg -n 'type InputHelper' --type go -A20Repository: projectdiscovery/nuclei
Length of output: 49
🏁 Script executed:
# Search for InputHelper interface/struct definition
rg -n 'InputHelper' --type go | head -40Repository: projectdiscovery/nuclei
Length of output: 2668
🏁 Script executed:
# Find the input.Helper implementation and its Transform method
fd -e go 'input' pkg/ | xargs grep -l 'type Helper' | head -5Repository: projectdiscovery/nuclei
Length of output: 49
🏁 Script executed:
# Search for input package and Helper type
rg -n 'package input' --type go | head -10Repository: projectdiscovery/nuclei
Length of output: 149
🏁 Script executed:
# Check the input package files
ls -la pkg/input/Repository: projectdiscovery/nuclei
Length of output: 544
🏁 Script executed:
# Read the transform.go file to see Helper and Transform implementation
cat -n pkg/input/transform.goRepository: projectdiscovery/nuclei
Length of output: 4978
🏁 Script executed:
# Check the test file to understand how inputs are actually passed
cat -n pkg/core/hosttechcache/hosttestcache_test.go | head -100Repository: projectdiscovery/nuclei
Length of output: 4607
🏁 Script executed:
# Check how input is initially created and what format it has
rg -n 'NewWithInput\|ctx.Input.MetaInput.Input =' pkg/core/execute_options.go pkg/core/executors.goRepository: projectdiscovery/nuclei
Length of output: 49
🏁 Script executed:
# Check exactly where in the HTTP request flow RecordServerHeader is called vs transformations
sed -n '925,935p' pkg/protocols/http/request.goRepository: projectdiscovery/nuclei
Length of output: 498
Normalize host key consistently between cache recording and lookups to prevent cache misses.
The ShouldSkipTemplate check at template execution time uses ctx.Input.MetaInput.Input in its original form, but RecordServerHeader is called after the HTTP protocol executor has processed the input through InputHelper.Transform(). For HTTP protocol requests, this transforms raw inputs (like "example.com") into full URLs (like "https://example.com"), causing a key format mismatch.
Scenario where this fails:
- Template execution starts with original input
"example.com" ShouldSkipTemplate()checks cache with key"example.com"→ miss (cache empty)- HTTP protocol transforms input to
"https://example.com"viaInputHelper.Transform() RecordServerHeader()records hint with key"https://example.com"- Next scan of
"example.com"looks up"example.com"again → still a miss, despite hint existing
Apply the suggested normalization approach to extract a consistent key (host+port only) in both RecordServerHeader and ShouldSkipTemplate methods:
🔧 Suggested normalization
+import (
+ "net"
+ "strings"
+ urlutil "github.com/projectdiscovery/utils/url"
+)
+
+// normalizeHost extracts host:port or just host from various input formats
+func normalizeHost(input string) string {
+ if parsed, err := urlutil.Parse(input); err == nil && parsed.Host != "" {
+ // Extract just the host part (removing path, query, fragment)
+ return strings.ToLower(parsed.Host)
+ }
+ return strings.ToLower(input)
+}
+
func (c *HostTechCache) RecordServerHeader(host, serverHeader string) {
+ host = normalizeHost(host)
lower := strings.ToLower(serverHeader)
// ... rest of function
}
func (c *HostTechCache) ShouldSkipTemplate(host string, templateTags []string) bool {
+ host = normalizeHost(host)
c.mu.RLock()
// ... rest of function
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/core/hosttechcache/hosttechcache.go` around lines 39 - 67,
RecordServerHeader currently stores hints keyed by the transformed input (e.g.
full URL) which mismatches lookups in ShouldSkipTemplate that use the original
raw input; fix by normalizing the cache key to the same host:port form in both
places: implement a helper (e.g., normalizeHostKey(input string) string) that
tries net/url.Parse(input) and returns url.Host when parse succeeds, otherwise
returns the original input, and use that helper when setting c.hints in
RecordServerHeader (and in ShouldSkipTemplate lookups) so keys are consistently
host[:port] across recording and lookup; preserve existing locking around
c.hints and keep TechHint/Tags behavior unchanged.
| package hosttechcache | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "sync" | ||
| "testing" | ||
| ) |
There was a problem hiding this comment.
Filename typo: hosttestcache_test.go should be hosttechcache_test.go.
The test file is named hosttestcache_test.go but the implementation file is hosttechcache.go. This appears to be a typo (test vs tech). Consider renaming for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/core/hosttechcache/hosttestcache_test.go` around lines 1 - 7, The test
file name has a typo: rename the test file from hosttestcache_test.go to
hosttechcache_test.go so it matches the package and implementation (package
hosttechcache / hosttechcache.go); update any test runner references if present
and ensure the test functions in the file (e.g., any Test... functions) remain
unchanged after renaming.
| if resp != nil { | ||
| if serverHdr := resp.Header.Get("Server"); serverHdr != "" { | ||
| if tc := request.options.HostTechCache; tc != nil { | ||
| tc.RecordServerHeader(input.MetaInput.Input, serverHdr) | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Server header is recorded before redirect chain processing.
The Server header is captured from the initial response before the redirect chain is evaluated (line 966+). If the target redirects to a different server technology, the recorded hint may not reflect the final destination's stack. This could lead to incorrect template filtering.
Consider whether recording should happen on the final response in the chain, or if recording multiple hints per redirect is desirable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/protocols/http/request.go` around lines 925 - 933, The code records the
Server header from the initial resp before any redirect handling; to fix this,
move the tc.RecordServerHeader call so it records the Server header from the
final response after redirect chain processing (or alternatively record each
redirect response's Server header if you want multiple hints). Concretely,
update the logic around resp and redirect handling in the request execution flow
(look for resp, request.options.HostTechCache, tc.RecordServerHeader and
input.MetaInput.Input) so that either (a) recording happens once using the last
response seen after redirects, or (b) iterate over the redirect responses and
call tc.RecordServerHeader for each Server header encountered.
| // --- Tech-stack based template filtering --- | ||
| if tc := e.options.HostTechCache; tc != nil { | ||
| tags := e.options.TemplateInfo.Tags.ToSlice() | ||
| host := ctx.Input.MetaInput.Input | ||
| if tc.ShouldSkipTemplate(host, tags) { | ||
| gologger.Debug().Msgf("[tech-filter] SKIPPED template '%s' (tags: %v) for host '%s' — no matching tech hint", | ||
| e.options.TemplateID, tags, host) | ||
| return nil, nil | ||
| } else { | ||
| gologger.Debug().Msgf("[tech-filter] ALLOWED template '%s' (tags: %v) for host '%s'", | ||
| e.options.TemplateID, tags, host) | ||
| } | ||
| } | ||
| // --- end filtering --- |
There was a problem hiding this comment.
Skipped templates are indistinguishable from zero-match executions.
Returning (nil, nil) when a template is skipped makes it impossible for callers to distinguish between:
- Template was skipped due to tech filtering
- Template executed but found no matches
Based on relevant code snippets, this affects:
executeAllSelfContained(line 37): setsmatch = trueunconditionally when callback is used- Workflow executors: only check for errors, treating skipped templates as successful zero-result runs
This may cause inaccurate scan statistics and could mask issues where templates are being incorrectly skipped.
Consider returning a distinct signal, such as a sentinel error or a dedicated skip status, if accurate tracking is needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/tmplexec/exec.go` around lines 274 - 287, The tech-filter branch
currently returns (nil, nil) which makes skipped templates indistinguishable
from zero-match executions; define a package-level sentinel error (e.g.
ErrTemplateSkipped) in pkg/tmplexec and change the tc.ShouldSkipTemplate branch
to return (nil, ErrTemplateSkipped) instead of (nil, nil), then update callers
(notably executeAllSelfContained and the workflow executors) to check
errors.Is(err, ErrTemplateSkipped) and handle it as a skip (do not set match =
true / treat as successful match) so statistics and control flow correctly
reflect skipped templates; reference symbols: tc.ShouldSkipTemplate,
e.options.TemplateID, executeAllSelfContained, and ErrTemplateSkipped.
| // DisableTechStackFiltering disables the automatic per-host template | ||
| // filtering based on HTTP response headers (e.g. Server: Apache). | ||
| DisableTechStackFiltering bool `yaml:"disable-tech-filter,omitempty"` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Copy() method doesn't include the new field
rg -n 'DisableTechStackFiltering' pkg/types/types.goRepository: projectdiscovery/nuclei
Length of output: 216
🏁 Script executed:
sed -n '477,530p' pkg/types/types.goRepository: projectdiscovery/nuclei
Length of output: 3079
🏁 Script executed:
sed -n '527,550p' pkg/types/types.goRepository: projectdiscovery/nuclei
Length of output: 1480
🏁 Script executed:
sed -n '550,620p' pkg/types/types.goRepository: projectdiscovery/nuclei
Length of output: 4061
🏁 Script executed:
sed -n '620,690p' pkg/types/types.goRepository: projectdiscovery/nuclei
Length of output: 4130
🏁 Script executed:
sed -n '100,150p' pkg/types/types.goRepository: projectdiscovery/nuclei
Length of output: 2520
🏁 Script executed:
# Verify the field exists in the struct definition and is NOT in Copy()
echo "=== Field definition in struct ===" && \
sed -n '135,140p' pkg/types/types.go && \
echo -e "\n=== Checking Copy() method for DisableTechStackFiltering ===" && \
sed -n '477,690p' pkg/types/types.go | grep -i "DisableTechStackFiltering" || echo "NOT FOUND in Copy() method"Repository: projectdiscovery/nuclei
Length of output: 552
DisableTechStackFiltering is missing from the Options.Copy() method.
The new field is not copied when options are duplicated. This causes the setting to be lost during option copying, potentially resulting in inconsistent behavior.
🐛 Add missing field to Copy()
Add this line in the Copy() method around line 527-528 (after NoHostErrors):
NoHostErrors: options.NoHostErrors,
+DisableTechStackFiltering: options.DisableTechStackFiltering,
BulkSize: options.BulkSize,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/types/types.go` around lines 135 - 137, The Options.Copy() method fails
to copy the new DisableTechStackFiltering field, causing it to be lost when
duplicating options; update the Options.Copy() implementation to set the
copied.Options.DisableTechStackFiltering = o.DisableTechStackFiltering (i.e.,
copy the DisableTechStackFiltering boolean from the source Options into the
returned copy), placing it alongside the other scalar copies such as
NoHostErrors to ensure the flag is preserved during duplication.
Mzack9999
left a comment
Mzack9999
left a comment
There was a problem hiding this comment.
Hey @charles1024, thanks for the contribution! Skipping templates based on detected tech stack is a solid idea for scan optimization, but there are a few blockers.
Main concern: the filtering logic is inverted. When Apache is detected, every template without an apache tag gets skipped. Most templates (XSS, SQLi, path traversal, RCE, etc.) are tech-agnostic and don't carry server-specific tags, so they'd all be silently dropped on Apache hosts. The filter should skip templates tagged for an incompatible tech (ex. skip iis-only templates on Apache) and let everything else through.
Must address:
- Flip the filtering logic as described above
- Only Apache is recognized. nginx, IIS, Tomcat, etc. are all ignored, making this asymmetrically harmful to Apache targets
-
ShouldSkipTemplateruns in bothexecutors.goandexec.goon the same code path. Pick one - Remove all debug log lines (
"trying to skip","tryting skip-success","trying ExecuteWithResults", etc.) - Fix spaces-instead-of-tabs indentation in
request.go
Cleanup:
- Restore missing newlines at EOF across six files
- Rename
hosttestcache_test.gotohosttechcache_test.go(typo) - Move
disable-tech-filterflag out of the "Headless" group - Drop
execute_options.gofrom the PR (no functional changes) - Update PR title and description to match what the code does (says "Version ranges" but implements tech-stack filtering)
Addressed feedback from @Mzack9999:All review comments have been addressed in the latest commit: Core Logic Fixes
Code Quality Fixes
Test Updates
Note: Network issues prevented pushing to the fork. Changes are ready locally and can be pushed manually. |
- Fix inverted tech-stack filtering logic (skip incompatible tech templates) - Add support for multiple tech stacks (nginx, IIS, Tomcat) - Remove duplicate ShouldSkipTemplate calls from executors.go - Remove debug logging from exec.go and executors.go - Fix indentation (tabs not spaces) in request.go - Move Server header capture to after redirect chain processing - Add DisableTechStackFiltering to Options.Copy() method - Move disable-tech-filter flag from Headless to Filtering group - Rename hosttestcache_test.go to hosttechcache_test.go - Add missing newlines at EOF - Update tests to match new filtering logic
Update: Changes PushedAll changes have been successfully pushed to: https://github.com/songfrank13141-dotcom/nuclei/tree/version-ranges The commit includes all fixes for the review comments. The changes can be reviewed and merged by a maintainer, or the original PR author (@charles1024) can cherry-pick the commit. Commit: |
dwisiswant0
left a comment
dwisiswant0
left a comment
There was a problem hiding this comment.
What is the goal of this PR? Please open an issue first for discussion.
4 participants
Proposed changes
Proof
Checklist
Summary by CodeRabbit
New Features
Tests