Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
c4844cc
proto: add ConnectionMode enum and p2p/relay timeout fields to PeerCo…
MichaelUray May 1, 2026
e0ed831
client: add connectionmode package with Mode type and proto bridge
MichaelUray May 1, 2026
c71c951
client/peer: ResolveModeFromEnv with NB_CONNECTION_MODE and deprecati…
MichaelUray May 1, 2026
7d90a5b
client: add --connection-mode, --relay-timeout, --p2p-timeout CLI flags
MichaelUray May 1, 2026
cc10c9f
client/conn_mgr: replace asymmetric Lazy/ForceRelay precedence with Mode
MichaelUray May 1, 2026
dfd48e9
client/peer: connection mode drives skip-ICE branch in Open()
MichaelUray May 1, 2026
82877f0
client/engine: forward resolved Mode to per-peer ConnConfig
MichaelUray May 1, 2026
cd0abe8
mgmt/types: add ConnectionMode + p2p/relay timeout to Settings
MichaelUray May 1, 2026
0022145
openapi: add connection_mode + p2p/relay timeout fields to AccountSet…
MichaelUray May 1, 2026
b22128e
mgmt/handlers/accounts: accept connection_mode + timeout settings on PUT
MichaelUray May 1, 2026
3d8cc98
mgmt/activity: add three new account-scoped event codes
MichaelUray May 1, 2026
b53322d
mgmt/account: emit audit events for connection_mode + timeout changes
MichaelUray May 1, 2026
f63e2a7
move connectionmode package + extend toPeerConfig to fill new wire fi…
MichaelUray May 1, 2026
77852a9
mgmt/grpc: tests for toPeerConfig connection-mode resolution
MichaelUray May 1, 2026
0304dc2
client/peer/handshaker: add RemoveICEListener + lock the listener-read
MichaelUray May 1, 2026
ec2b46e
client/lazyconn/inactivity: two-timer per-peer with separate channels
MichaelUray May 1, 2026
3528248
client/lazyconn/manager: Config gains ICEInactivityThreshold + Relay
MichaelUray May 1, 2026
5c18a23
feat(peer): add Conn.AttachICE / DetachICE for p2p-dynamic mode
MichaelUray May 1, 2026
ad789e3
client/peer/conn: Open() defers ICE-listener registration in p2p-dynamic
MichaelUray May 1, 2026
a9710d9
client/conn_mgr: per-mode DeactivatePeer + DetachICEForPeer
MichaelUray May 1, 2026
d662d9b
client/conn_mgr: wire p2p-dynamic two-timer lifecycle
MichaelUray May 1, 2026
76e2d0b
client/conn_mgr: resolve P2pTimeoutSeconds from server-pushed PeerConfig
MichaelUray May 1, 2026
efaa2b1
client/conn_mgr: ActivatePeer attaches ICE listener in p2p-dynamic
MichaelUray May 1, 2026
838702d
proto: add P2pRetryMaxSeconds field to PeerConfig (Phase 3 #5989)
MichaelUray May 1, 2026
90c3d9f
mgmt/types: add P2pRetryMaxSeconds to Settings
MichaelUray May 1, 2026
74265f3
openapi: add p2p_retry_max_seconds to AccountSettings
MichaelUray May 1, 2026
9f60f1a
mgmt/handlers/accounts: accept p2p_retry_max_seconds setting on PUT
MichaelUray May 1, 2026
37276ea
mgmt/account+activity: emit audit event for p2p_retry_max changes
MichaelUray May 1, 2026
f9db3b0
mgmt/grpc: include P2pRetryMaxSeconds in toPeerConfig with sentinel m…
MichaelUray May 1, 2026
f63852d
client/peer: add iceBackoffState with truncated exponential schedule
MichaelUray May 1, 2026
7928f05
client/peer/conn: hook iceBackoffState into Conn lifecycle
MichaelUray May 1, 2026
4bb38b6
client/peer/conn: AttachICE returns nil-no-op during ice backoff
MichaelUray May 1, 2026
a49534f
client/peer: hook pion ICE state changes into iceBackoff
MichaelUray May 1, 2026
68dd578
client/conn_mgr: resolve P2pRetryMaxSeconds from server PeerConfig
MichaelUray May 1, 2026
7349b95
client/conn_mgr: propagate P2pRetryMaxSeconds changes to active Conns
MichaelUray May 1, 2026
75713b9
client: --p2p-retry-max CLI flag + EngineConfig + profile field
MichaelUray May 1, 2026
32c4efb
client/engine: forward P2pRetryMaxSeconds to ConnConfig
MichaelUray May 1, 2026
451872e
client/cmd/status: show ICE-backoff state in --detail output
MichaelUray May 1, 2026
19fb079
client/status: suppress ICE-backoff line when nextRetry has passed
MichaelUray May 1, 2026
b22143d
client/proto+shared/management: regenerate after ConnectionMode + P2p…
MichaelUray May 5, 2026
bfdc73e
client/server: cover Phase 3.7i ConnectionMode fields in SetConfigReq…
MichaelUray May 6, 2026
7c1a7a1
client+shared: pin proto version headers to upstream values
MichaelUray May 6, 2026
701a20f
client/peer: fix codespell typo in env_test.go (unparseable -> unpars…
MichaelUray May 6, 2026
58eb4f8
client+shared: golangci-lint cleanups + proxy_service.pb.go protoc ve…
MichaelUray May 6, 2026
6dd1e44
client/internal/debug: render Phase 1+2+3 connection-mode fields in d…
MichaelUray May 6, 2026
7c80838
client/peer: reset ICE backoff + recreate workerICE on network change
MichaelUray May 2, 2026
b9a967f
client/peer/conn: send offer after workerICE recreate (Phase 3.5 foll…
MichaelUray May 2, 2026
8760fa1
client/peer/worker_ice: buffer remote candidates that race ahead of a…
MichaelUray May 2, 2026
6f86055
Revert "client/peer/worker_ice: buffer remote candidates that race ah…
MichaelUray May 2, 2026
939d946
client/peer/conn: refactor onNetworkChange to use the in-place agent …
MichaelUray May 2, 2026
15c6d90
client/peer/conn: drop SendOffer from onNetworkChange to fix offer-storm
MichaelUray May 2, 2026
78d2fdc
client/peer/worker_ice: skip new offers while ICE agent is connecting…
MichaelUray May 2, 2026
90dba34
client/internal: re-attach ICE on every signal trigger (Phase 3.7d)
MichaelUray May 2, 2026
bcb30b9
client/peer/conn: re-attach ICE listener inside onNetworkChange (Phas…
MichaelUray May 2, 2026
9e444b5
client/peer/ice_backoff: short delay for first failure post-network-c…
MichaelUray May 2, 2026
67e7f36
client/peer: skip workerICE.Close on network change when ICE still Co…
MichaelUray May 2, 2026
ddd1f87
client/peer/ice_backoff: widen post-network-change grace window (Phas…
MichaelUray May 2, 2026
34300d5
client/cmd/service: persist connection-mode + timeouts on install/rec…
MichaelUray May 2, 2026
b12be21
client/ui: Connection Mode + timeouts in Network tab (Phase 3.7h GUI)
MichaelUray May 2, 2026
f4ff7c7
client: surface server-pushed connection-mode/timeouts via daemon-RPC
MichaelUray May 3, 2026
0b85cf5
client/ui: Follow-Server (currently: ...) display + Lazy menu removal
MichaelUray May 3, 2026
672a9bc
client/android: gomobile getters for ConnectionMode + ServerPushed va…
MichaelUray May 3, 2026
abeecc6
client+shared: regenerate proto after Phase 3.7h GUI proto changes
MichaelUray May 5, 2026
3730df0
client+shared: regenerate proto on rebased PR-B + pin protoc version …
MichaelUray May 6, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions client/android/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,66 @@ func (c *Client) RemoveConnectionListener() {
c.recorder.RemoveConnectionListener()
}

// GetServerPushedConnectionMode returns the canonical name of the
// connection mode the management server most recently pushed via
// PeerConfig (independent of any local profile/env override). Returns
// an empty string when the engine has not connected yet or the server
// has not pushed a value -- the Android UI then knows to display
// just "Follow server" without the (currently: ...) suffix.
func (c *Client) GetServerPushedConnectionMode() string {
cm := c.connMgrSafe()
if cm == nil {
return ""
}
return cm.ServerPushedMode().String()
}

// GetServerPushedRelayTimeoutSecs returns the relay timeout in seconds
// most recently pushed by the management server, or 0 when no value
// has been received. Used by the Android UI as a hint.
func (c *Client) GetServerPushedRelayTimeoutSecs() int64 {
cm := c.connMgrSafe()
if cm == nil {
return 0
}
return int64(cm.ServerPushedRelayTimeoutSecs())
}

// GetServerPushedP2pTimeoutSecs returns the ICE-only timeout (seconds)
// most recently pushed by the management server.
func (c *Client) GetServerPushedP2pTimeoutSecs() int64 {
cm := c.connMgrSafe()
if cm == nil {
return 0
}
return int64(cm.ServerPushedP2pTimeoutSecs())
}

// GetServerPushedP2pRetryMaxSecs returns the ICE-backoff cap (seconds)
// most recently pushed by the management server.
func (c *Client) GetServerPushedP2pRetryMaxSecs() int64 {
cm := c.connMgrSafe()
if cm == nil {
return 0
}
return int64(cm.ServerPushedP2pRetryMaxSecs())
}

// connMgrSafe is a small helper that walks the Client -> ConnectClient
// -> Engine -> ConnMgr chain and returns nil at the first nil pointer.
// Each accessor that surfaces engine state to the Android UI uses it.
func (c *Client) connMgrSafe() *internal.ConnMgr {
cc := c.getConnectClient()
if cc == nil {
return nil
}
engine := cc.Engine()
if engine == nil {
return nil
}
return engine.ConnMgr()
}

func (c *Client) toggleRoute(command routeCommand) error {
return command.toggleRoute()
}
Expand Down
85 changes: 85 additions & 0 deletions client/android/preferences.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,91 @@ func (p *Preferences) SetBlockInbound(block bool) {
p.configInput.BlockInbound = &block
}

// GetConnectionMode returns the locally configured connection-mode override
// (canonical lower-kebab-case: "relay-forced", "p2p", "p2p-lazy",
// "p2p-dynamic", "follow-server"), or empty string if no local override
// is configured -- the daemon will then follow the server-pushed value.
func (p *Preferences) GetConnectionMode() (string, error) {
if p.configInput.ConnectionMode != nil {
return *p.configInput.ConnectionMode, nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
if err != nil {
return "", err
}
return cfg.ConnectionMode, nil
}

// SetConnectionMode stores a local override for the connection mode.
// Pass an empty string to clear the override (revert to following the
// server-pushed value).
func (p *Preferences) SetConnectionMode(mode string) {
m := mode
p.configInput.ConnectionMode = &m
}

// GetRelayTimeoutSeconds returns the locally configured relay-worker
// inactivity timeout in seconds, or 0 if no override is set (follow
// server-pushed value, or built-in default if the server has none).
func (p *Preferences) GetRelayTimeoutSeconds() (int64, error) {
if p.configInput.RelayTimeoutSeconds != nil {
return int64(*p.configInput.RelayTimeoutSeconds), nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
if err != nil {
return 0, err
}
return int64(cfg.RelayTimeoutSeconds), nil
}

// SetRelayTimeoutSeconds stores a local override for the relay timeout.
// Pass 0 to clear the override.
func (p *Preferences) SetRelayTimeoutSeconds(secs int64) {
v := uint32(secs)
p.configInput.RelayTimeoutSeconds = &v
}
Comment on lines +347 to +352
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard negative timeout inputs before casting to uint32.

These setters currently turn negative Android values into huge positive timeouts (-1 becomes 4294967295), which silently corrupts the persisted override instead of clearing/rejecting it. Clamp or reject < 0 before the cast for all three setters.

🛠️ Suggested fix
 func (p *Preferences) SetRelayTimeoutSeconds(secs int64) {
+	if secs < 0 {
+		secs = 0
+	}
 	v := uint32(secs)
 	p.configInput.RelayTimeoutSeconds = &v
 }
@@
 func (p *Preferences) SetP2pTimeoutSeconds(secs int64) {
+	if secs < 0 {
+		secs = 0
+	}
 	v := uint32(secs)
 	p.configInput.P2pTimeoutSeconds = &v
 }
@@
 func (p *Preferences) SetP2pRetryMaxSeconds(secs int64) {
+	if secs < 0 {
+		secs = 0
+	}
 	v := uint32(secs)
 	p.configInput.P2pRetryMaxSeconds = &v
 }

Also applies to: 368-373, 388-393

🤖 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 `@client/android/preferences.go` around lines 347 - 352, Validate negative
timeout inputs before casting to uint32: in SetRelayTimeoutSeconds (and the
other two timeout setters around the same area) check if the incoming secs < 0
and, if so, clear the override (set the corresponding
p.configInput.*TimeoutSeconds = nil) instead of casting; otherwise cast to
uint32 and assign as before. Ensure you update all three setters (the
SetRelayTimeoutSeconds function and the two other timeout setter functions
referenced) with the same guard.

Comment on lines +325 to +352
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm there's no other "clear override" path that nils the pointer in apply() or elsewhere.
rg -nP --type=go -C2 'ConnectionMode|RelayTimeoutSeconds|P2pTimeoutSeconds|P2pRetryMaxSeconds' -g '!**/*_test.go' -g 'client/internal/profilemanager/**'

Repository: netbirdio/netbird

Length of output: 3375


🏁 Script executed:

#!/bin/bash
# Check the actual SetConnectionMode, SetRelayTimeoutSeconds, SetP2pTimeoutSeconds, SetP2pRetryMaxSeconds implementations
cat -n client/android/preferences.go | head -400 | tail -100

Repository: netbirdio/netbird

Length of output: 4108


🏁 Script executed:

#!/bin/bash
# Search for all occurrences of these four setter functions to ensure we catch all of them
rg -n 'func.*Set(ConnectionMode|RelayTimeoutSeconds|P2pTimeoutSeconds|P2pRetryMaxSeconds)' --type=go -g '!**/*_test.go'

Repository: netbirdio/netbird

Length of output: 426


🏁 Script executed:

#!/bin/bash
# Check if there are any other code paths that might clear these overrides (e.g., by setting to nil)
rg -n 'ConnectionMode.*=.*nil|RelayTimeoutSeconds.*=.*nil|P2pTimeoutSeconds.*=.*nil|P2pRetryMaxSeconds.*=.*nil' --type=go -g '!**/*_test.go'

Repository: netbirdio/netbird

Length of output: 3062


Fix the "clear override" logic to set pointer to nil, not to zero value.

The documented behaviour for SetConnectionMode, SetRelayTimeoutSeconds, SetP2pTimeoutSeconds, and SetP2pRetryMaxSeconds promises that passing 0 (or empty string) clears the override and reverts to the server-pushed value. However, each setter currently stores a pointer to the zero value, and the apply() function in client/internal/profilemanager/config.go (lines 608–626) checks only != nil before assigning. This overwrites the persisted server-pushed value with 0 or "" instead of clearing the override.

To honour the documented semantics, set the pointer to nil when the sentinel value is passed:

Suggested fix pattern
 func (p *Preferences) SetConnectionMode(mode string) {
+	if mode == "" {
+		p.configInput.ConnectionMode = nil
+		return
+	}
 	m := mode
 	p.configInput.ConnectionMode = &m
 }
@@
 func (p *Preferences) SetRelayTimeoutSeconds(secs int64) {
+	if secs <= 0 {
+		p.configInput.RelayTimeoutSeconds = nil
+		return
+	}
 	v := uint32(secs)
 	p.configInput.RelayTimeoutSeconds = &v
 }

Apply the same pattern to SetP2pTimeoutSeconds and SetP2pRetryMaxSeconds.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// SetConnectionMode stores a local override for the connection mode.
// Pass an empty string to clear the override (revert to following the
// server-pushed value).
func (p *Preferences) SetConnectionMode(mode string) {
m := mode
p.configInput.ConnectionMode = &m
}
// GetRelayTimeoutSeconds returns the locally configured relay-worker
// inactivity timeout in seconds, or 0 if no override is set (follow
// server-pushed value, or built-in default if the server has none).
func (p *Preferences) GetRelayTimeoutSeconds() (int64, error) {
if p.configInput.RelayTimeoutSeconds != nil {
return int64(*p.configInput.RelayTimeoutSeconds), nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
if err != nil {
return 0, err
}
return int64(cfg.RelayTimeoutSeconds), nil
}
// SetRelayTimeoutSeconds stores a local override for the relay timeout.
// Pass 0 to clear the override.
func (p *Preferences) SetRelayTimeoutSeconds(secs int64) {
v := uint32(secs)
p.configInput.RelayTimeoutSeconds = &v
}
// SetConnectionMode stores a local override for the connection mode.
// Pass an empty string to clear the override (revert to following the
// server-pushed value).
func (p *Preferences) SetConnectionMode(mode string) {
if mode == "" {
p.configInput.ConnectionMode = nil
return
}
m := mode
p.configInput.ConnectionMode = &m
}
// GetRelayTimeoutSeconds returns the locally configured relay-worker
// inactivity timeout in seconds, or 0 if no override is set (follow
// server-pushed value, or built-in default if the server has none).
func (p *Preferences) GetRelayTimeoutSeconds() (int64, error) {
if p.configInput.RelayTimeoutSeconds != nil {
return int64(*p.configInput.RelayTimeoutSeconds), nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
if err != nil {
return 0, err
}
return int64(cfg.RelayTimeoutSeconds), nil
}
// SetRelayTimeoutSeconds stores a local override for the relay timeout.
// Pass 0 to clear the override.
func (p *Preferences) SetRelayTimeoutSeconds(secs int64) {
if secs <= 0 {
p.configInput.RelayTimeoutSeconds = nil
return
}
v := uint32(secs)
p.configInput.RelayTimeoutSeconds = &v
}
🤖 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 `@client/android/preferences.go` around lines 325 - 352, The setters store
pointers to zero values instead of clearing overrides, so change
SetConnectionMode, SetRelayTimeoutSeconds (and likewise SetP2pTimeoutSeconds and
SetP2pRetryMaxSeconds) to set the corresponding pointer to nil when the sentinel
is passed (empty string for SetConnectionMode, 0 for the timeout/setter
methods); otherwise allocate and assign a pointer to the non-zero value. Update
p.configInput.ConnectionMode and p.configInput.RelayTimeoutSeconds (and the P2P
fields) accordingly so apply()'s nil check correctly reverts to server-pushed
values.


// GetP2pTimeoutSeconds returns the locally configured ICE-worker
// inactivity timeout in seconds (only effective in p2p-dynamic mode),
// or 0 if no override is set.
func (p *Preferences) GetP2pTimeoutSeconds() (int64, error) {
if p.configInput.P2pTimeoutSeconds != nil {
return int64(*p.configInput.P2pTimeoutSeconds), nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
if err != nil {
return 0, err
}
return int64(cfg.P2pTimeoutSeconds), nil
}

// SetP2pTimeoutSeconds stores a local override for the p2p timeout.
// Pass 0 to clear the override.
func (p *Preferences) SetP2pTimeoutSeconds(secs int64) {
v := uint32(secs)
p.configInput.P2pTimeoutSeconds = &v
}

// GetP2pRetryMaxSeconds returns the locally configured cap on the
// per-peer ICE-failure backoff schedule, or 0 if no override is set.
func (p *Preferences) GetP2pRetryMaxSeconds() (int64, error) {
if p.configInput.P2pRetryMaxSeconds != nil {
return int64(*p.configInput.P2pRetryMaxSeconds), nil
}
cfg, err := profilemanager.ReadConfig(p.configInput.ConfigPath)
if err != nil {
return 0, err
}
return int64(cfg.P2pRetryMaxSeconds), nil
}

// SetP2pRetryMaxSeconds stores a local override for the backoff cap.
// Pass 0 to clear the override.
func (p *Preferences) SetP2pRetryMaxSeconds(secs int64) {
v := uint32(secs)
p.configInput.P2pRetryMaxSeconds = &v
}

// Commit writes out the changes to the config file
func (p *Preferences) Commit() error {
_, err := profilemanager.UpdateOrCreateConfig(p.configInput)
Expand Down
17 changes: 17 additions & 0 deletions client/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ const (
extraIFaceBlackListFlag = "extra-iface-blacklist"
dnsRouteIntervalFlag = "dns-router-interval"
enableLazyConnectionFlag = "enable-lazy-connection"
connectionModeFlag = "connection-mode"
relayTimeoutFlag = "relay-timeout"
p2pTimeoutFlag = "p2p-timeout"
p2pRetryMaxFlag = "p2p-retry-max"
mtuFlag = "mtu"
)

Expand Down Expand Up @@ -72,6 +76,10 @@ var (
anonymizeFlag bool
dnsRouteInterval time.Duration
lazyConnEnabled bool
connectionMode string
relayTimeoutSecs uint32
p2pTimeoutSecs uint32
p2pRetryMaxSecs uint32
mtu uint16
profilesDisabled bool
updateSettingsDisabled bool
Expand Down Expand Up @@ -192,6 +200,15 @@ func init() {
upCmd.PersistentFlags().BoolVar(&rosenpassPermissive, rosenpassPermissiveFlag, false, "[Experimental] Enable Rosenpass in permissive mode to allow this peer to accept WireGuard connections without requiring Rosenpass functionality from peers that do not have Rosenpass enabled.")
upCmd.PersistentFlags().BoolVar(&autoConnectDisabled, disableAutoConnectFlag, false, "Disables auto-connect feature. If enabled, then the client won't connect automatically when the service starts.")
upCmd.PersistentFlags().BoolVar(&lazyConnEnabled, enableLazyConnectionFlag, false, "[Experimental] Enable the lazy connection feature. If enabled, the client will establish connections on-demand. Note: this setting may be overridden by management configuration.")
upCmd.PersistentFlags().StringVar(&connectionMode, connectionModeFlag, "",
"[Experimental] Peer connection mode: relay-forced, p2p, p2p-lazy, p2p-dynamic, or follow-server. "+
"Overrides the server-pushed value when set. Use follow-server to clear a previously-set local override.")
upCmd.PersistentFlags().Uint32Var(&relayTimeoutSecs, relayTimeoutFlag, 0,
"[Experimental] Relay-worker idle timeout in seconds. 0 = use server-pushed value (or built-in default).")
upCmd.PersistentFlags().Uint32Var(&p2pTimeoutSecs, p2pTimeoutFlag, 0,
"[Experimental] ICE-worker idle timeout in seconds. 0 = use server-pushed value (or built-in default). Only effective in p2p-dynamic mode (Phase 2).")
upCmd.PersistentFlags().Uint32Var(&p2pRetryMaxSecs, p2pRetryMaxFlag, 0,
"[Experimental] Maximum ICE-failure-backoff interval in seconds. 0 = use server-pushed value (or built-in default 15 min). Effective in p2p-dynamic mode (Phase 3 of #5989).")

}

Expand Down
18 changes: 18 additions & 0 deletions client/cmd/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,24 @@ func init() {
installCmd.Flags().StringSliceVar(&serviceEnvVars, "service-env", nil, serviceEnvDesc)
reconfigureCmd.Flags().StringSliceVar(&serviceEnvVars, "service-env", nil, serviceEnvDesc)

// Profile-level connection-mode + timeout flags. Same semantics as on
// `netbird up` but writeable at install time so server/headless
// installs can pre-seed the active profile before the daemon starts.
// Same package-level vars are shared with upCmd; on `up` they take
// effect through setupConfig(), here we apply them once before
// installing the service so the daemon picks them up on first run.
for _, c := range []*cobra.Command{installCmd, reconfigureCmd} {
c.Flags().StringVar(&connectionMode, connectionModeFlag, "",
"[Experimental] Peer connection mode: relay-forced, p2p, p2p-lazy, p2p-dynamic, or follow-server. "+
"Overrides the server-pushed value when set. Use follow-server to clear a previously-set local override.")
c.Flags().Uint32Var(&relayTimeoutSecs, relayTimeoutFlag, 0,
"[Experimental] Relay-worker idle timeout in seconds. 0 = use server-pushed value (or built-in default).")
c.Flags().Uint32Var(&p2pTimeoutSecs, p2pTimeoutFlag, 0,
"[Experimental] ICE-worker idle timeout in seconds. 0 = use server-pushed value. Only effective in p2p-dynamic mode.")
c.Flags().Uint32Var(&p2pRetryMaxSecs, p2pRetryMaxFlag, 0,
"[Experimental] Maximum ICE-failure-backoff interval in seconds. 0 = use server-pushed value (or built-in default 15 min).")
}

rootCmd.AddCommand(serviceCmd)
}

Expand Down
57 changes: 57 additions & 0 deletions client/cmd/service_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/kardianos/service"
"github.com/spf13/cobra"

"github.com/netbirdio/netbird/client/internal/profilemanager"
"github.com/netbirdio/netbird/util"
)

Expand Down Expand Up @@ -131,6 +132,12 @@ var installCmd = &cobra.Command{
cmd.PrintErrf("Warning: failed to load saved service params: %v\n", err)
}

// Persist any profile-level connection-mode/timeout flags that
// were explicitly set so the daemon picks them up on first start.
if err := applyConnectionModeFlagsToProfile(cmd); err != nil {
cmd.PrintErrf("Warning: failed to persist connection-mode flags: %v\n", err)
}
Comment on lines +135 to +139
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make connection-mode persistence failures abort the command.

These flags are not passed through buildServiceArguments(), so if applyConnectionModeFlagsToProfile fails here and execution continues, install/reconfigure completes with different connection-mode/timeout behavior than the user explicitly requested. This should return an error instead of downgrading to a warning.

Suggested change
-		if err := applyConnectionModeFlagsToProfile(cmd); err != nil {
-			cmd.PrintErrf("Warning: failed to persist connection-mode flags: %v\n", err)
-		}
+		if err := applyConnectionModeFlagsToProfile(cmd); err != nil {
+			return fmt.Errorf("persist connection-mode flags: %w", err)
+		}

Apply the same change in both the install and reconfigure paths.

Based on learnings methods returning errors should be propagated early rather than continuing with partially applied state.

Also applies to: 263-265

🤖 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 `@client/cmd/service_installer.go` around lines 135 - 139, The call to
applyConnectionModeFlagsToProfile currently logs a warning on failure, allowing
install/reconfigure to continue with inconsistent connection-mode state; change
this so that any error returned by applyConnectionModeFlagsToProfile is
propagated and causes the command to abort (return the error) instead of
printing a warning. Update both places where this call appears (the install path
around applyConnectionModeFlagsToProfile(cmd) and the reconfigure path) to
return the error up the call stack rather than using cmd.PrintErrf, ensuring the
command stops if persistence fails and no further install/reconfigure steps run.


svcConfig, err := createServiceConfigForInstall()
if err != nil {
return err
Expand All @@ -157,6 +164,52 @@ var installCmd = &cobra.Command{
},
}

// applyConnectionModeFlagsToProfile writes the connection-mode +
// timeout flags into the active profile's config file so the daemon
// will use them on its next startup. Only fields whose flag was
// explicitly set are touched; missing flags leave the existing
// profile values intact. Used by install + reconfigure so headless
// deployments can pre-seed everything in a single command.
func applyConnectionModeFlagsToProfile(cmd *cobra.Command) error {
anyChanged := false
for _, name := range []string{connectionModeFlag, relayTimeoutFlag, p2pTimeoutFlag, p2pRetryMaxFlag} {
if f := cmd.Flag(name); f != nil && f.Changed {
anyChanged = true
break
}
}
if !anyChanged {
return nil
}

cfgPath := profilemanager.DefaultConfigPath
if configPath != "" {
cfgPath = configPath
}
if cfgPath == "" {
return fmt.Errorf("default config path is not set on this platform; pass --config")
}

ic := profilemanager.ConfigInput{ConfigPath: cfgPath}
if cmd.Flag(connectionModeFlag).Changed {
ic.ConnectionMode = &connectionMode
}
if cmd.Flag(relayTimeoutFlag).Changed {
ic.RelayTimeoutSeconds = &relayTimeoutSecs
}
if cmd.Flag(p2pTimeoutFlag).Changed {
ic.P2pTimeoutSeconds = &p2pTimeoutSecs
}
if cmd.Flag(p2pRetryMaxFlag).Changed {
ic.P2pRetryMaxSeconds = &p2pRetryMaxSecs
}
if _, err := profilemanager.UpdateOrCreateConfig(ic); err != nil {
return fmt.Errorf("write profile %s: %w", cfgPath, err)
}
cmd.Println("connection-mode/timeout flags persisted to profile:", cfgPath)
return nil
}

var uninstallCmd = &cobra.Command{
Use: "uninstall",
Short: "uninstalls NetBird service from system",
Expand Down Expand Up @@ -207,6 +260,10 @@ This command will temporarily stop the service, update its configuration, and re
cmd.PrintErrf("Warning: failed to load saved service params: %v\n", err)
}

if err := applyConnectionModeFlagsToProfile(cmd); err != nil {
cmd.PrintErrf("Warning: failed to persist connection-mode flags: %v\n", err)
}

wasRunning, err := isServiceRunning()
if err != nil && !errors.Is(err, ErrGetServiceStatus) {
return fmt.Errorf("check service status: %w", err)
Expand Down
39 changes: 39 additions & 0 deletions client/cmd/up.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@
return nil
}

func setupSetConfigReq(customDNSAddressConverted []byte, cmd *cobra.Command, profileName, username string) *proto.SetConfigRequest {

Check warning on line 341 in client/cmd/up.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This function has 102 lines of code, which is greater than the 100 authorized. Split it into smaller functions.

See more on https://sonarcloud.io/project/issues?id=netbirdio_netbird&issues=AZ36Tez0KrubnQCUtNHa&open=AZ36Tez0KrubnQCUtNHa&pullRequest=6082
var req proto.SetConfigRequest
req.ProfileName = profileName
req.Username = username
Expand Down Expand Up @@ -439,10 +439,23 @@
req.LazyConnectionEnabled = &lazyConnEnabled
}

if cmd.Flag(connectionModeFlag).Changed {
req.ConnectionMode = &connectionMode
}
if cmd.Flag(relayTimeoutFlag).Changed {
req.RelayTimeoutSeconds = &relayTimeoutSecs
}
if cmd.Flag(p2pTimeoutFlag).Changed {
req.P2PTimeoutSeconds = &p2pTimeoutSecs
}
if cmd.Flag(p2pRetryMaxFlag).Changed {
req.P2PRetryMaxSeconds = &p2pRetryMaxSecs
}
Comment on lines +442 to +453
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

New connection-mode flags are silently dropped in daemon mode.

setupSetConfigReq populates req.ConnectionMode, req.RelayTimeoutSeconds, req.P2PTimeoutSeconds, and req.P2PRetryMaxSeconds, but server.go's SetConfig handler never reads or persists these four fields from the incoming *proto.SetConfigRequest. In daemon mode the client.SetConfig call at runInDaemonMode succeeds with no error, but the values are silently discarded — the user's --connection-mode override never takes effect.

The same applies to setupLoginRequest (lines 699-710): the populated fields are carried over the wire but server.go's Login handler does not persist them either.

The fix is to extract and apply these fields inside server.go's SetConfig:

🐛 Proposed fix in server.go `SetConfig`
 	if msg.Mtu != nil {
 		mtu := uint16(*msg.Mtu)
 		config.MTU = &mtu
 	}
+
+	if msg.ConnectionMode != nil {
+		config.ConnectionMode = *msg.ConnectionMode
+	}
+	if msg.RelayTimeoutSeconds != nil {
+		config.RelayTimeoutSeconds = *msg.RelayTimeoutSeconds
+	}
+	if msg.P2PTimeoutSeconds != nil {
+		config.P2pTimeoutSeconds = *msg.P2PTimeoutSeconds
+	}
+	if msg.P2PRetryMaxSeconds != nil {
+		config.P2pRetryMaxSeconds = *msg.P2PRetryMaxSeconds
+	}

 	if _, err := profilemanager.UpdateConfig(config); err != nil {

Wait - this is actually not high effort. The fix is 4 simple nil-checks in one location.

🤖 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 `@client/cmd/up.go` around lines 442 - 453, The SetConfig handler in server.go
is ignoring the incoming optional fields (req.ConnectionMode,
req.RelayTimeoutSeconds, req.P2PTimeoutSeconds, req.P2PRetryMaxSeconds) so
daemon-mode client overrides are dropped; add four nil-checks in server.go's
SetConfig to apply each field when non-nil (e.g., if req.ConnectionMode != nil {
serverConfig.ConnectionMode = *req.ConnectionMode }) and persist/update the
server configuration/store accordingly; do the same for Login's handler to read
and persist the same four fields from the incoming request (setupSetConfigReq /
setupLoginRequest populate them) so the client flags are honored.


return &req
}

func setupConfig(customDNSAddressConverted []byte, cmd *cobra.Command, configFilePath string) (*profilemanager.ConfigInput, error) {

Check warning on line 458 in client/cmd/up.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This function has 105 lines of code, which is greater than the 100 authorized. Split it into smaller functions.

See more on https://sonarcloud.io/project/issues?id=netbirdio_netbird&issues=AZ36Tez0KrubnQCUtNHb&open=AZ36Tez0KrubnQCUtNHb&pullRequest=6082
ic := profilemanager.ConfigInput{
ManagementURL: managementURL,
ConfigPath: configFilePath,
Expand Down Expand Up @@ -555,10 +568,23 @@
if cmd.Flag(enableLazyConnectionFlag).Changed {
ic.LazyConnectionEnabled = &lazyConnEnabled
}

if cmd.Flag(connectionModeFlag).Changed {
ic.ConnectionMode = &connectionMode
}
if cmd.Flag(relayTimeoutFlag).Changed {
ic.RelayTimeoutSeconds = &relayTimeoutSecs
}
if cmd.Flag(p2pTimeoutFlag).Changed {
ic.P2pTimeoutSeconds = &p2pTimeoutSecs
}
if cmd.Flag(p2pRetryMaxFlag).Changed {
ic.P2pRetryMaxSeconds = &p2pRetryMaxSecs
}
return &ic, nil
}

func setupLoginRequest(providedSetupKey string, customDNSAddressConverted []byte, cmd *cobra.Command) (*proto.LoginRequest, error) {

Check warning on line 587 in client/cmd/up.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This function has 105 lines of code, which is greater than the 100 authorized. Split it into smaller functions.

See more on https://sonarcloud.io/project/issues?id=netbirdio_netbird&issues=AZ36Tez0KrubnQCUtNHc&open=AZ36Tez0KrubnQCUtNHc&pullRequest=6082
loginRequest := proto.LoginRequest{
SetupKey: providedSetupKey,
ManagementUrl: managementURL,
Expand Down Expand Up @@ -669,6 +695,19 @@
if cmd.Flag(enableLazyConnectionFlag).Changed {
loginRequest.LazyConnectionEnabled = &lazyConnEnabled
}

if cmd.Flag(connectionModeFlag).Changed {
loginRequest.ConnectionMode = &connectionMode
}
if cmd.Flag(relayTimeoutFlag).Changed {
loginRequest.RelayTimeoutSeconds = &relayTimeoutSecs
}
if cmd.Flag(p2pTimeoutFlag).Changed {
loginRequest.P2PTimeoutSeconds = &p2pTimeoutSecs
}
if cmd.Flag(p2pRetryMaxFlag).Changed {
loginRequest.P2PRetryMaxSeconds = &p2pRetryMaxSecs
}
return &loginRequest, nil
}

Expand Down
Loading
Loading