[client] Add --disable-networks flag to block network selection#5896
Merged
[client] Add --disable-networks flag to block network selection#5896
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a --disable-networks option and propagates its state from CLI to service installer, server constructor, gRPC feature reporting, server RPC guards, persisted service params, tests, and UI menu enablement. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (--disable-networks)
participant Installer as Service Installer
participant Server as Daemon Server
participant UI as Client UI
participant RPC as Client RPC
CLI->>Installer: start service with --disable-networks
Installer->>Server: New(..., networksDisabled=true)
Server->>Server: store s.networksDisabled
UI->>Server: GetFeatures()
Server-->>UI: GetFeaturesResponse{disable_networks:true}
UI->>UI: set networksEnabled = false (disable menus)
RPC->>Server: ListNetworks / SelectNetworks / DeselectNetworks
Server-->>RPC: gRPC Unavailable (errNetworksDisabled)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
lixmal
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
client/server/network.go (1)
32-34: Optional: extract a sharedcheckNetworksDisabledhelper to reduce duplication.Same guard logic is repeated in three methods. A small helper would keep future behavior changes centralized.
♻️ Proposed refactor
+func (s *Server) checkNetworksDisabled() error { + if s.networksDisabled { + return gstatus.Errorf(codes.Unavailable, errNetworksDisabled) + } + return nil +} + func (s *Server) ListNetworks(context.Context, *proto.ListNetworksRequest) (*proto.ListNetworksResponse, error) { s.mutex.Lock() defer s.mutex.Unlock() - if s.networksDisabled { - return nil, gstatus.Errorf(codes.Unavailable, errNetworksDisabled) + if err := s.checkNetworksDisabled(); err != nil { + return nil, err } @@ func (s *Server) SelectNetworks(_ context.Context, req *proto.SelectNetworksRequest) (*proto.SelectNetworksResponse, error) { s.mutex.Lock() defer s.mutex.Unlock() - if s.networksDisabled { - return nil, gstatus.Errorf(codes.Unavailable, errNetworksDisabled) + if err := s.checkNetworksDisabled(); err != nil { + return nil, err } @@ func (s *Server) DeselectNetworks(_ context.Context, req *proto.SelectNetworksRequest) (*proto.SelectNetworksResponse, error) { s.mutex.Lock() defer s.mutex.Unlock() - if s.networksDisabled { - return nil, gstatus.Errorf(codes.Unavailable, errNetworksDisabled) + if err := s.checkNetworksDisabled(); err != nil { + return nil, err }Also applies to: 127-129, 177-179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/server/network.go` around lines 32 - 34, Extract the repeated guard into a single helper on the server type, e.g. add a method func (s *Server) checkNetworksDisabled() error that returns gstatus.Errorf(codes.Unavailable, errNetworksDisabled) when s.networksDisabled is true and nil otherwise, then replace the three inline blocks (the if s.networksDisabled { return nil, gstatus.Errorf... } occurrences) with if err := s.checkNetworksDisabled(); err != nil { return nil, err } so the behavior is centralized and future changes are made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/ui/client_ui.go`:
- Around line 1307-1317: The current logic flips s.mNetworks and s.mExitNode to
enabled whenever networksEnabled becomes true regardless of daemon connection;
update the branch so that when networksEnabled is true you only call
s.mNetworks.Enable() and s.mExitNode.Enable() if the UI knows the daemon is
connected (use your existing connection flag, e.g. s.daemonConnected or
s.connected) and otherwise keep them disabled; keep the disable branch unchanged
so features disabling still calls s.mNetworks.Disable() and
s.mExitNode.Disable().
---
Nitpick comments:
In `@client/server/network.go`:
- Around line 32-34: Extract the repeated guard into a single helper on the
server type, e.g. add a method func (s *Server) checkNetworksDisabled() error
that returns gstatus.Errorf(codes.Unavailable, errNetworksDisabled) when
s.networksDisabled is true and nil otherwise, then replace the three inline
blocks (the if s.networksDisabled { return nil, gstatus.Errorf... } occurrences)
with if err := s.checkNetworksDisabled(); err != nil { return nil, err } so the
behavior is centralized and future changes are made in one place.
🪄 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: 21a7c75e-1922-4c2e-a68e-2fc9dfa533cd
⛔ Files ignored due to path filters (1)
client/proto/daemon.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (11)
client/cmd/root.goclient/cmd/service.goclient/cmd/service_controller.goclient/cmd/service_installer.goclient/cmd/service_params.goclient/cmd/service_params_test.goclient/cmd/testutil_test.goclient/proto/daemon.protoclient/server/network.goclient/server/server.goclient/ui/client_ui.go
Contributor
There was a problem hiding this comment.
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 `@client/ui/client_ui.go`:
- Line 372: The default optimistic initialization of networksEnabled
(networksEnabled: true) allows updateStatus() to briefly show Networks/Exit Node
as enabled before checkAndUpdateFeatures() runs; change initialization so
s.networksEnabled is set from the real daemon feature state before the first
status refresh — either initialize networksEnabled to false and then set
s.networksEnabled from the existing getFeatures() result in onTrayReady(), or
seed s.networksEnabled directly from getFeatures() prior to calling
updateStatus(); update the same pattern for the other occurrences noted around
the 925-928 block to avoid the transient UI gap.
🪄 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: 2e93db20-849c-4d50-9646-0750c6a043b3
📒 Files selected for processing (1)
client/ui/client_ui.go
|
pappz
approved these changes
Apr 16, 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.
Describe your changes
Add a
--disable-networksservice flag that blocks network and exit node selection at the daemon level. For deployments where users should not modify routing.disable_networksto the GetFeaturesResponse proto so the UI can hide the Networks and Exit Node menus--disable-profilesand--disable-update-settingsUsage:
netbird service install --disable-networksIssue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
Flag follows the exact same pattern as existing --disable-profiles and --disable-update-settings which are already documented. Can be added to the same docs page.
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__
Summary by CodeRabbit
New Features
Bug Fixes / UX