Add tools to rename and annotate behind --allow-write

* Without them, an agent could not record what it understood (the
  hypothesis -> name -> re-read loop of RE), short of raw commands, and
  its findings did not show up in later disassembly or decompilation.
* Add `rename_function` (afn), `rename_variable` (afvn), `set_comment`
  (CC, replacing unless `append`) and `remove_comment` (CC-), `set_flag`
  (f+, since `f` refuses an address that already has a flag) and
  `remove_flag` (f-), and `set_function_signature` (afs). Names, comments
  and prototypes are passed with `quote_arg()`; flag names must be
  `[A-Za-z0-9_.:]`. Each tool resolves the location to an address first
  and returns the state Rizin stored.
* `f`, `afvn` and others report success while only logging an error
  ("cannot find var by name"), so these tools also fail on a logged error.
* They change the analysis, not the file, so a read-only file is fine,
  but as the QA plan asks they are offered only with `--allow-write`.
* A location that is neither an address nor a flag is rejected also where
  the pipe cannot report the status of `s @ <loc>` (before Rizin 0.10).

README.md

@@ -123,6 +123,12 @@ produces the full listing, which the server then slices.
   `debug_registers`, `debug_breakpoint`/`debug_breakpoint_remove`,
   `debug_breakpoints`, `debug_backtrace`, `debug_maps`, `debug_status`
 
+**Annotating** (needs `--allow-write`; changes the analysis, not the file)
+- `rename_function`, `rename_variable`, `set_comment` / `remove_comment`,
+  `set_flag` / `remove_flag`, `set_function_signature` — record what was
+  understood so later disassembly and decompilation show it. Each returns
+  the state Rizin stored, and fails when Rizin only logs an error.
+
 **Writing** (disabled by default — see *Security*)
 - `write_hex`, `write_string`, `write_assembly` (instructions separated by `;`
   or newlines, assembled for the destination address). Every write is read
@@ -332,9 +338,9 @@ Two capabilities are gated:
 
 A tool whose capability is disabled is left out of `tools/list` altogether
 (and refused if called anyway), so clients do not offer or try it: the raw
