Skip to content
Open
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
61 changes: 54 additions & 7 deletions src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,27 @@ def _is_shell_command_word_start(content: str, start: int) -> bool:
return False


def _is_shell_command_position(content: str, start: int) -> bool:
"""Return whether ``start`` opens a line or follows a command separator."""
cursor = start - 1
while cursor >= 0 and content[cursor] in " \t":
cursor -= 1
return cursor < 0 or content[cursor] in "\n\r;|&("

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inside a shadow region, a runtime command after a prefix word is still hidden: sudo $CMD"" -rf / # ", if true; then $CMD"" -rf / # " and { $CMD"" -rf / # " are each partial on main when standalone. Each becomes complete after cat <<E / he said "hi / E, and here the $ after sudo /then /{ is not a command position, so it stays complete. Please also accept positions after _SHELL_CLAUSE_PREFIX_WORDS, if/while/until, {, a backtick, and the wrappers this module models (sudo, env, command, nohup, nice, timeout, xargs, exec). Alternatively, check $-led shadowed candidates with _is_shell_command_word_start. Please add these variants to the test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8dd0d6a. Shadowed candidates that open a runtime word ($, "$, or a single backtick) keep main's word-start check, which also covers sudo -u root $CMD, env A=1 $CMD and xargs -0 $CMD. The variants are in test_quote_spanning_lines_cannot_hide_a_later_command. Details in the PR comment.



def _opens_runtime_word(content: str, start: int) -> bool:
"""Return whether ``start`` opens a ``$`` or single-backtick word, bare or in ``"``."""
cursor = start + 1 if content.startswith('"', start) else start
if content.startswith("$", cursor):
return True
# A backtick run such as a Markdown fence never opens a substitution.
return (
content.startswith("`", cursor)
and not content.startswith("``", cursor)
and (cursor == 0 or content[cursor - 1] != "`")
)


