From a2fc471e14a9fcccb15e5265f6964b8a71b0399a Mon Sep 17 00:00:00 2001
From: Joey Hess <joeyh@joeyh.name>
Date: Tue, 4 Mar 2025 14:54:13 -0400
Subject: [PATCH] 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.
---
 Remote/Compute.hs                              | 18 ++++++++++++++----
 .../compute_special_remote_interface.mdwn      |  4 +++-
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/Remote/Compute.hs b/Remote/Compute.hs
index 53f08c6cf9..58e0ef6e8b 100644
--- 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 8b62a601fa..0dfd93e314 100644
--- 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