verify-action-build: normalize package.json before comparing

utils/tests/verify_action_build/test_diff_node_modules.py

@@ -183,3 +183,66 @@ def test_package_json_install_fields_ignored(self, tmp_path):
             orig, rebuilt, "test", "repo", "a" * 40,
         )
         assert result is True
+
+    def test_package_json_npm6_install_metadata_ignored(self, tmp_path):
+        # Real shape from reactivecircus/android-emulator-runner@a421e438:
+        # node_modules/tunnel/package.json was installed with npm v6-era
+        # tooling, which writes _args/_location (among others) that a modern
+        # npm ci rebuild does not.  An enumerated field list missed exactly
+        # those two and reported the package as modified — the only "content
+        # difference" in 217 files, which failed the whole JS build check.
+        import json
+        orig = tmp_path / "original"
+        rebuilt = tmp_path / "rebuilt"
+        orig.mkdir()
+        rebuilt.mkdir()
+        (orig / "tunnel").mkdir()
+        (rebuilt / "tunnel").mkdir()
+
+        published = {
+            "name": "tunnel",
+            "version": "0.0.6",
+            "main": "./index.js",
+            "license": "MIT",
+        }
+        installed = {
+            **published,
+            "_args": [["tunnel@0.0.6", "."]],
+            "_from": "tunnel@0.0.6",
+            "_id": "tunnel@0.0.6",
+            "_inBundle": False,
+            "_integrity": "sha512-1h/Lnq9yajKY2PEbBadPXj3VxsDDu844OnaAo52UVmIzIvwwtBPIuNvkjuzBlTWpfJyUbG3ez0KSBibQkj4ojg==",
+            "_location": "/tunnel",
+            "_phantomChildren": {},
+            "_requested": {"type": "version", "registry": True},
+            "_requiredBy": ["/@actions/http-client"],
+            "_resolved": "https://registry.npmjs.org/tunnel/-/tunnel-0.0.6.tgz",
+            "_spec": "0.0.6",
+            "_where": ".",
+        }
+
+        (orig / "tunnel" / "package.json").write_text(json.dumps(installed))
+        (rebuilt / "tunnel" / "package.json").write_text(json.dumps(published))
+
+        assert diff_node_modules(orig, rebuilt, "test", "repo", "a" * 40) is True
+
+    def test_package_json_real_field_change_still_flagged(self, tmp_path):
+        # Precision guard: stripping _-prefixed keys must not mask a change
+        # to a field that actually affects what runs.
+        import json
+        orig = tmp_path / "original"
+        rebuilt = tmp_path / "rebuilt"
+        orig.mkdir()
+        rebuilt.mkdir()
+        (orig / "tunnel").mkdir()
+        (rebuilt / "tunnel").mkdir()
+
+        (orig / "tunnel" / "package.json").write_text(
+            json.dumps({"name": "tunnel", "version": "0.0.6", "main": "./evil.js",
+                        "_resolved": "https://registry.npmjs.org/tunnel/-/tunnel-0.0.6.tgz"})
+        )
+        (rebuilt / "tunnel" / "package.json").write_text(
+            json.dumps({"name": "tunnel", "version": "0.0.6", "main": "./index.js"})
+        )
+
+        assert diff_node_modules(orig, rebuilt, "test", "repo", "a" * 40) is False

utils/tests/verify_action_build/test_npm_registry_verify.py

@@ -30,6 +30,8 @@
     _git_blob_sha1,
     _integrity_matches,
     _tarball_files,
+    normalize_package_json,
+    strip_npm_install_metadata,
     verify_vendored_node_modules,
 )
 
@@ -91,6 +93,18 @@ def _run(tree, lockfile_bytes, tarballs=None, truncated=False):
         return verify_vendored_node_modules("org", "repo", "deadbeef")
 
 
+def _run_with_files(tree, files, tarballs=None):
+    """Like :func:`_run`, but serves committed file bytes per path.
+
+    Needed once a check fetches a committed file (not just the lockfile).
+    """
+    tarballs = tarballs or {PKG_URL: PKG_TGZ}
+    with mock.patch.object(nrv, "_fetch_tree_with_sha", return_value=(tree, False)), \
+         mock.patch.object(nrv, "_fetch_lockfile", side_effect=lambda o, r, c, p: files.get(p)), \
+         mock.patch.object(nrv, "_download_tarball", side_effect=lambda url: tarballs.get(url)):
+        return verify_vendored_node_modules("org", "repo", "deadbeef")
+
+
 class TestHelpers:
     def test_git_blob_sha1_known_value(self):
         # git hash-object of an empty blob is well-known.