def _has_quoted_assignment_prefix(content: str, start: int) -> bool:
"""Return whether a quoted candidate starts with a shell assignment name."""
if start >= len(content) or content[start] not in "'\"":
Expand Down Expand Up @@ -1980,6 +2001,11 @@ def _has_shell_command_word_exhaustion(
which reparses no string.
"""
parsed_through = 0
# A word that spans lines may pair a heredoc, comment or prose quote with
# a later command line. Main skips the candidates inside such a word, so
# they are still checked here, but they never change what later
# candidates are skipped or owned.
shadow_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()
Expand Down Expand Up @@ -2008,6 +2034,17 @@ def _has_shell_command_word_exhaustion(
continue
if _has_quoted_assignment_prefix(content, start):
continue
shadowed = start < shadow_through
# A runtime-selected command may follow any prefix word (``sudo -u
# root``, ``then``, ``{``, ``env A=1``), so ``$``- and backtick-led
# candidates keep main's word-start check; other words must open a
# command position.
if (
shadowed
and not _opens_runtime_word(content, start)
and not _is_shell_command_position(content, start)
):
continue
candidate_word_positions: set[int] = set()
parsed = _parse_shell_command_word(
content,
Expand Down Expand Up @@ -2044,6 +2081,9 @@ def _has_shell_command_word_exhaustion(
check_runtime=check_runtime,
):
return True
if shadowed:
# The spanning word already closed this region's quotes.
continue
# An unclosed expansion or quote can consume the rest of the
# artifact. Once that unresolved span exceeds the command-word
# budget, treating it as clean would turn malformed, deeply nested
Expand All @@ -2068,7 +2108,7 @@ def _has_shell_command_word_exhaustion(
or _SHELL_COMMENT_START_RE.search(content, content.rfind("\n", 0, start) + 1, start)
is not None
)
if not confined_word:
if not confined_word and not shadowed:
# 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(
Expand All @@ -2088,12 +2128,19 @@ def _has_shell_command_word_exhaustion(
is not None
)
if (
not parsed.dynamic
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)
(
not parsed.dynamic
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)
and not shadowed
):
if python_source is None and ("\n" in raw_word or "\r" in raw_word):
shadow_through = max(shadow_through, parsed.end)
else:
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.
Expand Down
107 changes: 107 additions & 0 deletions tests/nodes/analyzers/test_multiline_word_shadow.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0

"""A word that spans lines never hides a later command line (#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
_TAIL = '\nE\n$CMD"" -rf / # "\n'


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"),
[
('# x "\n$CMD"" -rf / # "\n', "shell"),
('cat <<E\nhe said "hi' + _TAIL, "shell"),
("cat <<'E'\nhe said \"hi" + _TAIL, "shell"),
('cat <<E\n"${X}' + _TAIL, "shell"),
('cat <<E\n"${X}" "' + _TAIL, "shell"),
('cat <<E\n"$(date)$()"e"' + _TAIL, "shell"),
('cat <<E\n"`date`$()""' + _TAIL, "shell"),
('cat <<E\r"hi\rE\r$CMD"" -rf / # "\r', "shell"),
('Run "a\n```sh\n$CMD"" -rf / # "\n```\n', "markdown"),
('cat <<E\nhe said "hi\nE\nsudo $CMD"" -rf / # "\n', "shell"),
('cat <<E\nhe said "hi\nE\nsudo -u root $CMD"" -rf / # "\n', "shell"),
('cat <<E\nhe said "hi\nE\nif true; then $CMD"" -rf / # "\n', "shell"),
('cat <<E\nhe said "hi\nE\n{ $CMD"" -rf / # "\n', "shell"),
('cat <<E\nhe said "hi\nE\n! $CMD"" -rf / # "\n', "shell"),
('cat <<E\nhe said "hi\nE\nenv A=1 $CMD"" -rf / # "\n', "shell"),
('cat <<E\nhe said "hi\nE\nxargs -0 $CMD"" -rf / # "\n', "shell"),
('cat <<E\nhe said "hi\nE\nsudo "$CMD" -rf / # "\n', "shell"),
('cat <<E\nhe said "hi\nE\nsudo `echo x`"" -rf / # "\n', "shell"),
('cat <<E\nhe said "hi\nE\nthen "`echo x`" -rf / # "\n', "shell"),
],
ids=[
"comment",
"heredoc-prose-quote",
"quoted-delimiter",
"heredoc-parameter",
"heredoc-parameter-space",
"heredoc-letter-after-quote",
"heredoc-backtick",
"heredoc-carriage-return",
"markdown-prose",
"after-sudo",
"after-sudo-option",
"after-then",
"after-brace",
"after-bang",
"after-env-assignment",
"after-xargs-option",
"quoted-after-sudo",
"backtick-after-sudo",
"quoted-backtick-after-then",
],
)
def test_quote_spanning_lines_cannot_hide_a_later_command(content: str, file_type: str) -> None:
# A quote in a heredoc body, a comment or prose may pair with a quote on
# a later command line. The word between them must not hide that line.
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 <<EOF\nhe said "hi\nEOF\n'
'$CMD"" -rf / --no-preserve-root # done "\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


def test_multiline_string_with_parameter_stays_complete() -> None:
content = 'echo "line1\nline2 $HOME"\nls -la\n'
assert not _exhausted(content)
assert not _exhausted(content + _PADDING)


def test_dynamic_multiline_word_matches_main() -> None:
# Main never advanced past a dynamic ``$(`` word, so its inner commands
# were already independent candidates.
content = 'echo "$(date)\nnext"\nls\n'
assert not _exhausted(content)
assert not _exhausted(content + _PADDING)


def test_python_source_keeps_main_frontier() -> None:
content = 'x = """he said "hi\nmore"""\nprint(x)\n'
assert not _exhausted(content, "python")
assert not _exhausted(content + _PADDING, "python")
Loading