feat(pius-discovery): consume whois+email preseeds via reverse-whois-by-email#96
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Gemini Review
{
"session_id": "50fbdfdf-290e-4ac2-80a8-b86fc3025421",
"response": "",
"stats": {
"models": {
"gemini-3.1-pro-preview": {
"api": {
"totalRequests": 2,
"totalErrors": 0,
"totalLatencyMs": 4605
},
"tokens": {
"input": 14365,
"prompt": 22481,
"candidates": 26,
"total": 22604,
"cached": 8116,
"thoughts": 97,
"tool": 0
},
"roles": {
"main": {
"totalRequests": 2,
"totalErrors": 0,
"totalLatencyMs": 4605,
"tokens": {
"input": 14365,
"prompt": 22481,
"candidates": 26,
"total": 22604,
"cached": 8116,
"thoughts": 97,
"tool": 0
}
}
}
}
},
"tools": {
"totalCalls": 1,
"totalSuccess": 1,
"totalFail": 0,
"totalDurationMs": 9,
"totalDecisions": {
"accept": 1,
"reject": 0,
"modify": 0,
"auto_accept": 0
},
"byName": {
"read_file": {
"count": 1,
"success": 1,
"fail": 0,
"durationMs": 9,
"decisions": {
"accept": 1,
"reject": 0,
"modify": 0,
"auto_accept": 0
}
}
}
},
"files": {
"totalLinesAdded": 0,
"totalLinesRemoved": 0
}
},
"error": {
"type": "INVALID_STREAM",
"message": "Invalid stream: The model returned an empty response or malformed tool call."
}
}EOF
gemini_errors<<EOF
Warning: 256-color support not detected. Using a terminal with at least 256-color support is recommended for a better visual experience.
Warning: tools.exclude in settings.json is deprecated and will be removed in 1.0. Migrate to Policy Engine: https://geminicli.com/docs/core/policy-engine/
YOLO mode is enabled. All tool calls will be automatically approved.
YOLO mode is enabled. All tool calls will be automatically approved.
Ripgrep is not available. Falling back to GrepTool.
Reviewed by Gemini (gemini-3.1-pro-preview)
There was a problem hiding this comment.
Codex Review
Critical Issues
None.
Security
No security concerns flagged.
Suggestions
No suggestions.
No critical issues — LGTM pending human review.
Validation note: go test ./pkg/lib ./pkg/runner ./pkg/plugins/domains could not run because the sandbox is read-only and Go could not create /home/runner/go for the module cache.
Reviewed by Codex (gpt-5.5)
There was a problem hiding this comment.
Claude Review
Critical issues
None. The seed routing is wired end-to-end and consistent: whois.go emits whois+email with the email in Value, invoke.go switches Value→cfg.Email, runner/api.go sets Input.Email, and both ViewDNS/Whoxy plugins fall back to Email only when OrgName is empty. Input.Email already existed in pkg/plugins/plugin.go, so the change compiles. The default arm correctly preserves whois+company/whois+name/edgar+company→Org behavior.
Security
- Email seed is
url.QueryEscaped into bothq=(ViewDNS) andemail=(Whoxy) params, so theName <addr>display-name form (R3) and odd input degrade to zero results, not injection. - No new secret exposure: ViewDNS still strips the API-key-bearing URL from its error; Whoxy logs
query(the email/org) but not the key. Worth confirming downstream consumers are fine with registrant emails now appearing inData["org"]provenance and in Whoxy warn logs. - R2 (generic role mailboxes like
abuse@/hostmaster@reverse-whois'ing into unrelated portfolios) is acknowledged as deferred — fine to ship since it degrades to clean-zero, but a skip-list is the right follow-up before this runs broadly.
Test coverage
Present and adequate — org/email-mode Run tests for both plugins, Accepts matrices, Invoke routing assertions, and an end-to-end no-keys/no-error runner test.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c8056b1d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe PR adds ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pkg/plugins/domains/whoxy_reverse_whois_test.go (1)
213-215: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the exact query parameters.
This test would still pass if
email=were sent with the wrong value. Parse the query and assert the decoded parameter values.Proposed test tightening
- assert.Contains(t, r.URL.RawQuery, "reverse=whois") - assert.Contains(t, r.URL.RawQuery, "email=") - assert.NotContains(t, r.URL.RawQuery, "name=") // email mode must NOT send name= + q := r.URL.Query() + assert.Equal(t, "whois", q.Get("reverse")) + assert.Equal(t, "admin@acme.com", q.Get("email")) + assert.Empty(t, q.Get("name"), "email mode must NOT send name=")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/plugins/domains/whoxy_reverse_whois_test.go` around lines 213 - 215, The query assertion in the reverse WHOIS test is too loose because it only checks that "email=" exists, so it can pass even when the value is incorrect. Update the test around the request URL handling in the reverse whois flow to parse r.URL.RawQuery and assert the decoded values for the exact parameters, especially reverse=whois and email, while still confirming name is absent in email mode. Use the existing test case in whoxy_reverse_whois_test.go to tighten the assertions without relying on substring matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/lib/invoke.go`:
- Around line 37-42: The routing in invoke.go is fine, but the downstream error
formatting in the same flow now risks leaking registrant email PII through the
pipeline error message. Update the error construction around the `pipeline for
%q` path in `invoke` so the `whois+email` case does not include the raw
`input.Value` (redact it, omit it, or substitute a safe placeholder), while
preserving the existing org-path behavior and any useful context for debugging.
In `@pkg/plugins/domains/whoxy_reverse_whois.go`:
- Line 76: The warning in the WhoXY reverse WHOIS pagination path is logging the
raw query value, which exposes email seeds when byEmail is true. Update the
slog.Warn call inside the reverse whois pagination logic to avoid printing the
raw query string; instead log the seed type or a redacted/hashed form, using the
existing query/byEmail handling in the whoxy_reverse_whois flow.
---
Nitpick comments:
In `@pkg/plugins/domains/whoxy_reverse_whois_test.go`:
- Around line 213-215: The query assertion in the reverse WHOIS test is too
loose because it only checks that "email=" exists, so it can pass even when the
value is incorrect. Update the test around the request URL handling in the
reverse whois flow to parse r.URL.RawQuery and assert the decoded values for the
exact parameters, especially reverse=whois and email, while still confirming
name is absent in email mode. Use the existing test case in
whoxy_reverse_whois_test.go to tighten the assertions without relying on
substring matching.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f8aeaa50-fb44-4b74-844f-0353269380aa
📒 Files selected for processing (10)
pkg/lib/discovery.gopkg/lib/discovery_test.gopkg/lib/invoke.gopkg/lib/invoke_test.gopkg/plugins/domains/reverse_whois.gopkg/plugins/domains/reverse_whois_test.gopkg/plugins/domains/whoxy_reverse_whois.gopkg/plugins/domains/whoxy_reverse_whois_test.gopkg/runner/api.gopkg/runner/api_test.go
…ain) ViewDNS /reversewhois returns domains under response.matches[].domain, but the parser read query.domains[].domain_name — a shape the API never emits — so every real HTTP 200 yielded zero findings. The mocks reproduced the wrong shape, so tests passed vacuously (test-theater). Fix the struct + consume loop and rewrite the mocks to the real contract. Confirmed by Codex P2 + adversarial review against ViewDNS docs and the electrologue/viewdns Go consumer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
invoke.go embedded input.Value in the pipeline error; for whois+email that is a registrant email, leaking PII if the caller logs the error. Interpolate input.Type instead — consistent for all seed types, preserves the which-type context, drops the raw value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When byEmail is set, the stopping-pagination warning logged the raw email
seed. Log a non-PII query_type ("org"/"email") instead. The actual API
request still uses the real seed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F4: assert decoded email= query param value, not just its presence.
E1: credential-map test expected 15 keys but the map has 17 — add the
missing whoxy_api_key + builtwith_api_key (both real mappings).
E2: CIDR-confidence test asserted capmodel.Preseed but a FindingCIDR emits
capmodel.Asset, panicking the package. Assert Asset. Production code
was always correct; these tests were stale/wrong, leaving the
-tags compute suite red. Now fully green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Run() embedded the raw query in its request-failed and parse-response errors; in email-mode that query is a registrant email, which the runner then logs (Phase-0 plugin errors are logged). Mirror the whoxy plugin and make both errors generic. Closes the symmetric leak F2 left open in the viewdns path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problem
"Run Discovery" jobs failed with
unsupported preseed type "whois+email". The WHOIS plugin emitswhois+emailpreseeds (registrant emails), butpius-discoveryrejected them atMatch()— so any org whose WHOIS exposed a registrant email produced doomed jobs and red UI failures (the symptom that surfaced on a Zurich Global discovery run).Fix
Route the email seed to a NEW
input.Emailfield (notinput.OrgName) so org-name plugins self-disable and only the two reverse-whois plugins act on it:q=<email>(same/reversewhois/endpoint)&email=<email>mode (vs&name=)runner.Config.Email→plugins.Input.EmailwireInvokeswitches on preseedType;Matchallow-listswhois+emailRouting email to
input.Email(rather thanOrgName) is deliberate: org-name plugins (GLEIF/Censys/Wikidata/EDGAR/…) key offOrgNameand self-disable when it is empty, so they never query an email as a company name. When no reverse-whois API key is present, an email seed now yields a clean zero-result run instead of a hard failure.Post-review fixes (adversarial bot + agent review)
A second adversarial review pass (Codex P2 + CodeRabbit + internal lead/reviewer) surfaced real issues that are now fixed in this PR:
query.domains[].domain_name, but the real ViewDNS/reversewhoisAPI returnsresponse.matches[].domain(confirmed against the vendor docs and theelectrologue/viewdnsGo consumer). The old mocks reproduced the wrong shape, so tests passed vacuously. Fixed the struct + consume loop and rewrote the mocks to the real contract — ViewDNS email-mode now actually produces findings. (Pre-existing bug; also affected org-mode.)fab92adb1b3cbe); the Whoxy pagination warning logsquery_typenot the raw query (8676f80); the ViewDNS plugin's request/parse errors are now generic, mirroring Whoxy (1507fc6).-tags computesuite is now fully green (54101ac): tightened the Whoxy email-mode assertion to check the decodedemail=value; fixed the stale credential-map count (15→17, the map already hadwhoxy_api_key+builtwith_api_key); fixed the CIDR-confidence test that assertedcapmodel.Preseedfor aFindingCIDR(which emitscapmodel.Asset) and panicked the package.Testing
go build ./...andgo build -tags compute ./...both succeed.go test ./...(plain) green;go test -tags compute ./pkg/libnow fully green — the two previously-failing compute tests (TestPiusCredentialMapping_CoversAllPlugins,TestInvoke_CIDRWithConfidence) were stale/wrong tests and are repaired in54101ac.TestDiscovery_Parameters) — no new UI params.Deferred (out of scope, documented)
abuse@/hostmaster@, registrar/reseller mailboxes) can reverse-whois into unrelated portfolios. Degrades to clean-zero; no skip-list added yet.Name <addr>display-name form from the producer —url.QueryEscapekeeps it safe; yields zero matches, not an error.🤖 Generated with Claude Code