feat: allow overriding default exclude-ports in CLI#7324
feat: allow overriding default exclude-ports in CLI#7324chovanecadam wants to merge 1 commit intoprojectdiscovery:devfrom
Conversation
WalkthroughAdded a new CLI flag Changes
Sequence DiagramsequenceDiagram
actor User as User/CLI
participant CLI as CLI Flag Parser
participant Opts as Options
participant Tmpl as Template Executor
participant NetPort as UseNetworkPort
User->>CLI: -reserved-ports "22,25,..."
CLI->>Opts: Set ReservedPorts (string slice)
Tmpl->>Tmpl: Load template with excludePorts
Tmpl->>NetPort: executeWithResults(port,<br/>templateExcludePorts,<br/>cliExcludePorts)
NetPort->>NetPort: Precedence Check
alt CLI ports available
NetPort->>NetPort: Use cliExcludePorts
else Template ports specified
NetPort->>NetPort: Use templateExcludePorts
else
NetPort->>NetPort: Use defaultReservedPorts
end
NetPort-->>Tmpl: Updated context with final port
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Neo - PR Security ReviewNo security issues found 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: 1
🧹 Nitpick comments (1)
cmd/nuclei/main.go (1)
376-376: Clarify that-reserved-portsoverrides defaults.The flag wiring is good; consider tightening the help text so users understand this list replaces default exclusions rather than extending them.
✏️ Suggested wording
-flagSet.StringSliceVarP(&options.ReservedPorts, "reserved-ports", "rport", nil, "list of reserved ports that network templates should avoid", goflags.CommaSeparatedStringSliceOptions), +flagSet.StringSliceVarP(&options.ReservedPorts, "reserved-ports", "rport", nil, "comma-separated ports overriding default excluded ports for network templates (example: -rport 0)", goflags.CommaSeparatedStringSliceOptions),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/nuclei/main.go` at line 376, Update the flag help string for flagSet.StringSliceVarP so it clearly states that the "reserved-ports" (alias "rport") value replaces the built-in default exclusion list rather than appending to it; specifically modify the usage text passed to flagSet.StringSliceVarP (associated with options.ReservedPorts and the "reserved-ports"/"rport" flag) to say something like "list of reserved ports that network templates should avoid — this replaces the default exclusion list" (or equivalent concise wording).
🤖 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/protocols/common/contextargs/contextargs.go`:
- Around line 116-121: Normalize excluded-port tokens before using them: when
building ignorePorts from templateExcludePorts or cliExcludePorts ensure each
token is trimmed (use strings.TrimSpace on each Split token) and then
deduplicated (sliceutil.Dedupe). Update the block that sets ignorePorts
(variables templateExcludePorts, cliExcludePorts, and ignorePorts in
contextargs.go) to map TrimSpace over tokens coming from
strings.Split(templateExcludePorts, ",") and to trim any entries in
cliExcludePorts as well so comparisons later will match whitespace-padded
inputs.
---
Nitpick comments:
In `@cmd/nuclei/main.go`:
- Line 376: Update the flag help string for flagSet.StringSliceVarP so it
clearly states that the "reserved-ports" (alias "rport") value replaces the
built-in default exclusion list rather than appending to it; specifically modify
the usage text passed to flagSet.StringSliceVarP (associated with
options.ReservedPorts and the "reserved-ports"/"rport" flag) to say something
like "list of reserved ports that network templates should avoid — this replaces
the default exclusion list" (or equivalent concise wording).
🪄 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: ca4ff839-f1ca-40fa-91b8-159725b35b5e
📒 Files selected for processing (6)
cmd/nuclei/main.gopkg/protocols/common/contextargs/contextargs.gopkg/protocols/common/contextargs/contextargs_test.gopkg/protocols/javascript/js.gopkg/protocols/network/request.gopkg/types/types.go
| if len(cliExcludePorts) > 0 { | ||
| ignorePorts = cliExcludePorts | ||
| } else if templateExcludePorts != "" { | ||
| // TODO: add support for service names like http,https,ssh etc once https://github.com/projectdiscovery/netdb is ready | ||
| ignorePorts = sliceutil.Dedupe(strings.Split(excludePorts, ",")) | ||
| ignorePorts = sliceutil.Dedupe(strings.Split(templateExcludePorts, ",")) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect UseNetworkPort parsing:"
rg -n -C3 'UseNetworkPort|templateExcludePorts|cliExcludePorts|strings.Split|TrimSpace' pkg/protocols/common/contextargs/contextargs.go
echo
echo "Inspect TestUseNetworkPort cases:"
rg -n 'templateExcl|cliReserved|name:' pkg/protocols/common/contextargs/contextargs_test.goRepository: projectdiscovery/nuclei
Length of output: 1835
🏁 Script executed:
rg -n "normalizePorts|TrimSpace" pkg/protocols/common/contextargs/ --type goRepository: projectdiscovery/nuclei
Length of output: 49
🏁 Script executed:
# Check if sliceutil or other utility packages have port normalization
rg -n "TrimSpace|Dedupe" pkg/utils/ --type go -A 2 | head -30Repository: projectdiscovery/nuclei
Length of output: 174
🏁 Script executed:
# Check if there are other port-related parsing that might have similar issues
rg -n "strings\.Split.*port|excludePort" pkg/protocols/ --type go | head -20Repository: projectdiscovery/nuclei
Length of output: 152
🏁 Script executed:
# Check the javascript/js.go port parsing more thoroughly
rg -n -B5 -A10 'strings\.Split.*portStr' pkg/protocols/javascript/js.goRepository: projectdiscovery/nuclei
Length of output: 520
🏁 Script executed:
# Find where ignorePorts is actually used for comparison
rg -n -A15 'ignorePorts :=' pkg/protocols/common/contextargs/contextargs.go | head -40Repository: projectdiscovery/nuclei
Length of output: 636
🏁 Script executed:
# Check sliceutil.Dedupe to understand what it does
fd -t f "sliceutil" --exec cat {} \; 2>/dev/null | head -100Repository: projectdiscovery/nuclei
Length of output: 49
🏁 Script executed:
# Find sliceutil package/file
fd -t f -i sliceutilRepository: projectdiscovery/nuclei
Length of output: 49
🏁 Script executed:
# Check how ignorePorts is used after assignment (the actual comparison)
rg -n -A30 'if port ==' pkg/protocols/common/contextargs/contextargs.go | head -50Repository: projectdiscovery/nuclei
Length of output: 908
🏁 Script executed:
# Find stringsutil.EqualFoldAny implementation to confirm comparison behavior
fd -t f stringsutil || rg -n "func EqualFoldAny" --type goRepository: projectdiscovery/nuclei
Length of output: 49
🏁 Script executed:
# Get the full context of how ports flow through the system
rg -n "defaultReservedPorts" pkg/protocols/common/contextargs/contextargs.go -B 2 -A 2Repository: projectdiscovery/nuclei
Length of output: 556
🏁 Script executed:
# Search more broadly for EqualFoldAny and stringsutil
rg -n "EqualFoldAny" --type go | head -20Repository: projectdiscovery/nuclei
Length of output: 2619
🏁 Script executed:
# Check all imports in the contextargs file to understand what stringsutil is used
head -20 pkg/protocols/common/contextargs/contextargs.goRepository: projectdiscovery/nuclei
Length of output: 598
🏁 Script executed:
# Find the stringsutil package to check EqualFoldAny implementation
rg -n "package strings|func EqualFoldAny" --type go | grep -A5 "EqualFoldAny"Repository: projectdiscovery/nuclei
Length of output: 49
🏁 Script executed:
# Check if there are integration tests or other test files that might test with whitespace
find pkg/protocols/common/contextargs -name "*test*" -type fRepository: projectdiscovery/nuclei
Length of output: 119
🏁 Script executed:
# Examine the full test file to see all test cases
cat pkg/protocols/common/contextargs/contextargs_test.goRepository: projectdiscovery/nuclei
Length of output: 2137
Normalize excluded port tokens before matching.
templateExcludePorts and CLI-provided cliExcludePorts are not trimmed. Inputs like "80, 443" will create tokens with leading/trailing whitespace that fail to match during port comparison, causing incorrect port-rewrite behavior.
This differs from the port parsing in pkg/protocols/javascript/js.go, which correctly applies strings.TrimSpace() to each token. Test coverage also lacks whitespace-padded port cases.
💡 Proposed fix
func (ctx *Context) UseNetworkPort(port string, templateExcludePorts string, cliExcludePorts []string) error {
ignorePorts := defaultReservedPorts
if len(cliExcludePorts) > 0 {
- ignorePorts = cliExcludePorts
+ ignorePorts = normalizePorts(cliExcludePorts)
} else if templateExcludePorts != "" {
// TODO: add support for service names like http,https,ssh etc once https://github.com/projectdiscovery/netdb is ready
- ignorePorts = sliceutil.Dedupe(strings.Split(templateExcludePorts, ","))
+ ignorePorts = normalizePorts(strings.Split(templateExcludePorts, ","))
}
@@
return nil
}
+
+func normalizePorts(ports []string) []string {
+ normalized := make([]string, 0, len(ports))
+ for _, p := range ports {
+ p = strings.TrimSpace(p)
+ if p != "" {
+ normalized = append(normalized, p)
+ }
+ }
+ return sliceutil.Dedupe(normalized)
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if len(cliExcludePorts) > 0 { | |
| ignorePorts = cliExcludePorts | |
| } else if templateExcludePorts != "" { | |
| // TODO: add support for service names like http,https,ssh etc once https://github.com/projectdiscovery/netdb is ready | |
| ignorePorts = sliceutil.Dedupe(strings.Split(excludePorts, ",")) | |
| ignorePorts = sliceutil.Dedupe(strings.Split(templateExcludePorts, ",")) | |
| } | |
| if len(cliExcludePorts) > 0 { | |
| ignorePorts = normalizePorts(cliExcludePorts) | |
| } else if templateExcludePorts != "" { | |
| // TODO: add support for service names like http,https,ssh etc once https://github.com/projectdiscovery/netdb is ready | |
| ignorePorts = normalizePorts(strings.Split(templateExcludePorts, ",")) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/protocols/common/contextargs/contextargs.go` around lines 116 - 121,
Normalize excluded-port tokens before using them: when building ignorePorts from
templateExcludePorts or cliExcludePorts ensure each token is trimmed (use
strings.TrimSpace on each Split token) and then deduplicated (sliceutil.Dedupe).
Update the block that sets ignorePorts (variables templateExcludePorts,
cliExcludePorts, and ignorePorts in contextargs.go) to map TrimSpace over tokens
coming from strings.Split(templateExcludePorts, ",") and to trim any entries in
cliExcludePorts as well so comparisons later will match whitespace-padded
inputs.
1 participant
Proposed changes
Fixes: #7323
This MR adds a CLI option to overwrite the default
excluded-portsarray for the whole scan.Proof
Default behavior unchanged.
./bin/nuclei -verbose -t network/cves/2001/CVE-2001-1473.yaml -target TARGET:80 # [WRN] [CVE-2001-1473] Could not make network request for (147.251.125.30:22) : could not connect to server: cause="context cancelled before establishing connection" address=147.251.125.30:22 chain="context deadline exceeded"We can specify array of excluded ports in CLI. Networking templates now can scan SSH running on port 80.
Checklist
Summary by CodeRabbit
New Features
-reserved-ports(short-rport) CLI flag enabling users to specify a comma-separated list of ports that network templates should avoid during executionTests