diff --git a/util/db/repository_secrets.go b/util/db/repository_secrets.go index f39a7658b87a5..a2cec6556ed11 100644 --- a/util/db/repository_secrets.go +++ b/util/db/repository_secrets.go @@ -34,9 +34,9 @@ func (s *secretsRepositoryBackend) CreateRepository(ctx context.Context, reposit }, } - s.repositoryToSecret(repository, repositorySecret) + updatedSecret := s.repositoryToSecret(repository, repositorySecret) - _, err := s.db.createSecret(ctx, repositorySecret) + _, err := s.db.createSecret(ctx, updatedSecret) if err != nil { if apierrors.IsAlreadyExists(err) { hasLabel, err := s.hasRepoTypeLabel(secName) @@ -142,9 +142,9 @@ func (s *secretsRepositoryBackend) UpdateRepository(ctx context.Context, reposit return nil, err } - s.repositoryToSecret(repository, repositorySecret) + updatedSecret := s.repositoryToSecret(repository, repositorySecret) - _, err = s.db.kubeclientset.CoreV1().Secrets(s.db.ns).Update(ctx, repositorySecret, metav1.UpdateOptions{}) + _, err = s.db.kubeclientset.CoreV1().Secrets(s.db.ns).Update(ctx, updatedSecret, metav1.UpdateOptions{}) if err != nil { return nil, err } @@ -187,9 +187,9 @@ func (s *secretsRepositoryBackend) CreateRepoCreds(ctx context.Context, repoCred }, } - repoCredsToSecret(repoCreds, repoCredsSecret) + updatedSecret := repoCredsToSecret(repoCreds, repoCredsSecret) - _, err := s.db.createSecret(ctx, repoCredsSecret) + _, err := s.db.createSecret(ctx, updatedSecret) if err != nil { if apierrors.IsAlreadyExists(err) { return nil, status.Errorf(codes.AlreadyExists, "repository credentials %q already exists", repoCreds.URL) @@ -237,9 +237,9 @@ func (s *secretsRepositoryBackend) UpdateRepoCreds(ctx context.Context, repoCred return nil, err } - repoCredsToSecret(repoCreds, repoCredsSecret) + updatedSecret := repoCredsToSecret(repoCreds, repoCredsSecret) - repoCredsSecret, err = s.db.kubeclientset.CoreV1().Secrets(s.db.ns).Update(ctx, repoCredsSecret, metav1.UpdateOptions{}) + repoCredsSecret, err = s.db.kubeclientset.CoreV1().Secrets(s.db.ns).Update(ctx, updatedSecret, metav1.UpdateOptions{}) if err != nil { return nil, err } @@ -323,73 +323,75 @@ func (s *secretsRepositoryBackend) GetAllOCIRepoCreds(_ context.Context) ([]*app } func secretToRepository(secret *corev1.Secret) (*appsv1.Repository, error) { + secretCopy := secret.DeepCopy() + repository := &appsv1.Repository{ - Name: string(secret.Data["name"]), - Repo: string(secret.Data["url"]), - Username: string(secret.Data["username"]), - Password: string(secret.Data["password"]), - BearerToken: string(secret.Data["bearerToken"]), - SSHPrivateKey: string(secret.Data["sshPrivateKey"]), - TLSClientCertData: string(secret.Data["tlsClientCertData"]), - TLSClientCertKey: string(secret.Data["tlsClientCertKey"]), - Type: string(secret.Data["type"]), - GithubAppPrivateKey: string(secret.Data["githubAppPrivateKey"]), - GitHubAppEnterpriseBaseURL: string(secret.Data["githubAppEnterpriseBaseUrl"]), - Proxy: string(secret.Data["proxy"]), - NoProxy: string(secret.Data["noProxy"]), - Project: string(secret.Data["project"]), - GCPServiceAccountKey: string(secret.Data["gcpServiceAccountKey"]), - } - - insecureIgnoreHostKey, err := boolOrFalse(secret, "insecureIgnoreHostKey") + Name: string(secretCopy.Data["name"]), + Repo: string(secretCopy.Data["url"]), + Username: string(secretCopy.Data["username"]), + Password: string(secretCopy.Data["password"]), + BearerToken: string(secretCopy.Data["bearerToken"]), + SSHPrivateKey: string(secretCopy.Data["sshPrivateKey"]), + TLSClientCertData: string(secretCopy.Data["tlsClientCertData"]), + TLSClientCertKey: string(secretCopy.Data["tlsClientCertKey"]), + Type: string(secretCopy.Data["type"]), + GithubAppPrivateKey: string(secretCopy.Data["githubAppPrivateKey"]), + GitHubAppEnterpriseBaseURL: string(secretCopy.Data["githubAppEnterpriseBaseUrl"]), + Proxy: string(secretCopy.Data["proxy"]), + NoProxy: string(secretCopy.Data["noProxy"]), + Project: string(secretCopy.Data["project"]), + GCPServiceAccountKey: string(secretCopy.Data["gcpServiceAccountKey"]), + } + + insecureIgnoreHostKey, err := boolOrFalse(secretCopy, "insecureIgnoreHostKey") if err != nil { return repository, err } repository.InsecureIgnoreHostKey = insecureIgnoreHostKey - insecure, err := boolOrFalse(secret, "insecure") + insecure, err := boolOrFalse(secretCopy, "insecure") if err != nil { return repository, err } repository.Insecure = insecure - enableLfs, err := boolOrFalse(secret, "enableLfs") + enableLfs, err := boolOrFalse(secretCopy, "enableLfs") if err != nil { return repository, err } repository.EnableLFS = enableLfs - enableOCI, err := boolOrFalse(secret, "enableOCI") + enableOCI, err := boolOrFalse(secretCopy, "enableOCI") if err != nil { return repository, err } repository.EnableOCI = enableOCI - insecureOCIForceHTTP, err := boolOrFalse(secret, "insecureOCIForceHttp") + insecureOCIForceHTTP, err := boolOrFalse(secretCopy, "insecureOCIForceHttp") if err != nil { return repository, err } repository.InsecureOCIForceHttp = insecureOCIForceHTTP - githubAppID, err := intOrZero(secret, "githubAppID") + githubAppID, err := intOrZero(secretCopy, "githubAppID") if err != nil { return repository, err } repository.GithubAppId = githubAppID - githubAppInstallationID, err := intOrZero(secret, "githubAppInstallationID") + githubAppInstallationID, err := intOrZero(secretCopy, "githubAppInstallationID") if err != nil { return repository, err } repository.GithubAppInstallationId = githubAppInstallationID - forceBasicAuth, err := boolOrFalse(secret, "forceHttpBasicAuth") + forceBasicAuth, err := boolOrFalse(secretCopy, "forceHttpBasicAuth") if err != nil { return repository, err } repository.ForceHttpBasicAuth = forceBasicAuth - useAzureWorkloadIdentity, err := boolOrFalse(secret, "useAzureWorkloadIdentity") + useAzureWorkloadIdentity, err := boolOrFalse(secretCopy, "useAzureWorkloadIdentity") if err != nil { return repository, err } @@ -398,86 +400,92 @@ func secretToRepository(secret *corev1.Secret) (*appsv1.Repository, error) { return repository, nil } -func (s *secretsRepositoryBackend) repositoryToSecret(repository *appsv1.Repository, secret *corev1.Secret) { - if secret.Data == nil { - secret.Data = make(map[string][]byte) - } - - updateSecretString(secret, "name", repository.Name) - updateSecretString(secret, "project", repository.Project) - updateSecretString(secret, "url", repository.Repo) - updateSecretString(secret, "username", repository.Username) - updateSecretString(secret, "password", repository.Password) - updateSecretString(secret, "bearerToken", repository.BearerToken) - updateSecretString(secret, "sshPrivateKey", repository.SSHPrivateKey) - updateSecretBool(secret, "enableOCI", repository.EnableOCI) - updateSecretBool(secret, "insecureOCIForceHttp", repository.InsecureOCIForceHttp) - updateSecretString(secret, "tlsClientCertData", repository.TLSClientCertData) - updateSecretString(secret, "tlsClientCertKey", repository.TLSClientCertKey) - updateSecretString(secret, "type", repository.Type) - updateSecretString(secret, "githubAppPrivateKey", repository.GithubAppPrivateKey) - updateSecretInt(secret, "githubAppID", repository.GithubAppId) - updateSecretInt(secret, "githubAppInstallationID", repository.GithubAppInstallationId) - updateSecretString(secret, "githubAppEnterpriseBaseUrl", repository.GitHubAppEnterpriseBaseURL) - updateSecretBool(secret, "insecureIgnoreHostKey", repository.InsecureIgnoreHostKey) - updateSecretBool(secret, "insecure", repository.Insecure) - updateSecretBool(secret, "enableLfs", repository.EnableLFS) - updateSecretString(secret, "proxy", repository.Proxy) - updateSecretString(secret, "noProxy", repository.NoProxy) - updateSecretString(secret, "gcpServiceAccountKey", repository.GCPServiceAccountKey) - updateSecretBool(secret, "forceHttpBasicAuth", repository.ForceHttpBasicAuth) - updateSecretBool(secret, "useAzureWorkloadIdentity", repository.UseAzureWorkloadIdentity) - addSecretMetadata(secret, s.getSecretType()) +func (s *secretsRepositoryBackend) repositoryToSecret(repository *appsv1.Repository, secret *corev1.Secret) *corev1.Secret { + secretCopy := secret.DeepCopy() + + if secretCopy.Data == nil { + secretCopy.Data = make(map[string][]byte) + } + + updateSecretString(secretCopy, "name", repository.Name) + updateSecretString(secretCopy, "project", repository.Project) + updateSecretString(secretCopy, "url", repository.Repo) + updateSecretString(secretCopy, "username", repository.Username) + updateSecretString(secretCopy, "password", repository.Password) + updateSecretString(secretCopy, "bearerToken", repository.BearerToken) + updateSecretString(secretCopy, "sshPrivateKey", repository.SSHPrivateKey) + updateSecretBool(secretCopy, "enableOCI", repository.EnableOCI) + updateSecretBool(secretCopy, "insecureOCIForceHttp", repository.InsecureOCIForceHttp) + updateSecretString(secretCopy, "tlsClientCertData", repository.TLSClientCertData) + updateSecretString(secretCopy, "tlsClientCertKey", repository.TLSClientCertKey) + updateSecretString(secretCopy, "type", repository.Type) + updateSecretString(secretCopy, "githubAppPrivateKey", repository.GithubAppPrivateKey) + updateSecretInt(secretCopy, "githubAppID", repository.GithubAppId) + updateSecretInt(secretCopy, "githubAppInstallationID", repository.GithubAppInstallationId) + updateSecretString(secretCopy, "githubAppEnterpriseBaseUrl", repository.GitHubAppEnterpriseBaseURL) + updateSecretBool(secretCopy, "insecureIgnoreHostKey", repository.InsecureIgnoreHostKey) + updateSecretBool(secretCopy, "insecure", repository.Insecure) + updateSecretBool(secretCopy, "enableLfs", repository.EnableLFS) + updateSecretString(secretCopy, "proxy", repository.Proxy) + updateSecretString(secretCopy, "noProxy", repository.NoProxy) + updateSecretString(secretCopy, "gcpServiceAccountKey", repository.GCPServiceAccountKey) + updateSecretBool(secretCopy, "forceHttpBasicAuth", repository.ForceHttpBasicAuth) + updateSecretBool(secretCopy, "useAzureWorkloadIdentity", repository.UseAzureWorkloadIdentity) + addSecretMetadata(secretCopy, s.getSecretType()) + + return secretCopy } func (s *secretsRepositoryBackend) secretToRepoCred(secret *corev1.Secret) (*appsv1.RepoCreds, error) { + secretCopy := secret.DeepCopy() + repository := &appsv1.RepoCreds{ - URL: string(secret.Data["url"]), - Username: string(secret.Data["username"]), - Password: string(secret.Data["password"]), - BearerToken: string(secret.Data["bearerToken"]), - SSHPrivateKey: string(secret.Data["sshPrivateKey"]), - TLSClientCertData: string(secret.Data["tlsClientCertData"]), - TLSClientCertKey: string(secret.Data["tlsClientCertKey"]), - Type: string(secret.Data["type"]), - GithubAppPrivateKey: string(secret.Data["githubAppPrivateKey"]), - GitHubAppEnterpriseBaseURL: string(secret.Data["githubAppEnterpriseBaseUrl"]), - GCPServiceAccountKey: string(secret.Data["gcpServiceAccountKey"]), - Proxy: string(secret.Data["proxy"]), - NoProxy: string(secret.Data["noProxy"]), - } - - enableOCI, err := boolOrFalse(secret, "enableOCI") + URL: string(secretCopy.Data["url"]), + Username: string(secretCopy.Data["username"]), + Password: string(secretCopy.Data["password"]), + BearerToken: string(secretCopy.Data["bearerToken"]), + SSHPrivateKey: string(secretCopy.Data["sshPrivateKey"]), + TLSClientCertData: string(secretCopy.Data["tlsClientCertData"]), + TLSClientCertKey: string(secretCopy.Data["tlsClientCertKey"]), + Type: string(secretCopy.Data["type"]), + GithubAppPrivateKey: string(secretCopy.Data["githubAppPrivateKey"]), + GitHubAppEnterpriseBaseURL: string(secretCopy.Data["githubAppEnterpriseBaseUrl"]), + GCPServiceAccountKey: string(secretCopy.Data["gcpServiceAccountKey"]), + Proxy: string(secretCopy.Data["proxy"]), + NoProxy: string(secretCopy.Data["noProxy"]), + } + + enableOCI, err := boolOrFalse(secretCopy, "enableOCI") if err != nil { return repository, err } repository.EnableOCI = enableOCI - insecureOCIForceHTTP, err := boolOrFalse(secret, "insecureOCIForceHttp") + insecureOCIForceHTTP, err := boolOrFalse(secretCopy, "insecureOCIForceHttp") if err != nil { return repository, err } repository.InsecureOCIForceHttp = insecureOCIForceHTTP - githubAppID, err := intOrZero(secret, "githubAppID") + githubAppID, err := intOrZero(secretCopy, "githubAppID") if err != nil { return repository, err } repository.GithubAppId = githubAppID - githubAppInstallationID, err := intOrZero(secret, "githubAppInstallationID") + githubAppInstallationID, err := intOrZero(secretCopy, "githubAppInstallationID") if err != nil { return repository, err } repository.GithubAppInstallationId = githubAppInstallationID - forceBasicAuth, err := boolOrFalse(secret, "forceHttpBasicAuth") + forceBasicAuth, err := boolOrFalse(secretCopy, "forceHttpBasicAuth") if err != nil { return repository, err } repository.ForceHttpBasicAuth = forceBasicAuth - useAzureWorkloadIdentity, err := boolOrFalse(secret, "useAzureWorkloadIdentity") + useAzureWorkloadIdentity, err := boolOrFalse(secretCopy, "useAzureWorkloadIdentity") if err != nil { return repository, err } @@ -486,31 +494,35 @@ func (s *secretsRepositoryBackend) secretToRepoCred(secret *corev1.Secret) (*app return repository, nil } -func repoCredsToSecret(repoCreds *appsv1.RepoCreds, secret *corev1.Secret) { - if secret.Data == nil { - secret.Data = make(map[string][]byte) - } - - updateSecretString(secret, "url", repoCreds.URL) - updateSecretString(secret, "username", repoCreds.Username) - updateSecretString(secret, "password", repoCreds.Password) - updateSecretString(secret, "bearerToken", repoCreds.BearerToken) - updateSecretString(secret, "sshPrivateKey", repoCreds.SSHPrivateKey) - updateSecretBool(secret, "enableOCI", repoCreds.EnableOCI) - updateSecretBool(secret, "insecureOCIForceHttp", repoCreds.InsecureOCIForceHttp) - updateSecretString(secret, "tlsClientCertData", repoCreds.TLSClientCertData) - updateSecretString(secret, "tlsClientCertKey", repoCreds.TLSClientCertKey) - updateSecretString(secret, "type", repoCreds.Type) - updateSecretString(secret, "githubAppPrivateKey", repoCreds.GithubAppPrivateKey) - updateSecretInt(secret, "githubAppID", repoCreds.GithubAppId) - updateSecretInt(secret, "githubAppInstallationID", repoCreds.GithubAppInstallationId) - updateSecretString(secret, "githubAppEnterpriseBaseUrl", repoCreds.GitHubAppEnterpriseBaseURL) - updateSecretString(secret, "gcpServiceAccountKey", repoCreds.GCPServiceAccountKey) - updateSecretString(secret, "proxy", repoCreds.Proxy) - updateSecretString(secret, "noProxy", repoCreds.NoProxy) - updateSecretBool(secret, "forceHttpBasicAuth", repoCreds.ForceHttpBasicAuth) - updateSecretBool(secret, "useAzureWorkloadIdentity", repoCreds.UseAzureWorkloadIdentity) - addSecretMetadata(secret, common.LabelValueSecretTypeRepoCreds) +func repoCredsToSecret(repoCreds *appsv1.RepoCreds, secret *corev1.Secret) *corev1.Secret { + secretCopy := secret.DeepCopy() + + if secretCopy.Data == nil { + secretCopy.Data = make(map[string][]byte) + } + + updateSecretString(secretCopy, "url", repoCreds.URL) + updateSecretString(secretCopy, "username", repoCreds.Username) + updateSecretString(secretCopy, "password", repoCreds.Password) + updateSecretString(secretCopy, "bearerToken", repoCreds.BearerToken) + updateSecretString(secretCopy, "sshPrivateKey", repoCreds.SSHPrivateKey) + updateSecretBool(secretCopy, "enableOCI", repoCreds.EnableOCI) + updateSecretBool(secretCopy, "insecureOCIForceHttp", repoCreds.InsecureOCIForceHttp) + updateSecretString(secretCopy, "tlsClientCertData", repoCreds.TLSClientCertData) + updateSecretString(secretCopy, "tlsClientCertKey", repoCreds.TLSClientCertKey) + updateSecretString(secretCopy, "type", repoCreds.Type) + updateSecretString(secretCopy, "githubAppPrivateKey", repoCreds.GithubAppPrivateKey) + updateSecretInt(secretCopy, "githubAppID", repoCreds.GithubAppId) + updateSecretInt(secretCopy, "githubAppInstallationID", repoCreds.GithubAppInstallationId) + updateSecretString(secretCopy, "githubAppEnterpriseBaseUrl", repoCreds.GitHubAppEnterpriseBaseURL) + updateSecretString(secretCopy, "gcpServiceAccountKey", repoCreds.GCPServiceAccountKey) + updateSecretString(secretCopy, "proxy", repoCreds.Proxy) + updateSecretString(secretCopy, "noProxy", repoCreds.NoProxy) + updateSecretBool(secretCopy, "forceHttpBasicAuth", repoCreds.ForceHttpBasicAuth) + updateSecretBool(secretCopy, "useAzureWorkloadIdentity", repoCreds.UseAzureWorkloadIdentity) + addSecretMetadata(secretCopy, common.LabelValueSecretTypeRepoCreds) + + return secretCopy } func (s *secretsRepositoryBackend) getRepositorySecret(repoURL, project string, allowFallback bool) (*corev1.Secret, error) { diff --git a/util/db/repository_secrets_test.go b/util/db/repository_secrets_test.go index 7f40dc3e560e4..b9533f2262b0a 100644 --- a/util/db/repository_secrets_test.go +++ b/util/db/repository_secrets_test.go @@ -1,7 +1,9 @@ package db import ( + "fmt" "strconv" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -85,9 +87,9 @@ func TestSecretsRepositoryBackend_CreateRepository(t *testing.T) { t.Parallel() secret := &corev1.Secret{} s := secretsRepositoryBackend{} - s.repositoryToSecret(repo, secret) - delete(secret.Labels, common.LabelKeySecretType) - f := setupWithK8sObjects(secret) + updatedSecret := s.repositoryToSecret(repo, secret) + delete(updatedSecret.Labels, common.LabelKeySecretType) + f := setupWithK8sObjects(updatedSecret) f.clientSet.ReactionChain = nil f.clientSet.AddReactor("create", "secrets", func(_ k8stesting.Action) (handled bool, ret runtime.Object, err error) { gr := schema.GroupResource{ @@ -122,8 +124,8 @@ func TestSecretsRepositoryBackend_CreateRepository(t *testing.T) { }, } s := secretsRepositoryBackend{} - s.repositoryToSecret(repo, secret) - f := setupWithK8sObjects(secret) + updatedSecret := s.repositoryToSecret(repo, secret) + f := setupWithK8sObjects(updatedSecret) f.clientSet.ReactionChain = nil f.clientSet.WatchReactionChain = nil f.clientSet.AddReactor("create", "secrets", func(_ k8stesting.Action) (handled bool, ret runtime.Object, err error) { @@ -134,7 +136,7 @@ func TestSecretsRepositoryBackend_CreateRepository(t *testing.T) { return true, nil, apierrors.NewAlreadyExists(gr, "already exists") }) watcher := watch.NewFakeWithChanSize(1, true) - watcher.Add(secret) + watcher.Add(updatedSecret) f.clientSet.AddWatchReactor("secrets", func(_ k8stesting.Action) (handled bool, ret watch.Interface, err error) { return true, watcher, nil }) @@ -946,7 +948,7 @@ func TestRepoCredsToSecret(t *testing.T) { GithubAppInstallationId: 456, GitHubAppEnterpriseBaseURL: "GitHubAppEnterpriseBaseURL", } - repoCredsToSecret(creds, s) + s = repoCredsToSecret(creds, s) assert.Equal(t, []byte(creds.URL), s.Data["url"]) assert.Equal(t, []byte(creds.Username), s.Data["username"]) assert.Equal(t, []byte(creds.Password), s.Data["password"]) @@ -962,3 +964,169 @@ func TestRepoCredsToSecret(t *testing.T) { assert.Equal(t, map[string]string{common.AnnotationKeyManagedBy: common.AnnotationValueManagedByArgoCD}, s.Annotations) assert.Equal(t, map[string]string{common.LabelKeySecretType: common.LabelValueSecretTypeRepoCreds}, s.Labels) } + +func TestRaceConditionInRepoCredsOperations(t *testing.T) { + // Create a single shared secret that will be accessed concurrently + sharedSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: RepoURLToSecretName(repoSecretPrefix, "git@github.com:argoproj/argo-cd.git", ""), + Namespace: testNamespace, + Labels: map[string]string{ + common.LabelKeySecretType: common.LabelValueSecretTypeRepoCreds, + }, + }, + Data: map[string][]byte{ + "url": []byte("git@github.com:argoproj/argo-cd.git"), + "username": []byte("test-user"), + "password": []byte("test-pass"), + }, + } + + // Create test credentials that we'll use for conversion + repoCreds := &appsv1.RepoCreds{ + URL: "git@github.com:argoproj/argo-cd.git", + Username: "test-user", + Password: "test-pass", + } + + backend := &secretsRepositoryBackend{} + + var wg sync.WaitGroup + concurrentOps := 50 + errChan := make(chan error, concurrentOps*2) // Channel to collect errors + + // Launch goroutines that perform concurrent operations + for i := 0; i < concurrentOps; i++ { + wg.Add(2) + + // One goroutine converts from RepoCreds to Secret + go func() { + defer wg.Done() + defer func() { + if r := recover(); r != nil { + errChan <- fmt.Errorf("panic in repoCredsToSecret: %v", r) + } + }() + _ = repoCredsToSecret(repoCreds, sharedSecret) + }() + + // Another goroutine converts from Secret to RepoCreds + go func() { + defer wg.Done() + defer func() { + if r := recover(); r != nil { + errChan <- fmt.Errorf("panic in secretToRepoCred: %v", r) + } + }() + creds, err := backend.secretToRepoCred(sharedSecret) + if err != nil { + errChan <- fmt.Errorf("error in secretToRepoCred: %w", err) + return + } + // Verify data integrity + if creds.URL != repoCreds.URL || creds.Username != repoCreds.Username || creds.Password != repoCreds.Password { + errChan <- fmt.Errorf("data mismatch in conversion: expected %v, got %v", repoCreds, creds) + } + }() + } + + wg.Wait() + close(errChan) + + // Check for any errors that occurred during concurrent operations + for err := range errChan { + t.Errorf("concurrent operation error: %v", err) + } + + // Verify final state + finalCreds, err := backend.secretToRepoCred(sharedSecret) + require.NoError(t, err) + assert.Equal(t, repoCreds.URL, finalCreds.URL) + assert.Equal(t, repoCreds.Username, finalCreds.Username) + assert.Equal(t, repoCreds.Password, finalCreds.Password) +} + +func TestRaceConditionInRepositoryOperations(t *testing.T) { + // Create a single shared secret that will be accessed concurrently + sharedSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: RepoURLToSecretName(repoSecretPrefix, "git@github.com:argoproj/argo-cd.git", ""), + Namespace: testNamespace, + Labels: map[string]string{ + common.LabelKeySecretType: common.LabelValueSecretTypeRepository, + }, + }, + Data: map[string][]byte{ + "url": []byte("git@github.com:argoproj/argo-cd.git"), + "name": []byte("test-repo"), + "username": []byte("test-user"), + "password": []byte("test-pass"), + }, + } + + // Create test repository that we'll use for conversion + repo := &appsv1.Repository{ + Name: "test-repo", + Repo: "git@github.com:argoproj/argo-cd.git", + Username: "test-user", + Password: "test-pass", + } + + backend := &secretsRepositoryBackend{} + + var wg sync.WaitGroup + concurrentOps := 50 + errChan := make(chan error, concurrentOps*2) // Channel to collect errors + + // Launch goroutines that perform concurrent operations + for i := 0; i < concurrentOps; i++ { + wg.Add(2) + + // One goroutine converts from Repository to Secret + go func() { + defer wg.Done() + defer func() { + if r := recover(); r != nil { + errChan <- fmt.Errorf("panic in repositoryToSecret: %v", r) + } + }() + _ = backend.repositoryToSecret(repo, sharedSecret) + }() + + // Another goroutine converts from Secret to Repository + go func() { + defer wg.Done() + defer func() { + if r := recover(); r != nil { + errChan <- fmt.Errorf("panic in secretToRepository: %v", r) + } + }() + repository, err := secretToRepository(sharedSecret) + if err != nil { + errChan <- fmt.Errorf("error in secretToRepository: %w", err) + return + } + // Verify data integrity + if repository.Name != repo.Name || repository.Repo != repo.Repo || + repository.Username != repo.Username || repository.Password != repo.Password { + errChan <- fmt.Errorf("data mismatch in conversion: expected %v, got %v", repo, repository) + } + }() + } + + wg.Wait() + close(errChan) + + // Check for any errors that occurred during concurrent operations + for err := range errChan { + t.Errorf("concurrent operation error: %v", err) + } + + // Verify final state + finalRepo, err := secretToRepository(sharedSecret) + require.NoError(t, err) + assert.Equal(t, repo.Name, finalRepo.Name) + assert.Equal(t, repo.Repo, finalRepo.Repo) + assert.Equal(t, repo.Username, finalRepo.Username) + assert.Equal(t, repo.Password, finalRepo.Password) +} diff --git a/util/webhook/webhook.go b/util/webhook/webhook.go index 11de059918c58..ec3251acfa9aa 100644 --- a/util/webhook/webhook.go +++ b/util/webhook/webhook.go @@ -150,10 +150,12 @@ func (a *ArgoCDWebhookHandler) affectedRevisionInfo(payloadIf any) (webURLs []st case azuredevops.GitPushEvent: // See: https://learn.microsoft.com/en-us/azure/devops/service-hooks/events?view=azure-devops#git.push webURLs = append(webURLs, payload.Resource.Repository.RemoteURL) - revision = ParseRevision(payload.Resource.RefUpdates[0].Name) - change.shaAfter = ParseRevision(payload.Resource.RefUpdates[0].NewObjectID) - change.shaBefore = ParseRevision(payload.Resource.RefUpdates[0].OldObjectID) - touchedHead = payload.Resource.RefUpdates[0].Name == payload.Resource.Repository.DefaultBranch + if len(payload.Resource.RefUpdates) > 0 { + revision = ParseRevision(payload.Resource.RefUpdates[0].Name) + change.shaAfter = ParseRevision(payload.Resource.RefUpdates[0].NewObjectID) + change.shaBefore = ParseRevision(payload.Resource.RefUpdates[0].OldObjectID) + touchedHead = payload.Resource.RefUpdates[0].Name == payload.Resource.Repository.DefaultBranch + } // unfortunately, Azure DevOps doesn't provide a list of changed files case github.PushPayload: // See: https://developer.github.com/v3/activity/events/types/#pushevent @@ -251,13 +253,15 @@ func (a *ArgoCDWebhookHandler) affectedRevisionInfo(payloadIf any) (webURLs []st // Webhook module does not parse the inner links if payload.Repository.Links != nil { - for _, l := range payload.Repository.Links["clone"].([]any) { - link := l.(map[string]any) - if link["name"] == "http" { - webURLs = append(webURLs, link["href"].(string)) - } - if link["name"] == "ssh" { - webURLs = append(webURLs, link["href"].(string)) + clone, ok := payload.Repository.Links["clone"].([]any) + if ok { + for _, l := range clone { + link := l.(map[string]any) + if link["name"] == "http" || link["name"] == "ssh" { + if href, ok := link["href"].(string); ok { + webURLs = append(webURLs, href) + } + } } } } @@ -276,11 +280,13 @@ func (a *ArgoCDWebhookHandler) affectedRevisionInfo(payloadIf any) (webURLs []st // so we cannot update changedFiles for this type of payload case gogsclient.PushPayload: - webURLs = append(webURLs, payload.Repo.HTMLURL) revision = ParseRevision(payload.Ref) change.shaAfter = ParseRevision(payload.After) change.shaBefore = ParseRevision(payload.Before) - touchedHead = bool(payload.Repo.DefaultBranch == revision) + if payload.Repo != nil { + webURLs = append(webURLs, payload.Repo.HTMLURL) + touchedHead = payload.Repo.DefaultBranch == revision + } for _, commit := range payload.Commits { changedFiles = append(changedFiles, commit.Added...) changedFiles = append(changedFiles, commit.Modified...) diff --git a/util/webhook/webhook_test.go b/util/webhook/webhook_test.go index c341f9323f743..c41c91caa098e 100644 --- a/util/webhook/webhook_test.go +++ b/util/webhook/webhook_test.go @@ -15,6 +15,8 @@ import ( "text/template" "time" + "github.com/go-playground/webhooks/v6/azuredevops" + bb "github.com/ktrysmt/go-bitbucket" "github.com/stretchr/testify/mock" "k8s.io/apimachinery/pkg/types" @@ -702,6 +704,26 @@ func Test_affectedRevisionInfo_appRevisionHasChanged(t *testing.T) { {true, "refs/tags/no-slashes", bitbucketPushPayload("no-slashes"), "bitbucket push branch or tag name without slashes, targetRevision tag prefixed"}, {true, "refs/tags/no-slashes", bitbucketRefChangedPayload("no-slashes"), "bitbucket ref changed branch or tag name without slashes, targetRevision tag prefixed"}, {true, "refs/tags/no-slashes", gogsPushPayload("no-slashes"), "gogs push branch or tag name without slashes, targetRevision tag prefixed"}, + + // Tests fix for https://github.com/argoproj/argo-cd/security/advisories/GHSA-wp4p-9pxh-cgx2 + {true, "test", gogsclient.PushPayload{Ref: "test", Repo: nil}, "gogs push branch with nil repo in payload"}, + + // Testing fix for https://github.com/argoproj/argo-cd/security/advisories/GHSA-gpx4-37g2-c8pv + {false, "test", azuredevops.GitPushEvent{Resource: azuredevops.Resource{RefUpdates: []azuredevops.RefUpdate{}}}, "Azure DevOps malformed push event with no ref updates"}, + + {true, "some-ref", bitbucketserver.RepositoryReferenceChangedPayload{ + Changes: []bitbucketserver.RepositoryChange{ + {Reference: bitbucketserver.RepositoryReference{ID: "refs/heads/some-ref"}}, + }, + Repository: bitbucketserver.Repository{Links: map[string]any{"clone": "boom"}}, // The string "boom" here is what previously caused a panic. + }, "bitbucket push branch or tag name, malformed link"}, // https://github.com/argoproj/argo-cd/security/advisories/GHSA-f9gq-prrc-hrhc + + {true, "some-ref", bitbucketserver.RepositoryReferenceChangedPayload{ + Changes: []bitbucketserver.RepositoryChange{ + {Reference: bitbucketserver.RepositoryReference{ID: "refs/heads/some-ref"}}, + }, + Repository: bitbucketserver.Repository{Links: map[string]any{"clone": []any{map[string]any{"name": "http", "href": []string{}}}}}, // The href as an empty array is what previously caused a panic. + }, "bitbucket push branch or tag name, malformed href"}, } for _, testCase := range tests { testCopy := testCase