Skip to content

Conversation

bkabrda
Copy link

@bkabrda bkabrda commented Apr 22, 2025

Summary

This PR is related to sigstore/cosign#4098 - in order to fix that issue, the cosign code has to access the target name to see if it's one of the "fallbacks" or not (and treat it accordingly as explained in the linked issue).

I really wanted to keep this change contained to cosign codebase itself, but I just couldn't find a way to do this; I think adding this small PR here shouldn't hurt, as it is fully backwards compatible.

Release Note

Added a Name field to the TargetFile struct

Documentation

I believe this PR doesn't require any documentation update

@bkabrda bkabrda requested a review from a team as a code owner April 22, 2025 11:13
@cpanato
Copy link
Member

cpanato commented Apr 22, 2025

@bkabrda please sign the DCO

@bkabrda bkabrda force-pushed the target-file-add-name branch from 42c53ed to c3a271b Compare April 22, 2025 11:30
@bkabrda
Copy link
Author

bkabrda commented Apr 22, 2025

@cpanato Will do. I accidentally reset my Git signing config and I am struggling to get it to work again, but I should have it ready soon.

@bkabrda bkabrda force-pushed the target-file-add-name branch from c3a271b to 85b7015 Compare April 22, 2025 11:32
@bkabrda
Copy link
Author

bkabrda commented Apr 22, 2025

@cpanato done, I guess the DCO check didn't register me uploading the new public key to my GH account, so it might need to be rerun now (?) The GH UI is already telling me that the commit is "Verified", so I think I'm all set on my side.

@cpanato
Copy link
Member

cpanato commented Apr 22, 2025

you need to sign off the commit

DCO
There is one commit incorrectly signed off. This means that the author of this commit failed to include a Signed-off-by line in the commit message.

To avoid having PRs blocked in the future, always include Signed-off-by: Author Name <authoremail@example.com> in every commit message. You can also do this automatically by using the -s flag (i.e., git commit -s).

Here is how to fix the problem so that this code can be merged.

 ---- 
Rebase the branch
If you have a local git environment and meet the criteria below, one option is to rebase the branch and add your Signed-off-by lines in the new commits. Please note that if others have already begun work based upon the commits in this branch, this solution will rewrite history and may cause serious issues for collaborators ([described in the git documentation](https://git-scm.com/book/en/v2/Git-Branching-Rebasing) under "The Perils of Rebasing").

You should only do this if:

You are the only author of the commits in this branch
You are absolutely certain nobody else is doing any work based upon this branch
There are no empty commits in the branch (for example, a DCO Remediation Commit which was added using --allow-empty)
To add your Signed-off-by line to every commit in this branch:

Ensure you have a local copy of your branch by [checking out the pull request locally via command line](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/checking-out-pull-requests-locally).
In your local branch, run: git rebase HEAD~1 --signoff
Force push your changes to overwrite the branch: git push --force-with-lease origin target-file-add-name

Signed-off-by: Slavek Kabrda <bkabrda@redhat.com>
@bkabrda bkabrda force-pushed the target-file-add-name branch from 85b7015 to 33bf7f1 Compare April 22, 2025 12:27
@bkabrda
Copy link
Author

bkabrda commented Apr 22, 2025

@cpanato I think I forgot to force push, sorry. Should be all ok now.

Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This TUF client is deprecated but this is fine for fixing the bug. The longer-term fix is to migrate to the TUF v2 client and TrustedRoot, but this is of course a larger fix.

@haydentherapper haydentherapper merged commit 8f79f87 into sigstore:main Apr 22, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants