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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5.5
parent
6e5035671a
commit
ef35a35848
+1
-1
@@ -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:
|
||||
|
||||
@@ -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}"
|
||||
|
||||
@@ -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"]
|
||||
|
||||
@@ -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())
|
||||
|
||||
Reference in New Issue
Block a user