verify-action-build: detect the tarball root instead of assuming package/

utils/tests/verify_action_build/test_npm_registry_verify.py

@@ -36,12 +36,17 @@
 )
 
 
-def _make_tgz(files: dict[str, bytes]) -> bytes:
-    """Build an npm-style ``.tgz`` (everything under ``package/``)."""
+def _make_tgz(files: dict[str, bytes], root: str = "package") -> bytes:
+    """Build an npm-style ``.tgz``.
+
+    ``root`` is ``package`` by convention; DefinitelyTyped publishes under
+    the bare package name instead, so it is parameterised.  Pass ``""`` to
+    emit member names verbatim.
+    """
     buf = io.BytesIO()
     with tarfile.open(fileobj=buf, mode="w:gz") as tf:
         for rel, content in files.items():
-            info = tarfile.TarInfo(name=f"package/{rel}")
+            info = tarfile.TarInfo(name=f"{root}/{rel}" if root else rel)
             info.size = len(content)
             tf.addfile(info, io.BytesIO(content))
     return buf.getvalue()
@@ -60,6 +65,41 @@ def _integrity(data: bytes) -> str:
 PKG_TGZ = _make_tgz(PKG_FILES)
 PKG_URL = "https://registry.npmjs.org/foo/-/foo-1.0.0.tgz"
 
+# DefinitelyTyped roots its tarballs at the bare package name rather than
+# ``package/`` — this is the real @types/estree@1.0.9 member layout.
+TYPES_FILES = {
+    "LICENSE": b"MIT License\n",
+    "README.md": b"# Installation\n",
+    "flow.d.ts": b"// flow types\n",
+    "index.d.ts": b"export interface Node {}\n",
+    "package.json": b'{"name":"@types/estree","version":"1.0.9"}\n',
+}
+TYPES_TGZ = _make_tgz(TYPES_FILES, root="estree")
+TYPES_URL = "https://registry.npmjs.org/@types/estree/-/estree-1.0.9.tgz"
+
+
+def _types_tree(extra: dict[str, str] | None = None) -> dict[str, str]:
+    tree = {"node_modules/.package-lock.json": "abc123"}
+    for rel, content in TYPES_FILES.items():
+        tree[f"node_modules/@types/estree/{rel}"] = _git_blob_sha1(content)
+    if extra:
+        tree.update(extra)
+    return tree
+
+
+def _types_lock() -> bytes:
+    return json.dumps({
+        "lockfileVersion": 3,
+        "packages": {
+            "": {"name": "root"},
+            "node_modules/@types/estree": {
+                "version": "1.0.9",
+                "resolved": TYPES_URL,
+                "integrity": _integrity(TYPES_TGZ),
+            },
+        },
+    }).encode()
+
 
 def _tree_for(files: dict[str, bytes], extra: dict[str, str] | None = None) -> dict[str, str]:
     tree = {"node_modules/.package-lock.json": "abc123"}
@@ -124,6 +164,24 @@ def test_tarball_files_strips_package_prefix(self):
         out = _tarball_files(PKG_TGZ)
         assert out == PKG_FILES
 
+    def test_tarball_files_strips_definitelytyped_bare_name_root(self):
+        # @types/* tarballs root at the bare package name; assuming
+        # "package/" left every path prefixed with the root, so none of
+        # them matched node_modules/@types/<pkg>/<rel>.
+        assert _tarball_files(TYPES_TGZ) == TYPES_FILES
+
+    def test_tarball_files_strips_dot_slash_prefixed_root(self):
+        assert _tarball_files(_make_tgz(PKG_FILES, root="./package")) == PKG_FILES
+
+    def test_tarball_files_leaves_multi_root_tarball_untouched(self):
+        # No single shared root → nothing is safe to strip.
+        files = {"a/one.js": b"1\n", "b/two.js": b"2\n"}
+        assert _tarball_files(_make_tgz(files, root="")) == files
+
+    def test_tarball_files_leaves_root_level_files_untouched(self):
+        files = {"one.js": b"1\n"}
+        assert _tarball_files(_make_tgz(files, root="")) == 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.
@@ -254,6 +312,25 @@ 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_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
+        # so switched this check on for the first time.  Every file of every
+        # vendored @types package was then reported as injected code, even
+        # though each tarball had already passed integrity verification.
+        result = _run(_types_tree(), _types_lock(), tarballs={TYPES_URL: TYPES_TGZ})
+        assert result.ok is True
+        assert result.verified == ["@types/estree"]
+        assert not result.extra and not result.mismatched and not result.errors
+
+    def test_injected_file_in_definitelytyped_package_still_flagged(self):
+        # Precision guard: detecting the root must not stop real extra files
+        # inside a bare-name-rooted package from being caught.
+        tree = _types_tree(extra={"node_modules/@types/estree/evil.js": "deadbeef00"})
+        result = _run(tree, _types_lock(), tarballs={TYPES_URL: TYPES_TGZ})
+        assert result.ok is False
+        assert "node_modules/@types/estree/evil.js" in result.extra
+
     def test_noisy_bin_files_not_flagged_as_extra(self):
         tree = _tree_for(PKG_FILES, extra={"node_modules/.bin/foo": "shimsha00"})
         result = _run(tree, _lock())

utils/verify_action_build/npm_registry_verify.py

@@ -261,20 +261,33 @@ def _download_tarball(url: str) -> bytes | None:
 def _tarball_files(data: bytes) -> dict[str, bytes]:
     """Extract a ``.tgz`` into ``{relative_path: bytes}``.
 
-    npm tarballs root everything under ``package/``; that prefix is
-    stripped so paths line up with ``node_modules/<pkg>/<rel>``.
+    npm tarballs root everything under a single top-level directory; that
+    prefix is stripped so paths line up with ``node_modules/<pkg>/<rel>``.
+    The directory is ``package/`` by convention, but DefinitelyTyped
+    publishes ``@types/*`` under the bare package name instead
+    (``estree/index.d.ts``, ``json-schema/LICENSE``), so the root is
+    detected rather than assumed — hardcoding ``package/`` left every file
+    of every vendored ``@types`` package unaccounted for, and the caller
+    reports unaccounted files as injected code.
+
+    A tarball whose files do not all share one root is left untouched.
     """
     files: dict[str, bytes] = {}
     with tarfile.open(fileobj=io.BytesIO(data), mode="r:gz") as tf:
-        for member in tf.getmembers():
-            if not member.isfile():
-                continue
-            name = member.name
-            rel = name[len("package/"):] if name.startswith("package/") else name
+        # Some publishers emit "./package/..." member names.
+        members = [
+            (m.name[2:] if m.name.startswith("./") else m.name, m)
+            for m in tf.getmembers()
+            if m.isfile()
+        ]
+        roots = {name.split("/", 1)[0] for name, _ in members if "/" in name}
+        nested = all("/" in name for name, _ in members)
+        strip = f"{next(iter(roots))}/" if nested and len(roots) == 1 else ""
+        for name, member in members:
             extracted = tf.extractfile(member)
             if extracted is None:
                 continue
-            files[rel] = extracted.read()
+            files[name[len(strip):] if strip else name] = extracted.read()
     return files