Skip to content

Feature/ssh console#160

Open
evgeniyChepelev wants to merge 7 commits into
netbirdio:mainfrom
evgeniyChepelev:feature/ssh-console
Open

Feature/ssh console#160
evgeniyChepelev wants to merge 7 commits into
netbirdio:mainfrom
evgeniyChepelev:feature/ssh-console

Conversation

@evgeniyChepelev

@evgeniyChepelev evgeniyChepelev commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Description

Summary by CodeRabbit

  • New Features

    • Added an SSH tab for connecting to available peers and standalone hosts.
    • Added interactive terminal sessions with resizing, keyboard shortcuts, copy/paste, reconnect, and connection status indicators.
    • Added saved SSH sessions with optional secure password storage.
    • Added active-session management, including reopening and deleting sessions.
    • Added browser-based authentication support when required for NetBird SSH connections.
  • Bug Fixes

    • Updated VPN conflict navigation to open the correct Settings tab.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

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 end-to-end iOS SSH support, including network-extension session handling, terminal rendering through xterm.js, saved and active session management, SwiftUI connection screens, JWT authentication handoff, and Xcode resource/source wiring.

Changes

SSH transport and session lifecycle

Layer / File(s) Summary
Network extension SSH bridge
NetbirdKit/SSHIPC.swift, NetbirdNetworkExtension/SSHSessionManager.swift, NetbirdNetworkExtension/PacketTunnelProvider.swift, NetBird/Source/App/NetBirdApp.swift
Adds provider-message SSH commands, session creation and polling, terminal I/O, resizing, closure, JWT authentication URL handoff, and Darwin notification handling.
Session state and persistence
NetbirdKit/SavedSSHSession.swift, NetbirdKit/SSHSessionStore.swift, NetbirdKit/SSHKeychainStore.swift, NetBird/Source/App/ViewModels/*
Adds saved-session metadata, UserDefaults persistence, Keychain password storage, active-session tracking, connection states, reconnect handling, polling, and bounded output buffering.

Embedded terminal

Layer / File(s) Summary
Terminal web bridge
NetBird/Resources/Terminal/*, NetBird/Source/App/Views/SSH/SSHTerminalView.swift
Embeds xterm.js in a local web page, bridges terminal input/output and dimensions through WKWebView messages, and supports fitting, selection copy, status text, keyboard controls, and reconnect UI.

SwiftUI SSH experience

Layer / File(s) Summary
SSH entry points and screens
NetBird/Source/App/Views/MainView.swift, NetBird/Source/App/Views/Components/PeerDetailSheet.swift, NetBird/Source/App/Views/SSH/*
Adds the SSH tab, peer quick-connect flow, standalone and saved-session connection sheets, active and saved session lists, terminal presentation, deletion actions, status labels, and empty states.
Supporting view updates
NetBird/Source/App/Views/iOS/TroubleshootView.swift, NetBird/Source/App/Views/AdvancedView.swift
Adds the provided TroubleshootView line and adjusts whitespace in AdvancedView.

Project integration

Layer / File(s) Summary
Xcode and submodule wiring
NetBird.xcodeproj/project.pbxproj, .gitmodules, netbird-core
Registers SSH Swift sources and terminal resources, adds project groups and target build entries, removes package product dependencies from two extension targets, repoints the submodule URL and branch, and updates the submodule revision.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Poem

A bunny hops through shells of light,
With SSH tabs glowing bright.
Keys and sessions safely tucked,
Terminals resize with rabbit luck.
“Hop in!” the little helper cries—
New connections bloom before our eyes.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is missing actual content and does not fill the required Description section. Add a brief Description section summarizing the SSH console changes, key files touched, and any notable implementation or testing details.
Docstring Coverage ⚠️ Warning Docstring coverage is 14.71% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly points to the SSH console feature added in this PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@evgeniyChepelev

Copy link
Copy Markdown
Collaborator Author

/testflight

@github-actions

Copy link
Copy Markdown

Build failed (tvOS) 0.2.1 (70) for 6f753fb

View workflow run

@evgeniyChepelev

Copy link
Copy Markdown
Collaborator Author

/testflight

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (7)
NetbirdKit/SSHIPC.swift (1)

17-34: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Pipe-delimited wire format is fragile against delimiter characters in host/user.

sshConnectNetBirdPeer/sshConnect build messages like "SSHConnectNetBird:\(sessionID)|\(host)|\(port)|\(user)|\(cols)|\(rows)". Only the password is base64-encoded; host and user are inserted raw. If either contains a |, PacketTunnelProvider.handleSSHConnectNetBird/handleSSHConnect will fail the parts.count check and reject the connection with a generic "malformed payload" error instead of connecting. Practical risk is low (hostnames/usernames rarely contain |), but the protocol has no defense against it.

Since SSHPollResult already uses Codable/PropertyListEncoder for the poll response, consider using the same approach for the request payloads for consistency and robustness.

♻️ Example: base64-encode all free-form fields
-        let message = "SSHConnectNetBird:\(sessionID)|\(host)|\(port)|\(user)|\(cols)|\(rows)"
+        let hostB64 = Data(host.utf8).base64EncodedString()
+        let userB64 = Data(user.utf8).base64EncodedString()
+        let message = "SSHConnectNetBird:\(sessionID)|\(hostB64)|\(port)|\(userB64)|\(cols)|\(rows)"
🤖 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/SSHIPC.swift` around lines 17 - 34, Update sshConnectNetBirdPeer
and sshConnect so free-form host and user values cannot break the pipe-delimited
payload, preferably by encoding all request fields consistently with the
existing Codable/PropertyListEncoder approach used by SSHPollResult. Update the
corresponding PacketTunnelProvider.handleSSHConnectNetBird and handleSSHConnect
parsing paths to decode the new request format while preserving existing
validation and connection behavior.
NetbirdNetworkExtension/SSHSessionManager.swift (1)

15-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider moving these SSH IPC constants into the shared GlobalConstants.

sshJWTNotificationName, sshJWTURLDefaultsKey, sshJWTCodeDefaultsKey, and sshAppGroupID are re-declared as raw string literals in NetBirdApp.swift ("io.netbird.app.ssh.jwtRequired", "io.netbird.ssh.jwtURL", "group.io.netbird.app") rather than sharing a single source of truth. Elsewhere in this codebase (PacketTunnelProvider.swift), equivalent cross-process keys go through GlobalConstants (e.g., GlobalConstants.userPreferencesSuiteName, GlobalConstants.keyLoginRequired), which is already shared between the app and extension targets. Duplicating the literals here risks silent drift if one side is renamed without the other.

🤖 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 `@NetbirdNetworkExtension/SSHSessionManager.swift` around lines 15 - 18, Move
sshJWTNotificationName, sshJWTURLDefaultsKey, sshJWTCodeDefaultsKey, and
sshAppGroupID into the shared GlobalConstants definition, then update
SSHSessionManager.swift and NetBirdApp.swift to reference those shared symbols
instead of local raw string literals. Preserve the existing constant values and
ensure both app and extension targets use the single source of truth.
NetbirdNetworkExtension/PacketTunnelProvider.swift (1)

23-24: 🚀 Performance & Scalability | 🔵 Trivial

Shared concurrent queue for blocking long-polls could starve other SSH ops under many sessions.

sshQueue is a single concurrent queue backing every SSH operation (connect/write/resize/poll/close) for every active session. poll blocks its worker thread for up to the requested timeout (up to 25s per SSHSessionViewModel), so each concurrently polling session ties up one GCD worker thread from this queue for that duration. With enough simultaneous sessions this could approach the process's worker-thread pool limits and delay unrelated writes/resizes/closes for other sessions queued on the same sshQueue. Worth keeping in mind if concurrent-session usage grows (e.g., per-session queues, or an async/callback-based poll instead of a blocking wait).

🤖 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 `@NetbirdNetworkExtension/PacketTunnelProvider.swift` around lines 23 - 24,
Update the SSH operation scheduling around sshQueue and SSHSessionManager so
blocking poll calls cannot monopolize the shared queue used by connect, write,
resize, and close operations across sessions. Isolate polling per session or
replace the blocking wait with an asynchronous/callback-based implementation,
while preserving existing SSH behavior and operation ordering.
NetbirdKit/SSHSessionStore.swift (1)

9-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add @MainActor for consistency with SSHActiveSessionStore.

SSHActiveSessionStore is annotated @MainActor but SSHSessionStore is not, despite both being ObservableObjects with @Published properties used in SwiftUI. Without @MainActor, modifying sessions from a background thread triggers a runtime warning and can cause SwiftUI state races.

♻️ Proposed fix
-final class SSHSessionStore: ObservableObject {
+@MainActor
+final class SSHSessionStore: ObservableObject {
🤖 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/SSHSessionStore.swift` around lines 9 - 10, Annotate the
SSHSessionStore class with `@MainActor`, matching SSHActiveSessionStore, so its
published sessions state is isolated to the main actor for SwiftUI updates.
NetBird/Resources/Terminal/xterm.js (1)

1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Record the upstream xterm.js release next to the vendored bundle. The JS file is minified and has no version/source marker; add a short note beside the terminal resources so future xterm.js updates stay traceable. xterm.css still carries the MIT banner, so the provenance gap is only in the JS bundle.

🤖 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 `@NetBird/Resources/Terminal/xterm.js` around lines 1 - 2, Record the upstream
xterm.js release beside the vendored JavaScript bundle, adding a short
provenance note near the terminal resources without modifying the minified
bundle or duplicating the existing xterm.css license banner.

Source: Linters/SAST tools

NetBird/Source/App/Views/SSH/SSHConnectSheet.swift (1)

17-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Mark these @State properties private.

They are only initialized via the backing store in init and never read externally, so private satisfies private_swiftui_state without any behavior change.

♻️ Proposed change
-    `@State` var host: String
-    `@State` var port: String
-    `@State` var user: String
-    `@State` var password: String
+    `@State` private var host: String
+    `@State` private var port: String
+    `@State` private var user: String
+    `@State` private var password: String
🤖 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 `@NetBird/Source/App/Views/SSH/SSHConnectSheet.swift` around lines 17 - 20, In
the SSHConnectSheet state declarations, mark host, port, user, and password as
private while preserving their existing initialization and behavior.

Source: Linters/SAST tools

.gitmodules (1)

3-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Avoid depending on a contributor-owned fork for netbird-core.

In .gitmodules, netbird-core points to evgeniyChepelev/netbird on feature/ios-ssh-update, which ties builds to a non-official source and makes provenance/reproducibility harder. Move the SSH changes upstream or to an org-owned fork, then pin the submodule to a reviewed commit there.

🤖 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 @.gitmodules around lines 3 - 4, Update the netbird-core submodule
configuration in .gitmodules to use the official upstream repository or an
organization-owned fork instead of evgeniyChepelev/netbird, and replace the
feature branch reference with a reviewed, pinned commit from that trusted
source.
🤖 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.xcodeproj/project.pbxproj`:
- Around line 1034-1036: Restore GoogleService-Info.plist to the appropriate
final target resource phases in NetBird.xcodeproj, ensuring both the app and
network extension bundle it for the existing Crashlytics scripts and runtime
configuration. If resource membership is intentionally not restored, update both
Crashlytics scripts and the runtime configuration to use the supported
alternative instead.

In `@NetBird/Source/App/ViewModels/SSHSessionViewModel.swift`:
- Around line 61-107: Update onTerminalReady and/or start in SSHSessionViewModel
to ignore repeated terminal-ready events while state is already .connecting.
Preserve the existing resize/output behavior for .connected and only initiate
the SSH connection once until the current attempt completes or fails.

In `@NetBird/Source/App/Views/SSH/SavedSessionsListView.swift`:
- Around line 54-57: Update the onDelete handler in SavedSessionsListView to
snapshot the selected session IDs before mutating sessionStore.sessions, then
delete using those captured IDs rather than indexing the collection after each
deletion.

In `@NetBird/Source/App/Views/SSH/SSHTerminalView.swift`:
- Around line 183-190: Remove the backslash and apostrophe replacements from
setStatus(_:, color:), and build the base64 payload from the raw text so error
or close reasons render unchanged while retaining the existing JavaScript
injection safety.

In `@NetbirdKit/SSHKeychainStore.swift`:
- Around line 12-26: Update SSHKeychainStore.save to return Bool and propagate
the OSStatus from SecItemUpdate or SecItemAdd as success only when it equals
errSecSuccess. Preserve the existing update-versus-add lookup flow, and ensure
every path returns whether the password was persisted so callers can surface
failures.

In `@NetbirdNetworkExtension/SSHSessionManager.swift`:
- Around line 42-49: In open(_:userCode:), synchronize the shared app-group
UserDefaults instance after writing sshJWTURLDefaultsKey and
sshJWTCodeDefaultsKey, and before posting the JWT Darwin notification. Reuse the
same defaults instance for both writes and synchronization, preserving the
existing notification flow.
- Around line 123-139: Update SSHSessionManager.poll(timeout:) to drain any
excess dataAvailable semaphore signals after consuming pendingData, leaving at
most one signal for currently available data. Preserve the existing locking,
timeout behavior, and connected/error/closed result handling.

---

Nitpick comments:
In @.gitmodules:
- Around line 3-4: Update the netbird-core submodule configuration in
.gitmodules to use the official upstream repository or an organization-owned
fork instead of evgeniyChepelev/netbird, and replace the feature branch
reference with a reviewed, pinned commit from that trusted source.

In `@NetBird/Resources/Terminal/xterm.js`:
- Around line 1-2: Record the upstream xterm.js release beside the vendored
JavaScript bundle, adding a short provenance note near the terminal resources
without modifying the minified bundle or duplicating the existing xterm.css
license banner.

In `@NetBird/Source/App/Views/SSH/SSHConnectSheet.swift`:
- Around line 17-20: In the SSHConnectSheet state declarations, mark host, port,
user, and password as private while preserving their existing initialization and
behavior.

In `@NetbirdKit/SSHIPC.swift`:
- Around line 17-34: Update sshConnectNetBirdPeer and sshConnect so free-form
host and user values cannot break the pipe-delimited payload, preferably by
encoding all request fields consistently with the existing
Codable/PropertyListEncoder approach used by SSHPollResult. Update the
corresponding PacketTunnelProvider.handleSSHConnectNetBird and handleSSHConnect
parsing paths to decode the new request format while preserving existing
validation and connection behavior.

In `@NetbirdKit/SSHSessionStore.swift`:
- Around line 9-10: Annotate the SSHSessionStore class with `@MainActor`, matching
SSHActiveSessionStore, so its published sessions state is isolated to the main
actor for SwiftUI updates.

In `@NetbirdNetworkExtension/PacketTunnelProvider.swift`:
- Around line 23-24: Update the SSH operation scheduling around sshQueue and
SSHSessionManager so blocking poll calls cannot monopolize the shared queue used
by connect, write, resize, and close operations across sessions. Isolate polling
per session or replace the blocking wait with an asynchronous/callback-based
implementation, while preserving existing SSH behavior and operation ordering.

In `@NetbirdNetworkExtension/SSHSessionManager.swift`:
- Around line 15-18: Move sshJWTNotificationName, sshJWTURLDefaultsKey,
sshJWTCodeDefaultsKey, and sshAppGroupID into the shared GlobalConstants
definition, then update SSHSessionManager.swift and NetBirdApp.swift to
reference those shared symbols instead of local raw string literals. Preserve
the existing constant values and ensure both app and extension targets use the
single source of truth.
🪄 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: 8da7fb6e-b1f9-4fd3-afff-ce84b0eb6763

📥 Commits

Reviewing files that changed from the base of the PR and between 76177e3 and 6f753fb.

📒 Files selected for processing (27)
  • .gitmodules
  • NetBird.xcodeproj/project.pbxproj
  • NetBird/Resources/Terminal/addon-fit.js
  • NetBird/Resources/Terminal/terminal-bridge.js
  • NetBird/Resources/Terminal/terminal.html
  • NetBird/Resources/Terminal/xterm.css
  • NetBird/Resources/Terminal/xterm.js
  • NetBird/Source/App/NetBirdApp.swift
  • NetBird/Source/App/ViewModels/SSHActiveSessionStore.swift
  • NetBird/Source/App/ViewModels/SSHSessionViewModel.swift
  • NetBird/Source/App/Views/AdvancedView.swift
  • NetBird/Source/App/Views/Components/PeerDetailSheet.swift
  • NetBird/Source/App/Views/PeerTabView.swift
  • NetBird/Source/App/Views/SSH/ActiveSessionsView.swift
  • NetBird/Source/App/Views/SSH/SSHConnectSheet.swift
  • NetBird/Source/App/Views/SSH/SSHKeyboardAccessoryView.swift
  • NetBird/Source/App/Views/SSH/SSHTerminalView.swift
  • NetBird/Source/App/Views/SSH/SavedSessionsListView.swift
  • NetBird/Source/App/Views/iOS/TroubleshootView.swift
  • NetBird/Source/App/Views/iOS/iOSPeersView.swift
  • NetbirdKit/SSHIPC.swift
  • NetbirdKit/SSHKeychainStore.swift
  • NetbirdKit/SSHSessionStore.swift
  • NetbirdKit/SavedSSHSession.swift
  • NetbirdNetworkExtension/PacketTunnelProvider.swift
  • NetbirdNetworkExtension/SSHSessionManager.swift
  • netbird-core

Comment thread NetBird.xcodeproj/project.pbxproj
Comment thread NetBird/Source/App/ViewModels/SSHSessionViewModel.swift
Comment thread NetBird/Source/App/Views/SSH/SavedSessionsListView.swift Outdated
Comment thread NetBird/Source/App/Views/SSH/SSHTerminalView.swift
Comment thread NetbirdKit/SSHKeychainStore.swift Outdated
Comment thread NetbirdNetworkExtension/SSHSessionManager.swift Outdated
Comment thread NetbirdNetworkExtension/SSHSessionManager.swift
@evgeniyChepelev

Copy link
Copy Markdown
Collaborator Author

/testflight

@github-actions

Copy link
Copy Markdown

TestFlight builds uploaded 0.2.1 (71) for 12cdbdb — iOS + tvOS

View workflow run

evgeniyChepelev and others added 4 commits July 14, 2026 02:12
- Embedded xterm.js terminal (WKWebView) with PTY, keyboard accessory
  row (ESC/TAB/Ctrl/arrows), copy-paste, and keyboard-aware layout
- Two auth paths: NetBird JWT (no password, OAuth device-code flow) for
  peers with SSH enabled in the dashboard; username+password for regular
  SSH servers
- IPC layer between main app and Network Extension via
  NETunnelProviderSession messages (connect/write/resize/poll/close)
- Saved sessions (metadata in App Group UserDefaults, passwords in
  Keychain) and active session list with output replay on re-open
- SSH button in peer detail sheet (disabled for offline peers); standalone
  entry point in the Peers tab header for arbitrary hosts
- Darwin notification bridge so the extension can trigger browser OAuth
  from the main app process
- JWT token cached for 8 min to avoid re-auth on every reconnect
- WireGuard interface socket binding (IP_BOUND_IF) for correct routing
  in the Network Extension process
Switch submodule URL from netbirdio/netbird to evgeniyChepelev/netbird
and update the tracked commit to the tip of feature/ios-ssh-update which
contains the in-app SSH client additions (ConnectNetBirdPeer, JWT cache,
WireGuard interface binding).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… IPC churn

- SSHSessionViewModel.onTerminalReady: guard .connecting state to prevent
  a second start() when the WebView re-fires terminalReady mid-connect
- SavedSessionsListView: snapshot IDs before .onDelete loop to avoid
  index shifting when sessions array mutates during deletion
- SSHTerminalView.setStatus: remove redundant backslash/apostrophe escaping
  that corrupted status messages (text is already base64-encoded)
- SSHKeychainStore.save: return @discardableResult Bool so callers can
  detect Keychain write failures instead of silently losing passwords
- SSHSessionManager: call defaults.synchronize() before posting Darwin
  notification to ensure the reading process sees the JWT URL/code
- SSHSessionManager.poll: drain excess semaphore signals after the first
  wait() to prevent burst no-op IPC round trips after streamed output
@pappz

pappz commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

/testflight

@github-actions

Copy link
Copy Markdown

TestFlight builds uploaded 0.3.2 (1) for 85050f4 — iOS + tvOS

View workflow run

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
NetBird/Source/App/Views/SSH/SSHConnectSheet.swift (1)

17-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make SwiftUI state properties private.

In SwiftUI, @State properties should be explicitly marked as private to enforce encapsulation and ensure the state is only mutated by the view itself. This also resolves the static analysis warnings. Your custom initializer will continue to work correctly because it assigns directly to the underlying _host, _port, etc., state wrappers.

♻️ Proposed refactor
-    `@State` var host: String
-    `@State` var port: String
-    `@State` var user: String
-    `@State` var password: String
+    `@State` private var host: String
+    `@State` private var port: String
+    `@State` private var user: String
+    `@State` private var password: String
🤖 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 `@NetBird/Source/App/Views/SSH/SSHConnectSheet.swift` around lines 17 - 20,
Mark the `@State` properties host, port, user, and password in SSHConnectSheet as
private, preserving their existing types and custom initializer behavior through
the underlying state wrappers.

Source: Linters/SAST tools

🤖 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.

Nitpick comments:
In `@NetBird/Source/App/Views/SSH/SSHConnectSheet.swift`:
- Around line 17-20: Mark the `@State` properties host, port, user, and password
in SSHConnectSheet as private, preserving their existing types and custom
initializer behavior through the underlying state wrappers.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3382d36c-f9da-49ad-a0e4-9da4feb7f6a8

📥 Commits

Reviewing files that changed from the base of the PR and between c951738 and 23c1d6f.

📒 Files selected for processing (27)
  • .gitmodules
  • NetBird.xcodeproj/project.pbxproj
  • NetBird/Resources/Terminal/addon-fit.js
  • NetBird/Resources/Terminal/terminal-bridge.js
  • NetBird/Resources/Terminal/terminal.html
  • NetBird/Resources/Terminal/xterm.css
  • NetBird/Resources/Terminal/xterm.js
  • NetBird/Source/App/NetBirdApp.swift
  • NetBird/Source/App/ViewModels/SSHActiveSessionStore.swift
  • NetBird/Source/App/ViewModels/SSHSessionViewModel.swift
  • NetBird/Source/App/Views/AdvancedView.swift
  • NetBird/Source/App/Views/Components/PeerDetailSheet.swift
  • NetBird/Source/App/Views/PeerTabView.swift
  • NetBird/Source/App/Views/SSH/ActiveSessionsView.swift
  • NetBird/Source/App/Views/SSH/SSHConnectSheet.swift
  • NetBird/Source/App/Views/SSH/SSHKeyboardAccessoryView.swift
  • NetBird/Source/App/Views/SSH/SSHTerminalView.swift
  • NetBird/Source/App/Views/SSH/SavedSessionsListView.swift
  • NetBird/Source/App/Views/iOS/TroubleshootView.swift
  • NetBird/Source/App/Views/iOS/iOSPeersView.swift
  • NetbirdKit/SSHIPC.swift
  • NetbirdKit/SSHKeychainStore.swift
  • NetbirdKit/SSHSessionStore.swift
  • NetbirdKit/SavedSSHSession.swift
  • NetbirdNetworkExtension/PacketTunnelProvider.swift
  • NetbirdNetworkExtension/SSHSessionManager.swift
  • netbird-core
🚧 Files skipped from review as they are similar to previous changes (23)
  • NetBird/Source/App/Views/PeerTabView.swift
  • netbird-core
  • NetbirdKit/SavedSSHSession.swift
  • .gitmodules
  • NetBird/Source/App/Views/SSH/ActiveSessionsView.swift
  • NetBird/Resources/Terminal/addon-fit.js
  • NetBird/Resources/Terminal/terminal.html
  • NetBird/Resources/Terminal/terminal-bridge.js
  • NetbirdKit/SSHSessionStore.swift
  • NetBird/Source/App/ViewModels/SSHActiveSessionStore.swift
  • NetBird/Source/App/Views/SSH/SSHKeyboardAccessoryView.swift
  • NetBird/Source/App/Views/iOS/TroubleshootView.swift
  • NetBird/Source/App/Views/AdvancedView.swift
  • NetbirdNetworkExtension/SSHSessionManager.swift
  • NetBird/Source/App/Views/iOS/iOSPeersView.swift
  • NetBird/Source/App/Views/Components/PeerDetailSheet.swift
  • NetBird/Source/App/Views/SSH/SSHTerminalView.swift
  • NetbirdKit/SSHIPC.swift
  • NetBird/Source/App/NetBirdApp.swift
  • NetbirdNetworkExtension/PacketTunnelProvider.swift
  • NetbirdKit/SSHKeychainStore.swift
  • NetBird/Source/App/ViewModels/SSHSessionViewModel.swift
  • NetBird.xcodeproj/project.pbxproj

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
NetBird/Source/App/Views/MainView.swift (1)

187-187: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Simplify the badge count logic.

In SwiftUI, passing 0 to .badge(_:) automatically hides the badge. The ternary operator checking for isEmpty is redundant and can be replaced with just the count.

♻️ Proposed refactor
-                .badge(activeSessionStore.sessions.isEmpty ? 0 : activeSessionStore.sessions.count)
+                .badge(activeSessionStore.sessions.count)
🤖 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 `@NetBird/Source/App/Views/MainView.swift` at line 187, Update the .badge
modifier in MainView to pass activeSessionStore.sessions.count directly,
removing the redundant isEmpty ternary while preserving automatic hiding when
the count is zero.
NetBird/Source/App/Views/SSH/iOSSSHView.swift (1)

127-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Prevent duplicate IP address display when FQDN is missing.

If a peer lacks an FQDN, the current logic falls back to displaying the IP address as the primary text, but also continues to display the IP address as the secondary caption directly beneath it. Conditionally hiding the caption when the FQDN is empty will prevent this duplication and clean up the UI.

✨ Proposed refactor
                         VStack(alignment: .leading, spacing: 3) {
                             Text(peer.fqdn.isEmpty ? peer.ip : peer.fqdn)
                                 .foregroundColor(Color("TextPrimary"))
                                 .font(.body)
                                 .lineLimit(1)
-                            Text(peer.ip)
-                                .foregroundColor(.secondary)
-                                .font(.system(.caption, design: .monospaced))
+                            if !peer.fqdn.isEmpty {
+                                Text(peer.ip)
+                                    .foregroundColor(.secondary)
+                                    .font(.system(.caption, design: .monospaced))
+                            }
                         }
🤖 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 `@NetBird/Source/App/Views/SSH/iOSSSHView.swift` around lines 127 - 135, Update
the VStack in iOSSSHView so the secondary Text(peer.ip) caption is rendered only
when peer.fqdn is non-empty; preserve the primary display fallback of showing
peer.ip when the FQDN is missing.
🤖 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.

Nitpick comments:
In `@NetBird/Source/App/Views/MainView.swift`:
- Line 187: Update the .badge modifier in MainView to pass
activeSessionStore.sessions.count directly, removing the redundant isEmpty
ternary while preserving automatic hiding when the count is zero.

In `@NetBird/Source/App/Views/SSH/iOSSSHView.swift`:
- Around line 127-135: Update the VStack in iOSSSHView so the secondary
Text(peer.ip) caption is rendered only when peer.fqdn is non-empty; preserve the
primary display fallback of showing peer.ip when the FQDN is missing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 04008d59-c33d-4fd6-b054-9aab4577a8f7

📥 Commits

Reviewing files that changed from the base of the PR and between b279f7b and fbbc4b2.

📒 Files selected for processing (3)
  • NetBird.xcodeproj/project.pbxproj
  • NetBird/Source/App/Views/MainView.swift
  • NetBird/Source/App/Views/SSH/iOSSSHView.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • NetBird.xcodeproj/project.pbxproj

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.

2 participants