feat(#2302): migrate dashboard to the single hive-host-sock Ident newtype
This commit is contained in:
parent
384dcae5f4
commit
1286029947
10 changed files with 37 additions and 210 deletions
|
|
@ -18,7 +18,7 @@ use serde::{Deserialize, Serialize};
|
|||
use tokio_stream::Stream;
|
||||
use tokio_stream::wrappers::ReceiverStream;
|
||||
|
||||
use super::{AppState, error_response, idents::AgentName};
|
||||
use super::{AppState, Ident, error_response};
|
||||
|
||||
#[derive(Deserialize)]
|
||||
pub(super) struct BuildLogsAllQuery {
|
||||
|
|
@ -58,7 +58,7 @@ pub(super) async fn get_build_logs_agent(
|
|||
AxumPath(name): AxumPath<String>,
|
||||
axum::extract::Query(q): axum::extract::Query<BuildLogsQuery>,
|
||||
) -> Response {
|
||||
let name = match AgentName::parse(&name) {
|
||||
let name = match Ident::parse(&name) {
|
||||
Ok(n) => n,
|
||||
Err(reason) => {
|
||||
return (StatusCode::BAD_REQUEST, format!("bad agent name: {reason}")).into_response();
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ use axum::extract::{Form, Query};
|
|||
use axum::response::{IntoResponse, Response};
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use super::{error_response, idents::PlainIdent};
|
||||
use super::{Ident, error_response};
|
||||
use crate::coordinator::Coordinator;
|
||||
|
||||
#[derive(Deserialize)]
|
||||
|
|
@ -64,7 +64,7 @@ pub(super) struct ExtraForgesQuery {
|
|||
/// sidecar when present. Never returns a token.
|
||||
pub(super) async fn get_extra_forges(Query(q): Query<ExtraForgesQuery>) -> Response {
|
||||
let agent = q.agent.trim();
|
||||
let Ok(agent) = PlainIdent::parse(agent) else {
|
||||
let Ok(agent) = Ident::parse(agent) else {
|
||||
return error_response(&format!("extra-forges: invalid agent {agent:?}"));
|
||||
};
|
||||
let dir = Coordinator::agent_notes_dir(agent.as_str());
|
||||
|
|
@ -130,10 +130,10 @@ struct ExtraForgeAccountResult {
|
|||
pub(super) async fn post_extra_forge_account(Form(f): Form<ExtraForgeAccountForm>) -> Response {
|
||||
let agent = f.agent.trim();
|
||||
let label = f.label.trim();
|
||||
let Ok(agent) = PlainIdent::parse(agent) else {
|
||||
let Ok(agent) = Ident::parse(agent) else {
|
||||
return error_response(&format!("extra-forge-account: invalid agent {agent:?}"));
|
||||
};
|
||||
let Ok(label) = PlainIdent::parse(label) else {
|
||||
let Ok(label) = Ident::parse(label) else {
|
||||
return error_response(&format!("extra-forge-account: invalid label {label:?}"));
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -1,173 +0,0 @@
|
|||
//! Validated identifier newtypes for dashboard path-params — the
|
||||
//! "parse, don't validate" discipline applied to agent-name / plain-ident
|
||||
//! path parameters.
|
||||
//!
|
||||
//! [`AgentName`] can only be constructed through a validating parser, so
|
||||
//! "this string passed the naming whitelist" becomes a compile-time fact the
|
||||
//! type carries, instead of a convention every handler re-checks against the
|
||||
//! raw `String`. This is **format** validation only —
|
||||
//! whether the name refers to a *live* agent is a separate, stateful runtime
|
||||
//! concern kept at the lookup sites (see `guard_agent_name`), deliberately not
|
||||
//! folded into the constructor.
|
||||
//!
|
||||
//! [`PlainIdent`] is the slightly stricter sibling (no underscore, no length
|
||||
//! cap) used for dashboard-provisioned labels / account names — it folds the
|
||||
//! two hand-synced `is_plain_ident` copies that used to live in
|
||||
//! `extra_forges` + `matrix_accounts` into one parser.
|
||||
|
||||
use std::fmt;
|
||||
|
||||
/// A validated agent name: 1-63 chars of `[a-z0-9_-]`.
|
||||
///
|
||||
/// Constructed only via [`AgentName::parse`]. The invariant matches the
|
||||
/// historical `validate_agent_name` whitelist (conservative, tracking
|
||||
/// `nixos-container` basename rules and the agent-name convention across the
|
||||
/// codebase): rejects empty, over-long, uppercase, slashes, dots, and any
|
||||
/// non-ASCII (incl. unicode homoglyphs of dash / underscore).
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) struct AgentName(String);
|
||||
|
||||
impl AgentName {
|
||||
/// Parse + validate a path-param agent name (format only).
|
||||
///
|
||||
/// # Errors
|
||||
/// Returns `Err(reason)` — a caller-ready 400-body string — when `name`
|
||||
/// is empty, longer than 63 chars, or contains any byte outside
|
||||
/// `[a-z0-9_-]`.
|
||||
pub(crate) fn parse(name: &str) -> Result<Self, &'static str> {
|
||||
if name.is_empty() {
|
||||
return Err("agent name must not be empty");
|
||||
}
|
||||
if name.len() > 63 {
|
||||
return Err("agent name must be 63 characters or fewer");
|
||||
}
|
||||
if !name
|
||||
.bytes()
|
||||
.all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-' || b == b'_')
|
||||
{
|
||||
return Err("agent name must contain only [a-z0-9_-]");
|
||||
}
|
||||
Ok(Self(name.to_owned()))
|
||||
}
|
||||
|
||||
/// The validated name as a string slice.
|
||||
#[must_use]
|
||||
pub(crate) fn as_str(&self) -> &str {
|
||||
&self.0
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Display for AgentName {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
f.write_str(&self.0)
|
||||
}
|
||||
}
|
||||
|
||||
/// A validated plain identifier: one or more chars of `[a-z0-9-]`.
|
||||
///
|
||||
/// Matches hive-priv's `validate_name_chars` (lowercase ascii + digits +
|
||||
/// hyphens, no underscore, no length cap). Used for dashboard-provisioned
|
||||
/// labels + account names (extra-forge labels, matrix account names) that
|
||||
/// become filesystem path segments, so the same `../` / uppercase / slash
|
||||
/// rejects as [`AgentName`] apply — the two differ only in the underscore
|
||||
/// (allowed by `AgentName`, not here) and the 63-char cap.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) struct PlainIdent(String);
|
||||
|
||||
impl PlainIdent {
|
||||
/// Parse + validate a plain identifier.
|
||||
///
|
||||
/// # Errors
|
||||
/// Returns `Err(reason)` when `s` is empty or contains any byte outside
|
||||
/// `[a-z0-9-]`.
|
||||
pub(crate) fn parse(s: &str) -> Result<Self, &'static str> {
|
||||
if s.is_empty() {
|
||||
return Err("identifier must not be empty");
|
||||
}
|
||||
if !s
|
||||
.bytes()
|
||||
.all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-')
|
||||
{
|
||||
return Err("identifier must contain only [a-z0-9-]");
|
||||
}
|
||||
Ok(Self(s.to_owned()))
|
||||
}
|
||||
|
||||
/// The validated identifier as a string slice.
|
||||
#[must_use]
|
||||
pub(crate) fn as_str(&self) -> &str {
|
||||
&self.0
|
||||
}
|
||||
}
|
||||
|
||||
impl fmt::Display for PlainIdent {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
f.write_str(&self.0)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{AgentName, PlainIdent};
|
||||
|
||||
#[test]
|
||||
fn agent_name_accepts_canonical_shapes() {
|
||||
for ok in [
|
||||
"damocles",
|
||||
"hm1nd",
|
||||
"agent-with-dashes",
|
||||
"snake_case",
|
||||
"mixed_2-3",
|
||||
] {
|
||||
assert!(AgentName::parse(ok).is_ok(), "should accept {ok:?}");
|
||||
}
|
||||
let max = "a".repeat(63);
|
||||
assert!(AgentName::parse(&max).is_ok(), "63 chars is the boundary");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_name_rejects_bad_input() {
|
||||
let too_long = "a".repeat(64);
|
||||
for bad in [
|
||||
"",
|
||||
&too_long,
|
||||
"../etc/passwd",
|
||||
"alice/bob",
|
||||
"Alice",
|
||||
"alice bob",
|
||||
"alice.bob",
|
||||
"alice;DROP TABLE messages",
|
||||
// Non-ASCII, incl. unicode homoglyphs of ASCII dash.
|
||||
"damóclès",
|
||||
"alice\u{2013}bob", // en-dash
|
||||
] {
|
||||
assert!(AgentName::parse(bad).is_err(), "should reject {bad:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_name_round_trips_as_str() {
|
||||
assert_eq!(AgentName::parse("damocles").unwrap().as_str(), "damocles");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plain_ident_accepts_labels_and_accounts() {
|
||||
for ok in ["codeberg", "catgirl", "my-forge-1", "acct-1"] {
|
||||
assert!(PlainIdent::parse(ok).is_ok(), "should accept {ok:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plain_ident_rejects_bad_input() {
|
||||
// Underscore is allowed for AgentName but NOT here (matches
|
||||
// hive-priv's stricter `validate_name_chars`).
|
||||
for bad in ["", "MyForge", "my_forge", "../escape", "a/b", "a.b"] {
|
||||
assert!(PlainIdent::parse(bad).is_err(), "should reject {bad:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plain_ident_round_trips_as_str() {
|
||||
assert_eq!(PlainIdent::parse("codeberg").unwrap().as_str(), "codeberg");
|
||||
}
|
||||
}
|
||||
|
|
@ -18,7 +18,7 @@ use serde::Deserialize;
|
|||
|
||||
use problem_details::ProblemDetails;
|
||||
|
||||
use super::{error_problem, idents::AgentName, strip_container_prefix};
|
||||
use super::{Ident, error_problem, strip_container_prefix};
|
||||
use crate::lifecycle;
|
||||
|
||||
#[derive(Deserialize)]
|
||||
|
|
@ -57,7 +57,7 @@ pub(super) async fn get_journal(
|
|||
// shellout below — the `lifecycle::list()` existence check would
|
||||
// catch them anyway, but rejecting at the boundary keeps the
|
||||
// failure mode crisp.
|
||||
let name = match AgentName::parse(&name) {
|
||||
let name = match Ident::parse(&name) {
|
||||
Ok(n) => n,
|
||||
Err(reason) => {
|
||||
return Err(ProblemDetails::from_status_code(StatusCode::BAD_REQUEST)
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ use axum::extract::{Form, Query};
|
|||
use axum::response::{IntoResponse, Response};
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use super::{error_response, idents::PlainIdent};
|
||||
use super::{Ident, error_response};
|
||||
use crate::coordinator::Coordinator;
|
||||
|
||||
#[derive(Deserialize)]
|
||||
|
|
@ -104,17 +104,14 @@ fn account_name_from_filename(fname: &str) -> Option<String> {
|
|||
|
||||
pub(super) async fn get_matrix_accounts(Query(q): Query<MatrixAccountsQuery>) -> Response {
|
||||
let agent = q.agent.trim();
|
||||
// Agent names are simple identifiers; reject anything else so a crafted
|
||||
// `agent` can't escape the per-agent state root via path components.
|
||||
if agent.is_empty()
|
||||
|| !agent
|
||||
.chars()
|
||||
.all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_')
|
||||
{
|
||||
// Validate through the single `Ident` type so a crafted `agent` can't
|
||||
// escape the per-agent state root via path components — the same guard
|
||||
// every other agent-path builder goes through.
|
||||
let Ok(agent) = Ident::parse(agent) else {
|
||||
return error_response(&format!("matrix-accounts: invalid agent name {agent:?}"));
|
||||
}
|
||||
};
|
||||
|
||||
let dir = Coordinator::agent_notes_dir(agent);
|
||||
let dir = Coordinator::agent_notes_dir(agent.as_str());
|
||||
let (snapshot, as_of_unix) = read_accounts_snapshot(&dir);
|
||||
let mut accounts = Vec::new();
|
||||
match std::fs::read_dir(&dir) {
|
||||
|
|
@ -185,10 +182,10 @@ pub(super) async fn post_matrix_account_login(Form(f): Form<MatrixLoginForm>) ->
|
|||
let agent = f.agent.trim();
|
||||
let account = f.account.trim();
|
||||
let homeserver = f.homeserver.trim().trim_end_matches('/');
|
||||
let Ok(agent) = PlainIdent::parse(agent) else {
|
||||
let Ok(agent) = Ident::parse(agent) else {
|
||||
return error_response(&format!("matrix-account-login: invalid agent {agent:?}"));
|
||||
};
|
||||
let Ok(account) = PlainIdent::parse(account) else {
|
||||
let Ok(account) = Ident::parse(account) else {
|
||||
return error_response(&format!(
|
||||
"matrix-account-login: invalid account {account:?}"
|
||||
));
|
||||
|
|
@ -281,7 +278,7 @@ struct GithubAccountResult {
|
|||
pub(super) async fn post_github_account(Form(f): Form<GithubAccountForm>) -> Response {
|
||||
let agent = f.agent.trim();
|
||||
let token = f.token.trim();
|
||||
let Ok(agent) = PlainIdent::parse(agent) else {
|
||||
let Ok(agent) = Ident::parse(agent) else {
|
||||
return error_response(&format!("github-account: invalid agent {agent:?}"));
|
||||
};
|
||||
if token.is_empty() {
|
||||
|
|
@ -313,7 +310,7 @@ struct GithubAccountStatus {
|
|||
/// Never returns the token itself.
|
||||
pub(super) async fn get_github_account(Query(q): Query<GithubAccountQuery>) -> Response {
|
||||
let agent = q.agent.trim();
|
||||
let Ok(agent) = PlainIdent::parse(agent) else {
|
||||
let Ok(agent) = Ident::parse(agent) else {
|
||||
return error_response(&format!("github-account: invalid agent {agent:?}"));
|
||||
};
|
||||
let present = Coordinator::agent_notes_dir(agent.as_str())
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ use axum::{
|
|||
};
|
||||
use serde::Deserialize;
|
||||
|
||||
use super::{AppState, error_response, idents::AgentName, scan_validated_paths};
|
||||
use super::{AppState, Ident, error_response, scan_validated_paths};
|
||||
|
||||
/// Unread operator-directed messages for the dashboard's Y3R C4LL inbox.
|
||||
/// Returns messages addressed to `"operator"` that haven't been
|
||||
|
|
@ -113,7 +113,7 @@ pub(super) async fn post_mark_all_read(
|
|||
State(state): State<AppState>,
|
||||
AxumPath(name): AxumPath<String>,
|
||||
) -> Response {
|
||||
let name = match AgentName::parse(&name) {
|
||||
let name = match Ident::parse(&name) {
|
||||
Ok(n) => n,
|
||||
Err(reason) => {
|
||||
return (StatusCode::BAD_REQUEST, format!("bad agent name: {reason}")).into_response();
|
||||
|
|
|
|||
|
|
@ -19,8 +19,11 @@ use crate::lifecycle;
|
|||
mod approvals;
|
||||
mod build_logs;
|
||||
mod extra_forges;
|
||||
mod idents;
|
||||
pub(crate) use idents::AgentName;
|
||||
// The single validated identifier type — homed in `hive-host-sock` (the crate
|
||||
// owning agent-path facts) so every dashboard path-param validates through the
|
||||
// same type used to build agent paths. Re-exported so submodules + the socket
|
||||
// server reach it as `crate::dashboard::Ident`.
|
||||
pub(crate) use hive_host_sock::Ident;
|
||||
mod infra_containers;
|
||||
mod journal;
|
||||
mod lifecycle_ops;
|
||||
|
|
@ -296,7 +299,7 @@ fn try_bind(addr: SocketAddr) -> std::io::Result<tokio::net::TcpListener> {
|
|||
|
||||
/// Two-axis path-param guard for write routes. Combines:
|
||||
///
|
||||
/// 1. **format validation** ([`idents::AgentName::parse`]) — rejects
|
||||
/// 1. **format validation** ([`Ident::parse`]) — rejects
|
||||
/// path traversal / unicode homoglyphs / empty + too-long names with
|
||||
/// HTTP 400.
|
||||
/// 2. **existence check** — looks up `name` in the coordinator's
|
||||
|
|
@ -311,9 +314,9 @@ fn try_bind(addr: SocketAddr) -> std::io::Result<tokio::net::TcpListener> {
|
|||
/// 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
|
||||
/// [`idents::AgentName::parse`] directly and skip the existence check.
|
||||
/// [`Ident::parse`] directly and skip the existence check.
|
||||
async fn guard_agent_name(state: &AppState, name: &str) -> Option<Response> {
|
||||
if let Err(reason) = idents::AgentName::parse(name) {
|
||||
if let Err(reason) = Ident::parse(name) {
|
||||
return Some(
|
||||
(StatusCode::BAD_REQUEST, format!("bad agent name: {reason}")).into_response(),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ use serde::{Deserialize, Serialize};
|
|||
|
||||
use problem_details::ProblemDetails;
|
||||
|
||||
use super::{AppState, guard_agent_name, idents::AgentName, strip_container_prefix};
|
||||
use super::{AppState, Ident, guard_agent_name, strip_container_prefix};
|
||||
|
||||
#[derive(Serialize)]
|
||||
pub(super) struct ToolGroupsSnapshot {
|
||||
|
|
@ -371,14 +371,14 @@ pub(super) async fn get_stale_permissions(
|
|||
///
|
||||
/// Bypasses `guard_agent_name`'s live-roster check intentionally —
|
||||
/// the whole point is to remove entries for non-roster agents. Only
|
||||
/// the format check ([`AgentName::parse`]) is applied. No rebuild is
|
||||
/// the format check ([`Ident::parse`]) is applied. No rebuild is
|
||||
/// enqueued (the agent doesn't exist to rebuild); the SSE snapshots
|
||||
/// update the P3RM1SS10NS tab live.
|
||||
pub(super) async fn delete_agent_permissions(
|
||||
State(state): State<AppState>,
|
||||
AxumPath(name): AxumPath<String>,
|
||||
) -> Response {
|
||||
let logical = match AgentName::parse(&strip_container_prefix(&name)) {
|
||||
let logical = match Ident::parse(&strip_container_prefix(&name)) {
|
||||
Ok(n) => n,
|
||||
Err(reason) => {
|
||||
return (StatusCode::BAD_REQUEST, format!("bad agent name: {reason}")).into_response();
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ use crate::container_view::{ContainerView, claude_has_session};
|
|||
use crate::coordinator::Coordinator;
|
||||
use crate::lifecycle;
|
||||
|
||||
use super::{AppState, error_response, idents::AgentName};
|
||||
use super::{AppState, Ident, error_response};
|
||||
|
||||
#[derive(Serialize, Clone, Debug)]
|
||||
pub struct TombstoneView {
|
||||
|
|
@ -116,7 +116,7 @@ pub(super) async fn post_purge_tombstone(
|
|||
// `containers_snapshot()` is deliberately NOT used here:
|
||||
// tombstoned agents are gone from the snapshot by design; that's
|
||||
// the whole point of this endpoint.
|
||||
let name = match AgentName::parse(&name) {
|
||||
let name = match Ident::parse(&name) {
|
||||
Ok(n) => n,
|
||||
Err(reason) => {
|
||||
return (StatusCode::BAD_REQUEST, format!("bad agent name: {reason}")).into_response();
|
||||
|
|
|
|||
|
|
@ -425,7 +425,7 @@ async fn handle_get_agent_meta(
|
|||
// the OS level. Validate it before any path is built. The `None` default
|
||||
// (`target == agent`) is the caller's own authenticated name, already
|
||||
// valid — but validating unconditionally is simplest and harmless.
|
||||
if let Err(reason) = crate::dashboard::AgentName::parse(target) {
|
||||
if let Err(reason) = hive_host_sock::Ident::parse(target) {
|
||||
return hive_agent_sock::Response::Err {
|
||||
message: format!("get_agent_meta: invalid agent name {target:?}: {reason}"),
|
||||
};
|
||||
|
|
@ -445,7 +445,7 @@ async fn handle_get_agent_meta(
|
|||
// `@user:server` / `homeserver`) — the access token lives separately
|
||||
// in the agent's `matrix-token` and is never part of this response.
|
||||
// Peer visibility is intentional: it lets an agent verify/contact
|
||||
// another on a public matrix instance. The `AgentName::parse` gate
|
||||
// another on a public matrix instance. The `Ident::parse` gate
|
||||
// above is what closes the real vector here (path traversal via `../`
|
||||
// in an agent-supplied name).
|
||||
matrix_accounts: read_agent_matrix_identities(target),
|
||||
|
|
@ -748,7 +748,7 @@ fn require_group(agent: &str, group: &str, action: &str) -> Option<Response> {
|
|||
/// `submit_init_config`, which builds filesystem paths from it, so validate
|
||||
/// before that.
|
||||
fn require_new_child(agent: &str, target: &str, action: &str) -> Option<Response> {
|
||||
if let Err(reason) = crate::dashboard::AgentName::parse(target) {
|
||||
if let Err(reason) = hive_host_sock::Ident::parse(target) {
|
||||
return Some(Response::Err {
|
||||
message: format!("agent `{agent}` cannot {action} `{target}`: {reason}"),
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue