From 39244bd177303d3299b84e08340435fc4d55bd67 Mon Sep 17 00:00:00 2001 From: haroondilshad Date: Sun, 7 Sep 2025 00:06:56 +0200 Subject: [PATCH 1/2] fix: handle existing symlinks during tar extraction - Fix symlink extraction conflict when target file already exists - Check if existing symlink points to same target before removing - Add comprehensive tests for symlink conflict scenarios - Resolves the 'file exists' error during workspace uploads This fixes the issue where DevPod workspace uploads fail with: 'symlink GEMINI.md /path/to/AGENTS.md: file exists' The fix ensures that: 1. If a symlink already exists with the same target, it's preserved 2. If a symlink exists with a different target, it's replaced 3. If a regular file exists, it's replaced with the symlink 4. New symlinks are created normally --- pkg/extract/extract.go | 18 ++- pkg/extract/extract_test.go | 276 ++++++++++++++++++++++++++++++++++++ 2 files changed, 293 insertions(+), 1 deletion(-) create mode 100644 pkg/extract/extract_test.go diff --git a/pkg/extract/extract.go b/pkg/extract/extract.go index 55efc0031..2dd1dbd1d 100644 --- a/pkg/extract/extract.go +++ b/pkg/extract/extract.go @@ -119,9 +119,25 @@ func extractNext(tarReader *tar.Reader, destFolder string, options *Options) (bo return true, nil } else if header.Typeflag == tar.TypeSymlink { + // Check if a symlink or file already exists at the target location + if _, err := os.Lstat(outFileName); err == nil { + // File or symlink exists, check if it's the same symlink + if existingLink, err := os.Readlink(outFileName); err == nil { + // It's an existing symlink, check if it points to the same target + if existingLink == header.Linkname { + // Same symlink already exists, no need to recreate + return true, nil + } + } + // Different symlink or regular file exists, remove it first + if err := os.Remove(outFileName); err != nil { + return false, perrors.Wrapf(err, "remove existing file for symlink %s", outFileName) + } + } + err := os.Symlink(header.Linkname, outFileName) if err != nil { - return false, err + return false, perrors.Wrapf(err, "create symlink %s -> %s", outFileName, header.Linkname) } return true, nil diff --git a/pkg/extract/extract_test.go b/pkg/extract/extract_test.go new file mode 100644 index 000000000..30b8e274c --- /dev/null +++ b/pkg/extract/extract_test.go @@ -0,0 +1,276 @@ +package extract + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "os" + "path/filepath" + "testing" +) + +func TestExtractSymlinkConflicts(t *testing.T) { + // Create a temporary directory for testing + tempDir, err := os.MkdirTemp("", "extract_test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + tests := []struct { + name string + setupExisting func(string) error + expectedResult string + }{ + { + name: "create_new_symlink", + setupExisting: func(dir string) error { + // No existing file + return nil + }, + expectedResult: "target.txt", + }, + { + name: "replace_existing_symlink_different_target", + setupExisting: func(dir string) error { + // Create existing symlink with different target + return os.Symlink("old_target.txt", filepath.Join(dir, "test_symlink")) + }, + expectedResult: "target.txt", + }, + { + name: "preserve_existing_symlink_same_target", + setupExisting: func(dir string) error { + // Create existing symlink with same target + return os.Symlink("target.txt", filepath.Join(dir, "test_symlink")) + }, + expectedResult: "target.txt", + }, + { + name: "replace_existing_regular_file", + setupExisting: func(dir string) error { + // Create existing regular file + return os.WriteFile(filepath.Join(dir, "test_symlink"), []byte("content"), 0644) + }, + expectedResult: "target.txt", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testDir := filepath.Join(tempDir, tt.name) + err := os.MkdirAll(testDir, 0755) + if err != nil { + t.Fatalf("Failed to create test dir: %v", err) + } + + // Setup existing file/symlink if needed + if err := tt.setupExisting(testDir); err != nil { + t.Fatalf("Failed to setup existing file: %v", err) + } + + // Create tar archive with symlink + tarData := createTarWithSymlink(t, "test_symlink", "target.txt") + + // Extract the tar + err = Extract(bytes.NewReader(tarData), testDir) + if err != nil { + t.Fatalf("Extract failed: %v", err) + } + + // Verify the symlink was created correctly + symlinkPath := filepath.Join(testDir, "test_symlink") + linkTarget, err := os.Readlink(symlinkPath) + if err != nil { + t.Fatalf("Failed to read symlink: %v", err) + } + + if linkTarget != tt.expectedResult { + t.Errorf("Expected symlink target %q, got %q", tt.expectedResult, linkTarget) + } + }) + } +} + +func TestExtractSymlinkMultipleConflicts(t *testing.T) { + // Test the specific case from the bug report: AGENTS.md -> GEMINI.md + tempDir, err := os.MkdirTemp("", "extract_multi_test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + // Create the GEMINI.md file first (target of symlinks) + geminiPath := filepath.Join(tempDir, "GEMINI.md") + err = os.WriteFile(geminiPath, []byte("Gemini content"), 0644) + if err != nil { + t.Fatalf("Failed to create GEMINI.md: %v", err) + } + + // Create existing AGENTS.md symlink (this should be preserved/replaced correctly) + agentsPath := filepath.Join(tempDir, "AGENTS.md") + err = os.Symlink("GEMINI.md", agentsPath) + if err != nil { + t.Fatalf("Failed to create existing AGENTS.md symlink: %v", err) + } + + // Create tar archive with the same symlinks (simulating the re-upload scenario) + tarData := createTarWithMultipleSymlinks(t) + + // Extract should not fail even with existing symlinks + err = Extract(bytes.NewReader(tarData), tempDir) + if err != nil { + t.Fatalf("Extract failed with existing symlinks: %v", err) + } + + // Verify both symlinks point to GEMINI.md + agentsTarget, err := os.Readlink(agentsPath) + if err != nil { + t.Fatalf("Failed to read AGENTS.md symlink: %v", err) + } + if agentsTarget != "GEMINI.md" { + t.Errorf("Expected AGENTS.md -> GEMINI.md, got %q", agentsTarget) + } + + claudePath := filepath.Join(tempDir, "CLAUDE.md") + claudeTarget, err := os.Readlink(claudePath) + if err != nil { + t.Fatalf("Failed to read CLAUDE.md symlink: %v", err) + } + if claudeTarget != "GEMINI.md" { + t.Errorf("Expected CLAUDE.md -> GEMINI.md, got %q", claudeTarget) + } +} + +// Helper function to create a tar archive with a single symlink +func createTarWithSymlink(t *testing.T, linkName, target string) []byte { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + + // Add symlink to tar + hdr := &tar.Header{ + Name: linkName, + Linkname: target, + Typeflag: tar.TypeSymlink, + Mode: 0755, + } + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("Failed to write symlink header: %v", err) + } + + if err := tw.Close(); err != nil { + t.Fatalf("Failed to close tar writer: %v", err) + } + + return buf.Bytes() +} + +// Helper function to create a tar archive with multiple symlinks (GEMINI.md scenario) +func createTarWithMultipleSymlinks(t *testing.T) []byte { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + + // Add GEMINI.md file + geminiContent := []byte("Gemini content for AI assistants") + hdr := &tar.Header{ + Name: "GEMINI.md", + Size: int64(len(geminiContent)), + Mode: 0644, + } + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("Failed to write GEMINI.md header: %v", err) + } + if _, err := tw.Write(geminiContent); err != nil { + t.Fatalf("Failed to write GEMINI.md content: %v", err) + } + + // Add AGENTS.md symlink + hdr = &tar.Header{ + Name: "AGENTS.md", + Linkname: "GEMINI.md", + Typeflag: tar.TypeSymlink, + Mode: 0755, + } + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("Failed to write AGENTS.md symlink header: %v", err) + } + + // Add CLAUDE.md symlink + hdr = &tar.Header{ + Name: "CLAUDE.md", + Linkname: "GEMINI.md", + Typeflag: tar.TypeSymlink, + Mode: 0755, + } + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("Failed to write CLAUDE.md symlink header: %v", err) + } + + if err := tw.Close(); err != nil { + t.Fatalf("Failed to close tar writer: %v", err) + } + + return buf.Bytes() +} + +func TestExtractGzippedTarWithSymlinks(t *testing.T) { + // Test gzipped tar archives with symlinks + tempDir, err := os.MkdirTemp("", "extract_gzip_test") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + // Create gzipped tar with symlinks + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + + // Add a file + content := []byte("test content") + hdr := &tar.Header{ + Name: "test.txt", + Size: int64(len(content)), + Mode: 0644, + } + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("Failed to write file header: %v", err) + } + if _, err := tw.Write(content); err != nil { + t.Fatalf("Failed to write file content: %v", err) + } + + // Add symlink + hdr = &tar.Header{ + Name: "link.txt", + Linkname: "test.txt", + Typeflag: tar.TypeSymlink, + Mode: 0755, + } + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("Failed to write symlink header: %v", err) + } + + if err := tw.Close(); err != nil { + t.Fatalf("Failed to close tar writer: %v", err) + } + if err := gw.Close(); err != nil { + t.Fatalf("Failed to close gzip writer: %v", err) + } + + // Extract gzipped tar + err = Extract(bytes.NewReader(buf.Bytes()), tempDir) + if err != nil { + t.Fatalf("Extract failed: %v", err) + } + + // Verify symlink + linkPath := filepath.Join(tempDir, "link.txt") + target, err := os.Readlink(linkPath) + if err != nil { + t.Fatalf("Failed to read symlink: %v", err) + } + if target != "test.txt" { + t.Errorf("Expected symlink target test.txt, got %q", target) + } +} From 107594aa4bc4a8b2f2ab6f3ffb7dc5016c275054 Mon Sep 17 00:00:00 2001 From: haroondilshad Date: Sun, 7 Sep 2025 00:09:29 +0200 Subject: [PATCH 2/2] refactor: make symlink tests generic - Replace specific GEMINI.md/AGENTS.md test names with generic target.txt/link1.txt/link2.txt - Tests now focus on symlink behavior rather than specific file names - Maintains comprehensive coverage of symlink conflict scenarios --- pkg/extract/extract.go | 2 +- pkg/extract/extract_test.go | 70 ++++++++++++++++++------------------- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/pkg/extract/extract.go b/pkg/extract/extract.go index 2dd1dbd1d..b9b9f14e6 100644 --- a/pkg/extract/extract.go +++ b/pkg/extract/extract.go @@ -134,7 +134,7 @@ func extractNext(tarReader *tar.Reader, destFolder string, options *Options) (bo return false, perrors.Wrapf(err, "remove existing file for symlink %s", outFileName) } } - + err := os.Symlink(header.Linkname, outFileName) if err != nil { return false, perrors.Wrapf(err, "create symlink %s -> %s", outFileName, header.Linkname) diff --git a/pkg/extract/extract_test.go b/pkg/extract/extract_test.go index 30b8e274c..16950ef66 100644 --- a/pkg/extract/extract_test.go +++ b/pkg/extract/extract_test.go @@ -93,25 +93,25 @@ func TestExtractSymlinkConflicts(t *testing.T) { } func TestExtractSymlinkMultipleConflicts(t *testing.T) { - // Test the specific case from the bug report: AGENTS.md -> GEMINI.md + // Test multiple symlinks pointing to the same target with re-extraction tempDir, err := os.MkdirTemp("", "extract_multi_test") if err != nil { t.Fatalf("Failed to create temp dir: %v", err) } defer os.RemoveAll(tempDir) - // Create the GEMINI.md file first (target of symlinks) - geminiPath := filepath.Join(tempDir, "GEMINI.md") - err = os.WriteFile(geminiPath, []byte("Gemini content"), 0644) + // Create the target file first + targetPath := filepath.Join(tempDir, "target.txt") + err = os.WriteFile(targetPath, []byte("target content"), 0644) if err != nil { - t.Fatalf("Failed to create GEMINI.md: %v", err) + t.Fatalf("Failed to create target.txt: %v", err) } - // Create existing AGENTS.md symlink (this should be preserved/replaced correctly) - agentsPath := filepath.Join(tempDir, "AGENTS.md") - err = os.Symlink("GEMINI.md", agentsPath) + // Create existing symlinks (this should be preserved/replaced correctly) + link1Path := filepath.Join(tempDir, "link1.txt") + err = os.Symlink("target.txt", link1Path) if err != nil { - t.Fatalf("Failed to create existing AGENTS.md symlink: %v", err) + t.Fatalf("Failed to create existing link1.txt symlink: %v", err) } // Create tar archive with the same symlinks (simulating the re-upload scenario) @@ -123,22 +123,22 @@ func TestExtractSymlinkMultipleConflicts(t *testing.T) { t.Fatalf("Extract failed with existing symlinks: %v", err) } - // Verify both symlinks point to GEMINI.md - agentsTarget, err := os.Readlink(agentsPath) + // Verify both symlinks point to target.txt + link1Target, err := os.Readlink(link1Path) if err != nil { - t.Fatalf("Failed to read AGENTS.md symlink: %v", err) + t.Fatalf("Failed to read link1.txt symlink: %v", err) } - if agentsTarget != "GEMINI.md" { - t.Errorf("Expected AGENTS.md -> GEMINI.md, got %q", agentsTarget) + if link1Target != "target.txt" { + t.Errorf("Expected link1.txt -> target.txt, got %q", link1Target) } - claudePath := filepath.Join(tempDir, "CLAUDE.md") - claudeTarget, err := os.Readlink(claudePath) + link2Path := filepath.Join(tempDir, "link2.txt") + link2Target, err := os.Readlink(link2Path) if err != nil { - t.Fatalf("Failed to read CLAUDE.md symlink: %v", err) + t.Fatalf("Failed to read link2.txt symlink: %v", err) } - if claudeTarget != "GEMINI.md" { - t.Errorf("Expected CLAUDE.md -> GEMINI.md, got %q", claudeTarget) + if link2Target != "target.txt" { + t.Errorf("Expected link2.txt -> target.txt, got %q", link2Target) } } @@ -165,45 +165,45 @@ func createTarWithSymlink(t *testing.T, linkName, target string) []byte { return buf.Bytes() } -// Helper function to create a tar archive with multiple symlinks (GEMINI.md scenario) +// Helper function to create a tar archive with multiple symlinks func createTarWithMultipleSymlinks(t *testing.T) []byte { var buf bytes.Buffer tw := tar.NewWriter(&buf) - // Add GEMINI.md file - geminiContent := []byte("Gemini content for AI assistants") + // Add target file + targetContent := []byte("target content") hdr := &tar.Header{ - Name: "GEMINI.md", - Size: int64(len(geminiContent)), + Name: "target.txt", + Size: int64(len(targetContent)), Mode: 0644, } if err := tw.WriteHeader(hdr); err != nil { - t.Fatalf("Failed to write GEMINI.md header: %v", err) + t.Fatalf("Failed to write target.txt header: %v", err) } - if _, err := tw.Write(geminiContent); err != nil { - t.Fatalf("Failed to write GEMINI.md content: %v", err) + if _, err := tw.Write(targetContent); err != nil { + t.Fatalf("Failed to write target.txt content: %v", err) } - // Add AGENTS.md symlink + // Add first symlink hdr = &tar.Header{ - Name: "AGENTS.md", - Linkname: "GEMINI.md", + Name: "link1.txt", + Linkname: "target.txt", Typeflag: tar.TypeSymlink, Mode: 0755, } if err := tw.WriteHeader(hdr); err != nil { - t.Fatalf("Failed to write AGENTS.md symlink header: %v", err) + t.Fatalf("Failed to write link1.txt symlink header: %v", err) } - // Add CLAUDE.md symlink + // Add second symlink hdr = &tar.Header{ - Name: "CLAUDE.md", - Linkname: "GEMINI.md", + Name: "link2.txt", + Linkname: "target.txt", Typeflag: tar.TypeSymlink, Mode: 0755, } if err := tw.WriteHeader(hdr); err != nil { - t.Fatalf("Failed to write CLAUDE.md symlink header: %v", err) + t.Fatalf("Failed to write link2.txt symlink header: %v", err) } if err := tw.Close(); err != nil {