@@ -110,6 +124,63 @@ def test_tarball_files_strips_package_prefix(self):
         out = _tarball_files(PKG_TGZ)
         assert out == PKG_FILES
 
+    def test_strip_npm_install_metadata_drops_underscore_keys(self):
+        # npm's install bookkeeping is _-prefixed by convention; the exact
+        # set has varied across npm versions, so match the prefix.
+        assert strip_npm_install_metadata({
+            "name": "tunnel", "version": "0.0.6",
+            "_args": [["tunnel@0.0.6", "."]], "_location": "/tunnel",
+            "_resolved": "https://registry.npmjs.org/tunnel/-/tunnel-0.0.6.tgz",
+        }) == {"name": "tunnel", "version": "0.0.6"}
+        # Nothing else is touched.
+        assert strip_npm_install_metadata({"name": "x"}) == {"name": "x"}
+
+    def test_normalize_package_json_shorthand_fields(self):
+        # Exact shapes from tunnel@0.0.6 as vendored by
+        # reactivecircus/android-emulator-runner@a421e438 vs the published
+        # tarball.  npm's normalize-package-data expands author/bugs and
+        # prefixes repository.url with "git+" at install time.
+        published = {
+            "name": "tunnel",
+            "version": "0.0.6",
+            "author": "Koichi Kobayashi <koichik@improvement.jp>",
+            "bugs": "https://github.com/koichik/node-tunnel/issues",
+            "repository": {
+                "type": "git",
+                "url": "https://github.com/koichik/node-tunnel.git",
+            },
+        }
+        installed = {
+            "name": "tunnel",
+            "version": "0.0.6",
+            "author": {"name": "Koichi Kobayashi", "email": "koichik@improvement.jp"},
+            "bugs": {"url": "https://github.com/koichik/node-tunnel/issues"},
+            "repository": {
+                "type": "git",
+                "url": "git+https://github.com/koichik/node-tunnel.git",
+            },
+            "_location": "/tunnel",
+        }
+        assert normalize_package_json(installed) == normalize_package_json(published)
+
+    def test_normalize_package_json_leaves_runtime_fields_strict(self):
+        # Fields that decide what actually runs are compared as-is.
+        base = {"name": "foo", "version": "1.0.0"}
+        assert normalize_package_json({**base, "main": "./index.js"}) != \
+            normalize_package_json({**base, "main": "./evil.js"})
+        assert normalize_package_json({**base, "scripts": {"postinstall": "x"}}) != \
+            normalize_package_json(base)
+        assert normalize_package_json({**base, "dependencies": {"a": "1"}}) != \
+            normalize_package_json({**base, "dependencies": {"a": "2"}})
+
+    def test_normalize_person_handles_name_only_and_url(self):
+        assert normalize_package_json({"author": "Jane Doe"})["author"] == {"name": "Jane Doe"}
+        assert normalize_package_json(
+            {"author": "Jane Doe <j@example.com> (https://example.com)"}
+        )["author"] == {
+            "name": "Jane Doe", "email": "j@example.com", "url": "https://example.com",
+        }
+
 
 class TestVerify:
     def test_no_vendored_lockfile_returns_none(self):
@@ -123,6 +194,46 @@ def test_clean_match_passes(self):
         assert result.verified == ["foo"]
         assert not result.mismatched and not result.extra and not result.errors
 
