List for cgit developers and users
 help / color / mirror / Atom feed
* cgit and symlinks
@ 2017-03-06 10:42 oliver+cgit
  2017-03-06 23:35 ` john
  0 siblings, 1 reply; 10+ messages in thread
From: oliver+cgit @ 2017-03-06 10:42 UTC (permalink / raw)


Hey list,

first off, thanks for git, it is an amazing product and I've been a very 
happy user for years.

Having said that, I was playing around with a simple script and noticed 
wget gave 404's on symlinks.

As an example:

http://git.schinagl.nl/debootstrap.git/plain/scripts/stretch

at the very least I would expect the content of the link as for example 
github shows:
https://raw.githubusercontent.com/oliv3r/debootstrap/master/scripts/stretch

having said that, even github handles it wrong imo, as I would expect 
the webserver to 'get' a symlink and thus just follow the symlink and 
thus get the content of the file (in raw/plain mode).

I have tried googeling and checked cgitrc for my 0.12 but have not found 
anything yet.

Did I simply miss something configuration wise?

Thanks,

Olliver


^ permalink raw reply	[flat|nested] 10+ messages in thread

* cgit and symlinks
  2017-03-06 10:42 cgit and symlinks oliver+cgit
@ 2017-03-06 23:35 ` john
  2017-03-08 11:38   ` i
  0 siblings, 1 reply; 10+ messages in thread
From: john @ 2017-03-06 23:35 UTC (permalink / raw)


On Mon, Mar 06, 2017 at 11:42:14AM +0100, Olliver Schinagl wrote:
> Having said that, I was playing around with a simple script and noticed 
> wget gave 404's on symlinks.
> 
> As an example:
> 
> http://git.schinagl.nl/debootstrap.git/plain/scripts/stretch
> 
> at the very least I would expect the content of the link as for example 
> github shows:
> https://raw.githubusercontent.com/oliv3r/debootstrap/master/scripts/stretch
> 
> having said that, even github handles it wrong imo, as I would expect 
> the webserver to 'get' a symlink and thus just follow the symlink and 
> thus get the content of the file (in raw/plain mode).
> 
> I have tried googeling and checked cgitrc for my 0.12 but have not found 
> anything yet.
> 
> Did I simply miss something configuration wise?

Nope, it looks like we simply ignore symlinks in the plain UI.  Below is
a patch to fix this by printing the content in the same we as in the
tree UI.

We can't reliably follow the link because there is no guarantee that the
target lies within the repository and I don't know what we would output
for the case where we can't display the target.

-- >8 --
Subject: [PATCH] ui-plain: print symlink content

We currently ignore symlinks in ui-plain, leading to a 404.  In ui-tree
we print the content of the blob (that is, the path to the target of the
link), so it makes sense to do the same here.

Signed-off-by: John Keeping <john at keeping.me.uk>
---
 ui-plain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui-plain.c b/ui-plain.c
index 8d541e3..01e8b36 100644
--- a/ui-plain.c
+++ b/ui-plain.c
@@ -135,7 +135,7 @@ static int walk_tree(const unsigned char *sha1, struct strbuf *base,
 	struct walk_tree_context *walk_tree_ctx = cbdata;
 
 	if (base->len == walk_tree_ctx->match_baselen) {
-		if (S_ISREG(mode)) {
+		if (S_ISREG(mode) || S_ISLNK(mode)) {
 			if (print_object(sha1, pathname))
 				walk_tree_ctx->match = 1;
 		} else if (S_ISDIR(mode)) {
-- 
2.12.0.377.gf910686b23.dirty



^ permalink raw reply	[flat|nested] 10+ messages in thread

* cgit and symlinks
  2017-03-06 23:35 ` john
@ 2017-03-08 11:38   ` i
  2017-03-08 12:30     ` john
  0 siblings, 1 reply; 10+ messages in thread
From: i @ 2017-03-08 11:38 UTC (permalink / raw)


Am 07.03.2017 um 00:35 schrieb John Keeping:
> We can't reliably follow the link because there is no guarantee that the
> target lies within the repository and I don't know what we would output
> for the case where we can't display the target.

INADH (I'm not a dev here)

I would recommend to continue ignoring it or returning the blob, because
following symlinks (internally) might result -  if not done carefully -
in directory traversal security issues. Maybe resolving a symlink to a
HTTP301 could work.

For the UI there might be a html-link (in a notification box "This is a
symlink that points to ...") to the symlink-destination below or above
the blob, to get a user via click to a file/directory.

Regards,
MonkZ

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20170308/1daf3f21/attachment.asc>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* cgit and symlinks
  2017-03-08 11:38   ` i
@ 2017-03-08 12:30     ` john
  2017-03-08 13:28       ` i
  0 siblings, 1 reply; 10+ messages in thread
From: john @ 2017-03-08 12:30 UTC (permalink / raw)


On Wed, Mar 08, 2017 at 12:38:38PM +0100, MonkZ wrote:
> Am 07.03.2017 um 00:35 schrieb John Keeping:
> > We can't reliably follow the link because there is no guarantee that the
> > target lies within the repository and I don't know what we would output
> > for the case where we can't display the target.
> 
> INADH (I'm not a dev here)
> 
> I would recommend to continue ignoring it or returning the blob, because
> following symlinks (internally) might result -  if not done carefully -
> in directory traversal security issues. Maybe resolving a symlink to a
> HTTP301 could work.
> 
> For the UI there might be a html-link (in a notification box "This is a
> symlink that points to ...") to the symlink-destination below or above
> the blob, to get a user via click to a file/directory.

We're talking about the "plain" UI here (for example [0]), so we don't
have anywhere to put additional content and it has to be something
basic.

I'm not actually too worried about directory traversal if we were to try
following links because we're looking things up in a Git tree at a
particular commit and not on the filesystem.  A bigger concern would be
whether the internals of Git do anything bad (like invalid memory
access) if we give the tree traversal machinery a path that goes up out
of the repository; I doubt it but I have not checked.

[0] https://git.zx2c4.com/cgit/plain/README


^ permalink raw reply	[flat|nested] 10+ messages in thread

* cgit and symlinks
  2017-03-08 12:30     ` john
@ 2017-03-08 13:28       ` i
  2017-03-09  0:15         ` john
  0 siblings, 1 reply; 10+ messages in thread
From: i @ 2017-03-08 13:28 UTC (permalink / raw)




Am 08.03.2017 um 13:30 schrieb John Keeping:
> On Wed, Mar 08, 2017 at 12:38:38PM +0100, MonkZ wrote:
>> Am 07.03.2017 um 00:35 schrieb John Keeping:
>>> We can't reliably follow the link because there is no guarantee that the
>>> target lies within the repository and I don't know what we would output
>>> for the case where we can't display the target.
>>
>> INADH (I'm not a dev here)
>>
>> I would recommend to continue ignoring it or returning the blob, because
>> following symlinks (internally) might result -  if not done carefully -
>> in directory traversal security issues. Maybe resolving a symlink to a
>> HTTP301 could work.
>>
>> For the UI there might be a html-link (in a notification box "This is a
>> symlink that points to ...") to the symlink-destination below or above
>> the blob, to get a user via click to a file/directory.
> 
> We're talking about the "plain" UI here (for example [0]), so we don't
> have anywhere to put additional content and it has to be something
> basic.
Of course. It would be handled like a content-rewrite to return a http301.

Pseudocode:
handle_symlinks = True # new config item
if this_file_is_a_symlink and symlink_is_relative and handle_symlinks:
	if plain_ui:
		# rewrite blob to http301
		# by attaching the path to the end of current basedir
		# cgit is already able to handle ../ in a path
	if !plain_ui:
		# show blob
		# show notification that this is a symlink
		# show a link to a url
		# 	like the one that would be used in plain_ui

> 
> I'm not actually too worried about directory traversal if we were to try
> following links because we're looking things up in a Git tree at a
> particular commit and not on the filesystem.  A bigger concern would be
> whether the internals of Git do anything bad (like invalid memory
> access) if we give the tree traversal machinery a path that goes up out
> of the repository; I doubt it but I have not checked.
If we use url-rewrites (and let the http-client care about getting the
correct file or directory), this would be a non-issue.

MfG
MonkZ

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20170308/72ae087a/attachment.asc>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* cgit and symlinks
  2017-03-08 13:28       ` i
@ 2017-03-09  0:15         ` john
  2017-03-09  7:58           ` i
  0 siblings, 1 reply; 10+ messages in thread
From: john @ 2017-03-09  0:15 UTC (permalink / raw)


On Wed, Mar 08, 2017 at 02:28:11PM +0100, MonkZ wrote:
> 
> 
> Am 08.03.2017 um 13:30 schrieb John Keeping:
> > On Wed, Mar 08, 2017 at 12:38:38PM +0100, MonkZ wrote:
> >> Am 07.03.2017 um 00:35 schrieb John Keeping:
> >>> We can't reliably follow the link because there is no guarantee that the
> >>> target lies within the repository and I don't know what we would output
> >>> for the case where we can't display the target.
> >>
> >> INADH (I'm not a dev here)
> >>
> >> I would recommend to continue ignoring it or returning the blob, because
> >> following symlinks (internally) might result -  if not done carefully -
> >> in directory traversal security issues. Maybe resolving a symlink to a
> >> HTTP301 could work.
> >>
> >> For the UI there might be a html-link (in a notification box "This is a
> >> symlink that points to ...") to the symlink-destination below or above
> >> the blob, to get a user via click to a file/directory.
> > 
> > We're talking about the "plain" UI here (for example [0]), so we don't
> > have anywhere to put additional content and it has to be something
> > basic.
> Of course. It would be handled like a content-rewrite to return a http301.
> 
> Pseudocode:
> handle_symlinks = True # new config item
> if this_file_is_a_symlink and symlink_is_relative and handle_symlinks:
> 	if plain_ui:
> 		# rewrite blob to http301
> 		# by attaching the path to the end of current basedir
> 		# cgit is already able to handle ../ in a path
> 	if !plain_ui:
> 		# show blob
> 		# show notification that this is a symlink
> 		# show a link to a url
> 		# 	like the one that would be used in plain_ui
> 
> > 
> > I'm not actually too worried about directory traversal if we were to try
> > following links because we're looking things up in a Git tree at a
> > particular commit and not on the filesystem.  A bigger concern would be
> > whether the internals of Git do anything bad (like invalid memory
> > access) if we give the tree traversal machinery a path that goes up out
> > of the repository; I doubt it but I have not checked.
> If we use url-rewrites (and let the http-client care about getting the
> correct file or directory), this would be a non-issue.

It could also mean that cross-repository symlinks work if the server
layout matches that that is expected for checkouts of the repositories.

But it's not exactly helpful if a repository contains an absolute
symlink and I don't think we want to start figuring out whether a
redirect makes sense - what do we do if we decide it doesn't?


^ permalink raw reply	[flat|nested] 10+ messages in thread

* cgit and symlinks
  2017-03-09  0:15         ` john
@ 2017-03-09  7:58           ` i
  2017-03-12 14:18             ` john
  0 siblings, 1 reply; 10+ messages in thread
From: i @ 2017-03-09  7:58 UTC (permalink / raw)




Am 09.03.2017 um 01:15 schrieb John Keeping:
> On Wed, Mar 08, 2017 at 02:28:11PM +0100, MonkZ wrote:
>>
>>
>> Am 08.03.2017 um 13:30 schrieb John Keeping:
>>> On Wed, Mar 08, 2017 at 12:38:38PM +0100, MonkZ wrote:
>>>> Am 07.03.2017 um 00:35 schrieb John Keeping:
>>>>> We can't reliably follow the link because there is no guarantee that the
>>>>> target lies within the repository and I don't know what we would output
>>>>> for the case where we can't display the target.
>>>>
>>>> INADH (I'm not a dev here)
>>>>
>>>> I would recommend to continue ignoring it or returning the blob, because
>>>> following symlinks (internally) might result -  if not done carefully -
>>>> in directory traversal security issues. Maybe resolving a symlink to a
>>>> HTTP301 could work.
>>>>
>>>> For the UI there might be a html-link (in a notification box "This is a
>>>> symlink that points to ...") to the symlink-destination below or above
>>>> the blob, to get a user via click to a file/directory.
>>>
>>> We're talking about the "plain" UI here (for example [0]), so we don't
>>> have anywhere to put additional content and it has to be something
>>> basic.
>> Of course. It would be handled like a content-rewrite to return a http301.
>>
>> Pseudocode:
>> handle_symlinks = True # new config item
>> if this_file_is_a_symlink and symlink_is_relative and handle_symlinks:
>> 	if plain_ui:
>> 		# rewrite blob to http301
>> 		# by attaching the path to the end of current basedir
>> 		# cgit is already able to handle ../ in a path
>> 	if !plain_ui:
>> 		# show blob
>> 		# show notification that this is a symlink
>> 		# show a link to a url
>> 		# 	like the one that would be used in plain_ui
>>
>>>
>>> I'm not actually too worried about directory traversal if we were to try
>>> following links because we're looking things up in a Git tree at a
>>> particular commit and not on the filesystem.  A bigger concern would be
>>> whether the internals of Git do anything bad (like invalid memory
>>> access) if we give the tree traversal machinery a path that goes up out
>>> of the repository; I doubt it but I have not checked.
>> If we use url-rewrites (and let the http-client care about getting the
>> correct file or directory), this would be a non-issue.
> 
> It could also mean that cross-repository symlinks work if the server
> layout matches that that is expected for checkouts of the repositories.
> 
> But it's not exactly helpful if a repository contains an absolute
> symlink and I don't think we want to start figuring out whether a
> redirect makes sense - what do we do if we decide it doesn't?
> 

Absolute symlinks must be ignored. There is no deterministic way to
resolve them - every clone can be at a different location, and there
isn't really a deterministic mapping from url to filesystem. Absolute
symlinks would only work if resolved internally - with additional
security risks.

Relative inter-repository links may allowed/handled/redirected if
explicitly configured, otherwise it might be confusing if the server
layout doesn't match. On the other hand a notification "This is a
symlink outside this repository" might suffice (but i don't have a plan
for plain-ui).

MfG
MonkZ

P.S.: Interrepository links sounds a lot like resolving submodules - but
i think that is too much for cgit.

MfG
MonkZ

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20170309/6a564611/attachment.asc>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* cgit and symlinks
  2017-03-09  7:58           ` i
@ 2017-03-12 14:18             ` john
  2017-03-13  8:59               ` i
  2017-03-21  4:56               ` dlcampbell
  0 siblings, 2 replies; 10+ messages in thread
From: john @ 2017-03-12 14:18 UTC (permalink / raw)


On Thu, Mar 09, 2017 at 08:58:43AM +0100, MonkZ wrote:
> Am 09.03.2017 um 01:15 schrieb John Keeping:
> > On Wed, Mar 08, 2017 at 02:28:11PM +0100, MonkZ wrote:
> >>
> >>
> >> Am 08.03.2017 um 13:30 schrieb John Keeping:
> >>> On Wed, Mar 08, 2017 at 12:38:38PM +0100, MonkZ wrote:
> >>>> Am 07.03.2017 um 00:35 schrieb John Keeping:
> >>>>> We can't reliably follow the link because there is no guarantee that the
> >>>>> target lies within the repository and I don't know what we would output
> >>>>> for the case where we can't display the target.
> >>>>
> >>>> INADH (I'm not a dev here)
> >>>>
> >>>> I would recommend to continue ignoring it or returning the blob, because
> >>>> following symlinks (internally) might result -  if not done carefully -
> >>>> in directory traversal security issues. Maybe resolving a symlink to a
> >>>> HTTP301 could work.
> >>>>
> >>>> For the UI there might be a html-link (in a notification box "This is a
> >>>> symlink that points to ...") to the symlink-destination below or above
> >>>> the blob, to get a user via click to a file/directory.
> >>>
> >>> We're talking about the "plain" UI here (for example [0]), so we don't
> >>> have anywhere to put additional content and it has to be something
> >>> basic.
> >> Of course. It would be handled like a content-rewrite to return a http301.
> >>
> >> Pseudocode:
> >> handle_symlinks = True # new config item
> >> if this_file_is_a_symlink and symlink_is_relative and handle_symlinks:
> >> 	if plain_ui:
> >> 		# rewrite blob to http301
> >> 		# by attaching the path to the end of current basedir
> >> 		# cgit is already able to handle ../ in a path
> >> 	if !plain_ui:
> >> 		# show blob
> >> 		# show notification that this is a symlink
> >> 		# show a link to a url
> >> 		# 	like the one that would be used in plain_ui
> >>
> >>>
> >>> I'm not actually too worried about directory traversal if we were to try
> >>> following links because we're looking things up in a Git tree at a
> >>> particular commit and not on the filesystem.  A bigger concern would be
> >>> whether the internals of Git do anything bad (like invalid memory
> >>> access) if we give the tree traversal machinery a path that goes up out
> >>> of the repository; I doubt it but I have not checked.
> >> If we use url-rewrites (and let the http-client care about getting the
> >> correct file or directory), this would be a non-issue.
> > 
> > It could also mean that cross-repository symlinks work if the server
> > layout matches that that is expected for checkouts of the repositories.
> > 
> > But it's not exactly helpful if a repository contains an absolute
> > symlink and I don't think we want to start figuring out whether a
> > redirect makes sense - what do we do if we decide it doesn't?
> > 
> 
> Absolute symlinks must be ignored. There is no deterministic way to
> resolve them - every clone can be at a different location, and there
> isn't really a deterministic mapping from url to filesystem. Absolute
> symlinks would only work if resolved internally - with additional
> security risks.
> 
> Relative inter-repository links may allowed/handled/redirected if
> explicitly configured, otherwise it might be confusing if the server
> layout doesn't match. On the other hand a notification "This is a
> symlink outside this repository" might suffice (but i don't have a plan
> for plain-ui).

We can consider improvements to the tree UI separately, but I really
don't think we should be getting into anything clever with symlinks in
the plain UI because it ends up with complicated rules like the above.

It's difficult to explain and will end up surprising users, so my
preference is for my original patch that just displays the content of
the blob when a symlink is found.  This is consistent with both
"git show" and "git cat-file".


^ permalink raw reply	[flat|nested] 10+ messages in thread

* cgit and symlinks
  2017-03-12 14:18             ` john
@ 2017-03-13  8:59               ` i
  2017-03-21  4:56               ` dlcampbell
  1 sibling, 0 replies; 10+ messages in thread
From: i @ 2017-03-13  8:59 UTC (permalink / raw)




Am 12.03.2017 um 15:18 schrieb John Keeping:
> On Thu, Mar 09, 2017 at 08:58:43AM +0100, MonkZ wrote:
>> Am 09.03.2017 um 01:15 schrieb John Keeping:
>>> On Wed, Mar 08, 2017 at 02:28:11PM +0100, MonkZ wrote:
>>>>
>>>>
>>>> Am 08.03.2017 um 13:30 schrieb John Keeping:
>>>>> On Wed, Mar 08, 2017 at 12:38:38PM +0100, MonkZ wrote:
>>>>>> Am 07.03.2017 um 00:35 schrieb John Keeping:
>>>>>>> We can't reliably follow the link because there is no guarantee that the
>>>>>>> target lies within the repository and I don't know what we would output
>>>>>>> for the case where we can't display the target.
>>>>>>
>>>>>> INADH (I'm not a dev here)
>>>>>>
>>>>>> I would recommend to continue ignoring it or returning the blob, because
>>>>>> following symlinks (internally) might result -  if not done carefully -
>>>>>> in directory traversal security issues. Maybe resolving a symlink to a
>>>>>> HTTP301 could work.
>>>>>>
>>>>>> For the UI there might be a html-link (in a notification box "This is a
>>>>>> symlink that points to ...") to the symlink-destination below or above
>>>>>> the blob, to get a user via click to a file/directory.
>>>>>
>>>>> We're talking about the "plain" UI here (for example [0]), so we don't
>>>>> have anywhere to put additional content and it has to be something
>>>>> basic.
>>>> Of course. It would be handled like a content-rewrite to return a http301.
>>>>
>>>> Pseudocode:
>>>> handle_symlinks = True # new config item
>>>> if this_file_is_a_symlink and symlink_is_relative and handle_symlinks:
>>>> 	if plain_ui:
>>>> 		# rewrite blob to http301
>>>> 		# by attaching the path to the end of current basedir
>>>> 		# cgit is already able to handle ../ in a path
>>>> 	if !plain_ui:
>>>> 		# show blob
>>>> 		# show notification that this is a symlink
>>>> 		# show a link to a url
>>>> 		# 	like the one that would be used in plain_ui
>>>>
>>>>>
>>>>> I'm not actually too worried about directory traversal if we were to try
>>>>> following links because we're looking things up in a Git tree at a
>>>>> particular commit and not on the filesystem.  A bigger concern would be
>>>>> whether the internals of Git do anything bad (like invalid memory
>>>>> access) if we give the tree traversal machinery a path that goes up out
>>>>> of the repository; I doubt it but I have not checked.
>>>> If we use url-rewrites (and let the http-client care about getting the
>>>> correct file or directory), this would be a non-issue.
>>>
>>> It could also mean that cross-repository symlinks work if the server
>>> layout matches that that is expected for checkouts of the repositories.
>>>
>>> But it's not exactly helpful if a repository contains an absolute
>>> symlink and I don't think we want to start figuring out whether a
>>> redirect makes sense - what do we do if we decide it doesn't?
>>>
>>
>> Absolute symlinks must be ignored. There is no deterministic way to
>> resolve them - every clone can be at a different location, and there
>> isn't really a deterministic mapping from url to filesystem. Absolute
>> symlinks would only work if resolved internally - with additional
>> security risks.
>>
>> Relative inter-repository links may allowed/handled/redirected if
>> explicitly configured, otherwise it might be confusing if the server
>> layout doesn't match. On the other hand a notification "This is a
>> symlink outside this repository" might suffice (but i don't have a plan
>> for plain-ui).
> 
> We can consider improvements to the tree UI separately, but I really
> don't think we should be getting into anything clever with symlinks in
> the plain UI because it ends up with complicated rules like the above.
> 
> It's difficult to explain and will end up surprising users, so my
> preference is for my original patch that just displays the content of
> the blob when a symlink is found.  This is consistent with both
> "git show" and "git cat-file".

Yep, it's a tough decision to make - full support of symlinks(in my eyes
can't be done to serve all usecases), only relative symlinks or none.

One argument for "only relative symlinks":
Olliver Schinagl had his git to debian-repository usecase, that would
still be possible if he copies the files in place of the symlink - git
compression would take care of deduplication. On the other hand checking
out said repository/branch could lead to a full filesystem.

Config option would be s.th. like "handle_relative_symlinks" or offering
a "symlink filter" hook for users to write own scripts.


One argument for "none":
"do one thing and do it well" and if we can't handle absolute symlinks,
we should leave them alone.

MfG
MonkZ

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20170313/aa44a83b/attachment.asc>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* cgit and symlinks
  2017-03-12 14:18             ` john
  2017-03-13  8:59               ` i
@ 2017-03-21  4:56               ` dlcampbell
  1 sibling, 0 replies; 10+ messages in thread
From: dlcampbell @ 2017-03-21  4:56 UTC (permalink / raw)


On 03/12/2017 07:18 AM, John Keeping wrote:
> On Thu, Mar 09, 2017 at 08:58:43AM +0100, MonkZ wrote:
>> Am 09.03.2017 um 01:15 schrieb John Keeping:
>>> On Wed, Mar 08, 2017 at 02:28:11PM +0100, MonkZ wrote:
>>>>
>>>>
>>>> Am 08.03.2017 um 13:30 schrieb John Keeping:
>>>>> On Wed, Mar 08, 2017 at 12:38:38PM +0100, MonkZ wrote:
>>>>>> Am 07.03.2017 um 00:35 schrieb John Keeping:
>>>>>>> We can't reliably follow the link because there is no guarantee that the
>>>>>>> target lies within the repository and I don't know what we would output
>>>>>>> for the case where we can't display the target.
>>>>>>
>>>>>> INADH (I'm not a dev here)
>>>>>>
>>>>>> I would recommend to continue ignoring it or returning the blob, because
>>>>>> following symlinks (internally) might result -  if not done carefully -
>>>>>> in directory traversal security issues. Maybe resolving a symlink to a
>>>>>> HTTP301 could work.
>>>>>>
>>>>>> For the UI there might be a html-link (in a notification box "This is a
>>>>>> symlink that points to ...") to the symlink-destination below or above
>>>>>> the blob, to get a user via click to a file/directory.
>>>>>
>>>>> We're talking about the "plain" UI here (for example [0]), so we don't
>>>>> have anywhere to put additional content and it has to be something
>>>>> basic.
>>>> Of course. It would be handled like a content-rewrite to return a http301.
>>>>
>>>> Pseudocode:
>>>> handle_symlinks = True # new config item
>>>> if this_file_is_a_symlink and symlink_is_relative and handle_symlinks:
>>>> 	if plain_ui:
>>>> 		# rewrite blob to http301
>>>> 		# by attaching the path to the end of current basedir
>>>> 		# cgit is already able to handle ../ in a path
>>>> 	if !plain_ui:
>>>> 		# show blob
>>>> 		# show notification that this is a symlink
>>>> 		# show a link to a url
>>>> 		# 	like the one that would be used in plain_ui
>>>>
>>>>>
>>>>> I'm not actually too worried about directory traversal if we were to try
>>>>> following links because we're looking things up in a Git tree at a
>>>>> particular commit and not on the filesystem.  A bigger concern would be
>>>>> whether the internals of Git do anything bad (like invalid memory
>>>>> access) if we give the tree traversal machinery a path that goes up out
>>>>> of the repository; I doubt it but I have not checked.
>>>> If we use url-rewrites (and let the http-client care about getting the
>>>> correct file or directory), this would be a non-issue.
>>>
>>> It could also mean that cross-repository symlinks work if the server
>>> layout matches that that is expected for checkouts of the repositories.
>>>
>>> But it's not exactly helpful if a repository contains an absolute
>>> symlink and I don't think we want to start figuring out whether a
>>> redirect makes sense - what do we do if we decide it doesn't?
>>>
>>
>> Absolute symlinks must be ignored. There is no deterministic way to
>> resolve them - every clone can be at a different location, and there
>> isn't really a deterministic mapping from url to filesystem. Absolute
>> symlinks would only work if resolved internally - with additional
>> security risks.
>>
>> Relative inter-repository links may allowed/handled/redirected if
>> explicitly configured, otherwise it might be confusing if the server
>> layout doesn't match. On the other hand a notification "This is a
>> symlink outside this repository" might suffice (but i don't have a plan
>> for plain-ui).
>
> We can consider improvements to the tree UI separately, but I really
> don't think we should be getting into anything clever with symlinks in
> the plain UI because it ends up with complicated rules like the above.
>
> It's difficult to explain and will end up surprising users, so my
> preference is for my original patch that just displays the content of
> the blob when a symlink is found.  This is consistent with both
> "git show" and "git cat-file".
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit
>

I like this idea. It's intuitive. If the symlink points to a file in the 
repo, obviously we should display it. In the event a symlink points 
outside the repo, could we do something similar to `file foo.symlink`'s 
output and print the path it's pointing to? That would remain 
informative and (afaik) won't jeopardize security because we're not 
actually following an out-of-repo symlink.


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-03-21  4:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-06 10:42 cgit and symlinks oliver+cgit
2017-03-06 23:35 ` john
2017-03-08 11:38   ` i
2017-03-08 12:30     ` john
2017-03-08 13:28       ` i
2017-03-09  0:15         ` john
2017-03-09  7:58           ` i
2017-03-12 14:18             ` john
2017-03-13  8:59               ` i
2017-03-21  4:56               ` dlcampbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).