From efb90f31619d905a94035d53747a15d98755d988 Mon Sep 17 00:00:00 2001
From: Suliman Abdulrazzaq <suliman9000a@gmail.com>
Date: Sat, 26 Sep 2026 19:50:15 +0300
Subject: [PATCH] fix: clean up application images when an upload fails

A failed write left a partially written image file behind, and a failed
database update after saving the new image had already deleted the image
the application still referenced.

Write the uploaded image with a helper that removes the file if it can't
be written completely, and only delete the old image once the
application has been updated. If the update fails, remove the new image
instead.
---
 api/application.go      | 47 +++++++++++++++++++++++++++++++++-----
 api/application_test.go | 50 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 6 deletions(-)

diff --git a/api/application.go b/api/application.go
index 612c20c60..b8060fb73 100644
--- a/api/application.go
+++ b/api/application.go
@@ -3,6 +3,8 @@ package api
 import (
 	"errors"
 	"fmt"
+	"io"
+	"mime/multipart"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -438,20 +440,25 @@ func (a *ApplicationAPI) UploadApplicationImage(ctx *gin.Context) {
 				return generateImageName() + ext
 			})
 
-			err = ctx.SaveUploadedFile(file, a.ImageDir+name)
+			err = saveImage(file, a.ImageDir+name)
 			if err != nil {
 				ctx.AbortWithError(500, err)
 				return
 			}
 
-			if app.Image != "" {
-				os.Remove(a.ImageDir + app.Image)
-			}
-
+			oldImage := app.Image
 			app.Image = name
-			if success := successOrAbort(ctx, 500, a.DB.UpdateApplication(app)); !success {
+			if err := a.DB.UpdateApplication(app); err != nil {
+				// The application still references the old image, so keep
+				// it and drop the new one that nothing references.
+				os.Remove(a.ImageDir + name)
+				ctx.AbortWithError(500, err)
 				return
 			}
+
+			if oldImage != "" {
+				os.Remove(a.ImageDir + oldImage)
+			}
 			ctx.JSON(200, withResolvedImage(app))
 		} else {
 			ctx.AbortWithError(404, fmt.Errorf("app with id %d doesn't exists", id))
@@ -533,6 +540,34 @@ func withResolvedImage(app *model.Application) *model.Application {
 	return app
 }
 
+// saveImage writes the uploaded file to dst. If it can't be written
+// completely, the partially written file is removed.
+func saveImage(file *multipart.FileHeader, dst string) error {
+	src, err := file.Open()
+	if err != nil {
+		return err
+	}
+	defer src.Close()
+	return writeImage(dst, src)
+}
+
+func writeImage(dst string, src io.Reader) (err error) {
+	out, err := os.Create(dst)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		if closeErr := out.Close(); err == nil {
+			err = closeErr
+		}
+		if err != nil {
+			os.Remove(dst)
+		}
+	}()
+	_, err = io.Copy(out, src)
+	return err
+}
+
 func exist(path string) bool {
 	if _, err := os.Stat(path); os.IsNotExist(err) {
 		return false
diff --git a/api/application_test.go b/api/application_test.go
index 623ed8b50..dfa8edf52 100644
--- a/api/application_test.go
+++ b/api/application_test.go
@@ -483,6 +483,43 @@ func (s *ApplicationSuite) Test_UploadAppImage_WithImageFile_DeleteExstingImage(
 	assert.Len(s.T(), listing, 1)
 }
 
+func (s *ApplicationSuite) Test_UploadAppImage_UpdateFails_KeepsExistingImage() {
+	existingImageName := "existing.png"
+	s.db.User(5)
+	s.db.CreateApplication(&model.Application{UserID: 5, ID: 1, Image: existingImageName})
+	fakeImage(s.T(), s.imageDir.Path(existingImageName))
+	s.a.DB = &failingUpdateAppDB{ApplicationDatabase: s.db}
+
+	cType, buffer, err := upload(map[string]*os.File{"file": mustOpen("../test/assets/image.png")})
+	require.NoError(s.T(), err)
+	s.ctx.Request = httptest.NewRequest("POST", "/irrelevant", &buffer)
+	s.ctx.Request.Header.Set("Content-Type", cType)
+	test.WithUser(s.ctx, 5)
+	s.ctx.Params = gin.Params{{Key: "id", Value: "1"}}
+
+	s.a.UploadApplicationImage(s.ctx)
+
+	assert.Equal(s.T(), 500, s.recorder.Code)
+	app, err := s.db.GetApplicationByID(1)
+	require.NoError(s.T(), err)
+	assert.Equal(s.T(), existingImageName, app.Image)
+	listing, err := os.ReadDir(s.imageDir.Path())
+	require.NoError(s.T(), err)
+	require.Len(s.T(), listing, 1)
+	assert.Equal(s.T(), existingImageName, listing[0].Name())
+}
+
+func (s *ApplicationSuite) Test_WriteImage_RemovesPartialFileOnError() {
+	dst := s.imageDir.Path("partial.png")
+	src := io.MultiReader(strings.NewReader("partial image data"), errReader{errors.New("connection reset")})
+
+	err := writeImage(dst, src)
+
+	assert.EqualError(s.T(), err, "connection reset")
+	_, err = os.Stat(dst)
+	assert.True(s.T(), os.IsNotExist(err), "the partially written file must be removed")
+}
+
 func (s *ApplicationSuite) Test_UploadAppImage_WithTextFile_expectBadRequest() {
 	s.db.User(5).App(1)
 
@@ -762,3 +799,16 @@ func fakeImage(t *testing.T, path string) {
 	err = os.WriteFile(path, data, 0o644)
 	assert.Nil(t, err)
 }
+
+// failingUpdateAppDB fails every UpdateApplication call.
+type failingUpdateAppDB struct {
+	ApplicationDatabase
+}
+
+func (d *failingUpdateAppDB) UpdateApplication(*model.Application) error {
+	return errors.New("update failed")
+}
+
+type errReader struct{ err error }
+
+func (r errReader) Read([]byte) (int, error) { return 0, r.err }