chore: Enable gosec and do some internal types modifications (#16)

Most of the errors from gosec stemmed from dangerous type conversions (e.g. from uint32 to uint16). Instead of doing checks everywhere inside the codebase on whether the type semantics are observed (e.g. checking that the uint32 is not larger than math.MaxUint16), I opted to handle this on the types definition and Babylon retrieval layer.

I made gosec exclude the testutil/ and itest/ directories as they are related to tests. Also, for the following gosec errors I put a flag to remove them from the linter:
- Two conversions from uint32 to uint16. Even though I performed the check previously, the static analyzer could not identify it.
- Go file inclusion. This file is not passed from the web, but the user's machine, so it doesn't need further checks.

.github/workflows/ci.yml

@@ -7,14 +7,19 @@ on:
 
 jobs:
   lint_test:
-    uses: babylonlabs-io/.github/.github/workflows/reusable_go_lint_test.yml@v0.1.0
+    uses: babylonlabs-io/.github/.github/workflows/reusable_go_lint_test.yml@v0.6.0
     with:
       run-unit-tests: true
       run-integration-tests: true
       run-lint: true
+      run-build: true
+      run-gosec: true
+      gosec-args: "-exclude-generated -exclude-dir=itest -exclude-dir=testutil ./..."
 
   docker_pipeline:
-    uses: babylonlabs-io/.github/.github/workflows/reusable_docker_pipeline.yml@v0.1.0
+    uses: babylonlabs-io/.github/.github/workflows/reusable_docker_pipeline.yml@v0.6.0
     secrets: inherit
     with:
       publish: false
+      dockerfile: ./Dockerfile
+      repoName: covenant-emulator

.github/workflows/publish.yml

@@ -9,15 +9,20 @@ on:
 
 jobs:
   lint_test:
-    uses: babylonlabs-io/.github/.github/workflows/reusable_go_lint_test.yml@v0.1.0
+    uses: babylonlabs-io/.github/.github/workflows/reusable_go_lint_test.yml@v0.6.0
     with:
       run-unit-tests: true
       run-integration-tests: true
       run-lint: true
+      run-build: true
+      run-gosec: true
+      gosec-args: "-exclude-generated -exclude-dir=itest -exclude-dir=testutil ./..."
 
   docker_pipeline:
     needs: ["lint_test"]
-    uses: babylonlabs-io/.github/.github/workflows/reusable_docker_pipeline.yml@v0.1.0
+    uses: babylonlabs-io/.github/.github/workflows/reusable_docker_pipeline.yml@v0.6.0
     secrets: inherit
     with:
       publish: true
+      dockerfile: ./Dockerfile
+      repoName: covenant-emulator

Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.21.4 as builder
+FROM golang:1.21.4 AS builder
 
 RUN apt-get update && apt-get install -y make git bash gcc curl jq
 

clientcontroller/babylon.go

@@ -3,6 +3,7 @@ package clientcontroller
 import (
 	"context"
 	"fmt"
+	"math"
 	"time"
 
 	sdkmath "cosmossdk.io/math"
@@ -112,6 +113,18 @@ func (bc *BabylonController) QueryStakingParamsByVersion(version uint32) (*types
 		covenantPks = append(covenantPks, covPk)
 	}
 
+	if stakingParamRes.Params.MinStakingTimeBlocks > math.MaxUint16 {
+		return nil, fmt.Errorf("babylon min staking time blocks (%d) is larger than the maximum uint16", stakingParamRes.Params.MinStakingTimeBlocks)
+	}
+	// #nosec G115 -- performed the conversion check above
+	minStakingTimeBlocksUint16 := uint16(stakingParamRes.Params.MinStakingTimeBlocks)
+
+	if stakingParamRes.Params.MaxStakingTimeBlocks > math.MaxUint16 {
+		return nil, fmt.Errorf("babylon max staking time blocks (%d) is larger than the maximum uint16", stakingParamRes.Params.MaxStakingTimeBlocks)
+	}
+	// #nosec G115 -- performed the conversion check above
+	maxStakingTimeBlocksUint16 := uint16(stakingParamRes.Params.MaxStakingTimeBlocks)
+
 	return &types.StakingParams{
 		ComfirmationTimeBlocks:    ckptParamRes.Params.BtcConfirmationDepth,
 		FinalizationTimeoutBlocks: ckptParamRes.Params.CheckpointFinalizationTimeout,
@@ -123,8 +136,8 @@ func (bc *BabylonController) QueryStakingParamsByVersion(version uint32) (*types
 		MinComissionRate:          stakingParamRes.Params.MinCommissionRate,
 		MinUnbondingTime:          stakingParamRes.Params.MinUnbondingTimeBlocks,
 		UnbondingFee:              btcutil.Amount(stakingParamRes.Params.UnbondingFeeSat),
-		MinStakingTime:            uint16(stakingParamRes.Params.MinStakingTimeBlocks),
-		MaxStakingTime:            uint16(stakingParamRes.Params.MaxStakingTimeBlocks),
+		MinStakingTime:            minStakingTimeBlocksUint16,
+		MaxStakingTime:            maxStakingTimeBlocksUint16,
 		MinStakingValue:           btcutil.Amount(stakingParamRes.Params.MinStakingValueSat),
 		MaxStakingValue:           btcutil.Amount(stakingParamRes.Params.MaxStakingValueSat),
 	}, nil
@@ -248,17 +261,25 @@ func DelegationRespToDelegation(del *btcstakingtypes.BTCDelegationResponse) (*ty
 		fpBtcPks = append(fpBtcPks, fp.MustToBTCPK())
 	}
 
+	if del.UnbondingTime > uint32(math.MaxUint16) {
+		return nil, fmt.Errorf("unbonding time should be smaller than max uint16")
+	}
+
+	if del.TotalSat > uint64(math.MaxInt64) {
+		return nil, fmt.Errorf("total sat (%d) is larger than the maximum int64", del.TotalSat)
+	}
+
 	return &types.Delegation{
 		BtcPk:            del.BtcPk.MustToBTCPK(),
 		FpBtcPks:         fpBtcPks,
-		TotalSat:         del.TotalSat,
+		TotalSat:         btcutil.Amount(del.TotalSat),
 		StartHeight:      del.StartHeight,
 		EndHeight:        del.EndHeight,
 		StakingTxHex:     del.StakingTxHex,
 		SlashingTxHex:    del.SlashingTxHex,
 		StakingOutputIdx: del.StakingOutputIdx,
 		CovenantSigs:     covenantSigs,
-		UnbondingTime:    del.UnbondingTime,
+		UnbondingTime:    uint16(del.UnbondingTime),
 		BtcUndelegation:  undelegation,
 	}, nil
 }

covenant/covenant.go

@@ -152,7 +152,7 @@ func (ce *CovenantEmulator) AddCovenantSignatures(btcDels []*types.Delegation) (
 		if uint64(unbondingTime) <= minUnbondingTime {
 			ce.logger.Error("invalid unbonding time",
 				zap.Uint64("min_unbonding_time", minUnbondingTime),
-				zap.Uint32("got_unbonding_time", unbondingTime),
+				zap.Uint16("got_unbonding_time", unbondingTime),
 			)
 			continue
 		}
@@ -171,11 +171,11 @@ func (ce *CovenantEmulator) AddCovenantSignatures(btcDels []*types.Delegation) (
 			continue
 		}
 
-		if btcDel.TotalSat < uint64(params.MinStakingValue) || btcDel.TotalSat > uint64(params.MaxStakingValue) {
+		if btcDel.TotalSat < params.MinStakingValue || btcDel.TotalSat > params.MaxStakingValue {
 			ce.logger.Error("invalid staking value",
-				zap.Uint64("min_staking_value", uint64(params.MinStakingValue)),
-				zap.Uint64("max_staking_value", uint64(params.MaxStakingValue)),
-				zap.Uint64("got_staking_value", btcDel.TotalSat),
+				zap.Int64("min_staking_value", int64(params.MinStakingValue)),
+				zap.Int64("max_staking_value", int64(params.MaxStakingValue)),
+				zap.Int64("got_staking_value", int64(btcDel.TotalSat)),
 			)
 			continue
 		}
@@ -296,7 +296,7 @@ func signSlashUnbondingSignatures(
 		del.FpBtcPks,
 		params.CovenantPks,
 		params.CovenantQuorum,
-		uint16(del.UnbondingTime),
+		del.UnbondingTime,
 		btcutil.Amount(unbondingTx.TxOut[0].Value),
 		btcNet,
 	)
@@ -341,15 +341,14 @@ func signSlashAndUnbondSignatures(
 	params *types.StakingParams,
 	btcNet *chaincfg.Params,
 ) ([][]byte, *schnorr.Signature, error) {
-
 	// sign slash signatures with every finality providers
 	stakingInfo, err := btcstaking.BuildStakingInfo(
 		del.BtcPk,
 		del.FpBtcPks,
 		params.CovenantPks,
 		params.CovenantQuorum,
 		del.GetStakingTime(),
-		btcutil.Amount(del.TotalSat),
+		del.TotalSat,
 		btcNet,
 	)
 	if err != nil {
@@ -429,7 +428,7 @@ func decodeDelegationTransactions(del *types.Delegation, params *types.StakingPa
 		params.SlashingRate,
 		params.SlashingPkScript,
 		del.BtcPk,
-		uint16(del.UnbondingTime),
+		del.UnbondingTime,
 		btcNet,
 	); err != nil {
 		return nil, nil, fmt.Errorf("invalid txs in the delegation: %w", err)
@@ -459,7 +458,7 @@ func decodeUndelegationTransactions(del *types.Delegation, params *types.Staking
 		params.SlashingRate,
 		params.SlashingPkScript,
 		del.BtcPk,
-		uint16(del.UnbondingTime),
+		del.UnbondingTime,
 		btcNet,
 	); err != nil {
 		return nil, nil, fmt.Errorf("invalid txs in the undelegation: %w", err)

covenant/covenant_test.go

@@ -2,6 +2,7 @@ package covenant_test
 
 import (
 	"encoding/hex"
+	"github.com/btcsuite/btcd/btcutil"
 	"math/rand"
 	"testing"
 
@@ -87,8 +88,8 @@ func FuzzAddCovenantSig(f *testing.F) {
 				FpBtcPks:         fpPks,
 				StartHeight:      startHeight, // not relevant here
 				EndHeight:        startHeight + uint64(stakingTimeBlocks),
-				TotalSat:         uint64(stakingValue),
-				UnbondingTime:    uint32(unbondingTime),
+				TotalSat:         btcutil.Amount(stakingValue),
+				UnbondingTime:    unbondingTime,
 				StakingTxHex:     hex.EncodeToString(stakingTxBytes),
 				StakingOutputIdx: stakingOutputIdx,
 				SlashingTxHex:    testInfo.SlashingTx.ToHexStr(),

log/log.go

@@ -62,7 +62,8 @@ func NewRootLoggerWithFile(logFile string, level string) (*zap.Logger, error) {
 	if err := util.MakeDirectory(filepath.Dir(logFile)); err != nil {
 		return nil, err
 	}
-	f, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
+	// #nosec G304 - The log file path is provided by the user and not externally
+	f, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600)
 	if err != nil {
 		return nil, err
 	}

types/delegation.go

@@ -1,6 +1,7 @@
 package types
 
 import (
+	"github.com/btcsuite/btcd/btcutil"
 	"math"
 
 	bbn "github.com/babylonlabs-io/babylon/types"
@@ -23,7 +24,7 @@ type Delegation struct {
 	EndHeight uint64
 	// The total amount of BTC stakes in this delegation
 	// quantified in satoshi
-	TotalSat uint64
+	TotalSat btcutil.Amount
 	// The hex string of the staking tx
 	StakingTxHex string
 	// The index of the staking output in the staking tx
@@ -32,7 +33,7 @@ type Delegation struct {
 	SlashingTxHex string
 	// UnbondingTime describes how long the funds will be locked either in unbonding output
 	// or slashing change output
-	UnbondingTime uint32
+	UnbondingTime uint16
 	// The signatures on the slashing tx
 	// by the covenants (i.e., SKs corresponding to covenant_pks in params)
 	// It will be a part of the witness for the staking tx output.
@@ -47,7 +48,7 @@ type Delegation struct {
 // HasCovenantQuorum returns whether a delegation has sufficient sigs
 // from Covenant members to make a quorum
 func (d *Delegation) HasCovenantQuorum(quorum uint32) bool {
-	return uint32(len(d.CovenantSigs)) >= quorum && d.BtcUndelegation.HasAllSignatures(quorum)
+	return len(d.CovenantSigs) >= int(quorum) && d.BtcUndelegation.HasAllSignatures(quorum)
 }
 
 func (d *Delegation) GetStakingTime() uint16 {

types/params.go

@@ -36,16 +36,16 @@ type StakingParams struct {
 	// Fee required by unbonding transaction
 	UnbondingFee btcutil.Amount
 
-	// Minimum staking time required by bayblon
+	// Minimum staking time required by babylon
 	MinStakingTime uint16
 
-	// Maximum staking time required by bayblon
+	// Maximum staking time required by babylon
 	MaxStakingTime uint16
 
-	// Minimum staking value required by bayblon
+	// Minimum staking value required by babylon
 	MinStakingValue btcutil.Amount
 
-	// Maximum staking value required by bayblon
+	// Maximum staking value required by babylon
 	MaxStakingValue btcutil.Amount
 }