Skip to content

Commit 5dfc388

Browse files
misc: Sql query fixes (#6097)
* sql injection fix * more query fixes and error handling * handle deleted / non-existent environment --------- Co-authored-by: Prakash Kumar <prakash.kumar@devtron.ai>
1 parent cf1f6e6 commit 5dfc388

File tree

7 files changed

+30
-7
lines changed

7 files changed

+30
-7
lines changed

internal/sql/repository/AppListingRepository.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,11 @@ func (impl AppListingRepositoryImpl) FetchJobs(appIds []int, statuses []string,
124124
impl.Logger.Error(appsErr)
125125
return jobContainers, appsErr
126126
}
127-
jobContainers = impl.extractEnvironmentNameFromId(jobContainers)
127+
jobContainers, err := impl.extractEnvironmentNameFromId(jobContainers)
128+
if err != nil {
129+
impl.Logger.Errorw("Error in extractEnvironmentNameFromId", "jobContainers", jobContainers, "err", err)
130+
return nil, err
131+
}
128132
return jobContainers, nil
129133
}
130134

@@ -137,7 +141,11 @@ func (impl AppListingRepositoryImpl) FetchOverviewCiPipelines(jobId int) ([]*bea
137141
impl.Logger.Error(appsErr)
138142
return jobContainers, appsErr
139143
}
140-
jobContainers = impl.extractEnvironmentNameFromId(jobContainers)
144+
jobContainers, err := impl.extractEnvironmentNameFromId(jobContainers)
145+
if err != nil {
146+
impl.Logger.Errorw("Error in extractEnvironmentNameFromId", "jobContainers", jobContainers, "err", err)
147+
return nil, err
148+
}
141149
return jobContainers, nil
142150
}
143151

@@ -732,7 +740,7 @@ func (impl AppListingRepositoryImpl) FindAppCount(isProd bool) (int, error) {
732740
return count, nil
733741
}
734742

735-
func (impl AppListingRepositoryImpl) extractEnvironmentNameFromId(jobContainers []*bean.JobListingContainer) []*bean.JobListingContainer {
743+
func (impl AppListingRepositoryImpl) extractEnvironmentNameFromId(jobContainers []*bean.JobListingContainer) ([]*bean.JobListingContainer, error) {
736744
var envIds []*int
737745
for _, job := range jobContainers {
738746
if job.EnvironmentId != 0 {
@@ -742,7 +750,11 @@ func (impl AppListingRepositoryImpl) extractEnvironmentNameFromId(jobContainers
742750
envIds = append(envIds, &job.LastTriggeredEnvironmentId)
743751
}
744752
}
745-
envs, _ := impl.environmentRepository.FindByIds(envIds)
753+
envs, err := impl.environmentRepository.FindByIds(envIds)
754+
if err != nil {
755+
impl.Logger.Errorw("Error in getting environment", "envIds", envIds, "err", err)
756+
return nil, err
757+
}
746758

747759
envIdNameMap := make(map[int]string)
748760

@@ -759,5 +771,5 @@ func (impl AppListingRepositoryImpl) extractEnvironmentNameFromId(jobContainers
759771
}
760772
}
761773

762-
return jobContainers
774+
return jobContainers, nil
763775
}

pkg/appStore/installedApp/repository/InstalledAppVersionHistory.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@ func (impl *InstalledAppVersionHistoryRepositoryImpl) UpdateDeploymentHistoryMes
216216
_, err := impl.dbConnection.Model((*InstalledAppVersionHistory)(nil)).
217217
Set("message = ?", message).
218218
Where("id = ?", installedAppVersionHistoryId).
219-
Where("active = ?", true).
220219
Update()
221220
return err
222221
}

pkg/cluster/EnvironmentService.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,7 @@ func (impl EnvironmentServiceImpl) FindByIds(ids []*int) ([]*bean2.EnvironmentBe
472472
models, err := impl.environmentRepository.FindByIds(ids)
473473
if err != nil {
474474
impl.logger.Errorw("error in fetching environment", "err", err)
475+
return nil, err
475476
}
476477
var beans []*bean2.EnvironmentBean
477478
for _, model := range models {

pkg/cluster/repository/EnvironmentRepository.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,9 @@ func (repositoryImpl EnvironmentRepositoryImpl) FindByClusterId(clusterId int) (
325325

326326
func (repositoryImpl EnvironmentRepositoryImpl) FindByIds(ids []*int) ([]*Environment, error) {
327327
var apps []*Environment
328+
if len(ids) == 0 {
329+
return []*Environment{}, nil
330+
}
328331
err := repositoryImpl.dbConnection.Model(&apps).Where("active = ?", true).Where("id in (?)", pg.In(ids)).Select()
329332
return apps, err
330333
}

pkg/externalLink/ExternalLinkIdentifierMappingRepository.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func (impl ExternalLinkIdentifierMappingRepositoryImpl) FindAllActiveByLinkIdent
108108
elim.id as mapping_id,elim.active,elim.type,elim.identifier,elim.env_id,elim.app_id,elim.cluster_id
109109
FROM external_link el
110110
LEFT JOIN external_link_identifier_mapping elim ON el.id = elim.external_link_id
111-
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)
111+
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 = ?)
112112
or (elim.type = -1) );`
113113
queryParams = append(queryParams, TypeMappings[linkIdentifier.Type], linkIdentifier.Identifier, clusterId)
114114
}

pkg/pipeline/DeploymentPipelineConfigService.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,10 @@ func (impl *CdPipelineConfigServiceImpl) GetCdPipelineById(pipelineId int) (cdPi
237237
impl.logger.Errorw("error in fetching pipeline", "err", err)
238238
return cdPipeline, err
239239
}
240+
if environment == nil || environment.Id == 0 {
241+
impl.logger.Errorw("environment doesn't exists", "environmentId", dbPipeline.EnvironmentId)
242+
return cdPipeline, err
243+
}
240244
strategies, err := impl.pipelineConfigRepository.GetAllStrategyByPipelineId(dbPipeline.Id)
241245
if err != nil && errors.IsNotFound(err) {
242246
impl.logger.Errorw("error in fetching strategies", "err", err)

pkg/security/policyService.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,10 @@ func (impl *PolicyServiceImpl) GetPolicies(policyLevel securityBean.PolicyLevel,
621621
envId = append(envId, &pipeline.EnvironmentId)
622622
}
623623
envs, err := impl.environmentService.FindByIds(envId)
624+
if err != nil {
625+
impl.logger.Errorw("Error in fetching env", "envId", envId, "err", err)
626+
return nil, err
627+
}
624628
for _, env := range envs {
625629
cvePolicy, severityPolicy, err := impl.getPolicies(policyLevel, env.ClusterId, env.Id, appId)
626630
if err != nil {

0 commit comments

Comments
 (0)