From fd3709dff217a9f8cf4484188c9c802209d29a69 Mon Sep 17 00:00:00 2001 From: Zong Xun Date: Thu, 19 Oct 2023 23:16:17 +0800 Subject: [PATCH 01/12] Deny git access to nested server root --- jupyterlab_git/git.py | 11 +++++++++-- jupyterlab_git/handlers.py | 3 ++- src/model.ts | 18 +++++++++++++++++- src/tokens.ts | 4 ++++ src/utils.ts | 13 +++++++++++++ 5 files changed, 45 insertions(+), 4 deletions(-) diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py index 5454db9f9..3dd6a856c 100644 --- a/jupyterlab_git/git.py +++ b/jupyterlab_git/git.py @@ -946,7 +946,7 @@ async def show_top_level(self, path): "message": my_error, } - async def show_prefix(self, path): + async def show_prefix(self, path, contents_manager): """ Execute git --show-prefix command & return the result. """ @@ -955,8 +955,15 @@ async def show_prefix(self, path): cmd, cwd=path, ) + + relative_path = os.path.relpath(path, contents_manager.root_dir) + if code == 0: - result = {"code": code, "path": my_output.strip("\n")} + result = { + "code": code, + "path": my_output.strip("\n"), + "relative_path": relative_path, + } return result else: # Handle special case where cwd not inside a git repo diff --git a/jupyterlab_git/handlers.py b/jupyterlab_git/handlers.py index 35098bfec..b64a0c367 100644 --- a/jupyterlab_git/handlers.py +++ b/jupyterlab_git/handlers.py @@ -174,7 +174,8 @@ async def post(self, path: str = ""): POST request handler, displays the prefix path of a directory in a repository, with respect to the root directory. """ - result = await self.git.show_prefix(self.url2localpath(path)) + local_path, cm = self.url2localpath(path, with_contents_manager=True) + result = await self.git.show_prefix(local_path, cm) if result["code"] != 0: self.set_status(500) diff --git a/src/model.ts b/src/model.ts index 4358c08b8..f45dd4094 100644 --- a/src/model.ts +++ b/src/model.ts @@ -8,7 +8,7 @@ import { ISignal, Signal } from '@lumino/signaling'; import { AUTH_ERROR_MESSAGES, requestAPI } from './git'; import { TaskHandler } from './taskhandler'; import { Git, IGitExtension } from './tokens'; -import { decodeStage } from './utils'; +import { arraysAreEqual, decodeStage } from './utils'; // Default refresh interval (in milliseconds) for polling the current Git status (NOTE: this value should be the same value as in the plugin settings schema): const DEFAULT_REFRESH_INTERVAL = 3000; // ms @@ -1473,6 +1473,22 @@ export class GitExtension implements IGitExtension { ); } ); + if (data?.relative_path && data?.path) { + if (data?.relative_path === '.') { + if (data.path !== '') { + return null; + } + } else { + if ( + !arraysAreEqual( + data.relative_path.split('\\'), + data.path.split('/').slice(0, -1) // slicing accounts for additional "/" at end of path + ) + ) { + return null; + } + } + } return data.path ?? null; } catch (error) { if ( diff --git a/src/tokens.ts b/src/tokens.ts index 467d4fd96..b2cc97317 100644 --- a/src/tokens.ts +++ b/src/tokens.ts @@ -903,6 +903,10 @@ export namespace Git { * Relative path of the current folder within its Git repository */ path?: string; + /** + * Relative path of the root folder + */ + relative_path?: string; } /** diff --git a/src/utils.ts b/src/utils.ts index fa421600a..b89b8eaff 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -123,3 +123,16 @@ export const openFileDiff = } } }; + +/** Shallow comparison of two arrays */ +export function arraysAreEqual(array1: string[], array2: string[]) { + if (array1.length !== array2.length) { + return false; + } + for (let i = 0; i < array1.length; i++) { + if (array1[i] !== array2[i]) { + return false; + } + } + return true; +} From 5dfe0455aa43d5e9f88344ee2d8ba6423cbdd4a8 Mon Sep 17 00:00:00 2001 From: Zong Xun Date: Thu, 19 Oct 2023 23:20:57 +0800 Subject: [PATCH 02/12] Clean up code --- jupyterlab_git/git.py | 4 +--- src/model.ts | 7 ++----- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py index 3dd6a856c..94fe0af6f 100644 --- a/jupyterlab_git/git.py +++ b/jupyterlab_git/git.py @@ -956,13 +956,11 @@ async def show_prefix(self, path, contents_manager): cwd=path, ) - relative_path = os.path.relpath(path, contents_manager.root_dir) - if code == 0: result = { "code": code, "path": my_output.strip("\n"), - "relative_path": relative_path, + "relative_path": os.path.relpath(path, contents_manager.root_dir), } return result else: diff --git a/src/model.ts b/src/model.ts index f45dd4094..745d52f67 100644 --- a/src/model.ts +++ b/src/model.ts @@ -1474,10 +1474,8 @@ export class GitExtension implements IGitExtension { } ); if (data?.relative_path && data?.path) { - if (data?.relative_path === '.') { - if (data.path !== '') { - return null; - } + if (data?.relative_path === '.' && data.path !== '') { + return null; } else { if ( !arraysAreEqual( @@ -1809,7 +1807,6 @@ export class GitExtension implements IGitExtension { * @throws {ServerConnection.NetworkError} If the request cannot be made */ async tags(): Promise { - const path = await this._getPathRepository(); return await this._taskHandler.execute( 'git:tag:list', async () => { From dbb3cdd16bef92abb6b3a20fdea8ac602700a2d7 Mon Sep 17 00:00:00 2001 From: Zong Xun Date: Wed, 25 Oct 2023 11:26:11 +0800 Subject: [PATCH 03/12] Shift code logic to server side --- jupyterlab_git/git.py | 15 ++++++++++++--- src/model.ts | 16 +--------------- src/tokens.ts | 4 ---- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py index 94fe0af6f..f4d983ca2 100644 --- a/jupyterlab_git/git.py +++ b/jupyterlab_git/git.py @@ -957,12 +957,21 @@ async def show_prefix(self, path, contents_manager): ) if code == 0: + relative_git_path = my_output.strip("\n") + relative_server_path = os.path.relpath(path, contents_manager.root_dir) + result = { "code": code, - "path": my_output.strip("\n"), - "relative_path": os.path.relpath(path, contents_manager.root_dir), + "path": relative_git_path, + "relative_path": relative_server_path, } - return result + + if relative_server_path == "." and relative_git_path == "": + return result + elif relative_git_path.split("/")[:-1] == relative_server_path.split("\\"): + return result + else: + return {"code": code, "path": None} else: # Handle special case where cwd not inside a git repo lower_error = my_error.lower() diff --git a/src/model.ts b/src/model.ts index 745d52f67..101587ba4 100644 --- a/src/model.ts +++ b/src/model.ts @@ -8,7 +8,7 @@ import { ISignal, Signal } from '@lumino/signaling'; import { AUTH_ERROR_MESSAGES, requestAPI } from './git'; import { TaskHandler } from './taskhandler'; import { Git, IGitExtension } from './tokens'; -import { arraysAreEqual, decodeStage } from './utils'; +import { decodeStage } from './utils'; // Default refresh interval (in milliseconds) for polling the current Git status (NOTE: this value should be the same value as in the plugin settings schema): const DEFAULT_REFRESH_INTERVAL = 3000; // ms @@ -1473,20 +1473,6 @@ export class GitExtension implements IGitExtension { ); } ); - if (data?.relative_path && data?.path) { - if (data?.relative_path === '.' && data.path !== '') { - return null; - } else { - if ( - !arraysAreEqual( - data.relative_path.split('\\'), - data.path.split('/').slice(0, -1) // slicing accounts for additional "/" at end of path - ) - ) { - return null; - } - } - } return data.path ?? null; } catch (error) { if ( diff --git a/src/tokens.ts b/src/tokens.ts index b2cc97317..467d4fd96 100644 --- a/src/tokens.ts +++ b/src/tokens.ts @@ -903,10 +903,6 @@ export namespace Git { * Relative path of the current folder within its Git repository */ path?: string; - /** - * Relative path of the root folder - */ - relative_path?: string; } /** From feb4e749bc6078b17b6e3fac330575bc32104548 Mon Sep 17 00:00:00 2001 From: Zong Xun Date: Wed, 25 Oct 2023 11:26:32 +0800 Subject: [PATCH 04/12] Remove redundant utility function --- src/utils.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index b89b8eaff..fa421600a 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -123,16 +123,3 @@ export const openFileDiff = } } }; - -/** Shallow comparison of two arrays */ -export function arraysAreEqual(array1: string[], array2: string[]) { - if (array1.length !== array2.length) { - return false; - } - for (let i = 0; i < array1.length; i++) { - if (array1[i] !== array2[i]) { - return false; - } - } - return true; -} From 4281cb01aa928ceed688f06fd369b1ce59052d5d Mon Sep 17 00:00:00 2001 From: Zong Xun Date: Fri, 10 Nov 2023 00:17:51 -0500 Subject: [PATCH 05/12] Restore unintended line deletion --- src/model.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/model.ts b/src/model.ts index 101587ba4..4358c08b8 100644 --- a/src/model.ts +++ b/src/model.ts @@ -1793,6 +1793,7 @@ export class GitExtension implements IGitExtension { * @throws {ServerConnection.NetworkError} If the request cannot be made */ async tags(): Promise { + const path = await this._getPathRepository(); return await this._taskHandler.execute( 'git:tag:list', async () => { From eee28dd958ea4d775bf11e6af5f62e72fc875e1a Mon Sep 17 00:00:00 2001 From: Zong Xun Date: Fri, 10 Nov 2023 00:31:00 -0500 Subject: [PATCH 06/12] Enable path parsing for windows and linux --- jupyterlab_git/git.py | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py index f4d983ca2..049e89c05 100644 --- a/jupyterlab_git/git.py +++ b/jupyterlab_git/git.py @@ -958,20 +958,26 @@ async def show_prefix(self, path, contents_manager): if code == 0: relative_git_path = my_output.strip("\n") - relative_server_path = os.path.relpath(path, contents_manager.root_dir) - - result = { - "code": code, - "path": relative_git_path, - "relative_path": relative_server_path, - } - - if relative_server_path == "." and relative_git_path == "": - return result - elif relative_git_path.split("/")[:-1] == relative_server_path.split("\\"): - return result + repository_path = path[: -len(relative_git_path)] + try: + # Raise an error is the repository_path is not a subpath of root_dir + Path(repository_path).absolute().relative_to( + Path(contents_manager.root_dir).absolute() + ) + except ValueError: + return { + "code": code, + "path": None, + "relative_path": relative_git_path, + "repo_path": repository_path, + "root_dir": contents_manager.root_dir, + } else: - return {"code": code, "path": None} + result = { + "code": code, + "path": relative_git_path, + } + return result else: # Handle special case where cwd not inside a git repo lower_error = my_error.lower() From c6558a855d03abb273b89d608c4e54ab0af7f2da Mon Sep 17 00:00:00 2001 From: Zong Xun Date: Fri, 10 Nov 2023 09:14:27 -0500 Subject: [PATCH 07/12] Remove un-needed response attributes --- .gitignore | 5 ++++- jupyterlab_git/git.py | 2 -- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 5c82327c6..6bbc24fa7 100644 --- a/.gitignore +++ b/.gitignore @@ -135,4 +135,7 @@ dmypy.json .vscode # vim stuff -*.swp \ No newline at end of file +*.swp + +# virtual env +venv/ diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py index aaf465501..dffca0de9 100644 --- a/jupyterlab_git/git.py +++ b/jupyterlab_git/git.py @@ -973,8 +973,6 @@ async def show_prefix(self, path, contents_manager): "code": code, "path": None, "relative_path": relative_git_path, - "repo_path": repository_path, - "root_dir": contents_manager.root_dir, } else: result = { From aca902900cf2ff967470dc8aafce140979429dd8 Mon Sep 17 00:00:00 2001 From: Zong Xun Date: Fri, 10 Nov 2023 09:38:22 -0500 Subject: [PATCH 08/12] Add test case --- jupyterlab_git/git.py | 1 - jupyterlab_git/tests/test_handlers.py | 48 +++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py index dffca0de9..ebcbca479 100644 --- a/jupyterlab_git/git.py +++ b/jupyterlab_git/git.py @@ -972,7 +972,6 @@ async def show_prefix(self, path, contents_manager): return { "code": code, "path": None, - "relative_path": relative_git_path, } else: result = { diff --git a/jupyterlab_git/tests/test_handlers.py b/jupyterlab_git/tests/test_handlers.py index 07b7d80ac..737357501 100644 --- a/jupyterlab_git/tests/test_handlers.py +++ b/jupyterlab_git/tests/test_handlers.py @@ -10,6 +10,8 @@ from .testutils import assert_http_error, maybe_future from tornado.httpclient import HTTPClientError +from pathlib import Path + def test_mapping_added(): mock_web_app = Mock() @@ -112,6 +114,52 @@ async def test_git_show_prefix(mock_execute, jp_fetch, jp_root_dir): ) +@patch("jupyterlab_git.git.execute") +async def test_git_show_prefix_nested_directory(mock_execute, jp_fetch, jp_root_dir): + # Given + path = "path/to/repo" + + local_path = jp_root_dir / "test_path" + + mock_execute.return_value = maybe_future((0, str(path), "")) + + # When + response = await jp_fetch( + NAMESPACE, + local_path.name + "/subfolder", + "show_prefix", + body="{}", + method="POST", + ) + + # Then + assert response.code == 200 + payload = json.loads(response.body) + assert payload["path"] == str(path) + + mock_execute.assert_has_calls( + [ + call( + ["git", "rev-parse", "--show-prefix"], + cwd=str(local_path / "subfolder"), + timeout=20, + env=None, + username=None, + password=None, + is_binary=False, + ), + ] + ) + + try: + Path(path).absolute().relative_to(Path(local_path).absolute()) + except ValueError: + # pass the test if the path is not a subdirectory of the root directory + pass + else: + assert False, "The path should not be a subdirectory of the root directory" + + async def test_git_show_prefix_for_excluded_path( jp_fetch, jp_server_config, jp_root_dir ): From 5519cc2639717be7a210518f93e50dabd60bd25e Mon Sep 17 00:00:00 2001 From: Zong Xun Date: Mon, 20 Nov 2023 11:22:44 -0500 Subject: [PATCH 09/12] Replace test for new case --- jupyterlab_git/tests/test_handlers.py | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/jupyterlab_git/tests/test_handlers.py b/jupyterlab_git/tests/test_handlers.py index 737357501..6a6f5fc37 100644 --- a/jupyterlab_git/tests/test_handlers.py +++ b/jupyterlab_git/tests/test_handlers.py @@ -116,32 +116,23 @@ async def test_git_show_prefix(mock_execute, jp_fetch, jp_root_dir): @patch("jupyterlab_git.git.execute") async def test_git_show_prefix_nested_directory(mock_execute, jp_fetch, jp_root_dir): - # Given - path = "path/to/repo" - - local_path = jp_root_dir / "test_path" - - mock_execute.return_value = maybe_future((0, str(path), "")) - + mock_execute.return_value = maybe_future((0, f"{jp_root_dir.name}/", "")) # When response = await jp_fetch( NAMESPACE, - local_path.name + "/subfolder", "show_prefix", body="{}", method="POST", ) - # Then assert response.code == 200 payload = json.loads(response.body) - assert payload["path"] == str(path) - + assert payload["path"] is None mock_execute.assert_has_calls( [ call( ["git", "rev-parse", "--show-prefix"], - cwd=str(local_path / "subfolder"), + cwd=str(jp_root_dir), timeout=20, env=None, username=None, @@ -151,14 +142,6 @@ async def test_git_show_prefix_nested_directory(mock_execute, jp_fetch, jp_root_ ] ) - try: - Path(path).absolute().relative_to(Path(local_path).absolute()) - except ValueError: - # pass the test if the path is not a subdirectory of the root directory - pass - else: - assert False, "The path should not be a subdirectory of the root directory" - async def test_git_show_prefix_for_excluded_path( jp_fetch, jp_server_config, jp_root_dir From 1fd4458eb6327fe44afd9035a7e3ca6a2a930977 Mon Sep 17 00:00:00 2001 From: Zong Xun <63457492+Zxun2@users.noreply.github.com> Date: Wed, 22 Nov 2023 09:09:10 +0800 Subject: [PATCH 10/12] Add trailing slash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Frédéric Collonval --- jupyterlab_git/tests/test_handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyterlab_git/tests/test_handlers.py b/jupyterlab_git/tests/test_handlers.py index 6a6f5fc37..0c8c9ca4f 100644 --- a/jupyterlab_git/tests/test_handlers.py +++ b/jupyterlab_git/tests/test_handlers.py @@ -132,7 +132,7 @@ async def test_git_show_prefix_nested_directory(mock_execute, jp_fetch, jp_root_ [ call( ["git", "rev-parse", "--show-prefix"], - cwd=str(jp_root_dir), + cwd=str(jp_root_dir) + "/", timeout=20, env=None, username=None, From 63f33befb69ca71db8b18c93223faba4113a882a Mon Sep 17 00:00:00 2001 From: Zong Xun <63457492+Zxun2@users.noreply.github.com> Date: Wed, 22 Nov 2023 09:09:29 +0800 Subject: [PATCH 11/12] Fix testcase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Frédéric Collonval --- jupyterlab_git/git.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py index 186a19c7a..be4a79a11 100644 --- a/jupyterlab_git/git.py +++ b/jupyterlab_git/git.py @@ -962,7 +962,7 @@ async def show_prefix(self, path, contents_manager): if code == 0: relative_git_path = my_output.strip("\n") - repository_path = path[: -len(relative_git_path)] + repository_path = path[: -len(relative_git_path)] if relative_git_path else path try: # Raise an error is the repository_path is not a subpath of root_dir Path(repository_path).absolute().relative_to( From 32d10ee3f6e9d16efd2663117da08b77c50b4c37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= Date: Wed, 29 Nov 2023 17:28:45 +0100 Subject: [PATCH 12/12] Fix format --- jupyterlab_git/git.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py index be4a79a11..645a43128 100644 --- a/jupyterlab_git/git.py +++ b/jupyterlab_git/git.py @@ -962,7 +962,9 @@ async def show_prefix(self, path, contents_manager): if code == 0: relative_git_path = my_output.strip("\n") - repository_path = path[: -len(relative_git_path)] if relative_git_path else path + repository_path = ( + path[: -len(relative_git_path)] if relative_git_path else path + ) try: # Raise an error is the repository_path is not a subpath of root_dir Path(repository_path).absolute().relative_to(