From f054cbacde4b5bcdc81647109af624923a3684bd Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Wed, 12 Aug 2026 09:13:44 +0000 Subject: [PATCH] [client] Address review: redact and clear the MDM upload URL - Never log the MDM-provided upload URL (it can embed credentials or signed query tokens): mark the key secret so it is redacted, and drop the raw value from the invalid-URL warning. - Clear DebugBundleUploadURL when a replacement policy no longer carries the key, so a removed override can never keep directing uploads to a previously-enforced host; covered by a policy-replacement test. - macOS docs: state "https URL with a host" consistently, and make the managed-plist helper fail closed on an invalid allowRemoteJobs value (emit false rather than dropping the key). --- client/internal/profilemanager/config.go | 11 ++++++++--- client/internal/profilemanager/config_test.go | 8 ++++++++ client/mdm/policy.go | 2 ++ docs/netbird-macos.mobileconfig | 4 ++-- docs/netbird-macos.sh | 10 +++++++--- 5 files changed, 27 insertions(+), 8 deletions(-) diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go index 8a2140fac..16a781907 100644 --- a/client/internal/profilemanager/config.go +++ b/client/internal/profilemanager/config.go @@ -770,13 +770,18 @@ func (config *Config) applyMDMPolicy(policy *mdm.Policy) { if v, ok := policy.GetString(mdm.KeyBundleUploadURL); ok { // Must be a well-formed https URL with a host, matching the client's // remote-job upload-URL validation. Invalid values are skipped so a - // bad policy cannot break bundle uploads. + // bad policy cannot break bundle uploads. The URL is not logged: it + // can embed credentials or signed query tokens. if u, err := url.Parse(v); err != nil || u.Scheme != "https" || u.Host == "" { - log.Warnf("MDM debug bundle upload URL %q invalid (must be an https URL with a host); keeping previous value", v) + log.Warnf("MDM debug bundle upload URL is invalid (must be an https URL with a host); keeping previous value") } else { config.DebugBundleUploadURL = v - logApplied(mdm.KeyBundleUploadURL, v) + logApplied(mdm.KeyBundleUploadURL, "") } + } else { + // The key was dropped from the policy: clear any stale override so it + // can never keep directing uploads to a previously-enforced host. + config.DebugBundleUploadURL = "" } } diff --git a/client/internal/profilemanager/config_test.go b/client/internal/profilemanager/config_test.go index 89d63d77b..3b04ceed6 100644 --- a/client/internal/profilemanager/config_test.go +++ b/client/internal/profilemanager/config_test.go @@ -324,6 +324,14 @@ func TestApplyMDMPolicyRemoteJobs(t *testing.T) { })) assert.Empty(t, cfg.DebugBundleUploadURL, "a non-https upload URL must be skipped") }) + + t.Run("dropping the key clears a previously-applied override", func(t *testing.T) { + cfg := &Config{DebugBundleUploadURL: "https://old.example.com"} + // A replacement policy that no longer carries the key must not leave + // the old upload target directing bundles. + cfg.applyMDMPolicy(mdm.NewPolicy(map[string]any{mdm.KeyRemoteJobsAllowed: true})) + assert.Empty(t, cfg.DebugBundleUploadURL, "the stale upload URL override must be cleared") + }) } func TestUpdateOldManagementURL(t *testing.T) { diff --git a/client/mdm/policy.go b/client/mdm/policy.go index 774d162c8..081bc4189 100644 --- a/client/mdm/policy.go +++ b/client/mdm/policy.go @@ -82,6 +82,8 @@ const ( // SecretKeys lists keys whose values must be redacted in logs. var SecretKeys = map[string]struct{}{ KeyPreSharedKey: {}, + // The upload URL can embed credentials or signed query tokens. + KeyBundleUploadURL: {}, } // boolStringLiterals enumerates the textual boolean encodings the diff --git a/docs/netbird-macos.mobileconfig b/docs/netbird-macos.mobileconfig index 203ca02af..5d886c283 100644 --- a/docs/netbird-macos.mobileconfig +++ b/docs/netbird-macos.mobileconfig @@ -125,8 +125,8 @@ allowRemoteJobs : opt into management-requested remote jobs. Off by default. debugBundleUploadURL : override the debug-bundle upload - service (https URL); precedence - over the management value. --> + service (https URL with a host); + precedence over the management value. -->