diff --git a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py index eb7fc60d5..e5240a58e 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py +++ b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py @@ -66,6 +66,9 @@ _RUNTIME_SHELL_PARAMETER_SENTINEL = "\ue002" _SIMPLE_BRACED_PARAMETER_RE = re.compile(r"\$\{(?:[A-Za-z_][A-Za-z0-9_]*|[0-9]+|[@*#?$!-])\}") _SHELL_DELIMITER_WORD_RE = re.compile(r"[^$'\"`\\(){}<>#;|&!\s]++") +# A ``#`` that starts a word begins a shell comment. Quoting is not tracked, so +# ``echo "a # b"`` also matches; callers only use this to restrict ownership. +_SHELL_COMMENT_START_RE = re.compile(r"(?])#") _PERL_LITERAL_PRINT_RE = re.compile( r"^[ \t]*+print\b(?:[ \t]++(?:STDOUT|STDERR)\b)?[ \t]*+(?P\()?[ \t]*+" r"(?P\"(?:\\[\\\"'nrt]|[^\\\"$@`\r\n])*+\"" @@ -1421,9 +1424,14 @@ def _parse_shell_command_word( backtick_end_cache: dict[int, int | None] | None = None, *, check_runtime: Callable[[], None] | None = None, + owned_word_positions: set[int] | None = None, + enclosing_delimiter: str | None = None, ) -> _ShellCommandWord | None: runtime_check = check_runtime or (lambda: None) output: list[str] = [] + wrapper_quote = enclosing_delimiter or ( + _command_wrapper_quote(content, start) if content[start] in "$`" else None + ) quote: str | None = None ansi_c_quote = False dynamic = False @@ -1437,6 +1445,8 @@ def _parse_shell_command_word( character = content[cursor] if quote is not None: if character == quote: + if owned_word_positions is not None: + owned_word_positions.add(cursor) quote = None ansi_c_quote = False elif character == "\\" and ansi_c_quote: @@ -1598,6 +1608,11 @@ def _parse_shell_command_word( cursor = parameter_end continue elif character == "`": + if enclosing_delimiter == "`": + # The enclosing substitution ends here; this is not an opener. + if owned_word_positions is not None: + owned_word_positions.add(cursor) + break substitution_end = _skip_backtick_substitution( content, cursor, @@ -1630,6 +1645,8 @@ def _parse_shell_command_word( cursor = substitution_end continue elif character in "'\"": + if character == wrapper_quote: + break quote = character elif character == "\\" and cursor + 1 < limit: if content[cursor + 1] == "\n": @@ -1726,6 +1743,9 @@ def _has_shell_command_word_exhaustion( ) -> bool: """Find candidate command words whose deterministic parse hit a safety bound.""" parsed_through = 0 + # Completed words own closing quotes, never executable expansion starts. + # Inner commands remain independent candidates; never suppress their bodies. + owned_word_positions: set[int] = set() parameter_end_cache: dict[int, _ParameterExpansionEnd] = {} substitution_end_cache: dict[int, int | None] = {} backtick_end_cache: dict[int, int | None] = {} @@ -1734,17 +1754,23 @@ def _has_shell_command_word_exhaustion( for candidate in _SHELL_COMMAND_WORD_START_RE.finditer(content): check_runtime() start = candidate.start() + if start in owned_word_positions: + continue if structural_quote_closers is not None and start in structural_quote_closers: continue json_string_start = structural_quote_openers is not None and ( start in structural_quote_openers or start - 1 in structural_quote_openers ) + assignment_quote = start > 0 and content[start - 1] == "=" and content[start] in "'\"" if start < parsed_through or ( - not json_string_start and not _is_shell_command_word_start(content, start) + not json_string_start + and not assignment_quote + and not _is_shell_command_word_start(content, start) ): continue if _has_quoted_assignment_prefix(content, start): continue + candidate_word_positions: set[int] = set() parsed = _parse_shell_command_word( content, start, @@ -1752,6 +1778,10 @@ def _has_shell_command_word_exhaustion( substitution_end_cache, backtick_end_cache, check_runtime=check_runtime, + owned_word_positions=candidate_word_positions, + enclosing_delimiter=( + _assignment_value_wrapper(content, start) if assignment_quote else None + ), ) if parsed is None: substitution_start = ( @@ -1789,6 +1819,21 @@ def _has_shell_command_word_exhaustion( if unresolved_end - start > _SHELL_COMMAND_WORD_CHARS: return True continue + # Comments end at a newline, and a quote after ``=`` in prose or host + # source may be mis-paired. Such a word cannot own a later line. + confined_word = "\n" in content[start : parsed.end] and ( + assignment_quote + or _SHELL_COMMENT_START_RE.search(content, content.rfind("\n", 0, start) + 1, start) + is not None + ) + if not confined_word: + # A quote that opens a runtime-selected word stays an independent + # candidate, so a mis-paired claim cannot hide its operands. + owned_word_positions.update( + position + for position in candidate_word_positions + if content[position + 1 : position + 2] not in ("$", "`") + ) # 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. @@ -1805,8 +1850,12 @@ def _has_shell_command_word_exhaustion( or "$" not in raw_word or not any(marker in raw_word for marker in ("$(", "`")) or simple_backtick_parameter - ): + ) and not (assignment_quote and confined_word): parsed_through = max(parsed_through, parsed.end) + if assignment_quote: + # An assignment value never names the command, and main never + # parsed this position, so it only claims ownership. + continue if parsed.limited: return True if parsed.dynamic and may_have_destructive_outer_operands: @@ -2089,6 +2138,19 @@ def _command_wrapper_quote(content: str, command_start: int) -> str | None: return content[command_start - 1] if backslashes % 2 == 0 else None +def _assignment_value_wrapper(content: str, value_start: int) -> str | None: + """Return an unescaped quote or backtick directly enclosing an assignment.""" + name_start = value_start - 1 + if name_start > 0 and content[name_start - 1] == "+": + name_start -= 1 + while name_start > 0 and ( + content[name_start - 1] == "_" + or (content[name_start - 1].isascii() and content[name_start - 1].isalnum()) + ): + name_start -= 1 + return _command_wrapper_quote(content, name_start) + + def _perl_literal_print_shell_text( content: str, check_runtime: Callable[[], None], diff --git a/tests/nodes/analyzers/test_shell_assignment_quotes.py b/tests/nodes/analyzers/test_shell_assignment_quotes.py new file mode 100644 index 000000000..ef664f9b8 --- /dev/null +++ b/tests/nodes/analyzers/test_shell_assignment_quotes.py @@ -0,0 +1,150 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +import json +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from skillspector.cli import app +from skillspector.nodes.analyzers.static_patterns_tool_misuse import has_bounded_parse_exhaustion + +_COMMENT_PADDING = "\n" + "#" * 6000 + "\n" +_APOSTROPHE_PADDING = "\n" + "y = 'b'\n" * 800 + + +@pytest.mark.parametrize( + "source", + [ + 'out="$(date)"', + 'v="a (b)"', + 'v="|$a|"', + 'printf "%s" "$(f "$x")"', + '[ "$(f)" = true ]', + 'cat <<< "$(f)"', + 'v="a \\" (b)"', + 'v="a \\" |$a|"', + ], +) +def test_complete_shell_quotes_do_not_consume_following_padding(source: str) -> None: + assert not has_bounded_parse_exhaustion(source + "\n" + "#" * 6000 + "\n", lambda: None) + + +@pytest.mark.parametrize( + "source", ['x="$(unterminated', 'x="unclosed', '"$(printf "%s" "$x")" -rf /'] +) +def test_unresolved_shell_words_still_exhaust(source: str) -> None: + assert has_bounded_parse_exhaustion(source + "\n" + "#" * 6000 + "\n", lambda: None) + + +def test_assignment_ownership_retains_nested_command_budget() -> None: + nested_word = "r" * 4097 + source = f'out="$({nested_word} -rf /)"' + assert has_bounded_parse_exhaustion(source, lambda: None) + + +def test_failed_word_parse_does_not_claim_following_command() -> None: + source = 'Test-Path "$($_.FullName)\\cli-path"; $($CMD) -rf /' + assert has_bounded_parse_exhaustion(source, lambda: None) + + +@pytest.mark.parametrize( + "source", ['Run "$(resolve_tool) -rf /"', '# see "notes\n$(resolve_tool) -rf /\n# "'] +) +def test_quoted_runtime_commands_keep_fail_closed_coverage(source: str) -> None: + from skillspector.inspection_ledger import LedgerOutcome, LedgerReason + from skillspector.nodes.analyzers import static_patterns_tool_misuse, static_runner + + assert has_bounded_parse_exhaustion(source, lambda: None) + path = "script.sh" if source.startswith("#") else "SKILL.md" + result = static_runner.run_static_patterns_with_ledger( + {"components": [path], "local_file_cache": {path: source}, "file_cache": {path: source}}, + [static_patterns_tool_misuse], + ) + assert any( + e["outcome"] == LedgerOutcome.PARTIAL + and e["reason_code"] == LedgerReason.STATIC_PARSE_LIMIT + for e in result["inspection_ledger"] + ) + + +@pytest.mark.parametrize("file_type", ["shell", "markdown", "python"]) +def test_non_shell_unclosed_assignment_remains_conservative(file_type: str) -> None: + source = 'name="value' + "\n" + "text " * 1200 + assert has_bounded_parse_exhaustion(source, lambda: None, file_type=file_type) + + +@pytest.mark.parametrize( + ("source", "file_type"), + [ + ('# use "$(cmd)" here', "shell"), + ('```bash\nout="$(date)"\n```', "markdown"), + ('x="$(printf \'%s\' "$X")"', "shell"), + ("model_count=\"$(printf '%s\\n' \"$j\" | jq -r 'length')\"", "shell"), + ], + ids=["issue-comment-form", "markdown-fence", "printf-value", "printf-pipeline-value"], +) +def test_benign_quoted_values_do_not_consume_comment_padding(source: str, file_type: str) -> None: + assert not has_bounded_parse_exhaustion( + source + _COMMENT_PADDING, lambda: None, file_type=file_type + ) + + +@pytest.mark.parametrize( + ("source", "file_type"), + [ + ('# Keep `X="${X}"` so X\'s value wins.', "python"), + ("# Keep `X='${X}'`, so X's value wins.", "python"), + ('Use `X="${X}"`. That\'s the self-reference.', "shell"), + ], + ids=["double-quoted", "single-quoted", "prose"], +) +def test_backtick_enclosed_assignment_stops_at_its_closing_backtick( + source: str, file_type: str +) -> None: + assert not has_bounded_parse_exhaustion( + source + _APOSTROPHE_PADDING, lambda: None, file_type=file_type + ) + + +@pytest.mark.parametrize( + ("source", "file_type"), + [ + ('ls # see "notes\n"$(resolve_tool)" -rf /\n# "', "shell"), + ('Set name="value in prose.\n\n```bash\n"$(resolve_tool)" -rf /\n```\n"\n', "markdown"), + ('x = f(name="value)\n"$(resolve_tool)" -rf /\n"\n', "python"), + ('Set name="value\n$T -rf /\n"\n', "markdown"), + ('name="a "$(resolve_tool)" -rf /"', "shell"), + ], + ids=[ + "trailing-comment", + "prose-assignment-before-fence", + "host-language-assignment", + "prose-assignment-parameter", + "same-line-runtime-opener", + ], +) +def test_misaligned_quote_claims_keep_runtime_commands_partial(source: str, file_type: str) -> None: + assert has_bounded_parse_exhaustion(source, lambda: None, file_type=file_type) + + +def test_issue_628_bundle_reports_complete_coverage(tmp_path: Path) -> None: + (tmp_path / "SKILL.md").write_text( + "---\nname: repro\ndescription: Minimal repro.\n---\nRun `scripts/run.sh`.\n", + encoding="utf-8", + ) + padding = "".join(f"# padding line {index} " + "." * 50 + "\n" for index in range(90)) + (tmp_path / "scripts").mkdir() + (tmp_path / "scripts" / "run.sh").write_text( + '#!/bin/bash\nout="$(date)"\necho "$out"\n' + padding, encoding="utf-8" + ) + + result = CliRunner().invoke(app, ["scan", str(tmp_path), "--format", "json", "--no-llm"]) + + # Exit 1 is the documented high-risk verdict, not a scan execution failure. + assert result.exit_code in {0, 1}, result.output + report = json.loads(result.output) + assert report["analysis_completeness"]["status"] == "complete" + assert report["analysis_completeness"]["ledger_exceptions"] == [] + assert not any(issue["id"] == "AE1" for issue in report["issues"])