Skip to content

Conversation

@N6REJ
Copy link
Contributor

@N6REJ N6REJ commented Nov 14, 2025

PR Type

Enhancement, Documentation


Description

  • Complete migration from Ant/hybrid build system to pure Gradle build system with Groovy DSL

  • Added comprehensive Gradle build configuration (build.gradle.bruno-reference) with 1183 lines implementing version resolution, core tasks (release, releaseAll, clean), verification tasks, and automatic module downloads with caching

  • Created extensive documentation suite covering build system overview, tasks reference, configuration guide, and migration guide from Ant to Gradle

  • Added Gradle performance configuration (gradle.properties) with daemon, parallel execution, and build caching settings

  • Implemented Ant build commons (build-commons-ant.xml) with reusable macros for compression, hashing, and module management

  • Restructured README with Gradle focus and quick start commands

  • Documented design decisions, system requirements, and migration procedures with troubleshooting guides


Diagram Walkthrough

flowchart LR
  A["Ant/Hybrid Build System"] -->|"Migration"| B["Pure Gradle Build"]
  B --> C["build.gradle.bruno-reference"]
  B --> D["gradle.properties"]
  B --> E["build-commons-ant.xml"]
  C --> F["Core Tasks<br/>release, clean, verify"]
  C --> G["Version Resolution<br/>with Caching"]
  H["Documentation Suite"] --> I["README.md"]
  H --> J[".gradle-docs/"]
  J --> K["TASKS.md"]
  J --> L["CONFIGURATION.md"]
  J --> M["MIGRATION.md"]
  J --> N["INDEX.md"]
Loading

File Walkthrough

Relevant files
Enhancement
1 files
build.gradle.bruno-reference
Complete Gradle build system for Bruno module with version management

build.gradle.bruno-reference

  • Added comprehensive Gradle build configuration for Bruno module with
    1183 lines of Groovy DSL code
  • Implements version resolution strategy with 3-tier fallback (local
    releases.properties, remote modules-untouched, standard URL format)
  • Includes core build tasks (release, releaseAll, clean) with
    interactive and non-interactive modes
  • Provides verification tasks (verify, validateProperties,
    checkModulesUntouched) and information tasks (info, listVersions,
    listReleases)
  • Implements automatic download and extraction from modules-untouched
    repository with caching support
  • Generates hash files (MD5, SHA1, SHA256, SHA512) for all archives
+1183/-0
Configuration changes
2 files
build-commons-ant.xml
Ant build commons with reusable macros and library management

build-commons-ant.xml

  • Added 510 lines of Ant build commons configuration with reusable
    macros and targets
  • Defines helper macros for 7-Zip compression (sevenzip, unsevenzip),
    file hashing (hashfile, hashfiles), and module downloads (getmodule,
    download)
  • Includes utility macros for MSI extraction, property assertions, and
    directory validation
  • Provides library loading targets for ANT Contrib, InnoSetup,
    HashMyFiles, Composer, and lessmsi
  • Implements hash generation targets for apps, bins, tools, and releases
    directories
+510/-0 
gradle.properties
Gradle performance configuration properties file                 

gradle.properties

  • New file with Gradle daemon and performance optimization settings
  • Enables parallel execution, build caching, and configuration on demand
  • Configures JVM arguments, console output, and file system watching for
    optimal build performance
+30/-0   
Documentation
12 files
README.md
Comprehensive Gradle build documentation and user guide   

.gradle-docs/README.md

  • Added 495 lines of comprehensive Gradle build documentation covering
    overview, quick start, and installation
  • Includes detailed build tasks reference with core, verification, and
    information tasks
  • Documents complete architecture and build process flow with packaging
    details and archive structure examples
  • Provides modules-untouched integration guide explaining automatic
    downloads, caching, and version resolution
  • Includes troubleshooting section with common issues and solutions,
    plus migration guide from Ant to Gradle
+495/-0 
CONFIGURATION.md
Complete configuration guide for build system settings     

.gradle-docs/CONFIGURATION.md

  • Added 551 lines of detailed configuration guide covering all build
    system settings
  • Documents build.properties with required and optional properties
    (bundle.name, bundle.release, bundle.type, bundle.format, build.path)
  • Explains gradle.properties settings for daemon, parallel execution,
    caching, and JVM configuration
  • Describes environment variables (BEARSAMPP_BUILD_PATH, 7Z_HOME,
    JAVA_HOME) with usage examples
  • Provides build path structure, archive configuration, configuration
    examples, and best practices
+551/-0 
TASKS.md
Complete Gradle tasks reference with examples and best practices

.gradle-docs/TASKS.md

  • Added 620 lines of comprehensive Gradle tasks reference documentation
  • Documents all build tasks (release, releaseAll, clean) with usage
    examples and output samples
  • Covers verification tasks (verify, validateProperties,
    checkModulesUntouched) with detailed descriptions
  • Includes information tasks (info, listVersions, listReleases) with
    example outputs
  • Provides task dependencies, global Gradle options, project properties,
    and best practices for task usage
+620/-0 
FINAL-SUMMARY.md
Migration summary and completion status documentation       

FINAL-SUMMARY.md

  • Added 295 lines summarizing the completed migration to Gradle build
    system
  • Highlights key changes: clean ASCII output, interactive release mode,
    directory location display, simplified build script
  • Documents build system specifications and provides usage examples for
    interactive/non-interactive modes
  • Verifies consistency with Bruno and Apache modules across all features
  • Lists available tasks, testing results, key improvements, and confirms
    migration completion status
+295/-0 
MIGRATION.md
Complete Ant to Gradle migration guide documentation         

.gradle-docs/MIGRATION.md

  • Comprehensive migration guide from Ant to Gradle build system with 511
    lines of detailed documentation
  • Includes command mapping tables showing equivalents between Ant and
    Gradle commands
  • Documents file changes, configuration changes, and task equivalents
    with before/after examples
  • Provides troubleshooting section for common migration issues and next
    steps for developers, CI/CD, and contributors
+511/-0 
DESIGN-DECISIONS.md
Design decisions and architectural rationale documentation

DESIGN-DECISIONS.md

  • Explains design rationale for using -PbundleVersion parameter instead
    of -Pversion
  • Documents directory display convention using [bin] location indicators
    for consistency with other Bearsampp modules
  • Justifies pure Gradle architecture choice and Groovy DSL selection
    over Kotlin DSL
  • Details property naming conventions, directory structure, and release
    naming format with future extensibility considerations
+371/-0 
BUILD-SYSTEM.md
Build system specification and requirements documentation

BUILD-SYSTEM.md

  • Specifies pure Gradle build system with Groovy DSL (no wrapper
    required)
  • Documents system requirements (Java 8+, Gradle 7.0+, 7-Zip) and
    installation instructions
  • Provides build process phases, verification steps, and migration notes
    from Ant and Kotlin DSL
  • Includes troubleshooting section and best practices for Groovy DSL and
    system Gradle usage
+418/-0 
INDEX.md
Documentation index and navigation guide                                 

.gradle-docs/INDEX.md

  • Complete documentation index with quick links to all Gradle build
    documentation
  • Organizes documentation by topic (Build System, Tasks, Configuration,
    Migration) with cross-references
  • Provides keyword search index (A-Z) for finding documentation by topic
  • Includes document summaries, version history, and contribution
    guidelines
+340/-0 
build-bundle-ant.xml
Legacy Ant build file reference documentation                       

build-bundle-ant.xml

  • Reference implementation of legacy Ant build file for historical
    documentation purposes
  • Contains Ant macros and targets for bundle release, version
    management, and archive creation
  • Demonstrates previous build approach with XML-based configuration and
    property management
+203/-0 
MIGRATION-SUMMARY.md
Migration summary from Ant to pure Gradle build                   

MIGRATION-SUMMARY.md

  • Summarizes successful migration from hybrid Ant/Gradle to pure Gradle
    build system
  • Documents removed Ant files, updated build configuration, and new
    Gradle features
  • Lists command changes with before/after examples and benefits of
    migration
  • Includes file structure comparison, testing results, and verification
    procedures
+298/-0 
DOCS-UPDATE-SUMMARY.md
Documentation restructuring and consolidation summary       

