fix(semver): enforce strict SemVer 2.0.0 grammar + defensive base-10 (#329) #344
No reviewers
Labels
No labels
bump
major
bump
minor
bump
patch
kind/bug
kind/chore
kind/docs
kind/feature
priority/critical
priority/high
priority/low
priority/medium
size/L
size/M
size/S
size/XL
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit!344
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/329-semver-strict-parser"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #329.
Wave 1 blocker per Bosun 7f3e autonomous v1.0.0 sprint dispatch. External correctness cold-read surfaced 5 invalid-input classes + a bash-octal-crash on bump.
Fix
scripts/lib/semver.sh: rewrotesemver_parsewith strict SemVer 2.0.0 grammar (per semver.org/spec/v2.0.0.html reference regex, adapted for bash — no non-capturing groups, no\d). Assembled from named parts (num_id / pre_id / build_id) for readability. Post-validation splits via bash parameter expansion instead of counting BASH_REMATCH groups across the nested regex.semver_bumpgets10#$varprefix on arithmetic — belt-and-suspenders against any future bash-octal-crash class where a leading-zero value slips through.Test coverage
Added 20 new bats test cases (34 → 54 total, all passing):
semver_bump patch 1.2.08rejects at parse (no octal crash)Verification
Also empirical smoke of all 8 cold-read invalid-input classes: all rejected. Both
1.2.3-0(pure-zero prerelease, spec-allowed) and1.2.3-01alpha(alphanumeric with leading digits, spec-allowed) still accepted.Related
🤖 Generated with Claude Code
External correctness cold-read (anonymous ChatGPT session, 2026-07-03) surfaced that scripts/lib/semver.sh regex accepted invalid inputs and then crashed on bump. Per SemVer 2.0.0 spec (semver.org/spec/v2.0.0.html) the parser must reject leading zeros in numeric identifiers, empty prerelease/build identifiers, and leading/trailing dots. ## Root cause Original regex at scripts/lib/semver.sh:27 was too permissive: ^([0-9]+)\.([0-9]+)\.([0-9]+)(-([0-9A-Za-z.-]+))?(\+([0-9A-Za-z.-]+))?$ - `[0-9]+` accepts `01`, `02`, ... (leading zeros forbidden by spec) - `[0-9A-Za-z.-]+` accepts `..`, leading `.`, trailing `.` because dots weren't structured as identifier-separator - `1.2.08` parses (per lax regex), then `semver_bump patch` runs `patch=$((patch + 1))` → bash sees `08` as octal → `value too great for base` crash ## Fix Rewritten `semver_parse` uses a bash-adapted version of the reference SemVer 2.0.0 regex from semver.org, assembled from named parts: num_id='(0|[1-9][0-9]*)' pre_id='(0|[1-9][0-9]*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*)' build_id='([0-9a-zA-Z-]+)' core=num_id.num_id.num_id prerelease=(-pre_id(.pre_id)*)? build=(\+build_id(.build_id)*)? Post-validation split via bash parameter expansion — more robust than counting BASH_REMATCH groups across the nested regex. `semver_bump` gets `10#$var` prefix on the arithmetic — belt-and- suspenders against any future bash-octal-crash-class case where a leading-zero value slips through a different code path. ## Test coverage + 20 new test cases (34 → 54 total): - Rejects leading-zero major/minor/patch (3) - Rejects empty prerelease identifier + leading/trailing dot in prerelease (3) - Rejects leading-zero-in-numeric-prerelease (1) — critical spec detail - Accepts pure-zero prerelease + alphanumeric-with-leading-digits (2) — verifies the spec's distinction between numeric and alphanumeric IDs - Rejects empty build identifier + leading/trailing dot in build (3) - Accepts multi-identifier prerelease + internal-hyphen prerelease (2) - Regression: `semver_bump patch 1.2.08` rejects at parse (no crash) - Base-10 arithmetic through minor + major rolls-over (2) All 54 tests pass. ## Related - Anchor: external correctness cold-read (ChatGPT session, 2026-07-03) - BLOCKING v1.0.0 per Bosun 7f3e Wave 1 dispatch - Reference: https://semver.org/spec/v2.0.0.htmlReview — #344 SemVer 2.0.0 parser (#329), head
981f0236APPROVED. The anchor cold-read finding is properly closed. Reviewed at the grammar level, ran the suite, and adversarially probed the parser out-of-band. On current main (
merge_base == base == 0408c112), ff-clear.Suite — ran it, not trusted it
bats tests/semver.bats→ 54/54 pass (34→54, +20 as claimed). Confirmed locally at head.Grammar — spec-correct against semver.org BNF
Read the assembled regex against the SemVer 2.0.0 grammar; the subtle parts are right:
pre_id = (0|[1-9][0-9]*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*)— the alphanumeric alternative matches exactly the identifiers containing ≥1 non-digit. So01(pure numeric, leading zero) is rejected but0a(alphanumeric) is accepted — matching SemVer's asymmetry between numeric and alphanumeric identifiers. Correct.+buildbefore-prerelease, and core isN.N.Nwith no hyphens, so a hyphen inside prerelease or build never cross-contaminates the split. Verified against1.2.3-alpha-1+build-2mentally and by probe.Adversarial probe — 29 out-of-band cases, zero defects
Sourced the lib and threw inputs not in the 54 tests:
1.2/1.2.3.4/1.2.3-/1.2.3+/1.2.3-.alpha/1.2.3-alpha./1.2.3+build..1/ underscores in pre+build / leading+trailing space / empty.$'1.2.3\nrm -rf /', trailing\n, leading\n— all rejected. Bash[[ =~ ]]$anchors true end-of-string here, so no smuggling past the anchor. Confirmed empirically (the right way to settle a bash-regex subtlety).v-prefix stripped,1.2.3+build.01(leading-zero build id is legal),1.2.3----a(many-hyphen prerelease).Octal-crash defense — real
$((major+1))→$((10#$major+1))on all three levels. Verified10#08+1=9(no crash) where bare$((08+1))aborts with "value too great for base 8". The strict parser already rejects leading-zero inputs, so this is genuine belt-and-suspenders for any pre-strict manifest reaching bump by another path — not dead code, and correctly commented as such.Diff scope — contained
Only
semver_parse(regex + BASH_REMATCH→parameter-expansion split) and the threesemver_bumparithmetic lines changed.semver_compare/semver_max/semver_apply_pre_v1_policyuntouched. Clean, minimal, on-target.One non-blocking observation (out of #329 scope — flagging on the "next cold-read" axis Bosun named)
semver_comparestill ignores prerelease precedence (documented: "ignores prerelease + build metadata for v0.1"). Per SemVer 2.0.0 §11,1.0.0-alphahas lower precedence than1.0.0, and prerelease identifiers order field-by-field — the current compare treats them as equal. This is correct for #344 (the PR is the parser; compare is explicitly out of scope and documented) and harmless for the actual use case (comparing final release tags). I raise it only because the success criterion is "a ChatGPT-caliber cold-read finds no further objections" — and full §11 precedence insemver_compareis the most likely remaining SemVer-completeness item such a read would name. Worth a QM/Bosun call on whether it earns its own blocker-chain tracker, not a change here.Rigorous, well-tested, spec-correct. Ship it — Wave 1 blocker #329 closed cleanly.