vinzenz/hyperhive
vinzenz/hyperhive
Watch
0
0
Fork
You've already forked hyperhive
 0
Code Activity

config repos: drop the hive's branch-protection edit and core from the merge gate

Browse source 
Folds in #4853: hive-c0re no longer writes branch protection on
`agent-configs` repos (apply_config_repo_branch_protection,
config_repo_protection_edit, record_branch_protection_result and the
now-unused main_branch_protection_option go). swarm-controller's
CreateRepo rule and its forge-objects convergence own `main`'s gate.

Nothing merges into config `main` as `core` any more, so the converged
rule's merge user list is empty. `push_config` still pushes `main` as
core, through push rights, not the merge whitelist.

Refs #4850
Refs #4853
This commit is contained in:
atlas 2026-10-02 22:32:17 +02:00
commit 0cee0382e9
3 changed files with 36 additions and 235 deletions
Download patch file Download diff file Expand all files Collapse all files

8
hive-c0re/src/forge/reconcile.rs
View file

@ -8,8 +8,8 @@
//! the divergence). `Apply(Forge)` resets the local applied checkout to //! the divergence). `Apply(Forge)` resets the local applied checkout to
//! forge `main` — the change takes effect on the next deploy (the deploy's //! forge `main` — the change takes effect on the next deploy (the deploy's
//! `--override-input` re-locks against the reset local tree); it does NOT //! `--override-input` re-locks against the reset local tree); it does NOT
//! auto-deploy. `Apply(Local)` is not supported: forge `main` is core-only //! auto-deploy. `Apply(Local)` is not supported: forge `main` is
//! branch-protected (advanced solely by the config-PR ff-merge API). //! branch-protected, so it only advances through a config-PR merge.
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
@ -141,8 +141,8 @@ pub async fn reconcile_config_apply(
) -> Result<HostResponse> { ) -> Result<HostResponse> {
match direction { match direction {
ReconcileDirection::Local => Ok(HostResponse::error( ReconcileDirection::Local => Ok(HostResponse::error(
"reconciling forge from local is not supported yet: forge main is core-only \ "reconciling forge from local is not supported yet: forge main is \
branch-protected (no-push, ff-merge-API-only). Resolve via a config PR, or use \ branch-protected. Resolve via a config PR, or use \
`--from forge` to reset the local checkout to forge main.", `--from forge` to reset the local checkout to forge main.",
)), )),
ReconcileDirection::Forge => { ReconcileDirection::Forge => {

231
hive-c0re/src/forge/repos.rs
View file

@ -9,8 +9,7 @@ use std::sync::Mutex;
use anyhow::{Context, Result}; use anyhow::{Context, Result};
use forgejo_api::structs::{ use forgejo_api::structs::{
AddCollaboratorOption, AddCollaboratorOptionPermission, CreateBranchProtectionOption, AddCollaboratorOption, AddCollaboratorOptionPermission, CreateRepoOption, Repository,
CreateRepoOption, EditBranchProtectionOption, Repository,
}; };
use forgejo_api::{ApiErrorKind, ForgejoError}; use forgejo_api::{ApiErrorKind, ForgejoError};
use reqwest::StatusCode; use reqwest::StatusCode;
@ -157,12 +156,11 @@ pub async fn push_meta(dir: &Path) -> Result<()> {
} }
/// Ensure the `agent-configs/<name>` repo exists so the first /// Ensure the `agent-configs/<name>` repo exists so the first
/// `push_config` doesn't 404, and wire it as the agent-editable PR surface: /// `push_config` doesn't 404, and add the agent as a **write** collaborator
/// the agent is a **write** collaborator (can push feature branches + /// (it can push feature branches + open config PRs). Branch protection on
/// open config PRs) and `main` is branch-protected core-only (only hive-c0re's /// `main` is swarm-controller's, not set here. No-op when the forge isn't
/// merge handler lands on it; operator approval required). No-op when the forge /// running or the core token isn't minted yet. Safe to call on every spawn
/// isn't running or the core token isn't minted yet. Safe to call on every /// and on every startup (all steps idempotent).
/// spawn and on every startup (all steps idempotent).
pub async fn ensure_config_repo(name: &str) -> Result<()> { pub async fn ensure_config_repo(name: &str) -> Result<()> {
if !is_present().await { if !is_present().await {
return Ok(()); return Ok(());
@ -171,8 +169,6 @@ pub async fn ensure_config_repo(name: &str) -> Result<()> {
return Ok(()); return Ok(());
}; };
ensure_org_repo(CONFIG_ORG, name, &token).await?; ensure_org_repo(CONFIG_ORG, name, &token).await?;
// Agent = write collaborator: it can push config-PR branches + open PRs,
// but the branch protection below keeps it off `main` directly.
add_collaborator( add_collaborator(
CONFIG_ORG, CONFIG_ORG,
name, name,
@ -180,61 +176,7 @@ pub async fn ensure_config_repo(name: &str) -> Result<()> {
AddCollaboratorOptionPermission::Write, AddCollaboratorOptionPermission::Write,
&token, &token,
) )
.await?; .await
// Protect `main` core-only, fast-forward-only (no auto force-push). A
// failure here is security-relevant (an unprotected config repo lets an
// operator-merged config PR bypass the deploy pipeline), so it's tracked
// on the dashboard banner in addition to the journal warning callers
// already log — see `record_branch_protection_result`.
let result = apply_config_repo_branch_protection(name, &token).await;
record_branch_protection_result(name, result.is_ok());
result
}
/// Dashboard-banner tracker for [`apply_config_repo_branch_protection`]
/// failures. The registry ([`crate::warnings::set_warning`]) only takes
/// `&'static str` kinds, so a dynamic per-agent key isn't possible — instead
/// this keeps one static `crit` warning (`"branch_protection_missing"`) whose
/// message lists every agent currently failing to protect, and clears it
/// once the set is empty. Called on every `ensure_config_repo` pass (startup
/// sweep + per-rebuild), so a fixed agent drops out of the message on its
/// next successful sweep without requiring a restart.
fn record_branch_protection_result(name: &str, ok: bool) {
use std::collections::BTreeSet;
use std::sync::{OnceLock, PoisonError};
use crate::warnings::{WarningGuard, set_warning};
static FAILING: OnceLock<Mutex<BTreeSet<String>>> = OnceLock::new();
static GUARD: OnceLock<Mutex<Option<WarningGuard>>> = OnceLock::new();
let mut failing = FAILING
.get_or_init(|| Mutex::new(BTreeSet::new()))
.lock()
.unwrap_or_else(PoisonError::into_inner);
if ok {
failing.remove(name);
} else {
failing.insert(name.to_owned());
}
let mut guard = GUARD
.get_or_init(|| Mutex::new(None))
.lock()
.unwrap_or_else(PoisonError::into_inner);
if failing.is_empty() {
*guard = None;
return;
}
let names = failing.iter().cloned().collect::<Vec<_>>().join(", ");
let message = format!(
"config-repo branch protection not applied for: {names} — \
operator-merged config PRs for these agents could bypass the deploy pipeline"
);
match guard.as_ref() {
Some(g) => g.update("crit", message),
None => *guard = Some(set_warning("branch_protection_missing", "crit", message)),
}
} }
/// Grant agent `name` read-only collaborator access to `internal/docs`. /// Grant agent `name` read-only collaborator access to `internal/docs`.
@ -318,10 +260,9 @@ pub async fn ensure_meta_remote(name: &str) -> Result<()> {
/// ///
/// **Two separate pushes, not one.** The status tags are id-suffixed /// **Two separate pushes, not one.** The status tags are id-suffixed
/// (`deployed/<id>`, …) and add-only, so they must always land. `main`, /// (`deployed/<id>`, …) and add-only, so they must always land. `main`,
/// by contrast, is core-only branch-protected and authoritatively /// by contrast, is branch-protected and advances through an operator's
/// advanced by `pr_merge`'s ff-only merge API — so a mirror push of an /// merge on the forge — so a mirror push of an already-established `main`
/// already-established `main` is routinely rejected (protected-branch, /// is routinely rejected (protected-branch, or non-fast-forward). Git's pre-receive
/// or non-fast-forward after a rolled-back deploy). Git's pre-receive
/// hook is all-or-nothing: bundling both refspecs in one push means that /// hook is all-or-nothing: bundling both refspecs in one push means that
/// `main` reject declines the whole push, dropping the tags too. /// `main` reject declines the whole push, dropping the tags too.
/// So we push the tags on their own first, then attempt `main` /// So we push the tags on their own first, then attempt `main`
@ -354,8 +295,8 @@ pub async fn push_config(name: &str) -> Result<()> {
); );
} }
// Then main on its own — a protected-branch / non-ff reject of an // Then main on its own — a protected-branch / non-ff reject of an
// established main is expected (pr_merge owns main); only the initial // established main is expected (an operator's forge merge advances it);
// empty-repo seed actually advances it here. // only the initial empty-repo seed actually advances it here.
let out = run_config_push(&dir, &url, &auth, "refs/heads/main:refs/heads/main").await?; let out = run_config_push(&dir, &url, &auth, "refs/heads/main:refs/heads/main").await?;
if !out.status.success() { if !out.status.success() {
let stderr = String::from_utf8_lossy(&out.stderr); let stderr = String::from_utf8_lossy(&out.stderr);
@ -365,8 +306,8 @@ pub async fn push_config(name: &str) -> Result<()> {
{ {
tracing::info!( tracing::info!(
%name, %name,
"forge: mirror push of main skipped — forge main is protected / \ "forge: mirror push of main skipped — forge main is protected \
owned by the config-PR ff-merge (expected); tags mirrored" and advances by an operator merge (expected); tags mirrored"
); );
return Ok(()); return Ok(());
} }
@ -624,147 +565,3 @@ async fn add_collaborator(
tracing::debug!(%owner, %repo, %user, ?permission, "forge: collaborator set"); tracing::debug!(%owner, %repo, %user, ?permission, "forge: collaborator set");
Ok(()) Ok(())
} }
/// `CreateBranchProtectionOption` protecting `main` with every other
/// field unset — Forgejo treats `None` fields as their defaults, same
/// as the sparse JSON bodies the raw-HTTP predecessor sent. Callers
/// set the whitelist/approval fields they need on top.
fn main_branch_protection_option() -> CreateBranchProtectionOption {
CreateBranchProtectionOption {
apply_to_admins: None,
approvals_whitelist_teams: None,
approvals_whitelist_username: None,
block_on_official_review_requests: None,
block_on_outdated_branch: None,
block_on_rejected_reviews: None,
branch_name: Some("main".to_owned()),
dismiss_stale_approvals: None,
enable_approvals_whitelist: None,
enable_merge_whitelist: None,
enable_push: None,
enable_push_whitelist: None,
enable_status_check: None,
ignore_stale_approvals: None,
merge_whitelist_teams: None,
merge_whitelist_usernames: None,
protected_file_patterns: None,
push_whitelist_deploy_keys: None,
push_whitelist_teams: None,
push_whitelist_usernames: None,
require_signed_commits: None,
required_approvals: None,
rule_name: None,
status_check_contexts: None,
unprotected_file_patterns: None,
}
}
/// Apply branch protection to an `agent-configs/<name>` repo's `main` so it
/// can serve as the agent-editable, PR-merge config surface:
/// - **`main` is never directly pushable** — no push is enabled on the
/// protected branch, so neither the agent (a write collaborator) nor
/// hive-c0re can `git push` it. It only advances via the config-PR merge
/// node (`actions::run_deploy_apply`), which fast-forward-*merges* the reviewed
/// head through the forge merge API (`Do=fast-forward-only`,
/// `head_commit_id` pinned to the reviewed sha).
/// - **merge is whitelisted to `core`** — the agent can push feature branches +
/// open PRs but can't land them. swarm-controller adds the `operators` team
/// to the whitelist, so an operator can also merge in the forge UI.
/// - **the operator's dashboard approval is the gate for `core`** — approval
/// happens on the `MergeConfigPr` card and hive-c0re only merges an approved
/// PR. There's deliberately no Forgejo `required_approvals` review
/// requirement: that flow never does an in-forge review, so requiring one
/// would only dead-block the `core` merge.
/// - **fast-forward-only** — `main` only ever advances by fast-forward; a raced
/// non-ff `main` is refused by the merge API rather than force-moved.
///
/// Idempotent: an existing rule for the branch (200/409/422) is success.
async fn apply_config_repo_branch_protection(repo: &str, token: &str) -> Result<()> {
let client = api(token)?;
let mut rule = main_branch_protection_option();
// Only `core` may merge (through the config-PR merge API); no push is
// enabled at all, so `main` can only advance via that merge. Push +
// approval defaults are off in `main_branch_protection_option`, so a fresh
// rule needs nothing but the merge whitelist. (`config_repo_protection_edit`
// must clear the old push/approval fields explicitly, since a PATCH leaves
// unset fields untouched.)
rule.enable_merge_whitelist = Some(true);
rule.merge_whitelist_usernames = Some(vec!["core".to_owned()]);
let Err(create_err) = client
.repo_create_branch_protection(CONFIG_ORG, repo, rule)
.await
else {
tracing::info!(%repo, "forge: applied config-repo branch protection");
return Ok(());
};
// Create failed. This is ambiguous — "rule already exists" (the common
// idempotent case) OR a silent rejection that created no rule. Either way a
// create never *updates* an existing rule, and repos protected under an
// older shape (the push-based / approval-gated rules) carry stale settings.
// So converge the existing rule with a PATCH that explicitly clears them:
// it fixes those stale repos on the next boot's `ensure_config_repo` pass,
// is a harmless no-op when the rule is already correct, and still fails
// loudly when no rule can be established (you can't edit a rule that isn't
// there) — so it can't leave a repo silently unprotected.
match client
.repo_edit_branch_protection(CONFIG_ORG, repo, "main", config_repo_protection_edit())
.await
{
Ok(_) => {
// Debug, not info: this PATCH runs on every `ensure_config_repo`
// boot pass for every existing repo (create → 409 → converge), so
// it's almost always a no-op re-assertion — info-logging it would
// be N lines of noise per boot on a many-agent hive.
tracing::debug!(
%repo, create_error = %create_err,
"forge: converged existing config-repo branch protection"
);
Ok(())
}
Err(edit_err) => anyhow::bail!(
"branch protection for {CONFIG_ORG}/{repo} not applied: create failed \
({create_err}); edit of existing `main` rule failed ({edit_err})"
),
}
}
/// The `EditBranchProtectionOption` that converges an existing
/// `agent-configs/<name>` `main` rule to the current desired shape: merge
/// whitelisted to `core`, **no direct push at all**, and **no in-forge approval
/// requirement** (the dashboard approval + `core`-only merge whitelist are the
/// gate). The push/approval fields are set to their explicit off-values, not
/// left `None`, so a repo carrying an older push-based or approval-gated rule is
/// actually *converged* rather than merely re-asserted — a PATCH leaves unset
/// fields untouched. Every field unrelated to this policy stays `None`.
///
/// The approval and merge whitelists' **teams** stay `None` too, and so does
/// `enable_approvals_whitelist`: swarm-controller puts the `operators` team on
/// both, so operators can merge config PRs in the forge UI, and this PATCH
/// runs on every boot.
fn config_repo_protection_edit() -> EditBranchProtectionOption {
EditBranchProtectionOption {
apply_to_admins: None,
approvals_whitelist_teams: None,
approvals_whitelist_username: None,
block_on_official_review_requests: Some(false),
block_on_outdated_branch: None,
block_on_rejected_reviews: None,
dismiss_stale_approvals: None,
enable_approvals_whitelist: None,
enable_merge_whitelist: Some(true),
enable_push: Some(false),
enable_push_whitelist: Some(false),
enable_status_check: None,
ignore_stale_approvals: None,
merge_whitelist_teams: None,
merge_whitelist_usernames: Some(vec!["core".to_owned()]),
protected_file_patterns: None,
push_whitelist_deploy_keys: None,
push_whitelist_teams: None,
push_whitelist_usernames: Some(Vec::new()),
require_signed_commits: None,
required_approvals: Some(0),
status_check_contexts: None,
unprotected_file_patterns: None,
}
}

32
swarm-controller/src/forge/objects.rs
View file

@ -33,7 +33,6 @@ use forgejo_api::{ApiErrorKind, ForgejoError};
use reqwest::StatusCode; use reqwest::StatusCode;
use serde::Deserialize; use serde::Deserialize;
use super::legacy_tokens::CORE_USER;
use super::{ use super::{
CONFIG_ORG, Client, KNOWLEDGE_ORG, KNOWLEDGE_REPO, OPERATORS_TEAM, base64_encode, CONFIG_ORG, Client, KNOWLEDGE_ORG, KNOWLEDGE_REPO, OPERATORS_TEAM, base64_encode,
folds_into_success, is_ambiguous_validation_failure, is_confirmed_conflict, folds_into_success, is_ambiguous_validation_failure, is_confirmed_conflict,
@ -523,11 +522,11 @@ fn team_matches(t: &Team) -> bool {
} }
/// The merge gate on every config repo's `main`: the `operators` team approves /// The merge gate on every config repo's `main`: the `operators` team approves
/// and merges, and `core` merges too, for the hive's dashboard approval. /// and merges, and no user does. The empty user list removes any user an
/// older rule carries.
/// ///
/// Forgejo replaces each list this sends wholesale and keeps every field left /// Forgejo replaces each list this sends wholesale and keeps every field left
/// `None`, `required_approvals` included — the hive's own boot PATCH sets /// `None` as the rule has it, `required_approvals` included.
/// that one.
fn config_rule_edit() -> EditBranchProtectionOption { fn config_rule_edit() -> EditBranchProtectionOption {
EditBranchProtectionOption { EditBranchProtectionOption {
apply_to_admins: None, apply_to_admins: None,
@ -544,7 +543,7 @@ fn config_rule_edit() -> EditBranchProtectionOption {
enable_status_check: None, enable_status_check: None,
ignore_stale_approvals: None, ignore_stale_approvals: None,
merge_whitelist_teams: Some(vec![OPERATORS_TEAM.to_owned()]), merge_whitelist_teams: Some(vec![OPERATORS_TEAM.to_owned()]),
merge_whitelist_usernames: Some(vec![CORE_USER.to_owned()]), merge_whitelist_usernames: Some(Vec::new()),
protected_file_patterns: None, protected_file_patterns: None,
push_whitelist_deploy_keys: None, push_whitelist_deploy_keys: None,
push_whitelist_teams: None, push_whitelist_teams: None,
@ -561,7 +560,10 @@ fn config_rule_matches(rule: &BranchProtection) -> bool {
let only = |list: &Option<Vec<String>>, want: &str| matches!(list.as_deref(), Some([entry]) if entry == want); let only = |list: &Option<Vec<String>>, want: &str| matches!(list.as_deref(), Some([entry]) if entry == want);
rule.enable_merge_whitelist == Some(true) rule.enable_merge_whitelist == Some(true)
&& only(&rule.merge_whitelist_teams, OPERATORS_TEAM) && only(&rule.merge_whitelist_teams, OPERATORS_TEAM)
&& only(&rule.merge_whitelist_usernames, CORE_USER) && rule
.merge_whitelist_usernames
.as_deref()
.is_none_or(<[String]>::is_empty)
&& rule.enable_approvals_whitelist == Some(true) && rule.enable_approvals_whitelist == Some(true)
&& only(&rule.approvals_whitelist_teams, OPERATORS_TEAM) && only(&rule.approvals_whitelist_teams, OPERATORS_TEAM)
} }
@ -1315,14 +1317,16 @@ mod tests {
} }
#[test] #[test]
fn a_config_rule_matches_only_with_operators_and_core() { fn a_config_rule_matches_only_with_operators_and_no_user() {
assert!(config_rule_matches(&rule(&[CORE_USER]))); assert!(config_rule_matches(&rule(&[])));
assert!(!config_rule_matches(&rule(&[]))); let mut users_absent = rule(&[]);
assert!(!config_rule_matches(&rule(&[CORE_USER, "mallory"]))); users_absent.merge_whitelist_usernames = None;
let mut core_only = rule(&[CORE_USER]); assert!(config_rule_matches(&users_absent));
core_only.merge_whitelist_teams = None; assert!(!config_rule_matches(&rule(&["core"])));
assert!(!config_rule_matches(&core_only)); let mut no_team = rule(&[]);
let mut approvals_off = rule(&[CORE_USER]); no_team.merge_whitelist_teams = None;
assert!(!config_rule_matches(&no_team));
let mut approvals_off = rule(&[]);
approvals_off.enable_approvals_whitelist = Some(false); approvals_off.enable_approvals_whitelist = Some(false);
assert!(!config_rule_matches(&approvals_off)); assert!(!config_rule_matches(&approvals_off));
} }