From 5055f283ebffd5ca59322fe8f7eb79e1dfab9495 Mon Sep 17 00:00:00 2001 From: Mohit Gupta Date: Wed, 16 Sep 2026 02:38:48 +0530 Subject: [PATCH 1/3] fix: preserve complete Markdown reference destinations Resolve inline and reference-definition destinations with bounded parsing, one-time URI decoding, and preserved containment. Keep required missing files incomplete and retain real numeric-extension references. Add 90 paired resolver and CLI/MCP regression tests covering titles, spaces, balanced parentheses, encoded paths, and deadline enforcement. Prepared by Codex on behalf of Mohit Gupta. Signed-off-by: Mohit Gupta --- src/skillspector/references.py | 227 +++++++++++++++++++-------- tests/test_reference_destinations.py | 224 ++++++++++++++++++++++++++ 2 files changed, 389 insertions(+), 62 deletions(-) create mode 100644 tests/test_reference_destinations.py diff --git a/src/skillspector/references.py b/src/skillspector/references.py index 6ac2de8f2..28bb2f4c8 100644 --- a/src/skillspector/references.py +++ b/src/skillspector/references.py @@ -23,8 +23,15 @@ MAX_REFERENCE_RECORDS = 1024 MAX_REFERENCE_RUNTIME_SECONDS = 2.0 _MAX_EVIDENCE = 160 -_MARKDOWN_IMAGE_DESTINATION = re.compile(r"!\[[^\]\n]{0,200}\]\(([^)\n]{1,512})\)") -_MARKDOWN_LINK_DESTINATION = re.compile(r"(?])") _PASSIVE_IMAGE_DESTINATION = re.compile( r"[^\s\\()\[\]<>]+(?:[ \t]+(?:\"[^\"\\\r\n()]*\"|'[^'\\\r\n()]*'))?" ) @@ -77,6 +84,120 @@ def _evidence(cleaned_line: str, column: int) -> str: return cleaned_line[start : start + _MAX_EVIDENCE] +@dataclass(frozen=True) +class _ReferenceCandidate: + raw: str + start: int + end: int + kind: ReferenceKind + syntax_start: int + passive_image: bool = False + + +def _markdown_candidates( + line: str, *, deadline: float, clock: Callable[[], float], limitations: set[str] +) -> Iterator[_ReferenceCandidate]: + """Read bounded destinations without splitting spaces or balanced parentheses. + + Explicit reference definitions use the same destination grammar as inline + links. Recognizing them separately keeps slash-separated prose excluded. + """ + consumed_until = 0 + for opening in _MARKDOWN_REFERENCE_START.finditer(line): + # Malformed openings may never yield a candidate. Bound their work too. + if clock() >= deadline: + return + if opening.start() < consumed_until: + continue + start = opening.end() + while start < len(line) and line[start] in " \t": + start += 1 + if start >= len(line): + continue + limit = min(len(line), start + _MAX_MARKDOWN_DESTINATION_CHARS + 2) + end = start + if line[start] == "<": + start += 1 + end = start + while end < limit and line[end] not in "<>\r\n": + if line[end] == "\\" and end + 1 < limit: + end += 1 + end += 1 + if end - start > _MAX_MARKDOWN_DESTINATION_CHARS: + limitations.add("markdown_destination") + continue + if end >= limit or line[end] != ">": + continue + raw = line[start:end] + destination_end = end + 1 + else: + depth = 0 + while end < limit and not line[end].isspace(): + char = line[end] + if char == "\\" and end + 1 < limit: + end += 2 + continue + if char == "(": + depth += 1 + elif char == ")": + if depth == 0: + break + depth -= 1 + elif char in "<>": + break + end += 1 + if end - start > _MAX_MARKDOWN_DESTINATION_CHARS: + limitations.add("markdown_destination") + continue + if depth or end == start: + continue + raw = line[start:end] + destination_end = end + if opening.group().endswith("("): + ending = _MARKDOWN_INLINE_END.match( + line, destination_end, min(len(line), destination_end + 512) + ) + if ending is None: + if len(line) - destination_end > 512: + limitations.add("markdown_title") + continue + else: + if len(line) - destination_end > 512: + limitations.add("markdown_title") + continue + ending = _MARKDOWN_DEFINITION_END.fullmatch(line, destination_end) + # A destination can only be followed by a title, not arbitrary prose. + if ending is None: + continue + consumed_until = ending.end() + is_image = opening.group().startswith("![") + label = line[opening.start() + 2 : opening.end() - 2] if is_image else "" + # Retain main's conservative passive-image grammar even when the + # destination parser can recover a more complex literal filename. + passive_image = ( + is_image + and not any(char in label for char in "[]\\") + and bool(_PASSIVE_IMAGE_DESTINATION.fullmatch(line[opening.end() : consumed_until - 1])) + ) + yield _ReferenceCandidate( + _MARKDOWN_STRUCTURAL_ESCAPE.sub(r"\1", raw), + start, + consumed_until, + ReferenceKind.MARKDOWN_IMAGE if is_image else ReferenceKind.MARKDOWN_LINK, + opening.start(), + passive_image, + ) + + +def _pattern_candidates( + pattern: re.Pattern[str], line: str, kind: ReferenceKind +) -> Iterator[_ReferenceCandidate]: + for match in pattern.finditer(line): + yield _ReferenceCandidate( + match.group(1), match.start(1), match.end(1), kind, match.start(1) + ) + + def _advance_inline_code_state( line: str, end: int, @@ -156,20 +277,14 @@ def _candidate_strings( ) -> tuple[list[tuple[str, int, int, str, ReferenceKind]], tuple[str, ...]]: """Extract path-like strings without materializing all matches or lines. - Each regular expression contributes at most one pending match to a small + Each candidate iterator contributes at most one pending match to a small merge heap. This preserves source ordering while ensuring a dense, attacker-controlled line cannot be fully enumerated and sorted before the candidate and time ceilings are enforced. """ candidates: list[tuple[str, int, int, str, ReferenceKind]] = [] + limitations: set[str] = set() seen: set[tuple[int, int, str]] = set() - patterns = ( - (_MARKDOWN_IMAGE_DESTINATION, ReferenceKind.MARKDOWN_IMAGE, True), - (_MARKDOWN_LINK_DESTINATION, ReferenceKind.MARKDOWN_LINK, True), - (_INLINE_CODE_COMMAND_PATH, ReferenceKind.INLINE_COMMAND, False), - (_QUOTED_OR_CODE_PATH, ReferenceKind.QUOTED_OR_CODE, False), - (_PLAIN_RELATIVE_PATH, ReferenceKind.PLAIN_PATH, False), - ) active_fence: tuple[str, int, int, int] | None = None in_html = False html_end: re.Pattern[str] | None = None @@ -225,75 +340,71 @@ def _candidate_strings( in_html = False html_end = None cleaned_line = " ".join(line.strip().split()) - iterators: list[Iterator[re.Match[str]]] = [ - pattern.finditer(line) for pattern, _, _ in patterns + iterators = [ + _markdown_candidates(line, deadline=deadline, clock=clock, limitations=limitations), + _pattern_candidates(_INLINE_CODE_COMMAND_PATH, line, ReferenceKind.INLINE_COMMAND), + _pattern_candidates(_QUOTED_OR_CODE_PATH, line, ReferenceKind.QUOTED_OR_CODE), + _pattern_candidates(_PLAIN_RELATIVE_PATH, line, ReferenceKind.PLAIN_PATH), ] - pending: list[tuple[int, int, int, int, re.Match[str]]] = [] + pending: list[tuple[int, int, _ReferenceCandidate]] = [] image_label_spans: list[tuple[int, int, str]] = [] + markdown_destination_span: tuple[int, int] | None = None inline_code_cursor = 0 for pattern_index, iterator in enumerate(iterators): match = next(iterator, None) if match is not None: - _, _, is_markdown = patterns[pattern_index] - heapq.heappush( - pending, - ( - match.start(0) if is_markdown else match.start(1), - match.start(1), - match.end(1), - pattern_index, - match, - ), - ) + heapq.heappush(pending, (match.syntax_start, pattern_index, match)) if clock() >= deadline: return candidates, ("runtime",) while pending: if clock() >= deadline: return candidates, ("runtime",) - sort_start, _, _, pattern_index, match = heapq.heappop(pending) - _, reference_kind, is_markdown = patterns[pattern_index] + sort_start, pattern_index, match = heapq.heappop(pending) + reference_kind = match.kind if not line_in_fence and not line_is_indented_code and not line_in_html: inline_code_cursor, inline_code_delimiter = _advance_inline_code_state( - line, - sort_start, - inline_code_cursor, - inline_code_delimiter, + line, sort_start, inline_code_cursor, inline_code_delimiter ) if reference_kind is ReferenceKind.MARKDOWN_IMAGE: - label = line[match.start(0) + 2 : match.start(1) - 2] - unambiguous_image = not any(char in label for char in "[]\\") and bool( - _PASSIVE_IMAGE_DESTINATION.fullmatch(match.group(1)) - ) if ( line_in_fence or line_is_indented_code or line_in_html or inline_code_delimiter is not None - or not unambiguous_image + or not match.passive_image ): reference_kind = ReferenceKind.QUOTED_OR_CODE - elif _is_escaped_marker(line, match.start(0)): + elif _is_escaped_marker(line, match.syntax_start): reference_kind = ReferenceKind.PLAIN_PATH - raw = match.group(1).strip().split(maxsplit=1)[0] + raw = match.raw if reference_kind is ReferenceKind.MARKDOWN_IMAGE: - # Only the same literal target in passive image alt text is - # redundant. Visible link labels and command operands can name - # independent artifacts whose coverage must still be checked. - image_label_spans.append((match.start(0), match.start(1), raw)) + # Same-target image alt text is redundant; different visible + # targets and command operands still require their own records. + image_label_spans.append((match.syntax_start, match.start, raw)) redundant_image_label = reference_kind is ReferenceKind.PLAIN_PATH and any( - start <= match.start(1) < end and raw == destination + start <= match.start < end and raw == destination for start, end, destination in image_label_spans ) - if not redundant_image_label: - key = (line_number, match.start(1), raw) + # Markdown candidates sort at their opening marker so their labels + # can be classified. Suppress only destination/title text, not an + # independent artifact named in a visible link or image label. + inside_destination = ( + pattern_index != 0 + and markdown_destination_span is not None + and markdown_destination_span[0] <= match.start < markdown_destination_span[1] + ) + if pattern_index == 0: + markdown_destination_span = (match.start, match.end) + if not inside_destination and not redundant_image_label: + key = (line_number, match.start, raw) if key not in seen: seen.add(key) candidates.append( ( raw, line_number, - match.start(1) + 1, - _evidence(cleaned_line, match.start(1) + 1), + match.start + 1, + _evidence(cleaned_line, match.start + 1), reference_kind, ) ) @@ -301,17 +412,7 @@ def _candidate_strings( return candidates, ("raw_candidates",) next_match = next(iterators[pattern_index], None) if next_match is not None: - _, _, next_is_markdown = patterns[pattern_index] - heapq.heappush( - pending, - ( - next_match.start(0) if next_is_markdown else next_match.start(1), - next_match.start(1), - next_match.end(1), - pattern_index, - next_match, - ), - ) + heapq.heappush(pending, (next_match.syntax_start, pattern_index, next_match)) if not line_in_fence and not line_is_indented_code and not line_in_html: _, inline_code_delimiter = _advance_inline_code_state( line, @@ -321,17 +422,19 @@ def _candidate_strings( ) if closes_fence: active_fence = None - return candidates, () + return candidates, tuple(sorted(limitations)) def _normalize_candidate(raw: str, source_path: str) -> str | None: """Return a contained relative POSIX candidate, or None when unsupported.""" - raw = unquote(raw.strip().strip("<>")) + raw = raw.strip() split = urlsplit(raw) if split.scheme or split.netloc or raw.startswith(("/", "\\", "#")): return None - path_part = split.path.replace("\\", "/") - if not path_part: + # Split URI syntax before decoding so %23/%3F remain filename characters. + # Decode exactly once, then apply containment checks to the decoded path. + path_part = unquote(split.path).replace("\\", "/") + if not path_part or path_part.startswith("/"): return None if len(path_part) >= 2 and path_part[1] == ":": return None @@ -410,7 +513,7 @@ def resolve_bundle_references_with_metadata( resolved_target = target status = "resolved" disposition = ArtifactDisposition.ANALYZED - elif "/" not in raw.replace("\\", "/"): + elif "/" not in unquote(raw).replace("\\", "/"): matches = basename_index.get(PurePosixPath(target).name, []) if len(matches) == 1: resolved_target = matches[0] diff --git a/tests/test_reference_destinations.py b/tests/test_reference_destinations.py new file mode 100644 index 000000000..867040dc4 --- /dev/null +++ b/tests/test_reference_destinations.py @@ -0,0 +1,224 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Resolve Markdown destinations without weakening missing-reference coverage.""" + +import base64 +import json +import os +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from skillspector import references as references_module +from skillspector.cli import app +from skillspector.mcp_server import run_scan +from skillspector.references import resolve_bundle_references_with_metadata + + +@pytest.mark.parametrize( + ("source", "target"), + [ + ("Read [guide]().", "docs/user guide.md"), + (r'Read [guide]( "A \"quoted\" title").', "docs/user guide.md"), + (r"Read [guide]( 'A \'quoted\' title').", "docs/user guide.md"), + (r"Read [guide]( (A \(quoted\) title)).", "docs/user guide.md"), + (r"Read [guide](<\.md>).", ".md"), + (r"Read [manual](\).", ""), + ("Read [guide]().", "docs/user.md guide.md"), + ("Read [guide](docs/guide(v1).md).", "docs/guide(v1).md"), + (r"Read [guide](docs/guide\(v1\).md).", "docs/guide(v1).md"), + ("Read [guide](docs/guide(a(b(c))).md).", "docs/guide(a(b(c))).md"), + ('Read [guide](docs/guide.md "Guide title").', "docs/guide.md"), + ("Read [guide][manual].\n\n[manual]: docs/user%20guide.md", "docs/user guide.md"), + ("Read [guide][manual].\n\n[manual]: ", "docs/user guide.md"), + ("Read [guide][manual].\n\n[manual]: docs/guide(v1).md", "docs/guide(v1).md"), + ("Read [guide](docs/part%23one.md#summary).", "docs/part#one.md"), + ("Read [guide](docs/part%3Fone.md?view=1).", "docs/part?one.md"), + ("Read [guide](docs/part%252Fone.md).", "docs/part%2Fone.md"), + ("Read [guide](docs/caf%C3%A9.md).", "docs/café.md"), + ("Read [manual](tool.1).", "tool.1"), + ("Read `tool.1`.", "tool.1"), + ], +) +@pytest.mark.parametrize("present", [True, False]) +def test_markdown_destinations_preserve_present_and_missing_targets( + tmp_path: Path, source: str, target: str, present: bool +) -> None: + result = resolve_bundle_references_with_metadata( + tmp_path, + source_path="SKILL.md", + source_text=source, + known_paths=["SKILL.md", target] if present else ["SKILL.md"], + ) + assert result.complete is True # Extraction completed; resolution may be missing. + assert result.records + assert {record["status"] for record in result.records} == {"resolved" if present else "missing"} + assert {record["target_path"] for record in result.records} == {target if present else None} + + +@pytest.mark.parametrize( + "target", + ["../outside.md", "%2e%2e/outside.md", "%2Foutside.md", "%5Coutside.md", "C%3A/file.md"], +) +def test_decoded_destination_cannot_escape_bundle(tmp_path: Path, target: str) -> None: + result = resolve_bundle_references_with_metadata( + tmp_path, + source_path="SKILL.md", + source_text=f"Read [guide]({target}).", + known_paths=["SKILL.md"], + ) + assert result.complete is True + assert result.records + assert all(record["status"] == "rejected" for record in result.records) + + +def test_reference_definitions_do_not_restore_slash_prose_false_positives(tmp_path: Path) -> None: + result = resolve_bundle_references_with_metadata( + tmp_path, + source_path="SKILL.md", + source_text="Compare process I/O, reads/writes, and environment/profile settings.", + known_paths=["SKILL.md"], + ) + assert result.complete is True + assert result.records == [] + + +@pytest.mark.parametrize("channel", ["cli", "mcp"]) +@pytest.mark.parametrize("present", [True, False]) +@pytest.mark.parametrize( + ("body", "target"), + [ + ("Read [guide][manual].\n\n[manual]: docs/user%20guide.md", "docs/user guide.md"), + ("Read [guide]().", "docs/user.md guide.md"), + (r'Read [guide]( "A \"quoted\" title").', "docs/user guide.md"), + (r"Read [guide]( 'A \'quoted\' title').", "docs/user guide.md"), + (r"Read [guide]( (A \(quoted\) title)).", "docs/user guide.md"), + (r"Read [guide](<\.md>).", ".md"), + (r"Read [manual](\).", ""), + ("Read [guide](docs/guide(v1).md).", "docs/guide(v1).md"), + ("Read [guide](docs/part%23one.md#summary).", "docs/part#one.md"), + ], +) +async def test_cli_and_mcp_reference_completeness_agree( + tmp_path: Path, body: str, target: str, present: bool, channel: str +) -> None: + if "<" in target and os.name == "nt": + pytest.skip("Literal angle filenames are unsupported on Windows") + (tmp_path / "SKILL.md").write_text( + f"---\nname: reference-control\ndescription: Summarize the guide.\n---\n{body}\n", + encoding="utf-8", + ) + if present: + path = tmp_path / target + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("# Guide\nSummarize the supplied text.\n", encoding="utf-8") + if channel == "cli": + result = CliRunner().invoke( + app, ["scan", str(tmp_path), "--no-llm", "--format", "json", "--fail-on-incomplete"] + ) + assert result.exit_code == (0 if present else 1), result.output + report = json.loads(result.stdout) + else: + result = await run_scan(str(tmp_path), use_llm=False, output_format="json") + assert result["safe_to_install"] is present + report = json.loads(result["report"]) + assert report["execution_successful"] is True + assert report["analysis_completeness"]["is_complete"] is present + assert report["risk_assessment"]["recommendation"] == ("SAFE" if present else "CAUTION") + assert {r["status"] for r in report["analysis_completeness"]["references"]} == { + "resolved" if present else "missing" + } + + +@pytest.mark.parametrize( + ("body", "target"), + [("Read [manual](tool.1).", "tool.1"), (r"Read [manual](\).", "")], +) +async def test_numeric_extension_reference_retains_uninspected_artifact_gap( + tmp_path: Path, body: str, target: str +) -> None: + if "<" in target and os.name == "nt": + pytest.skip("Literal angle filenames are unsupported on Windows") + (tmp_path / "SKILL.md").write_text(f"# Guide\n{body}\n", encoding="utf-8") + if target != "tool.1": + (tmp_path / "tool.1").write_text( + "# Safe decoy\nSummarize supplied text.\n", encoding="utf-8" + ) + # A real raster artifact, independent of its man-page-like filename. + (tmp_path / target).write_bytes( + base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAusB9Wl6" + "SAAAAABJRU5ErkJggg==" + ) + ) + result = await run_scan(str(tmp_path), use_llm=False, output_format="json") + report = json.loads(result["report"]) + assert result["safe_to_install"] is False + assert report["analysis_completeness"]["is_complete"] is False + assert any(r["target_path"] == target for r in report["analysis_completeness"]["references"]) + assert any(finding["id"] == "AE1" for finding in report["issues"]) + + +@pytest.mark.parametrize("separator", ["%2F", "%5C"]) +def test_encoded_directory_does_not_fall_back_to_another_basename( + tmp_path: Path, separator: str +) -> None: + result = resolve_bundle_references_with_metadata( + tmp_path, + source_path="SKILL.md", + source_text=f"Read [guide](missing{separator}guide.md).", + known_paths=["SKILL.md", "other/guide.md"], + ) + assert result.complete is True + assert len(result.records) == 1 + assert result.records[0]["status"] == "missing" + assert result.records[0]["target_path"] is None + + +@pytest.mark.parametrize("body", ["[status]: All checks passed.", "[note]: Read the guide."]) +def test_prose_after_bracket_label_is_not_a_reference(tmp_path: Path, body: str) -> None: + result = resolve_bundle_references_with_metadata( + tmp_path, source_path="SKILL.md", source_text=body, known_paths=["SKILL.md"] + ) + assert result.complete is True + assert result.records == [] + + +def test_escaped_angle_filename_does_not_resolve_different_file(tmp_path: Path) -> None: + result = resolve_bundle_references_with_metadata( + tmp_path, + source_path="SKILL.md", + source_text=r"Read [manual](\).", + known_paths=["SKILL.md", "tool.1"], + ) + assert len(result.records) == 1 + assert result.records[0]["status"] == "missing" + assert result.records[0]["target_path"] is None + + +def test_malformed_markdown_openings_observe_deadline_before_yield( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + original = references_module._MARKDOWN_REFERENCE_START + observed = 0 + + class CountedStarts: + def finditer(self, line: str): + nonlocal observed + for match in original.finditer(line): + observed += 1 + yield match + + monkeypatch.setattr(references_module, "_MARKDOWN_REFERENCE_START", CountedStarts()) + result = resolve_bundle_references_with_metadata( + tmp_path, + source_path="SKILL.md", + source_text="[x](" * 5000, + known_paths=["SKILL.md"], + clock=lambda: observed / 1000, + ) + assert "runtime" in result.limitations + assert result.complete is False + assert observed <= 2001 From 4ee20e1ca3fa7b09dcd7e508910191318c888b5c Mon Sep 17 00:00:00 2001 From: Chandrashekar Ramachandran Date: Tue, 22 Sep 2026 16:44:28 +0530 Subject: [PATCH 2/3] fix(references): preserve bounded Markdown coverage Mark reference extraction incomplete when Markdown destinations or titles exceed the parser limits. Consume the full validated link span so title text is not treated as another local reference. Signed-off-by: Chandrashekar Ramachandran --- tests/test_reference_destinations.py | 88 +++++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/tests/test_reference_destinations.py b/tests/test_reference_destinations.py index 867040dc4..633492c67 100644 --- a/tests/test_reference_destinations.py +++ b/tests/test_reference_destinations.py @@ -35,6 +35,9 @@ ("Read [guide][manual].\n\n[manual]: ", "docs/user guide.md"), ("Read [guide][manual].\n\n[manual]: docs/guide(v1).md", "docs/guide(v1).md"), ("Read [guide](docs/part%23one.md#summary).", "docs/part#one.md"), + ('[guide](docs/guide.md "see [sample](missing.md)")', "docs/guide.md"), + ('[guide](docs/guide.md "missing.md")', "docs/guide.md"), + ('[guide]: docs/guide.md "see docs/missing.md"', "docs/guide.md"), ("Read [guide](docs/part%3Fone.md?view=1).", "docs/part?one.md"), ("Read [guide](docs/part%252Fone.md).", "docs/part%2Fone.md"), ("Read [guide](docs/caf%C3%A9.md).", "docs/café.md"), @@ -99,6 +102,9 @@ def test_reference_definitions_do_not_restore_slash_prose_false_positives(tmp_pa (r"Read [manual](\).", ""), ("Read [guide](docs/guide(v1).md).", "docs/guide(v1).md"), ("Read [guide](docs/part%23one.md#summary).", "docs/part#one.md"), + ('[guide](docs/guide.md "see [sample](missing.md)")', "docs/guide.md"), + ('[guide](docs/guide.md "missing.md")', "docs/guide.md"), + ('[guide]: docs/guide.md "see docs/missing.md"', "docs/guide.md"), ], ) async def test_cli_and_mcp_reference_completeness_agree( @@ -122,7 +128,9 @@ async def test_cli_and_mcp_reference_completeness_agree( report = json.loads(result.stdout) else: result = await run_scan(str(tmp_path), use_llm=False, output_format="json") - assert result["safe_to_install"] is present + # MCP keeps missing-reference-only caveats install-eligible while the + # rendered report still exposes the incomplete analysis. + assert result["safe_to_install"] is True report = json.loads(result["report"]) assert report["execution_successful"] is True assert report["analysis_completeness"]["is_complete"] is present @@ -222,3 +230,81 @@ def finditer(self, line: str): assert "runtime" in result.limitations assert result.complete is False assert observed <= 2001 + + +@pytest.mark.parametrize( + "template", ["[guide]({})", "[guide](<{}>)", "[guide]: {}", "[guide]: <{}>"] +) +@pytest.mark.parametrize("length", [511, 512, 513]) +def test_destination_length_limit_is_reported(tmp_path: Path, template: str, length: int) -> None: + destination = "a%20/" * 100 + "x" * (length - 503) + ".md" + result = resolve_bundle_references_with_metadata( + tmp_path, + source_path="SKILL.md", + source_text=template.format(destination), + known_paths=["SKILL.md"], + ) + assert result.complete is (length <= 512) + if length > 512: + assert "markdown_destination" in result.limitations + else: + assert len(result.records) == 1 + assert result.records[0]["status"] == "missing" + + +@pytest.mark.parametrize("template", ['[guide](docs/a%20b.md "{}")', '[guide]: docs/a%20b.md "{}"']) +def test_title_length_limit_is_reported(tmp_path: Path, template: str) -> None: + result = resolve_bundle_references_with_metadata( + tmp_path, + source_path="SKILL.md", + source_text=template.format("a" * 513), + known_paths=["SKILL.md"], + ) + assert result.complete is False + assert "markdown_title" in result.limitations + + +@pytest.mark.parametrize("template", ['[guide](docs/guide.md "{}")', '[guide]: docs/guide.md "{}"']) +@pytest.mark.parametrize("title", ["see [sample](missing.md)", "missing.md", "see docs/missing.md"]) +def test_title_text_is_not_a_reference(tmp_path: Path, template: str, title: str) -> None: + result = resolve_bundle_references_with_metadata( + tmp_path, + source_path="SKILL.md", + source_text=template.format(title) + "\n[other](docs/other.md)", + known_paths=["SKILL.md", "docs/guide.md", "docs/other.md"], + ) + assert result.complete is True + assert [record["target_path"] for record in result.records] == [ + "docs/guide.md", + "docs/other.md", + ] + + +@pytest.mark.parametrize("channel", ["cli", "mcp"]) +@pytest.mark.parametrize( + "body", + [ + "[guide]: " + "/".join(["a%20b"] * 90) + ".md", + "[guide](<" + "a /" * 180 + "guide.md>)", + '[guide](docs/a%20b.md "' + "a" * 513 + '")', + ], +) +async def test_markdown_limits_block_complete_verdict( + tmp_path: Path, body: str, channel: str +) -> None: + (tmp_path / "SKILL.md").write_text( + "---\nname: reference-control\ndescription: Summarize the guide.\n---\n" + body + "\n", + encoding="utf-8", + ) + if channel == "cli": + result = CliRunner().invoke( + app, ["scan", str(tmp_path), "--no-llm", "--format", "json", "--fail-on-incomplete"] + ) + assert result.exit_code == 1, result.output + report = json.loads(result.stdout) + else: + result = await run_scan(str(tmp_path), use_llm=False, output_format="json") + assert result["safe_to_install"] is False + report = json.loads(result["report"]) + assert report["execution_successful"] is True + assert report["analysis_completeness"]["is_complete"] is False From 4329389a7932bc14c72ef172e6e22095841a2551 Mon Sep 17 00:00:00 2001 From: Mohit Gupta Date: Wed, 23 Sep 2026 14:11:40 +0530 Subject: [PATCH 3/3] test: retain Markdown provenance regression coverage after rebase Preserve the destination/provenance interaction tests introduced during the main integration, including CLI and MCP cases with LLMs disabled and successful mocked semantic transports. Retain the complete escaped filename expectation with conservative reference classification. Restored by Codex on behalf of Mohit Gupta from the tested PR tree. Signed-off-by: Mohit Gupta --- tests/nodes/test_security_remediation.py | 20 +- tests/test_reference_destination_kinds.py | 232 ++++++++++++++++++++++ 2 files changed, 242 insertions(+), 10 deletions(-) create mode 100644 tests/test_reference_destination_kinds.py diff --git a/tests/nodes/test_security_remediation.py b/tests/nodes/test_security_remediation.py index 837f81c62..238ced76f 100644 --- a/tests/nodes/test_security_remediation.py +++ b/tests/nodes/test_security_remediation.py @@ -433,14 +433,14 @@ def test_markdown_image_alt_text_is_not_a_second_plain_reference(tmp_path: Path) @pytest.mark.parametrize( - "source_text", + ("source_text", "target"), [ - r"![Chart\](assets/chart.png)", - "![[Chart](assets/chart.png)", - r"![Chart](assets/chart.png\))", - "![Chart](assets/chart.png Color chart)", - "![Chart](assets/chart.png (Color chart))", - r'![Chart](assets/chart.png "Color \"chart\"")', + (r"![Chart\](assets/chart.png)", "assets/chart.png"), + ("![[Chart](assets/chart.png)", "assets/chart.png"), + (r"![Chart](assets/chart.png\))", "assets/chart.png)"), + ("![Chart](assets/chart.png Color chart)", "assets/chart.png"), + ("![Chart](assets/chart.png (Color chart))", "assets/chart.png"), + (r'![Chart](assets/chart.png "Color \"chart\"")', "assets/chart.png"), ], ids=[ "escaped-label-delimiter", @@ -452,16 +452,16 @@ def test_markdown_image_alt_text_is_not_a_second_plain_reference(tmp_path: Path) ], ) def test_ambiguous_image_syntax_does_not_prove_passive_use( - tmp_path: Path, source_text: str + tmp_path: Path, source_text: str, target: str ) -> None: records = resolve_bundle_references( tmp_path, source_path="SKILL.md", source_text=source_text, - known_paths=["SKILL.md", "assets/chart.png"], + known_paths=["SKILL.md", target], ) - references = [record for record in records if record["target_path"] == "assets/chart.png"] + references = [record for record in records if record["target_path"] == target] assert references assert all(record["reference_kind"] != "markdown_image" for record in references) diff --git a/tests/test_reference_destination_kinds.py b/tests/test_reference_destination_kinds.py new file mode 100644 index 000000000..d3d4ac157 --- /dev/null +++ b/tests/test_reference_destination_kinds.py @@ -0,0 +1,232 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Complete Markdown destinations retain conservative reference-use provenance.""" + +from __future__ import annotations + +import json +import struct +import zlib +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from skillspector.cli import app +from skillspector.mcp_server import run_scan +from skillspector.references import resolve_bundle_references_with_metadata +from tests.nodes.analyzers.test_documentation_reconstruction import ( + successful_llm_transport as successful_llm_transport, +) + + +@pytest.mark.parametrize("present", [True, False]) +@pytest.mark.parametrize( + ("source", "target", "kind"), + [ + ("[Guide](docs/user%20guide.md)", "docs/user guide.md", "markdown_link"), + ("[Guide]()", "docs/user guide.md", "markdown_link"), + ("[Guide](docs/guide(v1).md)", "docs/guide(v1).md", "markdown_link"), + (r"[Guide](docs/guide\(v1\).md)", "docs/guide(v1).md", "markdown_link"), + ( + "[Guide][manual]\n\n[manual]: docs/user%20guide.md", + "docs/user guide.md", + "markdown_link", + ), + ("![Chart](assets/chart%20one.png)", "assets/chart one.png", "markdown_image"), + ("![](assets/chart%20one.png)", "assets/chart one.png", "markdown_image"), + ( + "![Chart][image]\n\n[image]: assets/chart%20one.png", + "assets/chart one.png", + "markdown_link", + ), + ("![Chart](assets/chart(v1).png)", "assets/chart(v1).png", "quoted_or_code"), + (r"![Chart](assets/chart.png\))", "assets/chart.png)", "quoted_or_code"), + ( + '![Chart](assets/chart%20one.png "see [sample](missing.md)")', + "assets/chart one.png", + "quoted_or_code", + ), + ( + r'![Chart](assets/chart%20one.png "A \"quoted\" title")', + "assets/chart one.png", + "quoted_or_code", + ), + ], +) +def test_complete_destinations_preserve_reference_kind( + tmp_path: Path, source: str, target: str, kind: str, present: bool +) -> None: + result = resolve_bundle_references_with_metadata( + tmp_path, + source_path="SKILL.md", + source_text=source, + known_paths=["SKILL.md", target] if present else ["SKILL.md"], + ) + + assert result.complete is True + assert len(result.records) == 1 + record = result.records[0] + assert record["target_path"] == (target if present else None) + assert record["status"] == ("resolved" if present else "missing") + assert record["reference_kind"] == kind + + +@pytest.mark.parametrize( + ("source", "kind"), + [ + ("`![Chart](assets/chart%20one.png)`", "quoted_or_code"), + ("```markdown\n![Chart](assets/chart%20one.png)\n```", "quoted_or_code"), + (" ![Chart](assets/chart%20one.png)", "quoted_or_code"), + ("", "quoted_or_code"), + (r"\![Chart](assets/chart%20one.png)", "plain_path"), + ], +) +def test_encoded_images_in_literal_context_remain_non_passive( + tmp_path: Path, source: str, kind: str +) -> None: + result = resolve_bundle_references_with_metadata( + tmp_path, + source_path="SKILL.md", + source_text=source + "\n\n![Rendered](assets/rendered%20chart.png)", + known_paths=["SKILL.md", "assets/chart one.png", "assets/rendered chart.png"], + ) + + assert result.complete is True + assert len(result.records) == 2 + assert all(record["status"] == "resolved" for record in result.records) + assert {(record["target_path"], record["reference_kind"]) for record in result.records} == { + ("assets/chart one.png", kind), + ("assets/rendered chart.png", "markdown_image"), + } + + +@pytest.mark.parametrize("marker", ["", "!"]) +def test_complete_destination_keeps_distinct_visible_label_reference( + tmp_path: Path, marker: str +) -> None: + result = resolve_bundle_references_with_metadata( + tmp_path, + source_path="SKILL.md", + source_text=f"{marker}[Inspect docs/extra.md](assets/chart%20one.png)", + known_paths=["SKILL.md", "docs/extra.md", "assets/chart one.png"], + ) + + assert result.complete is True + assert len(result.records) == 2 + assert {(record["target_path"], record["reference_kind"]) for record in result.records} == { + ("docs/extra.md", "plain_path"), + ("assets/chart one.png", "markdown_image" if marker else "markdown_link"), + } + + +def _png_payload() -> bytes: + def chunk(kind: bytes, content: bytes) -> bytes: + return ( + struct.pack(">I", len(content)) + + kind + + content + + struct.pack(">I", zlib.crc32(kind + content)) + ) + + return ( + b"\x89PNG\r\n\x1a\n" + + chunk(b"IHDR", struct.pack(">IIBBBBB", 1, 1, 8, 6, 0, 0, 0)) + + chunk(b"IDAT", zlib.compress(b"\x00\x40\x80\xc0\xff")) + + chunk(b"IEND", b"") + ) + + +@pytest.mark.parametrize("channel", ["cli", "mcp"]) +@pytest.mark.parametrize( + ("marker", "present", "use_llm"), + [ + ("!", True, False), + ("!", True, True), + ("", True, False), + ("", True, True), + ("!", False, False), + ("!", False, True), + ], + ids=[ + "passive-image-no-llm", + "passive-image-successful-mocked-llm", + "ordinary-link-no-llm", + "ordinary-link-successful-mocked-llm", + "missing-image-no-llm", + "missing-image-successful-mocked-llm", + ], +) +async def test_encoded_destination_reporting_preserves_opaque_coverage( + tmp_path: Path, + successful_llm_transport: list[str], + channel: str, + use_llm: bool, + marker: str, + present: bool, +) -> None: + (tmp_path / "SKILL.md").write_text( + "---\nname: chart-guide\ndescription: Explain the chart colors.\n---\n" + f"# Chart guide\n\n{marker}[Chart](assets/chart%20one.png)\n", + encoding="utf-8", + ) + target = "assets/chart one.png" + if present: + (tmp_path / "assets").mkdir() + (tmp_path / target).write_bytes(_png_payload()) + + if channel == "cli": + args = ["scan", str(tmp_path), "--format", "json", "--fail-on-incomplete"] + if not use_llm: + args.append("--no-llm") + result = CliRunner().invoke(app, args) + assert result.exit_code == 1, result.output + report = json.loads(result.stdout) + else: + verdict = await run_scan(str(tmp_path), use_llm=use_llm, output_format="json") + assert verdict["safe_to_install"] is (not present) + assert verdict["llm_used"] is use_llm + report = json.loads(verdict["report"]) + + assert report["execution_successful"] is True + completeness = report["analysis_completeness"] + assert completeness["is_complete"] is False + assert len(completeness["references"]) == 1 + reference = completeness["references"][0] + assert reference["status"] == ("resolved" if present else "missing") + assert reference["target_path"] == (target if present else None) + assert reference["reference_kind"] == ("markdown_image" if marker else "markdown_link") + assert any(issue["id"] == "AE1" for issue in report["issues"]) is (present and not marker) + if marker: + assert report["risk_assessment"]["recommendation"] == "CAUTION" + if present: + assert completeness["entirely_uninspected_files"] == 1 + assert any(item["path"] == target for item in completeness["ledger_exceptions"]) + if use_llm: + # AE1 concerns bytes excluded from the LLM cache. Its locally retained + # meta finding keeps the existing conservative runtime caveat, even + # though all three semantic analyzers below completed successfully. + if marker: + assert report["metadata"].get("llm_degraded", False) is False + else: + assert report["metadata"]["llm_degraded"] is True + assert "semantic runtime telemetry was incomplete" in report["metadata"]["llm_error"] + assert report["metadata"]["llm_calls_succeeded"] >= 3 + assert ( + report["metadata"]["llm_calls_succeeded"] == report["metadata"]["llm_calls_attempted"] + ) + else: + assert successful_llm_transport == [] + semantic_statuses = { + item["analyzer_id"]: item["status"] + for item in completeness["analyzer_statuses"] + if item["analyzer_id"] + in { + "semantic_security_discovery", + "semantic_developer_intent", + "semantic_quality_policy", + } + } + assert len(semantic_statuses) == 3 + assert set(semantic_statuses.values()) == {"completed" if use_llm else "disabled"}