[results-processor] Provide GitHub token for api.github.com #4025
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -112,6 +112,14 @@ def known_extension(path): | |||
| return e | |||
| return None | |||
|
|
|||
| def _secret(self, token_name): | |||
| key = self.datastore.key('Token', token_name) | |||
There was a problem hiding this comment.
I think all paths that trigger this code are guarded by _internal_only. Which ensures that '/api/results/process' is only called by the task queue. So we are good. But any other code starts to use this in the future, we probably want to have some more control over reading this secret when hitting '/api/results/process'. Or at least some more logging to tell help.
The other thing that helps: We are only doing GET requests with the token, so the damage is limited.
There was a problem hiding this comment.
Figuring out how to pass that through to the processor (which doesn't inherently rely on Flask/main/etc. today) seems kinda difficult. We can certainly add logging, though?
There was a problem hiding this comment.
Can you add a follow up issue for this? We can address that part later.
Something around logging which uploader is accessing the secret.
After you add the issue and the lint errors, feel free to merge.
gsnedders
force-pushed
the
results-processor-github-api-token
branch
from
48512b3 to
f2b7494
Compare
Sometimes (e.g., from GitHub Actions), we might get passed a URL to process which on api.github.com, which requires a token to access. Thus, we should pull down our GitHub token and add that as a header.
gsnedders
force-pushed
the
results-processor-github-api-token
branch
from
f2b7494 to
ff7d947
Compare
|
I haven't had the time to investigate, but it looks like the web_components_test CI job is just constantly failing now? |
Filed #4046 for this; I'm just going to land this despite the failing check because it's pre-existing. |
|
https://staging.wpt.fyi/status seems to show everything getting stuck as WPTFYI_PROCESSING, though not showing up as empty any more. But probably time to go back to #4020 to analyse what's going on? |
Sometimes (e.g., from GitHub Actions), we might get passed a URL to process which on api.github.com, which requires a token to access. Thus, we should pull down our GitHub token and add that as a header.
See also #4020, which this hopefully fixes.