Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 64 additions & 2 deletions src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"(?<![^\s;&|()<>])#")
_PERL_LITERAL_PRINT_RE = re.compile(
r"^[ \t]*+print\b(?:[ \t]++(?:STDOUT|STDERR)\b)?[ \t]*+(?P<paren>\()?[ \t]*+"
r"(?P<literal>\"(?:\\[\\\"'nrt]|[^\\\"$@`\r\n])*+\""
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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] = {}
Expand All @@ -1734,24 +1754,34 @@ 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:
Comment thread
rng1995 marked this conversation as resolved.
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 "'\""
Comment thread
rng1995 marked this conversation as resolved.
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,
parameter_end_cache,
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 = (
Expand Down Expand Up @@ -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.
Expand All @@ -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:
Expand Down Expand Up @@ -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],
Expand Down
150 changes: 150 additions & 0 deletions tests/nodes/analyzers/test_shell_assignment_quotes.py
Original file line number Diff line number Diff line change
@@ -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"])
Loading