fix: validate review identity before CI verdict

src/lib.rs

@@ -977,7 +977,9 @@ fn poll_deadline(minutes: u64) -> Result<tokio::time::Instant, PollTimeoutTooLar
 }
 
 /// Reads the review, polling until it finishes or `deadline` passes. Without
-/// a deadline it reads once.
+/// a deadline it reads once. Every read confirms the server's answer is of
+/// the review requested, not a cache or routing mix-up that would otherwise
+/// let a result for a different review decide this one's outcome.
 async fn read_review(
     api: &Api,
     review_id: Uuid,
@@ -987,6 +989,7 @@ async fn read_review(
     let mut delay = FIRST_POLL_SECONDS;
     loop {
         let (raw, detail) = api.review(review_id).await?;
+        outcome::same_review(&detail, review_id)?;
         if sarif {
             sarif::validate_subject(&detail, review_id)?;
         }
@@ -1103,6 +1106,52 @@ async fn ci_job(
 /// it asks for one.
 const REQUEST_AFTER: Duration = Duration::from_secs(120);
 
+/// Confirms `detail` is actually the pull-request review of `commit` in
+/// `repository_id` before it is allowed to decide a CI verdict or produce a
+/// SARIF artifact. [`read_review`] already confirms `detail` is the review
+/// requested by id; a search match on `commit` against a summary row does
+/// not guarantee the detail fetched moments later, by that id, kept the same
+/// story, so the fields the search relied on are checked again here against
+/// the server's own account of the subject. The repository is compared by
+/// id, not `repository_full_name`: a GitHub rename changes the name but not
+/// the id, and comparing names would then refuse a review that is actually
+/// of the right repository. The id is checked only when the server names
+/// one, since it is absent on an old server or a review that names no
+/// repository.
+fn confirm_ci_subject(
+    detail: &ReviewDetail,
+    commit: &str,
+    repository_id: Uuid,
+) -> anyhow::Result<()> {
+    if detail.kind != "PULL_REQUEST" {
+        bail!(
+            "proofline asked for the pull-request review of {commit}, but the server returned a {} review",
+            text::one_line(&detail.kind)
+        );
+    }
+    if !detail
+        .head_commit_sha
+        .as_deref()
+        .is_some_and(|head| head.eq_ignore_ascii_case(commit))
+    {
+        bail!(
+            "proofline asked for the review of {commit}, but the server returned head commit {}",
+            detail
+                .head_commit_sha
+                .as_deref()
+                .map_or_else(|| "none".to_owned(), text::one_line)
+        );
+    }
+    if let Some(detail_repository) = detail.repository_id
+        && detail_repository != repository_id
+    {
+        bail!(
+            "proofline asked for a review in repository {repository_id}, but the server returned a review in repository {detail_repository}"
+        );
+    }
+    Ok(())
+}
+
 /// Waits for the review the App runs for the pull request whose head is
 /// `commit`, then reports it. With `request`, a job that has found no review
 /// after [`REQUEST_AFTER`] asks once for one and reports the review it gets;
@@ -1165,6 +1214,7 @@ async fn ci_review(
     let sarif_path = args.sarif.as_deref();
     let (raw, detail, state) =
         read_review(api, review_id, Some(deadline), sarif_path.is_some()).await?;
+    confirm_ci_subject(&detail, commit, repository_id)?;
     if let Some(path) = sarif_path {
         let document = sarif::review(
             &detail,

src/model.rs

@@ -82,6 +82,10 @@ pub struct ReviewDetail {
     pub head_commit_sha: Option<String>,
     pub open_items: Option<OpenItems>,
     pub pull_request_number: Option<i64>,
+    /// The review's repository. Set only on a single-review read, and
+    /// absent there when the review names no repository. Stable across a
+    /// repository rename, unlike `repository_full_name`.
+    pub repository_id: Option<Uuid>,
     pub repository_full_name: Option<String>,
     pub run_number: Option<i64>,
     #[serde(default)]

src/outcome.rs

@@ -2,11 +2,37 @@
 //! rules match `waitForReview` in the TypeScript SDK: unknown is never
 //! complete, and an assessment with a gap is never reported as clean.
 
+use uuid::Uuid;
+
 use crate::model::{
     Assessment, CodeAssessment, CodeAssessmentRead, CodeAssessmentStatus, ReviewDetail,
 };
 use crate::severity::Severity;
 
+/// A review-reading endpoint is keyed by review id, so its response should
+/// always echo the id requested. A cache or routing bug that serves another
+/// review's body under that id would otherwise read as a clean result for
+/// the wrong subject.
+#[derive(Debug, thiserror::Error)]
+#[error("requested review {requested}, but the server returned review {returned}")]
+pub(crate) struct WrongReview {
+    pub(crate) requested: Uuid,
+    pub(crate) returned: Uuid,
+}
+
+/// Confirms `detail` is the review `requested`, not merely a review that
+/// happened to come back from that endpoint.
+pub(crate) fn same_review(detail: &ReviewDetail, requested: Uuid) -> Result<(), WrongReview> {
+    if detail.id == requested {
+        Ok(())
+    } else {
+        Err(WrongReview {
+            requested,
+            returned: detail.id,
+        })
+    }
+}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum Outcome {
     Completed,
@@ -174,7 +200,7 @@ mod tests {
 
     fn assessment(status: &str) -> Assessment {
         Assessment {
-            id: uuid::Uuid::nil(),
+            id: Uuid::nil(),
             environment_display_name: "production".to_owned(),
             status: status.to_owned(),
             findings: Vec::new(),
@@ -191,7 +217,7 @@ mod tests {
 
     fn review(status: &str, assessments: Vec<Assessment>, count: i64) -> ReviewDetail {
         ReviewDetail {
-            id: uuid::Uuid::nil(),
+            id: Uuid::nil(),
             kind: "PULL_REQUEST".to_owned(),
             status: status.to_owned(),
             assessment_count: count,
@@ -202,6 +228,7 @@ mod tests {
             head_commit_sha: None,
             open_items: None,
             pull_request_number: Some(7),
+            repository_id: None,
             repository_full_name: Some("acme/web".to_owned()),
             run_number: Some(1),
             local_reverifications: Vec::new(),

src/render.rs

@@ -212,6 +212,7 @@ mod tests {
             head_commit_sha: Some("0123456789abcdef0123456789abcdef01234567".to_owned()),
             open_items: None,
             pull_request_number: Some(42),
+            repository_id: None,
             repository_full_name: Some("acme/web".to_owned()),
             run_number: Some(2),
             local_reverifications: Vec::new(),

src/sarif.rs

@@ -295,10 +295,14 @@ enum AssessmentSource<'a> {
 }
 
 pub(super) fn validate_subject(detail: &ReviewDetail, requested: Uuid) -> Result<(), SarifError> {
-    if detail.id != requested {
+    if let Err(outcome::WrongReview {
+        requested,
+        returned,
+    }) = outcome::same_review(detail, requested)
+    {
         return Err(SarifError::WrongReview {
             requested,
-            returned: detail.id,
+            returned,
         });
     }
     if detail.kind != "PULL_REQUEST" {