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:
Martin Vogel
2026-08-13 23:03:38 +02:00
parent 3904e59372
commit 094929fddc
2 changed files with 26 additions and 9 deletions
+19 -5
View File
@@ -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")
+7 -4
View File
@@ -290,10 +290,13 @@ grep -Fq 'upload-alias' "$RESULTS" || \
fail "results must retain the action's submitted analysis ID"
grep -q $'undetected\t\tclean\t' "$RESULTS" || \
fail "clean result must carry an explicit clean policy classification"
[ "$(run_gate "binaries/objects/probe=$(url_for mismatched-response-id)")" != "0" ] || \
fail "a response for a different analysis ID must block even when hash/size match"
grep -q 'response analysis id does not match' "$FIX/last.log" || \
fail "analysis-ID mismatch failure is not explicit"
# A differing analysis ID is NOT a failure: VirusTotal is content-addressed and
# may answer for already-known bytes with its own canonical analysis. The
# verdict is bound to the object by hash and size (wrong-hash / wrong-size
# below), which is what actually keeps a tuple's stripped and unstripped
# candidates apart — they differ in hash by construction.
[ "$(run_gate "binaries/objects/probe=$(url_for mismatched-response-id)")" = "0" ] || \
fail "a content-bound response must pass even when VirusTotal returns its own analysis ID: $(cat "$FIX/last.log")"
for id in one-malicious one-suspicious; do
[ "$(run_gate "binaries/objects/probe=$(url_for "$id")")" != "0" ] || \