From ae2d9c46cf7edf928ba329969ae0527705fc7409 Mon Sep 17 00:00:00 2001 From: John-Michael Mulesa Date: Wed, 22 Apr 2026 16:55:42 -0400 Subject: [PATCH 1/3] fix: prevent path traversal vulnerabilities by validating absolute paths in file unpacking and test utilities, and add workflow permissions. --- .github/workflows/go.yml | 2 ++ download/download.go | 7 ++++++- googet.goospec | 3 ++- testutil/testutil.go | 12 +++++++++++- 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 70cc66a..6e85c63 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -7,6 +7,8 @@ on: branches: [ master ] schedule: - cron: '0 0 * * 0' # weekly +permissions: + contents: read jobs: diff --git a/download/download.go b/download/download.go index 7690879..a71c500 100644 --- a/download/download.go +++ b/download/download.go @@ -226,7 +226,12 @@ func ExtractPkg(src string) (dst string, err error) { } name := filepath.Clean(header.Name) - if strings.HasPrefix(name, ".."+string(os.PathSeparator)) { + absDst, err := filepath.Abs(dst) + if err != nil { + return "", err + } + absPath := filepath.Join(absDst, name) + if !strings.HasPrefix(absPath, absDst) { return "", fmt.Errorf("error unpacking package, file contains path traversal: %q", name) } diff --git a/googet.goospec b/googet.goospec index d71ca3c..4a6b99b 100644 --- a/googet.goospec +++ b/googet.goospec @@ -1,4 +1,4 @@ -{{$version := "3.3.0@0" -}} +{{$version := "3.3.1@0" -}} { "name": "googet", "version": "{{$version}}", @@ -15,6 +15,7 @@ "path": "install.ps1" }, "releaseNotes": [ + "3.3.1 - Fix: Prevent path traversal vulnerabilities in file unpacking and add workflow permissions.", "3.3.0 - Refactor: Update FindRepoLatest to prioritize repo priority, version, and architecture with lock support.", "3.3.0 - Refactor: Add file ownership conflict checks to prevent overwrites.", "3.2.1 - Refactor: Update GooDB.FetchPkg to accept goolib.PackageInfo.", diff --git a/testutil/testutil.go b/testutil/testutil.go index a493e83..cd3c3ce 100644 --- a/testutil/testutil.go +++ b/testutil/testutil.go @@ -12,6 +12,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "time" @@ -65,7 +66,16 @@ func GenGoo(t *testing.T, dir, dst string, ps goolib.PkgSpec) goolib.RepoSpec { func ServeGoo(t *testing.T, dir string) *httptest.Server { t.Helper() return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - f, err := os.Open(filepath.Join(dir, r.URL.Path)) + absDir, err := filepath.Abs(dir) + if err != nil { + t.Fatal(err) + } + absPath, err := filepath.Abs(filepath.Join(absDir, r.URL.Path)) + if err != nil || !strings.HasPrefix(absPath, absDir) { + http.Error(w, "Invalid file name", http.StatusBadRequest) + return + } + f, err := os.Open(absPath) if err != nil { t.Logf("couldn't find file: %v", r.URL.Path) http.Error(w, "couldn't find requested file", http.StatusNotFound) From 451cf7dc2d4982fa973bda90462342933b23883a Mon Sep 17 00:00:00 2001 From: John-Michael Mulesa Date: Fri, 24 Apr 2026 16:09:46 -0400 Subject: [PATCH 2/3] feat: introduce StrictConflicts setting to optionally enforce file ownership conflict errors --- cli/install/install.go | 2 +- cli/update/update.go | 2 +- install/install.go | 9 +++++++-- install/install_test.go | 22 +++++++++++++++++++--- settings/settings.go | 14 +++++++++----- 5 files changed, 37 insertions(+), 12 deletions(-) diff --git a/cli/install/install.go b/cli/install/install.go index b0e4000..e2e3801 100644 --- a/cli/install/install.go +++ b/cli/install/install.go @@ -58,7 +58,7 @@ func (cmd *installCmd) SetFlags(f *flag.FlagSet) { f.BoolVar(&cmd.dbOnly, "db_only", false, "only make changes to DB, don't perform install system actions") f.StringVar(&cmd.sources, "sources", "", "comma separated list of sources, setting this overrides local .repo files") f.BoolVar(&cmd.dryRun, "dry_run", false, "show what would be installed but do not install") - f.BoolVar(&cmd.force, "force", false, "force overwrite of conflicting files") + f.BoolVar(&cmd.force, "force", false, "force overwrite of conflicting files (only required if StrictConflicts is enabled in config)") } func (cmd *installCmd) Execute(ctx context.Context, flags *flag.FlagSet, _ ...any) subcommands.ExitStatus { diff --git a/cli/update/update.go b/cli/update/update.go index e4e42e2..17dcb48 100644 --- a/cli/update/update.go +++ b/cli/update/update.go @@ -53,7 +53,7 @@ func (cmd *updateCmd) SetFlags(f *flag.FlagSet) { f.BoolVar(&cmd.dbOnly, "db_only", false, "only make changes to DB, don't perform install system actions") f.StringVar(&cmd.sources, "sources", "", "comma separated list of sources, setting this overrides local .repo files") f.BoolVar(&cmd.dryRun, "dry_run", false, "check for updates and print them, but do not prompt to install") - f.BoolVar(&cmd.force, "force", false, "force overwrite of conflicting files") + f.BoolVar(&cmd.force, "force", false, "force overwrite of conflicting files (only required if StrictConflicts is enabled in config)") } func (cmd *updateCmd) Execute(ctx context.Context, _ *flag.FlagSet, _ ...interface{}) subcommands.ExitStatus { diff --git a/install/install.go b/install/install.go index 5b95038..cf8e3c6 100644 --- a/install/install.go +++ b/install/install.go @@ -31,6 +31,7 @@ import ( "github.com/google/googet/v2/goolib" "github.com/google/googet/v2/oswrap" "github.com/google/googet/v2/remove" + "github.com/google/googet/v2/settings" "github.com/google/googet/v2/system" "github.com/google/logger" ) @@ -410,10 +411,14 @@ func makeInstallFunction(src, dst string, insFiles map[string]string, dbOnly, fo outPath := filepath.Join(dst, strings.TrimPrefix(path, src)) if owner, ok := conflictMap[outPath]; ok && !fi.IsDir() { - if !force { + if settings.StrictConflicts && !force { return fmt.Errorf("file conflict: %s is already owned by package %s", outPath, owner) } - logger.Infof("Warning: file conflict: %s is already owned by package %s, overwriting due to force flag", outPath, owner) + if force { + logger.Infof("Warning: file conflict: %s is already owned by package %s, overwriting due to force flag", outPath, owner) + } else { + logger.Infof("Warning: file conflict: %s is already owned by package %s, overwriting because `StrictConflicts` is not set", outPath, owner) + } fmt.Printf("Warning: file conflict: %s is already owned by package %s, overwriting...\n", outPath, owner) } diff --git a/install/install_test.go b/install/install_test.go index b79d97d..9366a15 100644 --- a/install/install_test.go +++ b/install/install_test.go @@ -572,11 +572,11 @@ func TestMakeInstallFunction(t *testing.T) { fi, _ := f.Stat() f.Close() - // Test 1: Conflict without force -> Error + // Test 1: Conflict without force -> Success by default fnBlock := makeInstallFunction(srcDir, dstDir, make(map[string]string), false, false, cm) errBlock := fnBlock(filepath.Join(srcDir, "conflicting_file"), fi, nil) - if errBlock == nil { - t.Errorf("expected conflict error, got nil") + if errBlock != nil { + t.Errorf("expected no conflict error by default, got %v", errBlock) } // Test 2: Conflict with force -> Success @@ -585,4 +585,20 @@ func TestMakeInstallFunction(t *testing.T) { if errForce != nil { t.Errorf("expected no error with force, got %v", errForce) } + + // Test 3: Conflict without force in strict mode -> Error + settings.StrictConflicts = true + defer func() { settings.StrictConflicts = false }() + fnStrict := makeInstallFunction(srcDir, dstDir, make(map[string]string), false, false, cm) + errStrict := fnStrict(filepath.Join(srcDir, "conflicting_file"), fi, nil) + if errStrict == nil { + t.Errorf("expected conflict error in strict mode, got nil") + } + + // Test 4: Conflict with force in strict mode -> Success + fnStrictForce := makeInstallFunction(srcDir, dstDir, make(map[string]string), false, true, cm) + errStrictForce := fnStrictForce(filepath.Join(srcDir, "conflicting_file"), fi, nil) + if errStrictForce != nil { + t.Errorf("expected no error with force in strict mode, got %v", errStrictForce) + } } diff --git a/settings/settings.go b/settings/settings.go index 75d188e..59a0bec 100644 --- a/settings/settings.go +++ b/settings/settings.go @@ -29,6 +29,8 @@ var ( ProxyServer string // AllowUnsafeURL allows HTTP repos; set from googet.conf. AllowUnsafeURL bool + // StrictConflicts enables strict enforcement of file ownership conflicts. + StrictConflicts bool ) // Initialize reads the initial settings. @@ -76,11 +78,12 @@ func RepoDir() string { // conf represents a googet configuration file. type conf struct { - Archs []string - CacheLife string - LockFileMaxAge string - ProxyServer string - AllowUnsafeURL bool + Archs []string + CacheLife string + LockFileMaxAge string + ProxyServer string + AllowUnsafeURL bool + StrictConflicts bool } // unmarshalConfFile returns a conf from a YAML configuration file. @@ -138,4 +141,5 @@ func readConf(filename string) { } AllowUnsafeURL = gc.AllowUnsafeURL + StrictConflicts = gc.StrictConflicts } From c42eb5e463df4eeaa557649397296f7885e7832c Mon Sep 17 00:00:00 2001 From: John-Michael Mulesa Date: Fri, 24 Apr 2026 16:12:31 -0400 Subject: [PATCH 3/3] chore: bump version to 3.3.2 and add StrictConflicts release note --- googet.goospec | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/googet.goospec b/googet.goospec index 4a6b99b..b9fa5e3 100644 --- a/googet.goospec +++ b/googet.goospec @@ -1,4 +1,4 @@ -{{$version := "3.3.1@0" -}} +{{$version := "3.3.2@0" -}} { "name": "googet", "version": "{{$version}}", @@ -15,6 +15,7 @@ "path": "install.ps1" }, "releaseNotes": [ + "3.3.2 - Feat: introduce StrictConflicts setting to optionally enforce file ownership conflict errors.", "3.3.1 - Fix: Prevent path traversal vulnerabilities in file unpacking and add workflow permissions.", "3.3.0 - Refactor: Update FindRepoLatest to prioritize repo priority, version, and architecture with lock support.", "3.3.0 - Refactor: Add file ownership conflict checks to prevent overwrites.",