6 corpus

README.md

@@ -724,6 +724,13 @@ Other operators are handled accordingly.
 
 The evaluator should eventually pass all Cedar evaluator test using the DRT framework in the `cedar-spec` repo, as well as the `cedar-integration-tests` bundle.
 
+Status: the [`cedar-sql-spec`] fork of `cedar-spec` differentially tests this evaluator against `cedar-policy`'s
+authorizer with the targets `sql-is-authorized-drt` (concrete requests) and `sql-query-drt` (an unknown
+`principal` and/or `resource`, every candidate checked), and runs the `cedar-integration-tests` corpus through
+it. The envelope is: policies that validate strictly, schemas without extension types, and strings without NUL
+characters (which Postgres `text` cannot hold); within it every handwritten integration test and every checked
+corpus test agrees with `cedar-policy`.
+
 ## Status and phases
 
 The implementation is built in phases, one branch and plan document each (`docs/plans/N-slug.md` here, with the

docs/followups/6-corpus.md

@@ -0,0 +1,21 @@
+# Follow-ups for branch 6 — corpus
+
+## PR description
+
+The integration corpus runs through the crate from `cedar-sql-spec` (`docs/plans/cedar-sql-6-corpus.md`):
+of 7497 corpus tests, 4304 (34413 requests) are inside the envelope and all agree with the expected
+results; the 24 handwritten tests all pass. This branch records that in the README, compiles `hasTag` on
+an action type as `false` (found by the corpus), and applies the review of branches 4 and 5.
+
+## Review findings of branches 4 and 5 (fixed here)
+
+- Generated join aliases joined attribute path segments with `.`, which an attribute name may contain.
+- The differential tests matched loader errors on the substring `NUL`; the crate now has `Error::Nul`.
+- Missing tests: a same-type principal and resource both unknown, a candidate type without rows, an enum
+  type, a dotted attribute name.
+
+## Suggested follow-ups
+
+- Extension types (Plan 7): 580 corpus tests and 864 requests wait on them.
+- The enumerated candidates of an enum entity type are its table rows, not its declared values (the same
+  choice `PolicySet::query_resource` makes); worth stating in the README when Plan 7 touches enums.

docs/plans/6-corpus.md

@@ -0,0 +1,23 @@
+# Plan 6 — corpus: results of the integration tests
+
+## Goal
+
+No code: this branch records that the `cedar-integration-tests` corpus runs through the crate
+(`cedar-sql-spec` branch `cedar-sql-6-corpus`) and what its envelope is, in the README.
+
+## Design
+
+- The README's "Testing" section states the envelope (strictly validating policies, no extension types, no
+  NUL strings) and the results, and points to the differential tests.
+
+## Files
+
+`README.md`, `docs/plans/6-corpus.md`.
+
+## Verification
+
+See `cedar-sql-spec`'s `docs/plans/cedar-sql-6-corpus.md`.
+
+## History
+
+New; the sixth `cedar-sql` branch.

src/compile.rs

