From 16924a51db7d44698d999c8b9c3bf2fdc74bc60f Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Mon, 25 Aug 2025 10:16:03 -0400 Subject: [PATCH] Fix: Removed unsafe umask manipulation in tests Problem: Tests used syscall.Umask(0000) which could leave your shell creating files with 777 permissions if interrupted. Solution: Instead of changing umask, tests now detect the current umask and calculate expected permissions after it's applied. Result: Same test coverage, but safe from system-wide side effects. Co-authored-by: Jesse Simpson Signed-off-by: Scott Rigby --- .../plugin/installer/http_installer_test.go | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/internal/plugin/installer/http_installer_test.go b/internal/plugin/installer/http_installer_test.go index ac74b8cf6..453021b76 100644 --- a/internal/plugin/installer/http_installer_test.go +++ b/internal/plugin/installer/http_installer_test.go @@ -210,11 +210,9 @@ func TestExtract(t *testing.T) { tempDir := t.TempDir() - // Set the umask to default open permissions so we can actually test - oldmask := syscall.Umask(0000) - defer func() { - syscall.Umask(oldmask) - }() + // Get current umask to predict expected permissions + currentUmask := syscall.Umask(0) + syscall.Umask(currentUmask) // Write a tarball to a buffer for us to extract var tarbuf bytes.Buffer @@ -274,14 +272,19 @@ func TestExtract(t *testing.T) { t.Fatalf("Did not expect error but got error: %v", err) } + // Calculate expected permissions after umask is applied + expectedPluginYAMLPerm := os.FileMode(0600 &^ currentUmask) + expectedReadmePerm := os.FileMode(0777 &^ currentUmask) + pluginYAMLFullPath := filepath.Join(tempDir, "plugin.yaml") if info, err := os.Stat(pluginYAMLFullPath); err != nil { if errors.Is(err, fs.ErrNotExist) { t.Fatalf("Expected %s to exist but doesn't", pluginYAMLFullPath) } t.Fatal(err) - } else if info.Mode().Perm() != 0600 { - t.Fatalf("Expected %s to have 0600 mode it but has %o", pluginYAMLFullPath, info.Mode().Perm()) + } else if info.Mode().Perm() != expectedPluginYAMLPerm { + t.Fatalf("Expected %s to have %o mode but has %o (umask: %o)", + pluginYAMLFullPath, expectedPluginYAMLPerm, info.Mode().Perm(), currentUmask) } readmeFullPath := filepath.Join(tempDir, "README.md") @@ -290,8 +293,9 @@ func TestExtract(t *testing.T) { t.Fatalf("Expected %s to exist but doesn't", readmeFullPath) } t.Fatal(err) - } else if info.Mode().Perm() != 0777 { - t.Fatalf("Expected %s to have 0777 mode it but has %o", readmeFullPath, info.Mode().Perm()) + } else if info.Mode().Perm() != expectedReadmePerm { + t.Fatalf("Expected %s to have %o mode but has %o (umask: %o)", + readmeFullPath, expectedReadmePerm, info.Mode().Perm(), currentUmask) } } @@ -353,12 +357,6 @@ func TestExtractWithNestedDirectories(t *testing.T) { source := "https://repo.localdomain/plugins/nested-plugin-0.0.1.tar.gz" tempDir := t.TempDir() - // Set the umask to default open permissions so we can actually test - oldmask := syscall.Umask(0000) - defer func() { - syscall.Umask(oldmask) - }() - // Write a tarball with nested directory structure var tarbuf bytes.Buffer tw := tar.NewWriter(&tarbuf)