Skip to content

Feature/lookup enhancements - #32

Open
cognitivegears wants to merge 7 commits into
mainfrom
feature/lookup_enhancements
Open

Feature/lookup enhancements#32
cognitivegears wants to merge 7 commits into
mainfrom
feature/lookup_enhancements

Conversation

@cognitivegears

Copy link
Copy Markdown
Owner

Added lookup enhancements to improve the reliability of scans, especially when API keys not set

return None

# GitHub repositories are always /owner/repo; ignore any trailing resource paths.
if "github.com" in host:

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High

The string
github.com
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 6 months ago

In general, the problem should be fixed by comparing against the parsed hostname, not by substring search. For exact hosts like github.com and gitlab.com, use equality checks (==) on the fully lowercased host. If subdomains should be treated as the same host type, use an explicit suffix check that enforces a dot boundary (for example, host == "github.com" or host.endswith(".github.com")), not a raw substring.

In this file, there are two places using substring checks on hosts:

  • _create_repo_ref:
    if 'github.com' in host:
        host_type = 'github'
    elif 'gitlab.com' in host:
        host_type = 'gitlab'
  • _parse_http_repo_url:
    if "github.com" in host:
        ...
    if "gitlab.com" in host:
        ...

The safest, least-disruptive fix is to replace these with exact host comparisons. Because we don't know requirements for subdomains, the conservative assumption is that only the canonical domains github.com and gitlab.com should be treated as those host types. That preserves existing behavior for common inputs like https://github.com/owner/repo while avoiding misclassification of hosts merely containing those strings.

Concretely:

  • In _create_repo_ref, change the detection logic to:
    if host == "github.com":
        host_type = "github"
    elif host == "gitlab.com":
        host_type = "gitlab"
    else:
        host_type = "other"
  • In _parse_http_repo_url, change the host matching similarly:
    if host == "github.com":
        ...
    if host == "gitlab.com":
        ...

No new imports or helper methods are needed; we reuse the existing host.lower() normalization to ensure case-insensitive comparison.

Suggested changeset 1
src/repository/url_normalize.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/repository/url_normalize.py b/src/repository/url_normalize.py
--- a/src/repository/url_normalize.py
+++ b/src/repository/url_normalize.py
@@ -106,13 +106,13 @@
     if repo.endswith(".git"):
         repo = repo[:-4]
 
-    # Detect host type
-    if 'github.com' in host:
-        host_type = 'github'
-    elif 'gitlab.com' in host:
-        host_type = 'gitlab'
+    # Detect host type using exact host matching to avoid substring confusion
+    if host == "github.com":
+        host_type = "github"
+    elif host == "gitlab.com":
+        host_type = "gitlab"
     else:
-        host_type = 'other'
+        host_type = "other"
 
     # Construct normalized URL
     normalized_url = f'https://{host}/{owner}/{repo}'
@@ -138,11 +137,11 @@
         return None
 
     # GitHub repositories are always /owner/repo; ignore any trailing resource paths.
-    if "github.com" in host:
+    if host == "github.com":
         return host, segments[0], segments[1]
 
     # GitLab can have nested groups: /group/subgroup/repo
