Bump version to 2.9.9 (#17558 )

Signed-off-by: GitHub <noreply@github.com> Co-authored-by: crenshaw-dev <crenshaw-dev@users.noreply.github.com>
Merge pull request from GHSA-6v85-wr92-q4p7
2026-04-10 18:58:47 +02:00 · 2024-03-18 04:03:25 -04:00 · 2024-03-18 03:59:06 -04:00 · 2024-03-18 03:58:18 -04:00 · 2024-03-15 17:32:21 -04:00 · 2024-03-13 14:45:51 -04:00
21 changed files with 352 additions and 97 deletions
--- a/2
+++ b/2
@@ -1 +1 @@
-2.9.7
+2.9.9
--- a/manifests/base/kustomization.yaml
+++ b/manifests/base/kustomization.yaml
@@ -5,7 +5,7 @@ kind: Kustomization
 images:
 - name: quay.io/argoproj/argocd
  newName: quay.io/argoproj/argocd
-  newTag: v2.9.7
+  newTag: v2.9.9
 resources:
 - ./application-controller
 - ./dex
--- a/manifests/core-install.yaml
+++ b/manifests/core-install.yaml
@@ -20750,7 +20750,7 @@ spec:
              key: applicationsetcontroller.allowed.scm.providers
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: argocd-applicationset-controller
        ports:
@@ -21050,7 +21050,7 @@ spec:
          value: /helm-working-dir
        - name: HELM_DATA_HOME
          value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
@@ -21102,7 +21102,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        name: copyutil
        securityContext:
          allowPrivilegeEscalation: false
@@ -21321,7 +21321,7 @@ spec:
              key: controller.kubectl.parallelism.limit
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: argocd-application-controller
        ports:
--- a/manifests/core-install/kustomization.yaml
+++ b/manifests/core-install/kustomization.yaml
@@ -12,4 +12,4 @@ resources:
 images:
 - name: quay.io/argoproj/argocd
  newName: quay.io/argoproj/argocd
-  newTag: v2.9.7
+  newTag: v2.9.9
--- a/manifests/ha/base/kustomization.yaml
+++ b/manifests/ha/base/kustomization.yaml
@@ -12,7 +12,7 @@ patches:
 images:
 - name: quay.io/argoproj/argocd
  newName: quay.io/argoproj/argocd
-  newTag: v2.9.7
+  newTag: v2.9.9
 resources:
 - ../../base/application-controller
 - ../../base/applicationset-controller
--- a/manifests/ha/install.yaml
+++ b/manifests/ha/install.yaml
@@ -22007,7 +22007,7 @@ spec:
              key: applicationsetcontroller.allowed.scm.providers
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: argocd-applicationset-controller
        ports:
@@ -22130,7 +22130,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: copyutil
        securityContext:
@@ -22206,7 +22206,7 @@ spec:
              key: application.namespaces
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          tcpSocket:
@@ -22537,7 +22537,7 @@ spec:
          value: /helm-working-dir
        - name: HELM_DATA_HOME
          value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
@@ -22589,7 +22589,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        name: copyutil
        securityContext:
          allowPrivilegeEscalation: false
@@ -22884,7 +22884,7 @@ spec:
              key: server.api.content.types
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
@@ -23130,7 +23130,7 @@ spec:
              key: controller.kubectl.parallelism.limit
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: argocd-application-controller
        ports:
--- a/manifests/ha/namespace-install.yaml
+++ b/manifests/ha/namespace-install.yaml
@@ -1662,7 +1662,7 @@ spec:
              key: applicationsetcontroller.allowed.scm.providers
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: argocd-applicationset-controller
        ports:
@@ -1785,7 +1785,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: copyutil
        securityContext:
@@ -1861,7 +1861,7 @@ spec:
              key: application.namespaces
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          tcpSocket:
@@ -2192,7 +2192,7 @@ spec:
          value: /helm-working-dir
        - name: HELM_DATA_HOME
          value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
@@ -2244,7 +2244,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        name: copyutil
        securityContext:
          allowPrivilegeEscalation: false
@@ -2539,7 +2539,7 @@ spec:
              key: server.api.content.types
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
@@ -2785,7 +2785,7 @@ spec:
              key: controller.kubectl.parallelism.limit
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: argocd-application-controller
        ports:
--- a/manifests/install.yaml
+++ b/manifests/install.yaml
@@ -21102,7 +21102,7 @@ spec:
              key: applicationsetcontroller.allowed.scm.providers
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: argocd-applicationset-controller
        ports:
@@ -21225,7 +21225,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: copyutil
        securityContext:
@@ -21301,7 +21301,7 @@ spec:
              key: application.namespaces
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          tcpSocket:
@@ -21583,7 +21583,7 @@ spec:
          value: /helm-working-dir
        - name: HELM_DATA_HOME
          value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
@@ -21635,7 +21635,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        name: copyutil
        securityContext:
          allowPrivilegeEscalation: false
@@ -21928,7 +21928,7 @@ spec:
              key: server.api.content.types
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
@@ -22174,7 +22174,7 @@ spec:
              key: controller.kubectl.parallelism.limit
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: argocd-application-controller
        ports:
--- a/manifests/namespace-install.yaml
+++ b/manifests/namespace-install.yaml
@@ -757,7 +757,7 @@ spec:
              key: applicationsetcontroller.allowed.scm.providers
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: argocd-applicationset-controller
        ports:
@@ -880,7 +880,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /shared/argocd-dex
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: copyutil
        securityContext:
@@ -956,7 +956,7 @@ spec:
              key: application.namespaces
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          tcpSocket:
@@ -1238,7 +1238,7 @@ spec:
          value: /helm-working-dir
        - name: HELM_DATA_HOME
          value: /helm-working-dir
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          failureThreshold: 3
@@ -1290,7 +1290,7 @@ spec:
        - -n
        - /usr/local/bin/argocd
        - /var/run/argocd/argocd-cmp-server
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        name: copyutil
        securityContext:
          allowPrivilegeEscalation: false
@@ -1583,7 +1583,7 @@ spec:
              key: server.api.content.types
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        livenessProbe:
          httpGet:
@@ -1829,7 +1829,7 @@ spec:
              key: controller.kubectl.parallelism.limit
              name: argocd-cmd-params-cm
              optional: true
-        image: quay.io/argoproj/argocd:v2.9.7
+        image: quay.io/argoproj/argocd:v2.9.9
        imagePullPolicy: Always
        name: argocd-application-controller
        ports:
--- a/server/application/application.go
+++ b/server/application/application.go
@@ -325,6 +325,15 @@ func (s *Server) Create(ctx context.Context, q *application.ApplicationCreateReq
 		return nil, security.NamespaceNotPermittedError(appNs)
 	}

+	// Don't let the app creator set the operation explicitly. Those requests should always go through the Sync API.
+	if a.Operation != nil {
+		log.WithFields(log.Fields{
+			"application":            a.Name,
+			argocommon.SecurityField: argocommon.SecurityLow,
+		}).Warn("User attempted to set operation on application creation. This could have allowed them to bypass branch protection rules by setting manifests directly. Ignoring the set operation.")
+		a.Operation = nil
+	}
+
 	created, err := s.appclientset.ArgoprojV1alpha1().Applications(appNs).Create(ctx, a, metav1.CreateOptions{})
 	if err == nil {
 		s.logAppEvent(created, ctx, argo.EventReasonResourceCreated, "created application")
--- a/server/application/application_test.go
+++ b/server/application/application_test.go
@@ -1439,6 +1439,27 @@ func TestCreateAppWithDestName(t *testing.T) {
 	assert.Equal(t, app.Spec.Destination.Server, "https://cluster-api.com")
 }

+// TestCreateAppWithOperation tests that an application created with an operation is created with the operation removed.
+// Avoids regressions of https://github.com/argoproj/argo-cd/security/advisories/GHSA-g623-jcgg-mhmm
+func TestCreateAppWithOperation(t *testing.T) {
+	appServer := newTestAppServer(t)
+	testApp := newTestAppWithDestName()
+	testApp.Operation = &appsv1.Operation{
+		Sync: &appsv1.SyncOperation{
+			Manifests: []string{
+				"test",
+			},
+		},
+	}
+	createReq := application.ApplicationCreateRequest{
+		Application: testApp,
+	}
+	app, err := appServer.Create(context.Background(), &createReq)
+	require.NoError(t, err)
+	require.NotNil(t, app)
+	assert.Nil(t, app.Operation)
+}
+
 func TestUpdateApp(t *testing.T) {
 	testApp := newTestApp()
 	appServer := newTestAppServer(t, testApp)
--- a/server/application/terminal.go
+++ b/server/application/terminal.go
@@ -38,12 +38,12 @@ type terminalHandler struct {
 	allowedShells     []string
 	namespace         string
 	enabledNamespaces []string
-	sessionManager    util_session.SessionManager
+	sessionManager    *util_session.SessionManager
 }

 // NewHandler returns a new terminal handler.
 func NewHandler(appLister applisters.ApplicationLister, namespace string, enabledNamespaces []string, db db.ArgoDB, enf *rbac.Enforcer, cache *servercache.Cache,
-	appResourceTree AppResourceTreeFn, allowedShells []string, sessionManager util_session.SessionManager) *terminalHandler {
+	appResourceTree AppResourceTreeFn, allowedShells []string, sessionManager *util_session.SessionManager) *terminalHandler {
 	return &terminalHandler{
 		appLister:         appLister,
 		db:                db,
--- a/server/application/websocket.go
+++ b/server/application/websocket.go
@@ -37,7 +37,7 @@ type terminalSession struct {
 	tty            bool
 	readLock       sync.Mutex
 	writeLock      sync.Mutex
-	sessionManager util_session.SessionManager
+	sessionManager *util_session.SessionManager
 	token          *string
 }

@@ -48,7 +48,7 @@ func getToken(r *http.Request) (string, error) {
 }

 // newTerminalSession create terminalSession
-func newTerminalSession(w http.ResponseWriter, r *http.Request, responseHeader http.Header, sessionManager util_session.SessionManager) (*terminalSession, error) {
+func newTerminalSession(w http.ResponseWriter, r *http.Request, responseHeader http.Header, sessionManager *util_session.SessionManager) (*terminalSession, error) {
 	token, err := getToken(r)
 	if err != nil {
 		return nil, err
--- a/server/server.go
+++ b/server/server.go
@@ -997,7 +997,7 @@ func (a *ArgoCDServer) newHTTPServer(ctx context.Context, port int, grpcWebHandl
 	}
 	mux.Handle("/api/", handler)

-	terminal := application.NewHandler(a.appLister, a.Namespace, a.ApplicationNamespaces, a.db, a.enf, a.Cache, appResourceTreeFn, a.settings.ExecShells, *a.sessionMgr).
+	terminal := application.NewHandler(a.appLister, a.Namespace, a.ApplicationNamespaces, a.db, a.enf, a.Cache, appResourceTreeFn, a.settings.ExecShells, a.sessionMgr).
 		WithFeatureFlagMiddleware(a.settingsMgr.GetSettings)
 	th := util_session.WithAuthMiddleware(a.DisableAuth, a.sessionMgr, terminal)
 	mux.Handle("/terminal", th)
--- a/ui/src/app/applications/components/application-summary/application-summary.tsx
+++ b/ui/src/app/applications/components/application-summary/application-summary.tsx
@@ -30,6 +30,7 @@ import {EditAnnotations} from './edit-annotations';

 import './application-summary.scss';
 import {DeepLinks} from '../../../shared/components/deep-links';
+import {ExternalLinks} from '../application-urls';

 function swap(array: any[], a: number, b: number) {
    array = array.slice();
@@ -326,20 +327,19 @@ export const ApplicationSummary = (props: ApplicationSummaryProps) => {
            )
        }
    ];
-
-    const urls = app.status.summary.externalURLs || [];
+    const urls = ExternalLinks(app.status.summary.externalURLs);
    if (urls.length > 0) {
        attributes.push({
            title: 'URLs',
            view: (
                <React.Fragment>
-                    {urls
-                        .map(item => item.split('|'))
-                        .map((parts, i) => (
-                            <a key={i} href={parts.length > 1 ? parts[1] : parts[0]} target='__blank'>
-                                {parts[0]} &nbsp;
+                    {urls.map((url, i) => {
+                        return (
+                            <a key={i} href={url.ref} target='__blank'>
+                                {url.title} &nbsp;
                            </a>
-                        ))}
+                        );
+                    })}
                </React.Fragment>
            )
        });
--- a/ui/src/app/applications/components/application-urls.test.ts
+++ b/ui/src/app/applications/components/application-urls.test.ts
@@ -1,4 +1,4 @@
-import {ExternalLink, InvalidExternalLinkError} from './application-urls';
+import { ExternalLink, ExternalLinks, InvalidExternalLinkError } from './application-urls';

 test('rejects malicious URLs', () => {
    expect(() => {
@@ -7,6 +7,16 @@ test('rejects malicious URLs', () => {
    expect(() => {
        const _ = new ExternalLink('data:text/html;<h1>hi</h1>');
    }).toThrowError(InvalidExternalLinkError);
+    expect(() => {
+        const _ = new ExternalLink('title|data:text/html;<h1>hi</h1>');
+    }).toThrowError(InvalidExternalLinkError);
+    expect(() => {
+        const _ = new ExternalLink('data:title|data:text/html;<h1>hi</h1>');
+    }).toThrowError(InvalidExternalLinkError);
+
+    expect(() => {
+        const _ = new ExternalLink('data:title|https://localhost:8080/applications');
+    }).not.toThrowError(InvalidExternalLinkError);
 });

 test('allows absolute URLs', () => {
@@ -18,3 +28,59 @@ test('allows relative URLs', () => {
    window.location = new URL('https://localhost:8080/applications');
    expect(new ExternalLink('/applications').ref).toEqual('/applications');
 });
+
+
+test('URLs format', () => {
+    expect(new ExternalLink('https://localhost:8080/applications')).toEqual({
+        ref: 'https://localhost:8080/applications',
+        title: 'https://localhost:8080/applications',
+    })
+    expect(new ExternalLink('title|https://localhost:8080/applications')).toEqual({
+        ref: 'https://localhost:8080/applications',
+        title: 'title',
+    })
+});
+
+
+test('malicious URLs from list to be removed', () => {
+    const urls: string[] = [
+        'javascript:alert("hi")',
+        'https://localhost:8080/applications',
+    ]
+    const links = ExternalLinks(urls);
+
+    expect(links).toHaveLength(1);
+    expect(links).toContainEqual({
+        ref: 'https://localhost:8080/applications',
+        title: 'https://localhost:8080/applications',
+    });
+});
+
+
+test('list to be sorted', () => {
+    const urls: string[] = [
+        'https://a',
+        'https://b',
+        'a|https://c',
+        'z|https://c',
+        'x|https://d',
+        'x|https://c',
+    ]
+    const links = ExternalLinks(urls);
+
+    // 'a|https://c',
+    // 'x|https://c',
+    // 'x|https://d',
+    // 'z|https://c',
+    // 'https://a',
+    // 'https://b',
+    expect(links).toHaveLength(6);
+    expect(links[0].title).toEqual('a')
+    expect(links[1].title).toEqual('x')
+    expect(links[1].ref).toEqual('https://c')
+    expect(links[2].title).toEqual('x')
+    expect(links[2].ref).toEqual('https://d')
+    expect(links[3].title).toEqual('z')
+    expect(links[4].title).toEqual('https://a')
+    expect(links[5].title).toEqual('https://b')
+});
--- a/ui/src/app/applications/components/application-urls.tsx
+++ b/ui/src/app/applications/components/application-urls.tsx
@@ -29,7 +29,7 @@ export class ExternalLink {
    }
 }

-export const ApplicationURLs = ({urls}: {urls: string[]}) => {
+export const ExternalLinks = (urls?: string[]) => {
    const externalLinks: ExternalLink[] = [];
    for (const url of urls || []) {
        try {
@@ -42,16 +42,26 @@ export const ApplicationURLs = ({urls}: {urls: string[]}) => {

    // sorted alphabetically & links with titles first
    externalLinks.sort((a, b) => {
-        if (a.title !== '' && b.title !== '') {
+        const hasTitle = (x: ExternalLink): boolean => {
+            return x.title !== x.ref && x.title !== '';
+        };
+
+        if (hasTitle(a) && hasTitle(b) && a.title !== b.title) {
            return a.title > b.title ? 1 : -1;
-        } else if (a.title === '') {
+        } else if (hasTitle(b) && !hasTitle(a)) {
            return 1;
-        } else if (b.title === '') {
+        } else if (hasTitle(a) && !hasTitle(b)) {
            return -1;
        }
        return a.ref > b.ref ? 1 : -1;
    });

+    return externalLinks;
+};
+
+export const ApplicationURLs = ({urls}: {urls: string[]}) => {
+    const externalLinks: ExternalLink[] = ExternalLinks(urls);
+
    return (
        ((externalLinks || []).length > 0 && (
            <div className='applications-list__external-links-icon-container'>
--- a/util/helm/client.go
+++ b/util/helm/client.go
@@ -8,7 +8,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	executil "github.com/argoproj/argo-cd/v2/util/exec"
 	"io"
 	"net/http"
 	"net/url"
@@ -19,6 +18,8 @@ import (
 	"strings"
 	"time"

+	executil "github.com/argoproj/argo-cd/v2/util/exec"
+
 	"github.com/argoproj/pkg/sync"
 	log "github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v2"
@@ -34,6 +35,8 @@ import (
 var (
 	globalLock = sync.NewKeyLock()
 	indexLock  = sync.NewKeyLock()
+
+	OCINotEnabledErr = errors.New("could not perform the action when oci is not enabled")
 )

 type Creds struct {
@@ -401,6 +404,10 @@ func getIndexURL(rawURL string) (string, error) {
 }

 func (c *nativeHelmChart) GetTags(chart string, noCache bool) (*TagsList, error) {
+	if !c.enableOci {
+		return nil, OCINotEnabledErr
+	}
+
 	tagsURL := strings.Replace(fmt.Sprintf("%s/%s", c.repoURL, chart), "https://", "", 1)
 	indexLock.Lock(tagsURL)
 	defer indexLock.Unlock(tagsURL)
@@ -428,10 +435,12 @@ func (c *nativeHelmChart) GetTags(chart string, noCache bool) (*TagsList, error)
 			TLSClientConfig:   tlsConf,
 			DisableKeepAlives: true,
 		}}
+
+		repoHost, _, _ := strings.Cut(tagsURL, "/")
 		repo.Client = &auth.Client{
 			Client: client,
 			Cache:  nil,
-			Credential: auth.StaticCredential(c.repoURL, auth.Credential{
+			Credential: auth.StaticCredential(repoHost, auth.Credential{
 				Username: c.creds.Username,
 				Password: c.creds.Password,
 			}),
--- a/util/helm/client_test.go
+++ b/util/helm/client_test.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"math"
+	"net/url"
 	"os"
 	"strings"
 	"testing"
@@ -159,41 +160,129 @@ func TestGetIndexURL(t *testing.T) {
 }

 func TestGetTagsFromUrl(t *testing.T) {
+	t.Run("should return tags correctly while following the link header", func(t *testing.T) {
+		server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			t.Logf("called %s", r.URL.Path)
+			responseTags := TagsList{}
+			w.Header().Set("Content-Type", "application/json")
+			if !strings.Contains(r.URL.String(), "token") {
+				w.Header().Set("Link", fmt.Sprintf("<https://%s%s?token=next-token>; rel=next", r.Host, r.URL.Path))
+				responseTags.Tags = []string{"first"}
+			} else {
+				responseTags.Tags = []string{
+					"second",
+					"2.8.0",
+					"2.8.0-prerelease",
+					"2.8.0_build",
+					"2.8.0-prerelease_build",
+					"2.8.0-prerelease.1_build.1234",
+				}
+			}
+			w.WriteHeader(http.StatusOK)
+			err := json.NewEncoder(w).Encode(responseTags)
+			if err != nil {
+				t.Fatal(err)
+			}
+		}))
+
+		client := NewClient(server.URL, Creds{InsecureSkipVerify: true}, true, "")
+
+		tags, err := client.GetTags("mychart", true)
+		assert.NoError(t, err)
+		assert.ElementsMatch(t, tags.Tags, []string{
+			"first",
+			"second",
+			"2.8.0",
+			"2.8.0-prerelease",
+			"2.8.0+build",
+			"2.8.0-prerelease+build",
+			"2.8.0-prerelease.1+build.1234",
+		})
+	})
+
+	t.Run("should return an error not when oci is not enabled", func(t *testing.T) {
+		client := NewClient("example.com", Creds{}, false, "")
+
+		_, err := client.GetTags("my-chart", true)
+		assert.ErrorIs(t, OCINotEnabledErr, err)
+	})
+}
+
+func TestGetTagsFromURLPrivateRepoAuthentication(t *testing.T) {
 	server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		t.Logf("called %s", r.URL.Path)
-		responseTags := TagsList{}
-		w.Header().Set("Content-Type", "application/json")
-		if !strings.Contains(r.URL.String(), "token") {
-			w.Header().Set("Link", fmt.Sprintf("<https://%s%s?token=next-token>; rel=next", r.Host, r.URL.Path))
-			responseTags.Tags = []string{"first"}
-		} else {
-			responseTags.Tags = []string{
-				"second",
+
+		authorization := r.Header.Get("Authorization")
+		if authorization == "" {
+			w.Header().Set("WWW-Authenticate", `Basic realm="helm repo to get tags"`)
+			w.WriteHeader(http.StatusUnauthorized)
+			return
+		}
+
+		t.Logf("authorization received %s", authorization)
+
+		responseTags := TagsList{
+			Tags: []string{
 				"2.8.0",
 				"2.8.0-prerelease",
 				"2.8.0_build",
 				"2.8.0-prerelease_build",
 				"2.8.0-prerelease.1_build.1234",
-			}
+			},
 		}
+
+		w.Header().Set("Content-Type", "application/json")
 		w.WriteHeader(http.StatusOK)
 		err := json.NewEncoder(w).Encode(responseTags)
 		if err != nil {
 			t.Fatal(err)
 		}
 	}))
+	t.Cleanup(server.Close)

-	client := NewClient(server.URL, Creds{InsecureSkipVerify: true}, true, "")
-
-	tags, err := client.GetTags("mychart", true)
+	serverURL, err := url.Parse(server.URL)
 	assert.NoError(t, err)
-	assert.ElementsMatch(t, tags.Tags, []string{
-		"first",
-		"second",
-		"2.8.0",
-		"2.8.0-prerelease",
-		"2.8.0+build",
-		"2.8.0-prerelease+build",
-		"2.8.0-prerelease.1+build.1234",
-	})
+
+	testCases := []struct {
+		name    string
+		repoURL string
+	}{
+		{
+			name:    "should login correctly when the repo path is in the server root with http scheme",
+			repoURL: server.URL,
+		},
+		{
+			name:    "should login correctly when the repo path is not in the server root with http scheme",
+			repoURL: fmt.Sprintf("%s/my-repo", server.URL),
+		},
+		{
+			name:    "should login correctly when the repo path is in the server root without http scheme",
+			repoURL: serverURL.Host,
+		},
+		{
+			name:    "should login correctly when the repo path is not in the server root without http scheme",
+			repoURL: fmt.Sprintf("%s/my-repo", serverURL.Host),
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			client := NewClient(testCase.repoURL, Creds{
+				InsecureSkipVerify: true,
+				Username:           "my-username",
+				Password:           "my-password",
+			}, true, "")
+
+			tags, err := client.GetTags("mychart", true)
+
+			assert.NoError(t, err)
+			assert.ElementsMatch(t, tags.Tags, []string{
+				"2.8.0",
+				"2.8.0-prerelease",
+				"2.8.0+build",
+				"2.8.0-prerelease+build",
+				"2.8.0-prerelease.1+build.1234",
+			})
+		})
+	}
 }
--- a/util/session/sessionmanager.go
+++ b/util/session/sessionmanager.go
@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"os"
 	"strings"
+	"sync"
 	"time"

 	"github.com/coreos/go-oidc/v3/oidc"
@@ -41,6 +42,7 @@ type SessionManager struct {
 	storage                       UserStateStorage
 	sleep                         func(d time.Duration)
 	verificationDelayNoiseEnabled bool
+	failedLock                    sync.RWMutex
 }

 // LoginAttempts is a timestamped counter for failed login attempts
@@ -69,7 +71,7 @@ const (
 	// Maximum length of username, too keep the cache's memory signature low
 	maxUsernameLength = 32
 	// The default maximum session cache size
-	defaultMaxCacheSize = 1000
+	defaultMaxCacheSize = 10000
 	// The default number of maximum login failures before delay kicks in
 	defaultMaxLoginFailures = 5
 	// The default time in seconds for the failure window
@@ -284,7 +286,7 @@ func (mgr *SessionManager) Parse(tokenString string) (jwt.Claims, string, error)
 	return token.Claims, newToken, nil
 }

-// GetLoginFailures retrieves the login failure information from the cache
+// GetLoginFailures retrieves the login failure information from the cache. Any modifications to the LoginAttemps map must be done in a thread-safe manner.
 func (mgr *SessionManager) GetLoginFailures() map[string]LoginAttempts {
 	// Get failures from the cache
 	var failures map[string]LoginAttempts
@@ -299,25 +301,43 @@ func (mgr *SessionManager) GetLoginFailures() map[string]LoginAttempts {
 	return failures
 }

-func expireOldFailedAttempts(maxAge time.Duration, failures *map[string]LoginAttempts) int {
+func expireOldFailedAttempts(maxAge time.Duration, failures map[string]LoginAttempts) int {
 	expiredCount := 0
-	for key, attempt := range *failures {
+	for key, attempt := range failures {
 		if time.Since(attempt.LastFailed) > maxAge*time.Second {
 			expiredCount += 1
-			delete(*failures, key)
+			delete(failures, key)
 		}
 	}
 	return expiredCount
 }

+// Protect admin user from login attempt reset caused by attempts to overflow cache in a brute force attack. Instead remove random non-admin to make room in cache. 
+func pickRandomNonAdminLoginFailure(failures map[string]LoginAttempts, username string) *string {
+	idx := rand.Intn(len(failures) - 1)
+	i := 0
+	for key := range failures {
+		if i == idx {
+			if key == common.ArgoCDAdminUsername || key == username {
+				return pickRandomNonAdminLoginFailure(failures, username)
+			}
+			return &key
+		}
+		i++
+	}
+	return nil
+}
+
 // Updates the failure count for a given username. If failed is true, increases the counter. Otherwise, sets counter back to 0.
 func (mgr *SessionManager) updateFailureCount(username string, failed bool) {
+	mgr.failedLock.Lock()
+	defer mgr.failedLock.Unlock()

 	failures := mgr.GetLoginFailures()

 	// Expire old entries in the cache if we have a failure window defined.
 	if window := getLoginFailureWindow(); window > 0 {
-		count := expireOldFailedAttempts(window, &failures)
+		count := expireOldFailedAttempts(window, failures)
 		if count > 0 {
 			log.Infof("Expired %d entries from session cache due to max age reached", count)
 		}
@@ -327,23 +347,13 @@ func (mgr *SessionManager) updateFailureCount(username string, failed bool) {
 	// prevent overbloating the cache with fake entries, as this could lead to
 	// memory exhaustion and ultimately in a DoS. We remove a single entry to
 	// replace it with the new one.
-	//
-	// Chances are that we remove the one that is under active attack, but this
-	// chance is low (1:cache_size)
 	if failed && len(failures) >= getMaximumCacheSize() {
 		log.Warnf("Session cache size exceeds %d entries, removing random entry", getMaximumCacheSize())
-		idx := rand.Intn(len(failures) - 1)
-		var rmUser string
-		i := 0
-		for key := range failures {
-			if i == idx {
-				rmUser = key
-				delete(failures, key)
-				break
-			}
-			i++
+		rmUser := pickRandomNonAdminLoginFailure(failures, username)
+		if rmUser != nil {
+			delete(failures, *rmUser)
+			log.Infof("Deleted entry for user %s from cache", *rmUser)
 		}
-		log.Infof("Deleted entry for user %s from cache", rmUser)
 	}

 	attempt, ok := failures[username]
@@ -374,6 +384,8 @@ func (mgr *SessionManager) updateFailureCount(username string, failed bool) {

 // Get the current login failure attempts for given username
 func (mgr *SessionManager) getFailureCount(username string) LoginAttempts {
+	mgr.failedLock.RLock()
+	defer mgr.failedLock.RUnlock()
 	failures := mgr.GetLoginFailures()
 	attempt, ok := failures[username]
 	if !ok {
--- a/util/session/sessionmanager_test.go
+++ b/util/session/sessionmanager_test.go
@@ -1173,3 +1173,42 @@ requestedScopes: ["oidc"]`, oidcTestServer.URL),
 		assert.ErrorIs(t, err, common.TokenVerificationErr)
 	})
 }
+
+func Test_PickFailureAttemptWhenOverflowed(t *testing.T) {
+	t.Run("Not pick admin user from the queue", func(t *testing.T) {
+		failures := map[string]LoginAttempts{
+			"admin": {
+				FailCount: 1,
+			},
+			"test2": {
+				FailCount: 1,
+			},
+		}
+
+		// inside pickRandomNonAdminLoginFailure, it uses random, so we need to test it multiple times
+		for i := 0; i < 1000; i++ {
+			user := pickRandomNonAdminLoginFailure(failures, "test")
+			assert.Equal(t, "test2", *user)
+		}
+	})
+
+	t.Run("Not pick admin user and current user from the queue", func(t *testing.T) {
+		failures := map[string]LoginAttempts{
+			"test": {
+				FailCount: 1,
+			},
+			"admin": {
+				FailCount: 1,
+			},
+			"test2": {
+				FailCount: 1,
+			},
+		}
+
+		// inside pickRandomNonAdminLoginFailure, it uses random, so we need to test it multiple times
+		for i := 0; i < 1000; i++ {
+			user := pickRandomNonAdminLoginFailure(failures, "test")
+			assert.Equal(t, "test2", *user)
+		}
+	})
+}
Author	SHA1	Message	Date
github-actions[bot]	8281b18831	Bump version to 2.9.9 (#17558 ) Signed-off-by: GitHub <noreply@github.com> Co-authored-by: crenshaw-dev <crenshaw-dev@users.noreply.github.com>	2024-03-18 04:03:25 -04:00
Dan Garfield	5bbb51ab42	Merge pull request from GHSA-6v85-wr92-q4p7 * fix: Fix concurrency issue in session manager Signed-off-by: jannfis <jann@mistrust.net> * Add note that modification to the map must be done in a thread safe manner * chore: fix linter issues Signed-off-by: pashakostohrys <pavel@codefresh.io> --------- Signed-off-by: jannfis <jann@mistrust.net> Signed-off-by: pashakostohrys <pavel@codefresh.io> Co-authored-by: jannfis <jann@mistrust.net> Co-authored-by: pashakostohrys <pavel@codefresh.io>	2024-03-18 03:59:06 -04:00
pasha-codefresh	6e181d72b3	Merge pull request from GHSA-2vgg-9h6w-m454 * feat: pick random user and exclude admin user and current user from deletion candidates Signed-off-by: pashakostohrys <pavel@codefresh.io> * feat: increase default max cache size Signed-off-by: pashakostohrys <pavel@codefresh.io> * add nil protection Signed-off-by: pashakostohrys <pavel@codefresh.io> * Update util/session/sessionmanager.go Signed-off-by: Dan Garfield <dan@codefresh.io> Signed-off-by: Dan Garfield <dan@codefresh.io> * chore: fix linter issues Signed-off-by: pashakostohrys <pavel@codefresh.io> --------- Signed-off-by: pashakostohrys <pavel@codefresh.io> Signed-off-by: Dan Garfield <dan@codefresh.io> Co-authored-by: Dan Garfield <dan@codefresh.io>	2024-03-18 03:58:18 -04:00
gcp-cherry-pick-bot[bot]	c415d3f3d5	fix: registry argument to be only the host instead full URL (#17381 ) (#17535 ) Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br> Co-authored-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>	2024-03-15 17:32:21 -04:00
github-actions[bot]	759bd2888d	Bump version to 2.9.8 (#17514 ) Signed-off-by: GitHub <noreply@github.com> Co-authored-by: crenshaw-dev <crenshaw-dev@users.noreply.github.com>	2024-03-13 14:45:51 -04:00
Michael Crenshaw	2a410f3441	Merge pull request from GHSA-g623-jcgg-mhmm Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>	2024-03-13 14:28:43 -04:00
Alexandre Gaudreault	689a73a729	Merge pull request from GHSA-jwv5-8mqv-g387 * fix: Validate external URLs for applicatins Signed-off-by: Ry0taK <49341894+Ry0taK@users.noreply.github.com> * fix(ui): remove invalid external-link Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> * linting Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> --------- Signed-off-by: Ry0taK <49341894+Ry0taK@users.noreply.github.com> Signed-off-by: Alexandre Gaudreault <alexandre_gaudreault@intuit.com> Co-authored-by: Ry0taK <49341894+Ry0taK@users.noreply.github.com>	2024-03-13 14:26:47 -04:00
@@ -1 +1 @@
 .9.7
 .9.9