Add library validation step when installing from zip or git (#1234)

* Add library validation when installing from zip or git

* [skip changelog] Fix tests temp directory deletion on Windows

* [skip changelog] Fix test_install_git_invalid_library test

* [skip changelog] Add more check in lib install git repo tests

Author: silvanocerza

arduino/libraries/librariesmanager/install.go

@@ -133,6 +133,11 @@ func (lm *LibrariesManager) InstallZipLib(ctx context.Context, archivePath strin
 
 	extractionPath := paths[0]
 	libraryName := extractionPath.Base()
+
+	if err := validateLibrary(libraryName, extractionPath); err != nil {
+		return err
+	}
+
 	installPath := libsDir.Join(libraryName)
 
 	if err := libsDir.MkdirAll(); err != nil {
@@ -218,6 +223,13 @@ func (lm *LibrariesManager) InstallGitLib(gitURL string, overwrite bool) error {
 			Warn("Cloning git repository")
 		return err
 	}
+
+	if err := validateLibrary(libraryName, installPath); err != nil {
+		// Clean up installation directory since this is not a valid library
+		installPath.RemoveAll()
+		return err
+	}
+
 	// We don't want the installed library to be a git repository thus we delete this folder
 	installPath.Join(".git").RemoveAll()
 	return nil
@@ -239,3 +251,23 @@ func parseGitURL(gitURL string) (string, error) {
 	}
 	return res, nil
 }
+
+// validateLibrary verifies the dir contains a valid library, meaning it has both
+// an header <name>.h, either in src or root folder, and a library.properties file
+func validateLibrary(name string, dir *paths.Path) error {
+	// Verify library contains library header.
+	// Checks also root folder for legacy reasons.
+	// For more info see:
+	// https://arduino.github.io/arduino-cli/latest/library-specification/#source-code
+	libraryHeader := name + ".h"
+	if !dir.Join("src", libraryHeader).Exist() && !dir.Join(libraryHeader).Exist() {
+		return fmt.Errorf(`library is not valid: missing header file "%s"`, libraryHeader)
+	}
+
+	// Verifies library contains library.properties
+	if !dir.Join("library.properties").Exist() {
+		return fmt.Errorf(`library is not valid: missing file "library.properties"`)
+	}
+
+	return nil
+}

test/conftest.py

@@ -56,7 +56,9 @@ def data_dir(tmpdir_factory):
     if platform.system() == "Windows":
         with tempfile.TemporaryDirectory() as tmp:
             yield tmp
-            shutil.rmtree(tmp, ignore_errors=True)
+            # We don't need to remove the directory since
+            # tempfile.TemporaryDirectory deletes itself
+            # automatically when exits its scope.
     else:
         data = tmpdir_factory.mktemp("ArduinoTest")
         yield str(data)

test/test_lib.py

@@ -18,6 +18,34 @@
 import pytest
 from git import Repo
 from pathlib import Path
+import tempfile
+import requests
+import zipfile
+import io
+import re
+
+
+# Util function to download library from URL
+def download_lib(url, download_dir):
+    tmp = Path(tempfile.TemporaryDirectory().name)
+    tmp.mkdir(parents=True, exist_ok=True)
+    regex = re.compile(r"^(.*)-[0-9]+.[0-9]+.[0-9]")
+    response = requests.get(url)
+    # Download and unzips library removing version suffix
+    with zipfile.ZipFile(io.BytesIO(response.content)) as thezip:
+        for zipinfo in thezip.infolist():
+            with thezip.open(zipinfo) as f:
+                dest_dir = tmp / regex.sub("\\g<1>", zipinfo.filename)
+                if zipinfo.is_dir():
+                    dest_dir.mkdir(parents=True, exist_ok=True)
+                else:
+                    dest_dir.write_bytes(f.read())
+
+    # Recreates zip with folder without version suffix
+    z = zipfile.ZipFile(download_dir, "w")
+    for f in tmp.glob("**/*"):
+        z.write(f, arcname=f.relative_to(tmp))
+    z.close()
 
 
 def test_list(run_command):
@@ -252,8 +280,12 @@ def test_install_git_url_and_zip_path_flags_visibility(run_command, data_dir, do
     assert res.failed
     assert "--git-url and --zip-path are disabled by default, for more information see:" in res.stderr
 
-    assert run_command("lib download AudioZero@1.0.0")
-    zip_path = Path(downloads_dir, "libraries", "AudioZero-1.0.0.zip")
+    # Download library
+    url = "https://github.com/arduino-libraries/AudioZero/archive/refs/tags/1.1.1.zip"
+    zip_path = Path(downloads_dir, "libraries", "AudioZero.zip")
+    zip_path.parent.mkdir(parents=True, exist_ok=True)
+    download_lib(url, zip_path)
+
     res = run_command(f"lib install --zip-path {zip_path}")
     assert res.failed
     assert "--git-url and --zip-path are disabled by default, for more information see:" in res.stderr
@@ -330,13 +362,16 @@ def test_install_with_zip_path(run_command, data_dir, downloads_dir):
     assert run_command("config init --dest-dir .", custom_env=env)
 
     # Download a specific lib version
-    assert run_command("lib download AudioZero@1.0.0")
+    # Download library
+    url = "https://github.com/arduino-libraries/AudioZero/archive/refs/tags/1.1.1.zip"
+    zip_path = Path(downloads_dir, "libraries", "AudioZero.zip")
+    zip_path.parent.mkdir(parents=True, exist_ok=True)
+    download_lib(url, zip_path)
 
-    lib_install_dir = Path(data_dir, "libraries", "AudioZero-1.0.0")
+    lib_install_dir = Path(data_dir, "libraries", "AudioZero")
     # Verifies library is not already installed
     assert not lib_install_dir.exists()
 
-    zip_path = Path(downloads_dir, "libraries", "AudioZero-1.0.0.zip")
     # Test zip-path install
     res = run_command(f"lib install --zip-path {zip_path}")
     assert res.ok
@@ -688,13 +723,14 @@ def test_install_with_zip_path_multiple_libraries(run_command, downloads_dir, da
     }
 
     # Downloads zip to be installed later
-    assert run_command("lib download WiFi101@0.16.1")
-    assert run_command("lib download ArduinoBLE@1.1.3")
     wifi_zip_path = Path(downloads_dir, "libraries", "WiFi101-0.16.1.zip")
     ble_zip_path = Path(downloads_dir, "libraries", "ArduinoBLE-1.1.3.zip")
+    download_lib("https://github.com/arduino-libraries/WiFi101/archive/refs/tags/0.16.1.zip", wifi_zip_path)
+    download_lib("https://github.com/arduino-libraries/ArduinoBLE/archive/refs/tags/1.1.3.zip", ble_zip_path)
+
+    wifi_install_dir = Path(data_dir, "libraries", "WiFi101")
+    ble_install_dir = Path(data_dir, "libraries", "ArduinoBLE")
 
-    wifi_install_dir = Path(data_dir, "libraries", "WiFi101-0.16.1")
-    ble_install_dir = Path(data_dir, "libraries", "ArduinoBLE-1.1.3")
     # Verifies libraries are not installed
     assert not wifi_install_dir.exists()
     assert not ble_install_dir.exists()
@@ -860,6 +896,7 @@ def test_install_zip_lib_with_macos_metadata(run_command, data_dir, downloads_di
     assert lib_install_dir.exists()
     files = list(lib_install_dir.glob("**/*"))
     assert lib_install_dir / "library.properties" in files
+    assert lib_install_dir / "src" / "fake-lib.h" in files
 
     # Reinstall library
     assert run_command(f"lib install --zip-path {zip_path}")
@@ -868,3 +905,81 @@ def test_install_zip_lib_with_macos_metadata(run_command, data_dir, downloads_di
     assert lib_install_dir.exists()
     files = list(lib_install_dir.glob("**/*"))
     assert lib_install_dir / "library.properties" in files
+    assert lib_install_dir / "src" / "fake-lib.h" in files
+
+
+def test_install_zip_invalid_library(run_command, data_dir, downloads_dir):
+    # Initialize configs to enable --zip-path flag
+    env = {
+        "ARDUINO_DATA_DIR": data_dir,
+        "ARDUINO_DOWNLOADS_DIR": downloads_dir,
+        "ARDUINO_SKETCHBOOK_DIR": data_dir,
+        "ARDUINO_ENABLE_UNSAFE_LIBRARY_INSTALL": "true",
+    }
+    assert run_command("config init --dest-dir .", custom_env=env)
+
+    lib_install_dir = Path(data_dir, "libraries", "lib-without-header")
+    # Verifies library is not already installed
+    assert not lib_install_dir.exists()
+
+    zip_path = Path(__file__).parent / "testdata" / "lib-without-header.zip"
+    # Test zip-path install
+    res = run_command(f"lib install --zip-path {zip_path}")
+    assert res.failed
+    assert 'library is not valid: missing header file "lib-without-header.h"' in res.stderr
+
+    lib_install_dir = Path(data_dir, "libraries", "lib-without-properties")
+    # Verifies library is not already installed
+    assert not lib_install_dir.exists()
+
+    zip_path = Path(__file__).parent / "testdata" / "lib-without-properties.zip"
+    # Test zip-path install
+    res = run_command(f"lib install --zip-path {zip_path}")
+    assert res.failed
+    assert 'library is not valid: missing file "library.properties"' in res.stderr
+
+
+def test_install_git_invalid_library(run_command, data_dir, downloads_dir):
+    # Initialize configs to enable --zip-path flag
+    env = {
+        "ARDUINO_DATA_DIR": data_dir,
+        "ARDUINO_DOWNLOADS_DIR": downloads_dir,
+        "ARDUINO_SKETCHBOOK_DIR": data_dir,
+        "ARDUINO_ENABLE_UNSAFE_LIBRARY_INSTALL": "true",
+    }
+    assert run_command("config init --dest-dir .", custom_env=env)
+
+    # Create fake library repository
+    repo_dir = Path(data_dir, "lib-without-header")
+    with Repo.init(repo_dir) as repo:
+        lib_properties = Path(repo_dir, "library.properties")
+        lib_properties.touch()
+        repo.index.add([str(lib_properties)])
+        repo.index.commit("First commit")
+
+    lib_install_dir = Path(data_dir, "libraries", "lib-without-header")
+    # Verifies library is not already installed
+    assert not lib_install_dir.exists()
+
+    res = run_command(f"lib install --git-url {repo_dir}", custom_env=env)
+    assert res.failed
+    assert 'library is not valid: missing header file "lib-without-header.h"' in res.stderr
+    assert not lib_install_dir.exists()
+
+    # Create another fake library repository
+    repo_dir = Path(data_dir, "lib-without-properties")
+    with Repo.init(repo_dir) as repo:
+        lib_header = Path(repo_dir, "src", "lib-without-properties.h")
+        lib_header.parent.mkdir(parents=True, exist_ok=True)
+        lib_header.touch()
+        repo.index.add([str(lib_header)])
+        repo.index.commit("First commit")
+
+    lib_install_dir = Path(data_dir, "libraries", "lib-without-properties")
+    # Verifies library is not already installed
+    assert not lib_install_dir.exists()
+
+    res = run_command(f"lib install --git-url {repo_dir}", custom_env=env)
+    assert res.failed
+    assert 'library is not valid: missing file "library.properties"' in res.stderr
+    assert not lib_install_dir.exists()