refactor: enhance module renaming workflows and add validation

- Introduced `RenameProjectModule` to handle module renaming along with related files.
- Added validation for zip file size and enhanced `downloadZipReader` with timeout handling.
- Improved file path safety checks and error handling workflows.
- Updated related test cases to include comprehensive end-to-end integration testing for renaming and build processes.

Author: tbxark

cmd/rename.go

@@ -25,6 +25,12 @@ func init() {
 		if *oldMod == "" || *newMod == "" {
 			return errors.New("--old and --new are required")
 		}
-		return renamer.RenameDirModule(*oldMod, *newMod, *target)
+		if *oldMod == *newMod {
+			return errors.New("--old and --new must be different")
+		}
+		return renamer.RenameProjectModule(*oldMod, *newMod, *target, []string{
+			"buf.gen.yaml",
+			"buf.binding.yaml",
+		}, true)
 	}
 }

internal/create/create.go

@@ -153,21 +153,13 @@ func initGitRepo(target string) error {
 
 func renameGoModule(oldModName, newModName, target string) error {
 	log.Printf("rename module: %s -> %s", oldModName, newModName)
-	err := renamer.RenameDirModule(oldModName, newModName, target)
-	if err != nil {
-		return err
-	}
-	files := []string{
+	if err := renamer.RenameProjectModule(oldModName, newModName, target, []string{
 		"buf.gen.yaml",
 		"buf.binding.yaml",
+	}, false); err != nil {
+		return err
 	}
-	for _, file := range files {
-		e := replaceFileContent(oldModName, newModName, filepath.Join(target, file))
-		if e != nil {
-			return e
-		}
-	}
-	err = execCommands(target,
+	err := execCommands(target,
 		[]string{"go", "mod", "edit", "-module", newModName},
 		[]string{"make", "init"},
 		[]string{"go", "mod", "tidy"},
@@ -198,20 +190,3 @@ func execCommands(dir string, commands ...[]string) error {
 	}
 	return nil
 }
-
-func replaceFileContent(old, new, filePath string) error {
-	content, err := os.ReadFile(filePath)
-	if err != nil {
-		return err
-	}
-	replacer := strings.NewReplacer(old, new)
-	file, err := os.Create(filePath)
-	if err != nil {
-		return err
-	}
-	defer func() {
-		_ = file.Close()
-	}()
-	_, err = replacer.WriteString(file, string(content))
-	return err
-}

internal/create/create_test.go

@@ -3,7 +3,10 @@ package create
 import (
 	"flag"
 	"os"
+	"path/filepath"
 	"testing"
+
+	"github.com/go-sphere/sphere-cli/internal/renamer"
 )
 
 var createTest = flag.Bool("create_test", false, "run create tests that create files and directories")
@@ -33,3 +36,47 @@ func TestLayout(t *testing.T) {
 	}
 	_ = os.RemoveAll("simple")
 }
+
+func TestSimpleLayoutCreateAndRenameBuild(t *testing.T) {
+	if os.Getenv("CI") != "" {
+		t.Skip("Skipping integration test in CI")
+	}
+	if !*createTest {
+		t.Skip("Skipping integration test, run with -create_test to enable")
+	}
+
+	workspace := t.TempDir()
+	prevDir, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Chdir(workspace); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() {
+		_ = os.Chdir(prevDir)
+	})
+
+	projectName := "simple-e2e"
+	oldModule := "github.com/example/simple-e2e"
+	newModule := "github.com/example/simple-e2e-renamed"
+
+	if err := Project(projectName, oldModule, templateLayouts["simple"]); err != nil {
+		t.Fatal(err)
+	}
+
+	projectDir := filepath.Join(workspace, projectName)
+	if err := renamer.RenameProjectModule(oldModule, newModule, projectDir, []string{
+		"buf.gen.yaml",
+		"buf.binding.yaml",
+	}, true); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := execCommands(projectDir,
+		[]string{"make", "init"},
+		[]string{"make", "build"},
+	); err != nil {
+		t.Fatal(err)
+	}
+}

internal/renamer/renamer.go

@@ -1,19 +1,25 @@
 package renamer
 
 import (
+	"errors"
+	"fmt"
 	"go/ast"
 	"go/parser"
 	"go/printer"
 	"go/token"
+	"io/fs"
 	"log"
 	"os"
 	"path/filepath"
 	"strings"
 )
 
 func RenameDirModule(oldModule, newModule string, dir string) error {
-	err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
-		if info.IsDir() {
+	err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if d.IsDir() {
 			return nil
 		}
 		if strings.HasSuffix(path, ".go") {
@@ -24,6 +30,22 @@ func RenameDirModule(oldModule, newModule string, dir string) error {
 	if err != nil {
 		return err
 	}
+	return renameGoModuleFile(oldModule, newModule, filepath.Join(dir, "go.mod"))
+}
+
+func RenameProjectModule(oldModule, newModule, dir string, relatedFiles []string, ignoreMissingFiles bool) error {
+	if err := RenameDirModule(oldModule, newModule, dir); err != nil {
+		return err
+	}
+	for _, file := range relatedFiles {
+		filePath := filepath.Join(dir, file)
+		if err := replaceFileContent(oldModule, newModule, filePath); err != nil {
+			if ignoreMissingFiles && errors.Is(err, os.ErrNotExist) {
+				continue
+			}
+			return err
+		}
+	}
 	return nil
 }
 
@@ -60,3 +82,47 @@ func RenameModule(oldModule, newModule string, path string) error {
 	}
 	return nil
 }
+
+func renameGoModuleFile(oldModule, newModule, modPath string) error {
+	content, err := os.ReadFile(modPath)
+	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
+			return fmt.Errorf("go.mod not found in target: %s", modPath)
+		}
+		return err
+	}
+
+	lines := strings.Split(string(content), "\n")
+	found := false
+	for i, line := range lines {
+		trimmed := strings.TrimSpace(line)
+		if !strings.HasPrefix(trimmed, "module ") {
+			continue
+		}
+		fields := strings.Fields(trimmed)
+		if len(fields) < 2 {
+			return fmt.Errorf("invalid module directive in %s", modPath)
+		}
+		currentModule := strings.Trim(fields[1], `"`)
+		if oldModule != "" && currentModule != oldModule {
+			return fmt.Errorf("go.mod module mismatch: expected %q, got %q", oldModule, currentModule)
+		}
+		lines[i] = "module " + newModule
+		found = true
+		break
+	}
+
+	if !found {
+		return fmt.Errorf("module directive not found in %s", modPath)
+	}
+	return os.WriteFile(modPath, []byte(strings.Join(lines, "\n")), 0o644)
+}
+
+func replaceFileContent(old, new, filePath string) error {
+	content, err := os.ReadFile(filePath)
+	if err != nil {
+		return err
+	}
+	replaced := strings.ReplaceAll(string(content), old, new)
+	return os.WriteFile(filePath, []byte(replaced), 0o644)
+}

internal/zip/zip.go

@@ -9,10 +9,19 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
+)
+
+const (
+	httpTimeout     = 90 * time.Second
+	maxZipSizeBytes = 100 << 20 // 100 MiB
 )
 
 func downloadZipReader(url string) (*zip.Reader, func(), error) {
-	resp, err := http.Get(url)
+	client := http.Client{
+		Timeout: httpTimeout,
+	}
+	resp, err := client.Get(url)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -22,27 +31,50 @@ func downloadZipReader(url string) (*zip.Reader, func(), error) {
 	if resp.StatusCode != http.StatusOK {
 		return nil, nil, errors.New(resp.Status)
 	}
+	if resp.ContentLength > maxZipSizeBytes {
+		return nil, nil, fmt.Errorf("zip file too large: %d bytes", resp.ContentLength)
+	}
 	tempFile, err := os.CreateTemp("", "zip-*")
 	if err != nil {
 		return nil, nil, err
 	}
-	length, err := io.Copy(tempFile, resp.Body)
+	cleanup := func() {
+		_ = tempFile.Close()
+		_ = os.Remove(tempFile.Name())
+	}
+	length, err := io.Copy(tempFile, io.LimitReader(resp.Body, maxZipSizeBytes+1))
 	if err != nil {
+		cleanup()
 		return nil, nil, err
 	}
+	if length > maxZipSizeBytes {
+		cleanup()
+		return nil, nil, fmt.Errorf("zip file too large: exceeded %d bytes", maxZipSizeBytes)
+	}
 	reader, err := zip.NewReader(tempFile, length)
 	if err != nil {
+		cleanup()
 		return nil, nil, err
 	}
 	return reader, func() {
-		_ = tempFile.Close()
-		_ = os.Remove(tempFile.Name())
+		cleanup()
 	}, nil
 }
 
 func ensureSafePath(tempDir, fileName string) (string, error) {
-	filePath := filepath.Join(tempDir, fileName)
-	if !strings.HasPrefix(filePath, filepath.Clean(tempDir)) {
+	basePath, err := filepath.Abs(filepath.Clean(tempDir))
+	if err != nil {
+		return "", err
+	}
+	filePath, err := filepath.Abs(filepath.Join(basePath, fileName))
+	if err != nil {
+		return "", err
+	}
+	relPath, err := filepath.Rel(basePath, filePath)
+	if err != nil {
+		return "", err
+	}
+	if relPath == ".." || strings.HasPrefix(relPath, ".."+string(os.PathSeparator)) {
 		return "", fmt.Errorf("unsafe file path: %s", filePath)
 	}
 	return filePath, nil