DOCS-UPDATE-SUMMARY.md

  • Documents consolidation of documentation from 12 files to 5 focused
    documents
  • Lists files updated (README.md, INDEX.md, TASKS.md, CONFIGURATION.md,
    MIGRATION.md) and files removed
  • Highlights key improvements in structure, content, and usability
  • References module-php gradle-convert branch as implementation
    reference
+141/-0 
README.md
README restructured with Gradle build system focus             

README.md

  • Completely restructured with new Build System section highlighting
    Gradle features
  • Added quick start commands showing gradle info, gradle release, and
    other key tasks
  • Replaced verbose documentation with table-based prerequisites and
    available tasks
  • Added links to comprehensive documentation in .gradle-docs/ directory
+61/-2   
Formatting
1 files
build.xml.reference
Line ending normalization for reference file                         

build.xml.reference

  • Line ending normalization (CRLF to LF) for legacy Ant build file
    reference
  • No functional changes to the XML content
+38/-38 
Additional files
12 files
bearsampp.conf [link]   
bearsampp.conf [link]   
bearsampp.conf [link]   
bearsampp.conf [link]   
bearsampp.conf [link]   
bearsampp.conf [link]   
bearsampp.conf [link]   
bearsampp.conf [link]   
bearsampp.conf [link]   
bearsampp.conf [link]   
bearsampp.conf [link]   
bearsampp.conf [link]   

@N6REJ N6REJ added the enhancement ✨ Improve program label Nov 14, 2025
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Nov 14, 2025

PR Compliance Guide 🔍

(Compliance updated until commit fb8a8a2)

Below is a summary of compliance checks for this PR:

Security Compliance
Unverified downloads

Description: The build downloads artifacts from a remote URL over HTTPS without verifying signatures or
pinning and then extracts/uses them, enabling a potential supply-chain attack if the
remote or connection is compromised; no checksum/signature verification of downloaded
archives is performed before extraction.
build.gradle.bruno-reference [170-183]

Referred Code
    ant.get(src: untouchedUrl, dest: downloadedFile, verbose: true)
    println "  Download complete from modules-untouched"
} catch (Exception e) {
    throw new GradleException("""
        Failed to download from modules-untouched: ${e.message}

        Tried URL: ${untouchedUrl}

        Please verify:
        1. Version ${version} exists in modules-untouched repository
        2. The URL is correct in bruno.properties or matches format: bruno-{version}/bruno-{version}-win64.7z
        3. You have internet connectivity
    """.stripIndent())
}
Executable hijacking

Description: The script executes an external 7-Zip binary discovered via environment or PATH using
ProcessBuilder without validating the executable path/signature, allowing PATH hijacking
leading to arbitrary code execution during extraction/compression.
build.gradle.bruno-reference [247-273]

Referred Code
if (filename.endsWith('.7z')) {
    // Try to use 7zip if available
    def sevenZipPath = find7ZipExecutable()
    if (sevenZipPath) {
        def command = [
            sevenZipPath.toString(),
            'x',
            downloadedFile.absolutePath.toString(),
            "-o${extractPath.absolutePath}".toString(),
            '-y'
        ]
        def process = new ProcessBuilder(command as String[])
            .directory(extractPath)
            .redirectErrorStream(true)
            .start()

        process.inputStream.eachLine { line ->
            if (line.trim()) println "    ${line}"
        }

        def exitCode = process.waitFor()


 ... (clipped 6 lines)
Executable hijacking

Description: Archives are created by invoking an external 7z executable via ProcessBuilder without path
hardening or validation, again exposing the build to PATH or environment hijacking to
execute a malicious binary.
build.gradle.bruno-reference [621-639]

Referred Code
    '-t7z',
    archiveFile.absolutePath.toString(),
    '.'
]

def process = new ProcessBuilder(command as String[])
    .directory(brunoPrepPath)
    .redirectErrorStream(true)
    .start()

process.inputStream.eachLine { line ->
    if (line.trim()) println "  ${line}"
}

def exitCode = process.waitFor()
if (exitCode != 0) {
    throw new GradleException("7zip compression failed with exit code: ${exitCode}")
}
Weak hash algorithms

Description: Generated MD5 and SHA-1 checksums are produced and presumably published alongside stronger
hashes; MD5/SHA-1 are cryptographically broken and should not be used to validate
integrity, which may mislead consumers into using weak verification.
build.gradle.bruno-reference [722-745]

Referred Code
// Generate MD5
def md5File = new File("${file.absolutePath}.md5")
def md5Hash = calculateHash(file, 'MD5')
md5File.text = "${md5Hash} ${file.name}\n"
println "  Created: ${md5File.name}"

// Generate SHA1
def sha1File = new File("${file.absolutePath}.sha1")
def sha1Hash = calculateHash(file, 'SHA-1')
sha1File.text = "${sha1Hash} ${file.name}\n"
println "  Created: ${sha1File.name}"

// Generate SHA256
def sha256File = new File("${file.absolutePath}.sha256")
def sha256Hash = calculateHash(file, 'SHA-256')
sha256File.text = "${sha256Hash} ${file.name}\n"
println "  Created: ${sha256File.name}"

// Generate SHA512
def sha512File = new File("${file.absolutePath}.sha512")
def sha512Hash = calculateHash(file, 'SHA-512')


 ... (clipped 3 lines)
Insecure binary discovery

Description: The function attempts to locate 7z.exe using the Windows 'where' command and environment
paths without constraining directories, enabling command injection or unintended binary
execution if PATH is manipulated in CI or developer environments.
build.gradle.bruno-reference [700-714]

Referred Code
    try {
        def process = ['where', '7z.exe'].execute()
        process.waitFor()
        if (process.exitValue() == 0) {
            def output = process.text.trim()
            if (output) {
                return output.split('\n')[0].trim()
            }
        }
    } catch (Exception e) {
        // Ignore
    }

    return null
}
Dynamic script execution

Description: Ant uses JavaScript (Nashorn) to execute inline code, which can increase attack surface
if properties are user-controlled; while powerful, it executes dynamic script at build
time without sandboxing.
build-commons-ant.xml [13-21]

Referred Code
<scriptdef name="randomstring" language="javascript">
  <attribute name="property"/>
  <![CDATA[
  importClass(java.security.SecureRandom);
  importClass(java.math.BigInteger);
  project.setProperty(attributes.get("property"), new BigInteger(130, new SecureRandom()).toString(32));
  ]]>
</scriptdef>
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logging: The added build scripts perform network downloads, file extractions, and packaging without
implementing structured audit logs capturing user, timestamp, action, and outcome for
critical operations.

Referred Code
    def releases = new Properties()
    releasesFile.withInputStream { releases.load(it) }

    return releases.getProperty(version) != null
}

