Skip to content

feat: Add upload url validation before making requests #120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
calvin-codecov wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: Add upload url validation before making requests #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c43195
feat: Validate service part of api request url
calvin-codecov Apr 14, 2026
7d9b848
fix: Add upload url validation to upload sender
calvin-codecov Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions codecov-cli/codecov_cli/helpers/upload_url_validation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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}"
)
6 changes: 4 additions & 2 deletions codecov-cli/codecov_cli/services/commit/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -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()
Copy link
Copy Markdown
Collaborator

@thomasrockhu-codecov thomasrockhu-codecov Apr 14, 2026

Choose a reason for hiding this comment

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

would service ever be empty? rather, is the "" necessary

Copy link
Copy Markdown
Contributor Author

@calvin-codecov calvin-codecov Apr 14, 2026

Choose a reason for hiding this comment

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

service is typed as Optional string. If a CI adapter doesn't return anything for it as a fallback or if the user doesn't provide through the arguments, it will be none

validate_upload_service(service_part)
Comment thread
sentry[bot] marked this conversation as resolved.
url = f"{upload_url}/upload/{service_part}/{slug}/commits"
return send_post_request(
url=url,
data=data,
Expand Down
6 changes: 5 additions & 1 deletion codecov-cli/codecov_cli/services/empty_upload/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
13 changes: 10 additions & 3 deletions codecov-cli/codecov_cli/services/report/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)


Expand Down Expand Up @@ -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)


Expand All @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions codecov-cli/codecov_cli/services/upload/upload_sender.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand All @@ -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"

Expand Down
5 changes: 4 additions & 1 deletion codecov-cli/codecov_cli/services/upload_completion/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
}
Expand Down
41 changes: 35 additions & 6 deletions codecov-cli/tests/services/commit/test_commit_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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())
Expand All @@ -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,
)
Expand Down Expand Up @@ -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={},
)
Expand All @@ -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={},
)
Expand All @@ -112,7 +114,7 @@ def test_commit_sender_200(mocker):
"branch",
"owner::::repo",
token,
"service",
"github",
None,
None,
)
Expand All @@ -134,7 +136,7 @@ def test_commit_sender_403(mocker):
"branch",
"owner::::repo",
token,
"service",
"github",
None,
None,
)
Expand Down Expand Up @@ -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",
Expand Down
12 changes: 6 additions & 6 deletions codecov-cli/tests/services/empty_upload/test_empty_upload.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def test_empty_upload_with_warnings(mocker):
"commit_sha",
"owner/repo",
uuid.uuid4(),
"service",
"github",
None,
False,
False,
Expand Down Expand Up @@ -62,7 +62,7 @@ def test_empty_upload_with_error(mocker):
"commit_sha",
"owner/repo",
uuid.uuid4(),
"service",
"github",
None,
False,
False,
Expand Down Expand Up @@ -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 == [
Expand All @@ -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",
Expand All @@ -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 == [
Expand All @@ -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())
Expand Down
Loading
Loading