From c0964387f175e6867f54461259bd5e05ebbdd58a Mon Sep 17 00:00:00 2001
From: Antoine Pietri <antoine.pietri1@gmail.com>
Date: Thu, 5 Dec 2024 16:30:33 +0100
Subject: [PATCH] Fix a crash in mismatched output check for nested rules
 (#1086)

Minimal repro:

    rule:
      match:
        - condition: "1 < 0"
          output: "true"
        - condition: "1 > 0"
          rule:
            match:
              - output: "'false'"

This would segfault the Policy Compiler, because it would try to access
`.Output().SourceID()` of the second block to log an error, but
`.Output()` is nil for nested rules.
---
 policy/compiler.go                            |  8 ++++++-
 policy/helper_test.go                         |  8 ++++++-
 .../nested_incompatible_outputs/config.yaml   | 15 ++++++++++++
 .../nested_incompatible_outputs/policy.yaml   | 23 +++++++++++++++++++
 4 files changed, 52 insertions(+), 2 deletions(-)
 create mode 100644 policy/testdata/nested_incompatible_outputs/config.yaml
 create mode 100644 policy/testdata/nested_incompatible_outputs/policy.yaml

diff --git a/policy/compiler.go b/policy/compiler.go
index 0b7010c1..93505ff9 100644
--- a/policy/compiler.go
+++ b/policy/compiler.go
@@ -378,7 +378,13 @@ func (c *compiler) checkMatchOutputTypesAgree(rule *CompiledRule, iss *cel.Issue
 		// Handle assignability as the output type is assignable to the match output or vice versa.
 		// During composition, this is roughly how the type-checker will handle the type agreement check.
 		if !(outputType.IsAssignableType(matchOutputType) || matchOutputType.IsAssignableType(outputType)) {
-			iss.ReportErrorAtID(m.Output().SourceID(), "incompatible output types: match block has output type %s, but previous match blocks have output type %s", matchOutputType, outputType)
+			sourceID := m.SourceID()
+			if m.Output() != nil {
+				sourceID = m.Output().SourceID()
+			} else if m.NestedRule() != nil {
+				sourceID = m.NestedRule().SourceID()
+			}
+			iss.ReportErrorAtID(sourceID, "incompatible output types: block has output type %s, but previous outputs have type %s", matchOutputType, outputType)
 			return
 		}
 	}
diff --git a/policy/helper_test.go b/policy/helper_test.go
index a0d2d171..0729dc9a 100644
--- a/policy/helper_test.go
+++ b/policy/helper_test.go
@@ -206,7 +206,7 @@ ERROR: testdata/errors/policy.yaml:38:75: Syntax error: extraneous input ']' exp
 ERROR: testdata/errors/policy.yaml:41:67: undeclared reference to 'format' (in container '')
  |         "invalid values provided on one or more labels: %s".format([variables.invalid])
  | ..................................................................^
-ERROR: testdata/errors/policy.yaml:45:16: incompatible output types: match block has output type string, but previous match blocks have output type bool
+ERROR: testdata/errors/policy.yaml:45:16: incompatible output types: block has output type string, but previous outputs have type bool
  |       output: "'false'"
  | ...............^`,
 		},
@@ -233,6 +233,12 @@ ERROR: testdata/errors_unreachable/policy.yaml:36:13: match creates unreachable
  |           - output: |
  | ............^`,
 		},
+		{
+			name: "nested_incompatible_outputs",
+			err: `ERROR: testdata/nested_incompatible_outputs/policy.yaml:22:9: incompatible output types: block has output type string, but previous outputs have type bool
+ |         match:
+ | ........^`,
+		},
 	}
 )
 
diff --git a/policy/testdata/nested_incompatible_outputs/config.yaml b/policy/testdata/nested_incompatible_outputs/config.yaml
new file mode 100644
index 00000000..71a30729
--- /dev/null
+++ b/policy/testdata/nested_incompatible_outputs/config.yaml
@@ -0,0 +1,15 @@
+# Copyright 2024 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "nested_rule"
diff --git a/policy/testdata/nested_incompatible_outputs/policy.yaml b/policy/testdata/nested_incompatible_outputs/policy.yaml
new file mode 100644
index 00000000..87557df8
--- /dev/null
+++ b/policy/testdata/nested_incompatible_outputs/policy.yaml
@@ -0,0 +1,23 @@
+# Copyright 2024 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: nested_incompatible_outputs
+rule:
+  match:
+    - condition: "1 < 0"
+      output: "true"
+    - condition: "1 > 0"
+      rule:
+        match:
+          - output: "'false'"