fix: validate consolidated metadata listing keys and hierarchy

changes/4485.bugfix.md

@@ -0,0 +1,10 @@
+Validate consolidated metadata listings at parse time. In v3 listings,
+keys containing a reserved node name (`.`, `..`, `zarr.json`, or a
+`__`-prefixed segment) now emit a `ZarrDeprecationWarning` instead of
+being accepted silently; stores written before these names were rejected
+still open. Keys with empty path segments still raise
+`MetadataValidationError`, as do listings whose parent group entry is
+missing or that place a node below an array; those cases previously
+surfaced as `KeyError` or left the group unopenable. The reserved-name
+check does not apply to v2 listings, whose specification does not
+reserve node names.

src/zarr/core/group.py

@@ -64,6 +64,7 @@
     GroupNotFoundError,
     MetadataValidationError,
     NodeTypeValidationError,
+    ZarrDeprecationWarning,
     ZarrUserWarning,
 )
 from zarr.storage import StoreLike, StorePath
@@ -140,6 +141,53 @@ def _parse_async_node(
         raise TypeError(f"Unknown node type, got {type(node)}")
 
 
+def _validate_consolidated_key(key: str, zarr_format: ZarrFormat) -> None:
+    """A consolidated metadata key is a node path: every segment must be non-empty.
+
+    The v3 specification also reserves the names ``.``, ``..``, ``zarr.json``, and
+    every ``__``-prefixed name. Stores written before those names were rejected on
+    creation still open today, so for now such keys only warn. The v2 specification
+    does not reserve any node names, so no name checks apply to v2 listings."""
+    segments = key.split("/")
+    if any(not seg for seg in segments):
+        msg = f"Invalid node path in consolidated metadata: {key!r}"
+        raise MetadataValidationError(msg)
+    if zarr_format == 3 and any(
+        seg in (".", "..", ZARR_JSON) or seg.startswith("__") for seg in segments
+    ):
+        msg = (
+            f"Consolidated metadata lists {key!r}, which contains a name reserved "
+            "by the v3 specification. Rename the node, as reading such listings "
+            "will become an error."
+        )
+        warnings.warn(msg, category=ZarrDeprecationWarning, stacklevel=1)
+
+
+def _validate_consolidated_hierarchy(
+    metadata: dict[str, ArrayV2Metadata | ArrayV3Metadata | GroupMetadata],
+) -> None:
+    """Every non-root listing key must have all of its ancestors present as
+    group entries. A missing ancestor makes the group unreadable, and a
+    listing below an array is invalid: array nodes may not have children."""
+    for key in metadata:
+        parts = key.split("/")
+        for i in range(1, len(parts)):
+            prefix = "/".join(parts[:i])
+            parent = metadata.get(prefix)
+            if parent is None:
+                msg = (
+                    f"Consolidated metadata lists {key!r} without a listing "
+                    f"for its parent {prefix!r}"
+                )
+                raise MetadataValidationError(msg)
+            if not isinstance(parent, GroupMetadata):
+                msg = (
+                    f"Consolidated metadata lists {key!r} below {prefix!r}, "
+                    "which is an array; array nodes may not have children"
+                )
+                raise MetadataValidationError(msg)
+
+
 @dataclass(frozen=True)
 class ConsolidatedMetadata:
     """
@@ -199,6 +247,7 @@ def from_dict(cls, data: dict[str, JSON], *, path: str | None = None) -> Consoli
 
                 # zarr_format is present in v2 and v3.
                 zarr_format = parse_zarr_format(v["zarr_format"])
+                _validate_consolidated_key(k, zarr_format)
 
                 if zarr_format == 3:
                     node_type = parse_node_type(v.get("node_type", None))
@@ -216,6 +265,7 @@ def from_dict(cls, data: dict[str, JSON], *, path: str | None = None) -> Consoli
                 else:
                     assert_never(zarr_format)
 
+            _validate_consolidated_hierarchy(metadata)
             cls._flat_to_nested(metadata)
 
         return cls(metadata=metadata)

tests/test_metadata/test_consolidated.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import json
+import warnings
 from typing import TYPE_CHECKING, Any
 
 import numpy as np
@@ -22,7 +23,7 @@
 from zarr.core.group import ConsolidatedMetadata, GroupMetadata
 from zarr.core.metadata import ArrayV3Metadata
 from zarr.core.metadata.v2 import ArrayV2Metadata
-from zarr.errors import ZarrUserWarning
+from zarr.errors import MetadataValidationError, ZarrDeprecationWarning, ZarrUserWarning
 from zarr.storage import StorePath
 
 if TYPE_CHECKING:
@@ -901,3 +902,101 @@ async def test_consolidated_metadata_case_differing_siblings(memory_store: Store
         "obs/study/categories",
         "obs/study/codes",
     ]
+
+
+# Regression tests for https://github.com/zarr-developers/zarr-python/issues/4453
+# item 2: consolidated metadata accepted invalid listing keys, a listing with no
+# parent entry, and a listing below an array.
+
+_ARRAY_V3 = {
+    "zarr_format": 3,
+    "node_type": "array",
+    "shape": [1],
+    "data_type": "int8",
+    "chunk_grid": {"name": "regular", "configuration": {"chunk_shape": [1]}},
+    "chunk_key_encoding": {"name": "default"},
+    "fill_value": 0,
+    "codecs": [{"name": "bytes", "configuration": {"endian": "little"}}],
+}
+_GROUP_V3 = {"zarr_format": 3, "node_type": "group", "attributes": {}}
+_GROUP_V2 = {"zarr_format": 2}
+
+
+def _consolidated_doc(metadata: dict[str, Any]) -> dict[str, Any]:
+    return {"kind": "inline", "must_understand": False, "metadata": metadata}
+
+
+@pytest.mark.parametrize(
+    "key",
+    ["", "a//b", "/a", "a/"],
+)
+def test_consolidated_metadata_rejects_invalid_keys(key: str) -> None:
+    """Listing keys are node paths; every segment must be non-empty."""
+    with pytest.raises(MetadataValidationError, match="Invalid node path"):
+        ConsolidatedMetadata.from_dict(_consolidated_doc({key: _GROUP_V3}))
+
+
+@pytest.mark.parametrize(
+    "key",
+    [".", "..", "zarr.json", "__a", "a/zarr.json", "a/__x"],
+)
+def test_consolidated_metadata_warns_on_reserved_v3_keys(key: str) -> None:
+    """v3 reserves '.', '..', 'zarr.json', and '__'-prefixed names, but stores
+    written before the names were rejected still open, so they only warn."""
+    with pytest.warns(ZarrDeprecationWarning, match="reserved"):
+        consolidated = ConsolidatedMetadata.from_dict(
+            _consolidated_doc({key: _GROUP_V3, "a": _GROUP_V3})
+        )
+    if "/" in key:
+        parent, leaf = key.split("/")
+        parent_meta = consolidated.metadata[parent]
+        assert isinstance(parent_meta, GroupMetadata)
+        nested = parent_meta.consolidated_metadata
+        assert nested is not None
+        assert leaf in nested.metadata
+    else:
+        assert key in consolidated.metadata
+
+
+def test_consolidated_metadata_reserved_name_below_missing_parent() -> None:
+    """A reserved name warns before the missing-parent error is raised."""
+    with pytest.warns(ZarrDeprecationWarning, match="reserved"):
+        with pytest.raises(MetadataValidationError, match="without a listing"):
+            ConsolidatedMetadata.from_dict(_consolidated_doc({"a": _GROUP_V3, "a/../b": _GROUP_V3}))
+
+
+@pytest.mark.parametrize(
+    "key",
+    [".", "..", "zarr.json", "__a"],
+)
+def test_consolidated_metadata_ignores_reserved_v2_keys(key: str) -> None:
+    """The v2 specification does not reserve node names."""
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        consolidated = ConsolidatedMetadata.from_dict(_consolidated_doc({key: _GROUP_V2}))
+    assert key in consolidated.metadata
+
+
+def test_consolidated_metadata_rejects_child_without_parent() -> None:
+    """A listing for 'a/b' with no 'a' listing made the root group unopenable."""
+    with pytest.raises(MetadataValidationError, match="without a listing"):
+        ConsolidatedMetadata.from_dict(_consolidated_doc({"a/b": _GROUP_V3}))
+
+
+def test_consolidated_metadata_rejects_node_below_array() -> None:
+    """Array nodes may not have children; 'arr/child' below an 'arr' array is
+    invalid."""
+    with pytest.raises(MetadataValidationError, match="array"):
+        ConsolidatedMetadata.from_dict(
+            _consolidated_doc({"arr": _ARRAY_V3, "arr/child": _GROUP_V3})
+        )
+
+
+def test_consolidated_metadata_accepts_nested_listings() -> None:
+    consolidated = ConsolidatedMetadata.from_dict(
+        _consolidated_doc({"a": _GROUP_V3, "a/b": _GROUP_V3, "a/b/c": _ARRAY_V3})
+    )
+    a = consolidated.metadata["a"]
+    assert isinstance(a, GroupMetadata)
+    assert a.consolidated_metadata is not None
+    assert sorted(a.consolidated_metadata.metadata) == ["b"]