From f83588bae653a6ef9d4024e7ada1ed9c2e2e0f47 Mon Sep 17 00:00:00 2001 From: psrsingh Date: Sat, 16 May 2026 11:37:08 +0530 Subject: [PATCH 1/3] fix: validate github return url metadata Signed-off-by: psrsingh --- cla-backend-go/github/github_repository.go | 47 ++++++++++++++++++++-- cla-backend-go/v2/sign/service.go | 5 +++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/cla-backend-go/github/github_repository.go b/cla-backend-go/github/github_repository.go index 5aa87c365..776672e62 100644 --- a/cla-backend-go/github/github_repository.go +++ b/cla-backend-go/github/github_repository.go @@ -2597,26 +2597,67 @@ 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 || repo.Owner == nil || repo.Owner.Login == nil || repo.Name == nil { + err := fmt.Errorf("missing repository owner or name 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 || pullRequest.HTMLURL == nil || *pullRequest.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) diff --git a/cla-backend-go/v2/sign/service.go b/cla-backend-go/v2/sign/service.go index 238f26b6f..6eccfed38 100644 --- a/cla-backend-go/v2/sign/service.go +++ b/cla-backend-go/v2/sign/service.go @@ -1572,6 +1572,7 @@ func (s *service) getIndividualSignatureCallbackURLGitlab(ctx context.Context, u if found, ok := metadata["repository_id"].(string); ok { repositoryID = found } else { + err = errors.New("missing repository_id in metadata") log.WithFields(f).WithError(err).Warnf("unable to get repository ID for user: %s", userID) return "", err } @@ -1579,6 +1580,7 @@ func (s *service) getIndividualSignatureCallbackURLGitlab(ctx context.Context, u if found, ok := metadata["merge_request_id"].(string); ok { mergeRequestID = found } else { + err = errors.New("missing merge_request_id in metadata") log.WithFields(f).WithError(err).Warnf("unable to get merge request ID for user: %s", userID) return "", err } @@ -1599,6 +1601,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 } @@ -1631,6 +1634,7 @@ func (s *service) getIndividualSignatureCallbackURL(ctx context.Context, userID if found, ok := metadata["repository_id"].(string); ok { repositoryID = found } else { + err = errors.New("missing repository_id in metadata") log.WithFields(f).WithError(err).Warnf("unable to get repository ID for user: %s", userID) return "", err } @@ -1640,6 +1644,7 @@ func (s *service) getIndividualSignatureCallbackURL(ctx context.Context, userID if found, ok := metadata["pull_request_id"].(string); ok { pullRequestID = found } else { + err = errors.New("missing pull_request_id in metadata") log.WithFields(f).WithError(err).Warnf("unable to get pull request ID for user: %s", userID) return "", err } From b009a3a88f435981cd17a679e4c260b8fd327a58 Mon Sep 17 00:00:00 2001 From: psrsingh Date: Wed, 27 May 2026 14:25:56 +0530 Subject: [PATCH 2/3] fix: improve metadata validation error handling Signed-off-by: psrsingh --- cla-backend-go/v2/sign/service.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/cla-backend-go/v2/sign/service.go b/cla-backend-go/v2/sign/service.go index 6eccfed38..6a29223f6 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", @@ -1687,7 +1703,7 @@ func (s *service) getInstallationIDFromRepositoryID(ctx context.Context, reposit installationId = githubOrg.OrganizationInstallationID if installationId == 0 { - err = fmt.Errorf("installation ID missing for repository ID: %s", repositoryID) + err = fmt.Errorf("missing installation ID for repository ID in metadata: %s", repositoryID) log.WithFields(f).WithError(err).Warnf("unable to get installation ID for repository ID: %s", repositoryID) return 0, err } From 3a1b0e85b3f266ecdadd8307277f2d4f7b80bc8d Mon Sep 17 00:00:00 2001 From: psrsingh Date: Thu, 28 May 2026 17:54:03 +0530 Subject: [PATCH 3/3] fix(github): address review feedback Signed-off-by: psrsingh --- cla-backend-go/github/github_repository.go | 17 +++++++++----- cla-backend-go/v2/sign/service.go | 26 ++++++++-------------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/cla-backend-go/github/github_repository.go b/cla-backend-go/github/github_repository.go index 776672e62..2017160d4 100644 --- a/cla-backend-go/github/github_repository.go +++ b/cla-backend-go/github/github_repository.go @@ -2629,8 +2629,8 @@ func GetReturnURL(ctx context.Context, installationID, repositoryID int64, pullR log.WithFields(f).WithError(err).Warnf("unable to get repository by ID: %d", repositoryID) return "", err } - if repo == nil || repo.Owner == nil || repo.Owner.Login == nil || repo.Name == nil { - err := fmt.Errorf("missing repository owner or name for repository ID %d", repositoryID) + if repo == nil { + err := fmt.Errorf("missing repository for repository ID %d", repositoryID) log.WithFields(f).WithError(err).Warn("invalid repository metadata") return "", err } @@ -2653,13 +2653,20 @@ func GetReturnURL(ctx context.Context, installationID, repositoryID int64, pullR log.WithFields(f).WithError(err).Warnf("unable to get pull request by ID: %d", pullRequestID) return "", err } - if pullRequest == nil || pullRequest.HTMLURL == nil || *pullRequest.HTMLURL == "" { + 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 6a29223f6..f5b583d5e 100644 --- a/cla-backend-go/v2/sign/service.go +++ b/cla-backend-go/v2/sign/service.go @@ -1585,18 +1585,14 @@ func (s *service) getIndividualSignatureCallbackURLGitlab(ctx context.Context, u } } - if found, ok := metadata["repository_id"].(string); ok { - repositoryID = found - } else { - err = errors.New("missing repository_id in metadata") + 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 { - err = errors.New("missing merge_request_id in metadata") + 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 } @@ -1647,20 +1643,16 @@ func (s *service) getIndividualSignatureCallbackURL(ctx context.Context, userID } } - if found, ok := metadata["repository_id"].(string); ok { - repositoryID = found - } else { - err = errors.New("missing repository_id in metadata") + 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 { - err = errors.New("missing pull_request_id in metadata") + 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 } @@ -1703,7 +1695,7 @@ func (s *service) getInstallationIDFromRepositoryID(ctx context.Context, reposit installationId = githubOrg.OrganizationInstallationID if installationId == 0 { - err = fmt.Errorf("missing installation ID for repository ID in metadata: %s", repositoryID) + err = fmt.Errorf("installation ID missing for repository ID: %s", repositoryID) log.WithFields(f).WithError(err).Warnf("unable to get installation ID for repository ID: %s", repositoryID) return 0, err }