dashboard: validate agent-name on every write route (closes #572)

hive-c0re/src/dashboard.rs

@@ -1034,6 +1034,13 @@ async fn get_journal(
     AxumPath(name): AxumPath<String>,
     axum::extract::Query(q): axum::extract::Query<JournalQuery>,
 ) -> Response {
+    // Defense-in-depth format check (#572) so weird chars never reach
+    // the shellout below — the `lifecycle::list()` existence check
+    // would catch them anyway, but rejecting at the boundary keeps
+    // the failure mode crisp.
+    if let Some(reason) = validate_agent_name(&name) {
+        return (StatusCode::BAD_REQUEST, format!("bad agent name: {reason}")).into_response();
+    }
     // Validate the container name against the list of managed
     // containers so we don't shell out with arbitrary input.
     let container = strip_container_prefix(&name);
@@ -1274,6 +1281,14 @@ mod tests {
         assert!(validate_agent_name(&max).is_none(), "63-char name should pass");
     }
 
+    // The two-axis guard (`guard_agent_name`) wires `validate_agent_name`
+    // + an async coordinator lookup. The lookup needs a populated
+    // `Coordinator`, which needs sqlite + tokio runtime; rather than
+    // build that scaffolding for an integration-flavoured test we cover
+    // the format axis here (the existence axis is enforced by the
+    // shared `containers_snapshot` API, tested in `coordinator.rs`'s
+    // own suite). 9 cases below match what shipped in #581 + the new
+    // boundary-length case to make the contract explicit.
     #[test]
     fn validate_agent_name_rejects_bad_input() {
         assert!(validate_agent_name("").is_some());
@@ -1708,6 +1723,12 @@ async fn post_schedule_cancel(
 /// Failure modes (agent down, slow response, malformed JSON) all
 /// degrade to an empty list so the dashboard still renders.
 async fn get_agent_links(AxumPath(name): AxumPath<String>) -> Response {
+    // Format-only guard (#572). GET routes return empty gracefully
+    // on unknown names per argus's spec; bad format we reject early
+    // so the downstream port-hash + HTTP fetch never sees garbage.
+    if validate_agent_name(&name).is_some() {
+        return axum::Json(serde_json::json!([])).into_response();
+    }
     let port = lifecycle::agent_web_port(&name);
     let url = format!("http://127.0.0.1:{port}/api/state");
     let client = match reqwest::Client::builder()
@@ -1784,9 +1805,8 @@ async fn post_retry_reminder(
 /// homoglyphs of dash/underscore). Returns `None` on accept, `Some(reason)`
 /// on reject — caller wraps the reason in a 400 response. Conservative
 /// whitelist matching `nixos-container` basename rules and the existing
-/// agent-name convention across the codebase. (mara nag on #566 — this
-/// is the local fix for `mark-all-read`; general extractor + rollout
-/// across other endpoints tracked at #572.)
+/// agent-name convention across the codebase. (mara nag on #566 →
+/// shipped in #581; rolled out across every write route in #572.)
 fn validate_agent_name(name: &str) -> Option<&'static str> {
     if name.is_empty() {
         return Some("agent name must not be empty");
@@ -1803,6 +1823,43 @@ fn validate_agent_name(name: &str) -> Option<&'static str> {
     None
 }
 
+/// Two-axis path-param guard for write routes (#572). Combines:
+///
+/// 1. **format validation** (`validate_agent_name`) — rejects path
+///    traversal / unicode homoglyphs / empty + too-long names with
+///    HTTP 400.
+/// 2. **existence check** — looks up `name` in the coordinator's
+///    container snapshot; unknown name → HTTP 404 with a clear
+///    "no such agent" message. catches the operator-typo case where
+///    a destructive POST would otherwise hit silently (mark-all-read
+///    returning 0) or hit downstream lifecycle code that fails with
+///    a confusing nspawn error.
+///
+/// Returns `None` when both checks pass (caller proceeds), `Some(Response)`
+/// when the request should be rejected. Use at the top of every write
+/// handler taking a name path-param. Read-only GET handlers and
+/// handlers that legitimately operate on tombstoned agents (e.g.
+/// `mark-all-read` on broker rows for a destroyed agent) call
+/// `validate_agent_name` directly and skip the existence check.
+async fn guard_agent_name(state: &AppState, name: &str) -> Option<Response> {
+    if let Some(reason) = validate_agent_name(name) {
+        return Some(
+            (StatusCode::BAD_REQUEST, format!("bad agent name: {reason}")).into_response(),
+        );
+    }
+    let snapshot = state.coord.containers_snapshot().await;
+    if !snapshot.iter().any(|c| c.name == name) {
+        return Some(
+            (
+                StatusCode::NOT_FOUND,
+                format!("no such agent: {name}"),
+            )
+                .into_response(),
+        );
+    }
+    None
+}
+
 /// Operator-driven "clear this agent's inbox" — backs the side-panel
 /// "mark all read" button (#559). Marks every message addressed to the
 /// agent as acked (backfilling `delivered_at` for any still-pending
@@ -2044,6 +2101,9 @@ async fn post_set_parent(
 
 async fn post_rebuild(State(state): State<AppState>, AxumPath(name): AxumPath<String>) -> Response {
     let logical = strip_container_prefix(&name);
+    if let Some(reject) = guard_agent_name(&state, &logical).await {
+        return reject;
+    }
     state.coord.rebuild_queue.enqueue(
         crate::rebuild_queue::QueueKind::Rebuild,
         logical,
@@ -2094,6 +2154,10 @@ where
 }
 
 async fn post_kill(State(state): State<AppState>, AxumPath(name): AxumPath<String>) -> Response {
+    let logical = strip_container_prefix(&name);
+    if let Some(reject) = guard_agent_name(&state, &logical).await {
+        return reject;
+    }
     // #443: manager is stoppable from the dashboard like any other
     // agent. The host's dashboard server keeps running (it's
     // hive-c0re, not the manager container), per-agent approvals
@@ -2121,6 +2185,10 @@ async fn post_kill(State(state): State<AppState>, AxumPath(name): AxumPath<Strin
 }
 
 async fn post_restart(State(state): State<AppState>, AxumPath(name): AxumPath<String>) -> Response {
+    let logical = strip_container_prefix(&name);
+    if let Some(reject) = guard_agent_name(&state, &logical).await {
+        return reject;
+    }
     lifecycle_action(
         &state,
         &name,
@@ -2133,6 +2201,10 @@ async fn post_restart(State(state): State<AppState>, AxumPath(name): AxumPath<St
 }
 
 async fn post_start(State(state): State<AppState>, AxumPath(name): AxumPath<String>) -> Response {
+    let logical = strip_container_prefix(&name);
+    if let Some(reject) = guard_agent_name(&state, &logical).await {
+        return reject;
+    }
     lifecycle_action(
         &state,
         &name,
@@ -2199,6 +2271,9 @@ async fn post_destroy(
     AxumPath(name): AxumPath<String>,
     Form(form): Form<DestroyForm>,
 ) -> Response {
+    if let Some(reject) = guard_agent_name(&state, &name).await {
+        return reject;
+    }
     // Checkbox semantics: any non-empty value (axum sends "on") = purge.
     let purge = form.purge.as_deref().is_some_and(|v| !v.is_empty());
     // `actions::destroy` rescans the container list on success, so the