Merge pull request #407 from safaci2000/feature/scanners

Fixing various security issues and updating null_types to handle over…

.circleci/config.yml

@@ -8,7 +8,7 @@ jobs:
   build_and_tests:
     docker:
       # specify the version
-      - image: cimg/go:1.21.6
+      - image: cimg/go:1.22.8
       - image: cimg/postgres:14.10
         environment:
           POSTGRES_USER: jet
@@ -128,17 +128,13 @@ jobs:
 
       # to create test results report
       - run:
-          name: Install go-junit-report
+          name: Install gotestsum
-          command: go install github.com/jstemmer/go-junit-report@latest
+          command: go install gotest.tools/gotestsum@latest
-
       - run: mkdir -p $TEST_RESULTS
 
-      # this will run all tests and exclude test files from code coverage report
+      - run:
-      - run:  |
+          name: Running tests
-          go test -v ./... \
+          command:  gotestsum --junitfile $TEST_RESULTS/report.xml --format testname   -- -coverprofile=cover.out  -covermode=atomic -coverpkg=github.com/go-jet/jet/v2/postgres/...,github.com/go-jet/jet/v2/mysql/...,github.com/go-jet/jet/v2/sqlite/...,github.com/go-jet/jet/v2/qrm/...,github.com/go-jet/jet/v2/generator/...,github.com/go-jet/jet/v2/internal/... ./...
-            -covermode=atomic \
-            -coverpkg=github.com/go-jet/jet/v2/postgres/...,github.com/go-jet/jet/v2/mysql/...,github.com/go-jet/jet/v2/sqlite/...,github.com/go-jet/jet/v2/qrm/...,github.com/go-jet/jet/v2/generator/...,github.com/go-jet/jet/v2/internal/... \
-            -coverprofile=cover.out 2>&1 | go-junit-report > $TEST_RESULTS/results.xml
 
       # run mariaDB and cockroachdb tests. No need to collect coverage, because coverage is already included with mysql and postgres tests
       - run: MY_SQL_SOURCE=MariaDB go test -v  ./tests/mysql/
@@ -163,4 +159,4 @@ workflows:
   version: 2
   build_and_test:
     jobs:
-      - build_and_tests
+      - build_and_tests

.github/workflows/code_scanner.yml

@@ -0,0 +1,45 @@
+name: Code Scanners
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+
+permissions:
+  contents: read
+
+env:
+  go_version: "1.22.8"
+
+
+jobs:
+  security_scanning:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout Source
+        uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.go_version }}
+          cache: true
+      - name: Setup Tools
+        run: |
+          go install github.com/securego/gosec/v2/cmd/gosec@latest
+      - name: Running Scan
+        run: gosec --exclude=G402,G304  ./... 
+  lint_scanner:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout Source
+        uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.go_version }}
+          cache: true
+      - name: Setup Tools
+        run: |
+          go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
+      - name: Running Scan
+        run:  golangci-lint run --timeout=30m  ./...

.golangci.yml

@@ -0,0 +1,19 @@
+run:
+  # The default concurrency value is the number of available CPU.
+  concurrency: 4
+  # Timeout for analysis, e.g. 30s, 5m.
+  # Default: 1m
+  timeout: 30m
+  # Exit code when at least one issue was found.
+  # Default: 1
+  issues-exit-code: 2
+  # Include test files or not.
+  # Default: true
+  tests: false
+
+issues:
+  exclude-dirs:
+    - tests
+  exclude-files:
+    - "_test.go"
+    - "testutils.go"

examples/quick-start/quick-start.go

@@ -90,7 +90,7 @@ func main() {
 func jsonSave(path string, v interface{}) {
 	jsonText, _ := json.MarshalIndent(v, "", "\t")
 
-	err := os.WriteFile(path, jsonText, 0644)
+	err := os.WriteFile(path, jsonText, 0600)
 
 	panicOnError(err)
 }

go.mod

@@ -4,16 +4,16 @@ go 1.18
 
 require (
 	github.com/go-sql-driver/mysql v1.8.1
+	github.com/google/go-cmp v0.6.0
 	github.com/google/uuid v1.6.0
 	github.com/jackc/pgconn v1.14.3
+	github.com/jackc/pgtype v1.14.3
+	github.com/jackc/pgx/v4 v4.18.3
 	github.com/lib/pq v1.10.9
 	github.com/mattn/go-sqlite3 v1.14.17
 )
 
 require (
-	github.com/google/go-cmp v0.6.0
-	github.com/jackc/pgtype v1.14.3
-	github.com/jackc/pgx/v4 v4.18.3
 	github.com/pkg/profile v1.7.0
 	github.com/shopspring/decimal v1.4.0
 	github.com/stretchr/testify v1.9.0

internal/jet/statement.go

@@ -180,7 +180,7 @@ func duration(f func()) time.Duration {
 
 	f()
 
-	return time.Now().Sub(start)
+	return time.Since(start)
 }
 
 // ExpressionStatement interfacess

internal/testutils/test_utils.go

@@ -103,11 +103,12 @@ func AssertJSON(t *testing.T, data interface{}, expectedJSON string) {
 }
 
 // SaveJSONFile saves v as json at testRelativePath
+// nolint:unused
 func SaveJSONFile(v interface{}, testRelativePath string) {
 	jsonText, _ := json.MarshalIndent(v, "", "\t")
 
 	filePath := getFullPath(testRelativePath)
-	err := os.WriteFile(filePath, jsonText, 0644)
+	err := os.WriteFile(filePath, jsonText, 0600)
 
 	throw.OnError(err)
 }
@@ -116,7 +117,7 @@ func SaveJSONFile(v interface{}, testRelativePath string) {
 func AssertJSONFile(t *testing.T, data interface{}, testRelativePath string) {
 
 	filePath := getFullPath(testRelativePath)
-	fileJSONData, err := os.ReadFile(filePath)
+	fileJSONData, err := os.ReadFile(filePath) // #nosec G304
 	require.NoError(t, err)
 
 	if runtime.GOOS == "windows" {
@@ -243,7 +244,7 @@ func AssertQueryPanicErr(t *testing.T, stmt jet.Statement, db qrm.DB, dest inter
 
 // AssertFileContent check if file content at filePath contains expectedContent text.
 func AssertFileContent(t *testing.T, filePath string, expectedContent string) {
-	enumFileData, err := os.ReadFile(filePath)
+	enumFileData, err := os.ReadFile(filePath) // #nosec G304
 
 	require.NoError(t, err)
 

internal/utils/filesys/filesys.go

@@ -1,6 +1,7 @@
 package filesys
 
 import (
+	"errors"
 	"fmt"
 	"go/format"
 	"os"
@@ -16,7 +17,7 @@ func FormatAndSaveGoFile(dirPath, fileName string, text []byte) error {
 		newGoFilePath += ".go"
 	}
 
-	file, err := os.Create(newGoFilePath)
+	file, err := os.Create(newGoFilePath) // #nosec 304
 
 	if err != nil {
 		return err
@@ -28,7 +29,10 @@ func FormatAndSaveGoFile(dirPath, fileName string, text []byte) error {
 
 	// if there is a format error we will write unformulated text for debug purposes
 	if err != nil {
-		file.Write(text)
+		_, writeErr := file.Write(text)
+		if writeErr != nil {
+			return errors.Join(writeErr, fmt.Errorf("failed to format '%s', check '%s' for syntax errors: %w", fileName, newGoFilePath, err))
+		}
 		return fmt.Errorf("failed to format '%s', check '%s' for syntax errors: %w", fileName, newGoFilePath, err)
 	}
 
@@ -43,7 +47,7 @@ func FormatAndSaveGoFile(dirPath, fileName string, text []byte) error {
 // EnsureDirPathExist ensures dir path exists. If path does not exist, creates new path.
 func EnsureDirPathExist(dirPath string) error {
 	if _, err := os.Stat(dirPath); os.IsNotExist(err) {
-		err := os.MkdirAll(dirPath, os.ModePerm)
+		err := os.MkdirAll(dirPath, 0o750)
 
 		if err != nil {
 			return fmt.Errorf("can't create directory - %s: %w", dirPath, err)

mysql/interval.go

@@ -14,25 +14,25 @@ type unitType string
 // List of interval unit types for MySQL
 const (
 	MICROSECOND        unitType = "MICROSECOND"
-	SECOND                      = "SECOND"
+	SECOND             unitType = "SECOND"
-	MINUTE                      = "MINUTE"
+	MINUTE             unitType = "MINUTE"
-	HOUR                        = "HOUR"
+	HOUR               unitType = "HOUR"
-	DAY                         = "DAY"
+	DAY                unitType = "DAY"
-	WEEK                        = "WEEK"
+	WEEK               unitType = "WEEK"
-	MONTH                       = "MONTH"
+	MONTH              unitType = "MONTH"
-	QUARTER                     = "QUARTER"
+	QUARTER            unitType = "QUARTER"
-	YEAR                        = "YEAR"
+	YEAR               unitType = "YEAR"
-	SECOND_MICROSECOND          = "SECOND_MICROSECOND"
+	SECOND_MICROSECOND unitType = "SECOND_MICROSECOND"
-	MINUTE_MICROSECOND          = "MINUTE_MICROSECOND"
+	MINUTE_MICROSECOND unitType = "MINUTE_MICROSECOND"
-	MINUTE_SECOND               = "MINUTE_SECOND"
+	MINUTE_SECOND      unitType = "MINUTE_SECOND"
-	HOUR_MICROSECOND            = "HOUR_MICROSECOND"
+	HOUR_MICROSECOND   unitType = "HOUR_MICROSECOND"
-	HOUR_SECOND                 = "HOUR_SECOND"
+	HOUR_SECOND        unitType = "HOUR_SECOND"
-	HOUR_MINUTE                 = "HOUR_MINUTE"
+	HOUR_MINUTE        unitType = "HOUR_MINUTE"
-	DAY_MICROSECOND             = "DAY_MICROSECOND"
+	DAY_MICROSECOND    unitType = "DAY_MICROSECOND"
-	DAY_SECOND                  = "DAY_SECOND"
+	DAY_SECOND         unitType = "DAY_SECOND"
-	DAY_MINUTE                  = "DAY_MINUTE"
+	DAY_MINUTE         unitType = "DAY_MINUTE"
-	DAY_HOUR                    = "DAY_HOUR"
+	DAY_HOUR           unitType = "DAY_HOUR"
-	YEAR_MONTH                  = "YEAR_MONTH"
+	YEAR_MONTH         unitType = "YEAR_MONTH"
 )
 
 // Interval is representation of MySQL interval

qrm/internal/null_types.go

@@ -10,6 +10,10 @@ import (
 	"time"
 )
 
+var (
+	castOverFlowError = fmt.Errorf("cannot cast a negative value to an unsigned value, buffer overflow error")
+)
+
 // NullBool struct
 type NullBool struct {
 	sql.NullBool
@@ -119,32 +123,47 @@ func (n *NullUInt64) Scan(value interface{}) error {
 		n.Valid = false
 		return nil
 	case int64:
+		if v < 0 {
+			return castOverFlowError
+		}
+		n.UInt64, n.Valid = uint64(v), true
+		return nil
+	case int32:
+		if v < 0 {
+			return castOverFlowError
+		}
+		n.UInt64, n.Valid = uint64(v), true
+		return nil
+	case int16:
+		if v < 0 {
+			return castOverFlowError
+		}
+		n.UInt64, n.Valid = uint64(v), true
+		return nil
+	case int8:
+		if v < 0 {
+			return castOverFlowError
+		}
+		n.UInt64, n.Valid = uint64(v), true
+		return nil
+	case int:
+		if v < 0 {
+			return castOverFlowError
+		}
 		n.UInt64, n.Valid = uint64(v), true
 		return nil
 	case uint64:
 		n.UInt64, n.Valid = v, true
 		return nil
-	case int32:
-		n.UInt64, n.Valid = uint64(v), true
-		return nil
 	case uint32:
 		n.UInt64, n.Valid = uint64(v), true
 		return nil
-	case int16:
-		n.UInt64, n.Valid = uint64(v), true
-		return nil
 	case uint16:
 		n.UInt64, n.Valid = uint64(v), true
 		return nil
-	case int8:
-		n.UInt64, n.Valid = uint64(v), true
-		return nil
 	case uint8:
 		n.UInt64, n.Valid = uint64(v), true
 		return nil
-	case int:
-		n.UInt64, n.Valid = uint64(v), true
-		return nil
 	case uint:
 		n.UInt64, n.Valid = uint64(v), true
 		return nil

qrm/internal/null_types_test.go

@@ -2,6 +2,7 @@ package internal
 
 import (
 	"fmt"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"testing"
 	"time"
@@ -62,11 +63,21 @@ func TestNullUInt64(t *testing.T) {
 	value, _ := nullUInt64.Value()
 	require.Equal(t, value, uint64(11))
 
+	require.NoError(t, nullUInt64.Scan(uint64(11)))
+	require.Equal(t, nullUInt64.Valid, true)
+	value, _ = nullUInt64.Value()
+	require.Equal(t, value, uint64(11))
+
 	require.NoError(t, nullUInt64.Scan(int32(32)))
 	require.Equal(t, nullUInt64.Valid, true)
 	value, _ = nullUInt64.Value()
 	require.Equal(t, value, uint64(32))
 
+	require.NoError(t, nullUInt64.Scan(uint32(32)))
+	require.Equal(t, nullUInt64.Valid, true)
+	value, _ = nullUInt64.Value()
+	require.Equal(t, value, uint64(32))
+
 	require.NoError(t, nullUInt64.Scan(int16(20)))
 	require.Equal(t, nullUInt64.Valid, true)
 	value, _ = nullUInt64.Value()
@@ -88,4 +99,29 @@ func TestNullUInt64(t *testing.T) {
 	require.Equal(t, value, uint64(30))
 
 	require.Error(t, nullUInt64.Scan("text"), "can't scan int32 from text")
+
+	//Validate negative use cases
+	err := nullUInt64.Scan(int64(-5))
+	assert.NotNil(t, err)
+	assert.Error(t, err, castOverFlowError)
+
+	//Validate negative use cases
+	err = nullUInt64.Scan(-5)
+	assert.NotNil(t, err)
+	assert.Error(t, err, castOverFlowError)
+
+	//Validate negative use cases
+	err = nullUInt64.Scan(int32(-5))
+	assert.NotNil(t, err)
+	assert.Error(t, err, castOverFlowError)
+
+	//Validate negative use cases
+	err = nullUInt64.Scan(int16(-5))
+	assert.NotNil(t, err)
+	assert.Error(t, err, castOverFlowError)
+
+	//Validate negative use cases
+	err = nullUInt64.Scan(int8(-5))
+	assert.NotNil(t, err)
+	assert.Error(t, err, castOverFlowError)
 }

tests/init/init.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"database/sql"
+	"errors"
 	"flag"
 	"fmt"
 	"github.com/go-jet/jet/v2/generator/mysql"
@@ -124,7 +125,7 @@ func initMySQLDB(isMariaDB bool) error {
 
 		fmt.Println(cmdLine)
 
-		cmd := exec.Command("sh", "-c", cmdLine)
+		cmd := exec.Command("sh", "-c", cmdLine) // #nosec G204
 
 		cmd.Stderr = os.Stderr
 		cmd.Stdout = os.Stdout
@@ -183,7 +184,7 @@ func initPostgresDB(dbType string, connectionString string) error {
 }
 
 func execFile(db *sql.DB, sqlFilePath string) error {
-	testSampleSql, err := os.ReadFile(sqlFilePath)
+	testSampleSql, err := os.ReadFile(sqlFilePath) // #nosec G304
 	if err != nil {
 		return fmt.Errorf("failed to read sql file - %s: %w", sqlFilePath, err)
 	}
@@ -210,7 +211,10 @@ func execInTx(db *sql.DB, f func(tx *sql.Tx) error) error {
 	err = f(tx)
 
 	if err != nil {
-		tx.Rollback()
+		rollBackError := tx.Rollback()
+		if rollBackError != nil {
+			return errors.Join(rollBackError, err)
+		}
 		return err
 	}
 

tests/internal/utils/file/file.go

@@ -10,7 +10,7 @@ import (
 // Exists expects file to exist on path constructed from pathElems and returns content of the file
 func Exists(t *testing.T, pathElems ...string) (fileContent string) {
 	modelFilePath := path.Join(pathElems...)
-	file, err := os.ReadFile(modelFilePath)
+	file, err := os.ReadFile(modelFilePath) // #nosec G304
 	require.Nil(t, err)
 	require.NotEmpty(t, file)
 	return string(file)
@@ -19,6 +19,6 @@ func Exists(t *testing.T, pathElems ...string) (fileContent string) {
 // NotExists expects file not to exist on path constructed from pathElems
 func NotExists(t *testing.T, pathElems ...string) {
 	modelFilePath := path.Join(pathElems...)
-	_, err := os.ReadFile(modelFilePath)
+	_, err := os.ReadFile(modelFilePath) // #nosec G304
 	require.True(t, os.IsNotExist(err))
 }

tests/mysql/main_test.go

@@ -6,12 +6,9 @@ import (
 	jetmysql "github.com/go-jet/jet/v2/mysql"
 	"github.com/go-jet/jet/v2/postgres"
 	"github.com/go-jet/jet/v2/tests/dbconfig"
-	"github.com/stretchr/testify/require"
-	"math/rand"
-	"runtime"
-	"time"
-
 	_ "github.com/go-sql-driver/mysql"
+	"github.com/stretchr/testify/require"
+	"runtime"
 
 	"github.com/pkg/profile"
 	"os"
@@ -33,7 +30,6 @@ func sourceIsMariaDB() bool {
 }
 
 func TestMain(m *testing.M) {
-	rand.Seed(time.Now().Unix())
 	defer profile.Start().Stop()
 
 	var err error

tests/postgres/main_test.go

@@ -5,13 +5,10 @@ import (
 	"database/sql"
 	"fmt"
 	"github.com/go-jet/jet/v2/tests/internal/utils/repo"
-	"math/rand"
+	"github.com/jackc/pgx/v4/stdlib"
 	"os"
 	"runtime"
 	"testing"
-	"time"
-
-	"github.com/jackc/pgx/v4/stdlib"
 
 	"github.com/go-jet/jet/v2/postgres"
 	"github.com/go-jet/jet/v2/tests/dbconfig"
@@ -44,7 +41,6 @@ func skipForCockroachDB(t *testing.T) {
 }
 
 func TestMain(m *testing.M) {
-	rand.Seed(time.Now().Unix())
 	defer profile.Start().Stop()
 
 	setTestRoot()

tests/sqlite/main_test.go

@@ -8,30 +8,21 @@ import (
 	"github.com/go-jet/jet/v2/postgres"
 	"github.com/go-jet/jet/v2/sqlite"
 	"github.com/go-jet/jet/v2/tests/dbconfig"
-	"github.com/stretchr/testify/require"
-	"math/rand"
-	"os"
-	"os/exec"
-	"runtime"
-	"strings"
-	"testing"
-	"time"
-
 	"github.com/pkg/profile"
+	"github.com/stretchr/testify/require"
+	"os"
+	"runtime"
+	"testing"
 
 	_ "github.com/mattn/go-sqlite3"
 )
 
 var db *sql.DB
 var sampleDB *sql.DB
-var testRoot string
 
 func TestMain(m *testing.M) {
-	rand.Seed(time.Now().Unix())
 	defer profile.Start().Stop()
 
-	setTestRoot()
-
 	var err error
 	db, err = sql.Open("sqlite3", "file:"+dbconfig.SakilaDBPath)
 	throw.OnError(err)
@@ -50,16 +41,6 @@ func TestMain(m *testing.M) {
 	}
 }
 
-func setTestRoot() {
-	cmd := exec.Command("git", "rev-parse", "--show-toplevel")
-	byteArr, err := cmd.Output()
-	if err != nil {
-		panic(err)
-	}
-
-	testRoot = strings.TrimSpace(string(byteArr)) + "/tests/"
-}
-
 var loggedSQL string
 var loggedSQLArgs []interface{}
 var loggedDebugSQL string