Skip to content

No way to scope a scan, so a skill that ships test fixtures cannot pass #335

Description

@shi-eric

A skill that ships buildable test fixtures cannot pass a scan, and there is no supported way to tell the scanner which files are fixtures rather than skill code.

docs/EVAL_DATASETS.md opens by stating the principle: "SkillSpector treats authored eval datasets as test-case data, not installed skill logic." That principle is applied to eight dataset manifest filenames and to nothing those manifests reference.

The skill in question carries eleven small Python projects under evals/files/, staged as fixtures by its eval dataset. They are scanned as skill code, and they decide the verdict.

What was observed

Configuration Findings Score Verdict
Scan as authored 23, of which 22 are under evals/files/ 100/100 CRITICAL / DO NOT INSTALL
Every fixture dependency pinned 21 100/100 CRITICAL / DO NOT INSTALL
Fixture pyproject.toml files deleted 2 42/100 MEDIUM / CAUTION

Pinning changes almost nothing because the advisory lookup keys on package name and ignores version constraints, so a pinned floor still returns that package's worst recorded advisory. That behavior is already tracked in #318.

The only configuration that passed was the third one. Deleting packaging metadata from test fixtures is not a security improvement, and it leaves the fixtures harder to build and run.

For context on why this has probably not come up before: of the 324 skills in the public catalog, none ships buildable projects under evals/files/. The shape is unusual, and the scanner has evidently not been exercised on it.

What scoping exists today

Mechanism Scope Applies to User-settable
_SKIP_DIRS in build_context.py 7 fixed directory names discovery, so all analyzers no
_EVAL_DATASET_FILES 8 exact relative paths static patterns only no
--baseline one content fingerprint per finding findings already seen yes, after the fact

_SKIP_DIRS is the real scope boundary. It holds .git, __pycache__, node_modules, .venv, venv, .tox, and .pytest_cache, and it is applied during discovery, before any analyzer runs, which is why excluded trees show up in the ledger as excluded_directory rather than as suppressed findings. It is a frozenset in source.

_EVAL_DATASET_FILES is narrower than its name suggests. It is a set of eight exact relative paths (evals/evals.json, .jsonl, .yaml, .yml, and the eval/dataset.* equivalents), matched by exact membership after path normalization. It covers the dataset manifest and nothing the manifest references. It is also consulted only from static_runner.py, so it has no effect on the semantic analyzers, none of which carry any exclusion logic.

This is where the documented principle and the implementation part company. docs/EVAL_DATASETS.md says security analysis should still cover "executable skill code, instructions, scripts, dependencies, MCP metadata, and other install-time surfaces", which is the right boundary. A pyproject.toml belonging to a test fixture is read as a dependency surface, and the eight-path exemption cannot reach it.

That leaves --baseline, which is the only user-facing mechanism of any kind. The full option set is --baseline, --format, --no-llm, --output, --reason, --recursive, --show-suppressed, --verbose, --version, and --yara-rules-dir. There is no exclude, ignore, or scope option, and no configuration file is read from anywhere.

Why this matters left alone

The verdict is decided by test fixtures rather than by the skill. Twenty-two of twenty-three findings came from fixture packaging. A reader of that report learns almost nothing about the skill they were considering installing, and the one finding that was about the skill is buried under twenty-two that were not.

The only way through degrades the repository. Passing required deleting pyproject.toml from eleven test fixtures. Nothing about the skill's own instructions, scripts, or declared dependencies changed, the fixtures became harder to build, and the score moved by 58 points. A gate that can be cleared this way is measuring layout rather than risk.

Baselines cannot carry this. --baseline records a content fingerprint per finding, so an accepted fixture finding stops matching as soon as the fixture changes for an unrelated reason, and each new fixture arrives unsuppressed. Using it here would mean regenerating and re-reviewing a fingerprint file on ordinary edits to test data, which is a lot of ceremony for a decision that was already made once.

The incentive points the wrong way. Skills are asked to ship eval datasets, and a dataset is only as good as the fixtures it exercises. Right now the scanner charges a skill for shipping those fixtures, and the cheapest response is to stop shipping them, or to strip them until they no longer build. That trades away evaluation quality to satisfy a security score, on files the tool's own documentation classifies as test-case data.

The scoping the tool already performs is invisible to the people affected by it. Seven directory names are skipped and eight dataset paths are exempted, both decided in source. A repository whose layout falls outside those constants has no way to say so, and no indication in the output that saying so is even a concept.

Environment

  • SkillSpector 2.5.1
  • skillspector scan <dir> --format json
  • Source inspected at a818f50
  • Linux, Python 3.12

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions