AE1 classifies opaque-by-type assets as `analysis-evasion` HIGH, discarding the ledger `reason_code` that distinguishes them from real coverage failures

[sdkkds]

Summary

AE1 emits severity: HIGH, confidence: 1.0, category: analysis-evasion for any resolved reference whose target has a non-clean disposition. It does not read the ledger reason_code, so a PNG correctly classified opaque_content is reported identically to an artifact that genuinely failed inspection or resolved out of tree.

This is the follow-up to #413 that @rng1995 asked for in #413 (comment) — the AE1 policy question, filed separately now that PR #422 has closed the AE3/AE4 slice.

I want to lead with a correction to my own original report, because it no longer holds on main.

Correction: the blocking impact from #413 does not reproduce

#413 reported an 85-byte PNG producing 55/100 HIGH DO_NOT_INSTALL. After #422 that is no longer the behavior, and the remaining AE1 finding does not restore it. Measured on main @ d162d9b343e559be13df8ebba093df3bc9d58c90, SkillSpector 2.11.2:

Fixture	Command	Score	Severity	Recommendation	Issues
SKILL.md + 1 PNG	skillspector scan ./ae1-png --no-llm --format json	25	MEDIUM	CAUTION	1 × AE1
SKILL.md + 4 PNGs	skillspector scan ./ae1-png4 --no-llm --format json	43	MEDIUM	CAUTION	4 × AE1
SKILL.md + 8 PNGs	skillspector scan ./ae1-png8 --no-llm --format json	43	MEDIUM	CAUTION	8 × AE1

The score saturates at 43 and never reaches DO_NOT_INSTALL. So the "preserves the reported blocking impact" premise for keeping #413 open was not correct — the install gate is no longer the problem. What remains is a classification and reporting problem, which is why I think it deserves its own issue rather than continued life on #413.

Reproduction

Skill directory, nothing else in it:

ae1-png/
  SKILL.md
  diagram.png     # 70 bytes, valid 1x1 PNG

SKILL.md:

---
name: ae1-png
description: Minimal skill that references one small PNG asset and nothing else.
---

# ae1-png

This skill does nothing but reference a diagram.

![diagram](diagram.png)

Run skillspector scan ./ae1-png --no-llm --format json. --no-llm is deliberate — the finding is purely static, so this is deterministic and reproducible without a provider.

Observed

One issue:

{
  "id": "AE1",
  "category": "analysis-evasion",
  "severity": "HIGH",
  "confidence": 1.0,
  "finding": "diagram.png (partial)",
  "explanation": "Referenced artifact was not completely inspected",
  "remediation": "Make the referenced artifact locally available and fully analyzable, or remove the reference.",
  "tags": ["coverage", "reference", "target-disposition:partial"]
}

And, in the same report, the ledger has already recorded exactly why the artifact was not inspected:

{
  "outcome": "partial",
  "phase": "static",
  "reason_code": "opaque_content",
  "message": "Artifact contents could not be fully interpreted.",
  "path": "diagram.png",
  "fatal": false,
  "analyzers": ["static_patterns_agent_snooping", "...15 static pattern analyzers..."]
}

Why I think this is a defect and not just a policy preference

1. The discriminating information exists and is discarded. LedgerReason already separates the cases — BINARY_CONTENT ("Binary content is unsupported by this analyzer") and OPAQUE_CONTENT ("Artifact contents could not be fully interpreted") at src/skillspector/inspection_ledger.py:52 and :84 — from outcomes like failed. Ledger events carry that value: event["reason_code"] = reason at inspection_ledger.py:437 and :502. But _reference_coverage_findings reads only the outcome:

# src/skillspector/nodes/finalize_inspection_ledger.py:44
outcome = str(event.get("outcome", ""))
if outcome in {"partial", "failed", "out_of_scope"}:
    exceptional_outcomes.setdefault(str(event.get("path", "")), set()).add(outcome)

reason_code is never consulted, so opaque_content and a genuine inspection failure collapse into the same partial and produce identical findings.

2. The codebase already has the idiom, in two places. Ledger events are filtered on their reason elsewhere — event.get("reason_code") == LedgerReason.OUTPUT_LIMIT at inspection_ledger.py:720. And the neighbouring rule in AE1's own module, _size_coverage_findings (AE7), scopes itself the same way:

# src/skillspector/nodes/finalize_inspection_ledger.py:121
if str(item.get("reason", "")) != LedgerReason.SIZE_LIMIT.value:

AE7 scopes itself to one reason and stays honest about what it is claiming. AE1 is the outlier in its own file, which suggests an oversight rather than a deliberate policy.

