vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix(#3422): swarmctl reads and writes users.yml directly

Browse source 
The second half of making one file canonical. swarmctl kept its own
private JSON store and rendered users.yml from it, against the same
physical file the bridge wrote -- the seam that made `swarm agent create`
refuse to run.

- users.yml is read before it is written, so users another writer added
  are loaded rather than treated as a file to refuse or clobber. The
  overwrite guard and the seed check go with the second store: they
  existed to police two stores that could disagree.
- serde_norway replaces the hand-rolled emitter. Unknown top-level and
  per-user keys round-trip through `extra`, so two writers cannot delete
  each other's fields.
- The synthetic email moves from render time to the write path and is
  stored. With the file as the store, "rendered but not persisted" has
  nowhere left to live, and mara ruled the stored address correct.
- `--store` is removed rather than deprecated: a flag whose only
  remaining effect is nothing reads as accepted and does nothing.

Three tests asserted the emitter's exact bytes, and one asserted the
guard. Rewritten rather than deleted -- as round-trips for the former,
and inverted for the latter, since "a populated file is READ" is the
behaviour this change is for and deleting its test would leave it
unpinned.
This commit is contained in:
atlas 2026-08-18 10:25:26 +02:00
commit af1203c78b
2 changed files with 256 additions and 230 deletions
Download patch file Download diff file Expand all files Collapse all files

152
swarmctl/src/main.rs
View file

@ -34,13 +34,6 @@ use clap::{Args, Parser, Subcommand};
use users::{User, UserStore};
/// Canonical user store. A compiled-in default is legitimate here for the
/// same reason it is on the daemon's socket path: this is a path this
/// process **creates**, not an address it hopes to find something at. It
/// lives under the controller's state directory because the controller is
/// this store's eventual reader.
const DEFAULT_STORE: &str = "/var/lib/swarm-controller/users.json";
#[derive(Parser)]
#[command(name = "swarmctl", version, about = "swarm-level operator CLI")]
struct Cli {
@ -67,17 +60,20 @@ struct PathArgs {
authelia_bin: Option<PathBuf>,
/// Host-side path of authelia's users database — i.e. the path inside
/// the container, prefixed with the container's root.
///
/// ⚠️ This is the **only** user store. There used to be a `--store`
/// flag naming a private canonical JSON that this file was rendered
/// from; it is gone rather than deprecated, because a flag whose only
/// remaining effect would be nothing is worse than an unknown-argument
/// error — the operator sets it, sees success, and gets none of what
/// they asked for.
#[arg(long, value_name = "PATH")]
users_file: Option<PathBuf>,
/// Canonical user store.
#[arg(long, value_name = "PATH")]
store: Option<PathBuf>,
}
struct Paths {
authelia_bin: PathBuf,
users_file: PathBuf,
store: PathBuf,
}
impl PathArgs {
@ -85,10 +81,6 @@ impl PathArgs {
Ok(Paths {
authelia_bin: path_from(self.authelia_bin, "SWARMCTL_AUTHELIA_BIN")?,
users_file: path_from(self.users_file, "SWARMCTL_AUTHELIA_USERS_FILE")?,
store: self
.store
.or_else(|| std::env::var_os("SWARMCTL_STORE").map(PathBuf::from))
.unwrap_or_else(|| PathBuf::from(DEFAULT_STORE)),
})
}
}
@ -238,12 +230,12 @@ fn main() -> Result<()> {
fn user_add(paths: &Paths, args: AddArgs) -> Result<()> {
users::validate_username(&args.username)?;
let mut store = load_store(&paths.store, &paths.users_file)?;
let mut store = load_store(&paths.users_file)?;
if store.users.contains_key(&args.username) {
bail!(
"user {:?} already exists in {}",
args.username,
paths.store.display()
paths.users_file.display()
);
}
@ -256,10 +248,11 @@ fn user_add(paths: &Paths, args: AddArgs) -> Result<()> {
password: generated.digest,
email: args.email,
groups: args.groups,
extra: std::collections::BTreeMap::new(),
},
);
publish(paths, &store)?;
publish(paths, &mut store)?;
println!("added {} to {}", args.username, paths.users_file.display());
println!("password: {}", generated.password);
@ -268,12 +261,12 @@ fn user_add(paths: &Paths, args: AddArgs) -> Result<()> {
}
fn user_update(paths: &Paths, args: UpdateArgs) -> Result<()> {
let mut store = load_store(&paths.store, &paths.users_file)?;
let mut store = load_store(&paths.users_file)?;
let Some(user) = store.users.get_mut(&args.username) else {
bail!(
"no user {:?} in {} — `swarmctl user add` creates one",
args.username,
paths.store.display()
paths.users_file.display()
);
};
@ -292,7 +285,7 @@ fn user_update(paths: &Paths, args: UpdateArgs) -> Result<()> {
// list is the only way to notice.
let groups = users::fmt_groups(&user.groups);
publish(paths, &store)?;
publish(paths, &mut store)?;
for change in &changes {
println!("{change}");
@ -306,9 +299,9 @@ fn user_update(paths: &Paths, args: UpdateArgs) -> Result<()> {
fn user_list(paths: &Paths) -> Result<()> {
use std::fmt::Write as _;
let store = load_store(&paths.store, &paths.users_file)?;
let store = load_store(&paths.users_file)?;
if store.users.is_empty() {
println!("no users in {}", paths.store.display());
println!("no users in {}", paths.users_file.display());
return Ok(());
}
for (username, user) in &store.users {
@ -324,23 +317,23 @@ fn user_list(paths: &Paths) -> Result<()> {
Ok(())
}
/// Write the store + the rendered users file, then restart authelia.
/// Write the users database.
///
/// Shared by every verb that mutates the store, so the ordering rules
/// below hold for all of them rather than for whichever one was written
/// first.
fn publish(paths: &Paths, store: &UserStore) -> Result<()> {
// Render before writing anything: a value this refuses to emit should
// stop the whole operation, not leave the canonical store one user
// ahead of the file authelia reads.
let rendered = users::render_yaml(store)?;
let store_json = serde_json::to_string_pretty(store).context("serialising the user store")?;
/// Shared by every verb that mutates it, so the rules below hold for all of
/// them rather than for whichever one was written first.
fn publish(paths: &Paths, store: &mut UserStore) -> Result<()> {
// Before rendering, not at creation: a user can also arrive by being
// *read* — from a file the bridge wrote, or one an operator edited —
// and an authelia subject with no `email` breaks any relying party that
// asks for the claim (grafana's OIDC login is the measured case,
// #3393). Filling it here is the only place no entry point can skip.
for name in users::fill_missing_emails(store) {
println!("note: {name} had no email; set to a synthetic address");
}
// Store first, and the order matters. If the store lands and the
// users file does not, the next run re-renders and repairs it. The
// other order loses a user: the store would not know about someone
// authelia does, and the next render would silently drop them.
write_atomic(&paths.store, &format!("{store_json}\n"))?;
// Render before writing: a value this refuses to emit should stop the
// whole operation rather than land a partial file.
let rendered = users::render_yaml(store)?;
write_atomic(&paths.users_file, &rendered)?;
// No restart: authelia watches this file
@ -351,38 +344,25 @@ fn publish(paths: &Paths, store: &UserStore) -> Result<()> {
Ok(())
}
/// Load the canonical store, or start an empty one if this deployment has
/// never had a user added.
/// Load the users database, or start empty when it does not exist yet.
///
/// The guard exists because starting empty means the next write
/// **overwrites** authelia's users file. That is only safe when the file
/// is the untouched first-boot seed; anything else is a user database
/// somebody meant to be there, and losing it is the one unrecoverable
/// mistake this tool can make.
fn load_store(store_path: &Path, users_file: &Path) -> Result<UserStore> {
match fs::read_to_string(store_path) {
Ok(raw) => serde_json::from_str(&raw)
.with_context(|| format!("parsing the user store at {}", store_path.display())),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
match fs::read_to_string(users_file) {
Ok(existing) if !users::is_untouched_seed(&existing) => bail!(
"no user store at {} but {} already holds users — refusing to \
overwrite it. Reconstruct the store, or move the file aside if \
it is disposable.",
store_path.display(),
users_file.display()
),
Ok(_) => Ok(UserStore::default()),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(UserStore::default()),
Err(e) => Err(e).with_context(|| {
format!(
"reading {} to check it is safe to take over",
users_file.display()
)
}),
}
}
Err(e) => Err(e).with_context(|| format!("reading {}", store_path.display())),
/// ⚠️ **The overwrite guard that used to live here is gone, and its absence
/// is the fix rather than a regression.** It refused to write when this
/// crate's own JSON store was missing but `users.yml` held users — it
/// existed to police *two* stores that could disagree, and there is now one.
/// Reading the same file we are about to write means we cannot clobber users
/// we did not know about: we just read them.
///
/// An absent or empty file is an empty store, not an error. First boot is a
/// legitimate state, and the seed document authelia's own unit writes
/// (`users: {}`) deserialises to exactly that with no special case.
fn load_store(users_file: &Path) -> Result<UserStore> {
match fs::read_to_string(users_file) {
Ok(raw) if raw.trim().is_empty() => Ok(UserStore::default()),
Ok(raw) => serde_norway::from_str(&raw)
.with_context(|| format!("parsing the users database at {}", users_file.display())),
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(UserStore::default()),
Err(e) => Err(e).with_context(|| format!("reading {}", users_file.display())),
}
}
@ -562,25 +542,37 @@ mod tests {
fs::remove_dir_all(&dir).ok();
}
/// The guard that stands between a missing store and an overwritten
/// user database.
/// Replaces `load_store_refuses_to_take_over_a_populated_users_file`,
/// which pinned the guard this change removes.
///
/// The guard refused to write when this crate's own JSON store was
/// missing but `users.yml` held users — the state every real hive is in,
/// and the reason `swarm agent create` failed. Deleting the test with
/// the guard would have left the *new* behaviour unpinned, so it is
/// inverted rather than dropped: a populated file must now be **read**,
/// and users somebody else wrote must survive.
#[test]
fn load_store_refuses_to_take_over_a_populated_users_file() {
let dir = std::env::temp_dir().join(format!("swarmctl-guard-{}", std::process::id()));
fn a_populated_users_file_is_read_rather_than_refused() {
let dir = std::env::temp_dir().join(format!("swarmctl-takeover-{}", std::process::id()));
fs::create_dir_all(&dir).expect("temp dir");
let store = dir.join("absent.json");
let users_file = dir.join("users.yml");
fs::write(&users_file, "users:\n mara:\n password: \"x\"\n").expect("write");
let err = load_store(&store, &users_file).expect_err("must refuse");
fs::write(
&users_file,
"users:\n mara:\n displayname: \"mara\"\n password: \"x\"\n",
)
.expect("write");
let store = load_store(&users_file).expect("a populated file is readable");
assert!(
err.to_string().contains("refusing to overwrite"),
"unexpected error: {err}"
store.users.contains_key("mara"),
"a user this process did not write must be read, not refused"
);
fs::write(&users_file, "users: {}\n").expect("seed");
let taken = load_store(&store, &users_file).expect("the seed is takeable");
assert!(taken.users.is_empty());
assert!(
load_store(&users_file).expect("the seed loads").users.is_empty(),
"the first-boot seed is an empty store, with no special case"
);
fs::remove_dir_all(&dir).ok();
}