Skip to content

feat(cli): add install scripts for macOS, Linux, and Windows#587

Merged
shadowfax92 merged 3 commits intomainfrom
fix/mar26-cli-updates
Mar 27, 2026
Merged

feat(cli): add install scripts for macOS, Linux, and Windows#587
shadowfax92 merged 3 commits intomainfrom
fix/mar26-cli-updates

Conversation

@shadowfax92
Copy link
Contributor

@shadowfax92 shadowfax92 commented Mar 26, 2026

Summary

  • Add cross-platform install scripts for browseros-cli on macOS, Linux, and Windows
  • Resolve versions from GitHub Releases and download the matching prebuilt archive for the current platform
  • Verify archive integrity against release checksums.txt before install when checksum data is available

Rework

Addressed the PR review feedback by tightening checksum handling and archive extraction behavior instead of widening scope. Windows installs now verify the downloaded zip, bash warns on every skipped verification path, and both scripts tolerate nested archive layouts.

Test plan

  • bash packages/browseros-agent/apps/cli/scripts/install.sh --version 0.1.0 --dir "$TMPDIR/bin"
  • browseros-cli --help after install from the script output path
  • Verified live checksums.txt format for browseros-cli-v0.1.0
  • Run .\install.ps1 -Version "0.1.0" on Windows

Bash script (install.sh) for macOS/Linux and PowerShell script
(install.ps1) for Windows. Both download the correct platform binary
from GitHub Releases with checksum verification, version resolution,
and PATH setup.
@shadowfax92
Copy link
Contributor Author

@greptileai review

@shadowfax92
Copy link
Contributor Author

@claude review

@claude
Copy link

claude bot commented Mar 26, 2026

Claude finished @shadowfax92's task in 1m 25s —— View job


Code Review

  • Gather context and read changed files
  • Review install.sh
  • Review install.ps1
  • Post findings

Overall the scripts are well-structured with good platform detection and argument handling. Here are the findings:


install.sh

