Skip to content

Add IPv6 overlay settings and peer display#594

Open
lixmal wants to merge 4 commits intomainfrom
dashboard-ipv6-setting
Open

Add IPv6 overlay settings and peer display#594
lixmal wants to merge 4 commits intomainfrom
dashboard-ipv6-setting

Conversation

@lixmal
Copy link
Copy Markdown
Contributor

@lixmal lixmal commented Mar 25, 2026

Describe your changes

Dashboard support for the IPv6 overlay feature. Depends on management PR netbirdio/netbird#5698.

  • IPv6 network range input with /48-/112 CIDR validation
  • Per-group IPv6 enabled selector (replaces simple toggle)
  • Peer IPv6 display: table cell, tooltip, detail page, selectors, DeviceCard
  • Routing peers tab shows IPv6 when available
image image image

Issue ticket number and link

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor

Summary by CodeRabbit

  • New Features
    • Edit and validate IPv6 addresses for peers via a modal.
    • Configure IPv6 network settings: CIDR input with validation and per-group enable/disable.
    • Option to select IP version for remote SSH sessions (affects host selection and connection).
  • UX / Display
    • IPv6 shown alongside IPv4 across peer lists, selectors, cards, tooltips, and copy-to-clipboard.
  • Search
    • Peer search and filters now match IPv6 addresses.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 25, 2026

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

Walkthrough

Adds IPv6 support across the app: optional ipv6 on Peer, account IPv6 settings, UI to view/edit peer IPv6 (modal + validation), search/display updates to include IPv6, provider/API payload changes, network settings for IPv6 CIDR and per-group enablement.

Changes

Cohort / File(s) Summary
Peer Edit UI
src/app/(dashboard)/peer/page.tsx
Adds EditIPv6Modal and showEditIPv6Modal state, IPv6 validation (uses ip-cidr + ":" check), disables Save when empty/unchanged/invalid, calls update({ ipv6 }) and SWR-mutates /peers/:id on success; conditional UI based on permission.peers.update.
Context / API
src/contexts/PeerProvider.tsx
PeerProvider.update() accepts optional ipv6?: string; peerRequest.put payload includes ipv6.
Models / Interfaces
src/interfaces/Peer.ts, src/interfaces/Account.ts
Peer gains optional ipv6?: string; Account.settings.extra adds ipv6_enabled_groups?: string[] and network_range_v6?: string.
Selectors / Dropdowns
src/components/PeerSelector.tsx, src/components/PeerGroupSelector.tsx
Search predicates extended to match ipv6; displayed labels append , {ipv6} when present.
Peer Display & Tooltip
src/modules/peers/PeerAddressCell.tsx, src/modules/peers/PeerAddressTooltipContent.tsx
Address cell and tooltip include peer.ipv6 when present and add copy-to-clipboard entry for IPv6.
Device & Routing
src/components/DeviceCard.tsx, src/modules/networks/routing-peers/NetworkRoutingPeersTabContent.tsx
Device description fallback may include device.ipv6; routing peers search string includes peer.ipv6.
Network Settings
src/modules/settings/NetworkSettingsTab.tsx
Adds IPv6 CIDR input, ipv6EnabledGroups selector, CIDR validation, change detection, conditional save payload handling for network_range_v6 and ipv6_enabled_groups, and includes new fields in update tracking.
Remote-access SSH
src/app/(remote-access)/peer/ssh/page.tsx, src/modules/remote-access/ssh/useSSH.ts, src/modules/remote-access/ssh/useSSHQueryParams.ts, src/modules/remote-access/useNetBirdClient.ts
Propagates optional ipVersion through query params, SSHTerminal props, SSH connection creation, and client API (createSSHConnection(host,port,user,jwt?,ipVersion?)).
Misc / Formatting
src/utils/version.ts
Minor formatting (blank line).

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant PeerPage as Peer Page UI
    participant PeerProvider as PeerProvider (context)
    participant API as peerRequest.put
    participant SWR as SWR cache

    rect rgba(0,128,0,0.5)
    User->>PeerPage: Open Edit IPv6 modal
    PeerPage->>User: Show modal with current ipv6
    end

    rect rgba(0,0,255,0.5)
    User->>PeerPage: Enter new IPv6 and Save
    PeerPage->>PeerPage: Validate using ip-cidr + ":" check
    PeerPage->>PeerProvider: call update({ ipv6: newIPv6 })
    PeerProvider->>API: PUT /peers/:id { ipv6: newIPv6 }
    API-->>PeerProvider: 200 OK
    PeerProvider->>SWR: mutate("/peers/"+peer.id)
    PeerProvider-->>PeerPage: update resolved
    PeerPage->>User: Close modal, refresh display
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • mlsmaycon
  • heisbrot

