fix some file modes in calls to withTmpFileIn to honor umask

Also audited for other calls to openTempFile, and all are ok,
except for viaTmp which will need further work.

Remote.Directory fixed to set umask mode when writing to an export,
although it has another one using viaTmp that's not fixed.
Will make exports that are published via a http server running as
another user work, for example.

Remote.BitTorrent fixed to set umask mode when downloading the torrent
file. Normally this does not matter as that file does not hang around
after the download, but if a bittorrent download were started by one user,
got interrupted and then another user ran it, this will let them access
the torrent file created by the first user.

diff --git a/Annex/Perms.hs b/Annex/Perms.hs
index fe01d13a2bffdf095745214bcd193402c29224bc..bd00d4885534672afc0ddc34eee21f7fb2038603 100644 (file)
--- a/Annex/Perms.hs
+++ b/Annex/Perms.hs
@@ -9,6 +9,7 @@ module Annex.Perms (
        FileMode,
        setAnnexFilePerm,
        setAnnexDirPerm,
+       resetAnnexFilePerm,
        annexFileMode,
        createAnnexDirectory,
        createWorkTreeDirectory,
@@ -42,23 +43,49 @@ setAnnexDirPerm = setAnnexPerm True
 
 {- Sets appropriate file mode for a file or directory in the annex,
  - other than the content files and content directory. Normally,
- - use the default mode, but with core.sharedRepository set,
+ - don't change the mode, but with core.sharedRepository set,
  - allow the group to write, etc. -}
 setAnnexPerm :: Bool -> FilePath -> Annex ()
-setAnnexPerm isdir file = unlessM crippledFileSystem $
+setAnnexPerm = setAnnexPerm' Nothing
+
+setAnnexPerm' :: Maybe ([FileMode] -> FileMode -> FileMode) -> Bool -> FilePath -> Annex ()
+setAnnexPerm' modef isdir file = unlessM crippledFileSystem $
        withShared $ liftIO . go
   where
-       go GroupShared = void $ tryIO $ modifyFileMode file $ addModes $
+       go GroupShared = void $ tryIO $ modifyFileMode file $ modef' $
                groupSharedModes ++
                if isdir then [ ownerExecuteMode, groupExecuteMode ] else []
-       go AllShared = void $ tryIO $ modifyFileMode file $ addModes $
+       go AllShared = void $ tryIO $ modifyFileMode file $ modef' $
                readModes ++
                [ ownerWriteMode, groupWriteMode ] ++
                if isdir then executeModes else []
-       go _ = noop
+       go _ = case modef of
+               Nothing -> noop
+               Just f -> void $ tryIO $
+                       modifyFileMode file $ f []
+       modef' = fromMaybe addModes modef
+
+resetAnnexFilePerm :: FilePath -> Annex ()
+resetAnnexFilePerm = resetAnnexPerm False
+
+{- Like setAnnexPerm, but ignores the current mode of the file entirely,
+ - and sets the same mode that the umask would result in when creating a
+ - new file.
+ -
+ - Useful eg, after creating a temporary file with locked down modes,
+ - which is going to be moved to a non-temporary location and needs
+ - usual modes.
+ -}
+resetAnnexPerm :: Bool -> FilePath -> Annex ()
+resetAnnexPerm isdir file = unlessM crippledFileSystem $ do
+       defmode <- liftIO defaultFileMode
+       let modef moremodes _oldmode = addModes moremodes defmode
+       setAnnexPerm' (Just modef) isdir file
 
 {- Gets the appropriate mode to use for creating a file in the annex
- - (other than content files, which are locked down more). -}
+ - (other than content files, which are locked down more). The umask is not
+ - taken into account; this is for use with actions that create the file
+ - and apply the umask automatically. -}
 annexFileMode :: Annex FileMode
 annexFileMode = withShared $ return . go
   where

