Fix security issue and add go linter (#677)

* Fix security issue and add go linter * Fix error checked by golangci-lint * Delete jwt-go pkg --------- Co-authored-by: 杨赫然 <[email protected]>
haiwen · Aug 12, 2024 · 7387164 · 7387164
1 parent 026a6d5
commit 7387164
Show file tree

Hide file tree

Showing 19 changed files with 134 additions and 78 deletions.
diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml
@@ -0,0 +1,36 @@
+name: golangci-lint
+
+on: [push, pull_request]
+
+permissions:
+  contents: read
+  # Optional: allow read access to pull request. Use with `only-new-issues` option.
+  # pull-requests: read
+
+jobs:
+  golangci-fileserver:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: stable
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v6
+        with:
+          version: v1.59
+          working-directory: ./fileserver
+  golangci-notification-server:
+    name: lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: stable
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v6
+        with:
+          version: v1.59
+          working-directory: ./notification-server
diff --git a/fileserver/.golangci.yml b/fileserver/.golangci.yml
@@ -0,0 +1,13 @@
+run:
+  timeout: 2m
+
+linters:
+  enable:
+   - govet
+   - gosimple
+   - ineffassign
+   - staticcheck
+   - unused
+   - gofmt
+  disable:
+   - errcheck
diff --git a/fileserver/diff/diff.go b/fileserver/diff/diff.go
@@ -188,7 +188,7 @@ func diffDirectories(baseDir string, dents []*fsmgr.SeafDirent, opt *DiffOptions
 func direntSame(dentA, dentB *fsmgr.SeafDirent) bool {
 	return dentA.ID == dentB.ID &&
 		dentA.Mode == dentB.Mode &&
-		dentA.Mtime == dentA.Mtime
+		dentA.Mtime == dentB.Mtime
 }
 
 // Diff type and diff status.

diff --git a/fileserver/diff/diff_test.go b/fileserver/diff/diff_test.go
@@ -44,7 +44,7 @@ var diffTestDirID2 string
 */
 
 func TestDiffTrees(t *testing.T) {
-	fsmgr.Init(diffTestSeafileConfPath, diffTestSeafileDataDir)
+	fsmgr.Init(diffTestSeafileConfPath, diffTestSeafileDataDir, 2<<30)
 
 	err := diffTestCreateTestDir()
 	if err != nil {

diff --git a/fileserver/fileop.go b/fileserver/fileop.go
@@ -295,10 +295,7 @@ func doFile(rsp http.ResponseWriter, r *http.Request, repo *repomgr.Repo, fileID
 
 func isNetworkErr(err error) bool {
 	_, ok := err.(net.Error)
-	if ok {
-		return true
-	}
-	return false
+	return ok
 }
 
 type blockMap struct {
@@ -513,7 +510,7 @@ func setCommonHeaders(rsp http.ResponseWriter, r *http.Request, operation, fileN
 	fileType := parseContentType(fileName)
 	if fileType != "" {
 		var contentType string
-		if strings.Index(fileType, "text") != -1 {
+		if strings.Contains(fileType, "text") {
 			contentType = fileType + "; " + "charset=gbk"
 		} else {
 			contentType = fileType
@@ -1392,8 +1389,6 @@ func clearTmpFile(fsm *recvData, parentDir string) {
 		}
 		repomgr.DelUploadTmpFile(fsm.repoID, filePath)
 	}
-
-	return
 }
 
 func parseUploadHeaders(r *http.Request) (*recvData, *appError) {
@@ -1493,7 +1488,7 @@ func postMultiFiles(rsp http.ResponseWriter, r *http.Request, repoID, parentDir,
 			return &appError{nil, msg, http.StatusBadRequest}
 		}
 	}
-	if strings.Index(parentDir, "//") != -1 {
+	if strings.Contains(parentDir, "//") {
 		msg := "parent_dir contains // sequence.\n"
 		return &appError{nil, msg, http.StatusBadRequest}
 	}
@@ -1798,11 +1793,11 @@ func genCommitNeedRetry(repo *repomgr.Repo, base *commitmgr.Commit, commit *comm
 		}
 
 		if !opt.conflict {
-			mergeDesc = fmt.Sprintf("Auto merge by system")
+			mergeDesc = "Auto merge by system"
 		} else {
 			mergeDesc = genMergeDesc(repo, opt.mergedRoot, currentHead.RootID, newRoot)
 			if mergeDesc == "" {
-				mergeDesc = fmt.Sprintf("Auto merge by system")
+				mergeDesc = "Auto merge by system"
 			}
 		}
 
@@ -2155,7 +2150,7 @@ func shouldIgnoreFile(fileName string) bool {
 		return true
 	}
 
-	if strings.Index(fileName, "/") != -1 {
+	if strings.Contains(fileName, "/") {
 		return true
 	}
 
@@ -2343,7 +2338,7 @@ func chunkFile(job chunkingData) (string, error) {
 		defer f.Close()
 		file = f
 	}
-	_, err := file.Seek(offset, os.SEEK_SET)
+	_, err := file.Seek(offset, io.SeekStart)
 	if err != nil {
 		err := fmt.Errorf("failed to seek file: %v", err)
 		return "", err
@@ -2456,11 +2451,7 @@ func checkParentDir(repoID string, parentDir string) *appError {
 func isParentMatched(uploadDir, parentDir string) bool {
 	uploadCanon := filepath.Join("/", uploadDir)
 	parentCanon := filepath.Join("/", parentDir)
-	if uploadCanon != parentCanon {
-		return false
-	}
-
-	return true
+	return uploadCanon == parentCanon
 }
 
 func parseContentRange(ranges string, fsm *recvData) bool {
@@ -2573,7 +2564,7 @@ func updateDir(repoID, dirPath, newDirID, user, headID string) (string, error) {
 	if dirPath == "/" {
 		commitDesc := genCommitDesc(repo, newDirID, headCommit.RootID)
 		if commitDesc == "" {
-			commitDesc = fmt.Sprintf("Auto merge by system")
+			commitDesc = "Auto merge by system"
 		}
 		newCommitID, err := genNewCommit(repo, headCommit, newDirID, user, commitDesc, true)
 		if err != nil {
@@ -2613,7 +2604,7 @@ func updateDir(repoID, dirPath, newDirID, user, headID string) (string, error) {
 
 	commitDesc := genCommitDesc(repo, rootID, headCommit.RootID)
 	if commitDesc == "" {
-		commitDesc = fmt.Sprintf("Auto merge by system")
+		commitDesc = "Auto merge by system"
 	}
 
 	newCommitID, err := genNewCommit(repo, headCommit, rootID, user, commitDesc, true)
@@ -2937,7 +2928,7 @@ func putFile(rsp http.ResponseWriter, r *http.Request, repoID, parentDir, user,
 		return &appError{nil, msg, http.StatusBadRequest}
 	}
 
-	if strings.Index(parentDir, "//") != -1 {
+	if strings.Contains(parentDir, "//") {
 		msg := "parent_dir contains // sequence.\n"
 		return &appError{nil, msg, http.StatusBadRequest}
 	}
@@ -3212,7 +3203,7 @@ func commitFileBlocks(repoID, parentDir, fileName, blockIDsJSON, user string, fi
 		return "", &appError{nil, msg, http.StatusBadRequest}
 	}
 
-	if strings.Index(parentDir, "//") != -1 {
+	if strings.Contains(parentDir, "//") {
 		msg := "parent_dir contains // sequence.\n"
 		return "", &appError{nil, msg, http.StatusBadRequest}
 	}

diff --git a/fileserver/fileserver.go b/fileserver/fileserver.go
@@ -46,9 +46,6 @@ var dbType string
 var seafileDB, ccnetDB *sql.DB
 var seahubURL, seahubPK string
 
-// when SQLite is used, user and group db are separated.
-var userDB, groupDB *sql.DB
-
 func init() {
 	flag.StringVar(&centralDir, "F", "", "central config directory")
 	flag.StringVar(&dataDir, "d", "", "seafile data directory")
@@ -284,12 +281,12 @@ func loadSeahubPK() {
 
 	scanner := bufio.NewScanner(file)
 
-	pkRe, err := regexp.Compile("SECRET_KEY\\s*=\\s*'([^']*)'")
+	pkRe, err := regexp.Compile(`SECRET_KEY\\s*=\\s*'([^']*)'`)
 	if err != nil {
 		log.Warnf("Failed to compile regex: %v", err)
 		return
 	}
-	siteRootRe, err := regexp.Compile("SITE_ROOT\\s*=\\s*'([^']*)'")
+	siteRootRe, err := regexp.Compile(`SITE_ROOT\\s*=\\s*'([^']*)'`)
 	if err != nil {
 		log.Warnf("Failed to compile regex: %v", err)
 		return
@@ -461,13 +458,10 @@ func handleSignals() {
 func handleUser1Signal() {
 	signalChan := make(chan os.Signal, 1)
 	signal.Notify(signalChan, syscall.SIGUSR1)
-	<-signalChan
 
 	for {
-		select {
-		case <-signalChan:
-			logRotate()
-		}
+		<-signalChan
+		logRotate()
 	}
 }
 

diff --git a/fileserver/fsmgr/fsmgr_test.go b/fileserver/fsmgr/fsmgr_test.go
@@ -65,7 +65,7 @@ func delFile() error {
 }
 
 func TestMain(m *testing.M) {
-	Init(seafileConfPath, seafileDataDir)
+	Init(seafileConfPath, seafileDataDir, 2<<30)
 	err := createFile()
 	if err != nil {
 		fmt.Printf("Failed to create test file : %v.\n", err)

diff --git a/fileserver/go.mod b/fileserver/go.mod
@@ -4,14 +4,14 @@ go 1.17
 
 require (
 	github.com/dgraph-io/ristretto v0.1.1
-	github.com/dgrijalva/jwt-go v3.2.0+incompatible
 	github.com/go-sql-driver/mysql v1.5.0
+	github.com/golang-jwt/jwt/v5 v5.2.1
 	github.com/google/uuid v1.1.1
 	github.com/gorilla/mux v1.7.4
 	github.com/json-iterator/go v1.1.12
 	github.com/mattn/go-sqlite3 v1.14.0
 	github.com/sirupsen/logrus v1.8.1
-	golang.org/x/text v0.3.7
+	golang.org/x/text v0.3.8
 	gopkg.in/ini.v1 v1.55.0
 )
 
@@ -23,5 +23,5 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/smartystreets/goconvey v1.6.4 // indirect
-	golang.org/x/sys v0.0.0-20221010170243-090e33056c14 // indirect
+	golang.org/x/sys v0.1.0 // indirect
 )
diff --git a/fileserver/go.sum b/fileserver/go.sum
@@ -11,10 +11,10 @@ github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2 h1:tdlZCpZ/P9DhczC
 github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2/go.mod h1:SqUrOPUnsFjfmXRMNPybcSiG0BgUW2AuFH8PAnS2iTw=
 github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
-github.com/dgrijalva/jwt-go v3.2.0+incompatible h1:7qlOGliEKZXTDg6OTjfoBKDXWrumCAMpl/TFQ4/5kLM=
-github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
 github.com/go-sql-driver/mysql v1.5.0 h1:ozyZYNQW3x3HtqT1jira07DN2PArx2v7/mN66gGcHOs=
 github.com/go-sql-driver/mysql v1.5.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg=
+github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk=
+github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b h1:VKtxabqXZkF25pY9ekfRL6a582T4P37/31XEstQ5p58=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
@@ -49,21 +49,41 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
+github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
+golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
+golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
 golang.org/x/net v0.0.0-20180218175443-cbe0f9307d01/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
+golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200202094626-16171245cfb2/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
+golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
+golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
+golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20221010170243-090e33056c14 h1:k5II8e6QD8mITdi+okbbmR/cIyEbeXLBhy5Ha4nevyc=
+golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20221010170243-090e33056c14/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U=
+golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
+golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
-golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk=
+golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
+golang.org/x/text v0.3.8 h1:nAL+RVCQ9uMn3vJZbV+MRnydTJFPf8qqY42YiA6MrqY=
+golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20190328211700-ab21143f2384/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
+golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
+golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
+golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/ini.v1 v1.55.0 h1:E8yzL5unfpW3M6fz/eB7Cb5MQAYSZ7GKo4Qth+N2sgQ=
 gopkg.in/ini.v1 v1.55.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=

diff --git a/fileserver/merge.go b/fileserver/merge.go
@@ -10,7 +10,7 @@ import (
 	"strings"
 	"time"
 
-	jwt "github.com/dgrijalva/jwt-go"
+	jwt "github.com/golang-jwt/jwt/v5"
 	"github.com/haiwen/seafile-server/fileserver/commitmgr"
 	"github.com/haiwen/seafile-server/fileserver/fsmgr"
 	"github.com/haiwen/seafile-server/fileserver/utils"
@@ -233,8 +233,9 @@ func mergeEntries(storeID string, dents []*fsmgr.SeafDirent, baseDir string, opt
 			mergedDents = append(mergedDents, head)
 			opt.conflict = true
 		}
-	} else if base != nil && head == nil && remote == nil {
-	}
+	} /* else if base != nil && head == nil && remote == nil {
+	    Don't need to add anything to mergeDents.
+	}*/
 
 	return mergedDents, nil
 }
@@ -264,17 +265,14 @@ func mergeDirectories(storeID string, dents []*fsmgr.SeafDirent, baseDir string,
 		if dents[0].ID == dents[1].ID {
 			return mergedDents, nil
 		}
-		break
 	case 4:
 		mergedDents = append(mergedDents, dents[2])
 		return mergedDents, nil
 	case 5:
 		if dents[0].ID == dents[2].ID {
 			return mergedDents, nil
 		}
-		break
-	case 6:
-	case 7:
+	case 6, 7:
 		if dents[1].ID == dents[2].ID {
 			mergedDents = append(mergedDents, dents[1])
 			return mergedDents, nil
@@ -285,7 +283,6 @@ func mergeDirectories(storeID string, dents []*fsmgr.SeafDirent, baseDir string,
 			mergedDents = append(mergedDents, dents[1])
 			return mergedDents, nil
 		}
-		break
 	default:
 		err := fmt.Errorf("wrong dir mask for merge")
 		return nil, err
@@ -400,6 +397,7 @@ func getNickNameByModifier(emailToNickname map[string]string, modifier string) s
 type SeahubClaims struct {
 	Exp        int64
 	IsInternal bool `json:"is_internal"`
+	jwt.RegisteredClaims
 }
 
 func (*SeahubClaims) Valid() error {
@@ -410,6 +408,7 @@ func postGetNickName(modifier string) string {
 	claims := SeahubClaims{
 		time.Now().Add(time.Second * 300).Unix(),
 		true,
+		jwt.RegisteredClaims{},
 	}
 
 	token := jwt.NewWithClaims(jwt.GetSigningMethod("HS256"), &claims)

diff --git a/fileserver/merge_test.go b/fileserver/merge_test.go
@@ -27,7 +27,6 @@ var mergeTestTree1CommitID string
 var mergeTestTree2CommitID string
 var mergeTestTree3CommitID string
 var mergeTestTree4CommitID string
-var mergeTestTree5CommitID string
 
 /*
 test directory structure: