diff --git a/src/bors/handlers/workflow.rs b/src/bors/handlers/workflow.rs index 2e99ca22..054133f3 100644 --- a/src/bors/handlers/workflow.rs +++ b/src/bors/handlers/workflow.rs @@ -691,4 +691,27 @@ min_ci_time = 20 }) .await; } + + #[sqlx::test(migrator = "crate::MIGRATOR")] + async fn ignore_pull_request_event_workflow(pool: sqlx::PgPool) { + run_test(pool.clone(), async |ctx: &mut BorsTester| { + ctx.post_comment("@bors try").await?; + ctx.expect_comments((), 1).await; + + let workflow = ctx.try_workflow(); + ctx.modify_workflow(workflow, |run| { + run.set_event("pull_request"); + }); + + ctx.skip_waiting_for_marker(async |ctx| { + ctx.workflow_event(WorkflowEvent::started(workflow)).await?; + ctx.workflow_event(WorkflowEvent::success(workflow)).await + }) + .await?; + + Ok(()) + }) + .await; + assert!(get_all_workflows(&pool).await.unwrap().is_empty()); + } } diff --git a/src/server/webhook.rs b/src/server/webhook.rs index 577ac8aa..5572d1f8 100644 --- a/src/server/webhook.rs +++ b/src/server/webhook.rs @@ -335,6 +335,14 @@ fn parse_pull_request_review_comment_events(body: &[u8]) -> anyhow::Result anyhow::Result> { let payload: WebhookWorkflowRun = serde_json::from_slice(body)?; let repository_name = parse_repository_name(&payload.repository)?; + + // As a security precaution, we eagerly prefilter all workflow runs other than "push" here, + // to ensure that only workflows from privileged pushes to branches in the repository are + // registered by bors. + if payload.workflow_run.run.event != "push" { + return Ok(None); + } + let result = match payload.action { "requested" => Some(BorsEvent::Repository(BorsRepositoryEvent::WorkflowStarted( WorkflowRunStarted { diff --git a/src/tests/github.rs b/src/tests/github.rs index 26f171d8..1097a028 100644 --- a/src/tests/github.rs +++ b/src/tests/github.rs @@ -1117,6 +1117,7 @@ pub struct WorkflowRun { /// How long did the workflow run for? duration: Duration, status: WorkflowStatus, + event: String, } impl WorkflowRun { @@ -1130,6 +1131,7 @@ impl WorkflowRun { jobs: vec![], head_sha: branch.sha(), duration: Duration::from_secs(3600), + event: "push".to_string(), } } @@ -1172,6 +1174,13 @@ impl WorkflowRun { self.status } + pub fn event(&self) -> &str { + &self.event + } + pub fn set_event(&mut self, event: &str) { + self.event = event.to_string(); + } + pub fn add_job(&mut self, status: WorkflowStatus) { self.jobs.push(WorkflowJob { id: JobId(self.run_id.0 * 1000 + self.jobs.len() as u64), diff --git a/src/tests/mock/workflow.rs b/src/tests/mock/workflow.rs index ba9f96a7..f791f9e7 100644 --- a/src/tests/mock/workflow.rs +++ b/src/tests/mock/workflow.rs @@ -95,7 +95,7 @@ impl GitHubWorkflowRun { head_branch: run.head_branch().to_owned(), head_sha: run.head_sha().to_owned(), run_number: 0, - event: "".to_string(), + event: run.event().to_string(), status, conclusion, created_at, diff --git a/src/tests/mod.rs b/src/tests/mod.rs index d3dd6128..7e52dc82 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -195,6 +195,7 @@ pub struct BorsTester { mergeability_queue_rx: MergeabilityQueueReceiver, gitops_queue_rx: GitOpsQueueReceiver, ctx: Arc, + wait_for_markers: bool, } impl BorsTester { @@ -267,6 +268,7 @@ impl BorsTester { mergeability_queue_rx, gitops_queue_rx, ctx, + wait_for_markers: true, }, bors, ) @@ -525,6 +527,7 @@ impl BorsTester { .unwrap(); Ok(()) }, + self.wait_for_markers, &WAIT_FOR_BUILD_QUEUE, ) .await @@ -542,6 +545,7 @@ impl BorsTester { .unwrap(); Ok(()) }, + self.wait_for_markers, &WAIT_FOR_MERGEABILITY_STATUS_REFRESH, ) .await @@ -558,6 +562,7 @@ impl BorsTester { .unwrap(); Ok(()) }, + self.wait_for_markers, &WAIT_FOR_PR_STATUS_REFRESH, ) .await @@ -577,6 +582,7 @@ impl BorsTester { self.senders.merge_queue().perform_tick().await.unwrap(); Ok(()) }, + self.wait_for_markers, &WAIT_FOR_MERGE_QUEUE, ) .await @@ -592,6 +598,7 @@ impl BorsTester { .unwrap(); Ok(()) }, + self.wait_for_markers, &WAIT_FOR_MERGE_QUEUE, ) .await @@ -1180,6 +1187,7 @@ impl BorsTester { ); Ok(()) }, + self.wait_for_markers, &WAIT_FOR_WEBHOOK_COMPLETED, ) .await?; @@ -1196,6 +1204,18 @@ impl BorsTester { .await } + /// Run the provided async function while not waiting for markers to be hit. + pub async fn skip_waiting_for_marker(&mut self, func: F) -> R + where + F: AsyncFnOnce(&mut Self) -> R, + { + let orig = self.wait_for_markers; + self.wait_for_markers = false; + let res = func(self).await; + self.wait_for_markers = orig; + res + } + async fn finish(self, bors: JoinHandle<()>) -> anyhow::Result { // Tell the mergeability queue that it should shutdown once it has nothing else to do self.senders.mergeability_queue().shutdown(); @@ -1519,7 +1539,11 @@ impl PullRequestProxy { /// Start an async operation and wait until a specific [`TestSyncMarker`] /// is marked. -async fn wait_for_marker(func: Func, marker: &TestSyncMarker) -> anyhow::Result +async fn wait_for_marker( + func: Func, + wait: bool, + marker: &TestSyncMarker, +) -> anyhow::Result where Func: AsyncFnOnce() -> anyhow::Result, { @@ -1530,7 +1554,7 @@ where marker.drain().await; let res = func().await; - if res.is_ok() { + if wait && res.is_ok() { tokio::time::timeout(SYNC_MARKER_TIMEOUT, marker.sync()) .await .map_err(|_| anyhow::anyhow!("Timed out waiting for a test marker to be marked"))?;