From 0574d421585e5f513c804578b7e92a689ebd4e0f Mon Sep 17 00:00:00 2001 From: Maximilian Pass <22845248+mpass99@users.noreply.github.com> Date: Tue, 20 Aug 2024 15:18:43 +0200 Subject: [PATCH] Fix missing link target for listing the file system with a link, the executing user has not enough privileges to read the target. --- pkg/nullio/ls2json.go | 10 +++++++--- tests/e2e/runners_test.go | 41 +++++++++++++++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/pkg/nullio/ls2json.go b/pkg/nullio/ls2json.go index 56c7c277..16c8ece2 100644 --- a/pkg/nullio/ls2json.go +++ b/pkg/nullio/ls2json.go @@ -163,11 +163,15 @@ func (w *Ls2JsonWriter) parseFileHeader(matches [][]byte) ([]byte, error) { if entryType == dto.EntryTypeLink { parts := strings.Split(string(name), " -> ") const NumberOfPartsInALink = 2 - if len(parts) == NumberOfPartsInALink { + switch len(parts) { + case NumberOfPartsInALink: name = dto.FilePath(parts[0]) linkTarget = dto.FilePath(parts[1]) - } else { - log.WithContext(w.sentrySpan.Context()).Error("could not split link into name and target") + case 1: + // This case happens when a user tries to read the target of a link without permission. See #596. + // Nothing to do. The target remains empty. The name is set. + default: + log.WithContext(w.sentrySpan.Context()).WithField("name", name).Error("could not split link into name and target") } } diff --git a/tests/e2e/runners_test.go b/tests/e2e/runners_test.go index 4c5efa94..e3641e4f 100644 --- a/tests/e2e/runners_test.go +++ b/tests/e2e/runners_test.go @@ -101,10 +101,10 @@ func (s *E2ETestSuite) TestListFileSystem_Nomad() { ExecutionEnvironmentID: tests.DefaultEnvironmentIDAsInteger, }) s.Require().NoError(err) + getFileURL, err := url.Parse(helpers.BuildURL(api.BasePath, api.RunnersPath, runnerID, api.UpdateFileSystemPath)) + s.Require().NoError(err) s.Run("No files", func() { - getFileURL, err := url.Parse(helpers.BuildURL(api.BasePath, api.RunnersPath, runnerID, api.UpdateFileSystemPath)) - s.Require().NoError(err) response, err := http.Get(getFileURL.String()) s.Require().NoError(err) s.Equal(http.StatusOK, response.StatusCode) @@ -120,8 +120,6 @@ func (s *E2ETestSuite) TestListFileSystem_Nomad() { s.Require().NoError(err) s.Equal(http.StatusNoContent, resp.StatusCode) - getFileURL, err := url.Parse(helpers.BuildURL(api.BasePath, api.RunnersPath, runnerID, api.UpdateFileSystemPath)) - s.Require().NoError(err) response, err := http.Get(getFileURL.String()) s.Require().NoError(err) s.Equal(http.StatusOK, response.StatusCode) @@ -138,6 +136,41 @@ func (s *E2ETestSuite) TestListFileSystem_Nomad() { s.Equal("root", fileHeader.Group) s.Equal("rwxr--r--", fileHeader.Permissions) }) + + s.Run("With links", func() { + s.Run("allowed", func() { + path := "/proc/self/cwd" + getFileURL.RawQuery = url.Values{api.PathKey: []string{path}}.Encode() + + response, err := http.Get(getFileURL.String()) + s.Require().NoError(err) + s.Equal(http.StatusOK, response.StatusCode) + listFilesResponse := new(dto.ListFileSystemResponse) + err = json.NewDecoder(response.Body).Decode(listFilesResponse) + s.Require().NoError(err) + s.Require().Equal(1, len(listFilesResponse.Files)) + fileHeader := listFilesResponse.Files[0] + s.Equal(dto.FilePath(path), fileHeader.Name) + s.Equal(dto.EntryTypeLink, fileHeader.EntryType) + s.Equal(dto.FilePath("/workspace"), fileHeader.LinkTarget) + }) + s.Run("denied", func() { + path := "/proc/1/cwd" + getFileURL.RawQuery = url.Values{api.PathKey: []string{path}}.Encode() + + response, err := http.Get(getFileURL.String()) + s.Require().NoError(err) + s.Equal(http.StatusOK, response.StatusCode) + listFilesResponse := new(dto.ListFileSystemResponse) + err = json.NewDecoder(response.Body).Decode(listFilesResponse) + s.Require().NoError(err) + s.Require().Equal(1, len(listFilesResponse.Files)) + fileHeader := listFilesResponse.Files[0] + s.Equal(dto.FilePath(path), fileHeader.Name) + s.Equal(dto.EntryTypeLink, fileHeader.EntryType) + s.Empty(fileHeader.LinkTarget) + }) + }) } func (s *E2ETestSuite) TestCopyFilesRoute() {