diff --git a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py index d38b835e1..ddf20cd28 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py +++ b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py @@ -2167,6 +2167,138 @@ def _quoted_candidate_is_complete( ) +def _shell_comment_start_before(content: str, start: int, *, ansi_c: bool = False) -> int | None: + """Return a genuine shell comment ``#`` on the candidate's line, if any. + + This scans from the start of ``content`` so quote state is inherited + across lines: a ``#`` on the candidate's line that sits inside a string + opened on an earlier line (``echo "prefix\\n # literal"; ...``) is not a + comment start, and the exemption is declined. A ``#`` that opens a + comment on an earlier line ends at that line's newline, so quotes inside + comment text never leak into later lines. When the initial quote state + cannot be proven from the source handed to the analyzer, the scan still + starts unquoted at the content boundary, which is the most the analyzer + can prove; a genuinely unprovable fragment stays conservative because + the caller only reaches this helper after the line-local comment idiom + already matched on the candidate's own line. + + With ``ansi_c`` (shell only), a ``$'...'`` string follows the same + ownership rule as the main word parser: a backslash escapes the next + character, so ``\\'`` inside the string does not close it and a ``#`` + after it is still string content, not a comment start. Other + hash-comment languages do not share Bash ANSI-C quoting (``$'`` is a + variable in Perl and Ruby), so they keep the plain single-quote rule + and never get the ANSI-C treatment. + + A ``${...}`` parameter expansion is skipped whole via the existing + bounded parser, so a ``#`` inside it (for example the default value in + ``${NOTE:- # literal}``) never reads as a comment start. When the + expansion cannot be proven (the parser returns no end), the exemption + is declined rather than guessed. + """ + line_start = content.rfind("\n", 0, start) + 1 + stack: list[str | None] = [None] + i = 0 + in_comment = False + while i < start: + ch = content[i] + if in_comment: + if ch == "\n": + in_comment = False + i += 1 + continue + top = stack[-1] + if top == "ansi": + if ch == "\\": + i += 2 + continue + if ch == "'": + stack.pop() + i += 1 + continue + if top == "'": + if ch == "'": + stack.pop() + i += 1 + continue + if top == '"': + if ch == "\\": + i += 2 + continue + if ch == '"': + stack.pop() + elif ch == "`": + stack.append("`") + elif ch == "$" and content[i + 1 : i + 2] == "(": + stack.append("sub") + i += 1 + i += 1 + continue + # Command context: the base text, a $( ) substitution, or backticks. + if ch == "\\": + i += 2 + continue + if ch == "$" and content[i + 1 : i + 2] == "{": + parameter_end = _skip_parameter_expansion(content, i, len(content), None) + if parameter_end is None: + return None + i = parameter_end + continue + if ch == "$" and ansi_c and content[i + 1 : i + 2] == "'": + stack.append("ansi") + i += 1 + elif ch in ("'", '"'): + stack.append(ch) + elif ch == "`": + if top == "`": + stack.pop() + else: + stack.append("`") + elif ch == "$" and content[i + 1 : i + 2] == "(": + stack.append("sub") + i += 1 + elif ch == ")" and top == "sub": + stack.pop() + elif ch == "#": + if i == 0 or content[i - 1] in " \t\r\n;&|()<>": + if i >= line_start: + return i + in_comment = True + i += 1 + return None + + +def _is_comment_candidate( + content: str, + start: int, + python_source: _PythonSourceOwnership | None, + *, + file_type: str | None = None, +) -> bool: + """Return whether a candidate position sits inside a ``#`` comment. + + The shell comment-start idiom is checked first so host Python source only + pays for proven token ownership when a ``#`` actually precedes the + candidate on its line. A ``#`` inside a string literal never counts: for + host source the comment token must be proven, and without host source + the candidate's line is scanned with quote state inherited from the + start of the source, so a ``#`` inside an already closed string, or + inside a string opened on an earlier line, cannot exempt a later command. + """ + line_start = content.rfind("\n", 0, start) + 1 + if _SHELL_COMMENT_START_RE.search(content, line_start, start) is None: + return False + if python_source is None: + return _shell_comment_start_before(content, start, ansi_c=file_type == "shell") is not None + return python_source.comment(start) is not None + + +# File types where ``#`` opens a comment. Markdown and plain text are +# excluded: there ``#`` starts a heading or is prose, and the conservative +# partial result for fragmented headings is deliberate. +_HASH_COMMENT_FILE_TYPES = frozenset({"shell", "python", "perl", "ruby", "powershell"}) + + def _has_shell_command_word_exhaustion( content: str, check_runtime: Callable[[], None], @@ -2176,6 +2308,7 @@ def _has_shell_command_word_exhaustion( python_source: _PythonSourceOwnership | None = None, perl_eval_blocks: bool = False, quote_limits: dict[int, int] | None = None, + file_type: str = "shell", _command_string_depth: int = 0, ) -> bool: """Find candidate command words whose deterministic parse hit a safety bound. @@ -2306,6 +2439,15 @@ def _has_shell_command_word_exhaustion( # parsed this position, so it only claims ownership. continue if parsed.limited: + if file_type in _HASH_COMMENT_FILE_TYPES and _is_comment_candidate( + content, start, python_source, file_type=file_type + ): + # A comment is never executed: a backtick there is literal + # text, so it cannot open a command substitution that + # reconstructs a printf invocation (issue #901). Unresolved + # spans and runtime-selected operands keep their existing + # conservative treatment. + continue if not _quoted_candidate_is_complete(content, start, quote_limits, check_runtime): return True if ( @@ -5046,6 +5188,41 @@ def _markdown_table_delimiter_columns( return len(cells) +def _blank_line_local_paired_backticks(line: str) -> str: + """Blank backtick runs on one line that pair as inline-code delimiters. + + Mirrors the equal-length, right-to-left pairing of the paragraph-scope + inline-code masking: a backtick that demonstrably delimits inline code on + its own line is Markdown syntax, never a shell substitution opener. Runs + that could only pair across lines stay visible, keeping the prose check + conservative, and escaped runs stay visible exactly as the masking does. + """ + runs = [(match.start(), match.end()) for match in re.finditer(r"`+", line)] + next_by_length: dict[int, int] = {} + closing: dict[int, int] = {} + for index in range(len(runs) - 1, -1, -1): + length = runs[index][1] - runs[index][0] + if length in next_by_length: + closing[index] = next_by_length[length] + next_by_length[length] = index + chars = list(line) + index = 0 + while index < len(runs): + start, end = runs[index] + escape_start = start + while escape_start > 0 and line[escape_start - 1] == "\\": + escape_start -= 1 + close_index = closing.get(index) + if (start - escape_start) % 2 or close_index is None: + index += 1 + continue + close_start, close_end = runs[close_index] + chars[start:end] = [" "] * (end - start) + chars[close_start:close_end] = [" "] * (close_end - close_start) + index = close_index + 1 + return "".join(chars) + + def _markdown_shell_text( content: str, check_runtime: Callable[[], None], @@ -5293,7 +5470,13 @@ def mask_inline_delimiters() -> None: and not runs and not heading and "'" in leading - and _MARKDOWN_PROSE_SHELL_SYNTAX_RE.search(leading) is None + # Inline-code delimiters are Markdown, not shell + # syntax: judge the line with its line-local pairs + # blanked, exactly as the masking pass will. + and _MARKDOWN_PROSE_SHELL_SYNTAX_RE.search( + _blank_line_local_paired_backticks(leading) + ) + is None and ( line_index == 0 or ( @@ -5443,6 +5626,7 @@ def _has_bounded_parse_exhaustion( if file_type == "shell" and complete_context else None ), + file_type=file_type, ): return True covered_until = 0 diff --git a/tests/nodes/analyzers/test_static_parse_limit_issue_901.py b/tests/nodes/analyzers/test_static_parse_limit_issue_901.py new file mode 100644 index 000000000..508dc4fdd --- /dev/null +++ b/tests/nodes/analyzers/test_static_parse_limit_issue_901.py @@ -0,0 +1,207 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Ordinary quotes and backticks must not read as ``static_parse_limit``. + +Issue #901: two kinds of everyday text made ``static_patterns_tool_misuse`` +report ``static_parse_limit``, so the file counted as partially inspected and +the referencing file picked up an AE1 HIGH finding: + +1. A backtick-quoted word inside a shell comment, as in + ``# Uses the `printf` builtin.``. The shell never executes comments, so the + backticks are literal text and cannot open a command substitution. +2. An unmatched apostrophe in Markdown prose after inline code, as in + ```Run `rm -f build.log` to clean the project's output.``` + The prose reading already masks possessives; it must also apply when the + line carries inline-code delimiters. +""" + +from __future__ import annotations + +import pytest + +from skillspector.inspection_ledger import LedgerOutcome, LedgerReason +from skillspector.nodes.analyzers import static_patterns_tool_misuse as tm +from skillspector.nodes.analyzers import static_runner + +# Verbatim reproducer 1 from the issue: backticks inside a shell comment. +_COMMENT_BACKTICK_SCRIPT = "#!/usr/bin/env bash\n# Uses the `printf` builtin.\necho ok\n" + +# Verbatim reproducer 2 from the issue: inline code plus a possessive. +_INLINE_CODE_POSSESSIVE_MD = "Run `rm -f build.log` to clean the project's output.\n" + + +def test_backtick_in_shell_comment_completes_static_analysis() -> None: + assert ( + tm.has_bounded_parse_exhaustion(_COMMENT_BACKTICK_SCRIPT, lambda: None, file_type="shell") + is False + ) + + +def test_backtick_in_python_comment_completes_static_analysis() -> None: + source = "# Uses the `printf` builtin.\nprint('ok')\n" + assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="python") is False + + +def test_hash_inside_shell_string_is_not_a_comment() -> None: + # A ``#`` inside quotes never opens a comment: the backtick substitution + # that follows is still parsed, keeping the conservative bound. + source = 'echo "# not a comment `printf`"\n' + assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="shell") is True + + +def test_quoted_hash_after_closed_string_is_not_a_comment() -> None: + # Review follow-up on #902: a whitespace-preceded ``#`` inside an + # already closed string is not a comment start, so the substitution + # that follows still runs and its failed printf reconstruction keeps + # the conservative bound. The earlier negative only covered a ``#`` + # directly after the opening quote, which the comment regex already + # rejected for lack of a preceding separator. + source = 'echo " # literal"; $(printf "$FMT" "$CMD") -rf /\n' + assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="shell") is True + + +def test_multiline_string_hash_is_not_a_comment() -> None: + # Review follow-up on #902 (second round): a ``#`` on the second line + # of a string opened on the first line is still inside that string, so + # the substitution that follows it still runs and its failed printf + # reconstruction keeps the conservative bound. The previous fix scanned + # only the candidate's own line, which wrongly read this hash as a + # comment start and exempted the substitution. + source = 'echo "prefix\n # literal"; $(printf "$FMT" "$CMD") -rf /\n' + assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="shell") is True + + +def test_quote_inside_comment_does_not_leak_to_next_line() -> None: + # A quote opened inside a comment ends with that comment's newline: it + # must not swallow the next line's genuine comment start. + source = '# it\'s a `printf` note\n# real; $(printf "$FMT" "$CMD") -rf /\n' + assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="shell") is False + + +def test_real_comment_before_substitution_exempts_it() -> None: + # The paired case: the same substitution after a genuine comment start + # never executes, so the failed reconstruction is exempt and the + # analysis completes. + source = '# literal; $(printf "$FMT" "$CMD") -rf /\n' + assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="shell") is False + + +def test_hash_inside_python_string_is_not_a_comment() -> None: + source = 'x = "# not a comment `printf`"\n' + assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="python") is True + + +def test_runtime_command_in_comment_keeps_conservative_result() -> None: + # A runtime-selected destructive command documented in a comment keeps + # its partial result: only the printf-reconstruction trigger is + # resolved for comments, never the operand-undecidability one. + source = "# $TOOL -rf /\nvalue = 1\n" + assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="shell") is True + + +def test_apostrophe_after_inline_code_completes_static_analysis() -> None: + assert ( + tm.has_bounded_parse_exhaustion( + _INLINE_CODE_POSSESSIVE_MD, lambda: None, file_type="markdown" + ) + is False + ) + + +def test_unpaired_backtick_keeps_prose_check_conservative() -> None: + # A backtick that cannot pair on its own line may still open a shell + # substitution, so the prose reading must not mask the apostrophe. + source = "Run `date and bob's your uncle\n" + projected = tm._markdown_shell_text(source, lambda: None) + assert "`" in projected + + +@pytest.mark.parametrize( + "line,expected", + [ + pytest.param( + "Run `rm -f build.log` to clean.", + "Run rm -f build.log to clean.", + id="paired-delimiters-blanked", + ), + pytest.param( + "Run `date and bob's your uncle", + "Run `date and bob's your uncle", + id="unpaired-tick-visible", + ), + pytest.param( + "Use ``a ` b`` here", + "Use a ` b here", + id="inner-tick-kept", + ), + ], +) +def test_line_local_backtick_pairing(line: str, expected: str) -> None: + assert tm._blank_line_local_paired_backticks(line) == expected + + +def test_ansi_c_escaped_quote_hash_is_not_a_comment() -> None: + # Review follow-up on #902 (third round): in a Bash ANSI-C string the + # escaped apostrophe does not close the string, so the ``#`` after it + # is string content, not a comment start. The substitution that follows + # the string still runs, so its failed printf reconstruction keeps the + # conservative bound. Before the fix the comment scan popped its quote + # at the escaped apostrophe, read the hash as a comment start, and + # wrongly exempted the executable substitution. + source = 'echo $\'x\\\' # literal\'; $(printf "$FMT" "$CMD") -rf /\n' + assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="shell") is True + + +def test_ansi_c_string_then_genuine_comment_exempts() -> None: + # The paired case: once the ANSI-C string genuinely closes, a later + # ``#`` on the same line is a real comment start, so the substitution + # after it never executes and the analysis completes. + source = 'echo $\'x\\\' # literal\' # real; $(printf "$FMT" "$CMD") -rf /\n' + assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="shell") is False + + +def test_ansi_c_comment_rule_is_shell_only() -> None: + # The ANSI-C ownership rule is a Bash rule: ``$'`` is a variable, not a + # quote, in Perl and Ruby, so the helper only applies it when the + # caller says shell. The same source therefore reads the hash as a + # comment start without the flag and as string content with it. + source = 'echo $\'x\\\' # literal\'; $(printf "$FMT" "$CMD") -rf /\n' + start = source.index("$(printf") + assert tm._shell_comment_start_before(source, start, ansi_c=True) is None + assert tm._shell_comment_start_before(source, start, ansi_c=False) == 11 + + +def test_parameter_expansion_hash_is_not_a_comment() -> None: + # Review follow-up on #902 (fourth round): a ``#`` inside a + # ``${...}`` default value is literal expansion text, not a comment + # start, so the substitution that follows the closing brace still + # runs and its failed printf reconstruction keeps the conservative + # bound. Before the fix the comment scan read the hash inside the + # expansion as a comment start and wrongly exempted the substitution. + source = 'echo ${NOTE:- # literal}; $(printf "$FMT" "$CMD") -rf /\n' + assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="shell") is True + + +def test_parameter_expansion_then_genuine_comment_exempts() -> None: + # The paired case: once the parameter expansion genuinely closes, a + # later ``#`` on the same line is a real comment start, so the + # substitution after it never executes and the analysis completes. + source = 'echo ${NOTE:- # literal} # real; $(printf "$FMT" "$CMD") -rf /\n' + assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="shell") is False + + +def test_issue_reproducer_scripts_reach_completed_ledger_outcome() -> None: + for path, source in ( + ("scripts/run.sh", _COMMENT_BACKTICK_SCRIPT), + ("references/guide.md", _INLINE_CODE_POSSESSIVE_MD), + ): + result = static_runner.run_static_patterns_with_ledger( + {"components": [path], "file_cache": {path: source}}, [tm] + ) + row = result["inspection_ledger"][0] + assert row["path"] == path + assert row["analyzer_id"] == "static_patterns_tool_misuse" + assert row["outcome"] is LedgerOutcome.COMPLETED + assert "reason_code" not in row + assert row.get("reason") != LedgerReason.STATIC_PARSE_LIMIT