Repository: jupyter-server/jupyter_server

Issue:
Symlinks with inaccessible targets cause filemanager to not list anything in the same directory
## Description

It seems that in jupyter_core, in `is_file_hidden_posix()` an exception is raised if it comes across a symlink where the target path is not accessible to the user (PermissionError). This has the effect of stopping the file contents manager in JupyterLab 3 from displaying the contents of a directory containing such links.

The traceback looks like the following, where the symlink points to a directory that is not accessible to user1:

    Traceback (most recent call last):
      File "/usr/local/lib/python3.8/dist-packages/tornado/web.py", line 1704, in _execute
        result = await result
      File "/usr/local/lib/python3.8/dist-packages/jupyter_server/services/contents/handlers.py", line 110, in get
        model = await ensure_async(self.contents_manager.get(
      File "/usr/local/lib/python3.8/dist-packages/jupyter_server/services/contents/filemanager.py", line 381, in get
        model = self._dir_model(path, content=content)
      File "/usr/local/lib/python3.8/dist-packages/jupyter_server/services/contents/filemanager.py", line 288, in _dir_model
        if self.allow_hidden or not is_file_hidden(os_path, stat_res=st):
      File "/usr/local/lib/python3.8/dist-packages/jupyter_core/paths.py", line 297, in is_file_hidden_posix
        stat_res = os.stat(abs_path)
    PermissionError: [Errno 13] Permission denied: '/home/user1/subdir/secret'

## Reproduce

1. Have user 1 create a directory that user 2 cannot see, and have user 2 link to the directory created by user 1
2. Have user 2 do something like `touch a b c` in the same directory as their symlink
2. Have user 2 start up JupyterLab 3 with jupyter_server
3. Browsing to the directory containing the symlink cause the file listing to freeze
4. Logs will show traceback similar to above 

## Expected behavior

The file listing should just omit the disallowed link since but list everything else that can normally be listed.
I think this means that jupyter_server should handle the exception.  See also jupyter/jupyter_core#224

## Context

- Operating System and version: ubuntu:focal
- Browser and version: Chrome 90.0.4430.93
- Jupyter Server version: 1.6.4


Discussion:

Patch:
diff --git a/.github/workflows/check-release.yml b/.github/workflows/check-release.yml
deleted file mode 100644
index 335ab9d03..000000000
--- a/.github/workflows/check-release.yml
+++ /dev/null
@@ -1,47 +0,0 @@
-name: Check Release
-on:
-  push:
-    branches: ["master"]
-  pull_request:
-    branches: ["*"]
-
-jobs:
-  check_release:
-    runs-on: ubuntu-latest
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v2
-      - name: Install Python
-        uses: actions/setup-python@v2
-        with:
-          python-version: 3.9
-          architecture: "x64"
-      - name: Get pip cache dir
-        id: pip-cache
-        run: |
-          echo "::set-output name=dir::$(pip cache dir)"
-      - name: Cache pip
-        uses: actions/cache@v2
-        with:
-          path: ${{ steps.pip-cache.outputs.dir }}
-          key: ${{ runner.os }}-pip-${{ hashFiles('setup.cfg') }}
-          restore-keys: |
-            ${{ runner.os }}-pip-
-            ${{ runner.os }}-pip-
-      - name: Cache checked links
-        uses: actions/cache@v2
-        with:
-          path: ~/.cache/pytest-link-check
-          key: ${{ runner.os }}-linkcheck-${{ hashFiles('**/.md') }}-md-links
-          restore-keys: |
-            ${{ runner.os }}-linkcheck-
-      - name: Upgrade packaging dependencies
-        run: |
-          pip install --upgrade pip setuptools wheel --user
-      - name: Install Dependencies
-        run: |
-          pip install -e .
-      - name: Check Release
-        uses: jupyter-server/jupyter_releaser/.github/actions/check-release@v1
-        with:
-          token: ${{ secrets.GITHUB_TOKEN }}
diff --git a/.github/workflows/python-linux.yml b/.github/workflows/python-linux.yml
index eed4081e3..8fedfc3ec 100644
--- a/.github/workflows/python-linux.yml
+++ b/.github/workflows/python-linux.yml
@@ -68,3 +68,13 @@ jobs:
         pushd test_install
         ./bin/pytest --pyargs jupyter_server
         popd
+    - name: Check the Manifest
+      run: |
+        pip install check-manifest
+        git clean -dfx
+        check-manifest -v
+    - name: Check Version Bump
+      run: |
+        pip install tbump
+        tbump --non-interactive --only-patch 100.1.1
+        git checkout .
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8771c9329..4295da340 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,8 +2,6 @@
 
 All notable changes to this project will be documented in this file.
 
-<!-- <START NEW CHANGELOG ENTRY> -->
-
 ## 1.6.4
 
 ([Full Changelog](https://github.com/jupyter-server/jupyter_server/compare/v1.6.3...68a64ea13be5d0d86460f04e0c47eb0b6662a0af))
@@ -18,8 +16,6 @@ All notable changes to this project will be documented in this file.
 
 [@afshin](https://github.com/search?q=repo%3Ajupyter-server%2Fjupyter_server+involves%3Aafshin+updated%3A2021-04-21..2021-04-21&type=Issues)
 
-<!-- <END NEW CHANGELOG ENTRY> -->
-
 ## 1.6.3
 
 ([Full Changelog](https://github.com/jupyter-server/jupyter_server/compare/v1.6.2...aa2636795ae1d87e3055febb3931f891dd6b4451))
diff --git a/MANIFEST.in b/MANIFEST.in
index 70ccbaaa9..84c24303c 100644
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -25,8 +25,7 @@ prune docs/dist
 global-exclude *~
 global-exclude *.pyc
 global-exclude *.pyo
-prune .git
-prune **/.ipynb_checkpoints
-prune **/.pytest_cache
-prune **/.coverage
-prune **/.pytest_cache
+global-exclude .git
+global-exclude .ipynb_checkpoints
+global-exclude .pytest_cache
+global-exclude .coverage
diff --git a/RELEASE.md b/RELEASE.md
index a785a2870..b492af54d 100644
--- a/RELEASE.md
+++ b/RELEASE.md
@@ -1,48 +1,36 @@
 # Making a Jupyter Server Release
+To create a release, perform the following steps...
 
-## Using `jupyter_releaser`
-
-The recommended way to make a release is to use [`jupyter_releaser`](https://github.com/jupyter-server/jupyter_releaser#checklist-for-adoption).
-
-## Manual Release
-
-To create a manual release, perform the following steps:
-
-### Set up
-
-```bash
+## Set up
+```
 pip install tbump twine build
 git pull origin $(git branch --show-current)
 git clean -dffx
 ```
 
-### Update the version and apply the tag
-
-```bash
+## Update the version and apply the tag
+```
 echo "Enter new version"
 read script_version
 tbump ${script_version}
 ```
 
-### Build the artifacts
-
-```bash
+## Build the artifacts
+```
 rm -rf dist
 python -m build .
 ```
 
-### Update the version back to dev
-
-```bash
+## Update the version back to dev
+```
 echo "Enter dev version"
 read dev_version
 tbump ${dev_version} --no-tag
 git push origin $(git branch --show-current)
 ```
 
-### Publish the artifacts to pypi
-
-```bash
+## Publish the artifacts to pypi
+```
 twine check dist/*
 twine upload dist/*
 ```
diff --git a/jupyter_server/services/contents/filemanager.py b/jupyter_server/services/contents/filemanager.py
index 6f465cf9e..516696942 100644
--- a/jupyter_server/services/contents/filemanager.py
+++ b/jupyter_server/services/contents/filemanager.py
@@ -273,7 +273,7 @@ class FileContentsManager(FileManagerMixin, ContentsManager):
                     # skip over broken symlinks in listing
                     if e.errno == errno.ENOENT:
                         self.log.warning("%s doesn't exist", os_path)
-                    else:
+                    elif e.errno != errno.EACCES:  # Don't provide clues about protected files
                         self.log.warning("Error stat-ing %s: %s", os_path, e)
                     continue
 
@@ -283,17 +283,25 @@ class FileContentsManager(FileManagerMixin, ContentsManager):
                     self.log.debug("%s not a regular file", os_path)
                     continue
 
-                if self.should_list(name):
-                    if self.allow_hidden or not is_file_hidden(os_path, stat_res=st):
-                        contents.append(
+                try:
+                    if self.should_list(name):
+                        if self.allow_hidden or not is_file_hidden(os_path, stat_res=st):
+                            contents.append(
                                 self.get(path='%s/%s' % (path, name), content=False)
+                            )
+                except OSError as e:
+                    # ELOOP: recursive symlink, also don't show failure due to permissions
+                    if e.errno not in [errno.ELOOP, errno.EACCES]:
+                        self.log.warning(
+                            "Unknown error checking if file %r is hidden",
+                            os_path,
+                            exc_info=True,
                         )
 
             model['format'] = 'json'
 
         return model
 
-
     def _file_model(self, path, content=True, format=None):
         """Build a model for a file
 
@@ -585,7 +593,7 @@ class AsyncFileContentsManager(FileContentsManager, AsyncFileManagerMixin, Async
                     # skip over broken symlinks in listing
                     if e.errno == errno.ENOENT:
                         self.log.warning("%s doesn't exist", os_path)
-                    else:
+                    elif e.errno != errno.EACCES:  # Don't provide clues about protected files
                         self.log.warning("Error stat-ing %s: %s", os_path, e)
                     continue
 
@@ -595,10 +603,19 @@ class AsyncFileContentsManager(FileContentsManager, AsyncFileManagerMixin, Async
                     self.log.debug("%s not a regular file", os_path)
                     continue
 
-                if self.should_list(name):
-                    if self.allow_hidden or not is_file_hidden(os_path, stat_res=st):
-                        contents.append(
-                                await self.get(path='%s/%s' % (path, name), content=False)
+                try:
+                    if self.should_list(name):
+                        if self.allow_hidden or not is_file_hidden(os_path, stat_res=st):
+                            contents.append(
+                                    await self.get(path='%s/%s' % (path, name), content=False)
+                        )
+                except OSError as e:
+                    # ELOOP: recursive symlink, also don't show failure due to permissions
+                    if e.errno not in [errno.ELOOP, errno.EACCES]:
+                        self.log.warning(
+                            "Unknown error checking if file %r is hidden",
+                            os_path,
+                            exc_info=True,
                         )
 
             model['format'] = 'json'


Test patch:
diff --git a/jupyter_server/tests/services/contents/test_manager.py b/jupyter_server/tests/services/contents/test_manager.py
index 27534dd8f..98f358645 100644
--- a/jupyter_server/tests/services/contents/test_manager.py
+++ b/jupyter_server/tests/services/contents/test_manager.py
@@ -2,18 +2,17 @@ import os
 import sys
 import time
 import pytest
-import functools
 from traitlets import TraitError
 from tornado.web import HTTPError
 from itertools import combinations
 
-
 from nbformat import v4 as nbformat
 
 from jupyter_server.services.contents.filemanager import AsyncFileContentsManager, FileContentsManager
 from jupyter_server.utils import ensure_async
 from ...utils import expected_http_error
 
+
 @pytest.fixture(params=[(FileContentsManager, True),
                         (FileContentsManager, False),
                         (AsyncFileContentsManager, True),
@@ -29,6 +28,7 @@ def file_contents_manager_class(request, tmp_path):
 
 # -------------- Functions ----------------------------
 
+
 def _make_dir(jp_contents_manager, api_path):
     """
     Make a directory.
@@ -99,6 +99,7 @@ async def check_populated_dir_files(jp_contents_manager, api_path):
 
 # ----------------- Tests ----------------------------------
 
+
 def test_root_dir(file_contents_manager_class, tmp_path):
     fm = file_contents_manager_class(root_dir=str(tmp_path))
     assert fm.root_dir == str(tmp_path)
@@ -116,6 +117,7 @@ def test_invalid_root_dir(file_contents_manager_class, tmp_path):
     with pytest.raises(TraitError):
        file_contents_manager_class(root_dir=str(temp_file))
 
+
 def test_get_os_path(file_contents_manager_class, tmp_path):
     fm = file_contents_manager_class(root_dir=str(tmp_path))
     path = fm._get_os_path('/path/to/notebook/test.ipynb')
@@ -146,10 +148,6 @@ def test_checkpoint_subdir(file_contents_manager_class, tmp_path):
     assert cp_dir == os.path.join(str(tmp_path), cpm.checkpoint_dir, cp_name)
 
 
-@pytest.mark.skipif(
-    sys.platform == 'win32' and sys.version_info[0] < 3,
-    reason="System platform is Windows, version < 3"
-)
 async def test_bad_symlink(file_contents_manager_class, tmp_path):
     td = str(tmp_path)
 
@@ -172,9 +170,31 @@ async def test_bad_symlink(file_contents_manager_class, tmp_path):
 
 
 @pytest.mark.skipif(
-    sys.platform == 'win32' and sys.version_info[0] < 3,
-    reason="System platform is Windows, version < 3"
+    sys.platform.startswith('win'),
+    reason="Windows doesn't detect symlink loops"
 )
+async def test_recursive_symlink(file_contents_manager_class, tmp_path):
+    td = str(tmp_path)
+
+    cm = file_contents_manager_class(root_dir=td)
+    path = 'test recursive symlink'
+    _make_dir(cm, path)
+
+    file_model = await ensure_async(cm.new_untitled(path=path, ext='.txt'))
+
+    # create recursive symlink
+    symlink(cm, '%s/%s' % (path, "recursive"), '%s/%s' % (path, "recursive"))
+    model = await ensure_async(cm.get(path))
+
+    contents = {
+        content['name']: content for content in model['content']
+    }
+    assert 'untitled.txt' in contents
+    assert contents['untitled.txt'] == file_model
+    # recursive symlinks should not be shown in the contents manager
+    assert 'recursive' not in contents
+
+
 async def test_good_symlink(file_contents_manager_class, tmp_path):
     td = str(tmp_path)
     cm = file_contents_manager_class(root_dir=td)
@@ -213,6 +233,7 @@ async def test_403(file_contents_manager_class, tmp_path):
     except HTTPError as e:
         assert e.status_code == 403
 
+
 async def test_escape_root(file_contents_manager_class, tmp_path):
     td = str(tmp_path)
     cm = file_contents_manager_class(root_dir=td)