From d43c946ace50e9e22e1e78ee2bbada002e892a05 Mon Sep 17 00:00:00 2001 From: Milad Heydari Date: Tue, 1 Sep 2026 02:02:21 +1000 Subject: [PATCH] fix(analyzer): avoid parse limits for bare shell variables Signed-off-by: Milad Heydari --- .../analyzers/static_patterns_tool_misuse.py | 48 +++++++++++-- .../analyzers/test_security_reconstruction.py | 67 +++++++++++++++++++ 2 files changed, 111 insertions(+), 4 deletions(-) diff --git a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py index 177a94c8..d6feaa1a 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py +++ b/src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py @@ -50,9 +50,13 @@ _PRINTF_STATIC_CHARS = 256 _PRINTF_STATIC_ARGUMENTS = 32 _PRINTF_STATIC_WORD_RE = re.compile(r"[-A-Za-z0-9_./*?%]{0,64}") +_PLAIN_BRACED_SHELL_PARAMETER_RE = re.compile(r"\$\{(?:[A-Za-z_][A-Za-z0-9_]*|[0-9]+|[*@#?$!-])\}") _DESTRUCTIVE_COMMAND_BASENAMES = frozenset({"rm", "del", "erase"}) _QUOTED_GLOB_SENTINEL = "\ue000" _DYNAMIC_SHELL_WORD_SENTINEL = "\ue001" +# Distinguish plain parameter expansions from dynamic forms that can synthesize +# an executable command word, such as ``${COMMAND:-printf}``. +_DYNAMIC_SHELL_PARAM_SENTINEL = "\ue002" _ROOT_GLOB_DOCUMENTATION_LINE_RE = re.compile( r"[ \t]*(?:(?:[-*+]|#{1,6})[ \t]+)?" r"(?:(?:(?:documentation|note|example)[ \t]*:[ \t]*)" @@ -691,6 +695,19 @@ def _is_ifs_expansion(content: str, start: int, end: int) -> bool: return content[start:end] in {"$IFS", "${IFS}"} +def _is_plain_parameter_expansion(content: str, start: int, end: int) -> bool: + """Return whether an expansion is a parameter name without operators.""" + return ( + content[start + 1] != "{" + or _PLAIN_BRACED_SHELL_PARAMETER_RE.fullmatch( + content, + start, + end, + ) + is not None + ) + + def _consume_printf_invocation( next_word: Callable[[], str | None], ) -> tuple[bool, bool]: @@ -702,8 +719,17 @@ def _consume_printf_invocation( if word is None: return False, False if _DYNAMIC_SHELL_WORD_SENTINEL in word: - # A runtime expansion participates in the invocation or wrapper - # command word. Its executable basename is not deterministic. + # A substitution or operator-bearing parameter expansion can + # synthesize a command name, so keep the fail-closed contract. + return True, False + if _DYNAMIC_SHELL_PARAM_SENTINEL in word: + # A word made entirely of plain parameters (a bare ``$ARGUMENTS``) + # does not statically identify an allowlisted command, so decline it + # rather than degrading the parse. A word that mixes literal text + # with a parameter (``cmd${VAR}``) may still be a wrapper whose + # basename is not deterministic. + if not word.replace(_DYNAMIC_SHELL_PARAM_SENTINEL, ""): + return False, False return True, False command = word.casefold().rsplit("/", 1)[-1] if command == "printf": @@ -855,6 +881,7 @@ def _next_shell_invocation_word( continue elif quote == '"' and character == "$": inherited_quote_closed = [False] + dynamic_sentinel = _DYNAMIC_SHELL_WORD_SENTINEL if cursor + 1 < limit and content[cursor + 1] == "(": parameter_end = _skip_command_substitution( content, @@ -877,6 +904,12 @@ def _next_shell_invocation_word( True, inherited_quote_closed, ) + if parameter_end is not None and _is_plain_parameter_expansion( + content, + cursor, + parameter_end, + ): + dynamic_sentinel = _DYNAMIC_SHELL_PARAM_SENTINEL if parameter_end is None: if cursor + 1 < limit and content[cursor + 1] in "({": return None, cursor, True @@ -884,7 +917,7 @@ def _next_shell_invocation_word( word_started = True cursor += 1 continue - output.append(_DYNAMIC_SHELL_WORD_SENTINEL) + output.append(dynamic_sentinel) word_started = True cursor = parameter_end if inherited_quote_closed[0]: @@ -932,6 +965,7 @@ def _next_shell_invocation_word( word_started = True cursor += 2 continue + dynamic_sentinel = _DYNAMIC_SHELL_WORD_SENTINEL if cursor + 1 < limit and content[cursor + 1] == "(": parameter_end = _skip_command_substitution( content, @@ -952,6 +986,12 @@ def _next_shell_invocation_word( substitution_end_cache, backtick_end_cache, ) + if parameter_end is not None and _is_plain_parameter_expansion( + content, + cursor, + parameter_end, + ): + dynamic_sentinel = _DYNAMIC_SHELL_PARAM_SENTINEL if parameter_end is None: if cursor + 1 < limit and content[cursor + 1] in "({": return None, cursor, True @@ -959,7 +999,7 @@ def _next_shell_invocation_word( word_started = True cursor += 1 continue - output.append(_DYNAMIC_SHELL_WORD_SENTINEL) + output.append(dynamic_sentinel) word_started = True cursor = parameter_end continue diff --git a/tests/nodes/analyzers/test_security_reconstruction.py b/tests/nodes/analyzers/test_security_reconstruction.py index 016a192d..8e74ec10 100644 --- a/tests/nodes/analyzers/test_security_reconstruction.py +++ b/tests/nodes/analyzers/test_security_reconstruction.py @@ -1725,6 +1725,73 @@ def test_long_non_invocation_printf_mention_is_not_partial(content: str) -> None assert result["inspection_ledger"][0]["outcome"] is LedgerOutcome.COMPLETED +@pytest.mark.parametrize( + "content", + [ + "`$ARGUMENTS`", + "$ARGUMENTS", + "${ARGUMENTS}", + "`${ARGUMENTS}`", + "`$@`", + "$( $ARGUMENTS )", + ], +) +def test_bare_variable_command_word_is_not_partial(content: str) -> None: + state = {"components": ["SKILL.md"], "file_cache": {"SKILL.md": content}} + + result = static_runner.run_static_patterns_with_ledger(state, [tm_module]) + + assert result["findings"] == [] + assert result["inspection_ledger"][0]["outcome"] is LedgerOutcome.COMPLETED + + +@pytest.mark.parametrize( + "content", + [ + "$ARGUMENTS", + "${ARGUMENTS}", + "$@$ARGUMENTS", + ], +) +def test_bare_variable_word_is_not_recognized_as_printf_invocation(content: str) -> None: + assert tm_module._printf_invocation_arguments(content) == (False, []) + + +@pytest.mark.parametrize( + "content", + [ + "cmd${VAR}", + "${VAR}cmd", + "$(command)", + "${COMMAND:-printf}", + "${!COMMAND}", + ], +) +def test_dynamic_printf_command_word_remains_inexact(content: str) -> None: + assert tm_module._printf_invocation_arguments(content) == (True, []) + + +@pytest.mark.parametrize( + "content", + [ + "$(cmd${VAR})", + '$("cmd${VAR}")', + "$(${COMMAND:-printf} rm) -rf *", + '$("${COMMAND:-printf}" rm) -rf *', + "$(${!COMMAND} rm) -rf *", + ], +) +def test_dynamic_parameter_command_word_remains_partial(content: str) -> None: + state = {"components": ["SKILL.md"], "file_cache": {"SKILL.md": content}} + + result = static_runner.run_static_patterns_with_ledger(state, [tm_module]) + + assert result["findings"] == [] + event = result["inspection_ledger"][0] + assert event["outcome"] is LedgerOutcome.PARTIAL + assert event["reason_code"] is LedgerReason.STATIC_PARSE_LIMIT + + def test_nested_env_parameter_assignments_are_scanned_linearly( monkeypatch: pytest.MonkeyPatch, ) -> None: