diff --git a/AGENTS.md b/AGENTS.md index 2eb0a7f..eec96c6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -88,8 +88,9 @@ Mirror the existing patterns (e.g. `field-property`, `prompt-config`, `ruleset`) Details in [`specs/04-testing.md`](./specs/04-testing.md). The shape: -- Tests live in `tests/` and drive the CLI through `click.testing.CliRunner` — no live PolySwarm stack needed. -- Two styles: **SDK-boundary mocks** (`mock.patch('polyswarm_api.api.PolyswarmAPI.')`, e.g. `tests/field_property_test.py`) and **VCR cassettes** (`tests/cli_test.py`) that replay recorded HTTP for end-to-end CLI runs. Cassettes live in `tests/vcr/` (`.vcr` for the HTTP interactions, `.click` for the expected rendered output). +- Tests live in `tests/` and drive command behaviour through `click.testing.CliRunner` — no live PolySwarm stack needed. +- Two styles for that: **SDK-boundary mocks** (`mock.patch('polyswarm_api.api.PolyswarmAPI.')`, e.g. `tests/field_property_test.py`) and **VCR cassettes** (`tests/cli_test.py`) that replay recorded HTTP for end-to-end CLI runs. Cassettes live in `tests/vcr/` (`.vcr` for the HTTP interactions, `.click` for the expected rendered output). +- Pure rendering logic — which line a given field set produces, no command-tree behaviour — is instead unit-tested against the formatter directly (e.g. `tests/known_good_field_test.py`); see the spec's *Style 3* for when that's the right choice. - VCR is an **efficiency cache, not a requirement** — the suite must pass against a live e2e stack with VCR off. Re-record a cassette by deleting it and re-running the test against a live stack; never hand-edit a cassette or `cp` one from a sibling test. ## Commit + PR hygiene diff --git a/pyproject.toml b/pyproject.toml index c312fd7..03e289b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "setuptools.build_meta" [project] name = "polyswarm" -version = "4.2.1" +version = "4.3.0" description = "CLI for using the PolySwarm Customer APIs" readme = "README.md" authors = [{ name = "PolySwarm Developers", email = "info@polyswarm.io" }] @@ -22,7 +22,7 @@ classifiers = [ ] dependencies = [ - "polyswarm_api>=4.0.0,<5.0.0", + "polyswarm_api>=4.3.0,<5.0.0", "click>=7.1", "colorama>=0.4.6", "click-log>=0.4.0", @@ -51,7 +51,7 @@ include-package-data = true where = ["src"] [tool.bumpversion] -current_version = "4.2.1" +current_version = "4.3.0" commit = true tag = false sign_tags = true diff --git a/specs/03-formatters.md b/specs/03-formatters.md index 062aa63..b717ae1 100644 --- a/specs/03-formatters.md +++ b/specs/03-formatters.md @@ -20,7 +20,7 @@ How command output is rendered: the `BaseOutput` interface, the concrete formatt | Formatter | Module | Output | |---|---|---| -| `TextOutput` | `text.py` | Human-readable, labelled blocks; honours `--color/--no-color`. Dates via `polyswarm_api.core.parse_isoformat`. | +| `TextOutput` | `text.py` | Human-readable, labelled blocks; honours `--color/--no-color` through `_paint`, the single place `self.color` is read. The five `_white`/`_red`/… helpers all route through it — a convention, not an enforcement point: a new helper calling `click.style` directly would re-break the flag, so add colours by adding a `_paint` caller. Dates via `polyswarm_api.core.parse_isoformat`. | | `JSONOutput` | `json.py` | Machine-readable JSON (typically the resource's `.json` plus derived fields). | | `SHA256Output` / `SHA1Output` / `MD5Output` | `hashes.py` | Hash-only output — prints the relevant digest per result. | @@ -45,19 +45,104 @@ Formatters don't set exit codes — that's `ExceptionHandlingGroup`'s job, by ex **green** "Known Good" signal (a stronger benign signal than "clean") instead of the misleading "no engines responded — rescan now" a window-closed/no-assertion instance would otherwise get. It is gated by a single -`is_known_good = (state == 'KNOWN_GOOD') or known_good_sources` flag: +`is_known_good = (state == 'KNOWN_GOOD')` flag: - **`state == 'KNOWN_GOOD'`** (the SDK's `ArtifactInstance.state`, the friendly - bounty-state name) is the reliable signal — it fires even for a scan-bypassed - instance that carries **no** feed metadata. + bounty-state name) is **the** signal, and the only one — it is the server's single + reliable statement that this artifact is known-good and its bytes are withheld, and + it fires even for a scan-bypassed instance that carries **no** feed metadata. There + is deliberately no separate "bytes withheld" field to consult. + +**The wire dependency, and where it is actually pinned.** `state` is load-bearing with no +fallback, and a Style-3 test cannot see the transport boundary — it compares against a dict the +test wrote itself. So the key and the label were **read off the server's serializer**, not +inferred: `ArtifactInstanceSerializer` emits `'state': instance.state.name`, and +`BountyState.KNOWN_GOOD.name` is the exact string `'KNOWN_GOOD'`. What would catch a *future* +rename is the server's own suite (which asserts that field and the `/v3/sample` status), not a +cassette here: this repo's CI replays frozen recordings with no VCR-off e2e job, so a recorded +body would keep replaying the old shape after a rename. If a live-e2e job is ever added here, a +`cli_test.py` cassette over a known-good hash becomes the right place to pin it. - **`known_good_sources`** (the sorted flagging-feed names, from - `ArtifactInstance.known_good`) is the richer signal: when present, the - **Detections** line names the feeds ("…known-good binary (flagged by: …); it is - not scanned."); otherwise it reads "…is a known-good binary; it is not scanned." - It also keeps known-good rendering working against an older SDK that has the feed - list but not `.state`. - -The **Status** line reads "Known good" whenever `is_known_good`. Both attributes are -read with `getattr(..., None)` so a CLI on an older SDK (missing either field) -renders exactly as before. `JSONOutput` needs no change — it dumps the resource's + `ArtifactInstance.known_good`) only **shapes the message** for an instance already + known-good by state: when present, the **Detections** line names the feeds + ("…known-good binary (flagged by: …); it is not scanned."); otherwise it reads + "…is a known-good binary; it is not scanned." It must **never** decide + known-goodness — the server emits the feed list for *any* instance whose sha256 + matches a known-good record, including a fully scanned one with real + detections/PolyScore, so treating it as the signal would render a scanned artifact + as "known-good … not scanned". It is therefore read as `[]` unless `is_known_good` — + a statement of that coupling, not a guard: the list is only ever read inside the + known-good branch, so the ternary can't change what renders today. Keep it, and keep + reading the feeds through it if a second call site ever appears. + +### A known-good instance that also carries results + +Known-goodness and collected results are **not** mutually exclusive. An instance that was +scanned before its artifact was catalogued gets reconciled to `KNOWN_GOOD` with its +assertions, detections and PolyScore deliberately preserved, so that pairing is something +a client really receives — the rendering must state both facts without claiming the +artifact was never scanned. The trailing clause of the known-good **Detections** line +therefore switches on `instance.valid_assertions`: + +- **no valid assertions** → "…is a known-good binary; it is not scanned." — the withheld, + never-scanned case. +- **valid assertions, window closed** → "…is a known-good binary; N/M engines reported + malicious." — the same count the ordinary window-closed branch renders, folded into the + known-good sentence. +- **valid assertions, window still open** → "…is a known-good binary; its scan has not + finished running yet." Every other **Detections** branch guards its counts on + `window_closed`, because an open window's numbers are not final. Unreachable through the + server's reconciliation — verified against its `RECONCILABLE_STATES`, which is `STORED` + (a row that was never submitted, so it carries no assertions) plus `SETTLED` **and only + when `window_closed`**; the ingest gate's own path sets `window_closed = True`. So this is + a guard against a future state rather than a case seen in practice, stated here so the + parity with the other branches reads as intentional. If a reconciled row ever *can* carry + an open window alongside preserved malicious assertions, redden on `malicious_assertions` + alone and keep only the *text* guarded on the window — the sentence would still be + accurate, and green on a malicious detection is the mis-signal this rule exists to fix. + +The flagging-feed attribution ("(flagged by: …)") is orthogonal and applies to all three. In +every branch the per-engine verdict list, the PolyScore line and "Status: Known good" +render as usual; the switch exists so "it is not scanned" is never printed directly above a +list of engine verdicts. + +**Colour: any malicious verdict outranks the withheld-bytes signal.** The line is **green** +except on the *count* branch (valid assertions, window closed) when at least one of them is +malicious, where it is **red** — the same threshold and the same colour the ordinary branch +applies to that same count (1/50 reddens there too). The clause and its colour are chosen in +the **same** `if`/`elif`, not by a second condition: written separately they had to agree by +coincidence, and only the SDK's guarantee that `malicious_assertions ⊆ valid_assertions` (both +filter on `mask`) kept them in step — a fact about another repo propping up this rendering. Known-goodness is the +dominant *fact*, but it is not a stronger *warning*: rendering "40/50 engines reported +malicious" in green would be a weaker signal than the very same instance produced before it +was reconciled, which is the class of mis-signal this rendering exists to fix. "Status: Known +good" stays green in both cases — it labels the catalogue status (the counterpart of "Status: +Assertion window closed"), not the verdict. The colour decision is invisible to a test that unstyles its +output, so it is pinned against the **styled** render — `TextOutput(color=True)` read without +`click.unstyle`, which is what `_render_styled` in `known_good_field_test.py` exists for (both +halves matter: the flag makes it paint, the missing `unstyle` keeps the codes). Whether the +`--color/--no-color` **flag** reaches that rendering at all is a separate question a formatter +unit test cannot answer; `test_color_flag_reaches_the_text_formatter` (`cli_test.py`) covers it +through `CliRunner(… color=True)`. + +**Scope of `--no-color`.** It governs the text formatter (via `_paint`), `PrettyJSONOutput` +(whose `_to_json` skips the pygments `ClickFormatter` when the flag is off — plain `JSONOutput` +emits unstyled JSON and has nothing to honour), and the log prefix +(`setup_logging(verbosity, color=…)` — the `NamedColorFormatter` used to style unconditionally, +so `polyswarm --no-color -v …` still emitted a green prefix on a tty). It does not attempt to +suppress colour inside third-party output. + +The **Status** line reads "Known good" whenever `is_known_good`, except on a **failed** +instance — "Status: Failed" is ordered first and the known-good **Detections** branch is +gated on `not instance.failed`, so a failure is reported as a failure and never as +"…it is not scanned". + +Both attributes are read with `getattr(..., None)` so a CLI on an older SDK (missing +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 `.json`, which already carries the raw `state` and `known_good` keys. diff --git a/specs/04-testing.md b/specs/04-testing.md index 5aa424c..7b100e6 100644 --- a/specs/04-testing.md +++ b/specs/04-testing.md @@ -2,11 +2,11 @@ ## Scope -How the CLI is tested: the `CliRunner` harness, the two mocking styles (SDK-boundary mocks vs VCR cassettes), the cassette layout and record workflow, and how to run the suite. Files: `tests/`, `tests/vcr/`, `src/conftest.py`, `pyproject.toml` (`[project.optional-dependencies].tests`, `[tool.pytest.ini_options]`). +How the CLI is tested: the `CliRunner` harness, the two mocking styles (SDK-boundary mocks vs VCR cassettes), the formatter-unit style for pure rendering, the cassette layout and record workflow, and how to run the suite. Files: `tests/`, `tests/vcr/`, `src/conftest.py`, `pyproject.toml` (`[project.optional-dependencies].tests`, `[tool.pytest.ini_options]`). ## Invariants -- **Tests drive the CLI through `click.testing.CliRunner`** — they invoke the real command tree, not internal functions. No live PolySwarm stack is required. +- **Anything that is command behaviour is driven through `click.testing.CliRunner`** — argument parsing, the SDK call, the wiring, the exit code: exercise the real command tree, never an internal function standing in for it. No live PolySwarm stack is required. The one sanctioned exception is pure rendering logic — see [Style 3](#style-3--formatter-unit-tests). - **Mock at the SDK boundary, or replay HTTP with VCR — never both for the same path.** A test either patches `polyswarm_api.api.PolyswarmAPI.` (unit-style) or lets VCR replay recorded HTTP (end-to-end). The CLI's own code is exercised either way. - **VCR is an efficiency cache, not a load-bearing requirement.** The suite must pass against a live e2e stack with VCR off. Don't hardcode `record_mode='none'`; if a test only works against its recorded cassette, that's a bug in the test. - **Never `cp` a cassette from a sibling test, never hand-edit cassette bytes.** Re-record against a live stack. @@ -46,6 +46,12 @@ pytest tests/cli_test.py:::: # records against whatever stack you Point your environment at a live e2e stack, run the test, and commit the freshly recorded `.vcr` (+ updated `.click`). The deletion is what makes VCR record. +## Style 3 — formatter unit tests + +For **rendering logic with no command-tree behaviour** — which labelled line a given field set produces — construct the formatter directly (`TextOutput(color=False)`) and call the resource method with an SDK resource built from a literal dict, asserting on the returned lines (`write=False`, no stream, no cassette). Example: `tests/known_good_field_test.py` renders `ArtifactInstance`s that differ only in `state` / `known_good` and asserts which Detections/Status line comes out. This is the right choice when the branch matrix is wide and every branch is a function of the resource's fields — a cassette per branch would mean recording a server state that only the formatter cares about. + +Use it **only** for that. Argument parsing, SDK calls, generator consumption, `ctx.obj` wiring and exit codes are command behaviour: a formatter unit test can't observe them, so those need Style 1 or Style 2. A command whose rendering is covered by Style 3 still needs at least one `CliRunner` test proving the command reaches the formatter at all. + ## What to test for a new command 1. The command parses its arguments and calls the expected SDK method with the expected arguments. diff --git a/specs/05-sdk-contract.md b/specs/05-sdk-contract.md index 2b3f154..3957a91 100644 --- a/specs/05-sdk-contract.md +++ b/specs/05-sdk-contract.md @@ -69,6 +69,19 @@ When a CLI feature needs an SDK surface that doesn't exist yet: - The pin lives in `pyproject.toml` `dependencies` (`polyswarm_api>=…`). Floor it at the lowest SDK version exposing everything the CLI uses; cap it below the next known-incompatible major when one is anticipated. - The CLI is **sync-only** — it imports `polyswarm_api.api.PolyswarmAPI`, never `polyswarm_api.aio`. Don't add the `polyswarm_api[async]` extra. - Bumping the pin is a normal code change; bumping the CLI's *own* version is a release step (`AGENTS.md` §Gitflow). They're unrelated. +- 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. + +### Current floor — `polyswarm_api>=4.2.0` + +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: + +1. **`llm_report_create` sends the client's community.** 4.2.0 passes `community=self.community` when it builds the report resource; 4.1.0 omits it. `report llm-create` (`client/report.py`) supplies no community of its own — it relies entirely on the client's — so on 4.1.0 a report requested for a sample in a private community is created without one. No error, wrong resource. +2. **A streaming download answered `204 No Content` raises `NoResultsException`.** The streaming path bypasses `parse_response`, so the 204 has to be raised by the session itself; 4.2.0 does that, 4.1.0 has no such raise anywhere in its session. The CLI's `download` commands depend on it for the no-results **exit code `1`** (§No-results signalling); against 4.1.0 an empty response reads as a successful download and exits `0`. + +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. ## Worked example — the httpx SDK migration diff --git a/src/polyswarm/__init__.py b/src/polyswarm/__init__.py index 0d6a4f2..5ee6158 100644 --- a/src/polyswarm/__init__.py +++ b/src/polyswarm/__init__.py @@ -1 +1 @@ -__version__ = '4.2.1' +__version__ = '4.3.0' diff --git a/src/polyswarm/client/polyswarm.py b/src/polyswarm/client/polyswarm.py index 1a3f83f..8db3c81 100644 --- a/src/polyswarm/client/polyswarm.py +++ b/src/polyswarm/client/polyswarm.py @@ -92,7 +92,7 @@ def resolve_api_uri(api_uri, api_uri_from_cli, shortcuts): return PROD_API_URI -def setup_logging(verbosity): +def setup_logging(verbosity, color=True): # explicitly set to stderr just in case # this is the new default for click_log it seems core.ClickHandler._use_stderr = True @@ -105,8 +105,12 @@ def format(self, record): level = record.levelname.lower() msg = record.getMessage() if level in self.colors: - prefix = click.style(f'{level} [{record.name}]: ', - **self.colors[level]) + # `--no-color` governs the log prefix too. Without the flag here it + # styled unconditionally, so `polyswarm --no-color -v …` still emitted a + # green prefix on a tty — the same half-honoured flag the formatters had. + prefix = f'{level} [{record.name}]: ' + if color: + prefix = click.style(prefix, **self.colors[level]) msg = '\n'.join(prefix + x for x in msg.splitlines()) return msg return logging.Formatter.format(self, record) @@ -209,7 +213,7 @@ def polyswarm_cli(ctx, api_key, api_uri, output_file, output_format, color, verb This is a PolySwarm CLI client, which allows you to interact directly with the PolySwarm network to scan files, search hashes, and more. """ - setup_logging(verbose) + setup_logging(verbose, color=color) logger.info('Running polyswarm-cli version %s with polyswarm-api version %s', polyswarm.__version__, polyswarm_api.__version__) diff --git a/src/polyswarm/formatters/text.py b/src/polyswarm/formatters/text.py index 9c3fddf..1810eba 100644 --- a/src/polyswarm/formatters/text.py +++ b/src/polyswarm/formatters/text.py @@ -32,7 +32,6 @@ def __init__(self, color=True, output=sys.stdout, **kwargs): super().__init__(output) self.color = color self._depth = 0 - self.color = color def _get_score_format(self, score): if score < 0.3: @@ -79,21 +78,47 @@ def artifact_instance(self, instance, write=True, timeout=False): # Defensive getattr: these attributes ship in the paired SDK release, but a # CLI running against an older installed SDK won't have them (no AttributeError). - known_good_sources = getattr(instance, 'known_good_sources', None) or [] - # A known-good binary is signalled by the bounty state (KNOWN_GOOD) — reliable - # even for a scan-bypassed instance with no feed metadata. known_good_sources - # (the flagging feeds) is the richer signal when present, and also covers an - # older SDK that lacks .state but still carries the feed list. - is_known_good = getattr(instance, 'state', None) == 'KNOWN_GOOD' or bool(known_good_sources) + # The bounty state (KNOWN_GOOD) is the only reliable signal that this artifact is + # a known-good binary whose bytes are withheld — it alone decides. known_good_sources + # (the flagging feeds) is emitted for any instance whose sha256 matches a known-good + # record, including a fully scanned one carrying real detections, so it only shapes + # the message; reading it as the signal would render a scanned artifact "not scanned". + is_known_good = getattr(instance, 'state', None) == 'KNOWN_GOOD' + known_good_sources = (getattr(instance, 'known_good_sources', None) or []) if is_known_good else [] if is_known_good and not instance.failed: - if known_good_sources: - feeds = ', '.join(known_good_sources) - output.append(self._green( - f'Detections: This artifact is a known-good binary (flagged by: {feeds}); it is not scanned.')) + # A known-good binary can still carry results: an instance scanned before the + # artifact was catalogued is reconciled to KNOWN_GOOD with its assertions, + # detections and PolyScore preserved. Report those instead of the "not scanned" + # clause, which would contradict the per-engine verdicts printed just below. + attribution = f' (flagged by: {", ".join(known_good_sources)})' if known_good_sources else '' + # The clause and its colour are decided in the SAME branch, deliberately. They + # were two differently-shaped conditions that had to agree — the text keyed on + # `valid_assertions and window_closed`, the colour on `malicious_assertions and + # window_closed` — and only the SDK's guarantee that malicious_assertions is a + # subset of valid_assertions (both filter on `mask`, one additionally on `verdict`) + # kept them in step. That is a fact about another repo holding this rendering + # together; deciding both at once removes the dependency. + if len(instance.valid_assertions) > 0 and instance.window_closed: + detections = f'{len(instance.malicious_assertions)}/{len(instance.valid_assertions)} engines reported malicious' + # ANY malicious verdict outranks the withheld-bytes signal, which is the same + # threshold the ordinary branch applies to this same count: green on "40/50 + # engines reported malicious" is a weaker warning than the instance gave before + # it was reconciled. Red only here, because this is the only branch that renders + # a final count. + paint = self._red if instance.malicious_assertions else self._green + elif len(instance.valid_assertions) > 0: + # Every other branch guards its counts on window_closed, because an open + # window's numbers are not final. Not reachable through reconciliation today + # (a STORED row carries no assertions and a SETTLED one has a closed window), + # stated rather than left implied. + detections = 'its scan has not finished running yet' + paint = self._green else: - output.append(self._green( - 'Detections: This artifact is a known-good binary; it is not scanned.')) + detections = 'it is not scanned' + paint = self._green + output.append(paint( + f'Detections: This artifact is a known-good binary{attribution}; {detections}.')) elif instance.community == 'stream': output.append(self._white('Detections: This artifact has not been scanned. You can trigger a scan now.')) elif len(instance.valid_assertions) == 0 and instance.window_closed and not instance.failed: @@ -887,25 +912,37 @@ def sample(self, result, write=True): return self._output(output, write) + def _paint(self, text, fg): + """Apply a colour, or don't — the one place `self.color` is honoured. + + It used to be assigned and never read, so every one of these helpers styled + unconditionally and `--no-color` was a no-op for text output — invisible in practice + because click strips ANSI when stdout is not a tty. (`PrettyJSONOutput` did honour it, + which is why the flag looked wired up; plain `JSONOutput` emits unstyled JSON and has + nothing to honour.) Not decorated with `is_grouped`: the callers already are, and the + indent must be applied exactly once. + """ + return click.style(text, fg=fg) if self.color else text + @is_grouped def _white(self, text): - return click.style(text, fg='white') + return self._paint(text, 'white') @is_grouped def _yellow(self, text): - return click.style(text, fg='yellow') + return self._paint(text, 'yellow') @is_grouped def _red(self, text): - return click.style(text, fg='red') + return self._paint(text, 'red') @is_grouped def _blue(self, text): - return click.style(text, fg='blue') + return self._paint(text, 'blue') @is_grouped def _green(self, text): - return click.style(text, fg='green') + return self._paint(text, 'green') def _open_group(self): self._depth += 1 diff --git a/tests/cli_test.py b/tests/cli_test.py index 7df6967..6f23b7c 100644 --- a/tests/cli_test.py +++ b/tests/cli_test.py @@ -3,10 +3,13 @@ import json import yaml import traceback -from unittest import TestCase +from unittest import TestCase, mock + +from polyswarm_api import resources from pathlib import Path import vcr as vcr_ +import click from click.testing import CliRunner from polyswarm.client import polyswarm as client @@ -53,9 +56,12 @@ def click_vcr(self, result, name='result', replace=None): yaml.dump(data, f) return entry - def _run_cli(self, commands): + def _run_cli(self, commands, color=False): commands = ['-a', self.api_key, '-u', self.api_url, '-c', self.community] + commands - return self.cli.invoke(client.polyswarm_cli, commands, catch_exceptions=False) + # color=False is CliRunner's default and strips ANSI, which is what every cassette + # expectation was recorded against. Pass color=True only to assert the styling itself. + return self.cli.invoke(client.polyswarm_cli, commands, catch_exceptions=False, + color=color) def _assert_text_result(self, result, expected_result, expected_return_code=0, replace=None): current_result = self._replace(replace, result.output) @@ -319,6 +325,74 @@ def test_search_hash_with_text_output(self): '--output-format', 'text', 'search', 'hash', self.eicar_hash]) self._assert_text_result(result, self.click_vcr(result)) + @staticmethod + def _one_instance(): + """A minimal parsed instance for the colour tests. + + They mock at the **SDK boundary** — `polyswarm_api.api.PolyswarmAPI.search`, which is + what specs/04 Style 2 means — rather than replaying a cassette: the response content is + irrelevant to whether `--color` reaches the renderer, a cassette would have to be + recorded against a live stack for a test that never exercises the server, and borrowing + another test's cassette couples the two through the re-record path (unittest orders + methods alphabetically, so this one would end up authoring it). + + Patching `Polyswarm.search_hashes` instead would be the wrong seam: that is CLI code, + so it would cut `utils.parallel_executor_iterable_results` out of the run. + """ + return resources.ArtifactInstance({ + 'sha256': 'a' * 64, 'md5': 'c' * 32, 'sha1': 'b' * 40, + 'mimetype': 'text/plain', 'size': 68, 'extended_type': '', + 'first_seen': '2020-01-01T00:00:00', 'upload_url': '', 'metadata': [], + 'id': '111', 'community': 'gamma', 'assertions': [], 'votes': [], + 'failed': False, 'window_closed': True, 'polyscore': None, + }) + + def _run_color_pair(self, extra_args=()): + """The same command twice, differing only in the colour flag.""" + outputs = [] + for flag in ('--color', '--no-color'): + with mock.patch('polyswarm_api.api.PolyswarmAPI.search', + return_value=iter([self._one_instance()])): + result = self._run_cli( + [*extra_args, flag, '--output-format', 'text', + 'search', 'hash', 'a' * 64], + color=True) + assert result.exit_code == 0, result.output + outputs.append(result) + return outputs + + def test_color_flag_reaches_the_text_formatter(self): + # The bug was that `--color/--no-color` never reached the rendering: TextOutput + # assigned `self.color` and read it nowhere. A formatter unit test cannot see this — + # the flag travels through `formatters[output_format](color=color, …)` in the command + # group, and specs/04 says argument parsing and ctx.obj wiring need Style 1 or 2. + # CliRunner's color=True stops click stripping ANSI off the non-tty capture, so the + # two runs differ only in the flag. + colored, plain = self._run_color_pair() + + assert '\x1b[' in colored.output + assert '\x1b[' not in plain.output + # Same content either way — the flag drops the wrapper, never the text. + assert click.unstyle(colored.output) == plain.output + + def test_color_flag_reaches_the_log_prefix(self): + # The other half of the flag, and it needs -v: at the default verbosity the level is + # WARNING, so no record ever reaches NamedColorFormatter and the formatter-only test + # above cannot observe this branch. The prefix used to be styled unconditionally, so + # `--no-color -v` still emitted a green `info [polyswarm]: ` on a tty. + colored, plain = self._run_color_pair(extra_args=('-v',)) + + # The version line is logged at INFO by the group itself, so it is always present. + assert 'Running polyswarm-cli version' in click.unstyle(colored.output) + assert 'Running polyswarm-cli version' in plain.output + # The prefix names the emitting logger, e.g. `info [polyswarm.client.polyswarm]: `. + assert 'info [polyswarm' in click.unstyle(colored.output) + assert 'info [polyswarm' in plain.output + # Styled only under --color: the prefix is what carries the ANSI here, and the + # formatter output alongside it is white-styled the same way. + assert '\x1b[32minfo [polyswarm' in colored.output + assert '\x1b[' not in plain.output + @vcr.use_cassette() def test_search_hash_with_no_results(self): result = self._run_cli([ diff --git a/tests/known_good_field_test.py b/tests/known_good_field_test.py index f35f9b7..24b5248 100644 --- a/tests/known_good_field_test.py +++ b/tests/known_good_field_test.py @@ -1,10 +1,13 @@ """Pure-unit rendering tests for the known_good field on an artifact instance. No CliRunner / VCR — these drive the text formatter directly with a constructed -ArtifactInstance (the SDK resource), asserting that a known-good binary renders -its flagging feeds and a "Known good" status instead of the misleading -"no engines responded — rescan now" message, and that a normal instance is -unchanged. +ArtifactInstance (the SDK resource), asserting that a known-good binary — signalled +by its state, the only reliable signal — renders its flagging feeds and a +"Known good" status instead of the misleading "no engines responded — rescan now" +message, that the flagging feeds on their own never make an instance known-good, +that a failure outranks the known-good signal, that a known-good instance which +also carries preserved results reports them instead of claiming it was not +scanned, and that a normal instance is unchanged. """ import click from polyswarm_api import resources @@ -13,6 +16,13 @@ SHA = 'a' * 64 +FEEDS = [ + {'tool': 'commercial', 'tool_metadata': {}, 'created': '2026-06-11T00:00:00', + 'updated': '2026-06-11T00:00:00'}, + {'tool': 'nsrl', 'tool_metadata': {}, 'created': '2026-06-11T00:00:00', + 'updated': '2026-06-11T00:00:00'}, +] + def _instance(**overrides): content = { @@ -26,25 +36,31 @@ def _instance(**overrides): return resources.ArtifactInstance(content) +def _assertion(engine, verdict): + return { + 'author': '0x' + 'd' * 40, 'author_name': engine, 'engine': {'name': engine}, + 'bid': '1000000000000000', 'mask': True, 'metadata': None, 'verdict': verdict, + } + + def _render(instance): return click.unstyle('\n'.join(TextOutput(color=False).artifact_instance(instance, write=False))) -class TestKnownGoodTextRendering: - def test_known_good_instance_renders_feeds_and_status(self): - feeds = [ - {'tool': 'commercial', 'tool_metadata': {}, 'created': '2026-06-11T00:00:00', - 'updated': '2026-06-11T00:00:00'}, - {'tool': 'nsrl', 'tool_metadata': {}, 'created': '2026-06-11T00:00:00', - 'updated': '2026-06-11T00:00:00'}, - ] - text = _render(_instance(known_good=feeds)) - # Feeds are listed (sorted) and the status reflects known-good. - assert 'known-good binary (flagged by: commercial, nsrl); it is not scanned.' in text - assert 'Status: Known good' in text - # A known-good binary must NOT be told to rescan / that no engines responded. - assert 'trigger a rescan' not in text +def _render_styled(instance): + """Keeps the ANSI wrapper, so the green/red decision itself is observable — every other + test here unstyles, which makes the colour (the whole point of the known-good rendering) + invisible. **Both** halves are load-bearing: `color=True` is what makes TextOutput paint + at all (see `_paint`), and the absent `click.unstyle` is what keeps the codes in the + string. Flip either and every colour assertion below stops asserting anything.""" + return '\n'.join(TextOutput(color=True).artifact_instance(instance, write=False)) + + +def _detections_line(styled_text): + return next(line for line in styled_text.split('\n') if 'Detections:' in line) + +class TestKnownGoodTextRendering: def test_normal_instance_unchanged(self): text = _render(_instance()) assert 'known-good' not in text.lower() @@ -67,14 +83,158 @@ def test_state_known_good_without_feeds(self): assert 'No engines responded' not in text def test_state_known_good_with_feeds_lists_them(self): - feeds = [{'tool': 'commercial', 'tool_metadata': {}, - 'created': '2026-06-11T00:00:00', 'updated': '2026-06-11T00:00:00'}] - text = _render(_instance(state='KNOWN_GOOD', known_good=feeds)) - # When feeds are present the richer message still lists them. - assert 'known-good binary (flagged by: commercial); it is not scanned.' in text + text = _render(_instance(state='KNOWN_GOOD', known_good=FEEDS)) + # When feeds are present the richer message still lists them (sorted). + assert 'known-good binary (flagged by: commercial, nsrl); it is not scanned.' in text + assert 'Status: Known good' in text + + def test_known_good_with_results_reports_them_instead_of_not_scanned(self): + # A previously scanned instance reconciled to KNOWN_GOOD keeps its assertions, + # detections and PolyScore. The assertion loop is not gated on the known-good + # branch, so the old copy printed "it is not scanned." immediately above the + # per-engine verdicts — a self-contradiction. The Detections line must state + # both facts: still a known-good binary, and here is what the engines found. + assertions = [_assertion('engine-a', True), _assertion('engine-b', False)] + text = _render(_instance(state='KNOWN_GOOD', known_good=FEEDS, assertions=assertions, + polyscore=0.9)) + assert ('Detections: This artifact is a known-good binary (flagged by: commercial, nsrl); ' + '1/2 engines reported malicious.') in text + assert 'it is not scanned' not in text + # The verdicts that made the old phrasing contradictory still render, as does the + # known-good status and the preserved PolyScore. + assert 'engine-a: Malicious' in text + assert 'engine-b: Clean' in text assert 'Status: Known good' in text + # Anchored on the line, not on a prefix: `PolyScore: 0.9` also matches 0.95, and the + # renderer's :.20f means the line is not the literal '0.9' either. + polyscore_line = next(line for line in text.split('\n') if line.startswith('PolyScore:')) + assert float(polyscore_line.removeprefix('PolyScore:').strip()) == 0.9 + + def test_malicious_detections_outrank_the_known_good_colour(self): + # The colour is the signal here, and no other test can see it (they all unstyle). + # Rendering "1/2 engines reported malicious" in green — as this branch first did, + # replacing code that rendered the identical count in red — is a weaker warning + # than the same instance gave before it was reconciled. + assertions = [_assertion('engine-a', True), _assertion('engine-b', False)] + styled = _render_styled( + _instance(state='KNOWN_GOOD', known_good=FEEDS, assertions=assertions)) + assert _detections_line(styled) == click.style( + 'Detections: This artifact is a known-good binary (flagged by: commercial, nsrl); ' + '1/2 engines reported malicious.', fg='red') + # The Status line is the counterpart of "Assertion window closed": it labels the + # catalogue status, not the verdict, so it stays green while the verdict goes red. + # Stated as a rule in specs/03 — pinned here so the two can't drift. + assert click.style('Status: Known good', fg='green') in styled + + def test_clean_known_good_stays_green(self): + # The other side of the same decision: nothing reported malicious, so the benign + # colouring is right and the red must not leak into it. + line = _detections_line(_render_styled( + _instance(state='KNOWN_GOOD', known_good=FEEDS, + assertions=[_assertion('engine-a', False)]))) + assert line == click.style( + 'Detections: This artifact is a known-good binary (flagged by: commercial, nsrl); ' + '0/1 engines reported malicious.', fg='green') + + def test_open_window_does_not_present_counts_as_final(self): + # Every other Detections branch guards its counts on window_closed. Not reachable + # through reconciliation (a STORED row carries no assertions, a SETTLED one has a + # closed window), pinned so the missing guard cannot come back by accident. + instance = _instance(state='KNOWN_GOOD', known_good=FEEDS, window_closed=False, + assertions=[_assertion('engine-a', True)]) + text = _render(instance) + assert 'its scan has not finished running yet.' in text + assert 'engines reported malicious' not in text + # ...and it stays GREEN. This is the one combination where the `and window_closed` + # conjunct in the colour check is load-bearing: the instance has a malicious + # assertion, so without it the line reddens while saying the scan is unfinished. + assert _detections_line(_render_styled(instance)) == click.style( + 'Detections: This artifact is a known-good binary (flagged by: commercial, nsrl); ' + 'its scan has not finished running yet.', fg='green') + + def test_known_good_with_results_and_no_feeds(self): + # Same reconciliation without a flagging-feed attribution: the feed clause is + # orthogonal to the results clause, so dropping one must not drop the other. + assertions = [_assertion('engine-a', False)] + text = _render(_instance(state='KNOWN_GOOD', assertions=assertions)) + assert 'Detections: This artifact is a known-good binary; 0/1 engines reported malicious.' in text + assert 'it is not scanned' not in text + assert 'Status: Known good' in text + + def test_known_good_with_only_non_responding_engines_is_still_not_scanned(self): + # Assertions that all declined (mask False) are not results — valid_assertions is + # empty — so the withheld/never-scanned clause is still the accurate one. Keying + # the switch on `instance.assertions` instead would render "0/0 engines". + declined = _assertion('engine-a', None) + declined['mask'] = False + text = _render(_instance(state='KNOWN_GOOD', assertions=[declined])) + assert 'Detections: This artifact is a known-good binary; it is not scanned.' in text + assert 'engines reported malicious' not in text def test_non_known_good_state_unchanged(self): text = _render(_instance(state='SETTLED')) assert 'known-good' not in text.lower() assert 'Status: Known good' not in text + + def test_failed_instance_reports_the_failure_not_known_good(self): + # A failure outranks the known-good signal: the Detections branch is gated on + # `not instance.failed` and 'Status: Failed' is ordered ahead of 'Status: Known + # good'. Dropping either would tell the user a failed lookup is a benign binary. + text = _render(_instance(state='KNOWN_GOOD', known_good=FEEDS, failed=True, + failed_reason='artifact storage unavailable')) + assert 'Detections: This scan has failed. Please try again.' in text + assert 'Status: Failed' in text + assert 'Failure Reason: artifact storage unavailable' in text + # Never claim the artifact is a known-good binary that was not scanned. + assert 'it is not scanned' not in text + assert 'Status: Known good' not in text + + +class TestColorFlag: + """`--no-color` reaches the formatter as `TextOutput(color=False)`. It used to be + assigned and never read, so text output styled unconditionally and the flag did + nothing — masked in practice by click stripping ANSI off a non-tty.""" + + def test_no_color_emits_no_ansi(self): + assertions = [_assertion('engine-a', True), _assertion('engine-b', False)] + instance = _instance(state='KNOWN_GOOD', known_good=FEEDS, assertions=assertions) + plain = '\n'.join(TextOutput(color=False).artifact_instance(instance, write=False)) + assert '\x1b[' not in plain + # Same text, just unpainted — the flag drops the wrapper, never the content. + assert plain == click.unstyle('\n'.join( + TextOutput(color=True).artifact_instance(instance, write=False))) + + +class TestKnownGoodFeedsAreNotTheSignal: + """The flagging-feed list is served for every instance whose sha256 matches a + known-good record — including one that was scanned before it was flagged — so it + only shapes the message for an instance already known-good by state.""" + + def test_feeds_without_known_good_state_are_ignored(self): + text = _render(_instance(state='SETTLED', known_good=FEEDS)) + assert 'known-good' not in text.lower() + assert 'Status: Known good' not in text + assert 'Status: Assertion window closed' in text + + def test_feeds_without_any_state_render_as_an_ordinary_instance(self): + # The no-state path: an older server omits `state`, which parses to None, leaving the + # feed list as the only known-good hint — and it must not be used. This is the + # assertion that catches the removed feed-list fallback being reintroduced. + # (`getattr(instance, 'state', None)` also tolerates an SDK with no such attribute at + # all, but that is belt-and-braces rather than a supported configuration, and pinning + # it would mean deleting an attribute off a parsed SDK resource — reverse-engineering + # internals the CLI is not allowed to depend on. See specs/05.) + text = _render(_instance(known_good=FEEDS)) + assert 'known-good' not in text.lower() + assert 'Status: Known good' not in text + # Rendered exactly like any other window-closed instance with no assertions. + assert 'Detections: No engines responded to this scan. You can trigger a rescan now.' in text + assert 'Status: Assertion window closed' in text + + def test_scanned_instance_with_feeds_reports_its_detections(self): + assertions = [_assertion('engine-a', True), _assertion('engine-b', False)] + text = _render(_instance(state='SETTLED', known_good=FEEDS, assertions=assertions, + polyscore=0.9)) + # The real scan results are reported, never overwritten by a "not scanned" claim. + assert 'Detections: 1/2 engines reported malicious' in text + assert 'it is not scanned' not in text