fix(ci): bind the VirusTotal verdict to the bytes, not to the analysis id
The dry-run for this branch (run 31729540899) failed in `select-package`, the new job, after every build had completed and before smoke — the exact shape of failure that would have taken a release down: BLOCKED: VirusTotal response analysis id does not match the submitted analysis: objects/70fbbc37... That object is the linux-arm64 UNSTRIPPED candidate, 295,476,856 bytes. VirusTotal is content-addressed, and for bytes it already holds it may answer with its own canonical analysis rather than the one this upload created. A large release binary is exactly the artifact most likely to be already on file, so requiring the ids to be equal would have recurred on most releases rather than being a one-off. The equality check was guarding the right idea at the wrong level. What has to hold is that a verdict describes THESE bytes, and parse_completed already enforces that a few lines below against meta.file_info.sha256 and file_info.size — a strictly stronger binding than the id. It is also what keeps a tuple's two candidates apart: stripped and unstripped differ in hash by construction (70fbbc37... vs 28b3890f... on linux-arm64), so neither can be read as the other whatever ids VirusTotal hands out. The wrong-hash and wrong-size contract cases pin that and still block. The property deliberately given up is freshness: an older cached analysis of identical bytes is now accepted. For identical bytes that is the same content risk, and it buys a gate that is deterministic instead of one that fails whenever VirusTotal recognises our own binary. The submitted id is still recorded in the results manifest as evidence. The selection policy is unchanged and needs no change: stripped clean ships stripped; stripped microsoft-ml with a clean unstripped ships the clean one; both microsoft-ml ships stripped; a hard verdict on either sibling still blocks the release for analysis. Contract test updated to assert the passing direction, and revert-checked: with the fix reverted it fails with the production message, with it applied it passes. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
@@ -426,11 +426,25 @@ def parse_completed(document: object, submission: Submission) -> Tuple[str, Opti
|
||||
response_id = data.get("id")
|
||||
if not isinstance(response_id, str) or ANALYSIS_ID_RE.fullmatch(response_id) is None:
|
||||
raise GateError(f"VirusTotal response has an invalid analysis id: {submission.expected.scan_path}")
|
||||
if response_id != submission.analysis_id:
|
||||
raise GateError(
|
||||
f"VirusTotal response analysis id does not match the submitted analysis: "
|
||||
f"{submission.expected.scan_path}"
|
||||
)
|
||||
# The id is recorded as evidence, NOT required to equal the submitted one.
|
||||
#
|
||||
# VirusTotal is content-addressed: for bytes it already holds it may answer
|
||||
# with its own canonical analysis rather than the one this upload created,
|
||||
# and the id then differs legitimately. Requiring equality blocked a real
|
||||
# release dry-run on the 282 MB linux-arm64 candidate — precisely the kind
|
||||
# of large, previously-seen artifact where this happens, so it would have
|
||||
# recurred on most releases.
|
||||
#
|
||||
# What must hold is that the verdict describes THESE bytes, and the
|
||||
# completed branch below enforces exactly that against file_info.sha256 and
|
||||
# size. That is also what keeps a tuple's two candidates apart: stripped and
|
||||
# unstripped have different hashes by construction, so neither can be read
|
||||
# as the other regardless of what ids VirusTotal hands out (the wrong-hash
|
||||
# and wrong-size contract cases pin this).
|
||||
#
|
||||
# The property given up is freshness: an older cached analysis of identical
|
||||
# bytes is accepted. For identical bytes that is the same content risk, and
|
||||
# it is the trade this gate deliberately takes to stay deterministic.
|
||||
attributes = data.get("attributes")
|
||||
if not isinstance(attributes, dict):
|
||||
raise GateError("VirusTotal response has no analysis attributes")
|
||||
|
||||
Reference in New Issue
Block a user