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: capture OpenSSL and glibc version in build information #2015

Merged
merged 2 commits into from
Feb 14, 2025

Conversation

pedro-pelicioni-cw
Copy link
Contributor

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

User description

Add compile-time detection of OpenSSL and glibc versions to the build information. This enhancement provides additional system context by retrieving and exposing the versions of these critical libraries during the build process.


PR Type

Enhancement


Description

  • Add OpenSSL version to build information

  • Capture glibc version for Linux builds

  • Export new versions as compile-time environment variables

  • Include new version info in JSON output


Changes walkthrough 📝

Relevant files
Enhancement
build.rs
Capture and export OpenSSL and glibc versions                       

build.rs

  • Capture OpenSSL version using openssl version command
  • Detect glibc version on Linux using ldd --version
  • Export captured versions as compile-time environment variables
  • +26/-0   
    build_info.rs
    Add OpenSSL and glibc version constants and JSON output   

    src/infra/build_info.rs

  • Add constants for OpenSSL and glibc versions
  • Include new version information in JSON output
  • +4/-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 compile-time detection of OpenSSL and glibc versions to the build information. This enhancement provides additional system context by retrieving and exposing the versions of these critical libraries during the build process.
    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 code uses unwrap_or_else for error handling when capturing OpenSSL and glibc versions. Consider using more robust error handling and logging to provide better visibility into potential issues during the build process.

    let openssl_version = Command::new("openssl")
        .arg("version")
        .output()
        .ok()
        .and_then(|output| String::from_utf8(output.stdout).ok())
        .unwrap_or_else(|| "unknown".to_string());
    
    // Capture glibc version (Linux only)
    let glibc_version = if cfg!(target_os = "linux") {
        Command::new("ldd")
            .arg("--version")
            .output()
            .ok()
            .and_then(|output| String::from_utf8(output.stdout).ok())
            .and_then(|s| s.lines().next().map(|s| s.to_string()))
            .unwrap_or_else(|| "unknown".to_string())
    } else {
        "not applicable".to_string()
    };
    Tracing

    The new functionality for capturing OpenSSL and glibc versions is not logged. Consider adding tracing events to log the captured versions and any potential errors during the process.

    // Capture OpenSSL version
    let openssl_version = Command::new("openssl")
        .arg("version")
        .output()
        .ok()
        .and_then(|output| String::from_utf8(output.stdout).ok())
        .unwrap_or_else(|| "unknown".to_string());
    
    // Capture glibc version (Linux only)
    let glibc_version = if cfg!(target_os = "linux") {
        Command::new("ldd")
            .arg("--version")
            .output()
            .ok()
            .and_then(|output| String::from_utf8(output.stdout).ok())
            .and_then(|s| s.lines().next().map(|s| s.to_string()))
            .unwrap_or_else(|| "unknown".to_string())
    } else {
        "not applicable".to_string()
    };
    
    // Export as compile-time environment variables
    println!("cargo:rustc-env=BUILD_OPENSSL_VERSION={}", openssl_version.trim());
    println!("cargo:rustc-env=BUILD_GLIBC_VERSION={}", glibc_version.trim());

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Security
    Sanitize command output for safety

    Sanitize and validate the output of system commands before using them as environment
    variables to prevent potential injection vulnerabilities.

    build.rs [71-72]

    -println!("cargo:rustc-env=BUILD_OPENSSL_VERSION={}", openssl_version.trim());
    -println!("cargo:rustc-env=BUILD_GLIBC_VERSION={}", glibc_version.trim());
    +fn sanitize_version(version: &str) -> String {
    +    version.chars().filter(|c| c.is_ascii_alphanumeric() || *c == '.').collect()
    +}
     
    +println!("cargo:rustc-env=BUILD_OPENSSL_VERSION={}", sanitize_version(&openssl_version.trim()));
    +println!("cargo:rustc-env=BUILD_GLIBC_VERSION={}", sanitize_version(&glibc_version.trim()));
    +
    Suggestion importance[1-10]: 9

    __

    Why: This suggestion addresses a critical security concern by sanitizing command outputs before using them as environment variables, preventing potential injection vulnerabilities.

    High
    Possible issue
    Improve error handling for commands

    Handle potential errors when executing system commands to retrieve OpenSSL and glibc
    versions. Use a more robust error handling approach instead of defaulting to
    "unknown" or "not applicable".

    build.rs [50-55]

     let openssl_version = Command::new("openssl")
         .arg("version")
         .output()
    -    .ok()
    -    .and_then(|output| String::from_utf8(output.stdout).ok())
    -    .unwrap_or_else(|| "unknown".to_string());
    +    .map_err(|e| format!("Failed to execute openssl command: {}", e))
    +    .and_then(|output| String::from_utf8(output.stdout)
    +        .map_err(|e| format!("Failed to parse openssl output: {}", e)))
    +    .unwrap_or_else(|e| format!("Error: {}", e));
    Suggestion importance[1-10]: 8

    __

    Why: The suggestion significantly improves error handling, providing more detailed error messages instead of defaulting to "unknown". This enhances debugging and system understanding.

    Medium

    Enhance version parsing for system libraries by extracting only the version number from command outputs. Specifically:
    - Modify OpenSSL version extraction to capture just the version number
    - Refine glibc version parsing to remove the "ldd (GNU libc)" prefix
    @pedro-pelicioni-cw pedro-pelicioni-cw merged commit 4d63122 into main Feb 14, 2025
    39 checks passed
    @pedro-pelicioni-cw pedro-pelicioni-cw deleted the feat-add-build-env-info branch February 14, 2025 17:56
    @gabriel-aranha-cw
    Copy link
    Contributor

    Final benchmark:
    Run ID: bench-3511227984

    Git Info:

    Configuration:

    • Target Account Strategy: Default

    RPS Stats: Max: 1958.00, Min: 1279.00, Avg: 1664.76, StdDev: 158.29
    TPS Stats: Max: 1914.00, Min: 1341.00, Avg: 1616.23, StdDev: 164.15

    Plot: View Plot

    @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