Azure / ShieldGuard

Enables best security practices for your project from day zero.
MIT License
8 stars 6 forks source link

Duplicated failure/warning results for repeated rules #25

Closed bcho closed 1 year ago

bcho commented 1 year ago

Using following policy:

package main

deny_name[msg] {
    input.name == "foo"
    msg := sprintf("%s is not allowed", [input.name])
}

deny_name[msg] {
    input.name == "bar"
    msg := sprintf("%s is not allowed", [input.name])
}

deny_name[msg] {
    input.name == "baz"
    msg := sprintf("%s is not allowed", [input.name])
}

against following data:

{"name": "foo"}

will produce 3 duplicated failures:

[
  {
    "filename": "data.json",
    "namespace": "main",
    "success": 0,
    "failures": [
      {
        "query": "data.main.deny_name",
        "rule": {
          "name": "name"
        },
        "message": "foo is not allowed"
      },
      {
        "query": "data.main.deny_name",
        "rule": {
          "name": "name"
        },
        "message": "foo is not allowed"
      },
      {
        "query": "data.main.deny_name",
        "rule": {
          "name": "name"
        },
        "message": "foo is not allowed"
      }
    ],
    "warnings": [],
    "exceptions": []
  }
]

Since the other 2 deny rules are not being called, we should not count them as new failure results.

everjing commented 1 year ago

Another test case which fails under current code:

Using following policy:

package main

deny_name[msg] {
    input.name == "foo" #1 success
    msg := sprintf("%s is not allowed", [input.name])
}

warn_name[msg] {
    input.name == "foo" # 1 success
    msg := sprintf("%s is not allowed", [input.name])
}

deny_name[msg] { # 2 success
    input.name == "baz"
    msg := sprintf("%s is not allowed", [input.name])
}

deny_other[msg] { #1 success
    input.name == "foo"
    msg := sprintf("%s is not allowed", [input.name])
}

against following data:

{"name": "foo"}, {"name": "is-not-foo"}

Will produce 4 success, 2 failures and 1 warning:

[
  {
    "filename": "tempDataAndPolicy/foo.yaml",
    "namespace": "main",
    "success": 4,
    "failures": [
      {
        "query": "data.main.deny_name",
        "rule": {
          "name": "name"
        },
        "message": "foo is not allowed"
      },
      {
        "query": "data.main.deny_other",
        "rule": {
          "name": "other"
        },
        "message": "foo is not allowed"
      }
    ],
    "warnings": [
      {
        "query": "data.main.warn_name",
        "rule": {
          "name": "name"
        },
        "message": "foo is not allowed"
      }
    ],
    "exceptions": []
  }
]
Error: test failed: found 2 failure(s), 1 warning(s)

Sucess number should be 5 according to analyze below:

package main

deny_name[msg] {
    input.name == "foo" # 1 success for "is-not-foo"
    msg := sprintf("%s is not allowed", [input.name])
}

warn_name[msg] {
    input.name == "foo" # 1 success for "is-not-foo"
    msg := sprintf("%s is not allowed", [input.name])
}

deny_name[msg] { # 2 success for "foo" and "is-not-foo"
    input.name == "baz"
    msg := sprintf("%s is not allowed", [input.name])
}

deny_other[msg] { # 1 success for "is-not-foo"
    input.name == "foo"
    msg := sprintf("%s is not allowed", [input.name])
}

We should return 5 success instead of 4.