Skip to content

Commit

Permalink
Apply security-related changes, update codecov config
Browse files Browse the repository at this point in the history
Am trying out github.com/securego/gosec. Ran v2.10.0 on this project and
fixed the issues.

Change codecov config to disable status checks. I would prefer not to
have the red X next to the commit hash when the code coverage takes a
dip. I evaluate code coverage on a case-by-case basis. In this case,
code changes are mostly for closing files, which can be difficult to
test anyways. It's okay if coverage dips every now and then as long as
it's not consistently going downwards.
  • Loading branch information
rafaelespinoza committed Mar 5, 2022
1 parent e882801 commit 8995fc8
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 19 deletions.
32 changes: 32 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
GO ?= go
BIN_DIR=bin
PKG_IMPORT_PATH=github.com/rafaelespinoza/godfish
GOSEC ?= gosec

CORE_SRC_PKG_PATHS=$(PKG_IMPORT_PATH) $(PKG_IMPORT_PATH)/internal/...
CASSANDRA_PATH=$(PKG_IMPORT_PATH)/drivers/cassandra
Expand Down Expand Up @@ -29,6 +30,20 @@ clean:
_mkdir:
mkdir -pv $(BIN_DIR)

# Run a security scanner over the source code. This Makefile won't install the
# scanner binary for you, so check out the gosec README for instructions:
# https://github.com/securego/gosec
#
# If necessary, specify the path to the built binary with the GOSEC env var.
#
# Also note, the package paths (last positional input to gosec command) should
# be a "relative" package path. That is, starting with a dot.
gosec:
$(GOSEC) $(ARGS) . ./internal/...

#
# Cassandra
#
build-cassandra: BIN=$(BIN_DIR)/godfish_cassandra
build-cassandra: _mkdir
$(GO) build -o $(BIN) -v \
Expand All @@ -40,7 +55,12 @@ test-cassandra:
$(GO) test $(ARGS) $(CASSANDRA_PATH)/...
vet-cassandra: vet
$(GO) vet $(ARGS) $(CASSANDRA_PATH)/...
gosec-cassandra: gosec
$(GOSEC) $(ARGS) ./drivers/cassandra/...

#
# Postgres
#
build-postgres: BIN=$(BIN_DIR)/godfish_postgres
build-postgres: _mkdir
$(GO) build -o $(BIN) -v \
Expand All @@ -52,7 +72,12 @@ test-postgres:
$(GO) test $(ARGS) $(POSTGRES_PATH)/...
vet-postgres: vet
$(GO) vet $(ARGS) $(POSTGRES_PATH)/...
gosec-postgres: gosec
$(GOSEC) $(ARGS) ./drivers/postgres/...

#
# MySQL
#
build-mysql: BIN=$(BIN_DIR)/godfish_mysql
build-mysql: _mkdir
$(GO) build -o $(BIN) -v \
Expand All @@ -64,7 +89,12 @@ test-mysql:
$(GO) test $(ARGS) $(MYSQL_PATH)/...
vet-mysql: vet
$(GO) vet $(ARGS) $(MYSQL_PATH)/...
gosec-mysql: gosec
$(GOSEC) $(ARGS) ./drivers/mysql/...

#
# SQLite3
#
build-sqlite3: BIN=$(BIN_DIR)/godfish_sqlite3
build-sqlite3: _mkdir
CGO_ENABLED=1 $(GO) build -o $(BIN) -v \
Expand All @@ -76,3 +106,5 @@ test-sqlite3:
$(GO) test $(ARGS) $(SQLITE3_PATH)/...
vet-sqlite3: vet
$(GO) vet $(ARGS) $(SQLITE3_PATH)/...
gosec-sqlite3: gosec
$(GOSEC) $(ARGS) ./drivers/sqlite3/...
5 changes: 5 additions & 0 deletions codecov.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
coverage:
status:
project: off
patch: off

