Merge pull request #12 from TheManticoreProject/bugfix-property-value-validation

Fix property validation accepting null and non-primitive arrays (#11)

Author: p0dalirius

properties/Properties.go

@@ -70,21 +70,66 @@ func (p *Properties) Clear() {
 	p.Properties = make(map[string]interface{})
 }
 
-// isValidPropertyValue checks if a value is a valid property type
+// IsPropertyValueValid reports whether value is a valid OpenGraph property value.
+//
+// The BloodHound OpenGraph schema restricts a property value to a single
+// primitive (string, number, or boolean) or a homogeneous array of primitives.
+// null values, nested objects, arrays of objects, and arrays mixing primitive
+// types are not valid.
+//
+// Source: https://bloodhound.specterops.io/opengraph/developer/nodes
 func (p *Properties) IsPropertyValueValid(value interface{}) bool {
 	if value == nil {
-		return true
+		return false
+	}
+
+	switch reflect.TypeOf(value).Kind() {
+	case reflect.Slice, reflect.Array:
+		return isHomogeneousPrimitiveSequence(reflect.ValueOf(value))
+	default:
+		return primitiveCategory(value) != ""
+	}
+}
+
+// primitiveCategory classifies value into one of the OpenGraph primitive
+// categories ("string", "number", "boolean"). It returns "" when value is nil
+// or not a primitive (e.g. an object, slice, or array).
+func primitiveCategory(value interface{}) string {
+	if value == nil {
+		return ""
 	}
 
-	valueType := reflect.TypeOf(value)
-	switch valueType.Kind() {
-	case reflect.String, reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
+	switch reflect.TypeOf(value).Kind() {
+	case reflect.String:
+		return "string"
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
 		reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64,
-		reflect.Float32, reflect.Float64, reflect.Bool, reflect.Slice:
-		return true
+		reflect.Float32, reflect.Float64:
+		return "number"
+	case reflect.Bool:
+		return "boolean"
 	default:
-		return false
+		return ""
+	}
+}
+
+// isHomogeneousPrimitiveSequence reports whether v is a slice or array whose
+// elements are all primitives belonging to the same OpenGraph category. An
+// empty sequence is considered valid.
+func isHomogeneousPrimitiveSequence(v reflect.Value) bool {
+	category := ""
+	for i := 0; i < v.Len(); i++ {
+		c := primitiveCategory(v.Index(i).Interface())
+		if c == "" {
+			return false
+		}
+		if category == "" {
+			category = c
+		} else if c != category {
+			return false
+		}
 	}
+	return true
 }
 
 // ToDict converts properties to map for JSON serialization

properties/Properties_test.go

@@ -46,8 +46,8 @@ func TestSetProperty(t *testing.T) {
 		{"int", 42},
 		{"float", 3.14},
 		{"bool", true},
-		{"nil", nil},
-		{"slice", []string{"a", "b", "c"}},
+		{"string-slice", []string{"a", "b", "c"}},
+		{"int-slice", []int{1, 2, 3}},
 	}
 
 	for _, test := range validTests {
@@ -69,9 +69,12 @@ func TestSetProperty(t *testing.T) {
 
 	// Test invalid property types (should panic)
 	invalidTests := []interface{}{
-		map[string]string{"key": "value"}, // map
-		struct{}{},                        // struct
-		func() {},                         // function
+		nil,                                   // null is not an allowed property value
+		map[string]string{"key": "value"},     // map / nested object
+		struct{}{},                            // struct
+		func() {},                             // function
+		[]map[string]string{{"key": "value"}}, // array of objects
+		[]interface{}{1, "a"},                 // array mixing primitive types
 	}
 
 	for _, invalidValue := range invalidTests {
@@ -260,9 +263,10 @@ func TestIsValidPropertyValue(t *testing.T) {
 		42,
 		3.14,
 		true,
-		nil,
 		[]string{"a", "b"},
 		[]int{1, 2, 3},
+		[]bool{true, false},
+		[]string{}, // empty homogeneous array
 	}
 
 	for _, value := range validTypes {
@@ -273,10 +277,14 @@ func TestIsValidPropertyValue(t *testing.T) {
 
 	// Test invalid types
 	invalidTypes := []interface{}{
-		map[string]string{"key": "value"},
+		nil,                               // null is not an allowed property value
+		map[string]string{"key": "value"}, // nested object
 		struct{ field string }{field: "value"},
 		func() {},
 		make(chan int),
+		[]map[string]string{{"key": "value"}}, // array of objects
+		[]interface{}{1, "a"},                 // array mixing primitive types
+		[][]int{{1, 2}},                       // array of arrays
 	}
 
 	for _, value := range invalidTypes {