Uniform all temporary directories and allow customizing temp path

cmd/web.go

@@ -213,6 +213,10 @@ func serveInstalled(ctx *cli.Context) error {
 		log.Fatal("Can not find APP_DATA_PATH %q", setting.AppDataPath)
 	}
 
+	// the AppDataTempDir is fully managed by us with a safe sub-path
+	// so it's safe to automatically remove the outdated files
+	setting.AppDataTempDir("").RemoveOutdated(3 * 24 * time.Hour)
+
 	// Override the provided port number within the configuration
 	if ctx.IsSet("port") {
 		if err := setPort(ctx.String("port")); err != nil {

custom/conf/app.example.ini

@@ -197,13 +197,6 @@ RUN_USER = ; git
 ;; relative paths are made absolute relative to the APP_DATA_PATH
 ;SSH_SERVER_HOST_KEYS=ssh/gitea.rsa, ssh/gogs.rsa
 ;;
-;; Directory to create temporary files in when testing public keys using ssh-keygen,
-;; default is the system temporary directory.
-;SSH_KEY_TEST_PATH =
-;;
-;; Use `ssh-keygen` to parse public SSH keys. The value is passed to the shell. By default, Gitea does the parsing itself.
-;SSH_KEYGEN_PATH =
-;;
 ;; Enable SSH Authorized Key Backup when rewriting all keys, default is false
 ;SSH_AUTHORIZED_KEYS_BACKUP = false
 ;;
@@ -294,6 +287,9 @@ RUN_USER = ; git
 ;; Default path for App data
 ;APP_DATA_PATH = data ; relative paths will be made absolute with _`AppWorkPath`_
 ;;
+;; Base path for App's temp files, leave empty to use the managed tmp directory in APP_DATA_PATH
+;APP_TEMP_PATH =
+;;
 ;; Enable gzip compression for runtime-generated content, static resources excluded
 ;ENABLE_GZIP = false
 ;;
@@ -1069,15 +1065,6 @@ LEVEL = Info
 ;; Separate extensions with a comma. To line wrap files without an extension, just put a comma
 ;LINE_WRAP_EXTENSIONS = .txt,.md,.markdown,.mdown,.mkd,.livemd,
 
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;[repository.local]
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
-;;
-;; Path for local repository copy. Defaults to `tmp/local-repo` (content gets deleted on gitea restart)
-;LOCAL_COPY_PATH = tmp/local-repo
-
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;[repository.upload]
@@ -1087,9 +1074,6 @@ LEVEL = Info
 ;; Whether repository file uploads are enabled. Defaults to `true`
 ;ENABLED = true
 ;;
-;; Path for uploads. Defaults to `data/tmp/uploads` (content gets deleted on gitea restart)
-;TEMP_PATH = data/tmp/uploads
-;;
 ;; Comma-separated list of allowed file extensions (`.zip`), mime types (`text/plain`) or wildcard type (`image/*`, `audio/*`, `video/*`). Empty value or `*/*` allows all types.
 ;ALLOWED_TYPES =
 ;;
@@ -2594,9 +2578,6 @@ LEVEL = Info
 ;; Currently, only `minio` and `azureblob` is supported.
 ;SERVE_DIRECT = false
 ;;
-;; Path for chunked uploads. Defaults to APP_DATA_PATH + `tmp/package-upload`
-;CHUNKED_UPLOAD_PATH = tmp/package-upload
-;;
 ;; Maximum count of package versions a single owner can have (`-1` means no limits)
 ;LIMIT_TOTAL_OWNER_COUNT = -1
 ;; Maximum size of packages a single owner can use (`-1` means no limits, format `1000`, `1 MB`, `1 GiB`)

models/asymkey/ssh_key_fingerprint.go

@@ -6,27 +6,13 @@ package asymkey
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"code.gitea.io/gitea/models/db"
-	"code.gitea.io/gitea/modules/log"
-	"code.gitea.io/gitea/modules/process"
-	"code.gitea.io/gitea/modules/setting"
-	"code.gitea.io/gitea/modules/util"
 
 	"golang.org/x/crypto/ssh"
 	"xorm.io/builder"
 )
 
-// ___________.__                                         .__        __
-// \_   _____/|__| ____    ____   ________________________|__| _____/  |_
-//  |    __)  |  |/    \  / ___\_/ __ \_  __ \____ \_  __ \  |/    \   __\
-//  |     \   |  |   |  \/ /_/  >  ___/|  | \/  |_> >  | \/  |   |  \  |
-//  \___  /   |__|___|  /\___  / \___  >__|  |   __/|__|  |__|___|  /__|
-//      \/            \//_____/      \/      |__|                 \/
-//
-// This file contains functions for fingerprinting SSH keys
-//
 // The database is used in checkKeyFingerprint however most of these functions probably belong in a module
 
 // checkKeyFingerprint only checks if key fingerprint has been used as public key,
@@ -41,29 +27,6 @@ func checkKeyFingerprint(ctx context.Context, fingerprint string) error {
 	return nil
 }
 
-func calcFingerprintSSHKeygen(publicKeyContent string) (string, error) {
-	// Calculate fingerprint.
-	tmpPath, err := writeTmpKeyFile(publicKeyContent)
-	if err != nil {
-		return "", err
-	}
-	defer func() {
-		if err := util.Remove(tmpPath); err != nil {
-			log.Warn("Unable to remove temporary key file: %s: Error: %v", tmpPath, err)
-		}
-	}()
-	stdout, stderr, err := process.GetManager().Exec("AddPublicKey", "ssh-keygen", "-lf", tmpPath)
-	if err != nil {
-		if strings.Contains(stderr, "is not a public key file") {
-			return "", ErrKeyUnableVerify{stderr}
-		}
-		return "", util.NewInvalidArgumentErrorf("'ssh-keygen -lf %s' failed with error '%s': %s", tmpPath, err, stderr)
-	} else if len(stdout) < 2 {
-		return "", util.NewInvalidArgumentErrorf("not enough output for calculating fingerprint: %s", stdout)
-	}
-	return strings.Split(stdout, " ")[1], nil
-}
-
 func calcFingerprintNative(publicKeyContent string) (string, error) {
 	// Calculate fingerprint.
 	pk, _, _, _, err := ssh.ParseAuthorizedKey([]byte(publicKeyContent))
@@ -75,15 +38,12 @@ func calcFingerprintNative(publicKeyContent string) (string, error) {
 
 // CalcFingerprint calculate public key's fingerprint
 func CalcFingerprint(publicKeyContent string) (string, error) {
-	// Call the method based on configuration
-	useNative := setting.SSH.KeygenPath == ""
-	calcFn := util.Iif(useNative, calcFingerprintNative, calcFingerprintSSHKeygen)
-	fp, err := calcFn(publicKeyContent)
+	fp, err := calcFingerprintNative(publicKeyContent)
 	if err != nil {
 		if IsErrKeyUnableVerify(err) {
 			return "", err
 		}
-		return "", fmt.Errorf("CalcFingerprint(%s): %w", util.Iif(useNative, "native", "ssh-keygen"), err)
+		return "", fmt.Errorf("CalcFingerprint: %w", err)
 	}
 	return fp, nil
 }

models/asymkey/ssh_key_parse.go

@@ -13,12 +13,9 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
-	"os"
-	"strconv"
 	"strings"
 
 	"code.gitea.io/gitea/modules/log"
-	"code.gitea.io/gitea/modules/process"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 
@@ -175,20 +172,9 @@ func CheckPublicKeyString(content string) (_ string, err error) {
 		return content, nil
 	}
 
-	var (
-		fnName  string
-		keyType string
-		length  int
-	)
-	if len(setting.SSH.KeygenPath) == 0 {
-		fnName = "SSHNativeParsePublicKey"
-		keyType, length, err = SSHNativeParsePublicKey(content)
-	} else {
-		fnName = "SSHKeyGenParsePublicKey"
-		keyType, length, err = SSHKeyGenParsePublicKey(content)
-	}
+	keyType, length, err := SSHNativeParsePublicKey(content)
 	if err != nil {
-		return "", fmt.Errorf("%s: %w", fnName, err)
+		return "", fmt.Errorf("SSHNativeParsePublicKey: %w", err)
 	}
 	log.Trace("Key info [native: %v]: %s-%d", setting.SSH.StartBuiltinServer, keyType, length)
 
@@ -258,56 +244,3 @@ func SSHNativeParsePublicKey(keyLine string) (string, int, error) {
 	}
 	return "", 0, fmt.Errorf("unsupported key length detection for type: %s", pkey.Type())
 }
-
-// writeTmpKeyFile writes key content to a temporary file
-// and returns the name of that file, along with any possible errors.
-func writeTmpKeyFile(content string) (string, error) {
-	tmpFile, err := os.CreateTemp(setting.SSH.KeyTestPath, "gitea_keytest")
-	if err != nil {
-		return "", fmt.Errorf("TempFile: %w", err)
-	}
-	defer tmpFile.Close()
-
-	if _, err = tmpFile.WriteString(content); err != nil {
-		return "", fmt.Errorf("WriteString: %w", err)
-	}
-	return tmpFile.Name(), nil
-}
-
-// SSHKeyGenParsePublicKey extracts key type and length using ssh-keygen.
-func SSHKeyGenParsePublicKey(key string) (string, int, error) {
-	tmpName, err := writeTmpKeyFile(key)
-	if err != nil {
-		return "", 0, fmt.Errorf("writeTmpKeyFile: %w", err)
-	}
-	defer func() {
-		if err := util.Remove(tmpName); err != nil {
-			log.Warn("Unable to remove temporary key file: %s: Error: %v", tmpName, err)
-		}
-	}()
-
-	keygenPath := setting.SSH.KeygenPath
-	if len(keygenPath) == 0 {
-		keygenPath = "ssh-keygen"
-	}
-
-	stdout, stderr, err := process.GetManager().Exec("SSHKeyGenParsePublicKey", keygenPath, "-lf", tmpName)
-	if err != nil {
-		return "", 0, fmt.Errorf("fail to parse public key: %s - %s", err, stderr)
-	}
-	if strings.Contains(stdout, "is not a public key file") {
-		return "", 0, ErrKeyUnableVerify{stdout}
-	}
-
-	fields := strings.Split(stdout, " ")
-	if len(fields) < 4 {
-		return "", 0, fmt.Errorf("invalid public key line: %s", stdout)
-	}
-
-	keyType := strings.Trim(fields[len(fields)-1], "()\r\n")
-	length, err := strconv.ParseInt(fields[0], 10, 32)
-	if err != nil {
-		return "", 0, err
-	}
-	return strings.ToLower(keyType), int(length), nil
-}

models/asymkey/ssh_key_test.go

@@ -18,7 +18,6 @@ import (
 
 	"github.com/42wim/sshsig"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 func Test_SSHParsePublicKey(t *testing.T) {
@@ -45,27 +44,6 @@ func Test_SSHParsePublicKey(t *testing.T) {
 				assert.Equal(t, tc.keyType, keyTypeN)
 				assert.Equal(t, tc.length, lengthN)
 			})
-			if tc.skipSSHKeygen {
-				return
-			}
-			t.Run("SSHKeygen", func(t *testing.T) {
-				keyTypeK, lengthK, err := SSHKeyGenParsePublicKey(tc.content)
-				if err != nil {
-					// Some servers do not support ecdsa format.
-					if !strings.Contains(err.Error(), "line 1 too long:") {
-						require.NoError(t, err)
-					}
-				}
-				assert.Equal(t, tc.keyType, keyTypeK)
-				assert.Equal(t, tc.length, lengthK)
-			})
-			t.Run("SSHParseKeyNative", func(t *testing.T) {
-				keyTypeK, lengthK, err := SSHNativeParsePublicKey(tc.content)
-				require.NoError(t, err)
-
-				assert.Equal(t, tc.keyType, keyTypeK)
-				assert.Equal(t, tc.length, lengthK)
-			})
 		})
 	}
 }
