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

webhooks: Handle the case where a signature arrives after an unsigned artifact had been stored #919

Merged
merged 2 commits into from
Sep 9, 2023

Conversation

jhrozek
Copy link
Contributor

@jhrozek jhrozek commented Sep 8, 2023

It appears that there can be a race condition when we listen to GH
webhook messages that notify about new packages. First, an unsigned
package would be delivered and then a package signature, manifesting as
a package whose name corresponds to the digest of the package being signed.

As a result, we would first store the unsigned package but then receive
the signature separately but wouldn't link the two or even process the
signature message.

As a result, github actions that produced signed packages would sometimes fail
our policies, seemingly at random. Even more confusingly, if the developer
tried to just redeliver the message payload, the code would have worked because
at that point the signature would have been published and mediator would
process it.

In order to handle it, this patch, upon receiving a signature package
notification tries to find an artifact version with the same SHA as the
signature and instead of processing the signature, re-evalute the
package that corresponds to the signature again.

The first patch in this PR just modifies the GetVersionBySHA call
to only search by SHA, that is always unique anyway and at the point we
need to call this query we might not have the artifact ID anyway.

… artifact had been stored

it appears that there can be a race condition when we listen to GH
webhook messages that notify about new packages. First, an unsigned
package would be delivered and then a package signature, manifesting as
a package whose name corresponds to the digest of the package being signed.

As a result, we would first store the unsigned package but then receive
the signature separately but wouldn't link the two or even process the
signature message.

As a result, github actions that produced signed packages would sometimes fail
our policies, seemingly at random. Even more confusingly, if the developer
tried to just redeliver the message payload, the code would have worked because
at that point the signature would have been published and mediator would
process it.

In order to handle it, this patch, upon receiving a signature package
notification tries to find an artifact version with the same SHA as the
signature and instead of processing the signature, re-evalute the
package that corresponds to the signature again.

The first patch in this PR just modifies the GetVersionBySHA call
to only search by SHA, that is always unique anyway and at the point we
need to call this query we might not have the artifact ID anyway.
@jhrozek
Copy link
Contributor Author

jhrozek commented Sep 8, 2023

@lukehinds I think this patch would fix the issue you were seeing. I don't know why you could reproduce the issue so easily and I couldn't, but eventually I managed to hit it.

@lukehinds
Copy link
Contributor

Nice work finding that, I figured it would be this, but thought the existing code was filtering correctly. Thanks and will test this out!

@lukehinds lukehinds merged commit 0824729 into mindersec:main Sep 9, 2023
12 checks passed
@lukehinds lukehinds deleted the art_debug branch September 9, 2023 05:47
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