(Minor note while I was in here: AE7 reads reason on inventory items while ledger events use reason_code. Two near-identical concepts under two field names in one module is an easy trap for anyone writing the next rule — happy to normalize that separately if you want it, but I've kept it out of the proposal below.)

3. analysis-evasion is the wrong category for this case. Evasion implies an actor did something to avoid inspection. A PNG referenced from SKILL.md, present on disk, correctly typed, and hashable is not evading anything — the scanner simply does not read image formats. Filing that under evasion at confidence: 1.0 means the category cannot distinguish "someone hid something" from "we don't parse this format," which is the distinction the category exists to make.

To be clear about what I am not arguing: the confidence of 1.0 is correct. AE1 states an observed fact — the artifact was not inspected — and that fact is true. My objection is to severity and category, not to the finding's existence or its confidence.

4. The completeness block becomes unreadable, which is the part I actually rely on. The 8-PNG fixture reports:

coverage_percent: 11.1
entirely_uninspected_files: 8
is_complete: false
status: partial

A reviewer reading that sees a skill 89% uninspected. In reality every inspectable byte was inspected; the other eight files are 70-byte images that no static pattern analyzer will ever read, in this or any future version. Counting permanently-unparseable formats against coverage means coverage_percent measures the scanner's format support rather than how much of the skill got reviewed, and it will read as alarming on every skill that ships a diagram. For anyone using the completeness block as the trust signal — which your own reporting design encourages — this is the most damaging part of the behavior, more so than the score.

The counter-argument, stated fairly

Binary assets in skills are a real attack surface: polyglot files, steganographic payloads, an executable renamed .png. If the project's position is that any uninspectable binary must surface loudly, that is defensible and I am not asking you to drop the signal. I am asking that it not be called evasion, not be scored as HIGH, and not be counted as missing coverage.

Proposed change

The defect fix, which I think stands on its own:

1. Propagate the ledger reason_code into exceptional_outcomes alongside the outcome in _reference_coverage_findings.
2. Keep AE1 unchanged (HIGH, analysis-evasion, confidence 1.0) for partial / failed and for any out_of_scope whose reason is not BINARY_CONTENT / OPAQUE_CONTENT. Those are the cases where something genuinely went wrong or a reference escaped the tree, and they should stay loud.

The policy question, which is yours and not mine: items 1-2 remove a signal without putting anything in its place, and you may not want that. The obvious replacement is a distinct rule for the opaque-by-type case — something like AE8, category: uninspectable-asset, severity: MEDIUM, confidence: 1.0, remediation along the lines of "this asset's format cannot be statically analyzed; confirm its provenance independently." Same factual claim, honest category, no evasion implication.

I'm not proposing that as part of the fix, because it adds a rule id and a category and that is a product decision rather than a defect repair. #422 was scrupulous about not changing AE1 policy, artifact classification, or analyzer registration unasked, and I'd rather not be the PR that does it unasked either.

There's also a reporting question I've deliberately left alone: whether artifacts whose only exception is BINARY_CONTENT / OPAQUE_CONTENT should be excluded from the coverage_percent denominator and surfaced as a separate uninspectable_by_format count. That changes a reported metric other consumers may key on, so it seems squarely yours.

Offer

I'm happy to open a PR for items 1-2 — contained to finalize_inspection_ledger.py plus test updates. Note for whoever reviews it: there are 33 AE1 assertions across 9 test files, so I'd expect to touch fixtures beyond the obvious ones, and I'd want to confirm none of them are asserting AE1 on a binary deliberately.

If you decide you want the AE8 replacement rule as well, I'll write that too — but only on your say-so, and as a separate PR so the defect fix isn't held up behind a policy call. Equally happy to just leave you the reproduction and let a maintainer take it; the fixtures are trivial to regenerate and I can push them as test fixtures if that's useful.

Environment

• SkillSpector 2.11.2, main @ d162d9b343e559be13df8ebba093df3bc9d58c90
• Run via uv run skillspector scan <fixture> --no-llm --format json from a clean shallow clone
• Windows 11, Python via uv
• All three fixtures are generated by a short script; happy to attach it or push them as test fixtures if useful

Refs #413, #422

[chrisknvidia]

There is a new PR open and working on it: #597.

It is currently a draft while I implement the reason-aware AE1 fix and complete multiple local review passes and end-to-end validation. I will mark it ready for review after those checks.

[chrisknvidia]

PR #597 is now ready for review. All changes are pushed at 2fd5f97.

Completed two independent local implementation review passes, fixed the identified edge cases, and passed real CLI, MCP stdio, and packaged Docker end-to-end checks. All six hosted checks are green, including 6,007 passing tests; the local integration sweep passed all 34 tests. The PR description records the validation and scope.

GitHub did not allow assigning this issue to @chrisknvidia, so issue assignment still needs a maintainer.

[chrisknvidia]

PR #597 is ready for review at 2f59a5bd9010392762610580119531a504d3e3e1.

After moving it back to Draft for the deeper passes, I fixed the additional ledger/schema and executable-signature edge cases found by Review Guru, Bugbot, and security review. All six exact-head hosted checks are green; the hosted unit job reported 6,059 passed, 14 skipped, 62 deselected, and 4 xfailed. Packaged CLI, repository Docker smoke, and a 14-scan adversarial Docker matrix also passed.

The PR remains unapproved pending human review. GitHub still does not allow @chrisknvidia to assign this upstream issue because the account has read-only repository permission, so a maintainer must make that assignment.