diff --git a/internal/controller/gitrepository_controller.go b/internal/controller/gitrepository_controller.go index 1208c8ae0..c9a2bd1c5 100644 --- a/internal/controller/gitrepository_controller.go +++ b/internal/controller/gitrepository_controller.go @@ -860,6 +860,16 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *pat } defer unlock() + // Resolve symlinks before archiving to ensure their content is included + if err := resolveSymlinks(dir); err != nil { + e := serror.NewGeneric( + fmt.Errorf("failed to resolve symlinks in repository: %w", err), + sourcev1.ArchiveOperationFailedReason, + ) + conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } + // Load ignore rules for archiving ignoreDomain := strings.Split(dir, string(filepath.Separator)) ps, err := sourceignore.LoadIgnorePatterns(dir, ignoreDomain) @@ -1335,6 +1345,199 @@ func commitReference(obj *sourcev1.GitRepository, commit *git.Commit) string { return commit.String() } +// resolveSymlinks recursively resolves symlinks in the given directory by replacing +// them with copies of their target files/directories. This ensures that symlink +// content is included in the archive, as the Archive function skips symlinks. +// Symlinks pointing outside the root directory are skipped for security reasons. +func resolveSymlinks(rootDir string) error { + rootDir, err := filepath.Abs(rootDir) + if err != nil { + return fmt.Errorf("failed to get absolute path: %w", err) + } + + // First pass: collect all symlinks + type symlinkInfo struct { + path string + target string + } + var symlinks []symlinkInfo + + err = filepath.Walk(rootDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Use Lstat to check for symlinks + lstatInfo, err := os.Lstat(path) + if err != nil { + return err + } + + // Check if this is a symlink + if lstatInfo.Mode()&os.ModeSymlink != 0 { + // Resolve the symlink target + target, err := os.Readlink(path) + if err != nil { + return fmt.Errorf("failed to read symlink %s: %w", path, err) + } + + // Make target path absolute if it's relative + if !filepath.IsAbs(target) { + target = filepath.Join(filepath.Dir(path), target) + } + target, err = filepath.Abs(target) + if err != nil { + return fmt.Errorf("failed to resolve symlink target %s: %w", target, err) + } + + // Security check: ensure target is within root directory + // First check: target must be an absolute path that starts with rootDir + if !strings.HasPrefix(target, rootDir+string(filepath.Separator)) && target != rootDir { + // Symlink points outside root directory - skip it + return nil + } + // Second check: use filepath.Rel to catch edge cases with ../ + relPath, err := filepath.Rel(rootDir, target) + if err != nil || strings.HasPrefix(relPath, "..") { + // Symlink points outside root directory - skip it + return nil + } + + symlinks = append(symlinks, symlinkInfo{ + path: path, + target: target, + }) + } + + return nil + }) + if err != nil { + return err + } + + // Second pass: resolve symlinks (process in reverse order to handle nested symlinks) + for i := len(symlinks) - 1; i >= 0; i-- { + sym := symlinks[i] + + // Check if target still exists + targetInfo, err := os.Lstat(sym.target) + if err != nil { + // Target doesn't exist - skip broken symlink + continue + } + + // Remove the symlink + if err := os.Remove(sym.path); err != nil { + return fmt.Errorf("failed to remove symlink %s: %w", sym.path, err) + } + + // Copy target to symlink location + if targetInfo.IsDir() { + // Copy directory recursively + if err := copyDir(sym.target, sym.path); err != nil { + return fmt.Errorf("failed to copy directory from %s to %s: %w", sym.target, sym.path, err) + } + } else { + // Copy file + if err := copyFile(sym.target, sym.path); err != nil { + return fmt.Errorf("failed to copy file from %s to %s: %w", sym.target, sym.path, err) + } + } + } + + return nil +} + +// copyFile copies a file from src to dst. +func copyFile(src, dst string) error { + sourceFile, err := os.Open(src) + if err != nil { + return err + } + defer sourceFile.Close() + + destFile, err := os.Create(dst) + if err != nil { + return err + } + defer destFile.Close() + + _, err = destFile.ReadFrom(sourceFile) + if err != nil { + return err + } + + // Preserve file mode + srcInfo, err := os.Stat(src) + if err != nil { + return err + } + return os.Chmod(dst, srcInfo.Mode()) +} + +// copyDir recursively copies a directory from src to dst. +func copyDir(src, dst string) error { + srcInfo, err := os.Stat(src) + if err != nil { + return err + } + + if err := os.MkdirAll(dst, srcInfo.Mode()); err != nil { + return err + } + + entries, err := os.ReadDir(src) + if err != nil { + return err + } + + for _, entry := range entries { + srcPath := filepath.Join(src, entry.Name()) + dstPath := filepath.Join(dst, entry.Name()) + + if entry.IsDir() { + if err := copyDir(srcPath, dstPath); err != nil { + return err + } + } else { + // Check if it's a symlink + info, err := os.Lstat(srcPath) + if err != nil { + return err + } + if info.Mode()&os.ModeSymlink != 0 { + // Resolve symlink recursively + target, err := os.Readlink(srcPath) + if err != nil { + return err + } + if !filepath.IsAbs(target) { + target = filepath.Join(filepath.Dir(srcPath), target) + } + targetInfo, err := os.Lstat(target) + if err != nil { + continue // Skip broken symlink + } + if targetInfo.IsDir() { + if err := copyDir(target, dstPath); err != nil { + return err + } + } else { + if err := copyFile(target, dstPath); err != nil { + return err + } + } + } else { + if err := copyFile(srcPath, dstPath); err != nil { + return err + } + } + } + } + + return nil +} + // requiresVerification inspects a GitRepository's verification spec and its status // to determine whether the Git repository needs to be verified again. It does so by // first checking if the GitRepository has a verification spec. If it does, then diff --git a/internal/controller/gitrepository_controller_test.go b/internal/controller/gitrepository_controller_test.go index f9f7a591d..524df30f9 100644 --- a/internal/controller/gitrepository_controller_test.go +++ b/internal/controller/gitrepository_controller_test.go @@ -17,10 +17,13 @@ limitations under the License. package controller import ( + "archive/tar" + "compress/gzip" "context" "encoding/json" "errors" "fmt" + "io" "net/http" "net/url" "os" @@ -1498,6 +1501,365 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { } } +func TestGitRepositoryReconciler_reconcileArtifact_symlinks(t *testing.T) { + g := NewWithT(t) + + // Create a temporary directory structure with symlinks + tmpDir := t.TempDir() + + // Create target directory with content + targetDir := filepath.Join(tmpDir, "library", "cozy-lib") + g.Expect(os.MkdirAll(targetDir, 0o750)).To(Succeed()) + + // Create a file in the target directory + targetFile := filepath.Join(targetDir, "Chart.yaml") + g.Expect(os.WriteFile(targetFile, []byte("name: cozy-lib\nversion: 1.0.0\n"), 0o600)).To(Succeed()) + + // Create a subdirectory with content + templatesDir := filepath.Join(targetDir, "templates") + g.Expect(os.MkdirAll(templatesDir, 0o750)).To(Succeed()) + templateFile := filepath.Join(templatesDir, "deployment.yaml") + g.Expect(os.WriteFile(templateFile, []byte("apiVersion: apps/v1\n"), 0o600)).To(Succeed()) + + // Create source directory structure + sourceDir := filepath.Join(tmpDir, "apps", "tenant", "charts") + g.Expect(os.MkdirAll(sourceDir, 0o750)).To(Succeed()) + + // Create symlink pointing to target directory + symlinkPath := filepath.Join(sourceDir, "cozy-lib") + relativeTarget := filepath.Join("..", "..", "library", "cozy-lib") + g.Expect(os.Symlink(relativeTarget, symlinkPath)).To(Succeed()) + + // Create a regular file in source directory + regularFile := filepath.Join(sourceDir, "Chart.yaml") + g.Expect(os.WriteFile(regularFile, []byte("name: tenant\nversion: 1.0.0\n"), 0o600)).To(Succeed()) + + server, err := testserver.NewTempArtifactServer() + g.Expect(err).NotTo(HaveOccurred()) + server.Start() + defer server.Stop() + storage, err := newTestStorage(server.HTTPServer) + g.Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(storage.BasePath) + + r := &GitRepositoryReconciler{ + EventRecorder: record.NewFakeRecorder(32), + Storage: storage, + features: features.FeatureGates(), + patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), + } + + obj := &sourcev1.GitRepository{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "reconcile-artifact-symlinks-", + Generation: 1, + }, + Status: sourcev1.GitRepositoryStatus{}, + Spec: sourcev1.GitRepositorySpec{ + Interval: metav1.Duration{Duration: interval}, + }, + } + + commit := git.Commit{ + Hash: []byte("test-commit-hash"), + Reference: "refs/heads/main", + } + var includes artifactSet + sp := patch.NewSerialPatcher(obj, r.Client) + + got, err := r.reconcileArtifact(ctx, sp, obj, &commit, &includes, tmpDir) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).To(Equal(sreconcile.ResultSuccess)) + g.Expect(obj.GetArtifact()).ToNot(BeNil()) + + // Verify that the artifact was created + artifactPath := storage.LocalPath(*obj.GetArtifact()) + g.Expect(artifactPath).To(BeAnExistingFile()) + + // Extract and verify archive contents + extractDir := t.TempDir() + g.Expect(extractArchive(artifactPath, extractDir)).To(Succeed()) + + // Verify that symlink content is included in the archive + // The symlink should be resolved and its content should be present + symlinkResolvedPath := filepath.Join(extractDir, "apps", "tenant", "charts", "cozy-lib") + g.Expect(symlinkResolvedPath).To(BeADirectory()) + + // Verify that files from the symlink target are present + chartFile := filepath.Join(symlinkResolvedPath, "Chart.yaml") + g.Expect(chartFile).To(BeAnExistingFile()) + content, err := os.ReadFile(chartFile) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(string(content)).To(ContainSubstring("name: cozy-lib")) + + // Verify that subdirectories are included + templatesPath := filepath.Join(symlinkResolvedPath, "templates", "deployment.yaml") + g.Expect(templatesPath).To(BeAnExistingFile()) + + // Verify that regular files are still present + regularFilePath := filepath.Join(extractDir, "apps", "tenant", "charts", "Chart.yaml") + g.Expect(regularFilePath).To(BeAnExistingFile()) + + // Verify that the symlink itself is NOT present (it should be replaced) + // Check that it's a directory, not a symlink + info, err := os.Lstat(symlinkResolvedPath) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(info.Mode()&os.ModeSymlink).To(BeZero(), "symlink should be resolved to a directory") +} + +// TestGitRepositoryReconciler_resolveSymlinks_security tests that symlinks pointing +// outside the repository root are properly blocked to prevent directory traversal attacks. +// This is critical for security when processing untrusted Git repositories. +func TestGitRepositoryReconciler_resolveSymlinks_security(t *testing.T) { + g := NewWithT(t) + + // Create a temporary directory structure simulating a real system + tmpDir := t.TempDir() + + // Create repository root directory + repoRoot := filepath.Join(tmpDir, "repo") + g.Expect(os.MkdirAll(repoRoot, 0o750)).To(Succeed()) + + // Create a safe file inside the repo + safeFile := filepath.Join(repoRoot, "safe.txt") + g.Expect(os.WriteFile(safeFile, []byte("safe content"), 0o600)).To(Succeed()) + + // Create sensitive directories OUTSIDE the repo (simulating system directories) + outsideDir := filepath.Join(tmpDir, "sensitive") + g.Expect(os.MkdirAll(outsideDir, 0o750)).To(Succeed()) + + sensitiveFile := filepath.Join(outsideDir, "secret.txt") + g.Expect(os.WriteFile(sensitiveFile, []byte("SECRET_DATA"), 0o600)).To(Succeed()) + + // Create another sensitive directory at a different level + etcDir := filepath.Join(tmpDir, "etc") + g.Expect(os.MkdirAll(etcDir, 0o750)).To(Succeed()) + passwdFile := filepath.Join(etcDir, "passwd") + g.Expect(os.WriteFile(passwdFile, []byte("root:x:0:0:root:/root:/bin/bash"), 0o600)).To(Succeed()) + + // Test case 1: Relative symlink pointing outside with ../ + symlinkPath1 := filepath.Join(repoRoot, "malicious-link-relative") + relativeTarget1 := filepath.Join("..", "sensitive", "secret.txt") + g.Expect(os.Symlink(relativeTarget1, symlinkPath1)).To(Succeed()) + + // Test case 2: Multiple levels of ../ + symlinkPath2 := filepath.Join(repoRoot, "malicious-link-deep") + relativeTarget2 := filepath.Join("..", "..", "etc", "passwd") + g.Expect(os.Symlink(relativeTarget2, symlinkPath2)).To(Succeed()) + + // Test case 3: Absolute symlink pointing outside + symlinkPath3 := filepath.Join(repoRoot, "malicious-link-absolute") + g.Expect(os.Symlink(sensitiveFile, symlinkPath3)).To(Succeed()) + + // Test case 4: Symlink pointing to parent directory + symlinkPath4 := filepath.Join(repoRoot, "malicious-link-parent") + g.Expect(os.Symlink("..", symlinkPath4)).To(Succeed()) + + // Test case 5: Symlink with absolute path to /tmp or other system paths + // (if running on Unix-like system) + if tmpBase := os.TempDir(); tmpBase != "" { + systemTmpFile := filepath.Join(tmpBase, "system-secret") + g.Expect(os.WriteFile(systemTmpFile, []byte("SYSTEM_SECRET"), 0o600)).To(Succeed()) + defer os.Remove(systemTmpFile) + + symlinkPath5 := filepath.Join(repoRoot, "malicious-link-system") + g.Expect(os.Symlink(systemTmpFile, symlinkPath5)).To(Succeed()) + } + + // Test case 6: Valid symlink pointing inside repo (should be allowed) + validSymlinkPath := filepath.Join(repoRoot, "valid-link") + g.Expect(os.Symlink("safe.txt", validSymlinkPath)).To(Succeed()) + + // Test case 7: Symlink pointing to subdirectory inside repo (should be allowed) + subDir := filepath.Join(repoRoot, "subdir") + g.Expect(os.MkdirAll(subDir, 0o750)).To(Succeed()) + subFile := filepath.Join(subDir, "subfile.txt") + g.Expect(os.WriteFile(subFile, []byte("sub content"), 0o600)).To(Succeed()) + + validSymlinkPath2 := filepath.Join(repoRoot, "valid-link-subdir") + g.Expect(os.Symlink("subdir", validSymlinkPath2)).To(Succeed()) + + // Now resolve symlinks - malicious ones should be skipped + err := resolveSymlinks(repoRoot) + g.Expect(err).NotTo(HaveOccurred()) + + // Verify malicious symlinks are still symlinks (NOT resolved) + info1, err := os.Lstat(symlinkPath1) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(info1.Mode()&os.ModeSymlink).NotTo(BeZero(), "malicious relative symlink should NOT be resolved") + + info2, err := os.Lstat(symlinkPath2) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(info2.Mode()&os.ModeSymlink).NotTo(BeZero(), "malicious deep relative symlink should NOT be resolved") + + info3, err := os.Lstat(symlinkPath3) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(info3.Mode()&os.ModeSymlink).NotTo(BeZero(), "malicious absolute symlink should NOT be resolved") + + info4, err := os.Lstat(symlinkPath4) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(info4.Mode()&os.ModeSymlink).NotTo(BeZero(), "malicious parent symlink should NOT be resolved") + + // Verify sensitive files were NOT copied into repo + _, err = os.Stat(filepath.Join(repoRoot, "secret.txt")) + g.Expect(os.IsNotExist(err)).To(BeTrue(), "sensitive file should NOT be copied into repo") + + _, err = os.Stat(filepath.Join(repoRoot, "passwd")) + g.Expect(os.IsNotExist(err)).To(BeTrue(), "system file should NOT be copied into repo") + + // Verify valid symlinks ARE resolved + infoValid, err := os.Lstat(validSymlinkPath) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(infoValid.Mode()&os.ModeSymlink).To(BeZero(), "valid symlink should be resolved") + g.Expect(validSymlinkPath).To(BeARegularFile()) + content, err := os.ReadFile(validSymlinkPath) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(string(content)).To(Equal("safe content")) + + infoValid2, err := os.Lstat(validSymlinkPath2) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(infoValid2.Mode()&os.ModeSymlink).To(BeZero(), "valid subdirectory symlink should be resolved") + g.Expect(validSymlinkPath2).To(BeADirectory()) + + // Verify original sensitive files still exist outside repo (untouched) + g.Expect(sensitiveFile).To(BeAnExistingFile()) + sensitiveContent, err := os.ReadFile(sensitiveFile) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(string(sensitiveContent)).To(Equal("SECRET_DATA"), "sensitive file should remain untouched") + + g.Expect(passwdFile).To(BeAnExistingFile()) + passwdContent, err := os.ReadFile(passwdFile) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(string(passwdContent)).To(ContainSubstring("root"), "system file should remain untouched") +} + +// TestGitRepositoryReconciler_resolveSymlinks_security_edgeCases tests edge cases +// for symlink security, including various path manipulation attempts. +func TestGitRepositoryReconciler_resolveSymlinks_security_edgeCases(t *testing.T) { + g := NewWithT(t) + + tmpDir := t.TempDir() + repoRoot := filepath.Join(tmpDir, "repo") + g.Expect(os.MkdirAll(repoRoot, 0o750)).To(Succeed()) + + // Create target outside repo + outsideTarget := filepath.Join(tmpDir, "outside", "target.txt") + g.Expect(os.MkdirAll(filepath.Dir(outsideTarget), 0o750)).To(Succeed()) + g.Expect(os.WriteFile(outsideTarget, []byte("outside"), 0o600)).To(Succeed()) + + // Edge case 1: Symlink with mixed absolute/relative components + symlink1 := filepath.Join(repoRoot, "edge1") + // This creates a path that resolves outside but uses relative components + g.Expect(os.Symlink(filepath.Join("..", "outside", "target.txt"), symlink1)).To(Succeed()) + + // Edge case 2: Symlink pointing to root directory + symlink2 := filepath.Join(repoRoot, "edge2") + rootPath := filepath.VolumeName(tmpDir) + string(filepath.Separator) + if rootPath == "" { + // On Unix, try /etc + rootPath = "/etc" + if _, err := os.Stat(rootPath); os.IsNotExist(err) { + rootPath = tmpDir // Fallback if /etc doesn't exist + } + } + g.Expect(os.Symlink(rootPath, symlink2)).To(Succeed()) + + // Edge case 3: Symlink with null bytes (should be handled safely) + // Note: Go's filepath operations should handle this, but we test anyway + symlink3 := filepath.Join(repoRoot, "edge3") + g.Expect(os.Symlink("../outside/target.txt", symlink3)).To(Succeed()) + + // Edge case 4: Symlink pointing to itself (should not cause infinite loop) + symlink4 := filepath.Join(repoRoot, "edge4") + g.Expect(os.Symlink("edge4", symlink4)).To(Succeed()) + + // Edge case 5: Symlink chain that eventually points outside + chain1 := filepath.Join(repoRoot, "chain1") + chain2 := filepath.Join(repoRoot, "chain2") + g.Expect(os.Symlink("chain2", chain1)).To(Succeed()) + g.Expect(os.Symlink("../outside/target.txt", chain2)).To(Succeed()) + + // Resolve symlinks + err := resolveSymlinks(repoRoot) + g.Expect(err).NotTo(HaveOccurred()) + + // All malicious symlinks should remain as symlinks + info1, _ := os.Lstat(symlink1) + g.Expect(info1.Mode()&os.ModeSymlink).NotTo(BeZero(), "edge case 1 should not be resolved") + + info2, _ := os.Lstat(symlink2) + g.Expect(info2.Mode()&os.ModeSymlink).NotTo(BeZero(), "edge case 2 should not be resolved") + + info3, _ := os.Lstat(symlink3) + g.Expect(info3.Mode()&os.ModeSymlink).NotTo(BeZero(), "edge case 3 should not be resolved") + + // Self-referencing symlink should remain (or be handled safely) + info4, err := os.Lstat(symlink4) + if err == nil { + // If it exists, it should still be a symlink (not resolved to avoid loops) + g.Expect(info4.Mode()&os.ModeSymlink).NotTo(BeZero(), "self-referencing symlink should not be resolved") + } + + // Chain symlinks should remain + infoChain1, _ := os.Lstat(chain1) + g.Expect(infoChain1.Mode()&os.ModeSymlink).NotTo(BeZero(), "chain symlink should not be resolved") + + // Verify outside file was not copied + _, err = os.Stat(filepath.Join(repoRoot, "target.txt")) + g.Expect(os.IsNotExist(err)).To(BeTrue(), "outside file should not be copied") +} + +// extractArchive extracts a tar.gz archive to the given directory +func extractArchive(archivePath, destDir string) error { + file, err := os.Open(archivePath) + if err != nil { + return err + } + defer file.Close() + + gzr, err := gzip.NewReader(file) + if err != nil { + return err + } + defer gzr.Close() + + tr := tar.NewReader(gzr) + for { + header, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + return err + } + + target := filepath.Join(destDir, header.Name) + switch header.Typeflag { + case tar.TypeDir: + if err := os.MkdirAll(target, os.FileMode(header.Mode)); err != nil { + return err + } + case tar.TypeReg: + if err := os.MkdirAll(filepath.Dir(target), 0o750); err != nil { + return err + } + f, err := os.Create(target) + if err != nil { + return err + } + if _, err := io.Copy(f, tr); err != nil { + f.Close() + return err + } + f.Close() + if err := os.Chmod(target, os.FileMode(header.Mode)); err != nil { + return err + } + } + } + return nil +} + func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) { g := NewWithT(t)