fix: stop discarding file close and test setup errors (#39987)

- Fix
https://github.com/go-gitea/gitea/security/quality/rules/go%2Funhandled-writable-file-close,
ignored close errors hide failed flushes, which could lose rotated logs
and truncate authorized_keys
- Fix `TestProjects` fixture reset, from
https://github.com/go-gitea/gitea/security/quality/ai-findings,
`TruncateBeans` rejected the table names and the ignored error let the
test run against fixture data

cmd/embedded.go

@@ -261,13 +261,7 @@ func extractAsset(c *cli.Command, d string, a assetFile, overwrite, rename bool)
 		perms = fi.Mode()
 	}
 
-	file, err := os.OpenFile(dest, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, perms)
+	if err := os.WriteFile(dest, data, perms); err != nil {
-	if err != nil {
-		return fmt.Errorf("%s: %w", dest, err)
-	}
-	defer file.Close()
-
-	if _, err = file.Write(data); err != nil {
 		return fmt.Errorf("%s: %w", dest, err)
 	}
 

models/asymkey/ssh_key_authorized_keys.go

@@ -6,6 +6,7 @@ package asymkey
 import (
 	"bufio"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -107,7 +108,7 @@ func writeAuthorizedStringForKey(key *PublicKey, w io.Writer) (keyValid bool, er
 }
 
 // appendAuthorizedKeysToFile appends new SSH keys' content to authorized_keys file.
-func appendAuthorizedKeysToFile(keys ...*PublicKey) error {
+func appendAuthorizedKeysToFile(keys ...*PublicKey) (err error) {
 	// Don't need to rewrite this file if builtin SSH server is enabled.
 	if setting.SSH.StartBuiltinServer || !setting.SSH.CreateAuthorizedKeysFile {
 		return nil
@@ -133,7 +134,7 @@ func appendAuthorizedKeysToFile(keys ...*PublicKey) error {
 	if err != nil {
 		return err
 	}
-	defer f.Close()
+	defer func() { err = errors.Join(err, f.Close()) }()
 
 	// Note: chmod command does not support in Windows.
 	if !consts.IsWindows {

modules/assetfs/embed.go

@@ -6,6 +6,7 @@ package assetfs
 import (
 	"bytes"
 	"compress/gzip"
+	"errors"
 	"io"
 	"io/fs"
 	"os"
@@ -301,12 +302,12 @@ var GetExecutableModTime = sync.OnceValue(func() (modTime time.Time) {
 	return st.ModTime()
 })
 
-func GenerateEmbedBindata(fsRootPath, outputFile string) error {
+func GenerateEmbedBindata(fsRootPath, outputFile string) (err error) {
 	output, err := os.OpenFile(outputFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm)
 	if err != nil {
 		return err
 	}
-	defer output.Close()
+	defer func() { err = errors.Join(err, output.Close()) }()
 
 	meta := &EmbeddedMeta{}
 	meta.Root = &embeddedFileInfo{}

modules/util/rotatingfilewriter/writer.go

@@ -191,19 +191,16 @@ func compressOldFile(fname string, compressionLevel int) error {
 	if err != nil {
 		return fmt.Errorf("compressOldFile: failed to open new file %s: %w", fnameGz, err)
 	}
-	defer fw.Close()
 
 	zw, err := gzip.NewWriterLevel(fw, compressionLevel)
 	if err != nil {
+		_ = fw.Close()
 		return fmt.Errorf("compressOldFile: failed to create gzip writer: %w", err)
 	}
-	defer zw.Close()
 
 	_, err = buffer.WriteTo(zw)
-	if err != nil {
+	if err = errors.Join(err, zw.Close(), fw.Close()); err != nil {
-		_ = zw.Close()
+		_ = util.RemoveWithRetry(fnameGz)
-		_ = fw.Close()
-		_ = util.RemoveWithRetry(fname + ".gz")
 		return fmt.Errorf("compressOldFile: failed to write to gz file: %w", err)
 	}
 	_ = reader.Close()

services/asymkey/ssh_key_authorized_keys.go

@@ -74,6 +74,8 @@ func rewriteAllPublicKeys(ctx context.Context) error {
 		return err
 	}
 
-	_ = t.Close()
+	if err := t.Close(); err != nil {
+		return err
+	}
 	return util.RenameWithRetry(tmpPath, fPath)
 }

services/asymkey/ssh_key_authorized_principals.go

@@ -85,7 +85,9 @@ func rewriteAllPrincipalKeys(ctx context.Context) error {
 		return err
 	}
 
-	_ = t.Close()
+	if err := t.Close(); err != nil {
+		return err
+	}
 	return util.RenameWithRetry(tmpPath, fPath)
 }
 

services/pull/merge_prepare.go

@@ -152,16 +152,16 @@ func prepareTemporaryRepoForMerge(ctx *mergeContext) error {
 		log.Error("%-v Unable to write .git/info/sparse-checkout file in %s: %v", ctx.pr, ctx.tmpBasePath, err)
 		return fmt.Errorf("Unable to write .git/info/sparse-checkout file in tmpBasePath: %w", err)
 	}
-	defer sparseCheckoutListFile.Close() // we will close it earlier but we need to ensure it is closed if there is an error
 
-	if err := getDiffTree(ctx, ctx.tmpRepo, tmpRepoBaseBranch, tmpRepoTrackingBranch, sparseCheckoutListFile); err != nil {
+	diffErr := getDiffTree(ctx, ctx.tmpRepo, tmpRepoBaseBranch, tmpRepoTrackingBranch, sparseCheckoutListFile)
-		log.Error("%-v getDiffTree(%s, %s, %s): %v", ctx.pr, ctx.tmpBasePath, tmpRepoBaseBranch, tmpRepoTrackingBranch, err)
+	closeErr := sparseCheckoutListFile.Close()
-		return fmt.Errorf("getDiffTree: %w", err)
+	if diffErr != nil {
+		log.Error("%-v getDiffTree(%s, %s, %s): %v", ctx.pr, ctx.tmpBasePath, tmpRepoBaseBranch, tmpRepoTrackingBranch, diffErr)
+		return fmt.Errorf("getDiffTree: %w", diffErr)
 	}
-
+	if closeErr != nil {
-	if err := sparseCheckoutListFile.Close(); err != nil {
+		log.Error("%-v Unable to close .git/info/sparse-checkout file in %s: %v", ctx.pr, ctx.tmpBasePath, closeErr)
-		log.Error("%-v Unable to close .git/info/sparse-checkout file in %s: %v", ctx.pr, ctx.tmpBasePath, err)
+		return fmt.Errorf("Unable to close .git/info/sparse-checkout file in tmpBasePath: %w", closeErr)
-		return fmt.Errorf("Unable to close .git/info/sparse-checkout file in tmpBasePath: %w", err)
 	}
 
 	setConfig := func(key, value string) error {

services/pull/temp_repo.go

@@ -7,6 +7,7 @@ package pull
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -103,8 +104,8 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest)
 		if err != nil {
 			return fmt.Errorf("unable to open %s file in %s: %w", altFileRelPath, filepath.Base(repoPath), err)
 		}
-		defer f.Close()
+		_, err = fmt.Fprintln(f, newAltLine)
-		if _, err := fmt.Fprintln(f, newAltLine); err != nil {
+		if err = errors.Join(err, f.Close()); err != nil {
 			return fmt.Errorf("unable to write %s file in %s: %w", altFileRelPath, filepath.Base(repoPath), err)
 		}
 		return nil

tests/integration/project_test.go

@@ -395,18 +395,18 @@ func TestProjects(t *testing.T) {
 		assert.Contains(t, resp.Body.String(), linkedPR)
 	})
 	t.Run("LoadIssuesAssigneesForProject", func(t *testing.T) {
-		_ = db.TruncateBeans(t.Context(), "project_issue", "issue_assignees")
+		require.NoError(t, db.TruncateBeans(t.Context(), &project_model.ProjectIssue{}, &issues_model.IssueAssignees{}))
-		_ = db.Insert(t.Context(),
+		require.NoError(t, db.Insert(t.Context(),
 			&project_model.ProjectIssue{ProjectID: 1, IssueID: 1},
 			&project_model.ProjectIssue{ProjectID: 1, IssueID: 6},
-		)
+		))
-		_ = db.Insert(t.Context(),
+		require.NoError(t, db.Insert(t.Context(),
 			&issues_model.IssueAssignees{IssueID: 1, AssigneeID: 1},
 			&issues_model.IssueAssignees{IssueID: 1, AssigneeID: 10},
 			&issues_model.IssueAssignees{IssueID: 1, AssigneeID: 2},
 			&issues_model.IssueAssignees{IssueID: 6, AssigneeID: 2},
 			&issues_model.IssueAssignees{IssueID: 6, AssigneeID: 4},
-		)
+		))
 		assignees, err := project.LoadIssuesAssigneesForProject(t.Context(), 1)
 		require.NoError(t, err)
 		require.Len(t, assignees, 4)