From 709823e49193bbce174ec496e91f732d190dfc1a Mon Sep 17 00:00:00 2001 From: Mohammed Alkindi Date: Tue, 1 Sep 2026 17:53:11 +0400 Subject: [PATCH 1/3] fix(build_context): strip leading BOMs before manifest frontmatter detection decode_text() decodes with plain utf-8, which never strips a byte-order mark, so a BOM-prefixed SKILL.md left content[0] == U+FEFF and the content.startswith("---") sniff in _parse_manifest returned an empty manifest. Every manifest-gated analyzer then early-exits as not_applicable/manifest_absent, so LP1-LP4 and TP1-TP4 stop contributing findings for that skill. Strip leading BOMs in a loop, scoped to delimiter detection. decode_text() itself is left alone because P2/TP1/P9 treat U+FEFF in file bodies as a hidden-character injection signal. Signed-off-by: Mohammed Alkindi --- src/skillspector/nodes/build_context.py | 9 +++++ tests/nodes/test_build_context.py | 49 +++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/src/skillspector/nodes/build_context.py b/src/skillspector/nodes/build_context.py index fa8ae93c..863c3e1f 100644 --- a/src/skillspector/nodes/build_context.py +++ b/src/skillspector/nodes/build_context.py @@ -1575,6 +1575,15 @@ def _check_runtime() -> None: runtime_limit=runtime_limit, ) return {} + while content.startswith("\ufeff"): + # decode_text() decodes with plain "utf-8", which never strips a + # leading byte-order mark (only "utf-8-sig" does), so a BOM-prefixed + # SKILL.md leaves content[0] == "\ufeff" and the delimiter check below + # silently sees {} instead of the frontmatter. Strip leading BOMs here, + # scoped to delimiter detection — decode_text() itself stays untouched + # because P2/TP1/P9 treat U+FEFF as a hidden-character injection signal + # in file bodies. + content = content[1:] if not content.startswith("---"): return {} end_match = re.search(r"\n---\s*\n", content[3:]) diff --git a/tests/nodes/test_build_context.py b/tests/nodes/test_build_context.py index f841d2bd..eb4fb7c9 100644 --- a/tests/nodes/test_build_context.py +++ b/tests/nodes/test_build_context.py @@ -744,6 +744,55 @@ def test_build_context_skill_md_lowercase(tmp_path: Path) -> None: assert "references/guide.md" in result["components"] +def test_build_context_parses_manifest_with_utf8_bom(tmp_path: Path) -> None: + """A leading UTF-8 BOM before the frontmatter delimiter must not blank the manifest. + + Regression guard: ``decode_text()`` decodes with plain "utf-8", which does not + strip a leading byte-order mark (only "utf-8-sig" does), so + ``content[0] == "\\ufeff"`` and the naive ``content.startswith("---")`` check in + ``_parse_manifest`` used to fail silently, returning ``{}`` with no ledger record. + """ + (tmp_path / "SKILL.md").write_bytes( + b"\xef\xbb\xbf---\nname: bom-skill\ndescription: has a leading BOM\n---\n# Skill\n" + ) + state: SkillspectorState = {"skill_path": str(tmp_path)} + result = build_context(state) + + assert result["manifest"]["name"] == "bom-skill" + assert result["manifest"]["description"] == "has a leading BOM" + + +def test_build_context_parses_manifest_without_bom_identically(tmp_path: Path) -> None: + """Regression guard: the BOM-free sibling of the fixture above parses identically.""" + (tmp_path / "SKILL.md").write_bytes( + b"---\nname: bom-skill\ndescription: has a leading BOM\n---\n# Skill\n" + ) + state: SkillspectorState = {"skill_path": str(tmp_path)} + result = build_context(state) + + assert result["manifest"]["name"] == "bom-skill" + assert result["manifest"]["description"] == "has a leading BOM" + + +def test_build_context_parses_manifest_with_stacked_utf8_bom(tmp_path: Path) -> None: + """A doubled leading BOM must not reproduce the original silent ``{}`` manifest. + + Regression guard: the single-strip fix (``if content.startswith("\\ufeff")``) + only removes one BOM, so ``content[0] == "\\ufeff"`` again after the strip and + ``content.startswith("---")`` still fails on a stacked BOM. ``_parse_manifest`` + strips leading BOMs in a loop so repeated markers are all removed. + """ + (tmp_path / "SKILL.md").write_bytes( + b"\xef\xbb\xbf\xef\xbb\xbf---\nname: stacked-bom-skill\ndescription: doubled BOM\n" + b"---\n# Skill\n" + ) + state: SkillspectorState = {"skill_path": str(tmp_path)} + result = build_context(state) + + assert result["manifest"]["name"] == "stacked-bom-skill" + assert result["manifest"]["description"] == "doubled BOM" + + def test_build_context_parses_manifest_from_cached_snapshot_after_file_disappears( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: From a285218f6d6f045aec3796dbd00eeb15fa5f7eb1 Mon Sep 17 00:00:00 2001 From: Mohammed Alkindi Date: Tue, 1 Sep 2026 17:53:23 +0400 Subject: [PATCH 2/3] fix(multi_skill): strip leading BOM before raw-byte frontmatter sniff _extract_skill_name checks prefix.startswith(b"---") on raw bytes before decoding, so a BOM-prefixed sub-skill fell through to the fallback and was listed under its directory name instead of the name: declared in its frontmatter. Byte-level counterpart to the same strip in build_context._parse_manifest. Signed-off-by: Mohammed Alkindi --- src/skillspector/multi_skill.py | 5 +++++ tests/test_multi_skill.py | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/skillspector/multi_skill.py b/src/skillspector/multi_skill.py index fea6d869..ecf50c33 100644 --- a/src/skillspector/multi_skill.py +++ b/src/skillspector/multi_skill.py @@ -520,6 +520,11 @@ def _extract_skill_name(skill_dir: Path, *, budget: _DetectionBudget) -> str: raise _read_error("multi_skill_manifest_content") from exc prefix = observed[:MAX_MULTI_SKILL_MANIFEST_FRONTMATTER_BYTES] + while prefix.startswith(b"\xef\xbb\xbf"): + # Byte-level counterpart to the BOM strip in build_context._parse_manifest: + # this sniff runs on raw bytes before decode, so a BOM-prefixed SKILL.md + # would otherwise fall through to `fallback` (the directory name) below. + prefix = prefix[3:] if not prefix.startswith(b"---"): return fallback content = prefix.decode("utf-8", errors="replace") diff --git a/tests/test_multi_skill.py b/tests/test_multi_skill.py index e13dfc61..bd75b7e2 100644 --- a/tests/test_multi_skill.py +++ b/tests/test_multi_skill.py @@ -128,6 +128,32 @@ def test_skill_names_extracted_from_frontmatter(self, multi_skill_dir: Path) -> names = {s.name for s in result.skills} assert names == {"weather-lookup", "email-sender", "file-manager"} + def test_skill_name_extracted_from_bom_prefixed_frontmatter(self, tmp_path: Path) -> None: + """A BOM-prefixed sub-skill resolves to its declared `name:`, not the directory name. + + Regression guard: `_extract_skill_name` sniffs raw bytes for a leading + `b"---"` before decoding. A UTF-8 BOM (`b"\\xef\\xbb\\xbf"`) in front of that + delimiter used to defeat the sniff and silently fall back to `skill_dir.name` + instead of the frontmatter's declared name. + """ + clean_dir = tmp_path / "clean-skill" + clean_dir.mkdir() + (clean_dir / "SKILL.md").write_bytes( + b"---\nname: clean-declared-name\ndescription: no BOM\n---\n# Clean\n" + ) + + bom_dir = tmp_path / "bom-dir-name" + bom_dir.mkdir() + (bom_dir / "SKILL.md").write_bytes( + b"\xef\xbb\xbf---\nname: bom-declared-name\ndescription: has a BOM\n---\n# BOM\n" + ) + + result = detect_skills(tmp_path) + + names = {s.relative_path: s.name for s in result.skills} + assert names["clean-skill"] == "clean-declared-name" + assert names["bom-dir-name"] == "bom-declared-name" + def test_structured_skill_subdir_detected(self, tmp_path: Path) -> None: """An immediate subdirectory with a valid AISOP/AISP bundle is detected.""" sub = tmp_path / "workflow-bundle" From db3068a433b2f1792766a70fd7967d472cb2a6d8 Mon Sep 17 00:00:00 2001 From: Mohammed Alkindi Date: Wed, 2 Sep 2026 14:17:37 +0400 Subject: [PATCH 3/3] fix(excessive_agency): match leading BOM in SKILL.md frontmatter bounds _frontmatter_bounds anchored on \A---, so a BOM-prefixed SKILL.md returned None and EA5 silently reported no frontmatter model pin. This is the third frontmatter sniff in the same class as the two already fixed on this branch. The BOM is absorbed into the opening match rather than stripped, because the offsets this function returns index the caller's original content: _ea5_findings slices it and derives line numbers and context from it, so removing characters would shift every reported position by one. --- .../analyzers/static_patterns_excessive_agency.py | 5 ++++- tests/unit/test_patterns_new.py | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/skillspector/nodes/analyzers/static_patterns_excessive_agency.py b/src/skillspector/nodes/analyzers/static_patterns_excessive_agency.py index f5a89e2f..6567d294 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_excessive_agency.py +++ b/src/skillspector/nodes/analyzers/static_patterns_excessive_agency.py @@ -186,7 +186,10 @@ def _frontmatter_bounds(content: str, file_path: str) -> tuple[int, int] | None: """Return the YAML-frontmatter byte offsets for a SKILL.md file.""" if file_path.rsplit("/", 1)[-1].lower() != "skill.md": return None - opening = re.match(r"\A---[ \t]*\r?\n", content) + # A leading BOM is matched here rather than stripped: the offsets this + # function returns index the caller's original `content` (line numbers and + # context are derived from it), so removing characters would shift them. + opening = re.match(r"\A\ufeff*---[ \t]*\r?\n", content) if opening is None: return None closing = re.search(r"^---[ \t]*$", content[opening.end() :], re.MULTILINE) diff --git a/tests/unit/test_patterns_new.py b/tests/unit/test_patterns_new.py index 204c5ec4..7949f5d4 100644 --- a/tests/unit/test_patterns_new.py +++ b/tests/unit/test_patterns_new.py @@ -260,6 +260,21 @@ def test_ea5_frontmatter_pin_is_medium(self, key: str) -> None: "selection_key": key, } + def test_ea5_frontmatter_pin_detected_through_leading_bom(self) -> None: + """A BOM-prefixed SKILL.md must not hide the frontmatter model pin.""" + content = "\ufeff---\nname: example\nmodel: claude-sonnet-4-6\n---\n\n# Example\n" + findings = ea_mod.analyze(content, "SKILL.md", "markdown") + ea5 = [finding for finding in findings if finding.rule_id == "EA5"] + assert len(ea5) == 1 + assert ea5[0].severity == Severity.MEDIUM + # The BOM is matched, not stripped, so offsets still index the original + # content and the reported line matches the no-BOM case exactly. + assert ea5[0].location.start_line == 3 + assert ea5[0].evidence == { + "selection_surface": "frontmatter", + "selection_key": "model", + } + def test_ea5_only_matches_top_level_skill_frontmatter(self) -> None: content = ( "---\n"