PREQ-7260 Validate build number before export in get-build-number#319
PREQ-7260 Validate build number before export in get-build-number#319Tim-Pohlmann wants to merge 2 commits into
Conversation
hashicorp-vault-sonar-prod
Bot
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds a safety check in the composite get-build-number GitHub Action to ensure the build number read from .build_number.txt is numeric before exporting it to GITHUB_ENV / GITHUB_OUTPUT, covering cache-hit and env-pass-through paths as well as freshly-generated values.
Changes:
- Validate
BUILD_NUMBERagainst^[0-9]+$immediately before exporting. - Emit a GitHub Actions
::errorannotation and fail fast when the value is invalid.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hedinasr
left a comment
hedinasr
left a comment
There was a problem hiding this comment.
LGTM thanks @Tim-Pohlmann
The "Export build number" step reads .build_number.txt and exports it as BUILD_NUMBER unconditionally, on all three code paths (env passthrough, cache hit, freshly generated) - unlike get_build_number.sh, which validates the value with the same regex before incrementing it, but only on the freshly-generated path. A cache entry with unexpected/corrupted content would flow straight through to BUILD_NUMBER unchecked. Found while investigating sonar-scanner-msbuild's CI migration: an empty BUILD_NUMBER there causes scripts/set-version.ps1 to treat the run as a release-version bump, checking out a branch, committing, and opening a PR - an unattended side effect disproportionate to a missing one-line check.
hedinasr
force-pushed
the
tim/validate-build-number-export
branch
from
3042f87 to
a610142
Compare
Code Review ✅ ApprovedAdds a regex validation for the BUILD_NUMBER export to prevent downstream CI failures from corrupted cache or environment data. No issues found. OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
What
Validates
BUILD_NUMBERagainst^[0-9]+$in the "Export build number" step, right before exporting it - the same checkget_build_number.shalready applies, just at the single point where all three code paths (env passthrough, cache hit, freshly generated) converge.Why
The "Export build number" step trusts
.build_number.txt's content unconditionally on all three paths.get_build_number.shvalidates the value it fetches from the repo property, but only on the freshly-generated path - a cache hit (or an externally-providedBUILD_NUMBERenv var) skips that check entirely. A corrupted or empty cache entry would flow straight through toBUILD_NUMBERunchecked.Found this while migrating
sonar-scanner-msbuild's CI to GitHub Actions: an emptyBUILD_NUMBERthere causesscripts/set-version.ps1to treat the run as a release-version bump - checking out a branch, committing, and opening a PR unattended. Not something this action is responsible for, but the blast radius of a missing one-line check seemed worth closing here rather than only guarding defensively downstream.