Add --exclude glob flag to skill scan

[serval-frenchie]

Summary

• Adds a repeatable --exclude PATTERN option to skillspector scan
• Patterns are applied during file discovery in build_context, so excluded files are absent from both the Components table and any analyzer findings
• Uses fnmatch semantics against the path relative to the scan root

Problem

SkillSpector's static analyzers read every file with utf-8 + errors='replace' and run regex patterns against the result. When a skill bundle ships binary assets (PDFs, images), raw bytes can incidentally match patterns like shell=True, -rf /, or --no-verify, producing HIGH-severity false positives. There is currently no way to suppress these short of moving the file out of the skill folder, which is a non-starter for adoption in CI: every push surfaces a HIGH finding that has to be dismissed manually in the GitHub Security tab.

Solution

skillspector scan ./my-skill/ --exclude '*.pdf' --exclude 'assets/*'

• Repeatable flag (List[str] via Typer), threaded into graph state as exclude_patterns
• Filtering happens once, in _walk_skill_files, before components, file_cache, and component_metadata are built — so the rest of the pipeline is unchanged
• Each exclusion is logged at DEBUG (visible with --verbose)
• Excluding everything is valid: the scan succeeds with no findings

Tests

• tests/nodes/test_build_context.py: exclusion filters components/file_cache/metadata, directory glob patterns, no-match leaves the list intact, exclude-everything yields empty state
• tests/unit/test_cli.py: end-to-end CLI invocation with a fixture skill containing a PDF whose bytes match TM1 (Tool Parameter Abuse) — asserts the PDF is absent from components and produces no issues; also covers the repeatable form and the exclude-everything case
• Existing suite passes (make test-unit)

Test plan

• make test-unit is green
• make lint and make format-check are clean
• Manual: scan a skill containing a binary asset that previously triggered TM1; confirm the finding disappears with --exclude '*.pdf' and reappears without it

[keshprad]

This may need more discussion. Imagine if a someone uses --exclude '*'. This will silently skip all files (including SKILL.md), but the skillspector report indicates the skill as SAFE

[rng1995]

Review: COMMENT — solid, well-tested change, but it needs a rebase before it can land.

Thanks for this. The feature is well-scoped and the approach is sound: filtering once in _walk_skill_files, before components, file_cache, and component_metadata are built, means every downstream analyzer inherits the exclusion for free. I confirmed that the static pattern runner and the YARA node both iterate state["components"] and read from state["file_cache"] (and no analyzer re-walks the skill directory), so excluded files genuinely produce no findings — the "absent from the Components table and any analyzer findings" claim checks out. Docs and tests are thorough (directory glob, no-match, exclude-everything, plus an end-to-end CLI test against a binary fixture).

