bindings/go/helm/go.mod (1)

29-29: Update indirect dependency review: yaml/v4 is an RC, but security advisories don’t hit the pinned versions

• go.yaml.in/yaml/v4 is pinned to v4.0.0-rc.2; Go module proxy shows only v4.0.0-rc.* variants (no stable v4.0.0), so treat this as a deliberate pre-release risk/upgrade tracking item.
• The pinned indirect versions exist on the Go module proxy (including github.com/pb33f/ordered-map/v2 v2.3.1 and go.yaml.in/yaml/v4 v4.0.0-rc.2).
• GitHub security advisories show no vulnerabilities for github.com/invopop/jsonschema, github.com/pb33f/ordered-map/v2, github.com/veqryn/slog-context, or go.yaml.in/yaml/v4; github.com/buger/jsonparser advisories apply to earlier versions (fixed starting at 1.1.2), and the pinned v1.2.0 is outside the vulnerable ranges.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindings/go/helm/go.mod` at line 29, The go.mod pins an RC of
go.yaml.in/yaml/v4 (v4.0.0-rc.2) which is a pre-release risk; update the repo
metadata to explicitly document this (e.g., add a short comment in go.mod or PR
description) noting the deliberate RC pin and plan to upgrade when a stable
release is available, confirm github.com/buger/jsonparser remains >= v1.1.2 (the
pinned v1.2.0 is OK), and verify other indirect pins
(github.com/pb33f/ordered-map/v2 v2.3.1, github.com/invopop/jsonschema,
github.com/veqryn/slog-context) are tracked for future updates so reviewers know
the RC/tracking decision.

bindings/go/helm/repository/resource/resource_repository.go (1)

105-130: 💤 Low value

Consider extracting credential conversion and scheme branching to a shared helper.

The credential conversion logic (convert → check prefix → type assert → append option) is duplicated verbatim in input/method.go, digest/digest.go, and this file. A helper in helmcredsv1 or helminternal could reduce repetition and centralize the OCI vs HTTP branching:

// Example signature in helmcredsv1 or helminternal:
func ResolveCredentialOptions(credentials runtime.Typed, repoURL string) ([]download.Option, error)

This is a minor concern given the phased migration; consider addressing in a follow-up consolidation pass.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindings/go/helm/repository/resource/resource_repository.go` around lines 105
- 130, Extract the duplicated credential conversion and scheme-branching into a
shared helper (e.g., helmcredsv1.ResolveCredentialOptions or
helminternal.ResolveCredentialOptions) that accepts the runtime.Typed
credentials and repo URL and returns ([]helmdownload.Option, error); move the
logic from resource_repository.go (the block using
helmcredsv1.ConvertCredentials, strings.HasPrefix(helmURL,"oci://"), type
assertions to *ocicredsv1.OCICredentials and *helmcredsv1.HelmHTTPCredentials,
and appending helmdownload.WithOCICredentials/WithCredentials) into that helper,
preserve the behavior of passing nil when converted is nil and returning
typed-errors on wrong type, then replace the inline block in
resource_repository.go (and similar blocks in input/method.go and
digest/digest.go) with a call to the new ResolveCredentialOptions helper and
append the returned options to opts.

bindings/go/helm/transformation/get_helm_chart.go (1)

129-134: ⚡ Quick win

Wrap credential resolution errors with operation context.

The fallback branch returns the resolver error unwrapped, which makes downstream logs less actionable.

♻️ Proposed improvement

 	typed, err := t.CredentialProvider.Resolve(ctx, consumerId)
 	if err != nil {
 		if errors.Is(err, credentials.ErrNotFound) {
 			return nil, nil
 		}
-		return nil, err
+		return nil, fmt.Errorf("failed resolving credentials for resource consumer identity: %w", err)
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindings/go/helm/transformation/get_helm_chart.go` around lines 129 - 134,
The credential resolver error returned in the fallback branch is propagated raw,
making logs unhelpful; update the error return in the block that checks
errors.Is(err, credentials.ErrNotFound) to wrap the non-ErrNotFound errors with
operation context (e.g., using fmt.Errorf("resolve credentials: %w", err)) so
callers of the function (getHelmChart or the credential resolution call site)
receive an error that includes "resolve credentials" context while preserving
the original error via %w.

bindings/go/helm/spec/credentials/v1/convert.go (1)

56-59: ⚡ Quick win

Include credential type in conversion error messages.

Current errors make triage harder when multiple credential types flow through this path. Include creds.GetType() (or converted type) in both failure branches.

♻️ Proposed improvement

-	typed, err := convertScheme.NewObject(creds.GetType())
+	credType := creds.GetType()
+	typed, err := convertScheme.NewObject(credType)
 	if err != nil {
-		return nil, fmt.Errorf("error converting credential type: %w", err)
+		return nil, fmt.Errorf("error creating credential object for type %q: %w", credType.String(), err)
 	}
 	if err = convertScheme.Convert(creds, typed); err != nil {
-		return nil, fmt.Errorf("error converting credential type: %w", err)
+		return nil, fmt.Errorf("error converting credential type %q: %w", credType.String(), err)
 	}
@@
-	return nil, fmt.Errorf("unsupported credential type %v", typed.GetType())
+	return nil, fmt.Errorf("unsupported converted credential type %q (from %q)", typed.GetType().String(), credType.String())

Also applies to: 71-71

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindings/go/helm/spec/credentials/v1/convert.go` around lines 56 - 59, Update
the error messages in the conversion failure branches to include the credential
type to aid triage: when the first conversion fails (before calling
convertScheme.Convert) and when convertScheme.Convert(creds, typed) returns an
error, include creds.GetType() (or the resolved type string) in the fmt.Errorf
calls so the messages read like "error converting credential type '%s': %w";
modify the error formatting in the blocks around convertScheme.Convert and the
earlier conversion branch (the conversion logic in convert.go that uses creds,
typed and convertScheme.Convert) and apply the same change to the other similar
failure at the later branch referenced near the second occurrence.