ImportValue in Output Value check W6001 is bugged

[lessa-t-winston]

CloudFormation Lint Version

v1.9.5

What operating system are you using?

RHEL, Ubuntu

Describe the bug

Initially we received the following error on a template that passed under cfn-lint v.1.3.5, but not under v1.9.2 or v.1.9.5: E1016 Exception 'deque index out of range' raised while validating 'fn_importvalue' complex.cfn.yaml:519:5

This template has an output with a value containing an ImportValue statement, and that's what's at the line of code pointed to by the error. Because of this, our call to cfn-lint includes --ignore-checks W6001, but that must filter the output instead of blocking the checks from running, because I eventually tracked the issue to a bug in the rules/output/ImportValue.py file for W6001:

   if len(validator.context.path.path) >= 2:
      if (
         validator.context.path.path[0] == "Outputs"
         and validator.context.path.path[2] == "Value"
      ):

The if statement checks whether the length of validator.context.path.path is greater than or equal to 2, but then proceeds to try and access validator.context.path.path[2], which causes the deque index error when the length of validator.context.path.path is equal to 2.

The above code is the same in v1.3.5, though, and we didn't experience this issue with that version, so I did some more digging. It looks like this change made the above bug possible to trigger:

diff --git a/src/cfnlint/rules/outputs/Value.py b/src/cfnlint/rules/outputs/Value.py
index 3125be6c6..4d86d9fb8 100644
--- a/src/cfnlint/rules/outputs/Value.py
+++ b/src/cfnlint/rules/outputs/Value.py
@@ -23,14 +23,25 @@ class Value(CfnLintJsonSchema):

     def __init__(self):
         super().__init__(
-            keywords=["Outputs/*/Value"],
+            keywords=["Outputs/*"],
             all_matches=True,
         )

I confirmed if I changed the above-mentioned if statement to check for >= 3 or if I changed keywords=["Outputs/*"] back to keywords=["Outputs/*/Value"], the test would properly proc W6001, or pass with --ignore-checks W6001.

I'm filing this as a bug report instead of a PR because I'm unsure of the reason for the change to the keywords variable, which might require the logic of the if statement to be further changed depending on whether "Value" will ever appear in validator.context.path.path, and I didn't want to risk mangling things further.

Expected behavior

W6001 would be triggered, or ignored when passing --ignore-checks W6001

Reproduction template

AWSTemplateFormatVersion: 2010-09-09

Parameters:

  ComplexName:
    Type: String
    AllowedPattern: '[a-z0-9]+'
 
  VpcStackName:
    Type: String

Resources:
  # omitted

Outputs:

  VPC:
    Value: 
      Fn::ImportValue:
        !Sub ${VpcStackName}-VPC
    Export:
      Name: !Sub ${ComplexName}-VPC

[lessa-t-winston]

Thank you, the quick fix is appreciated. At the same time, the issue of the if statement checking for >= 2 and then accessing index 2 is still there and could cause issues in the future.

[kddejong]

Put into the next PR along with a little smarter logic