fix(cedar): always use ForgeDB::Document as resource entity type

crates/cli/src/tui.rs

@@ -297,7 +297,8 @@ impl App {
             let col = &self.collections[i];
             // Explicitly ask for JSON — without this header, the server defaults
             // to application/msgpack, which reqwest's .json() can't decode.
-            let req = self.client
+            let req = self
+                .client
                 .get(format!("{}/v1/{}?limit=50", self.url, col))
                 .header(reqwest::header::ACCEPT, "application/json");
             let req = if let Some(t) = &self.bearer_token {
@@ -311,7 +312,11 @@ impl App {
                     if let Some(data) = json.get("data").and_then(|d| d.as_array()) {
                         self.docs = data.clone();
                         // Reset doc selection if we switched collections or the index is now out of range.
-                        if self.docs_state.selected().is_none_or(|s| s >= self.docs.len()) {
+                        if self
+                            .docs_state
+                            .selected()
+                            .is_none_or(|s| s >= self.docs.len())
+                        {
                             self.docs_state.select(if !self.docs.is_empty() {
                                 Some(0)
                             } else {
@@ -406,13 +411,16 @@ impl App {
                             modified = true;
                         }
 
-                        if let Some(actions) =
-                            ns.get_mut("actions").and_then(|a| a.as_object_mut())
+                        if let Some(actions) = ns.get_mut("actions").and_then(|a| a.as_object_mut())
                         {
                             for action in ["Read", "Write", "Delete"] {
-                                if let Some(act) = actions.get_mut(action).and_then(|a| a.as_object_mut())
-                                    && let Some(applies) = act.get_mut("appliesTo").and_then(|ap| ap.as_object_mut())
-                                    && let Some(rt) = applies.get_mut("resourceTypes").and_then(|r| r.as_array_mut())
+                                if let Some(act) =
+                                    actions.get_mut(action).and_then(|a| a.as_object_mut())
+                                    && let Some(applies) =
+                                        act.get_mut("appliesTo").and_then(|ap| ap.as_object_mut())
+                                    && let Some(rt) = applies
+                                        .get_mut("resourceTypes")
+                                        .and_then(|r| r.as_array_mut())
                                 {
                                     let val = serde_json::Value::String(coll_name_cap.clone());
                                     if !rt.contains(&val) {
@@ -825,19 +833,20 @@ async fn run_app<B: Backend>(terminal: &mut Terminal<B>, app: &mut App) -> io::R
                             app.screen = AppScreen::NewUser;
                         }
                         KeyCode::Char('e') => {
-                            // Gotta make sure we actually have a document and a collection selected.
-                            // It's a bit defensive, sure, but I've seen too many TUIs blow up because
-                            // they assumed state that wasn't there. Trust but verify.
                             if let (Some(i), Some(col_idx)) =
                                 (app.docs_state.selected(), app.collections_state.selected())
-                                && let Some(doc) = app.docs.get(i)
+                                && let Some(wrapper) = app.docs.get(i)
                             {
-                                let id = doc
+                                let id = wrapper
                                     .get("id")
                                     .and_then(|v| v.as_str())
                                     .map(|s| s.to_string());
                                 let col = app.collections[col_idx].clone();
-                                let buf = serde_json::to_string_pretty(doc).unwrap_or_default();
+                                // Server returns {id, doc} wrappers — edit only the inner object.
+                                // Showing the wrapper confused users and caused redundant id fields
+                                // on re-save. Fall back to the full wrapper if doc is missing.
+                                let inner = wrapper.get("doc").unwrap_or(wrapper);
+                                let buf = serde_json::to_string_pretty(inner).unwrap_or_default();
 
                                 app.editor = Some(EditorState {
                                     buffer: buf,

crates/query/src/context.rs

@@ -1,12 +1,17 @@
 //! Request context for Cedar policy evaluation.
 //!
-//! Every single time a user hits the database to touch a document, we have to build a Cedar
-//! `Request` and pass it to the engine. The `AuthContext` struct holds the raw strings —
-//! usually yanked straight from PASETO tokens or the route itself — and handles the slightly
-//! tedious, but necessary, conversion into typed Cedar `EntityUid` objects.
+//! Every database hit builds a Cedar `Request` from this struct — principal comes
+//! from the PASETO sub claim, action from the HTTP verb, resource from the route.
 //!
-//! We pass this thing by reference (`&AuthContext`) on the hot path. Why? To keep allocations
-//! as low as Cedar's API physically allows. Allocating in the query path is just asking for a bad time.
+//! One design choice worth flagging: all resources are always typed `ForgeDB::Document`.
+//! The entity ID carries the full `"collection"` or `"collection/id"` string for
+//! fine-grained policy discrimination. We could map each collection to its own entity
+//! type (which the dynamic schema hot-reload work sets up), but that's a v0.3 feature;
+//! doing it at the context level caused a nasty class of bugs where `collection == "user"`
+//! produced `ForgeDB::User::\"user\"` — a *principal* type, not a resource type — and
+//! Cedar's strict request validation blew up before any policy ever ran.
+//!
+//! Kept by reference on the hot path to avoid gratuitous clones.
 
 use cedar_policy::{Context, EntityUid, Request};
 
@@ -52,39 +57,30 @@ impl AuthContext {
 
     /// Converts this context into a strictly typed Cedar `Request`.
     ///
+    /// Resources are always typed `ForgeDB::Document` regardless of the collection name.
+    /// The full `"collection"` or `"collection/id"` string lives in the entity ID so Cedar
+    /// policies can still discriminate at whatever granularity they want:
+    ///
+    /// ```cedar
+    /// // Permit reads on the whole "posts" collection
+    /// permit(principal, action == ForgeDB::Action::"Read",
+    ///        resource == ForgeDB::Document::"posts");
+    ///
+    /// // Or a single document
+    /// permit(principal, action == ForgeDB::Action::"Read",
+    ///        resource == ForgeDB::Document::"posts/abc-123");
+    /// ```
+    ///
     /// # Errors
     ///
-    /// Returns [`ForgeError::Policy`] if the principal, action, or resource
-    /// strings can't be wrangled into valid Cedar `EntityUid`s. For instance,
-    /// if some client sends over malicious characters that Cedar outright rejects in entity IDs.
+    /// Returns [`ForgeError::Policy`] if the action string is unrecognized or if Cedar
+    /// rejects malformed characters in the principal/resource IDs after escaping.
     pub fn to_cedar_request(&self, schema: Option<&cedar_policy::Schema>) -> Result<Request> {
-        let parts: Vec<&str> = self.resource.splitn(2, '/').collect();
-        let entity_type = if parts.len() == 2 {
-            // "users/123" -> entity_type = "users", entity_id = "123"
-            parts[0]
-        } else {
-            // "users" -> entity_type = "users", entity_id = "*"
-            parts[0]
-        };
-        let entity_type_cap = match entity_type {
-            "_" => "Document".to_string(), // fallback
-            other => {
-                // Capitalize first letter to match Cedar conventions usually
-                let mut c = other.chars();
-                match c.next() {
-                    None => String::new(),
-                    Some(f) => f.to_uppercase().collect::<String>() + c.as_str(),
-                }
-            }
-        };
-
         let principal_eid = format!(r#"ForgeDB::User::"{}""#, cedar_escape(&self.principal));
         let action_eid = format!(r#"ForgeDB::Action::"{}""#, cedar_escape(&self.action));
-        let resource_eid = format!(
-            r#"ForgeDB::{}::"{}""#,
-            entity_type_cap,
-            cedar_escape(&self.resource)
-        );
+
+        // Always Document — see module doc for why we stopped deriving from the collection name.
+        let resource_eid = format!(r#"ForgeDB::Document::"{}""#, cedar_escape(&self.resource));
 
         let p_uid: EntityUid = principal_eid
             .parse()
@@ -122,12 +118,27 @@ mod tests {
             req.action().unwrap().to_string(),
             r#"ForgeDB::Action::"Read""#
         );
+        // Resource is always ForgeDB::Document regardless of what the collection name is.
         assert_eq!(
             req.resource().unwrap().to_string(),
             r#"ForgeDB::Document::"document/123""#
         );
     }
 
+    #[test]
+    fn collection_named_user_resolves_as_document_not_principal_type() {
+        let ctx = AuthContext::new("admin", "Read", "user");
+        let req = ctx
+            .to_cedar_request(None)
+            .expect("collection named 'user' must not blow up Cedar request construction");
+
+        assert_eq!(
+            req.resource().unwrap().to_string(),
+            r#"ForgeDB::Document::"user""#,
+            "resource must always be Document, never a principal type"
+        );
+    }
+
     #[test]
     fn context_handles_complex_strings() {
         let ctx = AuthContext::new("user_name@domain.com", "Write", "a/b/c/d");