diff --git a/hive-c0re/src/dashboard.rs b/hive-c0re/src/dashboard.rs index 17662b48..eda66f71 100644 --- a/hive-c0re/src/dashboard.rs +++ b/hive-c0re/src/dashboard.rs @@ -1034,13 +1034,6 @@ async fn get_journal( AxumPath(name): AxumPath, axum::extract::Query(q): axum::extract::Query, ) -> 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); @@ -1281,14 +1274,6 @@ 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()); @@ -1723,12 +1708,6 @@ 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) -> 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() @@ -1805,8 +1784,9 @@ 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 → -/// shipped in #581; rolled out across every write route in #572.) +/// 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.) fn validate_agent_name(name: &str) -> Option<&'static str> { if name.is_empty() { return Some("agent name must not be empty"); @@ -1823,43 +1803,6 @@ 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 { - 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 @@ -1886,17 +1829,6 @@ async fn post_purge_tombstone( State(state): State, AxumPath(name): AxumPath, ) -> Response { - // Format guard FIRST so a name like `..` can't traverse into the - // parent of `/var/lib/hyperhive/agents/{name}` and have - // `remove_dir_all` wipe `/var/lib/hyperhive/` itself. Existing - // manager + live-container checks below don't catch `..` — only - // the whitelist does. (argus #593 🔴.) Existence check via - // `containers_snapshot()` is deliberately NOT used here: - // tombstoned agents are gone from the snapshot by design; that's - // the whole point of this endpoint. - if let Some(reason) = validate_agent_name(&name) { - return (StatusCode::BAD_REQUEST, format!("bad agent name: {reason}")).into_response(); - } if name == lifecycle::MANAGER_NAME { return error_response("refusing to purge the manager's state"); } @@ -2112,9 +2044,6 @@ async fn post_set_parent( async fn post_rebuild(State(state): State, AxumPath(name): AxumPath) -> 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, @@ -2165,10 +2094,6 @@ where } async fn post_kill(State(state): State, AxumPath(name): AxumPath) -> 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 @@ -2196,10 +2121,6 @@ async fn post_kill(State(state): State, AxumPath(name): AxumPath, AxumPath(name): AxumPath) -> Response { - let logical = strip_container_prefix(&name); - if let Some(reject) = guard_agent_name(&state, &logical).await { - return reject; - } lifecycle_action( &state, &name, @@ -2212,10 +2133,6 @@ async fn post_restart(State(state): State, AxumPath(name): AxumPath, AxumPath(name): AxumPath) -> Response { - let logical = strip_container_prefix(&name); - if let Some(reject) = guard_agent_name(&state, &logical).await { - return reject; - } lifecycle_action( &state, &name, @@ -2282,9 +2199,6 @@ async fn post_destroy( AxumPath(name): AxumPath, Form(form): Form, ) -> 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