+    def test_package_json_install_metadata_is_not_a_mismatch(self):
+        # reactivecircus/android-emulator-runner@a421e438 vendors a
+        # node_modules installed with npm v6-era tooling, so every
+        # package.json carries _args/_location/... that the registry tarball
+        # never had.  Byte comparison alone reported the package modified.
+        installed = json.dumps({
+            "name": "foo", "version": "1.0.0",
+            "_args": [["foo@1.0.0", "."]],
+            "_location": "/foo",
+            "_resolved": PKG_URL,
+            "_integrity": _integrity(PKG_TGZ),
+        }).encode()
+        tree = _tree_for(PKG_FILES)
+        tree["node_modules/foo/package.json"] = _git_blob_sha1(installed)
+
+        result = _run_with_files(tree, {
+            "node_modules/.package-lock.json": _lock(),
+            "node_modules/foo/package.json": installed,
+        })
+        assert result.ok is True
+        assert result.verified == ["foo"]
+        assert not result.mismatched
+
+    def test_package_json_real_change_still_mismatches(self):
+        # Precision guard: normalising _-prefixed keys must not hide an edit
+        # to a field that changes what actually runs.
+        tampered = json.dumps({
+            "name": "foo", "version": "1.0.0", "main": "./evil.js",
+            "_resolved": PKG_URL,
+        }).encode()
+        tree = _tree_for(PKG_FILES)
+        tree["node_modules/foo/package.json"] = _git_blob_sha1(tampered)
+
+        result = _run_with_files(tree, {
+            "node_modules/.package-lock.json": _lock(),
+            "node_modules/foo/package.json": tampered,
+        })
+        assert result.ok is False
+        assert "node_modules/foo/package.json" in result.mismatched
+
     def test_content_mismatch_fails(self):
         tree = _tree_for(PKG_FILES)
         tree["node_modules/foo/index.js"] = _git_blob_sha1(b"EVIL();\n")  # tampered

utils/tests/verify_action_build/test_security.py

@@ -1491,6 +1491,26 @@ def test_gradle_wrapper_jar_exempt(self):
         assert _looks_like_in_tree_binary("dist/gradle-wrapper.jar") is True
         assert _looks_like_in_tree_binary("Library.jar") is True
 
+    def test_actions_tool_cache_7zdec_exempt(self):
+        # 7zdec.exe ships inside the first-party @actions/tool-cache npm
+        # package (it backs tc.extractZip on Windows), so every action that
+        # vendors node_modules carries it verbatim from the published
+        # tarball.  reactivecircus/android-emulator-runner was false-flagged
+        # for it.
+        assert _looks_like_in_tree_binary(
+            "node_modules/@actions/tool-cache/scripts/externals/7zdec.exe"
+        ) is False
+        # Exempt under a nested action sub-path too.
+        assert _looks_like_in_tree_binary(
+            "subdir/node_modules/@actions/tool-cache/scripts/externals/7zdec.exe"
+        ) is False
+        # Precision: the same name dropped anywhere else is still caught —
+        # the suffix match needs the canonical package path.
+        assert _looks_like_in_tree_binary("dist/7zdec.exe") is True
+        assert _looks_like_in_tree_binary(
+            "node_modules/evil-pkg/scripts/externals/7zdec.exe"
+        ) is True
+
     def test_matlab_platform_dir_naming(self):
         # MATLAB's launcher convention: dist/bin/<platform>/run-matlab-command
         # where <platform> is MATLAB's own arch identifier and the file has

utils/verify_action_build/diff_node_modules.py

@@ -24,6 +24,7 @@
 
 from .console import console, link
 from .diff_display import show_colored_diff
+from .npm_registry_verify import normalize_package_json
 
 
 def diff_node_modules(
@@ -129,15 +130,9 @@ def collect_files(base: Path) -> dict[Path, str]:
         if rel_path.name == "package.json":
             orig_text = (original_dir / rel_path).read_text(errors="replace")
             rebuilt_text = (rebuilt_dir / rel_path).read_text(errors="replace")
-            install_fields = {"_resolved", "_integrity", "_from", "_where", "_id",
-                              "_requested", "_requiredBy", "_shasum", "_spec",
-                              "_phantomChildren", "_inBundle"}
             try:
-                orig_json = json.loads(orig_text)
-                rebuilt_json = json.loads(rebuilt_text)
-                for field in install_fields:
-                    orig_json.pop(field, None)
-                    rebuilt_json.pop(field, None)
+                orig_json = normalize_package_json(json.loads(orig_text))
+                rebuilt_json = normalize_package_json(json.loads(rebuilt_text))
                 if orig_json == rebuilt_json:
                     continue
             except (json.JSONDecodeError, ValueError):