From caa9b7c4d8923aa519895a985d4c2946ef7e9720 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Wed, 16 Sep 2026 14:15:02 +0200 Subject: [PATCH] verify-action-build: credit vendored files git normalised from CRLF A package whose npm tarball ships CRLF can never byte-match the committed copy: git normalises line endings on commit, so the vendored blob is the LF form while the tarball is not. The registry check reported every such file as modified. whatwg-url@5.0.0, vendored under node-fetch by benchmark-action/github- action-benchmark (#1283), is the live case -- lib/url-state-machine.js is 33573 bytes with 1297 CRs as published and 32276 bytes with none as committed. Folding the tarball's CRLF to LF hashes to 27d977a25f9011fc31ed28c17ba11f729b85edea, exactly the committed blob. Hashing the folded bytes is a proof, not a heuristic: a match means the committed file is the published content, line endings aside. A real edit still fails, which the precision-guard test pins. Reported separately from verified so the output says why the file did not byte-match rather than hiding it. Generated-by: Claude Opus 5 --- .../test_npm_registry_verify.py | 38 +++++++++++++++++++ .../npm_registry_verify.py | 15 ++++++++ 2 files changed, 53 insertions(+) diff --git a/utils/tests/verify_action_build/test_npm_registry_verify.py b/utils/tests/verify_action_build/test_npm_registry_verify.py index dab9b77d3..baba43453 100644 --- a/utils/tests/verify_action_build/test_npm_registry_verify.py +++ b/utils/tests/verify_action_build/test_npm_registry_verify.py @@ -312,6 +312,44 @@ def test_extra_file_in_verified_package_fails(self): assert result.ok is False assert "node_modules/foo/sneaky.js" in result.extra + def test_crlf_tarball_normalised_by_git_is_not_a_mismatch(self): + # apache/infrastructure-actions#1283: github-action-benchmark vendors + # node-fetch's nested whatwg-url@5.0.0, whose 2016-era tarball ships + # lib/url-state-machine.js with CRLF. git normalises to LF on commit, + # so the committed blob can never byte-match the tarball: the published + # file is 33573 bytes with 1297 CRs, the committed one 32276 with none. + # Byte comparison alone reported the package modified on every bump. + crlf = { + "index.js": b"module.exports = 1;\r\n", + "package.json": b'{"name":"foo","version":"1.0.0"}\r\n', + } + tgz = _make_tgz(crlf) + # The repo holds the LF-normalised form that git checked in. + tree = _tree_for({k: v.replace(b"\r\n", b"\n") for k, v in crlf.items()}) + + result = _run(tree, _lock(integrity=_integrity(tgz)), tarballs={PKG_URL: tgz}) + assert result is not None + assert result.ok is True + assert result.verified == ["foo"] + assert not result.mismatched + assert "node_modules/foo/index.js" in result.crlf_normalized + + def test_crlf_normalisation_does_not_hide_a_real_edit(self): + # Precision guard: matching after CRLF folding must not excuse a file + # whose actual content differs. + crlf = { + "index.js": b"module.exports = 1;\r\n", + "package.json": b'{"name":"foo","version":"1.0.0"}\r\n', + } + tgz = _make_tgz(crlf) + tree = _tree_for({k: v.replace(b"\r\n", b"\n") for k, v in crlf.items()}) + tree["node_modules/foo/index.js"] = _git_blob_sha1(b"EVIL();\n") + + result = _run(tree, _lock(integrity=_integrity(tgz)), tarballs={PKG_URL: tgz}) + assert result.ok is False + assert "node_modules/foo/index.js" in result.mismatched + assert "foo" not in result.verified + def test_definitelytyped_package_verifies_clean(self): # apache/infrastructure-actions#1171: github-pages-deploy-action v4.9.0 # migrated yarn → npm, which added node_modules/.package-lock.json and diff --git a/utils/verify_action_build/npm_registry_verify.py b/utils/verify_action_build/npm_registry_verify.py index 6200d3cfb..1cc5f0ae8 100644 --- a/utils/verify_action_build/npm_registry_verify.py +++ b/utils/verify_action_build/npm_registry_verify.py @@ -78,6 +78,7 @@ def __init__(self) -> None: self.skipped: list[str] = [] self.foreign: list[str] = [] self.mismatched: list[str] = [] + self.crlf_normalized: list[str] = [] self.extra: list[str] = [] self.errors: list[str] = [] self.truncated: bool = False @@ -421,6 +422,13 @@ def verify_vendored_node_modules( if committed_sha is None: continue # tarball ships a file the repo omits — benign if committed_sha != _git_blob_sha1(content): + if committed_sha == _git_blob_sha1(content.replace(b"\r\n", b"\n")): + # The tarball ships CRLF and git normalised it to LF on + # commit, so the committed blob can never byte-match. + # Hashing the folded bytes proves the committed file *is* + # exactly the published content, line endings aside. + result.crlf_normalized.append(committed_path) + continue if rel.split("/")[-1] == "package.json" and _package_json_equivalent( org, repo, commit_hash, prefix + committed_path, content, ): @@ -463,6 +471,13 @@ def _render(result: NpmRegistryResult, org: str, repo: str, commit_hash: str) -> f"{', '.join(result.skipped[:8])}" + (" …" if len(result.skipped) > 8 else "") ) + if result.crlf_normalized: + console.print( + f" [green]✓[/green] {len(result.crlf_normalized)} file(s) match " + f"after folding the tarball's CRLF to LF (git normalises line " + f"endings on commit): {', '.join(result.crlf_normalized[:4])}" + + (" …" if len(result.crlf_normalized) > 4 else "") + ) for path in result.mismatched: # link() takes (url, text) — naming the offending file matters more # than linking the tree root, which told a reviewer nothing.