parsers:
go:
# Codecov has the concept of partial hits, which have a lower code coverage
Expand Down
8 changes: 6 additions & 2 deletions godfish.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func figureOutBasename(directoryPath string, direction internal.Direction, versi
// should be relative to the current working directory.
func runMigration(driver Driver, pathToFile string, mig internal.Migration) (err error) {
var data []byte
if data, err = os.ReadFile(pathToFile); err != nil {
if data, err = os.ReadFile(filepath.Clean(pathToFile)); err != nil {
return
}
gerund := "migrating"
Expand Down Expand Up @@ -334,7 +334,11 @@ func (m *migrationFinder) available() (out []internal.Migration, err error) {
}
return
}
defer fileDir.Close()
defer func() {
if ierr := fileDir.Close(); ierr != nil {
fmt.Fprintln(os.Stderr, ierr)
}
}()
if filenames, err = fileDir.Readdirnames(0); err != nil {
return
}
Expand Down
12 changes: 6 additions & 6 deletions godfish_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"database/sql"
"encoding/json"
"os"
"path/filepath"
"strings"
"testing"

Expand Down Expand Up @@ -131,7 +132,7 @@ func TestInit(t *testing.T) {
var err error
testOutputDir := makeTestDir(t, "")

pathToFile := testOutputDir + "/config.json"
pathToFile := filepath.Clean(filepath.Join(testOutputDir, "config.json"))

// setup: file should not exist at first
if _, err = os.Stat(pathToFile); !os.IsNotExist(err) {
Expand All @@ -154,11 +155,10 @@ func TestInit(t *testing.T) {
if data, err := json.MarshalIndent(conf, "", "\t"); err != nil {
t.Fatal(err)
} else {
os.WriteFile(
pathToFile,
append(data, byte('\n')),
os.FileMode(0644),
)
err = os.WriteFile(pathToFile, append(data, byte('\n')), os.FileMode(0640))
if err != nil {
t.Fatal(err)
}
}
if err := godfish.Init(pathToFile); err != nil {
t.Fatal(err)
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"flag"
"fmt"
"os"
"path/filepath"
"strings"

"github.com/rafaelespinoza/alf"
Expand Down Expand Up @@ -106,7 +107,7 @@ Examples:
// Look for config file and if present, merge those values with
// input flag values.
var conf internal.Config
if data, ierr := os.ReadFile(pathToConfig); ierr != nil {
if data, ierr := os.ReadFile(filepath.Clean(pathToConfig)); ierr != nil {
// probably no config file present, rely on arguments instead.
} else if ierr = json.Unmarshal(data, &conf); ierr != nil {
return ierr
Expand Down
7 changes: 4 additions & 3 deletions internal/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func (m *MigrationParams) GenerateFiles() (err error) {
return
}
fmt.Fprintln(os.Stderr, "created forward file:", forwardFile.Name())
defer forwardFile.Close()
defer func() { _ = forwardFile.Close() }()

if !m.Reversible {
fmt.Fprintln(os.Stderr, "migration marked irreversible, did not create reverse file")
Expand All @@ -130,12 +130,13 @@ func (m *MigrationParams) GenerateFiles() (err error) {
return
}
fmt.Fprintln(os.Stderr, "created reverse file:", reverseFile.Name())
defer reverseFile.Close()
defer func() { _ = reverseFile.Close() }()
return
}

func newMigrationFile(m Migration, baseDir string) (*os.File, error) {
return os.Create(baseDir + "/" + MakeMigrationFilename(m))
name := filepath.Join(baseDir, MakeMigrationFilename(m))
return os.Create(filepath.Clean(name))
}

// MakeMigrationFilename converts a Migration m to a filename.
Expand Down
2 changes: 1 addition & 1 deletion internal/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func TestParseMigration(t *testing.T) {

func TestMigrationParams(t *testing.T) {
testOutputDir := baseTestOutputDir + "/" + t.Name()
if err := os.MkdirAll(testOutputDir, 0755); err != nil {
if err := os.MkdirAll(testOutputDir, 0750); err != nil {
t.Fatal(err)
}

Expand Down
15 changes: 9 additions & 6 deletions internal/test/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package test
import (
"fmt"
"os"
"path/filepath"
"strconv"
"testing"

Expand Down Expand Up @@ -69,8 +70,8 @@ const (

// setup prepares state before running a test.
func setup(driver godfish.Driver, testName string, stubs []testDriverStub, migrateTo string) (path string, err error) {
path = "/tmp/godfish_test/drivers/" + driver.Name() + "/" + testName
if err = os.MkdirAll(path, 0755); err != nil {
path = filepath.Join("/tmp/godfish_test/drivers/", driver.Name(), testName)
if err = os.MkdirAll(path, 0750); err != nil {
return
}
if err = generateMigrationFiles(path, stubs); err != nil {
Expand Down Expand Up @@ -108,8 +109,10 @@ func teardown(driver godfish.Driver, path string, tablesToDrop ...string) {
if err = driver.Execute(truncate); err != nil {
panic(err)
}
os.RemoveAll(path)
driver.Close()
_ = os.RemoveAll(path)
if err := driver.Close(); err != nil {
panic(err)
}
}

func formattedTime(v string) internal.Version {
Expand Down Expand Up @@ -162,11 +165,11 @@ func generateMigrationFiles(pathToTestDir string, stubs []testDriverStub) error
"%s/%s-%s-%s.sql",
pathToTestDir, mig.Indirection().Label, mig.Version().String(), mig.Label(),
)
file, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE, 0755)
file, err := os.OpenFile(filepath.Clean(filename), os.O_RDWR|os.O_CREATE, 0600)
if err != nil {
return err
}
defer file.Close()
defer func() { _ = file.Close() }()

// this only works if the slice we're iterating through has
// migrations where each Direction is in the order:
Expand Down

0 comments on commit 8995fc8

Please sign in to comment.