From f5439dac3d2c6ac3ae55c19fbbab40ffaa4856b2 Mon Sep 17 00:00:00 2001 From: Varshith Puli Date: Wed, 28 Oct 2026 19:27:07 +0000 Subject: [PATCH 1/4] fix(static): port #902 comment and prose exemptions onto current main Port the #901 fix from fix-901-static-parse-limit-comment-prose-quotes onto upstream main (d501c0d), which refactored the limited-parse path around _shell_quoted_candidate_limits and dropped the file_type parameter from _has_shell_command_word_exhaustion. - Reintroduce file_type (keyword-only, default shell) on _has_shell_command_word_exhaustion and thread it from _has_bounded_parse_exhaustion, so the parsed.limited branch again exempts a candidate inside a proven # comment for shell, python, perl, ruby, and powershell. The exemption runs before the quote-completeness check and keeps the conservative treatment of unresolved spans and runtime-selected operands. - Carry _shell_comment_start_before unchanged in behavior: quote state is inherited from the start of the source, so a hash inside a string opened on an earlier line (review round 2) does not exempt a later substitution, and quotes inside comment text never leak past the comment newline. Host Python source still uses proven token ownership via python_source.comment. - Carry _blank_line_local_paired_backticks and wire it into the Markdown prose-apostrophe check in the new _markdown_shell_text signature, so line-local inline-code pairs no longer read as shell syntax while unpaired ticks stay visible. - Add the 15 tests from the reviewed head, adapted only by ruff format (line joining and quote style, no semantic change). Fixes #901. Signed-off-by: Varshith Puli --- .../analyzers/static_patterns_tool_misuse.py | 153 ++++++++++++++++- .../test_static_parse_limit_issue_901.py | 157 ++++++++++++++++++ 2 files changed, 309 insertions(+), 1 deletion(-) create mode 100644 tests/nodes/analyzers/test_static_parse_limit_issue_901.py diff --git a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py index d38b835e1..3acbeb70f 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py +++ b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py @@ -2167,6 +2167,105 @@ def _quoted_candidate_is_complete( ) +def _shell_comment_start_before(content: str, start: int) -> 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. + """ + 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 == "'": + 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 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, +) -> 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) 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 +2275,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 +2406,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 + ): + # 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 +5155,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 +5437,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 +5593,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..17d3548f6 --- /dev/null +++ b/tests/nodes/analyzers/test_static_parse_limit_issue_901.py @@ -0,0 +1,157 @@ +# 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_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 From 0ae0b663d5ead5892b0d8caa339c8ca5e86ddfad Mon Sep 17 00:00:00 2001 From: Varshith Puli Date: Thu, 29 Oct 2026 19:31:37 +0000 Subject: [PATCH 2/4] fix(static): honor ANSI-C escapes in shell comment scan The shell comment scan closed a single quote on every apostrophe, so in echo $'x\' # literal' it popped its quote at the escaped apostrophe and read the hash inside the string as a comment start. That wrongly exempted the executable substitution that follows the string, dropping its bounded-reconstruction warning. Track ANSI-C $'...' strings with their own stack marker, following the same ownership rule as the main word parser: a backslash escapes the next character, so \' does not close the string. The rule applies to shell only, since $' is a variable rather than a quote in Perl and Ruby; other hash-comment languages keep the plain single-quote rule. Adds three regression tests: the reviewer's ANSI-C reproducer, the paired genuine-comment case, and the shell-only gating of the helper. Signed-off-by: Varshith Puli --- .../analyzers/static_patterns_tool_misuse.py | 34 ++++++++++++++++--- .../test_static_parse_limit_issue_901.py | 31 +++++++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py index 3acbeb70f..e4af1017a 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py +++ b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py @@ -2167,7 +2167,9 @@ def _quoted_candidate_is_complete( ) -def _shell_comment_start_before(content: str, start: int) -> int | None: +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 @@ -2181,6 +2183,14 @@ def _shell_comment_start_before(content: str, start: int) -> int | None: 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. """ line_start = content.rfind("\n", 0, start) + 1 stack: list[str | None] = [None] @@ -2194,6 +2204,14 @@ def _shell_comment_start_before(content: str, start: int) -> int | None: 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() @@ -2216,7 +2234,10 @@ def _shell_comment_start_before(content: str, start: int) -> int | None: if ch == "\\": i += 2 continue - if ch in ("'", '"'): + 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 == "`": @@ -2241,6 +2262,8 @@ 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. @@ -2256,7 +2279,10 @@ def _is_comment_candidate( 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) is not None + return ( + _shell_comment_start_before(content, start, ansi_c=file_type == "shell") + is not None + ) return python_source.comment(start) is not None @@ -2407,7 +2433,7 @@ def _has_shell_command_word_exhaustion( continue if parsed.limited: if file_type in _HASH_COMMENT_FILE_TYPES and _is_comment_candidate( - content, start, python_source + 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 diff --git a/tests/nodes/analyzers/test_static_parse_limit_issue_901.py b/tests/nodes/analyzers/test_static_parse_limit_issue_901.py index 17d3548f6..4787d2af8 100644 --- a/tests/nodes/analyzers/test_static_parse_limit_issue_901.py +++ b/tests/nodes/analyzers/test_static_parse_limit_issue_901.py @@ -141,6 +141,37 @@ 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_issue_reproducer_scripts_reach_completed_ledger_outcome() -> None: for path, source in ( ("scripts/run.sh", _COMMENT_BACKTICK_SCRIPT), From d2fc8457afec2ae12fe97af5024666c2f1f36102 Mon Sep 17 00:00:00 2001 From: Varshith Puli Date: Thu, 29 Oct 2026 22:04:47 +0000 Subject: [PATCH 3/4] style: apply ruff format to the ANSI-C comment scan fix The format-check CI job failed on the previous commit: two hunks in the analyzer and the new regression tests were not ruff formatted. Formatting only, no behavior change. Signed-off-by: Varshith Puli --- .../nodes/analyzers/static_patterns_tool_misuse.py | 9 ++------- .../nodes/analyzers/test_static_parse_limit_issue_901.py | 6 +++--- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py index e4af1017a..b88ec6e4d 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py +++ b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py @@ -2167,9 +2167,7 @@ def _quoted_candidate_is_complete( ) -def _shell_comment_start_before( - content: str, start: int, *, ansi_c: bool = False -) -> int | None: +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 @@ -2279,10 +2277,7 @@ def _is_comment_candidate( 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 _shell_comment_start_before(content, start, ansi_c=file_type == "shell") is not None return python_source.comment(start) is not None diff --git a/tests/nodes/analyzers/test_static_parse_limit_issue_901.py b/tests/nodes/analyzers/test_static_parse_limit_issue_901.py index 4787d2af8..b443a3107 100644 --- a/tests/nodes/analyzers/test_static_parse_limit_issue_901.py +++ b/tests/nodes/analyzers/test_static_parse_limit_issue_901.py @@ -149,7 +149,7 @@ def test_ansi_c_escaped_quote_hash_is_not_a_comment() -> None: # 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" + source = 'echo $\'x\\\' # literal\'; $(printf "$FMT" "$CMD") -rf /\n' assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="shell") is True @@ -157,7 +157,7 @@ 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" + source = 'echo $\'x\\\' # literal\' # real; $(printf "$FMT" "$CMD") -rf /\n' assert tm.has_bounded_parse_exhaustion(source, lambda: None, file_type="shell") is False @@ -166,7 +166,7 @@ def test_ansi_c_comment_rule_is_shell_only() -> None: # 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" + 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 From 4946bfae764a7e1d693692441fc3597138417f7d Mon Sep 17 00:00:00 2001 From: Varshith Puli Date: Fri, 30 Oct 2026 19:08:36 +0000 Subject: [PATCH 4/4] fix(static): skip parameter expansions in shell comment scan The comment scan treated a hash inside a ${...} default value as a comment start, wrongly exempting the executable substitution after the closing brace (for example, echo ${NOTE:- # literal}; $(printf ...) -rf /). The scan now skips whole parameter expansions via the existing bounded parser, so only a hash outside any expansion, quote, or substitution counts as a comment start. When the expansion cannot be proven, the exemption is declined. Adds the reviewer's default-value case and the genuine comment after the closing brace as controls. Signed-off-by: Varshith Puli --- .../analyzers/static_patterns_tool_misuse.py | 12 ++++++++++++ .../test_static_parse_limit_issue_901.py | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py index b88ec6e4d..ddf20cd28 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py +++ b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py @@ -2189,6 +2189,12 @@ def _shell_comment_start_before(content: str, start: int, *, ansi_c: bool = Fals 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] @@ -2232,6 +2238,12 @@ def _shell_comment_start_before(content: str, start: int, *, ansi_c: bool = Fals 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 diff --git a/tests/nodes/analyzers/test_static_parse_limit_issue_901.py b/tests/nodes/analyzers/test_static_parse_limit_issue_901.py index b443a3107..508dc4fdd 100644 --- a/tests/nodes/analyzers/test_static_parse_limit_issue_901.py +++ b/tests/nodes/analyzers/test_static_parse_limit_issue_901.py @@ -172,6 +172,25 @@ def test_ansi_c_comment_rule_is_shell_only() -> 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),