refactor(storage): gate clone() behind an opt-in allowlist; expand duplicate tests
Follow-up polish from the duplicate-subgraph review: - BaseSqliteStore.clone() now requires an opt-in `_cloneable` flag (default off), so a new or secret-bearing store can never be cloned by accident — defence-in-depth on top of the endpoint's `_DUPLICABLE_KINDS` restriction. Only value-source and colour-strip stores are flagged. - Fix `_check_name_unique` annotation (`str | None = None`). - Add tests: `layers[].brightness_source_id` remap, the warnings safety net firing, the clone allowlist invariant, and clone() refusing a non-cloneable store.
This commit is contained in:
@@ -94,6 +94,35 @@ def test_duplicate_remaps_bindable_slot_to_cloned_value_source(stores):
|
||||
assert c_new["intensity"]["source_id"] == res["id_map"][vs.id]
|
||||
|
||||
|
||||
def test_duplicate_remaps_layer_brightness_source(stores):
|
||||
"""A composite layer's value-source brightness binding (list + value ref) is
|
||||
remapped when that value source is also in the selection."""
|
||||
css, vss = stores
|
||||
vs = vss.create_source(name="Dim", source_type="static", value=0.3)
|
||||
leaf = css.create_source(name="Leaf", source_type="single_color", colors=[[9, 9, 9]])
|
||||
comp = css.create_source(
|
||||
name="Comp",
|
||||
source_type="composite",
|
||||
layers=[{**_layer(leaf.id), "brightness_source_id": vs.id}],
|
||||
)
|
||||
res = _duplicate_subgraph([comp.id, leaf.id, vs.id], " (copy)")
|
||||
layer = serialize_entity(css.get(res["id_map"][comp.id]))["layers"][0]
|
||||
assert layer["source_id"] == res["id_map"][leaf.id]
|
||||
assert layer["brightness_source_id"] == res["id_map"][vs.id]
|
||||
|
||||
|
||||
def test_duplicate_safety_net_flags_unremapped_ref(stores, monkeypatch):
|
||||
"""If a reference somehow isn't remapped, the post-clone safety net reports
|
||||
it in `warnings` rather than silently leaving two pipelines sharing a node."""
|
||||
css, _vss = stores
|
||||
b = css.create_source(name="B", source_type="single_color", colors=[[1, 2, 3]])
|
||||
a = css.create_source(name="A", source_type="composite", layers=[_layer(b.id)])
|
||||
# Force remap to a no-op so the clone keeps pointing at the original in-set id.
|
||||
monkeypatch.setattr("ledgrab.api.routes.graph.remap_refs", lambda *a, **k: 0)
|
||||
res = _duplicate_subgraph([a.id, b.id], " (copy)")
|
||||
assert any(w["id"] == res["id_map"][a.id] for w in res["warnings"])
|
||||
|
||||
|
||||
def test_duplicate_value_source(stores):
|
||||
_css, vss = stores
|
||||
v = vss.create_source(name="V", source_type="static", value=0.5)
|
||||
@@ -120,3 +149,23 @@ def test_duplicate_name_collision_is_suffixed(stores):
|
||||
res = _duplicate_subgraph([v.id], " (copy)")
|
||||
new = vss.get(res["id_map"][v.id])
|
||||
assert new.name == "V (copy) 2"
|
||||
|
||||
|
||||
def test_clone_allowlist_invariant():
|
||||
"""Only explicitly-flagged (secret-free) stores are cloneable; the base
|
||||
default is off so a new store is never cloneable by accident."""
|
||||
from ledgrab.storage.base_sqlite_store import BaseSqliteStore
|
||||
|
||||
assert BaseSqliteStore._cloneable is False
|
||||
assert ColorStripStore._cloneable is True
|
||||
assert ValueSourceStore._cloneable is True
|
||||
|
||||
|
||||
def test_clone_refuses_non_cloneable_store(stores, monkeypatch):
|
||||
"""clone() refuses stores not on the allowlist (defence-in-depth even though
|
||||
the duplicate endpoint already restricts to the safe kinds)."""
|
||||
css, _vss = stores
|
||||
src = css.create_source(name="X", source_type="single_color", colors=[[1, 1, 1]])
|
||||
monkeypatch.setattr(type(css), "_cloneable", False)
|
||||
with pytest.raises(NotImplementedError):
|
||||
css.clone(src.id, "X (copy)")
|
||||
|
||||
Reference in New Issue
Block a user