From a9abdcb9e74ab9d328b5c0b41dfdcc8155aee218 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Fri, 28 Feb 2025 03:14:18 +0000 Subject: [PATCH 01/11] chore: add agent endpoint for querying file system --- agent/api.go | 1 + agent/ls.go | 136 ++++++++++++++++++++++++++++++ agent/ls_internal_test.go | 172 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 309 insertions(+) create mode 100644 agent/ls.go create mode 100644 agent/ls_internal_test.go diff --git a/agent/api.go b/agent/api.go index a3241feb3b7ee..05ed760219a6a 100644 --- a/agent/api.go +++ b/agent/api.go @@ -41,6 +41,7 @@ func (a *agent) apiHandler() http.Handler { r.Get("/api/v0/containers", ch.ServeHTTP) r.Get("/api/v0/listening-ports", lp.handler) r.Get("/api/v0/netcheck", a.HandleNetcheck) + r.Post("/api/v0/ls", a.HandleLS) r.Get("/debug/logs", a.HandleHTTPDebugLogs) r.Get("/debug/magicsock", a.HandleHTTPDebugMagicsock) r.Get("/debug/magicsock/debug-logging/{state}", a.HandleHTTPMagicsockDebugLoggingState) diff --git a/agent/ls.go b/agent/ls.go new file mode 100644 index 0000000000000..2485d8e99d31a --- /dev/null +++ b/agent/ls.go @@ -0,0 +1,136 @@ +package agent + +import ( + "errors" + "net/http" + "os" + "path/filepath" + "runtime" + + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/codersdk" +) + +func (*agent) HandleLS(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + var query LSQuery + if !httpapi.Read(ctx, rw, r, &query) { + return + } + + resp, err := listFiles(query) + if err != nil { + switch { + case errors.Is(err, os.ErrNotExist): + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: "Directory does not exist", + }) + case errors.Is(err, os.ErrPermission): + httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ + Message: "Permission denied", + }) + default: + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: err.Error(), + }) + } + return + } + + httpapi.Write(ctx, rw, http.StatusOK, resp) +} + +func listFiles(query LSQuery) (LSResponse, error) { + var base string + switch query.Relativity { + case LSRelativityHome: + home, err := os.UserHomeDir() + if err != nil { + return LSResponse{}, xerrors.Errorf("failed to get user home directory: %w", err) + } + base = home + case LSRelativityRoot: + if runtime.GOOS == "windows" { + // TODO: Eventually, we could have a empty path with a root base + // return all drives. + // C drive should be good enough for now. + base = "C:\\" + } else { + base = "/" + } + default: + return LSResponse{}, xerrors.Errorf("unsupported relativity type %q", query.Relativity) + } + + fullPath := append([]string{base}, query.Path...) + absolutePathString, err := filepath.Abs(filepath.Join(fullPath...)) + if err != nil { + return LSResponse{}, xerrors.Errorf("failed to get absolute path: %w", err) + } + + f, err := os.Open(absolutePathString) + if err != nil { + return LSResponse{}, xerrors.Errorf("failed to open directory: %w", err) + } + defer f.Close() + + stat, err := f.Stat() + if err != nil { + return LSResponse{}, xerrors.Errorf("failed to stat directory: %w", err) + } + + if !stat.IsDir() { + return LSResponse{}, xerrors.New("path is not a directory") + } + + // `contents` may be partially populated even if the operation fails midway. + contents, _ := f.Readdir(-1) + respContents := make([]LSFile, 0, len(contents)) + for _, file := range contents { + respContents = append(respContents, LSFile{ + Name: file.Name(), + AbsolutePathString: filepath.Join(absolutePathString, file.Name()), + IsDir: file.IsDir(), + }) + } + + return LSResponse{ + AbsolutePathString: absolutePathString, + Contents: respContents, + }, nil +} + +type LSQuery struct { + // e.g. [], ["repos", "coder"], + Path []string `json:"path"` + // Whether the supplied path is relative to the user's home directory, + // or the root directory. + Relativity LSRelativity `json:"relativity"` +} + +type LSResponse struct { + // Returned so clients can display the full path to the user, and + // copy it to configure file sync + // e.g. Windows: "C:\\Users\\coder" + // Linux: "/home/coder" + AbsolutePathString string `json:"absolute_path_string"` + Contents []LSFile `json:"contents"` +} + +type LSFile struct { + Name string `json:"name"` + // e.g. "C:\\Users\\coder\\hello.txt" + // "/home/coder/hello.txt" + AbsolutePathString string `json:"absolute_path_string"` + IsDir bool `json:"is_dir"` +} + +type LSRelativity string + +const ( + LSRelativityRoot LSRelativity = "root" + LSRelativityHome LSRelativity = "home" +) diff --git a/agent/ls_internal_test.go b/agent/ls_internal_test.go new file mode 100644 index 0000000000000..846365d45f3b2 --- /dev/null +++ b/agent/ls_internal_test.go @@ -0,0 +1,172 @@ +package agent + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestListFilesNonExistentDirectory(t *testing.T) { + t.Parallel() + + query := LSQuery{ + Path: []string{"idontexist"}, + Relativity: LSRelativityHome, + } + _, err := listFiles(query) + require.ErrorIs(t, err, os.ErrNotExist) +} + +func TestListFilesPermissionDenied(t *testing.T) { + t.Parallel() + + if runtime.GOOS == "windows" { + t.Skip("creating an unreadable-by-user directory is non-trivial on Windows") + } + + home, err := os.UserHomeDir() + require.NoError(t, err) + + tmpDir := t.TempDir() + + reposDir := filepath.Join(tmpDir, "repos") + err = os.Mkdir(reposDir, 0o000) + require.NoError(t, err) + + rel, err := filepath.Rel(home, reposDir) + require.NoError(t, err) + + query := LSQuery{ + Path: pathToArray(rel), + Relativity: LSRelativityHome, + } + _, err = listFiles(query) + require.ErrorIs(t, err, os.ErrPermission) +} + +func TestListFilesNotADirectory(t *testing.T) { + t.Parallel() + + home, err := os.UserHomeDir() + require.NoError(t, err) + + tmpDir := t.TempDir() + + filePath := filepath.Join(tmpDir, "file.txt") + err = os.WriteFile(filePath, []byte("content"), 0o600) + require.NoError(t, err) + + rel, err := filepath.Rel(home, filePath) + require.NoError(t, err) + + query := LSQuery{ + Path: pathToArray(rel), + Relativity: LSRelativityHome, + } + _, err = listFiles(query) + require.ErrorContains(t, err, "path is not a directory") +} + +func TestListFilesSuccess(t *testing.T) { + t.Parallel() + + tc := []struct { + name string + baseFunc func(t *testing.T) string + relativity LSRelativity + }{ + { + name: "home", + baseFunc: func(t *testing.T) string { + home, err := os.UserHomeDir() + require.NoError(t, err) + return home + }, + relativity: LSRelativityHome, + }, + { + name: "root", + baseFunc: func(t *testing.T) string { + if runtime.GOOS == "windows" { + return "C:\\" + } + return "/" + }, + relativity: LSRelativityRoot, + }, + } + + // nolint:paralleltest // Not since Go v1.22. + for _, tc := range tc { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + base := tc.baseFunc(t) + tmpDir := t.TempDir() + + reposDir := filepath.Join(tmpDir, "repos") + err := os.Mkdir(reposDir, 0o755) + require.NoError(t, err) + + downloadsDir := filepath.Join(tmpDir, "Downloads") + err = os.Mkdir(downloadsDir, 0o755) + require.NoError(t, err) + + rel, err := filepath.Rel(base, tmpDir) + require.NoError(t, err) + relComponents := pathToArray(rel) + + query := LSQuery{ + Path: relComponents, + Relativity: tc.relativity, + } + resp, err := listFiles(query) + require.NoError(t, err) + + require.Equal(t, tmpDir, resp.AbsolutePathString) + + var foundRepos, foundDownloads bool + for _, file := range resp.Contents { + switch file.Name { + case "repos": + foundRepos = true + expectedPath := filepath.Join(tmpDir, "repos") + require.Equal(t, expectedPath, file.AbsolutePathString) + require.True(t, file.IsDir) + case "Downloads": + foundDownloads = true + expectedPath := filepath.Join(tmpDir, "Downloads") + require.Equal(t, expectedPath, file.AbsolutePathString) + require.True(t, file.IsDir) + } + } + require.True(t, foundRepos && foundDownloads, "expected to find both repos and Downloads directories, got: %+v", resp.Contents) + }) + } +} + +func TestListFilesWindowsRoot(t *testing.T) { + t.Parallel() + + if runtime.GOOS != "windows" { + t.Skip("skipping test on non-Windows OS") + } + + query := LSQuery{ + Path: []string{}, + Relativity: LSRelativityRoot, + } + resp, err := listFiles(query) + require.NoError(t, err) + require.Equal(t, "C:\\", resp.AbsolutePathString) +} + +func pathToArray(path string) []string { + return strings.FieldsFunc(path, func(r rune) bool { + return r == os.PathSeparator + }) +} From 4c4adc89ecc4e1577cf289a8e38491e7b3d25dca Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Fri, 28 Feb 2025 05:36:24 +0000 Subject: [PATCH 02/11] windows multi-drive, add absolutepath --- agent/ls.go | 58 ++++++++++++++++++++++++++++++++------- agent/ls_internal_test.go | 45 ++++++++++++++++++++---------- go.mod | 2 +- 3 files changed, 80 insertions(+), 25 deletions(-) diff --git a/agent/ls.go b/agent/ls.go index 2485d8e99d31a..7bde9894d2c02 100644 --- a/agent/ls.go +++ b/agent/ls.go @@ -6,7 +6,9 @@ import ( "os" "path/filepath" "runtime" + "strings" + "github.com/shirou/gopsutil/v3/disk" "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/httpapi" @@ -26,11 +28,11 @@ func (*agent) HandleLS(rw http.ResponseWriter, r *http.Request) { switch { case errors.Is(err, os.ErrNotExist): httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ - Message: "Directory does not exist", + Message: err.Error(), }) case errors.Is(err, os.ErrPermission): httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ - Message: "Permission denied", + Message: err.Error(), }) default: httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -44,28 +46,27 @@ func (*agent) HandleLS(rw http.ResponseWriter, r *http.Request) { } func listFiles(query LSQuery) (LSResponse, error) { - var base string + var fullPath []string switch query.Relativity { case LSRelativityHome: home, err := os.UserHomeDir() if err != nil { return LSResponse{}, xerrors.Errorf("failed to get user home directory: %w", err) } - base = home + fullPath = []string{home} case LSRelativityRoot: if runtime.GOOS == "windows" { - // TODO: Eventually, we could have a empty path with a root base - // return all drives. - // C drive should be good enough for now. - base = "C:\\" + if len(query.Path) == 0 { + return listDrives() + } } else { - base = "/" + fullPath = []string{"/"} } default: return LSResponse{}, xerrors.Errorf("unsupported relativity type %q", query.Relativity) } - fullPath := append([]string{base}, query.Path...) + fullPath = append(fullPath, query.Path...) absolutePathString, err := filepath.Abs(filepath.Join(fullPath...)) if err != nil { return LSResponse{}, xerrors.Errorf("failed to get absolute path: %w", err) @@ -97,12 +98,48 @@ func listFiles(query LSQuery) (LSResponse, error) { }) } + absolutePath := pathToArray(absolutePathString) + return LSResponse{ + AbsolutePath: absolutePath, AbsolutePathString: absolutePathString, Contents: respContents, }, nil } +func listDrives() (LSResponse, error) { + aa, err := disk.Partitions(true) + if err != nil { + return LSResponse{}, xerrors.Errorf("failed to get partitions: %w", err) + } + contents := make([]LSFile, 0, len(aa)) + for _, a := range aa { + name := a.Mountpoint + string(os.PathSeparator) + contents = append(contents, LSFile{ + Name: name, + AbsolutePathString: name, + IsDir: true, + }) + } + + return LSResponse{ + AbsolutePath: []string{}, + AbsolutePathString: "", + Contents: contents, + }, nil +} + +func pathToArray(path string) []string { + out := strings.FieldsFunc(path, func(r rune) bool { + return r == os.PathSeparator + }) + // Drive letters on Windows should have a trailing separator. + if runtime.GOOS == "windows" && len(out) > 0 { + out[0] += string(os.PathSeparator) + } + return out +} + type LSQuery struct { // e.g. [], ["repos", "coder"], Path []string `json:"path"` @@ -112,6 +149,7 @@ type LSQuery struct { } type LSResponse struct { + AbsolutePath []string `json:"absolute_path"` // Returned so clients can display the full path to the user, and // copy it to configure file sync // e.g. Windows: "C:\\Users\\coder" diff --git a/agent/ls_internal_test.go b/agent/ls_internal_test.go index 846365d45f3b2..3d28bf658969c 100644 --- a/agent/ls_internal_test.go +++ b/agent/ls_internal_test.go @@ -4,7 +4,6 @@ import ( "os" "path/filepath" "runtime" - "strings" "testing" "github.com/stretchr/testify/require" @@ -90,9 +89,9 @@ func TestListFilesSuccess(t *testing.T) { }, { name: "root", - baseFunc: func(t *testing.T) string { + baseFunc: func(*testing.T) string { if runtime.GOOS == "windows" { - return "C:\\" + return "" } return "/" }, @@ -116,12 +115,18 @@ func TestListFilesSuccess(t *testing.T) { err = os.Mkdir(downloadsDir, 0o755) require.NoError(t, err) - rel, err := filepath.Rel(base, tmpDir) - require.NoError(t, err) - relComponents := pathToArray(rel) + var queryComponents []string + // We can't get an absolute path relative to empty string on Windows. + if runtime.GOOS == "windows" && base == "" { + queryComponents = pathToArray(tmpDir) + } else { + rel, err := filepath.Rel(base, tmpDir) + require.NoError(t, err) + queryComponents = pathToArray(rel) + } query := LSQuery{ - Path: relComponents, + Path: queryComponents, Relativity: tc.relativity, } resp, err := listFiles(query) @@ -149,7 +154,7 @@ func TestListFilesSuccess(t *testing.T) { } } -func TestListFilesWindowsRoot(t *testing.T) { +func TestListFilesListDrives(t *testing.T) { t.Parallel() if runtime.GOOS != "windows" { @@ -162,11 +167,23 @@ func TestListFilesWindowsRoot(t *testing.T) { } resp, err := listFiles(query) require.NoError(t, err) - require.Equal(t, "C:\\", resp.AbsolutePathString) -} - -func pathToArray(path string) []string { - return strings.FieldsFunc(path, func(r rune) bool { - return r == os.PathSeparator + require.Contains(t, resp.Contents, LSFile{ + Name: "C:\\", + AbsolutePathString: "C:\\", + IsDir: true, }) + + query = LSQuery{ + Path: []string{"C:\\"}, + Relativity: LSRelativityRoot, + } + resp, err = listFiles(query) + require.NoError(t, err) + + query = LSQuery{ + Path: resp.AbsolutePath, + Relativity: LSRelativityRoot, + } + resp, err = listFiles(query) + require.NoError(t, err) } diff --git a/go.mod b/go.mod index 4b38c65265f4d..cdc1eefd59f38 100644 --- a/go.mod +++ b/go.mod @@ -401,7 +401,7 @@ require ( github.com/ryanuber/go-glob v1.0.0 // indirect github.com/satori/go.uuid v1.2.1-0.20181028125025-b2ce2384e17b // indirect github.com/secure-systems-lab/go-securesystemslib v0.7.0 // indirect - github.com/shirou/gopsutil/v3 v3.24.4 // indirect + github.com/shirou/gopsutil/v3 v3.24.4 github.com/shoenig/go-m1cpu v0.1.6 // indirect github.com/sirupsen/logrus v1.9.3 // indirect github.com/spaolacci/murmur3 v1.1.0 // indirect From d644fbd4e0fdf6c9fd4004104bc3e030fc9a34a4 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Fri, 28 Feb 2025 06:04:36 +0000 Subject: [PATCH 03/11] dean review --- agent/ls.go | 19 ++++++++++--------- agent/ls_internal_test.go | 16 ++++++++-------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/agent/ls.go b/agent/ls.go index 7bde9894d2c02..db1717ab7da79 100644 --- a/agent/ls.go +++ b/agent/ls.go @@ -18,7 +18,7 @@ import ( func (*agent) HandleLS(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() - var query LSQuery + var query LSRequest if !httpapi.Read(ctx, rw, r, &query) { return } @@ -45,7 +45,7 @@ func (*agent) HandleLS(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, resp) } -func listFiles(query LSQuery) (LSResponse, error) { +func listFiles(query LSRequest) (LSResponse, error) { var fullPath []string switch query.Relativity { case LSRelativityHome: @@ -67,28 +67,29 @@ func listFiles(query LSQuery) (LSResponse, error) { } fullPath = append(fullPath, query.Path...) - absolutePathString, err := filepath.Abs(filepath.Join(fullPath...)) + fullPathRelative := filepath.Join(fullPath...) + absolutePathString, err := filepath.Abs(fullPathRelative) if err != nil { - return LSResponse{}, xerrors.Errorf("failed to get absolute path: %w", err) + return LSResponse{}, xerrors.Errorf("failed to get absolute path of %q: %w", fullPathRelative, err) } f, err := os.Open(absolutePathString) if err != nil { - return LSResponse{}, xerrors.Errorf("failed to open directory: %w", err) + return LSResponse{}, xerrors.Errorf("failed to open directory %q: %w", absolutePathString, err) } defer f.Close() stat, err := f.Stat() if err != nil { - return LSResponse{}, xerrors.Errorf("failed to stat directory: %w", err) + return LSResponse{}, xerrors.Errorf("failed to stat directory %q: %w", absolutePathString, err) } if !stat.IsDir() { - return LSResponse{}, xerrors.New("path is not a directory") + return LSResponse{}, xerrors.Errorf("path %q is not a directory", absolutePathString) } // `contents` may be partially populated even if the operation fails midway. - contents, _ := f.Readdir(-1) + contents, _ := f.ReadDir(-1) respContents := make([]LSFile, 0, len(contents)) for _, file := range contents { respContents = append(respContents, LSFile{ @@ -140,7 +141,7 @@ func pathToArray(path string) []string { return out } -type LSQuery struct { +type LSRequest struct { // e.g. [], ["repos", "coder"], Path []string `json:"path"` // Whether the supplied path is relative to the user's home directory, diff --git a/agent/ls_internal_test.go b/agent/ls_internal_test.go index 3d28bf658969c..133fd0709695e 100644 --- a/agent/ls_internal_test.go +++ b/agent/ls_internal_test.go @@ -12,7 +12,7 @@ import ( func TestListFilesNonExistentDirectory(t *testing.T) { t.Parallel() - query := LSQuery{ + query := LSRequest{ Path: []string{"idontexist"}, Relativity: LSRelativityHome, } @@ -39,7 +39,7 @@ func TestListFilesPermissionDenied(t *testing.T) { rel, err := filepath.Rel(home, reposDir) require.NoError(t, err) - query := LSQuery{ + query := LSRequest{ Path: pathToArray(rel), Relativity: LSRelativityHome, } @@ -62,12 +62,12 @@ func TestListFilesNotADirectory(t *testing.T) { rel, err := filepath.Rel(home, filePath) require.NoError(t, err) - query := LSQuery{ + query := LSRequest{ Path: pathToArray(rel), Relativity: LSRelativityHome, } _, err = listFiles(query) - require.ErrorContains(t, err, "path is not a directory") + require.ErrorContains(t, err, "is not a directory") } func TestListFilesSuccess(t *testing.T) { @@ -125,7 +125,7 @@ func TestListFilesSuccess(t *testing.T) { queryComponents = pathToArray(rel) } - query := LSQuery{ + query := LSRequest{ Path: queryComponents, Relativity: tc.relativity, } @@ -161,7 +161,7 @@ func TestListFilesListDrives(t *testing.T) { t.Skip("skipping test on non-Windows OS") } - query := LSQuery{ + query := LSRequest{ Path: []string{}, Relativity: LSRelativityRoot, } @@ -173,14 +173,14 @@ func TestListFilesListDrives(t *testing.T) { IsDir: true, }) - query = LSQuery{ + query = LSRequest{ Path: []string{"C:\\"}, Relativity: LSRelativityRoot, } resp, err = listFiles(query) require.NoError(t, err) - query = LSQuery{ + query = LSRequest{ Path: resp.AbsolutePath, Relativity: LSRelativityRoot, } From 81fca3b04245ff569e6f3318e2ac6b02c88d7556 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Fri, 28 Feb 2025 06:29:37 +0000 Subject: [PATCH 04/11] go mod tidy --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index cdc1eefd59f38..b9170e3ff5055 100644 --- a/go.mod +++ b/go.mod @@ -164,6 +164,7 @@ require ( github.com/prometheus/common v0.62.0 github.com/quasilyte/go-ruleguard/dsl v0.3.21 github.com/robfig/cron/v3 v3.0.1 + github.com/shirou/gopsutil/v3 v3.24.4 github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 github.com/spf13/afero v1.12.0 github.com/spf13/pflag v1.0.5 @@ -401,7 +402,6 @@ require ( github.com/ryanuber/go-glob v1.0.0 // indirect github.com/satori/go.uuid v1.2.1-0.20181028125025-b2ce2384e17b // indirect github.com/secure-systems-lab/go-securesystemslib v0.7.0 // indirect - github.com/shirou/gopsutil/v3 v3.24.4 github.com/shoenig/go-m1cpu v0.1.6 // indirect github.com/sirupsen/logrus v1.9.3 // indirect github.com/spaolacci/murmur3 v1.1.0 // indirect From e0089bd6e003ee5fb5e2267f8eb7bbd578faec49 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Fri, 28 Feb 2025 07:30:07 +0000 Subject: [PATCH 05/11] review --- agent/ls.go | 20 +++++++++-------- agent/ls_internal_test.go | 46 ++++++++++++++++++++++++--------------- 2 files changed, 40 insertions(+), 26 deletions(-) diff --git a/agent/ls.go b/agent/ls.go index db1717ab7da79..f2eb847a4bc97 100644 --- a/agent/ls.go +++ b/agent/ls.go @@ -5,6 +5,7 @@ import ( "net/http" "os" "path/filepath" + "regexp" "runtime" "strings" @@ -25,20 +26,17 @@ func (*agent) HandleLS(rw http.ResponseWriter, r *http.Request) { resp, err := listFiles(query) if err != nil { + status := http.StatusInternalServerError switch { case errors.Is(err, os.ErrNotExist): - httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ - Message: err.Error(), - }) + status = http.StatusNotFound case errors.Is(err, os.ErrPermission): - httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ - Message: err.Error(), - }) + status = http.StatusForbidden default: - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: err.Error(), - }) } + httpapi.Write(ctx, rw, status, codersdk.Response{ + Message: err.Error(), + }) return } @@ -59,6 +57,10 @@ func listFiles(query LSRequest) (LSResponse, error) { if len(query.Path) == 0 { return listDrives() } + re := regexp.MustCompile(`^[a-zA-Z]:\\$`) + if !re.MatchString(query.Path[0]) { + return LSResponse{}, xerrors.Errorf("invalid drive letter %q", query.Path[0]) + } } else { fullPath = []string{"/"} } diff --git a/agent/ls_internal_test.go b/agent/ls_internal_test.go index 133fd0709695e..c649908e1c1aa 100644 --- a/agent/ls_internal_test.go +++ b/agent/ls_internal_test.go @@ -115,6 +115,10 @@ func TestListFilesSuccess(t *testing.T) { err = os.Mkdir(downloadsDir, 0o755) require.NoError(t, err) + textFile := filepath.Join(tmpDir, "file.txt") + err = os.WriteFile(textFile, []byte("content"), 0o600) + require.NoError(t, err) + var queryComponents []string // We can't get an absolute path relative to empty string on Windows. if runtime.GOOS == "windows" && base == "" { @@ -133,23 +137,23 @@ func TestListFilesSuccess(t *testing.T) { require.NoError(t, err) require.Equal(t, tmpDir, resp.AbsolutePathString) - - var foundRepos, foundDownloads bool - for _, file := range resp.Contents { - switch file.Name { - case "repos": - foundRepos = true - expectedPath := filepath.Join(tmpDir, "repos") - require.Equal(t, expectedPath, file.AbsolutePathString) - require.True(t, file.IsDir) - case "Downloads": - foundDownloads = true - expectedPath := filepath.Join(tmpDir, "Downloads") - require.Equal(t, expectedPath, file.AbsolutePathString) - require.True(t, file.IsDir) - } - } - require.True(t, foundRepos && foundDownloads, "expected to find both repos and Downloads directories, got: %+v", resp.Contents) + require.ElementsMatch(t, []LSFile{ + { + Name: "repos", + AbsolutePathString: reposDir, + IsDir: true, + }, + { + Name: "Downloads", + AbsolutePathString: downloadsDir, + IsDir: true, + }, + { + Name: "file.txt", + AbsolutePathString: textFile, + IsDir: false, + }, + }, resp.Contents) }) } } @@ -186,4 +190,12 @@ func TestListFilesListDrives(t *testing.T) { } resp, err = listFiles(query) require.NoError(t, err) + + query = LSRequest{ + // Network drives are not supported. + Path: []string{"\\sshfs\\work"}, + Relativity: LSRelativityRoot, + } + resp, err = listFiles(query) + require.ErrorContains(t, err, "drive") } From 01c2e72c1099dcb54e4e329ae100eeb5ac442416 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 3 Mar 2025 01:17:52 +0000 Subject: [PATCH 06/11] review --- agent/ls.go | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/agent/ls.go b/agent/ls.go index f2eb847a4bc97..330d0975d5ab6 100644 --- a/agent/ls.go +++ b/agent/ls.go @@ -16,6 +16,8 @@ import ( "github.com/coder/coder/v2/codersdk" ) +var WindowsDriveRegex = regexp.MustCompile(`^[a-zA-Z]:\\$`) + func (*agent) HandleLS(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -57,8 +59,7 @@ func listFiles(query LSRequest) (LSResponse, error) { if len(query.Path) == 0 { return listDrives() } - re := regexp.MustCompile(`^[a-zA-Z]:\\$`) - if !re.MatchString(query.Path[0]) { + if !WindowsDriveRegex.MatchString(query.Path[0]) { return LSResponse{}, xerrors.Errorf("invalid drive letter %q", query.Path[0]) } } else { @@ -74,6 +75,7 @@ func listFiles(query LSRequest) (LSResponse, error) { if err != nil { return LSResponse{}, xerrors.Errorf("failed to get absolute path of %q: %w", fullPathRelative, err) } + absolutePath := pathToArray(absolutePathString) f, err := os.Open(absolutePathString) if err != nil { @@ -87,7 +89,18 @@ func listFiles(query LSRequest) (LSResponse, error) { } if !stat.IsDir() { - return LSResponse{}, xerrors.Errorf("path %q is not a directory", absolutePathString) + // `ls` on a file should return just that file. + return LSResponse{ + AbsolutePath: absolutePath, + AbsolutePathString: absolutePathString, + Contents: []LSFile{ + { + Name: f.Name(), + AbsolutePathString: absolutePathString, + IsDir: false, + }, + }, + }, nil } // `contents` may be partially populated even if the operation fails midway. @@ -101,8 +114,6 @@ func listFiles(query LSRequest) (LSResponse, error) { }) } - absolutePath := pathToArray(absolutePathString) - return LSResponse{ AbsolutePath: absolutePath, AbsolutePathString: absolutePathString, @@ -111,12 +122,12 @@ func listFiles(query LSRequest) (LSResponse, error) { } func listDrives() (LSResponse, error) { - aa, err := disk.Partitions(true) + partitionStats, err := disk.Partitions(true) if err != nil { return LSResponse{}, xerrors.Errorf("failed to get partitions: %w", err) } - contents := make([]LSFile, 0, len(aa)) - for _, a := range aa { + contents := make([]LSFile, 0, len(partitionStats)) + for _, a := range partitionStats { name := a.Mountpoint + string(os.PathSeparator) contents = append(contents, LSFile{ Name: name, From b1e414f688addfd86287b2f3dffe56a9076aa1bf Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 3 Mar 2025 01:59:24 +0000 Subject: [PATCH 07/11] fixup --- agent/ls.go | 2 +- agent/ls_internal_test.go | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/agent/ls.go b/agent/ls.go index 330d0975d5ab6..b882ccc028d3e 100644 --- a/agent/ls.go +++ b/agent/ls.go @@ -95,7 +95,7 @@ func listFiles(query LSRequest) (LSResponse, error) { AbsolutePathString: absolutePathString, Contents: []LSFile{ { - Name: f.Name(), + Name: filepath.Base(f.Name()), AbsolutePathString: absolutePathString, IsDir: false, }, diff --git a/agent/ls_internal_test.go b/agent/ls_internal_test.go index c649908e1c1aa..0ce3e401f4f5b 100644 --- a/agent/ls_internal_test.go +++ b/agent/ls_internal_test.go @@ -66,8 +66,16 @@ func TestListFilesNotADirectory(t *testing.T) { Path: pathToArray(rel), Relativity: LSRelativityHome, } - _, err = listFiles(query) - require.ErrorContains(t, err, "is not a directory") + resp, err := listFiles(query) + require.NoError(t, err) + require.Equal(t, filePath, resp.AbsolutePathString) + require.ElementsMatch(t, []LSFile{ + { + Name: "file.txt", + AbsolutePathString: filePath, + IsDir: false, + }, + }, resp.Contents) } func TestListFilesSuccess(t *testing.T) { From 369852ca4631163807992775cf0ce04906814fc8 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 3 Mar 2025 02:30:50 +0000 Subject: [PATCH 08/11] undo ls on file --- agent/ls.go | 16 +++------------- agent/ls_internal_test.go | 12 ++---------- 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/agent/ls.go b/agent/ls.go index b882ccc028d3e..3385f682173a9 100644 --- a/agent/ls.go +++ b/agent/ls.go @@ -75,7 +75,6 @@ func listFiles(query LSRequest) (LSResponse, error) { if err != nil { return LSResponse{}, xerrors.Errorf("failed to get absolute path of %q: %w", fullPathRelative, err) } - absolutePath := pathToArray(absolutePathString) f, err := os.Open(absolutePathString) if err != nil { @@ -89,18 +88,7 @@ func listFiles(query LSRequest) (LSResponse, error) { } if !stat.IsDir() { - // `ls` on a file should return just that file. - return LSResponse{ - AbsolutePath: absolutePath, - AbsolutePathString: absolutePathString, - Contents: []LSFile{ - { - Name: filepath.Base(f.Name()), - AbsolutePathString: absolutePathString, - IsDir: false, - }, - }, - }, nil + return LSResponse{}, xerrors.Errorf("path %q is not a directory", absolutePathString) } // `contents` may be partially populated even if the operation fails midway. @@ -114,6 +102,8 @@ func listFiles(query LSRequest) (LSResponse, error) { }) } + absolutePath := pathToArray(absolutePathString) + return LSResponse{ AbsolutePath: absolutePath, AbsolutePathString: absolutePathString, diff --git a/agent/ls_internal_test.go b/agent/ls_internal_test.go index 0ce3e401f4f5b..c649908e1c1aa 100644 --- a/agent/ls_internal_test.go +++ b/agent/ls_internal_test.go @@ -66,16 +66,8 @@ func TestListFilesNotADirectory(t *testing.T) { Path: pathToArray(rel), Relativity: LSRelativityHome, } - resp, err := listFiles(query) - require.NoError(t, err) - require.Equal(t, filePath, resp.AbsolutePathString) - require.ElementsMatch(t, []LSFile{ - { - Name: "file.txt", - AbsolutePathString: filePath, - IsDir: false, - }, - }, resp.Contents) + _, err = listFiles(query) + require.ErrorContains(t, err, "is not a directory") } func TestListFilesSuccess(t *testing.T) { From c58b7201b176818641d78e16ccd26eb55b63095f Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 3 Mar 2025 07:07:21 +0000 Subject: [PATCH 09/11] gopsutil v4, add windows drive seperator clarification --- agent/ls.go | 7 +++++-- go.mod | 5 +++-- go.sum | 6 ++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/agent/ls.go b/agent/ls.go index 3385f682173a9..1d8adea12e0b4 100644 --- a/agent/ls.go +++ b/agent/ls.go @@ -9,7 +9,7 @@ import ( "runtime" "strings" - "github.com/shirou/gopsutil/v3/disk" + "github.com/shirou/gopsutil/v4/disk" "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/httpapi" @@ -118,6 +118,8 @@ func listDrives() (LSResponse, error) { } contents := make([]LSFile, 0, len(partitionStats)) for _, a := range partitionStats { + // Drive letters on Windows have a trailing separator as part of their name. + // i.e. `os.Open("C:")` does not work, but `os.Open("C:\\")` does. name := a.Mountpoint + string(os.PathSeparator) contents = append(contents, LSFile{ Name: name, @@ -137,7 +139,8 @@ func pathToArray(path string) []string { out := strings.FieldsFunc(path, func(r rune) bool { return r == os.PathSeparator }) - // Drive letters on Windows should have a trailing separator. + // Drive letters on Windows have a trailing separator as part of their name. + // i.e. `os.Open("C:")` does not work, but `os.Open("C:\\")` does. if runtime.GOOS == "windows" && len(out) > 0 { out[0] += string(os.PathSeparator) } diff --git a/go.mod b/go.mod index b9170e3ff5055..1e68a84f47002 100644 --- a/go.mod +++ b/go.mod @@ -164,7 +164,7 @@ require ( github.com/prometheus/common v0.62.0 github.com/quasilyte/go-ruleguard/dsl v0.3.21 github.com/robfig/cron/v3 v3.0.1 - github.com/shirou/gopsutil/v3 v3.24.4 + github.com/shirou/gopsutil/v4 v4.25.2 github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 github.com/spf13/afero v1.12.0 github.com/spf13/pflag v1.0.5 @@ -286,7 +286,7 @@ require ( github.com/dop251/goja v0.0.0-20241024094426-79f3a7efcdbd // indirect github.com/dustin/go-humanize v1.0.1 github.com/eapache/queue/v2 v2.0.0-20230407133247-75960ed334e4 // indirect - github.com/ebitengine/purego v0.6.0-alpha.5 // indirect + github.com/ebitengine/purego v0.8.2 // indirect github.com/elastic/go-windows v1.0.0 // indirect github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect github.com/felixge/httpsnoop v1.0.4 // indirect @@ -402,6 +402,7 @@ require ( github.com/ryanuber/go-glob v1.0.0 // indirect github.com/satori/go.uuid v1.2.1-0.20181028125025-b2ce2384e17b // indirect github.com/secure-systems-lab/go-securesystemslib v0.7.0 // indirect + github.com/shirou/gopsutil/v3 v3.24.4 // indirect github.com/shoenig/go-m1cpu v0.1.6 // indirect github.com/sirupsen/logrus v1.9.3 // indirect github.com/spaolacci/murmur3 v1.1.0 // indirect diff --git a/go.sum b/go.sum index 6496dfc84118d..bd29a7b7bef56 100644 --- a/go.sum +++ b/go.sum @@ -301,8 +301,8 @@ github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkp github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= github.com/eapache/queue/v2 v2.0.0-20230407133247-75960ed334e4 h1:8EXxF+tCLqaVk8AOC29zl2mnhQjwyLxxOTuhUazWRsg= github.com/eapache/queue/v2 v2.0.0-20230407133247-75960ed334e4/go.mod h1:I5sHm0Y0T1u5YjlyqC5GVArM7aNZRUYtTjmJ8mPJFds= -github.com/ebitengine/purego v0.6.0-alpha.5 h1:EYID3JOAdmQ4SNZYJHu9V6IqOeRQDBYxqKAg9PyoHFY= -github.com/ebitengine/purego v0.6.0-alpha.5/go.mod h1:ah1In8AOtksoNK6yk5z1HTJeUkC1Ez4Wk2idgGslMwQ= +github.com/ebitengine/purego v0.8.2 h1:jPPGWs2sZ1UgOSgD2bClL0MJIqu58nOmIcBuXr62z1I= +github.com/ebitengine/purego v0.8.2/go.mod h1:iIjxzd6CiRiOG0UyXP+V1+jWqUXVjPKLAI0mRfJZTmQ= github.com/elastic/go-sysinfo v1.15.0 h1:54pRFlAYUlVNQ2HbXzLVZlV+fxS7Eax49stzg95M4Xw= github.com/elastic/go-sysinfo v1.15.0/go.mod h1:jPSuTgXG+dhhh0GKIyI2Cso+w5lPJ5PvVqKlL8LV/Hk= github.com/elastic/go-windows v1.0.0 h1:qLURgZFkkrYyTTkvYpsZIgf83AUsdIHfvlJaqaZ7aSY= @@ -825,6 +825,8 @@ github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 h1:n661drycOFuPLCN github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3/go.mod h1:A0bzQcvG0E7Rwjx0REVgAGH58e96+X0MeOfepqsbeW4= github.com/shirou/gopsutil/v3 v3.24.4 h1:dEHgzZXt4LMNm+oYELpzl9YCqV65Yr/6SfrvgRBtXeU= github.com/shirou/gopsutil/v3 v3.24.4/go.mod h1:lTd2mdiOspcqLgAnr9/nGi71NkeMpWKdmhuxm9GusH8= +github.com/shirou/gopsutil/v4 v4.25.2 h1:NMscG3l2CqtWFS86kj3vP7soOczqrQYIEhO/pMvvQkk= +github.com/shirou/gopsutil/v4 v4.25.2/go.mod h1:34gBYJzyqCDT11b6bMHP0XCvWeU3J61XRT7a2EmCRTA= github.com/shoenig/go-m1cpu v0.1.6 h1:nxdKQNcEB6vzgA2E2bvzKIYRuNj7XNJ4S/aRSwKzFtM= github.com/shoenig/go-m1cpu v0.1.6/go.mod h1:1JJMcUBvfNwpq05QDQVAnx3gUHr9IYF7GNg9SUEw2VQ= github.com/shoenig/test v0.6.4 h1:kVTaSd7WLz5WZ2IaoM0RSzRsUD+m8wRR+5qvntpn4LU= From b2959dbc4531c2ac1e525f108593285b14d30abe Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 3 Mar 2025 07:19:33 +0000 Subject: [PATCH 10/11] require C:\Windows exists in tests --- agent/ls_internal_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/agent/ls_internal_test.go b/agent/ls_internal_test.go index c649908e1c1aa..acc4ea2929444 100644 --- a/agent/ls_internal_test.go +++ b/agent/ls_internal_test.go @@ -190,6 +190,12 @@ func TestListFilesListDrives(t *testing.T) { } resp, err = listFiles(query) require.NoError(t, err) + // System directory should always exist + require.Contains(t, resp.Contents, LSFile{ + Name: "Windows", + AbsolutePathString: "C:\\Windows", + IsDir: true, + }) query = LSRequest{ // Network drives are not supported. From c2f15f496ecee5dbb37b2659764505dcd0107ef7 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Fri, 7 Mar 2025 04:21:36 +0000 Subject: [PATCH 11/11] ls -> list-directory --- agent/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/api.go b/agent/api.go index 05ed760219a6a..259866797a3c4 100644 --- a/agent/api.go +++ b/agent/api.go @@ -41,7 +41,7 @@ func (a *agent) apiHandler() http.Handler { r.Get("/api/v0/containers", ch.ServeHTTP) r.Get("/api/v0/listening-ports", lp.handler) r.Get("/api/v0/netcheck", a.HandleNetcheck) - r.Post("/api/v0/ls", a.HandleLS) + r.Post("/api/v0/list-directory", a.HandleLS) r.Get("/debug/logs", a.HandleHTTPDebugLogs) r.Get("/debug/magicsock", a.HandleHTTPDebugMagicsock) r.Get("/debug/magicsock/debug-logging/{state}", a.HandleHTTPMagicsockDebugLoggingState)