From 5cf8b3388b34ccf5e35cd63b4e75787d16e609e2 Mon Sep 17 00:00:00 2001 From: eitsupi Date: Wed, 30 Sep 2026 03:26:01 +0000 Subject: [PATCH 1/3] test: isolate Windows registry and driver directories --- cmd/dbc/registry_test.go | 5 +- cmd/dbc/subcommand_windows_test.go | 2 +- cmd/dbc/test_isolation_windows_test.go | 246 ++++++++++++++++ config/dirs_windows.go | 3 +- config/test_isolation_windows_test.go | 58 ++++ internal/systempath/program_files_windows.go | 50 ++++ internal/wintest/isolation_windows.go | 281 +++++++++++++++++++ 7 files changed, 641 insertions(+), 4 deletions(-) create mode 100644 cmd/dbc/test_isolation_windows_test.go create mode 100644 config/test_isolation_windows_test.go create mode 100644 internal/systempath/program_files_windows.go create mode 100644 internal/wintest/isolation_windows.go diff --git a/cmd/dbc/registry_test.go b/cmd/dbc/registry_test.go index d4233dee..c2b5e001 100644 --- a/cmd/dbc/registry_test.go +++ b/cmd/dbc/registry_test.go @@ -32,8 +32,9 @@ import ( ) // This test suite is only run when the "test_registry" build tag is set. -// Only run these tests if you're able to modify the windows registry and won't be broken -// if the ADBC registry keys are cleared/modified/etc. +// The Windows test main redirects both registry hives and driver roots to +// process-private temporary fixtures. These tests do not exercise real +// Program Files permissions or the machine registry's ACLs. type RegistryTestSuite struct { suite.Suite diff --git a/cmd/dbc/subcommand_windows_test.go b/cmd/dbc/subcommand_windows_test.go index 8a7909b8..62900363 100644 --- a/cmd/dbc/subcommand_windows_test.go +++ b/cmd/dbc/subcommand_windows_test.go @@ -26,7 +26,7 @@ import ( ) func (suite *SubcommandTestSuite) TearDownTest() { - // Clean up the registry and filesystem after each test + // Clean up the process-private registry and filesystem fixtures after each test. _, user := os.LookupEnv("DBC_TEST_LEVEL_USER") _, system := os.LookupEnv("DBC_TEST_LEVEL_SYSTEM") diff --git a/cmd/dbc/test_isolation_windows_test.go b/cmd/dbc/test_isolation_windows_test.go new file mode 100644 index 00000000..fb34a725 --- /dev/null +++ b/cmd/dbc/test_isolation_windows_test.go @@ -0,0 +1,246 @@ +// Copyright 2026 Columnar Technologies Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +package main + +import ( + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/columnar-tech/dbc/config" + "github.com/columnar-tech/dbc/internal/wintest" + "golang.org/x/sys/windows/registry" +) + +const ( + windowsTestIsolationChildEnv = "DBC_WINDOWS_TEST_ISOLATION_CHILD" + windowsTestIsolationUserEnv = "DBC_WINDOWS_TEST_ISOLATION_PARENT_USER" + windowsTestIsolationSysEnv = "DBC_WINDOWS_TEST_ISOLATION_PARENT_SYSTEM" + windowsTestIsolationDriverID = "dbc-process-isolation-sentinel" + windowsTestIsolationRelative = ".dbc-package-g-process-isolation/driver.dll" +) + +func TestMain(m *testing.M) { + isolation, err := wintest.Setup() + if err != nil { + _, _ = fmt.Fprintf(os.Stderr, "set up isolated Windows CLI tests: %v\n", err) + os.Exit(1) + } + if err := verifyWindowsCLIConfigRoots(isolation); err != nil { + cleanupErr := isolation.Cleanup() + _, _ = fmt.Fprintf(os.Stderr, "verify isolated Windows CLI roots: %v\n", errors.Join(err, cleanupErr)) + os.Exit(1) + } + + code := m.Run() + if err := isolation.Cleanup(); err != nil { + _, _ = fmt.Fprintf(os.Stderr, "clean up isolated Windows CLI tests: %v\n", err) + code = 1 + } + os.Exit(code) +} + +func verifyWindowsCLIConfigRoots(isolation *wintest.Isolation) error { + if got, want := filepath.Clean(config.ConfigUser.ConfigLocation()), filepath.Clean(isolation.UserRoot()); !strings.EqualFold(got, want) { + return fmt.Errorf("ConfigUser root = %q, want private root %q", got, want) + } + if got, want := filepath.Clean(config.ConfigSystem.ConfigLocation()), filepath.Clean(isolation.SystemRoot()); !strings.EqualFold(got, want) { + return fmt.Errorf("ConfigSystem root = %q, want private root %q", got, want) + } + return nil +} + +func TestWindowsTestIsolationAcrossProcesses(t *testing.T) { + if os.Getenv(windowsTestIsolationChildEnv) == "1" { + runWindowsTestIsolationChild(t) + return + } + + parentUserRoot := config.ConfigUser.ConfigLocation() + parentSystemRoot := config.ConfigSystem.ConfigLocation() + parentUserPath := windowsIsolationPayload(parentUserRoot) + parentSystemPath := windowsIsolationPayload(parentSystemRoot) + parentUserBytes := []byte("parent user registry and payload sentinel") + parentSystemBytes := []byte("parent system registry and payload sentinel") + writeIsolationPayload(t, parentUserPath, parentUserBytes) + writeIsolationPayload(t, parentSystemPath, parentSystemBytes) + writeIsolationRegistration(t, registry.CURRENT_USER, parentUserPath) + writeIsolationRegistration(t, registry.LOCAL_MACHINE, parentSystemPath) + t.Setenv("ADBC_DRIVER_PATH", parentUserRoot) + t.Setenv("VIRTUAL_ENV", parentSystemRoot) + t.Setenv("CONDA_PREFIX", parentSystemRoot) + t.Cleanup(func() { + var cleanupErr error + cleanupErr = errors.Join(cleanupErr, deleteRegistryKeyRecursive(registry.CURRENT_USER, "SOFTWARE\\ADBC\\Drivers\\"+windowsTestIsolationDriverID)) + cleanupErr = errors.Join(cleanupErr, deleteRegistryKeyRecursive(registry.LOCAL_MACHINE, "SOFTWARE\\ADBC\\Drivers\\"+windowsTestIsolationDriverID)) + cleanupErr = errors.Join(cleanupErr, os.RemoveAll(filepath.Dir(parentUserPath))) + cleanupErr = errors.Join(cleanupErr, os.RemoveAll(filepath.Dir(parentSystemPath))) + if cleanupErr != nil { + t.Errorf("remove parent isolation sentinels: %v", cleanupErr) + } + }) + + child := exec.Command(os.Args[0], "-test.run=^TestWindowsTestIsolationAcrossProcesses$") + child.Env = append(os.Environ(), + windowsTestIsolationChildEnv+"=1", + windowsTestIsolationUserEnv+"="+parentUserRoot, + windowsTestIsolationSysEnv+"="+parentSystemRoot, + ) + output, err := child.CombinedOutput() + if err != nil { + t.Fatalf("isolated child process failed: %v\n%s", err, output) + } + + assertIsolationPayload(t, parentUserPath, parentUserBytes) + assertIsolationPayload(t, parentSystemPath, parentSystemBytes) + assertIsolationRegistration(t, registry.CURRENT_USER, parentUserPath) + assertIsolationRegistration(t, registry.LOCAL_MACHINE, parentSystemPath) +} + +func runWindowsTestIsolationChild(t *testing.T) { + for _, name := range []string{"ADBC_DRIVER_PATH", "VIRTUAL_ENV", "CONDA_PREFIX"} { + if value, present := os.LookupEnv(name); present { + t.Fatalf("child inherited unsafe %s value %q", name, value) + } + } + + parentUserRoot := os.Getenv(windowsTestIsolationUserEnv) + parentSystemRoot := os.Getenv(windowsTestIsolationSysEnv) + childUserRoot := config.ConfigUser.ConfigLocation() + childSystemRoot := config.ConfigSystem.ConfigLocation() + if sameWindowsPath(childUserRoot, parentUserRoot) || sameWindowsPath(childSystemRoot, parentSystemRoot) { + t.Fatalf("child fixture roots are not private: User=%q parent=%q; System=%q parent=%q", childUserRoot, parentUserRoot, childSystemRoot, parentSystemRoot) + } + + childUserPath := windowsIsolationPayload(childUserRoot) + childSystemPath := windowsIsolationPayload(childSystemRoot) + writeIsolationPayload(t, childUserPath, []byte("child user payload")) + writeIsolationPayload(t, childSystemPath, []byte("child system payload")) + writeIsolationRegistration(t, registry.CURRENT_USER, childUserPath) + writeIsolationRegistration(t, registry.LOCAL_MACHINE, childSystemPath) + + if err := deleteRegistryKeyRecursive(registry.CURRENT_USER, "SOFTWARE\\ADBC\\Drivers"); err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(childUserRoot); err != nil { + t.Fatalf("remove child User driver root: %v", err) + } + assertIsolationRegistrationRemoved(t, registry.CURRENT_USER) + assertIsolationPathRemoved(t, childUserRoot) + assertIsolationPayload(t, childSystemPath, []byte("child system payload")) + assertIsolationRegistration(t, registry.LOCAL_MACHINE, childSystemPath) + + if err := deleteRegistryKeyRecursive(registry.LOCAL_MACHINE, "SOFTWARE\\ADBC\\Drivers"); err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(childSystemRoot); err != nil { + t.Fatalf("remove child System driver root: %v", err) + } + assertIsolationRegistrationRemoved(t, registry.LOCAL_MACHINE) + assertIsolationPathRemoved(t, childSystemRoot) +} + +func windowsIsolationPayload(root string) string { + return filepath.Join(root, filepath.FromSlash(windowsTestIsolationRelative)) +} + +func writeIsolationPayload(t *testing.T, path string, contents []byte) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, contents, 0o600); err != nil { + t.Fatal(err) + } +} + +func writeIsolationRegistration(t *testing.T, root registry.Key, driver string) { + t.Helper() + key, _, err := registry.CreateKey(root, "SOFTWARE\\ADBC\\Drivers\\"+windowsTestIsolationDriverID, registry.ALL_ACCESS) + if err != nil { + t.Fatal(err) + } + defer func() { + if err := key.Close(); err != nil { + t.Errorf("close isolation registration: %v", err) + } + }() + for name, value := range map[string]string{ + "driver": driver, + "name": "Process Isolation Sentinel", + "version": "1.0.0", + } { + if err := key.SetStringValue(name, value); err != nil { + t.Fatal(err) + } + } +} + +func assertIsolationRegistration(t *testing.T, root registry.Key, want string) { + t.Helper() + key, err := registry.OpenKey(root, "SOFTWARE\\ADBC\\Drivers\\"+windowsTestIsolationDriverID, registry.READ) + if err != nil { + t.Fatal(err) + } + got, _, readErr := key.GetStringValue("driver") + closeErr := key.Close() + if err := errors.Join(readErr, closeErr); err != nil { + t.Fatal(err) + } + if got != want { + t.Fatalf("isolation registration driver = %q, want %q", got, want) + } +} + +func assertIsolationRegistrationRemoved(t *testing.T, root registry.Key) { + t.Helper() + key, err := registry.OpenKey(root, "SOFTWARE\\ADBC\\Drivers\\"+windowsTestIsolationDriverID, registry.READ) + if errors.Is(err, registry.ErrNotExist) { + return + } + if err != nil { + t.Fatalf("check removed isolation registration: %v", err) + } + closeErr := key.Close() + t.Fatalf("isolation registration remains after cleanup (close error: %v)", closeErr) +} + +func assertIsolationPayload(t *testing.T, path string, want []byte) { + t.Helper() + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read isolation payload %q: %v", path, err) + } + if string(got) != string(want) { + t.Fatalf("isolation payload %q = %q, want %q", path, got, want) + } +} + +func assertIsolationPathRemoved(t *testing.T, path string) { + t.Helper() + if _, err := os.Lstat(path); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("isolation path %q remains after cleanup or could not be checked: %v", path, err) + } +} + +func sameWindowsPath(left, right string) bool { + return strings.EqualFold(filepath.Clean(left), filepath.Clean(right)) +} diff --git a/config/dirs_windows.go b/config/dirs_windows.go index 5805fee5..b3a22bce 100644 --- a/config/dirs_windows.go +++ b/config/dirs_windows.go @@ -24,6 +24,7 @@ import ( "slices" "github.com/Masterminds/semver/v3" + "github.com/columnar-tech/dbc/internal/systempath" "golang.org/x/sys/windows/registry" ) @@ -62,7 +63,7 @@ func (c ConfigLevel) ConfigLocation() string { var prefix string switch c { case ConfigSystem: - prefix = "C:\\Program Files" + prefix = systempath.ProgramFilesRoot() case ConfigUser: prefix, _ = os.UserConfigDir() case ConfigEnv: diff --git a/config/test_isolation_windows_test.go b/config/test_isolation_windows_test.go new file mode 100644 index 00000000..be1308cd --- /dev/null +++ b/config/test_isolation_windows_test.go @@ -0,0 +1,58 @@ +// Copyright 2026 Columnar Technologies Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +package config + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/columnar-tech/dbc/internal/wintest" +) + +func TestMain(m *testing.M) { + isolation, err := wintest.Setup() + if err != nil { + _, _ = fmt.Fprintf(os.Stderr, "set up isolated Windows config tests: %v\n", err) + os.Exit(1) + } + if err := verifyWindowsConfigRoots(isolation); err != nil { + cleanupErr := isolation.Cleanup() + _, _ = fmt.Fprintf(os.Stderr, "verify isolated Windows config roots: %v\n", errors.Join(err, cleanupErr)) + os.Exit(1) + } + + code := m.Run() + if err := isolation.Cleanup(); err != nil { + _, _ = fmt.Fprintf(os.Stderr, "clean up isolated Windows config tests: %v\n", err) + code = 1 + } + os.Exit(code) +} + +func verifyWindowsConfigRoots(isolation *wintest.Isolation) error { + if got, want := filepath.Clean(ConfigUser.ConfigLocation()), filepath.Clean(isolation.UserRoot()); !strings.EqualFold(got, want) { + return fmt.Errorf("ConfigUser root = %q, want private root %q", got, want) + } + if got, want := filepath.Clean(ConfigSystem.ConfigLocation()), filepath.Clean(isolation.SystemRoot()); !strings.EqualFold(got, want) { + return fmt.Errorf("ConfigSystem root = %q, want private root %q", got, want) + } + return nil +} diff --git a/internal/systempath/program_files_windows.go b/internal/systempath/program_files_windows.go new file mode 100644 index 00000000..8e095c6d --- /dev/null +++ b/internal/systempath/program_files_windows.go @@ -0,0 +1,50 @@ +// Copyright 2026 Columnar Technologies Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +// Package systempath provides the Windows system installation root. +package systempath + +import "sync" + +const defaultProgramFilesRoot = `C:\Program Files` + +var ( + programFilesRootMu sync.RWMutex + programFilesRoot = defaultProgramFilesRoot +) + +// ProgramFilesRoot returns the root used for system-level driver files. +func ProgramFilesRoot() string { + programFilesRootMu.RLock() + defer programFilesRootMu.RUnlock() + return programFilesRoot +} + +// SetProgramFilesRootForTests temporarily overrides the system driver root. +// Windows test binaries call this once before running tests and restore it +// after their test suite completes. +func SetProgramFilesRootForTests(root string) func() { + programFilesRootMu.Lock() + previous := programFilesRoot + programFilesRoot = root + programFilesRootMu.Unlock() + + return func() { + programFilesRootMu.Lock() + programFilesRoot = previous + programFilesRootMu.Unlock() + } +} diff --git a/internal/wintest/isolation_windows.go b/internal/wintest/isolation_windows.go new file mode 100644 index 00000000..f2986baa --- /dev/null +++ b/internal/wintest/isolation_windows.go @@ -0,0 +1,281 @@ +// Copyright 2026 Columnar Technologies Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +// Package wintest contains Windows-only test fixture support. The fixture +// redirects registry and filesystem state to process-private temporary roots; +// it does not exercise real Program Files permissions or HKLM ACLs. +package wintest + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "unsafe" + + "github.com/columnar-tech/dbc/internal/systempath" + "golang.org/x/sys/windows" + "golang.org/x/sys/windows/registry" +) + +const regProcessAppKey = 0x00000001 + +var clearForTestsEnvironment = []string{"ADBC_DRIVER_PATH", "VIRTUAL_ENV", "CONDA_PREFIX"} + +var ( + advapi32DLL = windows.NewLazySystemDLL("advapi32.dll") + regLoadAppKeyWProc = advapi32DLL.NewProc("RegLoadAppKeyW") + regOverridePredefKeyProc = advapi32DLL.NewProc("RegOverridePredefKey") +) + +// Isolation redirects HKCU and HKLM to separate subkeys in a private registry +// hive and places User/System driver files under separate temporary roots. +type Isolation struct { + tempDir string + appData string + systemRoot string + priorAppData string + hadAppData bool + priorEnv []environmentValue + appHive registry.Key + userHive registry.Key + systemHive registry.Key + userMapped bool + systemMapped bool + restoreRoot func() +} + +type environmentValue struct { + name string + value string + present bool +} + +// Setup creates the process-private Windows test fixture. Any setup failure +// rolls back all state before returning, so callers can fail closed. +func Setup() (*Isolation, error) { + isolation := &Isolation{} + isolation.priorAppData, isolation.hadAppData = os.LookupEnv("APPDATA") + for _, name := range clearForTestsEnvironment { + value, present := os.LookupEnv(name) + isolation.priorEnv = append(isolation.priorEnv, environmentValue{name: name, value: value, present: present}) + } + + dir, err := os.MkdirTemp("", "dbc-windows-tests-") + if err != nil { + return nil, fmt.Errorf("create private Windows test directory: %w", err) + } + isolation.tempDir = dir + if err := isolation.setup(); err != nil { + return nil, errors.Join(err, isolation.Cleanup()) + } + return isolation, nil +} + +func (isolation *Isolation) setup() error { + for _, name := range clearForTestsEnvironment { + if err := os.Unsetenv(name); err != nil { + return fmt.Errorf("clear %s for isolated Windows tests: %w", name, err) + } + } + + isolation.appData = filepath.Join(isolation.tempDir, "user") + isolation.systemRoot = filepath.Join(isolation.tempDir, "system") + for _, path := range []string{isolation.appData, isolation.systemRoot} { + if err := os.MkdirAll(path, 0o700); err != nil { + return fmt.Errorf("create private Windows test root %q: %w", path, err) + } + } + if err := os.Setenv("APPDATA", isolation.appData); err != nil { + return fmt.Errorf("set private APPDATA: %w", err) + } + isolation.restoreRoot = systempath.SetProgramFilesRootForTests(isolation.systemRoot) + + userConfigRoot, err := os.UserConfigDir() + if err != nil { + return fmt.Errorf("resolve private User config root: %w", err) + } + wantUserRoot := filepath.Join(isolation.appData, "ADBC", "Drivers") + wantSystemRoot := filepath.Join(isolation.systemRoot, "ADBC", "Drivers") + if got := filepath.Join(userConfigRoot, "ADBC", "Drivers"); !samePath(got, wantUserRoot) { + return fmt.Errorf("private User driver root = %q, want %q", got, wantUserRoot) + } + if got := filepath.Join(systempath.ProgramFilesRoot(), "ADBC", "Drivers"); !samePath(got, wantSystemRoot) { + return fmt.Errorf("private System driver root = %q, want %q", got, wantSystemRoot) + } + + hivePath := filepath.Join(isolation.tempDir, "registry.hiv") + if _, err := os.Lstat(hivePath); err == nil || !errors.Is(err, os.ErrNotExist) { + if err == nil { + err = errors.New("private registry hive path already exists") + } + return fmt.Errorf("verify private registry hive path: %w", err) + } + isolation.appHive, err = loadPrivateRegistryHive(hivePath) + if err != nil { + return err + } + isolation.userHive, _, err = registry.CreateKey(isolation.appHive, "User", registry.ALL_ACCESS) + if err != nil { + return fmt.Errorf("create private HKCU hive root: %w", err) + } + isolation.systemHive, _, err = registry.CreateKey(isolation.appHive, "System", registry.ALL_ACCESS) + if err != nil { + return fmt.Errorf("create private HKLM hive root: %w", err) + } + if err := overridePredefinedKey(registry.CURRENT_USER, isolation.userHive, "HKCU"); err != nil { + return err + } + isolation.userMapped = true + if err := overridePredefinedKey(registry.LOCAL_MACHINE, isolation.systemHive, "HKLM"); err != nil { + return err + } + isolation.systemMapped = true + if err := verifyRegistryOverride(registry.CURRENT_USER, isolation.userHive, "user"); err != nil { + return err + } + if err := verifyRegistryOverride(registry.LOCAL_MACHINE, isolation.systemHive, "system"); err != nil { + return err + } + return nil +} + +// UserRoot returns the private ConfigUser-compatible driver directory. +func (isolation *Isolation) UserRoot() string { + return filepath.Join(isolation.appData, "ADBC", "Drivers") +} + +// SystemRoot returns the private ConfigSystem-compatible driver directory. +func (isolation *Isolation) SystemRoot() string { + return filepath.Join(isolation.systemRoot, "ADBC", "Drivers") +} + +// Cleanup restores predefined registry mappings and APPDATA, closes all hive +// handles, restores the production system root, and removes the fixture. +func (isolation *Isolation) Cleanup() error { + var cleanupErr error + if isolation.systemMapped { + cleanupErr = errors.Join(cleanupErr, resetPredefinedKey(registry.LOCAL_MACHINE, "HKLM")) + isolation.systemMapped = false + } + if isolation.userMapped { + cleanupErr = errors.Join(cleanupErr, resetPredefinedKey(registry.CURRENT_USER, "HKCU")) + isolation.userMapped = false + } + if isolation.systemHive != 0 { + cleanupErr = errors.Join(cleanupErr, isolation.systemHive.Close()) + isolation.systemHive = 0 + } + if isolation.userHive != 0 { + cleanupErr = errors.Join(cleanupErr, isolation.userHive.Close()) + isolation.userHive = 0 + } + if isolation.appHive != 0 { + cleanupErr = errors.Join(cleanupErr, isolation.appHive.Close()) + isolation.appHive = 0 + } + if isolation.restoreRoot != nil { + isolation.restoreRoot() + isolation.restoreRoot = nil + } + if isolation.hadAppData { + cleanupErr = errors.Join(cleanupErr, os.Setenv("APPDATA", isolation.priorAppData)) + } else { + cleanupErr = errors.Join(cleanupErr, os.Unsetenv("APPDATA")) + } + if isolation.tempDir != "" { + cleanupErr = errors.Join(cleanupErr, os.RemoveAll(isolation.tempDir)) + isolation.tempDir = "" + } + for _, prior := range isolation.priorEnv { + if prior.present { + cleanupErr = errors.Join(cleanupErr, os.Setenv(prior.name, prior.value)) + } else { + cleanupErr = errors.Join(cleanupErr, os.Unsetenv(prior.name)) + } + } + return cleanupErr +} + +func loadPrivateRegistryHive(path string) (registry.Key, error) { + filename, err := windows.UTF16PtrFromString(path) + if err != nil { + return 0, fmt.Errorf("encode private registry hive path: %w", err) + } + var hive uintptr + status, _, _ := regLoadAppKeyWProc.Call( + uintptr(unsafe.Pointer(filename)), + uintptr(unsafe.Pointer(&hive)), + uintptr(registry.ALL_ACCESS), + regProcessAppKey, + 0, + ) + if status != 0 { + return 0, fmt.Errorf("RegLoadAppKeyW: %w", windows.Errno(status)) + } + return registry.Key(hive), nil +} + +func overridePredefinedKey(predefined, replacement registry.Key, name string) error { + status, _, _ := regOverridePredefKeyProc.Call(uintptr(predefined), uintptr(replacement)) + if status != 0 { + return fmt.Errorf("RegOverridePredefKey(%s): %w", name, windows.Errno(status)) + } + return nil +} + +func resetPredefinedKey(predefined registry.Key, name string) error { + status, _, _ := regOverridePredefKeyProc.Call(uintptr(predefined), 0) + if status != 0 { + return fmt.Errorf("RegOverridePredefKey(%s, nil): %w", name, windows.Errno(status)) + } + return nil +} + +func verifyRegistryOverride(predefined, privateRoot registry.Key, name string) error { + probePath := "Software\\dbc-test-isolation-probe" + want := "private-" + name + privateKey, _, err := registry.CreateKey(privateRoot, probePath, registry.ALL_ACCESS) + if err != nil { + return fmt.Errorf("create private %s probe: %w", name, err) + } + writeErr := privateKey.SetStringValue("probe", want) + closeErr := privateKey.Close() + if err := errors.Join(writeErr, closeErr); err != nil { + return fmt.Errorf("write private %s probe: %w", name, err) + } + viewKey, err := registry.OpenKey(predefined, probePath, registry.READ) + if err != nil { + return fmt.Errorf("read private %s probe through predefined key: %w", name, err) + } + got, _, readErr := viewKey.GetStringValue("probe") + closeErr = viewKey.Close() + if err := errors.Join(readErr, closeErr); err != nil { + return fmt.Errorf("read private %s probe through predefined key: %w", name, err) + } + if got != want { + return fmt.Errorf("%s isolation probe = %q, want %q", name, got, want) + } + if err := registry.DeleteKey(privateRoot, probePath); err != nil { + return fmt.Errorf("remove private %s probe: %w", name, err) + } + return nil +} + +func samePath(left, right string) bool { + return strings.EqualFold(filepath.Clean(left), filepath.Clean(right)) +} From 80a6eef79de5adcc01dc1ffae67bc6f9809fe043 Mon Sep 17 00:00:00 2001 From: eitsupi Date: Wed, 30 Sep 2026 03:37:13 +0000 Subject: [PATCH 2/3] fix: sign-extend Windows test registry handles --- internal/wintest/isolation_windows.go | 11 ++++-- internal/wintest/isolation_windows_test.go | 40 ++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 internal/wintest/isolation_windows_test.go diff --git a/internal/wintest/isolation_windows.go b/internal/wintest/isolation_windows.go index f2986baa..ee5c58f8 100644 --- a/internal/wintest/isolation_windows.go +++ b/internal/wintest/isolation_windows.go @@ -231,7 +231,7 @@ func loadPrivateRegistryHive(path string) (registry.Key, error) { } func overridePredefinedKey(predefined, replacement registry.Key, name string) error { - status, _, _ := regOverridePredefKeyProc.Call(uintptr(predefined), uintptr(replacement)) + status, _, _ := regOverridePredefKeyProc.Call(predefinedRegistryKeyArgument(predefined), uintptr(replacement)) if status != 0 { return fmt.Errorf("RegOverridePredefKey(%s): %w", name, windows.Errno(status)) } @@ -239,13 +239,20 @@ func overridePredefinedKey(predefined, replacement registry.Key, name string) er } func resetPredefinedKey(predefined registry.Key, name string) error { - status, _, _ := regOverridePredefKeyProc.Call(uintptr(predefined), 0) + status, _, _ := regOverridePredefKeyProc.Call(predefinedRegistryKeyArgument(predefined), 0) if status != 0 { return fmt.Errorf("RegOverridePredefKey(%s, nil): %w", name, windows.Errno(status)) } return nil } +func predefinedRegistryKeyArgument(predefined registry.Key) uintptr { + // The Windows SDK defines predefined HKEYs by sign-extending a LONG before + // converting it to ULONG_PTR; Go's registry constants are zero-extended. + // See https://github.com/microsoft/win32metadata/blob/main/generation/WinSDK/RecompiledIdlHeaders/um/winreg.h. + return uintptr(int64(int32(predefined))) +} + func verifyRegistryOverride(predefined, privateRoot registry.Key, name string) error { probePath := "Software\\dbc-test-isolation-probe" want := "private-" + name diff --git a/internal/wintest/isolation_windows_test.go b/internal/wintest/isolation_windows_test.go new file mode 100644 index 00000000..4d6a2cbb --- /dev/null +++ b/internal/wintest/isolation_windows_test.go @@ -0,0 +1,40 @@ +// Copyright 2026 Columnar Technologies Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +package wintest + +import ( + "testing" + "unsafe" + + "golang.org/x/sys/windows/registry" +) + +func TestPredefinedRegistryKeyArgumentUsesWindowsABIValue(t *testing.T) { + wantUser := uint64(0x80000001) + wantMachine := uint64(0x80000002) + if unsafe.Sizeof(uintptr(0)) == 8 { + wantUser = 0xffffffff80000001 + wantMachine = 0xffffffff80000002 + } + + if got := uint64(predefinedRegistryKeyArgument(registry.CURRENT_USER)); got != wantUser { + t.Fatalf("HKCU ABI argument = %#x, want %#x", got, wantUser) + } + if got := uint64(predefinedRegistryKeyArgument(registry.LOCAL_MACHINE)); got != wantMachine { + t.Fatalf("HKLM ABI argument = %#x, want %#x", got, wantMachine) + } +} From 6b4e99dbedbeac43985636653048b5074de01a81 Mon Sep 17 00:00:00 2001 From: eitsupi Date: Wed, 30 Sep 2026 03:52:42 +0000 Subject: [PATCH 3/3] fix: scope Windows test registry isolation to dbc --- cmd/dbc/registry_test.go | 9 +- cmd/dbc/subcommand_windows_test.go | 5 +- cmd/dbc/test_isolation_windows_test.go | 55 ++++++++++--- config/dirs_windows.go | 5 +- internal/winregroot/roots_windows.go | 62 ++++++++++++++ internal/wintest/isolation_windows.go | 96 +++++++--------------- internal/wintest/isolation_windows_test.go | 40 --------- 7 files changed, 146 insertions(+), 126 deletions(-) create mode 100644 internal/winregroot/roots_windows.go delete mode 100644 internal/wintest/isolation_windows_test.go diff --git a/cmd/dbc/registry_test.go b/cmd/dbc/registry_test.go index c2b5e001..ab368e1c 100644 --- a/cmd/dbc/registry_test.go +++ b/cmd/dbc/registry_test.go @@ -27,13 +27,14 @@ import ( tea "charm.land/bubbletea/v2" "github.com/columnar-tech/dbc/config" + "github.com/columnar-tech/dbc/internal/winregroot" "github.com/stretchr/testify/suite" "golang.org/x/sys/windows/registry" ) // This test suite is only run when the "test_registry" build tag is set. -// The Windows test main redirects both registry hives and driver roots to -// process-private temporary fixtures. These tests do not exercise real +// The Windows test main sets dbc's User/System registry roots and driver roots +// to process-private temporary fixtures. These tests do not exercise real // Program Files permissions or the machine registry's ACLs. type RegistryTestSuite struct { suite.Suite @@ -63,7 +64,7 @@ func (s *RegistryTestSuite) run(m tea.Model) string { func (s *RegistryTestSuite) clearRegistry() { // Clear out any existing ADBC registry keys to ensure a clean slate. - k, err := registry.OpenKey(registry.CURRENT_USER, "SOFTWARE\\ADBC\\Drivers", registry.ALL_ACCESS) + k, err := registry.OpenKey(winregroot.UserRoot(), "SOFTWARE\\ADBC\\Drivers", registry.ALL_ACCESS) if errors.Is(err, registry.ErrNotExist) { return } @@ -101,7 +102,7 @@ func (s *RegistryTestSuite) TestInstallDriver() { out := s.run(m) s.Equal("\nInstalled test-driver-1 1.1.0 to "+s.cfgUserPath, out) - k, err := registry.OpenKey(registry.CURRENT_USER, "SOFTWARE\\ADBC\\Drivers\\test-driver-1", registry.READ) + k, err := registry.OpenKey(winregroot.UserRoot(), "SOFTWARE\\ADBC\\Drivers\\test-driver-1", registry.READ) s.Require().NoError(err) defer k.Close() diff --git a/cmd/dbc/subcommand_windows_test.go b/cmd/dbc/subcommand_windows_test.go index 62900363..3e3ed158 100644 --- a/cmd/dbc/subcommand_windows_test.go +++ b/cmd/dbc/subcommand_windows_test.go @@ -22,6 +22,7 @@ import ( "os" "github.com/columnar-tech/dbc/config" + "github.com/columnar-tech/dbc/internal/winregroot" "golang.org/x/sys/windows/registry" ) @@ -31,11 +32,11 @@ func (suite *SubcommandTestSuite) TearDownTest() { _, system := os.LookupEnv("DBC_TEST_LEVEL_SYSTEM") if user { - suite.Require().NoError(deleteRegistryKeyRecursive(registry.CURRENT_USER, "SOFTWARE\\ADBC\\Drivers")) + suite.Require().NoError(deleteRegistryKeyRecursive(winregroot.UserRoot(), "SOFTWARE\\ADBC\\Drivers")) suite.Require().NoError(os.RemoveAll(config.Get()[config.ConfigUser].Location)) } if system { - suite.Require().NoError(deleteRegistryKeyRecursive(registry.LOCAL_MACHINE, "SOFTWARE\\ADBC\\Drivers")) + suite.Require().NoError(deleteRegistryKeyRecursive(winregroot.SystemRoot(), "SOFTWARE\\ADBC\\Drivers")) suite.Require().NoError(os.RemoveAll(config.Get()[config.ConfigSystem].Location)) } } diff --git a/cmd/dbc/test_isolation_windows_test.go b/cmd/dbc/test_isolation_windows_test.go index fb34a725..2684173a 100644 --- a/cmd/dbc/test_isolation_windows_test.go +++ b/cmd/dbc/test_isolation_windows_test.go @@ -19,13 +19,18 @@ package main import ( "errors" "fmt" + "io" + "net/http" + "net/http/httptest" "os" "os/exec" "path/filepath" "strings" "testing" + "time" "github.com/columnar-tech/dbc/config" + "github.com/columnar-tech/dbc/internal/winregroot" "github.com/columnar-tech/dbc/internal/wintest" "golang.org/x/sys/windows/registry" ) @@ -82,15 +87,15 @@ func TestWindowsTestIsolationAcrossProcesses(t *testing.T) { parentSystemBytes := []byte("parent system registry and payload sentinel") writeIsolationPayload(t, parentUserPath, parentUserBytes) writeIsolationPayload(t, parentSystemPath, parentSystemBytes) - writeIsolationRegistration(t, registry.CURRENT_USER, parentUserPath) - writeIsolationRegistration(t, registry.LOCAL_MACHINE, parentSystemPath) + writeIsolationRegistration(t, winregroot.UserRoot(), parentUserPath) + writeIsolationRegistration(t, winregroot.SystemRoot(), parentSystemPath) t.Setenv("ADBC_DRIVER_PATH", parentUserRoot) t.Setenv("VIRTUAL_ENV", parentSystemRoot) t.Setenv("CONDA_PREFIX", parentSystemRoot) t.Cleanup(func() { var cleanupErr error - cleanupErr = errors.Join(cleanupErr, deleteRegistryKeyRecursive(registry.CURRENT_USER, "SOFTWARE\\ADBC\\Drivers\\"+windowsTestIsolationDriverID)) - cleanupErr = errors.Join(cleanupErr, deleteRegistryKeyRecursive(registry.LOCAL_MACHINE, "SOFTWARE\\ADBC\\Drivers\\"+windowsTestIsolationDriverID)) + cleanupErr = errors.Join(cleanupErr, deleteRegistryKeyRecursive(winregroot.UserRoot(), "SOFTWARE\\ADBC\\Drivers\\"+windowsTestIsolationDriverID)) + cleanupErr = errors.Join(cleanupErr, deleteRegistryKeyRecursive(winregroot.SystemRoot(), "SOFTWARE\\ADBC\\Drivers\\"+windowsTestIsolationDriverID)) cleanupErr = errors.Join(cleanupErr, os.RemoveAll(filepath.Dir(parentUserPath))) cleanupErr = errors.Join(cleanupErr, os.RemoveAll(filepath.Dir(parentSystemPath))) if cleanupErr != nil { @@ -111,8 +116,8 @@ func TestWindowsTestIsolationAcrossProcesses(t *testing.T) { assertIsolationPayload(t, parentUserPath, parentUserBytes) assertIsolationPayload(t, parentSystemPath, parentSystemBytes) - assertIsolationRegistration(t, registry.CURRENT_USER, parentUserPath) - assertIsolationRegistration(t, registry.LOCAL_MACHINE, parentSystemPath) + assertIsolationRegistration(t, winregroot.UserRoot(), parentUserPath) + assertIsolationRegistration(t, winregroot.SystemRoot(), parentSystemPath) } func runWindowsTestIsolationChild(t *testing.T) { @@ -121,6 +126,7 @@ func runWindowsTestIsolationChild(t *testing.T) { t.Fatalf("child inherited unsafe %s value %q", name, value) } } + assertLocalHTTPRoundTrip(t) parentUserRoot := os.Getenv(windowsTestIsolationUserEnv) parentSystemRoot := os.Getenv(windowsTestIsolationSysEnv) @@ -134,30 +140,53 @@ func runWindowsTestIsolationChild(t *testing.T) { childSystemPath := windowsIsolationPayload(childSystemRoot) writeIsolationPayload(t, childUserPath, []byte("child user payload")) writeIsolationPayload(t, childSystemPath, []byte("child system payload")) - writeIsolationRegistration(t, registry.CURRENT_USER, childUserPath) - writeIsolationRegistration(t, registry.LOCAL_MACHINE, childSystemPath) + writeIsolationRegistration(t, winregroot.UserRoot(), childUserPath) + writeIsolationRegistration(t, winregroot.SystemRoot(), childSystemPath) - if err := deleteRegistryKeyRecursive(registry.CURRENT_USER, "SOFTWARE\\ADBC\\Drivers"); err != nil { + if err := deleteRegistryKeyRecursive(winregroot.UserRoot(), "SOFTWARE\\ADBC\\Drivers"); err != nil { t.Fatal(err) } if err := os.RemoveAll(childUserRoot); err != nil { t.Fatalf("remove child User driver root: %v", err) } - assertIsolationRegistrationRemoved(t, registry.CURRENT_USER) + assertIsolationRegistrationRemoved(t, winregroot.UserRoot()) assertIsolationPathRemoved(t, childUserRoot) assertIsolationPayload(t, childSystemPath, []byte("child system payload")) - assertIsolationRegistration(t, registry.LOCAL_MACHINE, childSystemPath) + assertIsolationRegistration(t, winregroot.SystemRoot(), childSystemPath) - if err := deleteRegistryKeyRecursive(registry.LOCAL_MACHINE, "SOFTWARE\\ADBC\\Drivers"); err != nil { + if err := deleteRegistryKeyRecursive(winregroot.SystemRoot(), "SOFTWARE\\ADBC\\Drivers"); err != nil { t.Fatal(err) } if err := os.RemoveAll(childSystemRoot); err != nil { t.Fatalf("remove child System driver root: %v", err) } - assertIsolationRegistrationRemoved(t, registry.LOCAL_MACHINE) + assertIsolationRegistrationRemoved(t, winregroot.SystemRoot()) assertIsolationPathRemoved(t, childSystemRoot) } +func assertLocalHTTPRoundTrip(t *testing.T) { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, "local loopback works") + })) + defer server.Close() + + client := server.Client() + client.Timeout = 5 * time.Second + response, err := client.Get(server.URL) + if err != nil { + t.Fatalf("GET local httptest server: %v", err) + } + body, readErr := io.ReadAll(response.Body) + closeErr := response.Body.Close() + if err := errors.Join(readErr, closeErr); err != nil { + t.Fatalf("read local httptest response: %v", err) + } + if response.StatusCode != http.StatusOK || string(body) != "local loopback works" { + t.Fatalf("local HTTP response = %d %q", response.StatusCode, body) + } +} + func windowsIsolationPayload(root string) string { return filepath.Join(root, filepath.FromSlash(windowsTestIsolationRelative)) } diff --git a/config/dirs_windows.go b/config/dirs_windows.go index b3a22bce..a20b5ff7 100644 --- a/config/dirs_windows.go +++ b/config/dirs_windows.go @@ -25,6 +25,7 @@ import ( "github.com/Masterminds/semver/v3" "github.com/columnar-tech/dbc/internal/systempath" + "github.com/columnar-tech/dbc/internal/winregroot" "golang.org/x/sys/windows/registry" ) @@ -40,9 +41,9 @@ func init() { func (c ConfigLevel) key() registry.Key { switch c { case ConfigSystem: - return registry.LOCAL_MACHINE + return winregroot.SystemRoot() case ConfigUser: - return registry.CURRENT_USER + return winregroot.UserRoot() default: return 0 } diff --git a/internal/winregroot/roots_windows.go b/internal/winregroot/roots_windows.go new file mode 100644 index 00000000..444b5e13 --- /dev/null +++ b/internal/winregroot/roots_windows.go @@ -0,0 +1,62 @@ +// Copyright 2026 Columnar Technologies Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +// Package winregroot provides the registry roots used by dbc's Windows +// registration code. Test overrides affect only callers of this package and do +// not change the registry view seen by Windows system libraries. +package winregroot + +import ( + "sync" + + "golang.org/x/sys/windows/registry" +) + +var ( + rootsMu sync.RWMutex + userRoot = registry.CURRENT_USER + systemRoot = registry.LOCAL_MACHINE +) + +// UserRoot returns the root used for per-user ADBC registration. +func UserRoot() registry.Key { + rootsMu.RLock() + defer rootsMu.RUnlock() + return userRoot +} + +// SystemRoot returns the root used for machine-wide ADBC registration. +func SystemRoot() registry.Key { + rootsMu.RLock() + defer rootsMu.RUnlock() + return systemRoot +} + +// SetRootsForTests replaces the roots used by dbc's own registry accesses and +// returns a function that restores the previous pair. Windows system APIs keep +// using their normal registry view. +func SetRootsForTests(user, system registry.Key) func() { + rootsMu.Lock() + previousUser, previousSystem := userRoot, systemRoot + userRoot, systemRoot = user, system + rootsMu.Unlock() + + return func() { + rootsMu.Lock() + userRoot, systemRoot = previousUser, previousSystem + rootsMu.Unlock() + } +} diff --git a/internal/wintest/isolation_windows.go b/internal/wintest/isolation_windows.go index ee5c58f8..a1e75821 100644 --- a/internal/wintest/isolation_windows.go +++ b/internal/wintest/isolation_windows.go @@ -15,8 +15,8 @@ //go:build windows // Package wintest contains Windows-only test fixture support. The fixture -// redirects registry and filesystem state to process-private temporary roots; -// it does not exercise real Program Files permissions or HKLM ACLs. +// supplies private roots to dbc's own registry and filesystem code without +// changing the registry view used by Windows system APIs. package wintest import ( @@ -28,6 +28,7 @@ import ( "unsafe" "github.com/columnar-tech/dbc/internal/systempath" + "github.com/columnar-tech/dbc/internal/winregroot" "golang.org/x/sys/windows" "golang.org/x/sys/windows/registry" ) @@ -37,26 +38,25 @@ const regProcessAppKey = 0x00000001 var clearForTestsEnvironment = []string{"ADBC_DRIVER_PATH", "VIRTUAL_ENV", "CONDA_PREFIX"} var ( - advapi32DLL = windows.NewLazySystemDLL("advapi32.dll") - regLoadAppKeyWProc = advapi32DLL.NewProc("RegLoadAppKeyW") - regOverridePredefKeyProc = advapi32DLL.NewProc("RegOverridePredefKey") + advapi32DLL = windows.NewLazySystemDLL("advapi32.dll") + regLoadAppKeyWProc = advapi32DLL.NewProc("RegLoadAppKeyW") ) -// Isolation redirects HKCU and HKLM to separate subkeys in a private registry -// hive and places User/System driver files under separate temporary roots. +// Isolation supplies separate private registry handles and filesystem roots to +// dbc's own User/System registration code. It does not change Windows system +// APIs' registry view or exercise real Program Files/HKLM ACLs. type Isolation struct { - tempDir string - appData string - systemRoot string - priorAppData string - hadAppData bool - priorEnv []environmentValue - appHive registry.Key - userHive registry.Key - systemHive registry.Key - userMapped bool - systemMapped bool - restoreRoot func() + tempDir string + appData string + systemRoot string + priorAppData string + hadAppData bool + priorEnv []environmentValue + appHive registry.Key + userHive registry.Key + systemHive registry.Key + restoreRoot func() + restoreRegistryRoot func() } type environmentValue struct { @@ -137,18 +137,11 @@ func (isolation *Isolation) setup() error { if err != nil { return fmt.Errorf("create private HKLM hive root: %w", err) } - if err := overridePredefinedKey(registry.CURRENT_USER, isolation.userHive, "HKCU"); err != nil { + isolation.restoreRegistryRoot = winregroot.SetRootsForTests(isolation.userHive, isolation.systemHive) + if err := verifyRegistryRoot(winregroot.UserRoot(), isolation.userHive, "user"); err != nil { return err } - isolation.userMapped = true - if err := overridePredefinedKey(registry.LOCAL_MACHINE, isolation.systemHive, "HKLM"); err != nil { - return err - } - isolation.systemMapped = true - if err := verifyRegistryOverride(registry.CURRENT_USER, isolation.userHive, "user"); err != nil { - return err - } - if err := verifyRegistryOverride(registry.LOCAL_MACHINE, isolation.systemHive, "system"); err != nil { + if err := verifyRegistryRoot(winregroot.SystemRoot(), isolation.systemHive, "system"); err != nil { return err } return nil @@ -164,17 +157,13 @@ func (isolation *Isolation) SystemRoot() string { return filepath.Join(isolation.systemRoot, "ADBC", "Drivers") } -// Cleanup restores predefined registry mappings and APPDATA, closes all hive -// handles, restores the production system root, and removes the fixture. +// Cleanup restores dbc's registry roots and APPDATA, closes all hive handles, +// restores the production system root, and removes the fixture. func (isolation *Isolation) Cleanup() error { var cleanupErr error - if isolation.systemMapped { - cleanupErr = errors.Join(cleanupErr, resetPredefinedKey(registry.LOCAL_MACHINE, "HKLM")) - isolation.systemMapped = false - } - if isolation.userMapped { - cleanupErr = errors.Join(cleanupErr, resetPredefinedKey(registry.CURRENT_USER, "HKCU")) - isolation.userMapped = false + if isolation.restoreRegistryRoot != nil { + isolation.restoreRegistryRoot() + isolation.restoreRegistryRoot = nil } if isolation.systemHive != 0 { cleanupErr = errors.Join(cleanupErr, isolation.systemHive.Close()) @@ -230,30 +219,7 @@ func loadPrivateRegistryHive(path string) (registry.Key, error) { return registry.Key(hive), nil } -func overridePredefinedKey(predefined, replacement registry.Key, name string) error { - status, _, _ := regOverridePredefKeyProc.Call(predefinedRegistryKeyArgument(predefined), uintptr(replacement)) - if status != 0 { - return fmt.Errorf("RegOverridePredefKey(%s): %w", name, windows.Errno(status)) - } - return nil -} - -func resetPredefinedKey(predefined registry.Key, name string) error { - status, _, _ := regOverridePredefKeyProc.Call(predefinedRegistryKeyArgument(predefined), 0) - if status != 0 { - return fmt.Errorf("RegOverridePredefKey(%s, nil): %w", name, windows.Errno(status)) - } - return nil -} - -func predefinedRegistryKeyArgument(predefined registry.Key) uintptr { - // The Windows SDK defines predefined HKEYs by sign-extending a LONG before - // converting it to ULONG_PTR; Go's registry constants are zero-extended. - // See https://github.com/microsoft/win32metadata/blob/main/generation/WinSDK/RecompiledIdlHeaders/um/winreg.h. - return uintptr(int64(int32(predefined))) -} - -func verifyRegistryOverride(predefined, privateRoot registry.Key, name string) error { +func verifyRegistryRoot(root, privateRoot registry.Key, name string) error { probePath := "Software\\dbc-test-isolation-probe" want := "private-" + name privateKey, _, err := registry.CreateKey(privateRoot, probePath, registry.ALL_ACCESS) @@ -265,14 +231,14 @@ func verifyRegistryOverride(predefined, privateRoot registry.Key, name string) e if err := errors.Join(writeErr, closeErr); err != nil { return fmt.Errorf("write private %s probe: %w", name, err) } - viewKey, err := registry.OpenKey(predefined, probePath, registry.READ) + viewKey, err := registry.OpenKey(root, probePath, registry.READ) if err != nil { - return fmt.Errorf("read private %s probe through predefined key: %w", name, err) + return fmt.Errorf("read private %s probe through dbc registry root: %w", name, err) } got, _, readErr := viewKey.GetStringValue("probe") closeErr = viewKey.Close() if err := errors.Join(readErr, closeErr); err != nil { - return fmt.Errorf("read private %s probe through predefined key: %w", name, err) + return fmt.Errorf("read private %s probe through dbc registry root: %w", name, err) } if got != want { return fmt.Errorf("%s isolation probe = %q, want %q", name, got, want) diff --git a/internal/wintest/isolation_windows_test.go b/internal/wintest/isolation_windows_test.go deleted file mode 100644 index 4d6a2cbb..00000000 --- a/internal/wintest/isolation_windows_test.go +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright 2026 Columnar Technologies Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//go:build windows - -package wintest - -import ( - "testing" - "unsafe" - - "golang.org/x/sys/windows/registry" -) - -func TestPredefinedRegistryKeyArgumentUsesWindowsABIValue(t *testing.T) { - wantUser := uint64(0x80000001) - wantMachine := uint64(0x80000002) - if unsafe.Sizeof(uintptr(0)) == 8 { - wantUser = 0xffffffff80000001 - wantMachine = 0xffffffff80000002 - } - - if got := uint64(predefinedRegistryKeyArgument(registry.CURRENT_USER)); got != wantUser { - t.Fatalf("HKCU ABI argument = %#x, want %#x", got, wantUser) - } - if got := uint64(predefinedRegistryKeyArgument(registry.LOCAL_MACHINE)); got != wantMachine { - t.Fatalf("HKLM ABI argument = %#x, want %#x", got, wantMachine) - } -}