From d4e91bfeebdfdd52f9fccd51ce9670658e6edb7d Mon Sep 17 00:00:00 2001 From: damocles Date: Sun, 19 Jul 2026 17:56:55 +0200 Subject: [PATCH] feat(#2302): fold is_plain_ident into PlainIdent newtype --- hive-c0re/src/dashboard/extra_forges.rs | 57 +++++----------- hive-c0re/src/dashboard/idents.rs | 76 +++++++++++++++++++++- hive-c0re/src/dashboard/matrix_accounts.rs | 65 ++++++------------ 3 files changed, 112 insertions(+), 86 deletions(-) diff --git a/hive-c0re/src/dashboard/extra_forges.rs b/hive-c0re/src/dashboard/extra_forges.rs index 751f8c0d..03cea449 100644 --- a/hive-c0re/src/dashboard/extra_forges.rs +++ b/hive-c0re/src/dashboard/extra_forges.rs @@ -23,20 +23,9 @@ use axum::extract::{Form, Query}; use axum::response::{IntoResponse, Response}; use serde::{Deserialize, Serialize}; -use super::error_response; +use super::{error_response, idents::PlainIdent}; use crate::coordinator::Coordinator; -/// Plain-identifier check matching hive-priv's `validate_name_chars` -/// (lowercase ascii + digits + hyphens) — same guard used by -/// `matrix_accounts::is_plain_ident`. Duplicated locally (private, not -/// worth a shared-util churn for one predicate) rather than exported from -/// that module, since both call sites are dashboard-only. -fn is_plain_ident(s: &str) -> bool { - !s.is_empty() - && s.chars() - .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-') -} - #[derive(Deserialize)] struct ForgeSidecar { base_url: String, @@ -75,10 +64,10 @@ pub(super) struct ExtraForgesQuery { /// sidecar when present. Never returns a token. pub(super) async fn get_extra_forges(Query(q): Query) -> Response { let agent = q.agent.trim(); - if !is_plain_ident(agent) { + let Ok(agent) = PlainIdent::parse(agent) else { return error_response(&format!("extra-forges: invalid agent {agent:?}")); - } - let dir = Coordinator::agent_notes_dir(agent); + }; + let dir = Coordinator::agent_notes_dir(agent.as_str()); let mut forges = Vec::new(); match std::fs::read_dir(&dir) { Ok(entries) => { @@ -141,12 +130,12 @@ struct ExtraForgeAccountResult { pub(super) async fn post_extra_forge_account(Form(f): Form) -> Response { let agent = f.agent.trim(); let label = f.label.trim(); - if !is_plain_ident(agent) { + let Ok(agent) = PlainIdent::parse(agent) else { return error_response(&format!("extra-forge-account: invalid agent {agent:?}")); - } - if !is_plain_ident(label) { + }; + let Ok(label) = PlainIdent::parse(label) else { return error_response(&format!("extra-forge-account: invalid label {label:?}")); - } + }; match f.action.as_str() { "add" => { @@ -160,9 +149,13 @@ pub(super) async fn post_extra_forge_account(Form(f): Form { - if let Err(e) = crate::priv_client::delete_agent_extra_forge_account(agent, label).await + if let Err(e) = + crate::priv_client::delete_agent_extra_forge_account(agent.as_str(), label.as_str()) + .await { return error_response(&format!( "extra-forge-account: delete account failed: {e:#}" @@ -187,19 +182,3 @@ pub(super) async fn post_extra_forge_account(Form(f): Form Result { + 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; + use super::{AgentName, PlainIdent}; #[test] fn agent_name_accepts_canonical_shapes() { @@ -100,4 +149,25 @@ mod tests { 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"); + } } diff --git a/hive-c0re/src/dashboard/matrix_accounts.rs b/hive-c0re/src/dashboard/matrix_accounts.rs index 4f5f5c52..e80ba7ea 100644 --- a/hive-c0re/src/dashboard/matrix_accounts.rs +++ b/hive-c0re/src/dashboard/matrix_accounts.rs @@ -23,7 +23,7 @@ use axum::extract::{Form, Query}; use axum::response::{IntoResponse, Response}; use serde::{Deserialize, Serialize}; -use super::error_response; +use super::{error_response, idents::PlainIdent}; use crate::coordinator::Coordinator; #[derive(Deserialize)] @@ -176,17 +176,6 @@ struct MatrixLoginResult { user_id: String, } -/// Plain-identifier check matching hive-priv's `validate_name_chars` -/// exactly (lowercase ascii + digits + hyphens) — the root-side guard -/// re-applies the same rule before building the token path. Keeping the -/// dashboard check identical means a name that passes here can't then be -/// rejected at the priv boundary with a confusing "write token failed". -fn is_plain_ident(s: &str) -> bool { - !s.is_empty() - && s.chars() - .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-') -} - /// Provision (or refresh) the token for an agent's extra matrix account. /// password mode → `m.login.password`; token mode → validate via `whoami`. /// On success writes the token to `matrix-token-` via hive-priv and @@ -196,15 +185,15 @@ pub(super) async fn post_matrix_account_login(Form(f): Form) -> let agent = f.agent.trim(); let account = f.account.trim(); let homeserver = f.homeserver.trim().trim_end_matches('/'); - if !is_plain_ident(agent) { + let Ok(agent) = PlainIdent::parse(agent) else { return error_response(&format!("matrix-account-login: invalid agent {agent:?}")); - } - if !is_plain_ident(account) { + }; + let Ok(account) = PlainIdent::parse(account) else { return error_response(&format!( "matrix-account-login: invalid account {account:?}" )); - } - if account == "main" { + }; + if account.as_str() == "main" { return error_response( "matrix-account-login: 'main' is the hive-internal account; it is \ provisioned via the normal flow, not this form", @@ -244,15 +233,19 @@ pub(super) async fn post_matrix_account_login(Form(f): Form) -> } }; - if let Err(e) = - crate::priv_client::write_agent_matrix_token(agent, &token, Some(account), Some(homeserver)) - .await + if let Err(e) = crate::priv_client::write_agent_matrix_token( + agent.as_str(), + &token, + Some(account.as_str()), + Some(homeserver), + ) + .await { return error_response(&format!("matrix-account-login: write token failed: {e:#}")); } // Best-effort kick so the daemon picks up the new account without a full // container restart; not fatal if the container isn't running. - if let Err(e) = crate::priv_client::restart_matrix_daemon(agent).await { + if let Err(e) = crate::priv_client::restart_matrix_daemon(agent.as_str()).await { tracing::warn!( %agent, %account, error = ?e, "matrix-account-login: daemon restart failed (token written; loads on next start)" @@ -288,13 +281,13 @@ struct GithubAccountResult { pub(super) async fn post_github_account(Form(f): Form) -> Response { let agent = f.agent.trim(); let token = f.token.trim(); - if !is_plain_ident(agent) { + let Ok(agent) = PlainIdent::parse(agent) else { return error_response(&format!("github-account: invalid agent {agent:?}")); - } + }; if token.is_empty() { return error_response("github-account: token is required"); } - if let Err(e) = crate::priv_client::write_agent_github_token(agent, token).await { + if let Err(e) = crate::priv_client::write_agent_github_token(agent.as_str(), token).await { return error_response(&format!("github-account: write token failed: {e:#}")); } tracing::info!(%agent, "github-account: provisioned github PAT"); @@ -320,10 +313,10 @@ struct GithubAccountStatus { /// Never returns the token itself. pub(super) async fn get_github_account(Query(q): Query) -> Response { let agent = q.agent.trim(); - if !is_plain_ident(agent) { + let Ok(agent) = PlainIdent::parse(agent) else { return error_response(&format!("github-account: invalid agent {agent:?}")); - } - let present = Coordinator::agent_notes_dir(agent) + }; + let present = Coordinator::agent_notes_dir(agent.as_str()) .join("github-token") .exists(); axum::Json(GithubAccountStatus { present }).into_response() @@ -402,7 +395,7 @@ async fn matrix_whoami(homeserver: &str, token: &str) -> Result #[cfg(test)] mod tests { - use super::{account_name_from_filename, is_plain_ident, read_accounts_snapshot}; + use super::{account_name_from_filename, read_accounts_snapshot}; fn unique_dir(tag: &str) -> std::path::PathBuf { let d = std::env::temp_dir().join(format!( @@ -488,20 +481,4 @@ mod tests { assert_eq!(account_name_from_filename("notes.md"), None); assert_eq!(account_name_from_filename("matrix-avatar-icon-hash"), None); } - - #[test] - fn is_plain_ident_matches_validate_name_chars() { - // Accepts exactly what hive-priv's validate_name_chars does: - // lowercase ascii + digits + hyphens. - assert!(is_plain_ident("catgirl")); - assert!(is_plain_ident("acct-1")); - assert!(!is_plain_ident("")); - // Rejected: uppercase + underscore (would pass a looser check - // then fail at the priv boundary), and path chars. - assert!(!is_plain_ident("MyAccount")); - assert!(!is_plain_ident("my_account")); - assert!(!is_plain_ident("../escape")); - assert!(!is_plain_ident("a/b")); - assert!(!is_plain_ident("a.b")); - } }