GitHub: CoT can only verify a task of a Pull Request if that task is still the tip of the PR branch

[JohanLorenzo]

This behavior has been around ever since Chain of Trust supports Github Pull Requests (#307). I've never taken the time to document it because I used to be one of few people who ran some scriptworker tasks on PRs. The projects have evolved since then and this is definitely not true anymore. The best example is Fenix: on every single PR, signingscript signs APKs with a dummy key so Firebase can test them on real devices.

Last Friday, someone ran into this issue. The logs were these ones:

2020-11-12T22:59:16 CRITICAL - scriptworker:parent BB780dv5TqO3c8qXj0xo1w: the runtime task doesn't match any rebuilt definition!
["[('change',\n"
 "  'metadata.source',\n"
 '  '
 "('https://github.com/gabrielluong/fenix/raw/439603f3ab14f7367261ce557c64b30e9cef3476/.taskcluster.yml',\n"
 '   '
 "'https://github.com/gabrielluong/fenix/raw/3283e0bf2feb9c13c260c6555fe642b7f95f1679/.taskcluster.yml')),\n"
 " ('change',\n"
 "  'payload.env.MOBILE_HEAD_REV',\n"
 "  ('439603f3ab14f7367261ce557c64b30e9cef3476',\n"
 "   '3283e0bf2feb9c13c260c6555fe642b7f95f1679'))]"]
2020-11-12T22:59:16 CRITICAL - Chain of Trust verification error!
Traceback (most recent call last):
  File "/app/lib/python3.8/site-packages/scriptworker/cot/verify.py", line 1648, in verify_parent_task
    await verify_parent_task_definition(chain, link)
  File "/app/lib/python3.8/site-packages/scriptworker/cot/verify.py", line 1531, in verify_parent_task_definition
    compare_jsone_task_definition(parent_link, rebuilt_definitions)
  File "/app/lib/python3.8/site-packages/scriptworker/cot/verify.py", line 1605, in compare_jsone_task_definition
    raise CoTError(error_msg)
scriptworker.exceptions.CoTError: 'scriptworker:parent BB780dv5TqO3c8qXj0xo1w: the runtime task doesn\'t match any rebuilt definition!\n["[(\'change\',\\n"\n "  \'metadata.source\',\\n"\n \'  \'\n "(\'https://github.com/gabrielluong/fenix/raw/439603f3ab14f7367261ce557c64b30e9cef3476/.taskcluster.yml\',\\n"\n \'   \'\n "\'https://github.com/gabrielluong/fenix/raw/3283e0bf2feb9c13c260c6555fe642b7f95f1679/.taskcluster.yml\')),\\n"\n " (\'change\',\\n"\n "  \'payload.env.MOBILE_HEAD_REV\',\\n"\n "  (\'439603f3ab14f7367261ce557c64b30e9cef3476\',\\n"\n "   \'3283e0bf2feb9c13c260c6555fe642b7f95f1679\'))]"]'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/app/lib/python3.8/site-packages/scriptworker/cot/verify.py", line 1965, in verify_chain_of_trust
    await verify_task_types(chain)
  File "/app/lib/python3.8/site-packages/scriptworker/cot/verify.py", line 1724, in verify_task_types
    await valid_task_types[task_type](chain, obj)
  File "/app/lib/python3.8/site-packages/scriptworker/cot/verify.py", line 1650, in verify_parent_task
    raise CoTError(e)
scriptworker.exceptions.CoTError: CoTError('scriptworker:parent BB780dv5TqO3c8qXj0xo1w: the runtime task doesn\'t match any rebuilt definition!\n["[(\'change\',\\n"\n "  \'metadata.source\',\\n"\n \'  \'\n "(\'https://github.com/gabrielluong/fenix/raw/439603f3ab14f7367261ce557c64b30e9cef3476/.taskcluster.yml\',\\n"\n \'   \'\n "\'https://github.com/gabrielluong/fenix/raw/3283e0bf2feb9c13c260c6555fe642b7f95f1679/.taskcluster.yml\')),\\n"\n " (\'change\',\\n"\n "  \'payload.env.MOBILE_HEAD_REV\',\\n"\n "  (\'439603f3ab14f7367261ce557c64b30e9cef3476\',\\n"\n "   \'3283e0bf2feb9c13c260c6555fe642b7f95f1679\'))]"]')

https://firefox-ci-tc.services.mozilla.com/tasks/AF7-of6tTL-_k_UghNfbRw/runs/0/logs/https%3A%2F%2Ffirefox-ci-tc.services.mozilla.com%2Fapi%2Fqueue%2Fv1%2Ftask%2FAF7-of6tTL-_k_UghNfbRw%2Fruns%2F0%2Fartifacts%2Fpublic%2Flogs%2Fchain_of_trust.log#L649

These logs are not self-explanatory. Let me walk you through what happens:

1. A developer creates a pull request which kicks off a Taskcluster decision task.
2. That decision task spawns one or many legit scriptworker tasks.
3. The developer pushes some new commits their pull request. History hasn't been rewritten, meaning the tasks spawned in step 2 are still valid and must pass.
4. Later, the scriptworker tasks from step 2 are run. Chain of Trust wrongly assumes the only valid commit on that pull request is the head of the branch. Thus, there's a hash mismatch on MOBILE_HEAD_REV.

This wrong assumption happens at this line:

scriptworker/src/scriptworker/cot/verify.py

Line 1132 in 24b011b

pull_request_data = await github_repo.get_pull_request(pull_request_number)

I'm not sure what the best fix is. Maybe we should just reuse the hash baked in the decision task as MOBILE_HEAD_REV. Maybe we could do something cleverer with the Github v3 API. Although, I don't see what at the moment.

[escapewindow]

Maybe we should just reuse the hash baked in the decision task as MOBILE_HEAD_REV.

I'm ok with this for PRs, especially since we're marking PRs as non-trusted already.

[JohanLorenzo]

Great! It should be a good first bug, then!