diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8775d53..f19591f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,9 +30,9 @@ jobs: TZ: "America/Chicago" - name: golangci-lint - uses: golangci/golangci-lint-action@v3 + uses: golangci/golangci-lint-action@v7 with: - version: latest + version: v2.0.2 - name: install goveralls run: go install github.com/mattn/goveralls@latest diff --git a/.gitignore b/.gitignore index 3b735ec..bdf93ff 100644 --- a/.gitignore +++ b/.gitignore @@ -13,9 +13,9 @@ # Output of the go coverage tool, specifically when used with LiteIDE *.out - -# Dependency directories (remove the comment below to include it) -# vendor/ +*.cov # Go workspace file go.work + +**/CLAUDE.local.md diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..4ebff07 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,61 @@ +version: "2" +run: + concurrency: 4 +linters: + default: none + enable: + - copyloopvar + - gochecknoinits + - gocritic + - gosec + - govet + - ineffassign + - misspell + - nakedret + - prealloc + - revive + - staticcheck + - unconvert + - unparam + - unused + - testifylint + - nestif + settings: + goconst: + min-len: 2 + min-occurrences: 2 + gocritic: + disabled-checks: + - wrapperFunc + enabled-tags: + - performance + - style + - experimental + gocyclo: + min-complexity: 15 + lll: + line-length: 140 + misspell: + locale: US + exclusions: + generated: lax + rules: + - linters: + - gosec + text: 'G114: Use of net/http serve function that has no support for setting timeouts' + - linters: + - revive + - unparam + path: _test\.go$ + text: unused-parameter + paths: + - third_party$ + - builtin$ + - examples$ +formatters: + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/README.md b/README.md index f77c028..a8a16ec 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,7 @@ These capture utilities are useful for testing functions that write directly to The `containers` package provides several test containers for integration testing: - `SSHTestContainer`: SSH server container for testing SSH connections and operations +- `FTPTestContainer`: FTP server container for testing FTP file transfers and operations - `PostgresTestContainer`: PostgreSQL database container with automatic database creation - `MySQLTestContainer`: MySQL database container with automatic database creation - `MongoTestContainer`: MongoDB container with support for multiple versions (5, 6, 7) @@ -196,4 +197,35 @@ func TestWithS3(t *testing.T) { }) require.NoError(t, err) } + +// FTP test container +func TestWithFTP(t *testing.T) { + ctx := context.Background() + ftpContainer := containers.NewFTPTestContainer(ctx, t) + defer ftpContainer.Close(ctx) + + // Connection details + ftpHost := ftpContainer.GetIP() // Container host + ftpPort := ftpContainer.GetPort() // Container port (default: 2121) + ftpUser := ftpContainer.GetUser() // Default: "ftpuser" + ftpPassword := ftpContainer.GetPassword() // Default: "ftppass" + + // Upload a file + localFile := "/path/to/local/file.txt" + remotePath := "file.txt" + err := ftpContainer.SaveFile(ctx, localFile, remotePath) + require.NoError(t, err) + + // Download a file + downloadPath := "/path/to/download/location.txt" + err = ftpContainer.GetFile(ctx, remotePath, downloadPath) + require.NoError(t, err) + + // List files + entries, err := ftpContainer.ListFiles(ctx, "/") + require.NoError(t, err) + for _, entry := range entries { + fmt.Println(entry.Name, entry.Type) // Type: 0 for file, 1 for directory + } +} ``` \ No newline at end of file diff --git a/capture.go b/capture.go index d67300f..37418c1 100644 --- a/capture.go +++ b/capture.go @@ -1,3 +1,4 @@ +// Package testutils provides utilities for testing Go applications package testutils import ( diff --git a/capture_test.go b/capture_test.go index ddbd182..5468458 100644 --- a/capture_test.go +++ b/capture_test.go @@ -134,8 +134,14 @@ func TestCaptureStdoutAndStderr(t *testing.T) { wantOut: string([]byte{0, 1, 2, 3}), wantErr: string([]byte{4, 5, 6, 7}), f: func() { - os.Stdout.Write([]byte{0, 1, 2, 3}) - os.Stderr.Write([]byte{4, 5, 6, 7}) + _, err := os.Stdout.Write([]byte{0, 1, 2, 3}) + if err != nil { + t.Logf("failed to write to stdout: %v", err) + } + _, err = os.Stderr.Write([]byte{4, 5, 6, 7}) + if err != nil { + t.Logf("failed to write to stderr: %v", err) + } }, }, { diff --git a/containers/ftp.go b/containers/ftp.go new file mode 100644 index 0000000..ee75242 --- /dev/null +++ b/containers/ftp.go @@ -0,0 +1,389 @@ +// Package containers implements various test containers for integration testing +package containers + +import ( + "context" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/docker/go-connections/nat" + "github.com/jlaffaye/ftp" + "github.com/stretchr/testify/require" + "github.com/testcontainers/testcontainers-go" + "github.com/testcontainers/testcontainers-go/wait" +) + +// FTPTestContainer is a wrapper around a testcontainers.Container that provides an FTP server +// for testing purposes. It allows file uploads, downloads, and directory operations. +type FTPTestContainer struct { + Container testcontainers.Container + Host string + Port nat.Port // represents the *host* port struct + User string + Password string +} + +// NewFTPTestContainer uses delfer/alpine-ftp-server, minimal env vars, fixed host port mapping syntax. +func NewFTPTestContainer(ctx context.Context, t *testing.T) *FTPTestContainer { + const ( + defaultUser = "ftpuser" + defaultPassword = "ftppass" + pasvMinPort = "21000" // default passive port range for the image + pasvMaxPort = "21010" + fixedHostControlPort = "2121" + ) + + testcontainers.Logger = testcontainers.TestLogger(t) + + pasvPortRangeContainer := fmt.Sprintf("%s-%s", pasvMinPort, pasvMaxPort) + pasvPortRangeHost := fmt.Sprintf("%s-%s", pasvMinPort, pasvMaxPort) // map 1:1 + exposedPortsWithBinding := []string{ + fmt.Sprintf("%s:21/tcp", fixedHostControlPort), // "2121:21/tcp" + fmt.Sprintf("%s:%s/tcp", pasvPortRangeHost, pasvPortRangeContainer), // "21000-21010:21000-21010/tcp" + } + + imageName := "delfer/alpine-ftp-server:latest" + t.Logf("Using FTP server image: %s", imageName) + + req := testcontainers.ContainerRequest{ + Image: imageName, + ExposedPorts: exposedPortsWithBinding, + Env: map[string]string{ + "USERS": fmt.Sprintf("%s|%s", defaultUser, defaultPassword), + }, + WaitingFor: wait.ForListeningPort(nat.Port("21/tcp")).WithStartupTimeout(2 * time.Minute), + } + + t.Logf("creating FTP container using %s (minimal env vars, fixed host port %s)...", imageName, fixedHostControlPort) + container, err := testcontainers.GenericContainer(ctx, testcontainers.GenericContainerRequest{ + ContainerRequest: req, + Started: true, + }) + // create the container instance to use its methods + ftpContainer := &FTPTestContainer{} + + // error handling with detailed logging for container startup issues + if err != nil { + ftpContainer.logContainerError(ctx, t, container, err, imageName) + } + t.Logf("FTP container created and started (ID: %s)", container.GetContainerID()) + + host, err := container.Host(ctx) + require.NoError(t, err, "Failed to get container host") + + // since we requested a fixed port, construct the nat.Port struct directly + // we still call MappedPort just to ensure the container is properly exposing *something* for port 21 + _, err = container.MappedPort(ctx, "21") + require.NoError(t, err, "Failed to get mapped port info for container port 21/tcp (even though fixed)") + + // construct the Port struct based on our fixed request + fixedHostNatPort, err := nat.NewPort("tcp", fixedHostControlPort) + require.NoError(t, err, "Failed to create nat.Port for fixed host port") + + t.Logf("FTP container should be accessible at: %s:%s (Control Plane)", host, fixedHostControlPort) + t.Logf("FTP server using default config, passive ports %s mapped to host %s", pasvPortRangeContainer, pasvPortRangeHost) + + time.Sleep(1 * time.Second) + + return &FTPTestContainer{ + Container: container, + Host: host, + Port: fixedHostNatPort, // use the manually constructed nat.Port for the fixed host port + User: defaultUser, + Password: defaultPassword, + } +} + +// connect function (Use default EPSV enabled) +func (fc *FTPTestContainer) connect(ctx context.Context) (*ftp.ServerConn, error) { + opts := []ftp.DialOption{ + ftp.DialWithTimeout(30 * time.Second), + ftp.DialWithContext(ctx), + ftp.DialWithDebugOutput(os.Stdout), // keep for debugging + // *** Use default (EPSV enabled) *** + // ftp.DialWithDisabledEPSV(true), + } + + connStr := fc.ConnectionString() // will use the fixed host port (e.g., 2121) + fmt.Printf("Attempting FTP connection to: %s (User: %s)\n", connStr, fc.User) + + c, err := ftp.Dial(connStr, opts...) + if err != nil { + fmt.Printf("FTP Dial Error to %s: %v\n", connStr, err) + return nil, fmt.Errorf("failed to dial FTP server %s: %w", connStr, err) + } + fmt.Printf("FTP Dial successful to %s\n", connStr) + + fmt.Printf("Attempting FTP login with user: %s\n", fc.User) + if err := c.Login(fc.User, fc.Password); err != nil { + fmt.Printf("FTP Login Error for user %s: %v\n", fc.User, err) + if quitErr := c.Quit(); quitErr != nil { + fmt.Printf("Warning: error closing FTP connection: %v\n", quitErr) + } + return nil, fmt.Errorf("failed to login to FTP server with user %s: %w", fc.User, err) + } + fmt.Printf("FTP Login successful for user %s\n", fc.User) + + return c, nil +} + +// createDirRecursive creates directories recursively relative to the current working directory. +func (fc *FTPTestContainer) createDirRecursive(c *ftp.ServerConn, remotePath string) error { + parts := splitPath(remotePath) + if len(parts) == 0 { + return nil + } + + // get current directory and setup return to it after the operation + originalWD, err := fc.saveCurrentDirectory(c) + if err == nil { + defer fc.restoreWorkingDirectory(c, originalWD) + } + + // process each directory in the path + for _, part := range parts { + if err := fc.ensureDirectoryExists(c, part); err != nil { + return err + } + } + + fmt.Printf("Finished ensuring directory structure for: %s\n", remotePath) + return nil +} + +// saveCurrentDirectory gets the current working directory and handles any errors +func (fc *FTPTestContainer) saveCurrentDirectory(c *ftp.ServerConn) (string, error) { + originalWD, err := c.CurrentDir() + if err != nil { + fmt.Printf("Warning: failed to get current directory: %v. Will not return to original WD.\n", err) + return "", err + } + return originalWD, nil +} + +// restoreWorkingDirectory attempts to return to the original working directory +func (fc *FTPTestContainer) restoreWorkingDirectory(c *ftp.ServerConn, originalWD string) { + if originalWD == "" { + return + } + + fmt.Printf("Returning to original directory: %s\n", originalWD) + if err := c.ChangeDir(originalWD); err != nil { + // try to go to root first and then to the original directory + _ = c.ChangeDir("/") + if err2 := c.ChangeDir(originalWD); err2 != nil { + fmt.Printf("Warning: failed to return to original directory '%s' (even from root): %v\n", originalWD, err2) + } + } +} + +// ensureDirectoryExists checks if a directory exists, creates it if needed, and changes into it +func (fc *FTPTestContainer) ensureDirectoryExists(c *ftp.ServerConn, dirName string) error { + fmt.Printf("Checking/Changing into directory segment: %s (relative to current)\n", dirName) + + // first try to change into the directory to see if it exists + if err := c.ChangeDir(dirName); err == nil { + fmt.Printf("Directory %s exists, changed into it.\n", dirName) + return nil + } + + // directory doesn't exist or can't be accessed, try to create it + fmt.Printf("Directory %s does not exist or ChangeDir failed, attempting MakeDir...\n", dirName) + if err := c.MakeDir(dirName); err != nil { + return fc.handleMakeDirFailure(c, dirName, err) + } + + // successfully created the directory, now change into it + fmt.Printf("MakeDir %s succeeded. Changing into it...\n", dirName) + if err := c.ChangeDir(dirName); err != nil { + return fmt.Errorf("failed to change into newly created directory '%s': %w", dirName, err) + } + + fmt.Printf("Successfully changed into newly created directory: %s\n", dirName) + return nil +} + +// handleMakeDirFailure handles the case where MakeDir fails +// (often because the directory already exists but ChangeDir initially failed for some reason) +func (fc *FTPTestContainer) handleMakeDirFailure(c *ftp.ServerConn, dirName string, makeDirErr error) error { + fmt.Printf("MakeDir %s failed: %v. Checking again with ChangeDir...\n", dirName, makeDirErr) + if err := c.ChangeDir(dirName); err != nil { + return fmt.Errorf("failed to create directory '%s' (MakeDir error: %w) and "+ + "failed to change into it subsequently (ChangeDir error: %w)", + dirName, makeDirErr, err) + } + + fmt.Printf("ChangeDir %s succeeded after MakeDir failed. Assuming directory existed.\n", dirName) + return nil +} + +// ConnectionString returns the FTP connection string for this container +func (fc *FTPTestContainer) ConnectionString() string { + return fmt.Sprintf("%s:%d", fc.Host, fc.Port.Int()) +} + +// GetIP returns the host IP address +func (fc *FTPTestContainer) GetIP() string { return fc.Host } + +// GetPort returns the mapped port +func (fc *FTPTestContainer) GetPort() int { return fc.Port.Int() } + +// GetUser returns the FTP username +func (fc *FTPTestContainer) GetUser() string { return fc.User } + +// GetPassword returns the FTP password +func (fc *FTPTestContainer) GetPassword() string { return fc.Password } + +// GetFile downloads a file from the FTP server +func (fc *FTPTestContainer) GetFile(ctx context.Context, remotePath, localPath string) error { + c, err := fc.connect(ctx) + if err != nil { + return fmt.Errorf("failed to connect to FTP server for GetFile: %w", err) + } + defer func() { + if quitErr := c.Quit(); quitErr != nil { + fmt.Printf("Warning: error closing FTP connection: %v\n", quitErr) + } + }() + localDir := filepath.Dir(localPath) + if err := os.MkdirAll(localDir, 0o750); err != nil { + return fmt.Errorf("failed to create local directory %s: %w", localDir, err) + } + // create file with secure permissions, validating path is within expected directory + if !strings.HasPrefix(filepath.Clean(localPath), filepath.Clean(localDir)) { + return fmt.Errorf("localPath %s attempts to escape from directory %s", localPath, localDir) + } + f, err := os.OpenFile(localPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) + if err != nil { + return fmt.Errorf("failed to create local file %s: %w", localPath, err) + } + defer f.Close() + r, err := c.Retr(filepath.ToSlash(remotePath)) + if err != nil { + return fmt.Errorf("failed to retrieve remote file %s: %w", remotePath, err) + } + defer r.Close() + if _, err := io.Copy(f, r); err != nil { + return fmt.Errorf("failed to copy file content from %s to %s: %w", remotePath, localPath, err) + } + return nil +} + +// SaveFile uploads a file to the FTP server +func (fc *FTPTestContainer) SaveFile(ctx context.Context, localPath, remotePath string) error { + c, err := fc.connect(ctx) + if err != nil { + return fmt.Errorf("failed to connect to FTP server for SaveFile: %w", err) + } + defer func() { + if quitErr := c.Quit(); quitErr != nil { + fmt.Printf("Warning: error closing FTP connection: %v\n", quitErr) + } + }() + // validate the file path to prevent path traversal + if !strings.HasPrefix(filepath.Clean(localPath), filepath.Clean(filepath.Dir(localPath))) { + return fmt.Errorf("localPath %s attempts to escape from its directory", localPath) + } + f, err := os.Open(localPath) + if err != nil { + return fmt.Errorf("failed to open local file %s: %w", localPath, err) + } + defer f.Close() + remoteDir := filepath.Dir(filepath.ToSlash(remotePath)) + if remoteDir != "." && remoteDir != "/" { + if err := fc.createDirRecursive(c, remoteDir); err != nil { + return fmt.Errorf("failed to create remote directory %s: %w", remoteDir, err) + } + } + if err := c.Stor(filepath.ToSlash(remotePath), f); err != nil { + return fmt.Errorf("failed to store file %s as %s: %w", localPath, remotePath, err) + } + return nil +} + +// ListFiles lists files in a directory on the FTP server +func (fc *FTPTestContainer) ListFiles(ctx context.Context, remotePath string) ([]ftp.Entry, error) { + c, err := fc.connect(ctx) + if err != nil { + return nil, fmt.Errorf("failed to connect to FTP server for ListFiles: %w", err) + } + defer func() { + if quitErr := c.Quit(); quitErr != nil { + fmt.Printf("Warning: error closing FTP connection: %v\n", quitErr) + } + }() + cleanRemotePath := filepath.ToSlash(remotePath) + if cleanRemotePath == "" || cleanRemotePath == "." { + cleanRemotePath = "/" + } + ptrEntries, err := c.List(cleanRemotePath) + if err != nil { + return nil, fmt.Errorf("failed to list files in remote path '%s': %w", cleanRemotePath, err) + } + entries := make([]ftp.Entry, 0, len(ptrEntries)) + for _, entry := range ptrEntries { + if entry != nil { + entries = append(entries, *entry) + } + } + return entries, nil +} + +// Close terminates the container +func (fc *FTPTestContainer) Close(ctx context.Context) error { + if fc.Container != nil { + containerID := fc.Container.GetContainerID() + fmt.Printf("terminating FTP container %s...\n", containerID) + err := fc.Container.Terminate(ctx) + fc.Container = nil + if err != nil { + fmt.Printf("error terminating FTP container %s: %v\n", containerID, err) + return err + } + fmt.Printf("FTP container %s terminated.\n", containerID) + } + return nil +} +func splitPath(path string) []string { + cleanPath := filepath.ToSlash(path) + cleanPath = strings.Trim(cleanPath, "/") + if cleanPath == "" { + return []string{} + } + return strings.Split(cleanPath, "/") +} + +// logContainerError handles container startup errors with detailed logging +func (fc *FTPTestContainer) logContainerError(_ context.Context, t *testing.T, container testcontainers.Container, err error, imageName string) { + logCtx, logCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer logCancel() + + fc.logContainerLogs(logCtx, t, container) + require.NoError(t, err, "Failed to create or start FTP container %s", imageName) +} + +// logContainerLogs attempts to fetch and log container logs +func (fc *FTPTestContainer) logContainerLogs(ctx context.Context, t *testing.T, container testcontainers.Container) { + if container == nil { + t.Logf("Container object was nil after GenericContainer failure.") + return + } + + logs, logErr := container.Logs(ctx) + if logErr != nil { + t.Logf("Could not retrieve container logs after startup failure: %v", logErr) + return + } + + logBytes, _ := io.ReadAll(logs) + if closeErr := logs.Close(); closeErr != nil { + t.Logf("warning: failed to close logs reader: %v", closeErr) + } + + t.Logf("Container logs on startup failure:\n%s", string(logBytes)) +} diff --git a/containers/ftp_test.go b/containers/ftp_test.go new file mode 100644 index 0000000..355d986 --- /dev/null +++ b/containers/ftp_test.go @@ -0,0 +1,139 @@ +// file: testutils/containers/ftp_test.go +package containers + +import ( + "context" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestFTPContainer uses Testcontainers to manage the FTP server lifecycle. +func TestFTPContainer(t *testing.T) { + if testing.Short() { + t.Skip("skipping FTP container test in short mode") + } + if os.Getenv("CI") != "" && os.Getenv("RUN_FTP_TESTS_ON_CI") == "" { + t.Skip("skipping FTP container test in CI environment unless RUN_FTP_TESTS_ON_CI is set") + } + + ctx := context.Background() + ftpContainer := NewFTPTestContainer(ctx, t) // uses the final working logic from ftp.go + defer func() { assert.NoError(t, ftpContainer.Close(context.Background())) }() + + require.NotEmpty(t, ftpContainer.GetIP()) + require.Positive(t, ftpContainer.GetPort()) + require.Equal(t, "ftpuser", ftpContainer.GetUser()) + require.Equal(t, "ftppass", ftpContainer.GetPassword()) + connectionString := ftpContainer.ConnectionString() + require.Equal(t, fmt.Sprintf("%s:%d", ftpContainer.GetIP(), ftpContainer.GetPort()), connectionString) + t.Logf("connection string: %s", connectionString) + + t.Logf("FTP Connection: %s", ftpContainer.ConnectionString()) + t.Logf("FTP Username: %s", ftpContainer.GetUser()) + t.Logf("FTP Password: %s", ftpContainer.GetPassword()) + logs, err := ftpContainer.Container.Logs(ctx) + if err == nil { + defer logs.Close() + logBytes, _ := io.ReadAll(logs) + t.Logf("FTP Container logs: %s", string(logBytes)) + } else { + t.Logf("Could not get container logs: %v", err) + } + + tempDir := t.TempDir() + testFiles := map[string][]byte{ + "test1.txt": []byte("Hello world from test1"), + "test2.txt": []byte("This is test2 content: " + fmt.Sprint(time.Now().UnixNano())), + "testdir/nested.txt": []byte("Nested file"), + "testdir/more.txt": []byte("Another nested: " + time.Now().Format(time.RFC3339)), + } + for filename, content := range testFiles { + localPath := filepath.Join(tempDir, filename) + if dir := filepath.Dir(localPath); dir != tempDir { + require.NoError(t, os.MkdirAll(dir, 0o750)) + } + require.NoError(t, os.WriteFile(localPath, content, 0o600)) + } + + t.Run("Upload", func(t *testing.T) { + for filename := range testFiles { + localPath := filepath.Join(tempDir, filename) + remotePath := filename + err := ftpContainer.SaveFile(ctx, localPath, remotePath) + require.NoError(t, err, "Failed to upload file %s", filename) + } + }) + + t.Run("ListHome", func(t *testing.T) { + // *** List the explicit home directory path *** + // using "." might be less reliable if connection state resets + homeDir := "/ftp/ftpuser" // based on PWD logs + homeEntries, err := ftpContainer.ListFiles(ctx, homeDir) + require.NoError(t, err, "Failed to list home directory %s", homeDir) + require.NotEmpty(t, homeEntries, "Home directory %s should not be empty", homeDir) + + foundFiles := make(map[string]bool) + for _, entry := range homeEntries { + t.Logf("Found in home dir (%s): %s", homeDir, entry.Name) + // assuming 0 is file and 1 is folder + if (entry.Name == "test1.txt" || entry.Name == "test2.txt") && entry.Type == 0 { + foundFiles[entry.Name] = true + } + if entry.Name == "testdir" && entry.Type == 1 { + foundFiles[entry.Name] = true + } + } + assert.True(t, foundFiles["test1.txt"], "test1.txt should be in home directory") + assert.True(t, foundFiles["test2.txt"], "test2.txt should be in home directory") + assert.True(t, foundFiles["testdir"], "testdir should be in home directory") + }) + + t.Run("ListSubdir", func(t *testing.T) { + // list relative path "testdir" or absolute path "/ftp/ftpuser/testdir" + subdirPath := "testdir" // relative path should work fine here + subdirEntries, err := ftpContainer.ListFiles(ctx, subdirPath) + require.NoError(t, err, "Failed to list subdirectory '%s'", subdirPath) + require.NotEmpty(t, subdirEntries, "Subdirectory '%s' should not be empty", subdirPath) + + foundSubdirFiles := make(map[string]bool) + for _, entry := range subdirEntries { + t.Logf("Found in %s: %s", subdirPath, entry.Name) + if (entry.Name == "nested.txt" || entry.Name == "more.txt") && entry.Type == 0 { + foundSubdirFiles[entry.Name] = true + } + } + assert.True(t, foundSubdirFiles["nested.txt"], "nested.txt should be in subdirectory '%s'", subdirPath) + assert.True(t, foundSubdirFiles["more.txt"], "more.txt should be in subdirectory '%s'", subdirPath) + }) + + t.Run("DownloadAndVerify", func(t *testing.T) { + downloadDir := filepath.Join(tempDir, "download") + require.NoError(t, os.MkdirAll(downloadDir, 0o750)) + downloadFiles := []string{"test1.txt", "testdir/nested.txt"} + for _, filename := range downloadFiles { + originalContent, ok := testFiles[filename] + require.True(t, ok) + remotePath := filename + localRelPath := strings.ReplaceAll(filename, "/", string(filepath.Separator)) + downloadPath := filepath.Join(downloadDir, localRelPath) + if dir := filepath.Dir(downloadPath); dir != downloadDir { + require.NoError(t, os.MkdirAll(dir, 0o750)) + } + t.Logf("Downloading remote '%s' to local '%s'", remotePath, downloadPath) + require.NoError(t, ftpContainer.GetFile(ctx, remotePath, downloadPath), "Failed to download file %s", filename) + // read file is safe here since we control the downloadPath in tests + downloadedContent, err := os.ReadFile(downloadPath) // #nosec G304 -- safe file access in test + require.NoError(t, err) + require.Equal(t, originalContent, downloadedContent, "Content mismatch for file %s", filename) + t.Logf("Verified content for downloaded file %s", filename) + } + }) +} diff --git a/containers/mongo_test.go b/containers/mongo_test.go index 3c7de4f..c90b2cf 100644 --- a/containers/mongo_test.go +++ b/containers/mongo_test.go @@ -60,13 +60,19 @@ func TestMongoTestContainer(t *testing.T) { // save current MONGO_TEST value origEnv := os.Getenv("MONGO_TEST") testValue := "mongodb://original-value:27017" - os.Setenv("MONGO_TEST", testValue) + if err := os.Setenv("MONGO_TEST", testValue); err != nil { + t.Fatalf("Failed to set MONGO_TEST environment variable: %v", err) + } defer func() { // restore original value if origEnv == "" { - os.Unsetenv("MONGO_TEST") + if err := os.Unsetenv("MONGO_TEST"); err != nil { + t.Logf("Warning: failed to unset MONGO_TEST environment variable: %v", err) + } } else { - os.Setenv("MONGO_TEST", origEnv) + if err := os.Setenv("MONGO_TEST", origEnv); err != nil { + t.Logf("Warning: failed to restore MONGO_TEST environment variable: %v", err) + } } }() @@ -82,11 +88,15 @@ func TestMongoTestContainer(t *testing.T) { t.Run("close with no original environment variable", func(t *testing.T) { // save current MONGO_TEST value origEnv := os.Getenv("MONGO_TEST") - os.Unsetenv("MONGO_TEST") + if err := os.Unsetenv("MONGO_TEST"); err != nil { + t.Fatalf("Failed to unset MONGO_TEST environment variable: %v", err) + } defer func() { // restore original value if origEnv != "" { - os.Setenv("MONGO_TEST", origEnv) + if err := os.Setenv("MONGO_TEST", origEnv); err != nil { + t.Logf("Warning: failed to restore MONGO_TEST environment variable: %v", err) + } } }() diff --git a/containers/ssh_test.go b/containers/ssh_test.go index beceec2..59d6b75 100644 --- a/containers/ssh_test.go +++ b/containers/ssh_test.go @@ -3,7 +3,6 @@ package containers import ( "context" "os/exec" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -36,11 +35,13 @@ func TestSSHTestContainer(t *testing.T) { defer func() { require.NoError(t, ssh.Close(ctx)) }() // use ssh-keyscan to verify the host is accessible - cmd := exec.Command("ssh-keyscan", "-p", ssh.Port.Port(), ssh.Host) + // values come from the test container itself, so this is not vulnerable to command injection + // ssh.Port and ssh.Host are generated by the test container and are not user input + cmd := exec.Command("ssh-keyscan", "-p", ssh.Port.Port(), ssh.Host) // #nosec G204 -- these params are from our test container out, err := cmd.CombinedOutput() require.NoError(t, err) t.Logf("ssh-keyscan output: %s", out) - assert.True(t, strings.Contains(string(out), "ssh-"), "should return ssh key") + assert.Contains(t, string(out), "ssh-", "should return ssh key") }) t.Run("multiple containers", func(t *testing.T) { diff --git a/file_utils_test.go b/file_utils_test.go index adaf221..0b53f14 100644 --- a/file_utils_test.go +++ b/file_utils_test.go @@ -19,7 +19,10 @@ func TestWriteTestFile(t *testing.T) { require.False(t, os.IsNotExist(err), "WriteTestFile did not create file at %s", filePath) // check content - data, err := os.ReadFile(filePath) + // although ReadFile takes a path from a test-generated file, this is safe + // because the file is created in a temporary directory controlled by the test + // this is safe because the file path was created by WriteTestFile in a controlled manner + data, err := os.ReadFile(filePath) // #nosec G304 -- safe file access in test require.NoError(t, err, "Failed to read test file") require.Equal(t, content, string(data), "File content doesn't match expected") @@ -43,7 +46,10 @@ func TestWriteTestFile(t *testing.T) { content := "line 1\nline 2\nline 3" filePath := WriteTestFile(t, content) - data, err := os.ReadFile(filePath) + // although ReadFile takes a path from a test-generated file, this is safe + // because the file is created in a temporary directory controlled by the test + // this is safe because the file path was created by WriteTestFile in a controlled manner + data, err := os.ReadFile(filePath) // #nosec G304 -- safe file access in test require.NoError(t, err) require.Equal(t, content, string(data)) }) diff --git a/go.mod b/go.mod index 26c6897..c815398 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/s3 v1.77.1 github.com/docker/go-connections v0.5.0 github.com/go-sql-driver/mysql v1.9.0 + github.com/jlaffaye/ftp v0.2.0 github.com/lib/pq v1.10.9 github.com/stretchr/testify v1.10.0 github.com/testcontainers/testcontainers-go v0.35.0 @@ -50,6 +51,8 @@ require ( github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/snappy v0.0.4 // indirect github.com/google/uuid v1.6.0 // indirect + github.com/hashicorp/errwrap v1.1.0 // indirect + github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/klauspost/compress v1.17.4 // indirect github.com/kr/text v0.2.0 // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect diff --git a/go.sum b/go.sum index a534ef7..c6cea00 100644 --- a/go.sum +++ b/go.sum @@ -91,6 +91,13 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0 h1:YBftPWNWd4WwGqtY2yeZL2ef8rHAxPBD8KFhJpmcqms= github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0/go.mod h1:YN5jB8ie0yfIUg6VvR9Kz84aCaG7AsGZnLjhHbUqwPg= +github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= +github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= +github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= +github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= +github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= +github.com/jlaffaye/ftp v0.2.0 h1:lXNvW7cBu7R/68bknOX3MrRIIqZ61zELs1P2RAiA3lg= +github.com/jlaffaye/ftp v0.2.0/go.mod h1:is2Ds5qkhceAPy2xD6RLI6hmp/qysSoymZ+Z2uTnspI= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.17.4 h1:Ej5ixsIri7BrIjBkRZLTo6ghwrEtHFk7ijlczPW4fZ4= diff --git a/http_utils_test.go b/http_utils_test.go index c997d17..45b902b 100644 --- a/http_utils_test.go +++ b/http_utils_test.go @@ -17,8 +17,10 @@ func TestMockHTTPServer(t *testing.T) { w.WriteHeader(http.StatusOK) // in real request handling, the error is typically ignored as it's a disconnect which HTTP server handles // ResponseWriter interface doesn't have a way to check for errors in tests - // nolint:errcheck // http.ResponseWriter errors are handled by the HTTP server - w.Write([]byte(`{"status":"ok"}`)) + _, err := w.Write([]byte(`{"status":"ok"}`)) + if err != nil { + t.Logf("failed to write response: %v", err) + } }) // get the server URL and cleanup function @@ -68,8 +70,10 @@ func TestHTTPRequestCaptor(t *testing.T) { // create a test handler that will receive forwarded requests testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) - // nolint:errcheck // http.ResponseWriter errors are handled by the HTTP server - w.Write([]byte("response")) + _, err := w.Write([]byte("response")) + if err != nil { + t.Logf("failed to write response: %v", err) + } }) // create the request captor @@ -119,7 +123,7 @@ func TestHTTPRequestCaptor(t *testing.T) { // test GetRequests allRequests := captor.GetRequests() - require.Equal(t, 3, len(allRequests), "Wrong number of requests from GetRequests") + require.Len(t, allRequests, 3, "Wrong number of requests from GetRequests") // test Reset captor.Reset()