feat: ship bun runtime instead of compiled binary#367
Open
shadowfax92 wants to merge 5 commits intomainfrom
Open
feat: ship bun runtime instead of compiled binary#367shadowfax92 wants to merge 5 commits intomainfrom
shadowfax92 wants to merge 5 commits intomainfrom
Conversation
New build script that ships the Bun runtime alongside a bundled index.js
instead of compiling to a standalone executable via `bun build --compile`.
Output per platform: dist/server/browseros-server-{platform}/resources/
- bin/bun (platform-specific Bun runtime from GitHub releases)
- index.js (bundled server with WASM inlined)
- index.js.map (linked source map)
This matches the directory structure Chromium expects after the
bun-binary-based-start migration (resources/bin/bun + resources/index.js).
Key changes from server.ts:
- Eliminates `bun build --compile` step entirely
- Downloads official Bun binaries per platform from GitHub releases
- Reads bun version from root package.json packageManager field
- Parallel downloads with per-version caching in dist/server/.cache/
- Same bundle step (Bun.build with WASM plugin, env injection)
- Same sourcemap/Sentry upload pipeline
- Same CLI interface (--mode=prod/dev --target=...)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Greptile SummaryChanges build pipeline from standalone executable ( Key improvements from previous feedback:
New R2 upload script ( Issues found:
Confidence Score: 3/5
Important Files Changed
Last reviewed commit: 9ad1f99 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
271
to
293
| async function downloadBunRuntime( | ||
| bunVersion: string, | ||
| target: BuildTarget, | ||
| ): Promise<string> { | ||
| const cacheDir = join(CACHE_DIR, `bun-v${bunVersion}`, target.bunReleaseName) | ||
| const cachedBinary = join(cacheDir, target.bunBinaryName) | ||
|
|
||
| if (existsSync(cachedBinary)) { | ||
| log.info(`Cached: ${target.name}`) | ||
| return cachedBinary | ||
| } | ||
|
|
||
| const url = `${BUN_GITHUB_URL}/bun-v${bunVersion}/${target.bunReleaseName}.zip` | ||
| log.info(`Downloading: ${target.name}`) | ||
|
|
||
| mkdirSync(CACHE_DIR, { recursive: true }) | ||
| const zipPath = join(CACHE_DIR, `${target.bunReleaseName}.zip`) | ||
|
|
||
| await runCommand( | ||
| 'curl', | ||
| ['-fSL', '--retry', '3', '-o', zipPath, url], | ||
| process.env, | ||
| ) |
Contributor
There was a problem hiding this comment.
Downloading binaries without integrity verification creates a security risk. If the download is intercepted (MITM) or GitHub is compromised, malicious binaries could be executed.
Consider adding SHA256 checksum verification against known-good hashes published by Bun, or use GPG signature verification if available.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/build/server-new.ts
Line: 271-293
Comment:
Downloading binaries without integrity verification creates a security risk. If the download is intercepted (MITM) or GitHub is compromised, malicious binaries could be executed.
Consider adding SHA256 checksum verification against known-good hashes published by Bun, or use GPG signature verification if available.
How can I resolve this? If you propose a fix, please make it concise.
scripts/build/server-new.ts
Outdated
| rmSync(extractDir, { recursive: true, force: true }) | ||
| mkdirSync(extractDir, { recursive: true }) | ||
|
|
||
| await runCommandQuiet('unzip', ['-q', '-o', zipPath, '-d', extractDir]) |
Contributor
There was a problem hiding this comment.
unzip command is not available by default on Windows, causing builds to fail on Windows.
Consider using a JavaScript-based unzip library like adm-zip for cross-platform compatibility.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/build/server-new.ts
Line: 299
Comment:
`unzip` command is not available by default on Windows, causing builds to fail on Windows.
Consider using a JavaScript-based unzip library like `adm-zip` for cross-platform compatibility.
How can I resolve this? If you propose a fix, please make it concise.Address review feedback: - Verify downloaded bun binaries against SHASUMS256.txt from official releases - Use platform-aware extraction (unzip on Unix, Expand-Archive on Windows) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines
351
to
357
| const expectedHash = sha256Sums.get(zipName) | ||
| if (expectedHash) { | ||
| verifySha256(zipPath, expectedHash) | ||
| log.info(`Verified: ${target.name}`) | ||
| } else { | ||
| log.info(`Warning: no checksum found for ${zipName}, skipping verification`) | ||
| } |
Contributor
There was a problem hiding this comment.
missing checksum allows unverified binary to be used, defeating the security check
Suggested change
| const expectedHash = sha256Sums.get(zipName) | |
| if (expectedHash) { | |
| verifySha256(zipPath, expectedHash) | |
| log.info(`Verified: ${target.name}`) | |
| } else { | |
| log.info(`Warning: no checksum found for ${zipName}, skipping verification`) | |
| } | |
| const expectedHash = sha256Sums.get(zipName) | |
| if (!expectedHash) { | |
| throw new Error(`No checksum found for ${zipName} in SHASUMS256.txt`) | |
| } | |
| verifySha256(zipPath, expectedHash) | |
| log.info(`Verified: ${target.name}`) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/build/server-new.ts
Line: 351-357
Comment:
missing checksum allows unverified binary to be used, defeating the security check
```suggestion
const expectedHash = sha256Sums.get(zipName)
if (!expectedHash) {
throw new Error(`No checksum found for ${zipName} in SHASUMS256.txt`)
}
verifySha256(zipPath, expectedHash)
log.info(`Verified: ${target.name}`)
```
How can I resolve this? If you propose a fix, please make it concise.
scripts/build/server-new.ts
Outdated
| 'Root package.json must have packageManager set to bun@{version}', | ||
| ) | ||
| } | ||
| return pm.slice(4) |
Contributor
There was a problem hiding this comment.
empty version not validated - if packageManager is 'bun@', returns empty string causing invalid URLs
Suggested change
| return pm.slice(4) | |
| const version = pm.slice(4) | |
| if (!version) { | |
| throw new Error( | |
| 'Root package.json packageManager must include version (e.g., bun@1.3.6)', | |
| ) | |
| } | |
| return version |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/build/server-new.ts
Line: 202
Comment:
empty version not validated - if `packageManager` is 'bun@', returns empty string causing invalid URLs
```suggestion
const version = pm.slice(4)
if (!version) {
throw new Error(
'Root package.json packageManager must include version (e.g., bun@1.3.6)',
)
}
return version
```
How can I resolve this? If you propose a fix, please make it concise.- Fail hard when no SHA256 checksum is found instead of warning - Validate that packageManager version string is non-empty Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines
+339
to
+342
| if (existsSync(cachedBinary)) { | ||
| log.info(`Cached: ${target.name}`) | ||
| return cachedBinary | ||
| } |
Contributor
There was a problem hiding this comment.
cached binary returned without checksum verification — if cache is compromised, malicious binary will be used
Suggested change
| if (existsSync(cachedBinary)) { | |
| log.info(`Cached: ${target.name}`) | |
| return cachedBinary | |
| } | |
| if (existsSync(cachedBinary)) { | |
| const expectedHash = sha256Sums.get(`${target.bunReleaseName}.zip`) | |
| if (expectedHash) { | |
| try { | |
| verifySha256(cachedBinary, expectedHash) | |
| log.info(`Cached: ${target.name}`) | |
| return cachedBinary | |
| } catch { | |
| log.info(`Cache invalid for ${target.name}, re-downloading...`) | |
| rmSync(cachedBinary, { force: true }) | |
| } | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/build/server-new.ts
Line: 339-342
Comment:
cached binary returned without checksum verification — if cache is compromised, malicious binary will be used
```suggestion
if (existsSync(cachedBinary)) {
const expectedHash = sha256Sums.get(`${target.bunReleaseName}.zip`)
if (expectedHash) {
try {
verifySha256(cachedBinary, expectedHash)
log.info(`Cached: ${target.name}`)
return cachedBinary
} catch {
log.info(`Cache invalid for ${target.name}, re-downloading...`)
rmSync(cachedBinary, { force: true })
}
}
}
```
How can I resolve this? If you propose a fix, please make it concise.- scripts/upload/server.ts: zips and uploads per-platform resources/ to R2 - scripts/.env.example: R2 credential template - Adds @aws-sdk/client-s3 for S3-compatible R2 uploads - New command: bun run upload:server Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines
+339
to
+341
| if (existsSync(cachedBinary)) { | ||
| log.info(`Cached: ${target.name}`) | ||
| return cachedBinary |
Contributor
There was a problem hiding this comment.
cached binary used without checksum verification — if cache directory is compromised, malicious binary will execute without detection
Suggested change
| if (existsSync(cachedBinary)) { | |
| log.info(`Cached: ${target.name}`) | |
| return cachedBinary | |
| if (existsSync(cachedBinary)) { | |
| const expectedHash = sha256Sums.get(zipName) | |
| if (expectedHash) { | |
| const binaryHash = createHash('sha256').update(readFileSync(cachedBinary)).digest('hex') | |
| const zipPath = join(CACHE_DIR, zipName) | |
| if (existsSync(zipPath)) { | |
| try { | |
| verifySha256(zipPath, expectedHash) | |
| log.info(`Cached: ${target.name}`) | |
| return cachedBinary | |
| } catch { | |
| // Cache corrupted, re-download | |
| rmSync(cacheDir, { recursive: true, force: true }) | |
| } | |
| } | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/build/server-new.ts
Line: 339-341
Comment:
cached binary used without checksum verification — if cache directory is compromised, malicious binary will execute without detection
```suggestion
if (existsSync(cachedBinary)) {
const expectedHash = sha256Sums.get(zipName)
if (expectedHash) {
const binaryHash = createHash('sha256').update(readFileSync(cachedBinary)).digest('hex')
const zipPath = join(CACHE_DIR, zipName)
if (existsSync(zipPath)) {
try {
verifySha256(zipPath, expectedHash)
log.info(`Cached: ${target.name}`)
return cachedBinary
} catch {
// Cache corrupted, re-download
rmSync(cacheDir, { recursive: true, force: true })
}
}
}
}
```
How can I resolve this? If you propose a fix, please make it concise.| outputPath: string, | ||
| ): Promise<void> { | ||
| const absOutput = resolve(outputPath) | ||
| const proc = Bun.spawn(['zip', '-r', '-q', absOutput, 'resources'], { |
Contributor
There was a problem hiding this comment.
zip command not available on Windows by default — will cause upload failures on Windows
follow the pattern from server-new.ts:319-328 which uses PowerShell's Compress-Archive on Windows
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/upload/server.ts
Line: 186
Comment:
`zip` command not available on Windows by default — will cause upload failures on Windows
follow the pattern from `server-new.ts:319-328` which uses PowerShell's `Compress-Archive` on Windows
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/build/server-new.ts— a new build script that ships the Bun runtime alongside a bundledindex.jsinstead of compiling to a standalone executable viabun build --compilepackageManagerfield)dist/server/browseros-server-{platform}/resources/matching the directory structure Chromium expects after the bun-binary-based-start migration (resources/bin/bun+resources/index.js)What changed
Old approach (
server.ts):Bun.build()→bun build --compile→ single executable per platformNew approach (
server-new.ts):Bun.build()→ download bun runtime → packageresources/directory per platformOutput structure:
This aligns with:
GetBrowserOSServerExecutablePath()→resources/bin/bunProcessControllerImpl::Launch()→bun index.js --config=...versions/{v}/resources/Key design decisions
packageManagerin rootpackage.json(single source of truth)dist/server/.cache/bun-v{version}/wasmBinaryPlugin(user request: put everything in index.js)--mode=prod/dev,--target=...)Test plan
darwin-arm64anddarwin-x64bun --version→ 1.3.6)🤖 Generated with Claude Code