Skip to content

Ttx/replace random nonce - #1406

Merged
adecaro merged 3 commits into
LFDT-Panurus:mainfrom
sid200727:ttx/replace-random-nonce
Mar 11, 2026
Merged

Ttx/replace random nonce#1406
adecaro merged 3 commits into
LFDT-Panurus:mainfrom
sid200727:ttx/replace-random-nonce

Conversation

@sid200727

Copy link
Copy Markdown
Contributor

Fixes #919

This PR addresses the TODO in GetRandomBytes within token/services/ttx/random.go.

Previously, rand.Read was used to generate random bytes. However, rand.Read does not guarantee that the requested number of bytes will be fully populated in a single call. This could potentially result in partially filled nonces without being detected.

This change replaces rand.Read with io.ReadFull(rand.Reader, ...), ensuring that either:

  • The exact number of requested bytes is generated, or
  • An error is returned.

This guarantees correct and complete nonce generation in GetRandomNonce, improving reliability and aligning with Go best practices for secure randomness.

@adecaro
adecaro self-requested a review March 6, 2026 05:30
@adecaro
adecaro force-pushed the ttx/replace-random-nonce branch from 50561ba to 5bcfd2a Compare March 6, 2026 05:30
@adecaro adecaro self-assigned this Mar 6, 2026
@adecaro adecaro added the chore label Mar 6, 2026
@adecaro adecaro added this to the Q1/26 milestone Mar 6, 2026

@adecaro adecaro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@adecaro

adecaro commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

Hi @sid200727 , thanks for submitting this. Please, fix the DCO by following the relative instructions. Thanks a lot.

@sid200727
sid200727 force-pushed the ttx/replace-random-nonce branch 6 times, most recently from b9d56e4 to f2a0720 Compare March 7, 2026 12:38
@sid200727

Copy link
Copy Markdown
Contributor Author

Hi @adecaro,

Thanks for the review and for pointing out the DCO requirement. I’ve updated the commits to include the proper sign-off. The checks are now running again.

Please let me know if any further changes are needed.

@sid200727

Copy link
Copy Markdown
Contributor Author

Hi @adecaro,

It looks like the CI failure is caused by a golangci-lint download error (HTTP 502) during the install-tools step rather than an issue with the changes in this PR.

Would it be possible to rerun the workflow?

Thanks!

@adecaro
adecaro force-pushed the ttx/replace-random-nonce branch from f2a0720 to 3e981c7 Compare March 9, 2026 15:36
@adecaro

adecaro commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

Hi @sid200727 , yes, re-running now. I'll merge as soon as the CI gives the okay. Thanks much.

@adecaro
adecaro force-pushed the ttx/replace-random-nonce branch 3 times, most recently from f3f595e to 6c83cc7 Compare March 10, 2026 15:06
Signed-off-by: Siddhi Khandelwal <siddhi.200727@gmail.com>
Signed-off-by: Siddhi Khandelwal <siddhi.200727@gmail.com>
Signed-off-by: Siddhi Khandelwal <siddhi.200727@gmail.com>
@adecaro
adecaro force-pushed the ttx/replace-random-nonce branch from 6c83cc7 to ec436a6 Compare March 11, 2026 08:52
@adecaro
adecaro merged commit dde03e1 into LFDT-Panurus:main Mar 11, 2026
97 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ttx service: replace GetRandomBytes

2 participants