6de6bd5d87
`token_path`'s doc claimed the compiler was the check and that nothing had to remember to perform one. True of `agent`, which is an `Ident`. Not true of `account`, a bare `&str` concatenated into the filename — safe only because `deliver` happened to validate it first, which is the caller-must-remember pattern the comment denied. Observably a no-op today: the one call site already rejects a bad account before reaching here. What changes is that the signature now enforces what the comment asserted, so a second caller cannot skip it. The check is `path::checked_segment`, made public rather than reimplemented. Two copies of a charset are two charsets: they agree until one is edited, and the day they diverge a name is legal in the store and not on disk. An account name cannot simply become an `Ident` the way an agent name is: it is an attribute name in `hyperhive.matrixAccounts`, so uppercase and underscore are already configurable, and narrowing that is a decision rather than a refactor. The new test's controls pin both. Found by argus reviewing the merged PR.
108 lines
3.7 KiB
Rust
108 lines
3.7 KiB
Rust
//! Where a credential lives, for both ends of the store.
|
|
//!
|
|
//! The controller writes and a hive reads, and neither is senior to the other,
|
|
//! so the path they must agree on is built here rather than formatted at each
|
|
//! call site.
|
|
|
|
use crate::Error;
|
|
|
|
/// The KV v2 mount every swarm secret lives under.
|
|
///
|
|
/// A literal because the store's existing reader already hardcodes the same
|
|
/// one (`secret/swarm/matrix/registration-token`); an option nobody sets would
|
|
/// be two ways to say one thing.
|
|
pub const MOUNT: &str = "secret";
|
|
|
|
/// The prefix under [`MOUNT`] owned by per-agent credentials.
|
|
pub const AGENT_PREFIX: &str = "swarm/agents";
|
|
|
|
/// A path segment that cannot change the path's shape.
|
|
///
|
|
/// The charset is deliberately narrower than what the store accepts: a `/`
|
|
/// turns one agent's segment into another agent's directory, and `..` walks
|
|
/// out of the prefix entirely. Both are names this crate receives from
|
|
/// elsewhere — an agent name from the topology, an account name from an
|
|
/// agent's own config — so neither is trusted to be well-formed here.
|
|
///
|
|
/// Public because the same name is also used to build a path **on disk**, and
|
|
/// that guard must accept exactly what this one does. Two copies of a charset
|
|
/// are two charsets: they agree until one is edited, and the day they diverge
|
|
/// is the day a name is legal in the store and not on the filesystem, or the
|
|
/// reverse.
|
|
///
|
|
/// # Errors
|
|
/// [`Error::PathSegment`] when `value` is empty or holds anything outside
|
|
/// `[A-Za-z0-9_-]`.
|
|
pub fn checked_segment(kind: &'static str, value: &str) -> Result<(), Error> {
|
|
if value.is_empty() {
|
|
return Err(Error::PathSegment {
|
|
kind,
|
|
value: value.to_owned(),
|
|
});
|
|
}
|
|
if !value
|
|
.bytes()
|
|
.all(|b| b.is_ascii_alphanumeric() || b == b'-' || b == b'_')
|
|
{
|
|
return Err(Error::PathSegment {
|
|
kind,
|
|
value: value.to_owned(),
|
|
});
|
|
}
|
|
Ok(())
|
|
}
|
|
|
|
/// The path holding `agent`'s token for the external matrix account `account`.
|
|
///
|
|
/// # Errors
|
|
/// [`Error::PathSegment`] when either name contains anything but
|
|
/// `[A-Za-z0-9_-]`, which is what keeps one agent's name from addressing
|
|
/// another agent's secret.
|
|
pub fn matrix_account(agent: &str, account: &str) -> Result<String, Error> {
|
|
checked_segment("agent", agent)?;
|
|
checked_segment("account", account)?;
|
|
Ok(format!("{AGENT_PREFIX}/{agent}/matrix/{account}"))
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
#[test]
|
|
fn a_well_formed_pair_lands_under_the_agent_prefix() {
|
|
let p = matrix_account("atlas", "ops-relay").expect("both segments are legal");
|
|
assert_eq!(p, "swarm/agents/atlas/matrix/ops-relay");
|
|
assert!(p.starts_with(AGENT_PREFIX));
|
|
}
|
|
|
|
#[test]
|
|
fn a_segment_cannot_escape_its_own_directory() {
|
|
// Each of these is a *different* way to address another agent's tree,
|
|
// and the last two are the ones a charset check catches but a
|
|
// `contains("..")` check does not.
|
|
for bad in [
|
|
"../argus",
|
|
"atlas/../argus",
|
|
"atlas/matrix",
|
|
"a b",
|
|
"a.b",
|
|
"",
|
|
] {
|
|
assert!(
|
|
matrix_account(bad, "ops-relay").is_err(),
|
|
"agent segment {bad:?} must be refused"
|
|
);
|
|
assert!(
|
|
matrix_account("atlas", bad).is_err(),
|
|
"account segment {bad:?} must be refused"
|
|
);
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn the_legal_charset_is_actually_reachable() {
|
|
// The control for the test above: if `checked_segment` rejected
|
|
// everything, the escape cases would pass for the wrong reason.
|
|
assert!(matrix_account("a-b_C9", "d-e_F0").is_ok());
|
|
}
|
|
}
|