fix(text): render NOT_STORED instead of claiming a finished scan - #273
mjbradford89 wants to merge 1 commit into
Conversation
An artifact whose bytes were declined as known-good at submission carries window_closed=True, so it fell through to "Status: Assertion window closed" — which reads as a completed scan that produced nothing, for an artifact that was never scanned and holds no bytes to scan. Give NOT_STORED its own branch, ahead of window_closed, naming both the cause and the remedy (resubmit the file). Also corrects the known_good_sources comment, which described a server contract that no longer holds. The server gates that field on the same predicate it reports KNOWN_GOOD from, so the feeds cannot arrive without the state. The converse does not hold — the state can arrive with no feeds — which is why the attribution stays guarded rather than assuming the list is populated.
Review — 2 blocking, 3 smallerGitflow is clean (base 1. Correctness — the Detections line still claims a finished scan and offers a rescan that can't happenThe fix only touches the Status ladder. The Detections ladder is untouched, and a That's the pre-existing test It's also the same mis-signal Test gap, same item: the new tests assert only the Status line. The 2. Spec drift —
|
SummaryWhen the platform refuses a download or rescan for a hash whose bytes it deliberately never stored, it said "the file does not exist" — the same words a genuinely absent artifact gets. The reporter read that as data loss and spent an investigation hunting a retention bug across 297 files in 47 container images. The server side splits the refusal so it names the cause; this PR is the CLI half, giving Severity: 0 HIGH · 3 MODERATE · 5 LOW. Prior feedback: 9 checked · 9 open.
Fixes are proposed, not applied; nothing was run. Cross-repo coordination
Merge order: Contracts crossing the set: Coherence: fixes are independent; no cross-repo adjustment needed. Gaps: this PR's branch Findings (round 1)Every finding below is work for this change set; each entry's [MODERATE] F3. The CLI offers a rescan the server always refusesWhat happens: One block of CLI output tells the user two incompatible things: that no engines responded and they can trigger a rescan now, and — once this PR lands — that the bytes were never stored and the file must be resubmitted. The rescan it invites returns 404. The user retries an operation that can never succeed, which is the "is my data gone?" confusion the ticket was opened to remove.
Lands in: polyswarm-cli elif is_not_stored and not instance.failed and len(instance.valid_assertions) == 0:
# Ahead of the "no engines responded" arm for the same reason the Status
# branch sits ahead of window_closed: these rows carry window_closed=True
# with no assertions, so that arm offered a rescan the server refuses with
# a 404 (check_binary) for bytes it never stored.
output.append(self._white(
'Detections: This artifact has not been scanned; no bytes were stored '
'to scan.'))This needs the branch back: restore
elsewhere: F1, F2, F4, F8 → polyswarm/artifact-index#1978 · F5, F6 → polyswarm/polyswarm-api#326 Outstanding review feedback
Standards conformitySet-level. §14 delivery order ✓ — the server PR ships no capability needing SDK or CLI support, since Change-level. §15 ✗ → F7. Project-level. §16 ✓ — no floor change is owed: this PR reads Clean: §2, §16. Non-clean: §15 ✗. Not applicable: §3–§14, §17 — a client, so no queues, storage, AKM, charts, clusters or request paths. |
SummaryWhen the platform refuses a download or rescan for a hash whose bytes it deliberately never stored, it said "the file does not exist" — the same words a genuinely absent artifact gets, which sent the reporter hunting a retention bug across 297 files in 47 container images. The server now answers that row first, above Severity: 0 HIGH · 1 MODERATE · 6 LOW. Prior feedback: 13 checked · 10 open.
Fixes are proposed, not applied; nothing was run. Cross-repo coordination
Merge order: Contracts crossing the set: Coherence: fixes are independent; no cross-repo adjustment needed. Gaps: this PR's branch Findings (round 2)Every finding below is work for this change set; each entry's [MODERATE] F1. The CLI offers a rescan the server always refusesWhat happens: One block of CLI output tells the user two incompatible things: that no engines responded and they can trigger a rescan now, and — once this PR lands — that the bytes were never stored and the file must be resubmitted. The rescan it invites returns 404. The user retries an operation that can never succeed, which is the "is my data gone?" confusion the ticket was opened to remove.
Lands in: polyswarm-cli elif is_not_stored and not instance.failed and len(instance.valid_assertions) == 0:
# Ahead of the "no engines responded" arm for the same reason the Status
# branch sits ahead of window_closed: these rows carry window_closed=True
# with no assertions, so that arm offered a rescan the server refuses with
# a 404 (check_binary) for bytes it never stored.
output.append(self._white(
'Detections: This artifact has not been scanned; no bytes were stored '
'to scan.'))It needs the branch back first: restore
elsewhere: F2, F3, F7 → polyswarm/artifact-index#1978 · F4, F5 → polyswarm/polyswarm-api#326 Outstanding review feedback
Standards conformityAudited in round 1 for all three members; that block stands. No new change-level violation in this PR — it is byte-identical to round 1. Set-level rows unchanged: §14 ✓, Rule 6 ◐ (this repo's branch is deleted from origin, so the set can no longer resolve companion code by name), |
2 participants
TL;DR
Status: Assertion window closed— a completed scan that found nothing — for something that was never scanned and holds no bytes to scan.NOT_STOREDits own branch, ahead ofwindow_closed, naming the cause and the remedy.statealready ships well below the pin.The bug
These records carry
window_closed=True, so the status ladder fell straight through the known-good branch to the ordinary closed-window line:…on a record with
polyscore: Noneand no assertions. It reads as "we scanned it and nothing came back", which is the opposite of what happened: the platform knows the hash, deliberately never stored its bytes, and never scanned it at all. Resubmitting the file scans it normally.Now:
Placed ahead of
window_closeddeliberately — a comment records why, since the ordering is the whole fix.The comment correction
The
known_good_sourcesnote claimed the field is "emitted for any instance whose sha256 matches a known-good record, including a fully scanned one". That described the server before it gated the field. It now gatesknown_goodon the same predicate it reportsKNOWN_GOODfrom, so the feeds cannot arrive without the state — the scenario the comment warned about can't occur.Worth keeping, and now stated: the implication runs one way only.
state == 'KNOWN_GOOD'does not guarantee non-empty sources (the server can render the state with no feeds if the catalogue entry stops resolving between gate and render), which is exactly why the attribution stays guarded onknown_good_sourcesrather than assuming it is populated. Left as-is, that comment would have justified "simplifying" the guard away.reported_stateis now read once and compared twice, rather than a secondgetattrfor the same attribute — the file's own note warns against adding those.Test plan
Two new rendering tests in
tests/known_good_field_test.py(Style 3 — formatter-direct, no CliRunner): the status line and its wording, and that aNOT_STOREDinstance is never reported as known-good even when the server sent a feed list.Full suite: 198 passed.
Companion PRs
Same branch name, independently mergeable — no ordering dependency:
polyswarm-api— exposes the server'serrors.codeonNotFoundException. This PR does not depend on it; it reads the instancestatethat already ships.