mirror of
https://github.com/open-policy-agent/opa.git
synced 2026-08-12 19:32:48 -06:00
fix: treat empty JSON Schema enum as unsatisfiable (#8934)
## Description
`json.match_schema()` treated `{"enum": []}` as if the `enum` keyword
were absent, so every instance validated successfully (strings, numbers,
null, booleans, arrays, objects).
Per the [JSON Schema validation
spec](https://json-schema.org/draft/2020-12/json-schema-validation#section-6.1.2),
`enum` succeeds only when the instance deep-equals one of the listed
values. An empty list therefore has nothing to match and must make the
schema **unsatisfiable** (fail for every instance).
### Root cause
In vendored `internal/gojsonschema`, enum presence was only checked via
`len(enum) > 0`. A present empty `enum` array left a nil/empty slice and
skipped validation entirely.
### Fix
- When the `enum` keyword is present, store a non-nil slice (possibly
empty).
- When the keyword is absent, leave `enum` as `nil`.
- Validate whenever `enum != nil`, so empty enum always produces an enum
error.
### Breaking change
Schemas with `"enum": []` previously matched **any** value; they now
reject **every** value.
This is intentional and aligns with the JSON Schema spec. Accidental
empty allow-lists (e.g. schema generation from a zero-entry allow-list)
now fail closed rather than fail open.
No change for:
- missing `enum` keyword (still accepts any value)
- non-empty `enum` (same accept/reject behavior as before)
### Tests
- `internal/gojsonschema`: `TestEmptyEnumUnsatisfiable` (unit)
- `v1/topdown`: `json.match_schema` cases for empty and non-empty enum
Fixes #8910
## Checklist
- [x] I have read the [contribution
guidelines](https://www.openpolicyagent.org/docs/latest/contributing/).
- [x] I have added/updated tests for my change.
- [x] All new/updated code follows the existing conventions of the
affected area.
---------
Signed-off-by: Dean Chen <862469039@qq.com>
This commit is contained in:
@@ -676,18 +676,24 @@ func (d *Schema) parseSchema(documentNode any, currentSchema *SubSchema) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
for _, v := range enum {
|
||||
is, err := marshalWithoutNumber(v)
|
||||
if err != nil {
|
||||
return err
|
||||
// Distinguish a present empty enum from a missing enum keyword.
|
||||
// JSON Schema: enum validation succeeds only if the instance equals one of
|
||||
// the listed values, so {"enum": []} is unsatisfiable (always fails).
|
||||
if enum != nil {
|
||||
currentSchema.enum = make([]string, 0, len(enum))
|
||||
for _, v := range enum {
|
||||
is, err := marshalWithoutNumber(v)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if isStringInSlice(currentSchema.enum, *is) {
|
||||
return errors.New(formatErrorDescription(
|
||||
Locale.KeyItemsMustBeUnique(),
|
||||
ErrorDetails{"key": KeyEnum},
|
||||
))
|
||||
}
|
||||
currentSchema.enum = append(currentSchema.enum, *is)
|
||||
}
|
||||
if isStringInSlice(currentSchema.enum, *is) {
|
||||
return errors.New(formatErrorDescription(
|
||||
Locale.KeyItemsMustBeUnique(),
|
||||
ErrorDetails{"key": KeyEnum},
|
||||
))
|
||||
}
|
||||
currentSchema.enum = append(currentSchema.enum, *is)
|
||||
}
|
||||
|
||||
// validation : SubSchema
|
||||
|
||||
@@ -410,3 +410,57 @@ func TestIncorrectRef(t *testing.T) {
|
||||
t.Errorf("Expected error 'Object has no key 'fail'' but got '%s'", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
// Empty enum is unsatisfiable: every instance must fail validation.
|
||||
// Note: draft meta-schemas require minItems:1 on enum, so this is only
|
||||
// exercised when meta-schema validation is off (the default for NewSchema
|
||||
// and for OPA's json.match_schema builtin).
|
||||
// https://github.com/open-policy-agent/opa/issues/8910
|
||||
func TestEmptyEnumUnsatisfiable(t *testing.T) {
|
||||
type tc struct {
|
||||
name string
|
||||
schema string
|
||||
data string
|
||||
valid bool
|
||||
}
|
||||
cases := []tc{
|
||||
// empty enum rejects every instance type
|
||||
{"empty enum rejects string", `{"enum": []}`, `"a"`, false},
|
||||
{"empty enum rejects number", `{"enum": []}`, `1`, false},
|
||||
{"empty enum rejects null", `{"enum": []}`, `null`, false},
|
||||
{"empty enum rejects bool", `{"enum": []}`, `true`, false},
|
||||
{"empty enum rejects array", `{"enum": []}`, `[]`, false},
|
||||
{"empty enum rejects object", `{"enum": []}`, `{}`, false},
|
||||
// missing enum is unrestricted
|
||||
{"missing enum accepts string", `{}`, `"a"`, true},
|
||||
{"missing enum accepts number", `{}`, `1`, true},
|
||||
{"missing enum accepts null", `{}`, `null`, true},
|
||||
{"missing enum accepts bool", `{}`, `true`, true},
|
||||
{"missing enum accepts array", `{}`, `[]`, true},
|
||||
{"missing enum accepts object", `{}`, `{}`, true},
|
||||
// non-empty enum control cases
|
||||
{"non-empty enum accepts listed value", `{"enum": ["a", "b", "c"]}`, `"a"`, true},
|
||||
{"non-empty enum rejects unlisted value", `{"enum": ["a", "b", "c"]}`, `"z"`, false},
|
||||
}
|
||||
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
schema, err := NewSchema(NewStringLoader(c.schema))
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected schema compile error: %v", err)
|
||||
}
|
||||
result, err := schema.Validate(NewStringLoader(c.data))
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected validate error: %v", err)
|
||||
}
|
||||
if result.Valid() != c.valid {
|
||||
t.Errorf("valid=%v, want %v (errors=%v)", result.Valid(), c.valid, result.Errors())
|
||||
}
|
||||
if !c.valid {
|
||||
if len(result.Errors()) == 0 || result.Errors()[0].Type() != "enum" {
|
||||
t.Errorf("expected enum error, got %v", result.Errors())
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -419,8 +419,9 @@ func (v *SubSchema) validateCommon(currentSubSchema *SubSchema, value any, resul
|
||||
}
|
||||
}
|
||||
|
||||
// enum:
|
||||
if len(currentSubSchema.enum) > 0 {
|
||||
// enum: nil means the keyword is absent; non-nil (including empty) means
|
||||
// the instance must deep-equal one of the listed values.
|
||||
if currentSubSchema.enum != nil {
|
||||
vString, err := marshalWithoutNumber(value)
|
||||
if err != nil {
|
||||
result.addInternalError(new(InternalError), context, value, ErrorDetails{"error": err})
|
||||
|
||||
Reference in New Issue
Block a user