-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add jsmon #1637
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add jsmon #1637
Conversation
WalkthroughAdds a new passive subscraping source "jsmon": registers it in AllSources, implements its Run flow to call the jsmon API with token/workspace key format, emits discovered subdomains, exposes statistics, and updates tests to expect "jsmon" in all/default sources. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Subfinder as Passive Engine
participant JSMon as jsmon.Source
participant API as jsmon API
User->>Subfinder: subdomain enumeration (domain)
Subfinder->>JSMon: Run(ctx, domain, session)
rect rgba(200,235,255,0.3)
note right of JSMon: Select API key (token:wkspId)<br/>Set headers and body
JSMon->>API: POST /api/v2/subfinderScan2?wkspId=...<br/>X-Jsmon-Key: token<br/>{ "domain": "<domain>" }
API-->>JSMon: 200 OK + { subdomains: [...] } or error
end
alt 200 OK
loop for each subdomain
JSMon-->>Subfinder: Result{Type: Subdomain, Value}
end
else non-200 / network error
JSMon-->>Subfinder: Result{Type: Error, Info}
end
JSMon-->>Subfinder: Statistics{Results, Errors, TimeTaken, Skipped}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNone found. Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (8)
pkg/passive/sources.go (1)
106-107: Registered &jsmon.Source{} — consider adding a user-facing warning for API key formatSince jsmon expects JSMON_API_KEY in "token:workspaceID" form, surface a friendly warning like other sources to reduce misconfiguration churn.
You can set a warning once during package init:
var sourceWarnings = mapsutil.NewSyncLockMap[string, string]( mapsutil.WithMap(mapsutil.Map[string, string]{})) +func init() { + // jsmon requires "token:workspaceID" format + sourceWarnings.Set("jsmon", "Set JSMON_API_KEY as <TOKEN>:<WORKSPACE_ID> (e.g., abc123:ws_456).") +}Note: multiple init() functions are allowed in the same package.
pkg/subscraping/sources/jsmon/jsmon.go (7)
35-39: Reset skipped flag per run to avoid stale statisticss.skipped is only ever set to true; if a previous run skipped, subsequent successful runs will still report Skipped=true.
func (s *Source) Run(ctx context.Context, domain string, session *subscraping.Session) <-chan subscraping.Result { results := make(chan subscraping.Result) s.errors = 0 s.results = 0 + s.skipped = false
52-60: API key parsing: use SplitN and trim; adopt Go ID casingTokens can unexpectedly contain a colon, and whitespace can sneak in. Also prefer wkspID for Go style.
- randomApiInfo := strings.Split(randomApiKey, ":") - if len(randomApiInfo) != 2 { + parts := strings.SplitN(strings.TrimSpace(randomApiKey), ":", 2) + if len(parts) != 2 { s.skipped = true return } - - authToken := randomApiInfo[0] - wkspId := randomApiInfo[1] + authToken := strings.TrimSpace(parts[0]) + wkspID := strings.TrimSpace(parts[1])
61-67: Build JSON payload via marshaling, and prefer baseURL/wkspID namingString interpolation risks malformed JSON if domain ever contains characters needing escaping; marshaling is safer. Also normalize URL/ID names.
- subfinderScanURL := fmt.Sprintf("%s/api/v2/subfinderScan2?wkspId=%s", baseUrl, wkspId) - requestBody := fmt.Sprintf(`{"domain":"%s"}`, domain) + subfinderScanURL := fmt.Sprintf("%s/api/v2/subfinderScan2?wkspId=%s", baseURL, wkspID) + payload, _ := jsoniter.Marshal(struct { + Domain string `json:"domain"` + }{Domain: domain}) headers := map[string]string{ "X-Jsmon-Key": authToken, "Content-Type": "application/json", } - resp, err := session.Post(ctx, subfinderScanURL, "", headers, bytes.NewReader([]byte(requestBody))) + resp, err := session.Post(ctx, subfinderScanURL, "", headers, bytes.NewReader(payload))And rename the constant:
-const ( - baseUrl = "https://api.jsmon.sh" -) +const ( + baseURL = "https://api.jsmon.sh" +)
84-97: Optional: handle API-declared error states before emitting subdomainsResponse has Status/Message; consider surfacing server-declared errors explicitly instead of silently proceeding.
var response subdomainsResponse if err := jsoniter.NewDecoder(resp.Body).Decode(&response); err != nil { results <- subscraping.Result{Source: s.Name(), Type: subscraping.Error, Error: err} s.errors++ session.DiscardHTTPResponse(resp) return } session.DiscardHTTPResponse(resp) + if response.Status != "" && !strings.EqualFold(response.Status, "ok") && !strings.EqualFold(response.Status, "success") { + results <- subscraping.Result{ + Source: s.Name(), Type: subscraping.Error, + Error: fmt.Errorf("subfinderScan API status=%s message=%s", response.Status, response.Message), + } + s.errors++ + return + }
1-3: Package comment style nitConsider an idiomatic package comment.
-// Package jsmon logic +// Package jsmon implements the Subfinder source backed by the jsmon API.
17-19: Identifier casing nitUse baseURL per Go style for initialisms.
(See diff proposed in a prior comment.)
108-110: Confirm default inclusion is intendedIsDefault() returns true, so jsmon participates without explicit opt-in. If jsmon commonly requires credentials, default-enabled may surprise users without JSMON_API_KEY configured (it will be marked skipped). If that’s intentional for visibility, ignore this; otherwise consider making it non-default.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
pkg/passive/sources.go(2 hunks)pkg/passive/sources_test.go(2 hunks)pkg/subscraping/sources/jsmon/jsmon.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
pkg/passive/sources.go (1)
pkg/subscraping/sources/jsmon/jsmon.go (1)
Source(27-33)
pkg/subscraping/sources/jsmon/jsmon.go (2)
pkg/subscraping/types.go (2)
Session(71-78)Statistics(29-34)pkg/subscraping/utils.go (1)
PickRandom(12-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Builds (windows-latest)
- GitHub Check: Test Builds (ubuntu-latest)
- GitHub Check: Test Builds (macOS-13)
- GitHub Check: release-test
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
pkg/passive/sources_test.go (2)
60-61: Added "jsmon" to expectedAllSources — LGTMConsistent with the new source registration. No issues.
98-99: Added "jsmon" to expectedDefaultSources — LGTM (matches IsDefault=true)This aligns with jsmon.Source.IsDefault() returning true. expectedDefaultRecursiveSources correctly remains unchanged since HasRecursiveSupport() is false.
pkg/passive/sources.go (1)
38-39: Importing jsmon source — LGTMImport is accurate and scoped. No side effects.
pkg/subscraping/sources/jsmon/jsmon.go (2)
124-131: Statistics accessor — LGTMFields are populated as expected; once the per-run reset of skipped is added, stats will be consistent.
68-75: Possible nil dereference on error path (resp may be nil)If session.Post fails before creating a response, DiscardHTTPResponse(resp) can panic.
Apply this guard:
- if err != nil { - results <- subscraping.Result{Source: s.Name(), Type: subscraping.Error, Error: err} - s.errors++ - session.DiscardHTTPResponse(resp) - return - } + if err != nil { + results <- subscraping.Result{Source: s.Name(), Type: subscraping.Error, Error: err} + s.errors++ + if resp != nil { + session.DiscardHTTPResponse(resp) + } + return + }⛔ Skipped due to learnings
Learnt from: x-stp PR: projectdiscovery/subfinder#0 File: :0-0 Timestamp: 2025-06-24T16:03:30.467Z Learning: When fixing HTTP response handling bugs in subfinder sources, the correct pattern is to use `defer session.DiscardHTTPResponse(resp)` after successful requests to ensure the response body remains open for reading, and call `session.DiscardHTTPResponse(resp)` immediately in error cases.Learnt from: x-stp PR: projectdiscovery/subfinder#1608 File: v2/pkg/subscraping/sources/shrewdeye/shrewdeye.go:32-38 Timestamp: 2025-06-20T19:02:59.053Z Learning: The DiscardHTTPResponse method in subfinder's Session already includes a built-in nil check for the response parameter, so it's safe to call with a potentially nil http.Response without additional nil checking.Learnt from: x-stp PR: projectdiscovery/subfinder#0 File: :0-0 Timestamp: 2025-06-20T19:05:25.823Z Learning: The subfinder session provides a `DiscardHTTPResponse` method that properly handles HTTP response cleanup by draining the body and closing it with error handling. Sources should use this method instead of manually closing response bodies to avoid redundant syscalls.
|
After thoroughly testing it, we realized that it can be non-deterministic and occasionally doesn't produce great results in some domains. So, we've decided not to include it for now, but we'll definitely keep it in mind for future considerations. |
closes #1636
Summary by CodeRabbit