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

Committing with the Github's desktop app breaks Chain of Trust #334

Closed
JohanLorenzo opened this issue Apr 29, 2019 · 9 comments · Fixed by #338
Closed

Committing with the Github's desktop app breaks Chain of Trust #334

JohanLorenzo opened this issue Apr 29, 2019 · 9 comments · Fixed by #338

Comments

@JohanLorenzo
Copy link
Contributor

Error spotted at https://treeherder.mozilla.org/#/jobs?repo=fenix&revision=4cd0d464a7d5e15cc85f29e0dc14fcd127cca4ea&selectedJob=242996267:

2019-04-26T21:26:59 CRITICAL - signing:parent fzYG7Cy0RviQIijkGw8jvg: the runtime task doesn't match any rebuilt definition!
["[('change', 'payload.env.MOBILE_TRIGGERED_BY', ('web-flow', 'sblatz')),\n"
 " ('change',\n"
 "  'metadata.owner',\n"
 "  ('web-flow@users.noreply.github.com', 'sblatz@users.noreply.github.com'))]"]

https://api.github.com/repos/mozilla-mobile/fenix/commits/4cd0d464a7d5e15cc85f29e0dc14fcd127cca4ea returns:

 "committer": {
    "login": "web-flow",
  }

which points to this Github user:

git show --format=fuller 4cd0d464a7d5e15cc85f29e0dc14fcd127cca4ea shows:

commit 4cd0d464a7d5e15cc85f29e0dc14fcd127cca4ea
Author:     Sawyer Blatz <sdblatz@gmail.com>
AuthorDate: Fri Apr 26 13:54:38 2019 -0700
Commit:     GitHub <noreply@github.com>
CommitDate: Fri Apr 26 13:54:38 2019 -0700

I confirmed with @sblatz that he used Github's desktop app to make this commit. That error happened on other commits of his.

In #327, I changed CoT to use the committer data, instead of the author one because of a similar kind of breakage (#326).

We might want to change

'login': commit_data['committer']['login'],
to detect if the committer is web-flow and then fall back to the author data instead. We may run into another error if the person who merges the PR is different. Let's check that first.

@JohanLorenzo
Copy link
Contributor Author

@sblatz could you make a commit (on Fenix) with the desktop app and let someone else merge the PR? This data would be useful to assess what's the best fix.

@sblatz
Copy link

sblatz commented Apr 29, 2019

@JohanLorenzo: this is an example of me committing with desktop app and someone else merging: mozilla-mobile/fenix#2121

@JohanLorenzo
Copy link
Contributor Author

Thanks for the link! So, it seems the bug only occurs when someone makes a commit with the desktop app and then merges the PR themselves.

mozilla-mobile/fenix#2121 shows:

@JohanLorenzo
Copy link
Contributor Author

JohanLorenzo commented May 6, 2019

https://help.github.com/en/desktop/getting-started-with-github-desktop/authenticating-to-github-using-the-browser might explain why the "Web Flow" user is also used on desktop.

I didn't find any other explanation on Github's help pages.

@MihaiTabara
Copy link
Contributor

Rolling it out to production workers via mozilla-releng/build-puppet#476

@JohanLorenzo
Copy link
Contributor Author

Hmmm, I don't understand. Even though the change is deployed, the error remains: https://tools.taskcluster.net/groups/UDHP3ZpqSE-PyoDmxl_2sg/tasks/Dr4ies5VSWWpTE96Y2qXdw/runs/1/logs/public%2Flogs%2Fchain_of_trust.log

I'm looking into it

@JohanLorenzo
Copy link
Contributor Author

Actually, the change isn't deployed because the worker is pinned to another environment. Unpinning it.

@JohanLorenzo
Copy link
Contributor Author

@JohanLorenzo
Copy link
Contributor Author

See also bug 1781051 for another issue with web-flow.

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 a pull request may close this issue.

3 participants