[config_validator] Disallow numeric bounds fields when not required

Adds validation to check that the min_value, max_value, and
max_count fields of a ReportDefinition are set only when
required for the report's type and privacy level.

Bug: 70382
Change-Id: I4cb30bd966c60d3cca514f0985d09b5994670ceb
Reviewed-on: https://fuchsia-review.googlesource.com/c/cobalt/+/487002
Commit-Queue: Laura Peskin <pesk@google.com>
Reviewed-by: Zach Bush <zmbush@google.com>
diff --git a/src/bin/config_parser/src/config_validator/report_definitions.go b/src/bin/config_parser/src/config_validator/report_definitions.go
index fb1f9e6..808d6bf 100644
--- a/src/bin/config_parser/src/config_validator/report_definitions.go
+++ b/src/bin/config_parser/src/config_validator/report_definitions.go
@@ -430,10 +430,14 @@
 func validateMinValueMaxValue(r config.ReportDefinition) error {
 	switch r.PrivacyLevel {
 	case
-		config.ReportDefinition_PRIVACY_LEVEL_UNKNOWN,
-		config.ReportDefinition_NO_ADDED_PRIVACY:
+		config.ReportDefinition_PRIVACY_LEVEL_UNKNOWN:
 		return nil
 	case
+		config.ReportDefinition_NO_ADDED_PRIVACY:
+		if r.MaxValue != 0 || r.MinValue != 0 {
+			return fmt.Errorf("min_value and max_value should not be set for reports with no added privacy")
+		}
+	case
 		config.ReportDefinition_LOW_PRIVACY,
 		config.ReportDefinition_MEDIUM_PRIVACY,
 		config.ReportDefinition_HIGH_PRIVACY:
@@ -456,7 +460,9 @@
 			config.ReportDefinition_UNIQUE_DEVICE_HISTOGRAMS,
 			config.ReportDefinition_HOURLY_VALUE_HISTOGRAMS,
 			config.ReportDefinition_STRING_COUNTS:
-			return nil
+			if r.MinValue != 0 || r.MaxValue != 0 {
+				return fmt.Errorf("min_value and max_value should not be set for reports of type %s", r.ReportType)
+			}
 		default:
 			return fmt.Errorf("unrecognized report type %s", r.ReportType)
 		}
@@ -467,10 +473,14 @@
 func validateMaxCount(r config.ReportDefinition) error {
 	switch r.PrivacyLevel {
 	case
-		config.ReportDefinition_PRIVACY_LEVEL_UNKNOWN,
-		config.ReportDefinition_NO_ADDED_PRIVACY:
+		config.ReportDefinition_PRIVACY_LEVEL_UNKNOWN:
 		return nil
 	case
+		config.ReportDefinition_NO_ADDED_PRIVACY:
+		if r.MaxCount != 0 {
+			return fmt.Errorf("max_count should not be set for reports with no added privacy")
+		}
+	case
 		config.ReportDefinition_LOW_PRIVACY,
 		config.ReportDefinition_MEDIUM_PRIVACY,
 		config.ReportDefinition_HIGH_PRIVACY:
@@ -489,7 +499,10 @@
 			config.ReportDefinition_HOURLY_VALUE_NUMERIC_STATS,
 			config.ReportDefinition_UNIQUE_DEVICE_HISTOGRAMS,
 			config.ReportDefinition_HOURLY_VALUE_HISTOGRAMS:
-			return nil
+			if r.MaxCount != 0 {
+				return fmt.Errorf("max_count should not be set for reports of type %s", r.ReportType)
+			}
+
 		default:
 			return fmt.Errorf("unrecognized report type %s", r.ReportType)
 		}
diff --git a/src/bin/config_parser/src/config_validator/report_definitions_test.go b/src/bin/config_parser/src/config_validator/report_definitions_test.go
index cbb8075..1ba324b 100644
--- a/src/bin/config_parser/src/config_validator/report_definitions_test.go
+++ b/src/bin/config_parser/src/config_validator/report_definitions_test.go
@@ -141,6 +141,52 @@
 	}
 }
 
