From 974882fd02f287391f7fcf8ecb681ec22492c170 Mon Sep 17 00:00:00 2001 From: Samir Faci Date: Tue, 8 Oct 2024 10:17:25 -0400 Subject: [PATCH] Adding gosec and lint, fixing null_type overflow ChangeLog: - Adding gosec linting - Adding static type to enum - fixing nulltype overflow - Trying out gotestsum as an alternative to go-junit-report.xml --- .circleci/config.yml | 21 ++++++++----- .github/workflows/code_scanner.yml | 48 +++++++++++++++++++++++++++++ .golangci.yml | 19 ++++++++++++ examples/quick-start/quick-start.go | 2 +- go.mod | 8 ++--- internal/jet/statement.go | 2 +- internal/testutils/test_utils.go | 7 +++-- internal/utils/filesys/filesys.go | 10 ++++-- mysql/interval.go | 38 +++++++++++------------ qrm/internal/null_types.go | 37 ++++++++++++++++------ qrm/internal/null_types_test.go | 36 ++++++++++++++++++++++ tests/init/init.go | 10 ++++-- tests/internal/utils/file/file.go | 4 +-- tests/mysql/main_test.go | 6 +--- tests/postgres/main_test.go | 6 +--- tests/sqlite/main_test.go | 21 +------------ 16 files changed, 192 insertions(+), 83 deletions(-) create mode 100644 .github/workflows/code_scanner.yml create mode 100644 .golangci.yml diff --git a/.circleci/config.yml b/.circleci/config.yml index e21f00a4..05bf13c8 100644 --- a/.circleci/config.yml +++ b/.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.0 - image: cimg/postgres:14.10 environment: POSTGRES_USER: jet @@ -129,16 +129,21 @@ jobs: # to create test results report - run: name: Install go-junit-report - command: go install github.com/jstemmer/go-junit-report@latest - + command: go install github.com/jstemmer/go-junit-report/v2@v2.1.0 + - run: + name: Install gotestsum + 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: | - go test -v ./... \ - -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: + name: Running tests + command: gotestsum --junitfile report.xml --format testname + #- run: | + # go test -v ./... \ + # -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/ diff --git a/.github/workflows/code_scanner.yml b/.github/workflows/code_scanner.yml new file mode 100644 index 00000000..affc7564 --- /dev/null +++ b/.github/workflows/code_scanner.yml @@ -0,0 +1,48 @@ +name: Code Scanners +on: + push: + branches: + - master + pull_request: + branches: + - master + +env: + GO_VERSION: 1.22.0 + + +permissions: + contents: read + # Optional: allow read access to pull request. Use with `only-new-issues` option. + # pull-requests: read + + +jobs: + security_scanning: + runs-on: ubuntu-latest + steps: + - name: Checkout Source + uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: 1.22.0 + 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: 1.22.0 + 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 ./... diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..4eabe050 --- /dev/null +++ b/.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" diff --git a/examples/quick-start/quick-start.go b/examples/quick-start/quick-start.go index b4707f6d..dc849898 100644 --- a/examples/quick-start/quick-start.go +++ b/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) } diff --git a/go.mod b/go.mod index cb204af9..ffa977e2 100644 --- a/go.mod +++ b/go.mod @@ -1,19 +1,19 @@ module github.com/go-jet/jet/v2 -go 1.18 +go 1.22.0 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 diff --git a/internal/jet/statement.go b/internal/jet/statement.go index 67031546..2ca229df 100644 --- a/internal/jet/statement.go +++ b/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 diff --git a/internal/testutils/test_utils.go b/internal/testutils/test_utils.go index cfedde4f..c2fdc8d9 100644 --- a/internal/testutils/test_utils.go +++ b/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) diff --git a/internal/utils/filesys/filesys.go b/internal/utils/filesys/filesys.go index d904be88..07244707 100644 --- a/internal/utils/filesys/filesys.go +++ b/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) diff --git a/mysql/interval.go b/mysql/interval.go index ce1c6092..23be21f6 100644 --- a/mysql/interval.go +++ b/mysql/interval.go @@ -14,25 +14,25 @@ type unitType string // List of interval unit types for MySQL const ( MICROSECOND unitType = "MICROSECOND" - SECOND = "SECOND" - MINUTE = "MINUTE" - HOUR = "HOUR" - DAY = "DAY" - WEEK = "WEEK" - MONTH = "MONTH" - QUARTER = "QUARTER" - YEAR = "YEAR" - SECOND_MICROSECOND = "SECOND_MICROSECOND" - MINUTE_MICROSECOND = "MINUTE_MICROSECOND" - MINUTE_SECOND = "MINUTE_SECOND" - HOUR_MICROSECOND = "HOUR_MICROSECOND" - HOUR_SECOND = "HOUR_SECOND" - HOUR_MINUTE = "HOUR_MINUTE" - DAY_MICROSECOND = "DAY_MICROSECOND" - DAY_SECOND = "DAY_SECOND" - DAY_MINUTE = "DAY_MINUTE" - DAY_HOUR = "DAY_HOUR" - YEAR_MONTH = "YEAR_MONTH" + SECOND unitType = "SECOND" + MINUTE unitType = "MINUTE" + HOUR unitType = "HOUR" + DAY unitType = "DAY" + WEEK unitType = "WEEK" + MONTH unitType = "MONTH" + QUARTER unitType = "QUARTER" + YEAR unitType = "YEAR" + SECOND_MICROSECOND unitType = "SECOND_MICROSECOND" + MINUTE_MICROSECOND unitType = "MINUTE_MICROSECOND" + MINUTE_SECOND unitType = "MINUTE_SECOND" + HOUR_MICROSECOND unitType = "HOUR_MICROSECOND" + HOUR_SECOND unitType = "HOUR_SECOND" + HOUR_MINUTE unitType = "HOUR_MINUTE" + DAY_MICROSECOND unitType = "DAY_MICROSECOND" + DAY_SECOND unitType = "DAY_SECOND" + DAY_MINUTE unitType = "DAY_MINUTE" + DAY_HOUR unitType = "DAY_HOUR" + YEAR_MONTH unitType = "YEAR_MONTH" ) // Interval is representation of MySQL interval diff --git a/qrm/internal/null_types.go b/qrm/internal/null_types.go index ab75cf62..85d9c68d 100644 --- a/qrm/internal/null_types.go +++ b/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,30 +123,45 @@ 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 uint64: - n.UInt64, n.Valid = v, true - return nil case int32: + if v < 0 { + return castOverFlowError + } n.UInt64, n.Valid = uint64(v), true return nil - case uint32: + case int16: + if v < 0 { + return castOverFlowError + } n.UInt64, n.Valid = uint64(v), true return nil - case int16: + case int8: + if v < 0 { + return castOverFlowError + } n.UInt64, n.Valid = uint64(v), true return nil - case uint16: + case int: + if v < 0 { + return castOverFlowError + } n.UInt64, n.Valid = uint64(v), true return nil - case int8: + case uint64: + n.UInt64, n.Valid = v, true + return nil + case uint32: n.UInt64, n.Valid = uint64(v), true return nil - case uint8: + case uint16: n.UInt64, n.Valid = uint64(v), true return nil - case int: + case uint8: n.UInt64, n.Valid = uint64(v), true return nil case uint: diff --git a/qrm/internal/null_types_test.go b/qrm/internal/null_types_test.go index a15b104d..eab2dd28 100644 --- a/qrm/internal/null_types_test.go +++ b/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) } diff --git a/tests/init/init.go b/tests/init/init.go index 10631f1d..5a0ccdb0 100644 --- a/tests/init/init.go +++ b/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 } diff --git a/tests/internal/utils/file/file.go b/tests/internal/utils/file/file.go index fea06342..73095ea3 100644 --- a/tests/internal/utils/file/file.go +++ b/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)) } diff --git a/tests/mysql/main_test.go b/tests/mysql/main_test.go index f6ce57d8..4e3b5d51 100644 --- a/tests/mysql/main_test.go +++ b/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/go-sql-driver/mysql" "github.com/stretchr/testify/require" - "math/rand" "runtime" - "time" - - _ "github.com/go-sql-driver/mysql" "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 diff --git a/tests/postgres/main_test.go b/tests/postgres/main_test.go index 08af67a0..5b7f48d2 100644 --- a/tests/postgres/main_test.go +++ b/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() diff --git a/tests/sqlite/main_test.go b/tests/sqlite/main_test.go index 49758451..c5d4fb62 100644 --- a/tests/sqlite/main_test.go +++ b/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/pkg/profile" "github.com/stretchr/testify/require" - "math/rand" "os" - "os/exec" "runtime" - "strings" "testing" - "time" - - "github.com/pkg/profile" _ "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