From 36abc2dd7d2eee76cae26493082e3d17197577a3 Mon Sep 17 00:00:00 2001 From: Matt Topol Date: Thu, 29 Jan 2026 15:50:26 -0500 Subject: [PATCH 1/5] add test for search panic --- cmd/dbc/search_test.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/cmd/dbc/search_test.go b/cmd/dbc/search_test.go index a7e4b50c..ca489d6c 100644 --- a/cmd/dbc/search_test.go +++ b/cmd/dbc/search_test.go @@ -15,7 +15,9 @@ package main import ( + "os" "path/filepath" + "strings" "github.com/columnar-tech/dbc/config" ) @@ -114,3 +116,35 @@ func (suite *SubcommandTestSuite) TestSearchCmdVerboseWithInstalled() { " Available Versions:\n"+ " ╰── 1.0.0\n", suite.runCmd(m)) } + +func (suite *SubcommandTestSuite) TestSearchCmdWithMissingVersionInManifest() { + // Install a driver + m := InstallCmd{Driver: "test-driver-1", Level: config.ConfigEnv}. + GetModelCustom(baseModel{getDriverRegistry: getTestDriverRegistry, downloadPkg: downloadTestPkg}) + suite.runCmd(m) + + // Corrupt the manifest by removing the version key + manifestPath := filepath.Join(suite.tempdir, "test-driver-1.toml") + manifestData, err := os.ReadFile(manifestPath) + suite.Require().NoError(err, "should be able to read manifest file") + + // Remove the version line from the manifest + lines := []string{} + for _, line := range strings.Split(string(manifestData), "\n") { + if !strings.HasPrefix(line, "version =") { + lines = append(lines, line) + } + } + corruptedManifest := strings.Join(lines, "\n") + + err = os.WriteFile(manifestPath, []byte(corruptedManifest), 0644) + suite.Require().NoError(err, "should be able to write corrupted manifest") + + // This should panic when trying to access the nil Version pointer + suite.Require().NotPanics(func() { + m = SearchCmd{}.GetModelCustom( + baseModel{getDriverRegistry: getTestDriverRegistry, + downloadPkg: downloadTestPkg}) + suite.runCmd(m) + }, "Search should not panic when manifest is missing version key") +} From 58c009a36d2cb35fa9550742e1899ef83a42e0f6 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Thu, 29 Jan 2026 13:48:39 -0800 Subject: [PATCH 2/5] add validation logic to decodeManifest --- cmd/dbc/uninstall_test.go | 2 ++ config/config.go | 14 ++++++++++++-- config/driver_test.go | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/cmd/dbc/uninstall_test.go b/cmd/dbc/uninstall_test.go index e58b175b..3c87a5c5 100644 --- a/cmd/dbc/uninstall_test.go +++ b/cmd/dbc/uninstall_test.go @@ -40,6 +40,7 @@ func (suite *SubcommandTestSuite) TestUninstallManifestOnly() { } contents := `name = "Some Found Driver" +version = "1.0.0" # Doesn't matter what's in here @@ -60,6 +61,7 @@ func (suite *SubcommandTestSuite) TestUninstallDriverAndManifest() { pkgdir := path.Join(suite.tempdir, "somepath") os.Mkdir(pkgdir, 0o755) contents := `name = "Found Driver" +version = "1.0.0" # Doesn't matter what's in here diff --git a/config/config.go b/config/config.go index e9dded52..a28edfc5 100644 --- a/config/config.go +++ b/config/config.go @@ -35,6 +35,8 @@ const adbcEnvVar = "ADBC_DRIVER_PATH" var platformTuple string +var ErrInvalidManifest = errors.New("invalid manifest") + func init() { os := runtime.GOOS switch os { @@ -299,6 +301,14 @@ func decodeManifest(r io.Reader, driverName string, requireShared bool) (Manifes di.ManifestVersion, currentManifestVersion) } + // Callers can assume these fields are set so return an error if they aren't + if di.Name == "" { + return Manifest{}, fmt.Errorf("%w: %s", ErrInvalidManifest, "name is required") + } + if di.Version == nil { + return Manifest{}, fmt.Errorf("%w: %s", ErrInvalidManifest, "version is required") + } + result := Manifest{ DriverInfo: DriverInfo{ ID: driverName, @@ -323,12 +333,12 @@ func decodeManifest(r io.Reader, driverName string, requireShared bool) (Manifes if strVal, ok := v.(string); ok { result.Driver.Shared.platformMap[k] = strVal } else { - return Manifest{}, fmt.Errorf("invalid type for platform %s, expected string", k) + return Manifest{}, fmt.Errorf("%w: invalid type for platform %s, expected string", ErrInvalidManifest, k) } } default: if requireShared { - return Manifest{}, errors.New("invalid type for 'Driver.shared' in manifest, expected string or table") + return Manifest{}, fmt.Errorf("%w: %s", ErrInvalidManifest, "invalid type for 'Driver.shared' in manifest, expected string or table") } } diff --git a/config/driver_test.go b/config/driver_test.go index 0ea48c54..4de8df4e 100644 --- a/config/driver_test.go +++ b/config/driver_test.go @@ -125,3 +125,36 @@ version = '0.1.0' _, err := loadDriverFromManifest(prefix, driverName) require.ErrorContains(t, err, "manifest version 100 is unsupported, only 1 and lower are supported by this version of dbc") } + +func TestLoadDriverFromInvalidManifest(t *testing.T) { + tests := []struct { + name string + manifest string + errContains string + }{ + { + name: "missing name", + manifest: `version = '1.0.0'`, + errContains: "name is required", + }, + { + name: "missing version", + manifest: `name = 'Test Driver'`, + errContains: "version is required", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + prefix := t.TempDir() + driverName := "test_driver" + manifestPath := filepath.Join(prefix, driverName+".toml") + + require.NoError(t, os.WriteFile(manifestPath, []byte(tt.manifest), 0644)) + + _, err := loadDriverFromManifest(prefix, driverName) + require.ErrorIs(t, err, ErrInvalidManifest) + require.ErrorContains(t, err, tt.errContains) + }) + } +} From 118ec8e76921988d50977a78d0a5c63f70f0d5d1 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Thu, 29 Jan 2026 13:50:26 -0800 Subject: [PATCH 3/5] remove outdated comment --- cmd/dbc/search_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/dbc/search_test.go b/cmd/dbc/search_test.go index ca489d6c..eedbd4aa 100644 --- a/cmd/dbc/search_test.go +++ b/cmd/dbc/search_test.go @@ -140,7 +140,6 @@ func (suite *SubcommandTestSuite) TestSearchCmdWithMissingVersionInManifest() { err = os.WriteFile(manifestPath, []byte(corruptedManifest), 0644) suite.Require().NoError(err, "should be able to write corrupted manifest") - // This should panic when trying to access the nil Version pointer suite.Require().NotPanics(func() { m = SearchCmd{}.GetModelCustom( baseModel{getDriverRegistry: getTestDriverRegistry, From cd875cca0c2307e45edd7e772f4bfc4d13a0b9c8 Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Thu, 29 Jan 2026 14:04:50 -0800 Subject: [PATCH 4/5] inline part of error string --- config/config.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config/config.go b/config/config.go index a28edfc5..cc776241 100644 --- a/config/config.go +++ b/config/config.go @@ -303,10 +303,10 @@ func decodeManifest(r io.Reader, driverName string, requireShared bool) (Manifes // Callers can assume these fields are set so return an error if they aren't if di.Name == "" { - return Manifest{}, fmt.Errorf("%w: %s", ErrInvalidManifest, "name is required") + return Manifest{}, fmt.Errorf("%w: name is required", ErrInvalidManifest) } if di.Version == nil { - return Manifest{}, fmt.Errorf("%w: %s", ErrInvalidManifest, "version is required") + return Manifest{}, fmt.Errorf("%w: version is required", ErrInvalidManifest) } result := Manifest{ @@ -338,7 +338,7 @@ func decodeManifest(r io.Reader, driverName string, requireShared bool) (Manifes } default: if requireShared { - return Manifest{}, fmt.Errorf("%w: %s", ErrInvalidManifest, "invalid type for 'Driver.shared' in manifest, expected string or table") + return Manifest{}, fmt.Errorf("%w: invalid type for 'Driver.shared' in manifest, expected string or table", ErrInvalidManifest) } } From e6549fee5b5f3090bd16855877e5997eb967682b Mon Sep 17 00:00:00 2001 From: Bryce Mecum Date: Thu, 29 Jan 2026 14:05:12 -0800 Subject: [PATCH 5/5] update test to ensure we print path --- config/driver_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/config/driver_test.go b/config/driver_test.go index 4de8df4e..6c94ee9a 100644 --- a/config/driver_test.go +++ b/config/driver_test.go @@ -155,6 +155,7 @@ func TestLoadDriverFromInvalidManifest(t *testing.T) { _, err := loadDriverFromManifest(prefix, driverName) require.ErrorIs(t, err, ErrInvalidManifest) require.ErrorContains(t, err, tt.errContains) + require.ErrorContains(t, err, manifestPath) }) } }