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

`swarm agent create` failed with "no user store at
swarm-authelia-bridge-users.json but users.yml already holds users" on
any hive that had users. The guard was correct; what was wrong is that
two files both claimed to be canonical for one physical file.

The bridge kept a private users.json and rendered users.yml from it,
while swarmctl kept its own pair against the same users.yml. A writer
whose own JSON was absent could not tell "nothing here yet" from
"someone else's users", so it refused to write at all.

users.yml becomes the store: read before write, through serde_norway
rather than a hand-rolled emitter. Unknown top-level and per-user keys
round-trip through `extra`, or whichever process writes second would
silently delete what the first added. Validation stays on the write path
-- a bare to_string(&store) serialises perfectly and drops the "no
control character ever reaches this file" guarantee silently.

The seed constant goes with the guard: nothing writes a seed now, an
absent file is an empty store, and left as a pub constant it read as if
the seed dance were still load-bearing.
This commit is contained in:
atlas 2026-08-18 10:31:48 +02:00
commit 24f4cd42a9
5 changed files with 190 additions and 101 deletions

21
Cargo.lock generated
View file

@ -4324,6 +4324,19 @@ dependencies = [
"serde",
]
[[package]]
name = "serde_norway"
version = "0.9.42"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e408f29489b5fd500fab51ff1484fc859bb655f32c671f307dcd733b72e8168c"
dependencies = [
"indexmap",
"itoa",
"ryu",
"serde",
"unsafe-libyaml-norway",
]
[[package]]
name = "serde_path_to_error"
version = "0.1.20"
@ -4564,6 +4577,7 @@ dependencies = [
"reqwest 0.13.1",
"serde",
"serde_json",
"serde_norway",
"swarm-authelia-bridge-sock",
"tokio",
"tracing",
@ -4649,6 +4663,7 @@ dependencies = [
"clap_complete",
"serde",
"serde_json",
"serde_norway",
]
[[package]]
@ -5236,6 +5251,12 @@ dependencies = [
"subtle",
]
[[package]]
name = "unsafe-libyaml-norway"
version = "0.2.15"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b39abd59bf32521c7f2301b52d05a6a2c975b6003521cbd0c6dc1582f0a22104"
[[package]]
name = "untrusted"
version = "0.9.0"

View file

@ -14,6 +14,7 @@ axum.workspace = true
reqwest.workspace = true
serde.workspace = true
serde_json.workspace = true
serde_norway = "0.9.42"
swarm-authelia-bridge-sock.workspace = true
tokio.workspace = true
tracing.workspace = true

View file

@ -47,7 +47,6 @@ use swarm_authelia_bridge_sock::{BridgeRequest, BridgeResponse};
/// that provisions this bridge's own client alongside authelia.
struct Config {
bind: String,
store_path: std::path::PathBuf,
users_file: std::path::PathBuf,
authelia_bin: std::path::PathBuf,
introspection_url: String,
@ -59,7 +58,6 @@ impl Config {
fn from_env() -> Result<Self> {
Ok(Self {
bind: env_var("SWARM_AUTHELIA_BRIDGE_BIND")?,
store_path: env_var("SWARM_AUTHELIA_BRIDGE_STORE")?.into(),
users_file: env_var("SWARM_AUTHELIA_BRIDGE_USERS_FILE")?.into(),
authelia_bin: env_var("SWARM_AUTHELIA_BRIDGE_AUTHELIA_BIN")?.into(),
introspection_url: env_var("SWARM_AUTHELIA_BRIDGE_INTROSPECTION_URL")?,
@ -265,7 +263,7 @@ async fn handle(state: &AppState, name: String) -> Result<BridgeResponse> {
// practice, not just theoretically.
let _write_guard = state.write_lock.lock().await;
let mut user_store = store::load_store(&cfg.store_path, &cfg.users_file)?;
let mut user_store = store::load_store(&cfg.users_file)?;
if user_store.users.contains_key(&name) {
return Ok(BridgeResponse::AlreadyExists);
}
@ -277,9 +275,10 @@ async fn handle(state: &AppState, name: String) -> Result<BridgeResponse> {
password: generated,
email: None,
groups: Vec::new(),
extra: std::collections::BTreeMap::new(),
},
);
store::publish(&cfg.store_path, &cfg.users_file, &user_store)?;
store::publish(&cfg.users_file, &user_store)?;
tracing::info!(agent = %name, "created authelia identity");
Ok(BridgeResponse::Created)
}

View file

@ -13,15 +13,27 @@
//! `swarm-authelia` and `swarm-controller` split across hosts, and this
//! bridge only ever runs where `swarm-authelia` does.
//!
//! ⚠️ **Known limitation, not solved here**: `swarmctl` still writes its
//! own independent `users.json`/`users.yml` for human accounts, assuming
//! co-location with `swarm-controller`'s host. Two independent canonical
//! stores for the same physical `users.yml` is a real seam — tracked as a
//! follow-up (route `swarmctl` through this bridge too), not attempted in
//! this slice, whose scope is agent identities only.
//! # `users.yml` is the store, not a rendering of one
//!
//! There used to be a private `users.json` here, canonical, with `users.yml`
//! rendered from it — and `swarmctl` had its own pair against the *same*
//! physical `users.yml`. Two canonical stores for one file is a seam, and it
//! bit: a bridge whose own JSON was absent refused to write at all, because
//! it could not tell "nothing here yet" from "someone else's users".
//!
//! The JSON bought nothing. It was read back on every load, so the
//! round-trip it seemed to avoid was already being paid — the two files
//! differed only in *format*. One file removes the class: there is no second
//! store to disagree with, and the overwrite guard that policed them has
//! nothing left to do.
//!
//! ⚠️ Consequence, deliberate: this file is now **round-tripped**, so
//! comments and hand-formatting in it do not survive a write. An operator
//! editing it directly gets their *values* kept and their *comments* dropped.
//! Unknown keys are preserved (see `extra` below) so a field this binary does
//! not know about is not deleted by it.
use std::collections::BTreeMap;
use std::fmt::Write as _;
use std::fs::{self, File, Permissions};
use std::io::Write as _;
use std::os::unix::fs::PermissionsExt as _;
@ -30,17 +42,21 @@ use std::path::Path;
use anyhow::{Context, Result, bail};
use serde::{Deserialize, Serialize};
/// What the `swarm-authelia` module's first-boot unit writes into
/// `users.yml` when there is no database yet — same constant/shape
/// `swarmctl::users::SEED_USERS_FILE` uses, since both write the same
/// physical file format.
pub const SEED_USERS_FILE: &str = "users: {}";
/// The canonical store, serialised as JSON. `BTreeMap` for a stable,
/// diffable render — same rationale as `swarmctl::users::UserStore`.
/// The canonical store — this *is* `users.yml`, deserialised. `BTreeMap`
/// for a stable, diffable file: authelia does not care about key order, but
/// a human reading `git diff` does.
#[derive(Debug, Default, Serialize, Deserialize)]
pub struct UserStore {
pub users: BTreeMap<String, User>,
/// Top-level keys this binary does not model, carried through a
/// round-trip untouched.
///
/// Two processes write this file and neither is authoritative about the
/// other's fields. Without this, whichever writes second silently
/// deletes anything the first added — the same shape as the bug that
/// made one file canonical in the first place, one level down.
#[serde(flatten)]
pub extra: BTreeMap<String, serde_norway::Value>,
}
#[derive(Debug, Serialize, Deserialize)]
@ -52,6 +68,10 @@ pub struct User {
pub email: Option<String>,
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub groups: Vec<String>,
/// Per-user keys this binary does not model (authelia's `disabled`, for
/// one). Same preservation rule as [`UserStore::extra`].
#[serde(flatten)]
pub extra: BTreeMap<String, serde_norway::Value>,
}
/// Same conservative charset `swarmctl::users::validate_username` already
@ -87,99 +107,74 @@ fn reject_control_chars(field: &str, value: &str) -> Result<()> {
Ok(())
}
/// A double-quoted YAML scalar — everything is quoted, including values
/// that would be fine bare, since an argon2 digest alone contains `$`,
/// `=`, `,` and `/`. Control characters are excluded upstream, so `"` and
/// `\` are the complete escape set.
fn quote(s: &str) -> String {
let mut out = String::with_capacity(s.len() + 2);
out.push('"');
for c in s.chars() {
match c {
'"' => out.push_str("\\\""),
'\\' => out.push_str("\\\\"),
_ => out.push(c),
}
}
out.push('"');
out
/// Render the store as authelia's YAML users database.
///
/// Serialisation is `serde_norway`'s — quoting an argon2 digest (`$`, `=`,
/// `,`, `/`) is exactly the kind of thing a real emitter gets right and a
/// hand-rolled one gets right until it doesn't.
///
/// ⚠️ **Still fallible, and that is the load-bearing part.** It re-validates
/// every value it is about to emit, so *"no control character ever reaches
/// this file"* stays a property of the **one path that writes it** rather
/// than a rule every caller has to remember. A bare `to_string(&store)`
/// would serialise perfectly and drop that guarantee silently.
pub fn render_yaml(store: &UserStore) -> Result<String> {
validate(store)?;
serde_norway::to_string(store).context("serialising the users database")
}
/// Render the store as authelia's block-style YAML users database.
/// Fallible for the same reason `swarmctl::users::render_yaml` is: it
/// re-validates every value it is about to emit, so "no control character
/// ever reaches the file" is a property of the one path that writes it.
pub fn render_yaml(store: &UserStore) -> Result<String> {
if store.users.is_empty() {
return Ok(format!("{SEED_USERS_FILE}\n"));
}
let mut out = String::from("users:\n");
/// Everything [`render_yaml`] refuses to write. Separate so the rule can be
/// tested directly, and so a second writer can call it without going
/// through serialisation.
fn validate(store: &UserStore) -> Result<()> {
for (name, user) in &store.users {
validate_username(name)?;
reject_control_chars("displayname", &user.displayname)?;
reject_control_chars("password digest", &user.password)?;
writeln!(out, " {name}:")?;
writeln!(out, " displayname: {}", quote(&user.displayname))?;
writeln!(out, " password: {}", quote(&user.password))?;
if let Some(email) = &user.email {
reject_control_chars("email", email)?;
writeln!(out, " email: {}", quote(email))?;
}
if !user.groups.is_empty() {
writeln!(out, " groups:")?;
for group in &user.groups {
reject_control_chars("group", group)?;
writeln!(out, " - {}", quote(group))?;
}
for group in &user.groups {
reject_control_chars("group", group)?;
}
}
Ok(out)
Ok(())
}
/// Load the canonical store, or start an empty one if this bridge has
/// never written a user. Same overwrite guard as
/// `swarmctl::load_store`: starting empty means the next write
/// **overwrites** `users_file`, which is only safe when that file is
/// still the untouched first-boot seed.
pub 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 !is_untouched_seed(&existing) => bail!(
"no user store at {} but {} already holds users — refusing to \
overwrite it",
store_path.display(),
users_file.display()
),
Ok(_) | Err(_) => Ok(UserStore::default()),
}
}
Err(e) => Err(e).with_context(|| format!("reading {}", store_path.display())),
/// Load the users database, or start empty when it does not exist yet.
///
/// ⚠️ **There is deliberately no overwrite guard here any more.** The old
/// one refused to write when a private JSON store was missing but
/// `users.yml` held users — it existed to police *two* stores that could
/// disagree, and with one store there is nothing to disagree with. 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.
pub 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())),
}
}
/// Whether an existing `users.yml` is safe to take over — i.e. it is the
/// untouched first-boot seed. Empty counts too: a zero-byte file holds
/// nothing to lose.
fn is_untouched_seed(contents: &str) -> bool {
let trimmed = contents.trim();
trimmed.is_empty() || trimmed == SEED_USERS_FILE
}
/// Write the store + the rendered users file. **No restart** (the load-
/// bearing difference from `swarmctl::publish`): this bridge relies on
/// authelia's `authentication_backend.file.watch`, confirmed working
/// against the pinned 4.39.20 build during this design work — a
/// restart would drop every active SSO session, which mara ruled out for
/// agent creation specifically (not a rare, human-initiated event).
pub fn publish(store_path: &Path, users_file: &Path, store: &UserStore) -> Result<()> {
pub fn publish(users_file: &Path, store: &UserStore) -> Result<()> {
// One file, so there is no longer an ordering question between two
// writes — the old version wrote the JSON store first so a crash
// between them left something to repair from. That whole failure mode
// belonged to having two files.
let rendered = render_yaml(store)?;
let store_json = serde_json::to_string_pretty(store).context("serialising the user store")?;
// Store first — if the store lands and the users file doesn't, the
// next run re-renders and repairs it. The other order loses a user.
write_atomic(store_path, &format!("{store_json}\n"))?;
write_atomic(users_file, &rendered)
}
@ -225,29 +220,76 @@ fn write_atomic(path: &Path, contents: &str) -> Result<()> {
mod tests {
use super::*;
/// What the `swarm-authelia` module's first-boot unit writes into
/// `users.yml` when there is no database yet.
///
/// A test fixture rather than a production constant: nothing in this
/// binary writes a seed any more — the store deserialises whatever is
/// there and an absent file is an empty store. Keeping it `pub` in the
/// module would be a constant nothing reads, which is exactly the kind
/// of leftover that makes the next reader think the seed dance is still
/// load-bearing.
const SEED_USERS_FILE: &str = "users: {}";
fn user(password: &str) -> User {
User {
displayname: "atlas".to_owned(),
password: password.to_owned(),
email: None,
groups: Vec::new(),
extra: BTreeMap::new(),
}
}
/// The document `swarm-authelia`'s first-boot unit writes must load as
/// an empty store with no special case — otherwise a fresh hive's very
/// first `EnsureAgentIdentity` fails on a file we wrote ourselves.
#[test]
fn an_empty_store_renders_the_seed_document() {
let out = render_yaml(&UserStore::default()).expect("renders");
assert_eq!(out, "users: {}\n");
assert!(is_untouched_seed(&out));
fn the_first_boot_seed_loads_as_an_empty_store() {
let store: UserStore = serde_norway::from_str(SEED_USERS_FILE).expect("seed parses");
assert!(store.users.is_empty());
assert!(store.extra.is_empty());
}
/// An argon2 digest is `$`, `=`, `,` and `/` — the reason a real
/// emitter replaced the hand-rolled one. Asserted by **round-trip**,
/// not by looking for quotes: how the emitter chooses to quote is its
/// business, that the value survives is ours.
#[test]
fn an_argon2_digest_survives_quoting() {
fn an_argon2_digest_survives_a_round_trip() {
let digest = "$argon2id$v=19$m=65536,t=3,p=4$c29tZXNhbHQ$aGFzaA+/w==";
let mut store = UserStore::default();
store.users.insert("atlas".to_owned(), user(digest));
let out = render_yaml(&store).expect("renders");
assert!(out.contains(&format!("password: \"{digest}\"")));
let back: UserStore = serde_norway::from_str(&out).expect("reparses");
assert_eq!(back.users["atlas"].password, digest);
}
/// A key neither writer models must survive the other's write. Without
/// this, two processes sharing one file silently delete each other's
/// fields — the same class of bug as the two stores this replaced.
#[test]
fn unknown_keys_survive_a_round_trip() {
let source = "\
users:
atlas:
displayname: atlas
password: \"$argon2id$x\"
disabled: true
some_future_top_level_key: 7
";
let store: UserStore = serde_norway::from_str(source).expect("parses");
let out = render_yaml(&store).expect("renders");
assert!(
out.contains("disabled"),
"a per-user key this binary does not model was dropped: {out}"
);
assert!(
out.contains("some_future_top_level_key"),
"a top-level key this binary does not model was dropped: {out}"
);
}
#[test]
@ -276,20 +318,45 @@ mod tests {
/// `write_atomic` round-trips through a real temp dir — the property
/// under test is the rename-into-place, not just the render.
#[test]
fn publish_writes_both_files_and_the_store_reloads() {
fn publish_then_load_round_trips_through_the_real_file() {
let dir = tempdir();
let store_path = dir.join("users.json");
let users_file = dir.join("users.yml");
fs::write(&users_file, SEED_USERS_FILE).unwrap();
let mut store = UserStore::default();
store.users.insert("atlas".to_owned(), user("$argon2id$x"));
publish(&store_path, &users_file, &store).expect("publish");
publish(&users_file, &store).expect("publish");
let reloaded = load_store(&store_path, &users_file).expect("reload");
let reloaded = load_store(&users_file).expect("reload");
assert!(reloaded.users.contains_key("atlas"));
fs::remove_dir_all(&dir).ok();
}
/// The bug this change closes: a `users.yml` holding users that this
/// binary did not write is **loaded**, not refused. The old code kept a
/// private store alongside it and bailed when the two disagreed.
#[test]
fn a_users_file_written_by_someone_else_is_read_not_refused() {
let dir = tempdir();
let users_file = dir.join("users.yml");
fs::write(
&users_file,
"users:\n mara:\n displayname: mara\n password: \"$argon2id$x\"\n",
)
.unwrap();
let mut store = load_store(&users_file).expect("must load a foreign users file");
assert!(store.users.contains_key("mara"));
// …and adding to it keeps the existing user rather than replacing.
store
.users
.insert("atlas".to_owned(), user("$argon2id$second"));
publish(&users_file, &store).expect("publish");
let reloaded = load_store(&users_file).expect("reload");
assert!(reloaded.users.contains_key("mara"), "existing user lost");
assert!(reloaded.users.contains_key("atlas"));
let yaml = fs::read_to_string(&users_file).unwrap();
assert!(yaml.contains("atlas"));
fs::remove_dir_all(&dir).ok();
}

View file

@ -15,6 +15,7 @@ clap-markdown = "0.1"
clap_complete.workspace = true
serde.workspace = true
serde_json.workspace = true
serde_norway = "0.9.42"
[lints]
workspace = true