Skip to content

Commit

Permalink
EVM-439 Custom Lint rule to check file permissions (0xPolygon#1271)
Browse files Browse the repository at this point in the history
* New lint rule

* Remove test file

* change uts

* Add unit tests for custom lint rules

* Comments fix
  • Loading branch information
goran-ethernal authored Mar 7, 2023
1 parent 87936ea commit cd3b34d
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 8 deletions.
7 changes: 7 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,20 @@ linters:
- dupl # Code clone detection
- errname # Checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error
- errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13
- gocritic

linters-settings:
gofmt:
simplify: true
goconst:
min-len: 3
min-occurrences: 3
gocritic:
enabled-checks:
- ruleguard
settings:
ruleguard:
rules: "./gorules/rules.go"

issues:
new-from-rev: origin/develop # report only new issues with reference to develop branch
Expand Down
8 changes: 4 additions & 4 deletions consensus/ibft/fork/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func Test_loadSnapshotMetadata(t *testing.T) {

dirPath := createTestTempDirectory(t)
filePath := path.Join(dirPath, "test.dat")
assert.NoError(t, os.WriteFile(filePath, fileData, os.ModePerm))
assert.NoError(t, os.WriteFile(filePath, fileData, 0775))

res, err := loadSnapshotMetadata(filePath)

Expand Down Expand Up @@ -124,7 +124,7 @@ func Test_loadSnapshots(t *testing.T) {

dirPath := createTestTempDirectory(t)
filePath := path.Join(dirPath, "test.dat")
assert.NoError(t, os.WriteFile(filePath, fileData, os.ModePerm))
assert.NoError(t, os.WriteFile(filePath, fileData, 0775))

res, err := loadSnapshots(filePath)

Expand Down Expand Up @@ -166,7 +166,7 @@ func Test_readDataStore(t *testing.T) {

assert.NoError(
t,
os.WriteFile(filePath, []byte("hello: world"), os.ModePerm),
os.WriteFile(filePath, []byte("hello: world"), 0775),
)

data := map[string]interface{}{}
Expand All @@ -186,7 +186,7 @@ func Test_readDataStore(t *testing.T) {

assert.NoError(
t,
os.WriteFile(filePath, []byte(sampleJSON), os.ModePerm),
os.WriteFile(filePath, []byte(sampleJSON), 0775),
)

data := map[string]interface{}{}
Expand Down
4 changes: 2 additions & 2 deletions consensus/ibft/fork/storewrapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,14 +247,14 @@ func TestSnapshotValidatorStoreWrapper(t *testing.T) {
if len(test.storedSnapshotMetadata) != 0 {
assert.NoError(
t,
os.WriteFile(path.Join(dirPath, snapshotMetadataFilename), []byte(test.storedSnapshotMetadata), os.ModePerm),
os.WriteFile(path.Join(dirPath, snapshotMetadataFilename), []byte(test.storedSnapshotMetadata), 0775),
)
}

if len(test.storedSnapshots) != 0 {
assert.NoError(
t,
os.WriteFile(path.Join(dirPath, snapshotSnapshotsFilename), []byte(test.storedSnapshots), os.ModePerm),
os.WriteFile(path.Join(dirPath, snapshotSnapshotsFilename), []byte(test.storedSnapshots), 0775),
)
}

Expand Down
2 changes: 1 addition & 1 deletion consensus/polybft/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func newTestState(t *testing.T) *State {
t.Helper()

dir := fmt.Sprintf("/tmp/consensus-temp_%v", time.Now().Format(time.RFC3339Nano))
err := os.Mkdir(dir, 0777)
err := os.Mkdir(dir, 0775)

if err != nil {
t.Fatal(err)
Expand Down
9 changes: 8 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ require (
github.com/valyala/fastjson v1.6.3 // indirect
go.uber.org/zap v1.22.0 // indirect
golang.org/x/sys v0.4.0 // indirect
golang.org/x/tools v0.1.12 // indirect
golang.org/x/tools v0.1.12
google.golang.org/genproto v0.0.0-20221010155953-15ba04fc1c0e
gopkg.in/yaml.v3 v3.0.1
lukechampine.com/blake3 v1.1.7 // indirect
Expand All @@ -68,6 +68,8 @@ require (

require (
github.com/dave/jennifer v1.6.0
github.com/quasilyte/go-ruleguard v0.3.19
github.com/quasilyte/go-ruleguard/dsl v0.3.22
golang.org/x/sync v0.0.0-20220929204114-8fcdb60fdcc0
gopkg.in/DataDog/dd-trace-go.v1 v1.43.1
pgregory.net/rapid v0.5.5
Expand Down Expand Up @@ -108,6 +110,8 @@ require (
github.com/flynn/noise v1.0.0 // indirect
github.com/francoispqt/gojay v1.2.13 // indirect
github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0 // indirect
github.com/go-toolsmith/astcopy v1.0.2 // indirect
github.com/go-toolsmith/astequal v1.0.3 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
Expand Down Expand Up @@ -195,6 +199,8 @@ require (
github.com/prometheus/client_model v0.2.0 // indirect
github.com/prometheus/common v0.37.0 // indirect
github.com/prometheus/procfs v0.8.0 // indirect
github.com/quasilyte/gogrep v0.5.0 // indirect
github.com/quasilyte/stdinfo v0.0.0-20220114132959-f7386bf02567 // indirect
github.com/raulk/go-watchdog v1.3.0 // indirect
github.com/ryanuber/go-glob v1.0.0 // indirect
github.com/sirupsen/logrus v1.8.1 // indirect
Expand All @@ -212,6 +218,7 @@ require (
go.uber.org/multierr v1.8.0 // indirect
go4.org/intern v0.0.0-20211027215823-ae77deb06f29 // indirect
go4.org/unsafe/assume-no-moving-gc v0.0.0-20220617031537-928513b29760 // indirect
golang.org/x/exp/typeparams v0.0.0-20221002003631-540bb7301a08 // indirect
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
golang.org/x/net v0.0.0-20221012135044-0b7e1fb9d458 // indirect
golang.org/x/oauth2 v0.0.0-20221006150949-b44042a4b9c1 // indirect
Expand Down
19 changes: 19 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,13 @@ github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/me
github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0 h1:p104kn46Q8WdvHunIJ9dAyjPVtrBPhSr3KT2yUst43I=
github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE=
github.com/go-test/deep v1.0.2 h1:onZX1rnHT3Wv6cqNgYyFOOlgVKJrksuCMCRvJStbMYw=
github.com/go-toolsmith/astcopy v1.0.2 h1:YnWf5Rnh1hUudj11kei53kI57quN/VH6Hp1n+erozn0=
github.com/go-toolsmith/astcopy v1.0.2/go.mod h1:4TcEdbElGc9twQEYpVo/aieIXfHhiuLh4aLAck6dO7Y=
github.com/go-toolsmith/astequal v1.0.2/go.mod h1:9Ai4UglvtR+4up+bAD4+hCj7iTo4m/OXVTSLnCyTAx4=
github.com/go-toolsmith/astequal v1.0.3 h1:+LVdyRatFS+XO78SGV4I3TCEA0AC7fKEGma+fH+674o=
github.com/go-toolsmith/astequal v1.0.3/go.mod h1:9Ai4UglvtR+4up+bAD4+hCj7iTo4m/OXVTSLnCyTAx4=
github.com/go-toolsmith/strparse v1.0.0 h1:Vcw78DnpCAKlM20kSbAyO4mPfJn/lyYA4BJUDxe2Jb4=
github.com/go-toolsmith/strparse v1.0.0/go.mod h1:YI2nUKP9YGZnL/L1/DLFBfixrcjslWct4wyljWhSRy8=
github.com/godbus/dbus/v5 v5.0.3/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk=
Expand Down Expand Up @@ -271,6 +278,7 @@ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-github v17.0.0+incompatible/go.mod h1:zLgOLi98H3fifZn+44m+umXrS52loVEgC2AApnigrVQ=
Expand Down Expand Up @@ -641,6 +649,14 @@ github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1
github.com/prometheus/procfs v0.7.3/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA=
github.com/prometheus/procfs v0.8.0 h1:ODq8ZFEaYeCaZOJlZZdJA2AbQR98dSHSM1KW/You5mo=
github.com/prometheus/procfs v0.8.0/go.mod h1:z7EfXMXOkbkqb9IINtpCn86r/to3BnA0uaxHdg830/4=
github.com/quasilyte/go-ruleguard v0.3.19 h1:tfMnabXle/HzOb5Xe9CUZYWXKfkS1KwRmZyPmD9nVcc=
github.com/quasilyte/go-ruleguard v0.3.19/go.mod h1:lHSn69Scl48I7Gt9cX3VrbsZYvYiBYszZOZW4A+oTEw=
github.com/quasilyte/go-ruleguard/dsl v0.3.22 h1:wd8zkOhSNr+I+8Qeciml08ivDt1pSXe60+5DqOpCjPE=
github.com/quasilyte/go-ruleguard/dsl v0.3.22/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
github.com/quasilyte/gogrep v0.5.0 h1:eTKODPXbI8ffJMN+W2aE0+oL0z/nh8/5eNdiO34SOAo=
github.com/quasilyte/gogrep v0.5.0/go.mod h1:Cm9lpz9NZjEoL1tgZ2OgeUKPIxL1meE7eo60Z6Sk+Ng=
github.com/quasilyte/stdinfo v0.0.0-20220114132959-f7386bf02567 h1:M8mH9eK4OUR4lu7Gd+PU1fV2/qnDNfzT635KRSObncs=
github.com/quasilyte/stdinfo v0.0.0-20220114132959-f7386bf02567/go.mod h1:DWNGW8A4Y+GyBgPuaQJuWiy0XYftx4Xm/y5Jqk9I6VQ=
github.com/raulk/go-watchdog v1.3.0 h1:oUmdlHxdkXRJlwfG0O9omj8ukerm8MEQavSiDTEtBsk=
github.com/raulk/go-watchdog v1.3.0/go.mod h1:fIvOnLbF0b0ZwkB9YU4mOW9Did//4vPZtDqv66NfsMU=
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
Expand Down Expand Up @@ -804,6 +820,9 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
golang.org/x/exp/typeparams v0.0.0-20220428152302-39d4317da171/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk=
golang.org/x/exp/typeparams v0.0.0-20221002003631-540bb7301a08 h1:VpoGhesgULkabDHoDFGayS1wnkasmT95Jq2xZDwN45Q=
golang.org/x/exp/typeparams v0.0.0-20221002003631-540bb7301a08/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk=
golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
golang.org/x/lint v0.0.0-20180702182130-06c8688daad7/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
Expand Down
15 changes: 15 additions & 0 deletions gorules/rules.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package gorules

import (
"github.com/quasilyte/go-ruleguard/dsl"
)

func OsFilePermissionRule(m dsl.Matcher) {
m.Match(`os.$name($file, $number, 0777)`).Report("os.$name called with file mode 0777")
m.Match(`os.$name($file, 0777)`).Report("os.$name called with file mode 0777")
m.Match(`os.$name(0777)`).Report("os.$name called with file mode 0777")

m.Match(`os.$name($file, $number, os.ModePerm)`).Report("os.$name called with file mode os.ModePerm (0777)")
m.Match(`os.$name($file, os.ModePerm)`).Report("os.$name called with file mode os.ModePerm (0777)")
m.Match(`os.$name(os.ModePerm)`).Report("os.$name called with file mode os.ModePerm (0777)")
}
21 changes: 21 additions & 0 deletions gorules/rules_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package gorules

import (
"testing"

"github.com/quasilyte/go-ruleguard/analyzer"
"golang.org/x/tools/go/analysis/analysistest"
)

func TestRules(t *testing.T) {
t.Parallel()

testdata := analysistest.TestData()

rules := "rules.go" // if we are to separate rules in different files, here we would write them (e.g., rule1.go,rule2.go,rule3.go)
if err := analyzer.Analyzer.Flags.Set("rules", rules); err != nil {
t.Fatalf("set rules flag: %v", err)
}

analysistest.Run(t, testdata, analyzer.Analyzer, "./...")
}
25 changes: 25 additions & 0 deletions gorules/testdata/rules_test_data.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package target

import "os"

func testLintError() {
os.Mkdir("test", 0777) // want `OsFilePermissionRule: os.Mkdir called with file mode`
os.Mkdir("test", os.ModePerm) // want `OsFilePermissionRule: os.Mkdir called with file mode`
os.MkdirAll("test", 0777) // want `OsFilePermissionRule: os.MkdirAll called with file mode`
os.MkdirAll("test", os.ModePerm) // want `OsFilePermissionRule: os.MkdirAll called with file mode`
os.Chmod("test", 0777) // want `OsFilePermissionRule: os.Chmod called with file mode`
os.Chmod("test", os.ModePerm) // want `OsFilePermissionRule: os.Chmod called with file mode`
os.OpenFile("test", 0, 0777) // want `OsFilePermissionRule: os.OpenFile called with file mode`
os.OpenFile("test", 0, os.ModePerm) // want `OsFilePermissionRule: os.OpenFile called with file mode`
}

func testNoLintError() {
os.Mkdir("test", 67108864)
os.Mkdir("test", os.ModeDevice)
os.MkdirAll("test", 67108864)
os.MkdirAll("test", os.ModeDevice)
os.Chmod("test", 67108864)
os.Chmod("test", os.ModeDevice)
os.OpenFile("test", 0, 67108864)
os.OpenFile("test", 0, os.ModeDevice)
}

0 comments on commit cd3b34d

Please sign in to comment.