// Function to fetch bruno.properties from modules-untouched repository
// This is the primary source for version information when not in releases.properties
def fetchModulesUntouchedProperties() {
    def propsUrl = "https://raw.githubusercontent.com/Bearsampp/modules-untouched/main/modules/bruno.properties"

    println "Fetching bruno.properties from modules-untouched repository..."
    println "  URL: ${propsUrl}"

    def tempFile = file("${bundleTmpDownloadPath}/bruno-untouched.properties")
    tempFile.parentFile.mkdirs()

    try {
        ant.get(src: propsUrl, dest: tempFile, verbose: false, ignoreerrors: false)

        def props = new Properties()


 ... (clipped 181 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Partial error handling: While many failures throw GradleException with messages, some external process executions
(e.g., 'where 7z.exe') swallow exceptions and several network operations rely on
println warnings without retries or structured context, which may miss edge cases.

Referred Code
// Try to find in PATH
try {
    def process = ['where', '7z.exe'].execute()
    process.waitFor()
    if (process.exitValue() == 0) {
        def output = process.text.trim()
        if (output) {
            return output.split('\n')[0].trim()
        }
    }
} catch (Exception e) {
    // Ignore
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Logs may leak paths: The scripts print full local filesystem paths, URLs, and environment-derived locations
which could expose sensitive environment details in logs; no redaction or structured
logging is implemented.

Referred Code
doLast {
    println """
    ================================================================
              Bearsampp Module Bruno - Build Info
    ================================================================

    Project:        ${projectName}
    Version:        ${projectVersion}
    Description:    ${projectDescription}

    Bundle Properties:
      Name:         ${bundleNameValue}
      Release:      ${bundleReleaseValue}
      Type:         ${bundleTypeValue}
      Format:       ${bundleFormatValue}

    Paths:
      Project Dir:  ${projectBasedirValue}
      Root Dir:     ${rootDirValue}
      Dev Path:     ${devPathValue}
      Build Base:   ${buildBasePathValue}


 ... (clipped 28 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unvalidated inputs: External inputs like remote URLs and version strings from properties/user input are used
for downloads and process execution without explicit validation or allowlisting,
increasing risk of misuse or injection into shell commands.

Referred Code
def versionProperty = project.findProperty('bundleVersion')

doLast {
    def versionToBuild = versionProperty

    if (!versionToBuild) {
        // Interactive mode - prompt for version
        def availableVersions = getAvailableVersions()

        if (availableVersions.isEmpty()) {
            throw new GradleException("No versions found in bin/ directory")
        }

        println ""
        println "=".multiply(70)
        println "Interactive Release Mode"
        println "=".multiply(70)
        println ""
        println "Available versions:"

        // Show versions with location tags


 ... (clipped 111 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit f9abd59
Security Compliance
🔴
Weak hash algorithms

Description: The build generates and publishes MD5 and SHA1 hashes alongside stronger SHA256/512, which
encourages use of weak, collision-prone algorithms that are considered insecure for
integrity; avoid publishing MD5/SHA1.
build.gradle.bruno-reference [722-745]

Referred Code
// Generate MD5
def md5File = new File("${file.absolutePath}.md5")
def md5Hash = calculateHash(file, 'MD5')
md5File.text = "${md5Hash} ${file.name}\n"
println "  Created: ${md5File.name}"

// Generate SHA1
def sha1File = new File("${file.absolutePath}.sha1")
def sha1Hash = calculateHash(file, 'SHA-1')
sha1File.text = "${sha1Hash} ${file.name}\n"
println "  Created: ${sha1File.name}"

// Generate SHA256
def sha256File = new File("${file.absolutePath}.sha256")
def sha256Hash = calculateHash(file, 'SHA-256')
sha256File.text = "${sha256Hash} ${file.name}\n"
println "  Created: ${sha256File.name}"

// Generate SHA512
def sha512File = new File("${file.absolutePath}.sha512")
def sha512Hash = calculateHash(file, 'SHA-512')


 ... (clipped 3 lines)
Executable trust risk

Description: The 7-Zip executable is resolved from environment and hardcoded Windows paths then
executed with user-controlled inputs, which on non-Windows or PATH-hijack scenarios could
execute an unintended binary leading to command execution; restrict execution to trusted
paths or verify the binary signature.
build.gradle.bruno-reference [673-714]

Referred Code
// Helper function to find 7-Zip executable
def find7ZipExecutable() {
    // Check environment variable
    def sevenZipHome = System.getenv('7Z_HOME')
    if (sevenZipHome) {
        def exe = file("${sevenZipHome}/7z.exe")
        if (exe.exists()) {
            return exe.absolutePath
        }
    }

    // Check common installation paths
    def commonPaths = [
        'C:/Program Files/7-Zip/7z.exe',
        'C:/Program Files (x86)/7-Zip/7z.exe',
        'D:/Program Files/7-Zip/7z.exe',
        'D:/Program Files (x86)/7-Zip/7z.exe'
    ]

    for (path in commonPaths) {
        def exe = file(path)


 ... (clipped 21 lines)
Supply chain trust

Description: The build fetches bruno.properties over HTTPS but does not verify authenticity beyond TLS
and accepts any content to decide download sources, enabling supply-chain poisoning if the
remote is compromised; add pinning, signature verification, or checksum validation.
build.gradle.bruno-reference [105-128]

Referred Code
// This is the primary source for version information when not in releases.properties
def fetchModulesUntouchedProperties() {
    def propsUrl = "https://raw.githubusercontent.com/Bearsampp/modules-untouched/main/modules/bruno.properties"

    println "Fetching bruno.properties from modules-untouched repository..."
    println "  URL: ${propsUrl}"

    def tempFile = file("${bundleTmpDownloadPath}/bruno-untouched.properties")
    tempFile.parentFile.mkdirs()

    try {
        ant.get(src: propsUrl, dest: tempFile, verbose: false, ignoreerrors: false)

        def props = new Properties()
        tempFile.withInputStream { props.load(it) }

        println "  ✓ Successfully loaded ${props.size()} versions from modules-untouched"
        return props
    } catch (Exception e) {
        println "  ✗ Warning: Could not fetch bruno.properties from modules-untouched: ${e.message}"
        println "  Will fall back to standard URL format if needed"


 ... (clipped 3 lines)
Unsigned binary download

Description: When a version is missing locally, the code constructs a release URL and downloads
binaries from GitHub without validating signatures or expected checksums before
extraction, risking malicious binaries; enforce checksum/signature verification prior to
use.
build.gradle.bruno-reference [130-189]

Referred Code
// Function to download from modules-untouched repository
def downloadFromModulesUntouched(String version, File destDir) {
    println "Version ${version} not found in releases.properties"
    println "Checking modules-untouched repository..."

    // First, try to fetch bruno.properties from modules-untouched
    def untouchedProps = fetchModulesUntouchedProperties()
    def untouchedUrl = null

    if (untouchedProps) {
        untouchedUrl = untouchedProps.getProperty(version)
        if (untouchedUrl) {
            println "Found version ${version} in modules-untouched bruno.properties"
            println "Downloading from:"
            println "  ${untouchedUrl}"
        } else {
            println "Version ${version} not found in modules-untouched bruno.properties"
            println "Attempting to construct URL based on standard format..."
            // Fallback to constructed URL
            untouchedUrl = "https://github.com/Bearsampp/modules-untouched/releases/download/bruno-${version}/bruno-${version}-win64.7z"
            println "  ${untouchedUrl}"


 ... (clipped 39 lines)
External process execution

Description: The script executes an external 7-Zip process with paths derived from downloaded archives;
while arguments are passed as an array (mitigating injection), lack of validation of paths
and reliance on system PATH may allow unintended binary execution; ensure path
canonicalization and trusted executable location.
build.gradle.bruno-reference [246-273]

Referred Code
// Use 7zip or built-in extraction
if (filename.endsWith('.7z')) {
    // Try to use 7zip if available
    def sevenZipPath = find7ZipExecutable()
    if (sevenZipPath) {
        def command = [
            sevenZipPath.toString(),
            'x',
            downloadedFile.absolutePath.toString(),
            "-o${extractPath.absolutePath}".toString(),
            '-y'
        ]
        def process = new ProcessBuilder(command as String[])
            .directory(extractPath)
            .redirectErrorStream(true)
            .start()

        process.inputStream.eachLine { line ->
            if (line.trim()) println "    ${line}"
        }



 ... (clipped 7 lines)
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing audit logs: The new build tasks perform critical actions (downloading, extracting, packaging) without
structured audit logging that captures user ID and standardized timestamps, relying mainly
on println outputs.

Referred Code
    println "  ${downloadUrl}"

    // Determine filename from URL
    def filename = downloadUrl.substring(downloadUrl.lastIndexOf('/') + 1)
    def downloadDir = file(bundleTmpDownloadPath)
    downloadDir.mkdirs()

    downloadedFile = file("${downloadDir}/${filename}")

    // Download if not already present
    if (!downloadedFile.exists()) {
        println "  Downloading to: ${downloadedFile}"
        ant.get(src: downloadUrl, dest: downloadedFile, verbose: true)
        println "  Download complete"
    } else {
        println "  Using cached file: ${downloadedFile}"
    }
}

// Extract the archive
def extractDir = file(bundleTmpExtractPath)


 ... (clipped 65 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Limited error context: Several GradleExceptions provide generic messages without including actionable context
like target URLs or paths in all cases, and some downloads use ant.get without try/catch,
risking unhandled failures.

Referred Code
    // Download if not already present
    if (!downloadedFile.exists()) {
        println "  Downloading to: ${downloadedFile}"
        ant.get(src: downloadUrl, dest: downloadedFile, verbose: true)
        println "  Download complete"
    } else {
        println "  Using cached file: ${downloadedFile}"
    }
}

// Extract the archive
def extractDir = file(bundleTmpExtractPath)
extractDir.mkdirs()
println "  Extracting archive..."
def extractPath = file("${extractDir}/${version}")
if (extractPath.exists()) {
    delete extractPath
}
extractPath.mkdirs()

// Determine filename from downloaded file


 ... (clipped 38 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Verbose errors: Error and warning outputs echo full filesystem paths and external URLs directly to
console, which may be user-facing in some contexts and could disclose internal details.

Referred Code
            // Download and extract to bearsampp-build/tmp
            bundleSrcFinal = downloadAndExtractBruno(bundleVersion, file(bundleTmpExtractPath))
        } catch (Exception e) {
            throw new GradleException("""
                Failed to download Bruno binaries: ${e.message}

                You can manually download and extract Bruno binaries to:
                  ${bundleSrcDest}/

                Or check that version ${bundleVersion} exists in releases.properties
            """.stripIndent())
        }
    }
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unsanitized inputs: External inputs like constructed URLs and version strings from properties/user input are
used for downloads and command execution without explicit validation against allowed
patterns or schemes.

Referred Code
def versionProperty = project.findProperty('bundleVersion')

doLast {
    def versionToBuild = versionProperty

    if (!versionToBuild) {
        // Interactive mode - prompt for version
        def availableVersions = getAvailableVersions()

        if (availableVersions.isEmpty()) {
            throw new GradleException("No versions found in bin/ directory")
        }

        println ""
        println "=".multiply(70)
        println "Interactive Release Mode"
        println "=".multiply(70)
        println ""
        println "Available versions:"

        // Show versions with location tags


 ... (clipped 51 lines)

Learn more about managing compliance generic rules or creating your own custom rules

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Nov 14, 2025

PR Code Suggestions ✨

Latest suggestions up to fb8a8a2

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent hangs in external process

To prevent potential hangs when running the 7-Zip process, concurrently consume
both stdout and stderr streams in separate threads, add a timeout to the process
execution, and ensure the process is destroyed on timeout.

build.gradle.bruno-reference [258-270]

-def process = new ProcessBuilder(command as String[])
+def pb = new ProcessBuilder(command as String[])
     .directory(extractPath)
-    .redirectErrorStream(true)
-    .start()
+    .redirectErrorStream(false)
+def process = pb.start()
 
-process.inputStream.eachLine { line ->
-    if (line.trim()) println "    ${line}"
+// Consume stdout and stderr concurrently to avoid blocking
+def stdout = new StringBuilder()
+def stderr = new StringBuilder()
+def tOut = Thread.start {
+    process.inputStream.withReader { it.eachLine { line -> if (line?.trim()) { println "    ${line}"; stdout.append(line).append('\n') } } }
+}
+def tErr = Thread.start {
+    process.errorStream.withReader { it.eachLine { line -> if (line?.trim()) { println "    ${line}"; stderr.append(line).append('\n') } } }
 }
 
-def exitCode = process.waitFor()
+// Wait with timeout
+def finished = process.waitFor(10, java.util.concurrent.TimeUnit.MINUTES)
+tOut.join(1000)
+tErr.join(1000)
+if (!finished) {
+    process.destroyForcibly()
+    throw new GradleException("7zip extraction timed out after 10 minutes")
+}
+def exitCode = process.exitValue()
 if (exitCode != 0) {
-    throw new GradleException("7zip extraction failed with exit code: ${exitCode}")
+    throw new GradleException("7zip extraction failed (exit ${exitCode}). Stderr:\n${stderr.toString()}")
 }
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential deadlock when executing an external process and provides a robust solution using concurrent stream consumption and a timeout, which significantly improves the build script's reliability.

Medium
Fix contradictory download behavior note

Correct the note in README.md about automatic downloads, as it contradicts other
documentation stating that pre-existing binaries from the bin/ directory are
used.

README.md [38]

-**Note**: The build automatically downloads and extracts Memcached versions from the modules-untouched repository. You don't need to manually download or place files in the `bin/` directory.
+**Note**: This module uses pre-existing Memcached binaries located under the `bin/` directory (e.g., `bin/memcached1.6.39/`). You do not need to download files during the build. If a version is missing locally, add it under `bin/` before running `gradle release`.
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a critical contradiction in the documentation regarding how Memcached versions are obtained, which could mislead users and cause build failures.

Medium
General
Improve 7-Zip discovery portability

Improve the 7-Zip discovery logic to be cross-platform and more resilient. On
Windows, check for both 7z.exe and 7za.exe. On Unix-like systems, use which to
find 7z or 7za. Also, validate that the found file is executable.

build.gradle.bruno-reference [676-711]

 def sevenZipHome = System.getenv('7Z_HOME')
 if (sevenZipHome) {
-    def exe = file("${sevenZipHome}/7z.exe")
-    if (exe.exists()) {
+    for (n in ['7z.exe','7za.exe','7z','7za']) {
+        def exe = file("${sevenZipHome}/${n}")
+        if (exe.exists() && exe.canExecute()) {
+            return exe.absolutePath
+        }
+    }
+}
+
+// Common Windows installation paths
+def commonPaths = [
+    'C:/Program Files/7-Zip/7z.exe',
+    'C:/Program Files (x86)/7-Zip/7z.exe',
+    'C:/Program Files/7-Zip/7za.exe',
+    'C:/Program Files (x86)/7-Zip/7za.exe'
+]
+for (path in commonPaths) {
+    def exe = file(path)
+    if (exe.exists() && exe.canExecute()) {
         return exe.absolutePath
     }
 }
-...
+
+// Try PATH resolution on Windows and Unix-like systems
 try {
-    def process = ['where', '7z.exe'].execute()
-    process.waitFor()
-    if (process.exitValue() == 0) {
-        def output = process.text.trim()
-        if (output) {
-            return output.split('\n')[0].trim()
+    if (org.gradle.internal.os.OperatingSystem.current().isWindows()) {
+        for (n in ['7z.exe','7za.exe']) {
+            def p = ["cmd","/c","where", n].execute()
+            p.waitFor()
+            if (p.exitValue() == 0) {
+                def out = p.in.text.trim()
+                if (out) return out.readLines().first().trim()
+            }
+        }
+    } else {
+        for (n in ['7z','7za']) {
+            def p = ["bash","-lc","which ${n}"].execute()
+            p.waitFor()
+            if (p.exitValue() == 0) {
+                def out = p.in.text.trim()
+                if (out) return out
+            }
         }
     }
-} catch (Exception e) {
-    // Ignore
-}
+} catch (Exception ignore) {}
 
+return null
+

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion improves the 7-Zip discovery logic by making it cross-platform (checking for Unix commands) and more resilient (checking for 7za and executability), enhancing the build script's portability.

Medium
Ensure clean removes all outputs

Update the clean task example in MIGRATION.md to delete all build outputs,
including those in external directories like bearsampp-build/, to ensure a truly
clean state.

.gradle-docs/MIGRATION.md [291-322]

-### Clean Task
-
-#### After (Gradle)
-
-```groovy
 tasks.named('clean') {
     group = 'build'
     description = 'Clean build artifacts'
-    
+
     doLast {
-        def buildDir = file("${projectDir}/build")
-        if (buildDir.exists()) {
-            delete buildDir
-        }
+        // Default Gradle build dir
+        delete layout.buildDirectory.get().asFile
+
+        // Also clean external build outputs if used by the build
+        def externalBuildBase = project.findProperty('buildBasePath') ?: "${rootDir}/bearsampp-build"
+        delete file(externalBuildBase)
+
         println "[SUCCESS] Build artifacts cleaned"
     }
 }
-```

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that the example clean task is incomplete and may not clean all build artifacts, which is a valid but moderate issue in a documentation example.

Low
Clarify accurate Gradle minimum version

Update the minimum required Gradle version in BUILD-SYSTEM.md to reflect the
actual version needed for all documented features, such as 8.0+, to prevent
incompatibility issues.

BUILD-SYSTEM.md [12]

-| **Gradle Version**       | 7.0+ (tested with 9.2.0)                         |
+| **Gradle Version**       | 8.0+ (tested with 9.2.0)                         |
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies a potential inconsistency between the stated minimum Gradle version and the features used, which could cause issues for users on older Gradle versions.

Low
  • More

Previous suggestions

Suggestions up to commit 5f2502f
CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent 7-Zip process hangs

Refactor the 7-Zip process handling to consume stdout and stderr asynchronously
in separate threads and add a timeout. This prevents potential deadlocks and
build hangs.

build.gradle.bruno-reference [258-270]

-def process = new ProcessBuilder(command as String[])
+def pb = new ProcessBuilder(command as String[])
     .directory(extractPath)
-    .redirectErrorStream(true)
-    .start()
+    .redirectErrorStream(false)
+def process = pb.start()
 
-process.inputStream.eachLine { line ->
-    if (line.trim()) println "    ${line}"
+// Consume stdout and stderr asynchronously
+def stdout = new StringBuilder()
+def stderr = new StringBuilder()
+def outThread = Thread.start {
+    process.inputStream.withReader('UTF-8') { it.eachLine { l -> if (l?.trim()) { println "    ${l}"; stdout.append(l).append('\n') } } }
+}
+def errThread = Thread.start {
+    process.errorStream.withReader('UTF-8') { it.eachLine { l -> if (l?.trim()) { println "    [ERR] ${l}"; stderr.append(l).append('\n') } } }
 }
 
-def exitCode = process.waitFor()
-if (exitCode != 0) {
-    throw new GradleException("7zip extraction failed with exit code: ${exitCode}")
+// Wait with timeout (e.g., 10 minutes)
+def finished = process.waitFor(600, java.util.concurrent.TimeUnit.SECONDS)
+if (!finished) {
+    process.destroyForcibly()
+    outThread.join(5_000); errThread.join(5_000)
+    throw new GradleException("7zip extraction timed out after 600s")
+}
+outThread.join(); errThread.join()
+
+if (process.exitValue() != 0) {
+    throw new GradleException("7zip extraction failed with exit code ${process.exitValue()}:\n${stderr.toString()}")
 }
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential deadlock when handling external process streams and proposes a robust solution using asynchronous stream consumption and a timeout, significantly improving the build script's reliability.

Medium
General
Clarify download behavior

Clarify in README.md that Memcached versions must exist locally in the bin/
directory, as the build does not download them automatically, contrary to what
is currently stated.

README.md [38]

-**Note**: The build automatically downloads and extracts Memcached versions from the modules-untouched repository. You don't need to manually download or place files in the `bin/` directory.
+Note: For this module, Memcached versions are expected to exist locally under the `bin/` directory (for example, `bin/memcached1.6.39/`). The build does not download versions automatically. Use `gradle listVersions` to see locally available versions or place the required version in `bin/` before running `gradle release -PbundleVersion=...`.
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a critical contradiction in the documentation regarding how software versions are sourced, which would directly lead to user confusion and build failures.

Medium
Verify downloaded archive integrity

Add integrity verification for downloaded archives by checking for non-empty
file size and, if it's a .7z file, running a 7-Zip test command.

build.gradle.bruno-reference [159-186]

 // Determine filename from URL
 def filename = untouchedUrl.substring(untouchedUrl.lastIndexOf('/') + 1)
 def downloadDir = file(bundleTmpDownloadPath)
 downloadDir.mkdirs()
 
 def downloadedFile = file("${downloadDir}/${filename}")
 
 // Download if not already present
 if (!downloadedFile.exists()) {
     println "  Downloading to: ${downloadedFile}"
     try {
         ant.get(src: untouchedUrl, dest: downloadedFile, verbose: true)
         println "  Download complete from modules-untouched"
     } catch (Exception e) {
-        throw new GradleException("""
-            Failed to download from modules-untouched: ${e.message}
-
-            Tried URL: ${untouchedUrl}
-
-            Please verify:
-            1. Version ${version} exists in modules-untouched repository
-            2. The URL is correct in bruno.properties or matches format: bruno-{version}/bruno-{version}-win64.7z
-            3. You have internet connectivity
-        """.stripIndent())
+        throw new GradleException(("Failed to download from modules-untouched: ${e.message}\nTried URL: ${untouchedUrl}").stripIndent())
     }
 } else {
     println "  Using cached file: ${downloadedFile}"
 }
 
+// Basic integrity checks
+if (!downloadedFile.exists() || downloadedFile.length() == 0) {
+    throw new GradleException("Downloaded file is missing or empty: ${downloadedFile}")
+}
+if (downloadedFile.name.toLowerCase().endsWith('.7z')) {
+    def sevenZipPath = find7ZipExecutable()
+    if (sevenZipPath) {
+        println "  Verifying archive integrity with 7-Zip test..."
+        def testCmd = [sevenZipPath.toString(), 't', downloadedFile.absolutePath.toString(), '-y']
+        def testProc = new ProcessBuilder(testCmd as String[]).redirectErrorStream(true).start()
+        def output = new StringBuilder()
+        testProc.inputStream.withReader('UTF-8') { it.eachLine { l -> output.append(l).append('\n') } }
+        def ok = testProc.waitFor(120, java.util.concurrent.TimeUnit.SECONDS) && testProc.exitValue() == 0
+        if (!ok) {
+            throw new GradleException("Archive integrity check failed for ${downloadedFile}:\n${output.toString()}")
+        }
+    } else {
+        println "  Warning: 7-Zip not available to verify archive; proceeding without test."
+    }
+}
+
Suggestion importance[1-10]: 7

__

Why: This is a valuable enhancement for build robustness, as it adds integrity checks for downloaded files, preventing confusing downstream errors from corrupt or partial downloads.

Medium
Suggestions up to commit 14fc857
CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent 7z process hangs

Improve the 7-Zip extraction process by ensuring process streams are closed in a
finally block and adding a timeout to process.waitFor() to prevent potential
deadlocks or hangs.

build.gradle.bruno-reference [247-273]

 if (filename.endsWith('.7z')) {
-    // Try to use 7zip if available
     def sevenZipPath = find7ZipExecutable()
     if (sevenZipPath) {
         def command = [
             sevenZipPath.toString(),
             'x',
             downloadedFile.absolutePath.toString(),
             "-o${extractPath.absolutePath}".toString(),
             '-y'
         ]
         def process = new ProcessBuilder(command as String[])
             .directory(extractPath)
             .redirectErrorStream(true)
             .start()
 
-        process.inputStream.eachLine { line ->
-            if (line.trim()) println "    ${line}"
-        }
-
-        def exitCode = process.waitFor()
-        if (exitCode != 0) {
-            throw new GradleException("7zip extraction failed with exit code: ${exitCode}")
+        try {
+            process.inputStream.withReader { reader ->
+                reader.eachLine { line ->
+                    if (line?.trim()) println "    ${line}"
+                }
+            }
+            // Optional: enforce a max wait to avoid hangs
+            if (!process.waitFor(10, java.util.concurrent.TimeUnit.MINUTES)) {
+                process.destroyForcibly()
+                throw new GradleException("7zip extraction timed out after 10 minutes")
+            }
+            def exitCode = process.exitValue()
+            if (exitCode != 0) {
+                throw new GradleException("7zip extraction failed with exit code: ${exitCode}")
+            }
+        } finally {
+            try { process.inputStream?.close() } catch (ignore) {}
+            try { process.errorStream?.close() } catch (ignore) {}
+            try { process.outputStream?.close() } catch (ignore) {}
         }
     } else {
         throw new GradleException("7zip not found. Please install 7zip or extract manually.")
     }
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out a potential for process deadlocks by not closing streams and adds a timeout to prevent hangs, which are valid and important improvements for handling external processes robustly.

Medium
Validate build.properties presence

Add an explicit check for the existence of build.properties and wrap the file
loading in a try-catch block to provide a clearer error message if the file is
missing or unreadable.

build.gradle.bruno-reference [37-39]

-// Load build properties
+// Load build properties with validation
 def buildProps = new Properties()
-file('build.properties').withInputStream { buildProps.load(it) }
+def buildPropsFile = file('build.properties')
+if (!buildPropsFile.exists()) {
+    throw new GradleException("Required build.properties not found at ${buildPropsFile.absolutePath}. Please create it with bundle.name, bundle.release, bundle.type, and bundle.format.")
+}
+try {
+    buildPropsFile.withInputStream { buildProps.load(it) }
+} catch (Exception e) {
+    throw new GradleException("Failed to read build.properties: ${e.message}", e)
+}
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the build will fail with an unhelpful error if build.properties is missing, and improves robustness by adding an explicit check and better error handling at the point of loading.

Low
General
Validate remote properties download

Add validation to the fetchModulesUntouchedProperties function to check that the
downloaded properties file is not empty and contains at least one property
before it is used.

build.gradle.bruno-reference [106-128]

 def fetchModulesUntouchedProperties() {
     def propsUrl = "https://raw.githubusercontent.com/Bearsampp/modules-untouched/main/modules/bruno.properties"
-    
+
     println "Fetching bruno.properties from modules-untouched repository..."
     println "  URL: ${propsUrl}"
-    
+
     def tempFile = file("${bundleTmpDownloadPath}/bruno-untouched.properties")
     tempFile.parentFile.mkdirs()
-    
+
     try {
         ant.get(src: propsUrl, dest: tempFile, verbose: false, ignoreerrors: false)
-        
+
+        if (!tempFile.exists() || tempFile.length() == 0) {
+            throw new IOException("Downloaded properties file is empty")
+        }
+
         def props = new Properties()
         tempFile.withInputStream { props.load(it) }
-        
+
+        if (props.isEmpty()) {
+            throw new IOException("No versions found in downloaded properties")
+        }
+
         println "  ✓ Successfully loaded ${props.size()} versions from modules-untouched"
         return props
     } catch (Exception e) {
-        println "  ✗ Warning: Could not fetch bruno.properties from modules-untouched: ${e.message}"
+        println "  ✗ Warning: Could not fetch usable bruno.properties from modules-untouched: ${e.message}"
         println "  Will fall back to standard URL format if needed"
         return null
     }
 }
Suggestion importance[1-10]: 7

__

Why: This suggestion improves the robustness of fetching remote properties by adding checks for an empty file and empty properties, preventing the build from proceeding with invalid data from a failed download.

Medium
Suggestions up to commit 520124b
CategorySuggestion                                                                                                                                    Impact
Possible issue
Quote paths in 7-Zip command

Quote the file and output paths in the 7-Zip extraction command to correctly
handle spaces and special characters.

build.gradle.bruno-reference [249-273]

 def sevenZipPath = find7ZipExecutable()
 if (sevenZipPath) {
+    // Quote paths to handle spaces/special characters safely
     def command = [
         sevenZipPath.toString(),
         'x',
-        downloadedFile.absolutePath.toString(),
-        "-o${extractPath.absolutePath}".toString(),
+        "\"${downloadedFile.absolutePath}\"",
+        "\"-o${extractPath.absolutePath}\"",
         '-y'
     ]
     def process = new ProcessBuilder(command as String[])
         .directory(extractPath)
         .redirectErrorStream(true)
         .start()
 
     process.inputStream.eachLine { line ->
         if (line.trim()) println "    ${line}"
     }
 
     def exitCode = process.waitFor()
     if (exitCode != 0) {
         throw new GradleException("7zip extraction failed with exit code: ${exitCode}")
     }
 } else {
     throw new GradleException("7zip not found. Please install 7zip or extract manually.")
 }
Suggestion importance[1-10]: 6

__

Why: This is a valid robustness improvement that prevents the build from failing if file paths contain spaces, which is a common scenario on Windows environments.

Low
Quote 7-Zip add command paths

Quote the archive and input paths in the 7-Zip compression command to correctly
handle spaces and special characters.

build.gradle.bruno-reference [596-644]

 // Build archive filename
 def destFile = file("${buildBinsPath}/bearsampp-${bundleName}-${bundleVersion}-${bundleRelease}")
 
-// Compress based on format
 if (bundleFormat == '7z') {
-    // 7z format
     def archiveFile = file("${destFile}.7z")
     if (archiveFile.exists()) {
         delete archiveFile
     }
 
     println "Compressing ${bundleName}${bundleVersion} to ${archiveFile.name}..."
 
-    // Find 7z executable
     def sevenZipExe = find7ZipExecutable()
     if (!sevenZipExe) {
         throw new GradleException("7-Zip not found. Please install 7-Zip or set 7Z_HOME environment variable.")
     }
 
     println "Using 7-Zip: ${sevenZipExe}"
 
-    // Create 7z archive
+    // Quote archive path and input path to handle spaces
     def command = [
-        sevenZipExe,
+        sevenZipExe.toString(),
         'a',
         '-t7z',
-        archiveFile.absolutePath.toString(),
-        '.'
+        "\"${archiveFile.absolutePath}\"",
+        "\".\""
     ]
 
     def process = new ProcessBuilder(command as String[])
         .directory(brunoPrepPath)
         .redirectErrorStream(true)
         .start()
 
     process.inputStream.eachLine { line ->
         if (line.trim()) println "  ${line}"
     }
 
     def exitCode = process.waitFor()
     if (exitCode != 0) {
         throw new GradleException("7zip compression failed with exit code: ${exitCode}")
     }
 
     println "Archive created: ${archiveFile}"
 
-    // Generate hash files
     println "Generating hash files..."
     generateHashFiles(archiveFile)
Suggestion importance[1-10]: 6

__

Why: This is a valid robustness improvement that prevents the build from failing if file paths contain spaces, which is a common scenario on Windows environments.

Low
General
Add network timeout to downloads

Add a maxtime attribute to the ant.get call to set a network timeout, preventing
the build from hanging indefinitely on network issues.

build.gradle.bruno-reference [106-128]

-// Function to fetch bruno.properties from modules-untouched repository
-// This is the primary source for version information when not in releases.properties
 def fetchModulesUntouchedProperties() {
     def propsUrl = "https://raw.githubusercontent.com/Bearsampp/modules-untouched/main/modules/bruno.properties"
     
     println "Fetching bruno.properties from modules-untouched repository..."
     println "  URL: ${propsUrl}"
     
     def tempFile = file("${bundleTmpDownloadPath}/bruno-untouched.properties")
     tempFile.parentFile.mkdirs()
     
     try {
-        ant.get(src: propsUrl, dest: tempFile, verbose: false, ignoreerrors: false)
+        // Set a max time (in seconds) to avoid indefinite hangs on bad networks
+        ant.get(src: propsUrl, dest: tempFile, verbose: false, ignoreerrors: false, maxtime: 60)
         
         def props = new Properties()
         tempFile.withInputStream { props.load(it) }
         
         println "  ✓ Successfully loaded ${props.size()} versions from modules-untouched"
         return props
     } catch (Exception e) {
         println "  ✗ Warning: Could not fetch bruno.properties from modules-untouched: ${e.message}"
         println "  Will fall back to standard URL format if needed"
         return null
     }
 }
Suggestion importance[1-10]: 5

__

Why: This suggestion improves the build's resilience to network issues by adding a timeout to the ant.get call, preventing indefinite hangs and making the build more robust.

Low
Suggestions up to commit ac3bd8a
CategorySuggestion                                                                                                                                    Impact
Possible issue
Enable fallback when local file missing

Modify the downloadAndExtractBruno function to handle a missing
releases.properties file gracefully, allowing the build to proceed with remote
fallback logic instead of throwing an exception.

build.gradle.bruno-reference [194-197]

+def releases = new Properties()
+def downloadUrl = null
+
 def releasesFile = file('releases.properties')
-if (!releasesFile.exists()) {
-    throw new GradleException("releases.properties not found")
+if (releasesFile.exists()) {
+    releasesFile.withInputStream { releases.load(it) }
+    downloadUrl = releases.getProperty(version)
+} else {
+    println "releases.properties not found, will attempt remote fallback (modules-untouched) and constructed URL."
 }
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical bug where the build fails if releases.properties is missing, which prevents the documented 3-tier fallback mechanism from working.

High
General
Add ZIP fallback if 7-Zip missing

In the release task, add a fallback to create a ZIP archive if the bundle.format
is 7z but the 7-Zip executable cannot be found, preventing a hard build failure.

build.gradle.bruno-reference [610-613]

 def sevenZipExe = find7ZipExecutable()
 if (!sevenZipExe) {
-    throw new GradleException("7-Zip not found. Please install 7-Zip or set 7Z_HOME environment variable.")
+    println "[WARNING] 7-Zip not found. Falling back to ZIP format for this build."
+    // Fallback to ZIP archive creation
+    def zipArchive = file("${destFile}.zip")
+    if (zipArchive.exists()) {
+        delete zipArchive
+    }
+    ant.zip(destfile: zipArchive, basedir: brunoPrepPath)
+    println "Archive created (fallback ZIP): ${zipArchive}"
+    println "Generating hash files..."
+    generateHashFiles(zipArchive)
+    return
 }
Suggestion importance[1-10]: 7

__

Why: This is a valuable suggestion that improves the build script's resilience by adding a fallback to ZIP compression if 7-Zip is not found, which is especially useful for non-Windows or CI environments.

Medium
✅ Suggestions up to commit f9abd59
CategorySuggestion                                                                                                                                    Impact
High-level
Extract build logic into a reusable Gradle plugin

Extract the common build logic from the large, monolithic build script into a
reusable custom Gradle plugin. This will reduce code duplication and improve
maintainability across modules.

Examples:

build.gradle.bruno-reference [1-1183]
/*
 * Bearsampp Module Bruno - Gradle Build
 *
 * This is a 100% Gradle build configuration for the Bruno module.
 * It handles downloading, extracting, and packaging Bruno releases.
 *
 * VERSION RESOLUTION STRATEGY (3-tier fallback):
 *   1. Local releases.properties (primary source)
 *   2. Remote modules-untouched bruno.properties (automatic fallback)
 *      URL: https://github.com/Bearsampp/modules-untouched/blob/main/modules/bruno.properties

 ... (clipped 1173 lines)

Solution Walkthrough:

Before:

// In module-bruno/build.gradle.bruno-reference
plugins { id 'base' }

// ~100 lines of path and property configuration for bruno
ext {
  bundleName = 'bruno'
  // ... many other paths
}

// ~100 lines of helper functions for downloading/extracting bruno
def downloadAndExtractBruno(String version, File destDir) {
  // ... complex logic to download from github, fallback, etc.
}

// ~200 lines for the 'release' task for bruno
tasks.register('release') {
  doLast {
    // ... complex logic to find binaries, copy files, run 7zip
  }
}

// ~500 more lines for other tasks (info, verify, clean, etc.) for bruno
// ... and the same ~1000 lines are copied and adapted in other modules

After:

// In buildSrc/src/main/groovy/com.bearsampp.module-build.gradle
class ModuleBuildPlugin implements Plugin<Project> {
    void apply(Project project) {
        // ~1000 lines of generic build logic extracted here
        // - Generic download/extract functions
        // - Generic release task
        // - Generic info, verify tasks

        project.tasks.register('release', ReleaseTask) {
            // Task is configured by an extension
        }
    }
}

// In module-bruno/build.gradle
plugins {
    id 'com.bearsampp.module-build'
}

// Configure the plugin for this specific module
moduleBuild {
    bundleName = 'bruno'
    bundleType = 'tools'
    executableName = 'bruno.exe'
}
Suggestion importance[1-10]: 9

__

Why: This is a critical architectural suggestion that correctly identifies the major flaw of code duplication in the monolithic build script and proposes a scalable, maintainable solution using a custom Gradle plugin.

High
Security
Prevent path traversal security vulnerability

Add input validation for the bundleVersion project property to prevent a path
traversal security vulnerability. The validation should ensure the version
string contains only safe characters.

build.gradle.bruno-reference [422-427]

 doLast {
     def versionToBuild = versionProperty
+
+    if (versionToBuild && (versionToBuild.contains('..') || !versionToBuild.matches("^[\\w.-]+\$"))) {
+        throw new GradleException("Invalid characters in bundleVersion. Only alphanumeric characters, dots, and hyphens are allowed.")
+    }
 
     if (!versionToBuild) {
         // Interactive mode - prompt for version
         def availableVersions = getAvailableVersions()
Suggestion importance[1-10]: 9

__

Why: This is a valid and critical security suggestion that addresses a potential path traversal vulnerability when a malicious bundleVersion is passed, which could lead to file system manipulation outside the intended directories.

High
General
Improve executable discovery from PATH

Improve the reliability of finding the 7z.exe executable from the system PATH.
Instead of using the first result from the where command, iterate through all
returned paths and use the first one that executes successfully.

build.gradle.bruno-reference [701-708]

 def process = ['where', '7z.exe'].execute()
 process.waitFor()
 if (process.exitValue() == 0) {
     def output = process.text.trim()
     if (output) {
-        return output.split('\n')[0].trim()
+        // Find the first valid 7z.exe that can execute
+        for (String path in output.split('\n')) {
+            def candidate = path.trim()
+            try {
+                def testProcess = [candidate, '--help'].execute()
+                testProcess.waitFor()
+                if (testProcess.exitValue() == 0) {
+                    return candidate
+                }
+            } catch (Exception ignored) {
+                // Ignore if this path is not a valid executable
+            }
+        }
     }
 }
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential issue where the first 7z.exe in the PATH might be non-functional and proposes a robust solution to iterate and test all found executables, improving the build script's reliability.

Low
Fix corrupted characters in documentation

Fix corrupted Unicode characters in an example output in MIGRATION-SUMMARY.md by
replacing them with ASCII characters, ensuring consistency with the project's
design decisions.

MIGRATION-SUMMARY.md [239-241]

-╔════════════════════════════════════════════════════════════════════════════╗
-║                    Build Environment Verification                          ║
-╚═══════════════════════════════════════════════════════════════════════════╝
+============================================================================
+|                    Build Environment Verification                          |
+============================================================================
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies corrupted Unicode characters in the documentation and proposes a fix that aligns with the project's stated goal of using ASCII for compatibility.

Low
Update documentation for consistency
Suggestion Impact:The commit updated example output blocks to use ASCII separators instead of Unicode box-drawing characters in the README, aligning with the suggestion for documentation consistency.

code diff:

-Expected output:
-
-```
-╔════════════════════════════════════════════════════════════════════════════╗
-║                    Build Environment Verification                          ║
-╚════════════════════════════════════════════════════════════════════════════╝
-
-Environment Check Results:
-────────────────────────────────────────────────────────────────────────────
-  ✓ PASS     Java 8+
-  ✓ PASS     build.gradle
-  ✓ PASS     build.properties
-  ✓ PASS     releases.properties
-  ✓ PASS     settings.gradle
-  ✓ PASS     bin/ directory
-  ✓ PASS     7z command
-────────────────────────────────────────────────────────────────────────────
-
-[SUCCESS] All checks passed! Build environment is ready.
-```
-
-#### 4. Build Release
-
-```bash
-gradle release -PbundleVersion=1.6.39
-```
-
-Expected output:
-
-```
-╔════════════════════════════════════════════════════════════════════════════╗
-║                         Release Build                                      ║
-╚════════════════════════════════════════════════════════════════════════════╝
-
-Building release for memcached version 1.6.39...
-
-Bundle path: E:/Bearsampp-development/module-memcached/bin/memcached1.6.39
-
-Preparing bundle...
-Copying bundle files...
-Creating archive: bearsampp-memcached-1.6.39-2025.8.20.7z
-
-╔════════════════════════════════════════════════════════════════════════════╗
-║                      Release Build Completed                               ║
-╚════════════════════════════════════════════════════════════════════════════╝
-
-Release package created:
-  C:\Users\troy\Bearsampp-build\release\bearsampp-memcached-1.6.39-2025.8.20.7z
-
-Package size: 0.45 MB

Update an example output in .gradle-docs/README.md to use ASCII characters
instead of Unicode, ensuring it accurately reflects the build system's output
and maintains documentation consistency.

.gradle-docs/README.md [216-219]

 Expected output:
 

-╔════════════════════════════════════════════════════════════════════════════╗
-║ Build Environment Verification ║
-╚════════════════════════════════════════════════════════════════════════════╝
+============================================================================
+| Build Environment Verification |
+============================================================================







<details><summary>Suggestion importance[1-10]: 6</summary>

__

Why: The suggestion correctly points out an inconsistency in the documentation, as the example output uses Unicode characters while the project standard is ASCII, improving documentation accuracy.


</details></details></td><td align=center>Low

</td></tr><tr><td>



<details><summary>✅ <s>Use relative path in documentation<!-- not_implemented --></s></summary>

___

<details><summary><b>Suggestion Impact:</b></summary>The commit removed the absolute path example line and reworked the README, effectively eliminating the hardcoded path from the documentation.


code diff:

```diff
-### Quick Release
-
-```bash
-# 1. Update build.properties with release date
-# 2. Verify environment
-gradle verify
-
-# 3. Build release
-gradle release -PbundleVersion=1.6.39
-
-# 4. Test package
-7z t C:\Users\troy\Bearsampp-build\release\bearsampp-memcached-1.6.39-2025.8.20.7z
-
-# 5. Create Git tag and push
-git tag -a 2025.8.20 -m "Release 2025.8.20"
-git push origin main --tags
-
-# 6. Create GitHub release and upload package
-# 7. Update releases.properties
-```
-
-See [RELEASE-PROCESS.md](.gradle-docs/RELEASE-PROCESS.md) for detailed instructions.

Replace the hardcoded absolute path in the README.md example with a generic,
relative path to make the documentation universally applicable.

README.md [120-121]

 # 4. Test package
-7z t C:\Users\troy\Bearsampp-build\release\bearsampp-memcached-1.6.39-2025.8.20.7z
+# The release package is created in the 'build/release' directory
+7z t build/release/bearsampp-memcached-1.6.39-2025.8.20.7z
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies a hardcoded, user-specific path in the documentation, and replacing it with a generic path improves its usability for other developers.

Low

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Nov 15, 2025

PR Reviewer Guide 🔍

(Review updated until commit fb8a8a2)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 Security concerns

Remote download integrity:
The build downloads binaries over HTTPS and immediately consumes them without integrity verification. Consider adding expected checksums/signatures (e.g., from releases.properties) and verifying before extraction; also sanitize constructed URLs to reduce risk of tampering.

⚡ Recommended focus areas for review

Windows-only Assumptions

Several helpers and task logic hardcode Windows tools and paths (7z.exe discovery, 'where' command, bruno.exe lookup) which may break on non-Windows environments despite documentation stating cross-platform support. Validate portability or guard with OS checks and alternatives.

// Helper function to find 7-Zip executable
def find7ZipExecutable() {
    // Check environment variable
    def sevenZipHome = System.getenv('7Z_HOME')
    if (sevenZipHome) {
        def exe = file("${sevenZipHome}/7z.exe")
        if (exe.exists()) {
            return exe.absolutePath
        }
    }

    // Check common installation paths
    def commonPaths = [
        'C:/Program Files/7-Zip/7z.exe',
        'C:/Program Files (x86)/7-Zip/7z.exe',
        'D:/Program Files/7-Zip/7z.exe',
        'D:/Program Files (x86)/7-Zip/7z.exe'
    ]

    for (path in commonPaths) {
        def exe = file(path)
        if (exe.exists()) {
            return exe.absolutePath
        }
    }

    // Try to find in PATH
    try {
        def process = ['where', '7z.exe'].execute()
        process.waitFor()
        if (process.exitValue() == 0) {
            def output = process.text.trim()
            if (output) {
                return output.split('\n')[0].trim()
            }
        }
    } catch (Exception e) {
        // Ignore
    }

    return null
}
Network Robustness

Remote fetches use ant.get without retries/timeouts and treat modules-untouched as primary fallback. Add retry/backoff, checksum verification, and clearer failure handling to avoid flaky builds and partial downloads in cache.

// This is the primary source for version information when not in releases.properties
def fetchModulesUntouchedProperties() {
    def propsUrl = "https://raw.githubusercontent.com/Bearsampp/modules-untouched/main/modules/bruno.properties"

    println "Fetching bruno.properties from modules-untouched repository..."
    println "  URL: ${propsUrl}"

    def tempFile = file("${bundleTmpDownloadPath}/bruno-untouched.properties")
    tempFile.parentFile.mkdirs()

    try {
        ant.get(src: propsUrl, dest: tempFile, verbose: false, ignoreerrors: false)

        def props = new Properties()
        tempFile.withInputStream { props.load(it) }

        println "  ✓ Successfully loaded ${props.size()} versions from modules-untouched"
        return props
    } catch (Exception e) {
        println "  ✗ Warning: Could not fetch bruno.properties from modules-untouched: ${e.message}"
        println "  Will fall back to standard URL format if needed"
        return null
    }
}

// Function to download from modules-untouched repository
def downloadFromModulesUntouched(String version, File destDir) {
    println "Version ${version} not found in releases.properties"
    println "Checking modules-untouched repository..."

    // First, try to fetch bruno.properties from modules-untouched
    def untouchedProps = fetchModulesUntouchedProperties()
    def untouchedUrl = null

    if (untouchedProps) {
        untouchedUrl = untouchedProps.getProperty(version)
        if (untouchedUrl) {
            println "Found version ${version} in modules-untouched bruno.properties"
            println "Downloading from:"
            println "  ${untouchedUrl}"
        } else {
            println "Version ${version} not found in modules-untouched bruno.properties"
            println "Attempting to construct URL based on standard format..."
            // Fallback to constructed URL
            untouchedUrl = "https://github.com/Bearsampp/modules-untouched/releases/download/bruno-${version}/bruno-${version}-win64.7z"
            println "  ${untouchedUrl}"
        }
    } else {
        println "Could not fetch bruno.properties, using standard URL format..."
        // Fallback to constructed URL
        untouchedUrl = "https://github.com/Bearsampp/modules-untouched/releases/download/bruno-${version}/bruno-${version}-win64.7z"
        println "  ${untouchedUrl}"
    }

    // Determine filename from URL
    def filename = untouchedUrl.substring(untouchedUrl.lastIndexOf('/') + 1)
    def downloadDir = file(bundleTmpDownloadPath)
    downloadDir.mkdirs()

    def downloadedFile = file("${downloadDir}/${filename}")

    // Download if not already present
    if (!downloadedFile.exists()) {
        println "  Downloading to: ${downloadedFile}"
        try {
            ant.get(src: untouchedUrl, dest: downloadedFile, verbose: true)
            println "  Download complete from modules-untouched"
        } catch (Exception e) {
            throw new GradleException("""
                Failed to download from modules-untouched: ${e.message}

                Tried URL: ${untouchedUrl}

                Please verify:
                1. Version ${version} exists in modules-untouched repository
                2. The URL is correct in bruno.properties or matches format: bruno-{version}/bruno-{version}-win64.7z
                3. You have internet connectivity
            """.stripIndent())
        }
    } else {
        println "  Using cached file: ${downloadedFile}"
    }

    return downloadedFile
}
Documentation Mismatch

Docs repeatedly reference Memcached while this module/build file targets Bruno; ensure module names, paths, and examples match the actual module to avoid user confusion.

# Bearsampp Module Memcached - Gradle Build Documentation

## Table of Contents

- [Overview](#overview)
- [Quick Start](#quick-start)
- [Installation](#installation)
- [Build Tasks](#build-tasks)
- [Configuration](#configuration)
- [Architecture](#architecture)
- [Troubleshooting](#troubleshooting)
- [Migration Guide](#migration-guide)

---

## Overview

The Bearsampp Module Memcached project has been converted to a **pure Gradle build system**, replacing the legacy Ant build configuration. This provides:

- **Modern Build System**     - Native Gradle tasks and conventions
- **Better Performance**       - Incremental builds and caching
- **Simplified Maintenance**   - Pure Groovy/Gradle DSL
- **Enhanced Tooling**         - IDE integration and dependency management
- **Cross-Platform Support**   - Works on Windows, Linux, and macOS

### Project Information

| Property          | Value                                    |
|-------------------|------------------------------------------|
| **Project Name**  | module-memcached                         |
| **Group**         | com.bearsampp.modules                    |
| **Type**          | Memcached Module Builder                 |
| **Build Tool**    | Gradle 8.x+                              |
| **Language**      | Groovy (Gradle DSL)                      |

---

## Quick Start

### Prerequisites

| Requirement       | Version       | Purpose                                  |
|-------------------|---------------|------------------------------------------|
| **Java**          | 8+            | Required for Gradle execution            |
| **Gradle**        | 8.0+          | Build automation tool                    |
| **7-Zip**         | Latest        | Archive creation (.7z format)            |

### Basic Commands

```bash
# Display build information
gradle info

# List all available tasks
gradle tasks

# Verify build environment
gradle verify

# Build a release (interactive)
gradle release

# Build a specific version (non-interactive)
gradle release -PbundleVersion=1.6.29

# Build all versions
gradle releaseAll

# Clean build artifacts
gradle clean

Installation

1. Clone the Repository

git clone https://github.com/bearsampp/module-memcached.git

@jwaisner jwaisner merged commit ba1eb4c into main Nov 19, 2025
@jwaisner jwaisner deleted the gradle-convert branch November 19, 2025 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement ✨ Improve program

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants