feat: improve Helm chart #12691

fernandezcuesta · 2025-06-25T11:41:32Z

Add extraInitContainers to celery+django deployments.
Add extraEnv to all deployments
Remove existing volume logic in favor of agnostic extraVolumes and extraVolumeMounts
Add the ability to deploy secrets as regular (non-hooked) resources due to issues found with ArgoCD
Make some secret references optional (secret might not be created)
Fix optional secret mounts + reference
Update bitnami chart reference (OCI)
Bump up redis chart

Reasoning behind the logic change for volumes, with the existing behaviour we cannot mount projected volumes (e.g. a secret mount where we want the key names being renamed) or per-container volumeMounts (e.g. nginx emptyDir when readOnlyRootFs is enforced).

- Add extraInitContainers to celery+django deployments. - Add extraEnv to all deployments - Remove existing volume logic in favor of agnostic extraVolumes and extraVolumeMounts - Fix optional secret mounts + reference - Update bitnami chart reference (OCI) - Bump up redis chart

dryrunsecurity · 2025-06-25T11:43:10Z

This pull request introduces multiple security regressions: it weakens Redis TLS by defaulting Celery broker params to ssl_cert_reqs=optional (allowing MitM), makes DD_SECRET_KEY and DD_CREDENTIAL_AES_256_KEY optional, and adds CI log output that could expose those secrets. It also permits arbitrary YAML/env injection via extraEnv/extra* Helm values so an attacker who controls chart values can override critical environment variables or inject malicious pod configuration.

Weak SSL/TLS Configuration for Celery Broker in helm/defectdojo/templates/configmap.yaml

Vulnerability	Weak SSL/TLS Configuration for Celery Broker
Description	The Helm chart sets `DD_CELERY_BROKER_PARAMS` to `ssl_cert_reqs=optional` by default when Redis TLS is enabled but no specific parameters are provided. This configuration allows the Celery client to connect to the Redis server over TLS without validating the server's certificate. This is a form of Improper Certificate Validation (CWE-295), which makes the connection vulnerable to Man-in-the-Middle (MitM) attacks.

django-DefectDojo/helm/defectdojo/templates/configmap.yaml

Lines 1 to 5 in 529c123

    
           {{- $fullName := include "defectdojo.fullname" . -}} 
        
           {{- $defaultBrokerParams := ternary "ssl_cert_reqs=optional" "" .Values.redis.tls.enabled -}} 
        
           apiVersion: v1 
        
           kind: ConfigMap 
        
           metadata:

Arbitrary Environment Variable Override in helm/defectdojo/templates/initializer-job.yaml

Vulnerability	Arbitrary Environment Variable Override
Description	The Helm chart templates for various components (initializer, Celery worker, Celery beat, Django) allow arbitrary environment variables to be injected via `extraEnv` fields. These `extraEnv` blocks are rendered after critical, explicitly-defined environment variables (such as `DD_SECRET_KEY`, `DD_CREDENTIAL_AES_256_KEY`, and `DD_DATABASE_PASSWORD`). Due to Kubernetes' environment variable precedence rules (last definition wins), an attacker with control over the Helm values can override these sensitive variables.

django-DefectDojo/helm/defectdojo/templates/initializer-job.yaml

Lines 145 to 161 in 529c123

    
               name: {{ $fullName }} 
        
           - secretRef: 
        
               name: {{ $fullName }} 
        
               optional: true 
        
           - secretRef: 
        
               name: {{ $fullName }}-extrasecrets 
        
               optional: true 
        
           env: 
        
           - name: DD_DATABASE_PASSWORD 
        
             valueFrom: 
        
               secretKeyRef: 
        
                 name: {{ .Values.postgresql.auth.existingSecret }} 
        
                 key: {{ .Values.postgresql.auth.secretKeys.userPasswordKey }} 
        
           {{- with .Values.initializer.extraEnv }} 
        
           {{- toYaml . | nindent 8 }} 
        
           {{- end }} 
        
           resources:

Missing Critical Secrets in helm/defectdojo/templates/django-deployment.yaml

Vulnerability	Missing Critical Secrets
Description	The Helm chart for DefectDojo now allows the `DD_SECRET_KEY` and `DD_CREDENTIAL_AES_256_KEY` environment variables to be optional in the `django-deployment.yaml` file. The `DD_SECRET_KEY` is analogous to Django's `SECRET_KEY`, which is fundamental for cryptographic signing, session integrity, and CSRF protection. If this key is missing, Django's security mechanisms are severely compromised. The `DD_CREDENTIAL_AES_256_KEY` is likely used for encrypting sensitive credentials. Making this optional could lead to credentials being stored or transmitted in an unencrypted or weakly encrypted manner.

django-DefectDojo/helm/defectdojo/templates/django-deployment.yaml

Lines 198 to 219 in 529c123

    
               secretKeyRef: 
        
                 name: {{ $fullName }} 
        
                 key: DD_SECRET_KEY 
        
                 optional: true 
        
           - name: DD_CREDENTIAL_AES_256_KEY 
        
             valueFrom: 
        
               secretKeyRef: 
        
                 name: {{ $fullName }} 
        
                 key: DD_CREDENTIAL_AES_256_KEY 
        
                 optional: true 
        
           - name: DD_SESSION_COOKIE_SECURE 
        
             value: {{- if or .Values.django.ingress.activateTLS .Values.django.nginx.tls.enabled }} "True" {{- else }} "False" {{- end }} 
        
           - name: DD_CSRF_COOKIE_SECURE 
        
             value: {{- if or .Values.django.ingress.activateTLS .Values.django.nginx.tls.enabled }} "True" {{- else }} "False" {{- end }} 
        
           {{- with .Values.extraEnv }} 
        
             {{- . | toYaml | nindent 8 }} 
        
           {{- end }} 
        
           {{- with .Values.django.uwsgi.extraEnv }} 
        
             {{- . | toYaml | nindent 8 }} 
        
           {{- end }} 
        
           {{- if .Values.django.uwsgi.livenessProbe.enabled }} 
        
           livenessProbe:

Increased Attack Surface via Flexible Configuration in helm/defectdojo/values.yaml

Vulnerability	Increased Attack Surface via Flexible Configuration
Description	The Helm chart introduces new configuration options (`extraEnv`, `extraInitContainers`, `extraVolumes`, `livenessProbe`, `readinessProbe`, `startupProbe`) for Celery Beat and Worker components. These options are rendered directly into the Kubernetes pod specification using `toYaml`, allowing for arbitrary YAML injection. This enables an attacker who can control the Helm values to inject malicious configurations, potentially leading to privilege escalation, command injection, or information disclosure.

django-DefectDojo/helm/defectdojo/values.yaml

Lines 148 to 184 in 529c123

    
           # Components 
        
           celery: 
        
             broker: redis 
        
             logLevel: INFO 
        
             # Common annotations to worker and beat deployments and pods. 
        
             annotations: {} 
        
             beat: 
        
               # Annotations for the Celery beat deployment. 
        
               annotations: {} 
        
               affinity: {} 
        
               # Additional environment variables injected to Celery beat containers. 
        
               extraEnv: [] 
        
               # A list of additional initContainers to run before celery beat containers. 
        
               extraInitContainers: [] 
        
               # Array of additional volume mount points for the celery beat containers. 
        
               extraVolumeMounts: [] 
        
               # A list of extra volumes to mount 
        
               # @type: array<map> 
        
               extraVolumes: [] 
        
               # Enable liveness probe for Celery beat container. 
        
               livenessProbe: {} 
        
                 # exec: 
        
                 #   command: 
        
                 #     - bash 
        
                 #     - -c 
        
                 #     - celery -A dojo inspect ping -t 5 
        
                 # initialDelaySeconds: 30 
        
                 # periodSeconds: 60 
        
                 # timeoutSeconds: 10 
        
               nodeSelector: {} 
        
               # Annotations for the Celery beat pods. 
        
               podAnnotations: {} 
        
               # Enable readiness probe for Celery beat container. 
        
               readinessProbe: {} 
        
               replicas: 1 
        
               resources: 
        
                 requests:

Information Disclosure (Sensitive Logs) in .github/workflows/k8s-tests.yml

