fix: clear gosec 2.24.7 findings and stabilize Go 1.24 / go-ethereum 1.17 CI#17
Merged
Conversation
- block_counter.go (G115): use strconv.ParseUint(_, 0, 64) instead of ParseInt with bitSize=32 + uint64 cast. Removes the int64->uint64 conversion gosec flagged and lifts the latent int32 ceiling on block numbers. - disk_persistence.go (G301): tighten EnsureDirectoryExists from os.ModePerm (0o777) to 0o750. Excludes 'other' from the node data directory while still allowing same-group access for operational patterns (backup, sidecar under same group). Existing deployments keep their current perms; only newly-created dirs use the tighter mode. - template.go (G703): extend existing '#nosec G304' annotation to also cover G703. Gosec 2.20+ split path-traversal analysis into AST-based G304 and SSA-based G703; the build-time codegen taint-from-os.Args pattern is the same one the original author already accepted.
…17.3 - promise.go, template.go: replace non-constant format string in fmt.Printf/Fprintf with literal '%s' format. Go 1.24's go vet now errors on non-constant first arg. - resubscribe_test.go: switch fmt.Errorf(s) to errors.New(s) for the same non-constant format string check. - rate_limiter_test.go: implement SubscribeTransactionReceipts on mockEthereumClient. go-ethereum v1.17.3 added this method to ethereum.TransactionReader, which EthereumClient embeds, so all mocks transitively need it to satisfy the interface.
gosec v2.19.0 runs in a Docker image with Go 1.21.3, which fails to load this module after the go-ethereum v1.17.3 bump pushed go.mod to 'go 1.24.0'. v2.24.7 ships with a Go toolchain that satisfies the requirement. Supersedes #15 (this commit bundles the bump that PR was tracking).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two-part fix to unblock the gosec action bump in #15 and turn main green again post-#16.
1. Address gosec 2.24.7 findings
Running gosec v2.24.7 locally against current main surfaces three issues:
pkg/chain/ethereum/block_counter.go:114strconv.ParseInt(_, 0, 32)+uint64(...)cast →strconv.ParseUint(_, 0, 64). Removes the gosec finding and lifts the latent int32 block-height ceiling (~800 years away, but free to fix).pkg/persistence/disk_persistence.go:210os.ModePerm(0o777) →0o750. Used by keep-core'sinitializeKeyStorePersistenceandinitializeWorkPersistence— the node data and keystore dirs. Excludes "other" while still allowing same-group ops (backup, sidecar under same group).tools/generators/template/template.go:38#nosec G304annotation to#nosec G304 G703. Gosec 2.20+ split path-traversal analysis into an AST-based rule (G304) and an SSA-based rule (G703); the original author's false-positive call for this build-time codegen tool still applies.Forward-only behavior change: for G301, existing deployed nodes keep their current dir permissions. Only directories created from this version onward use 0o750.
2. Stabilize CI after #16
Main has been red post-#16 from two pre-existing problems that #16 surfaced but didn't fix:
go vetrejects non-constant format strings. Fixed intools/generators/promise/promise.go,tools/generators/template/template.go, andpkg/chain/ethereum/ethutil/resubscribe_test.go.mockEthereumClientno longer satisfiesEthereumClient.ethereum.TransactionReadergainedSubscribeTransactionReceiptsin go-ethereum v1.17.3; added a noop implementation tomockEthereumClientinpkg/chain/ethereum/ethutil/rate_limiter_test.go. The two mocks that embed it (mockAdaptedEthereumClient,mockAdaptedEthereumClientWithReceipt) inherit it transitively.Verification
Locally, against this branch:
go vet ./...— cleango build ./...— cleangosec ./...(v2.24.7) — Issues: 0go test ./...— 342 pass (one pre-existing flaky test inclientinfopasses on retry; not introduced here)Merge order
This PR should land before #15 (the gosec action bump). After both land, CI runs gosec v2.24.7 with zero findings on main.
Test plan