Skip to content

Commit

Permalink
Fixing various security issues and updating null_types to handle over…
Browse files Browse the repository at this point in the history
…flow use case.
  • Loading branch information
safaci2000 committed Oct 8, 2024
1 parent c30a350 commit 85e4ad9
Show file tree
Hide file tree
Showing 11 changed files with 163 additions and 43 deletions.
48 changes: 48 additions & 0 deletions .github/workflows/code_scanner.yml
Original file line number Diff line number Diff line change
@@ -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: $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: "$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 ./...
18 changes: 18 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
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"
2 changes: 1 addition & 1 deletion examples/quick-start/quick-start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
12 changes: 5 additions & 7 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,23 +1,21 @@
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/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/lib/pq v1.10.9
github.com/mattn/go-sqlite3 v1.14.17
github.com/pkg/profile v1.7.0
github.com/shopspring/decimal v1.4.0
github.com/stretchr/testify v1.9.0
github.com/volatiletech/null/v8 v8.1.2
golang.org/x/exp v0.0.0-20241004190924-225e2abe05e6
gopkg.in/guregu/null.v4 v4.0.0
)

Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5y
golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU=
golang.org/x/crypto v0.20.0 h1:jmAMJJZXr5KiCw05dfYK9QnqaqKLYXijU23lsEdcQqg=
golang.org/x/crypto v0.20.0/go.mod h1:Xwo95rrVNIoSMx9wa1JroENMToLWn3RNVrTBpLHgZPQ=
golang.org/x/exp v0.0.0-20241004190924-225e2abe05e6 h1:1wqE9dj9NpSm04INVsJhhEUzhuDVjbcyKH91sVyPATw=
golang.org/x/exp v0.0.0-20241004190924-225e2abe05e6/go.mod h1:NQtJDoLvd6faHhE7m4T/1IY708gDefGGjR/iUW8yQQ8=
golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
golang.org/x/mod v0.0.0-20190513183733-4bf6d317e70e/go.mod h1:mXi4GBBbnImb6dmsKGUJ2LatrhH/nqhxcFungHvyanc=
golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg=
Expand Down
15 changes: 2 additions & 13 deletions internal/testutils/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"encoding/json"
"fmt"
"github.com/go-jet/jet/v2/internal/jet"
"github.com/go-jet/jet/v2/internal/utils/throw"
"github.com/go-jet/jet/v2/qrm"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -103,21 +102,11 @@ func AssertJSON(t *testing.T, data interface{}, expectedJSON string) {
require.Equal(t, dataJson, expectedJSON)
}

// SaveJSONFile saves v as json at testRelativePath
func SaveJSONFile(v interface{}, testRelativePath string) {
jsonText, _ := json.MarshalIndent(v, "", "\t")

filePath := getFullPath(testRelativePath)
err := os.WriteFile(filePath, jsonText, 0644)

throw.OnError(err)
}

// AssertJSONFile check if data json representation is the same as json at testRelativePath
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" {
Expand Down Expand Up @@ -244,7 +233,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)

Expand Down
10 changes: 7 additions & 3 deletions internal/utils/filesys/filesys.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package filesys

import (
"errors"
"fmt"
"go/format"
"os"
Expand All @@ -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
Expand All @@ -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)
}

Expand All @@ -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)
Expand Down
47 changes: 33 additions & 14 deletions qrm/internal/null_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"database/sql/driver"
"fmt"
"github.com/go-jet/jet/v2/internal/utils/min"
"golang.org/x/exp/constraints"
"reflect"
"strconv"
"time"
Expand Down Expand Up @@ -111,40 +112,58 @@ type NullUInt64 struct {
Valid bool
}

// validatePositive validates that the numeric value of any time is a positive entity, safely casts to unsigned type
func validatePositive[T constraints.Integer](n T) bool {
return n > 0
}

// Scan implements the Scanner interface.
func (n *NullUInt64) Scan(value interface{}) error {
var badState = fmt.Errorf("cannot cast a negative value to an unsigned value, buffer overflow error")
handleSignedInts := func(isPositive bool, SetValue func()) error {
if isPositive {
SetValue()
return nil
}
return badState
}
var stringValue string
switch v := value.(type) {
case nil:
n.Valid = false
return nil
case int64:
n.UInt64, n.Valid = uint64(v), true
return nil
return handleSignedInts(validatePositive(v), func() {
n.UInt64, n.Valid = uint64(v), true // #nosec G115
})
case int32:
return handleSignedInts(validatePositive(v), func() {
n.UInt64, n.Valid = uint64(v), true // #nosec G115
})
case int16:
return handleSignedInts(validatePositive(v), func() {
n.UInt64, n.Valid = uint64(v), true // #nosec G115
})
case int8:
return handleSignedInts(validatePositive(v), func() {
n.UInt64, n.Valid = uint64(v), true // #nosec G115
})
case int:
return handleSignedInts(validatePositive(v), func() {
n.UInt64, n.Valid = uint64(v), true // #nosec G115
})
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
Expand Down
38 changes: 38 additions & 0 deletions qrm/internal/null_types_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package internal

import (
"errors"
"fmt"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"testing"
"time"
Expand Down Expand Up @@ -53,6 +55,7 @@ func TestNullTime(t *testing.T) {

func TestNullUInt64(t *testing.T) {
var nullUInt64 NullUInt64
var bufferOverflowError = errors.New("cannot cast a negative value to an unsigned value, buffer overflow error")

require.NoError(t, nullUInt64.Scan(nil))
require.Equal(t, nullUInt64.Valid, false)
Expand All @@ -62,11 +65,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()
Expand All @@ -88,4 +101,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.Equal(t, err.Error(), bufferOverflowError.Error())

//Validate negative use cases
err = nullUInt64.Scan(-5)
assert.NotNil(t, err)
assert.Equal(t, err.Error(), bufferOverflowError.Error())

//Validate negative use cases
err = nullUInt64.Scan(int32(-5))
assert.NotNil(t, err)
assert.Equal(t, err.Error(), bufferOverflowError.Error())

//Validate negative use cases
err = nullUInt64.Scan(int16(-5))
assert.NotNil(t, err)
assert.Equal(t, err.Error(), bufferOverflowError.Error())

//Validate negative use cases
err = nullUInt64.Scan(int8(-5))
assert.NotNil(t, err)
assert.Equal(t, err.Error(), bufferOverflowError.Error())
}
10 changes: 7 additions & 3 deletions tests/init/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"database/sql"
"errors"
"flag"
"fmt"
"github.com/go-jet/jet/v2/generator/mysql"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions tests/internal/utils/file/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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))
}

0 comments on commit 85e4ad9

Please sign in to comment.