feat: Add upload url validation before making requests (#701)

* feat: Validate service part of api request url

* fix: Add upload url validation to upload sender

Author: calvin-codecov

codecov-cli/codecov_cli/helpers/upload_url_validation.py

@@ -0,0 +1,21 @@
+import click
+
+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_service(
+    service: str,
+) -> None:
+    allowed = ", ".join(sorted(_CODECOV_UPLOAD_SERVICES))
+    if not service:
+        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:
+        raise click.ClickException(
+            f"Invalid upload service {service!r}. Use one of: {allowed}"
+        )

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_service
 
 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_service(service_part)
+    url = f"{upload_url}/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,8 @@
 
 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_upload_service
+
 from codecov_cli.helpers.request import (
     get_token_header,
     log_warnings_and_errors_if_any,
@@ -25,7 +27,9 @@ 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_service(service_part)
+    url = f"{upload_url}/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,7 @@
     request_result,
     send_post_request,
 )
+from codecov_cli.helpers.upload_url_validation import validate_upload_service
 
 logger = logging.getLogger("codecovcli")
 MAX_NUMBER_TRIES = 3
@@ -61,7 +62,9 @@ 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_service(service_part)
+    url = f"{upload_url}/upload/{service_part}/{encoded_slug}/commits/{commit_sha}/reports"
     return send_post_request(url=url, headers=headers, data=data)
 
 
@@ -106,7 +109,9 @@ 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_service(service_part)
+    url = f"{upload_url}/upload/{service_part}/{encoded_slug}/commits/{commit_sha}/reports/{report_code}/results"
     return send_post_request(url=url, data=data, headers=headers)
 
 
@@ -121,7 +126,9 @@ 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_service(service_part)
+    url = f"{upload_url}/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/upload_sender.py

@@ -12,6 +12,7 @@
 from codecov_cli.helpers.config import CODECOV_INGEST_URL
 from codecov_cli.helpers.encoder import encode_slug
 from codecov_cli.helpers.upload_type import ReportType
+from codecov_cli.helpers.upload_url_validation import validate_upload_service
 from codecov_cli.helpers.request import (
     get_token_header,
     send_post_request,
@@ -218,8 +219,11 @@ def get_url_and_possibly_update_data(
         upload_coverage=False,
         file_not_found=False,
     ):
+        service_part = (git_service or "").strip()
+
         if report_type == ReportType.COVERAGE:
-            base_url = f"{upload_url}/upload/{git_service}/{encoded_slug}"
+            validate_upload_service(service_part)
+            base_url = f"{upload_url}/upload/{service_part}/{encoded_slug}"
             if upload_coverage:
                 url = f"{base_url}/upload-coverage"
             else:
@@ -228,7 +232,7 @@ def get_url_and_possibly_update_data(
             data["slug"] = encoded_slug
             data["branch"] = branch
             data["commit"] = commit_sha
-            data["service"] = git_service
+            data["service"] = service_part if service_part else None
             data["file_not_found"] = file_not_found
             url = f"{upload_url}/upload/test_results/v1"
 

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

@@ -3,6 +3,7 @@
 
 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_upload_service
 from codecov_cli.helpers.request import (
     get_token_header,
     log_warnings_and_errors_if_any,
@@ -24,7 +25,9 @@ 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_service(service_part)
+    url = f"{upload_url}/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,33 @@ 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"),
+    ],
+)
+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())