diff --git a/codecov-cli/codecov_cli/helpers/upload_url_validation.py b/codecov-cli/codecov_cli/helpers/upload_url_validation.py new file mode 100644 index 00000000..6940192c --- /dev/null +++ b/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." + ) diff --git a/codecov-cli/codecov_cli/services/commit/__init__.py b/codecov-cli/codecov_cli/services/commit/__init__.py index be6caf35..1209f007 100644 --- a/codecov-cli/codecov_cli/services/commit/__init__.py +++ b/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, diff --git a/codecov-cli/codecov_cli/services/empty_upload/__init__.py b/codecov-cli/codecov_cli/services/empty_upload/__init__.py index 587bb756..2bb85566 100644 --- a/codecov-cli/codecov_cli/services/empty_upload/__init__.py +++ b/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, diff --git a/codecov-cli/codecov_cli/services/report/__init__.py b/codecov-cli/codecov_cli/services/report/__init__.py index 757a7f47..91bd35fa 100644 --- a/codecov-cli/codecov_cli/services/report/__init__.py +++ b/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) diff --git a/codecov-cli/codecov_cli/services/upload_completion/__init__.py b/codecov-cli/codecov_cli/services/upload_completion/__init__.py index b595ba7f..d5e8b363 100644 --- a/codecov-cli/codecov_cli/services/upload_completion/__init__.py +++ b/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, } diff --git a/codecov-cli/tests/services/commit/test_commit_service.py b/codecov-cli/tests/services/commit/test_commit_service.py index c574b9c0..e40a46e7 100644 --- a/codecov-cli/tests/services/commit/test_commit_service.py +++ b/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", diff --git a/codecov-cli/tests/services/empty_upload/test_empty_upload.py b/codecov-cli/tests/services/empty_upload/test_empty_upload.py index 38933c6b..71c1052b 100644 --- a/codecov-cli/tests/services/empty_upload/test_empty_upload.py +++ b/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()) diff --git a/codecov-cli/tests/services/report/test_report_results.py b/codecov-cli/tests/services/report/test_report_results.py index 9d2cf0bb..50b97110 100644 --- a/codecov-cli/tests/services/report/test_report_results.py +++ b/codecov-cli/tests/services/report/test_report_results.py @@ -27,7 +27,7 @@ def test_report_results_command_with_warnings(mocker): res = create_report_results_logic( commit_sha="commit_sha", code="code", - service="service", + service="github", slug="owner/repo", token="token", enterprise_url=None, @@ -46,7 +46,7 @@ def test_report_results_command_with_warnings(mocker): args=None, commit_sha="commit_sha", report_code="code", - service="service", + service="github", encoded_slug="owner::::repo", token="token", enterprise_url=None, @@ -72,7 +72,7 @@ def test_report_results_command_with_error(mocker): res = create_report_results_logic( commit_sha="commit_sha", code="code", - service="service", + service="github", slug="owner/repo", token="token", enterprise_url=None, @@ -89,7 +89,7 @@ def test_report_results_command_with_error(mocker): args=None, commit_sha="commit_sha", report_code="code", - service="service", + service="github", encoded_slug="owner::::repo", token="token", enterprise_url=None, @@ -103,7 +103,7 @@ def test_report_results_request_200(mocker): ) token = uuid.uuid4() res = send_reports_result_request( - "commit_sha", "report_code", "encoded_slug", "service", token, None, None + "commit_sha", "report_code", "encoded_slug", "github", token, None, None ) assert res.error is None assert res.warnings == [] @@ -116,7 +116,7 @@ def test_report_results_request_no_token(mocker): return_value=mocker.MagicMock(status_code=200), ) res = send_reports_result_request( - "commit_sha", "report_code", "encoded_slug", "service", None, None, None + "commit_sha", "report_code", "encoded_slug", "github", None, None, None ) assert res.error is None assert res.warnings == [] @@ -130,7 +130,7 @@ def test_report_results_403(mocker): ) token = uuid.uuid4() res = send_reports_result_request( - "commit_sha", "report_code", "encoded_slug", "service", token, None, None + "commit_sha", "report_code", "encoded_slug", "github", token, None, None ) assert res.error == RequestError( code="HTTP Error 403", @@ -150,7 +150,7 @@ def test_get_report_results_200_completed(mocker, capsys): ) token = uuid.uuid4() res = send_reports_result_get_request( - "commit_sha", "report_code", "encoded_slug", "service", token, None + "commit_sha", "report_code", "encoded_slug", "github", token, None ) output = parse_outstreams_into_log_lines(capsys.readouterr().err) assert res.error is None @@ -171,7 +171,7 @@ def test_get_report_results_no_token(mocker, capsys): ), ) res = send_reports_result_get_request( - "commit_sha", "report_code", "encoded_slug", "service", None, None + "commit_sha", "report_code", "encoded_slug", "github", None, None ) assert res.error is None assert res.warnings == [] @@ -189,7 +189,7 @@ def test_get_report_results_200_pending(mocker, capsys): ) token = uuid.uuid4() res = send_reports_result_get_request( - "commit_sha", "report_code", "encoded_slug", "service", token, None + "commit_sha", "report_code", "encoded_slug", "github", token, None ) output = parse_outstreams_into_log_lines(capsys.readouterr().err) assert res.error is None @@ -207,7 +207,7 @@ def test_get_report_results_200_error(mocker, capsys): ) token = uuid.uuid4() res = send_reports_result_get_request( - "commit_sha", "report_code", "encoded_slug", "service", token, None + "commit_sha", "report_code", "encoded_slug", "github", token, None ) output = parse_outstreams_into_log_lines(capsys.readouterr().err) assert res.error is None @@ -228,7 +228,7 @@ def test_get_report_results_200_undefined_state(mocker, capsys): ) token = uuid.uuid4() res = send_reports_result_get_request( - "commit_sha", "report_code", "encoded_slug", "service", token, None + "commit_sha", "report_code", "encoded_slug", "github", token, None ) output = parse_outstreams_into_log_lines(capsys.readouterr().err) assert res.error is None @@ -246,7 +246,7 @@ def test_get_report_results_401(mocker, capsys): ) token = uuid.uuid4() res = send_reports_result_get_request( - "commit_sha", "report_code", "encoded_slug", "service", token, None + "commit_sha", "report_code", "encoded_slug", "github", token, None ) output = parse_outstreams_into_log_lines(capsys.readouterr().err) assert res.error == RequestError( diff --git a/codecov-cli/tests/services/report/test_report_service.py b/codecov-cli/tests/services/report/test_report_service.py index 0ac3b5fa..e302d00d 100644 --- a/codecov-cli/tests/services/report/test_report_service.py +++ b/codecov-cli/tests/services/report/test_report_service.py @@ -18,7 +18,7 @@ def test_send_create_report_request_200(mocker): "github", uuid.uuid4(), "owner::::repo", - "enterprise_url", + "https://enterprise.example.com", 1, None, ) @@ -38,7 +38,7 @@ def test_send_create_report_request_no_token(mocker): "github", None, "owner::::repo", - "enterprise_url", + "https://enterprise.example.com", 1, None, ) @@ -125,7 +125,7 @@ def test_create_report_command_with_error(mocker): service="github", token="token", pull_request_number=1, - enterprise_url="enterprise_url", + enterprise_url="https://enterprise.example.com", ) out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue()) @@ -149,7 +149,7 @@ def test_create_report_command_with_error(mocker): "github", "token", "owner::::repo", - "enterprise_url", + "https://enterprise.example.com", 1, None, ) diff --git a/codecov-cli/tests/services/upload_completion/test_upload_completion.py b/codecov-cli/tests/services/upload_completion/test_upload_completion.py index 40b0e4c8..ac69435f 100644 --- a/codecov-cli/tests/services/upload_completion/test_upload_completion.py +++ b/codecov-cli/tests/services/upload_completion/test_upload_completion.py @@ -23,7 +23,7 @@ def test_upload_completion_with_warnings(mocker): runner = CliRunner() with runner.isolation() as outstreams: res = upload_completion_logic( - "commit_sha", "owner/repo", uuid.uuid4(), "service", None + "commit_sha", "owner/repo", uuid.uuid4(), "github", None ) out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue()) assert out_bytes == [ @@ -52,7 +52,7 @@ def test_upload_completion_with_error(mocker): runner = CliRunner() with runner.isolation() as outstreams: res = upload_completion_logic( - "commit_sha", "owner/repo", uuid.uuid4(), "service", None + "commit_sha", "owner/repo", uuid.uuid4(), "github", None ) out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue()) assert out_bytes == [ @@ -80,7 +80,7 @@ def test_upload_completion_200(mocker): runner = CliRunner() with runner.isolation() as outstreams: res = upload_completion_logic( - "commit_sha", "owner/repo", token, "service", None + "commit_sha", "owner/repo", token, "github", None ) out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue()) assert out_bytes == [ @@ -110,7 +110,7 @@ def test_upload_completion_no_token(mocker): ) runner = CliRunner() with runner.isolation() as outstreams: - res = upload_completion_logic("commit_sha", "owner/repo", None, "service", None) + res = upload_completion_logic("commit_sha", "owner/repo", None, "github", None) out_bytes = parse_outstreams_into_log_lines(outstreams[0].getvalue()) assert out_bytes == [ ("info", "Upload Completion complete"), @@ -130,7 +130,7 @@ def test_upload_completion_403(mocker): return_value=mocker.MagicMock(status_code=403, text="Permission denied"), ) token = uuid.uuid4() - res = upload_completion_logic("commit_sha", "owner/repo", token, "service", None) + res = upload_completion_logic("commit_sha", "owner/repo", token, "github", None) assert res.error == RequestError( code="HTTP Error 403", description="Permission denied",