From ef35a35848b0794a209ad3d73cf7d24f2f495ebb Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:08:47 +0300 Subject: [PATCH] fix: import guard sees a group-level helper importing a module via ..; provider errors said once per provider; tests for both log lines (attacker round 18) Co-Authored-By: Claude Sonnet 5.5 --- core/registry.py | 2 +- core/tests/test_module_imports.py | 13 ++++++++----- core/tests/test_registry.py | 13 +++++++++++++ core/tests/test_retired_modules.py | 8 ++++++++ 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/core/registry.py b/core/registry.py index d7816c48..467a4069 100644 --- a/core/registry.py +++ b/core/registry.py @@ -54,7 +54,7 @@ class Registry: answer = fn(*args, **kwargs) except Exception as exc: # noqa: BLE001 from . import log - log.once(f"provider:{capability}:{_spec.id}:{exc}", log.WARNING, f"{capability} provider {_spec.id}", + log.once(f"provider:{capability}:{_spec.id}", log.WARNING, f"{capability} provider {_spec.id}", f"failed while answering -- {type(exc).__name__}: {exc}") # skipped, never silently, said once continue if answer is not None: diff --git a/core/tests/test_module_imports.py b/core/tests/test_module_imports.py index 9363c52e..bcdaedf5 100644 --- a/core/tests/test_module_imports.py +++ b/core/tests/test_module_imports.py @@ -31,7 +31,7 @@ def _absolute_core_imports(path: Path): Walks the AST rather than the text, so an import nested in a function, a method or a try block is found exactly like a top-level one. """ - tree = ast.parse(path.read_text(), filename=str(path)) + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) found = [] for node in ast.walk(tree): if isinstance(node, ast.ImportFrom): @@ -61,7 +61,7 @@ def test_a_module_reaches_core_only_through_the_shim(path): def _module_dirs(): """group/name of every directory that announces a module (has `ID = ` in its __init__).""" - return {(p.parent.parent.name, p.parent.name) for p in MODULES.glob("*/*/__init__.py") if "\nID = " in "\n" + p.read_text()} + return {(p.parent.parent.name, p.parent.name) for p in MODULES.glob("*/*/__init__.py") if "\nID = " in "\n" + p.read_text(encoding="utf-8")} def _import_nodes(body): @@ -92,7 +92,7 @@ def _other_module_imports(path: Path, mods=None, root: Path = MODULES): group = parts[0] own = parts[:2] if len(parts) >= 3 else None # a file at group level belongs to no module found = [] - for node in _import_nodes(ast.parse(path.read_text()).body): + for node in _import_nodes(ast.parse(path.read_text(encoding="utf-8")).body): names = [a.name for a in node.names] if isinstance(node, ast.Import): targets = [tuple(n.split(".")[1:3]) for n in names if n.split(".")[0] == "modules"] @@ -102,6 +102,8 @@ def _other_module_imports(path: Path, mods=None, root: Path = MODULES): targets = ([tuple(mod[1:3])] if len(mod) >= 3 else [(mod[1], n) for n in names]) if mod[:1] == ["modules"] and len(mod) >= 2 else [] elif node.level >= 3: targets = [tuple(mod[:2])] if len(mod) >= 2 else [(mod[0], n) for n in names] if mod else [] + elif node.level == 2 and own is None: # from a group-level file, `..` is the modules root: `..group.module` + targets = [tuple(mod[:2])] if len(mod) >= 2 else [(mod[0], n) for n in names] if mod else [] elif node.level == 2: targets = [(group, mod[0])] if mod else [(group, n) for n in names] elif node.level == 1 and own is None: # a group-level helper: its `.x` is a sibling of the modules @@ -144,5 +146,6 @@ def test_the_guard_sees_every_way_to_import_another_module_at_load(tmp_path): f.write_text(src + "\n") assert bool(_other_module_imports(f, mods, tmp_path)) is bad, src helper = tmp_path / "sampling" / "helper.py" - helper.write_text("from .b import x\n") - assert _other_module_imports(helper, mods, tmp_path), "a shared helper may not import a module" + for src in ("from .b import x", "from ..loaders.c import x", "from ..loaders import c"): + helper.write_text(src + "\n") + assert _other_module_imports(helper, mods, tmp_path), f"a shared helper may not import a module: {src}" diff --git a/core/tests/test_registry.py b/core/tests/test_registry.py index 985cbe79..175b45c1 100644 --- a/core/tests/test_registry.py +++ b/core/tests/test_registry.py @@ -246,3 +246,16 @@ def test_a_scan_under_a_redirected_modules_dir_does_not_outlive_the_test(monkeyp assert redirected() == {}, "the redirect did not take effect" monkeypatch.undo() assert registry_mod.current().specs, "the empty scan outlived its test" + + +def test_a_provider_that_raises_is_skipped_and_said_once(): + """One broken provider must not take the question down, and must not stay silent or repeat on every call.""" + from core import log + from core.schema import validate + log._reset() + reg = registry_mod.Registry() + reg.add(validate({"id": "bad", "title": "B", "mount": "m", "provides": {"cap": lambda: 1 / 0}}, source="a")) + reg.add(validate({"id": "good", "title": "G", "mount": "m", "provides": {"cap": lambda: "ok"}}, source="b")) + assert reg.ask("cap") == "ok" and reg.ask("cap") == "ok" + said = [r for r in log.history() if "failed while answering" in r["message"]] + assert len(said) == 1 and "ZeroDivisionError" in said[0]["message"] diff --git a/core/tests/test_retired_modules.py b/core/tests/test_retired_modules.py index 45a67900..e2025977 100644 --- a/core/tests/test_retired_modules.py +++ b/core/tests/test_retired_modules.py @@ -8,3 +8,11 @@ def test_values_saved_for_a_module_that_no_longer_exists_are_ignored_not_refused saved = json.dumps({gone: {"enabled": True}}) assert N._problems(saved) == [] assert N.execute(saved).args[1] == "settings for nothing set" + + +def test_a_name_with_no_module_is_said_in_the_log(comfyui): + from core import log + from modules.sampling.modifiers.nodes import FunPackModifierSettings as N + log._reset() + N.execute(json.dumps({"gone_module": {"x": 1}})) + assert any("'gone_module'" in r["message"] and "ignored" in r["message"] for r in log.history())