Looks like this needs some additional permissions per step one of the action's docs.

I can't see the vuln that Fossa's complaining about. @nitrocode can you take a look?

Thanks for the contribution!

Could you test this out in your fork by merging your signing-and-sbom branch into your fork's main branch? That should trigger a push and then we can then look at the released image to see if it successfully added image attestation.

the vulnerbility issue should be fixed per #5165

@nitrocode @chenrui333 Friends, thank you for your patience. We conducted some additional tests and were able to successfully sign/attest to an image in my fork.

I would appreciate it if you could review the most recent commit here, and give it a maintainer-grade 👍 if you're okay with it.

@jamengual ok - it's still failing, but that's expected. it's because we're hitting this issue in the attest-build-provenance action - tl;dr, the action won't run if it's run as part of a merge from a fork, the merge has to be completed from a branch in the same repo.

in my fork, the attestation works on the latest build. are you comfortable approving a merge with this in mind?

The image building is expected to fail because the github token changes are not merged to main yet.

@notdurson question, I saw we were gating the attest and push on the global PUSH var. Why was it removed?

remove conditional for image attestation step

@GenPage I removed it because it was causing the attestation step to skip during test builds. I can include dependency updates in tests in the future, in order to trigger a push; however, this might fail due to my lack of member/maintainer permission. It'll also slow down the process.

Would you like me to add the gate back, or are you okay with leaving it out?

@notdurson Ah, so it was for testing. Our main image has a gate for pushing where we want to test builds and not push (on PRs). Let's add it back for consistency unless I misunderstand the attestation point. It should only be required for images pushed not solely built correct?

@GenPage Sounds good, incoming!

@GenPage @jamengual apologies for delay. I cleaned the tree and force-pushed the relevant changes.