diff --git a/Cargo.lock b/Cargo.lock index 57531fd2..8ede80f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1593,6 +1593,7 @@ dependencies = [ "anyhow", "hive-agent-sock", "hive-sh4re", + "hive-types", "libc", "rmcp", "rusqlite", diff --git a/docs/tools/bash.md b/docs/tools/bash.md index 1e802ac1..d0a4b500 100644 --- a/docs/tools/bash.md +++ b/docs/tools/bash.md @@ -33,8 +33,8 @@ delivered the terminal result inline, in which case no todo is created the loose-ends list — a memorable label instead of an opaque id. A name is **reusable once its previous task has finished**; submitting a name whose task is still `pending`/`running` is rejected. Allowed - characters: ASCII letters, digits, `.`, `_`, `-` (max 64). Omit for - the auto-generated id. + characters: `[a-z0-9-]` (a valid identifier — lowercase, digits, + hyphen; max 63). Omit for the auto-generated id. Exposed as `mcp__bash__run`. diff --git a/hive-bash-mcp/Cargo.toml b/hive-bash-mcp/Cargo.toml index 06269257..e1f2a766 100644 --- a/hive-bash-mcp/Cargo.toml +++ b/hive-bash-mcp/Cargo.toml @@ -10,6 +10,7 @@ workspace = true anyhow.workspace = true hive-agent-sock.workspace = true hive-sh4re.workspace = true +hive-types.workspace = true libc.workspace = true rmcp.workspace = true rusqlite.workspace = true diff --git a/hive-bash-mcp/src/bin/mcp.rs b/hive-bash-mcp/src/bin/mcp.rs index 9c365a0a..0bab814e 100644 --- a/hive-bash-mcp/src/bin/mcp.rs +++ b/hive-bash-mcp/src/bin/mcp.rs @@ -190,8 +190,9 @@ struct BashRunArgs { /// in `status` lookups and your loose-ends list — handy for recognising /// a task later instead of an opaque hex id. A name can be reused once /// its previous task has finished; reusing a name whose task is still - /// running is rejected. Allowed chars: ASCII letters, digits, `.`, - /// `_`, `-` (max 64). Omit to get the auto-generated id. + /// running is rejected. Allowed chars: `[a-z0-9-]` (a valid + /// identifier — lowercase, digits, hyphen; max 63). Omit to get the + /// auto-generated id. #[serde(default)] name: Option, } diff --git a/hive-bash-mcp/src/runner.rs b/hive-bash-mcp/src/runner.rs index e17319fc..ef401ac6 100644 --- a/hive-bash-mcp/src/runner.rs +++ b/hive-bash-mcp/src/runner.rs @@ -247,31 +247,16 @@ async fn clear_bash_todo(socket: &Path, id: &str) { // Public API used by daemon dispatch // --------------------------------------------------------------------------- -/// Longest accepted caller-chosen task name. -const MAX_TASK_NAME_LEN: usize = 64; - /// Validate a caller-chosen task name. The name doubles as the task id -/// and therefore as the `.json` filename, so it must be a single -/// safe path segment. Allows ASCII alphanumerics plus `.`, `_`, `-`; -/// rejects empties, over-long names, `.`/`..`, and anything that could -/// escape the tasks dir or collide with the `.json.tmp` scratch suffix. +/// and therefore as the `.json` filename, so it must be a valid +/// [`hive_types::Ident`] — a single safe path segment of `[a-z0-9-]`, +/// non-empty and ≤63 bytes. Delegating to `Ident` also rejects `.`/`..` +/// traversal and the `.json.tmp` scratch suffix (both contain `.`), and +/// makes the derived `bash-task-` wake sender a valid identifier. fn validate_task_name(name: &str) -> Result<()> { - if name.is_empty() { - bail!("task name must not be empty"); - } - if name.len() > MAX_TASK_NAME_LEN { - bail!("task name too long (max {MAX_TASK_NAME_LEN} chars)"); - } - if name == "." || name == ".." { - bail!("task name {name:?} is reserved"); - } - if !name - .chars() - .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-')) - { - bail!("task name {name:?} may only contain ASCII letters, digits, '.', '_', '-'"); - } - Ok(()) + hive_types::Ident::parse(name) + .map(|_| ()) + .map_err(|e| anyhow::anyhow!("invalid task name {name:?}: {e}")) } /// Submit a new pending task. Returns the task ID. @@ -834,18 +819,20 @@ fn done_summary(id: &str, summary: &str, output: Option<(bool, bool)>) -> String #[cfg(test)] mod tests { - use super::{MAX_TASK_NAME_LEN, validate_task_name}; + use super::validate_task_name; + use hive_types::Ident; #[test] - fn accepts_reasonable_names() { - for ok in ["build", "ci-check", "nix_flake.check", "t1", "A.B-C_9"] { + fn accepts_ident_names() { + for ok in ["build", "ci-check", "t1", "task-123", "abc"] { assert!(validate_task_name(ok).is_ok(), "{ok} should be valid"); } } #[test] - fn rejects_unsafe_names() { - // Empty, traversal, separators, control/space, and over-long. + fn rejects_non_ident_names() { + // Empty, traversal, separators, control/space, and — now that names + // must be a valid `Ident` ([a-z0-9-]) — uppercase, `.`, and `_` too. for bad in [ "", ".", @@ -855,15 +842,18 @@ mod tests { "has space", "tab\tname", "slash\\back", + "Uppercase", + "under_score", + "dot.name", ] { assert!( validate_task_name(bad).is_err(), "{bad:?} should be rejected" ); } - assert!(validate_task_name(&"x".repeat(MAX_TASK_NAME_LEN + 1)).is_err()); + assert!(validate_task_name(&"x".repeat(Ident::MAX_LEN + 1)).is_err()); // Exactly at the cap is allowed. - assert!(validate_task_name(&"x".repeat(MAX_TASK_NAME_LEN)).is_ok()); + assert!(validate_task_name(&"x".repeat(Ident::MAX_LEN)).is_ok()); } // Wake suppression is a single process-wide registry (see