Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add binary checksum generation to build workflow #2018

Merged
merged 3 commits into from
Feb 17, 2025

Conversation

pedro-pelicioni-cw
Copy link
Contributor

@pedro-pelicioni-cw pedro-pelicioni-cw commented Feb 17, 2025

User description

  • Add checksum generation step for stratus binary
  • Generate checksums.txt file with SHA256 hash
  • Upload checksums file as a separate artifact with commit SHA
  • Keep checksum file in project root for easy access

PR Type

Enhancement


Description

  • Add checksum generation for stratus binary

  • Upload checksums as separate artifact

  • Improve build workflow configuration

  • Update Rust setup and dependencies


Changes walkthrough 📝

Relevant files
Enhancement
build-binary.yml
Enhance build workflow with checksum generation and configuration
updates

.github/workflows/build-binary.yml

  • Added checksum generation for stratus binary
  • Implemented artifact upload for checksums
  • Updated Rust setup and cache configuration
  • Added installation of necessary dependencies
  • +69/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • - Add checksum generation step for stratus binary
    - Generate checksums.txt file with SHA256 hash
    - Upload checksums file as a separate artifact with commit SHA
    - Keep checksum file in project root for easy access
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The checksum generation step doesn't include error handling or validation. Consider adding checks to ensure the binary exists and the checksum was generated successfully.

    - name: Generate checksums
      run: |
        cd target/release
        sha256sum stratus > ../../checksums.txt
    Logging

    The workflow lacks proper logging for important steps, such as the checksum generation. Consider adding echo statements or using GitHub Actions' logging commands to improve visibility and debugging.

    - name: Generate checksums
      run: |
        cd target/release
        sha256sum stratus > ../../checksums.txt

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Include filename in checksum output

    Include the file name in the checksum output to provide clear association between
    the checksum and the specific file it represents.

    .github/workflows/build-binary.yml [124-127]

     - name: Generate checksums
       run: |
         cd target/release
         sha256sum stratus > ../../checksums.txt
    +    echo "stratus: $(cat ../../checksums.txt)" > ../../checksums.txt
    Suggestion importance[1-10]: 5

    __

    Why: This suggestion improves clarity by explicitly associating the checksum with the file name. It enhances readability and reduces potential confusion, especially if multiple files are involved in the future.

    Low
    Security
    Use stronger hash algorithm

    Consider using a more secure hashing algorithm like SHA-512 instead of SHA-256 for
    generating checksums. This provides stronger protection against potential hash
    collisions.

    .github/workflows/build-binary.yml [124-127]

     - name: Generate checksums
       run: |
         cd target/release
    -    sha256sum stratus > ../../checksums.txt
    +    sha512sum stratus > ../../checksums.txt
    Suggestion importance[1-10]: 3

    __

    Why: While using SHA-512 does provide stronger protection, SHA-256 is still considered secure for most applications. The change would offer only a marginal security improvement in this context.

    Low

    Copy link
    Contributor

    @gabriel-aranha-cw gabriel-aranha-cw left a comment

    Choose a reason for hiding this comment

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

    Need to remove duplicate logic. The PR should only have added Generate and upload checksum steps.

    pedro-pelicioni-cw and others added 2 commits February 17, 2025 13:53
    Remove redundant workflow configuration in build-binary.yml, simplifying the GitHub Actions workflow definition
    @pedro-pelicioni-cw
    Copy link
    Contributor Author

    Screenshot 2025-02-17 at 14 36 07

    @pedro-pelicioni-cw pedro-pelicioni-cw merged commit 834fad7 into main Feb 17, 2025
    2 checks passed
    @pedro-pelicioni-cw pedro-pelicioni-cw deleted the pedro-pelicioni-cw-patch-1 branch February 17, 2025 18:56
    @pedro-pelicioni-cw pedro-pelicioni-cw linked an issue Feb 21, 2025 that may be closed by this pull request
    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.

    Additional build information in Stratus page
    2 participants