diff --git a/pkg/provider/dir.go b/pkg/provider/dir.go index b37fd9d92..1a922c8a1 100644 --- a/pkg/provider/dir.go +++ b/pkg/provider/dir.go @@ -192,9 +192,10 @@ func SaveWorkspaceResult(workspace *Workspace, result *config2.Result) error { return err } - // #nosec G301 -- TODO Consider using a more secure permission setting and ownership if needed. - err = os.MkdirAll(workspaceDir, 0o755) - if err != nil { + if _, err := os.Stat(filepath.Join(workspaceDir, WorkspaceConfigFile)); err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("workspace %s no longer exists", workspace.ID) + } return err } diff --git a/pkg/provider/dir_test.go b/pkg/provider/dir_test.go new file mode 100644 index 000000000..4dc47fb2e --- /dev/null +++ b/pkg/provider/dir_test.go @@ -0,0 +1,46 @@ +package provider + +import ( + "os" + "path/filepath" + "testing" + + "github.com/devsy-org/devsy/pkg/config" + devcontainerconfig "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/stretchr/testify/require" +) + +func setupTestHome(t *testing.T) { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + config.ResetPathManager() + t.Cleanup(config.ResetPathManager) +} + +func TestSaveWorkspaceResult_RefusesWithoutConfig(t *testing.T) { + setupTestHome(t) + + ws := &Workspace{ID: "ghost", Context: config.DefaultContext} + err := SaveWorkspaceResult(ws, &devcontainerconfig.Result{}) + require.Error(t, err) + + // Must not leave an orphan dir behind. + dir, dirErr := GetWorkspaceDir(ws.Context, ws.ID) + require.NoError(t, dirErr) + require.NoDirExists(t, dir) +} + +func TestSaveWorkspaceResult_WritesWhenConfigExists(t *testing.T) { + setupTestHome(t) + + ws := &Workspace{ID: "real", Context: config.DefaultContext} + require.NoError(t, SaveWorkspaceConfig(ws)) + require.NoError(t, SaveWorkspaceResult(ws, &devcontainerconfig.Result{})) + + dir, err := GetWorkspaceDir(ws.Context, ws.ID) + require.NoError(t, err) + _, err = os.Stat(filepath.Join(dir, WorkspaceResultFile)) + require.NoError(t, err) +} diff --git a/pkg/workspace/delete.go b/pkg/workspace/delete.go index 223c1bc0b..06c9c5a2e 100644 --- a/pkg/workspace/delete.go +++ b/pkg/workspace/delete.go @@ -3,6 +3,9 @@ package workspace import ( "context" "fmt" + "os" + "path/filepath" + "strings" client2 "github.com/devsy-org/devsy/pkg/client" "github.com/devsy-org/devsy/pkg/client/clientimplementation" @@ -10,6 +13,7 @@ import ( "github.com/devsy-org/devsy/pkg/ide/opener" "github.com/devsy-org/devsy/pkg/log" "github.com/devsy-org/devsy/pkg/platform" + providerpkg "github.com/devsy-org/devsy/pkg/provider" ) // DeleteOptions holds the parameters for deleting a workspace. @@ -51,7 +55,11 @@ func Delete(ctx context.Context, opts DeleteOptions) (string, error) { stopIfRunning(ctx, client, status) - return deleteWorkspace(ctx, client, opts) + id, err := deleteWorkspace(ctx, client, opts) + if err == nil { + SweepOrphanWorkspaceDirs(opts.DevsyConfig.DefaultContext) + } + return id, err } // stopIfRunning stops the workspace before deletion when it is currently @@ -308,3 +316,40 @@ func hasOtherWorkspaces( return false, nil } + +func SweepOrphanWorkspaceDirs(contextName string) { + workspaceDir, err := providerpkg.GetWorkspacesDir(contextName) + if err != nil { + log.Debugf("sweep orphan workspaces: get dir: %v", err) + return + } + + entries, err := os.ReadDir(workspaceDir) + if err != nil { + if !os.IsNotExist(err) { + log.Debugf("sweep orphan workspaces: read dir: %v", err) + } + return + } + + for _, entry := range entries { + removeIfOrphan(workspaceDir, entry) + } +} + +func removeIfOrphan(workspaceDir string, entry os.DirEntry) { + if !entry.IsDir() || strings.HasPrefix(entry.Name(), ".") { + return + } + + configPath := filepath.Join(workspaceDir, entry.Name(), providerpkg.WorkspaceConfigFile) + if _, err := os.Stat(configPath); err == nil || !os.IsNotExist(err) { + return + } + + if err := os.RemoveAll(filepath.Join(workspaceDir, entry.Name())); err != nil { + log.Warnf("remove orphan workspace dir %s: %v", entry.Name(), err) + return + } + log.Debugf("removed orphan workspace dir without config: workspace=%s", entry.Name()) +} diff --git a/pkg/workspace/delete_test.go b/pkg/workspace/delete_test.go new file mode 100644 index 000000000..8677939c3 --- /dev/null +++ b/pkg/workspace/delete_test.go @@ -0,0 +1,44 @@ +package workspace + +import ( + "os" + "path/filepath" + "testing" + + "github.com/devsy-org/devsy/pkg/provider" + "github.com/stretchr/testify/require" +) + +func TestSweepOrphanWorkspaceDirs(t *testing.T) { + setupTestPathManager(t) + + require.NoError(t, provider.SaveWorkspaceConfig( + &provider.Workspace{ID: "healthy", Context: testDefaultContext}, + )) + + workspacesDir, err := provider.GetWorkspacesDir(testDefaultContext) + require.NoError(t, err) + + // Orphan dir with only auxiliary state, plus a dotfile that must survive. + orphanDir := filepath.Join(workspacesDir, "orphan") + require.NoError(t, os.MkdirAll(filepath.Join(orphanDir, "logs"), 0o750)) + require.NoError(t, os.WriteFile( + filepath.Join(orphanDir, provider.WorkspaceResultFile), []byte("{}"), 0o600, + )) + dotDir := filepath.Join(workspacesDir, ".keep") + require.NoError(t, os.MkdirAll(dotDir, 0o750)) + + SweepOrphanWorkspaceDirs(testDefaultContext) + + healthyDir := filepath.Join(workspacesDir, "healthy") + require.DirExists(t, healthyDir) + require.NoDirExists(t, orphanDir) + require.DirExists(t, dotDir) +} + +func TestSweepOrphanWorkspaceDirs_MissingDirIsNoop(t *testing.T) { + setupTestPathManager(t) + + // No workspaces dir created yet — sweep must not panic or error. + SweepOrphanWorkspaceDirs(testDefaultContext) +} diff --git a/pkg/workspace/rename_integration_test.go b/pkg/workspace/rename_integration_test.go index 37d13f0d6..1d98ef244 100644 --- a/pkg/workspace/rename_integration_test.go +++ b/pkg/workspace/rename_integration_test.go @@ -40,6 +40,7 @@ func writeWorkspaceResult( t.Helper() ws := &provider.Workspace{ID: workspaceID, Context: testDefaultContext} + require.NoError(t, provider.SaveWorkspaceConfig(ws)) require.NoError(t, provider.SaveWorkspaceResult(ws, result)) } @@ -353,7 +354,9 @@ func TestUpdateWorkspaceResult_RawJSON(t *testing.T) { wsDir, err := provider.GetWorkspaceDir(testDefaultContext, newName) require.NoError(t, err) require.NoError(t, os.MkdirAll(wsDir, 0o750)) - + require.NoError(t, provider.SaveWorkspaceConfig( + &provider.Workspace{ID: newName, Context: testDefaultContext}, + )) rawJSON := `{ "SubstitutionContext": { "ContainerWorkspaceFolder": "/workspaces/old-ws",