From 725dc451ca324e69c152e326042884f1ac3fae6d Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Tue, 11 Aug 2026 01:43:03 +0000 Subject: [PATCH] [management] Validate anonymize_level on the debug bundle job API The client resolves an unknown anonymization level to strict, a fail-safe that is right for the wire but wrong for the API boundary: a caller that misspells the level should be told so at job creation, not have a different level than they asked for applied silently on the peer. Reject any anonymize_level other than the known wire forms when building a bundle job; an omitted or empty value still crosses the wire as empty and defaults on the client. The accepted forms are taken from the client anonymize package so the API and the consumer cannot drift. --- management/server/types/job.go | 12 ++++++++ management/server/types/job_test.go | 44 +++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/management/server/types/job.go b/management/server/types/job.go index efb70a911..e6d1be0ab 100644 --- a/management/server/types/job.go +++ b/management/server/types/job.go @@ -3,10 +3,12 @@ package types import ( "encoding/json" "fmt" + "strings" "time" "github.com/google/uuid" + "github.com/netbirdio/netbird/client/anonymize" "github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/proto" "github.com/netbirdio/netbird/shared/management/status" @@ -150,6 +152,16 @@ func validateAndBuildBundleParams(req api.WorkloadRequest, workload *Workload) e if bundle.Parameters.LogFileCount < 1 || bundle.Parameters.LogFileCount > 1000 { return fmt.Errorf("log-file-count must be between 1 and 1000, got %d", bundle.Parameters.LogFileCount) } + // validate anonymize_level: omitted or empty defaults on the client; + // otherwise it must name a known level. An unknown value is rejected here + // rather than silently escalated, so a typo surfaces at job creation. + if lvl := bundle.Parameters.AnonymizeLevel; lvl != nil { + switch strings.ToLower(strings.TrimSpace(*lvl)) { + case "", anonymize.LevelDefaultString, anonymize.LevelStrictString: + default: + return fmt.Errorf("anonymize_level must be %q or %q, got %q", anonymize.LevelDefaultString, anonymize.LevelStrictString, *lvl) + } + } workload.Parameters, err = json.Marshal(bundle.Parameters) if err != nil { diff --git a/management/server/types/job_test.go b/management/server/types/job_test.go index 357b6e3e4..aad3e720e 100644 --- a/management/server/types/job_test.go +++ b/management/server/types/job_test.go @@ -52,6 +52,50 @@ func TestBuildStreamBundleResponse_CarriesIdentityAndUploadFields(t *testing.T) assert.Equal(t, int32(100), bundle.GetLogFileCount(), "existing fields must still map") } +// newBundleJobRequest builds an api.JobRequest carrying a bundle workload with +// the given parameters, mirroring what the REST handler decodes. +func newBundleJobRequest(t *testing.T, p api.BundleParameters) *api.JobRequest { + t.Helper() + var wr api.WorkloadRequest + require.NoError(t, wr.FromBundleWorkloadRequest(api.BundleWorkloadRequest{ + Type: api.WorkloadTypeBundle, + Parameters: p, + }), "build bundle workload request") + return &api.JobRequest{Workload: wr} +} + +// TestNewJob_AnonymizeLevelValidation verifies the management API accepts only +// known anonymization levels (empty defaults on the client) and rejects an +// unknown value instead of silently escalating it. +func TestNewJob_AnonymizeLevelValidation(t *testing.T) { + base := api.BundleParameters{BundleFor: false, LogFileCount: 100, Anonymize: true} + + for _, tc := range []struct { + name string + level *string + wantErr bool + }{ + {name: "omitted", level: nil}, + {name: "empty", level: strPtr("")}, + {name: "default", level: strPtr("default")}, + {name: "strict", level: strPtr("strict")}, + {name: "mixed case", level: strPtr("Strict")}, + {name: "unknown", level: strPtr("verbose"), wantErr: true}, + } { + t.Run(tc.name, func(t *testing.T) { + p := base + p.AnonymizeLevel = tc.level + _, err := NewJob("user-1", "acc-1", "peer-1", newBundleJobRequest(t, p)) + if tc.wantErr { + require.Error(t, err, "an unknown anonymize_level must be rejected") + assert.Contains(t, err.Error(), "anonymize_level", "the error must name the offending field") + return + } + require.NoError(t, err, "a known anonymize_level must be accepted") + }) + } +} + // TestBuildStreamBundleResponse_OmittedFieldsMapToEmpty verifies that omitted // optional fields map to the empty proto string, which the client resolves to // its defaults (default anonymization level, default upload server).