Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds IPv6 support across the NetBird iOS client: new optional ChangesIPv6 Support Across Configuration, Network Extension, and UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
TestFlight build failed for |
|
TestFlight build failed for |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
TestFlight build failed for |
lixmal
force-pushed
the
feature/ipv6-support
branch
3 times, most recently
from
0fc19cc to
192ea6d
Compare
|
Build failed (iOS, tvOS) for |
lixmal
force-pushed
the
feature/ipv6-support
branch
from
192ea6d to
3421c8b
Compare
|
Build failed (iOS, tvOS) for |
…nnel configuration
lixmal
force-pushed
the
feature/ipv6-support
branch
from
3421c8b to
7e15c12
Compare
|
Build failed (tvOS) for |
# Conflicts: # NetbirdNetworkExtension/PacketTunnelProviderSettingsManager.swift # netbird-core
|
/testflight |
|
Build failed (iOS) |
|
TestFlight builds uploaded |
|
/testflight |
|
Build failed (iOS, tvOS) |
|
/testflight |
|
Build failed (iOS, tvOS) |
|
/testflight |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
NetbirdKit/NetworkChangeListener.swift (1)
111-128:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPre-existing IPv6 regex only matches fully-expanded 8-group addresses — all compressed forms will be silently dropped
The regex at line 113:
^([0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(\/\d{1,3})?$does not match any compressed IPv6 notation (e.g.
::1,fe80::1,::/0,fd00::/8,2001:db8::1/64). Routes sent by the Go SDK in compressed form will be classified as.invalidand silently discarded, leavingipv6Routespermanently empty. This was a dormant bug before this PR, but now it directly breaks the IPv6 routing functionality being added.A minimal fix is to replace the string-based regex with a presence heuristic or use
inet_pton:🐛 Proposed fix — replace the broken IPv6 regex
func detectIPAddressType(_ address: String) -> IPAddressType { let ipv4Pattern = "^(\\d{1,3}\\.){3}\\d{1,3}(\\/\\d{1,2})?$" - let ipv6Pattern = "^([0-9a-fA-F]{1,4}:){7}[0-9a-fA-F]{1,4}(\\/\\d{1,3})?$" let ipv4Regex = try! NSRegularExpression(pattern: ipv4Pattern, options: []) - let ipv6Regex = try! NSRegularExpression(pattern: ipv6Pattern, options: []) let ipv4Matches = ipv4Regex.numberOfMatches(in: address, options: [], range: NSRange(location: 0, length: address.utf16.count)) - let ipv6Matches = ipv6Regex.numberOfMatches(in: address, options: [], range: NSRange(location: 0, length: address.utf16.count)) if ipv4Matches > 0 { return .ipv4 - } else if ipv6Matches > 0 { + } else if address.contains(":") { + // IPv6 addresses always contain ":" — handles full, compressed, and CIDR forms return .ipv6 } else { return .invalid } }🤖 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 `@NetbirdKit/NetworkChangeListener.swift` around lines 111 - 128, detectIPAddressType currently uses a strict IPv6 regex that only matches fully-expanded 8-group addresses and thus misclassifies valid compressed IPv6 forms as .invalid; replace the regex approach in detectIPAddressType with a robust validation that strips any CIDR suffix (split on "/") and uses inet_pton (or equivalent system call) to test the bare address for AF_INET and AF_INET6, returning .ipv4, .ipv6 or .invalid accordingly; update references in the function detectIPAddressType and ensure IPAddressType enum values are used unchanged.
🤖 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 `@NetBird/Source/App/ViewModels/MainViewModel.swift`:
- Around line 695-701: The UI state (self.disableIPv6) is being set before
persistence, so if configProvider.commit() fails the UI remains wrong; change
setDisableIPv6 to only update the UI and configProvider.disableIPv6 after a
successful commit (or, if you must set the UI first, revert self.disableIPv6
back to the prior value when configProvider.commit() returns false) by calling
configProvider.commit() with the intended value and on failure restore the
previous self.disableIPv6 and ensure configProvider.disableIPv6 reflects the
persisted value; key symbols: setDisableIPv6(disabled:), self.disableIPv6,
configProvider.disableIPv6, configProvider.commit().
In `@NetbirdKit/ConfigurationProvider.swift`:
- Around line 177-180: The disableIPv6 setter currently calls updateJSONField
which aborts when the JSON key is missing, so toggling the UI will silently fail
for configs without "DisableIPv6"; change updateJSONField (used by the
disableIPv6 setter) to perform an upsert instead of update-only—remove the
existence guard that checks dict[field] != nil and always assign dict[field] =
value (and then persist the JSON), or alternatively special-case the
"DisableIPv6" key to insert when absent; keep extractJSONBool as-is for reads.
In `@NetbirdKit/NetworkChangeListener.swift`:
- Around line 47-52: The method setInterfaceIPv6 currently only forwards the
value to tunnelManager and never stores it locally nor does
parseRoutesToNESettings inject an IPv6 interface route, and the IPv6 regex only
matches full 8-group addresses; fix by adding a stored property (e.g.,
interfaceIPv6) in NetworkChangeListener and assign validIPv6 to it inside
setInterfaceIPv6(interfaceIPv6:), then update parseRoutesToNESettings to add an
interface route for IPv6 peers analogous to the IPv4 path that uses
self.interfaceIP; finally replace the strict regex at the IPv6 route-parsing
site with a more permissive IPv6/CIDR matcher (or use system parsing like
IPv6Address/inet_pton to validate CIDR/compressed forms) so compressed addresses
and prefixes (e.g., ::1, 2001:db8::/32, ::/0, fd00::/8) are accepted.
In `@NetbirdNetworkExtension/PacketTunnelProviderSettingsManager.swift`:
- Around line 137-144: The helper extractIPv6AddressAndPrefix currently parses
any integer after the slash but doesn't validate it; update
extractIPv6AddressAndPrefix(from:) to verify the parsed prefix is within 0...128
and return nil for out-of-range values (and still return nil for malformed
input). Locate the function extractIPv6AddressAndPrefix(from cidr: String) and
add a range check for the parsed prefix (Int) before returning
(String(parts[0]), prefix) so callers like NEIPv6Settings receive only valid
prefix lengths.
- Around line 103-122: The code currently always creates and assigns an
NEIPv6Settings even when v6Addresses is empty; change the logic in
PacketTunnelProviderSettingsManager so you only instantiate and assign
NEIPv6Settings (using NEIPv6Settings(addresses:networkPrefixLengths:)) when
v6Addresses.count > 0 (i.e. when interfaceIPv6 or containsDefaultRoute supplied
an address); if v6Addresses is empty leave tunnelNetworkSettings.ipv6Settings
nil (and do not set includedRoutes) to avoid applying an empty IPv6 settings
object that breaks setTunnelNetworkSettings; look for symbols interfaceIPv6,
containsDefaultRoute, v6Addresses, NEIPv6Settings and tunnelNetworkSettings in
the diff to implement this guard.
---
Outside diff comments:
In `@NetbirdKit/NetworkChangeListener.swift`:
- Around line 111-128: detectIPAddressType currently uses a strict IPv6 regex
that only matches fully-expanded 8-group addresses and thus misclassifies valid
compressed IPv6 forms as .invalid; replace the regex approach in
detectIPAddressType with a robust validation that strips any CIDR suffix (split
on "/") and uses inet_pton (or equivalent system call) to test the bare address
for AF_INET and AF_INET6, returning .ipv4, .ipv6 or .invalid accordingly; update
references in the function detectIPAddressType and ensure IPAddressType enum
values are used 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1613742c-4e58-4f5b-a8ec-3e0b375a0bd9
📒 Files selected for processing (12)
NetBird/Source/App/ViewModels/MainViewModel.swiftNetBird/Source/App/Views/AdvancedView.swiftNetBird/Source/App/Views/Components/PeerCard.swiftNetBird/Source/App/Views/Components/PeerDetailSheet.swiftNetBird/Source/App/Views/TV/TVSettingsView.swiftNetBirdTVNetworkExtension/PacketTunnelProvider.swiftNetbirdKit/ConfigurationProvider.swiftNetbirdKit/NetworkChangeListener.swiftNetbirdKit/StatusDetails.swiftNetbirdNetworkExtension/PacketTunnelProvider.swiftNetbirdNetworkExtension/PacketTunnelProviderSettingsManager.swiftnetbird-core
|
TestFlight builds uploaded |
|
/testflight |
|
TestFlight builds uploaded |
1 participant
Describe your changes
Add IPv6 support to the iOS and tvOS client UI, complementing the Go-side changes in netbirdio/netbird#5738.
Checklist
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores