Conversation
WalkthroughAdded a new Profundis passive subdomain source with a streaming HTTP integration, registered it in the passive sources list, and updated tests to include "profundis" in expected source sets. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Runner
participant Source as profundis.Source
participant Session as Session (results/ch)
participant Profundis as Profundis API
Note over Source,Profundis: New streaming subdomain flow
Caller->>Source: Run(ctx, domain, session)
Source->>Source: select random API key
alt no API key
Source->>Session: send Result{Type: Error/Skip}
Source->>Caller: close channel
else has API key
Source->>Profundis: HTTP POST /subdomain-enumeration {domain} (X-API-KEY, Accept: text/event-stream)
Profundis-->>Source: 200 (streaming body)
loop for each line
Profundis-->>Source: event-stream line (subdomain or ping)
Source->>Session: send Result{Type: Subdomain, Value: line}
Note right of Session: respects ctx cancellation
end
alt scanner error
Source->>Session: send Result{Type: Error}
end
Source->>Caller: close channel
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (4)
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/passive/sources_test.go (1)
46-103: Align recursive-source expectations withHasRecursiveSupport()for profundisYou’ve correctly added
"profundis"to bothexpectedAllSourcesandexpectedDefaultSources, butexpectedDefaultRecursiveSources(lines 105–119) does not include"profundis"whileprofundis.Source.HasRecursiveSupport()currently returnstrue. InTestSourceCategorization,recursiveSourcesis built from all sources whereHasRecursiveSupport()is true, so this will cause theassert.ElementsMatchonexpectedDefaultRecursiveSourcesto fail.Decide whether profundis should participate in recursive runs:
- If yes (likely, given it’s a large passive dataset), update the test list to include it, e.g.:
expectedDefaultRecursiveSources = []string{ "alienvault", "bufferover", "certspotter", "crtsh", "dnsdb", "digitorus", "driftnet", "hackertarget", "securitytrails", "virustotal", "leakix", "facebook", + "profundis", }
- If no, then
profundis.Source.HasRecursiveSupport()should returnfalseinstead.Right now tests won’t pass until these are made consistent.
🧹 Nitpick comments (2)
pkg/passive/sources.go (1)
29-41: Profundis wiring intoAllSourcesand env‑var handling looks correctThe new
profundisimport and&profundis.Source{}entry inAllSourcesare consistent with the existing pattern. WithName()returning"profundis",Agent.Newwill look forPROFUNDIS_API_KEY, which matches the usual${SOURCE}_API_KEYconvention.From a passive-source wiring perspective this is good to go; just ensure the new env var name is documented for users of the new source.
Also applies to: 61-111
pkg/subscraping/sources/profundis/profundis.go (1)
55-95: Harden streaming logic: HTTP status handling and event-stream formatThe core streaming logic is sound, but a few tweaks would make it more robust:
- HTTP status handling: You don’t check
resp.StatusCode. If the API returns a non‑2xx with a body (auth error, quota exceeded, etc.), the scanner will happily emit those lines asSubdomainresults. It’s safer to treat non‑success statuses as errors and bail out:- resp, err := session.Post(ctx, "https://api.profundis.io/api/v2/common/data/subdomains", "", - headers, bytes.NewReader(requestBody)) + resp, err := session.Post(ctx, "https://api.profundis.io/api/v2/common/data/subdomains", "", + headers, bytes.NewReader(requestBody)) if err != nil { results <- subscraping.Result{Source: s.Name(), Type: subscraping.Error, Error: err} s.errors++ session.DiscardHTTPResponse(resp) return } + + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + session.DiscardHTTPResponse(resp) + results <- subscraping.Result{ + Source: s.Name(), + Type: subscraping.Error, + Error: fmt.Errorf("profundis: unexpected status code %d", resp.StatusCode), + } + s.errors++ + return + }(You’d need a
fmtimport here or reuse whatever pattern other sources use.)
Event‑stream vs plain lines: You advertise
Accept: text/event-streambut treat each line as a raw subdomain. If Profundis responds with standard SSE (data: <value>), you’ll end up emitting thedata:prefix. Please confirm the response format; if it is SSE, strip thedata:prefix (and ignore other event/control lines) before treating a line as a subdomain.Context‑aware error sends (nice‑to‑have): In error paths (marshal error, Post error, scanner error, body close error), you send directly on
resultswithout checkingctx.Done(). If the consumer stops reading when the context is canceled, these could block the goroutine. Mirroring theselectyou already use in the scan loop would avoid that, but it’s more of a robustness tweak than a blocker.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pkg/passive/sources.go(3 hunks)pkg/passive/sources_test.go(2 hunks)pkg/subscraping/sources/profundis/profundis.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/passive/sources.go (2)
pkg/subscraping/sources/profundis/profundis.go (1)
Source(16-22)pkg/subscraping/sources/domainsproject/domainsproject.go (1)
Source(16-22)
⏰ 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 (macOS-latest)
- GitHub Check: Test Builds (ubuntu-latest)
- GitHub Check: Test Builds (windows-latest)
- GitHub Check: Analyze (go)
- GitHub Check: release-test
🔇 Additional comments (1)
pkg/subscraping/sources/profundis/profundis.go (1)
100-127: Source metadata and API‑key plumbing are consistent with the passive framework
Name(),IsDefault(),NeedsKey(),AddApiKeys(), andStatistics()all line up with the existingsubscraping.Sourcecontract:
Name()returns"profundis", matching the test expectations and env‑var derivation (PROFUNDIS_API_KEY).IsDefault()returningtruemakes Profundis participate in default runs, which fits the intent for a large passive dataset.HasRecursiveSupport()returningtrueensures it’s eligible for recursive mode; once you update the tests as noted insources_test.go, this will be fully aligned.Statistics()correctly exposes the internal counters you maintain inRun.No changes needed here beyond keeping the recursion behavior/tests in sync.
2 participants
closes #1681
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.