Skip to content

N
nebula
Commit 6d57ace1 authored by smartcontracts's avatar smartcontracts Committed by GitHub
Browse files
Options

maint(ct): replace natspec semver check with semgrep (#12625) 

Replaces the golang package for natspec/semver matching with a
semgrep rule. Code reduction is good.
parent 34f61011
Hide whitespace changes
Inline Side-by-side
Showing with 60 additions and 356 deletions
.circleci/config.yml
View file @ 6d57ace1
...@@ -684,8 +684,6 @@ jobs: ...@@ -684,8 +684,6 @@ jobs:
command: semver-lock command: semver-lock
- run-contracts-check: - run-contracts-check:
command: semver-diff-check-no-build command: semver-diff-check-no-build
- run-contracts-check:
command: semver-natspec-check-no-build
- run-contracts-check: - run-contracts-check:
command: validate-deploy-configs command: validate-deploy-configs
- run-contracts-check: - run-contracts-check:
...@@ -864,7 +862,6 @@ jobs: ...@@ -864,7 +862,6 @@ jobs:
op-service op-service
op-supervisor op-supervisor
op-deployer op-deployer
packages/contracts-bedrock/scripts/checks/semver-natspec
) )
formatted_packages="" formatted_packages=""
for package in "${packages[@]}"; do for package in "${packages[@]}"; do
......
packages/contracts-bedrock/justfile
View file @ 6d57ace1
...@@ -155,14 +155,6 @@ semver-diff-check-no-build: ...@@ -155,14 +155,6 @@ semver-diff-check-no-build:
# Checks that any contracts with a modified semver lock also have a modified semver version. # Checks that any contracts with a modified semver lock also have a modified semver version.
semver-diff-check: build semver-diff-check-no-build semver-diff-check: build semver-diff-check-no-build
# Checks that semver natspec is equal to the actual semver version.
# Does not build contracts.
semver-natspec-check-no-build:
go run ./scripts/checks/semver-natspec
# Checks that semver natspec is equal to the actual semver version.
semver-natspec-check: build semver-natspec-check-no-build
# Checks that the semgrep tests are valid. # Checks that the semgrep tests are valid.
semgrep-test-validity-check: semgrep-test-validity-check:
forge fmt ../../semgrep/sol-rules.t.sol --check forge fmt ../../semgrep/sol-rules.t.sol --check
...@@ -206,7 +198,6 @@ check: ...@@ -206,7 +198,6 @@ check:
snapshots-check-no-build \ snapshots-check-no-build \
lint-check \ lint-check \
semver-diff-check-no-build \ semver-diff-check-no-build \
semver-natspec-check-no-build \
validate-deploy-configs \ validate-deploy-configs \
validate-spacers-no-build \ validate-spacers-no-build \
interfaces-check-no-build interfaces-check-no-build
......
packages/contracts-bedrock/scripts/checks/semver-natspec/main.go deleted 100644 → 0
View file @ 34f61011
package main
import (
"bufio"
"bytes"
"encoding/json"
"fmt"
"io"
"os"
"path/filepath"
"regexp"
"runtime"
"strings"
"sync"
"sync/atomic"
)
type ArtifactsWrapper struct {
RawMetadata string `json:"rawMetadata"`
}
type Artifacts struct {
Output struct {
Devdoc struct {
StateVariables struct {
Version struct {
Semver string `json:"custom:semver"`
} `json:"version"`
} `json:"stateVariables,omitempty"`
Methods struct {
Version struct {
Semver string `json:"custom:semver"`
} `json:"version()"`
} `json:"methods,omitempty"`
} `json:"devdoc"`
} `json:"output"`
}
var ConstantVersionPattern = regexp.MustCompile(`string.*constant.*version\s+=\s+"([^"]+)";`)
var FunctionVersionPattern = regexp.MustCompile(`^\s+return\s+"((?P<major>0|[1-9]\d*)\.(?P<minor>0|[1-9]\d*)\.(?P<patch>0|[1-9]\d*)(?:-(?P<prerelease>(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+(?P<buildmetadata>[0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?)";$`)
var InteropVersionPattern = regexp.MustCompile(`^\s+return\s+string\.concat\(super\.version\(\), "((.*)\+interop(.*)?)"\);`)
func main() {
if err := run(); err != nil {
writeStderr("an error occurred: %v", err)
os.Exit(1)
}
}
func writeStderr(msg string, args ...any) {
_, _ = fmt.Fprintf(os.Stderr, msg+"\n", args...)
}
func run() error {
cwd, err := os.Getwd()
if err != nil {
return fmt.Errorf("failed to get current working directory: %w", err)
}
writeStderr("working directory: %s", cwd)
artifactsDir := filepath.Join(cwd, "forge-artifacts")
srcDir := filepath.Join(cwd, "src")
artifactFiles, err := glob(artifactsDir, ".json")
if err != nil {
return fmt.Errorf("failed to get artifact files: %w", err)
}
contractFiles, err := glob(srcDir, ".sol")
if err != nil {
return fmt.Errorf("failed to get contract files: %w", err)
}
var hasErr int32
var outMtx sync.Mutex
fail := func(msg string, args ...any) {
outMtx.Lock()
writeStderr("❌ "+msg, args...)
outMtx.Unlock()
atomic.StoreInt32(&hasErr, 1)
}
sem := make(chan struct{}, runtime.NumCPU())
for contractName, artifactPath := range artifactFiles {
contractName := contractName
artifactPath := artifactPath
sem <- struct{}{}
go func() {
defer func() {
<-sem
}()
af, err := os.Open(artifactPath)
if err != nil {
fail("%s: failed to open contract artifact: %v", contractName, err)
return
}
defer af.Close()
var wrapper ArtifactsWrapper
if err := json.NewDecoder(af).Decode(&wrapper); err != nil {
fail("%s: failed to parse artifact file: %v", contractName, err)
return
}
if wrapper.RawMetadata == "" {
return
}
var artifactData Artifacts
if err := json.Unmarshal([]byte(wrapper.RawMetadata), &artifactData); err != nil {
fail("%s: failed to unwrap artifact metadata: %v", contractName, err)
return
}
artifactVersion := artifactData.Output.Devdoc.StateVariables.Version.Semver
isConstant := true
if artifactData.Output.Devdoc.StateVariables.Version.Semver == "" {
artifactVersion = artifactData.Output.Devdoc.Methods.Version.Semver
isConstant = false
}
if artifactVersion == "" {
return
}
// Skip mock contracts
if strings.HasPrefix(contractName, "Mock") {
return
}
contractPath := contractFiles[contractName]
if contractPath == "" {
fail("%s: Source file not found (For test mock contracts, prefix the name with 'Mock' to ignore this warning)", contractName)
return
}
cf, err := os.Open(contractPath)
if err != nil {
fail("%s: failed to open contract source: %v", contractName, err)
return
}
defer cf.Close()
sourceData, err := io.ReadAll(cf)
if err != nil {
fail("%s: failed to read contract source: %v", contractName, err)
return
}
var sourceVersion string
if isConstant {
sourceVersion = findLine(sourceData, ConstantVersionPattern)
} else {
sourceVersion = findLine(sourceData, FunctionVersionPattern)
}
// Need to define a special case for interop contracts since they technically
// use an invalid semver format. Checking for sourceVersion == "" allows the
// team to update the format to a valid semver format in the future without
// needing to change this program.
if sourceVersion == "" && strings.HasSuffix(contractName, "Interop") {
sourceVersion = findLine(sourceData, InteropVersionPattern)
}
if sourceVersion == "" {
fail("%s: version not found in source", contractName)
return
}
if sourceVersion != artifactVersion {
fail("%s: version mismatch: source=%s, artifact=%s", contractName, sourceVersion, artifactVersion)
return
}
_, _ = fmt.Fprintf(os.Stderr, "✅ %s: code: %s, artifact: %s\n", contractName, sourceVersion, artifactVersion)
}()
}
for i := 0; i < cap(sem); i++ {
sem <- struct{}{}
}
if atomic.LoadInt32(&hasErr) == 1 {
return fmt.Errorf("semver check failed, see logs above")
}
return nil
}
func glob(dir string, ext string) (map[string]string, error) {
out := make(map[string]string)
err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
if !info.IsDir() && filepath.Ext(path) == ext {
out[strings.TrimSuffix(filepath.Base(path), ext)] = path
}
return nil
})
if err != nil {
return nil, fmt.Errorf("failed to walk directory: %w", err)
}
return out, nil
}
func findLine(in []byte, pattern *regexp.Regexp) string {
scanner := bufio.NewScanner(bytes.NewReader(in))
for scanner.Scan() {
match := pattern.FindStringSubmatch(scanner.Text())
if len(match) > 0 {
return match[1]
}
}
return ""
}
packages/contracts-bedrock/scripts/checks/semver-natspec/main_test.go deleted 100644 → 0
View file @ 34f61011
package main
import (
"regexp"
"testing"
"github.com/stretchr/testify/require"
)
func TestRegexes(t *testing.T) {
t.Run("ConstantVersionPattern", func(t *testing.T) {
testRegex(t, ConstantVersionPattern, []regexTest{
{
name: "constant version",
input: `string constant version = "1.2.3";`,
capture: "1.2.3",
},
{
name: "constant version with weird spaces",
input: ` string constant version = "1.2.3";`,
capture: "1.2.3",
},
{
name: "constant version with visibility",
input: `string public constant version = "1.2.3";`,
capture: "1.2.3",
},
{
name: "different variable name",
input: `string constant VERSION = "1.2.3";`,
capture: "",
},
{
name: "different type",
input: `uint constant version = 1;`,
capture: "",
},
{
name: "not constant",
input: `string version = "1.2.3";`,
capture: "",
},
{
name: "unterminated",
input: `string constant version = "1.2.3"`,
capture: "",
},
})
})
t.Run("FunctionVersionPattern", func(t *testing.T) {
testRegex(t, FunctionVersionPattern, []regexTest{
{
name: "function version",
input: ` return "1.2.3";`,
capture: "1.2.3",
},
{
name: "function version with weird spaces",
input: ` return "1.2.3";`,
capture: "1.2.3",
},
{
name: "function version with prerelease",
input: ` return "1.2.3-alpha.1";`,
capture: "1.2.3-alpha.1",
},
{
name: "invalid semver",
input: ` return "1.2.cabdab";`,
capture: "",
},
{
name: "not a return statement",
input: `function foo()`,
capture: "",
},
})
})
t.Run("InteropVersionPattern", func(t *testing.T) {
testRegex(t, InteropVersionPattern, []regexTest{
{
name: "interop version",
input: ` return string.concat(super.version(), "+interop");`,
capture: "+interop",
},
{
name: "interop version but as a valid semver",
input: ` return string.concat(super.version(), "0.0.0+interop");`,
capture: "0.0.0+interop",
},
{
name: "not an interop version",
input: ` return string.concat(super.version(), "hello!");`,
capture: "",
},
{
name: "invalid syntax",
input: ` return string.concat(super.version(), "0.0.0+interop`,
capture: "",
},
{
name: "something else is concatted",
input: ` return string.concat("superduper", "mart");`,
capture: "",
},
})
})
}
type regexTest struct {
name string
input string
capture string
}
func testRegex(t *testing.T, re *regexp.Regexp, tests []regexTest) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
require.Equal(t, test.capture, findLine([]byte(test.input), re))
})
}
}
semgrep/sol-rules.t.sol
View file @ 6d57ace1
...@@ -120,6 +120,40 @@ contract SemgrepTest__sol_safety_expectrevert_no_args { ...@@ -120,6 +120,40 @@ contract SemgrepTest__sol_safety_expectrevert_no_args {
} }
} }
contract SemgrepTest__sol_safety_natspec_semver_match {
// ok: sol-safety-natspec-semver-match
/// @custom:semver 2.8.1-beta.4
string public constant version = "2.8.1-beta.4";
// ok: sol-safety-natspec-semver-match
/// @custom:semver 2.8.1-beta.3
function version() public pure virtual returns (string memory) {
return "2.8.1-beta.3";
}
// ok: sol-safety-natspec-semver-match
/// @custom:semver +interop-beta.1
function version() public pure override returns (string memory) {
return string.concat(super.version(), "+interop-beta.1");
}
// ruleid: sol-safety-natspec-semver-match
/// @custom:semver 2.8.1-beta.5
string public constant version = "2.8.1-beta.4";
// ruleid: sol-safety-natspec-semver-match
/// @custom:semver 2.8.1-beta.4
function version() public pure virtual returns (string memory) {
return "2.8.1-beta.3";
}
// ruleid: sol-safety-natspec-semver-match
/// @custom:semver +interop-beta.2
function version() public pure override returns (string memory) {
return string.concat(super.version(), "+interop-beta.1");
}
}
contract SemgrepTest__sol_style_input_arg_fmt { contract SemgrepTest__sol_style_input_arg_fmt {
// ok: sol-style-input-arg-fmt // ok: sol-style-input-arg-fmt
event Test(address indexed src, address indexed guy, uint256 wad); event Test(address indexed src, address indexed guy, uint256 wad);
......
semgrep/sol-rules.yaml
View file @ 6d57ace1
...@@ -39,6 +39,32 @@ rules: ...@@ -39,6 +39,32 @@ rules:
exclude: exclude:
- packages/contracts-bedrock/test/dispute/WETH98.t.sol - packages/contracts-bedrock/test/dispute/WETH98.t.sol
- id: sol-safety-natspec-semver-match
languages: [generic]
severity: ERROR
message: Semgrep defined in contract must match natspec $VERSION1 $VERSION2
patterns:
- pattern-either:
- pattern-regex: /// @custom:semver
(?P<VERSION1>[0-9]+\.[0-9]+\.[0-9]+(?:-[a-zA-Z0-9.]+)?)\s+string
public constant version =
"(?P<VERSION2>[0-9]+\.[0-9]+\.[0-9]+(?:-[a-zA-Z0-9.]+)?)";
- pattern-regex: /// @custom:semver
(?P<VERSION1>[0-9]+\.[0-9]+\.[0-9]+(?:-[a-zA-Z0-9.]+)?)\s+function
version\(\) public pure virtual returns \(string memory\)
\{\s+return
"(?P<VERSION2>[0-9]+\.[0-9]+\.[0-9]+(?:-[a-zA-Z0-9.]+)?)";
- pattern-regex: /// @custom:semver (?P<VERSION1>[a-zA-Z0-9.+-]+)\s+function
version\(\) public pure override returns \(string memory\)
\{\s+return string\.concat\(super\.version\(\),
"(?P<VERSION2>[a-zA-Z0-9.+-]+)"\);
- metavariable-comparison:
comparison: $VERSION1 != $VERSION2
metavariable: $VERSION1
paths:
include:
- packages/contracts-bedrock/src
- id: sol-style-input-arg-fmt - id: sol-style-input-arg-fmt
languages: [solidity] languages: [solidity]
severity: ERROR severity: ERROR
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment