Repository navigation
fix(analyzer): end an expansion word at its enclosing double quote #780
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,215 @@ | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| """An expansion inside a closed double-quoted span ends at that span's quote (#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 | ||
|
|
||
|
|
||
| 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"), | ||
| [ | ||
| ('echo "x: $(date)"\n', "shell"), | ||
| ('echo "a $(date) b $(id -u)"\n', "shell"), | ||
| ('echo "x: ${A:-$(date)}"\n', "shell"), | ||
| ('echo "x: $(echo "a b")"\n', "shell"), | ||
| ('Write-Warning "Stale path: $($cfg.command)"\n', "powershell"), | ||
| ('Write-Host "Home: $($env:HOME)"\n', "powershell"), | ||
| ('echo "Path: $($HOME)/bin"\n', "shell"), | ||
| ], | ||
| ids=[ | ||
| "command-substitution", | ||
| "two-substitutions", | ||
| "parameter-default", | ||
| "nested-quotes", | ||
| "powershell-member", | ||
| "powershell-environment", | ||
| "path-suffix", | ||
| ], | ||
| ) | ||
| def test_expansion_inside_a_closed_string_is_complete(content: str, file_type: str) -> None: | ||
| assert not _exhausted(content + _PADDING, file_type) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "content", | ||
| [ | ||
| 'echo "x: `date`"\n', | ||
| 'echo "x: ${HOME}"\n', | ||
| 'echo "$(date)"\n', | ||
| 'echo "x: $(date) y"\n', | ||
| ], | ||
| ids=["backtick", "parameter", "adjacent-quote", "trailing-word"], | ||
| ) | ||
| def test_forms_that_were_already_complete_stay_complete(content: str) -> None: | ||
| assert not _exhausted(content + _PADDING) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "content", | ||
| [ | ||
| 'echo "x: $($TOOL -rf /)"\n', | ||
| 'echo "x: $(echo "a b"; $TOOL -rf /)"\n', | ||
| 'echo "x: `$TOOL -rf /`"\n', | ||
| 'echo "x: $(printf %s r m) -rf /"\n', | ||
| 'x; "$CMD $(date)" -rf /\n', | ||
| 'sh -c "echo $(date); $CMD -rf /"\n', | ||
| 'echo "x: $(date)\n', | ||
| 'echo "x: $(date) y\n', | ||
| 'echo "Path: $($HOME)\\bin"\n', | ||
| ], | ||
| ids=[ | ||
| "runtime-command", | ||
| "nested-runtime-command", | ||
| "backtick-runtime-command", | ||
| "printf-reconstruction", | ||
| "quoted-runtime-command-word", | ||
| "shell-command-string", | ||
| "unclosed-string", | ||
| "unclosed-string-with-word", | ||
| "runtime-output-joined-to-an-escape", | ||
| ], | ||
| ) | ||
| def test_unresolved_commands_stay_partial(content: str) -> None: | ||
| assert _exhausted(content + _PADDING) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "content", | ||
| [ | ||
| 'echo "x: $(date)"\nrm -rf /tmp/build\n', | ||
| 'echo "a $(x)" ; rm -rf ./build\n', | ||
| 'echo "a $(x)"\nrm -rf "$HOME"/cache\n', | ||
| ], | ||
| ids=["later-cleanup", "same-line-cleanup", "quoted-cleanup-path"], | ||
| ) | ||
| def test_short_script_with_a_later_cleanup_stays_complete(content: str) -> None: | ||
| assert not _exhausted(content) | ||
| assert not _exhausted(content + _PADDING) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "content", | ||
| [ | ||
| '# don\'t "$(x)"\n$CMD"" -rf /\n', | ||
| 'echo \'a "$(x)\' "$(printf r)"$X -rf /\n', | ||
| 'echo "a $(x)"; $(printf r)"$X" -rf /\n', | ||
| ], | ||
| ids=["comment-quote", "single-quoted-quote", "runtime-command-after-string"], | ||
| ) | ||
| def test_runtime_command_after_a_mispaired_or_closed_quote_stays_partial(content: str) -> None: | ||
| assert _exhausted(content) | ||
|
|
||
|
|
||
| def test_destructive_command_in_a_substitution_keeps_its_finding() -> None: | ||
| content = 'echo "x: $(rm -rf /)"\n' + _PADDING | ||
|
|
||
| findings = tm_module.analyze(content, "run.sh", "shell") | ||
|
|
||
| assert any(finding.rule_id == "TM1" for finding in findings) | ||
|
|
||
|
|
||
| def test_scan_level_helper_script_is_fully_inspected() -> None: | ||
| script = '#!/bin/sh\necho "Started: $(date)"\n' + _PADDING | ||
|
|
||
| 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 LedgerOutcome.COMPLETED | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "content", | ||
| [ | ||
| '# "prefix $(date)\n$CMD"" -rf /\n', | ||
| '# "prefix $(date)\n$CMD" x"" -rf /\n', | ||
| '#!/bin/sh\n# see "notes $(date)\n$CMD"" -rf /\n', | ||
| '"\'$(x)\'"$CMD\n$CMD"" -rf /\n', | ||
| ], | ||
| ids=["failed-comment-parse", "completed-comment-parse", "after-shebang", "failed-code-parse"], | ||
| ) | ||
| def test_quote_from_a_discarded_or_comment_parse_grants_no_ownership(content: str) -> None: | ||
| # The real command on the last line owns its quotes. A discarded parse, or | ||
| # one confined to a comment, must not mark that command as enclosed by a | ||
| # quote and thereby hide ``-rf /``. | ||
| assert _exhausted(content) | ||
| assert _exhausted(content + _PADDING) | ||
|
|
||
|
|
||
| def test_reviewer_script_is_not_fully_inspected() -> None: | ||
| script = '# "prefix $(date)\n$CMD"" -rf /\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 | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("content", "file_type"), | ||
| [ | ||
| ('cat <<E\n"a $(date)\nE\n$CMD"" -rf /\n', "shell"), | ||
| ('```sh\n# "a $(date)\n$CMD"" -rf /\n```\n', "markdown"), | ||
| ('Run "a $(date)\n```sh\n$CMD"" -rf /\n```\n', "markdown"), | ||
| # A later quote completes the mis-paired parse instead of failing it. | ||
| ('cat <<E\n"a $(date)\nE\n$CMD"" -rf / # "\n', "shell"), | ||
| ('cat <<\'E\'\n"a $(date)\nE\n$CMD"" -rf / # "\n', "shell"), | ||
| ('cat <<E\n"a `date`\nE\n$CMD"" -rf / # "\n', "shell"), | ||
| ('cat <<E\n"a $(date)\nE\n$CMD"" -rf /\necho "\n', "shell"), | ||
| ('Run "a $(date)\n```sh\n$CMD"" -rf / # "\n```\n', "markdown"), | ||
| ('Note: "a $(date)\n```bash\n$CMD"" -rf / #"\n```\n', "markdown"), | ||
| # A lone carriage return also ends a line in Markdown. | ||
| ('Run "a $(date)\r```sh\r$CMD"" -rf / # "\r```\r', "markdown"), | ||
| ('cat <<E\r"a $(date)\rE\r$CMD"" -rf / # "\r', "shell"), | ||
| ], | ||
| ids=[ | ||
| "heredoc-body", | ||
| "markdown-fence-comment", | ||
| "markdown-prose-quote", | ||
| "heredoc-body-closed-by-comment", | ||
| "quoted-heredoc-body-closed-by-comment", | ||
| "heredoc-backtick-closed-by-comment", | ||
| "heredoc-body-closed-by-later-line", | ||
| "markdown-prose-closed-by-comment", | ||
| "markdown-note-closed-by-comment", | ||
| "markdown-carriage-return-lines", | ||
| "heredoc-carriage-return-lines", | ||
| ], | ||
| ) | ||
| def test_cross_line_quote_pairing_stays_partial(content: str, file_type: str) -> None: | ||
| # A double-quoted span across lines may pair a heredoc or prose quote with | ||
| # a later command line, so it must never end that command's operands. | ||
| 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\nBuild "$(date)\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 |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we keep these positions local to the candidate and commit them only after the full word parses successfully and passes the comment/assignment checks? A referenced
run.shcontaining:leaves
$CMDin the shared set when the parse starting in the comment later returnsNone. Its independent scan then treats the first quote as an enclosing delimiter and skips the destructive operands, even though the empty quotes belong to the real command (CMDcould berm). In both source and fresh-wheel scans, BASE retainsstatic_parse_limitand AE1 withsafe_to_install=false; HEAD reports complete inspection, drops AE1, and returnssafe_to_install=true. Please discard quote ownership from failed/comment parses and cover this concatenated-quote case in a regression test.