filter answer section only

[dogancanbakir]

closes #923

Summary by CodeRabbit

• Tests

  • Added an integration test validating DNS responses that place answers in Authority/Additional sections to ensure correct filtering and parsing.

• Chores

  • Updated a DNS-related dependency to a newer version.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds an integration test that verifies DNS additional-section records are not treated as answer A records; also bumps a retryabledns dependency version in go.mod.

Changes

Cohort / File(s)	Summary
DNS Additional Section Integration Test cmd/integration-test/dns.go	Adds a new test case "DNS Filter Additional Section", implements dnsFilterAdditionalSection.Execute() which launches an auxiliary UDP DNS server and invokes dnsx -a -resp -json, and adds dnshandlerWithAdditional.ServeDNS() to craft responses with empty Answer, NS in Authority, and A in Additional sections to validate parsing.
Dependency Update go.mod	Updates github.com/projectdiscovery/retryabledns to v1.0.114-0.20260321013547-428c4a196bd2 (direct dependency version bump).

Sequence Diagram(s)

sequenceDiagram
    participant Test as Integration Test
    participant AuxDNS as Auxiliary UDP DNS Server (127.0.0.1:15001)
    participant Dnsx as dnsx CLI process
    participant Parser as JSON result parser

    Test->>AuxDNS: start dnshandlerWithAdditional
    Test->>Dnsx: run dnsx querying AuxDNS (-a -resp -json)
    Dnsx->>AuxDNS: UDP DNS query for invalid domain
    AuxDNS-->>Dnsx: DNS response (Answer: empty, Authority: NS, Additional: A records)
    Dnsx->>Parser: emit JSON results including fields (a, resp, etc.)
    Parser-->>Test: check that `a` does NOT contain Additional-section IPs

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped a test across the DNS stream,
Empty answers, authorities gleam,
Additionals hide their A-record glow,
dnsx reads true — not what they show,
A tiny hop to keep outputs clean. 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Out of Scope Changes check	❓ Inconclusive	The PR adds integration test code and updates a retryabledns dependency, but lacks visible implementation changes to the actual filtering logic required to fix the reported bug.	Add implementation details showing how the core DNS parsing logic now filters to ANSWER section only, and clarify the purpose of the retryabledns version bump.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'filter answer section only' directly summarizes the main objective to filter DNS response parsing to the ANSWER section, matching the PR's core intent.
Linked Issues check	✅ Passed	The PR introduces an integration test that validates filtering of DNS ANSWER sections, detecting when non-ANSWER records incorrectly appear in parsed output, directly addressing issue #923.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 923_filter_answer_section_only

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

cmd/integration-test/dns.go (2)

11-15: Integration test correctly reproduces the ADDITIONAL-section bug scenario

The new "DNS Filter Additional Section" testcase and dnsFilterAdditionalSection.Execute reliably simulate a response with only NS/ADDITIONAL A records and assert that root-server IPs are not present in dnsx output. This is a good end-to-end guard for the regression.

One optional enhancement: if the intended behavior is “no answers at all” for such a query, you could additionally assert len(results) == 0 to make that expectation explicit.

Also applies to: 138-175

---

138-141: Unused expectedOutput field in dnsFilterAdditionalSection

expectedOutput is defined on dnsFilterAdditionalSection but never used in Execute. Since the test only checks for the absence of specific IPs, this field can be safely removed to reduce noise, unless you plan to extend the assertions later.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 31932fd and 05b1645.

📒 Files selected for processing (4)

• cmd/integration-test/dns.go (2 hunks)
• internal/runner/runner.go (1 hunks)
• libs/dnsx/util.go (2 hunks)
• libs/dnsx/util_test.go (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

internal/runner/runner.go (1)

libs/dnsx/util.go (1)

• FilterAnswerSectionOnly (45-128)

libs/dnsx/util_test.go (1)

libs/dnsx/util.go (1)

• FilterAnswerSectionOnly (45-128)

cmd/integration-test/dns.go (1)

internal/testutils/integration.go (1)

• RunDnsxAndGetResults (11-36)

🪛 GitHub Check: Lint Test

cmd/integration-test/dns.go

[failure] 225-225:
Error return value of w.WriteMsg is not checked (errcheck)

---

[failure] 153-153:
Error return value of srv.Shutdown is not checked (errcheck)

---

[failure] 152-152:
Error return value of srv.ListenAndServe is not checked (errcheck)

⏰ 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)

• GitHub Check: release-test
• GitHub Check: Functional Test (macOS-latest)
• GitHub Check: Functional Test (windows-latest)
• GitHub Check: Analyze (go)

🔇 Additional comments (3)

internal/runner/runner.go (1)

635-643: Good placement of FilterAnswerSectionOnly in the worker pipeline

Calling dnsx.FilterAnswerSectionOnly(dnsData.DNSData) right after QueryMultiple and the nil check ensures all subsequent consumers (JSON output, record-type printing, wildcard storage, ASN/CDN lookups) operate on ANSWER-only data. The added nil guard inside the helper keeps this safe.

libs/dnsx/util.go (1)

43-127: ANSWER-only filtering logic looks correct and robust

The implementation cleanly rebuilds all record slices from RawResp.Answer only, with appropriate lowercasing and SOA struct preservation, and safely handles nil and empty-ANSWER cases. This matches the objective of excluding AUTHORITY/ADDITIONAL data from downstream use.

libs/dnsx/util_test.go (1)

11-137: Unit test coverage for FilterAnswerSectionOnly is solid

The table-driven tests cover the key edge cases (ADDITIONAL-only A records, mixed ANSWER/ADDITIONAL, CNAME-only ANSWER, nil RawResp, nil DNSData) and align with the intended semantics of using ANSWER-only data.

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

cmd/integration-test/dns.go (1)

153-154: //nolint usage resolves errcheck; consider narrowing to //nolint:errcheck

Adding //nolint to srv.ListenAndServe(), srv.Shutdown(), and w.WriteMsg(&msg) addresses the previous errcheck lint failures.

If your project convention prefers targeting only the specific linter (as suggested in the earlier review), you may want to switch these to //nolint:errcheck so you don’t accidentally suppress other linters on the same lines:

-go srv.ListenAndServe() //nolint
-defer srv.Shutdown()    //nolint
+go srv.ListenAndServe() //nolint:errcheck
+defer srv.Shutdown()    //nolint:errcheck

- w.WriteMsg(&msg) //nolint
+ w.WriteMsg(&msg) //nolint:errcheck

Purely stylistic/precision; functionally things are fine as-is.

Also applies to: 236-236

🧹 Nitpick comments (3)

cmd/integration-test/dns.go (3)

4-4: Strengthen the JSON-based assertion in the new “DNS Filter Additional Section” test

The overall structure (local DNS server with only NS/Additional A records, running dnsx with -a -resp -json, and scanning JSON for root IPs) matches the bug report and should catch the original regression.

Two small robustness gaps you might want to tighten:

• The test passes when len(results) == 0 or when json.Unmarshal fails (you continue), so if dnsx stops emitting JSON or changes its output format, this test will silently succeed instead of flagging a failure.
• Given the scenario (no A answers at all), it would be stronger to assert that the "a" field is either absent or empty, not just that it does not contain those three specific root IPs.

A possible refinement would be to:

• Fail if no JSON line is successfully parsed, and
• When "a" is present, additionally assert len(aField) == 0.

That keeps the intent clear and makes the test more future-proof while still validating that ADDITIONAL-section IPs never leak into "a".

Also applies to: 13-16, 144-186

---

172-180: Avoid hard type assertion in JSON parsing loop

ipStr := strings.ToLower(ip.(string)) will panic if the "a" field ever stops being an array of strings (schema change, bug, or malformed output). That would bring down the whole integration test run with a panic instead of a clean assertion failure.

You could guard this cheaply:

- for _, ip := range aField {
-   ipStr := strings.ToLower(ip.(string))
+ for _, ip := range aField {
+   s, ok := ip.(string)
+   if !ok {
+     return errIncorrectResult("expected string IP in 'a' field", result)
+   }
+   ipStr := strings.ToLower(s)

This keeps the test strict about schema while failing in a controlled way instead of panicking.

---

188-237: Handler for NS/Additional records looks correct; consider a small defensive check

The new dnshandlerWithAdditional correctly:

• Validates the question name before responding.
• Returns an authoritative reply with an empty Answer, NS records in msg.Ns, and root A records in msg.Extra.
• Uses w.WriteMsg(&msg) to emit the crafted response.

That matches the scenario needed to reproduce the bug.

If you want to make this a bit more defensive without changing behavior, you could add a quick guard before accessing r.Question[0]:

if len(r.Question) == 0 {
    return
}

Not required for the current tests (which always send a normal query), but it makes the handler safer if it’s ever reused.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 532c626 and 398afb7.

📒 Files selected for processing (2)

• cmd/integration-test/dns.go (3 hunks)
• libs/dnsx/util_test.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• libs/dnsx/util_test.go

🧰 Additional context used

🧬 Code graph analysis (1)

cmd/integration-test/dns.go (1)

internal/testutils/integration.go (1)

• RunDnsxAndGetResults (11-36)

[nil0x42]

IMHO It doesn't fix the issue, please take a look at my review:
#923 (comment)

The filter should really parse from raw response & filter data from ANSWER SECTION only (at least for A records). Only records mentionned in the SECTION should appear in the list of found A records

[Mzack9999]

Let's review the reported edge cases at #923 (comment)

[neo-by-projectdiscovery-dev]

Neo - PR Security Review

No security issues found

Highlights

• Adds GitHub workflow automation for auto-merge, compatibility checks, and stale issue management
• Introduces new integration test to verify DNS answer section filtering (prevents AUTHORITY/ADDITIONAL section records from being incorrectly parsed as answers)
• Updates retryabledns dependency to v1.0.114-0.20260321013547-428c4a196bd2

_{Comment @pdneo help for available commands. · Open in Neo}