diff --git a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py index 8d10c6553..4ef5bd08b 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py +++ b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py @@ -1629,6 +1629,7 @@ def _parse_shell_command_word( check_runtime: Callable[[], None] | None = None, owned_word_positions: set[int] | None = None, enclosing_delimiter: str | None = None, + quoted_expansion_starts: set[int] | None = None, ) -> _ShellCommandWord | None: runtime_check = check_runtime or (lambda: None) # The shell recognizes ``NAME=value`` before expansion and never runs the @@ -1657,6 +1658,10 @@ def reconstructs_command( dynamic = False limited = False unquoted_characters = 0 + # Expansions directly inside the current double-quoted span. They are + # reported only once that span's closing quote is proven. + pending_quoted_expansions: list[int] = [] + quote_open = start cursor = start limit = len(content) while cursor < limit: @@ -1667,6 +1672,14 @@ def reconstructs_command( if character == quote: if owned_word_positions is not None: owned_word_positions.add(cursor) + # A span across lines may pair a heredoc or prose quote with a + # later command line, so only a single-line span ends the word + # of an expansion inside it. A lone carriage return also ends + # a line in Markdown. + span = content[quote_open:cursor] + if quoted_expansion_starts is not None and "\n" not in span and "\r" not in span: + quoted_expansion_starts.update(pending_quoted_expansions) + pending_quoted_expansions.clear() quote = None ansi_c_quote = False elif character == "\\" and ansi_c_quote: @@ -1693,6 +1706,7 @@ def reconstructs_command( ) if substitution_end is None: return None + pending_quoted_expansions.append(cursor) static_value = _static_printf_substitution( content, cursor, @@ -1720,10 +1734,15 @@ def reconstructs_command( if parameter_end is not None: output.append("$PARAM") dynamic = True - cursor = parameter_end if inherited_quote_closed[0]: + # The expansion consumed the closing quote, so this + # span has no proven boundary of its own. + pending_quoted_expansions.clear() quote = None ansi_c_quote = False + else: + pending_quoted_expansions.append(cursor) + cursor = parameter_end continue elif quote == '"' and character == "`": substitution_end = _skip_backtick_substitution( @@ -1737,6 +1756,7 @@ def reconstructs_command( ) if substitution_end is None: return None + pending_quoted_expansions.append(cursor) static_value = _static_printf_substitution( content, cursor, @@ -1772,6 +1792,7 @@ def reconstructs_command( elif character == "$" and cursor + 1 < limit and content[cursor + 1] in "'\"": quote = content[cursor + 1] ansi_c_quote = quote == "'" + quote_open = cursor cursor += 2 continue elif character == "$" and cursor + 1 < limit and content[cursor + 1] == "(": @@ -1848,6 +1869,7 @@ def reconstructs_command( if character == wrapper_quote: break quote = character + quote_open = cursor elif character == "\\" and cursor + 1 < limit: if content[cursor + 1] == "\n": cursor += 2 @@ -1954,6 +1976,9 @@ def _has_shell_command_word_exhaustion( # Completed words own closing quotes, never executable expansion starts. # Inner commands remain independent candidates; never suppress their bodies. owned_word_positions: set[int] = set() + # Expansions that a parsed word placed directly inside a closed + # double-quoted span. Their own word and operands end at that quote. + quoted_expansion_starts: set[int] = set() parameter_end_cache: dict[int, _ParameterExpansionEnd] = {} substitution_end_cache: dict[int, int | None] = {} backtick_end_cache: dict[int, int | None] = {} @@ -1980,6 +2005,7 @@ def _has_shell_command_word_exhaustion( if _has_quoted_assignment_prefix(content, start): continue candidate_word_positions: set[int] = set() + candidate_quoted_expansions: set[int] = set() parsed = _parse_shell_command_word( content, start, @@ -1989,8 +2015,13 @@ def _has_shell_command_word_exhaustion( check_runtime=check_runtime, owned_word_positions=candidate_word_positions, enclosing_delimiter=( - _assignment_value_wrapper(content, start) if assignment_quote else None + _assignment_value_wrapper(content, start) + if assignment_quote + else '"' + if start in quoted_expansion_starts + else None ), + quoted_expansion_starts=candidate_quoted_expansions, ) if parsed is None: substitution_start = ( @@ -2047,6 +2078,10 @@ def _has_shell_command_word_exhaustion( for position in candidate_word_positions if content[position + 1 : position + 2] not in ("$", "`") ) + # Quote ownership follows the same rule: a discarded parse, or one + # confined to a comment or assignment, never decides where a later + # command's operands end. + quoted_expansion_starts.update(candidate_quoted_expansions) # Only executable nested substitutions retain independent command # positions. A plain dynamic data argument still owns its inner bytes; # revisiting those as commands would turn quoted printf data into code. @@ -2079,6 +2114,7 @@ def _has_shell_command_word_exhaustion( start, parsed.end, check_runtime, + enclosing_delimiter='"' if start in quoted_expansion_starts else None, ) ): comment = python_source.comment(start) if python_source is not None else None @@ -2119,6 +2155,8 @@ def _has_runtime_command_operand_exhaustion( start: int, word_end: int, check_runtime: Callable[[], None], + *, + enclosing_delimiter: str | None = None, ) -> bool: """Return whether a runtime-selected command's operands are undecidable.""" tokens, command_end, exhausted = _bounded_shell_tokens( @@ -2126,6 +2164,7 @@ def _has_runtime_command_operand_exhaustion( start, word_end, check_runtime=check_runtime, + enclosing_delimiter=enclosing_delimiter, ) if exhausted and command_end - start < _ROOT_GLOB_COMMAND_CHARS: # Earlier prose (for example "row-first") cannot supply this @@ -2647,6 +2686,7 @@ def _bounded_shell_tokens( body_start: int, *, check_runtime: Callable[[], None] | None = None, + enclosing_delimiter: str | None = None, ) -> tuple[tuple[_ShellToken, ...], int, bool]: """Return argument words from one security-view-bounded shell command.""" runtime_check = check_runtime or (lambda: None) @@ -2670,7 +2710,7 @@ def _bounded_shell_tokens( parse_limited = False cursor = body_start limit = min(len(content), body_start + _ROOT_GLOB_COMMAND_CHARS) - wrapper_quote = _command_wrapper_quote(content, command_start) + wrapper_quote = enclosing_delimiter or _command_wrapper_quote(content, command_start) def start_word() -> None: nonlocal word_started, current_is_argument, expect_redirection_target diff --git a/tests/nodes/analyzers/test_double_quoted_expansion_word.py b/tests/nodes/analyzers/test_double_quoted_expansion_word.py new file mode 100644 index 000000000..97527d963 --- /dev/null +++ b/tests/nodes/analyzers/test_double_quoted_expansion_word.py @@ -0,0 +1,215 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""An expansion inside a closed double-quoted span ends at that span's quote (#694).""" + +from __future__ import annotations + +import pytest + +from skillspector.inspection_ledger import LedgerOutcome +from skillspector.nodes.analyzers import static_patterns_tool_misuse as tm_module +from skillspector.nodes.analyzers import static_runner + +_PADDING = "\n# ordinary padding\n" * 400 + + +def _exhausted(content: str, file_type: str = "shell") -> bool: + return tm_module.has_bounded_parse_exhaustion( + content, lambda: None, file_type=file_type, complete_context=True + ) + + +@pytest.mark.parametrize( + ("content", "file_type"), + [ + ('echo "x: $(date)"\n', "shell"), + ('echo "a $(date) b $(id -u)"\n', "shell"), + ('echo "x: ${A:-$(date)}"\n', "shell"), + ('echo "x: $(echo "a b")"\n', "shell"), + ('Write-Warning "Stale path: $($cfg.command)"\n', "powershell"), + ('Write-Host "Home: $($env:HOME)"\n', "powershell"), + ('echo "Path: $($HOME)/bin"\n', "shell"), + ], + ids=[ + "command-substitution", + "two-substitutions", + "parameter-default", + "nested-quotes", + "powershell-member", + "powershell-environment", + "path-suffix", + ], +) +def test_expansion_inside_a_closed_string_is_complete(content: str, file_type: str) -> None: + assert not _exhausted(content + _PADDING, file_type) + + +@pytest.mark.parametrize( + "content", + [ + 'echo "x: `date`"\n', + 'echo "x: ${HOME}"\n', + 'echo "$(date)"\n', + 'echo "x: $(date) y"\n', + ], + ids=["backtick", "parameter", "adjacent-quote", "trailing-word"], +) +def test_forms_that_were_already_complete_stay_complete(content: str) -> None: + assert not _exhausted(content + _PADDING) + + +@pytest.mark.parametrize( + "content", + [ + 'echo "x: $($TOOL -rf /)"\n', + 'echo "x: $(echo "a b"; $TOOL -rf /)"\n', + 'echo "x: `$TOOL -rf /`"\n', + 'echo "x: $(printf %s r m) -rf /"\n', + 'x; "$CMD $(date)" -rf /\n', + 'sh -c "echo $(date); $CMD -rf /"\n', + 'echo "x: $(date)\n', + 'echo "x: $(date) y\n', + 'echo "Path: $($HOME)\\bin"\n', + ], + ids=[ + "runtime-command", + "nested-runtime-command", + "backtick-runtime-command", + "printf-reconstruction", + "quoted-runtime-command-word", + "shell-command-string", + "unclosed-string", + "unclosed-string-with-word", + "runtime-output-joined-to-an-escape", + ], +) +def test_unresolved_commands_stay_partial(content: str) -> None: + assert _exhausted(content + _PADDING) + + +@pytest.mark.parametrize( + "content", + [ + 'echo "x: $(date)"\nrm -rf /tmp/build\n', + 'echo "a $(x)" ; rm -rf ./build\n', + 'echo "a $(x)"\nrm -rf "$HOME"/cache\n', + ], + ids=["later-cleanup", "same-line-cleanup", "quoted-cleanup-path"], +) +def test_short_script_with_a_later_cleanup_stays_complete(content: str) -> None: + assert not _exhausted(content) + assert not _exhausted(content + _PADDING) + + +@pytest.mark.parametrize( + "content", + [ + '# don\'t "$(x)"\n$CMD"" -rf /\n', + 'echo \'a "$(x)\' "$(printf r)"$X -rf /\n', + 'echo "a $(x)"; $(printf r)"$X" -rf /\n', + ], + ids=["comment-quote", "single-quoted-quote", "runtime-command-after-string"], +) +def test_runtime_command_after_a_mispaired_or_closed_quote_stays_partial(content: str) -> None: + assert _exhausted(content) + + +def test_destructive_command_in_a_substitution_keeps_its_finding() -> None: + content = 'echo "x: $(rm -rf /)"\n' + _PADDING + + findings = tm_module.analyze(content, "run.sh", "shell") + + assert any(finding.rule_id == "TM1" for finding in findings) + + +def test_scan_level_helper_script_is_fully_inspected() -> None: + script = '#!/bin/sh\necho "Started: $(date)"\n' + _PADDING + + result = static_runner.run_static_patterns_with_ledger( + {"components": ["run.sh"], "file_cache": {"run.sh": script}}, [tm_module] + ) + + event = result["inspection_ledger"][0] + assert event["outcome"] is LedgerOutcome.COMPLETED + + +@pytest.mark.parametrize( + "content", + [ + '# "prefix $(date)\n$CMD"" -rf /\n', + '# "prefix $(date)\n$CMD" x"" -rf /\n', + '#!/bin/sh\n# see "notes $(date)\n$CMD"" -rf /\n', + '"\'$(x)\'"$CMD\n$CMD"" -rf /\n', + ], + ids=["failed-comment-parse", "completed-comment-parse", "after-shebang", "failed-code-parse"], +) +def test_quote_from_a_discarded_or_comment_parse_grants_no_ownership(content: str) -> None: + # The real command on the last line owns its quotes. A discarded parse, or + # one confined to a comment, must not mark that command as enclosed by a + # quote and thereby hide ``-rf /``. + assert _exhausted(content) + assert _exhausted(content + _PADDING) + + +def test_reviewer_script_is_not_fully_inspected() -> None: + script = '# "prefix $(date)\n$CMD"" -rf /\n' + + result = static_runner.run_static_patterns_with_ledger( + {"components": ["run.sh"], "file_cache": {"run.sh": script}}, [tm_module] + ) + + event = result["inspection_ledger"][0] + assert event["outcome"] is not LedgerOutcome.COMPLETED + + +@pytest.mark.parametrize( + ("content", "file_type"), + [ + ('cat < None: + # A double-quoted span across lines may pair a heredoc or prose quote with + # a later command line, so it must never end that command's operands. + assert _exhausted(content, file_type) + assert _exhausted(content + _PADDING, file_type) + + +def test_heredoc_quote_cannot_hide_a_runtime_rm_at_scan_level() -> None: + script = ( + '#!/bin/sh\nCMD="${TOOL:-rm}"\ncat <