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.