fix(updater): linear-time version parsing, skip malformed release tags (#172)

[rowkav09]

What changed

• parseVersion in src/update-check.js no longer uses the single SemVer regex with the nested [0-9A-Za-z-]*[A-Za-z-][0-9A-Za-z-]* groups. It matches the shape with one unambiguous pattern (-([0-9A-Za-z.-]+)), then checks each prerelease identifier on its own: non-empty, alphanumerics and hyphens only, no leading zeros on numeric parts. Versions are capped at 128 characters. Valid versions parse exactly as before.
• A release with a malformed tag is now skipped, so one bad tag no longer fails the whole update check.
• Tests: crafted tags from the CodeQL reports finish in linear time, invalid identifiers and overlong versions are rejected, and valid mixed identifiers still parse.

Why

Fixes CodeQL alerts #2 (js/redos, error) and #1 (js/polynomial-redos, warning) on src/update-check.js. Release tags come from the network, so a crafted tag could hang the updater.

Checks

• Tests pass locally (full suite green)
• No secrets, tokens, server URLs, or personal media data are committed
• Docs or tests were updated when behavior changed
• The change is scoped to one roadmap issue

Issue

Part of #172

[github-actions]

/mira pause

[mira-reviewer-rk]

Mira PR Walkthrough

This PR fixes a ReDoS vulnerability in the version parsing logic by replacing a complex regex with linear-time validation. It also improves robustness by skipping malformed release tags instead of failing the entire update check. The changes ensure security while maintaining backward compatibility with valid version formats.

⏳ Code review in progress…

---

Comment @mira-reviewer-rk help to get the list of available commands and usage tips.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

[mira-reviewer-rk]

Mira Review Summary

The PR addresses CodeQL alerts by replacing the exponential-time SemVer regex with linear-time parsing and adding graceful handling of malformed tags. The changes look good overall. I found a minor issue: empty tag names would pass the type check but be rejected later; adding explicit empty check improves clarity. Also, the test's 500ms timeout may be flaky on slow CI. The error messages could be more specific for debugging. No critical bugs found.

[mira-reviewer-rk]

Clarity
💡 Suggestion

Error message could be more specific

The error message "version: expected semver" is thrown for multiple different failure cases: non-string, too long, pattern mismatch, invalid identifier. When debugging, it's hard to know which condition failed. Consider adding more specific error messages for each validation step, or at least differentiate between length and pattern failures.

Suggested change

	}
	if (typeof value !== "string") throw new TypeError("version: expected semver");
	if (value.length > MAX_VERSION_LENGTH) throw new TypeError("version: too long");

---

Prompt for AI Agents

In src/update-check.js line 30, split the validation to provide more specific error messages. First check if value is a string, then check length separately with a distinct error message like 'version: too long'.

Apply this code change:

  if (typeof value !== "string") throw new TypeError("version: expected semver");
  if (value.length > MAX_VERSION_LENGTH) throw new TypeError("version: too long");

Not useful? Reply @mira-reviewer-rk reject to dismiss this suggestion.

[rowkav09]

Done: the length case now says "expected semver (too long)".