feat: Add upload url validation before making requests

codecov-cli/codecov_cli/helpers/upload_url_validation.py

@@ -0,0 +1,54 @@
+import typing
+from urllib.parse import urlparse
+
+import click
+
+from codecov_cli.helpers.config import CODECOV_INGEST_URL
+from codecov_cli.helpers.git import GitService
+
+# The set of acceptable upload services from Shelter
+_CODECOV_UPLOAD_SERVICES = frozenset(s.value for s in GitService) | {"github-actions"}
+
+
+def validate_upload_url_base_parts(
+    upload_url: str,
+    service: str,
+    slug: typing.Optional[str],
+    default_base_url_for_error: str = CODECOV_INGEST_URL,
+) -> None:
+    parsed = urlparse(upload_url)
+    if parsed.scheme not in ("http", "https") or not parsed.netloc:
+        raise click.ClickException(
+            f"Invalid Codecov base URL {upload_url!r}. Use an absolute http(s) URL with a host "
+            f"(default: {default_base_url_for_error})."
+        )
+    if not slug or not str(slug).strip():
+        raise click.ClickException(
+            "Repository slug is missing or empty. Pass -r / --slug owner/repo (or set SLUG)."
+        )
+    if not service:
+        allowed = ", ".join(sorted(_CODECOV_UPLOAD_SERVICES))
+        raise click.ClickException(
+            "Upload service is missing (Codecov requires it for upload URLs). "
+            f"Pass --git-service with one of: {allowed}"
+        )
+    if service not in _CODECOV_UPLOAD_SERVICES:
+        allowed = ", ".join(sorted(_CODECOV_UPLOAD_SERVICES))
+        raise click.ClickException(
+            f"Invalid upload service {service!r}. Use one of: {allowed}"
+        )
+
+
+def validate_url_commit_sha(commit_sha: typing.Optional[str]) -> None:
+    if commit_sha is None or not str(commit_sha).strip():
+        raise click.ClickException(
+            "Commit SHA is missing or empty. Pass -C / --sha / --commit-sha."
+        )
+
+# This function includes validate_commit_sha check
+def validate_url_report_path_segments(commit_sha: typing.Optional[str], report_code: typing.Optional[str]) -> None:
+    validate_url_commit_sha(commit_sha)
+    if report_code is None or not str(report_code).strip():
+        raise click.ClickException(
+            "Report code is missing or empty. Pass --code / --report-code."
+        )

codecov-cli/codecov_cli/services/commit/__init__.py

@@ -9,10 +9,10 @@
     log_warnings_and_errors_if_any,
     send_post_request,
 )
+from codecov_cli.helpers.upload_url_validation import validate_upload_url_base_parts
 
 logger = logging.getLogger("codecovcli")
 
-
 def create_commit_logic(
     commit_sha: str,
     parent_sha: typing.Optional[str],
@@ -78,7 +78,9 @@ def send_commit_data(
     }
 
     upload_url = enterprise_url or CODECOV_INGEST_URL
-    url = f"{upload_url}/upload/{service}/{slug}/commits"
+    service_part = (service or "").strip()
+    validate_upload_url_base_parts(upload_url, service_part, slug, CODECOV_INGEST_URL)
+    url = f"{upload_url.rstrip('/')}/upload/{service_part}/{slug}/commits"
     return send_post_request(
         url=url,
         data=data,

codecov-cli/codecov_cli/services/empty_upload/__init__.py

@@ -3,6 +3,10 @@
 
 from codecov_cli.helpers.config import CODECOV_API_URL
 from codecov_cli.helpers.encoder import encode_slug
+from codecov_cli.helpers.upload_url_validation import (
+    validate_url_commit_sha,
+    validate_upload_url_base_parts,
+)
 from codecov_cli.helpers.request import (
     get_token_header,
     log_warnings_and_errors_if_any,
@@ -25,7 +29,12 @@ def empty_upload_logic(
     encoded_slug = encode_slug(slug)
     headers = get_token_header(token)
     upload_url = enterprise_url or CODECOV_API_URL
-    url = f"{upload_url}/upload/{git_service}/{encoded_slug}/commits/{commit_sha}/empty-upload"
+    service_part = (git_service or "").strip()
+    validate_upload_url_base_parts(
+        upload_url, service_part, encoded_slug, CODECOV_API_URL
+    )
+    validate_url_commit_sha(commit_sha)
+    url = f"{upload_url.rstrip('/')}/upload/{service_part}/{encoded_slug}/commits/{commit_sha}/empty-upload"
     sending_result = send_post_request(
         url=url,
         headers=headers,

codecov-cli/codecov_cli/services/report/__init__.py

@@ -14,6 +14,10 @@
     request_result,
     send_post_request,
 )
+from codecov_cli.helpers.upload_url_validation import (
+    validate_url_report_path_segments,
+    validate_upload_url_base_parts,
+)
 
 logger = logging.getLogger("codecovcli")
 MAX_NUMBER_TRIES = 3
@@ -61,7 +65,10 @@ def send_create_report_request(
     }
     headers = get_token_header(token)
     upload_url = enterprise_url or CODECOV_INGEST_URL
-    url = f"{upload_url}/upload/{service}/{encoded_slug}/commits/{commit_sha}/reports"
+    service_part = (service or "").strip()
+    validate_upload_url_base_parts(upload_url, service_part, encoded_slug, CODECOV_INGEST_URL)
+    validate_url_report_path_segments(commit_sha, code)
+    url = f"{upload_url.rstrip('/')}/upload/{service_part}/{encoded_slug}/commits/{commit_sha}/reports"
     return send_post_request(url=url, headers=headers, data=data)
 
 
@@ -106,7 +113,12 @@ def send_reports_result_request(
     }
     headers = get_token_header(token)
     upload_url = enterprise_url or CODECOV_API_URL
-    url = f"{upload_url}/upload/{service}/{encoded_slug}/commits/{commit_sha}/reports/{report_code}/results"
+    service_part = (service or "").strip()
+    validate_upload_url_base_parts(
+        upload_url, service_part, encoded_slug, CODECOV_API_URL
+    )
+    validate_url_report_path_segments(commit_sha, report_code)
+    url = f"{upload_url.rstrip('/')}/upload/{service_part}/{encoded_slug}/commits/{commit_sha}/reports/{report_code}/results"
     return send_post_request(url=url, data=data, headers=headers)
 
 
@@ -121,7 +133,12 @@ def send_reports_result_get_request(
 ):
     headers = get_token_header(token)
     upload_url = enterprise_url or CODECOV_API_URL
-    url = f"{upload_url}/upload/{service}/{encoded_slug}/commits/{commit_sha}/reports/{report_code}/results"
+    service_part = (service or "").strip()
+    validate_upload_url_base_parts(
+        upload_url, service_part, encoded_slug, CODECOV_API_URL
+    )
+    validate_url_report_path_segments(commit_sha, report_code)
+    url = f"{upload_url.rstrip('/')}/upload/{service_part}/{encoded_slug}/commits/{commit_sha}/reports/{report_code}/results"
     number_tries = 0
     while number_tries < MAX_NUMBER_TRIES:
         resp = request.get(url=url, headers=headers)

codecov-cli/codecov_cli/services/upload_completion/__init__.py

@@ -3,6 +3,10 @@
 
 from codecov_cli.helpers.config import CODECOV_API_URL
 from codecov_cli.helpers.encoder import encode_slug
+from codecov_cli.helpers.upload_url_validation import (
+    validate_url_commit_sha,
+    validate_upload_url_base_parts,
+)
 from codecov_cli.helpers.request import (
     get_token_header,
     log_warnings_and_errors_if_any,
@@ -24,7 +28,12 @@ def upload_completion_logic(
     encoded_slug = encode_slug(slug)
     headers = get_token_header(token)
     upload_url = enterprise_url or CODECOV_API_URL
-    url = f"{upload_url}/upload/{git_service}/{encoded_slug}/commits/{commit_sha}/upload-complete"
+    service_part = (git_service or "").strip()
+    validate_upload_url_base_parts(
+        upload_url, service_part, encoded_slug, CODECOV_API_URL
+    )
+    validate_url_commit_sha(commit_sha)
+    url = f"{upload_url.rstrip('/')}/upload/{service_part}/{encoded_slug}/commits/{commit_sha}/upload-complete"
     data = {
         "cli_args": args,
     }

codecov-cli/tests/services/commit/test_commit_service.py

@@ -1,5 +1,7 @@
 import uuid
 
+import click
+import pytest
 from click.testing import CliRunner
 
 from codecov_cli.services.commit import create_commit_logic, send_commit_data
@@ -26,7 +28,7 @@ def test_commit_command_with_warnings(mocker):
             branch="branch",
             slug="owner/repo",
             token="token",
-            service="service",
+            service="github",
         )
 
     out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue())
@@ -43,7 +45,7 @@ def test_commit_command_with_warnings(mocker):
         branch="branch",
         slug="owner::::repo",
         token="token",
-        service="service",
+        service="github",
         enterprise_url=None,
         args=None,
     )
@@ -72,7 +74,7 @@ def test_commit_command_with_error(mocker):
             branch="branch",
             slug="owner/repo",
             token="token",
-            service="service",
+            service="github",
             enterprise_url=None,
             args={},
         )
@@ -93,7 +95,7 @@ def test_commit_command_with_error(mocker):
         branch="branch",
         slug="owner::::repo",
         token="token",
-        service="service",
+        service="github",
         enterprise_url=None,
         args={},
     )
@@ -112,7 +114,7 @@ def test_commit_sender_200(mocker):
         "branch",
         "owner::::repo",
         token,
-        "service",
+        "github",
         None,
         None,
     )
