From 877da10cd0d7ae45f8b1e385fa1f5a335e7adf1f Mon Sep 17 00:00:00 2001 From: Ryan <37374243+rashley-iqt@users.noreply.github.com> Date: Tue, 7 Jun 2022 13:00:40 -0400 Subject: [PATCH] Merge pull request from GHSA-q874-g24w-4q9g * added checks for hidden files and directories on FileManager Class * added checks for hidden files and directories in api handlers * updated error messages to not mention hidden files * cleaned up issues flagged by pre-commit --- .../services/contents/filemanager.py | 35 ++- jupyter_server/services/contents/handlers.py | 38 +++ tests/services/contents/test_api.py | 251 ++++++++++++++++++ tests/services/contents/test_manager.py | 160 +++++++++++ 4 files changed, 479 insertions(+), 5 deletions(-) diff --git a/jupyter_server/services/contents/filemanager.py b/jupyter_server/services/contents/filemanager.py index 7baaf842f6..b04ab4a3dc 100644 --- a/jupyter_server/services/contents/filemanager.py +++ b/jupyter_server/services/contents/filemanager.py @@ -188,6 +188,12 @@ def _base_model(self, path): os_path = self._get_os_path(path) info = os.lstat(os_path) + four_o_four = "file or directory does not exist: %r" % path + + if is_hidden(os_path, self.root_dir) and not self.allow_hidden: + self.log.info("Refusing to serve hidden file or directory %r, via 404 Error", os_path) + raise web.HTTPError(404, four_o_four) + try: # size of file size = info.st_size @@ -365,11 +371,16 @@ def get(self, path, content=True, type=None, format=None): of the file or directory as well. """ path = path.strip("/") + os_path = self._get_os_path(path) + four_o_four = "file or directory does not exist: %r" % path if not self.exists(path): - raise web.HTTPError(404, "No such file or directory: %s" % path) + raise web.HTTPError(404, four_o_four) + + if is_hidden(os_path, self.root_dir) and not self.allow_hidden: + self.log.info("Refusing to serve hidden file or directory %r, via 404 Error", os_path) + raise web.HTTPError(404, four_o_four) - os_path = self._get_os_path(path) if os.path.isdir(os_path): if type not in (None, "directory"): raise web.HTTPError( @@ -389,7 +400,7 @@ def get(self, path, content=True, type=None, format=None): def _save_directory(self, os_path, model, path=""): """create a directory""" if is_hidden(os_path, self.root_dir) and not self.allow_hidden: - raise web.HTTPError(400, "Cannot create hidden directory %r" % os_path) + raise web.HTTPError(400, "Cannot create directory %r" % os_path) if not os.path.exists(os_path): with self.perm_to_403(): os.mkdir(os_path) @@ -410,6 +421,10 @@ def save(self, model, path=""): raise web.HTTPError(400, "No file content provided") os_path = self._get_os_path(path) + + if is_hidden(os_path, self.root_dir) and not self.allow_hidden: + raise web.HTTPError(400, f"Cannot create file or directory {os_path!r}") + self.log.debug("Saving %s", os_path) validation_error: dict = {} @@ -452,8 +467,13 @@ def delete_file(self, path): path = path.strip("/") os_path = self._get_os_path(path) rm = os.unlink - if not os.path.exists(os_path): - raise web.HTTPError(404, "File or directory does not exist: %s" % os_path) + four_o_four = "file or directory does not exist: %r" % path + + if not self.exists(path): + raise web.HTTPError(404, four_o_four) + + if is_hidden(os_path, self.root_dir) and not self.allow_hidden: + raise web.HTTPError(400, f"Cannot delete file or directory {os_path!r}") def _check_trash(os_path): if sys.platform in {"win32", "darwin"}: @@ -518,6 +538,11 @@ def rename_file(self, old_path, new_path): new_os_path = self._get_os_path(new_path) old_os_path = self._get_os_path(old_path) + if ( + is_hidden(old_os_path, self.root_dir) or is_hidden(new_os_path, self.root_dir) + ) and not self.allow_hidden: + raise web.HTTPError(400, f"Cannot rename file or directory {old_os_path!r}") + # Should we proceed with the move? if os.path.exists(new_os_path) and not samefile(old_os_path, new_os_path): raise web.HTTPError(409, "File already exists: %s" % new_path) diff --git a/jupyter_server/services/contents/handlers.py b/jupyter_server/services/contents/handlers.py index 6b98c5d6cf..462cbff35e 100644 --- a/jupyter_server/services/contents/handlers.py +++ b/jupyter_server/services/contents/handlers.py @@ -95,6 +95,8 @@ async def get(self, path=""): of the files and directories it contains. """ path = path or "" + cm = self.contents_manager + type = self.get_query_argument("type", default=None) if type not in {None, "directory", "file", "notebook"}: raise web.HTTPError(400, "Type %r is invalid" % type) @@ -107,6 +109,9 @@ async def get(self, path=""): raise web.HTTPError(400, "Content %r is invalid" % content_str) content = int(content_str or "") + if await ensure_async(cm.is_hidden(path)) and not cm.allow_hidden: + raise web.HTTPError(404, f"file or directory {path!r} does not exist") + model = await ensure_async( self.contents_manager.get( path=path, @@ -126,6 +131,17 @@ async def patch(self, path=""): model = self.get_json_body() if model is None: raise web.HTTPError(400, "JSON body missing") + + old_path = model.get("path") + if ( + old_path + and ( + await ensure_async(cm.is_hidden(path)) or await ensure_async(cm.is_hidden(old_path)) + ) + and not cm.allow_hidden + ): + raise web.HTTPError(400, f"Cannot rename file or directory {path!r}") + model = await ensure_async(cm.update(model, path)) validate_model(model, expect_content=False) self._finish_model(model) @@ -191,6 +207,16 @@ async def post(self, path=""): raise web.HTTPError(400, "Cannot POST to files, use PUT instead.") model = self.get_json_body() + copy_from = model.get("copy_from") + if ( + copy_from + and ( + await ensure_async(cm.is_hidden(path)) + or await ensure_async(cm.is_hidden(copy_from)) + ) + and not cm.allow_hidden + ): + raise web.HTTPError(400, f"Cannot copy file or directory {path!r}") if model is not None: copy_from = model.get("copy_from") @@ -217,9 +243,17 @@ async def put(self, path=""): create a new empty notebook. """ model = self.get_json_body() + cm = self.contents_manager + if model: if model.get("copy_from"): raise web.HTTPError(400, "Cannot copy with PUT, only POST") + if ( + (model.get("path") and await ensure_async(cm.is_hidden(model.get("path")))) + or await ensure_async(cm.is_hidden(path)) + ) and not cm.allow_hidden: + raise web.HTTPError(400, f"Cannot create file or directory {path!r}") + exists = await ensure_async(self.contents_manager.file_exists(path)) if exists: await self._save(model, path) @@ -233,6 +267,10 @@ async def put(self, path=""): async def delete(self, path=""): """delete a file in the given path""" cm = self.contents_manager + + if await ensure_async(cm.is_hidden(path)) and not cm.allow_hidden: + raise web.HTTPError(400, f"Cannot delete file or directory {path!r}") + self.log.warning("delete %s", path) await ensure_async(cm.delete(path)) self.set_status(204) diff --git a/tests/services/contents/test_api.py b/tests/services/contents/test_api.py index 31b07e4137..ed72d16577 100644 --- a/tests/services/contents/test_api.py +++ b/tests/services/contents/test_api.py @@ -230,6 +230,36 @@ async def test_get_text_file_contents(jp_fetch, contents, path, name): ) assert expected_http_error(e, 400) +@pytest.mark.skipif(sys.platform == "win32", reason="Disabled retrieving hidden files on Windows") +async def test_get_404_hidden(jp_fetch, contents, contents_dir): + # Create text files + hidden_dir = contents_dir / '.hidden' + hidden_dir.mkdir(parents=True, exist_ok=True) + txt = f"visible text file in hidden dir" + txtname = hidden_dir.joinpath(f"visible.txt") + txtname.write_text(txt, encoding="utf-8") + + txt2 = f"hidden text file" + txtname2 = contents_dir.joinpath(f".hidden.txt") + txtname2.write_text(txt2, encoding="utf-8") + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch( + "api", + "contents", + ".hidden/visible.txt", + method="GET", + ) + assert expected_http_error(e, 404) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch( + "api", + "contents", + ".hidden.txt", + method="GET", + ) + assert expected_http_error(e, 404) @pytest.mark.parametrize("path,name", dirs) async def test_get_binary_file_contents(jp_fetch, contents, path, name): @@ -408,6 +438,55 @@ async def test_upload_txt(jp_fetch, contents, contents_dir, _check_created): assert model["content"] == body +@pytest.mark.skipif(sys.platform == "win32", reason="Disabled uploading hidden files on Windows") +async def test_upload_txt_hidden(jp_fetch, contents, contents_dir): + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + body = 'ünicode téxt' + model = { + 'content' : body, + 'format' : 'text', + 'type' : 'file', + } + path = '.hidden/Upload tést.txt' + await jp_fetch("api", "contents", path, method="PUT", body=json.dumps(model)) + assert expected_http_error(e, 400) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + body = 'ünicode téxt' + model = { + 'content' : body, + 'format' : 'text', + 'type' : 'file', + 'path': '.hidden/test.txt' + } + path = 'Upload tést.txt' + await jp_fetch("api", "contents", path, method="PUT", body=json.dumps(model)) + assert expected_http_error(e, 400) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + body = 'ünicode téxt' + model = { + 'content' : body, + 'format' : 'text', + 'type' : 'file', + } + path = '.hidden.txt' + await jp_fetch("api", "contents", path, method="PUT", body=json.dumps(model)) + assert expected_http_error(e, 400) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + body = 'ünicode téxt' + model = { + 'content' : body, + 'format' : 'text', + 'type' : 'file', + 'path': '.hidden.txt' + } + path = 'Upload tést.txt' + await jp_fetch("api", "contents", path, method="PUT", body=json.dumps(model)) + assert expected_http_error(e, 400) + + async def test_upload_b64(jp_fetch, contents, contents_dir, _check_created): body = b"\xFFblob" b64body = encodebytes(body).decode("ascii") @@ -501,6 +580,49 @@ async def test_copy_put_400(jp_fetch, contents, contents_dir, _check_created): assert expected_http_error(e, 400) +@pytest.mark.skipif(sys.platform == "win32", reason="Disabled copying hidden files on Windows") +async def test_copy_put_400_hidden(jp_fetch, contents, contents_dir,): + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch( + "api", + "contents", + ".hidden/old.txt", + method="PUT", + body=json.dumps({"copy_from": "new.txt"}), + ) + assert expected_http_error(e, 400) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch( + "api", + "contents", + "old.txt", + method="PUT", + body=json.dumps({"copy_from": ".hidden/new.txt"}), + ) + assert expected_http_error(e, 400) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch( + "api", + "contents", + ".hidden.txt", + method="PUT", + body=json.dumps({"copy_from": "new.txt"}), + ) + assert expected_http_error(e, 400) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch( + "api", + "contents", + "old.txt", + method="PUT", + body=json.dumps({"copy_from": ".hidden.txt"}), + ) + assert expected_http_error(e, 400) + + async def test_copy_dir_400(jp_fetch, contents, contents_dir, _check_created): with pytest.raises(tornado.httpclient.HTTPClientError) as e: await jp_fetch( @@ -513,6 +635,63 @@ async def test_copy_dir_400(jp_fetch, contents, contents_dir, _check_created): assert expected_http_error(e, 400) +@pytest.mark.skipif(sys.platform == "win32", reason="Disabled copying hidden files on Windows") +async def test_copy_400_hidden(jp_fetch, contents, contents_dir,): + + # Create text files + hidden_dir = contents_dir / '.hidden' + hidden_dir.mkdir(parents=True, exist_ok=True) + txt = f"visible text file in hidden dir" + txtname = hidden_dir.joinpath(f"new.txt") + txtname.write_text(txt, encoding="utf-8") + + paths = ['new.txt', '.hidden.txt'] + for name in paths: + txt = f"{name} text file" + txtname = contents_dir.joinpath(f"{name}.txt") + txtname.write_text(txt, encoding="utf-8") + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch( + "api", + "contents", + ".hidden/old.txt", + method="POST", + body=json.dumps({"copy_from": "new.txt"}), + ) + assert expected_http_error(e, 400) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch( + "api", + "contents", + "old.txt", + method="POST", + body=json.dumps({"copy_from": ".hidden/new.txt"}), + ) + assert expected_http_error(e, 400) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch( + "api", + "contents", + ".hidden.txt", + method="POST", + body=json.dumps({"copy_from": "new.txt"}), + ) + assert expected_http_error(e, 400) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch( + "api", + "contents", + "old.txt", + method="POST", + body=json.dumps({"copy_from": ".hidden.txt"}), + ) + assert expected_http_error(e, 400) + + @pytest.mark.parametrize("path,name", dirs) async def test_delete(jp_fetch, contents, contents_dir, path, name, _check_created): nbname = name + ".ipynb" @@ -550,6 +729,24 @@ async def test_delete_non_empty_dir(jp_fetch, contents): await jp_fetch("api", "contents", "å b", method="GET") assert expected_http_error(e, 404) +@pytest.mark.skipif(sys.platform == "win32", reason="Disabled deleting hidden dirs on Windows") +async def test_delete_hidden_dir(jp_fetch, contents): + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch("api", "contents", ".hidden", method="DELETE") + assert expected_http_error(e, 400) + +@pytest.mark.skipif(sys.platform == "win32", reason="Disabled deleting hidden dirs on Windows") +async def test_delete_hidden_file(jp_fetch, contents): + #Test deleting file in a hidden directory + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch("api", "contents", ".hidden/test.txt", method="DELETE") + assert expected_http_error(e, 400) + + #Test deleting a hidden file + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + await jp_fetch("api", "contents", ".hidden.txt", method="DELETE") + assert expected_http_error(e, 400) + async def test_rename(jp_fetch, jp_base_url, contents, contents_dir): path = "foo" @@ -581,6 +778,60 @@ async def test_rename(jp_fetch, jp_base_url, contents, contents_dir): assert "z.ipynb" in nbnames assert "a.ipynb" not in nbnames +@pytest.mark.skipif(sys.platform == "win32", reason="Disabled copying hidden files on Windows") +async def test_rename_400_hidden(jp_fetch, jp_base_url, contents, contents_dir): + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + old_path = '.hidden/old.txt' + new_path = 'new.txt' + # Rename the file + r = await jp_fetch( + "api", + "contents", + old_path, + method="PATCH", + body=json.dumps({"path": new_path}), + ) + assert expected_http_error(e, 400) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + old_path = 'old.txt' + new_path = '.hidden/new.txt' + # Rename the file + r = await jp_fetch( + "api", + "contents", + old_path, + method="PATCH", + body=json.dumps({"path": new_path}), + ) + assert expected_http_error(e, 400) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + old_path = '.hidden.txt' + new_path = 'new.txt' + # Rename the file + r = await jp_fetch( + "api", + "contents", + old_path, + method="PATCH", + body=json.dumps({"path": new_path}), + ) + assert expected_http_error(e, 400) + + with pytest.raises(tornado.httpclient.HTTPClientError) as e: + old_path = 'old.txt' + new_path = '.hidden.txt' + # Rename the file + r = await jp_fetch( + "api", + "contents", + old_path, + method="PATCH", + body=json.dumps({"path": new_path}), + ) + assert expected_http_error(e, 400) + async def test_checkpoints_follow_file(jp_fetch, contents): path = "foo" diff --git a/tests/services/contents/test_manager.py b/tests/services/contents/test_manager.py index e3b37642b3..57c06135f8 100644 --- a/tests/services/contents/test_manager.py +++ b/tests/services/contents/test_manager.py @@ -259,6 +259,166 @@ async def test_403(jp_file_contents_manager_class, tmp_path): except HTTPError as e: assert e.status_code == 403 +@pytest.mark.skipif(sys.platform.startswith('win'), reason="Can't test hidden files on Windows") +async def test_400(jp_file_contents_manager_class, tmp_path): + #Test Delete behavior + #Test delete of file in hidden directory + with pytest.raises(HTTPError) as excinfo: + td = str(tmp_path) + cm = jp_file_contents_manager_class(root_dir=td) + hidden_dir = '.hidden' + file_in_hidden_path = os.path.join(hidden_dir,'visible.txt') + _make_dir(cm, hidden_dir) + model = await ensure_async(cm.new(path=file_in_hidden_path)) + os_path = cm._get_os_path(model['path']) + + try: + result = await ensure_async(cm.delete_file(os_path)) + except HTTPError as e: + assert e.status_code == 400 + + #Test delete hidden file in visible directory + with pytest.raises(HTTPError) as excinfo: + td = str(tmp_path) + cm = jp_file_contents_manager_class(root_dir=td) + hidden_dir = 'visible' + file_in_hidden_path = os.path.join(hidden_dir,'.hidden.txt') + _make_dir(cm, hidden_dir) + model = await ensure_async(cm.new(path=file_in_hidden_path)) + os_path = cm._get_os_path(model['path']) + + try: + result = await ensure_async(cm.delete_file(os_path)) + except HTTPError as e: + assert e.status_code == 400 + + #Test Save behavior + #Test save of file in hidden directory + with pytest.raises(HTTPError) as excinfo: + td = str(tmp_path) + cm = jp_file_contents_manager_class(root_dir=td) + hidden_dir = '.hidden' + file_in_hidden_path = os.path.join(hidden_dir,'visible.txt') + _make_dir(cm, hidden_dir) + model = await ensure_async(cm.new(path=file_in_hidden_path)) + os_path = cm._get_os_path(model['path']) + + try: + result = await ensure_async(cm.save(model,path=os_path)) + except HTTPError as e: + assert e.status_code == 400 + + #Test save hidden file in visible directory + with pytest.raises(HTTPError) as excinfo: + td = str(tmp_path) + cm = jp_file_contents_manager_class(root_dir=td) + hidden_dir = 'visible' + file_in_hidden_path = os.path.join(hidden_dir,'.hidden.txt') + _make_dir(cm, hidden_dir) + model = await ensure_async(cm.new(path=file_in_hidden_path)) + os_path = cm._get_os_path(model['path']) + + try: + result = await ensure_async(cm.save(model,path=os_path)) + except HTTPError as e: + assert e.status_code == 400 + + #Test rename behavior + #Test rename with source file in hidden directory + with pytest.raises(HTTPError) as excinfo: + td = str(tmp_path) + cm = jp_file_contents_manager_class(root_dir=td) + hidden_dir = '.hidden' + file_in_hidden_path = os.path.join(hidden_dir,'visible.txt') + _make_dir(cm, hidden_dir) + model = await ensure_async(cm.new(path=file_in_hidden_path)) + old_path = cm._get_os_path(model['path']) + new_path = "new.txt" + + try: + result = await ensure_async(cm.rename_file(old_path, new_path)) + except HTTPError as e: + assert e.status_code == 400 + + #Test rename of dest file in hidden directory + with pytest.raises(HTTPError) as excinfo: + td = str(tmp_path) + cm = jp_file_contents_manager_class(root_dir=td) + hidden_dir = '.hidden' + file_in_hidden_path = os.path.join(hidden_dir,'visible.txt') + _make_dir(cm, hidden_dir) + model = await ensure_async(cm.new(path=file_in_hidden_path)) + new_path = cm._get_os_path(model['path']) + old_path = "old.txt" + + try: + result = await ensure_async(cm.rename_file(old_path, new_path)) + except HTTPError as e: + assert e.status_code == 400 + + #Test rename with hidden source file in visible directory + with pytest.raises(HTTPError) as excinfo: + td = str(tmp_path) + cm = jp_file_contents_manager_class(root_dir=td) + hidden_dir = 'visible' + file_in_hidden_path = os.path.join(hidden_dir,'.hidden.txt') + _make_dir(cm, hidden_dir) + model = await ensure_async(cm.new(path=file_in_hidden_path)) + old_path = cm._get_os_path(model['path']) + new_path = "new.txt" + + try: + result = await ensure_async(cm.rename_file(old_path, new_path)) + except HTTPError as e: + assert e.status_code == 400 + + #Test rename with hidden dest file in visible directory + with pytest.raises(HTTPError) as excinfo: + td = str(tmp_path) + cm = jp_file_contents_manager_class(root_dir=td) + hidden_dir = 'visible' + file_in_hidden_path = os.path.join(hidden_dir,'.hidden.txt') + _make_dir(cm, hidden_dir) + model = await ensure_async(cm.new(path=file_in_hidden_path)) + new_path = cm._get_os_path(model['path']) + old_path = "old.txt" + + try: + result = await ensure_async(cm.rename_file(old_path, new_path)) + except HTTPError as e: + assert e.status_code == 400 + +@pytest.mark.skipif(sys.platform.startswith('win'), reason="Can't test hidden files on Windows") +async def test_404(jp_file_contents_manager_class, tmp_path): + #Test visible file in hidden folder + with pytest.raises(HTTPError) as excinfo: + td = str(tmp_path) + cm = jp_file_contents_manager_class(root_dir=td) + hidden_dir = '.hidden' + file_in_hidden_path = os.path.join(hidden_dir,'visible.txt') + _make_dir(cm, hidden_dir) + model = await ensure_async(cm.new(path=file_in_hidden_path)) + os_path = cm._get_os_path(model['path']) + + try: + result = await ensure_async(cm.get(os_path, 'w')) + except HTTPError as e: + assert e.status_code == 404 + + #Test hidden file in visible folder + with pytest.raises(HTTPError) as excinfo: + td = str(tmp_path) + cm = jp_file_contents_manager_class(root_dir=td) + hidden_dir = 'visible' + file_in_hidden_path = os.path.join(hidden_dir,'.hidden.txt') + _make_dir(cm, hidden_dir) + model = await ensure_async(cm.new(path=file_in_hidden_path)) + os_path = cm._get_os_path(model['path']) + + try: + result = await ensure_async(cm.get(os_path, 'w')) + except HTTPError as e: + assert e.status_code == 404 async def test_escape_root(jp_file_contents_manager_class, tmp_path): td = str(tmp_path)