fix(analyzers): drop P2 BOM, RA2 plist-substring and EA3 OFL false positives

[jwcastillo]

Problem

Scanning public third-party skills with --no-llm produced false positives from three narrow pattern bugs (issue #644, items A1 to A3):

• P2 Hidden Instructions fires on a UTF-8 byte-order mark (U+FEFF at offset 0), for example in ECMA-376 .xsd files that start with EF BB BF <?xml.
• RA2 Session Persistence matches plist anywhere, case-insensitively, including inside identifiers such as relationshipList (JS) and CT_GradientStopList (XSD). One schema set produced 11 hits and one HTML template produced 30.
• EA3 Scope Creep fires on the SIL Open Font License disclaimer ("INCLUDING BUT NOT LIMITED TO ANY WARRANTIES OF") in OFL.txt / <Font>-OFL.txt. The fix(analyzer): filter license boilerplate from EA3 static findings (#312) #328 filter only covers Apache/MIT/BSD ranges and LICENSE/COPYING/NOTICE names.

Reproduction

A synthetic skill with a BOM-prefixed XSD, a JS file containing relationshipList, and a SomeFont-OFL.txt disclaimer, scanned with skillspector scan <dir> --no-llm --format json:

	score	recommendation	issues
main @ 89e9087	32	CAUTION	P2 (xsd:1), RA2 (app.js:1), RA2 (xsd:3), EA3 (OFL:3)
this branch	0	SAFE	none

Fix

• P2: skip a U+FEFF match only when it is at offset 0 of the file. Large files are scanned in overlapping windows, so a U+FEFF at the start of a later window is still covered by the previous window.
• RA2: (?<![a-z])plist. .plist paths, PlistBuddy and write_plist(...) still match. The known limit is that camelCase savePlist no longer matches, but defaults write, launchctl load and the .plist file path of a LaunchAgent do.
• EA3: add the OFL disclaimer as a canonical range and accept ofl, optionally prefixed (<Font>-OFL.txt), as a license basename. As with the existing ranges, only the exact canonical lines are suppressed.

Tests

Each new FP test fails on main and passes on this branch (verified by reverting src/ only):

• test_p2_leading_byte_order_mark_no_false_positive[SKILL.md|schemas/types.xsd]
• TestRogueAgent::test_ra2_plist_substring_inside_identifier_not_detected[camel_case_identifier|xsd_type_name]
• TestLicenseFiles::test_ofl_font_license_disclaimer_suppresses_ea3[OFL.txt|assets/SomeFont-OFL.txt]
• TestLicenseFiles::test_helper_boundaries[OFL.txt-True|fonts/SomeFont-OFL.txt-True]

True-positive guards:

• test_p2_zero_width_after_byte_order_mark_still_produces_finding: covers a mid-file U+FEFF, a BOM followed by U+200B on a later line, and a BOM followed by U+200B on the same line.
• test_ra2_detected[launch_agent_plist|snake_case_plist]
• test_ofl_named_file_with_non_boilerplate_content_reports_ea3, and test_helper_boundaries[profl.txt-False|ofl.py-False].
• The existing parametrized test_each_canonical_range_suppresses_only_ea3 and test_attacker_line_after_canonical_range_reports_ea3 now also cover the new range.

Local results: make lint and make format-check are clean. make test-unit gave 7926 passed, 14 skipped, 4 xfailed. make test-integration (no provider keys) gave 127 passed, 4 skipped.

Out of scope

Everything else is in #644 and is not changed here:

• P2 HTML-comment spanning is already handled by Fix P2 HTML comment boundary matching #452 / P2 "Hidden Instructions" rule produces false positives due to missing word boundary on GET #297. Static-pattern false positives found scanning public skills (P2 BOM, RA2 plist, EA3 OFL, plus design-level items) #644 notes that the [//]: # pattern has the same DOTALL spanning problem.
• The remaining items are policy questions: AE1 severity for static_parse_limit, RP1 in tests and docs, E2 on env pass-through (E2 fires HIGH on subprocess env pass-through (env={**os.environ, …}) with the same severity and confidence as a real harvester — precision cost of the #329 fix #441 / fix(e2): exempt child-process env pass-through from harvesting #492), TM1/TM2/TM3, PE5/TM4 context, and a suggested rule for predictable temp paths.

Refs #644.

https://claude.ai/code/session_01HgEFtvq6aWpN5jC1MRCpEB

[rng1995]

[SkillSpector Review]

Hi @jwcastillo, thank you for your contribution to SkillSpector — we really appreciate it!

This PR fixes three narrow false-positive sources from #644:

• P2: a U+FEFF at offset 0 of the scanned text is skipped in the zero-width branch of _p2_pattern_matches.
• RA2: plist must not follow a letter.
• EA3: the SIL OFL disclaimer is added as a canonical license range, and OFL / <Font>-OFL basenames are recognized.

For each fix I checked for the false negatives it could introduce, read the diff against the surrounding code and current main, and evaluated the changed regexes on sample inputs.

What I verified

• P2 / BOM. The skip applies only to a zero-width candidate that starts at offset 0 and is U+FEFF, and scanning then continues on the same line. These still report P2, and the new tests cover all three:

  • a BOM followed by U+200B, on the same line or a later one
  • a second U+FEFF
  • any mid-file U+FEFF

  Offset 0 of a later raw window is at least 8 KiB into the previous window, which owns that offset, so windowing cannot hide a mid-file U+FEFF. HTML-comment, bidi, tag-block and data-URI detection are unchanged. The change is consistent with #471 on main (a leading BOM is stripped for frontmatter detection) and composes with #473 (the bidi loop does not use the zero-width branch).

• RA2. Under re.IGNORECASE, (?<![a-z]) excludes any ASCII letter. These still match: .plist paths including ~/Library/LaunchAgents/*.plist, PlistBuddy, plistlib, write_plist(...), defaults write, and launchctl load. Only camelCase identifiers lose the match, which the PR acknowledges. I found no other LaunchAgent/plist persistence signal in the static rules or YARA, so this alternative carries that coverage alone (see Finding 1).

• EA3. _is_license_basename only gates suppression of EA3 on exact canonical lines. Broadening the basenames to OFL* / *-OFL* therefore cannot hide non-boilerplate text, and a test pins an attacker line in an OFL-named file. The canonical lines match the OFL 1.1 disclaimer after casefold and whitespace normalization, and the existing range-parametrized tests pick up the new range automatically.

Findings

1. [Non-blocking, optional] src/skillspector/nodes/analyzers/static_patterns_rogue_agent.py:174: you can recover camelCase coverage without re-admitting the …pList identifiers by adding a case-sensitive alternative, (?-i:[a-z]Plist). On sample strings it matches savePlist, agentPlist and writeLaunchAgentPlist, but not relationshipList, CT_GradientStopList, IPList or shopList. If you add it, include positives for those names.
2. [Non-blocking, optional] The window-overlap argument in the description is correct but not pinned by a test. A regression would guard the invariant against future windowing changes: put U+FEFF exactly at the second raw-window start (static_runner._RAW_WINDOW_OWNED_CHARS - static_runner._WINDOW_OVERLAP_CHARS, i.e. offset 231,424) in a markdown file longer than 256K characters and assert P2.
3. [Note] This PR overlaps with #622 in static_patterns_prompt_injection.py, but the code paths are disjoint (zero-width iteration here, comment carve-out there). Applying #622 and then this PR onto current main merges cleanly, and there is no semantic interaction. The [//]: # spanning issue noted in #644 (B1) is still open and out of scope here.

Tests/CI
All checks pass at this head, and GitHub reports it as mergeable with current main. Each new false-positive test targets behavior that fails on main, and the true-positive guards (a mid-file or later U+FEFF, LaunchAgent .plist and write_plist, and an OFL-named file with non-boilerplate text) are meaningful. No docs or CHANGELOG change is needed; the CHANGELOG is updated at release time.

---

Decision: Approved (reviewed head 0ac5d2a6215b9d9dc01d7dea165cf104f689acc5)