fix(uninstall): fix issues with uninstalling manifest-only drivers

.github/workflows/release.yml

@@ -16,7 +16,7 @@ jobs:
   snapshot:
     runs-on: ubuntu-latest
     environment: snapshot
-    if: github.event_name == 'pull_request'
+    if: github.event_name == 'pull_request' && !github.event.pull_request.head.repo.fork
     steps:
       - uses: actions/checkout@v5
         with:

cmd/dbc/install_test.go

@@ -120,7 +120,7 @@ func (suite *SubcommandTestSuite) TestInstallManifestOnlyDriver() {
 	m := InstallCmd{Driver: "test-driver-manifest-only"}.
 		GetModelCustom(baseModel{getDriverList: getTestDriverList, downloadPkg: downloadTestPkg})
 	suite.validateOutput("\r[✓] searching\r\n[✓] downloading\r\n[✓] installing\r\n[✓] verifying signature\r\n"+
-		"\r\nInstalled test-driver-manifest-only 0.1.0 to "+suite.tempdir+"\r\n"+
+		"\r\nInstalled test-driver-manifest-only 1.0.0 to "+suite.tempdir+"\r\n"+
 		"\r\nMust have libtest_driver installed to load this driver\r\n", suite.runCmd(m))
 	if runtime.GOOS != "windows" {
 		suite.FileExists(filepath.Join(suite.tempdir, "test-driver-manifest-only.toml"))

cmd/dbc/search_test.go

@@ -41,7 +41,7 @@ func (suite *SubcommandTestSuite) TestSearchCmdVerbose() {
 		"Available Versions:\r\n    ├── 2.0.0\r\n    ╰── 2.1.0\r\n"+
 		"• test-driver-manifest-only\r\n   Title: Test Driver Manifest Only\r\n   "+
 		"Description: This is manifest-only driver\r\n   License: Apache-2.0\r\n   "+
-		"Available Versions:\r\n    ╰── 0.1.0\r\n\r ", suite.runCmd(m))
+		"Available Versions:\r\n    ╰── 1.0.0\r\n\r ", suite.runCmd(m))
 }
 
 func (suite *SubcommandTestSuite) TestSearchCmdVerboseWithInstalled() {
@@ -64,5 +64,5 @@ func (suite *SubcommandTestSuite) TestSearchCmdVerboseWithInstalled() {
 		"   Description: This is manifest-only driver\r\n"+
 		"   License: Apache-2.0\r\n"+
 		"   Available Versions:\r\n"+
-		"    ╰── 0.1.0\r\n\r ", suite.runCmd(m))
+		"    ╰── 1.0.0\r\n\r ", suite.runCmd(m))
 }

cmd/dbc/testdata/test_manifest.yaml

@@ -54,7 +54,7 @@ drivers:
     license: Apache-2.0
     path: test-driver-manifest-only
     pkginfo:
-      - version: v0.1.0
+      - version: v1.0.0
         packages:
           - platform: linux_amd64
           - platform: macos_amd64

cmd/dbc/uninstall_test.go

