Skip to content

fix: use proper URL parsing for GitHub API domain#1039

Merged
mergify[bot] merged 1 commit intopython-wheel-build:mainfrom
mprpic:fix/url-substring-sanitization
Apr 8, 2026
Merged

fix: use proper URL parsing for GitHub API domain#1039
mergify[bot] merged 1 commit intopython-wheel-build:mainfrom
mprpic:fix/url-substring-sanitization

Conversation

@mprpic
Copy link
Copy Markdown
Contributor

@mprpic mprpic commented Apr 8, 2026

Pull Request Description

What

Replace substring check with urlparse hostname comparison to prevent potential bypass via crafted URLs. Resolves CodeQL alert py/incomplete-url-substring-sanitization (CWE-20):

https://github.com/python-wheel-build/fromager/security/code-scanning/1

Why

Resolves https://github.com/python-wheel-build/fromager/security/code-scanning/1 while still being a minimal, unobtrusive change.

@mprpic mprpic requested a review from a team as a code owner April 8, 2026 19:40
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 8, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5d644470-e314-4f02-ab9b-06f1204ea59d

📥 Commits

Reviewing files that changed from the base of the PR and between 7c6324a and 22be37f.

📒 Files selected for processing (1)
  • src/fromager/http_retry.py
✅ Files skipped from review due to trivial changes (1)
  • src/fromager/http_retry.py

📝 Walkthrough

Walkthrough

Added an import from urllib.parse and changed GitHub rate-limit detection in RetryHTTPAdapter.send to parse the request URL and compare its hostname to "api.github.com" (using urlparse(request.url).hostname == "api.github.com") instead of using a substring check. This narrows which 403 responses are classified as GitHub rate limiting.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: use proper URL parsing for GitHub API domain' accurately and specifically describes the main change—replacing a substring check with urlparse hostname comparison for GitHub API validation.
Description check ✅ Passed The description is directly relevant, explaining the security fix (CodeQL alert py/incomplete-url-substring-sanitization) and referencing the specific issue being resolved.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@rd4398 rd4398 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@LalatenduMohanty
Copy link
Copy Markdown
Member

@mergify rebase

Replace substring check with urlparse hostname comparison to prevent
potential bypass via crafted URLs. Resolves CodeQL alert
py/incomplete-url-substring-sanitization (CWE-20):

https://github.com/python-wheel-build/fromager/security/code-scanning/1

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Martin Prpič <mprpic@redhat.com>
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Apr 8, 2026

Deprecation notice: This pull request comes from a fork and was rebased using bot_account impersonation. This capability will be removed on July 1, 2026. After this date, the rebase action will no longer be able to rebase fork pull requests with this configuration. Please switch to the update action/command to ensure compatibility going forward.

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Apr 8, 2026

rebase

✅ Branch has been successfully rebased

@LalatenduMohanty LalatenduMohanty force-pushed the fix/url-substring-sanitization branch from 7c6324a to 22be37f Compare April 8, 2026 21:49
@mergify mergify bot merged commit 3e7c564 into python-wheel-build:main Apr 8, 2026
39 checks passed
@mprpic
Copy link
Copy Markdown
Contributor Author

mprpic commented Apr 8, 2026

@mergify rebase

@LalatenduMohanty Why run this before every merge? (also it seems this behavior will be deprecated in a few months; see above)

@mprpic mprpic deleted the fix/url-substring-sanitization branch April 8, 2026 23:43
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 this pull request may close these issues.

3 participants