Poem

🐰 I hopped through code with whiskers twitching bright,
IPv6 tucked in, ready for flight.
Modal checks colons, CIDR in sight,
Peers now wear two addresses at night,
I saved with a nibble — networks delight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: IPv6 overlay settings (network range, enabled groups) and peer display updates (IPv6 shown across UI components).

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dashboard-ipv6-setting

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.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/contexts/PeerProvider.tsx (1)

26-33: ⚠️ Potential issue | 🟠 Major

Context type definition is missing ipv6 parameter.

The update function signature in the PeerContext type (lines 26-33) does not include ipv6?: string, but the implementation at line 83 accepts it. This type mismatch will cause TypeScript errors for consumers calling update({ ipv6: ... }).

🔧 Proposed fix
     update: (props: {
       name?: string;
       ssh?: boolean;
       loginExpiration?: boolean;
       inactivityExpiration?: boolean;
       approval_required?: boolean;
       ip?: string;
+      ipv6?: string;
     }) => Promise<Peer>;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/contexts/PeerProvider.tsx` around lines 26 - 33, The PeerContext type's
update signature is missing the optional ipv6 field, causing a mismatch with the
implementation that accepts ipv6; update the type definition for update (the
props parameter) to include ipv6?: string so callers can pass { ipv6: ... }
without TypeScript errors and the implementation and type remain consistent
(refer to the update function signature in PeerProvider.tsx / PeerContext).
🧹 Nitpick comments (1)
src/modules/settings/NetworkSettingsTab.tsx (1)

49-51: Prefer ID-based dirty-checking for IPv6 group selection.

Tracking by group names can diverge from persisted data (IDs). Use sorted ID lists for useHasChanges/updateRef so change detection matches the payload field.

♻️ Suggested refactor
-  const ipv6GroupNames = useMemo(
-    () => ipv6EnabledGroups.map((g) => g.name).sort(),
+  const ipv6GroupIds = useMemo(
+    () =>
+      ipv6EnabledGroups
+        .map((g) => g.id)
+        .filter((id): id is string => typeof id === "string" && id.length > 0)
+        .sort(),
     [ipv6EnabledGroups],
   );

   const { hasChanges, updateRef } = useHasChanges([
     customDNSDomain,
     networkRange,
     networkRangeV6,
-    ipv6GroupNames,
+    ipv6GroupIds,
   ]);
...
-          updateRef([customDNSDomain, networkRange, networkRangeV6, ipv6GroupNames]);
+          updateRef([customDNSDomain, networkRange, networkRangeV6, ipv6GroupIds]);

Also applies to: 76-81, 117-117

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/settings/NetworkSettingsTab.tsx` around lines 49 - 51, The code
currently derives ipv6GroupNames via useMemo from ipv6EnabledGroups and uses
those names for dirty-checking/updateRef; change this to compute a sorted list
of group IDs instead (e.g., map to g.id then sort) and use that ID list wherever
useHasChanges or updateRef rely on ipv6GroupNames so change detection matches
persisted payload IDs; apply the same refactor for the other occurrences that
derive IPv6 selections (the blocks around the other useMemo calls and any
updateRef/useHasChanges usage).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/app/`(dashboard)/peer/page.tsx:
- Around line 911-915: Normalize the IPv6 input once before checks: inside the
isDisabled useMemo compute a normalizedIpv6 = trim(ipv6) (and normalizedPeerIpv6
= trim(peer.ipv6) if peer value may contain whitespace) and then use
normalizedIpv6 for the length check and pass normalizedIpv6 to isValidIPv6;
compare normalizedIpv6 to normalizedPeerIpv6 for equality so Save is disabled
for no-op whitespace changes. Apply the same pattern to the other related blocks
handling ipv6 (the validation/save logic around the same peer IPv6 fields) so
all checks consistently use the trimmed value.
- Around line 499-506: The PeerContext update type is missing the ipv6 property
used by the UI; update the PeerContext declaration for the update(props)
parameter (the type defined around PeerContext and its update method) to include
ipv6?: string so calls like update({ ipv6: newIPv6 }) compile; ensure the same
optional ipv6?: string is added to the props parameter type referenced by the
update function (the implementation used around the update(...) method).

In `@src/components/DeviceCard.tsx`:
- Around line 28-37: The useMemo for descriptionText references
resource?.address but resource is missing from the dependency array, which can
cause stale values; update the dependency array for the useMemo that computes
descriptionText to include resource (so the hook depends on description,
address, device, and resource) to ensure the memo recomputes when resource
changes.

In `@src/modules/settings/NetworkSettingsTab.tsx`:
- Around line 84-87: The current construction of updatedSettings sets
ipv6_enabled_groups to ipv6EnabledGroups.map(g => g.id), which can include
undefined since Group.id is optional; filter out missing ids before assigning to
ipv6_enabled_groups so the resulting array is string[] only. Update the code
near updatedSettings / ipv6EnabledGroups to produce an array excluding
falsy/undefined ids (e.g., map then filter or flatMap to only include g.id when
present) so account.settings.ipv6_enabled_groups never contains null/undefined
entries.

---

Outside diff comments:
In `@src/contexts/PeerProvider.tsx`:
- Around line 26-33: The PeerContext type's update signature is missing the
optional ipv6 field, causing a mismatch with the implementation that accepts
ipv6; update the type definition for update (the props parameter) to include
ipv6?: string so callers can pass { ipv6: ... } without TypeScript errors and
the implementation and type remain consistent (refer to the update function
signature in PeerProvider.tsx / PeerContext).

---

Nitpick comments:
In `@src/modules/settings/NetworkSettingsTab.tsx`:
- Around line 49-51: The code currently derives ipv6GroupNames via useMemo from
ipv6EnabledGroups and uses those names for dirty-checking/updateRef; change this
to compute a sorted list of group IDs instead (e.g., map to g.id then sort) and
use that ID list wherever useHasChanges or updateRef rely on ipv6GroupNames so
change detection matches persisted payload IDs; apply the same refactor for the
other occurrences that derive IPv6 selections (the blocks around the other
useMemo calls and any updateRef/useHasChanges usage).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4f9b7d0e-dd66-4c45-85e4-56b16f9277ff

📥 Commits

Reviewing files that changed from the base of the PR and between c784676 and 1ea7efa.

📒 Files selected for processing (12)
  • src/app/(dashboard)/peer/page.tsx
  • src/components/DeviceCard.tsx
  • src/components/PeerGroupSelector.tsx
  • src/components/PeerSelector.tsx
  • src/contexts/PeerProvider.tsx
  • src/interfaces/Account.ts
  • src/interfaces/Peer.ts
  • src/modules/networks/routing-peers/NetworkRoutingPeersTabContent.tsx
  • src/modules/peers/PeerAddressCell.tsx
  • src/modules/peers/PeerAddressTooltipContent.tsx
  • src/modules/settings/NetworkSettingsTab.tsx
  • src/utils/version.ts

Comment on lines +911 to +915
const isDisabled = useMemo(() => {
if (ipv6 === peer.ipv6) return true;
const trimmed = trim(ipv6);
return trimmed.length === 0 || !isValidIPv6(trimmed);
}, [ipv6, peer.ipv6]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Normalize once and validate consistently to avoid false errors/no-op updates.

Line 914 uses trim() for button disabling, but Line 922 validates untrimmed input. This can show an error while Save is enabled and allow whitespace-only “changes”.

Suggested fix
 function EditIPv6Modal({ onSuccess, peer }: Readonly<EditIPv6ModalProps>) {
   const [ipv6, setIPv6] = useState(peer.ipv6 || "");
   const [error, setError] = useState("");
+  const normalizedIPv6 = trim(ipv6);

   const isDisabled = useMemo(() => {
-    if (ipv6 === peer.ipv6) return true;
-    const trimmed = trim(ipv6);
-    return trimmed.length === 0 || !isValidIPv6(trimmed);
-  }, [ipv6, peer.ipv6]);
+    if (normalizedIPv6 === (peer.ipv6 || "")) return true;
+    return normalizedIPv6.length === 0 || !isValidIPv6(normalizedIPv6);
+  }, [normalizedIPv6, peer.ipv6]);

   React.useEffect(() => {
     switch (true) {
-      case ipv6 === peer.ipv6:
+      case normalizedIPv6 === (peer.ipv6 || ""):
         setError("");
         break;
-      case !isValidIPv6(ipv6):
+      case !isValidIPv6(normalizedIPv6):
         setError("Please enter a valid IPv6 address, e.g., fd00:1234::1");
         break;
       default:
         setError("");
         break;
     }
-  }, [ipv6, peer.ipv6]);
+  }, [normalizedIPv6, peer.ipv6]);
 ...
-              onClick={() => onSuccess(ipv6.trim())}
+              onClick={() => onSuccess(normalizedIPv6)}

Also applies to: 917-929, 964-965

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/`(dashboard)/peer/page.tsx around lines 911 - 915, Normalize the IPv6
input once before checks: inside the isDisabled useMemo compute a normalizedIpv6
= trim(ipv6) (and normalizedPeerIpv6 = trim(peer.ipv6) if peer value may contain
whitespace) and then use normalizedIpv6 for the length check and pass
normalizedIpv6 to isValidIPv6; compare normalizedIpv6 to normalizedPeerIpv6 for
equality so Save is disabled for no-op whitespace changes. Apply the same
pattern to the other related blocks handling ipv6 (the validation/save logic
around the same peer IPv6 fields) so all checks consistently use the trimmed
value.

