Fix rename_file to handle relative paths properly (related to GHSA-v7vq-3x77-87vg?)

[yacchin1205]

Fix #6473 and I assume that this completely resolves GHSA-v7vq-3x77-87vg.

As @jinzhen-lin mentions in #6473, if a path of the same length as notebook_dir is given in rename_file, it is interpreted as an attempt to rename a file named .ipynb, which is misjudged as a hidden file because the first character is ..
This problem could be caused by old_path and new_path of rename_file are relative paths, but is_hidden is assumed to be an absolute path. Therefore, I modified the code of rename_file so that absolute paths are provided to is_hidden.

Additionally, the test was also fixed. The test passed until now, but it did not test if rename_file raises the expected error. Because the FileContentsManager.new before it raised HTTPError and marks it as success. (is_hidden is also called in new.)

[Related to the security advisory?]
The original commit seems to be jupyter-server/jupyter_server@877da10 on jupyter_server. (This commit seems to handle absolute paths properly.) This appears to be related to Security Advisory GHSA-v7vq-3x77-87vg So I consider this PR may be an important fix.

[RRosio]

Hi @yacchin1205, thank you for submitting this PR! One point of interest that has come up while speaking with @echarles, would it be possible to update the tests to be more in line with the testing done for this functionality in jupyter-server? After this PR is merged it would be great to have these changes added to the 6.5.x branch as well!

[yacchin1205]

Thank you for your reply, @RRosio . OK, I will update the tests!

[yacchin1205]

@RRosio I referred to jupyter-server and found the same problem in the test of jupyter-server, so I fixed the test and found the (new) problem 😓 .
I made a Pull Request for the issue. jupyter-server/jupyter_server#1073 .
I will wait for it to be accepted and try to fix this Pull Request as well.

[yacchin1205]

Hi @RRosio , The fixes to the test on jupyter-server have also been merged, and I have modified this test accordingly. Please review it.

[RRosio]

Hi @yacchin1205 thank you for implementing these updates. I have brought up this PR in the notebook community meeting and there was interest in a little more testing on our end to ensure that these updates have no other side-effects.

[RRosio]

Hi @yacchin1205, thank you again for your work with this update. One question we have is in regards to the order of the is_hidden function call in the delete_file methods. The changes recently merged into jupyter_server mean the is_hidden checks happen at a different point than in Notebook. Would you please share with us the reason this may be necessary? Thank you in advance!

[yacchin1205]

Hi @RRosio , The reason why delete_file checks for hidden files before checking for existence, I understand, is due to a problem that allows some files (e.g. .ssh/id_rsa) to be checked for existence.
(So I assume that the current implementation of rename_file also checks whether the file is hidden first.)

If you would like to add the same fix to delete_file of jupyter_server here, I can add it this PR. How about it?

[RRosio]

Thank you for your response @yacchin1205!

If you would like to add the same fix to delete_file of jupyter_server here, I can add it this PR. How about it?

Yes, if you could please add that fix here, that would be great!

[yacchin1205]

Hi @RRosio , I have committed a fix regarding delete_file. Please review it!

[echarles]

I have tested the latest commit and confirms the hidden files are not shown on my env and can not be addressed directly.

[RRosio]

Thank you for the updates @yacchin1205! This looks great! Feel free to let us know whether you would like to submit these same updates to 6.5.x as well.

[yacchin1205]

Thank you for the merge @RRosio ! I have also created a Pull Request for 6.5.x (PR #6660). I would appreciate it if you could check that as well.