safer git sha object filename

Rather than use the filename provided by INPUT, which could come from user
input, and so could be something that looks like a dashed parameter,
use a .git/object/<sha> filename.

This avoids user input passing through INPUT and back out, with the file
path then passed to a command, which could do something unexpected with
a dashed parameter, or other special parameter.

Added a note in the design about being careful of passing user input to
commands. They still have to be careful of that in general, just not in
this case.

diff --git a/Remote/Compute.hs b/Remote/Compute.hs
index 53f08c6cf954a2468a49488fcf1e991505fbb301..58e0ef6e8b18e21aba149ba4b8d8f490fe07b7f2 100644 (file)
--- a/Remote/Compute.hs
+++ b/Remote/Compute.hs
@@ -425,10 +425,9 @@ runComputeProgram (ComputeProgram program) state (ImmutableState immutablestate)
                                        Nothing -> pure Nothing
                                        Just (Right f'') -> liftIO $
                                                Just <$> relPathDirToFile subdir f''
-                                       Just (Left gitsha) -> do
-                                               liftIO . F.writeFile (subdir </> f')
-                                                       =<< catObject gitsha
-                                               return (Just f')
+                                       Just (Left gitsha) ->
+                                               Just <$> (liftIO . relPathDirToFile subdir 
+                                                       =<< populategitsha gitsha tmpdir)
                                liftIO $ hPutStrLn (stdinHandle p) $
                                        maybe "" fromOsPath mp
                                liftIO $ hFlush (stdinHandle p)
@@ -479,6 +478,17 @@ runComputeProgram (ComputeProgram program) state (ImmutableState immutablestate)
        calcduration (MonotonicTimestamp starttime) (MonotonicTimestamp endtime) =
                fromIntegral (endtime - starttime) :: NominalDiffTime
 
+       -- Writes to a .git/objects/ file in the tmpdir, rather than
+       -- using the input filename, to avoid exposing the input filename
+       -- to the program as a parameter, which could parse it as a dashed
+       -- option or other special parameter.
+       populategitsha gitsha tmpdir = do
+               let f = tmpdir </> ".git" </> "objects"
+                       </> toOsPath (Git.fromRef' gitsha)
+               liftIO $ createDirectoryIfMissing True $ takeDirectory f
+               liftIO . F.writeFile f =<< catObject gitsha
+               return f
+
 computationBehaviorChangeError :: ComputeProgram -> String -> OsPath -> Annex a
 computationBehaviorChangeError (ComputeProgram program) requestdesc p =
        giveup $ program ++ " is not behaving the same way it used to, now " ++ requestdesc ++ ": " ++ fromOsPath p

diff --git a/doc/design/compute_special_remote_interface.mdwn b/doc/design/compute_special_remote_interface.mdwn
index 8b62a601fad064a5c52d3a15b861c0f2b998bf9c..0dfd93e314e19b4101fa35e323b48bf8b40e9c59 100644 (file)
--- a/doc/design/compute_special_remote_interface.mdwn
+++ b/doc/design/compute_special_remote_interface.mdwn
@@ -23,7 +23,9 @@ that is in the form "foo=bar" will also result in an environment variable
 being set, eg `ANNEX_COMPUTE_passes=10` or `ANNEX_COMPUTE_--level=9`.
 
 For security, the program should avoid exposing user input to the shell
-unprotected, or otherwise executing it.
+unprotected, or otherwise executing it. And when running a command, make
+sure that whatever user input is passed to it can result in only safe and
+expected behavior.
 
 The program is run in a temporary directory, which will be cleaned up after
 it exits. Note that it may be run in a subdirectory of a temporary