@@ -134,7 +136,7 @@ def test_commit_sender_403(mocker):
         "branch",
         "owner::::repo",
         token,
-        "service",
+        "github",
         None,
         None,
     )
@@ -176,6 +178,36 @@ def test_commit_sender_with_forked_repo(mocker):
     )
 
 
+@pytest.mark.parametrize(
+    "service,slug,enterprise_url,fragment",
+    [
+        (None, "o::::r", None, "Upload service is missing"),
+        ("", "o::::r", None, "Upload service is missing"),
+        ("circleci", "o::::r", None, "Invalid upload service"),
+        ("github", "", None, "Repository slug is missing"),
+        ("github", "   ", None, "Repository slug is missing"),
+        ("github", "o::::r", "not-a-url", "Invalid Codecov base URL"),
+    ],
+)
+def test_commit_sender_rejects_invalid_url_parts(
+    mocker, service, slug, enterprise_url, fragment
+):
+    mocker.patch("codecov_cli.helpers.request.requests.post")
+    with pytest.raises(click.ClickException) as excinfo:
+        send_commit_data(
+            "commit_sha",
+            "parent_sha",
+            "pr",
+            "branch",
+            slug,
+            uuid.uuid4(),
+            service,
+            enterprise_url,
+            None,
+        )
+    assert fragment in str(excinfo.value)
+
+
 def test_commit_without_token(mocker):
     mocked_response = mocker.patch(
         "codecov_cli.services.commit.send_post_request",

codecov-cli/tests/services/empty_upload/test_empty_upload.py

@@ -26,7 +26,7 @@ def test_empty_upload_with_warnings(mocker):
             "commit_sha",
             "owner/repo",
             uuid.uuid4(),
-            "service",
+            "github",
             None,
             False,
             False,
@@ -62,7 +62,7 @@ def test_empty_upload_with_error(mocker):
             "commit_sha",
             "owner/repo",
             uuid.uuid4(),
-            "service",
+            "github",
             None,
             False,
             False,
@@ -93,7 +93,7 @@ def test_empty_upload_200(mocker):
     runner = CliRunner()
     with runner.isolation() as outstreams:
         res = empty_upload_logic(
-            "commit_sha", "owner/repo", token, "service", None, False, False, None
+            "commit_sha", "owner/repo", token, "github", None, False, False, None
         )
     out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue())
     assert out_bytes == [
@@ -113,7 +113,7 @@ def test_empty_upload_403(mocker):
     )
     token = uuid.uuid4()
     res = empty_upload_logic(
-        "commit_sha", "owner/repo", token, "service", None, False, False, None
+        "commit_sha", "owner/repo", token, "github", None, False, False, None
     )
     assert res.error == RequestError(
         code="HTTP Error 403",
@@ -138,7 +138,7 @@ def test_empty_upload_force(mocker):
     runner = CliRunner()
     with runner.isolation() as outstreams:
         res = empty_upload_logic(
-            "commit_sha", "owner/repo", token, "service", None, False, True, None
+            "commit_sha", "owner/repo", token, "github", None, False, True, None
         )
     out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue())
     assert out_bytes == [
@@ -165,7 +165,7 @@ def test_empty_upload_no_token(mocker):
     runner = CliRunner()
     with runner.isolation() as outstreams:
         res = empty_upload_logic(
-            "commit_sha", "owner/repo", None, "service", None, False, False, None
+            "commit_sha", "owner/repo", None, "github", None, False, False, None
         )
 
     out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue())