Patchwork [3,of,4] lfs: add an experimental knob to disable blob serving

login
register
mail settings
Submitter Matt Harbison
Date March 31, 2018, 10:28 p.m.
Message ID <f3e750cdf0b3fb1977b7.1522535306@Envy>
Download mbox | patch
Permalink /patch/30081/
State Accepted
Headers show

Comments

Matt Harbison - March 31, 2018, 10:28 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1522524043 14400
#      Sat Mar 31 15:20:43 2018 -0400
# Node ID f3e750cdf0b3fb1977b7ea0f7685d2a86d3c199c
# Parent  40f0b221aee73bca2072812062d0f112a134bedf
lfs: add an experimental knob to disable blob serving

The use case here is the server admin may want to store the blobs elsewhere.  As
it stands now, the `lfs.url` config on the client side is all that enforces this
(the web.allow-* permissions aren't able to block LFS blobs without also
blocking normal hg traffic).  The real solution to this is to implement the
'verify' action on the client and server, but that's not a near term goal.
Whether this is useful in its own right, and should be promoted out of
experimental at some point is TBD.

Since the other two tests that deal with LFS and `hg serve` are already complex
and have #testcases, this seems like a good time to start a new test dedicated
to access checks against the server.  I believe that accessing some random URI
(that isn't a protocol API) will 404 before the access checks, so that's what I
did here, for simplicity.
Matt Harbison - March 31, 2018, 11:33 p.m.
On second thought, I can probably just not wrap the wire protocol function.  But the other patches here shouldn’t depend on this.

> On Mar 31, 2018, at 6:28 PM, Matt Harbison <mharbison72@gmail.com> wrote:
> 
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1522524043 14400
> #      Sat Mar 31 15:20:43 2018 -0400
> # Node ID f3e750cdf0b3fb1977b7ea0f7685d2a86d3c199c
> # Parent  40f0b221aee73bca2072812062d0f112a134bedf
> lfs: add an experimental knob to disable blob serving
> 
> The use case here is the server admin may want to store the blobs elsewhere.  As
> it stands now, the `lfs.url` config on the client side is all that enforces this
> (the web.allow-* permissions aren't able to block LFS blobs without also
> blocking normal hg traffic).  The real solution to this is to implement the
> 'verify' action on the client and server, but that's not a near term goal.
> Whether this is useful in its own right, and should be promoted out of
> experimental at some point is TBD.
> 
> Since the other two tests that deal with LFS and `hg serve` are already complex
> and have #testcases, this seems like a good time to start a new test dedicated
> to access checks against the server.  I believe that accessing some random URI
> (that isn't a protocol API) will 404 before the access checks, so that's what I
> did here, for simplicity.
> 
> diff --git a/hgext/lfs/__init__.py b/hgext/lfs/__init__.py
> --- a/hgext/lfs/__init__.py
> +++ b/hgext/lfs/__init__.py
> @@ -166,6 +166,9 @@ testedwith = 'ships-with-hg-core'
> configtable = {}
> configitem = registrar.configitem(configtable)
> 
> +configitem('experimental', 'lfs.serve',
> +    default=True,
> +)
> configitem('experimental', 'lfs.user-agent',
>     default=None,
> )
> diff --git a/hgext/lfs/wireprotolfsserver.py b/hgext/lfs/wireprotolfsserver.py
> --- a/hgext/lfs/wireprotolfsserver.py
> +++ b/hgext/lfs/wireprotolfsserver.py
> @@ -35,12 +35,22 @@ def handlewsgirequest(orig, rctx, req, r
>         return False
> 
>     try:
> +        serve = rctx.repo.ui.configbool('experimental', 'lfs.serve')
> +
>         if req.dispatchpath == b'.git/info/lfs/objects/batch':
> +            if not serve:
> +                res.status = hgwebcommon.statusmessage(404)
> +                res.setbodybytes(b'')
> +                return True
>             checkperm(rctx, req, 'pull')
>             return _processbatchrequest(rctx.repo, req, res)
>         # TODO: reserve and use a path in the proposed http wireprotocol /api/
>         #       namespace?
>         elif req.dispatchpath.startswith(b'.hg/lfs/objects'):
> +            if not serve:
> +                res.status = hgwebcommon.statusmessage(404)
> +                res.setbodybytes(b'')
> +                return True
>             return _processbasictransfer(rctx.repo, req, res,
>                                          lambda perm:
>                                                 checkperm(rctx, req, perm))
> diff --git a/tests/test-lfs-serve-access.t b/tests/test-lfs-serve-access.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-lfs-serve-access.t
> @@ -0,0 +1,67 @@
> +#require serve
> +
> +  $ cat >> $HGRCPATH <<EOF
> +  > [extensions]
> +  > lfs=
> +  > [lfs]
> +  > url=http://localhost:$HGPORT/.git/info/lfs
> +  > track=all()
> +  > [web]
> +  > push_ssl = False
> +  > allow-push = *
> +  > EOF
> +
> +Serving LFS files can experimentally be turned off.  The long term solution is
> +to support the 'verify' action in both client and server, so that the server can
> +tell the client to store files elsewhere.
> +
> +  $ hg init server
> +  $ hg --config "lfs.usercache=$TESTTMP/servercache" \
> +  >    --config experimental.lfs.serve=False -R server serve -d \
> +  >    -p $HGPORT --pid-file=hg.pid -A $TESTTMP/access.log -E $TESTTMP/errors.log
> +  $ cat hg.pid >> $DAEMON_PIDS
> +
> +Uploads fail...
> +
> +  $ hg init client
> +  $ echo 'this-is-an-lfs-file' > client/lfs.bin
> +  $ hg -R client ci -Am 'initial commit'
> +  adding lfs.bin
> +  $ hg -R client push http://localhost:$HGPORT
> +  pushing to http://localhost:$HGPORT/
> +  searching for changes
> +  abort: LFS HTTP error: HTTP Error 404: Not Found (action=upload)!
> +  [255]
> +
> +... so do a local push to make the data available.  Remove the blob from the
> +default cache, so it attempts to download.
> +  $ hg --config "lfs.usercache=$TESTTMP/servercache" \
> +  >    --config "lfs.url=null://" \
> +  >    -R client push -q server
> +  $ rm -rf `hg config lfs.usercache`
> +
> +Downloads fail...
> +
> +  $ hg clone http://localhost:$HGPORT httpclone
> +  requesting all changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 1 changes to 1 files
> +  new changesets 525251863cad
> +  updating to branch default
> +  abort: LFS HTTP error: HTTP Error 404: Not Found (action=download)!
> +  [255]
> +
> +  $ $PYTHON $RUNTESTDIR/killdaemons.py $DAEMON_PIDS
> +
> +  $ cat $TESTTMP/access.log $TESTTMP/errors.log
> +  $LOCALIP - - [$LOGDATE$] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> +  $LOCALIP - - [$LOGDATE$] "GET /?cmd=batch HTTP/1.1" 200 - x-hgarg-1:cmds=heads+%3Bknown+nodes%3D525251863cad618e55d483555f3d00a2ca99597e x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ (glob)
> +  $LOCALIP - - [$LOGDATE$] "GET /?cmd=listkeys HTTP/1.1" 200 - x-hgarg-1:namespace=phases x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ (glob)
> +  $LOCALIP - - [$LOGDATE$] "GET /?cmd=listkeys HTTP/1.1" 200 - x-hgarg-1:namespace=bookmarks x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ (glob)
> +  $LOCALIP - - [$LOGDATE$] "POST /.git/info/lfs/objects/batch HTTP/1.1" 404 - (glob)
> +  $LOCALIP - - [$LOGDATE$] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> +  $LOCALIP - - [$LOGDATE$] "GET /?cmd=batch HTTP/1.1" 200 - x-hgarg-1:cmds=heads+%3Bknown+nodes%3D x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ (glob)
> +  $LOCALIP - - [$LOGDATE$] "GET /?cmd=getbundle HTTP/1.1" 200 - x-hgarg-1:bookmarks=1&bundlecaps=HG20%2Cbundle2%3DHG20%250Abookmarks%250Achangegroup%253D01%252C02%252C03%250Adigests%253Dmd5%252Csha1%252Csha512%250Aerror%253Dabort%252Cunsupportedcontent%252Cpushraced%252Cpushkey%250Ahgtagsfnodes%250Alistkeys%250Aphases%253Dheads%250Apushkey%250Aremote-changegroup%253Dhttp%252Chttps%250Arev-branch-cache%250Astream%253Dv2&cg=1&common=0000000000000000000000000000000000000000&heads=525251863cad618e55d483555f3d00a2ca99597e&listkeys=bookmarks&phases=1 x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ (glob)
> +  $LOCALIP - - [$LOGDATE$] "POST /.git/info/lfs/objects/batch HTTP/1.1" 404 - (glob)

Patch

diff --git a/hgext/lfs/__init__.py b/hgext/lfs/__init__.py
--- a/hgext/lfs/__init__.py
+++ b/hgext/lfs/__init__.py
@@ -166,6 +166,9 @@  testedwith = 'ships-with-hg-core'
 configtable = {}
 configitem = registrar.configitem(configtable)
 
+configitem('experimental', 'lfs.serve',
+    default=True,
+)
 configitem('experimental', 'lfs.user-agent',
     default=None,
 )
diff --git a/hgext/lfs/wireprotolfsserver.py b/hgext/lfs/wireprotolfsserver.py
--- a/hgext/lfs/wireprotolfsserver.py
+++ b/hgext/lfs/wireprotolfsserver.py
@@ -35,12 +35,22 @@  def handlewsgirequest(orig, rctx, req, r
         return False
 
     try:
+        serve = rctx.repo.ui.configbool('experimental', 'lfs.serve')
+
         if req.dispatchpath == b'.git/info/lfs/objects/batch':
+            if not serve:
+                res.status = hgwebcommon.statusmessage(404)
+                res.setbodybytes(b'')
+                return True
             checkperm(rctx, req, 'pull')
             return _processbatchrequest(rctx.repo, req, res)
         # TODO: reserve and use a path in the proposed http wireprotocol /api/
         #       namespace?
         elif req.dispatchpath.startswith(b'.hg/lfs/objects'):
+            if not serve:
+                res.status = hgwebcommon.statusmessage(404)
+                res.setbodybytes(b'')
+                return True
             return _processbasictransfer(rctx.repo, req, res,
                                          lambda perm:
                                                 checkperm(rctx, req, perm))
diff --git a/tests/test-lfs-serve-access.t b/tests/test-lfs-serve-access.t
new file mode 100644
--- /dev/null
+++ b/tests/test-lfs-serve-access.t
@@ -0,0 +1,67 @@ 
+#require serve
+
+  $ cat >> $HGRCPATH <<EOF
+  > [extensions]
+  > lfs=
+  > [lfs]
+  > url=http://localhost:$HGPORT/.git/info/lfs
+  > track=all()
+  > [web]
+  > push_ssl = False
+  > allow-push = *
+  > EOF
+
+Serving LFS files can experimentally be turned off.  The long term solution is
+to support the 'verify' action in both client and server, so that the server can
+tell the client to store files elsewhere.
+
+  $ hg init server
+  $ hg --config "lfs.usercache=$TESTTMP/servercache" \
+  >    --config experimental.lfs.serve=False -R server serve -d \
+  >    -p $HGPORT --pid-file=hg.pid -A $TESTTMP/access.log -E $TESTTMP/errors.log
+  $ cat hg.pid >> $DAEMON_PIDS
+
+Uploads fail...
+
+  $ hg init client
+  $ echo 'this-is-an-lfs-file' > client/lfs.bin
+  $ hg -R client ci -Am 'initial commit'
+  adding lfs.bin
+  $ hg -R client push http://localhost:$HGPORT
+  pushing to http://localhost:$HGPORT/
+  searching for changes
+  abort: LFS HTTP error: HTTP Error 404: Not Found (action=upload)!
+  [255]
+
+... so do a local push to make the data available.  Remove the blob from the
+default cache, so it attempts to download.
+  $ hg --config "lfs.usercache=$TESTTMP/servercache" \
+  >    --config "lfs.url=null://" \
+  >    -R client push -q server
+  $ rm -rf `hg config lfs.usercache`
+
+Downloads fail...
+
+  $ hg clone http://localhost:$HGPORT httpclone
+  requesting all changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  new changesets 525251863cad
+  updating to branch default
+  abort: LFS HTTP error: HTTP Error 404: Not Found (action=download)!
+  [255]
+
+  $ $PYTHON $RUNTESTDIR/killdaemons.py $DAEMON_PIDS
+
+  $ cat $TESTTMP/access.log $TESTTMP/errors.log
+  $LOCALIP - - [$LOGDATE$] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
+  $LOCALIP - - [$LOGDATE$] "GET /?cmd=batch HTTP/1.1" 200 - x-hgarg-1:cmds=heads+%3Bknown+nodes%3D525251863cad618e55d483555f3d00a2ca99597e x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ (glob)
+  $LOCALIP - - [$LOGDATE$] "GET /?cmd=listkeys HTTP/1.1" 200 - x-hgarg-1:namespace=phases x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ (glob)
+  $LOCALIP - - [$LOGDATE$] "GET /?cmd=listkeys HTTP/1.1" 200 - x-hgarg-1:namespace=bookmarks x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ (glob)
+  $LOCALIP - - [$LOGDATE$] "POST /.git/info/lfs/objects/batch HTTP/1.1" 404 - (glob)
+  $LOCALIP - - [$LOGDATE$] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
+  $LOCALIP - - [$LOGDATE$] "GET /?cmd=batch HTTP/1.1" 200 - x-hgarg-1:cmds=heads+%3Bknown+nodes%3D x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ (glob)
+  $LOCALIP - - [$LOGDATE$] "GET /?cmd=getbundle HTTP/1.1" 200 - x-hgarg-1:bookmarks=1&bundlecaps=HG20%2Cbundle2%3DHG20%250Abookmarks%250Achangegroup%253D01%252C02%252C03%250Adigests%253Dmd5%252Csha1%252Csha512%250Aerror%253Dabort%252Cunsupportedcontent%252Cpushraced%252Cpushkey%250Ahgtagsfnodes%250Alistkeys%250Aphases%253Dheads%250Apushkey%250Aremote-changegroup%253Dhttp%252Chttps%250Arev-branch-cache%250Astream%253Dv2&cg=1&common=0000000000000000000000000000000000000000&heads=525251863cad618e55d483555f3d00a2ca99597e&listkeys=bookmarks&phases=1 x-hgproto-1:0.1 0.2 comp=$USUAL_COMPRESSIONS$ (glob)
+  $LOCALIP - - [$LOGDATE$] "POST /.git/info/lfs/objects/batch HTTP/1.1" 404 - (glob)