Security: No Timeout on HTTP Requests Leading to Potential Denial of Service#2103
Conversation
Multiple scripts make HTTP requests using `requests.get()` and `requests.post()` without specifying a `timeout` parameter. If the remote server is unresponsive, the script will block indefinitely, potentially causing resource exhaustion or hanging CI/CD pipelines. Affected files: Identify_Old_Issue_And_PR.py Signed-off-by: Trần Bách <[email protected]>
|
Approved, but please fix linter error |
There was a problem hiding this comment.
Pull request overview
Adds explicit HTTP timeouts to prevent the watchdog script from hanging indefinitely on unresponsive endpoints.
Changes:
- Add
timeout=30to the GitHub Issues APIrequests.get()call. - Add
timeout=30to the Slack webhookrequests.post()call.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| response = requests.post(sys.argv[1], headers=request_headers, data=message, timeout=30) | ||
| if response.status_code != 200: | ||
| print("Cannot send notification to slack: HTTP %s received!" % response.status_code) | ||
| sys.exit(2) No newline at end of file |
There was a problem hiding this comment.
requests.post(..., timeout=30) can raise requests.exceptions.Timeout/RequestException and currently would terminate the script with an unhandled exception. Consider catching requests.exceptions.RequestException around the Slack webhook call and exiting with code 2 while printing a concise error (including that a timeout occurred).
| # Grab the list of open Issues/PR | ||
| buffer = "Grab the list of open Issues/PR via the GitHub API...\n" | ||
| response = requests.get(ISSUE_API) | ||
| response = requests.get(ISSUE_API, timeout=30) |
There was a problem hiding this comment.
PR description mentions "multiple scripts" lacking request timeouts, but this change only updates Identify_Old_Issue_And_PR.py. For example, scripts/Generate_RSS_Feed.py and scripts/Generate_Technologies_JSON.py still call requests.get(...) without a timeout (as of current main). Either expand the fix set or narrow the PR description/scope accordingly.
| # Grab the list of open Issues/PR | ||
| buffer = "Grab the list of open Issues/PR via the GitHub API...\n" | ||
| response = requests.get(ISSUE_API) | ||
| response = requests.get(ISSUE_API, timeout=30) |
There was a problem hiding this comment.
requests.get(..., timeout=30) can raise requests.exceptions.Timeout/RequestException before status_code is available. Right now that would crash the script with a traceback. Wrap this call in a try/except requests.exceptions.RequestException and exit with code 1 (and a clear message) on network/timeout failures.
| response = requests.get(ISSUE_API, timeout=30) | |
| try: | |
| response = requests.get(ISSUE_API, timeout=30) | |
| except requests.exceptions.RequestException as exc: | |
| print("Cannot load the list of Issues/PR content: %s" % exc) | |
| sys.exit(1) |
Problem
Multiple scripts make HTTP requests using
requests.get()andrequests.post()without specifying atimeoutparameter. If the remote server is unresponsive, the script will block indefinitely, potentially causing resource exhaustion or hanging CI/CD pipelines.Severity:
lowFile:
scripts/Identify_Old_Issue_And_PR.pySolution
Add a
timeoutparameter to allrequests.get()andrequests.post()calls. For example:response = requests.get(ISSUE_API, timeout=30)Changes
scripts/Identify_Old_Issue_And_PR.py(modified)Testing