Skip to content

Commit d5f02fe

Browse files
committed
Added improvements suggested in the discussion of PR #2448:
* Added `DirectoryTreeOptions` type that allows one to control the behaviout or to-directory-tree * Added command line flags for these options: --allow-parent-directory and --allow-absolute-paths * Simplified code a bit
1 parent 2595454 commit d5f02fe

File tree

4 files changed

+87
-43
lines changed

4 files changed

+87
-43
lines changed

dhall/src/Dhall/DirectoryTree.hs

Lines changed: 48 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@
1010
-- | Implementation of the @dhall to-directory-tree@ subcommand
1111
module Dhall.DirectoryTree
1212
( -- * Filesystem
13-
toDirectoryTree
13+
DirectoryTreeOptions(..)
14+
, defaultDirectoryTreeOptions
15+
, toDirectoryTree
1416
, FilesystemError(..)
1517

1618
-- * Low-level types and functions
@@ -38,7 +40,7 @@ import Dhall.Syntax
3840
, RecordField (..)
3941
, Var (..)
4042
)
41-
import System.FilePath ((</>))
43+
import System.FilePath ((</>), isAbsolute, splitDirectories, takeDirectory)
4244
import System.PosixCompat.Types (FileMode, GroupID, UserID)
4345

4446
import qualified Control.Exception as Exception
@@ -63,11 +65,31 @@ import qualified System.Posix.User as Posix
6365
#endif
6466
import qualified System.PosixCompat.Files as Posix
6567

68+
{- | Options affecting the interpretation of a directory tree specification.
69+
-}
70+
data DirectoryTreeOptions = DirectoryTreeOptions
71+
{ allowAbsolute :: Bool
72+
-- ^ Whether to allow absolute paths in the spec.
73+
, allowParent :: Bool
74+
-- ^ Whether to allow ".." in the spec.
75+
, allowSeparators :: Bool
76+
-- ^ Whether to allow path separators in file names.
77+
}
78+
79+
-- | The default 'DirectoryTreeOptions'. All flags are set to 'False'.
80+
defaultDirectoryTreeOptions :: DirectoryTreeOptions
81+
defaultDirectoryTreeOptions = DirectoryTreeOptions
82+
{ allowAbsolute = False
83+
, allowParent = False
84+
, allowSeparators = False
85+
}
86+
6687
{-| Attempt to transform a Dhall record into a directory tree where:
6788
6889
* Records are translated into directories
6990
70-
* @Map@s are translated into directory trees, if allowSeparators option is enabled
91+
* @Map@s are translated into directories or whole directory trees, if
92+
`allowSeparators` option is enabled
7193
7294
* @Text@ values or fields are translated into files
7395
@@ -122,10 +144,11 @@ import qualified System.PosixCompat.Files as Posix
122144
/Construction of directory trees from maps/
123145
124146
In @Map@s, the keys specify paths relative to the work dir.
125-
Only forward slashes (@/@) must be used as directory separators.
126-
They will be automatically transformed on Windows.
127-
Absolute paths (starting with @/@) and parent directory segments (@..@)
128-
are prohibited for security concerns.
147+
Absolute paths and parent directory segments (@..@) are by default
148+
prohibited for security concerns, but these checks can be disabled using
149+
`allowAbsolute` option field (or the @--allow-absolute-paths@ CLI flag) and
150+
`allowParent` option field (or the @--allow-parent-directory@ CLI flag)
151+
respectively.
129152
130153
/Advanced construction of directory trees/
131154
@@ -180,16 +203,16 @@ import qualified System.PosixCompat.Files as Posix
180203
that cannot be converted as-is.
181204
-}
182205
toDirectoryTree
183-
:: Bool -- ^ Whether to allow path separators in file names or not
206+
:: DirectoryTreeOptions
184207
-> FilePath
185208
-> Expr Void Void
186209
-> IO ()
187-
toDirectoryTree allowSeparators path expression = case expression of
210+
toDirectoryTree options path expression = case expression of
188211
RecordLit keyValues ->
189212
Map.unorderedTraverseWithKey_ process $ recordFieldValue <$> keyValues
190213

191214
ListLit (Just (App List (Record [ ("mapKey", recordFieldValue -> Text), ("mapValue", _) ]))) [] ->
192-
Directory.createDirectoryIfMissing allowSeparators path
215+
Directory.createDirectoryIfMissing (allowSeparators options) path
193216

194217
ListLit _ records
195218
| not (null records)
@@ -200,10 +223,10 @@ toDirectoryTree allowSeparators path expression = case expression of
200223
Text.IO.writeFile path text
201224

202225
Some value ->
203-
toDirectoryTree allowSeparators path value
226+
toDirectoryTree options path value
204227

