fix(bundler): harden filepath.Join with SafeJoin for path-traversal protection

.golangci.yaml

@@ -144,6 +144,7 @@ linters:
           - errcheck
           - funlen
           - gocognit
+          - goconst
           - gocyclo
           - gosec
           - noctx

pkg/bundler/attestation/binary.go

@@ -24,6 +24,7 @@ import (
 	"google.golang.org/protobuf/encoding/protojson"
 
 	"github.com/NVIDIA/aicr/pkg/bundler/checksum"
+	"github.com/NVIDIA/aicr/pkg/bundler/deployer"
 	"github.com/NVIDIA/aicr/pkg/errors"
 )
 
@@ -47,7 +48,10 @@ func FindBinaryAttestation(binaryPath string) (string, error) {
 	// in the same directory as the binary.
 	dir := filepath.Dir(binaryPath)
 	base := filepath.Base(binaryPath)
-	attestPath := filepath.Join(dir, base+AttestationFileSuffix)
+	attestPath, joinErr := deployer.SafeJoin(dir, base+AttestationFileSuffix)
+	if joinErr != nil {
+		return "", errors.Wrap(errors.ErrCodeInvalidRequest, "unsafe attestation path", joinErr)
+	}
 
 	if _, err := os.Stat(attestPath); err != nil {
 		if os.IsNotExist(err) {

pkg/bundler/bundler.go

@@ -731,7 +731,10 @@ func (b *DefaultBundler) copyDataFiles(dir string) ([]string, error) {
 	}
 
 	// Copy the entire external directory into bundle/data/ using os.CopyFS
-	dataDir := filepath.Join(dir, "data")
+	dataDir, joinErr := deployer.SafeJoin(dir, "data")
+	if joinErr != nil {
+		return nil, errors.Wrap(errors.ErrCodeInternal, "unsafe data directory path", joinErr)
+	}
 	externalFS := os.DirFS(layered.ExternalDir())
 	if err := os.CopyFS(dataDir, externalFS); err != nil {
 		return nil, errors.Wrap(errors.ErrCodeInternal, "failed to copy external data files", err)
@@ -740,6 +743,9 @@ func (b *DefaultBundler) copyDataFiles(dir string) ([]string, error) {
 	// Build the list of copied files (relative to bundle dir)
 	copiedFiles := make([]string, 0, len(externalFiles))
 	for _, relPath := range externalFiles {
+		if _, pathErr := deployer.SafeJoin(dataDir, relPath); pathErr != nil {
+			return nil, errors.Wrap(errors.ErrCodeInvalidRequest, "unsafe external data path", pathErr)
+		}
 		copiedFiles = append(copiedFiles, filepath.Join("data", relPath))
 	}
 
@@ -757,7 +763,11 @@ func (b *DefaultBundler) attestBundle(ctx context.Context, dir string, dataFiles
 	}
 
 	// Read checksums.txt and compute its digest
-	digest, err := attestation.ComputeFileDigest(filepath.Join(dir, checksum.ChecksumFileName))
+	checksumPath, joinErr := deployer.SafeJoin(dir, checksum.ChecksumFileName)
+	if joinErr != nil {
+		return nil, errors.Wrap(errors.ErrCodeInternal, "unsafe checksum path", joinErr)
+	}
+	digest, err := attestation.ComputeFileDigest(checksumPath)
 	if err != nil {
 		// If checksums don't exist (IncludeChecksums=false), attestation is not possible
 		slog.Debug("attestation not possible: checksums not available", "error", err)
@@ -807,7 +817,10 @@ func (b *DefaultBundler) attestBundle(ctx context.Context, dir string, dataFiles
 
 	// Add data files as resolved dependencies
 	for _, dataFile := range dataFiles {
-		dataPath := fmt.Sprintf("%s/%s", dir, dataFile)
+		dataPath, pathErr := deployer.SafeJoin(dir, dataFile)
+		if pathErr != nil {
+			return nil, errors.Wrap(errors.ErrCodeInvalidRequest, "unsafe data file path in attestation", pathErr)
+		}
 		dataDigest, digestErr := attestation.ComputeFileDigest(dataPath)
 		if digestErr == nil {
 			subject.ResolvedDependencies = append(subject.ResolvedDependencies, attestation.Dependency{
@@ -831,30 +844,47 @@ func (b *DefaultBundler) attestBundle(ctx context.Context, dir string, dataFiles
 	var attestFiles []string
 
 	// Create attestation subdirectory
-	attestDir := filepath.Join(dir, attestation.AttestationDir)
-	if mkdirErr := os.MkdirAll(attestDir, 0755); mkdirErr != nil { //nolint:gosec // dir is filepath.Clean'd on entry; attestDir uses constant subpath
+	attestDir, joinErr := deployer.SafeJoin(dir, attestation.AttestationDir)
+	if joinErr != nil {
+		return nil, errors.Wrap(errors.ErrCodeInternal, "unsafe attestation directory path", joinErr)
+	}
+	if mkdirErr := os.MkdirAll(attestDir, 0755); mkdirErr != nil { //nolint:gosec // attestDir validated by SafeJoin
 		return nil, errors.Wrap(errors.ErrCodeInternal, "failed to create attestation directory", mkdirErr)
 	}
 
 	// Write bundle attestation
-	bundleAttestPath := filepath.Join(dir, attestation.BundleAttestationFile)
-	if writeErr := os.WriteFile(bundleAttestPath, bundleJSON, 0600); writeErr != nil { //nolint:gosec // path built from filepath.Clean'd dir + constant filename
+	bundleAttestPath, joinErr := deployer.SafeJoin(dir, attestation.BundleAttestationFile)
+	if joinErr != nil {
+		return nil, errors.Wrap(errors.ErrCodeInternal, "unsafe bundle attestation path", joinErr)
+	}
+	if writeErr := os.WriteFile(bundleAttestPath, bundleJSON, 0600); writeErr != nil { //nolint:gosec // path validated by SafeJoin
 		return nil, errors.Wrap(errors.ErrCodeInternal, "failed to write bundle attestation", writeErr)
 	}
 	attestFiles = append(attestFiles, attestation.BundleAttestationFile)
 	slog.Info("bundle attestation written", "path", bundleAttestPath)
 
 	// Copy binary attestation into bundle — errors are fatal since the user
 	// opted into attestation (remove --attest to skip).
+	if err := b.verifyAndCopyBinaryAttestation(ctx, dir); err != nil {
+		return nil, err
+	}
+	attestFiles = append(attestFiles, attestation.BinaryAttestationFile)
+
+	return attestFiles, nil
+}
+
+// verifyAndCopyBinaryAttestation resolves the running binary's attestation,
+// cryptographically verifies it (REQ-6), and copies it into the bundle directory.
+func (b *DefaultBundler) verifyAndCopyBinaryAttestation(ctx context.Context, dir string) error {
 	binaryPath, execErr := os.Executable()
 	if execErr != nil {
-		return nil, errors.Wrap(errors.ErrCodeInternal,
+		return errors.Wrap(errors.ErrCodeInternal,
 			"could not resolve executable path; remove --attest to skip", execErr)
 	}
 
 	binaryAttestPath, findErr := attestation.FindBinaryAttestation(binaryPath)
 	if findErr != nil {
-		return nil, errors.Wrap(errors.ErrCodeNotFound,
+		return errors.Wrap(errors.ErrCodeNotFound,
 			"binary attestation not found; reinstall from a release archive or remove --attest to skip", findErr)
 	}
 
@@ -863,15 +893,15 @@ func (b *DefaultBundler) attestBundle(ctx context.Context, dir string, dataFiles
 	// attestation binds to this specific binary's content.
 	binaryDigest, digestErr := checksum.SHA256Raw(binaryPath)
 	if digestErr != nil {
-		return nil, errors.Wrap(errors.ErrCodeInternal,
+		return errors.Wrap(errors.ErrCodeInternal,
 			"failed to compute binary digest for provenance verification", digestErr)
 	}
 
 	identityPattern := verifier.TrustedRepositoryPattern
 	if b.Config.CertificateIdentityRegexp() != "" {
 		identityPattern = b.Config.CertificateIdentityRegexp()
 		if err := verifier.ValidateIdentityPattern(identityPattern); err != nil {
-			return nil, err
+			return err
 		}
 		slog.Warn("using custom certificate identity pattern for binary attestation — "+
 			"bundle will not pass verification with default settings",
@@ -880,27 +910,29 @@ func (b *DefaultBundler) attestBundle(ctx context.Context, dir string, dataFiles
 
 	binaryBuilder, verifyErr := verifier.VerifyBinaryAttestation(ctx, binaryAttestPath, identityPattern, binaryDigest)
 	if verifyErr != nil {
-		return nil, errors.Wrap(errors.ErrCodeUnauthorized,
+		return errors.Wrap(errors.ErrCodeUnauthorized,
 			"binary attestation verification failed; only NVIDIA-built binaries can attest bundles — "+
 				"remove --attest to skip", verifyErr)
 	}
 	slog.Info("binary provenance verified", "builder", binaryBuilder)
 
 	binaryAttestData, readErr := os.ReadFile(binaryAttestPath)
 	if readErr != nil {
-		return nil, errors.Wrap(errors.ErrCodeInternal,
+		return errors.Wrap(errors.ErrCodeInternal,
 			"binary attestation exists but cannot be read: "+binaryAttestPath, readErr)
 	}
 
-	destPath := filepath.Join(dir, attestation.BinaryAttestationFile)
-	if copyErr := os.WriteFile(destPath, binaryAttestData, 0600); copyErr != nil { //nolint:gosec // path built from filepath.Clean'd dir + constant filename
-		return nil, errors.Wrap(errors.ErrCodeInternal,
+	destPath, joinErr := deployer.SafeJoin(dir, attestation.BinaryAttestationFile)
+	if joinErr != nil {
+		return errors.Wrap(errors.ErrCodeInternal, "unsafe binary attestation path", joinErr)
+	}
+	if copyErr := os.WriteFile(destPath, binaryAttestData, 0600); copyErr != nil { //nolint:gosec // path validated by SafeJoin
+		return errors.Wrap(errors.ErrCodeInternal,
 			"failed to copy binary attestation into bundle", copyErr)
 	}
-	attestFiles = append(attestFiles, attestation.BinaryAttestationFile)
 	slog.Info("binary attestation copied into bundle", "path", destPath)
 
-	return attestFiles, nil
+	return nil
 }
 
 // writeRecipeFile serializes the recipe to the bundle directory.
@@ -910,7 +942,10 @@ func (b *DefaultBundler) writeRecipeFile(recipeResult *recipe.RecipeResult, dir
 		return 0, errors.Wrap(errors.ErrCodeInternal, "failed to serialize recipe", err)
 	}
 
-	recipePath := fmt.Sprintf("%s/recipe.yaml", dir)
+	recipePath, joinErr := deployer.SafeJoin(dir, "recipe.yaml")
+	if joinErr != nil {
+		return 0, errors.Wrap(errors.ErrCodeInternal, "unsafe recipe file path", joinErr)
+	}
 	if err := os.WriteFile(recipePath, recipeData, 0600); err != nil {
 		return 0, errors.Wrap(errors.ErrCodeInternal, "failed to write recipe file", err)
 	}

pkg/bundler/checksum/checksum.go

@@ -25,6 +25,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/NVIDIA/aicr/pkg/bundler/deployer"
 	"github.com/NVIDIA/aicr/pkg/errors"
 )
 
@@ -63,7 +64,10 @@ func GenerateChecksums(ctx context.Context, bundleDir string, files []string) er
 		checksums = append(checksums, fmt.Sprintf("%s  %s", hex.EncodeToString(digest), relPath))
 	}
 
-	checksumPath := filepath.Join(bundleDir, ChecksumFileName)
+	checksumPath, joinErr := deployer.SafeJoin(bundleDir, ChecksumFileName)
+	if joinErr != nil {
+		return errors.Wrap(errors.ErrCodeInternal, "unsafe checksum path", joinErr)
+	}
 	content := strings.Join(checksums, "\n") + "\n"
 
 	if err := os.WriteFile(checksumPath, []byte(content), 0600); err != nil {
@@ -80,6 +84,8 @@ func GenerateChecksums(ctx context.Context, bundleDir string, files []string) er
 
 // GetChecksumFilePath returns the full path to the checksums.txt file
 // in the given bundle directory.
+// filepath.Join is safe here: ChecksumFileName is a compile-time constant
+// and the return type (string) has no error channel for SafeJoin.
 func GetChecksumFilePath(bundleDir string) string {
 	return filepath.Join(bundleDir, ChecksumFileName)
 }
@@ -88,7 +94,10 @@ func GetChecksumFilePath(bundleDir string) string {
 // Returns a list of error descriptions for any mismatches or read failures.
 // An empty return means all checksums are valid.
 func VerifyChecksums(bundleDir string) []string {
-	checksumPath := filepath.Join(bundleDir, ChecksumFileName)
+	checksumPath, joinErr := deployer.SafeJoin(bundleDir, ChecksumFileName)
+	if joinErr != nil {
+		return []string{fmt.Sprintf("unsafe checksum path: %v", joinErr)}
+	}
 	data, err := os.ReadFile(checksumPath)
 	if err != nil {
 		return []string{fmt.Sprintf("failed to read %s: %v", ChecksumFileName, err)}
@@ -116,20 +125,8 @@ func VerifyChecksumsFromData(bundleDir string, data []byte) []string {
 
 		expectedDigest := parts[0]
 		relativePath := parts[1]
-		filePath := filepath.Join(bundleDir, relativePath)
-
-		// Prevent path traversal — resolved path must stay within bundleDir
-		absBundle, absErr := filepath.Abs(bundleDir)
-		if absErr != nil {
-			errs = append(errs, fmt.Sprintf("failed to resolve bundle directory: %v", absErr))
-			continue
-		}
-		absFile, absErr := filepath.Abs(filePath)
-		if absErr != nil {
-			errs = append(errs, fmt.Sprintf("failed to resolve path %s: %v", relativePath, absErr))
-			continue
-		}
-		if !strings.HasPrefix(absFile, absBundle+string(filepath.Separator)) {
+		filePath, joinErr := deployer.SafeJoin(bundleDir, relativePath)
+		if joinErr != nil {
 			errs = append(errs, fmt.Sprintf("path traversal detected in checksum entry: %s", relativePath))
 			continue
 		}
@@ -150,6 +147,8 @@ func VerifyChecksumsFromData(bundleDir string, data []byte) []string {
 }
 
 // CountEntries returns the number of entries in a checksums.txt file.
+// filepath.Join is safe here: ChecksumFileName is a compile-time constant
+// and the return type (int) has no error channel for SafeJoin.
 func CountEntries(bundleDir string) int {
 	checksumPath := filepath.Join(bundleDir, ChecksumFileName)
 	data, err := os.ReadFile(checksumPath)

pkg/bundler/deployer/helm/helm.go

@@ -158,7 +158,10 @@ func (g *Generator) Generate(ctx context.Context, outputDir string) (*deployer.O
 
 	// Include external data files in the file list (for checksums)
 	for _, dataFile := range g.DataFiles {
-		absPath := filepath.Join(outputDir, dataFile)
+		absPath, joinErr := deployer.SafeJoin(outputDir, dataFile)
+		if joinErr != nil {
+			return nil, errors.Wrap(errors.ErrCodeInvalidRequest, "unsafe data file path", joinErr)
+		}
 		output.Files = append(output.Files, absPath)
 	}
 

pkg/bundler/deployer/helm/helm_test.go

@@ -2274,3 +2274,68 @@ func TestGenerate_DoesNotMutateComponentValues(t *testing.T) {
 		t.Error("original driver.version was mutated (removed) — deep copy is missing")
 	}
 }
+
+func TestGenerate_DataFiles(t *testing.T) {
+	t.Run("valid data file included in output", func(t *testing.T) {
+		ctx := context.Background()
+		outputDir := t.TempDir()
+
+		// Create a data file on disk so checksums can read it
+		dataDir := filepath.Join(outputDir, "data")
+		if err := os.MkdirAll(dataDir, 0755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(filepath.Join(dataDir, "overrides.yaml"), []byte("key: value"), 0600); err != nil {
+			t.Fatal(err)
+		}
+
+		g := &Generator{
+			RecipeResult: createTestRecipeResult(),
+			ComponentValues: map[string]map[string]any{
+				"cert-manager": {},
+				"gpu-operator": {},
+			},
+			Version:   "v1.0.0",
+			DataFiles: []string{"data/overrides.yaml"},
+		}
+
+		output, err := g.Generate(ctx, outputDir)
+		if err != nil {
+			t.Fatalf("Generate() error = %v", err)
+		}
+
+		found := false
+		for _, f := range output.Files {
+			if strings.HasSuffix(f, "data/overrides.yaml") {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Error("data file not included in output.Files")
+		}
+	})
+
+	t.Run("path traversal rejected", func(t *testing.T) {
+		ctx := context.Background()
+		outputDir := t.TempDir()
+
+		g := &Generator{
+			RecipeResult: createTestRecipeResult(),
+			ComponentValues: map[string]map[string]any{
+				"cert-manager": {},
+				"gpu-operator": {},
+			},
+			Version:   "v1.0.0",
+			DataFiles: []string{"../../../etc/passwd"},
+		}
+
+		_, err := g.Generate(ctx, outputDir)
+		if err == nil {
+			t.Fatal("Generate() should reject path traversal in DataFiles")
+		}
+		if !strings.Contains(err.Error(), "unsafe data file path") {
+			t.Errorf("expected 'unsafe data file path' error, got: %v", err)
+		}
+	})
+}