Hackweek: Add Gitea SCM support - #122201
Conversation
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
| # 5xx here would have Gitea - and our own webhookpayload retries - | ||
| # redeliver something that can never succeed. | ||
| extra["webhook.reason"] = GITEA_WEBHOOK_SECRET_MISSING_ERROR | ||
| logger.warning("gitea.webhook.missing-webhook-secret", extra=extra) |
| # malformed one is user error rather than an integration | ||
| # failure. | ||
| lifecycle.record_halt(e) | ||
| return Response({"detail": str(e)}, status=400) |
| return Response({"detail": str(e)}, status=400) | ||
| except ApiError as e: | ||
| lifecycle.record_failure(e) | ||
| return Response({"detail": str(e)}, status=400) |
| repos = installation.get_repositories(query=query) | ||
| except (ApiError, IntegrationError) as e: | ||
| lifecycle.record_failure(e) | ||
| return Response({"detail": str(e)}, status=400) |
Dispatches `GiteaProvider` from `fetch_service_provider` and derives the `web_base_url` it needs in `fetch_repository`. Together these are the last hop that lets Seer/autofix act on Gitea repositories: the integration, client, and repository provider already existed, but nothing told the SCM platform to wrap that client in an `scm` provider, so `fetch_service_provider` fell through to `None` and every call surfaced as `ProviderNotFound`. `web_base_url` comes from `metadata["base_url"]` rather than `instance` or `domain_name`. Gitea's ROOT_URL is free-form and may include a sub-path (`https://host/gitea/`), and the other two keys are hostname-only, so using them would silently drop it and point every API call at the wrong URL. A repository that somehow lacks the base URL returns `None` rather than letting the provider raise, matching how this module already handles unusable repository state. Requires sentry-scm with the Gitea provider. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01WVBi2Vg8XYgpADRiSZw1Lr
| def get_issue(self, repo_path: str, issue_index: str | int) -> Any: | ||
| """https://docs.gitea.com/api/1.22/#tag/issue/operation/issueGetIssue""" | ||
| return self.get(GiteaApiPath.issue.format(repo=repo_path, issue_index=issue_index)) | ||
|
|
||
| def create_issue_comment( | ||
| self, repo_path: str, issue_index: str | int, data: dict[str, Any] | ||
| ) -> Any: | ||
| """https://docs.gitea.com/api/1.22/#tag/issue/operation/issueCreateComment""" | ||
| return self.post( | ||
| GiteaApiPath.issue_comments.format(repo=repo_path, issue_index=issue_index), data=data |
There was a problem hiding this comment.
Unvalidated Gitea issue index bypasses the linked-repository boundary
externalIssue is interpolated into the Gitea issue URL without rejecting relative path segments, allowing an organization member to use a value such as ../../../other-owner/other-repo/issues/1 to read an issue from another repository accessible to the installation token, even though only the submitted repo is checked against the organization's linked repositories. Validate issue indexes as numeric values or reject dot segments before constructing the API path.
Evidence
- The authenticated link endpoint passes caller-controlled
request.datatoGiteaIssuesSpec.get_issue(), which validates onlydata["repo"];externalIssueis checked only for non-emptiness before being passed toGiteaApiClient.get_issue(). get_issue()formatsissue_indexdirectly into/repos/{repo}/issues/{issue_index}. A value such as../../../other-owner/other-repo/issues/1escapes the validated repository prefix and resolves to the other repository's issue route.- A normal Gitea issue response contains the fields consumed by
get_issue(), so the foreign issue's title, body, and URL are persisted in the Sentry external issue and returned to the requester. - The raw traversal is not used for the comment POST:
after_link_issue()reconstructs the index fromexternal_issue.key, which is generated from the returned issue number and the originally validated repository. The impact is therefore unauthorized reads, not arbitrary comment creation.
Identified by Warden · security-review · CCW-MW4
| def compare_commits( | ||
| self, repo: Any, start_sha: str | None, end_sha: str | ||
| ) -> Sequence[Mapping[str, Any]]: | ||
| installation = self.get_installation(repo.integration_id, repo.organization_id) | ||
| client = installation.get_client() | ||
| repo_path = repo.config["path"] | ||
|
|
||
| try: | ||
| if start_sha is None: | ||
| commits = client.get_commits(repo_path, sha=end_sha) | ||
| else: | ||
| # `compare/{basehead}` has been available since 1.22. | ||
| commits = client.compare_commits(repo_path, start_sha, end_sha)["commits"] or [] |
There was a problem hiding this comment.
Gitea compare path traversal can import private repository commits
An authenticated organization user can place traversal characters in previousCommit and query characters in the release commit, causing the Gitea client to escape the linked repository's compare route and use the installation token to compare another repository; returned private commit metadata is then imported into the organization's Sentry release.
Evidence
ReleaseHeadCommitSerializeronly length-checkspreviousCommit, whilecommitaccepts non-SHA characters such as?when it does not contain the range delimiter.release.set_refs()queuesfetch_commits, which passes the request-controlledpreviousCommitandcommitthroughresolve_ref()intoGiteaRepositoryProvider.compare_commits().GiteaApiClient.compare_commits()interpolates both values into/repos/{repo}/compare/{start_sha}...{end_sha}without encoding or relative-segment checks. For example, a start value such as../../../other-owner/other-repo/compare/baseand an end value such ashead?produces a normalized request for/repos/other-owner/other-repo/compare/base...head.- The installation's bearer token authenticates that request, and the resulting commits are formatted and passed to
release.set_commits(), allowing metadata from a private repository accessible to the Gitea installer to enter the attacker's Sentry organization.
Identified by Warden · security-review · AVD-TZQ
| def update_repo_data(self, repo: Repository, event: Mapping[str, Any]) -> None: | ||
| """ | ||
| Keep the stored URL and ``owner/name`` path current - renaming a repo or | ||
| moving the instance behind a new ``ROOT_URL`` changes both, and every | ||
| API call and stacktrace link is built from them. | ||
| """ | ||
| repository = event.get("repository") or {} | ||
| url_from_event = repository.get("html_url") | ||
| path_from_event = repository.get("full_name") | ||
|
|
||
| if not url_from_event or not path_from_event: | ||
| return | ||
|
|
||
| if repo.url != url_from_event or repo.config.get("path") != path_from_event: | ||
| repo.update( | ||
| url=url_from_event, | ||
| config=dict(repo.config, path=path_from_event), | ||
| ) |
There was a problem hiding this comment.
Webhook can poison repository path used in Gitea API routes
Signed push/PR webhooks write repository.full_name into repo.config["path"] with no is_repo_path check, so a path like x/../../user is later interpolated into /repos/{repo}/... and can escape repo scope using the installing user's token. Validate with is_repo_path (and preferably host-match html_url) before updating.
Evidence
update_repo_data()assigns webhookrepository.full_name/html_urldirectly ontorepo.config["path"]andrepo.urlwith no shape or host checks.PushEventWebhook/PullRequestEventWebhookcallupdate_repo_data()after HMAC verification; Gitea repo admins can read the hook secret and forge that signature (noted inGiteaApiClient.webhook_secret).- Downstream callers (
GiteaApiClient.repo_path,compare_commits, webhook create/delete, stacktrace file fetch) putrepo.config["path"]raw into/repos/{repo}/.... is_repo_path/has_relative_segmentsexist because Gitea resolves..out of repo scope (e.g. toward/api/v1/user), and are enforced on link/issue form input but not on this webhook write path.
Also found at 1 additional location
src/sentry/integrations/gitea/webhooks.py:202-202
Identified by Warden · security-review · 383-5FJ
| # exception, so the auth failure has to be spotted by hand. | ||
| return self._attempt_request_after_refreshing_token(*args, **kwargs) | ||
|
|
||
| self._track_rate_limit(response) |
There was a problem hiding this comment.
_track_rate_limit crashes on 204 responses from DELETE requests
_track_rate_limit unconditionally accesses response.headers, but the parent BaseApiClient returns {} for HTTP 204 responses, causing AttributeError when deleting webhooks or on any other 204-returning endpoint.
Evidence
BaseApiClient._requestreturns{}for any response with status code 204.delete_repo_webhookcallsself.delete()which routes through_issue_request_with_auto_token_refresh, falling through to_track_rate_limit(response)on success.get_rate_limit_info_from_responseaccessesresponse.headers, which plain dicts do not have, producingAttributeErrorat runtime.
Also found at 1 additional location
src/sentry/integrations/gitea/client.py:63-63
Identified by Warden · sentry-backend-bugs · 8ZQ-668
| # `/repos/{repo}` onto an unrelated route with the install user's token. | ||
| repo_path = config["identifier"] | ||
| if not isinstance(repo_path, str) or not is_repo_path(repo_path): |
There was a problem hiding this comment.
Missing identifier key causes 500 instead of validation error
config["identifier"] is accessed directly from the request body without checking existence. If the key is missing, KeyError becomes a 500 response via handle_api_error instead of the intended 400.
Evidence
- The
configparameter originates asrequest.datainIntegrationRepositoryProvider.dispatch(integration_repository.py:324). repo_path = config["identifier"]raisesKeyErrorbefore theisinstance/is_repo_pathvalidation on the next line.dispatchcatches the exception but itshandle_api_errorreturns a 500 for anything that is notIntegrationErrororIntegration.DoesNotExist.- Raising
IntegrationErrorhere would produce a 400, but theKeyErrorfires first and bypasses it.
Identified by Warden · sentry-backend-bugs · 9LJ-JV6
| "client_id": installation_data.get("client_id"), | ||
| "error_status": getattr(resp, "status_code"), # error might not be an HTTP error | ||
| "error_message": f"{e}", | ||
| }, |
There was a problem hiding this comment.
get_user_info assumes API response is always valid JSON
resp.json() is called outside the try/except block. A 200 response carrying HTML from a proxy or misconfigured Gitea instance will raise an unhandled JSONDecodeError instead of being logged and re-raised gracefully.
Evidence
get_user_info()makes an HTTP request to the customer's Gitea instance.resp.raise_for_status()is wrapped intry/except, butreturn resp.json()on line 65 is outside that block.- A proxy or misconfigured instance can return HTTP 200 with an HTML body, which passes
raise_for_status()and then crashes on.json(). - The caller
build_integration()inintegrations/gitea/integration.pydoes not catchJSONDecodeError, so the exception propagates unhandled. refresh_identity()in the same file already wrapsorjson.loads()in try/except for exactly this scenario, confirming the risk is real.
Identified by Warden · sentry-backend-bugs · KBE-DDP
|
|
||
| def get_file( | ||
| self, repo: Repository, path: str, ref: str | None, codeowners: bool = False | ||
| ) -> str: | ||
| """ | ||
| File contents. | ||
|
|
||
| ``/raw`` hands back the bytes directly, saving the base64 round trip | ||
| that ``/contents`` would require. | ||
| """ | ||
| contents = self.get( | ||
| GiteaApiPath.raw.format(repo=self.repo_path(repo), path=quote_path(path)), |
There was a problem hiding this comment.
get_file decodes error response bodies as file contents when raw_response=True bypasses raise_for_status
get_file returns the decoded body of a failed request as file contents because raw_response=True skips status-code checking and the method never verifies contents.ok.
Evidence
self.get(..., raw_response=True)returns the rawrequests.Responseobject without callingraise_for_status(), so 4xx/5xx responses are returned directly to the caller.contents.content.decode('utf-8')is executed unconditionally, turning an error payload (e.g. 404 HTML) into the return value.- The caller
SCMIntegration.get_codeowner_filewraps this call intry/except ApiErrorand expects an exception on failure; when Gitea silently returns error HTML the caller proceeds with corrupt CODEOWNERS data. - GitHub's client in the same codebase (
src/sentry/integrations/github/client.py:1105) explicitly checkscontents.okand raisesApiError.from_response(contents)before decoding when usingraw_response=True.
Identified by Warden · sentry-backend-bugs · T4Q-QKV
|
|
||
| config.update( | ||
| { | ||
| "instance": instance, | ||
| "path": repo["full_name"], | ||
| "name": repo["full_name"], | ||
| "external_id": installation.get_repo_external_id(repo), | ||
| "url": repo["html_url"], | ||
| } |
There was a problem hiding this comment.
Gitea API response fields accessed outside error-handling block
repo["full_name"], repo["html_url"], and repo["id"] (via get_repo_external_id) are read after the try/except that only wraps the network call. A successful but malformed API response raises an unhandled KeyError that returns 500.
Evidence
client.get_repo(repo_path)is the only call inside thetry/exceptblock.- The
config.update(...)block immediately after accessesrepo["full_name"]andrepo["html_url"]directly. installation.get_repo_external_id(repo)callsrepo["id"]internally.- If Gitea returns a 200 with an incomplete or unexpected payload, any missing key becomes an unhandled
KeyError. dispatchmaps unhandled exceptions to a 500 response.
Identified by Warden · sentry-backend-bugs · MPV-HHV
| provider_updated_at = parse_scm_timestamp(pull_request.get("updated_at")) | ||
|
|
||
| defaults = { | ||
| "title": title, | ||
| "author": author, | ||
| "message": pull_request.get("body") or "", | ||
| "merge_commit_sha": merge_commit_sha, | ||
| "head_commit_sha": (pull_request.get("head") or {}).get("sha"), | ||
| "date_added": parse_scm_timestamp(pull_request.get("created_at")), | ||
| "opened_at": parse_scm_timestamp(pull_request.get("created_at")), | ||
| "closed_at": parse_scm_timestamp(pull_request.get("closed_at")), | ||
| "merged_at": parse_scm_timestamp(pull_request.get("merged_at")), |
There was a problem hiding this comment.
Pull request webhook handler crashes on malformed timestamp strings
Raw parse_scm_timestamp calls on unvalidated Gitea webhook fields raise ValueError or TypeError for non-date strings, bypassing the existing IntegrityError guard and returning a 500 that triggers redelivery.
Evidence
parse_scm_timestampdelegates todateutil.parser.parse, which raisesValueErrororTypeErrorfor invalid or non-string input.- Lines 327 and 335-338 call it on raw
pull_request.get(...)values without any preceding validation. - The surrounding
try/except IntegrityErroronly guardsupdate_pull_request_from_scm_snapshot, so parser exceptions propagate uncaught. PushEventWebhookin the same file already wrapsparse_dateinexcept (KeyError, TypeError, ValueError)at line 226, confirming this risk is known for the push path.
Identified by Warden · sentry-backend-bugs · X57-MJX
| try: | ||
| data = orjson.loads(self.request.body) | ||
| except orjson.JSONDecodeError: | ||
| data = {} |
There was a problem hiding this comment.
Non-dict JSON body crashes mailbox routing with AttributeError
orjson.loads can return lists, strings, or null — valid JSON that is not an object. When this happens mailbox_bucket_id calls .get() on a non-dict and raises AttributeError, causing a 500 response to the webhook sender and triggering retries. The endpoint handler already guards against this, but the middleware parser was missed.
Evidence
get_response_from_gitea_webhookparsesself.request.bodywithorjson.loadson line 101 and only catchesJSONDecodeError; a bare list ([]), string, ornullpasses through unchanged.- The parsed
datais passed toget_mailbox_identifieron line 107, which invokes_build_bucketed_identifier→mailbox_bucket_id. mailbox_bucket_idon line 118 doesdata.get("repository"), which raisesAttributeErrorwhendatais not a dict.GiteaWebhookEndpoint.postin the same PR contains an identicalorjson.loadsand explicitly validatesisinstance(event, dict)with a comment noting that a bare list/string "blows up on the first.getinside a handler" and produces a 500.
Also found at 1 additional location
src/sentry/middleware/integrations/classifications.py:51
Identified by Warden · sentry-backend-bugs · WHM-JYR
| prepared_request.headers["Authorization"] = f"Bearer {self.token}" | ||
| return prepared_request | ||
|
|
||
| @control_silo_function |
There was a problem hiding this comment.
Gitea issue-linking GET traverses repository boundary via unvalidated externalIssue
GiteaApiClient.get_issue formats the caller-supplied issue_index directly into /repos/{repo}/issues/{issue_index} without validation, so a value such as ../../../other-owner/other-repo/issues/1 escapes the repository scoping enforced by _validated_repo and reads an arbitrary issue with the integration bearer token.
Evidence
GiteaIssuesSpec.get_issuevalidates therepoparameter via_validated_repo(shape check plus active-installation ownership), but only checks thatexternalIssueis non-empty before it passes the raw string toGiteaApiClient.get_issue.GiteaApiClient.get_issueat lines 330-332 interpolatesissue_indexstraight into the path templateGiteaApiPath.issuewith.format(). No validation, quoting, orhas_relative_segmentsguard is applied to the index.BaseApiClient._requestbuilds the full URL and hands it torequests, which transmits..path segments verbatim. Go’s HTTP server (and common proxies such as nginx) normalize the path, resolving the request to a different repository.- The installation bearer token is included in the request, so if the Gitea user can read the target repository, the cross-repo issue is returned to the Sentry caller.
Identified by Warden · wrdn-authz · ZQB-MSC
Support Gitea as an SCM provider.
Done during hackweek so most of this is built with vibes.
Successfully tested linking and issue creation against a test repo hosted on gitea.com. Still need to test against a self-hosted instance.
Everything is currently in this (frontend, backend, and static changes) which is why it's over 6k lines
Related to #15738