feat: add self-update command and unify versioning to v9.1.0#43
feat: add self-update command and unify versioning to v9.1.0#43
Conversation
github-actions
bot
changed the title
🤖 Review Buddy - General Code Review
Oho, @AnkanSaha! Kya baat hai! 9.0.1 se 9.1.0 pe jump maar di? Version number badhane se code quality nahi badhti bhai. Ye 'self-update' feature dekh ke toh lag raha hai ki tumne bas 'curl | bash' ko Go code mein lapet ke bech diya hai. Bhai, version strings ko har file mein manually change kar rahe ho? Documentation, Extension, VERSION file, main.go, repl.go... Arre bhai, DRY (Don't Repeat Yourself) ka naam suna hai ya tum bas copy-paste engineering mein PhD kar rahe ho? Checksum verification add toh kiya hai, chalo thoda dimaag toh lagaya, par shell scripts mein itna hardcoded logic? Aur Go code mein command line parsing ke naam pe ye kya mazaak hai? Code Quality Score: 3/10 (Sirf isliye kyunki checksum check kiya, warna toh 1 milta). Generated by Review Buddy | Tone: roast | Language: hinglish |
⚡ Review Buddy - Performance Analysis
Bhai, performance ki toh tumne 'Dhoom 3' bana di hai—sirf dikhava, logic zero!
Summary: Tumhara code performance ke maamle mein 'Race 3' hai. Dekhne mein fast lagta hai, par actually bohot slow aur bloated hai. Generated by Review Buddy | Tone: roast | Language: hinglish |
🔐 Review Buddy - Security Audit
Arre bhai bhai bhai! Security ke naam pe toh tumne darwaze khulle chhod diye hain aur bahar 'Chori Mana Hai' ka board laga diya hai.
Generated by Review Buddy | Tone: roast | Language: hinglish |
📊 Review Buddy - Code Quality & Maintainability Analysis
🎯 Overall Benchmark: 40/100 (Poor)Quality? Bhai, quality toh 'Gunda' movie ki script jaisi hai—legendary for all the wrong reasons.
Refactoring suggestion: Ek Generated by Review Buddy | Tone: roast | Language: hinglish |
💡 Review Buddy - Best Practices & Alternative Suggestions
Bhai, ye lo thoda 'Gyaan' (Improvement tips): 1. Versioning - Don't HardcodeCurrent: fmt.Println("║ BanglaCode v9.1.0 ║")Better: // main.go
var Version = "9.1.0"
fmt.Printf("║ BanglaCode v%s ║\n", Version)Why: Ek hi jagah change karna padega. 2. Argument ParsingCurrent: if len(os.Args) == 2 && update.CheckUpdateCommand(os.Args[1:], "update")Better: if len(os.Args) > 1 && os.Args[1] == "update" {
update.Updater()
return
}Why: Clarity! Loop chalane ki zaroorat hi nahi hai agar command fix hai. 3. Error PrintingCurrent: fmt.Fprintf(os.Stderr, "\n\033[31mError: Update failed: %v\033[0m\n", err)Better: 4. PowerShell CleanupCurrent: Remove-Item $TMP_FILE, $CHECKSUM_FILE -Force -ErrorAction SilentlyContinueBetter: Generated by Review Buddy | Tone: roast | Language: hinglish |
|
github-actions
bot
changed the title
github-actions
bot
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds an auto-update command to the BanglaCode CLI and increments the version from 9.0.1 to 9.1.0 across all project modules. The implementation uses a controversial curl | bash pattern to execute remote installation scripts, which poses significant security risks as acknowledged in the PR description. Positively, the install scripts themselves (install.sh and install.ps1) have been enhanced with SHA256 checksum verification to validate downloaded binaries, partially mitigating supply chain risks.
Changes:
- Added new
Updatepackage with OS-specific update commands that pipe remote scripts to shell interpreters - Bumped version to 9.1.0 in VERSION file, both package.json files, repl.go, and main.go
- Enhanced install scripts with SHA256 checksum verification for downloaded binaries
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Update/Updater.go | New package implementing auto-update via remote script execution (curl|bash pattern) |
| main.go | Integrated update command handling, updated version strings to 9.1.0, added help text for update command |
| src/repl/repl.go | Bumped Version constant to 9.1.0 |
| VERSION | Updated version to 9.1.0 |
| Extension/package.json | Updated version to 9.1.0 for VS Code extension |
| Documentation/package.json | Updated version to 9.1.0 for documentation site |
| Scripts/install.sh | Added SHA256 checksum download and verification before binary installation |
| Scripts/install.ps1 | Added SHA256 checksum download and verification before binary installation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| // Update checks if the update command was passed and executes the appropriate update script | ||
| func Updater() { |
There was a problem hiding this comment.
The function comment says "Update checks if the update command was passed" but the function doesn't check for command arguments - it directly executes the update. The comment should describe what the function actually does: executing the update process for the detected OS. Consider renaming this function to "Execute" or "Run" to better reflect its behavior, or update the comment to match the actual implementation.
| func CheckUpdateCommand(args []string, target string) bool { | ||
| for _, arg := range args { | ||
| if strings.ToLower(arg) == target { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
This function accepts a generic "target" parameter but it's only ever called with the hardcoded value "update". This adds unnecessary complexity. Consider simplifying the function signature to not take a target parameter, or document why this flexibility is needed if there are plans to support other commands in the future.
| } | ||
|
|
||
| // checkUpdateCommand checks if the "update" argument is present in the command-line args | ||
| func CheckUpdateCommand(args []string, target string) bool { |
There was a problem hiding this comment.
The CheckUpdateCommand function is exported (starts with capital letter) but only used within main.go. Based on Go conventions and examining similar system packages in this codebase (like src/evaluator/builtins/system/), internal helper functions should be unexported unless they need to be part of the public API. Consider making this function unexported by renaming it to "checkUpdateCommand".
|
|
||
| // Check for flags | ||
| // Check for commands and flags | ||
| if len(os.Args) == 2 && update.CheckUpdateCommand(os.Args[1:], "update") { |
There was a problem hiding this comment.
The condition checks both that len(os.Args) == 2 AND that CheckUpdateCommand returns true. The CheckUpdateCommand function already validates the argument value, making the length check partially redundant with how it's being called. More importantly, this means "banglacode -h update" or "banglacode update extra-arg" would not trigger the update command, which may be unexpected behavior. Consider either: 1) Simplifying to just check for the "update" command regardless of other arguments, or 2) Explicitly validating that "update" is the only argument and showing an error for extra arguments.
| if len(os.Args) == 2 && update.CheckUpdateCommand(os.Args[1:], "update") { | |
| if update.CheckUpdateCommand(os.Args[1:], "update") { | |
| if len(os.Args) != 2 { | |
| fmt.Fprintln(os.Stderr, "Error: 'update' command does not accept additional arguments.") | |
| return | |
| } |
| package update | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
| "os/exec" | ||
| "runtime" | ||
| "strings" | ||
| ) | ||
|
|
||
| // Update commands for each operating system | ||
| var updateCommands = map[string]updateCommand{ | ||
| "linux": { | ||
| shell: "bash", | ||
| shellArg: "-c", | ||
| script: "curl -fsSL https://raw.githubusercontent.com/nexoral/BanglaCode/main/Scripts/install.sh | bash", | ||
| }, | ||
| "darwin": { | ||
| shell: "bash", | ||
| shellArg: "-c", | ||
| script: "curl -fsSL https://raw.githubusercontent.com/nexoral/BanglaCode/main/Scripts/install.sh | bash", | ||
| }, | ||
| "windows": { | ||
| shell: "powershell", | ||
| shellArg: "-Command", | ||
| script: "irm https://raw.githubusercontent.com/nexoral/BanglaCode/main/Scripts/install.ps1 | iex", | ||
| }, | ||
| } | ||
|
|
||
| type updateCommand struct { | ||
| shell string | ||
| shellArg string | ||
| script string | ||
| } | ||
|
|
||
| // Update checks if the update command was passed and executes the appropriate update script | ||
| func Updater() { | ||
| fmt.Println("\033[1;36m╔════════════════════════════════════════════════════════╗") | ||
| fmt.Println("║ Updating BanglaCode... ║") | ||
| fmt.Println("╚════════════════════════════════════════════════════════╝\033[0m") | ||
| fmt.Println() | ||
|
|
||
| // Detect operating system | ||
| currentOS := runtime.GOOS | ||
| cmd, exists := updateCommands[currentOS] | ||
|
|
||
| if !exists { | ||
| fmt.Fprintf(os.Stderr, "\033[31mError: Unsupported operating system '%s'\033[0m\n", currentOS) | ||
| fmt.Fprintf(os.Stderr, "Supported systems: Linux, macOS (darwin), Windows\n") | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| fmt.Printf("Detected OS: \033[1;32m%s\033[0m\n", currentOS) | ||
| fmt.Printf("Running update command...\n\n") | ||
|
|
||
| // Execute the appropriate update command | ||
| command := exec.Command(cmd.shell, cmd.shellArg, cmd.script) | ||
| command.Stdout = os.Stdout | ||
| command.Stderr = os.Stderr | ||
|
|
||
| if err := command.Run(); err != nil { | ||
| fmt.Fprintf(os.Stderr, "\n\033[31mError: Update failed: %v\033[0m\n", err) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| fmt.Println("\n\033[1;32m✓ Update completed successfully!\033[0m") | ||
| } | ||
|
|
||
| // checkUpdateCommand checks if the "update" argument is present in the command-line args | ||
| func CheckUpdateCommand(args []string, target string) bool { | ||
| for _, arg := range args { | ||
| if strings.ToLower(arg) == target { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
The new Update package lacks test coverage. Given that this codebase has comprehensive test coverage for other packages (as seen in the test directory), the update functionality should also have tests. Consider adding tests that: 1) Verify CheckUpdateCommand correctly identifies the update argument, 2) Mock the exec.Command to test different OS scenarios without actually executing shell commands, 3) Test error handling for unsupported operating systems. This is particularly important for security-sensitive code that executes external commands.
| fmt.Println("\n\033[1;32m✓ Update completed successfully!\033[0m") | ||
| } | ||
|
|
||
| // checkUpdateCommand checks if the "update" argument is present in the command-line args |
There was a problem hiding this comment.
The comment says "checkUpdateCommand" (lowercase) but the function is actually named "CheckUpdateCommand" (uppercase/exported). The comment should match the actual function name for consistency and to avoid confusion.
| // checkUpdateCommand checks if the "update" argument is present in the command-line args | |
| // CheckUpdateCommand checks if the "update" argument is present in the command-line args |
| func Updater() { | ||
| fmt.Println("\033[1;36m╔════════════════════════════════════════════════════════╗") | ||
| fmt.Println("║ Updating BanglaCode... ║") | ||
| fmt.Println("╚════════════════════════════════════════════════════════╝\033[0m") | ||
| fmt.Println() | ||
|
|
||
| // Detect operating system | ||
| currentOS := runtime.GOOS | ||
| cmd, exists := updateCommands[currentOS] | ||
|
|
||
| if !exists { | ||
| fmt.Fprintf(os.Stderr, "\033[31mError: Unsupported operating system '%s'\033[0m\n", currentOS) | ||
| fmt.Fprintf(os.Stderr, "Supported systems: Linux, macOS (darwin), Windows\n") | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| fmt.Printf("Detected OS: \033[1;32m%s\033[0m\n", currentOS) | ||
| fmt.Printf("Running update command...\n\n") | ||
|
|
||
| // Execute the appropriate update command | ||
| command := exec.Command(cmd.shell, cmd.shellArg, cmd.script) | ||
| command.Stdout = os.Stdout | ||
| command.Stderr = os.Stderr | ||
|
|
||
| if err := command.Run(); err != nil { | ||
| fmt.Fprintf(os.Stderr, "\n\033[31mError: Update failed: %v\033[0m\n", err) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| fmt.Println("\n\033[1;32m✓ Update completed successfully!\033[0m") | ||
| } |
There was a problem hiding this comment.
The implementation calls os.Exit(1) directly on error, which prevents proper cleanup and makes the function difficult to test. This pattern is inconsistent with Go best practices for library code. Consider returning an error value instead and letting the caller (main.go) decide when to exit. This would make the update package more testable and reusable.
| script: "curl -fsSL https://raw.githubusercontent.com/nexoral/BanglaCode/main/Scripts/install.sh | bash", | ||
| }, | ||
| "darwin": { | ||
| shell: "bash", | ||
| shellArg: "-c", | ||
| script: "curl -fsSL https://raw.githubusercontent.com/nexoral/BanglaCode/main/Scripts/install.sh | bash", | ||
| }, | ||
| "windows": { | ||
| shell: "powershell", | ||
| shellArg: "-Command", | ||
| script: "irm https://raw.githubusercontent.com/nexoral/BanglaCode/main/Scripts/install.ps1 | iex", | ||
| }, |
There was a problem hiding this comment.
The update mechanism downloads and executes remote scripts without any verification that the script contents match what's expected. While the install scripts themselves now verify checksums of binaries (as seen in Scripts/install.sh), the updater executes the install scripts themselves without any integrity check. An attacker who compromises the GitHub repository or performs a man-in-the-middle attack could inject malicious code that would be executed with the user's privileges. Consider one of these alternatives: 1) Download the script, verify its checksum, then execute it locally, 2) Use a native Go implementation that directly downloads and verifies binaries, or 3) At minimum, document this security risk prominently in user-facing documentation.
| @@ -0,0 +1,77 @@ | |||
| package update | |||
There was a problem hiding this comment.
The directory is named "Update" (capitalized) which is inconsistent with Go naming conventions. All other packages in the codebase use lowercase directory names (e.g., src/repl/, src/parser/, src/evaluator/builtins/crypto/). The directory should be renamed to "update" to match Go conventions and maintain consistency with the rest of the codebase.
| fmt.Println("║ BanglaCode v9.1.0 ║") | ||
| fmt.Println("║ A Programming Language in Bengali (Banglish) ║") | ||
| fmt.Println("╠════════════════════════════════════════════════════════╣\033[0m") | ||
| fmt.Println("\033[1;36m║\033[0m 📦 \033[1mVersion:\033[0m \033[1;32m9.0.1\033[0m \033[1;36m║\033[0m") | ||
| fmt.Println("\033[1;36m║\033[0m 📦 \033[1mVersion:\033[0m \033[1;32m9.1.0\033[0m \033[1;36m║\033[0m") |
There was a problem hiding this comment.
The version strings are hardcoded in the printVersion function. This creates a maintenance burden and risk of version mismatch. Consider using the Version constant from the repl package (repl.Version) instead of hardcoding the version string twice. This would ensure consistency and make version updates less error-prone.
2 participants
Summary
This PR introduces a self-update mechanism for the BanglaCode CLI and bumps the version from 9.0.1 to 9.1.0. It also adds checksum verification to the installation scripts for improved security.
Changes
banglacode updatecommand to trigger the update process.updatepackage in Go that detects the OS and executes the corresponding shell script (install.shorinstall.ps1).package.json,main.go,repl.go, andVERSIONfile.Verification
banglacode updateon Windows/Linux/macOS.banglacode --versionshows9.1.0.