Merge pull request #282 from sandydoo/fix-executable-perms

nar: fix executable permissions logic

Author: sorki

hnix-store-nar/hnix-store-nar.cabal

@@ -96,6 +96,7 @@ test-suite nar
     tasty-discover:tasty-discover
   build-depends:
       base
+    , cryptonite
     , hnix-store-nar
     , base64-bytestring
     , cereal

hnix-store-nar/src/System/Nix/Nar/Effects.hs

@@ -4,6 +4,8 @@ module System.Nix.Nar.Effects
   ( NarEffects(..)
   , narEffectsIO
   , IsExecutable(..)
+  , isExecutable
+  , setExecutable
   ) where
 
 import Control.Monad.Trans.Control (MonadBaseControl)
@@ -19,10 +21,21 @@ import qualified Data.ByteString
 import qualified Data.ByteString.Lazy        as Bytes.Lazy
 import qualified System.Directory            as Directory
 import           System.Posix.Files          ( createSymbolicLink
+                                             , fileMode
                                              , fileSize
+                                             , FileStatus
                                              , getFileStatus
+                                             , getSymbolicLinkStatus
+                                             , groupExecuteMode
+                                             , intersectFileModes
                                              , isDirectory
+                                             , isRegularFile
+                                             , nullFileMode
+                                             , otherExecuteMode
+                                             , ownerExecuteMode
                                              , readSymbolicLink
+                                             , setFileMode
+                                             , unionFileModes
                                              )
 import qualified System.IO                   as IO
 import qualified Control.Exception.Lifted    as Exception.Lifted
