CVE-2024-31455: Insufficient Information (NVD-CWE-noinfo)
value: Minder by Stacklok is an open source software supply chain security platform. A refactoring in commit `5c381cf` added the ability to get GitHub repositories registered to a project without specifying a specific provider.  Unfortunately, the SQL query for doing so was missing parenthesis, and would select a random repository. This issue is patched in pull request 2941. As a workaround, revert prior to `5c381cf`, or roll forward past `2eb94e7`.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-04-09T17:16Z
Repository: https://github.com/stacklok/minder
Language: Go
Files changed: database/query/repositories.sql, internal/db/repositories.sql.go, internal/db/repositories_test.go
Commit date: 2024-04-04 06:51:19 -0700
Commit message: Fix repository delete by name (#2941)

Fix #2938
Diff: diff --git a/database/query/repositories.sql b/database/query/repositories.sql
index 91f28dc1a..67daa13ef 100644
--- a/database/query/repositories.sql
+++ b/database/query/repositories.sql
@@ -20,11 +20,11 @@ INSERT INTO repositories (
 SELECT * FROM repositories WHERE repo_id = $1;
 
 -- name: GetRepositoryByRepoName :one
 SELECT * FROM repositories
     WHERE repo_owner = $1 AND repo_name = $2 AND project_id = $3
-    AND lower(provider) = lower(sqlc.narg('provider')::text) OR sqlc.narg('provider')::text IS NULL;
+    AND (lower(provider) = lower(sqlc.narg('provider')::text) OR sqlc.narg('provider')::text IS NULL);
 
 -- avoid using this, where possible use GetRepositoryByIDAndProject instead
 -- name: GetRepositoryByID :one
 SELECT * FROM repositories WHERE id = $1;
 
@@ -40,11 +40,11 @@ ORDER BY project_id, provider, repo_id
 LIMIT sqlc.narg('limit')::bigint;
 
 -- name: ListRegisteredRepositoriesByProjectIDAndProvider :many
 SELECT * FROM repositories
 WHERE project_id = $1 AND webhook_id IS NOT NULL
-    AND lower(provider) = lower(sqlc.narg('provider')::text) OR sqlc.narg('provider')::text IS NULL
+    AND (lower(provider) = lower(sqlc.narg('provider')::text) OR sqlc.narg('provider')::text IS NULL)
 ORDER BY repo_name;
 
 -- name: DeleteRepository :exec
 DELETE FROM repositories
 WHERE id = $1;
diff --git a/internal/db/repositories.sql.go b/internal/db/repositories.sql.go
index 703f2f2c7..f55c7caf6 100644
--- a/internal/db/repositories.sql.go
+++ b/internal/db/repositories.sql.go
@@ -203,11 +203,11 @@ func (q *Queries) GetRepositoryByRepoID(ctx context.Context, repoID int64) (Repo
 }
 
 const getRepositoryByRepoName = `-- name: GetRepositoryByRepoName :one
 SELECT id, provider, project_id, repo_owner, repo_name, repo_id, is_private, is_fork, webhook_id, webhook_url, deploy_url, clone_url, created_at, updated_at, default_branch, license, provider_id FROM repositories
     WHERE repo_owner = $1 AND repo_name = $2 AND project_id = $3
-    AND lower(provider) = lower($4::text) OR $4::text IS NULL
+    AND (lower(provider) = lower($4::text) OR $4::text IS NULL)
 `
 
 type GetRepositoryByRepoNameParams struct {
 	RepoOwner string         `json:"repo_owner"`
 	RepoName  string         `json:"repo_name"`
@@ -246,11 +246,11 @@ func (q *Queries) GetRepositoryByRepoName(ctx context.Context, arg GetRepository
 }
 
 const listRegisteredRepositoriesByProjectIDAndProvider = `-- name: ListRegisteredRepositoriesByProjectIDAndProvider :many
 SELECT id, provider, project_id, repo_owner, repo_name, repo_id, is_private, is_fork, webhook_id, webhook_url, deploy_url, clone_url, created_at, updated_at, default_branch, license, provider_id FROM repositories
 WHERE project_id = $1 AND webhook_id IS NOT NULL
-    AND lower(provider) = lower($2::text) OR $2::text IS NULL
+    AND (lower(provider) = lower($2::text) OR $2::text IS NULL)
 ORDER BY repo_name
 `
 
 type ListRegisteredRepositoriesByProjectIDAndProviderParams struct {
 	ProjectID uuid.UUID      `json:"project_id"`
diff --git a/internal/db/repositories_test.go b/internal/db/repositories_test.go
index c5c8464f4..147d0157e 100644
--- a/internal/db/repositories_test.go
+++ b/internal/db/repositories_test.go
@@ -154,10 +154,42 @@ func TestGetRepositoryByRepoName(t *testing.T) {
 	require.Equal(t, repo1.DeployUrl, repo2.DeployUrl)
 	require.Equal(t, repo1.CreatedAt, repo2.CreatedAt)
 	require.Equal(t, repo1.UpdatedAt, repo2.UpdatedAt)
 }
 
+func TestGetRepositoryByRepoNameNoProvider(t *testing.T) {
+	t.Parallel()
+
+	org := createRandomOrganization(t)
+	project := createRandomProject(t, org.ID)
+	prov := createRandomProvider(t, project.ID)
+	createRandomRepository(t, project.ID, prov)
+	repo1 := createRandomRepository(t, project.ID, prov)
+
+	repo2, err := testQueries.GetRepositoryByRepoName(context.Background(), GetRepositoryByRepoNameParams{
+		RepoOwner: repo1.RepoOwner,
+		RepoName:  repo1.RepoName,
+		ProjectID: project.ID,
+	})
+	require.NoError(t, err)
+	require.NotEmpty(t, repo2)
+
+	require.Equal(t, repo1.ID, repo2.ID)
+	require.Equal(t, repo1.Provider, repo2.Provider)
+	require.Equal(t, repo1.ProjectID, repo2.ProjectID)
+	require.Equal(t, repo1.RepoOwner, repo2.RepoOwner)
+	require.Equal(t, repo1.RepoName, repo2.RepoName)
+	require.Equal(t, repo1.RepoID, repo2.RepoID)
+	require.Equal(t, repo1.IsPrivate, repo2.IsPrivate)
+	require.Equal(t, repo1.IsFork, repo2.IsFork)
+	require.Equal(t, repo1.WebhookID, repo2.WebhookID)
+	require.Equal(t, repo1.WebhookUrl, repo2.WebhookUrl)
+	require.Equal(t, repo1.DeployUrl, repo2.DeployUrl)
+	require.Equal(t, repo1.CreatedAt, repo2.CreatedAt)
+	require.Equal(t, repo1.UpdatedAt, repo2.UpdatedAt)
+}
+
 func TestListRepositoriesByProjectID(t *testing.T) {
 	t.Parallel()
 
 	org := createRandomOrganization(t)
 	project := createRandomProject(t, org.ID)
@@ -186,10 +218,41 @@ func TestListRepositoriesByProjectID(t *testing.T) {
 		require.NotEmpty(t, repo)
 		require.Equal(t, arg.ProjectID, repo.ProjectID)
 	}
 }
 
+func TestListRepositoriesByProjectIDAndProvider(t *testing.T) {
+	t.Parallel()
+
+	org := createRandomOrganization(t)
+	otherProject := createRandomProject(t, org.ID)
+	project := createRandomProject(t, org.ID)
+	otherProv := createRandomProvider(t, otherProject.ID)
+	prov := createRandomProvider(t, project.ID)
+	createRandomRepository(t, project.ID, otherProv)
+	createRandomRepository(t, otherProject.ID, prov)
+
+	for i := 1001; i < 1020; i++ {
+		createRandomRepository(t, project.ID, prov, func(r *CreateRepositoryParams) {
+			r.RepoID = int64(i)
+		})
+	}
+
+	arg := ListRegisteredRepositoriesByProjectIDAndProviderParams{
+		ProjectID: project.ID,
+	}
+
+	repos, err := testQueries.ListRegisteredRepositoriesByProjectIDAndProvider(context.Background(), arg)
+	require.NoError(t, err)
+	require.NotEmpty(t, repos)
+
+	for _, repo := range repos {
+		require.NotEmpty(t, repo)
+		require.Equal(t, arg.ProjectID, repo.ProjectID)
+	}
+}
+
 func TestDeleteRepository(t *testing.T) {
 	t.Parallel()
 
 	org := createRandomOrganization(t)
 	project := createRandomProject(t, org.ID)

Vulnerable code:     AND lower(provider) = lower(sqlc.narg('provider')::text) OR sqlc.narg('provider')::text IS NULL;
    AND lower(provider) = lower(sqlc.narg('provider')::text) OR sqlc.narg('provider')::text IS NULL
    AND lower(provider) = lower($4::text) OR $4::text IS NULL
    AND lower(provider) = lower($2::text) OR $2::text IS NULL
Fixed code:     AND (lower(provider) = lower(sqlc.narg('provider')::text) OR sqlc.narg('provider')::text IS NULL);
    AND (lower(provider) = lower(sqlc.narg('provider')::text) OR sqlc.narg('provider')::text IS NULL)
    AND (lower(provider) = lower($4::text) OR $4::text IS NULL)
    AND (lower(provider) = lower($2::text) OR $2::text IS NULL)
func TestGetRepositoryByRepoNameNoProvider(t *testing.T) {
	t.Parallel()

	org := createRandomOrganization(t)
	project := createRandomProject(t, org.ID)
	prov := createRandomProvider(t, project.ID)
	createRandomRepository(t, project.ID, prov)
	repo1 := createRandomRepository(t, project.ID, prov)

	repo2, err := testQueries.GetRepositoryByRepoName(context.Background(), GetRepositoryByRepoNameParams{
		RepoOwner: repo1.RepoOwner,
		RepoName:  repo1.RepoName,
		ProjectID: project.ID,
	})
	require.NoError(t, err)
	require.NotEmpty(t, repo2)

	require.Equal(t, repo1.ID, repo2.ID)
	require.Equal(t, repo1.Provider, repo2.Provider)
	require.Equal(t, repo1.ProjectID, repo2.ProjectID)
	require.Equal(t, repo1.RepoOwner, repo2.RepoOwner)
	require.Equal(t, repo1.RepoName, repo2.RepoName)
	require.Equal(t, repo1.RepoID, repo2.RepoID)
	require.Equal(t, repo1.IsPrivate, repo2.IsPrivate)
	require.Equal(t, repo1.IsFork, repo2.IsFork)
	require.Equal(t, repo1.WebhookID, repo2.WebhookID)
	require.Equal(t, repo1.WebhookUrl, repo2.WebhookUrl)
	require.Equal(t, repo1.DeployUrl, repo2.DeployUrl)
	require.Equal(t, repo1.CreatedAt, repo2.CreatedAt)
	require.Equal(t, repo1.UpdatedAt, repo2.UpdatedAt)
}

func TestListRepositoriesByProjectIDAndProvider(t *testing.T) {
	t.Parallel()

	org := createRandomOrganization(t)
	otherProject := createRandomProject(t, org.ID)
	project := createRandomProject(t, org.ID)
	otherProv := createRandomProvider(t, otherProject.ID)
	prov := createRandomProvider(t, project.ID)
	createRandomRepository(t, project.ID, otherProv)
	createRandomRepository(t, otherProject.ID, prov)

	for i := 1001; i < 1020; i++ {
		createRandomRepository(t, project.ID, prov, func(r *CreateRepositoryParams) {
			r.RepoID = int64(i)
		})
	}

	arg := ListRegisteredRepositoriesByProjectIDAndProviderParams{
		ProjectID: project.ID,
	}

	repos, err := testQueries.ListRegisteredRepositoriesByProjectIDAndProvider(context.Background(), arg)
	require.NoError(t, err)
	require.NotEmpty(t, repos)

	for _, repo := range repos {
		require.NotEmpty(t, repo)
		require.Equal(t, arg.ProjectID, repo.ProjectID)
	}
}