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

CLI cedar format does not tolerate policy files with trailing newline #1217

Closed
3 tasks done
adamrothman opened this issue Sep 19, 2024 · 2 comments · Fixed by #1220
Closed
3 tasks done

CLI cedar format does not tolerate policy files with trailing newline #1217

adamrothman opened this issue Sep 19, 2024 · 2 comments · Fixed by #1220
Labels
papercut Small annoyances in the Cedar SDK. Lower priority fixes than bugs. Smaller than a fature request

Comments

@adamrothman
Copy link

adamrothman commented Sep 19, 2024

Before opening, please confirm:

Bug Category

Other

Describe the bug

Given a file containing the following policy (taken from the Cedar docs), with a trailing newline:

// Jane's friends can view all photos in her janeTrips album
permit (
  principal in Group::"janeFriends",
  action in [Action::"view", Action::"comment"],
  resource in Album::"janeTrips"
);

The cedar format --check command exits with a non-0 status:

$ cedar format --policies ./test.cedar --line-width 80 --indent-width 2 --check --error-format human
// Jane's friends can view all photos in her janeTrips album
permit (
  principal in Group::"janeFriends",
  action in [Action::"view", Action::"comment"],
  resource in Album::"janeTrips"
);

adam.rothman at adamrot-ltmnvzm in ~/Desktop 
$ echo $?
1

Expected behavior

The format command should not treat a policy file that ends with a newline as malformed or in need of formatting. It's very common for editors to automatically add a trailing newline to files on save, and quite a pain to try to disable this feature for only .cedar files.

When invoked with --check, policy files that end with a trailing newline should pass (i.e. exit status 0).

When invoked with --write, the CLI should not remove trailing newlines from policy files where they are present.

Reproduction steps

  1. Install cedar-policy-cli crate.
  2. Save the example policy above into a file called test.cedar, ensuring that a trailing newline is added.
  3. Invoke cedar format --check.
  4. Check the exit code of the previous command.
$ cargo install cedar-policy-cli  
    Updating crates.io index
  Downloaded cedar-policy-cli v4.0.0
  Downloaded 1 crate (43.5 KB) in 0.22s
  Installing cedar-policy-cli v4.0.0
<snip>
   Compiling cedar-policy-cli v4.0.0
    Finished `release` profile [optimized] target(s) in 34.42s
  Installing /Users/adam.rothman/.cargo/bin/cedar
   Installed package `cedar-policy-cli v4.0.0` (executable `cedar`)

$ cedar -V
cedar-policy-cli 4.0.0

$ cat test.cedar 
// Jane's friends can view all photos in her janeTrips album
permit (
  principal in Group::"janeFriends",
  action in [Action::"view", Action::"comment"],
  resource in Album::"janeTrips"
);

$ cat -e test.cedar 
// Jane's friends can view all photos in her janeTrips album$
permit ($
  principal in Group::"janeFriends",$
  action in [Action::"view", Action::"comment"],$
  resource in Album::"janeTrips"$
);$

$ cedar format --policies ./test.cedar --line-width 80 --indent-width 2 --check --error-format human
// Jane's friends can view all photos in her janeTrips album
permit (
  principal in Group::"janeFriends",
  action in [Action::"view", Action::"comment"],
  resource in Album::"janeTrips"
);

$ echo $?
1

Code Snippet

No response

Log output

No response

Additional configuration

No response

Operating System

macOS Sonoma 14.7

Additional information and screenshots

No response

@adamrothman adamrothman added bug Something isn't working. This is as high priority issue. pending-triage The cedar maintainers haven't looked at this yet. Automicaly added to all new issues. labels Sep 19, 2024
@khieta
Copy link
Contributor

khieta commented Sep 20, 2024

Thanks for opening this issue! That behavior does indeed sound annoying. Should be a quick fix -- I'll take a look now.

@khieta khieta added work-in-progress papercut Small annoyances in the Cedar SDK. Lower priority fixes than bugs. Smaller than a fature request and removed bug Something isn't working. This is as high priority issue. pending-triage The cedar maintainers haven't looked at this yet. Automicaly added to all new issues. labels Sep 20, 2024
@khieta
Copy link
Contributor

khieta commented Sep 23, 2024

Fixed by #1220. This change will naturally be included in the next minor version (4.1.0). But let us know if you need it asap and we can release a patch version 4.0.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
papercut Small annoyances in the Cedar SDK. Lower priority fixes than bugs. Smaller than a fature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants