fix: harden deploy scripts and upgrade safety workflows#32
Merged
Conversation
- Add post-fallback validation for BLOCKSCOUT_URL in resolve-network.sh - Validate KEY=VALUE format and key names in prepare-env.sh export - Add curl timeouts and normalize URL in verify-blockscout.sh status poll
- Hard fail when base branch build fails (opt-in ALLOW_FALLBACK=true) - Pin @openzeppelin/upgrades-core version (default 1.44.2) - Require upgrade-safety success for deploy (no longer allows skipped) - Quote deploy-script input to prevent flag injection - Fail on empty contracts array when base branch has entries - Remove duplicate Node.js setup steps in both workflows
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the repo’s deployment helper scripts and CI upgrade-safety workflows in response to prior review feedback and security audit findings, aiming to reduce validation bypasses and make deploy-time automation more robust.
Changes:
- Tighten upgrade-safety validation to (a) block empty-contract “skips” when base has contracts, (b) fail hard when base checkout/build fails unless explicitly allowed, and (c) pin the OpenZeppelin upgrades-core CLI version.
- Harden deploy scripts by validating/resolving Blockscout URLs, adding curl timeouts, and safely parsing
DEPLOY_ENV_VARS. - Update reusable workflows to remove duplicate Node.js setup, require upgrade-safety success before deploy, and quote the deploy-script input.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/upgrade-safety/validate.sh |
Adds stricter failure modes and version pinning for upgrade-safety validation. |
scripts/deploy/verify-blockscout.sh |
Adds curl timeouts and normalizes trailing slashes for Blockscout polling. |
scripts/deploy/resolve-network.sh |
Adds post-fallback validation that Blockscout URL is non-empty and http(s). |
scripts/deploy/prepare-env.sh |
Parses DEPLOY_ENV_VARS as KEY=VALUE and rejects malformed/unsafe keys. |
.github/workflows/_upgrade-safety.yml |
Consolidates Node.js setup and conditionally enables caching. |
.github/workflows/_foundry-cicd.yml |
Requires upgrade-safety success for deploy and quotes deploy-script; consolidates Node.js setup in upgrade-safety job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- validate.sh: fail hard when base branch fetch/read fails during empty-contracts check (gated by ALLOW_FALLBACK=true) - verify-blockscout.sh: normalize BLOCKSCOUT_URL once and use consistently for both submission and polling - test-prepare-env.sh: add tests for malformed lines, invalid key names, and values containing equals signs
rubydusa
requested changes
Apr 2, 2026
5 tasks
…dening-fixes fix: address PR #32 review comments
rubydusa
approved these changes
Apr 6, 2026
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
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
Addresses review comments from PR #30 and findings from the security audit.
Deploy script hardening (commit 1)
BLOCKSCOUT_URLis non-empty, not"null", and starts withhttp(s)://KEY=VALUEformat before exporting, skip malformed lines and invalid key names--connect-timeout 10and--max-time 30to status poll curl; normalize URL trailing slashUpgrade safety & workflow hardening (commit 2)
ALLOW_FALLBACK=trueto downgrade to upgradeability-only)@openzeppelin/upgrades-coreversion viaOZ_UPGRADES_CORE_VERSIONenv var (default1.44.2)upgrade-safetyjob to succeed for deploy (no longer allowsskipped)deploy-scriptinput to prevent flag injectionAddress PR review comments (commit 3)
ALLOW_FALLBACK=trueBLOCKSCOUT_URLonce at the top and use consistently for both submission and polling (no more double-slash risk)=), invalid key names (digits, hyphens, spaces), and values containing=Test plan
bash tests/test-prepare-env.sh)