From 04d5bf5f3e609c460a795df99d9aadf9b048696d Mon Sep 17 00:00:00 2001 From: atlas Date: Thu, 24 Sep 2026 13:34:04 +0200 Subject: [PATCH 1/3] swarm-controller: disable repo creation on the agent forge users it creates `ensure_agent_user` (the `CreateForgeUser` node) created the agent's forge account but never set `max_repo_creation = 0`, relying on hive-c0re's per-hive `ensure_repo_creation_disabled` pass to lock it down later. That pass is being removed (#3507, #4669), and it is the only guard against an agent token creating, owning and self-merging in its own repo. The node now PATCHes `max_repo_creation = 0` via `admin_edit_user` after the create, on both the created and the already-exists path, and a refused PATCH fails the node with Forgejo's message. The body mirrors hive-c0re's `sparse_edit_user_option` (`login_name` + `source_id = 0`, everything else unset). The PATCH is unconditional: Forgejo's API never returns `max_repo_creation` (it is in `EditUserOption` only, not `User`), so there is no current value to verify against first. Closes #4689 --- swarm-controller/src/forge.rs | 195 +++++++++++++++++++++++++++++++--- 1 file changed, 179 insertions(+), 16 deletions(-) diff --git a/swarm-controller/src/forge.rs b/swarm-controller/src/forge.rs index cdfb0540..8ec3a112 100644 --- a/swarm-controller/src/forge.rs +++ b/swarm-controller/src/forge.rs @@ -20,9 +20,9 @@ use forgejo_api::structs::{ AddCollaboratorOption, AddCollaboratorOptionPermission, ChangeFileOperation, ChangeFileOperationOperation, ChangeFilesOptions, CreateBranchProtectionOption, CreateHookOption, CreateHookOptionConfig, CreateHookOptionType, CreateRepoOption, - CreateUserOption, IssueListIssuesQuery, IssueListIssuesQueryState, IssueListIssuesQueryType, - RepoGetContentsQuery, RepoListPullRequestsQuery, RepoListPullRequestsQueryState, - RepoSearchQuery, StateType, + CreateUserOption, EditUserOption, IssueListIssuesQuery, IssueListIssuesQueryState, + IssueListIssuesQueryType, RepoGetContentsQuery, RepoListPullRequestsQuery, + RepoListPullRequestsQueryState, RepoSearchQuery, StateType, }; use forgejo_api::{ApiErrorKind, Auth, Forgejo, ForgejoError}; use futures_util::{StreamExt as _, TryStreamExt as _}; @@ -361,8 +361,8 @@ impl Client { .await } - /// Ensure `agent` exists as a Forgejo user account — the whole job of - /// the `CreateForgeUser` node, and the fix for the "user does not + /// Ensure `agent` exists as a Forgejo user account that cannot create + /// repos — the whole job of the `CreateForgeUser` node, and the fix for the "user does not /// exist" failure `AddRepoMember` hit before this node existed: adding /// a nonexistent user as a collaborator is a Forgejo validation error, /// not an idempotent no-op, so something has to create the account @@ -373,12 +373,8 @@ impl Client { /// `forgejo admin` CLI, which assumes co-location with the forge host. /// This daemon has no such assumption — like every other call in this /// file, it only ever talks to the forge over HTTP — so this goes through - /// `admin_create_user` instead. - /// - /// The password itself is a throwaway: 32 random bytes, generated once, - /// never persisted anywhere, and never needed again (unlike - /// `hive-c0re`'s CLI path, which can ask forgejo to `--random-password` - /// on its own, the HTTP admin API requires a real value up front — see + /// `admin_create_user` instead. The password is a throwaway: 32 random + /// bytes, generated once, never persisted or read again (see /// [`crate::webhook::generate_hex_secret`], reused here for the same /// reason a webhook secret and this password are both "32 random bytes /// nothing reads back"). @@ -386,12 +382,41 @@ impl Client { /// Idempotent, same as [`Self::ensure_org_repo`]: 409 folds to success /// unconditionally, 422 only after a follow-up `user_get` confirms the /// user (see [`folds_into_success`] — not every 422 means it exists). - /// Deliberately does not align the account's email or disable its own - /// repo-creation rights the way `hive-c0re`'s per-hive provisioning does - /// (`ensure_user_email`, `ensure_repo_creation_disabled`) — this account - /// only exists so `AddRepoMember` has something to add, and the owning - /// hive's own provisioning pass self-heals both once the agent spawns. + /// + /// Then locks the account out of creating repos + /// ([`Self::disable_repo_creation`]), whether it was just created or + /// already existed. Nothing else does that for a swarm-created agent, so + /// a failure fails the node. Does not align the account's email the way + /// `hive-c0re`'s `ensure_user_email` does — cosmetic, not a guard. pub async fn ensure_agent_user(&self, agent: &str) -> Result<()> { + self.create_agent_user(agent).await?; + self.disable_repo_creation(agent).await + } + + /// Set `max_repo_creation = 0` on `agent`'s account. Agents create repos + /// through the hive, never with their own token — a write-scoped token + /// can otherwise create and own a repo and self-merge in it, bypassing + /// the operator-only-merge policy. Existing repos are untouched; push, + /// PRs and clone keep working. + /// + /// Unconditional rather than verify-then-patch: Forgejo's API never + /// returns `max_repo_creation` (it is a field of `EditUserOption` only, + /// not of `User`), so there is no current value to compare against. + /// Setting 0 on an account already at 0 is a no-op. + async fn disable_repo_creation(&self, agent: &str) -> Result<()> { + self.api + .admin_edit_user(agent, repo_creation_lockdown(agent)) + .await + .with_context(|| { + format!("disable repo creation for forge user {agent} (max_repo_creation = 0)") + })?; + tracing::info!(%agent, "swarm forge: disabled repo creation (max_repo_creation = 0)"); + Ok(()) + } + + /// Create `agent`'s account. Idempotent: an existing user (409/422) is + /// folded into success, same as [`Self::ensure_org_repo`]. + async fn create_agent_user(&self, agent: &str) -> Result<()> { let password = crate::webhook::generate_hex_secret() .context("generating a throwaway password for the agent's forge account")?; let res = self @@ -1084,6 +1109,37 @@ fn initial_flake_nix() -> &'static str { "{\n description = \"hyperhive agent\";\n inputs = { };\n outputs =\n { self, ... }@inputs:\n {\n nixosModules.default = {\n imports = [ ./agent.nix ];\n _module.args.flakeInputs = builtins.removeAttrs inputs [ \"self\" ];\n };\n };\n}\n" } +/// The `admin_edit_user` body that sets `max_repo_creation = 0` on `agent` +/// and nothing else. Same shape as `hive-c0re::forge::users`' +/// `sparse_edit_user_option`: `login_name` + `source_id = 0` (local auth, +/// which is what [`Client::create_agent_user`] creates) ride along because +/// Forgejo resets `use_custom_avatar` on an admin edit that omits +/// `login_name`; every other field is left unset so nothing else is touched. +fn repo_creation_lockdown(agent: &str) -> EditUserOption { + EditUserOption { + active: None, + admin: None, + allow_create_organization: None, + allow_git_hook: None, + allow_import_local: None, + description: None, + email: None, + full_name: None, + hide_email: None, + location: None, + login_name: Some(agent.to_owned()), + max_repo_creation: Some(0), + must_change_password: None, + password: None, + prohibit_login: None, + pronouns: None, + restricted: None, + source_id: Some(0), + visibility: None, + website: None, + } +} + /// Whether a create-style call failed because the object already /// exists. Forgejo signals this as HTTP 409 (conflict) or 422 /// (validation) — match both defensively, same as @@ -1328,4 +1384,111 @@ mod tests { "new logic: correctly refuses to fold a 422 with no confirmed existence" ); } + + /// What [`stub_forge`] saw: method, path and JSON body of each request. + type Seen = std::sync::Arc>>; + + /// A stand-in forge on a loopback port — never the real one — answering + /// the admin user create (`POST`) with `create` and the admin user edit + /// (`PATCH`) with `edit` (status, JSON body), and recording every request. + async fn stub_forge(create: (u16, &'static str), edit: (u16, &'static str)) -> (Client, Seen) { + use axum::http::{Method, StatusCode, Uri, header::CONTENT_TYPE}; + + let seen = Seen::default(); + let recorded = seen.clone(); + let app = axum::Router::new().fallback( + move |method: Method, uri: Uri, body: axum::body::Bytes| { + let recorded = recorded.clone(); + async move { + let json = serde_json::from_slice(&body).unwrap_or_default(); + recorded.lock().unwrap().push(( + method.to_string(), + uri.path().to_owned(), + json, + )); + let (status, body) = match method { + Method::POST => create, + Method::PATCH => edit, + _ => (404, r#"{"message":"not stubbed"}"#), + }; + let status = StatusCode::from_u16(status).unwrap(); + (status, [(CONTENT_TYPE, "application/json")], body) + } + }, + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = url::Url::parse(&format!("http://{}/", listener.local_addr().unwrap())).unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await }); + let api = Forgejo::new(Auth::Token("stub-token"), url).unwrap(); + (Client { api }, seen) + } + + /// The `PATCH` [`Client::ensure_agent_user`] sent, if any. + fn lockdown_patch(seen: &Seen, agent: &str) -> Option { + let path = format!("/api/v1/admin/users/{agent}"); + seen.lock() + .unwrap() + .iter() + .find(|(method, p, _)| method == "PATCH" && *p == path) + .map(|(_, _, body)| body.clone()) + } + + /// A freshly created agent account is locked out of repo creation by + /// this node itself — not left for a hive-level pass to catch later — + /// and the edit sets nothing beyond `max_repo_creation` plus the + /// `login_name` / `source_id` pair Forgejo needs to leave the rest alone. + #[tokio::test] + async fn a_created_agent_user_gets_repo_creation_disabled() { + let (client, seen) = stub_forge((201, "{}"), (200, "{}")).await; + + client.ensure_agent_user("alice").await.unwrap(); + + let body = lockdown_patch(&seen, "alice").expect("no lockdown PATCH sent"); + assert_eq!(body["max_repo_creation"], 0); + let set: Vec<&str> = body + .as_object() + .unwrap() + .iter() + .filter(|(_, v)| !v.is_null()) + .map(|(k, _)| k.as_str()) + .collect(); + assert_eq!(set, ["login_name", "max_repo_creation", "source_id"]); + assert_eq!(body["login_name"], "alice"); + assert_eq!(body["source_id"], 0); + } + + /// An account that already existed is locked down too: the node is + /// re-run, and an account created before this lockdown existed must + /// not keep repo creation just because the create step folded to + /// success. 409 rather than 422 so the case doesn't hinge on how a 422 + /// is disambiguated. + #[tokio::test] + async fn an_existing_agent_user_still_gets_repo_creation_disabled() { + let (client, seen) = + stub_forge((409, r#"{"message":"user already exists"}"#), (200, "{}")).await; + + client.ensure_agent_user("alice").await.unwrap(); + + let body = lockdown_patch(&seen, "alice").expect("no lockdown PATCH sent"); + assert_eq!(body["max_repo_creation"], 0); + } + + /// A refused lockdown fails the node with Forgejo's own message, rather + /// than warning and reporting the account as provisioned — that is + /// `hive-c0re`'s `ensure_repo_creation_disabled` shape, which leaves an + /// agent able to create and self-merge with nothing marked failed. + #[tokio::test] + async fn a_refused_lockdown_fails_the_node() { + let (client, _seen) = stub_forge( + (201, "{}"), + (403, r#"{"message":"token does not have required scope"}"#), + ) + .await; + + let err = client.ensure_agent_user("alice").await.unwrap_err(); + + let msg = format!("{err:#}"); + assert!(msg.contains("max_repo_creation"), "{msg}"); + assert!(msg.contains("token does not have required scope"), "{msg}"); + } } From 58f50ed50694f9a56f81b62d9bfdf349ac279613 Mon Sep 17 00:00:00 2001 From: atlas Date: Thu, 24 Sep 2026 15:11:23 +0200 Subject: [PATCH 2/3] swarm-controller: guard the lockdown PATCH against a forge name collision disable_repo_creation now reads the account once before PATCHing max_repo_creation/source_id, and fails the node if the email isn't the {agent}@hyperhive.local marker create_agent_user itself sets. The 409/422 create-fold (#4681) only proves some account with that name exists, not that this node created it, so a pre-existing non-agent account sharing an agent's chosen name could otherwise get locked onto local auth with repo creation disabled. Leaves the fold untouched (Option B, per atlas/argus on #4693); the read moves into disable_repo_creation instead. Also fixes the nix-sandboxed cargo-test check: forgejo_api::Forgejo::new builds a reqwest client that eagerly resolves TLS roots via rustls-native-certs even for the tests' plain-http loopback stub server, which panics with "No CA certificates were loaded from the system" in the CA-less build sandbox. Gives that check's nativeBuildInputs pkgs.cacert and sets SSL_CERT_FILE, same pattern this repo's runtime deployment already uses for the same reqwest/rustls resolution. --- nix/checks.nix | 19 ++++- swarm-controller/src/forge.rs | 137 +++++++++++++++++++++++++++++++--- 2 files changed, 144 insertions(+), 12 deletions(-) diff --git a/nix/checks.nix b/nix/checks.nix index 706fba9e..c5238637 100644 --- a/nix/checks.nix +++ b/nix/checks.nix @@ -156,7 +156,24 @@ in # per-container toplevels stay fully cached. cargo-test = craneLib.cargoTest { src = cleanSrc; - inherit cargoArtifacts nativeBuildInputs; + inherit cargoArtifacts; + # `swarm-controller`'s forge tests build a real `forgejo_api::Forgejo` + # (hence a real `reqwest::Client`) against a loopback stub server, never + # the real forge — but `reqwest`/`rustls` still resolves roots through + # `rustls-native-certs` at *client-build* time, unconditionally, even + # though every request that client ever sends is plain `http://` to + # `127.0.0.1`. The nix build sandbox has no system CA store, so that + # resolution finds zero certs and `ClientBuilder::build()` itself fails + # with "No CA certificates were loaded from the system" — the tests + # never get as far as making a request. Same root cause the runtime + # deployment already works around (see `nix/host-modules/swarm- + # controller.nix`'s `caTrust` comment) and the same fix: give + # `rustls-native-certs` something to find via `SSL_CERT_FILE`. The + # bundle's actual contents don't matter here — nothing in these tests + # ever presents or verifies a real certificate — only that the store is + # non-empty. + nativeBuildInputs = nativeBuildInputs ++ [ pkgs.cacert ]; + SSL_CERT_FILE = "${pkgs.cacert}/etc/ssl/certs/ca-bundle.crt"; pname = "hyperhive-workspace"; version = "0.1.0"; cargoTestExtraArgs = "--workspace"; diff --git a/swarm-controller/src/forge.rs b/swarm-controller/src/forge.rs index 8ec3a112..f1b4c7a3 100644 --- a/swarm-controller/src/forge.rs +++ b/swarm-controller/src/forge.rs @@ -399,11 +399,35 @@ impl Client { /// the operator-only-merge policy. Existing repos are untouched; push, /// PRs and clone keep working. /// - /// Unconditional rather than verify-then-patch: Forgejo's API never - /// returns `max_repo_creation` (it is a field of `EditUserOption` only, - /// not of `User`), so there is no current value to compare against. - /// Setting 0 on an account already at 0 is a no-op. + /// The PATCH itself is unconditional rather than verify-then-patch: + /// Forgejo's API never returns `max_repo_creation` (it is a field of + /// `EditUserOption` only, not of `User`), so there is no current value + /// to compare against, and setting 0 on an account already at 0 is a + /// no-op. But `create_agent_user`'s idempotent fold only proves *some* + /// account named `agent` exists — a 409 folds with no read at all, and + /// a 422's follow-up `user_get` only confirms the username, not that + /// swarm-controller created it — so before that PATCH this reads the + /// account once and checks the one thing that *is* comparable: the + /// email `create_agent_user` sets at creation (see [`agent_email`]), + /// the same marker `hive-c0re`'s legacy `ensure_user_exists` / + /// `ensure_user_email` set and converge to. A mismatch means the name + /// collided with an account this node didn't create, so it fails the + /// node rather than sending the PATCH (or silently skipping) to an + /// account nobody verified is agent-controlled. async fn disable_repo_creation(&self, agent: &str) -> Result<()> { + let user = self + .api + .user_get(agent) + .await + .with_context(|| format!("read forge user {agent} before locking it down"))?; + let expected = agent_email(agent); + if user.email.as_deref() != Some(expected.as_str()) { + anyhow::bail!( + "forge account `{agent}` exists but is not this agent's (email {:?}, expected \ + `{expected}`); refusing to lock it down", + user.email + ); + } self.api .admin_edit_user(agent, repo_creation_lockdown(agent)) .await @@ -423,7 +447,7 @@ impl Client { .api .admin_create_user(CreateUserOption { created_at: None, - email: format!("{agent}@hyperhive.local"), + email: agent_email(agent), full_name: None, login_name: None, must_change_password: Some(false), @@ -1109,6 +1133,19 @@ fn initial_flake_nix() -> &'static str { "{\n description = \"hyperhive agent\";\n inputs = { };\n outputs =\n { self, ... }@inputs:\n {\n nixosModules.default = {\n imports = [ ./agent.nix ];\n _module.args.flakeInputs = builtins.removeAttrs inputs [ \"self\" ];\n };\n };\n}\n" } +/// Canonical email for an agent's Forgejo account, set by +/// [`Client::create_agent_user`] at creation and checked by +/// [`Client::disable_repo_creation`] before it sends the account's PATCH. Same +/// format string as `hive-c0re::forge::users::agent_email` — not called +/// across the crate boundary (see this module's doc comment), but both +/// creators (and `hive-c0re`'s `ensure_user_email`, which aligns older +/// accounts to it) converge on this exact value, so it doubles as the +/// marker that distinguishes an agent's own account from an unrelated one +/// that happens to share its username. +fn agent_email(agent: &str) -> String { + format!("{agent}@hyperhive.local") +} + /// The `admin_edit_user` body that sets `max_repo_creation = 0` on `agent` /// and nothing else. Same shape as `hive-c0re::forge::users`' /// `sparse_edit_user_option`: `login_name` + `source_id = 0` (local auth, @@ -1389,9 +1426,14 @@ mod tests { type Seen = std::sync::Arc>>; /// A stand-in forge on a loopback port — never the real one — answering - /// the admin user create (`POST`) with `create` and the admin user edit - /// (`PATCH`) with `edit` (status, JSON body), and recording every request. - async fn stub_forge(create: (u16, &'static str), edit: (u16, &'static str)) -> (Client, Seen) { + /// the admin user create (`POST`) with `create`, the pre-lockdown + /// `user_get` (`GET`) with `get`, and the admin user edit (`PATCH`) with + /// `edit` (status, JSON body each), and recording every request. + async fn stub_forge( + create: (u16, &'static str), + get: (u16, &'static str), + edit: (u16, &'static str), + ) -> (Client, Seen) { use axum::http::{Method, StatusCode, Uri, header::CONTENT_TYPE}; let seen = Seen::default(); @@ -1408,6 +1450,7 @@ mod tests { )); let (status, body) = match method { Method::POST => create, + Method::GET => get, Method::PATCH => edit, _ => (404, r#"{"message":"not stubbed"}"#), }; @@ -1433,13 +1476,26 @@ mod tests { .map(|(_, _, body)| body.clone()) } + /// The email [`stub_forge`]'s `user_get` stub should answer with for an + /// account this agent actually owns — same value + /// [`Client::create_agent_user`] itself sets. + fn own_account_get(agent: &str) -> (u16, &'static str) { + // Leaked rather than borrowed: `stub_forge`'s `get` param needs + // `&'static str` (it moves into a `'static` axum handler), and this + // helper builds the body per-agent instead of hardcoding "alice". + ( + 200, + Box::leak(format!(r#"{{"email":"{agent}@hyperhive.local"}}"#).into_boxed_str()), + ) + } + /// A freshly created agent account is locked out of repo creation by /// this node itself — not left for a hive-level pass to catch later — /// and the edit sets nothing beyond `max_repo_creation` plus the /// `login_name` / `source_id` pair Forgejo needs to leave the rest alone. #[tokio::test] async fn a_created_agent_user_gets_repo_creation_disabled() { - let (client, seen) = stub_forge((201, "{}"), (200, "{}")).await; + let (client, seen) = stub_forge((201, "{}"), own_account_get("alice"), (200, "{}")).await; client.ensure_agent_user("alice").await.unwrap(); @@ -1464,8 +1520,12 @@ mod tests { /// is disambiguated. #[tokio::test] async fn an_existing_agent_user_still_gets_repo_creation_disabled() { - let (client, seen) = - stub_forge((409, r#"{"message":"user already exists"}"#), (200, "{}")).await; + let (client, seen) = stub_forge( + (409, r#"{"message":"user already exists"}"#), + own_account_get("alice"), + (200, "{}"), + ) + .await; client.ensure_agent_user("alice").await.unwrap(); @@ -1481,6 +1541,7 @@ mod tests { async fn a_refused_lockdown_fails_the_node() { let (client, _seen) = stub_forge( (201, "{}"), + own_account_get("alice"), (403, r#"{"message":"token does not have required scope"}"#), ) .await; @@ -1491,4 +1552,58 @@ mod tests { assert!(msg.contains("max_repo_creation"), "{msg}"); assert!(msg.contains("token does not have required scope"), "{msg}"); } + + /// The guard this PR adds: a name collision with a pre-existing + /// non-agent account (or any account whose email isn't the + /// `{agent}@hyperhive.local` marker) must fail the node rather than + /// PATCH `max_repo_creation`/`source_id` onto an account nobody + /// verified this node created. 409 create-fold, same as + /// `an_existing_agent_user_still_gets_repo_creation_disabled` — the + /// fold itself is untouched by this PR; the fix lives in the read right + /// before the PATCH. + #[tokio::test] + async fn a_name_collision_with_a_non_agent_account_fails_without_patching() { + let (client, seen) = stub_forge( + (409, r#"{"message":"user already exists"}"#), + (200, r#"{"email":"someone@example.com"}"#), + (200, "{}"), + ) + .await; + + let err = client.ensure_agent_user("alice").await.unwrap_err(); + + let msg = format!("{err:#}"); + assert!(msg.contains("alice"), "{msg}"); + assert!( + msg.contains("not this agent's"), + "must name the collision, got: {msg}" + ); + assert!( + lockdown_patch(&seen, "alice").is_none(), + "a collision must never reach the PATCH" + ); + } + + /// A legacy account `hive-c0re::forge::users::ensure_user_exists` (or + /// its `ensure_user_email` alignment pass) created carries the same + /// `{agent}@hyperhive.local` marker, plus the `login_name`/`source_id` + /// fields its own `sparse_edit_user_option` sets — this node must still + /// lock such an account down, not just ones it created itself. + #[tokio::test] + async fn a_legacy_hive_created_agent_account_still_gets_locked_down() { + let (client, seen) = stub_forge( + (409, r#"{"message":"user already exists"}"#), + ( + 200, + r#"{"id":42,"login":"alice","login_name":"alice","email":"alice@hyperhive.local","source_id":0}"#, + ), + (200, "{}"), + ) + .await; + + client.ensure_agent_user("alice").await.unwrap(); + + let body = lockdown_patch(&seen, "alice").expect("no lockdown PATCH sent"); + assert_eq!(body["max_repo_creation"], 0); + } } From 2fda529ca8159ab539b8233f947e0e76196bf5fa Mon Sep 17 00:00:00 2001 From: atlas Date: Thu, 24 Sep 2026 15:40:18 +0200 Subject: [PATCH 3/3] swarm-controller: accept the legacy pre-alignment email in the lockdown guard disable_repo_creation's ownership guard now matches either the aligned {agent}@hyperhive.local email or the legacy {agent}@hive.local one hive-c0re::forge::users::ensure_user_exists used before its own ensure_user_email alignment pass existed. That pass only runs on the forge-host hive, and only once it has a core token and has ticked, so a real pre-existing agent can still carry the old email when this node reads it. Without this, the guard would bail "not this agent's" on a genuine agent during that rollout window (argus, round 2). Verified separately (not a code change): swarm-controller's forge account is a site admin (created with --admin), and Forgejo's convert.toUser/ToUser only hides an account's email when the caller isn't the admin and isn't the account itself (services/convert/user.go), so user_get already returns the real email regardless of hide_email on the target account. No endpoint change needed for that half of the review. --- swarm-controller/src/forge.rs | 57 ++++++++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/swarm-controller/src/forge.rs b/swarm-controller/src/forge.rs index f1b4c7a3..bb4af221 100644 --- a/swarm-controller/src/forge.rs +++ b/swarm-controller/src/forge.rs @@ -408,11 +408,17 @@ impl Client { /// a 422's follow-up `user_get` only confirms the username, not that /// swarm-controller created it — so before that PATCH this reads the /// account once and checks the one thing that *is* comparable: the - /// email `create_agent_user` sets at creation (see [`agent_email`]), - /// the same marker `hive-c0re`'s legacy `ensure_user_exists` / - /// `ensure_user_email` set and converge to. A mismatch means the name - /// collided with an account this node didn't create, so it fails the - /// node rather than sending the PATCH (or silently skipping) to an + /// email. A match against either [`agent_email`] (what + /// `create_agent_user` itself sets) or [`legacy_agent_email`] (what + /// `hive-c0re::forge::users::ensure_user_exists` used before its own + /// `ensure_user_email` alignment pass runs) counts as this agent's + /// account — the legacy value has to be accepted too, not just the + /// aligned one, because `ensure_user_email` only runs on the forge-host + /// hive and only once it has a core token and has ticked at least once, + /// so a real, existing legacy agent can still carry the old email when + /// this node reads it mid-rollout. A mismatch against both means the + /// name collided with an account this node didn't create, so it fails + /// the node rather than sending the PATCH (or silently skipping) to an /// account nobody verified is agent-controlled. async fn disable_repo_creation(&self, agent: &str) -> Result<()> { let user = self @@ -421,10 +427,11 @@ impl Client { .await .with_context(|| format!("read forge user {agent} before locking it down"))?; let expected = agent_email(agent); - if user.email.as_deref() != Some(expected.as_str()) { + let legacy = legacy_agent_email(agent); + if !matches!(user.email.as_deref(), Some(e) if e == expected || e == legacy) { anyhow::bail!( "forge account `{agent}` exists but is not this agent's (email {:?}, expected \ - `{expected}`); refusing to lock it down", + `{expected}` or the legacy `{legacy}`); refusing to lock it down", user.email ); } @@ -1146,6 +1153,19 @@ fn agent_email(agent: &str) -> String { format!("{agent}@hyperhive.local") } +/// The pre-alignment email `hive-c0re::forge::users::ensure_user_exists` +/// gave an agent account before that crate's own `ensure_user_email` +/// existed — see that function's doc comment ("Existing agents were +/// created with `{name}@hive.local`; this corrects that"). Accepted by +/// [`Client::disable_repo_creation`]'s guard alongside [`agent_email`] +/// because `ensure_user_email`'s alignment PATCH only runs on the +/// forge-host hive, and only after it has a core token and has ticked at +/// least once, so a real legacy agent can still carry this value when this +/// node reads it. +fn legacy_agent_email(agent: &str) -> String { + format!("{agent}@hive.local") +} + /// The `admin_edit_user` body that sets `max_repo_creation = 0` on `agent` /// and nothing else. Same shape as `hive-c0re::forge::users`' /// `sparse_edit_user_option`: `login_name` + `source_id = 0` (local auth, @@ -1606,4 +1626,27 @@ mod tests { let body = lockdown_patch(&seen, "alice").expect("no lockdown PATCH sent"); assert_eq!(body["max_repo_creation"], 0); } + + /// argus's round-2 🟡: `hive-c0re`'s `ensure_user_email` alignment pass + /// (the thing that rewrites a legacy `{name}@hive.local` account to + /// `{name}@hyperhive.local`) only runs on the forge-host hive, and only + /// once it has a core token and has ticked — so a real, pre-existing + /// agent can still be sitting on the pre-alignment `@hive.local` email + /// when this node reads it. The guard must accept that value too, not + /// just the aligned one, or a real agent would fail with "not this + /// agent's" during the rollout window. + #[tokio::test] + async fn a_pre_alignment_legacy_email_still_gets_locked_down() { + let (client, seen) = stub_forge( + (409, r#"{"message":"user already exists"}"#), + (200, r#"{"email":"alice@hive.local"}"#), + (200, "{}"), + ) + .await; + + client.ensure_agent_user("alice").await.unwrap(); + + let body = lockdown_patch(&seen, "alice").expect("no lockdown PATCH sent"); + assert_eq!(body["max_repo_creation"], 0); + } }