diff --git a/CHANGELOG.md b/CHANGELOG.md index 791838e..663d116 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ All notable changes to SkillEvaluator are documented in this file. ### Fixed +- Schema, frontmatter, quality parsing, and security PII scanning accept a leading + UTF-8 BOM, matching the unicode scanner's "benign BOM" note + ([#91](https://github.com/NVIDIA/SkillEvaluator/issues/91)). - Windows personal-path PII now flags `C:\Users\...` usernames that start with `s` (for example `steve`), matching the intended whitespace class rather than excluding the letter `s` ([#87](https://github.com/NVIDIA/SkillEvaluator/issues/87)). diff --git a/src/skillevaluator/validators/frontmatter_parser.py b/src/skillevaluator/validators/frontmatter_parser.py index 9c8f1aa..a1aeac1 100644 --- a/src/skillevaluator/validators/frontmatter_parser.py +++ b/src/skillevaluator/validators/frontmatter_parser.py @@ -18,7 +18,7 @@ # Regex pattern for extracting frontmatter between --- markers FRONTMATTER_PATTERN = re.compile( - r"^---[^\S\r\n]*\r?\n(.*?)\r?\n---[^\S\r\n]*(?=\r?\n|\Z)(?:\r?\n)?(.*)", + r"^\ufeff?---[^\S\r\n]*\r?\n(.*?)\r?\n---[^\S\r\n]*(?=\r?\n|\Z)(?:\r?\n)?(.*)", re.DOTALL, ) @@ -47,7 +47,7 @@ def parse_frontmatter(file_path: Path) -> tuple[ParsedFrontmatter | None, Valida result = ValidationResult() try: - content = file_path.read_text(encoding="utf-8") + content = file_path.read_text(encoding="utf-8-sig") except Exception as e: result.add_error(f"Failed to read {file_path}: {e}") return None, result diff --git a/src/skillevaluator/validators/quality_score.py b/src/skillevaluator/validators/quality_score.py index 93baf2c..d62e37d 100644 --- a/src/skillevaluator/validators/quality_score.py +++ b/src/skillevaluator/validators/quality_score.py @@ -38,6 +38,7 @@ from skillevaluator.models.result import Finding, Severity, ValidationResult from skillevaluator.models.skill import XML_TAG_RE from skillevaluator.validators.base import ValidatorBase +from skillevaluator.validators.frontmatter_parser import FRONTMATTER_PATTERN from skillevaluator.validators.markdown import markdown_link_targets logger = get_logger(__name__) @@ -281,7 +282,7 @@ def _validate_single_skill(self, skill_path: Path) -> ValidationResult: result.metadata["quality_scores"] = qs.to_dict() return result - content = manifest.read_text(encoding="utf-8") + content = manifest.read_text(encoding="utf-8-sig") lines = content.split("\n") frontmatter_data = self._parse_frontmatter(content) @@ -333,7 +334,7 @@ def _validate_single_skill(self, skill_path: Path) -> ValidationResult: @staticmethod def _parse_frontmatter(content: str) -> dict | None: - fm_match = re.match(r"^---\s*\n(.*?)\n---", content, re.DOTALL) + fm_match = FRONTMATTER_PATTERN.match(content) if not fm_match: return None try: @@ -758,7 +759,7 @@ def _check_efficiency( # Token estimates qs.total_tokens = len(content) // 4 - fm_match = re.match(r"^---\s*\n(.*?)\n---", content, re.DOTALL) + fm_match = FRONTMATTER_PATTERN.match(content) if fm_match: qs.frontmatter_tokens = len(fm_match.group(1)) // 4 inst_start = content.find("---", 3) + 3 diff --git a/src/skillevaluator/validators/schema.py b/src/skillevaluator/validators/schema.py index e85e472..26d2bbf 100644 --- a/src/skillevaluator/validators/schema.py +++ b/src/skillevaluator/validators/schema.py @@ -179,7 +179,7 @@ def _validate_frontmatter(self, skill_md: Path) -> ValidationResult: file_path = str(skill_md) try: - content = skill_md.read_text(encoding="utf-8") + content = skill_md.read_text(encoding="utf-8-sig") except Exception as e: result.add_finding( Finding( @@ -439,7 +439,7 @@ def _validate_line_count(self, skill_md: Path) -> ValidationResult: file_path = str(skill_md) try: - line_count = len(skill_md.read_text(encoding="utf-8").splitlines()) + line_count = len(skill_md.read_text(encoding="utf-8-sig").splitlines()) if line_count > MAX_SKILL_MD_LINES: result.add_finding( Finding( @@ -490,7 +490,7 @@ def _validate_body_content(self, skill_md: Path) -> ValidationResult: file_path = str(skill_md) try: - content = skill_md.read_text(encoding="utf-8") + content = skill_md.read_text(encoding="utf-8-sig") except Exception: return result diff --git a/src/skillevaluator/validators/security.py b/src/skillevaluator/validators/security.py index 62f01a3..e4cdae6 100644 --- a/src/skillevaluator/validators/security.py +++ b/src/skillevaluator/validators/security.py @@ -1590,6 +1590,11 @@ def _passes_luhn(digits: str) -> bool: total += n return total % 10 == 0 + @staticmethod + def _is_frontmatter_delimiter(line: str) -> bool: + """True when a line is a YAML frontmatter fence, including BOM-prefixed openers.""" + return line.strip().removeprefix("\ufeff").strip() == "---" + def _scan_file_for_pii(self, file_path: Path, protected_usernames: set[str] | None = None) -> list[dict]: """Scan a single file for PII patterns, yielding findings with full context. @@ -1601,7 +1606,7 @@ def _scan_file_for_pii(self, file_path: Path, protected_usernames: set[str] | No protected_usernames = self._protected_home_usernames(file_path.parent) try: - content = file_path.read_text(encoding="utf-8", errors="ignore") + content = file_path.read_text(encoding="utf-8-sig", errors="ignore") except Exception as e: logger.warning(f"Could not read {file_path}: {e}") return [] @@ -1630,10 +1635,12 @@ def _scan_file_for_pii(self, file_path: Path, protected_usernames: set[str] | No def _frontmatter_author_emails(self, file_path: Path, lines: list[str]) -> dict[int, str]: """Map valid frontmatter author lines to the public contributor email.""" - if file_path.name not in SKILL_MANIFEST_VARIANTS or not lines or lines[0].strip() != "---": + if file_path.name not in SKILL_MANIFEST_VARIANTS or not lines or not self._is_frontmatter_delimiter(lines[0]): return {} try: - frontmatter_end = next(index for index, line in enumerate(lines[1:], 1) if line.strip() == "---") + frontmatter_end = next( + index for index, line in enumerate(lines[1:], 1) if self._is_frontmatter_delimiter(line) + ) except StopIteration: return {} diff --git a/tests/validators/test_frontmatter_parser.py b/tests/validators/test_frontmatter_parser.py index 9baf27b..6f44445 100644 --- a/tests/validators/test_frontmatter_parser.py +++ b/tests/validators/test_frontmatter_parser.py @@ -43,6 +43,25 @@ def test_first_body_line_indentation_is_preserved(self, tmp_path: Path, newline: assert result.passed assert parsed.content == " [code](false.md)\n" + def test_utf8_bom_is_accepted(self, tmp_path: Path): + """A valid frontmatter file that starts with a UTF-8 BOM still parses (#91).""" + test_file = tmp_path / "bom.md" + body = """--- +title: Bom +description: A file whose only extra is a leading UTF-8 BOM +--- + +# Content +""" + test_file.write_bytes(b"\xef\xbb\xbf" + body.encode("utf-8")) + + parsed, result = parse_frontmatter(test_file) + + assert parsed is not None + assert result.passed + assert parsed.yaml_data["title"] == "Bom" + assert parsed.content.strip() == "# Content" + def test_missing_frontmatter(self, tmp_path: Path): """Test file without frontmatter markers.""" test_file = tmp_path / "test.mdc" diff --git a/tests/validators/test_quality_score.py b/tests/validators/test_quality_score.py index c6a47d2..d556a4c 100644 --- a/tests/validators/test_quality_score.py +++ b/tests/validators/test_quality_score.py @@ -322,6 +322,33 @@ def test_xml_tags_in_description_remain_quality_error(self, tmp_path): assert any("Description contains XML tags" in finding.message for finding in result.findings) + def test_bom_prefixed_manifest_still_parses_frontmatter(self, tmp_path): + """A UTF-8 BOM must not hide frontmatter from the quality parser.""" + skill_dir = tmp_path / "bom-xml-desc" + skill_dir.mkdir() + body = ( + "---\n" + "name: bom-xml-desc\n" + "description: \"A skill with injected tags\"\n" + "metadata:\n" + " author: Test User \n" + "---\n\n" + "# XML Description\n\n" + "## Instructions\n\n1. Inspect frontmatter quality findings.\n\n" + "## Examples\n\n" + "```text\n" + "Validate the skill.\n" + "```\n" + ) + (skill_dir / "SKILL.md").write_bytes(b"\xef\xbb\xbf" + body.encode("utf-8")) + + result = QualityScoreValidator(min_score=0).validate(skill_dir) + scores = result.metadata["quality_scores"] + + assert scores["metrics"]["has_frontmatter"] is True + assert scores["metrics"]["frontmatter_tokens"] > 0 + assert any("Description contains XML tags" in finding.message for finding in result.findings) + def test_unclosed_xml_tag_in_description_remains_quality_error(self, tmp_path): """Unclosed tag-like descriptions remain covered by XML-tag detection.""" skill_dir = tmp_path / "unclosed-xml-desc" diff --git a/tests/validators/test_schema.py b/tests/validators/test_schema.py index 78db3ad..43474af 100644 --- a/tests/validators/test_schema.py +++ b/tests/validators/test_schema.py @@ -685,6 +685,34 @@ def test_all_required_sections_present_passes(self, tmp_path: Path): assert result.passed, f"Skill with complete body should pass. Errors: {result.errors}" + def test_utf8_bom_skill_md_passes_schema(self, tmp_path: Path): + """A valid SKILL.md that only adds a UTF-8 BOM must still pass schema (#91).""" + skill_dir = tmp_path / "bom-skill" + skill_dir.mkdir() + body = """--- +name: bom-skill +description: Valid skill whose SKILL.md starts with a UTF-8 BOM +metadata: + author: Bom User +--- + +# BOM Skill + +## Instructions + +1. Open the file in an editor that writes a BOM. + +## Examples + +Example usage. +""" + (skill_dir / "SKILL.md").write_bytes(b"\xef\xbb\xbf" + body.encode("utf-8")) + + result = SchemaValidator().validate(skill_dir) + + assert result.passed, f"BOM-only difference should still pass schema. Errors: {result.errors}" + assert all(f.check_name != "frontmatter_format" for f in result.findings) + def test_canonical_support_dirs_accepted(self, tmp_path: Path): """Canonical public skill support directories must not be flagged. diff --git a/tests/validators/test_security.py b/tests/validators/test_security.py index 7e6975f..06af20c 100644 --- a/tests/validators/test_security.py +++ b/tests/validators/test_security.py @@ -612,6 +612,45 @@ def test_valid_public_author_email_is_exempt_only_in_frontmatter(self, tmp_path: assert len(email_findings) == 1 assert email_findings[0].line_content == "Contact contributor@contributors.invalid for private support." + @pytest.mark.parametrize("newline", ["\n", "\r\n"]) + def test_bom_prefixed_frontmatter_author_email_not_flagged_in_security_scan( + self, tmp_path: Path, newline: str + ): + """UTF-8 BOM must not turn a valid frontmatter author email into a PII finding.""" + skill_dir = tmp_path / "bom-public-author-skill" + skill_dir.mkdir() + skill_md = skill_dir / "SKILL.md" + body = newline.join( + [ + "---", + "name: bom-public-author-skill", + "description: A public skill with contributor metadata and a body contact leak", + "metadata:", + " author: Example Contributor ", + "---", + "", + "# Public Author Skill", + "", + "## Instructions", + "", + "Contact contributor@contributors.invalid for private support.", + "", + "## Examples", + "", + "Run the documented workflow.", + "", + ] + ) + skill_md.write_bytes(b"\xef\xbb\xbf" + body.encode("utf-8")) + + schema_result = SchemaValidator().validate(skill_dir) + pii_result = SecurityValidator(submitter_usernames=[]).validate_pii_only(skill_dir) + + assert not [finding for finding in schema_result.findings if finding.check_name == "author_format"] + email_findings = [finding for finding in pii_result.findings if finding.check_name == "emails"] + assert len(email_findings) == 1 + assert email_findings[0].line_content == "Contact contributor@contributors.invalid for private support." + def test_unrelated_home_roots_not_flagged(self, tmp_path: Path): """Unrelated /home roots stay unflagged without an organization allowlist.""" skill_dir = tmp_path / "shared-home-skill"