205228
App (Field (Union _) _) value -> do
206-
toDirectoryTree allowSeparators path value
229+
toDirectoryTree options path value
207230

208231
App None _ ->
209232
return ()
@@ -214,7 +237,8 @@ toDirectoryTree allowSeparators path expression = case expression of
214237
Lam _ _ (Lam _ _ _) -> do
215238
entries <- decodeDirectoryTree expression
216239

217-
processFilesystemEntryList allowSeparators path entries
240+
-- TODO: Support allowAbsolute and allowParent as well ?
241+
processFilesystemEntryList (allowSeparators options) path entries
218242

219243
_ ->
220244
die
@@ -230,31 +254,24 @@ toDirectoryTree allowSeparators path expression = case expression of
230254
empty
231255

232256
process key value = do
257+
let
258+
keyPath = Text.unpack key
259+
keyPathSegments = splitDirectories keyPath
260+
path' = path </> keyPath
261+
233262
-- Fail if path is absolute, which is a security risk.
234-
when (FilePath.isAbsolute (Text.unpack key)) die
235-
236-
let keyPathSegments =
237-
fmap Text.unpack $ Text.splitOn "/" key
263+
when (not (allowAbsolute options) && isAbsolute keyPath) die
238264

239265
-- Fail if path contains attempts to go to container directory,
240266
-- which is a security risk.
241-
when (elem ".." keyPathSegments) die
242-
243-
(dirPathSegments, fileName) <- case reverse keyPathSegments of
244-
h : t ->
245-
return (reverse t, h)
246-
_ ->
247-
die
248-
249-
-- Fail if separators are not allowed by the option but we have directories in the path.
250-
when (not allowSeparators && not (null dirPathSegments)) die
267+
when (not (allowParent options) && ".." `elem` keyPathSegments) die
251268

252-
let dirPath =
253-
Foldable.foldl' (</>) path dirPathSegments
269+
-- Fail if separators are not allowed by the option.
270+
when (not (allowSeparators options) && length keyPathSegments > 1) die
254271

255-
Directory.createDirectoryIfMissing True dirPath
272+
Directory.createDirectoryIfMissing (allowSeparators options) (takeDirectory path')
256273

257-
toDirectoryTree allowSeparators (dirPath </> fileName) value
274+
toDirectoryTree options path' value
258275

259276
die = Exception.throwIO FilesystemError{..}
260277
where

dhall/src/Dhall/Main.hs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ data Mode
163163
| Encode { file :: Input, json :: Bool }
164164
| Decode { file :: Input, json :: Bool, quiet :: Bool }
165165
| Text { file :: Input, output :: Output }
166-
| DirectoryTree { allowSeparators :: Bool, file :: Input, path :: FilePath }
166+
| DirectoryTree { directoryTreeOptions :: DirectoryTree.DirectoryTreeOptions, file :: Input, path :: FilePath }
167167
| Schemas { file :: Input, outputMode :: OutputMode, schemas :: Text }
168168
| SyntaxTree { file :: Input, noted :: Bool }
169169
| Package
@@ -277,7 +277,7 @@ parseMode =
277277
Generate
278278
"to-directory-tree"
279279
"Convert nested records of Text literals into a directory tree"
280-
(DirectoryTree <$> parseDirectoryTreeAllowSeparators <*> parseFile <*> parseDirectoryTreeOutput)
280+
(DirectoryTree <$> parseDirectoryTreeOptions <*> parseFile <*> parseDirectoryTreeOutput)
281281
<|> subcommand
282282
Interpret
283283
"resolve"
@@ -546,8 +546,16 @@ parseMode =
546546
<> Options.Applicative.metavar "EXPR"
547547
)
548548

549-
parseDirectoryTreeAllowSeparators =
550-
Options.Applicative.switch
549+
parseDirectoryTreeOptions = DirectoryTree.DirectoryTreeOptions
550+
<$> Options.Applicative.switch
551+
( Options.Applicative.long "allow-absolute-paths"
552+
<> Options.Applicative.help "Whether to allow absolute file paths"
553+
)
554+
<*> Options.Applicative.switch
555+
( Options.Applicative.long "allow-parent-directory"
556+
<> Options.Applicative.help "Whether to allow references to the parent directory (\"..\") in file paths"
557+
)
558+
<*> Options.Applicative.switch
551559
( Options.Applicative.long "allow-path-separators"
552560
<> Options.Applicative.help "Whether to allow path separators in file names"
553561
)
@@ -1043,7 +1051,7 @@ command (Options {..}) = do
10431051

10441052
let normalizedExpression = Dhall.Core.normalize resolvedExpression
10451053

