fix(golang): warn instead of error on transitive co-update requirements (#60)

* fix(golang): warn instead of error on transitive co-update requirements

* fix(lint): remove unused nolint directives and convert checkMissingTransitiveDeps to void

* fix(security): resolve gosec G703/G704/prealloc lint findings

* fix(security): construct proxy URL from struct fields to eliminate G704 taint path

* fix(security): parse proxy path through url.Parse to break G704 taint chain

Author: kbsteere

.golangci.yaml

@@ -15,6 +15,13 @@ linters:
           - dupl
           - gosec
           - goconst
+      # G704 taint analysis false positive: fetchFromProxy constructs the URL
+      # with a hardcoded scheme and host (proxyHost constant); only the path
+      # component is dynamic (module-escaped package path and version).
+      - path: 'pkg/languages/golang/indirect_resolver\.go'
+        linters:
+          - gosec
+        text: 'G704'
   enable:
     # Error handling
     - errcheck # Check for unchecked errors

pkg/languages/golang/golang.go

@@ -30,9 +30,6 @@ var (
 
 	// ErrUnexpectedGoListOutput is returned when go list output has unexpected format.
 	ErrUnexpectedGoListOutput = errors.New("unexpected go list output")
-
-	// ErrTransitiveDepsRequired is returned when updating a package requires co-updating other dependencies.
-	ErrTransitiveDepsRequired = errors.New("transitive dependencies need co-updating")
 )
 
 // Golang implements the Language interface for Go projects.
@@ -408,17 +405,15 @@ func resolveAndFilterPackages(ctx context.Context, packages map[string]*Package,
 		log.Infof("Will update %s from %s to %s", name, currentVersion, resolvedVersion)
 	}
 
-	if err := checkMissingTransitiveDeps(ctx, filtered, modFile); err != nil {
-		return nil, err
-	}
+	checkMissingTransitiveDeps(ctx, filtered, modFile)
 
 	return filtered, nil
 }
 
 // checkMissingTransitiveDeps checks all packages being updated for transitive dependency
-// requirements not satisfied by the current go.mod, and returns an error with co-update
+// requirements not satisfied by the current go.mod, and logs a warning with co-update
 // recommendations if any are found.
-func checkMissingTransitiveDeps(ctx context.Context, filtered map[string]*Package, modFile *modfile.File) error {
+func checkMissingTransitiveDeps(ctx context.Context, filtered map[string]*Package, modFile *modfile.File) {
 	log := clog.FromContext(ctx)
 
 	// Build set of packages being updated
@@ -459,7 +454,7 @@ func checkMissingTransitiveDeps(ctx context.Context, filtered map[string]*Packag
 	}
 
 	if len(allMissingDeps) == 0 && len(apiCompatibilityAlerts) == 0 {
-		return nil
+		return
 	}
 
 	var msg strings.Builder
@@ -514,17 +509,12 @@ func checkMissingTransitiveDeps(ctx context.Context, filtered map[string]*Packag
 		fmt.Fprintf(&msg, "\n")
 	}
 
-	// Only error if there are required co-updates; API alerts are informational
 	if len(allMissingDeps) > 0 {
 		fmt.Fprintf(&msg, "SUGGESTED UPDATE COMMAND\n\n")
 		fmt.Fprintf(&msg, "%s", buildSuggestedCommand(filtered, allMissingDeps, apiCompatibilityAlerts, modFile))
 		fmt.Fprintf(&msg, "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n")
-		return fmt.Errorf("%w:%s", ErrTransitiveDepsRequired, msg.String())
 	}
-
-	// If only API alerts (no required co-updates), just log as warning
-	log.Warnf("API compatibility alerts:\n%s", msg.String())
-	return nil
+	log.Warnf("%s", msg.String())
 }
 
 // buildSuggestedCommand builds the omnibump --packages "..." command string.

pkg/languages/golang/golang_test.go

@@ -1525,21 +1525,9 @@ require (
 		},
 	}
 
-	// This should fail with missing transitive dependencies (unless the package doesn't have any)
+	// Transitive co-update issues are now warnings, not errors.
 	err := g.Update(context.Background(), cfg)
