From 0595e1de27cfa9b6a45df6fc8eddacd578df963d Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Tue, 12 Nov 2024 18:06:10 +0530 Subject: [PATCH 1/3] sql injection fix --- pkg/externalLink/ExternalLinkIdentifierMappingRepository.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/externalLink/ExternalLinkIdentifierMappingRepository.go b/pkg/externalLink/ExternalLinkIdentifierMappingRepository.go index a3fd5ed3f1..84c00bd1a5 100644 --- a/pkg/externalLink/ExternalLinkIdentifierMappingRepository.go +++ b/pkg/externalLink/ExternalLinkIdentifierMappingRepository.go @@ -108,7 +108,7 @@ func (impl ExternalLinkIdentifierMappingRepositoryImpl) FindAllActiveByLinkIdent elim.id as mapping_id,elim.active,elim.type,elim.identifier,elim.env_id,elim.app_id,elim.cluster_id FROM external_link el LEFT JOIN external_link_identifier_mapping elim ON el.id = elim.external_link_id - WHERE el.active = true and elim.active = true and ( (elim.type = %d and elim.identifier = '%s' and elim.cluster_id = 0) or (elim.type = 0 and elim.app_id = 0 and elim.cluster_id = %d) + WHERE el.active = true and elim.active = true and ( (elim.type = ? and elim.identifier = ? and elim.cluster_id = 0) or (elim.type = 0 and elim.app_id = 0 and elim.cluster_id = ?) or (elim.type = -1) );` queryParams = append(queryParams, TypeMappings[linkIdentifier.Type], linkIdentifier.Identifier, clusterId) } From 8aeda4d3d6c77d88908060173e7e6a0a22c143fe Mon Sep 17 00:00:00 2001 From: ayu-devtron Date: Tue, 12 Nov 2024 19:29:16 +0530 Subject: [PATCH 2/3] more query fixes and error handling --- .../sql/repository/AppListingRepository.go | 22 ++++++++++++++----- .../repository/InstalledAppVersionHistory.go | 1 - pkg/cluster/EnvironmentService.go | 1 + .../repository/EnvironmentRepository.go | 3 +++ pkg/security/policyService.go | 4 ++++ 5 files changed, 25 insertions(+), 6 deletions(-) diff --git a/internal/sql/repository/AppListingRepository.go b/internal/sql/repository/AppListingRepository.go index 359d69f9a5..8759a43c3a 100644 --- a/internal/sql/repository/AppListingRepository.go +++ b/internal/sql/repository/AppListingRepository.go @@ -124,7 +124,11 @@ func (impl AppListingRepositoryImpl) FetchJobs(appIds []int, statuses []string, impl.Logger.Error(appsErr) return jobContainers, appsErr } - jobContainers = impl.extractEnvironmentNameFromId(jobContainers) + jobContainers, err := impl.extractEnvironmentNameFromId(jobContainers) + if err != nil { + impl.Logger.Errorw("Error in extractEnvironmentNameFromId", "jobContainers", jobContainers, "err", err) + return nil, err + } return jobContainers, nil } @@ -137,7 +141,11 @@ func (impl AppListingRepositoryImpl) FetchOverviewCiPipelines(jobId int) ([]*bea impl.Logger.Error(appsErr) return jobContainers, appsErr } - jobContainers = impl.extractEnvironmentNameFromId(jobContainers) + jobContainers, err := impl.extractEnvironmentNameFromId(jobContainers) + if err != nil { + impl.Logger.Errorw("Error in extractEnvironmentNameFromId", "jobContainers", jobContainers, "err", err) + return nil, err + } return jobContainers, nil } @@ -732,7 +740,7 @@ func (impl AppListingRepositoryImpl) FindAppCount(isProd bool) (int, error) { return count, nil } -func (impl AppListingRepositoryImpl) extractEnvironmentNameFromId(jobContainers []*bean.JobListingContainer) []*bean.JobListingContainer { +func (impl AppListingRepositoryImpl) extractEnvironmentNameFromId(jobContainers []*bean.JobListingContainer) ([]*bean.JobListingContainer, error) { var envIds []*int for _, job := range jobContainers { if job.EnvironmentId != 0 { @@ -742,7 +750,11 @@ func (impl AppListingRepositoryImpl) extractEnvironmentNameFromId(jobContainers envIds = append(envIds, &job.LastTriggeredEnvironmentId) } } - envs, _ := impl.environmentRepository.FindByIds(envIds) + envs, err := impl.environmentRepository.FindByIds(envIds) + if err != nil { + impl.Logger.Errorw("Error in getting environment", "envIds", envIds, "err", err) + return nil, err + } envIdNameMap := make(map[int]string) @@ -759,5 +771,5 @@ func (impl AppListingRepositoryImpl) extractEnvironmentNameFromId(jobContainers } } - return jobContainers + return jobContainers, nil } diff --git a/pkg/appStore/installedApp/repository/InstalledAppVersionHistory.go b/pkg/appStore/installedApp/repository/InstalledAppVersionHistory.go index 6c34605a25..7c067818c8 100644 --- a/pkg/appStore/installedApp/repository/InstalledAppVersionHistory.go +++ b/pkg/appStore/installedApp/repository/InstalledAppVersionHistory.go @@ -216,7 +216,6 @@ func (impl *InstalledAppVersionHistoryRepositoryImpl) UpdateDeploymentHistoryMes _, err := impl.dbConnection.Model((*InstalledAppVersionHistory)(nil)). Set("message = ?", message). Where("id = ?", installedAppVersionHistoryId). - Where("active = ?", true). Update() return err } diff --git a/pkg/cluster/EnvironmentService.go b/pkg/cluster/EnvironmentService.go index e114d2e154..f4b980ba50 100644 --- a/pkg/cluster/EnvironmentService.go +++ b/pkg/cluster/EnvironmentService.go @@ -472,6 +472,7 @@ func (impl EnvironmentServiceImpl) FindByIds(ids []*int) ([]*bean2.EnvironmentBe models, err := impl.environmentRepository.FindByIds(ids) if err != nil { impl.logger.Errorw("error in fetching environment", "err", err) + return nil, err } var beans []*bean2.EnvironmentBean for _, model := range models { diff --git a/pkg/cluster/repository/EnvironmentRepository.go b/pkg/cluster/repository/EnvironmentRepository.go index 15e806e2ca..aaea7b361f 100644 --- a/pkg/cluster/repository/EnvironmentRepository.go +++ b/pkg/cluster/repository/EnvironmentRepository.go @@ -325,6 +325,9 @@ func (repositoryImpl EnvironmentRepositoryImpl) FindByClusterId(clusterId int) ( func (repositoryImpl EnvironmentRepositoryImpl) FindByIds(ids []*int) ([]*Environment, error) { var apps []*Environment + if len(ids) == 0 { + return []*Environment{}, nil + } err := repositoryImpl.dbConnection.Model(&apps).Where("active = ?", true).Where("id in (?)", pg.In(ids)).Select() return apps, err } diff --git a/pkg/security/policyService.go b/pkg/security/policyService.go index e00fa0eacf..c0fc679b5e 100644 --- a/pkg/security/policyService.go +++ b/pkg/security/policyService.go @@ -621,6 +621,10 @@ func (impl *PolicyServiceImpl) GetPolicies(policyLevel securityBean.PolicyLevel, envId = append(envId, &pipeline.EnvironmentId) } envs, err := impl.environmentService.FindByIds(envId) + if err != nil { + impl.logger.Errorw("Error in fetching env", "envId", envId, "err", err) + return nil, err + } for _, env := range envs { cvePolicy, severityPolicy, err := impl.getPolicies(policyLevel, env.ClusterId, env.Id, appId) if err != nil { From 0d9204dc8a0d5a6ccf4632d5b489759ad0db6c39 Mon Sep 17 00:00:00 2001 From: ayu-devtron Date: Thu, 14 Nov 2024 11:59:08 +0530 Subject: [PATCH 3/3] handle deleted / non-existent environment --- pkg/pipeline/DeploymentPipelineConfigService.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/pipeline/DeploymentPipelineConfigService.go b/pkg/pipeline/DeploymentPipelineConfigService.go index 155687645f..cc4c4a39fa 100644 --- a/pkg/pipeline/DeploymentPipelineConfigService.go +++ b/pkg/pipeline/DeploymentPipelineConfigService.go @@ -237,6 +237,10 @@ func (impl *CdPipelineConfigServiceImpl) GetCdPipelineById(pipelineId int) (cdPi impl.logger.Errorw("error in fetching pipeline", "err", err) return cdPipeline, err } + if environment == nil || environment.Id == 0 { + impl.logger.Errorw("environment doesn't exists", "environmentId", dbPipeline.EnvironmentId) + return cdPipeline, err + } strategies, err := impl.pipelineConfigRepository.GetAllStrategyByPipelineId(dbPipeline.Id) if err != nil && errors.IsNotFound(err) { impl.logger.Errorw("error in fetching strategies", "err", err)