feat: add -enable-global-matchers flag
#5857
Conversation
Signed-off-by: Dwi Siswanto <[email protected]>
WalkthroughThe changes introduced in this pull request add a new command-line flag, Changes
Assessment against linked issues
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (4)
pkg/templates/compile.go (2)
Line range hint
85-96: Document the early return behavior and add error handlingThe early return with
nilvalues when global matchers are enabled could be confusing to callers. Consider:
- Adding documentation to explain this behavior
- Adding error handling for
AddOperatorApply this diff to improve the code:
if template.isGlobalMatchersEnabled(options) { item := &globalmatchers.Item{ TemplateID: template.ID, TemplatePath: filePath, TemplateInfo: template.Info, } for _, request := range template.RequestsHTTP { item.Operators = append(item.Operators, request.CompiledOperators) } - options.GlobalMatchers.AddOperator(item) - return nil, nil + if err := options.GlobalMatchers.AddOperator(item); err != nil { + return nil, fmt.Errorf("failed to add global matcher: %w", err) + } + // Return nil template when global matchers are enabled as they are handled separately + return nil, nil }
122-126: Extend support for global matchers to all protocolsThe current implementation only checks HTTP requests for global matchers, potentially missing templates that use other protocols. This limitation is noted in the TODO comment.
Consider:
- Adding support for other protocols like DNS, Network, etc.
- Documenting the current HTTP-only limitation in the method's documentation.
- Creating a tracking issue for implementing global matchers support in other protocols.
Would you like me to help create a GitHub issue to track this enhancement?
pkg/types/types.go (1)
388-389: Add documentation for the new flag.The new
EnableGlobalMatchersTemplatesfield is well-placed and follows the existing pattern. However, it would benefit from a documentation comment explaining its purpose and default behavior.Add a descriptive comment:
+ // EnableGlobalMatchersTemplates enables processing of global-matchers templates. + // When disabled (default), global matchers templates will not be executed. EnableGlobalMatchersTemplates boolcmd/nuclei/main.go (1)
267-267: Consider adding usage examples in the help text.The flag would benefit from having an example in the help text section to demonstrate its usage, similar to other flags.
Add an example like this to the help text:
Run nuclei with global matchers disabled: + $ nuclei -target example.com -no-global-matchers
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
cmd/nuclei/main.go(1 hunks)pkg/templates/compile.go(2 hunks)pkg/types/types.go(1 hunks)
🔇 Additional comments (3)
pkg/templates/compile.go (1)
122-126: Verify impact on existing templates
Let's verify how this change affects existing templates and identify potential breaking changes.
✅ Verification successful
Let me gather more information about this change to understand its full impact.
Based on the gathered information, I can now provide a final response:
No impact on existing templates - feature is opt-in
The EnableGlobalMatchersTemplates flag is a new opt-in feature that is disabled by default. The code change is safe because:
- No existing templates use global matchers (verified by searching the codebase)
- The feature must be explicitly enabled via the
EnableGlobalMatchersTemplatesflag - The implementation follows the same pattern as other similar opt-in features like
EnableSelfContainedTemplatesandEnableFileTemplates
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for templates using global matchers and verify their protocol types
# Find templates with global matchers
echo "Templates with global matchers:"
rg -l "globalMatchers:\s*true" || echo "No templates found with global matchers"
# Check for non-HTTP protocols using global matchers
echo -e "\nNon-HTTP protocols using global matchers:"
rg -B5 -A5 "globalMatchers:\s*true" | rg "(dns|network|headless|ssl|websocket|whois|code|javascript):" -B10 || echo "No non-HTTP protocols found with global matchers"
Length of output: 510
Script:
#!/bin/bash
# Check for the usage of EnableGlobalMatchersTemplates flag
echo "Checking EnableGlobalMatchersTemplates usage:"
rg "EnableGlobalMatchersTemplates" -A 2 -B 2
# Check for any other global matcher related code
echo -e "\nChecking other global matcher related code:"
rg "globalMatchers" -A 2 -B 2
# Check the Options struct definition
echo -e "\nChecking Options struct definition:"
ast-grep --pattern 'type Options struct {
$$$
EnableGlobalMatchersTemplates $_
$$$
}'
Length of output: 28706
pkg/types/types.go (1)
386-386: LGTM! Clear comment update.
The comment update for EnableSelfContainedTemplates improves clarity by correctly stating that it "enables" rather than "disables" processing.
cmd/nuclei/main.go (1)
267-267: LGTM! The flag implementation looks good.
The new -enable-global-matchers flag is properly integrated into the templates flag group with appropriate naming and description. The implementation follows the existing patterns in the codebase.
Let's verify the flag integration:
✅ Verification successful
The flag implementation is properly integrated and functional
The verification shows that:
- The flag is properly defined in the
types.gowith appropriate documentation - It's correctly implemented in
pkg/templates/compile.gowhere it controls the execution of global matchers through theisGlobalMatchersEnabledfunction - The implementation follows a clean pattern: if the flag is disabled, it returns false early, otherwise proceeds with checking global matchers in HTTP requests
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the flag integration and related code
# Check if the flag is properly integrated with the Options struct
rg -A 2 "EnableGlobalMatchersTemplates"
# Check for any related template processing code that uses this flag
ast-grep --pattern 'func $_($$) {
$$$
EnableGlobalMatchersTemplates
$$$
}'
Length of output: 543
Script:
#!/bin/bash
# Let's check the implementation in compile.go to understand how the flag is used
rg -B 5 -A 5 "EnableGlobalMatchersTemplates" pkg/templates/compile.go
# Also check if there are any global matchers related files/templates
fd "global-matchers"
Length of output: 459
Signed-off-by: Dwi Siswanto <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
pkg/templates/compile.go (2)
115-116: Track TODO: Support for all protocols in global matchersThe documentation indicates that global matchers currently only support HTTP protocol. Let's ensure this limitation is tracked for future implementation.
Would you like me to create a GitHub issue to track the implementation of global matchers support for other protocols?
124-127: Add unit tests for the new flagPlease add unit tests to verify:
- Global matchers are disabled by default
- Global matchers are enabled only when the flag is set
- Behavior with nil Options
Would you like me to help generate the unit test cases?
Signed-off-by: Dwi Siswanto <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
lib/config.go (1)
396-402: Enhance function documentation while implementation LGTM!The implementation follows the established pattern for SDK options and correctly sets the flag. However, the documentation could be more comprehensive to help users make informed decisions.
Consider expanding the documentation to include:
-// EnableGlobalMatchersTemplates allows loading/executing global-matchers templates +// EnableGlobalMatchersTemplates allows loading/executing global-matchers templates. +// Global matchers templates are special templates that run against all targets and can be used +// for tasks like passive scanning or gathering additional information. Enable this option only +// when you specifically need these templates, as they may increase scan time and resource usage. +// +// Example usage: +// engine := nuclei.New(nuclei.EnableGlobalMatchersTemplates())
ehsandeep
deleted the
dwisiswant0/feat/add-enable-global-matchers-flag
branch
Proposed changes
Close #5856
Checklist
Summary by CodeRabbit
New Features
EnableGlobalMatchersTemplates, allowing users to enable loading of global matchers templates for enhanced configurability.Bug Fixes
EnableSelfContainedTemplatesoption.Documentation