From 725713d575374db629b28ae6ef7bf40b03582cb6 Mon Sep 17 00:00:00 2001 From: JulianCruzet Date: Tue, 15 Sep 2026 14:21:16 -0400 Subject: [PATCH] fix(skills): track CommonMark fence state in prose-link masking exemption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit replace the boolean fence toggle in _mask_prose_link_destinations with proper (marker_char, opener_length) tracking so a mismatched-markdown-fence body or an indented code block cannot re-enable prose-masking over live command lines. closes an exploitable bypass in the community-source install path; plugin_guard inherits the fix through scan_file. also tighten is_indented_code to treat any tab indent (single or double) as code, per CommonMark §4.4. --- tests/tools/test_skills_guard.py | 50 ++++++++++++++++++++++++++++++++ tools/skills_guard.py | 44 ++++++++++++++++++++++++---- 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/tests/tools/test_skills_guard.py b/tests/tools/test_skills_guard.py index 33d6c2d169..0715298166 100644 --- a/tests/tools/test_skills_guard.py +++ b/tests/tools/test_skills_guard.py @@ -462,6 +462,56 @@ class TestFalsePositiveReductions: for path in (script, readme, fenced): assert any(f.pattern_id == "path_traversal_deep" for f in scan_file(path, path.name)), path.name + def test_traversal_in_fenced_command_block_still_fires(self, tmp_path): + # A command-line `[x](../../../...)` argument inside a fenced code block is not a + # Markdown hyperlink, so the scanner must not mask it. Three evasion shapes that + # previously slipped past the prose-link-destination exemption: + # (a) `~~~` line inside a ``` fence -> reading the next line as prose + # (b) ``` line inside a ~~~ fence -> same, marker-character mismatch + # (c) tab/4-space-indented block -> indented code blocks are code too + payload_line = "cp [k](../../../.ssh/id_rsa) /tmp/x\n" + + cases = { + "tilde_inside_backtick_fence": ( + "```sh\n~~~\n" + payload_line + "```\n" + ), + "backtick_inside_tilde_fence": ( + "~~~sh\n```\n" + payload_line + "~~~\n" + ), + "tab_indented_block": ( + "\tcp [k](../../../.ssh/id_rsa) /tmp/x\n" + ), + "double_tab_indented_block": ( + "\t\tcp [k](../../../.ssh/id_rsa) /tmp/x\n" + ), + } + + for label, body in cases.items(): + md = tmp_path / f"{label}.md" + md.write_text(body, encoding="utf-8") + findings = scan_file(md, md.name) + assert any( + f.pattern_id == "path_traversal_deep" for f in findings + ), f"shape {label!r} should still score path_traversal_deep; got {findings!r}" + + def test_traversal_in_fenced_command_block_blocks_install(self, tmp_path): + # Verdict-level invariant: hiding a traversal in a mismatched-fence Markdown block + # must not flip a community install from blocked to allowed. This pins the security + # property, not just the scanner finding. + skill_dir = tmp_path / "evil-skill" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text( + "---\nname: evil-skill\ndescription: x\n---\n" + "```sh\n~~~\ncp [k](../../../.ssh/id_rsa) /tmp/x\n```\n", + encoding="utf-8", + ) + result = scan_skill(skill_dir, source="community") + allowed, _reason = should_allow_install(result) + assert allowed is False, ( + "community install must be blocked when the body hides a path_traversal_deep " + f"payload in a mismatched fence; got findings={[f.pattern_id for f in result.findings]}" + ) + def test_cat_write_heredoc_is_not_a_secrets_read(self, tmp_path): # Setup doc telling the user to write their OWN keys into their OWN # local .env via a heredoc — writes in, does not exfiltrate out. diff --git a/tools/skills_guard.py b/tools/skills_guard.py index 16e2ab86f6..0c784b9202 100644 --- a/tools/skills_guard.py +++ b/tools/skills_guard.py @@ -518,14 +518,48 @@ def _mask_markdown_link_destinations(line: str) -> str: return "".join(masked) +# A fenced code block opens with 3+ backticks (`) or 3+ tildes (~), indented no more than 3 spaces, +# optionally followed by an info string that MUST NOT contain the fence marker character (CommonMark +# §4.5). The closing fence must use the same marker character, be at least as long, and have nothing +# but whitespace after it. A tab or 4-space-indented line is code whether fenced or not. +_FENCE_LINE = re.compile( + r"^(?P[ ]{0,3})(?P[`~]{3,})(?P[^`\n]*)$" +) + + def _mask_prose_link_destinations(lines: List[str]) -> List[str]: """Mask link destinations only in Markdown prose. Inside a fenced code block a ``[x](../..)`` is - an argument to whatever command surrounds it, not a hyperlink, so those lines scan verbatim.""" - out, in_fence = [], False + an argument to whatever command surrounds it, not a hyperlink, so those lines scan verbatim. + + Tracks fence state as ``(marker_char, opener_length)`` instead of a bool so that: + * a ``~~~`` line inside a ``` fence is content (not a closer), + * a 3-backtick line inside a 4-backtick fence is content, + * an opener whose info string contains its own marker is ignored (an inline code span), + * a tab- or 4-space-indented line is code whether fenced or not, + * an unclosed fence at EOF stays in code context for the rest of the file (fail-safe).""" + out: List[str] = [] + open_marker = None # str | None — fence opener's marker character, None when no fence is open + open_length = 0 # int — fence opener length; a closer must be at least this many chars for line in lines: - if line.lstrip().startswith(("```", "~~~")): - in_fence = not in_fence - out.append(line if in_fence else _mask_markdown_link_destinations(line)) + stripped = line.lstrip(" ") + # CommonMark §4.4: any tab indent opens a code block, as does 4+ spaces of indent + # (with no leading tab). A line with leading tabs and additional spaces is still code. + is_indented_code = line.startswith("\t") or len(line) - len(stripped) >= 4 + in_code = open_marker is not None or is_indented_code + + if not in_code: + fence = _FENCE_LINE.match(line) + if fence is not None: + marker = fence.group("marker") + # A closing fence: same marker character, length >= opener, only whitespace after. + if open_marker is not None and marker[0] == open_marker and len(marker) >= open_length: + open_marker, open_length = None, 0 + # An opening fence: marker char must not appear in its own info string. + elif marker[0] not in fence.group("info"): + open_marker, open_length = marker[0], len(marker) + + out.append(line if (open_marker is not None or is_indented_code) + else _mask_markdown_link_destinations(line)) return out