merge-request: verify guarded target updates
This commit is contained in:
@@ -319,10 +319,24 @@ impl RepositoryRegistryReader {
|
||||
result_commit: &str,
|
||||
) -> Result<(), RepositoryLookupError> {
|
||||
let repository = self.merge_repository(id)?;
|
||||
if !selector.starts_with("refs/heads/")
|
||||
|| selector.starts_with('-')
|
||||
|| selector.as_bytes().contains(&0)
|
||||
{
|
||||
let target_ref = if selector.starts_with("refs/heads/") {
|
||||
selector.to_owned()
|
||||
} else if selector.starts_with("refs/") {
|
||||
return Err(RepositoryLookupError::InvalidSelector {
|
||||
id: id.into(),
|
||||
selector: selector.into(),
|
||||
});
|
||||
} else {
|
||||
format!("refs/heads/{selector}")
|
||||
};
|
||||
let valid_ref = Command::new("git")
|
||||
.args(["check-ref-format", target_ref.as_str()])
|
||||
.status()
|
||||
.map_err(|_| RepositoryLookupError::ProviderFailure {
|
||||
id: id.into(),
|
||||
operation: "validate target branch ref".into(),
|
||||
})?;
|
||||
if !valid_ref.success() || selector.starts_with('-') || selector.as_bytes().contains(&0) {
|
||||
return Err(RepositoryLookupError::InvalidSelector {
|
||||
id: id.into(),
|
||||
selector: selector.into(),
|
||||
@@ -332,7 +346,12 @@ impl RepositoryRegistryReader {
|
||||
let status = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(&repository.path)
|
||||
.args(["update-ref", selector, result_commit, expected_target])
|
||||
.args([
|
||||
"update-ref",
|
||||
target_ref.as_str(),
|
||||
result_commit,
|
||||
expected_target,
|
||||
])
|
||||
.status()
|
||||
.map_err(|_| RepositoryLookupError::ProviderFailure {
|
||||
id: id.into(),
|
||||
@@ -724,7 +743,7 @@ mod tests {
|
||||
);
|
||||
reader.ensure_ancestor("main", &base, &source).unwrap();
|
||||
reader
|
||||
.update_merge_target("main", "refs/heads/main", &base, &source)
|
||||
.update_merge_target("main", "main", &base, &source)
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
reader
|
||||
|
||||
@@ -4028,6 +4028,35 @@ async fn scoped_complete_merge_request(
|
||||
)
|
||||
.map_err(repository_merge_evidence_error)?;
|
||||
}
|
||||
let verified_target = repositories.observe_merge_target(&mr.repository_id, Some(selector));
|
||||
let verified_result = matches!(
|
||||
verified_target.as_ref(),
|
||||
Ok(target) if target.commit == input.result_commit
|
||||
);
|
||||
if !verified_result {
|
||||
if !target_was_already_updated {
|
||||
if let Err(rollback_error) = repositories.update_merge_target(
|
||||
&mr.repository_id,
|
||||
selector,
|
||||
&input.result_commit,
|
||||
&input.target_commit,
|
||||
) {
|
||||
return Err(Error::InvalidInput(format!(
|
||||
"post-update target verification failed and guarded rollback also failed: verification={verified_target:?}; rollback={rollback_error:?}"
|
||||
))
|
||||
.into());
|
||||
}
|
||||
}
|
||||
return Err(Error::InvalidInput(format!(
|
||||
"post-update target verification failed: expected result {}, observed {:?}",
|
||||
input.result_commit,
|
||||
verified_target
|
||||
.as_ref()
|
||||
.map(|target| target.commit.as_str())
|
||||
.map_err(|error| error)
|
||||
))
|
||||
.into());
|
||||
}
|
||||
let completion = merge_request::CompleteMergeRequest {
|
||||
operation_id: input.operation_id,
|
||||
ticket_id,
|
||||
|
||||
Reference in New Issue
Block a user