Policy with `--dry-run` flag is overwriting policy

gchappel commented 1 month ago

Summary

We control Conjur via Git repositories, I'm looking to build a pre-commit style check to make sure what you're about to push to a PR for review is in fact a valid policy. We have issues sometimes where reviewing a policy looks good, but when it is merged there may be an issue in the policy which then blocks the policy from being loaded. My intention is that these issues can be caught earlier by using --dry-run to validate the intended Conjur policy, but without actually loading it to Conjur. This flag is documented as:

Fully replace an existing policy.

Examples:
- conjur policy replace -b staging -f /policy/staging.yml

Usage:
  conjur policy replace [flags]

Flags:
  -h, --help   help for replace

Global Flags:
  -b, --branch string   The parent policy branch
  -d, --debug           Debug logging enabled
      --dry-run         Dry run mode (input policy will be validated without applying the changes)
  -f, --file string     The policy file to load

When testing this, I've found that even though I have --dry-run on my command line, my policy in Conjur is being affected and replaced.

Steps to Reproduce

Install via Homebrew (cyberark/tools/conjur-cli)
Verify before policy (see screenshots of our internal custom UI)
Create a valid policy file
Run conjur policy replace --dry-run --file policy.yml --branch policy:my/namespace/gchappel-testing/dry-run-load

See that the output confirms dry run mode was used:

Dry run policy 'policy:my/namespace/gchappel-testing/dry-run-load'
{
"status": "",
"errors": null
}

Verify after policy

Expected Results

Since --dry-run was used, the policy should NOT be changed.

Actual Results

The policy was entirely overridden with the file provided on the command line

Reproducible

[x] Always
[ ] Sometimes
[ ] Non-Reproducible

Version/Tag number

Conjur CLI version 8.0.16-6f9eefb

Environment setup

macOS 15
Conjur CLI version as above
Conjur server 13.3.2 (from curl https://conjur.company.com/info | jq -r '.release'

Additional Information

Original policy:

- !policy
  id: before-dry-run
  body:

Before screenshot: Before

New policy:

- !policy
  id: after-dry-run
  body:

After screenshot: After

CLI demonstration:

$ conjur list
[
  "company:policy:my/namespace/gchappel-testing/dry-run-load",
  "company:policy:my/namespace/gchappel-testing/dry-run-load/old",
  "company:variable:my/namespace/gchappel-testing/dry-run-load/test-variable"
]

$ cat after.yml
- !policy
  id: new
  body:

$ conjur policy replace --dry-run --file after.yml --branch my/namespace/gchappel-testing/dry-run-load
Dry run policy 'my/namespace/gchappel-testing/dry-run-load'
{
  "status": "",
  "errors": null
}

$ conjur list
[
  "company:policy:my/namespace/gchappel-testing/dry-run-load",
  "company:policy:my/namespace/gchappel-testing/dry-run-load/new",
  "company:variable:my/namespace/gchappel-testing/dry-run-load/test-variable"
]

gchappel commented 1 month ago

Bump.

I see now from the release notes that this is a new feature in the 8.0.16 version of the CLI tool - does this need a particular release on the server side? I just accidentally loaded a policy over another policy because I had temporarily forgotten that dry-run isn't working so I thought I'd check in.

gchappel commented 3 weeks ago

Bump, again. Anything the CyberArk team can offer here as to why this is happening, and how I might be able to stop it?

gchappel commented 1 week ago

Any chance of getting eyes on this. It still affects me today. I don't like doing it, but tagging @szh as the top contributor to this repository, who mentions @cyberark in their profile...

szh commented 1 week ago

CC @codihuston

codihuston commented 1 week ago

@gchappel Hello,

This query parameter should be respected on conjur server versions 1.21.1 and above, and should not modify the policy state. Can you confirm the version of conjur that you are running on the server side?

Thanks, -Codi

codihuston commented 1 week ago

@gchappel Ah, I see you mentioned 13.3.2. You'll want to be on 13.4.

jtuttle commented 1 week ago

@codihuston I think this would take some doing, but it would be great if we could have the CLI check the Conjur version and fail w/ a warning if it wasn't compatible. I don't believe there's currently a way to check Conjur version though. That would be a nice thing to add but might be more of a product thing since we'll have to weave it into the API, etc.

szh commented 1 week ago

Any chance of getting eyes on this. It still affects me today. I don't like doing it, but tagging @szh as the top contributor to this repository, who mentions @cyberark in their profile...

@gchappel Please don't apologize - I sincerely appreciate that you tagged me directly. Normally me (and my whole team) would get notified of new GH issues but there's some configuration glitch and we haven't been. I am so sorry that it took this long for us to notice.

gchappel commented 3 days ago

@gchappel Ah, I see you mentioned 13.3.2. You'll want to be on 13.4.

Cool - this does make sense, and I suspected as much as soon as I noticed that the dry run feature is literally brand new and only in the very latest version. My company is somewhat risk-averse so I don't think we'll be on 13.4 any time soon but this doesn't need to be a showstopper. I'm looking to use the dry run feature to create pre-commit hooks to stop people accidentally committing invalid Conjur policies into our automation but now I know I can't do that exactly, I'll just document the hook a different way to make sure there's no risk of overwriting anything

Perhaps in the short-term, the docs could be improved?

the dry run feature isn't called out as having a specific server version requirement
in fact, it doesn't even mention that it contacts your server at all. My first expectation was that the YAML file was simply validated offline in the same way as something like JSON Schemas would

I do feel that given the "silent" nature of this bug, one or both of these ought to be called out otherwise IMO there is a significant risk that someone could read that as being an offline, safe operation, before accidentally overwriting a real policy even though they got a dry run message in response.

@gchappel Please don't apologize - I sincerely appreciate that you tagged me directly. Normally me (and my whole team) would get notified of new GH issues but there's some configuration glitch and we haven't been. I am so sorry that it took this long for us to notice.

Stuff happens :) it's not a huge deal, but if (as it turns out) this is a feature that needs some server side support, I just need to create my hook a slightly different way and document it appropriately

cyberark / conjur-cli-go