[False positive] Output references like "save to save.md" make a clean skill partial: is_complete=false / safe_to_install=false #510

Summary

When a SKILL.md says the agent will write a file at runtime (an output, not a bundled input) — for example "save the analysis result to save.md" — that file is typically absent from the scanned bundle. Depending on how the filename is written, the scanner treats it as a missing local reference, records reference_unresolved, and flips an otherwise clean scan from complete to partial:

• analysis_completeness.is_complete becomes false,
• the recommendation is raised from SAFE to CAUTION,
• the MCP verdict returns safe_to_install: false,

even though the scan has 100% file coverage, zero findings, and execution_successful: true. A completely benign skill can therefore be reported as "not safe to install" solely because its documentation mentions an output filename in a machine-readable way. This is not hypothetical: the project's own docs use the same wording (e.g. contrib/batch_scan/docs/archive/FUTURE_WORK.md:30: "Write per-skill results to _batch_checkpoint.jsonl"), so a skill authored in that style would be flagged identically.

Environment

• SkillSpector 2.11.1, main @ 704bc9544260c2f41222dc0f92982521709496ab
• --no-llm (static only)
• Tested on macOS with Python 3.12. Reference resolution is pure Python, so the behavior should be platform-independent.

Minimal reproduction

mkdir -p repro/{a,b,c,d}

# Case A — plain prose, unquoted, no directory component (control)
cat > repro/a/SKILL.md <<'EOF'
# Demo skill

Save the analysis result to save.md.
EOF

# Case B — code span
cat > repro/b/SKILL.md <<'EOF'
# Demo skill

Save the analysis result to `save.md`.
EOF

# Case C — Markdown link
cat > repro/c/SKILL.md <<'EOF'
# Demo skill

Save the analysis result to [save.md](save.md).
EOF

# Case D — path with a directory component
cat > repro/d/SKILL.md <<'EOF'
# Demo skill

Save the analysis result to output/save.md.
EOF

# Run each and inspect analysis_completeness in the JSON output.
for c in a b c d; do
  skillspector scan "repro/$c" --no-llm --format json
done

Actual result

Reproduced on the commit cited above (verified independently against the checkout):

Cases B/C/D all report execution_successful: true, coverage_percent: 100.0, fully_inspected_files: 1, partially_inspected_files: 0, entirely_uninspected_files: 0, and no findings. Representative analysis_completeness:

{
  "is_complete": false,
  "status": "partial",
  "execution_successful": true,
  "coverage_percent": 100.0,
  "fully_inspected_files": 1,
  "partially_inspected_files": 0,
  "entirely_uninspected_files": 0,
  "ledger_exceptions": [{
    "outcome": "partial",
    "phase": "reference_resolution",
    "reason_code": "reference_unresolved",
    "path": "SKILL.md",
    "start_line": 3
  }]
}

On the MCP path, mcp_server.py computes (simplified) safe_to_install = risk_score <= RISK_THRESHOLD and execution_successful and entirely_uninspected == 0 and is_complete; the last clause fails here, so safe_to_install is false (see also tests/unit/test_mcp_server.py::test_mcp_blocks_install_when_analysis_is_incomplete).

Expected result

Mentioning a runtime output file is not a content-coverage gap. Both of the following must hold after a fix; they are the acceptance criteria used in Verification below:

• Gating level (minimum). safe_to_install must not be false when the scan's only incompleteness is a reference-class caveat (reference_unresolved, i.e. missing/ambiguous references): execution_successful: true, all discovered files fully inspected (entirely_uninspected: 0 and partially_inspected: 0), no AE1-style coverage findings, and — for the zero-finding reproduction scenario — no findings of any other kind. (Outside that scenario, findings keep being governed by the existing risk-score threshold, not by this rule.) The report may still say partial / CAUTION so the caveat remains visible to a human or policy layer — recommendation: CAUTION together with safe_to_install: true is the intended, deliberate combination and must be documented as such.
• Extraction level (preferred). Output references are recognized (or declarable) as non-blocking output dispositions, so B/C/D actually report complete / SAFE, with the output reference shown informationally.

In no case should a zero-finding, fully-inspected scan become safe_to_install: false solely because its SKILL.md mentions an output file.

Conversely — and this is equally important — the fix must not weaken today's hard block for content-bearing or accounting loss: unreadable/opaque/size-limited bytes, analyzer or LLM failures, truncated reference extraction (see "Why a missing reference hides no bytes" and the refined Option A below). Those cases must keep failing closed.

Root cause

