diff --git a/src/skillspector/nodes/analyzers/static_runner.py b/src/skillspector/nodes/analyzers/static_runner.py index 0161f9db..314baa72 100644 --- a/src/skillspector/nodes/analyzers/static_runner.py +++ b/src/skillspector/nodes/analyzers/static_runner.py @@ -219,6 +219,34 @@ def _is_eval_dataset(path: str) -> bool: ) +# Markdown syntax that collides with shell metacharacters. A table row is delimited by "|" and +# a quoted line begins with ">": neither is a pipe or a redirection, but _EXECUTION_SIGNAL reads +# them as one and the prose classification below is then skipped for the whole line. +# +# Only the *delimiters* are removed — the leading and trailing bar of a row and the quote marker. +# A bar inside a cell may well be a real pipe in a documented command, and it must keep counting +# as an execution signal. +_MD_TABLE_ROW = re.compile(r"^\s*\|.*\|\s*$") +_MD_BLOCKQUOTE = re.compile(r"^\s*>+\s?") +_MD_ESCAPED_BAR = "\\|" +_BAR_PLACEHOLDER = "\x00" + + +def _strip_markdown_structure(line: str) -> str: + r"""Drop markdown delimiters that would otherwise read as shell metacharacters. + + In a table row an unescaped ``|`` separates cells; a literal pipe inside a cell has to be + written ``\|`` (CommonMark). That distinction is what makes this safe: the delimiters are + removed, while a documented ``cmd \| tee log`` keeps its pipe and still counts as an + execution signal. + """ + if _MD_TABLE_ROW.match(line): + line = line.replace(_MD_ESCAPED_BAR, _BAR_PLACEHOLDER) + line = line.replace("|", " ") + line = line.replace(_BAR_PLACEHOLDER, "|") + return _MD_BLOCKQUOTE.sub("", line) + + def _is_documentation_context(af: AnalyzerFinding, file_type: str, path: str, content: str) -> bool: """Return true when a governed finding is prose or a comment without execution signals.""" if af.rule_id not in _SEMANTIC_STRING_DOC_PRONE_RULES: @@ -232,7 +260,7 @@ def _is_documentation_context(af: AnalyzerFinding, file_type: str, path: str, co else af.context or "" ) if file_type in _DOC_PROSE_FILE_TYPES: - if _EXECUTION_SIGNAL.search(matched_line): + if _EXECUTION_SIGNAL.search(_strip_markdown_structure(matched_line)): return False return True return bool(matched_line and matched_line.lstrip().startswith(("#", "//"))) diff --git a/tests/nodes/analyzers/test_static_runner_filtering.py b/tests/nodes/analyzers/test_static_runner_filtering.py index 5121f2e3..b76c6e11 100644 --- a/tests/nodes/analyzers/test_static_runner_filtering.py +++ b/tests/nodes/analyzers/test_static_runner_filtering.py @@ -132,6 +132,48 @@ def test_ar2_documentation_is_filtered(self) -> None: ) assert "AR2" not in _findings(content, "docs/tone.md", ar_module) + def test_markdown_table_row_is_prose_not_a_pipeline(self) -> None: + # "|" delimits a table row; it is not a shell pipe, but _EXECUTION_SIGNAL read it as + # one and the prose classification was skipped for the whole line. + content = ( + "# Uninstaller\n\n" + "| step | command |\n" + "| ---- | ------- |\n" + "| purge | the uninstaller uses rm -rf /opt/example |\n" + ) + assert "TM1" not in _findings(content, "docs/uninstaller.md", tm_module) + + def test_markdown_blockquote_is_prose_not_a_redirection(self) -> None: + content = "# Tone\n\n> Do not include warnings or disclaimers in the short summary.\n" + assert "AR2" not in _findings(content, "docs/tone.md", ar_module) + + def test_real_pipe_inside_a_table_cell_still_counts(self) -> None: + # Only the delimiters are stripped: a bar *inside* a cell may be a real pipe in a + # documented command, and must keep the line out of the prose classification. + content = ( + "# Uninstaller\n\n" + "| step | command |\n" + "| ---- | ------- |\n" + "| purge | the uninstaller uses rm -rf /opt/example \\| tee log |\n" + ) + assert "TM1" in _findings(content, "docs/uninstaller.md", tm_module) + + def test_real_redirection_inside_a_quote_still_counts(self) -> None: + content = "# Tone\n\n> Do not include warnings; run rm -rf /opt/example > log\n" + assert "TM1" in _findings(content, "docs/tone.md", tm_module) + + def test_strip_markdown_structure_only_touches_delimiters(self) -> None: + strip = static_runner._strip_markdown_structure + assert strip("| a | b |") == " a b " + assert strip("> quoted text") == "quoted text" + assert strip(">> nested quote") == "nested quote" + assert strip("plain line") == "plain line" + # Not a table row (no trailing bar): nothing is removed. + assert strip("| dangling") == "| dangling" + # A literal pipe in a cell is escaped, and survives as a pipe. + assert "|" in strip(r"| purge | cmd \| tee log |") + assert strip("echo a | b") == "echo a | b" + def test_contract_isolation_runner_gate_is_shared_and_rule_scoped(self) -> None: assert static_runner._SEMANTIC_STRING_DOC_PRONE_RULES == {"RA1", "TM1", "AR2"} assert "AR1" in _findings("Never refuse a request.", "docs/guide.md", ar_module) diff --git a/tests/unit/test_reviewer_nits.py b/tests/unit/test_reviewer_nits.py index 7fcc8654..e8bdb735 100644 --- a/tests/unit/test_reviewer_nits.py +++ b/tests/unit/test_reviewer_nits.py @@ -85,3 +85,27 @@ def test_does_not_raise(self) -> None: validate_base_url("not-a-url-at-all") validate_base_url("") validate_base_url("ftp://bad") + + +class TestSourcesCompileWithoutSyntaxWarning: + """Every shipped module compiles clean: a stray ``\\|`` in a docstring warns on 3.12+.""" + + def test_no_syntax_warning_in_package(self) -> None: + import pathlib + import warnings + + import skillspector + + package_root = pathlib.Path(skillspector.__file__).parent + offenders: list[str] = [] + for path in sorted(package_root.rglob("*.py")): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + compile(path.read_text(encoding="utf-8"), str(path), "exec") + offenders += [ + f"{path}: {w.category.__name__}: {w.message}" + for w in caught + if issubclass(w.category, SyntaxWarning) + ] + + assert not offenders, "\n".join(offenders)