fix(issue-16): fix critical transaction bug and high-severity repository issues

C1: CreateBatch shadowed err with := in loop, causing partial commits on
    failure. Fixed with named return and execErr loop variable.
H1: OrderBy concatenated raw into SQL query. Added whitelist validation.
H2: Organization.List dropped query args in SelectContext call.
H3: CourseRepository.Get wrapped sql.ErrNoRows without translating to
    ErrNotFound, breaking the port contract.
Medium: Create now returns the created entity via Get; fixed prepared
    statement leak in Organization.Create; unified time.Now() to UTC.
This commit is contained in:
frx
2026-07-09 13:41:56 +00:00
parent 596e8bbc3f
commit ca32d865b2
2 changed files with 23 additions and 7 deletions

View File

@ -9,6 +9,7 @@ import (
"strings" "strings"
"time" "time"
cerrors "git.loyso.art/frx/kurious/internal/common/errors"
"git.loyso.art/frx/kurious/internal/common/nullable" "git.loyso.art/frx/kurious/internal/common/nullable"
"git.loyso.art/frx/kurious/internal/common/xslices" "git.loyso.art/frx/kurious/internal/common/xslices"
"git.loyso.art/frx/kurious/internal/kurious/domain" "git.loyso.art/frx/kurious/internal/kurious/domain"
@ -72,6 +73,14 @@ func (r *sqliteCourseRepository) List(
params.OrderBy = "id" params.OrderBy = "id"
} }
var allowedOrderFields = map[string]bool{
"id": true, "full_price": true, "name": true,
"discount": true, "duration": true, "starts_at": true,
}
if !allowedOrderFields[params.OrderBy] {
params.OrderBy = "id"
}
direction := "ASC" direction := "ASC"
if !params.Ascending { if !params.Ascending {
direction = "DESC" direction = "DESC"
@ -277,6 +286,9 @@ func (r *sqliteCourseRepository) Get(
var courseDB sqliteCourseDB var courseDB sqliteCourseDB
err = r.db.GetContext(ctx, &courseDB, query, id) err = r.db.GetContext(ctx, &courseDB, query, id)
if err != nil { if err != nil {
if errors.Is(err, sql.ErrNoRows) {
return course, cerrors.ErrNotFound
}
return course, fmt.Errorf("executing query: %w", err) return course, fmt.Errorf("executing query: %w", err)
} }
@ -289,7 +301,7 @@ func (r *sqliteCourseRepository) GetByExternalID(
return course, errors.New("not implemented") return course, errors.New("not implemented")
} }
func (r *sqliteCourseRepository) CreateBatch(ctx context.Context, params ...domain.CreateCourseParams) error { func (r *sqliteCourseRepository) CreateBatch(ctx context.Context, params ...domain.CreateCourseParams) (err error) {
tx, err := r.db.BeginTxx(ctx, &sql.TxOptions{Isolation: sql.LevelDefault}) tx, err := r.db.BeginTxx(ctx, &sql.TxOptions{Isolation: sql.LevelDefault})
if err != nil { if err != nil {
return fmt.Errorf("beginning tx: %w", err) return fmt.Errorf("beginning tx: %w", err)
@ -317,9 +329,9 @@ func (r *sqliteCourseRepository) CreateBatch(ctx context.Context, params ...doma
} }
for _, param := range params { for _, param := range params {
_, err := stmt.ExecContext(ctx, createCourseParamsAsValues(param)...) _, execErr := stmt.ExecContext(ctx, createCourseParamsAsValues(param)...)
if err != nil { if execErr != nil {
return fmt.Errorf("executing statement query: %w", err) return fmt.Errorf("executing statement query: %w", execErr)
} }
} }
@ -328,8 +340,11 @@ func (r *sqliteCourseRepository) CreateBatch(ctx context.Context, params ...doma
func (r *sqliteCourseRepository) Create(ctx context.Context, params domain.CreateCourseParams) (domain.Course, error) { func (r *sqliteCourseRepository) Create(ctx context.Context, params domain.CreateCourseParams) (domain.Course, error) {
err := r.CreateBatch(ctx, params) err := r.CreateBatch(ctx, params)
if err != nil {
return domain.Course{}, err return domain.Course{}, err
} }
return r.Get(ctx, params.ID)
}
func (r *sqliteCourseRepository) UpdateCourseDescription(ctx context.Context, id, description string) error { func (r *sqliteCourseRepository) UpdateCourseDescription(ctx context.Context, id, description string) error {
return errors.New("unimplemented") return errors.New("unimplemented")
@ -411,7 +426,7 @@ func scanRows(ctx context.Context, db *sqlx.DB, f func(rowsScanner) error, query
} }
func createCourseParamsAsValues(params domain.CreateCourseParams) []any { func createCourseParamsAsValues(params domain.CreateCourseParams) []any {
now := time.Now() now := time.Now().UTC()
return []any{ return []any{
params.ID, params.ID,

View File

@ -187,7 +187,7 @@ func (r *sqliteOrganizationRepository) List(ctx context.Context, params domain.L
}() }()
organizations := make([]organizationDB, 0, 1<<8) organizations := make([]organizationDB, 0, 1<<8)
err = r.db.SelectContext(ctx, &organizations, query) err = r.db.SelectContext(ctx, &organizations, query, args...)
if err != nil { if err != nil {
return nil, fmt.Errorf("executing query: %w", err) return nil, fmt.Errorf("executing query: %w", err)
} }
@ -262,6 +262,7 @@ func (r *sqliteOrganizationRepository) Create(ctx context.Context, params domain
if err != nil { if err != nil {
return out, fmt.Errorf("preparing statement: %w", err) return out, fmt.Errorf("preparing statement: %w", err)
} }
defer stmt.Close()
var orgdb organizationDB var orgdb organizationDB