diff --git a/server/src/ledgrab/storage/base_sqlite_store.py b/server/src/ledgrab/storage/base_sqlite_store.py index 1d253d4..46feb3e 100644 --- a/server/src/ledgrab/storage/base_sqlite_store.py +++ b/server/src/ledgrab/storage/base_sqlite_store.py @@ -29,6 +29,10 @@ class BaseSqliteStore(Generic[T]): _table_name: str _entity_name: str + # Opt-in allowlist for clone(): defaults off so a new (possibly + # secret-bearing) store is never cloneable by accident. Subclasses that hold + # no inline secrets and are safe to duplicate set this True. + _cloneable: bool = False def __init__(self, db: Database, deserializer: Callable[[dict], T]): self._db = db @@ -153,6 +157,8 @@ class BaseSqliteStore(Generic[T]): model might hold. Callers must restrict cloning to non-secret-bearing kinds (see ``_DUPLICABLE_KINDS`` in ``api/routes/graph.py``). """ + if not self._cloneable: + raise NotImplementedError(f"{self._entity_name} store does not support cloning") with self._lock: if item_id not in self._items: from ledgrab.storage.base_store import EntityNotFoundError @@ -180,7 +186,7 @@ class BaseSqliteStore(Generic[T]): # -- Helpers ------------------------------------------------------------- - def _check_name_unique(self, name: str, exclude_id: str = None) -> None: + def _check_name_unique(self, name: str, exclude_id: str | None = None) -> None: """Raise ValueError if *name* is empty or already taken. Must be called while holding ``self._lock``. diff --git a/server/src/ledgrab/storage/color_strip_store.py b/server/src/ledgrab/storage/color_strip_store.py index 5c7acc1..90140b1 100644 --- a/server/src/ledgrab/storage/color_strip_store.py +++ b/server/src/ledgrab/storage/color_strip_store.py @@ -33,6 +33,7 @@ class ColorStripStore(BaseSqliteStore[ColorStripSource]): _table_name = "color_strip_sources" _entity_name = "Color strip source" + _cloneable = True # no inline secrets — only references shared entities by id def __init__(self, db: Database): super().__init__(db, ColorStripSource.from_dict) diff --git a/server/src/ledgrab/storage/value_source_store.py b/server/src/ledgrab/storage/value_source_store.py index 8374727..4fb4021 100644 --- a/server/src/ledgrab/storage/value_source_store.py +++ b/server/src/ledgrab/storage/value_source_store.py @@ -27,6 +27,7 @@ class ValueSourceStore(BaseSqliteStore[ValueSource]): _table_name = "value_sources" _entity_name = "Value source" + _cloneable = True # no inline secrets — only references shared entities by id def __init__(self, db: Database): super().__init__(db, ValueSource.from_dict) diff --git a/server/tests/api/test_graph_duplicate.py b/server/tests/api/test_graph_duplicate.py index ba0b991..8d677a2 100644 --- a/server/tests/api/test_graph_duplicate.py +++ b/server/tests/api/test_graph_duplicate.py @@ -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)")