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

Return an validation warning when we know a policy always evaluates to true #1263

Open
2 tasks
john-h-kastner-aws opened this issue Oct 4, 2024 · 1 comment
Labels
feature-request This issue requets a substantial new feature

Comments

@john-h-kastner-aws
Copy link
Contributor

john-h-kastner-aws commented Oct 4, 2024

Category

Cedar validation features/changes

Describe the feature you'd like to request

We already have this for policies that evaluate to false, and a policy that always applies is potentially more problematic.

E.g.,

permit(principal, action, resource);

is always true, so we should return a warning

A slightly complex example might be

forbid(principal, action == Action::"view", resource) when {
  principal != Group::"jane-family"
};

When the view action applies to users and not groups.

Note that, like the warning for false policies, this does not aim to be a sound analysis. There will be many policies that are always true which we will not detect (without using our SMT based analysis tools).


Two possible concerns:

  1. Users who want to emulate default-allow in Cedar might intentionally include a policy permit(principal, action, resource);. but the proposed change is a warning, so they can freely ignore it.
  2. We encourage users to write multiple simple policies, but the proposed change would not detect a set of policies which together always evaluate to at least one true. This isn't necessarily a problem because the proposed warning does not aim to be sound, but it would lead to some mistakes we should be able to notice being missed when split between policies. The might instead be run against the disjunction of all policy expressions, short-circuiting is a problem here because || short-circuits on true, but policy evaluation does not.

Describe alternatives you've considered

.

Additional context

No response

Is this something that you'd be interested in working on?

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change
@john-h-kastner-aws john-h-kastner-aws added pending-triage The cedar maintainers haven't looked at this yet. Automicaly added to all new issues. feature-request This issue requets a substantial new feature and removed pending-triage The cedar maintainers haven't looked at this yet. Automicaly added to all new issues. labels Oct 4, 2024
@john-h-kastner-aws
Copy link
Contributor Author

Related to #103. Implementing that issue would very mean doing this one at the same time, but this issue is substantially simpler, taking a day or two rather than up to a week or two.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request This issue requets a substantial new feature
Projects
None yet
Development

No branches or pull requests

1 participant