fix secrets lookup logic #6059
fix secrets lookup logic #6059
Conversation
WalkthroughThe update modifies the logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant F as FileAuthProvider.LookupAddr
C->>F: Call LookupAddr(addr)
F->>F: Initialize strategies slice
F->>F: Iterate over domain & regex patterns
alt Match found
F->>F: Append strategy to strategies slice
end
F->>C: Return strategies slice
Poem
✨ Finishing Touches
🪧 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/authprovider/file.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Tests (macOS-latest)
- GitHub Check: Tests (windows-latest)
- GitHub Check: Tests (ubuntu-latest)
🔇 Additional comments (3)
pkg/authprovider/file.go (3)
125-126: LGTM! Good initialization for accumulating strategies.The initialization of the
strategiesslice is well-placed and supports the new behavior of collecting all matching strategies.
134-138: LGTM! Correctly accumulates all matching domain strategies.The modification to append matching domain strategies instead of returning immediately is a good improvement. The case-insensitive comparison is preserved, ensuring reliable domain matching.
139-143: LGTM! Correctly accumulates all matching regex strategies.The modification to append matching regex strategies instead of returning immediately aligns well with the domain matching behavior, ensuring all applicable strategies are collected.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/authprovider/multi.go (1)
32-50: Consider using consistent length checks across all lookup methods.For consistency with the updated
LookupAddrmethod, consider updating the nil checks inLookupURLandLookupURLXto use length checks as well.Apply these diffs to maintain consistency:
func (m *MultiAuthProvider) LookupURL(u *url.URL) []authx.AuthStrategy { for _, provider := range m.Providers { strategy := provider.LookupURL(u) - if strategy != nil { + if len(strategy) > 0 { return strategy } } return nil } func (m *MultiAuthProvider) LookupURLX(u *urlutil.URL) []authx.AuthStrategy { for _, provider := range m.Providers { strategy := provider.LookupURLX(u) - if strategy != nil { + if len(strategy) > 0 { return strategy } } return nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/authprovider/multi.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
pkg/authprovider/multi.go (1)
Learnt from: dogancanbakir
PR: projectdiscovery/nuclei#6059
File: pkg/authprovider/file.go:144-145
Timestamp: 2025-02-19T13:21:35.933Z
Learning: In the Nuclei project, when checking auth strategies returned from LookupAddr, use len() check instead of nil check as the function returns an empty slice for no matches.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Tests (macOS-latest)
- GitHub Check: Tests (windows-latest)
- GitHub Check: Tests (ubuntu-latest)
🔇 Additional comments (1)
pkg/authprovider/multi.go (1)
22-30: LGTM! The len() check is the correct approach.The change from nil check to len() check aligns with the project's preferred way of handling empty results from LookupAddr.
Proposed changes
Closes #5498
Checklist
Summary by CodeRabbit