Fossa 3.14.0

[rchenzheng]

• Merges upstream 3.14.0 with helm 3.20.0 and kubectl 1.34.4
• Add git config --global --add safe.directory '*' to allow git functions to continue working after git 2.47+

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

[coderabbitai]

Walkthrough

This pull request updates chart-testing across multiple layers: CI/CD workflows to use newer action versions and explicit Go 1.25 support; Go dependencies upgraded to 1.24.0 with updated Cobra, Viper, and Helm; CLI flags reorganized with new options for Helm lint arguments, release naming, and Helmignore support; the core Testing API simplified by removing extraSetArgs parameter; and Helm upgrade invocations replaced with UpgradeWithValues to accept explicit values files. Additional changes include Dockerfile updates with alpine:3.23 and tool version bumps, default branch migration from master to main, and GitHub Actions output grouping support through new utility functions.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

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.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fossa 3.14.0' directly reflects the main objective of merging upstream version 3.14.0 with updated dependencies.
Description check	✅ Passed	The description is clearly related to the changeset, detailing version merges (3.14.0 with Helm 3.20.0 and kubectl 1.34.4) and Git safety configuration changes.

_{✏️ 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.}

[coderabbitai]

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

[coderabbitai]

⚠️ 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.

[coderabbitai]

⚠️ 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.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Avoid wildcard safe.directory in the image.

Using '*' disables Git’s safety checks globally, which is risky if the container ever processes untrusted repos. Prefer a specific allowlist of directories used at runtime.

🔒 Proposed safer allowlist

-# Allow git to access all directories
-RUN git config --global --add safe.directory '*'
+# Allow git to access known working directories
+RUN git config --global --add safe.directory /workdir \
+    && git config --global --add safe.directory /github/workspace

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 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.

[coderabbitai]

⚠️ 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.

[lackstein]

Can you update this comment to explain why we need to mark all directories as safe, rather than just /workdir as the upstream does?

[rchenzheng]

This is because git 2.35.2 (container is on 2.47+) will not try to access directories not owned by the user is running it, /workdir is owned by the host user and the container runs as root. To keep it simple it is easier to allow root to be able to access everything.

Added.

[lackstein]

All of the chart testing done by this tool happens in /workdir though, so why do we need git to be able to access anything outside of that folder? How does our usage of this tool differ from the intended upstream usage, such that we need this and other users of the upstream chart-testing image do not?

[coderabbitai]

♻️ 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