1046-
DirectoryTree.toDirectoryTree allowSeparators path normalizedExpression
1054+
DirectoryTree.toDirectoryTree directoryTreeOptions path normalizedExpression
10471055

10481056
Dhall.Main.Schemas{..} ->
10491057
Dhall.Schemas.schemasCommand Dhall.Schemas.Schemas{ input = file, ..}

dhall/tests/Dhall/Test/DirectoryTree.hs

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ tests = testGroup "to-directory-tree"
3333
, fixpointedUserGroup
3434
#endif
3535
]
36+
, testGroup "path separators"
37+
[ issue1305
38+
]
3639
]
3740

3841
fixpointedType :: TestTree
@@ -48,14 +51,14 @@ fixpointedEmpty :: TestTree
4851
fixpointedEmpty = testCase "empty" $ do
4952
let outDir = "./tests/to-directory-tree/fixpoint-empty.out"
5053
path = "./tests/to-directory-tree/fixpoint-empty.dhall"
51-
entries <- runDirectoryTree False outDir path
54+
entries <- runDirectoryTree defaultDirectoryTreeOptions outDir path
5255
entries @?= [Directory outDir]
5356

5457
fixpointedSimple :: TestTree
5558
fixpointedSimple = testCase "simple" $ do
5659
let outDir = "./tests/to-directory-tree/fixpoint-simple.out"
5760
path = "./tests/to-directory-tree/fixpoint-simple.dhall"
58-
entries <- runDirectoryTree False outDir path
61+
entries <- runDirectoryTree defaultDirectoryTreeOptions outDir path
5962
entries @?= Data.List.sort
6063
[ Directory outDir
6164
, File $ outDir </> "file"
@@ -66,7 +69,10 @@ fixpointedAllowPathSeparators :: TestTree
6669
fixpointedAllowPathSeparators = testCase "allow-path-separators" $ do
6770
let outDir = "./tests/to-directory-tree/fixpoint-allow-path-separators.out"
6871
path = "./tests/to-directory-tree/fixpoint-allow-path-separators.dhall"
69-
entries <- runDirectoryTree True outDir path
72+
options = defaultDirectoryTreeOptions
73+
{ allowSeparators = True
74+
}
75+
entries <- runDirectoryTree options outDir path
7076
entries @?= Data.List.sort
7177
[ Directory outDir
7278
, Directory $ outDir </> "non-existent-1"
@@ -84,7 +90,7 @@ fixpointedPermissions :: TestTree
8490
fixpointedPermissions = testCase "permissions" $ do
8591
let outDir = "./tests/to-directory-tree/fixpoint-permissions.out"
8692
path = "./tests/to-directory-tree/fixpoint-permissions.dhall"
87-
entries <- runDirectoryTree False outDir path
93+
entries <- runDirectoryTree defaultDirectoryTreeOptions outDir path
8894
entries @?=
8995
[ Directory outDir
9096
, File $ outDir </> "file"
@@ -115,8 +121,20 @@ fixpointedUserGroup = testCase "user and group" $ do
115121
}
116122
]
117123

118-
runDirectoryTree :: Bool -> FilePath -> FilePath -> IO [WalkEntry]
119-
runDirectoryTree allowSeparators outDir path = do
124+
issue1305 :: TestTree
125+
issue1305 = testCase "separators in map keys" $ do
126+
let outDir = "./tests/to-directory-tree/T1305.out"
127+
path = "./tests/to-directory-tree/T1305.dhall"
128+
opts = defaultDirectoryTreeOptions { allowSeparators = True }
129+
entries <- runDirectoryTree opts outDir path
130+
entries @?=
131+
[ Directory outDir
132+
, Directory $ outDir </> "A"
133+
, File $ outDir </> "A/B"
134+
]
135+
136+
runDirectoryTree :: DirectoryTreeOptions -> FilePath -> FilePath -> IO [WalkEntry]
137+
runDirectoryTree opts outDir path = do
120138
doesOutDirExist <- Directory.doesDirectoryExist outDir
121139
when doesOutDirExist $
122140
Directory.removeDirectoryRecursive outDir
@@ -129,7 +147,7 @@ runDirectoryTree allowSeparators outDir path = do
129147
$ Dhall.defaultInputSettings
130148
expr <- Dhall.inputExprWithSettings inputSettings text
131149

132-
toDirectoryTree allowSeparators outDir $ Dhall.Core.denote expr
150+
toDirectoryTree opts outDir $ Dhall.Core.denote expr
133151

134152
Data.List.sort <$> walkFsTree outDir
135153

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[ { mapKey = "A/B", mapValue = "" } ]

0 commit comments

Comments
 (0)