Fix provider CodeQL and v2 e2e image reuse

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

e2e/Makefile

@@ -73,13 +73,17 @@ test.v2: start-kind e2e-image ## Run v2 e2e tests against current kube context
 	$(if $(filter true,$(SKIP_PROVIDER_KUBERNETES_BUILD)),,$(TEST_V2_PROVIDER_KUBERNETES_BUILD_CMD))
 	$(TEST_V2_PROVIDER_AWS_BUILD_CMD)
 	$(TEST_V2_PROVIDER_FAKE_BUILD_CMD)
+ifneq ($(IMAGE_NAME),$(OCI_IMAGE_NAME))
 	$(MAKE) -C ../ docker.build.controller.e2e \
 		IMAGE_NAME=$(OCI_IMAGE_NAME) \
 		VERSION=$(VERSION) \
 		ARCH=amd64 \
 		DOCKER_BUILD_ARGS="${DOCKER_BUILD_ARGS} --build-arg TARGETARCH=amd64 --build-arg TARGETOS=linux"
+endif
 	kind load docker-image --name="$(KIND_CLUSTER_NAME)" $(IMAGE_NAME):$(VERSION)
+ifneq ($(IMAGE_NAME),$(OCI_IMAGE_NAME))
 	kind load docker-image --name="$(KIND_CLUSTER_NAME)" $(OCI_IMAGE_NAME):$(VERSION)
+endif
 	kind load docker-image --name="$(KIND_CLUSTER_NAME)" $(E2E_IMAGE_NAME):$(VERSION)
 	kind load docker-image --name="$(KIND_CLUSTER_NAME)" ghcr.io/external-secrets/provider-kubernetes:$(VERSION)
 	kind load docker-image --name="$(KIND_CLUSTER_NAME)" ghcr.io/external-secrets/provider-aws:$(VERSION)

e2e/makefile_test.go

