[simonchrz]

Builds on #473 (merged) and git-pkgs/integrity v0.2.0, which includes git-pkgs/integrity#3 and adds the SHA-1 support this needs.

This is the follow-up andrew asked for in #473. It adds the SHA-1 checks, now that the integrity module can verify registry checksums.

Changes

• Checksums in the existing checks: newIntegrityChecks accepts native integrity given as a single algorithm-prefixed hex checksum (sha1-<hex>) when it isn't SRI. That checksum is verified with integrity's exact-digest check (Result.VerifyDigest). It goes through the same stored, streamed and cache-hit paths as SRI, so a recorded shasum is re-checked on cache hits too.
• npm: falls back to dist.shasum when dist.integrity is missing or can't be parsed, which is the case for versions published before integrity existed. A usable dist.integrity still takes precedence.
• Composer: checks an archive against its dist.shasum, which most private repositories (Satis, Private Packagist, GitLab) list. Dev branches (dev-*) aren't checked. Their listed shasum describes only the current head, but every commit of the branch is cached under the same version, and a version records a single integrity value. A cache hit for an older pinned commit would be checked against the head's shasum. Composer checks dist.shasum client-side anyway.
• Served unverified as before: when no shasum is listed (Packagist gives none for GitHub archives) or the value isn't a SHA-1 digest.

Tests

• npm, npm_integrity_test.go, stored and streamed:
  • a mismatched dist.shasum returns 502 (or aborts the stream) and nothing is cached;
  • a matching one is served;
  • dist.integrity wins over dist.shasum;
  • an unparseable integrity falls back to the shasum;
  • an unparseable shasum is ignored.
• Composer, composer_shasum_test.go:
  • a mismatched shasum returns 502 and nothing is cached;
  • matching, empty and unparseable shasums are served and cached;
  • a pinned older commit of a branch isn't checked against the head's shasum.
  • Regression: a pinned commit, then the branch head, then both again from the cache are each served with their own bytes, fetched once, and still cached. Before the dev-branch exemption, the pinned commit's archive was discarded.
• integrity_test.go: an sha1-<hex> native checksum is verified and a mismatch is reported.
• Mutation checks: I broke each rule in turn and a test failed every time. That covered the checksum never being verified, the pinned commit being checked, and no npm fallback.

go test -race ./... and golangci-lint v2.13.1 pass.

[andrew]

The npm side looks right: a usable dist.integrity still wins, v0.2.0 ParseSRI rejects the old sha1-<base64> values so those versions fall through to dist.shasum, and the ParseChecksum fallback in newIntegrityChecks lets readStored re-check a recorded sha1-<hex> on cache hits.

The Composer dev-branch case breaks cache hits. versions.integrity is one value per version PURL and GetCachedArtifact joins it onto every artifact of that version, but a dev version caches each commit's archive under its own filename. So: a lock file pinned to commit C0 fetches dev-main (unchecked, integrity NULL), another client fetches the listed head C1 (records C1's shasum on the shared row), and the next cache hit for C0 is verified against C1's shasum. It fails at EOF, the client gets an aborted download, and readStored discards the C0 archive. The refetch writes NULL again, which drops checking for C1's cache hits too. Every move of the branch head does the same to older pinned commits. TestComposerDownloadPinnedCommitIgnoresListedShasum only covers the first fetch, so it doesn't see this.

Please have composerIntegrity return "" for isDevVersion(version) so branch archives stay unverified. The listed shasum for a moving branch only describes one commit, and Composer checks dist.shasum client-side anyway. Add a test that fetches the pinned commit, then the head, then the pinned commit again from cache, and expects 200 with the original bytes.

[simonchrz]

Confirmed, and fixed in ba5e6e8 as you suggested. composerIntegrity now returns "" for isDevVersion(version), so branch archives stay unverified.

TestComposerDevBranchArchivesStayServableFromCache fetches the pinned commit, then the listed head, then both again. It expects each one to come back with its own bytes, to be fetched from upstream only once, and to still be cached at the end. Before the fix it failed with the pinned commit's archive gone from the cache. A mismatch on a cache hit is reported and the record discarded, while the bytes are still served, so here the effect was the discard and refetch rather than an aborted download.

The branch is also rebased onto main now that #473 is in, so it carries only the v0.2.0 bump and the shasum commits.

[andrew]

The dev-branch fix and the cache-hit regression test cover it, thanks.

One small leftover: with dev versions returning early, the listed parameter of composerIntegrity is dead, since only branch versions can resolve to an unlisted URL in upstreamDistURL. The parameter, the downloadURL == listedURL argument and the "A shasum also only describes the listed archive" sentence can go.