Skip to content

feat: add zip archive support for bun-based server resources#385

Open
shadowfax92 wants to merge 1 commit intomainfrom
feat/server-resource-zip-support
Open

feat: add zip archive support for bun-based server resources#385
shadowfax92 wants to merge 1 commit intomainfrom
feat/server-resource-zip-support

Conversation

@shadowfax92
Copy link
Contributor

Summary

  • Adds zip archive download + extraction support to the build pipeline's download module (download.py)
  • Updates download_resources.yaml to download versioned server zip archives from R2 (server/v{version}/browseros-server-{platform}.zip)
  • Updates copy_resources.yaml to copy extracted resource directories (containing bin/bun, index.js, index.js.map) instead of individual server binaries
  • Adds BROWSEROS_SERVER_VERSION file for version tracking with {server_version} placeholder substitution in R2 keys
  • Updates validate_resources.py patch to validate new resource layout (bin/bun + index.js)

Context

The new bun-based server (from the new-bun-runtime feature) uploads zip archives to R2 containing a resources/ directory with the Bun runtime, bundled server JS, and source maps. This PR updates the Chromium build pipeline to support downloading and extracting these archives.

Test plan

  • Verify download.py correctly downloads and extracts zip archives from R2
  • Verify {server_version} substitution reads from BROWSEROS_SERVER_VERSION
  • Verify copy_resources.yaml copies the extracted resources/ directory to chrome/browser/browseros/server/resources/
  • Verify resources/server/ is properly gitignored
  • Test with actual server zip uploaded to R2 at server/v0.0.57/browseros-server-darwin-arm64.zip

🤖 Generated with Claude Code

The new bun-based server uploads zip archives to R2 containing
resources/bin/bun, resources/index.js, and resources/index.js.map.
This updates the build pipeline to download and extract these zips
instead of individual server binaries.

- Add zip archive download + extraction support to download.py
- Add {server_version} placeholder substitution in R2 keys
- Update download_resources.yaml for zip archive downloads
- Update copy_resources.yaml to copy extracted resource directories
- Add BROWSEROS_SERVER_VERSION file for version tracking
- Update validate_resources.py patch for new resource layout
- Add resources/server/ to .gitignore

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@greptile-apps
Copy link

greptile-apps bot commented Feb 27, 2026

Greptile Summary

Adds zip archive support to the build pipeline for downloading and extracting bun-based server resources from R2.

Key Changes:

  • Updated download.py to support zip archive extraction with {server_version} placeholder substitution from BROWSEROS_SERVER_VERSION file
  • Changed resource downloads from individual server binaries to versioned zip archives containing bin/bun, index.js, and index.js.map
  • Updated copy operations to copy entire extracted resource directories instead of individual files
  • Modified validation to check for new resource layout (bin/bun + index.js)

Issues Found:

  • Zip extraction vulnerable to path traversal attacks (zip slip) - should validate member paths before extraction
  • Missing executable files after extraction fail silently instead of raising errors, which could cause confusing build failures later
  • Temporary zip file cleanup not guaranteed if extraction fails

Confidence Score: 3/5

  • This PR has security and error handling concerns that should be addressed before merging
  • The implementation is functionally sound for the controlled R2 environment, but has two logic issues: zip slip vulnerability in extraction and silent failures when expected executables are missing. While the zip files come from a trusted source, the lack of path validation is a security concern worth addressing.
  • Pay close attention to packages/browseros/build/modules/storage/download.py - the zip extraction logic needs security hardening and better error handling

Important Files Changed

Filename Overview
packages/browseros/build/modules/storage/download.py added zip extraction with version substitution; has zip slip vulnerability and silent failure issues
packages/browseros/build/config/download_resources.yaml updated to download versioned zip archives instead of individual binaries; configuration looks correct
packages/browseros/build/config/copy_resources.yaml updated to copy extracted resource directories instead of individual binaries; paths are consistent

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Build Pipeline Starts] --> B[Read BROWSEROS_SERVER_VERSION]
    B --> C{Resolve r2_key with version}
    C --> D[Download zip from R2]
    D --> E[Clear existing extract_dir]
    E --> F[Extract zip to extract_dir]
    F --> G{Set executable perms}
    G --> H[Copy resources to Chromium tree]
    H --> I[Validate resources]
    I --> J{bin/bun + index.js exist?}
    J -->|Yes| K[Build continues]
    J -->|No| L[Build fails]
    
    style D fill:#e1f5ff
    style F fill:#e1f5ff
    style I fill:#fff3cd
    style J fill:#fff3cd
Loading

Last reviewed commit: a166c7e

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +152 to +153
with zipfile.ZipFile(tmp_zip, "r") as zf:
zf.extractall(extract_dir)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extractall() vulnerable to zip slip attacks (malicious zips with ../ paths could write outside extract_dir)

Suggested change
with zipfile.ZipFile(tmp_zip, "r") as zf:
zf.extractall(extract_dir)
with zipfile.ZipFile(tmp_zip, "r") as zf:
for member in zf.namelist():
# Validate each member to prevent path traversal
member_path = (extract_dir / member).resolve()
if not member_path.is_relative_to(extract_dir.resolve()):
raise RuntimeError(f"Unsafe zip entry: {member}")
zf.extractall(extract_dir)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros/build/modules/storage/download.py
Line: 152-153

Comment:
`extractall()` vulnerable to zip slip attacks (malicious zips with `../` paths could write outside `extract_dir`)

```suggestion
        with zipfile.ZipFile(tmp_zip, "r") as zf:
            for member in zf.namelist():
                # Validate each member to prevent path traversal
                member_path = (extract_dir / member).resolve()
                if not member_path.is_relative_to(extract_dir.resolve()):
                    raise RuntimeError(f"Unsafe zip entry: {member}")
            zf.extractall(extract_dir)
```

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

Comment on lines +160 to +163
exe_path = extract_dir / rel_path
if exe_path.exists():
exe_path.chmod(exe_path.stat().st_mode | 0o755)
log_info(f" Set executable: {rel_path}")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

silently continues if executable doesn't exist after extraction - should fail loudly to catch misconfigured archives

Suggested change
exe_path = extract_dir / rel_path
if exe_path.exists():
exe_path.chmod(exe_path.stat().st_mode | 0o755)
log_info(f" Set executable: {rel_path}")
for rel_path in executable_paths:
exe_path = extract_dir / rel_path
if not exe_path.exists():
raise RuntimeError(f"Expected executable not found after extraction: {rel_path}")
exe_path.chmod(exe_path.stat().st_mode | 0o755)
log_info(f" Set executable: {rel_path}")
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros/build/modules/storage/download.py
Line: 160-163

Comment:
silently continues if executable doesn't exist after extraction - should fail loudly to catch misconfigured archives

```suggestion
        for rel_path in executable_paths:
            exe_path = extract_dir / rel_path
            if not exe_path.exists():
                raise RuntimeError(f"Expected executable not found after extraction: {rel_path}")
            exe_path.chmod(exe_path.stat().st_mode | 0o755)
            log_info(f"    Set executable: {rel_path}")
```

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant