sigstore / sigstore-conformance

Conformance testing for Sigstore clients
https://sigstore.dev
7 stars 10 forks source link

action: restrict label check to third-party PRs #62

Closed woodruffw closed 1 year ago

woodruffw commented 1 year ago

See https://github.com/sigstore/sigstore-python/pull/350#issuecomment-1358062994.

Signed-off-by: William Woodruff william@trailofbits.com

woodruffw commented 1 year ago

N.B.: This PR needs to be labeled because changes to pull_request_target flows don't take effect until they're merged into main.

tetsuo-cpp commented 1 year ago

I don't think this does what we want.

Our current guidance tells users to configure their workflow with:

on:
  pull_request_target:
    types: [labeled]

So the conformance testing gets kicked off as soon as label is applied and we do the check within the action itself in case its not the "safe to test" label. If we want to run conformance testing for a newer commit in the same PR, we need to reapply the label. If we're not requiring the "safe to test" label for non-fork PRs, then some other label needs to be applied to trigger testing.

The obvious thing to do would be to do something like:

on:
  pull_request_target:
    types: [labeled, push]

But this comes with its own set of problems because now 3rd party PRs are less restricted. If a user submits something that looks reasonable, we can apply the "safe to test" label and then they can immediately push malicious code and have it run since we're no longer only triggering off a label.

woodruffw commented 1 year ago

Oh, I think you're right -- I was inadvertently triggering the labeled event by pre-tagging my PRs with different labels. We should revert this.

woodruffw commented 1 year ago

Reverted.