From f8988c51cb94709cdda13715d1758c6d8ec7de97 Mon Sep 17 00:00:00 2001 From: Virgil Date: Mon, 30 Mar 2026 22:56:51 +0000 Subject: [PATCH] refactor(ax): tighten naming and comment surfaces --- datanode/medium.go | 1 - local/medium.go | 4 --- node/node.go | 5 --- s3/s3.go | 4 --- sqlite/sqlite.go | 16 --------- sqlite/sqlite_test.go | 2 -- workspace/service.go | 6 ++-- workspace/service_test.go | 72 +++++++++++++++++++-------------------- 8 files changed, 39 insertions(+), 71 deletions(-) diff --git a/datanode/medium.go b/datanode/medium.go index bad0d75..574679c 100644 --- a/datanode/medium.go +++ b/datanode/medium.go @@ -356,7 +356,6 @@ func (medium *Medium) List(filePath string) ([]fs.DirEntry, error) { return nil, core.E("datanode.List", core.Concat("not found: ", filePath), fs.ErrNotExist) } - // Also include explicit subdirectories not discovered via files prefix := filePath if prefix != "" { prefix += "/" diff --git a/local/medium.go b/local/medium.go index b1c43ed..95aa5bb 100644 --- a/local/medium.go +++ b/local/medium.go @@ -177,9 +177,6 @@ func (medium *Medium) sandboxedPath(path string) string { return medium.filesystemRoot } - // If the path is relative and the medium is rooted at "/", - // treat it as relative to the current working directory. - // This makes io.Local behave more like the standard 'os' package. if medium.filesystemRoot == dirSeparator() && !core.PathIsAbs(normalisePath(path)) { return core.Path(currentWorkingDir(), normalisePath(path)) } @@ -222,7 +219,6 @@ func (medium *Medium) validatePath(path string) (string, error) { // Verify the resolved part is still within the root if !isWithinRoot(medium.filesystemRoot, realNext) { - // Security event: sandbox escape attempt logSandboxEscape(medium.filesystemRoot, path, realNext) return "", fs.ErrPermission } diff --git a/node/node.go b/node/node.go index 73f3b68..a62d88a 100644 --- a/node/node.go +++ b/node/node.go @@ -41,7 +41,6 @@ func (node *Node) AddData(name string, content []byte) { if name == "" { return } - // Directories are implicit, so we don't store them. if core.HasSuffix(name, "/") { return } @@ -159,7 +158,6 @@ func (node *Node) WalkWithOptions(root string, fn fs.WalkDirFunc, options WalkOp } } - // Call the user's function first so the entry is visited. result := fn(entryPath, entry, err) // After visiting a directory at MaxDepth, prevent descending further. @@ -194,7 +192,6 @@ func (node *Node) CopyFile(sourcePath, destinationPath string, perm fs.FileMode) sourcePath = core.TrimPrefix(sourcePath, "/") file, ok := node.files[sourcePath] if !ok { - // Check if it's a directory — can't copy directories this way. info, err := node.Stat(sourcePath) if err != nil { return core.E("node.CopyFile", core.Concat("source not found: ", sourcePath), fs.ErrNotExist) @@ -257,7 +254,6 @@ func (node *Node) Open(name string) (fs.File, error) { if dataFile, ok := node.files[name]; ok { return &dataFileReader{file: dataFile}, nil } - // Check if it's a directory prefix := name + "/" if name == "." || name == "" { prefix = "" @@ -275,7 +271,6 @@ func (node *Node) Stat(name string) (fs.FileInfo, error) { if dataFile, ok := node.files[name]; ok { return dataFile.Stat() } - // Check if it's a directory prefix := name + "/" if name == "." || name == "" { prefix = "" diff --git a/s3/s3.go b/s3/s3.go index 53366e6..384b8ce 100644 --- a/s3/s3.go +++ b/s3/s3.go @@ -329,7 +329,6 @@ func (medium *Medium) List(filePath string) ([]fs.DirEntry, error) { return nil, core.E("s3.List", core.Concat("failed to list objects: ", prefix), err) } - // Common prefixes are "directories" for _, commonPrefix := range listOutput.CommonPrefixes { if commonPrefix.Prefix == nil { continue @@ -351,7 +350,6 @@ func (medium *Medium) List(filePath string) ([]fs.DirEntry, error) { }) } - // Contents are "files" (excluding the prefix itself) for _, object := range listOutput.Contents { if object.Key == nil { continue @@ -517,7 +515,6 @@ func (medium *Medium) Exists(filePath string) bool { return false } - // Check as an exact object _, err := medium.client.HeadObject(context.Background(), &awss3.HeadObjectInput{ Bucket: aws.String(medium.bucket), Key: aws.String(key), @@ -526,7 +523,6 @@ func (medium *Medium) Exists(filePath string) bool { return true } - // Check as a "directory" prefix prefix := key if !core.HasSuffix(prefix, "/") { prefix += "/" diff --git a/sqlite/sqlite.go b/sqlite/sqlite.go index a6fc0ae..081e11c 100644 --- a/sqlite/sqlite.go +++ b/sqlite/sqlite.go @@ -53,13 +53,11 @@ func New(options Options) (*Medium, error) { return nil, core.E("sqlite.New", "failed to open database", err) } - // Enable WAL mode for better concurrency if _, err := database.Exec("PRAGMA journal_mode=WAL"); err != nil { database.Close() return nil, core.E("sqlite.New", "failed to set WAL mode", err) } - // Create the schema createSQL := `CREATE TABLE IF NOT EXISTS ` + medium.table + ` ( path TEXT PRIMARY KEY, content BLOB NOT NULL, @@ -141,7 +139,6 @@ func (medium *Medium) WriteMode(filePath, content string, mode fs.FileMode) erro func (medium *Medium) EnsureDir(filePath string) error { key := normaliseEntryPath(filePath) if key == "" { - // Root always "exists" return nil } @@ -187,7 +184,6 @@ func (medium *Medium) Delete(filePath string) error { return core.E("sqlite.Delete", "path is required", fs.ErrInvalid) } - // Check if it's a directory with children var isDir bool err := medium.database.QueryRow( `SELECT is_dir FROM `+medium.table+` WHERE path = ?`, key, @@ -200,7 +196,6 @@ func (medium *Medium) Delete(filePath string) error { } if isDir { - // Check for children prefix := key + "/" var count int err := medium.database.QueryRow( @@ -234,7 +229,6 @@ func (medium *Medium) DeleteAll(filePath string) error { prefix := key + "/" - // Delete the exact path and all children res, err := medium.database.Exec( `DELETE FROM `+medium.table+` WHERE path = ? OR path LIKE ?`, key, prefix+"%", @@ -263,7 +257,6 @@ func (medium *Medium) Rename(oldPath, newPath string) error { } defer tx.Rollback() - // Check if source exists var content []byte var mode int var isDir bool @@ -278,7 +271,6 @@ func (medium *Medium) Rename(oldPath, newPath string) error { return core.E("sqlite.Rename", core.Concat("query failed: ", oldKey), err) } - // Insert or replace at new path _, err = tx.Exec( `INSERT INTO `+medium.table+` (path, content, mode, is_dir, mtime) VALUES (?, ?, ?, ?, ?) ON CONFLICT(path) DO UPDATE SET content = excluded.content, mode = excluded.mode, is_dir = excluded.is_dir, mtime = excluded.mtime`, @@ -288,13 +280,11 @@ func (medium *Medium) Rename(oldPath, newPath string) error { return core.E("sqlite.Rename", core.Concat("insert at new path failed: ", newKey), err) } - // Delete old path _, err = tx.Exec(`DELETE FROM `+medium.table+` WHERE path = ?`, oldKey) if err != nil { return core.E("sqlite.Rename", core.Concat("delete old path failed: ", oldKey), err) } - // If it's a directory, move all children if isDir { oldPrefix := oldKey + "/" newPrefix := newKey + "/" @@ -337,7 +327,6 @@ func (medium *Medium) Rename(oldPath, newPath string) error { } } - // Delete old children _, err = tx.Exec(`DELETE FROM `+medium.table+` WHERE path LIKE ?`, oldPrefix+"%") if err != nil { return core.E("sqlite.Rename", "delete old children failed", err) @@ -354,7 +343,6 @@ func (medium *Medium) List(filePath string) ([]fs.DirEntry, error) { prefix += "/" } - // Query all paths under the prefix rows, err := medium.database.Query( `SELECT path, content, mode, is_dir, mtime FROM `+medium.table+` WHERE path LIKE ? OR path LIKE ?`, prefix+"%", prefix+"%", @@ -382,10 +370,8 @@ func (medium *Medium) List(filePath string) ([]fs.DirEntry, error) { continue } - // Check if this is a direct child or nested parts := core.SplitN(rest, "/", 2) if len(parts) == 2 { - // Nested - register as a directory dirName := parts[0] if !seen[dirName] { seen[dirName] = true @@ -401,7 +387,6 @@ func (medium *Medium) List(filePath string) ([]fs.DirEntry, error) { }) } } else { - // Direct child if !seen[rest] { seen[rest] = true entries = append(entries, &dirEntry{ @@ -550,7 +535,6 @@ func (medium *Medium) WriteStream(filePath string) (goio.WriteCloser, error) { func (medium *Medium) Exists(filePath string) bool { key := normaliseEntryPath(filePath) if key == "" { - // Root always exists return true } diff --git a/sqlite/sqlite_test.go b/sqlite/sqlite_test.go index 99a521b..dbe4f30 100644 --- a/sqlite/sqlite_test.go +++ b/sqlite/sqlite_test.go @@ -116,7 +116,6 @@ func TestSqlite_EnsureDir_Good(t *testing.T) { func TestSqlite_EnsureDir_EmptyPath_Good(t *testing.T) { m := newTestMedium(t) - // Root always exists, no-op err := m.EnsureDir("") assert.NoError(t, err) } @@ -579,7 +578,6 @@ func TestSqlite_Exists_Good(t *testing.T) { func TestSqlite_Exists_EmptyPath_Good(t *testing.T) { m := newTestMedium(t) - // Root always exists assert.True(t, m.Exists("")) } diff --git a/workspace/service.go b/workspace/service.go index a53a18a..8b767a3 100644 --- a/workspace/service.go +++ b/workspace/service.go @@ -99,9 +99,9 @@ func (service *Service) CreateWorkspace(identifier, password string) (string, er return "", core.E("workspace.CreateWorkspace", "workspace already exists", nil) } - for _, d := range []string{"config", "log", "data", "files", "keys"} { - if err := service.medium.EnsureDir(core.Path(workspaceDirectory, d)); err != nil { - return "", core.E("workspace.CreateWorkspace", core.Concat("failed to create directory: ", d), err) + for _, directoryName := range []string{"config", "log", "data", "files", "keys"} { + if err := service.medium.EnsureDir(core.Path(workspaceDirectory, directoryName)); err != nil { + return "", core.E("workspace.CreateWorkspace", core.Concat("failed to create directory: ", directoryName), err) } } diff --git a/workspace/service_test.go b/workspace/service_test.go index c707754..d218cd0 100644 --- a/workspace/service_test.go +++ b/workspace/service_test.go @@ -13,11 +13,11 @@ type stubCryptProvider struct { err error } -func (s stubCryptProvider) CreateKeyPair(_, _ string) (string, error) { - if s.err != nil { - return "", s.err +func (provider stubCryptProvider) CreateKeyPair(_, _ string) (string, error) { + if provider.err != nil { + return "", provider.err } - return s.key, nil + return provider.key, nil } func newTestService(t *testing.T) (*Service, string) { @@ -26,9 +26,9 @@ func newTestService(t *testing.T) (*Service, string) { tempHome := t.TempDir() t.Setenv("HOME", tempHome) - svc, err := New(Options{CryptProvider: stubCryptProvider{key: "private-key"}}) + service, err := New(Options{CryptProvider: stubCryptProvider{key: "private-key"}}) require.NoError(t, err) - return svc, tempHome + return service, tempHome } func TestService_New_MissingCryptProvider_Bad(t *testing.T) { @@ -37,9 +37,9 @@ func TestService_New_MissingCryptProvider_Bad(t *testing.T) { } func TestService_Workspace_RoundTrip_Good(t *testing.T) { - s, tempHome := newTestService(t) + service, tempHome := newTestService(t) - workspaceID, err := s.CreateWorkspace("test-user", "pass123") + workspaceID, err := service.CreateWorkspace("test-user", "pass123") require.NoError(t, err) assert.NotEmpty(t, workspaceID) @@ -48,55 +48,55 @@ func TestService_Workspace_RoundTrip_Good(t *testing.T) { assert.DirExists(t, core.Path(workspacePath, "keys")) assert.FileExists(t, core.Path(workspacePath, "keys", "private.key")) - err = s.SwitchWorkspace(workspaceID) + err = service.SwitchWorkspace(workspaceID) require.NoError(t, err) - assert.Equal(t, workspaceID, s.activeWorkspaceID) + assert.Equal(t, workspaceID, service.activeWorkspaceID) - err = s.WorkspaceFileSet("secret.txt", "top secret info") + err = service.WorkspaceFileSet("secret.txt", "top secret info") require.NoError(t, err) - got, err := s.WorkspaceFileGet("secret.txt") + got, err := service.WorkspaceFileGet("secret.txt") require.NoError(t, err) assert.Equal(t, "top secret info", got) } func TestService_SwitchWorkspace_TraversalBlocked_Bad(t *testing.T) { - s, tempHome := newTestService(t) + service, tempHome := newTestService(t) outside := core.Path(tempHome, ".core", "escaped") - require.NoError(t, s.medium.EnsureDir(outside)) + require.NoError(t, service.medium.EnsureDir(outside)) - err := s.SwitchWorkspace("../escaped") + err := service.SwitchWorkspace("../escaped") require.Error(t, err) - assert.Empty(t, s.activeWorkspaceID) + assert.Empty(t, service.activeWorkspaceID) } func TestService_WorkspaceFileSet_TraversalBlocked_Bad(t *testing.T) { - s, tempHome := newTestService(t) + service, tempHome := newTestService(t) - workspaceID, err := s.CreateWorkspace("test-user", "pass123") + workspaceID, err := service.CreateWorkspace("test-user", "pass123") require.NoError(t, err) - require.NoError(t, s.SwitchWorkspace(workspaceID)) + require.NoError(t, service.SwitchWorkspace(workspaceID)) keyPath := core.Path(tempHome, ".core", "workspaces", workspaceID, "keys", "private.key") - before, err := s.medium.Read(keyPath) + before, err := service.medium.Read(keyPath) require.NoError(t, err) - err = s.WorkspaceFileSet("../keys/private.key", "hijack") + err = service.WorkspaceFileSet("../keys/private.key", "hijack") require.Error(t, err) - after, err := s.medium.Read(keyPath) + after, err := service.medium.Read(keyPath) require.NoError(t, err) assert.Equal(t, before, after) - _, err = s.WorkspaceFileGet("../keys/private.key") + _, err = service.WorkspaceFileGet("../keys/private.key") require.Error(t, err) } func TestService_HandleWorkspaceMessage_Good(t *testing.T) { - s, _ := newTestService(t) + service, _ := newTestService(t) - create := s.HandleWorkspaceMessage(core.New(), WorkspaceCommand{ + create := service.HandleWorkspaceMessage(core.New(), WorkspaceCommand{ Action: WorkspaceCreateAction, Identifier: "ipc-user", Password: "pass123", @@ -107,14 +107,14 @@ func TestService_HandleWorkspaceMessage_Good(t *testing.T) { require.True(t, ok) require.NotEmpty(t, workspaceID) - switchResult := s.HandleWorkspaceMessage(core.New(), WorkspaceCommand{ + switchResult := service.HandleWorkspaceMessage(core.New(), WorkspaceCommand{ Action: WorkspaceSwitchAction, WorkspaceID: workspaceID, }) assert.True(t, switchResult.OK) - assert.Equal(t, workspaceID, s.activeWorkspaceID) + assert.Equal(t, workspaceID, service.activeWorkspaceID) - legacyCreate := s.HandleWorkspaceMessage(core.New(), map[string]any{ + legacyCreate := service.HandleWorkspaceMessage(core.New(), map[string]any{ "action": WorkspaceCreateAction, "identifier": "legacy-user", "password": "pass123", @@ -125,34 +125,34 @@ func TestService_HandleWorkspaceMessage_Good(t *testing.T) { require.True(t, ok) require.NotEmpty(t, legacyWorkspaceID) - legacySwitch := s.HandleWorkspaceMessage(core.New(), WorkspaceCommand{ + legacySwitch := service.HandleWorkspaceMessage(core.New(), WorkspaceCommand{ Action: WorkspaceSwitchAction, WorkspaceID: legacyWorkspaceID, }) assert.True(t, legacySwitch.OK) - assert.Equal(t, legacyWorkspaceID, s.activeWorkspaceID) + assert.Equal(t, legacyWorkspaceID, service.activeWorkspaceID) - rejectedLegacySwitch := s.HandleWorkspaceMessage(core.New(), map[string]any{ + rejectedLegacySwitch := service.HandleWorkspaceMessage(core.New(), map[string]any{ "action": WorkspaceSwitchAction, "name": workspaceID, }) assert.False(t, rejectedLegacySwitch.OK) - assert.Equal(t, legacyWorkspaceID, s.activeWorkspaceID) + assert.Equal(t, legacyWorkspaceID, service.activeWorkspaceID) - failedSwitch := s.HandleWorkspaceMessage(core.New(), map[string]any{ + failedSwitch := service.HandleWorkspaceMessage(core.New(), map[string]any{ "action": WorkspaceSwitchAction, "workspaceID": "missing", }) assert.False(t, failedSwitch.OK) - unknown := s.HandleWorkspaceMessage(core.New(), "noop") + unknown := service.HandleWorkspaceMessage(core.New(), "noop") assert.True(t, unknown.OK) } func TestService_HandleIPCEvents_Compatibility_Good(t *testing.T) { - s, _ := newTestService(t) + service, _ := newTestService(t) - result := s.HandleIPCEvents(core.New(), WorkspaceCommand{ + result := service.HandleIPCEvents(core.New(), WorkspaceCommand{ Action: WorkspaceCreateAction, Identifier: "compat-user", Password: "pass123",