fix: resolve path traversal errors and isolate integration tests

- Canonicalize paths in tasks using filepath.Abs and Clean to fix "outside target path" errors

- Refactor checkFilePreconditions to ensure robust security jail validation

- Update CreateDirTask and ExecCommandTask to use absolute paths for transaction tracking

- Isolate integration tests by injecting Git identity via environment variables (GIT_AUTHOR_ID)

- Prevent global Git config leakage and ensure hermetic test execution

- Decompose TestNewCommand into subtests to verify modular language and template optionality

- Reduce cyclomatic complexity in test suites by extracting state verification helpers

Author: Emin-ACIKGOZ

pkg/tasks/create_directory.go

@@ -7,6 +7,7 @@ package tasks
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"scbake/internal/types"
 	"scbake/internal/util/fileutil"
 )
@@ -20,17 +21,23 @@ type CreateDirTask struct {
 
 // Execute performs the task of creating the directory.
 func (t *CreateDirTask) Execute(tc types.TaskContext) error {
+	// Canonicalize the path to ensure consistency across relative/absolute calls.
+	absPath, err := filepath.Abs(t.Path)
+	if err != nil {
+		return fmt.Errorf("failed to resolve directory path %s: %w", t.Path, err)
+	}
+
 	// Safety Tracking: If a transaction manager is present, register this path.
 	// If the directory doesn't exist, it will be marked for cleanup on rollback.
 	// If it does exist, this is a no-op (idempotent).
 	if !tc.DryRun && tc.Tx != nil {
-		if err := tc.Tx.Track(t.Path); err != nil {
+		if err := tc.Tx.Track(absPath); err != nil {
 			return fmt.Errorf("failed to track directory %s: %w", t.Path, err)
 		}
 	}
 
-	// Use the constant from the centralized location
-	if err := os.MkdirAll(t.Path, fileutil.DirPerms); err != nil {
+	// Use the constant from the centralized location for secure permissions.
+	if err := os.MkdirAll(absPath, fileutil.DirPerms); err != nil {
 		return fmt.Errorf("failed to create directory %s: %w", t.Path, err)
 	}
 	return nil

pkg/tasks/create_directory_test.go

@@ -53,6 +53,7 @@ func TestCreateDirTask_Transaction(t *testing.T) {
 	tx, _ := transaction.New(rootDir)
 
 	targetDir := filepath.Join(rootDir, "tracked_dir")
+	absTargetDir, _ := filepath.Abs(targetDir)
 
 	task := &CreateDirTask{
 		Path:     targetDir,
@@ -75,7 +76,7 @@ func TestCreateDirTask_Transaction(t *testing.T) {
 		t.Fatalf("Rollback failed: %v", err)
 	}
 
-	if _, err := os.Stat(targetDir); !os.IsNotExist(err) {
-		t.Error("Directory was not removed by rollback, likely wasn't tracked")
+	if _, err := os.Stat(absTargetDir); !os.IsNotExist(err) {
+		t.Error("Directory was not removed by rollback, likely wasn't tracked via absolute path")
 	}
 }

pkg/tasks/create_template.go

@@ -47,22 +47,37 @@ func (t *CreateTemplateTask) Priority() int {
 
 // checkFilePreconditions handles path safety, directory creation, and existence checks.
 func checkFilePreconditions(finalPath, output, target string, force bool) error {
-	// 1. Path Safety Check
-	if !strings.HasPrefix(finalPath, target) {
-		return fmt.Errorf("output path '%s' is outside the target path '%s'", output, target)
+	// 1. Path Safety Check (Canonicalization)
+	// We resolve both target and finalPath to absolute cleaned paths to ensure
+	// that indicators like "." and relative subdirectories match correctly.
+	absTarget, err := filepath.Abs(target)
+	if err != nil {
+		return fmt.Errorf("failed to resolve target path: %w", err)
+	}
+	absFinal, err := filepath.Abs(finalPath)
+	if err != nil {
+		return fmt.Errorf("failed to resolve output path: %w", err)
+	}
+
+	cleanTarget := filepath.Clean(absTarget)
+	cleanFinalPath := filepath.Clean(absFinal)
+
+	if !strings.HasPrefix(cleanFinalPath, cleanTarget) {
+		return fmt.Errorf("task failed (%s): output path '%s' is outside the target path '%s'",
+			filepath.Base(output), output, target)
 	}
 
 	// 2. Ensure the directory exists
-	dir := filepath.Dir(finalPath)
+	dir := filepath.Dir(cleanFinalPath)
 	if err := os.MkdirAll(dir, fileutil.DirPerms); err != nil {
 		return fmt.Errorf("failed to create directory %s: %w", dir, err)
 	}
 
 	// 3. Existence Check
 	if !force {
-		if _, err := os.Stat(finalPath); err == nil {
+		if _, err := os.Stat(cleanFinalPath); err == nil {
 			// File exists and Force is false.
-			return fmt.Errorf("file already exists: %s. Use --force to overwrite", finalPath)
+			return fmt.Errorf("file already exists: %s. Use --force to overwrite", output)
 		} else if !errors.Is(err, os.ErrNotExist) {
 			// Some other error occurred (e.g., permissions).
 			return err
@@ -89,27 +104,30 @@ func (t *CreateTemplateTask) Execute(tc types.TaskContext) (err error) {
 	}
 
 	// 2. Determine and check the final output path
-	finalPath := filepath.Join(tc.TargetPath, filepath.Clean(t.OutputPath))
+	finalPath := filepath.Join(tc.TargetPath, t.OutputPath)
 
 	// Check directory and file existence using the helper function.
 	if err = checkFilePreconditions(finalPath, t.OutputPath, tc.TargetPath, tc.Force); err != nil {
 		return err
 	}
 
+	// Canonical path for tracking and writing
+	absPath, _ := filepath.Abs(finalPath)
+
 	// 3. Safety Tracking: Register the file with the transaction manager.
 	// If it exists, it will be backed up. If not, it will be marked for cleanup.
 	if tc.Tx != nil {
-		if err := tc.Tx.Track(finalPath); err != nil {
-			return fmt.Errorf("failed to track file %s: %w", finalPath, err)
+		if err := tc.Tx.Track(absPath); err != nil {
+			return fmt.Errorf("failed to track file %s: %w", t.OutputPath, err)
 		}
 	}
 
 	// 4. Create the output file
 	// G304: Path is explicitly sanitized and verified in checkFilePreconditions
 	//nolint:gosec
-	f, err := os.Create(finalPath)
+	f, err := os.OpenFile(absPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, fileutil.FilePerms)
 	if err != nil {
-		return fmt.Errorf("failed to create file %s: %w", finalPath, err)
+		return fmt.Errorf("failed to create file %s: %w", t.OutputPath, err)
 	}
 
 	// Check the return value of f.Close()

pkg/tasks/create_template_test.go

@@ -35,7 +35,7 @@ func TestCreateTemplateTask(t *testing.T) {
 		Force:      false,
 	}
 
-	// Case 1: Render a valid template
+	// Case 1: Render a valid template (Standard relative path)
 	task := &CreateTemplateTask{
 		TemplateFS:   testTemplates,
 		TemplatePath: "testdata/simple.tpl",
@@ -62,10 +62,10 @@ func TestCreateTemplateTask(t *testing.T) {
 
 	// Case 2: Existence Check (Should fail without Force)
 	if err := task.Execute(tc); err == nil {
-		t.Error("Overwriting existing file should fail without Force, but it succeeded")
+		t.Error("Overwriting existing file should fail without Force")
 	}
 
-	// Case 3: Force Overwrite (Should succeed)
+	// Case 3: Force Overwrite
 	tc.Force = true
 	if err := task.Execute(tc); err != nil {
 		t.Errorf("Force overwrite failed: %v", err)
@@ -92,7 +92,7 @@ func TestCreateTemplateTask_Transaction(t *testing.T) {
 	rootDir := t.TempDir()
 	tx, _ := transaction.New(rootDir)
 
-	// FIX: Provide a valid manifest so the template can render {{ (index .Projects 0).Name }}
+	// Provide a valid manifest so the template can render {{ (index .Projects 0).Name }}
 	manifest := &types.Manifest{
 		SbakeVersion: "v1.0.0",
 		Projects: []types.Project{
@@ -122,7 +122,9 @@ func TestCreateTemplateTask_Transaction(t *testing.T) {
 
 	// File should exist
 	path := filepath.Join(rootDir, "tracked_file.txt")
-	if _, err := os.Stat(path); os.IsNotExist(err) {
+	absPath, _ := filepath.Abs(path)
+
+	if _, err := os.Stat(absPath); os.IsNotExist(err) {
 		t.Fatal("File was not created")
 	}