search

w3c / ash-nazg

One interface to find all group contributors and in IPR bind them
https://labs.w3.org/repo-manager/
MIT License
22 stars 24 forks source link

Remediate "GitHub signature does not match known secret" error #213

Closed erik-anderson closed 3 years ago

erik-anderson commented 3 years ago

It looks like because of the change in #212, we now have a repo where the IPR bot is now bubbling up an error but was presumably falling over silently before. Great!

The repo seeing the error is privacycg/first-party-sets. Here's an example PR showing the IPR bot error: https://github.com/privacycg/first-party-sets/pull/58

It's not clear to me how I as a repo owner can remediate the issue. Is there an internal database on the https://labs.w3.org/repo-manager/ server that holds the secret and, if so, how can we reset it?

As far as how we entered this state, I'm curious if it's because the repo originally migrated from WICG/first-party-sets which was also being managed by the bot.

erik-anderson commented 3 years ago

I also see two webhooks for https://labs.w3.org/repo-manager/api/hook in the repo which appears to lend credence to the idea that it having been migrated having an impact.

I tried enabling one at a time, but both appear to be getting the same error about the secret not matching.

I left both hooks in a disabled state for now but can do something else with them if needed.

deniak commented 3 years ago

Indeed, the problem is probably related to the repository renaming. I see both WICG/first-party-sets and privacycg/first-party-sets are listed here so I'm guessing the repository was imported again into the repo-manager after the rename.

I have the action to fix the code so renames can be handled automatically but in the meantime, this requires a manual intervention to update the token and the name of the repo. Any chance you can grant me admin access to the repository so I can generate a new token and update the repo-manager database?

wanderview commented 3 years ago

I'm getting this in wicg/urlpattern as well. See WICG/urlpattern#107.

erik-anderson commented 3 years ago

Indeed, the problem is probably related to the repository renaming. I see both WICG/first-party-sets and privacycg/first-party-sets are listed here so I'm guessing the repository was imported again into the repo-manager after the rename.

I have the action to fix the code so renames can be handled automatically but in the meantime, this requires a manual intervention to update the token and the name of the repo. Any chance you can grant me admin access to the repository so I can generate a new token and update the repo-manager database?

Sure, I sent you an invite. I'll remove access after you've remediated the issue.

deniak commented 3 years ago

@erik-anderson, Thanks for the invite. Updating the token of the webhook did the trick. The last 2 PRs now pass the check. Feel free to delete the duplicated disabled webhook and remove my access to the repo.

deniak commented 3 years ago

@wanderview that's odd, the error on https://github.com/WICG/urlpattern/pull/100 is different from the signature issue but the logs show a problem with the webhook secret. From a quick look, it doesn't seem that repository was renamed or transferred. Anyhow, can you give me admin access to the repo so I can update the secret and debug more?

wanderview commented 3 years ago

Anyhow, can you give me admin access to the repo so I can update the secret and debug more?

Done. Thanks!

deniak commented 3 years ago

@wanderview, the secret was updated and https://github.com/WICG/urlpattern/pull/100 now pass the IPR check. Feel free to remove my access to the repository.

deniak commented 3 years ago

Closing this issue as it's being tracked in https://github.com/w3c/ash-nazg/issues/210

reillyeon commented 3 years ago

This is also happening for w3c/geolocation-sensor. I have granted @deniak temporary admin so that they can remediate.

deniak commented 3 years ago

@reillyeon Thanks for the report. It should now be fixed.

reillyeon commented 3 years ago

This is also happening for wicg/webusb. I've granted @deniak admin to the repo so they can remediate.

deniak commented 3 years ago

