From 803b0d6db09e9257fad0f11cfad02077d1d7192e Mon Sep 17 00:00:00 2001
From: riccardom <riccardomanfrin@gmail.com>
Date: Mon, 14 Sep 2026 09:16:10 +0200
Subject: [PATCH] [management,client] Keep the published upload destination
 across partial config updates
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

pushNewTURNTokens and pushNewRelayTokens send a SyncResponse whose NetbirdConfig
carries only Turns and Relay. handleDebugUploadUpdate read the absent Debug as
an empty destination and stored it, so every TURN credential refresh — every few
minutes — silently dropped the operator's choice and the next bundle went to the
service NetBird runs. That is the exact failure this branch exists to prevent.

A nil DebugConfig now carries no information and is left alone. To keep an
operator's clear reaching the peer, toNetbirdConfig always emits Debug on the
full config it builds, empty URL included, so the peer can tell "cleared" from
"not mentioned".

Reported by cubic on #7514.
---
 client/internal/engine.go                      | 15 ++++++++++++---
 client/internal/engine_bundle_test.go          | 11 +++++++++--
 management/internals/shared/grpc/conversion.go |  7 ++++---
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/client/internal/engine.go b/client/internal/engine.go
index e2ff901ed..21c6a4fe8 100644
--- a/client/internal/engine.go
+++ b/client/internal/engine.go
@@ -1239,10 +1239,19 @@ func (e *Engine) handleMetricsUpdate(config *mgmProto.MetricsConfig) {
 }
 
 // handleDebugUploadUpdate records the debug-bundle destination the management
-// server published. A nil DebugConfig clears it: a management server that stops
-// publishing a destination must take it away from the peer, not leave the peer
-// uploading to a host the operator has since removed.
+// server published.
+//
+// A nil DebugConfig carries no information and is left alone: the partial
+// updates that refresh TURN and relay credentials ship a NetbirdConfig holding
+// only those fields, and treating their absent Debug as "no destination" would
+// silently drop the operator's choice on every credential refresh. An operator
+// clearing the destination is an empty UploadUrl on a full config, which does
+// reach the store below.
 func (e *Engine) handleDebugUploadUpdate(config *mgmProto.DebugConfig) {
+	if config == nil {
+		return
+	}
+
 	url := config.GetUploadUrl()
 	e.debugUploadURL.Store(&url)
 }
diff --git a/client/internal/engine_bundle_test.go b/client/internal/engine_bundle_test.go
index 135453613..37a989d3f 100644
--- a/client/internal/engine_bundle_test.go
+++ b/client/internal/engine_bundle_test.go
@@ -50,8 +50,15 @@ func TestEngineDebugUploadURL(t *testing.T) {
 	e.handleDebugUploadUpdate(&mgmProto.DebugConfig{UploadUrl: "https://upload.example.com/upload-url"})
 	assert.Equal(t, "https://upload.example.com/upload-url", e.DebugUploadURL())
 
-	// An operator that removes the destination must take it away from the peer,
-	// not leave it uploading to a host that no longer exists.
+	// The partial updates that refresh TURN and relay credentials carry a
+	// NetbirdConfig with no Debug at all. Treating that as "no destination"
+	// would drop the operator's choice on every credential refresh.
+	e.handleDebugUploadUpdate(nil)
+	assert.Equal(t, "https://upload.example.com/upload-url", e.DebugUploadURL(),
+		"a partial config update must not clear the published destination")
+
+	// An operator that removes the destination sends an empty UploadUrl on a
+	// full config, and that does reach the peer.
 	e.handleDebugUploadUpdate(&mgmProto.DebugConfig{})
 	assert.Empty(t, e.DebugUploadURL())
 }
diff --git a/management/internals/shared/grpc/conversion.go b/management/internals/shared/grpc/conversion.go
index 7067fa787..9fb4fccb5 100644
--- a/management/internals/shared/grpc/conversion.go
+++ b/management/internals/shared/grpc/conversion.go
@@ -123,9 +123,10 @@ func toNetbirdConfig(config *nbconfig.Config, turnCredentials *Token, relayToken
 	if settings != nil && settings.DebugBundleUploadURL != "" {
 		debugUploadURL = settings.DebugBundleUploadURL
 	}
-	if debugUploadURL != "" {
-		nbConfig.Debug = &proto.DebugConfig{UploadUrl: debugUploadURL}
-	}
+	// Always sent, empty included: this is a full config, so the peer can tell an
+	// operator clearing the destination from the partial updates that carry only
+	// TURN or relay credentials and say nothing about it.
+	nbConfig.Debug = &proto.DebugConfig{UploadUrl: debugUploadURL}
 
 	return nbConfig
 }