Fix case-insensitive HTTP scheme bypass in isSecure()

[jsmid1]

Lowercase the URL before checking for insecure HTTP schemes, preventing mixed-case variants like Http:// or HTTP:// from bypassing the transport security check.

Ref: https://issues.redhat.com/browse/EC-2014

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 94afceb8-14c7-4716-83d9-ba2cb421d89b

📥 Commits

Reviewing files that changed from the base of the PR and between a915bcf and 70f42e7.

📒 Files selected for processing (2)

• internal/downloader/downloader.go
• internal/downloader/downloader_test.go

---

📝 Walkthrough

Walkthrough

The downloader now performs case-insensitive insecure URL checks. Tests cover mixed- and uppercase HTTP schemes, including nested git:: sources.

Changes

Secure URL validation

Layer / File(s)	Summary
Case-insensitive URL checks internal/downloader/downloader.go, internal/downloader/downloader_test.go	isSecure lowercases URLs before checking insecure HTTP patterns. Tests cover mixed- and uppercase schemes, including git::Http://....

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: st3penta, robnester-rh

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description explains what changed, why it changed, and links issue EC-2014.
Title check	✅ Passed	The title clearly identifies the case-insensitive HTTP scheme bypass fix in isSecure().
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-for-conforma]

PR Summary by Qodo

Fix mixed-case HTTP scheme bypass in downloader isSecure()

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Normalize URLs to lowercase before applying insecure transport checks.
• Block mixed-case "http:" variants that previously bypassed the prefix/regex checks.
• Extend isSecure() tests to cover case-insensitive HTTP scheme inputs.

Diagram

graph TD
  A["Downloader callers"] --> B["isSecure(url)"] --> C["strings.ToLower"] --> D["Insecure checks"] --> E["secure? bool"]
  D --> F["prefix: http:"]
  D --> G["regex: ::http:"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use case-insensitive regex / EqualFold checks

• ➕ Avoids allocating a fully lowercased copy of the URL for long inputs.
• ➕ Keeps logic localized to the specific comparisons.
• ➖ Harder to keep consistent across multiple checks (prefix + regexp).
• ➖ regexp changes (e.g., (?i)) can be less readable and easier to misconfigure.

2. Parse scheme with net/url where possible

• ➕ More semantically correct for standard URLs (scheme extraction vs string prefix).
• ➕ Naturally handles case-insensitive schemes per RFC expectations.
• ➖ Doesn’t directly cover non-standard patterns like "git::http://..." without additional parsing logic.
• ➖ More code/branching and potential behavior changes for edge-case inputs.

Recommendation: The PR’s approach (lowercasing once, then applying existing prefix/regex checks) is the best fit here: it’s minimal risk, fixes the bypass for both plain and VCS-prefixed URLs, and keeps behavior stable while closing the case-sensitivity gap.

Files changed (2) +6 / -1

Bug fix (1) +2 / -1

downloader.goLowercase URL before insecure scheme checks in isSecure() +2/-1

Lowercase URL before insecure scheme checks in isSecure()

• Adds a lowercase normalization step and performs both the "http:" prefix check and the insecure regexp match on the normalized value. This prevents mixed-case HTTP schemes from bypassing transport security validation.

internal/downloader/downloader.go

Tests (1) +4 / -0

downloader_test.goAdd mixed-case HTTP scheme coverage to TestIsSecure +4/-0

Add mixed-case HTTP scheme coverage to TestIsSecure

• Extends the insecure URL table to include multiple mixed-case "http://" variants and a mixed-case VCS-prefixed variant (git::Http://...). Ensures the security check remains case-insensitive across supported URL formats.

internal/downloader/downloader_test.go

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:40 AM UTC · Completed 11:48 AM UTC
Commit: 87c4a29 · View workflow run →

[qodo-for-conforma]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Avoidable URL lowercasing alloc ✗ Dismissed 🐞 Bug ➹ Performance

Description

isSecure() now lowercases the entire URL string, doing extra O(n) work and potentially allocating a
full-length copy for every Download() call even though only the scheme portion needs
case-insensitive checking. This is low severity but avoidable, especially since policy source URLs
can be long and may include query/userinfo components.

Code

internal/downloader/downloader.go[R154-155]

+	lower := strings.ToLower(url)
+	return !strings.HasPrefix(lower, "http:") && !insecure.MatchString(lower)

Relevance

●● Moderate

Micro-optimization; team sometimes accepts avoiding extra work, but no close precedent on URL
lowercasing changes.

PR-#3043

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR change introduces a new full-string normalization step in isSecure(). Download() invokes
isSecure() for every policy/data download, and test cases show URLs may include long components
(userinfo/query), so avoiding whole-string lowercasing reduces unnecessary per-call work.

internal/downloader/downloader.go[121-125]
internal/downloader/downloader.go[153-156]
internal/policy/source/source.go[178-184]
internal/downloader/downloader_test.go[119-157]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`isSecure()` lowercases the full URL (`strings.ToLower(url)`) to perform case-insensitive checks. This adds an extra full-string pass and may allocate a full-length copy of the URL even though the logic only needs case-insensitive matching for `http:` and `*::http:` prefixes.

### Issue Context
This runs on every `downloader.Download()` call before dispatching to gatherers.

### Fix Focus Areas
- internal/downloader/downloader.go[154-155]

### Suggested fix
- Avoid lowercasing the entire string.
- For the direct scheme check, use a bounded, case-insensitive prefix compare, e.g.:
 - `len(url) >= 5 && strings.EqualFold(url[:5], "http:")`
- For the `git::http:`-style check, make the regexp itself case-insensitive (e.g. `(?i)^[a-z0-9]*::http:`) and run it against the original `url`, or do a small manual case-insensitive check up to the `::http:` portion.
- Ensure existing tests for mixed-case `Http://` still pass.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context used

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

✅ Compliance rules (platform): 36 rules

✅ Skills: build-and-lint, pr-checklist, write-tests

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[fullsend-ai-review]

Looks good to me

---

Labels: PR fixes a case-insensitive HTTP scheme bypass — a security-related bug in transport security checks

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag	Coverage Δ
acceptance	54.40% <100.00%> (+<0.01%)	⬆️
generative	16.36% <0.00%> (-0.01%)	⬇️
integration	27.57% <0.00%> (-0.01%)	⬇️
unit	71.97% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
internal/downloader/downloader.go	95.83% <100.00%> (+0.08%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.