@@ -186,14 +164,6 @@ func Test_calcFingerprint(t *testing.T) {
 				assert.NoError(t, err)
 				assert.Equal(t, tc.fp, fpN)
 			})
-			if tc.skipSSHKeygen {
-				return
-			}
-			t.Run("SSHKeygen", func(t *testing.T) {
-				fpK, err := calcFingerprintSSHKeygen(tc.content)
-				assert.NoError(t, err)
-				assert.Equal(t, tc.fp, fpK)
-			})
 		})
 	}
 }

models/issues/issue_update.go

@@ -845,6 +845,7 @@ func DeleteOrphanedIssues(ctx context.Context) error {
 
 	// Remove issue attachment files.
 	for i := range attachmentPaths {
+		// FIXME: it's not right, because the attachment might not be on local filesystem
 		system_model.RemoveAllWithNotice(ctx, "Delete issue attachment", attachmentPaths[i])
 	}
 	return nil

models/migrations/base/tests.go

@@ -15,6 +15,7 @@ import (
 	"code.gitea.io/gitea/models/unittest"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/tempdir"
 	"code.gitea.io/gitea/modules/test"
 	"code.gitea.io/gitea/modules/testlogger"
 
@@ -114,15 +115,16 @@ func MainTest(m *testing.M) {
 		setting.CustomConf = giteaConf
 	}
 
-	tmpDataPath, err := os.MkdirTemp("", "data")
+	tmpDataPath, cleanup, err := tempdir.OsTempDir("gitea-test").MkdirTempRandom("data")
 	if err != nil {
 		testlogger.Fatalf("Unable to create temporary data path %v\n", err)
 	}
+	defer cleanup()
 
 	setting.CustomPath = filepath.Join(setting.AppWorkPath, "custom")
 	setting.AppDataPath = tmpDataPath
 
-	unittest.InitSettings()
+	unittest.InitSettingsForTesting()
 	if err = git.InitFull(context.Background()); err != nil {
 		testlogger.Fatalf("Unable to InitFull: %v\n", err)
 	}
@@ -134,8 +136,5 @@ func MainTest(m *testing.M) {
 	if err := removeAllWithRetry(setting.RepoRootPath); err != nil {
 		fmt.Fprintf(os.Stderr, "os.RemoveAll: %v\n", err)
 	}
-	if err := removeAllWithRetry(tmpDataPath); err != nil {
-		fmt.Fprintf(os.Stderr, "os.RemoveAll: %v\n", err)
-	}
 	os.Exit(exitStatus)
 }

models/repo/upload.go

@@ -51,14 +51,10 @@ func init() {
 	db.RegisterModel(new(Upload))
 }
 
-// UploadLocalPath returns where uploads is stored in local file system based on given UUID.
-func UploadLocalPath(uuid string) string {
-	return filepath.Join(setting.Repository.Upload.TempPath, uuid[0:1], uuid[1:2], uuid)
-}
-
-// LocalPath returns where uploads are temporarily stored in local file system.
+// LocalPath returns where uploads are temporarily stored in local file system based on given UUID.
 func (upload *Upload) LocalPath() string {
-	return UploadLocalPath(upload.UUID)
+	uuid := upload.UUID
+	return setting.AppDataTempDir("repo-uploads").JoinPath(uuid[0:1], uuid[1:2], uuid)
 }
 
 // NewUpload creates a new upload object.