From 81a8dcda54ece683f84f25e4183231646341b8ba Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Sun, 2 Aug 2026 10:45:11 -0400 Subject: [PATCH] Address CodeRabbit review: font-name traversal + offline test guard Two Minor findings from CodeRabbit's first review of this PR. - resolve_font_path: reject relative font names carrying path components. font_name comes from plugin config, which the web UI writes; a value like "../../config/config.json" escaped assets/fonts/ after os.path.join and let a config probe arbitrary paths for existence (disclosure unlikely, since Pillow/freetype reject non-font files, but the probe is real). Relative names must now be bare filenames (os.path.basename(name) == name); absolute paths keep their existing isfile() gate. Test confirms the traversal resolved the real config.json before the guard. - build_manager fixture: patch requests.Session.get BEFORE constructing the manager. Construction creates both SportsCore.session and the ESPNDataSource.session; the old code only replaced manager.session after the fact, leaving data_source.session real and able to reach the network on an accidental fetch. Patching the class makes every session built in the fixture offline. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --- src/element_style.py | 7 +++++++ test/test_element_style.py | 16 ++++++++++++++++ test/test_sports_base_characterization.py | 11 +++++++++++ 3 files changed, 34 insertions(+) diff --git a/src/element_style.py b/src/element_style.py index 76bb0511..1d9a08ea 100644 --- a/src/element_style.py +++ b/src/element_style.py @@ -106,6 +106,13 @@ def resolve_font_path(font_name: str) -> Optional[str]: return None if os.path.isabs(font_name): return font_name if os.path.isfile(font_name) else None + # A relative name must be a bare filename. font_name comes from plugin + # config, which the web UI writes; a value like "../../config/config.json" + # would otherwise escape assets/fonts/ once joined and let a config probe + # arbitrary paths for existence. os.path.basename collapses any such value + # to its last component, so a name that isn't already bare is rejected. + if os.path.basename(font_name) != font_name: + return None candidates = ( os.path.join(os.getcwd(), _FONTS_SUBDIR, font_name), os.path.join(_CORE_ROOT, _FONTS_SUBDIR, font_name), diff --git a/test/test_element_style.py b/test/test_element_style.py index 6270e06f..8d0535ab 100644 --- a/test/test_element_style.py +++ b/test/test_element_style.py @@ -377,6 +377,22 @@ class TestResolverPlumbing: font = load_font("5x7.bdf", 7) assert isinstance(font, freetype.Face) + @pytest.mark.parametrize("hostile", [ + "../../config/config.json", + "../secrets.txt", + "sub/dir/font.ttf", + "..", + ]) + def test_relative_font_name_with_path_components_is_rejected(self, hostile): + # font_name comes from plugin config (web-UI writable); a relative name + # carrying path separators would escape assets/fonts/ after os.path.join + # and let a config probe arbitrary paths. Only bare filenames resolve. + assert resolve_font_path(hostile) is None + + def test_bare_filename_still_resolves(self): + # The guard must not reject legitimate bare names. + assert resolve_font_path("PressStart2P-Regular.ttf") is not None + def test_schema_manager_expands_on_load(self, tmp_path): # The web-UI form path: SchemaManager.load_schema serves the # expanded schema so the style blocks actually appear in the UI. diff --git a/test/test_sports_base_characterization.py b/test/test_sports_base_characterization.py index ff51ddb7..55411f8e 100644 --- a/test/test_sports_base_characterization.py +++ b/test/test_sports_base_characterization.py @@ -403,6 +403,17 @@ def build_manager(monkeypatch, tmp_path): "src.base_classes.sports.get_background_service", lambda *args, **kwargs: MagicMock()) + # Rig requests.Session BEFORE any manager is built. Construction creates + # both SportsCore.session and the ESPNDataSource.session; replacing only + # manager.session after the fact (below) leaves data_source.session real, + # so an accidental fetch during or right after construction could reach + # the network. Patching the class makes every session created here raise. + def _offline_get(*args, **kwargs): + raise requests.exceptions.ConnectionError( + "characterization tests are offline") + + monkeypatch.setattr(requests.Session, "get", _offline_get) + def build(cls, schedule, **mode_cfg): config = { "timezone": "UTC",