From bd214b9f3e37f2894a70d2f584d3898d5bea3664 Mon Sep 17 00:00:00 2001 From: atlas Date: Tue, 27 Oct 2026 10:46:36 +0100 Subject: [PATCH] require both attic options, follow attic's cache-name rule, read the token from a file --- docs/swarm/builder.md | 14 ++- nix/host-modules/swarm-attic-options.nix | 9 +- nix/host-modules/swarm-builder.nix | 18 ++-- nix/module-eval/builder.nix | 39 ++++++++ swarm-builder/src/push.rs | 117 +++++++++++++++++------ 5 files changed, 156 insertions(+), 41 deletions(-) diff --git a/docs/swarm/builder.md b/docs/swarm/builder.md index 87e4da60..10385504 100644 --- a/docs/swarm/builder.md +++ b/docs/swarm/builder.md @@ -267,12 +267,16 @@ inside the 61 minutes the controller waits. `attic` checks the server's certific against the system roots and pins nothing, so a Let's Encrypt certificate works. +Set both options or neither: evaluation refuses a domain without a cache name. +The cache name follows attic's rule, 1 to 50 letters, digits, `-`, `_` or `+`, +starting with a letter or digit, and evaluation refuses any other name. + The push token is the operator's to write. The builder reads it on every push -from `secret/swarm/services/attic/push-token` (field `value`), writes an `attic` -config file holding it in the unit's private runtime directory (`/run/swarm-builder`, -mode `0700`, the file `0600`), points `attic` at it with `XDG_CONFIG_HOME` and -removes it after the push. The token is in no argument or variable, so no -process listing shows it. +from `secret/swarm/services/attic/push-token` (field `value`), writes it to +a file and an `attic` config naming that file (`token-file`) in the unit's private +runtime directory (`/run/swarm-builder`, mode `0700`, both files `0600`), points +`attic` at the config with `XDG_CONFIG_HOME` and removes both after the push. +The token is in no argument or variable, so no process listing shows it. The reply's `cached` says whether the push finished. A push that fails, times out or can't read the token leaves the build green with `cached` false and logs a diff --git a/nix/host-modules/swarm-attic-options.nix b/nix/host-modules/swarm-attic-options.nix index 481ffa0d..6b8858a9 100644 --- a/nix/host-modules/swarm-attic-options.nix +++ b/nix/host-modules/swarm-attic-options.nix @@ -12,7 +12,8 @@ Name of the attic server, with no scheme: the swarm builder pushes to `https://`, and trusts the certificate the system roots do, so a Let's Encrypt certificate works and none is pinned. Set together with - {option}`services.hyperhive.swarm.attic.cache`. + {option}`services.hyperhive.swarm.attic.cache`; evaluation fails when only + one is set. Null pushes nothing: the swarm builder does not push, and the closure record says it is not cached. @@ -24,13 +25,15 @@ }; cache = lib.mkOption { - type = lib.types.nullOr lib.types.str; + # attic's own rule for a cache name; the builder checks the same at start. + type = lib.types.nullOr (lib.types.strMatching "^[A-Za-z0-9][A-Za-z0-9_+-]{0,49}$"); default = null; example = "grid"; description = '' Name of the cache on {option}`services.hyperhive.swarm.attic.domain` that the swarm builder pushes closures to. The push token must be - allowed to push to it. + allowed to push to it. The name follows attic's rule: 1 to 50 letters, + digits, `-`, `_` or `+`, starting with a letter or digit. ''; }; }; diff --git a/nix/host-modules/swarm-builder.nix b/nix/host-modules/swarm-builder.nix index 49ff166a..1fb12bd7 100644 --- a/nix/host-modules/swarm-builder.nix +++ b/nix/host-modules/swarm-builder.nix @@ -90,8 +90,8 @@ let hyperhiveFlake = hyperhiveCfg.c0re.hyperhiveFlake; nixpkgsFlake = hyperhiveCfg.c0re.nixpkgsFlake; - # Either half alone reaches the daemon too, which refuses to start on it. - pushesToCache = atticCfg.domain != null || atticCfg.cache != null; + # The assertion below keeps the two options together. + pushesToCache = atticCfg.domain != null; privateNetwork = true; in @@ -133,6 +133,14 @@ in host's /var/lib/swarm-bao-pki/ and name them in those two options. ''; } + { + assertion = (atticCfg.domain == null) == (atticCfg.cache == null); + message = '' + services.hyperhive.swarm.attic.domain and services.hyperhive.swarm.attic.cache + must be set together: the swarm builder pushes to a cache on a server, and + either alone names nothing to push to. + ''; + } { assertion = lib.all oneWord (lib.attrValues storeFiles); message = '' @@ -257,10 +265,8 @@ in SWARM_BUILDER_FORGE_URL = "https://${forgeCfg.domain}"; SWARM_BUILDER_CONFIG_ORG = "agent-configs"; } - // lib.optionalAttrs (atticCfg.domain != null) { + // lib.optionalAttrs pushesToCache { SWARM_BUILDER_ATTIC_DOMAIN = atticCfg.domain; - } - // lib.optionalAttrs (atticCfg.cache != null) { SWARM_BUILDER_ATTIC_CACHE = atticCfg.cache; } // lib.optionalAttrs (storeFiles ? builder-bao-server-ca) { @@ -276,7 +282,7 @@ in DynamicUser = true; CacheDirectory = unit; # /run/swarm-builder, mode 0700 and owned by the dynamic user: the - # push writes its token-bearing `attic` config under it. + # push writes its token file and `attic` config under it. RuntimeDirectory = unit; RuntimeDirectoryMode = "0700"; # Bare ids: the system credentials nspawn handed in. diff --git a/nix/module-eval/builder.nix b/nix/module-eval/builder.nix index 0ec09f6d..97cddf4c 100644 --- a/nix/module-eval/builder.nix +++ b/nix/module-eval/builder.nix @@ -39,6 +39,21 @@ let m: lib.any (a: !a.assertion && lib.hasInfix "deploy.builder.enable requires" a.message) m.assertions; + # The one fixture that fires the cache assertion: a domain with no cache name. + halfCache = hive { + deploy.bao.enable = true; + deploy.builder.enable = true; + swarm.attic.domain = "cache.example.org"; + }; + refusedHalfCache = + m: + lib.any ( + a: + !a.assertion && lib.hasInfix "swarm.attic.domain and services.hyperhive.swarm.attic.cache" a.message + ) m.assertions; + # Only the option is read, so the host around it is never evaluated. + cacheNamed = name: (hive { swarm.attic.cache = name; }).services.hyperhive.swarm.attic.cache; + natsHere = shared.natsQueueOnly; autheliaHere = hive { @@ -139,6 +154,30 @@ let && !(withoutCache.environment ? SWARM_BUILDER_ATTIC_DOMAIN) && !(withoutCache.environment ? SWARM_BUILDER_ATTIC_CACHE); } + { + # A domain with no cache name would crash-loop the builder at start, so + # evaluation refuses it. `withBuilder`, with both set, is the control: the + # same assertion list holds the entry and it passes. + name = "the builder refuses a cache domain without a cache name"; + ok = + refusedHalfCache halfCache + && !(refusedHalfCache withBuilder) + && lib.any (a: lib.hasInfix "swarm.attic.domain and" a.message) withBuilder.assertions; + } + { + # attic refuses a cache name outside its rule, which would fail every push. + # `grid` is the control that the lookup reads a name. + name = "the cache name must follow attic's rule"; + ok = + cacheNamed "grid" == "grid" + && cacheNamed "A-b_c+d" == "A-b_c+d" + && lib.all (n: !(builtins.tryEval (cacheNamed n)).success) [ + "grid.v2" + "-grid" + "" + (lib.concatStrings (lib.genList (_: "a") 51)) + ]; + } { # The minting unit goes inactive after each run, so a start that pulls it # runs it again and mints a leaf that is missing; an active diff --git a/swarm-builder/src/push.rs b/swarm-builder/src/push.rs index 30493f7a..8e5726d3 100644 --- a/swarm-builder/src/push.rs +++ b/swarm-builder/src/push.rs @@ -1,10 +1,11 @@ //! The push of a built closure to the binary cache, with `attic push`. //! //! The cache is an attic instance outside the swarm. Its push token is read -//! from the secret store per push and written to an `attic` config file in a -//! directory only the builder's unit can enter, which `attic` finds through -//! `XDG_CONFIG_HOME`. `attic login ` is not used: it puts -//! the token in the process list. `attic` verifies the server's certificate +//! from the secret store per push and written to a file in a +//! directory only the builder's unit can enter, beside a config that names it with +//! `token-file`. `attic` finds the config through `XDG_CONFIG_HOME`. +//! `attic login ` is not used: it puts the token in the +//! process list. `attic` verifies the server's certificate //! against the system roots. //! //! A push that does not happen or does not finish leaves the build green with @@ -54,13 +55,15 @@ impl Remote { (Some(domain), Some(cache)) => (domain, cache), _ => bail!("the cache domain and the cache name must be set together"), }; - let plain = |s: &str| { - !s.is_empty() - && s.bytes() - .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'-' | b'_' | b':')) - }; - if !plain(&domain) || !plain(&cache) { - bail!("the cache domain {domain:?} and name {cache:?} must be plain names"); + let plain_domain = !domain.is_empty() + && domain + .bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'-' | b':')); + if !plain_domain { + bail!("the cache domain {domain:?} is not a host name with an optional port"); + } + if !cache_name_ok(&cache) { + bail!("the cache name {cache:?} is not one attic accepts"); } let Some(state_dir) = state_dir else { bail!("a cache is set, but the unit has no runtime directory to hold its config"); @@ -106,7 +109,7 @@ impl Remote { } /// `attic push` of `out_path`, reading its config from `config_home`. The - /// token is in the file there and in no argument or variable. + /// token is in a file beside it and in no argument or variable. fn command(&self, config_home: &Path, out_path: &str) -> tokio::process::Command { let mut cmd = tokio::process::Command::new(&self.attic); cmd.arg("push") @@ -121,6 +124,16 @@ impl Remote { } } +/// Whether `name` is a cache name attic accepts, which is +/// `^[A-Za-z0-9][A-Za-z0-9-_+]{0,49}$`. `services.hyperhive.swarm.attic.cache` +/// states the same rule in nix. +fn cache_name_ok(name: &str) -> bool { + let mut bytes = name.bytes(); + bytes.next().is_some_and(|b| b.is_ascii_alphanumeric()) + && name.len() <= 50 + && bytes.all(|b| b.is_ascii_alphanumeric() || matches!(b, b'-' | b'_' | b'+')) +} + /// Numbers the config directories of one process's pushes. static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); @@ -128,14 +141,14 @@ static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0) struct ConfigHome(PathBuf); impl ConfigHome { - /// `/push-/attic/config.toml`, readable by this user alone. + /// `/push-/attic/config.toml`, which names `token` beside it, both + /// readable by this user alone. fn write(parent: &Path, domain: &str, token: &str) -> std::io::Result { use std::io::Write as _; use std::os::unix::fs::{DirBuilderExt as _, OpenOptionsExt as _}; - // A quote or a control character would end the TOML string early. - if token.is_empty() || token.bytes().any(|b| b == b'"' || b == b'\\' || b < b' ') { + if token.is_empty() || token.bytes().any(|b| b < b' ') { return Err(std::io::Error::other( - "the push token is empty or holds a character the config cannot", + "the push token is empty or holds a control character", )); } let home = Self(parent.join(format!( @@ -144,19 +157,30 @@ impl ConfigHome { NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed) ))); let dir = home.0.join("attic"); + let token_file = dir.join("token"); + let token_path = token_file.to_string_lossy(); + // A quote or backslash in the path would end the TOML string early. + if token_path.contains(['"', '\\']) { + return Err(std::io::Error::other( + "the runtime directory's path holds a quote or a backslash", + )); + } std::fs::DirBuilder::new() .recursive(true) .mode(0o700) .create(&dir)?; - let mut file = std::fs::OpenOptions::new() - .write(true) - .create_new(true) - .mode(0o600) - .open(dir.join("config.toml"))?; + let private = |name: &str| { + std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(dir.join(name)) + }; + private("token")?.write_all(token.as_bytes())?; write!( - file, + private("config.toml")?, "default-server = \"{SERVER}\"\n\n[servers.{SERVER}]\n\ - endpoint = \"https://{domain}\"\ntoken = \"{token}\"\n" + endpoint = \"https://{domain}\"\ntoken-file = \"{token_path}\"\n" )?; Ok(home) } @@ -378,19 +402,26 @@ mod tests { } #[test] - fn the_token_is_in_a_private_config_file_that_is_removed_after() { + fn the_token_is_in_a_private_file_the_config_names_and_both_are_removed_after() { let parent = std::env::temp_dir().join(format!("push-test-{}", std::process::id())); std::fs::create_dir_all(&parent).expect("parent"); let home = ConfigHome::write(&parent, "cache.example.org", TOKEN).expect("written"); let file = home.0.join("attic/config.toml"); let text = std::fs::read_to_string(&file).expect("reads"); - assert!(text.contains(&format!("token = \"{TOKEN}\"")), "{text}"); + let token_file = home.0.join("attic/token"); + assert!( + text.contains(&format!("token-file = \"{}\"", token_file.display())), + "{text}" + ); + assert!(!text.contains(TOKEN), "{text}"); + assert_eq!(std::fs::read_to_string(&token_file).expect("reads"), TOKEN); assert!( text.contains("endpoint = \"https://cache.example.org\""), "{text}" ); let mode = |p: &Path| std::fs::metadata(p).expect("stat").permissions().mode() & 0o777; assert_eq!(mode(&file), 0o600); + assert_eq!(mode(&token_file), 0o600); assert_eq!(mode(&home.0), 0o700); let root = home.0.clone(); drop(home); @@ -399,13 +430,42 @@ mod tests { } #[test] - fn a_token_that_would_end_the_config_string_is_refused() { + fn an_empty_token_or_one_with_a_control_character_is_refused() { let parent = std::env::temp_dir(); - for bad in ["", "a\"b", "a\\b", "a\nb"] { + for bad in ["", "a\nb", "a\0b"] { assert!(ConfigHome::write(&parent, "h", bad).is_err(), "{bad:?}"); } } + #[test] + fn a_path_that_would_end_the_config_string_is_refused() { + let parent = std::env::temp_dir().join("a\"b"); + assert!(ConfigHome::write(&parent, "h", TOKEN).is_err()); + assert!(!parent.exists()); + } + + #[test] + fn the_cache_name_rule_is_attics() { + let fifty = "a".repeat(50); + for ok in ["grid", "a", "A-b_c+d", "0", fifty.as_str()] { + assert!(cache_name_ok(ok), "{ok:?}"); + } + let fifty_one = "a".repeat(51); + for bad in [ + "", + "grid.v2", + "-x", + "_x", + "+x", + "a:b", + "a b", + "a/b", + fifty_one.as_str(), + ] { + assert!(!cache_name_ok(bad), "{bad:?}"); + } + } + #[tokio::test] async fn a_path_that_is_not_a_store_path_is_not_pushed() { let cache = cache_with("true"); @@ -432,6 +492,9 @@ mod tests { assert!(Remote::from_settings(None, some("grid"), state()).is_err()); assert!(Remote::from_settings(some("https://cache"), some("grid"), state()).is_err()); assert!(Remote::from_settings(some("cache"), some("a\"b"), state()).is_err()); + assert!(Remote::from_settings(some("cache"), some("grid.v2"), state()).is_err()); + assert!(Remote::from_settings(some("cache"), some("-grid"), state()).is_err()); + assert!(Remote::from_settings(some("cache"), some("g_r+i-d"), state()).is_ok()); assert!(Remote::from_settings(some("cache"), some("grid"), None).is_err()); } }