mirror of
https://github.com/danielmiessler/fabric.git
synced 2026-09-10 07:36:44 -04:00
fix: prevent pattern loader temporary directory leaks
- Create pattern temporary directories only during database population. - Remove temporary directories after successful or failed downloads. - Add regression tests for lazy creation and cleanup. - Document pattern loader cleanup behavior in changelog.
This commit is contained in:
parent
8dce453ed7
commit
f141b68f12
4
cmd/generate_changelog/incoming/2211.txt
Normal file
4
cmd/generate_changelog/incoming/2211.txt
Normal file
|
|
@ -0,0 +1,4 @@
|
|||
### PR [#2211](https://github.com/danielmiessler/Fabric/pull/2211) by [ksylvan](https://github.com/ksylvan): fix: prevent pattern loader temporary directory leaks
|
||||
|
||||
- Prevent pattern loader leaks by lazily creating temporary directories during database population and cleaning them up after successful or failed downloads.
|
||||
- Add regression tests for lazy directory creation and cleanup.
|
||||
|
|
@ -59,13 +59,6 @@ type PatternsLoader struct {
|
|||
|
||||
func (o *PatternsLoader) configure() (err error) {
|
||||
o.pathPatternsPrefix = fmt.Sprintf("%v/", o.DefaultFolder.Value)
|
||||
// Use a consistent temp folder name regardless of the source path structure
|
||||
tempDir, err := os.MkdirTemp("", "fabric-patterns-")
|
||||
if err != nil {
|
||||
return fmt.Errorf(i18n.T("patterns_failed_create_temp_folder"), err)
|
||||
}
|
||||
o.tempPatternsFolder = tempDir
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -96,6 +89,15 @@ func (o *PatternsLoader) PopulateDB() (err error) {
|
|||
fmt.Println()
|
||||
fmt.Println()
|
||||
|
||||
// Create the temp folder here, not in configure(), so invocations that
|
||||
// do not download patterns do not leak an empty directory (issue #2190).
|
||||
var tempDir string
|
||||
if tempDir, err = os.MkdirTemp("", "fabric-patterns-"); err != nil {
|
||||
return fmt.Errorf(i18n.T("patterns_failed_create_temp_folder"), err)
|
||||
}
|
||||
o.tempPatternsFolder = tempDir
|
||||
defer os.RemoveAll(tempDir)
|
||||
|
||||
originalPath := o.DefaultFolder.Value
|
||||
if err = o.gitCloneAndCopy(); err != nil {
|
||||
return fmt.Errorf(i18n.T("patterns_failed_download_from_git"), err)
|
||||
|
|
|
|||
63
internal/tools/patterns_loader_test.go
Normal file
63
internal/tools/patterns_loader_test.go
Normal file
|
|
@ -0,0 +1,63 @@
|
|||
package tools
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/danielmiessler/fabric/internal/plugins/db/fsdb"
|
||||
)
|
||||
|
||||
// Configure runs on every fabric invocation via the plugin registry. It must
|
||||
// not create the patterns temp directory; only PopulateDB uses it.
|
||||
func TestConfigureDoesNotCreateTempDir(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
t.Setenv("TMPDIR", tmp)
|
||||
t.Setenv("TMP", tmp)
|
||||
t.Setenv("TEMP", tmp)
|
||||
|
||||
loader := NewPatternsLoader(&fsdb.PatternsEntity{
|
||||
StorageEntity: &fsdb.StorageEntity{Dir: t.TempDir()},
|
||||
})
|
||||
if err := loader.Configure(); err != nil {
|
||||
t.Fatalf("Configure() failed: %v", err)
|
||||
}
|
||||
|
||||
matches, err := filepath.Glob(filepath.Join(tmp, "fabric-patterns-*"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(matches) != 0 {
|
||||
t.Errorf("Configure() created temp directories: %v", matches)
|
||||
}
|
||||
}
|
||||
|
||||
// PopulateDB must create the temp directory lazily and remove it when done,
|
||||
// even on failure.
|
||||
func TestPopulateDBCleansUpTempDir(t *testing.T) {
|
||||
tmp := t.TempDir()
|
||||
t.Setenv("TMPDIR", tmp)
|
||||
t.Setenv("TMP", tmp)
|
||||
t.Setenv("TEMP", tmp)
|
||||
|
||||
loader := NewPatternsLoader(&fsdb.PatternsEntity{
|
||||
StorageEntity: &fsdb.StorageEntity{Dir: t.TempDir()},
|
||||
})
|
||||
if err := loader.Configure(); err != nil {
|
||||
t.Fatalf("Configure() failed: %v", err)
|
||||
}
|
||||
// Point at an invalid repo so PopulateDB fails fast without network.
|
||||
loader.DefaultGitRepoUrl.Value = filepath.Join(t.TempDir(), "no-such-repo")
|
||||
|
||||
if err := loader.PopulateDB(); err == nil {
|
||||
t.Fatal("PopulateDB() unexpectedly succeeded with invalid repo")
|
||||
}
|
||||
|
||||
entries, err := os.ReadDir(tmp)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, e := range entries {
|
||||
t.Errorf("PopulateDB() left temp entry behind: %v", e.Name())
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue