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
This commit is contained in:
parent
065b11a99d
commit
04d5bf5f3e
1 changed files with 179 additions and 16 deletions
|
|
@ -20,9 +20,9 @@ use forgejo_api::structs::{
|
||||||
AddCollaboratorOption, AddCollaboratorOptionPermission, ChangeFileOperation,
|
AddCollaboratorOption, AddCollaboratorOptionPermission, ChangeFileOperation,
|
||||||
ChangeFileOperationOperation, ChangeFilesOptions, CreateBranchProtectionOption,
|
ChangeFileOperationOperation, ChangeFilesOptions, CreateBranchProtectionOption,
|
||||||
CreateHookOption, CreateHookOptionConfig, CreateHookOptionType, CreateRepoOption,
|
CreateHookOption, CreateHookOptionConfig, CreateHookOptionType, CreateRepoOption,
|
||||||
CreateUserOption, IssueListIssuesQuery, IssueListIssuesQueryState, IssueListIssuesQueryType,
|
CreateUserOption, EditUserOption, IssueListIssuesQuery, IssueListIssuesQueryState,
|
||||||
RepoGetContentsQuery, RepoListPullRequestsQuery, RepoListPullRequestsQueryState,
|
IssueListIssuesQueryType, RepoGetContentsQuery, RepoListPullRequestsQuery,
|
||||||
RepoSearchQuery, StateType,
|
RepoListPullRequestsQueryState, RepoSearchQuery, StateType,
|
||||||
};
|
};
|
||||||
use forgejo_api::{ApiErrorKind, Auth, Forgejo, ForgejoError};
|
use forgejo_api::{ApiErrorKind, Auth, Forgejo, ForgejoError};
|
||||||
use futures_util::{StreamExt as _, TryStreamExt as _};
|
use futures_util::{StreamExt as _, TryStreamExt as _};
|
||||||
|
|
@ -361,8 +361,8 @@ impl Client {
|
||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Ensure `agent` exists as a Forgejo user account — the whole job of
|
/// Ensure `agent` exists as a Forgejo user account that cannot create
|
||||||
/// the `CreateForgeUser` node, and the fix for the "user does not
|
/// 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
|
/// exist" failure `AddRepoMember` hit before this node existed: adding
|
||||||
/// a nonexistent user as a collaborator is a Forgejo validation error,
|
/// a nonexistent user as a collaborator is a Forgejo validation error,
|
||||||
/// not an idempotent no-op, so something has to create the account
|
/// 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.
|
/// `forgejo admin` CLI, which assumes co-location with the forge host.
|
||||||
/// This daemon has no such assumption — like every other call in this
|
/// 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
|
/// file, it only ever talks to the forge over HTTP — so this goes through
|
||||||
/// `admin_create_user` instead.
|
/// `admin_create_user` instead. The password is a throwaway: 32 random
|
||||||
///
|
/// bytes, generated once, never persisted or read again (see
|
||||||
/// 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
|
|
||||||
/// [`crate::webhook::generate_hex_secret`], reused here for the same
|
/// [`crate::webhook::generate_hex_secret`], reused here for the same
|
||||||
/// reason a webhook secret and this password are both "32 random bytes
|
/// reason a webhook secret and this password are both "32 random bytes
|
||||||
/// nothing reads back").
|
/// nothing reads back").
|
||||||
|
|
@ -386,12 +382,41 @@ impl Client {
|
||||||
/// Idempotent, same as [`Self::ensure_org_repo`]: 409 folds to success
|
/// Idempotent, same as [`Self::ensure_org_repo`]: 409 folds to success
|
||||||
/// unconditionally, 422 only after a follow-up `user_get` confirms the
|
/// unconditionally, 422 only after a follow-up `user_get` confirms the
|
||||||
/// user (see [`folds_into_success`] — not every 422 means it exists).
|
/// 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
|
/// Then locks the account out of creating repos
|
||||||
/// (`ensure_user_email`, `ensure_repo_creation_disabled`) — this account
|
/// ([`Self::disable_repo_creation`]), whether it was just created or
|
||||||
/// only exists so `AddRepoMember` has something to add, and the owning
|
/// already existed. Nothing else does that for a swarm-created agent, so
|
||||||
/// hive's own provisioning pass self-heals both once the agent spawns.
|
/// 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<()> {
|
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()
|
let password = crate::webhook::generate_hex_secret()
|
||||||
.context("generating a throwaway password for the agent's forge account")?;
|
.context("generating a throwaway password for the agent's forge account")?;
|
||||||
let res = self
|
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"
|
"{\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
|
/// Whether a create-style call failed because the object already
|
||||||
/// exists. Forgejo signals this as HTTP 409 (conflict) or 422
|
/// exists. Forgejo signals this as HTTP 409 (conflict) or 422
|
||||||
/// (validation) — match both defensively, same as
|
/// (validation) — match both defensively, same as
|
||||||
|
|
@ -1328,4 +1384,111 @@ mod tests {
|
||||||
"new logic: correctly refuses to fold a 422 with no confirmed existence"
|
"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<std::sync::Mutex<Vec<(String, String, serde_json::Value)>>>;
|
||||||
|
|
||||||
|
/// 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<serde_json::Value> {
|
||||||
|
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}");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue