Fix missing link target

for listing the file system with a link, the executing user has not enough privileges to read the target.
openHPI · Aug 20, 2024 · 5758a0e · 5758a0e
1 parent 8390b90
commit 5758a0e
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 7 deletions.
diff --git a/pkg/nullio/ls2json.go b/pkg/nullio/ls2json.go
@@ -163,11 +163,14 @@ 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.
+		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
@@ -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("/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() {