diff --git a/pyproject.toml b/pyproject.toml index 941682c8..84c00fc3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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", 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/05-sdk-contract.md b/specs/05-sdk-contract.md index 7a0749b4..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: @@ -99,7 +125,11 @@ that is not supported rather than against a version the floor permits.) The hunt 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). Code and tests use them directly. +§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/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/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