Skip to content

Commit 9bf308a

Browse files
Fix formatter soundness check (#865)
Signed-off-by: Shaobo He <[email protected]>
1 parent 501b008 commit 9bf308a

File tree

1 file changed

+58
-7
lines changed
  • cedar-policy-formatter/src/pprint

1 file changed

+58
-7
lines changed

cedar-policy-formatter/src/pprint/fmt.rs

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@
1414
* limitations under the License.
1515
*/
1616

17+
use std::collections::BTreeMap;
18+
1719
use miette::{miette, Result, WrapErr};
1820

19-
use cedar_policy_core::ast::{PolicySet, Template};
21+
use cedar_policy_core::ast::PolicySet;
2022
use cedar_policy_core::parser::parse_policyset;
2123
use cedar_policy_core::parser::{err::ParseErrors, text_to_cst::parse_policies};
24+
use smol_str::ToSmolStr;
2225

2326
use crate::token::get_comment;
2427

@@ -42,15 +45,19 @@ fn tree_to_pretty<T: Doc>(t: &T, context: &mut config::Context<'_>) -> Result<St
4245
fn soundness_check(ps: &str, ast: &PolicySet) -> Result<()> {
4346
let formatted_ast = parse_policyset(ps).wrap_err("formatter produces invalid policies")?;
4447
let (formatted_policies, policies) = (
45-
formatted_ast.templates().collect::<Vec<&Template>>(),
46-
ast.templates().collect::<Vec<&Template>>(),
48+
formatted_ast
49+
.policies()
50+
.map(|p| (p.id().to_smolstr(), p))
51+
.collect::<BTreeMap<_, _>>(),
52+
ast.policies()
53+
.map(|p| (p.id().to_smolstr(), p))
54+
.collect::<BTreeMap<_, _>>(),
4755
);
4856

4957
if formatted_policies.len() != policies.len() {
5058
return Err(miette!("missing formatted policies"));
5159
}
52-
53-
for (f_p, p) in formatted_policies.into_iter().zip(policies.into_iter()) {
60+
for ((f_p_id, f_p), (p_id, p)) in formatted_policies.into_iter().zip(policies.into_iter()) {
5461
let (f_anno, anno) = (
5562
f_p.annotations()
5663
.map(|(k, v)| (k, &v.val))
@@ -59,7 +66,8 @@ fn soundness_check(ps: &str, ast: &PolicySet) -> Result<()> {
5966
.map(|(k, v)| (k, &v.val))
6067
.collect::<std::collections::BTreeMap<_, _>>(),
6168
);
62-
if !(f_anno == anno
69+
if !(f_p_id == p_id
70+
&& f_anno == anno
6371
&& f_p.effect() == p.effect()
6472
&& f_p.principal_constraint() == p.principal_constraint()
6573
&& f_p.action_constraint() == p.action_constraint()
@@ -69,7 +77,7 @@ fn soundness_check(ps: &str, ast: &PolicySet) -> Result<()> {
6977
.eq_shape(p.non_scope_constraints()))
7078
{
7179
return Err(miette!(
72-
"policies differ in meaning or annotations:\noriginal: {p}\nformatted: {f_p}"
80+
"policies differ in policy ids or meaning or annotations:\noriginal: {p}\nformatted: {f_p}"
7381
));
7482
}
7583
}
@@ -135,6 +143,49 @@ mod tests {
135143

136144
use super::*;
137145

146+
#[test]
147+
fn test_soundness_check() {
148+
let p1 = r#"permit (principal, action, resource)
149+
when { "
150+
151+
a
152+
" };"#;
153+
let p2 = r#"permit (principal, action, resource)
154+
when { "
155+
a
156+
" };"#;
157+
assert!(soundness_check(p2, &parse_policyset(p1).unwrap()).is_err());
158+
159+
let p1 = r#"
160+
permit (principal, action, resource)
161+
when { "a"};
162+
permit (principal, action, resource)
163+
when { "
164+
165+
a
166+
" };"#;
167+
let p2 = r#"
168+
permit (principal, action, resource)
169+
when { "
170+
a
171+
" };
172+
permit (principal, action, resource)
173+
when { "a"};"#;
174+
assert!(soundness_check(p2, &parse_policyset(p1).unwrap()).is_err());
175+
176+
let p1 = r#"
177+
permit (principal, action, resource)
178+
when { "a" };
179+
permit (principal, action, resource)
180+
when { "b" };"#;
181+
let p2 = r#"
182+
permit (principal, action, resource)
183+
when { "a" };
184+
permit (principal, action, resource)
185+
when { "b"};"#;
186+
assert!(soundness_check(p2, &parse_policyset(p1).unwrap()).is_ok());
187+
}
188+
138189
#[test]
139190
fn test_format_files() {
140191
let config = Config {

0 commit comments

Comments
 (0)