From 11717c94c6a41d0530d5d3e6a551deaef3321893 Mon Sep 17 00:00:00 2001
From: Gates Wang <9372086+SetagGnaw@users.noreply.github.com>
Date: Fri, 24 Jul 2026 23:42:22 -0400
Subject: [PATCH] fix(storage/sql): don't leak or silently commit transactions

The SQL storage driver mishandled its explicit transactions in Create
and Delete:

- Delete's not-found and build-select-fail early returns returned
  without Rollback or Commit, permanently leaking the pooled connection
  and an open server-side transaction.
- Both methods finished with `defer transaction.Commit()`, discarding
  the commit error. A commit-time failure (connection drop,
  serialization failure, disk full) left the release unpersisted while
  the method reported success. In Delete the deferred commit also fired
  on the post-delete error paths, committing a partial delete.

Register a single `defer transaction.Rollback()` right after Beginx (a
no-op once committed) so every early return releases the transaction,
and commit explicitly at the success point, returning any commit error.
Error paths after the delete now roll back, making Delete atomic.

Covered by the regression tests added in the previous commit.

Also convert the driver's direct Exec calls to ExecContext with
context.Background(), matching the secrets and cfgmaps drivers. The
new tests import database/sql, which activates noctx's database/sql
checks for the package and flags these pre-existing call sites.

Signed-off-by: Gates Wang <9372086+SetagGnaw@users.noreply.github.com>
---
 pkg/storage/driver/sql.go | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/pkg/storage/driver/sql.go b/pkg/storage/driver/sql.go
index 85e6cbd3f..dc1a81cc7 100644
--- a/pkg/storage/driver/sql.go
+++ b/pkg/storage/driver/sql.go
@@ -17,6 +17,7 @@ limitations under the License.
 package driver
 
 import (
+	"context"
 	"fmt"
 	"log/slog"
 	"maps"
@@ -486,6 +487,7 @@ func (s *SQL) Create(key string, rel release.Releaser) error {
 		s.Logger().Debug("failed to start SQL transaction", slog.Any("error", err))
 		return fmt.Errorf("error beginning transaction: %w", err)
 	}
+	defer transaction.Rollback()
 
 	insertQuery, args, err := s.statementBuilder.
 		Insert(sqlReleaseTableName).
@@ -516,9 +518,7 @@ func (s *SQL) Create(key string, rel release.Releaser) error {
 		return err
 	}
 
-	if _, err := transaction.Exec(insertQuery, args...); err != nil {
-		defer transaction.Rollback()
-
+	if _, err := transaction.ExecContext(context.Background(), insertQuery, args...); err != nil {
 		selectQuery, args, buildErr := s.statementBuilder.
 			Select(sqlReleaseTableKeyColumn).
 			From(sqlReleaseTableName).
@@ -558,18 +558,20 @@ func (s *SQL) Create(key string, rel release.Releaser) error {
 			).ToSql()
 
 		if err != nil {
-			defer transaction.Rollback()
 			s.Logger().Debug("failed to build insert query", slog.Any("error", err))
 			return err
 		}
 
-		if _, err := transaction.Exec(insertLabelsQuery, args...); err != nil {
-			defer transaction.Rollback()
+		if _, err := transaction.ExecContext(context.Background(), insertLabelsQuery, args...); err != nil {
 			s.Logger().Debug("failed to write Labels", slog.Any("error", err))
 			return err
 		}
 	}
-	defer transaction.Commit()
+
+	if err := transaction.Commit(); err != nil {
+		s.Logger().Debug("failed to commit release creation transaction", slog.Any("error", err))
+		return fmt.Errorf("error committing transaction: %w", err)
+	}
 
 	return nil
 }
@@ -609,7 +611,7 @@ func (s *SQL) Update(key string, rel release.Releaser) error {
 		return err
 	}
 
-	if _, err := s.db.Exec(query, args...); err != nil {
+	if _, err := s.db.ExecContext(context.Background(), query, args...); err != nil {
 		s.Logger().Debug("failed to update release in SQL database", slog.String("key", key), slog.Any("error", err))
 		return err
 	}
@@ -624,6 +626,7 @@ func (s *SQL) Delete(key string) (release.Releaser, error) {
 		s.Logger().Debug("failed to start SQL transaction", slog.Any("error", err))
 		return nil, fmt.Errorf("error beginning transaction: %w", err)
 	}
+	defer transaction.Rollback()
 
 	selectQuery, args, err := s.statementBuilder.
 		Select(sqlReleaseTableBodyColumn).
@@ -646,10 +649,8 @@ func (s *SQL) Delete(key string) (release.Releaser, error) {
 	release, err := decodeRelease(record.Body)
 	if err != nil {
 		s.Logger().Debug("failed to decode release", slog.String("key", key), slog.Any("error", err))
-		transaction.Rollback()
 		return nil, err
 	}
-	defer transaction.Commit()
 
 	deleteQuery, args, err := s.statementBuilder.
 		Delete(sqlReleaseTableName).
@@ -661,7 +662,7 @@ func (s *SQL) Delete(key string) (release.Releaser, error) {
 		return nil, err
 	}
 
-	_, err = transaction.Exec(deleteQuery, args...)
+	_, err = transaction.ExecContext(context.Background(), deleteQuery, args...)
 	if err != nil {
 		s.Logger().Debug("failed perform delete query", slog.Any("error", err))
 		return release, err
@@ -686,8 +687,17 @@ func (s *SQL) Delete(key string) (release.Releaser, error) {
 		s.Logger().Debug("failed to build delete Labels query", slog.Any("error", err))
 		return nil, err
 	}
-	_, err = transaction.Exec(deleteCustomLabelsQuery, args...)
-	return release, err
+	if _, err = transaction.ExecContext(context.Background(), deleteCustomLabelsQuery, args...); err != nil {
+		s.Logger().Debug("failed to delete release custom labels", slog.String("key", key), slog.Any("error", err))
+		return release, err
+	}
+
+	if err := transaction.Commit(); err != nil {
+		s.Logger().Debug("failed to commit release deletion transaction", slog.Any("error", err))
+		return release, fmt.Errorf("error committing transaction: %w", err)
+	}
+
+	return release, nil
 }
 
 // Get release custom labels from database