Expose cluster ids mapping to template ids #6788
Conversation
WalkthroughAdds generation and safe storage of cluster→template-ID mappings during template clustering, exposes those mappings on ExecutorOptions, and provides SDK methods to read a cluster's template IDs or retrieve all mappings. Changes
Sequence DiagramsequenceDiagram
participant SDK as SDK User
participant Engine as NucleiEngine
participant Core as Executor/Core
participant Templates as Template Clustering
participant Storage as ClusterMappingsMap
SDK->>Engine: GetClusterTemplateIDs(clusterID)
Engine->>Storage: Get(clusterID)
Storage-->>Engine: []string
Engine-->>SDK: []string
Core->>Templates: ClusterTemplates(templatesList, options)
Templates->>Templates: build clusters and mappings
Templates-->>Core: templates, count, mappings
Core->>Storage: NewClusterMappingsMap(mappings)
Core->>Core: executerOpts.ClusterMappings = ...
SDK->>Engine: GetAllClusterMappings()
Engine->>Storage: GetAll()
Storage-->>Engine: map[clusterID][]string
Engine-->>SDK: map[clusterID][]string
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/protocols/protocols.go (1)
274-316:Copy()method does not copy theClusterMappingsfield.The
ClusterMappingsfield (which stores cluster ID to template IDs mapping during execution) is not included in theCopy()method. All other fields are copied, but this field is missing. This could cause issues if copiedExecutorOptionsare used in contexts expecting cluster mappings information, as the field will be nil. Add the field to the copy assignment (either share the reference withcopy.ClusterMappings = e.ClusterMappingsor copy the map data appropriately).
🤖 Fix all issues with AI agents
In `@pkg/protocols/protocols.go`:
- Around line 150-151: Change the ClusterMappings field type to
*templateTypes.ClusterMappingsMap and ensure the templates types package is
imported with the templateTypes alias; then update the Copy() method on the same
struct to deep-copy or clone ClusterMappings into the returned object (preserve
nil handling) and update ApplyNewEngineOptions() to copy/assign the
ClusterMappings from the source options to the target when applying new engine
options so cluster mappings are preserved; reference the ClusterMappings field,
Copy() method, and ApplyNewEngineOptions() method when making these edits.
🧹 Nitpick comments (5)
pkg/templates/types/cluster_mappings.go (3)
4-6: Consider making theMapfield unexported for encapsulation.The
Mapfield is exported, which allows direct mutation and bypasses the accessor methods (Get,GetAll). This could lead to unintended modifications. If external read access is intentional, consider providing a read-only interface instead.Suggested encapsulation improvement
// ClusterMappingsMap wraps cluster ID to template IDs mapping type ClusterMappingsMap struct { - Map map[string][]string + m map[string][]string }Then update the constructor and methods to use
c.minstead ofc.Map.
14-16:Getreturns the internal slice directly, allowing external mutation.Unlike
GetAllwhich deep-copies slices,Getreturns the internal slice reference. Callers can mutate the returned slice, affecting the internal state.Return a copy for consistency with GetAll
// Get returns the template IDs for a given cluster ID func (c *ClusterMappingsMap) Get(clusterID string) ([]string, bool) { v, ok := c.Map[clusterID] + if !ok { + return nil, false + } - return v, ok + return append([]string{}, v...), true }
14-17: Add nil receiver/map guards to prevent panics.Both
GetandGetAllwill panic if called on a nil receiver or if the internal map is nil. Consider adding defensive checks.Add nil guards
// Get returns the template IDs for a given cluster ID func (c *ClusterMappingsMap) Get(clusterID string) ([]string, bool) { + if c == nil || c.Map == nil { + return nil, false + } v, ok := c.Map[clusterID] return v, ok } // GetAll returns a copy of the entire map func (c *ClusterMappingsMap) GetAll() map[string][]string { + if c == nil || c.Map == nil { + return nil + } result := make(map[string][]string, len(c.Map)) for k, v := range c.Map { result[k] = append([]string{}, v...) } return result }Also applies to: 20-25
lib/sdk.go (1)
364-373: Consider returning a defensive copy to prevent external mutation.If
ClusterMappings.Get()returns the internal slice directly (as implemented in cluster_mappings.go), callers of this SDK method could mutate the internal state. For a public SDK API, returning a copy would be safer.Return a defensive copy
func (e *NucleiEngine) GetClusterTemplateIDs(clusterID string) []string { if e.executerOpts == nil || e.executerOpts.ClusterMappings == nil { return nil } templateIDs, ok := e.executerOpts.ClusterMappings.Get(clusterID) if !ok { return nil } - return templateIDs + return append([]string{}, templateIDs...) }Alternatively, fix this at the
ClusterMappingsMap.Get()level to ensure consistency.pkg/core/execute_options.go (1)
44-49: Comment mentions "thread-safe" butClusterMappingsMaplacks synchronization.The comment on line 46 states "thread-safe" but
ClusterMappingsMapdoesn't include any synchronization primitives (mutex, RWMutex, sync.Map). If the mappings are only written once during initialization and then read-only, this is fine for concurrent reads, but the comment could be clarified.Clarify the comment
- // Store cluster mappings in executerOpts for SDK access (thread-safe) + // Store cluster mappings in executerOpts for SDK access (read-only after initialization)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pkg/templates/types/cluster_mappings.go`:
- Around line 14-17: The Get method on ClusterMappingsMap can panic when called
on a nil receiver or when c.Map is nil; update ClusterMappingsMap.Get to first
check if c == nil || c.Map == nil and return nil, false in that case so callers
get a safe, consistent nil-safe behavior (matching how Copy() handles nil
receivers); modify the function body of ClusterMappingsMap.Get to perform these
nil checks before accessing c.Map[clusterID].
- Around line 20-26: GetAll currently panics when called on a nil receiver or
when c.Map is nil; add a nil receiver check at the start of
ClusterMappingsMap.GetAll (e.g., if c == nil || c.Map == nil) and return an
empty map[string][]string to match the behavior of Copy(), then proceed to
deep-copy entries as before; reference the GetAll method on the
ClusterMappingsMap type and mirror the nil-safe behavior used in Copy().
🧹 Nitpick comments (1)
pkg/templates/types/cluster_mappings.go (1)
8-11: Constructor performs shallow copy of the input map.
NewClusterMappingsMapassigns the map directly without deep copying. If the caller mutates the original map after construction, it will affect the internal state ofClusterMappingsMap. Consider whether this is intentional or if a deep copy is needed for encapsulation.♻️ Proposed fix for defensive deep copy
func NewClusterMappingsMap(m map[string][]string) *ClusterMappingsMap { - return &ClusterMappingsMap{Map: m} + if m == nil { + return &ClusterMappingsMap{Map: nil} + } + copied := make(map[string][]string, len(m)) + for k, v := range m { + copied[k] = append([]string{}, v...) + } + return &ClusterMappingsMap{Map: copied} }
|
@yaron12n Curious, is there a use case for this? |
|
@dogancanbakir |
|
@Mzack9999 @dogancanbakir Can you CR this? 🙏 |
2 participants
Proposed changes
Adding API for getting mapping of cluster id template ids.
logs are published with templateID (cluster ID)
i'd like to know which templates refer to the exectued request
Proof
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.