Vulnerability	Information Disclosure (Sensitive Logs)
Description	The CI/CD workflow in `.github/workflows/k8s-tests.yml` now logs the last 30 lines of the `uwsgi` container's output on test failure. The `uwsgi` container is configured with sensitive environment variables, specifically `DD_SECRET_KEY` and `DD_CREDENTIAL_AES_256_KEY`. In the event of an unhandled exception or error within the Django application, a detailed stack trace or error message containing these sensitive environment variables could be logged to standard output/error. Since the repository is public, these logs would be publicly accessible via GitHub Actions, leading to the disclosure of critical secrets.

django-DefectDojo/.github/workflows/k8s-tests.yml

Lines 132 to 146 in 529c123

    
                               --max-time 20 \ 
        
                               --head \ 
        
                               --header "Host: $DD_HOSTNAME" \ 
        
                               "http://${DJANGO_IP}/login?next=/") 
        
                          echo $OUT 
        
                          CR=$(echo $OUT | egrep "^HTTP" | cut -d' ' -f2) 
        
                          echo $CR 
        
                          if [[ $CR -ne 200 ]]; then 
        
                             echo $RETRY 
        
                             if [[ $RETRY -gt 2 ]]; then 
        
                               kubectl get pods 
        
                               echo $(kubectl logs --tail=30 -l defectdojo.org/component=django -c uwsgi) 
        
                               echo "ERROR: cannot display login screen; got HTTP code $CR" 
        
                               exit 1 
        
                             else

All finding details can be found in the DryRun Security Dashboard.

github-actions · 2025-06-25T18:20:11Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

valentijnscholten · 2025-06-25T19:49:26Z

@fernandezcuesta Thanks for the PR, we'll ask some helm/k8s experienced devs to look at it. In the mean time could you look at the conflicts? Also this seems to be a bigger change, could you look at basing it against the dev branch

github-actions · 2025-06-26T06:28:52Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

fernandezcuesta · 2025-06-26T06:30:47Z

@valentijnscholten done, thanks for having a look at it! I also changed the base to dev branch.

kiblik

I will need more time for the rest (this is not my final review).

helm/defectdojo/Chart.lock

helm/defectdojo/Chart.yaml

helm/defectdojo/values.yaml

github-actions · 2025-08-21T11:51:24Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

fernandezcuesta · 2025-08-21T12:04:41Z

@kiblik Thanks for your detailed review and suggestions. I tried to go through all, but have some doubts about one of them (see inline comments).

kiblik

Many of the issues have been solved; some small ones are still open.
However, I'm still missing some info in the release notes
Mainly parts that have potential side-effects (rename annotations ot podAnnotations in celery)

helm/defectdojo/Chart.lock

helm/defectdojo/templates/configmap.yaml

kiblik · 2025-08-26T14:29:01Z

helm/defectdojo/values.yaml

-  # To use an external Redis instance, set enabled to false and uncomment
-  # the line below:
-  # redisServer: myrediscluster


Hmmm... I'm just checking _helpers.tpl and I'm thinking what is the right approach here 🤔.

django-DefectDojo/helm/defectdojo/templates/_helpers.tpl

Lines 45 to 67 in 941dd6c

{{/*

Determine the hostname to use for PostgreSQL/Redis.

*/}}

{{- define "postgresql.hostname" -}}

{{- if .Values.postgresql.enabled -}}

{{- if eq .Values.postgresql.architecture "replication" -}}

{{- printf "%s-%s-%s" .Release.Name "postgresql" .Values.postgresql.primary.name | trunc 63 | trimSuffix "-" -}}

{{- else -}}

{{- printf "%s-%s" .Release.Name "postgresql" | trunc 63 | trimSuffix "-" -}}

{{- end -}}

{{- else -}}

{{- printf "%s" ( .Values.postgresql.postgresServer | default "127.0.0.1" ) -}}

{{- end -}}

{{- end -}}

{{- define "redis.hostname" -}}

{{- if eq .Values.celery.broker "redis" -}}

{{- if .Values.redis.enabled -}}

{{- printf "%s-%s" .Release.Name "redis-master" | trunc 63 | trimSuffix "-" -}}

{{- else -}}

{{- printf "%s" (.Values.celery.brokerHost | default .Values.redis.redisServer) -}}

{{- end -}}

{{- end -}}

{{- end -}}

Postgres and Redis are both essential parts for DD.

