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
18 changes: 17 additions & 1 deletion src/specify_cli/workflows/overlays/layer_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,27 @@ def collect(self, workflow_id: str, *, include_disabled: bool = False) -> list[L
if path.is_symlink():
raise OverlayLoadError(path, ["Symlinked overlay files are not allowed"])
try:
data = yaml.safe_load(path.read_text(encoding="utf-8")) or {}
text = path.read_text(encoding="utf-8")
# ``safe_load`` returns None for BOTH an empty document and an
# explicit null scalar (``null``, ``~``, ``Null``, ``NULL``), so
# it cannot tell them apart on its own. ``compose`` yields no
# node only for a genuinely empty document.
is_empty_document = yaml.compose(text) is None
data = yaml.safe_load(text)
except yaml.YAMLError as exc:
raise OverlayLoadError(path, [f"Invalid YAML: {exc}"]) from exc
except (OSError, UnicodeDecodeError) as exc:
raise OverlayLoadError(path, [f"Cannot load overlay: {exc}"]) from exc
# Only a genuinely EMPTY document becomes an empty mapping, so its
# missing-field errors are reported. Every non-mapping document --
# including an explicit ``null``/``~`` and the falsy shapes ``[]``,
# ``false``, ``0``, ``''`` that the previous ``or {}`` masked -- must
# reach ``validate_overlay_yaml`` unchanged so it reports the wrong
# manifest shape, like the truthy twins (``- a``, ``hello``) already
# do. The sibling reader for these same files, ``_read_overlay`` in
# overlays/_commands.py, does not coerce either.
if is_empty_document:
data = {}
if (
not include_disabled
and isinstance(data, dict)
Expand Down
55 changes: 55 additions & 0 deletions tests/workflows/test_overlay_layer_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,61 @@ def _write_overlay_file(project_dir: Path, workflow_id: str, overlay_id: str, da
return path


class TestProjectOverlaySourceManifestShape:
"""A non-mapping overlay manifest is reported as a shape error."""

@pytest.mark.parametrize(
"content", ["[]", "false", "0", "''", "null", "~", "NULL"]
)
def test_falsy_non_mapping_manifest_reports_shape_error(
self, project_dir: Path, content: str
) -> None:
"""Every non-mapping document reports the mapping-shape error.

`validate_overlay_yaml` opens with an `isinstance(data, dict)` check, so a
truthy non-mapping (`- a`, `hello`) correctly reports "Overlay manifest
must be a mapping." Two things masked that for other documents:

* `yaml.safe_load(...) or {}` replaced the falsy shapes `[]`, `false`,
`0` and `''` with an empty mapping.
* `safe_load` returns `None` for an explicit null scalar (`null`, `~`,
`NULL`) as well as for an empty document, so a `data is None` check
swallowed those too.

Both now reach the validator unchanged; only a genuinely empty document
is normalised to `{}` (pinned separately below), using `yaml.compose`,
which yields no node only for an empty document.
"""
ov_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf"
ov_dir.mkdir(parents=True, exist_ok=True)
(ov_dir / "ov.yml").write_text(content, encoding="utf-8")

source = ProjectOverlaySource(project_dir)
with pytest.raises(OverlayLoadError) as exc_info:
source.collect("wf")

assert exc_info.value.errors == ["Overlay manifest must be a mapping."], (
exc_info.value.errors
)

def test_empty_document_still_reports_missing_fields(
self, project_dir: Path
) -> None:
"""An empty document is not a wrong shape — it is a mapping with no keys,
so the missing-field errors must still be what is reported."""
ov_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf"
ov_dir.mkdir(parents=True, exist_ok=True)
(ov_dir / "ov.yml").write_text("", encoding="utf-8")

source = ProjectOverlaySource(project_dir)
with pytest.raises(OverlayLoadError) as exc_info:
source.collect("wf")

assert any("is required" in err for err in exc_info.value.errors), (
exc_info.value.errors
)


class TestProjectOverlaySourceFileReadErrors:
"""File-read errors must be wrapped in OverlayLoadError, not leaked as raw tracebacks."""

Expand Down