feat(#2302): parse agent names into AgentName newtype at dashboard boundary
This commit is contained in:
parent
427328c4eb
commit
cfac917b4d
8 changed files with 162 additions and 103 deletions
|
|
@ -19,6 +19,8 @@ use crate::lifecycle;
|
|||
mod approvals;
|
||||
mod build_logs;
|
||||
mod extra_forges;
|
||||
mod idents;
|
||||
pub(crate) use idents::AgentName;
|
||||
mod infra_containers;
|
||||
mod journal;
|
||||
mod lifecycle_ops;
|
||||
|
|
@ -292,33 +294,10 @@ fn try_bind(addr: SocketAddr) -> std::io::Result<tokio::net::TcpListener> {
|
|||
sock.listen(1024)
|
||||
}
|
||||
|
||||
/// Validate that a path-param agent name conforms to the hyperhive
|
||||
/// naming whitelist: 1-63 chars of `[a-z0-9_-]`. Rejects empty,
|
||||
/// uppercase, slashes, dots, and any non-ASCII (incl. unicode
|
||||
/// 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.
|
||||
pub(crate) fn validate_agent_name(name: &str) -> Option<&'static str> {
|
||||
if name.is_empty() {
|
||||
return Some("agent name must not be empty");
|
||||
}
|
||||
if name.len() > 63 {
|
||||
return Some("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 Some("agent name must contain only [a-z0-9_-]");
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Two-axis path-param guard for write routes. Combines:
|
||||
///
|
||||
/// 1. **format validation** (`validate_agent_name`) — rejects path
|
||||
/// traversal / unicode homoglyphs / empty + too-long names with
|
||||
/// 1. **format validation** ([`idents::AgentName::parse`]) — 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
|
||||
|
|
@ -332,9 +311,9 @@ pub(crate) fn validate_agent_name(name: &str) -> Option<&'static str> {
|
|||
/// 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.
|
||||
/// [`idents::AgentName::parse`] 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) {
|
||||
if let Err(reason) = idents::AgentName::parse(name) {
|
||||
return Some(
|
||||
(StatusCode::BAD_REQUEST, format!("bad agent name: {reason}")).into_response(),
|
||||
);
|
||||
|
|
@ -396,45 +375,4 @@ mod tests {
|
|||
let fv = serde_json::to_value(&five).expect("problem details serialise");
|
||||
assert_eq!(fv["status"], 500);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_agent_name_accepts_canonical_shapes() {
|
||||
assert!(validate_agent_name("damocles").is_none());
|
||||
assert!(validate_agent_name("hm1nd").is_none());
|
||||
assert!(validate_agent_name("agent-with-dashes").is_none());
|
||||
assert!(validate_agent_name("snake_case").is_none());
|
||||
assert!(validate_agent_name("mixed_2-3").is_none());
|
||||
let max = "a".repeat(63);
|
||||
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 cover the boundary-length case and
|
||||
// other expected rejects to make the contract explicit.
|
||||
#[test]
|
||||
fn validate_agent_name_rejects_bad_input() {
|
||||
assert!(validate_agent_name("").is_some());
|
||||
let too_long = "a".repeat(64);
|
||||
assert!(validate_agent_name(&too_long).is_some());
|
||||
// Path-traversal attempts.
|
||||
assert!(validate_agent_name("../etc/passwd").is_some());
|
||||
assert!(validate_agent_name("alice/bob").is_some());
|
||||
// Uppercase rejected — canonical lowercase convention.
|
||||
assert!(validate_agent_name("Alice").is_some());
|
||||
// No spaces, dots, special chars.
|
||||
assert!(validate_agent_name("alice bob").is_some());
|
||||
assert!(validate_agent_name("alice.bob").is_some());
|
||||
assert!(validate_agent_name("alice;DROP TABLE messages").is_some());
|
||||
// Non-ASCII (incl. unicode homoglyphs of ASCII dash).
|
||||
assert!(validate_agent_name("damóclès").is_some());
|
||||
assert!(validate_agent_name("alice\u{2013}bob").is_some()); // en-dash
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue