CCExtractor / CCExtractor/sample-platform
[BUG] Missing timeouts on 5 outbound HTTP calls outside mod_ci (follow-up to #943)
- Dominant language
- Python
- Stars
- 30
- Forks
- 85
- Avg merge
- 6d 21h
- Merged PRs (30d)
- 12
Description
Sample platform commit: **fe2b4a4**
**In raising this issue, I confirm the following (please check boxes, eg [X]):**
- [X] I have read and understood the [contributors guide](https://github.com/CCExtractor/sample-platform/blob/master/.github/CONTRIBUTING.md).
- [X] I have checked that the bug-fix I am reporting can be replicated, or that the feature I am suggesting isn't already present.
- [X] I have checked that the issue I'm posting isn't already reported.
- [X] I have checked that the issue I'm posting isn't already solved and no duplicates exist in [closed issues](https://github.com/CCExtractor/sample-platform/issues?q=is%3Aissue+is%3Aclosed) and in [opened issues](https://github.com/CCExtractor/sample-platform/issues)
- [X] I have checked the pull requests tab for existing solutions/implementations to my issue/suggestion.
**My familiarity with the project is as follows (check one, eg [X]):**
- [ ] I have never visited/used the platform.
- [X] I have used the platform just a couple of times.
- [ ] I have used the platform extensively, but have not contributed previously.
- [ ] I am an active contributor to the platform.
---
## Summary
#942 identified "GitHub API calls without timeouts" as a critical reliability gap, and #943 fixed it — but only within `mod_ci/controllers.py`. The audit table in #942 only enumerated call sites in that one file.
Five outbound HTTP calls in `utility.py`, `mod_auth/controllers.py`, `mailer.py` and `mod_upload/controllers.py` still have no `timeout=`. Since `requests` has **no default timeout**, each of these blocks its worker indefinitely if the remote end accepts the TCP connection but never responds.
## Affected call sites
| # | File | Call | Reached from |
|---|------|------|--------------|
| 1 | `utility.py:140` | `requests.get('https://api.github.com/meta', ...)` | **unauthenticated** `POST /start-ci` |
| 2 | `mod_auth/controllers.py:212` | `requests.post(...)` OAuth token exchange | `GET /account/github_callback` |
| 3 | `mod_auth/controllers.py:142` | `session.post(...)` token validity check | `github_token_validity()` |
| 4 | `mailer.py:45` | `requests.post(...)` Mailgun send | signup, password reset |
| 5 | `mod_upload/controllers.py:106` | `session.post(...)` create GitHub issue | upload flow |
## Why site 1 is the most serious
The outbound call happens **before any authentication**, because fetching the IP allowlist *is* the first check:
```
POST /start-ci <- no auth required to send this
└─ @request_from_github() utility.py:61
└─ is_github_web_hook_ip() utility.py:110 <- FIRST check
└─ get_cached_web_hook_blocks() utility.py:126
└─ requests.get(...) utility.py:140 <- no timeout
```
The IP allowlist cannot gate this call, because the call is what retrieves the allowlist.
There is also a cache-invalidation flaw that amplifies it. In `get_cached_web_hook_blocks()`, the `KeyError` branch logs the failure but never updates `cached_load_time`, and `cached_web_hook_blocks` stays empty. Since the guard is:
```python
if len(cached_web_hook_blocks) == 0 or cache_has_expired(cached_load_time):
```
...an unexpected payload from the GitHub meta API means the cache never populates and **every subsequent webhook request re-fetches**. Combined with the missing timeout, that turns one hung call into one hung worker per inbound request.
I'd rate the overall severity as moderate rather than critical: it requires GitHub's API to be hanging rather than merely down (a refused connection fails fast). It is primarily a resilience-under-upstream-failure problem, which is exactly the class #942 was opened to address.
## Reproduction
A listener that accepts the connection and never replies reproduces the hang. This is the specific failure mode that a `ConnectionError` handler does *not* catch:
```bash
python -c "import socket; s=socket.socket(); s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1); s.bind(('127.0.0.1', 9999)); s.listen(1); input()"
```
Point any of the five call sites at `http://127.0.0.1:9999` and the request never returns.
## The convention already exists in-tree
This is an inconsistency rather than an unknown. `mod_ci/controllers.py:55-63` defines named constants (`GITHUB_API_TIMEOUT`, `GCP_API_TIMEOUT`, `ARTIFACT_DOWNLOAD_TIMEOUT`), and `mod_auth/controllers.py:185` already does it correctly:
```python
response = session.get(url, timeout=(3.05, 10)) # line 185 - has a timeout
```
...43 lines below a call in the same file that does not:
```python
response = session.post(url, json={"access_token": token}) # line 142 - no timeout
```
## Proposed fix
Add `timeout=(3.05, 10)` at the five sites, matching the existing `(connect, read)` tuple form at `mod_auth/controllers.py:185`. No signatures, return types, or call sites change.
Worth noting explicitly: sites 1, 2, 3 and 5 have no exception handler, so a timeout converts a hang into a 500. That is still a strict improvement (a fast, logged failure beats a wedged worker), but adding handlers could be a sensible follow-up if you'd prefer that in the same PR.
Tests would assert the kwarg is actually passed, so the regression can't silently return:
```python
mock_get.assert_called_once_with(
'https://api.github.com/meta', auth=mock.ANY, timeout=(3.05, 10))
```
`tests/test_utility.py` already patches `requests.get`, so this extends cleanly.
## Out of scope, happy to file separately
While tracing the above I found that `is_valid_signature()` (`utility.py:152`) raises unhandled exceptions on a malformed `X-Hub-Signature` header, producing a 500 instead of a clean 418:
```
'garbage' -> ValueError: not enough values to unpack (expected 2, got 1)
'bogus=abc' -> TypeError: Missing required parameter 'digestmod'
'__name__=abc' -> ValueError: unsupported hash type hashlib
```
`hashlib.__dict__.get(hash_algorithm)` performs an attribute lookup on a caller-supplied string. Real-world impact is low, since it is only reachable after the GitHub IP check passes and GitHub always sends well-formed signatures. Let me know if you'd like that as its own issue.
---
I'd be glad to take this one if it's not already spoken for.
Contributor guide
Assessment
This issue has not been assessed yet.