Blocking (rebase): this branch currently conflicts with main. It is not mergeable as-is — the conflict is in src/skillspector/nodes/build_context.py, against the recently merged change that switched path collection to rel.as_posix() (#86 / #87). When resolving, please make the final append use the posix form rather than str(rel):

# after the exclude check
paths.append(rel_str)   # rel_str = rel.as_posix(), already computed above

Keeping paths.append(str(rel)) would re-introduce OS-specific backslashes in component paths on Windows and undo that portability fix (these strings are used as dict keys and as SARIF/URI locations). Since you already compute rel_str = rel.as_posix() for matching, reusing it keeps everything consistent.

Minor / optional (non-blocking):

• fnmatch.fnmatch applies OS-dependent case normalization (case-sensitive on Linux, case-insensitive on Windows), so --exclude '*.pdf' would not match REPORT.PDF on Linux but would on Windows. Given the repo's cross-platform care, consider fnmatch.fnmatchcase for deterministic behavior (and document the casing), or keep fnmatch intentionally.
• "Exclude everything" exits 0 with no findings. That is a reasonable, tested choice, but for a security scanner an over-broad pattern could mask a real result; a single INFO/DEBUG note when an exclusion removes all files might prevent a confusing "clean" outcome. Optional.

Nice, focused PR overall — once it is rebased with the posix-path resolution above, this looks good to me.

[rng1995]

[Automated SkillSpector Review]

Summary: Adds a repeatable --exclude PATTERN flag to skillspector scan, filtering files during discovery in _walk_skill_files (fnmatch against scan-root-relative paths) so excluded files are absent from components, file_cache, component_metadata, and all analyzer findings. Includes node-level and end-to-end CLI tests plus README docs. The core approach (filter once at discovery) is sound and the tests are generally thorough, but there are blocking issues.

Blockers

1. Branch conflicts with main (not mergeable). The conflict is in src/skillspector/nodes/build_context.py: main now appends rel.as_posix() (#86/#87, commits bc6c542/54d9074) and _resolve_skill_dir now raises ValueError instead of returning None (a8d284f), so the hunk anchored on return _minimal_update() no longer applies. A rebase is required.
2. Windows path regression baked into the resolution. The new code computes rel_str = rel.as_posix() for matching but then does paths.append(str(rel)), which re-introduces OS-specific backslashes in component paths on Windows — undoing the #87 fix (these strings are dict keys and SARIF URI locations). Use paths.append(rel_str).
3. Exclusions are invisible in every report format — silent detection gap. Excluded files leave no trace in JSON/SARIF/markdown output (only a DEBUG log). This repo's existing false-positive mechanism (src/skillspector/suppression.py baseline) deliberately records every suppression in the report (suppressed, suppressed_count, required reason); --exclude bypasses that convention. A CI job running --exclude '*' (or --exclude 'SKILL.md') exits 0 with a SAFE verdict and zero audit trail — and the PR's tests enshrine this as valid. Maintainer @keshprad raised exactly this on the thread. Please: (a) record applied exclude patterns and excluded-file count in report metadata across formats, and (b) emit a visible warning (or error) when exclusions remove all files or SKILL.md.
4. Post-rebase: --exclude will silently not apply to multi-skill scans. On current main, scan routes multi-skill directories through _scan_multi_skill(), which calls _scan_state(str(skill.path), format, no_llm, yara_rules_dir=yara_dir) without exclude patterns (src/skillspector/cli.py:375). When rebasing, either thread exclude_patterns through (deciding pattern semantics relative to each skill root vs. the scan root) or explicitly reject/warn when --exclude is combined with a multi-skill input.

Non-blocking

• fnmatch.fnmatch applies os.path.normcase, so matching is case-insensitive on Windows but case-sensitive on Linux/macOS (--exclude '*.pdf' misses REPORT.PDF on Linux only). Prefer fnmatch.fnmatchcase for deterministic cross-platform behavior and document the casing rule.
• Excluding SKILL.md does not exclude the manifest: _parse_manifest(skill_dir) reads SKILL.md directly regardless of components, so manifest-derived findings can still fire against an "excluded" file. Either document this or make the behavior consistent.
• tests/unit/test_cli.py::test_cli_scan_exclude_drops_pdf_from_components_and_findings never asserts that the PDF does produce a TM1 finding without --exclude, so it passes vacuously if the fixture stops triggering the pattern. Add a control scan without the flag asserting the finding is present.
• README: consider cross-referencing the baseline suppression mechanism as the audit-friendly alternative for finding-level false positives, so users pick the right tool.

[rng1995]

This should be paths.append(rel_str). str(rel) produces backslash-separated paths on Windows, regressing the as_posix() portability fix merged in #87 (these strings are used as file_cache keys and SARIF URI locations). You already compute rel_str = rel.as_posix() above — reuse it. Note this function is also the site of the merge conflict with main, so this needs to be resolved as part of the rebase.

[rng1995]

fnmatch.fnmatch applies os.path.normcase, making matching case-insensitive on Windows but case-sensitive on POSIX — --exclude '*.pdf' will not match REPORT.PDF on Linux but will on Windows. Consider fnmatch.fnmatchcase for deterministic cross-platform behavior, and document the casing rule in the README either way.

[rng1995]

Exclusions are only visible at DEBUG level and leave no trace in JSON/SARIF/markdown output. The repo's suppression baseline records every suppressed finding in the report (suppressed, suppressed_count); --exclude should similarly surface applied patterns and excluded-file count in report metadata, and warn when exclusions remove all files or SKILL.md — otherwise --exclude '*' in CI yields a silent SAFE verdict with no audit trail.

[rng1995]

This test asserts the PDF produces no findings with --exclude, but never establishes that it produces a TM1 finding without the flag — if the fixture bytes stop triggering the pattern, the test passes vacuously. Add a control invocation without --exclude asserting the TM1 finding is present against assets/template-style.pdf.