diff --git a/pyproject.toml b/pyproject.toml index 941682c8..75dbe5f9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "setuptools.build_meta" [project] name = "polyswarm" -version = "4.4.0" +version = "4.5.0" description = "CLI for using the PolySwarm Customer APIs" readme = "README.md" authors = [{ name = "PolySwarm Developers", email = "info@polyswarm.io" }] @@ -22,7 +22,7 @@ classifiers = [ ] dependencies = [ - "polyswarm_api>=4.4.0,<5.0.0", + "polyswarm_api>=4.5.0,<5.0.0", "click>=7.1", "colorama>=0.4.6", "click-log>=0.4.0", @@ -51,7 +51,7 @@ include-package-data = true where = ["src"] [tool.bumpversion] -current_version = "4.4.0" +current_version = "4.5.0" commit = true tag = false sign_tags = true diff --git a/specs/02-commands.md b/specs/02-commands.md index fd1aeb7e..62a62aae 100644 --- a/specs/02-commands.md +++ b/specs/02-commands.md @@ -30,7 +30,7 @@ The top-level command groups, what each is for, and the primary `polyswarm-api` | `tag` (`tags.py`) | Tag CRUD | `tag_{create,delete,get,list}` | | `link` (`links.py`) | Tag/family links on artifacts | `tag_link_multiple`, `tag_link_get`, `tag_link_list` | | `family` (`families.py`) | Malware-family CRUD | `family_{create,update,delete,get,list}` | -| `rules` (`rules.py`) | YARA ruleset CRUD plus `favorite [--unfavorite]` (the star toggle: renders the new state + the server-owned "N of M used" budget, and converts the machine-readable `FAVORITE_LIMIT` refusal into a clean actionable message at exit 2, never 1 — 1 is reserved for no-results/not-found; 2 is the broad bucket `ExceptionHandlingGroup` maps the PolyswarmException hierarchies to. **2 does not identify a server refusal:** click exits 2 for a `UsageError` too, so a scripted caller cannot tell “the favorite budget is full” from “you passed a bad flag” without reading the message). `list` takes the server-side filters `--name` / `--status active` / `--favorites-only` / `--has-new-results` (conjunctive; the list is keyset-paginated, so filtering locally would mean walking every page). `rules favorite` and the `rules list` filters need SDK 4.4.0, which the pin requires (see [05-sdk-contract.md](./05-sdk-contract.md) §Current floor), so they are called directly. The formatters read the hunt-page fields directly: the pin guarantees the SDK parses them, so `None` means the *server* had no answer | `ruleset_{create,delete,update,get,list,favorite}` | +| `rules` (`rules.py`) | YARA ruleset CRUD (`list` takes the server-side filters plus `--sort active-first`, the hunt page's order: rulesets carrying a live hunt link first, deduped by id because the key is mutable — neither the position nor a moved row's own fields are evidence a hunt is running, see [05-sdk-contract.md](./05-sdk-contract.md) §A mutable order makes the walk the caller's problem. Ordering is the server's, never a local re-sort of one keyset page) plus `favorite [--unfavorite]` (the star toggle: renders the new state + the server-owned "N of M used" budget, and converts the machine-readable `FAVORITE_LIMIT` refusal into a clean actionable message at exit 2, never 1 — 1 is reserved for no-results/not-found; 2 is the broad bucket `ExceptionHandlingGroup` maps the PolyswarmException hierarchies to. **2 does not identify a server refusal:** click exits 2 for a `UsageError` too, so a scripted caller cannot tell “the favorite budget is full” from “you passed a bad flag” without reading the message). `list` takes the server-side filters `--name` / `--status active` / `--favorites-only` / `--exclude-favorites` (its inverse, refused together with it — for a client that lists the favorites separately) / `--has-new-results` (conjunctive; the list is keyset-paginated, so filtering locally would mean walking every page). `rules favorite` and the `rules list` filters need SDK 4.4.0 and `rules list --sort` needs 4.5.0, which the pin requires (see [05-sdk-contract.md](./05-sdk-contract.md) §Current floor), so they are called directly. The formatters read the hunt-page fields directly: the pin guarantees the SDK parses them, so `None` means the *server* had no answer | `ruleset_{create,delete,update,get,list,favorite}` | | `metadata` (`metadata.py`) | Rerun metadata; scan lookup; IP/URL analysis | `rerun_metadata`, `scan_lookup`, `submit_url` | | `activity` (`event.py`) | List account activity/events | `event_list` | | `account` (`account.py`) | Account whois / features | `account_whois`, `account_features` | diff --git a/specs/03-formatters.md b/specs/03-formatters.md index e3ac6e51..997ff603 100644 --- a/specs/03-formatters.md +++ b/specs/03-formatters.md @@ -173,3 +173,105 @@ the attributes directly: - `source_rule_changed` is tri-state: `None` means UNKNOWN, not "unchanged", and prints nothing; the label names its reference point — "changed since this hunt froze it" — so it cannot read as "edited recently". + +## Matched strings on hunt results + +`TextOutput.historical_result` / `TextOutput.live_result` render `result.matched_strings` +— the yara strings behind a hit — between `Tags:` and `Download Url:`, via the shared +`TextOutput._matched_strings` helper. + +The attribute is **three-state** (the SDK's `05-downstream-contract.md` is authoritative) +and the three must stay distinguishable in the output — but they do **not** each get a +line. + +| `matched_strings` | Rendered | +|---|---| +| `None` | *nothing* — no line is emitted | +| `[]`, no count | `Matched Strings: none -- the rule matched without byte evidence (a structural or negative match, or private strings)` | +| `[]`, count > 0 | `Matched Strings: none shown (N withheld, result size limit)` — the count **overrides** the row above, because asserting a structural match while discarding a withheld count would be a confident false statement. Should be unreachable; see below. | +| `[…]` | `Matched Strings:` followed by one indented ` $ident @ 0xOFFSET (N bytes[, truncated]): DATA` line per entry | + +**The silent-`None` branch depends on list routes sending `null`, and that is measured +rather than assumed.** If a list route ever returned `[]` per row instead, every row of a +large hunt would carry the loud "matched without byte evidence" line — the permanent +false alarm this design exists to avoid, arriving through the branch deliberately kept +loud. The server pins it for **both** hunt pairs in artifact-index's +`test_list_serializers_render_nulls_not_payloads`, which asserts the key is present-and-null on +`ScanResultListSerializer` *and* `LiveResultListSerializer`, against fixture rows that do +carry evidence. This repo cannot verify it; it relies on that test. + +Two constraints, both counter-intuitive enough to be worth stating: + +- **`None` emits nothing, and `[]` must not follow it into silence.** The instinct is to + explain the absence. Resist it: `None` overwhelmingly means "this is a list route", + which *can never* carry strings, so a line there is a permanent false alarm on every + row rather than information — and `live feed` loops over this same method, with nothing + on the resource to tell the routes apart (`live_feed` and `live_result` both yield a + `LiveHuntResult`). Route-awareness would mean threading a flag from the command layer + into a new parameter on **every** `BaseOutput` implementation (`text`, `json`, all three + `hashes` subclasses), which buys too little. `[]` is the opposite case and keeps its + line: it only ever reaches a detail route, and "the rule matched with no byte evidence" + is a real answer to "why did this hit". Since the analyzer always sends `strings` once + this feature ships, `None` on a detail route means a result predating it — nothing to + say. +- **The entry keys are subscripted, deliberately.** `identifier`, `offset`, `length`, + `data` and `truncated` are read by subscript, because a partial entry is not version + skew but a producer violating its own contract, and rendering half a match as though it + were whole is worse than failing. The attribute itself needs no such defence: the floor + names an SDK that parses it (§Current floor in [`05-sdk-contract.md`](./05-sdk-contract.md)). +- **`data` is sanitised before rendering.** It is the only sample-derived field in a hunt + result, so it is attacker-controlled end to end. yara escapes non-printables upstream + and the analyzer preserves that rendering, so `_safe_data` is a no-op on valid input — + it exists because the guarantee lives in another repo, and a raw CSI sequence reaching + a terminal would repaint or clear an analyst's screen. +- **ASCII only, and true of this whole block.** The literals are ASCII; the two + server-supplied *string* fields — `data` and `identifier` — go through `_safe_data`; + and the three server-supplied *numbers* — `offset`, `length`, `dropped` — carry an + integer format spec (`:x` / `:d`), which is what pins them rather than `_safe_data`. + Fields *outside* the block (`rule_name`, `tags`) are unfiltered and outside this claim. + Stdout under a C/POSIX locale replaces non-ASCII with `?`. +- **`truncated` is not a byte count.** The stored length is capped server-side, so the + marker means "there was more than this" and over-reports at exactly the cap. Never + render it as an exact size. + +**Read both attributes directly** — `result.matched_strings`, no `getattr`. The floor +(`polyswarm_api>=4.4.0`, [`05-sdk-contract.md`](./05-sdk-contract.md) §Current floor) +names an SDK that parses them, so `pip` refuses the install a probe would guard against; +`specs/05-project-standards.md` §16 has the general rule. `None` then means exactly what +the table above says — the *server* did not report — which is the reading the silent +branch depends on. + +### The dropped-count line + +When `result.matched_strings_dropped` is non-zero, a final line is appended **inside** the +block, in **yellow** rather than white: + +``` + ... 19 more not shown (result size limit) +``` + +It is the one line here reporting something the platform withheld, which is why it is not +white like the entries above it. Omitted entirely when the count is zero or `None`. + +**An empty list with a non-zero count does not claim "no byte evidence".** That +combination should be unreachable — the analyzer keeps a match's first string, so +`[] ⇒ dropped == 0` — but the renderer must not *depend* on an invariant owned by another +repo while making a positive claim about the rule. It reports what is certain instead +(`none shown (N withheld, result size limit)`), because asserting a structural match and +discarding the count is the precise wrong inference this line exists to prevent. + +This is not cosmetic. Without it a truncated list reads as the whole truth and a user +concludes their rule hit twice when it hit twenty-one times — the same wrong-inference +class the three-state contract above exists to prevent, one level down. + +`JSONOutput` needs no change — it dumps the resource's `.json`, which already carries the +raw `matched_strings` and `matched_strings_dropped` keys. + +Coverage is two modules. `tests/hunt_matched_strings_test.py` is Style 3 — the formatter +driven directly with constructed SDK resources — and owns *which line a field value +produces*. `tests/hunt_matched_strings_cli_test.py` is the Style 1 counterpart +[`04-testing.md`](./04-testing.md) requires: it drives `live result` through `CliRunner` +so a broken `output.extend` call is caught, which the Style 3 module cannot see because +it calls `_matched_strings` itself. The `cli_test.py` cassettes predate the field, so +every result they render takes the silent `None` branch — they pin that no stray line +appears, and nothing more. diff --git a/specs/05-sdk-contract.md b/specs/05-sdk-contract.md index 63630a49..ed2ae902 100644 --- a/specs/05-sdk-contract.md +++ b/specs/05-sdk-contract.md @@ -39,6 +39,32 @@ for item in api.iocs_by_hash(type, value): Because the generators are **lazy**, calling one does no I/O and raises nothing until iterated. Code that runs SDK calls through a thread pool must consume the generator **inside the worker** so per-item exception handling fires where it's expected — this is why `utils.parallel_executor_iterable_results` materialises each generator inside the submitted callable (see `01-architecture.md`). +### A mutable order makes the walk the caller's problem + +Most list endpoints are keyset-paginated on an immutable key, so a walk sees every row once. +`ruleset_list(sort='active_first')` is the exception in the current surface: its key is the +live-hunt link, which the hunt itself flips, and the SDK's own docstring puts the consequence +on the caller — *"This generator streams pages and does not dedupe — dedupe by `id` if you +consume more than one page"*. + +A command that walks every page of such an endpoint must therefore: + +- **Dedupe by `id`.** A row whose key changes mid-walk drops below the cursor and is served + again. `rules list` keeps a `seen` set (`client/rules.py`); the dedupe is unconditional, + because the id is unique under either order and a gate is one more thing to update when the + next mutable order appears. +- **Keep the FIRST copy, and know what that costs.** A streaming printer has already written + copy one when copy two arrives, so first-wins is the only option — and copy one carries the + PRE-transition values. A ruleset whose hunt stopped mid-walk prints with its old + `Live Hunt Id`. Under a mutable order neither the position nor the row's own fields are + authoritative for a row that moved; a fresh run shows the settled state. +- **Say both in the command's help**, not only here. The user reading `--help` is the one who + will act on a stale field. + +What cannot be repaired client-side: a row whose key changes so that it moves ABOVE the +cursor is never served at all, so it is missing from that walk entirely. Document it; a +re-run lists it. + ### No-results signalling A search that the server answers `204 No Content` raises `NoResultsException` from the SDK **when the generator is iterated**. The CLI's `ExceptionHandlingGroup` maps that (and the CLI's own aggregate `NoResultsException` from `parallel_executor`) to exit code `1`. Don't swallow it in command code. @@ -79,7 +105,7 @@ When a CLI feature needs an SDK surface that doesn't exist yet: **Read the declared version off the archive's own tree, and mind pre-release suffixes.** PEP 440 orders `4.2.0.dev1 < 4.2.0`, so a `develop` head carrying a dev suffix (the SDK's `pyproject.toml` has a `[tool.bumpversion.parts.dev]`) would *not* satisfy a `>=4.2.0` floor even though it looks like 4.2.0 — and the archive build would be silently replaced from PyPI. Check the version string in the SDK branch's `pyproject.toml` / `__init__.py`, not the last release tag. When the floor was last verified this way both were read from `origin/develop` as `4.2.0`, no suffix; the pin has since moved on (§Current floor is the one authoritative statement of its value), and every bump should be re-checked the same way. -### Current floor — `polyswarm_api>=4.4.0` +### Current floor — `polyswarm_api>=4.5.0` The floor is whatever `pyproject.toml` pins; this header follows it. It lives in ONE authoritative place for a reason — a copy here drifted behind the pin once already. The 4.2.0 rationale below still holds transitively; on 4.1.0 both behaviours fail *silently*, which is why the floor is a hard requirement rather than a preference: @@ -96,8 +122,14 @@ predates this and is documented where it lives: the known-good rendering attribu that is not supported rather than against a version the floor permits.) The hunt-page surfaces — `ruleset_favorite` and the `YaraRulesetFavorite` resource, the `ruleset_list` filters, `live_feed(livescan_id=, max_results=)`, and the tracking/provenance fields the -formatters render — are what moved the floor to 4.4.0. Code and tests use them -directly. +formatters render — are what moved the floor to 4.4.0, together with +`matched_strings` / `matched_strings_dropped` on the four hunt-result classes +(the yara evidence behind a hit; see [`03-formatters.md`](./03-formatters.md) +§Matched strings on hunt results). `rules list --sort active-first` forwards +`ruleset_list(sort='active_first')`, a keyword 4.5.0 adds, and that is what moved the +floor to 4.5.0 (the `tests/formatter_hunt_fields_test.py` autospec assertion is the +signature check: against a 4.4.0 SDK it fails at the mock, not at the server). Code and +tests use them directly. **Raising the floor is the whole procedure** when this repo needs something new from the SDK: diff --git a/src/polyswarm/__init__.py b/src/polyswarm/__init__.py index 26a6c390..330025d8 100644 --- a/src/polyswarm/__init__.py +++ b/src/polyswarm/__init__.py @@ -1 +1 @@ -__version__ = '4.4.0' +__version__ = '4.5.0' diff --git a/src/polyswarm/client/rules.py b/src/polyswarm/client/rules.py index 9f8fa7e2..2b3aa376 100644 --- a/src/polyswarm/client/rules.py +++ b/src/polyswarm/client/rules.py @@ -36,23 +36,66 @@ def delete(ctx, rule_id): @click.option('-s', '--status', type=click.Choice(['active']), help='Only rulesets whose live hunt is currently running.') @click.option('--favorites-only', is_flag=True, help='Only favorited (starred) rulesets.') +@click.option('--exclude-favorites', is_flag=True, + help='Only rulesets that are NOT favorited. The inverse of ' + '--favorites-only, and refused together with it. For a ' + 'client that lists the favorites separately.') @click.option('--has-new-results', is_flag=True, help='Only rulesets whose stored new-results counter is positive.') +@click.option('--sort', type=click.Choice(['active-first']), + help='Order: rulesets carrying a live hunt link first, newest first ' + 'within each block. Default is newest first. The rank is the ' + 'stored link, which is WIDER than what Live Hunt Id renders ' + 'from: a legacy row whose hunt was stopped without clearing ' + 'the link leads the list while rendering no Live Hunt Id at ' + 'all, indistinguishable from an idle one — so the position ' + 'is not evidence that a hunt is running. Neither is the ' + 'field for a row that MOVED during the walk: see the note ' + 'on stale copies below.') @click.pass_context -def list_rules(ctx, name, status, favorites_only, has_new_results): +def list_rules(ctx, name, status, favorites_only, exclude_favorites, has_new_results, sort): """List rulesets, optionally filtered. All filters are conjunctive. - Filtering is applied SERVER-side: the list is keyset-paginated, so a - client filtering locally would have to walk every page to find matches. + Filtering and ordering are applied SERVER-side: the list is + keyset-paginated, so a client filtering or sorting locally would have to + walk every page to get it right. + + This command walks EVERY page, which makes it the consumer the SDK puts the + dedupe obligation on: under --sort active-first the ordering key is mutable + (it is the live-hunt link the sort ranks on), so a ruleset whose hunt stops + between two page fetches drops below the cursor and the server serves it a + second time. Rows are therefore emitted at most once per run, keyed on id. + + The copy that survives is the FIRST one, which is the only choice a + streaming printer has — it wrote that copy before the second arrived — and + it carries the values from BEFORE the transition. So the one row the dedupe + acts on prints its old Live Hunt Id, for a hunt that has since stopped. + Under this order a moved row is authoritative in neither its position nor + its fields; a fresh run shows the settled state. + + The symmetric case cannot be repaired from here and is not hidden: a hunt + STARTED mid-walk moves its ruleset above the cursor, so that row never + reaches this client at all. A re-run lists it. """ api = ctx.obj['api'] output = ctx.obj['output'] # A False flag is not a filter: send only what the caller actually asked for. + # The CLI spells the sort with a hyphen; the server token is 'active_first'. kwargs = {k: v for k, v in (('name', name), ('status', status), ('favorites_only', favorites_only or None), - ('has_new_results', has_new_results or None)) + ('exclude_favorites', exclude_favorites or None), + ('has_new_results', has_new_results or None), + ('sort', sort.replace('-', '_') if sort else None)) if v is not None} + seen = set() for ruleset in api.ruleset_list(**kwargs): + # Unconditional rather than gated on `sort`: the id is unique either + # way, one set of ids costs nothing next to the rows already rendered, + # and a gate would be a second place to update when another mutable + # order appears. Under the default order this never drops anything. + if ruleset.id in seen: + continue + seen.add(ruleset.id) output.ruleset(ruleset) diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index 0b32d90d..3dfcfd69 100644 --- a/src/polyswarm/formatters/text.py +++ b/src/polyswarm/formatters/text.py @@ -1,5 +1,6 @@ import sys import functools +import re import json from datetime import datetime @@ -17,6 +18,18 @@ def pretty_print_datetime(value): return datetime.strftime(value, '%Y-%m-%d %H:%M:%S UTC') +# `data` and `identifier` are sample-derived, so attacker-controlled; the escaping that +# makes them safe is upstream's contract, not ours. Whitelist, not blacklist -- an +# earlier blacklist stopped at \x7f and let U+009B (8-bit CSI) through. +# Rationale: specs/03-formatters.md, Matched strings. +_UNPRINTABLE = re.compile(r'[^\x20-\x7e]') + + +def _safe_data(value): + """Matched bytes as yara rendered them, with anything unprintable neutralised.""" + return _UNPRINTABLE.sub('.', value) + + def is_grouped(fn): @functools.wraps(fn) def wrapper(self, text): @@ -41,6 +54,33 @@ def _get_score_format(self, score): else: return self._red + # Three-state, and NOT interchangeable: None renders nothing (it is dominated by list + # routes, which can never carry strings), [] keeps its line, a list renders entries. + # Do not collapse them or add a line for None. Why: specs/03-formatters.md. + def _matched_strings(self, strings, dropped=None): + if strings is None: + return [] + if not strings: + if dropped: + # Contradictory input (the analyzer keeps a match's first string). + # Report only what is certain rather than assert a rule property. + return [self._yellow( + f'Matched Strings: none shown ({dropped:d} withheld, result size limit)')] + return [self._white('Matched Strings: none -- the rule matched without byte ' + 'evidence (a structural or negative match, or private strings)')] + lines = [self._white('Matched Strings:')] + for string in strings: + size = f'{string["length"]:d} bytes' + if string['truncated']: + size += ', truncated' + lines.append(self._white( + f' {_safe_data(string["identifier"])} @ 0x{string["offset"]:x} ({size}): {_safe_data(string["data"])}')) + if dropped: + # Yellow: the one line here reporting something the platform withheld. + lines.append(self._yellow( + f' ... {dropped:d} more not shown (result size limit)')) + return lines + def _output(self, output, write): if write: click.echo('\n'.join(output) + '\n', file=self.out) @@ -252,6 +292,8 @@ def historical_result(self, result, write=True): output.append(self._white(malicious)) if result.tags: output.append(self._white(f'Tags: {result.tags}')) + output.extend(self._matched_strings(result.matched_strings, + result.matched_strings_dropped)) if result.download_url: output.append(self._white(f'Download Url: {result.download_url}')) return self._output(output, write) @@ -283,6 +325,8 @@ def live_result(self, result, write=True): output.append(self._white(malicious)) if result.tags: output.append(self._white(f'Tags: {result.tags}')) + output.extend(self._matched_strings(result.matched_strings, + result.matched_strings_dropped)) if result.download_url: output.append(self._white(f'Download Url: {result.download_url}')) return self._output(output, write) diff --git a/tests/formatter_hunt_fields_test.py b/tests/formatter_hunt_fields_test.py index 1d0675da..05c7afa8 100644 --- a/tests/formatter_hunt_fields_test.py +++ b/tests/formatter_hunt_fields_test.py @@ -15,7 +15,10 @@ favorite`` renders the toggle response and converts the machine-readable FAVORITE_LIMIT refusal into a clean message. All are asserted through autospec'd mocks, so every call is signature-checked against the SDK the - pin actually installs. + pin actually installs; and +* the active-first order (4.5.0): the ``--sort`` token that reaches the server, + and what walking every page of a MUTABLY ordered list obliges this command to + do — dedupe by id, first copy wins, stale values and all. """ from unittest import TestCase, mock @@ -132,10 +135,13 @@ def test_hunt_unknown_tri_state_prints_nothing(self): class RulesListZeroArgTest(TestCase): - """`rules list` calls a zero-argument ``ruleset_list()`` — a False flag - is not a filter, so an unfiltered list forwards no - behaviour at all. autospec makes the assertion a signature check against - the installed SDK.""" + """`rules list` calls a zero-argument ``ruleset_list()`` — a False flag is + not a filter, so an unfiltered list forwards no behaviour at all, and a + filtered one forwards exactly the filters given. autospec makes each + assertion a signature check against the installed SDK. + + The `--sort` token and the mid-walk dedupe live in + ``RulesListSortAndDedupeTest`` below.""" def test_list_passes_no_kwargs_at_all(self): with mock.patch('polyswarm_api.api.PolyswarmAPI.ruleset_list', @@ -169,6 +175,156 @@ def test_filters_are_forwarded_only_when_given(self): favorites_only=True, has_new_results=True) +class RulesListSortAndDedupeTest(TestCase): + """`rules list --sort active-first` — the token that reaches the server, and + what walking every page of a MUTABLY ordered list obliges this command to do. + + Two separate contracts. The token: the CLI spelling is hyphenated, the + server's is not, the sort is forwarded only when given, and an unknown one + is refused here rather than at the server. The walk: the ordering key is the + live-hunt link the sort ranks on, so a row whose hunt changes mid-walk is + served twice or missed — this command dedupes by id, keeps the FIRST copy, + and therefore renders that row's PRE-transition values + (specs/05-sdk-contract.md §A mutable order makes the walk the caller's + problem). Each dedupe test fails against a different wrong implementation: + keyed on the name, or last-wins. + """ + + def test_sort_active_first_is_forwarded_as_the_server_token(self): + """`--sort active-first` (CLI spelling, hyphen) reaches the SDK as + the server's `sort='active_first'` token — and, like the filters, only + when given: the unsorted default sends no `sort` at all, so the list + keeps its id-desc order and the request stays byte-compatible.""" + with mock.patch('polyswarm_api.api.PolyswarmAPI.ruleset_list', + autospec=True, return_value=iter(())) as ruleset_list: + result = CliRunner().invoke( + client.polyswarm_cli, + ['-a', '1' * 32, '-u', 'http://ai:9696/v3', '-c', 'gamma', + 'rules', 'list', '--sort', 'active-first'], + catch_exceptions=False) + assert result.exit_code == 0, result.output + ruleset_list.assert_called_once_with(mock.ANY, sort='active_first') + + def test_sort_composes_with_the_filters(self): + # The kwargs comprehension is the one site that could drop or + # mistranslate the sort when filters ride along. + with mock.patch('polyswarm_api.api.PolyswarmAPI.ruleset_list', + autospec=True, return_value=iter(())) as ruleset_list: + result = CliRunner().invoke( + client.polyswarm_cli, + ['-a', '1' * 32, '-u', 'http://ai:9696/v3', '-c', 'gamma', + 'rules', 'list', '--status', 'active', '--favorites-only', + '--sort', 'active-first'], + catch_exceptions=False) + assert result.exit_code == 0, result.output + ruleset_list.assert_called_once_with( + mock.ANY, status='active', favorites_only=True, sort='active_first') + + def test_a_row_served_twice_by_the_mutable_sort_is_printed_once(self): + """The command walks every page, so it is the consumer the SDK puts the + dedupe obligation on. Under the active-first order a ruleset whose hunt + stops between two page fetches falls below the cursor and the server + serves it again; without the dedupe the run prints it twice and any + script counting the output double-counts it. + + The two copies differ in the field the sort ranks on — that is WHY the + row moved — so this is the real re-serve shape, not a repeated row.""" + repeated = [_ruleset(id='5', name='stops-mid-walk', livescan_id='77'), + _ruleset(id='7', name='other'), + _ruleset(id='5', name='stops-mid-walk', livescan_id=None)] + with mock.patch('polyswarm_api.api.PolyswarmAPI.ruleset_list', + autospec=True, return_value=iter(repeated)): + result = CliRunner().invoke( + client.polyswarm_cli, + ['-a', '1' * 32, '-u', 'http://ai:9696/v3', '-c', 'gamma', + 'rules', 'list', '--sort', 'active-first'], + catch_exceptions=False) + assert result.exit_code == 0, result.output + assert result.output.count('stops-mid-walk') == 1, result.output + # The row between the duplicates still renders — dedupe, not truncation. + assert 'other' in result.output, result.output + + def test_the_surviving_copy_is_the_first_one_stale_values_and_all(self): + """First-wins is a decision, not an accident of the loop: a streaming + printer has already written copy one when copy two arrives. The cost is + that the surviving copy carries the PRE-transition values — the row + prints the Live Hunt Id of a hunt that has since stopped — which is why + the help and specs/05 say a moved row is authoritative in neither its + position nor its fields. A last-wins rewrite would flip this.""" + served = [_ruleset(id='5', name='stops-mid-walk', livescan_id='77'), + _ruleset(id='5', name='stops-mid-walk', livescan_id=None)] + with mock.patch('polyswarm_api.api.PolyswarmAPI.ruleset_list', + autospec=True, return_value=iter(served)): + result = CliRunner().invoke( + client.polyswarm_cli, + ['-a', '1' * 32, '-u', 'http://ai:9696/v3', '-c', 'gamma', + 'rules', 'list', '--sort', 'active-first'], + catch_exceptions=False) + assert result.exit_code == 0, result.output + assert result.output.count('stops-mid-walk') == 1, result.output + # The formatter gates the pair on a truthy livescan_id, so the line is + # present iff the copy that survived is the one from before the stop. + assert 'Live Hunt Id' in result.output, result.output + assert '77' in result.output, result.output + + def test_two_rulesets_sharing_a_name_both_render(self): + """The key is the id, and only the id. Ruleset names are not unique, so + an implementation that deduped on the name — or on the whole rendered + block — would swallow a real row from an inventory listing while passing + every other test in this class.""" + rows = [_ruleset(id='5', name='dup'), _ruleset(id='7', name='dup')] + with mock.patch('polyswarm_api.api.PolyswarmAPI.ruleset_list', + autospec=True, return_value=iter(rows)): + result = CliRunner().invoke( + client.polyswarm_cli, + ['-a', '1' * 32, '-u', 'http://ai:9696/v3', '-c', 'gamma', + 'rules', 'list', '--sort', 'active-first'], + catch_exceptions=False) + assert result.exit_code == 0, result.output + assert result.output.count('dup') == 2, result.output + + def test_the_default_order_is_not_narrowed_by_the_dedupe(self): + # The id-desc default cannot repeat a row, so every row it yields must + # still reach the output; the dedupe is unconditional and must be inert + # here rather than dropping a distinct row that happens to look alike. + rows = [_ruleset(id='5', name='first'), _ruleset(id='7', name='second')] + with mock.patch('polyswarm_api.api.PolyswarmAPI.ruleset_list', + autospec=True, return_value=iter(rows)): + result = CliRunner().invoke( + client.polyswarm_cli, + ['-a', '1' * 32, '-u', 'http://ai:9696/v3', '-c', 'gamma', + 'rules', 'list'], + catch_exceptions=False) + assert result.exit_code == 0, result.output + assert 'first' in result.output and 'second' in result.output, result.output + + def test_exclude_favorites_is_forwarded_only_when_given(self): + """The inverse filter the hunt page sends alongside the sort: the + favorites are their own list above the page, so the paginated list asks + for the non-favorites. Like every other flag here, a False one is not a + filter and must not reach the SDK.""" + with mock.patch('polyswarm_api.api.PolyswarmAPI.ruleset_list', + autospec=True, return_value=iter(())) as ruleset_list: + result = CliRunner().invoke( + client.polyswarm_cli, + ['-a', '1' * 32, '-u', 'http://ai:9696/v3', '-c', 'gamma', + 'rules', 'list', '--exclude-favorites', '--sort', 'active-first'], + catch_exceptions=False) + assert result.exit_code == 0, result.output + ruleset_list.assert_called_once_with( + mock.ANY, exclude_favorites=True, sort='active_first') + + def test_sort_rejects_an_unknown_order(self): + # A closed choice on the CLI side too: the server would 400 an unknown + # sort, but the CLI should not have to make the round trip to say so. + result = CliRunner().invoke( + client.polyswarm_cli, + ['-a', '1' * 32, '-u', 'http://ai:9696/v3', '-c', 'gamma', + 'rules', 'list', '--sort', 'newest']) + assert result.exit_code == 2, result.output + assert 'active-first' in result.output + + class LiveFeedOptionsTest(TestCase): """`live feed` — the badge's drill-down (--livescan-id) and its bound diff --git a/tests/hunt_matched_strings_cli_test.py b/tests/hunt_matched_strings_cli_test.py new file mode 100644 index 00000000..baecd1ef --- /dev/null +++ b/tests/hunt_matched_strings_cli_test.py @@ -0,0 +1,83 @@ +"""Command-tree coverage for the matched-strings block (Style 1 — SDK-boundary mocks). + +`hunt_matched_strings_test.py` drives the formatter directly and covers *which line a +given field value produces*, including the `output.extend` call inside `live_result` -- +it invokes that method, so a deletion there does fail it. What it cannot observe is +everything ABOVE the formatter: that `live result` parses its argument, calls the SDK, +and hands the result to `output.live_result` rather than some other renderer. +`specs/04-testing.md` §Style 3 requires exactly this counterpart -- a command whose +rendering is covered that way "still needs at least one `CliRunner` test proving the +command reaches the formatter". + +Measured, not assumed: rewiring the command to `output.live_feed([...])` leaves all 36 +assertions in the sibling module passing and fails this module. The cassette tests cannot +catch it either -- they predate the field, so every result they render takes the silent +`None` branch. +""" +from unittest import TestCase, mock + +from click.testing import CliRunner +from polyswarm_api import resources + +from polyswarm.client import polyswarm as client + +_API_KEY = '1' * 32 +_API_URL = 'http://artifact-index-e2e:9696/v3' +_COMMUNITY = 'gamma' + +_CONTENT = { + 'id': 123, + 'instance_id': 2, + 'livescan_id': 3, + 'created': '2022-05-26T19:41:33.797898', + 'sha256': 'f' * 64, + 'rule_name': 'dos_stub_message', + 'tags': '{pe,stub}', + 'polyscore': 0.5, + 'malware_family': None, + 'detections': {'malicious': 1, 'total': 1}, +} + +_STRINGS = [ + {'offset': 78, 'identifier': '$stub', 'length': 14, + 'data': '54 68 69 73 20 70 72 6F 67 72 61 6D 20 63', 'truncated': False}, +] + + +def _live_result(**overrides): + """A real `LiveHuntResult`, not a mock — the formatter reads polyscore, detections and + the rest of the row, so a bare stub dies before it reaches the block under test. The + floor (`polyswarm_api>=4.4.0`) guarantees this parses both matched-strings keys.""" + return resources.LiveHuntResult(dict(_CONTENT, **overrides)) + + +class HuntMatchedStringsCliTest(TestCase): + def setUp(self): + self.cli = CliRunner() + + def _run(self, *cmd): + return self.cli.invoke( + client.polyswarm_cli, + ['-a', _API_KEY, '-u', _API_URL, '-c', _COMMUNITY] + list(cmd), + catch_exceptions=False, + ) + + def _live_result_output(self, **overrides): + with mock.patch('polyswarm_api.api.PolyswarmAPI.live_result', + return_value=_live_result(**overrides)): + return self._run('--output-format', 'text', 'live', 'result', '123').output + + def test_live_result_renders_the_block_through_the_command_tree(self): + output = self._live_result_output(matched_strings=_STRINGS) + assert 'Matched Strings:' in output + assert '$stub @ 0x4e' in output + + def test_live_result_reports_a_withheld_count_through_the_command_tree(self): + output = self._live_result_output(matched_strings=_STRINGS, + matched_strings_dropped=19) + assert '19 more not shown' in output + + def test_live_result_stays_silent_when_nothing_was_reported(self): + # No matched_strings key at all -- what a detail route serves for a result that + # predates the feature, and what every list route serves always. + assert 'Matched Strings' not in self._live_result_output() diff --git a/tests/hunt_matched_strings_test.py b/tests/hunt_matched_strings_test.py new file mode 100644 index 00000000..d6eeb5d8 --- /dev/null +++ b/tests/hunt_matched_strings_test.py @@ -0,0 +1,247 @@ +"""Pure-unit rendering tests for matched strings on hunt results. + +No CliRunner / VCR — these drive the text formatter directly with constructed SDK +resources (specs/04-testing.md, Style 3), because the point is which line a given +field value produces, not command-tree behaviour. + +The contract under test is three-state (specs/05-downstream-contract.md in the SDK): +`None` is "not reported", `[]` is "matched with no byte evidence", `[...]` is the +evidence, and the three must stay distinguishable in the output. + +`None` renders as silence on purpose: it is dominated by the list route, which can +never carry strings, so a per-row explanation there is a false alarm rather than +information. `[]` keeps its line — that one only ever reaches a detail route, where +"the rule matched with nothing to show" is a real answer to "why did this hit". +""" +import click +import pytest +from polyswarm_api import resources + +from polyswarm.formatters.text import TextOutput + +_COMMON = { + 'id': 123, + 'instance_id': 2, + 'created': '2022-05-26T19:41:33.797898', + 'sha256': 'f' * 64, + 'rule_name': 'dos_stub_message', + 'tags': '{pe,stub}', + 'polyscore': 0.5, + 'malware_family': None, + 'detections': {'malicious': 1, 'total': 1}, +} + +_STRINGS = [ + {'offset': 78, 'identifier': '$stub', 'length': 14, + 'data': '54 68 69 73 20 70 72 6F 67 72 61 6D 20 63', 'truncated': False}, + {'offset': 0, 'identifier': '$mz', 'length': 512, + 'data': '4D 5A 90 00 ...', 'truncated': True}, +] + +# (resource class, formatter method name) +PATHS = [ + (resources.LiveHuntResult, 'live_result'), + (resources.HistoricalHuntResult, 'historical_result'), +] + + +def _render(cls, method, **extra): + content = dict(_COMMON, **extra) + content['livescan_id' if cls is resources.LiveHuntResult else 'historicalscan_id'] = 3 + output = TextOutput(color=False) + lines = getattr(output, method)(cls(content), write=False) + return click.unstyle('\n'.join(lines)) + + +def _matched_lines(text): + """The matched-strings block -- header plus indented entries -- or [] if absent.""" + lines = text.splitlines() + start = next((i for i, line in enumerate(lines) if line.startswith('Matched Strings:')), None) + if start is None: + return [] + end = start + 1 + while end < len(lines) and lines[end].startswith(' '): + end += 1 + return lines[start:end] + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_absent_renders_nothing(cls, method): + """None is dominated by the list route, which can never carry strings -- `live feed` + loops over this same method -- so an explanation there would be a permanent false + alarm on every row. Silence, not a message.""" + assert _matched_lines(_render(cls, method)) == [] + assert 'Matched Strings' not in _render(cls, method) + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_explicit_null_renders_the_same_as_absent(cls, method): + assert _matched_lines(_render(cls, method)) == \ + _matched_lines(_render(cls, method, matched_strings=None)) + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_empty_says_the_rule_matched_without_evidence(cls, method): + """`[]` must NOT read as an error or as the absent case — the rule really did match.""" + line, = _matched_lines(_render(cls, method, matched_strings=[])) + assert 'none' in line + assert 'without byte evidence' in line + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_empty_and_absent_are_distinguishable(cls, method): + """The whole reason the server keeps them apart; collapsing them here wastes that. + Absent is silent, empty says the rule matched with nothing to show -- and it is the + EMPTY side that must never go silent, since it only ever reaches a detail route.""" + assert _matched_lines(_render(cls, method)) == [] + assert len(_matched_lines(_render(cls, method, matched_strings=[]))) == 1 + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_populated_renders_identifier_offset_length_and_data(cls, method): + header, first, second = _matched_lines(_render(cls, method, matched_strings=_STRINGS)) + assert header == 'Matched Strings:' + assert first == ' $stub @ 0x4e (14 bytes): 54 68 69 73 20 70 72 6F 67 72 61 6D 20 63' + assert second == ' $mz @ 0x0 (512 bytes, truncated): 4D 5A 90 00 ...' + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_truncation_is_marked_only_where_it_applies(cls, method): + _, first, second = _matched_lines(_render(cls, method, matched_strings=_STRINGS)) + assert 'truncated' not in first + assert 'truncated' in second + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_block_sits_between_tags_and_download_url(cls, method): + """Placement is the acceptance criteria — alongside Rule / Tags, not appended last.""" + text = _render(cls, method, matched_strings=_STRINGS, download_url='http://minio/x') + lines = text.splitlines() + tags = next(i for i, line in enumerate(lines) if line.startswith('Tags:')) + matched = next(i for i, line in enumerate(lines) if line.startswith('Matched Strings:')) + download = next(i for i, line in enumerate(lines) if line.startswith('Download Url:')) + assert tags < matched < download + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_dropped_count_is_reported_to_the_user(cls, method): + """A short list must not read as the whole truth. + + Without this line a user concludes their rule hit twice when it hit 21 times -- + exactly the wrong-inference class the three-state contract exists to prevent. + """ + lines = _matched_lines(_render(cls, method, matched_strings=_STRINGS, + matched_strings_dropped=19)) + assert lines[-1].strip().startswith('...') + assert '19 more not shown' in lines[-1] + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_no_dropped_line_when_nothing_was_dropped(cls, method): + rendered = _render(cls, method, matched_strings=_STRINGS) + assert 'not shown' not in rendered + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_dropped_line_does_not_fabricate_a_strings_block(cls, method): + """A dropped count with no strings is not a thing the server can send -- the + first string is always kept -- but rendering must not invent a block if it did.""" + rendered = _render(cls, method, matched_strings=None, matched_strings_dropped=19) + assert 'Matched Strings' not in rendered + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_empty_list_with_a_dropped_count_does_not_claim_no_evidence(cls, method): + """Contradictory input must not produce a confident false statement. + + The analyzer keeps a match's first string, so this should be unreachable -- but the + renderer trusted that invariant while asserting "matched without byte evidence" AND + discarding the count. Report what is certain instead. + """ + line, = _matched_lines(_render(cls, method, matched_strings=[], + matched_strings_dropped=19)) + assert 'without byte evidence' not in line, 'must not assert a rule property' + assert '19' in line and 'withheld' in line, 'the count must survive' + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_empty_list_without_a_count_still_says_no_evidence(cls, method): + """The normal empty case is unchanged.""" + line, = _matched_lines(_render(cls, method, matched_strings=[])) + assert 'without byte evidence' in line + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_control_characters_in_data_are_neutralised(cls, method): + """`data` is sample-derived, so it is the one attacker-controlled field here. + + yara escapes non-printables upstream and the analyzer keeps that rendering, so valid + data never contains a raw control byte -- but that guarantee lives in another repo. + A CSI sequence reaching a terminal unescaped could repaint or clear an analyst's + screen, so the renderer neutralises rather than trusting. + """ + # \x9b is the 8-bit CSI and is the reason this whitelists printable ASCII rather than + # blacklisting C0: an earlier version stopped at \x7f and let it through, so `\x9b2J` + # still cleared the screen -- a hole in the exact byte the sanitiser exists to block. + hostile = [{'offset': 0, 'identifier': '$evil', 'length': 9, + 'data': 'A\x1b[2JB\r\nC\x00D\x9b2JE\u00e9F', 'truncated': False}] + rendered = _render(cls, method, matched_strings=hostile) + for bad in ('\x1b', '\x9b', '\r', '\n\n', '\x00', '\u00e9'): + assert bad not in rendered.split('Matched Strings:')[1], repr(bad) + # the surviving printable bytes still render, so a legitimate match is unharmed + assert 'A.[2JB..C.D.2JE.F' in rendered + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_ordinary_data_is_untouched_by_the_sanitiser(cls, method): + """The escaping is a no-op on what yara actually emits.""" + rendered = _render(cls, method, matched_strings=_STRINGS) + assert '54 68 69 73 20 70 72 6F 67 72 61 6D 20 63' in rendered + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_output_is_ascii_only(cls, method): + """stdout under a C/POSIX locale replaces non-ASCII with '?'. Nothing here needs it.""" + rendered = _render(cls, method, matched_strings=_STRINGS, matched_strings_dropped=19) + block = '\n'.join(_matched_lines(rendered)) + assert block.isascii(), [c for c in block if not c.isascii()] + + +def _render_styled(cls, method, **extra): + """Keeps the ANSI wrapper, so the colour decision is observable. + + Every other render here uses color=False and unstyles, which makes colour invisible -- + both dropped-count lines could regress to white and the module would stay green. Both + halves are load-bearing: color=True is what makes TextOutput paint, and the absent + click.unstyle is what keeps the codes in the string. Mirrors _render_styled in + known_good_field_test.py, the established pattern for this (specs/03-formatters.md). + """ + content = dict(_COMMON, **extra) + content['livescan_id' if cls is resources.LiveHuntResult else 'historicalscan_id'] = 3 + return '\n'.join(getattr(TextOutput(color=True), method)(cls(content), write=False)) + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_dropped_count_line_is_yellow(cls, method): + """specs/03 states the colour as a deliberate signal: it is the one line in the block + reporting something the platform withheld, so it must not read as ordinary output.""" + rendered = _render_styled(cls, method, matched_strings=_STRINGS, + matched_strings_dropped=19) + expected = click.style(' ... 19 more not shown (result size limit)', fg='yellow') + assert expected in rendered, rendered + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_empty_with_a_count_line_is_yellow(cls, method): + """The other withheld-reporting line, and the one a reader is most likely to meet.""" + rendered = _render_styled(cls, method, matched_strings=[], matched_strings_dropped=19) + expected = click.style('Matched Strings: none shown (19 withheld, result size limit)', + fg='yellow') + assert expected in rendered, rendered + + +@pytest.mark.parametrize('cls,method', PATHS) +def test_the_ordinary_block_is_not_yellow(cls, method): + """Otherwise the two tests above would pass on a formatter that painted everything.""" + rendered = _render_styled(cls, method, matched_strings=_STRINGS) + assert click.style('Matched Strings:', fg='yellow') not in rendered