Require an approved result before a non-awaitingReview application state decodes

Open
#1,222 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
76/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
rust
Domain
backend, security

Research direction

Start in crates/registry-breg-client/src/lifecycle.rs at decode_external_review_status around line 2244, then inspect decode_external_review_application around line 2396 and the existing applicationId/state checks. Add decoder unit tests for rejected, cancelled, and pending results paired with each non-awaitingReview application state. Done means these malformed payloads are rejected while approved results continue to decode correctly.

Written by the indexing model from the issue text.

Description

area:breg bug rust

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.

Dominant language
Rust
Stars
2
Forks
0
Avg merge
3h 45m
Merged PRs (30d)
134

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from registrystack/registry-stack

All issues in registrystack/registry-stack

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.