-    if "gitlab.com" in host:
+    if host == "gitlab.com":
         marker_index = _find_marker_index(segments, {
             "-", "blob", "tree", "issues", "merge_requests", "wikis",
             "commits", "tags", "releases",
EOF
@@ -106,13 +106,13 @@
if repo.endswith(".git"):
repo = repo[:-4]

# Detect host type
if 'github.com' in host:
host_type = 'github'
elif 'gitlab.com' in host:
host_type = 'gitlab'
# Detect host type using exact host matching to avoid substring confusion
if host == "github.com":
host_type = "github"
elif host == "gitlab.com":
host_type = "gitlab"
else:
host_type = 'other'
host_type = "other"

# Construct normalized URL
normalized_url = f'https://{host}/{owner}/{repo}'
@@ -138,11 +137,11 @@
return None

# GitHub repositories are always /owner/repo; ignore any trailing resource paths.
if "github.com" in host:
if host == "github.com":
return host, segments[0], segments[1]

# GitLab can have nested groups: /group/subgroup/repo
if "gitlab.com" in host:
if host == "gitlab.com":
marker_index = _find_marker_index(segments, {
"-", "blob", "tree", "issues", "merge_requests", "wikis",
"commits", "tags", "releases",
Copilot is powered by AI and may make mistakes. Always verify output.
return host, segments[0], segments[1]

# GitLab can have nested groups: /group/subgroup/repo
if "gitlab.com" in host:

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High

The string
gitlab.com
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 6 months ago

General fix: Instead of using substring tests ("gitlab.com" in host / "github.com" in host), perform precise host checks based on the parsed hostname. We should use parsed.hostname (which strips any port) and compare it exactly to expected hosts or, if subdomains are intended (e.g., sub.gitlab.com), use an explicit suffix check hostname == "gitlab.com" or hostname.endswith(".gitlab.com").

Best concrete fix here without changing existing functionality:
Within _parse_http_repo_url, derive a normalized hostname from parsed.hostname (lowercased), then:

  • Replace if "github.com" in host: with an exact check on hostname == "github.com".
  • Replace if "gitlab.com" in host: with a check that allows the main domain and any subdomain: if hostname == "gitlab.com" or hostname.endswith(".gitlab.com"):.

We keep returning host (the original parsed.netloc.lower()) from this function to avoid altering downstream expectations (e.g., preserving ports like gitlab.com:8443). No new imports are required; we only add a local hostname variable and adjust the conditions.

Specific changes in src/repository/url_normalize.py inside _parse_http_repo_url:

  • Right after host = parsed.netloc.lower(), add hostname = (parsed.hostname or "").lower().
  • Update the GitHub conditional to use hostname == "github.com".
  • Update the GitLab conditional to use hostname == "gitlab.com" or hostname.endswith(".gitlab.com").

Suggested changeset 1
src/repository/url_normalize.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/repository/url_normalize.py b/src/repository/url_normalize.py
--- a/src/repository/url_normalize.py
+++ b/src/repository/url_normalize.py
@@ -133,16 +133,17 @@
         return None
 
     host = parsed.netloc.lower()
+    hostname = (parsed.hostname or "").lower()
     segments = [seg for seg in (parsed.path or "").split("/") if seg]
     if len(segments) < 2:
         return None
 
     # GitHub repositories are always /owner/repo; ignore any trailing resource paths.
-    if "github.com" in host:
+    if hostname == "github.com":
         return host, segments[0], segments[1]
 
     # GitLab can have nested groups: /group/subgroup/repo
-    if "gitlab.com" in host:
+    if hostname == "gitlab.com" or hostname.endswith(".gitlab.com"):
         marker_index = _find_marker_index(segments, {
             "-", "blob", "tree", "issues", "merge_requests", "wikis",
             "commits", "tags", "releases",
EOF
@@ -133,16 +133,17 @@
return None

host = parsed.netloc.lower()
hostname = (parsed.hostname or "").lower()
segments = [seg for seg in (parsed.path or "").split("/") if seg]
if len(segments) < 2:
return None

# GitHub repositories are always /owner/repo; ignore any trailing resource paths.
if "github.com" in host:
if hostname == "github.com":
return host, segments[0], segments[1]

# GitLab can have nested groups: /group/subgroup/repo
if "gitlab.com" in host:
if hostname == "gitlab.com" or hostname.endswith(".gitlab.com"):
marker_index = _find_marker_index(segments, {
"-", "blob", "tree", "issues", "merge_requests", "wikis",
"commits", "tags", "releases",
Copilot is powered by AI and may make mistakes. Always verify output.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds comprehensive lookup enhancements to improve the reliability of dependency scans, especially when GitHub API keys are not configured. The PR introduces rate limit awareness, optimized API lookups, caching mechanisms, and fallback strategies across multiple registry types.

Changes:

  • Added GitHub rate limit configuration and proactive throttling to spread API requests evenly across reset windows
  • Implemented optimized tag/release lookup using exact endpoints before pagination fallback to save API calls
  • Added monorepo detection to distinguish "never tags" repos from "missing specific version" scenarios
  • Introduced fallback downloads API for npm packages missing npms.io data
  • Added comprehensive caching for RTD, POM files, NuGet service index, and registry metadata
  • Deprecated GPG signature checking for PyPI (deprecated by PyPI in May 2023) and added checksums as trust signal
  • Rebalanced heuristic weights to include weekly downloads (0.12) and adjusted other weights
  • Added session pooling for HTTP connections to improve performance

Reviewed changes

Copilot reviewed 44 out of 44 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_weekly_downloads.py Added tests for npm downloads API fallback when npms.io returns 0
tests/test_trust_signals.py Added tests for weighted boolean signal scoring
tests/test_trust_signal_extraction.py Updated tests for PyPI trust signal extraction with filename matching and checksums
tests/test_serialization_exports.py Updated tests for repo_present_in_registry default value (False)
tests/test_score_normalization.py Updated tests for version match scoring with has_any_version_artifacts
tests/test_rtd.py Added cache clearing in setup to avoid test leakage
tests/test_repo_url_normalize.py Added tests for GitHub blob/tree URL normalization and GitLab subgroup support
tests/test_pypi_repo_discovery.py Added tests for repo key recognition and cooldown-based enrichment skipping
tests/test_provider_validation_matching.py Added extensive tests for monorepo detection and optimized matching
tests/test_provider_adapters_new_metrics.py Added tests confirming get_releases no longer falls back to tags
tests/test_npm_repo_discovery.py Simplified version match assertions
tests/test_http_client_wrapped_unit.py Added session parameter to middleware call expectations
tests/test_heuristic_scoring_fixes.py NEW: Comprehensive tests for weekly downloads normalization and proactive throttling
tests/test_github_rate_limit_config.py NEW: Comprehensive tests for GitHub rate limit configuration and logging
tests/test_github_client.py Added tests for optimized release/tag lookups and case-insensitive header parsing
tests/test_gitlab_client.py Added tests for optimized lookups and Accept header
src/versioning/resolvers/*.py Added Accept headers to registry API calls
src/repository/url_normalize.py Enhanced URL parsing to salvage repos from blob/tree/issues URLs
src/repository/rtd.py Added caching for RTD API calls with test-aware invalidation
src/repository/provider_validation.py Added monorepo detection and optimized provider matching
src/repository/provider_adapters.py Removed fallback from releases to tags; added optimized match methods
src/repository/gitlab.py Added optimized release/tag lookup methods with exact endpoints
src/repository/github.py Added optimized lookups, logging for rate limits, and early-stop pagination
src/registry/pypi/enrich.py Added cooldown awareness and GitHub token warnings
src/registry/pypi/discovery.py Added "repo" to recognized project URL keys
src/registry/pypi/client.py Deprecated GPG signatures, added checksum signals, added caching
src/registry/nuget/client.py Added caching for service index and repository signature policy
src/registry/npm/client.py Added batching for npms.io mget and fallback downloads API
src/registry/maven/client.py Changed rows parameter from 20 to 1 (only need existence check)
src/registry/maven/discovery.py Added POM file caching
src/registry/depsdev/enrich.py Skip enrichment when license and repo already populated
src/depgate.py Apply GitHub overrides on startup
src/constants.py Added GitHub configuration constants and rebalanced heuristic weights
src/common/trust_signals.py Added weighted scoring support
src/common/http_rate_middleware.py Added proactive throttling for GitHub API
src/common/http_client.py Added session pooling and configurable rate limit behavior
src/cli_io.py Simplified repo_present_in_registry export logic
src/cli_config.py Added GitHub override application function
src/args.py Added --github-token and --github-on-rate-limit arguments
src/analysis/heuristics.py Added weekly downloads normalization and version match nuance
docs/depgate.example.yml Added comprehensive GitHub configuration examples
docs/configuration.md Added GitHub settings documentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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