diff --git a/cla-backend-go/github/github_repository.go b/cla-backend-go/github/github_repository.go index 5aa87c365..2017160d4 100644 --- a/cla-backend-go/github/github_repository.go +++ b/cla-backend-go/github/github_repository.go @@ -2597,28 +2597,76 @@ func GetReturnURL(ctx context.Context, installationID, repositoryID int64, pullR "pullRequestID": pullRequestID, } - client, err := NewGithubAppClient(installationID) + if installationID <= 0 { + err := errors.New("invalid installation ID") + log.WithFields(f).WithError(err).Warn("invalid installation ID") + return "", err + } + if repositoryID <= 0 { + err := errors.New("invalid repository ID") + log.WithFields(f).WithError(err).Warn("invalid repository ID") + return "", err + } + if pullRequestID <= 0 { + err := errors.New("invalid pull request ID") + log.WithFields(f).WithError(err).Warn("invalid pull request ID") + return "", err + } + client, err := NewGithubAppClient(installationID) if err != nil { log.WithFields(f).WithError(err).Warn("unable to create Github client") return "", err } log.WithFields(f).Debugf("getting github repository by id: %d", repositoryID) - repo, _, err := client.Repositories.GetByID(ctx, repositoryID) + repo, resp, err := client.Repositories.GetByID(ctx, repositoryID) if err != nil { + if ok, wrapped := CheckAndWrapForKnownErrors(resp, err); ok { + log.WithFields(f).WithError(wrapped).Warnf("unable to get repository by ID: %d", repositoryID) + return "", wrapped + } log.WithFields(f).WithError(err).Warnf("unable to get repository by ID: %d", repositoryID) return "", err } + if repo == nil { + err := fmt.Errorf("missing repository for repository ID %d", repositoryID) + log.WithFields(f).WithError(err).Warn("invalid repository metadata") + return "", err + } + + owner := repo.GetOwner().GetLogin() + name := repo.GetName() + if owner == "" || name == "" { + err := fmt.Errorf("invalid repository owner/name for repository ID %d", repositoryID) + log.WithFields(f).WithError(err).Warn("invalid repository metadata") + return "", err + } log.WithFields(f).Debugf("getting pull request by id: %d", pullRequestID) - pullRequest, _, err := client.PullRequests.Get(ctx, *repo.Owner.Login, *repo.Name, pullRequestID) + pullRequest, resp, err := client.PullRequests.Get(ctx, owner, name, pullRequestID) if err != nil { + if ok, wrapped := CheckAndWrapForKnownErrors(resp, err); ok { + log.WithFields(f).WithError(wrapped).Warnf("unable to get pull request by ID: %d", pullRequestID) + return "", wrapped + } log.WithFields(f).WithError(err).Warnf("unable to get pull request by ID: %d", pullRequestID) return "", err } + if pullRequest == nil { + err := fmt.Errorf("missing pull request %d/%s/%s", pullRequestID, owner, name) + log.WithFields(f).WithError(err).Warn("invalid pull request metadata") + return "", err + } + + htmlURL := pullRequest.GetHTMLURL() + if htmlURL == "" { + err := fmt.Errorf("missing html url for pull request %d/%s/%s", pullRequestID, owner, name) + log.WithFields(f).WithError(err).Warn("invalid pull request metadata") + return "", err + } - log.WithFields(f).Debugf("returning pull request html url: %s", *pullRequest.HTMLURL) + log.WithFields(f).Debugf("returning pull request html url: %s", htmlURL) - return *pullRequest.HTMLURL, nil + return htmlURL, nil } diff --git a/cla-backend-go/v2/sign/service.go b/cla-backend-go/v2/sign/service.go index 238f26b6f..f5b583d5e 100644 --- a/cla-backend-go/v2/sign/service.go +++ b/cla-backend-go/v2/sign/service.go @@ -1550,6 +1550,22 @@ func getUserName(user *v1Models.User) string { return "" } +// metadataStringValue extracts a string value from metadata and validates it. +func metadataStringValue(metadata map[string]interface{}, key string) (string, error) { + if metadata == nil { + return "", fmt.Errorf("missing %s in metadata", key) + } + v, ok := metadata[key] + if !ok || v == nil { + return "", fmt.Errorf("missing %s in metadata", key) + } + s := strings.TrimSpace(fmt.Sprintf("%v", v)) + if s == "" || s == "" { + return "", fmt.Errorf("missing %s in metadata", key) + } + return s, nil +} + func (s *service) getIndividualSignatureCallbackURLGitlab(ctx context.Context, userID string, metadata map[string]interface{}) (string, error) { f := logrus.Fields{ "functionName": "sign.getIndividualSignatureCallbackURLGitlab", @@ -1569,16 +1585,14 @@ func (s *service) getIndividualSignatureCallbackURLGitlab(ctx context.Context, u } } - if found, ok := metadata["repository_id"].(string); ok { - repositoryID = found - } else { + repositoryID, err = metadataStringValue(metadata, "repository_id") + if err != nil { log.WithFields(f).WithError(err).Warnf("unable to get repository ID for user: %s", userID) return "", err } - if found, ok := metadata["merge_request_id"].(string); ok { - mergeRequestID = found - } else { + mergeRequestID, err = metadataStringValue(metadata, "merge_request_id") + if err != nil { log.WithFields(f).WithError(err).Warnf("unable to get merge request ID for user: %s", userID) return "", err } @@ -1599,6 +1613,7 @@ func (s *service) getIndividualSignatureCallbackURLGitlab(ctx context.Context, u } if gitlabOrg.OrganizationID == "" { + err = errors.New("missing organization ID for GitLab repository") log.WithFields(f).WithError(err).Warnf("unable to get organization ID for repository ID: %s", repositoryID) return "", err } @@ -1628,18 +1643,16 @@ func (s *service) getIndividualSignatureCallbackURL(ctx context.Context, userID } } - if found, ok := metadata["repository_id"].(string); ok { - repositoryID = found - } else { + repositoryID, err = metadataStringValue(metadata, "repository_id") + if err != nil { log.WithFields(f).WithError(err).Warnf("unable to get repository ID for user: %s", userID) return "", err } log.WithFields(f).Debugf("found repository ID: %s", repositoryID) - if found, ok := metadata["pull_request_id"].(string); ok { - pullRequestID = found - } else { + pullRequestID, err = metadataStringValue(metadata, "pull_request_id") + if err != nil { log.WithFields(f).WithError(err).Warnf("unable to get pull request ID for user: %s", userID) return "", err }