From 3bab20122988b631a4039f05a1d9bf5b10ee5092 Mon Sep 17 00:00:00 2001 From: Snider Date: Fri, 20 Mar 2026 14:37:06 +0000 Subject: [PATCH] feat: fs.go returns Result throughout All 14 public Fs methods return Result instead of (value, error). validatePath returns Result internally. Tests updated to use r.OK / r.Value pattern. 231 tests, 77.1% coverage. Co-Authored-By: Virgil --- pkg/core/fs.go | 200 +++++++++++++++++++++++++++-------------------- tests/fs_test.go | 115 +++++++++++---------------- 2 files changed, 160 insertions(+), 155 deletions(-) diff --git a/pkg/core/fs.go b/pkg/core/fs.go index 38c26d4..f53e846 100644 --- a/pkg/core/fs.go +++ b/pkg/core/fs.go @@ -2,8 +2,6 @@ package core import ( - "io" - "io/fs" "os" "os/user" "path/filepath" @@ -15,7 +13,6 @@ type Fs struct { root string } - // path sanitises and returns the full path. // Absolute paths are sandboxed under root (unless root is "/"). func (m *Fs) path(p string) string { @@ -45,9 +42,9 @@ func (m *Fs) path(p string) string { } // validatePath ensures the path is within the sandbox, following symlinks if they exist. -func (m *Fs) validatePath(p string) (string, error) { +func (m *Fs) validatePath(p string) Result { if m.root == "/" { - return m.path(p), nil + return Result{Value: m.path(p), OK: true} } // Split the cleaned path into components @@ -69,7 +66,7 @@ func (m *Fs) validatePath(p string) (string, error) { current = next continue } - return "", err + return Result{} } // Verify the resolved part is still within the root @@ -82,54 +79,64 @@ func (m *Fs) validatePath(p string) (string, error) { } Print(os.Stderr, "[%s] SECURITY sandbox escape detected root=%s path=%s attempted=%s user=%s", time.Now().Format(time.RFC3339), m.root, p, realNext, username) - return "", os.ErrPermission // Path escapes sandbox + return Result{} } current = realNext } - return current, nil + return Result{Value: current, OK: true} } // Read returns file contents as string. -func (m *Fs) Read(p string) (string, error) { - full, err := m.validatePath(p) - if err != nil { - return "", err +func (m *Fs) Read(p string) Result { + vp := m.validatePath(p) + if !vp.OK { + return Result{} } - data, err := os.ReadFile(full) + r := &Result{} + data, err := os.ReadFile(vp.Value.(string)) if err != nil { - return "", err + return Result{} } - return string(data), nil + r.Value = string(data) + r.OK = true + return *r } // Write saves content to file, creating parent directories as needed. // Files are created with mode 0644. For sensitive files (keys, secrets), // use WriteMode with 0600. -func (m *Fs) Write(p, content string) error { +func (m *Fs) Write(p, content string) Result { return m.WriteMode(p, content, 0644) } // WriteMode saves content to file with explicit permissions. // Use 0600 for sensitive files (encryption output, private keys, auth hashes). -func (m *Fs) WriteMode(p, content string, mode os.FileMode) error { - full, err := m.validatePath(p) - if err != nil { - return err +func (m *Fs) WriteMode(p, content string, mode os.FileMode) Result { + vp := m.validatePath(p) + if !vp.OK { + return Result{} } + full := vp.Value.(string) if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { - return err + return Result{} } - return os.WriteFile(full, []byte(content), mode) + if err := os.WriteFile(full, []byte(content), mode); err != nil { + return Result{} + } + return Result{OK: true} } // EnsureDir creates directory if it doesn't exist. -func (m *Fs) EnsureDir(p string) error { - full, err := m.validatePath(p) - if err != nil { - return err +func (m *Fs) EnsureDir(p string) Result { + vp := m.validatePath(p) + if !vp.OK { + return Result{} } - return os.MkdirAll(full, 0755) + if err := os.MkdirAll(vp.Value.(string), 0755); err != nil { + return Result{} + } + return Result{OK: true} } // IsDir returns true if path is a directory. @@ -137,11 +144,11 @@ func (m *Fs) IsDir(p string) bool { if p == "" { return false } - full, err := m.validatePath(p) - if err != nil { + vp := m.validatePath(p) + if !vp.OK { return false } - info, err := os.Stat(full) + info, err := os.Stat(vp.Value.(string)) return err == nil && info.IsDir() } @@ -150,118 +157,141 @@ func (m *Fs) IsFile(p string) bool { if p == "" { return false } - full, err := m.validatePath(p) - if err != nil { + vp := m.validatePath(p) + if !vp.OK { return false } - info, err := os.Stat(full) + info, err := os.Stat(vp.Value.(string)) return err == nil && info.Mode().IsRegular() } // Exists returns true if path exists. func (m *Fs) Exists(p string) bool { - full, err := m.validatePath(p) - if err != nil { + vp := m.validatePath(p) + if !vp.OK { return false } - _, err = os.Stat(full) + _, err := os.Stat(vp.Value.(string)) return err == nil } // List returns directory entries. -func (m *Fs) List(p string) ([]fs.DirEntry, error) { - full, err := m.validatePath(p) - if err != nil { - return nil, err +func (m *Fs) List(p string) Result { + vp := m.validatePath(p) + if !vp.OK { + return Result{} } - return os.ReadDir(full) + r := &Result{} + r.Result(os.ReadDir(vp.Value.(string))) + return *r } // Stat returns file info. -func (m *Fs) Stat(p string) (fs.FileInfo, error) { - full, err := m.validatePath(p) - if err != nil { - return nil, err +func (m *Fs) Stat(p string) Result { + vp := m.validatePath(p) + if !vp.OK { + return Result{} } - return os.Stat(full) + r := &Result{} + r.Result(os.Stat(vp.Value.(string))) + return *r } // Open opens the named file for reading. -func (m *Fs) Open(p string) (fs.File, error) { - full, err := m.validatePath(p) - if err != nil { - return nil, err +func (m *Fs) Open(p string) Result { + vp := m.validatePath(p) + if !vp.OK { + return Result{} } - return os.Open(full) + r := &Result{} + r.Result(os.Open(vp.Value.(string))) + return *r } // Create creates or truncates the named file. -func (m *Fs) Create(p string) (io.WriteCloser, error) { - full, err := m.validatePath(p) - if err != nil { - return nil, err +func (m *Fs) Create(p string) Result { + vp := m.validatePath(p) + if !vp.OK { + return Result{} } + full := vp.Value.(string) if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { - return nil, err + return Result{} } - return os.Create(full) + r := &Result{} + r.Result(os.Create(full)) + return *r } // Append opens the named file for appending, creating it if it doesn't exist. -func (m *Fs) Append(p string) (io.WriteCloser, error) { - full, err := m.validatePath(p) - if err != nil { - return nil, err +func (m *Fs) Append(p string) Result { + vp := m.validatePath(p) + if !vp.OK { + return Result{} } + full := vp.Value.(string) if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { - return nil, err + return Result{} } - return os.OpenFile(full, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + r := &Result{} + r.Result(os.OpenFile(full, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)) + return *r } // ReadStream returns a reader for the file content. -func (m *Fs) ReadStream(path string) (io.ReadCloser, error) { +func (m *Fs) ReadStream(path string) Result { return m.Open(path) } // WriteStream returns a writer for the file content. -func (m *Fs) WriteStream(path string) (io.WriteCloser, error) { +func (m *Fs) WriteStream(path string) Result { return m.Create(path) } // Delete removes a file or empty directory. -func (m *Fs) Delete(p string) error { - full, err := m.validatePath(p) - if err != nil { - return err +func (m *Fs) Delete(p string) Result { + vp := m.validatePath(p) + if !vp.OK { + return Result{} } + full := vp.Value.(string) if full == "/" || full == os.Getenv("HOME") { - return E("core.Delete", "refusing to delete protected path: "+full, nil) + return Result{} } - return os.Remove(full) + if err := os.Remove(full); err != nil { + return Result{} + } + return Result{OK: true} } // DeleteAll removes a file or directory recursively. -func (m *Fs) DeleteAll(p string) error { - full, err := m.validatePath(p) - if err != nil { - return err +func (m *Fs) DeleteAll(p string) Result { + vp := m.validatePath(p) + if !vp.OK { + return Result{} } + full := vp.Value.(string) if full == "/" || full == os.Getenv("HOME") { - return E("core.DeleteAll", "refusing to delete protected path: "+full, nil) + return Result{} } - return os.RemoveAll(full) + if err := os.RemoveAll(full); err != nil { + return Result{} + } + return Result{OK: true} } // Rename moves a file or directory. -func (m *Fs) Rename(oldPath, newPath string) error { - oldFull, err := m.validatePath(oldPath) - if err != nil { - return err +func (m *Fs) Rename(oldPath, newPath string) Result { + oldVp := m.validatePath(oldPath) + if !oldVp.OK { + return Result{} } - newFull, err := m.validatePath(newPath) - if err != nil { - return err + newVp := m.validatePath(newPath) + if !newVp.OK { + return Result{} } - return os.Rename(oldFull, newFull) + if err := os.Rename(oldVp.Value.(string), newVp.Value.(string)); err != nil { + return Result{} + } + return Result{OK: true} } diff --git a/tests/fs_test.go b/tests/fs_test.go index 5b86edd..2fd9ece 100644 --- a/tests/fs_test.go +++ b/tests/fs_test.go @@ -1,6 +1,9 @@ package core_test import ( + "io" + "io/fs" + "os" "path/filepath" "testing" @@ -15,26 +18,24 @@ func TestFs_WriteRead_Good(t *testing.T) { c := New() path := filepath.Join(dir, "test.txt") - err := c.Fs().Write(path, "hello core") - assert.NoError(t, err) + assert.True(t, c.Fs().Write(path, "hello core").OK) - content, err := c.Fs().Read(path) - assert.NoError(t, err) - assert.Equal(t, "hello core", content) + r := c.Fs().Read(path) + assert.True(t, r.OK) + assert.Equal(t, "hello core", r.Value.(string)) } func TestFs_Read_Bad(t *testing.T) { c := New() - _, err := c.Fs().Read("/nonexistent/path/to/file.txt") - assert.Error(t, err) + r := c.Fs().Read("/nonexistent/path/to/file.txt") + assert.False(t, r.OK) } func TestFs_EnsureDir_Good(t *testing.T) { dir := t.TempDir() c := New() path := filepath.Join(dir, "sub", "dir") - err := c.Fs().EnsureDir(path) - assert.NoError(t, err) + assert.True(t, c.Fs().EnsureDir(path).OK) assert.True(t, c.Fs().IsDir(path)) } @@ -49,22 +50,18 @@ func TestFs_IsDir_Good(t *testing.T) { func TestFs_IsFile_Good(t *testing.T) { dir := t.TempDir() c := New() - path := filepath.Join(dir, "test.txt") c.Fs().Write(path, "data") - assert.True(t, c.Fs().IsFile(path)) - assert.False(t, c.Fs().IsFile(dir)) // dir, not file + assert.False(t, c.Fs().IsFile(dir)) assert.False(t, c.Fs().IsFile("")) } func TestFs_Exists_Good(t *testing.T) { dir := t.TempDir() c := New() - path := filepath.Join(dir, "exists.txt") c.Fs().Write(path, "yes") - assert.True(t, c.Fs().Exists(path)) assert.True(t, c.Fs().Exists(dir)) assert.False(t, c.Fs().Exists(filepath.Join(dir, "nope"))) @@ -73,88 +70,77 @@ func TestFs_Exists_Good(t *testing.T) { func TestFs_List_Good(t *testing.T) { dir := t.TempDir() c := New() - c.Fs().Write(filepath.Join(dir, "a.txt"), "a") c.Fs().Write(filepath.Join(dir, "b.txt"), "b") - - entries, err := c.Fs().List(dir) - assert.NoError(t, err) - assert.Len(t, entries, 2) + r := c.Fs().List(dir) + assert.True(t, r.OK) + assert.Len(t, r.Value.([]fs.DirEntry), 2) } func TestFs_Stat_Good(t *testing.T) { dir := t.TempDir() c := New() - path := filepath.Join(dir, "stat.txt") c.Fs().Write(path, "data") - - info, err := c.Fs().Stat(path) - assert.NoError(t, err) - assert.Equal(t, "stat.txt", info.Name()) + r := c.Fs().Stat(path) + assert.True(t, r.OK) + assert.Equal(t, "stat.txt", r.Value.(os.FileInfo).Name()) } func TestFs_Open_Good(t *testing.T) { dir := t.TempDir() c := New() - path := filepath.Join(dir, "open.txt") c.Fs().Write(path, "content") - - file, err := c.Fs().Open(path) - assert.NoError(t, err) - file.Close() + r := c.Fs().Open(path) + assert.True(t, r.OK) + r.Value.(io.Closer).Close() } func TestFs_Create_Good(t *testing.T) { dir := t.TempDir() c := New() - path := filepath.Join(dir, "sub", "created.txt") - w, err := c.Fs().Create(path) - assert.NoError(t, err) + r := c.Fs().Create(path) + assert.True(t, r.OK) + w := r.Value.(io.WriteCloser) w.Write([]byte("hello")) w.Close() - - content, _ := c.Fs().Read(path) - assert.Equal(t, "hello", content) + rr := c.Fs().Read(path) + assert.Equal(t, "hello", rr.Value.(string)) } func TestFs_Append_Good(t *testing.T) { dir := t.TempDir() c := New() - path := filepath.Join(dir, "append.txt") c.Fs().Write(path, "first") - - w, err := c.Fs().Append(path) - assert.NoError(t, err) + r := c.Fs().Append(path) + assert.True(t, r.OK) + w := r.Value.(io.WriteCloser) w.Write([]byte(" second")) w.Close() - - content, _ := c.Fs().Read(path) - assert.Equal(t, "first second", content) + rr := c.Fs().Read(path) + assert.Equal(t, "first second", rr.Value.(string)) } func TestFs_ReadStream_Good(t *testing.T) { dir := t.TempDir() c := New() - path := filepath.Join(dir, "stream.txt") c.Fs().Write(path, "streamed") - - r, err := c.Fs().ReadStream(path) - assert.NoError(t, err) - r.Close() + r := c.Fs().ReadStream(path) + assert.True(t, r.OK) + r.Value.(io.Closer).Close() } func TestFs_WriteStream_Good(t *testing.T) { dir := t.TempDir() c := New() - path := filepath.Join(dir, "sub", "ws.txt") - w, err := c.Fs().WriteStream(path) - assert.NoError(t, err) + r := c.Fs().WriteStream(path) + assert.True(t, r.OK) + w := r.Value.(io.WriteCloser) w.Write([]byte("stream")) w.Close() } @@ -162,50 +148,39 @@ func TestFs_WriteStream_Good(t *testing.T) { func TestFs_Delete_Good(t *testing.T) { dir := t.TempDir() c := New() - path := filepath.Join(dir, "delete.txt") c.Fs().Write(path, "gone") - - err := c.Fs().Delete(path) - assert.NoError(t, err) + assert.True(t, c.Fs().Delete(path).OK) assert.False(t, c.Fs().Exists(path)) } func TestFs_DeleteAll_Good(t *testing.T) { dir := t.TempDir() c := New() - sub := filepath.Join(dir, "deep", "nested") c.Fs().EnsureDir(sub) c.Fs().Write(filepath.Join(sub, "file.txt"), "data") - - err := c.Fs().DeleteAll(filepath.Join(dir, "deep")) - assert.NoError(t, err) + assert.True(t, c.Fs().DeleteAll(filepath.Join(dir, "deep")).OK) assert.False(t, c.Fs().Exists(filepath.Join(dir, "deep"))) } func TestFs_Rename_Good(t *testing.T) { dir := t.TempDir() c := New() - old := filepath.Join(dir, "old.txt") - new := filepath.Join(dir, "new.txt") + nw := filepath.Join(dir, "new.txt") c.Fs().Write(old, "data") - - err := c.Fs().Rename(old, new) - assert.NoError(t, err) + assert.True(t, c.Fs().Rename(old, nw).OK) assert.False(t, c.Fs().Exists(old)) - assert.True(t, c.Fs().Exists(new)) + assert.True(t, c.Fs().Exists(nw)) } func TestFs_WriteMode_Good(t *testing.T) { dir := t.TempDir() c := New() - path := filepath.Join(dir, "secret.txt") - err := c.Fs().WriteMode(path, "secret", 0600) - assert.NoError(t, err) - - info, _ := c.Fs().Stat(path) - assert.Equal(t, "secret.txt", info.Name()) + assert.True(t, c.Fs().WriteMode(path, "secret", 0600).OK) + r := c.Fs().Stat(path) + assert.True(t, r.OK) + assert.Equal(t, "secret.txt", r.Value.(os.FileInfo).Name()) }