- IPv6 network range input with /48-/112 CIDR validation
- Per-group IPv6 enabled selector (replaces simple toggle)
- Peer IPv6 display: table cell, tooltip, detail page, selectors, search
- Gray out IPv6 for peers whose client does not support it yet
- DeviceCard and routing peers show IPv6 when available
@lixmal lixmal force-pushed the dashboard-ipv6-setting branch from 1ea7efa to 9be1fa7 Compare March 25, 2026 16:45
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
src/app/(dashboard)/peer/page.tsx (2)

499-515: ⚠️ Potential issue | 🔴 Critical

Add ipv6 to the PeerContext update type definition to fix the build.

The pipeline failure indicates that the update function's TypeScript type in PeerContext (around lines 26-33 in src/contexts/PeerProvider.tsx) does not include ipv6?: string, even though the implementation at lines 76-84 does accept it. Add ipv6?: string; to the context type definition to resolve the TypeScript error.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/`(dashboard)/peer/page.tsx around lines 499 - 515, The PeerContext
type for the update function is missing the ipv6 property; update the
PeerContext (the context type/interface that declares update) so the update
parameter type includes ipv6?: string to match the implementation in
PeerProvider's update function, ensuring callers like the EditIPv6Modal usage
compile without errors.

911-929: ⚠️ Potential issue | 🟡 Minor

Normalize IPv6 input once to avoid validation inconsistencies.

The code compares untrimmed ipv6 with peer.ipv6 for equality checks (lines 912, 919) but uses trimmed values for validation (lines 914, 922). This can allow the Save button to be enabled for whitespace-only changes that are effectively no-ops.

🔧 Proposed fix
 function EditIPv6Modal({ onSuccess, peer }: Readonly<EditIPv6ModalProps>) {
   const [ipv6, setIPv6] = useState(peer.ipv6 || "");
   const [error, setError] = useState("");
+  const trimmedIPv6 = useMemo(() => trim(ipv6), [ipv6]);
+  const peerIPv6 = peer.ipv6 || "";

   const isDisabled = useMemo(() => {
-    if (ipv6 === peer.ipv6) return true;
-    const trimmed = trim(ipv6);
-    return trimmed.length === 0 || !isValidIPv6(trimmed);
-  }, [ipv6, peer.ipv6]);
+    if (trimmedIPv6 === peerIPv6) return true;
+    return trimmedIPv6.length === 0 || !isValidIPv6(trimmedIPv6);
+  }, [trimmedIPv6, peerIPv6]);

   React.useEffect(() => {
     switch (true) {
-      case ipv6 === peer.ipv6:
+      case trimmedIPv6 === peerIPv6:
         setError("");
         break;
-      case !isValidIPv6(trim(ipv6)):
+      case !isValidIPv6(trimmedIPv6):
         setError("Please enter a valid IPv6 address, e.g., fd00:1234::1");
         break;
       default:
         setError("");
         break;
     }
-  }, [ipv6, peer.ipv6]);
+  }, [trimmedIPv6, peerIPv6]);

And update the onClick handler:

-              onClick={() => onSuccess(ipv6.trim())}
+              onClick={() => onSuccess(trimmedIPv6)}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/`(dashboard)/peer/page.tsx around lines 911 - 929, Normalize the IPv6
input once (e.g., const normalized = trim(ipv6)) and use that single normalized
value everywhere: replace direct uses of ipv6 in the isDisabled useMemo and the
React.useEffect switch with normalized, use isValidIPv6(normalized) for
validation, compare normalized === peer.ipv6 for equality, update dependencies
to include normalized instead of raw ipv6, and update the Save onClick handler
to compare/send normalized to avoid whitespace-only changes enabling the button
or being submitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/app/`(dashboard)/peer/page.tsx:
- Around line 499-515: The PeerContext type for the update function is missing
the ipv6 property; update the PeerContext (the context type/interface that
declares update) so the update parameter type includes ipv6?: string to match
the implementation in PeerProvider's update function, ensuring callers like the
EditIPv6Modal usage compile without errors.
- Around line 911-929: Normalize the IPv6 input once (e.g., const normalized =
trim(ipv6)) and use that single normalized value everywhere: replace direct uses
of ipv6 in the isDisabled useMemo and the React.useEffect switch with
normalized, use isValidIPv6(normalized) for validation, compare normalized ===
peer.ipv6 for equality, update dependencies to include normalized instead of raw
ipv6, and update the Save onClick handler to compare/send normalized to avoid
whitespace-only changes enabling the button or being submitted.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 085d848f-07cd-4c56-9ba4-271a59da1730

📥 Commits

Reviewing files that changed from the base of the PR and between 1ea7efa and 9be1fa7.

📒 Files selected for processing (12)
  • src/app/(dashboard)/peer/page.tsx
  • src/components/DeviceCard.tsx
  • src/components/PeerGroupSelector.tsx
  • src/components/PeerSelector.tsx
  • src/contexts/PeerProvider.tsx
  • src/interfaces/Account.ts
  • src/interfaces/Peer.ts
  • src/modules/networks/routing-peers/NetworkRoutingPeersTabContent.tsx
  • src/modules/peers/PeerAddressCell.tsx
  • src/modules/peers/PeerAddressTooltipContent.tsx
  • src/modules/settings/NetworkSettingsTab.tsx
  • src/utils/version.ts
✅ Files skipped from review due to trivial changes (4)
  • src/utils/version.ts
  • src/modules/peers/PeerAddressCell.tsx
  • src/interfaces/Peer.ts
  • src/interfaces/Account.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/modules/networks/routing-peers/NetworkRoutingPeersTabContent.tsx
  • src/components/PeerGroupSelector.tsx
  • src/modules/peers/PeerAddressTooltipContent.tsx
  • src/components/PeerSelector.tsx
  • src/contexts/PeerProvider.tsx

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/modules/settings/NetworkSettingsTab.tsx`:
- Line 121: The call to updateRef currently references a nonexistent variable
ipv6GroupNames causing build failure; replace ipv6GroupNames with the correct
ipv6GroupIds so the array matches the one passed into useHasChanges (see
useHasChanges and updateRef usages) — update the invocation
updateRef([customDNSDomain, networkRange, networkRangeV6, ipv6GroupNames]) to
use ipv6GroupIds instead and ensure any surrounding logic still references
ipv6GroupIds consistently.
🪄 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: 7987ec0d-216b-4ffa-bfed-d810c3f5a56a

📥 Commits

Reviewing files that changed from the base of the PR and between 9be1fa7 and 492bbbd.

📒 Files selected for processing (2)
  • src/contexts/PeerProvider.tsx
  • src/modules/settings/NetworkSettingsTab.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/contexts/PeerProvider.tsx

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/modules/settings/NetworkSettingsTab.tsx (1)

156-165: Consider adding validation when clearing an existing IPv6 range.

Unlike the IPv4 validation (lines 139-143), this doesn't warn when a previously-configured IPv6 range is cleared. Clearing network_range_v6 would disable IPv6 for all peers, which may warrant user confirmation or at least a warning message.

If intentional (IPv6 is optional), this is fine as-is; otherwise, consider adding:

💡 Suggested enhancement
 const networkRangeV6Error = useMemo(() => {
-  if (networkRangeV6 == "") return "";
+  if (networkRangeV6 == "") {
+    if (account.settings.network_range_v6) {
+      return "Clearing this will disable IPv6 for all peers";
+    }
+    return "";
+  }
   if (!networkRangeV6.includes(":") || !cidr.isValidCIDR(networkRangeV6)) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/settings/NetworkSettingsTab.tsx` around lines 156 - 165, The IPv6