@@ -654,10 +654,9 @@ impl<'a> Compiler<'a> {
                 if b.repr != Repr::Text {
                     return Err(Error::Unsupported("a non-string tag"));
                 }
-                let Some((_, table)) = self.config.table_for(ety) else {
-                    return Err(Error::Unsupported("tags of an entity type without a table"));
-                };
-                let Some(tags) = table.tags.clone() else {
+                // An entity type without a table (an action type) has no tags either.
+                let tags = self.config.table_for(ety).and_then(|(_, t)| t.tags.clone());
+                let Some(tags) = tags else {
                     if op == BinaryOp::HasTag {
                         // The validator types `hasTag` on a tagless type as `false`.
                         return Ok(plain(
@@ -1037,25 +1036,35 @@ fn root_raw_name(root: &RootKey) -> String {
     }
 }
 
+/// The separator of attribute path segments in generated names: attribute
+/// names may contain `.`, so `principal["b.c"]` and `principal.b.c` must
+/// not collide.
+const SEGMENT: &str = "\x1f";
+
 /// The quoted alias of the join for `path` under `root` (the root itself
 /// for the empty path).
 fn alias_name(root: &RootKey, path: &[SmolStr]) -> String {
     if path.is_empty() {
         return root_name(root);
     }
-    shortened(&format!("{}.{}", root_raw_name(root), path.join("."))).to_string()
+    shortened(&format!(
+        "{}{SEGMENT}{}",
+        root_raw_name(root),
+        path.join(SEGMENT)
+    ))
+    .to_string()
 }
 
 /// The quoted CTE column holding the value of an attribute path.
 fn value_column(path: &[SmolStr]) -> String {
-    shortened(&format!("$v:{}", path.join("."))).to_string()
+    shortened(&format!("$v:{}", path.join(SEGMENT))).to_string()
 }
 
 fn ancestors_name(anchor: &Anchor) -> SQLIdentifier {
     let path = if anchor.path.is_empty() {
         String::new()
     } else {
-        format!(".{}", anchor.path.join("."))
+        format!("{SEGMENT}{}", anchor.path.join(SEGMENT))
     };
     shortened(&format!("{}{path}$ancestors", root_raw_name(&anchor.root)))
 }

src/error.rs

@@ -40,6 +40,10 @@ pub enum Error {
     /// The query returned rows this crate cannot interpret.
     #[error("unexpected query result: {0}")]
     Query(String),
+    /// A string holds a NUL character, which Postgres `text` cannot store: a
+    /// documented limitation, distinguishable from a loader bug.
+    #[error("the string {0:?} contains a NUL character, which Postgres cannot store")]
+    Nul(String),
     /// The entities cannot be turned into rows of the configured tables.
     #[error("cannot load entities: {0}")]
     Load(String),

src/ident.rs

@@ -104,9 +104,7 @@ fn fnv1a(s: &str) -> u64 {
 /// When `s` contains a NUL character, which Postgres `text` cannot store.
 pub fn quoted_literal(s: &str) -> Result<String> {
     if s.contains('\0') {
-        return Err(Error::Load(format!(
-            "the string {s:?} contains a NUL character, which Postgres cannot store"
-        )));
+        return Err(Error::Nul(s.to_owned()));
     }
     Ok(format!("'{}'", s.replace('\'', "''")))
 }

src/load.rs

@@ -257,9 +257,7 @@ pub fn canonical_json(value: &Value) -> Result<serde_json::Value> {
         ValueKind::Lit(Literal::Long(n)) => json!(n),
         ValueKind::Lit(Literal::String(s)) => {
             if s.contains('\0') {
-                return Err(Error::Load(format!(
-                    "the string {s:?} contains a NUL character, which Postgres cannot store"
-                )));
+                return Err(Error::Nul(s.to_string()));
             }
             json!(s.as_str())
         }
@@ -272,9 +270,7 @@ pub fn canonical_json(value: &Value) -> Result<serde_json::Value> {
                 .iter()
                 .map(|(k, v)| {
                     if k.contains('\0') {
-                        return Err(Error::Load(format!(
-                            "the record key {k:?} contains a NUL character, which Postgres cannot store"
-                        )));
+                        return Err(Error::Nul(k.to_string()));
                     }
                     Ok((k.to_string(), canonical_json(v)?))
                 })

tests/authorize_pg.rs

@@ -110,7 +110,7 @@ const D1: &str = "Doc::\"d1\"";
 const GHOST_NAME: &str = "User::\"ghost\".name == \"x\"";
 
 fn permit(condition: &str) -> String {
-    format!("permit(principal, action, resource) when {{ {condition} }};")
+    format!("permit(principal, action == Action::\"view\", resource) when {{ {condition} }};")
 }
 
 fn ids(response: &Response) -> (Vec<String>, Vec<String>) {
@@ -543,7 +543,7 @@ fn hierarchy_and_tags() {
 fn decisions() {
     let mut fx = Fixture::new(true);
     let both = format!(
-        "{}\nforbid(principal, action, resource) when {{ principal.name == \"Alice\" }};",
+        "{}\nforbid(principal, action == Action::\"view\", resource) when {{ principal.name == \"Alice\" }};",
         permit("true")
     );
     let r = fx.check(&both, ALICE, D1);
@@ -569,7 +569,7 @@ fn decisions() {
     assert_eq!(ids(&r), (vec![], vec!["policy0".to_owned()]));
     // A forbid that errors does not deny.
     let forbid_errs = format!(
-        "{}\nforbid(principal, action, resource) when {{ {GHOST_NAME} }};",
+        "{}\nforbid(principal, action == Action::\"view\", resource) when {{ {GHOST_NAME} }};",
         permit("true")
     );
     let r = fx.check(&forbid_errs, ALICE, D1);
@@ -787,6 +787,22 @@ fn review_probes() {
             ALICE,
         );
         allow(&mut fx, "principal in [principal]", ALICE);
+        // An attribute name containing a dot next to the path it spells.
+        allow(
+            &mut fx,
+            "principal has friend && principal.friend has name && principal has \"friend.name\" && principal[\"friend.name\"] == principal.friend.name",
+            ALICE,
+        );
+        deny(
+            &mut fx,
+            "principal has \"friend.name\" && principal[\"friend.name\"] == \"Bob\" && principal has friend && principal.friend has name",
+            CAROL,
+        );
+        allow(
+            &mut fx,
+            "principal has \"friend.name\" && principal[\"friend.name\"] == \"Bob\"",
+            CAROL,
+        );
     }
 }
 
@@ -804,4 +820,14 @@ fn has_tag_on_a_tagless_type() {
         ALICE,
         D1,
     );
+    // An action type has no table and no tags.
+    assert_eq!(
+        fx.clean(
+            &permit("!(Action::\"view\".hasTag(principal.name))"),
+            ALICE,
+            D1
+        )
+        .decision,
+        Decision::Allow
+    );
 }

tests/entities/kitchen_sink.json

@@ -6,7 +6,8 @@
      "groups": ["b", "a", "a"],
      "profile": {"city": "Zürich", "pets": [3, 1, 2], "boss": {"__entity": {"type": "User", "id": "bob"}}},
      "friend": {"__entity": {"type": "User", "id": "bob"}},
-     "friends": [{"__entity": {"type": "User", "id": "bob"}}]
+     "friends": [{"__entity": {"type": "User", "id": "bob"}}],
+     "friend.name": "Bob"
    },
    "parents": [{"type": "Group", "id": "admins"}],
    "tags": {"k": "v", "it's": "q'"}},
@@ -15,7 +16,7 @@
    "parents": []},
   {"uid": {"type": "User", "id": "carol"},
    "attrs": {"name": "Carol", "admin": false, "groups": ["x"], "profile": {"city": "Oslo", "pets": [7]},
-             "friend": {"__entity": {"type": "User", "id": "ghost"}}, "friends": []},
+             "friend": {"__entity": {"type": "User", "id": "ghost"}}, "friends": [], "friend.name": "Bob"},
    "parents": []},
   {"uid": {"type": "User", "id": "weird\"q\\s"},
    "attrs": {"name": "50%_x\\y\nz", "admin": false, "groups": [], "profile": {"city": "", "pets": []}, "friends": []},
@@ -24,5 +25,9 @@
    "attrs": {"owner": {"__entity": {"type": "User", "id": "alice"}}},
    "parents": [{"type": "Group", "id": "admins"}],
    "tags": {"reviewer": {"__entity": {"type": "User", "id": "bob"}}}},
-  {"uid": {"type": "Action", "id": "view"}, "attrs": {}, "parents": []}
+  {"uid": {"type": "Color", "id": "red"}, "attrs": {}, "parents": []},
+  {"uid": {"type": "Action", "id": "view"}, "attrs": {}, "parents": []},
+  {"uid": {"type": "Action", "id": "mate"}, "attrs": {}, "parents": []},
+  {"uid": {"type": "Action", "id": "empty"}, "attrs": {}, "parents": []},
+  {"uid": {"type": "Action", "id": "color"}, "attrs": {}, "parents": []}
 ]

tests/golden/kitchen_sink.sql

@@ -18,6 +18,11 @@ BEGIN
   END IF;
 END
 $cedar$;
+CREATE TABLE "Color" (
+  "__entity_id" TEXT NOT NULL,
+  "__entity_type" TEXT NOT NULL GENERATED ALWAYS AS ('Color') STORED,
+  PRIMARY KEY ("__entity_id")
+);
 CREATE TABLE "Doc" (
   "__entity_id" TEXT NOT NULL,
   "__entity_type" TEXT NOT NULL GENERATED ALWAYS AS ('Doc') STORED,
@@ -30,6 +35,11 @@ CREATE TABLE "Doc_tags" (
   "value" TEXT NOT NULL,
   PRIMARY KEY ("entity_id", "tag")
 );
+CREATE TABLE "Empty" (
+  "__entity_id" TEXT NOT NULL,
+  "__entity_type" TEXT NOT NULL GENERATED ALWAYS AS ('Empty') STORED,
+  PRIMARY KEY ("__entity_id")
+);
 CREATE TABLE "Group" (
   "__entity_id" TEXT NOT NULL,
   "__entity_type" TEXT NOT NULL GENERATED ALWAYS AS ('Group') STORED,
@@ -41,6 +51,7 @@ CREATE TABLE "User" (
   "admin" BOOLEAN NOT NULL,
   "age" BIGINT,
   "friend" TEXT,
+  "friend.name" TEXT,
   "friends" JSONB NOT NULL,
   "groups" JSONB NOT NULL,
   "name" TEXT NOT NULL,

tests/load_pg.rs

@@ -22,8 +22,12 @@ fn round_trip() {
     let json = std::fs::read_to_string("tests/entities/kitchen_sink.json").unwrap();
     let entities = Entities::from_json_str(&json, Some(&schema)).unwrap();
     let load = entities_to_sql(&entities, &schema, &config, &Postgres).unwrap();
-    assert_eq!(load.actions.len(), 1);
-    assert_eq!(load.actions[0].uid().to_string(), "Action::\"view\"");
+    assert_eq!(load.actions.len(), 4);
+    assert!(
+        load.actions
+            .iter()
+            .any(|a| a.uid().to_string() == "Action::\"view\"")
+    );
 
     let mut db = SharedPostgres::get().unwrap().connect().unwrap();
     db.begin().unwrap();