fix(#2311): verify operator branch protection exists on create error instead of failing open
This commit is contained in:
parent
ee61a3d7e1
commit
17fa4a3ea5
1 changed files with 26 additions and 18 deletions
|
|
@ -714,21 +714,20 @@ fn main_branch_protection_option() -> CreateBranchProtectionOption {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Whether a branch-protection create failed because a rule for the
|
|
||||||
/// branch already exists: 409/422 ([`is_already_exists`]) or the 200
|
|
||||||
/// Forgejo answers instead of 201 for a duplicate rule (unlisted in
|
|
||||||
/// the endpoint spec, so it surfaces as `UnexpectedStatusCode(200)`).
|
|
||||||
fn is_protection_already_present(e: &ForgejoError) -> bool {
|
|
||||||
is_already_exists(e)
|
|
||||||
|| matches!(e, ForgejoError::UnexpectedStatusCode(s) if *s == StatusCode::OK)
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Apply the operator merge-gate branch protection to `repo`'s default
|
/// Apply the operator merge-gate branch protection to `repo`'s default
|
||||||
/// branch: only [`OPERATORS_TEAM`] members can merge, and an
|
/// branch: only [`OPERATORS_TEAM`] members can merge, and an
|
||||||
/// approving review from that team is required — so the author (a write-level
|
/// approving review from that team is required — so the author (a write-level
|
||||||
/// agent, not in the team) cannot merge its own PR. Idempotent: an existing
|
/// agent, not in the team) cannot merge its own PR.
|
||||||
/// rule for the branch (200/409/422) is treated as success.
|
///
|
||||||
|
/// Idempotent, but **verify-don't-trust**: a create failure is ambiguous —
|
||||||
|
/// "rule already exists" (success) OR a silent rejection that created NO rule
|
||||||
|
/// (e.g. a 422 where `OPERATORS_TEAM` doesn't exist in `AGENTS_ORG`). The old
|
||||||
|
/// code folded 200/409/422 into `Ok` and left the repo unprotected with no
|
||||||
|
/// error — a fail-open merge gate. So on any create error, GET the `main` rule
|
||||||
|
/// and only treat it as success if the rule is actually present (the exact fix
|
||||||
|
/// already applied to [`apply_config_repo_branch_protection`]).
|
||||||
async fn apply_operator_branch_protection(repo: &str, token: &str) -> Result<()> {
|
async fn apply_operator_branch_protection(repo: &str, token: &str) -> Result<()> {
|
||||||
|
let client = api(token)?;
|
||||||
let mut rule = main_branch_protection_option();
|
let mut rule = main_branch_protection_option();
|
||||||
rule.enable_merge_whitelist = Some(true);
|
rule.enable_merge_whitelist = Some(true);
|
||||||
rule.merge_whitelist_teams = Some(vec![OPERATORS_TEAM.to_owned()]);
|
rule.merge_whitelist_teams = Some(vec![OPERATORS_TEAM.to_owned()]);
|
||||||
|
|
@ -736,19 +735,28 @@ async fn apply_operator_branch_protection(repo: &str, token: &str) -> Result<()>
|
||||||
rule.approvals_whitelist_teams = Some(vec![OPERATORS_TEAM.to_owned()]);
|
rule.approvals_whitelist_teams = Some(vec![OPERATORS_TEAM.to_owned()]);
|
||||||
rule.required_approvals = Some(1);
|
rule.required_approvals = Some(1);
|
||||||
rule.block_on_official_review_requests = Some(true);
|
rule.block_on_official_review_requests = Some(true);
|
||||||
match api(token)?
|
let Err(create_err) = client
|
||||||
.repo_create_branch_protection(AGENTS_ORG, repo, rule)
|
.repo_create_branch_protection(AGENTS_ORG, repo, rule)
|
||||||
.await
|
.await
|
||||||
|
else {
|
||||||
|
tracing::info!(%repo, "forge: applied operator branch protection");
|
||||||
|
return Ok(());
|
||||||
|
};
|
||||||
|
match client
|
||||||
|
.repo_get_branch_protection(AGENTS_ORG, repo, "main")
|
||||||
|
.await
|
||||||
{
|
{
|
||||||
Ok(_) => {
|
Ok(_) => {
|
||||||
tracing::info!(%repo, "forge: applied operator branch protection");
|
tracing::debug!(
|
||||||
|
%repo, create_error = %create_err,
|
||||||
|
"forge: operator branch protection already present"
|
||||||
|
);
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
Err(e) if is_protection_already_present(&e) => {
|
Err(check_err) => anyhow::bail!(
|
||||||
tracing::debug!(%repo, "forge: branch protection already present");
|
"branch protection for {AGENTS_ORG}/{repo} not applied: create failed \
|
||||||
Ok(())
|
({create_err}); GET main rule failed ({check_err}), no `main` rule present"
|
||||||
}
|
),
|
||||||
Err(e) => Err(e).with_context(|| format!("create branch protection {AGENTS_ORG}/{repo}")),
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue