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/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/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: 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" 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"