They might be used as part of the stack, or both can be used aaS or any combination between

Postgres is used by all Django components (uwsgi, celery, initializer), same for Redis (well, except initializer, but definitely not only celery)

The following two statements should use the same conventions (they do not follow right now)

{{- printf "%s" ( .Values.postgresql.postgresServer | default "127.0.0.1" ) -}} {{- printf "%s" (.Values.celery.brokerHost | default .Values.redis.redisServer) -}}

helm/defectdojo/templates/tests/unit-tests.yaml

github-actions · 2025-08-27T07:13:30Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2025-08-27T07:18:35Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

kiblik

Just a small issue - I would drop the Bitnami reference from the release notes (see the context)

And TBH, I would probably change the chart version to 1.7.1 (it needs to be 1.7.1-dev, the suffix will be removed during releasing).
Because this is a radical change, and it should be noticeable in the version number as well. It not only adds value, but it might also affect existing deployments.

Huge thanks, this PR is a large cleanup and makes the chart more flexible and easier to read and use.

docs/content/en/open_source/upgrading/2.50.md

kiblik · 2025-08-27T11:44:33Z

helm/defectdojo/templates/_helpers.tpl

@@ -53,15 +53,16 @@ Create the name of the service account to use
 {{-   printf "%s-%s" .Release.Name "postgresql" | trunc 63 | trimSuffix "-" -}}
 {{-  end -}}
 {{- else -}}
-{{-  printf "%s" ( .Values.postgresql.postgresServer | default "127.0.0.1" ) -}}
+{{- .Values.postgresServer | default "127.0.0.1" | quote -}}


CC @lchastel
JFYI, your #12965 has not been released yet, but it will probably change the format before it is officially released. Please see #12691 (comment) for full context.

kiblik · 2025-08-27T11:48:28Z

helm/defectdojo/values.yaml

-  # To use an external Redis instance, set enabled to false and uncomment
-  # the line below:
-  # redisServer: myrediscluster


