Skip to content

Fossa 3.14.0#5

Merged
rchenzheng merged 228 commits intofossafrom
fossa-3.14.0
Feb 24, 2026
Merged

Fossa 3.14.0#5
rchenzheng merged 228 commits intofossafrom
fossa-3.14.0

Conversation

@rchenzheng
Copy link

@rchenzheng rchenzheng commented Feb 23, 2026

  • Merges upstream 3.14.0 with helm 3.20.0 and kubectl 1.34.4

Tested using fossa-3.14.0 https://github.com/fossas/helm/pull/794

dependabot bot and others added 30 commits April 14, 2023 00:08
Bumps [sigstore/cosign-installer](https://github.com/sigstore/cosign-installer) from 3.0.1 to 3.0.2.
- [Release notes](https://github.com/sigstore/cosign-installer/releases)
- [Commits](sigstore/cosign-installer@c3667d9...9e9de22)

---
updated-dependencies:
- dependency-name: sigstore/cosign-installer
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/spf13/cobra](https://github.com/spf13/cobra) from 1.6.1 to 1.7.0.
- [Release notes](https://github.com/spf13/cobra/releases)
- [Commits](spf13/cobra@v1.6.1...v1.7.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/cobra
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [anchore/sbom-action](https://github.com/anchore/sbom-action) from 0.13.4 to 0.14.1.
- [Release notes](https://github.com/anchore/sbom-action/releases)
- [Commits](anchore/sbom-action@448520c...422cb34)

---
updated-dependencies:
- dependency-name: anchore/sbom-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* feat: verify if targetBranch is present in git repo

If a targetBranch is not present in the git repository the `ct list-changed` command will fail.
The error message for this is not really meaningful.

This commit will validate if the branch exists and throw a error message which the user will understand.

closes helm#330

Signed-off-by: Marco Lecheler <marco@task.media>

* chore(test): mocking test for chart.BranchExists()

Signed-off-by: Marco Lecheler <marco@task.media>

* fix(lint): use fmt.Errorf for error msg

Signed-off-by: Marco Lecheler <marco@task.media>

---------

Signed-off-by: Marco Lecheler <marco@task.media>
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.5.0 to 3.5.2.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@8f4b7f8...8e5e7e5)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [sigstore/cosign-installer](https://github.com/sigstore/cosign-installer) from 3.0.2 to 3.0.3.
- [Release notes](https://github.com/sigstore/cosign-installer/releases)
- [Commits](sigstore/cosign-installer@9e9de22...204a51a)

---
updated-dependencies:
- dependency-name: sigstore/cosign-installer
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [anchore/sbom-action](https://github.com/anchore/sbom-action) from 0.14.1 to 0.14.2.
- [Release notes](https://github.com/anchore/sbom-action/releases)
- [Commits](anchore/sbom-action@422cb34...4d571ad)

---
updated-dependencies:
- dependency-name: anchore/sbom-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 4.0.0 to 4.0.1.
- [Release notes](https://github.com/actions/setup-go/releases)
- [Commits](actions/setup-go@4d34df0...fac708d)

---
updated-dependencies:
- dependency-name: actions/setup-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps alpine from 3.17 to 3.18.

---
updated-dependencies:
- dependency-name: alpine
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.8.2 to 1.8.3.
- [Release notes](https://github.com/stretchr/testify/releases)
- [Commits](stretchr/testify@v1.8.2...v1.8.3)

---
updated-dependencies:
- dependency-name: github.com/stretchr/testify
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [helm/kind-action](https://github.com/helm/kind-action) from 1.5.0 to 1.7.0.
- [Release notes](https://github.com/helm/kind-action/releases)
- [Commits](helm/kind-action@d8ccf8f...fa81e57)

---
updated-dependencies:
- dependency-name: helm/kind-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [sigstore/cosign-installer](https://github.com/sigstore/cosign-installer) from 3.0.3 to 3.0.5.
- [Release notes](https://github.com/sigstore/cosign-installer/releases)
- [Commits](sigstore/cosign-installer@204a51a...dd6b2e2)

---
updated-dependencies:
- dependency-name: sigstore/cosign-installer
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.15.0 to 1.16.0.
- [Release notes](https://github.com/spf13/viper/releases)
- [Commits](spf13/viper@v1.15.0...v1.16.0)

---
updated-dependencies:
- dependency-name: github.com/spf13/viper
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 3.4.0 to 3.5.0.
- [Release notes](https://github.com/golangci/golangci-lint-action/releases)
- [Commits](golangci/golangci-lint-action@08e2f20...5f1fec7)

---
updated-dependencies:
- dependency-name: golangci/golangci-lint-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.8.3 to 1.8.4.
- [Release notes](https://github.com/stretchr/testify/releases)
- [Commits](stretchr/testify@v1.8.3...v1.8.4)

---
updated-dependencies:
- dependency-name: github.com/stretchr/testify
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
)

Bumps [github.com/hashicorp/go-retryablehttp](https://github.com/hashicorp/go-retryablehttp) from 0.7.2 to 0.7.4.
- [Changelog](https://github.com/hashicorp/go-retryablehttp/blob/main/CHANGELOG.md)
- [Commits](hashicorp/go-retryablehttp@v0.7.2...v0.7.4)

---
updated-dependencies:
- dependency-name: github.com/hashicorp/go-retryablehttp
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [docker/login-action](https://github.com/docker/login-action) from 2.1.0 to 2.2.0.
- [Release notes](https://github.com/docker/login-action/releases)
- [Commits](docker/login-action@f4ef78c...465a078)

---
updated-dependencies:
- dependency-name: docker/login-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 3.5.0 to 3.6.0.
- [Release notes](https://github.com/golangci/golangci-lint-action/releases)
- [Commits](golangci/golangci-lint-action@5f1fec7...639cd34)

---
updated-dependencies:
- dependency-name: golangci/golangci-lint-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 3.5.2 to 3.5.3.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@8e5e7e5...c85c95e)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [docker/setup-qemu-action](https://github.com/docker/setup-qemu-action) from 2.1.0 to 2.2.0.
- [Release notes](https://github.com/docker/setup-qemu-action/releases)
- [Commits](docker/setup-qemu-action@e81a89b...2b82ce8)

---
updated-dependencies:
- dependency-name: docker/setup-qemu-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [goreleaser/goreleaser-action](https://github.com/goreleaser/goreleaser-action) from 4.2.0 to 4.3.0.
- [Release notes](https://github.com/goreleaser/goreleaser-action/releases)
- [Commits](goreleaser/goreleaser-action@f82d6c1...336e299)

---
updated-dependencies:
- dependency-name: goreleaser/goreleaser-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 2.5.0 to 2.6.0.
- [Release notes](https://github.com/docker/setup-buildx-action/releases)
- [Commits](docker/setup-buildx-action@4b4e9c3...6a58db7)

---
updated-dependencies:
- dependency-name: docker/setup-buildx-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 2.6.0 to 2.7.0.
- [Release notes](https://github.com/docker/setup-buildx-action/releases)
- [Commits](docker/setup-buildx-action@6a58db7...ecf9528)

---
updated-dependencies:
- dependency-name: docker/setup-buildx-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Rafael Matias <rafael@skyle.net>
Bumps [sigstore/cosign-installer](https://github.com/sigstore/cosign-installer) from 3.0.5 to 3.1.0.
- [Release notes](https://github.com/sigstore/cosign-installer/releases)
- [Commits](sigstore/cosign-installer@dd6b2e2...d130283)

---
updated-dependencies:
- dependency-name: sigstore/cosign-installer
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [anchore/sbom-action](https://github.com/anchore/sbom-action) from 0.14.2 to 0.14.3.
- [Release notes](https://github.com/anchore/sbom-action/releases)
- [Commits](anchore/sbom-action@4d571ad...78fc58e)

---
updated-dependencies:
- dependency-name: anchore/sbom-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [sigstore/cosign-installer](https://github.com/sigstore/cosign-installer) from 3.1.0 to 3.1.1.
- [Release notes](https://github.com/sigstore/cosign-installer/releases)
- [Commits](sigstore/cosign-installer@d130283...6e04d22)

---
updated-dependencies:
- dependency-name: sigstore/cosign-installer
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [docker/setup-buildx-action](https://github.com/docker/setup-buildx-action) from 2.7.0 to 2.8.0.
- [Release notes](https://github.com/docker/setup-buildx-action/releases)
- [Commits](docker/setup-buildx-action@ecf9528...16c0bc4)

---
updated-dependencies:
- dependency-name: docker/setup-buildx-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: cpanato <ctadeu@gmail.com>
@rchenzheng rchenzheng marked this pull request as ready for review February 23, 2026 21:19
@coderabbitai
Copy link

coderabbitai bot commented Feb 23, 2026

Walkthrough

The PR upgrades CI workflows and action versions, pins Go in workflows, bumps the Go toolchain and numerous dependencies (go.mod → 1.24.0), and updates the Dockerfile and example image tags. It adds helmignore support (pkg/ignore), new config fields (SkipHelmDependencies, HelmExtraSetArgs, HelmLintExtraArgs, ReleaseName, GithubGroups, UseHelmignore), changes several CLI flags and docs, and refactors chart/helm APIs and Testing constructors (NewTesting signature, Helm.Upgrade → UpgradeWithValues, Chart.CreateInstallParams accepts releaseName). Tests and utilities adjusted accordingly.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.94% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Fossa 3.14.0' is vague and does not clearly convey the main changes in the pull request, which involves upgrading to version 3.14.0, integrating Helm 3.20.0, and updating kubectl to 1.34.4. Revise the title to be more descriptive, such as 'Upgrade to 3.14.0 with Helm 3.20.0 and kubectl 1.34.4' or 'Update dependencies: Helm 3.20.0, kubectl 1.34.4, and chart-testing 3.14.0'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description is related to the changeset and clearly explains the main objectives: merging upstream 3.14.0 with Helm 3.20.0 and kubectl 1.34.4, plus adding Git configuration for compatibility with Git 2.47+.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
pkg/exec/exec.go (1)

92-103: ⚠️ Potential issue | 🟠 Major

Avoid goroutine/FD leak when cmd.Start() fails.

With the Close calls moved inside the goroutine, a cmd.Start() failure leaves the scanner goroutine blocked on Scan() because the pipe writers remain open, leaking goroutines and FDs. Start the goroutine only after a successful Start() and close the pipes on the error path.

🔧 Proposed fix
 scanner := bufio.NewScanner(io.MultiReader(outReader, errReader))
-go func() {
-	defer outReader.Close()
-	defer errReader.Close()
-	for scanner.Scan() {
-		fmt.Println(scanner.Text())
-	}
-}()
-
 err = cmd.Start()
 if err != nil {
+	_ = outReader.Close()
+	_ = errReader.Close()
 	return fmt.Errorf("failed running process: %w", err)
 }
+
+go func() {
+	defer outReader.Close()
+	defer errReader.Close()
+	for scanner.Scan() {
+		fmt.Println(scanner.Text())
+	}
+}()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/exec/exec.go` around lines 92 - 103, The scanner goroutine is started
before cmd.Start(), causing it to block and leak when cmd.Start() fails because
pipe writers remain open; change the flow so you call cmd.Start() first and only
launch the goroutine (that uses scanner, outReader, errReader) after Start
succeeds, and on the cmd.Start() error path explicitly close outReader and
errReader to release FDs and unblock the scanner if needed; ensure the goroutine
still defers closing outReader and errReader when it runs and that error
handling around cmd.Start() closes the readers before returning the wrapped
error.
build.sh (1)

64-66: ⚠️ Potential issue | 🟠 Major

Replace --debug with --verbose for goreleaser v2 compatibility.

--debug was removed in GoReleaser v2.0 (released May 26, 2024). The correct replacement is --verbose. While this line is pre-existing, the --skip=sign on line 67 and the context of this PR target goreleaser v2, making this a breaking issue: invoking build.sh -d will fail on a v2 goreleaser binary.

Proposed fix
     if [[ -n "$debug" ]]; then
-        goreleaser_args+=(--debug)
+        goreleaser_args+=(--verbose)
         set -x
     fi

Also update the help text in the README and build.sh comments to reflect this change.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@build.sh` around lines 64 - 66, The script uses goreleaser_args+=(--debug)
when the debug flag is set, but GoReleaser v2 removed --debug; update the
invocation to goreleaser_args+=(--verbose) and keep set -x for shell tracing;
also update the help text strings and any build.sh comments and README
references that mention "--debug" or the debug flag to say "--verbose" (or
"verbose") so the documented flag and the goreleaser_args handling in build.sh
remain consistent with GoReleaser v2; ensure you update the occurrence that
checks the debug variable and any related usage of goreleaser_args.
.github/workflows/ci.yaml (1)

141-147: ⚠️ Potential issue | 🟠 Major

Add explicit permissions for dependency-scan.

Top-level permissions: {} removes default token access. Without a job-level override, actions/checkout in dependency-scan may fail due to missing contents: read.

✅ Suggested fix
 dependency-scan:
   name: dependency-scan
   runs-on: ubuntu-latest
+  permissions:
+    contents: read

   steps:
     - uses: actions/checkout@v2
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yaml around lines 141 - 147, The dependency-scan job
currently runs with top-level empty permissions which removes the default token
scopes, causing actions/checkout@v2 to fail; update the dependency-scan job
definition (job name "dependency-scan") to include a job-level permissions block
granting at least contents: read so the checkout action can access the
repository (e.g., add permissions: { contents: read } under the dependency-scan
job).
doc/ct_list-changed.md (1)

7-8: ⚠️ Potential issue | 🟡 Minor

Stray double quotes in the synopsis text.

Lines 7 and 8 have leading " characters that appear as literal quotes in the rendered Markdown. These look like artifacts from a Go heredoc string that weren't cleaned up during doc generation.

Proposed fix
-"List changed charts based on configured charts directories,
-"remote, and target branch
+List changed charts based on configured charts directories,
+remote, and target branch
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@doc/ct_list-changed.md` around lines 7 - 8, Remove the stray leading
double-quote characters from the synopsis lines so the rendered Markdown no
longer shows literal quotes; specifically edit the lines containing "List
changed charts based on configured charts directories, and "remote, and target
branch" (the two lines starting with a double-quote) and remove the leading "
characters so the sentence reads normally as plain text in ct_list-changed.md.
pkg/chart/chart.go (1)

211-229: ⚠️ Potential issue | 🟠 Major

releaseName parameter is ignored for charts in subdirectories — this is a bug.

The releaseName parameter is only checked when filepath.Base(chart.Path()) equals "." or "/" (line 213). For any chart in a subdirectory (e.g., charts/myapp), the parameter is silently ignored. This contradicts the CLI documentation (ct/cmd/install.go), which states: "Name for the release. If not specified, is set to the chart name and a random identifier"—implying the override should apply whenever specified.

For example, running ct install --release-name "production" --charts charts/myapp produces release = "myapp-<random>" instead of the expected "production-<random>".

Suggested fix
 func (c *Chart) CreateInstallParams(buildID string, releaseName string) (release string, namespace string) {
-	release = filepath.Base(c.Path())
-	if release == "." || release == "/" {
-		if releaseName != "" {
-			release = releaseName
-		} else {
+	if releaseName != "" {
+		release = releaseName
+	} else {
+		release = filepath.Base(c.Path())
+		if release == "." || release == "/" {
 			yaml := c.Yaml()
 			release = yaml.Name
 		}
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/chart/chart.go` around lines 211 - 229, The CreateInstallParams method
ignores the releaseName argument for charts in subdirectories; update
Chart.CreateInstallParams so that after deriving the default release (using
filepath.Base(c.Path()) and falling back to c.Yaml().Name when base is "." or
"/"), you override that value if releaseName != "" (i.e., set release =
releaseName), then proceed to build namespace, append buildID and randomSuffix,
and sanitize via util.SanitizeName with maxNameLength; reference c.Path(),
c.Yaml(), releaseName, util.RandomString, util.SanitizeName, and maxNameLength
when making the change.
🧹 Nitpick comments (5)
pkg/ignore/ignore.go (1)

40-44: Normalize input paths before building MapFS.

If files contains OS-specific separators (e.g., \ on Windows), fstest.MapFS expects /-separated paths and ignore matching can miss. Normalizing before insertion makes the behavior consistent across platforms.

♻️ Suggested tweak
 func FilterFiles(files []string, rules *helmignore.Rules) ([]string, error) {
 	fsys := fstest.MapFS{}
 	for _, file := range files {
-		fsys[file] = &fstest.MapFile{}
+		file = filepath.ToSlash(file)
+		fsys[file] = &fstest.MapFile{}
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/ignore/ignore.go` around lines 40 - 44, Normalize input file paths before
inserting into the fstest.MapFS in FilterFiles: for each entry in the files
slice (in the FilterFiles function) apply filepath.Clean followed by
filepath.ToSlash (or equivalent) so MapFS gets POSIX-style paths and helmignore
matching is consistent across OSes; update the loop that writes to fstest.MapFS
(the fsys[file] = &fstest.MapFile{} block) to use the normalized path variable
instead of the original file string.
README.md (1)

158-158: Stale Go version reference.

Line 158 states `ct` is built using Go 1.13 or higher but the PR updates CI to use Go 1.25. While this line isn't part of the current diff, since this PR is updating version references throughout, it would be a good opportunity to correct this.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 158, Update the stale Go version statement in README.md
that currently reads "`ct` is built using Go 1.13 or higher" to reflect the CI
change to Go 1.25; locate the exact string in README.md and replace "Go 1.13"
with "Go 1.25" (or a range like "Go 1.25 or higher") so the documentation
matches the updated CI configuration.
ct/cmd/root.go (1)

68-68: Default target branch updated from "master" to "main".

Reasonable change aligning with current industry convention. Note this is a behavior change for users relying on the default — existing CI pipelines that depend on the old "master" default without explicitly setting --target-branch will break silently (no error, just no changed charts detected if "master" doesn't exist).

Consider mentioning this default change prominently in the release notes or migration guide, as it could cause silent failures for users who haven't migrated their default branch to "main".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ct/cmd/root.go` at line 68, The default for the target branch was changed in
the flags.String("target-branch", "main", ...) call which can silently break
users still on "master"; either (A) revert to "master" or (B) keep "main" but
add a clear migration notice: update release notes and the CLI help/usage text
to call out the default change, and add a runtime warning when the
configured/default target branch does not exist (detect in the code path that
uses the target branch and log a warning referencing the
flags.String("target-branch", ...) flag name and its value). Ensure the release
notes/migration guide explicitly mention the default change from "master" to
"main".
pkg/chart/chart_test.go (1)

156-175: Duplicated helmignore rule parsing logic in test stub.

The loadRules stub has identical parsing logic for "test_charts/foo" and "test_chart_at_multi_level/foo/baz". Consider consolidating with a helper or a set-based lookup to reduce duplication.

Suggested consolidation
 		loadRules: func(dir string) (*helmignore.Rules, error) {
-			rules := helmignore.Empty()
-			if dir == "test_charts/foo" {
-				var err error
-				rules, err = helmignore.Parse(strings.NewReader("Chart.yaml\n"))
-				if err != nil {
-					return nil, err
-				}
-				rules.AddDefaults()
-			}
-			if dir == "test_chart_at_multi_level/foo/baz" {
-				var err error
-				rules, err = helmignore.Parse(strings.NewReader("Chart.yaml\n"))
-				if err != nil {
-					return nil, err
-				}
-				rules.AddDefaults()
+			dirsWithIgnore := map[string]bool{
+				"test_charts/foo":                        true,
+				"test_chart_at_multi_level/foo/baz":      true,
+			}
+			if dirsWithIgnore[dir] {
+				rules, err := helmignore.Parse(strings.NewReader("Chart.yaml\n"))
+				if err != nil {
+					return nil, err
+				}
+				rules.AddDefaults()
+				return rules, nil
 			}
+			rules := helmignore.Empty()
 			return rules, nil
 		},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/chart/chart_test.go` around lines 156 - 175, The loadRules test stub
duplicates the helmignore.Parse + rules.AddDefaults logic for two directory
cases; refactor loadRules in pkg/chart/chart_test.go to consolidate by checking
membership (e.g., a map or slice lookup) for the directories "test_charts/foo"
and "test_chart_at_multi_level/foo/baz" and then performing
helmignore.Parse(strings.NewReader("Chart.yaml\n")) and rules.AddDefaults once,
returning the parsed rules; keep the same error handling and return rules, nil
for other dirs so behavior of loadRules, helmignore.Parse, and rules.AddDefaults
is preserved.
pkg/tool/account.go (1)

33-33: url local variable now shadows the newly imported net/url package.

Line 33 declares url := fmt.Sprintf(...), which shadows the net/url import introduced in this PR. It compiles fine because the package isn't used within Validate, but it's a latent hazard for future edits.

♻️ Suggested rename
-	url := fmt.Sprintf("https://%s/%s", domain, account)
-	response, err := http.Head(url) // nolint: gosec
+	accountURL := fmt.Sprintf("https://%s/%s", domain, account)
+	response, err := http.Head(accountURL) // nolint: gosec
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/tool/account.go` at line 33, The local variable url in the Validate
function shadows the imported net/url package; rename the local variable (for
example to accountURL or formattedURL) in the statement currently reading url :=
fmt.Sprintf("https://%s/%s", domain, account) and update any subsequent
references in Validate to that new name so the net/url package remains available
without collision.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/stale.yaml:
- Around line 19-26: The README/workflow has inconsistent PR close timing: the
string stale-pr-message says "closed ... in 10 days" while the
days-before-pr-close variable (days-before-pr-close) is set to 5; update
days-before-pr-close to 10 or change stale-pr-message to reflect 5 days so both
stale-pr-message and days-before-pr-close match. Locate the keys
stale-pr-message and days-before-pr-close in the stale workflow and make the
values consistent.

In `@ct/cmd/install.go`:
- Around line 99-102: The install.go branch prints and ignores the error
returned by chart.NewTesting(*configuration) instead of returning it, which can
pass a broken Testing into InstallCharts; update install.go to handle NewTesting
errors like the other commands by returning the error (rather than fmt.Println)
when chart.NewTesting(*configuration) fails, ensuring the variable testing is
only used after a successful call and that InstallCharts is not invoked with an
invalid Testing instance.

In `@doc/ct_lint-and-install.md`:
- Around line 66-67: Remove the trailing space at the end of the description
line that begins with "--release-name string" (the line ending "...set to the
chart name and a random "). Edit the markdown line so it ends with "random" (no
trailing whitespace) to eliminate the formatting nit.

In `@Dockerfile`:
- Around line 26-33: The Dockerfile currently extracts targetArch from
TARGETPLATFORM and sets HELM_ARCH for amd64/arm64 but leaves HELM_ARCH unset for
empty/unexpected TARGETPLATFORM, causing broken download URLs; update the
kubectl and Helm blocks that use TARGETPLATFORM/targetArch/HELM_ARCH to include
an explicit default/else branch that prints a clear error (including the value
of TARGETPLATFORM) and exits non‑zero (e.g., echo "Unsupported TARGETPLATFORM:
$TARGETPLATFORM" >&2; exit 1) so the build fails fast when TARGETPLATFORM is
missing or unrecognized, and ensure the guard runs before any curl/download
commands.
- Around line 14-15: The Dockerfile currently configures Git with a global
wildcard safe.directory ('RUN git config --global --add safe.directory '*'')
which disables safety checks; change this to add only the specific runtime repo
paths instead of '*' (e.g., the application workspace or mounted repo
directories). Update the RUN git config command to use a fixed allowlist or a
build/runtime variable (referencing the existing RUN git config --global --add
safe.directory invocation and the safe.directory setting) so only known
directories are whitelisted rather than every path.

In `@pkg/chart/chart.go`:
- Around line 989-1013: In printDetails (method on *Testing) ensure
GithubGroupsEnd is always called when a group was begun: if
t.config.GithubGroups is true, either call util.GithubGroupsEnd(os.Stdout)
before returning on the printFunc error or, better, immediately after calling
util.GithubGroupsBegin(os.Stdout, ...) register a deferred closure to call
util.GithubGroupsEnd(os.Stdout) so the group is closed on both success and error
paths; reference the symbols printDetails, t.config.GithubGroups, printFunc,
util.GithubGroupsBegin and util.GithubGroupsEnd.

In `@pkg/tool/account.go`:
- Line 26: The regex assigned to scpStylePattern uses a named capture
"(?<host>...)" that isn't referenced by name; update scpStylePattern to remove
the named group and use a plain capturing group for the host (e.g.,
"^(?:[^@]+@)?([^@/:]+):.+$") or, if you prefer a named capture, convert it to
Go's traditional syntax "(?P<host>...)" and then update any code that references
the name; ensure scpStylePattern remains a valid regexp.

In `@pkg/tool/helm.go`:
- Around line 40-49: In Helm.AddRepo, the OCI registry domain extraction
currently uses url[len(ociPrefix):] which includes path components (e.g.,
"ghcr.io/myorg/charts"); instead parse the substring after "oci://" and take
only the host portion (split on '/' and use the first segment or use net/url to
parse and extract Host) and pass that hostname to h.exec.RunProcess("helm",
"registry", "login", registryDomain, extraArgs); update the registryDomain
computation in AddRepo accordingly so helm registry login receives only the
hostname.

In `@pkg/tool/kubectl.go`:
- Around line 222-225: The GetEvents method in Kubectl is passing a bare field
name to kubectl's --sort-by flag; change the argument to use a JSONPath
expression (e.g. `{.lastTimestamp}`) so sorting works correctly. Locate the
exec.RunProcess call inside GetEvents and replace the "lastTimestamp" token with
the JSONPath form for the --sort-by flag (for example
"--sort-by={.lastTimestamp}" or build it via fmt.Sprintf), ensuring the rest of
the arguments stay the same.

In `@pkg/util/util.go`:
- Around line 225-227: The GithubGroupsBegin function writes the raw title into
a GitHub Actions ::group:: command which can be broken or abused if title
contains newlines; sanitize the title in GithubGroupsBegin by stripping or
replacing newline characters (e.g., replace '\n' and '\r' with a space or empty
string) before calling fmt.Fprintf so the emitted ::group::%s line is always
single-line and cannot inject extra workflow commands. Ensure you update the
GithubGroupsBegin function to perform this sanitization (reference:
GithubGroupsBegin) and preserve existing writer behavior.

---

Outside diff comments:
In @.github/workflows/ci.yaml:
- Around line 141-147: The dependency-scan job currently runs with top-level
empty permissions which removes the default token scopes, causing
actions/checkout@v2 to fail; update the dependency-scan job definition (job name
"dependency-scan") to include a job-level permissions block granting at least
contents: read so the checkout action can access the repository (e.g., add
permissions: { contents: read } under the dependency-scan job).

In `@build.sh`:
- Around line 64-66: The script uses goreleaser_args+=(--debug) when the debug
flag is set, but GoReleaser v2 removed --debug; update the invocation to
goreleaser_args+=(--verbose) and keep set -x for shell tracing; also update the
help text strings and any build.sh comments and README references that mention
"--debug" or the debug flag to say "--verbose" (or "verbose") so the documented
flag and the goreleaser_args handling in build.sh remain consistent with
GoReleaser v2; ensure you update the occurrence that checks the debug variable
and any related usage of goreleaser_args.

In `@doc/ct_list-changed.md`:
- Around line 7-8: Remove the stray leading double-quote characters from the
synopsis lines so the rendered Markdown no longer shows literal quotes;
specifically edit the lines containing "List changed charts based on configured
charts directories, and "remote, and target branch" (the two lines starting with
a double-quote) and remove the leading " characters so the sentence reads
normally as plain text in ct_list-changed.md.

In `@pkg/chart/chart.go`:
- Around line 211-229: The CreateInstallParams method ignores the releaseName
argument for charts in subdirectories; update Chart.CreateInstallParams so that
after deriving the default release (using filepath.Base(c.Path()) and falling
back to c.Yaml().Name when base is "." or "/"), you override that value if
releaseName != "" (i.e., set release = releaseName), then proceed to build
namespace, append buildID and randomSuffix, and sanitize via util.SanitizeName
with maxNameLength; reference c.Path(), c.Yaml(), releaseName,
util.RandomString, util.SanitizeName, and maxNameLength when making the change.

In `@pkg/exec/exec.go`:
- Around line 92-103: The scanner goroutine is started before cmd.Start(),
causing it to block and leak when cmd.Start() fails because pipe writers remain
open; change the flow so you call cmd.Start() first and only launch the
goroutine (that uses scanner, outReader, errReader) after Start succeeds, and on
the cmd.Start() error path explicitly close outReader and errReader to release
FDs and unblock the scanner if needed; ensure the goroutine still defers closing
outReader and errReader when it runs and that error handling around cmd.Start()
closes the readers before returning the wrapped error.

---

Nitpick comments:
In `@ct/cmd/root.go`:
- Line 68: The default for the target branch was changed in the
flags.String("target-branch", "main", ...) call which can silently break users
still on "master"; either (A) revert to "master" or (B) keep "main" but add a
clear migration notice: update release notes and the CLI help/usage text to call
out the default change, and add a runtime warning when the configured/default
target branch does not exist (detect in the code path that uses the target
branch and log a warning referencing the flags.String("target-branch", ...) flag
name and its value). Ensure the release notes/migration guide explicitly mention
the default change from "master" to "main".

In `@pkg/chart/chart_test.go`:
- Around line 156-175: The loadRules test stub duplicates the helmignore.Parse +
rules.AddDefaults logic for two directory cases; refactor loadRules in
pkg/chart/chart_test.go to consolidate by checking membership (e.g., a map or
slice lookup) for the directories "test_charts/foo" and
"test_chart_at_multi_level/foo/baz" and then performing
helmignore.Parse(strings.NewReader("Chart.yaml\n")) and rules.AddDefaults once,
returning the parsed rules; keep the same error handling and return rules, nil
for other dirs so behavior of loadRules, helmignore.Parse, and rules.AddDefaults
is preserved.

In `@pkg/ignore/ignore.go`:
- Around line 40-44: Normalize input file paths before inserting into the
fstest.MapFS in FilterFiles: for each entry in the files slice (in the
FilterFiles function) apply filepath.Clean followed by filepath.ToSlash (or
equivalent) so MapFS gets POSIX-style paths and helmignore matching is
consistent across OSes; update the loop that writes to fstest.MapFS (the
fsys[file] = &fstest.MapFile{} block) to use the normalized path variable
instead of the original file string.

In `@pkg/tool/account.go`:
- Line 33: The local variable url in the Validate function shadows the imported
net/url package; rename the local variable (for example to accountURL or
formattedURL) in the statement currently reading url :=
fmt.Sprintf("https://%s/%s", domain, account) and update any subsequent
references in Validate to that new name so the net/url package remains available
without collision.

In `@README.md`:
- Line 158: Update the stale Go version statement in README.md that currently
reads "`ct` is built using Go 1.13 or higher" to reflect the CI change to Go
1.25; locate the exact string in README.md and replace "Go 1.13" with "Go 1.25"
(or a range like "Go 1.25 or higher") so the documentation matches the updated
CI configuration.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d2f5fe and 657e6a4.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (43)
  • .github/dependabot.yml
  • .github/workflows/ci.yaml
  • .github/workflows/release.yaml
  • .github/workflows/stale.yaml
  • .golangci.yml
  • .goreleaser.yml
  • Dockerfile
  • README.md
  • build.sh
  • code-of-conduct.md
  • ct/cmd/install.go
  • ct/cmd/lint.go
  • ct/cmd/lintAndInstall.go
  • ct/cmd/listChanged.go
  • ct/cmd/root.go
  • doc/ct_install.md
  • doc/ct_lint-and-install.md
  • doc/ct_lint.md
  • doc/ct_list-changed.md
  • etc/chart_schema.yaml
  • examples/docker-for-mac/my_test.sh
  • examples/gke/Dockerfile
  • examples/kind/.circleci/config.yml
  • examples/kind/README.md
  • examples/kind/test/e2e-kind.sh
  • go.mod
  • pkg/chart/chart.go
  • pkg/chart/chart_test.go
  • pkg/chart/integration_test.go
  • pkg/config/config.go
  • pkg/config/config_test.go
  • pkg/config/test_config.json
  • pkg/config/test_config.yaml
  • pkg/exec/exec.go
  • pkg/ignore/ignore.go
  • pkg/ignore/ignore_test.go
  • pkg/tool/account.go
  • pkg/tool/account_test.go
  • pkg/tool/git.go
  • pkg/tool/helm.go
  • pkg/tool/kubectl.go
  • pkg/util/util.go
  • pkg/util/util_test.go

Comment on lines +19 to +26
stale-issue-message: 'This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.'
stale-pr-message: 'This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.'
close-issue-message: 'This issue was closed because it has been stalled for 5 days with no activity.'
close-pr-message: 'This PR was closed because it has been stalled for 10 days with no activity.'
days-before-issue-stale: 30
days-before-pr-stale: 45
days-before-issue-close: 5
days-before-pr-close: 5
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Align stale PR close timing with the messages.

Lines 20–22 state PRs close in 10 days, but days-before-pr-close is 5. This is misleading.

🔧 Suggested alignment
-          days-before-pr-close: 5
+          days-before-pr-close: 10
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
stale-issue-message: 'This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.'
stale-pr-message: 'This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.'
close-issue-message: 'This issue was closed because it has been stalled for 5 days with no activity.'
close-pr-message: 'This PR was closed because it has been stalled for 10 days with no activity.'
days-before-issue-stale: 30
days-before-pr-stale: 45
days-before-issue-close: 5
days-before-pr-close: 5
stale-issue-message: 'This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.'
stale-pr-message: 'This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.'
close-issue-message: 'This issue was closed because it has been stalled for 5 days with no activity.'
close-pr-message: 'This PR was closed because it has been stalled for 10 days with no activity.'
days-before-issue-stale: 30
days-before-pr-stale: 45
days-before-issue-close: 5
days-before-pr-close: 10
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/stale.yaml around lines 19 - 26, The README/workflow has
inconsistent PR close timing: the string stale-pr-message says "closed ... in 10
days" while the days-before-pr-close variable (days-before-pr-close) is set to
5; update days-before-pr-close to 10 or change stale-pr-message to reflect 5
days so both stale-pr-message and days-before-pr-close match. Locate the keys
stale-pr-message and days-before-pr-close in the stale workflow and make the
values consistent.

Comment on lines +99 to 102
testing, err := chart.NewTesting(*configuration)
if err != nil {
fmt.Println(err)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

rg -n 'NewTesting' --type=go -A3

Repository: fossas/chart-testing

Length of output: 1196


🏁 Script executed:

# Check the git diff to see what was actually changed on line 99
git show HEAD:ct/cmd/install.go | head -110 | tail -15

Repository: fossas/chart-testing

Length of output: 418


🏁 Script executed:

# Look for similar error-handling patterns with fmt.Println(err) in the codebase
rg -n 'fmt\.Println\(err\)' --type=go -B2 -A1

Repository: fossas/chart-testing

Length of output: 411


NewTesting error handling is inconsistent — install.go swallows errors while all other command files return them properly.

Every other command (lint.go, lintAndInstall.go, listChanged.go) returns the NewTesting error, but install.go only prints it and continues execution. If Helm is not found or version check fails, InstallCharts() will still be called with a potentially broken Testing struct, leading to confusing downstream failures.

Suggested fix
 	testing, err := chart.NewTesting(*configuration)
 	if err != nil {
-		fmt.Println(err)
+		return fmt.Errorf("failed setting up chart testing: %w", err)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
testing, err := chart.NewTesting(*configuration)
if err != nil {
fmt.Println(err)
}
testing, err := chart.NewTesting(*configuration)
if err != nil {
return fmt.Errorf("failed setting up chart testing: %w", err)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ct/cmd/install.go` around lines 99 - 102, The install.go branch prints and
ignores the error returned by chart.NewTesting(*configuration) instead of
returning it, which can pass a broken Testing into InstallCharts; update
install.go to handle NewTesting errors like the other commands by returning the
error (rather than fmt.Println) when chart.NewTesting(*configuration) fails,
ensuring the variable testing is only used after a successful call and that
InstallCharts is not invoked with an invalid Testing instance.

Comment on lines +26 to +33
RUN targetArch=$(echo $TARGETPLATFORM | cut -f2 -d '/') \
&& if [ ${targetArch} = "amd64" ]; then \
HELM_ARCH="linux/amd64"; \
elif [ ${targetArch} = "arm64" ]; then \
HELM_ARCH="linux/arm64"; \
fi \
&& curl -LO "dl.k8s.io/$kubectl_version/bin/$HELM_ARCH/kubectl" \
&& chmod +x kubectl \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Handle unsupported TARGETPLATFORM values explicitly.

If TARGETPLATFORM is empty or unexpected, HELM_ARCH remains unset and the downloads fail with a confusing URL. Add a default case that errors early. (Apply the same pattern in both kubectl and Helm blocks.)

🛠️ Example guard (kubectl block)
-RUN targetArch=$(echo $TARGETPLATFORM | cut -f2 -d '/') \
-    && if [ ${targetArch} = "amd64" ]; then \
-    HELM_ARCH="linux/amd64"; \
-elif [ ${targetArch} = "arm64" ]; then \
-    HELM_ARCH="linux/arm64"; \
-fi \
-    && curl -LO "dl.k8s.io/$kubectl_version/bin/$HELM_ARCH/kubectl" \
+RUN targetArch=$(echo "$TARGETPLATFORM" | cut -f2 -d '/') \
+    && case "$targetArch" in \
+      amd64) KUBE_ARCH="linux/amd64" ;; \
+      arm64) KUBE_ARCH="linux/arm64" ;; \
+      *) echo "Unsupported TARGETPLATFORM=$TARGETPLATFORM" >&2; exit 1 ;; \
+    esac \
+    && curl -LO "dl.k8s.io/$kubectl_version/bin/$KUBE_ARCH/kubectl" \
     && chmod +x kubectl \
     && mv kubectl /usr/local/bin/

Also applies to: 40-46

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 26 - 33, The Dockerfile currently extracts
targetArch from TARGETPLATFORM and sets HELM_ARCH for amd64/arm64 but leaves
HELM_ARCH unset for empty/unexpected TARGETPLATFORM, causing broken download
URLs; update the kubectl and Helm blocks that use
TARGETPLATFORM/targetArch/HELM_ARCH to include an explicit default/else branch
that prints a clear error (including the value of TARGETPLATFORM) and exits
non‑zero (e.g., echo "Unsupported TARGETPLATFORM: $TARGETPLATFORM" >&2; exit 1)
so the build fails fast when TARGETPLATFORM is missing or unrecognized, and
ensure the guard runs before any curl/download commands.

@rchenzheng rchenzheng requested a review from lackstein February 23, 2026 22:39
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
Dockerfile (2)

28-35: Add an explicit guard for unsupported TARGETPLATFORM values (still applies).

If TARGETPLATFORM is empty or unexpected, HELM_ARCH stays unset and the download URLs break silently. Please fail fast with a clear error.

Also applies to: 42-47

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 28 - 35, The current Dockerfile block computes
targetArch from TARGETPLATFORM and may leave HELM_ARCH unset if TARGETPLATFORM
is empty or unexpected, causing the curl download to silently break; update the
RUN sequence that sets targetArch/HELM_ARCH (same pattern used again at lines
~42-47) to validate TARGETPLATFORM/targetArch immediately after extraction and,
if empty or not one of the supported values ("amd64" or "arm64"), print a clear
error to stderr and exit with a non-zero status so the build fails fast; ensure
the subsequent curl uses a guaranteed HELM_ARCH variable (or is skipped) and
reference the variables TARGETPLATFORM, targetArch, HELM_ARCH and the curl
download that uses kubectl_version when applying this change.

14-17: Avoid wildcard Git safe.directory (still applies).

Using '*' disables Git’s safety checks globally; please scope to known working directories (or a build-time allowlist).

#!/bin/bash
# Identify candidate workspace paths used in this repo to build a safer allowlist.
rg -n -C2 '/workdir|/github/workspace|/workspace' .
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 14 - 17, The Dockerfile sets git safe.directory with
a global wildcard via the RUN git config --global --add safe.directory '*' line
which disables Git safety checks; replace the wildcard with an explicit
allowlist of known build/workspace paths (or use a build ARG to inject an
allowlist) and apply git config only for those paths (e.g., set safe.directory
for /workdir, /workspace, or ${ALLOWED_DIRS} passed at build time) so you don't
globally disable Git security.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@Dockerfile`:
- Around line 28-35: The current Dockerfile block computes targetArch from
TARGETPLATFORM and may leave HELM_ARCH unset if TARGETPLATFORM is empty or
unexpected, causing the curl download to silently break; update the RUN sequence
that sets targetArch/HELM_ARCH (same pattern used again at lines ~42-47) to
validate TARGETPLATFORM/targetArch immediately after extraction and, if empty or
not one of the supported values ("amd64" or "arm64"), print a clear error to
stderr and exit with a non-zero status so the build fails fast; ensure the
subsequent curl uses a guaranteed HELM_ARCH variable (or is skipped) and
reference the variables TARGETPLATFORM, targetArch, HELM_ARCH and the curl
download that uses kubectl_version when applying this change.
- Around line 14-17: The Dockerfile sets git safe.directory with a global
wildcard via the RUN git config --global --add safe.directory '*' line which
disables Git safety checks; replace the wildcard with an explicit allowlist of
known build/workspace paths (or use a build ARG to inject an allowlist) and
apply git config only for those paths (e.g., set safe.directory for /workdir,
/workspace, or ${ALLOWED_DIRS} passed at build time) so you don't globally
disable Git security.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 657e6a4 and 1145b99.

📒 Files selected for processing (2)
  • .github/dependabot.yml
  • Dockerfile
💤 Files with no reviewable changes (1)
  • .github/dependabot.yml

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
Dockerfile (1)

42-52: Same missing default case for TARGETPLATFORM applies here.

The Helm block has the same issue as the kubectl block — if TARGETPLATFORM is empty or unexpected, HELM_ARCH remains unset and the curl URL will be malformed. Add an else branch that exits with a clear error, consistent with any fix applied to the kubectl block above.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 42 - 52, The Helm install RUN block fails to set
HELM_ARCH when TARGETPLATFORM is empty/unexpected; update the conditional that
sets HELM_ARCH (the RUN block using TARGETPLATFORM, HELM_ARCH, helm_tag and
helm_version) to add an else branch that prints a clear error and exits nonzero
(consistent with the kubectl fix) so the curl URL is never constructed with an
empty HELM_ARCH.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Dockerfile`:
- Around line 14-17: Update the misleading comment above the git config line to
reflect that only /workdir is being marked safe rather than "all directories";
locate the Dockerfile comment that currently reads "Allow git to access all
directories" immediately above the RUN git config --global --add safe.directory
'/workdir' instruction and change it to text that accurately states that
/workdir is being added as a safe.directory (e.g., "Allow git to access /workdir
by marking it as a safe.directory").
- Around line 28-36: The kubectl download block uses an insecure/missing URL
scheme and a confusing variable name: change the curl URL to include the HTTPS
scheme ("https://dl.k8s.io/...") and rename the architecture variable used for
kubectl from HELM_ARCH to a clearer name like KUBE_ARCH (the code that sets
targetArch from TARGETPLATFORM and the curl/mv/chmod sequence that manipulates
kubectl should reference KUBE_ARCH instead of HELM_ARCH) so the kubectl download
uses https and the variable intent is unambiguous.

---

Duplicate comments:
In `@Dockerfile`:
- Around line 42-52: The Helm install RUN block fails to set HELM_ARCH when
TARGETPLATFORM is empty/unexpected; update the conditional that sets HELM_ARCH
(the RUN block using TARGETPLATFORM, HELM_ARCH, helm_tag and helm_version) to
add an else branch that prints a clear error and exits nonzero (consistent with
the kubectl fix) so the curl URL is never constructed with an empty HELM_ARCH.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1145b99 and 30d76f1.

📒 Files selected for processing (1)
  • Dockerfile

Comment on lines +28 to +36
RUN targetArch=$(echo $TARGETPLATFORM | cut -f2 -d '/') \
&& if [ ${targetArch} = "amd64" ]; then \
HELM_ARCH="linux/amd64"; \
elif [ ${targetArch} = "arm64" ]; then \
HELM_ARCH="linux/arm64"; \
fi \
&& curl -LO "dl.k8s.io/$kubectl_version/bin/$HELM_ARCH/kubectl" \
&& chmod +x kubectl \
&& mv kubectl /usr/local/bin/
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing URL scheme and confusing variable name in kubectl block.

Two issues:

  1. Missing https:// on Line 34curl -LO "dl.k8s.io/..." has no protocol scheme. Curl will default to HTTP, adding an unnecessary plaintext hop before the redirect to HTTPS (or may fail in some configurations). The official kubectl install docs use https://dl.k8s.io/....

  2. HELM_ARCH is misleading — This variable is being used in the kubectl block; consider naming it KUBE_ARCH to avoid confusion with the Helm block below.

🔧 Proposed fix
-RUN targetArch=$(echo $TARGETPLATFORM | cut -f2 -d '/') \
-    && if [ ${targetArch} = "amd64" ]; then \
-    HELM_ARCH="linux/amd64"; \
-elif [ ${targetArch} = "arm64" ]; then \
-    HELM_ARCH="linux/arm64"; \
-fi \
-    && curl -LO "dl.k8s.io/$kubectl_version/bin/$HELM_ARCH/kubectl" \
+RUN targetArch=$(echo "$TARGETPLATFORM" | cut -f2 -d '/') \
+    && if [ "${targetArch}" = "amd64" ]; then \
+    KUBE_ARCH="linux/amd64"; \
+elif [ "${targetArch}" = "arm64" ]; then \
+    KUBE_ARCH="linux/arm64"; \
+else \
+    echo "Unsupported TARGETPLATFORM=$TARGETPLATFORM" >&2; exit 1; \
+fi \
+    && curl -LO "https://dl.k8s.io/$kubectl_version/bin/$KUBE_ARCH/kubectl" \
     && chmod +x kubectl \
     && mv kubectl /usr/local/bin/
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 28 - 36, The kubectl download block uses an
insecure/missing URL scheme and a confusing variable name: change the curl URL
to include the HTTPS scheme ("https://dl.k8s.io/...") and rename the
architecture variable used for kubectl from HELM_ARCH to a clearer name like
KUBE_ARCH (the code that sets targetArch from TARGETPLATFORM and the
curl/mv/chmod sequence that manipulates kubectl should reference KUBE_ARCH
instead of HELM_ARCH) so the kubectl download uses https and the variable intent
is unambiguous.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
Dockerfile (2)

23-31: ⚠️ Potential issue | 🟠 Major

Two unresolved issues from prior review: HELM_ARCH naming and missing fallback for unsupported TARGETPLATFORM.

  1. HELM_ARCH in the kubectl block (Lines 25, 27, 29) — this variable drives kubectl's download path, not Helm's; the name is misleading and was flagged previously.
  2. No default case — if TARGETPLATFORM is empty or unsupported, HELM_ARCH remains unset and the curl on Line 29 silently constructs a broken URL (e.g. https://dl.k8s.io/v1.34.4/bin//kubectl), causing a confusing failure downstream.
🔧 Proposed fix
-RUN targetArch=$(echo $TARGETPLATFORM | cut -f2 -d '/') \
-    && if [ ${targetArch} = "amd64" ]; then \
-    HELM_ARCH="linux/amd64"; \
-elif [ ${targetArch} = "arm64" ]; then \
-    HELM_ARCH="linux/arm64"; \
-fi \
-    && curl -LO "https://dl.k8s.io/$kubectl_version/bin/$HELM_ARCH/kubectl" \
+RUN targetArch=$(echo "$TARGETPLATFORM" | cut -f2 -d '/') \
+    && case "$targetArch" in \
+      amd64) KUBE_ARCH="linux/amd64" ;; \
+      arm64) KUBE_ARCH="linux/arm64" ;; \
+      *) echo "Unsupported TARGETPLATFORM=$TARGETPLATFORM" >&2; exit 1 ;; \
+    esac \
+    && curl -LO "https://dl.k8s.io/$kubectl_version/bin/$KUBE_ARCH/kubectl" \
     && chmod +x kubectl \
     && mv kubectl /usr/local/bin/
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 23 - 31, The kubectl download block uses a
misleading variable name HELM_ARCH and lacks a fallback for unsupported or empty
TARGETPLATFORM; rename HELM_ARCH to a clearer KUBECTL_ARCH (or similar) in the
block that computes targetArch from TARGETPLATFORM and update references where
kubectl is downloaded (the curl using $kubectl_version and the $HELM_ARCH
variable), and add a default case or explicit error path so that if targetArch
is empty or not one of "amd64"/"arm64" the Dockerfile either sets a sensible
default (e.g., "linux/amd64") or exits with a clear error message before
attempting curl to avoid constructing a broken URL.

37-47: ⚠️ Potential issue | 🟠 Major

Helm block also lacks a fallback for unsupported TARGETPLATFORM (also flagged in prior review).

If TARGETPLATFORM is empty or unrecognized, HELM_ARCH is unset and the curl on Line 43 constructs a broken tarball URL, silently downloading the wrong content or failing.

🔧 Proposed fix (mirrors the kubectl block pattern)
-RUN targetArch=$(echo $TARGETPLATFORM | cut -f2 -d '/') \
-    && if [ ${targetArch} = "amd64" ]; then \
-    HELM_ARCH="linux-amd64"; \
-elif [ ${targetArch} = "arm64" ]; then \
-    HELM_ARCH="linux-arm64"; \
-fi \
+RUN targetArch=$(echo "$TARGETPLATFORM" | cut -f2 -d '/') \
+    && case "$targetArch" in \
+      amd64) HELM_ARCH="linux-amd64" ;; \
+      arm64) HELM_ARCH="linux-arm64" ;; \
+      *) echo "Unsupported TARGETPLATFORM=$TARGETPLATFORM" >&2; exit 1 ;; \
+    esac \
     && curl -LO "https://github.com/fossas/helm-cli/releases/download/$helm_tag/helm-$helm_version-$HELM_ARCH.tar.gz" \
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 37 - 47, The Helm install RUN block fails when
TARGETPLATFORM is empty/unrecognized because HELM_ARCH remains unset; modify the
logic that computes targetArch and HELM_ARCH (the RUN stanza using targetArch,
HELM_ARCH, helm_tag, helm_version) to mirror the kubectl pattern: detect empty
or unknown targetArch and set a safe default (e.g., HELM_ARCH="linux-amd64") or
explicitly error out, ensuring HELM_ARCH is always defined before constructing
the curl URL for "helm-$helm_version-$HELM_ARCH.tar.gz".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Dockerfile`:
- Around line 34-36: The Dockerfile label `LABEL helm_version=$helm_tag` is
misleading because it uses the ARG `helm_tag` while a separate ARG
`helm_version` exists; change the LABEL to reference the correct ARG or rename
the ARG so names align — update the LABEL to use `$helm_version` (or rename
`helm_tag` to `helm_version` and use that consistently) so the LABEL key
`helm_version` matches the value source (ARG helm_version or renamed helm_tag)
and remove the ambiguity when inspecting the image.

---

Duplicate comments:
In `@Dockerfile`:
- Around line 23-31: The kubectl download block uses a misleading variable name
HELM_ARCH and lacks a fallback for unsupported or empty TARGETPLATFORM; rename
HELM_ARCH to a clearer KUBECTL_ARCH (or similar) in the block that computes
targetArch from TARGETPLATFORM and update references where kubectl is downloaded
(the curl using $kubectl_version and the $HELM_ARCH variable), and add a default
case or explicit error path so that if targetArch is empty or not one of
"amd64"/"arm64" the Dockerfile either sets a sensible default (e.g.,
"linux/amd64") or exits with a clear error message before attempting curl to
avoid constructing a broken URL.
- Around line 37-47: The Helm install RUN block fails when TARGETPLATFORM is
empty/unrecognized because HELM_ARCH remains unset; modify the logic that
computes targetArch and HELM_ARCH (the RUN stanza using targetArch, HELM_ARCH,
helm_tag, helm_version) to mirror the kubectl pattern: detect empty or unknown
targetArch and set a safe default (e.g., HELM_ARCH="linux-amd64") or explicitly
error out, ensuring HELM_ARCH is always defined before constructing the curl URL
for "helm-$helm_version-$HELM_ARCH.tar.gz".

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 30d76f1 and 0f8e87a.

📒 Files selected for processing (1)
  • Dockerfile

Comment on lines +34 to +36
ARG helm_tag=v3.20.0-fossa.1
ARG helm_version=v3.20.0-fossa
LABEL helm_version=$helm_tag
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

LABEL helm_version uses $helm_tag, not $helm_version — potentially confusing.

The label key says helm_version but its value comes from helm_tag (v3.20.0-fossa.1), while the separate ARG helm_version carries v3.20.0-fossa. Anyone inspecting the image with docker inspect gets the tag value under helm_version, but the ARG named helm_version holds a different string. Aligning the label name with its source ARG removes the ambiguity.

📝 Suggested fix
 ARG helm_tag=v3.20.0-fossa.1
 ARG helm_version=v3.20.0-fossa
-LABEL helm_version=$helm_tag
+LABEL helm_tag=$helm_tag
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ARG helm_tag=v3.20.0-fossa.1
ARG helm_version=v3.20.0-fossa
LABEL helm_version=$helm_tag
ARG helm_tag=v3.20.0-fossa.1
ARG helm_version=v3.20.0-fossa
LABEL helm_tag=$helm_tag
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 34 - 36, The Dockerfile label `LABEL
helm_version=$helm_tag` is misleading because it uses the ARG `helm_tag` while a
separate ARG `helm_version` exists; change the LABEL to reference the correct
ARG or rename the ARG so names align — update the LABEL to use `$helm_version`
(or rename `helm_tag` to `helm_version` and use that consistently) so the LABEL
key `helm_version` matches the value source (ARG helm_version or renamed
helm_tag) and remove the ambiguity when inspecting the image.

@rchenzheng rchenzheng merged commit 957b35f into fossa Feb 24, 2026
5 checks passed
@rchenzheng rchenzheng deleted the fossa-3.14.0 branch February 24, 2026 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.