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

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

Closed
erik-anderson opened this issue Aug 31, 2021 · 31 comments
Closed

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

erik-anderson opened this issue Aug 31, 2021 · 31 comments
Assignees

Comments

@erik-anderson
Copy link

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:
WICG/first-party-sets#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
Copy link
Author

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 deniak self-assigned this Aug 31, 2021
@deniak
Copy link
Member

deniak commented Aug 31, 2021

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
Copy link
Member

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

@erik-anderson
Copy link
Author

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
Copy link
Member

deniak commented Sep 1, 2021

@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
Copy link
Member

deniak commented Sep 1, 2021

@wanderview that's odd, the error on whatwg/urlpattern#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
Copy link
Member

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

Done. Thanks!

@deniak
Copy link
Member

deniak commented Sep 1, 2021

@wanderview, the secret was updated and whatwg/urlpattern#100 now pass the IPR check.
Feel free to remove my access to the repository.

@deniak
Copy link
Member

deniak commented Sep 1, 2021

Closing this issue as it's being tracked in #210

@deniak deniak closed this as completed Sep 1, 2021
@reillyeon
Copy link
Member

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

@deniak
Copy link
Member

deniak commented Sep 2, 2021

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

@reillyeon
Copy link
Member

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

@deniak
Copy link
Member

deniak commented Sep 9, 2021

@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
Copy link
Member

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

@csharrison
Copy link

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
Copy link
Member

deniak commented Sep 9, 2021

@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
Copy link
Member

xfq commented Sep 15, 2021

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

@deniak
Copy link
Member

deniak commented Sep 15, 2021

@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
Copy link
Member

xfq commented Sep 15, 2021

@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
Copy link
Member

tidoust commented Sep 22, 2021

@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
Copy link
Member

deniak commented Sep 22, 2021

@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
Copy link
Member

tidoust commented Sep 22, 2021

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
Copy link
Member

deniak commented Sep 22, 2021

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
Copy link
Member

xfq commented Sep 23, 2021

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

@deniak
Copy link
Member

deniak commented Sep 23, 2021

@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 w3c/miniapp-lifecycle#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
Copy link
Member

xfq commented Sep 23, 2021

@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".

@deniak
Copy link
Member

deniak commented Sep 23, 2021

@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
Copy link

himorin commented Sep 30, 2021

we are also encountering this error on immersive-web/webxr-hand-input#105

@deniak
Copy link
Member

deniak commented Oct 1, 2021

@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
Copy link

bokand commented Jan 4, 2022

@deniak - I'm seeing this error message on WICG/visual-viewport#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
Copy link
Member

deniak commented Jan 4, 2022

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

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

No branches or pull requests

9 participants