Skip to content

Commit

Permalink
Fix a crash in mismatched output check for nested rules (#1086)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
seirl authored Dec 5, 2024
1 parent 000958d commit c096438
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 2 deletions.
8 changes: 7 additions & 1 deletion policy/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
8 changes: 7 additions & 1 deletion policy/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
| ...............^`,
},
Expand All @@ -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:
| ........^`,
},
}
)

Expand Down
15 changes: 15 additions & 0 deletions policy/testdata/nested_incompatible_outputs/config.yaml
Original file line number Diff line number Diff line change
@@ -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"
23 changes: 23 additions & 0 deletions policy/testdata/nested_incompatible_outputs/policy.yaml
Original file line number Diff line number Diff line change
@@ -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'"

0 comments on commit c096438

Please sign in to comment.