fix: roll back InsertImage transaction when related clears fail

pkg/infrastructure/database/repository.go

@@ -53,11 +53,14 @@ func (r *Repository) ImageExists(name string) (bool, error) {
 }
 
 // InsertImage inserts an image and all its related data in a transaction.
-func (r *Repository) InsertImage(img *domain.ImageRecord) error {
+// err is a named result so the deferred rollback always sees failures from
+// every return path (including short-declaration shadows in nested scopes).
+func (r *Repository) InsertImage(img *domain.ImageRecord) (err error) {
 	tx, err := r.db.Begin()
 	if err != nil {
 		return fmt.Errorf("beginning transaction: %w", err)
 	}
+	// Defer is registered only after Begin succeeds, so tx is always non-nil here.
 	defer func() {
 		if err != nil {
 			if rbErr := tx.Rollback(); rbErr != nil {
@@ -118,10 +121,11 @@ func (r *Repository) InsertImage(img *domain.ImageRecord) error {
 		return fmt.Errorf("getting image id: %w", err)
 	}
 
-	// Clear old related data on upsert
+	// Clear old related data on upsert.
+	// Assign to the named err (not :=) so deferred rollback observes failures.
 	relTables := []string{"languages", "vulnerabilities", "package_managers", "capabilities", "system_packages", "security_findings"}
 	for _, t := range relTables {
-		if _, err := tx.Exec(fmt.Sprintf("DELETE FROM %s WHERE image_id = ?", t), imageID); err != nil {
+		if _, err = tx.Exec(fmt.Sprintf("DELETE FROM %s WHERE image_id = ?", t), imageID); err != nil {
 			return fmt.Errorf("clearing %s: %w", t, err)
 		}
 	}
@@ -183,8 +187,7 @@ func (r *Repository) InsertImage(img *domain.ImageRecord) error {
 		}
 	}
 
-	err = tx.Commit()
-	if err != nil {
+	if err = tx.Commit(); err != nil {
 		return fmt.Errorf("committing transaction: %w", err)
 	}
 

pkg/infrastructure/database/repository_test.go

@@ -25,6 +25,7 @@ package database
 import (
 	"database/sql"
 	"fmt"
+	"path/filepath"
 	"testing"
 
 	"github.com/microsoft/sbi/pkg/domain"
@@ -395,3 +396,60 @@ func TestQueryLanguages_BaseSortsLast(t *testing.T) {
 	assert.Equal(t, "python", languages[1])
 	assert.Equal(t, "base", languages[2])
 }
+
+// TestInsertImage_RollbackOnClearFailure ensures a failure while clearing
+// related tables rolls back the whole upsert and releases the connection.
+//
+// This guards against the classic Go bug where `if _, err := tx.Exec(...)`
+// shadows the outer err used by deferred rollback, leaving the transaction
+// open (and, with MaxOpenConns(1), blocking all further DB use).
+func TestInsertImage_RollbackOnClearFailure(t *testing.T) {
+	dbPath := filepath.Join(t.TempDir(), "rollback.db")
+	db, err := sql.Open("sqlite", dbPath)
+	require.NoError(t, err)
+	defer func() { _ = db.Close() }()
+
+	// Match production OpenDB: a single connection so a leaked tx is observable.
+	db.SetMaxOpenConns(1)
+	_, err = db.Exec("PRAGMA foreign_keys=ON")
+	require.NoError(t, err)
+	require.NoError(t, CreateTables(db))
+
+	repo := NewRepository(db)
+	img := &domain.ImageRecord{
+		Name:                 "test:1.0",
+		Registry:             "r",
+		Repository:           "repo",
+		Tag:                  "1.0",
+		TotalVulnerabilities: 5,
+		Languages: []domain.Language{
+			{Language: "python", Version: "3.12"},
+		},
+	}
+	require.NoError(t, repo.InsertImage(img))
+
+	// Force the related-table clear path to fail on the next upsert.
+	_, err = db.Exec("DROP TABLE languages")
+	require.NoError(t, err)
+
+	img.TotalVulnerabilities = 99
+	err = repo.InsertImage(img)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "clearing languages")
+
+	// Upsert must not have been committed.
+	var total int
+	err = db.QueryRow(`SELECT total_vulnerabilities FROM images WHERE name = ?`, img.Name).Scan(&total)
+	require.NoError(t, err)
+	assert.Equal(t, 5, total, "failed clear must roll back the image upsert")
+
+	// Connection must be released so further writes can proceed.
+	require.NoError(t, CreateTables(db)) // recreates languages (IF NOT EXISTS)
+	img.TotalVulnerabilities = 7
+	img.Languages = []domain.Language{{Language: "python", Version: "3.12.1"}}
+	require.NoError(t, repo.InsertImage(img))
+
+	err = db.QueryRow(`SELECT total_vulnerabilities FROM images WHERE name = ?`, img.Name).Scan(&total)
+	require.NoError(t, err)
+	assert.Equal(t, 7, total)
+}