-	// The error message format is what we're testing
-	// It should contain the formatted sections if there are missing deps
-	if err != nil {
-		errMsg := err.Error()
-		// If the error is about transitive dependencies, verify the format
-		if strings.Contains(errMsg, "transitive dependencies need co-updating") {
-			// Check for formatted sections in the error message
-			require.Contains(t, errMsg, "REQUIRED CO-UPDATES")
-			require.Contains(t, errMsg, "────────────────────")
-			require.Contains(t, errMsg, "SUGGESTED UPDATE COMMAND")
-			require.Contains(t, errMsg, "━━━━━━━━━━━━━━━━━━━━━━")
-		}
-	}
+	require.NoError(t, err)
 }
 
 func TestGolang_Update_WithDuplicatePackagesDeduplicates(t *testing.T) {

pkg/languages/golang/indirect_resolver.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -360,7 +361,10 @@ func checkModFileForIndirectDep(
 }
 
 // goProxyBase is the base URL for the Go module proxy.
-const goProxyBase = "https://proxy.golang.org"
+const (
+	goProxyBase = "https://proxy.golang.org"
+	proxyHost   = "proxy.golang.org"
+)
 
 // proxyClient is used for all Go module proxy requests with a reasonable timeout.
 var proxyClient = &http.Client{Timeout: 30 * time.Second}
@@ -432,12 +436,23 @@ func (c goModCache) has(pkg, ver string) bool {
 // fetchFromProxy performs an HTTP GET request to the Go module proxy and returns the response body.
 // path must begin with "/" and is appended to goProxyBase.
 func fetchFromProxy(ctx context.Context, path string) ([]byte, error) {
-	req, err := http.NewRequestWithContext(ctx, "GET", goProxyBase+path, nil)
+	// Parse the path to validate it before use; only .Path is taken so the
+	// host component of the final request always comes from the proxyHost constant.
+	parsedPath, err := url.Parse(path)
+	if err != nil {
+		return nil, fmt.Errorf("invalid proxy path: %w", err)
+	}
+	u := &url.URL{
+		Scheme: "https",
+		Host:   proxyHost,
+		Path:   parsedPath.Path,
+	}
+	req, err := http.NewRequestWithContext(ctx, "GET", u.String(), nil)
 	if err != nil {
 		return nil, err
 	}
 
-	resp, err := proxyClient.Do(req) //nolint:gosec // G704: URL is always goProxyBase + an escaped module path/version
+	resp, err := proxyClient.Do(req)
 	if err != nil {
 		return nil, err
 	}

pkg/languages/java/java.go

@@ -57,7 +57,7 @@ func (j *Java) Detect(ctx context.Context, dir string) (bool, error) {
 // GetManifestFiles returns Java manifest files from the detected build tool.
 func (j *Java) GetManifestFiles() []string {
 	// Return all possible manifest files from all build tools
-	var files []string //nolint: prealloc
+	files := make([]string, 0, len(registeredBuildTools))
 	for _, tool := range registeredBuildTools {
 		files = append(files, tool.GetManifestFiles()...)
 	}

pkg/languages/java/maven/updater.go

@@ -227,8 +227,11 @@ func ParsePom(pomPath string) (*gopom.Project, error) {
 // parsePatchesFromFile reads and parses patches from a YAML file.
 func parsePatchesFromFile(ctx context.Context, patchFile string) ([]Patch, error) {
 	var patchList PatchList
-	// filepath.Clean sanitizes the path to prevent traversal attacks
-	file, err := os.Open(filepath.Clean(patchFile)) //nolint:gosec // G703: filepath.Clean() sanitizes user input
+	absPath, err := filepath.Abs(patchFile)
+	if err != nil {
+		return nil, fmt.Errorf("failed to resolve patch file path: %w", err)
+	}
+	file, err := os.Open(absPath) //nolint:gosec // G304: path is a user-supplied CLI flag resolved to absolute
 	if err != nil {
 		return nil, fmt.Errorf("failed reading file: %w", err)
 	}
@@ -294,8 +297,11 @@ func parsePatches(ctx context.Context, patchFile, patchFlag string) ([]Patch, er
 // parsePropertiesFromFile reads and parses properties from a YAML file.
 func parsePropertiesFromFile(ctx context.Context, propertyFile string) (map[string]string, error) {
 	var propertyList PropertyList
-	// filepath.Clean sanitizes the path to prevent traversal attacks
-	file, err := os.Open(filepath.Clean(propertyFile)) //nolint:gosec // G703: filepath.Clean() sanitizes user input
+	absPath, err := filepath.Abs(propertyFile)
+	if err != nil {
+		return nil, fmt.Errorf("failed to resolve properties file path: %w", err)
+	}
+	file, err := os.Open(absPath) //nolint:gosec // G304: path is a user-supplied CLI flag resolved to absolute
 	if err != nil {
 		return nil, fmt.Errorf("failed reading file: %w", err)
 	}