feat: add patch/minor bump_type option to build-release-main workflow - #9979
decentraland-bot wants to merge 1 commit into
Conversation
Adds a `bump_type` input (choices: `minor` | `patch`, default: `minor`) to the `workflow_dispatch` trigger of `build-release-main.yml` and to the `version` composite action so hotfixes can produce a patch release (e.g. 0.175.0 → 0.175.1) instead of always bumping minor. Push-to-main runs retain their existing minor-bump behaviour since `inputs.bump_type` falls back to `'minor'` when unset. Closes #9978 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
decentraland-bot
added
the
ext-contribution
🚦 CI StatusBuild skipped — no changes detected under Waiting for lint to start… Waiting for tests to start… Bare-metal benchmarks run automatically after each successful build; results land in this section. Add the On demand — comment |
|
Slack notification sent to #explorer-ext-contributions for external review. |
|
Claude finished @decentraland-bot's task in 2m 9s —— View job Security & Workflow Review
Dependency reviewNo dependency changes in this PR. The Workflow reviewThe bump arithmetic itself is correct, and the claim in the PR body that push-to-main paths are unaffected holds: the None of the W.1–W.10 HIGH security patterns apply: no prompt files, no LLM invocation, no However, there is one release-integrity bug that defeats the feature's purpose. 🔴 The published tag will not match the built artifacts on a
|
| Artifact | Value | Source |
|---|---|---|
| Built binary version | v0.175.1-name-main |
build-release-main.yml:24 → build-unitycloud.yml |
| Unity Cloud release path | v0.175.1-name |
build-unitycloud.yml:964 |
| Git tag pushed | v0.176.0-name |
build-release-main-page.yml:51 |
| GitHub Release name | v0.176.0-name |
build-release-main-page.yml:70 |
So the hotfix ships as 0.175.1 inside the client but is tagged/published as 0.176.0, and the next regular minor release would then compute v0.177.0 off that phantom tag — silently skipping a minor.
The page workflow can't read the dispatch inputs of the triggering run (the workflow_run REST payload does not expose inputs), so the fix has to carry the resolved version forward rather than recompute it. Two workable options:
- Publish the resolved tag as an artifact. Have
get-infoinbuild-release-main.ymlwritenext_tag_versionto a file andactions/upload-artifactit; the page workflow already downloads artifacts fromgithub.event.workflow_run.id(line 55-63), so it can pick it up there and drop its ownversionaction call entirely. This makes the release workflow the single source of truth for the version. - Move tag creation into
build-release-main.yml, and have the page workflow resolve the tag withgit describeon the already-taggedmaininstead of computing anext_*value.
Option 1 is the smaller change and removes a duplicated computation that was already fragile.
🟡 MEDIUM — expression interpolated into run: (W-style script injection)
.github/actions/version/action.yml:103:
BUMP_TYPE="${{ inputs.bump_type || 'minor' }}"${{ }} is substituted textually before bash parses the script, so a value containing " plus $(…) executes on the runner. In this PR the only caller constrains it to a choice of minor/patch and workflow_dispatch requires write access, so it is not exploitable today — but the composite action is reusable and the guarantee lives in the caller, not here. The existing inputs.tag (line 53, unquoted) and inputs.commit_sha (lines 81-82) have the same shape, so this is a pre-existing pattern rather than something this PR introduces.
The standard fix is to pass through env: so the value never reaches the shell parser:
- name: Parse version tag
id: parser
env:
BUMP_TYPE_IN: ${{ inputs.bump_type }}
TAG_IN: ${{ inputs.tag }}
COMMIT_SHA_IN: ${{ inputs.commit_sha }}
run: |
BUMP_TYPE="${BUMP_TYPE_IN:-minor}"Reasonable to scope to just the new line here and leave the pre-existing two for a follow-up, but worth doing at least for the line this PR adds.
🟡 MEDIUM — no validation of bump_type in the composite action
The else branch is the catch-all, so any typo or unexpected value (Patch, major, "") silently produces a minor bump. For an action that decides release version numbers, failing loudly is better:
case "$BUMP_TYPE" in
minor|patch) ;;
*) echo "Invalid bump_type: $BUMP_TYPE (expected 'minor' or 'patch')"; exit 1 ;;
esacThis matters because the composite action is callable from workflows that don't have the choice constraint.
Minor / non-blocking
NEXT_MAJOR="$MAJOR"is assigned identically in both branches (lines 105, 109) — hoist it above theifso the branches show only what actually differs.- Redundant double default. The action declares
default: minorand the caller passes${{ inputs.bump_type || 'minor' }}and the script has a third|| 'minor'. All three are harmless, but the workflow-side|| 'minor'is the only one that's load-bearing (it handles thepushevent whereinputsis null). Consider dropping the in-script fallback once theenv:change above is made. - Operational note: this PR targets
dev, but release builds are dispatched frommain.workflow_dispatchinput definitions are read from the workflow file on the selected ref, so thebump_typedropdown won't appear when dispatching frommainuntil this lands there through the normal dev→main flow. Not a defect — just don't expect the option to show up right after merge. - Pre-existing, untouched by this PR (noted for context, not as blockers of this change):
build-release-main.ymlhas no explicitpermissions:block at workflow or job level and usessecrets: inheritintobuild-unitycloud.yml(W.10);build-release-main-page.ymluses tag-pinned third-party actions in the publish path —dawidd6/action-download-artifact@v6,softprops/action-gh-release@v2— which should be SHA-pinned (W.7b).
Verdict
The bump arithmetic is right and the no-op-for-push claim checks out, but the feature is incomplete: on a patch dispatch the release is tagged and published with a minor-bumped version that doesn't match the artifacts it contains. That should be resolved before merge, along with the env:-passing and validation nits.
No security-relevant HIGH findings — this is a release-correctness issue, not a supply-chain one.
DEPENDENCY_REVIEW: NEEDS_ATTENTION
• Branch fix/9978-patch-minor-bump-option
| echo "Bump type: $BUMP_TYPE" | ||
| echo "[Next Major]: $NEXT_MAJOR" | ||
| echo "[Next Minor]: $NEXT_MINOR" | ||
| echo "[Next Build]: $NEXT_BUILD" |
There was a problem hiding this comment.
| echo "Bump type: $BUMP_TYPE" | |
| echo "[Next Major]: $NEXT_MAJOR" | |
| echo "[Next Minor]: $NEXT_MINOR" | |
| echo "[Next Build]: $NEXT_BUILD" |
| echo "Tag version: $TAGVER" | ||
| echo "tag_version=$TAGVER" >> "$GITHUB_OUTPUT" | ||
|
|
||
| # Determine next version components based on bump_type |
There was a problem hiding this comment.
| # Determine next version components based on bump_type |
| workflow_dispatch: | ||
| inputs: | ||
| bump_type: | ||
| description: 'Version bump type — use "patch" for hotfixes (e.g. 0.175.0 → 0.175.1), "minor" for regular releases (e.g. 0.175.0 → 0.176.0)' |
There was a problem hiding this comment.
| description: 'Version bump type — use "patch" for hotfixes (e.g. 0.175.0 → 0.175.1), "minor" for regular releases (e.g. 0.175.0 → 0.176.0)' | |
| description: 'Version bump type: default is `minor` (regular releases) 0.1.0 → 0.2.0`, `patch` for hotfixes takes 0.1.0 to 0.1.1``` |
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
PR Review — #9979 feat: add patch/minor bump_type option to build-release-main workflow
STEP 1 — Context & Scope
Files changed: 2 (+38 −7)
.github/actions/version/action.yml— composite action gains abump_typeinput (minor|patch) and conditional version arithmetic.github/workflows/build-release-main.yml—workflow_dispatchgains abump_typechoice input, forwarded to the version action
Other callers of the version action (verified via code search — build-release-main-page.yml, build-profile-nightly.yml, build-unitycloud.yml): none pass bump_type, so they inherit the default: minor and produce identical output to today. ✅ Backward-compatible.
STEP 2 — Root-cause check ✅
The PR adds a genuine new capability: dispatching a hotfix (patch) release build that bumps BUILD instead of MINOR. This is a feature, not a workaround.
STEP 3 — Design & integration ✅
This is a pure CI/CD change — no runtime code, no ECS systems, no lifecycle management. The design is straightforward:
- A new optional input with a sensible default propagated through a single composite action
- The version arithmetic is correct:
patch→MINORstays,BUILD + 1;minor(default) →MINOR + 1,BUILD = 0 - The
|| 'minor'fallback chain handles thepushtrigger (whereinputs.bump_typeis null) correctly
No lifecycle owners, no teardown concerns, no design flags.
STEP 4 — Member audit
N/A — YAML workflow files, no class members.
STEP 5 — Line-level findings
See inline comments below.
| # | Severity | File | Finding |
|---|---|---|---|
| 1 | P2 | action.yml:103 |
No input validation — composite action accepts any string for bump_type |
Security review
- Secrets & credentials: No secrets introduced or exposed. ✅
- Input validation / injection: The
${{ inputs.bump_type || 'minor' }}expression is interpolated directly into the shell script body (line 103). Current callers are safe — the workflow constrains the input to achoicetype (minor/patch), and thepushtrigger falls back to'minor'. However, the composite action itself accepts arbitrary strings, so a future caller passing untrusted input would be vulnerable to expression injection. The inline comment below suggests adding shell-level validation as defense-in-depth. For maximal safety, consider using a step-levelenv:block instead of${{ }}interpolation in therun:script (same pattern as the pre-existing${{ inputs.commit_sha }}and${{ inputs.tag }}interpolations, which carry the same theoretical risk). - Permissions: No new permissions requested. ✅
- Sensitive data exposure: Log output contains only version strings. ✅
STEP 6 — Complexity
SIMPLE — 2 CI/CD files, ~38 lines, pure shell arithmetic addition with no runtime code changes.
STEP 7 — QA assessment
NO — Changes are limited to GitHub Actions workflow/action YAML. No user-facing runtime behavior is affected.
STEP 8 — Non-blocking warnings
None.
STEP 9 — Verdict
REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Pure CI/CD workflow change — version action shell arithmetic and workflow_dispatch input, no runtime code.
QA_REQUIRED: NO
Reviewed by Jarvis 🤖 · Requested by unknown (<@unknown>) via Slack
| echo "tag_version=$TAGVER" >> "$GITHUB_OUTPUT" | ||
|
|
||
| # Determine next version components based on bump_type | ||
| BUMP_TYPE="${{ inputs.bump_type || 'minor' }}" |
There was a problem hiding this comment.
[P2] Input validation — defense-in-depth. The composite action accepts any string for bump_type since the input has no type constraint (unlike the workflow's choice type). While current callers are safe, adding a shell-level guard protects against future misuse and makes the contract explicit.
| BUMP_TYPE="${{ inputs.bump_type || 'minor' }}" | |
| BUMP_TYPE="${{ inputs.bump_type || 'minor' }}" | |
| if [ "$BUMP_TYPE" != "minor" ] && [ "$BUMP_TYPE" != "patch" ]; then | |
| echo "::error::Invalid bump_type '$BUMP_TYPE'. Must be 'minor' or 'patch'." | |
| exit 1 | |
| fi |
2 participants
Summary
bump_typeinput (minor|patch, defaultminor) to theworkflow_dispatchtrigger ofbuild-release-main.ymlbump_typethrough to theversioncomposite action, which now applies the correct arithmetic:minorbumps MINOR and resets BUILD to 0 (existing behaviour);patchbumps BUILD only (hotfix behaviour)inputs.bump_typefalls back to'minor'when unsetChanges
Example
v0.175.0-...minor(default)v0.176.0-...v0.175.0-...patchv0.175.1-...Testing
inputs.bump_type = ''→ fallback'minor'→ identical output to todayCloses
#9978
🤖 Created via Slack with Claude
Requested by Esteban Ordano (U9DLN0485)