Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] empty exists != null don't work as documented #377

Closed
yukihiko-shinoda opened this issue Jun 28, 2023 · 2 comments
Closed

[BUG] empty exists != null don't work as documented #377

yukihiko-shinoda opened this issue Jun 28, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@yukihiko-shinoda
Copy link

Describe the bug

  1. Actually, empty can be used to check if string value queries have an empty string ("") defined.
  2. Actually, exists can't be used in place of != null.
  3. != null returns strange result.

cloudformation-guard/docs/CLAUSES.md at a3992cac13d227453945afe8af199faad4e7ca10 · aws-cloudformation/cloudformation-guard · GitHub

empty can’t be used to check if string value queries have an empty string ("") defined.
exists - Checks if each occurrence of the query has a value and can be used in place of != null.

To Reproduce

Actually, empty can be used to check if string value queries have an empty string ("") defined.

test.guard:

Resources empty

test.yml:

- name: Empty string
  input:
    Resources: ''
  expectations:
    rules:
      default: PASS
- name: Not empty string
  input:
    Resources: foo
  expectations:
    rules:
      default: FAIL

cfn-guard test -d guard-files -v:

Testing Guard File guard-files/test.guard
Test Case #1
Name: Empty string
`- File(, Status=PASS)[Context=File(rules=1)]
   `- Rule(default, Status=PASS)[Context=default]
      `- GuardClauseBlock(Status = PASS)[Context=GuardAccessClause#block Resources EMPTY  ]
         `- GuardClauseValueCheck(Status=PASS)[Context= Resources EMPTY  ]
  PASS Rules:
    default: Expected = PASS

Test Case #2
Name: Not empty string
`- File(, Status=FAIL)[Context=File(rules=1)]
   `- Rule(default, Status=FAIL)[Context=default]
      `- GuardClauseBlock(Status = FAIL)[Context=GuardAccessClause#block Resources EMPTY  ]
         `- GuardClauseUnaryCheck(Status=FAIL, Comparison= EMPTY, Value-At=(resolved, Path=/Resources[L:0,C:0] Value="foo"))[Context= Resources EMPTY  ]
  PASS Rules:
    default: Expected = FAIL

Actually, exists can't be used in place of != null.

test.guard:

Resources exists

test.yml:

- name: Not exist
  input: {}
  expectations:
    rules:
      default: FAIL
- name: Empty string
  input:
    Resources: ''
  expectations:
    rules:
      default: PASS
- name: Empty array
  input:
    Resources: []
  expectations:
    rules:
      default: PASS
- name: Empty dictionary
  input:
    Resources: {}
  expectations:
    rules:
      default: PASS
- name: 'Null'
  input:
    Resources: null
  expectations:
    rules:
      default: FAIL

cfn-guard test -d guard-files -v:

Testing Guard File guard-files/test.guard
Test Case #1
Name: Not exist
`- File(, Status=FAIL)[Context=File(rules=1)]
   `- Rule(default, Status=FAIL)[Context=default]
      `- GuardClauseBlock(Status = FAIL)[Context=GuardAccessClause#block Resources EXISTS  ]
         `- GuardClauseUnaryCheck(Status=FAIL, Comparison= EXISTS, Value-At=(unresolved, Path=[L:0,C:0] Value={}))[Context= Resources EXISTS  ]
  PASS Rules:
    default: Expected = FAIL

Test Case #2
Name: Empty string
`- File(, Status=PASS)[Context=File(rules=1)]
   `- Rule(default, Status=PASS)[Context=default]
      `- GuardClauseBlock(Status = PASS)[Context=GuardAccessClause#block Resources EXISTS  ]
         `- GuardClauseValueCheck(Status=PASS)[Context= Resources EXISTS  ]
  PASS Rules:
    default: Expected = PASS

Test Case #3
Name: Empty array
`- File(, Status=PASS)[Context=File(rules=1)]
   `- Rule(default, Status=PASS)[Context=default]
      `- GuardClauseBlock(Status = PASS)[Context=GuardAccessClause#block Resources EXISTS  ]
         `- GuardClauseValueCheck(Status=PASS)[Context= Resources EXISTS  ]
  PASS Rules:
    default: Expected = PASS

Test Case #4
Name: Empty dictionary
`- File(, Status=PASS)[Context=File(rules=1)]
   `- Rule(default, Status=PASS)[Context=default]
      `- GuardClauseBlock(Status = PASS)[Context=GuardAccessClause#block Resources EXISTS  ]
         `- GuardClauseValueCheck(Status=PASS)[Context= Resources EXISTS  ]
  PASS Rules:
    default: Expected = PASS