+func TestValidateMinValueMaxValueNoAddedPrivacy(t *testing.T) {
+	var reportTypes = []config.ReportDefinition_ReportType{
+		config.ReportDefinition_UNIQUE_DEVICE_COUNTS,
+		config.ReportDefinition_FLEETWIDE_OCCURRENCE_COUNTS,
+		config.ReportDefinition_UNIQUE_DEVICE_NUMERIC_STATS,
+		config.ReportDefinition_HOURLY_VALUE_NUMERIC_STATS,
+		config.ReportDefinition_UNIQUE_DEVICE_HISTOGRAMS,
+		config.ReportDefinition_HOURLY_VALUE_HISTOGRAMS,
+		config.ReportDefinition_FLEETWIDE_HISTOGRAMS,
+		config.ReportDefinition_FLEETWIDE_MEANS,
+		config.ReportDefinition_STRING_COUNTS,
+	}
+	type args struct {
+		minValue int64
+		maxValue int64
+	}
+	var tests = []struct {
+		input args
+		valid bool
+	}{
+		// Valid reports: no added privacy, no bounds set
+		{args{0, 0}, true},
+		// Invalid reports: no added privacy, at least one of min_value and max_value set
+		{args{0, 1}, false},
+		{args{1, 0}, false},
+		{args{1, 1}, false},
+	}
+	for _, reportType := range reportTypes {
+		for _, test := range tests {
+			r := makeValidReportWithType(reportType)
+			r.PrivacyLevel = config.ReportDefinition_NO_ADDED_PRIVACY
+			r.MinValue = test.input.minValue
+			r.MaxValue = test.input.maxValue
+			err := validateMinValueMaxValue(r)
+
+			if test.valid && err != nil {
+				t.Errorf("rejected valid report with type %v and (min_value, max_value) = %v error: %v",
+					reportType, test.input, err)
+			} else if !test.valid && err == nil {
+				t.Errorf("accepted invalid report with type %v and (min_value, max_value) = %v",
+					reportType, test.input)
+			}
+		}
+	}
+}
+
 func TestValidateMinValueMaxValueWithAddedPrivacy(t *testing.T) {
 	var required = []config.ReportDefinition_ReportType{
 		config.ReportDefinition_FLEETWIDE_OCCURRENCE_COUNTS,
@@ -148,6 +194,13 @@
 		config.ReportDefinition_HOURLY_VALUE_NUMERIC_STATS,
 		config.ReportDefinition_FLEETWIDE_MEANS,
 	}
+	var notAllowed = []config.ReportDefinition_ReportType{
+		config.ReportDefinition_UNIQUE_DEVICE_COUNTS,
+		config.ReportDefinition_UNIQUE_DEVICE_HISTOGRAMS,
+		config.ReportDefinition_HOURLY_VALUE_HISTOGRAMS,
+		config.ReportDefinition_FLEETWIDE_HISTOGRAMS,
+		config.ReportDefinition_STRING_COUNTS,
+	}
 	type args struct {
 		reportTypes []config.ReportDefinition_ReportType
 		minValue    int64
@@ -157,10 +210,15 @@
 		input args
 		valid bool
 	}{
+		// Valid reports: min value and max value should not be set
+		{args{notAllowed, 0, 0}, true},
 		// Valid reports: at least one of min value and max value should be set
 		{args{required, 0, 1}, true},
 		{args{required, -1, 1}, true},
 		{args{required, 1, 1}, true},
+		// Invalid reports: min value and max value should not be set
+		{args{notAllowed, 0, 1}, false},
+		{args{notAllowed, -1, 0}, false},
 		// Invalid reports: at least one of min value and max value should be set
 		{args{required, 0, 0}, false},
 		// Invalid reports: min value is larger than max value
@@ -184,12 +242,59 @@
 	}
 }
 
+func TestValidateMaxCountNoAddedPrivacy(t *testing.T) {
+	var reportTypes = []config.ReportDefinition_ReportType{
+		config.ReportDefinition_UNIQUE_DEVICE_COUNTS,
+		config.ReportDefinition_FLEETWIDE_OCCURRENCE_COUNTS,
+		config.ReportDefinition_UNIQUE_DEVICE_NUMERIC_STATS,
+		config.ReportDefinition_HOURLY_VALUE_NUMERIC_STATS,
+		config.ReportDefinition_UNIQUE_DEVICE_HISTOGRAMS,
+		config.ReportDefinition_HOURLY_VALUE_HISTOGRAMS,
+		config.ReportDefinition_FLEETWIDE_HISTOGRAMS,
+		config.ReportDefinition_FLEETWIDE_MEANS,
+		config.ReportDefinition_STRING_COUNTS,
+	}
+	var tests = []struct {
+		maxCount uint64
+		valid    bool
+	}{
+		// Valid reports: no added privacy, no bounds set
+		{0, true},
+		// Invalid reports: no added privacy, max_count set
+		{1, false},
+	}
+	for _, reportType := range reportTypes {
+		for _, test := range tests {
+			r := makeValidReportWithType(reportType)
+			r.PrivacyLevel = config.ReportDefinition_NO_ADDED_PRIVACY
+			r.MaxCount = test.maxCount
+			err := validateMaxCount(r)
+
+			if test.valid && err != nil {
+				t.Errorf("rejected valid report with type %v and max_count = %d error: %v",
+					reportType, test.maxCount, err)
+			} else if !test.valid && err == nil {
+				t.Errorf("accepted invalid report with type %v and max_count = %d",
+					reportType, test.maxCount)
+			}
+		}
+	}
+}
+
 func TestValidateMaxCountWithAddedPrivacy(t *testing.T) {
 	var required = []config.ReportDefinition_ReportType{
 		config.ReportDefinition_FLEETWIDE_HISTOGRAMS,
 		config.ReportDefinition_FLEETWIDE_MEANS,
 		config.ReportDefinition_STRING_COUNTS,
 	}
+	var notAllowed = []config.ReportDefinition_ReportType{
+		config.ReportDefinition_UNIQUE_DEVICE_COUNTS,
+		config.ReportDefinition_FLEETWIDE_OCCURRENCE_COUNTS,
+		config.ReportDefinition_UNIQUE_DEVICE_NUMERIC_STATS,
+		config.ReportDefinition_HOURLY_VALUE_NUMERIC_STATS,
+		config.ReportDefinition_UNIQUE_DEVICE_HISTOGRAMS,
+		config.ReportDefinition_HOURLY_VALUE_HISTOGRAMS,
+	}
 	type args struct {
 		reportTypes []config.ReportDefinition_ReportType
 		maxCount    uint64
@@ -200,6 +305,12 @@
 	}{
 		// Valid reports: max count should be set
 		{args{required, 1}, true},
+		// Valid reports: max count should not be set
+		{args{notAllowed, 0}, true},
+		// Valid reports: max count should be set
+		{args{required, 1}, true},
+		// Invalid reports: max count should not be set
+		{args{notAllowed, 1}, false},
 		// Invalid reports: max count should be set
 		{args{required, 0}, false},
 	}