diff --git a/Remote/BitTorrent.hs b/Remote/BitTorrent.hs
index ab1af9e2ab04c0c6ed676f5f64e2cdff6386a76c..4adde73dda46f7af206627795ea5e82dacee74d0 100644 (file)
--- a/Remote/BitTorrent.hs
+++ b/Remote/BitTorrent.hs
@@ -202,6 +202,7 @@ downloadTorrentFile u = do
                                else withOtherTmp $ \othertmp -> do
                                        withTmpFileIn othertmp "torrent" $ \f h -> do
                                                liftIO $ hClose h
+                                               resetAnnexFilePerm f
                                                ok <- Url.withUrlOptions $ 
                                                        Url.download nullMeterUpdate u f
                                                when ok $

diff --git a/Remote/Directory.hs b/Remote/Directory.hs
index 8d320c6707fc80e309aefa8f238d974a9b523fe3..377e6ce80fddc978386230a2c3743a800ce6bc67 100644 (file)
--- a/Remote/Directory.hs
+++ b/Remote/Directory.hs
@@ -31,6 +31,7 @@ import Remote.Helper.ExportImport
 import Types.Import
 import qualified Remote.Directory.LegacyChunked as Legacy
 import Annex.Content
+import Annex.Perms
 import Annex.UUID
 import Backend
 import Types.KeySource
@@ -436,6 +437,7 @@ storeExportWithContentIdentifierM dir src _k loc overwritablecids p = do
                liftIO $ withMeteredFile src p (L.hPut tmph)
                liftIO $ hFlush tmph
                liftIO $ hClose tmph
+               resetAnnexFilePerm tmpf
                liftIO (getFileStatus tmpf) >>= liftIO . mkContentIdentifier tmpf >>= \case
                        Nothing -> giveup "unable to generate content identifier"
                        Just newcid -> do

diff --git a/Utility/Tmp.hs b/Utility/Tmp.hs
index bc8af141075b8a04f6895921dbded07fc97d2404..905bd8c301a2f76e27b09d2b08247b871bca7cf1 100644 (file)
--- a/Utility/Tmp.hs
+++ b/Utility/Tmp.hs
@@ -30,6 +30,10 @@ type Template = String
 {- Runs an action like writeFile, writing to a temp file first and
  - then moving it into place. The temp file is stored in the same
  - directory as the final file to avoid cross-device renames.
+ -
+ - Note that the tmp file will have a file mode that only allows the
+ - current user to access it. The write action can change the mode
+ - to whatever is desired.
  -}
 viaTmp :: (MonadMask m, MonadIO m) => (FilePath -> v -> m ()) -> FilePath -> v -> m ()
 viaTmp a file content = bracketIO setup cleanup use
@@ -55,7 +59,11 @@ withTmpFile template a = do
        withTmpFileIn tmpdir template a
 
 {- Runs an action with a tmp file located in the specified directory,
- - then removes the file. -}
+ - then removes the file.
+ -
+ - Note that the tmp file will have a file mode that only allows the
+ - current user to access it.
+ -}
 withTmpFileIn :: (MonadIO m, MonadMask m) => FilePath -> Template -> (FilePath -> Handle -> m a) -> m a
 withTmpFileIn tmpdir template a = bracket create remove use
   where

diff --git a/doc/bugs/directory_special_remote_export_file_mode.mdwn b/doc/bugs/directory_special_remote_export_file_mode.mdwn
index 0773b9f307269313ccb15e501d2dd278076cef4d..2afc8575e1a1fe255e0227ee09388fff62101f8f 100644 (file)
--- a/doc/bugs/directory_special_remote_export_file_mode.mdwn
+++ b/doc/bugs/directory_special_remote_export_file_mode.mdwn
@@ -11,5 +11,11 @@ init, and some stuff in ~/.config/git-annex like autostart.
 `withTmpFileIn` also uses openTempFile, and probably its callers do need to
 adjust perms if desired since it could be used with a real temp directory.
 
+> Audited and fixed these. It affected only directory special remote and
+> bittorrent special remote if a download from it were interrupted and then
+> resumed by a different user than the one who started it. --[[Joey]]
+
 There are also a couple of other uses of openTempFile, which need to be
 audited for this problem. --[[Joey]]
+
+> Checked, all were ok. --[[Joey]]