fix(ufw): expose numbered rules before deletion

CHANGELOG.md

@@ -17,6 +17,9 @@ Releases before `0.2.5` predate the public launch; their notes live in the
 - Make Debian stable releases 12 and later eligible, while refusing an unknown
   version and releases below the security-support floor. Debian eligibility is
   separate from live-VM validation; Ubuntu-only actions remain excluded (#238).
+- Let `UfwStatus` return numbered rules with `numbered: true`, retaining verbose
+  output by default. Add `query_ufw_rules` so the planner can read the indices
+  required by `UfwDeleteRule` instead of guessing them (#234).
 - Separate Ubuntu identity requirements from Debian-family mechanisms and
   planner defaults. Canonical services, PPAs and the reboot sentinel require
   Ubuntu itself; portable tools are no longer refused merely for being another

crates/sysknife-brain/src/planning_tools/propose_plan.rs

@@ -357,7 +357,7 @@ this is runtime status, NOT the saved configuration; on Ubuntu the saved config
     ("UfwReset",
      "reset ufw to defaults, removing all rules — no params; Ubuntu only; High risk; irreversible"),
     ("UfwStatus",
-     "show current ufw status and rules — no params; Ubuntu only; read-only"),
+     "show current ufw status and rules — optional param: numbered (boolean, default false); true runs ufw status numbered and exposes rule_number values for UfwDeleteRule, false keeps verbose status; read-only"),
     // ── Ubuntu / distrobox — container environment ────────────────────────────
     ("DistroboxList",
      "list distrobox containers — no params; Ubuntu only; read-only"),
@@ -434,7 +434,7 @@ reports live interface state"),
      "regenerate netplan backend config without applying — no params; Ubuntu only; Medium risk; dry-run before NetplanApply"),
     // ── Ubuntu / Tier 3 — ufw extensions ─────────────────────────────────────
     ("UfwDeleteRule",
-     "delete a ufw rule by number — param: rule_number* (positive integer from 'ufw status numbered'); Ubuntu only; High risk"),
+     "delete a ufw rule by number — param: rule_number* (positive integer from query_ufw_rules or UfwStatus with numbered=true); never guess a rule number, and refresh after rule changes; High risk"),
     ("UfwLimit",
      "add rate-limiting rule on a port/service (>6 connections/30s blocked) — param: target* (e.g. '22' or 'ssh'); Ubuntu only; High risk; use for SSH brute-force mitigation"),
     // ── Ubuntu / Tier 3 — release upgrade ────────────────────────────────────

crates/sysknife-brain/src/planning_tools/query_tools.rs

@@ -8,6 +8,11 @@ use crate::provider::ToolDefinition;
 pub fn query_tools() -> Vec<ToolDefinition> {
     let empty_schema = serde_json::json!({"type": "object", "properties": {}, "required": [], "additionalProperties": false});
     vec![
+        ToolDefinition {
+            name: "query_ufw_rules".into(),
+            description: "Read ufw status numbered to obtain current rule indices for UfwDeleteRule. Never guess an index; query again after any rule change because indices can shift.".into(),
+            input_schema: empty_schema.clone(),
+        },
         ToolDefinition {
             name: "query_services".into(),
             description: "List all running systemd services. Returns one service name per line."
@@ -225,6 +230,7 @@ pub fn query_tool_to_action(
     input: &serde_json::Value,
 ) -> Result<Option<(&'static str, serde_json::Value)>, String> {
     match tool_name {
+        "query_ufw_rules" => Ok(Some(("UfwStatus", serde_json::json!({"numbered": true})))),
         "query_services" => Ok(Some(("ListServices", serde_json::json!({})))),
         "query_firewall" => Ok(Some(("GetFirewallState", serde_json::json!({})))),
         "query_deployments" => Ok(Some(("ListDeployments", serde_json::json!({})))),
@@ -298,6 +304,10 @@ mod tests {
     #[test]
     fn known_query_tools_map_to_actions() {
         let empty = empty_input();
+        assert_eq!(
+            query_tool_to_action("query_ufw_rules", &empty),
+            Ok(Some(("UfwStatus", serde_json::json!({"numbered": true}))))
+        );
         assert_eq!(
             query_tool_to_action("query_services", &empty),
             Ok(Some(("ListServices", serde_json::json!({}))))
@@ -449,9 +459,16 @@ mod tests {
     }
 
     #[test]
-    fn query_tools_returns_twenty_three_definitions() {
+    fn query_tools_returns_twenty_four_definitions() {
         let tools = query_tools();
-        assert_eq!(tools.len(), 23);
+        assert_eq!(tools.len(), 24);
+        let ufw = tools
+            .iter()
+            .find(|tool| tool.name == "query_ufw_rules")
+            .unwrap();
+        assert!(ufw.description.contains("UfwDeleteRule"));
+        assert!(ufw.description.contains("Never guess"));
+        assert_eq!(ufw.input_schema["additionalProperties"], false);
         for tool in &tools {
             assert!(tool.name.starts_with("query_"));
             assert!(!tool.description.is_empty());

crates/sysknife-brain/src/prompt.rs

@@ -903,7 +903,7 @@ const DEBIAN_PARAMS: &str = r#"
 **No params** — use `{}`: AptUpdate, AptAutoremove, AptListInstalled,
 AptListUpgradable, AptHistoryList, CheckPendingReboot,
 GrubGetKargs,
-UfwStatus, UfwEnable, UfwDisable, UfwReset, DistroboxList, NetplanGetConfig,
+UfwEnable, UfwDisable, UfwReset, DistroboxList, NetplanGetConfig,
 NetplanApply, NetplanGenerate,
 ProStatus, ProDetach, LivepatchStatus, MultipassList, UbuntuReleaseUpgrade.
 
@@ -928,8 +928,9 @@ ProStatus, ProDetach, LivepatchStatus, MultipassList, UbuntuReleaseUpgrade.
 - `GrubSetKargs`: `{"append":["quiet","nomodeset"],"delete":["splash"]}` — either list may be `[]` but at least one must be non-empty
 
 **UFW**:
+- `UfwStatus`: `{}` for verbose status, or `{"numbered":true}` for current rule indices. `query_ufw_rules` reads the numbered form during planning.
 - `UfwAllow` / `UfwDeny`: `{"port_or_service":"22/tcp"}` or `{"port_or_service":"ssh"}`
-- `UfwDeleteRule`: `{"rule_number":3}` — positive integer from `ufw status numbered`
+- `UfwDeleteRule`: `{"rule_number":3}` — use a rule number explicitly supplied by the operator or obtained from `query_ufw_rules`; never guess. Query again after any rule change because indices shift.
 - `UfwLimit`: `{"target":"22"}` or `{"target":"ssh"}`
 
 **Netplan**:
@@ -1507,9 +1508,12 @@ mod tests {
     /// that could only fail. The CLI now also refuses such a plan at plan time;
     /// this line is what stops the model proposing one in the first place.
     #[test]
-    fn debian_prompt_states_the_valid_port_range() {
+    fn debian_prompt_states_firewall_input_requirements() {
         let hint = debian_hint();
         let p = build_system_prompt(None, Some(&hint));
+        assert!(p.contains("query_ufw_rules"));
+        assert!(p.contains("{\"numbered\":true}"));
+        assert!(p.contains("indices shift"));
         assert!(
             p.contains("1-65535"),
             "Debian prompt must state the valid port range for ufw rules"

crates/sysknife-daemon/src/actions/ufw.rs

@@ -122,9 +122,21 @@ pub fn ufw_reset() -> ActionSpec {
 ///
 /// Risk: Low / Observer. Read-only; no system changes.
 pub fn ufw_status() -> ActionSpec {
+    ufw_status_with_numbered(false)
+}
+
+/// Show numbered rule indices for `UfwDeleteRule`, or verbose status otherwise.
+pub fn ufw_status_with_numbered(numbered: bool) -> ActionSpec {
     ActionSpec {
         action_name: "UfwStatus",
-        mechanism: command_mechanism("sudo", ["ufw", "status", "verbose"]),
+        mechanism: command_mechanism(
+            "sudo",
+            [
+                "ufw",
+                "status",
+                if numbered { "numbered" } else { "verbose" },
+            ],
+        ),
         risk_level: RiskLevel::Low,
         reboot_required: false,
         rollback_available: false,
@@ -326,6 +338,39 @@ mod tests {
         let (_, args) = extract_args(&spec);
         assert!(args.contains(&"status"));
         assert!(args.contains(&"verbose"));
+        for (numbered, rendering) in [(false, "verbose"), (true, "numbered")] {
+            let spec = ufw_status_with_numbered(numbered);
+            assert_eq!(
+                extract_args(&spec),
+                ("sudo", vec!["ufw", "status", rendering])
+            );
+            assert_eq!(spec.risk_level, RiskLevel::Low);
+            assert!(!spec.reboot_required && !spec.rollback_available);
+        }
+        for (params, rendering) in [
+            (serde_json::json!({}), "verbose"),
+            (serde_json::json!({"numbered": false}), "verbose"),
+            (serde_json::json!({"numbered": true}), "numbered"),
+        ] {
+            let spec = crate::executor::build_action_spec("UfwStatus", &params).unwrap();
+            assert_eq!(
+                extract_args(&spec),
+                ("sudo", vec!["ufw", "status", rendering])
+            );
+        }
+        for invalid in [
+            serde_json::json!("true"),
+            serde_json::json!(1),
+            serde_json::Value::Null,
+        ] {
+            assert!(matches!(
+                crate::executor::build_action_spec(
+                    "UfwStatus",
+                    &serde_json::json!({"numbered": invalid})
+                ),
+                Err(crate::executor::ExecutorError::InvalidParam("numbered"))
+            ));
+        }
     }
 
     #[test]

crates/sysknife-daemon/src/executor.rs

@@ -1608,7 +1608,15 @@ pub fn build_action_spec(action_name: &str, params: &Value) -> Result<ActionSpec
             Ok(ufw::ufw_deny(&port_or_service))
         }
         "UfwReset" => Ok(ufw::ufw_reset()),
-        "UfwStatus" => Ok(ufw::ufw_status()),
+        "UfwStatus" => {
+            let numbered = match params.get("numbered") {
+                None => false,
+                Some(value) => value
+                    .as_bool()
+                    .ok_or(ExecutorError::InvalidParam("numbered"))?,
+            };
+            Ok(ufw::ufw_status_with_numbered(numbered))
+        }
 
         // ── distrobox ────────────────────────────────────────────────────
         "DistroboxList" => Ok(distrobox::distrobox_list()),

docs/action-reference.md

@@ -338,8 +338,8 @@ Every row is derived from the live code: the command from each action's `ActionS
 | `UfwAllow` | `sudo ufw allow 22` | High | Ubuntu | – | – | allow inbound traffic on a port or service — param: port_or_service\* (e.g. 22, 22/tcp, OpenSSH); Ubuntu only; High risk |
 | `UfwDeny` | `sudo ufw deny 23` | High | Ubuntu | – | – | deny inbound traffic on a port or service — param: port_or_service\*; Ubuntu only; High risk |
 | `UfwReset` | `sudo ufw --force reset` | High | Ubuntu | – | – | reset ufw to defaults, removing all rules — no params; Ubuntu only; High risk; irreversible |
-| `UfwStatus` | `sudo ufw status verbose` | Low | Ubuntu | – | – | show current ufw status and rules — no params; Ubuntu only; read-only |
-| `UfwDeleteRule` | `sudo ufw --force delete 1` | High | Ubuntu | – | – | delete a ufw rule by number — param: rule_number\* (positive integer from 'ufw status numbered'); Ubuntu only; High risk |
+| `UfwStatus` | `sudo ufw status verbose` | Low | Ubuntu | – | – | show current ufw status and rules — optional param: numbered (boolean, default false); true runs ufw status numbered and exposes rule_number values for UfwDeleteRule, false keeps verbose status; read-only |
+| `UfwDeleteRule` | `sudo ufw --force delete 1` | High | Ubuntu | – | – | delete a ufw rule by number — param: rule_number\* (positive integer from query_ufw_rules or UfwStatus with numbered=true); never guess a rule number, and refresh after rule changes; High risk |
 | `UfwLimit` | `sudo ufw limit 22` | High | Ubuntu | – | – | add rate-limiting rule on a port/service (>6 connections/30s blocked) — param: target\* (e.g. '22' or 'ssh'); Ubuntu only; High risk; use for SSH brute-force mitigation |
 
 ## netplan

docs/typed-actions.md

@@ -6,6 +6,15 @@ allowlist or a regex over a shell command string. SysKnife answers it
 differently — the model is never given a channel to produce a shell string
 in the first place.
 
+## Reading ufw rule numbers
+
+For ufw rule deletion, read current indices with `UfwStatus` and
+`{"numbered":true}` (`sysknife_ufw_status` in MCP). During planning,
+`query_ufw_rules` performs that read. The default `{}` still returns verbose
+status. Pass the selected index to `UfwDeleteRule`; refresh the listing after
+rule changes, because indices shift. This does not make listing and deletion
+atomic or change the deletion approval requirement.
+
 ## Why string allowlists fail
 
 The independent security research known as **GuardFall** tested AI coding