Repository: cerbos/cerbos (go)

Issue:
Upgrade to goccy/go-yaml@v1.15.5
New version of goccy/go-yaml rewrote some parts of the parser. This results in some go panics during policy file parsing.

The PR is cerbos/cerbos#2397.

The following snippet returns nil, which results in panic:
https://github.com/goccy/go-yaml/blame/2ab584e0fa5a8ae227ce23228599f888930531ef/parser/context.go#L43-L48

It is indirectly called from https://github.com/cerbos/cerbos/blob/0a49f4206b2112270793089a76333f58ab478db3/internal/parser/parser.go#L198

Pull request:
chore: Upgrade to go-yaml 1.15.6
Requires patch from https://github.com/goccy/go-yaml/pull/565.

Fixes #2401

Signed-off-by: Charith Ellawala <charith@cerbos.dev>


Patch:
diff --git a/go.mod b/go.mod
index f87e4dca..88d00b26 100644
--- a/go.mod
+++ b/go.mod
@@ -32,7 +32,7 @@ require (
 	github.com/go-logr/zapr v1.3.0
 	github.com/go-sql-driver/mysql v1.8.1
 	github.com/gobwas/glob v0.2.3
-	github.com/goccy/go-yaml v1.14.3
+	github.com/goccy/go-yaml v1.15.6
 	github.com/golang-migrate/migrate/v4 v4.18.1
 	github.com/google/cel-go v0.21.0
 	github.com/google/go-cmp v0.6.0
diff --git a/go.sum b/go.sum
index a5700047..f0f88495 100644
--- a/go.sum
+++ b/go.sum
@@ -266,8 +266,8 @@ github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y=
 github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJAkT8=
 github.com/goccy/go-json v0.10.3 h1:KZ5WoDbxAIgm2HNbYckL0se1fHD6rz5j4ywS6ebzDqA=
 github.com/goccy/go-json v0.10.3/go.mod h1:oq7eo15ShAhp70Anwd5lgX2pLfOS3QCiwU/PULtXL6M=
-github.com/goccy/go-yaml v1.14.3 h1:8tVD+aqqPLWisSEhM+6wWoiURWXCx6BwaTKS6ZeITgM=
-github.com/goccy/go-yaml v1.14.3/go.mod h1:XBurs7gK8ATbW4ZPGKgcbrY1Br56PdM69F7LkFRi1kA=
+github.com/goccy/go-yaml v1.15.6 h1:gy5kf1yjMia3/c3wWD+u1z3lU5XlhpT8FZGaLJU9cOA=
+github.com/goccy/go-yaml v1.15.6/go.mod h1:XBurs7gK8ATbW4ZPGKgcbrY1Br56PdM69F7LkFRi1kA=
 github.com/gofrs/uuid v4.0.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
 github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
 github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
diff --git a/internal/parser/parser.go b/internal/parser/parser.go
index b0988dcb..69470e04 100644
--- a/internal/parser/parser.go
+++ b/internal/parser/parser.go
@@ -115,13 +115,18 @@ func Unmarshal[T proto.Message](r io.Reader, factory func() T, opts ...Unmarshal
 }
 
 func UnmarshalBytes[T proto.Message](contents []byte, factory func() T, opts ...UnmarshalOpt) (_ []T, _ []SourceCtx, outErr error) {
+	contentLen := len(bytes.TrimSpace(contents))
+	if contentLen == 0 {
+		return nil, nil, nil
+	}
+
 	f, err := parse(contents, true)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	if len(f.Docs) == 0 {
-		if len(bytes.TrimSpace(contents)) > 0 {
+		if contentLen > 0 {
 			// Special case for unterminated strings. See test case 20.
 			return nil, nil, NewUnmarshalError(&sourcev1.Error{
 				Kind:     sourcev1.Error_KIND_PARSE_ERROR,


Test patch:
diff --git a/internal/test/testdata/compile/rule_with_no_roles.yaml b/internal/test/testdata/compile/rule_with_no_roles.yaml
index 8ebf9779..78370409 100644
--- a/internal/test/testdata/compile/rule_with_no_roles.yaml
+++ b/internal/test/testdata/compile/rule_with_no_roles.yaml
@@ -8,5 +8,5 @@ wantErrors:
     position:
       line: 14
       column: 14
-      path: "$.resourcePolicy.rules[2].actions"
+      path: "$.resourcePolicy.rules[2]"
 mainDef: "resource_policies/leave_request_20210210.yaml"
diff --git a/internal/test/testdata/index/duplicate_rule_and_role_names.yaml b/internal/test/testdata/index/duplicate_rule_and_role_names.yaml
index 3e4084da..5dd2e862 100644
--- a/internal/test/testdata/index/duplicate_rule_and_role_names.yaml
+++ b/internal/test/testdata/index/duplicate_rule_and_role_names.yaml
@@ -11,7 +11,7 @@ wantErrList:
         position:
           line: 9
           column: 11
-          path: $.derivedRoles.definitions[1].name
+          path: $.derivedRoles.definitions[1]
     - error: |-
         duplicate action rule name "myrule": action rule #2 for resource "leave_request" has the same name as action rule #1 defined at 9:17
       file: principal.yaml
@@ -21,7 +21,7 @@ wantErrList:
         position:
           line: 16
           column: 17
-          path: $.principalPolicy.rules[0].actions[1].action
+          path: $.principalPolicy.rules[0].actions[1]
     - error: |-
         duplicate rule name "myrule": rule #2 has the same name as rule #1 defined at 8:12
       file: resource.yaml
@@ -31,7 +31,7 @@ wantErrList:
         position:
           line: 13
           column: 12
-          path: $.resourcePolicy.rules[1].actions
+          path: $.resourcePolicy.rules[1]
     - error: |-
         duplicate rule name "myrule": rule #3 has the same name as rule #1 defined at 8:12
       file: resource.yaml
@@ -41,7 +41,7 @@ wantErrList:
         position:
           line: 18
           column: 12
-          path: $.resourcePolicy.rules[2].actions
+          path: $.resourcePolicy.rules[2]
 files:
   "test.txt": |-
     this is some text
diff --git a/internal/test/testdata/inspect/missing_imports.yaml b/internal/test/testdata/inspect/missing_imports.yaml
index 034645d3..e6b7b087 100644
--- a/internal/test/testdata/inspect/missing_imports.yaml
+++ b/internal/test/testdata/inspect/missing_imports.yaml
@@ -17,7 +17,7 @@ inputs:
         local:
           label: '"dude"'
       rules:
-        - actions: [ "approve" ]
+        - actions: ["approve"]
           condition:
             match:
               expr: variables.commonLabel == constants.commonLabel || variables.label == constants.label
@@ -51,7 +51,7 @@ policiesExpectation:
           name: label
           source: resource.leave_request.vdefault
           used: true
-          value: "\"dude\""
+          value: '"dude"'
 
 policySetsExpectation:
   indexBuildErrors:
@@ -62,5 +62,5 @@ policySetsExpectation:
           position:
             column: 12
             line: 12
-            path: $.resourcePolicy.rules[0].actions
+            path: $.resourcePolicy.rules[0]
         file: resource.leave_request.vdefault.yaml
diff --git a/internal/test/testdata/parser/case_004.json b/internal/test/testdata/parser/case_004.json
index dc9bfb87..905ae299 100644
--- a/internal/test/testdata/parser/case_004.json
+++ b/internal/test/testdata/parser/case_004.json
@@ -1,14 +1,14 @@
 {
-    "description": "Invalid type for policyVersion field",
+    "description": "Invalid type for resource field",
     "wantErrors": [
         {
             "kind": "KIND_PARSE_ERROR",
             "position": {
                 "line": 7,
                 "column": 9,
-                "path": "$.resourcePolicy.resource.kind"
+                "path": "$.resourcePolicy.resource"
             },
-            "message": "expected string value got MappingValue"
+            "message": "expected string value got Mapping"
         }
     ],
     "want": [
@@ -22,9 +22,9 @@
                     "position": {
                         "line": 7,
                         "column": 9,
-                        "path": "$.resourcePolicy.resource.kind"
+                        "path": "$.resourcePolicy.resource"
                     },
-                    "message": "expected string value got MappingValue"
+                    "message": "expected string value got Mapping"
                 }
             ]
         }
diff --git a/internal/test/testdata/parser/case_023.json b/internal/test/testdata/parser/case_023.json
index ea3d5951..6f06311a 100644
--- a/internal/test/testdata/parser/case_023.json
+++ b/internal/test/testdata/parser/case_023.json
@@ -4,10 +4,10 @@
         {
             "kind": "KIND_PARSE_ERROR",
             "position": {
-                "line": 10,
+                "line": 9,
                 "column": 7
             },
-            "message": "required ':' and map value"
+            "message": "non-map value is specified"
         }
     ]
 }
diff --git a/internal/test/testdata/parser/case_024.json b/internal/test/testdata/parser/case_024.json
index 7848a41b..85411216 100644
--- a/internal/test/testdata/parser/case_024.json
+++ b/internal/test/testdata/parser/case_024.json
@@ -5,10 +5,10 @@
         {
             "kind": "KIND_PARSE_ERROR",
             "position": {
-                "line": 13,
-                "column": 7
+                "line": 12,
+                "column": 11
             },
-            "message": "required ':' and map value"
+            "message": "non-map value is specified"
         }
     ]
 }