Test Case #5
Name: Null
`- File(, Status=PASS)[Context=File(rules=1)]
   `- Rule(default, Status=PASS)[Context=default]
      `- GuardClauseBlock(Status = PASS)[Context=GuardAccessClause#block Resources EXISTS  ]
         `- GuardClauseValueCheck(Status=PASS)[Context= Resources EXISTS  ]
  FAIL Rules:
    default: Expected = FAIL, Evaluated = [PASS]

!= null returns strange result.

test.guard:

Resources != null

test.yml:

- name: Not exist
  input: {}
  expectations:
    rules:
      default: FAIL
- name: Empty string
  input:
    Resources: ''
  expectations:
    rules:
      default: PASS
- name: Empty array
  input:
    Resources: []
  expectations:
    rules:
      default: PASS
- name: Empty dictionary
  input:
    Resources: {}
  expectations:
    rules:
      default: PASS
- name: 'Null'
  input:
    Resources: null
  expectations:
    rules:
      default: FAIL

cfn-guard test -d guard-files -v:

Testing Guard File guard-files/test.guard
Test Case #1
Name: Not exist
`- File(, Status=FAIL)[Context=File(rules=1)]
   `- Rule(default, Status=FAIL)[Context=default]
      `- GuardClauseBlock(Status = FAIL)[Context=GuardAccessClause#block Resources not EQUALS  "NULL"]
         `- GuardClauseBinaryCheck(Status=FAIL, Comparison=not EQUALS, from=(unresolved, Path=[L:0,C:0] Value={}), to=)[Context= Resources not EQUALS  "NULL"]
  PASS Rules:
    default: Expected = FAIL

Test Case #2
Name: Empty string
`- File(, Status=FAIL)[Context=File(rules=1)]
   `- Rule(default, Status=FAIL)[Context=default]
      `- GuardClauseBlock(Status = FAIL)[Context=GuardAccessClause#block Resources not EQUALS  "NULL"]
         `- GuardClauseBinaryCheck(Status=FAIL, Comparison=not EQUALS, from=(resolved, Path=/Resources[L:0,C:0] Value=""), to=(resolved, Path=[L:0,C:0] Value="NULL"))[Context= Resources not EQUALS  "NULL"]
  FAIL Rules:
    default: Expected = PASS, Evaluated = [FAIL]

Test Case #3
Name: Empty array
`- File(, Status=PASS)[Context=File(rules=1)]
   `- Rule(default, Status=PASS)[Context=default]
      `- GuardClauseBlock(Status = PASS)[Context=GuardAccessClause#block Resources not EQUALS  "NULL"]
  PASS Rules:
    default: Expected = PASS

Test Case #4
Name: Empty dictionary
`- File(, Status=FAIL)[Context=File(rules=1)]
   `- Rule(default, Status=FAIL)[Context=default]
      `- GuardClauseBlock(Status = FAIL)[Context=GuardAccessClause#block Resources not EQUALS  "NULL"]
         `- GuardClauseBinaryCheck(Status=FAIL, Comparison=not EQUALS, from=(resolved, Path=/Resources[L:0,C:0] Value={}), to=(resolved, Path=[L:0,C:0] Value="NULL"))[Context= Resources not EQUALS  "NULL"]
  FAIL Rules:
    default: Expected = PASS, Evaluated = [FAIL]

Test Case #5
Name: Null
`- File(, Status=FAIL)[Context=File(rules=1)]
   `- Rule(default, Status=FAIL)[Context=default]
      `- GuardClauseBlock(Status = FAIL)[Context=GuardAccessClause#block Resources not EQUALS  "NULL"]
         `- GuardClauseBinaryCheck(Status=FAIL, Comparison=not EQUALS, from=(resolved, Path=/Resources[L:0,C:0] Value="NULL"), to=(resolved, Path=[L:0,C:0] Value="NULL"))[Context= Resources not EQUALS  "NULL"]
  PASS Rules:
    default: Expected = FAIL

Expected behavior

  • Fix documentation
  • != null should FAIL only when query failed or it's null

Operating System

Debian (Docker Container)

OS Version

GNU/Linux 11 bullseye (Docker Container)

Additional context

Tested by cfn-guard 2.1.4

@yukihiko-shinoda yukihiko-shinoda added the bug Something isn't working label Jun 28, 2023
@joshfried-aws
Copy link
Contributor

Hi @yukihiko-shinoda thank you for posting this detailed issue. I have begun to look into this and I believe the first point isn't a bug, and something that was improved upon but the docs were never updated.

As for the other 2 issues these are indeed strange behaviours. I will continue to track them as bugs and will update this issue as progress is made.

Thanks

@joshfried-aws
Copy link
Contributor

Hi @yukihiko-shinoda this issue has been taken care of as part of #398. We have now added the new is_null operator. The documentation has also been updated to clearly state the expected behaviour of the exists operator.

You can get this functionality currently by building from source or waiting for our next stable release.

I am going to go ahead and close out this issue. Feel free to reopen if need be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants