Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 fix signed-releases lookback limit precedence #4060

Merged
merged 3 commits into from
May 2, 2024

Conversation

spencerschrock
Copy link
Member

What kind of change does this PR introduce?

bug fix

What is the current behavior?

if the 6th release had no assets, the lookback limit exit condition was being skipped. This led to scenarios where too many releases were being considered by the Signed-Releases check.

What is the new behavior (if this is a feature change)?**

The lookback exit condition is now the first thing performed.

  • Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes

Fixes #4059

Special notes for your reviewer

Does this PR introduce a user-facing change?

For user-facing changes, please add a concise, human-readable release note to
the release-note

(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

Fixed a Signed-Releases bug where more releases were being analyzed than intended.

if the 6th release had no assets, the lookback limit exit condition was
being skipped. This led to scenarios where too many releases were being
considered by the Signed-Releases check.

ossf#4059

Signed-off-by: Spencer Schrock <sschrock@google.com>
@spencerschrock spencerschrock requested a review from a team as a code owner April 29, 2024 17:58
@spencerschrock spencerschrock requested review from justaugustus and raghavkaul and removed request for a team April 29, 2024 17:58
any release after the lookback should be skipped

Signed-off-by: Spencer Schrock <sschrock@google.com>
@spencerschrock
Copy link
Member Author

/scdiff generate Signed-Releases

Copy link

probes/releasesAreSigned/impl.go Show resolved Hide resolved
Signed-off-by: Spencer Schrock <sschrock@google.com>
@spencerschrock
Copy link
Member Author

Here's a link to the scdiff run

Just for the record. The output difference for github.com/pallets/jinja is expected, as the repo's releases were triggering the same bug and considering too many releases https://github.com/pallets/jinja/releases

@spencerschrock spencerschrock merged commit b9c1f3f into ossf:main May 2, 2024
36 checks passed
@spencerschrock spencerschrock deleted the fix-signed-releases branch May 2, 2024 17:43
seelder pushed a commit to seelder/scorecard that referenced this pull request May 3, 2024
* switch signed-releases lookback limit precedence

if the 6th release had no assets, the lookback limit exit condition was
being skipped. This led to scenarios where too many releases were being
considered by the Signed-Releases check.

ossf#4059

Signed-off-by: Spencer Schrock <sschrock@google.com>

* make exit condition stronger

any release after the lookback should be skipped

Signed-off-by: Spencer Schrock <sschrock@google.com>

---------

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: seelder <seelder@ncsu.edu>
seelder pushed a commit to seelder/scorecard that referenced this pull request May 3, 2024
* switch signed-releases lookback limit precedence

if the 6th release had no assets, the lookback limit exit condition was
being skipped. This led to scenarios where too many releases were being
considered by the Signed-Releases check.

ossf#4059

Signed-off-by: Spencer Schrock <sschrock@google.com>

* make exit condition stronger

any release after the lookback should be skipped

Signed-off-by: Spencer Schrock <sschrock@google.com>

---------

Signed-off-by: Spencer Schrock <sschrock@google.com>
Signed-off-by: seelder <seelder@ncsu.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

BUG Signed-Releases: internal error: too many releases, please report this
2 participants