From 719d90617ceaf88d3e50918dde385811be1ed3bc Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Fri, 21 Aug 2026 14:24:50 -0700 Subject: [PATCH 01/25] feat(text): render matched strings on hunt results Shows the yara strings behind a hit, between Tags and Download Url. None is deliberately not rendered as silence. The feature exists to answer "why did this rule hit", and an absent line answers nothing -- it is indistinguishable from a rule that matched with nothing to show. Each of the three states gets a line saying which it is. The None line names its causes rather than the command that would carry the evidence. A `try polyswarm live result ` hint is tempting, since the dominant None case is the list route omitting the strings rather than fetching a blob per row. But this method renders both routes -- live feed loops over it -- and nothing on the resource tells them apart, so on a detail fetch the hint would say to re-run the command you just ran. Getting that right means threading a route flag into a new parameter on every BaseOutput implementation, which this does not earn. The four text cassette expectations are regenerated through click_vcr's own record path; the recorded HTTP is untouched. They predate the field, so they pin only the None line. --- src/polyswarm/formatters/text.py | 36 +++++++++++++ .../test_historical_hunt_results_text.click | 45 ++++------------- tests/vcr/test_live_feed_text.click | 50 ++++--------------- tests/vcr/test_live_result_delete_text.click | 23 ++------- tests/vcr/test_live_result_text.click | 27 +++------- 5 files changed, 69 insertions(+), 112 deletions(-) diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index 1810eba6..22a5855b 100644 --- a/src/polyswarm/formatters/text.py +++ b/src/polyswarm/formatters/text.py @@ -41,6 +41,40 @@ def _get_score_format(self, score): else: return self._red + # `matched_strings` is three-state -- see the SDK's specs/05-downstream-contract.md. + # None is deliberately NOT rendered as silence. The whole point of the feature is + # answering "why did this rule hit", and an absent line answers nothing: the user + # cannot tell it apart from a rule that matched on structure alone. Each state gets + # a line that says which it is. + # + # The None line names its CAUSES rather than the command that would carry the + # evidence. Tempting as `try \`polyswarm live result \`` is -- the dominant None + # case is the list route, which omits the strings rather than fetch a blob per row -- + # this method renders both routes (`live feed` loops over it), and nothing on the + # resource distinguishes them: `live_feed` and `live_result` both yield a + # LiveHuntResult. Without that distinction the hint is wrong on the detail route -- + # it would tell you to re-run the command you just ran -- and getting it right means + # threading a flag down from the command layer into a new parameter on every + # BaseOutput implementation (text, json, and all three hashes subclasses). Naming the + # causes is true on both routes and costs none of that. + def _matched_strings(self, strings): + if strings is None: + return [self._white('Matched Strings: unavailable — match data was not recorded ' + 'for this result, or it was omitted from a list view')] + if not strings: + 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"]} bytes' + if string.get('truncated'): + # The stored length is capped, so this is "there was more than this" -- + # never report it as an exact byte count. + size += ', truncated' + lines.append(self._white( + f' {string["identifier"]} @ 0x{string["offset"]:x} ({size}): {string["data"]}')) + return lines + def _output(self, output, write): if write: click.echo('\n'.join(output) + '\n', file=self.out) @@ -239,6 +273,7 @@ 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)) if result.download_url: output.append(self._white(f'Download Url: {result.download_url}')) return self._output(output, write) @@ -270,6 +305,7 @@ 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)) if result.download_url: output.append(self._white(f'Download Url: {result.download_url}')) return self._output(output, write) diff --git a/tests/vcr/test_historical_hunt_results_text.click b/tests/vcr/test_historical_hunt_results_text.click index 4fd7d7de..85566c4e 100644 --- a/tests/vcr/test_historical_hunt_results_text.click +++ b/tests/vcr/test_historical_hunt_results_text.click @@ -1,35 +1,10 @@ -result: 'Id: 87292527615907450 - - Instance Id: 55295687268401015 - - Created at: 2023-08-23 15:13:07.968061 - - SHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f - - Rule: eicar_substring_test - - PolyScore: 0.23213458159978606066 - - Detections: 1/1 engines reported malicious - - Tags: {} - - - Id: 624933739746082 - - Instance Id: 55295687268401015 - - Created at: 2023-08-23 15:13:07.968061 - - SHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f - - Rule: eicar_av_test - - PolyScore: 0.23213458159978606066 - - Detections: 1/1 engines reported malicious - - Tags: {} - - - ' +result: "Id: 87292527615907450\nInstance Id: 55295687268401015\nCreated at: 2023-08-23\ + \ 15:13:07.968061\nSHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f\n\ + Rule: eicar_substring_test\nPolyScore: 0.23213458159978606066\nDetections: 1/1 engines\ + \ reported malicious\nTags: {}\nMatched Strings: unavailable \u2014 match data was\ + \ not recorded for this result, or it was omitted from a list view\n\nId: 624933739746082\n\ + Instance Id: 55295687268401015\nCreated at: 2023-08-23 15:13:07.968061\nSHA256:\ + \ 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f\nRule: eicar_av_test\n\ + PolyScore: 0.23213458159978606066\nDetections: 1/1 engines reported malicious\n\ + Tags: {}\nMatched Strings: unavailable \u2014 match data was not recorded for this\ + \ result, or it was omitted from a list view\n\n" diff --git a/tests/vcr/test_live_feed_text.click b/tests/vcr/test_live_feed_text.click index dc5b706c..45b9b11f 100644 --- a/tests/vcr/test_live_feed_text.click +++ b/tests/vcr/test_live_feed_text.click @@ -1,39 +1,11 @@ -result: 'Id: 85163241984390761 - - Instance Id: 56151690517356729 - - Created at: 2023-09-18 22:55:01.418046 - - SHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f - - Rule: eicar_substring_test - - Malware Family: EICAR - - PolyScore: 0.23213458159978606066 - - Detections: 1/1 engines reported malicious - - Tags: {} - - - Id: 48317612869530221 - - Instance Id: 56151690517356729 - - Created at: 2023-09-18 22:55:01.411548 - - SHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f - - Rule: eicar_av_test - - Malware Family: EICAR - - PolyScore: 0.23213458159978606066 - - Detections: 1/1 engines reported malicious - - Tags: {} - - - ' +result: "Id: 85163241984390761\nInstance Id: 56151690517356729\nCreated at: 2023-09-18\ + \ 22:55:01.418046\nSHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f\n\ + Rule: eicar_substring_test\nMalware Family: EICAR\nPolyScore: 0.23213458159978606066\n\ + Detections: 1/1 engines reported malicious\nTags: {}\nMatched Strings: unavailable\ + \ \u2014 match data was not recorded for this result, or it was omitted from a list\ + \ view\n\nId: 48317612869530221\nInstance Id: 56151690517356729\nCreated at: 2023-09-18\ + \ 22:55:01.411548\nSHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f\n\ + Rule: eicar_av_test\nMalware Family: EICAR\nPolyScore: 0.23213458159978606066\n\ + Detections: 1/1 engines reported malicious\nTags: {}\nMatched Strings: unavailable\ + \ \u2014 match data was not recorded for this result, or it was omitted from a list\ + \ view\n\n" diff --git a/tests/vcr/test_live_result_delete_text.click b/tests/vcr/test_live_result_delete_text.click index 1301b863..51ae0ee6 100644 --- a/tests/vcr/test_live_result_delete_text.click +++ b/tests/vcr/test_live_result_delete_text.click @@ -1,18 +1,5 @@ -result: 'Id: 11704609705052856 - - Instance Id: 99734963618630386 - - Created at: 2022-05-26 19:41:33.797898 - - SHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f - - Rule: eicar_substring_test - - PolyScore: 0.23213458159978606066 - - Detections: 1/1 engines reported malicious - - Tags: {} - - - ' +result: "Id: 11704609705052856\nInstance Id: 99734963618630386\nCreated at: 2022-05-26\ + \ 19:41:33.797898\nSHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f\n\ + Rule: eicar_substring_test\nPolyScore: 0.23213458159978606066\nDetections: 1/1 engines\ + \ reported malicious\nTags: {}\nMatched Strings: unavailable \u2014 match data was\ + \ not recorded for this result, or it was omitted from a list view\n\n" diff --git a/tests/vcr/test_live_result_text.click b/tests/vcr/test_live_result_text.click index f383a34c..d94e099f 100644 --- a/tests/vcr/test_live_result_text.click +++ b/tests/vcr/test_live_result_text.click @@ -1,20 +1,7 @@ -result: 'Id: 11704609705052856 - - Instance Id: 99734963618630386 - - Created at: 2022-05-26 19:41:33.797898 - - SHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f - - Rule: eicar_substring_test - - PolyScore: 0.23213458159978606066 - - Detections: 1/1 engines reported malicious - - Tags: {} - - Download Url: http://minio:9000/cache-public/27/5a/02/275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f3395856ce81f2b7382dee72602f798b642f1414044d88612fea8a8f36de82e1278abb02f?response-content-disposition=attachment%3Bfilename%3Dinfected&response-content-type=application%2Foctet-stream&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIOSFODNN7EXAMPLE%2F20220526%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220526T194602Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host&X-Amz-Signature=e2e6744b301c36c3a5336816a48f2b27693a308b1c0f6aaab8121c3d88759041 - - - ' +result: "Id: 11704609705052856\nInstance Id: 99734963618630386\nCreated at: 2022-05-26\ + \ 19:41:33.797898\nSHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f\n\ + Rule: eicar_substring_test\nPolyScore: 0.23213458159978606066\nDetections: 1/1 engines\ + \ reported malicious\nTags: {}\nMatched Strings: unavailable \u2014 match data was\ + \ not recorded for this result, or it was omitted from a list view\nDownload Url:\ + \ http://minio:9000/cache-public/27/5a/02/275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f3395856ce81f2b7382dee72602f798b642f1414044d88612fea8a8f36de82e1278abb02f?response-content-disposition=attachment%3Bfilename%3Dinfected&response-content-type=application%2Foctet-stream&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIOSFODNN7EXAMPLE%2F20220526%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220526T194602Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host&X-Amz-Signature=e2e6744b301c36c3a5336816a48f2b27693a308b1c0f6aaab8121c3d88759041\n\ + \n" From 8beee88f7626f333843b5eb9bd664f0a7d34caba Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Fri, 21 Aug 2026 14:24:51 -0700 Subject: [PATCH 02/25] test(text): pin the three matched-strings renderings Style 3 -- the formatter driven directly with constructed resources, since the question is which line a field value produces, not command behaviour. The cassettes above reach only the None line, so they are not a substitute. Asserts the empty and absent lines stay different from each other, that truncation is marked only where it applies, and that the block sits between Tags and Download Url rather than being appended last. The absent-line assertions match substrings rather than the whole sentence, so rewording the prose does not turn this into a spelling check. --- tests/hunt_matched_strings_test.py | 122 +++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 tests/hunt_matched_strings_test.py diff --git a/tests/hunt_matched_strings_test.py b/tests/hunt_matched_strings_test.py new file mode 100644 index 00000000..407b3c2e --- /dev/null +++ b/tests/hunt_matched_strings_test.py @@ -0,0 +1,122 @@ +"""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. All three must produce a DIFFERENT, self-explaining line — the feature +exists to answer "why did this rule hit", and rendering `None` as silence answers +nothing while looking identical to a rule that had nothing to show. +""" +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): + """Just the matched-strings block: its header line plus the indented entries.""" + lines = text.splitlines() + start = next(i for i, line in enumerate(lines) if line.startswith('Matched Strings:')) + 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_names_its_causes(cls, method): + """A silent omission is indistinguishable from a rule that had nothing to show, so + the line must appear and must say WHY. It names causes rather than a command to run: + this same method renders `live feed` rows and single-result fetches alike.""" + line, = _matched_lines(_render(cls, method)) + # Substrings, not the whole sentence: the line must keep saying "unavailable" and + # must keep naming BOTH causes, but the prose around them is free to be reworded + # without this test turning into a spelling check. + assert 'unavailable' in line + assert 'not recorded' in line # the legacy-result / removed-data cause + assert 'list view' in line # the list-route cause + + +@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 + assert 'unavailable' not 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.""" + assert _matched_lines(_render(cls, method)) != \ + _matched_lines(_render(cls, method, matched_strings=[])) + + +@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 From e7677f4f18218228a6e776021fc53b3ecbf18cde Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Fri, 21 Aug 2026 14:24:52 -0700 Subject: [PATCH 03/25] docs(formatters): record the matched-strings rendering rule The spec asks for a resource's rendering rules once they are non-obvious or contested, and this one is both: it forbids rendering the absent state as silence, and it explains why the absent line names causes instead of a command to run, so the hint is not helpfully reintroduced later. --- specs/03-formatters.md | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/specs/03-formatters.md b/specs/03-formatters.md index b717ae16..bf3b3b43 100644 --- a/specs/03-formatters.md +++ b/specs/03-formatters.md @@ -146,3 +146,41 @@ here with no substitute. Both attributes ship in SDK **4.1.0**, but the dependen fail silently on 4.1.0 (see [`05-sdk-contract.md`](./05-sdk-contract.md) §Version pin) — so every supported install has them. `JSONOutput` needs no change — it dumps the resource's `.json`, which already carries the raw `state` and `known_good` keys. + +## 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 each state gets its own line. Rendering `None` as *silence* is the one thing this +section exists to forbid: the feature exists to answer "why did this rule hit", and an +absent line is indistinguishable from a rule that matched with nothing to show. + +| `matched_strings` | Rendered | +|---|---| +| `None` | `Matched Strings: unavailable — match data was not recorded for this result, or it was omitted from a list view` | +| `[]` | `Matched Strings: none — the rule matched without byte evidence (a structural or negative match, or private strings)` | +| `[…]` | `Matched Strings:` followed by one indented ` $ident @ 0xOFFSET (N bytes[, truncated]): DATA` line per entry | + +Two constraints on the `None` line: + +- **It names causes, not a command.** `try \`polyswarm live result \`` is tempting, + because the dominant `None` case is the list route omitting the evidence rather than + fetching a blob per row. But `live feed` loops over this *same* method, and nothing on + the resource tells the two apart — `live_feed` and `live_result` both yield a + `LiveHuntResult`. So on a detail fetch the hint would tell you to re-run the command you + just ran, and fixing that means threading a route flag from the command layer into a new + parameter on **every** `BaseOutput` implementation (`text`, `json`, all three `hashes` + subclasses). Naming the causes is true on both routes at no such cost. +- **`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. + +`JSONOutput` needs no change — it dumps the resource's `.json`, which already carries the +raw `matched_strings` key. + +Coverage is `tests/hunt_matched_strings_test.py` (Style 3 — the formatter driven directly +with constructed SDK resources). The `cli_test.py` cassettes predate the field, so they +exercise only the `None` line; they are not a substitute for those unit tests. From 1057b276a7966ed50ac145e78531fce1a189b19d Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Tue, 25 Aug 2026 10:20:53 -0700 Subject: [PATCH 04/25] fix(text): say nothing about matched strings when none were queried The absent state used to render a line explaining why the evidence was missing. On a list route that is a permanent false alarm: those endpoints never look for the strings, so every row of every page carried an explanation for a lookup that was never attempted. Absent now renders nothing at all. The case worth explaining survives untouched -- an empty list still says the rule matched with no byte evidence, and that one only ever reaches a detail route, so its line is never noise. This method renders both routes (the feed loops over it) and nothing on the resource distinguishes them, so the choice is per-state rather than per-route. Route awareness would mean threading a flag from the command layer into a new parameter on every BaseOutput implementation, for a line that is unwanted on one route and near-vestigial on the other: once the analyzer always reports strings, absent on a detail route means only that the result predates the feature. The absent-state test inverts rather than disappears -- it now asserts no line is emitted -- so this cannot be quietly undone. The four text cassette expectations are regenerated through click_vcr's own record path; the recorded HTTP is untouched. --- specs/03-formatters.md | 36 +++++++------ src/polyswarm/formatters/text.py | 33 ++++++------ tests/hunt_matched_strings_test.py | 41 +++++++-------- .../test_historical_hunt_results_text.click | 45 +++++++++++++---- tests/vcr/test_live_feed_text.click | 50 +++++++++++++++---- tests/vcr/test_live_result_delete_text.click | 23 +++++++-- tests/vcr/test_live_result_text.click | 27 +++++++--- 7 files changed, 170 insertions(+), 85 deletions(-) diff --git a/specs/03-formatters.md b/specs/03-formatters.md index bf3b3b43..03da19cc 100644 --- a/specs/03-formatters.md +++ b/specs/03-formatters.md @@ -154,26 +154,29 @@ every supported install has them. `JSONOutput` needs no change — it dumps the `TextOutput._matched_strings` helper. The attribute is **three-state** (the SDK's `05-downstream-contract.md` is authoritative) -and each state gets its own line. Rendering `None` as *silence* is the one thing this -section exists to forbid: the feature exists to answer "why did this rule hit", and an -absent line is indistinguishable from a rule that matched with nothing to show. +and the three must stay distinguishable in the output — but they do **not** each get a +line. | `matched_strings` | Rendered | |---|---| -| `None` | `Matched Strings: unavailable — match data was not recorded for this result, or it was omitted from a list view` | +| `None` | *nothing* — no line is emitted | | `[]` | `Matched Strings: none — the rule matched without byte evidence (a structural or negative match, or private strings)` | | `[…]` | `Matched Strings:` followed by one indented ` $ident @ 0xOFFSET (N bytes[, truncated]): DATA` line per entry | -Two constraints on the `None` line: - -- **It names causes, not a command.** `try \`polyswarm live result \`` is tempting, - because the dominant `None` case is the list route omitting the evidence rather than - fetching a blob per row. But `live feed` loops over this *same* method, and nothing on - the resource tells the two apart — `live_feed` and `live_result` both yield a - `LiveHuntResult`. So on a detail fetch the hint would tell you to re-run the command you - just ran, and fixing that means threading a route flag from the command layer into a new - parameter on **every** `BaseOutput` implementation (`text`, `json`, all three `hashes` - subclasses). Naming the causes is true on both routes at no such cost. +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. - **`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. @@ -182,5 +185,6 @@ Two constraints on the `None` line: raw `matched_strings` key. Coverage is `tests/hunt_matched_strings_test.py` (Style 3 — the formatter driven directly -with constructed SDK resources). The `cli_test.py` cassettes predate the field, so they -exercise only the `None` line; they are not a substitute for those unit tests. +with constructed SDK resources). 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. They are not a substitute for those unit tests. diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index 22a5855b..7a2b8016 100644 --- a/src/polyswarm/formatters/text.py +++ b/src/polyswarm/formatters/text.py @@ -42,25 +42,26 @@ def _get_score_format(self, score): return self._red # `matched_strings` is three-state -- see the SDK's specs/05-downstream-contract.md. - # None is deliberately NOT rendered as silence. The whole point of the feature is - # answering "why did this rule hit", and an absent line answers nothing: the user - # cannot tell it apart from a rule that matched on structure alone. Each state gets - # a line that says which it is. # - # The None line names its CAUSES rather than the command that would carry the - # evidence. Tempting as `try \`polyswarm live result \`` is -- the dominant None - # case is the list route, which omits the strings rather than fetch a blob per row -- - # this method renders both routes (`live feed` loops over it), and nothing on the - # resource distinguishes them: `live_feed` and `live_result` both yield a - # LiveHuntResult. Without that distinction the hint is wrong on the detail route -- - # it would tell you to re-run the command you just ran -- and getting it right means - # threading a flag down from the command layer into a new parameter on every - # BaseOutput implementation (text, json, and all three hashes subclasses). Naming the - # causes is true on both routes and costs none of that. + # None renders NOTHING, and that is a deliberate reversal of the obvious instinct + # ("say why the evidence is missing"). None overwhelmingly means "you are looking at + # a list route", which omits the strings rather than fetch a blob per row -- and a + # list route can NEVER carry them, so an explanation there is a permanent false alarm + # on every row rather than information. This method renders both routes (`live feed` + # loops over it) and nothing on the resource tells them apart: `live_feed` and + # `live_result` both yield a LiveHuntResult. Silence is the honest default. + # + # The case worth explaining survives: [] means the analyzer looked and the rule + # matched with no byte evidence -- a structural or negative match, or private strings. + # That only ever reaches a detail route, so its line is never noise, and collapsing it + # into the silent branch would throw away the distinction the server keeps. + # + # After this feature shipped the analyzer always sends `strings`, so None on a DETAIL + # route means a result predating it (or, eventually, removed evidence) -- genuinely + # nothing to say. def _matched_strings(self, strings): if strings is None: - return [self._white('Matched Strings: unavailable — match data was not recorded ' - 'for this result, or it was omitted from a list view')] + return [] if not strings: return [self._white('Matched Strings: none — the rule matched without byte ' 'evidence (a structural or negative match, or private strings)')] diff --git a/tests/hunt_matched_strings_test.py b/tests/hunt_matched_strings_test.py index 407b3c2e..f6bba209 100644 --- a/tests/hunt_matched_strings_test.py +++ b/tests/hunt_matched_strings_test.py @@ -6,9 +6,12 @@ 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. All three must produce a DIFFERENT, self-explaining line — the feature -exists to answer "why did this rule hit", and rendering `None` as silence answers -nothing while looking identical to a rule that had nothing to show. +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 @@ -51,9 +54,11 @@ def _render(cls, method, **extra): def _matched_lines(text): - """Just the matched-strings block: its header line plus the indented entries.""" + """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:')) + 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 @@ -61,17 +66,12 @@ def _matched_lines(text): @pytest.mark.parametrize('cls,method', PATHS) -def test_absent_names_its_causes(cls, method): - """A silent omission is indistinguishable from a rule that had nothing to show, so - the line must appear and must say WHY. It names causes rather than a command to run: - this same method renders `live feed` rows and single-result fetches alike.""" - line, = _matched_lines(_render(cls, method)) - # Substrings, not the whole sentence: the line must keep saying "unavailable" and - # must keep naming BOTH causes, but the prose around them is free to be reworded - # without this test turning into a spelling check. - assert 'unavailable' in line - assert 'not recorded' in line # the legacy-result / removed-data cause - assert 'list view' in line # the list-route cause +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) @@ -86,14 +86,15 @@ def test_empty_says_the_rule_matched_without_evidence(cls, method): line, = _matched_lines(_render(cls, method, matched_strings=[])) assert 'none' in line assert 'without byte evidence' in line - assert 'unavailable' not 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.""" - assert _matched_lines(_render(cls, method)) != \ - _matched_lines(_render(cls, method, matched_strings=[])) + """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) diff --git a/tests/vcr/test_historical_hunt_results_text.click b/tests/vcr/test_historical_hunt_results_text.click index 85566c4e..4fd7d7de 100644 --- a/tests/vcr/test_historical_hunt_results_text.click +++ b/tests/vcr/test_historical_hunt_results_text.click @@ -1,10 +1,35 @@ -result: "Id: 87292527615907450\nInstance Id: 55295687268401015\nCreated at: 2023-08-23\ - \ 15:13:07.968061\nSHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f\n\ - Rule: eicar_substring_test\nPolyScore: 0.23213458159978606066\nDetections: 1/1 engines\ - \ reported malicious\nTags: {}\nMatched Strings: unavailable \u2014 match data was\ - \ not recorded for this result, or it was omitted from a list view\n\nId: 624933739746082\n\ - Instance Id: 55295687268401015\nCreated at: 2023-08-23 15:13:07.968061\nSHA256:\ - \ 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f\nRule: eicar_av_test\n\ - PolyScore: 0.23213458159978606066\nDetections: 1/1 engines reported malicious\n\ - Tags: {}\nMatched Strings: unavailable \u2014 match data was not recorded for this\ - \ result, or it was omitted from a list view\n\n" +result: 'Id: 87292527615907450 + + Instance Id: 55295687268401015 + + Created at: 2023-08-23 15:13:07.968061 + + SHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f + + Rule: eicar_substring_test + + PolyScore: 0.23213458159978606066 + + Detections: 1/1 engines reported malicious + + Tags: {} + + + Id: 624933739746082 + + Instance Id: 55295687268401015 + + Created at: 2023-08-23 15:13:07.968061 + + SHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f + + Rule: eicar_av_test + + PolyScore: 0.23213458159978606066 + + Detections: 1/1 engines reported malicious + + Tags: {} + + + ' diff --git a/tests/vcr/test_live_feed_text.click b/tests/vcr/test_live_feed_text.click index 45b9b11f..dc5b706c 100644 --- a/tests/vcr/test_live_feed_text.click +++ b/tests/vcr/test_live_feed_text.click @@ -1,11 +1,39 @@ -result: "Id: 85163241984390761\nInstance Id: 56151690517356729\nCreated at: 2023-09-18\ - \ 22:55:01.418046\nSHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f\n\ - Rule: eicar_substring_test\nMalware Family: EICAR\nPolyScore: 0.23213458159978606066\n\ - Detections: 1/1 engines reported malicious\nTags: {}\nMatched Strings: unavailable\ - \ \u2014 match data was not recorded for this result, or it was omitted from a list\ - \ view\n\nId: 48317612869530221\nInstance Id: 56151690517356729\nCreated at: 2023-09-18\ - \ 22:55:01.411548\nSHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f\n\ - Rule: eicar_av_test\nMalware Family: EICAR\nPolyScore: 0.23213458159978606066\n\ - Detections: 1/1 engines reported malicious\nTags: {}\nMatched Strings: unavailable\ - \ \u2014 match data was not recorded for this result, or it was omitted from a list\ - \ view\n\n" +result: 'Id: 85163241984390761 + + Instance Id: 56151690517356729 + + Created at: 2023-09-18 22:55:01.418046 + + SHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f + + Rule: eicar_substring_test + + Malware Family: EICAR + + PolyScore: 0.23213458159978606066 + + Detections: 1/1 engines reported malicious + + Tags: {} + + + Id: 48317612869530221 + + Instance Id: 56151690517356729 + + Created at: 2023-09-18 22:55:01.411548 + + SHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f + + Rule: eicar_av_test + + Malware Family: EICAR + + PolyScore: 0.23213458159978606066 + + Detections: 1/1 engines reported malicious + + Tags: {} + + + ' diff --git a/tests/vcr/test_live_result_delete_text.click b/tests/vcr/test_live_result_delete_text.click index 51ae0ee6..1301b863 100644 --- a/tests/vcr/test_live_result_delete_text.click +++ b/tests/vcr/test_live_result_delete_text.click @@ -1,5 +1,18 @@ -result: "Id: 11704609705052856\nInstance Id: 99734963618630386\nCreated at: 2022-05-26\ - \ 19:41:33.797898\nSHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f\n\ - Rule: eicar_substring_test\nPolyScore: 0.23213458159978606066\nDetections: 1/1 engines\ - \ reported malicious\nTags: {}\nMatched Strings: unavailable \u2014 match data was\ - \ not recorded for this result, or it was omitted from a list view\n\n" +result: 'Id: 11704609705052856 + + Instance Id: 99734963618630386 + + Created at: 2022-05-26 19:41:33.797898 + + SHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f + + Rule: eicar_substring_test + + PolyScore: 0.23213458159978606066 + + Detections: 1/1 engines reported malicious + + Tags: {} + + + ' diff --git a/tests/vcr/test_live_result_text.click b/tests/vcr/test_live_result_text.click index d94e099f..f383a34c 100644 --- a/tests/vcr/test_live_result_text.click +++ b/tests/vcr/test_live_result_text.click @@ -1,7 +1,20 @@ -result: "Id: 11704609705052856\nInstance Id: 99734963618630386\nCreated at: 2022-05-26\ - \ 19:41:33.797898\nSHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f\n\ - Rule: eicar_substring_test\nPolyScore: 0.23213458159978606066\nDetections: 1/1 engines\ - \ reported malicious\nTags: {}\nMatched Strings: unavailable \u2014 match data was\ - \ not recorded for this result, or it was omitted from a list view\nDownload Url:\ - \ http://minio:9000/cache-public/27/5a/02/275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f3395856ce81f2b7382dee72602f798b642f1414044d88612fea8a8f36de82e1278abb02f?response-content-disposition=attachment%3Bfilename%3Dinfected&response-content-type=application%2Foctet-stream&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIOSFODNN7EXAMPLE%2F20220526%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220526T194602Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host&X-Amz-Signature=e2e6744b301c36c3a5336816a48f2b27693a308b1c0f6aaab8121c3d88759041\n\ - \n" +result: 'Id: 11704609705052856 + + Instance Id: 99734963618630386 + + Created at: 2022-05-26 19:41:33.797898 + + SHA256: 275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f + + Rule: eicar_substring_test + + PolyScore: 0.23213458159978606066 + + Detections: 1/1 engines reported malicious + + Tags: {} + + Download Url: http://minio:9000/cache-public/27/5a/02/275a021bbfb6489e54d471899f7db9d1663fc695ec2fe2a2c4538aabf651fd0f3395856ce81f2b7382dee72602f798b642f1414044d88612fea8a8f36de82e1278abb02f?response-content-disposition=attachment%3Bfilename%3Dinfected&response-content-type=application%2Foctet-stream&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIOSFODNN7EXAMPLE%2F20220526%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20220526T194602Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host&X-Amz-Signature=e2e6744b301c36c3a5336816a48f2b27693a308b1c0f6aaab8121c3d88759041 + + + ' From 0d04002c93ee8e57e7cd9c2df979ec76aaadadad Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Tue, 25 Aug 2026 12:17:28 -0700 Subject: [PATCH 05/25] docs(text): tighten the matched-strings rendering comment Comment text only -- the parsed AST is identical before and after. The largest cut of the pass, because most of what was there argued against a design that is no longer in the code: a hint naming the command that would carry the evidence, which was tried, found to be circular on the detail route, and dropped. The reasoning for the choice that was actually made -- absent renders nothing, empty keeps its line -- is what remains. --- src/polyswarm/formatters/text.py | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index 7a2b8016..72bb38f4 100644 --- a/src/polyswarm/formatters/text.py +++ b/src/polyswarm/formatters/text.py @@ -41,24 +41,13 @@ def _get_score_format(self, score): else: return self._red - # `matched_strings` is three-state -- see the SDK's specs/05-downstream-contract.md. + # Three-state -- see the SDK's specs/05-downstream-contract.md. # - # None renders NOTHING, and that is a deliberate reversal of the obvious instinct - # ("say why the evidence is missing"). None overwhelmingly means "you are looking at - # a list route", which omits the strings rather than fetch a blob per row -- and a - # list route can NEVER carry them, so an explanation there is a permanent false alarm - # on every row rather than information. This method renders both routes (`live feed` - # loops over it) and nothing on the resource tells them apart: `live_feed` and - # `live_result` both yield a LiveHuntResult. Silence is the honest default. - # - # The case worth explaining survives: [] means the analyzer looked and the rule - # matched with no byte evidence -- a structural or negative match, or private strings. - # That only ever reaches a detail route, so its line is never noise, and collapsing it - # into the silent branch would throw away the distinction the server keeps. - # - # After this feature shipped the analyzer always sends `strings`, so None on a DETAIL - # route means a result predating it (or, eventually, removed evidence) -- genuinely - # nothing to say. + # None renders NOTHING, reversing the instinct to explain the absence: None almost + # always means "list route", which can never carry strings, so a line there is a false + # alarm on every row. `live feed` loops over this same method and nothing on the + # resource tells the routes apart. [] keeps its line -- it only reaches a detail route, + # where "matched, no byte evidence" is a real answer. def _matched_strings(self, strings): if strings is None: return [] From 2db3fab05e58fe89a714f2503aa6c977a1e0f486 Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Tue, 25 Aug 2026 14:35:55 -0700 Subject: [PATCH 06/25] fix(text): read matched_strings defensively, and subscript truncated `result.matched_strings` was a bare attribute read while the dependency pin still admits SDKs that predate the attribute, so on any released SDK in range it raises AttributeError -- and not only on the new output. All six text-mode hunt commands funnel through these two formatter methods, so `live feed`, `live result`, `live results-delete` and the three historical equivalents would all break. Now read with getattr, matching the defence the known-good attributes in this same file have used since they landed, and for the same reason. A missing attribute lands on the silent None branch, which is also the honest reading: an SDK that cannot see the field does not know. CI could not have caught this. The paired SDK branch shares this branch's name, so the archive install resolves to it and the feature-branch SDK gets installed -- green here means the paired-PR mechanism works, not that the pin is right. Nor could the existing tests: they build resources from the installed SDK, so a bare read passes all of them. The new test deletes the attribute to stand in for an older SDK and is the only guard; it fails with AttributeError against the previous code. Also subscripts `truncated` like the other four keys. Mixing a subscript for `length` with `.get()` for `truncated` defended against a shape the contract says cannot occur while a genuinely partial entry still raised two lines earlier. Unrelated and pre-existing: the specs still named `>=4.2.0` as the dependency floor, which moved to 4.3.0 in cdb7926 without a spec change. Corrected, with the provenance recorded -- the per-behaviour writeup below it is still the 4.2.0 one, accurate about why 4.2.0 was needed but no longer the binding constraint. --- specs/03-formatters.md | 21 +++++++++++++++++---- specs/05-sdk-contract.md | 9 ++++++++- src/polyswarm/formatters/text.py | 14 +++++++++----- tests/hunt_matched_strings_test.py | 19 +++++++++++++++++++ 4 files changed, 53 insertions(+), 10 deletions(-) diff --git a/specs/03-formatters.md b/specs/03-formatters.md index 03da19cc..ae83b202 100644 --- a/specs/03-formatters.md +++ b/specs/03-formatters.md @@ -141,10 +141,9 @@ Both attributes are read with `getattr(..., None)` so a CLI on an older SDK (mis either field) never raises `AttributeError`; an SDK without `.state` simply never takes the known-good branch, which is the safe fallback — the pre-known-good rendering. That degradation is belt-and-braces, not a supported configuration: `.state` is load-bearing -here with no substitute. Both attributes ship in SDK **4.1.0**, but the dependency floor is -`polyswarm_api>=4.2.0` — set by two *other* behaviours the CLI depends on, both of which -fail silently on 4.1.0 (see [`05-sdk-contract.md`](./05-sdk-contract.md) §Version pin) — so -every supported install has them. `JSONOutput` needs no change — it dumps the resource's +here with no substitute. Both attributes ship in SDK **4.1.0**, well under the dependency floor +(`polyswarm_api>=4.3.0`; see [`05-sdk-contract.md`](./05-sdk-contract.md) §Current floor), +so every supported install has them. `JSONOutput` needs no change — it dumps the resource's `.json`, which already carries the raw `state` and `known_good` keys. ## Matched strings on hunt results @@ -181,6 +180,20 @@ Two constraints, both counter-intuitive enough to be worth stating: marker means "there was more than this" and over-reports at exactly the cap. Never render it as an exact size. +**Read with `getattr(result, 'matched_strings', None)`, never a bare attribute access** — +the same defence, for the same reason, as the known-good attributes above. The attribute +ships in the paired SDK release, but the dependency floor admits older SDKs whose +resources lack it entirely, and a bare read would `AttributeError` on *every* text-mode +hunt command, not just the new output: `live result` / `live feed` / `live results-delete` +and the three `historical` equivalents all funnel through these two methods. A missing +attribute degrades to the silent `None` branch, which is also the honest reading — an SDK +that cannot see the field genuinely does not know. + +Nothing else can catch this. The rendering tests build resources from the *installed* +SDK, so with a paired SDK on the path a bare read passes every one of them; +`test_an_sdk_without_the_attribute_does_not_raise` deletes the attribute to stand in for +an older SDK, and is the only guard. + `JSONOutput` needs no change — it dumps the resource's `.json`, which already carries the raw `matched_strings` key. diff --git a/specs/05-sdk-contract.md b/specs/05-sdk-contract.md index 3957a916..0804ed2f 100644 --- a/specs/05-sdk-contract.md +++ b/specs/05-sdk-contract.md @@ -74,7 +74,14 @@ 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. For the current floor both were read from `origin/develop`: `version = "4.2.0"` and `__version__ = '4.2.0'`, no suffix. -### Current floor — `polyswarm_api>=4.2.0` +### Current floor — `polyswarm_api>=4.3.0` + +`pyproject.toml` floors at **4.3.0**, raised from 4.2.0 by `cdb7926` for the typed +known-good refusal (`KnownGoodWithheldException`, absent in 4.2.0) and the probe fixes. +That commit touched no spec, so the per-behaviour writeup below is still the **4.2.0** +one; it remains accurate about why 4.2.0 was needed, it is simply no longer the binding +constraint. Anyone raising the floor again should extend this section rather than +replace it. Two behaviours the CLI relies on only exist from **4.2.0**; on 4.1.0 both fail *silently*, which is why the floor is a hard requirement rather than a preference: diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index 72bb38f4..f5352dad 100644 --- a/src/polyswarm/formatters/text.py +++ b/src/polyswarm/formatters/text.py @@ -57,9 +57,9 @@ def _matched_strings(self, strings): lines = [self._white('Matched Strings:')] for string in strings: size = f'{string["length"]} bytes' - if string.get('truncated'): - # The stored length is capped, so this is "there was more than this" -- - # never report it as an exact byte count. + if string['truncated']: + # Subscripted like the other four: fail loudly on a partial entry + # rather than render half-right. size += ', truncated' lines.append(self._white( f' {string["identifier"]} @ 0x{string["offset"]:x} ({size}): {string["data"]}')) @@ -263,7 +263,9 @@ 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)) + # getattr: the pin admits SDKs predating this attribute -- same defence as the + # known-good reads below. Missing lands on the silent None branch. + output.extend(self._matched_strings(getattr(result, 'matched_strings', None))) if result.download_url: output.append(self._white(f'Download Url: {result.download_url}')) return self._output(output, write) @@ -295,7 +297,9 @@ 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)) + # getattr: the pin admits SDKs predating this attribute -- same defence as the + # known-good reads below. Missing lands on the silent None branch. + output.extend(self._matched_strings(getattr(result, 'matched_strings', None))) if result.download_url: output.append(self._white(f'Download Url: {result.download_url}')) return self._output(output, write) diff --git a/tests/hunt_matched_strings_test.py b/tests/hunt_matched_strings_test.py index f6bba209..1bd71ad5 100644 --- a/tests/hunt_matched_strings_test.py +++ b/tests/hunt_matched_strings_test.py @@ -121,3 +121,22 @@ def test_block_sits_between_tags_and_download_url(cls, method): 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_an_sdk_without_the_attribute_does_not_raise(cls, method): + """The dependency pin admits SDKs predating `matched_strings`, and nothing else here + would catch a bare `result.matched_strings`. + + Every other test in this file builds resources from the INSTALLED SDK, so with a + paired SDK on the path a bare attribute read passes all of them and then + AttributeErrors in the field -- on every text-mode hunt command, not just the new + output. Deleting the attribute is what an older SDK's resource looks like. + """ + content = dict(_COMMON) + content['livescan_id' if cls is resources.LiveHuntResult else 'historicalscan_id'] = 3 + result = cls(content) + del result.matched_strings + rendered = click.unstyle('\n'.join(getattr(TextOutput(color=False), method)(result, write=False))) + assert 'Matched Strings' not in rendered + assert 'Rule: dos_stub_message' in rendered # the rest of the row still renders From 39764927e9160e4605085b1066688feeba1a804d Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Fri, 28 Aug 2026 14:07:33 -0700 Subject: [PATCH 07/25] feat(text): tell the user when matched strings were withheld MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renders a final line inside the block when the server reports a withheld count: … 19 more not shown (result size limit) Yellow rather than white -- it is the one line here reporting something the platform did not send. Omitted entirely when nothing was withheld. 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, which is the same wrong-inference class the three-state contract already exists to prevent, one level down. Read with getattr for the same reason as `matched_strings` itself: the dependency floor admits SDKs predating the field, and a bare attribute read would AttributeError on every text-mode hunt command rather than just the new output. A test deletes the attribute to stand in for an older SDK. 142 tests pass. --- specs/03-formatters.md | 19 ++++++++++++++- src/polyswarm/formatters/text.py | 16 +++++++++--- tests/hunt_matched_strings_test.py | 39 ++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/specs/03-formatters.md b/specs/03-formatters.md index ae83b202..d505e648 100644 --- a/specs/03-formatters.md +++ b/specs/03-formatters.md @@ -194,8 +194,25 @@ SDK, so with a paired SDK on the path a bare read passes every one of them; `test_an_sdk_without_the_attribute_does_not_raise` deletes the attribute to stand in for an older SDK, and is the only guard. +### 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`. + +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. Read with +`getattr(..., None)` for the same SDK-floor reason as `matched_strings` itself. + `JSONOutput` needs no change — it dumps the resource's `.json`, which already carries the -raw `matched_strings` key. +raw `matched_strings` and `matched_strings_dropped` keys. Coverage is `tests/hunt_matched_strings_test.py` (Style 3 — the formatter driven directly with constructed SDK resources). The `cli_test.py` cassettes predate the field, so every result they diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index f5352dad..106198cf 100644 --- a/src/polyswarm/formatters/text.py +++ b/src/polyswarm/formatters/text.py @@ -48,7 +48,7 @@ def _get_score_format(self, score): # alarm on every row. `live feed` loops over this same method and nothing on the # resource tells the routes apart. [] keeps its line -- it only reaches a detail route, # where "matched, no byte evidence" is a real answer. - def _matched_strings(self, strings): + def _matched_strings(self, strings, dropped=None): if strings is None: return [] if not strings: @@ -63,6 +63,12 @@ def _matched_strings(self, strings): size += ', truncated' lines.append(self._white( f' {string["identifier"]} @ 0x{string["offset"]:x} ({size}): {string["data"]}')) + if dropped: + # Without this the list reads as the whole truth, and a user concludes their + # rule hit N times when it hit N + dropped. Yellow, not white: it is the one + # line here reporting something the platform withheld. + lines.append(self._yellow( + f' … {dropped} more not shown (result size limit)')) return lines def _output(self, output, write): @@ -265,7 +271,9 @@ def historical_result(self, result, write=True): output.append(self._white(f'Tags: {result.tags}')) # getattr: the pin admits SDKs predating this attribute -- same defence as the # known-good reads below. Missing lands on the silent None branch. - output.extend(self._matched_strings(getattr(result, 'matched_strings', None))) + output.extend(self._matched_strings( + getattr(result, 'matched_strings', None), + getattr(result, 'matched_strings_dropped', None))) if result.download_url: output.append(self._white(f'Download Url: {result.download_url}')) return self._output(output, write) @@ -299,7 +307,9 @@ def live_result(self, result, write=True): output.append(self._white(f'Tags: {result.tags}')) # getattr: the pin admits SDKs predating this attribute -- same defence as the # known-good reads below. Missing lands on the silent None branch. - output.extend(self._matched_strings(getattr(result, 'matched_strings', None))) + output.extend(self._matched_strings( + getattr(result, 'matched_strings', None), + getattr(result, 'matched_strings_dropped', None))) if result.download_url: output.append(self._white(f'Download Url: {result.download_url}')) return self._output(output, write) diff --git a/tests/hunt_matched_strings_test.py b/tests/hunt_matched_strings_test.py index 1bd71ad5..9077adfb 100644 --- a/tests/hunt_matched_strings_test.py +++ b/tests/hunt_matched_strings_test.py @@ -140,3 +140,42 @@ def test_an_sdk_without_the_attribute_does_not_raise(cls, method): rendered = click.unstyle('\n'.join(getattr(TextOutput(color=False), method)(result, write=False))) assert 'Matched Strings' not in rendered assert 'Rule: dos_stub_message' in rendered # the rest of the row still renders + + +@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_older_sdk_without_the_dropped_attribute_does_not_raise(cls, method): + """Same pin as matched_strings: the dependency floor admits SDKs without it.""" + content = dict(_COMMON, matched_strings=_STRINGS) + content['livescan_id' if cls is resources.LiveHuntResult else 'historicalscan_id'] = 3 + result = cls(content) + del result.matched_strings_dropped + rendered = click.unstyle('\n'.join(getattr(TextOutput(color=False), method)(result, write=False))) + assert 'Matched Strings:' in rendered + assert 'not shown' not in rendered From 0a43d2d5069145831a076246ee5264639baba999 Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Sat, 29 Aug 2026 10:04:10 -0700 Subject: [PATCH 08/25] fix(text): do not claim "no byte evidence" while discarding a withheld count The empty branch returned early, before the dropped check. So an empty list arriving with a non-zero count printed "the rule matched without byte evidence" and silently dropped the number -- a confident false statement about the rule, and the precise wrong inference the count was added to prevent. That combination should be unreachable: the analyzer keeps a match's first string, so [] implies dropped == 0. But the renderer was trusting an invariant owned by another repo while making a positive claim, in the same function that defends with getattr against shapes it does not control. Now it reports what is certain -- "none shown (N withheld, result size limit)" -- and asserts nothing about why the rule fired. The normal empty case is unchanged. Also records in 05-sdk-contract.md that the floor does not yet cover matched_strings / matched_strings_dropped. Not bumping it is right -- the SDK carrying them is not on PyPI, so neither precondition in the version-pin section is met -- but with no version written down nothing would later prompt the decision of whether the graceful degradation is still wanted. 146 tests pass. --- specs/03-formatters.md | 7 +++++++ specs/05-sdk-contract.md | 9 +++++++++ src/polyswarm/formatters/text.py | 8 ++++++++ tests/hunt_matched_strings_test.py | 21 +++++++++++++++++++++ 4 files changed, 45 insertions(+) diff --git a/specs/03-formatters.md b/specs/03-formatters.md index d505e648..9dfb78d6 100644 --- a/specs/03-formatters.md +++ b/specs/03-formatters.md @@ -206,6 +206,13 @@ block, in **yellow** rather than white: 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. Read with diff --git a/specs/05-sdk-contract.md b/specs/05-sdk-contract.md index 0804ed2f..82a79fcf 100644 --- a/specs/05-sdk-contract.md +++ b/specs/05-sdk-contract.md @@ -90,6 +90,15 @@ Two behaviours the CLI relies on only exist from **4.2.0**; on 4.1.0 both fail * The known-good rendering attributes (`ArtifactInstance.state`, `.known_good`/`.known_good_sources`, read by `formatters/text.py` — see [`03-formatters.md`](./03-formatters.md) §Known-good artifact instances) ship in **4.1.0**, so they are *not* what sets the floor; they are simply covered by it. +`matched_strings` / `matched_strings_dropped` are the opposite case and the floor does +**not** yet cover them: they are unreleased at the time of writing, so the CLI reads both +with `getattr(..., None)` and degrades to rendering nothing (see +[`03-formatters.md`](./03-formatters.md) §Matched strings). Raising the floor is not +possible until the SDK carrying them is on PyPI — §Version pin's two preconditions — so +**record the version here once it releases**, and only then decide whether the graceful +degradation is still wanted or the floor should move. Without a version written down +nothing prompts that decision. + ## Worked example — the httpx SDK migration The SDK's move to an `httpx`-based, three-layer architecture (pure-dataclass `PolyswarmRequest`, session-based execution, lazy generators) removed several 3.x affordances the CLI had reached into: diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index 106198cf..e5ee5eff 100644 --- a/src/polyswarm/formatters/text.py +++ b/src/polyswarm/formatters/text.py @@ -52,6 +52,14 @@ def _matched_strings(self, strings, dropped=None): if strings is None: return [] if not strings: + if dropped: + # Should be unreachable -- the analyzer keeps a match's first string, so an + # empty list with a non-zero count is contradictory. Report only what is + # certain: asserting "matched without byte evidence" here would be a false + # claim about the RULE, and silently discarding the count is the exact + # wrong inference this line exists to prevent. + return [self._yellow( + f'Matched Strings: none shown ({dropped} 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:')] diff --git a/tests/hunt_matched_strings_test.py b/tests/hunt_matched_strings_test.py index 9077adfb..2d92ac7b 100644 --- a/tests/hunt_matched_strings_test.py +++ b/tests/hunt_matched_strings_test.py @@ -179,3 +179,24 @@ def test_older_sdk_without_the_dropped_attribute_does_not_raise(cls, method): rendered = click.unstyle('\n'.join(getattr(TextOutput(color=False), method)(result, write=False))) assert 'Matched Strings:' in rendered assert 'not shown' 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 From 2e2212a0c94c75f3c9e72ee65e1766230f92a3c6 Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Sat, 29 Aug 2026 15:32:11 -0700 Subject: [PATCH 09/25] fix(text): sanitise matched-string data, and fix the older-SDK tests Three things, the first two of which were broken rather than merely untidy. The two "older SDK" tests errored on exactly the SDKs they modelled. Both did `del result.matched_strings[_dropped]`, and the attribute is set in __init__ -- so on an SDK that never set it, `del` raises AttributeError and the test errors instead of passing. That configuration is inside the declared pin: 4.3.0 is released without these fields, so `pip install .[tests] && pytest` against the floor errored on those two and failed a further ten. Now popped from __dict__, which works either way, with a module-level skip so a run against a floor SDK skips honestly rather than failing wholesale. CI was green throughout only because the paired SDK branch shares this branch's name. `data` is now sanitised before rendering. It is the one field in a hunt result derived from the sample, so it is attacker-controlled end to end, and it was interpolated into terminal output unescaped. yara escapes non-printables and the analyzer preserves that rendering, so the substitution is a no-op on anything valid -- it exists because that guarantee lives in another repo, and a raw CSI sequence reaching a terminal would repaint or clear an analyst's screen. Trusting an upstream promise is not the same as holding one. The em dash and ellipsis were the first non-ASCII characters TextOutput emitted; under a C/POSIX locale stdout degrades them to '?'. Replaced with ASCII and pinned by a test. Also records in specs/03 why the attribute is read with getattr while the keys inside an entry are subscripted -- they answer different questions, version skew versus a producer violating its own contract -- since side by side they read as an inconsistency. And 99-open-questions.md now carries the deferred floor decision, including that raising the floor is the trigger to drop both the getattr defence and the new skip guard. 152 tests pass. --- specs/03-formatters.md | 14 +++++++ specs/99-open-questions.md | 14 +++++++ src/polyswarm/formatters/text.py | 20 ++++++++-- tests/hunt_matched_strings_test.py | 62 ++++++++++++++++++++++++++++-- 4 files changed, 104 insertions(+), 6 deletions(-) diff --git a/specs/03-formatters.md b/specs/03-formatters.md index 9dfb78d6..a5f28b82 100644 --- a/specs/03-formatters.md +++ b/specs/03-formatters.md @@ -176,6 +176,20 @@ Two constraints, both counter-intuitive enough to be worth stating: 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 attribute is defended; the entry keys are not, and that is deliberate.** + `matched_strings` / `matched_strings_dropped` are read with `getattr(..., None)` because + the *dependency floor* admits SDKs without them — a version-skew problem. The keys + *inside* an entry (`identifier`, `offset`, `length`, `data`, `truncated`) are + subscripted, 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 two look inconsistent side by side and are answering different questions. +- **`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.** `TextOutput` emits no non-ASCII; stdout under a C/POSIX locale replaces + it with `?`. `test_output_is_ascii_only` pins that. - **`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. diff --git a/specs/99-open-questions.md b/specs/99-open-questions.md index be530112..27f25123 100644 --- a/specs/99-open-questions.md +++ b/specs/99-open-questions.md @@ -33,3 +33,17 @@ The SDK pin and the paired-PR `## Requires` convention ([`05-sdk-contract.md`](. **Status:** open. The `Polyswarm(PolyswarmAPI)` wrapper holds CLI-only orchestration (parallel fan-out, multi-step flows). Some of it (e.g. `submit_url`'s inline `/instance/url` endpoint) arguably belongs in the SDK so the CLI is a pure wrapper. **Action:** as the SDK grows methods that subsume wrapper logic, migrate the wrapper to call them and shrink the CLI-owned surface. + +## SDK floor for the matched-strings attributes + +**Status:** blocked on a release. + +`matched_strings` / `matched_strings_dropped` are not covered by the dependency floor — +the SDK carrying them is not on PyPI, so neither precondition in +[`05-sdk-contract.md`](./05-sdk-contract.md) §Version pin is met. The CLI reads both with +`getattr(..., None)` and degrades to rendering nothing, and +`tests/hunt_matched_strings_test.py` carries a module-level skip for the same reason. + +**Action once the SDK releases:** record the version in `05-sdk-contract.md` §Current +floor, decide whether to raise the floor past it, and if so drop *both* the `getattr` +defence and the test skip — they exist only to tolerate SDKs the floor still admits. diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index e5ee5eff..bd1933df 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,19 @@ def pretty_print_datetime(value): return datetime.strftime(value, '%Y-%m-%d %H:%M:%S UTC') +# Matched-string `data` is the one field in a hunt result derived from the SAMPLE, so it +# is attacker-controlled end to end. yara escapes non-printables before we ever see it and +# the analyzer keeps that rendering verbatim, so valid data is already printable ASCII and +# this is a no-op on it. It exists because that guarantee lives in ANOTHER repo: if it ever +# slips, a raw CSI sequence here would repaint or clear the analyst's terminal. +_CONTROL_CHARS = re.compile(r'[\x00-\x1f\x7f]') + + +def _safe_data(value): + """Matched bytes as yara rendered them, with any control character neutralised.""" + return _CONTROL_CHARS.sub('.', value) + + def is_grouped(fn): @functools.wraps(fn) def wrapper(self, text): @@ -60,7 +74,7 @@ def _matched_strings(self, strings, dropped=None): # wrong inference this line exists to prevent. return [self._yellow( f'Matched Strings: none shown ({dropped} withheld, result size limit)')] - return [self._white('Matched Strings: none — the rule matched without byte ' + 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: @@ -70,13 +84,13 @@ def _matched_strings(self, strings, dropped=None): # rather than render half-right. size += ', truncated' lines.append(self._white( - f' {string["identifier"]} @ 0x{string["offset"]:x} ({size}): {string["data"]}')) + f' {string["identifier"]} @ 0x{string["offset"]:x} ({size}): {_safe_data(string["data"])}')) if dropped: # Without this the list reads as the whole truth, and a user concludes their # rule hit N times when it hit N + dropped. Yellow, not white: it is the one # line here reporting something the platform withheld. lines.append(self._yellow( - f' … {dropped} more not shown (result size limit)')) + f' ... {dropped} more not shown (result size limit)')) return lines def _output(self, output, write): diff --git a/tests/hunt_matched_strings_test.py b/tests/hunt_matched_strings_test.py index 2d92ac7b..68d60a00 100644 --- a/tests/hunt_matched_strings_test.py +++ b/tests/hunt_matched_strings_test.py @@ -39,6 +39,24 @@ ] # (resource class, formatter method name) +def _sdk_carries_the_fields(): + """Whether the installed SDK parses the attributes this module renders. + + The declared pin (`polyswarm_api>=4.3.0`) still admits SDKs predating them -- 4.3.0 + itself is released without them -- so `pip install .[tests] && pytest` against the + floor would fail this module wholesale. CI resolves the paired SDK branch and runs it + for real. Remove this guard when the floor is raised past the release that adds them + (specs/05-sdk-contract.md, §Current floor). + """ + probe = resources.LiveHuntResult(dict(_COMMON, livescan_id=3)) + return hasattr(probe, 'matched_strings') and hasattr(probe, 'matched_strings_dropped') + + +pytestmark = pytest.mark.skipif( + not _sdk_carries_the_fields(), + reason='installed SDK predates matched_strings / matched_strings_dropped') + + PATHS = [ (resources.LiveHuntResult, 'live_result'), (resources.HistoricalHuntResult, 'historical_result'), @@ -136,7 +154,11 @@ def test_an_sdk_without_the_attribute_does_not_raise(cls, method): content = dict(_COMMON) content['livescan_id' if cls is resources.LiveHuntResult else 'historicalscan_id'] = 3 result = cls(content) - del result.matched_strings + # pop, not `del`: on an SDK that never SET the attribute -- exactly the configuration + # this test models, and one inside the declared pin -- `del` raises AttributeError and + # the test errors instead of passing. + result.__dict__.pop('matched_strings', None) + assert not hasattr(result, 'matched_strings') rendered = click.unstyle('\n'.join(getattr(TextOutput(color=False), method)(result, write=False))) assert 'Matched Strings' not in rendered assert 'Rule: dos_stub_message' in rendered # the rest of the row still renders @@ -151,7 +173,7 @@ def test_dropped_count_is_reported_to_the_user(cls, method): """ lines = _matched_lines(_render(cls, method, matched_strings=_STRINGS, matched_strings_dropped=19)) - assert lines[-1].strip().startswith('…') + assert lines[-1].strip().startswith('...') assert '19 more not shown' in lines[-1] @@ -175,7 +197,8 @@ def test_older_sdk_without_the_dropped_attribute_does_not_raise(cls, method): content = dict(_COMMON, matched_strings=_STRINGS) content['livescan_id' if cls is resources.LiveHuntResult else 'historicalscan_id'] = 3 result = cls(content) - del result.matched_strings_dropped + result.__dict__.pop('matched_strings_dropped', None) # see the sibling test: not `del` + assert not hasattr(result, 'matched_strings_dropped') rendered = click.unstyle('\n'.join(getattr(TextOutput(color=False), method)(result, write=False))) assert 'Matched Strings:' in rendered assert 'not shown' not in rendered @@ -200,3 +223,36 @@ 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. + """ + hostile = [{'offset': 0, 'identifier': '$evil', 'length': 9, + 'data': 'A\x1b[2JB\r\nC\x00D', 'truncated': False}] + rendered = _render(cls, method, matched_strings=hostile) + assert '\x1b' not in rendered and chr(27) not in rendered + assert '\r' not in rendered and '\x00' not in rendered + # the surviving printable bytes still render, so a legitimate match is unharmed + assert 'A.[2JB..C.D' 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()] From 153bb543967c392ba5c67f2b1321a60ba2e0eb4a Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Sat, 29 Aug 2026 23:21:59 -0700 Subject: [PATCH 10/25] fix(text): close the C1 hole in the sanitiser, and stop skipping the guard Two defects in the previous commit, both of which left a stated protection not actually in place. The sanitiser blacklisted [\x00-\x1f\x7f] -- C0 and DEL -- and let U+0080-U+009F through. U+009B IS the 8-bit CSI, honoured by xterm and VTE in UTF-8 mode, so `\x9b2J` still cleared the terminal: a hole in the precise byte the filter was written to block. The threat model is that the upstream escaping guarantee lives in another repo and may slip, and bytes that slip do not stay conveniently in the low range. Now a printable-ASCII whitelist, which also makes the ASCII-only rule true of this field rather than only of the literals around it. The test grows a \x9b case and a non-ASCII one. The module-level skipif took the two older-SDK tests out of the only install where they guard anything. They build a resource and pop the attribute off, so they pass fine on a floor SDK -- and on that SDK a regression to a bare attribute read would have skipped green and failed only in the field. The previous commit called one of them "the only guard" while disabling it there. The mark is now applied per-test, to the fifteen that genuinely need the fields. Also corrects the ASCII claim in specs/03, which asserted TextOutput emits no non-ASCII: server-supplied rule_name and tags pass through unfiltered and are outside it. And the spec's example blocks showed an em dash and an ellipsis while the code emits -- and ..., which matters because they read as literal expected output in a section that warns non-ASCII degrades under a C locale. Minor: two getattr comments pointed at the known-good reads as "below"; they are above. 152 tests pass. --- specs/03-formatters.md | 10 ++++++---- src/polyswarm/formatters/text.py | 17 +++++++++++----- tests/hunt_matched_strings_test.py | 32 +++++++++++++++++++++++++----- 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/specs/03-formatters.md b/specs/03-formatters.md index a5f28b82..e64bdbd3 100644 --- a/specs/03-formatters.md +++ b/specs/03-formatters.md @@ -159,7 +159,7 @@ line. | `matched_strings` | Rendered | |---|---| | `None` | *nothing* — no line is emitted | -| `[]` | `Matched Strings: none — the rule matched without byte evidence (a structural or negative match, or private strings)` | +| `[]` | `Matched Strings: none -- the rule matched without byte evidence (a structural or negative match, or private strings)` | | `[…]` | `Matched Strings:` followed by one indented ` $ident @ 0xOFFSET (N bytes[, truncated]): DATA` line per entry | Two constraints, both counter-intuitive enough to be worth stating: @@ -188,8 +188,10 @@ Two constraints, both counter-intuitive enough to be worth stating: 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.** `TextOutput` emits no non-ASCII; stdout under a C/POSIX locale replaces - it with `?`. `test_output_is_ascii_only` pins that. +- **ASCII only, and enforced where it can be.** The literals this module emits are + ASCII, and `data` is filtered to printable ASCII by `_safe_data`. Server-supplied + `rule_name` / `tags` are **not** filtered and are 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. @@ -214,7 +216,7 @@ When `result.matched_strings_dropped` is non-zero, a final line is appended **in block, in **yellow** rather than white: ``` - … 19 more not shown (result size limit) + ... 19 more not shown (result size limit) ``` It is the one line here reporting something the platform withheld, which is why it is not diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index bd1933df..bdd15d90 100644 --- a/src/polyswarm/formatters/text.py +++ b/src/polyswarm/formatters/text.py @@ -23,12 +23,19 @@ def pretty_print_datetime(value): # the analyzer keeps that rendering verbatim, so valid data is already printable ASCII and # this is a no-op on it. It exists because that guarantee lives in ANOTHER repo: if it ever # slips, a raw CSI sequence here would repaint or clear the analyst's terminal. -_CONTROL_CHARS = re.compile(r'[\x00-\x1f\x7f]') +# Everything outside printable ASCII, not just the C0 range. An earlier version stopped at +# \x7f and let U+009B through -- which IS the 8-bit CSI, acted on by xterm and VTE in UTF-8 +# mode, so `\x9b2J` still cleared the screen: a hole in exactly the byte this exists to +# block. The threat model is "the upstream escaping guarantee lives in another repo and may +# slip", and if it slips it slips into raw bytes, which do not stay conveniently low. +# Whitelisting printable ASCII also makes the ASCII-only rule in specs/03 true of this +# field rather than merely true of the literals around it. +_UNPRINTABLE = re.compile(r'[^\x20-\x7e]') def _safe_data(value): - """Matched bytes as yara rendered them, with any control character neutralised.""" - return _CONTROL_CHARS.sub('.', value) + """Matched bytes as yara rendered them, with anything unprintable neutralised.""" + return _UNPRINTABLE.sub('.', value) def is_grouped(fn): @@ -292,7 +299,7 @@ def historical_result(self, result, write=True): if result.tags: output.append(self._white(f'Tags: {result.tags}')) # getattr: the pin admits SDKs predating this attribute -- same defence as the - # known-good reads below. Missing lands on the silent None branch. + # known-good reads above. Missing lands on the silent None branch. output.extend(self._matched_strings( getattr(result, 'matched_strings', None), getattr(result, 'matched_strings_dropped', None))) @@ -328,7 +335,7 @@ def live_result(self, result, write=True): if result.tags: output.append(self._white(f'Tags: {result.tags}')) # getattr: the pin admits SDKs predating this attribute -- same defence as the - # known-good reads below. Missing lands on the silent None branch. + # known-good reads above. Missing lands on the silent None branch. output.extend(self._matched_strings( getattr(result, 'matched_strings', None), getattr(result, 'matched_strings_dropped', None))) diff --git a/tests/hunt_matched_strings_test.py b/tests/hunt_matched_strings_test.py index 68d60a00..18dd7e38 100644 --- a/tests/hunt_matched_strings_test.py +++ b/tests/hunt_matched_strings_test.py @@ -52,7 +52,11 @@ def _sdk_carries_the_fields(): return hasattr(probe, 'matched_strings') and hasattr(probe, 'matched_strings_dropped') -pytestmark = pytest.mark.skipif( +# Applied per-test, NOT as a module-level pytestmark. The two older-SDK tests below build +# a resource and pop the attribute off, so they pass on a floor SDK -- and that is the ONE +# install where they guard anything. A module-level skip took them out of exactly the +# configuration they model, leaving the getattr defence verified by nothing there. +needs_sdk_fields = pytest.mark.skipif( not _sdk_carries_the_fields(), reason='installed SDK predates matched_strings / matched_strings_dropped') @@ -83,6 +87,7 @@ def _matched_lines(text): return lines[start:end] +@needs_sdk_fields @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` @@ -92,12 +97,14 @@ def test_absent_renders_nothing(cls, method): assert 'Matched Strings' not in _render(cls, method) +@needs_sdk_fields @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)) +@needs_sdk_fields @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.""" @@ -106,6 +113,7 @@ def test_empty_says_the_rule_matched_without_evidence(cls, method): assert 'without byte evidence' in line +@needs_sdk_fields @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. @@ -115,6 +123,7 @@ def test_empty_and_absent_are_distinguishable(cls, method): assert len(_matched_lines(_render(cls, method, matched_strings=[]))) == 1 +@needs_sdk_fields @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)) @@ -123,6 +132,7 @@ def test_populated_renders_identifier_offset_length_and_data(cls, method): assert second == ' $mz @ 0x0 (512 bytes, truncated): 4D 5A 90 00 ...' +@needs_sdk_fields @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)) @@ -130,6 +140,7 @@ def test_truncation_is_marked_only_where_it_applies(cls, method): assert 'truncated' in second +@needs_sdk_fields @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.""" @@ -164,6 +175,7 @@ def test_an_sdk_without_the_attribute_does_not_raise(cls, method): assert 'Rule: dos_stub_message' in rendered # the rest of the row still renders +@needs_sdk_fields @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. @@ -177,12 +189,14 @@ def test_dropped_count_is_reported_to_the_user(cls, method): assert '19 more not shown' in lines[-1] +@needs_sdk_fields @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 +@needs_sdk_fields @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 @@ -204,6 +218,7 @@ def test_older_sdk_without_the_dropped_attribute_does_not_raise(cls, method): assert 'not shown' not in rendered +@needs_sdk_fields @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. @@ -218,6 +233,7 @@ def test_empty_list_with_a_dropped_count_does_not_claim_no_evidence(cls, method) assert '19' in line and 'withheld' in line, 'the count must survive' +@needs_sdk_fields @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.""" @@ -225,6 +241,7 @@ def test_empty_list_without_a_count_still_says_no_evidence(cls, method): assert 'without byte evidence' in line +@needs_sdk_fields @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. @@ -234,15 +251,19 @@ def test_control_characters_in_data_are_neutralised(cls, method): 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', 'truncated': False}] + 'data': 'A\x1b[2JB\r\nC\x00D\x9b2JE\u00e9F', 'truncated': False}] rendered = _render(cls, method, matched_strings=hostile) - assert '\x1b' not in rendered and chr(27) not in rendered - assert '\r' not in rendered and '\x00' not in rendered + 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' in rendered + assert 'A.[2JB..C.D.2JE.F' in rendered +@needs_sdk_fields @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.""" @@ -250,6 +271,7 @@ def test_ordinary_data_is_untouched_by_the_sanitiser(cls, method): assert '54 68 69 73 20 70 72 6F 67 72 61 6D 20 63' in rendered +@needs_sdk_fields @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.""" From 7a7a5af61fb162a11649cc80f2c7a3af94bac365 Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Sun, 30 Aug 2026 12:52:10 -0700 Subject: [PATCH 11/25] fix(text): sanitise identifier too, and correct three stale spec claims `identifier` was interpolated raw inside the matched-strings block -- the same block test_output_is_ascii_only asserts is ASCII, which passed only because the fixture uses ASCII identifiers. Routed through _safe_data alongside `data`, so the ASCII rule is now true of the whole block rather than needing another exception clause. yara's identifier grammar makes a hostile value close to impossible; this costs nothing and removes the caveat. Three spec statements had gone stale against changes in this same PR: - 99-open-questions said the test module "carries a module-level skip". The previous commit deliberately removed exactly that, and this section is the instruction sheet for the later floor decision -- a reader following it would hunt for a pytestmark that is not there and might reintroduce it. - 05-sdk-contract's version-pin guidance still used 4.2.0 as "the current floor" two paragraphs above the heading this PR corrected to 4.3.0. Reframed as the worked example it has become, since the check is the point rather than the number. - The ASCII bullet excluded rule_name and tags but not identifier. Also records that the silent-None branch depends on list routes sending null, and that this is measured rather than assumed: artifact-index's test_list_serializers_never_touch_storage pins present-and-null on BOTH hunt pairs against rows that carry evidence. This repo cannot verify it, so the spec now names what it relies on. 152 tests pass. --- specs/03-formatters.md | 17 +++++++++++++---- specs/05-sdk-contract.md | 2 +- specs/99-open-questions.md | 5 ++++- src/polyswarm/formatters/text.py | 2 +- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/specs/03-formatters.md b/specs/03-formatters.md index e64bdbd3..761bfc70 100644 --- a/specs/03-formatters.md +++ b/specs/03-formatters.md @@ -162,6 +162,15 @@ line. | `[]` | `Matched Strings: none -- the rule matched without byte evidence (a structural or negative match, or private strings)` | | `[…]` | `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_never_touch_storage`, 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 @@ -188,10 +197,10 @@ Two constraints, both counter-intuitive enough to be worth stating: 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 enforced where it can be.** The literals this module emits are - ASCII, and `data` is filtered to printable ASCII by `_safe_data`. Server-supplied - `rule_name` / `tags` are **not** filtered and are outside this claim. Stdout under a - C/POSIX locale replaces non-ASCII with `?`. +- **ASCII only, and true of this whole block.** The literals are ASCII, and both + server-supplied fields inside the matched-strings block — `data` and `identifier` — go + through `_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. diff --git a/specs/05-sdk-contract.md b/specs/05-sdk-contract.md index 82a79fcf..9d0f7b6f 100644 --- a/specs/05-sdk-contract.md +++ b/specs/05-sdk-contract.md @@ -72,7 +72,7 @@ When a CLI feature needs an SDK surface that doesn't exist yet: - There is **no lock file / compiled requirements** to keep in step: `pyproject.toml` is the only place the SDK version is expressed, and CI installs the SDK straight from the SDK repo's branch archive (see §Coordinated changes). A pin change is a one-file change *in this repo*, but it is not free of interactions — see below. - **The floor must be satisfied by the SDK archive CI installs, and by PyPI.** CI installs the archive build and *then* runs `pip install .[tests]`; if the archive's declared version is below the floor, that second install silently pulls a newer SDK from PyPI **over** the archive build, and CI stops testing the SDK branch at all — the mechanism §Coordinated changes rests on, defeated with no error. Symmetrically, a floor above the newest **published** version breaks `pip install polyswarm-cli` for every consumer the moment it reaches `master`. So a floor bump has two preconditions: the version is on PyPI, and the SDK's `develop` declares at least that version. - **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. For the current floor both were read from `origin/develop`: `version = "4.2.0"` and `__version__ = '4.2.0'`, no suffix. + **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. Worked example from when the floor was **4.2.0**: both were read from `origin/develop` as `version = "4.2.0"` / `__version__ = '4.2.0'`, no suffix. (The floor is now 4.3.0 — see §Current floor below. The check is the point, not the number.) ### Current floor — `polyswarm_api>=4.3.0` diff --git a/specs/99-open-questions.md b/specs/99-open-questions.md index 27f25123..6a2e923e 100644 --- a/specs/99-open-questions.md +++ b/specs/99-open-questions.md @@ -42,7 +42,10 @@ The `Polyswarm(PolyswarmAPI)` wrapper holds CLI-only orchestration (parallel fan the SDK carrying them is not on PyPI, so neither precondition in [`05-sdk-contract.md`](./05-sdk-contract.md) §Version pin is met. The CLI reads both with `getattr(..., None)` and degrades to rendering nothing, and -`tests/hunt_matched_strings_test.py` carries a module-level skip for the same reason. +`tests/hunt_matched_strings_test.py` carries a `needs_sdk_fields` mark on the tests that +construct populated resources — deliberately **per-test, not module-level**, so the two +older-SDK tests still run on a floor SDK, which is the only install where they guard +anything. **Action once the SDK releases:** record the version in `05-sdk-contract.md` §Current floor, decide whether to raise the floor past it, and if so drop *both* the `getattr` diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index bdd15d90..7b9ed26a 100644 --- a/src/polyswarm/formatters/text.py +++ b/src/polyswarm/formatters/text.py @@ -91,7 +91,7 @@ def _matched_strings(self, strings, dropped=None): # rather than render half-right. size += ', truncated' lines.append(self._white( - f' {string["identifier"]} @ 0x{string["offset"]:x} ({size}): {_safe_data(string["data"])}')) + f' {_safe_data(string["identifier"])} @ 0x{string["offset"]:x} ({size}): {_safe_data(string["data"])}')) if dropped: # Without this the list reads as the whole truth, and a user concludes their # rule hit N times when it hit N + dropped. Yellow, not white: it is the one From bb984079daf5c0bfdbc1188112fa10e189c09881 Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Sun, 30 Aug 2026 14:09:16 -0700 Subject: [PATCH 12/25] test(text): pin the yellow on both withheld-reporting lines specs/03 states the colour as a deliberate signal -- it is the one line in the block reporting something the platform did not send -- but every assertion in this module ran through TextOutput(color=False) and click.unstyle, so both lines could have regressed to white with the suite staying green. Adds a _render_styled helper mirroring the established pattern in known_good_field_test.py, and three cases: each withheld line is yellow, and the ordinary block is not -- without that last one the first two would pass on a formatter that painted everything. Also splits the `[]` row of the state table, which gave a single unconditional rendering while the section below it documents that a non-zero count overrides that line. The table is the part written to be consulted, and it was answering the case immediately below it incorrectly. 158 tests pass. --- specs/03-formatters.md | 3 ++- tests/hunt_matched_strings_test.py | 43 ++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/specs/03-formatters.md b/specs/03-formatters.md index 761bfc70..4d768a2f 100644 --- a/specs/03-formatters.md +++ b/specs/03-formatters.md @@ -159,7 +159,8 @@ line. | `matched_strings` | Rendered | |---|---| | `None` | *nothing* — no line is emitted | -| `[]` | `Matched Strings: none -- the rule matched without byte evidence (a structural or negative match, or private strings)` | +| `[]`, 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 diff --git a/tests/hunt_matched_strings_test.py b/tests/hunt_matched_strings_test.py index 18dd7e38..8453e8ce 100644 --- a/tests/hunt_matched_strings_test.py +++ b/tests/hunt_matched_strings_test.py @@ -278,3 +278,46 @@ def test_output_is_ascii_only(cls, method): 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)) + + +@needs_sdk_fields +@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 + + +@needs_sdk_fields +@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 + + +@needs_sdk_fields +@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 From cc8db8ca3c2f45c88b866388548d759e3c01fd24 Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Mon, 31 Aug 2026 09:25:02 -0700 Subject: [PATCH 13/25] test(text): make the older-SDK test floor-independent, and trim duplicated comments test_older_sdk_without_the_dropped_attribute_does_not_raise was left unmarked so it would run on a floor SDK -- the only install where it guards anything -- but it passed matched_strings through the content dict and then asserted the block renders. That requires the SDK to have PARSED matched_strings, which 4.3.0 does not, so it failed at the floor: the inverse of the intent, and it made the 99-open-questions claim false for one of the two tests. The field is now injected into __dict__ instead, so the test depends on nothing the floor omits. Verified against a stand-in resource class that sets neither field. That is the third correction to this pair -- del raising on old SDKs, then a module-level skip removing them from old SDKs, now a dependency on parsing. The common cause each time was a test that models an older SDK while quietly relying on a newer one. Also trims the formatter comments, which restated specs/03 nearly verbatim -- the CSI rationale, the three-state reasoning, the subscript-vs-getattr argument. Two copies of one argument is two places to keep in sync, and this PR already spent commits reconciling prose that drifted between them. The decisions stay in the code; the reasoning stays in the spec, which owns it. 158 tests pass. --- src/polyswarm/formatters/text.py | 47 +++++++++--------------------- tests/hunt_matched_strings_test.py | 11 +++++-- 2 files changed, 23 insertions(+), 35 deletions(-) diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index 7b9ed26a..dba6d49a 100644 --- a/src/polyswarm/formatters/text.py +++ b/src/polyswarm/formatters/text.py @@ -18,18 +18,10 @@ def pretty_print_datetime(value): return datetime.strftime(value, '%Y-%m-%d %H:%M:%S UTC') -# Matched-string `data` is the one field in a hunt result derived from the SAMPLE, so it -# is attacker-controlled end to end. yara escapes non-printables before we ever see it and -# the analyzer keeps that rendering verbatim, so valid data is already printable ASCII and -# this is a no-op on it. It exists because that guarantee lives in ANOTHER repo: if it ever -# slips, a raw CSI sequence here would repaint or clear the analyst's terminal. -# Everything outside printable ASCII, not just the C0 range. An earlier version stopped at -# \x7f and let U+009B through -- which IS the 8-bit CSI, acted on by xterm and VTE in UTF-8 -# mode, so `\x9b2J` still cleared the screen: a hole in exactly the byte this exists to -# block. The threat model is "the upstream escaping guarantee lives in another repo and may -# slip", and if it slips it slips into raw bytes, which do not stay conveniently low. -# Whitelisting printable ASCII also makes the ASCII-only rule in specs/03 true of this -# field rather than merely true of the literals around it. +# `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]') @@ -62,23 +54,16 @@ def _get_score_format(self, score): else: return self._red - # Three-state -- see the SDK's specs/05-downstream-contract.md. - # - # None renders NOTHING, reversing the instinct to explain the absence: None almost - # always means "list route", which can never carry strings, so a line there is a false - # alarm on every row. `live feed` loops over this same method and nothing on the - # resource tells the routes apart. [] keeps its line -- it only reaches a detail route, - # where "matched, no byte evidence" is a real answer. + # 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: - # Should be unreachable -- the analyzer keeps a match's first string, so an - # empty list with a non-zero count is contradictory. Report only what is - # certain: asserting "matched without byte evidence" here would be a false - # claim about the RULE, and silently discarding the count is the exact - # wrong inference this line exists to prevent. + # 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} withheld, result size limit)')] return [self._white('Matched Strings: none -- the rule matched without byte ' @@ -87,15 +72,13 @@ def _matched_strings(self, strings, dropped=None): for string in strings: size = f'{string["length"]} bytes' if string['truncated']: - # Subscripted like the other four: fail loudly on a partial entry - # rather than render half-right. + # Subscripted, unlike the attributes above -- a partial entry is a + # producer breaking its contract, not version skew. specs/03 records why. size += ', truncated' lines.append(self._white( f' {_safe_data(string["identifier"])} @ 0x{string["offset"]:x} ({size}): {_safe_data(string["data"])}')) if dropped: - # Without this the list reads as the whole truth, and a user concludes their - # rule hit N times when it hit N + dropped. Yellow, not white: it is the one - # line here reporting something the platform withheld. + # Yellow: the one line here reporting something the platform withheld. lines.append(self._yellow( f' ... {dropped} more not shown (result size limit)')) return lines @@ -298,8 +281,7 @@ def historical_result(self, result, write=True): output.append(self._white(malicious)) if result.tags: output.append(self._white(f'Tags: {result.tags}')) - # getattr: the pin admits SDKs predating this attribute -- same defence as the - # known-good reads above. Missing lands on the silent None branch. + # getattr: the pin admits SDKs predating these fields (specs/05-sdk-contract.md). output.extend(self._matched_strings( getattr(result, 'matched_strings', None), getattr(result, 'matched_strings_dropped', None))) @@ -334,8 +316,7 @@ def live_result(self, result, write=True): output.append(self._white(malicious)) if result.tags: output.append(self._white(f'Tags: {result.tags}')) - # getattr: the pin admits SDKs predating this attribute -- same defence as the - # known-good reads above. Missing lands on the silent None branch. + # getattr: the pin admits SDKs predating these fields (specs/05-sdk-contract.md). output.extend(self._matched_strings( getattr(result, 'matched_strings', None), getattr(result, 'matched_strings_dropped', None))) diff --git a/tests/hunt_matched_strings_test.py b/tests/hunt_matched_strings_test.py index 8453e8ce..01a325a5 100644 --- a/tests/hunt_matched_strings_test.py +++ b/tests/hunt_matched_strings_test.py @@ -207,10 +207,17 @@ def test_dropped_line_does_not_fabricate_a_strings_block(cls, method): @pytest.mark.parametrize('cls,method', PATHS) def test_older_sdk_without_the_dropped_attribute_does_not_raise(cls, method): - """Same pin as matched_strings: the dependency floor admits SDKs without it.""" - content = dict(_COMMON, matched_strings=_STRINGS) + """Same pin as matched_strings: the dependency floor admits SDKs without it. + + matched_strings is INJECTED rather than passed in the content dict. Relying on the SDK + to parse it makes the test require the very field the floor does not guarantee -- it + then fails at the floor, which is the one install where it guards anything. The sibling + above escapes this only because it asserts an ABSENCE. + """ + content = dict(_COMMON) content['livescan_id' if cls is resources.LiveHuntResult else 'historicalscan_id'] = 3 result = cls(content) + result.__dict__['matched_strings'] = _STRINGS result.__dict__.pop('matched_strings_dropped', None) # see the sibling test: not `del` assert not hasattr(result, 'matched_strings_dropped') rendered = click.unstyle('\n'.join(getattr(TextOutput(color=False), method)(result, write=False))) From 523aede8807f9a237417c607243be831d20b41be Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Thu, 3 Sep 2026 16:23:20 -0700 Subject: [PATCH 14/25] refactor: read the matched-strings attributes directly, not by probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dependency floor is now 4.4.0, which is the release that parses both fields, so pip refuses the install the getattr defence guarded against. specs/05-project-standards.md §16 is explicit that a consumer's need for a library surface is a version requirement, checked once by the packaging tool and never re-derived at runtime -- no attribute probing, and no per-test skip guards keyed on the dependency's shape. So the four getattr reads become plain attribute access, and the suite loses _sdk_carries_the_fields, its 18 skip decorators and the two tests that popped an attribute off a resource to model an install the floor now forbids. Their real job survives in test_absent_renders_nothing, which renders from a fixture that sets no key, so the None branch stays covered by a real resource rather than a mutilated one. 36 tests, none skipped. The entry keys stay subscripted, deliberately: a partial entry is a producer violating its contract, not version skew, and the two are answering different questions. Also in this diff, all small: - length and dropped gain an integer format spec, so every server-supplied value in a block whose spec claims ASCII is pinned by construction -- strings by _safe_data, numbers by :x / :d. offset was safe only by accident of already having one. - The server-side test cited as pinning the silent-None design was renamed on this same branch; the citation now names the test that exists. - The open question about this floor is resolved, so it is removed rather than left describing a probe that is gone. --- specs/03-formatters.md | 41 +++++++-------- specs/99-open-questions.md | 17 ------ src/polyswarm/formatters/text.py | 20 +++---- tests/hunt_matched_strings_test.py | 83 ------------------------------ 4 files changed, 27 insertions(+), 134 deletions(-) diff --git a/specs/03-formatters.md b/specs/03-formatters.md index 187e86d7..997ff603 100644 --- a/specs/03-formatters.md +++ b/specs/03-formatters.md @@ -224,27 +224,22 @@ Two constraints, both counter-intuitive enough to be worth stating: 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, and both - server-supplied fields inside the matched-strings block — `data` and `identifier` — go - through `_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 `?`. +- **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 with `getattr(result, 'matched_strings', None)`, never a bare attribute access** — -the same defence, for the same reason, as the known-good attributes above. The attribute -ships in the paired SDK release, but the dependency floor admits older SDKs whose -resources lack it entirely, and a bare read would `AttributeError` on *every* text-mode -hunt command, not just the new output: `live result` / `live feed` / `live results-delete` -and the three `historical` equivalents all funnel through these two methods. A missing -attribute degrades to the silent `None` branch, which is also the honest reading — an SDK -that cannot see the field genuinely does not know. - -Nothing else can catch this. The rendering tests build resources from the *installed* -SDK, so with a paired SDK on the path a bare read passes every one of them; -`test_an_sdk_without_the_attribute_does_not_raise` deletes the attribute to stand in for -an older SDK, and is the only guard. +**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 @@ -272,7 +267,11 @@ 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 `tests/hunt_matched_strings_test.py` (Style 3 — the formatter driven directly -with constructed SDK resources). 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. They are not a substitute for those unit tests. +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/99-open-questions.md b/specs/99-open-questions.md index 6a2e923e..be530112 100644 --- a/specs/99-open-questions.md +++ b/specs/99-open-questions.md @@ -33,20 +33,3 @@ The SDK pin and the paired-PR `## Requires` convention ([`05-sdk-contract.md`](. **Status:** open. The `Polyswarm(PolyswarmAPI)` wrapper holds CLI-only orchestration (parallel fan-out, multi-step flows). Some of it (e.g. `submit_url`'s inline `/instance/url` endpoint) arguably belongs in the SDK so the CLI is a pure wrapper. **Action:** as the SDK grows methods that subsume wrapper logic, migrate the wrapper to call them and shrink the CLI-owned surface. - -## SDK floor for the matched-strings attributes - -**Status:** blocked on a release. - -`matched_strings` / `matched_strings_dropped` are not covered by the dependency floor — -the SDK carrying them is not on PyPI, so neither precondition in -[`05-sdk-contract.md`](./05-sdk-contract.md) §Version pin is met. The CLI reads both with -`getattr(..., None)` and degrades to rendering nothing, and -`tests/hunt_matched_strings_test.py` carries a `needs_sdk_fields` mark on the tests that -construct populated resources — deliberately **per-test, not module-level**, so the two -older-SDK tests still run on a floor SDK, which is the only install where they guard -anything. - -**Action once the SDK releases:** record the version in `05-sdk-contract.md` §Current -floor, decide whether to raise the floor past it, and if so drop *both* the `getattr` -defence and the test skip — they exist only to tolerate SDKs the floor still admits. diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index e527c58c..3dfcfd69 100644 --- a/src/polyswarm/formatters/text.py +++ b/src/polyswarm/formatters/text.py @@ -65,22 +65,20 @@ def _matched_strings(self, strings, dropped=None): # 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} withheld, result size limit)')] + 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"]} bytes' + size = f'{string["length"]:d} bytes' if string['truncated']: - # Subscripted, unlike the attributes above -- a partial entry is a - # producer breaking its contract, not version skew. specs/03 records why. 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} more not shown (result size limit)')) + f' ... {dropped:d} more not shown (result size limit)')) return lines def _output(self, output, write): @@ -294,10 +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}')) - # getattr: the pin admits SDKs predating these fields (specs/05-sdk-contract.md). - output.extend(self._matched_strings( - getattr(result, 'matched_strings', None), - getattr(result, 'matched_strings_dropped', None))) + 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) @@ -329,10 +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}')) - # getattr: the pin admits SDKs predating these fields (specs/05-sdk-contract.md). - output.extend(self._matched_strings( - getattr(result, 'matched_strings', None), - getattr(result, 'matched_strings_dropped', None))) + 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/hunt_matched_strings_test.py b/tests/hunt_matched_strings_test.py index 01a325a5..d6eeb5d8 100644 --- a/tests/hunt_matched_strings_test.py +++ b/tests/hunt_matched_strings_test.py @@ -39,28 +39,6 @@ ] # (resource class, formatter method name) -def _sdk_carries_the_fields(): - """Whether the installed SDK parses the attributes this module renders. - - The declared pin (`polyswarm_api>=4.3.0`) still admits SDKs predating them -- 4.3.0 - itself is released without them -- so `pip install .[tests] && pytest` against the - floor would fail this module wholesale. CI resolves the paired SDK branch and runs it - for real. Remove this guard when the floor is raised past the release that adds them - (specs/05-sdk-contract.md, §Current floor). - """ - probe = resources.LiveHuntResult(dict(_COMMON, livescan_id=3)) - return hasattr(probe, 'matched_strings') and hasattr(probe, 'matched_strings_dropped') - - -# Applied per-test, NOT as a module-level pytestmark. The two older-SDK tests below build -# a resource and pop the attribute off, so they pass on a floor SDK -- and that is the ONE -# install where they guard anything. A module-level skip took them out of exactly the -# configuration they model, leaving the getattr defence verified by nothing there. -needs_sdk_fields = pytest.mark.skipif( - not _sdk_carries_the_fields(), - reason='installed SDK predates matched_strings / matched_strings_dropped') - - PATHS = [ (resources.LiveHuntResult, 'live_result'), (resources.HistoricalHuntResult, 'historical_result'), @@ -87,7 +65,6 @@ def _matched_lines(text): return lines[start:end] -@needs_sdk_fields @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` @@ -97,14 +74,12 @@ def test_absent_renders_nothing(cls, method): assert 'Matched Strings' not in _render(cls, method) -@needs_sdk_fields @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)) -@needs_sdk_fields @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.""" @@ -113,7 +88,6 @@ def test_empty_says_the_rule_matched_without_evidence(cls, method): assert 'without byte evidence' in line -@needs_sdk_fields @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. @@ -123,7 +97,6 @@ def test_empty_and_absent_are_distinguishable(cls, method): assert len(_matched_lines(_render(cls, method, matched_strings=[]))) == 1 -@needs_sdk_fields @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)) @@ -132,7 +105,6 @@ def test_populated_renders_identifier_offset_length_and_data(cls, method): assert second == ' $mz @ 0x0 (512 bytes, truncated): 4D 5A 90 00 ...' -@needs_sdk_fields @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)) @@ -140,7 +112,6 @@ def test_truncation_is_marked_only_where_it_applies(cls, method): assert 'truncated' in second -@needs_sdk_fields @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.""" @@ -152,30 +123,6 @@ def test_block_sits_between_tags_and_download_url(cls, method): assert tags < matched < download -@pytest.mark.parametrize('cls,method', PATHS) -def test_an_sdk_without_the_attribute_does_not_raise(cls, method): - """The dependency pin admits SDKs predating `matched_strings`, and nothing else here - would catch a bare `result.matched_strings`. - - Every other test in this file builds resources from the INSTALLED SDK, so with a - paired SDK on the path a bare attribute read passes all of them and then - AttributeErrors in the field -- on every text-mode hunt command, not just the new - output. Deleting the attribute is what an older SDK's resource looks like. - """ - content = dict(_COMMON) - content['livescan_id' if cls is resources.LiveHuntResult else 'historicalscan_id'] = 3 - result = cls(content) - # pop, not `del`: on an SDK that never SET the attribute -- exactly the configuration - # this test models, and one inside the declared pin -- `del` raises AttributeError and - # the test errors instead of passing. - result.__dict__.pop('matched_strings', None) - assert not hasattr(result, 'matched_strings') - rendered = click.unstyle('\n'.join(getattr(TextOutput(color=False), method)(result, write=False))) - assert 'Matched Strings' not in rendered - assert 'Rule: dos_stub_message' in rendered # the rest of the row still renders - - -@needs_sdk_fields @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. @@ -189,14 +136,12 @@ def test_dropped_count_is_reported_to_the_user(cls, method): assert '19 more not shown' in lines[-1] -@needs_sdk_fields @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 -@needs_sdk_fields @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 @@ -205,27 +150,6 @@ def test_dropped_line_does_not_fabricate_a_strings_block(cls, method): assert 'Matched Strings' not in rendered -@pytest.mark.parametrize('cls,method', PATHS) -def test_older_sdk_without_the_dropped_attribute_does_not_raise(cls, method): - """Same pin as matched_strings: the dependency floor admits SDKs without it. - - matched_strings is INJECTED rather than passed in the content dict. Relying on the SDK - to parse it makes the test require the very field the floor does not guarantee -- it - then fails at the floor, which is the one install where it guards anything. The sibling - above escapes this only because it asserts an ABSENCE. - """ - content = dict(_COMMON) - content['livescan_id' if cls is resources.LiveHuntResult else 'historicalscan_id'] = 3 - result = cls(content) - result.__dict__['matched_strings'] = _STRINGS - result.__dict__.pop('matched_strings_dropped', None) # see the sibling test: not `del` - assert not hasattr(result, 'matched_strings_dropped') - rendered = click.unstyle('\n'.join(getattr(TextOutput(color=False), method)(result, write=False))) - assert 'Matched Strings:' in rendered - assert 'not shown' not in rendered - - -@needs_sdk_fields @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. @@ -240,7 +164,6 @@ def test_empty_list_with_a_dropped_count_does_not_claim_no_evidence(cls, method) assert '19' in line and 'withheld' in line, 'the count must survive' -@needs_sdk_fields @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.""" @@ -248,7 +171,6 @@ def test_empty_list_without_a_count_still_says_no_evidence(cls, method): assert 'without byte evidence' in line -@needs_sdk_fields @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. @@ -270,7 +192,6 @@ def test_control_characters_in_data_are_neutralised(cls, method): assert 'A.[2JB..C.D.2JE.F' in rendered -@needs_sdk_fields @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.""" @@ -278,7 +199,6 @@ def test_ordinary_data_is_untouched_by_the_sanitiser(cls, method): assert '54 68 69 73 20 70 72 6F 67 72 61 6D 20 63' in rendered -@needs_sdk_fields @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.""" @@ -301,7 +221,6 @@ def _render_styled(cls, method, **extra): return '\n'.join(getattr(TextOutput(color=True), method)(cls(content), write=False)) -@needs_sdk_fields @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 @@ -312,7 +231,6 @@ def test_dropped_count_line_is_yellow(cls, method): assert expected in rendered, rendered -@needs_sdk_fields @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.""" @@ -322,7 +240,6 @@ def test_empty_with_a_count_line_is_yellow(cls, method): assert expected in rendered, rendered -@needs_sdk_fields @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.""" From 28fe88cbd90a74906e95ca7b8c4bcad04351c62f Mon Sep 17 00:00:00 2001 From: Kyle Buchmiller Date: Thu, 3 Sep 2026 16:23:21 -0700 Subject: [PATCH 15/25] test: drive a populated matched-strings block through the command tree specs/04-testing.md is explicit that a command whose rendering is covered by the formatter-only tier "still needs at least one CliRunner test proving the command reaches the formatter". This adds it. What it buys, measured rather than assumed: rewiring the command to render through the wrong formatter method leaves all 36 assertions in the sibling module passing -- they invoke the formatter themselves and cannot see the command tree above it -- and fails this module. The cassettes cannot catch it either, since they predate the field and every result they render takes the silent None branch. Uses real SDK resources rather than mocks: the formatter reads polyscore and the rest of the row, so a bare stub dies before reaching the block under test. --- tests/hunt_matched_strings_cli_test.py | 83 ++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 tests/hunt_matched_strings_cli_test.py 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() From ac1f8d46b30eda5a7197af364eea8cbf766ef794 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Tue, 1 Sep 2026 19:51:05 -0400 Subject: [PATCH 16/25] feat(rules): list --sort active-first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Forwards `ruleset_list(sort='active_first')` — the hunt page's order, rulesets with a running live hunt first (as recorded by the server's live-hunt link, the same one Livescan Id renders from), newest first within each block. Server-side like the filters: the list is keyset-paginated, so a local sort would only ever reorder one page. The option is a closed click.Choice (hyphenated CLI spelling, underscored server token) and is forwarded only when given, so the unsorted default request is unchanged. The autospec tests double as the signature check against the installed SDK, alone and combined with the filters. --- specs/02-commands.md | 2 +- src/polyswarm/client/rules.py | 15 ++++++++--- tests/formatter_hunt_fields_test.py | 40 +++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/specs/02-commands.md b/specs/02-commands.md index fd1aeb7e..de7358b6 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 running-hunts-first order — 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` / `--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/src/polyswarm/client/rules.py b/src/polyswarm/client/rules.py index 9f8fa7e2..d305aa47 100644 --- a/src/polyswarm/client/rules.py +++ b/src/polyswarm/client/rules.py @@ -38,19 +38,26 @@ def delete(ctx, rule_id): @click.option('--favorites-only', is_flag=True, help='Only favorited (starred) rulesets.') @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 with a running live hunt first (as recorded by the ' + "server's live-hunt link, the same one Livescan Id renders from), " + 'newest first within each block. Default is newest first.') @click.pass_context -def list_rules(ctx, name, status, favorites_only, has_new_results): +def list_rules(ctx, name, status, favorites_only, 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. """ 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)) + ('has_new_results', has_new_results or None), + ('sort', sort.replace('-', '_') if sort else None)) if v is not None} for ruleset in api.ruleset_list(**kwargs): output.ruleset(ruleset) diff --git a/tests/formatter_hunt_fields_test.py b/tests/formatter_hunt_fields_test.py index 1d0675da..187b8faa 100644 --- a/tests/formatter_hunt_fields_test.py +++ b/tests/formatter_hunt_fields_test.py @@ -168,6 +168,46 @@ def test_filters_are_forwarded_only_when_given(self): mock.ANY, name='alpha', status='active', favorites_only=True, has_new_results=True) + 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_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): From 5e07b7ccd28478488da265f3461185cc03157373 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Tue, 1 Sep 2026 19:51:05 -0400 Subject: [PATCH 17/25] chore: raise the SDK floor to polyswarm_api>=4.5.0 for ruleset_list(sort=) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The floor names the SDK version that adds the keyword `rules list --sort` forwards (specs/05 §Current floor follows the pin). Mergeable once the SDK's develop declares 4.5.0; releasable once that version is on PyPI — the SDK releases first. CI resolves the SDK from source by branch name, falling back to develop, so the paired SDK branch must carry the identical name. --- pyproject.toml | 2 +- specs/05-sdk-contract.md | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) 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/05-sdk-contract.md b/specs/05-sdk-contract.md index 7a0749b4..3eb39c27 100644 --- a/specs/05-sdk-contract.md +++ b/specs/05-sdk-contract.md @@ -79,7 +79,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 +99,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: From 20e3c03f41d696cfad26a8f80b900b7ffd197247 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Mon, 14 Sep 2026 13:10:25 -0300 Subject: [PATCH 18/25] docs(rules): name the label the formatter actually prints in --sort help --- src/polyswarm/client/rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/polyswarm/client/rules.py b/src/polyswarm/client/rules.py index d305aa47..5fdb0f80 100644 --- a/src/polyswarm/client/rules.py +++ b/src/polyswarm/client/rules.py @@ -40,7 +40,7 @@ def delete(ctx, rule_id): help='Only rulesets whose stored new-results counter is positive.') @click.option('--sort', type=click.Choice(['active-first']), help='Order: rulesets with a running live hunt first (as recorded by the ' - "server's live-hunt link, the same one Livescan Id renders from), " + "server's live-hunt link, the same one Live Hunt Id renders from), " 'newest first within each block. Default is newest first.') @click.pass_context def list_rules(ctx, name, status, favorites_only, has_new_results, sort): From fbd40cb1915cd7f02bc985fa2c5e0573318945b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Mon, 14 Sep 2026 16:25:48 -0300 Subject: [PATCH 19/25] docs(rules): --sort ranks on the stored hunt link, which is wider than Live Hunt Id The help text said the order came from "the same one Live Hunt Id renders from". The server ranks on the raw link and renders the id under a stricter predicate, so a legacy row whose hunt was stopped without clearing the link leads the list with an empty Live Hunt Id. Someone reading the list top-down for what is running would have stopped at that row. --- specs/02-commands.md | 2 +- src/polyswarm/client/rules.py | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/specs/02-commands.md b/specs/02-commands.md index de7358b6..873af124 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 (`list` takes the server-side filters plus `--sort active-first`, the hunt page's running-hunts-first order — 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` / `--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}` | +| `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 — the rank is that stored link, wider than what Live Hunt Id renders from, so a legacy stopped-but-unlinked row leads the list with an empty Live Hunt Id; 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` / `--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/src/polyswarm/client/rules.py b/src/polyswarm/client/rules.py index 5fdb0f80..ca01f15c 100644 --- a/src/polyswarm/client/rules.py +++ b/src/polyswarm/client/rules.py @@ -39,9 +39,11 @@ def delete(ctx, rule_id): @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 with a running live hunt first (as recorded by the ' - "server's live-hunt link, the same one Live Hunt Id renders from), " - 'newest first within each block. Default is newest 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 with an empty Live Hunt Id.') @click.pass_context def list_rules(ctx, name, status, favorites_only, has_new_results, sort): """List rulesets, optionally filtered. All filters are conjunctive. From bc80446a3190674730d4ad1ad54470e8e4cd1603 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Mon, 14 Sep 2026 19:09:20 -0300 Subject: [PATCH 20/25] fix(rules): dedupe the ruleset walk by id, and stop promising an empty Live Hunt Id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rules list` walks every page and exposes no limit, which makes it exactly the consumer the SDK puts the dedupe obligation on. Under --sort active-first the ordering key is the live-hunt link, so a ruleset whose hunt stops between two page fetches drops below the cursor and the server serves it again: the run printed it twice and any script counting the output double-counted it. Rows are now emitted at most once per run. The dedupe is unconditional — the id is unique under either order and one set of ids costs nothing next to the rendered rows. The symmetric case cannot be repaired from here and is documented rather than hidden: a hunt STARTED mid-walk moves its row above the cursor and it never reaches this client until the next run. Two doc corrections in the same push. The --sort help promised that a stale-link row "leads the list with an empty Live Hunt Id", but the formatter gates the whole Live Hunt Id pair on a truthy value, so such a row prints no such line at all and reads as idle. And the commands spec called that row "stopped-but-unlinked", the inverse of the state it means: it is stopped and STILL linked, which is why it leads a list ranked on the link being present. --- specs/02-commands.md | 2 +- src/polyswarm/client/rules.py | 24 +++++++++++++++++-- tests/formatter_hunt_fields_test.py | 36 +++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/specs/02-commands.md b/specs/02-commands.md index 873af124..868fa41e 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 (`list` takes the server-side filters plus `--sort active-first`, the hunt page's order: rulesets carrying a live hunt link first — the rank is that stored link, wider than what Live Hunt Id renders from, so a legacy stopped-but-unlinked row leads the list with an empty Live Hunt Id; 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` / `--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}` | +| `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 — the rank is that stored link, wider than what Live Hunt Id renders from, so a legacy row that was stopped while STILL LINKED leads the list and renders no Live Hunt Id at all — read the field, never the position. The command walks every page and so dedupes by id, the obligation the SDK puts on a multi-page consumer of this mutable key (a row whose hunt stops mid-walk is served twice; one started mid-walk is missed until the next run); 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` / `--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/src/polyswarm/client/rules.py b/src/polyswarm/client/rules.py index ca01f15c..a558b63b 100644 --- a/src/polyswarm/client/rules.py +++ b/src/polyswarm/client/rules.py @@ -41,9 +41,11 @@ def delete(ctx, rule_id): @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 ' + '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 with an empty Live Hunt Id.') + 'the link leads the list while rendering no Live Hunt Id at ' + 'all, indistinguishable from an idle one. Read the field, ' + 'never the position.') @click.pass_context def list_rules(ctx, name, status, favorites_only, has_new_results, sort): """List rulesets, optionally filtered. All filters are conjunctive. @@ -51,6 +53,16 @@ def list_rules(ctx, name, status, favorites_only, has_new_results, sort): 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 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'] @@ -61,7 +73,15 @@ def list_rules(ctx, name, status, favorites_only, has_new_results, sort): ('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 187b8faa..2dfe9d23 100644 --- a/tests/formatter_hunt_fields_test.py +++ b/tests/formatter_hunt_fields_test.py @@ -198,6 +198,42 @@ def test_sort_composes_with_the_filters(self): 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.""" + repeated = [_ruleset(id='5', name='stops-mid-walk'), + _ruleset(id='7', name='other'), + _ruleset(id='5', name='stops-mid-walk')] + 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_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_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. From 68e4f62c461ed652dcf6ef2504b992108fee1807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Mon, 14 Sep 2026 19:36:13 -0300 Subject: [PATCH 21/25] test: pin the dedupe key as the id, not the rendered name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two dedupe tests repeated the same row wholesale and compared rows that differed in every field, so an implementation keyed on the name — or on the rendered block — passed both. Ruleset names are not unique, so that variant would swallow a real row from any inventory listing. The new case is the one that separates them: two distinct rulesets sharing a name must both render. Re-keying the dedupe on the name fails it and nothing else. --- tests/formatter_hunt_fields_test.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/formatter_hunt_fields_test.py b/tests/formatter_hunt_fields_test.py index 2dfe9d23..04372f42 100644 --- a/tests/formatter_hunt_fields_test.py +++ b/tests/formatter_hunt_fields_test.py @@ -219,6 +219,22 @@ def test_a_row_served_twice_by_the_mutable_sort_is_printed_once(self): # The row between the duplicates still renders — dedupe, not truncation. assert 'other' 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 From eb1f8102d9245e5f5a1f62b8acf825cfe3e4c64f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Mon, 14 Sep 2026 19:43:01 -0300 Subject: [PATCH 22/25] docs+test: the surviving copy of a moved row is the stale one, and say so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round two of review. The dedupe keeps the first copy — the only choice a streaming printer has, since that copy is already on stdout when the second arrives — and the first copy carries the values from BEFORE the transition. So the one row the dedupe acts on prints the Live Hunt Id of a hunt that has already stopped. The help said to read the field rather than the position; under a mid-walk move neither is authoritative, and the help, the command docstring and the commands catalogue now say that instead of promising it. The dedupe test repeated a row wholesale, which is not the shape a re-serve takes: the two copies differ in exactly the field the sort ranks on, because that is why the row moved. It uses that shape now, and a new case pins first-wins as a decision — a last-wins rewrite fails it and nothing else. The invariant itself moved to specs/05 §Consuming the SDK correctly, beside the other SDK-consumption rules, where the next command that walks a mutably ordered endpoint will find it; the commands catalogue points at it. --- specs/02-commands.md | 2 +- specs/05-sdk-contract.md | 26 +++++++++++++++++++++++ src/polyswarm/client/rules.py | 13 ++++++++++-- tests/formatter_hunt_fields_test.py | 32 ++++++++++++++++++++++++++--- 4 files changed, 67 insertions(+), 6 deletions(-) diff --git a/specs/02-commands.md b/specs/02-commands.md index 868fa41e..04808eb9 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 (`list` takes the server-side filters plus `--sort active-first`, the hunt page's order: rulesets carrying a live hunt link first — the rank is that stored link, wider than what Live Hunt Id renders from, so a legacy row that was stopped while STILL LINKED leads the list and renders no Live Hunt Id at all — read the field, never the position. The command walks every page and so dedupes by id, the obligation the SDK puts on a multi-page consumer of this mutable key (a row whose hunt stops mid-walk is served twice; one started mid-walk is missed until the next run); 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` / `--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}` | +| `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 — the rank is that stored link, wider than what Live Hunt Id renders from, so a legacy row that was stopped while STILL LINKED leads the list and renders no Live Hunt Id at all — the position is not evidence a hunt is running, and for a row that moved mid-walk neither is the field. The command walks every page and so dedupes by id, keeping the first copy — see [05-sdk-contract.md](./05-sdk-contract.md) §A mutable order makes the walk the caller's problem for what that costs; 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` / `--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 3eb39c27..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. diff --git a/src/polyswarm/client/rules.py b/src/polyswarm/client/rules.py index a558b63b..51f45cf1 100644 --- a/src/polyswarm/client/rules.py +++ b/src/polyswarm/client/rules.py @@ -44,8 +44,10 @@ def delete(ctx, rule_id): '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. Read the field, ' - 'never the position.') + '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, sort): """List rulesets, optionally filtered. All filters are conjunctive. @@ -60,6 +62,13 @@ def list_rules(ctx, name, status, favorites_only, has_new_results, sort): 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. diff --git a/tests/formatter_hunt_fields_test.py b/tests/formatter_hunt_fields_test.py index 04372f42..e191859b 100644 --- a/tests/formatter_hunt_fields_test.py +++ b/tests/formatter_hunt_fields_test.py @@ -203,10 +203,13 @@ def test_a_row_served_twice_by_the_mutable_sort_is_printed_once(self): 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.""" - repeated = [_ruleset(id='5', name='stops-mid-walk'), + 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')] + _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( @@ -219,6 +222,29 @@ def test_a_row_served_twice_by_the_mutable_sort_is_printed_once(self): # 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 From 1620f4af5d2eedae91b2d29164dadd08f32fe726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Mon, 14 Sep 2026 20:04:04 -0300 Subject: [PATCH 23/25] test+docs: give the sort and dedupe pins their own class, and trim the duplicated prose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven tests about sort forwarding and mid-walk dedupe had accumulated inside a class whose docstring promised only the zero-argument call, so nobody landing on the name would expect the dedupe pins to be there. They are their own class now, with a docstring that states both contracts it holds; the module docstring enumerates the new one alongside the others. The same ~150 words about stale Live Hunt Id values were restated in the help, the commands catalogue and the SDK-contract spec. The help keeps them — the user reading --help is the one who acts on a stale field — and the catalogue cell, the copy most likely to drift, is back to the one-line claim plus its existing pointer. --- specs/02-commands.md | 2 +- tests/formatter_hunt_fields_test.py | 32 ++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/specs/02-commands.md b/specs/02-commands.md index 04808eb9..71c869ff 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 (`list` takes the server-side filters plus `--sort active-first`, the hunt page's order: rulesets carrying a live hunt link first — the rank is that stored link, wider than what Live Hunt Id renders from, so a legacy row that was stopped while STILL LINKED leads the list and renders no Live Hunt Id at all — the position is not evidence a hunt is running, and for a row that moved mid-walk neither is the field. The command walks every page and so dedupes by id, keeping the first copy — see [05-sdk-contract.md](./05-sdk-contract.md) §A mutable order makes the walk the caller's problem for what that costs; 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` / `--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}` | +| `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` / `--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/tests/formatter_hunt_fields_test.py b/tests/formatter_hunt_fields_test.py index e191859b..52a22921 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', @@ -168,6 +174,22 @@ def test_filters_are_forwarded_only_when_given(self): mock.ANY, name='alpha', status='active', 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 From 98b4842639c0e9feda1f7f1bac6e049542bd388b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Mon, 14 Sep 2026 22:45:59 -0300 Subject: [PATCH 24/25] feat(rules): list --exclude-favorites The inverse of --favorites-only, which the server refuses to combine with it. It exists for a client that renders the favorites as their own list: leaving them in the paginated list too makes a page repeat a row or come back short. A False flag is not a filter here either, so an unflagged invocation sends nothing new. --- specs/02-commands.md | 2 +- src/polyswarm/client/rules.py | 7 ++++++- tests/formatter_hunt_fields_test.py | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/specs/02-commands.md b/specs/02-commands.md index 71c869ff..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 (`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` / `--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}` | +| `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/src/polyswarm/client/rules.py b/src/polyswarm/client/rules.py index 51f45cf1..2b3aa376 100644 --- a/src/polyswarm/client/rules.py +++ b/src/polyswarm/client/rules.py @@ -36,6 +36,10 @@ 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']), @@ -49,7 +53,7 @@ def delete(ctx, rule_id): '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, sort): +def list_rules(ctx, name, status, favorites_only, exclude_favorites, has_new_results, sort): """List rulesets, optionally filtered. All filters are conjunctive. Filtering and ordering are applied SERVER-side: the list is @@ -79,6 +83,7 @@ def list_rules(ctx, name, status, favorites_only, has_new_results, sort): # 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), + ('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} diff --git a/tests/formatter_hunt_fields_test.py b/tests/formatter_hunt_fields_test.py index 52a22921..05c7afa8 100644 --- a/tests/formatter_hunt_fields_test.py +++ b/tests/formatter_hunt_fields_test.py @@ -298,6 +298,22 @@ def test_the_default_order_is_not_narrowed_by_the_dedupe(self): 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. From 3bb216869e9b96fc5a829acb2d05abd0185bc2fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Tue, 15 Sep 2026 14:46:43 -0300 Subject: [PATCH 25/25] =?UTF-8?q?Bump=20version:=204.4.0=20=E2=86=92=204.5?= =?UTF-8?q?.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Release bump for 4.5.0 — `rules list --sort active-first` and `--exclude-favorites`. The SDK floor `polyswarm_api>=4.5.0` is already pinned; polyswarm-api Release 4.5.0 must be on PyPI before this repo's develop → master merge. --- pyproject.toml | 4 ++-- src/polyswarm/__init__.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 84c00fc3..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" }] @@ -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/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'