feat: Add upload url validation before making requests#120
feat: Add upload url validation before making requests#120calvin-codecov wants to merge 2 commits intomainfrom
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2fe07af. Configure here.
thomasrockhu-codecov
left a comment
thomasrockhu-codecov
left a comment
There was a problem hiding this comment.
I think I'm a little removed with what we actually require to validate, and I'd probably back off mostly to just the service instead of doing slug also. Just my 2 cents to perhaps prevent an issue downstream
|
|
||
| upload_url = enterprise_url or CODECOV_INGEST_URL | ||
| url = f"{upload_url}/upload/{service}/{slug}/commits" | ||
| service_part = (service or "").strip() |
There was a problem hiding this comment.
would service ever be empty? rather, is the "" necessary
There was a problem hiding this comment.
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
| upload_url: str, | ||
| service: str, | ||
| slug: typing.Optional[str], | ||
| default_base_url_for_error: str = CODECOV_INGEST_URL, |
There was a problem hiding this comment.
I'm not sure we need this param
| 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(): |
There was a problem hiding this comment.
I don't know if slug is always given to us. This isn't a sure thing, I'd double-check. Perhaps we get the slug after decoding the token
| f"Pass --git-service with one of: {allowed}" | ||
| ) | ||
| if service not in _CODECOV_UPLOAD_SERVICES: | ||
| allowed = ", ".join(sorted(_CODECOV_UPLOAD_SERVICES)) |
There was a problem hiding this comment.
Not sure we need to create allowed at this level, can probably just go above the if not service: line
calvin-codecov
force-pushed
the
cy/validate_none_service
branch
from
1033ba9 to
0c43195
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
thomasrockhu-codecov
left a comment
thomasrockhu-codecov
left a comment
There was a problem hiding this comment.
Make sure to test this locally first
2 participants
Closes https://github.com/codecov/core-engineering/issues/52
serviceurl part of create_commit, empty_upload, send_create_report, send_reports_result, send_reports_result_get, upload_completion requests