@@ -30,6 +30,7 @@ const (
 	kubernetesProviderImage  = "ghcr.io/external-secrets/provider-kubernetes:test-version"
 	helmDependencyEnsureCmd  = "../hack/helm.dependency.ensure.sh ../deploy/charts/external-secrets"
 	controllerImageLoadCount = `kind load docker-image --name="external-secrets" ghcr.io/external-secrets/external-secrets:test-version`
+	controllerImageBuildCmd  = "docker.build.controller.e2e"
 )
 
 func TestClassicMakeTargetBuildsOnlyControllerImageOnce(t *testing.T) {
@@ -37,7 +38,7 @@ func TestClassicMakeTargetBuildsOnlyControllerImageOnce(t *testing.T) {
 
 	dryRun := runMakeDryRun(t, "test", testVersionArg, `TEST_SUITES=provider`, `GINKGO_LABELS=kubernetes && !v2`)
 
-	if !strings.Contains(dryRun, "docker.build.controller.e2e") {
+	if !strings.Contains(dryRun, controllerImageBuildCmd) {
 		t.Fatalf("expected classic test dry-run to build the controller image via docker.build.controller.e2e, output:\n%s", dryRun)
 	}
 	if strings.Contains(dryRun, kubernetesBuildTarget) {
@@ -67,12 +68,18 @@ func TestV2MakeTargetCanSkipKubernetesProviderBuild(t *testing.T) {
 	if !strings.Contains(defaultDryRun, kubernetesBuildTarget) {
 		t.Fatalf("expected default test.v2 dry-run to build the kubernetes provider image, output:\n%s", defaultDryRun)
 	}
+	if count := strings.Count(defaultDryRun, controllerImageBuildCmd); count != 1 {
+		t.Fatalf("expected default test.v2 dry-run to build the controller image once, got %d occurrences, output:\n%s", count, defaultDryRun)
+	}
 	if !strings.Contains(defaultDryRun, "docker.build.provider.aws") {
 		t.Fatalf("expected default test.v2 dry-run to build the aws provider image, output:\n%s", defaultDryRun)
 	}
 	if !strings.Contains(defaultDryRun, "docker.build.provider.fake") {
 		t.Fatalf("expected default test.v2 dry-run to build the fake provider image, output:\n%s", defaultDryRun)
 	}
+	if count := strings.Count(defaultDryRun, controllerImageLoadCount); count != 1 {
+		t.Fatalf("expected default test.v2 dry-run to load the controller image once, got %d occurrences, output:\n%s", count, defaultDryRun)
+	}
 	if !strings.Contains(defaultDryRun, kubernetesProviderImage) {
 		t.Fatalf("expected default test.v2 dry-run to still load the kubernetes provider image, output:\n%s", defaultDryRun)
 	}
@@ -90,9 +97,15 @@ func TestV2MakeTargetCanSkipKubernetesProviderBuild(t *testing.T) {
 	if strings.Contains(skippedDryRun, kubernetesBuildTarget) {
 		t.Fatalf("expected skipped test.v2 dry-run to omit the kubernetes provider build, output:\n%s", skippedDryRun)
 	}
+	if count := strings.Count(skippedDryRun, controllerImageBuildCmd); count != 1 {
+		t.Fatalf("expected skipped test.v2 dry-run to build the controller image once, got %d occurrences, output:\n%s", count, skippedDryRun)
+	}
 	if !strings.Contains(skippedDryRun, "docker.build.provider.fake") {
 		t.Fatalf("expected skipped test.v2 dry-run to still build the fake provider image, output:\n%s", skippedDryRun)
 	}
+	if count := strings.Count(skippedDryRun, controllerImageLoadCount); count != 1 {
+		t.Fatalf("expected skipped test.v2 dry-run to load the controller image once, got %d occurrences, output:\n%s", count, skippedDryRun)
+	}
 	if !strings.Contains(skippedDryRun, kubernetesProviderImage) {
 		t.Fatalf("expected skipped test.v2 dry-run to still load the kubernetes provider image, output:\n%s", skippedDryRun)
 	}

providers/v2/aws/main.go

@@ -109,6 +109,6 @@ func main() {
 			genpb.RegisterGeneratorProviderServer(registrar, adapterServer)
 		},
 	}); err != nil {
-		log.Fatalf("Failed to run provider server: %v", err)
+		log.Fatalf("Failed to run provider server: %q", err.Error())
 	}
 }

providers/v2/common/grpc/server/tls.go

@@ -22,7 +22,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
+	"regexp"
 )
 
 const (
@@ -36,6 +36,8 @@ const (
 	DefaultKeyFile = "tls.key"
 )
 
+var tlsFileNamePattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*$`)
+
 // TLSConfig holds configuration for provider server TLS.
 type TLSConfig struct {
 	CertDir    string
@@ -122,20 +124,10 @@ func getEnvOrDefault(key, defaultValue string) string {
 
 func resolveCertPath(certDir, fileName string) (string, error) {
 	cleanDir := filepath.Clean(certDir)
-	cleanFile := filepath.Clean(fileName)
-
-	if filepath.IsAbs(cleanFile) || cleanFile == "." || cleanFile == ".." || cleanFile != filepath.Base(cleanFile) {
-		return "", fmt.Errorf("invalid TLS file name %q", fileName)
-	}
 
-	fullPath := filepath.Join(cleanDir, cleanFile)
-	relPath, err := filepath.Rel(cleanDir, fullPath)
-	if err != nil {
-		return "", fmt.Errorf("resolve TLS file path: %w", err)
-	}
-	if relPath == ".." || strings.HasPrefix(relPath, ".."+string(filepath.Separator)) {
+	if !tlsFileNamePattern.MatchString(fileName) {
 		return "", fmt.Errorf("invalid TLS file name %q", fileName)
 	}
 
-	return fullPath, nil
+	return filepath.Join(cleanDir, fileName), nil
 }

providers/v2/common/grpc/server/tls_test.go

@@ -52,6 +52,11 @@ func TestResolveCertPath(t *testing.T) {
 			fileName: filepath.Join(string(filepath.Separator), "tmp", "ca.crt"),
 			wantErr:  true,
 		},
+		{
+			name:     "rejects windows style nested path",
+			fileName: `nested\ca.crt`,
+			wantErr:  true,
+		},
 	}
 
 	for _, tt := range tests {

providers/v2/fake/main.go

@@ -98,6 +98,6 @@ func main() {
 			genpb.RegisterGeneratorProviderServer(registrar, adapterServer)
 		},
 	}); err != nil {
-		log.Fatalf("Failed to run provider server: %v", err)
+		log.Fatalf("Failed to run provider server: %q", err.Error())
 	}
 }

providers/v2/hack/generate-provider-main.go

@@ -27,7 +27,6 @@ import (
 	"log"
 	"os"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"text/template"
 
@@ -164,11 +163,10 @@ func main() {
 			log.Fatalf("Failed to load/validate config %s: %v", configPath, err)
 		}
 
-		providerName := logSafeValue(config.Provider.Name)
-		providerDisplayName := logSafeValue(config.Provider.DisplayName)
+		providerName := config.Provider.Name
+		providerDisplayName := config.Provider.DisplayName
 		if *verbose {
-			// # codeql[go/log-injection] -- logSafeValue quotes and ASCII-escapes provider metadata for logs.
-			log.Printf("  Provider: %s (%s)", providerName, providerDisplayName)
+			log.Printf("  Provider: %q (%q)", providerName, providerDisplayName)
 			log.Printf("  Stores: %d, Generators: %d", len(config.Stores), len(config.Generators))
 		}
 
@@ -178,15 +176,13 @@ func main() {
 		// Generate main.go
 		mainContent, err := executeTemplate(mainTemplate, templateData)
 		if err != nil {
-			// # codeql[go/log-injection] -- providerName is sanitized with logSafeValue above.
-			log.Fatalf("Failed to generate main.go for %s: %v", providerName, err)
+			log.Fatalf("Failed to generate main.go for %q: %q", providerName, err.Error())
 		}
 
 		// Format with goimports/gofmt
 		formattedMain, err := formatGoCode(mainContent)
 		if err != nil {
-			// # codeql[go/log-injection] -- providerName is sanitized with logSafeValue above.
-			log.Printf("Warning: Failed to format main.go for %s: %v", providerName, err)
+			log.Printf("Warning: Failed to format main.go for %q: %q", providerName, err.Error())
 			formattedMain = mainContent // Use unformatted if formatting fails
 		}
 
@@ -203,8 +199,7 @@ func main() {
 		// Generate Dockerfile
 		dockerContent, err := executeTemplate(dockerfileTemplate, templateData)
 		if err != nil {
-			// # codeql[go/log-injection] -- providerName is sanitized with logSafeValue above.
-			log.Fatalf("Failed to generate Dockerfile for %s: %v", providerName, err)
+			log.Fatalf("Failed to generate Dockerfile for %q: %q", providerName, err.Error())
 		}
 
 		dockerPath := filepath.Join(providerDir, "Dockerfile")
@@ -239,10 +234,6 @@ func findProviderConfigs(baseDir string) ([]string, error) {
 	return configs, err
 }
 
-func logSafeValue(value string) string {
-	return strconv.QuoteToASCII(strings.ToValidUTF8(value, "?"))
-}
-
 func loadAndValidateConfig(configPath string, schemaLoader gojsonschema.JSONLoader) (*ProviderConfig, error) {
 	// Read YAML file
 	//nolint:gosec // configPath comes from controlled provider config discovery under providers-dir.

providers/v2/hack/generate_provider_main_test.go

@@ -103,3 +103,37 @@ func TestMainTemplateStartsProviderMetricsServer(t *testing.T) {
 		t.Fatalf("main template did not register services through the shared provider runtime:\n%s", renderedText)
 	}
 }
+
+func TestMainTemplateQuotesProviderRuntimeErrors(t *testing.T) {
+	tmpl, err := loadTemplate("templates/main.go.tmpl")
+	if err != nil {
+		t.Fatalf("load template: %v", err)
+	}
+
+	data := prepareTemplateData(&ProviderConfig{
+		Provider: providerMetadata{
+			Name:        "aws",
+			DisplayName: "AWS",
+			V2Package:   "github.com/external-secrets/external-secrets/apis/provider/aws/v2alpha1",
+		},
+		Stores: []storeConfig{{
+			GVK: gvkConfig{
+				Group:   "provider.external-secrets.io",
+				Version: "v2alpha1",
+				Kind:    "SecretsManager",
+			},
+			V1Provider:     "github.com/external-secrets/external-secrets/providers/v2/aws/store",
+			V1ProviderFunc: "NewProvider",
+		}},
+	})
+
+	rendered, err := executeTemplate(tmpl, data)
+	if err != nil {
+		t.Fatalf("execute template: %v", err)
+	}
+
+	renderedText := string(rendered)
+	if !strings.Contains(renderedText, `log.Fatalf("Failed to run provider server: %q", err.Error())`) {
+		t.Fatalf("main template did not quote provider runtime errors:\n%s", renderedText)
+	}
+}

providers/v2/hack/templates/main.go.tmpl

@@ -144,6 +144,6 @@ func main() {
 			{{- end}}
 		},
 	}); err != nil {
-		log.Fatalf("Failed to run provider server: %v", err)
+		log.Fatalf("Failed to run provider server: %q", err.Error())
 	}
 }

providers/v2/kubernetes/main.go

@@ -83,6 +83,6 @@ func main() {
 			pb.RegisterSecretStoreProviderServer(registrar, adapterServer)
 		},
 	}); err != nil {
-		log.Fatalf("Failed to run provider server: %v", err)
+		log.Fatalf("Failed to run provider server: %q", err.Error())
 	}
 }