Always validate zipped package for bogus names

6 files changed, 72 insertions, 73 deletions

diff --git a/client/go/internal/cli/cmd/cert_test.go b/client/go/internal/cli/cmd/cert_test.go
index 0a20ab8eb1a..491857f2bf2 100644
--- a/client/go/internal/cli/cmd/cert_test.go
+++ b/client/go/internal/cli/cmd/cert_test.go
@@ -50,12 +50,6 @@ func testCert(t *testing.T, subcommand []string) {
 }
 
 func TestCertCompressedPackage(t *testing.T) {
-	t.Run("auth cert", func(t *testing.T) {
-		testCertCompressedPackage(t, []string{"auth", "cert"})
-	})
-}
-
-func testCertCompressedPackage(t *testing.T, subcommand []string) {
 	_, pkgDir := mock.ApplicationPackageDir(t, true, false)
 	zipFile := filepath.Join(pkgDir, "target", "application.zip")
 	err := os.MkdirAll(filepath.Dir(zipFile), 0755)
@@ -68,16 +62,14 @@ func testCertCompressedPackage(t *testing.T, subcommand []string) {
 	stdout.Reset()
 	stderr.Reset()
 
-	args := append(subcommand, pkgDir)
-	err = cli.Run(args...)
+	err = cli.Run("auth", "cert", zipFile)
 	assert.NotNil(t, err)
 	assert.Contains(t, stderr.String(), "Error: cannot add certificate to compressed application package")
 
 	err = os.Remove(zipFile)
 	assert.Nil(t, err)
 
-	args = append(subcommand, "-f", pkgDir)
-	err = cli.Run(args...)
+	err = cli.Run("auth", "cert", "-f", pkgDir)
 	assert.Nil(t, err)
 	assert.Contains(t, stdout.String(), "Success: Certificate written to")
 	assert.Contains(t, stdout.String(), "Success: Private key written to")
diff --git a/client/go/internal/cli/cmd/deploy_test.go b/client/go/internal/cli/cmd/deploy_test.go
index d578b2a4629..d2aa52bc08f 100644
--- a/client/go/internal/cli/cmd/deploy_test.go
+++ b/client/go/internal/cli/cmd/deploy_test.go
@@ -133,7 +133,7 @@ func TestDeployApplicationDirectoryWithSource(t *testing.T) {
 }
 
 func TestDeployApplicationDirectoryWithPomAndTarget(t *testing.T) {
-	assertDeploy("testdata/applications/withTarget/target/application.zip",
+	assertDeploy("testdata/applications/withTarget/target/application",
 		[]string{"deploy", "--wait=0", "testdata/applications/withTarget"}, t)
 }
 
@@ -141,7 +141,7 @@ func TestDeployApplicationDirectoryWithPomAndEmptyTarget(t *testing.T) {
 	cli, _, stderr := newTestCLI(t)
 	assert.NotNil(t, cli.Run("deploy", "--wait=0", "testdata/applications/withEmptyTarget"))
 	assert.Equal(t,
-		"Error: found pom.xml, but target/application.zip does not exist: run 'mvn package' first\n",
+		"Error: found pom.xml, but testdata/applications/withEmptyTarget/target/application does not exist: run 'mvn package' first\n",
 		stderr.String())
 }
 
diff --git a/client/go/internal/cli/cmd/prod.go b/client/go/internal/cli/cmd/prod.go
index 1a2f88311b6..5337a346654 100644
--- a/client/go/internal/cli/cmd/prod.go
+++ b/client/go/internal/cli/cmd/prod.go
@@ -147,7 +147,7 @@ $ vespa prod deploy`,
 			if err != nil {
 				return err
 			}
-			if !pkg.HasDeployment() {
+			if !pkg.HasDeploymentSpec() {
 				return errHint(fmt.Errorf("no deployment.xml found"), "Try creating one with vespa prod init")
 			}
 			if err := verifyTests(cli, pkg); err != nil {
diff --git a/client/go/internal/cli/cmd/prod_test.go b/client/go/internal/cli/cmd/prod_test.go
index 944f09b3d42..4cca54a76c8 100644
--- a/client/go/internal/cli/cmd/prod_test.go
+++ b/client/go/internal/cli/cmd/prod_test.go
@@ -50,12 +50,12 @@ func TestProdInit(t *testing.T) {
 	assert.Nil(t, cli.Run("prod", "init", pkgDir))
 
 	// Verify contents
-	deploymentPath := filepath.Join(pkgDir, "src", "main", "application", "deployment.xml")
+	deploymentPath := filepath.Join(pkgDir, "deployment.xml")
 	deploymentXML := readFileString(t, deploymentPath)
 	assert.Contains(t, deploymentXML, `<region>aws-us-west-2a</region>`)
 	assert.Contains(t, deploymentXML, `<region>aws-eu-west-1a</region>`)
 
-	servicesPath := filepath.Join(pkgDir, "src", "main", "application", "services.xml")
+	servicesPath := filepath.Join(pkgDir, "services.xml")
 	servicesXML := readFileString(t, servicesPath)
 	containerFragment := `<container id="qrs" version="1.0">
     <document-api></document-api>
@@ -80,6 +80,7 @@ func TestProdInit(t *testing.T) {
 }
 
 func readFileString(t *testing.T, filename string) string {
+	t.Helper()
 	content, err := os.ReadFile(filename)
 	if err != nil {
 		t.Fatal(err)
@@ -88,12 +89,15 @@ func readFileString(t *testing.T, filename string) string {
 }
 
 func createApplication(t *testing.T, pkgDir string, java bool, skipTests bool) {
-	appDir := filepath.Join(pkgDir, "src", "main", "application")
-	targetDir := filepath.Join(pkgDir, "target")
+	appDir := pkgDir
+	testsDir := pkgDir
+	if java {
+		appDir = filepath.Join(pkgDir, "target", "application")
+		testsDir = filepath.Join(pkgDir, "target", "application-test")
+	}
 	if err := os.MkdirAll(appDir, 0755); err != nil {
 		t.Fatal(err)
 	}
-
 	deploymentXML := `<deployment version="1.0">
   <prod>
     <region>aws-us-east-1c</region>
@@ -102,7 +106,6 @@ func createApplication(t *testing.T, pkgDir string, java bool, skipTests bool) {
 	if err := os.WriteFile(filepath.Join(appDir, "deployment.xml"), []byte(deploymentXML), 0644); err != nil {
 		t.Fatal(err)
 	}
-
 	servicesXML := `<services version="1.0" xmlns:deploy="vespa" xmlns:preprocess="properties">
   <container id="qrs" version="1.0">
     <document-api/>
@@ -123,19 +126,16 @@ func createApplication(t *testing.T, pkgDir string, java bool, skipTests bool) {
 	if err := os.WriteFile(filepath.Join(appDir, "services.xml"), []byte(servicesXML), 0644); err != nil {
 		t.Fatal(err)
 	}
-	if err := os.MkdirAll(targetDir, 0755); err != nil {
-		t.Fatal(err)
-	}
 	if java {
-		if skipTests {
-			t.Fatalf("skipTests=%t has no effect when java=%t", skipTests, java)
-		}
 		if err := os.WriteFile(filepath.Join(pkgDir, "pom.xml"), []byte(""), 0644); err != nil {
 			t.Fatal(err)
 		}
-	} else if !skipTests {
-		testsDir := filepath.Join(pkgDir, "src", "test", "application", "tests")
-		testBytes, _ := io.ReadAll(strings.NewReader("{\"steps\":[{}]}"))
+	}
+	if !skipTests {
+		if err := os.MkdirAll(testsDir, 0755); err != nil {
+			t.Fatal(err)
+		}
+		testBytes := []byte("{\"steps\":[{}]}")
 		writeTest(filepath.Join(testsDir, "system-test", "test.json"), testBytes, t)
 		writeTest(filepath.Join(testsDir, "staging-setup", "test.json"), testBytes, t)
 		writeTest(filepath.Join(testsDir, "staging-test", "test.json"), testBytes, t)
@@ -203,23 +203,17 @@ func TestProdDeployWithJava(t *testing.T) {
 
 	httpClient := &mock.HTTPClient{}
 	httpClient.NextResponseString(200, `ok`)
-	cli, stdout, _ := newTestCLI(t, "CI=true")
+	cli, stdout, stderr := newTestCLI(t, "CI=true")
 	cli.httpClient = httpClient
 	assert.Nil(t, cli.Run("config", "set", "application", "t1.a1.i1"))
 	assert.Nil(t, cli.Run("config", "set", "target", "cloud"))
 	assert.Nil(t, cli.Run("auth", "api-key"))
 	assert.Nil(t, cli.Run("auth", "cert", "--no-add"))
 
-	// Copy an application package pre-assembled with mvn package
-	testAppDir := filepath.Join("testdata", "applications", "withDeployment", "target")
-	zipFile := filepath.Join(testAppDir, "application.zip")
-	copyFile(t, filepath.Join(pkgDir, "target", "application.zip"), zipFile)
-	testZipFile := filepath.Join(testAppDir, "application-test.zip")
-	copyFile(t, filepath.Join(pkgDir, "target", "application-test.zip"), testZipFile)
-
 	stdout.Reset()
 	cli.Environment["VESPA_CLI_API_KEY_FILE"] = filepath.Join(cli.config.homeDir, "t1.api-key.pem")
-	assert.Nil(t, cli.Run("prod", "deploy", pkgDir))
+	assert.Nil(t, cli.Run("prod", "deploy", "--add-cert", pkgDir))
+	assert.Equal(t, "", stderr.String())
 	assert.Contains(t, stdout.String(), "Success: Deployed")
 	assert.Contains(t, stdout.String(), "See https://console.vespa-cloud.com/tenant/t1/application/a1/prod/deployment for deployment progress")
 }
@@ -240,11 +234,8 @@ func TestProdDeployInvalidZip(t *testing.T) {
 	// Copy an invalid application package containing relative file names
 	testAppDir := filepath.Join("testdata", "applications", "withInvalidEntries", "target")
 	zipFile := filepath.Join(testAppDir, "application.zip")
-	copyFile(t, filepath.Join(pkgDir, "target", "application.zip"), zipFile)
-	testZipFile := filepath.Join(testAppDir, "application-test.zip")
-	copyFile(t, filepath.Join(pkgDir, "target", "application-test.zip"), testZipFile)
 
-	assert.NotNil(t, cli.Run("prod", "deploy", pkgDir))
+	assert.NotNil(t, cli.Run("prod", "deploy", zipFile))
 	assert.Equal(t, "Error: found invalid path inside zip: ../../../../../../../tmp/foo\n", stderr.String())
 }
 
diff --git a/client/go/internal/cli/cmd/testdata/applications/withTarget/target/application/.gitkeep b/client/go/internal/cli/cmd/testdata/applications/withTarget/target/application/.gitkeep
new file mode 100644
index 00000000000..e69de29bb2d
--- /dev/null
+++ b/client/go/internal/cli/cmd/testdata/applications/withTarget/target/application/.gitkeep
diff --git a/client/go/internal/vespa/application.go b/client/go/internal/vespa/application.go
index 13d3b63cfa9..dd1b580517b 100644
--- a/client/go/internal/vespa/application.go
+++ b/client/go/internal/vespa/application.go
@@ -17,49 +17,57 @@ type ApplicationPackage struct {
 	TestPath string
 }
 
-func (ap *ApplicationPackage) HasCertificate() bool {
-	return ap.hasFile(filepath.Join("security", "clients.pem"), "security/clients.pem")
-}
+func (ap *ApplicationPackage) HasCertificate() bool { return ap.hasFile("security", "clients.pem") }
 
-func (ap *ApplicationPackage) HasDeployment() bool { return ap.hasFile("deployment.xml", "") }
+func (ap *ApplicationPackage) HasDeploymentSpec() bool { return ap.hasFile("deployment.xml", "") }
 
-func (ap *ApplicationPackage) hasFile(filename, zipName string) bool {
-	if zipName == "" {
-		zipName = filename
+func (ap *ApplicationPackage) hasFile(pathSegment ...string) bool {
+	if !ap.IsZip() {
+		return util.PathExists(filepath.Join(append([]string{ap.Path}, pathSegment...)...))
 	}
-	if ap.IsZip() {
-		r, err := zip.OpenReader(ap.Path)
-		if err != nil {
-			return false
-		}
-		defer r.Close()
-		for _, f := range r.File {
-			if f.Name == zipName {
-				return true
-			}
-		}
+	zipName := filepath.Join(pathSegment...)
+	return ap.hasZipEntry(func(name string) bool { return zipName == name })
+}
+
+func (ap *ApplicationPackage) hasZipEntry(matcher func(zipName string) bool) bool {
+	r, err := zip.OpenReader(ap.Path)
+	if err != nil {
 		return false
 	}
-	return util.PathExists(filepath.Join(ap.Path, filename))
+	defer r.Close()
+	for _, f := range r.File {
+		if matcher(f.Name) {
+			return true
+		}
+	}
+	return false
 }
 
 func (ap *ApplicationPackage) IsZip() bool { return isZip(ap.Path) }
 
 func (ap *ApplicationPackage) IsJava() bool {
 	if ap.IsZip() {
-		r, err := zip.OpenReader(ap.Path)
-		if err != nil {
-			return false
-		}
-		defer r.Close()
-		for _, f := range r.File {
-			if filepath.Ext(f.Name) == ".jar" {
-				return true
-			}
+		return ap.hasZipEntry(func(name string) bool { return filepath.Ext(name) == ".jar" })
+	}
+	return util.PathExists(filepath.Join(ap.Path, "pom.xml"))
+}
+
+func (ap *ApplicationPackage) Validate() error {
+	if !ap.IsZip() {
+		return nil
+	}
+	invalidPath := ""
+	invalid := ap.hasZipEntry(func(name string) bool {
+		if !validPath(name) {
+			invalidPath = name
+			return true
 		}
 		return false
+	})
+	if invalid {
+		return fmt.Errorf("found invalid path inside zip: %s", invalidPath)
 	}
-	return util.PathExists(filepath.Join(ap.Path, "pom.xml"))
+	return nil
 }
 
 func isZip(filename string) bool { return filepath.Ext(filename) == ".zip" }
@@ -166,9 +174,6 @@ func (ap *ApplicationPackage) Unzip(test bool) (string, error) {
 	}
 	defer f.Close()
 	for _, f := range f.File {
-		if !validPath(f.Name) {
-			return "", fmt.Errorf("found invalid path inside zip: %s", f.Name)
-		}
 		dst := filepath.Join(tmp, f.Name)
 		if f.FileInfo().IsDir() {
 			if err := os.Mkdir(dst, f.FileInfo().Mode()); err != nil {
@@ -223,6 +228,17 @@ func copyFile(src *zip.File, dst string) error {
 // 3. src/main/application
 // 4. Given path, if it contains services.xml
 func FindApplicationPackage(zipOrDir string, requirePackaging bool) (ApplicationPackage, error) {
+	pkg, err := findApplicationPackage(zipOrDir, requirePackaging)
+	if err != nil {
+		return ApplicationPackage{}, err
+	}
+	if err := pkg.Validate(); err != nil {
+		return ApplicationPackage{}, err
+	}
+	return pkg, nil
+}
+
+func findApplicationPackage(zipOrDir string, requirePackaging bool) (ApplicationPackage, error) {
 	if isZip(zipOrDir) {
 		return ApplicationPackage{Path: zipOrDir}, nil
 	}