fix(store): address review — malformed manifests, and suffixed versions

Both CodeRabbit findings on #433 verified against the code and fixed.

1. Malformed manifest sections raised instead of degrading. `requires` as a
   list hit AttributeError ('list' object has no attribute 'get') and
   `versions` as a mapping hit KeyError: 0. Both reproduced.

   This got worse with the sunset rule in the previous commit: that branch
   resolves the floor for *every* manifest on an untrustworthy core, where the
   old code returned early. One hand-edited or third-party file with the wrong
   shape would have taken down the whole install path rather than just itself.
   Container types are now validated and an unrecognised shape reads as "no
   declared floor".

2. Prerelease and build metadata leaked into the version numbers. The digit
   scrape parsed "3.2.0+build42" as (3, 2, 42) and "3.2.0-rc1" as (3, 2, 1) --
   a release candidate ranking above its own release. Both fed reject
   decisions, and the consequence was demonstrable: a plugin pinned to exactly
   "3.2.0" refused a core running 3.2.0+build42, which is that same version.

   The suggested remedy -- use the strict token parser -- would not have
   fixed it. _parse_strict validates the shape but delegates the numbers to
   parse_semver, so it returned the same (3, 2, 42). The bug is in the scrape,
   so suffixes are now dropped before it. Prereleases compare equal to their
   release rather than below it; full prerelease ordering is more than any
   caller needs and equal is far closer to right than what it did before.

parse_semver is shared with PluginLoader, so its suite was re-run: unchanged,
and it only ever gets more correct here.

Verified: 839 core unit tests pass, 21 of them new -- six malformed shapes,
five suffixed forms, and the two demonstrated regressions. The real-registry
sweep is unchanged at 0 of 42 refused across cores 1.0.0, 3.1.0, 3.2.0,
3.2.0+build42 and an unparseable string.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
This commit is contained in:
ChuckBuilds
2026-08-04 13:29:01 -04:00
co-authored by Claude Opus 5
parent 08b16165fa
commit 06657de428
2 changed files with 88 additions and 7 deletions
+29 -7
View File
@@ -40,7 +40,20 @@ def parse_semver(value: Any) -> Optional[Tuple[int, int, int]]:
3-tuple, or ``None`` when unparseable. A leading ``v`` is tolerated.""" 3-tuple, or ``None`` when unparseable. A leading ``v`` is tolerated."""
if not isinstance(value, str): if not isinstance(value, str):
return None return None
parts = value.strip().lstrip('v').split('.') text = value.strip().lstrip('v')
# Drop the prerelease/build suffix before scraping digits. Without this the
# scrape pulls them into the numbers: "3.2.0+build42" parsed as (3, 2, 42)
# and "3.2.0-rc1" as (3, 2, 1) -- a release candidate ranking *above* its
# own release, and a build of 3.2.0 failing an exact "3.2.0" match.
#
# Prereleases compare equal to their release here rather than below it.
# Full prerelease ordering is more than any caller needs, and equal is far
# closer to right than the old behaviour.
for sep in ('+', '-'):
head, found, _tail = text.partition(sep)
if found:
text = head
parts = text.split('.')
try: try:
nums = [int(''.join(ch for ch in p if ch.isdigit()) or 0) for p in parts[:3]] nums = [int(''.join(ch for ch in p if ch.isdigit()) or 0) for p in parts[:3]]
except ValueError: except ValueError:
@@ -143,16 +156,25 @@ def declared_min_version(manifest: Dict[str, Any]) -> Optional[str]:
of `ledmatrix_min_version` (`store_manager._validate_manifest_fields` flags of `ledmatrix_min_version` (`store_manager._validate_manifest_fields` flags
it); both are read because a large share of published manifests still carry it); both are read because a large share of published manifests still carry
the old one. the old one.
Container types are validated rather than assumed. A hand-edited or
third-party manifest can carry `requires` as a list or `versions` as a
mapping, and both used to raise out of here (`AttributeError` and
`KeyError` respectively). That now matters far more than it did: the
untrustworthy-core branch of :func:`check` calls this for *every* manifest,
so one malformed file would take down the install path rather than just
itself. A shape we do not recognise means "no declared floor".
""" """
declared = ( declared = manifest.get('min_ledmatrix_version')
manifest.get('min_ledmatrix_version') if not declared:
or (manifest.get('requires') or {}).get('min_ledmatrix_version') requires = manifest.get('requires')
) if isinstance(requires, dict):
declared = requires.get('min_ledmatrix_version')
if declared: if declared:
return declared return declared
versions = manifest.get('versions') or [] versions = manifest.get('versions')
if versions and isinstance(versions[0], dict): if isinstance(versions, list) and versions and isinstance(versions[0], dict):
return (versions[0].get('ledmatrix_min_version') return (versions[0].get('ledmatrix_min_version')
or versions[0].get('ledmatrix_min')) or versions[0].get('ledmatrix_min'))
return None return None
+59
View File
@@ -369,3 +369,62 @@ class TestUntrustworthyCoreAndTheSunset:
assert "too old to identify" not in reason, ( assert "too old to identify" not in reason, (
"a believable version should get the ordinary message" "a believable version should get the ordinary message"
) )
class TestMalformedManifests:
"""A manifest we cannot parse must read as "no declared floor", not raise.
This matters more since the untrustworthy-core branch of check() began
resolving the floor for *every* manifest: one hand-edited or third-party
file with the wrong shape would take down the whole install path rather
than just itself.
"""
@pytest.mark.parametrize("manifest", [
{"requires": ["python>=3.9"]}, # a list, not a mapping
{"requires": "python>=3.9"}, # a bare string
{"versions": {"a": 1}}, # a mapping, not a list
{"versions": "1.0.0"}, # a bare string
{"versions": [None]}, # a list of the wrong thing
{"versions": []},
])
def test_shape_errors_read_as_no_floor(self, manifest):
assert compatibility.declared_min_version(manifest) is None
assert compatibility.check(manifest, "1.0.0") == (True, None)
assert compatibility.check(manifest, "3.2.0") == (True, None)
def test_a_valid_requires_block_still_works(self):
assert compatibility.declared_min_version(
{"requires": {"min_ledmatrix_version": "3.2.0"}}) == "3.2.0"
class TestSuffixedVersions:
"""Prerelease and build metadata must not leak into the numbers.
The digit scrape used to pull them in: "3.2.0+build42" became (3, 2, 42)
and "3.2.0-rc1" became (3, 2, 1) — a release candidate ranking above its
own release. Both fed reject decisions.
"""
@pytest.mark.parametrize("text,expected", [
("3.2.0", (3, 2, 0)),
("3.2.0+build42", (3, 2, 0)),
("3.2.0-rc1", (3, 2, 0)),
("3.2.0-rc.1+build.9", (3, 2, 0)),
("v3.2.0+build42", (3, 2, 0)),
])
def test_suffixes_are_dropped(self, text, expected):
assert compatibility.parse_semver(text) == expected
def test_a_build_of_the_pinned_version_is_not_refused(self):
"""The regression: an exact "3.2.0" pin refused a core running
3.2.0+build42, which is that same version."""
ok, reason = compatibility.check(
{"compatible_versions": ["3.2.0"]}, "3.2.0+build42")
assert ok is True, f"refused a build of the pinned version: {reason}"
def test_a_release_candidate_does_not_outrank_its_release(self):
m = {"versions": [{"ledmatrix_min_version": "3.2.0"}]}
assert compatibility.check(m, "3.2.0-rc1")[0] is True
# ...and still refuses something genuinely older.
assert compatibility.check(m, "3.1.0-rc1")[0] is False