Conversation
According to the latest documentation on the FOFA official website, update the API request construction,and convert some const variables used by FOFA into regular variables to allow customizable configuration when used as an SDK.
update zoomeye doc
Use the new censys sdk for better stability in API updates. As censys now returns multiple endpoints per search, we now iterate over every endpoint and create a new result. This also changes the way ip are saved in the raw response, as we dont have the problem anymore, as we have one ip per endpoint.
Adapt censys to new platform search
Fix(Onyphe): Handle quoted queries correctly
fix duplicate output
Add search Engine _ GreyNoise
Update FOFA API Request、Configurable SDK Integration For FOFA
WalkthroughThis PR integrates GreyNoise GNQL as a new query source with full agent implementation and CLI support, migrates Censys from manual HTTP calls to the censys-sdk-go library with updated credentials (API token and organization ID), standardizes HTTP response cleanup error handling across multiple agents, updates documentation and environment variables, and extends configuration to support GreyNoise authentication. Changes
Sequence DiagramsequenceDiagram
participant CLI as User/CLI
participant Runner as Runner
participant Session as Session
participant Agent as GreyNoise Agent
participant API as GreyNoise API
CLI->>Runner: --greynoise "query" --greynoise "query2"
activate Runner
Runner->>Session: Create with keys
activate Session
loop For each GreyNoise query
Runner->>Agent: Query(session, query)
activate Agent
Agent->>API: POST /v3/gnql (auth header, query params)
activate API
API-->>Agent: JSON response (GNQLItem[] with pagination)
deactivate API
loop Process items and pagination
Agent->>Agent: Extract IPs, hosts, ports from GNQLItem
Agent->>Agent: Emit Result per combination
Note over Agent: Check rate limits, handle errors
alt Rate limited (429)
Agent-->>Runner: ErrRateLimited
else Unauthorized (401)
Agent-->>Runner: ErrUnauthorized
else Success
Agent-->>Runner: chan Result (IP, Host, Port, Raw)
end
end
deactivate Agent
end
Runner->>Session: Write results
deactivate Session
Runner-->>CLI: Output
deactivate Runner
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
sources/agent/hunter/hunter.go (1)
113-121:StatusCodeandPortFilterparameters are set but never used in the query URL.The
hunterRequestpopulates bothStatusCode(line 53) andPortFilter(line 54), but thefmt.Sprintfcall on line 115 only passes 7 parameters:ApiKey,base64Query,Page,PageSize,IsWeb,StartTime, andEndTime. The URL template (line 17) has no placeholders for these filters.Per the Hunter QianXin API documentation,
status_codeshould be added as a URL parameter (e.g.,&status_code=%s). Theport_filtershould be embedded in the base64-encoded search query using DSL syntax (e.g.,ip.port="6379"), not appended to the URL.Fix: Update the URL template to include
&status_code=%sand modifyfmt.Sprintfto passhunterRequest.StatusCode. ForPortFilter, consider whether it should be part of the search query encoding or handled separately.sources/agent/onyphe/onyphe.go (1)
119-126: Close and drain response body on non-200 status to avoid connection leaksIn this branch, the function returns an error without consuming or closing
resp.Body. With the usualhttp.Clientsemantics, that can leak connections and prevent the transport from reusing them under repeated non-200 responses.You can defensively drain and close the body before returning:
- resp, err := session.Do(request, agent.Name()) - if err != nil { - return nil, err - } - - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("unexpected status code: %d", resp.StatusCode) - } - - return resp, nil + resp, err := session.Do(request, agent.Name()) + if err != nil { + return nil, err + } + + if resp.StatusCode != http.StatusOK { + // Drain and close body to allow connection reuse on error responses + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + return nil, fmt.Errorf("unexpected status code: %d", resp.StatusCode) + } + + return resp, nilsources/agent/google/google.go (1)
72-100: Potential resource leak:resp.Bodyis not closed.The
queryURLmethod returns an*http.Response, butresp.Bodyis never closed in thequerymethod. WhilegzipReader.Close()is properly deferred (lines 87-89), this only closes the gzip reader wrapper—not the underlying response body.Apply this diff to ensure the response body is closed:
func (agent *Agent) query(session *sources.Session, googleRequest *Request, results chan sources.Result) []string { resp, err := agent.queryURL(session, googleRequest) if err != nil { results <- sources.Result{Source: agent.Name(), Error: err} return nil } + defer func() { + _ = resp.Body.Close() + }() var apiResponse Response if resp.Header.Get("Content-Encoding") == "gzip" {sources/agent/driftnet/driftnet.go (1)
100-123: Deferred closure now leaks earlier response bodies in loopsWrapping
resp.Body.Close()in a closure inside the loops introduces a subtle bug:resp, queryError := agent.queryURL(...) defer func() { _ = resp.Body.Close() }()Because
respis a loop-scoped variable reused on each iteration (:=only defines it once), all deferred closures capture the samerespand will run with its final value. As a result:
- Only the last
resp.Bodyis closed (multiple times).- All previous response bodies are never closed, leading to descriptor/connection leaks under load.
Previously,
defer resp.Body.Close()evaluated the receiver at defer time, so each response was closed correctly.Fix by capturing the body value as a parameter to the deferred function in both locations:
- defer func() { - _ = resp.Body.Close() - }() + defer func(body io.ReadCloser) { + _ = body.Close() + }(resp.Body)and similarly for the CIDR path:
- defer func() { - _ = resp.Body.Close() - }() + defer func(body io.ReadCloser) { + _ = body.Close() + }(resp.Body)This keeps the “ignore close error” behavior but ensures each response body is closed exactly once.
Also applies to: 217-237
README.md (1)
179-196: Env var examples align with Censys changes but omit GreyNoiseAdding
CENSYS_API_TOKENandCENSYS_ORGANIZATION_IDhere matches the new env handling insources/provider.go. However, since the code readsGREYNOISE_API_KEY, it would be good to add it to this block as well for discoverability, e.g.:export DRIFTNET_API_KEY=xxx +export GREYNOISE_API_KEY=xxx
🧹 Nitpick comments (14)
runner/output_writer.go (1)
72-79: Clarify the intentional ignoring ofCloseerrors (optional).Using
_ = fileWriter.Close()keeps the previous behavior (ignoring errors) but makes it more explicit. If this is intentionally best-effort cleanup, consider adding a short comment or logging failures so future readers and linters know the errors are deliberately ignored, not accidentally forgotten.sources/agent/hunter/hunter.go (1)
20-27: Package-level mutable state may cause issues with concurrent usage.These variables are shared across all agent instances. If the agent is used concurrently with different configurations, this could lead to race conditions. Consider passing these as parameters or part of the session/query configuration if concurrent usage with different settings is expected.
sources/agent/fofa/fofa.go (2)
18-19: Full flag wiring is fine but can be made more flexible and consistentThe new
Fullplumbing (globalFullbool,&full=%tinURL, andFofaRequest.Full boolpassed intofmt.Sprintf) looks coherent and should produce URLs like...&full=true/...&full=false.A couple of small improvements to consider:
In
queryURL, you currently hard‑codeFields:fofaURL := fmt.Sprintf(URL, session.Keys.FofaKey, base64Query, Fields, fofaRequest.Page, fofaRequest.Size, fofaRequest.Full)but
FofaRequestalready has aFieldsfield. Using the struct field improves testability and future flexibility:fofaURL := fmt.Sprintf(URL, session.Keys.FofaKey, base64Query, Fields, fofaRequest.Page, fofaRequest.Size, fofaRequest.Full)
- fofaURL := fmt.Sprintf(URL, session.Keys.FofaKey, base64Query, fofaRequest.Fields, fofaRequest.Page, fofaRequest.Size, fofaRequest.Full)
- Exposing `Size`, `Fields`, and `Full` as package‑level vars is convenient for CLI overrides, but it also makes them mutable globals. As long as they’re only set once at startup before any queries run, you’re fine; if you ever need per‑query control or concurrent tweaking, it’d be safer to move them into `sources.Query` or `Session` instead of relying on globals. These aren’t blockers, but tightening them up now will make the FOFA agent easier to evolve. Also applies to: 21-29, 50-56, 73-76, 121-127 --- `61-64`: **Re‑verify pagination break condition vs meaning of `fofaResponse.Size`** The new loop termination condition: ```go numberOfResults += len(fofaResponse.Results) page++ size := fofaResponse.Size if size == 0 || numberOfResults >= query.Limit || len(fofaResponse.Results) == 0 || numberOfResults > size { break }mixes a cumulative counter (
numberOfResults) withfofaResponse.Size. IfSizerepresents the total number of matches,numberOfResults > sizeis reasonable. But ifSizeis per‑page size (common in APIs),numberOfResults > sizewill become true after the second page and prematurely stop pagination.This behavior predates the
Fullchange, but given you’re touching pagination logic (numberOfResults >= query.Limit), it’s a good moment to double‑check FOFA’s response schema and ensure:
fofaResponse.Sizeis indeed “total available results,” or- If it’s page size, you should compare against a “total” field instead (if available), or drop the
numberOfResults > sizeclause.Please confirm against FOFA’s API docs or a real response and adjust as needed.
sources/agent/onyphe/onyphe.go (2)
31-62: API key validation looks good; consider clarifying limit semanticsThe new empty-key check and the overall pagination loop look correct and consistent with other agents.
One behavioral quirk that remains:
maxResultsis enforced only at page granularity. Ifquery.Limitis not a multiple of the page size (10), the final page can push you over the requested limit (e.g. 90 + 20 ⇒ 110 results) before the loop terminates.If
Limitis intended as a strict cap rather than “best effort”, consider enforcing it while emitting results (or adjusting page size / stopping mid-page). Otherwise, a short comment noting that the limit is best-effort would help future readers.
10-10: Manual%22→"replacement afterurl.QueryEscapeis brittle; consider reverting or documenting
escapedQuery := url.QueryEscape(onypheRequest.Query)is the right primitive for building theqparameter. Replacing all%22with literal"afterward:escapedQuery = strings.ReplaceAll(escapedQuery, "%22", "\"")introduces a few risks:
- Queries that intentionally contain the literal substring
%22(already-encoded input) will be silently rewritten to include", changing their meaning.http.NewRequest/ URL parsing may re-encode the raw"back to%22anyway, so this line might be a no-op in practice but adds confusion.- Without a comment tying this to a specific Onyphe API quirk, future maintainers will have a hard time understanding or safely modifying this behavior.
Unless you’ve confirmed that Onyphe requires literal double quotes in the
qparameter and fails when they’re sent as%22, I’d suggest simplifying back to the standard escaping (and dropping thestringsimport):- escapedQuery := url.QueryEscape(onypheRequest.Query) - escapedQuery = strings.ReplaceAll(escapedQuery, "%22", "\"") - urlWithQuery := fmt.Sprintf(URLTemplate, escapedQuery, onypheRequest.Page) + escapedQuery := url.QueryEscape(onypheRequest.Query) + urlWithQuery := fmt.Sprintf(URLTemplate, escapedQuery, onypheRequest.Page)If this replacement is required to work around a real Onyphe quirk, please add a brief code comment and, ideally, a test that demonstrates the failure without it.
Also applies to: 107-110
sources/provider.go (1)
159-176: Censys & GreyNoise env var handling: OK, but consider migration messaging
LoadProviderKeysFromEnv:
- Uses
CENSYS_API_TOKEN+CENSYS_ORGANIZATION_ID, aligning with the new SDK auth model.- Reads
GREYNOISE_API_KEYintoprovider.GreyNoise, matching the rest of the integration.This does, however, silently drop support for any legacy
CENSYS_API_ID/CENSYS_API_SECRETenv vars. If many users depend on those, consider a transitional path (e.g., also reading old vars with a warning, or clearly documenting the breaking change in release notes).Would you confirm whether the previous Censys env var names are explicitly deprecated in your release notes/changelog? If not, it’s worth calling that out to avoid confusion for existing users.
integration-tests/source-test.go (2)
302-302: Inconsistent cleanup pattern.Other test cases in this file use
defer func() { _ = os.RemoveAll(ConfigFile) }()but this one usesdefer os.RemoveAll(ConfigFile). While functionally similar, the pattern should be consistent with the rest of the file.- defer os.RemoveAll(ConfigFile) + defer func() { + _ = os.RemoveAll(ConfigFile) + }()
304-318: Test silently passes on failures and empty results.This test returns
nil(success) even when the query fails or returns no results, which means CI will always report success regardless of actual GreyNoise functionality. While the comments explain this is due to Community vs Enterprise API key differences, consider returning an error whenGREYNOISE_API_KEYis set but the query unexpectedly fails with a non-plan-related error.sources/agent/censys/censys.go (2)
89-89: Remove commented-out code.Dead code should be removed rather than left as comments.
results <- sources.Result{Source: agent.Name(), Error: err} - // httputil.DrainResponseBody(resp) return nil
93-97: Variable shadowing:resultis declared twice in nested scopes.The inner
resultat line 97 shadows the outerresultfrom line 93. Consider renaming one of them (e.g.,sdkResultfor the outer orrfor the inner) to improve clarity.- if result := resp.ResponseEnvelopeSearchQueryResponse.Result; result != nil { - for _, censysResult := range result.Hits { + if sdkResult := resp.ResponseEnvelopeSearchQueryResponse.Result; sdkResult != nil { + for _, censysResult := range sdkResult.Hits {sources/agent/greynoise/response.go (1)
47-47:Tagsfield usesjson.RawMessagebutTagstruct is defined.The
Tagsfield is declared asjson.RawMessage(line 47), but there's a fully-definedTagstruct (lines 80-92). If the API consistently returns an array of tags, consider using[]Taginstead for type safety. If the structure varies,json.RawMessageis appropriate, but then theTagstruct may be dead code.sources/agent/greynoise/greynoise.go (2)
173-178: Ignored error fromurl.Parseand redundant URL construction.The error from
url.Parse(URL)is discarded (line 173). WhileURLis a constant and unlikely to fail, consider either handling the error or simplifying the construction sinceURLalready contains the path.- path := "/v3/gnql" - if request.ExcludeRaw { - path = "/v3/gnql/metadata" - } - - baseURL, _ := url.Parse(URL) - baseURL.Path = path - fullURL := baseURL.String() - if enc := params.Encode(); enc != "" { - fullURL = fullURL + "?" + enc - } + path := URL + if request.ExcludeRaw { + path = "https://api.greynoise.io/v3/gnql/metadata" + } + fullURL := path + if enc := params.Encode(); enc != "" { + fullURL = path + "?" + enc + }Alternatively, define both URL constants at the package level.
281-288: IPv6 address handling in hostname extraction.The port-stripping logic at lines 281-285 attempts to handle IPv6 addresses with ports (e.g.,
[::1]:8080), but the bracket handling is partial. Consider usingnet.SplitHostPortfor more robust host:port parsing that handles both IPv4 and IPv6 correctly.// More robust approach using net.SplitHostPort if host, _, err := net.SplitHostPort(h); err == nil { h = host }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
.github/workflows/build-test.ymlis excluded by!**/*.yml.github/workflows/provider-integration.ymlis excluded by!**/*.yml.github/workflows/release-binary.ymlis excluded by!**/*.yml.github/workflows/release-test.ymlis excluded by!**/*.ymlgo.sumis excluded by!**/*.sumsources/agent/censys/example.jsonis excluded by!**/*.jsonsources/agent/greynoise/example.jsonis excluded by!**/*.json
📒 Files selected for processing (25)
README.md(3 hunks)go.mod(2 hunks)integration-tests/integration-test.go(1 hunks)integration-tests/source-test.go(12 hunks)runner/options.go(7 hunks)runner/output_writer.go(1 hunks)runner/runner.go(0 hunks)sources/agent/binaryedge/binaryedge.go(1 hunks)sources/agent/censys/censys.go(3 hunks)sources/agent/censys/response.go(0 hunks)sources/agent/driftnet/driftnet.go(2 hunks)sources/agent/fofa/fofa.go(5 hunks)sources/agent/google/google.go(1 hunks)sources/agent/greynoise/greynoise.go(1 hunks)sources/agent/greynoise/request.go(1 hunks)sources/agent/greynoise/response.go(1 hunks)sources/agent/hunter/hunter.go(4 hunks)sources/agent/hunter/request.go(1 hunks)sources/agent/odin/odin.go(1 hunks)sources/agent/onyphe/onyphe.go(4 hunks)sources/agent/zoomeye/zoomeye.go(1 hunks)sources/keys.go(3 hunks)sources/provider.go(5 hunks)sources/session.go(2 hunks)uncover.go(3 hunks)
💤 Files with no reviewable changes (2)
- sources/agent/censys/response.go
- runner/runner.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-12T11:42:48.205Z
Learnt from: Xanderux
Repo: projectdiscovery/uncover PR: 672
File: sources/agent/onyphe/response.go:3-14
Timestamp: 2025-05-12T11:42:48.205Z
Learning: The Onyphe integration in Uncover deliberately captures only IP and Port information from API responses to minimize memory usage, as other fields returned by the Onyphe API are not needed for Uncover's functionality.
Applied to files:
README.md
🧬 Code graph analysis (8)
sources/agent/hunter/request.go (1)
sources/agent/hunter/hunter.go (1)
PortFilter(23-23)
sources/agent/greynoise/request.go (1)
sources/agent.go (1)
Query(3-6)
uncover.go (2)
sources/agent/greynoise/greynoise.go (1)
Agent(36-36)sources/agent.go (1)
Agent(8-11)
sources/agent/hunter/hunter.go (2)
sources/agent.go (1)
Query(3-6)sources/agent/hunterhow/hunterhow.go (1)
Size(14-14)
sources/agent/onyphe/onyphe.go (3)
sources/keys.go (1)
Keys(3-23)sources/agent/onyphe/response.go (2)
Result(11-14)OnypheResponse(3-9)sources/agent.go (1)
Query(3-6)
integration-tests/source-test.go (2)
testutils/integration.go (1)
RunUncoverAndGetResults(10-38)uncover.go (1)
New(57-113)
sources/agent/censys/censys.go (3)
sources/keys.go (1)
Keys(3-23)sources/result.go (1)
Result(8-17)sources/session.go (1)
Session(39-44)
sources/agent/fofa/fofa.go (2)
sources/keys.go (1)
Keys(3-23)sources/result.go (1)
Result(8-17)
⏰ 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: release-test
- GitHub Check: Test Builds (macOS-latest, 1.24.x)
- GitHub Check: Test Builds (ubuntu-latest, 1.24.x)
- GitHub Check: Test Builds (windows-latest, 1.24.x)
- GitHub Check: Analyze (go)
🔇 Additional comments (32)
sources/agent/hunter/request.go (1)
12-12: LGTM!The
PortFilterfield is correctly added with appropriate type and JSON tag. However, note that this field is currently not being used in the URL construction inhunter.go(see related comment there).sources/agent/onyphe/onyphe.go (1)
74-94: Response body closing and JSON/API error handling are correctDeferring
resp.Body.Close()via an anonymous function and explicitly ignoring the close error is idiomatic here, and the JSON unmarshal +apiResponse.Errorchecks correctly turn parse/API errors intosources.Result{Error: ...}and stop pagination by returningnil.No functional issues from this change.
sources/agent/zoomeye/zoomeye.go (1)
97-99: LGTM!The deferred closure pattern is consistent with the standardized approach across other agents in this PR. The response body is properly closed after the error check.
sources/agent/binaryedge/binaryedge.go (1)
44-46: LGTM!The deferred closure is correctly placed after the error check and follows the standardized pattern across the codebase.
sources/agent/odin/odin.go (1)
93-95: LGTM!The deferred closure follows the standardized pattern and is correctly placed after the error check.
go.mod (1)
3-3: Go 1.24.0 is stable and available for production use.Go 1.24 was released on February 11, 2025, making it a stable release that has been in production for several months. No action required.
sources/session.go (2)
19-36: GreyNoise default rate limit wiring looks consistent
"greynoise"entry matches the agent/engine naming and other 1 req/sec defaults. No issues from a rate limiting perspective.
120-123: Non-200 error context change is acceptableSwitching to
url.QueryUnescape(request.String())gives a more complete, human-readable representation of the request in errors. Since the unescape error is intentionally ignored and only used for logging, this is fine.README.md (1)
133-143: Censys provider config docs match new token/orgId formatThe Censys YAML examples now align with
provider.go(CENSYS_API_TOKEN:CENSYS_ORGANIZATION_ID). This is consistent with the newKeysfields and SDK usage.integration-tests/integration-test.go (1)
20-38: GreyNoise integration tests are correctly wiredAdding
"greynoise": greynoiseTestcases{}to thetestsmap matches the naming of the new agent and follows the existing pattern for other sources.uncover.go (2)
8-27: GreyNoise agent import is consistent with existing agent structureImporting
sources/agent/greynoisealongside the other agents is straightforward and matches the project’s layout.
59-95: GreyNoise agent selection is correctly integratedThe
"greynoise"case inNewappends&greynoise.Agent{}tos.Agents, andAllAgents()lists"greynoise"as supported. The engine string matches CLI (-greynoise) and the rate-limit key insources/session.go, so everything lines up.sources/provider.go (3)
23-40: GreyNoise provider field matches YAML tag and usageAdding
GreyNoise []string 'yaml:"greynoise"'toProvideris consistent with the configuration format used elsewhere and will deserialize cleanly fromprovider-config.yaml.
55-62: Key extraction for Censys and GreyNoise is coherent
- Censys now expects
token:orgIdand maps cleanly intokeys.CensysTokenandkeys.CensysOrgId.- GreyNoise selects a random key from
provider.GreyNoiseintokeys.GreyNoiseKey, matchingKeysand the greynoise agent’s expectations.No issues here.
Also applies to: 120-125
179-197: HasKeys updated correctly for GreyNoiseIncluding
len(provider.GreyNoise) > 0inHasKeys()keeps the “any provider has keys” logic in sync with the new source.sources/keys.go (1)
3-23: Keys struct and Empty() logic remain sound after Censys/GreyNoise changes
- Swapping
CensysSecretforCensysOrgIdand addingGreyNoiseKeyaligns with the new auth model and provider fields.Empty()now checksCensysOrgIdandGreyNoiseKeyin addition to existing fields, maintaining the invariant that it’s only true when all keys are unset.No functional issues spotted.
Also applies to: 25-45
runner/options.go (6)
49-67: Options struct cleanly extends to GreyNoiseAdding
GreyNoise goflags.StringSlicealongside other per-engine slices is consistent with the existing design and keeps configuration symmetric.
75-79: CLI flags and help text for GreyNoise are consistent
--engine/-ehelp now listsgreynoise, matching the internal engine string.- New
--greynoise/-gnflag mirrors the pattern of other per-engine flags, includinggoflags.FileStringSliceOptions.This should make GreyNoise discoverable and usable from the CLI.
Also applies to: 81-99
157-177: Engine defaulting logic correctly considers GreyNoiseExtending the
genericutil.EqualsAll(0, ...)check to includelen(options.Driftnet)andlen(options.GreyNoise)ensures:
- Shodan is only auto-selected when no engine or per-engine queries (including GreyNoise) are specified.
- Passing
-greynoiseor-e greynoiseprevents unintended fallback to shodan.Looks correct.
226-250: Validation correctly treats GreyNoise as a first-class query sourceAdding
len(options.GreyNoise)to the “no query provided” check means-greynoisealone counts as having queries; likewise, it’s included in the “no engine specified” check. This keeps GreyNoise behavior in line with other providers.
257-278: Engine validation includes GreyNoiseThe “no engine specified” validation now factors in
len(options.GreyNoise), making it consistent with other engines and preventing spurious errors when only GreyNoise-specific flags are used.
303-321: GreyNoise is correctly added in appendAllQueries
appendQuery(options, "greynoise", options.GreyNoise...)ensures:
- GreyNoise gets added to
options.Engineif there are greynoise-specific queries.- Those queries are merged into the global
options.Queryset without duplication.This matches the behavior of other engines.
sources/agent/greynoise/request.go (1)
1-8: LGTM!Clean data structure with appropriate JSON tags. The
omitemptytags on optional fields and thejson:"-"onExcludeRaware correct for the intended serialization behavior.integration-tests/source-test.go (1)
26-28: Cleanup pattern standardization looks good.The change to wrap
os.RemoveAllin an anonymous function with explicit error discarding (_ =) is consistently applied across all existing test cases, improving code uniformity.sources/agent/censys/censys.go (3)
72-82: SDK query implementation looks correct.The SDK integration properly handles pagination with
PageTokenand uses the helpercensyssdkgo.Int64()for type conversion. On the first request,&censysRequest.Cursorwill point to an empty string—verify this is handled correctly by the SDK.
104-106: Verify Port type compatibility—SDK type definition not accessible.The review comment raises a valid concern. The Censys SDK port field is Int64 based on platform documentation. However, I cannot access the SDK code in the sandbox to confirm the exact Go type of
host.Portin censys-sdk-go v0.19.1.Code at lines 104–105 dereferences
host.Portdirectly without casting toresult.Port(which isint). In Go, this requireshost.Portto already be*intor compatible. If it's*int64as the web documentation suggests, the code would require explicit type conversion viaint()to compile.Please verify:
- Run
go buildto confirm the code compiles without type errors- If compilation fails, add explicit conversion:
result.Port = int(*host.Port)
93-116: The identified nil pointer dereference concern is incorrect based on how the censys-sdk-go SDK structures its response types.The censys-sdk-go v0.19.1 SDK is Speakeasy-generated and follows the standard pattern where nullable API response fields are represented as regular Go values (not pointers), accompanied by respjson.Field metadata to distinguish null/omitted/present states. Accessing
censysResult.WebpropertyV1.Resource.Endpointscannot panic—if these fields are unset, they are zero-valued and the range loop simply doesn't execute. No nil guards are needed.Likely an incorrect or invalid review comment.
sources/agent/greynoise/response.go (1)
1-149: Data models are well-structured for GNQL API responses.The response types comprehensively cover the GNQL API structure with appropriate JSON tags. The use of nested anonymous structs in
RawDatais acceptable for deserialization-only types.sources/agent/greynoise/greynoise.go (4)
116-129: Cartesian product of hosts and ports may cause result explosion.When both hosts and ports are present, the code emits
len(hosts) × len(ports)results per item. For items with many hosts/ports, this could significantly multiply the result count. Ensure this is the intended behavior, as a single GNQL item could produce dozens ofResultentries.
237-271: Port collection is comprehensive.The function correctly aggregates ports from multiple sources (Scan, JA3, HASSH, and HTTP hosts) using a map for deduplication. The logic for extracting ports from host strings (lines 259-264) complements the hostname extraction approach.
194-217: Comprehensive error handling with wrapped errors.The status code handling covers relevant error cases (401, 403, 404, 429, 5xx) with informative messages. The use of
%wallows callers to useerrors.Is()for error type checking, and theRetry-Afterhint extraction is a nice touch for rate limiting.
54-57: The review comment is incorrect; the code is already consistent with GreyNoise API limits.The default
pageSizeof 1000 does not exceed API limits—it matches the GreyNoise GNQL API default of 1,000 results per page, and the clamping to 10,000 correctly implements the API maximum of 10,000 results per page. The code is already properly aligned with the documented API constraints.Likely an incorrect or invalid review comment.
| go 1.24.0 | ||
|
|
||
| require ( | ||
| github.com/censys/censys-sdk-go v0.19.1 |
There was a problem hiding this comment.
🧩 Analysis chain
Verify censys-sdk-go version.
The new censys-sdk-go dependency aligns with the Censys migration mentioned in the PR summary. Confirm v0.19.1 is the latest stable version.
🏁 Script executed:
#!/bin/bash
# Check latest version of censys-sdk-go
curl -s "https://proxy.golang.org/github.com/censys/censys-sdk-go/@v/list" | tail -5Length of output: 39
Update censys-sdk-go to the latest stable version.
The censys-sdk-go dependency is pinned to v0.19.1, but v0.22.3 is the latest stable version available. Update to v0.22.3 or review the changelog to ensure v0.19.1 is intentionally pinned (e.g., due to compatibility constraints).
🤖 Prompt for AI Agents
In go.mod around line 6, the censys-sdk-go dependency is pinned to v0.19.1;
update the version to the latest stable (v0.22.3) unless there is a deliberate
compatibility constraint. Edit go.mod to replace v0.19.1 with v0.22.3, run `go
get github.com/censys/censys-sdk-go@v0.22.3` and `go mod tidy`, then run the
test suite and scan the changelog for breaking changes to confirm compatibility;
if incompatibilities exist, document the reason to keep the older pin.
| Required API keys can be obtained by signing up on following platform [Shodan](https://account.shodan.io/register), [Censys](https://censys.io/register), [Fofa](https://fofa.info/toLogin), [Quake](https://quake.360.net/quake/#/index), [Hunter](https://user.skyeye.qianxin.com/user/register?next=https%3A//hunter.qianxin.com/api/uLogin&fromLogin=1), [ZoomEye](https://www.zoomeye.ai), [Netlas](https://app.netlas.io/registration/), [CriminalIP](https://www.criminalip.io/register), [Publicwww](https://publicwww.com/profile/signup.html), Google [[1]](https://developers.google.com/custom-search/v1/introduction#identify_your_application_to_google_with_api_key),[[2]](https://programmablesearchengine.google.com/controlpanel/create), [Onyphe](https://search.onyphe.io/signup) and [Driftnet](https://driftnet.io/auth?state=signup). | ||
|
|
There was a problem hiding this comment.
Consider documenting GreyNoise in the “Required API keys” list
This list now includes Driftnet but not GreyNoise, even though GreyNoise is a first-class provider in the code (options, provider, keys, agent, tests). Recommend appending GreyNoise with its signup URL, e.g.:
..., Onyphe, Driftnet, and GreyNoise (or the appropriate signup page).
This keeps the README in sync with supported providers.
🤖 Prompt for AI Agents
In README.md around lines 198-199 the "Required API keys" list omits GreyNoise
even though the codebase supports it; add GreyNoise to the list with its signup
URL. Edit the sentence to append ",
[GreyNoise](https://www.greynoise.io/register)" (or the appropriate GreyNoise
signup page) after Driftnet so the README matches supported providers.
| fofaResponse := &FofaResponse{} | ||
|
|
||
| RespBodyByBodyBytes, _ := io.ReadAll(resp.Body) | ||
| if err := json.NewDecoder(resp.Body).Decode(fofaResponse); err != nil { | ||
| results <- sources.Result{Source: agent.Name(), Error: err} | ||
| result := sources.Result{Source: agent.Name()} | ||
| defer func(Body io.ReadCloser) { | ||
| if bodyCloseErr := Body.Close(); bodyCloseErr != nil { | ||
| gologger.Info().Msgf("response body close error : %v", bodyCloseErr) | ||
| } | ||
| }(resp.Body) | ||
| raw, _ := json.Marshal(RespBodyByBodyBytes) | ||
| result.Raw = raw | ||
| results <- result | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Response body handling and JSON decode are currently broken and leak connections
io.ReadAll(resp.Body)at Line 91 consumes the entire body; the subsequentjson.NewDecoder(resp.Body).Decode(fofaResponse)then reads from an already‑drained stream and will consistently return an error (typically EOF). That means you will never successfully parse a FOFA response and will always go down the error path.resp.Bodyis only closed inside the JSON‑decode error branch. On a “successful” decode (if it ever occurred), the body would never be closed, leaking connections and eventually exhausting the HTTP client’s pool.- In the error path you also double‑encode the raw body:
json.Marshal(RespBodyByBodyBytes)wraps the bytes in JSON again, andResult.Erroris left nil, so consumers can’t distinguish success from failure via the error field.
A safer pattern is:
- Immediately
deferclosing the body once you knowerr == nil, with logging on close failure. - Read the body once into a
[]byte. json.Unmarshalfrom that buffer.- On decode failure, emit a
Resultthat carries bothRawandError.
Suggested fix:
- fofaResponse := &FofaResponse{}
- RespBodyByBodyBytes, _ := io.ReadAll(resp.Body)
- if err := json.NewDecoder(resp.Body).Decode(fofaResponse); err != nil {
- result := sources.Result{Source: agent.Name()}
- defer func(Body io.ReadCloser) {
- if bodyCloseErr := Body.Close(); bodyCloseErr != nil {
- gologger.Info().Msgf("response body close error : %v", bodyCloseErr)
- }
- }(resp.Body)
- raw, _ := json.Marshal(RespBodyByBodyBytes)
- result.Raw = raw
- results <- result
- return nil
- }
+ defer func(body io.ReadCloser) {
+ if bodyCloseErr := body.Close(); bodyCloseErr != nil {
+ gologger.Info().Msgf("fofa: response body close error: %v", bodyCloseErr)
+ }
+ }(resp.Body)
+
+ fofaResponse := &FofaResponse{}
+ respBodyBytes, err := io.ReadAll(resp.Body)
+ if err != nil {
+ results <- sources.Result{Source: agent.Name(), Error: err}
+ return nil
+ }
+ if err := json.Unmarshal(respBodyBytes, fofaResponse); err != nil {
+ results <- sources.Result{
+ Source: agent.Name(),
+ Raw: respBodyBytes,
+ Error: err,
+ }
+ return nil
+ }This keeps Result.Raw as the exact response body, surfaces the decode failure via Error, and ensures the body is always closed.
| RespBodyByBodyBytes, _ := io.ReadAll(resp.Body) | ||
| if err := json.NewDecoder(resp.Body).Decode(hunterResponse); err != nil { | ||
| results <- sources.Result{Source: agent.Name(), Error: err} | ||
| result := sources.Result{Source: agent.Name()} | ||
| defer func(Body io.ReadCloser) { | ||
| if bodyCloseErr := Body.Close(); bodyCloseErr != nil { | ||
| gologger.Info().Msgf("response body close error : %v", bodyCloseErr) | ||
| } | ||
| }(resp.Body) | ||
| raw, _ := json.Marshal(RespBodyByBodyBytes) | ||
| result.Raw = raw | ||
| results <- result | ||
| return nil |
There was a problem hiding this comment.
Critical bug: Response body is exhausted before JSON decoding.
io.ReadAll(resp.Body) consumes the entire body, so json.NewDecoder(resp.Body).Decode() will always fail with an empty reader. Additionally, on the error path, json.Marshal(RespBodyByBodyBytes) will base64-encode the byte slice rather than store the raw response.
Apply this diff to fix the body handling:
- RespBodyByBodyBytes, _ := io.ReadAll(resp.Body)
- if err := json.NewDecoder(resp.Body).Decode(hunterResponse); err != nil {
- result := sources.Result{Source: agent.Name()}
- defer func(Body io.ReadCloser) {
- if bodyCloseErr := Body.Close(); bodyCloseErr != nil {
- gologger.Info().Msgf("response body close error : %v", bodyCloseErr)
- }
- }(resp.Body)
- raw, _ := json.Marshal(RespBodyByBodyBytes)
- result.Raw = raw
- results <- result
+ defer func() {
+ if err := resp.Body.Close(); err != nil {
+ gologger.Info().Msgf("response body close error: %v", err)
+ }
+ }()
+
+ bodyBytes, err := io.ReadAll(resp.Body)
+ if err != nil {
+ results <- sources.Result{Source: agent.Name(), Error: err}
+ return nil
+ }
+
+ if err := json.Unmarshal(bodyBytes, hunterResponse); err != nil {
+ result := sources.Result{Source: agent.Name()}
+ result.Raw = bodyBytes
+ results <- result
return nil
}🤖 Prompt for AI Agents
In sources/agent/hunter/hunter.go around lines 85 to 96, the code reads
resp.Body with io.ReadAll and then tries to decode JSON from the same resp.Body
(which is already exhausted) and also json.Marshal is being used incorrectly on
the raw bytes; fix it by first reading resp.Body into a []byte variable, close
resp.Body immediately after reading, create a bytes.Reader (or bytes.NewBuffer)
from that []byte and pass it to json.NewDecoder().Decode() so decoding operates
on the saved bytes; on the error path set result.Raw = bodyBytes (the raw
[]byte) rather than json.Marshal(bodyBytes), and keep the body-close logic but
ensure it runs after the initial read.
7 participants
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.