-tools with `--no-raw`, the write tools (including `flirt_create`, which
-writes a file) without `--allow-write`, and the `debug_*` tools without
-`--allow-debug`.
+tools with `--no-raw`, the write and annotation tools (including
+`flirt_create`, which writes a file) without `--allow-write`, and the
+`debug_*` tools without `--allow-debug`.
 
 Addresses and flag names passed to tools are validated against a strict
 character set, and every command is checked for the NUL byte (and, for the pipe

src/params.rs

@@ -186,6 +186,68 @@ pub struct SearchBytesParams {
     pub page: PageParams,
 }
 
+/// Parameters for `rename_function`.
+#[derive(Debug, Deserialize, JsonSchema)]
+pub struct RenameFunctionParams {
+    /// A location inside the function (hex address or flag/symbol name).
+    pub at: String,
+    /// The new name, e.g. `parse_args`.
+    pub name: String,
+}
+
+/// Parameters for `rename_variable`.
+#[derive(Debug, Deserialize, JsonSchema)]
+pub struct RenameVariableParams {
+    /// A location inside the function (hex address or flag/symbol name).
+    pub at: String,
+    /// The current name of the argument or local variable (see
+    /// `function_variables`).
+    pub old_name: String,
+    /// The new name.
+    pub new_name: String,
+}
+
+/// Parameters for `set_comment`.
+#[derive(Debug, Deserialize, JsonSchema)]
+pub struct SetCommentParams {
+    /// Address (hex) or flag/symbol name.
+    pub address: String,
+    /// The comment text (one line).
+    pub text: String,
+    /// Append to an existing comment instead of replacing it. Defaults to
+    /// `false`.
+    #[serde(default)]
+    pub append: Option<bool>,
+}
+
+/// Parameters for `set_flag`.
+#[derive(Debug, Deserialize, JsonSchema)]
+pub struct SetFlagParams {
+    /// Flag name, made of `[A-Za-z0-9_.:]`, e.g. `crypto.key_schedule`.
+    pub name: String,
+    /// Address (hex) or flag/symbol name the flag marks.
+    pub address: String,
+    /// Size in bytes of the flagged item. Defaults to 1.
+    #[serde(default)]
+    pub size: Option<u64>,
+}
+
+/// Parameters for `remove_flag`.
+#[derive(Debug, Deserialize, JsonSchema)]
+pub struct FlagNameParams {
+    /// Flag name.
+    pub name: String,
+}
+
+/// Parameters for `set_function_signature`.
+#[derive(Debug, Deserialize, JsonSchema)]
+pub struct SetSignatureParams {
+    /// A location inside the function (hex address or flag/symbol name).
+    pub at: String,
+    /// C prototype, e.g. `int parse_args(int argc, char **argv)`.
+    pub signature: String,
+}
+
 /// Parameters for `write_hex` (write mode only).
 #[derive(Debug, Deserialize, JsonSchema)]
 pub struct WriteHexParams {

src/server.rs

@@ -26,8 +26,8 @@ use crate::session::{AnalysisLevel, SessionManager};
 use crate::status::CmdStatus;
 use crate::util::{
     at_suffix, clamp_count, gadget_matches_exactly, parse_hex_bytes, parse_instructions,
-    parse_rop_query, quote_arg, to_hex, validate_algo, validate_location, validate_number,
-    validate_path,
+    parse_rop_query, quote_arg, to_hex, validate_algo, validate_flag_name, validate_location,
+    validate_number, validate_path,
 };
 
 /// Resources exposed by the server: `(uri, display name, rizin JSON command)`.
@@ -58,6 +58,8 @@ Listings are paged: they return `items` with `total` and, when more remain, \
 page size). `decompile` uses rz-ghidra or jsdec when installed (see `decompilers`), else built-in \
 pseudo-disassembly. Name library functions with the `flirt_*` tools (FLIRT signatures) \
 and, when the rz-silhouette plugin is configured, with `silhouette`. \
+With --allow-write, record findings with `rename_function`, `rename_variable`, \
+`set_comment`, `set_flag`, and `set_function_signature`. \
 Dynamic analysis is available via the `debug_*` tools when the server is started with \
 --allow-debug. Addresses may be hex (e.g. 0x401000) or flag/symbol names (e.g. main, \
 sym.imp.printf). For anything not covered by a dedicated tool, use `command` (text) or \
@@ -75,6 +77,18 @@ pub const WRITE_TOOLS: &[&str] = &[
     "flirt_create",
 ];
 
+/// The tools that change the analysis (names, comments, flags, types),
+/// offered only with `--allow-write`. They do not modify the file.
+pub const ANNOTATION_TOOLS: &[&str] = &[
+    "rename_function",
+    "rename_variable",
+    "set_comment",
+    "remove_comment",
+    "set_flag",
+    "remove_flag",
+    "set_function_signature",
+];
+
 /// The debugger tools, offered only with `--allow-debug`.
 pub const DEBUG_TOOLS: &[&str] = &[
     "debug_start",
@@ -108,6 +122,7 @@ impl RizinServer {
         for (enabled, tools) in [
             (config.allow_raw, RAW_TOOLS),
             (config.allow_write, WRITE_TOOLS),
+            (config.allow_write, ANNOTATION_TOOLS),
             (config.allow_debug, DEBUG_TOOLS),
         ] {
             if !enabled {
@@ -224,6 +239,77 @@ impl RizinServer {
         Ok(())
     }
 
+    /// Guard used by the annotation tools: they change the analysis, not the
+    /// file, so a read-only file is fine.
+    async fn ensure_annotate(&self) -> Result<(), ErrorData> {
+        if !self.manager.config().allow_write {
+            return Err(RizinError::NotPermitted(
+                "annotation tools are disabled; start the server with --allow-write".into(),
+            )
+            .to_error_data());
+        }
+        if !self.manager.state().await.open {
+            return Err(RizinError::NoFileOpen.to_error_data());
+        }
+        Ok(())
+    }
+
+    /// Run a command that changes state, failing also when Rizin logs an
+    /// error but reports success, as `f`, `wx` and `Fs` do.
+    async fn cmd_strict(&self, cmd: &str) -> Result<String, ErrorData> {
+        let out = self.manager.exec(cmd).await.mcp()?;
+        let out = crate::backend::check(cmd, out).mcp()?;
+        let errors: Vec<&str> = out
+            .messages
+            .iter()
+            .map(String::as_str)
+            .filter(|m| m.starts_with("ERROR"))
+            .collect();
+        if !errors.is_empty() {
+            return Err(RizinError::CommandFailed {
+                cmd: cmd.to_string(),
+                status: CmdStatus::Error,
+                detail: errors.join("\n"),
+            }
+            .to_error_data());
+        }
+        Ok(out.output)
+    }
+
+    /// The address `loc` (a hex address or flag) stands for, as `0x...`.
+    async fn resolve(&self, loc: &str) -> Result<String, ErrorData> {
+        let loc = validate_location(loc).mcp()?;
+        let addr = self.manager.cmd(&format!("s @ {loc}")).await.mcp()?;
+        let addr = addr.trim();
+        // A pipe to Rizin before 0.10 prints nothing for an unknown flag.
+        if !addr.starts_with("0x") {
+            return Err(RizinError::InvalidArgument {
+                arg: loc,
+                reason: "not a known address or flag".into(),
+            }
+            .to_error_data());
+        }
+        Ok(addr.to_string())
+    }
+
+    /// The start address and `afij` entry of the function covering `loc`.
+    async fn function_at(&self, loc: &str) -> Result<(String, Value), ErrorData> {
+        let addr = self.resolve(loc).await?;
+        let info = self.manager.cmdj(&format!("afij @ {addr}")).await.mcp()?;
+        let Some(func) = info.as_array().and_then(|a| a.first()).cloned() else {
+            return Err(RizinError::InvalidArgument {
+                arg: loc.to_string(),
+                reason: "no function there (analyze it first, e.g. with analyze_function)".into(),
+            }
+            .to_error_data());
+        };
+        let start = func
+            .get("offset")
+            .and_then(Value::as_u64)
+            .map_or(addr, |o| format!("0x{o:x}"));
+        Ok((start, func))
+    }
+
     /// Guard used by the raw command tools.
     fn ensure_raw(&self) -> Result<(), ErrorData> {
         if !self.manager.config().allow_raw {
@@ -982,6 +1068,155 @@ impl RizinServer {
         ))
     }
 
+    // --- Annotation (guarded by --allow-write) ----------------------------
+
+    /// Rename a function (`afn`).
+    #[tool(
+        description = "Rename the function containing a location; returns the name Rizin \
+        stored. Changes the analysis only, not the file. Requires --allow-write (afn)."
+    )]
+    async fn rename_function(
+        &self,
+        Parameters(p): Parameters<RenameFunctionParams>,
+    ) -> Result<CallToolResult, ErrorData> {
+        self.ensure_annotate().await?;
+        let (addr, before) = self.function_at(&p.at).await?;
+        let name = quote_arg(&p.name).mcp()?;
+        self.cmd_strict(&format!("afn {name} @ {addr}")).await?;
+        let (_, after) = self.function_at(&addr).await?;
+        Ok(self.json_result(json!({
+            "function": addr,
+            "previous_name": before["name"],
+            "name": after["name"],
+        })))
+    }
+
+    /// Rename a function argument or local variable (`afvn`).
+    #[tool(
+        description = "Rename an argument or local variable of the function containing a \
+        location (see function_variables for the names). Requires --allow-write (afvn)."
+    )]
+    async fn rename_variable(
+        &self,
+        Parameters(p): Parameters<RenameVariableParams>,
+    ) -> Result<CallToolResult, ErrorData> {
+        self.ensure_annotate().await?;
+        let (addr, _) = self.function_at(&p.at).await?;
+        let new = quote_arg(&p.new_name).mcp()?;
+        let old = quote_arg(&p.old_name).mcp()?;
+        self.cmd_strict(&format!("afvn {new} {old} @ {addr}"))
+            .await?;
+        let vars = self.manager.cmdj(&format!("afvlj @ {addr}")).await.mcp()?;
+        let names: Vec<&str> = vars
+            .as_object()
+            .into_iter()
+            .flat_map(|o| o.values())
+            .filter_map(Value::as_array)
+            .flatten()
+            .filter_map(|v| v.get("name").and_then(Value::as_str))
+            .collect();
+        if !names.contains(&p.new_name.as_str()) {
+            return Err(RizinError::InvalidArgument {
+                arg: p.old_name,
+                reason: format!("no such variable in the function at {addr}"),
+            }
+            .to_error_data());
+        }
+        Ok(self.json_result(json!({
+            "function": addr,
+            "old_name": p.old_name,
+            "new_name": p.new_name,
+        })))
+    }
+
+    /// Set or append a comment (`CC`).
+    #[tool(
+        description = "Set the comment at an address (replacing any comment there, or \
+        appending with append=true); returns the resulting comment. Requires --allow-write \
+        (CC)."
+    )]
+    async fn set_comment(
+        &self,
+        Parameters(p): Parameters<SetCommentParams>,
+    ) -> Result<CallToolResult, ErrorData> {
+        self.ensure_annotate().await?;
+        let addr = self.resolve(&p.address).await?;
+        let text = quote_arg(&p.text).mcp()?;
+        if !p.append.unwrap_or(false) {
+            self.cmd_strict(&format!("CC- @ {addr}")).await?;
+        }
+        self.cmd_strict(&format!("CC {text} @ {addr}")).await?;
+        let comment = self.manager.cmd(&format!("CC. @ {addr}")).await.mcp()?;
+        Ok(self.json_result(json!({
+            "at": addr,
+            "comment": comment.trim_end_matches('\n'),
+        })))
+    }
+
+    /// Remove a comment (`CC-`).
+    #[tool(description = "Remove the comment at an address. Requires --allow-write (CC-).")]
+    async fn remove_comment(
+        &self,
+        Parameters(p): Parameters<AddressParams>,
+    ) -> Result<CallToolResult, ErrorData> {
+        self.ensure_annotate().await?;
+        let addr = self.resolve(&p.address).await?;
+        self.cmd_strict(&format!("CC- @ {addr}")).await?;
+        Ok(self.json_result(json!({ "at": addr, "removed": true })))
+    }
+
+    /// Create or move a flag (`f+`).
+    #[tool(
+        description = "Name an address with a flag (creating it, or moving an existing flag of \
+        that name), usable as a location afterwards. Requires --allow-write (f+)."
+    )]
+    async fn set_flag(
+        &self,
+        Parameters(p): Parameters<SetFlagParams>,
+    ) -> Result<CallToolResult, ErrorData> {
+        self.ensure_annotate().await?;
+        let name = validate_flag_name(&p.name).mcp()?;
+        let addr = self.resolve(&p.address).await?;
+        let size = clamp_count(p.size, 1, u64::MAX);
+        // `f` refuses an address that already has a flag; `f+` adds one in
+        // any case, and moves a flag of the same name.
+        self.cmd_strict(&format!("f+ {name} {size} @ {addr}"))
+            .await?;
+        let at = self.resolve(&name).await?;
+        Ok(self.json_result(json!({ "name": name, "at": at, "size": size })))
+    }
+
+    /// Remove a flag (`f-`).
+    #[tool(description = "Remove a flag by name. Requires --allow-write (f-).")]
+    async fn remove_flag(
+        &self,
+        Parameters(p): Parameters<FlagNameParams>,
+    ) -> Result<CallToolResult, ErrorData> {
+        self.ensure_annotate().await?;
+        let name = validate_flag_name(&p.name).mcp()?;
+        // `f-` of an unknown name succeeds silently.
+        self.resolve(&name).await?;
+        self.cmd_strict(&format!("f- {name}")).await?;
+        Ok(self.json_result(json!({ "name": name, "removed": true })))
+    }
+
+    /// Set a function's C signature (`afs`).
+    #[tool(
+        description = "Set the C prototype of the function containing a location, typing its \
+        arguments and return value (e.g. 'int parse_args(int argc, char **argv)'); returns \
+        the resulting signature. Requires --allow-write (afs)."
+    )]
+    async fn set_function_signature(
+        &self,
+        Parameters(p): Parameters<SetSignatureParams>,
+    ) -> Result<CallToolResult, ErrorData> {
+        self.ensure_annotate().await?;
+        let (addr, _) = self.function_at(&p.at).await?;
+        let sig = quote_arg(&p.signature).mcp()?;
+        self.cmd_strict(&format!("afs {sig} @ {addr}")).await?;
+        Ok(self.json_result(self.manager.cmdj(&format!("afsj @ {addr}")).await.mcp()?))
+    }
+
     // --- Writing (guarded) ------------------------------------------------
 
     /// Write hex bytes (`wx`).
@@ -2076,13 +2311,18 @@ mod tests {
             allow_debug: false,
             ..Default::default()
         });
-        for name in RAW_TOOLS.iter().chain(WRITE_TOOLS).chain(DEBUG_TOOLS) {
+        for name in RAW_TOOLS
+            .iter()
+            .chain(WRITE_TOOLS)
+            .chain(ANNOTATION_TOOLS)
+            .chain(DEBUG_TOOLS)
+        {
             assert!(all.iter().any(|n| n == name), "{name} missing");
             assert!(!locked.iter().any(|n| n == name), "{name} offered");
         }
         assert_eq!(
             all.len() - locked.len(),
-            RAW_TOOLS.len() + WRITE_TOOLS.len() + DEBUG_TOOLS.len()
+            RAW_TOOLS.len() + WRITE_TOOLS.len() + ANNOTATION_TOOLS.len() + DEBUG_TOOLS.len()
         );
         // The defaults offer raw commands, but neither writing nor debugging.
         let default = listed(ServerConfig::default());

src/util.rs

@@ -43,6 +43,30 @@ pub fn validate_location(at: &str) -> Result<String, RizinError> {
     Ok(trimmed.to_string())
 }
 
+/// Validate a flag name: `[A-Za-z0-9_.:]`, which Rizin keeps unchanged.
+///
+/// # Errors
+/// Returns [`RizinError::InvalidArgument`] for an empty or long name, or one
+/// with other characters.
+pub fn validate_flag_name(name: &str) -> Result<String, RizinError> {
+    if name.is_empty() || name.len() > 255 {
+        return Err(RizinError::InvalidArgument {
+            arg: name.to_string(),
+            reason: "flag name is empty or too long (max 255 chars)".into(),
+        });
+    }
+    if !name
+        .chars()
+        .all(|c| c.is_ascii_alphanumeric() || matches!(c, '_' | '.' | ':'))
+    {
+        return Err(RizinError::InvalidArgument {
+            arg: name.to_string(),
+            reason: "only [A-Za-z0-9_.:] are allowed in flag names".into(),
+        });
+    }
+    Ok(name.to_string())
+}
+
 /// Build a ` @ <location>` suffix for an optional location, or an empty string
 /// when no location is given.
 ///
@@ -349,6 +373,15 @@ mod tests {
         }
     }
 
+    #[test]
+    fn flag_names() {
+        assert!(validate_flag_name("crypto.key_schedule").is_ok());
+        assert!(validate_flag_name("a:b_1").is_ok());
+        for bad in ["", "a b", "a;f-*", "a@b", "x$"] {
+            assert!(validate_flag_name(bad).is_err(), "should reject {bad:?}");
+        }
+    }
+
     #[test]
     fn at_suffix_formats_or_empties() {
         assert_eq!(at_suffix(&None).unwrap(), "");

tests/tools.rs

@@ -678,3 +678,106 @@ async fn listing_one_flag_space_leaves_the_others_visible() {
         );
     }
 }
+
+#[tokio::test]
+async fn annotations_change_the_analysis() {
+    for t in transports() {
+        let s = Session::start(t, |c| c.allow_write = true).await;
+        // Annotating needs no write-mode file.
+        s.ok(
+            "open_file",
+            json!({ "path": target(), "analysis": "basic" }),
+        )
+        .await;
+
+        let r = s
+            .ok(
+                "rename_function",
+                json!({ "at": "main", "name": "parse_args" }),
+            )
+            .await;
+        assert_eq!(r["previous_name"], "main", "{t:?}: {r}");
+        assert_eq!(r["name"], "parse_args", "{t:?}: {r}");
+        let info = s
+            .items("function_info", json!({ "at": "parse_args" }))
+            .await;
+        assert_eq!(info[0]["name"], "parse_args", "{t:?}");
+
+        let r = s
+            .ok(
+                "rename_variable",
+                json!({ "at": "parse_args", "old_name": "argc", "new_name": "nargs" }),
+            )
+            .await;
+        assert_eq!(r["new_name"], "nargs", "{t:?}");
+        let vars = s
+            .ok("function_variables", json!({ "at": "parse_args" }))
+            .await;
+        assert!(vars.to_string().contains("\"nargs\""), "{t:?}: {vars}");
+        let err = s
+            .call(
+                "rename_variable",
+                json!({ "at": "parse_args", "old_name": "nosuchvar", "new_name": "x" }),
+            )
+            .await
+            .expect_err("unknown variable");
+        // `afvn` reports success here, only logging the error.
+        assert!(err.message.contains("nosuchvar"), "{t:?}: {}", err.message);
+
+        // Comments keep shell metacharacters verbatim.
+        let text = "checks argv; see @0x10 | \"quoted\" $(x)";
+        let r = s
+            .ok(
+                "set_comment",
+                json!({ "address": "parse_args", "text": text }),
+            )
+            .await;
+        assert_eq!(r["comment"], text, "{t:?}");
+        let r = s
+            .ok(
+                "set_comment",
+                json!({ "address": "parse_args", "text": "more", "append": true }),
+            )
+            .await;
+        assert_eq!(r["comment"], format!("{text} more"), "{t:?}");
+        let r = s
+            .ok(
+                "set_comment",
+                json!({ "address": "parse_args", "text": "fresh" }),
+            )
+            .await;
+        assert_eq!(r["comment"], "fresh", "{t:?}");
+        s.ok("remove_comment", json!({ "address": "parse_args" }))
+            .await;
+
+        // A flag becomes a usable location, and is gone once removed.
+        let r = s
+            .ok(
+                "set_flag",
+                json!({ "name": "mcp.marker", "address": "parse_args", "size": 16 }),
+            )
+            .await;
+        assert_eq!(
+            r["at"],
+            s.ok("seek", json!({ "address": "parse_args" })).await["offset"]
+        );
+        s.ok("read_bytes", json!({ "at": "mcp.marker", "count": 4 }))
+            .await;
+        s.ok("remove_flag", json!({ "name": "mcp.marker" })).await;
+        assert!(
+            s.call("remove_flag", json!({ "name": "mcp.marker" }))
+                .await
+                .is_err(),
+            "{t:?}: removing an unknown flag must fail"
+        );
+
+        let sig = s
+            .ok(
+                "set_function_signature",
+                json!({ "at": "parse_args", "signature": "int parse_args(int nargs, char **args)" }),
+            )
+            .await;
+        assert_eq!(sig["ret"], "int", "{t:?}: {sig}");
+        assert_eq!(sig["args"][1]["name"], "args", "{t:?}: {sig}");
+    }
+}