validation in the useMemo that computes networkRangeV6Error currently returns
empty string when networkRangeV6 == "" and therefore doesn't warn on clearing an
existing value; update the useMemo (networkRangeV6Error) to detect when
networkRangeV6 is empty and return a warning message (e.g., "Clearing IPv6 will
disable IPv6 for all peers — confirm or re-enter a valid IPv6 CIDR") or trigger
the same confirmation flow used for IPv4, while keeping the existing CIDR and
prefix-length checks for networkRangeV6.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/modules/settings/NetworkSettingsTab.tsx`:
- Around line 156-165: The IPv6 validation in the useMemo that computes
networkRangeV6Error currently returns empty string when networkRangeV6 == "" and
therefore doesn't warn on clearing an existing value; update the useMemo
(networkRangeV6Error) to detect when networkRangeV6 is empty and return a
warning message (e.g., "Clearing IPv6 will disable IPv6 for all peers — confirm
or re-enter a valid IPv6 CIDR") or trigger the same confirmation flow used for
IPv4, while keeping the existing CIDR and prefix-length checks for
networkRangeV6.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: eebf455d-2af0-4242-bad5-2bb365f55b2e

📥 Commits

Reviewing files that changed from the base of the PR and between 492bbbd and d156e53.

📒 Files selected for processing (1)
  • src/modules/settings/NetworkSettingsTab.tsx

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/app/(remote-access)/peer/ssh/page.tsx (1)

86-91: Use sshHost consistently in status text.

After introducing sshHost, the loading/disconnected text still uses peer.ip, which can mislead users in IPv6/FQDN mode.

🧩 Suggested refactor
   if (isSSHDisconnected && sshConnectedOnce.current) {
     return (
       <DisconnectedMessage
         username={username}
-        peerIp={peer.ip}
+        peerIp={sshHost}
         onReconnect={handleReconnect}
       />
     );
   }
@@
       {session && <Terminal session={session} onClose={disconnect} />}
       {!isSSHConnected && (
-        <LoadingMessage message={`Connecting to ${username}@${peer.ip}...`} />
+        <LoadingMessage message={`Connecting to ${username}@${sshHost}...`} />
       )}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/`(remote-access)/peer/ssh/page.tsx around lines 86 - 91, The
status/feedback strings still reference peer.ip instead of the new sshHost,
causing incorrect addresses in IPv6/FQDN mode; find where the UI renders
loading/disconnected status text (the same component using sshHost and the
useEffect that sets document.title) and replace any occurrences of peer.ip in
those status messages with sshHost so the displayed host matches the dialed
address family (keep sshHost in component state/deps like username, peer,
client, sshHost).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/modules/remote-access/ssh/useSSHQueryParams.ts`:
- Around line 23-31: The code reads ip_version from searchParams
(searchParams.get("ip_version")) and stores it into state via setParams without
validation, which can lead to invalid host-selection; update the logic in the
useEffect blocks that hydrate state/router (the block using searchParams.get and
the later block at lines 49-70) to validate ipVersion before using it: accept
only allowed values (e.g., "4" or "6" or your app's canonical tokens), treat any
other value as undefined/null (drop it) and do not include it when calling
setParams or updating the router state; ensure the validation is applied both
where URL params are read and where localStorage values are restored so invalid
ip_version values are ignored early.

---

Nitpick comments:
In `@src/app/`(remote-access)/peer/ssh/page.tsx:
- Around line 86-91: The status/feedback strings still reference peer.ip instead
of the new sshHost, causing incorrect addresses in IPv6/FQDN mode; find where
the UI renders loading/disconnected status text (the same component using
sshHost and the useEffect that sets document.title) and replace any occurrences
of peer.ip in those status messages with sshHost so the displayed host matches
the dialed address family (keep sshHost in component state/deps like username,
peer, client, sshHost).
🪄 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: c5050fe4-45ae-4d8e-b457-5968507a2c47

📥 Commits

Reviewing files that changed from the base of the PR and between d156e53 and c8d22d5.

📒 Files selected for processing (4)
  • src/app/(remote-access)/peer/ssh/page.tsx
  • src/modules/remote-access/ssh/useSSH.ts
  • src/modules/remote-access/ssh/useSSHQueryParams.ts
  • src/modules/remote-access/useNetBirdClient.ts
✅ Files skipped from review due to trivial changes (1)
  • src/modules/remote-access/ssh/useSSH.ts

Comment on lines 23 to +31
useEffect(() => {
const peerId = searchParams.get("id");
const username = searchParams.get("user");
const port = searchParams.get("port");
const ipVersion = searchParams.get("ip_version");

// If all params are present in URL, use them
if (peerId && username && port) {
setParams({ peerId, username, port });
setParams({ peerId, username, port, ipVersion });
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Validate ip_version before hydrating state/router.

ip_version is accepted as-is from URL/localStorage. Invalid values should be dropped early to avoid unintended SSH host-selection behavior.

🛠️ Suggested fix
+const normalizeIpVersion = (value: string | null): string | null => {
+  if (!value) return null;
+  return /^(4|6|ipv4|ipv6)$/i.test(value) ? value.toLowerCase() : null;
+};

 export function useSSHQueryParams() {
@@
-    const ipVersion = searchParams.get("ip_version");
+    const ipVersion = normalizeIpVersion(searchParams.get("ip_version"));
@@
-    const storedIpVersion = urlParams.get("ip_version");
+    const storedIpVersion = normalizeIpVersion(urlParams.get("ip_version"));

Also applies to: 49-70

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/remote-access/ssh/useSSHQueryParams.ts` around lines 23 - 31, The
code reads ip_version from searchParams (searchParams.get("ip_version")) and
stores it into state via setParams without validation, which can lead to invalid
host-selection; update the logic in the useEffect blocks that hydrate
state/router (the block using searchParams.get and the later block at lines
49-70) to validate ipVersion before using it: accept only allowed values (e.g.,
"4" or "6" or your app's canonical tokens), treat any other value as
undefined/null (drop it) and do not include it when calling setParams or
updating the router state; ensure the validation is applied both where URL
params are read and where localStorage values are restored so invalid ip_version
values are ignored early.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant