feat(#2621): tighten bash task names to a valid ident ([a-z0-9-])
This commit is contained in:
parent
d0ac48299a
commit
e58457da17
5 changed files with 27 additions and 34 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -1593,6 +1593,7 @@ dependencies = [
|
||||||
"anyhow",
|
"anyhow",
|
||||||
"hive-agent-sock",
|
"hive-agent-sock",
|
||||||
"hive-sh4re",
|
"hive-sh4re",
|
||||||
|
"hive-types",
|
||||||
"libc",
|
"libc",
|
||||||
"rmcp",
|
"rmcp",
|
||||||
"rusqlite",
|
"rusqlite",
|
||||||
|
|
|
||||||
|
|
@ -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
|
the loose-ends list — a memorable label instead of an opaque id. A name
|
||||||
is **reusable once its previous task has finished**; submitting a
|
is **reusable once its previous task has finished**; submitting a
|
||||||
name whose task is still `pending`/`running` is rejected. Allowed
|
name whose task is still `pending`/`running` is rejected. Allowed
|
||||||
characters: ASCII letters, digits, `.`, `_`, `-` (max 64). Omit for
|
characters: `[a-z0-9-]` (a valid identifier — lowercase, digits,
|
||||||
the auto-generated id.
|
hyphen; max 63). Omit for the auto-generated id.
|
||||||
|
|
||||||
Exposed as `mcp__bash__run`.
|
Exposed as `mcp__bash__run`.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -10,6 +10,7 @@ workspace = true
|
||||||
anyhow.workspace = true
|
anyhow.workspace = true
|
||||||
hive-agent-sock.workspace = true
|
hive-agent-sock.workspace = true
|
||||||
hive-sh4re.workspace = true
|
hive-sh4re.workspace = true
|
||||||
|
hive-types.workspace = true
|
||||||
libc.workspace = true
|
libc.workspace = true
|
||||||
rmcp.workspace = true
|
rmcp.workspace = true
|
||||||
rusqlite.workspace = true
|
rusqlite.workspace = true
|
||||||
|
|
|
||||||
|
|
@ -190,8 +190,9 @@ struct BashRunArgs {
|
||||||
/// in `status` lookups and your loose-ends list — handy for recognising
|
/// 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
|
/// 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
|
/// its previous task has finished; reusing a name whose task is still
|
||||||
/// running is rejected. Allowed chars: ASCII letters, digits, `.`,
|
/// running is rejected. Allowed chars: `[a-z0-9-]` (a valid
|
||||||
/// `_`, `-` (max 64). Omit to get the auto-generated id.
|
/// identifier — lowercase, digits, hyphen; max 63). Omit to get the
|
||||||
|
/// auto-generated id.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
name: Option<String>,
|
name: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -247,31 +247,16 @@ async fn clear_bash_todo(socket: &Path, id: &str) {
|
||||||
// Public API used by daemon dispatch
|
// 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
|
/// Validate a caller-chosen task name. The name doubles as the task id
|
||||||
/// and therefore as the `<name>.json` filename, so it must be a single
|
/// and therefore as the `<name>.json` filename, so it must be a valid
|
||||||
/// safe path segment. Allows ASCII alphanumerics plus `.`, `_`, `-`;
|
/// [`hive_types::Ident`] — a single safe path segment of `[a-z0-9-]`,
|
||||||
/// rejects empties, over-long names, `.`/`..`, and anything that could
|
/// non-empty and ≤63 bytes. Delegating to `Ident` also rejects `.`/`..`
|
||||||
/// escape the tasks dir or collide with the `.json.tmp` scratch suffix.
|
/// traversal and the `.json.tmp` scratch suffix (both contain `.`), and
|
||||||
|
/// makes the derived `bash-task-<name>` wake sender a valid identifier.
|
||||||
fn validate_task_name(name: &str) -> Result<()> {
|
fn validate_task_name(name: &str) -> Result<()> {
|
||||||
if name.is_empty() {
|
hive_types::Ident::parse(name)
|
||||||
bail!("task name must not be empty");
|
.map(|_| ())
|
||||||
}
|
.map_err(|e| anyhow::anyhow!("invalid task name {name:?}: {e}"))
|
||||||
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(())
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Submit a new pending task. Returns the task ID.
|
/// 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)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::{MAX_TASK_NAME_LEN, validate_task_name};
|
use super::validate_task_name;
|
||||||
|
use hive_types::Ident;
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn accepts_reasonable_names() {
|
fn accepts_ident_names() {
|
||||||
for ok in ["build", "ci-check", "nix_flake.check", "t1", "A.B-C_9"] {
|
for ok in ["build", "ci-check", "t1", "task-123", "abc"] {
|
||||||
assert!(validate_task_name(ok).is_ok(), "{ok} should be valid");
|
assert!(validate_task_name(ok).is_ok(), "{ok} should be valid");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn rejects_unsafe_names() {
|
fn rejects_non_ident_names() {
|
||||||
// Empty, traversal, separators, control/space, and over-long.
|
// Empty, traversal, separators, control/space, and — now that names
|
||||||
|
// must be a valid `Ident` ([a-z0-9-]) — uppercase, `.`, and `_` too.
|
||||||
for bad in [
|
for bad in [
|
||||||
"",
|
"",
|
||||||
".",
|
".",
|
||||||
|
|
@ -855,15 +842,18 @@ mod tests {
|
||||||
"has space",
|
"has space",
|
||||||
"tab\tname",
|
"tab\tname",
|
||||||
"slash\\back",
|
"slash\\back",
|
||||||
|
"Uppercase",
|
||||||
|
"under_score",
|
||||||
|
"dot.name",
|
||||||
] {
|
] {
|
||||||
assert!(
|
assert!(
|
||||||
validate_task_name(bad).is_err(),
|
validate_task_name(bad).is_err(),
|
||||||
"{bad:?} should be rejected"
|
"{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.
|
// 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
|
// Wake suppression is a single process-wide registry (see
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue