diff --git a/hive-c0re/src/forge/reconcile.rs b/hive-c0re/src/forge/reconcile.rs index 3483ed1f..a0242d2c 100644 --- a/hive-c0re/src/forge/reconcile.rs +++ b/hive-c0re/src/forge/reconcile.rs @@ -8,8 +8,8 @@ //! the divergence). `Apply(Forge)` resets the local applied checkout to //! 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 -//! auto-deploy. `Apply(Local)` is not supported: forge `main` is core-only -//! branch-protected (advanced solely by the config-PR ff-merge API). +//! auto-deploy. `Apply(Local)` is not supported: forge `main` is +//! branch-protected, so it only advances through a config-PR merge. use std::path::{Path, PathBuf}; @@ -141,8 +141,8 @@ pub async fn reconcile_config_apply( ) -> Result { match direction { ReconcileDirection::Local => Ok(HostResponse::error( - "reconciling forge from local is not supported yet: forge main is core-only \ - branch-protected (no-push, ff-merge-API-only). Resolve via a config PR, or use \ + "reconciling forge from local is not supported yet: forge main is \ + branch-protected. Resolve via a config PR, or use \ `--from forge` to reset the local checkout to forge main.", )), ReconcileDirection::Forge => { diff --git a/hive-c0re/src/forge/repos.rs b/hive-c0re/src/forge/repos.rs index d69ab244..93771621 100644 --- a/hive-c0re/src/forge/repos.rs +++ b/hive-c0re/src/forge/repos.rs @@ -9,8 +9,7 @@ use std::sync::Mutex; use anyhow::{Context, Result}; use forgejo_api::structs::{ - AddCollaboratorOption, AddCollaboratorOptionPermission, CreateBranchProtectionOption, - CreateRepoOption, EditBranchProtectionOption, Repository, + AddCollaboratorOption, AddCollaboratorOptionPermission, CreateRepoOption, Repository, }; use forgejo_api::{ApiErrorKind, ForgejoError}; use reqwest::StatusCode; @@ -157,12 +156,11 @@ pub async fn push_meta(dir: &Path) -> Result<()> { } /// Ensure the `agent-configs/` repo exists so the first -/// `push_config` doesn't 404, and wire it as the agent-editable PR surface: -/// the agent is a **write** collaborator (can push feature branches + -/// open config PRs) and `main` is branch-protected core-only (only hive-c0re's -/// merge handler lands on it; operator approval required). No-op when the forge -/// isn't running or the core token isn't minted yet. Safe to call on every -/// spawn and on every startup (all steps idempotent). +/// `push_config` doesn't 404, and add the agent as a **write** collaborator +/// (it can push feature branches + open config PRs). Branch protection on +/// `main` is swarm-controller's, not set here. No-op when the forge isn't +/// running or the core token isn't minted yet. Safe to call on every spawn +/// and on every startup (all steps idempotent). pub async fn ensure_config_repo(name: &str) -> Result<()> { if !is_present().await { return Ok(()); @@ -171,8 +169,6 @@ pub async fn ensure_config_repo(name: &str) -> Result<()> { return Ok(()); }; 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( CONFIG_ORG, name, @@ -180,61 +176,7 @@ pub async fn ensure_config_repo(name: &str) -> Result<()> { AddCollaboratorOptionPermission::Write, &token, ) - .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>> = OnceLock::new(); - static GUARD: OnceLock>> = 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::>().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)), - } + .await } /// 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 /// (`deployed/`, …) and add-only, so they must always land. `main`, -/// by contrast, is core-only branch-protected and authoritatively -/// advanced by `pr_merge`'s ff-only merge API — so a mirror push of an -/// already-established `main` is routinely rejected (protected-branch, -/// or non-fast-forward after a rolled-back deploy). Git's pre-receive +/// by contrast, is branch-protected and advances through an operator's +/// merge on the forge — so a mirror push of an already-established `main` +/// is routinely rejected (protected-branch, or non-fast-forward). Git's pre-receive /// hook is all-or-nothing: bundling both refspecs in one push means that /// `main` reject declines the whole push, dropping the tags too. /// 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 - // established main is expected (pr_merge owns main); only the initial - // empty-repo seed actually advances it here. + // established main is expected (an operator's forge merge advances it); + // 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?; if !out.status.success() { let stderr = String::from_utf8_lossy(&out.stderr); @@ -365,8 +306,8 @@ pub async fn push_config(name: &str) -> Result<()> { { tracing::info!( %name, - "forge: mirror push of main skipped — forge main is protected / \ - owned by the config-PR ff-merge (expected); tags mirrored" + "forge: mirror push of main skipped — forge main is protected \ + and advances by an operator merge (expected); tags mirrored" ); return Ok(()); } @@ -624,147 +565,3 @@ async fn add_collaborator( tracing::debug!(%owner, %repo, %user, ?permission, "forge: collaborator set"); 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/` 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/` `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, - } -} diff --git a/swarm-controller/src/forge/objects.rs b/swarm-controller/src/forge/objects.rs index 8397b604..0a055447 100644 --- a/swarm-controller/src/forge/objects.rs +++ b/swarm-controller/src/forge/objects.rs @@ -33,7 +33,6 @@ use forgejo_api::{ApiErrorKind, ForgejoError}; use reqwest::StatusCode; use serde::Deserialize; -use super::legacy_tokens::CORE_USER; use super::{ CONFIG_ORG, Client, KNOWLEDGE_ORG, KNOWLEDGE_REPO, OPERATORS_TEAM, base64_encode, 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 -/// 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 -/// `None`, `required_approvals` included — the hive's own boot PATCH sets -/// that one. +/// `None` as the rule has it, `required_approvals` included. fn config_rule_edit() -> EditBranchProtectionOption { EditBranchProtectionOption { apply_to_admins: None, @@ -544,7 +543,7 @@ fn config_rule_edit() -> EditBranchProtectionOption { enable_status_check: None, ignore_stale_approvals: None, 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, push_whitelist_deploy_keys: None, push_whitelist_teams: None, @@ -561,7 +560,10 @@ fn config_rule_matches(rule: &BranchProtection) -> bool { let only = |list: &Option>, want: &str| matches!(list.as_deref(), Some([entry]) if entry == want); rule.enable_merge_whitelist == Some(true) && 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) && only(&rule.approvals_whitelist_teams, OPERATORS_TEAM) } @@ -1315,14 +1317,16 @@ mod tests { } #[test] - fn a_config_rule_matches_only_with_operators_and_core() { - assert!(config_rule_matches(&rule(&[CORE_USER]))); - assert!(!config_rule_matches(&rule(&[]))); - assert!(!config_rule_matches(&rule(&[CORE_USER, "mallory"]))); - let mut core_only = rule(&[CORE_USER]); - core_only.merge_whitelist_teams = None; - assert!(!config_rule_matches(&core_only)); - let mut approvals_off = rule(&[CORE_USER]); + fn a_config_rule_matches_only_with_operators_and_no_user() { + assert!(config_rule_matches(&rule(&[]))); + let mut users_absent = rule(&[]); + users_absent.merge_whitelist_usernames = None; + assert!(config_rule_matches(&users_absent)); + assert!(!config_rule_matches(&rule(&["core"]))); + let mut no_team = rule(&[]); + no_team.merge_whitelist_teams = None; + assert!(!config_rule_matches(&no_team)); + let mut approvals_off = rule(&[]); approvals_off.enable_approvals_whitelist = Some(false); assert!(!config_rule_matches(&approvals_off)); }