remove docker package: CVE-2024-41110

[Greyeye]

This is to address CVE-2024-41110

Docker package was used to check for git urls.
this PR implements the fork inside locations.go instead of pulling entire package.

[zapier-sre-bot]

Mergecat's Review

Click to read mergecats review!

😼 Mergecat review of go.mod

@@ -15,7 +15,6 @@ require (
 	github.com/cenkalti/backoff/v4 v4.3.0
 	github.com/chainguard-dev/git-urls v1.0.2
 	github.com/creasty/defaults v1.7.0
-	github.com/docker/docker v27.2.1+incompatible
 	github.com/ghodss/yaml v1.0.0
 	github.com/go-git/go-git-fixtures/v4 v4.3.2-0.20231010084843-55a94097c399
 	github.com/go-logr/zerologr v1.2.3
@@ -120,6 +119,7 @@ require (
 	github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
 	github.com/distribution/reference v0.5.0 // indirect
 	github.com/docker/distribution v2.8.3+incompatible // indirect
+	github.com/docker/docker v27.2.1+incompatible // indirect
 	github.com/docker/go-connections v0.4.0 // indirect
 	github.com/docker/go-units v0.5.0 // indirect
 	github.com/emicklei/go-restful/v3 v3.10.2 // indirect

Feedback & Suggestions:

1. Consistency: The github.com/docker/docker dependency was moved from a direct dependency to an indirect one. Ensure that this change is intentional and that the dependency is not required directly by your codebase. If it is required directly, it should remain in the direct dependencies section.
2. Documentation: If this change is intentional, consider documenting why the dependency was moved to the indirect section. This can help future maintainers understand the reasoning behind the change.
3. Dependency Management: Regularly review and update your dependencies to ensure compatibility and security. If github.com/docker/docker is no longer needed directly, verify that all indirect dependencies are still necessary and remove any that are not.

---

😼 Mergecat review of cmd/locations.go

@@ -3,10 +3,10 @@ package cmd
 import (
 	"context"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"time"
 
-	"github.com/docker/docker/builder/remotecontext/urlutil"
 	"github.com/pkg/errors"
 	"github.com/rs/zerolog/log"
 
@@ -35,7 +35,7 @@ var ErrCannotUseQueryWithFilePath = errors.New("relative and absolute file paths
 
 func maybeCloneGitUrl(ctx context.Context, repoManager cloner, repoRefreshDuration time.Duration, location, vcsUsername string) (string, error) {
 	result := strings.SplitN(location, "?", 2)
-	if !urlutil.IsGitURL(result[0]) {
+	if !isGitURL(result[0]) {
 		if len(result) > 1 {
 			return "", ErrCannotUseQueryWithFilePath
 		}
@@ -82,3 +82,26 @@ func maybeCloneGitUrl(ctx context.Context, repoManager cloner, repoRefreshDurati
 
 	return path, nil
 }
+
+func isGitURL(str string) bool {
+	if IsURL(str) && urlPathWithFragmentSuffix.MatchString(str) {
+		return true
+	}
+	for _, prefix := range []string{"git://", "github.com/", "git@"} {
+		if strings.HasPrefix(str, prefix) {
+			return true
+		}
+	}
+	return false
+}
+
+// urlPathWithFragmentSuffix matches fragments to use as Git reference and build
+// context from the Git repository. See IsGitURL for details.
+var urlPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`)
+
+// IsURL returns true if the provided str is an HTTP(S) URL by checking if it
+// has a http:// or https:// scheme. No validation is performed to verify if the
+// URL is well-formed.
+func IsURL(str string) bool {
+	return strings.HasPrefix(str, "https://") || strings.HasPrefix(str, "http://")
+}

Feedback & Suggestions:

1. Security: The IsURL function only checks for the presence of "http://" or "https://". This is a very basic check and does not ensure the URL is well-formed or safe. Consider using a more robust URL validation method, such as the net/url package's url.Parse function, to ensure the URL is valid and properly formed.

2. Performance: The isGitURL function iterates over a slice of prefixes to check if the string starts with any of them. This could be optimized by using a more efficient data structure or method, such as a switch statement or a map lookup.

3. Code Duplication: The isGitURL function and the IsURL function both perform string prefix checks. Consider refactoring to avoid code duplication and improve maintainability.

4. Error Handling: The isGitURL function does not handle potential errors from the regexp.MustCompile call. Although MustCompile will panic on an invalid pattern, it might be safer to handle this explicitly, especially if the pattern could be dynamic in the future.

5. Documentation: The comments for urlPathWithFragmentSuffix and IsURL are helpful, but consider adding more detailed comments or examples for the isGitURL function to clarify its purpose and usage.

Suggested changes:

func isGitURL(str string) bool {
	if isValidURL(str) && urlPathWithFragmentSuffix.MatchString(str) {
		return true
	}
	switch {
	case strings.HasPrefix(str, "git://"),
		strings.HasPrefix(str, "github.com/"),
		strings.HasPrefix(str, "git@"):
		return true
	}
	return false
}

// urlPathWithFragmentSuffix matches fragments to use as Git reference and build
// context from the Git repository. See isGitURL for details.
var urlPathWithFragmentSuffix = regexp.MustCompile(`\.git(?:#.+)?$`)

// isValidURL returns true if the provided str is a well-formed HTTP(S) URL.
func isValidURL(str string) bool {
	u, err := url.Parse(str)
	return err == nil && (u.Scheme == "http" || u.Scheme == "https")
}

---

---

Dependency Review

Click to read mergecats review!

No suggestions found

[github-actions]

Temporary image available at ghcr.io/zapier/kubechecks:0.0.0-pr275.