@reillyeon It's not fixed. I also noticed there are 2 webhooks so I'm guessing the repository was renamed and re-imported in the repository manager but one of the webhook was pointing to the wrong URL (https://labs.w3.org/hatchery/ash-nazg/api/hook instead of https://labs.w3.org/hatchery/repo-manager/api/hook). I'm wondering if this comes from a bug with the import tool or if someone had updated the webhook... Also can you confirm if the repository was indeed renamed? It'll help track the bug regarding the GH signature.

reillyeon commented 3 years ago

I don't think the wicg/webusb repository was renamed.

csharrison commented 3 years ago

This is also happening for wicg/conversion-measurement-api. I've granted @deniak admin to the repo to remediate. @deniak do you mind taking a look? Thanks1

deniak commented 3 years ago

@csharrison it should now be fixed. The repository had 3 webhooks pointing to the repository manager. I disabled 2 of them but something is adding these webhooks and it's messing with the secret used.

xfq commented 3 years ago

This is also happening for w3c/miniapp-manifest. @deniak would you mind taking a look? Thank you!

deniak commented 3 years ago

@xfq this is now fixed. I noticed the secret was updated about 3 weeks ago. Do you know if that repository was imported again in the repo-manager around that time?

xfq commented 3 years ago

@xfq this is now fixed.

Thank you.

I noticed the secret was updated about 3 weeks ago. Do you know if that repository was imported again in the repo-manager around that time?

I don't know. I didn't do this.

tidoust commented 2 years ago

@deniak Not sure if I should abuse this now closed issue, but we're getting the same problem on w3c/webcodecs. Started at least a couple of weeks ago, so probably consistent with the previous errors. Any magic you may do to fix that?

deniak commented 2 years ago

@tidoust it's now fixed. We did deploy a few changes that would help wrt this issue but for w3c/webcodecs, I can't find anything that could explain the problem. The secret we have was not updated and afaik, there was no major change to that repo in the last couple of weeks. I'll keep investigating.

tidoust commented 2 years ago

Thanks, @deniak! I don't recall any recent change on the settings of the w3c/webcodecs repo, indeed. I see repo migration could perhaps explain why this happened. The w3c/webcodecs was originally under the WICG organization, but that was a long time ago, already.

deniak commented 2 years ago

Thanks, @deniak! I don't recall any recent change on the settings of the w3c/webcodecs repo, indeed. I see repo migration could perhaps explain why this happened. The w3c/webcodecs was originally under the WICG organization, but that was a long time ago, already.

Looking at the logs, that signature issue was actually there for at least a few months but we only started to surface the error on the PR recently so I suspect the transfer did cause the problem. I'm also guessing the repository was re-imported again in the repository manager after the transfer which duplicated the webhook.

xfq commented 2 years ago

This is also happening for w3c/miniapp-lifecycle. @deniak would you mind taking a look? Thank you!

deniak commented 2 years ago

@xfq, all the secrets of known repositories have been updated earlier token so that issue should not happen anymore. Looking at the different PRs, I see https://github.com/w3c/miniapp-lifecycle/pull/16 failed but because @QingAn didn't connected his W3C account and Github acccount. He's fixed it since and revalidating the PR from the repository manager now pass.

xfq commented 2 years ago

@deniak Thank you.

Sorry for abusing this issue, but https://github.com/w3c/miniapp-packaging/pull/34 also failed, and I saw @espinr connected his W3C account and Github account. I also tried to revalidate the PR from the repository manager but it said "PR not found: w3c/miniapp-packaging/pulls/34".

deniak commented 2 years ago

@deniak Thank you.

Sorry for abusing this issue, but w3c/miniapp-packaging#34 also failed, and I saw @espinr connected his W3C account and Github account. I also tried to revalidate the PR from the repository manager but it said "PR not found: w3c/miniapp-packaging/pulls/34".

Ah, that one is due to the fact that the PR was created before we patch the code. I resubmitted the payloads for these commits and it now passes. Please do ping me if you see other PRs that need to be sync'ed again. Hopefully with the recent changes we made, this will no longer occur for future PRs.

himorin commented 2 years ago

we are also encountering this error on https://github.com/immersive-web/webxr-hand-input/pull/105

deniak commented 2 years ago

@himorin, I see the secret we have was updated recently. Can you invite me to the repo so I can debug the webhook and eventually sync the secret?

bokand commented 2 years ago

@deniak - I'm seeing this error message on https://github.com/WICG/visual-viewport/pull/80 - the repo was renamed (from VisualViewport) but a long long time ago (years ago). I've granted you admin access - could you please take a look? Thanks!

deniak commented 2 years ago

@bokand this should now be fixed. I deleted the duplicated webhook, which was probably added during a new repo import.