fix(server): stop assignee updates bypassing the assign capability

crates/jirakeep-core/src/policy.rs

@@ -16,15 +16,40 @@ use anyhow::{bail, Context as _};
 use chrono::{DateTime, Duration, Utc};
 use serde_json::Value;
 
-/// A capability a policy grant can carry.
-///
-/// The only implication is `read` ⇒ `summary` (invariant I6), applied in
-/// [`Access::allows`] — never stored in the set itself.
-#[derive(
-    Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, serde::Serialize, serde::Deserialize,
-)]
-#[serde(rename_all = "snake_case")]
-pub enum Capability {
+/// Declares [`Capability`] and [`Capability::ALL`] from a single variant
+/// list, so `ALL` structurally cannot omit a variant: adding a capability
+/// here extends the enum and `ALL` in the same edit, and every exhaustive
+/// `match` on [`Capability`] elsewhere (e.g. the I7 capability-owned-fields
+/// map in the server) fails to compile until it is revisited. A variant
+/// that existed but was missing from `ALL` would be grantable by `restrict`
+/// rules yet silently skipped by every `ALL`-driven check — this macro makes
+/// that state unrepresentable.
+macro_rules! capabilities {
+    ($($(#[$meta:meta])* $variant:ident,)+) => {
+        /// A capability a policy grant can carry.
+        ///
+        /// The only implication is `read` ⇒ `summary` (invariant I6), applied in
+        /// [`Access::allows`] — never stored in the set itself.
+        #[derive(
+            Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, serde::Serialize, serde::Deserialize,
+        )]
+        #[serde(rename_all = "snake_case")]
+        pub enum Capability {
+            $($(#[$meta])* $variant,)+
+        }
+
+        impl Capability {
+            /// Every capability, in declaration order. Used to expand `allow`
+            /// grants and to enumerate capability-owned fields (I7).
+            /// Generated from the same variant list as the enum itself, so it
+            /// can never fall out of sync with the variant set.
+            pub const ALL: [Capability; [$(Capability::$variant),+].len()] =
+                [$(Capability::$variant),+];
+        }
+    };
+}
+
+capabilities! {
     /// Full issue details (implies [`Capability::Summary`], I6).
     Read,
     /// Redacted summary-only view of an issue.
@@ -54,23 +79,6 @@ pub enum Capability {
 }
 
 impl Capability {
-    /// Every capability, in declaration order. Used to expand `allow` grants.
-    pub const ALL: [Capability; 13] = [
-        Capability::Read,
-        Capability::Summary,
-        Capability::Comments,
-        Capability::History,
-        Capability::Attachments,
-        Capability::Comment,
-        Capability::Status,
-        Capability::Fields,
-        Capability::Assign,
-        Capability::Watchers,
-        Capability::Links,
-        Capability::Create,
-        Capability::Attach,
-    ];
-
     /// Whether this capability permits mutating Jira state.
     pub fn is_write(self) -> bool {
         matches!(

crates/jirakeep/Cargo.toml

@@ -71,4 +71,8 @@ tokio = { version = "1", features = [
     "process",
     "time",
 ] }
+# MCP integration tests through the library server (DESIGN.md testing bar):
+# an rmcp client on an in-process duplex transport, wiremock standing in
+# for Jira. Dev-only; the shipped binary gains no client-side code.
+rmcp = { version = "3.1", features = ["client"] }
 wiremock = "0.6"

crates/jirakeep/src/server.rs

@@ -92,6 +92,68 @@ fn note_refused(ctx: &RequestContext<RoleServer>) {
     }
 }
 
+/// Privileged Jira `fields` keys no capability grants through any tool (I7).
+const PRIVILEGED_FIELDS: &[&str] = &["security", "project", "reporter"];
+
+/// Jira `fields` keys whose mutation is owned by `cap`'s dedicated gate
+/// rather than the generic `fields` gate.
+///
+/// `update_issue_fields` (gated on [`Capability::Fields`]) refuses every key
+/// returned here (I7): granting `fields` must never exercise a capability the
+/// operator withheld. `assignee` belongs to `assign_issue`
+/// ([`Capability::Assign`]); `resolution` belongs to the status gate
+/// ([`Capability::Status`], "transition status / resolve / mark duplicate"),
+/// because Jira's edit endpoint sets any edit-screen field and Resolution can
+/// be placed on the edit screen; `parent` re-parents the issue — a hierarchy
+/// and visibility change owned by the linking gate ([`Capability::Links`],
+/// I11/I14), not a plain field edit.
+///
+/// The match is deliberately exhaustive, and [`Capability::ALL`] (which
+/// [`refused_field`] iterates) is generated from the same variant list as
+/// the enum: adding a [`Capability`] variant fails compilation here, forcing
+/// a decision about which fields it owns, and that decision cannot be
+/// skipped at runtime by an out-of-date `ALL`.
+fn capability_owned_fields(cap: Capability) -> &'static [&'static str] {
+    match cap {
+        // Read-side capabilities set nothing.
+        Capability::Read
+        | Capability::Summary
+        | Capability::Comments
+        | Capability::History
+        | Capability::Attachments => &[],
+        // These writes go through dedicated endpoints or request sections,
+        // never the edit endpoint's `fields` object.
+        Capability::Comment | Capability::Watchers | Capability::Create | Capability::Attach => &[],
+        // The generic field gate itself reserves no keys.
+        Capability::Fields => &[],
+        // Transitions use the transitions endpoint, but `resolution` is an
+        // ordinary edit-screen field on instances that expose it, so a bare
+        // `fields` grant could otherwise resolve or mark-duplicate an issue.
+        Capability::Status => &["resolution"],
+        Capability::Assign => &["assignee"],
+        Capability::Links => &["parent"],
+    }
+}
+
+/// First key in `fields` that `update_issue_fields` must refuse (I7): a
+/// privileged field no capability grants, or a field owned by a dedicated
+/// capability's own gate.
+fn refused_field(fields: &serde_json::Map<String, Value>) -> Option<&'static str> {
+    for banned in PRIVILEGED_FIELDS.iter().copied() {
+        if fields.contains_key(banned) {
+            return Some(banned);
+        }
+    }
+    for cap in Capability::ALL {
+        for owned in capability_owned_fields(cap).iter().copied() {
+            if fields.contains_key(owned) {
+                return Some(owned);
+            }
+        }
+    }
+    None
+}
+
 // --- params -----------------------------------------------------------------
 
 #[derive(Debug, Deserialize, schemars::JsonSchema)]
@@ -162,7 +224,9 @@ pub struct AssignParams {
 #[derive(Debug, Deserialize, schemars::JsonSchema)]
 pub struct UpdateFieldsParams {
     pub key: String,
-    /// Fields object (Jira REST shape). Security level changes are rejected.
+    /// Fields object (Jira REST shape). Privileged fields (security, project,
+    /// reporter) and fields owned by a dedicated capability (assignee,
+    /// parent) are rejected.
     pub fields: Value,
 }
 
@@ -906,7 +970,7 @@ impl JiraKeep {
     }
 
     #[tool(
-        description = "Update issue fields (Jira fields object). Rejects security level and project smuggling.",
+        description = "Update issue fields (Jira fields object). Rejects privileged fields (security, project, reporter) and fields owned by a dedicated capability: assignee (use assign_issue), resolution (use transition_issue), and parent.",
         annotations(read_only_hint = false, open_world_hint = true)
     )]
     async fn update_issue_fields(
@@ -922,15 +986,13 @@ impl JiraKeep {
         {
             return Ok(deny);
         }
-        // I7: block privileged field smuggling.
+        // I7: block privileged field smuggling and capability bypass.
         if let Some(obj) = p.fields.as_object() {
-            for banned in ["security", "project", "reporter"] {
-                if obj.contains_key(banned) {
-                    note_refused(&ctx);
-                    return Ok(err_text(format!(
-                        "field \"{banned}\" cannot be set through update_issue_fields"
-                    )));
-                }
+            if let Some(banned) = refused_field(obj) {
+                note_refused(&ctx);
+                return Ok(err_text(format!(
+                    "field \"{banned}\" cannot be set through update_issue_fields"
+                )));
             }
         } else {
             note_refused(&ctx);
@@ -1273,6 +1335,56 @@ mod tests {
         }
     }
 
+    #[test]
+    fn update_fields_refuses_capability_owned_keys() {
+        // I7: `assignee` is owned by Capability::Assign (assign_issue),
+        // `resolution` by Capability::Status (Jira's edit endpoint sets any
+        // edit-screen field, so this would resolve/mark-duplicate), and
+        // `parent` by the linking gate — a `fields` grant alone must never
+        // exercise any of them.
+        let fields = json!({"assignee": {"accountId": "5b10a"}});
+        assert_eq!(refused_field(fields.as_object().unwrap()), Some("assignee"));
+        let fields = json!({"resolution": {"name": "Done"}});
+        assert_eq!(
+            refused_field(fields.as_object().unwrap()),
+            Some("resolution")
+        );
+        let fields = json!({"summary": "s", "parent": {"key": "OPS-1"}});
+        assert_eq!(refused_field(fields.as_object().unwrap()), Some("parent"));
+    }
+
+    #[test]
+    fn update_fields_still_refuses_privileged_fields() {
+        for banned in ["security", "project", "reporter"] {
+            let mut obj = serde_json::Map::new();
+            obj.insert(banned.to_owned(), json!({"id": "1"}));
+            assert_eq!(refused_field(&obj), Some(banned));
+        }
+    }
+
+    #[test]
+    fn update_fields_permits_ordinary_edits() {
+        let fields = json!({
+            "summary": "new summary",
+            "labels": ["triage"],
+            "priority": {"name": "High"},
+        });
+        assert_eq!(refused_field(fields.as_object().unwrap()), None);
+    }
+
+    #[test]
+    fn every_capability_owned_field_is_refused() {
+        // Structural pin: any field a capability owns must be refused by the
+        // generic fields gate, so a future capability cannot reopen I7.
+        for cap in Capability::ALL {
+            for owned in capability_owned_fields(cap).iter().copied() {
+                let mut obj = serde_json::Map::new();
+                obj.insert(owned.to_owned(), json!("x"));
+                assert_eq!(refused_field(&obj), Some(owned), "capability {cap:?}");
+            }
+        }
+    }
+
     #[test]
     fn read_only_removes_write_tools() {
         let s = test_server(true);