open-policy-agent / conftest

Write tests against structured configuration data using the Open Policy Agent Rego query language
https://conftest.dev
Other
2.82k stars 298 forks source link

Bug: conftest pull breaks policy files if a file name is conflicted #957

Open suzuki-shunsuke opened 1 month ago

suzuki-shunsuke commented 1 month ago

How to reproduce

$ conftest -v
Conftest: 0.53.0
OPA: 0.65.0
  1. Pull a policy. https://raw.githubusercontent.com/open-policy-agent/conftest/master/examples/spdx/policy/policy.rego
conftest pull https://raw.githubusercontent.com/open-policy-agent/conftest/master/examples/spdx/policy/policy.rego
$ cat policy/policy.rego 
package main

deny[msg] {
    expected_data_license := "conftest-demo"
    input.CreationInfo.DataLicense != expected_data_license
    msg := sprintf("DataLicense should be %d, but found %d", [expected_data_license, input.CreationInfo.DataLicense])
}
  1. Pull a same name policy. https://raw.githubusercontent.com/open-policy-agent/conftest/master/examples/cyclonedx/policy/policy.rego
conftest pull https://raw.githubusercontent.com/open-policy-agent/conftest/master/examples/cyclonedx/policy/policy.rego

Then the command exits successfully but the policy is broken.

$ echo $?
0

$ cat policy/policy.rego
package main

deny[msg] {
    expected_data_license := "conftest-demo"
    input.CreationInfo.DataLicense != expected_data_license
    msg := sprintf("DataLicense should be %d, but found %d", [expected_data_license, input.CreationInfo.DataLicense])
}
o expected SHA256 %s", [input.metadata.component.version, expected_shas256]
    )
}% 

Expected behaviour

There are several options.

  1. Command fails and the file isn't changed
  2. Command succeeds but outputs warning and the file isn't changed
  3. Command succeeds but outputs warning and the file is overwritten

I think the option 1 is intuitive.

Actual behaviour

Command succeeds but breaks the file.

boranx commented 6 days ago

yeah, option 1 sounds reasonable. I think the governance of the policies should still be on people rather than conftest managing them due to the single responsibility principle