I really like this separation. I was planning to add values.schema.json (see #12984) to avoid wrong values. And I wasn't sure what to do with the overlap with subchart values. As soon as we separate them, it is much easier to adopt them.

Co-authored-by: kiblik <5609770+kiblik@users.noreply.github.com>

github-actions · 2025-09-02T14:40:14Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2025-09-02T20:29:59Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

kiblik · 2025-09-03T15:56:24Z

I really hoped this will be part of 2.50.0
As it wasn't merged before release, changelog has to be moved to 2.51.md file.

fernandezcuesta · 2025-09-04T08:19:46Z

I really hoped this will be part of 2.50.0 As it wasn't merged before release, changelog has to be moved to 2.51.md file.

Done!

Maffooch

Apologies for missing this one folks 😅

valentijnscholten · 2025-09-05T19:34:53Z

@kiblik I merged this as it looks it's still valid and doesn't conflict with the vendoring of the bitnami charts we did last week?

fernandezcuesta requested review from Maffooch and mtesauro as code owners June 25, 2025 11:41

github-actions bot added the helm label Jun 25, 2025

fernandezcuesta changed the title ~~**Summary:**~~ feat: improve Helm chart Jun 25, 2025

chore: add livenessProbe entries for celery

475ed17

Maffooch changed the base branch from master to bugfix June 25, 2025 17:47

github-actions bot added the conflicts-detected label Jun 25, 2025

fix: reference to removed field

6b18b96

fernandezcuesta changed the base branch from bugfix to dev June 26, 2025 06:28

fix: conflict

4165ca8

github-actions bot removed the conflicts-detected label Jun 26, 2025

fernandezcuesta added 4 commits June 26, 2025 08:32

chore: add reference to upstream chart

cbf8778

fix: missing default values from upstream chart used in templates

67d6348

chore: rephrase

b34e833

feat: allow deploy secret as regular non-hooked resources

330a127

Maffooch requested a review from rossops June 26, 2025 16:07

kiblik requested changes Jun 27, 2025

View reviewed changes

valentijnscholten added this to the 2.49.0 milestone Jun 29, 2025

fernandezcuesta added 3 commits June 30, 2025 09:19

fix: review

b474297

chore: restore Chart.lock

5c2f804

chore: update chart.lock

fba0cd0

rossops approved these changes Jul 7, 2025

View reviewed changes

fernandezcuesta requested a review from kiblik July 23, 2025 11:49

chore: wrap services url

108e7b4

chore: mount extraVolumes in initContainers too

3f502ad

fernandezcuesta requested a review from kiblik August 21, 2025 12:05

kiblik requested changes Aug 26, 2025

View reviewed changes

chore: move external db values to separate fields, add release notes

4f07742

github-actions bot added the docs label Aug 27, 2025

github-actions bot added the conflicts-detected label Aug 27, 2025

Merge branch 'dev' into feat/improve-helm-chart

77511d2

github-actions bot removed the conflicts-detected label Aug 27, 2025

kiblik requested changes Aug 27, 2025

View reviewed changes

fernandezcuesta and others added 2 commits August 28, 2025 07:15

Update docs/content/en/open_source/upgrading/2.50.md

25ebe6a

Co-authored-by: kiblik <5609770+kiblik@users.noreply.github.com>

chore: bump chart version and remove bitnami dependency comment from RN

596b0c9

kiblik approved these changes Aug 28, 2025

View reviewed changes

github-actions bot added the conflicts-detected label Sep 2, 2025

valentijnscholten modified the milestones: 2.50.0, 2.51.0 Sep 2, 2025

Merge branch 'dev' into feat/improve-helm-chart

6bacb88

github-actions bot removed the conflicts-detected label Sep 2, 2025

fernandezcuesta added 2 commits September 4, 2025 10:14

chore: move release notes to 2.51

08f1be7

chore: restore 2.50.md

529c123

Maffooch approved these changes Sep 5, 2025

View reviewed changes

valentijnscholten merged commit 43434d6 into DefectDojo:dev Sep 5, 2025
85 checks passed

	{{/*
	Determine the hostname to use for PostgreSQL/Redis.
	*/}}
	{{- define "postgresql.hostname" -}}
	{{- if .Values.postgresql.enabled -}}
	{{- if eq .Values.postgresql.architecture "replication" -}}
	{{- printf "%s-%s-%s" .Release.Name "postgresql" .Values.postgresql.primary.name \| trunc 63 \| trimSuffix "-" -}}
	{{- else -}}
	{{- printf "%s-%s" .Release.Name "postgresql" \| trunc 63 \| trimSuffix "-" -}}
	{{- end -}}
	{{- else -}}
	{{- printf "%s" ( .Values.postgresql.postgresServer \| default "127.0.0.1" ) -}}
	{{- end -}}
	{{- end -}}
	{{- define "redis.hostname" -}}
	{{- if eq .Values.celery.broker "redis" -}}
	{{- if .Values.redis.enabled -}}
	{{- printf "%s-%s" .Release.Name "redis-master" \| trunc 63 \| trimSuffix "-" -}}
	{{- else -}}
	{{- printf "%s" (.Values.celery.brokerHost \| default .Values.redis.redisServer) -}}
	{{- end -}}
	{{- end -}}
	{{- end -}}

feat: improve Helm chart #12691

feat: improve Helm chart #12691

Uh oh!

Conversation

fernandezcuesta commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dryrunsecurity bot commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 25, 2025

Uh oh!

valentijnscholten commented Jun 25, 2025

Uh oh!

github-actions bot commented Jun 26, 2025

Uh oh!

fernandezcuesta commented Jun 26, 2025

Uh oh!

kiblik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 21, 2025

Uh oh!

fernandezcuesta commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kiblik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kiblik Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Aug 27, 2025

Uh oh!

github-actions bot commented Aug 27, 2025

Uh oh!

kiblik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kiblik Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

kiblik Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 2, 2025

Uh oh!

github-actions bot commented Sep 2, 2025

Uh oh!

kiblik commented Sep 3, 2025

Uh oh!

fernandezcuesta commented Sep 4, 2025

Uh oh!

Maffooch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

valentijnscholten commented Sep 5, 2025

Uh oh!

Uh oh!

fernandezcuesta commented Jun 25, 2025 •

edited

Loading

dryrunsecurity bot commented Jun 25, 2025 •

edited

Loading

fernandezcuesta commented Aug 21, 2025 •

edited

Loading