Reference extraction (references.py::_candidate_strings) is purely syntactic and has no notion of output direction. Any path-like token — code span, Markdown link, or directory+extension path — must resolve against the already-discovered bundle inventory (resolve_bundle_references_with_metadata resolves only against known_paths). A runtime output never exists in the bundle, so it resolves to missing; build_context then emits a REFERENCE_UNRESOLVED partial ledger event, finalize_ledger marks the scan partial (is_complete=false), and the report/MCP layers fail closed.

Two distinct problems follow:

1. False-positive surface (extraction). Output-direction paths (save/write/export/create ... <path>) are treated exactly like missing inputs. I acknowledge there is no reliable purely-syntactic way to tell an output from a missing input (the same root-cause statement as Plain slash-separated prose is misclassified as local file references #450) — that is why the fix options below are conservative and some are opt-in.
2. Signal granularity (gating). is_complete / safe_to_install flatten very different causes into one boolean: an unresolved output reference (semantically harmless, no bytes involved) is indistinguishable from genuinely uninspected content (unreadable file, opaque binary, size limit, analyzer failure — cf. AE3/AE4 analyze binary assets the ledger already marks opaque_content (85-byte PNG -> HIGH, DO_NOT_INSTALL) #413, Oversized companion file causes SkillSpector to skip analysis of unrelated skill files (v2.8.2) (another malicious skill example that bypasses this scanner) #363). A "reference caveat" thus becomes an install block.

Why a missing reference hides no bytes (and what still does)

This is the security argument that justifies stopping reference caveats from blocking installs without opening an evasion hole:

• Analysis scope is discovery-driven, not reference-driven. Every discovered file is inventoried and accounted (text files are analyzed; binary/opaque files are recorded as such) regardless of whether SKILL.md mentions it. A reference only annotates (and, when resolved, retains out-of-scope inventory for inspection); it never gates the inspection of another bundled file.
• Missing/ambiguous references never synthesize AE1. finalize_inspection_ledger.py::_reference_coverage_findings creates AE1 only for status == "resolved" targets whose disposition ended up partial/failed/out_of_scope; unresolved references are deliberately excluded (locked by test_unresolved_reference_does_not_synthesize_ae1). So a missing reference does not currently produce any finding — only a caveat.
• What genuinely hides content is a resolved-but-uninspected target (opaque binary, size limit, unreadable file, analyzer failure). Those already surface as entirely_uninspected > 0 / partially_inspected > 0 / AE1, and they must keep blocking. Verified: a SKILL.md linking to a bundled binary yields entirely_uninspected_files: 1 with an opaque_content exception and is_complete: false.

This reading is consistent with the intent behind #410 (enforce discovery and analysis completeness in the MCP verdict): #410 was about content that was not inspected; a documentation mention of a file that does not exist in the bundle is not uninspected content, because all discovered bytes were fully inspected.

The decision this issue asks maintainers to make

Under Option A, a genuinely missing input reference (e.g. "load config.yaml", where the file should have been bundled) is indistinguishable from an output reference — both keep safe_to_install: true and stay visible as a CAUTION caveat. The case for A is that a missing reference hides no bundled bytes (every discovered file was still inspected) and the caveat remains visible. If maintainers instead consider an undeclared missing-input reference install-blocking — the position behind #410's completeness gate — then only Option C (author-side declaration) can tell outputs from missing inputs precisely, and the sensible end-state is: declared outputs are non-blocking, all other unresolved references keep blocking. Either end-state is defensible; this issue asks for that decision to be explicit rather than the current implicit conflation.

Why #451 and #507 do not cover this

• Plain slash-separated prose is misclassified as local file references #450 / Fix false local references from slash-separated prose #451 address unquoted, extensionless, slash-separated prose (e.g. reads/writes). Fix false local references from slash-separated prose #451's stated tradeoff preserves references/guide.md (directory segment + extension) and all quoted/code-span/Markdown-link forms — cases B/C/D above remain missing even after Fix false local references from slash-separated prose #451 merges.
• False positive error with backtick variables like $ARGUMENTS #464 / fix: avoid false parser limits and reference accounting failures #507 are about backticked shell variables (`$ARGUMENTS`) tripping the bounded printf/parser path (static_parse_limit), a different root cause that does not change reference resolution.

So this is a distinct gap: the resolver cannot express "this path is a runtime output", and no author-side escape hatch exists.

One boundary case to keep in mind: #464/#507 live in the same is_complete boolean and currently block installs too. They are accounting failures (the reference layer was truncated), not wording-level caveats, and they must be handled separately from the present report — the refined Option A below is careful not to sweep them into the non-blocking bucket.

Suggested fix directions

Option A — Stop treating a pure reference caveat as an install blocker, by exception kind (smallest, refined)

is_complete / status: partial and all reporting stay exactly as today. Only the MCP safe_to_install gate changes.

A naive rewrite ("block only when execution_successful == false, entirely_uninspected > 0, partially_inspected > 0, or AE1 exists") is not equivalent to "block only on content-bearing coverage loss": the file-count projection in inspection_ledger.py is file-level and does not reflect reference-phase or analyzer-phase exceptions that carry no per-file planned work (e.g. reference_extraction_limit, parsing-phase static_parse_limit, output_limit). Those would silently become non-blocking, which is wrong — they indicate the accounting layer itself was incomplete, not a wording choice.

The precise rule is therefore: safe_to_install stays false for every incompleteness except a pure reference-class caveat. Concretely, block when any of the following is true; only a scan whose sole incompleteness is reference_unresolved (missing/ambiguous references) keeps safe_to_install: true:

• execution_successful == false (any fatal ledger exception);
• entirely_uninspected_files > 0 or partially_inspected_files > 0;
• any AE1 finding (rule_id == "AE1", resolved target not completely inspected);
• any non-fatal ledger exception whose reason is not reference_unresolved (content/analysis/accounting loss: opaque_content, size_limit, syntax_error, static_parse_limit, reference_extraction_limit, output_limit, llm_*, analyzer_runtime_error, ...);
• any limitations entry that is not itself a pure reference caveat — limitations is a separate list from ledger_exceptions and also drives is_complete today, e.g. a requested/configured analyzer that did not execute or degraded (semantic analyzer unavailable, meta degraded).

If these projections do not already carry a machine-readable kind, the classification should be derived from LedgerReason (and the limitation source) rather than hand-maintained — which is exactly what Option D provides.

To implement this cleanly, tag ledger_exceptions (and, where relevant, limitations) entries with a kind (reference_caveat vs coverage_loss/accounting), i.e. land Option A together with Option D; the allowlist/blocklist then falls out of the tag instead of drifting.

• Pros: no extraction change; the evasion surface stays small (a genuinely missing input is still reported, still CAUTION, and still visible — it only stops flipping the install boolean); bytes actually skipped or unread still fail closed; consistent with fix(security): enforce discovery and analysis completeness #410 once the "missing references hide no bytes" argument above is accepted.
• Cons: changes an existing MCP semantic (safe_to_install no longer equals is_complete — and can legitimately be true while recommendation is CAUTION); needs deliberate acceptance, docs, test updates (including rewriting test_mcp_blocks_install_when_analysis_is_incomplete, whose current fixture carries no content-bearing signal and would flip under the new rule — see Verification).

Option B — Output-direction triage in the resolver (medium, heuristic)

Classify references in an output-direction context (save/write/export/create/append ... to <path>) as a new non-blocking disposition (e.g. output), applied only to quoted/code-span/Markdown-link forms; they are reported like scope_exclusions but do not emit reference_unresolved and do not flip is_complete.

• Pros: removes the false positive for the most common benign wording.
• Cons: heuristic can be gamed (a missing input phrased as an output); needs an explicit opt-in or documented heuristic, plus a conservative extension allowlist (e.g. .md/.json/.csv/.log/.txt).

Important: Option B must never be the sole mechanism. Because B removes the reference from completeness accounting entirely, a gamed skill would lose even the human-visible CAUTION that Option A preserves. B is only acceptable on top of A (so the report still shows the caveat) and off by default as a documented heuristic.

Option C — Author-side explicit declaration (most robust, opt-in)

Support an explicit marker so documentation can declare runtime outputs without weakening any other reference's strictness, e.g. frontmatter outputs: or an HTML comment:

<!-- skillspector: output save.md, output/report.json -->

Declared outputs are validated syntactically but excluded from completeness accounting (still reported). Everything else keeps today's strict behavior.

• Pros: precise, no heuristic, minimal evasion risk; covers B/C/D and future file-like prose.
• Cons: new spec surface (parsing, docs, tests); only helps skills that opt in.

Note: under C, B/C/D become complete, so the report-level SAFE→CAUTION lift (which fires on any incomplete scan) stops applying and their recommendation returns to SAFE. That is intended, but it is an observable classification change for those cases and should be called out in the changelog; the SAFE/CAUTION/DO_NOT_INSTALL mapping rules themselves are unchanged.

Option D — Richer coverage status (observability, now part of the recommended path)

Split partial into partial(references) vs partial(coverage) (or tag ledger_exceptions and, where relevant, limitations entries with a kind) and document that consumers should hard-block only on content-coverage/accounting loss, not on reference caveats. D is a prerequisite for implementing A precisely (see the refined predicate above), not merely complementary.

Recommended combination

A + D as the primary fix (gate semantics + exception kind tag so the gate is defined by reason class, not by hand-maintained counts), C as the long-term opt-in for authors who want output mentions to be recognized at extraction, and — only on top of A and off by default — B as a documented heuristic for the common benign wording. Under every accepted combination, B/C/D must be complete or at minimum safe_to_install: true with the reference caveat visible; A's rule must never let content-bearing or accounting loss pass silently.

Scope

• Reference extraction / accounting: src/skillspector/references.py, src/skillspector/nodes/build_context.py
• Completeness projection: src/skillspector/inspection_ledger.py (add kind/exception classification for Option A+D), src/skillspector/nodes/finalize_inspection_ledger.py
• MCP verdict: src/skillspector/mcp_server.py
• Report/recommendation lift: src/skillspector/nodes/report.py (only if C lands — the SAFE→CAUTION lift no longer fires for recognized outputs; no rule change)
• No change to risk scoring, finding rules (including AE1), or the SAFE/CAUTION/DO_NOT_INSTALL mapping rules. Classifications of these specific cases may change under C (CAUTION → SAFE), as documented above.

Verification / regression expectations

Fixtures: add A–D as fixtures, plus an AE1-style case (SKILL.md linking a bundled opaque binary) and a reference-accounting case (extraction-limit/static_parse_limit) to prove the gate still fails closed on content/accounting loss. Concretely:

• A stays complete / SAFE in every configuration.
• Under A+D: B/C/D report partial / CAUTION but safe_to_install: true (new positive tests, e.g. test_mcp_output_reference_caveat_does_not_block_install and test_reference_caveat_kind_does_not_block_install). Under C: B/C/D report complete / SAFE.
• Never a silent pass: the caveat must remain visible (CAUTION in the report / on the MCP result) even when safe_to_install is true, and the CAUTION + safe_to_install: true combination must be documented as intentional.
• Still fail closed: opaque/binary, unreadable, size-limited, analyzer-failed, LLM-failed, and truncated-accounting (reference_extraction_limit, parsing static_parse_limit) scans keep safe_to_install: false, including scans whose only incompleteness is a non-reference limitations entry (a requested/configured analyzer that did not run or degraded).
• Existing tests to keep or adjust deliberately:
  • tests/nodes/test_finalize_inspection_ledger.py::test_unresolved_reference_does_not_synthesize_ae1 — unchanged (missing refs still never become AE1).
  • tests/nodes/test_security_remediation.py::test_missing_primary_reference_blocks_complete_verdict — unchanged (is_complete still false, recommendation still not SAFE for a genuinely missing input; only the MCP install boolean changes).
  • tests/unit/test_mcp_server.py::test_mcp_blocks_install_when_analysis_is_incomplete — must be rewritten: its current fixture sets is_complete: false with entirely_uninspected_files: 0 and ledger_exceptions: [] and no AE1, i.e. no content-bearing signal at all; under A+D that fixture would (correctly) yield safe_to_install: true. Rewrite it to carry a content/accounting signal (e.g. entirely_uninspected_files: 1 with an opaque_content exception) so the "content loss still blocks" intent is preserved, and add the positive reference-caveat test above.

Related

All open at the time of writing (2026-09-09):

• Plain slash-separated prose is misclassified as local file references #450 — Plain slash-separated prose is misclassified as local file references
• Fix false local references from slash-separated prose #451 — Fix false local references from slash-separated prose (does not cover output paths)
• False positive error with backtick variables like $ARGUMENTS #464 — False positive error with backtick variables like $ARGUMENTS (same symptom shape, different root cause)
• fix: avoid false parser limits and reference accounting failures #507 — fix: avoid false parser limits and reference accounting failures (different root cause)
• Coverage is computed but never affects the exit code: a partially-blind scan is indistinguishable from a complete one in CI #389 — Coverage is computed but never affects the exit code (gate semantics context)
• AE3/AE4 analyze binary assets the ledger already marks opaque_content (85-byte PNG -> HIGH, DO_NOT_INSTALL) #413 — AE3/AE4 analyze binary assets the ledger already marks opaque_content (ledger/analyzer disagreement, safe_to_install impact)
• Oversized companion file causes SkillSpector to skip analysis of unrelated skill files (v2.8.2) (another malicious skill example that bypasses this scanner) #363 — Oversized companion file causes SkillSpector to skip analysis of unrelated skill files (content-coverage loss that should keep failing closed)
• fix(security): enforce discovery and analysis completeness #410 — fix(security): enforce discovery and analysis completeness (rationale for keeping completeness tied to the MCP verdict)

AI assistance disclosure: Codex helped inspect the resolver, reproduce the defect on main, review the draft against the codebase, and revise this report; I reviewed the root cause, the reproduction output, the proposed behavior, and the revised gate semantics before writing it.