Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions src/specify_cli/bundler/models/catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

from .. import BundlerError
from ..lib.yamlio import ensure_within, load_yaml
from .manifest import _text

CONFIG_FILENAME = "bundle-catalogs.yml"
# Supported bundle-catalogs.yml schema (major version). Both readers of the
Expand Down Expand Up @@ -70,8 +71,14 @@ def install_allowed(self) -> bool:
def from_dict(cls, data: Any, scope: Scope) -> "CatalogSource":
if not isinstance(data, dict):
raise BundlerError("Each catalog source must be a mapping.")
source_id = str(data.get("id", "")).strip()
url = str(data.get("url", "")).strip()
# ``_text`` rather than ``str(...get(k, ""))``: the default only covers a
# *missing* key. A key present but null -- how YAML spells an empty field
# (``id:`` with nothing after it) -- yields ``None``, and ``str(None)``
# is the literal ``"None"``, which is truthy and so sailed straight past
# the required-field guards below: a source with ``id: null`` was
# accepted and registered under the name ``"None"``.
source_id = _text(data.get("id"))
url = _text(data.get("url"))
if not source_id:
raise BundlerError("A catalog source is missing its 'id'.")
if not url:
Expand Down Expand Up @@ -185,14 +192,16 @@ def from_dict(cls, data: Any) -> "CatalogEntry":
)
return cls(
id=entry_id,
name=str(data.get("name", "")).strip(),
version=str(data.get("version", "")).strip(),
role=str(data.get("role", "")).strip(),
description=str(data.get("description", "")).strip(),
author=str(data.get("author", "")).strip(),
license=str(data.get("license", "")).strip(),
download_url=str(data.get("download_url", "")).strip(),
requires_speckit_version=str(requires.get("speckit_version", "")).strip(),
# See the note in ``CatalogSource.from_dict``: an explicitly null
# field must read as empty, not as the literal string "None".
name=_text(data.get("name")),
version=_text(data.get("version")),
role=_text(data.get("role")),
description=_text(data.get("description")),
author=_text(data.get("author")),
license=_text(data.get("license")),
download_url=_text(data.get("download_url")),
requires_speckit_version=_text(requires.get("speckit_version")),
sha256=(
None
if data.get("sha256") is None
Expand Down
66 changes: 66 additions & 0 deletions tests/contract/test_catalog_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,72 @@ def test_catalog_entry_rejects_non_boolean_verified():
CatalogEntry.from_dict(data)


@pytest.mark.parametrize(
"field",
[
"name",
"version",
"role",
"description",
"author",
"license",
"download_url",
],
)
def test_catalog_entry_explicit_null_field_reads_as_empty(field: str):
"""An explicitly null field must read as "", not the literal "None".

`str(data.get(key, ""))` only defaults for a *missing* key. A key present
but null — how YAML spells an empty field (`author:` with nothing after
it) — yields `None`, and `str(None)` is the truthy string `"None"`. The
same constructor already guards `sha256` and `repository` against exactly
this.
"""
from specify_cli.bundler.models.catalog import CatalogEntry

data = catalog_entry_dict("demo")
data[field] = None

entry = CatalogEntry.from_dict(data)

assert getattr(entry, field) == ""


def test_catalog_source_rejects_an_explicitly_null_id():
"""`id: null` must be refused, not registered as a source named "None".

The `if not source_id` guard was defeated by the truthy literal, so the
source was accepted and carried the name `"None"` into the stack.
"""
from specify_cli.bundler.models.catalog import CatalogSource, Scope

with pytest.raises(BundlerError, match="missing its 'id'"):
CatalogSource.from_dict(
{
"id": None,
"url": "https://example.test/catalog.json",
"priority": 5,
"install_policy": "install-allowed",
},
Scope.PROJECT,
)


def test_catalog_source_rejects_an_explicitly_null_url():
from specify_cli.bundler.models.catalog import CatalogSource, Scope

with pytest.raises(BundlerError, match="missing its 'url'"):
CatalogSource.from_dict(
{
"id": "demo",
"url": None,
"priority": 5,
"install_policy": "install-allowed",
},
Scope.PROJECT,
)


def test_catalog_entry_preserves_sha256_through_provenance():
digest = "a" * 64
payload = catalog_payload(
Expand Down