An automated review of #1189 flagged a missing cross-field invariant in the BReg external review status decoder. decode_external_review_application in crates/registry-breg-client/src/lifecycle.rs (around line 2396) decodes application.state (ready, queued, applying, applied, blocked, in addition to awaitingReview) independently of result.state; the caller, decode_external_review_status (around line 2244), only cross-checks that application.applicationId is present exactly when application.state == Applied, and never checks result.state against application.state.
Failure scenario: a malformed or corrupted status payload that pairs a rejected, cancelled, or even still-pending result with an application state of ready/queued/applying/applied/blocked decodes successfully, even though only an approved result should ever authorize moving an application out of awaitingReview. Any downstream code that trusts a decoded non-awaitingReview application state as proof of approval would be acting on an unapproved or rejected change.
Suggested fix:
- In
decode_external_review_status, require result.state == Approved whenever application.state != AwaitingReview, and reject the payload otherwise.
- Add decoder unit tests covering a
rejected/cancelled/pending result paired with each non-awaitingReview application state.
Filed from the review of #1189; not merge-blocking.
An automated review of #1189 flagged a missing cross-field invariant in the BReg external review status decoder.
decode_external_review_applicationincrates/registry-breg-client/src/lifecycle.rs(around line 2396) decodesapplication.state(ready,queued,applying,applied,blocked, in addition toawaitingReview) independently ofresult.state; the caller,decode_external_review_status(around line 2244), only cross-checks thatapplication.applicationIdis present exactly whenapplication.state == Applied, and never checksresult.stateagainstapplication.state.Failure scenario: a malformed or corrupted status payload that pairs a
rejected,cancelled, or even still-pendingresult with an application state ofready/queued/applying/applied/blockeddecodes successfully, even though only an approved result should ever authorize moving an application out ofawaitingReview. Any downstream code that trusts a decoded non-awaitingReviewapplication state as proof of approval would be acting on an unapproved or rejected change.Suggested fix:
decode_external_review_status, requireresult.state == Approvedwheneverapplication.state != AwaitingReview, and reject the payload otherwise.rejected/cancelled/pendingresult paired with each non-awaitingReviewapplication state.Filed from the review of #1189; not merge-blocking.