Skip to content

chore(node-api): bound validator index user input on proof apis#3103

Open
bar-bera wants to merge 4 commits into
mainfrom
claude-security-fixes/proof-apis-bound-offset
Open

chore(node-api): bound validator index user input on proof apis#3103
bar-bera wants to merge 4 commits into
mainfrom
claude-security-fixes/proof-apis-bound-offset

Conversation

@bar-bera
Copy link
Copy Markdown
Collaborator

There's a misleading #nosec statement which is only true if the user provided input is actually bounded to the max registry size.

Not causing any intentional or unintentional damage. Just adding the bounds for correctness.

There's a misleading [`#nosec`](https://github.com/berachain/beacon-kit/blob/88b030fdd6beaa1a46f1fa7b56782c62c6c58731/node-api/handlers/proof/merkle/validator_pubkey.go#L99) statement which is only true if the user provided input is actually bounded to the max registry size.

Not causing any intentional or unintentional damage. Just adding the bounds for correctness.
Copilot AI review requested due to automatic review settings May 11, 2026 12:38
@bar-bera bar-bera requested a review from a team as a code owner May 11, 2026 12:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the node-api Merkle proof helpers by bounding user-supplied validator indices to the maximum validator registry size, making the existing #nosec G115 justifications for uint64 -> int conversions accurate.

Changes:

  • Add validatorIndex bounds checks (< constants.ValidatorsRegistryLimit) before computing offsets used in int(...) conversions.
  • Update #nosec G115 annotations/comments to clarify why the int conversion is safe (on 64-bit builds) given the new bounds.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
node-api/handlers/proof/merkle/validator_pubkey.go Adds validator index bounds check before computing the validator gIndex offset; adjusts #nosec rationale.
node-api/handlers/proof/merkle/validator_credentials.go Adds validator index bounds check before computing offsets used in gIndex calculation; adjusts #nosec rationale.
node-api/handlers/proof/merkle/validator_balance.go Adds validator index bounds check and updates #nosec rationale for gIndex computation (but one comment needs correction).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread node-api/handlers/proof/merkle/validator_pubkey.go Outdated
Comment thread node-api/handlers/proof/merkle/validator_credentials.go Outdated
Comment thread node-api/handlers/proof/merkle/validator_balance.go Outdated
Comment thread node-api/handlers/proof/merkle/validator_balance.go
@bar-bera bar-bera requested a review from a team May 13, 2026 08:42
@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.24%. Comparing base (ffe5e16) to head (a11d9ce).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3103      +/-   ##
==========================================
+ Coverage   61.21%   61.24%   +0.03%     
==========================================
  Files         369      370       +1     
  Lines       18925    18941      +16     
==========================================
+ Hits        11584    11600      +16     
  Misses       6382     6382              
  Partials      959      959              
Files with missing lines Coverage Δ
...ode-api/handlers/proof/merkle/validator_balance.go 66.31% <100.00%> (+0.72%) ⬆️
...api/handlers/proof/merkle/validator_credentials.go 60.29% <100.00%> (+1.83%) ⬆️
...pi/handlers/proof/merkle/validator_index_bounds.go 100.00% <100.00%> (ø)
node-api/handlers/proof/merkle/validator_pubkey.go 61.19% <100.00%> (+1.81%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread node-api/handlers/proof/merkle/validator_pubkey.go Outdated
Comment thread node-api/handlers/proof/merkle/validator_balance.go Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants