Fix DNS resolution failure when toggling exit nodes #82
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughWorkflows: testflight triggers and gate logic revised to remove comment-based triggers and derive upload/version from tag refs or a Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Author
|
/testflight netbird-ref=add-default-resolver version=0.67.5 |
|
TestFlight build failed for |
|
TestFlight build failed for |
|
TestFlight build uploaded for The build has been submitted to App Store Connect and will be available in TestFlight shortly. |
Contributor
Author
|
/testflight netbird-ref=add-default-resolver |
|
TestFlight build failed for |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@NetbirdKit/NetworkChangeListener.swift`:
- Around line 24-28: The code currently logs the full routesString in
NetworkChangeListener (onNetworkChanged), which can expose private CIDRs;
instead parse routesString to produce only aggregate metadata (e.g., number of
routes and whether a default route like 0.0.0.0/0 or ::/0 is present) and
replace the direct routesString log with a sanitized message such as
"onNetworkChanged: routesCount=\(count), hasDefault=\(true/false)"; update the
AppLogger.shared.log call(s) in the onNetworkChanged handling to emit only these
aggregates and never the raw routesString.
- Around line 30-32: Guard against applying empty routes before interface IP is
initialized: after calling parseRoutesToNESettings and logging, only call
self.tunnelManager.setRoutes(v4Routes:v4Routes, v6Routes:v6Routes,
containsDefault:containsDefault) when at least one of v4Routes or v6Routes is
non-empty or when the tunnel manager reports the interface IP is already set; if
both route lists are empty and the interface IP isn't set, skip setRoutes and
log a warning so PacketTunnelProviderSettingsManager won't apply empty included
routes prematurely. Use the existing symbols parseRoutesToNESettings,
AppLogger.shared.log, and self.tunnelManager.setRoutes and, if available, check
a tunnelManager property or method indicating interface IP state (e.g.,
interfaceIP / hasInterfaceIP()) to decide whether to defer applying routes.
In `@NetbirdNetworkExtension/PacketTunnelProviderSettingsManager.swift`:
- Line 55: The current log in PacketTunnelProviderSettingsManager
(AppLogger.shared.log call in the setDNS logic) prints raw DNS server and search
domain values; change it to avoid logging sensitive values by replacing
config.serverIP and searchDomains with redacted or summarized information (e.g.,
"server=<redacted>" or "serverCount=\(servers.count)") and log counts/flags
instead (searchDomains.count, config.routeAll, config.domains.count). Update the
message in the AppLogger.shared.log invocation inside the setDNS path to use
masked placeholders or a small helper that returns counts/flags rather than full
values.
- Around line 34-56: The setDNS function currently forces full DNS tunneling by
unconditionally setting dnsSettings.matchDomains = [""], ignoring
HostDNSConfig.routeAll; update setDNS to honor config.routeAll: if routeAll is
true keep matchDomains = [""], otherwise build matchDomains from config.domains
(include domains that are not disabled and include match-only domains
appropriately, e.g., add domain.domain entries for matchOnly and non-matchOnly
where split-DNS is desired) and only set dnsSettings.matchDomains when
non-empty; also update the AppLogger.shared.log call to reflect the actual
matchDomains used and ensure dnsSettings.searchDomains logic remains unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4f0cbaba-031b-43a1-a201-0dff7872ba16
📒 Files selected for processing (5)
.github/workflows/build-upload.yml.github/workflows/testflight.ymlNetBird/Source/App/Views/Components/RouteCard.swiftNetbirdKit/NetworkChangeListener.swiftNetbirdNetworkExtension/PacketTunnelProviderSettingsManager.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/testflight.yml
NetbirdNetworkExtension/PacketTunnelProviderSettingsManager.swift
Outdated
Show resolved
Hide resolved
|
TestFlight build uploaded for The build has been submitted to App Store Connect and will be available in TestFlight shortly. |
mlsmaycon
changed the title
|
TestFlight build uploaded for The build has been submitted to App Store Connect and will be available in TestFlight shortly. |
|
TestFlight build failed for |
Contributor
Author
|
/testflight netbird-ref=add-default-resolver |
|
TestFlight build uploaded for The build has been submitted to App Store Connect and will be available in TestFlight shortly. |
lixmal
approved these changes
Apr 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tunnel settings that only matched specific domains
stale 0.0.0.0/0 in the tunnel and black-holing all traffic
Test plan
Summary by CodeRabbit