Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
49b834e to
6086f6f
Compare
There was a problem hiding this comment.
Pull request overview
Updates the Ruby-based sandbox examples to reduce Bundler install failures in the Vercel Sandbox environment by forcing Bundler to use project-local config/home/path/cache locations.
Changes:
- Add a
bundle_envprefix to ensure Bundler uses local.bundle*andvendor/*directories. - Configure Bundler
pathandcache_path, and apply these settings tobundle installandbundle execinvocations. - For Rails, generate the app with
--skip-bundleand run Bundler explicitly afterward.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| examples/sandbox_09_rails_api.py | Adds Bundler env/config + skips automatic bundling during rails new, then runs Bundler with local paths. |
| examples/sandbox_07_ruby_sinatra.py | Adds Bundler env/config and runs install/exec with local paths to avoid unwritable home/config locations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
examples/sandbox_09_rails_api.py
Outdated
| bundle_env = ( | ||
| "BUNDLE_APP_CONFIG=.bundle " | ||
| "BUNDLE_USER_HOME=.bundle-home " | ||
| "BUNDLE_PATH=vendor/bundle " | ||
| "BUNDLE_CACHE_PATH=vendor/cache" | ||
| ) |
There was a problem hiding this comment.
bundle_env is being built as a shell fragment and then interpolated into multiple bash -lc strings. Since AsyncSandbox.run_command_detached already supports an env: dict[str, str] parameter, consider storing these as a dict and passing them via env=... instead of string concatenation. This avoids brittle spacing/quoting issues and reduces the risk of shell-injection when additional env vars are appended later.
| bundle_env = ( | |
| "BUNDLE_APP_CONFIG=.bundle " | |
| "BUNDLE_USER_HOME=.bundle-home " | |
| "BUNDLE_PATH=vendor/bundle " | |
| "BUNDLE_CACHE_PATH=vendor/cache" | |
| ) | |
| bundle_env = { | |
| "BUNDLE_APP_CONFIG": ".bundle", | |
| "BUNDLE_USER_HOME": ".bundle-home", | |
| "BUNDLE_PATH": "vendor/bundle", | |
| "BUNDLE_CACHE_PATH": "vendor/cache", | |
| } |
examples/sandbox_07_ruby_sinatra.py
Outdated
| bundle_env = ( | ||
| "BUNDLE_APP_CONFIG=.bundle " | ||
| "BUNDLE_USER_HOME=.bundle-home " | ||
| "BUNDLE_PATH=vendor/bundle " | ||
| "BUNDLE_CACHE_PATH=vendor/cache" | ||
| ) |
There was a problem hiding this comment.
bundle_env is assembled as a shell prefix string and spliced into several bash -lc commands. The sandbox API supports env=... (dict) on run_command_detached, so it would be more robust to pass these values via env rather than string interpolation. This prevents subtle quoting/spacing bugs and keeps commands easier to read/modify.
examples/sandbox_09_rails_api.py
Outdated
| f"cd {app_path} && " | ||
| f"ALLOWED_HOST={allowed_host} bundle exec rails server -b 0.0.0.0 -p {port}" | ||
| f"ALLOWED_HOST={allowed_host} {bundle_env} " | ||
| f"bundle exec rails server -b 0.0.0.0 -p {port}" |
There was a problem hiding this comment.
allowed_host is interpolated directly into a bash -lc command as ALLOWED_HOST=... without any shell escaping. If the hostname ever contains unexpected characters, this can break the command or become a shell-injection vector. Prefer passing ALLOWED_HOST via the env parameter to run_command_detached (or, if keeping inline assignment, ensure it is shell-escaped).
| f"bundle exec rackup -s webrick -o 0.0.0.0 -p {port}" | ||
| ), | ||
| ], | ||
| ) |
There was a problem hiding this comment.
RACK_ENV=production and the bundler env settings are currently injected by concatenating shell fragments into the bash -lc string. Since run_command_detached supports an explicit env dict, passing these via env would avoid shell quoting issues and makes it clearer which variables are being set for the process.
Getting this new runtime error: * Creating isolated environment: venv+pip... * Installing packages in isolated environment: - setuptools>=61.0 > /home/runner/work/vercel-py/vercel-py/.venv/bin/python3 -m pip --python /tmp/build-env-3ee0ml40/bin/python install --use-pep517 --no-warn-script- location --no-compile -r /tmp/build-requirements-_p80z08g.txt < /home/runner/work/vercel-py/vercel-py/.venv/bin/python3: No module named pip ERROR Command '['/home/runner/work/vercel-py/vercel-py/.venv/bin/python3', '-m', 'pip', '--python', '/tmp/build-env-3ee0ml40/bin/python', 'install', '--use-pep517', '--no-warn-script-location', '--no-compile', '-r', '/tmp/build-requirements-_p80z08g.txt']' returned non-zero exit status 1.
Probably missed some pinned environment thing, but this unblocks the rest of CI. Will investigate the underlying cause more thoroughly soon.