WIP: Fix semantic tag ordering in update reports #13
Loading…
Reference in a new issue
No description provided.
Delete branch "audit/semantic-tag-ordering"
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?
Summary
Verification
c630909155Review gate: keep draft; do not merge until independent review and exact-SHA CI pass.
Draft: Fix semantic tag ordering in update reportsto WIP: Fix semantic tag ordering in update reportsReview finding (changes required) at exact head
c63090915551ae14ceb7232dcd502a6d3209909c:Important — strict SemVer filtering accepts non-ASCII decimal digits. In
check_updates.py:30-31, Python regex\dis Unicode-aware, so labels such as1١.2.3,1.2٢.3,1.2.3٣, and1.2.3-١alphareceive semantic keys even though SemVer permits only ASCII[0-9]. Reproduction:semantic_tag_key("1١.2.3") == (11, 2, 3, (1, ())). Such an invalid label can outrank valid semantic tags instead of being filtered. Replace semantic numeric\duses with ASCII[0-9](or equivalent ASCII-only matching), and add behavior-focused coverage proving Unicode-digit labels are rejected.Coverage should also directly lock the stated contract: numeric prerelease identifiers sort below alphanumeric identifiers and compare numerically; shorter prerelease lists sort lower when equal so far; optional leading
vis accepted; build metadata does not affect precedence; leading-zero numeric identifiers and malformed refs are filtered; deterministic no-SemVer fallback remains. Current tests do not directly exercise those edge rules.Verification: full local suite 24 passed; Ruff format/lint and
py_compilepassed; Forgejo CI status is success for this exact SHA (run #12 / run id 3401). No secrets or unrelated-scope changes found. PR remains draft and was not merged.Forgejo refused the explicit request-changes review because the configured reviewer login (
clawlter) is also the PR author (reject your own pull is not allowed), so this durable PR comment records the disposition.Independent technical review of
c630909155: code is APPROVAL-READY, but this is a non-approving COMMENT disposition because Forgejo rejected the attempted APPROVED review with HTTP 422 “approve your own pull is not allowed.” The available credential resolves to the PR author; a distinct Forgejo reviewer must submit the formal approval.Verified from fetched Git refs/objects: refs/pull/13/head and origin/audit/semantic-tag-ordering both resolve to
c630909155; origin/main and the merge base resolve to9d9b0eaf09. Reviewed the complete 3-file, +53/-4 base-to-head diff.No blocking or important code findings. The strict SemVer key correctly handles numeric core precedence, release-over-prerelease, numeric-before-alphanumeric prerelease identifiers, identifier-list precedence, optional leading v, and build-metadata neutrality. Invalid semantic tags are ignored when valid versions exist; lexical max is a deterministic documented fallback when none do. README matches behavior.
Local commands all exited 0: .venv/bin/python -m pytest -q (24 passed); .venv/bin/ruff format --check . (7 files already formatted); .venv/bin/ruff check . (all checks passed); .venv/bin/python -m py_compile shim.py ensure_shim.py cron/ensure_camofox_firecrawl_shim.py check_updates.py; independent inline strict-SemVer behavior matrix; git diff --check base..head.
Forgejo Actions run 3401 / repository run #12 is success and associated with exact commit
c630909155. Its only job, “Validate formatting, linting, and tests” (job 3800), succeeded; all displayed setup, checkout, dependency, format, lint, compile, test, and completion steps succeeded.Security/scope: no new dependency, workflow, network destination, credential handling, executable side effect, or apparent secret exposure. Scope is limited to semantic tag selection, two behavior tests, and matching documentation. Minor residual risk: committed tests cover the reported lexical-order regression and fallback, but not every strict-SemVer edge independently exercised during review.
Exact unblock criterion: a distinct non-author Forgejo identity reviews this unchanged SHA and submits APPROVED. If the SHA moves, repeat the review.
CHANGE REQUEST for
c630909155(posted as COMMENT only because Forgejo rejected the authenticated author account’s REQUEST_CHANGES submission with “reject your own pull is not allowed”). This head is not approved.IMPORTANT — check_updates.py:29-32 and :64 do not implement strict SemVer ASCII syntax. Python regex
\dandstr.isdigit()are Unicode-aware, so non-SemVer labels such as1١.2.3,1.2.3-1١, and1.2.3-١aare accepted; the first is parsed as major 11 and can incorrectly outrank valid tags. An independent behavior matrix reproduced all three acceptances. Replace\d/Unicode digit classification with explicit ASCII[0-9]checks throughout, then add behavior tests proving non-ASCII digits are filtered in core, numeric prerelease, and alphanumeric prerelease positions.All other reviewed behavior was acceptable: canonical prerelease ordering (including numeric before alphanumeric and prefix-length rules), release over prerelease, optional leading-v equivalence, build-metadata-neutral precedence, deterministic lexical fallback with no valid SemVer tags, and README accuracy. Complete scope: README.md, check_updates.py, tests/test_check_updates.py (+53/-4). No dependency/workflow change, secret exposure, new credential path, or supply-chain expansion found.
Local exit-0 checks:
.venv/bin/python -m pytest -q(24 passed);.venv/bin/ruff format --check .;.venv/bin/ruff check .;.venv/bin/python -m py_compile shim.py ensure_shim.py cron/ensure_camofox_firecrawl_shim.py check_updates.py;git diff --check 9d9b0eaf09bdae8681a6be510a65a8d0a5d10843..c63090915551ae14ceb7232dcd502a6d3209909c. The independent strict-SemVer matrix exited 1 because of the reproduced defect.CI: Forgejo Actions database run 3401 / repository run #12 reports success for exact commit
c630909155. Sole job 3800, “Validate formatting, linting, and tests,” is success; authenticated run-page evidence reports setup, checkout, Python setup, dependency installation, format, lint, compile, tests, and completion all successful.Exact unblock criteria: restrict every SemVer digit match/classification to ASCII, add the three regression classes above, obtain successful exact-head CI, and repeat independent review against the new head using a distinct non-author Forgejo reviewer able to submit REQUEST_CHANGES/APPROVED. If the SHA moves, re-review the complete new diff.
REQUEST CHANGES for
c63090915551ae14ceb7232dcd502a6d3209909c.Important correctness finding — strict SemVer filtering accepts non-ASCII digits.
SEMANTIC_TAG_PATTERNuses Python\din core and prerelease numeric positions. Python treats\das Unicode decimal digits, while SemVer numeric identifiers are restricted to ASCII[0-9]. Reproduced at this head:semantic_tag_key("1٢.0.0")returns(12, 0, 0, (1, ()));semantic_tag_key("1.0.0-1٢")returns a valid numeric prerelease key; andsemantic_tag_key("1.0.0-٢alpha")is also accepted. These tags must be rejected as invalid.Unblock criteria:
[0-9]semantics (or an equivalently strict parser).Verification: complete base-to-head diff reviewed.
.venv/bin/ruff format --check .,.venv/bin/ruff check .,.venv/bin/python -m py_compile shim.py ensure_shim.py cron/ensure_camofox_firecrawl_shim.py check_updates.py, and.venv/bin/python -m pytest -qall exited 0 (24 passed). Forgejo Actions run 3401 / repository run #12 succeeded for this exact SHA with one successful job, “Validate formatting, linting, and tests,” and all nine displayed steps successful. No dependency changes, secret exposure, or unintended scope found.Forgejo refused an official
REQUEST_CHANGESdisposition because the only available authenticated account is the PR author (422: reject your own pull is not allowed). This COMMENT review records the blocking finding durably but is not an approval.REQUEST CHANGES for
c63090915551ae14ceb7232dcd502a6d3209909cafter independent complete-diff review.IMPORTANT — strict SemVer filtering is not ASCII-strict.
check_updates.pyuses Python\dandstr.isdigit(), both Unicode-aware, although SemVer permits only ASCII[0-9]. At this exact head,semantic_tag_key("1٢.0.0")returns a 12.0.0 key,semantic_tag_key("1.0.0-1٢")returns numeric prerelease 12, andsemantic_tag_key("1.0.0-٢alpha")is accepted. An invalid remote tag can therefore outrank valid tags instead of being filtered.Unblock criteria: (1) enforce ASCII digits throughout the parser; (2) add behavior tests rejecting non-ASCII digits in core, numeric-prerelease, and alphanumeric-prerelease positions; (3) directly cover numeric-vs-alphanumeric prerelease ordering and build-metadata-neutral precedence; (4) rerun full validation and exact-head CI; (5) independently re-review the new SHA.
Other assessment: canonical prerelease ordering, optional leading
v, release-over-prerelease, build metadata omission from the precedence key, deterministic lexical fallback, and README behavior are otherwise acceptable. The +53/-4 diff is limited toREADME.md,check_updates.py, andtests/test_check_updates.py; no dependency/workflow change, secret exposure, new credential path, supply-chain expansion, or unrelated scope was found.Verification: Ruff format/lint, Python compilation,
git diff --check, and full pytest (24 passed underenv -u PYTHONPATH) succeeded. The targeted ASCII-strict probe exited 1 and reproduced the defect. Forgejo Actions run id 3401 / repository run #12 is success for this exact commit; sole job 3800, “Validate formatting, linting, and tests,” and all displayed steps are successful. Worktree remained clean.If Forgejo records this as COMMENT, that is because its authenticated account is also the PR author and the server forbids a formal self-authored REQUEST_CHANGES disposition; the requested disposition remains changes required, not approval.
APPROVAL-READY for exact commit
2f42803798a393f816a298c60db5c4b01c237180after independent re-review againstmainat9d9b0eaf09bdae8681a6be510a65a8d0a5d10843. Forgejo records this as COMMENT only because its authenticatedclawlteraccount is also the PR author and rejected the immediately preceding formal APPROVED attempt withapprove your own pull is not allowed.No blocker, important, minor, or question findings. Strict ASCII SemVer parsing rejects Unicode decimal digits in core, numeric-prerelease, and alphanumeric-prerelease positions. Independent probes passed the canonical SemVer precedence chain, numeric-before-alphanumeric prerelease identifiers, release-over-prerelease, build-metadata-neutral precedence, optional leading
v, invalid-tag filtering, and deterministic lexical fallback. Behavior-focused regression tests cover the remediated classes. README accurately describes selection behavior.The complete
origin/main...reviewed-headdiff is limited toREADME.md,check_updates.py, andtests/test_check_updates.py(+91/-4). No dependency, workflow, secret, credential, network destination, executable side effect, supply-chain expansion, or unrelated scope change was found. Both commits have valid EDDSA signatures.Independent local verification exited 0:
ruff format --check .(7 files formatted),ruff check .,python -m py_compile ..., fullpytest -q(30 passed), a SemVer matrix (8 canonical tags and 19 invalid tags),git diff --check, clean merge-tree, and clean tracked/untracked worktree checks. Forgejo Actions run 3475 / repository run #23 issuccessfor this exact commit; exact-commit statusCI / Validate formatting, linting, and tests (pull_request)is successful.Immediately before this review submission, local HEAD,
origin/audit/semantic-tag-ordering,refs/pull/13/head, and the Forgejo API head all equaled the reviewed SHA.origin/main, API base, and merge base all equaled the stated main SHA. PR #13 remained open,draft=true, and unmerged. No merge or draft-state change was performed.Exact unblock criterion: a distinct non-author Forgejo identity may convert this approval-ready result into formal APPROVED for this unchanged SHA. If the SHA moves, repeat exact-head review.
Closing without merge because the owner cancelled the
code-security-audit-2026-07campaign. This does not revert previously merged work.Pull request closed