make IPv6 default #1600
make IPv6 default #1600
Conversation
WalkthroughDefaults now enable both IPv4 and IPv6 by default; DNS AAAA lookups use the IPv6 constant; direct IP target handling correctly classifies IPv4 vs IPv6; tests and README updated to reflect the new default behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
🧹 Nitpick comments (1)
pkg/runner/runner.go (1)
101-104: Consider usingscan.IPv6constant for consistency.Line 102 uses the string literal
"6"whileisDefaultIPVersionon line 101 usesscan.IPv4. For consistency and maintainability, consider usingscan.IPv6instead.isDefaultIPVersion := len(options.IPVersion) == 1 && sliceutil.Contains(options.IPVersion, scan.IPv4) -if sliceutil.Contains(options.IPVersion, "6") || isDefaultIPVersion { +if sliceutil.Contains(options.IPVersion, scan.IPv6) || isDefaultIPVersion { dnsOptions.QuestionTypes = append(dnsOptions.QuestionTypes, dns.TypeAAAA) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pkg/runner/runner.go(1 hunks)pkg/runner/targets.go(2 hunks)pkg/runner/util.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
pkg/runner/targets.go (2)
pkg/routing/router.go (2)
IPv4(16-16)IPv6(17-17)pkg/scan/scan_common.go (2)
IPv4(14-14)IPv6(15-15)
pkg/runner/util.go (2)
pkg/routing/router.go (2)
IPv4(16-16)IPv6(17-17)pkg/scan/scan_common.go (2)
IPv4(14-14)IPv6(15-15)
⏰ 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). (7)
- GitHub Check: build-windows
- GitHub Check: build-linux
- GitHub Check: build-mac
- GitHub Check: release-test-mac
- GitHub Check: release-test-windows
- GitHub Check: Functional Test (ubuntu-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
pkg/runner/util.go (2)
23-40: Logic for default IP version detection looks correct.The conditional structure properly separates explicit user configuration from the default IPv4-only case, enabling auto-detection of IPv6 when the user hasn't explicitly specified IP versions. The approach of fetching both A and AAAA records in the default case aligns with the PR objective.
45-50: Clean separation of IPv4 and IPv6 handling for direct IP inputs.The explicit branching for IPv4 vs IPv6 targets improves clarity and correctly populates the respective return slices.
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @pkg/runner/targets.go:
- Around line 156-157: isDefaultIPVersion reads r.options.IPVersion without
synchronization while enableIPv6 writes it under enableIPv6Once, causing a data
race; fix by protecting access to r.options.IPVersion with a shared lock: add a
sync.RWMutex (e.g., r.optionsMu) and wrap writes to r.options.IPVersion in
enableIPv6 with r.optionsMu.Lock()/Unlock(), and wrap reads in
isDefaultIPVersion with r.optionsMu.RLock()/RUnlock() so both read and write use
the same synchronization.
🧹 Nitpick comments (1)
pkg/runner/runner.go (1)
103-105: Consider deduplicating theisDefaultIPVersionlogic.The
isDefaultIPVersioncheck is defined inline here and presumably also exists as a method inutil.go(referenced intargets.go:156). Consider using the helper method consistently to avoid divergence:-isDefaultIPVersion := len(options.IPVersion) == 1 && sliceutil.Contains(options.IPVersion, scan.IPv4) -if sliceutil.Contains(options.IPVersion, scan.IPv6) || isDefaultIPVersion { +if sliceutil.Contains(options.IPVersion, scan.IPv6) || runner.isDefaultIPVersion() { dnsOptions.QuestionTypes = append(dnsOptions.QuestionTypes, dns.TypeAAAA) }Note: This change enables AAAA queries (and thus auto-detection) for any IPv4-only configuration, including when the user explicitly specifies
-iv 4. If you want to distinguish between implicit default (enable auto-detection) and explicit-iv 4(disable auto-detection), you'll need additional logic to track whether the user explicitly set the flag.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pkg/runner/runner.gopkg/runner/targets.gopkg/runner/util.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/runner/util.go
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/runner/runner.go (2)
pkg/routing/router.go (2)
IPv4(16-16)IPv6(17-17)pkg/scan/scan_common.go (2)
IPv4(14-14)IPv6(15-15)
⏰ 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). (7)
- GitHub Check: build-windows
- GitHub Check: build-mac
- GitHub Check: build-linux
- GitHub Check: release-test-windows
- GitHub Check: release-test-mac
- GitHub Check: Functional Test (ubuntu-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
pkg/runner/runner.go (1)
59-59: LGTM! Thread-safety field added.The
enableIPv6Oncefield correctly usessync.Onceto coordinate one-time IPv6 enabling across concurrent goroutines. This resolves the data race concern identified in the previous review.
Mzack9999
left a comment
Mzack9999
left a comment
There was a problem hiding this comment.
After comparing different scan engines (nmap, masscan, rustscan), naabu’s behavior does not appear surprising or incorrect. Like other scanners, it requires an explicit flag to enable the IPv6 stack and operates in single-stack mode by default. In contrast, naabu already supports dual-stack scanning within a single execution when explicitly enabled.
Given this, it’s unclear how we should change the current behavior. Simply specifying both IP versions on the command line is sufficient to enable dual-stack scanning today. Introducing forced auto-detection would mean always considering both stacks, which could be undesirable in some environments.
My suggestion would be either:
- recommend users explicitly enable dual-stack scanning when needed (since it already works correctly), or
- consider making dual-stack the default behavior, if we agree that’s the better path
dogancanbakir
force-pushed
the
1255_auto_ip_version_detection
branch
2 times, most recently
from
4da8c1d to
9d3a6e1
Compare
Instead of auto-detecting IPv6, simply enable both IP versions by default. Users can still explicitly specify -iv 4 or -iv 6 to scan only one version. - Change default IPVersion to include both "4" and "6" - Fix AAAA query check to use scan.IPv6 constant - Fix host2ips to properly categorize direct IP inputs - Add IPv6 addresses to fallback branch in host2ips - Update README to reflect new default behavior closes #1255 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
dogancanbakir
force-pushed
the
1255_auto_ip_version_detection
branch
from
9d3a6e1 to
cbccbf3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/runner/util.go (1)
38-43: LGTM! Direct IP targets correctly classified.The logic properly differentiates IPv4 and IPv6 direct IP inputs, appending them to the appropriate target slices. The debug logging provides useful feedback for direct IP targets.
Optional: Add debug logging for domain resolution
Consider adding a similar debug log after line 33 for domain resolution to provide consistent visibility across both code paths:
targetIPsV4 = append(targetIPsV4, dnsData.A...) targetIPsV6 = append(targetIPsV6, dnsData.AAAA...) + gologger.Debug().Msgf("Found %d IPv4 and %d IPv6 addresses for %s\n", len(targetIPsV4), len(targetIPsV6), target) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.mdpkg/runner/options.gopkg/runner/runner.gopkg/runner/runner_test.gopkg/runner/util.go
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- pkg/runner/runner.go
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/runner/options.go (1)
pkg/scan/scan_common.go (2)
IPv4(14-14)IPv6(15-15)
⏰ 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). (6)
- GitHub Check: build-linux
- GitHub Check: build-windows
- GitHub Check: build-mac
- GitHub Check: release-test-windows
- GitHub Check: Functional Test (ubuntu-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (3)
pkg/runner/options.go (1)
184-184: LGTM! IPv4 and IPv6 enabled by default.The default IPVersion now includes both IPv4 and IPv6, enabling dual-stack scanning by default. This aligns with the PR objective of automatic IP version detection. Users who want IPv4-only scanning must now explicitly specify
-iv 4.pkg/runner/runner_test.go (1)
48-50: LGTM! Test correctly validates the new default.The expected default IPVersion is updated to match the new dual-stack default behavior.
pkg/runner/util.go (1)
32-32: LGTM! Defensive fallback for unset IPVersion.This fallback ensures both IPv4 and IPv6 addresses are collected when IPVersion is not set, matching the behavior of the new
["4","6"]default.
Mzack9999
added
the
Type: Enhancement
closes #1255
Summary by CodeRabbit
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.