Repository navigation
Publish Helm chart to GHCR - #16276
Pavan19047 wants to merge 3 commits into
Conversation
Maffooch
left a comment
There was a problem hiding this comment.
Adversarial review: Publish Helm chart to GHCR
Verdict: request-changes
Reviewed at: aa0c4e4 Profile: code-change (release workflows + Helm README)
Repo rules followed: origin/dev .claude/skills/review-pr/SKILL.md and .claude/skills/defectdojo-dev/SKILL.md (Helm/infra checklist), plus the generic review-pr skill
Attempts to break it
- Permission scope of the new top-level block in
release-2-tag-docker-push.yml:17-19.gh api repos/DefectDojo/django-DefectDojo/actions/permissions/workflowreturnsdefault_workflow_permissions: write, so on base every job already haspackages: write. The new block adds nothing the helm job needs, but it appliespackages: writeto every job and called workflow (tag, docker publish, digests, tag-as-latest, release-drafter, fetch-oas) and sets every other scope tononefor all of them. zizmor (base vs PR diff) reports 2 new high-confidenceexcessive-permissionserrors at lines 18 and 19. Blocker. - Reusable-workflow permission ceiling. Release path: caller grants
contents: write, packages: write, callee job (release-x-manual-helm-chart.yml:43-45) asks for the same, so it starts. Nightly path (release-nightly-dev.yml->release-x-nightly.yml:75-82-> helm chart) has no permissions block, so the repo default (write) covers the callee request. No break. - Untrusted input in
run:. The new step (release-x-manual-helm-chart.yml:82-87) passes the token throughenvand--password-stdin, never on the command line.${{ env.chart_version }}is interpolated into the script; it comes fromChart.yamlat a maintainer-created tag, so not attacker-controlled, but zizmor flags it (help level) and"$chart_version"is the safe form. No break, nit. - Third-party actions. No new
uses:added. The helm CLI comes from the existing SHA-pinnedazure/setup-helm@9bc31f4e...(v5.0.1). No break. - OCI push target, hands-on. Ran the workflow's package step on the PR head (
helm dependency update,helm package, samels | cut | sedextraction) givingchart_version=1.9.56-dev, thenhelm push ./build/defectdojo-1.9.56-dev.tgz oci://localhost:35276/defectdojo --plain-httpagainst a throwawayregistry:2. Result:Pushed: localhost:35276/defectdojo/defectdojo:1.9.56-dev. So the real ref will beghcr.io/defectdojo/defectdojo:<version>, matching the README command. No break. - Unversioned install from the README. With only a
-devtag in the registry,helm show chart oci://.../defectdojo/defectdojofails with "could not locate a version matching provided version string". After pushing 1.9.55 as well, unversioned resolves 1.9.55 and--develresolves 1.9.56-dev. So the README command fails until the first stable release is pushed (and the package is public). Also, the nightly job will pushx.y.z-devcharts to the same public path every day. Follow-up / maintainer decision. - helm-docs fail-on-diff. Ran
helm-docs1.14.2 on a copy ofhelm/defectdojofrom the PR head: generatedREADME.mdis identical to the committed one. No break. artifacthub.io/changesannotation check (test-helm-chart.yml:78-102).ct list-changed --config ct.yaml --target-branch devlistshelm/defectdojo, so the check runs. The PR does not touchChart.yaml. The check would pass today only because the head is stale: the PR head annotation still holds a valkey entry from its merge base while origin/dev has"". After an update-branch the check fails. Precedent #16178 (same kind of README change) updated the annotation. Blocker per the repo Helm checklist.- Release ordering. The GHCR push now runs before "Create release" and "Update Helm repository index" with no failure isolation. If the first push fails (package not linked to the repo, org package policy, an existing
defectdojopackage owned elsewhere), the GitHub release asset, thehelm-chartsindex update, andrelease-drafter(whichneedsthis job) are all skipped. The nightly will surface it first, but it will also kill the nightly prerelease and index update. Not a blocker; recommend moving the push after the index update orcontinue-on-error: true. - actionlint on both workflows. Only SC2012/SC2086 (info) on the pre-existing package step (line 77); base reports the same 2. No break.
Blocking
.github/workflows/release-2-tag-docker-push.yml:17-19Drop the workflow-levelpermissionsblock. It is not needed (repo default is write) and it widenspackages: writeto every job while zeroing all other scopes for the whole release chain, includingrelease-drafter(reads PR labels; I did not verify whetherpull-requests: nonebreaks it on a public repo). If explicit permissions are wanted, putcontents: write+packages: writeon therelease-helm-chartjob only, andcontents: writeontag.helm/defectdojo/Chart.yamlAdd anartifacthub.io/changesentry for the chart change (README is packaged into the chart; see #16178). The CI check passes now only because the branch is behind dev.
Nice to have
release-x-manual-helm-chart.yml:82-87Make the GHCR push non-fatal or move it after the index update, so a GHCR setup problem cannot block a release.release-x-manual-helm-chart.yml:87Use"./build/defectdojo-${chart_version}.tgz"instead of${{ env.chart_version }}.helm/defectdojo/README.md.gotmpl:20-28(and README.md) The new paragraph sits between "published to thehelm-chartsbranch" and "To use it, you can add our repo", so "it" now reads as the OCI artifact. "remain available for existing installations" frames charts.defectdojo.com as legacy, which conflicts with #16178, and the OCI path bypasses that endpoint. Maintainer decision on positioning.- Use a plain or
bashfence instead ofzshto match the rest of the README. docs/content/get_started/open_source/installation.mdwas updated alongside the README in #16178; this PR does not mention OCI there.- No milestone set (AGENTS.md: every new PR gets the earliest open future milestone).
Verified hands-on
- Chart packaging, version extraction, and
helm pushto a local OCI registry produce<registry>/defectdojo/defectdojo:<version>(Helm v4.1.3). - Unversioned OCI install resolves only stable versions; fails when only
-devtags exist. - helm-docs regeneration matches the committed README.
ct list-changedflags the chart; annotation comparison done with yq against origin/dev.- actionlint and zizmor (offline), base vs PR.
Checked only statically
- GitHub Actions permission inheritance for reusable workflows on both the release and nightly paths.
- Token handling in the new step (env +
--password-stdin, no echo).
Still needs verification
- CI has not run: every pull_request workflow for this head is
action_required(fork, needs maintainer approval). Required checksruff-lintingandUnit Tests Completeare missing. - GHCR side, which needs org admin access: whether a
defectdojopackage already exists in the org (anonymous token request returned 403, inconclusive), whether Actions in this repo may create or write it, and its visibility. A new package is likely private until an admin makes it public (unverified), in which case the README command fails for users. - Whether
release-drafterworks withpull-requests: noneon this public repo.
|
This pull request contains multiple low-severity findings where the author 'Pavan19047' modified various sensitive codepaths in the 'dojo' directory without being on the allowed authors list. These issues are non-blocking and do not prevent the merge.
Configured Sensitive Codepath Modified by Non-Allowed Author in
|
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/api_v2/exception_handler.py' matches configured sensitive codepath pattern 'dojo/api_v2/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/apps.py (drs_f2fd01df)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/apps.py' matches configured sensitive codepath pattern 'dojo/apps.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/authorization/api_permissions.py (drs_9e6410e1)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/authorization/api_permissions.py' matches configured sensitive codepath pattern 'dojo/authorization/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/db_migrations/0299_finding_test_numsev_index.py (drs_74e282ca)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/db_migrations/0299_finding_test_numsev_index.py' matches configured sensitive codepath pattern 'dojo/db_migrations/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/db_migrations/0300_finding_upper_title_index.py (drs_1c9607b8)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/db_migrations/0300_finding_upper_title_index.py' matches configured sensitive codepath pattern 'dojo/db_migrations/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/endpoint/utils.py (drs_21f4a2d0)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/endpoint/utils.py' matches configured sensitive codepath pattern 'dojo/endpoint/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/finding/deduplication.py (drs_51a0ea29)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/finding/deduplication.py' matches configured sensitive codepath pattern 'dojo/finding/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/finding/helper.py (drs_a74a02c4)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/finding/helper.py' matches configured sensitive codepath pattern 'dojo/finding/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/finding/models.py (drs_18eda903)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/finding/models.py' matches configured sensitive codepath pattern 'dojo/finding/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/base_importer.py (drs_15b33e69)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/importers/base_importer.py' matches configured sensitive codepath pattern 'dojo/importers/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/default_importer.py (drs_78ba77e4)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/importers/default_importer.py' matches configured sensitive codepath pattern 'dojo/importers/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/default_reimporter.py (drs_a483662c)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/importers/default_reimporter.py' matches configured sensitive codepath pattern 'dojo/importers/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/import_lock.py (drs_6733e671)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/importers/import_lock.py' matches configured sensitive codepath pattern 'dojo/importers/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/importers/location_manager.py (drs_028bcb5d)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/importers/location_manager.py' matches configured sensitive codepath pattern 'dojo/importers/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/tasks.py (drs_52a1c8c5)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/tasks.py' matches configured sensitive codepath pattern 'dojo/tasks.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/templates/dojo/filter_js_snippet.html (drs_a747fef7)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/templates/dojo/filter_js_snippet.html' matches configured sensitive codepath pattern 'dojo/templates/**/*.html' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/templates/dojo/findings_list_snippet.html (drs_08690333)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/templates/dojo/findings_list_snippet.html' matches configured sensitive codepath pattern 'dojo/templates/**/*.html' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/templates/dojo/view_test.html (drs_ba5aab48)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/templates/dojo/view_test.html' matches configured sensitive codepath pattern 'dojo/templates/**/*.html' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/user/authentication.py (drs_ca2fb4c0)
| Vulnerability | Configured Sensitive Codepath Modified by Non-Allowed Author |
|---|---|
| Description | File 'dojo/user/authentication.py' matches configured sensitive codepath pattern 'dojo/user/*.py' and was modified by 'Pavan19047' (commit f20483c) who is not in the allowed authors list. |
Comment to provide feedback on these findings.
Report false positive: @dryrunsecurity fp [FINDING ID] [FEEDBACK]
Report low-impact: @dryrunsecurity nit [FINDING ID] [FEEDBACK]
Example: @dryrunsecurity fp drs_90eda195 This code is not user-facing
All finding details can be found in the DryRun Security Dashboard.
Drop the workflow-wide token permissions, publish the chart after the index update without failing the job, and record the README change for Artifact Hub.
Publishes each Helm release to GHCR as an OCI artifact and adds the install command to the chart README.
Closes #16019.
Tests: not run.