feat(#3549): refuse agent names the swarm already routes on
An agent name was checked for shape and never for meaning, so a name the system already answers to could be taken. The list is derived, not invented: operator/manager/root are broker recipients, and forge/reminder/schedule/ system/todo are message senders. Senders are in it for a reason that is not cosmetic. The harness switches on the sender name, so an agent called todo would have its messages read as todo-wakes and one called system as helper events. That is a parse, not a display quirk. ⚠️ The check is deliberately its own function rather than part of validate_username, which runs over every user on every write — including humans this bridge did not create. Folding it in would mean a store already containing an operator named 'operator' could never be written again: a rule about what an agent may be NAMED would have become a rule about what the file may CONTAIN. A test pins that. This is the static half only. The dynamic half — a name already taken by another subject — waits on the heal-vs-refuse question open on #3550.
This commit is contained in:
parent
775ab285b7
commit
dd1156d45c
2 changed files with 86 additions and 0 deletions
|
|
@ -254,6 +254,10 @@ fn refuse(refusal: Refusal) -> Response {
|
|||
|
||||
async fn handle(state: &AppState, name: String) -> Result<BridgeResponse> {
|
||||
store::validate_username(&name)?;
|
||||
// After the charset check and before anything touches the store: a
|
||||
// reserved name is refused on its own terms, not as a side effect of a
|
||||
// collision with whatever happens to be in the file today.
|
||||
store::reject_reserved_name(&name)?;
|
||||
let cfg = &state.config;
|
||||
|
||||
// Held across the whole load → insert → publish sequence, not just the
|
||||
|
|
|
|||
|
|
@ -57,6 +57,44 @@ pub struct User {
|
|||
pub extra: BTreeMap<String, serde_norway::Value>,
|
||||
}
|
||||
|
||||
/// Names an agent may never take, because something else already answers to
|
||||
/// them.
|
||||
///
|
||||
/// **Derived, not invented** — every entry is a literal that exists in the
|
||||
/// code today. The first three are broker *recipients* (`send(to: …)`); the
|
||||
/// rest are message *senders*, and they matter for a reason that is not
|
||||
/// cosmetic: the harness switches on the sender name, so an agent called
|
||||
/// `todo` would have its messages interpreted as todo-wakes and one called
|
||||
/// `system` as helper events. That is a parse, not a display quirk.
|
||||
///
|
||||
/// Sorted so a reader can scan it; the lookup is a linear walk over eight
|
||||
/// short strings on a path that already reads a file off disk.
|
||||
pub const RESERVED_AGENT_NAMES: &[&str] = &[
|
||||
"forge", "manager", "operator", "reminder", "root", "schedule", "system", "todo",
|
||||
];
|
||||
|
||||
/// Refuse a name the swarm has already given a meaning.
|
||||
///
|
||||
/// ⚠️ **Deliberately NOT part of [`validate_username`].** That function runs
|
||||
/// from [`validate`] over *every* user on *every* write, including humans
|
||||
/// this bridge did not create. Folding the reserved list into it would mean
|
||||
/// that a store already containing an operator called `operator` could never
|
||||
/// be written again — a rule about what an agent may be *named* would have
|
||||
/// become a rule about what the file may *contain*, and bricked it.
|
||||
///
|
||||
/// So this is called from the one path that names a new agent, and nowhere
|
||||
/// else.
|
||||
pub fn reject_reserved_name(name: &str) -> Result<()> {
|
||||
if RESERVED_AGENT_NAMES.contains(&name) {
|
||||
bail!(
|
||||
"{name:?} is reserved: the swarm already routes messages to or from that name, so an \
|
||||
agent with it would have its own messages misread. Reserved: {}",
|
||||
RESERVED_AGENT_NAMES.join(", ")
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Same conservative charset `swarmctl::users::validate_username` already
|
||||
/// enforces — usernames are YAML map keys, log lines, and access-control
|
||||
/// subjects, so keeping them to plain ASCII means nothing downstream ever
|
||||
|
|
@ -301,6 +339,50 @@ some_future_top_level_key: 7
|
|||
);
|
||||
}
|
||||
|
||||
/// Every reserved word is refused, named individually rather than looped
|
||||
/// over the constant — a test that iterates the list it is testing passes
|
||||
/// just as happily when the list is empty.
|
||||
#[test]
|
||||
fn every_reserved_name_is_refused() {
|
||||
for name in [
|
||||
"operator", "manager", "root", "todo", "forge", "reminder", "system", "schedule",
|
||||
] {
|
||||
assert!(
|
||||
reject_reserved_name(name).is_err(),
|
||||
"{name:?} must be reserved"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The control: an ordinary agent name is not caught. Without this the
|
||||
/// arm above would pass a function that refused everything.
|
||||
#[test]
|
||||
fn an_ordinary_name_is_not_reserved() {
|
||||
for name in ["atlas", "iris", "damocles", "operator-2", "todos"] {
|
||||
reject_reserved_name(name)
|
||||
.unwrap_or_else(|e| panic!("{name:?} should be allowed: {e}"));
|
||||
}
|
||||
}
|
||||
|
||||
/// 🔑 The rule must NOT live in `validate_username`, because that runs
|
||||
/// from `validate` over every user on every write — including humans this
|
||||
/// bridge did not create. A store that already contains an operator named
|
||||
/// `operator` has to stay writable, or a naming rule about *agents* would
|
||||
/// silently become a rule about what the *file* may contain.
|
||||
#[test]
|
||||
fn a_reserved_name_already_in_the_store_does_not_block_writes() {
|
||||
let mut store = UserStore::default();
|
||||
store
|
||||
.users
|
||||
.insert("operator".to_owned(), user("$argon2id$a"));
|
||||
|
||||
render_yaml(&store).expect("an existing subject with a reserved name must stay writable");
|
||||
assert!(
|
||||
reject_reserved_name("operator").is_err(),
|
||||
"...while still being refused as a NEW agent name"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn usernames_outside_the_conservative_set_are_refused() {
|
||||
for bad in ["", "-leading", "has space", "quote\"d", "sla/sh"] {
|
||||
|
|
|
|||
Loading…
Reference in a new issue