From 3cd031b0f4fb7d0589ca538d57225a1036ac7593 Mon Sep 17 00:00:00 2001 From: elliottwaves-20 Date: Tue, 6 Oct 2026 22:48:07 +0200 Subject: [PATCH] fix(analyzer): treat a command wrapper with no command as complete A clause such as `| timeout |` in a Markdown parameter table, or `signal.alarm(timeout)` in host code, makes `_command_string_from_clause` see the wrapper `timeout` (also `sudo`, `nice`, `xargs`) followed directly by a control operator. Those branches returned "unresolved command string", so the file was recorded as `static_parse_limit`. The `env`, `command` and `nohup` branches already treat the same situation as "no command". When the wrapper's next token is `;`, `|`, `)` or `&` (but not `&>`), no other command can run, so report no command string. A redirection, a `(`, `&>` and the end of the view stay unresolved. Refs #694 Co-Authored-By: Claude Opus 5.5 Signed-off-by: elliottwaves-20 --- .../analyzers/static_patterns_tool_misuse.py | 26 +++- .../test_command_wrapper_without_command.py | 138 ++++++++++++++++++ 2 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 tests/nodes/analyzers/test_command_wrapper_without_command.py diff --git a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py index 183288cd5..7303586f4 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py +++ b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py @@ -2276,8 +2276,10 @@ def resolved_command_string(word: str | None, limited: bool) -> str | None: wrapper_seen = True while True: option, limited = next_word() - if limited or option is None: + if limited: return True, None + if option is None: + return not _ends_wrapped_clause(content, cursor), None if option == "--": pending, limited = next_word() if limited: @@ -2309,8 +2311,10 @@ def resolved_command_string(word: str | None, limited: bool) -> str | None: wrapper_seen = True while True: option, limited = next_word() - if limited or option is None: + if limited: return True, None + if option is None: + return not _ends_wrapped_clause(content, cursor), None if option in {"-k", "--kill-after", "-s", "--signal"}: _, limited = next_word() if limited: @@ -2327,6 +2331,24 @@ def resolved_command_string(word: str | None, limited: bool) -> str | None: return wrapper_seen, None +def _ends_wrapped_clause(content: str, cursor: int) -> bool: + """Return whether a wrapper's missing command is a proven end of its clause. + + When a control operator follows ``sudo``, ``nice``, ``xargs`` or + ``timeout`` directly, the clause names no wrapped command, so there is no + command string to reconstruct. A shell such a wrapper starts on its own + (``sudo -s``) reads its input like ``sh`` in a pipeline, which this check + does not model either. Commands after the operator start their own + clause and are checked separately. A redirection may still precede the + wrapped command, and a fragment may continue past its end, so both + remain unresolved. + """ + if cursor >= len(content): + return False + character = content[cursor] + return character in ";|)" or (character == "&" and content[cursor + 1 : cursor + 2] != ">") + + def _command_wrapper_quote(content: str, command_start: int) -> str | None: if command_start == 0 or content[command_start - 1] not in "'\"`": return None diff --git a/tests/nodes/analyzers/test_command_wrapper_without_command.py b/tests/nodes/analyzers/test_command_wrapper_without_command.py new file mode 100644 index 000000000..b7a18d259 --- /dev/null +++ b/tests/nodes/analyzers/test_command_wrapper_without_command.py @@ -0,0 +1,138 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""A command wrapper followed directly by a control operator runs no command (#694).""" + +from __future__ import annotations + +import pytest + +from skillspector.inspection_ledger import LedgerOutcome, LedgerReason +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, *, complete_context: bool = True) -> bool: + return tm_module.has_bounded_parse_exhaustion( + content, lambda: None, file_type=file_type, complete_context=complete_context + ) + + +@pytest.mark.parametrize("wrapper", ["timeout", "sudo", "nice", "xargs"]) +def test_markdown_table_cell_naming_a_wrapper_is_complete(wrapper: str) -> None: + content = f"| Name | Description |\n|---|---|\n| {wrapper} | Max wait in seconds |\n" + + assert not _exhausted(content, "markdown") + assert not _exhausted(content + _PADDING, "markdown") + + +def test_markdown_api_parameter_table_is_complete() -> None: + content = ( + "| Name | Type | Default | Description |\n" + "|---|---|---|---|\n" + "| timeout | float | 150 | Seconds to wait |\n" + "| retries | int | 3 | Attempts |\n" + ) + + assert not _exhausted(content, "markdown") + + +@pytest.mark.parametrize( + ("content", "file_type"), + [ + ("sudo | cat\n", "shell"), + ("timeout; echo ok\n", "shell"), + ("xargs || true\n", "shell"), + ("nice & wait\n", "shell"), + ("(nice)\n", "shell"), + ("sudo -u | cat\n", "shell"), + ("sudo |& cat\n", "shell"), + ("timeout && echo ok\n", "shell"), + ("case $a in x) nice ;; esac\n", "shell"), + ("signal.alarm(timeout)\nvalue = 1\n", "python"), + ("if (timeout) { start(); }\n", "javascript"), + ], + ids=[ + "pipe", + "semicolon", + "or-list", + "background", + "subshell", + "option-without-value", + "pipe-with-stderr", + "and-list", + "case-item-end", + "python-call", + "javascript-condition", + ], +) +def test_wrapper_before_a_clause_end_is_complete(content: str, file_type: str) -> None: + assert not _exhausted(content + _PADDING, file_type) + + +@pytest.mark.parametrize( + "content", + [ + "sudo >log $CMD -rf /\n", + "sudo log $CMD -rf /\n", + "timeout ($CMD) -rf /\n", + 'x | sudo sh -c "$CMD"\n', + 'sudo |& sh -c "$CMD"\n', + 'sudo && eval "$CMD"\n', + "case $a in sudo) $CMD -rf / ;; esac\n", + 'sudo -s; eval "$CMD"\n', + ], + ids=[ + "stdout-redirection", + "stdin-redirection", + "bash-and-redirection", + "parenthesis", + "command-string", + "command-string-after-pipe", + "eval-after-and-list", + "runtime-command-in-case-item", + "eval-after-shell-option", + ], +) +def test_wrapper_with_an_unresolved_command_stays_partial(content: str) -> None: + assert _exhausted(content, "shell") + + +def test_wrapper_at_the_end_of_a_fragment_stays_partial() -> None: + assert _exhausted("x | sudo", "shell") + assert _exhausted("x | timeout", "shell", complete_context=False) + + +def test_markdown_cell_with_a_real_command_keeps_its_finding() -> None: + literal = "| Step | Command |\n|---|---|\n| wipe | `sudo rm -rf /` |\n" + runtime = "| Step | Command |\n|---|---|\n| wipe | `sudo $CMD -rf /` |\n" + + findings = tm_module.analyze(literal, "SKILL.md", "markdown") + + assert any(finding.rule_id == "TM1" for finding in findings) + assert _exhausted(runtime, "markdown") + + +def test_scan_level_parameter_table_is_fully_inspected() -> None: + content = "# Tool\n\n| Name | Type |\n|---|---|\n| timeout | float |\n" + "\nText.\n" * 400 + + result = static_runner.run_static_patterns_with_ledger( + {"components": ["SKILL.md"], "file_cache": {"SKILL.md": content}}, [tm_module] + ) + + event = result["inspection_ledger"][0] + assert event["outcome"] is LedgerOutcome.COMPLETED + + +def test_scan_level_redirected_wrapper_stays_partial() -> None: + result = static_runner.run_static_patterns_with_ledger( + {"components": ["SKILL.md"], "file_cache": {"SKILL.md": "sudo >log $CMD -rf /\n"}}, + [tm_module], + ) + + event = result["inspection_ledger"][0] + assert event["outcome"] is LedgerOutcome.PARTIAL + assert event["reason_code"] is LedgerReason.STATIC_PARSE_LIMIT