Security

  1. Checksum verification silently skipped (lines 93–110): If sha256sum/shasum is unavailable (common in minimal Docker images or CI environments), the binary is installed without integrity verification. This should at minimum print a warning:

    else
      echo "Warning: no sha256sum/shasum found; skipping checksum verification." >&2
    fi

    Fix this →

  2. Checksum download failure is silent (line 94): If the checksums.txt download fails (e.g., the file doesn't exist for a release), the script proceeds without any integrity check. Worth printing a warning here too.

  3. Partial filename match in grep (line 95): grep "$FILENAME" could match other filenames that contain the target filename as a substring. Prefer:

    expected=$(grep " ${FILENAME}$" "${TMPDIR_DL}/checksums.txt" | awk '{print $1}')

    Fix this →

Robustness

  1. GoReleaser archive structure assumption (lines 112–117): The script checks for ${TMPDIR_DL}/${BINARY} directly, but GoReleaser can optionally put the binary inside a directory within the tarball. If that's the case, the binary check will fail. Suggest using find as a fallback:

    BINARY_PATH=$(find "$TMPDIR_DL" -name "$BINARY" -type f | head -1)
    if [[ -z "$BINARY_PATH" ]]; then
      echo "Error: binary not found in archive." >&2; exit 1
    fi
  2. GitHub API rate limiting: No support for GITHUB_TOKEN env var. In CI environments, unauthenticated calls to the GitHub API are rate-limited to 60/hour. Suggest:

    CURL_AUTH=""
    [[ -n "${GITHUB_TOKEN:-}" ]] && CURL_AUTH="-H \"Authorization: token $GITHUB_TOKEN\""
  3. JSON parsing is fragile (lines 48–52): Parsing JSON with grep + sed can break with unexpected whitespace or ordering. Consider checking for jq first and falling back to the current approach if unavailable.

Minor

  1. --help doesn't mention env vars: Not applicable here since bash scripts support params directly, but worth noting if a GITHUB_TOKEN env var is added.

install.ps1

Security

  1. No checksum verification at all: Unlike the bash script, the PowerShell script performs no integrity check on the downloaded zip. This is the more significant gap since Windows users have no way to verify the binary. A Get-FileHash based check should be added.
    Fix this →

Robustness

  1. API pagination (line 34): Invoke-RestMethod defaults to 30 results per page. If browseros-cli releases aren't in the first 30 GitHub releases (possible as other releases accumulate), version resolution will silently fail. Add ?per_page=100 to the URL like the bash script does.
    Fix this →

  2. $TmpDir not uniquely named: $env:TEMP\browseros-cli-install is a fixed path. If two installs run concurrently (e.g., in CI), they'll collide. The bash script uses mktemp -d for uniqueness; consider [System.IO.Path]::GetTempPath() + [System.IO.Path]::GetRandomFileName().

Behavior difference

  1. Auto-modifies PATH vs. just printing a hint: The bash script only prints a PATH hint; the PS1 script automatically modifies the user-level Path environment variable (line 110). This is a different UX contract — users may not expect a silent PATH modification. Consider adding a -AddToPath flag to make it opt-in, or at least print clearly before doing it (currently it does print, but the modification happens regardless).

Minor

  1. No -Help parameter: The bash script has --help; the PS1 script has no equivalent. Adding [switch]$Help and a corresponding usage block would improve consistency.

  2. Invoke-WebRequest -UseBasicParsing (line 80): This flag is deprecated in PowerShell 6+ (though still functional). Not urgent, but a #Requires -Version 5.1 comment at the top would clarify the minimum version expectation.


Summary

Issue Severity File
No checksum verification in PS1 High install.ps1
Checksum silently skipped when tools missing Medium install.sh
Partial grep match on filename Medium install.sh
API pagination (30 vs 100 per page) Medium install.ps1
Fixed temp dir (concurrency issue) Low install.ps1
GoReleaser archive structure assumption Low install.sh
No GITHUB_TOKEN support Low both
Auto PATH modification vs hint Low install.ps1

The bash script is in good shape with minor fixes needed. The PowerShell script needs checksum verification added before this is production-ready.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 26, 2026

Greptile Summary

This PR adds cross-platform install scripts (install.sh for macOS/Linux and install.ps1 for Windows) that resolve the latest browseros-cli release from the GitHub API, download the platform-specific archive, verify its integrity against a checksums.txt file, and place the binary in an appropriate directory with PATH guidance.\n\nThe rework successfully addresses both previously flagged concerns: Windows now performs checksum verification with explicit warnings on every skip path, and the bash script replaces the unanchored grep regex with awk exact-field matching. Both scripts are well-structured with proper error handling, cleanup via trap/finally, and ARM architecture detection.\n\nKey findings:\n- exit 1 after every Write-Error call in install.ps1 is dead code — $ErrorActionPreference = \"Stop\" causes Write-Error to throw a terminating exception first, so those lines are never reached.\n- The awk '$2 == filename' checksum lookup in install.sh could silently fall through to the "checksum not found" warning if checksums.txt was generated with ./-prefixed filenames, though the author confirmed the live format is compatible for v0.1.0.

Confidence Score: 4/5

Safe to merge; previous critical gaps (missing Windows checksum, silent skip paths) are resolved — remaining items are a minor dead-code cleanup and a defensive awk tweak.

Prior review concerns are fully addressed, the logic is sound end-to-end, and there are no security regressions. The two remaining findings are a P2 style issue (unreachable exit 1 lines) and a low-probability silent-skip edge case in checksum matching, neither of which affects correctness in the tested happy path.

Both files are low risk; install.sh deserves a quick look at the awk checksum field matching if checksums.txt generation changes in the future.

Important Files Changed

Filename Overview
packages/browseros-agent/apps/cli/scripts/install.ps1 New Windows install script with solid checksum verification (addressing prior review), correct ARM64 detection via .NET RuntimeInformation, and proper try/finally cleanup; four unreachable exit 1 lines are dead code after Write-Error with $ErrorActionPreference = "Stop".
packages/browseros-agent/apps/cli/scripts/install.sh New cross-platform bash install script; previous silent-skip and regex-match concerns are well addressed with explicit warnings and awk exact-field matching, though the checksum lookup could silently skip if checksums.txt uses ./-prefixed paths.

Sequence Diagram

sequenceDiagram
    participant User
    participant Script as install.sh / install.ps1
    participant GH_API as GitHub Releases API
    participant GH_DL as GitHub Download CDN

    User->>Script: run (optional --version / -Version)
    alt Version not specified
        Script->>GH_API: GET /releases?per_page=100
        GH_API-->>Script: release list (JSON)
        Script->>Script: filter browseros-cli-v*, exclude -rc, take first
    end
    Script->>Script: detect OS + arch (uname / RuntimeInformation)
    Script->>GH_DL: GET /{tag}/{binary}_{ver}_{os}_{arch}.tar.gz|zip
    GH_DL-->>Script: archive bytes → TmpDir
    Script->>GH_DL: GET /{tag}/checksums.txt
    alt checksums.txt available
        GH_DL-->>Script: checksums.txt
        Script->>Script: awk/foreach exact-field match on filename
        alt hash found
            Script->>Script: sha256sum/Get-FileHash → compare
            alt mismatch
                Script-->>User: Error: checksum mismatch → exit
            end
            Script-->>User: Checksum verified
        else hash not found
            Script-->>User: Warning: checksum not found, skipping
        end
    else fetch failed
        Script-->>User: Warning: could not fetch checksums.txt, skipping
    end
    Script->>Script: extract archive → find binary
    Script->>Script: mv binary → INSTALL_DIR / Move-Item
    Script->>Script: chmod +x (bash) / update user PATH (both)
    Script-->>User: Installed! Run browseros-cli --help
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/cli/scripts/install.ps1
Line: 38-41

Comment:
**Unreachable `exit 1` statements — dead code**

Because `$ErrorActionPreference = "Stop"` is set at the top of the script, every `Write-Error` call immediately throws a terminating exception. The `exit 1` lines that follow can never be reached. The `finally` block still runs correctly (exception propagation triggers it), so cleanup is fine — but the four `exit 1` calls are dead code and can be removed.

Affected locations:
- Line 40 — after `Write-Error "Could not determine latest version..."`
- Line 63 — after `Write-Error "32-bit Windows is not supported."`
- Line 105 — after `Write-Error "Checksum mismatch..."`
- Line 118 — after `Write-Error "Binary not found in archive."`

```suggestion
    if (-not $tag) {
        Write-Error "Could not determine latest version. Try: -Version 0.1.0"
    }
```
The same pattern applies to lines 62-64, 104-106, and 117-119.

**Rule Used:** Remove unused/dead code rather than leaving it in ... ([source](https://app.greptile.com/review/custom-context?memory=9b045db4-2630-428c-95b7-ccf048d34547))

**Learnt From**
[browseros-ai/BrowserOS-agent#126](https://github.com/browseros-ai/BrowserOS-agent/pull/126)

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: packages/browseros-agent/apps/cli/scripts/install.sh
Line: 95

Comment:
**`awk` checksum lookup assumes bare filename in `checksums.txt`**

`awk '$2 == filename'` performs an exact match against the second whitespace-delimited field. The `sha256sum` and `shasum` tools prepend `./` to filenames when they are fed a path (e.g., `sha256sum ./file.tar.gz``abc123  ./file.tar.gz`). If whoever generated `checksums.txt` for the release ran `sha256sum` on a relative path, the second field would be `./browseros-cli_0.1.0_linux_amd64.tar.gz` and the match would silently fail — falling through to the "Warning: checksum not found" branch instead of verifying.

The PR description notes the live format was manually verified for `v0.1.0`, so this is benign today, but it's fragile across releases. Consider also matching on a trailing-component basis:

```bash
expected=$(awk -v filename="$FILENAME" '
  $2 == filename || $2 == "./" filename { print $1; exit }
' "${TMPDIR_DL}/checksums.txt")
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (2): Last reviewed commit: "fix(cli): address installer review feedb..." | Re-trigger Greptile

- Add checksum verification to install.ps1 using Get-FileHash
- Add warnings on all checksum skip paths in install.sh
- Use grep -F (fixed-string) instead of regex for filename matching
- Add ?per_page=100 to GitHub API call in install.ps1
- Use random temp directory name in install.ps1 to avoid collisions
@shadowfax92
Copy link
Contributor Author

@greptileai review

@shadowfax92
Copy link
Contributor Author

@claude review

@claude
Copy link

claude bot commented Mar 26, 2026

Claude finished @shadowfax92's task in 1m 57s —— View job


Code Review

  • Gather context
  • Read install.sh
  • Read install.ps1
  • Post findings

The previous review feedback has been well addressed. Here's the current state:

What Was Fixed ✅

Issue Resolution
No checksum verification in PS1 Added — Get-FileHash against checksums.txt (lines 82–111)
Silent checksum skip in bash Warnings on all three skip paths (lines 93–115)
Unanchored grep on filename Replaced with awk -v filename="$FILENAME" '$2 == filename' — exact field match, better than suggested grep -F
API pagination (30 vs 100) in PS1 Fixed — ?per_page=100 on line 34
Fixed temp dir in PS1 Fixed — uses GetRandomFileName() on line 72
GoReleaser archive structure Fixed — find fallback on lines 120–122

Remaining Items

install.sh

  1. No GITHUB_TOKEN support (lines 48, 91): Both the version-resolution API call and the binary download are unauthenticated. In CI environments, GitHub's unauthenticated API limit is 60 req/hr — hitting it gives a cryptic curl error rather than a useful message. Low priority but worth adding before wide distribution:

    CURL_HEADERS=(-H "Accept: application/vnd.github+json")
    [[ -n "${GITHUB_TOKEN:-}" ]] && CURL_HEADERS+=(-H "Authorization: Bearer $GITHUB_TOKEN")
    curl -fsSL "${CURL_HEADERS[@]}" "https://api.github.com/repos/..."

    Fix this →

  2. Fragile JSON parsing (lines 48–52): grep -o '"tag_name":...' + sed works now but is brittle against GitHub API whitespace/ordering changes. Not blocking, but if jq is available it's safer. Low priority.

install.ps1

  1. Auto PATH modification (lines 136–144): The bash script prints a PATH hint and lets the user add it themselves; the PS1 unconditionally writes to the registry User PATH. It does print a message first, so it's not invisible, but users piping irm | iex may not expect a registry write. Consider making this opt-in with an -AddToPath switch — or at minimum document this behavior prominently in the usage comment at the top of the file. Low priority.
    Fix this →

  2. No -Help parameter in PS1: Minor inconsistency — install.sh has --help but install.ps1 has no equivalent. Easy to add with Get-Help support via comment-based help.


Summary

The high and medium severity issues from the prior review are resolved. The remaining items are all low priority. The scripts are in good shape for an initial release — the GITHUB_TOKEN gap and the PS1 PATH behavior are the only things worth revisiting before wider promotion in docs.

@shadowfax92 shadowfax92 merged commit 03b4501 into main Mar 27, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant