Deny git access to nested server root

[Zxun2]

Fixes #1205

Changes

1. IShowPrefixResult now returns an additional attribute relative_path which determines the current path relative to the server root directory.
2. Using relative_path, i compared the server root to the git root. The main logic is that if relative_path == "." then you simply check if path === "". This would be an indication that server root is equal to the git root since path is relative to git root. Otherwise, determine if normalize(relative_path) === normalize(path). If git root is indeed equal to server root, then relative path must be equal to the path itself.

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.

You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[github-actions]

👈 Launch a Binder on branch Zxun2/jupyterlab-git/zxun2/deny-git-subdirectory-access

[fcollonval]

@Zxun2 the main branch has been updated to JupyterLab 4 - to ease the work here, I changed the target to the branch jlab-3 to avoid complex rebasing.

[Zxun2]

I have made the code ever slightly more compact by shifting all the relevant logic to server side. Let me know if there are any more changes to be made!

[fcollonval]

Thanks @Zxun2

I looked a bit at the new pathlib library that is much better than os.path. So here is an alternative proposal.

Would you mind adding a test for this? You can look at that test as an example to start a new one.

[Zxun2]

Hey @fcollonval! Thanks for the review. I tried looking into the test case that you linked to, but i am a little confused how the function works. Both test_git_show_prefix and test_git_show_prefix_not_a_git_repo appears to have the similar method calls and code logic but has different assertions (expected return value). It'll be great if you can give some insights on how these method calls work!

For example,

local_path = jp_root_dir / "test_path"

Changing "test_path" to any other string causes the testcase to fail, but i am unsure why. Are these strings predetermined (from the codebase, i only see "test_path" and "ignored_path")?

[fcollonval]

Thanks @Zxun2

The difference between the two tests is the mocked result from the git command:

• Returning a valid answer in test_git_show_prefix

jupyterlab-git/jupyterlab_git/tests/test_handlers.py

Line 85 in a901350

mock_execute.return_value = maybe_future((0, str(path), ""))

• Returning an error in test_git_show_prefix_not_a_git_repo

jupyterlab-git/jupyterlab_git/tests/test_handlers.py

Line 138 in a901350

(128, "", "fatal: not a git repository (or any")

So in your case you will need to return a valid answer as the error must be raised from the relative path computation.

https://github.com/Zxun2/jupyterlab-git/blob/eee28dd958ea4d775bf11e6af5f62e72fc875e1a/jupyterlab_git/git.py#L967

[Zxun2]

Hey @fcollonval, thanks for the quick response! I merged the main repository, hence the huge LoC changes

[fcollonval]

Kicking the CI following targeted branch change

[fcollonval]

Almost through, here are some suggestions to fix the CI

[Zxun2]

@fcollonval I am done with this PR!

[fcollonval]

Thanks @Zxun2

[welcome]

Congrats on your first merged pull request in this project! 🎉

Thank you for contributing, we are very proud of you! ❤️

[fcollonval]

@all-contributors please add @Zxun2 for code

[allcontributors]

@fcollonval

I've put up a pull request to add @Zxun2! 🎉