I don't know how to safely test it via a shell script in tests/, so if
someone
versed in Haskell unit testing could add a test checking something like
makeSubPathOf (AbsolutePath "/") (AbsolutePath "/dir") = Just (SubPath
"dir")
...

Attachments

• resolve-issue2537_-allow-_-as-root-directory.dpatch

It occurred to me that this patch possibly doesn't work or could even do 
harmful things in Windows, depending on how file paths are represented.
More specifically:

1. if absolute paths are in the form '/<volume>/<path>' (unlikely, I 
think), this patch does evil things(tm), like considering '/D/file' to 
be part of '/C/' repo.

2. if absolute paths are in the form '<volume>:\<path>' and the root of 
a repo created in the top dir of a volume is '<volume>:\', i.e.: 
'C:\file' and 'C:\' respectively, this patch fixes the problem on POSIX 
but the issue still stands on Windows.

3. if absolute paths are in the form '<volume>:\<path>' and the root of 
a repo created in the top dir of a volume is '<volume>:', i.e.: 
'C:\file' and 'C:'(note the lack of slash) respectively, this patch 
fixes the problem on POSIX. The issue was not present on Windows nor 
this patch introduces a regression.

My guess is the right answer is the number 2, but I would be grateful if 
someone could create a repo on the top dir of a volume and let me know 
the content of the 'Root:' line in the output of `darcs show repo`.

Thanks,
Gian Piero.

PS: 'direction' (forward or backward) of the slashes is irrelevant as, 
if I've read correctly the code, all backward slashes are transformed to 
forward ones before comparing the paths.

As mentionned by GianPiero in http://bugs.darcs.net/patch1634 , a fix
for this shall be tested to work well under both Linux/Unixes and Windows.

No follow-ups since almost a year. Besides, I think it is a bad idea to
allow that for obvious security reasons.

Resurrecting a very old thread...

* [Thu, Jan 24, 2019 at 03:35:29PM +0000] Ben Franksen:
>No follow-ups since almost a year. Besides, I think it is a bad idea to
>allow that for obvious security reasons.

I'm not complaining about the rejection of the patch, and I still 
neither have nor plan to have access to a Windows machine, so I've given 
up on this.

Anyway I do not agree that there's a security risk specifically arising 
from running darcs on the root. I'm pretty sure you're thinking about 
running it as the root user, but that's a privileges issue that also 
affect, e.g. /etc. In other worlds, I do not think there are security 
problems that could only show when running darcs on the root dir and do 
not manifest themselves in any other directories.

I agree that managing "/" with a VCS is not *necessarily* a security 
risk per se. Can you come up with a hypothetical scenario where I 
would want to do that on purpose?

* [Sat, Jan 07, 2023 at 12:01:53PM +0000] Ben Franksen:
>I agree that managing "/" with a VCS is not *necessarily* a security
>risk per se. Can you come up with a hypothetical scenario where I
>would want to do that on purpose?

According to OP, it was in order "to track changes to an
operating system" [http://bugs.darcs.net/msg19805].

I've thought about using version control to directly manage config files (mostly in /etc) but haven't actually tried doing it. I'm not necessarily advocating for the patch; just came across this thread.

Hm, so the idea is to support something like etckeeper but not limited 
to /etc. That might indeed work. Let's reopen this if and when there 
is actual demand for this.

makeSubPathOf is no longer used for this, so the patch doesn't work 
anyway. Instead we now have makeRelativeTo and that works just fine 
with "/". Ineed I tested manually that the issue is fixed with current 
screened.