@@ -3,6 +3,7 @@
 package main
 
 import (
+	"fmt"
 	"os"
 	"path"
 	"path/filepath"
@@ -124,3 +125,31 @@ func (suite *SubcommandTestSuite) TestUninstallMultipleLocationsNonDefault() {
 	suite.FileExists(filepath.Join(suite.tempdir, "test-driver-1.toml"))
 	suite.NoFileExists(filepath.Join(installModel.cfg.Location, "test-driver-1.toml"))
 }
+
+func (suite *SubcommandTestSuite) TestUninstallManifestOnlyDriver() {
+	m := InstallCmd{Driver: "test-driver-manifest-only"}.
+		GetModelCustom(baseModel{getDriverList: getTestDriverList, downloadPkg: downloadTestPkg})
+	suite.validateOutput("\r[✓] searching\r\n[✓] downloading\r\n[✓] installing\r\n[✓] verifying signature\r\n"+
+		"\r\nInstalled test-driver-manifest-only 1.0.0 to "+suite.tempdir+"\r\n"+
+		"\r\nMust have libtest_driver installed to load this driver\r\n", suite.runCmd(m))
+	if runtime.GOOS != "windows" {
+		suite.FileExists(filepath.Join(suite.tempdir, "test-driver-manifest-only.toml"))
+	}
+
+	// Verify the sidecar folder exists before we uninstall
+	new_sidecar_path := fmt.Sprintf("test-driver-manifest-only_%s_v1.0.0", config.PlatformTuple())
+	err := os.Rename(filepath.Join(suite.tempdir, "test-driver-manifest-only"), filepath.Join(suite.tempdir, new_sidecar_path))
+	if err != nil {
+		suite.Fail(fmt.Sprintf("Failed to rename sidecar folder. Something is wrong with this test: %v", err))
+	}
+	suite.DirExists(filepath.Join(suite.tempdir, new_sidecar_path))
+
+	// Now uninstall and verify we clean up
+	m = UninstallCmd{Driver: "test-driver-manifest-only"}.
+		GetModelCustom(baseModel{getDriverList: getTestDriverList, downloadPkg: downloadTestPkg})
+	suite.validateOutput("Driver `test-driver-manifest-only` uninstalled successfully!\r\n\r\n\r ", suite.runCmd(m))
+	if runtime.GOOS != "windows" {
+		suite.NoFileExists(filepath.Join(suite.tempdir, "test-driver-manifest-only.toml"))
+	}
+	suite.NoDirExists(filepath.Join(suite.tempdir, new_sidecar_path))
+}

config/config.go

@@ -11,7 +11,6 @@ import (
 	"io/fs"
 	"maps"
 	"os"
-	"path"
 	"path/filepath"
 	"runtime"
 	"slices"
@@ -201,7 +200,7 @@ func InstallDriver(cfg Config, shortName string, downloaded *os.File) (Manifest,
 	if loc, err = EnsureLocation(cfg); err != nil {
 		return Manifest{}, fmt.Errorf("could not ensure config location: %w", err)
 	}
-	base := strings.TrimSuffix(path.Base(downloaded.Name()), ".tar.gz")
+	base := strings.TrimSuffix(filepath.Base(downloaded.Name()), ".tar.gz")
 	finalDir := filepath.Join(loc, base)
 
 	if err := os.MkdirAll(finalDir, 0o755); err != nil {
@@ -315,3 +314,60 @@ func decodeManifest(r io.Reader, driverName string, requireShared bool) (Manifes
 
 	return result, nil
 }
+
+// Common, non-platform-specific code for uninstalling a driver. Called by
+// platform-specific UninstallDriver function.
+func UninstallDriverShared(cfg Config, info DriverInfo) error {
+	for sharedPath := range info.Driver.Shared.Paths() {
+		// Run filepath.Clean on sharedPath mainly to catch inner ".." in the path
+		sharedPath = filepath.Clean(sharedPath)
+
+		// Don't remove anything that isn't contained withing the found driver's
+		// config directory (i.e., avoid malicious driver manifests)
+		if !strings.HasPrefix(sharedPath, cfg.Location) {
+			continue
+		}
+
+		// dbc installs drivers in a folder, other tools may not so we handle each
+		// differently.
+		if info.Source == "dbc" {
+			if err := os.RemoveAll(filepath.Dir(sharedPath)); err != nil {
+				// Ignore only when not found. This supports manifest-only drivers.
+				// TODO: Come up with a better mechanism to handle manifest-only drivers
+				// and remove this continue when we do
+				if errors.Is(err, fs.ErrNotExist) {
+					continue
+				}
+				return fmt.Errorf("error removing driver %s: %w", info.ID, err)
+			}
+		} else {
+			if err := os.Remove(sharedPath); err != nil {
+				// Ignore only when not found. This supports manifest-only drivers.
+				// TODO: Come up with a better mechanism to handle manifest-only drivers
+				// and remove this continue when we do
+				if errors.Is(err, fs.ErrNotExist) {
+					continue
+				}
+				return fmt.Errorf("error removing driver %s: %w", info.ID, err)
+			}
+		}
+	}
+
+	// Manifest only drivers can come with extra files such as a LICENSE and we
+	// create a folder next to the driver manifest to store them, same as we'd
+	// store the actual driver shared library. Above, we find the path of this
+	// folder by looking at the Driver.shared path. For manifest-only drivers,
+	// Driver.shared is not a valid path (it's just a name), so this trick doesn't
+	// work. We do want to clean this folder up so here we guess what it is and
+	// try to remove it e.g., "somedriver_macos_arm64_v1.2.3."
+	extra_folder := fmt.Sprintf("%s_%s_v%s", info.ID, platformTuple, info.Version)
+	extra_folder = filepath.Clean(extra_folder)
+	extra_path := filepath.Join(cfg.Location, extra_folder)
+	finfo, err := os.Stat(extra_path)
+	if err == nil && finfo.IsDir() && extra_path != "." {
+		_ = os.RemoveAll(extra_path)
+		// ignore errors
+	}
+
+	return nil
+}

config/dirs_unixlike.go

@@ -5,9 +5,7 @@
 package config
 
 import (
-	"errors"
 	"fmt"
-	"io/fs"
 	"maps"
 	"os"
 	"path/filepath"
@@ -90,36 +88,14 @@ func CreateManifest(cfg Config, driver DriverInfo) (err error) {
 }
 
 func UninstallDriver(cfg Config, info DriverInfo) error {
-	if info.Source == "dbc" {
-		for sharedPath := range info.Driver.Shared.Paths() {
-			if err := os.RemoveAll(filepath.Dir(sharedPath)); err != nil {
-				// Ignore only when not found. This supports manifest-only drivers.
-				// TODO: Come up with a better mechanism to handle manifest-only drivers
-				// and remove this continue when we do
-				if errors.Is(err, fs.ErrNotExist) {
-					continue
-				}
-				return fmt.Errorf("error removing driver %s: %w", info.ID, err)
-			}
-		}
-	} else {
-		for sharedPath := range info.Driver.Shared.Paths() {
-			if err := os.Remove(sharedPath); err != nil {
-				// Ignore only when not found. This supports manifest-only drivers.
-				// TODO: Come up with a better mechanism to handle manifest-only drivers
-				// and remove this continue when we do
-				if errors.Is(err, fs.ErrNotExist) {
-					continue
-				}
-				return fmt.Errorf("error removing driver %s: %w", info.ID, err)
-			}
-		}
-	}
-
 	manifest := filepath.Join(cfg.Location, info.ID+".toml")
 	if err := os.Remove(manifest); err != nil {
 		return fmt.Errorf("error removing manifest %s: %w", manifest, err)
 	}
 
+	if err := UninstallDriverShared(cfg, info); err != nil {
+		return fmt.Errorf("failed to delete driver shared object: %w", err)
+	}
+
 	return nil
 }

config/dirs_windows.go

@@ -5,7 +5,6 @@ package config
 import (
 	"errors"
 	"fmt"
-	"io/fs"
 	"log"
 	"maps"
 	"os"
@@ -275,40 +274,20 @@ func CreateManifest(cfg Config, driver DriverInfo) (err error) {
 }
 
 func UninstallDriver(cfg Config, info DriverInfo) error {
-	k, err := registry.OpenKey(cfg.Level.key(), regKeyADBC, registry.WRITE)
-	if err != nil {
-		return err
-	}
-	defer k.Close()
+	if cfg.Level != ConfigEnv {
+		k, err := registry.OpenKey(cfg.Level.key(), regKeyADBC, registry.WRITE)
+		if err != nil {
+			return err
+		}
+		defer k.Close()
 
-	if err := registry.DeleteKey(k, info.ID); err != nil {
-		return fmt.Errorf("failed to delete driver registry key: %w", err)
+		if err := registry.DeleteKey(k, info.ID); err != nil {
+			return fmt.Errorf("failed to delete driver registry key: %w", err)
+		}
 	}
 
-	if info.Source == "dbc" {
-		for sharedPath := range info.Driver.Shared.Paths() {
-			if err := os.RemoveAll(filepath.Dir(sharedPath)); err != nil {
-				// Ignore only when not found. This supports manifest-only drivers.
-				// TODO: Come up with a better mechanism to handle manifest-only drivers
-				// and remove this continue when we do
-				if errors.Is(err, fs.ErrNotExist) {
-					continue
-				}
-				return fmt.Errorf("error removing driver %s: %w", info.ID, err)
-			}
-		}
-	} else {
-		for sharedPath := range info.Driver.Shared.Paths() {
-			if err := os.Remove(sharedPath); err != nil {
-				// Ignore only when not found. This supports manifest-only drivers.
-				// TODO: Come up with a better mechanism to handle manifest-only drivers
-				// and remove this continue when we do
-				if errors.Is(err, fs.ErrNotExist) {
-					continue
-				}
-				return fmt.Errorf("error removing driver %s: %w", info.ID, err)
-			}
-		}
+	if err := UninstallDriverShared(cfg, info); err != nil {
+		return fmt.Errorf("failed to delete driver shared object: %w", err)
 	}
 
 	return nil