@@ -59,13 +72,13 @@ narEffectsIO = NarEffects {
     narReadFile   = liftIO . Bytes.Lazy.readFile
   , narWriteFile  = \f e c -> liftIO $ do
       Bytes.Lazy.writeFile f c
-      p <- Directory.getPermissions f
-      Directory.setPermissions f (p { Directory.executable = e == Executable })
+      Control.Monad.when (e == Executable) $
+        setExecutable f
   , narStreamFile = streamStringOutIO
   , narListDir    = liftIO . Directory.listDirectory
   , narCreateDir  = liftIO . Directory.createDirectory
   , narCreateLink = \f -> liftIO . createSymbolicLink f
-  , narIsExec     = liftIO . (fmap (bool NonExecutable Executable . Directory.executable)) . Directory.getPermissions
+  , narIsExec     = liftIO . fmap (bool NonExecutable Executable . isExecutable) . getSymbolicLinkStatus
   , narIsDir      = fmap isDirectory . liftIO . getFileStatus
   , narIsSymLink  = liftIO . Directory.pathIsSymbolicLink
   , narFileSize   = fmap (fromIntegral . fileSize) . liftIO . getFileStatus
@@ -102,10 +115,34 @@ streamStringOutIO f executable getChunk =
         liftIO $ Data.ByteString.hPut handle c
         go handle
   updateExecutablePermissions =
-    Control.Monad.when (executable == Executable) $ do
-      p <- Directory.getPermissions f
-      Directory.setPermissions f (p { Directory.executable = True })
+    Control.Monad.when (executable == Executable) $
+      setExecutable f
   cleanupException (e :: Exception.Lifted.SomeException) = do
     liftIO $ Directory.removeFile f
     Control.Monad.fail $
       "Failed to stream string to " <> f <> ": " <> show e
+
+-- | Check whether the file is executable by the owner.
+--
+--   Matches the logic used by Nix.
+--
+--   access() should not be used for this purpose on macOS.
+--   It returns false for executables when placed in certain directories.
+--   For example, when in an app bundle: App.app/Contents/Resources/en.lproj/myexecutable.strings
+isExecutable :: FileStatus -> Bool
+isExecutable st =
+  isRegularFile st
+    && fileMode st `intersectFileModes` ownerExecuteMode /= nullFileMode
+
+-- | Set the file to be executable by the owner, group, and others.
+--
+--   Matches the logic used by Nix.
+setExecutable :: FilePath -> IO ()
+setExecutable f = do
+  st <- getSymbolicLinkStatus f
+  let p =
+        fileMode st
+          `unionFileModes` ownerExecuteMode
+          `unionFileModes` groupExecuteMode
+          `unionFileModes` otherExecuteMode
+  setFileMode f p

hnix-store-nar/tests/NarFormat.hs

@@ -7,6 +7,7 @@ import           Control.Applicative              (many, optional, (<|>))
 import qualified Control.Concurrent               as Concurrent
 import           Control.Exception                (SomeException, try)
 import           Control.Monad                    (replicateM, void, forM_, when)
+import           Crypto.Hash                      (hash, Digest, SHA256)
 import           Data.Serialize                   (Serialize(..))
 import           Data.Serialize                   (Get, getByteString,
                                                    getInt64le,
@@ -35,6 +36,7 @@ import           System.Environment               (getEnv)
 import           System.FilePath                  ((<.>), (</>))
 import qualified System.IO                        as IO
 import qualified System.IO.Temp                   as Temp
+import qualified System.Posix.Files               as Unix
 import qualified System.Posix.Process             as Unix
 import qualified System.Process                   as P
 import           Test.Tasty                       as T
@@ -142,11 +144,19 @@ unit_nixStoreDirectory = filesystemNixStore "directory" (Nar sampleDirectory)
 unit_nixStoreDirectory' :: HU.Assertion
 unit_nixStoreDirectory' = filesystemNixStore "directory'" (Nar sampleDirectory')
 
+-- | Test that the executable permissions are handled correctly in app bundles on macOS.
+--   In this case, access() returns false for a file under this specific path, even when the executable bit is set.
+--   NAR implementations should avoid this syscall on macOS.
+test_nixStoreMacOSAppBundle :: TestTree
+test_nixStoreMacOSAppBundle = packThenExtract "App.app" $ \ baseDir -> do
+  let testDir = baseDir </> "App.app" </> "Resources" </> "en.lproj"
+  Directory.createDirectoryIfMissing True testDir
+  mkExecutableFile (testDir </> "test.strings")
+
 test_nixStoreBigFile :: TestTree
 test_nixStoreBigFile = packThenExtract "bigfile" $ \baseDir -> do
   mkBigFile (baseDir </> "bigfile")
 
-
 test_nixStoreBigDir :: TestTree
 test_nixStoreBigDir = packThenExtract "bigdir" $ \baseDir -> do
   let testDir = baseDir </> "bigdir"
@@ -350,7 +360,16 @@ packThenExtract testName setup =
         IO.withFile hnixNarFile IO.WriteMode $ \h ->
           buildNarIO narEffectsIO narFilePath h
 
-        -- BSL.writeFile hnixNarFile narBS
+        -- Compare the hash digests of the two NARs
+        nixHash :: Digest SHA256 <- hash <$> BS.readFile nixNarFile
+        hnixHash :: Digest SHA256 <- hash <$> BS.readFile hnixNarFile
+        step $ unlines
+          [ "Compare SHA256 digests between NARs:"
+          , "  nix: "  <> show nixHash
+          , "  hnix: " <> show hnixHash
+          ]
+
+        HU.assertEqual "Hash mismatch between NARs" nixHash hnixHash
 
         step $ "Unpack NAR to " <> outputFile
         _narHandle <- IO.withFile nixNarFile IO.ReadMode $ \h ->
@@ -567,6 +586,12 @@ mkBigFile path = do
   fsize <- getBigFileSize
   BSL.writeFile path (BSL.take fsize $ BSL.cycle "Lorem ipsum")
 
+mkExecutableFile :: FilePath -> IO ()
+mkExecutableFile path = do
+  BSL.writeFile path ""
+  st <- Unix.getSymbolicLinkStatus path
+  let p = Unix.fileMode st `Unix.unionFileModes` Unix.ownerExecuteMode
+  Unix.setFileMode path p
 
 -- | Construct FilePathPart from Text by checking that there
 --   are no '/' or '\\NUL' characters