From 2080afc9ed9cbbf1886a839eaf43841cff5b1898 Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Tue, 20 Oct 2026 12:39:35 -0700 Subject: [PATCH 1/5] test: cover cloud download direction and upload controls Signed-off-by: Christopher Kevin --- .../test_cloud_download_direction.py | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 tests/nodes/analyzers/test_cloud_download_direction.py diff --git a/tests/nodes/analyzers/test_cloud_download_direction.py b/tests/nodes/analyzers/test_cloud_download_direction.py new file mode 100644 index 000000000..97484b7fb --- /dev/null +++ b/tests/nodes/analyzers/test_cloud_download_direction.py @@ -0,0 +1,174 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Only proven, non-destructive cloud downloads can lower E5 evidence.""" + +import json +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from skillspector.cli import app +from skillspector.nodes.analyzers import static_patterns_data_exfiltration as data_exfiltration +from skillspector.nodes.analyzers import static_runner + + +@pytest.mark.parametrize( + "command", + [ + "aws s3 cp s3://example-bucket/public.csv ./public.csv", + "aws s3 sync s3://example-bucket/public ./public", + "gsutil cp gs://example-bucket/public.csv ./public.csv", + "gsutil rsync gs://example-bucket/public ./public", + "aws s3 cp s3://example-bucket/public.csv public.csv", + "aws s3 cp s3://example-bucket/public.csv /tmp/public.csv", + "aws s3 cp --region us-west-2 s3://example-bucket/public.csv ./public.csv", + "aws s3 cp s3://example-bucket/public.csv ./public.csv --only-show-errors", + "aws s3 cp --profile public --no-sign-request s3://example-bucket/public.csv ./public.csv", + "aws s3 cp --recursive s3://example-bucket/public ./public", + "gsutil cp -r gs://example-bucket/public ./public", + "gsutil rsync -r gs://example-bucket/public ./public", + 'aws s3 cp "s3://example-bucket/public data.csv" "./public data.csv"', + "aws s3 cp \\\n s3://example-bucket/public.csv \\\n ./public.csv", + "aws s3 cp \\\r\n s3://example-bucket/public.csv \\\r\n ./public.csv", + " aws s3 cp s3://example-bucket/public.csv ./public.csv\n", + ], +) +def test_literal_cloud_download_does_not_report_exfiltration(command: str) -> None: + findings = data_exfiltration.analyze(command, "download.sh", "shell") + assert not any(finding.rule_id == "E5" for finding in findings) + + +@pytest.mark.parametrize( + "command", + [ + "aws s3 cp ./public.csv s3://example-bucket/public.csv", + "aws s3 sync ./public s3://example-bucket/public", + "gsutil cp ./public.csv gs://example-bucket/public.csv", + "gsutil rsync ./public gs://example-bucket/public", + "aws s3 cp - s3://example-bucket/public.csv", + "aws s3 cp s3://one-bucket/public.csv s3://another-bucket/public.csv", + "gsutil cp gs://one-bucket/public.csv gs://another-bucket/public.csv", + "aws s3 mv s3://example-bucket/public.csv ./public.csv", + "gsutil mv gs://example-bucket/public.csv ./public.csv", + "aws s3 sync --delete s3://example-bucket/public ./public", + "gsutil rsync -d gs://example-bucket/public ./public", + "aws s3 cp s3://example-bucket/public.csv -", + 'aws s3 cp "$source" "$destination"', + 'aws s3 cp s3://example-bucket/public.csv "$destination"', + "aws s3 cp s3://example-bucket/public.csv ./public.csv --unknown-option", + "aws s3 cp s3://example-bucket/public.csv ./public.csv --region", + "aws s3 cp s3://example-bucket/public.csv ./public.csv --region=us-west-2", + "aws s3 cp s3://example-bucket/public.csv ./public.csv extra.csv", + "aws s3 cp s3://example-bucket/public.csv './public.csv", + "aws s3 cp s3://example-bucket/public.csv ./public.csv; echo done", + "aws s3 cp s3://example-bucket/public.csv ./public.csv | cat", + "aws s3 cp s3://example-bucket/public.csv ./public.csv && echo done", + "aws s3 cp s3://example-bucket/public.csv ./public.csv > ./log", + "(aws s3 cp s3://example-bucket/public.csv ./public.csv)", + "{ aws s3 cp s3://example-bucket/public.csv ./public.csv; }", + 'echo "$(aws s3 cp s3://example-bucket/public.csv ./public.csv)"', + 'aws s3 cp s3://example-bucket/public.csv "./public$(echo name).csv"', + "aws s3 cp s3://example-bucket/public.csv ./public`echo name`.csv", + "AWS S3 CP s3://example-bucket/public.csv ./public.csv", + "aws s3 cp s3:///public.csv ./public.csv", + "aws s3 cp s3://example-bucket/public.csv https://example.invalid/public.csv", + "aws s3 cp s3://example-bucket/public.csv ./public.csv # trailing comment", + "aws s3 cp s3://example-bucket/public.csv " + "x" * 4200, + ], +) +def test_upload_or_unproved_direction_keeps_cloud_finding(command: str) -> None: + findings = data_exfiltration.analyze(command, "transfer.sh", "shell") + assert any(finding.rule_id == "E5" for finding in findings) + + +@pytest.mark.parametrize( + "content, expected_lines", + [ + ( + "aws s3 cp s3://example-bucket/public.csv ./public.csv\n" + "aws s3 cp ./public.csv s3://destination-bucket/public.csv\n", + [2], + ), + ( + "aws s3 cp s3://example-bucket/public.csv ./public.csv; " + "aws s3 cp ./public.csv s3://destination-bucket/public.csv", + [1, 1], + ), + ( + "aws s3 cp s3://example-bucket/public.csv - | " + "aws s3 cp - s3://destination-bucket/public.csv", + [1, 1], + ), + ( + "aws s3 cp s3://example-bucket/public.csv " + "./public$(aws s3 cp ./other.csv s3://destination-bucket/other.csv).csv", + [1, 1], + ), + ], +) +def test_each_cloud_occurrence_retains_its_own_direction_and_location( + content: str, expected_lines: list[int] +) -> None: + findings = data_exfiltration.analyze(content, "transfer.sh", "shell") + e5 = [finding for finding in findings if finding.rule_id == "E5"] + assert [finding.location.start_line for finding in e5] == expected_lines + + +def test_runner_keeps_upload_after_a_download_without_changing_source_offsets() -> None: + content = ( + "#!/bin/sh\n" + "aws s3 cp s3://example-bucket/public.csv ./public.csv\n" + "aws s3 cp ./public.csv s3://destination-bucket/public.csv\n" + ) + findings = static_runner.run_static_patterns( + {"components": ["transfer.sh"], "file_cache": {"transfer.sh": content}}, + [data_exfiltration], + ) + e5 = [finding for finding in findings if finding.rule_id == "E5"] + assert len(e5) == 1 + assert e5[0].start_line == 3 + assert e5[0].start_column == 0 + assert e5[0].matched_text == "aws s3 cp" + + +def test_real_cli_download_passes_strict_static_policy(tmp_path: Path) -> None: + (tmp_path / "SKILL.md").write_text( + "---\nname: public-data-helper\ndescription: Read a selected public data file.\n---\n" + "# Public data\nRead [the helper](download.sh).\n" + ) + (tmp_path / "download.sh").write_text( + "#!/bin/sh\naws s3 cp s3://example-bucket/public.csv ./public.csv\n" + ) + result = CliRunner().invoke( + app, + [ + "scan", + str(tmp_path), + "--no-llm", + "--format", + "json", + "--fail-on-findings", + "--fail-on-incomplete", + "--min-coverage", + "100", + ], + ) + assert result.exit_code == 0, result.output + report = json.loads(result.stdout) + assert report["execution_successful"] is True + assert report["analysis_completeness"]["is_complete"] is True + assert report["issues"] == [] From 51854064ca68276e8fbd44af678c03c62123614d Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Tue, 20 Oct 2026 12:56:24 -0700 Subject: [PATCH 2/5] fix(e5): distinguish literal cloud downloads from uploads Signed-off-by: Christopher Kevin --- README.md | 11 +- .../static_patterns_data_exfiltration.py | 115 +++++++++++++++++- .../test_cloud_download_direction.py | 69 +++++++++++ 3 files changed, 193 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 167469b7b..c1edae86b 100644 --- a/README.md +++ b/README.md @@ -556,7 +556,7 @@ SkillSpector detects **71 vulnerability patterns** across 17 categories: | AR2 | Disclaimer Suppression | HIGH | Instructions to omit warnings, disclaimers, or ethical commentary (e.g. "no disclaimers", "do not moralize") | | AR3 | Safety Policy Nullification | HIGH | Jailbreak framing that nullifies guardrails (e.g. "you have no restrictions", "ignore your guidelines", "do anything now") | -### Data Exfiltration (4 patterns) +### Data Exfiltration (5 patterns) | ID | Pattern | Severity | Description | |----|---------|----------|-------------| @@ -564,6 +564,15 @@ SkillSpector detects **71 vulnerability patterns** across 17 categories: | E2 | Env Variable Harvesting | HIGH | Enumerating, copying, or searching environment data to collect secrets | | E3 | File System Enumeration | MEDIUM | Scanning directories for sensitive files | | E4 | Context Leakage | HIGH | Transmitting conversation context externally | +| E5 | Cloud Storage Exfiltration | MEDIUM | Uploading data through cloud storage SDKs or CLIs | + +Within bounded, fully observed context, E5 distinguishes literal AWS/GCS downloads +from uploads. Simple remote-to-local `cp`, AWS `sync`, and `gsutil rsync` commands do not produce E5 +findings. Moves, deletion options, unknown flags, dynamic operands, streams and +compound shell commands retain conservative findings. This direction check does +not change SDK upload detection or the CLI's exit-code policy. Direction proofs +require the complete preceding context to fit within 4,096 characters; visible +command redefinitions or unknown shell setup also retain the original finding. ### Privilege Escalation (3 patterns) diff --git a/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py b/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py index 9ccad0da5..bb9229c1e 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py +++ b/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py @@ -18,6 +18,7 @@ from __future__ import annotations import ast +import posixpath import re import sys from collections.abc import Iterator @@ -35,10 +36,16 @@ get_context, get_context_from_lines, get_line_number, + logical_line_starts, resolve_call_name, resolve_dotted_name, ) from .pattern_defaults import PatternCategory +from .static_patterns_supply_chain import ( + _sc2_has_unproved_compound_context, + _sc2_logical_command, + _shell_tokens, +) from .static_patterns_tool_misuse import _bounded_shell_tokens logger = get_logger(__name__) @@ -162,6 +169,107 @@ (r"\.upload_blob\s*\(", 0.55), # Azure SDK ] +_CLOUD_DOWNLOAD_AWS_FLAGS = frozenset( + {"--quiet", "--only-show-errors", "--no-progress", "--no-sign-request", "--dryrun"} +) +_CLOUD_DOWNLOAD_AWS_VALUE_OPTIONS = frozenset({"--region", "--profile", "--include", "--exclude"}) +_CLOUD_DOWNLOAD_SHELL_META = re.compile(r"[;&|<>(){}\[\]*?$\x60%!#~\x00-\x1f\x7f]") +_CLOUD_DOWNLOAD_BUCKET = re.compile(r"[a-z0-9][a-z0-9.-]*") +_CLOUD_DOWNLOAD_CONTEXT_CHARS = 4_096 +_CLOUD_DOWNLOAD_PREFIX_META = re.compile(r"""[\\'"$\x60]""") +_CLOUD_COMMAND_REDEFINITION = re.compile( + r"\b(?:alias|function|source|eval|trap)\b" + r"|(?:^|[;\r\n])[ \t]*\.[ \t]+" + r"|\([ \t\r\n]*\)" + r"|\bPATH\s*\+?=" +) + + +def _is_literal_cloud_download( + content: str, + offset: int, + line_starts: tuple[int, ...], + file_path: str, + file_type: str, +) -> bool: + """Prove only a bounded, standalone, non-destructive cloud download. + + The existing shell helpers retain unknown context and cap logical commands + at 4096 characters. No line or enclosing match is rewritten: callers decide + separately for every E5 occurrence, including nested or later uploads. + """ + if file_type not in {"shell", "markdown", "text"} or file_path.lower().endswith( + (".ps1", ".psm1", ".psd1", ".bat", ".cmd") + ): + return False + if offset > _CLOUD_DOWNLOAD_CONTEXT_CHARS or _sc2_has_unproved_compound_context( + content, offset, () + ): + return False + if _CLOUD_DOWNLOAD_PREFIX_META.search(content, 0, offset) or _CLOUD_COMMAND_REDEFINITION.search( + content, 0, offset + ): + # Quotes, escapes or expansions can conceal alias/setup command names. + # Without evaluating that setup, retain the existing upload evidence. + return False + command = _sc2_logical_command(content, offset, line_starts) + if command is None or "\\" in command or _CLOUD_DOWNLOAD_SHELL_META.search(command) is not None: + return False + tokens = _shell_tokens(command) + if not tokens: + return False + if tokens[:3] in (["aws", "s3", "cp"], ["aws", "s3", "sync"]): + scheme = "s3://" + arguments = tokens[3:] + flags = _CLOUD_DOWNLOAD_AWS_FLAGS + if tokens[2] == "cp": + flags = flags | {"--recursive"} + value_options = _CLOUD_DOWNLOAD_AWS_VALUE_OPTIONS + elif tokens[:2] in (["gsutil", "cp"], ["gsutil", "rsync"]): + scheme = "gs://" + arguments = tokens[2:] + flags = frozenset({"-r"}) + value_options = frozenset() + else: + # In particular, mv and sync deletion flags do more than copy data. + return False + + operands: list[str] = [] + index = 0 + while index < len(arguments): + argument = arguments[index] + if argument in flags: + index += 1 + continue + if argument in value_options: + index += 1 + if index == len(arguments) or not arguments[index] or arguments[index].startswith("-"): + return False + elif argument.startswith("-"): + return False + else: + operands.append(argument) + index += 1 + if len(operands) != 2: + return False + source, destination = operands + if not source.startswith(scheme): + return False + bucket = source[len(scheme) :].split("/", 1)[0] + if _CLOUD_DOWNLOAD_BUCKET.fullmatch(bucket) is None: + return False + normalized_destination = posixpath.normpath(destination) + # A static local destination cannot be a cloud URI, stream or shell expansion. + # Colons remain conservative for other URL schemes and host-language paths. + return ( + bool(destination) + and destination != "-" + and ":" not in destination + and not destination.startswith("//") + and normalized_destination not in {"/dev", "/proc", "/sys"} + and not normalized_destination.startswith(("/dev/", "/proc/", "/sys/")) + ) + def _e1_shell_regions(content: str, file_type: str) -> tuple[tuple[int, int], ...]: """Identify source regions whose physical separators have shell semantics. @@ -539,9 +647,14 @@ def ctx(start: int) -> str: complete_match=match.group(0), ) ) - # E5: cloud-storage exfiltration. Example filtering is delegated to the runner. + # E5: retain uploads and uncertain directions; lower only proven downloads. + line_starts = logical_line_starts(content) for pattern, confidence in E5_PATTERNS: for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE): + if _is_literal_cloud_download( + content, match.start(), line_starts, file_path, file_type + ): + continue line_num = get_line_number(content, match.start()) findings.append( AnalyzerFinding( diff --git a/tests/nodes/analyzers/test_cloud_download_direction.py b/tests/nodes/analyzers/test_cloud_download_direction.py index 97484b7fb..94160e54f 100644 --- a/tests/nodes/analyzers/test_cloud_download_direction.py +++ b/tests/nodes/analyzers/test_cloud_download_direction.py @@ -88,6 +88,17 @@ def test_literal_cloud_download_does_not_report_exfiltration(command: str) -> No "aws s3 cp s3://example-bucket/public.csv https://example.invalid/public.csv", "aws s3 cp s3://example-bucket/public.csv ./public.csv # trailing comment", "aws s3 cp s3://example-bucket/public.csv " + "x" * 4200, + "aws s3 cp s3://example-bucket/public.csv //server/share/public.csv", + r"aws s3 cp s3://example-bucket/public.csv \\server\share\public.csv", + "aws s3 cp s3://example-bucket/public.csv /dev/stdout", + "aws s3 cp s3://example-bucket/public.csv /tmp/../dev/stdout", + "aws s3 cp s3://example-bucket/public.csv /proc/self/fd/1", + "aws s3 cp s3://example-bucket/public.csv %DEST%", + "aws s3 cp s3://example-bucket/public.csv !DEST!", + "aws s3 cp s3://example-bucket/public.csv ~/public.csv", + "(\naws s3 cp s3://example-bucket/public.csv ./public.csv\n)", + "{\naws s3 cp s3://example-bucket/public.csv ./public.csv\n}", + 'echo "$(\naws s3 cp s3://example-bucket/public.csv ./public.csv\n)"', ], ) def test_upload_or_unproved_direction_keeps_cloud_finding(command: str) -> None: @@ -145,6 +156,64 @@ def test_runner_keeps_upload_after_a_download_without_changing_source_offsets() assert e5[0].matched_text == "aws s3 cp" +@pytest.mark.parametrize( + "path, file_type", + [ + ("download.ps1", "shell"), + ("download.bat", "shell"), + ("download.cmd", "shell"), + ("helper.py", "python"), + ("helper.js", "javascript"), + ], +) +def test_other_host_language_syntax_keeps_legacy_cloud_evidence(path: str, file_type: str) -> None: + findings = data_exfiltration.analyze( + "aws s3 cp s3://example-bucket/public.csv ./public.csv", path, file_type + ) + assert any(finding.rule_id == "E5" for finding in findings) + + +@pytest.mark.parametrize( + "prefix", + [ + "alias aws=custom\n", + "alias 'aws=custom'\n", + "al\\ias aws=custom\n", + "al''ias aws=custom\n", + 'aws() { custom "$@"; }\n', + 'aws ()\n{\n custom "$@"\n}\n', + 'function aws { custom "$@"; }\n', + 'a\\ws() { custom "$@"; }\n', + "source ./plugin.sh\n", + ". ./plugin.sh\n", + "\\. ./plugin.sh\n", + "so''urce ./plugin.sh\n", + 'eval "$setup"\n', + 'trap "$setup" DEBUG\n', + "PATH=./tools\n", + ], +) +def test_visible_command_redefinition_keeps_cloud_finding(prefix: str) -> None: + findings = data_exfiltration.analyze( + prefix + "aws s3 cp s3://example-bucket/public.csv ./public.csv", + "download.sh", + "shell", + ) + assert any(finding.rule_id == "E5" for finding in findings) + + +def test_large_prefix_cannot_forget_an_earlier_alias() -> None: + content = ( + "alias aws=custom\n# " + "x" * 4500 + "\n" + "aws s3 cp s3://example-bucket/public.csv ./public.csv\n" + ) + findings = static_runner.run_static_patterns( + {"components": ["transfer.sh"], "file_cache": {"transfer.sh": content}}, + [data_exfiltration], + ) + assert any(finding.rule_id == "E5" for finding in findings) + + def test_real_cli_download_passes_strict_static_policy(tmp_path: Path) -> None: (tmp_path / "SKILL.md").write_text( "---\nname: public-data-helper\ndescription: Read a selected public data file.\n---\n" From 20799cdfe65a7525d0d8ae4087572af84d448ea5 Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Tue, 20 Oct 2026 13:21:02 -0700 Subject: [PATCH 3/5] refactor(e5): bound download indexing and simplify option parsing Signed-off-by: Christopher Kevin --- .../static_patterns_data_exfiltration.py | 29 +++++++------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py b/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py index bb9229c1e..17d5de8e8 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py +++ b/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py @@ -176,12 +176,9 @@ _CLOUD_DOWNLOAD_SHELL_META = re.compile(r"[;&|<>(){}\[\]*?$\x60%!#~\x00-\x1f\x7f]") _CLOUD_DOWNLOAD_BUCKET = re.compile(r"[a-z0-9][a-z0-9.-]*") _CLOUD_DOWNLOAD_CONTEXT_CHARS = 4_096 -_CLOUD_DOWNLOAD_PREFIX_META = re.compile(r"""[\\'"$\x60]""") -_CLOUD_COMMAND_REDEFINITION = re.compile( - r"\b(?:alias|function|source|eval|trap)\b" - r"|(?:^|[;\r\n])[ \t]*\.[ \t]+" - r"|\([ \t\r\n]*\)" - r"|\bPATH\s*\+?=" +_CLOUD_DOWNLOAD_SETUP = re.compile( + r"""[\\'"$\x60]|\b(?:alias|function|source|eval|trap)\b""" + r"|(?:^|[;\r\n])[ \t]*\.[ \t]+|\([ \t\r\n]*\)|\bPATH\s*\+?=" ) @@ -206,9 +203,7 @@ def _is_literal_cloud_download( content, offset, () ): return False - if _CLOUD_DOWNLOAD_PREFIX_META.search(content, 0, offset) or _CLOUD_COMMAND_REDEFINITION.search( - content, 0, offset - ): + if _CLOUD_DOWNLOAD_SETUP.search(content, 0, offset): # Quotes, escapes or expansions can conceal alias/setup command names. # Without evaluating that setup, retain the existing upload evidence. return False @@ -220,14 +215,14 @@ def _is_literal_cloud_download( return False if tokens[:3] in (["aws", "s3", "cp"], ["aws", "s3", "sync"]): scheme = "s3://" - arguments = tokens[3:] + arguments = iter(tokens[3:]) flags = _CLOUD_DOWNLOAD_AWS_FLAGS if tokens[2] == "cp": flags = flags | {"--recursive"} value_options = _CLOUD_DOWNLOAD_AWS_VALUE_OPTIONS elif tokens[:2] in (["gsutil", "cp"], ["gsutil", "rsync"]): scheme = "gs://" - arguments = tokens[2:] + arguments = iter(tokens[2:]) flags = frozenset({"-r"}) value_options = frozenset() else: @@ -235,21 +230,17 @@ def _is_literal_cloud_download( return False operands: list[str] = [] - index = 0 - while index < len(arguments): - argument = arguments[index] + for argument in arguments: if argument in flags: - index += 1 continue if argument in value_options: - index += 1 - if index == len(arguments) or not arguments[index] or arguments[index].startswith("-"): + value = next(arguments, "") + if not value or value.startswith("-"): return False elif argument.startswith("-"): return False else: operands.append(argument) - index += 1 if len(operands) != 2: return False source, destination = operands @@ -648,7 +639,7 @@ def ctx(start: int) -> str: ) ) # E5: retain uploads and uncertain directions; lower only proven downloads. - line_starts = logical_line_starts(content) + line_starts = logical_line_starts(content[: _CLOUD_DOWNLOAD_CONTEXT_CHARS + 1]) for pattern, confidence in E5_PATTERNS: for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE): if _is_literal_cloud_download( From 6295582db4411bdec7ff2763d98f7d54d5182757 Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Tue, 20 Oct 2026 19:52:36 -0700 Subject: [PATCH 4/5] fix(e5): retain findings after shell lookup and directory changes Signed-off-by: Christopher Kevin --- README.md | 4 +- .../static_patterns_data_exfiltration.py | 15 +++- .../test_cloud_download_direction.py | 72 +++++++++++++++++-- 3 files changed, 82 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index c1edae86b..93b80eaea 100644 --- a/README.md +++ b/README.md @@ -572,7 +572,9 @@ findings. Moves, deletion options, unknown flags, dynamic operands, streams and compound shell commands retain conservative findings. This direction check does not change SDK upload detection or the CLI's exit-code policy. Direction proofs require the complete preceding context to fit within 4,096 characters; visible -command redefinitions or unknown shell setup also retain the original finding. +command redefinitions (including command hashes) or unknown shell setup also +retain the original finding. Relative destinations after directory changes +remain unproved; literal absolute file destinations are still eligible. ### Privilege Escalation (3 patterns) diff --git a/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py b/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py index 17d5de8e8..5ce8431c0 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py +++ b/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py @@ -177,9 +177,13 @@ _CLOUD_DOWNLOAD_BUCKET = re.compile(r"[a-z0-9][a-z0-9.-]*") _CLOUD_DOWNLOAD_CONTEXT_CHARS = 4_096 _CLOUD_DOWNLOAD_SETUP = re.compile( - r"""[\\'"$\x60]|\b(?:alias|function|source|eval|trap)\b""" - r"|(?:^|[;\r\n])[ \t]*\.[ \t]+|\([ \t\r\n]*\)|\bPATH\s*\+?=" + r"""[\\'"$\x60]|\b(?:alias|function|source|eval|trap|hash)\b""" + r"|(?:^|[;\r\n])[ \t]*\.[ \t]+|\([ \t\r\n]*\)" + r"|\b(?:PATH|path|commands)\s*(?:\+?=|\[)" + r"|\b(?:read|readarray|mapfile|printf|set|typeset|declare|local|export|readonly|unset)\b" + r"[^;\r\n]*\b(?:PATH|path|commands)\b" ) +_CLOUD_DOWNLOAD_DIRECTORY_CHANGE = re.compile(r"\b(?:cd|chdir|pushd|popd)\b") def _is_literal_cloud_download( @@ -205,6 +209,7 @@ def _is_literal_cloud_download( return False if _CLOUD_DOWNLOAD_SETUP.search(content, 0, offset): # Quotes, escapes or expansions can conceal alias/setup command names. + # Hash tables and shell variables can also redirect executable lookup. # Without evaluating that setup, retain the existing upload evidence. return False command = _sc2_logical_command(content, offset, line_starts) @@ -249,6 +254,12 @@ def _is_literal_cloud_download( bucket = source[len(scheme) :].split("/", 1)[0] if _CLOUD_DOWNLOAD_BUCKET.fullmatch(bucket) is None: return False + if not destination.startswith("/") and _CLOUD_DOWNLOAD_DIRECTORY_CHANGE.search( + content, 0, offset + ): + # A relative file may resolve to a stream after cd /dev (or a directory + # stack change). Absolute destinations do not depend on this setup. + return False normalized_destination = posixpath.normpath(destination) # A static local destination cannot be a cloud URI, stream or shell expansion. # Colons remain conservative for other URL schemes and host-language paths. diff --git a/tests/nodes/analyzers/test_cloud_download_direction.py b/tests/nodes/analyzers/test_cloud_download_direction.py index 94160e54f..eb403bbda 100644 --- a/tests/nodes/analyzers/test_cloud_download_direction.py +++ b/tests/nodes/analyzers/test_cloud_download_direction.py @@ -191,6 +191,17 @@ def test_other_host_language_syntax_keeps_legacy_cloud_evidence(path: str, file_ 'eval "$setup"\n', 'trap "$setup" DEBUG\n', "PATH=./tools\n", + "hash -p ./custom-aws aws\n", + "builtin hash -p ./custom-aws aws\n", + "hash aws=./custom-aws\n", + "commands[aws]=./custom-aws\n", + "commands=(aws ./custom-aws)\n", + "path=./tools\n", + "path+=./tools\n", + "path[1]=./tools\n", + "set -A path ./tools\n", + "printf -v PATH ./tools\n", + "read PATH\n", ], ) def test_visible_command_redefinition_keeps_cloud_finding(prefix: str) -> None: @@ -202,6 +213,39 @@ def test_visible_command_redefinition_keeps_cloud_finding(prefix: str) -> None: assert any(finding.rule_id == "E5" for finding in findings) +@pytest.mark.parametrize("setup", ["cd /dev", "chdir /dev", "pushd /dev", "popd"]) +@pytest.mark.parametrize( + "transfer", + [ + "aws s3 cp s3://example-bucket/public.csv", + "aws s3 sync s3://example-bucket/public", + "gsutil cp gs://example-bucket/public.csv", + "gsutil rsync gs://example-bucket/public", + ], +) +@pytest.mark.parametrize("destination", ["stdout", "./stdout", "../dev/stdout"]) +def test_directory_changes_keep_relative_destination_unproved( + setup: str, transfer: str, destination: str +) -> None: + findings = data_exfiltration.analyze( + f"{setup}\n{transfer} {destination}\n", "download.sh", "shell" + ) + e5 = [finding for finding in findings if finding.rule_id == "E5"] + assert len(e5) == 1 + assert e5[0].location.start_line == 2 + + +@pytest.mark.parametrize( + "transfer", ["aws s3 cp s3://example-bucket/a", "gsutil cp gs://example-bucket/a"] +) +@pytest.mark.parametrize("setup", ["cd /dev", "set -eu", "read filename"]) +def test_proved_file_destination_survives_unrelated_setup(transfer: str, setup: str) -> None: + findings = data_exfiltration.analyze( + f"{setup}\n{transfer} /tmp/public.csv\n", "download.sh", "shell" + ) + assert not any(finding.rule_id == "E5" for finding in findings) + + def test_large_prefix_cannot_forget_an_earlier_alias() -> None: content = ( "alias aws=custom\n# " + "x" * 4500 + "\n" @@ -214,14 +258,28 @@ def test_large_prefix_cannot_forget_an_earlier_alias() -> None: assert any(finding.rule_id == "E5" for finding in findings) -def test_real_cli_download_passes_strict_static_policy(tmp_path: Path) -> None: +@pytest.mark.parametrize( + "commands, expected_lines", + [ + ("aws s3 cp s3://example-bucket/public.csv ./public.csv\n", []), + ("set -eu\naws s3 cp s3://example-bucket/public.csv ./public.csv\n", []), + ( + "hash -p ./custom-aws aws\naws s3 cp s3://example-bucket/public.csv ./public.csv\n", + [3], + ), + ("cd /dev\naws s3 cp s3://example-bucket/public.csv stdout\n", [3]), + ("cd /dev\ngsutil cp gs://example-bucket/public.csv stdout\n", [3]), + ("cd /dev\naws s3 cp s3://example-bucket/public.csv /tmp/public.csv\n", []), + ], +) +def test_real_cli_download_strict_static_policy( + tmp_path: Path, commands: str, expected_lines: list[int] +) -> None: (tmp_path / "SKILL.md").write_text( "---\nname: public-data-helper\ndescription: Read a selected public data file.\n---\n" "# Public data\nRead [the helper](download.sh).\n" ) - (tmp_path / "download.sh").write_text( - "#!/bin/sh\naws s3 cp s3://example-bucket/public.csv ./public.csv\n" - ) + (tmp_path / "download.sh").write_text("#!/bin/sh\n" + commands) result = CliRunner().invoke( app, [ @@ -236,8 +294,10 @@ def test_real_cli_download_passes_strict_static_policy(tmp_path: Path) -> None: "100", ], ) - assert result.exit_code == 0, result.output + assert result.exit_code == bool(expected_lines), result.output report = json.loads(result.stdout) assert report["execution_successful"] is True assert report["analysis_completeness"]["is_complete"] is True - assert report["issues"] == [] + assert [issue["id"] for issue in report["issues"]] == ["E5"] * len(expected_lines) + assert [issue["location"]["start_line"] for issue in report["issues"]] == expected_lines + assert all(issue["location"]["file"] == "download.sh" for issue in report["issues"]) From bdc072d53b6cf66e1b05406f04ed0262ca8c17bd Mon Sep 17 00:00:00 2001 From: Christopher Kevin Date: Mon, 2 Nov 2026 10:02:24 -0800 Subject: [PATCH 5/5] fix(e5): retain findings after unproved shell setup Signed-off-by: Christopher Kevin --- README.md | 8 +- .../static_patterns_data_exfiltration.py | 39 +++++--- .../test_cloud_download_direction.py | 94 ++++++++++++++++++- 3 files changed, 123 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 93b80eaea..540fd6f32 100644 --- a/README.md +++ b/README.md @@ -571,9 +571,11 @@ from uploads. Simple remote-to-local `cp`, AWS `sync`, and `gsutil rsync` comman findings. Moves, deletion options, unknown flags, dynamic operands, streams and compound shell commands retain conservative findings. This direction check does not change SDK upload detection or the CLI's exit-code policy. Direction proofs -require the complete preceding context to fit within 4,096 characters; visible -command redefinitions (including command hashes) or unknown shell setup also -retain the original finding. Relative destinations after directory changes +require the complete preceding context to fit within 4,096 characters. Only +blank/comment lines, literal `set` flags (`-e`, `-u`, `-f`, and `-o pipefail`), +and simple directory changes may precede a proved download. Other setup, +including sourcing, autoloading, variable assignments, and earlier transfers, +retains the original finding. Relative destinations after directory changes remain unproved; literal absolute file destinations are still eligible. ### Privilege Escalation (3 patterns) diff --git a/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py b/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py index 5ce8431c0..325e7d163 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py +++ b/src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py @@ -176,14 +176,9 @@ _CLOUD_DOWNLOAD_SHELL_META = re.compile(r"[;&|<>(){}\[\]*?$\x60%!#~\x00-\x1f\x7f]") _CLOUD_DOWNLOAD_BUCKET = re.compile(r"[a-z0-9][a-z0-9.-]*") _CLOUD_DOWNLOAD_CONTEXT_CHARS = 4_096 -_CLOUD_DOWNLOAD_SETUP = re.compile( - r"""[\\'"$\x60]|\b(?:alias|function|source|eval|trap|hash)\b""" - r"|(?:^|[;\r\n])[ \t]*\.[ \t]+|\([ \t\r\n]*\)" - r"|\b(?:PATH|path|commands)\s*(?:\+?=|\[)" - r"|\b(?:read|readarray|mapfile|printf|set|typeset|declare|local|export|readonly|unset)\b" - r"[^;\r\n]*\b(?:PATH|path|commands)\b" +_CLOUD_DOWNLOAD_SAFE_SET = re.compile( + r"set[ \t]+(?:-[efu]+[ \t]+)*(?:-[efu]+|-[efu]*o[ \t]+pipefail)" ) -_CLOUD_DOWNLOAD_DIRECTORY_CHANGE = re.compile(r"\b(?:cd|chdir|pushd|popd)\b") def _is_literal_cloud_download( @@ -207,10 +202,28 @@ def _is_literal_cloud_download( content, offset, () ): return False - if _CLOUD_DOWNLOAD_SETUP.search(content, 0, offset): - # Quotes, escapes or expansions can conceal alias/setup command names. - # Hash tables and shell variables can also redirect executable lookup. - # Without evaluating that setup, retain the existing upload evidence. + directory_changed = False + for line in content[:offset].splitlines(): + setup = line.strip(" \t\r") + if not setup or setup.startswith("#"): + continue + if _CLOUD_DOWNLOAD_SAFE_SET.fullmatch(setup): + continue + if "\\" in setup or _CLOUD_DOWNLOAD_SHELL_META.search(setup): + return False + setup_tokens = _shell_tokens(setup) + if setup_tokens == ["popd"] or ( + setup_tokens + and len(setup_tokens) == 2 + and setup_tokens[0] in {"cd", "chdir", "pushd"} + and setup_tokens[1] + and not setup_tokens[1].startswith("-") + ): + directory_changed = True + continue + # Unknown setup can load or replace the cloud command. Even an earlier + # download can overwrite an executable, so only the preambles above + # preserve this proof; do not enumerate unsafe command-loading forms. return False command = _sc2_logical_command(content, offset, line_starts) if command is None or "\\" in command or _CLOUD_DOWNLOAD_SHELL_META.search(command) is not None: @@ -254,9 +267,7 @@ def _is_literal_cloud_download( bucket = source[len(scheme) :].split("/", 1)[0] if _CLOUD_DOWNLOAD_BUCKET.fullmatch(bucket) is None: return False - if not destination.startswith("/") and _CLOUD_DOWNLOAD_DIRECTORY_CHANGE.search( - content, 0, offset - ): + if directory_changed and not destination.startswith("/"): # A relative file may resolve to a stream after cd /dev (or a directory # stack change). Absolute destinations do not depend on this setup. return False diff --git a/tests/nodes/analyzers/test_cloud_download_direction.py b/tests/nodes/analyzers/test_cloud_download_direction.py index eb403bbda..76cd3f814 100644 --- a/tests/nodes/analyzers/test_cloud_download_direction.py +++ b/tests/nodes/analyzers/test_cloud_download_direction.py @@ -238,7 +238,7 @@ def test_directory_changes_keep_relative_destination_unproved( @pytest.mark.parametrize( "transfer", ["aws s3 cp s3://example-bucket/a", "gsutil cp gs://example-bucket/a"] ) -@pytest.mark.parametrize("setup", ["cd /dev", "set -eu", "read filename"]) +@pytest.mark.parametrize("setup", ["cd /dev", "set -eu", "set -euo pipefail"]) def test_proved_file_destination_survives_unrelated_setup(transfer: str, setup: str) -> None: findings = data_exfiltration.analyze( f"{setup}\n{transfer} /tmp/public.csv\n", "download.sh", "shell" @@ -246,6 +246,96 @@ def test_proved_file_destination_survives_unrelated_setup(transfer: str, setup: assert not any(finding.rule_id == "E5" for finding in findings) +@pytest.mark.parametrize( + "setup", + [ + "true && . ./setup.sh", + "false || . ./setup.sh", + "if true; then . ./setup.sh; fi", + "for name in aws; do . ./setup.sh; done", + "builtin . ./setup.sh", + "command . ./setup.sh", + "autoload -Uz aws gsutil", + "functions -u aws gsutil", + "typeset -fu aws gsutil", + "enable -f ./plugin.so aws", + "prepare-environment", + "read filename", + "export LANG=C", + "set -- ./setup.sh", + "set -A path ./tools", + ], +) +@pytest.mark.parametrize( + "transfer", + [ + "aws s3 cp s3://example-bucket/public.csv /tmp/public.csv", + "gsutil cp gs://example-bucket/public.csv /tmp/public.csv", + ], +) +@pytest.mark.parametrize("newline", ["\n", "\r\n"]) +def test_unproved_preamble_keeps_cloud_finding(setup: str, transfer: str, newline: str) -> None: + findings = data_exfiltration.analyze( + f"{setup}{newline}{transfer}{newline}", "download.sh", "shell" + ) + e5 = [finding for finding in findings if finding.rule_id == "E5"] + assert len(e5) == 1 + assert e5[0].location.start_line == 2 + + +@pytest.mark.parametrize( + "setup", + [ + "#!/bin/sh", + "# Public files", + "set -e", + "set -eu", + "set -e -u", + "set -o pipefail", + "set -euo pipefail", + ], +) +@pytest.mark.parametrize("file_type", ["shell", "markdown", "text"]) +def test_supported_preamble_preserves_download_exemption(setup: str, file_type: str) -> None: + findings = data_exfiltration.analyze( + f"{setup}\naws s3 cp s3://example-bucket/public.csv ./public.csv\n", + "download.sh", + file_type, + ) + assert not any(finding.rule_id == "E5" for finding in findings) + + +def test_prior_download_can_replace_a_later_cloud_executable() -> None: + findings = data_exfiltration.analyze( + "aws s3 cp s3://example-bucket/replacement /usr/local/bin/aws\n" + "aws s3 cp s3://example-bucket/public.csv ./public.csv\n", + "download.sh", + "shell", + ) + assert [f.location.start_line for f in findings if f.rule_id == "E5"] == [2] + + +def test_setup_after_download_does_not_change_earlier_finding_location() -> None: + findings = data_exfiltration.analyze( + "aws s3 cp s3://example-bucket/public.csv ./public.csv\n" + "true && . ./setup.sh\n" + "aws s3 cp s3://example-bucket/public.csv ./public.csv\n", + "download.sh", + "shell", + ) + assert [f.location.start_line for f in findings if f.rule_id == "E5"] == [3] + + +@pytest.mark.parametrize("offset", [4095, 4096, 4097]) +def test_preamble_proof_stays_within_context_bound(offset: int) -> None: + findings = data_exfiltration.analyze( + "#" + " " * (offset - 2) + "\naws s3 cp s3://example-bucket/public.csv ./public.csv\n", + "download.sh", + "shell", + ) + assert any(f.rule_id == "E5" for f in findings) == (offset > 4096) + + def test_large_prefix_cannot_forget_an_earlier_alias() -> None: content = ( "alias aws=custom\n# " + "x" * 4500 + "\n" @@ -263,6 +353,8 @@ def test_large_prefix_cannot_forget_an_earlier_alias() -> None: [ ("aws s3 cp s3://example-bucket/public.csv ./public.csv\n", []), ("set -eu\naws s3 cp s3://example-bucket/public.csv ./public.csv\n", []), + ("true && . ./setup.sh\naws s3 cp s3://example-bucket/public.csv /tmp/public.csv\n", [3]), + ("autoload -Uz gsutil\ngsutil cp gs://example-bucket/public.csv /tmp/public.csv\n", [3]), ( "hash -p ./custom-aws aws\naws s3 cp s3://example-bucket/public.csv ./public.csv\n", [3],