Fix(Onyphe): Handle quoted queries correctly#693
Fix(Onyphe): Handle quoted queries correctly#693dogancanbakir merged 4 commits intoprojectdiscovery:devfrom
Conversation
release v1.1.0
…iscovery#689) Bumps [golang.org/x/oauth2](https://github.com/golang/oauth2) from 0.18.0 to 0.27.0. - [Commits](golang/oauth2@v0.18.0...v0.27.0) --- updated-dependencies: - dependency-name: golang.org/x/oauth2 dependency-version: 0.27.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe change updates the Onyphe agent's query URL construction logic to preserve double quotes in search queries. After URL-encoding the query, it replaces encoded double-quote sequences ("%22") with literal double-quote characters, ensuring the resulting API request URL matches the expected format for quoted queries. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Assessment against linked issues
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
sources/agent/onyphe/onyphe.go (2)
10-10: Remove unused import "strings"Once ReplaceAll is removed, "strings" becomes unused and will fail compilation.
import ( "encoding/json" "errors" "fmt" "io" "net/http" "net/url" - "strings" "github.com/projectdiscovery/uncover/sources" )
37-63: Consider enforcing query.Limit when emitting resultsCurrent pagination stops after counting fetched items, but individual pages are emitted in full, potentially exceeding query.Limit (page size is hardcoded to 10). Either:
- Pass the limit/remaining to query() and only emit up to the remaining count, or
- Make size dynamic per page (min(10, remaining)) by parameterizing URLTemplate.
If helpful, I can propose a minimal patch to thread remaining-through and cap emissions. Want me to draft it?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sources/agent/onyphe/onyphe.go(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
sources/agent/onyphe/onyphe.go (4)
sources/keys.go (1)
Keys(3-22)uncover.go (1)
New(56-110)sources/agent/onyphe/response.go (2)
Result(11-14)OnypheResponse(3-9)sources/agent.go (1)
Query(3-6)
🔇 Additional comments (4)
sources/agent/onyphe/onyphe.go (4)
31-33: Good: early validation of API keyClear error on missing Onyphe API key. LGTM.
82-86: LGTM: robust JSON parsing with error propagationProper unmarshalling and error propagation to the results channel.
88-92: LGTM: API-level error surfacedChecking apiResponse.Error and surfacing a clear error is good practice.
105-107: Regression: decoding %22 back to " strips quoted queries; remove ReplaceAllAfter url.QueryEscape, replacing %22 with " reintroduces the original bug. Onyphe expects the quotes to remain percent-encoded so the API receives exact-match queries. Keep the encoded %22.
Apply this fix:
- escapedQuery := url.QueryEscape(onypheRequest.Query) - escapedQuery = strings.ReplaceAll(escapedQuery, "%22", "\"") + escapedQuery := url.QueryEscape(onypheRequest.Query)Optionally, build the URL with url.Values to avoid any future mis-encoding:
u, _ := url.Parse("https://www.onyphe.io/api/v2/search/") q := url.Values{} q.Set("q", onypheRequest.Query) q.Set("page", strconv.Itoa(onypheRequest.Page)) q.Set("size", "10") u.RawQuery = q.Encode() urlWithQuery := u.String()To ensure no similar decoding remains elsewhere, run:
#!/bin/bash rg -n --fixed-strings 'strings.ReplaceAll(escapedQuery, "%22", "\"")' || trueLikely an incorrect or invalid review comment.
4 participants
What does this PR do?
This PR fixes a bug in the Onyphe search agent where queries containing double quotes (
") would fail. The root cause was a line of code that incorrectly reversed the URL encoding of double quotes, stripping them from the final API request.This change removes the faulty line, ensuring that quoted queries are correctly escaped and passed to the Onyphe API, which allows for proper exact-match searches.
Fixes #685
Changes
sources/agent/onyphe/onyphe.go:strings.ReplaceAllcall that was incorrectly decoding the%22characters back into"after they had been properly escaped byurl.QueryEscape.How to test
Set up API Key: Ensure your Onyphe API key is configured in
~/.config/uncover/provider-config.yamlor a temporaryprovider-config.yamlfile.Build the binary:
Run a test query:
To ensure your shell passes the query to the program correctly, wrap the entire query in single quotes (
').The command should now return results successfully.
Additional Notes
uncoverde-duplicates results based on the uniqueIP:Portcombination.Summary by CodeRabbit